fix(triage): carry the /verify lane's hardening across to /tmux

.github/workflows/qwen-triage.yml

@@ -973,7 +973,12 @@ jobs:
           set -euo pipefail
           apt-get install -y --no-install-recommends tmux util-linux
 
-          npm install -g --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest'
+          # Run the global install from RUNNER_TEMP: the persistent workspace
+          # still holds the PREVIOUS run's checked-out tree here, and npm
+          # reads a cwd .npmrc — whose settings (script-shell, hooks) a
+          # --registry flag does not override — into a root-privileged
+          # install. Same fix as the verify lane.
+          (cd "${RUNNER_TEMP:?}" && npm install -g --registry=https://registry.npmjs.org '@qwen-code/qwen-code@latest')
           qwen --version
           tmux -V
 
@@ -1062,6 +1067,14 @@ jobs:
             exit 1
           fi
 
+          # `npm ci`/`npm run build` just ran the PR's lifecycle scripts, which
+          # can plant a `tmp/<name>-tmux-<ts>/` directory holding a report.md
+          # and a transcript. The collector below globs `*-tmux-*` and the
+          # publisher takes the first match, so a planted dir can supply the
+          # comment's contents. Sweep after the last PR-controlled process and
+          # before the agent starts; from here only the agent writes them.
+          find tmp -maxdepth 2 -type d -name '*-tmux-*' -exec rm -rf {} + 2>/dev/null || true
+
           # Bypass the runner proxy before launching qwen: the proxy cuts the
           # SSE stream to the model host, and qwen reads HTTP(S)_PROXY directly
           # without honoring NO_PROXY. Clear proxy env for qwen itself while
@@ -1133,17 +1146,28 @@ jobs:
           unset ACTIONS_RUNTIME_TOKEN ACTIONS_RUNTIME_URL ACTIONS_CACHE_URL
 
           start_openai_proxy() {
-            local proxy_port proxy_script
-            proxy_port=8787
-            proxy_script="${RUNNER_TEMP:-/tmp}/qwen-openai-proxy.js"
+            local proxy_port proxy_script proxy_nonce port_file
+            # PR lifecycle code ran before this step and could have left a
+            # server on a fixed port: the real proxy would die with
+            # EADDRINUSE while the health probe succeeded against the
+            # squatter, and qwen would take its completions. Proven
+            # exploitable on the verify lane; same defences here — ephemeral
+            # port reported through a root-owned file, a per-run nonce the
+            # health endpoint must echo, and a liveness check on our PID.
+            proxy_script="${RUNNER_TEMP:-/tmp}/qwen-openai-proxy-tmux.js"
+            port_file="${RUNNER_TEMP:-/tmp}/qwen-openai-proxy-tmux.port"
+            proxy_nonce="$(head -c 24 /dev/urandom | od -An -tx1 | tr -d ' \n')"
+            rm -f "$port_file"
             cat > "$proxy_script" <<'NODE'
           const http = require('node:http');
+          const { writeFileSync } = require('node:fs');
           const { Readable } = require('node:stream');
 
-          const port = Number(process.argv[2]);
+          const portFile = process.argv[2];
           const baseUrl = process.env.REVIEW_OPENAI_BASE_URL;
           const apiKey = process.env.REVIEW_OPENAI_API_KEY;
-          if (!baseUrl || !apiKey || !Number.isInteger(port)) {
+          const nonce = process.env.QWEN_PROXY_NONCE;
+          if (!baseUrl || !apiKey || !portFile || !nonce) {
             console.error('missing proxy configuration');
             process.exit(1);
           }
@@ -1153,8 +1177,9 @@ jobs:
 
           const server = http.createServer(async (req, res) => {
             if (req.url === '/__health') {
-              res.writeHead(204);
-              res.end();
+              // Identity, not just liveness: a squatter cannot know the nonce.
+              res.writeHead(200, { 'content-type': 'text/plain' });
+              res.end(nonce);
               return;
             }
 
@@ -1227,27 +1252,41 @@ jobs:
             }
           });
 
-          server.listen(port, '127.0.0.1');
+          // Port 0: let the OS choose, then publish it where only root can write.
+          server.listen(0, '127.0.0.1', () => {
+            writeFileSync(portFile, String(server.address().port));
+          });
           NODE
 
             REVIEW_OPENAI_API_KEY="$REVIEW_OPENAI_API_KEY" \
               REVIEW_OPENAI_BASE_URL="$REVIEW_OPENAI_BASE_URL" \
-              node "$proxy_script" "$proxy_port" &
+              QWEN_PROXY_NONCE="$proxy_nonce" \
+              node "$proxy_script" "$port_file" &
             OPENAI_PROXY_PID=$!
             trap 'kill "$OPENAI_PROXY_PID" 2>/dev/null || true' EXIT
 
-            for _ in 1 2 3 4 5; do
-              if curl -fsS "http://127.0.0.1:${proxy_port}/__health" >/dev/null; then
-                break
-              fi
+            proxy_port=''
+            for _ in 1 2 3 4 5 6 7 8 9 10; do
               if ! kill -0 "$OPENAI_PROXY_PID" 2>/dev/null; then
                 echo "::error::OpenAI proxy exited before becoming ready"
                 exit 1
               fi
+              if [ -z "$proxy_port" ] && [ -s "$port_file" ]; then
+                proxy_port="$(tr -cd '0-9' < "$port_file")"
+              fi
+              if [ -n "$proxy_port" ] &&
+                 [ "$(curl -fsS "http://127.0.0.1:${proxy_port}/__health" 2>/dev/null)" = "$proxy_nonce" ]; then
+                break
+              fi
+              proxy_port=''
               sleep 1
             done
-            if ! curl -fsS "http://127.0.0.1:${proxy_port}/__health" >/dev/null; then
-              echo "::error::OpenAI proxy did not become ready"
+            # All three must hold: the PID we started is alive, the port it
+            # reported, and the nonce echoed back.
+            if [ -z "$proxy_port" ] ||
+               ! kill -0 "$OPENAI_PROXY_PID" 2>/dev/null ||
+               [ "$(curl -fsS "http://127.0.0.1:${proxy_port}/__health" 2>/dev/null)" != "$proxy_nonce" ]; then
+              echo "::error::OpenAI proxy did not become ready (or another process answered on its port)"
               exit 1
             fi
 
@@ -1347,7 +1386,16 @@ jobs:
         run: |-
           set -uo pipefail
           [ -e .git ] || exit 0
-          rm -rf .qwen/tmp/review-pr-* 2>/dev/null || true
+          # Never descend through a PR-writable parent: PR code ran in this
+          # workspace, so `.qwen` or `.qwen/tmp` can be a symlink pointing
+          # outside it (verified on the verify lane: the glob then deletes
+          # the link target's contents as root).
+          [ -L .qwen ] && rm -f .qwen
+          if [ -L .qwen/tmp ]; then
+            rm -f .qwen/tmp
+          elif [ -d .qwen/tmp ]; then
+            rm -rf .qwen/tmp/review-pr-* 2>/dev/null || true
+          fi
           find tmp -maxdepth 2 -type d -name '*-tmux-*' -exec rm -rf {} + 2>/dev/null || true
           git worktree prune -v || true
 
@@ -1415,16 +1463,32 @@ jobs:
             local summary="$1" file="$2" max="$3" content truncated='' summary_html
             [ -n "$file" ] && [ -f "$file" ] || return 0
             summary_html="$(printf '%s' "$summary" | html_escape)"
-            if [ "$(wc -c < "$file")" -gt "$max" ]; then
-              truncated=$'\n\n...truncated -- full log in the run artifacts.'
-            fi
-            if ! content="$(
+            # Escape FIRST, then cap. Escaping inflates every & < > by 4-5
+            # bytes, so a raw-side cap can push the assembled body past
+            # GitHub's 65,536-char comment limit, 422 the post, and leave no
+            # comment at all. Same fix the verify lane carries; truncation is
+            # done on a character boundary via node, because BSD `iconv -c`
+            # passes an incomplete trailing UTF-8 sequence through unchanged.
+            local esc_file="${TMPDIR:-/tmp}/tmux-emit-$$"
+            if ! (
               set -o pipefail
-              head -c "$max" "$file" | tr -d '\000' | html_escape
-            )"; then
+              head -c 400000 "$file" | tr -d '\000' | html_escape > "$esc_file"
+            ); then
               echo "::warning::emit_block failed while rendering $summary; see run artifacts." >&2
+              rm -f "$esc_file"
               content='<em>Log could not be rendered; see run artifacts.</em>'
-            elif [ -n "$truncated" ]; then
+            else
+              if [ "$(wc -c < "$esc_file")" -gt "$max" ] ||
+                 [ "$(wc -c < "$file")" -gt 400000 ]; then
+                truncated=$'\n\n...truncated -- full log in the run artifacts.'
+              fi
+              content="$(node -e '
+                const fs = require("node:fs");
+                const [file, max] = process.argv.slice(1);
+                const buf = fs.readFileSync(file).subarray(0, Number(max));
+                process.stdout.write(new TextDecoder("utf-8").decode(buf).replace(/�+$/, ""));
+              ' "$esc_file" "$max")" || content="$(head -c "$max" "$esc_file")"
+              rm -f "$esc_file"
               content="${content}${truncated}"
             fi
             printf '<details>\n<summary>%s</summary>\n\n<pre><code>\n' "$summary_html"

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

@@ -110,7 +110,10 @@ describe('qwen-triage tmux workflow', () => {
     expect(postStep).toContain('html_escape()');
     expect(postStep).toContain("tr -d '\\000'");
     expect(postStep).toContain('Log could not be rendered');
-    expect(postStep).toContain('if ! content="$(');
+    // The escape now writes to a file and the cap is applied afterwards, so
+    // the guarantee is "a render failure is caught", not the old inline
+    // capture shape. See the tmux-lane-parity suite for the cap itself.
+    expect(postStep).toContain('html_escape > "$esc_file"');
     expect(postStep).toContain('set -o pipefail');
     expect(postStep).toContain('::warning::emit_block failed');
     expect(postStep).toContain(
@@ -2049,6 +2052,12 @@ describe('qwen-triage verify maintainer-review round', () => {
     expect(publishJob).toContain(
       'needs.verify.outputs.pr_number || github.event.issue.number',
     );
+    // Same one-line class in the tmux sibling: a job cancelled while
+    // pending never evaluates its outputs either, so without the fallback
+    // publish-tmux hits the same null guard and posts nothing.
+    expect(job('publish-tmux')).toContain(
+      'needs.tmux-testing.outputs.pr_number || github.event.issue.number',
+    );
     // ...and the step that warned on the inverted premise is gone.
     expect(job('authorize')).not.toContain('Report saturated verify queue');
 
@@ -2183,3 +2192,139 @@ describe('qwen-triage verify maintainer-review round', () => {
     expect(runStep).not.toContain('proxy error: ${error instanceof Error');
   });
 });
+
+describe('qwen-triage tmux lane parity', () => {
+  // The verify lane earned these five controls the hard way; the tmux lane
+  // executes the same untrusted PR code on the same persistent pool, so
+  // leaving them out was a gap rather than a scope boundary.
+
+  // A fixed proxy port is squattable by a detached lifecycle process: the
+  // real proxy dies EADDRINUSE while the health probe succeeds against the
+  // squatter, and the agent takes ITS chat completions.
+  it('binds the tmux model proxy to an ephemeral port and authenticates it', () => {
+    const runStep = stepIn('tmux-testing', 'Run tmux real-user testing');
+    expect(runStep).not.toContain('proxy_port=8787');
+    expect(runStep).toContain("server.listen(0, '127.0.0.1'");
+    expect(runStep).toContain('QWEN_PROXY_NONCE');
+    expect(runStep).toContain('!= "$proxy_nonce"');
+    expect(runStep).toContain('kill -0 "$OPENAI_PROXY_PID"');
+
+    // Start the real proxy with the old fixed port occupied.
+    const proxy = runStep.match(/<<'NODE'\n([\s\S]*?)\n\s*NODE\n/)?.[1];
+    expect(proxy).toBeTruthy();
+    const dir = mkdtempSync(join(tmpdir(), 'tmux-proxy-'));
+    try {
+      writeFileSync(join(dir, 'proxy.js'), proxy.replace(/^ {10}/gm, ''));
+      writeFileSync(
+        join(dir, 'upstream.js'),
+        [
+          "const http = require('node:http');",
+          "const fs = require('node:fs');",
+          "const s = http.createServer((q, r) => { r.writeHead(200); r.end('{}'); });",
+          "s.listen(0, '127.0.0.1', () => fs.writeFileSync(process.argv[2], String(s.address().port)));",
+        ].join('\n'),
+      );
+      const driver = [
+        'set -u',
+        'node "$1/upstream.js" "$1/up.port" & UP=$!',
+        'for _ in 1 2 3 4 5 6 7 8 9 10; do [ -s "$1/up.port" ] && break; sleep 0.3; done',
+        'REVIEW_OPENAI_BASE_URL="http://127.0.0.1:$(cat "$1/up.port")/v1" \\',
+        '  REVIEW_OPENAI_API_KEY=k QWEN_PROXY_NONCE=n0nce \\',
+        '  node "$1/proxy.js" "$1/px.port" & PX=$!',
+        'for _ in 1 2 3 4 5 6 7 8 9 10; do [ -s "$1/px.port" ] && break; sleep 0.3; done',
+        'P="$(cat "$1/px.port")"',
+        'echo "port=$P"',
+        'echo "health=$(curl -sS "http://127.0.0.1:$P/__health")"',
+        'kill $UP $PX 2>/dev/null',
+      ].join('\n');
+      const out = spawnSync('bash', ['-c', driver, '_', dir], {
+        encoding: 'utf8',
+        timeout: 60000,
+      }).stdout;
+      // An OS-chosen port, and identity proven by the nonce rather than a
+      // bare 204 any squatter could return.
+      expect(out).toMatch(/port=\d+/);
+      expect(out).not.toContain('port=8787');
+      expect(out).toContain('health=n0nce');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  // PR lifecycle scripts run before the agent and can plant a
+  // tmp/<name>-tmux-<ts>/ directory whose report.md and transcript the
+  // collector would hand to the publisher.
+  it('sweeps planted tmux artifacts before the agent starts', () => {
+    const runStep = stepIn('tmux-testing', 'Run tmux real-user testing');
+    const sweep =
+      "find tmp -maxdepth 2 -type d -name '*-tmux-*' -exec rm -rf {} +";
+    const sweepAt = runStep.indexOf(sweep);
+    expect(sweepAt).toBeGreaterThan(-1);
+    // Before the proxy and the agent launch, after the build.
+    expect(sweepAt).toBeLessThan(runStep.indexOf('start_openai_proxy'));
+  });
+
+  // The global install must not read the previous PR's .npmrc: a --registry
+  // flag does not override script-shell or hooks.
+  it('runs the tmux global install away from the checked-out tree', () => {
+    expect(stepIn('tmux-testing', 'Install tmux runner tools')).toContain(
+      '(cd "${RUNNER_TEMP:?}" && npm install -g',
+    );
+  });
+
+  // Cleanup must not descend through a PR-writable parent.
+  it('unlinks tmux-lane symlinks instead of globbing through them', () => {
+    const code = job('tmux-testing')
+      .split('\n')
+      .filter((l) => !/^\s*#/.test(l))
+      .join('\n');
+    expect(code).toContain('[ -L .qwen ] && rm -f .qwen');
+    expect(code).toContain('if [ -L .qwen/tmp ]; then');
+  });
+
+  // Escaping inflates & < > by 4-5 bytes each, so a raw-side cap can push
+  // the assembled comment past GitHub's 65,536-char limit and 422 the post.
+  it('caps the tmux comment after escaping, on a character boundary', () => {
+    const publish = stepIn('publish-tmux', 'Post tmux result comment');
+    const escFirst = publish.indexOf('html_escape > "$esc_file"');
+    expect(escFirst).toBeGreaterThan(-1);
+    expect(publish).toContain('TextDecoder');
+    expect(publish).not.toContain('head -c "$max" "$file" | tr -d');
+
+    // Execute it: dense metacharacter content must stay under the cap and
+    // remain valid UTF-8.
+    const script = publish
+      .match(/run: \|-\n([\s\S]*)$/)?.[1]
+      .replace(/^ {10}/gm, '');
+    const helpers = script.slice(
+      script.indexOf('html_escape()'),
+      script.indexOf('if [ "${TMUX_RESULT:-}"'),
+    );
+    const dir = mkdtempSync(join(tmpdir(), 'tmux-emit-'));
+    try {
+      const dense = join(dir, 'dense.log');
+      writeFileSync(dense, '<T<U>>&'.repeat(7300));
+      const utf8 = join(dir, 'utf8.log');
+      // One ASCII byte of padding so the cut lands inside a 3-byte char.
+      writeFileSync(utf8, `x${'验证证据链路测试'.repeat(8000)}`);
+      const emit = (file) => {
+        const proc = spawnSync(
+          'bash',
+          ['-c', `${helpers}\nemit_block 'Log' "$1" 20000`, '_', file],
+          { encoding: 'utf8', maxBuffer: 20 * 1024 * 1024 },
+        );
+        expect(proc.status).toBe(0);
+        return proc.stdout;
+      };
+      const capped = emit(dense);
+      expect(Buffer.byteLength(capped)).toBeLessThan(65536);
+      expect(capped).toContain('truncated');
+      expect(capped).not.toContain('<T<U>');
+      const cut = emit(utf8);
+      expect(Buffer.from(cut, 'utf8').toString('utf8')).toBe(cut);
+      expect(cut).not.toContain('\ufffd');
+    } finally {
+      rmSync(dir, { recursive: true, force: true });
+    }
+  });
+});