fix(security): always block the age key and PEM private keys even when secretScan=off

docs/commands.md

@@ -376,8 +376,8 @@ 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. `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.secretScan` | `standard`\|`strict`\|`off` | Push-time secret-scan mode. `standard` = built-in credential patterns; `strict` also flags JWTs; `off` waives the ordinary API-token patterns (the catastrophic tier — age key, PEM private keys — still blocks in every mode). |
+| `security.allowSecretValues` | string[] (JSON) | Literal values exempt from ordinary-token detection and base64 redaction. Catastrophic-tier values (age key, PEM private keys) are never exemptible. |
 | `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
@@ -386,8 +386,10 @@ agentsync config set security.allowSecretValues '["AKIA-not-a-real-key"]'
 > **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
+> history. `off` waives the ordinary API-token patterns, but the **catastrophic
+> tier (age key, PEM private keys) still blocks in every mode**, and
+> **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 to store a recognised credential in any key other than

docs/operations.md

@@ -347,7 +347,7 @@ What it does **not** catch: a plain password, a bespoke or internal API token, a
 
 - `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.
+- `off` — waives the ordinary API-token patterns; those values ride into the (encrypted) vault unflagged. The **catastrophic tier still blocks in every mode, `off` included**: the vault's own age key (`AGE-SECRET-KEY-1…`) and PEM private keys can never be pushed — no encryption makes it safe to commit the key that decrypts the vault itself. **Skill-bundle interiors are still scanned at `standard`** as a fail-safe.
 
 `agentsync config set security.allowSecretValues '["<literal>"]'` 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).
 

src/agents/skills-walker.ts

@@ -374,6 +374,12 @@ export async function collectInteriorViolations(rootDir: string): Promise<Interi
         // above is already best-effort for non-readable files. Skip silently.
         continue;
       }
+      // No policy argument on purpose: bundle interiors are always scanned at
+      // the default `standard` policy, independent of the vault's `secretScan`
+      // setting. This is the documented fail-safe — do NOT thread the user
+      // policy through here, or an `off` vault would stop scanning skill bodies.
+      // (The catastrophic tier blocks in every mode regardless, but ordinary
+      // tokens inside bundles rely on this hardcoded `standard`.)
       secretWarnings.push(...scanForSecrets(body, childPath));
     }
   }

src/commands/__tests__/config.test.ts

@@ -122,6 +122,32 @@ describe("config command", () => {
     expect(config.security.allowSecretValues).toEqual(["AKIAEXAMPLE", "ghp_example"]);
   });
 
+  test("performConfigSet refuses a catastrophic-tier value even in allowSecretValues", async () => {
+    // allowSecretValues is exempt from ordinary-token detection, but the age
+    // key that decrypts the vault must never land in plaintext agentsync.toml —
+    // otherwise it would bypass the always-block guarantee at push time.
+    const ageKey = `AGE-SECRET-KEY-1${"A".repeat(58)}`;
+    const result = await configMod.performConfigSet(
+      "security.allowSecretValues",
+      JSON.stringify([ageKey]),
+    );
+    expect(result.status).toBe("invalid-value");
+    const config = await loadConfig(resolveConfigPath(machine.vaultDir));
+    expect(config.security.allowSecretValues).toEqual([]);
+  });
+
+  test("performConfigSet rejects a JSON-escaped catastrophic value (decoded scan)", async () => {
+    // The persisted value is parseScalar(rawValue), so a unicode-escaped age
+    // key would pass a raw-string scan but decode to a real secret on disk.
+    // Scanning the decoded value closes that bypass.
+    const ageKey = `AGE-SECRET-KEY-1${"A".repeat(58)}`;
+    const escaped = `["\\u0041${ageKey.slice(1)}"]`; // A decodes to "A"
+    const result = await configMod.performConfigSet("security.allowSecretValues", escaped);
+    expect(result.status).toBe("invalid-value");
+    const config = await loadConfig(resolveConfigPath(machine.vaultDir));
+    expect(config.security.allowSecretValues).toEqual([]);
+  });
+
   test("performConfigSet refuses protected sections", async () => {
     for (const key of ["version", "recipients.config-test", "remote.url"]) {
       const result = await configMod.performConfigSet(key, "x");

src/commands/config.ts

@@ -164,20 +164,37 @@ export async function performConfigSet(key: string, rawValue: string): Promise<C
 
     // agentsync.toml is committed in PLAINTEXT (only artifacts are encrypted),
     // so refuse to write a literal credential into it — e.g. a token pasted
-    // into the wrong field. The allowlist key is exempt by construction.
-    if (key !== SECRET_EXEMPT_KEY) {
-      const leaks = scanForSecrets(rawValue, key);
-      if (leaks.length > 0) {
-        return {
-          status: "invalid-value",
-          key,
-          error: `Refusing to store a literal secret in plaintext config (${leaks.join("; ")}).`,
-        };
-      }
+    // into the wrong field. The allowlist key is exempt from ordinary-token
+    // detection by construction (it exists to hold high-entropy false
+    // positives), but NEVER from the catastrophic tier: an age secret key or
+    // PEM private key must not land in plaintext config even as an "allowed"
+    // value, or it would sail past the always-block guarantee at push time.
+    // `off` mode scans exactly the catastrophic tier.
+    //
+    // Scan the DECODED value, not the raw string: `parseScalar` is what gets
+    // persisted, so a JSON-escaped form (e.g. "AGE-SECRET-KEY-…") would
+    // pass a raw-string scan yet decode into a real secret on disk.
+    const parsedValue = parseScalar(rawValue);
+    const exemptKey = key === SECRET_EXEMPT_KEY;
+    const scanTargets =
+      exemptKey && Array.isArray(parsedValue)
+        ? parsedValue.filter((v): v is string => typeof v === "string")
+        : [typeof parsedValue === "string" ? parsedValue : rawValue];
+    const leaks = scanTargets.flatMap((value) =>
+      exemptKey
+        ? scanForSecrets(value, key, { mode: "off", allow: [], redactBase64: true })
+        : scanForSecrets(value, key),
+    );
+    if (leaks.length > 0) {
+      return {
+        status: "invalid-value",
+        key,
+        error: `Refusing to store a literal secret in plaintext config (${leaks.join("; ")}).`,
+      };
     }
 
     const next = structuredClone(refreshed);
-    setByPath(next as unknown as Json, key, parseScalar(rawValue));
+    setByPath(next as unknown as Json, key, parsedValue);
 
     const validated = AgentSyncConfigSchema.safeParse(next);
     if (!validated.success) {

src/config/schema.ts

@@ -95,8 +95,10 @@ export const AgentSyncConfigSchema = z.object({
     .object({
       // How the push-time secret scan behaves:
       //   standard — the built-in high-precision credential patterns (default)
-      //   strict   — standard plus generic PEM private-key and JWT detection
-      //   off      — disable the embedded-secret scan (encryption still applies)
+      //   strict   — standard plus JWT detection
+      //   off      — waive the ordinary API-token patterns (values ride in
+      //              encrypted). The catastrophic tier (the vault's own age
+      //              key, PEM private keys) still blocks the push in EVERY mode.
       secretScan: z.enum(["standard", "strict", "off"]).default("standard"),
       // Literal values to exempt from secret detection AND base64 redaction.
       // The escape hatch for a legitimate high-entropy config value that the

src/core/__tests__/sanitizer.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test } from "bun:test";
 import { AGENTSYNC_HOME_PLACEHOLDER } from "../path-portability";
 import {
+  ALWAYS_BLOCK_PATTERNS,
   EMBEDDED_SECRET_PATTERNS,
   NEVER_SYNC_PATTERNS,
   redactionEnvNameForPath,
@@ -253,9 +254,44 @@ describe("secret policy", () => {
     ).toEqual({ mode: "strict", allow: ["x"], redactBase64: false });
   });
 
-  test("scanForSecrets detects a PEM private-key header in every mode", () => {
+  test("ALWAYS_BLOCK_PATTERNS is exactly the catastrophic tier", () => {
+    expect(ALWAYS_BLOCK_PATTERNS.map((p) => p.name).sort()).toEqual([
+      "age-secret-key",
+      "private-key-pem",
+    ]);
+  });
+
+  test("scanForSecrets detects a PEM private-key header in every mode, off included", () => {
     const body = "key:\n-----BEGIN OPENSSH PRIVATE KEY-----\nabc\n";
-    expect(scanForSecrets(body, "/tmp/x").some((w) => w.includes("private-key-pem"))).toBe(true);
+    const hit = (p?: SecretPolicy) =>
+      scanForSecrets(body, "/tmp/x", p).some((w) => w.includes("private-key-pem"));
+    expect(hit()).toBe(true); // standard (default)
+    expect(hit(strict)).toBe(true);
+    expect(hit(off)).toBe(true); // catastrophic tier survives `off`
+  });
+
+  test("the vault's own age key blocks the push in every mode", () => {
+    const ageKey = `AGE-SECRET-KEY-1${"A".repeat(58)}`;
+    const blocks = (p?: SecretPolicy) =>
+      scanForSecrets(`identity: ${ageKey}`, "/tmp/x", p).some((w) => w.includes("age-secret-key"));
+    expect(blocks()).toBe(true); // standard
+    expect(blocks(strict)).toBe(true);
+    expect(blocks(off)).toBe(true); // catastrophic tier survives `off`
+  });
+
+  test("allowSecretValues can NOT exempt a catastrophic-tier value", () => {
+    // Regression guard for the central guarantee: the allow-list silences
+    // ordinary tokens, never the age key / PEM that decrypt the vault.
+    const ageKey = `AGE-SECRET-KEY-1${"A".repeat(58)}`;
+    const exempt: SecretPolicy = { mode: "off", allow: [ageKey], redactBase64: true };
+    expect(scanForSecrets(ageKey, "/tmp/x", exempt)).toEqual([
+      "Detected literal secret (age-secret-key) in /tmp/x",
+    ]);
+    const pem = "-----BEGIN OPENSSH PRIVATE KEY-----";
+    const exemptPem: SecretPolicy = { mode: "standard", allow: [pem], redactBase64: true };
+    expect(
+      scanForSecrets(pem, "/tmp/x", exemptPem).some((w) => w.includes("private-key-pem")),
+    ).toBe(true);
   });
 
   test("scanForSecrets flags a JWT only in strict mode", () => {
@@ -265,7 +301,7 @@ describe("secret policy", () => {
     expect(scanForSecrets(body, "/tmp/x", strict).some((w) => w.includes("jwt"))).toBe(true);
   });
 
-  test("mode 'off' disables scanning and redaction", () => {
+  test("mode 'off' waives ordinary API-token patterns and redaction", () => {
     const secret = `ghp_${"a".repeat(36)}`;
     expect(scanForSecrets(`x ${secret}`, "/tmp/x", off)).toEqual([]);
     const redacted = redactSecretLiterals({ token: secret }, "root", off);

src/core/sanitizer.ts

@@ -111,6 +111,18 @@ export const EMBEDDED_SECRET_PATTERNS: ReadonlyArray<{ name: string; pattern: Re
   { name: "private-key-pem", pattern: /-----BEGIN (?:[A-Z0-9]+ )?PRIVATE KEY-----/ },
 ];
 
+// The catastrophic tier: patterns that block a push in EVERY mode — including
+// `off` — because they are never legitimate agent config and cannot be made
+// safe by encryption. The age secret key is the master key to the very vault
+// being written, so committing it (even encrypted to recipients) hands every
+// future reader the means to decrypt everything. A PEM private key is the same
+// class. Every other pattern is "ordinary" (an API token) whose handling the
+// `secretScan` mode chooses. Derived from EMBEDDED_SECRET_PATTERNS by name so
+// the pattern bodies stay defined in exactly one place.
+const ALWAYS_BLOCK_NAMES: ReadonlySet<string> = new Set(["age-secret-key", "private-key-pem"]);
+export const ALWAYS_BLOCK_PATTERNS: ReadonlyArray<{ name: string; pattern: RegExp }> =
+  EMBEDDED_SECRET_PATTERNS.filter((p) => ALWAYS_BLOCK_NAMES.has(p.name));
+
 // 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.
@@ -123,7 +135,11 @@ export const STRICT_SECRET_PATTERNS: ReadonlyArray<{ name: string; pattern: RegE
 
 /** 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). */
+  /**
+   * `standard` (built-in patterns), `strict` (+ JWT), or `off` (waive the
+   * ordinary API-token patterns). The catastrophic tier (age key, PEM) blocks
+   * in every mode, `off` included.
+   */
   mode: "standard" | "strict" | "off";
   /** Literal values exempt from both detection and redaction. */
   allow: readonly string[];
@@ -191,26 +207,37 @@ function looksLikeSecretLiteral(value: string, policy: SecretPolicy): boolean {
  * The prefix is a contract marker, not a description: the scan only
  * detects, the push aborts, the user removes the secret. Nothing is
  * actually redacted in this path.
+ *
+ * `off` does NOT mean "scan nothing": the catastrophic tier
+ * ({@link ALWAYS_BLOCK_PATTERNS} — age secret key, PEM private key) is scanned
+ * in every mode. `off` only waives the ordinary API-token patterns, accepting
+ * that those values ride into the vault protected by encryption alone.
  */
 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;
+    policy.mode === "off"
+      ? ALWAYS_BLOCK_PATTERNS
+      : policy.mode === "strict"
+        ? [...EMBEDDED_SECRET_PATTERNS, ...STRICT_SECRET_PATTERNS]
+        : EMBEDDED_SECRET_PATTERNS;
   const warnings: string[] = [];
   for (const { name, pattern } of patterns) {
+    // Catastrophic-tier hits (age secret key, PEM private key) are NEVER
+    // exemptible: an `allowSecretValues` entry must not be able to silence the
+    // key that decrypts the entire vault, or the always-block guarantee becomes
+    // a suggestion. Only ordinary patterns honour the allow-list.
+    const catastrophic = ALWAYS_BLOCK_NAMES.has(name);
     // 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])) {
+      if (catastrophic || !policy.allow.includes(m[0])) {
         warnings.push(`Detected literal secret (${name}) in ${sourcePath}`);
         break; // one warning per pattern is enough to abort the push
       }