Scope PR push file policy to agent commits by Copilot · Pull Request #67241 · github/gh-aw

Copilot · 2026-10-09T17:13:22Z

push_to_pull_request_branch could reject an allowed agent change because its emit-time allowed-files check scanned earlier, author-owned commits on the PR branch.

Policy scope: Check commits from the existing PR head to the agent’s pinned branch tip, rather than from the PR’s base branch. Per-commit inspection still catches disallowed files that the agent changes and later reverts.
Coverage: Add regressions for a pre-existing disallowed file, a disallowed agent commit, and bundle file detection after an existing PR head. Existing apply-time protection checks remain in place.

Fixes push_to_pull_request_branch: allowed-files policy validates branch history, not the agent patch #67074

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-10-09T19:52:54Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-10-09T19:52:59Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-10-09T19:53:01Z

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Generated by Ponytail Reviewer for #67241

github-actions · 2026-10-09T19:53:29Z

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

No ADR enforcement needed for PR #67241: the PR does not carry the 'implementation' label and has 0 new lines in business logic directories (3 files changed, threshold 100).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-10-09T19:53:49Z

✅ PR Code Quality Reviewer completed the code quality review.

Testing whether safeoutputs write intents can be emitted without local file writes.

🔎 Code quality review by PR Code Quality Reviewer

Copilot

🟡 Changes recommended

Merged upstream commits can still be misclassified as agent-owned, and changed-then-reverted coverage is incomplete.

2 open findings

What changed in this PR

Scopes PR push file-policy checks to agent-added commits after the existing PR head.

Changes:

Uses the existing PR head as the allowlist baseline.
Adds regressions for author-owned and agent-owned files.
Verifies bundle file detection after an existing PR head.

File	Description
`actions/setup/js/safe_outputs_handlers.cjs`	Implements incremental file-policy validation.
`actions/setup/js/safe_outputs_handlers.test.cjs`	Tests incremental allowlist behavior.
`actions/setup/js/push_to_pull_request_branch.integration.test.cjs`	Tests bundle file scoping.

🧠 Review effort: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Copilot · 2026-10-09T21:09:06Z

        } else {
-          const branchHistoryFiles = execGitSync(["log", "--name-only", "--pretty=format:", `origin/${baseBranch}..${pushPinnedSha}`, "--"], { cwd: pushGitCwd })
+          const incrementalBase = prHeadBaseline?.sha || prHeadBaseline?.ref || `refs/remotes/origin/${entry.branch}`;
+          const agentFiles = execGitSync(["log", "--name-only", "--pretty=format:", `${incrementalBase}..${pushPinnedSha}`, "--"], { cwd: pushGitCwd })


Updated the commit walk with --ancestry-path so upstream-only commits merged from the base branch are excluded, and added a merge regression. Fixed in 79eaa70.

Copilot · 2026-10-09T21:09:06Z

+      it("rejects disallowed files introduced after the existing PR head", async () => {
+        const { repoDir } = createRepoWithDisallowedHistoryFile();
+        fs.writeFileSync(path.join(repoDir, "agent.js"), "console.log('agent');\n");
+        execSync("git add agent.js", { cwd: repoDir, stdio: "pipe" });
+        execSync("git commit -m 'agent script'", { cwd: repoDir, stdio: "pipe" });


The regression now adds agent.js and removes it in a second commit before invoking the handler, proving per-commit detection survives a net-zero change. Fixed in 79eaa70.

github-actions · 2026-10-09T20:00:52Z

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-09T19:55:16.333+00:00
review_event: REQUEST_CHANGES
top_themes:
  - wrong PR-head baseline fallback in incremental allowed_files check
  - missing regression coverage for recorded PR-head baseline path
files_reviewed:
  - actions/setup/js/safe_outputs_handlers.cjs
  - actions/setup/js/safe_outputs_handlers.test.cjs
  - actions/setup/js/push_to_pull_request_branch.integration.test.cjs
comment_count: 2

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 60.7 AIC · ⌖ 5.72 AIC · ⊞ 20K · ◷
_{Comment /review to run again}

github-actions

Blocking on one correctness bug and one coverage gap in the new incremental allowed_files check.

### Why I'm requesting changes

The new fallback to origin/<branch> is not a reliable stand-in for the current PR head, so this preflight can make the allowlist decision against the wrong commit range for fork PRs or same-name/stale tracking refs.
The added regressions only cover that fallback path; they never exercise the recorded PR-head baseline env vars that production uses for existing PRs, so the critical path remains unverified.

Please tighten the check to a recorded PR-head SHA/ref (or skip it when that baseline is unknown) and add a regression that proves the recorded-baseline path behaves correctly.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 60.7 AIC · ⌖ 5.72 AIC · ⊞ 20K
_{Comment /review to run again}

Comments that could not be inline-anchored

actions/setup/js/safe_outputs_handlers.cjs:1607

Falling back to refs/remotes/origin/(branch) here reintroduces the wrong-PR baseline bug for fork or same-name branches, so this preflight can block on author-owned commits or miss disallowed agent commits entirely.

<details><summary>💡 Why this blocks</summary>

When the recorded PR-head baseline is missing or rejected, origin/(branch) is not necessarily the current PR head. In fork PRs or wildcard/batch runs, it can point at another branch with the same name or at a stale tracking ref, which…

actions/setup/js/safe_outputs_handlers.test.cjs:2594

These regressions never populate the recorded PR-head baseline env vars, so they only exercise the origin/(branch) fallback and still miss the production path this patch is trying to fix.

<details><summary>💡 Why this matters</summary>

The behavioral change here is specifically about scoping to the existing PR head, but the new tests only seed refs/remotes/origin/feature/work and GITHUB_BASE_REF. They never set GH_AW_PR_HEAD_BASE_SHA/REF/PR_NUMBER, so they do not verify the recorded-baseline …

github-actions

Impeccable review (harden/audit focus — bug-fix change)

Scope: Scoped the allowed_files policy check for push_to_pull_request_branch from the full PR-branch history (origin/<base>..<branch>) to only the commits added after the existing PR head (prHeadBaseline or origin/<branch> fallback). Per-commit git log --name-only inspection is preserved so a disallowed file changed-then-reverted by the agent is still caught.

Findings:

No security or correctness issues found. The narrowed range correctly uses the pinned SHA, falls back safely when no PR-head baseline is recorded, and remains non-fatal (falls through to apply-time enforcement) if git fails to resolve the base ref.
Test coverage is solid: author-owned pre-existing disallowed files are no longer blocked, agent-introduced disallowed files are still rejected, excluded_files exemption still works, and bundle-transport pre-apply detection is covered for a PR with an existing head.
One minor non-blocking style note left inline: a duplicated fallback expression for computing the incremental base ref.

No blocking issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 73.5 AIC · ⌖ 17.2 AIC · ⊞ 8.1K

github-actions · 2026-10-09T20:02:32Z

+          server.debug("Incremental allowed-files check skipped: branch SHA not pinned (non-bundle path)");
        } else {
-          const branchHistoryFiles = execGitSync(["log", "--name-only", "--pretty=format:", `origin/${baseBranch}..${pushPinnedSha}`, "--"], { cwd: pushGitCwd })
+          const incrementalBase = prHeadBaseline?.sha || prHeadBaseline?.ref || `refs/remotes/origin/${entry.branch}`;


Minor: this fallback expression (prHeadBaseline?.sha || prHeadBaseline?.ref || refs/remotes/origin/${entry.branch}) is duplicated verbatim at line 1559 for the merge-detection check. Consider computing it once into a local (e.g. incrementalBaseRef) right after prHeadBaseline is resolved and reusing it at both call sites, so the two copies can't drift if the baseline-resolution semantics change later. Not blocking.

@copilot please address this.

Computed incrementalBaseRef once after resolving the PR-head baseline and reused it for merge detection and allowed-file inspection. Fixed in 79eaa70.

github-actions

Skills-Based Review 🧠

Applied /tdd and /codebase-design to the allowed_files scoping fix. This is a well-targeted security correctness fix — moving from a full-branch history check to a per-commit check anchored on the existing PR head, with a documented non-fatal fallback. No blocking issues found; two minor suggestions below.

📋 Key Themes & Highlights

Key Themes

Test coverage gap (/tdd): several of the new/updated tests only assert on the error shape conditionally (if (result.isError) {...}), so they can silently pass without exercising the behavior they're meant to verify. Tightening these would make regressions in the allowed-files bypass path fail loudly instead of quietly.
Minor duplication (/codebase-design): the prHeadBaseline?.sha || prHeadBaseline?.ref || refs/remotes/origin/${entry.branch} fallback expression is repeated identically in two places; a shared constant would prevent drift.

Positive Highlights

✅ Good security instinct: per-commit inspection (git log --name-only) instead of only net diff, so a disallowed file changed-then-reverted by the agent is still caught.
✅ Uses the already-pinned SHA (pushPinnedSha) as the range head to avoid TOCTOU, consistent with the surrounding bundle-generation code.
✅ New integration test (checks only bundle commits added after an existing PR head) exercises the bundle-based path realistically with real git repos/bundles, not just mocks.
✅ Non-fatal fallback preserved and well-commented when the PR head baseline isn't resolvable, so apply-time policy enforcement still backstops this check.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 120.1 AIC · ⌖ 14.5 AIC · ⊞ 10.1K
_{Comment /matt to run again}

Comments that could not be inline-anchored

actions/setup/js/safe_outputs_handlers.test.cjs:2606

[/tdd] Weak assertion: if (result.isError) { ... } only checks the error shape when an error happens, but never asserts that the call actually succeeds (or at least doesn't trip the allowed-files check). If a future regression makes pushToPullRequestBranchHandler always return a non-error result for an unrelated reason (e.g. git plumbing failure swallowed elsewhere), this test still passes without ever exercising the real assertion.

<details>
<summary>💡 Suggested fix</summary>

Asse…

actions/setup/js/safe_outputs_handlers.cjs:1559

[/codebase-design] prHeadBaseline?.sha || prHeadBaseline?.ref || \refs/remotes/origin/${entry.branch}`is duplicated verbatim at line 1607 for the allowed-files check. Both call sites need the exact same "effective incremental base" concept — extracting it into a small named helper/const once (e.g. right afterprHeadBaseline` is resolved) would prevent the two from drifting if one is edited later.

<details>
<summary>💡 Suggested fix</summary>

const effectiveIncrementalBase = …

</details>

gh-aw-bot · 2026-10-09T20:46:02Z

@copilot address the following outstanding work in one pass:

Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
Review (actions/setup/js/safe_outputs_handlers.cjs:1608): The revision range also walks commits brought in from another parent. If the agent merges an updated base branch, baseline..tip includes those upstream commits, so an upstream change to a disallowed file will reject an otherwise allowed agent contribution. Restrict the walk to commits descended from the recorded PR head (and add a merge regression) so merged base-branch history is not treated as agent-owned. - Scope PR push file policy to agent commits #67241 (comment)
Review (actions/setup/js/safe_outputs_handlers.test.cjs:2621): This case leaves agent.js present at the tip, so a future net-diff implementation would still pass the test even though the new per-commit guarantee had regressed. Revert the file in a second commit before invoking the handler to cover the stated changed-then-reverted behavior. - Scope PR push file policy to agent commits #67241 (comment)
Review (actions/setup/js/safe_outputs_handlers.cjs:1607): Minor: this fallback expression (prHeadBaseline?.sha || prHeadBaseline?.ref || refs/remotes/origin/${entry.branch}) is duplicated verbatim at line 1559 for the merge-detection check. Consider computing it once into a local (e.g. incrementalBaseRef) right after prHeadBaseline is resolved and reusing it at both call sites, so the two copies can't drift if the baseline-resolution semantics change later. Not blocking. - Scope PR push file policy to agent commits #67241 (comment)
Fix failing check JS Tests (shard 4/4) (FAILURE): https://github.com/github/gh-aw/actions/runs/37982987394/job/113998547017.

Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI.

Sous-chef head: b3ebeca
Sous-chef work: 31f7e812d45f0792093269b7b90445aa9803225a204ec130694b9ceb0227d7cc 57b0d0b89af2b0b118832d82f0c5fa36708aff9923f190490a21db77e44927b2 7e46e8d241177e3dcc72dd15883abf892e8226bf0cedf41175839cf16243ccfa cdc16e426cbc9ff3218fdabd9ffa6852a0c412b80fb7a346ac32f05af8ffa079
Sous-chef state: 26d3f53ace869a60fd91cfb24ab6aae7105948f42e621af686db47de885833f7

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · 11.9 AIC · ⌖ 7.33 AIC · ⊞ 1K · ◷
_{Comment /souschef to run again}

…iles-policy Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

done

gh-aw-bot · 2026-10-09T21:59:49Z

@copilot address the following outstanding work in one pass:

Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
Fix failing check JS Tests (shard 1/4) (FAILURE): https://github.com/github/gh-aw/actions/runs/37993889892/job/114035386696.

Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI.

Sous-chef head: dcbda10
Sous-chef work: 70ff6ecd06af00e89084076feec51e0a21386ffd2b0d8da88d896c1bf1a06518
Sous-chef state: 7d0d2a488c72ec405e979562ed955ea02a52077d79757e7677c054f32eca7499

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · 3.27 AIC · ⌖ 7.57 AIC · ⊞ 1K · ◷
_{Comment /souschef to run again}

…iles-policy # Conflicts: # actions/setup/js/work_queue_compaction.test.cjs Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Initial plan

779ae50

Copilot AI assigned Copilot and pelikhan Oct 9, 2026

Copilot started work on behalf of pelikhan October 9, 2026 17:13 View session

Copilot AI linked an issue Oct 9, 2026 that may be closed by this pull request

push_to_pull_request_branch: allowed-files policy validates branch history, not the agent patch #67074

Closed

Scope PR push allowed-files preflight to agent commits

b3ebeca

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix allowed-files policy validation for agent commits~~ Scope PR push file policy to agent commits Oct 9, 2026

Copilot AI requested a review from pelikhan October 9, 2026 17:26

Copilot finished work on behalf of pelikhan October 9, 2026 17:26

github-actions Bot mentioned this pull request Oct 9, 2026

[PR Triage Report] PR triage report - 2026-10-09 #67258

Closed

pelikhan marked this pull request as ready for review October 9, 2026 19:50

Copilot AI balanced review requested due to automatic review settings October 9, 2026 19:50

Copilot started reviewing on behalf of pelikhan October 9, 2026 19:51 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 9, 2026

[aw] No-Op Runs #65943

Open

github-actions Bot previously requested changes Oct 9, 2026

View reviewed changes

github-actions Bot reviewed Oct 9, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 9, 2026

[pr-sous-chef] PR Sous Chef 🍳 – Nudge Report #67171

Closed

Copilot started work on behalf of gh-aw-bot October 9, 2026 20:46 View session

Copilot AI and others added 2 commits October 9, 2026 20:52

Merge remote-tracking branch 'origin/main' into copilot/fix-allowed-f…

82dbe26

…iles-policy Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Scope PR history checks to agent ancestry

79eaa70

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot finished work on behalf of gh-aw-bot October 9, 2026 21:10

Copilot AI requested a review from gh-aw-bot October 9, 2026 21:10

Merge branch 'main' into copilot/fix-allowed-files-policy

dcbda10

Copilot started work on behalf of gh-aw-bot October 9, 2026 22:00 View session

Merge remote-tracking branch 'origin/main' into copilot/fix-allowed-f…

8d4785c

…iles-policy # Conflicts: # actions/setup/js/work_queue_compaction.test.cjs Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot finished work on behalf of gh-aw-bot October 9, 2026 22:22

pelikhan merged commit 647b1d1 into main Oct 9, 2026
3 checks passed

pelikhan deleted the copilot/fix-allowed-files-policy branch October 9, 2026 22:34

github-actions Bot mentioned this pull request Oct 10, 2026

Suggested test: copilot/push-to-pull-request-branch-allowed-files-scoped-to-agent-commits githubnext/gh-aw-test#21344

Open

Conversation

Copilot AI commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 9, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Comments that could not be inline-anchored

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Impeccable review (harden/audit focus — bug-fix change)

Uh oh!

github-actions Bot Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Comments that could not be inline-anchored

Uh oh!

gh-aw-bot commented Oct 9, 2026

Uh oh!

gh-aw-bot commented Oct 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading