Skip to content

Commit a2dcf51

Browse files
committed
chore: Don't implicitly override shell rc perms
1 parent d5214d4 commit a2dcf51

5 files changed

Lines changed: 73 additions & 11 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ npx portkey mcp add
3939

4040
Config goes mainly into your shell profile and/or `.claude/` (the wizard explains each choice). Open a **new terminal** (or `source` your profile) after setup, then run `claude` as usual.
4141

42-
> **Shared / committed config (`PORTKEY_API_KEY`).** When you choose a git-tracked destination — `mcp add`**Repo file (.mcp.json)** or `setup`**Project (shared)** (`.claude/settings.json`) — the CLI **never** writes your key into the committed file. It writes the reference `"${PORTKEY_API_KEY}"` (Claude Code expands it from the environment at read time) and stores the real key in your shell profile (owner-only, `0600`). Each teammate just exports their own `PORTKEY_API_KEY`:
42+
> **Shared / committed config (`PORTKEY_API_KEY`).** When you choose a git-tracked destination — `mcp add`**Repo file (.mcp.json)** or `setup`**Project (shared)** (`.claude/settings.json`) — the CLI **never** writes your key into the committed file. It writes the reference `"${PORTKEY_API_KEY}"` (Claude Code expands it from the environment at read time) and stores the real key in your shell profile. Each teammate just exports their own `PORTKEY_API_KEY`:
4343
>
4444
> ```bash
4545
> export PORTKEY_API_KEY="pk-..." # add to ~/.zshrc, ~/.bashrc, or CI secrets
4646
> ```
4747
>
48-
> All files the CLI writes that hold a secret (shell profile, `~/.claude/settings.json`, `~/.codex/config.toml`) are created with `0600` permissions.
48+
> Config files the CLI creates itself (`~/.claude/settings.json`, `.mcp.json`) are written with owner-only `0600` permissions. Your **shell profile** (`~/.zshrc`, `~/.bashrc`, …) is left untouched — the CLI never changes its permissions — but it **warns** if that file is readable by other users while holding a secret, so you can `chmod 600` it yourself on shared machines.
4949
5050
**Later, from your project folder:**
5151

src/commands/claude-code/mcp.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
findProjectRoot,
1717
detectShellRc,
1818
writeShellRc,
19+
warnIfFileGroupOrWorldReadable,
1920
getMcpFilePath,
2021
settingsSetMcp,
2122
settingsRemoveMcp,
@@ -338,6 +339,7 @@ export async function doMcpAdd(args) {
338339
`.mcp.json references ${c.bold}${PORTKEY_KEY_ENV_REF}${c.reset} (no secret committed). ` +
339340
`Your key was saved to ${c.bold}${shellRc}${c.reset} — run ${c.bold}source ${shellRc}${c.reset} or open a new terminal.`
340341
);
342+
warnIfFileGroupOrWorldReadable(shellRc, `your ${PORTKEY_KEY_ENV}`);
341343
} else if (committed) {
342344
info(
343345
`.mcp.json references ${c.bold}${PORTKEY_KEY_ENV_REF}${c.reset} — no secret committed. ` +

src/commands/claude-code/setup.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
settingsSetKey,
2626
settingsSetMcp,
2727
writeShellRc,
28+
warnIfFileGroupOrWorldReadable,
2829
normalizeProvider,
2930
isClaudeInstalled,
3031
readExistingConfig,
@@ -843,6 +844,7 @@ export async function doSetup(args) {
843844
"# ── End Portkey + Codex ──",
844845
].join("\n"));
845846
ok(`PORTKEY_API_KEY written to ${shellRc}`);
847+
warnIfFileGroupOrWorldReadable(shellRc, "your PORTKEY_API_KEY");
846848

847849
const reloadCmd = isPwsh ? `. ${shellRc}` : `source ${shellRc}`;
848850
console.log();
@@ -884,6 +886,7 @@ export async function doSetup(args) {
884886

885887
writeShellRc(targetFile, lines.join("\n"));
886888
ok(`Gateway config written to ${targetFile}`);
889+
warnIfFileGroupOrWorldReadable(targetFile, "your ANTHROPIC_AUTH_TOKEN");
887890

888891
} else {
889892
// Write to settings.json. "project-shared" is committed to git, so reference
@@ -931,6 +934,7 @@ export async function doSetup(args) {
931934
} else {
932935
ok(`Auth token also written to ${shellRc}`);
933936
}
937+
warnIfFileGroupOrWorldReadable(shellRc, `your ${shellVar}`);
934938
}
935939

936940
// Reload reminder (once, at the end)

src/utils.js

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,37 @@ export function mask(key) {
5959
// ── Secure write ─────────────────────────────────────────────────────────────
6060

6161
/**
62-
* Write a file that may contain a secret (API key / auth token) with owner-only
63-
* permissions (0600). `fs.writeFileSync`'s `mode` only applies when the file is
64-
* created, so we also `chmod` to tighten any pre-existing file on the default
65-
* umask (which would otherwise leave it world-readable at 0644).
62+
* Write a file the CLI itself authors and that may contain a secret (API key /
63+
* auth token) with owner-only permissions (0600). `fs.writeFileSync`'s `mode`
64+
* only applies when the file is created, so we also `chmod` to tighten any
65+
* pre-existing file on the default umask (which would otherwise leave it
66+
* world-readable at 0644).
67+
*
68+
* Use this only for files we own (e.g. `.mcp.json`, `.claude/settings.json`).
69+
* Do NOT use it for user-managed files like shell rc — their permissions are the
70+
* user's choice; warn with `warnIfFileGroupOrWorldReadable` instead.
6671
*/
6772
export function writeFileSecure(filePath, data) {
6873
fs.writeFileSync(filePath, data, { mode: 0o600 });
6974
try { fs.chmodSync(filePath, 0o600); } catch {}
7075
}
7176

77+
/**
78+
* Warn (without modifying anything) when a file we just wrote a secret into is
79+
* readable by group or other. Intended for user-managed files such as shell rc,
80+
* where we deliberately never change permissions implicitly.
81+
*/
82+
export function warnIfFileGroupOrWorldReadable(filePath, secretLabel = "a credential") {
83+
let mode;
84+
try { mode = fs.statSync(filePath).mode & 0o777; } catch { return; }
85+
if (mode & 0o077) {
86+
warn(
87+
`${filePath} is readable by other users (permissions ${mode.toString(8)}) and now contains ${secretLabel}. ` +
88+
`On a shared machine, restrict it with: ${c.bold}chmod 600 ${filePath}${c.reset}`
89+
);
90+
}
91+
}
92+
7293
// ── JSON helpers ─────────────────────────────────────────────────────────────
7394

7495
export function jsonRead(filePath, keyPath) {
@@ -330,8 +351,10 @@ export function writeShellRc(filePath, block) {
330351
""
331352
);
332353
content += "\n" + block + "\n";
333-
// Shell rc may carry an exported API key — keep it owner-only.
334-
writeFileSecure(filePath, content);
354+
// Preserve the user's existing permissions — never implicitly chmod a
355+
// user-managed shell rc. Callers warn via warnIfFileGroupOrWorldReadable
356+
// when the block contains a secret.
357+
fs.writeFileSync(filePath, content);
335358
}
336359

337360
export function removeShellRcBlock(filePath) {

tests/utils.test.js

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, beforeEach, afterEach } from "vitest";
1+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
22
import fs from "node:fs";
33
import path from "node:path";
44
import os from "node:os";
@@ -16,6 +16,7 @@ import {
1616
writeShellRc,
1717
removeShellRcBlock,
1818
writeFileSecure,
19+
warnIfFileGroupOrWorldReadable,
1920
} from "../src/utils.js";
2021

2122
function mode(file) {
@@ -166,19 +167,51 @@ describe("writeFileSecure", () => {
166167
expect(mode(file)).toBe("600");
167168
});
168169

169-
it("is used by settings/mcp/shellrc writers so secrets are never world-readable", () => {
170+
it("is used by CLI-authored settings/mcp writers so secrets are never world-readable", () => {
170171
const settings = path.join(dir, "settings.json");
171172
settingsSetEnv(settings, { ANTHROPIC_AUTH_TOKEN: "pk-secret" });
172173
expect(mode(settings)).toBe("600");
173174

174175
const mcp = path.join(dir, ".mcp.json");
175176
settingsSetMcp(mcp, { srv: { type: "http", url: "u" } }, { scope: "project" });
176177
expect(mode(mcp)).toBe("600");
178+
});
179+
});
180+
181+
// ── writeShellRc preserves user-managed permissions ───────────────────────────
182+
183+
describe("writeShellRc permission handling", () => {
184+
let dir;
185+
beforeEach(() => { dir = tmpDir(); });
186+
afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); });
177187

188+
it("never changes the permissions of a pre-existing shell rc", () => {
178189
const rc = path.join(dir, ".bashrc");
179190
fs.writeFileSync(rc, "# rc", { mode: 0o644 });
180191
writeShellRc(rc, "export ANTHROPIC_AUTH_TOKEN=pk-secret");
181-
expect(mode(rc)).toBe("600");
192+
expect(mode(rc)).toBe("644");
193+
expect(fs.readFileSync(rc, "utf8")).toContain("pk-secret");
194+
});
195+
196+
it("warns (without chmod) when a world/group-readable file holds a secret", () => {
197+
const rc = path.join(dir, ".bashrc");
198+
fs.writeFileSync(rc, "# rc", { mode: 0o644 });
199+
const calls = [];
200+
const spy = vi.spyOn(console, "log").mockImplementation((m) => calls.push(m));
201+
warnIfFileGroupOrWorldReadable(rc, "your PORTKEY_API_KEY");
202+
spy.mockRestore();
203+
expect(mode(rc)).toBe("644");
204+
expect(calls.join("\n")).toMatch(/readable by other users/);
205+
});
206+
207+
it("stays silent for an owner-only (0600) file", () => {
208+
const rc = path.join(dir, ".bashrc");
209+
fs.writeFileSync(rc, "# rc", { mode: 0o600 });
210+
const calls = [];
211+
const spy = vi.spyOn(console, "log").mockImplementation((m) => calls.push(m));
212+
warnIfFileGroupOrWorldReadable(rc, "your PORTKEY_API_KEY");
213+
spy.mockRestore();
214+
expect(calls).toHaveLength(0);
182215
});
183216
});
184217

0 commit comments

Comments
 (0)