Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 140 additions & 12 deletions .github/scripts/resolve-sandbox-image.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -73,55 +73,177 @@ async function fetchLatestGhcrSemver() {
return latest;
}

function pullImage(command, image) {
// The `Digest: sha256:…` line docker prints for the tag it just resolved is
// the only pull-time content identity: the post-pull inspect can race a
// `docker tag` swap (see repoDigestOf), so the exported reference must be
// bound to what the pull itself reported, never to inspect alone.
export function parsePullDigest(pullOutput) {
return pullOutput.match(/^Digest: (sha256:[0-9a-f]{64})\s*$/m)?.[1] ?? '';
}

export function pullImage(command, image) {
return new Promise((resolve) => {
const child = spawn(command, ['pull', image], { stdio: 'inherit' });
const child = spawn(command, ['pull', image], {
stdio: ['ignore', 'pipe', 'inherit'],
});
let stdout = '';
let settled = false;
let timer;
const finish = (ok) => {
const finish = (result) => {
if (settled) return;
settled = true;
clearTimeout(timer);
resolve(ok);
resolve(result);
};
timer = setTimeout(() => {
console.error(
`::error::Timed out pulling ${image} after ${PULL_TIMEOUT_MS / 1000}s.`,
);
child.kill('SIGKILL');
finish(false);
finish({ ok: false, digest: '' });
}, PULL_TIMEOUT_MS);

child.stdout.on('data', (chunk) => {
stdout += chunk;
process.stdout.write(chunk);
});
child.on('error', (error) => {
console.error(
`::error::Failed to start '${command} pull ${image}': ${error.message}`,
);
finish(false);
finish({ ok: false, digest: '' });
});
child.on('close', (code) => {
if (code !== 0) {
console.error(
`::error::'${command} pull ${image}' exited with code ${code}.`,
);
finish({ ok: false, digest: '' });
return;
}
finish(code === 0);
finish({ ok: true, digest: parsePullDigest(stdout) });
});
});
}

function exportImage(image) {
// The repository part of an image reference: everything before the :tag /
// @digest. A registry port keeps its colon — the tag only ever follows the
// LAST '/'.
export function repoOfImage(image) {
const withoutDigest = image.split('@')[0];
const lastColon = withoutDigest.lastIndexOf(':');
const lastSlash = withoutDigest.lastIndexOf('/');
return lastColon > lastSlash
? withoutDigest.slice(0, lastColon)
: withoutDigest;
}

// Resolve a PULLED image to its content digest (repo@sha256:…). The tag
// alone is a mutable local handle: `docker run <tag>` resolves against the
// local store without re-pull, and a co-resident process with daemon access
// can `docker tag` different content under the same name between resolve
// and gate. A digest reference cannot be moved by `docker tag`/`docker build`.
// The export must be the EXACT `<repo>@<expectedDigest>` RepoDigests entry:
// RepoDigests is shared by every tag of the same content, so `docker tag`
// of the pulled image adds an alphabetically-sorted entry for the new name
// and index 0 can move OFF the pulled repo (a suffix-only digest check
// still passes) — and retagged attacker content keeps ITS original repo, so
// only the pulled repo + the pull's own `Digest:` line together bind the
// export to the content the pull fetched (#9214 review).
export function repoDigestOf(command, image, expectedDigest = '') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R11-1: The new digest binding trusts whichever daemon endpoint the docker CLI happens to resolve: the resolver's pull and image inspect spawns carry no --context/--host flag and no env pin, and the 'Resolve sandbox image' step (yml:4648-4657) has no env: block — so $HOME/.docker/config.json currentContext on the shared attacker-writable HOME steers both identity queries to a rogue endpoint. The empty-string pin shape (DOCKER_CONTEXT= DOCKER_HOST=, the gate steps' own form) STILL falls through to config.json, so this arm survives even that pin shape; closing it needs a flag bind, a non-empty context value, or HOME/config isolation. Sibling entrance of the R9-9 mechanism family at the resolver's call sites.

Failure scenario: A cross-round survivor (the resolver runs BEFORE branch checkout, so the plant is necessarily cross-round) writes {"currentContext":"evil"} + context meta pointing at a socket it listens on; the rogue endpoint answers docker pull with a forged Digest: sha256: (X = any older published digest, pullable with zero privileges per R9-8) and inspect with matching RepoDigests — both halves of the exact-match pair are attacker-supplied, so the honest script itself exports the attacker's reference and the gate's verdict, which gates the PAT push, is produced in the attacker-chosen environment.

Witness: Executed this round on docker 29.1.3: docker context show -> evil; docker pull / docker image inspect (the resolver's exact invocation shape) -> failed to connect to the docker API at unix:///tmp/p11-9-evil.sock; with DOCKER_CONTEXT= DOCKER_HOST= (gate-step pin shape) -> STILL dials evil.sock; control DOCKER_CONTEXT=default -> default.

Suggested fix: Pin the resolver's docker endpoint: give the sandbox step env: DOCKER_HOST/DOCKER_CONTEXT plus a fresh DOCKER_CONFIG (mktemp -d) so a planted config.json/contexts tree is unreadable, or pass --context default with DOCKER_CONFIG= through the spawn env in pullImage/repoDigestOf.

中文说明

新的 digest 绑定信任 docker CLI 恰好解析到的 daemon 端点:resolver 的 pull 和 image inspect spawn 不带 --context/--host 旗标、无 env 钉住,'Resolve sandbox image' 步骤(yml:4648-4657)根本没有 env: 块——共享攻击者可写 HOME 的 $HOME/.docker/config.json currentContext 把两个身份查询都导向恶意端点;空字符串钉住形态(DOCKER_CONTEXT= DOCKER_HOST=,正是门步骤自己的形态)仍然落入 config.json 回退,因此该臂连空钉住形态都能绕过;关闭它需要旗标绑定、非空 context 值或 HOME/config 隔离。这是 R9-9 机制家族在 resolver 调用点的兄弟入口。失败场景:跨轮残留(resolver 在分支 checkout 之前运行,植入必然是跨轮的)写入 currentContext=evil + 上下多元数据指向其监听的 socket;恶意端点对 docker pull 应答伪造的 Digest: sha256:(X = 任何更旧的已发布 digest,按 R9-8 零权限可拉取)、对 inspect 应答匹配的 RepoDigests——精确匹配对的两半都由攻击者提供,诚实脚本自己导出攻击者的引用,把关 PAT 推送的门裁决产生于攻击者选定的环境。

— qwen3.8-max via Qwen Code /review (v0.21.13)

return new Promise((resolve) => {
const child = spawn(
command,
['image', 'inspect', '--format', '{{json .RepoDigests}}', image],
{ stdio: ['ignore', 'pipe', 'pipe'] },
);
let stdout = '';
let settled = false;
let timer;
const finish = (value) => {
if (settled) return;
settled = true;
clearTimeout(timer);
resolve(value);
};
timer = setTimeout(() => {
console.error(
`::error::Timed out inspecting ${image} after ${FETCH_TIMEOUT_MS / 1000}s.`,
);
child.kill('SIGKILL');
finish('');
}, FETCH_TIMEOUT_MS);

child.stdout.on('data', (chunk) => {
stdout += chunk;
});
child.on('error', (error) => {
console.error(
`::error::Failed to start '${command} image inspect ${image}': ${error.message}`,
);
finish('');
});
child.on('close', (code) => {
finish(code === 0 ? stdout.trim() : '');
});
}).then((raw) => {
// `null`/`[]` (no RepoDigests, a locally built image), `<no value>` and
// empty (the inspect failed) all mean there is no repository digest —
// the mutable tag is exactly what must not be exported.
let digests = [];
try {
const parsed = JSON.parse(raw.trim());
if (Array.isArray(parsed)) {
digests = parsed.filter((entry) => typeof entry === 'string');
}
} catch {
// Non-JSON output carries no digests.
}
const repo = repoOfImage(image);
const digest = expectedDigest
? digests.find((entry) => entry === `${repo}@${expectedDigest}`) ?? ''
: digests.find((entry) => entry.startsWith(`${repo}@sha256:`)) ?? '';
if (digest.includes('@sha256:')) {
return digest;
}
if (digests.length > 0) {
throw new Error(
`Pulled image ${image} resolved to digests none of which is '${repo}@${expectedDigest || 'sha256:…'}' (${digests.join(', ')}); refusing to export a foreign or mutable reference.`,
);
}
throw new Error(
`Pulled image ${image} resolved to no repository digest ('${raw.trim()}'); refusing to export a mutable tag.`,
);
});
}

export function exportImage(image) {
if (process.env.GITHUB_ENV) {
appendFileSync(process.env.GITHUB_ENV, `QWEN_SANDBOX_IMAGE=${image}\n`);
}
// Also as a step OUTPUT: $GITHUB_ENV is a file later steps can append to,
// so a consumer that must not be steered by branch code (the verification
// gate's container image) reads the expression-context value instead.
if (process.env.GITHUB_OUTPUT) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R7-18: The new image= step-output channel is protected only against appends by later steps — the runner parses $GITHUB_OUTPUT at step END with last-wins semantics (the mechanism this PR itself documents in the severing comment), so a host-resident process (cross-round residue on the self-hosted pool — the premise R7-9/R7-11 were confirmed on) can append a forged image= line DURING the sandbox step's own window (up to the 10-minute pull) and displace the honest digest-bound value before it is captured into steps.sandbox.outputs.image. Both gate steps consume it; the wrapper's only guard is [[ -z "${GATE_IMAGE:-}" ]] — no shape validation anywhere on the path (grep-verified). The gate then runs inside the attacker's image, which forges the build/test/bite verdict trivially, and the PAT push ships attacker code. Distinct from R7-9 (verdict output, post-agent) and R7-11 (ENV/PATH channels): this is the gate's IMAGE INPUT corrupted at the earliest step, through the channel this diff adds. — Failure scenario: round N plants a watcher; round N+1's resolve step appends the honest ghcr.io/qwenlm/qwen-code@sha256:…; the watcher appends image=<attacker-registry>/backdoor@sha256:<evil> before step end → GATE_IMAGE is attacker-chosen → forged outcome=fixed → PAT push.

Witness: not run — the step-end output parse is runner-internal; grep verifies no shape validation on the GATE_IMAGE path. The resolver can only export ^ghcr\.io/qwenlm/qwen-code@sha256:[0-9a-f]{64}$ shapes, so the fix below is exact.

Suggested fix: at both gate steps (and defense-in-depth in the wrapper), refuse GATE_IMAGE unless it matches ^ghcr\.io/qwenlm/qwen-code@sha256:[0-9a-f]{64}$, exiting 125 (crash retry) on any other shape — the resolver can only export that form, so the pin is exact and a forged foreign-registry reference becomes an explicit refusal.

中文说明

[Critical] R7-18:新的 image= 步骤输出通道只防御了后续步骤的追加 —— runner 在步骤结束时以 last-wins 语义解析 $GITHUB_OUTPUT(本 PR 在 severing 注释中亲自记载的机制),因此一个宿主常驻进程(自托管池上的跨轮残留,即 R7-9/R7-11 赖以确认的前提)可以在 sandbox 步骤自己的窗口期内(最长 10 分钟的拉取过程)追加一行伪造的 image=,在值被捕获进 steps.sandbox.outputs.image 之前顶掉诚实的 digest 绑定值。两个门步骤都消费该值;wrapper 唯一的防护是 [[ -z "${GATE_IMAGE:-}" ]] —— 整条路径上没有任何格式校验(grep 已验证)。随后门在攻击者镜像内运行,可以轻易伪造 build/test/bite 裁决,PAT 推送便带上攻击者代码。与 R7-9(门步骤自身输出、agent 之后)和 R7-11(ENV/PATH 通道)不同:这是门的镜像输入在最早的步骤、经由本 diff 新增的通道被污染。失败场景:第 N 轮埋下监视进程;第 N+1 轮 resolve 步骤写入诚实的 ghcr.io/qwenlm/qwen-code@sha256:…;监视进程在步骤结束前追加 image=<攻击者仓库>/backdoor@sha256:<evil> → GATE_IMAGE 被攻击者选定 → 伪造 outcome=fixed → PAT 推送。

证据:未运行 —— 步骤结束时的输出解析属于 runner 内部行为;grep 确认 GATE_IMAGE 路径上无格式校验。resolver 只可能导出 ^ghcr\.io/qwenlm/qwen-code@sha256:[0-9a-f]{64}$ 形态,因此下述修复是精确的。

建议修复:在两个门步骤(并在 wrapper 中纵深防御)拒绝不匹配 ^ghcr\.io/qwenlm/qwen-code@sha256:[0-9a-f]{64}$ 的 GATE_IMAGE,其他形态一律 exit 125(崩溃重试)。

— qwen3.8-max via Qwen Code /review (v0.21.13)

appendFileSync(process.env.GITHUB_OUTPUT, `image=${image}\n`);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R11-2: The newly added step-output write opens the runner-managed $GITHUB_OUTPUT file (under attacker-writable $RUNNER_TEMP) with a plain blocking open(O_WRONLY|O_CREAT|O_APPEND) — neither the type discipline nor the timeout this same diff installs at its digest consumers — the fifth site of the confirmed FIFO-hang class (R10-1/R10-3/R10-5/R11-7) and the first one this diff adds.

Failure scenario: A same-uid survivor replaces the 'Resolve sandbox image' step's $GITHUB_OUTPUT with a writerless FIFO during the step's multi-minute pull window (the path is created at step start; the script writes only after pull <=10min + inspect); appendFileSync blocks in open() indefinitely (POSIX: O_WRONLY on a FIFO blocks until a reader appears); the step has no timeout-minutes, so the job burns to the 300-minute cap holding the qwen-pr-head-write- concurrency group, before prepare/gate ever runs — re-plantable every round.

Witness: Executed this round (node v22.23.2): appendFileSync on a writerless FIFO -> rc=124 killed by the 2s watchdog, never returned; same call on a regular file -> rc=0, content written. Probe flips.

Suggested fix: Open non-blocking to fail fast on a FIFO: openSync(..., O_WRONLY|O_CREAT|O_APPEND|O_NONBLOCK) — ENXIO on a readerless FIFO turns a 300-minute hang into an immediate step failure.

中文说明

新增的步骤输出写入以普通阻塞 open(O_WRONLY|O_CREAT|O_APPEND) 打开 runner 管理的 $GITHUB_OUTPUT 文件(位于攻击者可写的 $RUNNER_TEMP 下)——没有同一 diff 在其 digest 消费方安装的类型纪律与 timeout——已确认 FIFO 挂起类(R10-1/R10-3/R10-5/R11-7)的第五个站点,且是本 diff 新增的第一个。失败场景:同 uid 幸存者在步骤数分钟的拉取窗口内(路径在步骤开始时创建,脚本在 pull ≤10 分钟 + inspect 之后才写入)把该步骤的 $GITHUB_OUTPUT 换成无写者 FIFO;appendFileSync 在 open() 无限阻塞(POSIX:对 FIFO 的 O_WRONLY 阻塞到有写者为止);步骤没有 timeout-minutes,job 烧到 300 分钟上限并占用 qwen-pr-head-write- 并发组,在 prepare/门之前——每轮可重复植入。建议修复:以非阻塞方式打开以便对 FIFO 快速失败:openSync(..., O_WRONLY|O_CREAT|O_APPEND|O_NONBLOCK)——对无写者 FIFO 的 ENXIO 把 300 分钟挂起变成即时步骤失败。

— qwen3.8-max via Qwen Code /review (v0.21.13)

}
Comment thread
wenshao marked this conversation as resolved.
console.log(`QWEN_SANDBOX_IMAGE=${image}`);
}

async function main() {
const requestedImage = validateRequestedImage(process.argv[2]);

const command = process.env.SANDBOX_COMMAND || 'docker';
if (await pullImage(command, requestedImage)) {
exportImage(requestedImage);
const requestedPull = await pullImage(command, requestedImage);
if (requestedPull.ok) {
if (!requestedPull.digest) {
throw new Error(
`'${command} pull ${requestedImage}' reported no Digest line; refusing to export an unbound image reference.`,
);
}
exportImage(
await repoDigestOf(command, requestedImage, requestedPull.digest),
);
return;
}

Expand All @@ -136,10 +258,16 @@ async function main() {
console.warn(
`::warning::Falling back from ${requestedImage} to latest GHCR semver ${fallbackImage}; sandbox image version may differ from package version.`,
);
if (!(await pullImage(command, fallbackImage))) {
const fallbackPull = await pullImage(command, fallbackImage);
if (!fallbackPull.ok) {
throw new Error(`Fallback sandbox image failed to pull: ${fallbackImage}`);
}
exportImage(fallbackImage);
if (!fallbackPull.digest) {
throw new Error(
`'${command} pull ${fallbackImage}' reported no Digest line; refusing to export an unbound image reference.`,
);
}
exportImage(await repoDigestOf(command, fallbackImage, fallbackPull.digest));
}

if (
Expand Down
Loading
Loading