diff --git a/.github/workflows/integration-drizzle.yml b/.github/workflows/integration-drizzle.yml index f4967a09e..54c1b0897 100644 --- a/.github/workflows/integration-drizzle.yml +++ b/.github/workflows/integration-drizzle.yml @@ -59,6 +59,22 @@ on: - '.github/workflows/integration-drizzle.yml' - '.github/actions/integration-setup/**' - '.github/actions/integration-db/**' + - '.github/actions/build-ffi-binding/**' + # The Rust that produces every EQL payload these suites round-trip. + # Absorbing protect-ffi put it in-tree, so a crate change can now + # break them in a PR that touches no TypeScript at all. + # + # The manifests are here for the same reason as the sources: a + # dependency bump in Cargo.lock changes the compiled encryption core + # without touching a .rs file, and package.json / mise.toml carry the + # build scripts and the toolchain pin. Matching the filter in + # tests-rust.yml, which already covers all four. + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' pull_request: branches: ['**'] # Repeated verbatim: GitHub Actions does not support YAML anchors/aliases. @@ -106,6 +122,22 @@ on: - '.github/workflows/integration-drizzle.yml' - '.github/actions/integration-setup/**' - '.github/actions/integration-db/**' + - '.github/actions/build-ffi-binding/**' + # The Rust that produces every EQL payload these suites round-trip. + # Absorbing protect-ffi put it in-tree, so a crate change can now + # break them in a PR that touches no TypeScript at all. + # + # The manifests are here for the same reason as the sources: a + # dependency bump in Cargo.lock changes the compiled encryption core + # without touching a .rs file, and package.json / mise.toml carry the + # build scripts and the toolchain pin. Matching the filter in + # tests-rust.yml, which already covers all four. + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' jobs: integration: @@ -181,8 +213,9 @@ jobs: - uses: ./.github/actions/integration-setup # Fast pre-flight: fail in seconds if a secret was rotated or cleared, - # before the docker pull. The in-test `requireIntegrationEnv` is the - # correctness guarantee; this is the cheap one. + # before the binding build and the docker pull. The in-test + # `requireIntegrationEnv` is the correctness guarantee; this is the cheap + # one, so nothing expensive may be ordered ahead of it. - name: Require CipherStash secrets uses: ./.github/actions/require-cs-secrets with: @@ -191,6 +224,14 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # `wasm: true` because CS_IT_SUITE below includes `integration/wasm/**` + # — the `@cipherstash/stack/wasm-inline` adapter over the shared v3 + # matrix — which loads the real WASM, unlike the mocked unit tests. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + with: + wasm: 'true' + # No pre-`up` cleanup step any more: the project name is unique per job, so # a container leaked by a hard-killed prior run cannot hold this job's # name or its (ephemeral) port. Blanket-pruning would now be actively diff --git a/.github/workflows/integration-prisma-next.yml b/.github/workflows/integration-prisma-next.yml index ddace2c34..4af283142 100644 --- a/.github/workflows/integration-prisma-next.yml +++ b/.github/workflows/integration-prisma-next.yml @@ -33,6 +33,22 @@ on: - '.github/workflows/integration-prisma-next.yml' - '.github/actions/integration-setup/**' - '.github/actions/integration-db/**' + - '.github/actions/build-ffi-binding/**' + # The Rust that produces every EQL payload these suites round-trip. + # Absorbing protect-ffi put it in-tree, so a crate change can now + # break them in a PR that touches no TypeScript at all. + # + # The manifests are here for the same reason as the sources: a + # dependency bump in Cargo.lock changes the compiled encryption core + # without touching a .rs file, and package.json / mise.toml carry the + # build scripts and the toolchain pin. Matching the filter in + # tests-rust.yml, which already covers all four. + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' pull_request: branches: ['**'] # Repeated verbatim: GitHub Actions does not support YAML anchors/aliases. @@ -52,6 +68,22 @@ on: - '.github/workflows/integration-prisma-next.yml' - '.github/actions/integration-setup/**' - '.github/actions/integration-db/**' + - '.github/actions/build-ffi-binding/**' + # The Rust that produces every EQL payload these suites round-trip. + # Absorbing protect-ffi put it in-tree, so a crate change can now + # break them in a PR that touches no TypeScript at all. + # + # The manifests are here for the same reason as the sources: a + # dependency bump in Cargo.lock changes the compiled encryption core + # without touching a .rs file, and package.json / mise.toml carry the + # build scripts and the toolchain pin. Matching the filter in + # tests-rust.yml, which already covers all four. + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' jobs: integration: @@ -94,8 +126,9 @@ jobs: - uses: ./.github/actions/integration-setup # Fast pre-flight: fail in seconds if a secret was rotated or cleared, - # before the docker pull. The in-test `requireIntegrationEnv` is the - # correctness guarantee; this is the cheap one. + # before the binding build and the docker pull. The in-test + # `requireIntegrationEnv` is the correctness guarantee; this is the cheap + # one, so nothing expensive may be ordered ahead of it. - name: Require CipherStash secrets uses: ./.github/actions/require-cs-secrets with: @@ -104,6 +137,11 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # No `wasm: true` — the prisma-next family suites go through the native + # binding only. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + # No pre-`up` cleanup step any more: the project name is unique per job, so # a container leaked by a hard-killed prior run cannot hold this job's # name or its (ephemeral) port. Blanket-pruning would now be actively diff --git a/.github/workflows/integration-supabase.yml b/.github/workflows/integration-supabase.yml index 34b1c3349..9084dcbf8 100644 --- a/.github/workflows/integration-supabase.yml +++ b/.github/workflows/integration-supabase.yml @@ -51,6 +51,22 @@ on: - '.github/workflows/integration-supabase.yml' - '.github/actions/integration-setup/**' - '.github/actions/integration-db/**' + - '.github/actions/build-ffi-binding/**' + # The Rust that produces every EQL payload these suites round-trip. + # Absorbing protect-ffi put it in-tree, so a crate change can now + # break them in a PR that touches no TypeScript at all. + # + # The manifests are here for the same reason as the sources: a + # dependency bump in Cargo.lock changes the compiled encryption core + # without touching a .rs file, and package.json / mise.toml carry the + # build scripts and the toolchain pin. Matching the filter in + # tests-rust.yml, which already covers all four. + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' pull_request: branches: ['**'] paths: @@ -93,6 +109,22 @@ on: - '.github/workflows/integration-supabase.yml' - '.github/actions/integration-setup/**' - '.github/actions/integration-db/**' + - '.github/actions/build-ffi-binding/**' + # The Rust that produces every EQL payload these suites round-trip. + # Absorbing protect-ffi put it in-tree, so a crate change can now + # break them in a PR that touches no TypeScript at all. + # + # The manifests are here for the same reason as the sources: a + # dependency bump in Cargo.lock changes the compiled encryption core + # without touching a .rs file, and package.json / mise.toml carry the + # build scripts and the toolchain pin. Matching the filter in + # tests-rust.yml, which already covers all four. + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' jobs: integration: @@ -154,8 +186,10 @@ jobs: - uses: ./.github/actions/integration-setup # Fast pre-flight: fail in seconds if a secret was rotated or cleared, - # before paying for the ~2 GB supabase/postgres pull. The in-test - # `requireIntegrationEnv` is the correctness guarantee; this is the cheap one. + # before the binding build and before paying for the ~2 GB + # supabase/postgres pull. The in-test `requireIntegrationEnv` is the + # correctness guarantee; this is the cheap one, so nothing expensive may + # be ordered ahead of it. - name: Require CipherStash secrets uses: ./.github/actions/require-cs-secrets with: @@ -164,6 +198,11 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # No `wasm: true` — this job's CS_IT_SUITE is `integration/shared/**` + # only, which goes through the native binding. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + # No pre-`up` cleanup step any more: the project name is unique per job, so # a container leaked by a hard-killed prior run cannot hold this job's # name or its (ephemeral) port. Blanket-pruning would now be actively diff --git a/.github/workflows/prisma-example-readme-e2e.yml b/.github/workflows/prisma-example-readme-e2e.yml index f0fef5db6..a22ffac9b 100644 --- a/.github/workflows/prisma-example-readme-e2e.yml +++ b/.github/workflows/prisma-example-readme-e2e.yml @@ -18,12 +18,40 @@ on: paths: - 'examples/prisma/**' - '.github/workflows/prisma-example-readme-e2e.yml' + # The walkthrough encrypts against the live service, so the native + # binding and the action that builds it are inputs to this suite. Added + # with that build step: without them a protect-ffi change would reach main + # having never run the walkthrough. They are repeated verbatim under + # `pull_request` below, which is what makes that a gate rather than a + # report — a PR touching any of these runs the walkthrough before the + # change lands, not after. + - '.github/actions/build-ffi-binding/**' + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' pull_request: branches: - '**' paths: - 'examples/prisma/**' - '.github/workflows/prisma-example-readme-e2e.yml' + # The same entries as under `push` above, and the copy that does the + # work: a PR touching the native binding or the action that builds it + # runs the walkthrough on the PR, so a protect-ffi change is gated by + # this suite rather than reaching main unexercised. (GitHub Actions has + # no YAML anchors, so the list is written twice; + # scripts/__tests__/workflow-paths-filter-parity.test.mjs compares the + # two copies.) + - '.github/actions/build-ffi-binding/**' + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' jobs: walkthrough: @@ -72,6 +100,8 @@ jobs: # A missing / rotated / fork-PR-absent secret makes the walkthrough skip # its live steps silently, hiding regressions behind a green job. Fail loud. + # First, ahead of the binding build: this costs seconds and that costs + # minutes on a cold cache. - name: Require CipherStash secrets uses: ./.github/actions/require-cs-secrets with: @@ -80,6 +110,17 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # The walkthrough encrypts against the live service, so it needs the + # native binding — `packages/protect-ffi` is a workspace package now, so + # `index.node` is a build output rather than tarball contents. This step + # runs on push to main and on any PR whose diff matches the paths filter + # above, the absorption PR that introduced the need included: that PR + # edits this workflow, the build action, and the package itself, and all + # three are in the filter. The exception is a fork PR — it triggers the + # workflow, but the job's `if` skips it for want of CS_* credentials. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + # Build via turbo so `^build` on `@cipherstash/stack-prisma` and # its `@cipherstash/stack` peer is honoured. The test's # `pnpm install` subprocess inside `examples/prisma/` is a no-op diff --git a/.github/workflows/prisma-next-e2e.yml b/.github/workflows/prisma-next-e2e.yml index bfc8fee58..fd759f289 100644 --- a/.github/workflows/prisma-next-e2e.yml +++ b/.github/workflows/prisma-next-e2e.yml @@ -18,6 +18,19 @@ on: - 'packages/stack-prisma/**' - 'examples/prisma/**' - '.github/workflows/prisma-next-e2e.yml' + # This suite encrypts against the live service, so the native binding and + # the action that builds it are inputs to it. Added with that build step: + # without them a protect-ffi change would reach main having never run this + # suite. They are repeated verbatim under `pull_request` below, which is + # what makes that a gate rather than a report — a PR touching any of these + # runs the suite before the change lands, not after. + - '.github/actions/build-ffi-binding/**' + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' pull_request: branches: - '**' @@ -25,6 +38,20 @@ on: - 'packages/stack-prisma/**' - 'examples/prisma/**' - '.github/workflows/prisma-next-e2e.yml' + # The same entries as under `push` above, and the copy that does the + # work: a PR touching the native binding or the action that builds it + # runs this suite on the PR, so a protect-ffi change is gated by it + # rather than reaching main unexercised. (GitHub Actions has no YAML + # anchors, so the list is written twice; + # scripts/__tests__/workflow-paths-filter-parity.test.mjs compares the + # two copies.) + - '.github/actions/build-ffi-binding/**' + - 'packages/protect-ffi/crates/**' + - 'packages/protect-ffi/src/**' + - 'packages/protect-ffi/Cargo.toml' + - 'packages/protect-ffi/Cargo.lock' + - 'packages/protect-ffi/package.json' + - 'packages/protect-ffi/mise.toml' jobs: e2e: @@ -75,6 +102,8 @@ jobs: # The global-setup hook hard-errors without CS_WORKSPACE_CRN, but a # missing sibling secret could still degrade coverage silently — assert # all four up front so a rotated / fork-PR-absent secret fails loudly. + # First, ahead of the binding build: this costs seconds and that costs + # minutes on a cold cache. - name: Require CipherStash secrets uses: ./.github/actions/require-cs-secrets with: @@ -83,6 +112,17 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # This suite encrypts against the live service, so it needs the native + # binding — `packages/protect-ffi` is a workspace package now, so + # `index.node` is a build output rather than tarball contents. This step + # runs on push to main and on any PR whose diff matches the paths filter + # above, the absorption PR that introduced the need included: that PR + # edits this workflow, the build action, and the package itself, and all + # three are in the filter. The exception is a fork PR — it triggers the + # workflow, but the job's `if` skips it for want of CS_* credentials. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + # Write the CS_* credentials and the harness DATABASE_URL into the # example app's .env so the runtime + the `prisma-next migration # apply` invocation in global-setup both pick them up. The harness diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f394b73b3..77ad95222 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -108,14 +108,22 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} - # Vendoring protect-ffi put it in `turbo test --filter './packages/*'`, - # so root `pnpm test` now runs ITS suite too — and - # `src/nativeLoading.test.ts` asserts the platform binary loads. Nothing - # else in this job produces `index.node`. The type steps below need - # `lib/` for the same reason: they call package scripts directly, so - # turbo's `^build` never runs and protect-ffi's declarations resolve to - # nothing. No `wasm: true` — every `wasm-inline` unit test either mocks - # the module or asserts on the bundle graph. + # `pnpm run test` below runs the `packages/stack` suites, and only 8 of + # those 120 files mock `@cipherstash/protect-ffi` — the rest load + # `index.node`, which nothing else in this job produces. + # + # NOT for protect-ffi's own suite, though vendoring it did add + # `@cipherstash/protect-ffi#test` to `turbo test --filter './packages/*'`. + # That suite must pass with no binding at all (its `build` is `tsc` and + # its `test` is deliberately cargo-free), and `src/lintWiring.test.ts` + # re-runs it here with the artifact hidden to keep that true — this job + # having a binding is exactly why that nested run is the only place the + # artifact-free path gets exercised in CI. + # + # The type steps below need `lib/`: they call package scripts directly, + # so turbo's `^build` never runs and protect-ffi's declarations resolve + # to nothing. No `wasm: true` — every `wasm-inline` unit test either + # mocks the module or asserts on the bundle graph. - name: Build the protect-ffi binding uses: ./.github/actions/build-ffi-binding @@ -238,6 +246,14 @@ jobs: - name: Lint — no references to deleted package directories run: pnpm run lint:package-paths + # TEMPORARY — delete with the script in the phase-4 cutover PR. + # The seven protect-ffi packages live here but still publish from + # cipherstash/protectjs-ffi. A changeset naming any of them bumps all + # seven through their fixed group and the next release tries a publish + # npm has not been repointed to trust. + - name: Lint — no protect-ffi changeset before the publishing cutover + run: pnpm run lint:ffi-changeset + - name: Test — lint script self-tests run: pnpm run test:scripts @@ -309,6 +325,23 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # `e2e/vitest.config.ts` includes `tests/**/*.e2e.test.ts` and the step + # below applies no file filter, so this job runs + # `tests/prisma-example-readme.e2e.test.ts` — whose + # `describe.skipIf(!authConfigured)` un-skips the moment CS_CLIENT_ID and + # CS_CLIENT_KEY are set, which the job env above does. That walkthrough's + # `pnpm start` step encrypts against the live service through + # `@cipherstash/stack`, so it needs `index.node`; turbo's `^build` only + # gets as far as protect-ffi's `build` (tsc → `lib/`), which is the half + # that does not encrypt anything. + # + # After the pre-flight above, per + # scripts/__tests__/ffi-binding-step-order.test.mjs. No `wasm: true` — + # nothing in `e2e/tests/**` loads the WASM build (the Deno smoke test that + # does lives in `e2e/wasm/` and runs in `wasm-e2e-tests`). + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + # Run the standalone `e2e/` workspace via turbo so the `^build` # dep on the `test:e2e` task builds cli + wizard first. CLI's own # E2E (`packages/cli/tests/e2e/**`) is covered by the `run-tests` @@ -371,15 +404,16 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile - # The Deno smoke test imports the locally-built dist/wasm-inline.js - # via a file URL in e2e/wasm/deno.json — it needs a fresh build. - - name: Build stack - run: pnpm exec turbo run build --filter @cipherstash/stack - # The e2e/wasm suites FAIL when any of the four CS_* env vars is # missing (requireEnv throws — no skip gating), so a rotated / cleared # secret can't hide a real WASM regression behind a green job. This - # preflight just fails faster, before the Deno module downloads. + # preflight just fails faster, before the two builds below and the Deno + # module downloads. + # + # It is the preflight that moves above the binding build here, not the + # binding build that moves below it: `Build stack` consumes + # protect-ffi's dist/wasm output, so the two build steps have to stay in + # this order. - name: Require CipherStash secrets uses: ./.github/actions/require-cs-secrets with: @@ -388,6 +422,33 @@ jobs: client-key: ${{ secrets.CS_CLIENT_KEY }} client-access-key: ${{ secrets.CS_CLIENT_ACCESS_KEY }} + # `wasm: true` — this is the job the WASM output exists for. stack's + # dist/wasm-inline.js imports protect-ffi's dist/wasm/protect_ffi_inline.js + # by relative path, and only the three .d.ts under dist/wasm are tracked; + # the .js and .wasm come from wasm-pack and nothing else in CI runs it. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + with: + wasm: 'true' + + # The only job that can run this: the type tests read the GENERATED + # dist/wasm/*.d.ts (wasm-bindgen emits them from the `typescript_type` + # attributes in crates/protect-ffi/src/wasm.rs) and those declarations + # import ../../lib/types.js, so it needs both halves of the step above. + # That is why it is exempt from protect-ffi's `test` entry point, which + # must pass in a fresh clone with no dist/ — and the exemption in + # src/lintWiring.test.ts asserts this step exists, because between the + # absorption and this line the check ran nowhere at all: the jobs its + # exemption named were the upstream copies under + # packages/protect-ffi/.github/, which GitHub never executes. + - name: Typecheck the generated WASM declarations + run: pnpm --filter @cipherstash/protect-ffi run test:typecheck:wasm + + # The Deno smoke test imports the locally-built dist/wasm-inline.js + # via a file URL in e2e/wasm/deno.json — it needs a fresh build. + - name: Build stack + run: pnpm exec turbo run build --filter @cipherstash/stack + - name: Run Deno WASM smoke test working-directory: e2e/wasm run: deno task test @@ -435,6 +496,19 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + # The `packages/stack` suites below are the live ones — 120 files, of + # which only 8 mock `@cipherstash/protect-ffi` — so they load + # `index.node`, and the `Build packages` step below does not produce it + # (protect-ffi's `build` is `tsc`, deliberately cargo-free). No + # `wasm: true`: every `wasm-inline` unit test resolves the stubs in + # `vitest.shared.ts` or asserts on the bundle graph, same as `run-tests`. + # + # This job has no `require-cs-secrets` pre-flight to sit behind. It shares + # the native cache key with `run-tests`, so in the normal case this is a + # restore rather than a compile. + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + - name: Create .env file in ./packages/stack/ run: | touch ./packages/stack/.env diff --git a/AGENTS.md b/AGENTS.md index aa02e89a0..1629543fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,12 +102,17 @@ so that stays true for everyone else. Absorbing protect-ffi turned `lib/`, `index.node` and `dist/wasm/**` from tarball contents into build outputs, so every job that encrypts, decrypts, or typechecks against the package builds them first via - `.github/actions/build-ffi-binding` (pass `wasm: 'true'` for the two that - load the real WASM). The action caches `index.node` on a content hash of the + `.github/actions/build-ffi-binding` — passing `wasm: 'true'` only where the + job loads the real WASM build, which is the minority and costs a second cargo + build against wasm32. The action caches `index.node` on a content hash of the Rust inputs, so a PR touching no Rust pays a restore rather than a compile. **A new job that runs live encryption needs this step** — without it the failure is `Cannot find module '.../protect-ffi-linux-x64-gnu/index.node'`, reported once per test rather than once per job. + `scripts/__tests__/ffi-binding-step-order.test.mjs` holds both halves of this: + every job that receives a `CS_*` credential must build the binding, and the + `require-cs-secrets` pre-flight must come first. Both scan the workflow + directory rather than a list, so a new job is covered the day it lands. - **Rust checks live behind `test:cargo`** (`cargo test` + `cargo fmt --check`) and `mise run lint:rust` (clippy, host and wasm32). `build:native` carries `cargo build --release`. diff --git a/packages/protect-ffi/package.json b/packages/protect-ffi/package.json index 906f0c1c6..a56fb8713 100644 --- a/packages/protect-ffi/package.json +++ b/packages/protect-ffi/package.json @@ -15,6 +15,7 @@ "test": "pnpm run test:typecheck && pnpm run test:unit && pnpm run test:lint && pnpm run test:format:ts", "test:cargo": "pnpm run test:rust && pnpm run test:format:rust", "test:typecheck": "tsc && tsc -p tsconfig.test.json", + "test:typecheck:wasm": "tsc -p type-tests/tsconfig.json", "test:unit": "vitest run", "test:rust": "cargo test", "test:lint": "pnpm run test:lint:ts", diff --git a/packages/protect-ffi/src/lintWiring.test.ts b/packages/protect-ffi/src/lintWiring.test.ts index 1b3e28b84..38cceb68b 100644 --- a/packages/protect-ffi/src/lintWiring.test.ts +++ b/packages/protect-ffi/src/lintWiring.test.ts @@ -9,9 +9,26 @@ * So the property under test is reachability: a check that nothing invokes is * the failure, and it is invisible by construction. Every exemption below has * to name why. + * + * One test here is not static: `keeps cargo OUTPUT off the default test path + * too` re-runs this suite in a child process with the compiled binding made + * unresolvable. It sits with the wiring checks because it guards the same rule + * as `keeps cargo off the default test path` — the entry-point split — and + * because the half it covers cannot be read off the manifest: a test that + * requires `index.node` makes cargo a prerequisite of root `pnpm test` without + * a `cargo` token appearing anywhere in package.json. */ -import { existsSync, readdirSync, readFileSync } from 'node:fs' -import { join } from 'node:path' +import { spawnSync } from 'node:child_process' +import { + existsSync, + mkdtempSync, + readdirSync, + readFileSync, + writeFileSync, +} from 'node:fs' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' import { describe, expect, it } from 'vitest' // Vitest resolves cwd to the directory holding vitest.config.ts. `import.meta` @@ -187,12 +204,85 @@ function reachableFromAnyEntryPoint(): Set { * repository root alone, never executed. */ const ENTRY_POINT_EXEMPT: Record = { - // Empty, and that is the correct state rather than an oversight: every - // `test:*` script this package declares is reachable from `test` or - // `test:cargo`. `test:typecheck:wasm` is the one carve-out this list was - // written for, and it arrives with the root tests.yml job that runs it — - // adding the script here before that job exists is precisely the laundering - // the paragraph above describes. + // Runs against the generated wasm .d.ts, so it needs `pnpm run build:wasm` + // first. The default test must still pass in a clone with no dist/, so this + // one belongs to the wasm job in the root tests.yml. + 'test:typecheck:wasm': 'needs dist/wasm, run by the root wasm-e2e job', +} + +/** + * Set on the nested `vitest run` the artifact guard below spawns, so the guard + * does not spawn itself forever. + * + * It is the ONE place in this file a test is allowed to disappear, and it is a + * bounded one: the only process that ever carries this variable is one the + * guard itself started, and the guard then asserts on that process's exit + * status. The skip is reported by the assertion that caused it. + */ +const ARTIFACT_FREE_RUN = 'PROTECT_FFI_ARTIFACT_FREE_RUN' + +/** + * A `--require` preload that removes every cargo-built binding from the child's + * world: unresolvable through `require`, and absent from `fs`. + * + * BOTH halves are needed and they are not redundant. `Module._load` is what + * `@neon-rs/load`'s proxy goes through — for the six platform packages and for + * the `../index.node` debug fallback alike — so patching it simulates the + * missing binary. But `nativeLoading.test.ts` decides which case it is in by + * looking at the DISK, and it has to: an env var the test consults would be a + * back door that silently disables the check for anyone who exports it. So the + * filesystem has to agree with the loader, or the child fails the wrong way — + * loudly, but for the wrong reason, on precisely the machines this guard exists + * to serve. + * + * `syncBuiltinESMExports` is what makes the fs patch visible to + * `import { existsSync } from 'node:fs'`. Without it the named ESM export stays + * bound to the original function and only `require('node:fs').existsSync` sees + * the patch — a half-applied hook, which reads as a working one. + * + * The pid log is the proof that the preload reached the test WORKERS and not + * just the vitest process that spawns them. `--require` is per-process: forks + * inherit it through the environment, worker_threads would not. Vitest's + * default pool is forks today, and if that ever changes this guard would run + * against unhooked workers and pass by proving nothing — so the parent counts + * the pids rather than trusting the pool. + */ +function preloadSource(marker: string): string { + return ` +const fs = require('node:fs') +const Module = require('node:module') + +// The package root's \`index.node\`, a \`platforms//index.node\`, and the +// bare specifier of a platform package — every shape \`src/load.cts\` reaches for. +const BINDING = + /(?:^|[\\\\/])index\\.node$|@cipherstash[\\\\/]protect-ffi-(?:darwin|linux|win32)-/ + +fs.appendFileSync(${JSON.stringify(marker)}, process.pid + '\\n') + +const load = Module._load +Module._load = function (request, parent, isMain) { + if (BINDING.test(request)) { + // Shaped like the real thing: \`code\` is what packages/cli keys on. + const error = new Error("Cannot find module '" + request + "'") + error.code = 'MODULE_NOT_FOUND' + throw error + } + return load.call(this, request, parent, isMain) +} + +const existsSync = fs.existsSync +fs.existsSync = (path) => (BINDING.test(String(path)) ? false : existsSync(path)) + +const statSync = fs.statSync +fs.statSync = (path, ...rest) => { + if (!BINDING.test(String(path))) return statSync(path, ...rest) + const error = new Error('ENOENT: no such file or directory, stat ' + path) + error.code = 'ENOENT' + throw error +} + +Module.syncBuiltinESMExports() +` } describe('lint and format wiring', () => { @@ -337,6 +427,105 @@ describe('lint and format wiring', () => { expect([...cargoScripts, ...viaMise]).toEqual([]) }) + it.skipIf(process.env[ARTIFACT_FREE_RUN] === '1')( + 'keeps cargo OUTPUT off the default test path too', + () => { + // The other half of the rule the test above states, and the half nothing + // was checking. Keeping cargo out of the SCRIPTS is worth nothing if a + // test then requires what cargo produces: `src/nativeLoading.test.ts` + // asserted `assertNativeBindingAvailable()` does not throw, which needs + // an `index.node` that only `build:native` writes. Root `pnpm test` + // reaches this package through `turbo test --filter './packages/*'`, so + // that made a Rust build a prerequisite of the whole repo's default test + // — the exact thing the entry-point split exists to prevent, arriving + // through the tests instead of through the scripts. + // + // Static analysis cannot see this. A test does not name `index.node`; it + // calls an export that happens to reach the addon, four hops down. So + // the suite is RE-RUN with every binding artifact made unresolvable, and + // the property is the child's exit status. + // + // Which is also why it is not enough that this checkout currently has no + // binary. The failure is invisible exactly where it is introduced: an + // author who ran `build:native` sees green, and so does CI, which builds + // the binding before running this suite. Only the artifact-free + // contributor sees it — and by then it is on main. In CI this nested run + // is the ONLY execution of the artifact-free path. + const workspace = mkdtempSync(join(tmpdir(), 'protect-ffi-no-artifact-')) + const marker = join(workspace, 'preloaded-pids') + const preload = join(workspace, 'hide-binding.cjs') + writeFileSync(marker, '') + writeFileSync(preload, preloadSource(marker)) + + // Resolved through the manifest rather than assumed: `vitest/vitest.mjs` + // is the `bin` entry, and pnpm's store path is not guessable. + const resolve = createRequire(join(repoRoot, 'package.json')) + const manifestPath = resolve.resolve('vitest/package.json') + const vitestBin = join( + dirname(manifestPath), + JSON.parse(readFileSync(manifestPath, 'utf8')).bin.vitest, + ) + + const env: Record = { + ...process.env, + [ARTIFACT_FREE_RUN]: '1', + // The summary line is asserted on below, and vitest wraps every field + // of it in colour escapes when it thinks it has a TTY-ish consumer. + NO_COLOR: '1', + NODE_OPTIONS: [process.env.NODE_OPTIONS, `--require ${preload}`] + .filter(Boolean) + .join(' '), + } + // This process IS a vitest worker, and its VITEST_* variables describe + // that worker. Inherited, they make the child think it is one too. + for (const key of Object.keys(env)) { + if (key.startsWith('VITEST')) delete env[key] + } + + const child = spawnSync(process.execPath, [vitestBin, 'run'], { + cwd: repoRoot, + env, + encoding: 'utf8', + timeout: 300_000, + }) + const output = `${child.stdout ?? ''}\n${child.stderr ?? ''}` + + expect( + child.error, + `Could not start the nested vitest run at ${vitestBin}.`, + ).toBeUndefined() + + expect( + child.status, + `The default test suite does not survive a checkout with no cargo build.\n\`packages/protect-ffi\`'s \`test\` is what root \`pnpm test\` runs through turbo, and it must pass with no \`index.node\` anywhere — the six \`platforms/*\` packages are empty until someone compiles one, and \`build\` is \`tsc\`, not cargo.\nGate the assertion on the artifact being present (see \`builtArtifacts\` in nativeLoading.test.ts) and give the artifact-free case its own contract, rather than making a Rust toolchain a prerequisite of the repo's default test.\nThe nested run said:\n${output}`, + ).toBe(0) + + // Non-vacuity, in two parts. A child that ran nothing exits 0 on some + // configurations, and a preload that never reached the workers leaves + // the artifact visible to them — either one turns this green while + // testing nothing. + // Recursive, matching `vitest.config.ts`'s `src/**/*.test.ts`. A flat + // readdir agrees with it only for as long as nobody nests a test file, + // and then this fails on a correct suite. + const files = readdirSync(join(repoRoot, 'src'), { + recursive: true, + }).filter((name) => String(name).endsWith('.test.ts')) + expect( + output, + `The nested run did not report ${files.length} passing test files, so it did not run this suite.\n${output}`, + ).toContain(`Test Files ${files.length} passed (${files.length})`) + + const pids = new Set( + readFileSync(marker, 'utf8').split('\n').filter(Boolean), + ) + expect( + pids.size, + `The preload logged ${pids.size} process(es). It has to reach the vitest process AND the workers that run the test files — \`--require\` travels through the environment to forked children, but not into worker_threads. If vitest's pool is no longer fork-based, this guard ran against workers that could still see the binding.\nPids: ${[...pids].join(', ')}`, + ).toBeGreaterThan(1) + }, + 300_000, + ) + it('reaches every cargo check from the cargo entry point', () => { // The mirror of the check above: cargo scripts are allowed to exist, but // not to exist unreachable. Without this, moving a check off `test` and diff --git a/packages/protect-ffi/src/nativeLoading.test.ts b/packages/protect-ffi/src/nativeLoading.test.ts index af51bc288..89e3fd419 100644 --- a/packages/protect-ffi/src/nativeLoading.test.ts +++ b/packages/protect-ffi/src/nativeLoading.test.ts @@ -11,19 +11,30 @@ * `@cipherstash/stack-prisma` reaches this package through one entry out of * fifteen. * - * A regression here is silent for anyone with a binary installed, which is - * everyone who would notice — so the property is asserted against the EMITTED - * JavaScript rather than behaviour. `lib/` exists by the time this runs: - * `test:typecheck` emits it before `test:unit`. + * A regression here is silent for anyone with a binary installed — every + * consumer who installs from npm, and every CI job that has run + * `.github/actions/build-ffi-binding` — so the property is asserted against the + * EMITTED JavaScript rather than behaviour. `lib/` exists by the time this + * runs: `test:typecheck` emits it before `test:unit`. + * + * Nothing in this file may REQUIRE a binary, though, and that is a separate + * rule from the one above. `packages/protect-ffi`'s `test` is the default task + * root `pnpm test` reaches through `turbo test --filter './packages/*'`, and it + * is deliberately Rust-free: `index.node` stopped being tarball content when + * this package was absorbed, so on a fresh checkout there is no binary + * anywhere and the six `platforms/*` links are empty. `lintWiring.test.ts` + * guards that rule for the whole suite by re-running it with the artifact made + * unresolvable. */ -import { existsSync, readFileSync, statSync } from 'node:fs' +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' import { createRequire } from 'node:module' import { join } from 'node:path' import { describe, expect, it } from 'vitest' // Vitest resolves cwd to the directory holding vitest.config.ts. -const entryPath = join(process.cwd(), 'lib/index.cjs') +const packageRoot = process.cwd() +const entryPath = join(packageRoot, 'lib/index.cjs') // `lib/` is generated, so this file carries a prerequisite the package's `test` // chain satisfies and a bare `test:unit` does not. Without this guard the whole @@ -80,6 +91,60 @@ describe('native binding load timing', () => { }) }) +/** + * Every path a build of this package leaves an `index.node` at. + * + * This is the SPLIT the two branches below turn on, and it is deliberately a + * filesystem fact rather than `process.env.CI`. `CI` is a claim about which + * machine this is; the question is whether a cargo build has happened, which is + * observable directly. `CI` also lies in both directions — it is set by `act`, + * by pre-commit wrappers, and by anyone who exports it, none of which builds a + * binding; and it says nothing about a developer who ran `build:native` and + * whose positive case would then never be checked. + */ +function binariesUnder(dir: string): string[] { + if (!existsSync(dir)) return [] + return readdirSync(dir).map((name) => join(dir, name, 'index.node')) +} + +const BINDING_ARTIFACTS = [ + // `build:native` → the `postcargo-build` hook → a bare `neon dist`, which + // writes it at the package ROOT. That is the `debug:` fallback registered in + // `src/load.cts`, and it is what `.github/actions/build-ffi-binding` + // produces — CI never populates a platform package. + join(packageRoot, 'index.node'), + // `neon dist -o platforms/` — release packaging, and the shape an + // `npm install` of `@cipherstash/protect-ffi-` leaves behind. In + // this workspace `node_modules/@cipherstash/protect-ffi-*` symlinks to + // `platforms/*` so the two overlap; for a consumer only the first exists. + ...binariesUnder(join(packageRoot, 'platforms')), + ...binariesUnder(join(packageRoot, 'node_modules/@cipherstash')), +] + +// Zero-byte counts as absent, matching the `lib/index.cjs` guard above: an +// interrupted `neon dist` leaves one, and treating it as present would send +// this into the branch that demands a successful load. +const builtArtifacts = BINDING_ARTIFACTS.filter( + (path) => existsSync(path) && statSync(path).size > 0, +) + +/** + * The platform-package name in a loader failure, as `packages/cli` matches it + * (`PLATFORM_PKG` in `packages/cli/src/native.ts`). + * + * Copied rather than imported — this package does not depend on the CLI, and + * should not. The coupling is the point of the assertion: `index.cts`'s doc + * comment promises the loader's error propagates UNWRAPPED so that "existing + * classification of a missing binding keeps working unchanged", and the CLI is + * the code doing that classification. Nothing checked the promise against a + * real error until now: the CLI's own suite builds its inputs by hand + * (`moduleError("Cannot find module '@cipherstash/protect-ffi-darwin-arm64'")` + * in `packages/cli/src/__tests__/native.test.ts`), so it proves the matcher + * matches a string, not that the string is what the loader raises. + */ +const PLATFORM_PACKAGE = + /@cipherstash\/[a-z0-9-]+-(?:darwin|linux|win32)-[a-z0-9-]+/i + describe('assertNativeBindingAvailable', () => { // Loaded from the EMITTED entry, not the source. Vitest cannot parse `.cts` // ("content contains invalid JS syntax"), and the built artifact is what a @@ -91,22 +156,111 @@ describe('assertNativeBindingAvailable', () => { // `lintWiring.test.ts`). const mod = createRequire(entryPath)(entryPath) + // Called ONCE, here, rather than inside a test: the loader memoises, so a + // second call after a first success is not a second load, and the branches + // below are two views of the same outcome rather than two attempts. + const outcome: { error: unknown } = (() => { + try { + mod.assertNativeBindingAvailable() + return { error: undefined } + } catch (error) { + return { error } + } + })() + + const errorDetail = + outcome.error instanceof Error + ? `${outcome.error.name} [${(outcome.error as NodeJS.ErrnoException).code}]: ${outcome.error.message}` + : String(outcome.error) + it('is exported from the package entry', () => { // `stash doctor` will consume this by name across a package boundary, so // its presence is the contract — see the doc comment on the function. expect(typeof mod.assertNativeBindingAvailable).toBe('function') }) - it('succeeds when the binding is present', () => { - // This suite runs where a binary is installed, so the negative case (a - // `MODULE_NOT_FOUND` propagating unwrapped) belongs to the CLI's - // missing-binary fixture rather than here. - expect(() => mod.assertNativeBindingAvailable()).not.toThrow() + it('loads a built binding, and fails classifiably when none is built', () => { + // This used to be a bare `expect(...).not.toThrow()` justified by "this + // suite runs where a binary is installed". That premise died with the + // absorption. `index.node` arrived prebuilt inside the npm tarball; as a + // workspace package it is a cargo output, the six `platforms/*` links are + // empty until someone builds one, and `packages/protect-ffi`'s `test` is + // deliberately Rust-free. So on every fresh checkout the assertion failed + // — and it failed under root `pnpm test`, which reaches this package via + // `turbo test --filter './packages/*'`. + // + // Both branches assert. A `skipIf` here would be the trade this repo keeps + // refusing: it goes quiet on every contributor machine, and quiet is + // indistinguishable from passing. What replaces it is that the artifact- + // free case has a contract of its OWN worth checking, and it is the one + // `stash doctor` and `reportNativeBinaryMissing` depend on. + if (builtArtifacts.length > 0) { + expect( + outcome.error, + `A binding is built at ${builtArtifacts.join(', ')}, so the loader must resolve it. It threw instead:\n ${errorDetail}\nEither the artifact is broken (wrong architecture, truncated write) or \`assertNativeBindingAvailable\` no longer calls a native export that loads cleanly — \`native.isEncrypted(null)\` is chosen because it is pure, synchronous, and validates nothing before reaching the addon.`, + ).toBeUndefined() + return + } + + expect( + outcome.error, + `No index.node exists under packages/protect-ffi, so the loader cannot succeed and this call must throw. Looked at:\n${BINDING_ARTIFACTS.map((path) => ` ${path}`).join('\n')}\nA success here means the binding came from somewhere none of those paths covers, and the check above is then gated on a list that no longer describes reality.`, + ).toBeInstanceOf(Error) + + const error = outcome.error as NodeJS.ErrnoException & { + requireStack?: string[] + } + // The contract `index.cts` documents, verified against the real thing. + expect( + error.code, + `The loader failure must reach callers unwrapped as MODULE_NOT_FOUND. \`packages/cli\`'s \`isNativeBinaryMissing\` tests \`code\` first and returns false for anything else, so a wrapped or re-thrown error turns \`stash doctor\`'s actionable "native binary missing" note back into a raw stack trace. Got: ${errorDetail}`, + ).toBe('MODULE_NOT_FOUND') + expect( + `${error.message}\n${(error.requireStack ?? []).join('\n')}`, + `The failure must name the platform package so \`packages/cli\` can tell a missing NATIVE binary from any other missing module. Got: ${errorDetail}`, + ).toMatch(PLATFORM_PACKAGE) + }) + + it('has its success path exercised by the action that builds a binding', () => { + // The other half of the split above, and what stops the artifact-free + // branch from being a silent skip in disguise. The positive case only runs + // where a binding exists, which is no contributor machine by default — so + // the claim "it is checked where the artifact is guaranteed" has to be + // mechanically checkable rather than a sentence in a comment. + // + // `.github/actions/build-ffi-binding` is that place: it ends in a `Verify + // the binding loads` step calling this function through the emitted entry, + // immediately after producing `index.node`, and + // `scripts/__tests__/ffi-binding-step-order.test.mjs` requires every + // credentialed job (`tests.yml / run-tests` among them) to run it. + // + // Cut to `runs:` before searching, the way `integrationSuiteCi.test.ts` + // cuts to `jobs:`: the action's `description:` is prose, and a description + // that mentions proving the binding loads is not a step that proves it. + // Comments inside `runs:` go for the same reason. + const action = readFileSync( + join(packageRoot, '../../.github/actions/build-ffi-binding/action.yml'), + 'utf8', + ) + const runsAt = action.search(/^runs:/m) + expect( + runsAt, + 'build-ffi-binding/action.yml has no `runs:` key, so it defines no steps and the search below would scan prose alone.', + ).toBeGreaterThan(-1) + + expect( + action.slice(runsAt).replace(/^[ \t]*#.*$/gm, ''), + 'No step in .github/actions/build-ffi-binding calls `assertNativeBindingAvailable`. That step is the only place the success path runs against a real binding: the test above takes its artifact-free branch on any checkout without one, which is every fresh checkout and every contributor who has not run `build:native`.', + ).toContain('assertNativeBindingAvailable') }) it('reaches the loader rather than short-circuiting', () => { // The whole point is that it forces resolution, and a body that returned - // early would pass both tests above while proving nothing. Asserting on + // early would pass the load test above on any machine with a binding + // while proving nothing. (Without one it is caught there — an empty body + // does not throw, and the artifact-free branch requires a throw. Which is + // the wrong way round: the machines that have a binding are the ones this + // property is invisible on.) Asserting on // the emitted body — the technique the first describe block uses, and for // the same reason: on a machine with a binary installed, the difference // between forcing the load and not is invisible at runtime. diff --git a/packages/stack-drizzle/package.json b/packages/stack-drizzle/package.json index dc9713602..b67ee7bf7 100644 --- a/packages/stack-drizzle/package.json +++ b/packages/stack-drizzle/package.json @@ -62,7 +62,7 @@ "drizzle-orm": ">=0.33" }, "devDependencies": { - "@cipherstash/protect-ffi": "0.31.0", + "@cipherstash/protect-ffi": "workspace:*", "@cipherstash/test-kit": "workspace:*", "fta-cli": "3.0.0", "dotenv": "17.4.2", diff --git a/packages/stack-supabase/package.json b/packages/stack-supabase/package.json index 0efbfe0b6..5520ba9d7 100644 --- a/packages/stack-supabase/package.json +++ b/packages/stack-supabase/package.json @@ -69,7 +69,7 @@ } }, "devDependencies": { - "@cipherstash/protect-ffi": "0.31.0", + "@cipherstash/protect-ffi": "workspace:*", "@cipherstash/test-kit": "workspace:*", "fta-cli": "3.0.0", "@supabase/postgrest-js": "2.110.2", diff --git a/packages/stack/package.json b/packages/stack/package.json index d538e6f79..17de66bc0 100644 --- a/packages/stack/package.json +++ b/packages/stack/package.json @@ -216,7 +216,7 @@ "dependencies": { "@byteslice/result": "0.2.0", "@cipherstash/auth": "catalog:repo", - "@cipherstash/protect-ffi": "0.31.0", + "@cipherstash/protect-ffi": "workspace:*", "evlog": "1.11.0", "uuid": "14.0.1", "zod": "3.25.76" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0a036469f..5fd1b004d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -439,8 +439,8 @@ importers: specifier: catalog:repo version: 0.42.0(@cipherstash/auth-darwin-arm64@0.42.0)(@cipherstash/auth-darwin-x64@0.42.0)(@cipherstash/auth-linux-arm64-gnu@0.42.0)(@cipherstash/auth-linux-x64-gnu@0.42.0)(@cipherstash/auth-linux-x64-musl@0.42.0)(@cipherstash/auth-win32-x64-msvc@0.42.0) '@cipherstash/protect-ffi': - specifier: 0.31.0 - version: 0.31.0 + specifier: workspace:* + version: link:../protect-ffi evlog: specifier: 1.11.0 version: 1.11.0(next@15.5.21(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) @@ -538,8 +538,8 @@ importers: version: link:../stack devDependencies: '@cipherstash/protect-ffi': - specifier: 0.31.0 - version: 0.31.0 + specifier: workspace:* + version: link:../protect-ffi '@cipherstash/test-kit': specifier: workspace:* version: link:../test-kit @@ -666,8 +666,8 @@ importers: version: link:../stack devDependencies: '@cipherstash/protect-ffi': - specifier: 0.31.0 - version: 0.31.0 + specifier: workspace:* + version: link:../protect-ffi '@cipherstash/test-kit': specifier: workspace:* version: link:../test-kit @@ -1094,39 +1094,6 @@ packages: '@cipherstash/eql@3.0.4': resolution: {integrity: sha512-h+/1bMCuglE9pTCZLpVfPTQY2nic5JRoNl+rMQX2B/5cHCWaN5p6Q3OjEOa4Wqf/a7XWn43S/vcJLvWYYvGJuQ==} - '@cipherstash/protect-ffi-darwin-arm64@0.31.0': - resolution: {integrity: sha512-ZXMh+hsgddyKxbDSPN3JbpMOoUJK8LXgX3xL+wqb2IZo8RVFuHKfV3D0WSMFb1Avl/vDVjtzXLzCAM811gy14g==} - cpu: [arm64] - os: [darwin] - - '@cipherstash/protect-ffi-darwin-x64@0.31.0': - resolution: {integrity: sha512-O4KsoIgQchylk6ha3/gnetgjpBOO8catgV9WnNNHJ9gfqWW813ytBrCyKHv8zLx79pl7EkaA8UnE/U6YiOF17A==} - cpu: [x64] - os: [darwin] - - '@cipherstash/protect-ffi-linux-arm64-gnu@0.31.0': - resolution: {integrity: sha512-gYlkx5Ol7iv/cEwS/jy6wFuTI5NaO+BNJnXZnwuu4HrPsXSdcFRd+eY3Z7px9mnb+s6lED72pv6hMcNiwUFZJQ==} - cpu: [arm64] - os: [linux] - - '@cipherstash/protect-ffi-linux-x64-gnu@0.31.0': - resolution: {integrity: sha512-2zhNkN+T1+FN2hYAMOiUwk4r/oun+w0pwfeV2xKJhNtLzDvAfHO30ta/qyPdNtnGm/9+LBljfl4KGvz/0eVIuw==} - cpu: [x64] - os: [linux] - - '@cipherstash/protect-ffi-linux-x64-musl@0.31.0': - resolution: {integrity: sha512-Ll8T6feLmChCmRXAWnsXNH+GzwWAICD+it824BhJgEma8DF8aNSar+HsazkrHNfDvVWGVcsmBARfzxoP+6Wy4A==} - cpu: [x64] - os: [linux] - - '@cipherstash/protect-ffi-win32-x64-msvc@0.31.0': - resolution: {integrity: sha512-fx+61P4z0o+nyPkw7bc8QxC7+wV13I5m1llpvR/dO4juYOeCkrs4q7DdxnUyTIi1GOlQNGwGAV91PMKu70j9RQ==} - cpu: [x64] - os: [win32] - - '@cipherstash/protect-ffi@0.31.0': - resolution: {integrity: sha512-qzTJZE0agyWxlukQbA5CG3//gvmohJhM8a1ehYr8tH6awbrLs6p5mgUs2ClgImO8Mv7O5XcYKdnMIdGni5vC4Q==} - '@clack/core@1.4.3': resolution: {integrity: sha512-/kr3UWNtdJfxZtPgDqUOmG2pvwlmcLGheex5yiZKdwbzZJxhV+HMNR9QNmyY5cGwTNV6LrR7Jtp+KjhUAP1qBQ==} engines: {node: '>= 20.12.0'} @@ -4222,35 +4189,6 @@ snapshots: '@cipherstash/eql@3.0.4': {} - '@cipherstash/protect-ffi-darwin-arm64@0.31.0': - optional: true - - '@cipherstash/protect-ffi-darwin-x64@0.31.0': - optional: true - - '@cipherstash/protect-ffi-linux-arm64-gnu@0.31.0': - optional: true - - '@cipherstash/protect-ffi-linux-x64-gnu@0.31.0': - optional: true - - '@cipherstash/protect-ffi-linux-x64-musl@0.31.0': - optional: true - - '@cipherstash/protect-ffi-win32-x64-msvc@0.31.0': - optional: true - - '@cipherstash/protect-ffi@0.31.0': - dependencies: - '@neon-rs/load': 0.1.82 - optionalDependencies: - '@cipherstash/protect-ffi-darwin-arm64': 0.31.0 - '@cipherstash/protect-ffi-darwin-x64': 0.31.0 - '@cipherstash/protect-ffi-linux-arm64-gnu': 0.31.0 - '@cipherstash/protect-ffi-linux-x64-gnu': 0.31.0 - '@cipherstash/protect-ffi-linux-x64-musl': 0.31.0 - '@cipherstash/protect-ffi-win32-x64-msvc': 0.31.0 - '@clack/core@1.4.3': dependencies: fast-wrap-ansi: 0.2.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 06277a8c6..c3f592698 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -94,9 +94,6 @@ blockExoticSubdeps: true # integrate against directly. These ship on their own release cadence and # are not exotic deps. # - @prisma-next/* first-party Prisma packages the integration is built against -# - @cipherstash/protect-ffi* CipherStash-published native FFI and its -# per-platform binary packages (darwin-arm64, -# linux-x64-gnu, etc.); bumped in lockstep with stack # - @cipherstash/auth* CipherStash-published auth strategies (NAPI + # WASM-inline variant); also tracked in lockstep # with protect-ffi for the WASM path. @@ -105,8 +102,6 @@ blockExoticSubdeps: true # payload format protect-ffi emits; bumped in lockstep. minimumReleaseAgeExclude: - '@prisma-next/*' - - '@cipherstash/protect-ffi' - - '@cipherstash/protect-ffi-*' - '@cipherstash/auth' - '@cipherstash/auth-*' - '@cipherstash/eql' diff --git a/scripts/__tests__/bench-index-expressions.test.mjs b/scripts/__tests__/bench-index-expressions.test.mjs index 0ea8c9d10..898bc3c47 100644 --- a/scripts/__tests__/bench-index-expressions.test.mjs +++ b/scripts/__tests__/bench-index-expressions.test.mjs @@ -1,10 +1,8 @@ import { readFileSync } from 'node:fs' import { createRequire } from 'node:module' import { dirname, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' - -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') +import { REPO_ROOT } from './lib/repo-root.mjs' // The bench exists to prove the functional-index path engages. An index whose // expression is merely "a term extractor for the right column" satisfies diff --git a/scripts/__tests__/ffi-binding-action.test.mjs b/scripts/__tests__/ffi-binding-action.test.mjs index c6c90bc5d..a50b4b574 100644 --- a/scripts/__tests__/ffi-binding-action.test.mjs +++ b/scripts/__tests__/ffi-binding-action.test.mjs @@ -39,12 +39,10 @@ import { execFileSync } from 'node:child_process' import { existsSync, readFileSync, statSync } from 'node:fs' import { dirname, join, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' import { readJsonc } from './lib/read-jsonc.mjs' - -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') +import { REPO_ROOT } from './lib/repo-root.mjs' const ACTION = '.github/actions/build-ffi-binding/action.yml' const RUST_WORKFLOW = '.github/workflows/tests-rust.yml' diff --git a/scripts/__tests__/ffi-binding-step-order.test.mjs b/scripts/__tests__/ffi-binding-step-order.test.mjs new file mode 100644 index 000000000..3e2fb2d24 --- /dev/null +++ b/scripts/__tests__/ffi-binding-step-order.test.mjs @@ -0,0 +1,374 @@ +import { describe, expect, it } from 'vitest' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' + +/** + * `./.github/actions/require-cs-secrets` must run BEFORE + * `./.github/actions/build-ffi-binding` in any job that uses both. + * + * The secrets action is a pre-flight, and its whole value is being cheap: it + * reads four inputs and fails in seconds when a CS_* secret was rotated, + * cleared, or is absent because the PR came from a fork. Every workflow that + * carries it says so in a comment ("Fast pre-flight: fail in seconds if a + * secret was rotated or cleared, before the docker pull"). + * + * `build-ffi-binding` is the opposite kind of step. On a cache miss it compiles + * the Rust core from cold — minutes of runner time, and more again with + * `wasm: 'true'`. Put it first and a job with no usable credentials pays the + * full compile before it learns it was never going to be able to encrypt + * anything. The pre-flight still fails, just several minutes later and after + * the expensive half of the job has already been billed — which is the same as + * not having a pre-flight at all. + * + * So the order is load-bearing, and it does not look load-bearing: both steps + * are self-contained `uses:` blocks, and swapping them changes nothing about + * whether the job passes. That is exactly the shape of edit that gets made + * while "grouping the build steps together". Hence this test — the two steps + * are checked by position, across every workflow, discovered rather than + * listed, so a new workflow that adds the binding build is covered the day it + * lands. + * + * NOTE the direction of the fix when this fails. Moving the binding build down + * is only correct when nothing between the two steps needs the binding. In + * `tests.yml`'s `wasm-e2e-tests` job a `Build stack` step sits between them and + * consumes `dist/wasm/**`, so there the pre-flight moves UP instead. Same + * resulting order; the other edit would have broken the job. + */ + +const BUILD_FFI = './.github/actions/build-ffi-binding' +const REQUIRE_SECRETS = './.github/actions/require-cs-secrets' + +/** + * The JOBS that pair the two actions today. This is NOT the list the checks + * iterate — those scan the directory — it is the guard on the scan itself. A + * discovery test that matches nothing passes and proves nothing, and this repo + * has been bitten by that shape before (see the exit-2 "the linter could not + * run" contract in `scripts/lint-no-hardcoded-runners.mjs`, and + * `lintWiring.test.ts`'s "a check nothing invokes reads exactly like a check + * that passes"). + * + * Jobs, not files, and that distinction is the whole guard. The ordering check + * below is generated per paired job, so deleting a job's pre-flight does not + * fail it — it deletes it. A file-granular list cannot see that: drop the + * `Require CipherStash secrets` step from `tests.yml`'s `wasm-e2e-tests` job + * and `tests.yml` is still paired via `run-tests`, so the file is still found, + * the count still clears its floor, and the suite goes green having stopped + * checking the single most expensive job in the repo — the one that builds the + * binding with `wasm: 'true'`, i.e. the cold compile this pre-flight exists to + * stay ahead of. Mutation-tested: that deletion took the suite from 9 tests to + * 8 passing, with nothing red. + * + * Held as a minimum, not an equality: adding a job that builds the binding + * must not fail this. If one is renamed or genuinely stops needing the + * binding, update the list deliberately — the failure prints the ids the scan + * DID find (see `scanGuard`), so a rename is a copy from the message into the + * list rather than a re-derivation from the workflows. + */ +const EXPECTED_PAIRED_JOBS = [ + '.github/workflows/integration-drizzle.yml / integration', + '.github/workflows/integration-prisma-next.yml / integration', + '.github/workflows/integration-protect-ffi.yml / integration', + '.github/workflows/integration-supabase.yml / integration', + '.github/workflows/prisma-example-readme-e2e.yml / walkthrough', + '.github/workflows/prisma-next-e2e.yml / e2e', + '.github/workflows/tests.yml / e2e-tests', + '.github/workflows/tests.yml / run-tests', + '.github/workflows/tests.yml / wasm-e2e-tests', +] + +/** The `uses:` of a step, normalised — `uses` may carry trailing whitespace. */ +function stepUses(step) { + return typeof step?.uses === 'string' ? step.uses.trim() : null +} + +/** + * Every job that uses BOTH actions, with the EARLIEST step index of each. + * Indexed by position in the job's own `steps` list, which is the order GitHub + * runs them. + * + * First occurrence, not all of them, and that is the property rather than a + * simplification: the claim is "no credentialed work starts before the + * pre-flight", so what matters is whether the earliest pre-flight precedes the + * earliest build. A job that repeats either step still has to clear that, and + * comparing later pairs would let an early unguarded build through on the + * strength of a later guarded one. + */ +function pairedJobs(relPath) { + const wf = readWorkflow(relPath) + const found = [] + for (const [jobName, job] of Object.entries(wf?.jobs ?? {})) { + const steps = Array.isArray(job?.steps) ? job.steps : [] + const firstBuild = steps.findIndex((step) => stepUses(step) === BUILD_FFI) + const firstSecrets = steps.findIndex( + (step) => stepUses(step) === REQUIRE_SECRETS, + ) + if (firstBuild === -1 || firstSecrets === -1) continue + found.push({ relPath, jobName, steps, firstBuild, firstSecrets }) + } + return found +} + +const PAIRED = workflowFiles().flatMap(pairedJobs) +const PAIRED_JOB_IDS = PAIRED.map( + (entry) => `${entry.relPath} / ${entry.jobName}`, +) + +/** + * The four CipherStash credentials, as they appear in a workflow: `${{ vars.X }}` + * for the two public ones, `${{ secrets.X }}` for the two that aren't. A job + * carrying any of them is a job that intends to talk to the live service. + * + * Matched against the whole job — `env:` at job level, `env:` on a step, a + * `with:` input to the pre-flight action, or a `run:` body that writes a `.env` + * file. `run-tests-bun` is the last of those and the reason a job-level-`env` + * scan would not have been enough. + */ +const CREDENTIAL_EXPRESSION = + /\$\{\{\s*(?:vars|secrets)\.CS_(?:WORKSPACE_CRN|CLIENT_ID|CLIENT_KEY|CLIENT_ACCESS_KEY)\s*\}\}/ + +/** Every job in a workflow that receives at least one CS_* credential. */ +function credentialedJobs(relPath) { + const wf = readWorkflow(relPath) + const found = [] + for (const [jobName, job] of Object.entries(wf?.jobs ?? {})) { + if (!CREDENTIAL_EXPRESSION.test(JSON.stringify(job))) continue + const steps = Array.isArray(job?.steps) ? job.steps : [] + found.push({ + relPath, + jobName, + buildsBinding: steps.some((step) => stepUses(step) === BUILD_FFI), + }) + } + return found +} + +const CREDENTIALED = workflowFiles().flatMap(credentialedJobs) +const CREDENTIALED_JOB_IDS = CREDENTIALED.map( + (entry) => `${entry.relPath} / ${entry.jobName}`, +) + +/** + * The credentialed JOBS, as the guard on the coverage scan below — same role + * as `EXPECTED_PAIRED_JOBS` plays for the ordering scan, and held as a minimum + * for the same reason. If the credential names or expression syntax ever + * change, this empties the scan, and a coverage check that iterates nothing + * passes while proving nothing. + */ +const EXPECTED_CREDENTIALED_JOBS = [ + '.github/workflows/integration-drizzle.yml / integration', + '.github/workflows/integration-prisma-next.yml / integration', + '.github/workflows/integration-protect-ffi.yml / integration', + '.github/workflows/integration-supabase.yml / integration', + '.github/workflows/prisma-example-readme-e2e.yml / walkthrough', + '.github/workflows/prisma-next-e2e.yml / e2e', + '.github/workflows/tests.yml / e2e-tests', + '.github/workflows/tests.yml / run-tests', + '.github/workflows/tests.yml / run-tests-bun', + '.github/workflows/tests.yml / wasm-e2e-tests', +] + +/** + * Credentialed jobs that legitimately do NOT build the binding, each with the + * reason it doesn't need one. Empty today, and that is the intended steady + * state: every job we hand CS_* to currently runs code that encrypts. + * + * It exists because a job can hold credentials without touching the binding — + * one that only validates that the secrets are present, or one that passes them + * to a service container and never runs a suite. When that job arrives, name it + * here WITH its reason rather than loosening the scan; the second test below + * fails on an entry that no longer matches a credentialed job, so a stale + * exemption cannot outlive the job it was written for. + */ +const BINDING_EXEMPT_JOBS = new Map([ + // [' / ', 'why this job never loads index.node or dist/wasm'], +]) + +/** + * The three hand-maintained lists in this file are checked the same way: every + * id in the list must still match something the scan found. Same shape, and — + * more to the point — the same failure. A job gets renamed, the list still + * names the old id, and the fix is to swap in the new one. + * + * That fix costs two seconds if the message says what the scan DID find, and a + * source dive if it only says what went missing: the developer has the old id + * (it is in the diff) and needs the new one, which appears nowhere in the + * output. Mutation-tested — renaming `integration-drizzle.yml`'s `integration` + * job to `integration-suite` failed both guards, and only the credentialed one + * printed the new name. + * + * So the listing is built HERE rather than at each call site: it is part of + * what makes a guard fixable, and a guard that grew its own terser message + * would quietly lose that. + * + * `message` is returned as a value, not passed straight into `expect`, because + * vitest only evaluates an assertion message when the assertion FAILS. A + * diagnostic that is only read on failure is a diagnostic nothing tests — which + * is how it gets stripped back out. `names every job its scan found` below + * asserts on these strings directly. + */ +function scanGuard({ entries, found, hint }) { + return { + found, + unmatched: entries.filter((id) => !found.includes(id)), + message: `${hint}\nThe scan currently sees:\n${ + found.length === 0 + ? ' (nothing — the scan matched no jobs)' + : found.map((id) => ` ${id}`).join('\n') + }`, + } +} + +const SCAN_GUARDS = { + paired: scanGuard({ + entries: EXPECTED_PAIRED_JOBS, + found: PAIRED_JOB_IDS, + hint: `These jobs used both ${BUILD_FFI} and ${REQUIRE_SECRETS}, and the scan no longer sees them. Either an action path changed (update the constants in this file), or a job's pre-flight was dropped — in which case the ordering check for it did not fail, it stopped existing. Restore the step, or update EXPECTED_PAIRED_JOBS deliberately.`, + }), + credentialed: scanGuard({ + entries: EXPECTED_CREDENTIALED_JOBS, + found: CREDENTIALED_JOB_IDS, + hint: `These jobs pass a CS_* credential and the scan no longer sees them. Either the credential names or the \`\${{ }}\` syntax changed (update CREDENTIAL_EXPRESSION), or the job was renamed or deleted — update EXPECTED_CREDENTIALED_JOBS deliberately.`, + }), + exemptions: scanGuard({ + entries: [...BINDING_EXEMPT_JOBS.keys()], + found: CREDENTIALED_JOB_IDS, + hint: 'These BINDING_EXEMPT_JOBS entries do not match any credentialed job. Remove them, or fix the id — an exemption for a job that no longer exists exempts nothing and hides the next one.', + }), +} + +describe('protect-ffi binding builds after the secrets pre-flight', () => { + it('finds the jobs that pair the two actions', () => { + // The guard on the scan. Without it, a rename of either action path (or a + // js-yaml parse that quietly returned undefined) would empty `PAIRED` and + // every check below would pass by vacuum. + // + // No separate count assertion: job ids are unique, so an empty `unmatched` + // already means every expected job was found. A `PAIRED.length >= N` + // floor is what let the `wasm-e2e-tests` deletion through — it had slack + // in it, and slack in a scan guard is where the un-run check hides. + const guard = SCAN_GUARDS.paired + expect(guard.unmatched, guard.message).toEqual([]) + }) + + for (const file of workflowFiles()) { + const jobs = pairedJobs(file) + if (jobs.length === 0) continue + + for (const { jobName, steps, firstBuild, firstSecrets } of jobs) { + it(`${file} / ${jobName} requires secrets before building the binding`, () => { + const order = steps + .map( + (step, index) => + ` ${index}: ${step?.name ?? stepUses(step) ?? '(unnamed)'}`, + ) + .join('\n') + + expect( + firstSecrets, + `"Require CipherStash secrets" must run before "Build the protect-ffi binding" in ${file} job "${jobName}".\nThe secrets check costs seconds; a cold Rust build costs minutes. Running the build first means a job with a rotated or missing credential pays the whole compile before failing — which is the same as having no pre-flight.\nSteps as ordered:\n${order}`, + ).toBeLessThan(firstBuild) + }) + } + } +}) + +/** + * COVERAGE, where the checks above are ORDER. Those only look at jobs that + * already pair the two actions, so a job that never builds the binding at all + * is invisible to them — it is not failing them, it is not in them. + * + * That gap is not theoretical. `packages/protect-ffi` is a workspace package + * now, so `lib/`, `index.node` and `dist/wasm/**` are BUILD OUTPUTS rather than + * tarball contents, and every job that encrypts has to produce them itself. Two + * credentialed jobs in `tests.yml` were missed when the `workspace:*` links + * landed: `e2e-tests` (its unfiltered `test:e2e` run picks up + * `tests/prisma-example-readme.e2e.test.ts`, whose `describe.skipIf` un-skips + * the moment CS_CLIENT_ID and CS_CLIENT_KEY are set, and whose `pnpm start` + * step encrypts for real) and `run-tests-bun` (writes the four CS_* into + * `packages/stack/.env`, then runs 120 `packages/stack` suites of which only 8 + * mock protect-ffi). + * + * The failure mode is `Cannot find module '.../index.node'`, reported once per + * test rather than once per job — and in `run-tests-bun` not reported at all, + * since it carries `continue-on-error: true` AND a `|| true` around the vitest + * invocation. So the omission is silent in exactly the job least likely to be + * looked at. + * + * Discovered, not listed: a new credentialed job is covered the day it lands, + * which is the only way this stays true. Exemptions are explicit and carry + * their reason (see BINDING_EXEMPT_JOBS). + */ +describe('every credentialed job builds the protect-ffi binding', () => { + it('finds the jobs that receive CipherStash credentials', () => { + const guard = SCAN_GUARDS.credentialed + expect(guard.unmatched, guard.message).toEqual([]) + }) + + it('exempts only jobs that are still credentialed', () => { + // A stale exemption is the silent-skip failure this whole file exists to + // avoid: the named job gets renamed or gains a real need for the binding, + // and the entry sits there exempting nothing while reading as deliberate. + const guard = SCAN_GUARDS.exemptions + expect(guard.unmatched, guard.message).toEqual([]) + }) + + it('builds the binding in every credentialed job', () => { + const offenders = CREDENTIALED.filter( + (entry) => + !entry.buildsBinding && + !BINDING_EXEMPT_JOBS.has(`${entry.relPath} / ${entry.jobName}`), + ).map((entry) => `${entry.relPath} / ${entry.jobName}`) + + expect( + offenders, + `These jobs receive CipherStash credentials but never run ${BUILD_FFI}.\n\`packages/protect-ffi\` is a workspace package: \`lib/\`, \`index.node\` and \`dist/wasm/**\` are build outputs, not tarball contents, so a job that encrypts or decrypts must build them itself. Without the step the suite fails with \`Cannot find module '.../protect-ffi-linux-x64-gnu/index.node'\`, once per test.\nAdd the step AFTER any \`${REQUIRE_SECRETS}\` pre-flight and BEFORE anything that consumes the binding. Pass \`wasm: 'true'\` only if the job loads the real WASM build.\nIf a job genuinely does not need it, add it to BINDING_EXEMPT_JOBS with the reason.`, + ).toEqual([]) + }) +}) + +/** + * The guards above are only ever read when they FAIL, which is the one moment + * their text has to be right and the one moment nothing is watching it. So the + * text itself is asserted here, on the real strings the guards carry. + * + * The property is "the message names every job the scan found", and it is the + * difference between a two-second fix and a source dive on the most likely + * failure these guards will ever see: a job rename. The developer has the OLD + * id — it is in the list, and in the diff — and needs the NEW one. Without the + * listing the new id appears nowhere in the output, and the only way to get it + * is to open the workflow and re-derive what the scan would have matched. + */ +describe('the scan guards name what they found', () => { + for (const [label, guard] of Object.entries(SCAN_GUARDS)) { + it(`${label}: the failure message lists every job its scan found`, () => { + // Not vacuous: an empty scan would satisfy a bare forEach over `found`. + // These guards exist precisely because a scan can silently empty out. + expect( + guard.found.length, + `The ${label} scan matched no jobs at all, so this test would pass without checking anything.`, + ).toBeGreaterThan(0) + + const unlisted = guard.found.filter((id) => !guard.message.includes(id)) + expect( + unlisted, + `The ${label} guard's failure message does not name these jobs, and they are exactly what a reader needs.\nWhen a job is renamed the guard reports the OLD id as missing; the NEW id is only visible if the message lists what the scan found. Keep the listing (see \`scanGuard\`).\nMessage as it stands:\n${guard.message}`, + ).toEqual([]) + }) + } + + it('says so explicitly when the scan matched nothing', () => { + // The empty scan is the catastrophic case, not a corner case: rename either + // action path and `pairedJobs` matches nothing at all (mutation-tested — + // pointing BUILD_FFI at a path no workflow uses took the file from 17 tests + // to 8, with the nine ordering checks not failing but ceasing to exist). + // Rendered as a bare empty list the message + // ends on a dangling "The scan currently sees:" with vitest's own + // `expected [...] to deeply equal []` running on from the same line, which + // reads as a broken template rather than as the finding. + const guard = scanGuard({ + entries: ['workflow.yml / job'], + found: [], + hint: 'hint', + }) + expect(guard.message).toContain('(nothing — the scan matched no jobs)') + }) +}) diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/uses-build-ffi-binding.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/uses-build-ffi-binding.yml new file mode 100644 index 000000000..5062ca112 --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/uses-build-ffi-binding.yml @@ -0,0 +1,17 @@ +# The exact edit `.github/actions/build-ffi-binding/action.yml` warns against in +# its "DO NOT USE FROM A PUBLISHING WORKFLOW" header: a credential-bearing +# publishing job pulling the binding in through the composite. `uses:` here +# names the real action in this repo, so the test that lints this file proves +# the header's claim rather than restating it. +name: Uses Build Ffi Binding +on: + push: + tags: ['v*'] +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Build the protect-ffi binding + uses: ./.github/actions/build-ffi-binding + - run: pnpm changeset:publish diff --git a/scripts/__tests__/integration-workflow-paths.test.mjs b/scripts/__tests__/integration-workflow-paths.test.mjs index 76fd71ef7..eb5fd0374 100644 --- a/scripts/__tests__/integration-workflow-paths.test.mjs +++ b/scripts/__tests__/integration-workflow-paths.test.mjs @@ -1,8 +1,8 @@ import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' -import { join, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' -import yaml from 'js-yaml' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** * An integration workflow only runs when its `paths:` filter matches the diff. @@ -28,13 +28,14 @@ import { describe, expect, it } from 'vitest' * derives which manifest that is (package manifest for exact pins, * `pnpm-workspace.yaml` for `catalog:` ones). * - * Finally, GitHub Actions has no YAML anchors, so every filter is written twice. - * A one-sided edit disables the job on pull requests while leaving it green on - * `main` — the exact inversion of what you want — so the two copies are - * compared directly. + * The third property this file used to hold — that the `push` and + * `pull_request` copies of each filter are identical, GitHub Actions having no + * YAML anchors — now lives in `workflow-paths-filter-parity.test.mjs`. It was + * never integration-specific, and holding it here scoped it to the discovery + * below: two workflows, out of the nine that write the list twice. The two it + * did cover are covered there. */ -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') const STACK_SRC = 'packages/stack/src' const STACK_MANIFEST = 'packages/stack/package.json' const CATALOG_MANIFEST = 'pnpm-workspace.yaml' @@ -42,10 +43,6 @@ const CATALOG_MANIFEST = 'pnpm-workspace.yaml' /** Both trigger events, in the order GitHub evaluates them. */ const TRIGGER_EVENTS = ['push', 'pull_request'] -function readWorkflow(relPath) { - return yaml.load(readFileSync(join(REPO_ROOT, relPath), 'utf8')) -} - /** * The integration workflows, discovered rather than listed: any workflow whose * `CS_IT_SUITE` globs select suites out of `packages/stack/` is in scope. A @@ -53,27 +50,15 @@ function readWorkflow(relPath) { * anyone remembering to add it here. */ function discoverWorkflows() { - const dir = join(REPO_ROOT, '.github/workflows') - return readdirSync(dir) - .filter((name) => name.endsWith('.yml') || name.endsWith('.yaml')) - .map((name) => `.github/workflows/${name}`) - .filter( - (relPath) => suiteFiles(suiteGlobs(readWorkflow(relPath))).length > 0, - ) - .sort() + return workflowFiles().filter( + (relPath) => suiteFiles(suiteGlobs(readWorkflow(relPath))).length > 0, + ) } /** * `on:` parses as the boolean `true` under YAML 1.1 (the "Norway problem"), * which is why this reads both keys rather than `wf.on`. */ -function triggerFilters(wf) { - const on = wf.on ?? wf[true] - return Object.fromEntries( - TRIGGER_EVENTS.map((event) => [event, on?.[event]?.paths]), - ) -} - function triggerBlocks(wf) { const on = wf.on ?? wf[true] return TRIGGER_EVENTS.map((event) => on?.[event]).filter( @@ -203,20 +188,6 @@ describe('integration workflow paths filters', () => { }) for (const relPath of WORKFLOWS) { - /** - * GitHub Actions has no YAML anchors, so each filter is written out twice. - * A one-sided edit is silent: the workflow keeps running on `push` to main - * and stops running on the PR that introduced the break, which is the only - * time it matters. Compare the two lists rather than trusting the comment. - */ - it(`${relPath} repeats an identical paths filter under push and pull_request`, () => { - const filters = triggerFilters(readWorkflow(relPath)) - for (const event of TRIGGER_EVENTS) { - expect(Array.isArray(filters[event])).toBe(true) - } - expect(filters.pull_request).toEqual(filters.push) - }) - it(`${relPath} triggers on the manifests pinning its suites' dependencies`, () => { const wf = readWorkflow(relPath) const blocks = triggerBlocks(wf) diff --git a/scripts/__tests__/lib/repo-root.mjs b/scripts/__tests__/lib/repo-root.mjs new file mode 100644 index 000000000..f1b1db383 --- /dev/null +++ b/scripts/__tests__/lib/repo-root.mjs @@ -0,0 +1,19 @@ +import { resolve } from 'node:path' +import { fileURLToPath } from 'node:url' + +/** + * The repository root, resolved from this file's own location. + * + * Every guard in `scripts/__tests__/` asserts against real repo files — + * workflows, manifests, source trees, `turbo.json` — so every one of them needs + * this, and thirteen of them declared it byte-for-byte identically before it + * was extracted. One copy is one place to fix if the directory ever moves; the + * other twelve would have been found one failing suite at a time, which is not + * a plan. + * + * Note the depth. This lives one directory deeper than the test files that used + * to declare it, so it climbs `../../../..` where they climbed `../../..`. + * `resolve` treats the file path as the starting segment, hence the extra level + * that looks off by one and is not. + */ +export const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../../..') diff --git a/scripts/__tests__/lib/workflows.mjs b/scripts/__tests__/lib/workflows.mjs new file mode 100644 index 000000000..9f25387f6 --- /dev/null +++ b/scripts/__tests__/lib/workflows.mjs @@ -0,0 +1,55 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import yaml from 'js-yaml' +import { REPO_ROOT } from './repo-root.mjs' + +/** + * Reading `.github/workflows`, for the guards that assert things about CI. + * + * Several of those guards are DISCOVERY tests — they scan the directory rather + * than iterate a list, so a workflow added tomorrow is held to the same bar + * without anyone remembering to register it. That design only works if every + * guard discovers the same set, and before this module they each rolled their + * own: three byte-for-byte copies of `workflowFiles` and `readWorkflow`, plus + * two more spelled differently (`discoverWorkflows`, a bare `WORKFLOWS` const) + * and a `readYaml` that was `readWorkflow` under another name. Copies of a + * discovery helper are the worst kind to let drift, because a copy that finds + * nothing does not fail — it passes, having checked nothing. + * + * Extracted alongside `read-jsonc.mjs`, for the same reason it was. + */ + +/** + * Where GitHub reads workflows from — and the only place it reads them from. A + * workflow file deposited under a package's own `.github/workflows` is inert, + * which is the failure `packages/protect-ffi/src/integrationSuiteCi.test.ts` + * exists to catch. + */ +export const WORKFLOW_DIR = '.github/workflows' + +/** + * Every workflow, as a repo-relative path, sorted. + * + * Both extensions: GitHub accepts `.yml` and `.yaml`, so a guard that filtered + * on one of them would silently stop covering a workflow the day someone spelt + * it the other way. + */ +export function workflowFiles() { + return readdirSync(join(REPO_ROOT, WORKFLOW_DIR)) + .filter((name) => /\.ya?ml$/.test(name)) + .map((name) => `${WORKFLOW_DIR}/${name}`) + .sort() +} + +/** + * Parse one repo-relative YAML file from the workflow graph — a workflow, or a + * composite action manifest that a workflow reaches through `uses: ./...`. + * `workflow-node-gyp.test.mjs` follows those `uses:` edges and needs to parse + * both kinds with the same reader. + * + * Note for callers: `on:` parses as the boolean `true` under YAML 1.1 (the + * "Norway problem"), so read triggers as `wf.on ?? wf[true]`. + */ +export function readWorkflow(relPath) { + return yaml.load(readFileSync(join(REPO_ROOT, relPath), 'utf8')) +} diff --git a/scripts/__tests__/lint-no-dead-package-paths.test.mjs b/scripts/__tests__/lint-no-dead-package-paths.test.mjs index 92f9d7aff..b95315932 100644 --- a/scripts/__tests__/lint-no-dead-package-paths.test.mjs +++ b/scripts/__tests__/lint-no-dead-package-paths.test.mjs @@ -4,14 +4,13 @@ import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' const SCRIPT = resolve( fileURLToPath(import.meta.url), '../../lint-no-dead-package-paths.mjs', ) -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') - function run(...targets) { return runWith({}, ...targets) } diff --git a/scripts/__tests__/lint-no-ffi-changeset.test.mjs b/scripts/__tests__/lint-no-ffi-changeset.test.mjs index 10379bfa4..c66fdaf9b 100644 --- a/scripts/__tests__/lint-no-ffi-changeset.test.mjs +++ b/scripts/__tests__/lint-no-ffi-changeset.test.mjs @@ -4,13 +4,12 @@ import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { afterAll, describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' const SCRIPT = resolve( fileURLToPath(import.meta.url), '../../lint-no-ffi-changeset.mjs', ) -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') - function run(dir) { try { const stdout = execFileSync('node', dir ? [SCRIPT, dir] : [SCRIPT], { diff --git a/scripts/__tests__/lint-no-workflow-caching.test.mjs b/scripts/__tests__/lint-no-workflow-caching.test.mjs index e9f5c89a8..0ce52a7db 100644 --- a/scripts/__tests__/lint-no-workflow-caching.test.mjs +++ b/scripts/__tests__/lint-no-workflow-caching.test.mjs @@ -4,6 +4,7 @@ import { resolve } from 'node:path' import { fileURLToPath } from 'node:url' import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' // Workflows the supply-chain gate is responsible for. const TARGET_WORKFLOWS = [ @@ -15,8 +16,6 @@ const SCRIPT = resolve( fileURLToPath(import.meta.url), '../../lint-no-workflow-caching.mjs', ) -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') - function run(...targets) { try { execFileSync('node', [SCRIPT, ...targets], { encoding: 'utf8' }) @@ -94,9 +93,9 @@ describe('lint-no-workflow-caching', () => { // The gate read a step's own `uses:` and stopped there, so a workflow could // reach `actions/cache` through one indirection — `uses: ./.github/actions/x` - // — and stay green. Verified against a copy of release.yml with a - // cache-restoring composite spliced in: exit 0, no output, while the - // composite it never opened restores two caches. + // — and stay green. Verified against a copy of release.yml with + // `.github/actions/build-ffi-binding` spliced in: exit 0, no output, while + // the composite it never opened restores two caches. describe('local composite actions', () => { const cfx = (name) => resolve( @@ -221,6 +220,31 @@ describe('lint-no-workflow-caching', () => { expect(r.exitCode).toBe(2) expect(r.output).toMatch(/no-such-action/) }) + + // A live citation, not a fixture: `.github/actions/build-ffi-binding` is + // the composite whose header says publishing workflows must not use it. + // The first assertion is what keeps the second honest — drop the caching + // from that action and this fails, which is the prompt to fix the header + // too, rather than leaving a test that passes because it now proves + // nothing. + it('flags a publishing workflow that uses .github/actions/build-ffi-binding', () => { + const action = yaml.load( + readFileSync( + resolve(REPO_ROOT, '.github/actions/build-ffi-binding/action.yml'), + 'utf8', + ), + ) + expect( + action?.runs?.steps?.filter((s) => + /^actions\/cache(\/(restore|save))?@/.test(s?.uses ?? ''), + ), + ).not.toHaveLength(0) + + const r = run(fx('uses-build-ffi-binding.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/build-ffi-binding\/action\.yml/) + expect(r.output).toMatch(/actions\/cache@/) + }) }) // The second indirection the composite traversal left open. A job that calls diff --git a/scripts/__tests__/no-removed-drizzle-surface.test.mjs b/scripts/__tests__/no-removed-drizzle-surface.test.mjs index 64334bce4..1ee0f063c 100644 --- a/scripts/__tests__/no-removed-drizzle-surface.test.mjs +++ b/scripts/__tests__/no-removed-drizzle-surface.test.mjs @@ -1,10 +1,8 @@ import { execFileSync } from 'node:child_process' import { readFileSync } from 'node:fs' import { resolve } from 'node:path' -import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' - -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') +import { REPO_ROOT } from './lib/repo-root.mjs' /** * The `@cipherstash/stack-drizzle` names removed when EQL v2 went away and diff --git a/scripts/__tests__/no-removed-eql-version-flag.test.mjs b/scripts/__tests__/no-removed-eql-version-flag.test.mjs index cbc8323af..d2aea7e6a 100644 --- a/scripts/__tests__/no-removed-eql-version-flag.test.mjs +++ b/scripts/__tests__/no-removed-eql-version-flag.test.mjs @@ -1,10 +1,8 @@ import { execFileSync } from 'node:child_process' import { readFileSync } from 'node:fs' import { resolve } from 'node:path' -import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' - -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') +import { REPO_ROOT } from './lib/repo-root.mjs' /** Tracked executable examples that describe the current public surface. */ function publicCommandDocs() { diff --git a/scripts/__tests__/rewriter-copies-in-sync.test.mjs b/scripts/__tests__/rewriter-copies-in-sync.test.mjs index 96aae7b51..649a5ee6e 100644 --- a/scripts/__tests__/rewriter-copies-in-sync.test.mjs +++ b/scripts/__tests__/rewriter-copies-in-sync.test.mjs @@ -1,9 +1,7 @@ import { readFileSync } from 'node:fs' import { resolve } from 'node:path' -import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' - -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') +import { REPO_ROOT } from './lib/repo-root.mjs' /** * The ALTER-COLUMN rewriter exists twice: `@cipherstash/wizard` diff --git a/scripts/__tests__/shared-test-helpers.test.mjs b/scripts/__tests__/shared-test-helpers.test.mjs new file mode 100644 index 000000000..42c4ac66f --- /dev/null +++ b/scripts/__tests__/shared-test-helpers.test.mjs @@ -0,0 +1,154 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { pathToFileURL } from 'node:url' +import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { WORKFLOW_DIR } from './lib/workflows.mjs' + +/** + * `scripts/__tests__/lib/` is where a helper shared by more than one guard + * lives. This checks the guards actually use it. + * + * WHAT HAPPENED. Four declarations — `REPO_ROOT`, `WORKFLOW_DIR`, + * `workflowFiles()` and `readWorkflow()` — were copied byte-for-byte between + * test files: `REPO_ROOT` into thirteen of them, the workflow trio into three, + * with two further copies wearing different names (`discoverWorkflows`, a bare + * `WORKFLOWS` const) and one whole function renamed (`readYaml`). Nothing + * failed, because copies of a working helper work. + * + * WHY IT MATTERS MORE HERE THAN IN NORMAL CODE. Most of these guards are + * DISCOVERY tests: they scan the workflow directory rather than iterate a list, so + * a workflow added tomorrow is held to the same bar without anyone registering + * it. A discovery helper is the one kind of duplicate whose drift is silent in + * the dangerous direction — a copy that finds fewer files does not fail, it + * passes, having checked less. `ffi-binding-step-order.test.mjs` records a + * mutation test where a deleted step took its suite from 9 tests to 8 with + * nothing red; a copy that quietly stops matching `.yaml` files does the same + * thing to every guard that holds it. + * + * WHAT THIS CAN AND CANNOT SEE. It reads source text, so it catches a + * redeclaration under the SAME name as a shared export, and a hand-rolled scan + * of the workflow directory under any name. It cannot see a one-line copy given + * a fresh name and a fresh directory literal — no source-level check can, short + * of comparing semantics. The two checks below are named for exactly what they + * do rather than for the ambition. + */ + +const TESTS_DIR = 'scripts/__tests__' +const LIB_DIR = `${TESTS_DIR}/lib` + +/** Repo-relative paths of the shared helper modules. */ +function libModules() { + return readdirSync(join(REPO_ROOT, LIB_DIR)) + .filter((name) => name.endsWith('.mjs') && !name.endsWith('.test.mjs')) + .map((name) => `${LIB_DIR}/${name}`) + .sort() +} + +/** Repo-relative paths of the guards themselves — `lib/` is not among them. */ +function testFiles() { + return readdirSync(join(REPO_ROOT, TESTS_DIR)) + .filter((name) => name.endsWith('.test.mjs')) + .map((name) => `${TESTS_DIR}/${name}`) + .sort() +} + +const read = (relPath) => readFileSync(join(REPO_ROOT, relPath), 'utf8') + +/** + * Every name `lib/` exports, mapped to the module exporting it. Taken by + * importing the modules rather than by reading their source: an export added + * later is then protected the day it lands, which is the same discovered-not- + * listed property the guards themselves rely on. + */ +const SHARED = new Map() +for (const relPath of libModules()) { + const mod = await import(pathToFileURL(join(REPO_ROOT, relPath)).href) + for (const name of Object.keys(mod)) SHARED.set(name, relPath) +} + +/** + * The guard on the scan, in the shape this directory uses everywhere else. If + * a rename emptied `SHARED`, every check below would iterate nothing and pass — + * the failure mode the whole directory is written to avoid. + * + * A minimum, not an equality: extracting a fifth helper must not fail this. + */ +const EXPECTED_SHARED = [ + 'REPO_ROOT', + 'WORKFLOW_DIR', + 'readJsonc', + 'readWorkflow', + 'workflowFiles', +] + +/** + * A top-level declaration of `name`. Anchored to the start of a line, which is + * what makes it a DECLARATION test rather than a mention test: a local inside a + * function body is indented, and a reference in prose sits behind ` * ` or + * `// `. Destructured forms (`const { readWorkflow } = ...`) are not matched, + * and do not need to be — that is a re-export of the shared one, not a copy. + */ +function declaresLocally(source, name) { + return new RegExp( + `^(?:export\\s+)?(?:async\\s+)?(?:function\\s+${name}\\b|(?:const|let|var)\\s+${name}\\b)`, + 'm', + ).test(source) +} + +/** + * The workflow directory written as a bare string literal — the directory + * itself, rather than the prefix of a specific file. The closing quote is the + * whole discrimination: `'.github/workflows/tests.yml'` in an expected-jobs + * list is a fact being asserted, while the bare directory is the opening line + * of a hand-rolled `readdirSync` scan. It also keeps the fixture trees under + * `fixtures/lint-no-workflow-caching/` out of scope, since those are always + * spelled with a trailing file segment. + * + * This file quotes the directory nowhere itself — it interpolates + * `WORKFLOW_DIR` below — which is the rule being enforced, applied here first. + * The alternative was to exempt this file from its own check, and an exemption + * is a hole whether or not the file that opens it is the one describing the + * rule. + */ +const BARE_WORKFLOW_DIR = /['"`]\.github\/workflows['"`]/ + +describe('guards import shared helpers instead of copying them', () => { + it('lib/ still exports the helpers these checks are about', () => { + const missing = EXPECTED_SHARED.filter((name) => !SHARED.has(name)) + expect( + missing, + `These helpers are no longer exported from ${LIB_DIR}/. If one was renamed, update EXPECTED_SHARED deliberately; if the directory moved, the checks below are scanning nothing and passing. Found: ${[...SHARED.keys()].sort().join(', ') || '(nothing)'}`, + ).toEqual([]) + }) + + it('no test file redefines a helper that lib/ already exports', () => { + const offenders = [] + for (const relPath of testFiles()) { + const source = read(relPath) + for (const [name, module] of SHARED) { + if (declaresLocally(source, name)) { + offenders.push( + `${relPath} declares \`${name}\` (exported by ${module})`, + ) + } + } + } + + expect( + offenders, + `A helper that ${LIB_DIR}/ exports was declared locally again.\nImport it instead — \`import { } from './lib/.mjs'\`.\nThese guards discover what they check by scanning the repo, so a private copy that drifts does not fail: it finds less and passes.\nIf the local version genuinely needs to differ, give it a name that says how, and say why in a comment.`, + ).toEqual([]) + }) + + it('no test file scans the workflow directory itself', () => { + const offenders = testFiles().filter((relPath) => + BARE_WORKFLOW_DIR.test(read(relPath)), + ) + + expect( + offenders, + `These files spell out \`${WORKFLOW_DIR}\` as a directory. Reading it is \`workflowFiles()\` from './lib/workflows.mjs', and the constant is \`WORKFLOW_DIR\` from the same module.\nThis check exists because the previous copies were not all called the same thing — one was \`discoverWorkflows()\`, one a bare \`WORKFLOWS\` const — so matching on the helper NAME would have missed them. Matching on the directory catches a copy whatever it is called.\nNaming a specific workflow FILE (\`${WORKFLOW_DIR}/tests.yml\`) is fine and is not matched.`, + ).toEqual([]) + }) +}) diff --git a/scripts/__tests__/turbo-skills-inputs.test.mjs b/scripts/__tests__/turbo-skills-inputs.test.mjs index a2413a121..a69d2edea 100644 --- a/scripts/__tests__/turbo-skills-inputs.test.mjs +++ b/scripts/__tests__/turbo-skills-inputs.test.mjs @@ -18,12 +18,10 @@ */ import { existsSync, readdirSync, readFileSync } from 'node:fs' -import { join, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { readJsonc } from './lib/read-jsonc.mjs' - -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') +import { REPO_ROOT } from './lib/repo-root.mjs' /** Packages whose tsup config copies the repo-root `skills/` into `dist/`. */ function packagesCopyingSkills() { diff --git a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs index 11f0f27c1..72d807d24 100644 --- a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs +++ b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs @@ -1,8 +1,5 @@ -import { readdirSync, readFileSync } from 'node:fs' -import { join, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' -import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** * A workflow that declares `workflow_dispatch:` must actually run when someone @@ -40,9 +37,6 @@ import { describe, expect, it } from 'vitest' * job runs" must not produce the same green. */ -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') -const WORKFLOW_DIR = '.github/workflows' - /** * Synthetic, and only ever compared against itself: what matters is that the * fork context disagrees with `github.repository` and the same-repo one agrees. @@ -348,17 +342,6 @@ const CONTEXTS = { // Discovery // --------------------------------------------------------------------------- -function workflowFiles() { - return readdirSync(join(REPO_ROOT, WORKFLOW_DIR)) - .filter((name) => /\.ya?ml$/.test(name)) - .map((name) => `${WORKFLOW_DIR}/${name}`) - .sort() -} - -function readWorkflow(relPath) { - return yaml.load(readFileSync(join(REPO_ROOT, relPath), 'utf8')) -} - /** * `on:` parses as the boolean `true` under YAML 1.1 (the "Norway problem"), * hence the two keys. `workflow_dispatch:` with no value parses as null, so diff --git a/scripts/__tests__/workflow-node-gyp.test.mjs b/scripts/__tests__/workflow-node-gyp.test.mjs index 08a8220cb..3cb65bb2b 100644 --- a/scripts/__tests__/workflow-node-gyp.test.mjs +++ b/scripts/__tests__/workflow-node-gyp.test.mjs @@ -1,8 +1,8 @@ -import { existsSync, readdirSync, readFileSync } from 'node:fs' -import { join, resolve } from 'node:path' -import { fileURLToPath } from 'node:url' -import yaml from 'js-yaml' +import { existsSync } from 'node:fs' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** * Every job that runs `pnpm install` on a Linux runner must install `node-gyp` @@ -48,9 +48,6 @@ import { describe, expect, it } from 'vitest' * "looked and found nothing" must not produce the same green. */ -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') -const WORKFLOW_DIR = '.github/workflows' - /** * The JOBS that run a workspace install today. This is NOT the list the checks * iterate — those scan the directory — it is the guard on the scan. @@ -273,17 +270,6 @@ export function auditFlattened(flat) { // The repo scan // --------------------------------------------------------------------------- -function workflowFiles() { - return readdirSync(join(REPO_ROOT, WORKFLOW_DIR)) - .filter((name) => /\.ya?ml$/.test(name)) - .map((name) => `${WORKFLOW_DIR}/${name}`) - .sort() -} - -function readYaml(relPath) { - return yaml.load(readFileSync(join(REPO_ROOT, relPath), 'utf8')) -} - /** * GitHub resolves `uses: ./x` against the checkout root, not the calling file. * Both manifest spellings are valid, so probing only `action.yml` would stop @@ -294,7 +280,7 @@ function resolveRepoComposite(usesPath) { for (const name of ['action.yml', 'action.yaml']) { const rel = `${dir}/${name}` if (existsSync(join(REPO_ROOT, rel))) { - const doc = readYaml(rel) + const doc = readWorkflow(rel) return { id: rel, steps: doc?.runs?.steps ?? [] } } } @@ -304,7 +290,7 @@ function resolveRepoComposite(usesPath) { function scan() { const jobs = [] for (const relPath of workflowFiles()) { - const wf = readYaml(relPath) + const wf = readWorkflow(relPath) for (const [jobName, job] of Object.entries(wf?.jobs ?? {})) { const { flat, unresolved } = flattenSteps( job?.steps, diff --git a/scripts/__tests__/workflow-paths-filter-parity.test.mjs b/scripts/__tests__/workflow-paths-filter-parity.test.mjs new file mode 100644 index 000000000..1da305263 --- /dev/null +++ b/scripts/__tests__/workflow-paths-filter-parity.test.mjs @@ -0,0 +1,159 @@ +import { describe, expect, it } from 'vitest' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' + +/** + * GitHub Actions has no YAML anchors, so a workflow that wants the same + * `paths:` filter on `push` and `pull_request` has to write the list twice. + * Nothing makes the copies agree, and a one-sided edit is silent in the worst + * direction: drop an entry from the `pull_request` copy and the workflow keeps + * running on `main`, keeps reporting green, and stops running on the pull + * request — which is the only run that could have caught the change before it + * landed. The reverse edit is loud by comparison; you notice a job that never + * runs on main. + * + * This invariant is NOT integration-specific, and for a long time the check was. + * It lived in `integration-workflow-paths.test.mjs`, whose discovery selects + * workflows by their `CS_IT_SUITE` globs — two files today. The other seven + * workflows that write the list twice were covered by nothing, including the + * two prisma e2e workflows whose own comments (until this file existed) claimed + * otherwise. Generalising it costs nothing: all nine were already identical, so + * the widened check went green on the first run and the coverage roughly + * quadrupled. + * + * The comparison is on the PARSED lists, not the text. Comments between entries, + * ordering of unrelated keys and quoting style are all free to differ; only the + * sequence of path globs has to match, because only that is what GitHub reads. + * + * THE ASYMMETRY CASE is the interesting half. A workflow declaring `paths:` on + * one event and not the other has no second copy to compare, so it drops out of + * the parity check entirely — and "drops out of the check" is exactly what a + * one-sided DELETION looks like. `tests-rust.yml` is a legitimate instance and + * therefore the reason the expected-asymmetry map below is not just an + * exemption but a fixed list: a tenth workflow arriving in that state has to be + * justified in writing rather than quietly stop being checked. + */ + +/** The two events whose filters are written out separately. */ +const FILTERED_EVENTS = ['push', 'pull_request'] + +/** + * A workflow's `paths:` filters, per event, `null` where the event declares + * none. + * + * `on:` parses as the boolean `true` under YAML 1.1 — the "Norway problem" — + * hence `wf.on ?? wf[true]`. An event key present with no `paths:` (a bare + * `pull_request:`, or `push: { branches: [main] }`) is `null` here and means + * "unfiltered", which is a different thing from an empty list and a very + * different thing from a filter that lost an entry. + */ +function pathsFilters(relPath) { + const wf = readWorkflow(relPath) + const on = wf?.on ?? wf?.[true] + return Object.fromEntries( + FILTERED_EVENTS.map((event) => [ + event, + Array.isArray(on?.[event]?.paths) ? on[event].paths : null, + ]), + ) +} + +const CLASSIFIED = workflowFiles().map((relPath) => { + const filters = pathsFilters(relPath) + const declared = FILTERED_EVENTS.filter((event) => filters[event] !== null) + return { relPath, filters, declared } +}) + +/** Workflows writing the list twice — the ones the parity check applies to. */ +const PARITY = CLASSIFIED.filter((entry) => entry.declared.length === 2) + +/** Workflows filtering one event and not the other. */ +const ASYMMETRIC = CLASSIFIED.filter((entry) => entry.declared.length === 1) + +/** + * The guard on the scan, in the shape the rest of this directory uses. Every + * assertion below is "these two lists agree", and a scan that finds no + * workflows satisfies that for free. + * + * Held as a minimum: a new workflow that filters both events must not fail + * this. A workflow LEAVING the list is the event worth interrupting for — + * either it stopped filtering both events (see the asymmetry check, which will + * also fire) or the discovery broke. + */ +const EXPECTED_PARITY_WORKFLOWS = [ + '.github/workflows/fta-v3.yml', + '.github/workflows/integration-drizzle.yml', + '.github/workflows/integration-prisma-next.yml', + '.github/workflows/integration-protect-ffi.yml', + '.github/workflows/integration-supabase.yml', + '.github/workflows/prisma-example-readme-e2e.yml', + '.github/workflows/prisma-next-e2e.yml', + '.github/workflows/tests-bench.yml', + '.github/workflows/tests-supply-chain.yml', +] + +/** + * Workflows that deliberately filter one event and not the other, each with the + * reason. An entry here is a workflow the parity check cannot see, so the + * reason has to explain why the drift it guards against is not possible. + */ +const EXPECTED_ASYMMETRIES = new Map([ + [ + '.github/workflows/tests-rust.yml', + // `push:` is `branches: [main]` with no `paths:`, so every push to main runs + // the Rust checks and only pull requests are filtered. There is no second + // list to drift from, and the direction that would hurt — a `push` filter + // narrower than the `pull_request` one, letting a change land on main + // unchecked — is unreachable when `push` is unfiltered. + 'push is deliberately unfiltered (branches: [main]), so main runs a superset of what PRs run', + ], +]) + +describe('paths filters are written twice, identically', () => { + it('finds the workflows that filter both events', () => { + const found = PARITY.map((entry) => entry.relPath) + const missing = EXPECTED_PARITY_WORKFLOWS.filter( + (relPath) => !found.includes(relPath), + ) + expect( + missing, + `These workflows declared \`paths:\` under both \`push\` and \`pull_request\`, and the scan no longer sees it. A workflow that stops filtering both events stops being parity-checked — which is indistinguishable from a one-sided deletion, the exact edit this file exists to catch. Either justify the change in EXPECTED_ASYMMETRIES, or restore the filter. Found:\n${found.map((relPath) => ` ${relPath}`).join('\n')}`, + ).toEqual([]) + }) + + it('accounts for every workflow that filters only one event', () => { + const unexplained = ASYMMETRIC.filter( + (entry) => !EXPECTED_ASYMMETRIES.has(entry.relPath), + ).map( + (entry) => + ` ${entry.relPath} filters \`${entry.declared[0]}\` only (${entry.filters[entry.declared[0]].length} entries)`, + ) + expect( + unexplained, + `These workflows filter one event and not the other, so nothing compares their filters — and a workflow arrives in that state either deliberately or because someone deleted one copy.\nIf it is deliberate, add it to EXPECTED_ASYMMETRIES with the reason the drift this file guards against cannot happen there.\n${unexplained.join('\n')}`, + ).toEqual([]) + }) + + it('keeps no stale asymmetry exemptions', () => { + // The mirror of the check above, and the one that matters when a workflow + // is FIXED: an entry that no longer describes an asymmetry sits there + // reading as deliberate while exempting nothing, and hides the next one. + const asymmetric = ASYMMETRIC.map((entry) => entry.relPath) + const stale = [...EXPECTED_ASYMMETRIES.keys()].filter( + (relPath) => !asymmetric.includes(relPath), + ) + expect( + stale, + 'These EXPECTED_ASYMMETRIES entries no longer match a workflow that filters exactly one event. If the workflow gained the second filter, delete the entry — it now belongs to the parity check above. If it was renamed or removed, fix the key.', + ).toEqual([]) + }) + + for (const { relPath } of PARITY) { + it(`${relPath} filters push and pull_request identically`, () => { + const filters = pathsFilters(relPath) + expect( + filters.pull_request, + `The \`push\` and \`pull_request\` \`paths:\` filters in ${relPath} have diverged. GitHub Actions has no YAML anchors, so the list is written twice and nothing but this check keeps the copies together.\nA \`pull_request\` filter narrower than the \`push\` one is the silent direction: the workflow still runs on main and still reports green, while the PR that introduces the change no longer runs it at all.`, + ).toEqual(filters.push) + }) + } +}) diff --git a/scripts/__tests__/workflow-trigger-comments.test.mjs b/scripts/__tests__/workflow-trigger-comments.test.mjs new file mode 100644 index 000000000..8ac3d00e3 --- /dev/null +++ b/scripts/__tests__/workflow-trigger-comments.test.mjs @@ -0,0 +1,278 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import yaml from 'js-yaml' +import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' + +/** + * A CI comment that describes WHEN a step runs is load-bearing documentation, + * and it is the one kind of documentation nothing in this repo checks. The YAML + * around it is validated six ways — `integration-workflow-paths.test.mjs` + * compares the two copies of every `paths:` filter, + * `ffi-binding-step-order.test.mjs` checks step order and coverage, + * `workflow-dispatch-job-conditions.test.mjs` checks the fork gate — while the + * prose explaining the filter to the next reader is free to say the opposite of + * what the filter does. + * + * It did. Both prisma workflows carried "the step itself is only ever exercised + * by the push-to-main run", written when the protect-ffi inputs were about to be + * added to the `push` filter, and left in place after they were added to the + * `pull_request` filter as well. A PR touching `packages/protect-ffi/src/**` has + * run both workflows since the day those entries landed. The comment told you it + * could not — which is worse than no comment, because the next person reasoning + * about protect-ffi coverage on PRs reads it and concludes there is none, and + * either adds a redundant job or drops a filter entry to "make it consistent". + * + * So: the invariant here is agreement between the filter and the prose, DERIVED + * from the parsed `on:` block rather than asserted against a fixed sentence. A + * workflow that a protect-ffi change can trigger on `pull_request` may not + * contain a comment that names `push to main` as the trigger and stops there. If + * the filter loses its protect-ffi entries tomorrow, the workflow drops out of + * scope and the push-only sentence becomes legal again — which is the property + * you want, and the one a "these files must contain this string" check cannot + * express. + * + * WHAT THIS DOES NOT COVER, deliberately: + * + * - The fork-PR caveat. Both jobs are gated on + * `github.event.pull_request.head.repo.full_name == github.repository`, so a + * fork PR triggers the workflow and then skips the job for want of CS_* + * credentials. The comments say so in prose; requiring the word "fork" in + * every comment that mentions a PR trigger would fail comments that had no + * business discussing forks. + * - "Path-filtered away from the absorption PR", the other false half of the + * same two comments — false because each workflow lists its own path, the + * build action, and the protect-ffi package in its own filter, and the + * absorption branch edits all three. Checking it mechanically would mean + * knowing what a particular PR touches, which a test in the tree cannot. + * - Trailing inline comments (`key: value # note`). Only full-line comment + * blocks are scanned, because a `#` inside a quoted YAML scalar is not a + * comment and telling the two apart needs a real parser. Every long-form + * explanation in the workflow directory is a full-line block today; if one of + * these claims ever shows up inline, this will not see it. + */ + +const BUILD_FFI_ACTION = '.github/actions/build-ffi-binding' + +/** + * The files that, when edited, change what the binding build produces — + * read out of the action's own cache keys rather than listed here. + * + * `build-ffi-binding` caches `index.node` and `dist/wasm` on `hashFiles(...)` + * of its inputs, so those globs ARE the definition of "a change to the + * binding": anything in them must miss the cache and recompile. Reusing them + * means this scope follows the action. Add a build input to the key and the set + * of workflows held to the invariant below grows with it; the alternative — a + * hardcoded `packages/protect-ffi/**` — would keep passing while quietly + * describing a different package than the one CI builds. + * + * The action's own directory is in the set for the same reason it is in both + * workflows' `paths:` filters: editing the action changes the build. + */ +function bindingBuildInputs() { + const action = yaml.load( + readFileSync(join(REPO_ROOT, `${BUILD_FFI_ACTION}/action.yml`), 'utf8'), + ) + const inputs = new Set([`${BUILD_FFI_ACTION}/**`]) + for (const step of action?.runs?.steps ?? []) { + for (const call of String(step?.with?.key ?? '').matchAll( + /hashFiles\(([^)]*)\)/g, + )) { + for (const arg of call[1].matchAll(/'([^']*)'/g)) inputs.add(arg[1]) + } + } + return [...inputs] +} + +const BINDING_INPUTS = bindingBuildInputs() + +/** + * Do a `paths:` entry and a build input describe overlapping trees? + * + * Overlap in EITHER direction, because both sides are globs and either can be + * the broader one: the filter may say `packages/protect-ffi/src/**` against an + * input file, or `packages/protect-ffi/crates/protect-ffi/**` against the + * input glob `packages/protect-ffi/crates/**`. Both mean a change to the + * binding can trigger the workflow, which is the only question being asked. + */ +function overlaps(entry, input) { + const literal = entry.replace(/\*\*$/, '').replace(/\/$/, '') + const target = input.replace(/\*\*$/, '').replace(/\/$/, '') + return ( + literal === target || + target.startsWith(`${literal}/`) || + literal.startsWith(`${target}/`) + ) +} + +/** + * How a protect-ffi change reaches this workflow through `pull_request`: + * `'unfiltered'` (no `paths:`, so every PR runs it), `'filtered'` (a `paths:` + * entry overlaps a binding input), or `false` (it cannot). + * + * `on:` parses as the boolean `true` under YAML 1.1 — the "Norway problem" — + * hence `wf.on ?? wf[true]`. And `Object.hasOwn` rather than a truthiness test: + * a bare `pull_request:` with no body parses to `null` and triggers on + * everything, which is the opposite of what `!pr` would conclude. + */ +function pullRequestReach(wf) { + const on = wf.on ?? wf[true] + if (!on || !Object.hasOwn(on, 'pull_request')) return false + const paths = on.pull_request?.paths + if (!Array.isArray(paths) || paths.length === 0) return 'unfiltered' + const reached = paths.some((entry) => + BINDING_INPUTS.some((input) => overlaps(entry, input)), + ) + return reached ? 'filtered' : false +} + +/** + * Full-line comment blocks, as `{ startLine, text }`. Consecutive `#` lines are + * one block — a bare `#` separator line included, so a paragraph stays whole and + * a sentence split across two lines is matched as one string rather than two + * halves that each match nothing. + */ +function commentBlocks(relPath) { + const lines = readFileSync(join(REPO_ROOT, relPath), 'utf8').split('\n') + const blocks = [] + let current = null + for (const [index, line] of lines.entries()) { + if (line.trim().startsWith('#')) { + if (!current) { + current = { startLine: index + 1, parts: [] } + blocks.push(current) + } + current.parts.push(line.trim().replace(/^#+\s?/, '')) + } else { + current = null + } + } + return blocks.map(({ startLine, parts }) => ({ + startLine, + text: parts.join(' ').replace(/\s+/g, ' ').trim(), + })) +} + +/** Names the push-to-main trigger: "push to main", "push-to-main run". */ +const NAMES_PUSH_TRIGGER = /push[\s-]+to[\s-]+main/i + +/** + * Names the pull_request trigger too. Several forms, because the point is to + * recognise the acknowledgement rather than to mandate a phrasing — and one + * form it must NOT accept is a bare mention of some PR as a noun. "the + * absorption PR that introduced the need" is not a statement about when the + * step runs, and the comment that contained it was exactly the one claiming + * push-to-main exclusivity in the same breath. + */ +const NAMES_PR_TRIGGER = [ + /\bpull_request\b/, + /\bon (?:any|every|each|all|a) [^.]{0,60}?\bPRs?\b/i, + /\bon (?:any|every|each|all) [^.]{0,60}?pull requests?\b/i, + /\bPRs?\b[^.]{0,40}?(?:touching|touches|matching|match(?:es)? the|whose diff)/i, +] + +const IN_SCOPE = workflowFiles().filter( + (relPath) => pullRequestReach(readWorkflow(relPath)) !== false, +) + +/** Per in-scope workflow, the comment blocks that name the push trigger. */ +const CLAIMS = new Map( + IN_SCOPE.map((relPath) => [ + relPath, + commentBlocks(relPath).filter((block) => + NAMES_PUSH_TRIGGER.test(block.text), + ), + ]), +) + +/** + * The guard on the scan, in the shape the rest of this directory uses (see + * `EXPECTED_PAIRED_JOBS` in `ffi-binding-step-order.test.mjs`, and the + * scan-found-something assertions in `ffi-binding-action.test.mjs`). Every + * check below is "no comment in this set violates X", and an empty set + * satisfies that for free. + * + * Held as a minimum: a new workflow that a protect-ffi change can trigger on a + * PR must not fail this. If one genuinely stops being reachable that way, + * update the list deliberately — the entry disappearing is the same event as + * the coverage disappearing. + */ +const EXPECTED_IN_SCOPE = [ + '.github/workflows/integration-drizzle.yml', + '.github/workflows/integration-prisma-next.yml', + '.github/workflows/integration-protect-ffi.yml', + '.github/workflows/integration-supabase.yml', + '.github/workflows/prisma-example-readme-e2e.yml', + '.github/workflows/prisma-next-e2e.yml', + '.github/workflows/tests-rust.yml', + '.github/workflows/tests.yml', +] + +/** + * The workflows that today explain their trigger in prose naming push-to-main. + * The second half of the guard, and the one that catches a broken scan rather + * than a broken scope: `commentBlocks` returning nothing, or + * `NAMES_PUSH_TRIGGER` stopping matching, empties `CLAIMS` for every file and + * the per-file checks below stop being generated — silently, since a `describe` + * that produces no `it` reports success. + * + * If a comment here is deleted or reworded off the push trigger, that is fine + * and this needs a deliberate edit. It is not fine for all of them to vanish at + * once, which is what a regex or parser regression looks like. + */ +const EXPECTED_CLAIM_FILES = [ + '.github/workflows/prisma-example-readme-e2e.yml', + '.github/workflows/prisma-next-e2e.yml', +] + +describe('workflow trigger comments match the trigger', () => { + it('derives the binding build inputs from the action manifest', () => { + expect( + BINDING_INPUTS, + `No \`hashFiles(...)\` globs were parsed out of ${BUILD_FFI_ACTION}/action.yml, so "a change to the protect-ffi binding" resolves to nothing and every workflow falls out of scope. Either the cache steps moved (follow them), or the key syntax changed (update bindingBuildInputs).`, + ).toContain('packages/protect-ffi/crates/**') + expect(BINDING_INPUTS).toContain(`${BUILD_FFI_ACTION}/**`) + }) + + it('finds the workflows a protect-ffi change can trigger on a PR', () => { + const missing = EXPECTED_IN_SCOPE.filter((file) => !IN_SCOPE.includes(file)) + expect( + missing, + `A protect-ffi change used to reach these workflows through \`on.pull_request\`, and the scan no longer sees it. Either a \`paths:\` filter dropped its protect-ffi entries — in which case the coverage went with them — or \`pullRequestReach\` needs teaching about a new filter shape. Found:\n${IN_SCOPE.map((file) => ` ${file}`).join('\n')}`, + ).toEqual([]) + }) + + it('finds the trigger-claim comments it means to inspect', () => { + const missing = EXPECTED_CLAIM_FILES.filter( + (file) => (CLAIMS.get(file) ?? []).length === 0, + ) + expect( + missing, + `These workflows explained their trigger in a comment naming push-to-main, and the scan no longer finds one. If the comments were deliberately reworded, update EXPECTED_CLAIM_FILES; if not, \`commentBlocks\` or NAMES_PUSH_TRIGGER has stopped working and the per-file checks below are no longer being generated at all.`, + ).toEqual([]) + }) + + for (const relPath of IN_SCOPE) { + const blocks = CLAIMS.get(relPath) ?? [] + if (blocks.length === 0) continue + + it(`${relPath} does not describe the trigger as push-to-main only`, () => { + const reach = pullRequestReach(readWorkflow(relPath)) + const offenders = blocks + .filter( + (block) => !NAMES_PR_TRIGGER.some((form) => form.test(block.text)), + ) + .map((block) => ` line ${block.startLine}: ${block.text}`) + + expect( + offenders, + `These comments in ${relPath} name \`push\` to main as the trigger and never mention \`pull_request\`, but a change to the protect-ffi binding reaches this workflow on a pull request too — ${ + reach === 'unfiltered' + ? 'it has no `paths:` filter on `pull_request`, so every PR runs it' + : 'its `pull_request` `paths:` filter carries the binding inputs' + }. Reword them to name both events; a comment that under-states when a step runs sends the next reader looking for coverage that is already there.\n${offenders.join('\n')}`, + ).toEqual([]) + }) + } +}) diff --git a/scripts/__tests__/workflow-turbo-build-deps.test.mjs b/scripts/__tests__/workflow-turbo-build-deps.test.mjs index 50261d4be..5f3cfad28 100644 --- a/scripts/__tests__/workflow-turbo-build-deps.test.mjs +++ b/scripts/__tests__/workflow-turbo-build-deps.test.mjs @@ -1,9 +1,9 @@ -import { readdirSync, readFileSync } from 'node:fs' +import { readFileSync } from 'node:fs' import { resolve } from 'node:path' -import { fileURLToPath } from 'node:url' -import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' import { readJsonc } from './lib/read-jsonc.mjs' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** * A turbo task declaring `dependsOn: ["^build"]` gets its workspace @@ -27,8 +27,6 @@ import { readJsonc } from './lib/read-jsonc.mjs' * This test pins that routing so it cannot be quietly "simplified" back. */ -const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') - /** * The workflow that must carry the `typecheck:scaffold` step specifically. * Scoped, because "that step exists" is a claim about this file, not about @@ -44,10 +42,7 @@ const SCAFFOLD_WORKFLOW = '.github/workflows/tests.yml' * Narrowing the guard to one file left five real bare invocations unchecked * (#787 review follow-up). */ -const WORKFLOWS = readdirSync(resolve(REPO_ROOT, '.github/workflows')) - .filter((file) => /\.ya?ml$/.test(file)) - .map((file) => `.github/workflows/${file}`) - .sort() +const WORKFLOWS = workflowFiles() /** * Bare invocations that predate this guard. Each is the same latent trap: it @@ -118,7 +113,7 @@ const rootScriptDelegatesToTurbo = (task) => typeof rootScripts[task] === 'string' && /\bturbo\b/.test(rootScripts[task]) function workflowRunLines(path) { - const doc = yaml.load(readFileSync(resolve(REPO_ROOT, path), 'utf8')) + const doc = readWorkflow(path) const lines = [] for (const [jobName, job] of Object.entries(doc?.jobs ?? {})) { for (const step of job?.steps ?? []) { diff --git a/scripts/lint-no-workflow-caching.mjs b/scripts/lint-no-workflow-caching.mjs index 672e0c2a4..f7224042d 100644 --- a/scripts/lint-no-workflow-caching.mjs +++ b/scripts/lint-no-workflow-caching.mjs @@ -36,8 +36,8 @@ const LOCAL_USES = /^\.{1,2}\// // `useblacksmith/cache@v5` and `Swatinem/rust-cache@v2`: `OK`, exit 0. Both are // live-relevant here — eleven jobs in this repo run on `blacksmith-*` runners, // where `useblacksmith/cache` is the documented drop-in for `actions/cache`, -// and an in-tree Cargo workspace is exactly where someone reaches for -// `Swatinem/rust-cache`. +// and the absorbed Cargo workspace at `packages/protect-ffi` is exactly where +// someone reaches for `Swatinem/rust-cache`. // // The obvious repair is to enumerate the cache actions — by name // (`useblacksmith/cache`, `buildjet/cache`, `runs-on/cache`, `tespkg/actions-