diff --git a/.github/workflows/deep-review.yml b/.github/workflows/deep-review.yml index a3d3dda680..2185f3caa4 100644 --- a/.github/workflows/deep-review.yml +++ b/.github/workflows/deep-review.yml @@ -48,6 +48,18 @@ jobs: # Single-source the reviewer plugin ref: consumed by the plugin checkout, # the review gate, and the state marker. Bump in one place. PLUGIN_REF: compound-engineering-v3.6.1 + # TEMPORARY: pin the Claude Code CLI to the last version before the + # >=2.1.216 bwrap sandbox regression that breaks every Bash call. + # The SHA-512 is the npm dist.integrity of the + # @anthropic-ai/claude-code@ tarball; the pin step verifies the + # downloaded tarball against it before installing, so a compromised + # registry response cannot substitute the binary the credentialed review + # action later executes. Recompute it when bumping the version: + # npm view @anthropic-ai/claude-code@ dist.integrity + # UNPIN once upstream fixes it (tracked in HDX-4907). + # https://github.com/anthropics/claude-code-action/issues/1547 + CLAUDE_CLI_VERSION: 2.1.215 + CLAUDE_CLI_SHA512: sha512-lsWBvyMyBqg/rOZ06o/HEhlpOzHsH8IBf9RH4u5gzizQ1LWS/oXAh5nqWNUu3hn2d8QkyYg0aseNzMDlGD+3qg== permissions: contents: read pull-requests: write @@ -249,6 +261,64 @@ jobs: ref: ${{ env.PLUGIN_REF }} path: ce-plugin + # --- CLI pin (TEMPORARY) ---------------------------------------------- + # claude-code-action hardcodes the Claude Code CLI version it installs + # (currently 2.1.220) and exposes no version input -- the only override + # is `path_to_claude_code_executable`. CLI >= 2.1.216 has a bwrap sandbox + # regression: whenever subprocess isolation is on (which + # `allowed_non_write_users: '*'` below auto-enables via + # CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1), bwrap aborts EVERY Bash call while + # trying to mask a repo-root `.mcp.json`, with: + # bwrap: Can't create file at /home/.mcp.json: Permission denied + # That kills git/gh and the reviewer fan-out, so the review posts an + # "environment failure" comment -- and the job still reports success. + # CLAUDE_CLI_VERSION is the last version before the regression. Rather + # than piping a mutable remote installer to bash, fetch the immutable + # npm tarball for that version, verify it against the SHA-512 hardcoded + # in this workflow (CLAUDE_CLI_SHA512), and install from the verified + # local tarball -- so trust rests on the hash in this file, not on the + # installer delivery chain or on the binary's spoofable --version output. + # Upstream issue: https://github.com/anthropics/claude-code-action/issues/1547 + # UNPIN once the upstream regression is fixed (tracked in HDX-4907). + - name: Pin Claude Code CLI to a pre-regression version + if: steps.gate.outputs.should_review == 'true' + run: | + set -euo pipefail + TARBALL="$RUNNER_TEMP/claude-code-$CLAUDE_CLI_VERSION.tgz" + curl -fsSL -o "$TARBALL" \ + "https://registry.npmjs.org/@anthropic-ai/claude-code/-/claude-code-$CLAUDE_CLI_VERSION.tgz" + # Integrity gate: hard-fail unless the downloaded tarball matches the + # SHA-512 pinned in this workflow. Everything past this line runs + # only on verified content. + ACTUAL="sha512-$(openssl dgst -sha512 -binary "$TARBALL" | openssl base64 -A)" + if [ "$ACTUAL" != "$CLAUDE_CLI_SHA512" ]; then + echo "::error::Pinned CLI tarball integrity check failed: expected $CLAUDE_CLI_SHA512, got $ACTUAL. Refusing to install." + exit 1 + fi + INSTALL_DIR="$RUNNER_TEMP/claude-cli" + # --ignore-scripts so no dependency lifecycle script runs; the only + # script we execute is install.cjs from the hash-verified tarball, + # invoked explicitly below to link the platform-native binary. That + # binary comes from an optionalDependency exact-pinned (same version) + # by the verified package.json, so the whole chain is anchored to + # CLAUDE_CLI_SHA512. + npm install --prefix "$INSTALL_DIR" --no-audit --no-fund --ignore-scripts "$TARBALL" + node "$INSTALL_DIR/node_modules/@anthropic-ai/claude-code/install.cjs" + CLAUDE_BIN="$INSTALL_DIR/node_modules/.bin/claude" + if [ ! -x "$CLAUDE_BIN" ]; then + echo "::error::Pinned Claude Code CLI not found at $CLAUDE_BIN after install." + exit 1 + fi + # Sanity check (integrity is already guaranteed by the hash above): + # the binary should run and report the pinned version. + INSTALLED_VERSION="$("$CLAUDE_BIN" --version 2>/dev/null || true)" + echo "Installed Claude Code CLI: ${INSTALLED_VERSION:-}" + if ! printf '%s' "$INSTALLED_VERSION" | grep -qE "(^|[^0-9.])${CLAUDE_CLI_VERSION//./\\.}([^0-9.]|$)"; then + echo "::error::Pinned CLI check failed: expected $CLAUDE_CLI_VERSION, got '${INSTALLED_VERSION:-}'." + exit 1 + fi + echo "PINNED_CLAUDE=$CLAUDE_BIN" >> "$GITHUB_ENV" + - name: Run deep review id: review if: steps.gate.outputs.should_review == 'true' @@ -256,6 +326,11 @@ jobs: with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} github_token: ${{ secrets.GITHUB_TOKEN }} # bypasses OIDC auth (required for pull_request_target) + # Pin the CLI to dodge the >=2.1.216 bwrap sandbox regression that + # breaks every Bash call. Remove this input to revert to the action's + # bundled CLI once upstream fixes it. + # https://github.com/anthropics/claude-code-action/issues/1547 + path_to_claude_code_executable: ${{ env.PINNED_CLAUDE }} allowed_bots: dependabot,dependabot[bot],kodiakhq,kodiakhq[bot],github-actions,github-actions[bot],cursor,cursor[bot],claude,claude[bot] allowed_non_write_users: '*' # allow fork-PR contributors to trigger reviews @@ -419,6 +494,100 @@ jobs: --allowedTools "Bash(git:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*),Bash(gh api:*)" --json-schema '{"type":"object","properties":{"review":{"type":"string","description":"Complete markdown review starting with on the first line and ## Deep Review on the second line"}},"required":["review"]}' + # --- Sandbox smoke check ---------------------------------------------- + # PERMANENT guard -- KEEP this even after the CLI pin above is removed. + # It defends against the whole class of "green checkmark on a broken + # sandbox" failures, not just the specific #1547 regression. + # + # This bug's nastiest trait is that the job reports SUCCESS while every + # Bash call inside the action silently fails (the action's `conclusion` + # is derived only from the final result message, never from per-tool + # errors), so a zero-coverage "environment failure" review posts under a + # green checkmark. Detect the failure from the MACHINE-READABLE execution + # transcript rather than the model's free-text review body -- grepping + # the review would false-positive on any PR (like this one) that merely + # discusses the bwrap error. The execution_file is a JSON array of SDK + # messages; failed tool calls appear as `tool_result` blocks with + # `is_error: true` carrying the bwrap error text in `content`. + # + # To avoid false positives on transcripts that merely QUOTE the error + # text (e.g. an errored tool call whose output includes this workflow + # file), a match must (a) be a tool_result correlated to a Bash tool_use + # by id, and (b) carry the bwrap signature at the start of a line, as + # real bwrap stderr does (quoted occurrences are diff-/comment-prefixed). + # + # This step only DETECTS and records the verdict. The job is failed by + # "Fail if reviewer sandbox was unhealthy" below, AFTER the review + # comment is posted -- so a completed review is never discarded, and the + # state marker is omitted for unhealthy runs so the gate fail-opens and + # the next run re-reviews. + # https://github.com/anthropics/claude-code-action/issues/1547 + - name: Check reviewer sandbox health + id: sandbox + if: steps.gate.outputs.should_review == 'true' + env: + EXECUTION_FILE: ${{ steps.review.outputs.execution_file }} + run: | + set -euo pipefail + broken() { + echo "::warning::$1" + { echo "broken=true"; echo "reason=$1"; } >> "$GITHUB_OUTPUT" + exit 0 + } + # A missing, empty, or reshaped transcript on a review run is itself + # suspicious (the SDK iterator can hang before writing the file, or + # crash after truncating it), so treat those as unhealthy rather + # than silently passing with no evidence: jq emits nothing (exit 0) + # on empty input, which would otherwise fall through to + # broken=false and stamp a valid state marker. + if [ -z "${EXECUTION_FILE:-}" ] || [ ! -f "$EXECUTION_FILE" ]; then + broken "No execution transcript from the reviewer (execution_file missing). Cannot confirm the sandbox was healthy." + fi + if [ ! -s "$EXECUTION_FILE" ]; then + broken "Execution transcript exists but is empty. Cannot confirm the sandbox was healthy." + fi + if ! jq -e 'type == "array" and length > 0' "$EXECUTION_FILE" >/dev/null 2>&1; then + broken "Execution transcript is not a non-empty JSON array. Cannot confirm the sandbox was healthy." + fi + # Count errored tool_result blocks that (a) correlate to a Bash + # tool_use by id and (b) carry the bwrap signature at a line start. + # `content` may be a string or an array of text blocks, so normalize + # both. jq reads the file directly -- no pipe, so no + # pipefail/SIGPIPE fail-open. + BROKEN_COUNT=$(jq ' + ([ .[] + | select(.type == "assistant") + | .message.content[]? + | select(.type == "tool_use" and .name == "Bash") + | .id + ]) as $bash_ids + | [ .[] + | select(.type == "user") + | .message.content[]? + | select(.type == "tool_result" and .is_error == true) + | select((.tool_use_id // "") as $id | $bash_ids | index($id)) + | ( .content + | if type == "array" then map(.text // "") | join("\n") + elif type == "string" then . + else tostring end ) + | select(test("(^|\\n)bwrap: .*(Can.t create file|Permission denied)")) + ] | length + ' "$EXECUTION_FILE") + echo "bwrap sandbox-failure Bash tool_results: ${BROKEN_COUNT:-}" + # Belt-and-suspenders: never let a non-numeric count reach the + # comparison below -- `[ "" -gt 0 ]` errors but is set-e-exempt as + # an `if` condition, so it would silently fall through to healthy. + case "$BROKEN_COUNT" in + '' | *[!0-9]*) + broken "Sandbox check could not derive a failure count from the execution transcript (got: '${BROKEN_COUNT:-}')." + ;; + esac + if [ "$BROKEN_COUNT" -gt 0 ]; then + broken "Deep review ran in a broken sandbox ($BROKEN_COUNT bwrap Bash failures). The CLI pin is not taking effect or the regression changed shape. See https://github.com/anthropics/claude-code-action/issues/1547" + fi + echo "broken=false" >> "$GITHUB_OUTPUT" + echo "Sandbox smoke check passed -- no bwrap failures in the reviewer transcript." + # fromJSON() in `with:` has been observed to leave structured_output JSON # unparsed for the sibling claude-code-review workflow. Extract via jq. # @@ -427,12 +596,19 @@ jobs: # output a second time when the underlying skill returns a JSON-shaped # response. Detect that case (the inner string parses as an object with # a `review` key) and unwrap once more so we post markdown, not JSON. + # `always() && steps.review.outcome == 'success'` (rather than a bare + # `if:`, which is implicitly ANDed with success()) so that a failure in + # the sandbox health check above cannot discard an already-completed + # review: the review still posts, and the job is failed afterwards. - name: Extract review from structured output id: extract - if: steps.gate.outputs.should_review == 'true' + if: >- + always() && steps.gate.outputs.should_review == 'true' && + steps.review.outcome == 'success' env: STRUCTURED_OUTPUT: ${{ steps.review.outputs.structured_output }} DIFF_HASH: ${{ steps.gate.outputs.diff_hash }} + SANDBOX_BROKEN: ${{ steps.sandbox.outputs.broken }} run: | REVIEW="$(printf '%s' "$STRUCTURED_OUTPUT" | jq -r '.review')" if printf '%s' "$REVIEW" | jq -e 'type == "object" and has("review")' >/dev/null 2>&1; then @@ -440,7 +616,13 @@ jobs: fi # Prepend the hidden state marker consumed by the gate on the next # run. Keep this format in lockstep with the gate's parser (MARKER_RE). + # If the sandbox was unhealthy (or its check did not complete), omit + # the parseable marker so the gate fail-opens and the next run + # re-reviews instead of trusting a zero-coverage review. MARKER="" + if [ "${SANDBOX_BROKEN:-}" != "false" ]; then + MARKER="" + fi { echo 'review<> "$GITHUB_OUTPUT" - name: Post or update deep review - if: steps.gate.outputs.should_review == 'true' + if: >- + always() && steps.gate.outputs.should_review == 'true' && + steps.extract.outcome == 'success' uses: peter-evans/create-or-update-comment@v5 with: comment-id: ${{ steps.find-comment.outputs.comment-id }} issue-number: ${{ steps.pr.outputs.number }} body: ${{ steps.extract.outputs.review }} edit-mode: replace + + # Deferred failure for the sandbox health check: runs AFTER the review + # comment is posted so the evidence is preserved, then fails the job + # loud. Also treats a sandbox check that itself errored (e.g. jq choked + # on a reshaped transcript) as unhealthy rather than fail-open. + - name: Fail if reviewer sandbox was unhealthy + if: >- + always() && steps.gate.outputs.should_review == 'true' && + steps.review.outcome == 'success' + env: + SANDBOX_OUTCOME: ${{ steps.sandbox.outcome }} + SANDBOX_BROKEN: ${{ steps.sandbox.outputs.broken }} + SANDBOX_REASON: ${{ steps.sandbox.outputs.reason }} + run: | + set -euo pipefail + if [ "$SANDBOX_OUTCOME" != "success" ]; then + echo "::error::Sandbox health check did not complete (outcome: $SANDBOX_OUTCOME). Treating the reviewer sandbox as unhealthy." + exit 1 + fi + if [ "$SANDBOX_BROKEN" != "false" ]; then + echo "::error::${SANDBOX_REASON:-Reviewer sandbox unhealthy.}" + exit 1 + fi + echo "Reviewer sandbox healthy."