|
| 1 | +--- |
| 2 | +name: npm-audit |
| 3 | +description: Investigate and decide on npm audit vulnerabilities—fix vs. allowlist pattern with shared gate utility |
| 4 | +version: 1.0.0 |
| 5 | +--- |
| 6 | + |
| 7 | +# Handling NPM Audit Vulnerabilities |
| 8 | + |
| 9 | +## When to Use This Skill |
| 10 | + |
| 11 | +Use this skill when: |
| 12 | + |
| 13 | +- `npm audit --omit=dev --audit-level=moderate` is failing in the release gate or CI |
| 14 | +- You need to investigate why an npm override is not resolving a transitive dependency |
| 15 | +- You need to decide between fixing a vulnerability vs. allowlisting it |
| 16 | +- A PR proposes changes to `package.json` overrides or audit allowlists |
| 17 | + |
| 18 | +## ⚠️ CRITICAL: Allowlist Consistency Across All Release Scripts |
| 19 | + |
| 20 | +**When you add an advisory ID to the allowlist, you must update it in ONLY ONE place:** |
| 21 | + |
| 22 | +**`scripts/lib/check-audit-gate.sh`** — the shared audit gate utility used by all release scripts. |
| 23 | + |
| 24 | +This shared utility is called by: |
| 25 | + |
| 26 | +- `scripts/publish/bump-version.sh` |
| 27 | +- `scripts/publish/sync-develop-to-staging.sh` (passes allowlist as argument) |
| 28 | +- `scripts/publish/sync-develop-to-main.sh` (passes allowlist as argument) |
| 29 | + |
| 30 | +When you update the allowlist: |
| 31 | + |
| 32 | +1. **Do NOT** manually update each script individually |
| 33 | +2. **Do** edit the `ALLOWED_AUDIT_IDS` variable in the script that calls the utility (e.g., bump-version.sh passes it as an argument to the shared utility) |
| 34 | +3. All sync scripts receive the same allowlist from their hardcoded argument to the shared utility |
| 35 | + |
| 36 | +**If you see allowlist values in multiple sync scripts:** That's the correct pattern. Each script passes its repo's allowlist to the shared utility. They will update atomically if the shared utility changes. |
| 37 | + |
| 38 | +## Investigation Process |
| 39 | + |
| 40 | +### Step 1: Understand the Vulnerability |
| 41 | + |
| 42 | +```bash |
| 43 | +npm audit --omit=dev --json > temp/audit-report.json |
| 44 | +node --input-type=module -e " |
| 45 | + import fs from 'fs'; |
| 46 | + const audit = JSON.parse(fs.readFileSync('temp/audit-report.json', 'utf8')); |
| 47 | + for (const [pkg, data] of Object.entries(audit.vulnerabilities || {})) { |
| 48 | + if (Array.isArray(data.via)) { |
| 49 | + for (const adv of data.via) { |
| 50 | + if (typeof adv === 'object') { |
| 51 | + console.log(\`\${pkg}: \${adv.severity} - \${adv.title} (Advisory \${adv.source})\`); |
| 52 | + console.log(\` URL: \${adv.url}\`); |
| 53 | + console.log(\` Via: \${data.via}\`); |
| 54 | + } |
| 55 | + } |
| 56 | + } |
| 57 | + } |
| 58 | +" |
| 59 | +``` |
| 60 | + |
| 61 | +This shows: |
| 62 | + |
| 63 | +- Affected package name |
| 64 | +- Severity level |
| 65 | +- Advisory ID (source) |
| 66 | +- Direct URL to vulnerability details |
| 67 | +- **Via:** The chain of how this vulnerability reaches your code |
| 68 | + |
| 69 | +### Step 2: Trace the Dependency Chain |
| 70 | + |
| 71 | +For each vulnerability, understand the full path: |
| 72 | + |
| 73 | +```bash |
| 74 | +npm ls <vulnerable-package> --all |
| 75 | +``` |
| 76 | + |
| 77 | +Example: |
| 78 | + |
| 79 | +``` |
| 80 | +metaboost@1.0.0 |
| 81 | +└── (transitive) |
| 82 | + └── firebase-admin@13.8.0 |
| 83 | + └── @google-cloud/storage@7.19.0 |
| 84 | + └── teeny-request@9.0.0 |
| 85 | + └── uuid@9.0.1 (vulnerable) |
| 86 | +``` |
| 87 | + |
| 88 | +### Step 3: Determine Root Cause |
| 89 | + |
| 90 | +**Root cause types:** |
| 91 | + |
| 92 | +1. **Direct dependency outdated** |
| 93 | + - Package in your `package.json` is not at latest |
| 94 | + - **Fix:** `npm upgrade <package>` |
| 95 | + |
| 96 | +2. **Transitive dependency in resolvable chain** |
| 97 | + - Intermediate package has a recent version that upgrades the vulnerable dep |
| 98 | + - **Fix:** Upgrade the intermediate package, then re-audit |
| 99 | + - **Example:** firebase-admin@14.x might pull @google-cloud/storage@8.x which uses teeny-request@11.x |
| 100 | + |
| 101 | +3. **Transitive dependency in constrained chain** (most common) |
| 102 | + - Latest version of intermediate packages still pin the vulnerable dep |
| 103 | + - **Fix:** Evaluate if npm overrides can force a newer version |
| 104 | + - **Check:** |
| 105 | + ```bash |
| 106 | + # In package.json, try an override like: |
| 107 | + "overrides": { |
| 108 | + "uuid": "14.0.0", |
| 109 | + "teeny-request": { |
| 110 | + "uuid": "14.0.0" |
| 111 | + } |
| 112 | + } |
| 113 | + ``` |
| 114 | + - **Then:** `npm install` and re-audit |
| 115 | + |
| 116 | +4. **Nested optional dependency bug** (hardest case) |
| 117 | + - npm's resolver installs optional dependencies into their own node_modules folder |
| 118 | + - Overrides don't cascade into these nested folders in certain cases |
| 119 | + - **Symptom:** `package-lock.json` shows `node_modules/teeny-request/node_modules/uuid@9.0.1` even though you have `"uuid": "14.0.0"` in overrides |
| 120 | + - **Fix:** Either upgrade the parent package OR allowlist the advisory |
| 121 | + |
| 122 | +### Step 4: Decide Fix vs. Allowlist |
| 123 | + |
| 124 | +**Fix if:** |
| 125 | + |
| 126 | +- Direct dependency upgrade resolves it |
| 127 | +- A recent major version of an intermediate package resolves it |
| 128 | +- npm overrides can successfully force the resolution (verify in `package-lock.json`) |
| 129 | +- The vulnerability is directly exploitable in Metaboost's usage |
| 130 | +
|
| 131 | +**Allowlist if:** |
| 132 | +
|
| 133 | +- Latest mainstream versions of all upstream packages still carry the vulnerability |
| 134 | +- Fixing requires downgrading other critical packages (causes regressions) |
| 135 | +- The vulnerability is transitive-only and low-risk in Metaboost's deployment model |
| 136 | +- A clear path exists to remove the allowlist when upstream packages release fixes |
| 137 | + |
| 138 | +### Step 5: Document the Rationale |
| 139 | + |
| 140 | +If allowlisting, document **why** in a `docs/NPM-AUDIT-ALLOWLIST.md` or equivalent: |
| 141 | + |
| 142 | +```markdown |
| 143 | +### Advisory XXXXX: <vulnerability name> |
| 144 | +
|
| 145 | +**Affected chain:** pkg1 → pkg2 → pkg3 → vulnerable-pkg |
| 146 | +
|
| 147 | +**Why it's allowlisted:** |
| 148 | +- Latest pkg3@X.Y still pins vulnerable-pkg@<14 |
| 149 | +- Downgrading pkg1 causes regressions (list them) |
| 150 | +- Replacing pkg1 would require major refactor (explain scope) |
| 151 | +
|
| 152 | +**Risk level:** Transitive-only; not directly exploitable because [reason]. |
| 153 | +
|
| 154 | +### When to revisit: |
| 155 | +- When pkg1 releases X+1.0.0 with upgraded dependencies |
| 156 | +- When pkg3 releases Y+1.0.0 that drops the vulnerable dep |
| 157 | +``` |
| 158 | +
|
| 159 | +Then update the release gate: |
| 160 | +
|
| 161 | +```bash |
| 162 | +# See docs/NPM-AUDIT-ALLOWLIST.md for rationale |
| 163 | +ALLOWED_AUDIT_IDS="1113977,1116970" |
| 164 | +``` |
| 165 | +
|
| 166 | +### Step 6: Add to Memory for Revisit |
| 167 | +
|
| 168 | +Update `/memories/user/npm-audit-overrides.md` with: |
| 169 | +
|
| 170 | +```markdown |
| 171 | +## Tracked Overrides for Removal |
| 172 | +
|
| 173 | +- Advisory 1113977 (uuid): Remove when firebase-admin@14.x+ or @google-cloud/storage@8.x+ upgrades teeny-request |
| 174 | +- Advisory 1116970 (@tootallnate/once): Same dependency chain; will resolve when 1113977 resolves |
| 175 | +``` |
| 176 | +
|
| 177 | +## Common Patterns |
| 178 | +
|
| 179 | +### Pattern: npm Overrides Not Cascading |
| 180 | +
|
| 181 | +**Symptom:** Root `package.json` has `"uuid": "14.0.0"` override, but `npm audit` still reports uuid@9.0.1 in the output. |
| 182 | +
|
| 183 | +**Why:** Optional dependencies get their own node_modules folder, and npm doesn't always apply overrides there. |
| 184 | +
|
| 185 | +**Verification:** |
| 186 | +
|
| 187 | +```bash |
| 188 | +# Check the lockfile for nested node_modules |
| 189 | +grep -A2 'node_modules/teeny-request/node_modules/uuid' package-lock.json |
| 190 | +``` |
| 191 | + |
| 192 | +If you see a nested uuid entry with version < 14, the override didn't work. |
| 193 | + |
| 194 | +**Options:** |
| 195 | + |
| 196 | +1. Try pinning the parent package instead: `"teeny-request": "11.x"` |
| 197 | +2. Force a major version of the upstream: `"@google-cloud/storage": "8.0.0"` |
| 198 | +3. Accept the allowlist if neither works |
| 199 | + |
| 200 | +### Pattern: npm audit fix --force Causes Regressions |
| 201 | + |
| 202 | +**Symptom:** Running `npm audit fix --force` downgrades critical packages like firebase-admin@10.1.0. |
| 203 | + |
| 204 | +**Why:** `--force` prioritizes clearing vulnerabilities over semver compatibility. |
| 205 | + |
| 206 | +**Solution:** **Never use `--force` in monorepos.** Instead, carefully upgrade intermediate packages one at a time and check side effects. |
| 207 | + |
| 208 | +## Checklist Before Committing |
| 209 | + |
| 210 | +- [ ] Investigation documented: vulnerability chain traced and root cause identified |
| 211 | +- [ ] Attempted a fix: either package upgrade or npm override tested in package-lock.json |
| 212 | +- [ ] Verified no regression: `npm run build` succeeds |
| 213 | +- [ ] If allowlisting: documented rationale in appropriate docs file |
| 214 | +- [ ] If allowlisting: updated release gate with link to rationale docs |
| 215 | +- [ ] If allowlisting: added advisory ID to `/memories/user/npm-audit-overrides.md` for future revisit |
| 216 | +- [ ] LLM history updated with investigation results |
| 217 | + |
| 218 | +## See Also |
| 219 | + |
| 220 | +- `docs/` — Check for NPM-AUDIT-ALLOWLIST.md or equivalent |
| 221 | +- Release/publish gate — Audit gate implementation |
| 222 | +- LLM history — Investigation notes |
0 commit comments