Add sdkReviewAgent agentic workflow for automated PR review

[xinlian12]

Imports the shared review pipeline from xinlian12/sdk-auto-pr-review. Triggers on PR open/push/ready_for_review. Posts inline review comments with severity tags and AI disclaimer.

[github-actions]

🤖 AI-Powered PR Review — sdkReviewAgent Workflow

⚠️ AI-generated review — may be incorrect. Agree? → resolve the conversation. Disagree? → reply with your reasoning.

---

🔴 Blocking · Security: JSON Injection in post-review-comment

File: .github/aw/imports/.../mcp-tools.md, lines 89–91

COMMENT="{\"path\":\"$INPUT_PATH\",\"body\":$BODY_JSON"
if [ -n "$INPUT_LINE" ]; then COMMENT="$COMMENT,\"line\":$INPUT_LINE"; fi
if [ -n "$INPUT_START_LINE" ]; then COMMENT="$COMMENT,\"start_line\":$INPUT_START_LINE"; fi

$INPUT_PATH is interpolated directly into a JSON string literal via shell string concatenation. If a file path contains " or \, the resulting JSON is malformed or can inject extra fields. $INPUT_LINE and $INPUT_START_LINE are also inserted raw — they are declared as type: number in the schema but there is no shell-side validation, so any string value could be injected.

Fix: Use jq to construct the JSON safely rather than string concatenation:

COMMENT=$(jq -n \
  --arg path "$INPUT_PATH" \
  --argjson body "$BODY_JSON" \
  --argjson line "${INPUT_LINE:-null}" \
  --argjson start_line "${INPUT_START_LINE:-null}" \
  '{path: $path, body: $body} | if $line != null then . + {line: $line} else . end | if $start_line != null then . + {start_line: $start_line} else . end')
echo "{\"event\":\"COMMENT\",\"body\":\"\",\"comments\":[$COMMENT]}" | \
  gh api repos/$GITHUB_REPOSITORY/pulls/$INPUT_PR_NUMBER/reviews --input -

---

🔴 Blocking · Security: Unquoted $ARGS in git-log Enables Argument Injection

File: .github/aw/imports/.../mcp-tools.md, lines 54 and 56

if [ -n "$INPUT_GREP" ]; then ARGS="$ARGS --grep=$INPUT_GREP"; fi
git log $ARGS -- "$INPUT_PATH"

$ARGS is passed to git log unquoted, causing word splitting. More critically, if $INPUT_GREP contains spaces it is split into multiple tokens, breaking the --grep= argument. An adversarial value like foo --exec-path=/tmp/evil would be split into two separate git arguments. Since $INPUT_PATH is quoted but $ARGS is not, the -- separator does not protect against option injection in the unquoted portion.

Fix: Use a bash array to accumulate arguments:

ARGS=(--format="%H|%ad|%an|%s" --date=short "--since=${INPUT_SINCE_DAYS:-90}.days.ago" "-n" "${INPUT_MAX_COUNT:-30}")
if [ -n "$INPUT_GREP" ]; then ARGS+=("--grep=$INPUT_GREP"); fi
if [ "$INPUT_ALL_BRANCHES" = "true" ]; then ARGS+=(--all); fi
git log "${ARGS[@]}" -- "$INPUT_PATH"

---

🟡 Recommendation · Correctness: || echo "(none)" in get-pr-comments Is Dead Code

File: .github/aw/imports/.../mcp-tools.md, lines 36 and 40

gh api ... --paginate --jq '...' 2>/dev/null || echo "(none)"

gh api --paginate exits with code 0 and prints nothing when the response array is empty. It only exits non-zero on HTTP errors (e.g., 404, 401). The 2>/dev/null suppresses the error message, and then || echo "(none)" is executed — but gh api still exits 0 on empty lists, so the fallback never prints (none) when there are simply no comments. The agent will see empty output and may not know whether to interpret it as "no comments" or "tool failure."

Fix:

RESULT=$(gh api ... --paginate --jq '...' 2>/dev/null || true)
echo "${RESULT:-(none)}"

---

🟡 Recommendation · Reliability: No Diff Size Limit in get-pr-diff

File: .github/aw/imports/.../mcp-tools.md, line 16

git diff -U10 "$MERGE_BASE" "$HEAD"

For PRs with large generated files (e.g., lock files, auto-generated code), this command can produce hundreds of megabytes of output, overwhelming the LLM context window and causing the agent to either truncate critical context or fail. pr-review.lock.yml itself is 1,377 lines — a PR touching generated files would be even larger.

Fix: Pipe through a byte-limit and add a truncation note:

OUTPUT=$(git diff -U10 "$MERGE_BASE" "$HEAD" | head -c 400000)
echo "$OUTPUT"
TOTAL=$(git diff -U10 "$MERGE_BASE" "$HEAD" | wc -c)
if [ "$TOTAL" -gt 400000 ]; then
  echo "--- [DIFF TRUNCATED: ${TOTAL} bytes total, showing first 400000] ---"
fi

---

🟡 Recommendation · Architecture: Import Pinned to @main — Floating Upstream Dependency

File: .github/workflows/pr-review.md, line 15

imports:
  - xinlian12/sdk-auto-pr-review/.github/workflows/shared/pr-review-pipeline.md@main

The source .md file imports the shared pipeline at @main, a floating reference. While the compiled pr-review.lock.yml currently caches the resolved content at commit 3aa0798d9f8e03355b961f0c63fa0e9c25c4172f, any re-run of gh aw compile will resolve @main to whatever HEAD is at that moment, silently upgrading the prompt and tool definitions. If the upstream xinlian12/sdk-auto-pr-review repo changes its review criteria, adds new agents, or is compromised, a simple recompile would silently propagate those changes into this repo's workflow.

Fix: Pin to a specific commit SHA in the source:

imports:
  - xinlian12/sdk-auto-pr-review/.github/workflows/shared/pr-review-pipeline.md@3aa0798d9f8e03355b961f0c63fa0e9c25c4172f

---

🟡 Recommendation · Reliability: Silent git fetch Failure Masks Wrong Diff Base

File: .github/aw/imports/.../mcp-tools.md, line 14

git fetch origin "$BASE" "$HEAD" 2>/dev/null || true

Both stderr and the non-zero exit are discarded. If $HEAD cannot be fetched (e.g., the ref has since been force-pushed and the lock file has a stale SHA), git merge-base on the next line will either fail silently or use a wrong merge base, producing a diff of incorrect scope. The agent will review the wrong code without knowing it.

Fix: Log fetch failures to stdout so the agent is aware:

git fetch origin "$BASE" "$HEAD" 2>&1 || echo "WARNING: git fetch failed — diff may be incomplete"

---

🟢 Suggestion · Code Quality: Draft PR Trigger Creates Noisy Reviews

File: .github/workflows/pr-review.md, line 6

on:
  pull_request:
    types: [opened, synchronize, ready_for_review]

The opened event fires for draft PRs as well as ready PRs. This means the agent posts a review on every draft PR that is opened, before the author even considers it ready. The ready_for_review type already handles the transition from draft to ready, so the initial opened event for a draft PR generates an unwanted review.

Fix: Add a guard condition in the workflow or filter out draft PRs:

on:
  pull_request:
    types: [opened, synchronize, ready_for_review]
jobs:
  # add to activation condition:
  # && github.event.pull_request.draft == false
```

---

### :green_circle: **Suggestion** · Code Quality: Windows Local Temp Paths Leaked in Lock File Metadata

**File:** `.github/workflows/pr-review.lock.yml`, lines 30–35

```
#     - C:/Users/xinlian/AppData/Local/Temp/gh-aw-include-1375472578.md
#     - C:/Users/xinlian/AppData/Local/Temp/gh-aw-include-1996015015.md

The compiled lock file embeds the developer's Windows username and local temp paths in the auto-generated comment header. This is a minor privacy concern and also suggests the lock file was compiled on a personal development machine. While harmless now, it creates a maintenance issue (lock file always looks like it was "dirtied" by a local build, making authorship ambiguous in git history).

Note: This appears to be auto-generated by gh aw compile — you may want to file an issue with gh-aw to sanitize temp paths from the lock file output.

---

💬 Observation · Architecture: Good Use of Fork Protection Pattern

File: .github/workflows/pr-review.lock.yml, line 59

if: >
  (needs.pre_activation.outputs.activated == 'true') &&
  ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id))

The dual-layer fork protection (same check on both pre_activation and activation) correctly prevents the workflow from running on PRs from forked repositories. This is the right pattern for a workflow that uses COPILOT_GITHUB_TOKEN and posts automated comments — running this on external forks would expose secrets and allow attackers to trigger AI-generated comments from any PR. Well done.

---

💬 Observation · Architecture: Action SHAs Are Pinned

All uses: references in the lock file are pinned to full commit SHAs (e.g., actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd), which is the correct security hardening practice for GitHub Actions workflows. This prevents supply-chain attacks via tag mutation.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• releaseassets.githubusercontent.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "releaseassets.githubusercontent.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 4 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:Add sdkReviewAgent agentic workflow for automated PR review #22 (pull_request_read: Resource 'pr:Add sdkReviewAgent agentic workflow for automated PR review #22' has lower integrity than agent requires. Agent would need to drop integrity tags [unapproved:all approved:all] to trust this resource.)
• #3 (list_pull_requests: Resource 'pr:Removing Class.forName for implementation bridge helpers #3' has lower integrity than agent requires. Agent would need to drop integrity tags [approved:all] to trust this resource.)
• #1 (list_pull_requests: Resource 'pr:Throughput control optimizations #1' has lower integrity than agent requires. Agent would need to drop integrity tags [approved:all] to trust this resource.)
• #12 (list_pull_requests: Resource 'pr:Annie/release4.64.0 #12' has lower integrity than agent requires. Agent would need to drop integrity tags [approved:all] to trust this resource.)

Generated by sdkReviewAgent for issue #22 · ◷