diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a532373fe..064845065 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -74,6 +74,7 @@ In TUI chat mode there is no completion gate — the session stays open across t - `providers.ts` defines the `ProviderCatalogEntry` type and helpers for building TUI provider lists; `profiles.ts` handles profile-level selection logic. - `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. - 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. +- 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. ### TUI Runner (`src/tui/runner.ts`) diff --git a/src/auth/codex/store.ts b/src/auth/codex/store.ts index f83e844f9..382e2f2e5 100644 --- a/src/auth/codex/store.ts +++ b/src/auth/codex/store.ts @@ -37,9 +37,11 @@ export function withDefaultCodexExpiry( return { ...tokens, expiresAt: now + DEFAULT_EXPIRES_IN_S * 1000 }; } +export const CODEX_AUTH_FILENAME = "codex-auth.json"; + export function createCodexAuthStore(settingsDirName: string) { return createAuthStore({ - filename: "codex-auth.json", + filename: CODEX_AUTH_FILENAME, settingsDirName, isTokens: isCodexTokens, }); diff --git a/src/auth/credential-surface-coverage.test.ts b/src/auth/credential-surface-coverage.test.ts new file mode 100644 index 000000000..4d46eed49 --- /dev/null +++ b/src/auth/credential-surface-coverage.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, test } from "bun:test"; +import { Glob } from "bun"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { CODEX_AUTH_FILENAME } from "./codex/store.js"; +import { + buildCredentialPatterns, + credentialDirDescriptors, + credentialFileDescriptors, +} from "./credential-surface.js"; +import { MCP_AUTH_DIRNAME } from "../mcp/auth-store.js"; +import { XAI_AUTH_FILENAME } from "./xai/store.js"; + +const here = dirname(fileURLToPath(import.meta.url)); + +// Auth-owned credential literals live in exactly these store modules. Test +// fixtures (concurrent-auth.json, test-auth.json) live in *.test.ts, which the +// scan below excludes, so fixture names can never become denylist patterns. +const STORE_FILES = [ + join(here, "codex", "store.ts"), + join(here, "xai", "store.ts"), + join(here, "..", "mcp", "auth-store.ts"), +]; + +async function scanStoreSources(): Promise> { + const sources = new Map(); + const glob = new Glob("**/*.ts"); + for await (const entry of glob.scan({ cwd: here, absolute: true })) { + if (entry.endsWith(".test.ts")) continue; + sources.set(entry, await Bun.file(entry).text()); + } + for (const file of STORE_FILES) { + if (!sources.has(file)) sources.set(file, await Bun.file(file).text()); + } + return sources; +} + +function authFileLiterals(source: string): string[] { + const found: string[] = []; + const pattern = /["']([A-Za-z0-9_.-]+-auth\.json)["']/g; + let match: RegExpExecArray | null; + while ((match = pattern.exec(source)) !== null) { + const literal = match[1]; + if (literal !== undefined && !found.includes(literal)) found.push(literal); + } + return found; +} + +describe("CL-7789 credential-surface coverage", () => { + test("every *-auth.json literal in auth-owned stores is denied", async () => { + const sources = await scanStoreSources(); + const patterns = buildCredentialPatterns(); + const seen = new Set(); + for (const source of sources.values()) { + for (const literal of authFileLiterals(source)) { + seen.add(literal); + const probe = join("~", ".corbits", literal); + expect( + patterns.some((pattern) => pattern.test(probe)), + `${literal} has no denylist pattern`, + ).toBe(true); + } + } + expect([...seen].sort()).toEqual( + [CODEX_AUTH_FILENAME, XAI_AUTH_FILENAME].sort(), + ); + }); + + test("every registry descriptor resolves back to a store literal", async () => { + const sources = await scanStoreSources(); + const texts = [...sources.values()]; + for (const { filename } of credentialFileDescriptors) { + expect( + texts.some((source) => source.includes(`"${filename}"`)), + `${filename} is registered but no store writes it`, + ).toBe(true); + } + for (const { dirname } of credentialDirDescriptors) { + expect( + texts.some((source) => source.includes(`"${dirname}"`)), + `${dirname} is registered but no store writes it`, + ).toBe(true); + } + expect(texts.some((source) => source.includes(MCP_AUTH_DIRNAME))).toBe( + true, + ); + }); +}); diff --git a/src/auth/credential-surface.ts b/src/auth/credential-surface.ts new file mode 100644 index 000000000..b79fe2e25 --- /dev/null +++ b/src/auth/credential-surface.ts @@ -0,0 +1,73 @@ +import { SETTINGS_DIR_NAME } from "../branding.js"; + +// Auth-owned credential surface: every file under the settings directory whose +// bytes are OAuth tokens or provider credentials. The secret-guard plugin owns +// matching (lexical + realpath); mention-resolution consumes the resolved +// check. This module stays data-only — branding plus literals, no store +// factories or homedir calls — so the import direction stays plugins → auth → +// branding with no cycle. +export interface CredentialFileDescriptor { + settingsDirName: string; + filename: string; +} + +export interface CredentialDirDescriptor { + settingsDirName: string; + dirname: string; +} + +export const credentialFileDescriptors: CredentialFileDescriptor[] = [ + { settingsDirName: SETTINGS_DIR_NAME, filename: "codex-auth.json" }, + { settingsDirName: SETTINGS_DIR_NAME, filename: "xai-auth.json" }, +]; + +export const credentialDirDescriptors: CredentialDirDescriptor[] = [ + // Per-server files embed a content sha in the name, so they cannot be + // enumerated — match the whole directory instead. + { settingsDirName: SETTINGS_DIR_NAME, dirname: "mcp-auth" }, +]; + +// Every basename whose lock/temp sidecar carries full credential bytes +// mid-write. settings.json and permissions.json keep their hand-written base +// patterns in the plugin; their sidecars are still enumerated here so a torn +// write's temp file denies the same as the file itself. +const credentialSidecarBasenames: string[] = [ + ...credentialFileDescriptors.map((descriptor) => descriptor.filename), + "settings.json", + "permissions.json", +]; + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +// Guard-only patterns: deny reads/writes of credential-adjacent files without +// rotating, migrating, or encrypting them. +export function buildCredentialPatterns(): RegExp[] { + const dir = escapeRegExp(SETTINGS_DIR_NAME); + const patterns: RegExp[] = []; + for (const { settingsDirName, filename } of credentialFileDescriptors) { + const scope = escapeRegExp(settingsDirName); + patterns.push(new RegExp(`(^|\\/)${scope}\\/${escapeRegExp(filename)}$`)); + } + for (const { settingsDirName, dirname } of credentialDirDescriptors) { + const scope = escapeRegExp(settingsDirName); + patterns.push(new RegExp(`(^|\\/)${scope}\\/${escapeRegExp(dirname)}\\/`)); + } + for (const basename of credentialSidecarBasenames) { + const base = escapeRegExp(basename); + // Editor backup copies keep full credential bytes next to the live file. + // Scoped to the settings dir and anchored to known basenames, never a + // generic *.bak, *~, or *.swp. + patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.bak[^/]*$`)); + patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}~$`)); + patterns.push(new RegExp(`(^|\\/)${dir}\\/${base}\\.swp$`)); + patterns.push(new RegExp(`(^|\\/)${dir}\\/\\.${base}\\.swp$`)); + patterns.push(new RegExp(`(^|\\/)${base}\\.lock$`)); + // Writers emit a pid.counter middle segment (auth/store.ts, + // mcp/auth-store.ts), so the middle segment is required; a bare + // `.tmp` has no known writer and stays unmatched. + patterns.push(new RegExp(`(^|\\/)${base}\\.[^/]*\\.tmp$`)); + } + return patterns; +} diff --git a/src/auth/xai/store.ts b/src/auth/xai/store.ts index 33772694e..e7c8b1053 100644 --- a/src/auth/xai/store.ts +++ b/src/auth/xai/store.ts @@ -19,9 +19,11 @@ function isXaiTokens(value: unknown): value is XaiTokens { return !(XaiTokensShape(value) instanceof type.errors); } +export const XAI_AUTH_FILENAME = "xai-auth.json"; + export function createXaiAuthStore(settingsDirName: string) { return createAuthStore({ - filename: "xai-auth.json", + filename: XAI_AUTH_FILENAME, settingsDirName, isTokens: isXaiTokens, }); diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index 645e4a98d..290826bbc 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -33,8 +33,10 @@ export interface MCPAuthIdentity { serverURL: string; } +export const MCP_AUTH_DIRNAME = "mcp-auth"; + export function mcpAuthDir(home: string = homedir()): string { - return join(home, SETTINGS_DIR_NAME, "mcp-auth"); + return join(home, SETTINGS_DIR_NAME, MCP_AUTH_DIRNAME); } function legacyServerSlug(serverName: string): string { diff --git a/src/plugins/secret-guard-credential-surface.test.ts b/src/plugins/secret-guard-credential-surface.test.ts new file mode 100644 index 000000000..ced5027c2 --- /dev/null +++ b/src/plugins/secret-guard-credential-surface.test.ts @@ -0,0 +1,208 @@ +import { describe, expect, test } from "bun:test"; +import { mkdir, mkdtemp, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ToolCall, ToolResult } from "@intx/types/runtime"; +import { resolveAtMentions } from "../tui/mention-resolution.js"; +import { + isSensitivePath, + isSensitivePathResolved, + secretGuardPlugin, +} from "./secret-guard-plugin.js"; + +const CODEX_BYTES = + '{"profiles":{"personal":{"tokens":{"access":"CODEX-FIXTURE-ACCESS"}}}}'; +const XAI_BYTES = + '{"profiles":{"personal":{"tokens":{"access":"XAI-FIXTURE-ACCESS"}}}}'; +const MCP_BYTES = '{"tokens":{"access_token":"MCP-FIXTURE-ACCESS"}}'; + +async function fakeHome(): Promise<{ + home: string; + codexAuth: string; + xaiAuth: string; + settings: string; + permissions: string; + mcpAuth: string; + cleanup: () => Promise; +}> { + const home = await mkdtemp(join(tmpdir(), "cl7789-fake-home-")); + const configDir = join(home, ".corbits"); + const mcpDir = join(configDir, "mcp-auth"); + await mkdir(mcpDir, { recursive: true }); + const codexAuth = join(configDir, "codex-auth.json"); + const xaiAuth = join(configDir, "xai-auth.json"); + const settings = join(configDir, "settings.json"); + const permissions = join(configDir, "permissions.json"); + const mcpAuth = join( + mcpDir, + "my-server-9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00.json", + ); + await writeFile(codexAuth, `${CODEX_BYTES}\n`); + await writeFile(xaiAuth, `${XAI_BYTES}\n`); + await writeFile(settings, '{"provider":"codex"}\n'); + await writeFile(permissions, '{"approvals":[]}\n'); + await writeFile(mcpAuth, `${MCP_BYTES}\n`); + return { + home, + codexAuth, + xaiAuth, + settings, + permissions, + mcpAuth, + cleanup: () => rm(home, { recursive: true, force: true }), + }; +} + +const next = async (call: ToolCall): Promise => ({ + callId: call.id, + content: "ok", +}); + +function denyReason(path: string): Promise { + const plugin = secretGuardPlugin(); + const handler = plugin.middleware ? plugin.middleware(next) : next; + return handler( + { id: "c", name: "read_file", arguments: { path } }, + new AbortController().signal, + ); +} + +describe("CL-7789 default-install credential surface", () => { + test("lexical denylist covers every default credential file", async () => { + const fake = await fakeHome(); + try { + for (const path of [ + fake.codexAuth, + fake.xaiAuth, + fake.settings, + fake.permissions, + fake.mcpAuth, + ]) { + expect(isSensitivePath(path)).toBe(true); + expect(isSensitivePathResolved(path)).toBe(true); + } + } finally { + await fake.cleanup(); + } + }); + + test("read_file hard-denies every default credential file", async () => { + const fake = await fakeHome(); + try { + for (const path of [ + fake.codexAuth, + fake.xaiAuth, + fake.settings, + fake.permissions, + fake.mcpAuth, + ]) { + const result = await denyReason(path); + expect(result.isError).toBe(true); + expect(String(result.content)).toMatch(/sensitive file blocked/); + } + } finally { + await fake.cleanup(); + } + }); + + test("credential backups and sidecars resolve as sensitive", () => { + const home = join(tmpdir(), "cl7789-never-created"); + for (const basename of [ + "settings.json", + "permissions.json", + "codex-auth.json", + "xai-auth.json", + ]) { + expect( + isSensitivePathResolved( + join(home, ".corbits", `${basename}.bak-2026-01-01`), + ), + ).toBe(true); + } + expect( + isSensitivePathResolved( + join( + home, + ".corbits", + "mcp-auth", + "my-server-9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00.json.bak-2026-01-01", + ), + ), + ).toBe(true); + expect( + isSensitivePathResolved(join(home, ".corbits", "codex-auth.json.lock")), + ).toBe(true); + for (const basename of [ + "settings.json", + "permissions.json", + "codex-auth.json", + "xai-auth.json", + ]) { + expect( + isSensitivePathResolved(join(home, ".corbits", `${basename}~`)), + ).toBe(true); + expect( + isSensitivePathResolved(join(home, ".corbits", `${basename}.swp`)), + ).toBe(true); + expect( + isSensitivePathResolved(join(home, ".corbits", `.${basename}.swp`)), + ).toBe(true); + } + expect( + isSensitivePathResolved( + join(home, ".corbits", "xai-auth.json.12345.1.tmp"), + ), + ).toBe(true); + }); + + test("@mentions of credential files stay blocked without leaking bytes", async () => { + const fake = await fakeHome(); + const cwd = await mkdtemp(join(tmpdir(), "cl7789-mention-cwd-")); + try { + for (const [path, marker] of [ + [fake.codexAuth, "CODEX-FIXTURE-ACCESS"], + [fake.mcpAuth, "access_token"], + ] as const) { + const resolved = await resolveAtMentions(`read @${path}`, cwd); + expect(resolved).toContain("(blocked: sensitive path)"); + expect(resolved).not.toContain(marker); + } + } finally { + await fake.cleanup(); + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("@mentions of credential backup paths stay blocked without leaking bytes", async () => { + const fake = await fakeHome(); + const cwd = await mkdtemp(join(tmpdir(), "cl7789-mention-bak-cwd-")); + try { + const backup = `${fake.codexAuth}.bak-2026-01-01`; + await writeFile(backup, `${CODEX_BYTES}\n`); + const resolved = await resolveAtMentions(`read @${backup}`, cwd); + expect(resolved).toContain("(blocked: sensitive path)"); + expect(resolved).not.toContain("CODEX-FIXTURE-ACCESS"); + } finally { + await fake.cleanup(); + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("a symlink to a credential file is blocked by mention and by read", async () => { + const fake = await fakeHome(); + const cwd = await mkdtemp(join(tmpdir(), "cl7789-symlink-cwd-")); + try { + const link = join(cwd, "link-to-codex-auth"); + await symlink(fake.codexAuth, link); + const denied = await denyReason(link); + expect(denied.isError).toBe(true); + expect(String(denied.content)).toMatch(/sensitive file blocked/); + const resolved = await resolveAtMentions(`read @${link}`, cwd); + expect(resolved).toContain("(blocked: sensitive path)"); + expect(resolved).not.toContain("CODEX-FIXTURE-ACCESS"); + } finally { + await fake.cleanup(); + await rm(cwd, { recursive: true, force: true }); + } + }); +}); diff --git a/src/plugins/secret-guard-plugin.ts b/src/plugins/secret-guard-plugin.ts index 2c98745aa..10cc3fc3c 100644 --- a/src/plugins/secret-guard-plugin.ts +++ b/src/plugins/secret-guard-plugin.ts @@ -10,6 +10,7 @@ import { realpathNearestOr, UNRESOLVABLE, } from "../permission/path-restriction.js"; +import { buildCredentialPatterns } from "../auth/credential-surface.js"; import { productMutationPaths } from "../agent/product-mutation-tools.js"; import { looksLikePath } from "./path-escape-plugin.js"; @@ -88,6 +89,11 @@ const SENSITIVE_PATTERNS: RegExp[] = [ /(^|\/)\.config\/gcloud\//, // Azure CLI's credential cache — the equivalent of ~/.aws/credentials. /(^|\/)\.azure\/(accessTokens|azureProfile)\.json$/, + // The product's own OAuth token stores and credential sidecars come from the + // auth-owned registry, not literals here, so a new store cannot drift off + // the denylist. settings.json/permissions.json keep their hand-written + // patterns above; the registry only adds their lock/temp sidecars. + ...buildCredentialPatterns(), ]; export function isSensitivePath(value: string): boolean { @@ -177,7 +183,7 @@ function isPathLikeShellToken(token: string): boolean { // resolves the home symlink instead of a (usually missing) cwd child. // classify.ts's outside-workspace rule would ask anyway; the expansion fixes // the *reason* (sensitive-path) rather than relying on that coincidence. -function expandHome(token: string): string { +export function expandHome(token: string): string { if (token === "~") return homedir(); if (token.startsWith("~/")) return joinPath(homedir(), token.slice(2)); return token; diff --git a/src/tui/mention-resolution.ts b/src/tui/mention-resolution.ts index b2e9d183b..2a3832dce 100644 --- a/src/tui/mention-resolution.ts +++ b/src/tui/mention-resolution.ts @@ -1,6 +1,9 @@ import { readFile, opendir, realpath, stat } from "node:fs/promises"; import { resolve, isAbsolute } from "node:path"; -import { isSensitivePath } from "../plugins/secret-guard-plugin.js"; +import { + expandHome, + isSensitivePathResolved, +} from "../plugins/secret-guard-plugin.js"; const MAX_MENTION_FILE_BYTES = 200_000; const MAX_MENTION_TOTAL_BYTES = 400_000; @@ -44,7 +47,8 @@ async function summarizeDir(abs: string): Promise { return parts.length > 0 ? parts.join(", ") : "empty directory"; } -// An @mention is the operator directly asking the agent to read one path, once, +// `~` expansion lives in secret-guard-plugin.ts (single owner); the comment +// there documents the ordering. An @mention is the operator directly asking the agent to read one path, once, // right now — the same consent that already lets the agent read any workspace // file. There is no workspace-boundary check here: mentioning a path outside // the workspace inlines it exactly like a workspace path would, gated only by @@ -75,14 +79,8 @@ export async function resolveAtMentions( }); continue; } - if (path === "~" || path.startsWith("~/")) { - replacements.push({ - full, - replacement: `${full} (blocked: home-relative paths are not supported)`, - }); - continue; - } - if (isSensitivePath(path)) { + const expanded = expandHome(path); + if (isSensitivePathResolved(expanded)) { replacements.push({ full, replacement: `${full} (blocked: sensitive path)`, @@ -91,12 +89,14 @@ export async function resolveAtMentions( } let abs: string; try { - abs = await realpath(isAbsolute(path) ? path : resolve(cwd, path)); + abs = await realpath( + isAbsolute(expanded) ? expanded : resolve(cwd, expanded), + ); } catch { replacements.push({ full, replacement: `${full} (not found)` }); continue; } - if (isSensitivePath(abs)) { + if (isSensitivePathResolved(abs)) { replacements.push({ full, replacement: `${full} (blocked: sensitive path)`, diff --git a/tests/unit/tui/at-mention-resolution.test.ts b/tests/unit/tui/at-mention-resolution.test.ts index 3ff231300..d045e14d5 100644 --- a/tests/unit/tui/at-mention-resolution.test.ts +++ b/tests/unit/tui/at-mention-resolution.test.ts @@ -236,13 +236,28 @@ describe("resolveAtMentions", () => { } }); - test("blocks home-relative paths", async () => { + test("expands home-relative paths before the sensitivity check", async () => { const dir = await fixture(); try { - const resolved = await resolveAtMentions("read @~/some-file.txt", dir); - expect(resolved).toContain( - "(blocked: home-relative paths are not supported)", + const resolved = await resolveAtMentions( + "read @~/.corbits/settings.json", + dir, + ); + expect(resolved).toContain("(blocked: sensitive path)"); + expect(resolved).not.toContain("home-relative paths are not supported"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("reports missing home-relative files as not found", async () => { + const dir = await fixture(); + try { + const resolved = await resolveAtMentions( + "read @~/some-file-that-does-not-exist.txt", + dir, ); + expect(resolved).toContain("(not found)"); } finally { await rm(dir, { recursive: true, force: true }); }