-
Notifications
You must be signed in to change notification settings - Fork 322
ai-review: handle PRs over the 300-file diff cap in prefetch #2845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| set -euo pipefail | ||
|
|
||
| : "${PR_NUMBER:?PR_NUMBER required}" | ||
| : "${REPO:?REPO required (e.g. opentensor/subtensor)}" | ||
| : "${REPO:?REPO required (e.g. RaoFoundation/subtensor)}" | ||
| : "${GH_TOKEN:?GH_TOKEN required (used here only — NOT passed to Codex)}" | ||
| OUTPUT_DIR="${OUTPUT_DIR:-/tmp/ai-review-context}" | ||
|
|
||
|
|
@@ -79,8 +79,47 @@ jq -r '.body // ""' "$OUTPUT_DIR/pr.json" > "$OUTPUT_DIR/pr-body.md" | |
| # Files changed (paths + per-file additions/deletions; full content lives in the diff) | ||
| gh_retry gh pr view "$PR_NUMBER" --repo "$REPO" --json files > "$OUTPUT_DIR/pr-files.json" | ||
|
|
||
| # Full unified diff | ||
| gh_retry gh pr diff "$PR_NUMBER" --repo "$REPO" > "$OUTPUT_DIR/pr-diff.patch" | ||
| # Full unified diff. | ||
| # | ||
| # `gh pr diff` hits the `.diff` media type, which GitHub hard-caps at 300 | ||
| # changed files: on a larger PR it returns HTTP 406 "diff exceeded the maximum | ||
| # number of files" and no amount of retrying clears it. Since the Skeptic is a | ||
| # required check, that would leave the gate permanently red on big PRs. So try | ||
| # `gh pr diff` first (canonical output, correct for the common case) and fall | ||
| # back to computing the same diff from the local PR checkout, which has no | ||
| # file-count cap. | ||
| reconstruct_diff_locally() { | ||
| # The Files API alternative is unsuitable: it omits `patch` for binary AND | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH] Protected AI-review security helper modified This changes the trusted helper that determines exactly what code the security personas review. Policy requires every modification under |
||
| # individually-oversized textual files, and a placeholder there would be a | ||
| # review blind spot (an attacker could oversize exactly the file they want | ||
| # unreviewed). Instead, produce the complete diff with local git: the | ||
| # workflow runs this script inside a fetch-depth-0 checkout of the PR head, | ||
| # so the merge base is already present and no network or credentials are | ||
| # needed (the checkout uses persist-credentials: false). Any failure aborts | ||
| # the script (set -e), so this fails closed — the personas never see a | ||
| # partial diff. | ||
| local head_sha base_ref merge_base | ||
| head_sha=$(jq -r '.headRefOid' "$OUTPUT_DIR/pr.json") | ||
| base_ref=$(jq -r '.baseRefName' "$OUTPUT_DIR/pr.json") | ||
|
|
||
| git rev-parse --is-inside-work-tree >/dev/null \ | ||
| || { echo "::error::not inside the PR checkout; cannot reconstruct diff" >&2; return 1; } | ||
| [ "$(git rev-parse HEAD)" = "$head_sha" ] \ | ||
| || { echo "::error::checkout HEAD does not match PR head $head_sha; cannot reconstruct diff" >&2; return 1; } | ||
|
|
||
| merge_base=$(git merge-base "origin/$base_ref" "$head_sha") \ | ||
| || { echo "::error::could not resolve merge base of origin/$base_ref and $head_sha" >&2; return 1; } | ||
| git diff "$merge_base" "$head_sha" | ||
| } | ||
|
|
||
| if _gh_retry_inner gh pr diff "$PR_NUMBER" --repo "$REPO" > "$OUTPUT_DIR/pr-diff.patch"; then | ||
| : | ||
| else | ||
| echo "::warning::gh pr diff failed (likely >300 files / HTTP 406); reconstructing from the local checkout" >&2 | ||
| cat /tmp/gh_retry.err >&2 2>/dev/null || true | ||
| rm -f /tmp/gh_retry.err | ||
| reconstruct_diff_locally > "$OUTPUT_DIR/pr-diff.patch" | ||
| fi | ||
|
|
||
| # All PR comments (issue-style). `--paginate` alone writes one JSON array per | ||
| # page; `--slurp` wraps them as [[page1], [page2], ...]; we then flatten with | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,51 +1,82 @@ | ||
| name: cargo audit | ||
| name: Cargo Audit | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: | ||
| - labeled | ||
| - unlabeled | ||
| - synchronize | ||
| - opened | ||
| types: [labeled, unlabeled, synchronize, opened] | ||
|
|
||
| concurrency: | ||
| group: cargo-audit-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| trusted-pr: | ||
| name: Trusted PR source (non-fork) | ||
| if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - run: echo "Non-fork PR; self-hosted runners may execute checkout." | ||
|
|
||
| cargo-audit: | ||
| name: cargo audit | ||
| needs: trusted-pr | ||
| runs-on: [self-hosted, fireactions-light] | ||
| if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit') }} | ||
| steps: | ||
| - name: Check-out repositoroy under $GITHUB_WORKSPACE | ||
| uses: actions/checkout@v4 | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| sudo DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a apt-get update | ||
| sudo DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a apt-get install -y --no-install-recommends -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" build-essential clang curl libssl-dev llvm libudev-dev protobuf-compiler pkg-config | ||
|
|
||
| - name: Install Rust | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: stable | ||
|
|
||
| - name: Utilize Shared Rust Cache | ||
| uses: Swatinem/rust-cache@v2 | ||
| - uses: ./.github/actions/rust-setup | ||
| with: | ||
| key: cargo-audit | ||
| cache-on-failure: true | ||
| cache-key: cargo-audit | ||
|
|
||
| - name: Install cargo-audit | ||
| run: cargo install --force cargo-audit | ||
|
|
||
| - name: Display cargo-audit --version | ||
| run: cargo audit --version | ||
|
|
||
| - name: cargo audit | ||
| # Each ignore is a known, accepted advisory; revisit when bumping | ||
| # polkadot-sdk since most originate in the patched fork's tree. | ||
| run: | | ||
| # RUSTSEC-2026-0020/0021/0085..0096: wasmtime 8.0.1, pinned by the | ||
| # polkadot-sdk fork's sc-executor; fixed only by a major SDK bump. | ||
| # RUSTSEC-2026-0098/0099/0104: rustls-webpki 0.101 via the fork's | ||
| # libp2p networking stack. | ||
| # RUSTSEC-2026-0118/0119: hickory-proto via libp2p-dns (fork tree). | ||
| # RUSTSEC-2026-0204: crossbeam-epoch via the fork tree. | ||
| # RUSTSEC-2025-0137: ruint via the frontier/EVM fork tree. | ||
| # RUSTSEC-2025-0020 / RUSTSEC-2026-0177: pyo3 0.23 in py-sp-core; | ||
| # ours — revisit when pyo3 >=0.29 is adopted. | ||
| # RUSTSEC-2026-0007/0009/0037/0049/0185: bytes/time/quinn-proto/ | ||
| # rustls-webpki duplicates only present in main's pre-monorepo | ||
| # Cargo.lock; drop these once the consolidated lockfile lands. | ||
| cargo audit --ignore RUSTSEC-2023-0091 \ | ||
| --ignore RUSTSEC-2024-0438 \ | ||
| --ignore RUSTSEC-2025-0009 \ | ||
| --ignore RUSTSEC-2025-0055 \ | ||
| --ignore RUSTSEC-2025-0073 \ | ||
| --ignore RUSTSEC-2025-0118 | ||
| --ignore RUSTSEC-2025-0118 \ | ||
| --ignore RUSTSEC-2026-0020 \ | ||
| --ignore RUSTSEC-2026-0021 \ | ||
| --ignore RUSTSEC-2026-0085 \ | ||
| --ignore RUSTSEC-2026-0086 \ | ||
| --ignore RUSTSEC-2026-0087 \ | ||
| --ignore RUSTSEC-2026-0088 \ | ||
| --ignore RUSTSEC-2026-0089 \ | ||
| --ignore RUSTSEC-2026-0091 \ | ||
| --ignore RUSTSEC-2026-0092 \ | ||
| --ignore RUSTSEC-2026-0093 \ | ||
| --ignore RUSTSEC-2026-0094 \ | ||
| --ignore RUSTSEC-2026-0095 \ | ||
| --ignore RUSTSEC-2026-0096 \ | ||
| --ignore RUSTSEC-2026-0098 \ | ||
| --ignore RUSTSEC-2026-0099 \ | ||
| --ignore RUSTSEC-2026-0104 \ | ||
| --ignore RUSTSEC-2026-0118 \ | ||
| --ignore RUSTSEC-2026-0119 \ | ||
| --ignore RUSTSEC-2026-0204 \ | ||
| --ignore RUSTSEC-2025-0137 \ | ||
| --ignore RUSTSEC-2025-0020 \ | ||
| --ignore RUSTSEC-2026-0177 \ | ||
| --ignore RUSTSEC-2026-0007 \ | ||
| --ignore RUSTSEC-2026-0009 \ | ||
| --ignore RUSTSEC-2026-0037 \ | ||
| --ignore RUSTSEC-2026-0049 \ | ||
| --ignore RUSTSEC-2026-0185 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[HIGH] Protected AI-review security helper modified
This changes a protected
.github/ai-review/*helper that determines the exact code shown to the security personas. Once merged, this base-branch copy becomes trusted and executes on future PRs; any defect could create a steady-state review bypass. Per the protected-file policy, nucleus must independently validate the complete fallback and fail-closed behavior before merge.