fix(storage): write userData JSON files owner-only - #5
Merged
voidstackloop merged 1 commit intoJul 25, 2026
Merged
Conversation
writeJson backs secrets.json (provider API keys), sessions.json, settings.json and projects.json. It wrote with no mode, so the files landed at the process umask — 0644 normally, 0664 under the umask 002 several distributions ship — leaving their contents readable to anything that reaches them: another account on a shared machine, a backup or sync tool, an archive unpacked elsewhere. The userData directory is usually restrictive enough to cover that on a single-user desktop, but a stored credential shouldn't depend on its parent directory's mode. The mode has to go on the temp file rather than the destination: writeJson writes to a temp path and renames over the target, so the destination inode is replaced on every write and takes the temp file's mode with it. Chmod'ing the destination instead would leave a window where the contents are readable and be undone by the next write — which also means a user who chmod 600'd the file by hand had it silently reset. Because writeFileSync only applies `mode` when it creates the file, the temp path is removed first rather than chmod'd afterwards; that keeps a stale temp file from a crashed write from carrying its old mode through, without adding a syscall that could fail between the write and the rename and lose the data. Existing files are tightened on first read, once per path per run — a key set once and never changed is only ever read, so fixing this on write alone would never reach the installs that already have the problem. That step is best-effort and logs rather than throwing. No behaviour change on Windows, where the mode has no meaning and the tests are skipped.
voidstackloop
approved these changes
Jul 25, 2026
voidstackloop
added a commit
that referenced
this pull request
Jul 25, 2026
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.
Problem
writeJsonbackssecrets.json(provider API keys),sessions.json,settings.jsonandprojects.json. It wrote with nomode, so the files landed at the process umask — 0644 normally, 0664 under the umask 002 several distributions ship.That leaves the contents readable to anything that reaches the file: another account on a shared machine, a backup or sync tool, an archive unpacked somewhere else. On a single-user desktop the userData directory is usually restrictive enough to cover it — but a stored credential shouldn't depend on its parent directory's mode, and
secrets-store.tsfalls back to storing keys unencrypted whensafeStorage.isEncryptionAvailable()is false, which is common on Linux without a keyring.There was also no way for a user to fix it themselves:
writeJsonwrites a temp file and renames over the target, so a hand-appliedchmod 600was silently reset by the next write.Fix
The mode goes on the temp file. The rename replaces the destination inode, so the temp file's mode is the mode the stored file ends up with. Chmod'ing the destination instead would leave a window where the contents are readable and be undone on the next write.
Because
writeFileSynconly appliesmodewhen it creates the file, the temp path is removed first rather than chmod'd afterwards. That handles a stale temp file left by an interrupted write under the same pid, without adding a syscall between the write and the rename that could fail and lose the data.Existing files are tightened on first read, once per path per run. Without that this would only protect fresh installs: a key set once and never changed is only ever read, so a write-side fix would never reach the installs that actually have the problem. That step is best-effort — it logs and continues rather than throwing, so unusual ownership can't stop the app reading its own data.
No behaviour change on Windows, where the mode has no meaning; those tests are skipped.
Tests
Control experiment: the first three fail on
mainand pass with the patch.I checked the corrupted-file backup path is unaffected —
copyFileSyncclones the source mode, so a 0600 file yields a 0600.corrupted-*backup.npx tsc -p tsconfig.json --noEmitclean;npm test231 passed.