Repository navigation
ci + supply chain: security lint, dependency audit, generated SBOM, actions off Node 20, Dependabot, enforced SHA pinning (#71, #100, #131) - #149
Conversation
virtualsteve-star
left a comment
There was a problem hiding this comment.
Automated review of ci/hygiene (→ dev, head dbc1cfd). This one is in good shape — the findings are mostly hardening of new tooling rather than defects in what it claims to do.
Verified working, for the record: all three action SHAs resolve to their claimed tags (actions/checkout v7.0.1, astral-sh/setup-uv v10.0.1, actions/upload-artifact v7.0.1); ruff check --select S plugin/ passes clean on this branch (ruff 0.16.6, run locally); and gen_sbom.py --check passes with the SBOM's 33 components, hashes, purls, direct/transitive marking, dependency edges and both BOM cross-references all matching the lockfile, zero dangling refs. The supply-chain claims this PR makes are true.
The one theme worth acting on: the PR's thesis is "pin everything that can move" — actions to SHAs, deps to hashes — but the two new gating tools float unversioned (uv run --with ruff, --with pip-audit). A required check that resolves whatever is newest will eventually go red on an unrelated PR with no diff to explain it: ruff adds flake8-bandit rules regularly, and the next one that matches existing plugin/ code blocks the queue. Pinning them finishes the job the PR started.
One finding has no anchor — scripts/bump_version.py:86: --dry-run still reports only "would regenerate security/aibom.cdx.json + security/aibom.html", and the closing summary at line 113 still says only "AI BOM regenerated". The real run now rewrites two more files than the dry run admits, which is the one thing dry-run exists to get right.
🤖 Generated with Claude Code
| # ruff's flake8-bandit ruleset (S*) over the payload that lands on a user's machine. Every | ||
| # deliberate exception is a `# noqa: S###` with its reason beside it — the rule is that a | ||
| # swallowed exception in plugin/ is a documented decision, never an accident. (#100) | ||
| run: uv run --with ruff ruff check --select S plugin/ |
There was a problem hiding this comment.
🟠 The required security gate installs its linter unversioned.
uv run --with ruff resolves whatever ruff is newest at run time, and ruff ships new flake8-bandit (S###) rules regularly. The first release carrying a rule that matches existing plugin/ code turns a required check red on a PR that changed nothing related — the worst kind of CI failure to debug, because the diff contains no cause. Same shape for pip-audit on line 80.
It also cuts against this PR's own argument: the actions are SHA-pinned and the dependency tree is hash-pinned specifically so that nothing moves under CI, and then the two tools doing the gating are left floating. --with ruff==0.16.6 (the version I verified clean) plus a Dependabot-managed bump keeps the intent intact.
| # becomes known-vulnerable is a red build, not a surprise. Dependabot alerts cover the same | ||
| # tree asynchronously; this is the synchronous gate. | ||
| run: | | ||
| uv export --script plugin/connector/exabeam-mcp-bridge.py --format requirements-txt --no-hashes -q > /tmp/bridge-requirements.txt |
There was a problem hiding this comment.
🟡 The audit skips platform-gated packages, so the coverage claim overstates.
uv export --script … --format requirements-txt emits environment markers, and pip-audit resolves against the current environment. On ubuntu-latest the sys_platform == 'win32' entries (pywin32, colorama) are filtered out and never audited — so the step's comment ("every locked package checked against the OSV/PyPI advisory databases") and the matching claims in CHANGELOG and security/README.md aren't quite true: a Windows-only CVE in the locked tree would not turn this build red.
Either audit the full lock irrespective of markers, or narrow the claim to "every package resolved for the CI platform".
| lock manifest's, the root references the AI BOM and the AI BOM references it back, and the | ||
| committed copy is not stale (the same --check CI runs).""" | ||
| import subprocess, sys, tomllib | ||
| r = subprocess.run(["uv", "run", str(ROOT / "security" / "gen_sbom.py"), "--check"], capture_output=True, text=True, cwd=ROOT) |
There was a problem hiding this comment.
🟠 This test errors instead of skipping when uv isn't on PATH — and the repo already has the pattern for it.
subprocess.run(["uv", "run", ...]) has no shutil.which("uv") guard and no FileNotFoundError handler, so a contributor without uv on PATH gets a test error, not a skip. tests/test_bridge_static.py:88-103 handles exactly this case correctly (pytest.skip("uv not on PATH") plus the FileNotFoundError catch) — worth matching.
Better still: gen_sbom.py is pure stdlib, so sys.executable removes the uv dependency from this test entirely, or import build_sbom and call it in-process.
| print(" note: security/gen_aibom.py not present — skipped AI BOM regen", file=sys.stderr) | ||
| if GEN_SBOM.exists(): | ||
| # the SBOM's root component carries the version and its serial is version-derived | ||
| subprocess.run(["uv", "run", str(GEN_SBOM)], check=True, cwd=str(ROOT)) |
There was a problem hiding this comment.
🟠 A release can be left half-bumped: this shells out to uv while the line above uses sys.executable.
gen_sbom.py declares no dependencies, so uv buys nothing here except a hard PATH requirement — and the AI BOM regen two lines up already uses sys.executable.
Concrete failure: a maintainer runs python scripts/bump_version.py 0.8.6 without uv on PATH. Line 94 has already rewritten plugin.json, the Codex plugin.json and the README pill; line 99 then raises an uncaught FileNotFoundError with no cleanup, leaving the tree version-bumped with a stale AI BOM and stale SBOM. Use sys.executable for consistency with its neighbour.
|
|
||
| def _direct_specifiers(): | ||
| """{normalized name: specifier} from the bridge's PEP 723 header.""" | ||
| m = re.search(r"^# dependencies = \[(.*?)\]", SCRIPT.read_text(), re.M | re.S) |
There was a problem hiding this comment.
🟠 The PEP 723 dependency regex truncates on the first ], silently corrupting both BOMs.
re.search(r"^# dependencies = \[(.*?)\]", ...) is non-greedy. Add an extras marker to the bridge header — "mcp[cli]>=1.0,<2", an ordinary thing to do — and the captured group truncates to "mcp[cli. _direct_specifiers then reports socxen:specifier = "[cli" for mcp and silently drops the specifiers for every dependency after it.
gen_aibom.py:_pep723 has the same shape, so this would misreport in both artifacts at once, with --check blessing the result. Match to the closing ] of the header line, or parse the PEP 723 block as TOML.
| def main(argv): | ||
| check = "--check" in argv | ||
| bom = build_sbom(_timestamp()) | ||
| if check: |
There was a problem hiding this comment.
🟡 --check never verifies sbom.html, and that's the file the CI artifact publishes.
Reproduced: append a line to security/sbom.html, run --check → exits 0, "SBOM is current". So a change to render_html (or a hand-edit) leaves the committed human-facing render permanently stale while CI stays green — and that stale file is what ships in the socxen-sbom-<sha> artifact.
gen_aibom.py has the same gap, so this is an inherited convention rather than a regression — but this PR doubles the blast radius, which makes it a reasonable moment to close both.
| } | ||
|
|
||
|
|
||
| def _version_of(lock, name): |
There was a problem hiding this comment.
🟡 Latent: _version_of returns the first name match, and this lock already has resolution forks.
The lock carries four resolution-markers forks. A fork that resolves one package to two versions produces two [[package]] entries with the same name, after which every dependency edge and the root's dependsOn silently point at whichever came first — a wrong-but-plausible SBOM that neither --check nor the new test would catch. The "unknown" fallback likewise emits a dangling pkg:pypi/x@unknown bom-ref rather than failing.
No duplicate names today, so this is latent rather than broken — but the failure mode is a compliance artifact that is confidently incorrect, which is worth an explicit error instead of a silent first-match.
| JSON_OUT.write_text(_dumps(bom)) | ||
| HTML_OUT.write_text(render_html(bom)) | ||
| print(f"wrote {JSON_OUT.relative_to(ROOT)} and {HTML_OUT.relative_to(ROOT)} " | ||
| f"({len(bom['components'])} locked packages, {bom['metadata']['properties'][5]['value']} direct)") |
There was a problem hiding this comment.
🟡 The summary reads a metadata property by hard index.
bom['metadata']['properties'][5]['value'] — all seven properties are defined in one literal 30 lines above, so adding or reordering any of them makes this print a different field (e.g. lockRevision) labelled "N direct". Look it up by name.
| m = re.search(r"^# dependencies = \[(.*?)\]", SCRIPT.read_text(), re.M | re.S) | ||
| out = {} | ||
| for spec in re.findall(r'"([^"]+)"', m.group(1) if m else ""): | ||
| name = re.split(r"[<>=!~\[; ]", spec, 1)[0] |
There was a problem hiding this comment.
🟡 re.split(..., 1) passes maxsplit positionally — DeprecationWarning on 3.13, slated for removal.
Reproduced locally on 3.13.2. The script declares requires-python = ">=3.11", so uv is free to run it on an interpreter where this becomes an error. maxsplit=1.
| if h.startswith("sha256:"): | ||
| hashes.append({"alg": "SHA-256", "content": h.split(":", 1)[1]}) | ||
| if art.get("url"): | ||
| dists.append(art["url"]) |
There was a problem hiding this comment.
🟡 dists is built and never read.
Every artifact's url is collected into dists, then discarded — the emitted externalReferences carry only the registry index URL. Per-artifact distribution URLs are arguably the most useful provenance the lock holds after the hashes, so either emit them or drop the variable.
|
Review findings addressed in the commit above. ruff and pip-audit are pinned (0.16.6 / 2.10.1) with a note that the pin is bumped by hand; the audit claim is narrowed to what it covers (Linux-resolved packages; Windows-only entries are Dependabot's) in the workflow, |
mattwillems-exabeam
left a comment
There was a problem hiding this comment.
Approve. The failure mode for a security-lint PR is a lint that passes because everything is suppressed. Checked that first.
The lint passes honestly
- No ruff config anywhere — no
pyproject.toml[tool.ruff], noruff.toml. Nothing is globally ignored, so--select Soverplugin/passes on merit rather than by configuration. - One suppression in the whole payload, and it carries its reason:
observra_logging.py:213— "exit must never error; a failed flush is lost telemetry, not a failed run." That's a real justification for a broad catch on a shutdown path, not a blanket silence. - The other bare-
passhandlers inplugin/are narrowexcept OSErroron cleanup paths, which is the shape you want.
Pinning ruff==0.16.6 is the right call, and the reasoning in the comment is the part worth keeping — "a floating linter turns a required check red on an unrelated PR the day a new S-rule ships." Same discipline as the action SHAs in #130. Worth noting the tradeoff it accepts: Dependabot can't see a --with pin, so this one is bumped by hand or not at all. The comment says so, which is enough.
The advisory/required split is right too. scripts/, security/ and evals/ don't ship (#66), so gating on them would make an unrelated build red for code no user runs, while still surfacing the signal via --statistics.
Also verified
gen_sbom.py --check→ current, 33 locked packagesgen_aibom.py --check→ current- 444 passed, 3 skipped
- This PR's own CI is green on the new gates — Security lint + dependency audit passes, which is the authoritative run of
pip-audit --strictagainst the exported lockfile
Closing three tracker issues in one PR with no product behavior change and everything verifiable in the PR's own checks is the right shape for this kind of work.
… off Node 20; Dependabot and enforced SHA pinning (#71, #100, #131) - ruff --select S over plugin/ is a required check (the one S110 on dev — the telemetry exit flush — now carries its reason); the same lint over scripts/, security/, evals/ is advisory. - pip-audit over the bridge's hash-pinned lockfile (uv export --script -> requirements) on every push: #71 rung 3. Clean today. - actions/checkout v4.4.0 -> v7.0.1 and astral-sh/setup-uv v5.4.2 -> v10.0.1 (both were on the deprecated Node 20 runtime; setup-uv is used with no inputs, so the v6 breaking changes do not apply). - .github/dependabot.yml (github-actions, weekly) keeps the SHA pins and their version comments current; the repository setting sha_pinning_required is now ON (flipped 2026-09-05). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Q9UohWfy2scPhr4pavhqd # Conflicts: # .github/workflows/ci.yml
…ion bumps (#71, #100, #131) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Q9UohWfy2scPhr4pavhqd
…, rebuilt and published by CI, regenerated on release security/gen_sbom.py reads plugin/connector/exabeam-mcp-bridge.py.lock and nothing else, and emits a CycloneDX 1.6 SBOM: one library component per locked package (33 today, 5 direct) with version, purl, the SHA-256 of every locked sdist/wheel, direct/transitive marking, the full dependency graph, and the lock's own facts; sbom.html is the human render, sharing the AI BOM's stylesheet. The AI BOM and the SBOM now reference each other (externalReferences type "bom") and share the root bom-ref. Nothing is hand-maintained: CI runs `--check` on every PR (invariants job), rebuilds the SBOM and publishes it as a workflow artifact on every run (security job, beside the pip-audit of the same tree), and scripts/bump_version.py regenerates it with the AI BOM on a release. Documented in security/README.md (contents + "The SBOM"), CONTRIBUTING (lock changes regenerate both BOMs), the repository README's evidence row, and pinned by a repo invariant that the SBOM mirrors the lock. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Q9UohWfy2scPhr4pavhqd
…ecked; PEP 723 parsed as TOML; no first-match versions - ruff and pip-audit are version-pinned in the workflow (0.16.6 / 2.10.1, the versions verified clean), like the actions and the lockfile: a floating linter reddens a required check on an unrelated PR the day a new S-rule ships. The audit's claim is narrowed to what it covers — every locked package that resolves on the Linux runner; the lock's two Windows-only entries are Dependabot's. - --check verifies aibom.html and sbom.html too (rendered with the committed timestamp, so only content drift fails); before, a stale render shipped as the CI artifact while CI stayed green. - The PEP 723 dependency list is parsed as TOML (regex fallback matches the ] that ends the line): an extras marker like "mcp[cli]" truncated the old non-greedy match and silently dropped every later dependency from both BOMs. - The SBOM refuses a lock that resolves one name to two versions or names a dependency it does not lock, instead of first-match / "unknown"; the summary reads properties by name; maxsplit is a keyword; the unused per-artifact URL list is gone. - bump_version.py and the invariants test run gen_sbom.py with sys.executable (stdlib script), so a maintainer without uv on PATH gets a complete bump and a skip-free test; the dry run and the closing summary name the SBOM files the real run rewrites. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Q9UohWfy2scPhr4pavhqd
…identity.json and both BOMs Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Q9UohWfy2scPhr4pavhqd
b36758f to
bf0db90
Compare
Closes #71, closes #100, closes #131 — the CI-hygiene set from the August tracker sweep, in one PR. No product behavior changes; everything is verifiable in this PR's own checks.
What changes
ruff --select S(flake8-bandit) overplugin/as a required check. Each deliberatetry/except/passin the shipped code carries# noqa: S110and its reason — a swallowed exception in the payload is a documented decision, never an accident. Same lint overscripts/,security/,evals/is advisory (--exit-zero, statistics only; they never ship, #66).pip-audit --strictover the bridge's hash-pinned lockfile (uv export --script→ requirements) on every push. Clean today. Dependabot security alerts cover the same tree asynchronously; this is the synchronous gate.actions/checkoutv4.4.0 → v7.0.1 (3d3c42e5…),astral-sh/setup-uvv5.4.2 → v10.0.1 (20cfd1bf…), in all three workflows that use them.setup-uvis invoked with no inputs, so the v6 breaking changes (activate-environment,working-directory, cache glob) do not apply — this PR's green run is the proof..github/dependabot.yml,github-actionsecosystem, weekly; Dependabot rewrites SHA pins and their# vX.Y.Zcomments together.Why now
The 2026-09-05 Praxen scan held Manage Your Supply Chain at 3 with the rationale "no dependency or CVE scanning of any kind" and Build an AI Red Team at 4 for "no in-repo enforcement." These are the two cheapest movements on those rationales before the dev RC re-scan. Honest scope note:
ruff -Sscans our own code (#100); the CVE scanning Praxen names is thepip-auditstep (#71).Interaction with #148
#148 adds
plugin/hooks/gate.py, which has two deliberate exception-swallows of its own; they now carry their# noqa: S110reason on that branch (pushed), so the required lint is green in either merge order.Also in this PR: a generated SBOM (follow-on Steve asked for)
security/gen_sbom.pyreads the bridge's uv lockfile — and nothing else — and emits a CycloneDX 1.6 SBOM (security/sbom.cdx.json+sbom.html): 33 locked packages (5 direct), each with version, purl, the SHA-256 of every locked artifact, direct/transitive marking, and the full dependency graph; the AI BOM and the SBOM cross-reference each other. It keeps itself current without anyone editing it:--checkin the invariants job fails on drift; the security job rebuilds it on every run and publishes it as thesocxen-sbom-<sha>workflow artifact beside thepip-auditof the same tree;bump_version.pyregenerates it on a release; and a repo invariant asserts the SBOM mirrors the lock. Documented insecurity/README.md→ The SBOM, CONTRIBUTING, and the README evidence row.Not in this PR
A CodeQL schedule — worth considering once this lint has a baseline, not before (#100's own advice). Lockfiles for the non-shipped PEP 723 entrypoints (#71's optional tail).
🤖 Generated with Claude Code
https://claude.ai/code/session_016Q9UohWfy2scPhr4pavhqd