The last 3 CVE alerts close, and a hidden click vulnerability stops hiding (WOR-852) - #473
Merged
Merged
Conversation
…-852) typer 0.26 dropped its dependency on click and vendored its own copy. Three test-harness consequences, none user-visible: - typer.testing.CliRunner no longer accepts mix_stderr (32 files) - Result.output is now stdout+stderr mixed; under click 8.1 with mix_stderr=False it meant stdout-only (3 assertions re-pointed) - typer.main.get_command() returns TyperGroup, not click.Group (test_skill_md.py fixture + option introspection) Measured the real CLI on typer 0.23.1 / 0.26.8 / 0.27.0: `worthless status` and `worthless up -d` write to identical streams on all three. The failures were harness stream attribution only, and typer's new attribution matches what the real binary does. The migration cannot be split from the bump: removing mix_stderr=False while still on typer 0.23.1 flips those tests to mixed-stream and breaks them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… (WOR-852)
semgrep is CI-only SAST invoked in exactly one place, but it was declared in
the `qa` extra, so its own pins anchored the whole lockfile:
click~=8.1.8 -> held click below the PYSEC-2026-2132 fix (CVE-2026-7246,
click.edit() command injection, fixed in 8.3.3)
mcp==1.23.3 -> held mcp below dependabot alerts 66/67/68
uv's resolver stated it outright when asked for mcp==1.28.1: "we can conclude
that semgrep>=1.155.0 cannot be used ... your project's requirements are
unsatisfiable."
Running it as `uvx semgrep@1.172.0` in sast.yml keeps identical coverage
(same configs, same .semgrep/ rules, same SARIF upload) while removing it and
its transitive pins from uv.lock entirely.
Resulting lock movement:
click 8.1.8 -> 8.4.2 (closes PYSEC-2026-2132)
mcp 1.23.3 -> 1.29.0 (closes alerts 66, 67, 68)
mcp gains a `<2` ceiling (WOR-864): 2.0.0 renamed mcp.server.fastmcp ->
mcp.server.mcpserver with no shim, which breaks src/worthless/mcp/server.py.
The floor stays at 1.0 so users already resolved on an older 1.x are untouched.
The PYSEC-2026-2132 suppression is dropped from the uv-audit hook; pip-audit
reports no known vulnerabilities without it. The unrelated pip ignore
(GHSA-58qw-9mgm-455v, still unfixed upstream) stays.
beads worthless-fnvz predicted this needed a test migration first and measured
7 failures on click 8.3.3 in July. Those were against typer 0.23.1; on the
typer 0.26.8 migration in the parent commit all 7 pass. Full suite green:
3966 passed, 57 skipped, 8 xfailed, 0 failed.
Verified: uvx semgrep@1.172.0 scans 121 files / 6 rules and emits SARIF;
worthless.mcp.server imports on mcp 1.29.0; pre-push pyright, vulture, xenon,
pip-licenses and pip-audit all pass; zizmor passes --all-files on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change updates CI security tooling, dependency constraints and pins, Grype ignore enforcement, and CLI tests for current Click and Typer stream and command-inspection behavior. ChangesCLI compatibility and security tooling
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/sast.yml:
- Line 71: Update the Semgrep scan command in the workflow to include the
--error option, ensuring findings produce a non-zero exit status and fail the CI
job while preserving SARIF generation and upload.
In `@tests/cli/test_up_with_sidecar.py`:
- Around line 474-475: Update the test assertion around result.output and
result.exception so user-facing daemon, foreground, and WRTLS-115 strings are
validated only against result.output, without concatenating exception text into
out. Assert the expected exception type or code separately using the available
Click/Typer result attributes.
In `@tests/test_cli_lock_scan_prompt.py`:
- Around line 38-40: Update the interactive prompt assertion in the test using
CliRunner to inspect result.stdout instead of result.output, ensuring it
verifies stdout specifically and cannot pass due to unrelated stderr output.
In `@tests/test_relock_missing_features.py`:
- Around line 35-36: Correct the comment above the CliRunner setup to state that
Click >=8.2 exposes mixed terminal output through result.output, while
result.stdout and result.stderr provide channel-specific assertions; do not
change the runner behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b4ffc67-22a8-4050-b935-dce58505dd39
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.github/workflows/sast.yml.pre-commit-config.yamlpyproject.tomlrequirements.txttests/cli/doctor/test_json_output.pytests/cli/doctor/test_remediation.pytests/cli/test_confidence_verdict.pytests/cli/test_service_cli.pytests/cli/test_start_supervised_proxy_integration.pytests/cli/test_up_with_sidecar.pytests/cli/test_verify_command.pytests/live/test_e2e_live.pytests/test_blackbox.pytests/test_cli_default.pytests/test_cli_lock_failclosed.pytests/test_cli_lock_scan_prompt.pytests/test_cli_notice.pytests/test_cli_scan.pytests/test_cli_scan_code.pytests/test_cli_scan_failclosed.pytests/test_cli_security_hardening.pytests/test_cli_status.pytests/test_cli_user_experience.pytests/test_doctor_fix_behavior.pytests/test_doctor_home_checks.pytests/test_doctor_keychain_migration.pytests/test_doctor_uninstall_diagnosis.pytests/test_e2e.pytests/test_openclaw_lock_signature.pytests/test_relock_functional.pytests/test_relock_missing_features.pytests/test_skill_md.pytests/test_unsafe_rewrite_ux.pytests/test_wor277_no_plaintext_leaks.pytests/test_wor655_no_secret_leaks.pytests/user_flows/test_doctor_dogfood.pytests/user_flows/test_native_cli_journeys.pytests/user_flows/test_native_stress_journeys.pytests/user_flows/test_recovery_journeys.py
…(WOR-852) Three findings, one of them a real supply-chain exposure. codeql-action was pinned to a commit outside its own repo --------------------------------------------------------- All three upload-sarif steps pinned github/codeql-action@3b1a19a. That SHA does not exist in github/codeql-action — the commits API returns 422 "No commit found for SHA". GitHub Actions still resolves it because it resolves across a repo's fork network; the API does not. That is the impostor-commit pattern, and it defeats the entire point of pinning: the code being run is not from the upstream repo's history. Repinned to a2983b8bed1923f44751c5c43237f479442827b3, which is what the official v3 tag dereferences to and is verified present in the repo (2026-07-30, "Merge pull request #4058 from github/backport-v3.3"). Also covers scorecard.yml, which carried the same SHA. This was masked by a corrupted local zizmor cache: with the bad cache the impostor-commit audit errored out ("io error: unexpected end of file") instead of reporting. Clearing ~/.cache/zizmor makes zizmor pass and surfaced the finding. Worth knowing — a cache that fails open hides findings. .grype.yaml — the two CPython tarfile CVEs were missing ------------------------------------------------------ CVE-2026-11940 (extractall() filter bypass) and CVE-2026-11972 (streaming-mode EOF -> exponential parse) are High with no ignore entry, so `scan` failed on main and on every branch. Grype's only fix for both is 3.15.0b4, a pre-release. Verified unreachable rather than asserted: src/worthless has zero imports of tarfile, zero uses of shutil.unpack_archive, zero uses of zipfile, and both CVEs require calling tarfile on an attacker-supplied archive. Time-boxed to 2026-08-31 against WOR-852. Grype on the image now exits 0. A --no-cache rebuild also confirms liblzma5 resolves to the patched 5.4.1-1+deb12u1 via the existing apt-get upgrade; the earlier vulnerable reading was a local layer-cache artifact. sast.yml — semgrep could not fail its own job --------------------------------------------- `semgrep scan` exits 0 even when it finds things unless --error is passed, so the SARIF upload was the only signal. Measured before enabling: 1567 rules, 0 findings. tests — the typer migration silently weakened 61 assertions ----------------------------------------------------------- Dropping CliRunner(mix_stderr=False) changed result.output from "stdout only" to "stdout+stderr mixed", so assertions carried over verbatim stopped proving which stream the text reached. Classified every affected site empirically (rewrite -> run -> restore) rather than by inspection: 61 of 67 sites across 12 files pass unchanged against result.stdout, so 39 assertion conditions are tightened back to stdout-only. Failure *messages* keep the mixed stream — 11 were reverted after the first pass narrowed them, since a diagnostic should show more, not less. Two needed judgement: - test_confidence_verdict.py: the verdict renders through Rich Console to stderr, so the haystack is now `result.stderr + result.stdout` explicitly, matching the rest of that file. - test_up_with_sidecar.py: the assertion folded str(result.exception) into the haystack, so it could pass on text that only existed in a traceback and never reached the terminal. Now asserts what the user saw. Corrects a comment in test_relock_missing_features.py that stated the click >=8.2 stream contract backwards. Full suite: 3966 passed, 57 skipped, 8 xfailed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.grype.yaml:
- Around line 46-62: Add a CI check that parses the vulnerability ignore entries
in .grype.yaml and fails when any rule’s expiry date has passed, including
CVE-2026-11940 and CVE-2026-11972. Keep the expiry metadata for the CI check,
but do not depend on Grype to enforce or remove expired rules; integrate the
validation into the existing CI workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 66248bb4-3fbf-4c43-8ce7-b80b0c5c57ca
📒 Files selected for processing (16)
.github/workflows/sast.yml.github/workflows/scorecard.yml.grype.yamltests/cli/doctor/test_json_output.pytests/cli/doctor/test_remediation.pytests/cli/test_confidence_verdict.pytests/cli/test_up_with_sidecar.pytests/test_cli_default.pytests/test_cli_lock_scan_prompt.pytests/test_doctor_fix_behavior.pytests/test_doctor_home_checks.pytests/test_doctor_keychain_migration.pytests/test_doctor_uninstall_diagnosis.pytests/test_relock_functional.pytests/test_relock_missing_features.pytests/test_wor655_no_secret_leaks.py
🚧 Files skipped from review as they are similar to previous changes (12)
- tests/cli/doctor/test_json_output.py
- .github/workflows/sast.yml
- tests/test_doctor_keychain_migration.py
- tests/test_relock_functional.py
- tests/cli/test_up_with_sidecar.py
- tests/cli/doctor/test_remediation.py
- tests/cli/test_confidence_verdict.py
- tests/test_relock_missing_features.py
- tests/test_wor655_no_secret_leaks.py
- tests/test_doctor_uninstall_diagnosis.py
- tests/test_doctor_home_checks.py
- tests/test_cli_lock_scan_prompt.py
CodeRabbit was right, and it is worse than one PR's worth. Grype has no `expiry` field. Its IgnoreRule schema drops unknown keys silently, so a suppression carrying an expiry date suppresses forever. Measured against grype 0.114.0 on the real image: rewriting every expiry to 2020-01-01 — five years past — changed nothing. CVE-2026-11940 and CVE-2026-11972 stayed suppressed and grype still exited 0. That means the whole time-boxing premise was fiction, including the pre-existing CVE-2026-15308 entry from WOR-691. .grype.yaml's header actively claimed "past this point, Grype auto-re-fires the finding". It does not. A security suppression that silently outlives its own justification is the failure mode the file was written to prevent. scripts/hooks/check_grype_ignore_expiry.py is now the enforcement: - fails on any ignore past its expiry, naming the CVE and the days elapsed - fails on an ignore with no expiry at all (undated == never revisited) - fails on a date it cannot read rather than skipping it - handles both YAML forms: quoted strings and bare dates, which PyYAML hands back as str and datetime.date respectively Wired into pre-commit as "supply chain: grype ignores are dated and current", always_run so a stale ignore cannot slip through on an unrelated commit. The .grype.yaml header is corrected to state that grype ignores the field and that the hook is the only thing enforcing it. tests/test_grype_ignore_expiry.py covers current, expired, undated, malformed, the inclusive today-boundary, both YAML date forms, and asserts the shipped config itself satisfies the contract. 9 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y note (WOR-852)
Both from the adversarial pass on this PR.
The expiry gate only ran on a developer laptop
----------------------------------------------
scripts/hooks/check_grype_ignore_expiry.py was wired into pre-commit only.
That is bypassed by Dependabot, GitHub web edits, `--no-verify`, and any clone
that never ran `pre-commit install`. Since grype silently drops the unknown
`expiry` key, a lapsed suppression would have stayed effective everywhere
except the one machine that happened to run the hook.
It now also runs in docker-security.yml, immediately before the scan it is
protecting. Invoked as `uv run --with pyyaml` because the script needs PyYAML
and a bare runner python3 does not have it; the uv action is the same pinned
one this workflow already uses further down.
The tarfile reachability argument was true but incomplete
----------------------------------------------------------
It claimed "zero imports of tarfile in src/worthless" and stopped there. That
covers first-party code only, which is not good enough to justify suppressing
two High CVEs. Re-verified across installed dependencies:
- pygments is the only RUNTIME dependency importing tarfile, in
lexers/_php_builtins.py — a maintenance script for regenerating lexer
data. Confirmed not loaded after importing worthless.proxy + worthless.cli.
- backports.tarfile IS resident at runtime via keyring -> jaraco-context.
It is a separate PyPI package rather than the CPython stdlib module these
CVEs are filed against, and the ignore is scoped to
`package: {name: python, type: binary}`, so it cannot mask a finding filed
against backports-tarfile itself. Worthless never calls the
jaraco.context tarball helper that would exercise it.
The note in .grype.yaml now states all of that instead of the src/-only claim,
so the next person re-measuring has the real picture.
Verified: actionlint passes, the exact CI invocation exits 0 locally, grype
still reports 0 High on the image, expiry tests 9 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…WOR-852)
Two findings from the review panel on this PR. The first was a genuine hole in
the gate added one commit earlier.
The gate only guarded ONE of the two configs grype reads
---------------------------------------------------------
`grype config locations` (grype 0.114.0) reports that grype auto-discovers
BOTH `.grype.yaml` and `.grype/config.yaml` from the repo root, and
anchore/scan-action passes no explicit --config, so both are live in CI. The
hook hardcoded `.grype.yaml`.
Reproduced before fixing: planting
.grype/config.yaml:
ignore:
- vulnerability: CVE-2026-11940 # no expiry at all
left the hook exiting 0. A permanent, undated, invisible suppression — the
exact failure mode the hook exists to prevent, one directory over.
`check_all()` now walks every discovered config, and reports which file a
problem came from. Re-running the same reproduction now fails with
".grype/config.yaml: CVE-2026-11940: no `expiry`".
Two smaller holes closed at the same time:
- a rule with no `vulnerability` id is rejected outright. It matches on
package/type alone, silencing a whole class of findings, and no expiry
date can make that safe.
- a non-mapping list entry is reported instead of raising AttributeError.
The freshness assertion was a suite-wide time bomb
---------------------------------------------------
`test_the_real_repo_config_is_current` asserted the live config against
`date.today()`. The nearest expiry (CVE-2026-15308, from WOR-691) is
2026-08-10 — eight days out. On 2026-08-11 that would have turned the ENTIRE
unit suite red for every developer and every CI job, on completely unrelated
work.
The forcing function belongs in the security gate, not in `pytest`. The tests
now assert the config's *shape* — every entry named and dated — using a fixed
date, while freshness stays with the hook, which runs at commit time and again
in docker-security.yml immediately before the scan.
test_hook_would_fail_the_repo_config_once_an_expiry_lapses proves the forcing
function still fires: it derives the earliest expiry from the shipped config
and asserts the hook passes on that day and fails the day after. Verified —
the hook still FAILS on 2026-08-11 while the suite stays green.
15 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…expiry gate (WOR-852) Review findings on the gate added earlier in this PR. No behaviour change — the blind-spot reproduction still fails the hook (exit 1) and the shipped config still passes (exit 0). check() carried its own `if not config.exists()` guard, but check_all() filters non-existent paths before calling it, so that branch was unreachable from main() and untested. Worse, check_all()'s docstring claimed a missing primary config "is reported by check() itself" — it is not. Deleting .grype.yaml while keeping .grype/config.yaml passed silently, contradicting the stated invariant. The guard is gone and the docstring now states what actually holds: either location alone is legitimate because grype reads both, and the real invariant is that at least one must exist. A repo keeping all its ignores in .grype/config.yaml is fine; a repo with neither has no policy to enforce. test_every_real_grype_config_location_is_structurally_sound reduced to exactly test_the_real_repo_config_is_structurally_sound, since .grype/config.yaml does not exist here — both collapsed into one. The removed test_hook_would_fail_the_repo_config_once_an_expiry_lapses re-derived the date parser inside the test to prove boundary semantics that test_expiry_boundary_is_inclusive already proves synthetically, and min()'d an unguarded list that would raise ValueError the day someone empties `ignore:`. Replaced by test_missing_file_is_not_check_s_job, which pins the invariant that actually moved. The CI step gains --no-project: it parses ~50 lines of YAML and had no reason to sync the whole project venv first. 14 passed. Verified after the refactor: the undated-ignore reproduction at .grype/config.yaml still exits 1, the exact CI invocation exits 0, actionlint passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/hooks/check_grype_ignore_expiry.py`:
- Around line 44-51: Validate the YAML document and the `ignore` value before
calling `data.get()` or iterating in the hook’s rule-processing function: return
a diagnostic when the document is not a mapping, and when `ignore` is present
but is not a list. Preserve the existing empty-list behavior and diagnostic
formatting, and add regression tests covering a scalar/non-empty-list document
and a truthy non-list `ignore` value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8dfd7c23-87e6-4d6f-a029-abbed9f64a96
📒 Files selected for processing (3)
.github/workflows/docker-security.ymlscripts/hooks/check_grype_ignore_expiry.pytests/test_grype_ignore_expiry.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/docker-security.yml
…(WOR-852) Found by the end-to-end validation pass on this PR. `--ignore-vuln GHSA-58qw-9mgm-455v` (pip tarball handling, beads worthless-lwvs) was still on the uv-audit hook. Verified 2026-08-02: pip is still in the exported runtime set, and pip-audit reports "No known vulnerabilities found" with NO ignore flags at all. The suppression had outlived whatever it was suppressing. A dead ignore is worse than no ignore. It sits there silently absorbing the NEXT finding for that package, and nothing about it looks wrong on inspection — which is precisely the failure mode the rest of this PR exists to close. Leaving it while adding a hook that enforces expiry on grype ignores would have been inconsistent. Both suppressions are now gone and the hook runs clean with none: uv export --frozen --no-emit-project | uv run pip-audit --disable-pip -r /dev/stdin No known vulnerabilities found The comment block now records why each was dropped and states that any future ignore must cite a ticket and be re-checked quarterly. beads worthless-lwvs stays open — reopen the ignore only if it genuinely re-fires. Also corrected two PR-body inaccuracies the same pass caught: the "How" section claimed sast.yml keeps the "same flags" when `--error` was added, and the headline "full suite" figure omitted that the default addopts excludes the live, docker, user_flow and real_ipc tiers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Spec-vs-implementation review caught a hole in the blind-spot fix added
earlier in this PR.
`test_secondary_config_location_is_checked` proves an undated ignore in
.grype/config.yaml is caught — but it hands check_all() its own explicit
tuple. The only test that referenced the real CONFIGS constant passed
vacuously, because .grype/config.yaml does not exist in this repo.
Net effect: someone could delete the second entry from CONFIGS and the entire
suite would still pass, silently reopening the exact blind spot the previous
commit closed.
test_configs_covers_every_location_grype_reads asserts the constant itself
contains both paths. Verified by mutation: with the second entry removed,
CONFIGS reduces to {".grype.yaml"} and the assertion fails.
15 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…WOR-852)
Caught by the final reality-check pass. This one mattered most and I missed it.
.github/workflows/pre-release.yml:65 still ran
uv run pip-audit --ignore-vuln GHSA-58qw-9mgm-455v --ignore-vuln PYSEC-2026-2132
after the earlier commit removed both from the uv-audit pre-commit hook. The
file was not in this PR's diff at all, so nothing flagged the drift — while
its own comment declared the list "MUST stay in sync with the uv-audit hook in
.pre-commit-config.yaml". This PR broke that sync and left the comment
asserting it.
That made two claims in the PR body false at the gate that guards a SHIPPED
artifact: "pip-audit clean with nothing ignored" and "the vulnerability scan
tells the truth again" were true for pre-commit and pre-push, but not for
release. And PYSEC-2026-2132 was by then a dead ignore sitting on the release
path — if click ever resolved back under 8.3.3, pre-release would have stayed
green. Precisely the anti-pattern the comment added one commit earlier calls
out: "a dead ignore is worse than none — it silently absorbs the NEXT finding
for that package."
Verified at this workflow's own scope before stripping the flags, which is
wider than the pre-commit hook's (full dev env including the qa extra, versus
the exported runtime set):
uv sync --extra qa && uv run pip-audit
No known vulnerabilities found
The replacement comment records why each ID was dropped, notes that a dead
ignore on a release build is the most expensive kind, and requires any future
ignore to be added to both gates at once so they cannot drift apart again.
Also corrected the PR body line that still claimed the pip ignore "stays".
actionlint passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
Feature status
The feature in one sentenceA maintainer runs the vulnerability scan and gets the truth: every remaining advisory is real, every suppression is named, dated, and expires. Before this, three high-severity advisories were impossible to fix, one vulnerability was hidden behind a flag, and the expiry dates on every suppression did nothing at all. Real before/after# BEFORE — the fix the advisories require is unreachable
$ uv lock --upgrade-package "mcp==1.28.1"
we can conclude that semgrep>=1.155.0 cannot be used.
And because worthless[qa] depends on semgrep>=1.155.0 and your project
requires worthless[qa], we can conclude that your project's requirements
are unsatisfiable.# AFTER — semgrep no longer anchors the lockfile
$ grep -A1 '^name = "mcp"$' uv.lock | grep version
version = "1.29.0"
$ uv export --frozen --no-emit-project | uv run pip-audit --disable-pip -r /dev/stdin
No known vulnerabilities found# BEFORE — a suppression dated five years ago still suppressed
$ sed -i 's/2026-08-31/2020-01-01/' .grype.yaml && grype <image> --fail-on high
CVE-2026-11940 suppressed
CVE-2026-11972 suppressed
exit 0
# AFTER — the gate catches it
CVE-2026-11940: expired 2405 day(s) ago on 2020-01-01. Re-measure the
upstream fix status, then either drop the ignore or extend it with a
fresh reason — do not extend it blind.Why this is harder than it sounds
Where we areThe deviation is real and worth naming. The ask was to close a dependency backlog. Five of the ten commits fix security controls that were not working — found by review, not by the original brief. That expansion was approved as it happened, not assumed.
Tests — proof of what?
User storiesNow — a maintainer runs the scan and every result is trustworthy: no hidden suppressions, no fake expiry dates, no pin resolving to code outside the upstream repo. Unlocked the moment this merges. Next milestone — the container itself gets clean. Six Python CVEs remain whose only fix is a pre-release interpreter; the gate is moving off end-of-life Debian 12. Unlocked when a stable backport ships or the base image moves. Full launch — the scan runs on a schedule against the main branch, so a newly-published advisory surfaces on its own instead of on the next pull request. Gated on adding a scheduled trigger — the workflow currently has none. Risk dial
Where this fits in the productThis is the dependency-hygiene slice of the security milestone. Four sibling pull requests already merged and closed ten of the thirteen advisories across the docs site and the edge worker; this one closes the remaining three and is the only one touching the Python runtime. When it lands, the whole advisory backlog is at zero and the scan gates — commit, push, and release — all report honestly for the first time. What's left — and who's holding it
Step 1 gates everything. Nothing merges without an explicit go, and guessing that would mean shipping a security change nobody signed off on. Deliberately out of scope: the six container CVEs needing a pre-release interpreter, and adding a scheduled trigger to the container scan. Both are tracked separately. What I need from youApproval to merge. All gates are green and the review findings are closed. The one deviation — scope growing from closing advisories to repairing the gates that concealed them — was approved as it happened, so there is no silent decision waiting on you. WOR-852 · PR #473 Posted by Claude — /pr-feature-status |
|
This was referenced Aug 2, 2026
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
1 participant
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.
A scanner you only run in CI was holding your lockfile hostage.
TL;DR
Moving
semgrepout of the project's dependencies closes the last 3 CVE alerts and a 4th that was being silently suppressed.Features
worthless mcpworthless mcpprotected from a breaking release<2--ignore-vulnBefore / After
BEFORE — asking for the version the advisories require:
AFTER — same request, after semgrep leaves the lock:
Summary
semgrepis CI-only SAST, invoked in exactly one place, but it was declared in theqaextra. That put its own pins inside the resolution graph, whereclick~=8.1.8andmcp==1.23.3anchored the entire lockfile. Three high dependabot alerts onmcpwere unfixable, and a real click command-injection CVE sat suppressed behind an--ignore-vulnflag.Running semgrep as a pinned throwaway tool instead of a dependency removes those pins. Both packages immediately reach their fixed versions, and the suppression can be deleted.
Setup
uv.lockheldmcpat 1.23.3 andclickat 8.1.8. Dependabot alerts 66, 67 and 68 had no possible fix, andPYSEC-2026-2132(CVE-2026-7246, command injection inclick.edit(), fixed in click 8.3.3) was being hidden frompip-auditby an explicit ignore. beadsworthless-fnvzdiagnosed the cause in July and parked it, because clearing it needed a test migration nobody had done.What
mcpadvisories close.worthless mcpwill not break whenmcp2.0.0 gets resolved, because it is now capped below it.Why
Two of these were invisible. The click CVE only shows up in
pip-audit, never in dependabot, because its advisory has no GitHub counterpart — so it could sit suppressed indefinitely. And themcpalerts looked permanently unfixable, which is the kind of thing that trains people to ignore the alert list.How
pyproject.toml—semgrep>=1.155.0removed from theqaextra;mcpgains a<2ceiling (WOR-864). The mcp floor stays at1.0so anyone already resolved on an older 1.x is untouched..github/workflows/sast.yml—uv sync --extra qa+uv run semgrepbecomesuvx semgrep@1.172.0. Same four--configflags and the same SARIF upload;--erroris added, so findings now fail the job instead of only being uploaded..github/workflows/sast.ymland.github/workflows/scorecard.yml— all threegithub/codeql-action/upload-sarifsteps repinned from3b1a19a80ab047f35cbb237b5bd9bdc1e14f166ctoa2983b8bed1923f44751c5c43237f479442827b3. The old SHA does not exist ingithub/codeql-action— the commits API returns422 No commit found for SHA. Actions still resolved it because it resolves across a repo's fork network; the API does not. That is the impostor-commit pattern, and it defeats the point of pinning: the code being run was not from upstream's history. The new SHA is what the officialv3tag dereferences to and comparesidenticalto it. All 28 SHA pins in.github/were then re-checked; no second impostor..pre-commit-config.yaml—--ignore-vuln PYSEC-2026-2132deleted from theuv-audithook. Both former pip-audit ignores are now gone —PYSEC-2026-2132andGHSA-58qw-9mgm-455v— from theuv-auditpre-commit hook and frompre-release.yml, the gate guarding a shipped artifact. Verified clean at both scopes with nothing ignored.CliRunner(mix_stderr=...), so click cannot move without it.Follow-ups
worthless-fnvzcan be closed — its suppression is gone and its stated precondition is met.mcp<2). If that ticket is owned elsewhere, reconcile before merging.zizmor --all-filesfailure seen earlier in the session. It passes 2/2 here and 3/3 on the changed file, and main passes, but I could not separate "rebase fixed it" from "cache warmed". Flagging rather than calling it solved.Tests
uvx semgrep@1.172.0— scan completed, 6 custom rules over 121 files, SARIF emitted, version confirmed 1.172.0.worthless.mcp.serverimports cleanly on mcp 1.29.0.pip-auditwith the click suppression removed —No known vulnerabilities found.beads
worthless-fnvzmeasured 7 failures when it tried click 8.3.3 in July, against typer 0.23.1. On the typer 0.26.8 migration in the parent commit, all 7 pass.What this does NOT do
It does not make the container CVE-free. The image still ships Python 3.13.14 carrying
CVE-2026-11940andCVE-2026-11972(High) plusCVE-2025-15366,CVE-2025-15367,CVE-2026-12003andCVE-2026-0864(Medium). Every one has a single available fix: a 3.15 pre-release.Only the two Highs are suppressed in
.grype.yaml, each with a written reachability argument and a hard expiry. The four Mediums are neither suppressed nor argued — they simply fall belowseverity-cutoff: highindocker-security.yml. Nothing time-boxes them and nothing will re-raise them. That gap is real, predates this PR, and is not closed here.Measured across
3.13-slim-bookworm(current and latest digest),3.13-slim-trixieand3.14-slim-trixie— all carry them; only3.15-rcscans clean. The durable answer is WOR-852's own question: move off EOL Debian 12, or wait for a stable 3.13.x/3.14.x backport.The nearest expiry is close.
CVE-2026-15308(from WOR-691) expires 2026-08-10. On 2026-08-11 the expiry hook will fail at commit time and indocker-security.ymluntil someone re-measures. That is the forcing function working as designed. It deliberately does not fail the unit suite — the tests assert the config's shape, not today's date, so a lapsed expiry stops security work rather than everyone's unrelated work.It weakens supply-chain posture on semgrep itself.
uvx semgrep@1.172.0pins semgrep but not its transitive tree, where the previousqaextra resolved it through the hash-lockeduv.lock. That job holdssecurity-events: write. This is a real trade: three CVE alerts closed at the cost of live PyPI resolution for one CI scanner.uvxsupports--constraints, so a hash-pinned constraints file would close it — deliberately left as a follow-up rather than bolted on unreviewed.The mcp floor stays at 1.0. Only the ceiling moved. Anyone installing
worthless[mcp]without the lockfile can still resolve an old 1.x below the 1.28.1 the advisories require. The lockfile lands 1.29.0, so this affects unlocked installs only.No regression test guards the
mcp<2cap in this PR. The cap is verified by measurement here (uv refusesmcp==2.0.0with "worthless[mcp]'s requirements are unsatisfiable"), but nothing stops a future edit removing it. #474 adds exactly that test."Full suite" is narrower than it sounds. The repo's default
addoptsruns-m 'not live and not docker and not user_flow and not real_ipc'with--reruns 1. So 4003 passed / 0 failed excludes the live-provider, Docker, user-flow and real-IPC tiers. The Docker tier is covered separately bydocker-security.yml, but the headline number is not every test in the repo.The semgrep verification was partial. Local runs used the
.semgrep/rules; the CI command additionally pulls--config auto,p/secretsandp/security-auditfrom the registry, which needs network and only executes for real in CI.🤖 Generated with Claude Code
Summary by CodeRabbit
Security & Maintenance
Bug Fixes
Tests