Skip to content

Clarify CI submodule security: explain why .gitmodules edits bypass pull_request workflow isolation#2481

Closed
shino with Copilot wants to merge 1 commit into
diet-trivy-dispatchfrom
copilot/sub-pr-2476
Closed

Clarify CI submodule security: explain why .gitmodules edits bypass pull_request workflow isolation#2481
shino with Copilot wants to merge 1 commit into
diet-trivy-dispatchfrom
copilot/sub-pr-2476

Conversation

Copilot AI commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Addresses a review question about whether the git submodule attack vector is theoretically distinct from an attacker simply editing the workflow file directly.

Key distinction:

  • pull_request events run the workflow from the base branch — direct edits to .github/workflows/test.yml in a fork are ignored by GitHub's event isolation
  • actions/checkout with submodules: true reads .gitmodules from the checked-out merge commit (i.e., the fork's code) — a modified submodule URL bypasses the workflow file protection entirely and causes attacker-controlled code to be cloned and executed

The comment in analyze_golden_test.go and the decision to omit submodules: true from CI are intentional and correct; this PR adds a reply clarifying the mechanism that makes the two cases non-equivalent.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI changed the title [WIP] [WIP] Address feedback on removing fanal framework and Trivy parser usage Clarify CI submodule security: explain why .gitmodules edits bypass pull_request workflow isolation Mar 23, 2026
Copilot AI requested a review from shino March 23, 2026 05:09
@shino

shino commented Mar 23, 2026

Copy link
Copy Markdown
Collaborator

Thanks, :copilot: !!
My understanding got deeper.

@shino shino closed this Mar 23, 2026
@MaineK00n
MaineK00n deleted the copilot/sub-pr-2476 branch April 14, 2026 09:55
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.

2 participants