Skip to content

fix: harden CI npm installs with --ignore-scripts#252

Open
mvanhorn wants to merge 1 commit into
Infisical:mainfrom
mvanhorn:fix/108-npm-ignore-scripts-ci
Open

fix: harden CI npm installs with --ignore-scripts#252
mvanhorn wants to merge 1 commit into
Infisical:mainfrom
mvanhorn:fix/108-npm-ignore-scripts-ci

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

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) before vite build. This is item 3 of the hardening list in #108; the remaining items there are separate concerns and untouched.

Type of change

  • CI / build

Test plan

  • Existing tests pass (make test)
  • Manual testing (describe below)

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

  • No secrets or credentials in code
  • No new unauthenticated endpoints
  • Input validation on new API surfaces
  • Checked for OWASP top 10 (injection, XSS, etc.)

Refs #108

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens CI and the Node SDK release workflow against supply-chain attacks by adding --ignore-scripts to all npm ci invocations, preventing transitive dependency lifecycle scripts from executing during installs. The frontend build recovers the one known install-script dependency (esbuild) with an explicit npm rebuild esbuild before vite build.

  • ci.yml frontend job: npm ci --ignore-scripts && npm rebuild esbuild && npm run build — correctly isolated.
  • ci.yml sdk-ts job and release-node-sdk.yml: npm ci --ignore-scripts added, but no npm rebuild esbuild step, despite both jobs running tsup (an esbuild-based bundler) and vitest (which also uses esbuild). The SDK's prepublishOnly script runs tsup implicitly during npm publish, introducing the same esbuild-binary dependency that the frontend job explicitly handles.

Confidence Score: 4/5

Safe to merge with low risk; the hardening intent is correct and the frontend change is properly handled, but the SDK CI and release jobs may silently fail if esbuild's binary setup requires its install script.

The frontend job is correctly hardened. The SDK CI job and release workflow are inconsistent — both run tsup (and vitest), which depend on esbuild, without the npm rebuild esbuild step that the frontend job uses to restore esbuild's binary after --ignore-scripts. Whether this causes failures depends on the exact esbuild version and npm behaviour on the runner, but the inconsistency is a real gap that could surface as a broken SDK build or publish.

.github/workflows/ci.yml (sdk-ts job) and .github/workflows/release-node-sdk.yml both need a npm rebuild esbuild step added after npm ci --ignore-scripts to match the pattern used in the frontend-build job.

Important Files Changed

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)

  1. .github/workflows/ci.yml, line 166-176 (link)

    P2 Missing npm rebuild esbuild before SDK build steps

    The sdk-ts job adds --ignore-scripts but never calls npm rebuild esbuild. The SDK's build script (tsup) and test runner (vitest) both rely on esbuild for bundling and transformation. The frontend job explicitly adds npm rebuild esbuild for exactly this reason. If esbuild's postinstall is needed to locate or validate its binary on the runner, npm run build and npm test will silently break here. The inconsistency between the two jobs (frontend rebuilds esbuild, SDK does not) suggests this step may have been overlooked.

  2. .github/workflows/release-node-sdk.yml, line 36-43 (link)

    P2 prepublishOnly build uses tsup/esbuild with no prior npm rebuild esbuild

    After npm ci --ignore-scripts, npm publish triggers prepublishOnly: npm run build (declared in package.json), which runs tsup — an esbuild-based bundler. If esbuild's native binary is not properly initialised (the same concern addressed by npm rebuild esbuild in the frontend job), the publish step's implicit build will fail. Adding npm rebuild esbuild (or an explicit npm run build) before npm publish would 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

@saifsmailbox98

Copy link
Copy Markdown
Collaborator

gonna check this out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants