diff --git a/packages/core/src/fakes/sandbox-fake.ts b/packages/core/src/fakes/sandbox-fake.ts index e927d38..0a8dfa0 100644 --- a/packages/core/src/fakes/sandbox-fake.ts +++ b/packages/core/src/fakes/sandbox-fake.ts @@ -170,7 +170,11 @@ export const makeSandboxFake = ( ? Effect.fail( new ExecTimeout({ timeoutSec: canned.timeoutSec ?? opts.timeoutSec ?? 600, - command, + // Scrubbed, as the live layer does. `ExecTimeout.message` + // inlines the command and Workflows persists it, so a fake that + // left it raw would let a run-level test pass on a property the + // live layer establishes and the fake contradicts. + command: redact(command, opts.redactValues), }), ) : Effect.fail( diff --git a/packages/core/src/services/sandbox.ts b/packages/core/src/services/sandbox.ts index 9e7549e..e543211 100644 --- a/packages/core/src/services/sandbox.ts +++ b/packages/core/src/services/sandbox.ts @@ -47,8 +47,11 @@ export type ExecOpts = { readonly container?: Container; /** * Plaintext values (typically resolved Worker-secret values injected via - * `env`) to scrub from the captured stdout/stderr before EITHER the inline - * `ExecResult` tail or the full log streamed to R2 is persisted. A command + * `env`) to scrub from every durable surface this exec produces: the inline + * `ExecResult` tail, the full log streamed to R2, the COMMAND recorded in + * that log's meta line, and `ExecTimeout.command` (which Workflows persists + * as the attempt record). An implementation that scrubs only the streams does + * not satisfy this. A command * that echoes an injected credential — deliberately or via a misbehaving * tool — must not leak it through the log artifact's signed URL. Exact- * substring match, each occurrence replaced with `"***"`. Empty/omitted is diff --git a/packages/runtime-cf/src/sandbox-cf.test.ts b/packages/runtime-cf/src/sandbox-cf.test.ts index 9bb920f..4af2d62 100644 --- a/packages/runtime-cf/src/sandbox-cf.test.ts +++ b/packages/runtime-cf/src/sandbox-cf.test.ts @@ -570,6 +570,59 @@ describe("makeSandboxCloudflareLive — exec result folding (D)", () => { }), ); + it.effect("the COMMAND is scrubbed before it lands in the R2 meta line", () => + Effect.gen(function* () { + // `writeLog` writes `{stream:"meta", command}` into the durable log. A + // command that inlines a credential reached R2 in the clear, beside the + // streams that had just been scrubbed. + currentBox = makeFakeBox({ proc: null }); + currentBox.exec = vi.fn(async () => ({ + exitCode: 0, + duration: 1, + stdout: "", + stderr: "", + })); + const { bucket, puts } = makeBucket(); + const layer = makeSandboxCloudflareLive(ns, bucket, "exec-1"); + yield* Effect.flatMap(SandboxTag, (s) => + s.exec({ + command: 'curl -H "Authorization: Bearer tok-abc123" https://api.example.com', + cwd: "/w", + env: {}, + redactValues: ["tok-abc123"], + }), + ).pipe(Effect.provide(layer)); + const logBody = puts.map((p) => String(p.body)).join(""); + expect(logBody).not.toContain("tok-abc123"); + expect(logBody).toContain("***"); + }), + ); + + it.effect("a timed-out exec does not carry its raw command into the Workflow record", () => + Effect.gen(function* () { + // `ExecTimeout.message` inlines the command, and Workflows persists that + // as the attempt record — a second durable surface beside the R2 log. + currentBox = makeFakeBox({ proc: null }); + currentBox.exec = vi.fn(async () => { + throw new Error("Command timeout after 30000ms"); + }); + const { bucket } = makeBucket(); + const layer = makeSandboxCloudflareLive(ns, bucket, "exec-1"); + const exit = yield* Effect.flatMap(SandboxTag, (s) => + s.exec({ + command: 'curl -H "Authorization: Bearer tok-abc123" https://api.example.com', + cwd: "/w", + env: {}, + timeoutSec: 30, + redactValues: ["tok-abc123"], + }), + ).pipe(Effect.provide(layer), Effect.exit); + const failure = failureOf(exit); + expect(failure?._tag).toBe("ExecTimeout"); + expect(JSON.stringify(failure)).not.toContain("tok-abc123"); + }), + ); + it.effect("redacts before truncating, so a secret on the 4KB cut leaves no fragment", () => Effect.gen(function* () { const secret = "SECRET_TOKEN_VALUE"; diff --git a/packages/runtime-cf/src/sandbox-cf.ts b/packages/runtime-cf/src/sandbox-cf.ts index ae6e378..26afeb9 100644 --- a/packages/runtime-cf/src/sandbox-cf.ts +++ b/packages/runtime-cf/src/sandbox-cf.ts @@ -667,7 +667,12 @@ export const makeSandboxCloudflareLive = ( // FULL output → R2 (the durable log the artifact step promotes). Kept // even on the working-dir-missing path below, so the failure is // diagnosable from the log viewer. - await writeLog(logPath, cmd, stdout, stderr); + // The command rides the meta line into R2, so it is a persisted + // surface too. Scrubbing the streams beside it and leaving it in the + // clear only holds while no command inlines a credential, and + // `curl -H "Authorization: Bearer …"` is an ordinary thing for a + // consumer's own CI to run (adr/0006 § never-log list). + await writeLog(logPath, redact(cmd, redactValues), stdout, stderr); // A command whose shell could not even enter its working directory // never ran — the checkout did not survive to this exec (container // recycled between durable steps). Raise a real ExecFailed rather than @@ -700,9 +705,12 @@ export const makeSandboxCloudflareLive = ( // what Workflows persists via ExecFailed.message (#88). const message = cause instanceof Error ? cause.message : String(cause); if (/timed?\s*out|timeout/i.test(message)) { + // `ExecTimeout.message` inlines the command and Workflows persists + // that as the attempt record — a second durable surface, so it + // takes the same scrub as the R2 log. return new ExecTimeout({ timeoutSec: timeoutSec ?? 0, - command: cmd, + command: redact(cmd, redactValues), }); } return new ExecFailed({ diff --git a/packages/runtime-cf/src/sandbox-facade.ts b/packages/runtime-cf/src/sandbox-facade.ts index e4064bf..cf6a930 100644 --- a/packages/runtime-cf/src/sandbox-facade.ts +++ b/packages/runtime-cf/src/sandbox-facade.ts @@ -343,7 +343,11 @@ export const makeSandboxFacadeLive = (opts: SandboxFacadeOptions): Layer.Layer { }, ); + it.effect("does NOT dispatch self-heal when the failing command inlines a secret value", () => { + // Neither version of such a command can honestly reach the healer: raw, it + // smuggles a live credential into the credential-free agent sandbox and + // into a pack an injection-steerable LLM reads; scrubbed, its `***` makes + // the verify re-run fail after the agent spend is already paid. + const { layer, handles } = makeCFRuntimeTest({ + secrets: { DEPLOY_TOKEN: "tok-abc123" }, + sandboxProgram: { + "curl -H 'Authorization: Bearer tok-abc123' https://x": { exitCode: 7 }, + }, + config: { "self-heal.ci.enabled": "true" }, + }); + return Effect.gen(function* () { + yield* Effect.exit( + offloadTest.run({ + ...baseInput, + command: "curl -H 'Authorization: Bearer tok-abc123' https://x", + secrets: ["DEPLOY_TOKEN"], + }), + ); + expect(handles.childRuns.spawned).toHaveLength(0); + }).pipe(Effect.provide(layer)); + }); + + it.effect("scrubs the value the container actually received, not the shadowed store value", () => { + // Per-dispatch `env` wins over a same-named store secret. Scrubbing + // `Object.values(secretEnv)` alone would keep the LIVE credential in the + // rendered summary and redact a string that was never printed. + const { layer } = makeCFRuntimeTest({ + secrets: { DEPLOY_TOKEN: "shadowed-store-value" }, + sandboxProgram: { + "deploy --token live-dispatch-value": { exitCode: 3 }, + }, + config: { + "offload-test.stages:owner/name": "a", + "offload-test.command:owner/name:a": "deploy --token live-dispatch-value", + }, + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + offloadTest.run({ + repo: "owner/name", + sha: "abc123", + failOnNonZeroExit: true, + secrets: ["DEPLOY_TOKEN"], + env: { DEPLOY_TOKEN: "live-dispatch-value" }, + }), + ); + expect(Exit.isFailure(exit)).toBe(true); + const failure = Exit.isFailure(exit) + ? Option.getOrUndefined(Cause.failureOption(exit.cause)) + : undefined; + const rendered = `${(failure as { summaryMd?: string })?.summaryMd ?? ""} ${ + (failure as { cause?: unknown })?.cause ?? "" + }`; + expect(rendered).not.toContain("live-dispatch-value"); + expect(rendered).toContain("***"); + }).pipe(Effect.provide(layer)); + }); + + it.effect("the shadowed store value stays scrubbed too — it is still a live credential", () => { + // The mirror image of the shadow case: env overrides the key, but the + // command inlines the STORE value. Scrubbing only the effective value would + // put a live store credential in the rendered summary. + const { layer } = makeCFRuntimeTest({ + secrets: { DEPLOY_TOKEN: "store-secret-value" }, + sandboxProgram: { + "deploy --token store-secret-value": { exitCode: 3 }, + }, + config: { + "offload-test.stages:owner/name": "a", + "offload-test.command:owner/name:a": "deploy --token store-secret-value", + }, + }); + return Effect.gen(function* () { + const exit = yield* Effect.exit( + offloadTest.run({ + repo: "owner/name", + sha: "abc123", + failOnNonZeroExit: true, + secrets: ["DEPLOY_TOKEN"], + env: { DEPLOY_TOKEN: "override-value" }, + }), + ); + expect(Exit.isFailure(exit)).toBe(true); + const failure = Exit.isFailure(exit) + ? Option.getOrUndefined(Cause.failureOption(exit.cause)) + : undefined; + const rendered = `${(failure as { summaryMd?: string })?.summaryMd ?? ""} ${ + (failure as { cause?: unknown })?.cause ?? "" + }`; + expect(rendered).not.toContain("store-secret-value"); + expect(rendered).toContain("***"); + }).pipe(Effect.provide(layer)); + }); + it.effect("does NOT dispatch self-heal when the gate is unset", () => { const { layer, handles } = makeCFRuntimeTest({ sandboxProgram: { "pnpm test": { exitCode: 1 } }, @@ -866,6 +962,71 @@ describe("offload-test staged mode", () => { }, ); + it.effect("a secret inlined in a stage command never reaches the check-run summary", () => { + // `redactValues` on the exec covers what the sandbox layer persists. It + // does not reach the command string this run embeds in `summaryMd`, which + // `stepFailedMd` renders as UNFENCED markdown into the GitHub check-run + // summary — public on a public repo, and the loudest of the three surfaces. + const { layer } = makeCFRuntimeTest({ + secrets: { DEPLOY_TOKEN: "tok-abc123" }, + sandboxProgram: { + "curl -H 'Authorization: Bearer tok-abc123' https://x": { exitCode: 3 }, + }, + config: { + "offload-test.stages:owner/name": "a", + "offload-test.command:owner/name:a": "curl -H 'Authorization: Bearer tok-abc123' https://x", + }, + }); + + return Effect.gen(function* () { + const exit = yield* Effect.exit( + offloadTest.run({ ...webhookInput, secrets: ["DEPLOY_TOKEN"] }), + ); + expect(Exit.isFailure(exit)).toBe(true); + const failure = Exit.isFailure(exit) + ? Option.getOrUndefined(Cause.failureOption(exit.cause)) + : undefined; + const rendered = `${(failure as { summaryMd?: string })?.summaryMd ?? ""} ${ + (failure as { cause?: unknown })?.cause ?? "" + }`; + expect(rendered).not.toContain("tok-abc123"); + expect(rendered).toContain("***"); + }).pipe(Effect.provide(layer)); + }); + + it.effect("a DYING stage's command is scrubbed too — the path that skips ExecTimeout", () => { + // The non-obvious half, and the one the red-path test above does not reach. + // A stage that dies is caught and replaced by `deadFailure`, so the raw + // `ExecTimeout` (whose `command` the sandbox layer scrubs) never surfaces — + // the run renders `stage.command` itself into `cause` and `summaryMd`. + const { layer } = makeCFRuntimeTest({ + secrets: { DEPLOY_TOKEN: "tok-abc123" }, + sandboxProgram: { + "deploy --token tok-abc123": { fail: "ExecTimeout", timeoutSec: 600 }, + }, + config: { + "offload-test.stages:owner/name": "a", + "offload-test.command:owner/name:a": "deploy --token tok-abc123", + }, + }); + + return Effect.gen(function* () { + const exit = yield* Effect.exit( + offloadTest.run({ ...webhookInput, secrets: ["DEPLOY_TOKEN"] }), + ); + expect(Exit.isFailure(exit)).toBe(true); + const failure = Exit.isFailure(exit) + ? Option.getOrUndefined(Cause.failureOption(exit.cause)) + : undefined; + expect((failure as { _tag?: string })?._tag).toBe("StepFailed"); + const rendered = `${(failure as { summaryMd?: string })?.summaryMd ?? ""} ${ + (failure as { cause?: unknown })?.cause ?? "" + }`; + expect(rendered).not.toContain("tok-abc123"); + expect(rendered).toContain("***"); + }).pipe(Effect.provide(layer)); + }); + it.effect( "a non-zero stage stops the sequence — later stages skipped, failure names the stage, earlier logs uploaded", () => { diff --git a/runs/offload-test.ts b/runs/offload-test.ts index 8635bef..f11b7fa 100644 --- a/runs/offload-test.ts +++ b/runs/offload-test.ts @@ -646,6 +646,43 @@ export const offloadTest = defineRun({ required: true, }); + /** + * The command, safe to RENDER. + * + * `redactValues` on the exec covers what the sandbox layer persists — the + * R2 log and `ExecTimeout.message`. It does not reach the command string + * this run then embeds itself, and those surfaces are the loud ones: + * `summaryMd` is rendered as unfenced markdown straight into the GitHub + * check-run summary, which on a public repo is public, and the self-heal + * path carries the command into a child execution's `input_json`. A dying + * stage also never reaches `ExecTimeout` — it is caught and replaced by + * `deadFailure` — so the layer-level scrub does not apply there at all. + * + * Duplicated logic rather than a shared import: `redact` exists three + * times already (sandbox-cf, sandbox-facade, sandbox-fake) and unifying + * them is its own change. This is the narrow version, for one job. + */ + // + // The values scrubbed are BOTH the store value and the one the container + // actually receives for each secret-designated key. Per-dispatch `env` + // wins over a same-named store secret, so `Object.values(secretEnv)` + // alone holds the shadowed value on a collision — the log would keep the + // live credential and scrub a string that was never printed. The shadowed + // store value stays in the list too: it is still a live credential, and + // a command can inline it even while the env carries the override. + // Non-secret `input.env` keys are deliberately NOT in this list: dispatch + // inputs are documented non-sensitive (header note 3), and scrubbing a + // value like "production" or "1" from every log line trades a contract + // violation nobody has made for garbled logs everybody reads. + const effectiveEnv: Record = { ...secretEnv, ...input.env }; + const secretValues = [ + ...new Set( + Object.keys(secretEnv).flatMap((key) => [secretEnv[key] ?? "", effectiveEnv[key] ?? ""]), + ), + ].filter((v) => v.length > 0); + const renderable = (command: string): string => + secretValues.reduce((out, value) => out.split(value).join("***"), command); + // self-heal — (gated, OFF unless `self-heal.ci.enabled=true`) auto-dispatch // a fix for a DETERMINISTIC CI failure. Unlike the LLM-driven demo verdict // (which needs k-of-n confirmation), a non-zero exit IS ground truth — the @@ -666,6 +703,20 @@ export const offloadTest = defineRun({ Effect.gen(function* () { if (execResult.exitCode === 0) return; if ((yield* config.get("self-heal.ci.enabled")) !== "true") return; + // A command whose rendering CHANGED had a secret value inlined in it, + // and neither version of it can honestly go to the healer. The raw + // command would smuggle a live credential into the credential-free + // agent sandbox and into a pack an injection-steerable LLM reads + // (ci-incident.ts header); the scrubbed one carries `***` where the + // verify step's re-run needs the value, so the heal can only fail + // after the agent spend is already paid. Skip, and say why. + if (renderable(failedCommand) !== failedCommand) { + yield* io.log( + "warn", + `offload-test: skipping self-heal — the failing command inlines a secret value, so no honest repro can be handed to the healer`, + ); + return; + } const incident = commandFailureToIncident({ repo: input.repo, sha: input.sha, @@ -815,6 +866,11 @@ export const offloadTest = defineRun({ container: ws.container, command: stage.command, env: { ...secretEnv, ...input.env }, + // Defense in depth, as `check` and `worker-deploy` already + // do: this run executes the CONSUMER's own command, where + // `set -x` and a stray `env` are ordinary rather than + // exceptional, and the captured log is durable and signed. + redactValues: secretValues, timeoutSec: stageTimeoutSec, }); }), @@ -1017,9 +1073,9 @@ export const offloadTest = defineRun({ // for the Workflow error record. new StepFailed({ step: `exec-${outcome.stage.label}`, - cause: `stage \`${outcome.stage.label}\` (\`${outcome.stage.command}\`) died: ${outcome.errorClass} after ~${outcome.elapsedS}s`, + cause: `stage \`${outcome.stage.label}\` (\`${renderable(outcome.stage.command)}\`) died: ${outcome.errorClass} after ~${outcome.elapsedS}s`, summaryMd: [ - `Stage \`${outcome.stage.label}\` — \`${outcome.stage.command}\` — died (\`${outcome.errorClass}\`) after ~${outcome.elapsedS}s. ` + + `Stage \`${outcome.stage.label}\` — \`${renderable(outcome.stage.command)}\` — died (\`${outcome.errorClass}\`) after ~${outcome.elapsedS}s. ` + `Earlier stage logs are already uploaded; this stage's log is the marker artifact \`step-${outcome.stage.label}.log\`.`, "", ...lines, @@ -1034,7 +1090,7 @@ export const offloadTest = defineRun({ step: `upload-log-${outcome.stage.label}`, cause: `stage \`${outcome.stage.label}\` ran, but its log upload failed: ${outcome.errorClass}`, summaryMd: [ - `Stage \`${outcome.stage.label}\` — \`${outcome.stage.command}\` — ran to a verdict, but \`step-${outcome.stage.label}.log\` did not upload (\`${outcome.errorClass}\`). ` + + `Stage \`${outcome.stage.label}\` — \`${renderable(outcome.stage.command)}\` — ran to a verdict, but \`step-${outcome.stage.label}.log\` did not upload (\`${outcome.errorClass}\`). ` + `The verdict is in the rundown below; the log is not retrievable.`, "", ...lines, @@ -1141,7 +1197,7 @@ export const offloadTest = defineRun({ new AcceptanceFailed({ exitCode: outcome.exitCode, summaryMd: [ - `Stage \`${outcome.stage.label}\` — \`${outcome.stage.command}\` — exited \`${outcome.exitCode}\`; later stages skipped.`, + `Stage \`${outcome.stage.label}\` — \`${renderable(outcome.stage.command)}\` — exited \`${outcome.exitCode}\`; later stages skipped.`, "", ...lines, ].join("\n"), @@ -1232,6 +1288,8 @@ export const offloadTest = defineRun({ // Per-dispatch `env` wins over a same-named config-store secret // — the more specific source overrides the global one. env: { ...secretEnv, ...input.env }, + // Same scrub as the staged path above and as `check` does. + redactValues: secretValues, timeoutSec, }), ),