From d7ecd0f9df646dd540e2e8facf6440a05df68cfd Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:41 -0700 Subject: [PATCH 01/19] Add tests for secret redaction regressions Provider keys, PATs, free-text keyword+space secrets, key-named JSON fields, and stringified-object arguments all slip into error-sink and approval headlines unredacted. These tests pin the redaction behavior so the follow-up implementation lands against a failing test. --- packages/approvals/src/headline.test.ts | 91 +++++++++++++++++++++++++ packages/error-sink/src/redact.test.ts | 63 +++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 packages/approvals/src/headline.test.ts diff --git a/packages/approvals/src/headline.test.ts b/packages/approvals/src/headline.test.ts new file mode 100644 index 0000000000..85302a43ea --- /dev/null +++ b/packages/approvals/src/headline.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, test } from "bun:test"; +import { argumentsSummaryFor, headlineFor } from "./headline"; + +function writeFileDefinition(): { name: string; description: string } { + return { name: "write_file", description: "Write a file" }; +} + +describe("argumentsSummaryFor write_file preview", () => { + test("redacts a secret-like string in the content preview", () => { + const summary = argumentsSummaryFor(writeFileDefinition(), { + path: "/tmp/creds.json", + content: "api_key sk-abcdefgh12345678 value", + }); + expect(summary).toContain("[redacted]"); + expect(summary).not.toContain("sk-abcdefgh12345678"); + }); + + test("leaves non-secret content unchanged", () => { + const summary = argumentsSummaryFor(writeFileDefinition(), { + path: "/tmp/hello.txt", + content: "hello world", + }); + expect(summary).toBe('/tmp/hello.txt: "hello world"'); + }); +}); + +describe("generic tool argument rendering", () => { + function genericDefinition(): { name: string; description: string } { + return { name: "run_command", description: "Run a shell command" }; + } + + test("redacts a secret-like value in a generic tool's argument list", () => { + const summary = argumentsSummaryFor(genericDefinition(), { + command: "deploy", + target: "pk-live-ABCDEFGH12345678", + }); + expect(summary).toContain("target: [redacted]"); + expect(summary).not.toContain("ABCDEFGH12345678"); + }); + + test("redacts a secret inside a generic tool's title", () => { + const headline = headlineFor(genericDefinition(), { + title: "using pk-live-ABCDEFGH12345678 token", + }); + expect(headline).toContain("[redacted]"); + expect(headline).not.toContain("ABCDEFGH12345678"); + }); + + test("redacts a top-level password-key value with no secret prefix", () => { + const summary = argumentsSummaryFor(genericDefinition(), { + host: "localhost", + password: "supersecretvalue123", + }); + expect(summary).toContain("password: [redacted]"); + expect(summary).not.toContain("supersecretvalue123"); + }); + + test("redacts a free-form keyword+space secret in the title", () => { + const headline = headlineFor(genericDefinition(), { + title: "use password supersecret123 here", + }); + expect(headline).toContain("[redacted]"); + expect(headline).not.toContain("supersecret123"); + }); + + test("redacts a password-keyed secret nested inside a JSON value object", () => { + const summary = argumentsSummaryFor(genericDefinition(), { + config: { password: "hunter2-hunter2", host: "localhost" }, + }); + expect(summary).toContain('"password":"[redacted]"'); + expect(summary).not.toContain("hunter2-hunter2"); + }); + + test("redacts a password-keyed secret inside a JSON-stringified object value", () => { + const summary = argumentsSummaryFor(genericDefinition(), { + config: JSON.stringify({ password: "hunter2-hunter2", host: "localhost" }), + }); + expect(summary).toContain('"password":"[redacted]"'); + expect(summary).not.toContain("hunter2-hunter2"); + }); + + test("leaves a benign generic value unchanged", () => { + const summary = argumentsSummaryFor(genericDefinition(), { + host: "localhost", + port: 5432, + }); + expect(summary).toContain("host: localhost"); + expect(summary).toContain("port: 5432"); + expect(summary).not.toContain("[redacted]"); + }); +}); diff --git a/packages/error-sink/src/redact.test.ts b/packages/error-sink/src/redact.test.ts index 593acabf2f..70d0cb96ff 100644 --- a/packages/error-sink/src/redact.test.ts +++ b/packages/error-sink/src/redact.test.ts @@ -18,6 +18,50 @@ describe("redactText", () => { ); }); + test("redacts a segmented provider key with uppercase/mixed-case suffix", () => { + expect(redactText("using pk-live-ABCDEFGH12345678 to call provider")).toBe( + "using [redacted] to call provider", + ); + }); + + test("redacts an uppercase sk- provider key", () => { + expect(redactText("failed calling with sk-proj-ABCDEFGH12345678")).toBe( + "failed calling with [redacted]", + ); + }); + + test("redacts a github_pat_ personal access token", () => { + expect(redactText("auth failed github_pat_11abcdefghijklmnopGl000token")).toBe( + "auth failed [redacted]", + ); + }); + + test("redacts a segmented rk- provider key", () => { + expect(redactText("auth failed sending via rk-proj-ABCDEFGH12345678").toLowerCase()).toContain( + "[redacted]", + ); + }); + + test("does not redact benign hyphenated words and paths under a key-like prefix", () => { + // Over-redaction regression: the provider-key detector must not eat + // ordinary hyphenated words/paths that merely start with a key prefix. + expect(redactText("the build wrote pk-manifest.yaml to disk")).toBe( + "the build wrote pk-manifest.yaml to disk", + ); + expect(redactText("ran the sk-parallel-copy job")).toBe("ran the sk-parallel-copy job"); + expect(redactText("used pk-currency-converter to price it")).toBe( + "used pk-currency-converter to price it", + ); + expect(redactText("stalled on rk-pipeline-run-abcd")).toBe("stalled on rk-pipeline-run-abcd"); + expect(redactText("nudged the pk-abcdefgh record")).toBe("nudged the pk-abcdefgh record"); + }); + + test("does not redact a hyphenated GitHub-prefixed word (never a PAT)", () => { + expect(redactText("turned on ghp-garbage-collector nightly")).toBe( + "turned on ghp-garbage-collector nightly", + ); + }); + test("leaves ordinary text untouched", () => { expect(redactText("could not reach the hub")).toBe("could not reach the hub"); }); @@ -70,6 +114,25 @@ describe("redactText", () => { "auth failed token=[redacted]).authenticate() at line 4", ); }); + + test("redacts free-text keyword+space secret-shaped values", () => { + // These forms slip past the assignment/`=` paths; a bare keyword followed + // by a long, non-pure-English run is a leaked secret (e.g. an approval + // title like "use password supersecret123 here"). + expect(redactText("use password supersecret123 here")).toBe("use [redacted] here"); + expect(redactText("use api_key ABcdEf123456 here")).toBe("use [redacted] here"); + expect(redactText("use secret hunter2-hunter2 here")).toBe("use [redacted] here"); + expect(redactText("use token xyzQWErtyuiop9 here")).toBe("use [redacted] here"); + expect(redactText("use access_token Abcdefgh12345678 now")).toBe("use [redacted] now"); + }); + + test("does not redact free-text keyword+space ordinary prose", () => { + // The value here is a short word or a pure-English word -- not a secret. + expect(redactText("use secret sauce")).toBe("use secret sauce"); + expect(redactText("the token is here")).toBe("the token is here"); + expect(redactText("password recovery options")).toBe("password recovery options"); + expect(redactText("token ring protocol")).toBe("token ring protocol"); + }); }); describe("redactExtra", () => { From f38c61d0a51691a5b47ee094753817f05c00d9f5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:43 -0700 Subject: [PATCH 02/19] Redact provider keys and free-text secrets in error-sink Expand redactText's provider-key detection to the three distinct shapes (underscore PATs, segmented OpenAI-style keys, and bare sk-/pk-/rk- keys) without eating ordinary hyphenated words or paths. Add a keyword+space pattern so a bare secret-shaped run after password/secret/token/api_key in free text is masked. Export redactText and redactExtra from the package index so consumers can redact structural and free-text values. --- packages/error-sink/src/index.ts | 1 + packages/error-sink/src/redact.ts | 33 ++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/error-sink/src/index.ts b/packages/error-sink/src/index.ts index fe18fda760..19e62ec304 100644 --- a/packages/error-sink/src/index.ts +++ b/packages/error-sink/src/index.ts @@ -64,5 +64,6 @@ export function reportError(error: unknown, context: ErrorContextInput): string export type { ErrorContext } from "./context"; export { generateRefId } from "./ref-id"; +export { redactExtra, redactText } from "./redact"; export { ErrorEnvelopeShape, makeErrorEnvelope, parseErrorEnvelope } from "./error-envelope"; export type { ErrorEnvelope } from "./error-envelope"; diff --git a/packages/error-sink/src/redact.ts b/packages/error-sink/src/redact.ts index b486ccd7fc..6651aaa4b2 100644 --- a/packages/error-sink/src/redact.ts +++ b/packages/error-sink/src/redact.ts @@ -7,13 +7,40 @@ const SECRET_KEY_PATTERN = const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ /bearer\s+\S+/gi, /authorization\s*:\s*\S+/gi, - // Provider key prefixes use either a hyphen (OpenAI's `sk-...`) or an - // underscore (GitHub's `ghp_...`) -- both must match. - /\b(sk|pk|rk|ghp|gho|ghu|ghs)[-_][a-z0-9]{8,}\b/gi, + // Provider key prefixes come in three shapes, matched by how distinctive the + // separator + value run is so ordinary hyphenated words/paths are NOT eaten + // (e.g. `pk-manifest.yaml`, `sk-parallel-copy` must stay intact): + // 1. GitHub-family PATs are `_`-separated (`ghp_...`, `github_pat_11...`). + // Their short prefixes are not English words, so an 8+ base64-ish value + // after the `_` is enough. A hyphen (`ghp-garbage-collector`) is never a + // PAT, so `[-_]` here is deliberately just `_`. + /\b(?:github_pat|ghp|gho|ghu|ghs)_[a-z0-9_]{8,}\b/gi, + // 2. Segmented OpenAI-style keys (`sk-proj-...`, `pk-live-...`) put a known + // short sub-label (`live`/`proj`/`test`/...) right after the hyphen, and + // only then is the hyphen meaningful for masking. + /\b(?:sk|pk|rk)-(?:live|proj|test|dev|prod|org|key|secret)\b[_-][a-z0-9_]{4,}\b/gi, + // 3. Bare OpenAI-style keys (`sk-abcdefghijklmnop`) need a hyphen-free value + // run long enough that it cannot be an ordinary word (`sk-manifest` and + // `pk-abcdefgh` are 8 chars and MUST NOT redact; real keys are `sk-` + + // a long base64 run). `(?![-_])` stops a segment boundary from ending the + // match partway through a hyphenated word. + /\b(?:sk|pk|rk)-[a-z0-9_]{9,}(?![-_])\b/gi, // A raw JWT (header.payload.signature) carries no keyword prefix at all, // but its base64url header always starts with the literal `eyJ` (base64 // of `{"`), which is distinctive enough to key off heuristically. /\beyJ[\w-]{10,}\.[\w-]{10,}\.[\w-]{10,}\b/g, + // Free-text `keyword + space + value` forms (`use password supersecret123`, + // `api_key ABcdEf123456`, `secret hunter2-hunter2`) — the approval `title` + // is free-form caller input so a bare keyword followed by a secret-shaped + // run slips through the assignment/`=` paths. Two guards keep ordinary prose + // intact: the value must run at least 8 `[a-z0-9_-]` chars (spares short + // words like `sauce`/`ring`/`is`), and it must contain a digit, hyphen, or + // underscore — the one case-insensitive-safe distinguishing signal (pure + // `[A-Z0-9_-]` character classes are useless under `/i`, so `recovery` / a + // pure-English word would otherwise be eaten). The value charset excludes + // spaces/punctuation so the match stops at the end of the secret rather than + // swallowing the rest of the sentence. + /\b(?:password|passwd|secret|token|access_token|api[_-]?key)\s+(?=[a-z0-9_-]{8,}\b)[a-z0-9_-]*[\d_-][a-z0-9_-]*\b/gi, ]; // A value's character set once past `name=`: covers hex/base64/JWT-shaped From 7cbe7496879c8d941a80ee86c8eb5c8b9361b3aa Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:45 -0700 Subject: [PATCH 03/19] Redact secrets in approval headline sibling render paths The approval title is free-form caller input and the write_file content preview / generic argument bag can carry credentials; route them through error-sink's redactText and redactExtra so key-named and string-embedded secrets are masked from any headline a human sees. Adds the @corbits/error-sink workspace dependency approvals now imports. --- bun.lock | 11 +++------ packages/approvals/package.json | 3 +++ packages/approvals/src/headline.ts | 36 +++++++++++++++++++++++------- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/bun.lock b/bun.lock index 2d3f0dde7b..95e9d81357 100644 --- a/bun.lock +++ b/bun.lock @@ -155,6 +155,9 @@ "packages/approvals": { "name": "@corbits/approvals", "version": "0.0.1", + "dependencies": { + "@corbits/error-sink": "workspace:*", + }, "devDependencies": { "@types/bun": "catalog:", "typescript": "catalog:", @@ -1775,16 +1778,10 @@ "@corbits/mailbox/hono-openapi": ["hono-openapi@1.3.1", "", { "peerDependencies": { "@hono/standard-validator": "^0.2.0", "@standard-community/standard-json": "^0.3.5", "@standard-community/standard-openapi": "^0.2.9", "@types/json-schema": "^7.0.15", "hono": "^4.11.2", "openapi-types": "^12.1.3" }, "optionalPeers": ["@hono/standard-validator", "hono"] }, "sha512-NLVeVkhKZ3drmQNEIPac8HX8Y54uf1hJAgIM/7MfDsaeVVmB+QILWQxx5x3R3NvRHgedcbEbOCGY2uR7WQYyMw=="], - "@corbits/worker/@corbits/artifacts": ["@corbits/artifacts@github:corbitsdev/corbits-artifacts#4a2a3c2", { "dependencies": { "@hono/standard-validator": "^0.2.3" }, "peerDependencies": { "@intx/agent": "^0.3.0", "@intx/hub-api": "^0.3.0", "@intx/types": "^0.3.0", "arktype": "^2.1.29", "drizzle-orm": "^0.45.2", "hono": "^4.12.32", "hono-openapi": "^1.2.0", "postgres": "^3.4.9" } }, "corbitsdev-corbits-artifacts-4a2a3c2", "sha512-UY5nkQ31kGmYgqgIOSlbigwvm7c/s2kpqEbF6Oqo0rzoS1RejpPp+w/Yim6h2MRpYCl5CXgAET1MhkDboHuWBA=="], - "@esbuild-kit/core-utils/esbuild": ["esbuild@0.18.20", "", { "optionalDependencies": { "@esbuild/android-arm": "0.18.20", "@esbuild/android-arm64": "0.18.20", "@esbuild/android-x64": "0.18.20", "@esbuild/darwin-arm64": "0.18.20", "@esbuild/darwin-x64": "0.18.20", "@esbuild/freebsd-arm64": "0.18.20", "@esbuild/freebsd-x64": "0.18.20", "@esbuild/linux-arm": "0.18.20", "@esbuild/linux-arm64": "0.18.20", "@esbuild/linux-ia32": "0.18.20", "@esbuild/linux-loong64": "0.18.20", "@esbuild/linux-mips64el": "0.18.20", "@esbuild/linux-ppc64": "0.18.20", "@esbuild/linux-riscv64": "0.18.20", "@esbuild/linux-s390x": "0.18.20", "@esbuild/linux-x64": "0.18.20", "@esbuild/netbsd-x64": "0.18.20", "@esbuild/openbsd-x64": "0.18.20", "@esbuild/sunos-x64": "0.18.20", "@esbuild/win32-arm64": "0.18.20", "@esbuild/win32-ia32": "0.18.20", "@esbuild/win32-x64": "0.18.20" }, "bin": { "esbuild": "bin/esbuild" } }, "sha512-ceqxoedUrcayh7Y7ZX6NdbbDzGROiyVBgC4PriJThBKSVPWnnFHZAkfI1lJT8QFkOwH4qOS2SJkS4wvpGl8BpA=="], "@npmcli/agent/lru-cache": ["lru-cache@11.5.3", "", {}, "sha512-U4N8FgzmWxc8k1VH8Kr6lQg18U7Fjvby6wXHVRX/ZZ7IwWbRMgrRbP0Wrb5q5NVinryp4SQampHKdvtecItxUg=="], - "@workbench/hub/@corbits/artifacts": ["@corbits/artifacts@github:corbitsdev/corbits-artifacts#4a2a3c2", { "dependencies": { "@hono/standard-validator": "^0.2.3" }, "peerDependencies": { "@intx/agent": "^0.3.0", "@intx/hub-api": "^0.3.0", "@intx/types": "^0.3.0", "arktype": "^2.1.29", "drizzle-orm": "^0.45.2", "hono": "^4.12.32", "hono-openapi": "^1.2.0", "postgres": "^3.4.9" } }, "corbitsdev-corbits-artifacts-4a2a3c2", "sha512-UY5nkQ31kGmYgqgIOSlbigwvm7c/s2kpqEbF6Oqo0rzoS1RejpPp+w/Yim6h2MRpYCl5CXgAET1MhkDboHuWBA=="], - - "@workbench/hub/@corbits/mailbox": ["@corbits/mailbox@github:corbitsdev/corbits-mailbox#65590a8", { "dependencies": { "@hono/standard-validator": "0.2.3", "@intx/crypto": "0.3.0", "@standard-community/standard-json": "0.3.5", "@standard-community/standard-openapi": "0.2.9", "arktype": "2.1.29", "hono-openapi": "1.3.1" }, "peerDependencies": { "@intx/log": "^0.2.2", "drizzle-orm": "^0.45.2", "hono": "^4.12.0", "postgres": "^3.4.0" } }, "corbitsdev-corbits-mailbox-65590a8", "sha512-tK7xXcL780iX3I+5R68gmyqgEA0uwYO1qdTDNn/LF6tOiFIm0lt2szjspJ8JMKMTzKC6SSai03p723rEGM1jOg=="], - "better-call/@better-auth/utils": ["@better-auth/utils@0.5.0", "", { "dependencies": { "@noble/hashes": "^2.0.1" } }, "sha512-BL8W4EfIZFwlu0r54m3v1ztjDhu6dDe/amLTm0xybmbZaNgYUqhD3SjpAsnq0q8YD6/ki4iwIgxJNLP/N3TxiA=="], "body-parser/content-type": ["content-type@2.1.0", "", {}, "sha512-mj7UPXE0jaqaOsukNZRUEfEi2AcL7C/vwmwcHV0O97eO1E1pxBZuyjlZrx5seTaNBg1U6+o35wpa35Qfcc+7ag=="], @@ -1879,8 +1876,6 @@ "@esbuild-kit/core-utils/esbuild/@esbuild/win32-x64": ["@esbuild/win32-x64@0.18.20", "", { "os": "win32", "cpu": "x64" }, "sha512-kTdfRcSiDfQca/y9QIkng02avJ+NCaQvrMejlsB3RRv5sE9rRoeBPISaZpKxHELzRxZyLvNts1P27W3wV+8geQ=="], - "@workbench/hub/@corbits/mailbox/hono-openapi": ["hono-openapi@1.3.1", "", { "peerDependencies": { "@hono/standard-validator": "^0.2.0", "@standard-community/standard-json": "^0.3.5", "@standard-community/standard-openapi": "^0.2.9", "@types/json-schema": "^7.0.15", "hono": "^4.11.2", "openapi-types": "^12.1.3" }, "optionalPeers": ["@hono/standard-validator", "hono"] }, "sha512-NLVeVkhKZ3drmQNEIPac8HX8Y54uf1hJAgIM/7MfDsaeVVmB+QILWQxx5x3R3NvRHgedcbEbOCGY2uR7WQYyMw=="], - "cross-spawn/which/isexe": ["isexe@2.0.0", "", {}, "sha512-RHxMLp9lnKHGHRng9QFhRCMbYAcVpn69smSGcq3f36xjgVVWThj4qqLbTLlq7Ssj8B+fIQ1EuCEGI2lKsyQeIw=="], "minipass-flush/minipass/yallist": ["yallist@4.0.0", "", {}, "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A=="], diff --git a/packages/approvals/package.json b/packages/approvals/package.json index 3cdf2e2875..2263499674 100644 --- a/packages/approvals/package.json +++ b/packages/approvals/package.json @@ -13,6 +13,9 @@ "typecheck": "tsc --noEmit", "test": "bun test" }, + "dependencies": { + "@corbits/error-sink": "workspace:*" + }, "devDependencies": { "@types/bun": "catalog:", "typescript": "catalog:" diff --git a/packages/approvals/src/headline.ts b/packages/approvals/src/headline.ts index baf789ff3e..b9aba62502 100644 --- a/packages/approvals/src/headline.ts +++ b/packages/approvals/src/headline.ts @@ -1,6 +1,7 @@ // The human-readable line an approval is described by. Pure string work, no // database dependency, so the browser can compose the same headline the // hub-side tools do. +import { redactExtra, redactText } from "@corbits/error-sink"; function stringField(source: object, field: string): string | undefined { if (!(field in source)) return undefined; @@ -71,7 +72,9 @@ export function headlineFor(toolDefinition: unknown, toolArguments: unknown): st typeof toolArguments === "object" && toolArguments !== null ? stringField(toolArguments, "title") : undefined; - return title === undefined ? headline : `${headline}: "${title}"`; + // The title is a generic free-form field a caller can stuff a secret into; + // redact it the same way write_file's content preview is. + return title === undefined ? headline : `${headline}: "${redactText(title)}"`; } /** The bare tool name off a toolDefinition snapshot, for a row that wants to @@ -89,14 +92,25 @@ function truncate(text: string, limit = ARGUMENT_PREVIEW_LIMIT): string { } function formatArgumentValue(value: unknown): string { - if (typeof value === "string") return value; if (typeof value === "number" || typeof value === "boolean") return String(value); if (value === null || value === undefined) return ""; - try { - return JSON.stringify(value); - } catch { - return String(value); + // Default: redact a string (or a stringified object/array) structurally when + // it parses as JSON, so key-named secrets (`password`, `token`, ...) and + // nested JSON are caught; fall back to plain redactText for non-JSON strings. + if (typeof value === "string") { + try { + const parsed: unknown = JSON.parse(value); + if (parsed !== null && typeof parsed === "object") { + return JSON.stringify(redactExtra(parsed as Record)); + } + } catch { + // not JSON; redact as a plain string below + } + return redactText(value); } + // A live object/array argument: redact it structurally, then serialize. + const redacted = redactExtra(value as Record); + return JSON.stringify(redacted); } // `write_file`'s call is read as its target path plus a preview of what it @@ -106,7 +120,7 @@ function writeFileArgumentsSummary(toolArguments: object): string | undefined { const path = stringField(toolArguments, "path"); if (path === undefined) return undefined; const content = (toolArguments as Record).content; - const preview = typeof content === "string" ? `: "${truncate(content)}"` : ""; + const preview = typeof content === "string" ? `: "${truncate(redactText(content))}"` : ""; return `${path}${preview}`; } @@ -117,7 +131,13 @@ const ARGUMENT_SUMMARY_RENDERERS: Readonly< }; function genericArgumentsSummary(toolArguments: object): string | undefined { - const entries = Object.entries(toolArguments as Record); + // Redact the whole bag structurally first: this collapses key-named secrets + // (`password`, `token`, `apiKey`, ...) regardless of their value's shape and + // recurses into nested objects/strings, which the per-value format path + // below then renders (already-safe values pass through unchanged). + const redacted = redactExtra(toolArguments as Record); + if (redacted === undefined) return undefined; + const entries = Object.entries(redacted); if (entries.length === 0) return undefined; return truncate( entries.map(([key, value]) => `${key}: ${formatArgumentValue(value)}`).join(", "), From 6cf2b08117a1c2fad611d488332c129f036817da Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:46 -0700 Subject: [PATCH 04/19] Pin worker definition shape against CL-9712 regression The single-step, mail-triggered worker shape is deployment contract: a second step trades away the warm agent's durable memory and an awaitSignal gate parks the run forever. Pin buildWorkerWorkflow's output so the deploy path cannot reintroduce the broken shape. --- packages/worker/src/definition-json.test.ts | 70 +++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 packages/worker/src/definition-json.test.ts diff --git a/packages/worker/src/definition-json.test.ts b/packages/worker/src/definition-json.test.ts new file mode 100644 index 0000000000..853acba171 --- /dev/null +++ b/packages/worker/src/definition-json.test.ts @@ -0,0 +1,70 @@ +// The worker definition's single-step, mail-triggered shape is contract, not +// style: a second step silently trades away the warm agent's durable memory, +// and an `awaitSignal` gate parks the run forever on a signal nothing raises. +// CL-9712 fixed a deployment that shipped exactly that broken shape +// (`kickoff: awaitSignal({ name: "worker-kickoff" })` then the worker step +// gated on `after: ["kickoff"]`); these tests pin the deployed entry +// `buildWorkerWorkflow` so the deploy path cannot reintroduce it. +import { describe, expect, test } from "bun:test"; +import { type } from "arktype"; + +import { buildWorkerWorkflow } from "./index"; +import { WORKER_STEP_ID, WORKER_WORKFLOW_ID } from "./workflow-ids"; + +// The definition's shape at the trust boundary: every field the regression +// guard (and the harness that ships it) actually reads. Unknown fields are +// allowed but this schema is the source of truth for the asserted ones. +const WorkerDefinitionShape = type({ + id: "string", + triggers: [{ type: "'mail'", to: "string" }], + stepOrder: "string[]", + steps: { + worker: { + kind: "'step'", + id: "'worker'", + triggers: "'unbounded'", + "input?": { from: "'trigger.payload'" }, + }, + }, +}); + +type WorkerJson = typeof WorkerDefinitionShape.infer; + +function workerJson(triggerAddress = "worker@acme.test"): WorkerJson { + const definition = buildWorkerWorkflow({ + workflowId: WORKER_WORKFLOW_ID, + triggerAddress, + inferencePreferences: [{ provider: "noop", model: "noop" }], + systemPrompt: "You are the worker.", + hubCredentialId: "cred_1", + mcpServers: [], + }); + const parsed = WorkerDefinitionShape(definition); + if (parsed instanceof type.errors) { + throw new Error(`definition does not match the worker shape: ${parsed.message}`); + } + return parsed; +} + +describe("worker definition shape (CL-9712)", () => { + test("is triggered by mail to the worker address", () => { + expect(workerJson("worker@acme.test").triggers).toEqual([ + { type: "mail", to: "worker@acme.test" }, + ]); + }); + + test("carries exactly the worker step and no kickoff gate", () => { + const def = workerJson(); + expect(def.stepOrder).toEqual([WORKER_STEP_ID]); + expect(Object.keys(def.steps)).toEqual([WORKER_STEP_ID]); + expect(def.steps[WORKER_STEP_ID]).toMatchObject({ kind: "step", id: WORKER_STEP_ID }); + }); + + test("runs the worker step unbounded on the triggering mail's payload", () => { + const worker = workerJson().steps[WORKER_STEP_ID]; + expect(worker).toBeDefined(); + if (worker === undefined) throw new Error(`missing ${WORKER_STEP_ID} step`); + expect(worker.triggers).toBe("unbounded"); + expect(worker.input).toEqual({ from: "trigger.payload" }); + }); +}); From 646fd646de5f23c975b4cdcba1263f9e953cdc7a Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:48 -0700 Subject: [PATCH 05/19] Require 204 No Content on void settings API calls DELETE/role-assign/inference routes return nothing, so reading and schema-validating a body was unsound . Add a dedicated requestVoid helper (in api-request, tenancy-api, and inference api) that fails loudly if a void route answers with any 2xx other than 204, keeping the 204 contract in force instead of silently accepting a drifted body. Also report shadow-offering rollback failures through error-sink instead of swallowing them. --- apps/web/src/settings/api-request.ts | 36 ++++++++++++- apps/web/src/settings/credentials-api.ts | 6 +-- apps/web/src/settings/inference/api.ts | 14 ++++- apps/web/src/settings/tenancy-api.ts | 65 ++++++++++++++++-------- 4 files changed, 94 insertions(+), 27 deletions(-) diff --git a/apps/web/src/settings/api-request.ts b/apps/web/src/settings/api-request.ts index 3c50d667c6..c3e77948a0 100644 --- a/apps/web/src/settings/api-request.ts +++ b/apps/web/src/settings/api-request.ts @@ -42,7 +42,6 @@ export async function apiRequest( const body: unknown = await response.json().catch(() => undefined); throw new ErrorCtor(readErrorEnvelope(response.status, body, verb), response.status); } - if (response.status === 204) return undefined as T; const body: unknown = await response.json().catch(() => undefined); const parsed = schema(body); if (parsed instanceof type.errors) { @@ -50,3 +49,38 @@ export async function apiRequest( } return parsed; } + +/** For a route whose success response is `204 No Content` — nothing to + * validate against a schema, so this is the one place a call is allowed + * to end without a parsed body, rather than every `request` caller + * having to accept an unsound `undefined as T`. */ +export async function requestVoid( + path: string, + verb: string, + ErrorCtor: ApiErrorCtor, + init?: RequestInit, +): Promise { + let response: Response; + try { + response = await fetch(path, { + ...init, + headers: { "content-type": "application/json", ...init?.headers }, + }); + } catch (cause) { + throw new ErrorCtor(cause instanceof Error ? cause.message : String(cause)); + } + if (!response.ok) { + const body: unknown = await response.json().catch(() => undefined); + throw new ErrorCtor(readErrorEnvelope(response.status, body, verb), response.status); + } + // A void route answers `204 No Content` by contract. Any other 2xx (e.g. + // a 200/201-with-body a route drifted into returning where 204 is expected) + // is a contract mismatch and must fail loudly rather than be silently + // accepted as an empty success. + if (response.status !== 204) { + throw new ErrorCtor( + `Unexpected response while ${verb}: expected 204 No Content, got ${response.status}`, + response.status, + ); + } +} diff --git a/apps/web/src/settings/credentials-api.ts b/apps/web/src/settings/credentials-api.ts index 8915f854b9..326d2added 100644 --- a/apps/web/src/settings/credentials-api.ts +++ b/apps/web/src/settings/credentials-api.ts @@ -10,7 +10,7 @@ import { } from "@intx/types"; export { credentialTypes } from "@intx/types"; -import { apiRequest, type Validator } from "./api-request"; +import { apiRequest, requestVoid, type Validator } from "./api-request"; export type Credential = typeof CredentialResponse.infer; export type Provider = typeof ProviderResponse.infer; @@ -80,10 +80,10 @@ export function createCredential( } export function deleteCredential(tenantId: string, credentialId: string): Promise { - return request( + return requestVoid( `/api/tenants/${tenantId}/credentials/${credentialId}`, - (data) => data as void, "revoking that credential", + CredentialsApiError, { method: "DELETE" }, ); } diff --git a/apps/web/src/settings/inference/api.ts b/apps/web/src/settings/inference/api.ts index 8a76fcf892..534219098e 100644 --- a/apps/web/src/settings/inference/api.ts +++ b/apps/web/src/settings/inference/api.ts @@ -30,6 +30,7 @@ // reference it — never the ancestor's credential. import { type } from "arktype"; +import { reportError } from "@corbits/error-sink"; import { CreateCredential, CreateModel, @@ -146,6 +147,15 @@ async function requestVoid( response.status, ); } + // A void route answers `204 No Content` by contract. Any other 2xx (e.g. a + // 200/201-with-body the route drifted into returning where 204 is expected) + // is a contract mismatch and must fail loudly. + if (response.status !== 204) { + throw new InferenceSettingsApiError( + `The server answered ${response.status} while ${verb}; expected 204 No Content.`, + response.status, + ); + } } /** The tenant's resolved catalog — every model visible after inheritance, @@ -365,7 +375,9 @@ export async function shadowOffering( "rolling back the just-minted provider", { method: "DELETE" }, fetchImpl, - ).catch(() => undefined); + ).catch((rollbackCause: unknown) => { + reportError(rollbackCause, { operation: "shadowOffering.rollback", tenantId }); + }); } throw cause; } diff --git a/apps/web/src/settings/tenancy-api.ts b/apps/web/src/settings/tenancy-api.ts index 7122f6d93a..a0920fb3d7 100644 --- a/apps/web/src/settings/tenancy-api.ts +++ b/apps/web/src/settings/tenancy-api.ts @@ -62,7 +62,6 @@ async function request(path: string, schema: Validator, init?: RequestInit if (!response.ok) { throw new TenancyApiError(`The hub answered ${response.status} for ${path}.`, response.status); } - if (response.status === 204) return undefined as T; const body: unknown = await response.json().catch(() => undefined); const parsed = schema(body); if (parsed instanceof type.errors) { @@ -71,6 +70,40 @@ async function request(path: string, schema: Validator, init?: RequestInit return parsed; } +/** For a route whose success response is `204 No Content` — nothing to + * validate against a schema, so this is the one place a call is allowed + * to end without a parsed body, rather than every `request` caller + * having to accept an unsound `undefined as T`. */ +async function requestVoid(path: string, init?: RequestInit): Promise { + let response: Response; + try { + response = await fetch(path, { + ...init, + headers: { "content-type": "application/json", ...init?.headers }, + }); + } catch (cause) { + throw new TenancyApiError(cause instanceof Error ? cause.message : String(cause)); + } + if (response.status === 401) { + throw new UnauthenticatedError(); + } + if (response.status === 403) { + throw new TenancyApiError(`Not permitted to view ${path}.`, 403); + } + if (!response.ok) { + throw new TenancyApiError(`The hub answered ${response.status} for ${path}.`, response.status); + } + // A void route answers `204 No Content` by contract. Any other 2xx (e.g. a + // 200/201-with-body the route drifted into returning where 204 is expected) + // is a contract mismatch and must fail loudly. + if (response.status !== 204) { + throw new TenancyApiError( + `The hub answered ${response.status} for ${path}; expected 204 No Content.`, + response.status, + ); + } +} + // -- Principals -------------------------------------------------------- export function listPrincipals(tenantId: string): Promise { @@ -89,11 +122,7 @@ export function updatePrincipalStatus( } export function removePrincipal(tenantId: string, principalId: string): Promise { - return request( - `/api/tenants/${tenantId}/principals/${principalId}`, - (data) => data as void, - { method: "DELETE" }, - ); + return requestVoid(`/api/tenants/${tenantId}/principals/${principalId}`, { method: "DELETE" }); } export type InviteMemberInput = typeof InviteMember.infer; @@ -135,25 +164,19 @@ export function renameRole( } export function deleteRole(tenantId: string, roleId: string): Promise { - return request(`/api/tenants/${tenantId}/roles/${roleId}`, (data) => data as void, { - method: "DELETE", - }); + return requestVoid(`/api/tenants/${tenantId}/roles/${roleId}`, { method: "DELETE" }); } export function assignRole(tenantId: string, principalId: string, roleId: string): Promise { - return request( - `/api/tenants/${tenantId}/principals/${principalId}/roles/${roleId}`, - (data) => data as void, - { method: "POST" }, - ); + return requestVoid(`/api/tenants/${tenantId}/principals/${principalId}/roles/${roleId}`, { + method: "POST", + }); } export function unassignRole(tenantId: string, principalId: string, roleId: string): Promise { - return request( - `/api/tenants/${tenantId}/principals/${principalId}/roles/${roleId}`, - (data) => data as void, - { method: "DELETE" }, - ); + return requestVoid(`/api/tenants/${tenantId}/principals/${principalId}/roles/${roleId}`, { + method: "DELETE", + }); } // -- Grants ---------------------------------------------------------------- @@ -213,9 +236,7 @@ export function updateGrant( } export function revokeGrant(tenantId: string, grantId: string): Promise { - return request(`/api/tenants/${tenantId}/grants/${grantId}`, (data) => data as void, { - method: "DELETE", - }); + return requestVoid(`/api/tenants/${tenantId}/grants/${grantId}`, { method: "DELETE" }); } // The one grant-checked route that itself requires no grant, so it From 9d62cc2502cb83c502a8eaefb8ed9a6456f928d8 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:51 -0700 Subject: [PATCH 06/19] Fail closed when the e2e database does not resolve to an _e2e sibling DB-gated suites must only ever run against the derived _e2e sibling database, never a developer's own DATABASE_URL. dbGate now redirects the resolved URL onto the _e2e database (propagating it back onto process.env.DATABASE_URL so bootHub and direct reads hit the same DB) and refuses to proceed if that redirect ever lands on a non-_e2e name. baseUrlToE2eUrl becomes idempotent so a prior redirect is not stacked. --- e2e/lib/database-url.ts | 17 ++++++++++---- e2e/lib/db-gate.ts | 34 +++++++++++++++++++++++++--- e2e/lib/tenant-cleanup.ts | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 e2e/lib/tenant-cleanup.ts diff --git a/e2e/lib/database-url.ts b/e2e/lib/database-url.ts index ea3b656b13..250df14bbd 100644 --- a/e2e/lib/database-url.ts +++ b/e2e/lib/database-url.ts @@ -10,7 +10,6 @@ // handful of suites that do spawn the hub/sidecar keep one import. import { readFileSync } from "node:fs"; import path from "node:path"; -import { assertDatabaseConfigured } from "./db-gate.ts"; export const REPO_ROOT = path.resolve(import.meta.dir, "..", ".."); @@ -27,13 +26,20 @@ export const REPO_ROOT = path.resolve(import.meta.dir, "..", ".."); * even when the shell sourced `.env`. Fall back to the same repo-root * `.env` file `bun run` would load. */ + +/** + * The resolved DATABASE_URL redirected to the `_e2e` sibling, or undefined + * when no DATABASE_URL is configured. This is the one place a DB-gated suite + * learns the database it actually runs against — never the developer's own + * `DATABASE_URL`, always its `_e2e` sibling. Skip/hard-fail semantics live in + * `dbGate`; this resolver only answers "what DB would this suite own?". + */ export function e2eDatabaseUrl(): string | undefined { const fromProcess = process.env["DATABASE_URL"]; const url = fromProcess !== undefined && fromProcess !== "" ? fromProcess : databaseUrlFromRepoEnvFile(); - if (url !== undefined && url !== "") return baseUrlToE2eUrl(url); - assertDatabaseConfigured(undefined, "walking-skeleton suite"); - return undefined; + if (url === undefined || url === "") return undefined; + return baseUrlToE2eUrl(url); } /** @@ -91,6 +97,9 @@ export function baseUrlToE2eUrl(databaseUrl: string): string { "Expected e.g. postgres://localhost:5432/workbench.", ); } + // Idempotent: if the url already targets a `_e2e` sibling (a prior + // redirect), leave it alone rather than stacking `_e2e_e2e`. + if (database.endsWith("_e2e")) return url.toString(); url.pathname = `/${database}_e2e`; return url.toString(); } diff --git a/e2e/lib/db-gate.ts b/e2e/lib/db-gate.ts index 117a58397f..350487b3c8 100644 --- a/e2e/lib/db-gate.ts +++ b/e2e/lib/db-gate.ts @@ -8,6 +8,8 @@ // provision Postgres) — throws instead of skipping. import { afterAll, describe } from "bun:test"; +import { e2eDatabaseUrl } from "./database-url"; + export const TEST_COMPOSE_FILE = "compose.test.yml"; export const TEST_COMPOSE_UP = `docker compose -f ${TEST_COMPOSE_FILE} up -d`; export const TEST_DATABASE_URL = "postgres://postgres:postgres@localhost:5432/workbench"; @@ -72,11 +74,37 @@ function printSummary(): void { } /** - * databaseUrl: the resolved DATABASE_URL (or "" / undefined when absent). - * label: identifies the skipped suite in the summary — pass import.meta.path. + * databaseUrl: the raw configured DATABASE_URL (or "" / undefined when + * absent). label: identifies the skipped suite in the summary — pass + * import.meta.path. + * + * Every DB-gated suite runs against the `_e2e` sibling database, never the + * developer's own `DATABASE_URL`: when a database is configured, dbGate + * redirects the resolved url (via e2eDatabaseUrl) onto the derived `_e2e` + * database and propagates it onto process.env.DATABASE_URL so bootHub and + * every direct-DB read hit the sibling the suite owns outright. When + * DATABASE_URL is unset the suite skips (hard failure in CI that provisions + * Postgres), exactly as before. */ export function dbGate(databaseUrl: string | undefined, label: string): typeof describe { - if (databaseUrl !== undefined && databaseUrl !== "") return describe; + if (databaseUrl !== undefined && databaseUrl !== "") { + const e2eUrl = e2eDatabaseUrl(); + if (e2eUrl !== undefined) { + // Fail closed: a DB-gated suite must only ever run against the derived + // `_e2e` sibling, never the developer's own database. If the resolver + // ever regresses into returning a non-`_e2e` URL, refuse to redirect + // rather than silently run e2e against the real DB. + const e2eDatabase = new URL(e2eUrl).pathname.replace(/^\//, ""); + if (!e2eDatabase.endsWith("_e2e")) { + throw new Error( + `Resolved e2e database does not target a _e2e sibling: ${e2eUrl}. ` + + "Refusing to run a DB-gated suite against a non-_e2e database.", + ); + } + process.env["DATABASE_URL"] = e2eUrl; + } + return describe; + } if (databaseIsRequired()) throw missingDatabaseError(label); skipped.push(label); afterAll(printSummary); diff --git a/e2e/lib/tenant-cleanup.ts b/e2e/lib/tenant-cleanup.ts new file mode 100644 index 0000000000..82dcf77980 --- /dev/null +++ b/e2e/lib/tenant-cleanup.ts @@ -0,0 +1,47 @@ +// Shared teardown for the e2e seeding suites that create tenants/principals/ +// roles over stock HTTP routes and never delete them. Reruns of those suites +// used to accumulate rows in the shared `_e2e` database, so every seeder +// suite now registers one `afterAll` that removes exactly the tenants it +// created. Deleting a tenant cascades to the vast majority of its rows +// (principals, roles, grants, providers, credentials, catalog rows, workflow +// definitions/assets/runs, sessions, approvals, oauth clients, ...), but a +// few tables reference tenant with `onDelete: "restrict"` (sidecar_allocation, +// workflow_probe) or reference tenant with NO action (git_token), so those are +// deleted first while the tenant row is still present. +import { inArray } from "drizzle-orm"; + +import { createDB } from "@intx/db"; +import { gitToken, sidecarAllocation, tenant as tenantTable, workflowProbe } from "@intx/db/schema"; + +import { e2eDatabaseUrl } from "./database-url"; + +/** + * Deletes the tenants named by `tenantIds` together with every row that + * references them. Idempotent per id: filtering on `inArray` makes deleting + * already-absent ids a no-op, so a suite can pass ids of tenants that may have + * failed to be created and still tear down cleanly. + */ +export async function cleanupTenants(tenantIds: readonly string[]): Promise { + if (tenantIds.length === 0) return; + const databaseUrl = e2eDatabaseUrl(); + if (databaseUrl === undefined) return; + const parsed = new URL(databaseUrl); + const handle = createDB({ + host: parsed.hostname, + port: parsed.port === "" ? 5432 : Number(parsed.port), + user: decodeURIComponent(parsed.username), + password: decodeURIComponent(parsed.password), + database: parsed.pathname.replace(/^\//, ""), + }); + try { + const { db } = handle; + // No-action-on-tenant rows and restrict-on-tenant rows must go before the + // tenant row itself. + await db.delete(gitToken).where(inArray(gitToken.tenantId, tenantIds)); + await db.delete(sidecarAllocation).where(inArray(sidecarAllocation.tenantId, tenantIds)); + await db.delete(workflowProbe).where(inArray(workflowProbe.tenantId, tenantIds)); + await db.delete(tenantTable).where(inArray(tenantTable.id, tenantIds)); + } finally { + await handle.close(); + } +} From e25a8fe57b9eaf272ba78ca55528c47ed01fff70 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:53 -0700 Subject: [PATCH 07/19] Tear down seeded tenants and read the _e2e database in e2e suites Suites that create tenants/principals/roles over stock HTTP routes now register an afterAll that deletes exactly the tenants they made, so reruns stop accumulating rows in the shared _e2e database. Suites that open the database directly resolve it through e2eDatabaseUrl instead of the raw DATABASE_URL, and the browser suites assert the second form field exists before typing into it instead of silently skipping. --- e2e/grants/grants-not-walkable.test.ts | 9 ++++++++- e2e/grants/roles-assign-evaluate.test.ts | 7 ++++++- e2e/hub/mailbox-persist.test.ts | 3 ++- e2e/hub/mailbox-send-terminal.test.ts | 3 ++- e2e/install/agent-redeploy.test.ts | 4 ++++ e2e/install/install-package.test.ts | 4 ++++ e2e/workflows/agent-session.test.ts | 3 ++- e2e/workflows/first-reply-session.test.ts | 3 ++- 8 files changed, 30 insertions(+), 6 deletions(-) diff --git a/e2e/grants/grants-not-walkable.test.ts b/e2e/grants/grants-not-walkable.test.ts index eb5f33daf5..a32e68a94c 100644 --- a/e2e/grants/grants-not-walkable.test.ts +++ b/e2e/grants/grants-not-walkable.test.ts @@ -2,10 +2,11 @@ // from an ancestor tenant, grants never do. Booting the hub runs package // migrations, so the suite skips without a reachable DATABASE_URL. -import { expect, test } from "bun:test"; +import { afterAll, expect, test } from "bun:test"; import { type } from "arktype"; import { dbGate } from "../lib/db-gate"; import { bootHub } from "../lib/hub"; +import { cleanupTenants } from "../lib/tenant-cleanup"; const describeIfDb = dbGate(process.env["DATABASE_URL"] ?? "", import.meta.path); @@ -15,6 +16,10 @@ const Evaluation = type({ effect: "string", matchingGrants: type({ id: "string" describeIfDb("grants are explicit per tenant", () => { const booted = bootHub(); + const seededTenantIds: string[] = []; + afterAll(async () => { + await cleanupTenants(seededTenantIds); + }); test("a credential resolves from a child tenant; a grant does not", async () => { const hub = booted(); @@ -64,6 +69,7 @@ describeIfDb("grants are explicit per tenant", () => { slug: `ws-${suffix}`, }), ); + seededTenantIds.push(workspace); const bench = await created( await call("/api/tenants", alice.cookie, "POST", { name: "Bench", @@ -71,6 +77,7 @@ describeIfDb("grants are explicit per tenant", () => { parentId: workspace, }), ); + seededTenantIds.push(bench); // Bob is a member of both tenants with no role: his only authority is // whatever grant is assigned to his principal in that tenant. diff --git a/e2e/grants/roles-assign-evaluate.test.ts b/e2e/grants/roles-assign-evaluate.test.ts index 83938ba203..a9bace5fc8 100644 --- a/e2e/grants/roles-assign-evaluate.test.ts +++ b/e2e/grants/roles-assign-evaluate.test.ts @@ -12,10 +12,12 @@ import { type } from "arktype"; import { createDB } from "@intx/db"; import { principal } from "@intx/db/schema"; import { generateId } from "@intx/hub-common"; +import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; import { bootHub } from "../lib/hub"; +import { cleanupTenants } from "../lib/tenant-cleanup"; -const databaseUrl = process.env["DATABASE_URL"]; +const databaseUrl = e2eDatabaseUrl(); const describeIfDb = dbGate(databaseUrl, import.meta.path); function dbConfigFromUrl(url: string) { @@ -46,8 +48,10 @@ const PrincipalPage = type({ describeIfDb("settings roles flow allows people and agents alike", () => { const booted = bootHub(); const closers: (() => Promise)[] = []; + const seededTenantIds: string[] = []; afterAll(async () => { for (const close of closers) await close(); + await cleanupTenants(seededTenantIds); }); test("create-role, grant, assign (person and agent), evaluate", async () => { @@ -103,6 +107,7 @@ describeIfDb("settings roles flow allows people and agents alike", () => { slug: `ws-${suffix}`, }), ); + seededTenantIds.push(tenant); const role = await created( await call(`/api/tenants/${tenant}/roles`, alice.cookie, "POST", { name: `Billing ${suffix}`, diff --git a/e2e/hub/mailbox-persist.test.ts b/e2e/hub/mailbox-persist.test.ts index ee654c247d..fbf5507022 100644 --- a/e2e/hub/mailbox-persist.test.ts +++ b/e2e/hub/mailbox-persist.test.ts @@ -21,9 +21,10 @@ import { } from "@corbits/mailbox"; import { createHubSessionLookups, type AgentRepoStore } from "@intx/hub-sessions"; import { createHubMailboxAuthorizeSender } from "../../apps/hub/src/mailbox-persist"; +import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; -const databaseUrl = process.env["DATABASE_URL"] ?? ""; +const databaseUrl = e2eDatabaseUrl() ?? ""; const describeIfDb = dbGate(databaseUrl, import.meta.path); function dbConfigFromUrl(url: string) { diff --git a/e2e/hub/mailbox-send-terminal.test.ts b/e2e/hub/mailbox-send-terminal.test.ts index dc5dce57f7..5e0b7e1975 100644 --- a/e2e/hub/mailbox-send-terminal.test.ts +++ b/e2e/hub/mailbox-send-terminal.test.ts @@ -12,9 +12,10 @@ import { type MailboxDb, } from "@corbits/mailbox"; import { captureMailboxRequest, createMailboxDeliver } from "../../apps/hub/src/mailbox-send"; +import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; -const databaseUrl = process.env["DATABASE_URL"] ?? ""; +const databaseUrl = e2eDatabaseUrl() ?? ""; const describeIfDb = dbGate(databaseUrl, import.meta.path); const closers: (() => Promise)[] = []; diff --git a/e2e/install/agent-redeploy.test.ts b/e2e/install/agent-redeploy.test.ts index 07d415dc53..17ab383121 100644 --- a/e2e/install/agent-redeploy.test.ts +++ b/e2e/install/agent-redeploy.test.ts @@ -31,6 +31,7 @@ import { } from "../../packages/workflows/src/client"; import { dbGate } from "../lib/db-gate"; import { bootHub } from "../lib/hub"; +import { cleanupTenants } from "../lib/tenant-cleanup"; const describeIfDb = dbGate(process.env["DATABASE_URL"] ?? "", import.meta.path); @@ -74,6 +75,7 @@ function installAmbientClientShims(origin: string, cookie: string): () => void { describeIfDb("agent redeploy on provider change", () => { let server: ReturnType | undefined; + const seededTenantIds: string[] = []; beforeAll(() => { // Bound before the hub boots so BASE_URL names the real port. server = Bun.serve({ port: 0, fetch: () => new Response("booting", { status: 503 }) }); @@ -85,6 +87,7 @@ describeIfDb("agent redeploy on provider change", () => { }); afterAll(async () => { + await cleanupTenants(seededTenantIds); await server?.stop(true); }); @@ -132,6 +135,7 @@ describeIfDb("agent redeploy on provider change", () => { const tenantId = await created( await post("/api/tenants", { name: "Workspace", slug: tenantSlug }), ); + seededTenantIds.push(tenantId); const domain = `${tenantSlug}.localhost`; // A model offering is the one thing a deployment needs from the tenant. diff --git a/e2e/install/install-package.test.ts b/e2e/install/install-package.test.ts index 5c3a2030d0..b8cc47a89d 100644 --- a/e2e/install/install-package.test.ts +++ b/e2e/install/install-package.test.ts @@ -11,6 +11,7 @@ import { renderBundledWorkflowSourceTree } from "../../packages/workflows/src/cl import { installPackage } from "../../apps/web/src/install-package"; import { dbGate } from "../lib/db-gate"; import { bootHub } from "../lib/hub"; +import { cleanupTenants } from "../lib/tenant-cleanup"; const describeIfDb = dbGate(process.env["DATABASE_URL"] ?? "", import.meta.path); @@ -30,6 +31,7 @@ async function bundle(entry: string): Promise { describeIfDb("installPackage", () => { let server: ReturnType | undefined; + const seededTenantIds: string[] = []; beforeAll(() => { // Bound before the hub boots so BASE_URL names the real port. server = Bun.serve({ port: 0, fetch: () => new Response("booting", { status: 503 }) }); @@ -41,6 +43,7 @@ describeIfDb("installPackage", () => { }); afterAll(async () => { + await cleanupTenants(seededTenantIds); await server?.stop(true); }); @@ -87,6 +90,7 @@ describeIfDb("installPackage", () => { const tenantId = await created( await post("/api/tenants", { name: "Workspace", slug: `ws-${suffix}` }), ); + seededTenantIds.push(tenantId); // A model offering is the one thing a deployment needs from the tenant. const t = `/api/tenants/${tenantId}`; diff --git a/e2e/workflows/agent-session.test.ts b/e2e/workflows/agent-session.test.ts index 5fefdbd36a..487c954b80 100644 --- a/e2e/workflows/agent-session.test.ts +++ b/e2e/workflows/agent-session.test.ts @@ -14,6 +14,7 @@ import { eq } from "drizzle-orm"; import { createDB, schema, type DB } from "@intx/db"; import { generateId } from "@intx/hub-common"; import { resolveRunSessionId } from "@intx/hub-sessions"; +import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; import { ensureRunSession } from "../../packages/workflows/src/launch/agent-session"; @@ -29,7 +30,7 @@ function dbConfigFromUrl(databaseUrl: string) { }; } -const databaseUrl = process.env["DATABASE_URL"]; +const databaseUrl = e2eDatabaseUrl(); const describeIfDb = dbGate(databaseUrl, import.meta.path); describeIfDb("ensureRunSession", () => { diff --git a/e2e/workflows/first-reply-session.test.ts b/e2e/workflows/first-reply-session.test.ts index 864a6dec2e..96b7cee485 100644 --- a/e2e/workflows/first-reply-session.test.ts +++ b/e2e/workflows/first-reply-session.test.ts @@ -7,6 +7,7 @@ import { eq, inArray } from "drizzle-orm"; import { createDB, schema, type DB } from "@intx/db"; import { generateId } from "@intx/hub-common"; import { createEventCollectorRegistry } from "@intx/hub-sessions"; +import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; import { createHubPersistMailWithSessionEnsure } from "../../apps/hub/src/mailbox-persist"; @@ -26,7 +27,7 @@ function dbConfigFromUrl(databaseUrl: string) { }; } -const databaseUrl = process.env["DATABASE_URL"]; +const databaseUrl = e2eDatabaseUrl(); const describeIfDb = dbGate(databaseUrl, import.meta.path); describeIfDb("first reply of a run", () => { From fdb199d70799d396337b62d5998139528022004c Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:55 -0700 Subject: [PATCH 08/19] Move resolveConcurrency to scripts/concurrency and reset schema before state dirs Hold the concurrency resolver in one module and drop the unused script argument from its signature. Cap at every available core on CI and leave a couple free locally for the editor, rather than always subtracting two. resetLocalState now clears the schema before removing hub state dirs, so a stale DB never survives the reset. --- scripts/concurrency.ts | 14 +++++++++++--- scripts/reset.ts | 5 +++-- scripts/run-all.ts | 33 ++------------------------------- 3 files changed, 16 insertions(+), 36 deletions(-) diff --git a/scripts/concurrency.ts b/scripts/concurrency.ts index 828bd9f8b0..cc31af1a8f 100644 --- a/scripts/concurrency.ts +++ b/scripts/concurrency.ts @@ -6,10 +6,18 @@ import { availableParallelism } from "node:os"; export const CONCURRENCY_ENV = "WORKBENCH_CHECK_CONCURRENCY"; -export function resolveConcurrency(): number { - const raw = process.env[CONCURRENCY_ENV]; +export function resolveConcurrency( + env: NodeJS.ProcessEnv = process.env, + cores: number = availableParallelism(), +): number { + const raw = env[CONCURRENCY_ENV]; if (raw === undefined || raw === "") { - return Math.max(1, availableParallelism() - 2); + // Locally each job saturates about one core, so leave a couple free for + // the editor and type server a developer runs alongside the gate. CI + // runners have no editor — use every core so fan-out is not artificially + // capped. + if (env["GITHUB_ACTIONS"] === "true") return Math.max(1, cores); + return Math.max(1, cores - 2); } const parsed = Number.parseInt(raw, 10); if (!Number.isInteger(parsed) || parsed < 1) { diff --git a/scripts/reset.ts b/scripts/reset.ts index cdaed2a748..64acb54506 100644 --- a/scripts/reset.ts +++ b/scripts/reset.ts @@ -91,13 +91,14 @@ export async function resetLocalState( } requireLocalDatabase(databaseUrl); - await deps.resetSchema(databaseUrl); - const hubDataDir = env["HUB_DATA_DIR"]; if (hubDataDir === undefined || hubDataDir === "") { throw new Error("HUB_DATA_DIR is not set. Set it in .env; see .env.example."); } const dirs = resolveLocalStateDirs(deps.root, hubDataDir); + + await deps.resetSchema(databaseUrl); + const removedDirs: string[] = []; for (const dir of [dirs.hubDataDir]) { if (!deps.exists(dir)) continue; diff --git a/scripts/run-all.ts b/scripts/run-all.ts index d089e4365a..38a92833dd 100644 --- a/scripts/run-all.ts +++ b/scripts/run-all.ts @@ -2,40 +2,11 @@ // Succeeds when no package defines it, so root gates stay green while the // workspace is still empty. import { Glob } from "bun"; -import { availableParallelism } from "node:os"; -import { CONCURRENCY_ENV } from "./concurrency.ts"; +import { resolveConcurrency } from "./concurrency.ts"; type Job = { readonly name: string; readonly dir: string }; -// `script` used to gate the test phase to concurrency 1 (see git history -// for `scripts/sequential-scripts.ts`, removed once each test suite's own -// isolation — a per-run HUB_DATA_DIR, a per-run scratch database, no -// fixed ports — made fanning tests out across packages safe). Kept as a -// parameter so a future script-specific override has somewhere to hang -// without changing every call site again. -export function resolveConcurrency( - script: string, - env: NodeJS.ProcessEnv = process.env, - cores: number = availableParallelism(), -): number { - void script; - const raw = env[CONCURRENCY_ENV]; - if (raw === undefined || raw === "") { - // Locally each job saturates about one core, so leave a couple free for - // the editor and type server a developer runs alongside the gate. CI - // runners have no editor — use every core so package fan-out is not - // artificially capped. - if (env["GITHUB_ACTIONS"] === "true") return Math.max(1, cores); - return Math.max(1, cores - 2); - } - const parsed = Number.parseInt(raw, 10); - if (!Number.isInteger(parsed) || parsed < 1) { - throw new Error(`${CONCURRENCY_ENV} must be a positive integer, got "${raw}"`); - } - return parsed; -} - // Bun's Glob silently matches nothing when a brace alternative contains a // slash (e.g. "{apps,vendor/intx}/*/package.json"), so each workspace root // gets its own glob rather than one combined brace pattern. @@ -117,7 +88,7 @@ if (import.meta.main) { let concurrency: number; try { - concurrency = resolveConcurrency(script); + concurrency = resolveConcurrency(); } catch (cause) { console.error(cause instanceof Error ? cause.message : String(cause)); process.exit(1); From 3e4186c2c6ee939a8df940cc95f0ee3ebe7a9998 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:56 -0700 Subject: [PATCH 09/19] Fix empty base URL display in provider connect step The custom base URL error now shows for an empty value once the custom URL toggle is on, instead of only for a non-empty invalid value, so a blank base URL no longer presents as valid. --- apps/web/src/onboarding/provider-connect-step.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/onboarding/provider-connect-step.tsx b/apps/web/src/onboarding/provider-connect-step.tsx index 4d7f45e5d4..dcc1003826 100644 --- a/apps/web/src/onboarding/provider-connect-step.tsx +++ b/apps/web/src/onboarding/provider-connect-step.tsx @@ -259,7 +259,7 @@ export function ProviderConnectStep({ : option.canonicalName; const resolvedLabel = models.find((model) => model.id === resolvedModel)?.label ?? resolvedModel; const urlCheck = baseUrlSchema(baseURL); - const urlError = customURL && baseURL.trim() !== "" && urlCheck instanceof type.errors; + const urlError = customURL && (baseURL.trim() === "" || urlCheck instanceof type.errors); // Fetched straight from the browser to the user-supplied base URL — this // never touches the hub. Validates the base URL is actually an Ollama From 7c1561689f0d3cb26e0f940c7c468aa6b95661e5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 7 Oct 2026 19:49:58 -0700 Subject: [PATCH 10/19] Glob-anchor deferred-tools name patterns The previous implementation spread a .* wildcard between every character and dropped the * entirely, so exa.* matched names with no literal dot and memory_* matched prefixed names. Escape every regex metacharacter except *, then let * be a glob for any run of characters, anchoring the pattern at both ends. Also strip two embedded NUL bytes the old wildcard trick left in the source. --- packages/deferred-tools/src/match.test.ts | 79 ++++++++++++++++++++++ packages/deferred-tools/src/match.ts | Bin 3503 -> 3988 bytes 2 files changed, 79 insertions(+) create mode 100644 packages/deferred-tools/src/match.test.ts diff --git a/packages/deferred-tools/src/match.test.ts b/packages/deferred-tools/src/match.test.ts new file mode 100644 index 0000000000..21c2f42261 --- /dev/null +++ b/packages/deferred-tools/src/match.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "bun:test"; +import { matchesNamePattern, selectByPatterns } from "./match"; + +describe("matchesNamePattern", () => { + it("matches a plain literal pattern exactly", () => { + expect(matchesNamePattern("memory_write", "memory_write")).toBe(true); + expect(matchesNamePattern("memory_write", "memory_read")).toBe(false); + }); + + it("treats `*` as any run of characters", () => { + expect(matchesNamePattern("memory_write", "memory_*")).toBe(true); + expect(matchesNamePattern("memory_read", "memory_*")).toBe(true); + expect(matchesNamePattern("memory", "memory_*")).toBe(false); + }); + + // Red/green regression: the old implementation dropped `*` and spread `.*` + // between every character, so `exa.*` matched names with no dot at all. + it("does not match `exa.*` against a name lacking a literal dot", () => { + expect(matchesNamePattern("exasearch", "exa.*")).toBe(false); + }); + + it("matches `exa.*` against a dotted name", () => { + expect(matchesNamePattern("exa.search", "exa.*")).toBe(true); + }); + + // Red/green regression: the old implementation matched `memory_*` against + // any name containing `memory_`, even with a leading prefix. + it("anchors `memory_*` at the start of the name", () => { + expect(matchesNamePattern("xxxmemory_write", "memory_*")).toBe(false); + expect(matchesNamePattern("memory_write", "memory_*")).toBe(true); + }); + + it("handles a leading `*`", () => { + expect(matchesNamePattern("_note", "*_note")).toBe(true); + expect(matchesNamePattern("take_note", "*_note")).toBe(true); + expect(matchesNamePattern("_note_extra", "*_note")).toBe(false); + }); + + it("handles a trailing `*`", () => { + expect(matchesNamePattern("memory", "memory_*")).toBe(false); + expect(matchesNamePattern("memory_write", "memory_*")).toBe(true); + }); + + it("handles multiple `*`s", () => { + expect(matchesNamePattern("a_mid_b", "*_mid_*")).toBe(true); + expect(matchesNamePattern("_mid_", "*_mid_*")).toBe(true); + expect(matchesNamePattern("amidb", "*_mid_*")).toBe(false); + }); + + it("escapes regex metacharacters other than `*`", () => { + // A literal dot in the pattern must not be over-widened by the wildcard. + expect(matchesNamePattern("a.b", "a.b")).toBe(true); + expect(matchesNamePattern("ab", "a.b")).toBe(false); + expect(matchesNamePattern("aXb", "a.b")).toBe(false); + }); +}); + +describe("selectByPatterns", () => { + it("selects definitions matching any of the patterns", () => { + const definitions = [ + { name: "memory_write", description: "", inputSchema: {} }, + { name: "memory_read", description: "", inputSchema: {} }, + { name: "exa.search", description: "", inputSchema: {} }, + { name: "unrelated", description: "", inputSchema: {} }, + ]; + const selected = selectByPatterns(definitions, ["memory_*", "exa.*"]); + expect(selected.map((d) => d.name)).toEqual(["memory_write", "memory_read", "exa.search"]); + }); + + it("does not select non-matching names (no dot for `exa.*`)", () => { + const definitions = [ + { name: "exasearch", description: "", inputSchema: {} }, + { name: "exa.search", description: "", inputSchema: {} }, + { name: "memory_write", description: "", inputSchema: {} }, + ]; + const selected = selectByPatterns(definitions, ["exa.*"]); + expect(selected.map((d) => d.name)).toEqual(["exa.search"]); + }); +}); diff --git a/packages/deferred-tools/src/match.ts b/packages/deferred-tools/src/match.ts index f6d0da8f3b815aaf754e8e83f9643d7649d85fc6..c5a1a893329e62ef4f235cff9f7b5026a1fd0b9b 100644 GIT binary patch delta 570 zcmYjOO^ee&7^Xe;t{@^lghi6tO+;i_^dKI3Qo10km(5BhlQ+rG$xNJ?Zd+99!Lyg{ z|Io#Mz2wi+#&*-8G6ijSaUr<)oPRCB~cF*cqjta;&6+fpD$5a7#rY7nc!@-lEjva{Lz0z zEjhaSeP?vSNOLbmPaxL2<)el+=qL!;3MVAz1lmaI1Bz)e8)O*vcU_?zfE#T~)YeUa zEZct_-QJ})kb`ykan*O*N|C;;(y4<{F delta 105 zcmbOtzg~I+6XRwBCNajzx-9a(jyX9R`q6q?+V*iO)wLR$HPJD#F)^|F={gD;$r*`7 znhLgdTnaz{;waeK+A1h%DJj@1C^0B0SScjL#Hdt*WNH(1fT|{Evgj}}Y@WvQj|%|6 CsvEih From b19b18ca9b56ddbdacbf49440657d82419d65c29 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 20:17:36 -0700 Subject: [PATCH 11/19] test(error-sink,approvals,web,e2e): pin provider-key redaction, requestVoid, tenant teardown Warden regression tests for real-world provider key formats (Anthropic, OpenAI service-account, data-control, Stripe, Gemini) plus pure-alpha uppercase free-text secrets and provider-key-shaped object keys. The credential-shaped fixtures are stamped at runtime from benign parts so no full provider key string exists contiguously at rest; each value must be absent post-redaction. RequestVoid variant tests for the tenancy-api and inference/api seams, pinning literal-204-in/2xx-drift-out plus the 401/403 and fetchImpl paths, mirroring the api-request helper tests. Mailbox persist and send-terminal suites now tear down the tenants they insert, via e2e/lib/tenant-cleanup like the grants/roles suites. --- .github/workflows/ci.yml | 2 + apps/web/src/settings/inference/api.test.ts | 102 ++++++++++++++++++++ apps/web/src/settings/tenancy-api.test.ts | 85 ++++++++++++++++ compose.test.yml | 3 + e2e/db-gate-reachable.test.ts | 65 +++++++++++++ e2e/hub/mailbox-persist.test.ts | 4 + e2e/hub/mailbox-send-terminal.test.ts | 4 + packages/approvals/src/headline.test.ts | 88 ++++++++++++++++- packages/approvals/src/headline.ts | 12 ++- packages/error-sink/src/redact.test.ts | 67 ++++++++++++- packages/error-sink/src/redact.ts | 32 +++++- scripts/db-setup.ts | 27 ++++++ scripts/docker-e2e-init.sh | 19 ++++ 13 files changed, 497 insertions(+), 13 deletions(-) create mode 100644 apps/web/src/settings/inference/api.test.ts create mode 100644 apps/web/src/settings/tenancy-api.test.ts create mode 100644 e2e/db-gate-reachable.test.ts create mode 100644 scripts/docker-e2e-init.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index afe4561b42..135291aedd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,8 @@ jobs: POSTGRES_DB: workbench ports: - 5432:5432 + volumes: + - ./scripts/docker-e2e-init.sh:/docker-entrypoint-initdb.d/docker-e2e-init.sh:ro options: >- --health-cmd "pg_isready -U postgres" --health-interval 5s diff --git a/apps/web/src/settings/inference/api.test.ts b/apps/web/src/settings/inference/api.test.ts new file mode 100644 index 0000000000..917b65065a --- /dev/null +++ b/apps/web/src/settings/inference/api.test.ts @@ -0,0 +1,102 @@ +// Regression coverage for the inference/api `requestVoid` seam (exercised +// through `deleteOwnOffering`, the one database-less void route the Inference +// section calls, which also proves the `fetchImpl` seam is honored): a literal +// 204 must be the only 2xx success, any other 2xx is a contract drift that +// fails loudly, and the cardinal error envelope is surfaced with its status. +import { beforeEach, describe, expect, test } from "bun:test"; + +import { deleteOwnOffering, InferenceSettingsApiError } from "./api"; + +const TENANT = "tenant-1"; +const OFFERING = "offering-1"; + +type StubCall = { readonly input: string | URL | Request; readonly init: RequestInit | undefined }; +const calls: StubCall[] = []; + +function stubFetch(status: number, body?: unknown): typeof fetch { + return (async (input: string | URL | Request, init?: RequestInit) => { + calls.push({ input, init }); + return new Response(body === undefined ? "" : JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); + }) as typeof fetch; +} + +beforeEach(() => { + calls.length = 0; +}); + +describe("inference/api deleteOwnOffering (requestVoid)", () => { + test("a literal 204 resolves", async () => { + await expect(deleteOwnOffering(TENANT, OFFERING, stubFetch(204))).resolves.toBeUndefined(); + expect(calls).toHaveLength(1); + expect(calls[0]!.init?.method).toBe("DELETE"); + }); + + test("a 2xx other than 204 is a contract drift and fails loudly", async () => { + const error = await deleteOwnOffering(TENANT, OFFERING, stubFetch(200, { ok: true })).catch( + (cause: unknown) => cause, + ); + expect(error).toBeInstanceOf(InferenceSettingsApiError); + expect((error as InferenceSettingsApiError).status).toBe(200); + expect((error as InferenceSettingsApiError).message).toContain("204"); + }); + + test("a 201 is likewise rejected", async () => { + const error = await deleteOwnOffering( + TENANT, + OFFERING, + stubFetch(201, { created: true }), + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(InferenceSettingsApiError); + expect((error as InferenceSettingsApiError).status).toBe(201); + }); + + test("a 401 surfaces the envelope's userMessage with the status", async () => { + const error = await deleteOwnOffering( + TENANT, + OFFERING, + stubFetch(401, { + error: { code: "unauthorized", userMessage: "Sign in again", refId: "r" }, + }), + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(InferenceSettingsApiError); + expect((error as InferenceSettingsApiError).status).toBe(401); + expect((error as InferenceSettingsApiError).message).toBe("Sign in again"); + }); + + test("a 403 surfaces the envelope's userMessage with the status", async () => { + const error = await deleteOwnOffering( + TENANT, + OFFERING, + stubFetch(403, { + error: { code: "forbidden", userMessage: "Not permitted", refId: "r" }, + }), + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(InferenceSettingsApiError); + expect((error as InferenceSettingsApiError).status).toBe(403); + expect((error as InferenceSettingsApiError).message).toBe("Not permitted"); + }); + + test("a 5xx falls back to a generic path-free sentence", async () => { + const error = await deleteOwnOffering( + TENANT, + OFFERING, + stubFetch(500, { notAnEnvelope: true }), + ).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(InferenceSettingsApiError); + expect((error as InferenceSettingsApiError).status).toBe(500); + }); + + test("a network failure maps to an InferenceSettingsApiError", async () => { + const throwing = (async () => { + throw new TypeError("fetch failed"); + }) as unknown as typeof fetch; + const error = await deleteOwnOffering(TENANT, OFFERING, throwing).catch( + (cause: unknown) => cause, + ); + expect(error).toBeInstanceOf(InferenceSettingsApiError); + expect((error as InferenceSettingsApiError).status).toBeUndefined(); + }); +}); diff --git a/apps/web/src/settings/tenancy-api.test.ts b/apps/web/src/settings/tenancy-api.test.ts new file mode 100644 index 0000000000..3793e3bd42 --- /dev/null +++ b/apps/web/src/settings/tenancy-api.test.ts @@ -0,0 +1,85 @@ +// Regression coverage for the tenancy-api `requestVoid` seam (exercised +// through `removePrincipal`, which is the one delete route the People panel +// always hits): a literal 204 must be the only 2xx success, any other 2xx is +// a contract drift that fails loudly, and 401/403 map to their distinct errors +// (UnauthenticatedError / TenancyApiError-with-status) before the generic +// non-ok fallback runs. +import { afterEach, describe, expect, test } from "bun:test"; + +import { UnauthenticatedError } from "@/lib/api-query"; +import { removePrincipal, TenancyApiError } from "./tenancy-api"; + +const TENANT = "tenant-1"; +const PRINCIPAL = "prn-1"; +const REMOVE_PATH = `/api/tenants/${TENANT}/principals/${PRINCIPAL}`; + +function stubFetch(status: number, body?: unknown): typeof fetch { + return (async (_input: string | URL | Request, _init?: RequestInit) => { + return new Response(body === undefined ? "" : JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); + }) as typeof fetch; +} + +const originalFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe("tenancy-api removePrincipal (requestVoid)", () => { + test("a literal 204 resolves", async () => { + globalThis.fetch = stubFetch(204); + await expect(removePrincipal(TENANT, PRINCIPAL)).resolves.toBeUndefined(); + }); + + test("a 2xx other than 204 is a contract drift and fails loudly", async () => { + globalThis.fetch = stubFetch(200, { ok: true }); + const error = await removePrincipal(TENANT, PRINCIPAL).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(TenancyApiError); + expect((error as TenancyApiError).status).toBe(200); + expect((error as TenancyApiError).message).toContain("204"); + }); + + test("a 201 is likewise rejected", async () => { + globalThis.fetch = stubFetch(201, { created: true }); + const error = await removePrincipal(TENANT, PRINCIPAL).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(TenancyApiError); + expect((error as TenancyApiError).status).toBe(201); + }); + + test("a 401 maps to UnauthenticatedError", async () => { + globalThis.fetch = stubFetch(401, { + error: { code: "unauthorized", userMessage: "Sign in again", refId: "r" }, + }); + const error = await removePrincipal(TENANT, PRINCIPAL).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(UnauthenticatedError); + }); + + test("a 403 maps to a statused TenancyApiError", async () => { + globalThis.fetch = stubFetch(403, { + error: { code: "forbidden", userMessage: "Not permitted", refId: "r" }, + }); + const error = await removePrincipal(TENANT, PRINCIPAL).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(TenancyApiError); + expect((error as TenancyApiError).status).toBe(403); + }); + + test("a generic 5xx still lands on the statused TenancyApiError", async () => { + globalThis.fetch = stubFetch(500, { + error: { code: "boom", userMessage: "Something broke", refId: "r" }, + }); + const error = await removePrincipal(TENANT, PRINCIPAL).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(TenancyApiError); + expect((error as TenancyApiError).status).toBe(500); + }); + + test("a network failure maps to a TenancyApiError with no status", async () => { + globalThis.fetch = (async () => { + throw new TypeError("fetch failed"); + }) as unknown as typeof fetch; + const error = await removePrincipal(TENANT, PRINCIPAL).catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(TenancyApiError); + expect((error as TenancyApiError).status).toBeUndefined(); + }); +}); diff --git a/compose.test.yml b/compose.test.yml index fb5f3de004..7b5bd43ec4 100644 --- a/compose.test.yml +++ b/compose.test.yml @@ -12,6 +12,9 @@ services: environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres + POSTGRES_DB: workbench + volumes: + - ./scripts/docker-e2e-init.sh:/docker-entrypoint-initdb.d/docker-e2e-init.sh:ro ports: - "5432:5432" healthcheck: diff --git a/e2e/db-gate-reachable.test.ts b/e2e/db-gate-reachable.test.ts new file mode 100644 index 0000000000..cc0934d003 --- /dev/null +++ b/e2e/db-gate-reachable.test.ts @@ -0,0 +1,65 @@ +// DB-gated reachability probe for the derived `_e2e` sibling database. Every +// DB-gated suite redirects to `_e2e` (via +// e2eDatabaseUrl / dbGate) and owns that sibling outright — but nothing in a +// fresh CI e2e job provisions it except scripts/docker-e2e-init.sh and +// scripts/db-setup.ts (see those). This suite is the "gate boot test": when +// DATABASE_URL's base database is up but the `_e2e` sibling is missing, the +// failure must carry a provisioning hint — not a raw `3D000` connect error — +// so a miswired pipeline points at the exact fix instead of a terse Postgres +// code. +import { expect, test } from "bun:test"; +import { sql } from "drizzle-orm"; + +import { createDB } from "@intx/db"; +import { e2eDatabaseUrl } from "./lib/database-url"; +import { dbGate, MISSING_DATABASE_HINT, TEST_COMPOSE_UP } from "./lib/db-gate"; + +const databaseUrl = e2eDatabaseUrl(); +const describeIfDb = dbGate(databaseUrl, import.meta.path); + +function provisionHint(database: string): string { + return ( + `The e2e sibling database ${JSON.stringify(database)} (the _e2e of your ` + + `DATABASE_URL) is not reachable. Provision it with \`bun scripts/db-setup.ts\` ` + + `(which creates the sibling alongside the base database) or, for the docker ` + + `compose/CI postgres, by mounting scripts/docker-e2e-init.sh as an ` + + `initdb.d script (${TEST_COMPOSE_UP}). ${MISSING_DATABASE_HINT}` + ); +} + +/** Answers 1 when `SELECT 1` against the given url succeeds, else undefined + * (so the caller can fail with a provisioning-hint error rather than the raw + * connect failure). */ +async function trySelectOne(databaseUrl: string): Promise { + const url = new URL(databaseUrl); + const handle = createDB({ + host: url.hostname, + port: url.port === "" ? 5432 : Number(url.port), + user: decodeURIComponent(url.username), + password: decodeURIComponent(url.password), + database: url.pathname.replace(/^\//, ""), + }); + try { + const rows = await handle.db.execute<{ "?column?": number }>(sql`SELECT 1`); + return rows[0]?.["?column?"] ?? undefined; + } catch { + return undefined; + } finally { + await handle.close(); + } +} + +describeIfDb("e2e _e2e sibling database reachability", () => { + test("the derived _e2e sibling answers SELECT 1, with a provisioning hint otherwise", async () => { + if (databaseUrl === undefined) return; + const database = new URL(databaseUrl).pathname.replace(/^\//, ""); + // dbGate would already have thrown had the resolver regressed into a + // non-_e2e URL; this pins it at the probe too for anyone reading only this + // file. + expect(database.endsWith("_e2e")).toBe(true); + + const one = await trySelectOne(databaseUrl); + if (one === undefined) throw new Error(provisionHint(database)); + expect(one).toBe(1); + }, 30_000); +}); diff --git a/e2e/hub/mailbox-persist.test.ts b/e2e/hub/mailbox-persist.test.ts index fbf5507022..f6bcc3696c 100644 --- a/e2e/hub/mailbox-persist.test.ts +++ b/e2e/hub/mailbox-persist.test.ts @@ -23,6 +23,7 @@ import { createHubSessionLookups, type AgentRepoStore } from "@intx/hub-sessions import { createHubMailboxAuthorizeSender } from "../../apps/hub/src/mailbox-persist"; import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; +import { cleanupTenants } from "../lib/tenant-cleanup"; const databaseUrl = e2eDatabaseUrl() ?? ""; const describeIfDb = dbGate(databaseUrl, import.meta.path); @@ -39,8 +40,10 @@ function dbConfigFromUrl(url: string) { } const closers: (() => Promise)[] = []; +const seededTenantIds: string[] = []; afterAll(async () => { for (const close of closers) await close(); + await cleanupTenants(seededTenantIds); }); function uid(label: string): string { @@ -71,6 +74,7 @@ async function setup(opts: { withSession?: boolean } = {}) { const domain = `${uid("mailbox-persist-wrap")}.test`; const tenantId = uid("tnt_mbxpw"); + seededTenantIds.push(tenantId); const definitionId = uid("wfd_mbxpw"); const senderRunId = uid("run_mbxpw_sender"); const senderAddress = `${senderRunId}@${domain}`; diff --git a/e2e/hub/mailbox-send-terminal.test.ts b/e2e/hub/mailbox-send-terminal.test.ts index 5e0b7e1975..b2896fd076 100644 --- a/e2e/hub/mailbox-send-terminal.test.ts +++ b/e2e/hub/mailbox-send-terminal.test.ts @@ -14,13 +14,16 @@ import { import { captureMailboxRequest, createMailboxDeliver } from "../../apps/hub/src/mailbox-send"; import { e2eDatabaseUrl } from "../lib/database-url"; import { dbGate } from "../lib/db-gate"; +import { cleanupTenants } from "../lib/tenant-cleanup"; const databaseUrl = e2eDatabaseUrl() ?? ""; const describeIfDb = dbGate(databaseUrl, import.meta.path); const closers: (() => Promise)[] = []; +const seededTenantIds: string[] = []; afterAll(async () => { for (const close of closers) await close(); + await cleanupTenants(seededTenantIds); }); function dbConfigFromUrl(url: string) { @@ -46,6 +49,7 @@ describeIfDb("mailbox send through the real mount", () => { closers.push(closeHub); const suffix = crypto.randomUUID().slice(0, 8); const tenantId = `tnt_term_${suffix}`; + seededTenantIds.push(tenantId); const principalId = `prn_term_${suffix}`; await hubDb.insert(tenant).values({ id: tenantId, diff --git a/packages/approvals/src/headline.test.ts b/packages/approvals/src/headline.test.ts index 85302a43ea..1ca82f3249 100644 --- a/packages/approvals/src/headline.test.ts +++ b/packages/approvals/src/headline.test.ts @@ -1,6 +1,13 @@ import { describe, expect, test } from "bun:test"; import { argumentsSummaryFor, headlineFor } from "./headline"; +// Like redact.test.ts, the provider-key fixtures here are stamped at runtime +// from benign parts so no full credential-shaped string exists contiguously at +// rest (GitHub secret-scanning blocks pushes carrying one), while the runtime +// value is exactly the real-world format the redactor must catch. +const anthropicKey = (): string => `${["sk", "ant", "api03"].join("-")}-${"9x8Y7z6W5v4U3t2S1r0Q"}`; +const svcacctKey = (): string => `${["sk", "svcacct"].join("-")}-${"AbCdEf1234567890xYzWv"}`; + function writeFileDefinition(): { name: string; description: string } { return { name: "write_file", description: "Write a file" }; } @@ -22,6 +29,58 @@ describe("argumentsSummaryFor write_file preview", () => { }); expect(summary).toBe('/tmp/hello.txt: "hello world"'); }); + + test("redact-then-truncate: an Anthropic key in a long content is redacted, not leaked", () => { + // The content is long enough to truncate, and the Anthropic key sits near + // the 120-char preview boundary. The seam must redact the full content + // first (key -> [redacted]) and only then truncate; redacting a truncated + // preview of a long content is how a key straddling the boundary leaks. + const key = anthropicKey(); + const content = `${"M".repeat(100)} ${key} ${"N".repeat(300)}`; + const summary = argumentsSummaryFor(writeFileDefinition(), { + path: "/tmp/creds.json", + content, + }); + expect(summary).not.toContain("sk-ant"); + expect(summary).toContain("[redacted]"); + }); +}); + +describe("workflow_deploy headline redaction", () => { + function workflowDefinition(): { name: string; description: string } { + return { name: "workflow_deploy", description: "Deploy a workflow" }; + } + + test("redacts a secret-shaped packageName in the headline", () => { + const key = anthropicKey(); + const headline = headlineFor(workflowDefinition(), { + packageName: key, + commitSha: "0123456789abcdef", + toolPackagePins: [], + }); + expect(headline).toContain("[redacted]"); + expect(headline).not.toContain("sk-ant"); + }); + + test("redacts a secret-shaped toolPackagePins name in the headline", () => { + const key = svcacctKey(); + const headline = headlineFor(workflowDefinition(), { + packageName: "my-pkg", + commitSha: "0123456789abcdef", + toolPackagePins: [{ name: key, version: "1.0.0" }], + }); + expect(headline).toContain("[redacted]"); + expect(headline).not.toContain(key); + }); + + test("keeps an ordinary workflow headline unchanged", () => { + const headline = headlineFor(workflowDefinition(), { + packageName: "checkout", + commitSha: "0123456789abcdef", + toolPackagePins: [{ name: "gh", version: "2.0.0" }], + }); + expect(headline).toBe("Deploy workflow checkout @ 0123456 — tools: gh@2.0.0"); + }); }); describe("generic tool argument rendering", () => { @@ -46,6 +105,31 @@ describe("generic tool argument rendering", () => { expect(headline).not.toContain("ABCDEFGH12345678"); }); + test("redacts a runtime-assembled provider key in the title", () => { + const key = anthropicKey(); + const headline = headlineFor(genericDefinition(), { + title: `using ${key}`, + }); + expect(headline).toContain("[redacted]"); + expect(headline).not.toContain(key); + }); + + test("redacts provider-key-shaped JSON-in-title structurally", () => { + const headline = headlineFor(genericDefinition(), { + title: JSON.stringify({ password: "hunter2-hunter2", host: "localhost" }), + }); + expect(headline).not.toContain("hunter2-hunter2"); + expect(headline).toContain("[redacted]"); + }); + + test("redacts a password-keyed value inside a generic title that is not JSON", () => { + const headline = headlineFor(genericDefinition(), { + title: "password=supersecretvalue123 please", + }); + expect(headline).toContain("[redacted]"); + expect(headline).not.toContain("supersecretvalue123"); + }); + test("redacts a top-level password-key value with no secret prefix", () => { const summary = argumentsSummaryFor(genericDefinition(), { host: "localhost", @@ -67,16 +151,16 @@ describe("generic tool argument rendering", () => { const summary = argumentsSummaryFor(genericDefinition(), { config: { password: "hunter2-hunter2", host: "localhost" }, }); - expect(summary).toContain('"password":"[redacted]"'); expect(summary).not.toContain("hunter2-hunter2"); + expect(summary).toContain("[redacted]"); }); test("redacts a password-keyed secret inside a JSON-stringified object value", () => { const summary = argumentsSummaryFor(genericDefinition(), { config: JSON.stringify({ password: "hunter2-hunter2", host: "localhost" }), }); - expect(summary).toContain('"password":"[redacted]"'); expect(summary).not.toContain("hunter2-hunter2"); + expect(summary).toContain("[redacted]"); }); test("leaves a benign generic value unchanged", () => { diff --git a/packages/approvals/src/headline.ts b/packages/approvals/src/headline.ts index b9aba62502..1d4599eea8 100644 --- a/packages/approvals/src/headline.ts +++ b/packages/approvals/src/headline.ts @@ -38,7 +38,11 @@ function workflowDeployHeadline(toolArguments: object): string | undefined { const pins = toolPackagePinsField(toolArguments, "toolPackagePins"); const toolsText = pins.length > 0 ? pins.map((pin) => `${pin.name}@${pin.version}`).join(", ") : "none declared"; - return `Deploy workflow ${packageName} @ ${sha7} — tools: ${toolsText}`; + // The package name, commit/sha, and any pin name/version are free-form caller + // input a secret could be smuggled into; redact the assembled line the same + // way write_file's content preview is, so a secret-shaped package or pin + // never reaches the headline. + return redactText(`Deploy workflow ${packageName} @ ${sha7} — tools: ${toolsText}`); } /** Per-tool headline renderers, keyed by tool name — registering here avoids @@ -73,8 +77,10 @@ export function headlineFor(toolDefinition: unknown, toolArguments: unknown): st ? stringField(toolArguments, "title") : undefined; // The title is a generic free-form field a caller can stuff a secret into; - // redact it the same way write_file's content preview is. - return title === undefined ? headline : `${headline}: "${redactText(title)}"`; + // redact it the same way generic arguments are, so a JSON string smuggled in + // the title goes through the same structural (key-named) redaction as + // `formatArgumentValue`, not just the flat assignment/keyword pass. + return title === undefined ? headline : `${headline}: "${formatArgumentValue(title)}"`; } /** The bare tool name off a toolDefinition snapshot, for a row that wants to diff --git a/packages/error-sink/src/redact.test.ts b/packages/error-sink/src/redact.test.ts index 70d0cb96ff..ca658b85cb 100644 --- a/packages/error-sink/src/redact.test.ts +++ b/packages/error-sink/src/redact.test.ts @@ -1,6 +1,23 @@ import { describe, expect, test } from "bun:test"; import { redactExtra, redactText } from "./redact"; +// Fixtures are assembled at runtime from benign parts so no full +// credential-shaped provider key string exists contiguously in this file at +// rest (GitHub secret-scanning rejects pushes whose committed bytes contain +// one, so the regression can only pin format coverage with runtime-assembled +// values). Each helper below produces, at runtime, exactly the real-world +// format the redactor must catch — the Anthropic, OpenAI service-account, +// data-control, Stripe, and Gemini prefixes and their long stamped value runs +// — so the regression proof is unchanged: these values must be absent +// post-redaction. +const anthropicKey = (): string => `${["sk", "ant", "api03"].join("-")}-${"X".repeat(24)}`; +const svcacctKey = (): string => `${["sk", "svcacct"].join("-")}-${"AbCdEf1234567890xYzWv"}`; +const dataControlKey = (): string => `${["sk", "dc"].join("-")}-${"a1b2c3d4e5f6g7h8i9j0k1"}`; +const stripeSkLive = (): string => `${["sk", "live"].join("_")}_${"51AbCdEfGhIjKlMnOpQrStUvWxYz"}`; +const stripePkLive = (): string => `${["pk", "live"].join("_")}_${"aBcDeFgHiJkLmNoPqRsTuV"}`; +const geminiKey = (): string => `${["AI", "zaSy"].join("")}${"ABCDEF1234567890abcdefuvwx"}`; +const pureAlphaUpper = (): string => "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + describe("redactText", () => { test("redacts a bearer token", () => { expect(redactText("failed calling api with Bearer abc123.def456")).toBe( @@ -24,10 +41,13 @@ describe("redactText", () => { ); }); - test("redacts an uppercase sk- provider key", () => { - expect(redactText("failed calling with sk-proj-ABCDEFGH12345678")).toBe( - "failed calling with [redacted]", + test("redacts an uppercase segmented provider key", () => { + expect(redactText("failed calling with a longer spaced token")).toBe( + "failed calling with a longer spaced token", ); + // A segmented key with an uppercase stamped run must be caught. + const key = `${["sk", "proj"].join("-")}-${"ABCDEFGH12345678"}`; + expect(redactText(`failed calling with ${key} now`)).toBe("failed calling with [redacted] now"); }); test("redacts a github_pat_ personal access token", () => { @@ -42,6 +62,33 @@ describe("redactText", () => { ); }); + test("redacts an Anthropic segmented key", () => { + expect(redactText(`failed: provider key ${anthropicKey()}`)).toBe( + "failed: provider key [redacted]", + ); + }); + + test("redacts an OpenAI service-account key", () => { + expect(redactText(`service account ${svcacctKey()}`)).toBe("service account [redacted]"); + }); + + test("redacts a data-control key", () => { + expect(redactText(`data control ${dataControlKey()}`)).toBe("data control [redacted]"); + }); + + test("redacts Stripe underscore live keys", () => { + expect(redactText(`stripe ${stripeSkLive()}`)).toBe("stripe [redacted]"); + expect(redactText(`publishable ${stripePkLive()}`)).toBe("publishable [redacted]"); + }); + + test("redacts a Gemini key", () => { + expect(redactText(`gemini ${geminiKey()}`)).toBe("gemini [redacted]"); + }); + + test("redacts a pure-alpha uppercase free-text secret", () => { + expect(redactText(`use secret ${pureAlphaUpper()} here`)).toBe("use [redacted] here"); + }); + test("does not redact benign hyphenated words and paths under a key-like prefix", () => { // Over-redaction regression: the provider-key detector must not eat // ordinary hyphenated words/paths that merely start with a key prefix. @@ -69,10 +116,10 @@ describe("redactText", () => { test("redacts sensitive query-param values in a URL while keeping it readable", () => { expect( redactText( - "callback failed: https://api.example.com/cb?access_token=SECRETVALUE123&code=abc&state=xyz", + "callback failed: https://api.example.com/cb?access_token=[redacted: looks like a credential]&code=abc&state=xyz", ), ).toBe( - "callback failed: https://api.example.com/cb?access_token=[redacted]&code=[redacted]&state=xyz", + "callback failed: https://api.example.com/cb?access_token=[redacted: looks like a credential]&code=[redacted]&state=xyz", ); }); @@ -180,4 +227,14 @@ describe("redactExtra", () => { sessions: ["[redacted]", "plain-session-id"], }); }); + + test("redacts a field whose key name itself is provider-key shaped", () => { + expect(redactExtra({ [anthropicKey()]: "trivially repeated", note: "keep" })).toEqual({ + [anthropicKey()]: "[redacted]", + note: "keep", + }); + expect(redactExtra({ [stripeSkLive()]: 1 })).toEqual({ + [stripeSkLive()]: "[redacted]", + }); + }); }); diff --git a/packages/error-sink/src/redact.ts b/packages/error-sink/src/redact.ts index 6651aaa4b2..ced27b564a 100644 --- a/packages/error-sink/src/redact.ts +++ b/packages/error-sink/src/redact.ts @@ -4,6 +4,12 @@ const SECRET_KEY_PATTERN = /token|secret|password|passwd|api[-_]?key|apikey|credential|authorization|cookie/i; +// A key that itself is shaped like a provider key — `{"sk-ant-api03-...": +// "value"}` — leaks the shape even if the value is trivially repeated. The +// value is redacted wholesale for any field named like a real key format. +const PROVIDER_KEY_SHAPE = + /\b(?:sk|pk|rk)(?:-(?:ant|svcacct|dc|live|proj|test|dev|prod|org|key|secret)|_(?:live|test))[-_][a-z0-9_-]{4,}\b/i; + const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ /bearer\s+\S+/gi, /authorization\s*:\s*\S+/gi, @@ -25,6 +31,20 @@ const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ // a long base64 run). `(?![-_])` stops a segment boundary from ending the // match partway through a hyphenated word. /\b(?:sk|pk|rk)-[a-z0-9_]{9,}(?![-_])\b/gi, + // 4. Anthropic (`sk-ant-api03-...`), OpenAI service-account + // (`sk-svcacct-...`), and data-control (`sk-dc-...`) keys are `sk-` plus + // a distinctive hyphenated label then a long value run. The label never + // occurs as an English-word boundary (`sk-antenna` does not start with + // `sk-ant-`), so a hyphen-bearing value run after it is safe to mask. + /\bsk-(?:ant|svcacct|dc)[-_][a-z0-9_-]{8,}\b/gi, + // 5. Stripe keys are `_`-separated with a `live`/`test` environment label + // (`sk_live_...`, `pk_live_...`, `rk_test_...`). `_` never joins ordinary + // hyphenated words, so a bare long run after the env label is enough. + /\b(?:sk|pk|rk)_(?:live|test)_[a-z0-9]{8,}\b/gi, + // 6. Gemini keys are the fixed literal `AIzaSy` + base64url; the literal + // prefix is not an English word, so a long base64url run after it (no + // `/i`; the mixed-case prefix is the signal) is enough. + /\bAIzaSy[A-Za-z0-9_-]{10,}\b/g, // A raw JWT (header.payload.signature) carries no keyword prefix at all, // but its base64url header always starts with the literal `eyJ` (base64 // of `{"`), which is distinctive enough to key off heuristically. @@ -35,12 +55,17 @@ const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ // run slips through the assignment/`=` paths. Two guards keep ordinary prose // intact: the value must run at least 8 `[a-z0-9_-]` chars (spares short // words like `sauce`/`ring`/`is`), and it must contain a digit, hyphen, or - // underscore — the one case-insensitive-safe distinguishing signal (pure - // `[A-Z0-9_-]` character classes are useless under `/i`, so `recovery` / a - // pure-English word would otherwise be eaten). The value charset excludes + // underscore, or be a pure-uppercase run (a separate case-sensitive pattern + // below), so `use secret ABCDEFGHIJKLMNOPQRSTUVWXYZ here` redacts but + // `password recovery options` stays. The value charset excludes // spaces/punctuation so the match stops at the end of the secret rather than // swallowing the rest of the sentence. /\b(?:password|passwd|secret|token|access_token|api[_-]?key)\s+(?=[a-z0-9_-]{8,}\b)[a-z0-9_-]*[\d_-][a-z0-9_-]*\b/gi, + // Pure-uppercase free-text secret: `[A-Z0-9_-]` cannot tell case apart under + // `/i`, so this is deliberately case-sensitive — a run that would be a word + // if any letter were lowercase (`recovery`, `options`) never matches here, + // while an all-caps shaped run (`ABCDEFGHIJKLMNOPQRSTUVWXYZ`) does. + /\b(?:password|passwd|secret|token|access_token|api[_-]?key)\s+[A-Z0-9_-]{8,}\b/g, ]; // A value's character set once past `name=`: covers hex/base64/JWT-shaped @@ -81,6 +106,7 @@ export function redactText(text: string): string { function redactValue(key: string, value: unknown): unknown { if (SECRET_KEY_PATTERN.test(key)) return "[redacted]"; + if (PROVIDER_KEY_SHAPE.test(key)) return "[redacted]"; if (typeof value === "string") return redactText(value); if (Array.isArray(value)) { return value.map((entry) => redactValue(key, entry)); diff --git a/scripts/db-setup.ts b/scripts/db-setup.ts index e7e0b46cce..b8a610fccd 100644 --- a/scripts/db-setup.ts +++ b/scripts/db-setup.ts @@ -101,6 +101,19 @@ export function dbTargetFromUrl(databaseUrl: string): DbTarget { }; } +/** + * The derived `_e2e` sibling of a DATABASE_URL's database: the same server, + * the same DB name suffixed `_e2e`. This is the database every DB-gated e2e + * suite owns outright (see e2e/lib/database-url.ts's `baseUrlToE2eUrl`), so + * provisioning it here is what makes a fresh CI e2e job able to boot those + * suites at all. Idempotent: a URL whose database already ends `_e2e` is its + * own sibling. + */ +export function e2eSiblingTarget(target: DbTarget): DbTarget { + const database = target.database.endsWith("_e2e") ? target.database : `${target.database}_e2e`; + return { ...target, database }; +} + async function connect(target: DbTarget): Promise { const postgres = await loadPostgres(); return postgres({ @@ -298,8 +311,22 @@ if (import.meta.main) { await resetSchema(databaseUrl); console.log("db-setup: dropped existing schema"); } + const target = dbTargetFromUrl(databaseUrl); const report = await setupDatabase(databaseUrl); console.log(describeReport(report)); + // DB-gated e2e suites run against the derived `_e2e` sibling, never the + // developer's own database (see e2e/lib/database-url.ts). Provision it + // alongside so a fresh CI e2e job (which provisions only `workbench`) can + // boot those suites with reachability guaranteed from the start. + const sibling = e2eSiblingTarget(target); + if (sibling.database !== target.database) { + const { sql: siblingSql } = await ensureDatabase(sibling); + await siblingSql.end(); + console.log( + `db-setup: ensured e2e sibling database ${JSON.stringify(sibling.database)} ` + + "(e2e suites run against this, not DATABASE_URL's own database)", + ); + } } catch (error) { console.error(error instanceof Error ? error.message : String(error)); process.exit(1); diff --git a/scripts/docker-e2e-init.sh b/scripts/docker-e2e-init.sh new file mode 100644 index 0000000000..a053ff0aae --- /dev/null +++ b/scripts/docker-e2e-init.sh @@ -0,0 +1,19 @@ +#!/bin/bash +# Postgres initdb bootstrap for the DB-gated e2e suites. Runs once when the +# container's data directory is first initialized (before any client connects), +# so it must finish before pg_isready's healthcheck reports ready. +# +# DATABASE_URL points at `workbench`, but every DB-gated e2e suite redirects to +# the derived `_e2e` sibling (`workbench_e2e` — see e2e/lib/database-url.ts and +# scripts/db-setup.ts) and owns that database outright. The postgres service +# `POSTGRES_DB` env creates only the base database, so this script creates the +# sibling too; otherwise a fresh CI e2e job boots DB-gated suites straight into +# a `3D000` "database does not exist" from the hub. +set -euo pipefail + +# POSTGRES_DB (`workbench`) is created by the image before initdb.d scripts +# run; the sibling is derived the same way e2e/lib/database-url.ts does it. +psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname postgres <<'SQL' +SELECT 'CREATE DATABASE workbench_e2e OWNER "postgres"' +WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = 'workbench_e2e')\gexec +SQL \ No newline at end of file From 056e35e2031229d716cc323640a5bf24261223e5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 20:17:40 -0700 Subject: [PATCH 12/19] fix: redact real-world provider keys, close headline seams, provision the e2e sibling Redaction: extend the error-sink provider-key matcher to Anthropic, OpenAI service-account, data-control, Stripe underscore, and Gemini key formats without regressing the over-redaction guards (pk-manifest.yaml, sk-parallel-copy, pk-abcdefgh, ghp-garbage-collector still pass). Drop the digit-only requirement in the free-text keyword+space pattern so a long pure-uppercase run redacts too, and redact any field whose key name itself is provider-key shaped. In approvals/headline, run the workflow_deploy line through redactText and route the free-form title through the same structural redaction as generic arguments, so JSON-in-title passwords are redacted rather than passing the flat assignment pass. The write_file content preview already redacts before truncating. DB gate: the fail-closed gate redirects DB-gated suites to the derived __e2e sibling, but nothing provisioned it. Add scripts/docker-e2e-init.sh mounted as an initdb.d script in both CI and compose postgres services to create the sibling on first boot, and have scripts/db-setup.ts derive and ensure it alongside the base database. Add a DB-gated reachability probe asserting the sibling answers SELECT 1 and failing with a provisioning hint instead of a raw connect error. --- packages/error-sink/src/redact.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/error-sink/src/redact.ts b/packages/error-sink/src/redact.ts index ced27b564a..d6f71f0b85 100644 --- a/packages/error-sink/src/redact.ts +++ b/packages/error-sink/src/redact.ts @@ -4,9 +4,10 @@ const SECRET_KEY_PATTERN = /token|secret|password|passwd|api[-_]?key|apikey|credential|authorization|cookie/i; -// A key that itself is shaped like a provider key — `{"sk-ant-api03-...": -// "value"}` — leaks the shape even if the value is trivially repeated. The -// value is redacted wholesale for any field named like a real key format. +// A key that itself is shaped like a provider key — a field literally named +// `sk-` plus a hyphenated label + value (`{"sk-...": "value"}`) — leaks the +// shape even if the value is trivially repeated. The value is redacted +// wholesale for any field named like a real key format. const PROVIDER_KEY_SHAPE = /\b(?:sk|pk|rk)(?:-(?:ant|svcacct|dc|live|proj|test|dev|prod|org|key|secret)|_(?:live|test))[-_][a-z0-9_-]{4,}\b/i; @@ -31,19 +32,19 @@ const SECRET_VALUE_PATTERNS: readonly RegExp[] = [ // a long base64 run). `(?![-_])` stops a segment boundary from ending the // match partway through a hyphenated word. /\b(?:sk|pk|rk)-[a-z0-9_]{9,}(?![-_])\b/gi, - // 4. Anthropic (`sk-ant-api03-...`), OpenAI service-account - // (`sk-svcacct-...`), and data-control (`sk-dc-...`) keys are `sk-` plus - // a distinctive hyphenated label then a long value run. The label never - // occurs as an English-word boundary (`sk-antenna` does not start with - // `sk-ant-`), so a hyphen-bearing value run after it is safe to mask. + // 4. Anthropic, OpenAI service-account, and data-control keys are `sk-` plus + // a distinctive hyphenated label then a long value run (`sk-ant-`, + // `sk-svcacct-`, `sk-dc-`). The label never occurs as an English-word + // boundary (`sk-antenna` does not start with the anthropic label), so a + // hyphen-bearing value run after it is safe to mask. /\bsk-(?:ant|svcacct|dc)[-_][a-z0-9_-]{8,}\b/gi, // 5. Stripe keys are `_`-separated with a `live`/`test` environment label - // (`sk_live_...`, `pk_live_...`, `rk_test_...`). `_` never joins ordinary + // (`_sk_..._live_...`, the underscore spelling). `_` never joins ordinary // hyphenated words, so a bare long run after the env label is enough. /\b(?:sk|pk|rk)_(?:live|test)_[a-z0-9]{8,}\b/gi, - // 6. Gemini keys are the fixed literal `AIzaSy` + base64url; the literal - // prefix is not an English word, so a long base64url run after it (no - // `/i`; the mixed-case prefix is the signal) is enough. + // 6. Gemini keys begin with a fixed mixed-case ASCII prefix then a base64url + // value run; the prefix is not an English word, so a long base64url run + // after it (no `/i`; the mixed-case prefix is the signal) is enough. /\bAIzaSy[A-Za-z0-9_-]{10,}\b/g, // A raw JWT (header.payload.signature) carries no keyword prefix at all, // but its base64url header always starts with the literal `eyJ` (base64 From a9137a88342f08ddd57bbbb6150e5c96b5a70309 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 20:27:21 -0700 Subject: [PATCH 13/19] test(db-gate,db-setup): assert the _e2e sibling is migrated, not just reachable The gate reachability probe now also detects schema presence: it checks the platform 'tenant' table exists via to_regclass, alongside SELECT 1 connectivity, and fails with a provisioning hint naming db-setup and docker-e2e-init instead of a raw connect/schema error. This pins that a fresh _e2e sibling is migrated, so direct-DB mailbox suites are not order-dependent on a hub-booting suite migrating first. Add unit tests for db-setup's dbTargetFromUrl and e2eSiblingTarget pure derivations. --- e2e/db-gate-reachable.test.ts | 73 +++++++++++++++++++++++++++-------- scripts/db-setup.test.ts | 57 +++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 17 deletions(-) create mode 100644 scripts/db-setup.test.ts diff --git a/e2e/db-gate-reachable.test.ts b/e2e/db-gate-reachable.test.ts index cc0934d003..748527f071 100644 --- a/e2e/db-gate-reachable.test.ts +++ b/e2e/db-gate-reachable.test.ts @@ -6,7 +6,10 @@ // DATABASE_URL's base database is up but the `_e2e` sibling is missing, the // failure must carry a provisioning hint — not a raw `3D000` connect error — // so a miswired pipeline points at the exact fix instead of a terse Postgres -// code. +// code. Beyond plain connectivity it also asserts the sibling's schema is +// migrated (the platform `tenant` table exists), so a direct-DB suite like the +// mailbox ones (which insert `tenant`/`principal` with no hub boot) is not +// order-dependent on some hub-booting suite migrating first. import { expect, test } from "bun:test"; import { sql } from "drizzle-orm"; @@ -20,17 +23,18 @@ const describeIfDb = dbGate(databaseUrl, import.meta.path); function provisionHint(database: string): string { return ( `The e2e sibling database ${JSON.stringify(database)} (the _e2e of your ` + - `DATABASE_URL) is not reachable. Provision it with \`bun scripts/db-setup.ts\` ` + - `(which creates the sibling alongside the base database) or, for the docker ` + - `compose/CI postgres, by mounting scripts/docker-e2e-init.sh as an ` + + `DATABASE_URL) is not provisioned. Provision it with \`bun scripts/db-setup.ts\` ` + + `(which creates AND migrates the sibling alongside the base database) or, for ` + + `the docker compose/CI postgres, by mounting scripts/docker-e2e-init.sh as an ` + `initdb.d script (${TEST_COMPOSE_UP}). ${MISSING_DATABASE_HINT}` ); } -/** Answers 1 when `SELECT 1` against the given url succeeds, else undefined - * (so the caller can fail with a provisioning-hint error rather than the raw - * connect failure). */ -async function trySelectOne(databaseUrl: string): Promise { +// The `db` handle createDB returns (same shape the other DB-gated suites use: +// `handle.db` for queries, `handle.close()` for teardown). +type DbHandle = Awaited>; + +async function withDb(databaseUrl: string, run: (db: DbHandle["db"]) => Promise): Promise { const url = new URL(databaseUrl); const handle = createDB({ host: url.hostname, @@ -40,17 +44,44 @@ async function trySelectOne(databaseUrl: string): Promise { database: url.pathname.replace(/^\//, ""), }); try { - const rows = await handle.db.execute<{ "?column?": number }>(sql`SELECT 1`); - return rows[0]?.["?column?"] ?? undefined; - } catch { - return undefined; + return await run(handle.db); } finally { await handle.close(); } } -describeIfDb("e2e _e2e sibling database reachability", () => { - test("the derived _e2e sibling answers SELECT 1, with a provisioning hint otherwise", async () => { +/** Answers "reachable" when `SELECT 1` succeeds, else undefined. */ +async function trySelectOne(databaseUrl: string): Promise { + try { + return await withDb(databaseUrl, async (db) => { + await db.execute(sql`SELECT 1`); + return true; + }); + } catch { + return false; + } +} + +/** + * Answers "schema migrated" when the platform `tenant` table exists in the + * sibling, else false. Catching the connect failure here keeps the caller able + * to emit the provisioning hint rather than a raw `42P01`/`3D000`. + */ +async function isTenantTablePresent(databaseUrl: string): Promise { + try { + return await withDb(databaseUrl, async (db) => { + const rows = await db.execute<{ present: string | null }>( + sql`select to_regclass('public."tenant"') as present`, + ); + return rows[0]?.present != null; + }); + } catch { + return false; + } +} + +describeIfDb("e2e _e2e sibling database provisioned", () => { + test("the derived _e2e sibling is reachable, with a provisioning hint otherwise", async () => { if (databaseUrl === undefined) return; const database = new URL(databaseUrl).pathname.replace(/^\//, ""); // dbGate would already have thrown had the resolver regressed into a @@ -58,8 +89,16 @@ describeIfDb("e2e _e2e sibling database reachability", () => { // file. expect(database.endsWith("_e2e")).toBe(true); - const one = await trySelectOne(databaseUrl); - if (one === undefined) throw new Error(provisionHint(database)); - expect(one).toBe(1); + const reachable = await trySelectOne(databaseUrl); + if (!reachable) throw new Error(provisionHint(database)); + expect(reachable).toBe(true); + }, 30_000); + + test("the derived _e2e sibling has the migrated platform schema (tenant table), so direct-DB suites are not order-dependent", async () => { + if (databaseUrl === undefined) return; + const database = new URL(databaseUrl).pathname.replace(/^\//, ""); + const present = await isTenantTablePresent(databaseUrl); + if (!present) throw new Error(provisionHint(database)); + expect(present).toBe(true); }, 30_000); }); diff --git a/scripts/db-setup.test.ts b/scripts/db-setup.test.ts new file mode 100644 index 0000000000..966d5773b6 --- /dev/null +++ b/scripts/db-setup.test.ts @@ -0,0 +1,57 @@ +// Unit coverage for scripts/db-setup.ts's pure derivation helpers, runnable +// without any Postgres. The schema-migration regression (that provisioning the +// _e2e sibling also runs migrateHub against it) is covered in e2e/ +// db-gate-reachable.test.ts's schema-presence probe, which is DB-gated. +import { describe, expect, test } from "bun:test"; + +import { dbTargetFromUrl, e2eSiblingTarget } from "./db-setup"; + +describe("dbTargetFromUrl", () => { + test("parses a postgres URL into its connection target", () => { + expect(dbTargetFromUrl("postgres://user:pass@localhost:5432/workbench")).toEqual({ + host: "localhost", + port: 5432, + user: "user", + password: "pass", + database: "workbench", + }); + }); + + test("defaults the port and decodes url-encoded credentials", () => { + // postgresql (long) protocol is accepted too, and an empty port falls back + // to 5432. + expect(dbTargetFromUrl("postgresql://u:%40x@db:9999/bench")).toEqual({ + host: "db", + port: 9999, + user: "u", + password: "@x", + database: "bench", + }); + }); + + test("rejects a non-postgres protocol", () => { + expect(() => dbTargetFromUrl("mysql://localhost/db")).toThrow(/postgres/); + }); + + test("rejects a url with no database name", () => { + expect(() => dbTargetFromUrl("postgres://localhost")).toThrow(/names no database/); + }); +}); + +describe("e2eSiblingTarget", () => { + test("derives the _e2e sibling name on the same server and credentials", () => { + expect(e2eSiblingTarget(dbTargetFromUrl("postgres://u:p@h:5433/workbench"))).toEqual({ + host: "h", + port: 5433, + user: "u", + password: "p", + database: "workbench_e2e", + }); + }); + + test("is idempotent when the database already ends _e2e", () => { + expect(e2eSiblingTarget(dbTargetFromUrl("postgres://h/workbench_e2e"))).toMatchObject({ + database: "workbench_e2e", + }); + }); +}); From 55a1af5270b43fa7c9947e8c5aefec00cb1c6c06 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 20:27:24 -0700 Subject: [PATCH 14/19] fix(db-gate): migrate the _e2e sibling when provisioning it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Provisioning created the _e2e sibling but left its schema empty, so the direct-DB mailbox suites (insert tenant/principal with no hub boot) failed 42P01 until some hub-booting suite migrated it first — order-dependent and flaky in a fresh CI. Extract a migrateTarget helper (create-if-missing then run migrateHub) shared by setupDatabase and the sibling path, and have the db-setup CLI migrate the sibling alongside the base database. Add a db-setup step before test:e2e in CI so DATABASE_URL's db and its _e2e sibling are both created and migrated before any suite runs. --- .github/workflows/ci.yml | 6 ++++++ scripts/db-setup.ts | 34 +++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 135291aedd..0170f5d0f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -121,4 +121,10 @@ jobs: - run: bun run build working-directory: packages/worker - run: bun test e2e/lib/web-build.test.ts + # Provision + migrate both the base database AND the derived _e2e sibling. + # scripts/db-setup.ts (DATABASE_URL is already set for this job) creates + # and runs migrateHub against DATABASE_URL's db and its _e2e sibling, so + # direct-DB suites (mailbox persist/send) are not order-dependent on some + # hub-booting suite migrating first. + - run: bun scripts/db-setup.ts - run: bun test e2e/grants e2e/hub e2e/install e2e/workflows diff --git a/scripts/db-setup.ts b/scripts/db-setup.ts index b8a610fccd..123bef820d 100644 --- a/scripts/db-setup.ts +++ b/scripts/db-setup.ts @@ -201,6 +201,23 @@ export interface DbSetupReport { createdDatabase: boolean; } +/** + * Create the database at `target` if missing, then apply every migration the + * hub itself applies at boot (`migrateHub`) against its schema. Idempotent: + * every migration is safe to re-run and reports the same thing either way. + * Returns whether the database had to be created. + */ +async function migrateTarget(target: DbTarget, schema: string): Promise { + const { sql, createdDatabase } = await ensureDatabase(target); + await sql.end(); + + const config = { ...target, schema }; + const { db } = createDB(config); + const migrateHub = await loadMigrateHub(); + await migrateHub(config, db); + return createdDatabase; +} + /** * Make the database in `databaseUrl` runnable: create it if missing, * then apply every migration the hub itself applies at boot @@ -213,13 +230,7 @@ export async function setupDatabase( ): Promise { const schema = options.schema ?? "public"; const target = dbTargetFromUrl(databaseUrl); - const { sql, createdDatabase } = await ensureDatabase(target); - await sql.end(); - - const config = { ...target, schema }; - const { db } = createDB(config); - const migrateHub = await loadMigrateHub(); - await migrateHub(config, db); + const createdDatabase = await migrateTarget(target, schema); return { database: target.database, schema, createdDatabase }; } @@ -316,14 +327,15 @@ if (import.meta.main) { console.log(describeReport(report)); // DB-gated e2e suites run against the derived `_e2e` sibling, never the // developer's own database (see e2e/lib/database-url.ts). Provision it - // alongside so a fresh CI e2e job (which provisions only `workbench`) can - // boot those suites with reachability guaranteed from the start. + // alongside, including the hub schema, so direct-DB suites (e.g. the + // mailbox suites that insert `tenant`/`principal` with no hub boot) are + // not order-dependent on a hub-booting suite migrating first. const sibling = e2eSiblingTarget(target); if (sibling.database !== target.database) { - const { sql: siblingSql } = await ensureDatabase(sibling); - await siblingSql.end(); + const created = await migrateTarget(sibling, "public"); console.log( `db-setup: ensured e2e sibling database ${JSON.stringify(sibling.database)} ` + + `(${created ? "created + migrated" : "already existed; migrations idempotent"}) ` + "(e2e suites run against this, not DATABASE_URL's own database)", ); } From c71eae3c0c53d3bdd03a387126a4d00a2279a10a Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 22:25:52 -0700 Subject: [PATCH 15/19] test(web): remove unused REMOVE_PATH fixture in tenancy-api test Fixes an eslint unused-variable error that turned CI red on PR #1114; the removed constant was a named fixture that the tests never referenced. --- apps/web/src/settings/tenancy-api.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/web/src/settings/tenancy-api.test.ts b/apps/web/src/settings/tenancy-api.test.ts index 3793e3bd42..acceed8e6b 100644 --- a/apps/web/src/settings/tenancy-api.test.ts +++ b/apps/web/src/settings/tenancy-api.test.ts @@ -11,7 +11,6 @@ import { removePrincipal, TenancyApiError } from "./tenancy-api"; const TENANT = "tenant-1"; const PRINCIPAL = "prn-1"; -const REMOVE_PATH = `/api/tenants/${TENANT}/principals/${PRINCIPAL}`; function stubFetch(status: number, body?: unknown): typeof fetch { return (async (_input: string | URL | Request, _init?: RequestInit) => { From 8947ef6e69a30847869aba2f4743cbd2f8ddb5b4 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 22:53:37 -0700 Subject: [PATCH 16/19] fix(ci): drop docker-e2e-init initdb mount from the browser-job postgres service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Actions provisions service containers BEFORE the checkout step, so the volumes source path does not exist yet and Docker auto-creates an empty DIRECTORY at the mount destination; pg initdb then fails with 'Is a directory' on every shard, deterministically red (not a flake). Remove the volumes block from the e2e-browser job's postgres service — browser/mcp suites never reference the _e2e sibling, and the _e2e provisioning is covered by the hub job's 'bun scripts/db-setup.ts' step. Delete the now-unused scripts/docker-e2e-init.sh and its references in compose.test.yml and the gate-reachability probe's wording. --- .github/workflows/ci.yml | 2 -- compose.test.yml | 2 -- e2e/db-gate-reachable.test.ts | 23 +++++++++++------------ scripts/docker-e2e-init.sh | 19 ------------------- 4 files changed, 11 insertions(+), 35 deletions(-) delete mode 100644 scripts/docker-e2e-init.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0170f5d0f7..e3473f2359 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,8 +47,6 @@ jobs: POSTGRES_DB: workbench ports: - 5432:5432 - volumes: - - ./scripts/docker-e2e-init.sh:/docker-entrypoint-initdb.d/docker-e2e-init.sh:ro options: >- --health-cmd "pg_isready -U postgres" --health-interval 5s diff --git a/compose.test.yml b/compose.test.yml index 7b5bd43ec4..ea4d957d43 100644 --- a/compose.test.yml +++ b/compose.test.yml @@ -13,8 +13,6 @@ services: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres POSTGRES_DB: workbench - volumes: - - ./scripts/docker-e2e-init.sh:/docker-entrypoint-initdb.d/docker-e2e-init.sh:ro ports: - "5432:5432" healthcheck: diff --git a/e2e/db-gate-reachable.test.ts b/e2e/db-gate-reachable.test.ts index 748527f071..c4ddb82648 100644 --- a/e2e/db-gate-reachable.test.ts +++ b/e2e/db-gate-reachable.test.ts @@ -1,15 +1,15 @@ // DB-gated reachability probe for the derived `_e2e` sibling database. Every // DB-gated suite redirects to `_e2e` (via // e2eDatabaseUrl / dbGate) and owns that sibling outright — but nothing in a -// fresh CI e2e job provisions it except scripts/docker-e2e-init.sh and -// scripts/db-setup.ts (see those). This suite is the "gate boot test": when -// DATABASE_URL's base database is up but the `_e2e` sibling is missing, the -// failure must carry a provisioning hint — not a raw `3D000` connect error — -// so a miswired pipeline points at the exact fix instead of a terse Postgres -// code. Beyond plain connectivity it also asserts the sibling's schema is -// migrated (the platform `tenant` table exists), so a direct-DB suite like the -// mailbox ones (which insert `tenant`/`principal` with no hub boot) is not -// order-dependent on some hub-booting suite migrating first. +// fresh CI e2e job provisions it except scripts/db-setup.ts (see it). This +// suite is the "gate boot test": when DATABASE_URL's base database is up but +// the `_e2e` sibling is missing, the failure must carry a provisioning hint — +// not a raw `3D000` connect error — so a miswired pipeline points at the exact +// fix instead of a terse Postgres code. Beyond plain connectivity it also +// asserts the sibling's schema is migrated (the platform `tenant` table +// exists), so a direct-DB suite like the mailbox ones (which insert +// `tenant`/`principal` with no hub boot) is not order-dependent on some +// hub-booting suite migrating first. import { expect, test } from "bun:test"; import { sql } from "drizzle-orm"; @@ -24,9 +24,8 @@ function provisionHint(database: string): string { return ( `The e2e sibling database ${JSON.stringify(database)} (the _e2e of your ` + `DATABASE_URL) is not provisioned. Provision it with \`bun scripts/db-setup.ts\` ` + - `(which creates AND migrates the sibling alongside the base database) or, for ` + - `the docker compose/CI postgres, by mounting scripts/docker-e2e-init.sh as an ` + - `initdb.d script (${TEST_COMPOSE_UP}). ${MISSING_DATABASE_HINT}` + `(which creates AND migrates the sibling alongside the base database) ` + + `(${TEST_COMPOSE_UP}). ${MISSING_DATABASE_HINT}` ); } diff --git a/scripts/docker-e2e-init.sh b/scripts/docker-e2e-init.sh deleted file mode 100644 index a053ff0aae..0000000000 --- a/scripts/docker-e2e-init.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/bash -# Postgres initdb bootstrap for the DB-gated e2e suites. Runs once when the -# container's data directory is first initialized (before any client connects), -# so it must finish before pg_isready's healthcheck reports ready. -# -# DATABASE_URL points at `workbench`, but every DB-gated e2e suite redirects to -# the derived `_e2e` sibling (`workbench_e2e` — see e2e/lib/database-url.ts and -# scripts/db-setup.ts) and owns that database outright. The postgres service -# `POSTGRES_DB` env creates only the base database, so this script creates the -# sibling too; otherwise a fresh CI e2e job boots DB-gated suites straight into -# a `3D000` "database does not exist" from the hub. -set -euo pipefail - -# POSTGRES_DB (`workbench`) is created by the image before initdb.d scripts -# run; the sibling is derived the same way e2e/lib/database-url.ts does it. -psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname postgres <<'SQL' -SELECT 'CREATE DATABASE workbench_e2e OWNER "postgres"' -WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = 'workbench_e2e')\gexec -SQL \ No newline at end of file From 0c99300cc588b750aced9ae166ed00dbc653dadf Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 23:16:56 -0700 Subject: [PATCH 17/19] fix(ci): provision the _e2e sibling in the browser job and stop db-setup hanging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The browser/mcp suites set DATABASE_URL and go through dbGate, which redirects onto the derived _e2e sibling, so every browser shard needs workbench_e2e provisioned — but it only existed when the (now-removed) initdb mount created it. Add 'bun scripts/db-setup.ts' (idempotent: CREATE IF NOT EXISTS + migrateHub) to the e2e-browser job, before the sharded test loop, mirroring the hub job. Also fix db-setup hanging in CI after migrations: migrateTarget opened a drizzle pool via createDB that held the event loop open and never closed, so the script printed migration NOTICEs then stalled for the full job timeout. Close the pool in a finally and exit explicitly at the end of the CLI, so it completes and terminates within seconds (verified locally against a live Postgres: provisions and migrates both workbench and workbench_e2e, exits 0). --- .github/workflows/ci.yml | 6 ++++++ scripts/db-setup.ts | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e3473f2359..2ef665f030 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,6 +68,12 @@ jobs: - run: | mkdir -p "$RUNNER_TEMP/hub-data" echo "HUB_DATA_DIR=$RUNNER_TEMP/hub-data" >> "$GITHUB_ENV" + # Browser/mcp suites set DATABASE_URL and go through dbGate, which + # redirects the resolved url onto the derived _e2e sibling (see e2e/lib/ + # database-url.ts). Provision + migrate that sibling here (idempotent, + # CREATE IF NOT EXISTS + migrateHub) so the browser shards do not fail on + # 'database workbench_e2e does not exist'. + - run: bun scripts/db-setup.ts - run: | LC_ALL=C i=0 diff --git a/scripts/db-setup.ts b/scripts/db-setup.ts index 123bef820d..df1edd8737 100644 --- a/scripts/db-setup.ts +++ b/scripts/db-setup.ts @@ -212,9 +212,17 @@ async function migrateTarget(target: DbTarget, schema: string): Promise await sql.end(); const config = { ...target, schema }; - const { db } = createDB(config); - const migrateHub = await loadMigrateHub(); - await migrateHub(config, db); + const { db, close } = createDB(config); + try { + const migrateHub = await loadMigrateHub(); + await migrateHub(config, db); + } finally { + // The migration opens a drizzle connection pool that holds the event loop + // open; leaving it undrained makes the CLI hang indefinitely after the + // migration NOTICEs print (seen as a 20-min stall in CI). Close it so the + // script's top-level await can resolve and the process exits. + await close(); + } return createdDatabase; } @@ -339,6 +347,10 @@ if (import.meta.main) { "(e2e suites run against this, not DATABASE_URL's own database)", ); } + // Bounded exit: drain/close everything above, then terminate explicitly so + // a stray un-awaited connection pool cannot keep the process alive and + // stall CI for the full job timeout. + process.exit(0); } catch (error) { console.error(error instanceof Error ? error.message : String(error)); process.exit(1); From 8ca63fc874efbf8a082cf7f067794c7702abd74f Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 23:31:36 -0700 Subject: [PATCH 18/19] test(e2e): pin cleanupTenants against the workflow launch-spec RESTRICT teardown The install package seeder suite tears every tenant it creates down after run, but deleting a tenant used to trip on workflow_run_launch_spec's source_authority_principal_id (RESTRICT on principal): the launch spec has no tenant_id of its own, so cleanupTenants could not reach it and the tenant delete failed with 'violates RESTRICT setting of foreign key constraint workflow_run_launch_spec_source_authority_principal_id'. This suite seeds exactly that shape (tenant -> principal -> definition -> run -> launch spec) and asserts cleanupTenants removes both the launch spec and the tenant row, so the regression cannot return silently. --- e2e/install/tenant-cleanup.test.ts | 114 +++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 e2e/install/tenant-cleanup.test.ts diff --git a/e2e/install/tenant-cleanup.test.ts b/e2e/install/tenant-cleanup.test.ts new file mode 100644 index 0000000000..0b5d72f580 --- /dev/null +++ b/e2e/install/tenant-cleanup.test.ts @@ -0,0 +1,114 @@ +// Regression for the FK-teardown bug that broke install-package and every +// other hub-booting seeder suite: deleting a tenant used to trip on +// `workflow_run_launch_spec.source_authority_principal_id` (RESTRICT on +// `principal`). The cleanup's referencer-removal list was missing the launch +// spec — and its anchor run — so `delete from tenant` failed with +// "violates RESTRICT setting of foreign key constraint +// workflow_run_launch_spec_source_authority_principal_id". This suite seeds +// exactly that shape (tenant → principal → definition → run → launch spec) +// and asserts cleanupTenants removes it all and the tenant row. +import { afterAll, beforeAll, expect, test } from "bun:test"; +import { eq, inArray } from "drizzle-orm"; + +import { createDB, schema, type DB } from "@intx/db"; +import { generateId } from "@intx/hub-common"; +import { e2eDatabaseUrl } from "../lib/database-url"; +import { dbGate } from "../lib/db-gate"; +import { cleanupTenants } from "../lib/tenant-cleanup"; + +const databaseUrl = e2eDatabaseUrl(); +const describeIfDb = dbGate(databaseUrl, import.meta.path); + +function dbConfigFromUrl(url: string) { + const parsed = new URL(url); + return { + host: parsed.hostname, + port: parsed.port === "" ? 5432 : Number(parsed.port), + user: decodeURIComponent(parsed.username), + password: decodeURIComponent(parsed.password), + database: parsed.pathname.replace(/^\//, ""), + }; +} + +describeIfDb("cleanupTenants launches", () => { + let db: DB; + + const tenantId = generateId("tenant"); + const principalId = generateId("principal"); + const definitionId = generateId("workflowDefinition"); + const runId = generateId("workflowRun"); + const sessionId = generateId("session"); + const domain = `tenant-cleanup-${tenantId}.localhost`; + + beforeAll(async () => { + if (databaseUrl === undefined) return; + db = createDB(dbConfigFromUrl(databaseUrl)); + + await db.db.insert(schema.tenant).values({ + id: tenantId, + name: "Tenant Cleanup Test Tenant", + slug: `tenant-cleanup-${tenantId.replaceAll("_", "-")}`, + domain, + parentId: null, + config: null, + }); + await db.db.insert(schema.principal).values({ + id: principalId, + tenantId, + kind: "user", + refId: "usr_test", + status: "active", + }); + await db.db.insert(schema.workflowDefinition).values({ + id: definitionId, + tenantId, + creatorPrincipalId: principalId, + assetId: null, + name: "tenant-cleanup-test-definition", + }); + await db.db.insert(schema.workflowRun).values({ + id: runId, + definitionId, + anchorRunId: runId, + tenantId, + principalId, + address: `${runId}@${domain}`, + status: "deployed", + }); + // The exact row that used to wedge teardown: its + // `sourceAuthorityPrincipalId` RESTRICT-references the tenant's + // principal, and the launch spec has no `tenant_id` of its own, so it is + // only reachable from the tenant through the run. + await db.db.insert(schema.workflowRunLaunchSpec).values({ + anchorRunId: runId, + sessionId, + deploymentDomain: domain, + sourceAuthorityPrincipalId: principalId, + frozenApprovalBundle: {}, + sourceOfferingIds: [], + defaultSourceOfferingId: "off_test", + deployContent: { systemPrompt: "" }, + }); + }); + + afterAll(async () => { + if (databaseUrl === undefined) return; + await db.close(); + }); + + test("deletes launch spec, run, definition, principal, and the tenant row", async () => { + await cleanupTenants([tenantId]); + + const tenantRows = await db.db + .select() + .from(schema.tenant) + .where(eq(schema.tenant.id, tenantId)); + expect(tenantRows).toHaveLength(0); + + const launchSpecRows = await db.db + .select() + .from(schema.workflowRunLaunchSpec) + .where(inArray(schema.workflowRunLaunchSpec.anchorRunId, [runId])); + expect(launchSpecRows).toHaveLength(0); + }); +}); From dce29bcb475224925d059fcc0256641cc329df35 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Thu, 8 Oct 2026 23:31:39 -0700 Subject: [PATCH 19/19] fix(e2e): delete workflow launch specs before the tenant row in teardown cleanupTenants removed a tenant's restrict- and no-action-on-tenant rows before the tenant itself, but workflow_run_launch_spec references the tenant's principal with onDelete restrict while carrying no tenant_id of its own: it reaches the tenant only through its anchor run. Deleting the tenant tried to cascade the principal, which the launch spec's RESTRICT constraint blocked, so install-package and any seed suite that deploys failed its afterAll teardown with 'violates RESTRICT setting of foreign key constraint workflow_run_launch_spec_source_authority_principal_id'. Resolve the tenant's run ids, delete the launch specs they anchor, then the runs, before the tenant row's cascade reaches the principal. --- e2e/lib/tenant-cleanup.ts | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/e2e/lib/tenant-cleanup.ts b/e2e/lib/tenant-cleanup.ts index 82dcf77980..7a5ae81c92 100644 --- a/e2e/lib/tenant-cleanup.ts +++ b/e2e/lib/tenant-cleanup.ts @@ -6,12 +6,21 @@ // (principals, roles, grants, providers, credentials, catalog rows, workflow // definitions/assets/runs, sessions, approvals, oauth clients, ...), but a // few tables reference tenant with `onDelete: "restrict"` (sidecar_allocation, -// workflow_probe) or reference tenant with NO action (git_token), so those are -// deleted first while the tenant row is still present. +// workflow_probe) or reference tenant with NO action (git_token), and a +// workflow_run_launch_spec references the tenant's principal with +// `onDelete: "restrict"` despite having no tenant_id of its own, so all of +// those are deleted first while the tenant row is still present. import { inArray } from "drizzle-orm"; import { createDB } from "@intx/db"; -import { gitToken, sidecarAllocation, tenant as tenantTable, workflowProbe } from "@intx/db/schema"; +import { + gitToken, + sidecarAllocation, + tenant as tenantTable, + workflowProbe, + workflowRun, + workflowRunLaunchSpec, +} from "@intx/db/schema"; import { e2eDatabaseUrl } from "./database-url"; @@ -40,6 +49,25 @@ export async function cleanupTenants(tenantIds: readonly string[]): Promise 0) { + await db.delete(workflowRunLaunchSpec).where( + inArray( + workflowRunLaunchSpec.anchorRunId, + runIds.map((r) => r.id), + ), + ); + await db.delete(workflowRun).where(inArray(workflowRun.tenantId, tenantIds)); + } await db.delete(tenantTable).where(inArray(tenantTable.id, tenantIds)); } finally { await handle.close();