AI reviewer tweaks 1#2688
Conversation
sam0x17
added
the
skip-cargo-audit
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| BASE_REF_FOR_DIFF=$(jq -r '.baseRefName' "$OUTPUT_DIR/pr.json") | ||
| HEAD_SHA_FOR_DIFF=$(jq -r '.headRefOid' "$OUTPUT_DIR/pr.json") | ||
| if git rev-parse --verify --quiet "origin/${BASE_REF_FOR_DIFF}" >/dev/null; then | ||
| git diff "origin/${BASE_REF_FOR_DIFF}...${HEAD_SHA_FOR_DIFF}" \ | ||
| > "$OUTPUT_DIR/pr-diff.patch" | ||
| else | ||
| # Base ref not in local refs (e.g. running outside actions/checkout fetch-depth:0). | ||
| # Fall back to the REST endpoint; bail loudly if it 406s on a huge PR. | ||
| echo "::warning::origin/${BASE_REF_FOR_DIFF} not local; falling back to gh pr diff (may fail for >20k-line PRs)" | ||
| gh_retry gh pr diff "$PR_NUMBER" --repo "$REPO" > "$OUTPUT_DIR/pr-diff.patch" |
There was a problem hiding this comment.
[HIGH] Local git diff can be blinded by PR-controlled attributes
This protected helper now generates the personas' source-of-truth diff from the PR checkout. A future hostile PR can add .gitattributes entries that mark sensitive paths as binary or otherwise suppress textual hunks, causing local git diff to emit incomplete content into pr-diff.patch; pr-files.json only has paths/counts, so both personas can miss the hidden code. Harden the local diff by forcing text output, disabling textconv/external diff behavior, and pinning the comparison to the exact baseRefOid from pr.json instead of the mutable origin/<base> ref.
| BASE_REF_FOR_DIFF=$(jq -r '.baseRefName' "$OUTPUT_DIR/pr.json") | |
| HEAD_SHA_FOR_DIFF=$(jq -r '.headRefOid' "$OUTPUT_DIR/pr.json") | |
| if git rev-parse --verify --quiet "origin/${BASE_REF_FOR_DIFF}" >/dev/null; then | |
| git diff "origin/${BASE_REF_FOR_DIFF}...${HEAD_SHA_FOR_DIFF}" \ | |
| > "$OUTPUT_DIR/pr-diff.patch" | |
| else | |
| # Base ref not in local refs (e.g. running outside actions/checkout fetch-depth:0). | |
| # Fall back to the REST endpoint; bail loudly if it 406s on a huge PR. | |
| echo "::warning::origin/${BASE_REF_FOR_DIFF} not local; falling back to gh pr diff (may fail for >20k-line PRs)" | |
| gh_retry gh pr diff "$PR_NUMBER" --repo "$REPO" > "$OUTPUT_DIR/pr-diff.patch" | |
| BASE_REF_FOR_DIFF=$(jq -r '.baseRefName' "$OUTPUT_DIR/pr.json") | |
| BASE_SHA_FOR_DIFF=$(jq -r '.baseRefOid' "$OUTPUT_DIR/pr.json") | |
| HEAD_SHA_FOR_DIFF=$(jq -r '.headRefOid' "$OUTPUT_DIR/pr.json") | |
| SAFE_DIFF_OPTS=(--no-ext-diff --no-textconv --text) | |
| if git cat-file -e "${BASE_SHA_FOR_DIFF}^{commit}" 2>/dev/null; then | |
| git diff "${SAFE_DIFF_OPTS[@]}" "${BASE_SHA_FOR_DIFF}...${HEAD_SHA_FOR_DIFF}" \ | |
| > "$OUTPUT_DIR/pr-diff.patch" | |
| else | |
| # Base commit not local (e.g. branch advanced after checkout). | |
| # Fall back to the REST endpoint; bail loudly if it 406s on a huge PR. | |
| echo "::warning::base commit ${BASE_SHA_FOR_DIFF} not local; falling back to gh pr diff (may fail for >20k-line PRs)" | |
| gh_retry gh pr diff "$PR_NUMBER" --repo "$REPO" > "$OUTPUT_DIR/pr-diff.patch" | |
| fi |
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE BASELINE scrutiny: author is a repo admin with substantial prior subtensor history; branch ai-reviewer-tweaks -> devnet-ready; protected .github/ai-review helper modified. The previous local-diff suppression issue is addressed by forcing text diffs and disabling textconv/external diff drivers. However, this PR still modifies protected AI-review infrastructure, and the run instructions require Findings
Prior-comment reconciliation
ConclusionBlocked under the protected-review-infra rule: this PR changes the helper that prepares the context consumed by the no-network reviewers. I found no evidence of malicious intent. 📜 Previous run (superseded)
# 🔍 AI Review — Auditor (domain review) has not yet run on this PR. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
| # Full unified diff. Use local `git diff` rather than `gh pr diff` because | ||
| # the GitHub REST diff endpoint hard-caps at 20,000 lines (HTTP 406 | ||
| # `PullRequest.diff too_large`). The workflow already checked out the PR | ||
| # head with `fetch-depth: 0`, so all branches are available locally. | ||
| # | ||
| # Hardening against PR-controlled diff suppression: | ||
| # --text force textual diff so a `.gitattributes` `binary` mark | ||
| # cannot hide hunks in sensitive paths. | ||
| # --no-textconv ignore textconv filters (which can mangle/suppress output | ||
| # and execute external programs). | ||
| # --no-ext-diff ignore external diff drivers configured via `.gitattributes`. | ||
| # We also pin the comparison to the immutable `baseRefOid` SHA from pr.json | ||
| # rather than the moving `origin/<base>` tip — so an advance of the base | ||
| # branch between fetch and diff cannot change what gets reviewed. | ||
| BASE_SHA_FOR_DIFF=$(jq -r '.baseRefOid' "$OUTPUT_DIR/pr.json") | ||
| HEAD_SHA_FOR_DIFF=$(jq -r '.headRefOid' "$OUTPUT_DIR/pr.json") | ||
| SAFE_DIFF_OPTS=(--no-ext-diff --no-textconv --text) | ||
| if git cat-file -e "${BASE_SHA_FOR_DIFF}^{commit}" 2>/dev/null; then | ||
| git diff "${SAFE_DIFF_OPTS[@]}" "${BASE_SHA_FOR_DIFF}...${HEAD_SHA_FOR_DIFF}" \ | ||
| > "$OUTPUT_DIR/pr-diff.patch" | ||
| else | ||
| # Base commit not local (e.g. shallow checkout missing the merge base). | ||
| # Fall back to the REST endpoint; bail loudly if it 406s on a huge PR. | ||
| echo "::warning::base commit ${BASE_SHA_FOR_DIFF} not local; falling back to gh pr diff (may fail for >20k-line PRs)" | ||
| gh_retry gh pr diff "$PR_NUMBER" --repo "$REPO" > "$OUTPUT_DIR/pr-diff.patch" | ||
| fi |
There was a problem hiding this comment.
[HIGH] Protected AI-review prefetch helper modified
This PR modifies .github/ai-review/prefetch.sh, a protected review-infrastructure helper. The operating instructions for this run require any diff against trusted .github/ai-review/* files to be flagged as HIGH or CRITICAL. Because this script controls the context handed to the no-network review personas, this change should be treated as blocking until explicitly validated by a trusted human/nucleus reviewer.
|
🔄 AI review updated — Skeptic: VULNERABLE |
1 participant
No description provided.