diff --git a/.github/workflows/qwen-triage.yml b/.github/workflows/qwen-triage.yml index 68dc9b6a236..0c24246d1e6 100644 --- a/.github/workflows/qwen-triage.yml +++ b/.github/workflows/qwen-triage.yml @@ -2599,21 +2599,68 @@ jobs: run: |- set -uo pipefail WS="${GITHUB_WORKSPACE:?}" - # Refuse to run anywhere unexpected: a wipe pointed at the wrong - # path by a mangled env is far worse than a skipped wipe, and - # this job cannot proceed safely without it either way. This is - # the guard from qwen-code-pr-review.yml's checkout heal (#9220), - # backported per #9265: measured on main, the bare denylist let - # non-canonical spellings of the guarded roots through (/home/, - # /home/., //usr, /root/, /var/ all reached the rm). + # Strip trailing slashes on the RAW path, before anything reads it: + # `[ -L "$WS/" ]` and `[ ! -d "$WS/" ]` both resolve THROUGH a link + # and report its target, so one trailing slash hides the corruption + # the heal below exists to clear. + while [ "${WS%/}" != "$WS" ]; do WS="${WS%/}"; done + # The allowlist root is prepared BEFORE the heal, because it bounds + # what the heal may touch: canonical, slash-free and non-degenerate. + # An empty $RUNNER_WORKSPACE would turn every containment pattern + # below into the match-all "/*". + RWS="${RUNNER_WORKSPACE:?}" + RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } + while [ "${RWS%/}" != "$RWS" ]; do RWS="${RWS%/}"; done + if [ -z "$RWS" ]; then echo "::error::refusing to wipe: runner workspace resolved to /"; exit 1; fi + case "$RWS" in + ..|../*|*/..|*/../*) echo "::error::refusing runner workspace path containing '..': ${RWS}"; exit 1 ;; + esac + # Heal a workspace a previous job replaced with a symlink (or any + # non-directory) BEFORE canonicalizing it. Afterwards the path + # resolves to the link's target, the allowlist refuses that, and the + # refusal removes nothing — so every later job on this runner dies + # here, permanently, on corruption that is itself inside the runner + # workspace and safe to unlink. + if [ -L "$WS" ] || [ ! -d "$WS" ]; then + # Judge the PARENT, canonicalized. The heal necessarily acts on a + # raw path, and a raw containment match is not enough: the kernel + # resolves intermediate components too, so `$RWS/link/sub` matches + # "$RWS"/* as a string while naming a file outside it. Resolving + # the parent — never $WS itself, which would resolve through the + # very link being removed — is what makes the unlink containable. + HEAL_PARENT="$(realpath -m -- "$(dirname -- "$WS")" 2>/dev/null)" || { echo "::error::refusing to heal: realpath unavailable, cannot canonicalize the parent of ${WS}"; exit 1; } + case "$HEAL_PARENT" in + "$RWS"|"$RWS"/*) ;; + *) echo "::error::refusing to heal workspace outside the runner workspace: ${WS} (parent: ${HEAL_PARENT}, runner workspace: ${RWS})"; exit 1 ;; + esac + # The incident this heal exists for leaves no other trace: say what + # was found, and where it pointed, before it is gone. + if [ -L "$WS" ]; then + # The target is bytes a PREVIOUS job chose — on this pool that + # job may have run a contributor's code — and the runner parses + # `::` at the start of any stdout line as a workflow command. A + # target of $'x\n::error::forged' would therefore forge an + # annotation. Keep untrusted bytes off the command line itself, + # strip the line breaks that could start a new one, and cap the + # length. + heal_target="$(readlink -- "$WS" 2>/dev/null || printf '%s' '')" + heal_target="$(printf '%s' "$heal_target" | tr -d '\r\n' | cut -c1-200)" + echo "::warning::healing workspace ${WS}: it was a symlink" + printf 'heal: %s pointed at %s\n' "$WS" "$heal_target" + else + echo "::warning::healing workspace ${WS}: it was not a directory" + fi + # `rm -f` on the RAW path removes the link itself and never + # follows it. Both legs fail closed: under `-e` a failure that is + # not the last command of an && list is swallowed, and a swallowed + # one here would leave the wipe running against a corrupt path. + rm -f -- "$WS" || { echo "::error::refusing to continue: could not remove ${WS}"; exit 1; } + mkdir -- "$WS" || { echo "::error::refusing to continue: could not recreate ${WS}"; exit 1; } + fi # Canonicalize before matching: the kernel resolves non-canonical # spellings to the guarded roots (`/home/.` -> /home, `//usr` -> # /usr), so a raw string match lets them slip past the case arms. WS="$(realpath -m -- "$WS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${GITHUB_WORKSPACE}"; exit 1; } - # Trailing slashes slip past the exact-match case arms below - # (`/home/` would pass the guard and reach the rm); realpath strips - # them too; keep the guard whole if the path reaches this point with - # trailing slashes. while [ "${WS%/}" != "$WS" ]; do WS="${WS%/}"; done case "$WS" in ..|../*|*/..|*/../*) echo "::error::refusing to wipe path containing '..': ${WS}"; exit 1 ;; @@ -2623,19 +2670,7 @@ jobs: esac # A denylist can only enumerate known roots — the allowlist closes # every other one (/tmp, /opt, ...): only a directory inside the - # runner workspace may be wiped. RUNNER_WORKSPACE is set in every - # step env, and for container steps the runner translates it — - # together with GITHUB_WORKSPACE — to the container path, so the - # allowlist holds inside this job's container as well. - RWS="${RUNNER_WORKSPACE:?}" - RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } - # Mirror the WS strip before building the allowlist pattern; "/" - # stripped empty would match every path instead. - while [ "${RWS%/}" != "$RWS" ]; do RWS="${RWS%/}"; done - if [ -z "$RWS" ]; then echo "::error::refusing to wipe: runner workspace resolved to /"; exit 1; fi - case "$RWS" in - ..|../*|*/..|*/../*) echo "::error::refusing runner workspace path containing '..': ${RWS}"; exit 1 ;; - esac + # runner workspace may be wiped. case "$WS" in "$RWS"/*) ;; *) echo "::error::refusing to wipe workspace outside the runner workspace: ${WS} (runner workspace: ${RWS})"; exit 1 ;; @@ -4811,20 +4846,24 @@ jobs: if: "always() && needs.authorize.outputs.verify_trust == 'external'" run: |- set -uo pipefail - # Same guard as the pre-run wipe above (canonicalize, strip - # trailing slashes, denylist, RUNNER_WORKSPACE allowlist) — this - # copy predates the checkout-heal hardening and never received it - # (#9265). See that step's comments for what each layer catches; - # the suite pins this copy's behavior on its own. + # Same guard as the pre-run wipe above, layer for layer: raw + # trailing-slash strip, RUNNER_WORKSPACE allowlist root, symlink + # heal, canonicalize, strip, denylist, allowlist. Both copies now + # carry the checkout-heal hardening (#9277) and its heal (#9480); + # this header is the in-code inventory a future convergence of the + # wipe copies will read, so it must not understate what is here. + # See that step's comments for what each layer catches; the suite + # pins this copy's behavior on its own. WS="${GITHUB_WORKSPACE:?}" - WS="$(realpath -m -- "$WS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${GITHUB_WORKSPACE}"; exit 1; } + # Strip trailing slashes on the RAW path, before anything reads it: + # `[ -L "$WS/" ]` and `[ ! -d "$WS/" ]` both resolve THROUGH a link + # and report its target, so one trailing slash hides the corruption + # the heal below exists to clear. while [ "${WS%/}" != "$WS" ]; do WS="${WS%/}"; done - case "$WS" in - ..|../*|*/..|*/../*) echo "::error::refusing to wipe path containing '..': ${WS}"; exit 1 ;; - esac - case "$WS" in - /|/home|/root|/usr*|/etc*|/var|"") echo "::error::refusing to wipe suspicious workspace path: ${WS}"; exit 1 ;; - esac + # The allowlist root is prepared BEFORE the heal, because it bounds + # what the heal may touch: canonical, slash-free and non-degenerate. + # An empty $RUNNER_WORKSPACE would turn every containment pattern + # below into the match-all "/*". RWS="${RUNNER_WORKSPACE:?}" RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } while [ "${RWS%/}" != "$RWS" ]; do RWS="${RWS%/}"; done @@ -4832,6 +4871,62 @@ jobs: case "$RWS" in ..|../*|*/..|*/../*) echo "::error::refusing runner workspace path containing '..': ${RWS}"; exit 1 ;; esac + # Heal a workspace a previous job replaced with a symlink (or any + # non-directory) BEFORE canonicalizing it. Afterwards the path + # resolves to the link's target, the allowlist refuses that, and the + # refusal removes nothing — so every later job on this runner dies + # here, permanently, on corruption that is itself inside the runner + # workspace and safe to unlink. + if [ -L "$WS" ] || [ ! -d "$WS" ]; then + # Judge the PARENT, canonicalized. The heal necessarily acts on a + # raw path, and a raw containment match is not enough: the kernel + # resolves intermediate components too, so `$RWS/link/sub` matches + # "$RWS"/* as a string while naming a file outside it. Resolving + # the parent — never $WS itself, which would resolve through the + # very link being removed — is what makes the unlink containable. + HEAL_PARENT="$(realpath -m -- "$(dirname -- "$WS")" 2>/dev/null)" || { echo "::error::refusing to heal: realpath unavailable, cannot canonicalize the parent of ${WS}"; exit 1; } + case "$HEAL_PARENT" in + "$RWS"|"$RWS"/*) ;; + *) echo "::error::refusing to heal workspace outside the runner workspace: ${WS} (parent: ${HEAL_PARENT}, runner workspace: ${RWS})"; exit 1 ;; + esac + # The incident this heal exists for leaves no other trace: say what + # was found, and where it pointed, before it is gone. + if [ -L "$WS" ]; then + # The target is bytes a PREVIOUS job chose — on this pool that + # job may have run a contributor's code — and the runner parses + # `::` at the start of any stdout line as a workflow command. A + # target of $'x\n::error::forged' would therefore forge an + # annotation. Keep untrusted bytes off the command line itself, + # strip the line breaks that could start a new one, and cap the + # length. + heal_target="$(readlink -- "$WS" 2>/dev/null || printf '%s' '')" + heal_target="$(printf '%s' "$heal_target" | tr -d '\r\n' | cut -c1-200)" + echo "::warning::healing workspace ${WS}: it was a symlink" + printf 'heal: %s pointed at %s\n' "$WS" "$heal_target" + else + echo "::warning::healing workspace ${WS}: it was not a directory" + fi + # `rm -f` on the RAW path removes the link itself and never + # follows it. Both legs fail closed: under `-e` a failure that is + # not the last command of an && list is swallowed, and a swallowed + # one here would leave the wipe running against a corrupt path. + rm -f -- "$WS" || { echo "::error::refusing to continue: could not remove ${WS}"; exit 1; } + mkdir -- "$WS" || { echo "::error::refusing to continue: could not recreate ${WS}"; exit 1; } + fi + # Canonicalize before matching: the kernel resolves non-canonical + # spellings to the guarded roots (`/home/.` -> /home, `//usr` -> + # /usr), so a raw string match lets them slip past the case arms. + WS="$(realpath -m -- "$WS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${GITHUB_WORKSPACE}"; exit 1; } + while [ "${WS%/}" != "$WS" ]; do WS="${WS%/}"; done + case "$WS" in + ..|../*|*/..|*/../*) echo "::error::refusing to wipe path containing '..': ${WS}"; exit 1 ;; + esac + case "$WS" in + /|/home|/root|/usr*|/etc*|/var|"") echo "::error::refusing to wipe suspicious workspace path: ${WS}"; exit 1 ;; + esac + # A denylist can only enumerate known roots — the allowlist closes + # every other one (/tmp, /opt, ...): only a directory inside the + # runner workspace may be wiped. case "$WS" in "$RWS"/*) ;; *) echo "::error::refusing to wipe workspace outside the runner workspace: ${WS} (runner workspace: ${RWS})"; exit 1 ;; diff --git a/.github/workflows/serve-ab.yml b/.github/workflows/serve-ab.yml index 74a5d5c7c61..3000a1372fe 100644 --- a/.github/workflows/serve-ab.yml +++ b/.github/workflows/serve-ab.yml @@ -58,7 +58,10 @@ jobs: # other fork PRs stay on ephemeral hosted runners. Keep in sync with # ci.yml's classify_pr routing. Kill-switch: MAINTAINER_ECS_RUNNER_DISABLED. runs-on: '${{ (github.repository == ''QwenLM/qwen-code'' && vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && (github.event.pull_request.head.repo.full_name == github.repository || contains(fromJSON(''["OWNER","MEMBER","COLLABORATOR"]''), github.event.pull_request.author_association))) && fromJSON(''["self-hosted", "linux", "x64", "ecs-qwen"]'') || fromJSON(''["ubuntu-latest"]'') }}' - timeout-minutes: 30 + # Two full checkouts, each npm-ci + build + drive: a healthy run lands + # near twenty minutes, and a slow runner pushed a run past the old + # 30-minute bound, cancelling it. + timeout-minutes: 45 steps: - name: 'Restore workspace ownership' if: "${{ runner.environment == 'self-hosted' }}" @@ -88,14 +91,68 @@ jobs: # strip trailing slashes, denylist the known roots, and require # the target to sit inside the runner workspace before any rm. WS="${GITHUB_WORKSPACE:?}" + # Strip trailing slashes on the RAW path, before anything reads it: + # `[ -L "$WS/" ]` and `[ ! -d "$WS/" ]` both resolve THROUGH a link + # and report its target, so one trailing slash hides the corruption + # the heal below exists to clear. + while [ "${WS%/}" != "$WS" ]; do WS="${WS%/}"; done + # The allowlist root is prepared BEFORE the heal, because it bounds + # what the heal may touch: canonical, slash-free and non-degenerate. + # An empty $RUNNER_WORKSPACE would turn every containment pattern + # below into the match-all "/*". + RWS="${RUNNER_WORKSPACE:?}" + RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } + while [ "${RWS%/}" != "$RWS" ]; do RWS="${RWS%/}"; done + if [ -z "$RWS" ]; then echo "::error::refusing to wipe: runner workspace resolved to /"; exit 1; fi + case "$RWS" in + ..|../*|*/..|*/../*) echo "::error::refusing runner workspace path containing '..': ${RWS}"; exit 1 ;; + esac + # Heal a workspace a previous job replaced with a symlink (or any + # non-directory) BEFORE canonicalizing it. Afterwards the path + # resolves to the link's target, the allowlist refuses that, and the + # refusal removes nothing — so every later job on this runner dies + # here, permanently, on corruption that is itself inside the runner + # workspace and safe to unlink. + if [ -L "$WS" ] || [ ! -d "$WS" ]; then + # Judge the PARENT, canonicalized. The heal necessarily acts on a + # raw path, and a raw containment match is not enough: the kernel + # resolves intermediate components too, so `$RWS/link/sub` matches + # "$RWS"/* as a string while naming a file outside it. Resolving + # the parent — never $WS itself, which would resolve through the + # very link being removed — is what makes the unlink containable. + HEAL_PARENT="$(realpath -m -- "$(dirname -- "$WS")" 2>/dev/null)" || { echo "::error::refusing to heal: realpath unavailable, cannot canonicalize the parent of ${WS}"; exit 1; } + case "$HEAL_PARENT" in + "$RWS"|"$RWS"/*) ;; + *) echo "::error::refusing to heal workspace outside the runner workspace: ${WS} (parent: ${HEAL_PARENT}, runner workspace: ${RWS})"; exit 1 ;; + esac + # The incident this heal exists for leaves no other trace: say what + # was found, and where it pointed, before it is gone. + if [ -L "$WS" ]; then + # The target is bytes a PREVIOUS job chose — on this pool that + # job may have run a contributor's code — and the runner parses + # `::` at the start of any stdout line as a workflow command. A + # target of $'x\n::error::forged' would therefore forge an + # annotation. Keep untrusted bytes off the command line itself, + # strip the line breaks that could start a new one, and cap the + # length. + heal_target="$(readlink -- "$WS" 2>/dev/null || printf '%s' '')" + heal_target="$(printf '%s' "$heal_target" | tr -d '\r\n' | cut -c1-200)" + echo "::warning::healing workspace ${WS}: it was a symlink" + printf 'heal: %s pointed at %s\n' "$WS" "$heal_target" + else + echo "::warning::healing workspace ${WS}: it was not a directory" + fi + # `rm -f` on the RAW path removes the link itself and never + # follows it. Both legs fail closed: under `-e` a failure that is + # not the last command of an && list is swallowed, and a swallowed + # one here would leave the wipe running against a corrupt path. + rm -f -- "$WS" || { echo "::error::refusing to continue: could not remove ${WS}"; exit 1; } + mkdir -- "$WS" || { echo "::error::refusing to continue: could not recreate ${WS}"; exit 1; } + fi # Canonicalize before matching: the kernel resolves non-canonical # spellings to the guarded roots (`/home/.` -> /home, `//usr` -> # /usr), so a raw string match lets them slip past the case arms. WS="$(realpath -m -- "$WS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${GITHUB_WORKSPACE}"; exit 1; } - # Trailing slashes slip past the exact-match case arms below - # (`/home/` would pass the guard and reach the rm); realpath strips - # them too; keep the guard whole if the path reaches this point with - # trailing slashes. while [ "${WS%/}" != "$WS" ]; do WS="${WS%/}"; done case "$WS" in ..|../*|*/..|*/../*) echo "::error::refusing to wipe path containing '..': ${WS}"; exit 1 ;; @@ -106,15 +163,6 @@ jobs: # A denylist can only enumerate known roots — the allowlist closes # every other one (/tmp, /opt, ...): only a directory inside the # runner workspace may be wiped. - RWS="${RUNNER_WORKSPACE:?}" - RWS="$(realpath -m -- "$RWS" 2>/dev/null)" || { echo "::error::refusing to wipe: realpath unavailable, cannot canonicalize ${RUNNER_WORKSPACE}"; exit 1; } - # Mirror the WS strip before building the allowlist pattern; "/" - # stripped empty would match every path instead. - while [ "${RWS%/}" != "$RWS" ]; do RWS="${RWS%/}"; done - if [ -z "$RWS" ]; then echo "::error::refusing to wipe: runner workspace resolved to /"; exit 1; fi - case "$RWS" in - ..|../*|*/..|*/../*) echo "::error::refusing runner workspace path containing '..': ${RWS}"; exit 1 ;; - esac case "$WS" in "$RWS"/*) ;; *) echo "::error::refusing to wipe workspace outside the runner workspace: ${WS} (runner workspace: ${RWS})"; exit 1 ;; diff --git a/scripts/tests/qwen-triage-workflow.test.js b/scripts/tests/qwen-triage-workflow.test.js index 41d43282a3e..5671b20339c 100644 --- a/scripts/tests/qwen-triage-workflow.test.js +++ b/scripts/tests/qwen-triage-workflow.test.js @@ -8,6 +8,7 @@ import { execFileSync, spawn, spawnSync } from 'node:child_process'; import { chmodSync, existsSync, + lstatSync, mkdirSync, mkdtempSync, readdirSync, @@ -1632,6 +1633,38 @@ describe('qwen-triage verify hardening', () => { expect(wipe).toContain('runner workspace resolved to /'); }); + it('carries the symlink heal in both wipes, ordered and bounded (#9480)', () => { + // The guard's own wedge: a workspace replaced by a symlink resolves to + // its target, the allowlist refuses, and the refusal removes nothing — + // so the runner dies here on every later job. The heal must therefore + // run BEFORE canonicalization, AFTER the allowlist root that bounds it, + // and after the raw trailing-slash strip, since both of its predicates + // resolve through a link when the path ends in '/'. + for (const stepName of [ + 'Wipe workspace before external code', + 'Wipe workspace after external code', + ]) { + const run = stepIn('verify', stepName); + const healAt = run.indexOf('[ -L "$WS" ] || [ ! -d "$WS" ]'); + expect(healAt, `${stepName} has no heal`).toBeGreaterThan(-1); + expect(healAt).toBeLessThan(run.indexOf('realpath -m -- "$WS"')); + expect(run.indexOf('RWS="${RUNNER_WORKSPACE:?}"')).toBeLessThan(healAt); + expect(run.indexOf('while [ "${WS%/}" != "$WS" ]')).toBeLessThan(healAt); + // Containment on the canonical PARENT — resolving $WS would follow + // the very link being removed, and a raw match cannot see + // intermediate symlink components. + expect(run).toContain( + 'HEAL_PARENT="$(realpath -m -- "$(dirname -- "$WS")" 2>/dev/null)"', + ); + expect(run).toContain('"$RWS"|"$RWS"/*)'); + expect(run).toContain('refusing to heal workspace outside'); + // Both legs fail closed, and the incident leaves a trace. + expect(run).toContain('rm -f -- "$WS" || {'); + expect(run).toContain('mkdir -- "$WS" || {'); + expect(run).toContain('::warning::healing workspace'); + } + }); + // The wipe is the deny-by-default control, so run the real step text // against a workspace carrying the vectors the allowlist sweep is built // to enumerate — plus one it is not — and require every one to be gone. @@ -1783,12 +1816,13 @@ describe('qwen-triage verify hardening', () => { // identically whether `realpath -m` ran or not — deleting that line // ships green against the battery. A raw '..' spelling does not pin it // either: the '..' case arm refuses that vector first, mutant or not. - // A symlink INSIDE the runner workspace pointing outside is the - // spelling only the realpath line can catch: canonicalized, it lands - // outside and the allowlist refuses it; with the line deleted the raw - // link path matches "$RWS"/*, but find's default -P mode does not - // descend symlink operands, so the mutant exits 0 having wiped nothing - // — caught by the non-zero-status assertion below, not the rm recorder. + // What does pin it is a path whose INTERMEDIATE component leaves the + // runner workspace: it matches "$RWS"/* as a string and names a + // directory outside it, so with the line deleted find resolves the link + // through the kernel and hands that directory's entries to the rm + // recorder. It is deliberately a directory at the far end rather than + // the link itself — a workspace that IS a link is now healed rather + // than refused (#9480), and this test exists for the refusal. const extractRun = (stepName) => { const run = stepIn('verify', stepName) .match(/run: \|-\n([\s\S]*)$/)?.[1] @@ -1798,15 +1832,15 @@ describe('qwen-triage verify hardening', () => { }; it.skipIf(!hasGnuRealpath)( - 'refuses an allowlist-escaping symlink via canonicalization', + 'refuses an allowlist-escaping path reached through an intermediate symlink', () => { const dir = mkdtempSync(join(tmpdir(), 'verify-wipe-escape-')); const outside = mkdtempSync(join(tmpdir(), 'verify-wipe-outside-')); - writeFileSync(join(outside, 'canary'), 'x'); + mkdirSync(join(outside, 'sub')); + writeFileSync(join(outside, 'sub', 'canary'), 'x'); symlinkSync(outside, join(dir, 'link')); try { const calls = join(dir, 'rm-calls'); - writeFileSync(calls, ''); writeFileSync( join(dir, 'rm'), `#!/bin/sh\nprintf '%s\\n' "$*" >> '${calls}'\nexit 0\n`, @@ -1826,20 +1860,21 @@ describe('qwen-triage verify hardening', () => { env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, - // A link inside the recorder dir whose target sits - // outside it — no '..' component, so only canonicalization - // can resolve the escape. - GITHUB_WORKSPACE: join(dir, 'link'), + GITHUB_WORKSPACE: join(dir, 'link', 'sub'), RUNNER_WORKSPACE: dir, GITHUB_STEP_SUMMARY: join(dir, 'summary'), }, }, ); expect(res.status, `${stepName} did not refuse`).not.toBe(0); + expect(res.stdout + res.stderr).toContain( + 'outside the runner workspace', + ); expect( readFileSync(calls, 'utf8'), `${stepName} invoked rm on the escaping path`, ).toBe(''); + expect(readdirSync(join(outside, 'sub'))).toEqual(['canary']); } } finally { rmSync(dir, { recursive: true, force: true }); @@ -1848,6 +1883,284 @@ describe('qwen-triage verify hardening', () => { }, ); + // The wedge the guard itself created, and the hole the first attempt at + // healing it shipped. Both wipes carry the same layer, so both are driven. + const WIPE_STEPS = [ + 'Wipe workspace before external code', + 'Wipe workspace after external code', + ]; + + it.skipIf(!hasGnuRealpath)( + 'heals a workspace a previous job replaced with a symlink', + () => { + // Without the heal this is permanent: canonicalization resolves the + // link to its target, the allowlist refuses, the step exits 1 having + // removed nothing, and every later job on the runner dies at the same + // line. The unlink must take the LINK and leave the target alone — + // which is also what pins the heal judging the canonical PARENT + // rather than $WS itself, since resolving $WS follows the very link + // being removed and would refuse a repair that must succeed. + for (const stepName of WIPE_STEPS) { + const parent = mkdtempSync(join(tmpdir(), 'verify-heal-')); + const outside = mkdtempSync(join(tmpdir(), 'verify-heal-outside-')); + const ws = join(parent, 'workspace'); + writeFileSync(join(outside, 'canary'), 'x'); + symlinkSync(outside, ws); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: ws, + RUNNER_WORKSPACE: parent, + GITHUB_STEP_SUMMARY: join(parent, 'summary'), + }, + }, + ); + expect(res.status, `${stepName}: ${res.stdout}${res.stderr}`).toBe(0); + expect(res.stdout + res.stderr).toContain('healing workspace'); + expect(res.stdout + res.stderr).toContain(outside); + expect(lstatSync(ws).isSymbolicLink()).toBe(false); + expect(readdirSync(ws)).toEqual([]); + expect(readdirSync(outside)).toEqual(['canary']); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'refuses to heal through an intermediate symlink, before touching anything', + () => { + // The defect the first attempt kept through three rounds: it matched + // the RAW path, and `$RWS/link/sub` matches "$RWS"/* as a string while + // naming a file outside the runner workspace — so the unlink and the + // mkdir landed OUTSIDE, and only then did the allowlist refuse. + for (const stepName of WIPE_STEPS) { + const parent = mkdtempSync(join(tmpdir(), 'verify-heal-inter-')); + const outside = mkdtempSync(join(tmpdir(), 'verify-heal-outside-')); + writeFileSync(join(outside, 'sub'), 'canary'); + symlinkSync(outside, join(parent, 'link')); + try { + const calls = join(parent, 'rm-calls'); + writeFileSync(calls, ''); + writeFileSync( + join(parent, 'rm'), + `#!/bin/sh\nprintf '%s\\n' "$*" >> '${calls}'\nexit 0\n`, + { mode: 0o755 }, + ); + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${parent}:${process.env.PATH}`, + GITHUB_WORKSPACE: join(parent, 'link', 'sub'), + RUNNER_WORKSPACE: parent, + GITHUB_STEP_SUMMARY: join(parent, 'summary'), + }, + }, + ); + expect(res.status, stepName).not.toBe(0); + expect(res.stdout + res.stderr).toContain( + 'refusing to heal workspace outside the runner workspace', + ); + expect(readFileSync(calls, 'utf8')).toBe(''); + // The mutation the old shape performed before refusing: the file + // at the resolved target is still a file, with its contents. + expect(lstatSync(join(outside, 'sub')).isFile()).toBe(true); + expect(readFileSync(join(outside, 'sub'), 'utf8')).toBe('canary'); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'heals a non-directory workspace, and sees it through a trailing slash', + () => { + // The `|| [ ! -d "$WS" ]` half, and the raw strip that has to precede + // both predicates: `[ -L "$WS/" ]` is false and `[ ! -d "$WS/" ]` + // resolves through the link, so one trailing slash hides the + // corruption entirely. + for (const stepName of WIPE_STEPS) { + const parent = mkdtempSync(join(tmpdir(), 'verify-heal-file-')); + const ws = join(parent, 'workspace'); + writeFileSync(ws, 'not a directory'); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: ws, + RUNNER_WORKSPACE: parent, + GITHUB_STEP_SUMMARY: join(parent, 'summary'), + }, + }, + ); + expect(res.status, `${stepName}: ${res.stdout}${res.stderr}`).toBe(0); + expect(res.stdout + res.stderr).toContain('it was not a directory'); + expect(lstatSync(ws).isDirectory()).toBe(true); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + + const linkParent = mkdtempSync(join(tmpdir(), 'verify-heal-slash-')); + const outside = mkdtempSync(join(tmpdir(), 'verify-heal-outside-')); + const linkWs = join(linkParent, 'workspace'); + writeFileSync(join(outside, 'canary'), 'x'); + symlinkSync(outside, linkWs); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: `${linkWs}/`, + RUNNER_WORKSPACE: linkParent, + GITHUB_STEP_SUMMARY: join(linkParent, 'summary'), + }, + }, + ); + expect(res.status, `${stepName}: ${res.stdout}${res.stderr}`).toBe(0); + expect(lstatSync(linkWs).isSymbolicLink()).toBe(false); + expect(readdirSync(outside)).toEqual(['canary']); + } finally { + rmSync(linkParent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'keeps a forged workflow command in the symlink target out of the log', + () => { + // On this lane the previous job may have run a contributor's code, and + // the runner parses `::` at the start of ANY stdout line as a workflow + // command — so a target of $'…\n::error::forged' would forge an + // annotation from the very step reporting the corruption. + for (const stepName of WIPE_STEPS) { + const parent = mkdtempSync(join(tmpdir(), 'verify-heal-inject-')); + const outside = mkdtempSync(join(tmpdir(), 'verify-heal-outside-')); + const ws = join(parent, 'workspace'); + symlinkSync(`${outside}\n::error::forged-annotation`, ws); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: ws, + RUNNER_WORKSPACE: parent, + GITHUB_STEP_SUMMARY: join(parent, 'summary'), + }, + }, + ); + expect(res.status, `${stepName}: ${res.stdout}${res.stderr}`).toBe(0); + const out = res.stdout + res.stderr; + expect(out).toContain('healing workspace'); + expect(out).toContain('pointed at'); + for (const line of out.split('\n')) { + expect(line.startsWith('::error::')).toBe(false); + } + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'fails closed when the healed workspace cannot be recreated', + () => { + // The mkdir leg's own refusal, reachable without a permission trick: + // `rm -f` returns 0 for a path whose parent is not a directory, and + // the mkdir that follows cannot succeed. Swallowed, the wipe would run + // against a path that does not exist. + for (const stepName of WIPE_STEPS) { + const parent = mkdtempSync(join(tmpdir(), 'verify-heal-mkdir-')); + writeFileSync(join(parent, 'file'), 'not a directory'); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: join(parent, 'file', 'sub'), + RUNNER_WORKSPACE: parent, + GITHUB_STEP_SUMMARY: join(parent, 'summary'), + }, + }, + ); + expect(res.status, stepName).not.toBe(0); + expect(res.stdout + res.stderr).toContain('could not recreate'); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + } + }, + ); + + it.skipIf(!hasGnuRealpath || process.getuid?.() === 0)( + 'fails closed when the corrupt workspace cannot be unlinked', + () => { + // A swallowed `rm -f` failure would let the mkdir and the wipe run on + // a path that is still a symlink. Root bypasses the mode bits, so the + // fixture cannot produce the refusal there. + for (const stepName of WIPE_STEPS) { + const parent = mkdtempSync(join(tmpdir(), 'verify-heal-perm-')); + const outside = mkdtempSync(join(tmpdir(), 'verify-heal-outside-')); + const ws = join(parent, 'workspace'); + writeFileSync(join(outside, 'canary'), 'x'); + symlinkSync(outside, ws); + chmodSync(parent, 0o555); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', extractRun(stepName)], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: ws, + RUNNER_WORKSPACE: parent, + GITHUB_STEP_SUMMARY: join(parent, 'summary'), + }, + }, + ); + expect(res.status, stepName).not.toBe(0); + expect(res.stdout + res.stderr).toContain('could not remove'); + expect(lstatSync(ws).isSymbolicLink()).toBe(true); + expect(readdirSync(outside)).toEqual(['canary']); + } finally { + chmodSync(parent, 0o755); + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + } + }, + ); + // Fronts PATH with a failing realpath so the script must fail closed instead // of matching and wiping a raw, potentially misleading spelling. const stubRealpath = () => { diff --git a/scripts/tests/serve-ab-workflow.test.js b/scripts/tests/serve-ab-workflow.test.js index 997cf70c81a..aa7bbd5c365 100644 --- a/scripts/tests/serve-ab-workflow.test.js +++ b/scripts/tests/serve-ab-workflow.test.js @@ -7,6 +7,7 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { chmodSync, + lstatSync, mkdirSync, mkdtempSync, readdirSync, @@ -22,7 +23,8 @@ import { parse } from 'yaml'; const workflow = readFileSync('.github/workflows/serve-ab.yml', 'utf8'); -const steps = parse(workflow).jobs['ab'].steps; +const job = parse(workflow).jobs['ab']; +const steps = job.steps; const WIPE = 'Wipe stale workspace before checkout'; const wipe = steps.find((s) => s.name === WIPE); @@ -63,6 +65,14 @@ describe('serve-ab pre-checkout workspace wipe', () => { expect(wipe.if).toBe("${{ runner.environment == 'self-hosted' }}"); }); + // The job drives BOTH checkouts end-to-end (npm ci, full monorepo build, + // daemon drive, each); a healthy run lands near twenty minutes, so the + // old 30-minute bound left a slow runner no headroom and the run timed + // out as CANCELLED. Pin the floor — dropping it back re-cancels the run. + it('keeps a job timeout with headroom for two full build cycles', () => { + expect(job['timeout-minutes']).toBeGreaterThanOrEqual(45); + }); + it('carries the full checkout-heal guard (#9220, #9265)', () => { // Before the port this step had NO guard: under a mangled env even // `/home` or an empty string reached `find … -exec rm -rf {} +`. @@ -88,6 +98,42 @@ describe('serve-ab pre-checkout workspace wipe', () => { expect(wipe.run).not.toContain('|| true'); }); + it('carries the symlink heal, ordered and bounded (#9480)', () => { + // The heal has to sit BEFORE the canonicalization: afterwards the path + // has already resolved to the link's target, the allowlist refuses it, + // and that refusal removes nothing — the wedge. Order is the property, + // so it is asserted as one, not as the presence of two strings. + const healAt = wipe.run.indexOf('[ -L "$WS" ] || [ ! -d "$WS" ]'); + const canonAt = wipe.run.indexOf('realpath -m -- "$WS"'); + const rwsAt = wipe.run.indexOf('RWS="${RUNNER_WORKSPACE:?}"'); + expect(healAt).toBeGreaterThan(-1); + expect(healAt).toBeLessThan(canonAt); + // …and AFTER the allowlist root is prepared, since that root is what + // bounds the heal. A raw, empty $RUNNER_WORKSPACE would degenerate the + // containment pattern to the match-all `/*`. + expect(rwsAt).toBeLessThan(healAt); + // The raw strip has to precede the predicates: `[ -L "$WS/" ]` and + // `[ ! -d "$WS/" ]` both resolve THROUGH the link and report its target. + expect(wipe.run.indexOf('while [ "${WS%/}" != "$WS" ]')).toBeLessThan( + healAt, + ); + // Containment is judged on the canonical PARENT, never on $WS itself — + // resolving $WS would follow the very link being removed, and a raw + // match cannot see intermediate symlink components. + expect(wipe.run).toContain( + 'HEAL_PARENT="$(realpath -m -- "$(dirname -- "$WS")" 2>/dev/null)"', + ); + expect(wipe.run).toContain('"$RWS"|"$RWS"/*)'); + expect(wipe.run).toContain('refusing to heal workspace outside'); + // Both legs fail closed: under `-e` a failure that is not the last + // command of an && list is swallowed, and a swallowed one here leaves + // the wipe running against a corrupt path. + expect(wipe.run).toContain('rm -f -- "$WS" || {'); + expect(wipe.run).toContain('mkdir -- "$WS" || {'); + // The incident leaves no other trace. + expect(wipe.run).toContain('::warning::healing workspace'); + }); + it.skipIf(!hasGnuRealpath)( 'wipes a legitimate workspace inside the runner workspace', () => { @@ -180,22 +226,26 @@ describe('serve-ab pre-checkout workspace wipe', () => { }); it.skipIf(!hasGnuRealpath)( - 'refuses an allowlist-escaping symlink via canonicalization', + 'refuses an allowlist-escaping path reached through an intermediate symlink', () => { - // The bad paths above all sit outside the recorder dir, so the - // allowlist refuses them identically whether canonicalization runs - // or not — they cannot pin the realpath line. A raw '..' spelling - // does not pin it either: the '..' case arm refuses that vector - // first, mutant or not (executed mutant: exit 1 via 'refusing to - // wipe path containing ..', rm log empty). A symlink INSIDE the - // recorder dir pointing outside is the spelling only `realpath -m` - // can catch: with the line deleted the raw link path matches - // "$RWS"/*, but find's default -P mode does not descend symlink - // operands, so the mutant exits 0 having wiped nothing — caught by - // the non-zero-status assertion, not the rm recorder. + // Pins the canonicalization line. The bad paths above all sit outside + // the recorder dir, so the allowlist refuses them identically whether + // canonicalization runs or not, and a raw '..' spelling is refused by + // the '..' arm first — neither can pin it. The vector that can is a + // path whose INTERMEDIATE component is a link out of the runner + // workspace: it matches "$RWS"/* as a string and names a directory + // outside it. + // + // It is deliberately a directory at the far end, not the link itself: + // a workspace that IS a link is now healed rather than refused + // (#9480), and this test exists for the refusal, not the heal. + // Executed mutant (canonicalization line deleted): the raw path + // passes the allowlist and find, resolving the link through the + // kernel, hands the outside directory's entries to the rm recorder. const dir = mkdtempSync(join(tmpdir(), 'serve-ab-wipe-escape-')); const outside = mkdtempSync(join(tmpdir(), 'serve-ab-wipe-outside-')); - writeFileSync(join(outside, 'canary'), 'x'); + mkdirSync(join(outside, 'sub')); + writeFileSync(join(outside, 'sub', 'canary'), 'x'); symlinkSync(outside, join(dir, 'link')); try { const calls = join(dir, 'rm-calls'); @@ -213,16 +263,17 @@ describe('serve-ab pre-checkout workspace wipe', () => { env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, - // A link inside the recorder dir whose target sits outside - // it — no '..' component, so only canonicalization can - // resolve the escape. - GITHUB_WORKSPACE: join(dir, 'link'), + GITHUB_WORKSPACE: join(dir, 'link', 'sub'), RUNNER_WORKSPACE: dir, }, }, ); expect(res.status).not.toBe(0); + expect(res.stdout + res.stderr).toContain( + 'outside the runner workspace', + ); expect(readFileSync(calls, 'utf8')).toBe(''); + expect(readdirSync(join(outside, 'sub'))).toEqual(['canary']); } finally { rmSync(dir, { recursive: true, force: true }); rmSync(outside, { recursive: true, force: true }); @@ -379,4 +430,254 @@ describe('serve-ab pre-checkout workspace wipe', () => { } }, ); + + // The wedge this heal exists for, and the hole the first attempt at it + // shipped. Both are exec fixtures: the guard runs for real, and where a + // regression would delete something, `rm` is a PATH-fronted recorder so + // the assertion is on the decision and nothing on the machine can be lost. + const rmRecorder = (dir) => { + const calls = join(dir, 'rm-calls'); + writeFileSync(calls, ''); + writeFileSync( + join(dir, 'rm'), + `#!/bin/sh\nprintf '%s\\n' "$*" >> '${calls}'\nexit 0\n`, + { mode: 0o755 }, + ); + return calls; + }; + + it.skipIf(!hasGnuRealpath)( + 'heals a workspace a previous job replaced with a symlink', + () => { + // Without the heal this is a permanent wedge: canonicalization + // resolves the link to its target, the allowlist refuses, the step + // exits 1 having removed nothing, and every later job on the runner + // dies at the same line. The unlink must take the LINK and leave the + // target alone. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-')); + const outside = mkdtempSync(join(tmpdir(), 'serve-ab-heal-outside-')); + const ws = join(parent, 'repo'); + writeFileSync(join(outside, 'canary'), 'x'); + symlinkSync(outside, ws); + try { + const out = runWipe({ GITHUB_WORKSPACE: ws, RUNNER_WORKSPACE: parent }); + expect(out).toContain('healing workspace'); + expect(out).toContain(outside); + expect(lstatSync(ws).isSymbolicLink()).toBe(false); + expect(lstatSync(ws).isDirectory()).toBe(true); + expect(readdirSync(ws)).toEqual([]); + // The link was removed, not followed: the target keeps its contents. + expect(readdirSync(outside)).toEqual(['canary']); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'refuses to heal through an intermediate symlink, before touching anything', + () => { + // The defect the first attempt at this layer kept through three + // rounds: it matched the RAW path, and a raw `"$RWS"/*` accepts + // `$RWS/link/sub` as a string while the kernel resolves it to a file + // outside the runner workspace — so the unlink and the mkdir landed + // OUTSIDE, and only then did canonicalization and the allowlist + // refuse the wipe. + // + // What this fixture pins is the containment arm: deleting it lets the + // heal act here. It does NOT discriminate parent-from-self — resolving + // $WS itself also lands outside and also refuses. The fixture that + // separates the two is the legitimate heal above, where judging $WS + // resolves through the link being removed and refuses a repair that + // must succeed. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-inter-')); + const outside = mkdtempSync(join(tmpdir(), 'serve-ab-heal-outside-')); + writeFileSync(join(outside, 'sub'), 'canary'); + symlinkSync(outside, join(parent, 'link')); + try { + const calls = rmRecorder(parent); + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', wipe.run], + { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${parent}:${process.env.PATH}`, + GITHUB_WORKSPACE: join(parent, 'link', 'sub'), + RUNNER_WORKSPACE: parent, + }, + }, + ); + expect(res.status).not.toBe(0); + expect(res.stdout + res.stderr).toContain( + 'refusing to heal workspace outside the runner workspace', + ); + // Nothing was deleted, and the file at the resolved target is still + // a file — the mutation the old shape performed before refusing. + expect(readFileSync(calls, 'utf8')).toBe(''); + expect(lstatSync(join(outside, 'sub')).isFile()).toBe(true); + expect(readFileSync(join(outside, 'sub'), 'utf8')).toBe('canary'); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'heals a workspace that is not a directory', + () => { + // The other half of the predicate: a leftover regular file where the + // workspace should be wedges the step exactly the same way, and it is + // the half the first attempt left untested. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-file-')); + const ws = join(parent, 'repo'); + writeFileSync(ws, 'not a directory'); + try { + const out = runWipe({ GITHUB_WORKSPACE: ws, RUNNER_WORKSPACE: parent }); + expect(out).toContain('it was not a directory'); + expect(lstatSync(ws).isDirectory()).toBe(true); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'sees the corruption through a trailing-slash spelling', + () => { + // `[ -L "$WS/" ]` is false and `[ ! -d "$WS/" ]` resolves through the + // link, so without the raw strip ahead of the predicates the heal + // never fires and the wedge survives one keystroke. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-slash-')); + const outside = mkdtempSync(join(tmpdir(), 'serve-ab-heal-outside-')); + const ws = join(parent, 'repo'); + writeFileSync(join(outside, 'canary'), 'x'); + symlinkSync(outside, ws); + try { + runWipe({ GITHUB_WORKSPACE: `${ws}/`, RUNNER_WORKSPACE: parent }); + expect(lstatSync(ws).isSymbolicLink()).toBe(false); + expect(readdirSync(outside)).toEqual(['canary']); + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'leaves an ordinary workspace untouched by the heal', + () => { + // The heal must cost nothing on the path every real run takes. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-noop-')); + const ws = join(parent, 'repo'); + mkdirSync(ws); + writeFileSync(join(ws, 'leftover'), 'x'); + try { + const out = runWipe({ GITHUB_WORKSPACE: ws, RUNNER_WORKSPACE: parent }); + expect(out).not.toContain('healing workspace'); + expect(readdirSync(ws)).toEqual([]); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath || process.getuid?.() === 0)( + 'fails closed when the corrupt workspace cannot be unlinked', + () => { + // A swallowed `rm -f` failure would let the mkdir and the wipe run on + // a path that is still a symlink. Root bypasses the mode bits, so the + // fixture cannot produce the refusal there. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-perm-')); + const outside = mkdtempSync(join(tmpdir(), 'serve-ab-heal-outside-')); + const ws = join(parent, 'repo'); + writeFileSync(join(outside, 'canary'), 'x'); + symlinkSync(outside, ws); + chmodSync(parent, 0o555); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', wipe.run], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: ws, + RUNNER_WORKSPACE: parent, + }, + }, + ); + expect(res.status).not.toBe(0); + expect(res.stdout + res.stderr).toContain('could not remove'); + expect(lstatSync(ws).isSymbolicLink()).toBe(true); + expect(readdirSync(outside)).toEqual(['canary']); + } finally { + chmodSync(parent, 0o755); + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'keeps a forged workflow command in the symlink target out of the log', + () => { + // The target is bytes a previous job chose, and the runner parses `::` + // at the start of ANY stdout line as a workflow command — so a target + // of $'…\n::error::forged' would forge an annotation from a step that + // is reporting corruption. The annotation carries no untrusted bytes + // and the target is printed on a prefixed line with its newlines gone. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-inject-')); + const outside = mkdtempSync(join(tmpdir(), 'serve-ab-heal-outside-')); + const ws = join(parent, 'repo'); + symlinkSync(`${outside}\n::error::forged-annotation`, ws); + try { + const out = runWipe({ GITHUB_WORKSPACE: ws, RUNNER_WORKSPACE: parent }); + expect(out).toContain('healing workspace'); + // The target is still reported — just never as a command. + expect(out).toContain('pointed at'); + expect(out).toContain(basename(outside)); + for (const line of out.split('\n')) { + expect(line.startsWith('::error::')).toBe(false); + } + } finally { + rmSync(parent, { recursive: true, force: true }); + rmSync(outside, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(!hasGnuRealpath)( + 'fails closed when the healed workspace cannot be recreated', + () => { + // The mkdir leg's own refusal, reachable without a permission trick: + // `rm -f` returns 0 for a path whose parent is not a directory (it + // reads as "already absent"), and the mkdir that follows cannot + // succeed. Swallowed, the wipe would then run against a path that + // does not exist. + const parent = mkdtempSync(join(tmpdir(), 'serve-ab-heal-mkdir-')); + writeFileSync(join(parent, 'file'), 'not a directory'); + try { + const res = spawnSync( + 'bash', + ['-e', '-o', 'pipefail', '-c', wipe.run], + { + encoding: 'utf8', + env: { + ...process.env, + GITHUB_WORKSPACE: join(parent, 'file', 'sub'), + RUNNER_WORKSPACE: parent, + }, + }, + ); + expect(res.status).not.toBe(0); + expect(res.stdout + res.stderr).toContain('could not recreate'); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }, + ); });