Add CVE triage and remediation skills - #756
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 reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds two Claude skill playbooks: one for Jira-driven CVE remediation across npm, Cargo, and container RPMs, and one for triaging Dependabot alerts into deduplicated Jira vulnerability tickets. The remediation workflow includes advisory analysis, dependency updates, rebase rules, security review, and Jira transitions. The triage workflow filters alerts, verifies installed packages, creates Jira issues, links advisories, and reports results. Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Dependabot
participant TriageSkill
participant Jira
Dependabot->>TriageSkill: provide eligible grouped alerts
TriageSkill->>Jira: deduplicate CVEs and create vulnerability tickets
TriageSkill->>Jira: add advisory remote links or fallback comments
sequenceDiagram
participant Jira
participant RemediationSkill
participant AdvisorySources
participant Repository
Jira->>RemediationSkill: provide eligible CVE tickets
RemediationSkill->>AdvisorySources: retrieve affected and patched versions
RemediationSkill->>Repository: inspect and update dependencies
RemediationSkill->>Jira: transition remediated tickets
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
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: 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
| for dir in $(find . -name "package-lock.json" -not -path "*/node_modules/*" -not -path "*/.build/*" -not -path "*/vscode-reh-web-*" -not -path "*/.rebase/*" -not -path "*/.vscode/*" -exec dirname {} \;); do | ||
| result=$(cd "$dir" && npm ls <package-name> 2>/dev/null) | ||
| [ -n "$result" ] && echo "=== $dir ===" && echo "$result" | ||
| done | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target lines ---'
sed -n '1,140p' .claude/skills/dependabot-cve-triage/SKILL.md
echo '--- search for manifest discovery / npm ls usage ---'
rg -n 'package-lock\.json|package\.json|npm ls|npm ls <package-name>|find \.' .claude/skills/dependabot-cve-triage/SKILL.mdRepository: che-incubator/che-code
Length of output: 7238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '180,220p' .claude/skills/dependabot-cve-triage/SKILL.mdRepository: che-incubator/che-code
Length of output: 1255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
print("Need additional evidence on npm ls semantics; checking docs/search next.")
PYRepository: che-incubator/che-code
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
from collections import defaultdict
pkg_json_dirs = set()
pkg_lock_dirs = set()
for p in Path('.').rglob('package.json'):
if any(part in {'.git', 'node_modules', '.build', '.rebase', '.vscode'} for part in p.parts):
continue
pkg_json_dirs.add(str(p.parent))
for p in Path('.').rglob('package-lock.json'):
if any(part in {'.git', 'node_modules', '.build', '.rebase', '.vscode'} for part in p.parts):
continue
pkg_lock_dirs.add(str(p.parent))
only_json = sorted(pkg_json_dirs - pkg_lock_dirs)
only_lock = sorted(pkg_lock_dirs - pkg_json_dirs)
print("package.json dirs:", len(pkg_json_dirs))
print("package-lock.json dirs:", len(pkg_lock_dirs))
print("package.json-only dirs:", len(only_json))
for d in only_json[:50]:
print(" ", d)
print("package-lock-only dirs:", len(only_lock))
for d in only_lock[:50]:
print(" ", d)
PYRepository: che-incubator/che-code
Length of output: 2236
🌐 Web query:
npm ls exit status stdout missing package documentation
💡 Result:
The npm ls command is designed to print the dependency tree of an installed package to stdout [1][2]. Regarding its exit status, the command is intended for programmatic use and will exit with a non-zero status code (typically 1) if it detects dependency issues, such as missing, invalid, or unmet peer dependencies [3][4][5]. It is important to note the following nuances regarding this behavior: 1. Consistency and Evolution: While npm ls generally exits with a non-zero code when problems exist, its specific error-handling logic has evolved across versions [6][5][7]. For instance, older versions of npm sometimes exited with an error code for extraneous packages, whereas newer versions have been updated to treat extraneous packages as non-fatal, meaning they do not trigger a non-zero exit status [5][7]. 2. Handling Exit Status: Because npm ls is used to validate the state of the dependency graph, the non-zero exit code is considered intentional behavior [3]. If you need to run this command in a CI/CD environment or a script where you want to suppress the error code even when issues are found, you can use the OR operator (e.g., npm ls || true) to force a successful exit status [3]. 3. Official Documentation: The official npm documentation focuses on the command's functionality—listing packages and identifying missing, extraneous, or invalid dependencies—but often does not explicitly list every variation of exit codes in the primary command reference page [1][8][9]. Developers typically rely on the CLI's standard behavior of returning non-zero status codes for detected dependency problems to signal failures in build pipelines [3].
Citations:
- 1: https://docs.npmjs.com/cli/v12/commands/npm-ls/
- 2: https://manpages.debian.org/bullseye-backports/npm/npm-ls.1.en.html
- 3:
npm lsreturns an exit code of 1 if it finds unmet peer dependencies npm/npm#17624 - 4: `npm ls` should NOT return an exit code of 1 if it finds unmet peer dependencies npm/rfcs#456
- 5: [BUG]
npm lsfails if there are extraneous packages npm/cli#2664 - 6:
npm ls <pkgname>always exits in error (even when there are no ls problems) npm/cli#1979 - 7: npm/cli@42ca59e
- 8: https://docs.npmjs.com/cli/v9/commands/npm-ls
- 9: https://docs.npmjs.com/cli/v9/commands/npm-ls/
Scan manifest directories, not just package-lock.json trees (.claude/skills/dependabot-cve-triage/SKILL.md:75-79).
This skips package.json-only workspaces, and npm ls should be gated on its exit status rather than stdout because tree output alone doesn’t prove the package is present.
🤖 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 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.
| curl -s -o /dev/null -w "%{http_code}" -X POST \ | ||
| "https://redhat.atlassian.net/rest/api/3/issue/<ISSUE-KEY>/remotelink" \ | ||
| -u "$JIRA_USER:$JIRA_API_TOKEN" \ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not pass the Jira token in the curl command line.
-u "$JIRA_USER:$JIRA_API_TOKEN" places the expanded secret in the process arguments while curl runs, which can expose it through process inspection or command logging. Use a protected netrc/config file or another credential mechanism that does not put the token in argv, then remove temporary credential material.
🤖 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 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.
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
scan Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
… rebase rules Prefer parent-scoped cascading npm overrides over @major-scoped ones when fixing transitive CVEs. Update Step 6 to ask the user before running /add-rebase-rules. 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>
Remove the "unscoped" option and clarify that parent-scoped overrides should be used even for single-version cases. The @major scoping is an absolute last resort only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Document that `npm run prepare` should be used at the repo root and in launcher/ to regenerate lock files, compile, and run tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
After npm run prepare, verify that every modified package.json has its corresponding package-lock.json regenerated. Fall back to npm install in specific directories (build/, test/) if not. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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>
For transitive dependencies, analyze whether a newer version of the parent package pulls in a fixed transitive before falling back to overrides. Parent bumps are cleaner and more likely to align with upstream updates. 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>
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>
…nsitive deps Signed-off-by: Stephane Bouchet <sbouchet@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add state filter for single-alert Dependabot queries - Remove personal email from JIRA_USER example - Gate npm ls on exit status instead of stdout - Ensure GHSA ID is never a placeholder during CVE grouping - Add Jira comment for unresolved advisories to prevent cron retries Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Add detailed steps for checking production image packages, verifying feature/protocol availability, and providing context-aware Jira comments based on whether the CVE actually applies. 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>
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
| ### 5b. Add remote link to the advisory | ||
|
|
||
| After creating the ticket, add a remote link to the GitHub Security Advisory using the Jira REST API. The advisory URL follows the pattern `https://github.com/advisories/<GHSA-ID>`. | ||
|
|
||
| Use `curl` with a Jira API token (basic auth): | ||
|
|
||
| ```bash | ||
| curl -s -o /dev/null -w "%{http_code}" -X POST \ | ||
| "https://redhat.atlassian.net/rest/api/3/issue/<ISSUE-KEY>/remotelink" \ | ||
| -u "$JIRA_USER:$JIRA_API_TOKEN" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{ | ||
| "object": { | ||
| "url": "https://github.com/advisories/<GHSA-ID>", | ||
| "title": "GitHub Security Advisory: <GHSA-ID>", | ||
| "icon": { | ||
| "url16x16": "https://github.com/favicon.ico" | ||
| } | ||
| } | ||
| }' | ||
| ``` | ||
|
|
||
| - **`JIRA_USER`**: Jira account email (e.g. `user@redhat.com`) | ||
| - **`JIRA_API_TOKEN`**: Jira API token (generate at https://id.atlassian.com/manage-profile/security/api-tokens) | ||
|
|
||
| If the remote link creation fails (non-2xx response), fall back to adding a comment with `addCommentToJiraIssue`: | ||
| - **Cloud ID**: `redhat.atlassian.net` | ||
| - **Content format**: `markdown` | ||
| - **Body**: `GitHub Security Advisory: https://github.com/advisories/<GHSA-ID>` | ||
| - **Visibility**: `{"type": "group", "value": "Red Hat Employee"}` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate SKILL.md and related dependabot files =="
fd -a 'SKILL\.md$|dependabot|cve|triage' . | sed 's#^\./##' | head -80
echo
echo "== target file lines =="
if [ -f .claude/skills/dependabot-cve-triage/SKILL.md ]; then
cat -n .claude/skills/dependabot-cve-triage/SKILL.md | sed -n '120,195p'
else
echo "target file not found"
fi
echo
echo "== search ghsa/advisory URL handling =="
rg -n --hidden --glob '!**/.git/**' 'GHSA|advisories|addremote|remotelink|advisory|ghsa|CVE-only|cve-only|collect|find' .claude/skills/dependabot-cve-triage . 2>/dev/null | head -250Repository: che-incubator/che-code
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file relevant sections =="
cat -n .claude/skills/dependabot-cve-triage/SKILL.md | sed -n '1,180p'
echo
echo "== deterministic template substitution behavior for a missing GHSA/available advisory URL =="
python3 - <<'PY'
ghsa = None
advisory_url = "https://github.com/advisories/GHSA-real-id"
curl_body = r'''{
"object": {
"url": "https://github.com/advisories/GHSA-ID",
"title": "GitHub Security Advisory: GHSA-ID",
"icon": {
"url16x16": "https://github.com/favicon.ico"
}
}
}'''
# Simulating common shell/script substitution of literal "<GHSA-ID>" placeholders by an empty/null replacement.
url = "https://github.com/advisories/" + (ghsa or "")
comment = "GitHub Security Advisory: " + ("https://github.com/advisories/" + (ghsa or "") if ghsa else "")
print("placeholder_raw_url:", curl_body.split('"url": "')[1].split('"')[0])
print("substituted_url:", url)
print("substituted_comment:", comment)
print("uses_collected_advisory_url:", "https://github.com/advisories/GHSA-real-id" in [url, comment])
PYRepository: che-incubator/che-code
Length of output: 9477
Do not create advisory links from a missing GHSA ID.
CVE-only alerts are kept, and CVE groups can have no non-null ghsa while still collecting a non-null advisory_url. In Step 5b, use the collected advisory URL when available; otherwise skip the remote link and report that no advisory link was found.
🤖 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 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.
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
Add comprehensive workspace list (including launcher/ and code/test/*/), Dependabot cross-check step, EOVERRIDE warning for direct deps, and delegate rebase rule creation to /add-rebase-rules and validation to /validate-rebase-rules skills. 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>
Never add an npm override for a package that is already a direct dependency — the override is extraneous since npm resolves the direct dep version first. Bump the direct dep instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
Reduce verbosity while preserving all decision logic: - Replace hardcoded workspace table with dynamic find command - Collapse repeated podman/bash blocks into combined examples - Merge near-identical Jira comment templates - Deduplicate override guidance and Important notes - Remove redundant cross-references between sections Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Stephane Bouchet <sbouchet@redhat.com>
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
| Extract the **CVE ID** and **package name** (token after `code-rhel9:` and before the next `:`). | ||
|
|
||
| **Group tickets by package name.** Multiple CVEs often target the same package. Process them together — one branch, one version bump covers all. Collect all CVE IDs and Jira ticket keys per group, then proceed through Steps 3–8 once per group. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical
The existing package-input injection finding remains unresolved.
The workflow still directs the agent to interpolate Jira-derived package text into shell commands. Quoting the grep argument is insufficient when the value is substituted into the command text; command substitutions and option injection remain possible. Validate ecosystem-specific package names before use, then pass them via "$pkg"/--. Apply the same validation to CVE IDs used in the gh jq filter.
Also applies to: 58-60, 66-74, 107-111
🤖 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 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.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not group tickets by package name alone.
The same name can represent unrelated npm, Cargo, and RPM packages. Grouping before ecosystem resolution can merge independent vulnerabilities into one branch and version map, causing the wrong remediation to be applied. Resolve the ecosystem first, then group by an identity such as (ecosystem, package, workspace/image).
Proposed adjustment
-**Group tickets by package name.** Multiple CVEs often target the same package.
+Resolve each ticket's ecosystem first, then group only tickets sharing the
+same ecosystem, package, and workspace/image identity.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Extract the **CVE ID** and **package name** (token after `code-rhel9:` and before the next `:`). | |
| **Group tickets by package name.** Multiple CVEs often target the same package. Process them together — one branch, one version bump covers all. Collect all CVE IDs and Jira ticket keys per group, then proceed through Steps 3–8 once per group. | |
| Extract the **CVE ID** and **package name** (token after `code-rhel9:` and before the next `:`). | |
| Resolve each ticket's ecosystem first, then group only tickets sharing the | |
| same ecosystem, package, and workspace/image identity. |
🤖 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 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.
| For each ticket, fetch remote/web links using `getJiraIssueRemoteIssueLinks`. Look for: | ||
| 1. **GitHub Security Advisory** (preferred — contains patched version ranges) | ||
| 2. **CVE.org / NVD** (fallback) | ||
|
|
||
| Fetch the advisory page using WebFetch to extract affected version ranges, patched versions, and severity. If no link is found, use WebSearch. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict advisory fetching to trusted sources.
Jira remote links are untrusted input, but the workflow permits arbitrary URLs to be fetched and their contents consumed by the remediation agent. Allowlist GitHub Security Advisories, CVE.org, and NVD; reject other links and treat fetched content strictly as data, not instructions.
🤖 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 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.
| ### If NOT VULNERABLE: | ||
| Add a Jira comment: `Automated CVE scan: package "<package-name>" is present but installed version(s) are outside the vulnerable range. No fix needed.` (If not present at all, say so and note manual review may be needed for RPM packages.) Do NOT change the ticket status. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Gate Jira transitions by remediation outcome.
Step 8 unconditionally moves every grouped ticket to In Progress, contradicting the explicit “Do NOT change the ticket status” rules for not-vulnerable and RPM cases. Transition only tickets with a completed, validated code fix; leave those paths in their documented no-status-change outcome.
Proposed adjustment
-For each ticket in the group, get transitions ... to In Progress ...
+For each ticket with a completed and validated npm/Cargo code fix, get
+transitions ... to In Progress. Do not transition not-vulnerable, RPM, or
+unresolved-advisory tickets.Also applies to: 117-124, 150-152
🤖 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 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.
| #### 5c. Verification | ||
|
|
||
| 1. Run `npm install` in each modified workspace to regenerate lock files | ||
| 2. Run `npm ls <package-name>` in each workspace — confirm patched version, look for `overridden` markers | ||
| 3. Run `npm audit | grep -i "<package-name>"` to check for remaining vulnerabilities | ||
| 4. If any lock file didn't change, investigate whether the override was actually applied | ||
| 5. Cross-check with Dependabot: `gh api repos/che-incubator/che-code/dependabot/alerts --jq '.[] | select(.state=="open") | select(.security_advisory.cve_id=="<CVE-ID>") | .dependency.manifest_filename'` — every reported manifest must be covered |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
The npm verification path must fail closed.
npm audit | grep ... can hide an audit failure because the pipeline status may come from grep, and the instructions do not require stopping on failed install, listing, or audit commands. Require per-workspace verification with preserved exit status, and treat any failure or unverified lockfile as remediation failure.
🤖 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 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.
| ### If VULNERABLE — Rust/Cargo dependency fix: | ||
|
|
||
| Create a branch. For **direct** deps, bump the version in `code/cli/Cargo.toml`. For **transitive** deps, add a direct dependency with the patched version constraint. Verify with `cargo update && cargo check`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
The unrestricted Cargo update remains unsafe for targeted CVE fixes.
cargo update can rewrite unrelated dependencies in Cargo.lock. Update only the vulnerable crate to the selected patched version, then verify that exact resolution in Cargo.lock before continuing.
Proposed adjustment
-Verify with `cargo update && cargo check`.
+Run `cargo update -p <package-name> --precise <patched-version> && cargo check`,
+then confirm the patched version in `Cargo.lock`.🤖 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 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.
Reduce verbosity while preserving all decision logic: - Replace hardcoded workspace table with dynamic find command - Collapse repeated podman/bash blocks into combined examples - Merge near-identical Jira comment templates - Deduplicate override guidance and Important notes - Remove redundant cross-references between sections Co-Authored-By: Claude Opus 4.6 <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 |
1 similar comment
|
Pull Request images published ✨ Editor amd64: quay.io/che-incubator-pull-requests/che-code:pr-756-amd64 |
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