Skip to content

fix(core): restrict Dev database data dir and state files to owner - #1775

Merged
borisno2 merged 3 commits into
mainfrom
fix/issue-1659-dev-db-permissions
Oct 7, 2026
Merged

fix(core): restrict Dev database data dir and state files to owner#1775
borisno2 merged 3 commits into
mainfrom
fix/issue-1659-dev-db-permissions

Conversation

@borisno2

@borisno2 borisno2 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Summary

  • Data directory created at 0700 and tightened on boot if it already exists; a chmod failure throws an error naming the path
  • State file (including its temp file) and lock file written at 0600
  • No-op on Windows; mode tests skip on win32
  • Patch changeset for @opensaas/stack-core

Password enforcement is out of scope (#1758).

Test plan

  • Mode tests for fresh dir, pre-existing 0755 dir, state file
  • pnpm lint, typecheck pass

Closes #1659

🤖 Generated with Claude Code

https://claude.ai/code/session_019x6FTsyUiZqVBAitcJQQkW

Generated by Claude Code

@changeset-bot

changeset-bot Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 60e0cca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@opensaas/stack-core Patch
@opensaas/stack-auth Patch
@opensaas/stack-cli Patch
@opensaas/stack-rag Patch
@opensaas/stack-storage Patch
@opensaas/stack-tiptap Patch
@opensaas/stack-ui Patch
@opensaas/stack-storage-s3 Patch
@opensaas/stack-storage-vercel Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
stack-docs Ready Ready Preview Oct 7, 2026 11:35am UTC

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T11:33:12.609958Z 1cfda78 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@borisno2 Claude

borisno2 commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Review of #1775 (fixes #1659: Dev database file permissions)

The change does what the issue asks. The data dir is 0700, and the state and lock files are 0600. The changeset is present and is a patch. The tests cover a fresh dir, a pre-existing 0755 dir and the state-file write. I did not run the suite. This review is from reading the diff. I found nothing that blocks the PR, but I'd like the first item addressed before merge.

Requested changes

  1. restrictDataDir makes a chmod failure fatal (packages/core/src/db/dev-database.ts, ~L248).
    • chmodSync can fail with EPERM or EROFS on a pre-existing dataDir the user doesn't own. Examples are a Docker bind mount, a root-owned shared volume, or a FAT/NTFS mount.
    • That setup used to start. Now startDevDatabase throws Could not restrict ....
    • Please warn and continue for EPERM, EROFS and ENOTSUP, and throw only on unexpected errors. Alternatively, keep it fatal only when the dir was freshly created by this call.
    • There is currently no test for this error branch. Please add one.

Suggestions (non-blocking)

  1. State-file mode depends on the temp file being new (packages/core/src/db/state-file.ts, ~L99).

    • mode: 0o600 on writeFileSync applies only when the file is created. A stale <state>.<pid>.tmp left at 0644 by a crashed process with the same pid would be renamed into place world-readable.
    • The mkdirSync(directory, { recursive: true }) just above also leaves .opensaas/ at umask defaults.
    • An explicit chmodSync(temporary, 0o600) after the write, or flag: 'wx' after removing any stale temp file, makes the guarantee unconditional. This scenario is narrow.

  2. Scope of the hardening. This closes the file-permission channel only.

  3. The mode: 0o600 on the lock file is redundant. It sits inside a dir already forced to 0700. It is harmless defence in depth. If you keep it, the test that asserts it is fine.

  4. Test portability. The permission tests assert exact modes and skip only on win32. Filesystems that don't honour POSIX modes (some bind mounts, WSL drvfs) would fail them. Consider also skipping when a probe chmod doesn't round-trip.

Generated by Claude Code

@claude
fix(core): chmod state file temp before rename
5ecef1d 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019x6FTsyUiZqVBAitcJQQkW
@borisno2 Claude

borisno2 commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Addressed the review:

Generated by Claude Code

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 7, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cfda78251

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/core/src/db/dev-database.test.ts Outdated

const modeOf = (file: string) => statSync(file).mode & 0o777

test('a fresh data directory is 0700 and the state and lock files are 0600', async () => {

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Give the database-start tests the boot timeout

On this Linux checkout, pnpm exec vitest run src/db/state-file.test.ts src/db/dev-database.test.ts makes both new asynchronous permission tests exceed Vitest's 5-second default timeout—the starts took approximately 7.7 and 6.0 seconds—while the existing boot tests use BOOT_TIMEOUT = 60_000. Because the tests at lines 94 and 106 await a full startDevDatabase without that timeout, the core suite fails on slower CI or development hosts; pass BOOT_TIMEOUT to both tests and make the constant available before this describe block.

Useful? React with 👍 / 👎.

@borisno2 borisno2 Oct 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 60e0cca: both async permission tests now pass BOOT_TIMEOUT.

Generated by Claude Code

@claude
test(core): give permission boot tests the boot timeout
60e0cca 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019x6FTsyUiZqVBAitcJQQkW
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Core Package Coverage (./packages/core)

Status Category Percentage Covered / Total
🟢 Lines 94.5% (🎯 81%) 3819 / 4041
🟢 Statements 92.79% (🎯 76%) 4342 / 4679
🟢 Functions 96.09% (🎯 78%) 862 / 897
🟢 Branches 88.34% (🎯 71%) 2949 / 3338
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/core/src/db/dev-database.ts 88.54% 81.63% 94.44% 90.8% 133, 136, 138, 149, 175, 179-185, 194, 200, 221, 257
packages/core/src/db/state-file.ts 92.1% 95.45% 100% 93.75% 64, 103-104
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for UI Package Coverage (./packages/ui)

Status Category Percentage Covered / Total
🔵 Lines 78.7% 244 / 310
🔵 Statements 78.43% 251 / 320
🔵 Functions 69.81% 74 / 106
🔵 Branches 67.51% 160 / 237
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for CLI Package Coverage (./packages/cli)

Status Category Percentage Covered / Total
🔵 Lines 82.18% 2044 / 2487
🔵 Statements 81.94% 2196 / 2680
🔵 Functions 87.28% 350 / 401
🔵 Branches 75.44% 1100 / 1458
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Auth Package Coverage (./packages/auth)

Status Category Percentage Covered / Total
🔵 Lines 83.33% 405 / 486
🔵 Statements 82.19% 457 / 556
🔵 Functions 86.2% 100 / 116
🔵 Branches 78.28% 375 / 479
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Storage Package Coverage (./packages/storage)

Status Category Percentage Covered / Total
🔵 Lines 96.97% 417 / 430
🔵 Statements 96.02% 459 / 478
🔵 Functions 98.36% 120 / 122
🔵 Branches 93.43% 427 / 457
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for RAG Package Coverage (./packages/rag)

Status Category Percentage Covered / Total
🔵 Lines 88.79% 634 / 714
🔵 Statements 88.19% 695 / 788
🔵 Functions 95.48% 127 / 133
🔵 Branches 85.03% 449 / 528
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)

Status Category Percentage Covered / Total
🔵 Lines 100% 47 / 47
🔵 Statements 100% 48 / 48
🔵 Functions 100% 10 / 10
🔵 Branches 96.87% 31 / 32
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)

Status Category Percentage Covered / Total
🔵 Lines 100% 74 / 74
🔵 Statements 100% 78 / 78
🔵 Functions 100% 16 / 16
🔵 Branches 96.55% 56 / 58
File CoverageNo changed files found.
Generated in workflow #2989 for commit 60e0cca by the Vitest Coverage Report Action

@borisno2
borisno2 merged commit c1b689a into main Oct 7, 2026
9 checks passed
@borisno2
borisno2 deleted the fix/issue-1659-dev-db-permissions branch October 7, 2026 20:12

This branch was successfully deployed

1 active deployment
Preview — 60e0ccae Deployed Oct 7, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dev database accepts any credentials as superuser, and its data directory is world-readable

2 participants

@borisno2 @claude