[Bug]: writeFileStringAtomically widens a file's mode on replace and is not crash-durable #14967

Activity

1. juliusmarminge commented on Oct 3, 2026

   

   juliusmarminge

   on Oct 3, 2026

   Member

   More actions

   Note

   Grok responding on behalf of Julius.

   Triage

   Thanks for the careful report and the reference fix, @samkelly880! The mode bug is real and still present on main; apps/server/src/atomicWrite.ts is unchanged since c0995d2.

   writeFileStringAtomically writes a temp file with FileSystem.writeFileString and renames it over the target without passing a mode. Node's fs.writeFile uses 0o666 masked by the process umask, and rename replaces the destination inode, so the previous mode is lost. With umask 022, a 0600 file comes back 0644 after the next save. We reproduced that sequence: the new contents landed, and only the mode changed.

   Nothing in this repo sets a mode on the files this helper writes (settings.json, keybindings.json, server-runtime.json, the provider status cache, and the ACP sign-in flag). A file created here is already 0644 under a typical umask, so the regression is a later save undoing a mode the user tightened. That matters because settings.json can hold some secrets in plaintext (providers.opencode.serverPassword, the Antigravity apiKey). Provider environment secrets, Bitbucket tokens, and usage-limit management keys go to the secret store first, so they aren't in this file. Since ~/.t3/userdata is created with the default mkdir mode (0755 at umask 022), a file that flips back to 0644 is readable by any local user who can traverse the home directory.

   The crash-durability part is only half right. The helper never fsyncs the temp file or the parent directory, so the rename is atomic for concurrent readers but not power-fail durable. On default ext4 (data=ordered with auto_da_alloc), this replace-via-rename pattern is specifically handled so a crash doesn't leave a zero-length file; the usual result is the previous contents, which is the normal cost of skipping fsync. server-runtime.json is already ignored when its pid is dead, and the cache and ACP-auth readers tolerate a missing file.

   One small correction: ServerSecretStore isn't quite the pattern described. Its set (the replace path) chmods 0600 before and after the rename but doesn't sync. create opens with mode 0600 and calls file.sync, but it's an exclusive create rather than a replace, and it doesn't fsync the directory.

   No other issue tracks this. A fix that preserves the existing permission bits, and uses 0600 when the file doesn't exist yet, fits these callers. Note that Node's stat.mode includes the file-type bits (0o100644), so the chmod needs to mask to 0o777. Directory fsync would be separate hardening rather than a required part of this fix.
2. juliusmarminge

   added

   bugSomething is broken or behaving incorrectly.Something is broken or behaving incorrectly.

   security

   via-triageFiled through npx t3 triageFiled through npx t3 triage

   on Oct 3, 2026