fix: prevent worker artifact directory collisions (#3392)

* fix(workers): share verified artifact materialization

* fix(ci): restore prepublication batch limit
This commit is contained in:
Vincent Koc
2026-08-04 14:21:00 +08:00
committed by GitHub
parent 87ca030c30
commit 00dd3c3055
7 changed files with 155 additions and 53 deletions
@@ -9,7 +9,7 @@ on:
batch-limit:
description: "Maximum staged publish attempts to check per worker shard"
required: true
default: "4"
default: "2"
max-jobs:
description: "Optional total attempts cap per worker shard"
required: false
@@ -65,7 +65,7 @@ jobs:
shard: ${{ fromJSON((github.event_name == 'repository_dispatch' || (github.event_name == 'workflow_dispatch' && inputs['attempt-id'] != '')) && '[0]' || '[0,1]') }}
env:
CONVEX_URL: ${{ vars.CONVEX_URL || vars.VITE_CONVEX_URL || 'https://wry-manatee-359.convex.cloud' }}
PREPUBLICATION_CHECK_LIMIT: ${{ github.event.client_payload.batch_limit || inputs['batch-limit'] || '4' }}
PREPUBLICATION_CHECK_LIMIT: ${{ github.event.client_payload.batch_limit || inputs['batch-limit'] || '2' }}
PREPUBLICATION_CHECK_MAX_JOBS: ${{ github.event.client_payload.max_jobs || inputs['max-jobs'] || '' }}
PREPUBLICATION_CHECK_MAX_RUNTIME_MINUTES: ${{ github.event.client_payload.max_runtime_minutes || inputs['max-runtime-minutes'] || '15' }}
PREPUBLICATION_CHECK_ATTEMPT_ID: ${{ github.event.client_payload.attempt_id || inputs['attempt-id'] || '' }}
+1
View File
@@ -9,6 +9,7 @@
### Fixes
- Workers: materialize zero-byte directory markers without colliding with descendant files, while retaining real empty files and verifying every downloaded artifact digest.
- Deploy: allow an explicitly confirmed backend-only deploy to pause and reliably restore active external-skill rollouts instead of requiring a manual dashboard toggle.
- GitHub Actions/CLI: trigger exact pre-publication checks immediately and wait for package publication to finish, so a pending staged upload no longer reports a successful release.
- API: keep publish-time Plugin Inspector target preparation inside its disposable workspace when hosted runtimes expose an unusable home directory.
+52
View File
@@ -0,0 +1,52 @@
import { createHash } from "node:crypto";
import { mkdir, writeFile } from "node:fs/promises";
import { dirname, resolve, sep } from "node:path";
import { safeWorkerArtifactPathLabel } from "./workerRedaction";
type ArtifactFile = {
path: string;
sha256: string;
size: number;
};
function safeArtifactOutputPath(artifactRoot: string, artifactPath: string) {
const normalized = artifactPath.replace(/^\/+/, "");
const out = resolve(artifactRoot, normalized);
const root = resolve(artifactRoot);
if (!out.startsWith(`${root}${sep}`) && out !== root) {
throw new Error(`Unsafe artifact path: ${safeWorkerArtifactPathLabel(artifactPath)}`);
}
return out;
}
export async function materializeVerifiedArtifactFiles<T extends ArtifactFile>(input: {
artifactRoot: string;
download: (file: T) => Promise<Uint8Array>;
files: T[];
}) {
const candidates = input.files.map((file) => ({
file,
out: safeArtifactOutputPath(input.artifactRoot, file.path),
}));
for (const candidate of candidates) {
// Zero-byte entries are directories only when another stored path proves it.
// Otherwise they are real empty files and must still be downloaded and verified.
const isDirectoryMarker =
candidate.file.size === 0 &&
candidates.some(
(other) => other.out !== candidate.out && other.out.startsWith(`${candidate.out}${sep}`),
);
if (isDirectoryMarker) continue;
const bytes = await input.download(candidate.file);
const actualSha256 = createHash("sha256").update(bytes).digest("hex");
if (actualSha256 !== candidate.file.sha256.toLowerCase()) {
throw new Error(
`Downloaded artifact hash mismatch for artifact file ${safeWorkerArtifactPathLabel(candidate.file.path)}`,
);
}
await mkdir(dirname(candidate.out), { recursive: true });
await writeFile(candidate.out, bytes);
}
}
@@ -42,6 +42,10 @@ describe("pre-publication publish worker workflow", () => {
schedule?: Array<{ cron?: string }>;
workflow_dispatch?: {
inputs?: {
"batch-limit"?: {
default?: string;
required?: boolean;
};
"attempt-id"?: {
default?: string;
required?: boolean;
@@ -73,6 +77,10 @@ describe("pre-publication publish worker workflow", () => {
expect(workflow.on?.repository_dispatch?.types).toEqual(["clawhub-prepublication-publish"]);
expect(workflow.on?.schedule?.[0]?.cron).toBe("*/5 * * * *");
expect(workflow.on?.workflow_dispatch).toBeDefined();
expect(workflow.on?.workflow_dispatch?.inputs?.["batch-limit"]).toMatchObject({
required: true,
default: "2",
});
expect(workflow.on?.workflow_dispatch?.inputs?.runner).toEqual({
description: "Runner label for manual recovery dispatches",
required: true,
@@ -111,7 +119,7 @@ describe("pre-publication publish worker workflow", () => {
PREPUBLICATION_CHECK_ATTEMPT_ID:
"${{ github.event.client_payload.attempt_id || inputs['attempt-id'] || '' }}",
PREPUBLICATION_CHECK_LIMIT:
"${{ github.event.client_payload.batch_limit || inputs['batch-limit'] || '4' }}",
"${{ github.event.client_payload.batch_limit || inputs['batch-limit'] || '2' }}",
PREPUBLICATION_CHECK_KIND: "${{ github.event.client_payload.kind || inputs.kind || '' }}",
PREPUBLICATION_CHECK_MAX_JOBS:
"${{ github.event.client_payload.max_jobs || inputs['max-jobs'] || '' }}",
+7 -35
View File
@@ -1,15 +1,15 @@
import { spawn } from "node:child_process";
import { createHash } from "node:crypto";
import { mkdirSync, readFileSync } from "node:fs";
import { appendFile, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { basename, dirname, join, resolve, sep } from "node:path";
import { basename, dirname, join, resolve } from "node:path";
import { pathToFileURL } from "node:url";
import { ConvexHttpClient } from "convex/browser";
import { api } from "../../convex/_generated/api";
import type { Id } from "../../convex/_generated/dataModel";
import { parseLlmEvalResponse, type LlmEvalDimension } from "../../convex/lib/securityPrompt";
import { assertCodexWorkerExecutionAllowed, resolveCodexWorkerHome } from "../codex-worker-guard";
import { materializeVerifiedArtifactFiles } from "../lib/artifactMaterialization";
import { createWorkerLogger } from "../lib/workerLogger";
import {
maskGitHubActionsSecret,
@@ -703,16 +703,6 @@ export async function writeJobDiagnostic(input: JobDiagnosticInput) {
await writeFile(join(jobDir, "diagnostic.json"), `${JSON.stringify(diagnostic, null, 2)}\n`);
}
function safeOutputPath(workspace: string, artifactPath: string) {
const normalized = artifactPath.replace(/^\/+/, "");
const out = resolve(workspace, "artifact", normalized);
const artifactRoot = resolve(workspace, "artifact");
if (!out.startsWith(`${artifactRoot}/`) && out !== artifactRoot) {
throw new Error(`Unsafe artifact path: ${safeWorkerArtifactPathLabel(artifactPath)}`);
}
return out;
}
function artifactDownloadDescription(kind: "file" | "clawpack", artifactPath: string) {
const safePath = safeWorkerArtifactPathLabel(artifactPath);
return kind === "file" ? `artifact file ${safePath}` : `artifact tarball ${safePath}`;
@@ -748,29 +738,11 @@ export async function writeArtifactWorkspace(job: ClaimedJob, workspace: string)
};
await writeFile(join(workspace, "metadata.json"), `${JSON.stringify(metadata, null, 2)}\n`);
const files = (job.target.files ?? []).map((file) => ({
file,
out: safeOutputPath(workspace, file.path),
}));
for (const candidate of files) {
const isDirectoryMarker =
candidate.file.size === 0 &&
files.some(
(other) => other.out !== candidate.out && other.out.startsWith(`${candidate.out}${sep}`),
);
if (isDirectoryMarker) continue;
const { file, out } = candidate;
await mkdir(dirname(out), { recursive: true });
const bytes = await download(file.url, { kind: "file", path: file.path });
const actualSha256 = createHash("sha256").update(bytes).digest("hex");
if (actualSha256 !== file.sha256.toLowerCase()) {
throw new Error(
`Downloaded artifact hash mismatch for artifact file ${safeWorkerArtifactPathLabel(file.path)}`,
);
}
await writeFile(out, bytes);
}
await materializeVerifiedArtifactFiles({
artifactRoot: join(workspace, "artifact"),
files: job.target.files ?? [],
download: async (file) => await download(file.url, { kind: "file", path: file.path }),
});
// Legacy ZIP releases are already materialized above as individually verified files.
// Their archival copy is not an npm tarball and makes GNU tar fail before scanning.
@@ -1,4 +1,5 @@
/* @vitest-environment node */
import { createHash } from "node:crypto";
import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
@@ -250,6 +251,83 @@ describe("run-skill-card-worker Codex skill setup", () => {
fetchMock.mockRestore();
});
it("materializes directory markers, descendants, and real empty files", async () => {
const workspace = await tempDir();
const openAiConfig = "provider: openai\n";
const emptySha256 = createHash("sha256").update("").digest("hex");
await writeWorkspace(
{
job: {
_id: "skillCardGenerationJobs:directory-marker",
leaseToken: "lease-secret",
source: "scan",
},
target: {
evidence: {},
files: [
{
path: "agents",
sha256: emptySha256,
size: 0,
url: "data:application/octet-stream,",
},
{
path: "agents/openai.yaml",
sha256: createHash("sha256").update(openAiConfig).digest("hex"),
size: Buffer.byteLength(openAiConfig),
url: `data:text/plain,${encodeURIComponent(openAiConfig)}`,
},
{
path: "EMPTY",
sha256: emptySha256,
size: 0,
url: "data:application/octet-stream,",
},
],
skill: { displayName: "Demo Skill", slug: "demo-skill" },
version: { version: "1.2.3" },
},
},
workspace,
);
await expect(
readFile(join(workspace, "artifact", "agents", "openai.yaml"), "utf8"),
).resolves.toBe(openAiConfig);
await expect(readFile(join(workspace, "artifact", "EMPTY"))).resolves.toHaveLength(0);
});
it("rejects downloaded artifact bytes that do not match the stored hash", async () => {
const workspace = await tempDir();
await expect(
writeWorkspace(
{
job: {
_id: "skillCardGenerationJobs:hash-mismatch",
leaseToken: "lease-secret",
source: "scan",
},
target: {
evidence: {},
files: [
{
path: "SKILL.md",
sha256: "0".repeat(64),
size: 7,
url: "data:text/plain,%23%20Skill",
},
],
skill: { displayName: "Demo Skill", slug: "demo-skill" },
version: { version: "1.2.3" },
},
},
workspace,
),
).rejects.toThrow("Downloaded artifact hash mismatch for artifact file SKILL.md");
});
it("sanitizes download failures before logging or failing the Convex job", async () => {
const previousGitHubActions = process.env.GITHUB_ACTIONS;
process.env.GITHUB_ACTIONS = "true";
+6 -15
View File
@@ -8,6 +8,7 @@ import { ConvexHttpClient } from "convex/browser";
import { api } from "../../convex/_generated/api";
import type { Id } from "../../convex/_generated/dataModel";
import { assertCodexWorkerExecutionAllowed, resolveCodexWorkerHome } from "../codex-worker-guard";
import { materializeVerifiedArtifactFiles } from "../lib/artifactMaterialization";
import { createWorkerLogger } from "../lib/workerLogger";
import {
maskGitHubActionsSecret,
@@ -131,16 +132,6 @@ export function skillCardWorkerId(env: NodeJS.ProcessEnv = process.env) {
);
}
function safeOutputPath(workspace: string, artifactPath: string) {
const normalized = artifactPath.replace(/^\/+/, "");
const out = resolve(workspace, "artifact", normalized);
const artifactRoot = resolve(workspace, "artifact");
if (!out.startsWith(`${artifactRoot}/`) && out !== artifactRoot) {
throw new Error(`Unsafe artifact path: ${safeWorkerArtifactPathLabel(artifactPath)}`);
}
return out;
}
function artifactDownloadDescription(artifactPath: string) {
return `artifact file ${safeWorkerArtifactPathLabel(artifactPath)}`;
}
@@ -352,11 +343,11 @@ export async function writeWorkspace(job: ClaimedSkillCardJob, workspace: string
join(workspace, "evidence.json"),
`${JSON.stringify(job.target.evidence, null, 2)}\n`,
);
for (const file of job.target.files) {
const out = safeOutputPath(workspace, file.path);
await mkdir(dirname(out), { recursive: true });
await writeFile(out, await download(file.url, { path: file.path }));
}
await materializeVerifiedArtifactFiles({
artifactRoot: join(workspace, "artifact"),
files: job.target.files,
download: async (file) => await download(file.url, { path: file.path }),
});
}
async function generateSkillCardWithCodex(