Skip to content

Commit b9b384f

Browse files
committed
Harden runner workspace publication
1 parent 4409d0b commit b9b384f

6 files changed

Lines changed: 157 additions & 45 deletions

File tree

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,12 +388,13 @@ await rm(nativeResultPath, { force: true })
388388
const runtimeResult = sanitizeRuntimeSourceValue(nativeRuntimeResult, privateRuntimeSourceRootForSanitization)
389389
assertNoRuntimeSourcePaths(runtimeResult, privateRuntimeSourceRootForSanitization)
390390
let workspaceApply = { status: "no-op", changedFiles: [] }
391+
let runnerWorkspaceCore = null
391392
if (execution.code === 0 && runtimeResult.success === true && request.runner_workspace?.enabled) {
392-
const core = await import(pathToFileURL(join(codeboxRoot, "packages/runtime-core/dist/runner-workspace-apply.js")).href)
393+
runnerWorkspaceCore = await import(pathToFileURL(join(codeboxRoot, "packages/runtime-core/dist/runner-workspace-apply.js")).href)
393394
const publicCore = await import(pathToFileURL(join(codeboxRoot, "packages/runtime-core/dist/public.js")).href)
394395
const refs = publicCore.normalizePublicArtifactRefDTOs(runtimeResult)
395396
.filter((ref) => ref.kind === "codebox-patch" || ref.kind === "codebox-changed-files")
396-
workspaceApply = await core.applyRunnerWorkspacePatch({ artifactRoot: artifactsPath, artifactRefs: refs, workspaceRoot: workspace, writablePaths: String(request.writable_paths || "").split(",").map((value) => value.trim()).filter(Boolean) })
397+
workspaceApply = await runnerWorkspaceCore.applyRunnerWorkspacePatch({ artifactRoot: artifactsPath, artifactRefs: refs, workspaceRoot: workspace, writablePaths: String(request.writable_paths || "").split(",").map((value) => value.trim()).filter(Boolean) })
397398
}
398399
await cleanupPrivateRuntimeSources()
399400
assertNoRuntimeSourcePaths(runtimeResult, privateRuntimeSourceRootForSanitization)
@@ -422,9 +423,10 @@ const runtimeRecord = record(runtimeResult)
422423
const agentResult = record(runtimeRecord.agent_task_run_result)
423424
let publication = resultValue(runtimeRecord, "metadata.runner_workspace_publication")
424425
if (execution.code === 0 && runtimeRecord.success === true && verificationPassed && workspaceApply.status === "applied") {
426+
await runnerWorkspaceCore.verifyRunnerWorkspaceIntegrity(workspaceApply.integrity)
425427
const testPublisher = string(process.env.WP_CODEBOX_TEST_PUBLISHER_MODULE)
426428
const publisher = testPublisher ? (await import(pathToFileURL(resolve(testPublisher)).href)).publishRunnerWorkspace : publishRunnerWorkspace
427-
publication = await publisher({ request, workspace, changedFiles: workspaceApply.changedFiles, token: process.env.ACCESS_TOKEN || process.env.GITHUB_TOKEN })
429+
publication = await publisher({ request, changedFiles: workspaceApply.changedFiles, publicationFiles: workspaceApply.publicationFiles, token: process.env.ACCESS_TOKEN || process.env.GITHUB_TOKEN })
428430
runtimeRecord.metadata = { ...record(runtimeRecord.metadata), runner_workspace_publication: publication }
429431
runtimeRecord.outputs = { ...record(runtimeRecord.outputs), runner_workspace_publication: publication }
430432
}

.github/scripts/run-agent-task/runner-workspace-publisher.mjs

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
import { readFile } from "node:fs/promises"
2-
import { resolve } from "node:path"
31

42
function string(value) { return typeof value === "string" ? value.trim() : "" }
53
function record(value) { return value && typeof value === "object" && !Array.isArray(value) ? value : {} }
64

7-
export async function publishRunnerWorkspace({ request, workspace, changedFiles, token, fetchImpl = fetch }) {
5+
export async function publishRunnerWorkspace({ request, changedFiles, publicationFiles, workspace, token, fetchImpl = fetch }) {
86
const targetRepo = string(request.target_repo).toLowerCase()
97
const config = record(request.runner_workspace)
108
const configuredRepo = string(config.repo).toLowerCase()
@@ -23,28 +21,26 @@ export async function publishRunnerWorkspace({ request, workspace, changedFiles,
2321
if (!response.ok) throw new Error(`GitHub API ${method} ${path} failed with ${response.status}.`)
2422
return payload
2523
}
26-
const baseRef = await api("GET", `/git/ref/heads/${encodeURIComponent(base)}`)
27-
const baseSha = string(baseRef.object?.sha)
24+
let existing = null
25+
try { existing = await api("GET", `/git/ref/heads/${head.split("/").map(encodeURIComponent).join("/")}`) } catch (error) { if (!String(error.message).includes(" 404.")) throw error }
26+
// Existing PR branches are append-only publication targets. Their current tree
27+
// is the base so files from earlier agent turns cannot disappear.
28+
const parent = string(existing?.object?.sha)
29+
const baseRef = parent ? null : await api("GET", `/git/ref/heads/${encodeURIComponent(base)}`)
30+
const baseSha = parent || string(baseRef.object?.sha)
2831
const baseCommit = await api("GET", `/git/commits/${baseSha}`)
2932
const tree = []
30-
for (const changed of changedFiles) {
31-
const relativePath = string(changed)
33+
const captured = Array.isArray(publicationFiles) ? publicationFiles : []
34+
if (!captured.length) throw new Error("Runner workspace publication requires immutable approved file content.")
35+
for (const changed of captured) {
36+
const relativePath = string(changed?.path)
3237
if (!relativePath || relativePath.startsWith("/") || relativePath.split("/").some((part) => !part || part === "." || part === ".." || part === ".git")) throw new Error("Publication changed file path is invalid.")
33-
const absolute = resolve(workspace, relativePath)
34-
if (!absolute.startsWith(`${resolve(workspace)}/`)) throw new Error("Publication changed file escapes workspace.")
35-
try {
36-
const content = await readFile(absolute)
37-
const blob = await api("POST", "/git/blobs", { content: content.toString("base64"), encoding: "base64" })
38-
tree.push({ path: relativePath, mode: "100644", type: "blob", sha: string(blob.sha) })
39-
} catch (error) {
40-
if (error?.code === "ENOENT") tree.push({ path: relativePath, mode: "100644", type: "blob", sha: null })
41-
else throw error
42-
}
38+
if (changed.deleted) { tree.push({ path: relativePath, mode: "100644", type: "blob", sha: null }); continue }
39+
if (changed.mode !== "100644" && changed.mode !== "100755" || typeof changed.content !== "string") throw new Error("Publication file is not an approved regular file.")
40+
const blob = await api("POST", "/git/blobs", { content: changed.content, encoding: "base64" })
41+
tree.push({ path: relativePath, mode: changed.mode, type: "blob", sha: string(blob.sha) })
4342
}
4443
const nextTree = await api("POST", "/git/trees", { base_tree: string(baseCommit.tree?.sha), tree })
45-
let parent = baseSha
46-
let existing = null
47-
try { existing = await api("GET", `/git/ref/heads/${head.split("/").map(encodeURIComponent).join("/")}`); parent = string(existing.object?.sha) || baseSha } catch (error) { if (!String(error.message).includes(" 404.")) throw error }
4844
const commit = await api("POST", "/git/commits", { message: string(config.commit_message || request.workload?.label || "Apply agent task changes"), tree: string(nextTree.sha), parents: [parent] })
4945
if (existing) await api("PATCH", `/git/refs/heads/${head.split("/").map(encodeURIComponent).join("/")}`, { sha: string(commit.sha), force: false })
5046
else await api("POST", "/git/refs", { ref: `refs/heads/${head}`, sha: string(commit.sha) })

packages/runtime-core/src/runner-workspace-apply.ts

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { execFile } from "node:child_process"
2-
import { readFile, lstat, realpath } from "node:fs/promises"
3-
import { isAbsolute, resolve } from "node:path"
2+
import { readFile, lstat, realpath, readdir } from "node:fs/promises"
3+
import { isAbsolute, resolve, relative } from "node:path"
44
import { promisify } from "node:util"
55
import { createHash } from "node:crypto"
66
import { pathIsWithinRoot, relativePathMatchesExcludePattern } from "./file-tree-policy.js"
@@ -36,6 +36,22 @@ export interface RunnerWorkspaceApplyResult {
3636
status: "applied" | "no-op"
3737
changedFiles: string[]
3838
patchSha256?: string
39+
integrity?: RunnerWorkspaceIntegritySnapshot
40+
publicationFiles?: RunnerWorkspacePublicationFile[]
41+
}
42+
43+
export interface RunnerWorkspacePublicationFile {
44+
path: string
45+
mode: "100644" | "100755"
46+
content?: string
47+
sha256?: string
48+
deleted: boolean
49+
}
50+
51+
export interface RunnerWorkspaceIntegritySnapshot {
52+
workspaceRoot: string
53+
files: RunnerWorkspacePublicationFile[]
54+
baseline: RunnerWorkspacePublicationFile[]
3955
}
4056

4157
/**
@@ -51,6 +67,7 @@ export async function applyRunnerWorkspacePatch(request: RunnerWorkspaceApplyReq
5167
const patchPath = await artifactPath(artifactRoot, patchRef.path)
5268
const changedPath = await artifactPath(artifactRoot, changedRef.path)
5369
const [patch, changedRaw] = await Promise.all([readBoundedText(patchPath), readBoundedText(changedPath)])
70+
const baseline = await snapshotWorkspace(workspaceRoot)
5471
verifyDigest(patch, patchRef.sha256)
5572

5673
const changed = parseChangedFiles(changedRaw)
@@ -64,13 +81,28 @@ export async function applyRunnerWorkspacePatch(request: RunnerWorkspaceApplyReq
6481

6582
await execGit(workspaceRoot, ["apply", "--check", "--whitespace=error", "--", patchPath])
6683
await execGit(workspaceRoot, ["apply", "--whitespace=error", "--", patchPath])
84+
const files = await snapshotWorkspace(workspaceRoot)
85+
validateAppliedWorkspace(baseline, files, changed)
6786
if (request.verify) await request.verify()
6887

6988
return {
7089
schema: "wp-codebox/runner-workspace-apply-result/v1",
7190
status: "applied",
7291
changedFiles: changed.map((file) => file.relativePath),
7392
patchSha256: createHash("sha256").update(patch).digest("hex"),
93+
integrity: { workspaceRoot, files, baseline },
94+
publicationFiles: changed.map((change) => {
95+
const file = files.find((candidate) => candidate.path === change.relativePath)
96+
return file ? file : { path: change.relativePath, mode: "100644", deleted: true }
97+
}),
98+
}
99+
}
100+
101+
/** Ensures checks did not mutate approved output or introduce unrelated files. */
102+
export async function verifyRunnerWorkspaceIntegrity(snapshot: RunnerWorkspaceIntegritySnapshot): Promise<void> {
103+
const current = await snapshotWorkspace(snapshot.workspaceRoot)
104+
if (JSON.stringify(current) !== JSON.stringify(snapshot.files)) {
105+
throw new Error("Runner workspace changed after approval; refusing publication.")
74106
}
75107
}
76108

@@ -121,7 +153,7 @@ function validateChangedFiles(files: RunnerWorkspaceChangedFile[], writablePaths
121153
for (const file of files) {
122154
const path = file.relativePath.replaceAll("\\", "/")
123155
if (!path || path.startsWith("/") || path.split("/").some((part) => part === "" || part === "." || part === ".." || part === ".git" || part === ".codebox")) throw new Error(`Changed file has a denied path: ${file.relativePath}`)
124-
if (![file.beforeMode, file.afterMode].filter(Boolean).every((mode) => mode === "100644")) throw new Error(`Changed file has an unsupported mode: ${file.relativePath}`)
156+
if (![file.beforeMode, file.afterMode].filter(Boolean).every((mode) => mode === "100644" || mode === "100755")) throw new Error(`Changed file has an unsupported mode: ${file.relativePath}`)
125157
if (!writablePaths.some((pattern) => relativePathMatchesExcludePattern(path, pattern))) throw new Error(`Changed file is outside writable_paths: ${file.relativePath}`)
126158
}
127159
}
@@ -133,7 +165,53 @@ function validatePatchPaths(patch: string, changed: RunnerWorkspaceChangedFile[]
133165
const path = line.slice(4).split("\t", 1)[0].trim().replace(/^[ab]\//, "")
134166
return path === "/dev/null" ? [] : [path]
135167
})
136-
if (paths.length === 0 || paths.some((path) => !declared.has(path))) throw new Error("Patch paths do not exactly correspond to canonical changed-files.")
168+
if (paths.length === 0 || paths.some((path) => !declared.has(path)) || [...declared].some((path) => !paths.includes(path))) throw new Error("Patch paths do not exactly correspond to canonical changed-files.")
169+
for (const line of patch.split("\n")) {
170+
if (/^(old mode|new mode|new file mode|deleted file mode) /.test(line)) {
171+
const mode = line.split(" ").at(-1)
172+
if (mode !== "100644" && mode !== "100755") throw new Error("Patch contains an unsupported file mode.")
173+
}
174+
if (/^(similarity index|rename from|rename to|copy from|copy to|Subproject commit)/.test(line)) throw new Error("Patch contains unsupported git metadata.")
175+
}
176+
}
177+
178+
async function snapshotWorkspace(root: string): Promise<RunnerWorkspacePublicationFile[]> {
179+
const output: RunnerWorkspacePublicationFile[] = []
180+
async function visit(directory: string): Promise<void> {
181+
for (const entry of await readdir(directory, { withFileTypes: true })) {
182+
if (entry.name === ".git" || entry.name === ".codebox") continue
183+
const absolute = resolve(directory, entry.name)
184+
const path = relative(root, absolute).replaceAll("\\", "/")
185+
const stat = await lstat(absolute)
186+
if (stat.isSymbolicLink()) throw new Error(`Runner workspace contains an unsupported path type: ${path}`)
187+
if (stat.isDirectory()) {
188+
await visit(absolute)
189+
continue
190+
}
191+
if (!stat.isFile()) throw new Error(`Runner workspace contains an unsupported path type: ${path}`)
192+
const mode = (stat.mode & 0o111) ? "100755" : "100644"
193+
const bytes = await readFile(absolute)
194+
output.push({ path, mode, content: bytes.toString("base64"), sha256: createHash("sha256").update(bytes).digest("hex"), deleted: false })
195+
}
196+
}
197+
await visit(root)
198+
return output.sort((a, b) => a.path.localeCompare(b.path))
199+
}
200+
201+
function validateAppliedWorkspace(baseline: RunnerWorkspacePublicationFile[], current: RunnerWorkspacePublicationFile[], changed: RunnerWorkspaceChangedFile[]): void {
202+
const before = new Map(baseline.map((file) => [file.path, file]))
203+
const after = new Map(current.map((file) => [file.path, file]))
204+
const actual = new Set([...before.keys(), ...after.keys()].filter((path) => JSON.stringify(before.get(path)) !== JSON.stringify(after.get(path))))
205+
const declared = new Set(changed.map((file) => file.relativePath))
206+
if (actual.size !== declared.size || [...actual].some((path) => !declared.has(path))) throw new Error("Applied workspace differs from the canonical changed-files manifest.")
207+
for (const file of changed) {
208+
const value = after.get(file.relativePath)
209+
if (file.status === "deleted") {
210+
if (value) throw new Error(`Canonical deletion was not applied: ${file.relativePath}`)
211+
continue
212+
}
213+
if (!value || value.mode !== file.afterMode) throw new Error(`Applied file mode does not match canonical manifest: ${file.relativePath}`)
214+
}
137215
}
138216

139217
async function execGit(cwd: string, args: string[]): Promise<void> {

packages/wordpress-plugin/src/class-wp-codebox-browser-runner-template.php

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -912,7 +912,10 @@ public static function runner_contract( string $runner_php ): array {
912912

913913
/** Builds the generated PHP runtime-local tool registration fragment. */
914914
public static function runtime_tool_registration_fragment(): string {
915-
return '
915+
// The generated Playground runner is intentionally self-contained. Embed the
916+
// same bounded executor used by the host registration, never a host bridge.
917+
$sandbox_executor = file_get_contents( __DIR__ . '/class-wp-codebox-sandbox-workspace-executor.php' );
918+
return "\nif ( ! class_exists( 'WP_Codebox_Sandbox_Workspace_Executor' ) ) {\n" . $sandbox_executor . "\n}\n" . '
916919
class WP_Codebox_Browser_Filesystem_Write_Tool {
917920
public function handle_tool_call( array $parameters, array $tool_def = array() ): array {
918921
global $wp_codebox_browser_artifact_environment;
@@ -959,15 +962,22 @@ function wp_codebox_browser_runtime_tool_name( string $tool_id ): string {
959962
return \'filesystem_write\';
960963
}
961964
965+
if ( preg_match( "/^(?:client\\/)?workspace_(?:show|ls|read|grep|write|edit|apply_patch|git_status|git_diff)$/", $tool_id ) ) {
966+
return str_starts_with( $tool_id, "client/" ) ? substr( $tool_id, 7 ) : $tool_id;
967+
}
968+
962969
return \'\';
963970
}
964971
965972
function wp_codebox_browser_runtime_tool_declarations( array $tool_names ): array {
966973
global $wp_codebox_browser_artifact_environment;
967974
968975
$declarations = array();
969-
if ( ! in_array( \'filesystem_write\', $tool_names, true ) ) {
970-
return $declarations;
976+
foreach ( WP_Codebox_Sandbox_Workspace_Executor::tool_declarations() as $name => $declaration ) {
977+
$base = substr( $name, strlen( "client/" ) );
978+
if ( in_array( $base, $tool_names, true ) ) {
979+
$declarations[ $base ] = $declaration;
980+
}
971981
}
972982
973983
$environment = is_array( $wp_codebox_browser_artifact_environment ?? null ) ? $wp_codebox_browser_artifact_environment : array();
@@ -1119,7 +1129,16 @@ function wp_codebox_browser_runtime_replay_ability_lifecycle(): array {
11191129
}
11201130
11211131
function wp_codebox_browser_runtime_tool_callback( array $request, array $payload ) {
1122-
unset( $payload );
1132+
1133+
$workspace_tool = (string) ( $request[\'tool_name\'] ?? \'\' );
1134+
if ( preg_match( "/^(?:client\\/)?workspace_(?:show|ls|read|grep|write|edit|apply_patch|git_status|git_diff)$/", $workspace_tool ) ) {
1135+
$task_input = is_array( $payload[\'task_input\'] ?? null ) ? $payload[\'task_input\'] : array();
1136+
$context = array(
1137+
\'workspace_root\' => \'/workspace\',
1138+
\'writable_paths\' => is_array( $task_input[\'writable_paths\'] ?? null ) ? $task_input[\'writable_paths\'] : array(),
1139+
);
1140+
return ( new WP_Codebox_Sandbox_Workspace_Executor() )->executeWP_Agent_Tool_Call( $request, is_array( $request[\'tool_def\'] ?? null ) ? $request[\'tool_def\'] : array(), $context );
1141+
}
11231142
11241143
if ( ! in_array( (string) ( $request[\'tool_name\'] ?? \'\' ), array( \'filesystem_write\', \'client/filesystem-write\' ), true ) ) {
11251144
return null;

tests/runner-workspace-apply.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { tmpdir } from "node:os"
55
import { join } from "node:path"
66
import { promisify } from "node:util"
77
import { createHash } from "node:crypto"
8-
import { applyRunnerWorkspacePatch } from "../packages/runtime-core/src/runner-workspace-apply.js"
8+
import { applyRunnerWorkspacePatch, verifyRunnerWorkspaceIntegrity } from "../packages/runtime-core/src/runner-workspace-apply.js"
99

1010
const exec = promisify(execFile)
1111

@@ -35,6 +35,7 @@ const files = [{ path: "/workspace/README.md", relativePath: "README.md", status
3535
assert.equal(result.status, "applied")
3636
assert.equal(await readFile(join(input.workspace, "README.md"), "utf8"), "after\n")
3737
assert.deepEqual(order, ["verify"])
38+
await verifyRunnerWorkspaceIntegrity(result.integrity!)
3839
}
3940

4041
{
@@ -43,6 +44,27 @@ const files = [{ path: "/workspace/README.md", relativePath: "README.md", status
4344
assert.equal(result.status, "no-op")
4445
}
4546

47+
{
48+
const executablePatch = "diff --git a/README.md b/README.md\nold mode 100644\nnew mode 100755\n--- a/README.md\n+++ b/README.md\n@@ -1 +1 @@\n-before\n+after\n"
49+
const executableFiles = [{ ...files[0], afterMode: "100755" }]
50+
const input = await fixture(executablePatch, executableFiles)
51+
const result = await applyRunnerWorkspacePatch({ artifactRoot: input.artifacts, artifactRefs: input.refs, workspaceRoot: input.workspace, writablePaths: ["README.md"] })
52+
assert.equal(result.publicationFiles?.[0]?.mode, "100755")
53+
}
54+
55+
{
56+
const input = await fixture(patch, files)
57+
await writeFile(join(input.artifacts, "files", "changed-files.json"), JSON.stringify({ schema: "wp-codebox/changed-files/v1", files: [{ ...files[0], relativePath: "other.md" }] }))
58+
await assert.rejects(() => applyRunnerWorkspacePatch({ artifactRoot: input.artifacts, artifactRefs: input.refs, workspaceRoot: input.workspace, writablePaths: ["README.md", "other.md"] }), /exactly correspond/)
59+
}
60+
61+
{
62+
const input = await fixture(patch, files)
63+
const result = await applyRunnerWorkspacePatch({ artifactRoot: input.artifacts, artifactRefs: input.refs, workspaceRoot: input.workspace, writablePaths: ["README.md"] })
64+
await writeFile(join(input.workspace, "extra.txt"), "unexpected\n")
65+
await assert.rejects(() => verifyRunnerWorkspaceIntegrity(result.integrity!), /changed after approval/)
66+
}
67+
4668
{
4769
const input = await fixture(patch, files)
4870
await assert.rejects(() => applyRunnerWorkspacePatch({ artifactRoot: input.artifacts, artifactRefs: input.refs, workspaceRoot: input.workspace, writablePaths: ["src/**"] }), /outside writable_paths/)

0 commit comments

Comments
 (0)