Repository navigation
chore(deps): drop pip-audit, gate CI on uv audit, require uv >=0.12.15 (CLI-16) - #768
Conversation
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: auto_approve (risk 2/5) · profile _default
Dev-tooling/CI-only swap of pip-audit for uv audit; no runtime behavior or runtime deps change.
…5 (CLI-16) pip-audit entered the dev group in v0.6.0. No CI step, Makefile target, pre-commit hook, or doc ever ran it. It was also the largest dependency subtree: 29 of 80 locked packages, 19 exclusive. uv audit finds the same issues as pip-audit. It reads uv.lock directly and needs no pip. uv audit --frozen now runs in the CI check job and in make check. uv audit's CLI interface differs between the 0.11 and 0.12 lines: the --preview-features flag value was renamed from audit to audit-command. Every workflow now pins uv to 0.12.15, and [tool.uv] required-version sets that as the new minimum.
6c0bd8d to
4716057
Compare
Dismissing prior approval — a new commit was pushed and this review was for an earlier SHA. Run @keboola-pr-reviewer-bot review to get a fresh verdict.
|
New commit on |
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: needs_human (risk 3/5) · profile psgo
Escalating: this PR edits five CI/CD workflow files and the release pipeline, which policy never auto-approves.
Impact flags: possible rollback re-introduction — see Check Run summary.
Concerns:
.github/workflows/release.yml: Release/publish pipeline uv pin bumped 0.11.16->0.12.15 (minor-boundary CLI change).github/workflows/ci.yml: New uv audit --frozen gate added to CI check job; can block mergespyproject.toml: required-version >=0.12.15 makes 0.11 unusable for all contributors
Suggested reviewers: @, this PR needs a human reviewer because it modifies .github/workflows/** and the release pipeline, which policy never auto-approves; please assign a PSGO teammate via the Reviewers panel.
zajca
left a comment
There was a problem hiding this comment.
Actionable findings from the automated review.
|
|
||
| - name: Dependency audit | ||
| # Checks locked packages against OSV; the preview warning is expected. | ||
| run: uv audit --frozen |
There was a problem hiding this comment.
Reviewed by Opus.
uv audit --frozen is now a hard gate in the check job, but its result is not a function of the diff. It queries the OSV service over the network and re-evaluates all 61 locked packages against advisories published since the last run, so a newly disclosed CVE in any transitive dependency — or an OSV outage — turns every open PR red with no relation to its contents.
This cuts directly against the design the surrounding comments in this same file state. The job header at lines 37-42 describes these gates as "deterministic and interpreter-independent", and lines 44-48 explain that changelog-check is deliberately excluded from CI precisely because it needs network and audits state outside the diff ("a release-time concern, not a per-PR one"). A dependency audit has exactly that shape.
There is also no waiver path. Neither the CI step nor the audit target in Makefile:71 passes --ignore or --ignore-until-fixed, and nothing documents how to handle an advisory with no fixed version available. When that happens the only way to unblock merges is to edit the workflow or Makefile under time pressure. Separately, make check is now network-bound, which weakens the "run make check before pushing" instruction at CONTRIBUTING.md:871 for anyone working offline.
Worth considering: run the audit on a schedule (or as a continue-on-error / separate non-required job) so a fresh advisory opens a tracking issue instead of blocking unrelated work, and document the --ignore-until-fixed escape hatch next to the target.
Minor related note: uv audit still self-identifies as experimental ("warning: uv audit is experimental and may change without warning"), and this PR itself documents that its CLI changed incompatibly between uv 0.11 and 0.12. Passing --preview-features audit-command explicitly would make the preview opt-in deliberate and silence the warning the comment currently apologises for.
| make lint # Just the ruff linter | ||
| make format # Auto-format code | ||
| make typecheck # Static type check (Astral `ty`) | ||
| make audit # Audit locked dependencies for known vulnerabilities (uv audit, OSV) |
There was a problem hiding this comment.
Reviewed by Opus.
The "Running CI Locally" block was updated here (lines 855 and 859), but the "CI workflows" section further down was not. CONTRIBUTING.md:881-887 enumerates what the check job runs — "lint, format, ty type-check, SKILL.md freshness, version consistency, the command-sync and version-gate silent-drift gates, the serve endpoint-reference freshness check, and the error-code enum check" — and still omits the dependency audit that .github/workflows/ci.yml:73-75 now adds.
That same paragraph also asserts the job's gates "are deterministic and interpreter-independent", which no longer holds once an OSV lookup is part of it (see C001). Given how much this repo invests in keeping documentation and gates in sync, a contributor reading that section will not learn that a CI failure can now come from a dependency advisory rather than from their diff. Adding the audit to the enumeration — and noting why it can fail without any code change — keeps the section accurate.
aea2fbf to
811e9d1
Compare
Addresses the review. The audit was a step in the `check` job, so a finding turned that whole static-analysis gate red on the OSV database's schedule rather than on the diff. Move it to its own job that is not a required check: a finding shows red for visibility but never blocks a merge. Drop it from `make check` so that stays offline, keep the standalone `make audit` target, and document the job in CONTRIBUTING.md.
811e9d1 to
be6f5b3
Compare
keboola-pr-reviewer-bot
left a comment
There was a problem hiding this comment.
Verdict: auto_approve (risk 1/5) · profile _default
Dev-tooling and CI-config change only; no runtime code, reversible, and it adds rather than removes a security signal.
zajca
left a comment
There was a problem hiding this comment.
No actionable findings were found by the automated review.
Covers everything merged since v0.94.0: #768 (uv audit / uv >= 0.12.15), #772 (device-login "press c to copy") and #773 (test housekeeping). No `vNEXT` placeholders were outstanding (`make vnext-check` clean), and no `whatsnew.ts` entry is added -- nothing under `web/` changed this release, so the popup correctly has nothing new to show. gotchas.md: #772 reordered the device-login panel without updating the doc surface. The 0.92.0 "one-click link is conditional" gotcha quoted the old label ("Or open this link (code pre-filled):") and the old ordering, so a relay keying on either now picks the wrong line -- rewritten, plus a new entry for `press c to copy` being absent in exactly the situations an agent drives (`--json`, no TTY, no clipboard command). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why
pip-auditentered thedevgroup in v0.6.0 (dcc5413). No CI step, Makefile target, pre-commit hook, document, or agent instruction ever ran it. The check covered the full git history.uv.lock: 29 of the 80 locked packages. Nothing else uses 19 of those 29.pipitself enters the lockfile only throughpip-auditandpip-api.uv auditfinds the same vulnerability onmainas pip-audit does. Funnily enough, the vulnerable package ispipitself. For a project built on uv,uv auditfully replaces pip-audit: it readsuv.lockdirectly and needs no pip. Dependabot already watches the lockfile, so the audit runs as a complement, not a blocking gate.What
pyproject.tomldropspip-auditfrom[dependency-groups].dev. The re-lockeduv.lockremoves 19 packages and adds none..github/workflows/ci.ymladds aDependency auditstep to thecheckjob (uv audit --frozen), right afteruv sync. The step prints an expected preview warning line..github/workflows/ci.ymladds a separateauditjob that runsuv audit --frozen. It is not a required check: a finding shows red for visibility but does not block a merge. Its verdict depends on the OSV database over time, not on the diff, so it stays out of thecheckjob to keep that job deterministic.Makefileadds anaudittarget and includes it inmake check, which the CI job mirrors.CONTRIBUTING.mdlists it under "Running CI Locally".Makefileadds anaudittarget for local use; it is not part ofmake check, somake checkstays offline-capable and mirrors the CIcheckjob.CONTRIBUTING.mddocuments the new job under "CI workflows" and listsmake audit.ci.yml,e2e.yml,release.yml,release-kbagent.yml, and.github/actions/setup-buildnow pin uv to 0.12.15.[tool.uv] required-version = ">=0.12.15"makes 0.12 the new minimum.uv audit's CLI interface differs between 0.11 and 0.12: the--preview-featuresflag value was renamed fromaudittoaudit-command.docs/adr/0003names 0.11.16 as the pin at decision time. This PR leaves that line unchanged.Verified
uv lock --checkanduv lock --dry-runwith uv 0.12.15 report no lockfile changes.uv audit --frozenwith uv 0.12.15 on the branch reports no known vulnerabilities in 61 packages and exits 0.A cross-check against pip-audit 2.10.1 (
uvx --with pip pip-audit -r <uv export>) uses both the PyPI and OSV services. It finds the same result: the finding onmain, nothing on the branch. pip-audit also skipscolorama, a Windows-only marker.uv auditcovers it too.With uv 0.12.15, all of these pass:
uv sync --extra serverruff checkruff format --checkty check(only the known, downgraded hatchlingunresolved-importwarning)uv build --wheelmake auditmake -n checkrequired-versionrefuses uv 0.11.16 with a clear message.Follow-up (separate-job revision):
uv audit --frozenworks on a clean checkout with nouv sync, so the job needs no install step.ci.ymlparses as valid YAML with jobscheck,audit,test,build-windows.make -n checkno longer runs the audit;make -n auditdoes.Future
In the future we might consider dropping uv audit entirely in favor of Dependabot, which already runs on this repository. However, as it is a built-in feature of uv now, we will just perform the switch this time and make it at least remotely useful.
Closes CLI-16. Dependabot #727 (pip 26.1.2 to 26.2) becomes obsolete after merge.