Skip to content

Commit b2b5d45

Browse files
Syndicclaude
andcommitted
ci(security): scan every workspace member, and stop calling CodeQL DB-driven
Addresses both review findings on this PR. pip-audit's export named the wrong constraint. `uv export` emits only the *root* project's dependencies unless asked for the whole workspace, so a member that is registered in `[tool.uv.workspace].members`, resolved into uv.lock, and passing every ci.yml check still had none of its dependencies handed to pip-audit — a green scan of a smaller set, indistinguishable from a green scan. Reproduced in the devcontainer on uv 0.12.5 with a two-package workspace whose member depends on idna: the old invocation exports packaging and no idna; with `--all-packages` it exports both. `--no-emit-workspace` supersedes `--no-emit-project` ("Do not emit any workspace members, including the root project"), keeping first-party packages out of the audit as `--all-packages` pulls them in. Verified end to end: 6 packages, all hashed, `--require-hashes --strict` passes. No behaviour change today — `members` is `[]`, which is exactly how this would have shipped unnoticed. `//meta/scripts:test_pip_audit_coverage` pins both flags so it cannot go back to being correct by coincidence. The review's remediation detail did not reproduce: on uv 0.12.5 neither `--no-emit-project` nor `--no-emit-workspace` emits an `-e` line, so `--require-hashes` was not the forcing constraint. `--no-emit-workspace` is still right, on semantics rather than on hashes. The section comment above ("one scan covers every member") was wrong for the same reason and is corrected rather than inherited. CodeQL is dropped from CLAUDE.md's external-database list. `codeql-action/init` is SHA-pinned with no `tools:` input, so its query bundle rides the action release: the verdict moves on a Renovate bump, a tree change. Left in the list, the new rule would have told someone to move that job to ci.yml and drop the weekly re-analysis code scanning's UI is fed by. It is now written up as the exception, with the reason it stays on the cron. The requirements_lock.txt trio keeps `--no-emit-project` and is untouched; the step notes the divergence so the two sets don't get harmonised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 44e6d5f commit b2b5d45

4 files changed

Lines changed: 126 additions & 9 deletions

File tree

.claude/CLAUDE.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,16 @@ matter:
7676
- **A check that reads only the tree belongs in `ci.yml`.** Same commit, same answer, forever — so
7777
re-running it tells you nothing you did not already know.
7878
- **A check whose verdict moves with an external database belongs in `security.yml`.** govulncheck,
79-
pip-audit, Semgrep, CodeQL and Trivy can each turn red on an untouched commit when an advisory
80-
lands, which is what the Monday cron is for.
79+
pip-audit, Semgrep and Trivy each fetch at run time — the Go vuln DB, PyPI advisories, the
80+
registry rule packs, Trivy's own DB — so any of them can turn red on an untouched commit when an
81+
advisory lands. That is what the Monday cron is for.
82+
83+
`codeql` is the exception, and worth knowing about before the rule gets applied to it. Its
84+
`codeql-action/init` is SHA-pinned with no `tools:` input, so the query bundle rides the action
85+
release: the verdict moves on a Renovate bump, which is a tree change, not on an advisory landing.
86+
By the criterion above it reads like a `ci.yml` job. It stays on the cron for a different reason —
87+
code scanning's UI is fed by periodic re-analysis, and the weekly cadence is inherited from the
88+
default setup this job replaced (see "CodeQL runs as advanced setup"). Moving it would drop that.
8189

8290
That axis is why `golangci-lint` moved out of Security in #18, and why `modules-check` later
8391
followed it out — a completeness gate over hand-listed matrices is a pure function of the tree, so

.github/workflows/security.yml

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ jobs:
120120
# differs by language — govulncheck does call-graph reachability (only fires when our code
121121
# reaches the vulnerable symbol); pip-audit is manifest-based (fires on any flagged version in
122122
# the resolved set). Both run once per resolution unit: govulncheck per Go module, pip-audit
123-
# once over the workspace-wide uv resolution (uv enforces single-version-per-package across
124-
# workspace members, so one scan covers every member).
123+
# once over the workspace-wide uv resolution — but only because its export asks for the whole
124+
# workspace; see the flags on the export step, which are what make "every member" true.
125125
# TEND(lang-expand): add an equivalent per-language scanner as languages are adopted
126126
# (e.g. cargo-audit for Rust, trivy fs --scanners vuln for languages without a dedicated
127127
# reachability tool).
@@ -180,10 +180,11 @@ jobs:
180180
# file is currently in sync — pre-commit and the Renovate auto-commit workflow each
181181
# police that on their own paths.
182182
#
183-
# Its coverage rests on one narrower clause than govulncheck's: a per-project pyproject.toml
184-
# that matches no glob in `[tool.uv.workspace].members` is absent from uv.lock, so the export
185-
# carries none of its dependencies and this job passes having scanned none of them. That is
186-
# `check_python_workspace_members` in meta/scripts/check_modules.py, enforced by ci.yml.
183+
# What this job actually covers is decided by the export flags below, not by any check in
184+
# ci.yml — see the note on that step. The ci.yml-side clause
185+
# (`check_python_workspace_members`) is the weaker, second condition: a per-project
186+
# pyproject.toml matching no glob in `[tool.uv.workspace].members` never reaches uv.lock at
187+
# all, so no export flag could recover it.
187188
pip-audit:
188189
name: pip-audit
189190
runs-on: ubuntu-latest
@@ -198,9 +199,19 @@ jobs:
198199
with:
199200
# renovate: datasource=python-version depName=python
200201
python-version: "3.14"
202+
# `--all-packages` is load-bearing: without it `uv export` emits the *root* project's
203+
# dependencies only, so a workspace member that is registered, locked, and passing every
204+
# ci.yml check still has none of its dependencies scanned — a green job that looked at
205+
# nothing. Latent while `members` is empty, which is exactly how it would ship unnoticed.
206+
# `--no-emit-workspace` supersedes `--no-emit-project`: it drops every member rather than
207+
# just the root, so first-party packages stay out of the audit as `--all-packages` pulls
208+
# them in. //meta/scripts:test_pip_audit_coverage holds both flags here.
209+
# These deliberately differ from the `requirements_lock.txt` trio (pre-commit hook, ratify
210+
# script, freshness check), which must stay identical to *each other*; don't harmonise the
211+
# two sets — see TestUvExport in meta/scripts/test_check_modules.py.
201212
- name: Export uv.lock to requirements.txt
202213
run: |
203-
uv export --format requirements-txt --no-emit-project \
214+
uv export --format requirements-txt --all-packages --no-emit-workspace \
204215
--output-file "${RUNNER_TEMP}/requirements.txt"
205216
- name: pip-audit
206217
env:

meta/scripts/BUILD.bazel

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,16 @@ py_test(
115115
main = "test_codeql_toolchain.py",
116116
)
117117

118+
# Same shape as the two above: the assertion is the whole gate. What it guards is invisible while
119+
# the uv workspace has no members — the export is correct by coincidence until the day one lands.
120+
py_test(
121+
name = "test_pip_audit_coverage",
122+
size = "small",
123+
srcs = ["test_pip_audit_coverage.py"],
124+
data = ["//:.github/workflows/security.yml"],
125+
main = "test_pip_audit_coverage.py",
126+
)
127+
118128
py_library(
119129
name = "classify_changed_paths_lib",
120130
srcs = ["classify_changed_paths.py"],
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Holds pip-audit's export to the flags that decide what it actually scans.
2+
3+
`uv export` emits the **root** project's dependencies unless asked for the whole workspace. So
4+
without `--all-packages`, a workspace member that is registered in `[tool.uv.workspace].members`,
5+
resolved into `uv.lock`, and passing every check in ci.yml still contributes nothing to the file
6+
pip-audit reads — the job goes green having looked at none of its dependencies.
7+
8+
Reproduced on uv 0.12.5 with a two-package workspace whose member depends on `idna`: the default
9+
export carries `packaging` (the root's dep) and no `idna`; adding `--all-packages` carries both.
10+
11+
Nothing fails when the flag is dropped. `members` is `[]` today, so the export is correct by
12+
coincidence rather than by construction, and the day a member lands is the day the coincidence
13+
ends — silently, since a passing scan of a smaller set looks exactly like a passing scan.
14+
15+
`--no-emit-workspace` is the second half. It supersedes `--no-emit-project` (uv: "Do not emit any
16+
workspace members, including the root project"), keeping first-party packages out of the audit as
17+
`--all-packages` pulls them in. Swapping it back for `--no-emit-project` would emit members into a
18+
file consumed with `--require-hashes`.
19+
"""
20+
21+
import re
22+
import unittest
23+
from pathlib import Path
24+
25+
# Not .resolve(): the workflow is a cross-package data dep, so it lives in the runfiles tree
26+
# beside this file rather than at the source path a resolved symlink would lead back to.
27+
_WORKFLOW = Path(__file__).parent.parent.parent / ".github" / "workflows" / "security.yml"
28+
29+
_REQUIRED_FLAGS = ("--all-packages", "--no-emit-workspace")
30+
31+
32+
def export_command() -> str:
33+
"""The `uv export` invocation from the pip-audit job, line continuations folded out."""
34+
text = _WORKFLOW.read_text(encoding="utf-8")
35+
match = re.search(r"^\s*(uv export\b[\s\S]*?)(?=\n\s*- name:|\n\s*-\s+uses:)", text, re.M)
36+
if match is None:
37+
raise AssertionError("no `uv export` invocation in security.yml")
38+
return " ".join(match.group(1).replace("\\\n", " ").split())
39+
40+
41+
class ExportCoverageTest(unittest.TestCase):
42+
def setUp(self):
43+
self.command = export_command()
44+
45+
def test_export_covers_every_workspace_member(self):
46+
self.assertIn(
47+
"--all-packages",
48+
self.command,
49+
"without it `uv export` emits only the root project's dependencies, so a workspace "
50+
"member's deps are never handed to pip-audit and the job passes having skipped them",
51+
)
52+
53+
def test_first_party_packages_stay_out_of_the_audit(self):
54+
self.assertIn(
55+
"--no-emit-workspace",
56+
self.command,
57+
"`--all-packages` pulls members into the export; this is what keeps them from being "
58+
"emitted into a file that pip-audit consumes with --require-hashes",
59+
)
60+
61+
def test_the_superseded_flag_is_not_left_behind(self):
62+
"""`--no-emit-project` drops only the root, so alongside `--all-packages` it is a bug."""
63+
self.assertNotIn("--no-emit-project", self.command)
64+
65+
def test_the_command_was_actually_read(self):
66+
"""Non-vacuity guard: a regex that matched nothing would satisfy the negative test above."""
67+
self.assertTrue(self.command.startswith("uv export "), self.command)
68+
self.assertIn("--output-file", self.command, "the export's own contract, not this test's")
69+
70+
71+
class WorkflowPinTest(unittest.TestCase):
72+
"""The comment at the step is the only place a reader learns why the flags are there."""
73+
74+
def test_the_step_explains_the_flags(self):
75+
text = _WORKFLOW.read_text(encoding="utf-8")
76+
for flag in _REQUIRED_FLAGS:
77+
with self.subTest(flag=flag):
78+
# Named in prose somewhere other than the command line itself.
79+
self.assertGreater(
80+
text.count(flag),
81+
1,
82+
f"{flag} appears only in the command; a reader deleting it finds no reason "
83+
"not to",
84+
)
85+
86+
87+
if __name__ == "__main__":
88+
unittest.main()

0 commit comments

Comments
 (0)