Make audit:prod cross-platform so the release gate works on Windows - #20
Conversation
audit:prod used the POSIX-only "env -u npm_config_allow_scripts" prefix, which is not a command under Windows cmd.exe, so release:check and prepublishOnly fail there. CI is ubuntu-only so it never surfaced in the pipeline; it breaks a Windows contributor running the gate locally, and npm publish via prepublishOnly. Adopt the same Node helper the rest of the fleet now uses: it strips npm_config_allow_scripts from the environment, pins npm_config_userconfig to the platform null device, and spawns npm through a shell on win32 only. The shell flag is required rather than cosmetic: Node CVE-2024-27980 hardening refuses to execute .cmd files through spawn unless the shell option is set, so a helper spawning npm.cmd without it fails on Windows for a second, subtler reason. The argument vector is two constants with nothing interpolated, so enabling the shell introduces no injection surface. Not empirically verified on Windows - there is no Windows host here. POSIX behaviour is unchanged and verified: release:check passes end to end.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
WalkthroughThe ChangesProduction audit execution
Estimated code review effort: 2 (Simple) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR replaces the POSIX-only
Confidence Score: 5/5Safe to merge — the inline Node helper is a narrow, well-contained change that correctly addresses all three known failure modes (EALLOWSCRIPTS, missing env on Windows, Node's .cmd execution restriction post-CVE-2024-27980). The rewrite of audit:prod is logically correct: env vars are stripped via a case-insensitive scan, the null-device userconfig pin is idiomatic, the two hardcoded argument constants introduce no injection surface, and the fallback exit code of 1 on a null status is appropriate. The manifest version bump is consistent with package.json and the declared peer dependency range. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| package.json | audit:prod script replaced with a cross-platform Node.js inline helper; all design choices (case-insensitive env scrubbing, devNull userconfig pin, conditional shell flag) are correct and well-justified |
| manifest.json | pm_min_version bumped from 2026.7.20 to 2026.7.28, correctly aligned with the package version and peerDependencies declared in package.json |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[npm run audit:prod] --> B[Node ESM inline script starts]
B --> C[Spread process.env into new env object]
C --> D[Pin npm_config_userconfig to devNull]
D --> E[Case-insensitive scan: remove npm_config_allow_scripts]
E --> F{Windows?}
F -- Yes --> G[spawnSync npm.cmd with shell:true]
F -- No --> H[spawnSync npm with shell:false]
G --> I{r.status null?}
H --> I
I -- No --> J[process.exit with r.status]
I -- Yes --> K[process.exit with 1]
Reviews (4): Last reviewed commit: "fix(release): restore --ignore-scripts o..." | Re-trigger Greptile
manifest.json declared a pm_min_version older than the >=2026.7.28 peer requirement in package.json. The two disagreed about the floor, and the manifest promised support for a pm version this extension is neither built nor tested against - the activation tests run through the 2026.7.28 SDK harness. Set the manifest floor to 2026.7.28 so both declarations agree. Found by CodeRabbit on pm-presets; the same mismatch was present here.
The cross-platform helper was adopted from pm-ops, whose argv was ["audit", "--omit=dev"] -- so switching to it silently dropped the --ignore-scripts flag the previous env -u form passed. That is a reduction in the gate relative to what it replaced, not an intended change. Restore it, so the only difference from the previous form is the platform handling.
|
@greptileai Confirmed and fixed — upvoted, and you were right that this was a real reduction rather than a cosmetic difference. I adopted the cross-platform helper from pm-ops, whose argv was That is a good illustration of why "adopt the known-good form from a sibling repo" is not automatically safe: the reference implementation itself carried two defects — the missing Restored, so the only remaining difference from the previous form is platform handling: Swept across all 17 packages carrying the helper, including pm-ops itself. |
|
Good catch on tracing the omission back to the reference implementation itself — that's the riskier failure mode, since adopters have no reason to audit a "known-good" helper for correctness. The restored argv is now the right canonical form:
Sweeping pm-ops as the source of truth before any more packages adopt from it was the right call. Everything looks good here. |
What
audit:prodused the POSIX-onlyenv -u npm_config_allow_scriptsprefix.envis not a command under Windowscmd.exe, sorelease:checkandprepublishOnlyfail there with a shell error rather than an audit result.CI is
ubuntu-latestonly, so this never surfaced in the pipeline. It breaks a Windows contributor running the gate locally, andnpm publishviaprepublishOnly.Why the
env -uform existedThe script before it was a bare
npm audit --omit=dev, which aborts withEALLOWSCRIPTSwhenevernpm_config_allow_scriptsis present in the environment:So the gate failed for a reason unrelated to vulnerabilities.
env -ufixed that and traded it for the Windows problem. Each form fixed one platform and broke the other.The fix
Adopt the Node helper now used across the fleet: strip
npm_config_allow_scriptsfrom the environment, pinnpm_config_userconfigto the platform null device, and spawn npm through a shell on win32 only.The shell flag is required rather than cosmetic. Node's CVE-2024-27980 hardening (18.20.2 / 20.12.2 / 21.7.3+) refuses to execute
.cmdfiles throughspawnunless theshelloption is set, so a helper spawningnpm.cmdwithout it fails on Windows for a second, subtler reason. The argument vector is two constants (audit,--omit=dev) with nothing interpolated, so enabling the shell introduces no injection surface.npm audit --omit=devEALLOWSCRIPTSenv -u … npm audit …envis not a command.cmdshellon win32Verification
release:checkpasses end to end — typecheck, build, tests, production audit, pack dry-run, changelog check.Not empirically verified on Windows — there is no Windows host available here. This follows Node's documented
.cmdhandling; POSIX behaviour is unchanged and is verified.Found by Greptile on the 2026.7.28 adoption PRs for the starter templates, then swept across the fleet.
Summary by cubic
Make
audit:prodcross-platform sorelease:checkandprepublishOnlywork on Windows. Replace POSIX-onlyenv -uwith a Node helper that stripsnpm_config_allow_scripts, setsnpm_config_userconfigto the null device, spawnsnpmvia a shell on Windows, and keeps--ignore-scripts; also alignmanifest.jsonpm_min_versionto2026.7.28.Written for commit 004e876. Summary will update on new commits.