Skip to content

Commit 79d6456

Browse files
committed
Fix runner workspace patch artifact redaction
1 parent 8d49e91 commit 79d6456

3 files changed

Lines changed: 59 additions & 10 deletions

File tree

.github/scripts/run-agent-task/execute-native-agent-task.mjs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { rmSync } from "node:fs"
22
import { appendFile, lstat, mkdir, readFile, realpath, rm, writeFile } from "node:fs/promises"
3+
import { isUtf8 } from "node:buffer"
34
import { isAbsolute, join, relative, resolve } from "node:path"
45
import { pathToFileURL } from "node:url"
56
import { spawn } from "node:child_process"
@@ -9,6 +10,7 @@ import { readNativeResult } from "./native-result-file.mjs"
910
import { assertNoRuntimeSourcePaths, sanitizeRuntimeSourceJson, sanitizeRuntimeSourceText, sanitizeRuntimeSourceValue } from "./runtime-source-sanitizer.mjs"
1011
import { publishRunnerWorkspace } from "./runner-workspace-publisher.mjs"
1112
import { createRunnerWorkspaceSeedSnapshot, RUNNER_WORKSPACE_SEED_EXCLUDES } from "./runner-workspace-seed-snapshot.mjs"
13+
import { createTrustedArtifactSnapshot } from "./trusted-artifact-snapshot.mjs"
1214

1315
const requestPath = process.env.AGENT_TASK_REQUEST_PATH || ".codebox/agent-task-request.json"
1416
const workspace = resolve(process.env.AGENT_TASK_WORKSPACE || process.cwd())
@@ -474,10 +476,10 @@ async function redactArtifactFiles(directory, artifactRoot = directory) {
474476
const path = join(directory, entry.name)
475477
if (reviewerEvidence?.transcript && path === resolve(artifactRoot, reviewerEvidence.transcript.path)) continue
476478
if (entry.isDirectory()) await redactArtifactFiles(path, artifactRoot)
477-
if (entry.isFile() && path.endsWith(".json") && (await stat(path)).size <= 4 * 1024 * 1024) {
478-
const contents = await readFile(path, "utf8").catch(() => null)
479-
if (contents !== null) {
480-
const sanitized = sanitizeRuntimeSourceJson(bounded(contents, 4 * 1024 * 1024), privateRuntimeSourceRootForSanitization)
479+
if (entry.isFile() && (await stat(path)).size <= 4 * 1024 * 1024) {
480+
const contents = await readFile(path).catch(() => null)
481+
if (contents && !contents.includes(0) && isUtf8(contents)) {
482+
const sanitized = sanitizeRuntimeSourceJson(redact(bounded(contents.toString("utf8"), 4 * 1024 * 1024)), privateRuntimeSourceRootForSanitization)
481483
assertNoRuntimeSourcePaths(sanitized, privateRuntimeSourceRootForSanitization)
482484
await writeFile(path, sanitized)
483485
}
@@ -636,7 +638,12 @@ if (execution.code === 0 && runtimeResult.success === true && request.runner_wor
636638
const publicCore = await import(pathToFileURL(join(codeboxRoot, "packages/runtime-core/dist/public.js")).href)
637639
const refs = publicCore.normalizePublicArtifactRefDTOs(runtimeResult)
638640
.filter((ref) => ref.kind === "codebox-patch" || ref.kind === "codebox-changed-files")
639-
workspaceApply = await runnerWorkspaceCore.applyRunnerWorkspacePatch({ artifactRoot: artifactsPath, artifactRefs: refs, workspaceRoot: workspace, writablePaths, seedIdentity: runnerWorkspaceSeedSnapshot?.provenance.identity })
641+
const trustedArtifacts = await createTrustedArtifactSnapshot(artifactsPath, refs)
642+
try {
643+
workspaceApply = await runnerWorkspaceCore.applyRunnerWorkspacePatch({ artifactRoot: trustedArtifacts.root, artifactRefs: trustedArtifacts.refs, workspaceRoot: workspace, writablePaths, seedIdentity: runnerWorkspaceSeedSnapshot?.provenance.identity })
644+
} finally {
645+
await rm(trustedArtifacts.root, { recursive: true, force: true })
646+
}
640647
} catch (error) {
641648
downstreamFailure = { stage: "apply", message: bounded(error instanceof Error ? error.message : String(error), MAX_WORKFLOW_OUTPUT_BYTES), ...(error?.evidence ? { evidence: error.evidence } : {}) }
642649
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { lstat, mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"
2+
import { tmpdir } from "node:os"
3+
import { isAbsolute, join, relative, resolve } from "node:path"
4+
5+
const MAX_SNAPSHOT_FILE_BYTES = 5 * 1024 * 1024
6+
7+
function underRoot(root, candidate) {
8+
const contained = relative(root, candidate)
9+
return candidate === root || (contained !== ".." && !contained.startsWith(`..${String.fromCharCode(47)}`) && !isAbsolute(contained))
10+
}
11+
12+
/**
13+
* Copies selected regular artifacts to a private temporary root before a later
14+
* durable-artifact transformation. Callers retain their artifact references,
15+
* with paths rewritten relative to the trusted copy.
16+
*/
17+
export async function createTrustedArtifactSnapshot(artifactRoot, refs, maxFileBytes = MAX_SNAPSHOT_FILE_BYTES) {
18+
const root = await realpath(resolve(artifactRoot))
19+
const snapshotRoot = await mkdtemp(join(tmpdir(), "wp-codebox-trusted-artifacts-"))
20+
try {
21+
const copiedRefs = await Promise.all(refs.map(async (ref) => {
22+
const requested = isAbsolute(ref.path) ? resolve(ref.path) : resolve(root, ref.path)
23+
const stat = await lstat(requested)
24+
if (!stat.isFile() || stat.isSymbolicLink() || stat.size > maxFileBytes) throw new Error("Trusted artifact snapshot requires a bounded regular file.")
25+
const source = await realpath(requested)
26+
if (!underRoot(root, source)) throw new Error("Trusted artifact snapshot escapes the artifact root.")
27+
const path = relative(root, source).replaceAll("\\", "/")
28+
const destination = join(snapshotRoot, path)
29+
await mkdir(resolve(destination, ".."), { recursive: true })
30+
await writeFile(destination, await readFile(source))
31+
return { ...ref, path }
32+
}))
33+
return { root: snapshotRoot, refs: copiedRefs }
34+
} catch (error) {
35+
await rm(snapshotRoot, { recursive: true, force: true })
36+
throw error
37+
}
38+
}

tests/execute-native-agent-task-lifecycle.test.mjs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ async function run(mode = "success") {
1515
const temp = await mkdtemp(join(tmpdir(), "wp-codebox-native-lifecycle-"))
1616
const workspace = join(temp, "workspace")
1717
await mkdir(join(workspace, ".codebox"), { recursive: true })
18-
await writeFile(join(workspace, "README.md"), "before\n")
18+
await writeFile(join(workspace, "README.md"), "OPENAI_API_KEY\nbefore\n")
1919

2020
const request = {
2121
schema: "wp-codebox/agent-task-workflow-request/v1",
@@ -74,7 +74,7 @@ const output = process.argv[process.argv.indexOf("--result-file") + 1]
7474
const input = JSON.parse(await readFile(process.argv[process.argv.indexOf("--input-file") + 1], "utf8"))
7575
const root = join(process.cwd(), ".codebox", "agent-task-artifacts", "files")
7676
await mkdir(root, { recursive: true })
77-
const patch = ${JSON.stringify(noOp ? "" : applyReject ? "--- a/README.md\n+++ b/README.md\n@@ -1 +1 @@\n-not-before\n+after\n" : "--- a/README.md\n+++ b/README.md\n@@ -1 +1 @@\n-before\n+after\n")}
77+
const patch = ${JSON.stringify(noOp ? "" : applyReject ? "--- a/README.md\n+++ b/README.md\n@@ -1,2 +1,2 @@\n OPENAI_API_KEY\n-not-before\n+after\n" : "--- a/README.md\n+++ b/README.md\n@@ -1,2 +1,2 @@\n OPENAI_API_KEY\n-before\n+after\n")}
7878
await writeFile(join(root, "patch.diff"), patch)
7979
await writeFile(join(root, "changed-files.json"), JSON.stringify({
8080
schema: "wp-codebox/changed-files/v1",
@@ -146,6 +146,7 @@ process.stdout.write(JSON.stringify({
146146
WP_CODEBOX_TEST_PUBLISHER_MODULE: publisher,
147147
WP_CODEBOX_TEST_PUBLISHER_HOOK: JSON.stringify({ orderPath: publisherOrderPath }),
148148
GITHUB_TOKEN: "token",
149+
MODEL_PROVIDER_SECRET_1: "OPENAI_API_KEY",
149150
EXPLICIT_ACCESS_TOKEN_CONFIGURED: "true",
150151
EXTERNAL_PACKAGE_SOURCE_POLICY: JSON.stringify({
151152
version: 1,
@@ -169,7 +170,10 @@ process.stdout.write(JSON.stringify({
169170

170171
const success = await run()
171172
assert.equal(success.code, 0, success.stderr)
172-
assert.equal(await readFile(join(success.workspace, "README.md"), "utf8"), "after\n")
173+
assert.equal(await readFile(join(success.workspace, "README.md"), "utf8"), "OPENAI_API_KEY\nafter\n")
174+
const durablePatch = await readFile(join(success.workspace, ".codebox", "agent-task-artifacts", "files", "patch.diff"), "utf8")
175+
assert(!durablePatch.includes("OPENAI_API_KEY"), "durable patch artifacts redact configured secret values")
176+
assert.match(durablePatch, /\[REDACTED\]/)
173177
assert.match(success.order, /runtime\nvalidation\nverification\ndrift\npublish\n/)
174178
assert.equal(success.result.runtime_result.metadata.runner_workspace_publication.pull_request.url, "https://github.com/owner/repo/pull/1")
175179
assert.equal(success.result.runtime_result.agent_runtime_diagnostics.prepared_paths.workspaces[0].target, "/workspace")
@@ -214,12 +218,12 @@ await execFileAsync(process.execPath, [uploader], {
214218
AGENT_TASK_UPLOAD_PATH: join(mismatch.workspace, ".codebox", "agent-task-upload"),
215219
},
216220
})
217-
assert.equal(await readFile(join(mismatch.workspace, ".codebox", "agent-task-upload", ".codebox", "agent-task-artifacts", "apply-failure", "rejected.patch"), "utf8"), "--- a/README.md\n+++ b/README.md\n@@ -1 +1 @@\n-before\n+after\n")
221+
assert.equal(await readFile(join(mismatch.workspace, ".codebox", "agent-task-upload", ".codebox", "agent-task-artifacts", "apply-failure", "rejected.patch"), "utf8"), "--- a/README.md\n+++ b/README.md\n@@ -1,2 +1,2 @@\n [REDACTED]\n-before\n+after\n")
218222
assert.match(await readFile(join(mismatch.workspace, ".codebox", "agent-task-upload", ".codebox", "agent-task-artifacts", "apply-failure", "changed-files.json"), "utf8"), /README\.md/)
219223

220224
const applyReject = await run("apply-reject")
221225
assert.equal(applyReject.code, 1)
222-
assert.equal(await readFile(join(applyReject.workspace, "README.md"), "utf8"), "before\n", "a rejected patch does not partially mutate the matching host workspace")
226+
assert.equal(await readFile(join(applyReject.workspace, "README.md"), "utf8"), "OPENAI_API_KEY\nbefore\n", "a rejected patch does not partially mutate the matching host workspace")
223227
assert.equal(applyReject.result.failure.stage, "apply")
224228
assert.match(applyReject.result.failure.message, /Host git apply failed/)
225229
assert.deepEqual(applyReject.result.failure.evidence.expected_identity, applyReject.result.failure.evidence.actual_identity, "the failure records the matching seed and host identities")

0 commit comments

Comments
 (0)