chore(ci): automate Dependabot license updates - #1893
Conversation
This comment has been minimized.
This comment has been minimized.
Remove the over-defensive validation in apply_dependabot_license_update.yml
that duplicated checks already enforced by the generate workflow's job
gates, the workflow_run trigger, or the artifact download pinning. The
remaining checks are the ones with real security value.
What was removed, and why it is safe:
1. Run-provenance checks in "Validate update"
(run.name, run.path, run.event, run.conclusion, run.head_repository)
- The apply workflow is triggered by workflow_run pinned to
workflows: ["Generate Dependabot License Update"], so run.name and
run.path are guaranteed by the trigger filter.
- run.event, run.conclusion, and run.head_repository are already
checked by the apply job's `if:` gate:
workflow_run.conclusion == 'success'
&& workflow_run.event == 'pull_request'
&& workflow_run.head_repository.full_name == github.repository
2. Metadata fields guaranteed by construction
- metadata.repository: written by generate as context.repo.owner/repo;
the apply only runs for this repo's runs.
- metadata.run_id: the artifact is downloaded with
run-id: ${{ github.event.workflow_run.id }}, so it is by definition
from the triggering run; generate wrote context.runId.
- metadata.head_ref/head_sha vs run.head_branch/head_sha: generate wrote
pull_request.head.ref/sha, which for a pull_request event are exactly
the run's head values.
- head_ref format (dependabot/cargo/ prefix, length) and head_sha format
(40-hex): guaranteed by the generate job gates and by construction.
3. Immutable pull request properties
- pr.user.login == 'dependabot[bot]': the generate job gate requires it
and the PR author is immutable.
- pr.head.repo / pr.base.repo == expectedRepository: the generate job
gate requires a same-repo head; repos are immutable per PR.
4. The base-ref check moved to the generate workflow
- pr.base.ref == 'main' now lives in the generate job `if:` as
github.event.pull_request.base.ref == 'main', alongside the other
PR-scope gates. Non-main dependabot PRs skip generation entirely. The
apply-side check is removed; a post-generate retarget is irrelevant
because the apply writes to the head branch, not the base.
5. The second re-check in "Apply validated update" trimmed to the two
mutable fields: freshPr.state === 'open' && freshPr.head.sha ===
metadata.head_sha. Author, base, and head repo are immutable or were
validated moments earlier.
6. Dead metadata fields removed
- metadata.json now carries only version, pull_request, head_ref,
head_sha. repository and run_id had no consumer after the removals
above; the exact-key-set check is updated in lockstep.
Safety guarantees that remain, and where they live:
- Who can produce the artifact: the generate job `if:` gates
(dependabot[bot] author, same-repo head, dependabot/cargo/ ref prefix,
base == main) in generate_dependabot_license_update.yml. The generate
workflow is read-only and holds no secrets.
- Artifact provenance: the apply downloads the artifact pinned to the
triggering run's id, so it cannot be swapped for another run's artifact.
- TOCTOU: "Validate update" re-fetches the live PR and requires it to still
be open with head.ref/head.sha matching the artifact; the Apply step
re-checks state and head.sha immediately before constructing the commit.
- Untrusted payload validation: exact artifact file set, metadata
size/JSON/key-set/version, and LICENSE-3rdparty.csv content (size,
UTF-8, no NUL, header) before it is committed.
Why removal is safe: every removed check was either duplicated by a gate
that already runs (the generate `if:` or the apply job `if:`), guaranteed
by the trigger/download mechanism, or true by construction (the generate
workflow writes its own context). None could fail on a legitimate artifact,
and none blocked an attack that the remaining checks do not already block.
The threat model is unchanged: the artifact is produced only for gated
dependabot PRs, is pinned to the triggering run, is bound to an unchanged
open PR head, and is validated before being committed.
Remove the LICENSE-3rdparty.csv content checks (size, UTF-8, NUL, header) from the "Validate update" step in apply_dependabot_license_update.yml. The CSV is now read only for the current-file comparison and the commit. Why this is safe: - Fudging the CSV requires write access to the dependabot branch. The generate workflow only runs for same-repository dependabot[bot] PRs (head repo == github.repository, dependabot/cargo/ ref, base == main), so forks cannot trigger it and non-maintainers cannot push to the branch. Direct pushes to main are blocked by branch protection, so the write-access actor's real capability is opening a PR with a malicious workflow file, which for a same-repo PR runs with full secrets. That is arbitrary code execution and is entirely orthogonal to the license updater: it does not depend on this workflow at all. - The CSV is inert data committed to a fixed path. The apply workflow hardcodes `LICENSE-3rdparty.csv` as the only tree entry, and the artifact file-set check enforces exactly that file plus metadata.json, so the workflow cannot be used to commit executable content or arbitrary paths. Content validation of that data file defends nothing against the write-access actor's actual capabilities. - A large CSV is at most a denial of service, and it is self-limiting: GitHub artifact and per-file blob size limits bound how large the artifact and the committed blob can be. Safety guarantees that remain, and where they live: - Who can produce the artifact: the generate job `if:` gates (dependabot[bot] author, same-repo head, dependabot/cargo/ ref prefix, base == main) in generate_dependabot_license_update.yml. The generate workflow is read-only and holds no secrets. - Artifact provenance: the apply downloads the artifact pinned to the triggering run's id, so it cannot be swapped for another run's artifact. - TOCTOU: "Validate update" re-fetches the live PR and requires it to still be open with head.ref/head.sha matching the artifact; the Apply step re-checks state and head.sha immediately before constructing the commit. - Untrusted artifact structure: exact artifact file set, metadata size/JSON/key-set/version, before the CSV is committed. The threat model is unchanged: the artifact is produced only for gated dependabot PRs, is pinned to the triggering run, is bound to an unchanged open PR head, and is committed without trusting its content.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30c9bbf537
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (pr.head.ref !== metadata.head_ref) { | ||
| fail(`Pull request head ref does not match artifact: ${pr.head.ref}`); | ||
| } | ||
| if (pr.head.sha !== metadata.head_sha) { |
There was a problem hiding this comment.
Restrict validated artifacts to Dependabot PRs
At this validation point the privileged workflow accepts any artifact whose metadata names an open PR with a matching head ref/SHA; it never checks that the referenced PR is a same-repository Dependabot Cargo PR targeting main. If a same-repository PR changes the generator workflow to upload this artifact, it can point metadata.pull_request at a PR whose head is a protected branch such as main, causing the app token path below to fast-forward that branch with the artifact contents. Mirror the generator’s author/base/head-repo/head-ref checks here before setting apply.
Useful? React with 👍 / 👎.
Summary
Motivation
Cargo dependency updates commonly leave
LICENSE-3rdparty.csvstale, causing otherwise mechanical Dependabot PRs to fail until a maintainer runs the same deterministic command and commits its output. This is recurring, low-judgment manual work, so automating it reduces maintainer toil and shortens dependency-update turnaround without weakening validation.The same guarded pattern could later automate other trivial maintenance operations, such as running
cargo fmtwhencargo fmt --checkfails. That generalization is not included in this PR.Changes
vectordotdev-bottoken only after validation succeeds, then use it to commit one fixed file and trigger normal CI.Flow
The split preserves the trust boundary. The workflow that executes pull-request-controlled code has read-only permissions and no secrets. The trusted continuation never checks out or executes PR code; it validates the repository, PR author, branch, run ID, artifact contents, and unchanged head SHA using the built-in token. Only then does it mint the App token, re-check mutable PR state, and update one fixed path with a non-forcing ref update.
Change Type
Is this a breaking change?
How did you test this PR?
actionlint1.7.12 and YAML structural validation.LICENSE-3rdparty.csvand then passedmake check-licenses.vectordotdev-bot[bot]committed onlyLICENSE-3rdparty.csv.Does this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.References