-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
chore(llm): Add skill for fixing security vulnerabilities #19178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,232 @@ | ||
| --- | ||
| name: fix-security-vulnerability | ||
| description: Analyze and propose fixes for Dependabot security alerts | ||
| argument-hint: <dependabot-alert-url> | ||
| --- | ||
|
|
||
| # Fix Security Vulnerability Skill | ||
|
|
||
| Analyze Dependabot security alerts and propose fixes. **Does NOT auto-commit** - always presents analysis first and waits for user approval. | ||
|
|
||
| ## Input | ||
|
|
||
| - Dependabot URL: `https://github.com/getsentry/sentry-javascript/security/dependabot/1046` | ||
| - Or just the alert number: `1046` | ||
|
|
||
| ## Workflow | ||
|
|
||
| ### Step 1: Fetch Vulnerability Details | ||
|
|
||
| ```bash | ||
| gh api repos/getsentry/sentry-javascript/dependabot/alerts/<alert-number> | ||
| ``` | ||
|
|
||
| Extract: package name, vulnerable/patched versions, CVE ID, severity, description. | ||
|
|
||
| ### Step 2: Analyze Dependency Tree | ||
|
|
||
| ```bash | ||
| yarn why <package-name> | ||
| ``` | ||
|
|
||
| Determine if it's a direct or transitive dependency, and whether it's production or dev. | ||
|
|
||
| ### Step 3: Determine Fix Strategy | ||
|
|
||
| #### Check for version-specific test packages | ||
|
|
||
| Many packages in `dev-packages/e2e-tests/test-applications/` intentionally pin specific versions: | ||
|
|
||
| - `nextjs-13` - Tests Next.js 13.x, should NOT bump to 14 | ||
| - `remix-2` - Tests Remix 2.x specifically | ||
|
|
||
| **Do NOT bump these.** Recommend dismissing the alert with an explanation. | ||
|
|
||
| #### For other dependencies | ||
|
|
||
| | Type | Action | | ||
| | --------------------- | ----------------------------------- | | ||
| | Patch bump available | Preferred - lowest risk | | ||
| | Minor bump needed | Usually safe | | ||
| | Major bump needed | Analyze breaking changes first | | ||
| | Transitive dependency | Bump the parent package (see below) | | ||
|
|
||
| ### Step 3a: Transitive Dependencies | ||
|
|
||
| If the vulnerable package is pulled in by another package: | ||
|
|
||
| **1. Identify and check the parent:** | ||
|
|
||
| ```bash | ||
| yarn why <vulnerable-package> | ||
| npm view <parent-package>@latest dependencies.<vulnerable-package> | ||
| ``` | ||
|
|
||
| **2. Fix approach:** | ||
|
|
||
| | Scenario | Action | | ||
| | --------------------------------- | ------------------------------- | | ||
| | Parent has newer version with fix | **Bump the parent** | | ||
| | Parent hasn't released fix | Wait, or open an issue upstream | | ||
| | We control the parent | Fix in parent package first | | ||
|
|
||
| **AVOID RESOLUTIONS.** Using `resolutions` to force a transitive dependency version is risky - it can break the parent package silently. Only consider resolutions if: | ||
|
|
||
| - No upstream fix exists AND it's a production-critical vulnerability | ||
| - The forced version is a patch/minor bump (not major) | ||
| - You've manually verified compatibility | ||
|
|
||
| In most cases, it's better to wait for an upstream fix or accept the risk for dev-only dependencies than to use resolutions. | ||
|
|
||
| ### Step 4: Present Analysis | ||
|
|
||
| Present findings and **wait for user approval** before making changes: | ||
|
|
||
| ``` | ||
| ## Security Vulnerability Analysis | ||
|
|
||
| **Package:** <name> | **Severity:** <severity> | **CVE:** <id> | ||
| **Vulnerable:** <range> | **Patched:** <version> | ||
|
|
||
| ### Dependency Chain | ||
| <yarn why output> | ||
|
|
||
| ### Recommendation | ||
| <One of: Safe to bump / Version-specific test - do not bump / Bump parent package> | ||
|
|
||
| ### Proposed Fix | ||
| 1. Update <file>: "<package>": "<new-version>" | ||
| 2. yarn install && yarn dedupe-deps:fix | ||
| 3. Verify with: yarn why <package> | ||
|
|
||
| Proceed? | ||
| ``` | ||
|
|
||
| ### Step 5: Apply Fix (After Approval) | ||
|
|
||
| ```bash | ||
| # 1. Edit package.json | ||
| # 2. Update lockfile | ||
| yarn install | ||
| # 3. Deduplicate | ||
| yarn dedupe-deps:fix | ||
| # 4. Verify | ||
| yarn dedupe-deps:check | ||
| yarn why <package> | ||
| # 5. Show changes | ||
| git diff | ||
| ``` | ||
|
|
||
| **Do NOT commit** - let the user review first. | ||
|
|
||
| ### Step 5 (Alternative): Dismiss Alert | ||
|
|
||
| For alerts that should not be fixed (e.g., version-specific test packages), offer to dismiss instead. | ||
|
|
||
| **Always get user approval first.** Present the dismissal option: | ||
|
|
||
| ``` | ||
| This alert should be dismissed rather than fixed because: | ||
| - <reason: version-specific test / dev-only acceptable risk / etc.> | ||
|
|
||
| Dismiss with reason: <suggested reason> | ||
| Comment: "<suggested comment>" | ||
|
|
||
| Proceed with dismissal? | ||
| ``` | ||
|
|
||
| **After user approval**, dismiss via GitHub API: | ||
|
|
||
| ```bash | ||
| gh api --method PATCH repos/getsentry/sentry-javascript/dependabot/alerts/<number> \ | ||
| -f state=dismissed \ | ||
| -f dismissed_reason=<reason> \ | ||
| -f dismissed_comment="<comment>" | ||
| ``` | ||
|
|
||
| **Dismissal reasons:** | ||
|
|
||
| | Reason | When to use | | ||
| | ---------------- | -------------------------------------------- | | ||
| | `tolerable_risk` | Dev-only dependency, risk accepted | | ||
| | `no_bandwidth` | Will fix later, not urgent | | ||
| | `inaccurate` | False positive, not actually vulnerable | | ||
| | `not_used` | Vulnerable code path is not used in our code | | ||
|
|
||
| ## Commands Reference | ||
|
|
||
| | Command | Purpose | | ||
| | ------------------------------------------------------------------------------------------------- | ---------------------------- | | ||
| | `yarn why <pkg>` | Show dependency tree | | ||
| | `yarn dedupe-deps:fix` | Fix duplicates in yarn.lock | | ||
| | `yarn dedupe-deps:check` | Verify no duplicate issues | | ||
| | `gh api repos/getsentry/sentry-javascript/dependabot/alerts/<n>` | Fetch alert | | ||
| | `gh api --method PATCH .../dependabot/alerts/<n> -f state=dismissed -f dismissed_reason=<reason>` | Dismiss alert | | ||
| | `npm view <pkg>@latest dependencies.<dep>` | Check transitive dep version | | ||
|
|
||
| ## Examples | ||
|
|
||
| ### Dev dependency - safe to bump | ||
|
|
||
| ``` | ||
| Package: mongoose | ||
| Location: dev-packages/node-integration-tests/package.json | ||
| Type: Dev dependency (tests OTel instrumentation) | ||
|
|
||
| Recommendation: Safe to bump 5.x → 6.x | ||
| - Not version-specific, just tests instrumentation works | ||
| - OTel instrumentation supports mongoose 5.x-8.x | ||
| ``` | ||
|
|
||
| ### Version-specific test - dismiss instead | ||
|
|
||
| ``` | ||
| Package: next | ||
| Location: dev-packages/e2e-tests/test-applications/nextjs-13/package.json | ||
|
|
||
| Recommendation: DISMISS (do not bump) | ||
| This app specifically tests Next.js 13 compatibility. | ||
| Vulnerability only affects CI, not shipped code. | ||
|
|
||
| Proposed dismissal: | ||
| Reason: tolerable_risk | ||
| Comment: "Version-specific E2E test for Next.js 13 - intentionally pinned" | ||
|
|
||
| Proceed with dismissal? | ||
| ``` | ||
|
|
||
| ### Transitive dependency - bump parent | ||
|
|
||
| ``` | ||
| Package: vulnerable-lib@1.9.0 (needs >=2.0.1) | ||
| Chain: @sentry/node → @otel/instrumentation-foo@0.45.0 → vulnerable-lib | ||
|
|
||
| Check: npm view @otel/instrumentation-foo@latest dependencies.vulnerable-lib | ||
| Result: "^2.0.1" ✓ | ||
|
|
||
| Recommendation: Bump @otel/instrumentation-foo 0.45.0 → 0.47.0 | ||
| This pulls in the patched vulnerable-lib automatically. | ||
| ``` | ||
|
|
||
| ### Transitive dependency - no fix available | ||
|
|
||
| ``` | ||
| Package: deep-lib@2.9.0 (needs >=3.0.0) | ||
| Chain: @sentry/node → parent-pkg → middleware → deep-lib | ||
|
|
||
| No upstream fix available yet. Options: | ||
| 1. Wait for upstream fix (preferred) | ||
| 2. Accept risk if dev-only | ||
| 3. Consider alternative package if production-critical | ||
|
|
||
| AVOID using resolutions unless absolutely necessary. | ||
| ``` | ||
|
|
||
| ## Important Notes | ||
|
|
||
| - **Never auto-commit** - Always wait for user review | ||
| - **Version-specific tests should not be bumped** - They exist to test specific versions | ||
| - **Dev vs Prod matters** - Dev-only vulnerabilities are lower priority | ||
| - **Bump parents, not transitive deps** - If A depends on vulnerable B, bump A | ||
| - **Avoid resolutions** - They bypass the parent's dependency constraints and can cause subtle breakage | ||
| - **Always verify** - Run `yarn why <pkg>` after fixing to confirm the patched version is installed | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.