Install gate, Phase 2: gate the full would-install set (tree pass)#112
Open
juangaitanv wants to merge 3 commits into
Open
Install gate, Phase 2: gate the full would-install set (tree pass)#112juangaitanv wants to merge 3 commits into
juangaitanv wants to merge 3 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
juangaitanv
force-pushed
the
install-gate-phase-1
branch
from
68dbba9 to
5a99db0
Compare
juangaitanv
added a commit
that referenced
this pull request
Jun 12, 2026
…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
force-pushed
the
install-gate-phase-2
branch
from
936d69a to
dd4b575
Compare
juangaitanv
force-pushed
the
install-gate-phase-1
branch
from
5a99db0 to
ddd215b
Compare
juangaitanv
added a commit
that referenced
this pull request
Jun 12, 2026
…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
force-pushed
the
install-gate-phase-2
branch
from
dd4b575 to
c86aa2d
Compare
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.
…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.
… .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
force-pushed
the
install-gate-phase-1
branch
from
ddd215b to
5fb3e5d
Compare
juangaitanv
force-pushed
the
install-gate-phase-2
branch
from
c86aa2d to
33da96b
Compare
leenk7991
reviewed
Jun 15, 2026
| // user `--no-binary :all:` / `--only-binary :none:` placed in install_args | ||
| // must not re-enable sdist builds (which would run package code during the | ||
| // report step, violating this file's safety invariant). | ||
| let output = Command::new(resolved) |
There was a problem hiding this comment.
do we need a timeout here? same for resolve_npm_tree?
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
2 participants
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.
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 install --dry-run --report -, with--only-binary :all:enforced so resolution does not build sdists or run package code.-rfiles are pre-scanned for format-control directives such as--no-binary :all:that could re-enable sdist builds inside pip's parser.npm install --package-lock-only --ignore-scriptsin a throwaway temp dir, so the project lockfile is not touched.npm installgating from the nearest ancestorpackage.json, matching npm's own project-root discovery.npm cigating from lockfiles, including npm-shrinkwrap precedence, lockfileVersion 1 support, workspace stanza filtering, and fail-closed refusal for unparsable lockfiles unless--forceis used.--prefix,-C, and-g, where the install target cannot be safely resolved from the current project context.(transitive),(from requirements),(already in package.json), and(locked).-rentries are still parsed and checked in that fallback.vuln_api_urlresolution is now tested as a pure function instead of mutating process env in parallel tests.Deliberately Out Of Scope
Later phases add:
--jsonExit Criteria - Met
Covered by hermetic end-to-end tests in
tests/cli_tree.rs,tests/cli_bare_install.rs,tests/cli_npm_ci.rs, andtests/cli_provenance.rs. Also confirmed against real npm/pip resolution on thetest-clifixtures, including monorepo ancestor walking and-r requirements.txttrees../harness checkpasses.