feat(pip): add pyproject.toml support via pip --dry-run --report#465
Open
ruromero wants to merge 4 commits intoguacsec:mainfrom
Open
feat(pip): add pyproject.toml support via pip --dry-run --report#465ruromero wants to merge 4 commits intoguacsec:mainfrom
ruromero wants to merge 4 commits intoguacsec:mainfrom
Conversation
Review Summary by QodoAdd pip provider for PEP 621 pyproject.toml and workspace support
WalkthroughsDescription• Add Python_pip_pyproject provider for PEP 621 pyproject.toml without lock files • Resolve dependencies using pip install --dry-run --ignore-installed --report • Implement workspace/monorepo support with lock file discovery and boundary detection • Add comprehensive test coverage for pip provider and workspace scenarios Diagramflowchart LR
A["pyproject.toml<br/>PEP 621 format"] -->|"pip --dry-run<br/>--report"| B["pip report<br/>JSON"]
B -->|"Parse & build<br/>dependency graph"| C["Python_pip_pyproject<br/>Provider"]
C -->|"Fallback when<br/>no lock file"| D["SBOM<br/>with transitive deps"]
E["Workspace detection<br/>uv/poetry markers"] -->|"Walk up directory<br/>tree"| F["Find lock file<br/>or boundary"]
F -->|"Resolve to<br/>correct lock dir"| G["Accurate analysis<br/>per package"]
File Changes1. src/provider.js
|
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
src/providers/base_pyproject.js
Outdated
Comment on lines
+351
to
+352
| let lockFileDir = this._findLockFileDir(manifestDir, opts) || manifestDir | ||
| let { directDeps, graph } = await this._getDependencyData(lockFileDir, parsed, opts) |
There was a problem hiding this comment.
1. Poetry uses wrong project 🐞 Bug ≡ Correctness
Base_pyproject now calls _getDependencyData() using lockFileDir rather than the provided manifest’s directory, but Python_poetry derives its dependency data only from that passed directory and does not use the parsed member pyproject. When a workspace member pyproject.toml relies on a poetry.lock in a parent directory, dependency resolution can be performed against the parent project instead of the requested member manifest, producing an incorrect SBOM.
Agent Prompt
## Issue description
`Base_pyproject._createSbom()` now calls `_getDependencyData()` with `lockFileDir`, which changes the effective project context for providers that derive dependency data from the working directory (notably `Python_poetry`). This can cause member-manifest analysis to resolve dependencies for the lockfile directory’s project instead of the requested `pyproject.toml`.
## Issue Context
- `Base_pyproject._createSbom()` parses the requested manifest, but passes `lockFileDir` into `_getDependencyData()`.
- `Python_poetry._getDependencyData()` ignores `parsed` and uses `manifestDir` to run `poetry show ...`.
## Fix Focus Areas
- src/providers/base_pyproject.js[346-353]
- src/providers/python_poetry.js[17-42]
## Suggested fix approach
- Change the contract so providers receive both:
- `projectDir` (directory of the requested manifest)
- `lockFileDir` (directory where the lock file was found)
- Update `Base_pyproject._createSbom()` to call something like `_getDependencyData({ projectDir: manifestDir, lockFileDir }, parsed, opts)` (or add an extra parameter).
- For `Python_poetry`, ensure commands are executed in `projectDir` (or explicitly point Poetry at the requested project if it supports a directory flag), while still using the correct lockfile location as needed.
- Keep `Python_uv` behavior using `lockFileDir` if that’s required for workspace export, but do so explicitly via the new parameters.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
[sdlc-workflow/verify-pr] Classified as code change request — sub-task TC-4098 created to address this feedback.
Comment on lines
+63
to
+75
| let rootEntry = packages.find(p => p.download_info?.dir_info !== undefined) | ||
| let rootRequires = rootEntry?.metadata?.requires_dist || [] | ||
|
|
||
| let directDepNames = new Set() | ||
| for (let req of rootRequires) { | ||
| if (this._hasExtraMarker(req)) { continue } | ||
| let name = this._extractDepName(req) | ||
| if (name) { directDepNames.add(this._canonicalize(name)) } | ||
| } | ||
|
|
||
| let graph = new Map() | ||
| let nonRootPackages = packages.filter(p => p.download_info?.dir_info === undefined) | ||
|
|
There was a problem hiding this comment.
2. Pip root misidentified 🐞 Bug ≡ Correctness
Python_pip_pyproject identifies the root entry by checking download_info.dir_info and excludes all dir_info entries from the graph, which can misidentify the root and drop local/path dependencies. Pip reports already contain a requested flag suitable for reliable root identification, so the current approach can produce an incorrect dependency tree and wrong direct/transitive classification.
Agent Prompt
## Issue description
`Python_pip_pyproject._parsePipReport()` treats the first `download_info.dir_info` entry as the root and removes all `dir_info` entries from the dependency graph. This can misidentify the root project and omit legitimate local/path dependencies.
## Issue Context
Pip report entries contain a `requested` flag (seen in the repo’s pip report fixture) that can be used to identify the root request more reliably than `dir_info`.
## Fix Focus Areas
- src/providers/python_pip_pyproject.js[59-99]
- test/providers/tst_manifests/pyproject/pip_pep621/pip_report.json[4-13]
## Suggested fix approach
- Identify the root entry using `requested: true` (and/or matching the parsed project name), not `dir_info`.
- Build the graph from all non-root packages, including those with `dir_info` (local/path deps), instead of excluding them wholesale.
- Only exclude the actual root project from the graph, not every `dir_info` entry.
- Add/extend tests with a pip report containing a local/path dependency that also has `dir_info` to ensure it stays in the graph and is correctly classified.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
[sdlc-workflow/verify-pr] Classified as code change request — sub-task TC-4099 created to address this feedback.
Collaborator
Author
Verification Report — TC-4065
Root-Cause AnalysisTC-4098 —
TC-4099 —
Overall: ❌ FAILBlocking issues:
This comment was AI-generated by sdlc-workflow/verify-pr v0.5.11. |
Add Python_pip_pyproject provider that resolves PEP 621 pyproject.toml dependencies using `pip install --dry-run --ignore-installed --report`. This is the fallback provider when no lock file (uv.lock/poetry.lock) is found, enabling analysis of standard pyproject.toml projects without requiring uv or poetry. The pip report JSON provides resolved versions, requires_dist for building the dependency tree, and requested flags for direct vs transitive classification. Extras-only deps are filtered out. Supports TRUSTIFY_DA_PIP_REPORT env var for testing without pip. Implements TC-4065 Assisted-by: Claude Code
Collaborator
Author
Verification Report for TC-4065 (commit 7a2cffa)
Overall:
|
…se class Python_pip_pyproject._getDependencyData() had only 3 parameters (manifestDir, parsed, opts) instead of the 4 defined by the base class (manifestDir, workspaceDir, parsed, opts). When called from _createSbom() with 4 arguments, workspaceDir was silently received as parsed, and parsed was received as opts, causing argument misalignment. Add the missing _workspaceDir parameter to match the base class contract and sibling providers (python_uv, python_poetry). Implements TC-4098 Assisted-by: Claude Code
…ir_info entries _parsePipReport() filtered nonRootPackages by checking p.download_info?.dir_info === undefined, which excluded ALL packages with dir_info (including local/path dependencies). Changed to filter by identity (p !== rootEntry) so only the specific root project entry is excluded, matching the Java implementation's behavior. Implements TC-4099 Assisted-by: Claude Code
Collaborator
Author
|
/review |
Extract shared SBOM_CASES constant and use forEach loops to eliminate repetitive stack/component test pairs across uv, poetry, and pip suites. Parameterize validateLockFile and workspace tests similarly. Add JSDoc comments to all 39 test functions. Implements TC-4065 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
Python_pip_pyprojectprovider that resolves PEP 621pyproject.tomldependencies usingpip install --dry-run --ignore-installed --reportuv.lock/poetry.lock) is found — registered after uv/poetry in the provider matching orderTRUSTIFY_DA_PIP_REPORTenv var override for testing without pip installed_getDependencyDatasignature inPython_pip_pyprojectto match base class 4-param contract (TC-4098)_parsePipReportto only exclude the root entry from the graph, not alldir_infoentries — preserves local/path dependencies (TC-4099)Details
The pip report JSON provides:
requires_distper package for building the dependency treerequested: true/falsefor root project identificationExtras-only dependencies (e.g.
PySocks; extra == "socks") are filtered out.Only PEP 621 format (
[project.dependencies]) is in scope — poetry-formatpyproject.tomlshould use the lock-file-based provider.Implements TC-4065
Test plan
exhortignoremarker excludes deps from both analysis typescharset_normalizer→charset-normalizer)validateLockFilealways returns true (fallback)🤖 Generated with Claude Code