|
| 1 | +--- |
| 2 | +name: fix-security-vulnerability |
| 3 | +description: Analyze and propose fixes for Dependabot security alerts |
| 4 | +argument-hint: <dependabot-alert-url> |
| 5 | +--- |
| 6 | + |
| 7 | +# Fix Security Vulnerability Skill |
| 8 | + |
| 9 | +Analyze Dependabot security alerts and propose fixes. **Does NOT auto-commit** - always presents analysis first and waits for user approval. |
| 10 | + |
| 11 | +## Input |
| 12 | + |
| 13 | +- Dependabot URL: `https://github.com/getsentry/sentry-javascript/security/dependabot/1046` |
| 14 | +- Or just the alert number: `1046` |
| 15 | + |
| 16 | +## Workflow |
| 17 | + |
| 18 | +### Step 1: Fetch Vulnerability Details |
| 19 | + |
| 20 | +```bash |
| 21 | +gh api repos/getsentry/sentry-javascript/dependabot/alerts/<alert-number> |
| 22 | +``` |
| 23 | + |
| 24 | +Extract: package name, vulnerable/patched versions, CVE ID, severity, description. |
| 25 | + |
| 26 | +### Step 2: Analyze Dependency Tree |
| 27 | + |
| 28 | +```bash |
| 29 | +yarn why <package-name> |
| 30 | +``` |
| 31 | + |
| 32 | +Determine if it's a direct or transitive dependency, and whether it's production or dev. |
| 33 | + |
| 34 | +### Step 3: Determine Fix Strategy |
| 35 | + |
| 36 | +#### Check for version-specific test packages |
| 37 | + |
| 38 | +Many packages in `dev-packages/e2e-tests/test-applications/` intentionally pin specific versions: |
| 39 | +- `nextjs-13` - Tests Next.js 13.x, should NOT bump to 14 |
| 40 | +- `remix-2` - Tests Remix 2.x specifically |
| 41 | + |
| 42 | +**Do NOT bump these.** Recommend dismissing the alert with an explanation. |
| 43 | + |
| 44 | +#### For other dependencies |
| 45 | + |
| 46 | +| Type | Action | |
| 47 | +|------|--------| |
| 48 | +| Patch bump available | Preferred - lowest risk | |
| 49 | +| Minor bump needed | Usually safe | |
| 50 | +| Major bump needed | Analyze breaking changes first | |
| 51 | +| Transitive dependency | Bump the parent package (see below) | |
| 52 | + |
| 53 | +### Step 3a: Transitive Dependencies |
| 54 | + |
| 55 | +If the vulnerable package is pulled in by another package: |
| 56 | + |
| 57 | +**1. Identify and check the parent:** |
| 58 | +```bash |
| 59 | +yarn why <vulnerable-package> |
| 60 | +npm view <parent-package>@latest dependencies.<vulnerable-package> |
| 61 | +``` |
| 62 | + |
| 63 | +**2. Fix approach:** |
| 64 | + |
| 65 | +| Scenario | Action | |
| 66 | +|----------|--------| |
| 67 | +| Parent has newer version with fix | **Bump the parent** | |
| 68 | +| Parent hasn't released fix | Wait, or open an issue upstream | |
| 69 | +| We control the parent | Fix in parent package first | |
| 70 | + |
| 71 | +**AVOID RESOLUTIONS.** Using `resolutions` to force a transitive dependency version is risky - it can break the parent package silently. Only consider resolutions if: |
| 72 | +- No upstream fix exists AND it's a production-critical vulnerability |
| 73 | +- The forced version is a patch/minor bump (not major) |
| 74 | +- You've manually verified compatibility |
| 75 | + |
| 76 | +In most cases, it's better to wait for an upstream fix or accept the risk for dev-only dependencies than to use resolutions. |
| 77 | + |
| 78 | +### Step 4: Present Analysis |
| 79 | + |
| 80 | +Present findings and **wait for user approval** before making changes: |
| 81 | + |
| 82 | +``` |
| 83 | +## Security Vulnerability Analysis |
| 84 | +
|
| 85 | +**Package:** <name> | **Severity:** <severity> | **CVE:** <id> |
| 86 | +**Vulnerable:** <range> | **Patched:** <version> |
| 87 | +
|
| 88 | +### Dependency Chain |
| 89 | +<yarn why output> |
| 90 | +
|
| 91 | +### Recommendation |
| 92 | +<One of: Safe to bump / Version-specific test - do not bump / Bump parent package> |
| 93 | +
|
| 94 | +### Proposed Fix |
| 95 | +1. Update <file>: "<package>": "<new-version>" |
| 96 | +2. yarn install && yarn dedupe-deps:fix |
| 97 | +3. Verify with: yarn why <package> |
| 98 | +
|
| 99 | +Proceed? |
| 100 | +``` |
| 101 | + |
| 102 | +### Step 5: Apply Fix (After Approval) |
| 103 | + |
| 104 | +```bash |
| 105 | +# 1. Edit package.json |
| 106 | +# 2. Update lockfile |
| 107 | +yarn install |
| 108 | +# 3. Deduplicate |
| 109 | +yarn dedupe-deps:fix |
| 110 | +# 4. Verify |
| 111 | +yarn dedupe-deps:check |
| 112 | +yarn why <package> |
| 113 | +# 5. Show changes |
| 114 | +git diff |
| 115 | +``` |
| 116 | + |
| 117 | +**Do NOT commit** - let the user review first. |
| 118 | + |
| 119 | +## Commands Reference |
| 120 | + |
| 121 | +| Command | Purpose | |
| 122 | +|---------|---------| |
| 123 | +| `yarn why <pkg>` | Show dependency tree | |
| 124 | +| `yarn dedupe-deps:fix` | Fix duplicates in yarn.lock | |
| 125 | +| `yarn dedupe-deps:check` | Verify no duplicate issues | |
| 126 | +| `gh api repos/getsentry/sentry-javascript/dependabot/alerts/<n>` | Fetch alert | |
| 127 | +| `npm view <pkg>@latest dependencies.<dep>` | Check transitive dep version | |
| 128 | + |
| 129 | +## Examples |
| 130 | + |
| 131 | +### Dev dependency - safe to bump |
| 132 | + |
| 133 | +``` |
| 134 | +Package: mongoose |
| 135 | +Location: dev-packages/node-integration-tests/package.json |
| 136 | +Type: Dev dependency (tests OTel instrumentation) |
| 137 | +
|
| 138 | +Recommendation: Safe to bump 5.x → 6.x |
| 139 | +- Not version-specific, just tests instrumentation works |
| 140 | +- OTel instrumentation supports mongoose 5.x-8.x |
| 141 | +``` |
| 142 | + |
| 143 | +### Version-specific test - do not bump |
| 144 | + |
| 145 | +``` |
| 146 | +Package: next |
| 147 | +Location: dev-packages/e2e-tests/test-applications/nextjs-13/package.json |
| 148 | +
|
| 149 | +Recommendation: DO NOT BUMP |
| 150 | +This app specifically tests Next.js 13 compatibility. |
| 151 | +Dismiss the alert - vulnerability only affects CI, not shipped code. |
| 152 | +``` |
| 153 | + |
| 154 | +### Transitive dependency - bump parent |
| 155 | + |
| 156 | +``` |
| 157 | +Package: vulnerable-lib@1.9.0 (needs >=2.0.1) |
| 158 | +Chain: @sentry/node → @otel/instrumentation-foo@0.45.0 → vulnerable-lib |
| 159 | +
|
| 160 | +Check: npm view @otel/instrumentation-foo@latest dependencies.vulnerable-lib |
| 161 | +Result: "^2.0.1" ✓ |
| 162 | +
|
| 163 | +Recommendation: Bump @otel/instrumentation-foo 0.45.0 → 0.47.0 |
| 164 | +This pulls in the patched vulnerable-lib automatically. |
| 165 | +``` |
| 166 | + |
| 167 | +### Transitive dependency - no fix available |
| 168 | + |
| 169 | +``` |
| 170 | +Package: deep-lib@2.9.0 (needs >=3.0.0) |
| 171 | +Chain: @sentry/node → parent-pkg → middleware → deep-lib |
| 172 | +
|
| 173 | +No upstream fix available yet. Options: |
| 174 | +1. Wait for upstream fix (preferred) |
| 175 | +2. Accept risk if dev-only |
| 176 | +3. Consider alternative package if production-critical |
| 177 | +
|
| 178 | +AVOID using resolutions unless absolutely necessary. |
| 179 | +``` |
| 180 | + |
| 181 | +## Important Notes |
| 182 | + |
| 183 | +- **Never auto-commit** - Always wait for user review |
| 184 | +- **Version-specific tests should not be bumped** - They exist to test specific versions |
| 185 | +- **Dev vs Prod matters** - Dev-only vulnerabilities are lower priority |
| 186 | +- **Bump parents, not transitive deps** - If A depends on vulnerable B, bump A |
| 187 | +- **Avoid resolutions** - They bypass the parent's dependency constraints and can cause subtle breakage |
| 188 | +- **Always verify** - Run `yarn why <pkg>` after fixing to confirm the patched version is installed |
0 commit comments