fix: harden CI npm installs with --ignore-scripts#252
Conversation
|
| Filename | Overview |
|---|---|
| .github/workflows/ci.yml | Adds --ignore-scripts to npm ci in frontend-build and sdk-ts jobs; frontend correctly adds npm rebuild esbuild, but sdk-ts job is missing the same step despite using tsup (esbuild-based) |
| .github/workflows/release-node-sdk.yml | Adds --ignore-scripts to npm ci; npm publish will still trigger prepublishOnly (tsup build via esbuild) without a prior npm rebuild esbuild, inconsistent with the frontend approach |
Comments Outside Diff (2)
-
.github/workflows/ci.yml, line 166-176 (link)Missing
npm rebuild esbuildbefore SDK build stepsThe
sdk-tsjob adds--ignore-scriptsbut never callsnpm rebuild esbuild. The SDK's build script (tsup) and test runner (vitest) both rely on esbuild for bundling and transformation. The frontend job explicitly addsnpm rebuild esbuildfor exactly this reason. If esbuild's postinstall is needed to locate or validate its binary on the runner,npm run buildandnpm testwill silently break here. The inconsistency between the two jobs (frontend rebuilds esbuild, SDK does not) suggests this step may have been overlooked. -
.github/workflows/release-node-sdk.yml, line 36-43 (link)prepublishOnlybuild uses tsup/esbuild with no priornpm rebuild esbuildAfter
npm ci --ignore-scripts,npm publishtriggersprepublishOnly: npm run build(declared inpackage.json), which runstsup— an esbuild-based bundler. If esbuild's native binary is not properly initialised (the same concern addressed bynpm rebuild esbuildin the frontend job), the publish step's implicit build will fail. Addingnpm rebuild esbuild(or an explicitnpm run build) beforenpm publishwould make this consistent with how the frontend job handles the same dependency.
Reviews (1): Last reviewed commit: "fix: harden CI npm installs with --ignor..." | Re-trigger Greptile
|
gonna check this out |
Summary
CI and the node-sdk release workflow now run
npm ci --ignore-scripts, so lifecycle scripts from transitive dependencies cannot execute during installs. The frontend build keeps working by explicitly rebuilding the one dependency that needs its install script (npm rebuild esbuild) beforevite build. This is item 3 of the hardening list in #108; the remaining items there are separate concerns and untouched.Type of change
Test plan
make test)Both workflow files parse as valid YAML. The typecheck/test/publish steps that follow each modified install only need declared dependencies, not lifecycle scripts; esbuild is the only install-script dependency the frontend build requires, and it is explicitly rebuilt. No new tests are needed for a workflow-only change.
Security checklist
Refs #108