Uh oh!
There was an error while loading. Please reload this page.
fix: resolve high-severity npm audit vulnerabilities - #1184
Conversation
Run npm audit fix to address: - fast-xml-builder: attribute value quote bypass (high) - fast-uri: path traversal via percent-encoded dot segments (high, prod dep) - uuid: missing buffer bounds check (moderate) Remaining moderate vulnerabilities are in @aws-sdk transitive deps (fast-xml-parser < 5.7.0) which require upstream SDK updates.
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for the audit fix. The three high-severity bumps (fast-xml-builder, fast-uri, uuid) are legit — I verified each against the linked GHSAs and confirmed the top-level resolutions in the lockfile land on patched versions. All CI checks are green and the change is lockfile-only.
One substantive point worth addressing before merging:
The fast-xml-parser / @aws-sdk/xml-builder overrides in package.json (lines 151–159) are now stale and can be dropped as part of this PR
The overridesComments in package.json explicitly describe when each override should be removed:
fast-xml-parser→ "Remove once@aws-sdkupdates tofast-xml-parser >=5.5.7."@aws-sdk/xml-builder→ "Remove once@aws-sdk/client-*deps are bumped past 3.972.14."
Both conditions are now met by this PR's transitive bumps. Specifically, @aws-sdk/core goes from 3.974.6 → 3.974.8, which now requires @aws-sdk/xml-builder ^3.972.22. Published @aws-sdk/xml-builder@3.972.22 pins fast-xml-parser: 5.7.2, which in turn fixes the remaining medium advisory (GHSA-gh4j-gqv2-49f6, vulnerable range < 5.7.0).
Concrete consequences of leaving the overrides in place:
- Semver mismatch in the lockfile. Line 2148 of the new
package-lock.jsonrecords@aws-sdk/core@3.974.8as requiring"@aws-sdk/xml-builder": "^3.972.22", but the override pins the actually-installed tree to3.972.15(line 2892). It works becauseoverrideswin, but it's a real downgrade away from what@aws-sdk/coreexpects. - Residual medium CVE.
fast-xml-parser@5.5.7is still vulnerable to GHSA-gh4j-gqv2-49f6. The PR description attributes remaining moderates to upstream not having shipped yet — but the upstream fix has in fact shipped, and the override is the only thing preventing us from picking it up.
Options:
- (preferred) Remove both
fast-xml-parserand@aws-sdk/xml-builderentries from bothoverridesandoverridesCommentsinpackage.json, then regenerate the lockfile. This should pull in@aws-sdk/xml-builder@3.972.22andfast-xml-parser@5.7.2and fully clear the xml-related advisories (including the medium). - Or, if there's a reason to stay on these pinned versions for now, at least bump the
@aws-sdk/xml-builderoverride from3.972.15to3.972.22so it matches what@aws-sdk/core@3.974.8requests, and bumpfast-xml-parserfrom5.5.7to5.7.2to clear the medium advisory. The original justifications for the overrides are all satisfied at those versions. - Or, keep as-is and update the PR description to acknowledge the remaining medium comes from a stale in-repo override rather than unshipped upstream work — but this seems strictly worse than either option above.
Also a minor note for the description (non-blocking): the residual fast-uri@3.1.0 shows up as a bundled dep inside aws-cdk-lib (see node_modules/aws-cdk-lib/node_modules/fast-uri in the lockfile), not via @aws-sdk. It's filtered out by --omit=dev so it doesn't break the audit gate, but the description attributes it to the wrong place.
Uh oh!
There was an error while loading. Please reload this page.
Both override conditions are now met by upstream: - @aws-sdk/xml-builder@3.972.22 pins fast-xml-parser@5.7.2 - @aws-sdk/core@3.974.8 requires @aws-sdk/xml-builder ^3.972.22 Removing the overrides allows the natural resolution to pick up the patched versions, clearing the remaining moderate advisory (GHSA-gh4j-gqv2-49f6, fast-xml-parser < 5.7.0). security:audit now reports 0 vulnerabilities (--omit=dev).
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Thanks for following up on the override cleanup. Commit a46fe2d does exactly what the prior review asked for:
- Removes
fast-xml-parserand@aws-sdk/xml-builderfrom bothoverridesandoverridesCommentsinpackage.json(lines 150–157 now match the remaining override set). - Regenerates the lockfile so
@aws-sdk/xml-builder@3.972.22(what@aws-sdk/core@3.974.8actually requests) andfast-xml-parser@5.7.2resolve naturally, which also clears the residual medium advisory GHSA-gh4j-gqv2-49f6.
Verification on the new lockfile:
- Top-level
fast-xml-parser@5.7.2,fast-xml-builder@1.2.0,fast-uri@3.1.2,uuid@11.1.1,@aws-sdk/xml-builder@3.972.22,@aws-sdk/core@3.974.8— all patched against the linked GHSAs. - The only remaining
fast-uri@3.1.0copy is nested undernode_modules/aws-cdk-lib/node_modules/fast-uriand is markeddev: truein the lockfile, so it's correctly excluded bysecurity:audit(npm audit --audit-level=high --omit=dev). - No other overrides are stale —
minimatchandglobstill have the upstream pinning reasons described inoverridesComments.
Core CI on the latest SHA is green (lint, typecheck, security, format, unit-test 1/3, 2/3, 3/3, secrets, schema-check, CodeQL). The still-pending/failing entries in gh pr checks are either cancelled runs from the earlier commit or jobs still in progress.
One non-blocking nit (same as flagged in the previous review): the PR description still says "Remaining moderate vulnerabilities are in @aws-sdk transitive deps (fast-xml-parser < 5.7.0) which require upstream SDK updates" — this is now out of date since this PR does in fact pull in fast-xml-parser@5.7.2. Worth updating the description before merge, but not a code change.
LGTM.
Uh oh!
There was an error while loading. Please reload this page.
Description
Run
npm audit fixand remove stale overrides to resolve all production dependency vulnerabilities:fast-xml-parserand@aws-sdk/xml-builderoverrides, allowing natural resolution tofast-xml-parser@5.7.2via@aws-sdk/xml-builder@3.972.22The only remaining
fast-uri@3.1.0is bundled insideaws-cdk-lib(dev dependency), excluded by--omit=dev.npm run security:auditnow reports 0 vulnerabilities.Related Issue
N/A — security maintenance
Type of Change
Testing
How have you tested the change?
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.