Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions app/src/json-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,34 @@ describe("json-store", () => {
expect(entries).toEqual(["data.json"]);
});

// These files hold API keys and conversation history; the default umask
// would leave them readable by other accounts on the machine.
it.skipIf(process.platform === "win32")("writes owner-only files", () => {
writeJson(file, { token: "value" });
expect(fs.statSync(file).mode & 0o777).toBe(0o600);
});

it.skipIf(process.platform === "win32")("keeps the file owner-only on rewrite", () => {
writeJson(file, { token: "first" });
writeJson(file, { token: "second" });
expect(fs.statSync(file).mode & 0o777).toBe(0o600);
});

it.skipIf(process.platform === "win32")("stays owner-only when a stale temp file exists", () => {
const stale = `${file}.tmp-${process.pid}`;
fs.writeFileSync(stale, "leftover", { mode: 0o666 });
writeJson(file, { token: "value" });
expect(fs.statSync(file).mode & 0o777).toBe(0o600);
});

// A file written by an older build is only ever read if its contents never
// change, so tightening on write alone would never reach it.
it.skipIf(process.platform === "win32")("tightens an existing world-readable file on read", () => {
fs.writeFileSync(file, JSON.stringify({ token: "value" }), { mode: 0o644 });
expect(readJson(file, {})).toEqual({ token: "value" });
expect(fs.statSync(file).mode & 0o777).toBe(0o600);
});

it("backs up and falls back to the default when the file is corrupted", () => {
fs.writeFileSync(file, "{ not valid json");
const result = readJson(file, { safe: true });
Expand Down
43 changes: 42 additions & 1 deletion app/src/json-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export function readJson<T>(filePath: string, fallback: T): T {
return fallback;
}

restrictExistingPermissions(filePath);

try {
return JSON.parse(raw) as T;
} catch (err) {
Expand All @@ -43,9 +45,48 @@ export function readJson<T>(filePath: string, fallback: T): T {
}
}

// These files hold provider API keys (secrets.json) and full conversation
// history. Written with the process umask they land as 0644 — or 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 somewhere else. 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.
const PRIVATE_FILE_MODE = 0o600;

// Files written before this existed keep the mode they were created with, and
// writeJson alone would never reach them: a key set once and never changed is
// only ever read. So the mode is also tightened on first read, once per path
// per run to keep it off the hot path.
const permissionsChecked = new Set<string>();

function restrictExistingPermissions(filePath: string): void {
if (process.platform === "win32" || permissionsChecked.has(filePath)) return;
permissionsChecked.add(filePath);
try {
if ((fs.statSync(filePath).mode & 0o777) !== PRIVATE_FILE_MODE) {
fs.chmodSync(filePath, PRIVATE_FILE_MODE);
}
} catch (err) {
// Best effort — unusual ownership or an exotic filesystem must not
// stop the app from reading its own data.
logger.error(`Failed to restrict permissions on ${filePath}: ${(err as Error).message}`);
}
}

export function writeJson(filePath: string, data: unknown): void {
fs.mkdirSync(path.dirname(filePath), { recursive: true });
const tmpPath = `${filePath}.tmp-${process.pid}`;
fs.writeFileSync(tmpPath, JSON.stringify(data, null, 2));
// The mode goes on the temp file, because the rename below replaces the
// destination inode and takes the temp file's mode with it. Chmod'ing the
// destination afterwards would leave a window where the contents are
// readable, and would be undone by the next write.
//
// Removed first so writeFileSync always creates the file and so always
// applies `mode` — it ignores the option for a path that already exists,
// and an interrupted earlier write can leave one behind under this pid.
fs.rmSync(tmpPath, { force: true });
fs.writeFileSync(tmpPath, JSON.stringify(data, null, 2), { mode: PRIVATE_FILE_MODE });
fs.renameSync(tmpPath, filePath);
}