Integration cleanup: coherent fix hints, honest refusal blame, audit plumbing#108
Merged
Merged
Conversation
…plumbing Post-merge cleanup across the eight install-gate units: - Consolidate safe_version/advertised_fix into one highest_fix core (strict vs lenient parsing); the pre-existing fix hint's hedge now follows the steer re-check: Verified drops '(advertised fix)', Rejected suppresses the hint, Unverified keeps the hedge. - The existing-tree refusal only fires when every vulnerable finding predates the command: Requested (pip -r) findings and transitives pulled in by named targets get the generic refusal. Summary parenthetical reworded to 'from resolved tree'. - CORGEA_NO_NPM_AUDIT moves out of tree.rs into PrecheckOptions.npm_audit, read in main like the registry overrides; env semantics unchanged. - Audit collect window tightened to 1s: the warn-only signal never changes the outcome, so a finished gate shouldn't stall for it.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Two fixes from the PR #108 review: - refusal_blames_existing_tree filtered tree findings to Vulnerable before the origin test, but should_block_install refuses on Unverifiable too — a command-added unverifiable transitive alongside a pre-existing vulnerable dep printed 'none were added by this command'. Every blocking tree finding now passes the origin test. - The audit thread was detached with the Child trapped inside it; a fast gate exit orphaned a slow 'npm audit' past the CLI's lifetime. spawn_audit now returns an AuditHandle whose collect() does the 1s recv, then kills whatever is left in the shared child slot and joins the thread — no orphan, TempDir drops deterministically, happy-path latency unchanged. The hang test's fake now execs its sleep (a grandchild would dodge the kill) and asserts the child is dead after the CLI exits.
A requirements-only install (pip install -r reqs.txt) has no named outcomes — exactly like a bare install — so outcomes.is_empty() wrongly let a vulnerable transitive of a clean requirements entry blame the existing tree. PrecheckReport now carries bare_install (no CLI targets AND no requirements files), set from the parsed command, and the Transitive arm tests that instead of inferring from outcomes. Regressions: requirements-only e2e + full origin/named/bare unit matrix.
Contributor
Author
|
Re:
Regressions added:
|
The bin existed for live e2e dogfooding, now done. Every consumer in the repo (7 integration test files + precheck unit tests) uses the in-process vuln_api_stub::spawn_with_statuses, so the bin-only surface goes with it: the fixtures.rs file loader, spawn_from_file, VulnApiStub::block, and the StubFixtures/spawn_on_port indirection (the ephemeral-port bind now lives directly in spawn_with_statuses).
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.
Follow-up to #100–#107, addressing the cross-unit review findings:
Fix-version helpers consolidated.
safe_version(certification: every match must carry a parseable fix) andadvertised_fix(best-effort max) were parallel near-duplicates; both now delegate to onehighest_fixcore with a strict/lenient parse flag. Output coherence: thefix with:hint on pre-existing findings follows the steer re-check — a Verified steer drops the(advertised fix)hedge, a Rejected steer suppresses the hint (the rejection line already printed), only an unverified proposal keeps the hedge.Refusal blame is now origin-aware.
Refusing to run install: your existing dependency tree…previously fired whenever no named target was flagged — lying forpip install -r reqs.txt(finding labeled(from requirements)right above) and for fresh-project installs whose named target pulls a vulnerable transitive. New rule: blame the existing tree only when every vulnerable finding predates the command (bare installs, or manifest-declaredPreExistingdeps). The summary parenthetical(N from existing tree)is reworded to(N from resolved tree)for the same reason.Audit kill switch plumbed like every other knob.
CORGEA_NO_NPM_AUDITwas read viastd::envdeep intree.rs; it's nowPrecheckOptions::npm_audit, read once inmainalongside the registry overrides. Env semantics unchanged (e2e kill-switch test passes untouched).Audit collect window 2s → 1s. The warn-only second opinion never changes the outcome; a finished gate shouldn't stall 2s for it.
Verified:
./harness checkgreen (306 tests), and live staging re-runs —npm install mkdirp@0.5.1(vulnerable transitive minimist) now gets the generic refusal; bare install with poisoned manifest keeps the existing-tree refusal and prints the unhedgedfix with: corgea npm install axios@0.21.2after steer verification.