fix(ci): add trusted self-mutation reusable workflow (1/2) - #866
Open
darccio wants to merge 1 commit into
Open
Conversation
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd25e4bc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
darccio
added a commit
that referenced
this pull request
Aug 3, 2026
Addresses the Codex review on #866: matching the PR merge ref would let a pull request rewrite the token-minting steps, which is exactly what #846 set out to prevent. Instead of broadening the trust policy to untrusted merge refs, move the token minting and push into a new .github/workflows/self-mutation.yml reusable workflow, called from validate.yml pinned at @main. Because the OIDC job_workflow_ref claim resolves to the *called* reusable workflow, it now reads self-mutation.yml@refs/heads/main -- a ref no pull request can influence -- so the policy can assert it while the feature actually works again. Notes: - The call MUST use the full DataDog/orchestrion/... @main form. A relative ./.github/workflows/self-mutation.yml reference resolves to the caller's commit (the untrusted merge ref) and would defeat this. - The reusable workflow takes no inputs; the push target is derived from the pull_request event context so an untrusted caller cannot redirect it. The same-repo/maintainer_can_modify guard also moved into it, so it lives in trusted code. - ratchet:exclude and an allowlist entry are needed because this one reference is intentionally a branch ref rather than a SHA.
darccio
force-pushed
the
dario.castane/infallible-tharp-ba69a7
branch
from
August 3, 2026 16:01
86829b2 to
ae88a7d
Compare
Groundwork for fixing the self-mutation token exchange, which has been failing since #846. #846 constrained the dd-octo-sts policy to validate.yml@refs/heads/main. That constraint is unsatisfiable: for pull_request events GitHub runs workflows from the PR merge ref, so the job_workflow_ref claim is always validate.yml@refs/pull/<N>/merge. The policy has therefore never matched and self-mutation has been dead. Simply relaxing the policy to accept merge refs would reintroduce what #846 set out to prevent, since a pull request can rewrite validate.yml and mint the contents: write token from its own code. Instead, the token minting moves here, into a workflow that validate.yml will call pinned at @main. job_workflow_ref resolves to the *called* reusable workflow, so the claim becomes self-mutation.yml@refs/heads/main -- a ref no pull request can influence. This commit only adds the workflow; nothing calls it yet. GitHub resolves uses: ...@main when it creates a run, so this file must be on main before validate.yml can reference it (otherwise the whole run fails to load). The wiring and policy update follow once this has landed.
darccio
force-pushed
the
dario.castane/infallible-tharp-ba69a7
branch
from
August 3, 2026 16:03
ae88a7d to
fb37000
Compare
3 tasks
|
🎯 Code Coverage (details) 🔗 Commit SHA: fb37000 | Docs | Datadog PR Page | Give us feedback! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 1 of 2 to fix the
self-mutationtoken exchange, which has been failing on every PR since #846.The bug
#846 constrained the trust policy to
job_workflow_ref: ...validate.yml@refs/heads/main. That constraint is unsatisfiable. Forpull_requestevents GitHub runs workflows from the PR merge ref, so the claim is alwaysvalidate.yml@refs/pull/<N>/merge— as seen in a real failing run:So the policy has never matched,
dd-octo-stsreturnspermission denied, and self-mutation has been silently dead for ~5 weeks.Why not just relax the policy to accept merge refs
That was this PR's first attempt, and Codex correctly flagged it: matching any
refs/pull/<N>/mergereintroduces exactly what #846 set out to prevent, since a PR can rewritevalidate.yml's steps and mint the repo-widecontents: writetoken from its own code.Corrections to that review
Codex's finding was right in substance, but two details in it don't hold up and are worth recording so they don't get relitigated later:
id-token: readon public repos, and the org setting that would send write tokens to PR workflows applies only to private repos — so a fork cannot mint this token under any circumstance. The real exposure is the same-repository PR path Codex also named (anyone with push access rewritingvalidate.ymlon their own branch), which is what actually drove this fix.main") has an unstated trap. A reusable workflow called by a relative path (./.github/workflows/self-mutation.yml) resolves against the caller's commit, notmain— so ifvalidate.ymlreferenced it that way,job_workflow_refwould still be the untrusted merge ref, silently defeating the fix. Only the fully-qualifiedDataDog/orchestrion/.github/workflows/self-mutation.yml@mainform anchors it correctly. This is called out in the new file's comments so it doesn't get "simplified" away in a future edit.The fix
Move the token minting into this reusable workflow, which
validate.ymlwill call pinned at@main. Becausejob_workflow_refresolves to the called reusable workflow, the claim becomesself-mutation.yml@refs/heads/main— a ref no PR can influence. That satisfies #846's intent and makes the feature work again.DataDog/orchestrion/...@mainform (see correction Wrap http handler and handler func #2 above).pull_requestevent context so an untrusted caller can't redirect it. The same-repo/maintainer_can_modifyguard also moved in here, so it now lives in trusted code.Why this is split into two PRs
GitHub resolves
uses: ...@mainwhen it creates a run, not when the job executes. Adding the reference in the same PR that adds the file makes the entire workflow fail to load — verified empirically on this branch (run 30830039834: zero jobs, instant failure, run named by file path becausename: Testscouldn't even be parsed). So this file must reachmainfirst.This PR is inert:
on: workflow_callwith no callers, so it changes no behaviour and can't break anything. Thevalidate.ymlwiring + policy update are stacked on top in 2/2 and will merge once this lands onmain.Test plan
actionlintcleanpermission denied)