Skip to content

Tighten the pr review agent - #13921

Merged
JanKrivanek merged 4 commits into
dotnet:mainfrom
JanKrivanek:dev/jankrivanek/tighten-review-pr
Jun 2, 2026
Merged

Tighten the pr review agent#13921
JanKrivanek merged 4 commits into
dotnet:mainfrom
JanKrivanek:dev/jankrivanek/tighten-review-pr

Conversation

@JanKrivanek

Copy link
Copy Markdown
Member

Removes an unnecessary default checkout that can pose security concerns

Thanks to @ViktorHofer for pointing this out!

Copilot AI review requested due to automatic review settings June 2, 2026 13:16
@JanKrivanek
JanKrivanek requested a review from a team as a code owner June 2, 2026 13:16
@JanKrivanek
JanKrivanek enabled auto-merge June 2, 2026 13:17

Copilot AI left a comment

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.

Pull request overview

This PR updates the gh-aw agent source workflow for the “Expert Code Review (on open)” automation to disable the auto-injected repository checkout, aligning the workflow with the stated design of reading PR diffs via GitHub MCP tools and reducing exposure to untrusted PR content in a pull_request_target context.

Changes:

  • Adds checkout: false to prevent gh-aw from injecting a default checkout step.
  • Documents the rationale (MCP-based diff access; security motivation) in the workflow frontmatter comments.
Show a summary per file
File Description
.github/workflows/review-on-open.agent.md Disables auto-injected checkout via frontmatter flag and documents the reason.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment thread .github/workflows/review-on-open.agent.md Outdated
Comment thread .github/workflows/review-on-open.agent.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@github-actions github-actions Bot left a comment

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.

Expert Code Review — 24-Dimension Analysis

Summary

All 24 dimensions evaluated clean (LGTM). No issues found.

Wave Dimensions Evaluated Result
Key dimensions Security, Build Infrastructure, Backwards Compatibility, Correctness, Documentation, Design, Scope ✅ All LGTM
Remaining 17 ChangeWave, Performance, Allocation, Tests, Error Messages, Logging, String Comparison, API Surface, Target Authoring, Cross-Platform, Code Simplification, Concurrency, Naming, SDK Integration, Evaluation Model, Nullable Types, Dependency Management ✅ N/A (workflow config file)

Analysis

This is a clean, security-positive change:

  • Security ✅ — Disabling the auto-injected checkout on a pull_request_target workflow that handles fork PRs removes an unnecessary exposure vector. The agent doesn't need local code (reads diffs via MCP tools), so eliminating the checkout follows least-privilege principles.
  • Build Infrastructure ✅ — The explicit sparse checkout in steps: (for PAT selection from the base branch) is a separate mechanism and remains unaffected.
  • Correctness ✅ — The imports: directive is resolved by the gh-aw compiler at compile time, not via local file access, so checkout: false doesn't affect it.
  • Documentation ✅ — The inline comment clearly explains the "what" (checkout disabled), "why" (not needed + security), and "how" (agent uses MCP tools).
  • Design & Scope ✅ — Minimal, focused change that aligns with existing file comments (lines 11–13 already explain the agent reads diffs via MCP tools).

Note

🔒 Integrity filter blocked 1 item

The following item were blocked because they don't meet the GitHub integrity level.

  • #13921 pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Expert Code Review (on open) for issue #13921 · ● 2.6M

@JanKrivanek
JanKrivanek merged commit 9a2b356 into dotnet:main Jun 2, 2026
10 checks passed
@JanProvaznik

Copy link
Copy Markdown
Member

@JanKrivanek pls squash next time

@JanProvaznik

Copy link
Copy Markdown
Member

doesn't this make the reviewer "dumb" by not being able to agentically browse the repository and having to rely on diffs which mostly insufficient for review?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants