Repository navigation
Conversation
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
fbba66b7-1563-4191-9a2b-5a02beb49362
📒 Files selected for processing: 3
src/main/durable-file-write.tssrc/main/usage-cache-snapshot-writer.test.tssrc/main/usage-cache-snapshot-writer.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| await writer.flush() | ||
|
|
||
| // Why: checking the open mode also catches omissions when the runner's umask is 077. | ||
| expect(tempWrites).toEqual([{ creationMode: 0o600, actualMode: 0o600 }]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Account for umask in the private-file assertions.
If the POSIX process umask removes an owner bit, a file created with mode 0o600 will not have mode 0o600. For example, umask 0o200 produces mode 0o400. This assertion, and the later private-file mode assertions, then fail even though the writer honors the requested creation mode. Compare actualMode and the final file modes with 0o600 & ~process.umask(); keep the creationMode assertion at 0o600. Node applies the mode when it creates the file. (nodejs.org)
ELI5
Usage caches can contain session and project metadata. When a snapshot replaces a file, its new temporary file currently inherits the process's default creation permissions. Request owner-only permissions for new cache snapshots so a successful save does not turn a
0600file into0644under the tested umask022conditions.What Changed
modeargument towriteFileDurableIfCurrentand forward it towriteTempFileDurable.0o600only from the shared providerUsageCacheSnapshotWriter.Why
The cache writer can state its intended permissions at temporary-file creation, before the file is published. This keeps the change small and avoids changing every durable writer or chmodding userData and existing unrelated files. It is a storage-permissions improvement; actual cross-user exposure has not been established.
Linked Issue
Fixes #31026
Visual Proof
N/A — file creation permissions only; no visual or interaction change.
Testing
On macOS arm64, Node 24.17.0, using the repository's Vitest configuration:
All 41 related tests pass, including eight added cases. Before the fix, six of the eight added cases fail. Node type checking (
tc:node --compiler=native), the changed-code-quality gate, scoped oxlint, oxfmt, andgit diff --checkalso pass. The prepared patch was rechecked on main snapshotba6599eabbdfed02eb7954c174bea704f853ecdc; the three submitted files are byte-identical to the previously tested patch. Existing isolated dependencies were reused; the intervening lockfile change adds a node-gyp root declaration and does not alter the tools used by these checks.Separate synthetic filesystem checks exercised umask
022/077, initial creation, replacement of existing0600/0644files, and0700/0755parents. The regression tests do not modify the runner's umask and also inspect the requested creation mode, so an existing restrictive umask cannot hide its omission.Not run: full repository suite, aggregate
pnpm lint/pnpm typecheck, full app build, Bun runtime, application/real agent, Linux, Windows, or SSH. Full cross-platform CI remains outstanding; no claim that it has passed.AI Disclosure
OpenAI Codex (GPT-6) assisted with source review, implementation, synthetic tests, and this description. The investigation used public source and synthetic data only.
Review
AI code review: the optional argument leaves existing callers unchanged. Node path APIs remain portable; POSIX assertions are skipped on Windows and
0o600is not represented as a Windows ACL guarantee. The change runs on the host writing the snapshot, makes no local/SSH path or transport assumptions, and is shared across provider usage caches. It adds no scan, chmod sweep, extra write, or UI work. No provider-specific commands, credentials, integration settings, or git-provider behavior change. Existing temporary-file and directory access controls are not expanded by this patch.Agent skill upstream boundary
Notes
Creation mode applies to a new temporary inode and remains subject to umask/platform behavior; it does not chmod a pre-existing temporary file. Existing cache files are updated only after a successful replacement write. Parent modes and untouched files are unchanged. Actual userData/ancestor permissions, ACLs, and cross-user access were not inspected. This does not claim a confirmed vulnerability and is separate from terminal-history PR #19954.
Author
GitHub: @ka2umaz. X/Twitter handle: not provided.
Checklist
pnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass — outstanding for CI; only scoped checks above ran locally