CI: ignore install scripts, run only the allowlisted ones (#46) #63

Merged

james merged 1 commit from 46-ignore-scripts-allowlist into main 2026-06-15 13:14:16 +00:00

Conversation 0 Commits 1 Files changed 7 +1879 -79

james commented 2026-06-15 12:52:41 +00:00

Owner

Copy link

Closes #46.

Summary

• Every npm ci in .forgejo/workflows/pr.yml now runs with --ignore-scripts; an explicit npx allow-scripts step then executes only the lifecycle scripts listed in package.json under lavamoat.allowScripts.
• Added @lavamoat/allow-scripts as a devDependency. Chose Lavamoat over a manual npm rebuild wrapper because the ticket asks adding a new native-build dep to be a deliberate review step — Lavamoat fails CI on any un-configured install script, manual rebuild silently skips them.
• Allowlist scanned against the current dep tree:
  • vitest>vite>esbuild → true (esbuild's binary-link postinstall)
  • next>sharp → true (libvips bindings install/build check; needed for Next.js prod image optimization)
  • eslint-config-next>eslint-import-resolver-typescript>unrs-resolver → true (native Rust resolver, needed for the import/no-unresolved lint rule)
• Documented the policy in CLAUDE.md under Working in this repo, sibling to the SHA-pin convention from #45. Includes the workflow for extending the allowlist (npx allow-scripts auto, inspect, commit) and the gotcha that Lavamoat v2 requires fully-qualified parent>child paths, not leaf names.

Acceptance criteria

• CI installs run with scripts disabled by default — grep -n 'npm ci' .forgejo/workflows/pr.yml shows every invocation paired with --ignore-scripts.
• Explicit allowlist lives in package.json (lavamoat.allowScripts).
• npm test passes locally with the new install flow — 61 tests pass, native bindings (unrs-resolver, sharp, esbuild) all functional after allow-scripts.
• Policy documented next to the #45 SHA-pin convention.

Test plan

• CI green on this PR — both DB engines in the test matrix, lint (needs unrs-resolver), build (needs sharp + esbuild).
• Trivial sanity check: grep "npm ci" .forgejo/workflows/pr.yml — every line ends in --ignore-scripts.
• Negative check: temporarily remove an entry from lavamoat.allowScripts, re-run npx allow-scripts — should fail with "missing configuration".

Notes for reviewers

• The lockfile diff is huge (~1700 lines) because @lavamoat/allow-scripts pulls in npmlog/gauge/etc. — npm warns several of them are deprecated. They're dev-only and don't ship in the runtime image, so the npm_audit --omit=dev gate won't surface them. If you'd rather avoid those transitives, the alternative is the manual npm rebuild <pkgs> approach (zero new deps, weaker enforcement) — happy to switch.
• Lavamoat's own dependency tree contains test fixtures named bbb, evil_dep, good_dep with install scripts (they live inside other packages' test/ directories, not as real edges). Lavamoat correctly skips them; no allowlist entries needed.

Closes #46. ## Summary - Every `npm ci` in `.forgejo/workflows/pr.yml` now runs with `--ignore-scripts`; an explicit `npx allow-scripts` step then executes only the lifecycle scripts listed in `package.json` under `lavamoat.allowScripts`. - Added `@lavamoat/allow-scripts` as a devDependency. Chose Lavamoat over a manual `npm rebuild` wrapper because the ticket asks adding a new native-build dep to be a *deliberate review step* — Lavamoat fails CI on any un-configured install script, manual rebuild silently skips them. - Allowlist scanned against the current dep tree: - `vitest>vite>esbuild` → `true` (esbuild's binary-link postinstall) - `next>sharp` → `true` (libvips bindings install/build check; needed for Next.js prod image optimization) - `eslint-config-next>eslint-import-resolver-typescript>unrs-resolver` → `true` (native Rust resolver, needed for the `import/no-unresolved` lint rule) - Documented the policy in `CLAUDE.md` under *Working in this repo*, sibling to the SHA-pin convention from #45. Includes the workflow for extending the allowlist (`npx allow-scripts auto`, inspect, commit) and the gotcha that Lavamoat v2 requires fully-qualified parent>child paths, not leaf names. ## Acceptance criteria - [x] CI installs run with scripts disabled by default — `grep -n 'npm ci' .forgejo/workflows/pr.yml` shows every invocation paired with `--ignore-scripts`. - [x] Explicit allowlist lives in `package.json` (`lavamoat.allowScripts`). - [x] `npm test` passes locally with the new install flow — 61 tests pass, native bindings (`unrs-resolver`, `sharp`, `esbuild`) all functional after `allow-scripts`. - [x] Policy documented next to the #45 SHA-pin convention. ## Test plan - [ ] CI green on this PR — both DB engines in the test matrix, lint (needs `unrs-resolver`), build (needs `sharp` + `esbuild`). - [ ] Trivial sanity check: `grep "npm ci" .forgejo/workflows/pr.yml` — every line ends in `--ignore-scripts`. - [ ] Negative check: temporarily remove an entry from `lavamoat.allowScripts`, re-run `npx allow-scripts` — should fail with "missing configuration". ## Notes for reviewers - The lockfile diff is huge (~1700 lines) because `@lavamoat/allow-scripts` pulls in `npmlog`/`gauge`/etc. — npm warns several of them are deprecated. They're dev-only and don't ship in the runtime image, so the `npm_audit --omit=dev` gate won't surface them. If you'd rather avoid those transitives, the alternative is the manual `npm rebuild <pkgs>` approach (zero new deps, weaker enforcement) — happy to switch. - Lavamoat's own dependency tree contains test fixtures named `bbb`, `evil_dep`, `good_dep` with install scripts (they live inside other packages' `test/` directories, not as real edges). Lavamoat correctly skips them; no allowlist entries needed.

james added 1 commit 2026-06-15 12:52:41 +00:00

CI: ignore install scripts by default, run only the allowlisted ones (#46) ... ae96c5646f

Disable lifecycle scripts on every `npm ci` in `.forgejo/workflows/pr.yml`,
then invoke `npx allow-scripts` to run only the scripts explicitly listed
in `package.json` under `lavamoat.allowScripts`. A compromised transitive
dep can no longer execute arbitrary postinstall code on the runner; adding
a new install-script dep is now a deliberate review step because CI fails
on a missing config entry.

Allowlisted today:
- vitest>vite>esbuild (platform binary postinstall)
- next>sharp           (libvips bindings install check)
- eslint-config-next>eslint-import-resolver-typescript>unrs-resolver

Verified locally: clean install → allow-scripts → lint / typecheck /
build / test all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

james force-pushed 46-ignore-scripts-allowlist from ae96c5646f to 9b8cbdc975 2026-06-15 13:11:32 +00:00 Compare

james merged commit dd09fd0bff into main 2026-06-15 13:14:16 +00:00

james referenced this pull request from a commit 2026-06-15 13:14:18 +00:00

Merge pull request 'CI: ignore install scripts, run only the allowlisted ones (#46)' (#63) from 46-ignore-scripts-allowlist into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:auth

Authentication and authorization

  

area:ci

Forgejo Actions, build, test, security, release

  

area:db

Database abstraction, migrations, persistence

  

area:infra

Containerization, deployment, foundational tooling

  

area:native

React Native app (Android + Linux Flatpak)

  

area:pwa

PWA / web frontend

  

area:service

Backend service, API routes, persistence

  

epic

High-level tracker; links several feature tickets

  

feature

A user-facing feature

  

foundation

Foundational work that other tickets build on

No labels

area:auth

area:ci

area:db

area:infra

area:native

area:pwa

area:service

epic

feature

foundation

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

james/carol!63

No description provided.