Skip to content

AI reviewer tweaks 1#2688

Open
sam0x17 wants to merge 4 commits into
devnet-readyfrom
ai-reviewer-tweaks
Open

AI reviewer tweaks 1#2688
sam0x17 wants to merge 4 commits into
devnet-readyfrom
ai-reviewer-tweaks

Conversation

@sam0x17
Copy link
Copy Markdown
Contributor

@sam0x17 sam0x17 commented May 21, 2026

No description provided.

@sam0x17 sam0x17 added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label May 21, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread .github/ai-review/prefetch.sh Outdated
Comment on lines +86 to +95
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"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

🛡️ 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 AI-review infrastructure changed.

No runtime, pallet, dependency, or supply-chain changes are present. The remaining blocker is the explicit protected-review-infra rule for changes under .github/ai-review/*.

Findings

Sev File Finding
HIGH .github/ai-review/README.md:84 Protected AI-review documentation modified inline
HIGH .github/ai-review/prefetch.sh:100 Protected AI-review prefetch helper modified inline

Prior-comment reconciliation

  • 6396b537: not addressed.github/ai-review/prefetch.sh is still modified in the current diff, so the protected-review-infra finding remains active.

Conclusion

Blocked under the protected-review-infra rule: this PR modifies AI-review files, including the token-bearing prefetch helper. I found no evidence of malicious intent beyond that required HIGH-severity gate.


📜 Previous run (superseded)
Sev File Finding Status
HIGH .github/ai-review/prefetch.sh:107 Protected AI-review prefetch helper modified ➡️ Carried forward to current findings
.github/ai-review/prefetch.sh is still modified in the current diff, so the protected-review-infra finding remains active.

# 🔍 AI Review — Auditor (domain review) has not yet run on this PR.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +82 to +107
# 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +96 to +100
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}" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Protected AI-review prefetch helper modified

This PR changes .github/ai-review/prefetch.sh, the helper that prepares the no-network context consumed by the reviewers and runs before Codex with GitHub credentials available. The run instructions require diffs under .github/ai-review/* to be flagged HIGH or CRITICAL against the trusted base copy. The prior diff-suppression concern is addressed by --no-ext-diff --no-textconv --text, but the protected helper modification itself remains a blocking review-infra change.

**To AI-review a fork PR:** a nucleus member dispatches the workflow with
the PR number. `workflow_dispatch` runs in base context with secrets
available, performs the real review, and the required checks turn green.
Fork PRs are AI-reviewed automatically via the `pull_request_target` trigger.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Protected AI-review documentation modified

This PR also changes .github/ai-review/README.md. Even though this is documentation, it is still under .github/ai-review/*, and the run instructions require any diff in that protected review-infra tree to be flagged HIGH or CRITICAL against the trusted base copy.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant