Skip to content

Commit 12a5bb1

Browse files
authored
Add declared verification command artifacts (#1888)
1 parent bc98294 commit 12a5bb1

9 files changed

Lines changed: 411 additions & 74 deletions
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { isAbsolute } from "node:path"
2+
3+
const SOURCE_TREE = /(^|\/)(prepared-plugins|prepared-source-packages|source-package[^/]*)(\/|$)/i
4+
const SOURCE_FILE = /\.(?:php|phtml|js|mjs|cjs|jsx|ts|tsx)$/i
5+
const PHP_OPENING_TAG = /<\?(?:php|=)(?:\s|$)/i
6+
const PHP_DECLARATION = /\b(?:namespace\s+\\?[A-Za-z_]\w*(?:\\[A-Za-z_]\w*)*|(?:abstract\s+|final\s+|readonly\s+)*(?:class|interface|trait|enum)\s+[A-Za-z_]\w*|function\s+&?\s*[A-Za-z_]\w*\s*\()/i
7+
const WORDPRESS_PLUGIN_HEADER = /\/\*[\s\S]{0,200}?\bPlugin Name\s*:/i
8+
9+
export function artifactSourcePathCategory(path) {
10+
if (SOURCE_TREE.test(path)) return "source-tree"
11+
if (SOURCE_FILE.test(path)) return "source-file"
12+
return ""
13+
}
14+
15+
// Diagnostics commonly name runtime classes. Reject only PHP-shaped source, even
16+
// when a source file has been disguised with a reviewer-safe extension.
17+
export function containsRuntimeSourceContent(text) {
18+
const hasPhpTag = PHP_OPENING_TAG.test(text)
19+
const hasDeclaration = PHP_DECLARATION.test(text)
20+
return (hasPhpTag && hasDeclaration) || (WORDPRESS_PLUGIN_HEADER.test(text) && (hasPhpTag || hasDeclaration))
21+
}
22+
23+
export function assertNoSeedSnapshotPaths(text) {
24+
if (/wp-codebox-runner-workspace-seed-/i.test(text)) throw new Error("Temporary runner workspace seed paths must never be persisted in artifact uploads.")
25+
try {
26+
const visit = (value) => {
27+
if (Array.isArray(value)) return value.forEach(visit)
28+
const entry = value && typeof value === "object" && !Array.isArray(value) ? value : {}
29+
if (entry.seed && typeof entry.seed === "object" && !Array.isArray(entry.seed) && isAbsolute(entry.seed.source)) throw new Error("Absolute runner workspace seed paths must never be persisted in artifact uploads.")
30+
Object.values(entry).forEach(visit)
31+
}
32+
visit(JSON.parse(text))
33+
} catch (error) {
34+
if (error instanceof Error && /seed paths/.test(error.message)) throw error
35+
}
36+
}
37+
38+
export function sanitizeSeedSnapshotJson(text) {
39+
try {
40+
const compact = (value, key = "") => {
41+
if (typeof value === "string") return value.replace(/\/?[^\s"']*wp-codebox-runner-workspace-seed-[^\s"']*/gi, "[runner-workspace-seed]")
42+
if (Array.isArray(value)) return value.map((entry) => compact(entry))
43+
if (!value || typeof value !== "object") return value
44+
const entry = value
45+
if (key === "seed" && typeof entry.source === "string") return { kind: "runner-workspace-seed" }
46+
return Object.fromEntries(Object.entries(entry).map(([childKey, item]) => [childKey, compact(item, childKey)]))
47+
}
48+
return `${JSON.stringify(compact(JSON.parse(text)), null, 2)}\n`
49+
} catch {
50+
return text
51+
}
52+
}

.github/scripts/run-agent-task/build-codebox-task-request.mjs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,24 @@ function commandList(name) {
5959
if (entry.description !== undefined && (typeof entry.description !== "string" || !entry.description.trim())) {
6060
throw new Error(`${name}[${index}].description must be a non-empty string when provided.`)
6161
}
62-
return { command: entry.command.trim(), description: typeof entry.description === "string" ? entry.description.trim() : entry.command.trim() }
62+
let artifact
63+
if (entry.artifact !== undefined) {
64+
if (!entry.artifact || typeof entry.artifact !== "object" || Array.isArray(entry.artifact)) {
65+
throw new Error(`${name}[${index}].artifact must be an object with non-empty name, type, and path strings.`)
66+
}
67+
artifact = Object.fromEntries(["name", "type", "path"].map((key) => {
68+
const value = entry.artifact[key]
69+
if (typeof value !== "string" || !value.trim()) {
70+
throw new Error(`${name}[${index}].artifact.${key} must be a non-empty string.`)
71+
}
72+
return [key, value.trim()]
73+
}))
74+
}
75+
return {
76+
command: entry.command.trim(),
77+
description: typeof entry.description === "string" ? entry.description.trim() : entry.command.trim(),
78+
...(artifact ? { artifact } : {}),
79+
}
6380
})
6481
}
6582

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

Lines changed: 165 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
1-
import { rmSync } from "node:fs"
2-
import { appendFile, lstat, mkdir, readFile, realpath, rm, writeFile } from "node:fs/promises"
1+
import { constants, rmSync } from "node:fs"
2+
import { appendFile, lstat, mkdir, open, readFile, realpath, rm, writeFile } from "node:fs/promises"
33
import { isUtf8 } from "node:buffer"
44
import { isAbsolute, join, relative, resolve } from "node:path"
55
import { pathToFileURL } from "node:url"
66
import { spawn } from "node:child_process"
77
import { createHash } from "node:crypto"
88
import { canonicalExternalNativeAgentIdentity, materializeExternalNativePackage, materializeRuntimeSources, normalizeExternalPackageSource, normalizeRuntimeSources, parseExternalPackageSourcePolicy, sha256BytesV1, validateRuntimeSourceModel } from "./materialize-external-native-package.mjs"
99
import { readNativeResult } from "./native-result-file.mjs"
10-
import { assertNoRuntimeSourcePaths, sanitizeRuntimeSourceJson, sanitizeRuntimeSourceText, sanitizeRuntimeSourceValue } from "./runtime-source-sanitizer.mjs"
10+
import { assertNoRuntimeSourcePaths, sanitizeArtifactUploadText, sanitizeRuntimeSourceJson, sanitizeRuntimeSourceText, sanitizeRuntimeSourceValue } from "./runtime-source-sanitizer.mjs"
1111
import { publishRunnerWorkspace } from "./runner-workspace-publisher.mjs"
1212
import { createRunnerWorkspaceSeedSnapshot, RUNNER_WORKSPACE_SEED_EXCLUDES } from "./runner-workspace-seed-snapshot.mjs"
1313
import { createTrustedArtifactApplyChannel, trustedArtifactApplyRefs } from "./trusted-artifact-snapshot.mjs"
14+
import { artifactSourcePathCategory, assertNoSeedSnapshotPaths, containsRuntimeSourceContent, sanitizeSeedSnapshotJson } from "./artifact-upload-policy.mjs"
1415

1516
const requestPath = process.env.AGENT_TASK_REQUEST_PATH || ".codebox/agent-task-request.json"
1617
const workspace = resolve(process.env.AGENT_TASK_WORKSPACE || process.cwd())
@@ -19,6 +20,7 @@ const codeboxCliPath = process.env.WP_CODEBOX_CLI_PATH || join(codeboxRoot, "pac
1920
const outputPath = process.env.GITHUB_OUTPUT
2021
const MAX_CAPTURE_BYTES = 32768
2122
const MAX_WORKFLOW_OUTPUT_BYTES = 8192
23+
const MAX_COMMAND_ARTIFACT_BYTES = 4 * 1024 * 1024
2224
const secretValues = ["OPENAI_API_KEY", "MODEL_PROVIDER_SECRET_1", "MODEL_PROVIDER_SECRET_2", "MODEL_PROVIDER_SECRET_3", "MODEL_PROVIDER_SECRET_4", "MODEL_PROVIDER_SECRET_5", "GITHUB_TOKEN", "GH_TOKEN", "ACCESS_TOKEN", "EXTERNAL_PACKAGE_SOURCE_POLICY"].map((name) => process.env[name]).filter(Boolean)
2325
let privateRuntimeSourceRoot = ""
2426
let privateRuntimeSourceRootForSanitization = ""
@@ -254,7 +256,16 @@ function commandEntries(value, name) {
254256
if (!command) throw new Error(`${name}[${index}].command must be a non-empty string.`)
255257
const description = check.description === undefined ? command : string(check.description)
256258
if (!description) throw new Error(`${name}[${index}].description must be a non-empty string when provided.`)
257-
return { command, description }
259+
let artifact
260+
if (check.artifact !== undefined) {
261+
const output = record(check.artifact)
262+
artifact = Object.fromEntries(["name", "type", "path"].map((key) => {
263+
const value = string(output[key])
264+
if (!value) throw new Error(`${name}[${index}].artifact.${key} must be a non-empty string.`)
265+
return [key, value]
266+
}))
267+
}
268+
return { command, description, ...(artifact ? { artifact } : {}) }
258269
})
259270
}
260271

@@ -339,6 +350,151 @@ function requiredArtifacts(declarations) {
339350
})))
340351
}
341352

353+
function commandArtifactFailure(code, message, declaration) {
354+
const error = new Error(message)
355+
error.code = `wp-codebox.agent-task.command-artifact-${code}`
356+
error.artifact = declaration
357+
return error
358+
}
359+
360+
function commandArtifactError(error, declaration) {
361+
return {
362+
code: typeof error?.code === "string" ? error.code : "wp-codebox.agent-task.command-artifact-validation",
363+
message: bounded(error instanceof Error ? error.message : String(error), MAX_WORKFLOW_OUTPUT_BYTES),
364+
...declaration,
365+
}
366+
}
367+
368+
function safeCommandArtifactPath(value) {
369+
const path = string(value).replaceAll("\\", "/").replace(/^\.\//, "")
370+
if (!path || isAbsolute(path) || /^[A-Za-z]:\//.test(path) || path.split("/").some((part) => !part || part === "." || part === "..")) return ""
371+
return path
372+
}
373+
374+
function commandArtifactAuthorization(declaration, artifactDeclarations) {
375+
const outputs = (Array.isArray(artifactDeclarations) ? artifactDeclarations : []).map(record).filter((entry) => entry.direction !== "input")
376+
const match = outputs.find((entry) => string(entry.name) === declaration.name && string(entry.type) === declaration.type)
377+
if (match) return match
378+
const mismatch = outputs.some((entry) => string(entry.name) === declaration.name || string(entry.type) === declaration.type)
379+
throw commandArtifactFailure(mismatch ? "mismatch" : "undeclared", mismatch
380+
? `Command artifact ${declaration.name} (${declaration.type}) does not match its request artifact declaration.`
381+
: `Command artifact ${declaration.name} (${declaration.type}) is not declared by the request.`, declaration)
382+
}
383+
384+
async function readBoundedFile(handle, limit) {
385+
const bytes = Buffer.alloc(limit + 1)
386+
let offset = 0
387+
while (offset < bytes.length) {
388+
const { bytesRead } = await handle.read(bytes, offset, bytes.length - offset, offset)
389+
if (bytesRead === 0) break
390+
offset += bytesRead
391+
}
392+
return bytes.subarray(0, offset)
393+
}
394+
395+
async function commandArtifactReference(declaration, artifactDeclarations, artifactsPath, kind) {
396+
const authorized = commandArtifactAuthorization(declaration, artifactDeclarations)
397+
const path = safeCommandArtifactPath(declaration.path)
398+
if (!path) throw commandArtifactFailure("path", "Command artifact path must be artifact-relative and must not contain traversal.", declaration)
399+
if (artifactSourcePathCategory(path)) throw commandArtifactFailure("forbidden", `Command artifact ${path} is a source path that cannot be uploaded.`, declaration)
400+
401+
const rootMetadata = await lstat(artifactsPath)
402+
if (!rootMetadata.isDirectory() || rootMetadata.isSymbolicLink()) throw commandArtifactFailure("symlink", "Command artifact root must be a regular directory and must not be a symlink.", declaration)
403+
const canonicalRoot = await realpath(artifactsPath)
404+
const source = resolve(artifactsPath, path)
405+
if (!underRoot(artifactsPath, source)) throw commandArtifactFailure("path", "Command artifact path escapes the artifact root.", declaration)
406+
let current = artifactsPath
407+
let finalMetadata
408+
for (const part of path.split("/")) {
409+
current = join(current, part)
410+
let metadata
411+
try {
412+
metadata = await lstat(current)
413+
} catch (error) {
414+
if (error?.code === "ENOENT") throw commandArtifactFailure("missing", `Command artifact ${path} was not created.`, declaration)
415+
throw error
416+
}
417+
if (metadata.isSymbolicLink()) throw commandArtifactFailure("symlink", `Command artifact ${path} must not traverse symlinks.`, declaration)
418+
finalMetadata = metadata
419+
}
420+
if (!finalMetadata?.isFile()) throw commandArtifactFailure("not-regular", `Command artifact ${path} must be a regular file.`, declaration)
421+
if (finalMetadata.size > MAX_COMMAND_ARTIFACT_BYTES) throw commandArtifactFailure("too-large", `Command artifact ${path} exceeds the ${MAX_COMMAND_ARTIFACT_BYTES}-byte limit.`, declaration)
422+
const canonicalSource = await realpath(source)
423+
if (!underRoot(canonicalRoot, canonicalSource)) throw commandArtifactFailure("symlink", `Command artifact ${path} must not traverse symlinks.`, declaration)
424+
425+
const handle = await open(source, constants.O_RDONLY | constants.O_NOFOLLOW).catch((error) => {
426+
if (error?.code === "ELOOP") throw commandArtifactFailure("symlink", `Command artifact ${path} must not be a symlink.`, declaration)
427+
if (error?.code === "ENOENT") throw commandArtifactFailure("missing", `Command artifact ${path} was not created.`, declaration)
428+
throw error
429+
})
430+
let bytes
431+
try {
432+
const metadata = await handle.stat()
433+
const pathMetadata = await lstat(source)
434+
const canonicalOpenedSource = await realpath(source)
435+
if (metadata.dev !== pathMetadata.dev || metadata.ino !== pathMetadata.ino || !underRoot(canonicalRoot, canonicalOpenedSource)) {
436+
throw commandArtifactFailure("symlink", `Command artifact ${path} changed while it was being validated.`, declaration)
437+
}
438+
if (!metadata.isFile()) throw commandArtifactFailure("not-regular", `Command artifact ${path} must be a regular file.`, declaration)
439+
if (metadata.size > MAX_COMMAND_ARTIFACT_BYTES) throw commandArtifactFailure("too-large", `Command artifact ${path} exceeds the ${MAX_COMMAND_ARTIFACT_BYTES}-byte limit.`, declaration)
440+
bytes = await readBoundedFile(handle, MAX_COMMAND_ARTIFACT_BYTES)
441+
if (bytes.length > MAX_COMMAND_ARTIFACT_BYTES) throw commandArtifactFailure("too-large", `Command artifact ${path} exceeds the ${MAX_COMMAND_ARTIFACT_BYTES}-byte limit.`, declaration)
442+
if (bytes.includes(0) || !isUtf8(bytes)) throw commandArtifactFailure("malformed", `Command artifact ${path} must be a UTF-8 text file without NUL bytes.`, declaration)
443+
444+
const sanitizationRoots = [...(Array.isArray(privateRuntimeSourceRootForSanitization) ? privateRuntimeSourceRootForSanitization : [privateRuntimeSourceRootForSanitization]), workspace].filter(Boolean)
445+
const sanitized = sanitizeSeedSnapshotJson(sanitizeArtifactUploadText(bytes.toString("utf8"), sanitizationRoots, secretValues))
446+
assertNoRuntimeSourcePaths(sanitized, sanitizationRoots)
447+
assertNoSeedSnapshotPaths(sanitized)
448+
if (containsRuntimeSourceContent(sanitized)) throw commandArtifactFailure("forbidden", `Command artifact ${path} contains source content that cannot be uploaded.`, declaration)
449+
const sanitizedBytes = Buffer.from(sanitized)
450+
if (sanitizedBytes.length > MAX_COMMAND_ARTIFACT_BYTES) throw commandArtifactFailure("too-large", `Command artifact ${path} exceeds the ${MAX_COMMAND_ARTIFACT_BYTES}-byte limit after sanitization.`, declaration)
451+
return {
452+
schema: "wp-codebox/typed-artifact/v1",
453+
name: declaration.name,
454+
type: declaration.type,
455+
metadata: {},
456+
provenance: { direction: "output", source: `runner-${kind}-command` },
457+
artifact: {
458+
path,
459+
kind: "typed-artifact",
460+
contentType: string(authorized.contentType ?? authorized.content_type) || (path.endsWith(".json") ? "application/json" : "text/plain"),
461+
sha256: createHash("sha256").update(sanitizedBytes).digest("hex"),
462+
},
463+
}
464+
} finally {
465+
await handle.close()
466+
}
467+
}
468+
469+
async function verificationRecord(kind, check, artifactsPath, artifactDeclarations) {
470+
const checkResult = await command("bash", ["-lc", check.command], workspace)
471+
const result = { kind, command: check.command, description: check.description, success: checkResult.code === 0, exit_code: checkResult.code, stdout: checkResult.stdout, stderr: checkResult.stderr, stdout_truncated: checkResult.stdout_truncated, stderr_truncated: checkResult.stderr_truncated }
472+
if (checkResult.code !== 0 || !check.artifact) return result
473+
try {
474+
return { ...result, artifact: await commandArtifactReference(check.artifact, artifactDeclarations, artifactsPath, kind) }
475+
} catch (error) {
476+
return { ...result, success: false, artifact_error: commandArtifactError(error, check.artifact) }
477+
}
478+
}
479+
480+
async function finalizeCommandArtifacts(verification, artifactsPath, artifactDeclarations) {
481+
for (const check of verification.filter((entry) => entry.artifact)) {
482+
const accepted = check.artifact
483+
const declaration = { name: accepted.name, type: accepted.type, path: accepted.artifact.path }
484+
try {
485+
const current = await commandArtifactReference(declaration, artifactDeclarations, artifactsPath, check.kind)
486+
if (current.artifact.sha256 !== accepted.artifact.sha256) {
487+
throw commandArtifactFailure("changed", `Command artifact ${declaration.path} changed after it was validated.`, declaration)
488+
}
489+
check.artifact = current
490+
} catch (error) {
491+
delete check.artifact
492+
check.success = false
493+
check.artifact_error = commandArtifactError(error, declaration)
494+
}
495+
}
496+
}
497+
342498
async function testRuntimeFixtures(externalPackageSource) {
343499
if (process.env.NODE_ENV !== "test") return {}
344500
const packagePath = string(process.env.WP_CODEBOX_TEST_EXTERNAL_PACKAGE_PATH)
@@ -668,18 +824,18 @@ if (execution.code === 0 && request.run_agent && !request.dry_run && !downstream
668824
verification.push({ kind: "validation_dependencies", command: validationDependencies, description: "Install validation dependencies", success: checkResult.code === 0, exit_code: checkResult.code, stdout: checkResult.stdout, stderr: checkResult.stderr, stdout_truncated: checkResult.stdout_truncated, stderr_truncated: checkResult.stderr_truncated })
669825
}
670826
for (const check of verificationCommands) {
671-
const checkResult = await command("bash", ["-lc", check.command], workspace)
672-
verification.push({ kind: "verification", ...check, success: checkResult.code === 0, exit_code: checkResult.code, stdout: checkResult.stdout, stderr: checkResult.stderr, stdout_truncated: checkResult.stdout_truncated, stderr_truncated: checkResult.stderr_truncated })
827+
verification.push(await verificationRecord("verification", check, artifactsPath, request.artifacts?.declarations))
673828
}
674829
for (const check of driftChecks) {
675-
const checkResult = await command("bash", ["-lc", check.command], workspace)
676-
verification.push({ kind: "drift", ...check, success: checkResult.code === 0, exit_code: checkResult.code, stdout: checkResult.stdout, stderr: checkResult.stderr, stdout_truncated: checkResult.stdout_truncated, stderr_truncated: checkResult.stderr_truncated })
830+
verification.push(await verificationRecord("drift", check, artifactsPath, request.artifacts?.declarations))
677831
}
832+
await finalizeCommandArtifacts(verification, artifactsPath, request.artifacts?.declarations)
678833
}
679834

680835
const verificationPassed = verification.every((check) => check.success)
681836
if (!verificationPassed) {
682-
downstreamFailure ??= { stage: "verification", message: "Runner workspace verification did not pass." }
837+
const artifactError = verification.find((check) => check.artifact_error)?.artifact_error
838+
downstreamFailure ??= { stage: "verification", message: artifactError?.message || "Runner workspace verification did not pass.", ...(artifactError ? { artifact_error: artifactError } : {}) }
683839
}
684840
const runtimeRecord = record(runtimeResult)
685841
const agentResult = record(runtimeRecord.agent_task_run_result)

0 commit comments

Comments
 (0)