Skip to content

Install gate, Phase 2: gate the full would-install set (tree pass)#112

Open
juangaitanv wants to merge 9 commits into
mainfrom
install-gate-phase-2
Open

Install gate, Phase 2: gate the full would-install set (tree pass)#112
juangaitanv wants to merge 9 commits into
mainfrom
install-gate-phase-2

Conversation

@juangaitanv

@juangaitanv juangaitanv commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Overview

This PR is Phase 2 of the install-gate feature for the Corgea CLI.

Phase 2 expands the gate from named targets to the full would-install set for npm and pip. That means transitive dependencies, lockfile installs, and bare npm installs are checked instead of only the packages typed on the command line.

This keeps Phase 1's public fail-open behavior while making the gate honest about when it has full tree coverage and when it had to fall back to named-only checks.

Stacked on #111. Base branch: install-gate-phase-1. Review this PR's diff in isolation; it contains the Phase 2 slice.

What Phase 2 Includes

  • pip tree resolution through pip install --dry-run --report -, with --only-binary :all: enforced so resolution does not build sdists or run package code.
  • requirements-file safety: -r files are pre-scanned for format-control directives such as --no-binary :all: that could re-enable sdist builds inside pip's parser.
  • npm tree resolution through an isolated npm install --package-lock-only --ignore-scripts in a throwaway temp dir, so the project lockfile is not touched.
  • Bare npm install gating from the nearest ancestor package.json, matching npm's own project-root discovery.
  • npm ci gating from lockfiles, including npm-shrinkwrap precedence, lockfileVersion 1 support, workspace stanza filtering, and fail-closed refusal for unparsable lockfiles unless --force is used.
  • Root-redirect protection for npm flags such as --prefix, -C, and -g, where the install target cannot be safely resolved from the current project context.
  • Provenance labels for findings: (transitive), (from requirements), (already in package.json), and (locked).
  • Named-only fallback disclosure when a full tree pass cannot run. pip -r entries are still parsed and checked in that fallback.
  • Bounded verdict concurrency fixed at 8, with progress output for larger jobs and collapsed repeated outage errors.
  • Test de-flake: vuln_api_url resolution is now tested as a pure function instead of mutating process env in parallel tests.

Deliberately Out Of Scope

Later phases add:

  • uv, yarn, and pnpm
  • --json
  • token auth and fail-closed enforcement

Exit Criteria - Met

A vulnerable transitive dependency blocks the install. A vulnerable lockfile blocks npm ci. Fallback warnings fire when and only when the tree pass did not run.

Covered by hermetic end-to-end tests in tests/cli_tree.rs, tests/cli_bare_install.rs, tests/cli_npm_ci.rs, and tests/cli_provenance.rs. Also confirmed against real npm/pip resolution on the test-cli fixtures, including monorepo ancestor walking and -r requirements.txt trees. ./harness check passes.

@juangaitanv juangaitanv mentioned this pull request Jun 12, 2026
Open
cursor[bot]
cursor Bot reviewed Jun 12, 2026
View reviewed changes
Comment thread src/precheck/tree.rs
Comment thread src/precheck/tree.rs
Comment thread src/precheck/mod.rs
Comment thread src/precheck/tree.rs Outdated
Comment thread src/precheck/tree.rs Outdated
Comment thread src/precheck/tree.rs Outdated
@juangaitanv juangaitanv force-pushed the install-gate-phase-1 branch from 68dbba9 to 5a99db0 Compare June 12, 2026 14:51
juangaitanv added a commit that referenced this pull request Jun 12, 2026
@juangaitanv
Phase 2 review fixes: sdist guard order, root-redirect fail-closed, s…
dd4b575 
…hrinkwrap, lockfile v1, workspace stanzas

Addresses Cursor review on #112.

- SECURITY: the pip resolver's `--only-binary :all:` guard is now appended
  AFTER the user's args. pip format-control is last-wins, so a user
  `--no-binary :all:` could previously re-enable sdist builds and execute
  package code during the report step. e2e test pins the arg order.
- a bare `npm install --prefix <other>` (and `npm ci --prefix/-C`) now
  fails closed unless --force: it installs a redirected project's tree the
  gate can't resolve from the CWD, so it would otherwise install wholly
  unchecked. (Named installs still verify their targets + degrade the tree
  pass to the named-only warning.)
- npm lockfile reads now prefer npm-shrinkwrap.json (npm's precedence) over
  package-lock.json, in both the tree resolver and `npm ci`.
- parse_npm_lockfile supports lockfileVersion 1 (`dependencies` tree,
  recursing nested deps) so a v1 project's `npm ci` isn't forced to bypass
  the gate with --force.
- parse_npm_lockfile restricts the verdict set to `node_modules/` entries,
  excluding workspace SOURCE stanzas (`packages/foo`) — local packages with
  no registry identity that would otherwise be sent to the public vuln-api
  and could falsely block a monorepo install.
@juangaitanv juangaitanv force-pushed the install-gate-phase-2 branch from 936d69a to dd4b575 Compare June 12, 2026 14:51
@juangaitanv juangaitanv force-pushed the install-gate-phase-1 branch from 5a99db0 to ddd215b Compare June 12, 2026 16:42
juangaitanv added a commit that referenced this pull request Jun 12, 2026
@juangaitanv
Phase 2 review fixes: sdist guard order, root-redirect fail-closed, s…
c86aa2d 
…hrinkwrap, lockfile v1, workspace stanzas

Addresses Cursor review on #112.

- SECURITY: the pip resolver's `--only-binary :all:` guard is now appended
  AFTER the user's args. pip format-control is last-wins, so a user
  `--no-binary :all:` could previously re-enable sdist builds and execute
  package code during the report step. e2e test pins the arg order.
- a bare `npm install --prefix <other>` (and `npm ci --prefix/-C`) now
  fails closed unless --force: it installs a redirected project's tree the
  gate can't resolve from the CWD, so it would otherwise install wholly
  unchecked. (Named installs still verify their targets + degrade the tree
  pass to the named-only warning.)
- npm lockfile reads now prefer npm-shrinkwrap.json (npm's precedence) over
  package-lock.json, in both the tree resolver and `npm ci`.
- parse_npm_lockfile supports lockfileVersion 1 (`dependencies` tree,
  recursing nested deps) so a v1 project's `npm ci` isn't forced to bypass
  the gate with --force.
- parse_npm_lockfile restricts the verdict set to `node_modules/` entries,
  excluding workspace SOURCE stanzas (`packages/foo`) — local packages with
  no registry identity that would otherwise be sent to the public vuln-api
  and could falsely block a monorepo install.
@juangaitanv juangaitanv force-pushed the install-gate-phase-2 branch from dd4b575 to c86aa2d Compare June 12, 2026 16:42
@juangaitanv juangaitanv force-pushed the install-gate-phase-1 branch from ddd215b to 5fb3e5d Compare June 12, 2026 18:28
juangaitanv added a commit that referenced this pull request Jun 12, 2026
@juangaitanv
Phase 2 review fixes: sdist guard order, root-redirect fail-closed, s…
0aab78c 
…hrinkwrap, lockfile v1, workspace stanzas

Addresses Cursor review on #112.

- SECURITY: the pip resolver's `--only-binary :all:` guard is now appended
  AFTER the user's args. pip format-control is last-wins, so a user
  `--no-binary :all:` could previously re-enable sdist builds and execute
  package code during the report step. e2e test pins the arg order.
- a bare `npm install --prefix <other>` (and `npm ci --prefix/-C`) now
  fails closed unless --force: it installs a redirected project's tree the
  gate can't resolve from the CWD, so it would otherwise install wholly
  unchecked. (Named installs still verify their targets + degrade the tree
  pass to the named-only warning.)
- npm lockfile reads now prefer npm-shrinkwrap.json (npm's precedence) over
  package-lock.json, in both the tree resolver and `npm ci`.
- parse_npm_lockfile supports lockfileVersion 1 (`dependencies` tree,
  recursing nested deps) so a v1 project's `npm ci` isn't forced to bypass
  the gate with --force.
- parse_npm_lockfile restricts the verdict set to `node_modules/` entries,
  excluding workspace SOURCE stanzas (`packages/foo`) — local packages with
  no registry identity that would otherwise be sent to the public vuln-api
  and could falsely block a monorepo install.
@juangaitanv juangaitanv force-pushed the install-gate-phase-2 branch from c86aa2d to 33da96b Compare June 12, 2026 18:28
leenk7991
leenk7991 reviewed Jun 15, 2026
View reviewed changes
Comment thread src/precheck/tree.rs Outdated
Base automatically changed from install-gate-phase-1 to install-gate-phase-0 June 18, 2026 11:27
@juangaitanv
Phase 1: core install gate — corgea pip|npm install <named targets>
50e2e55 
Harvested from the install-vuln-gate spike (dfac68e), trimmed to
named-target paths: no tree resolution, no uv/yarn/pnpm, no --json, no
token auth — public fail-open mode only.

- corgea npm|pip wrap their package manager: install verbs (found
  behind global flags) gate named targets; everything else passes
  through with the manager's own exit code
- two independent blocks: publish recency (-t, default 2d) and the
  vuln-api verdict on each resolved version
- refusal output built for agent self-correction: per-advisory
  "fixed in <version>" lines and a "→ safe version:" steer naming the
  highest fix covering every advisory
- --force overrides everything; --no-fail demotes recency only
- git/URL/path/editable specs are noted, never blocked; -r files and
  bare installs noted, not gated
- public mode fails open: vuln-api outages warn and continue
- pip→pip3 binary fallback; pip3/pip-add "did you mean" guidance
- SKILL.md: install-wrapper section with limitations + staging targets

Verified end-to-end: all four deterministic staging targets
(axios@0.21.0, minimist@0.0.8, node-fetch@2.6.0, mezzanine==6.0.0)
block with exit 1 and steer to the fixed version; installing the
steered version passes.
@juangaitanv
Phase 1 review fixes: gate npm --tag and pip --pre; warn on custom re…
f23286c 
…gistry

Addresses Cursor review on #111.

- npm `--tag <value>` now resolves the named dist-tag for a bare spec
  (`npm install --tag beta pkg` gates the beta release, not latest), so a
  fresh/vulnerable beta/canary no longer bypasses both blocks. Explicit
  pins/tags still win.
- pip `--pre` makes prereleases eligible: PypiVersion now parses PEP 440
  prereleases (dev<a<b<rc, all below the plain release) and the resolver
  includes them only when `--pre` is set, so the gate verdicts the
  prerelease pip would install instead of the latest stable.
- a custom registry/index flag (`--registry`, `-i`, `--index-url`,
  `--extra-index-url`) now prints a loud warning that the gate resolves
  against the default registry and can't vouch the mirrored artifact —
  full mirror resolution / allow-listing stays out of scope (documented
  limitation, separate PRD).
@juangaitanv
Phase 1 second-round review fixes: canonical pypi names, last-wins --…
4855c7e 
…tag, no resolution guessing

- pypi resolution adopts the registry's canonical spelling (info.name,
  guarded to PEP 503-equivalent values so a hostile mirror can't redirect
  the verdict to another package's identity). The vuln-api keys advisories
  by lowercase(canonical), so checking a user-typed variant (Flask_Cors)
  would miss the flask-cors row and fail open.
- npm --tag is last-wins like npm's own config parser; gating the first
  of two --tag flags verdicts the wrong dist-tag.
- pick_latest_stable no longer guesses by upload time when nothing parses
  as PEP 440 (could pick a prerelease without --pre); a visible
  resolution error replaces the silent wrong pick.
- Resolution-error output now states the target is ungated.
@juangaitanv
Phase 2: gate the full would-install set (tree pass)
a782191 
Harvested from the install-vuln-gate spike (dfac68e), trimmed to pip/npm
(no uv, no --json, public mode only).

- pip tree via `pip install --dry-run --report -` (--only-binary :all:
  so resolution never builds sdists); npm tree via an isolated
  `npm install --package-lock-only --ignore-scripts` in a throwaway dir
  that never touches the project lockfile
- bare `npm install` gated from the nearest-ancestor package.json (like
  npm finds it); `npm ci` and aliases gated from the lockfile directly,
  with an unparsable-lockfile refusal (--force escape)
- transitive findings labeled by provenance: (transitive),
  (from requirements), (already in package.json) — with a
  "fix with: corgea npm install <pkg>@<fix>" hint — and (locked)
- honest named-only fallback: a failed dry-run or an npm root-redirect
  flag (--prefix, -g) degrades loudly, and pip -r requirements.txt
  entries are still parsed and verified in the fallback
- bare installs that block blame the existing tree explicitly when no
  finding was added by the command
- bounded verdict pool (fixed at 8) with a progress line above 8 jobs;
  repeated outage errors collapse into one line above 3 findings

Also de-flakes the config test: vuln_api_url resolution is tested as a
pure function instead of mutating process env, which raced concurrent
getenv calls under the parallel test harness.
@juangaitanv
Phase 2 review fixes: sdist guard order, root-redirect fail-closed, s…
0b4dfe3 
…hrinkwrap, lockfile v1, workspace stanzas

Addresses Cursor review on #112.

- SECURITY: the pip resolver's `--only-binary :all:` guard is now appended
  AFTER the user's args. pip format-control is last-wins, so a user
  `--no-binary :all:` could previously re-enable sdist builds and execute
  package code during the report step. e2e test pins the arg order.
- a bare `npm install --prefix <other>` (and `npm ci --prefix/-C`) now
  fails closed unless --force: it installs a redirected project's tree the
  gate can't resolve from the CWD, so it would otherwise install wholly
  unchecked. (Named installs still verify their targets + degrade the tree
  pass to the named-only warning.)
- npm lockfile reads now prefer npm-shrinkwrap.json (npm's precedence) over
  package-lock.json, in both the tree resolver and `npm ci`.
- parse_npm_lockfile supports lockfileVersion 1 (`dependencies` tree,
  recursing nested deps) so a v1 project's `npm ci` isn't forced to bypass
  the gate with --force.
- parse_npm_lockfile restricts the verdict set to `node_modules/` entries,
  excluding workspace SOURCE stanzas (`packages/foo`) — local packages with
  no registry identity that would otherwise be sent to the public vuln-api
  and could falsely block a monorepo install.
@juangaitanv
Phase 2 second-round review fixes: -r file sdist guard, v1 depth cap,…
dc142ed 
… .npmrc rationale

- SECURITY: pip applies format-control directives found INSIDE a -r file
  AFTER command-line parsing, so a '--no-binary :all:' line in a
  requirements file overrode the tree pass's trailing '--only-binary
  :all:' guard and built sdists — executing package code — during the
  dry-run. The tree pass now pre-scans -r files (transitively, following
  nested includes) and refuses to dry-run any file carrying a
  format-control directive, degrading to the named-only fallback whose
  parser skips option lines. The scan is best-effort on unreadable files
  (same uid — pip can't read them either and fails loudly itself).
  Unit + e2e tests prove the dry-run never executes against such a file
  while the file's entries still verdict.
- collect_v1_dependencies carries an explicit depth cap (serde_json's
  recursion limit fires first today; the cap is the backstop), with a
  depth-bomb test.
- Documented why the .npmrc copy is safe (CLI flags win; package-lock=false
  degrades to named-only by design).
- Adapted the phase-1 alternate-spelling test to the tree-aware fake pip.
@juangaitanv
Adapt phase-1/2 to main's phase-0 API: tier is Option<u8>
9138bf3 
Rebasing the install-gate stack onto main swaps in main's phase-0 squash
(#110), where VulnMatch.tier landed as Option<u8> after review. The stack
was built on the pre-squash phase-0 branch (tier: u8), so the test helper
in precheck used a bare u8 literal. Match main's type.
@juangaitanv juangaitanv force-pushed the install-gate-phase-2 branch from 33da96b to 9138bf3 Compare June 18, 2026 11:31
@juangaitanv juangaitanv changed the base branch from install-gate-phase-0 to main June 18, 2026 11:31
@juangaitanv
Bound the tree resolvers with a timeout (review: leenk7991)
44296d1 
resolve_pip_tree and resolve_npm_tree shelled out via Command::output(),
which blocks unbounded. The gate runs before the real install, so a stalled
pip/npm (slow or unreachable registry) would hang 'corgea ... install' with
no ceiling. Add run_bounded: spawn with piped stdio drained on threads (pip's
--report - JSON can exceed the pipe buffer, so it must be read concurrently),
poll for exit against a 120s deadline, kill on overrun. A timeout returns Err,
which the caller already routes to the named-only fallback. Tests cover both
the kill-on-overrun and fast-path-capture cases.
@juangaitanv juangaitanv requested a review from leenk7991 June 18, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@leenk7991 leenk7991 Awaiting requested review from leenk7991

@yhoztak yhoztak Awaiting requested review from yhoztak

@asadeddin asadeddin Awaiting requested review from asadeddin

@Ibrahimrahhal Ibrahimrahhal Awaiting requested review from Ibrahimrahhal

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@juangaitanv @leenk7991