diff --git a/.github/actions/register-gstack-skills/action.yml b/.github/actions/register-gstack-skills/action.yml index 62f534b4d..1a96953d1 100644 --- a/.github/actions/register-gstack-skills/action.yml +++ b/.github/actions/register-gstack-skills/action.yml @@ -46,6 +46,9 @@ runs: fi ln -snf "$REPO" "$SKILLS_DIR/gstack" for s in $SKILLS; do + # Input validation before rm -rf: a future caller passing '..' or a + # path segment would delete $HOME/.claude (claude adversarial). + case "$s" in */*|.|..|'') echo "ERROR: invalid skill name: '$s'" >&2; exit 1;; esac rm -rf "${SKILLS_DIR:?}/$s" mkdir -p "$SKILLS_DIR/$s" cp "$REPO/$s/SKILL.md" "$SKILLS_DIR/$s/SKILL.md" diff --git a/.github/actions/restore-deps/action.yml b/.github/actions/restore-deps/action.yml index ce81888ea..f3bd7bc47 100644 --- a/.github/actions/restore-deps/action.yml +++ b/.github/actions/restore-deps/action.yml @@ -20,5 +20,8 @@ runs: rm -rf node_modules cp -r /opt/node_modules_cache node_modules else - bun install + # Frozen: this composite is canonical for lanes that run PR code + # with provider keys in env — a drifted lockfile must fail loudly, + # never silently re-resolve versions (claude adversarial). + bun install --frozen-lockfile fi diff --git a/.github/workflows/ci-image.yml b/.github/workflows/ci-image.yml index cfd896ab4..17d03b5f6 100644 --- a/.github/workflows/ci-image.yml +++ b/.github/workflows/ci-image.yml @@ -36,7 +36,7 @@ jobs: contents: read packages: write steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 # Copy lockfile + package.json into Docker build context - run: cp package.json bun.lock .github/docker/ && cp -R patches .github/docker/patches @@ -48,7 +48,7 @@ jobs: - id: meta run: echo "tag=ghcr.io/${{ github.repository }}/ci:${{ hashFiles('.github/docker/Dockerfile.ci', 'bun.lock', 'patches/**') }}" >> "$GITHUB_OUTPUT" - - uses: docker/login-action@v4 + - uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4 with: registry: ghcr.io username: ${{ github.actor }} @@ -69,10 +69,10 @@ jobs: # Registry cache export needs a docker-container builder — the default # `docker` driver hard-errors on cache-to. - if: steps.check.outputs.exists == 'false' - uses: docker/setup-buildx-action@v4 + uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4 - if: steps.check.outputs.exists == 'false' - uses: docker/build-push-action@v7 + uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7 with: context: .github/docker file: .github/docker/Dockerfile.ci diff --git a/.github/workflows/evals-periodic.yml b/.github/workflows/evals-periodic.yml index 0256b34d3..67c92ef1a 100644 --- a/.github/workflows/evals-periodic.yml +++ b/.github/workflows/evals-periodic.yml @@ -292,7 +292,11 @@ jobs: run: | set +e EVALS_TIER=periodic bun run scripts/test-paid-shards.ts --tier periodic --report /tmp/paid-report | tee /tmp/report.txt - echo "exit=$?" >> "$GITHUB_OUTPUT" + # PIPESTATUS[0], NOT $?: GitHub's default run-step shell is + # `bash -e {0}` with NO pipefail, so $? after the pipe is tee's + # exit (always 0) — the fail-closed gate was silently fail-open + # (caught by the ship review army; the wiring test now pins this). + echo "exit=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT" # A red weekly lane nobody must action is waste — upsert ONE tracking # issue (never a new issue per week) with the reconciliation output, so diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 521b225f1..9e250ac3c 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -242,13 +242,18 @@ jobs: # missing slice artifact reading as green is the class this lane kills. if: always() && needs.plan-slices.result == 'success' timeout-minutes: 5 + # contents:read ONLY — this job executes PR-authored code (bun install + # lifecycle scripts + the reconcile runner from the PR checkout), so it + # must never hold a write-scoped token. The PR comment lives in the + # separate slices-comment job below, which runs NO repo code: a + # $GITHUB_ENV/BASH_ENV persistence trick is job-scoped, so the split is + # the trust boundary (codex adversarial finding, 2026-08-31 — the old + # matrix-era report job had this separation and the consolidation had + # regressed it). permissions: contents: read - pull-requests: write - # The comment upsert calls the REST `/issues/{n}/comments` endpoints - # (gh api ... issues/comments). With GITHUB_TOKEN those are gated by the - # `issues` permission, not `pull-requests` (#1802 CI fix). - issues: write + outputs: + reconcile-exit: ${{ steps.reconcile.outputs.exit }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: @@ -276,31 +281,82 @@ jobs: run: | set +e EVALS_TIER=gate bun run scripts/test-paid-shards.ts --tier gate --report /tmp/paid-report | tee /tmp/report.txt - echo "exit=$?" >> "$GITHUB_OUTPUT" + # PIPESTATUS[0], NOT $?: GitHub's default run-step shell is + # `bash -e {0}` with NO pipefail, so $? after the pipe is tee's + # exit (always 0) — the fail-closed gate was silently fail-open + # (caught by the ship review army; the wiring test now pins this). + echo "exit=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT" - # PR comment sourced from the slice artifacts' eval-store JSONs - # (shards//*.json inside each paid-slice-N artifact). Replaces the - # deleted matrix-artifact report job; keeps the "## E2E Evals" marker so - # the upsert keeps updating the same comment across the migration. Runs - # even when reconciliation failed — a red lane on the PR is the point. + - name: Upload reconciliation output for the comment job + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: report-verdict + path: /tmp/report.txt + if-no-files-found: ignore + retention-days: 30 + + - name: Fail the workflow when reconciliation failed + if: steps.reconcile.outputs.exit != '0' + run: exit 1 + + # PR comment in its OWN job with the write token and ZERO repo code: no + # checkout, no bun install — only downloaded artifacts, jq, and gh. See the + # trust-boundary note on slices-report. + slices-comment: + runs-on: ubicloud-standard-2 + needs: slices-report + if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository && needs.slices-report.result != 'skipped' + timeout-minutes: 5 + permissions: + pull-requests: write + # The comment upsert calls the REST `/issues/{n}/comments` endpoints + # (gh api ... issues/comments). With GITHUB_TOKEN those are gated by the + # `issues` permission, not `pull-requests` (#1802 CI fix). + issues: write + steps: + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + pattern: paid-slice-[0-9]* + path: /tmp/paid-report + merge-multiple: true + + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + name: report-verdict + path: /tmp/verdict + continue-on-error: true + + # Sourced from the slice artifacts' eval-store JSONs. Keeps the + # "## E2E Evals" marker so the upsert keeps updating the same comment. + # Runs even when reconciliation failed — a red lane on the PR is the point. - name: Post PR comment - if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - RECONCILE_EXIT: ${{ steps.reconcile.outputs.exit }} + RECONCILE_EXIT: ${{ needs.slices-report.outputs.reconcile-exit }} run: | # shellcheck disable=SC2086,SC2059 RESULTS=$(find /tmp/paid-report -name '*.json' ! -name 'manifest.json' ! -name 'slice-*.json' ! -name '_partial*' 2>/dev/null | sort) - TOTAL=0; PASSED=0; FAILED=0; COST="0" + TOTAL=0; PASSED=0; FAILED=0; FLAKY=0; COST="0" SUITE_LINES="" for f in $RESULTS; do if ! jq -e '.total_tests' "$f" >/dev/null 2>&1; then echo "Skipping malformed JSON: $f" continue fi - T=$(jq -r '.total_tests // 0' "$f") - P=$(jq -r '.passed // 0' "$f") - F=$(jq -r '.failed // 0' "$f") + # FINAL-attempt accounting: eval-store keeps EVERY retry attempt + # as its own record (that's the flake telemetry), so counting raw + # records marks a pass-on-retry as a failure and inflates totals. + # Group by test name, judge the LAST record — flaky passes render + # as the ⚠ line, never as ❌ (WS1 policy: recorded, not blocking). + # Guarded: a file with total_tests but a null/non-array `tests` + # passes the -e probe, the group_by then fails, and an empty $T + # would abort the whole step under bash -e ([ "" -eq 0 ] is an + # error) — killing the comment on exactly the corrupted-artifact + # runs where the red evidence matters (claude adversarial). + STATS=$(jq -r '[.tests | group_by(.name)[] | last] as $final | "\($final | length) \([$final[] | select(.passed)] | length) \([$final[] | select(.passed | not)] | length) \(.flaky_retries // [] | length)"' "$f" 2>/dev/null) || { echo "Skipping malformed tests[] in: $f"; continue; } + read -r T P F FL <<< "$STATS" + [ -z "$T" ] && { echo "Skipping malformed tests[] in: $f"; continue; } C=$(jq -r '.total_cost_usd // 0' "$f") TIER=$(jq -r '.tier // "unknown"' "$f") SHARD=$(jq -r '.shard // "-"' "$f") @@ -308,9 +364,11 @@ jobs: TOTAL=$((TOTAL + T)) PASSED=$((PASSED + P)) FAILED=$((FAILED + F)) + FLAKY=$((FLAKY + FL)) COST=$(echo "$COST + $C" | bc) STATUS_ICON="✅" [ "$F" -gt 0 ] && STATUS_ICON="❌" + [ "$F" -eq 0 ] && [ "$FL" -gt 0 ] && STATUS_ICON="✅⚠" SUITE_LINES="${SUITE_LINES}| ${TIER}/${SHARD} | ${P}/${T} | ${STATUS_ICON} | \$${C} |\n" done @@ -319,7 +377,7 @@ jobs: BODY="## E2E Evals: ${STATUS} - **${PASSED}/${TOTAL}** tests passed | **\$${COST}** total cost | reconcile exit: ${RECONCILE_EXIT:-missing} + **${PASSED}/${TOTAL}** tests passed | **\$${COST}** total cost | reconcile exit: ${RECONCILE_EXIT:-missing}$([ "$FLAKY" -gt 0 ] && printf ' | ⚠ %s flaky pass(es) — recorded, not blocking' "$FLAKY") | Shard | Result | Status | Cost | |-------|--------|--------|------| @@ -328,7 +386,7 @@ jobs:
Fail-closed reconciliation \`\`\` - $(tail -c 4000 /tmp/report.txt 2>/dev/null || echo '(no reconciliation output)') + $(tail -c 4000 /tmp/verdict/report.txt 2>/dev/null || echo '(no reconciliation output)') \`\`\`
@@ -339,9 +397,7 @@ jobs: FAILURES="" for f in $RESULTS; do if ! jq -e '.failed' "$f" >/dev/null 2>&1; then continue; fi - F=$(jq -r '.failed // 0' "$f") - [ "$F" -eq 0 ] && continue - FAILS=$(jq -r '.tests[] | select(.passed == false) | "- ❌ \(.name): \(.exit_reason // "unknown")"' "$f" 2>/dev/null || echo "- ⚠️ $(basename "$f"): parse error") + FAILS=$(jq -r '[.tests | group_by(.name)[] | last | select(.passed == false)][] | "- ❌ \(.name): \(.exit_reason // "unknown")"' "$f" 2>/dev/null || echo "- ⚠️ parse error") FAILURES="${FAILURES}${FAILS}\n" done BODY="${BODY} @@ -359,7 +415,3 @@ jobs: else gh pr comment "${{ github.event.pull_request.number }}" --body "$BODY" fi - - - name: Fail the workflow when reconciliation failed - if: steps.reconcile.outputs.exit != '0' - run: exit 1 diff --git a/.github/workflows/free-tests.yml b/.github/workflows/free-tests.yml index f7dd8e7ab..3dcfc1ad5 100644 --- a/.github/workflows/free-tests.yml +++ b/.github/workflows/free-tests.yml @@ -48,7 +48,7 @@ jobs: runs-on: ubicloud-standard-8 timeout-minutes: 20 steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: persist-credentials: false @@ -144,7 +144,7 @@ jobs: # GREEN — that's the point — so its evidence must survive green runs. - name: Upload flake ledger if: always() - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: flake-ledger path: ${{ runner.temp }}/flake-ledger.jsonl @@ -158,7 +158,7 @@ jobs: # need a local re-run, which fork contributors can't do on this image. - name: Upload shard logs on failure if: failure() - uses: actions/upload-artifact@v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: free-test-shard-logs path: /tmp/gstack-free-test-*.log diff --git a/test/evals-workflow-wiring.test.ts b/test/evals-workflow-wiring.test.ts index b4b2b8b0b..c49a10a30 100644 --- a/test/evals-workflow-wiring.test.ts +++ b/test/evals-workflow-wiring.test.ts @@ -75,13 +75,47 @@ describe('evals.yml sliced-lane wiring (post-matrix)', () => { expect(matrices[0]).toEqual(Array.from({ length: n }, (_, i) => i + 1)); }); - test('the PR comment survived the matrix-report deletion (moved to slices-report)', () => { + test('reconcile exit is captured via PIPESTATUS, never $? after a pipe', () => { + // GitHub's default run-step shell is `bash -e {0}` with NO pipefail, so + // `$?` after `... | tee` is tee's exit — always 0. That made the + // fail-closed reconcile gate silently fail-open (ship review army, + // 2026-08-31). Both lanes must read PIPESTATUS[0]. + for (const [name, source] of [['evals.yml', evalsYml], ['evals-periodic.yml', periodicYml]] as const) { + const reconcileBlocks = [...source.matchAll(/--report[^\n]*\| tee[^\n]*\n([\s\S]{0,400}?)GITHUB_OUTPUT/g)]; + expect(reconcileBlocks.length, `${name}: expected a tee'd reconcile step`).toBeGreaterThanOrEqual(1); + for (const block of reconcileBlocks) { + expect(block[1], `${name} reconcile captures tee's exit, not the runner's`).toContain('PIPESTATUS[0]'); + expect(block[1]).not.toMatch(/exit=\$\?/); + } + } + }); + + test('the PR comment survived the matrix-report deletion (moved to slices-comment)', () => { // Keyed on the upsert marker so the migration keeps updating the SAME // comment; and the job holding it needs the issues permission (#1802). expect(evalsYml).toContain('## E2E Evals'); expect(evalsYml).toMatch(/pull-requests: write/); expect(evalsYml).toMatch(/issues: write/); }); + + test('the write-token job runs ZERO repo code (token/exec separation)', () => { + // slices-report executes PR-authored code (bun install + the reconcile + // runner), so it must hold contents:read ONLY; the write token lives in + // slices-comment, which may only download artifacts and run jq/gh — + // $GITHUB_ENV persistence is job-scoped, so this split IS the trust + // boundary (codex adversarial, 2026-08-31; the matrix-era report job had + // this property and the consolidation briefly regressed it). + const commentJob = evalsYml.slice(evalsYml.indexOf(' slices-comment:')); + expect(commentJob.length).toBeGreaterThan(100); + expect(commentJob).not.toContain('actions/checkout'); + expect(commentJob).not.toContain('bun install'); + expect(commentJob).not.toMatch(/run: .*bun run/); + expect(commentJob).not.toContain('uses: ./'); + // And the code-executing report job must NOT hold write scopes. + const reportJob = evalsYml.slice(evalsYml.indexOf(' slices-report:'), evalsYml.indexOf(' slices-comment:')); + expect(reportJob).not.toMatch(/pull-requests: write/); + expect(reportJob).not.toMatch(/issues: write/); + }); }); describe('evals-periodic.yml sliced-lane wiring', () => {