Repository navigation
fix(installer): a rewritten config keeps its mode, so a 0600 file stays private - #2779
Open
colbymchenry wants to merge 1 commit into
Open
colbymchenry wants to merge 1 commit into
colbymchenry wants to merge 1 commit into
Conversation
…ys private Every installer write goes to <path>.tmp.<pid> and is renamed over the file, through a symbolic link too since the fix for #2721. A new file gets the default mode and this process's owner, so on macOS and Linux a 0600 ~/.claude.json or ~/.codex/config.toml, which can hold tokens, came out 0644 after install, refresh or uninstall: readable by every account on the machine. Found while fixing #2721. The temp file now takes the old file's mode and owner. It is opened with the old permission bits, which the umask can only narrow. Then, before any content is written, it gets the old owner (fchown, where this account may set it) and the exact old mode (fchmod). A refused owner change, or a file system without modes, doesn't fail the write. The write-through-link path uses the same code, so a linked file keeps its mode. A file that doesn't exist yet still gets the default mode. Validation (this machine is Windows, which has no POSIX modes): - installer-file-mode.test.ts reports a mode and owner through a mocked fs.statSync and records what is done to the temp file: the helper, a simulated link, a refused chown and chmod, a new file, and every agent's install and uninstall, plus a refresh. Of its 24 tests that run here, 22 fail on main's helper (all on assertions; the other 2 guard what stays the same) and all 24 pass with the fix. - The POSIX tests (real chmod and stat: 8 in the new file, 19 in installer-targets.test.ts) are gated with it.runIf and did not run here. - The real CLI (install, install --refresh, uninstall) against a private home, with POSIX mode rules simulated by a --require preload: main turned all 7 seeded 0600 configs into 0644, the fix kept all 7 at 0600 through every step. - On Windows a read-only config.toml is refused with the same EPERM on both, left unchanged, with no temp file left beside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
Every installer write goes to
<path>.tmp.<pid>and is then renamed over the file. That isatomicWriteFileSync, which every agent target uses, and since #2721 it also writes through symbolic links this way.The rename puts a new file in the old one's place, and a new file gets the default mode (
0666minus the umask, usually0644) and the current process's owner. So on macOS and Linux, a~/.claude.jsonor~/.codex/config.tomlthe user had made0600came back0644, readable by every account on the machine. Both files can hold tokens, for example in another MCP server'senv. It happened oncodegraph install, oninstall --refresh(whichcodegraph upgraderuns) and onuninstall.I found this while fixing #2721. There is no issue for it.
The fix
writeThenRename, behindatomicWriteFileSync, now:mode & 0o777). The umask can only narrow those, so the temp is never readable by more people than the old file, not even before the rename.fchown).fchmod,mode & 0o7777). This comes after the owner change, because changing the owner can clear the set-id bits.A file that doesn't exist yet is created exactly as before:
0666minus the umask, with no owner or mode changes.Calls I made
sudo, a user's config now keeps its owner instead of becoming root's.fchownorfchmodnever fails an install. The mode the temp file is opened with already caps its permissions at the old file's.Validation
This machine is Windows, which has no POSIX modes, and it has no Docker or WSL.
The logic, on this machine.
__tests__/installer-file-mode.test.tsreports a POSIX mode and owner (0600, uid 4242, gid 4343) through a mockedfs.statSync. It records every call made on each temp file and expectsopen 600 → chown 4242:4343 → chmod 600 → write → close → renameonto the right file. It covers:chownandchmodchownorchmod0600files~/.claude.jsonand~/.codex/config.tomlentries24 of its tests run here. On main's helper, 22 fail, all on assertions. The other 2 guard behaviour that stays the same. All 24 pass with the fix.
Real modes, POSIX only. These tests are gated with
it.runIf(process.platform !== 'win32')and did not run here:0600,0640,0444and0664files keep their mode0600when it is renamedinstaller-targets.test.ts:0600files stay0600through install and uninstall0600~/.claude.jsonandconfig.tomlThe new file runs the same flows on this machine with the mode mocked, so only the reads of real modes go unexercised here. They will run on macOS.
The real CLI, against a private home with 7 seeded
0600configs:codegraph install --target claude,codex,opencode,hermes,gemini --location global --yesinstall --refresh --yes, after making two entries staleuninstall … --keep-cliWindows has no modes, so POSIX mode rules were simulated by a
--requirepreload (posix-modes.cjs, in the kit):022umaskchmodandfchmodset the modecopyFilecopies the mode644600644600644600The user's own settings survive on both, and no temp file is left behind.
Windows read-only.
codegraph install --target codexwith a read-only~/.codex/config.tomlgives the same result on main and with this PR. Both refuse withEPERM … rename, the file is unchanged and still read-only, and no temp file is left. The existing #2721 test for a link to a read-only file also passes.Type-check. The touched test files add no new type errors.
Full suite on the pushed head 3c5f0c7 (Windows 11,
CODEGRAPH_KERNEL_EXPECT=1 npx vitest run --maxWorkers=4 --minWorkers=1): 701 files passed, 6 skipped (707); 8455 tests passed, 109 skipped, 0 failed. The 27 POSIX-only tests are among the skipped.🤖 Generated with Claude Code