Skip to content

Commit ae90cc8

Browse files
committed
Deny backups of OAuth token stores in the secret guard
1 parent 7f8db2f commit ae90cc8

5 files changed

Lines changed: 35 additions & 19 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ In TUI chat mode there is no completion gate — the session stays open across t
7474
- `providers.ts` defines the `ProviderCatalogEntry` type and helpers for building TUI provider lists; `profiles.ts` handles profile-level selection logic.
7575
- `loadConfig` is async (it reads settings files). Parses a leading `exec`/`run` subcommand, flags `--cwd`, `--config`, `--provider`, `--model`, `--dangerously-skip-permissions` (forces this process; TUI `/yolo` persists as the user-global default), `--auto` / `--no-auto` (auto mode defaults on); collects positional arguments as the optional initial task for the TUI or the required prompt for exec.
7676
- Both settings files and the project/global grant store (`.corbits/permissions.json`) are on the secret-guard denylist for path-keyed tools, so the agent cannot `read_file` its own credentials or persist standing auto-approvals. Shell commands that reference them still require explicit operator approval.
77-
- Credential-surface ownership: each auth store module enumerates its own files (`*_AUTH_FILENAME` / `MCP_AUTH_DIRNAME`), the data-only registry in `src/auth/credential-surface.ts` turns them into denylist patterns, `secret-guard-plugin.ts` owns matching (lexical plus realpath), and `@mention` resolution consumes the resolved check — never the registry directly. A new token store is denied only once its store module exports its filename and the registry lists it; the coverage test fails the build until both exist.
77+
- Credential-surface ownership: each auth store module enumerates its own files (`*_AUTH_FILENAME` / `MCP_AUTH_DIRNAME`), the data-only registry in `src/auth/credential-surface.ts` turns them into denylist patterns, `secret-guard-plugin.ts` owns matching (lexical plus realpath), and `@mention` resolution consumes the resolved check — never the registry directly. A new `*-auth.json` token store is denied only once its store module exports its filename and the registry lists it; the coverage test scans store sources for `*-auth.json` literals (registered dirnames get an includes-check instead) and fails the build until both exist.
7878

7979
### TUI Runner (`src/tui/runner.ts`)
8080

‎src/auth/credential-surface.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,16 @@ export function buildCredentialPatterns(): RegExp[] {
5454
const scope = escapeRegExp(settingsDirName);
5555
patterns.push(new RegExp(`(^|\\/)${scope}\\/${escapeRegExp(dirname)}\\/`));
5656
}
57-
// Editor backup copies keep full credential bytes next to the live file.
58-
patterns.push(new RegExp(`(^|\\/)${dir}\\/settings\\.json\\.bak[^/]*$`));
5957
for (const basename of credentialSidecarBasenames) {
6058
const base = escapeRegExp(basename);
59+
// Editor backup copies keep full credential bytes next to the live file.
60+
// Scoped to the settings dir and anchored to known basenames, never a
61+
// generic *.bak.
62+
patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.bak[^/]*$`));
6163
patterns.push(new RegExp(`(^|\\/)${base}\\.lock$`));
64+
// Writers emit a pid.counter middle segment (auth/store.ts,
65+
// mcp/auth-store.ts), so the middle segment is required; a bare
66+
// `<base>.tmp` has no known writer and stays unmatched.
6267
patterns.push(new RegExp(`(^|\\/)${base}\\.[^/]*\\.tmp$`));
6368
}
6469
return patterns;

‎src/plugins/secret-guard-credential-surface.test.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,26 @@ describe("CL-7789 default-install credential surface", () => {
107107

108108
test("credential backups and sidecars resolve as sensitive", () => {
109109
const home = join(tmpdir(), "cl7789-never-created");
110+
for (const basename of [
111+
"settings.json",
112+
"permissions.json",
113+
"codex-auth.json",
114+
"xai-auth.json",
115+
]) {
116+
expect(
117+
isSensitivePathResolved(
118+
join(home, ".corbits", `${basename}.bak-2026-01-01`),
119+
),
120+
).toBe(true);
121+
}
110122
expect(
111123
isSensitivePathResolved(
112-
join(home, ".corbits", "settings.json.bak-2026-01-01"),
124+
join(
125+
home,
126+
".corbits",
127+
"mcp-auth",
128+
"my-server-9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00.json.bak-2026-01-01",
129+
),
113130
),
114131
).toBe(true);
115132
expect(
@@ -128,7 +145,7 @@ describe("CL-7789 default-install credential surface", () => {
128145
try {
129146
for (const [path, marker] of [
130147
[fake.codexAuth, "CODEX-FIXTURE-ACCESS"],
131-
[fake.mcpAuth, "MCP-FIXTURE-ACCESS"],
148+
[fake.mcpAuth, "access_token"],
132149
] as const) {
133150
const resolved = await resolveAtMentions(`read @${path}`, cwd);
134151
expect(resolved).toContain("(blocked: sensitive path)");

‎src/plugins/secret-guard-plugin.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ function isPathLikeShellToken(token: string): boolean {
183183
// resolves the home symlink instead of a (usually missing) cwd child.
184184
// classify.ts's outside-workspace rule would ask anyway; the expansion fixes
185185
// the *reason* (sensitive-path) rather than relying on that coincidence.
186-
function expandHome(token: string): string {
186+
export function expandHome(token: string): string {
187187
if (token === "~") return homedir();
188188
if (token.startsWith("~/")) return joinPath(homedir(), token.slice(2));
189189
return token;

‎src/tui/mention-resolution.ts‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { readFile, opendir, realpath, stat } from "node:fs/promises";
2-
import { homedir } from "node:os";
3-
import { resolve, isAbsolute, join } from "node:path";
4-
import { isSensitivePathResolved } from "../plugins/secret-guard-plugin.js";
2+
import { resolve, isAbsolute } from "node:path";
3+
import {
4+
expandHome,
5+
isSensitivePathResolved,
6+
} from "../plugins/secret-guard-plugin.js";
57

68
const MAX_MENTION_FILE_BYTES = 200_000;
79
const MAX_MENTION_TOTAL_BYTES = 400_000;
@@ -45,16 +47,8 @@ async function summarizeDir(abs: string): Promise<string> {
4547
return parts.length > 0 ? parts.join(", ") : "empty directory";
4648
}
4749

48-
// `~` means the operator's home to the shell, not a cwd-relative name —
49-
// expand before both sensitivity gates (same ordering as the shell-token
50-
// matcher) so a home-relative credential path denies as itself instead of
51-
// resolving to a usually-missing cwd child that reads as merely not found.
52-
function expandHome(path: string): string {
53-
if (path === "~") return homedir();
54-
if (path.startsWith("~/")) return join(homedir(), path.slice(2));
55-
return path;
56-
}
57-
// An @mention is the operator directly asking the agent to read one path, once,
50+
// `~` expansion lives in secret-guard-plugin.ts (single owner); the comment
51+
// there documents the ordering. An @mention is the operator directly asking the agent to read one path, once,
5852
// right now — the same consent that already lets the agent read any workspace
5953
// file. There is no workspace-boundary check here: mentioning a path outside
6054
// the workspace inlines it exactly like a workspace path would, gated only by

0 commit comments

Comments
 (0)