From d8cbbd4aefc5d4bfc15d15bea04233a5eb7ca885 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 23 Jul 2026 07:49:39 -0700 Subject: [PATCH] feat(control-plane): deliver a bootstrap secret into hosted tenant containers (#8202) Resolves push-vs-pull: a second stub.start() call from injectSecrets can't reliably deliver a live secret (Cloudflare Containers only apply envVars at an actual cold boot, and createContainer already owns the tenant's one real start() call). Instead, provisionTenant now runs database -> secrets -> container, so injectSecrets' one-time exchange secret (previously discarded) rides the container's own cold-boot envVars as a bootstrap credential; the container exchanges it for the real custodied value via the new fetchBrokeredStoredSecret client against the broker's already-wired stored-secret path. Scoped to the mechanism + ORB (which reuses its unmodified self-host broker-client code for free); AMS's container-side wiring is a real separate lift and follow-up issue #8246. --- control-plane/src/container-driver.ts | 27 ++++++++--- control-plane/src/index.ts | 1 + control-plane/src/provisioning.ts | 33 ++++++++----- control-plane/src/secret-driver.ts | 22 +++++---- .../src/tenant-provisioning-driver.ts | 31 ++++++++---- control-plane/test/container-driver.test.ts | 36 ++++++++++++++ control-plane/test/driver-factory.test.ts | 4 +- .../test/provisioning-pagerduty.test.ts | 8 +++- control-plane/test/provisioning.test.ts | 46 +++++++++++++++++- control-plane/test/secret-driver.test.ts | 6 +-- src/orb/broker-client.ts | 34 ++++++++++++++ test/integration/orb-broker.test.ts | 10 ++++ test/unit/orb-broker-client.test.ts | 47 +++++++++++++++++++ 13 files changed, 261 insertions(+), 44 deletions(-) diff --git a/control-plane/src/container-driver.ts b/control-plane/src/container-driver.ts index 1d6eb48e5f..66716ba2c1 100644 --- a/control-plane/src/container-driver.ts +++ b/control-plane/src/container-driver.ts @@ -67,16 +67,31 @@ function bindingFor(config: ContainerDriverConfig, product: Product): ContainerN * `pinnedVersion` rides into the container, whose entrypoint resolves the versioned artifact itself. */ export const PINNED_VERSION_ENV_VAR = "LOOPOVER_PINNED_VERSION"; +/** The env var a tenant's container reads its one-time secret-bootstrap credential from at cold boot (#8202). + * Deliberately product-agnostic (no `ORB_`/`AMS_` prefix), same reasoning as {@link PINNED_VERSION_ENV_VAR}: + * both `OrbTenantContainer` and `AmsTenantContainer` (#8246) read the identical name. The value itself is a + * one-time secret from `injectSecrets` (`TenantProvisioningRequest.bootstrapSecret`) the container exchanges + * via `POST /v1/orb/token` (`src/orb/broker-client.ts`'s `fetchBrokeredStoredSecret`) for whatever the broker + * actually has custodied -- this driver never sees or needs to know what that is. */ +export const TENANT_SECRET_ENV_VAR = "LOOPOVER_TENANT_SECRET_TOKEN"; + /** Idempotent: an already-provisioned tenant's container is left running as-is, never restarted -- a repeat - * create must not interrupt a container mid-work. A tenant with a `pinnedVersion` (#4898) starts with that - * version in {@link PINNED_VERSION_ENV_VAR}; an unpinned tenant gets the exact pre-#4898 `start()` call, so - * every existing tenant's behavior is byte-identical until a rollout pins it. */ + * create must not interrupt a container mid-work. This is also the ONLY point in a tenant's lifecycle where + * `envVars` actually reach the container (confirmed against the real `@cloudflare/containers` SDK: a `start()` + * call against an already-running/starting instance is a no-op or throws, never re-applies `envVars`) -- so + * both of the values below must already be known by the time this runs, not supplied later. A tenant with a + * `pinnedVersion` (#4898) starts with that version in {@link PINNED_VERSION_ENV_VAR}; one with a + * `bootstrapSecret` (#8202, set on `request` by `provisionTenant` from `injectSecrets`' result) starts with it + * in {@link TENANT_SECRET_ENV_VAR}; a tenant with neither gets the exact pre-#4898 `start()` call, so every + * existing tenant's behavior is byte-identical until either rollout applies. */ export async function createTenantContainer(config: ContainerDriverConfig, request: TenantProvisioningRequest): Promise { const stub = bindingFor(config, request.product).getByName(instanceNameFor(request)); if (await stub.isProvisioned()) return; - const pinnedVersion = request.tenant.pinnedVersion; - if (pinnedVersion) { - await stub.start({ envVars: { [PINNED_VERSION_ENV_VAR]: pinnedVersion } }); + const envVars: Record = {}; + if (request.tenant.pinnedVersion) envVars[PINNED_VERSION_ENV_VAR] = request.tenant.pinnedVersion; + if (request.bootstrapSecret) envVars[TENANT_SECRET_ENV_VAR] = request.bootstrapSecret; + if (Object.keys(envVars).length > 0) { + await stub.start({ envVars }); } else { await stub.start(); } diff --git a/control-plane/src/index.ts b/control-plane/src/index.ts index 6bf134eb57..45851d8530 100644 --- a/control-plane/src/index.ts +++ b/control-plane/src/index.ts @@ -65,6 +65,7 @@ export { destroyTenantContainer, instanceNameFor, PINNED_VERSION_ENV_VAR, + TENANT_SECRET_ENV_VAR, tenantContainerExists, type ContainerDriver, type ContainerDriverConfig, diff --git a/control-plane/src/provisioning.ts b/control-plane/src/provisioning.ts index 471616ede0..c057712556 100644 --- a/control-plane/src/provisioning.ts +++ b/control-plane/src/provisioning.ts @@ -1,8 +1,13 @@ // provisionTenant / deprovisionTenant orchestration (#7524) over the injectable `TenantProvisioningDriver`. // Product-agnostic: an ORB tenant and an AMS tenant take the identical call shape — `product` is forwarded to -// every driver step but never branched on. Provision runs #7180's three steps in order (create-container, -// provision-DB, inject-secrets); deprovision tears them down in REVERSE (revoke-secrets, drop-DB, -// destroy-container) so a secret is never left addressable after the DB/container it belonged to is gone. +// every driver step but never branched on. Provision runs #7180's three steps as provision-DB, inject-secrets, +// create-container (#8202 reordered this from the original create-container-first sequence: a tenant's +// bootstrap secret, produced by inject-secrets, must exist BEFORE create-container's one real `stub.start()` +// call, since Cloudflare Containers only ever apply `envVars` at a container's actual cold (re)start -- never +// as a live update to one already running or starting, confirmed against the real `@cloudflare/containers` SDK). +// Deprovision tears down in the order revoke-secrets, drop-DB, destroy-container -- REVERSE of the ORIGINAL +// #7180 order, kept deliberately unchanged by #8202's reorder: revoking a secret before the DB/container it +// belonged to is gone is the security property that matters here, not exact step-order symmetry with provision. // // #7667: a driver-step failure in EITHER direction also pages, via the same PagerDuty Events API v2 contract // ORB uses in `src/services/notify-pagerduty.ts` (see ./pagerduty-notify.ts for the mirrored contract and why @@ -81,13 +86,16 @@ function pageAndRethrow( throw error; } -/** Provision a tenant by running #7180's three steps in order against the injected driver. Product-agnostic: - * `product` is forwarded to every step, never branched on, so ORB and AMS share one call shape. `injectSecrets` - * is called with `database` already attached to the request (#8066) -- a real secret driver needs the - * connection details to actually store, not just the tenant identity every other step operates on. A step - * failure pages (#7667) and always rethrows — provisioning never fails silently. `onFailure` (#7677, - * optional) runs first in that failure path — the caller's seam for persisting the `"failed"` lifecycle - * state — and is best-effort: its own rejection is swallowed so it can never mask the step error. */ +/** Provision a tenant by running #7180's three steps against the injected driver, in the order database, secrets, + * container (#8202 -- see this module's header for why). Product-agnostic: `product` is forwarded to every step, + * never branched on, so ORB and AMS share one call shape. `injectSecrets` is called with `database` already + * attached to the request (#8066) -- a real secret driver needs the connection details to actually store, not + * just the tenant identity every other step operates on. `createContainer` is in turn called with `database` + * still attached AND `bootstrapSecret` newly attached (#8202) whenever `injectSecrets` returned one -- a real + * container driver delivers it into the container's own cold-boot environment. A step failure pages (#7667) and + * always rethrows — provisioning never fails silently. `onFailure` (#7677, optional) runs first in that failure + * path — the caller's seam for persisting the `"failed"` lifecycle state — and is best-effort: its own + * rejection is swallowed so it can never mask the step error. */ export async function provisionTenant( tenant: Tenant, product: Product, @@ -99,9 +107,10 @@ export async function provisionTenant( let database: DatabaseConnectionDetails; let secretRef: string | undefined; try { - await driver.createContainer(request); database = await driver.provisionDatabase(request); - ({ secretRef } = await driver.injectSecrets({ ...request, database })); + const injected = await driver.injectSecrets({ ...request, database }); + secretRef = injected.secretRef; + await driver.createContainer({ ...request, database, ...(injected.bootstrapSecret !== undefined ? { bootstrapSecret: injected.bootstrapSecret } : {}) }); } catch (error) { // #7677 (ratified 2026-07-21): give the caller its chance to transition the tenant's registry record to // "failed" BEFORE the rethrow, so a customer polling the read path sees a terminal "Setup failed" instead diff --git a/control-plane/src/secret-driver.ts b/control-plane/src/secret-driver.ts index 70fda7bc2e..2f095ba63b 100644 --- a/control-plane/src/secret-driver.ts +++ b/control-plane/src/secret-driver.ts @@ -9,10 +9,12 @@ // calls the SAME two routes, just to STORE a tenant's DB credential rather than mint a GitHub token (#8064's // `tenant_db_credential` secret type), plus a third route (#8064) to revoke it on teardown. // -// Scope, deliberately narrow: this ONLY stores/revokes custody of the credential in the broker. It does NOT -// deliver the secret into a running container's environment -- that's separate, not-yet-built infrastructure -// (a container's own bootstrap would need to independently exchange its own enrollment secret, the same way a -// self-hosted container already does against `/v1/orb/token` today). #8066's own boundary excludes it. +// Scope, deliberately narrow: this ONLY stores custody of the credential in the broker and hands back the +// one-time exchange secret as `bootstrapSecret` -- it does NOT itself deliver anything into a running +// container's environment. That delivery is provisioning.ts's + container-driver.ts's job (#8202): provisioning +// threads `bootstrapSecret` from this driver's `injectSecrets` result into the SAME tenant's `createContainer` +// call, which is where it actually reaches `stub.start({envVars})`. #8066's own boundary excluded delivery +// entirely; #8202 is precisely the "separate, not-yet-built infrastructure" that comment pointed at. // // Deliberately does NOT implement the full `TenantProvisioningDriver` interface -- only injectSecrets/ // revokeSecrets (see `SecretDriver` below). `withRealSecretDriver` (driver-factory.ts) composes this onto an @@ -42,7 +44,7 @@ export type SecretDriverConfig = { /** The secret-only slice of `TenantProvisioningDriver` this module actually implements. Composed onto a full * driver by `withRealSecretDriver` (driver-factory.ts), never used standalone against `provisionTenant`. */ export type SecretDriver = { - injectSecrets(request: TenantProvisioningRequest): Promise<{ secretRef?: string }>; + injectSecrets(request: TenantProvisioningRequest): Promise<{ secretRef?: string; bootstrapSecret?: string }>; revokeSecrets(request: TenantProvisioningRequest): Promise; }; @@ -71,9 +73,11 @@ async function mainAppFetch(config: SecretDriverConfig, method: string, path: * object is stored (JSON-encoded), not just the bare `connectionString` -- a later reader gets every field * back, not just what it can re-parse out of a URI, mirroring that type's own "kept alongside the parts" * rationale. Returns the enrollment's `enrollId` as this driver's `secretRef` -- the caller (`provisionTenant`, - * via its own result) must persist this to revoke it later; the one-time exchange `secret` is intentionally - * discarded here, since this driver's job ends at custody, not consumption (see this file's header comment). */ -export async function injectTenantSecrets(config: SecretDriverConfig, request: TenantProvisioningRequest): Promise<{ secretRef?: string }> { + * via its own result) must persist this to revoke it later -- AND the one-time exchange `secret` as + * `bootstrapSecret` (#8202): the caller threads this into the tenant's container at its next `createContainer` + * call, so the container can itself present it to `/v1/orb/token` and get this exact value back. Previously + * discarded here (see this file's former header comment); #8202 is what actually consumes it now. */ +export async function injectTenantSecrets(config: SecretDriverConfig, request: TenantProvisioningRequest): Promise<{ secretRef?: string; bootstrapSecret?: string }> { if (!request.database) { throw new Error(`injectTenantSecrets: no database connection details on the request for tenant "${request.tenant.name}"`); } @@ -86,7 +90,7 @@ export async function injectTenantSecrets(config: SecretDriverConfig, request: T "/v1/internal/orb/enrollments", { secretType: SECRET_TYPE_TENANT_DB_CREDENTIAL, secretValue: JSON.stringify(request.database) }, ); - return { secretRef: result.enrollId }; + return { secretRef: result.enrollId, bootstrapSecret: result.secret }; } /** Idempotent teardown: a request with no `secretRef` (never provisioned with a real secret driver, or already diff --git a/control-plane/src/tenant-provisioning-driver.ts b/control-plane/src/tenant-provisioning-driver.ts index 40eee56f10..3bd1bd3d0f 100644 --- a/control-plane/src/tenant-provisioning-driver.ts +++ b/control-plane/src/tenant-provisioning-driver.ts @@ -41,15 +41,21 @@ export type TenantLifecycleState = export type TenantProvisioningRequest = { tenant: Tenant; product: Product; - /** The tenant's already-provisioned database connection details (#7653) -- populated ONLY for the - * `injectSecrets` call, by `provisionTenant`'s own orchestration right after `provisionDatabase` resolves - * (#8066). Every other step (createContainer, destroyContainer, etc.) never sees this field. */ + /** The tenant's already-provisioned database connection details (#7653) -- populated for the `injectSecrets` + * call (and, from there on, every step after it -- see `createContainer` below) by `provisionTenant`'s own + * orchestration right after `provisionDatabase` resolves (#8066). */ database?: DatabaseConnectionDetails; /** An opaque, driver-specific reference to a previously injected secret (#8066) -- whatever `injectSecrets` * returned as `secretRef`, threaded back in by `deprovisionTenant` so `revokeSecrets` knows what to revoke. * Absent when a tenant was never provisioned with a real secret driver configured (idempotent revoke of an * unconfigured tenant, matching every other driver's teardown contract). */ secretRef?: string; + /** A one-time credential the tenant's OWN container can later exchange for a real custodied secret (#8202) -- + * whatever `injectSecrets` returned as `bootstrapSecret`, threaded by `provisionTenant` into the SAME + * `createContainer` call that follows it (#8202 reordered provisioning so this is possible -- see + * provisioning.ts). Populated ONLY for that one `createContainer` call; no other step ever sees it, and it is + * never itself the delivered secret -- just the key the container uses to fetch one. */ + bootstrapSecret?: string; }; /** What `provisionDatabase` hands back (#7653): everything a caller needs to actually reach the tenant's @@ -69,7 +75,12 @@ export type DatabaseConnectionDetails = { }; export interface TenantProvisioningDriver { - /** Step 1 (#7180): stand up the tenant's isolated container. Real driver → Cloudflare Containers API. */ + /** Step 1 in call order (#7180), but the LAST of the three to run within `provisionTenant` as of #8202: stand + * up the tenant's isolated container. Real driver → Cloudflare Containers API. May see `request.bootstrapSecret` + * (#8202, set when `injectSecrets` returned one) to deliver into the container's own process environment at + * this, its actual cold-boot `start()` call -- the only point in a container's lifecycle Cloudflare Containers + * actually applies `envVars` (confirmed against the real `@cloudflare/containers` SDK: a repeat `start()` on + * an already-running/starting instance is a no-op or throws, never re-applies `envVars`). */ createContainer(request: TenantProvisioningRequest): Promise; /** Step 2 (#7180): provision the tenant's database, returning its connection details (#7653) -- a freshly * created role's password is typically retrievable from the provider only at creation time, so the caller @@ -78,10 +89,14 @@ export interface TenantProvisioningDriver { provisionDatabase(request: TenantProvisioningRequest): Promise; /** Step 3 (#7180): inject the tenant's secrets, given its database connection details (`request.database`, * #8066). Returns an opaque `secretRef` the caller must persist and thread back into a later `revokeSecrets` - * call via `request.secretRef` -- `undefined` when the driver has nothing to track (e.g. the fake). A real - * driver delegates to #7174's generalized broker (src/orb/broker.ts, via #8064's stored-secret type); the - * fake only records the call. */ - injectSecrets(request: TenantProvisioningRequest): Promise<{ secretRef?: string }>; + * call via `request.secretRef` -- `undefined` when the driver has nothing to track (e.g. the fake). Also + * returns `bootstrapSecret` (#8202): a one-time credential the caller threads into the SAME tenant's next + * `createContainer` call (provisioning.ts runs this step BEFORE createContainer specifically so this is + * possible), so the running container can itself exchange it later for the real secret this step just + * custodied -- `undefined` when the driver has nothing for a container to bootstrap with. A real driver + * delegates to #7174's generalized broker (src/orb/broker.ts, via #8064's stored-secret type); the fake only + * records the call. */ + injectSecrets(request: TenantProvisioningRequest): Promise<{ secretRef?: string; bootstrapSecret?: string }>; /** Teardown inverse of createContainer. MUST be idempotent — safe to call when the container was never * created — so deprovisioning a nonexistent tenant is a no-op, never a throw. */ destroyContainer(request: TenantProvisioningRequest): Promise; diff --git a/control-plane/test/container-driver.test.ts b/control-plane/test/container-driver.test.ts index eca723b987..8b460f7ca9 100644 --- a/control-plane/test/container-driver.test.ts +++ b/control-plane/test/container-driver.test.ts @@ -9,6 +9,7 @@ import { createTenantContainer, destroyTenantContainer, PINNED_VERSION_ENV_VAR, + TENANT_SECRET_ENV_VAR, tenantContainerExists, type ContainerDriverConfig, type ContainerNamespaceLike, @@ -188,3 +189,38 @@ test("a repeat create of an already-provisioned pinned tenant never restarts it assert.deepEqual(stub.startOptions, []); }); + +// #8202: a tenant's one-time secret-bootstrap credential rides into its container at cold boot the same way +// pinnedVersion does above -- the only point in a container's lifecycle envVars are actually applied. +test("a tenant with a bootstrap secret starts with TENANT_SECRET_ENV_VAR carrying it", async () => { + const stub = optionCapturingStub(); + + await createTenantContainer(configFor(stub), { tenant: { name: "acme" }, product: "orb", bootstrapSecret: "orbsec_xyz" }); + + assert.deepEqual(stub.startOptions, [{ envVars: { [TENANT_SECRET_ENV_VAR]: "orbsec_xyz" } }]); +}); + +test("a tenant with both a pinned version and a bootstrap secret starts with both env vars merged into one call", async () => { + const stub = optionCapturingStub(); + + await createTenantContainer(configFor(stub), { tenant: { name: "acme", pinnedVersion: "v1.4.2" }, product: "orb", bootstrapSecret: "orbsec_xyz" }); + + assert.deepEqual(stub.startOptions, [{ envVars: { [PINNED_VERSION_ENV_VAR]: "v1.4.2", [TENANT_SECRET_ENV_VAR]: "orbsec_xyz" } }]); +}); + +test("a tenant with neither a pinned version nor a bootstrap secret still gets the exact pre-#4898 call (no options at all)", async () => { + const stub = optionCapturingStub(); + + await createTenantContainer(configFor(stub), { tenant: { name: "acme" }, product: "orb", bootstrapSecret: undefined }); + + assert.deepEqual(stub.startOptions, [undefined]); +}); + +test("a repeat create of an already-provisioned tenant with a bootstrap secret never restarts it (idempotence contract holds here too)", async () => { + const stub = optionCapturingStub(); + await stub.markProvisioned(); + + await createTenantContainer(configFor(stub), { tenant: { name: "acme" }, product: "orb", bootstrapSecret: "orbsec_xyz" }); + + assert.deepEqual(stub.startOptions, []); +}); diff --git a/control-plane/test/driver-factory.test.ts b/control-plane/test/driver-factory.test.ts index a91c93fc83..a75942ee2a 100644 --- a/control-plane/test/driver-factory.test.ts +++ b/control-plane/test/driver-factory.test.ts @@ -251,7 +251,7 @@ test("createTenantProvisioningDriver: selects the real secret driver when both M const driver = createTenantProvisioningDriver({ MAIN_APP_BASE_URL: "https://api.loopover.test", INTERNAL_JOB_TOKEN: "internal-test-token" }); const result = await driver.injectSecrets({ ...REQUEST, database: { host: "h", port: 5432, database: "d", user: "u", password: "p", connectionString: "postgres://u:p@h:5432/d" } }); - assert.deepEqual(result, { secretRef: "orbenr_abc" }); + assert.deepEqual(result, { secretRef: "orbenr_abc", bootstrapSecret: "orbsec_xyz" }); assert.ok(calls.some((url) => url.includes("api.loopover.test"))); }); @@ -270,7 +270,7 @@ test("createTenantProvisioningDriver: composes the real database, container, AND assert.equal(await driver.containerExists(REQUEST), true); await assert.rejects(driver.provisionDatabase(REQUEST)); const injected = await driver.injectSecrets({ ...REQUEST, database: { host: "h", port: 5432, database: "d", user: "u", password: "p", connectionString: "postgres://u:p@h:5432/d" } }); - assert.deepEqual(injected, { secretRef: "orbenr_abc" }); + assert.deepEqual(injected, { secretRef: "orbenr_abc", bootstrapSecret: "orbsec_xyz" }); }); test("createTenantProvisioningDriver: defaults env to process.env when no override is passed", async () => { diff --git a/control-plane/test/provisioning-pagerduty.test.ts b/control-plane/test/provisioning-pagerduty.test.ts index e935e32973..7a5b2f6eef 100644 --- a/control-plane/test/provisioning-pagerduty.test.ts +++ b/control-plane/test/provisioning-pagerduty.test.ts @@ -20,13 +20,17 @@ function driverThatThrowsOn( error: Error, ): TenantProvisioningDriver { const noop = async (): Promise => {}; - const failing = async (): Promise => { + const failing = async (): Promise => { throw error; }; + // #8202: injectSecrets' real return type is destructured by provisionTenant (secretRef, bootstrapSecret), so + // its own "successfully did nothing" stand-in must return a real (empty) object, not void -- unlike every + // other step here, which provisionTenant/deprovisionTenant only ever await, never read a value from. + const noopInjectSecrets = async (): Promise<{ secretRef?: string; bootstrapSecret?: string }> => ({}); return { createContainer: step === "createContainer" ? failing : noop, provisionDatabase: step === "provisionDatabase" ? failing : noop, - injectSecrets: step === "injectSecrets" ? failing : noop, + injectSecrets: step === "injectSecrets" ? failing : noopInjectSecrets, destroyContainer: step === "destroyContainer" ? failing : noop, dropDatabase: step === "dropDatabase" ? failing : noop, revokeSecrets: step === "revokeSecrets" ? failing : noop, diff --git a/control-plane/test/provisioning.test.ts b/control-plane/test/provisioning.test.ts index 090d51bbed..6fe1006b77 100644 --- a/control-plane/test/provisioning.test.ts +++ b/control-plane/test/provisioning.test.ts @@ -32,10 +32,11 @@ test("provisionTenant runs the three #7180 steps in order and reports the tenant connectionString: "postgres://acme:fake-password-acme@fake-acme.control-plane.invalid:5432/acme", }, }); - // create-container → provision-DB → inject-secrets, in that order. + // provision-DB → inject-secrets → create-container, in that order (#8202: reordered from the original + // create-container-first sequence so a tenant's bootstrap secret exists before its one real start() call). assert.deepEqual( driver.calls.map((call) => call.step), - ["createContainer", "provisionDatabase", "injectSecrets"], + ["provisionDatabase", "injectSecrets", "createContainer"], ); // Container "exists"/reachable via the fake after provision. assert.equal(await driver.containerExists({ tenant, product: "orb" }), true); @@ -106,6 +107,47 @@ test("#8066: provisionTenant attaches the freshly provisioned database to the in assert.equal(result.secretRef, "orbenr_abc"); }); +test("#8202: provisionTenant threads injectSecrets' bootstrapSecret into the createContainer request", async () => { + const fake = createFakeTenantProvisioningDriver(); + let seenRequest: TenantProvisioningRequest | undefined; + const driver: TenantProvisioningDriver = { + ...fake, + injectSecrets: async () => ({ secretRef: "orbenr_abc", bootstrapSecret: "orbsec_xyz" }), + createContainer: async (request) => { + seenRequest = request; + }, + }; + const tenant: Tenant = { name: "acme" }; + + await provisionTenant(tenant, "orb", driver); + + assert.equal(seenRequest?.bootstrapSecret, "orbsec_xyz"); + assert.deepEqual(seenRequest?.database, { + host: "fake-acme.control-plane.invalid", + port: 5432, + database: "acme", + user: "acme", + password: "fake-password-acme", + connectionString: "postgres://acme:fake-password-acme@fake-acme.control-plane.invalid:5432/acme", + }); +}); + +test("#8202: provisionTenant's createContainer request omits bootstrapSecret entirely when injectSecrets returns none (the fake's own behavior)", async () => { + const fake = createFakeTenantProvisioningDriver(); + let seenRequest: TenantProvisioningRequest | undefined; + const driver: TenantProvisioningDriver = { + ...fake, + createContainer: async (request) => { + seenRequest = request; + }, + }; + const tenant: Tenant = { name: "acme" }; + + await provisionTenant(tenant, "orb", driver); + + assert.equal("bootstrapSecret" in (seenRequest ?? {}), false); +}); + test("#8066: provisionTenant's result omits secretRef entirely when the driver returns none (the fake's own behavior)", async () => { const driver = createFakeTenantProvisioningDriver(); const tenant: Tenant = { name: "acme" }; diff --git a/control-plane/test/secret-driver.test.ts b/control-plane/test/secret-driver.test.ts index ba047ac81d..4a5afbed95 100644 --- a/control-plane/test/secret-driver.test.ts +++ b/control-plane/test/secret-driver.test.ts @@ -31,12 +31,12 @@ function config(fetchImpl: typeof fetch): SecretDriverConfig { return { baseUrl: "https://api.loopover.test", internalJobToken: "internal-test-token", fetchImpl }; } -test("injectTenantSecrets: stores the WHOLE database connection details JSON-encoded, returns enrollId as secretRef", async () => { +test("injectTenantSecrets: stores the WHOLE database connection details JSON-encoded, returns enrollId as secretRef and the one-time secret as bootstrapSecret", async () => { const { fetchImpl, calls } = fakeFetch(() => Response.json({ enrollId: "orbenr_abc", secret: "orbsec_xyz" }, { status: 200 })); const result = await injectTenantSecrets(config(fetchImpl), REQUEST); - assert.deepEqual(result, { secretRef: "orbenr_abc" }); + assert.deepEqual(result, { secretRef: "orbenr_abc", bootstrapSecret: "orbsec_xyz" }); assert.equal(calls.length, 1); assert.equal(calls[0]!.url, "https://api.loopover.test/v1/internal/orb/enrollments"); assert.equal(calls[0]!.init.method, "POST"); @@ -101,7 +101,7 @@ test("createSecretDriver: bundles injectTenantSecrets/revokeTenantSecrets closed const driver = createSecretDriver(config(fetchImpl)); const injected = await driver.injectSecrets(REQUEST); - assert.deepEqual(injected, { secretRef: "orbenr_abc" }); + assert.deepEqual(injected, { secretRef: "orbenr_abc", bootstrapSecret: "orbsec_xyz" }); await driver.revokeSecrets({ ...REQUEST, secretRef: injected.secretRef }); assert.equal(calls.length, 2); diff --git a/src/orb/broker-client.ts b/src/orb/broker-client.ts index 8da1a45780..dcc4b7d53a 100644 --- a/src/orb/broker-client.ts +++ b/src/orb/broker-client.ts @@ -85,6 +85,40 @@ export async function fetchBrokeredInstallationToken( return { token: payload.token, installationId: payload.installationId ?? 0, expiresAtMs, permissions: payload.permissions ?? {} }; } +export type BrokeredStoredSecret = { secretValue: string; secretType: string }; + +/** Exchange a tenant's one-time bootstrap credential (#8202, `LOOPOVER_TENANT_SECRET_TOKEN` -- delivered into a + * hosted tenant container's own process env at its cold boot, via `control-plane/src/container-driver.ts`'s + * `createTenantContainer`) for whatever secret the broker actually has custodied under it, e.g. a Neon database + * connection string (`ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL`, `src/orb/broker.ts`). Same endpoint as + * {@link fetchBrokeredInstallationToken} (`POST /v1/orb/token`) -- the server disambiguates by the enrollment + * row's own `secret_type`, not by anything the caller specifies, so a distinct client function exists only to + * parse the OTHER half of `BrokerResult`'s union (`{secretValue, secretType}` instead of `{token, ...}`), not + * because the wire call itself differs. No cache/TTL concept here (unlike the installation-token path) -- a + * stored secret's value is fixed at issue time, so every call is a fresh exchange; a caller wanting to avoid + * repeat network calls should cache the RESULT itself, not rely on this function to. Throws on a non-OK + * response or a body missing `secretValue` -- a container with no other way to reach its own secret has + * nothing safe to fall back to, exactly like the installation-token path's own fatal-on-failure posture. */ +export async function fetchBrokeredStoredSecret( + env: { LOOPOVER_TENANT_SECRET_TOKEN?: string | undefined; ORB_BROKER_URL?: string | undefined }, + fetchImpl: typeof fetch = fetch, +): Promise { + const base = orbBrokerBaseUrl(env); + const response = await fetchImpl(`${base}/v1/orb/token`, { + method: "POST", + headers: { authorization: `Bearer ${env.LOOPOVER_TENANT_SECRET_TOKEN ?? ""}` }, + signal: AbortSignal.timeout(BROKER_TIMEOUT_MS), + }); + if (!response.ok) { + throw new Error(`Orb broker stored-secret exchange failed (${response.status}).`); + } + const payload = (await response.json()) as { secretValue?: string; secretType?: string }; + if (!payload.secretValue) { + throw new Error("Orb broker stored-secret response did not include a secretValue."); + } + return { secretValue: payload.secretValue, secretType: payload.secretType ?? "" }; +} + // Diagnosing a broker register failure (#selfhost-runtime-drift) needs more than a bare status code, but the // response body is attacker/operator-adjacent (the broker, or anything on-path to it) and must never be logged // verbatim. Only a short, structured hint is ever surfaced: a JSON body's own `error`/`message` string field, diff --git a/test/integration/orb-broker.test.ts b/test/integration/orb-broker.test.ts index dbc3a35cad..f01c4f4333 100644 --- a/test/integration/orb-broker.test.ts +++ b/test/integration/orb-broker.test.ts @@ -365,6 +365,16 @@ describe("brokerOrbToken", () => { expect((await db(e).prepare("SELECT last_token_at FROM orb_enrollments WHERE secret_hash = ?").bind(await hashToken(secret)).first<{ last_token_at: string | null }>())?.last_token_at).not.toBeNull(); }); + it("#8202: a revoked tenant-db-credential enrollment can no longer be exchanged -- revocation actually removes access, not just broker custody", async () => { + const e = await brokerEnv({ TOKEN_ENCRYPTION_SECRET: "orb-stored-secret-test" }); + const { enrollId, secret } = (await issueOrbStoredSecret(e, ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL, "postgres://tenant-acme:hunter2@neon/acme")) as { enrollId: string; secret: string }; + expect(await brokerOrbToken(e, secret)).toEqual({ secretValue: "postgres://tenant-acme:hunter2@neon/acme", secretType: ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL }); + + expect(await revokeOrbEnrollment(e, enrollId)).toEqual({ revoked: true }); + + expect(await brokerOrbToken(e, secret)).toEqual({ error: "invalid_enrollment" }); + }); + it("#8064: refuses to serve a stored secret with no TOKEN_ENCRYPTION_SECRET configured at exchange time", async () => { const e = await brokerEnv({ TOKEN_ENCRYPTION_SECRET: "orb-stored-secret-test" }); const { secret } = (await issueOrbStoredSecret(e, ORB_SECRET_TYPE_TENANT_DB_CREDENTIAL, "postgres://tenant-acme")) as { secret: string }; diff --git a/test/unit/orb-broker-client.test.ts b/test/unit/orb-broker-client.test.ts index 392780a1bd..57ed202211 100644 --- a/test/unit/orb-broker-client.test.ts +++ b/test/unit/orb-broker-client.test.ts @@ -4,6 +4,7 @@ import { createOrbRelayRegistrationState, drainOrbRelay, fetchBrokeredInstallationToken, + fetchBrokeredStoredSecret, isOrbBrokerMode, ORB_RELAY_REGISTER_RETRY_BACKOFF_MS, ORB_RELAY_REGISTER_UNHEALTHY_FAILURE_STREAK, @@ -114,6 +115,52 @@ describe("fetchBrokeredInstallationToken", () => { }); }); +describe("fetchBrokeredStoredSecret (#8202)", () => { + it("exchanges the tenant secret token for a stored secret (default broker URL + Bearer token)", async () => { + const { fetchImpl, calls } = captureFetch(Response.json({ secretValue: "postgres://tenant-acme:hunter2@neon/acme", secretType: "tenant_db_credential" })); + const out = await fetchBrokeredStoredSecret({ LOOPOVER_TENANT_SECRET_TOKEN: "orbsec_x" }, fetchImpl); + expect(out).toEqual({ secretValue: "postgres://tenant-acme:hunter2@neon/acme", secretType: "tenant_db_credential" }); + expect(calls[0]?.url).toBe("https://api.loopover.ai/v1/orb/token"); + expect((calls[0]?.init?.headers as Record).authorization).toBe("Bearer orbsec_x"); + expect(calls[0]?.init?.method).toBe("POST"); + }); + + it("respects a custom ORB_BROKER_URL, same as fetchBrokeredInstallationToken", async () => { + const { fetchImpl, calls } = captureFetch(Response.json({ secretValue: "v", secretType: "tenant_db_credential" })); + await fetchBrokeredStoredSecret({ LOOPOVER_TENANT_SECRET_TOKEN: "s", ORB_BROKER_URL: "https://broker.example/" }, fetchImpl); + expect(calls[0]?.url).toBe("https://broker.example/v1/orb/token"); + }); + + it("rejects unsafe broker URLs via the same shared validation fetchBrokeredInstallationToken uses", async () => { + const fetchImpl = (async () => { + throw new Error("fetch should not be called for an unsafe broker URL"); + }) as typeof fetch; + await expect(fetchBrokeredStoredSecret({ LOOPOVER_TENANT_SECRET_TOKEN: "s", ORB_BROKER_URL: "http://broker.example" }, fetchImpl)).rejects.toThrow(/must use https/); + }); + + it("sends an empty Bearer when no token is set (defensive ?? branch)", async () => { + const { fetchImpl, calls } = captureFetch(Response.json({ secretValue: "v", secretType: "t" })); + await fetchBrokeredStoredSecret({}, fetchImpl); + expect((calls[0]?.init?.headers as Record).authorization).toBe("Bearer "); + }); + + it("defaults secretType to an empty string when the broker response omits it (defensive ?? branch)", async () => { + const { fetchImpl } = captureFetch(Response.json({ secretValue: "v" })); + const out = await fetchBrokeredStoredSecret({ LOOPOVER_TENANT_SECRET_TOKEN: "s" }, fetchImpl); + expect(out).toEqual({ secretValue: "v", secretType: "" }); + }); + + it("throws on a non-OK broker response (e.g. 401 invalid_enrollment)", async () => { + const fetchImpl = (async () => new Response("nope", { status: 401 })) as typeof fetch; + await expect(fetchBrokeredStoredSecret({ LOOPOVER_TENANT_SECRET_TOKEN: "s" }, fetchImpl)).rejects.toThrow(/401/); + }); + + it("throws when the broker response has no secretValue", async () => { + const fetchImpl = (async () => Response.json({ secretType: "tenant_db_credential" })) as typeof fetch; + await expect(fetchBrokeredStoredSecret({ LOOPOVER_TENANT_SECRET_TOKEN: "s" }, fetchImpl)).rejects.toThrow(/did not include a secretValue/); + }); +}); + describe("registerOrbRelayTarget", () => { it("skips unless broker mode AND a public origin are configured", async () => { expect(await registerOrbRelayTarget({})).toEqual({ status: "skipped" }); // not broker mode