fix(server): atomic writes keep the file's permission bits

[Mnigos]

Fixes #14967.

Problem

writeFileStringAtomically writes a temp file and renames it over the target. The temp file gets Node's default mode (0o666 masked by the umask) and the rename replaces the inode, so whatever mode the target had is lost: a settings.json the user tightened to 0600 comes back 0644 after the next save. settings.json can hold plaintext secrets (providers.opencode.serverPassword, the Antigravity apiKey), and ~/.t3/userdata is 0755, so after one save the file is readable by any local user who can traverse the home directory.

Change

Before writing, the helper stats the resolved target (the symlink destination that #15009 already resolves for the rename) and keeps mode & 0o777 (Node's stat.mode carries the file-type bits); a target that does not exist yet gets 0o600. Reading the mode from the resolved target means a symlinked settings.json stays a link and its referent keeps its bits. The mode is applied to the temp file with chmod before the rename, so the replacement never exists at the target path with a wider mode. chmod rather than a mode option on the write, because the write mode is masked by the umask (a 0664 file would come back 0644); the temp directory comes from mkdtemp and is 0700, so the file is private until then.

Nothing else changes: same temp-directory-plus-rename shape, no fsync (the triage puts crash durability outside this fix). Every file this helper writes (settings.json, keybindings.json, server-runtime.json, the provider status cache, the ACP sign-in flag, and settings.json from cli/theme.ts) is read by processes of the same user, including the SSH tunnel and the user-level boot service, so 0600 for a new file loses nothing. On Windows chmod only toggles the read-only attribute and 0600 keeps the write bit, so normal files behave as before.

Scope and approval

Triaged bug #14967 (security), following the triage comment: preserve the existing permission bits, use 0600 when the file does not exist, mask stat.mode to 0o777; directory fsync is separate hardening. One function, no caller change.

Verification

Observed result. The real helper from main (bb79977) and from this branch, each run against fresh files with umask 022 (stat -f '%Sp'):

Fixture	main	this branch
Existing file chmodded 0600, then saved	-rw-r--r-- (0644)	-rw------- (0600)
Missing file, then saved	-rw-r--r-- (0644)	-rw------- (0600)

Contents were the new input in all four runs and no temp directory was left behind.

Tests. atomicWrite.test.ts on the real file system. Added cases: an existing 0600, 0644 and 0664 file keeps its mode and gets the new contents (the 0664 case fails if the write-mode option is used instead of chmod); a missing file, and a missing nested parent, come out 0600; contents are byte-identical including a trailing newline and non-ASCII text; a successful write leaves only the target in the directory; a read-only parent fails with the same PlatformError as before, keeps the original file and leaves no temp directory (skipped as root; mode assertions skipped on Windows). The existing symlink tests from #15009 now also assert permissions: an existing link's 0600 referent and a relative link's 0664 referent keep their bits, and a dangling link's new referent and a file in a missing directory come out 0600. 14 tests pass in the file (6 symlink tests from #15009 plus the 8 above), and 103 across the atomicWrite, serverSettings, keybindings and serverRuntimeState test files; against the main helper the mode assertions fail (0644); typecheck, targeted lint, format and knip were re-run after the rebase onto #15009. Server typecheck, targeted lint, format and knip are clean.

Not checked: Windows at runtime.

Implemented with Claude Code (Claude Opus 5.5, coordinated by Claude Fable 5.1); tests, independent review and the observed result by GPT-6 Astra via Codex.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The shared atomic-write helper now changes production file permissions, preserving existing modes and defaulting new files to owner-only access for potentially sensitive data. The PR also adds a file-level static-analysis suppression directive, so the security-sensitive behavior and diagnostic override warrant human review.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 317b9d1d-1849-4d45-a150-9d5a4772d9b7

📥 Commits

Reviewing files that changed from the base of the PR and between 8b694ac and 68d4107.

📒 Files selected for processing: 1

• packages/shared/src/atomicWrite.test.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.

📝 Walkthrough

writeFileStringAtomically now preserves existing target permission bits and uses mode 0600 for new files. Tests cover permissions, exact UTF-8 contents, temporary-directory cleanup, and failure behavior.

Changes: Atomic write permissions

Layer / File(s)	Summary
Select and apply file permissions packages/shared/src/atomicWrite.ts	The function reads the target’s permission bits, uses 0600 when the target is absent, and applies the selected mode to the temporary file before renaming it.
Verify permissions and write outcomes packages/shared/src/atomicWrite.test.ts	Tests check preservation of existing modes, 0600 for new files, exact UTF-8 contents, cleanup after success, and failure behavior. Permission checks are skipped on Windows; the read-only-parent test also skips when run as root.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 68d41

Atomic writes preserve existing permission modes and use 0600 for new files. No concrete merge-blocking regression is evident; the change is ready for normal checks.

Security architecture risk: 🔵 Low · up to 68d41

The change strengthens protection for secret-bearing files by applying permissions before replacement. No introduced security regression was established, but concurrent permission changes, Windows behavior, and deployment account and access-control assumptions remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The relevant confidentiality boundary is access to files written under the host process's filesystem authority. Inspected consumers derive paths from application state/cache configuration, including secret-bearing settings and authentication state. No new attacker-controlled path input or identity transition was identified in those call sites, but complete external reachability and deployment access permissions remain unproven.

Security Findings and Attack Paths

• inferred — A concurrent actor able to change the target or its directory can invalidate the selected path or permission snapshot before rename. This is not an established new attack path: the base already resolved then replaced the target without synchronization and used default replacement permissions. Whether the new snapshot worsens a particular deployment's exposure depends on its previous mode, umask, concurrent actors, and filesystem policy.

Trust Boundaries and Controls

• observed — Permission selection and chmod complete before publication. A non-NotFound stat error or a failure during directory creation, staging, or chmod stops the sequence before rename. Unix tests cover preserved modes and owner-only new files, while Windows permission assertions are skipped; Unix mode results therefore do not establish Windows access-control guarantees.

Resilience and Maintainability Implications

• observed — The existing staged-replacement and cleanup shape is retained. Tests check an unchanged target after read-only-parent failure and successful completion despite cleanup failure. Scoped cleanup does not establish recovery after process termination; crash durability and orphan cleanup are unverified, pre-existing limitations rather than demonstrated regressions from this permission change.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #14967 requires permission preservation or a restrictive default for new files, and crash durability with temp-file fsync, rename, and parent-directory fsync. atomicWrite.ts now applies `stat.…	Add an fsync for the temporary file before rename and an fsync for the parent directory after rename. Add automated tests that verify the durability sequence or its observable failure behavior.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main change: preserving file permission bits during atomic writes.
Description check		The description includes all required sections: Problem, Change, Scope and approval, and Verification. It provides reproduction context, implementation details, linked issue approval, test results, li…
Out of Scope Changes check		The changes modify writeFileStringAtomically and its tests. Permission preservation, restrictive defaults, symlink referent behavior, exact content, and failure cleanup directly support issue #14967…

Full details: Linked Issues check

Explanation

Issue #14967 requires permission preservation or a restrictive default for new files, and crash durability with temp-file fsync, rename, and parent-directory fsync. atomicWrite.ts now applies stat.mode & 0o777 or 0o600 with chmod before rename. The added tests cover these permission outcomes. The diff adds no fsync for the temporary file or parent directory, so the durability requirement remains unmet.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}