diff --git a/docs/architecture.md b/docs/architecture.md index 5bfe78d..f66f773 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -228,7 +228,7 @@ Daemon installation paths per OS, log locations, and the configuration table liv Three places own the security contract: - **Encryptor**: the only path that generates age identities, derives recipients, and encrypts content. Plaintext never leaves this layer for any artefact going to disk or to the network. -- **Sanitiser**: the only place that decides what is safe to encrypt. Never-sync paths and literal-secret detection are hard-coded rules, not opt-in policy. Loosening either requires a documented reason. +- **Sanitiser**: the only place that decides what is safe to encrypt. Never-sync paths are hard-coded rules. Literal-secret detection is a **known-credential-format** guard (vendor key prefixes, AWS/GitHub/GitLab/Slack/Google tokens, age identities, PEM private keys, and JWTs in strict mode) — not a general secret scanner: a plain password or bespoke token with no recognised shape is not caught, so encryption, not the scan, is the real protection. The scan's job is to keep well-known credentials out of git history. Its breadth and the base64 redactor are tuned through the `[security]` config, resolved by `securityToPolicy`; the never-sync rules are not configurable. - **Tar bundler**: exists because some agent assets are directory-shaped. The tar is built in memory before encryption so an intermediate plaintext archive never lands on disk. Private keys stay on disk in the local runtime directory (`~/.config/agentsync/key.txt` by default on Unix, with restrictive permissions). They are never committed and never logged. diff --git a/docs/commands.md b/docs/commands.md index b7185c9..6e3808d 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -346,18 +346,23 @@ agentsync config set security.allowSecretValues '["AKIA-not-a-real-key"]' | `sync.debounceMs` | integer 50–10000 | Daemon quiet-window before an auto-push. | | `sync.autoPush` | boolean | Whether the daemon auto-pushes on change. | | `claudePlugins.syncPlugins` | boolean | Record the Claude plugin reinstall manifest on push. | -| `security.secretScan` | `standard`\|`strict`\|`off` | Push-time secret-scan mode (see note). | -| `security.allowSecretValues` | string[] (JSON) | Literal values exempt from secret detection and base64 redaction (see note). | -| `security.redactBase64Values` | boolean | Replace long base64-looking JSON values with a redaction placeholder (see note). | - -> **`security.*` are recorded but not yet enforced.** This release stores the -> policy in `agentsync.toml`; the push-time secret scanner starts honouring -> `secretScan`, `allowSecretValues`, and `redactBase64Values` in a follow-up -> change. Until then the scan runs with its built-in defaults regardless of -> these values. Note `agentsync.toml` is committed in **plaintext** (only -> artefacts are encrypted), so `allowSecretValues` is for exempting legitimate +| `security.secretScan` | `standard`\|`strict`\|`off` | Push-time secret-scan mode. `standard` = built-in credential patterns; `strict` also flags JWTs; `off` disables the artefact-body scan. | +| `security.allowSecretValues` | string[] (JSON) | Literal values exempt from secret detection and base64 redaction. | +| `security.redactBase64Values` | boolean | When `true` (default), redact long base64-looking JSON values; set `false` if a config legitimately stores base64 that must round-trip. | + +> **What the secret scan is — and is not.** It matches a fixed set of +> high-precision **credential formats** (vendor API-key prefixes, AWS/GitHub/GitLab/Slack/Google +> tokens, age identities, PEM private-key headers; `strict` adds JWTs). It is +> **not** a general secret scanner — a plain password, a bespoke token, or a +> connection string with no recognised shape passes through. Encryption is the +> real protection; the scan only stops well-known credentials from entering git +> history. `off` disables the artefact-body scan, but **skill-bundle interiors +> are always scanned at `standard`** as a fail-safe. `agentsync.toml` itself is +> committed in **plaintext**, so `allowSecretValues` is for exempting legitimate > high-entropy *non-secret* values — never paste a real credential there. -> `config set` refuses a recognised credential in any other value. +> `config set` refuses to store a recognised credential in any key other than +> `security.allowSecretValues`. +> See [Push aborts because secrets were detected](operations.md#push-aborts-because-secrets-were-detected). **Outcome**: `list` and `get` are read-only. `set` validates the new value against the full config schema (so an out-of-range debounce or an invalid enum is rejected before anything is written), then — because `agentsync.toml` is shared across machines — reconciles fast-forward, commits, and pushes the change, exactly like `key add`. diff --git a/docs/operations.md b/docs/operations.md index 50aa535..2697a9b 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -310,7 +310,7 @@ and behaviour reference. ### Push aborts because secrets were detected -The sanitiser found literal tokens or credentials in content that would otherwise be encrypted and committed. Sanitiser hits are intentionally a hard stop — they prevent the agent from leaking a secret into the vault, where it would persist even after subsequent pushes. +The sanitiser found a literal token or credential in content that would otherwise be encrypted and committed. Sanitiser hits are intentionally a hard stop — a secret in git history persists even after later pushes and is one key-compromise away from retroactive exposure. Fix: @@ -320,6 +320,27 @@ Fix: Do not bypass this by editing the vault manually. +#### What the scan actually covers + +Be precise about the guarantee. The scan is a **known-credential-format** detector, not a general secret scanner. It matches a fixed set of high-precision patterns: + +- vendor API-key prefixes (`sk-ant-…`, `sk-proj-…`), GitHub (`ghp_…`, `github_pat_…`), GitLab (`glpat-…`), AWS access keys (`AKIA…`), Google (`AIza…`), Slack (`xox[abprs]-…`); +- AgentSync's own age identity (`AGE-SECRET-KEY-1…`); +- PEM private-key headers (`-----BEGIN … PRIVATE KEY-----`); +- JWTs (`eyJ….eyJ….…`) — only when `security.secretScan = "strict"`. + +What it does **not** catch: a plain password, a bespoke or internal API token, a database connection string, or any credential with no recognised shape. Those flow into the (encrypted) vault unflagged. **Encryption is the real protection** — the scan exists only to keep well-known credentials out of git history. Treat a clean push as "no recognised credential format found", not "no secrets present". + +#### Tuning the scan + +`agentsync config set security.secretScan `: + +- `standard` (default) — the built-in credential patterns above, minus JWTs. +- `strict` — adds JWT detection. Use when no legitimate JWT appears in your config. +- `off` — disables the artefact-body scan. **Skill-bundle interiors are still scanned at `standard`** as a fail-safe, and encryption still applies. + +`agentsync config set security.allowSecretValues '[""]'` exempts a specific value the scanner false-positives on (and exempts it from base64 redaction). `agentsync config set security.redactBase64Values false` stops AgentSync replacing long base64-looking JSON values with a placeholder, for configs that legitimately store such values. See [config](commands.md#config). + ### Daemon is not running Check, in order: diff --git a/src/agents/claude/__tests__/sanitize.test.ts b/src/agents/claude/__tests__/sanitize.test.ts index c19af88..a6b078d 100644 --- a/src/agents/claude/__tests__/sanitize.test.ts +++ b/src/agents/claude/__tests__/sanitize.test.ts @@ -57,4 +57,25 @@ describe("claude-sanitize", () => { }; expect(out.hooks.PreToolUse[0]?.command).toBe(`${home}/runner`); }); + + // ─── Secret policy threading ────────────────────────────────────────────── + // Proves the adapter honours the SecretPolicy it is handed, not just the + // default — guards against an adapter silently dropping the policy argument. + + test("sanitizeClaudeMcp redacts a secret value under the default policy", () => { + const raw = JSON.stringify({ mcpServers: { x: { env: { TOKEN: `ghp_${"a".repeat(36)}` } } } }); + const out = JSON.parse(sanitizeClaudeMcp(raw, "").value) as { + mcpServers: { x: { env: { TOKEN: string } } }; + }; + expect(out.mcpServers.x.env.TOKEN).toContain("REDACTED"); + }); + + test("sanitizeClaudeMcp leaves the secret unredacted when policy mode is off", () => { + const token = `ghp_${"a".repeat(36)}`; + const raw = JSON.stringify({ mcpServers: { x: { env: { TOKEN: token } } } }); + const out = JSON.parse( + sanitizeClaudeMcp(raw, "", { mode: "off", allow: [], redactBase64: true }).value, + ) as { mcpServers: { x: { env: { TOKEN: string } } } }; + expect(out.mcpServers.x.env.TOKEN).toBe(token); + }); }); diff --git a/src/agents/claude/index.ts b/src/agents/claude/index.ts index 149e162..a721329 100644 --- a/src/agents/claude/index.ts +++ b/src/agents/claude/index.ts @@ -2,6 +2,7 @@ import { homedir } from "node:os"; import { AgentPaths } from "../../config/paths"; import type { AgentSyncConfig } from "../../config/schema"; import { denormalizeFromVault } from "../../core/path-portability"; +import { securityToPolicy } from "../../core/sanitizer"; import { type ApplyPlan, defineFileArtifact, @@ -28,6 +29,7 @@ export type ClaudeSnapshotResult = SnapshotResult; /** Collect Claude files that are safe to store in the encrypted vault. */ export async function snapshotClaude(config: AgentSyncConfig): Promise { const syncPlugins = config.claudePlugins?.syncPlugins ?? false; + const policy = securityToPolicy(config.security); const artifacts: SnapshotArtifact[] = []; const warnings: string[] = []; @@ -40,7 +42,7 @@ export async function snapshotClaude(config: AgentSyncConfig): Promise { const parsed = JSON.parse(rawSettingsJson) as Record; const hooksOnly = { hooks: parsed.hooks ?? {} }; const normalized = normalizeForVault(hooksOnly, home); - const redacted = redactSecretLiterals(normalized, "hooks"); + const redacted = redactSecretLiterals(normalized, "hooks", policy); return { value: `${JSON.stringify(redacted.value, null, 2)}\n`, warnings: redacted.warnings, @@ -42,11 +48,12 @@ export function sanitizeClaudeHooks( export function sanitizeClaudeMcp( rawClaudeJson: string, home: string = homedir(), + policy: SecretPolicy = DEFAULT_SECRET_POLICY, ): RedactionResult { const parsed = JSON.parse(rawClaudeJson) as Record; const mcpOnly = { mcpServers: parsed.mcpServers ?? {} }; const normalized = normalizeForVault(mcpOnly, home); - const redacted = redactSecretLiterals(normalized, "mcpServers"); + const redacted = redactSecretLiterals(normalized, "mcpServers", policy); return { value: `${JSON.stringify(redacted.value, null, 2)}\n`, warnings: redacted.warnings, diff --git a/src/agents/codex/index.ts b/src/agents/codex/index.ts index 6885af8..d9c0c04 100644 --- a/src/agents/codex/index.ts +++ b/src/agents/codex/index.ts @@ -3,7 +3,13 @@ import * as TOML from "@iarna/toml"; import { AgentPaths } from "../../config/paths"; import type { AgentSyncConfig } from "../../config/schema"; import { denormalizeFromVault, normalizeForVault } from "../../core/path-portability"; -import { type RedactionResult, redactSecretLiterals } from "../../core/sanitizer"; +import { + DEFAULT_SECRET_POLICY, + type RedactionResult, + redactSecretLiterals, + type SecretPolicy, + securityToPolicy, +} from "../../core/sanitizer"; import { type ApplyPlan, defineFileArtifact, @@ -30,7 +36,11 @@ export type CodexSnapshotResult = SnapshotResult; * Using TOML parse → redact → stringify avoids the line-level regex approach which * misses multi-line values and nested tables. */ -function sanitizeCodexConfig(raw: string, home: string = homedir()): RedactionResult { +function sanitizeCodexConfig( + raw: string, + home: string = homedir(), + policy: SecretPolicy = DEFAULT_SECRET_POLICY, +): RedactionResult { const warnings: string[] = []; let parsed: TOML.JsonMap; try { @@ -41,7 +51,7 @@ function sanitizeCodexConfig(raw: string, home: string = homedir()): RedactionRe } const normalized = normalizeForVault(parsed as unknown, home); - const redacted = redactSecretLiterals(normalized, "codex_config"); + const redacted = redactSecretLiterals(normalized, "codex_config", policy); warnings.push(...redacted.warnings); return { value: TOML.stringify(redacted.value as TOML.JsonMap), @@ -50,7 +60,8 @@ function sanitizeCodexConfig(raw: string, home: string = homedir()): RedactionRe } /** Collect Codex instructions, rules, and config that are safe to sync. */ -export async function snapshotCodex(_config?: AgentSyncConfig): Promise { +export async function snapshotCodex(config?: AgentSyncConfig): Promise { + const policy = securityToPolicy(config?.security); const artifacts: SnapshotArtifact[] = []; const warnings: string[] = []; @@ -69,7 +80,7 @@ export async function snapshotCodex(_config?: AgentSyncConfig): Promise { +export async function snapshotCursor(config?: AgentSyncConfig): Promise { + const policy = securityToPolicy(config?.security); const artifacts: SnapshotArtifact[] = []; const warnings: string[] = []; @@ -86,7 +87,7 @@ export async function snapshotCursor(_config?: AgentSyncConfig): Promise { +export async function snapshotVsCode(config?: AgentSyncConfig): Promise { + const policy = securityToPolicy(config?.security); const artifacts: SnapshotArtifact[] = []; const warnings: string[] = []; const mcpRaw = await readIfExists(AgentPaths.vscode.mcpJson); if (mcpRaw !== null) { - const sanitized = sanitizeAndNormalizeJson(mcpRaw, "vscode_mcp"); + const sanitized = sanitizeAndNormalizeJson(mcpRaw, "vscode_mcp", homedir(), policy); const artifact = collect(sanitized, AgentPaths.vscode.mcpJson, "vscode/mcp.json.age"); artifacts.push(artifact); warnings.push(...sanitized.warnings); diff --git a/src/commands/__tests__/push.test.ts b/src/commands/__tests__/push.test.ts index dd1c309..147b5a6 100644 --- a/src/commands/__tests__/push.test.ts +++ b/src/commands/__tests__/push.test.ts @@ -12,11 +12,13 @@ import { existsSync, mkdirSync, writeFileSync } from "node:fs"; import { readFile, rm } from "node:fs/promises"; import { createRequire } from "node:module"; import { join } from "node:path"; +import { loadConfig, resolveConfigPath, writeConfig } from "../../config/loader"; import { AgentPaths, machineVaultRoot } from "../../config/paths"; import { createBareRepo, createMachineFixture, createTmpDir, + runGit, seedVaultRepo, type TestMachineFixture, } from "../../test-helpers/fixtures"; @@ -404,6 +406,67 @@ describe("performPush — literal secret embedded in markdown body", () => { ).toBe(false); }); + test("honours config.security.secretScan = off — the secret is no longer scanned", async () => { + // Proves performPush threads the [security] policy into the scan gate: with + // the scan turned off in vault config, the same leaky body now pushes. + mkdirSync(mutableCopilotPaths.promptsDir, { recursive: true }); + const promptPath = join(mutableCopilotPaths.promptsDir, "leaky.prompt.md"); + const fakeKey = `sk-ant-api03-${"A".repeat(48)}`; + writeFileSync(promptPath, `# Demo prompt\n\nMy API key is ${fakeKey}\n`, "utf8"); + + const configPath = resolveConfigPath(machine.vaultDir); + const config = await loadConfig(configPath); + config.security.secretScan = "off"; + await writeConfig(configPath, config); + runGit(["commit", "-am", "config: scan off"], machine.vaultDir); + runGit(["push", "origin", "main"], machine.vaultDir); + + const result = await pushMod.performPush({ agent: "copilot" }); + expect(result.fatal).toBe(false); + expect(result.pushed).toBeGreaterThan(0); + }); + + test("strict mode flags a JWT that standard mode lets through", async () => { + mkdirSync(mutableCopilotPaths.promptsDir, { recursive: true }); + const promptPath = join(mutableCopilotPaths.promptsDir, "jwt.prompt.md"); + const jwt = `eyJ${"a".repeat(20)}.eyJ${"b".repeat(20)}.${"c".repeat(20)}`; + writeFileSync(promptPath, `# Demo\n\ntoken ${jwt}\n`, "utf8"); + + // standard: a JWT is not a flagged pattern → push succeeds. + const standard = await pushMod.performPush({ agent: "copilot" }); + expect(standard.fatal).toBe(false); + + // strict: JWT detection turns on → the same body now aborts the push. + const configPath = resolveConfigPath(machine.vaultDir); + const config = await loadConfig(configPath); + config.security.secretScan = "strict"; + await writeConfig(configPath, config); + runGit(["commit", "-am", "config: strict"], machine.vaultDir); + runGit(["push", "origin", "main"], machine.vaultDir); + + const strict = await pushMod.performPush({ agent: "copilot" }); + expect(strict.fatal).toBe(true); + expect(strict.errors.some((e) => e.includes("jwt"))).toBe(true); + }); + + test("allowSecretValues lets an embedded, exempted credential through the gate", async () => { + mkdirSync(mutableCopilotPaths.promptsDir, { recursive: true }); + const promptPath = join(mutableCopilotPaths.promptsDir, "allow.prompt.md"); + const key = `sk-ant-api03-${"A".repeat(48)}`; + writeFileSync(promptPath, `# Demo\n\nMy key is ${key} in a sentence.\n`, "utf8"); + + const configPath = resolveConfigPath(machine.vaultDir); + const config = await loadConfig(configPath); + config.security.allowSecretValues = [key]; + await writeConfig(configPath, config); + runGit(["commit", "-am", "config: allow key"], machine.vaultDir); + runGit(["push", "origin", "main"], machine.vaultDir); + + const result = await pushMod.performPush({ agent: "copilot" }); + expect(result.fatal).toBe(false); + expect(result.pushed).toBeGreaterThan(0); + }); + test("vaultPaths allowlist skips the secret scan for unselected files", async () => { // A secret-bearing prompt sits in one file; a clean instructions file // sits in another. With vaultPaths scoped to ONLY the clean file, the diff --git a/src/commands/push.ts b/src/commands/push.ts index c58fc0f..8ff31f2 100644 --- a/src/commands/push.ts +++ b/src/commands/push.ts @@ -7,7 +7,7 @@ import { NEVER_SYNC_WARNING_PREFIX, WALKER_SECRET_WARNING_PREFIX } from "../agen import { machineVaultRoot } from "../config/paths"; import { encryptString } from "../core/encryptor"; import { GitClient } from "../core/git"; -import { scanForSecrets, shouldNeverSync } from "../core/sanitizer"; +import { scanForSecrets, securityToPolicy, shouldNeverSync } from "../core/sanitizer"; import { loadVaultConfigOrExit, resolveRuntimeContext } from "./shared"; let agentDefinitions: AgentDefinition[] = Agents; @@ -85,6 +85,9 @@ export async function performPush( // v2: every artifact lands under this machine's namespace, never the flat root. const machineRoot = machineVaultRoot(runtime.vaultDir, runtime.machineName); const recipients = Object.values(config.recipients); + // Secret-scan policy from [security]: honours mode (standard/strict/off) and + // the allow-list for the central artifact-body scan below. + const secretPolicy = securityToPolicy(config.security); if (recipients.length === 0) { errors.push("No recipients found in agentsync.toml. Run `agentsync init` first."); @@ -173,7 +176,7 @@ export async function performPush( if (artifact.vaultPath.endsWith(".tar.age")) { continue; } - for (const w of scanForSecrets(artifact.plaintext, artifact.sourcePath)) { + for (const w of scanForSecrets(artifact.plaintext, artifact.sourcePath, secretPolicy)) { secretErrors.push(`[${agent.name}] ${w}`); } } diff --git a/src/config/schema.ts b/src/config/schema.ts index 8edb74b..682b736 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -86,14 +86,11 @@ export const AgentSyncConfigSchema = z.object({ }) .default({ syncPlugins: false }), ), - // Secret-handling policy. Optional with safe defaults so existing - // agentsync.toml files validate unchanged. The schema is not `.strict()`, so - // an older binary that predates this section ignores it on load rather than - // failing — adding the section needs no vault version bump. - // - // NOTE: this section is the configuration surface only. The push-time secret - // scanner reads these fields in a follow-up change; until then the values are - // recorded but not yet enforced (the scan runs with its built-in defaults). + // Secret-handling policy honoured by the push-time secret scanner and the + // JSON redactor (`src/core/sanitizer.ts`, resolved via `securityToPolicy`). + // Optional with safe defaults so existing agentsync.toml files validate + // unchanged. The schema is not `.strict()`, so an older binary that predates + // this section ignores it on load rather than failing — no version bump. security: z .object({ // How the push-time secret scan behaves: diff --git a/src/core/__tests__/sanitizer.test.ts b/src/core/__tests__/sanitizer.test.ts index adf23b4..3840e00 100644 --- a/src/core/__tests__/sanitizer.test.ts +++ b/src/core/__tests__/sanitizer.test.ts @@ -5,8 +5,10 @@ import { NEVER_SYNC_PATTERNS, redactionEnvNameForPath, redactSecretLiterals, + type SecretPolicy, sanitizeAndNormalizeJson, scanForSecrets, + securityToPolicy, shouldNeverSync, } from "../sanitizer"; @@ -235,3 +237,69 @@ describe("sanitizer", () => { expect(out.value.endsWith("\n")).toBe(true); }); }); + +describe("secret policy", () => { + const strict: SecretPolicy = { mode: "strict", allow: [], redactBase64: true }; + const off: SecretPolicy = { mode: "off", allow: [], redactBase64: true }; + + test("securityToPolicy maps the [security] config and defaults to standard", () => { + expect(securityToPolicy(undefined).mode).toBe("standard"); + expect( + securityToPolicy({ + secretScan: "strict", + allowSecretValues: ["x"], + redactBase64Values: false, + }), + ).toEqual({ mode: "strict", allow: ["x"], redactBase64: false }); + }); + + test("scanForSecrets detects a PEM private-key header in every mode", () => { + const body = "key:\n-----BEGIN OPENSSH PRIVATE KEY-----\nabc\n"; + expect(scanForSecrets(body, "/tmp/x").some((w) => w.includes("private-key-pem"))).toBe(true); + }); + + test("scanForSecrets flags a JWT only in strict mode", () => { + const jwt = `eyJ${"a".repeat(12)}.eyJ${"b".repeat(12)}.${"c".repeat(12)}`; + const body = `token: ${jwt}`; + expect(scanForSecrets(body, "/tmp/x")).toEqual([]); // standard: not flagged + expect(scanForSecrets(body, "/tmp/x", strict).some((w) => w.includes("jwt"))).toBe(true); + }); + + test("mode 'off' disables scanning and redaction", () => { + const secret = `ghp_${"a".repeat(36)}`; + expect(scanForSecrets(`x ${secret}`, "/tmp/x", off)).toEqual([]); + const redacted = redactSecretLiterals({ token: secret }, "root", off); + expect((redacted.value as { token: string }).token).toBe(secret); + expect(redacted.warnings).toHaveLength(0); + }); + + test("an allow-listed decoy does not mask a real secret of the same shape later", () => { + // Regression: a non-global match would only check the FIRST occurrence, so + // an allow-listed value earlier in the text could hide a real secret later. + const decoy = `ghp_${"a".repeat(36)}`; + const real = `ghp_${"b".repeat(36)}`; + const policy: SecretPolicy = { mode: "standard", allow: [decoy], redactBase64: true }; + const warnings = scanForSecrets(`example: ${decoy}\nreal key: ${real}`, "/tmp/x", policy); + expect(warnings.some((w) => w.includes("github-classic-pat"))).toBe(true); + }); + + test("allowSecretValues exempts a value from both the scan and the redactor", () => { + const secret = `ghp_${"a".repeat(36)}`; + const policy: SecretPolicy = { mode: "standard", allow: [secret], redactBase64: true }; + expect(scanForSecrets(secret, "/tmp/x", policy)).toEqual([]); + const redacted = redactSecretLiterals({ token: secret }, "root", policy); + expect((redacted.value as { token: string }).token).toBe(secret); + }); + + test("redactBase64 false keeps a long base64 value while still redacting real keys", () => { + const base64 = "QUJDREVGR0hJSktMTU5PUFFSU1RVVldYWVowMTIzNDU2Nzg5"; // 48 base64 chars + const realKey = `ghp_${"a".repeat(36)}`; + const keep: SecretPolicy = { mode: "standard", allow: [], redactBase64: false }; + const out = redactSecretLiterals({ blob: base64, tok: realKey }, "root", keep).value as { + blob: string; + tok: string; + }; + expect(out.blob).toBe(base64); // base64 catch-all disabled + expect(out.tok).toContain("REDACTED"); // a real key prefix still redacts + }); +}); diff --git a/src/core/sanitizer.ts b/src/core/sanitizer.ts index 3380b7f..5cc9fe9 100644 --- a/src/core/sanitizer.ts +++ b/src/core/sanitizer.ts @@ -58,6 +58,12 @@ function globToRegex(glob: string): RegExp { const NEVER_SYNC_REGEXPS: RegExp[] = NEVER_SYNC_PATTERNS.map((p) => globToRegex(p)); +// The generic base64 whole-value catch-all. Broad by nature, so it is the one +// redaction pattern gated behind `redactBase64Values`: a config can disable it +// when it legitimately stores long base64 values that must round-trip +// unchanged. Pulled out as a named const so it can be filtered out by policy. +const BASE64_VALUE_PATTERN = /^[A-Za-z0-9+/]{40,}={0,2}$/; + // Anchored patterns: used by `redactSecretLiterals` to replace an entire JSON // string value with the redaction placeholder. The generic base64 catch-all // only belongs here, since unanchored it would false-positive on long @@ -73,7 +79,7 @@ const WHOLE_VALUE_SECRET_PATTERNS = [ // hyphens in the prefix mean the base64 catch-all below can never match it, // so this anchored entry is required to redact a key pasted as a JSON value. /^AGE-SECRET-KEY-1[A-Z0-9]{58}$/, - /^[A-Za-z0-9+/]{40,}={0,2}$/, + BASE64_VALUE_PATTERN, ]; // Unanchored, high-precision credential prefixes. Used by `scanForSecrets` @@ -99,8 +105,57 @@ export const EMBEDDED_SECRET_PATTERNS: ReadonlyArray<{ name: string; pattern: Re // native X25519 key bech32-encodes to a fixed 58-char body; the 16-char // prefix makes false positives on prose effectively impossible. { name: "age-secret-key", pattern: /AGE-SECRET-KEY-1[A-Z0-9]{58}/ }, + // A PEM private-key header is never legitimate in synced agent config, and + // the fixed banner makes a false positive on prose impossible — so it is + // scanned in every mode, not gated behind `strict`. + { name: "private-key-pem", pattern: /-----BEGIN (?:[A-Z0-9]+ )?PRIVATE KEY-----/ }, +]; + +// Additional patterns scanned only in `strict` mode. A JWT legitimately appears +// in API examples and docs, so its higher false-positive rate is opt-in rather +// than aborting every push that mentions one. +export const STRICT_SECRET_PATTERNS: ReadonlyArray<{ name: string; pattern: RegExp }> = [ + { + name: "jwt", + pattern: /eyJ[A-Za-z0-9_-]{10,}\.eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}/, + }, ]; +/** Secret-handling policy resolved from the vault's [security] config section. */ +export interface SecretPolicy { + /** `standard` (built-in patterns), `strict` (+ JWT), or `off` (no scan/redact). */ + mode: "standard" | "strict" | "off"; + /** Literal values exempt from both detection and redaction. */ + allow: readonly string[]; + /** When false, the generic base64 whole-value redaction is skipped. */ + redactBase64: boolean; +} + +/** + * The behaviour when no [security] config is supplied: the historical default. + * Frozen so the shared instance can never be mutated (e.g. a stray push to + * `allow`) and contaminate every other caller in a long-lived daemon process. + */ +export const DEFAULT_SECRET_POLICY: SecretPolicy = Object.freeze({ + mode: "standard", + allow: Object.freeze([]) as readonly string[], + redactBase64: true, +}); + +/** Resolve a {@link SecretPolicy} from the optional [security] config section. */ +export function securityToPolicy(security?: { + secretScan?: "standard" | "strict" | "off"; + allowSecretValues?: readonly string[]; + redactBase64Values?: boolean; +}): SecretPolicy { + if (!security) return DEFAULT_SECRET_POLICY; + return { + mode: security.secretScan ?? "standard", + allow: security.allowSecretValues ?? [], + redactBase64: security.redactBase64Values ?? true, + }; +} + export interface RedactionResult { value: T; warnings: string[]; @@ -116,8 +171,12 @@ export function shouldNeverSync(path: string): boolean { return NEVER_SYNC_REGEXPS.some((re) => re.test(normalized)); } -function looksLikeSecretLiteral(value: string): boolean { - return WHOLE_VALUE_SECRET_PATTERNS.some((pattern) => pattern.test(value)); +function looksLikeSecretLiteral(value: string, policy: SecretPolicy): boolean { + if (policy.allow.includes(value)) return false; + const patterns = policy.redactBase64 + ? WHOLE_VALUE_SECRET_PATTERNS + : WHOLE_VALUE_SECRET_PATTERNS.filter((pattern) => pattern !== BASE64_VALUE_PATTERN); + return patterns.some((pattern) => pattern.test(value)); } /** @@ -133,11 +192,28 @@ function looksLikeSecretLiteral(value: string): boolean { * detects, the push aborts, the user removes the secret. Nothing is * actually redacted in this path. */ -export function scanForSecrets(text: string, sourcePath: string): string[] { +export function scanForSecrets( + text: string, + sourcePath: string, + policy: SecretPolicy = DEFAULT_SECRET_POLICY, +): string[] { + if (policy.mode === "off") return []; + const patterns = + policy.mode === "strict" + ? [...EMBEDDED_SECRET_PATTERNS, ...STRICT_SECRET_PATTERNS] + : EMBEDDED_SECRET_PATTERNS; const warnings: string[] = []; - for (const { name, pattern } of EMBEDDED_SECRET_PATTERNS) { - if (pattern.test(text)) { - warnings.push(`Detected literal secret (${name}) in ${sourcePath}`); + for (const { name, pattern } of patterns) { + // Iterate EVERY occurrence (a global clone of the pattern) and exempt only + // the exact allow-listed literals. A non-global `match` would return just + // the FIRST occurrence, so an allow-listed decoy earlier in the text could + // mask a real secret of the same shape later — a fail-open leak. + const global = pattern.global ? pattern : new RegExp(pattern.source, `${pattern.flags}g`); + for (const m of text.matchAll(global)) { + if (!policy.allow.includes(m[0])) { + warnings.push(`Detected literal secret (${name}) in ${sourcePath}`); + break; // one warning per pattern is enough to abort the push + } } } return warnings; @@ -147,9 +223,17 @@ export function scanForSecrets(text: string, sourcePath: string): string[] { export function redactSecretLiterals( input: unknown, fieldName = "value", + policy: SecretPolicy = DEFAULT_SECRET_POLICY, ): RedactionResult { + // `off` disables redaction entirely — the input is returned unchanged and BY + // REFERENCE (no clone). Every in-repo caller re-serialises the result, so the + // alias never escapes; a future caller that mutates `.value` must clone first. + if (policy.mode === "off") { + return { value: input, warnings: [] }; + } + if (typeof input === "string") { - if (looksLikeSecretLiteral(input)) { + if (looksLikeSecretLiteral(input, policy)) { return { value: `$AGENTSYNC_REDACTED_${fieldName.toUpperCase()}`, warnings: [`Detected literal secret for field ${fieldName}`], @@ -161,7 +245,7 @@ export function redactSecretLiterals( if (Array.isArray(input)) { const warnings: string[] = []; const value = input.map((item, index) => { - const nested = redactSecretLiterals(item, `${fieldName}_${index}`); + const nested = redactSecretLiterals(item, `${fieldName}_${index}`, policy); warnings.push(...nested.warnings); return nested.value; }); @@ -173,7 +257,7 @@ export function redactSecretLiterals( const result: Record = {}; for (const [key, value] of Object.entries(input)) { - const nested = redactSecretLiterals(value, key); + const nested = redactSecretLiterals(value, key, policy); warnings.push(...nested.warnings); result[key] = nested.value; } @@ -193,10 +277,11 @@ export function sanitizeAndNormalizeJson( raw: string, fieldName: string, home: string = homedir(), + policy: SecretPolicy = DEFAULT_SECRET_POLICY, ): RedactionResult { const parsed: unknown = JSON.parse(raw); const normalized = normalizeForVault(parsed, home); - const redacted = redactSecretLiterals(normalized, fieldName); + const redacted = redactSecretLiterals(normalized, fieldName, policy); return { value: `${JSON.stringify(redacted.value, null, 2)}\n`, warnings: redacted.warnings,