Skip to content

Add resolve-cve skill for automated security vulnerability resolution #11650

Merged
angle943 merged 5 commits intoopensearch-project:mainfrom
angle943:tmp-and-cve-resolution
Apr 2, 2026
Merged

Add resolve-cve skill for automated security vulnerability resolution #11650
angle943 merged 5 commits intoopensearch-project:mainfrom
angle943:tmp-and-cve-resolution

Conversation

@angle943
Copy link
Copy Markdown
Collaborator

@angle943 angle943 commented Apr 2, 2026

Description

Adds a comprehensive CVE resolution skill that automatically identifies and resolves security vulnerabilities in project dependencies.

Key Features:

  • Automated CVE discovery: Searches GitHub issues for open CVE reports
  • Smart remediation: Attempts multiple fix strategies (direct updates, lock manipulation, yarn resolutions)
  • Dynamic PR generation: Uses live .github/pull_request_template.md for future-proof PR descriptions
  • Multi-platform support: Available for both Claude Code and Kiro users
  • Clean workflow: Generates reports in gitignored tmp/ directory to prevent conflicts

What this skill does:

  1. Scans GitHub issues for CVE vulnerabilities
  2. Verifies which packages are actually affected in the codebase
  3. Attempts automated fixes using 4 escalating strategies
  4. Validates fixes with builds and audits
  5. Generates ready-to-copy PR descriptions with auto-extracted "closes [Rename] tile_map plugin #123" references

Files Added:

  • .claude/skills/resolve_cve.md - Main skill definition
  • .claude/skills/resolve-cve-README.md - Comprehensive user documentation
  • .claude/skills/README.md - Skills directory overview
  • .kiro/prompts/resolve_cve.md - Kiro compatibility layer

Issues Resolved

N/A - This is a new feature, not fixing existing issues

Screenshot

N/A - Command-line skill with no UI changes

Testing the changes

Skill Functionality Testing:

# Test Claude Code skill (if using Claude)
/resolve-cve --help

# Test file generation
/resolve-cve --cve_id CVE-2024-12345
ls tmp/  # Should show cve-pr-description.md

# Verify Kiro compatibility (if using Kiro)
# Load .kiro/prompts/resolve_cve.md in Kiro interface

Documentation Testing:

# Verify all documentation files exist and are readable
cat .claude/skills/resolve-cve-README.md
cat .claude/skills/README.md
cat .kiro/prompts/resolve_cve.md

GitIgnore Testing:

# Verify tmp/ files are properly ignored
echo "test" > tmp/test.txt
git status  # Should not show tmp/test.txt

# Verify .claude/settings.json is now ignored
git status  # Should not show .claude/settings.json

Changelog

  • feat: Add automated CVE resolution skill with multi-platform support

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Justin Kim <jungkm@amazon.com>
@github-actions github-actions bot added distinguished-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Apr 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit a23516c.

PathLineSeverityDescription
package.json189highThree new yarn resolutions added for the 'tmp' package via targeted paths (**/selenium-webdriver/tmp, **/@lhci/cli/tmp, **/external-editor/tmp). Per mandatory rule, all dependency/resolution changes must be flagged regardless of stated rationale. Maintainers should verify the target version (^0.2.4 → 0.2.5) is from the legitimate 'tmp' npm package and that no namespace hijacking or typosquatting is occurring.
yarn.lock22719highMultiple distinct tmp package entries (0.0.30, 0.0.33, 0.1.0) removed and consolidated into a single entry resolving all tmp@* specifiers to version 0.2.5. This is a significant lock file dependency change affecting multiple dependency chains. Mandatory flag: maintainers must verify the resolved artifact at https://registry.yarnpkg.com/tmp/-/tmp-0.2.5.tgz is authentic and uncompromised.
yarn.lock18467highThe os-tmpdir package alias was narrowed from 'os-tmpdir@^1.0.0, os-tmpdir@~1.0.1, os-tmpdir@~1.0.2' to just 'os-tmpdir@^1.0.0', silently dropping the ~1.0.1 and ~1.0.2 specifiers. This is a dependency resolution change and must be flagged. Any package previously resolved through those specifiers will now resolve differently; maintainers should verify no unintended version shift occurs.
.claude/settings.json5mediumClaude Code automation is granted Bash(yarn:*) and Bash(npm:*) permissions. These glob patterns allow executing any yarn or npm subcommand, including 'npm install ' or 'yarn add ', which could be exploited by a malicious prompt or skill to introduce unvetted dependencies or trigger postinstall scripts. The scope is broader than needed for the stated CVE remediation workflow.
.claude/skills/resolve_cve.md1lowAn automated skill is introduced that can fetch external GitHub content, modify package.json and yarn.lock, and run bootstrap/audit commands without human review of each step. If an attacker controls the GitHub issues being queried, they could craft a CVE issue with a malicious 'safe version' recommendation that the skill automatically applies. The skill's error handling instructs it to restore backups rather than abort, which may mask partial malicious changes.

The table above displays the top 10 most important findings.

Total: 5 | Critical: 0 | High: 3 | Medium: 1 | Low: 1


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@tomkdgun
Copy link
Copy Markdown
Contributor

tomkdgun commented Apr 2, 2026

@angle943 Have you consider use of https://www.npmjs.com/package/yarn-audit-fix ? The yarn don't have build in alternative for 'npm audit fix', but I think yarn-audit-fix tool is something to consider to be part of skill. Also I've seen many PRs created by dependabot, which in many cases delivery of them can address CVEs

ananzh
ananzh previously approved these changes Apr 2, 2026
@angle943
Copy link
Copy Markdown
Collaborator Author

angle943 commented Apr 2, 2026

@tomkdgun i wasn't familiar with yarn-audit-fix. I just tried running it but looks like it runs yarn install:

Installing deps update...
invoke yarn install --update-checksums
yarn install v1.22.19
$ scripts/use_node ./preinstall_check

WARNING: When installing dependencies, prefer `yarn osd bootstrap`

do you know how to configure it to have it run yarn osd bootstrap instead? If so feel free to create a PR based on it! I will keep it out of scope for this PR

Signed-off-by: Justin Kim <jungkm@amazon.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

PR Reviewer Guide 🔍

(Review updated until commit 24ce05b)

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Add resolve-cve Claude skill with documentation

Relevant files:

  • .claude/skills/resolve_cve.md
  • .claude/skills/resolve_cve_readme.md
  • .claude/skills/README.md

Sub-PR theme: Add Kiro compatibility layer for resolve-cve skill

Relevant files:

  • .kiro/prompts/resolve_cve.md

⚡ Recommended focus areas for review

Destructive Operations

The skill instructs to delete yarn.lock entries and run bootstrap without sufficient safeguards. The backup strategy is mentioned in Error Handling but the actual step-by-step process (Steps 2-4) does not explicitly instruct creating backups before modifications. If bootstrap fails mid-process, the project could be left in a broken state.

#### Strategy B: Lock File Manipulation (if transitive dependency)

1. If package is NOT in package.json (transitive dependency):
   - Delete vulnerable package entries from yarn.lock
   - Run `yarn osd bootstrap` to regenerate with latest versions
   - Check if vulnerability is resolved

#### Strategy C: Peer Dependency Resolution

1. If Strategy B fails due to peer dependency constraints:
   - Identify which dependency requires the vulnerable version
   - Delete that dependency's yarn.lock entries too
   - Run `yarn osd bootstrap` again
   - May need to update parent dependency version

#### Strategy D: Resolutions (last resort)

1. If other strategies fail:
   - Add yarn resolutions to package.json to force safe version
   - Use the least invasive resolution path possible
   - Example: If package "a" has a CVE and the vulnerable version comes from dependency "b", use a targeted resolution like `"**/b/a": "^3.5.3"` instead of a global `"a": "^3.5.3"`
   - Document why the resolution was needed and which dependency path required it

### Step 4: Verification

After each remediation attempt:

1. Run `yarn osd bootstrap` to ensure build succeeds
2. Run `yarn audit` to check if vulnerability is resolved
3. Run basic smoke tests if available
4. Verify no new vulnerabilities were introduced
Gitignore Mutation

The skill instructs appending tmp/ to .gitignore with echo "tmp/" >> .gitignore without checking if it already exists. This could result in duplicate entries in .gitignore on repeated runs.

mkdir -p tmp
echo "tmp/" >> .gitignore  # if not already present
Unverified GitHub Data

The skill fetches CVE details and issue numbers from GitHub issues and auto-populates "closes #[issue-number]" references without any validation step. If the GitHub issue data is incorrect or the issue number is misidentified, incorrect issue references could be added to PRs, potentially closing unrelated issues.

4. **Auto-extract GitHub Issue Numbers**: Parse CVE issue URLs and add `closes #[number]` entries

**Content Mapping Strategy**:

- **Description section**: Fill with CVE summary, affected packages, resolution strategy
- **Issues Resolved section**: Auto-populate with `closes #[issue-number]` from found CVE issues
Cross-Tool Dependency

The Kiro prompt delegates entirely to .claude/skills/resolve_cve.md for its instructions. This creates a hard dependency between two different AI tool ecosystems. If the Claude skill file is moved, renamed, or restructured, the Kiro prompt will silently fail or behave unexpectedly with no fallback.

**IMPORTANT**: Read and follow the comprehensive CVE resolution process defined in `.claude/skills/resolve_cve.md`. That file contains the complete, detailed instructions for:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Persistent review updated to latest commit 6fa150f

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

PR Code Suggestions ✨

Latest suggestions up to 24ce05b

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Prevent duplicate gitignore entries

Appending tmp/ to .gitignore without checking if it already exists can result in
duplicate entries over multiple runs. Use grep to check before appending to avoid
polluting the .gitignore file.

.claude/skills/resolve_cve.md [129-130]

 mkdir -p tmp
-echo "tmp/" >> .gitignore  # if not already present
+grep -qxF "tmp/" .gitignore || echo "tmp/" >> .gitignore
Suggestion importance[1-10]: 5

__

Why: The suggestion is valid - appending tmp/ to .gitignore without checking for duplicates could create multiple entries over repeated runs. The grep -qxF approach is a standard way to prevent this, though the impact is minor since duplicate .gitignore entries are harmless.

Low
Add explicit backup and restore commands

The skill instructs to create backups but provides no explicit backup commands in
the process steps. Without concrete backup commands, the AI agent may skip this step
or implement it inconsistently, leaving the project in a broken state if a
remediation strategy fails.

.claude/skills/resolve_cve.md [179-180]

-- Always backup original package.json and yarn.lock before making changes
-- If any step fails, restore backups and document the failure
+- Always backup original package.json and yarn.lock before making changes:
+  ```bash
+  cp package.json package.json.bak
+  cp yarn.lock yarn.lock.bak
+  ```
+- If any step fails, restore backups and document the failure:
+  ```bash
+  mv package.json.bak package.json
+  mv yarn.lock.bak yarn.lock
+  ```
Suggestion importance[1-10]: 5

__

Why: Adding explicit cp and mv commands for backup/restore makes the instructions more actionable for the AI agent. However, the README already mentions *.bak files in the safety features section, so this is a moderate improvement to consistency and clarity rather than a critical fix.

Low
Pin exact versions in security resolutions

Using a caret range (^) in yarn resolutions can still allow minor/patch upgrades to
a version that may reintroduce the vulnerability or break compatibility. For
security resolutions, pinning to an exact safe version is safer and more
predictable.

.claude/skills/resolve_cve.md [75-81]

 #### Strategy D: Resolutions (last resort)
 
 1. If other strategies fail:
    - Add yarn resolutions to package.json to force safe version
    - Use the least invasive resolution path possible
-   - Example: If package "a" has a CVE and the vulnerable version comes from dependency "b", use a targeted resolution like `"**/b/a": "^3.5.3"` instead of a global `"a": "^3.5.3"`
+   - Pin to an exact safe version in resolutions to prevent unintended upgrades
+   - Example: If package "a" has a CVE and the vulnerable version comes from dependency "b", use a targeted resolution like `"**/b/a": "3.5.3"` instead of a global `"a": "^3.5.3"`
Suggestion importance[1-10]: 4

__

Why: Pinning exact versions in security resolutions is a valid security practice, but using caret ranges (^) is also common and allows patch updates. The suggestion has merit but is a matter of security policy preference rather than a clear bug, and the impact is moderate.

Low

Previous suggestions

Suggestions up to commit 24ce05b
CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle missing PR template file gracefully

If .github/pull_request_template.md does not exist, this command will silently fail
or error, leaving tmp/cve-pr-description.md missing and causing downstream steps to
fail. Add a check to handle the case where the template file is absent.

.claude/skills/resolve_cve.md [136]

-cp .github/pull_request_template.md tmp/cve-pr-description.md
+if [ -f .github/pull_request_template.md ]; then
+  cp .github/pull_request_template.md tmp/cve-pr-description.md
+else
+  touch tmp/cve-pr-description.md
+  echo "<!-- PR template not found, fill in manually -->" > tmp/cve-pr-description.md
+fi
Suggestion importance[1-10]: 6

__

Why: This is a valid concern - if .github/pull_request_template.md doesn't exist, the cp command will fail and leave tmp/cve-pr-description.md missing, breaking downstream steps. Adding a fallback ensures the skill continues to function even when the template is absent.

Low
General
Prevent duplicate .gitignore entries

Appending tmp/ to .gitignore without checking if it already exists can result in
duplicate entries over multiple runs. Use grep to check before appending to avoid
polluting the .gitignore file.

.claude/skills/resolve_cve.md [130-131]

 mkdir -p tmp
-echo "tmp/" >> .gitignore  # if not already present
+grep -qxF "tmp/" .gitignore || echo "tmp/" >> .gitignore
Suggestion importance[1-10]: 5

__

Why: The suggestion is valid - appending tmp/ to .gitignore without checking for duplicates could cause repeated entries across multiple runs. The grep -qxF approach is a standard way to prevent this. However, since this is a documentation/skill file with bash examples rather than production code, the impact is moderate.

Low
Add explicit backup commands before modifications

The skill instructs to create backups but provides no explicit backup commands in
the process steps. Without concrete backup commands, the AI agent may skip this step
or implement it inconsistently, risking data loss if remediation fails. Add explicit
backup commands to the remediation steps.

.claude/skills/resolve_cve.md [179-180]

-- Always backup original package.json and yarn.lock before making changes
-- If any step fails, restore backups and document the failure
+- Always backup original package.json and yarn.lock before making changes:
+  ```bash
+  cp package.json package.json.bak
+  cp yarn.lock yarn.lock.bak
+  ```
+- If any step fails, restore backups and document the failure:
+  ```bash
+  mv package.json.bak package.json
+  mv yarn.lock.bak yarn.lock
+  ```
Suggestion importance[1-10]: 5

__

Why: The suggestion improves the skill by adding concrete backup commands to the error handling section, making it more actionable for the AI agent. The resolve_cve_readme.md already mentions *.bak files, so this aligns with the intended behavior. However, it's a documentation enhancement rather than a critical fix.

Low
Suggestions up to commit bc7973e
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent duplicate .gitignore entries on repeated runs

The command echo "tmp/" >> .gitignore will append tmp/ to .gitignore every time the
skill runs, potentially creating duplicate entries. Use a conditional check to only
add the entry if it doesn't already exist.

.claude/skills/resolve_cve.md [129-130]

 mkdir -p tmp
-echo "tmp/" >> .gitignore  # if not already present
+grep -qxF "tmp/" .gitignore || echo "tmp/" >> .gitignore
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that running echo "tmp/" >> .gitignore repeatedly will create duplicate entries. The fix using grep -qxF is a standard and correct approach to conditionally append only if the entry doesn't exist.

Low
Handle missing PR template file gracefully

If .github/pull_request_template.md does not exist (e.g., in a fork or after a
template rename), this cp command will silently fail or error, leaving
tmp/cve-pr-description.md missing and causing downstream steps to fail. Add a
fallback to create a minimal template if the source file is absent.

.claude/skills/resolve_cve.md [136]

-cp .github/pull_request_template.md tmp/cve-pr-description.md
+if [ -f .github/pull_request_template.md ]; then
+  cp .github/pull_request_template.md tmp/cve-pr-description.md
+else
+  echo "# CVE Resolution PR\n\n## Description\n\n## Issues Resolved\n\n## Testing\n\n## Changelog\n" > tmp/cve-pr-description.md
+fi
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a potential failure point if .github/pull_request_template.md doesn't exist. The fallback template approach is reasonable, though this is an edge case for a project-specific skill where the template is expected to exist.

Low
General
Add explicit backup and restore commands for safety

The skill instructs to backup package.json and yarn.lock but provides no concrete
commands or location for where these backups should be stored. Without explicit
backup commands, the AI agent may skip this step or store backups inconsistently,
risking data loss if restoration is needed.

.claude/skills/resolve_cve.md [179-180]

-- Always backup original package.json and yarn.lock before making changes
-- If any step fails, restore backups and document the failure
+- Always backup original package.json and yarn.lock before making changes:
+  ```bash
+  cp package.json tmp/package.json.bak
+  cp yarn.lock tmp/yarn.lock.bak
+  ```
+- If any step fails, restore backups and document the failure:
+  ```bash
+  cp tmp/package.json.bak package.json
+  cp tmp/yarn.lock.bak yarn.lock
+  ```
Suggestion importance[1-10]: 4

__

Why: The suggestion adds concrete backup/restore commands which improves clarity for the AI agent executing the skill. However, since this is a documentation/instruction file for an AI agent rather than executable code, the impact is moderate.

Low
Suggestions up to commit 5a11efa
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent duplicate .gitignore entries on repeated runs

The echo "tmp/" >> .gitignore command will append tmp/ to .gitignore every time the
skill runs, creating duplicate entries. Use a conditional check to only add the
entry if it doesn't already exist.

.claude/skills/resolve_cve.md [129-130]

 mkdir -p tmp
-echo "tmp/" >> .gitignore  # if not already present
+grep -qxF 'tmp/' .gitignore || echo "tmp/" >> .gitignore
Suggestion importance[1-10]: 7

__

Why: The echo "tmp/" >> .gitignore command will indeed append a duplicate entry every time the skill runs. The fix using grep -qxF is correct and prevents this issue, which is a real functional problem for repeated executions.

Medium
General
Handle missing PR template file gracefully

If .github/pull_request_template.md does not exist (e.g., in a fork or after a
template rename), this command will silently fail or error, leaving
tmp/cve-pr-description.md missing or empty. Add a fallback to create a minimal
template when the file is absent.

.claude/skills/resolve_cve.md [136]

-cp .github/pull_request_template.md tmp/cve-pr-description.md
+if [ -f .github/pull_request_template.md ]; then
+  cp .github/pull_request_template.md tmp/cve-pr-description.md
+else
+  echo "# CVE Resolution PR\n\n## Description\n\n## Issues Resolved\n\n## Testing\n\n## Changelog\n" > tmp/cve-pr-description.md
+fi
Suggestion importance[1-10]: 5

__

Why: Adding a fallback for a missing .github/pull_request_template.md is a valid robustness improvement, preventing silent failures when the template doesn't exist. However, this is a documentation/skill file for an AI agent, so the impact is moderate.

Low
Add explicit backup and restore commands for safety

The skill instructs to backup package.json and yarn.lock but provides no concrete
commands or location for these backups. Without explicit backup commands, the AI
agent may skip this step or store backups inconsistently, risking data loss if
restoration is needed. Add explicit backup commands, for example storing them in
tmp/.

.claude/skills/resolve_cve.md [179-180]

-- Always backup original package.json and yarn.lock before making changes
-- If any step fails, restore backups and document the failure
+- Always backup original package.json and yarn.lock before making changes:
+  ```bash
+  cp package.json tmp/package.json.bak
+  cp yarn.lock tmp/yarn.lock.bak
+  ```
+- If any step fails, restore backups and document the failure:
+  ```bash
+  cp tmp/package.json.bak package.json
+  cp tmp/yarn.lock.bak yarn.lock
+  ```
Suggestion importance[1-10]: 4

__

Why: The suggestion adds concrete backup/restore commands which improves clarity, but this is a documentation/guidance file for an AI agent, and the lack of explicit commands is a minor issue since the agent can infer reasonable backup locations from context.

Low
Suggestions up to commit 6fa150f
CategorySuggestion                                                                                                                                    Impact
Security
Pin exact versions in security resolutions

Using a caret (^) in yarn resolutions (e.g., "^3.5.3") can allow yarn to resolve to
a version higher than intended, potentially introducing new vulnerabilities or
breaking changes. For security-critical resolutions, pin to an exact version to
guarantee the safe version is used.

.claude/skills/resolve_cve.md [75-80]

 #### Strategy D: Resolutions (last resort)
 
 1. If other strategies fail:
    - Add yarn resolutions to package.json to force safe version
    - Use the least invasive resolution path possible
-   - Example: If package "a" has a CVE and the vulnerable version comes from dependency "b", use a targeted resolution like `"**/b/a": "^3.5.3"` instead of a global `"a": "^3.5.3"`
+   - Example: If package "a" has a CVE and the vulnerable version comes from dependency "b", use a targeted resolution like `"**/b/a": "3.5.3"` (exact version, no caret) instead of a global `"a": "3.5.3"`
Suggestion importance[1-10]: 7

__

Why: Using caret (^) in yarn resolutions for security fixes is a valid concern — it could allow resolution to unintended versions. Pinning exact versions in security-critical contexts is a better practice and the suggestion is accurate and relevant.

Medium
Possible issue
Prevent duplicate .gitignore entries

The command echo "tmp/" >> .gitignore will append tmp/ to .gitignore every time the
skill runs, even if it already exists, causing duplicate entries. Use grep to check
before appending to avoid polluting the .gitignore file.

.claude/skills/resolve_cve.md [129-130]

 mkdir -p tmp
-echo "tmp/" >> .gitignore  # if not already present
+grep -qxF "tmp/" .gitignore || echo "tmp/" >> .gitignore
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that echo "tmp/" >> .gitignore will append duplicate entries on repeated runs. The fix using grep -qxF is a standard and correct approach to prevent this issue.

Low
General
Add explicit backup and restore commands

The skill instructs to backup package.json and yarn.lock but provides no concrete
commands or location for where these backups should be stored. Without explicit
backup commands, the AI agent may skip this step or store backups inconsistently,
risking unrecoverable project state on failure. Add explicit backup commands
pointing to the tmp/ directory.

.claude/skills/resolve_cve.md [179-180]

-- Always backup original package.json and yarn.lock before making changes
-- If any step fails, restore backups and document the failure
+- Always backup original package.json and yarn.lock before making changes:
+  ```bash
+  cp package.json tmp/package.json.bak
+  cp yarn.lock tmp/yarn.lock.bak
+  ```
+- If any step fails, restore backups and document the failure:
+  ```bash
+  cp tmp/package.json.bak package.json
+  cp tmp/yarn.lock.bak yarn.lock
+  ```
Suggestion importance[1-10]: 5

__

Why: Adding explicit backup commands improves clarity and reduces ambiguity for the AI agent executing the skill. However, since this is a documentation/instruction file for an AI agent rather than executable code, the impact is moderate.

Low
Suggestions up to commit 6fa150f
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent duplicate .gitignore entries

The command echo "tmp/" >> .gitignore will append tmp/ to .gitignore every time the
skill runs, even if it already exists, resulting in duplicate entries. Use grep to
check before appending to avoid this issue.

.claude/skills/resolve_cve.md [129-130]

 mkdir -p tmp
-echo "tmp/" >> .gitignore  # if not already present
+grep -qxF 'tmp/' .gitignore || echo "tmp/" >> .gitignore
Suggestion importance[1-10]: 6

__

Why: The echo "tmp/" >> .gitignore command will append a duplicate entry every time the skill runs. Using grep -qxF to check before appending is a valid improvement that prevents .gitignore pollution.

Low
General
Guard against missing PR template file

If .github/pull_request_template.md does not exist, this cp command will silently
fail or error, leaving tmp/cve-pr-description.md missing and causing downstream
steps to fail. Add a check to handle the case where the template file is absent.

.claude/skills/resolve_cve.md [136]

-cp .github/pull_request_template.md tmp/cve-pr-description.md
+if [ -f .github/pull_request_template.md ]; then
+  cp .github/pull_request_template.md tmp/cve-pr-description.md
+else
+  touch tmp/cve-pr-description.md
+  echo "<!-- PR template not found, fill in manually -->" > tmp/cve-pr-description.md
+fi
Suggestion importance[1-10]: 5

__

Why: If .github/pull_request_template.md doesn't exist, the cp command will fail and break the workflow. Adding a conditional check with a fallback is a valid defensive improvement, though in practice this template file should always exist in the OpenSearch Dashboards repo.

Low
Add explicit backup and restore commands

The skill instructs to backup package.json and yarn.lock but provides no concrete
commands or location for where these backups should be stored. Without explicit
backup commands, the AI agent may skip this step or store backups inconsistently,
risking data loss on failure. Add explicit backup commands to the setup step,
storing them in tmp/.

.claude/skills/resolve_cve.md [179-180]

-- Always backup original package.json and yarn.lock before making changes
-- If any step fails, restore backups and document the failure
+- Always backup original package.json and yarn.lock before making changes:
+  ```bash
+  cp package.json tmp/package.json.bak
+  cp yarn.lock tmp/yarn.lock.bak
+  ```
+- If any step fails, restore backups and document the failure:
+  ```bash
+  cp tmp/package.json.bak package.json
+  cp tmp/yarn.lock.bak yarn.lock
+  ```
Suggestion importance[1-10]: 4

__

Why: The suggestion adds concrete backup/restore commands which improves clarity for the AI agent following these instructions. However, since this is a markdown skill file guiding an AI agent (not executable code), the impact is moderate - the agent may infer reasonable backup locations without explicit commands.

Low

@angle943 angle943 changed the title [Security] resolve tmp CVE as well as add a skill to resolve cves Add resolve-cve skill for automated security vulnerability resolution Apr 2, 2026
@github-actions github-actions bot removed the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Apr 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Persistent review updated to latest commit 5a11efa

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Persistent review updated to latest commit bc7973e

LDrago27
LDrago27 previously approved these changes Apr 2, 2026
@LDrago27
Copy link
Copy Markdown
Collaborator

LDrago27 commented Apr 2, 2026

@angle943 Should we add a check to only address the High and Critical CVEs. For the auto scanning of CVEs I feel there may be a lot of minor ones which can crowd the fix.

@angle943
Copy link
Copy Markdown
Collaborator Author

angle943 commented Apr 2, 2026

@LDrago27 we currently only have like 5 minor ones, and ideally i want that number to be 0. if we are persistent w/ this, i think its fine to also resolve minor ones

Signed-off-by: Justin Kim <jungkm@amazon.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Persistent review updated to latest commit 24ce05b

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Persistent review updated to latest commit 24ce05b

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

✅ All unit and integration tests passing

🔗 Workflow run · commit 24ce05b4452d9e427b6373ad2be6386ae677b9e6

@angle943 angle943 merged commit 315f5aa into opensearch-project:main Apr 2, 2026
179 of 180 checks passed
@angle943 angle943 deleted the tmp-and-cve-resolution branch April 2, 2026 20:58
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (7782a80) to head (24ce05b).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #11650       +/-   ##
===========================================
- Coverage   61.58%        0   -61.59%     
===========================================
  Files        4995        0     -4995     
  Lines      137542        0   -137542     
  Branches    23901        0    -23901     
===========================================
- Hits        84707        0    -84707     
+ Misses      46692        0    -46692     
+ Partials     6143        0     -6143     
Flag Coverage Δ
Linux_1 ?
Linux_2 ?
Linux_3 ?
Linux_4 ?
Linux_5 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants