docs: add transitive dependencies section to Security and risk management#2676
docs: add transitive dependencies section to Security and risk management#2676LipeGheno wants to merge 3 commits into
Conversation
…ment Documents the transitive dependency import chain feature in the Dependencies section, including how findings are labelled, how to read the chain, upgrade labels, cases where no upgrade is available, and current limitations. Adds screenshot of the chain in the Findings tab. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Overall readability score: 54.21 (🟢 +0)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
Up to standards ✅🟢 Issues
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a new 'Transitive dependencies' section to the documentation, detailing how import chains are displayed for vulnerabilities found in indirect packages. The feedback suggests improving sentence flow by adding a comma and ensuring terminology consistency by using 'Software Composition Analysis (SCA)' instead of 'dependency scanning'.
|
|
||
| #### When no upgrade is available | ||
|
|
||
| If no package in the chain has a patched release yet, the chain shows the full path without an upgrade label. In that case the vulnerability cannot be resolved by a version bump alone; you may need to wait for an upstream fix, apply a workaround, or accept the risk per your organization's policy. |
There was a problem hiding this comment.
Add a comma after "In that case" to improve the readability and flow of the sentence.
| If no package in the chain has a patched release yet, the chain shows the full path without an upgrade label. In that case the vulnerability cannot be resolved by a version bump alone; you may need to wait for an upstream fix, apply a workaround, or accept the risk per your organization's policy. | |
| If no package in the chain has a patched release yet, the chain shows the full path without an upgrade label. In that case, the vulnerability cannot be resolved by a version bump alone; you may need to wait for an upstream fix, apply a workaround, or accept the risk per your organization's policy. |
|
|
||
| #### Limitations | ||
|
|
||
| - The import chain is shown only for findings that come from dependency scanning. Findings from other scan types (container scanning, app scanning) do not show a chain. |
There was a problem hiding this comment.
To maintain consistency with the "Scan types" table (line 338) and other sections of the documentation (e.g., line 184), use the formal term "Software Composition Analysis (SCA)" instead of "dependency scanning".
| - The import chain is shown only for findings that come from dependency scanning. Findings from other scan types (container scanning, app scanning) do not show a chain. | |
| - The import chain is shown only for findings that come from Software Composition Analysis (SCA). Findings from other scan types (container scanning, app scanning) do not show a chain. |
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
The documentation changes are technically sound and follow the project's quality standards according to Codacy. However, there is a critical gap: the image security-risk-management-transitive-chain.png referenced in the new section is not included in this pull request. This omission will cause a broken image link in the production documentation and fails to meet the acceptance criterion requiring a UI screenshot. There are also two minor suggestions for spelling and punctuation to maintain consistency with the existing documentation.
About this PR
- The PR description mentions adding a screenshot, and the Markdown references it, but the image file 'security-risk-management-transitive-chain.png' is not included in the provided code changes. Please ensure the image is added to the repository.
Test suggestions
- Verify that the 'Transitive dependencies' section and its subsections render correctly on the documentation site.
- Ensure the image file 'images/security-risk-management-transitive-chain.png' exists at the specified path.
- Check that the anchor link '#transitive-dependencies' is functional and correctly indexed.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that the 'Transitive dependencies' section and its subsections render correctly on the documentation site.
2. Ensure the image file 'images/security-risk-management-transitive-chain.png' exists at the specified path.
3. Check that the anchor link '#transitive-dependencies' is functional and correctly indexed.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
|
|
||
| Open the **Findings** tab under **Security and risk management**. Findings caused by a transitive dependency are labelled **Transitive Dependency** in the header. | ||
|
|
||
|  |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The image 'security-risk-management-transitive-chain.png' is referenced but not included in the PR, which will result in a broken image in the rendered documentation.
|
|
||
| #### When no upgrade is available | ||
|
|
||
| If no package in the chain has a patched release yet, the chain shows the full path without an upgrade label. In that case the vulnerability cannot be resolved by a version bump alone; you may need to wait for an upstream fix, apply a workaround, or accept the risk per your organization's policy. |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: Add a comma after the introductory phrase.
| If no package in the chain has a patched release yet, the chain shows the full path without an upgrade label. In that case the vulnerability cannot be resolved by a version bump alone; you may need to wait for an upstream fix, apply a workaround, or accept the risk per your organization's policy. | |
| If no package in the chain has a patched release yet, the chain shows the full path without an upgrade label. In that case, the vulnerability cannot be resolved by a version bump alone; you may need to wait for an upstream fix, apply a workaround, or accept the risk per your organization's policy. |
|
|
||
| #### Where you see it | ||
|
|
||
| Open the **Findings** tab under **Security and risk management**. Findings caused by a transitive dependency are labelled **Transitive Dependency** in the header. |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: Use the American English spelling for consistency with the rest of the documentation.
| Open the **Findings** tab under **Security and risk management**. Findings caused by a transitive dependency are labelled **Transitive Dependency** in the header. | |
| Open the **Findings** tab under **Security and risk management**. Findings caused by a transitive dependency are labeled **Transitive Dependency** in the header. |
Revises the "Reading the chain" section to reflect the actual chain format: Transitive label → intermediate packages → bold vulnerable package → Fixed version label at the end. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
The documentation for transitive dependencies is well-structured and meets the general acceptance criteria. However, the current examples contain logical inconsistencies regarding versioning and lack clarity on which package in the chain should be upgraded. Specifically, the example uses version numbers that imply a version downgrade for the vulnerable package, and the text does not explicitly identify the root dependency as the target for resolution.
Additionally, there is a discrepancy between the visualization components described in the PR summary (repository and CVE) and what is actually explained in the documentation text. Addressing these clarity issues will significantly improve the value of the documentation for end-users.
About this PR
- The documentation text and examples currently omit the 'repository' and 'CVE' elements of the chain visualization that were mentioned in the PR description. If these elements are present in the actual UI visualization, they should be explicitly mentioned in the 'Reading the chain' section for completeness.
Test suggestions
- Verify the 'Transitive dependencies' header and its anchor ID are correctly defined in the Markdown
- Confirm the screenshot file reference 'images/security-risk-management-transitive-chain.png' exists
- Check that the limitations section accurately reflects the constraints mentioned in the PR description
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| Transitive → peft@0.11.1 → accelerate@0.31.0 → Torch@2.4.0 Fixed version 1.1.1 | ||
| ``` | ||
|
|
||
| In this example, `Torch@2.4.0` is the vulnerable package, and upgrading to the indicated fixed version resolves the vulnerability across this dependency path. |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The relationship between the 'Fixed version' and the packages in the chain is unclear. In the example provided, '1.1.1' appears to be a version downgrade for 'Torch@2.4.0', which is confusing. It should be explicitly stated that the fixed version refers to the root dependency (peft), which is the package the user actually manages.
Suggested improvement:
'In this example, Torch@2.4.0 is the vulnerable package, and upgrading the root dependency (peft) to the indicated fixed version (1.1.1) resolves the vulnerability across this dependency path.'
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
security-risk-management-transitive-chain.png) showing the chain on a finding cardTest plan
#transitive-dependencies)🤖 Generated with Claude Code