Skip to content

Commit 4c62e31

Browse files
eric-engbergsirmalloc
authored andcommitted
fix: keep private settings files private when saving and backing up
- Saving ccstatusline's settings writes a temp file and renames it over settings.json, so the file took the temp file's default permissions: one the user had made 0600 came back 0644. The temp file is now created with the permissions of the file it replaces. - The .bak and .orig backups of Claude Code's settings.json were created 0644 whatever settings.json's permissions were, and that file can hold secrets (env, apiKeyHelper). Backups now get settings.json's permissions. An existing backup is removed first, since overwriting a file keeps its old permissions.
1 parent 75f07d6 commit 4c62e31

4 files changed

Lines changed: 61 additions & 2 deletions

File tree

‎src/utils/__tests__/claude-settings.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,23 @@ describe('backup and error handling behavior', () => {
489489
expect(orig.statusLine?.command).toBe('old-command');
490490
});
491491

492+
// settings.json can hold secrets (env, apiKeyHelper), so its backups must
493+
// not be readable by anyone who can't read it
494+
it.skipIf(process.platform === 'win32')('gives .bak and .orig backups the permissions of a private settings file', async () => {
495+
writeRawClaudeSettings(JSON.stringify({ effortLevel: 'high' }));
496+
const settingsPath = getClaudeSettingsPath();
497+
fs.chmodSync(settingsPath, 0o600);
498+
// A backup left from before keeps its own permissions when overwritten
499+
fs.writeFileSync(`${settingsPath}.bak`, '{}', 'utf-8');
500+
fs.chmodSync(`${settingsPath}.bak`, 0o644);
501+
502+
await installStatusLine({ commandMode: 'auto-npx' });
503+
504+
expect(fs.statSync(settingsPath).mode & 0o777).toBe(0o600);
505+
expect(fs.statSync(`${settingsPath}.orig`).mode & 0o777).toBe(0o600);
506+
expect(fs.statSync(`${settingsPath}.bak`).mode & 0o777).toBe(0o600);
507+
});
508+
492509
it('loadClaudeSettings should return empty object when settings file is missing', async () => {
493510
await expect(loadClaudeSettings()).resolves.toEqual({});
494511
});

‎src/utils/__tests__/config.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,19 @@ describe('config utilities', () => {
423423
expect(leftovers).toEqual([]);
424424
});
425425

426+
// Custom commands can carry credentials, so a save must not open up a
427+
// settings file the user made private
428+
it.skipIf(process.platform === 'win32')('keeps a private settings file private when saving', async () => {
429+
const { settingsPath, configDir } = getSettingsPaths();
430+
fs.mkdirSync(configDir, { recursive: true });
431+
fs.writeFileSync(settingsPath, JSON.stringify({ version: CURRENT_VERSION, lines: [[], [], []] }), 'utf-8');
432+
fs.chmodSync(settingsPath, 0o600);
433+
434+
await saveSettings({ ...DEFAULT_SETTINGS });
435+
436+
expect(fs.statSync(settingsPath).mode & 0o777).toBe(0o600);
437+
});
438+
426439
it('saves through a symlinked settings file without replacing the link', async () => {
427440
const { settingsPath, configDir } = getSettingsPaths();
428441
const targetDir = path.join(MOCK_HOME_DIR, 'dotfiles', 'ccstatusline');

‎src/utils/claude-settings.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ export type { ClaudeSettings };
2525
const readFile = fs.promises.readFile;
2626
const writeFile = fs.promises.writeFile;
2727
const mkdir = fs.promises.mkdir;
28+
const stat = fs.promises.stat;
29+
const unlink = fs.promises.unlink;
2830

2931
export const CCSTATUSLINE_COMMANDS = {
3032
AUTO_NPX: 'npx -y ccstatusline@latest',
@@ -140,6 +142,16 @@ export function getClaudeSettingsPath(): string {
140142
return path.join(getClaudeConfigDir(), 'settings.json');
141143
}
142144

145+
async function removeIfExists(filePath: string): Promise<void> {
146+
try {
147+
await unlink(filePath);
148+
} catch (error) {
149+
if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
150+
throw error;
151+
}
152+
}
153+
}
154+
143155
/**
144156
* Creates a backup of the current Claude settings file.
145157
*/
@@ -149,7 +161,12 @@ async function backupClaudeSettings(suffix = '.bak'): Promise<string | null> {
149161
try {
150162
if (fs.existsSync(settingsPath)) {
151163
const content = await readFile(settingsPath, 'utf-8');
152-
await writeFile(backupPath, content, 'utf-8');
164+
// The backup holds the same secrets (env, apiKeyHelper) as the
165+
// settings, so it gets their permissions. It's recreated because
166+
// an existing file keeps its own permissions when overwritten.
167+
const mode = (await stat(settingsPath)).mode & 0o777;
168+
await removeIfExists(backupPath);
169+
await writeFile(backupPath, content, { encoding: 'utf-8', mode });
153170
return backupPath;
154171
}
155172
} catch (error) {

‎src/utils/config.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const unlink = fs.promises.unlink;
2626
const lstat = fs.promises.lstat;
2727
const readlink = fs.promises.readlink;
2828
const realpath = fs.promises.realpath;
29+
const stat = fs.promises.stat;
2930

3031
const DEFAULT_SETTINGS_PATH = path.join(os.homedir(), '.config', 'ccstatusline', 'settings.json');
3132

@@ -111,6 +112,16 @@ async function resolveAtomicWriteTarget(paths: SettingsPaths): Promise<AtomicWri
111112
}
112113
}
113114

115+
// The permissions of the file a save replaces, so a private settings file
116+
// stays private; a new file gets the usual default
117+
async function getReplacedFileMode(filePath: string): Promise<number> {
118+
try {
119+
return (await stat(filePath)).mode & 0o777;
120+
} catch {
121+
return 0o666;
122+
}
123+
}
124+
114125
async function writeSettingsJson(settings: unknown, paths: SettingsPaths): Promise<void> {
115126
await mkdir(paths.configDir, { recursive: true });
116127

@@ -123,8 +134,9 @@ async function writeSettingsJson(settings: unknown, paths: SettingsPaths): Promi
123134
writeTarget.tempDir,
124135
`${path.basename(writeTarget.targetPath)}.${process.pid}.${Date.now()}.tmp`
125136
);
137+
const mode = await getReplacedFileMode(writeTarget.targetPath);
126138
try {
127-
await writeFile(tempPath, JSON.stringify(settings, null, 2), 'utf-8');
139+
await writeFile(tempPath, JSON.stringify(settings, null, 2), { encoding: 'utf-8', mode });
128140
await rename(tempPath, writeTarget.targetPath);
129141
} catch (error) {
130142
try {

0 commit comments

Comments
 (0)