Repository navigation
fix(deps): drop sprintf-js from the deploy CDK lockfiles - #721
Merged
Merged
Conversation
GHSA-hp3w-g68c-fv3c (sprintf-js <= 1.1.3, no patched release) reaches both deploy/cdk and deploy/cdk-constructs through jest's coverage plugin: babel-plugin-istanbul -> @istanbuljs/load-nyc-config -> js-yaml@3 -> argparse@1 -> sprintf-js. Every jest release, 30.5.2 included, still pulls load-nyc-config 1.1.0, which pins js-yaml ^3, so no direct upgrade removes it. Override js-yaml to 4.3.2 under @istanbuljs/load-nyc-config only. js-yaml 4 depends on argparse 2, which has no dependencies, so sprintf-js leaves both trees. load-nyc-config calls only yaml.load(), which js-yaml 4 keeps (safe schema by default).
Contributor
ASH Security Scan Report
Scan Metadata
SummaryScanner ResultsThe table below shows findings by scanner, with status based on severity thresholds and dependencies:
Report generated by Automated Security Helper (ASH) at 2026-10-06T03:51:18+00:00 |
…ssion Bump deploy/cdk to jest 30.5.2, @types/jest 30.0.0 and ts-jest 29.4.14, and deploy/cdk-constructs to ts-jest 29.4.14, so both packages run the same jest major. No jest config change was needed: 478 and 116 tests pass, coverage thresholds unchanged, synth:check and check:buildspec match. jest 30 no longer depends on micromatch, so braces leaves the deploy/cdk lockfile. Its GHSA-vfj7-8cjw-p6xm entry in .ash/.ash.yaml matched nothing afterwards and is removed; the deploy/cdk-constructs entry still matches (fast-glob and jsii-rosetta) and stays. jest 30 alone does not remove sprintf-js: babel-plugin-istanbul 8 still pulls @istanbuljs/load-nyc-config 1.1.0 and js-yaml 3. The js-yaml override from the previous commit is what keeps it out.
The hook sorts keys, so any commit touching deploy/*/package.json or package-lock.json had it rewrite the whole file into an order npm undoes on the next install. Exclude them, as the cdk synth templates already are, so lockfile commits no longer need the hook skipped.
awsmadi
marked this pull request as ready for review
October 6, 2026 15:15
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Every scan leg and
ash / SASTstarted failing withExiting due to 40 actionable findings. All 40 come from one new advisory: GHSA-hp3w-g68c-fv3c in sprintf-js (published 2026-09-24, affects<= 1.1.3, no patched version). The rest are thenpm-audit-transitive-*findings whose chains reach it.deploy/was unchanged, so main fails the same way.Root cause
Both
deploy/cdkanddeploy/cdk-constructspull sprintf-js in through jest's coverage instrumentation. It is a dev-only dependency.Upgrading jest alone does not remove it.
babel-plugin-istanbul@8.0.0, which jest 30.5.2 uses, still depends on@istanbuljs/load-nyc-config@^1. Its latest release, 1.1.0, pinsjs-yaml ^3.13.1. I checked this: after the jest 30 bump below, without the override, the lockfile still contains sprintf-js 1.0.3.Changes
"overrides": {"@istanbuljs/load-nyc-config": {"js-yaml": "4.3.2"}}(^4.3.2in cdk-constructs). js-yaml 4 depends onargparse@2, which has no dependencies, so sprintf-js, argparse 1 and esprima drop out of both lockfiles. load-nyc-config only callsrequire('js-yaml').load(), and only when a.nycrc.yamlexists; neither package has one. js-yaml 4 keepsload()and uses the safe schema by default. I loaded a sample.nycrc.yamlthrough the overridden tree in both packages and it parsed correctly.deploy/cdk/package-lock.json. TheGHSA-vfj7-8cjw-p6xmentry fordeploy/cdk/node_modules/braceswould have matched nothing, so it is removed and the block comment is updated. Thedeploy/cdk-constructsentry still matches (fast-glob and jsii-rosetta/jsii-pacmak pull in micromatch) and stays.The lockfiles were regenerated with npm 10.9.4 on Node 22.22.0.
Verification
deploy/cdk:npm run build;npm test(15 suites, 478 tests pass);npm run synth:check(templates/ matches a fresh synth (5 stacks)).deploy/cdk-constructs:npm run build(jsii, 0 errors);npm test(3 suites, 116 tests pass);npm run check:buildspec(3 buildspecs match).npm ls sprintf-jsis empty in both packages.ash --scanners npm-audit --build-target ci --mode localon clean exports. main gave 40 actionable findings (21 distinct rules). This branch gives 0 actionable findings and exits 0. No actionable finding was added. The only other change to the finding set is that the suppressednpm-audit-transitive-*rows for jest 29 packages in deploy/cdk are gone, because braces left that lockfile..ash/.ash.yaml:pytest tests/unit/config tests/unit/interactionsand the npm-audit transitive-suppression tests pass.npm-audit suppressions still in place
This PR adds no suppressions. It removes one dead entry. Each remaining entry and its upstream tracking:
The brace-expansion copy is inside aws-cdk-lib's bundle, so neither the lockfile nor
overridescan reach it. Every aws-cdk-lib release through 2.272.0 (the latest) still bundles 5.0.9.The brace-expansion entries keep their existing 2026-10-30 expiry on purpose. A later date was approved as a ceiling, not a minimum, and the earlier date forces a re-check of aws/aws-cdk#38929 sooner.
Pre-commit
pretty-format-jsonnow excludesdeploy/*/package.jsonanddeploy/*/package-lock.json, the same way it already excludes the cdk synth templates. The hook sorts keys, so a commit touching these npm-owned files rewrote them wholesale into an order thatnpm installthen undoes. The first two commits here, like #686, were made with the hook skipped. The third commit, which adds the exclude, passed the hook normally. Other JSON files are still formatted;deploy/cdk/tsconfig.json, for example, is still checked.