From 2a5dc98df9e6f7a2a4f17971d663487f630d2fa1 Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:19:16 +0000 Subject: [PATCH] fix(export): redact four drifted secret column families and guard against future drift REDACTED_COLUMNS was missing four secret-bearing families added since the DO-NOT-MIGRATE list was last touched: orb_enrollments.secret_value_*, provider_credentials envelope, orb_instances.ingest_secret_hash and orb_webhook_events.payload_hash. Add them, and introduce EXPORT_REVIEWED_NON_SECRET_COLUMNS plus a migrations/-scanning drift-guard test so a future ciphertext/secret/token column fails the suite until it is classified as redacted or explicitly-reviewed non-secret. --- scripts/export-d1-core.ts | 41 ++++++- test/unit/export-d1-core.test.ts | 198 ++++++++++++++++++++++++++++++- 2 files changed, 234 insertions(+), 5 deletions(-) diff --git a/scripts/export-d1-core.ts b/scripts/export-d1-core.ts index 3b685734f2..95b6bfb851 100644 --- a/scripts/export-d1-core.ts +++ b/scripts/export-d1-core.ts @@ -41,22 +41,59 @@ export const EXCLUDED_TABLES: Set = new Set([ // committed credential never crosses the boundary (#selfhost-migration DO-NOT-MIGRATE list): // • auth_sessions.token_hash — hashed browser session tokens, scoped to the cloud deploy. // • webhook_events.payload_hash — per-delivery dedup hash, scoped to the cloud deploy. +// • orb_webhook_events.payload_hash — the Orb App's own per-delivery dedup hash; same purpose and +// cloud-scope as webhook_events.payload_hash, separate table. // • repository_ai_keys.ciphertext — maintainer BYOK provider keys, encrypted with the cloud key. // • repository_linear_keys.ciphertext — Linear API keys; same never-serialize envelope as AI keys (#6295). +// • provider_credentials.ciphertext / iv / — the instance's own subscription-CLI credential envelope +// salt (AES-256-GCM via TOKEN_ENCRYPTION_SECRET, same envelope as the +// BYOK key tables above); never serialized off the cloud (#9543). // • auth_session_github_tokens.ciphertext / — per-session GitHub OAuth envelopes; isolated so a full-row // refresh_ciphertext serialize can't leak them (#6295 / #6114). // • submission_user_tokens.encrypted_token — short-lived GitHub OAuth token envelopes, cloud-scoped. // • orb_enrollments.secret_hash — one-time enrollment secret hashes. // • orb_enrollments.relay_secret_* — encrypted relay webhook signing secret material. -// • orb_enrollments.cached_token_json — encrypted GitHub installation-token cache envelope. +// • orb_enrollments.cached_token_json — encrypted GitHub installation-token cache envelope. +// • orb_enrollments.secret_value_ciphertext — the STORED (not minted) secret-value envelope; the other half +// / secret_value_iv / secret_value_salt of this table's encrypted-secret family (#8064). +// • orb_instances.ingest_secret_hash — per-instance risk-control ingest secret hash; sibling of +// orb_enrollments.secret_hash (#9121). export const REDACTED_COLUMNS: Record = { auth_sessions: ["token_hash"], webhook_events: ["payload_hash"], + orb_webhook_events: ["payload_hash"], repository_ai_keys: ["ciphertext"], repository_linear_keys: ["ciphertext"], + provider_credentials: ["ciphertext", "iv", "salt"], auth_session_github_tokens: ["ciphertext", "refresh_ciphertext"], submission_user_tokens: ["encrypted_token"], - orb_enrollments: ["secret_hash", "relay_secret_enc", "relay_secret_iv", "relay_secret_salt", "cached_token_json"], + orb_enrollments: [ + "secret_hash", + "relay_secret_enc", + "relay_secret_iv", + "relay_secret_salt", + "cached_token_json", + "secret_value_ciphertext", + "secret_value_iv", + "secret_value_salt", + ], + orb_instances: ["ingest_secret_hash"], +}; + +// Columns whose name MATCHES the sensitive-name pattern the drift guard scans for +// (/ciphertext|encrypted|secret|token/i across migrations/**) but which are deliberately EXPORTED because they +// carry no credential — reviewed and stated here rather than silently omitted, the same "say why, don't just +// leave it out" convention scripts/check-dead-source-files.ts's STAGED_AHEAD_OF_CONSUMERS uses. Every entry +// must name a column that still exists in migrations/** (the drift guard fails on a dead entry) and must be a +// genuine non-secret; do NOT add a real credential here to silence the guard — redact it above instead. +export const EXPORT_REVIEWED_NON_SECRET_COLUMNS: Readonly> = { + // LLM usage-metering token COUNTS (integers), not credentials — needed on self-host to render AI-spend. + ai_usage_events: ["input_tokens", "output_tokens", "total_tokens"], + orb_enrollments: [ + "secret_type", // discriminator string ('github_token' | ...), selects which secret family a row holds. + "secret_value_version", // integer envelope-version for secret_value_*; the ciphertext itself is redacted. + "last_token_at", // timestamp of the last brokered-token mint; observability metadata, no secret material. + ], }; // A table name is only ever interpolated into SQL (`SELECT * FROM ""`) after passing this allowlist, so a diff --git a/test/unit/export-d1-core.test.ts b/test/unit/export-d1-core.test.ts index 5381fa3afd..667b611c06 100644 --- a/test/unit/export-d1-core.test.ts +++ b/test/unit/export-d1-core.test.ts @@ -1,5 +1,17 @@ +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { buildExportManifest, buildTableExport, checksumRows, EXCLUDED_TABLES, filterRowsSince, isSafeTableName, redactRow, REDACTED_COLUMNS } from "../../scripts/export-d1-core"; +import { + buildExportManifest, + buildTableExport, + checksumRows, + EXCLUDED_TABLES, + EXPORT_REVIEWED_NON_SECRET_COLUMNS, + filterRowsSince, + isSafeTableName, + redactRow, + REDACTED_COLUMNS, +} from "../../scripts/export-d1-core"; describe("export-d1-core isSafeTableName (SQL-injection guard)", () => { it("accepts plain SQL identifiers", () => { @@ -36,16 +48,60 @@ describe("export-d1-core redaction (#selfhost-migration)", () => { expect(REDACTED_COLUMNS).toMatchObject({ auth_sessions: ["token_hash"], webhook_events: ["payload_hash"], + orb_webhook_events: ["payload_hash"], repository_ai_keys: ["ciphertext"], repository_linear_keys: ["ciphertext"], + provider_credentials: ["ciphertext", "iv", "salt"], auth_session_github_tokens: ["ciphertext", "refresh_ciphertext"], submission_user_tokens: ["encrypted_token"], - orb_enrollments: ["secret_hash", "relay_secret_enc", "relay_secret_iv", "relay_secret_salt", "cached_token_json"], + orb_enrollments: [ + "secret_hash", + "relay_secret_enc", + "relay_secret_iv", + "relay_secret_salt", + "cached_token_json", + "secret_value_ciphertext", + "secret_value_iv", + "secret_value_salt", + ], + orb_instances: ["ingest_secret_hash"], }); expect(redactRow("webhook_events", { delivery_id: "d1", payload_hash: "h" })).toEqual({ delivery_id: "d1" }); expect(redactRow("repository_ai_keys", { repo_full_name: "o/r", ciphertext: "ENCRYPTED" })).toEqual({ repo_full_name: "o/r" }); }); + // #9651: four secret-bearing column families added since the DO-NOT-MIGRATE list was last touched. redactRow + // must drop each named column so no envelope/hash reaches the emitted export. + it("redacts the four column families that drifted out of the list (#9651)", () => { + const enrollment = redactRow("orb_enrollments", { + enroll_id: "e1", + secret_value_ciphertext: "LEAK_SECRET_VALUE_CIPHERTEXT", + secret_value_iv: "LEAK_SECRET_VALUE_IV", + secret_value_salt: "LEAK_SECRET_VALUE_SALT", + secret_value_version: 1, + }); + expect(enrollment).not.toHaveProperty("secret_value_ciphertext"); + expect(enrollment).not.toHaveProperty("secret_value_iv"); + expect(enrollment).not.toHaveProperty("secret_value_salt"); + expect(enrollment).toEqual({ enroll_id: "e1", secret_value_version: 1 }); + + const provider = redactRow("provider_credentials", { provider: "anthropic", ciphertext: "LEAK_PROVIDER_CIPHERTEXT", iv: "LEAK_IV", salt: "LEAK_SALT", last4: "abcd" }); + expect(provider).not.toHaveProperty("ciphertext"); + expect(provider).not.toHaveProperty("iv"); + expect(provider).not.toHaveProperty("salt"); + expect(provider).toEqual({ provider: "anthropic", last4: "abcd" }); + + const instance = redactRow("orb_instances", { instance_id: "i1", ingest_secret_hash: "LEAK_INGEST_SECRET_HASH" }); + expect(instance).not.toHaveProperty("ingest_secret_hash"); + expect(instance).toEqual({ instance_id: "i1" }); + + const orbWebhook = redactRow("orb_webhook_events", { delivery_id: "d1", payload_hash: "LEAK_ORB_PAYLOAD_HASH" }); + expect(orbWebhook).not.toHaveProperty("payload_hash"); + expect(orbWebhook).toEqual({ delivery_id: "d1" }); + + expect(JSON.stringify([enrollment, provider, instance, orbWebhook])).not.toMatch(/LEAK_/); + }); + // Schema isolates these ciphertext columns specifically so they are NEVER serialized (#6295). Keep an // explicit allowlist here rather than parsing schema comments (comment phrasing drifts); when a new // never-serialize ciphertext table lands, add it to both REDACTED_COLUMNS and this list. @@ -98,7 +154,16 @@ describe("export-d1-core redaction (#selfhost-migration)", () => { cached_token_json: "LEAK_CACHED_ORB_TOKEN_ENVELOPE", }, ]); - expect(orbExport?.redactedColumns).toEqual(["secret_hash", "relay_secret_enc", "relay_secret_iv", "relay_secret_salt", "cached_token_json"]); + expect(orbExport?.redactedColumns).toEqual([ + "secret_hash", + "relay_secret_enc", + "relay_secret_iv", + "relay_secret_salt", + "cached_token_json", + "secret_value_ciphertext", + "secret_value_iv", + "secret_value_salt", + ]); expect(orbExport?.rows).toEqual([{ enroll_id: "e1", installation_id: 42 }]); expect(JSON.stringify([draftExport, orbExport])).not.toMatch(/LEAK_/); @@ -206,3 +271,130 @@ describe("export-d1-core buildTableExport + manifest", () => { expect(JSON.stringify(manifest)).not.toContain('"rows"'); }); }); + +// #9651 drift guard: #6295 added two missing ciphertext columns to the redaction list and nothing else — no +// mechanism — so the list drifted again across four migrations (orb_enrollments.secret_value_*, +// provider_credentials.*, orb_instances.ingest_secret_hash, orb_webhook_events.payload_hash). These tests are +// that mechanism: mirroring retention.test.ts's "read the real migrations/ from disk and diff two derived sets" +// pattern, every column across migrations/** whose NAME matches the sensitive pattern must be classified as +// either redacted or explicitly-reviewed-non-secret, so a future secret column fails the suite until triaged. +const SENSITIVE_COLUMN = /ciphertext|encrypted|secret|token/i; +const CONSTRAINT_CLAUSE = /^(primary|foreign|unique|check|constraint)\b/i; + +/** Strip `-- …` line comments so a comment word (e.g. "instance secret") is never mistaken for a column name. */ +function stripSqlLineComments(sql: string): string { + return sql + .split("\n") + .map((line) => { + const at = line.indexOf("--"); + return at === -1 ? line : line.slice(0, at); + }) + .join("\n"); +} + +/** + * Every (table, column) declared in `sql` — both `ALTER TABLE … ADD COLUMN` and CREATE TABLE column bodies — + * whose column name matches the sensitive pattern. Constraint clauses (PRIMARY/FOREIGN/UNIQUE/CHECK/CONSTRAINT) + * are skipped, and none of their keywords match the pattern anyway, so the result is real columns only. + */ +function sensitiveColumnsInSql(sql: string): Array<{ table: string; column: string }> { + const clean = stripSqlLineComments(sql); + const found: Array<{ table: string; column: string }> = []; + + const addColumn = /alter\s+table\s+([a-z0-9_]+)\s+add\s+column\s+([a-z0-9_]+)/gi; + for (let m = addColumn.exec(clean); m; m = addColumn.exec(clean)) { + const [, table, column] = m; + if (table && column) found.push({ table, column }); + } + + const createTable = /create\s+table\s+(?:if\s+not\s+exists\s+)?([a-z0-9_]+)\s*\(/gi; + for (let m = createTable.exec(clean); m; m = createTable.exec(clean)) { + const table = m[1]; + if (!table) continue; + let depth = 1; + let i = m.index + m[0].length; + const bodyStart = i; + for (; i < clean.length && depth > 0; i++) { + if (clean[i] === "(") depth++; + else if (clean[i] === ")") depth--; + } + for (const raw of clean.slice(bodyStart, i - 1).split("\n")) { + const line = raw.trim(); + if (!line || CONSTRAINT_CLAUSE.test(line)) continue; + const col = /^([a-z_][a-z0-9_]*)/i.exec(line); + if (col?.[1]) found.push({ table, column: col[1] }); + } + } + + return found.filter(({ column }) => SENSITIVE_COLUMN.test(column)); +} + +function sensitiveColumnsInMigrations(): Array<{ table: string; column: string }> { + const dir = join(process.cwd(), "migrations"); + return readdirSync(dir) + .filter((name) => name.endsWith(".sql")) + .flatMap((name) => sensitiveColumnsInSql(readFileSync(join(dir, name), "utf8"))); +} + +function isClassified(table: string, column: string): boolean { + return Boolean(REDACTED_COLUMNS[table]?.includes(column) || EXPORT_REVIEWED_NON_SECRET_COLUMNS[table]?.includes(column)); +} + +describe("export-d1-core redaction-list drift guard (#9651)", () => { + it("classifies every sensitive-named column in migrations/ as redacted or reviewed-non-secret", () => { + const unclassified = sensitiveColumnsInMigrations() + .filter(({ table, column }) => !isClassified(table, column)) + .map(({ table, column }) => `${table}.${column}`) + .sort(); + expect(unclassified).toEqual([]); + }); + + it("proves the four drifted families are actually present in migrations/ and now redacted", () => { + const seen = new Set(sensitiveColumnsInMigrations().map(({ table, column }) => `${table}.${column}`)); + const drifted: Array<[string, string]> = [ + ["orb_enrollments", "secret_value_ciphertext"], + ["orb_enrollments", "secret_value_iv"], + ["orb_enrollments", "secret_value_salt"], + ["provider_credentials", "ciphertext"], + ["orb_instances", "ingest_secret_hash"], + ]; + for (const [table, column] of drifted) { + expect(seen.has(`${table}.${column}`)).toBe(true); + expect(REDACTED_COLUMNS[table]).toContain(column); + } + // orb_webhook_events.payload_hash is NOT sensitive-named, so it never reaches the guard — but the + // DO-NOT-MIGRATE list still redacts it (same purpose as webhook_events.payload_hash), which is the point. + expect(REDACTED_COLUMNS.orb_webhook_events).toContain("payload_hash"); + }); + + it("is not trivially green — the detection helper flags a synthetic secret column", () => { + const fixture = "ALTER TABLE t ADD COLUMN some_secret_ciphertext TEXT;"; + expect(sensitiveColumnsInSql(fixture)).toEqual([{ table: "t", column: "some_secret_ciphertext" }]); + expect(isClassified("t", "some_secret_ciphertext")).toBe(false); + + // It also catches a CREATE TABLE body column and ignores constraint clauses + comment words. + const createFixture = ["CREATE TABLE demo (", " id TEXT PRIMARY KEY,", " api_token TEXT NOT NULL, -- an instance secret in a COMMENT, not a column", " PRIMARY KEY (id)", ");"].join("\n"); + expect(sensitiveColumnsInSql(createFixture)).toEqual([{ table: "demo", column: "api_token" }]); + }); + + it("has no dead EXPORT_REVIEWED_NON_SECRET_COLUMNS entries — every reviewed column still exists in migrations/", () => { + const live = new Set(sensitiveColumnsInMigrations().map(({ table, column }) => `${table}.${column}`)); + const dead: string[] = []; + for (const [table, columns] of Object.entries(EXPORT_REVIEWED_NON_SECRET_COLUMNS)) { + for (const column of columns) { + if (!live.has(`${table}.${column}`)) dead.push(`${table}.${column}`); + } + } + expect(dead).toEqual([]); + }); + + it("keeps EXPORT_REVIEWED_NON_SECRET_COLUMNS reviewed — no redacted column is also listed as non-secret", () => { + const overlap: string[] = []; + for (const [table, columns] of Object.entries(EXPORT_REVIEWED_NON_SECRET_COLUMNS)) { + for (const column of columns) { + if (REDACTED_COLUMNS[table]?.includes(column)) overlap.push(`${table}.${column}`); + } + } + expect(overlap).toEqual([]); + }); +});