Skip to content

Comments

🔒 Add fork PR detection, CI trigger restrictions, and integration tests#18160

Closed
dsyme wants to merge 13 commits intomainfrom
ptb
Closed

🔒 Add fork PR detection, CI trigger restrictions, and integration tests#18160
dsyme wants to merge 13 commits intomainfrom
ptb

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Feb 24, 2026

Summary

  • Fork PR guard: Exports GH_AW_IS_FORK_PR env var during checkout and blocks push_to_pull_request_branch early—at both the MCP handler and git-push layers—when a fork PR is detected, preventing permission errors and clarifying the limitation in docs.
  • CI trigger token hardening: The extra empty commit (used to trigger CI events) is now restricted to branches with exactly 1 new commit, preventing the CI trigger token from being used on multi-commit branches where workflow files may have been iteratively modified.
  • New tests: Adds a full unit test suite for push_to_pull_request_branch.cjs and a real-git integration test suite (git_patch_integration.test.cjs) covering patch generation/application, unicode/emoji handling, merge conflicts, concurrent pushes, and commit title suffix modification.

Copilot AI review requested due to automatic review settings February 24, 2026 14:01
Copy link
Contributor

Copilot AI left a comment

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 enhances the security and reliability of the GitHub Agentic Workflows system by adding fork PR detection, CI trigger token hardening, and comprehensive test coverage.

Changes:

  • Adds fork PR detection that blocks push_to_pull_request_branch early at both MCP handler and git-push layers, exporting GH_AW_IS_FORK_PR env var during checkout
  • Hardens CI trigger token by restricting extra empty commits to branches with exactly 1 new commit, preventing misuse on multi-commit branches where workflow files may have been modified
  • Adds comprehensive unit tests for push_to_pull_request_branch.cjs and real-git integration tests for patch generation/application

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/src/content/docs/reference/safe-outputs.md Documents fork PR limitation with a clear caution block
actions/setup/js/checkout_pr_branch.cjs Exports GH_AW_IS_FORK_PR environment variable during PR checkout
actions/setup/js/checkout_pr_branch.test.cjs Tests for fork PR detection and environment variable export
actions/setup/js/safe_outputs_handlers.cjs Early fork PR rejection at MCP handler level before patch generation
actions/setup/js/safe_outputs_handlers.test.cjs Tests for fork PR early rejection in MCP handler
actions/setup/js/push_to_pull_request_branch.cjs Fork PR detection at git-push layer and newCommitCount tracking for CI trigger restriction
actions/setup/js/push_to_pull_request_branch.test.cjs Comprehensive unit test suite (1072 lines) covering all scenarios
actions/setup/js/git_patch_integration.test.cjs Real-git integration tests for patch generation, unicode/emoji handling, conflicts, and concurrent pushes
actions/setup/js/extra_empty_commit.cjs Adds newCommitCount parameter to restrict empty commit to exactly 1 new commit
actions/setup/js/extra_empty_commit.test.cjs Tests for newCommitCount restriction including backward compatibility
actions/setup/js/create_pull_request.cjs Tracks newCommitCount for both patch and empty-branch paths

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dsyme dsyme closed this Feb 24, 2026
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.

1 participant