Skip to content

security: document intentional absence of PR head ref in pull_request_target checkouts - #855

Open
kmarekspartz wants to merge 2 commits into
inkeep:mainfrom
kmarekspartz:copilot/should-workflows-run-on-forks
Open

security: document intentional absence of PR head ref in pull_request_target checkouts#855
kmarekspartz wants to merge 2 commits into
inkeep:mainfrom
kmarekspartz:copilot/should-workflows-run-on-forks

Conversation

@kmarekspartz

Copy link
Copy Markdown
Contributor

What & why

pull_request_target runs in the base-branch context with full access to repository secrets. Adding ref: ${{ github.event.pull_request.head.sha }} to either checkout in the sync or close jobs would allow a PR author's scripts to execute with OSS_SYNC_APP credentials — enabling reads/writes to `inkeep/agents-private

How this was verified

Good news:

The current code appears safe: the missing ref: is intentional, and a contributor of a PR adding a ref: could not self-exploit. This PR makes that intent explicit with a comment so future reviewers recognize a PR adding that line as a security regression rather than a neutral cleanup.

pull_request_target always runs the base branch's version of the workflow file, never the PR head's. So if an attacker submits a PR that adds ref: ${{ github.event.pull_request.head.sha }} to the checkout in monorepo-pr-bridge.yml, that PR runs the current (safe) workflow — the ref: change has no effect until someone merges it.

Threat model

A potential attack chain would require two steps:

  1. Social engineering merge: The attacker submits a PR adding ref: ${{ github.event.pull_request.head.sha }} to the sync job's checkout, framed innocuously — e.g., "check out PR code to improve diff logging." A reviewer who doesn't deeply understand the pull_request_target security model sees a one-line workflow tweak and merges it.

  2. Exploit via a second PR: Now the base branch has the vulnerable workflow. The attacker (or anyone) opens a new PR. When a maintainer approves the inkeep-oss-sync environment — which they'd routinely do for legitimate-looking PRs — the workflow now checks out the PR head's code AND has secrets. The attacker's malicious version of .github/scripts/bridge-public-pr-to-monorepo.mjs runs with full access to OSS_SYNC_APP_ID and OSS_SYNC_APP_PRIVATE_KEY.

What could those credentials do?

The App token is minted with access to inkeep/agents-private. Based on the workflow, it has at minimum contents: write on that repo. An attacker could:

  • Read the entire inkeep/agents-private codebase (described in the workflow comments as containing internal specs, stories, strategy docs, etc.)
  • Push malicious commits or branches to inkeep/agents-private
  • Potentially pivot further depending on what other repos the App installation covers

Changes

  • .github/workflows/monorepo-pr-bridge.yml — adds a SECURITY: comment above the actions/checkout step in both the sync and close jobs explaining why ref: ${{ github.event.pull_request.head.sha }} must not be added there

Checklist

  • [N/A] Ran pnpm check (lint, typecheck, tests) locally
  • [N/A] Added a changeset (pnpm changeset) if this changes behavior
  • [N/A] Updated docs if this changes a user-facing surface
  • I have read CONTRIBUTING.md and agree to license my contribution under the project's terms (CLA)

After you open this PR

  • A maintainer will review your PR. If you don't hear back within a few business days, a comment to nudge is welcome.
  • When your change is accepted, this PR closes automatically — that's how it merges, and your authorship is preserved.

See CONTRIBUTING.md for details.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ kmarekspartz
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions

Copy link
Copy Markdown

Thanks for the contribution!

What happens next:

  • A maintainer will review your PR.
  • If you don't hear back within a few business days, please comment here to nudge our team.
  • This repository is maintained through an internal mirror. When your change is accepted, this PR will close automatically. Don't be alarmed when it closes — that's how it merges, and your authorship is preserved.

@kmarekspartz kmarekspartz changed the title Copilot/should workflows run on forks security: document intentional absence of PR head ref in pull_request_target checkouts Jul 28, 2026
@kmarekspartz

Copy link
Copy Markdown
Contributor Author

Given this isn't directly exploitable but more of a preventative comment, I did not go through the security process. Let me know if I should have!

@robert-inkeep
robert-inkeep removed the request for review from nick-inkeep July 28, 2026 18:33
@robert-inkeep robert-inkeep self-assigned this Jul 28, 2026
@robert-inkeep

Copy link
Copy Markdown
Contributor

thanks, i have a todo to bump to v7 and will address this too

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