fix(ci): install and audit the committed dependency graph (#678) - #700
Draft
willhea wants to merge 7 commits into
Draft
fix(ci): install and audit the committed dependency graph (#678)#700willhea wants to merge 7 commits into
willhea wants to merge 7 commits into
Conversation
When pyproject.toml and uv.lock disagreed, the test jobs and the required vulnerability audit operated on different dependency graphs and nothing detected the mismatch. `uv sync` re-resolved and rewrote uv.lock inside the runner, so the tests ran against a graph that was never committed or reviewed, while `uv export --frozen` read the stale committed lockfile without checking it, so pip-audit certified a set that did not contain the new dependency. A package with a known advisory could therefore enter the tested graph with the required security check reporting no findings, and a green audit of the wrong thing is indistinguishable from a green audit of the right one. `--locked` asserts rather than assumes, and is what Astral's own GitHub Actions guide uses. Applied to all eight `uv sync` steps and to the security export. The ten `uv run` invocations stay bare, matching that guide command-for-command, because a house-specific spelling invites a later contributor to normalise it back to the documented one. The regression test pins two rules, since the documented pattern carries an ordering dependency nothing else wrote down: every sync and export asserts --locked, and every `uv run` either asserts for itself or is preceded in its own job by a locked sync. Rule 2 exists because `uv run` is a second install path rather than merely a runner. uv locks and syncs before invoking the command, so deleting a job's install step reads as a harmless cleanup while silently returning that job to re-resolving, with a flags-only check still green. Verified against uv 0.12.5 on a tree declaring a dependency uv.lock did not contain: `uv sync --locked` and `uv export --locked` both fail closed and leave uv.lock untouched; the same declaration accompanied by its lock update passes and the new package then appears in the audited set; and `uv run` alone installs it and rewrites the lockfile. Each rule was mutation-tested against the real workflows, including the deleted-install-step case that rule 1 alone cannot see, and no pre-existing test reddens on any of those mutations. Fast suite, lint, format and the packaging gate are green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Repairs a wrong assumption in this branch's own guard, found in review.
The previous revision treated a preceding `uv sync --locked` as sufficient
protection for the bare `uv run` after it, on the reasoning that a failed step
ends the job. That is true for a workflow on the default `success()` step
condition, and it is what Astral's GitHub Actions guide shows. It is NOT true
here: every `uv run` step in ci.yml carries `if: ${{ !cancelled() }}`,
deliberately, so a red gate still reports the rest of its tier. That same
condition means a failed `uv sync --locked` does not stop the steps after it, and
`uv run` is a second install path rather than merely a runner.
Reproduced against uv 0.12.5 on a tree declaring a dependency `uv.lock` did not
contain, checking the lockfile hash after each command:
uv sync --locked exit 1, uv.lock unchanged (63d820f0...)
uv export --locked exit 2, uv.lock unchanged
uv run --locked ... exit 2, uv.lock unchanged
uv run ... exit 0, INSTALLED tabulate and REWROTE uv.lock (7ed9be75...)
With the lockfile updated, all three locked commands succeed and the new package
appears in the audited export.
So `--locked` now goes on all ten project-context `uv run` invocations as well as
the eight `uv sync` and the one `uv export`. `uv python install` (9) and `uvx`
(1) are untouched: neither reads the project lockfile and neither accepts the
flag. `update-examples.yml` invokes no uv at all.
The guard collapses from two rules to one: every workflow command that can
resolve the project graph must carry `--locked` itself. The `locked_sync_seen`
state is gone, and with it any dependence on job order, step order, `if:`
conditions or `continue-on-error`. The ordering, sibling-job and commented-out
install controls went with it -- they existed to pin the state machine, and a
stateless rule cannot exhibit the failures they described.
Four controls remain, one per branch of the verdict: a bare `uv sync` is
rejected, `uv export --frozen` is rejected, a bare `uv run` is rejected EVEN when
preceded by `uv sync --locked`, and `uv run --locked` is accepted. The third is
the case the old guard got wrong, and it fails under the old rule.
Verification, each mutation applied to a real workflow and reverted:
- `--locked` removed from one `uv sync` -> guard red, 25 others green
- the export's `--locked` back to `--frozen` -> guard red, 25 others green
- `--locked` removed from one `uv run` -> guard red, 25 others green
No pre-existing test fires on any of the three, so there is no overlapping
coverage to remove. The third mutation is the one the previous revision could not
see at all.
Also merges origin/develop (22d48c6) into the branch, which was 70 commits
behind. No conflicts; ci.yml auto-merged, and develop's newly added
tests/test_classify_bill.py entry is covered by the same guard.
Local: test_ci_workflow.py 26 passed; fast suite 2069 passed / 4 skipped /
15 xfailed; ruff check and format clean; packaging gate 7 passed.
Out of scope, deliberately: pinning the setup-uv action SHA or an exact uv
version. Current Astral guidance recommends it, but it is repository-wide work
rather than part of this issue.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes a false green in this branch's own guard, found in review. `_ASSERTS_LOCKED` searched the whole command string for `--locked`. For `uv run`, everything after the child command belongs to that child, so the flag can be present in the text and absent from uv's own arguments. The subcommand pattern also required `uv` and its subcommand to be adjacent, so a global option in between hid the command from the guard entirely. Both reproduced against uv 0.12.10 on a tree declaring a dependency `uv.lock` did not contain, hashing the lockfile after each command: uv run --locked python -c ... exit 2, uv.lock unchanged (63d820f0...) uv run python -c ... --locked exit 0, uv.lock REWRITTEN (7ed9be75...) uv --project . run python -c ... exit 0, uv.lock REWRITTEN The second printed `child got: ['--locked']`: python received the flag, uv never saw it. Run against the OLD predicate, both are accepted -- the first as "locked", the second not recognised as a uv invocation at all. The guard now tokenises with shlex, steps over `NAME=value` prefixes and uv's global options (the six that consume a following token, per `uv --help`, so `--project .` cannot be mistaken for the subcommand), and requires the repository's canonical spelling: `--locked` immediately after the subcommand. That is stricter than uv's own parser, which would accept the flag later among uv's options. Deliberately so: it is the one position that cannot silently belong to a child command, and #672 is this repository's record of what a convention admitting unchecked variation costs. Tests consolidated per review: the live invariant plus one parameterised negative control over five spellings (bare sync, frozen export, bare run after a locked sync, flag past the child command, global option before the subcommand). The separate positive control is gone -- the live workflows supply it, since all nineteen resolving commands carry the flag and a rule rejecting everything would redden there. The assertions pinning diagnostic wording are gone too, and the three branch-specific messages collapse into one that names the canonical form. Also corrects an overstatement in the previous revision's rationale. `if: !cancelled()` lets later steps EXECUTE after a failed `uv sync --locked`; it does not erase that failure, so the job and the required `test` aggregator are red either way and a stale lockfile cannot merge on this alone. What locking every `uv run` buys is that no step resolves or rewrites an uncommitted graph inside the runner before anyone reads the red, and that the failure is attributed to the command that caused it. Defence-in-depth and attribution, not the merge block. Verification, each mutation applied to a real workflow and reverted: - `--locked` moved past the child command -> guard red, 26 others green. This is the case the old predicate called safe. - `--locked` removed from a `uv sync` -> guard red, 26 others green - the export's `--locked` back to `--frozen` -> guard red, 26 others green - `--locked` removed from a `uv run` -> guard red, 26 others green No pre-existing test fires on any of them. Merges origin/develop (22d48c6..) into the branch, which was 15 commits behind. No conflicts; none of those commits touch the four files this PR changes. Local: test_ci_workflow.py 27 passed; fast suite 2080 passed / 4 skipped / 15 xfailed; ruff check and format clean; packaging gate 7 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g options (#678) Fixes the last fail-open in this branch's guard, found in review. The parser enumerated the six global options that consume a following token, so it could find where the subcommand began. That list was silently incomplete: `--trusted-host` is accepted by uv and appears nowhere in `uv --help`. Confirmed locally -- `uv --trusted-host example.com --version` prints the version. Reproduced against uv 0.12.10 on a stale tree: uv --trusted-host example.com run python -c 'print("ran")' exit 0, uv.lock REWRITTEN (63d820f0... -> 7ed9be75...) parser verdict: None, so the guard stayed green The parser skipped `--trusted-host` but not `example.com`, read that value as the subcommand, found it was not `sync`/`export`/`run`, and returned None. `setup-uv` installs an unpinned uv, so the list would have drifted as options are added even if it had been complete on the day it was written. The list is gone. A `uv` command whose first argument is an OPTION is now reported as unasserted whenever a resolving subcommand appears anywhere in it, rather than parsed. Enforcing the canonical shape needs no knowledge of option arity and fails closed on exactly the case it cannot read. No workflow here spells a resolving command any other way, and one that wants to is told to put the subcommand first. The fail-closed branch is deliberately narrow: it requires a resolving subcommand to be present, so `uv --version` is left alone. A second parameterised test pins that, over `uv --version`, `uv python install`, `uvx pip-audit` and `uv build`, because widening the branch to "any `uv -`" would otherwise go unnoticed. Parser verdicts across every spelling in play: ('run', False) uv --trusted-host example.com run python -c 'print(1)' ('run', False) uv --project . run pytest -v ('run', False) uv run python -c 'print(1)' --locked ('run', True) uv run --locked pytest -v ('sync', True) uv sync --locked ('export', True) uv export --locked --format requirements-txt -o r.txt None uv --version None uv python install 3.12 None uvx pip-audit -r r.txt None uv build --wheel Verification, each mutation applied to a real workflow and reverted. Live guard red, 31 others green, every time: - `uv --trusted-host example.com run --locked pytest ...` (the new case; note it carries a correct `--locked` that uv would honour, and is reported anyway because the spelling cannot be verified) - `--locked` removed from a `uv sync` - the export's `--locked` back to `--frozen` - `--locked` removed from a `uv run` Local: test_ci_workflow.py 32 passed; fast suite 2085 passed / 4 skipped / 15 xfailed; ruff check and format clean; packaging gate 7 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #678.
What changed
Nineteen one-word edits across three workflows, plus one regression guard.
uv syncsteps (seven inci.yml, one incorpus-parity.yml) useuv sync --locked.uv export --lockedin place of--frozen.uv runinvocations useuv run --locked.Untouched, because none of them reads the project lockfile or accepts the flag:
uv python install(9),uvx pip-audit(1).update-examples.ymlinvokes no uv at all.No separate
uv lock --checkstep, per the issue: it fires on exactly the condition these commands already fail on, in exactly the runs where they already fail.Why
When
pyproject.tomlanduv.lockdisagreed, the test jobs and the required vulnerability audit operated on different dependency graphs, and nothing detected the mismatch.uv syncre-resolved and rewroteuv.lockinside the runner, so the tests ran against a graph that was never committed.uv export --frozenread the stale committed lockfile without checking it, sopip-auditcertified a set that did not contain the new dependency.--lockedis the sibling flag that asserts rather than assumes: "Assert that theuv.lockwill remain unchanged."Why every command, and not just the install step
Corrected after review. An earlier revision argued that
uv run --lockedwas what stood between a stale lockfile anddevelop. It is not.if: ${{ !cancelled() }}lets the later steps execute after a faileduv sync --locked; it does not erase that failure, so the job and the requiredtestaggregator are red either way.What locking every
uv runactually buys is narrower and still worth having: without it, those steps still run — resolving and rewriting an uncommitted graph inside the runner, and testing against it, before anyone reads the red — and the failure is attributed to the wrong command. Defence-in-depth and correct attribution, not the merge block.The underlying fact is that
uv runis a second install path, not merely a runner; uv's docs are explicit that the project "is locked and synced before invoking the requested command."Reproduced against uv 0.12.10 on a tree declaring a production dependency
uv.lockdid not contain, hashing the lockfile after each command:uv.lockuv sync --locked63d820f0…uv export --locked63d820f0…uv run --locked python -c …63d820f0…uv run python -c … --locked7ed9be75…uv --project . run python -c …7ed9be75…uv --trusted-host example.com run python -c …7ed9be75…With the lockfile updated alongside the declaration, all three locked commands succeed and the new package appears in the audited export, so the fix does not block ordinary dependency work.
The guard
The rule is one sentence:
It reads no job order, no step order, no
if:condition and nocontinue-on-error.Two review rounds shaped this, and both findings were real. The table above is the record:
--locked. Everything afteruv run's child command belongs to that child, souv run python -c '…' --lockedpasses the flag to python — the reproduction printedchild got: ['--locked']— while uv rewrites the lockfile. A substring search called that safe.--trusted-hostis accepted by uv and appears nowhere inuv --help. The parser skipped the option but notexample.com, read that value as the subcommand, and returned "not a uv command".setup-uvinstalls an unpinned uv, so the list would have drifted regardless.So the list is gone. A
uvcommand whose first argument is an option is now reported as unasserted whenever a resolving subcommand appears anywhere in it, rather than parsed. Enforcing the canonical shape needs no knowledge of option arity and fails closed on exactly the case it cannot read.The fail-closed branch is deliberately narrow — it requires a resolving subcommand to be present — so unrelated uv commands are left alone. Parser verdicts across every spelling in play:
('run', False)uv --trusted-host example.com run python -c 'print(1)'('run', False)uv --project . run pytest -v('run', False)uv run python -c 'print(1)' --locked('run', True)uv run --locked pytest -v('sync', True)uv sync --locked('export', True)uv export --locked --format requirements-txt -o r.txtNoneuv --versionNoneuv python install 3.12Noneuvx pip-audit -r r.txtNoneuv build --wheelThe canonical requirement is stricter than uv's own parser, which would also accept
--lockedlater among uv's options. Deliberately: it is the one position that cannot silently belong to a child command, and #672 is this repository's record of what a convention admitting unchecked variation costs.Tests
The live invariant, plus two parameterised controls.
Negative — each is a way to resolve an uncommitted graph while looking correct:
bare-syncfrozen-exportbare-runlocked-after-child-commandglobal-option-before-subcommandundocumented-global-option--trusted-host, absent fromuv --helpPositive — commands that resolve nothing and must not be swept up by the fail-closed branch:
uv --version,uv python install,uvx pip-audit,uv build. Without these, widening the branch to "anyuv -" would go unnoticed.No positive control for the canonical spelling itself: the live workflows supply it, since all nineteen resolving commands carry the flag, so a rule that rejected everything would redden there.
Verification
Each mutation applied to a real workflow file, test run, failure observed, file restored. Live guard red, 31 others green, every time:
uv --trusted-host example.com run --locked pytest …--lockedthat uv would honour; reported anyway because the spelling cannot be verified--lockedremoved from auv sync--lockedback to--frozen--lockedremoved from auv runNo pre-existing test fires on any of them.
Local gates.
tests/test_ci_workflow.py32 passed; fast suite 2085 passed / 4 skipped / 15 xfailed;ruff checkandruff format --checkclean; packaging gate 7 passed — confirming--lockedon the dev-environment commands does not interfere with theuv build/uv pip installthe gate performs internally.Caveat: measured in a worktree lacking the gitignored fetched corpus, which reports 4 skips where the main checkout reports 1. None relate to this change.
Scope of the guard
.github/workflowsonly. Theuv runlines inAGENTS.mdandCONTRIBUTING.mdare documentation for a developer's own machine, where a rewritten lockfile appears ingit statusrather than silently inside a runner.Out of scope
Pinning the
setup-uvaction to a SHA, or pinning an exactuvversion. Current Astral guidance recommends both, and the--trusted-hostfinding is a reminder that an unpinned uv can change under the workflows — but that is repository-wide work rather than part of this issue and belongs in its own change.Not covered
pyproject.tomlchange without its lock update. Not searched.dependabot.ymlinteraction. Dependabot's uv updates normally carry the lock change, so its pull requests are probably not an exposed path; not checked.