fix(ci): route the review/resolve permission gate to the ECS pool (#16482)

* fix(ci): route the review/resolve permission gate to the ECS pool

The authorize job gates every @qwen-code /review and /resolve command, but its
ECS routing required github.event.pull_request, which issue_comment and
workflow_dispatch payloads do not carry. Every command therefore fell through
to ubuntu-latest, and the gate sat in the hosted queue for hours (PR #15286:
3h10m for a 4-second job) while the ECS pool was idle.

Route issue_comment and workflow_dispatch to the persistent ECS pool, matching
qwen-autofix.yml's route/review-scan pattern for PAT-bearing API-only steps.
authorize runs no agent, checks out nothing, and processes no PR content, so
this does not widen the fork attack surface. PR-payload events keep the
same-repo fork guard; the hosted fallback and ECS kill switch are unchanged.

resolve-pr and publish-resolution stay on hosted by design (ephemerality); this
removes the gate wait only.

* fix(ci): harden the authorize ECS-routing pins and risk docs

Address review feedback on the gate routing change:
- Pin the checkout-free invariant the ECS routing depends on: a uses: step on
  authorize would execute contributor-authored files on the persistent pool
  with CI_BOT_PAT in the step env while the expression pin stayed green (R1-1).
  Scoped to authorize; mutation-witnessed (adding actions/checkout reds it).
- Record the trigger-set widening in the design-doc Risks (both languages):
  issue_comment makes the gate fireable by any GitHub user on a fork PR, and
  the should_review authorization fact is now produced on the shared host and
  consumed downstream without re-deriving (R1-2).
- Correct the precedent wording: route loads only the read-only github.token;
  review-scan is the job that loads CI_DEV_BOT_PAT on the pool.

* fix(ci): harden the shared-pool authorize gate against gh config reroute

Address the round-2 CHANGES_REQUESTED review (R2-1 Critical, R2-2):

R2-1: routing the gate to the persistent ecs-qwen pool moved its CI_BOT_PAT
gh api call onto a host with a shared, writable ~/.config/gh. A co-resident
survivor could plant http_unix_socket there, receive the PAT, and forge the
permission response downstream consumes as should_review. Prepend the same
pins as qwen-autofix.yml's route/review-scan — export GH_HOST=github.com plus
a private, guarded GH_CONFIG_DIR (a failed mktemp exits 1; the gate fails
closed). No `unset GH_TOKEN`: it is this step's only credential. Witnessed
with real gh 2.103.0: the as-written step leaks the PAT to a planted socket
and approves a stranger; the hardened step denies with the socket untouched.

R2-2: extend the checkout-free guard to the run: half — pin the step
inventory to the single permission step and forbid `${{` spliced into any run
body (event values must arrive via env:). Mutation-witnessed: removing the
preamble and adding a run-only second step each red the test.

Also correct the design-doc parity claim (both languages): the exposure
matches review-scan only WITH these gh pins; document the hardening.

* fix(ci): extend the ECS gate routing to triage and harden the gate pins

Two batches, verified together before push:

1. Extend the command-gate routing to qwen-triage.yml's authorize (the
   /triage, /tmux, /verify sibling blind spot): the same issue_comment /
   workflow_dispatch disjuncts, and — because that job has three steps that
   spend CI_BOT_PAT on gh — the same self-contained GH_HOST + guarded
   GH_CONFIG_DIR preamble on each, per the repo's per-step convention. Raise
   the recorded workflow size baseline to match the added hardening.

2. Address round-3 review (R2-2 fix-induced, R3-1): the gate pins tested
   substring presence only. Now evaluate the comment-stripped live lines and
   pin the guarded mktemp form + fail-closed shape, require GH_HOST /
   GH_CONFIG_DIR exported before the first gh api, and forbid executing
   repository code via relative paths or interpreters (the pool cwd is the
   reused $GITHUB_WORKSPACE). Applied to both the review and triage gate
   tests. Mutation-witnessed: an unguarded one-liner, a relocated preamble,
   and an appended `node ./…` line each red the tests.

Tests: qwen-pr-review-workflow.test.js 271, qwen-triage-workflow.test.mjs 131,
ci-runner-routing.test.mjs 103 — all pass; prettier + ESLint clean.

* fix(ci): supply RUNNER_TEMP to the triage gate replay tests; harden the gate pins

R4-1 (Critical): the new ${RUNNER_TEMP} gh-config preamble is the first
RUNNER_TEMP reference in the triage permission step, and three replay
harnesses in scripts/tests/qwen-triage-workflow.test.js execute that body
verbatim without setting RUNNER_TEMP — off-CI (npm run test:scripts) the
fail-closed guard exits 1 before writing should_run. Set RUNNER_TEMP to each
harness's existing mkdtempSync dir (the :1773 precedent); the workflow's
fail-closed guard is unchanged. Verified: the file is red under
`env -u RUNNER_TEMP` before and 203/203 green after; dropping the line reds it
again. (The review named two harnesses; running the full file surfaced a third.)

Also clear the round-4 deferred notes:
- R4-2: the /verify ack comment no longer promises a receipt "even when the
  sandbox lane is backed up" — the gate now also queues with the pool.
- R4-3/R4-4: the triage routing pin is now an exact expression equality, the
  step inventory is pinned, and PAT-bearing steps are identified by env
  GH_TOKEN rather than gh subcommand spelling.
- R3-1: tighten the no-repository-code deny-list (quote boundaries +
  interpreter as a word) so `bash -c './x.sh'` no longer slips past;
  mutation-witnessed in both the review and triage gate tests.

Tests: triage vitest 203 (off-CI), triage .mjs 131, review vitest 271,
ci-runner-routing 103 — all pass; prettier + ESLint clean; size gate green.

* fix(ci): correct the gate-routing narrative; close the round-5 verify gaps

R5-1 / F1 (narrative): the motivating incident is a /resolve, whose
end-to-end latency this PR does not change — resolve-pr stays on
ubuntu-latest, so only the gate wait is removed. State this in both
design-doc languages and the pinning test comment; the saving lands on
/review, /triage, /tmux, /verify (pool-routed downstream). Correct the
"cancelled without ever running" sentence to the run's timeline (resolve-pr
started when the gate finished and was cancelled ~29m36s later).

S1: the no-repository-code deny-list missed absolute-path interpreters
(/usr/bin/node x.js) and versioned Python (python3.11 x.py); widen the regex
(optional path prefix + python[0-9.]*) in both gate tests. Mutation-witnessed:
both previously-surviving mutants now red, clean tree green.

S2: each hardened gh step minted a $RUNNER_TEMP/*gh-config.* dir that nothing
removed; add trap 'rm -rf "$GH_CONFIG_DIR"' EXIT to all four steps (RUNNER_TEMP
survives between jobs on the pool).

S3: record the exact final workflow sizes in .size-baseline.

Tests: triage vitest 203 (off-CI, RUNNER_TEMP unset), review vitest 271,
triage .mjs 131, ci-runner-routing 103 — all pass; prettier + ESLint clean;
size gate green.

* fix(ci): pin the no-repository-code invariant on every triage gate step

The triage gate meta-test applied the relative-path/interpreter denylist only to PAT-bearing steps, so a repository-code execution line added to the credential-free budget step survived the suite (witnessed by mutation). The no-repository-code invariant is a job-wide safety premise of the ECS routing: even a credential-free step that runs a planted file leaves a survivor on the persistent host. Pin it on every step's comment-stripped live lines, and align both design-doc versions with the widened coverage and the witnessed mutation set.

Co-authored-by: Qwen-Coder <[email protected]>

* docs(ci): correct the routing-matrix coverage claim for the triage gate

The validation plan said the triage gate is pinned by the triage meta-test AND the ci-runner-routing matrix, but the matrix never reads either authorize job (it pins the sibling pool jobs). State the actual coverage in both language versions.

Co-authored-by: Qwen-Coder <[email protected]>

---------

Co-authored-by: Qwen-Coder <[email protected]>

scripts/tests/qwen-pr-review-workflow.test.js

@@ -127,6 +127,90 @@ describe('qwen pr review runner routing', () => {
     expect(runsOn).not.toContain('ecs-qwen');
     expect(runsOn).not.toContain('ecs-agent');
   });
+
+  it('routes the command gate to ECS for payload-less events, keeps the fork guard', () => {
+    // authorize is the write-permission gate for /review and /resolve. It
+    // loads CI_BOT_PAT but checks out nothing and runs no agent, so the
+    // persistent pool is safe whenever the trigger carries no fork code.
+    // issue_comment and workflow_dispatch have no github.event.pull_request
+    // payload: the old same-repo-only guard fell through to ubuntu-latest for
+    // every command, parking the gate in the hosted queue for hours (#15286).
+    // This removes the GATE wait only: the saving lands where the downstream
+    // job is already pool-routed (/review, /triage, /tmux, /verify); /resolve
+    // end-to-end latency is unchanged because resolve-pr stays on ubuntu-latest.
+    // Pin the exact expression so neither the new ECS disjuncts nor the fork
+    // guard / hosted fallback can be dropped silently.
+    const runsOn = String(parse(workflow).jobs['authorize']['runs-on']);
+
+    expect(runsOn).toBe(
+      "${{ (github.repository == 'QwenLM/qwen-code' && vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true' && ((github.event.pull_request && github.event.pull_request.head.repo.full_name == github.repository) || github.event_name == 'issue_comment' || github.event_name == 'workflow_dispatch')) && fromJSON('[\"self-hosted\", \"linux\", \"x64\", \"ecs-qwen\"]') || fromJSON('[\"ubuntu-latest\"]') }}",
+    );
+    // The command events must reach the persistent pool...
+    expect(runsOn).toContain("github.event_name == 'issue_comment'");
+    expect(runsOn).toContain("github.event_name == 'workflow_dispatch'");
+    // ...while PR-payload events keep the same-repo fork guard, and the
+    // hosted fallback + ECS kill switch stay for forks / disabled routing.
+    expect(runsOn).toContain(
+      'github.event.pull_request.head.repo.full_name == github.repository',
+    );
+    expect(runsOn).toContain("vars.MAINTAINER_ECS_RUNNER_DISABLED != 'true'");
+    expect(runsOn).toContain('fromJSON(\'["ubuntu-latest"]\')');
+    // The ECS routing above is only safe while authorize stays checkout-free:
+    // a `uses:` step (e.g. actions/checkout to read a label or config) would
+    // execute contributor-authored files on the persistent pool with
+    // CI_BOT_PAT in the step env while the expression pin stayed green. Scope
+    // to authorize — other jobs in this workflow legitimately check out.
+    const authorizeJob = parse(workflow).jobs['authorize'];
+    expect(authorizeJob.steps.every((s) => s.uses === undefined)).toBe(true);
+    // Pin the run: half of the premise too: the step inventory is exactly
+    // the one permission step, and no event value is spliced into the shell
+    // — everything must keep arriving through env:. A run-only debug step
+    // interpolating github.event.comment.body adds no `uses:` key and would
+    // slip past the assertion above.
+    expect(authorizeJob.steps.map((s) => s.name)).toEqual([
+      'Check principal write permission',
+    ]);
+    for (const s of authorizeJob.steps) {
+      expect(String(s.run ?? '')).not.toContain('${{');
+    }
+    // On the persistent pool the step must also pin gh's config resolution:
+    // GH_HOST plus a private, guarded GH_CONFIG_DIR, like qwen-autofix.yml's
+    // route/review-scan. Without them a planted shared ~/.config/gh
+    // (http_unix_socket) receives CI_BOT_PAT and can forge the permission
+    // response that downstream jobs consume as should_review.
+    const authorizeStep = authorizeJob.steps.find(
+      (s) => s.id === 'principal_permission',
+    );
+    // Evaluate the live (comment-stripped) lines, not mere substrings: an
+    // unguarded `export GH_CONFIG_DIR="$(mktemp ...)"` one-liner (a failed
+    // mktemp then leaves it empty and gh falls back to the shared config) or
+    // a preamble relocated below the first gh call must both red this test.
+    const live = String(authorizeStep.run)
+      .split('\n')
+      .filter((l) => !l.trim().startsWith('#'))
+      .join('\n');
+    // Fail-closed guarded mint in the sibling route/review-scan shape.
+    const guard =
+      'if ! GH_CONFIG_DIR="$(mktemp -d "${RUNNER_TEMP}/authorize-gh-config.XXXXXX")"; then';
+    expect(live).toContain('export GH_HOST=github.com');
+    expect(live).toContain(guard);
+    expect(live).toMatch(/exit 1\n\s*fi\n\s*export GH_CONFIG_DIR\n/);
+    // Both pins must be exported BEFORE the first permission API call.
+    const api = live.indexOf(
+      'gh api "repos/${GITHUB_REPOSITORY}/collaborators/',
+    );
+    expect(api).toBeGreaterThan(-1);
+    expect(live.indexOf('export GH_HOST=github.com')).toBeLessThan(api);
+    expect(live.indexOf(guard)).toBeLessThan(api);
+    // The body must execute no repository code: on the pool the cwd is the
+    // reused $GITHUB_WORKSPACE, which other lanes fill with contributor
+    // files, so a relative-path or interpreter invocation here would run a
+    // planted file with CI_BOT_PAT in the environment.
+    expect(live).not.toMatch(/(^|[\s;&|(`'"])(\.{1,2}\/)/m);
+    expect(live).not.toMatch(
+      /(^|[\s;&|(`])(?:[^\s;&|()`'"]*\/)?(source|bash|sh|node|python[0-9.]*|npx|npm|pnpm|yarn)(\s|$)/m,
+    );
+  });
 });
 
 function runReviewStep() {

scripts/tests/qwen-triage-workflow.test.js

@@ -1594,6 +1594,7 @@ describe('qwen-triage verify workflow', () => {
           GITHUB_REPOSITORY: 'QwenLM/qwen-code',
           GITHUB_STEP_SUMMARY: '/dev/null',
           GITHUB_OUTPUT: out,
+          RUNNER_TEMP: dir,
           EVENT_NAME: 'issue_comment',
           COMMENT_BODY: commentBody,
           ISSUE_AUTHOR: author,
@@ -4127,6 +4128,7 @@ describe('qwen-triage verify hardening round 2', () => {
           GITHUB_REPOSITORY: 'QwenLM/qwen-code',
           GITHUB_STEP_SUMMARY: '/dev/null',
           GITHUB_OUTPUT: out,
+          RUNNER_TEMP: dir,
           EVENT_NAME: 'issue_comment',
           COMMENT_BODY: commentBody,
           ISSUE_AUTHOR: author,
@@ -4178,6 +4180,7 @@ describe('qwen-triage verify hardening round 2', () => {
           GITHUB_REPOSITORY: 'QwenLM/qwen-code',
           GITHUB_STEP_SUMMARY: '/dev/null',
           GITHUB_OUTPUT: out,
+          RUNNER_TEMP: dir,
           EVENT_NAME: 'issue_comment',
           COMMENT_BODY: '@qwen-code /verify',
           ISSUE_AUTHOR: '',