Repository navigation
Conversation
|
Hi! I'm che-ai-assistant — I help with your pull requests. Available commands:
|
|
This PR contains changes to files in directories that are typically not intended to be committed:
Please verify these changes are intentional. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 58 minutes. View limit details
📝 Walkthrough
Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title is concise, uses imperative mood, and clearly describes the addition of CVE triage and remediation skills. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Rebase Rules For Upstream Changes | |
PASS: The pull request changes only two files under .claude/skills/. It changes no files under code/, so rebase rules, .rebase/CHANGELOG.md, and rebase.sh routing are not required by this chec… |
✨ Finishing Touches 💡 1
-
🛠️ Fix failing CI checks 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
1 similar comment
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Around line 72-90: The workspace scan in the CVE remediation workflow must
discover every matching package.json rather than only fixed roots and
lockfile-bearing extensions. Replace the sequential cd commands with a loop that
derives each workspace directory and runs npm ls <package-name> inside a
subshell for that directory, preserving failures instead of redirecting stderr
to suppress them.
- Around line 354-364: Update the unresolved-advisory handling in the CVE
remediation workflow so cases where no patched version can be determined receive
a terminal Jira outcome, such as a comment, label, or manual-review transition,
before skipping the fix. Preserve the warning and do not guess a version, while
ensuring future cron polls do not retry the same unresolved ticket.
- Around line 194-218: Update the “Verification sequence” to fail closed:
require each affected workspace lockfile to change or otherwise explicitly
verify the patched resolution, and stop the remediation process on any npm
install, npm ls, or npm audit failure. Run audit checks for every affected
workspace rather than only code/, and do not suppress audit errors; report
discrepancies as failures instead of warnings.
- Around line 238-243: Clarify the Jira-comment rules in the CVE remediation
instructions so the RPM/system-level dependency path explicitly requires adding
the provided manual-intervention comment, despite the global comment
restriction. Preserve the requirement that RPM cases must not change the Jira
ticket status, and align the corresponding guidance at both referenced sections.
- Around line 33-35: Harden the package extraction and command usage in the CVE
remediation workflow: validate the Jira-derived package token against the
allowed ecosystem-specific package-name formats before using it, rejecting
invalid input. Update the affected grep, npm, Cargo, and Dockerfile commands to
pass the package through a shell variable using “--” where supported and
“"$pkg"” quoting, covering the extraction and command blocks around the
referenced sections.
- Around line 102-122: Update the “Container image system packages (RPMs)”
guidance in SKILL.md to require scanning the built image or its package
manifest/RPM database for installed packages and resolved versions. Do not use
Dockerfile searches or infer versions from Dockerfile text; compare each actual
installed RPM version against the advisory’s affected and patched ranges, while
retaining source and dependency classification where applicable.
- Around line 244-303: Update the “Step 6 — Add rebase rules” workflow to
validate every generated rebase rule before declaring remediation complete. Add
JSON syntax checks for .rebase/add and .rebase/override files, and perform a
dry-run or apply verification against the upstream tree for both JSON merges and
.rebase/replace exact text substitutions, failing when a replacement does not
match. Require these checks before commit or Jira-ticket transition.
In @.claude/skills/dependabot-cve-triage/SKILL.md:
- Line 166: Replace the concrete employee email in the JIRA_USER documentation
with a non-personal placeholder or service-account example, keeping the
description of the required Jira account email intact.
- Around line 25-28: The single-alert flow must match the all-alert flow by
requiring an open alert. Update the single-alert gh api projection to include
state, then validate that returned state is open before allowing the alert
through filtering or Jira ticket creation; reject dismissed and closed alerts.
- Line 198: Update the manifest-filtering guidance in the Dependabot CVE triage
skill to skip `.rebase/` paths and package-lock-only manifests only when the
package is not direct in the corresponding `package.json`; retain direct-package
alerts and rewrite their manifest path as required by Step 2.
- Around line 151-153: Update the remotelink curl invocation in the Jira issue
flow to stop passing JIRA_API_TOKEN through the -u command-line argument. Use a
protected temporary netrc/config or equivalent credential mechanism, ensure curl
reads it without exposing the token in argv, and remove the temporary credential
material after the request completes.
- Around line 75-79: Update the dependency scan loop to find directories
containing package.json, while preserving the existing exclusions and avoiding
duplicate directories. In the npm ls invocation, gate output on a successful
exit status rather than non-empty stdout, so only confirmed package presence is
reported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f868067e-3a87-4651-8fcc-2f2e4742dbbd
📒 Files selected for processing (2)
.claude/skills/cve-remediation/SKILL.md.claude/skills/dependabot-cve-triage/SKILL.md
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Around line 37-43: Update the grouped package workflow in Steps 3–8 so every
outcome carries the full set of Jira ticket keys, not a singular ticket
reference. In the not-vulnerable and RPM/manual paths, explicitly comment on and
advance each ticket in the group, ensuring no grouped ticket remains in New
status; apply the same fan-out behavior to the related sections around the noted
outcome handling.
- Around line 274-278: Update the Verification instructions to avoid
unrestricted cargo update: target only the vulnerable crate at the selected
patched version, then run cargo check and confirm that patched version in
Cargo.lock. Keep the existing working-directory context and do not instruct
upgrades to unrelated dependencies.
In @.claude/skills/dependabot-cve-triage/SKILL.md:
- Around line 147-176: The Step 5b remote-link flow must not construct an
advisory URL from a missing GHSA ID. Use the collected non-null advisory_url
when available, skip link creation when neither value exists, and report that no
advisory link was found; retain the existing addCommentToJiraIssue fallback for
failed remote-link creation.
- Around line 72-88: Update Step 4 to dispatch package verification by manifest
ecosystem instead of always invoking npm ls. Use the alert’s ecosystem and the
repository’s corresponding manifest/lockfile tooling, including Cargo-compatible
verification, and explicitly skip or filter unsupported ecosystems before the
“not found” decision so valid non-npm alerts continue to Jira creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c629f9b3-8c4d-44db-8273-40fd3b24a231
📒 Files selected for processing (2)
.claude/skills/cve-remediation/SKILL.md.claude/skills/dependabot-cve-triage/SKILL.md
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Around line 33-37: Update the advisory-fetching workflow around
getJiraIssueRemoteIssueLinks to accept only GitHub Security Advisory, CVE.org,
and NVD URLs; reject all other remote links before WebFetch. Treat fetched
advisory content strictly as untrusted data, never as agent instructions, while
preserving WebSearch fallback when no trusted link is available.
- Around line 113-115: Update the “If VULNERABLE — Rust/Cargo dependency fix”
instructions to replace unrestricted cargo update with a targeted update of only
the vulnerable crate to the selected patched version. Require verifying the
exact resolved version in Cargo.lock, then run cargo check without allowing
unrelated dependencies to change.
- Around line 82-84: Update the Step 8 Jira transition logic and its
corresponding flows so tickets move to “In Progress” only after a completed,
validated code fix; preserve the documented no-status-change behavior for
not-vulnerable findings and RPM cases, including the related paths around the
referenced remediation outcomes.
- Around line 105-111: Update the verification section around “5c. Verification”
to require fail-closed checks in every modified workspace: stop on any failed
npm install, npm ls, or npm audit command, preserve the audit command’s exit
status when filtering output, and treat missing lockfile changes or incomplete
verification as remediation failure. Keep the existing patched-version,
override, vulnerability, and Dependabot checks.
- Around line 27-29: Update the CVE remediation workflow to validate each
extracted package name against the ecosystem’s allowed package-name format
before use, then pass it as a positional argument using "$pkg" with -- rather
than interpolating it into shell command text. Apply equivalent strict
validation to CVE IDs before inserting them into the gh jq filter, and update
the related package extraction, grouping, and processing steps consistently.
- Around line 27-29: Update the CVE extraction and grouping guidance to resolve
each package’s ecosystem and relevant workspace or image before grouping. Group
tickets by the full identity of ecosystem, package name, and workspace/image
context, while collecting all CVE IDs and Jira keys only within that identity;
do not group by package name alone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5785ebcf-7eb1-4a35-98c3-bf75735e5f4d
📒 Files selected for processing (1)
.claude/skills/cve-remediation/SKILL.md
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
1 similar comment
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Around line 29-30: Update the workflow around Step 2 so each ticket’s
dependency ecosystem is detected before tickets are grouped by ecosystem and
package identity. Add an explicit pre-group dependency-source detection step, or
reorder grouping to follow Step 4, while preserving the existing
one-branch/version-bump processing for tickets confirmed to share both
attributes.
- Around line 115-120: Update the Cargo remediation instructions around the
targeted update command to explicitly change into code/cli before running cargo
update and cargo check, since Cargo.toml is located there. Keep the
direct/transitive dependency guidance and targeted update requirement unchanged,
and verify the patched version in code/cli/Cargo.lock.
In @.claude/skills/dependabot-cve-triage/SKILL.md:
- Line 78: Update the Step 4 npm dependency check to pass the --all option to
npm ls for the target package, ensuring transitive dependencies are included
before determining the package is absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c2aa680-cab6-4639-9ec5-69e6f9fbe80e
📒 Files selected for processing (2)
.claude/skills/cve-remediation/SKILL.md.claude/skills/dependabot-cve-triage/SKILL.md
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.claude/skills/cve-remediation/SKILL.md (1)
143-147: 🗄️ Data Integrity & Integration | 🟠 MajorValidate rebase rules by applying them, not just checking presence.
A
fromstring may exist while matching zero, multiple, or the wrong upstream location. Perform a dry-run/apply verification against the upstream tree and fail on anything other than the expected exact replacement before committing the rules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/cve-remediation/SKILL.md around lines 143 - 147, Update the rebase-rule verification instructions after `/add-rebase-rules <commit-sha>` to dry-run or apply the generated rules against the upstream tree, rather than only checking that replacement `from` text exists. Require verification to fail unless each replacement produces exactly the expected match and change, then commit the validated rules with the existing message and signoff.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Line 27: Update the CVE/package extraction flow in the remediation skill to
resolve the ecosystem first, then apply that ecosystem’s package validation;
reject option-like tokens, traversal/path-like values, malformed scoped npm
names, and shell metacharacters with a warning. Validate the extracted CVE ID
before interpolating it into the gh --jq expression, and reject invalid tickets
before any downstream command uses the values.
- Around line 115-120: Extend the Cargo CVE remediation workflow after the
targeted update and lockfile verification to audit the complete dependency tree,
using cargo tree or an equivalent audit command. Confirm every instance of the
affected package is outside the advisory range, including transitive duplicate
versions, before declaring remediation complete.
---
Outside diff comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Around line 143-147: Update the rebase-rule verification instructions after
`/add-rebase-rules <commit-sha>` to dry-run or apply the generated rules against
the upstream tree, rather than only checking that replacement `from` text
exists. Require verification to fail unless each replacement produces exactly
the expected match and change, then commit the validated rules with the existing
message and signoff.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 71da2d50-45fa-4d41-baab-3154baf5b46f
📒 Files selected for processing (2)
.claude/skills/cve-remediation/SKILL.md.claude/skills/dependabot-cve-triage/SKILL.md
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
1 similar comment
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
.claude/skills/cve-remediation/SKILL.md (4)
67-67: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse npm commands that cover every installed dependency.
npm ls <package-name>omits packages outside the output selection and can hide transitive or deduplicated instances. Usenpm ls --allornpm explain --all <package-name>in each workspace so discovery and remediation check every dependency level.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/cve-remediation/SKILL.md at line 67, Update the dependency inspection step to use a command that traverses every installed dependency level in each workspace, such as npm ls --all or npm explain --all <package-name>, instead of npm ls <package-name>. Preserve the goal of identifying all affected versions and dependency chains for the target package.
79-85: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse an immutable production image reference for RPM triage.
registry.redhat.io/devspaces/code-rhel9:latestcan change without the production image tag changing, so the triage output may not match the deployed image. Use the SHA256 digest referenced by the production deployment when running the RPM checks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/cve-remediation/SKILL.md around lines 79 - 85, Update the production-image RPM triage command in the CVE remediation guidance to use the immutable SHA256 digest referenced by the production deployment instead of the mutable registry.redhat.io/devspaces/code-rhel9:latest tag. Preserve the existing package and version checks while making the image reference match the deployed production image exactly.
87-87: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftCheck RPM reachability for every production consumer.
The RPM path only checks Node linkage from one target. If other executables or user-facing subprocesses can load the RPM, do not mark the CVE as not exploitable with “Node.js not linked against it” alone. Use advisory-specific reachability evidence for all relevant production readers.
Also applies to: 142-145.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/cve-remediation/SKILL.md at line 87, Expand the RPM reachability checks in the CVE remediation guidance beyond the Node.js binary: identify every production executable and user-facing subprocess that could load the affected library, then gather advisory-specific linkage or feature/protocol evidence for each consumer before declaring the CVE not exploitable. Update the related guidance at the referenced RPM-check section while preserving the existing Node.js checks.
52-52: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftBuild a compatible version map for every CVE.
Consolidating by highest patched version within a major line does not satisfy npm manifest constraints, Cargo ranges, or RPM package names, so it can leave some CVEs in the group unremediated. Keep each affected and patched range; select one candidate version that satisfies every manifest and fixes every CVE, or route the group to manual review.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/cve-remediation/SKILL.md at line 52, Update the “Build the consolidated version map” guidance to preserve each CVE’s affected and patched ranges rather than consolidating by the highest patch within a major line. Require selecting a candidate version compatible with every manifest constraint and fixing every CVE; if no such version exists, route the group to manual review..claude/skills/dependabot-cve-triage/SKILL.md (2)
41-41: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate CVE identifiers before Jira interpolation.
Reject
cvevalues that are not^CVE-\d{4}-\d{4,}$before Step 3; onlynullis currently rejected. This value is used in JQL, labels, summary, description, andcustomfield_10667.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/dependabot-cve-triage/SKILL.md at line 41, Update the CVE validation step in the dependabot-cve-triage workflow to accept only identifiers matching ^CVE-\d{4}-\d{4,}$, rejecting null and all malformed values before Step 3. Ensure only the validated CVE identifier is used for Jira JQL, labels, summary, description, and customfield_10667 interpolation.
72-89: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not return “package not found” solely because
npm lsfails.If this skill runs where
node_modulesis unavailable or another dependency-tree error occurs,npm ls --all <package-name>can fail even thoughpackage.jsonorpackage-lock.jsoncontains the affected package. Add fallback checks against manifests/lockfiles, or make the installed dependency-tree precondition explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/skills/dependabot-cve-triage/SKILL.md around lines 72 - 89, Update Step 4’s package verification flow so a failed npm ls command is not treated as proof that the package is absent. Add fallback checks against relevant package.json and package-lock.json manifests, and only log “package not found” after those checks also find no matching package; alternatively, explicitly validate and document an installed node_modules precondition before relying on npm ls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Line 120: Update the lockfile regeneration step in the CVE remediation
playbook to use npm install with lifecycle scripts disabled via
--ignore-scripts, while still failing if the command exits non-zero. Keep
required audit or versioning commands isolated from Jira and GitHub credentials.
- Line 124: Update the Dependabot cross-check command in Step 5 to paginate all
alert results using gh api --paginate --slurp or equivalent page/per_page
iteration, while preserving the CVE and open-state filters and manifest filename
output. Ensure any request failure terminates the step immediately instead of
allowing an incomplete check to pass.
---
Outside diff comments:
In @.claude/skills/cve-remediation/SKILL.md:
- Line 67: Update the dependency inspection step to use a command that traverses
every installed dependency level in each workspace, such as npm ls --all or npm
explain --all <package-name>, instead of npm ls <package-name>. Preserve the
goal of identifying all affected versions and dependency chains for the target
package.
- Around line 79-85: Update the production-image RPM triage command in the CVE
remediation guidance to use the immutable SHA256 digest referenced by the
production deployment instead of the mutable
registry.redhat.io/devspaces/code-rhel9:latest tag. Preserve the existing
package and version checks while making the image reference match the deployed
production image exactly.
- Line 87: Expand the RPM reachability checks in the CVE remediation guidance
beyond the Node.js binary: identify every production executable and user-facing
subprocess that could load the affected library, then gather advisory-specific
linkage or feature/protocol evidence for each consumer before declaring the CVE
not exploitable. Update the related guidance at the referenced RPM-check section
while preserving the existing Node.js checks.
- Line 52: Update the “Build the consolidated version map” guidance to preserve
each CVE’s affected and patched ranges rather than consolidating by the highest
patch within a major line. Require selecting a candidate version compatible with
every manifest constraint and fixing every CVE; if no such version exists, route
the group to manual review.
In @.claude/skills/dependabot-cve-triage/SKILL.md:
- Line 41: Update the CVE validation step in the dependabot-cve-triage workflow
to accept only identifiers matching ^CVE-\d{4}-\d{4,}$, rejecting null and all
malformed values before Step 3. Ensure only the validated CVE identifier is used
for Jira JQL, labels, summary, description, and customfield_10667 interpolation.
- Around line 72-89: Update Step 4’s package verification flow so a failed npm
ls command is not treated as proof that the package is absent. Add fallback
checks against relevant package.json and package-lock.json manifests, and only
log “package not found” after those checks also find no matching package;
alternatively, explicitly validate and document an installed node_modules
precondition before relying on npm ls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1cd3d432-b6c5-4382-9a52-4cbed5bc6cb4
📒 Files selected for processing (2)
.claude/skills/cve-remediation/SKILL.md.claude/skills/dependabot-cve-triage/SKILL.md
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
…ickets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Replace grep-based package detection with npm ls --package-lock-only and jq structural checks to prevent substring false positives (e.g. form-data matching form-data-encoder). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
When bumping a parent dependency, existing overrides for its transitive deps may become redundant. Add Step 5c to detect and remove them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Prevents gh api failures from being masked by sort in the pipeline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
- Scope cross-source dedup by package name, not CVE alone - Use cargo tree -d + name@version for duplicate-version audits - Retain all package names per CVE group in dependabot triage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Ensures the digest reflects the current image rather than a potentially stale local copy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
- cargo tree -d: run separately and check exit code instead of hiding errors with 2>/dev/null in a pipeline - dependabot triage: group by (CVE, package) pair instead of CVE alone so multi-package advisories create separate tickets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
…heck - Move user approval step before link/comment/close actions in cross-source dedup to match the guardrail - Add ecosystem check to single-ticket dedup (not just package name) Aligns with same fixes applied to jetbrains-ide-dev-server. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
- Add early ecosystem detection (Step 1e) before cross-source dedup so ecosystem comparison is available before Step 4 - RPM scan: use set -e and explicit RPM_SCAN_COMPLETE marker to fail closed on incomplete scans - ldd check: resolve RPM-provided sonames via rpm -ql before comparing against ldd output (e.g. openssl-libs provides libssl.so.3) - npm lockfile: use exact jq match with --arg and check all resolved instances, not just tail -1 - Triage dedup: match by (CVE, package) pair, not CVE alone, so multi-package advisories create separate tickets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
…or package - Early ecosystem detection: infer from manifest/description metadata instead of package name pattern (avoids cross-ecosystem ambiguity e.g. openssl-libs matching npm naming) - Triage dedup: also check ticket description and labels for package name when summary doesn't clearly identify it Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
- Collect every matching installed version instead of keeping only the first (head -1), so the recorded version matches the vulnerable occurrence when a package appears multiple times in the tree. - Document the authenticated flow for recording the advisory link: use the authenticated addCommentToJiraIssue MCP tool as the primary path, and describe credential handling for the optional Jira REST remotelink. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
The per-probe `|| true` defeated `set -e`, so a genuine rpm error was swallowed and RPM_SCAN_COMPLETE was still emitted, making an incomplete scan look successful. Replace it with a scan() helper that tolerates ONLY a confirmed "not installed" result (on stdout or stderr, depending on the rpm build) and aborts on any other failure — signal/OOM kill, unexpected or error-prefixed message — so an incomplete scan can never reach the marker. LC_ALL=C keeps messages in English for reliable matching; the --version probe stays unconditional and the empty-output blank line is suppressed. Found by CodeRabbit CLI and refined by the code-review pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Step 4 (identify dependency source) runs before any npm install, so the version/chain probe must read the resolved tree from the lockfile rather than node_modules. Switch the bare `npm ls` to `npm ls --all --package-lock-only` (matching the dependabot-cve-triage skill) and point Step 5a at that output instead of implying a fresh bare `npm ls`. The verification-sequence `npm ls` (Step 5d) stays bare — it correctly runs after `npm install`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
A ticket sharing the CVE and package name but belonging to a different ecosystem (e.g. a ProdSec RPM ticket vs an npm Dependabot alert for a same-named package) could wrongly suppress ticket creation. Derive the group's ecosystem from its manifest paths, derive each candidate ticket's ecosystem from its summary/description/labels, and require an ecosystem match alongside CVE and package before skipping. Treat an unknown ecosystem on either side as a non-match so a real vulnerability is never dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
- Do not treat a non-zero `npm ls` exit as absence: ELSPROBLEMS from unrelated peer-dep conflicts can set a non-zero status while still listing the vulnerable package, which would drop a real CVE. Decide presence from the resolved tree instead of the exit code. - Quote the package name in the grep -oP pattern with \Q...\E so regex metacharacters (e.g. the dot in `lodash.merge`) match literally, and anchor with a `(?<= )` lookbehind so a short name can't match inside a longer one (e.g. `data` within `form-data`). Found by the code-review pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Step 2c resolves a Cargo ecosystem, but Step 4 only scanned npm manifests, so a Cargo-only alert was reported not-found and got no Jira ticket. Dispatch Step 4 by the resolved ecosystem: keep the npm scan (4a), add Cargo verification in code/cli via cargo tree/metadata (4b), and check both when the ecosystem is unknown (4c). Found by CodeRabbit CLI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Replace Step 7 (full validate-rebase-rules + security-review) with a targeted run of test-rebase-rules on the code/ files touched by the fix commit, and flag touched upstream files with no handler as failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
| fi | ||
| [ -n "$out" ] && printf "%s\n" "$out" | ||
| } | ||
| scan rpm -q <pkg> <pkg>-minimal lib<pkg> lib<pkg>-minimal |
There was a problem hiding this comment.
The RPM probe never checks the -libs suffix, which is the actual RHEL name for library packages — the skill's own example at line 278 is openssl-libs. On the ubi9-minimal production image only openssl-libs is installed, so every probe returns "not installed" and a vulnerable linked library gets classified as absent → "no fix needed"
wdyt?
| # `(?<= )` anchors to the tree indent so `data` can't match inside `form-data`, | ||
| # and `\Q...\E` quotes the name so regex metacharacters (the dot in `lodash.merge`) | ||
| # are matched literally. | ||
| versions=$(printf '%s\n' "$output" | grep -oP '(?<= )\Q<package-name>\E@\K[^ ]+' | sort -u | paste -sd ',' -) |
There was a problem hiding this comment.
The npm version extraction uses grep -oP, which BSD grep on macOS does not support — it fails with "illegal option -- p" inside the command substitution, $versions comes back empty for every workspace, and every high/critical runtime alert gets logged as "not found in codebase" and skipped, with no Jira ticket created.
At minimum this must not fail silently: on macOS a contributor can't tell a genuine "not present" from a broken probe, so the skill should detect the unsupported grep and abort with a clear error — and better yet, replace the PCRE grep with npm ls --json --package-lock-only + jq.
| ``` | ||
| 3. Run the rebase-rule test runner on exactly those files: | ||
| ```bash | ||
| bash .claude/skills/test-rebase-rules/run-all-tests.sh $FILES |
There was a problem hiding this comment.
This step run-all-tests.sh $FILES with no guard for an empty $FILES.
When the fix touches no code/ files or only package-lock.json files, $FILES expands to nothing and the runner interprets no arguments as test all files, launching the full audit that the Step 7 intro explicitly forbids (Do not run the full /validate-rebase-rules audit).
Please guard it: if $FILES is empty, skip the runner and report no rebase-rule-testable files in this fix instead.
| ``` | ||
| This lists **every** resolved instance of the package (there can be multiple at different nested paths). Verify that **all** resolved versions are outside the advisory's affected range — a single vulnerable copy is enough to remain exposed. Then compare the highest resolved version against the caret range in the `package.json` override (e.g. `^3.1.7`). If npm resolved a **higher** version than the base of the caret range (e.g. resolved `3.1.8` but override says `^3.1.7`), **update the override to use the resolved version** as the caret base (e.g. `^3.1.8`). This ensures the override and lockfile stay aligned — a lockfile pinning `3.1.8` while the override says `^3.1.7` creates drift where a future `npm install` could resolve differently. Apply the same resolved version across **all** workspaces for consistency. After updating overrides, re-run `npm install --ignore-scripts --no-fund --no-audit` to confirm the lockfile is stable (no further changes). | ||
| 3. **Review lockfile diffs for scope.** Compare the regenerated `package-lock.json` against the pre-change version. If unrelated dependencies changed (resolutions, metadata, or versions for packages not in the target's dependency chain), stop and investigate — do not commit lockfile changes that extend beyond the intended fix. Re-run with `--package-lock-only` if the full install pulled in unrelated tree changes. | ||
| 4. Run `npm ls <package-name>` in each workspace — confirm patched version, look for `overridden` markers — fail if the command exits non-zero |
There was a problem hiding this comment.
This step says fail if the command exits non-zero for npm ls <package-name>, but the skill's own Step 4a warns that npm ls exits non-zero on unrelated problems (peer-dep conflicts, incomplete tree) even when the target package is present. A pre-existing peer-dep conflict in a tree like this one makes npm ls exit 1 while still printing the patched version — so a verified healthy fix gets treated as failed and Steps 6–8 are blocked, or a correct fix is reverted.
Please base the pass criterion on the printed version / overridden markers rather than the exit code, or treat the exit code as advisory the way 4a does.
RomanNikitenko
left a comment
RomanNikitenko
left a comment
There was a problem hiding this comment.
#839 single-sources agent instructions and skills under .agents/ (with .claude/skills/* reduced to symlinks and CLAUDE.md becoming @AGENTS.md).
Please consider following the same layout here: place cve-remediation and dependabot-cve-triage under .agents/skills/ and expose them to Claude Code via .claude/skills/<name> symlinks, rather than adding new real directories under .claude/skills/.
What does this PR do?
this PR introduce two skills.
Needs JIRA MCP correclty configured and github access to security panel
What issues does this PR fix?
https://redhat.atlassian.net/browse/CRW-11156
How to test this PR?
Does this PR contain changes that override default upstream Code-OSS behavior?
git rebasewere added to the .rebase folderSummary by CodeRabbit