diff --git a/.changeset/protect-ffi-repository-url.md.deferred b/.changeset/protect-ffi-repository-url.md.deferred new file mode 100644 index 000000000..78a7f5125 --- /dev/null +++ b/.changeset/protect-ffi-repository-url.md.deferred @@ -0,0 +1,19 @@ +--- +'@cipherstash/protect-ffi': patch +--- + +Point the published metadata at `cipherstash/stack`, the repository these +packages are now built and published from. The wrapper's `repository.url`, +`bugs.url` and `homepage`, and each platform package's `repository.url`, all +named `cipherstash/protectjs-ffi`; each platform package's +`repository.directory` also named `platforms/`, which resolves from +the root of the repository named above and so addressed nothing here. + +npm requires `repository.url` to match the publishing repository exactly for a +trusted publish, and rejects a mismatch rather than warning about it. A stale +`repository.directory` fails more quietly: the publish succeeds and the source +link on the package page 404s. + +The one repository URL that reaches an end user at runtime moves too — the Rust +core's `InvariantViolation` error asks the reader to file an issue, and the +repository it pointed at is archived at the end of the publishing cutover. diff --git a/.changeset/supply-chain-skill-ffi-release-path.md b/.changeset/supply-chain-skill-ffi-release-path.md new file mode 100644 index 000000000..22b5bb65f --- /dev/null +++ b/.changeset/supply-chain-skill-ffi-release-path.md @@ -0,0 +1,39 @@ +--- +'stash': patch +--- + +Document the native-binding publish path in the bundled +`stash-supply-chain-security` skill, and correct what it claims about +frozen-lockfile coverage. + +`@cipherstash/protect-ffi` and its six platform packages ship compiled binaries, +which `changeset publish` cannot produce — it packs from the workspace, where +`index.node` is a build output. The skill now describes the pipeline that does: +a registry-state gate, a target-explicit build matrix in a reusable workflow, +and a publish step that ships the six platform packages before the wrapper and +tags all seven itself, because changesets tags only what it published. It also +records two npm requirements that fail late and quietly — `repository.url` must +match the publishing repository exactly (and `repository.directory` resolves +from that repository's root), and trusted-publisher configurations created after +2026-05-20 need an explicit "Allowed actions" selection. + +It also now states, per action, which input disables that action's built-in +caching and what that input defaults to. Two of the three default to caching +ON — `actions/setup-node`'s `package-manager-cache` and `jdx/mise-action`'s +`cache` — so omitting the key is not "no caching", it is caching spelled +invisibly, and the gate's generic rule only sees a *truthy* value rather than a +missing one. + +The OIDC section said `permissions: id-token: write` is what mints the token and +left it there. It now says where that grant belongs: on the publishing jobs, not +at the workflow level. A trusted publisher is registered against a repository +*and a workflow filename*, so npm accepts a token minted by any job in the +registered file — declaring the scope at the top hands the publish credential to +every job that does not override it, including ones added later. + +The frozen-lockfile section said the rule was enforced in `tests.yml`, which was +true and misleading: that is where it was *checked*, and `release.yml` ran a +bare `pnpm install` from the day it was written — so the single install permitted +to resolve outside the lockfile was the one whose output goes to the registry. +The install is fixed and the check now scans every workflow and every local +composite action. diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 000000000..21fa08b11 --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,12 @@ +# actionlint validates every `runs-on:` against the list of GitHub-hosted runner +# labels it ships with. Blacksmith runners are self-hosted from its perspective, +# so without this file each of the fourteen jobs using one is reported as an +# unknown label — and the release lint gate (.github/workflows/lint-release.yml) +# would be red on the day it lands, for reasons that have nothing to do with the +# release machinery it exists to check. +# +# Nothing complained before because actionlint has never run in this repository; +# lint-release.yml is what introduces it. +self-hosted-runner: + labels: + - blacksmith-4vcpu-ubuntu-2404 diff --git a/.github/workflows/_build-ffi-artifacts.yml b/.github/workflows/_build-ffi-artifacts.yml new file mode 100644 index 000000000..62d0865cf --- /dev/null +++ b/.github/workflows/_build-ffi-artifacts.yml @@ -0,0 +1,468 @@ +name: Build FFI artifacts + +# Reusable. Builds the six platform bindings and the WASM output, packs all +# seven npm tarballs, and uploads them as `ffi-tarballs`. +# +# IT DOES NOT PUBLISH, and nothing here should be made to. npm validates a +# trusted publish against the ENTRY-POINT workflow's filename, and its own docs +# record what that means for `workflow_call`: "validation checks the calling +# workflow's name instead of the workflow that actually contains the publish +# command, which can cause configuration mismatches" — plus `id-token: write` in +# both parent and child. +# +# So a publish in here would be validated against whichever workflow happened to +# call it, and would depend on behaviour npm documents as a known issue. Keeping +# the publish in `release.yml` — the registered filename, reached as an +# entry-point job rather than through a call — is correct whichever way npm +# resolves that, which is why this workflow grants `contents: read` and no +# id-token at all. +# +# Two callers: `release.yml` (after the gate says an FFI version is +# unpublished) and `ffi-preflight.yml` (the manual dry run, which has no +# id-token permission and so cannot publish either). +# +# NO CACHING ANYWHERE IN HERE. These artifacts are published, so the workflow is +# on `scripts/lint-no-workflow-caching.mjs`'s target list: every +# `pnpm/action-setup` needs `cache: false`, every `actions/setup-node` needs +# `package-manager-cache: false`, and every remote action must be in that +# script's AUDITED_ACTIONS. Upstream's `.github/actions/setup` composite is +# therefore NOT ported — it sets `cache: npm` and `cache: true`. + +on: + workflow_call: + inputs: + ref: + description: Commit to build from + required: true + type: string + +permissions: + contents: read + +defaults: + run: + shell: bash + +jobs: + matrix: + name: Compute platform matrix + runs-on: ubuntu-latest + timeout-minutes: 10 + outputs: + matrix: ${{ steps.matrix.outputs.result }} + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ inputs.ref }} + persist-credentials: false + + - uses: actions/setup-node@v6.5.0 + with: + node-version: 22 + package-manager-cache: false + + # No pnpm and no install: the script reads `neon.rust` out of the six + # committed `platforms/*/package.json` files, so this job needs nothing + # but a checkout and Node. It used to shell out to `neon list-platforms`, + # which meant a cold ~1GB workspace install (caching is forbidden here) + # on the critical path ahead of all six builds, to read six fields that + # are in the tree. + # + # The matrix also decides which build script each platform runs and which + # log file `neon dist` then reads. All three fields are easy to get wrong + # in ways that produce a green job and a broken tarball, so they live in a + # script with a test — including one pinning the committed triples against + # `neon list-platforms` output. See scripts/ffi-release-matrix.mjs. + - name: Compute the matrix + id: matrix + run: node scripts/ffi-release-matrix.mjs + + binaries: + name: ${{ matrix.cfg.platform }} + needs: [matrix] + strategy: + # One platform failing must not cancel the other five: the failure is + # usually specific to a toolchain, and seeing which of the six are fine + # is most of the diagnosis. + fail-fast: false + matrix: + cfg: ${{ fromJSON(needs.matrix.outputs.matrix) }} + runs-on: ${{ matrix.cfg.os }} + timeout-minutes: 60 + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ inputs.ref }} + persist-credentials: false + + # Static OpenSSL; without it the Windows build fails to link. + - name: Install OpenSSL (Windows) + if: ${{ matrix.cfg.os == 'windows-latest' }} + shell: powershell + run: | + vcpkg install openssl:x64-windows-static + $vcpkgRoot = $env:VCPKG_INSTALLATION_ROOT + $opensslDir = "$vcpkgRoot\installed\x64-windows-static" + echo "OPENSSL_DIR=$opensslDir" >> $env:GITHUB_ENV + echo "OPENSSL_LIB_DIR=$opensslDir\lib" >> $env:GITHUB_ENV + echo "OPENSSL_INCLUDE_DIR=$opensslDir\include" >> $env:GITHUB_ENV + echo "OPENSSL_STATIC=1" >> $env:GITHUB_ENV + + # The aarch64 linker, for the one platform that cross-compiles to it. + # Scoped to that platform rather than to Linux: `linux-x64-musl` puts + # musl.cc's own `x86_64-linux-musl-gcc` on PATH below and never calls + # this one, and `linux-x64-gnu` is a native x86_64 build — so the other + # two legs were paying an `apt-get update` (full package indexes) plus a + # ~200MB toolchain they do not link against. + - name: Install cross-compile toolchain (linux-arm64-gnu) + if: ${{ matrix.cfg.platform == 'linux-arm64-gnu' }} + run: | + set -euo pipefail + sudo apt-get update + sudo apt-get install -y gcc-13-aarch64-linux-gnu + sudo ln -sf /usr/bin/aarch64-linux-gnu-gcc-13 /usr/bin/aarch64-linux-gnu-gcc + + # Rust comes from the runner image; only the TARGET has to be added. This + # is the single most load-bearing line in the matrix — `macos-latest` is + # arm64, so without an explicit target both Darwin jobs would emit ARM64 + # and `darwin-x64` would ship a binary that installs cleanly and then + # fails to dlopen. + # + # `rustup target add` rather than a toolchain action, matching + # `tests-rust.yml` and `.github/actions/build-ffi-binding`: every runner + # image here ships rustup, and this workflow's `uses:` set is audited by + # the no-caching gate, so not adding a fourth action to that list is worth + # something on its own. + - name: Add the Rust target + env: + CARGO_BUILD_TARGET: ${{ matrix.cfg.target }} + run: rustup target add "$CARGO_BUILD_TARGET" + + - uses: pnpm/action-setup@v6.0.10 + with: + run_install: false + cache: false + + - uses: actions/setup-node@v6.5.0 + with: + node-version: 22 + package-manager-cache: false + + - name: Install node-gyp + run: npm install -g node-gyp + + # zig + cargo-zigbuild, pinned in packages/protect-ffi/mise.toml. Only the + # gnu platforms use them, so the four others skip this rather than + # compiling cargo-zigbuild from source on runners that never call it. + # + # `working_directory` is load-bearing, not tidiness: mise reads config + # from the current directory and its parents, so an action running at the + # repo root never sees the nested config — it installs nothing and leaves + # the config untrusted, and the failure surfaces later as + # "cargo-zigbuild: not found", a toolchain problem rather than the trust + # problem it is. + # + # SHA-pinned, matching .github/actions/build-ffi-binding: mise-action is a + # trust dependency, and here it runs in the workflow whose output gets + # published with provenance. + # Keyed on the matrix's own decision rather than re-deriving "is this gnu" + # from the triple: scripts/ffi-release-matrix.mjs picks zigbuild, and this + # step exists to supply what zigbuild needs. Two spellings of one rule + # drift apart the day a platform moves between them. + - name: Install zig + cargo-zigbuild (zigbuild platforms only) + if: ${{ matrix.cfg.script == 'zigbuild' }} + uses: jdx/mise-action@5228313ee0372e111a38da051671ca30fc5a96db # v3.6.3 + with: + install: true + install_args: zig cargo:cargo-zigbuild + working_directory: packages/protect-ffi + cache: false + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Build binding + working-directory: packages/protect-ffi + env: + CARGO_BUILD_TARGET: ${{ matrix.cfg.target }} + NEON_BUILD_PLATFORM: ${{ matrix.cfg.platform }} + BUILD_SCRIPT: ${{ matrix.cfg.script }} + # The musl cross toolchain is fetched from musl.cc over plain HTTPS + # with no signature to check, and whatever it hands back LINKS THE + # BINARY that this workflow publishes to npm with provenance — a + # provenance attestation says where a build ran, not that its inputs + # were the intended ones. Upstream's build.yml took the download on + # trust; this pins it. + # + # Trust-on-first-use, and worth being precise about what that buys: + # the digest was taken from two independent fetches of the current + # artifact (2026-08-11), so it does not authenticate musl.cc — it + # makes any later substitution a hard failure instead of a silent one. + # If musl.cc rebuilds the tarball this step fails; re-verify the new + # artifact deliberately and update the digest here, do not delete the + # check to unblock a release. + MUSL_TOOLCHAIN_URL: https://musl.cc/x86_64-linux-musl-native.tgz + MUSL_TOOLCHAIN_SHA256: eb1db6f0f3c2bdbdbfb993d7ef7e2eeef82ac1259f6a6e1757c33a97dbcef3ad + # No `--` separator anywhere below: npm strips it, pnpm forwards it + # verbatim, and these scripts end in `> cargo.log` — so a forwarded flag + # lands after the redirect and cargo rejects it as a positional. + run: | + set -euo pipefail + # `x86_64-unknown-linux-musl` -> `x86_64-linux-musl-gcc`. Parameter + # expansion rather than upstream's `sed`, and assigned before export + # rather than through it: actionlint runs shellcheck over `run:` + # blocks and the original spelling draws SC2001 and SC2155. + # + # Each linker variable is exported by the branch that reads it. Set + # unconditionally, as upstream had them, both are also set on Windows + # and on the two Darwin legs — where `CARGO_TARGET_..._MUSL_LINKER` + # ends up naming `x86_64-apple-darwin-gcc`, a binary that does not + # exist and that nothing on those platforms reads. + linker="${CARGO_BUILD_TARGET/unknown-/}-gcc" + if [[ "$CARGO_BUILD_TARGET" =~ musl ]]; then + export CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER="$linker" + wget -4 -O musl-native.tgz "$MUSL_TOOLCHAIN_URL" + # Verified BEFORE anything is unpacked: a tarball that fails this + # check must not have written a single file, least of all one on the + # PATH the compiler is about to use. + echo "${MUSL_TOOLCHAIN_SHA256} musl-native.tgz" | sha256sum -c - || { + echo "::error::musl toolchain digest mismatch — got $(sha256sum musl-native.tgz | cut -d' ' -f1)" + exit 1; } + # Extracted once, into /opt, which is the copy the PATH below names. + # Upstream also unpacked a second copy into the working directory + # and never used it. + sudo tar zxf musl-native.tgz -C /opt/ + export PATH="/opt/x86_64-linux-musl-native/bin/:${PATH}" + # Keeps the binary dynamically linked against musl, which is what + # makes the libc check in ffi-preflight.yml meaningful. + export RUSTFLAGS="-C target-feature=-crt-static" + pnpm run "$BUILD_SCRIPT" + elif [ "$BUILD_SCRIPT" = zigbuild ]; then + export CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER="$linker" + # cargo-zigbuild >= 0.23.0 no longer reads CARGO_BUILD_TARGET from + # the environment, so the glibc-pinned target is passed as a flag. + pnpm run "$BUILD_SCRIPT" --target "${CARGO_BUILD_TARGET}.2.28" + else + pnpm run "$BUILD_SCRIPT" + fi + + - name: Place the binding in its platform package + working-directory: packages/protect-ffi + env: + PLATFORM: ${{ matrix.cfg.platform }} + BUILD_LOG: ${{ matrix.cfg.log }} + # Bare `neon dist` writes ./index.node — the `debug:` fallback in + # load.cts, which is right for local development and wrong here. The log + # file differs per build script; `neon dist` reads it to locate the + # artifact. + run: | + set -euo pipefail + pnpm exec neon dist -o "platforms/${PLATFORM}/index.node" < "${BUILD_LOG}" + test -s "platforms/${PLATFORM}/index.node" + + # `pnpm pack` writes into the packed package's own directory by default, + # and `--pack-destination` resolves relative to `--dir` rather than to the + # CWD (verified). Packing to the default location and moving the result + # keeps every path in this step relative to the workspace, which is the + # one spelling that behaves identically on the Windows runner. + - name: Pack the platform package + env: + PLATFORM: ${{ matrix.cfg.platform }} + run: | + set -euo pipefail + mkdir -p ffi-dist + pnpm --dir "packages/protect-ffi/platforms/${PLATFORM}" pack + mv "packages/protect-ffi/platforms/${PLATFORM}"/*.tgz ffi-dist/ + ls ffi-dist + + - name: Verify the tarball is the platform package, not the wrapper + env: + PLATFORM: ${{ matrix.cfg.platform }} + run: | + set -euo pipefail + shopt -s nullglob + tarballs=(ffi-dist/*.tgz) + test "${#tarballs[@]}" -eq 1 || { + echo "::error::expected one tarball, found ${#tarballs[@]}"; exit 1; } + tgz="${tarballs[0]}" + name=$(tar xzOf "$tgz" package/package.json | node -p \ + 'JSON.parse(require("node:fs").readFileSync(0,"utf8")).name') + test "$name" = "@cipherstash/protect-ffi-${PLATFORM}" || { + echo "::error::packed $name, expected the ${PLATFORM} platform package" + exit 1; } + tar tzf "$tgz" | grep -qx package/index.node || { + echo "::error::$tgz has no index.node"; exit 1; } + + - uses: actions/upload-artifact@v4 + with: + name: ffi-platform-${{ matrix.cfg.platform }} + path: ffi-dist/*.tgz + if-no-files-found: error + + wrapper: + name: WASM + wrapper tarball + # `needs: [binaries]` is what makes the seven-tarball check below possible: + # this job collects the six platform artifacts and uploads the complete set + # as one artifact, so a caller downloads `ffi-tarballs` and has everything. + needs: [binaries] + runs-on: blacksmith-4vcpu-ubuntu-2404 + timeout-minutes: 45 + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ inputs.ref }} + persist-credentials: false + + - uses: pnpm/action-setup@v6.0.10 + with: + run_install: false + cache: false + + - uses: actions/setup-node@v6.5.0 + with: + node-version: 22 + package-manager-cache: false + + - name: Install node-gyp + run: npm install -g node-gyp + + # wasm-pack, pinned in packages/protect-ffi/mise.toml — `build:wasm` + # shells out to it and nothing else supplies it. `install_args` narrows + # this to wasm-pack alone: a bare `mise install` would also build + # cargo-zigbuild from source, which this job never calls. The argument is + # the full backend id; the short name is not in mise's registry and + # resolves to nothing. + - name: Install wasm-pack + uses: jdx/mise-action@5228313ee0372e111a38da051671ca30fc5a96db # v3.6.3 + with: + install: true + install_args: aqua:wasm-bindgen/wasm-pack + working_directory: packages/protect-ffi + cache: false + + # `--all-targets` in the Rust lint means all target KINDS, not platforms; + # wasm32 is never a side effect of anything else. + - name: Add the wasm32 target + run: rustup target add wasm32-unknown-unknown + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + # The wrapper's `files` include dist/wasm/**. Only three .d.ts are + # tracked; the runtime .js and .wasm are generated here. Without this the + # published `./wasm` and `./wasm-inline` entries resolve to nothing. + - name: Build WASM + working-directory: packages/protect-ffi + run: pnpm run build:wasm + + - name: Pack the wrapper + run: | + set -euo pipefail + mkdir -p ffi-dist + pnpm --dir packages/protect-ffi pack + mv packages/protect-ffi/*.tgz ffi-dist/ + + - name: Verify the wrapper tarball + run: | + set -euo pipefail + tgz=$(ls ffi-dist/cipherstash-protect-ffi-[0-9]*.tgz) + # Every path the manifest's own `exports` map resolves to, walked out + # of the packed manifest rather than listed here. A hand-written list + # is a snapshot: this one was written with five runtime entries and + # silently omitted all three `types` targets, so a wrapper packed + # without declarations passed the gate and shipped type-less to every + # TypeScript consumer. Derived, it also covers a subpath added later. + tar xzOf "$tgz" package/package.json | node -e ' + const j = JSON.parse(require("node:fs").readFileSync(0, "utf8")) + const paths = new Set() + const walk = (node) => { + if (typeof node === "string") { if (node.startsWith("./")) paths.add("package/" + node.slice(2)) } + else if (node && typeof node === "object") { for (const v of Object.values(node)) walk(v) } + } + walk(j.exports) + if (paths.size < 5) { console.error("exports resolved to only " + paths.size + " paths"); process.exit(1) } + // The wasm blob is not an export: protect_ffi.js loads it, so it is + // the one path that has to be named here. Without it both wasm + // entries resolve and then fail at instantiation. (No backticks in + // this argument — shellcheck reads them as command substitution and + // reports SC2016.) + paths.add("package/dist/wasm/protect_ffi_bg.wasm") + console.log([...paths].sort().join("\n")) + ' > exports.txt + cat exports.txt + # The listing once, not once per required path — this loop + # decompressed the whole tarball on every iteration to answer a + # question about the same fixed set of names. + tar tzf "$tgz" > listing.txt + while read -r required ; do + grep -qx "$required" listing.txt || { + echo "::error::$required missing from $tgz"; exit 1; } + done < exports.txt + # The six optionalDependencies must be concrete versions — pnpm + # rewrites `workspace:*` at pack time, and a wrapper published without + # that rewrite resolves no binding for anyone. + tar xzOf "$tgz" package/package.json | node -e ' + const j = JSON.parse(require("node:fs").readFileSync(0, "utf8")) + const deps = Object.entries(j.optionalDependencies ?? {}) + // Against the manifests own "neon.platforms", not a literal 6. The + // invariant is "every declared platform has a matching + // optionalDependency" and both sides are sitting in this object, so + // a seventh platform is covered the day it is added rather than + // failing on a magic number nobody greps. Names too, not just the + // count: six entries naming five platforms twice would satisfy a + // length check. + const want = (j.neon?.platforms ?? []).map((p) => j.neon.org + "/" + j.neon.prefix + p) + if (want.length === 0) { console.error("wrapper declares no neon.platforms"); process.exit(1) } + const got = deps.map(([n]) => n).sort() + if (String(got) !== String([...want].sort())) { + console.error("optionalDependencies are " + got + ", expected " + [...want].sort()); process.exit(1) + } + for (const [n, v] of deps) { + // Concatenation rather than a template literal. Everything in + // this single-quoted argument is read by shellcheck as shell, + // and it reports SC2016 for a dollar-brace or a backtick in + // here — actionlint runs shellcheck over every run block, and + // lint-release.yml turns that into a gate failure. A JS comment + // is no exception: shell sees one string, and a backtick in a + // "//" line up there broke this once. An apostrophe is worse + // still, since it ENDS the argument outright. + // + // Pinned version-independently by + // scripts/__tests__/workflow-inline-node-quoting.test.mjs. The + // gate alone was not enough: actionlint is pinned but shellcheck + // comes from the runner image, and only 0.11.0 began reporting + // the backtick form. + if (!/^\d+\.\d+\.\d+/.test(v)) { console.error(n + " is " + v + ", not a concrete version"); process.exit(1) } + } + console.log("optionalDependencies OK") + ' + + - uses: actions/download-artifact@v4 + with: + pattern: ffi-platform-* + path: ffi-dist + merge-multiple: true + + - name: Verify all seven tarballs are present and distinct + run: | + set -euo pipefail + # A glob into an array rather than `ls | wc -l` (SC2012), with + # `nullglob` so an empty directory counts 0 instead of one literal + # `*.tgz`. + shopt -s nullglob + tarballs=(ffi-dist/*.tgz) + count=${#tarballs[@]} + test "$count" -eq 7 || { + echo "::error::expected 7 tarballs, found $count"; ls ffi-dist; exit 1; } + names=$(for t in "${tarballs[@]}" ; do + tar xzOf "$t" package/package.json | node -p \ + 'JSON.parse(require("node:fs").readFileSync(0,"utf8")).name' + done | sort -u | wc -l) + test "$names" -eq 7 || { + echo "::error::expected 7 distinct package names, found $names"; exit 1; } + + - uses: actions/upload-artifact@v4 + with: + name: ffi-tarballs + path: ffi-dist/*.tgz + if-no-files-found: error diff --git a/.github/workflows/ffi-preflight.yml b/.github/workflows/ffi-preflight.yml new file mode 100644 index 000000000..0600416d1 --- /dev/null +++ b/.github/workflows/ffi-preflight.yml @@ -0,0 +1,151 @@ +name: FFI release pre-flight + +# `changeset publish` has no --dry-run, so this IS the dry run: build the real +# artifacts, check every binary is the architecture and libc its package name +# claims, then install the host-matching pair into a scratch project and use +# them. Point it at the Version Packages PR branch so the tarballs tested carry +# the exact versions that will publish. +# +# It never publishes, and cannot — both authentication paths are absent, which +# takes more than the one line it looks like. No `id-token` permission closes +# OIDC; a plain `NPM_TOKEN` would authenticate a publish regardless, so there is +# also no secret passed (no `secrets: inherit` on the call below), no +# `registry-url` on setup-node (that is what writes an `_authToken` line into +# .npmrc), and no NPM_TOKEN or NODE_AUTH_TOKEN anywhere. Adding any one of them +# turns this dry run into a publisher. + +on: + workflow_dispatch: + inputs: + ref: + description: Ref to build and test (e.g. changeset-release/main) + required: true + type: string + +permissions: + contents: read + +defaults: + run: + shell: bash + +jobs: + artifacts: + name: Build artifacts + uses: ./.github/workflows/_build-ffi-artifacts.yml + with: + ref: ${{ inputs.ref }} + + smoke: + name: Install and smoke-test + needs: [artifacts] + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/download-artifact@v4 + with: + name: ffi-tarballs + path: ffi-dist + + - uses: actions/setup-node@v6.5.0 + with: + node-version: 22 + package-manager-cache: false + + # The five non-host platform tarballs cannot be installed here — npm + # rejects an explicitly-requested package whose os/cpu does not match, + # with EBADPLATFORM — so they are verified statically instead. + # + # ARCHITECTURE, not size. `macos-latest` is arm64, so a darwin-x64 job + # that lost its CARGO_BUILD_TARGET would emit a perfectly valid ARM binary + # of the expected size, install cleanly on an Intel Mac, and fail to + # dlopen. Size checks pass every one of those. + - name: Verify each binary's architecture + run: | + set -euo pipefail + declare -A EXPECT=( + [darwin-arm64]='Mach-O 64-bit.*arm64' + [darwin-x64]='Mach-O 64-bit.*x86_64' + [linux-arm64-gnu]='ELF 64-bit.*ARM aarch64' + [linux-x64-gnu]='ELF 64-bit.*x86-64' + [linux-x64-musl]='ELF 64-bit.*x86-64' + [win32-x64-msvc]='PE32\+.*x86-64' + ) + checked=0 + mkdir -p probe && cd probe + for tgz in ../ffi-dist/*.tgz ; do + name=$(tar xzOf "$tgz" package/package.json | node -p \ + "JSON.parse(require('node:fs').readFileSync(0,'utf8')).name") + platform="${name#@cipherstash/protect-ffi-}" + # The wrapper: its name has no platform suffix, so the strip above + # was a no-op. + [ "$platform" = "$name" ] && continue + test -n "${EXPECT[$platform]+set}" || { + echo "::error::no expected architecture recorded for $platform"; exit 1; } + rm -rf x && mkdir x && tar xzf "$tgz" -C x + desc=$(file -b x/package/index.node) + echo "$platform: $desc" + # Unquoted on purpose: quoting the right-hand side of =~ makes bash + # match it literally, and this is a pattern. + [[ "$desc" =~ ${EXPECT[$platform]} ]] || { + echo "::error::$platform binary is '$desc', expected ${EXPECT[$platform]}" + exit 1; } + + # `file` reads linux-x64-gnu and linux-x64-musl identically — both + # are "ELF 64-bit … x86-64" — so the check above passes if the two + # are swapped, and the failure lands on an Alpine user at runtime. + # The ABI is only visible in the dynamic section: the gnu build + # links libc.so.6, the musl build does not (RUSTFLAGS drops + # crt-static, so it stays dynamic against musl's own libc). + case "$platform" in + linux-x64-gnu|linux-arm64-gnu) + readelf -d x/package/index.node | grep -q 'NEEDED.*libc\.so\.6' || { + echo "::error::$platform does not link glibc"; exit 1; } ;; + linux-x64-musl) + if readelf -d x/package/index.node | grep -q 'NEEDED.*libc\.so\.6' ; then + echo "::error::linux-x64-musl links glibc — it is the gnu binary" + exit 1 + fi + echo "linux-x64-musl: no glibc NEEDED entry" ;; + esac + checked=$((checked + 1)) + done + # Every platform in the table above, or the loop skipped one and + # reported success for a set it never saw. Counted from the table + # rather than hardcoded, so adding a platform does not leave a stale + # number reading as a build defect. + test "$checked" -eq "${#EXPECT[@]}" || { + echo "::error::checked $checked platform binaries, expected ${#EXPECT[@]}" + exit 1; } + + - name: Install the wrapper and the host platform package + run: | + set -euo pipefail + mkdir -p /tmp/smoke && cd /tmp/smoke + echo '{"name":"smoke","version":"1.0.0","type":"module","private":true}' > package.json + # The wrapper's tarball name is the only one where a digit follows + # `protect-ffi-`; every platform package has its platform there. + wrapper=$(ls "$GITHUB_WORKSPACE"/ffi-dist/cipherstash-protect-ffi-[0-9]*.tgz) + host=$(ls "$GITHUB_WORKSPACE"/ffi-dist/*linux-x64-gnu*.tgz) + npm install --no-audit --no-fund "$wrapper" "$host" + + - name: Smoke-test the installed artifact + run: | + set -euo pipefail + cd /tmp/smoke + cat > smoke.mjs <<'EOF' + import { createRequire } from 'node:module' + const require = createRequire(import.meta.url) + const cjs = require('@cipherstash/protect-ffi') + // Since the laziness change a bare require proves nothing — the + // binding resolves on first use, not on import. This forces it, and + // is pure: no client, no credentials, no network. + cjs.assertNativeBindingAvailable() + if (typeof cjs.isEncrypted !== 'function') throw new Error('no isEncrypted') + const wasm = await import('@cipherstash/protect-ffi/wasm') + if (typeof wasm.newClient !== 'function') throw new Error('./wasm did not resolve') + const inline = await import('@cipherstash/protect-ffi/wasm-inline') + if (typeof inline.newClient !== 'function') throw new Error('./wasm-inline did not resolve') + console.log('smoke OK') + EOF + node smoke.mjs diff --git a/.github/workflows/lint-release.yml b/.github/workflows/lint-release.yml new file mode 100644 index 000000000..26c4d147b --- /dev/null +++ b/.github/workflows/lint-release.yml @@ -0,0 +1,85 @@ +name: Lint release tooling + +# The release machinery runs roughly once per release, so a syntax error, a +# shell mistake or an unknown runner label in it surfaces at the worst possible +# moment — mid-release, with a Version PR already merged. This runs on every +# pull request that touches any of it. +# +# actionlint is introduced here and nowhere else in the repo. It reads +# `.github/actionlint.yaml` for the Blacksmith runner label, and it runs +# shellcheck over every `run:` block — which is why the workflows it gates +# avoid `sed`-into-`export` (SC2001, SC2155), `ls | wc -l` (SC2012), and +# dollar-braces or backticks inside single-quoted `node -e` arguments (SC2016). + +on: + pull_request: + # Exactly what the job reads: the four workflows actionlint is pointed at, + # plus the config it resolves the Blacksmith label from. The filter also + # named scripts/release-gate.mjs, scripts/ffi-release-matrix.mjs, + # scripts/lint-no-workflow-caching.mjs and package.json — none of which this + # job looks at, since it deliberately does not run `test:scripts` (see the + # job comment). Editing one booted a runner and downloaded a Go binary to + # lint four unchanged files, which is how a job trains reviewers to ignore + # it. `tests.yml`'s `lint` job runs `test:scripts` unfiltered on every PR, + # so those four are already covered. + # + # `lint-release-scope.test.mjs` asserts this list and the actionlint + # argument list below stay the same set. + paths: + - .github/workflows/release.yml + - .github/workflows/_build-ffi-artifacts.yml + - .github/workflows/ffi-preflight.yml + - .github/workflows/lint-release.yml + - .github/actionlint.yaml + workflow_dispatch: {} + +permissions: + contents: read + +defaults: + run: + shell: bash + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + # No pnpm, no Node, no install: actionlint is a downloaded Go binary and + # shellcheck ships in the runner image, so this job is a checkout and one + # command. + # + # It deliberately does NOT also run `test:scripts` or `lint:workflow-cache`. + # `tests.yml`'s `lint` job runs `test:scripts` on every pull request with no + # path filter, and `tests-supply-chain.yml` runs both — so a copy here would + # be the second and third run of the same checks on any PR touching the + # release machinery, each behind its own uncached full-workspace install. What + # this workflow uniquely has is actionlint. + lint: + name: actionlint (release workflows) + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Install actionlint + run: | + set -euo pipefail + bash <(curl -sSfL https://raw.githubusercontent.com/rhysd/actionlint/v1.7.7/scripts/download-actionlint.bash) 1.7.7 + echo "$PWD" >> "$GITHUB_PATH" + + # Scoped to the release workflows rather than the whole directory: this + # gate has to be green on the day it lands, and actionlint has never run + # over the other twelve. Widening it is its own change, with its own + # findings to work through. + - name: actionlint (release workflows) + run: | + set -euo pipefail + actionlint \ + .github/workflows/release.yml \ + .github/workflows/_build-ffi-artifacts.yml \ + .github/workflows/ffi-preflight.yml \ + .github/workflows/lint-release.yml + diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fedb7f97d..ae03d708a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,11 +1,22 @@ name: Release JS -# Permissions for the workflow to use Trusted Actions +# READ-ONLY BY DEFAULT; the two jobs that publish escalate for themselves. +# +# npm trusted publishing is bound to a repository AND A WORKFLOW FILENAME, so +# once this file is the registered publisher, an OIDC token minted by ANY job in +# it is one npm accepts for a publish — the registry cannot tell `gate` apart +# from `publish-ffi`. These three scopes were declared here, where they are a +# default rather than a ceiling, and `gate` (a checkout and one `node` call) and +# `ffi-artifacts` inherited all of them. +# +# Granting them per job is not the same fix as overriding the two that had them +# wrongly: it makes omission the safe answer, so the next job added to this file +# has to ASK for the publish credential in its own diff. +# # See https://docs.npmjs.com/trusted-publishers#supported-cicd-providers +# Enforced by scripts/__tests__/workflow-publish-permissions.test.mjs. permissions: - id-token: write # Required for OIDC - contents: write # Required for changesets to commit and push - pull-requests: write # Required for changesets to check existing PRs + contents: read on: push: @@ -15,12 +26,217 @@ on: concurrency: ${{ github.workflow }}-${{ github.ref }} jobs: + # WHAT STILL HAS TO BE PUBLISHED, asked of the registry rather than of the + # `.changeset/` directory. "No unconsumed changesets" is also true of an + # ordinary docs commit and of the commit right after a release, so gating the + # native matrix on that would fire it routinely; asking npm which committed + # versions are missing is exact. + # + # This gate is load-bearing. A false negative skips the FFI branch below, and + # `changeset publish` then packs the six platform workspaces — where + # `index.node` is a build output nobody produced — and publishes them. Every + # failure mode in scripts/release-gate.mjs therefore throws rather than + # reporting "nothing to publish". + gate: + name: What needs publishing? + runs-on: ubuntu-latest + timeout-minutes: 10 + # `ffi` only. The gate also computes `js`, and it is in the job log, but no + # job can be keyed on it: `release` below has to run on every push to main + # to open and update the Version Packages PR, published or not. Declaring + # it as an output read nothing and implied a gate that does not exist. + outputs: + ffi: ${{ steps.gate.outputs.ffi }} + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - uses: actions/setup-node@v6.5.0 + with: + node-version: 22 + package-manager-cache: false + + # No pnpm, no install. scripts/release-gate.mjs imports node builtins + # only and shells out to the runner image's npm — deliberately, because + # this job runs on EVERY push to main and no caching is permitted in a + # publishing workflow, so an install here is a cold full-workspace one + # (~1GB, node-pty's node-gyp rebuild included) to answer a question the + # tree already holds. See the header of that script. + - name: Compute the gate + id: gate + run: node scripts/release-gate.mjs + + ffi-artifacts: + name: Build FFI artifacts + needs: [gate] + if: needs.gate.outputs.ffi == 'true' + uses: ./.github/workflows/_build-ffi-artifacts.yml + with: + ref: ${{ github.sha }} + + publish-ffi: + name: Publish FFI packages + needs: [gate, ffi-artifacts] + if: needs.gate.outputs.ffi == 'true' + # GitHub-hosted for the same reason the release job is: npm rejects + # provenance from a self-hosted runner with E422. This job only uploads + # prebuilt tarballs, so it needs no toolchain beyond node and npm. + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: write # the seven git tags and the GitHub release + id-token: write # npm OIDC trusted publishing + steps: + - uses: actions/download-artifact@v4 + with: + name: ffi-tarballs + path: ffi-dist + + # No `registry-url:`. setup-node with one writes a + # `//registry.npmjs.org/:_authToken` line into .npmrc, which shadows OIDC + # and fails every publish with E404. + - uses: actions/setup-node@v6.5.0 + with: + node-version: 22 + package-manager-cache: false + + - name: Upgrade npm for OIDC trusted publishing + run: npm install -g npm@^11.5.1 + + # BEFORE `changeset publish`, deliberately: changesets packs from the + # workspace, where the platform packages have no index.node, so running it + # first would publish six broken tarballs. Once these are on npm, + # changesets skips them ("is not being published because version X is + # already published on npm") and the ordering needs no extra condition. + # + # PLATFORM PACKAGES FIRST, WRAPPER LAST. A plain `*.tgz` glob is + # lexicographic and puts `cipherstash-protect-ffi-0.32.0.tgz` ahead of + # `cipherstash-protect-ffi-darwin-arm64-0.32.0.tgz` ('0' < 'd'), which + # would briefly publish a wrapper whose six optionalDependencies do not + # exist yet — and `npm install` during that window resolves no binding. + # + # Idempotent per tarball, so a re-run after a partial failure completes + # the set instead of aborting on the first already-published package. + - name: Publish the tarballs + id: publish + run: | + set -euo pipefail + meta () { tar xzOf "$1" package/package.json | node -p \ + "JSON.parse(require('node:fs').readFileSync(0,'utf8')).$2"; } + + shopt -s nullglob + wrapper="" + platforms=() + for tgz in ffi-dist/*.tgz ; do + if [ "$(meta "$tgz" name)" = "@cipherstash/protect-ffi" ]; then + wrapper="$tgz" + else + platforms+=("$tgz") + fi + done + test -n "$wrapper" || { echo "::error::no wrapper tarball"; exit 1; } + test "${#platforms[@]}" -eq 6 || { + echo "::error::expected 6 platform tarballs, got ${#platforms[@]}"; exit 1; } + + published=() + for tgz in "${platforms[@]}" "$wrapper" ; do + name=$(meta "$tgz" name) + version=$(meta "$tgz" version) + if npm view "${name}@${version}" version >/dev/null 2>&1; then + echo "${name}@${version} already published — skipping" + else + npm publish --access public --provenance "$tgz" + fi + published+=("${name}@${version}") + done + printf '%s\n' "${published[@]}" > published.txt + echo "version=$(meta "$wrapper" version)" >> "$GITHUB_OUTPUT" + + # Changesets tags only what IT published — `tagPublish` receives + # `publishedPackages.filter(p => p.result === "published")` — and it skips + # these seven as already-published. Without this step an FFI release has + # no git tag and no GitHub release at all. + - name: Tag and release + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + VERSION: ${{ steps.publish.outputs.version }} + run: | + set -euo pipefail + # Existence is not the question — where the tag POINTS is. A re-run + # after a partial failure must find its own tags and move on; a tag at + # a different commit means this version was released from another + # tree, and skipping silently would leave the published artifacts and + # the tagged source disagreeing with nothing in the log to say so. + # + # `git/matching-refs`, not `git/ref`. These tag names carry a slash + # (`@cipherstash/protect-ffi@0.32.0`), and `git/ref/{ref}` answers a + # non-exact match with an ARRAY of refs rather than an object — so + # `--jq .object.sha` yields nothing, the branch below reads "no tag", + # and the create then fails with 422 "Reference already exists" on a + # re-run that should have been a no-op. matching-refs always returns + # an array and an empty one for no match, so filtering it for the + # exact ref is well-defined in every case. + while read -r tag ; do + at=$(gh api "repos/${REPO}/git/matching-refs/tags/${tag}" \ + --jq ".[] | select(.ref == \"refs/tags/${tag}\") | .object.sha" 2>/dev/null || true) + if [ -n "$at" ]; then + test "$at" = "$GITHUB_SHA" || { + echo "::error::tag ${tag} points at ${at}, not ${GITHUB_SHA}"; exit 1; } + echo "tag ${tag} already at this commit — skipping" + else + gh api -X POST "repos/${REPO}/git/refs" \ + -f ref="refs/tags/${tag}" -f sha="$GITHUB_SHA" >/dev/null + echo "created ${tag}" + fi + done < published.txt + + # Attached to the wrapper's own tag, which the loop above just + # created. A `protect-ffi-v` release name would make + # `gh release create` mint an EIGHTH tag for the same commit; + # `--verify-tag` refuses to create a tag that does not already exist. + # The name matches what changesets produces for the JS packages. + rel="@cipherstash/protect-ffi@${VERSION}" + if ! gh release view "$rel" --repo "$REPO" >/dev/null 2>&1; then + gh release create "$rel" --repo "$REPO" --verify-tag \ + --title "protect-ffi v${VERSION}" \ + --notes "Native FFI bindings ${VERSION}. Published: $(tr '\n' ' ' < published.txt)" + fi + # Unconditional, and separate from creation: a release that exists + # with a partial asset set is what a failed re-run leaves behind, so + # skipping on existence is not idempotence. `--clobber` makes the + # complete case a no-op. + gh release upload "$rel" ffi-dist/*.tgz --repo "$REPO" --clobber + release: name: Release + needs: [gate, publish-ffi] + # `always()` because `publish-ffi` is SKIPPED for an ordinary JS release, + # and a skipped dependency would otherwise skip this job too. + # + # The condition has to tell "skipped because FFI was unnecessary" apart from + # "skipped because its prerequisite failed". If `ffi-artifacts` fails, + # `publish-ffi` is SKIPPED rather than failed — so the obvious + # `result != 'failure'` check passes, and `changeset publish` goes on to + # pack and publish the platform workspaces without their binaries. Keyed on + # the gate's own output instead: if FFI was in scope, its publish must have + # SUCCEEDED. + if: >- + always() && + needs.gate.result == 'success' && + ( + needs.gate.outputs.ffi != 'true' || + needs.publish-ffi.result == 'success' + ) # GitHub-hosted (not Blacksmith): npm provenance attestations, which are # generated automatically by OIDC trusted publishing, are only accepted # from github-hosted runners — self-hosted runners are rejected with E422. runs-on: ubuntu-latest + permissions: + id-token: write # npm OIDC trusted publishing + contents: write # changesets commits and pushes the Version Packages branch + pull-requests: write # …and opens/updates the PR for it steps: - name: Checkout Repo uses: actions/checkout@v6 @@ -55,7 +271,7 @@ jobs: run: npm install -g npm@^11.5.1 - name: Install dependencies - run: pnpm install + run: pnpm install --frozen-lockfile - name: Publish to npm id: changesets diff --git a/.github/workflows/tests-supply-chain.yml b/.github/workflows/tests-supply-chain.yml index fba7c8814..8f89fbb77 100644 --- a/.github/workflows/tests-supply-chain.yml +++ b/.github/workflows/tests-supply-chain.yml @@ -24,6 +24,10 @@ on: - main paths: - '.github/workflows/release.yml' + # The reusable workflow release.yml calls: everything it builds is packed + # and published, and it is on the lint's target list for that reason, so + # an edit to it has to run this gate too. + - '.github/workflows/_build-ffi-artifacts.yml' - '.github/workflows/tests-supply-chain.yml' - 'scripts/lint-no-workflow-caching.mjs' - 'scripts/__tests__/lint-no-workflow-caching.test.mjs' @@ -33,6 +37,10 @@ on: - '**' paths: - '.github/workflows/release.yml' + # The reusable workflow release.yml calls: everything it builds is packed + # and published, and it is on the lint's target list for that reason, so + # an edit to it has to run this gate too. + - '.github/workflows/_build-ffi-artifacts.yml' - '.github/workflows/tests-supply-chain.yml' - 'scripts/lint-no-workflow-caching.mjs' - 'scripts/__tests__/lint-no-workflow-caching.test.mjs' @@ -76,5 +84,5 @@ jobs: - name: Run lint script self-tests run: pnpm run test:scripts - - name: Verify no caching in release.yml and tests-supply-chain.yml + - name: Verify no caching in the release and supply-chain workflows run: pnpm run lint:workflow-cache diff --git a/AGENTS.md b/AGENTS.md index 47bffb3e7..da9f95f20 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,8 +140,24 @@ so that stays true for everyone else. Change the package freely — but write the changeset and park it as `.changeset/.md.deferred`, don't skip it. Changesets and the guard both select on `.endsWith('.md')`, so that extension is inert to - `changeset version`; the cutover PR renames it back. `protect-ffi-lazy-load.md.deferred` - is the one already waiting there. + `changeset version`; the cutover PR renames **every** one of them back + (`for f in .changeset/*.md.deferred; do git mv "$f" "${f%.deferred}"; done`). + Check what is parked rather than assuming a single file — `ls + .changeset/*.md.deferred`. Two are waiting today: the lazy native load, and + the manifest repoint to `cipherstash/stack`. +- **The pipeline that will publish them is built and inert.** `release.yml` + asks `scripts/release-gate.mjs` which committed versions are missing from npm; + if any FFI one is, `_build-ffi-artifacts.yml` compiles the six platforms with + an explicit `CARGO_BUILD_TARGET` each, packs all seven tarballs, and + `publish-ffi` publishes the six platform packages **before** the wrapper and + tags all seven — because `changeset publish` packs from the workspace, where + `index.node` does not exist, and tags only what it published itself. Nothing + fires until a version is unpublished, which the changeset guard above + prevents. `ffi-preflight.yml` is the dry run (`changeset publish` has no + `--dry-run`); dispatch it against the Version Packages branch before the + cutover. The seven manifests already name `cipherstash/stack`, which npm + requires of the publishing repository — so a publish attempted from the old + repository would now be rejected, and nothing publishes from there. ### The `integration-tests/` suite diff --git a/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md b/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md index 036045044..be4635bd6 100644 --- a/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md +++ b/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md @@ -14,7 +14,7 @@ - **Publish jobs run on GitHub-hosted runners.** npm rejects provenance from self-hosted with E422. Build matrices may stay on Blacksmith. - **Publish workflows must never restore the GitHub Actions cache.** Enforced by `scripts/lint-no-workflow-caching.mjs`. This is why upstream's `.github/actions/setup` composite action is **not** ported wholesale — it sets `cache: npm` on `setup-node` and `cache: true` on `mise-action`. - **npm >= 11.5.1 is required for OIDC trusted publishing**, installed *after* any `mise-action` step. -- **Trusted publishing binds to `(repository, workflow filename)`.** The job performing a publish must live in the registered file. Reusable workflows are fine for building artifacts, not for publishing. +- **npm validates a trusted publish against the ENTRY-POINT workflow's filename.** Its docs record `workflow_call` as a known issue — *"validation checks the calling workflow's name instead of the workflow that actually contains the publish command"* — and require `id-token: write` in both parent and child. So a publish inside a reusable workflow is validated against whichever workflow called it; keeping the publish in `release.yml`, as an entry-point job, is correct whichever way that resolves. Reusable workflows are for building artifacts. - **`repository.url` must exactly match the publishing GitHub repository** — verified against : *"your package's `repository.url` field in `package.json` must exactly match your GitHub repository."* - **No `NPM_TOKEN` in the release workflow.** `changesets/action` writes a token `.npmrc` that shadows OIDC; every publish then fails with E404 (npm/cli#8976). - **`pnpm pack` takes no positional directory argument.** Use `pnpm --dir pack` or `pnpm --filter pack`. See Task 4. @@ -43,11 +43,15 @@ **Working-tree state is not part of this plan's guarantees.** An earlier revision claimed "working tree clean"; that was true when written and false shortly after. A prior rewrite of this document was lost by being left uncommitted across a branch switch — **commit plan edits in the session that makes them.** -**Remaining: phases 3, 4, 5.** Phase 3 is specified as executable tasks below. Phase 4 contains the only irreversible steps. Phase 5 is blocked until phase 4 publishes. +**Phase 3 is built; phases 4 and 5 remain.** Phase 4 contains the only +irreversible steps and requires seven manual npmjs.com changes. Phase 5 is +blocked until phase 4 publishes. -**Phase 3 Task 0 is committed (`70e1f7da`) and awaiting a CI run.** It is the -prerequisite for everything after it: PR #858's board is red without it, so no -later task can be verified against a green baseline. +The pipeline is inert until a version is unpublished, and +`scripts/lint-no-ffi-changeset.mjs` is what keeps that from happening early: an +FFI changeset stays parked as `.changeset/.md.deferred` until the cutover +PR renames it. Two are waiting — `protect-ffi-lazy-load.md.deferred` and +`protect-ffi-repository-url.md.deferred`. ### Phase 3 progress @@ -61,6 +65,51 @@ later task can be verified against a green baseline. | `dfb8f4d3` | The `integration-tests/` suite runs from a root workflow again | | `48fb5254` | `lintWiring` exemptions made mechanically checkable; dead `release`/`dryrun` scripts removed | | `b5ab1ee5` | Dependabot monitors the in-tree Cargo workspace | +| `dc6db45c` | Task 1 — the crate is `publish = false`, with a guard over every workspace member | +| `6ce84816` | Task 2 — all seven manifests, the crate manifest and the runtime issue URL name `cipherstash/stack` | +| `a9210f69` | Task 3 — `scripts/release-gate.mjs` and its tests | +| `3f6ca3e8` | Task 4 — `_build-ffi-artifacts.yml`, the matrix script, `.github/actionlint.yaml` | +| `c1a40299` | Task 5 — gate → artifacts → publish-ffi → changesets, tags and release included | +| `37c89ecb` | Task 6 — `lint-release.yml` | +| `54fd265e` | Task 7 — `ffi-preflight.yml` | +| `872f6f97` | Task 8 — `packages/protect-ffi/.github/` deleted, `lintWiring` guards its absence | + +**Phase 3 is complete except for one step that cannot run yet.** Task 7 Step 3 +dispatches `ffi-preflight.yml`, and `workflow_dispatch` is resolved from the +default branch — so the button does not exist until this merges. Everything else +is done and verified locally. + +### Where the build departed from this plan + +Six deviations, each a correction to something the plan specified: + +1. **The matrix is a script, not `node -e` in the workflow.** + `scripts/ffi-release-matrix.mjs` plus `scripts/__tests__/ffi-release-matrix.test.mjs`. + The plan itself argues that all three derived fields are silently wrong if + ported verbatim — that is an argument for testing them. The test derives the + log filename from the package's own scripts, so moving a redirect fails it. +2. **`rustup target add`, not `dtolnay/rust-toolchain`.** Every runner image + here ships rustup, `tests-rust.yml` and `build-ffi-binding` already do it + this way, and the alternative means allowlisting a fourth remote action in + the no-caching gate. +3. **Pack to the package directory and `mv`, not `--pack-destination`.** + Verified: under `--dir`, a relative `--pack-destination` resolves against the + package directory rather than the CWD. The plan's absolute + `${{ github.workspace }}/…` spelling would work, but only the relative form + is identical on the Windows runner. +4. **`AUDITED_ACTIONS` needed three entries**, which the plan did not mention: + `actions/upload-artifact`, `actions/download-artifact` and `jdx/mise-action`. + Adding the workflow to `TARGETS` without them fails the gate — correctly, and + loudly. +5. **The caching lint's two copies of the target list are now checked against + each other.** The plan noted that nothing asserted they agree; the test reads + the script's real target list out of its success output, and the per-target + `actions/cache` checks are generated from it. +6. **The frozen-lockfile rule got the guard it was documented to have.** Task 5 + Step 1 fixes `release.yml`'s bare `pnpm install`; the supply-chain e2e check + that was supposed to prevent that read `tests.yml` alone. It now scans every + workflow and every local composite action. Mutation-checked: reverting the + fix fails the new check and leaves the old one green. ### The absorption was audited against upstream @@ -295,7 +344,7 @@ The crate has never been on crates.io (verified via its API) but carries no `pub **Interfaces:** Guard only; nothing consumes it. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```js // scripts/__tests__/cargo-publish-opt-out.test.mjs @@ -330,12 +379,12 @@ describe('cargo publish opt-out', () => { }) ``` -- [ ] **Step 2: Run the test to verify it fails** +- [x] **Step 2: Run the test to verify it fails** Run: `npx vitest run --config scripts/vitest.config.mjs cargo-publish-opt-out` Expected: FAIL — `protect-ffi declares publish = false unless allowlisted` -- [ ] **Step 3: Add the opt-out** +- [x] **Step 3: Add the opt-out** In `packages/protect-ffi/crates/protect-ffi/Cargo.toml`, in `[package]`, after the `version` line: @@ -347,17 +396,17 @@ In `packages/protect-ffi/crates/protect-ffi/Cargo.toml`, in `[package]`, after t publish = false ``` -- [ ] **Step 4: Run the test to verify it passes** +- [x] **Step 4: Run the test to verify it passes** Run: `npx vitest run --config scripts/vitest.config.mjs cargo-publish-opt-out` Expected: PASS -- [ ] **Step 5: Verify cargo still builds** +- [x] **Step 5: Verify cargo still builds** Run: `pnpm --filter @cipherstash/protect-ffi build:native` Expected: exit 0, `index.node` written -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add packages/protect-ffi/crates/protect-ffi/Cargo.toml \ @@ -379,7 +428,7 @@ The six platform manifests also carry `repository.directory: platforms/`, **Interfaces:** Guard only. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```js // scripts/__tests__/ffi-repository-urls.test.mjs @@ -446,12 +495,12 @@ describe('FFI manifests name this repository', () => { }) ``` -- [ ] **Step 2: Run the test to verify it fails** +- [x] **Step 2: Run the test to verify it fails** Run: `npx vitest run --config scripts/vitest.config.mjs ffi-repository-urls` Expected: FAIL — 15 of 16: the seven `repository.url` assertions, the wrapper's `bugs`/`homepage`, the old-repository scan, and the six `repository.directory` paths. Only the manifest count passes. -- [ ] **Step 3: Rewrite the URLs** +- [x] **Step 3: Rewrite the URLs** ```bash cd packages/protect-ffi @@ -460,7 +509,7 @@ perl -0pi -e 's{github\.com/cipherstash/protectjs-ffi}{github.com/cipherstash/st grep -rn "protectjs-ffi" package.json platforms/*/package.json || echo clean ``` -- [ ] **Step 4: Fix each platform's `repository.directory`** +- [x] **Step 4: Fix each platform's `repository.directory`** ```bash for d in platforms/*/ ; do @@ -474,16 +523,16 @@ for d in platforms/*/ ; do done ``` -- [ ] **Step 5: Update the Cargo manifest** +- [x] **Step 5: Update the Cargo manifest** In `packages/protect-ffi/crates/protect-ffi/Cargo.toml`, set any `repository` / `homepage` key to `https://github.com/cipherstash/stack`. The crate is `publish = false`, so this is documentation rather than a registry requirement — but a wrong URL in a shipped manifest is still wrong. -- [ ] **Step 6: Run the test to verify it passes** +- [x] **Step 6: Run the test to verify it passes** Run: `npx vitest run --config scripts/vitest.config.mjs ffi-repository-urls` Expected: PASS, 16 tests. Step 3 alone reaches only 10 of them — the six `directory` assertions are what makes skipping Step 4 visible. -- [ ] **Step 7: Verify the packed manifest carries the new URL** +- [x] **Step 7: Verify the packed manifest carries the new URL** ```bash pnpm --dir packages/protect-ffi pack --pack-destination /tmp/urlcheck @@ -491,7 +540,7 @@ tar xzOf /tmp/urlcheck/*.tgz package/package.json | grep -A2 '"repository"' ``` Expected: `cipherstash/stack` -- [ ] **Step 8: Commit** +- [x] **Step 8: Commit** ```bash git add packages/protect-ffi/package.json packages/protect-ffi/platforms \ @@ -511,7 +560,7 @@ git commit -m "chore(protect-ffi): point the manifests at cipherstash/stack" **Interfaces:** - Produces `unpublished(manifests, lookup): string[]` — `manifests` is `[{name, version, private?}]`, `lookup(name)` returns published versions or `null` for a 404; `classify(names): { ffi: boolean, js: boolean }`; CLI writes `ffi=`, `js=`, `unpublished=` to `$GITHUB_OUTPUT`. -- [ ] **Step 1: Write the failing test** +- [x] **Step 1: Write the failing test** ```js // scripts/__tests__/release-gate.test.mjs @@ -581,12 +630,12 @@ describe('classify', () => { }) ``` -- [ ] **Step 2: Run the test to verify it fails** +- [x] **Step 2: Run the test to verify it fails** Run: `npx vitest run --config scripts/vitest.config.mjs release-gate` Expected: FAIL — `Failed to resolve import "../release-gate.mjs"` -- [ ] **Step 3: Write the script** +- [x] **Step 3: Write the script** ```js // scripts/release-gate.mjs @@ -689,17 +738,17 @@ function main() { if (process.argv[1] === fileURLToPath(import.meta.url)) main() ``` -- [ ] **Step 4: Run the test to verify it passes** +- [x] **Step 4: Run the test to verify it passes** Run: `npx vitest run --config scripts/vitest.config.mjs release-gate` Expected: PASS, 10 tests -- [ ] **Step 5: Run the script against the live registry** +- [x] **Step 5: Run the script against the live registry** Run: `node scripts/release-gate.mjs` Expected: `nothing to publish — every committed version is on the registry`, then `ffi=false js=false`. -- [ ] **Step 6: Add the npm script and commit** +- [x] **Step 6: Add the npm script and commit** In root `package.json` `scripts`, before `"release"`: `"release:gate": "node scripts/release-gate.mjs",` @@ -724,7 +773,7 @@ Action pins match the rest of this repo (`actions/checkout@v6`, `actions/setup-n **Interfaces:** Produces artifact `ffi-tarballs` — seven `.tgz` files, downloaded **by name** in Tasks 5 and 7. Not exposed as a `workflow_call` output: a reusable workflow's `outputs..value` has to map to a job output (`${{ jobs.x.outputs.y }}`), a literal string is not that, and no caller reads one. -- [ ] **Step 1: Write the workflow** +- [x] **Step 1: Write the workflow** ```yaml # .github/workflows/_build-ffi-artifacts.yml @@ -1098,7 +1147,7 @@ jobs: if-no-files-found: error ``` -- [ ] **Step 2: Teach actionlint the self-hosted runner label** +- [x] **Step 2: Teach actionlint the self-hosted runner label** Thirteen jobs already run on `blacksmith-4vcpu-ubuntu-2404` and nothing has ever complained, because **actionlint has never run in this repo** — Task 6 is what introduces it. Its `runner-label` check knows only GitHub-hosted labels, so without this file every Blacksmith job is an error and the new gate is red on arrival: @@ -1112,7 +1161,7 @@ self-hosted-runner: - blacksmith-4vcpu-ubuntu-2404 ``` -- [ ] **Step 3: Lint the workflow** +- [x] **Step 3: Lint the workflow** ```bash bash <(curl -sSfL https://raw.githubusercontent.com/rhysd/actionlint/v1.7.7/scripts/download-actionlint.bash) 1.7.7 @@ -1123,12 +1172,12 @@ Expected: both clean — verified against the workflow above as written. actionlint bundles shellcheck and applies it to every `run:` block, which is why the snippet departs from upstream in three places that look like style: parameter expansion instead of `sed` into `export` (SC2001, SC2155), a glob-into-array instead of `ls | wc -l` (SC2012), and string concatenation instead of a template literal inside a single-quoted `node -e` (SC2016 — shellcheck reads dollar-brace as a shell expansion). Reintroduce any of them and Task 6's gate is red. -- [ ] **Step 4: Verify the target mapping locally** +- [x] **Step 4: Verify the target mapping locally** Run: `pnpm --dir packages/protect-ffi exec neon list-platforms` Expected: JSON mapping all six platform names to Rust triples (`darwin-x64` → `x86_64-apple-darwin`, etc.). This is the mapping the matrix depends on. -- [ ] **Step 5: Verify the two build scripts and their log files** +- [x] **Step 5: Verify the two build scripts and their log files** ```bash node -p "JSON.stringify(require('./packages/protect-ffi/package.json').scripts, null, 1)" \ @@ -1136,7 +1185,7 @@ node -p "JSON.stringify(require('./packages/protect-ffi/package.json').scripts, ``` Expected: `build` is `tsc` and nothing else — the matrix must select **`build:native`**, not `build`, for the non-gnu platforms. `build:native` → `cargo-build` → `> cargo.log`; `zigbuild` → `zig-build` → `> zig.log`. The `log` field in the matrix exists because those two differ and `neon dist` reads one of them. -- [ ] **Step 6: Commit** +- [x] **Step 6: Commit** ```bash git add .github/workflows/_build-ffi-artifacts.yml .github/actionlint.yaml @@ -1152,7 +1201,7 @@ git commit -m "ci: add the reusable FFI artifact build workflow" **Interfaces:** Consumes gate outputs `ffi` / `js` (Task 3) and artifact `ffi-tarballs` (Task 4). -- [ ] **Step 1: Fix the existing install** +- [x] **Step 1: Fix the existing install** `release.yml` runs bare `pnpm install`, violating the repository's own rule ("CI uses `pnpm install --frozen-lockfile`. Don't drop the flag."): @@ -1161,7 +1210,7 @@ git commit -m "ci: add the reusable FFI artifact build workflow" run: pnpm install --frozen-lockfile ``` -- [ ] **Step 2: Add the gate job** +- [x] **Step 2: Add the gate job** ```yaml gate: @@ -1189,7 +1238,7 @@ git commit -m "ci: add the reusable FFI artifact build workflow" run: node scripts/release-gate.mjs ``` -- [ ] **Step 3: Add the artifact and publish jobs** +- [x] **Step 3: Add the artifact and publish jobs** ```yaml ffi-artifacts: @@ -1319,7 +1368,7 @@ git commit -m "ci: add the reusable FFI artifact build workflow" gh release upload "$rel" ./ffi-dist/*.tgz --repo "$REPO" --clobber ``` -- [ ] **Step 4: Order the changesets job correctly** +- [x] **Step 4: Order the changesets job correctly** ```yaml release: @@ -1343,7 +1392,7 @@ git commit -m "ci: add the reusable FFI artifact build workflow" runs-on: ubuntu-latest ``` -- [ ] **Step 5: Register the new workflow with the caching lint** +- [x] **Step 5: Register the new workflow with the caching lint** In `scripts/lint-no-workflow-caching.mjs`, `TARGETS`: @@ -1357,7 +1406,7 @@ In `scripts/lint-no-workflow-caching.mjs`, `TARGETS`: Adding the target is what makes the three `package-manager-cache: false` lines in Task 4 load-bearing rather than decorative. -- [ ] **Step 6: Verify** +- [x] **Step 6: Verify** ```bash node scripts/lint-no-workflow-caching.mjs @@ -1366,7 +1415,7 @@ npx vitest run --config scripts/vitest.config.mjs lint-no-workflow-caching ``` Expected: the lint names all three workflows, tests pass. Reverting one `package-manager-cache: false` in `_build-ffi-artifacts.yml` must turn the lint red — if it does not, the target never registered. -- [ ] **Step 7: Commit** +- [x] **Step 7: Commit** ```bash git add .github/workflows/release.yml scripts/lint-no-workflow-caching.mjs \ @@ -1380,7 +1429,7 @@ git commit -m "ci(release): publish FFI tarballs, tagged, before changeset publi **Files:** Create `.github/workflows/lint-release.yml` -- [ ] **Step 1: Write the workflow** +- [x] **Step 1: Write the workflow** ```yaml # .github/workflows/lint-release.yml @@ -1458,12 +1507,12 @@ jobs: run: pnpm run lint:workflow-cache ``` -- [ ] **Step 2: Lint it with itself** +- [x] **Step 2: Lint it with itself** Run: `./actionlint .github/workflows/lint-release.yml` Expected: clean -- [ ] **Step 3: Commit** +- [x] **Step 3: Commit** ```bash git add .github/workflows/lint-release.yml @@ -1478,7 +1527,7 @@ git commit -m "ci: gate the release machinery on actionlint and script tests" **Files:** Create `.github/workflows/ffi-preflight.yml` -- [ ] **Step 1: Write the workflow** +- [x] **Step 1: Write the workflow** ```yaml # .github/workflows/ffi-preflight.yml @@ -1604,7 +1653,7 @@ jobs: node smoke.mjs ``` -- [ ] **Step 2: actionlint** +- [x] **Step 2: actionlint** Run: `./actionlint .github/workflows/ffi-preflight.yml` Expected: clean @@ -1623,7 +1672,7 @@ gh run watch Expected: green. First end-to-end proof of matrix, target selection, packing, architecture and install. -- [ ] **Step 4: Commit** +- [x] **Step 4: Commit** ```bash git add .github/workflows/ffi-preflight.yml @@ -1763,7 +1812,7 @@ const testWorkflow = read('../../.github/workflows/tests-rust.yml') Run: `pnpm --filter @cipherstash/protect-ffi test` Expected: PASS, 79 tests -- [ ] **Step 5: Delete the deposited upstream workflows** — *blocked on Task 4* +- [x] **Step 5: Delete the deposited upstream workflows** — *blocked on Task 4* Only once Tasks 4, 7 and 8 have consumed all of the reference material. Task 4 still cites `build.yml` and `actions/setup/action.yml`, so this cannot run yet. @@ -1772,14 +1821,14 @@ still cites `build.yml` and `actions/setup/action.yml`, so this cannot run yet. git rm -r packages/protect-ffi/.github ``` -- [ ] **Step 6: Verify nothing else referenced them** +- [x] **Step 6: Verify nothing else referenced them** ```bash grep -rn "protect-ffi/.github" --include="*.ts" --include="*.mjs" --include="*.yml" --include="*.md" . | grep -v node_modules ``` Expected: no hits outside `docs/plans/` and `.work/` -- [ ] **Step 7: Run the repo linters** +- [x] **Step 7: Run the repo linters** ```bash node scripts/lint-no-dead-package-paths.mjs @@ -1787,7 +1836,7 @@ node scripts/lint-no-dead-package-paths.mjs pnpm run test:scripts ``` -- [ ] **Step 8: Commit** +- [x] **Step 8: Commit** ```bash git add .github/workflows/tests-rust.yml packages/protect-ffi/src/lintWiring.test.ts @@ -1890,9 +1939,9 @@ The Rust emitting EQL payloads is generated from a different catalog commit than - [ ] Re-running the publish job on the same commit is a no-op that still completes a partial asset set (tag targets verified, `release upload --clobber`) - [ ] Stack tags are still created by changesets on the same run - [ ] `ffi-preflight.yml` runs against a release-PR ref without any publish step -- [ ] `release.yml` installs with `--frozen-lockfile` +- [x] `release.yml` installs with `--frozen-lockfile` — and the supply-chain e2e check now scans every workflow and composite action for it, not `tests.yml` alone - [x] `cargo test`, `cargo fmt --check` and clippy (host **and** wasm32) run in CI again — `tests-rust.yml` (`bc0cb132`) -- [ ] No dead upstream workflow files under `packages/protect-ffi/.github/` +- [x] No dead upstream workflow files under `packages/protect-ffi/.github/` — deleted, and `lintWiring.test.ts` now fails if the directory comes back - [x] `lintWiring.test.ts` asserts against a workflow GitHub actually executes - [x] `test:typecheck:wasm` runs in CI, and its exemption is checked against the root workflow **directory** rather than a hardcoded filename - [x] No package script dispatches a workflow this repo cannot run — trigger and declared inputs, not just the path diff --git a/e2e/tests/supply-chain.e2e.test.ts b/e2e/tests/supply-chain.e2e.test.ts index 253dd091b..6c991fd73 100644 --- a/e2e/tests/supply-chain.e2e.test.ts +++ b/e2e/tests/supply-chain.e2e.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process' -import { existsSync, globSync, readFileSync } from 'node:fs' +import { existsSync, globSync, readdirSync, readFileSync } from 'node:fs' import { basename, dirname, join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import semver from 'semver' @@ -249,6 +249,189 @@ describe('supply chain — pnpm-lock.yaml integrity', () => { }) }) +describe('supply chain — every workflow installs with --frozen-lockfile', () => { + /** + * The rule is "CI uses `pnpm install --frozen-lockfile`", and it was checked + * in one workflow. `release.yml` — the one workflow that publishes to npm — + * ran a bare `pnpm install` from the day it was written, so the single + * install permitted to resolve outside the lockfile was the one whose output + * goes to the registry. A lockfile-free install there can pick up a version + * nobody reviewed and publish artifacts built against it. + * + * Scanned across the directory, and through local composite actions, for the + * same reason the caching gate is: `.github/actions/integration-setup` + * installs on behalf of four jobs, and a rule that stops at the workflow file + * makes "move it into a composite" the way around it. + */ + /** + * `pnpm … install` / `pnpm … i` as a whole token. + * + * The `i` alternative is not decoration: `scripts/__tests__/workflow-node-gyp.test.mjs` + * matches it and pins `pnpm i --frozen-lockfile` as a spelling this repo uses, + * so a narrower pattern here would leave the two guards disagreeing about what + * an install IS — the node-gyp rule would cover a `pnpm i` step and this one + * would not. + */ + const PNPM_INSTALL = /(?:^|[\s;&|(])pnpm\s+(?:[^\n]*\s)?(?:install|i)(?:\s|$)/ + + /** + * A step's `run:` body as the commands it actually runs, one per entry. + * + * PER COMMAND, not per step, and the difference is a real hole rather than a + * refinement: a step holding both `pnpm install` and, later, + * `pnpm install --frozen-lockfile` satisfies a whole-body search for the flag + * while still running the unpinned install. + * + * A NEWLINE IS NOT WHAT SEPARATES TWO COMMANDS — shell separators do, and + * splitting on newlines alone left `pnpm install; pnpm install + * --frozen-lockfile` as one "command" carrying the flag, which is the same + * defect one level down. `;`, `&&`, `||`, `|` and a trailing `&` all start a + * new command, so all of them split. + * + * The split is quote-blind: `echo "pnpm install; ok"` becomes two fragments. + * That is a false POSITIVE at worst — a reported offender that is not one — + * and the pattern was already quote-blind before this, since it matched + * `pnpm install` inside an `echo` just the same. Fail-closed is the right + * direction here. + * + * Backslash continuations are joined first, so an install whose flag sits on + * the next line is still one command and still passes. + */ + const commandsOf = (run: string): string[] => + run + .replace(/\\\r?\n\s*/g, ' ') + .split(/\r?\n|;|&&|\|\||\||&/) + .map((fragment) => fragment.trim()) + .filter((fragment) => fragment !== '' && !fragment.startsWith('#')) + + /** + * The installs in one `run:` body that resolve outside the lockfile. + * + * One definition, used by both the property test and the repo sweep below — + * two copies of the predicate would let the test pin a rule the sweep no + * longer applies. + */ + const unpinnedInstalls = (run: string): string[] => + commandsOf(run).filter( + (command) => + PNPM_INSTALL.test(command) && !/--frozen-lockfile\b/.test(command), + ) + + const stepsOf = (relPath: string): Array<{ run?: string }> => { + const doc = readYaml(relPath) as { + jobs?: Record }> + runs?: { steps?: Array<{ run?: string }> } + } + return [ + ...Object.values(doc?.jobs ?? {}).flatMap((job) => job?.steps ?? []), + ...(doc?.runs?.steps ?? []), + ] + } + + const files = [ + ...globSync('.github/workflows/*.{yml,yaml}', { cwd: REPO_ROOT }), + // `**` because a composite action does not have to sit one level down: + // `uses: ./.github/actions/group/name` is valid, and the flat glob that + // covers today's four would silently stop covering a grouped one. Workflows + // stay flat on purpose — GitHub reads `.github/workflows/*` and does not + // descend, so a nested file there is not a workflow at all. + ...globSync('.github/actions/**/action.{yml,yaml}', { cwd: REPO_ROOT }), + ].sort() + + it('scans every workflow GitHub reads, with no slack', () => { + // Equality against the directory, not a floor with room in it: a + // `length > N` guard lets N files drop out of the scan before anything + // notices, and this repo has already been bitten by that shape (see the + // mutation note in scripts/__tests__/ffi-binding-step-order.test.mjs). The + // offender check below is one aggregated assertion, so a file falling out + // of `files` does not delete a visible test — it silently narrows this one. + const workflows = readdirSync(join(REPO_ROOT, '.github/workflows')) + .filter((name) => /\.ya?ml$/.test(name)) + .map((name) => `.github/workflows/${name}`) + .sort() + expect(files.filter((f) => f.startsWith('.github/workflows/'))).toEqual( + workflows, + ) + // The composite half cannot be compared to a directory listing the same + // way — `.github/actions/*` holds directories, not manifests — so it gets + // the floor the other half no longer needs. + expect(files).toContain('.github/actions/integration-setup/action.yml') + }) + + it('reads one command at a time, so a later flag cannot cover an earlier install', () => { + // The property, pinned against synthetic input rather than against the + // repo: a repo that happens to be clean says nothing about how strict the + // checker is. A step holding both installs is the shape a whole-body search + // for the flag waves through. + expect( + unpinnedInstalls('pnpm install\npnpm install --frozen-lockfile'), + ).toEqual(['pnpm install']) + + // A continuation is one command, not two, so the flag on the next line + // still counts. + expect(unpinnedInstalls('pnpm install \\\n --frozen-lockfile')).toEqual([]) + + // A comment naming the flag is not the flag. + expect( + unpinnedInstalls('# always --frozen-lockfile\npnpm install'), + ).toEqual(['pnpm install']) + + // `pnpm i` is an install. The node-gyp guard already treats it as one. + expect(unpinnedInstalls('pnpm i')).toEqual(['pnpm i']) + expect(unpinnedInstalls('pnpm i --frozen-lockfile')).toEqual([]) + + // …and `pnpm run install-x` is not, despite containing the word. + expect(unpinnedInstalls('pnpm run install-deps')).toEqual([]) + }) + + it('splits shell separators, so one line cannot hide an unpinned install', () => { + // The same defect as the multi-line case, one level down. Splitting on + // newlines alone leaves `pnpm install; pnpm install --frozen-lockfile` as a + // single "command" that contains the flag — so the guard reports nothing + // while the first install resolves outside the lockfile. A `run:` block is + // shell, and shell does not need a newline to run two commands. + expect( + unpinnedInstalls('pnpm install; pnpm install --frozen-lockfile'), + ).toEqual(['pnpm install']) + + // Every separator that starts a new command, not just `;`. + expect( + unpinnedInstalls('pnpm install --frozen-lockfile && pnpm install'), + ).toEqual(['pnpm install']) + expect( + unpinnedInstalls('pnpm i || pnpm install --frozen-lockfile'), + ).toEqual(['pnpm i']) + expect(unpinnedInstalls('echo start | pnpm install')).toEqual([ + 'pnpm install', + ]) + + // A pinned install keeps passing when it shares a line with other work — + // the split must not turn the flag into a different command from the + // install it belongs to. + expect( + unpinnedInstalls( + 'echo start && pnpm install --frozen-lockfile && echo done', + ), + ).toEqual([]) + }) + + it('carries --frozen-lockfile on every install', () => { + const offenders = files.flatMap((file) => + stepsOf(file) + .filter((step) => typeof step.run === 'string') + .flatMap((step) => + unpinnedInstalls(step.run as string).map( + (command) => `${file}: ${command}`, + ), + ), + ) + expect( + offenders, + 'A CI install that is not `--frozen-lockfile` resolves versions the lockfile does not name, with no review and no record.', + ).toEqual([]) + }) +}) + describe('supply chain — CI hardening (.github/workflows/tests.yml)', () => { const workflow = readYaml('.github/workflows/tests.yml') as { jobs: Record< @@ -264,22 +447,12 @@ describe('supply chain — CI hardening (.github/workflows/tests.yml)', () => { > } - it('every `pnpm install` invocation uses --frozen-lockfile', () => { - // Allow flag tokens (e.g. `pnpm --filter=foo install`, `pnpm -w install`) - // between `pnpm` and `install`, but not arbitrary words — that would - // false-match scripts like `pnpm run install-x`. - const PNPM_INSTALL = /\bpnpm\b(?:\s+-{1,2}\S+)*\s+install\b/ - for (const [jobName, job] of Object.entries(workflow.jobs)) { - const installSteps = job.steps.filter( - (s) => typeof s.run === 'string' && PNPM_INSTALL.test(s.run), - ) - for (const step of installSteps) { - expect(step.run, `${jobName} step "${step.run}"`).toMatch( - /--frozen-lockfile/, - ) - } - } - }) + // The `--frozen-lockfile` check that used to live here is gone, not moved by + // accident: the describe above supersedes it in both directions — every + // workflow and composite rather than this one file, and per command rather + // than per step body. Keeping the narrower copy would be worse than + // redundant, because it PASSES on inputs the broader one fails, so a reader + // who found it first would get a wrong answer about what this repo enforces. it('every pnpm-using job runs on Node 22 (literal or matrix incl. 22)', () => { for (const [jobName, job] of Object.entries(workflow.jobs)) { diff --git a/package.json b/package.json index 4bf111828..bcf28b542 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "lint:runners": "node scripts/lint-no-hardcoded-runners.mjs", "lint:typecheck-scope": "node scripts/lint-typecheck-scope.mjs", "lint:workflow-cache": "node scripts/lint-no-workflow-caching.mjs", + "release:gate": "node scripts/release-gate.mjs", "release": "pnpm run build && changeset publish", "test": "turbo test --filter './packages/*'", "test:e2e": "turbo run test:e2e", diff --git a/packages/protect-ffi/.github/.env b/packages/protect-ffi/.github/.env deleted file mode 100644 index cbe24b0d8..000000000 --- a/packages/protect-ffi/.github/.env +++ /dev/null @@ -1,5 +0,0 @@ -NODE_VERSION=20.x -NPM_REGISTRY=https://registry.npmjs.org -RUST_VERSION=stable -ACTIONS_USER=github-actions -ACTIONS_EMAIL=github-actions@github.com diff --git a/packages/protect-ffi/.github/actions/setup/action.yml b/packages/protect-ffi/.github/actions/setup/action.yml deleted file mode 100644 index c80509533..000000000 --- a/packages/protect-ffi/.github/actions/setup/action.yml +++ /dev/null @@ -1,88 +0,0 @@ -name: 'Setup Neon' -description: 'Setup the Neon toolchain.' -inputs: - platform: - description: 'Platform being built for.' - required: false - default: '' - use-rust: - description: 'Install Rust?' - required: false - default: 'true' - use-cross: - description: 'Install cross-rs?' - required: false - default: 'false' - workspace: - description: 'Path to workspace being setup.' - required: false - default: '.' -outputs: - rust: - description: 'Rust version installed.' - value: ${{ steps.rust.outputs.version }} - node: - description: 'Node version installed.' - value: ${{ steps.node.outputs.version }} - target: - description: 'Rust target architecture installed.' - value: ${{ steps.target.outputs.target }} -runs: - using: "composite" - steps: - - name: Set Environment Variables - uses: falti/dotenv-action@d1cd55661714e830a6e26f608f81d36e23424fed # v1.1.2 - with: - path: ./.github/.env - export-variables: true - keys-case: bypass - - - name: Install Node - uses: actions/setup-node@v3 - with: - node-version: ${{ env.NODE_VERSION }} - registry-url: ${{ env.NPM_REGISTRY }} - cache: npm - - - name: Install Dependencies - shell: bash - run: npm ci - - - name: Compute Rust Target - if: ${{ inputs['use-rust'] == 'true' }} - id: target - shell: bash - run: echo target=$(npx neon list-platforms | jq -r '.["${{ inputs.platform }}"]') | tee -a $GITHUB_OUTPUT - working-directory: ${{ inputs.workspace }} - - - name: Install Rust - if: ${{ inputs['use-rust'] == 'true' }} - uses: dtolnay/rust-toolchain@stable - with: - toolchain: ${{ env.RUST_VERSION }} - target: ${{ steps.target.outputs.target }} - components: clippy, rustfmt - - - uses: jdx/mise-action@v2 - with: - install: true # [default: true] run `mise install` - cache: true # [default: true] cache mise using GitHub's cache - - - name: Install cross-rs - if: ${{ inputs['use-cross'] == 'true' }} - uses: baptiste0928/cargo-install@v2 - with: - crate: cross - - - name: Node Version - id: node - shell: bash - run: | - echo node_version=$(node -e 'console.log(process.versions.node)') | tee -a $GITHUB_OUTPUT - - - name: Rust Version - if: ${{ inputs['use-rust'] == 'true' }} - id: rust - shell: bash - run: | - echo rust_version=$(cargo -Vv | fgrep release: | cut -d' ' -f2) | tee -a $GITHUB_OUTPUT diff --git a/packages/protect-ffi/.github/workflows/build.yml b/packages/protect-ffi/.github/workflows/build.yml deleted file mode 100644 index 6f2cf6c52..000000000 --- a/packages/protect-ffi/.github/workflows/build.yml +++ /dev/null @@ -1,291 +0,0 @@ -name: Build - -on: - workflow_call: - inputs: - ref: - description: 'The branch, tag, or SHA to check out' - required: true - type: string - update-version: - description: 'Update version before building?' - required: false - type: boolean - default: false - version: - description: 'Version update (ignored if update-version is false)' - required: false - type: string - default: 'patch' - github-release: - description: 'Publish GitHub release?' - required: false - type: boolean - default: false - tag: - description: 'The release tag (ignored if github-release is false)' - required: false - type: string - default: '' - -jobs: - matrix: - name: Matrix - runs-on: blacksmith-4vcpu-ubuntu-2404 - outputs: - matrix: ${{ steps.matrix.outputs.result }} - steps: - - name: Checkout Code - uses: actions/checkout@v3 - with: - ref: ${{ inputs.ref }} - - name: Setup Neon Environment - uses: ./.github/actions/setup - with: - use-rust: false - - name: Look Up Matrix Data - id: matrixData - shell: bash - run: | - cat package.json - echo "json=$(npx neon show ci github | jq -rc)" | tee -a $GITHUB_OUTPUT - - name: Compute Matrix - id: matrix - uses: actions/github-script@v7 - with: - script: | - const platforms = ${{ steps.matrixData.outputs.json }}; - const macOS = platforms.macOS.map(platform => { - return { os: "macos-latest", platform, script: "build" }; - }); - const windows = platforms.Windows.map(platform => { - return { os: "windows-latest", platform, script: "build" }; - }); - const linux = platforms.Linux.map(platform => { - const script = platform.includes('gnu') ? "zigbuild" : "build"; - return { os: "blacksmith-4vcpu-ubuntu-2404", platform, script: script }; - }); - return [...macOS, ...windows, ...linux]; - - binaries: - name: Binaries - needs: [matrix] - strategy: - matrix: - cfg: ${{ fromJSON(needs.matrix.outputs.matrix) }} - runs-on: ${{ matrix.cfg.os }} - permissions: - contents: write - steps: - - name: Checkout Code - uses: actions/checkout@v3 - with: - ref: ${{ inputs.ref }} - - name: Install OpenSSL (Windows) - if: ${{ matrix.cfg.os == 'windows-latest' }} - shell: powershell - run: | - vcpkg install openssl:x64-windows-static - $vcpkgRoot = $env:VCPKG_INSTALLATION_ROOT - $opensslDir = "$vcpkgRoot\installed\x64-windows-static" - echo "OPENSSL_DIR=$opensslDir" >> $env:GITHUB_ENV - echo "OPENSSL_LIB_DIR=$opensslDir\lib" >> $env:GITHUB_ENV - echo "OPENSSL_INCLUDE_DIR=$opensslDir\include" >> $env:GITHUB_ENV - echo "OPENSSL_STATIC=1" >> $env:GITHUB_ENV - - name: Install cross-compile toolchain - if: ${{ contains(matrix.cfg.platform, 'linux') }} - run: | - sudo apt-get update - sudo apt-get install -y gcc-13-aarch64-linux-gnu - sudo ln -s /usr/bin/aarch64-linux-gnu-gcc-13 /usr/bin/aarch64-linux-gnu-gcc - - name: Setup Neon Environment - id: neon - uses: ./.github/actions/setup - with: - use-cross: ${{ matrix.cfg.script == 'cross' }} - platform: ${{ matrix.cfg.platform }} - - name: Update Version - if: ${{ inputs.update-version }} - shell: bash - env: - # Bind the input to an env var so it never expands into the shell - # body — guards against template-injection via `${{ }}` in `run:`. - # Keep this guard in sync with the `wasm` job and release.yml's Tag - # Release step. - VERSION_INPUT: ${{ inputs.version }} - run: | - git config --global user.name $ACTIONS_USER - git config --global user.email $ACTIONS_EMAIL - # Allowlist gate: only npm `version` bump keywords or strict semver - # (with optional `v` prefix and pre-release suffix) get through. - case "$VERSION_INPUT" in - patch|minor|major|prepatch|preminor|premajor|prerelease) ;; - v[0-9]*.[0-9]*.[0-9]*|[0-9]*.[0-9]*.[0-9]*) ;; - v[0-9]*.[0-9]*.[0-9]*-*|[0-9]*.[0-9]*.[0-9]*-*) ;; - *) - echo "Invalid version input: '$VERSION_INPUT' (expected an npm bump keyword or semver)" >&2 - exit 1 - ;; - esac - npm version "$VERSION_INPUT" -m "v%s" - - name: Build - shell: bash - env: - CARGO_BUILD_TARGET: ${{ steps.neon.outputs.target }} - NEON_BUILD_PLATFORM: ${{ matrix.cfg.platform }} - run: | - export CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER=$(echo ${CARGO_BUILD_TARGET} | sed 's/unknown-//')-gcc - export CARGO_TARGET_X86_64_UNKNOWN_LINUX_MUSL_LINKER=$(echo ${CARGO_BUILD_TARGET} | sed 's/unknown-//')-gcc - if [[ $CARGO_BUILD_TARGET =~ musl ]]; then - wget -4 https://musl.cc/x86_64-linux-musl-native.tgz - tar zxvf x86_64-linux-musl-native.tgz - sudo tar zxvf x86_64-linux-musl-native.tgz -C /opt/ - export PATH="/opt/x86_64-linux-musl-native/bin/:${PATH}" - export RUSTFLAGS="-C target-feature=-crt-static" - npm run ${{ matrix.cfg.script }} - elif [[ $CARGO_BUILD_TARGET =~ gnu ]]; then - # Pass the glibc-versioned target as a flag: cargo-zigbuild >= 0.23.0 - # no longer reads CARGO_BUILD_TARGET from the environment. - npm run ${{ matrix.cfg.script }} -- --target "${CARGO_BUILD_TARGET}.2.28" - else - npm run ${{ matrix.cfg.script }} - fi - - name: Pack - id: pack - shell: bash - run: | - mkdir -p dist - echo filename=$(basename $(npm pack ./platforms/${{ matrix.cfg.platform }} --silent --pack-destination=./dist --json | jq -r '.[0].filename')) | tee -a $GITHUB_OUTPUT - - name: Release - if: ${{ inputs.github-release }} - uses: softprops/action-gh-release@9d7c94cfd0a1f3ed45544c887983e9fa900f0564 # v2.0.4 - with: - files: ./dist/${{ steps.pack.outputs.filename }} - tag_name: ${{ inputs.tag }} - - wasm: - name: Wasm - needs: [matrix] - runs-on: blacksmith-4vcpu-ubuntu-2404 - steps: - - name: Checkout Code - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - ref: ${{ inputs.ref }} - persist-credentials: false - - name: Setup Neon Environment - uses: ./.github/actions/setup - with: - use-rust: true - platform: linux-x64-gnu - - name: Install wasm32 target - shell: bash - run: rustup target add wasm32-unknown-unknown - - name: Install wasm-pack - uses: baptiste0928/cargo-install@c7dafcfd8a0035d58dfc65d0fbaf71a9ac347715 # v2 - with: - crate: wasm-pack - version: '0.13.1' - - name: Update Version - if: ${{ inputs.update-version }} - shell: bash - env: - # Bind the input to an env var so it never expands into the shell - # body — guards against template-injection via `${{ }}` in `run:`. - # Keep this guard in sync with the `binaries` job and release.yml's Tag - # Release step. - VERSION_INPUT: ${{ inputs.version }} - run: | - git config --global user.name $ACTIONS_USER - git config --global user.email $ACTIONS_EMAIL - # Allowlist gate: only npm `version` bump keywords or strict semver - # (with optional `v` prefix and pre-release suffix) get through. - case "$VERSION_INPUT" in - patch|minor|major|prepatch|preminor|premajor|prerelease) ;; - v[0-9]*.[0-9]*.[0-9]*|[0-9]*.[0-9]*.[0-9]*) ;; - v[0-9]*.[0-9]*.[0-9]*-*|[0-9]*.[0-9]*.[0-9]*-*) ;; - *) - echo "Invalid version input: '$VERSION_INPUT' (expected an npm bump keyword or semver)" >&2 - exit 1 - ;; - esac - npm version "$VERSION_INPUT" -m "v%s" - - name: Build wasm + inline shim - shell: bash - run: npm run build:wasm - # The generated .d.ts imports `../../lib/types.js`, and `lib/` is - # gitignored — nothing else in this job emits it (`npm ci` has no - # `prepare` hook, and `build:wasm` is wasm-pack plus inline-wasm.mjs), so - # without this the typecheck below fails on unresolvable imports. Kept a - # separate step rather than folded into `test:typecheck:wasm`: a script - # named "typecheck" should not emit `lib/` as a side effect. - - name: Build lib (declarations for the wasm .d.ts to import) - shell: bash - run: npx tsc - # The wasm .d.ts is generated by wasm-pack from the `typescript_type` / - # `typescript_custom_section` attributes in wasm.rs. These tests run - # against that output, so they belong here rather than in `npm test`, - # which must still pass in a clone with no dist/. They assert the negative - # cases with `@ts-expect-error`, so a declaration that stops catching a - # mistake fails the build rather than going quiet. - - name: Typecheck wasm declarations - shell: bash - run: npm run test:typecheck:wasm - - name: Upload wasm artifact - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 - with: - name: wasm-dist - path: dist/wasm - retention-days: 1 - - main: - name: Main - needs: [matrix, wasm] - runs-on: blacksmith-4vcpu-ubuntu-2404 - steps: - - name: Checkout Code - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - ref: ${{ inputs.ref }} - persist-credentials: false - - name: Setup Neon Environment - uses: ./.github/actions/setup - with: - use-rust: false - - name: Download wasm artifact - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 - with: - name: wasm-dist - path: dist/wasm - - name: Pack - id: pack - shell: bash - run: | - mkdir -p dist - echo "filename=$(npm pack --silent --pack-destination=./dist)" | tee -a $GITHUB_OUTPUT - - name: Extract release notes - if: ${{ inputs.github-release }} - shell: bash - env: - TAG: ${{ inputs.tag }} - run: | - # A release with no tag is a misconfiguration — fail fast with a - # clear message rather than emitting an empty version and a broken - # `blob//CHANGELOG.md` fallback link. - if [ -z "$TAG" ]; then - echo "Extract release notes: inputs.tag is empty but github-release is true" >&2 - exit 1 - fi - # Source the GitHub release body from the matching CHANGELOG section. - # Fall back to a pointer rather than failing the release if it's absent. - if ! node scripts/changelog-extract.mjs "${TAG#v}" > RELEASE_NOTES.md; then - echo "See [CHANGELOG.md](https://github.com/cipherstash/protectjs-ffi/blob/${TAG}/CHANGELOG.md)." > RELEASE_NOTES.md - fi - cat RELEASE_NOTES.md - - name: Release - if: ${{ inputs.github-release }} - uses: softprops/action-gh-release@9d7c94cfd0a1f3ed45544c887983e9fa900f0564 # v2.0.4 - with: - files: ./dist/${{ steps.pack.outputs.filename }} - tag_name: ${{ inputs.tag }} - body_path: RELEASE_NOTES.md diff --git a/packages/protect-ffi/.github/workflows/release.yml b/packages/protect-ffi/.github/workflows/release.yml deleted file mode 100644 index d739ea6fb..000000000 --- a/packages/protect-ffi/.github/workflows/release.yml +++ /dev/null @@ -1,167 +0,0 @@ -name: Release - -run-name: | - ${{ (inputs.dryrun && 'Dry run') - || format('Release: {0}', (inputs.version == 'custom' && inputs.custom) || inputs.version) }} - -on: - workflow_dispatch: - inputs: - dryrun: - description: 'Dry run (no npm publish)' - required: false - type: boolean - default: true - version: - description: 'Version component to update (or "custom" to provide exact version)' - required: true - type: choice - options: - - patch - - minor - - major - - prepatch - - preminor - - premajor - - prerelease - - custom - custom: - description: 'Custom version' - required: false - default: '' - -jobs: - setup: - name: Setup - runs-on: blacksmith-4vcpu-ubuntu-2404 - permissions: - contents: write - packages: write - actions: read - outputs: - dryrun: ${{ steps.dryrun.outputs.dryrun }} - publish: ${{ steps.publish.outputs.publish }} - ref: ${{ steps.tag.outputs.tag || github.ref_name || github.event.repository.default_branch }} - tag: ${{ steps.tag.outputs.tag || '' }} - steps: - - name: Validate Workflow Inputs - if: ${{ inputs.version == 'custom' && inputs.custom == '' }} - shell: bash - run: | - echo '::error::No custom version number provided' - exit 1 - - id: dryrun - name: Validate Dry Run Event - if: ${{ inputs.dryrun }} - shell: bash - run: echo dryrun=true | tee -a $GITHUB_OUTPUT - - id: publish - name: Validate Publish Event - if: ${{ !inputs.dryrun }} - shell: bash - # Publishing authenticates via npm OIDC trusted publishing, which is - # configured on npmjs.com for this repo + release.yml — no NPM_TOKEN - # secret is required. See the publish job below. - run: echo publish=true | tee -a $GITHUB_OUTPUT - - uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 - id: app-token - with: - app-id: ${{ vars.PUBLISHER_APP_ID }} - private-key: ${{ secrets.PUBLISHER_SECRET_KEY }} - # Least privilege: the token is only used to checkout and - # `git push --follow-tags` in the Tag Release step below. - permission-contents: write - - name: Checkout Code - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - with: - fetch-depth: 0 - token: ${{ steps.app-token.outputs.token }} - - name: Setup Neon Environment - uses: ./.github/actions/setup - with: - use-rust: false - - name: Tag Release - if: ${{ !inputs.dryrun }} - id: tag - shell: bash - env: - # Bind the resolved version to an env var so it never expands into the - # shell body — guards against template-injection via `${{ }}` in - # `run:` (a `custom` value could otherwise break out of the quotes). - # Keep this guard in sync with the Update Version steps in build.yml. - VERSION_INPUT: ${{ (inputs.version == 'custom' && inputs.custom) || inputs.version }} - run: | - git config --global user.name $ACTIONS_USER - git config --global user.email $ACTIONS_EMAIL - # Allowlist gate: only npm `version` bump keywords or strict semver - # (with optional `v` prefix and pre-release suffix) get through. - case "$VERSION_INPUT" in - patch|minor|major|prepatch|preminor|premajor|prerelease) ;; - v[0-9]*.[0-9]*.[0-9]*|[0-9]*.[0-9]*.[0-9]*) ;; - v[0-9]*.[0-9]*.[0-9]*-*|[0-9]*.[0-9]*.[0-9]*-*) ;; - *) - echo "Invalid version input: '$VERSION_INPUT' (expected an npm bump keyword or semver)" >&2 - exit 1 - ;; - esac - npm version -m 'v%s' "$VERSION_INPUT" - git push --follow-tags - echo tag=$(git describe --abbrev=0) | tee -a $GITHUB_OUTPUT - - build: - name: Build - needs: [setup] - permissions: - contents: write - uses: ./.github/workflows/build.yml - with: - ref: ${{ needs.setup.outputs.ref }} - tag: ${{ needs.setup.outputs.tag }} - update-version: ${{ !!needs.setup.outputs.dryrun }} - version: ${{ (inputs.version == 'custom' && inputs.custom) || inputs.version }} - github-release: ${{ !!needs.setup.outputs.publish }} - - publish: - name: Publish - if: ${{ needs.setup.outputs.publish }} - needs: [setup, build] - # GitHub-hosted (not Blacksmith): npm only accepts provenance attestations - # from github-hosted runners (self-hosted is rejected with E422). This job - # only publishes prebuilt tarballs, so it doesn't need Blacksmith — the - # build matrix above stays on Blacksmith. - runs-on: ubuntu-latest - permissions: - contents: read - id-token: write # Required for npm OIDC trusted publishing - steps: - - name: Checkout Code - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - with: - ref: ${{ needs.setup.outputs.ref }} - persist-credentials: false - # OIDC trusted publishing needs Node >= 22.14 and npm >= 11.5.1, and must - # NOT have a token .npmrc. Use setup-node WITHOUT registry-url (the Neon - # setup action sets registry-url, which writes a //registry/:_authToken - # line that shadows OIDC) — this job only publishes prebuilt tarballs, so - # it needs no Rust toolchain or npm install. - - name: Install Node.js - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 - with: - node-version: 22 - - name: Upgrade npm for OIDC trusted publishing - run: npm install -g npm@^11.5.1 - - name: Fetch - uses: robinraju/release-downloader@c39a3b234af58f0cf85888573d361fb6fa281534 # v1.10 - with: - tag: ${{ needs.setup.outputs.tag }} - fileName: "*.tgz" - out-file-path: ./dist - - name: Publish - shell: bash - # No NODE_AUTH_TOKEN — authenticate via OIDC trusted publishing. - # --provenance generates a signed provenance attestation; the repo is - # public and every package sets repository.url, which provenance needs. - run: | - for p in ./dist/*.tgz ; do - npm publish --access public --provenance "$p" - done diff --git a/packages/protect-ffi/.github/workflows/test.yml b/packages/protect-ffi/.github/workflows/test.yml deleted file mode 100644 index ee35c34b9..000000000 --- a/packages/protect-ffi/.github/workflows/test.yml +++ /dev/null @@ -1,123 +0,0 @@ -name: Test - -run-name: | - ${{ (github.event_name == 'pull_request' && format('Test (PR #{0}): {1}', github.event.number, github.event.pull_request.title)) - || format('Test: {0}', github.event.head_commit.message) }} - -on: - # Event: A maintainer has pushed commits or merged a PR to main or the - # eql_v3 staging branch. - push: - # Limiting push events to these branches prevents duplicate runs of this - # workflow when maintainers push to internal PRs. - branches: - - main - - eql_v3 - - # Event: A contributor has created or updated a PR. - pull_request: - types: [opened, synchronize, reopened, labeled] - branches: - - main - - eql_v3 - - # Event: manual invocation of the workflow - workflow_dispatch: - -jobs: - pr: - name: Pull Request Details - runs-on: blacksmith-4vcpu-ubuntu-2404 - if: ${{ github.event_name == 'pull_request' }} - outputs: - branch: ${{ steps.pr-ref.outputs.branch || github.event.repository.default_branch }} - steps: - - name: PR Branch - id: pr-ref - shell: bash - run: echo "branch=$(gh pr view $PR_NO --repo $REPO --json headRefName --jq '.headRefName')" | tee -a "$GITHUB_OUTPUT" - env: - REPO: ${{ github.repository }} - PR_NO: ${{ github.event.number }} - GH_TOKEN: ${{ github.token }} - - # Labeling a PR with a `ci:full-matrix` label does a full matrix build on - # every run of this workflow for that PR, in addition to the other tests. - full-matrix: - name: Build - if: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'ci:full-matrix') }} - needs: [pr] - permissions: - contents: write - uses: ./.github/workflows/build.yml - with: - ref: ${{ needs.pr.outputs.branch }} - update-version: true - github-release: false - - tests: - name: Tests - runs-on: blacksmith-4vcpu-ubuntu-2404 - env: - CS_WORKSPACE_CRN: ${{ secrets.CS_WORKSPACE_CRN }} - CS_CLIENT_ACCESS_KEY: ${{ secrets.CS_CLIENT_ACCESS_KEY }} - CS_CLIENT_ID: ${{ secrets.CS_CLIENT_ID }} - CS_CLIENT_KEY: ${{ secrets.CS_CLIENT_KEY }} - CS_ZEROKMS_HOST: https://ap-southeast-2.aws.zerokms.cipherstashmanaged.net - CS_CTS_HOST: https://ap-southeast-2.aws.cts.cipherstashmanaged.net - PGPORT: 5432 - PGDATABASE: cipherstash - PGUSER: cipherstash - PGPASSWORD: password - PGHOST: localhost - steps: - - name: Checkout Code - uses: actions/checkout@v3 - - name: Setup Neon Environment - id: neon - uses: ./.github/actions/setup - with: - platform: linux-x64-gnu - - - name: Build - shell: bash - run: npm run debug - - # Required by the `lint:rust:wasm` arm of the Rust Lint step below. - - name: Add wasm32 target - shell: bash - run: rustup target add wasm32-unknown-unknown - - # The `mise run test:integration:all` step below builds wasm artifacts - # because `tests/wasm-round-trip.test.ts` loads - # `dist/wasm/protect_ffi_inline.js` at module-graph time. Pinned to - # the same version used by the build workflow. - - name: Install wasm-pack - uses: baptiste0928/cargo-install@c7dafcfd8a0035d58dfc65d0fbaf71a9ac347715 # v2 - with: - crate: wasm-pack - version: '0.13.1' - - # Clippy for the host and for wasm32, plus `cargo fmt --check`. Subsumes - # the `cargo check --target wasm32-unknown-unknown` this step used to sit - # next to — clippy checks as it lints. - - name: Rust Lint - run: mise run lint:rust - - - name: Test (typecheck, lint, & Rust) - shell: bash - run: npm test - - - name: Integration test setup - run: mise setup - - - name: Test (integration) - run: mise run test:integration:all - - # Only runnable after the step above: it is what builds `dist/wasm/` - # (integration-tests/tasks.toml), and the generated .d.ts imports - # `../../lib/types.js`, emitted by the `npm run debug` / `npm test` steps - # earlier in this job. Release runs it too (build.yml's Wasm job) — this - # is the copy that runs on a PR. - - name: Typecheck wasm declarations - run: npm run test:typecheck:wasm diff --git a/packages/protect-ffi/README.md b/packages/protect-ffi/README.md index 579235040..021f4f8b4 100644 --- a/packages/protect-ffi/README.md +++ b/packages/protect-ffi/README.md @@ -392,9 +392,13 @@ selects changesets by `.endsWith('.md')`, so that extension is invisible to PR that repoints trusted publishing renames it back rather than reconstructing it from the git log. -The GitHub Actions workflows under `.github/` in this directory are the previous -repository's, kept as the reference for that port. GitHub does not read workflows -from a subdirectory, so nothing there runs. +The previous repository's GitHub Actions workflows were deposited under +`.github/` in this directory by the subtree import and kept as the reference for +that port. They are gone: the six-platform build matrix now lives in the +repository-root `.github/workflows/_build-ffi-artifacts.yml`, with +`ffi-preflight.yml` as its manually-dispatched dry run. Nothing under a package +directory is ever executed by GitHub, which reads workflows from the repository +root alone — `src/lintWiring.test.ts` fails if one comes back. ## Learn More diff --git a/packages/protect-ffi/crates/protect-ffi/Cargo.toml b/packages/protect-ffi/crates/protect-ffi/Cargo.toml index 2446aa1a5..dc66143b6 100644 --- a/packages/protect-ffi/crates/protect-ffi/Cargo.toml +++ b/packages/protect-ffi/crates/protect-ffi/Cargo.toml @@ -1,9 +1,16 @@ [package] name = "protect-ffi" version = "0.1.0" +# Never published to crates.io. This crate is a cdylib compiled into +# `index.node` and shipped inside the `@cipherstash/protect-ffi-` npm +# packages; it has no Rust-consumer identity. Without this key cargo treats it +# as publishable, so the release-plz adoption that arrives with the EQL import +# would try to publish it. Guarded by +# `scripts/__tests__/cargo-publish-opt-out.test.mjs`. +publish = false license = "MIT" description = "Native FFI bindings to the CipherStash Client SDK" -repository = "https://github.com/cipherstash/protectjs-ffi" +repository = "https://github.com/cipherstash/stack" edition = "2021" exclude = ["index.node"] diff --git a/packages/protect-ffi/crates/protect-ffi/src/lib.rs b/packages/protect-ffi/crates/protect-ffi/src/lib.rs index 3031b7da1..3cca59cc5 100644 --- a/packages/protect-ffi/crates/protect-ffi/src/lib.rs +++ b/packages/protect-ffi/crates/protect-ffi/src/lib.rs @@ -301,7 +301,7 @@ pub enum Error { Encryption(#[from] EncryptionError), #[error(transparent)] Eql(#[from] EqlError), - #[error("protect-ffi invariant violation: {0}. This is a bug in protect-ffi. Please file an issue at https://github.com/cipherstash/protectjs-ffi/issues.")] + #[error("protect-ffi invariant violation: {0}. This is a bug in protect-ffi. Please file an issue at https://github.com/cipherstash/stack/issues.")] #[diagnostic(code("INVARIANT_VIOLATION"))] InvariantViolation(String), /// An unknown `queryOp` wire value, raised from [`query_op::QueryOpName`]'s diff --git a/packages/protect-ffi/package.json b/packages/protect-ffi/package.json index a56fb8713..e24b2ced8 100644 --- a/packages/protect-ffi/package.json +++ b/packages/protect-ffi/package.json @@ -3,12 +3,12 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git" + "url": "git+https://github.com/cipherstash/stack.git" }, "bugs": { - "url": "https://github.com/cipherstash/protectjs-ffi/issues" + "url": "https://github.com/cipherstash/stack/issues" }, - "homepage": "https://github.com/cipherstash/protectjs-ffi#readme", + "homepage": "https://github.com/cipherstash/stack#readme", "description": "Native FFI bindings to the CipherStash Client SDK — powers @cipherstash/stack", "main": "./lib/index.cjs", "scripts": { diff --git a/packages/protect-ffi/platforms/darwin-arm64/package.json b/packages/protect-ffi/platforms/darwin-arm64/package.json index ad6ab6947..2dfa7aff0 100644 --- a/packages/protect-ffi/platforms/darwin-arm64/package.json +++ b/packages/protect-ffi/platforms/darwin-arm64/package.json @@ -4,8 +4,8 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git", - "directory": "platforms/darwin-arm64" + "url": "git+https://github.com/cipherstash/stack.git", + "directory": "packages/protect-ffi/platforms/darwin-arm64" }, "os": [ "darwin" diff --git a/packages/protect-ffi/platforms/darwin-x64/package.json b/packages/protect-ffi/platforms/darwin-x64/package.json index ae12060bb..9f288cdcd 100644 --- a/packages/protect-ffi/platforms/darwin-x64/package.json +++ b/packages/protect-ffi/platforms/darwin-x64/package.json @@ -4,8 +4,8 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git", - "directory": "platforms/darwin-x64" + "url": "git+https://github.com/cipherstash/stack.git", + "directory": "packages/protect-ffi/platforms/darwin-x64" }, "os": [ "darwin" diff --git a/packages/protect-ffi/platforms/linux-arm64-gnu/package.json b/packages/protect-ffi/platforms/linux-arm64-gnu/package.json index 53c945deb..3b4933598 100644 --- a/packages/protect-ffi/platforms/linux-arm64-gnu/package.json +++ b/packages/protect-ffi/platforms/linux-arm64-gnu/package.json @@ -4,8 +4,8 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git", - "directory": "platforms/linux-arm64-gnu" + "url": "git+https://github.com/cipherstash/stack.git", + "directory": "packages/protect-ffi/platforms/linux-arm64-gnu" }, "os": [ "linux" diff --git a/packages/protect-ffi/platforms/linux-x64-gnu/package.json b/packages/protect-ffi/platforms/linux-x64-gnu/package.json index db72b209a..44c695c09 100644 --- a/packages/protect-ffi/platforms/linux-x64-gnu/package.json +++ b/packages/protect-ffi/platforms/linux-x64-gnu/package.json @@ -4,8 +4,8 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git", - "directory": "platforms/linux-x64-gnu" + "url": "git+https://github.com/cipherstash/stack.git", + "directory": "packages/protect-ffi/platforms/linux-x64-gnu" }, "os": [ "linux" diff --git a/packages/protect-ffi/platforms/linux-x64-musl/package.json b/packages/protect-ffi/platforms/linux-x64-musl/package.json index 5eef54dc6..0c427e61d 100644 --- a/packages/protect-ffi/platforms/linux-x64-musl/package.json +++ b/packages/protect-ffi/platforms/linux-x64-musl/package.json @@ -4,8 +4,8 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git", - "directory": "platforms/linux-x64-musl" + "url": "git+https://github.com/cipherstash/stack.git", + "directory": "packages/protect-ffi/platforms/linux-x64-musl" }, "os": [ "linux" diff --git a/packages/protect-ffi/platforms/win32-x64-msvc/package.json b/packages/protect-ffi/platforms/win32-x64-msvc/package.json index ba3705238..37116ed34 100644 --- a/packages/protect-ffi/platforms/win32-x64-msvc/package.json +++ b/packages/protect-ffi/platforms/win32-x64-msvc/package.json @@ -4,8 +4,8 @@ "version": "0.31.0", "repository": { "type": "git", - "url": "git+https://github.com/cipherstash/protectjs-ffi.git", - "directory": "platforms/win32-x64-msvc" + "url": "git+https://github.com/cipherstash/stack.git", + "directory": "packages/protect-ffi/platforms/win32-x64-msvc" }, "os": [ "win32" diff --git a/packages/protect-ffi/src/lintWiring.test.ts b/packages/protect-ffi/src/lintWiring.test.ts index ee705496b..b8814f458 100644 --- a/packages/protect-ffi/src/lintWiring.test.ts +++ b/packages/protect-ffi/src/lintWiring.test.ts @@ -80,18 +80,12 @@ const rootWorkflows = rootWorkflowNames .map((name) => withoutComments(read(`${ROOT_WORKFLOW_DIR}/${name}`))) .join('\n') -// The upstream repo's CI, deposited here by the subtree merge and inert since: -// GitHub reads workflows from the repository root alone. It is kept on purpose -// — the phase-4 publishing cutover ports `build.yml`'s per-platform -// `CARGO_BUILD_TARGET` matrix — so this reads the directory rather than -// assuming it is gone, and the check below goes quiet on its own once the -// cutover deletes it. -const DEAD_WORKFLOW_DIR = '.github/workflows' -const deadWorkflowNames = existsSync(join(packageRoot, DEAD_WORKFLOW_DIR)) - ? readdirSync(join(packageRoot, DEAD_WORKFLOW_DIR)).filter((name) => - /\.ya?ml$/.test(name), - ) - : [] +// The upstream repo's CI was deposited here by the subtree merge and was inert +// from that moment: GitHub reads workflows from the repository root alone. It +// was kept while the release pipeline was ported from it, and deleted once +// `_build-ffi-artifacts.yml` and `ffi-preflight.yml` had consumed the last of +// it. +const DEAD_GITHUB_DIR = '.github' /** * Script names reachable from `root`, following `pnpm run` / `npm run` @@ -369,32 +363,25 @@ describe('lint and format wiring', () => { expect(problems).toEqual([]) }) - it('justifies nothing by pointing into the dead upstream workflow directory', () => { - // `packages/protect-ffi/.github/workflows/{build,test,release}.yml` are - // still present and still inert, kept until the phase-4 cutover has ported - // what it needs from them. Keeping them is the hazard: a comment or an - // exemption reason that names `test.yml` reads as a check that runs - // somewhere, and the only file with that name is one GitHub never - // executes. That is exactly how `test:typecheck:wasm` sat exempt "run by - // the wasm job" from the absorption onward with no job running it. + it('keeps no .github directory inside this package', () => { + // `packages/protect-ffi/.github/` is gone: the release pipeline ported the + // last of what it held (`build.yml`'s per-platform CARGO_BUILD_TARGET + // matrix, and `actions/setup`'s `neon list-platforms` step) into + // `.github/workflows/_build-ffi-artifacts.yml`. // - // mise.toml did it too — it told contributors CI installs the wasm32 - // target "in the `Add wasm32 target` step of test.yml", while the step that - // runs is in the root tests-rust.yml. + // It must not come back. A workflow file under a package reads as live CI + // and is not — that is how `test:typecheck:wasm` sat exempt "run by the + // wasm job" from the absorption onward with no job running it, and how + // mise.toml told contributors CI installs the wasm32 target "in the `Add + // wasm32 target` step of test.yml" while the step that runs is in the root + // tests-rust.yml. A re-deposit is not far-fetched: the next subtree import + // brings its own `.github/`, and it will arrive looking exactly as + // authoritative as this one did. // - // Only names unique to that directory are checked: `release.yml` exists at - // the root as well, so a citation of it is ambiguous and this under-reports - // rather than guessing — the same trade the workflow-dispatch check makes. - const deadOnly = deadWorkflowNames.filter( - (name) => !rootWorkflowNames.includes(name), - ) - const liveConfig = [ - miseToml, - JSON.stringify(scripts), - ...Object.values(ENTRY_POINT_EXEMPT), - ].join('\n') - - expect(deadOnly.filter((name) => liveConfig.includes(name))).toEqual([]) + // The whole directory, not the `.ya?ml` files in it. The deposit also + // carried `.env` and `actions/setup/action.yml`, which read as live CI just + // as readily as a workflow does. + expect(existsSync(join(packageRoot, DEAD_GITHUB_DIR))).toBe(false) }) it('runs the Rust format check from the cargo entry point', () => { diff --git a/scripts/__tests__/cargo-publish-opt-out.test.mjs b/scripts/__tests__/cargo-publish-opt-out.test.mjs new file mode 100644 index 000000000..fccf62389 --- /dev/null +++ b/scripts/__tests__/cargo-publish-opt-out.test.mjs @@ -0,0 +1,76 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' + +/** + * Every crate in the nested Cargo workspace must opt out of crates.io unless it + * is deliberately allowlisted below. + * + * A crate with no `publish` key is publishable BY DEFAULT, and `protect-ffi` + * carries none: it has never been on crates.io (verified against the registry + * API), it is a cdylib compiled into `index.node` and shipped inside the six + * `@cipherstash/protect-ffi-` npm packages, and it has no + * Rust-consumer identity at all. Nothing today would publish it — but this repo + * is about to grow a crates.io publisher (`eql-bindings`, via release-plz, when + * `cipherstash/encrypt-query-language` is absorbed), and release-plz publishes + * every workspace member that has not opted out. The convention EQL already + * uses, and which this workspace inherits with that import, is exactly one + * publishable crate with every other member explicitly `publish = false`, so + * release-plz needs no per-package configuration. + * + * The list below is the audit surface: adding a name to it means "a future + * release-plz run will publish this crate to crates.io". + */ + +const WORKSPACE = join(REPO_ROOT, 'packages/protect-ffi') + +/** Crates deliberately published to crates.io. Adding a name here is a decision. */ +const PUBLISHABLE = new Set([]) + +/** + * The workspace's members, expanded from its own `[workspace] members` list. + * + * Read from the manifest rather than by listing `crates/`, because the manifest + * is what cargo obeys: a member added at a path outside that directory + * (`members = ["crates/*", "xtask"]`) is one release-plz would publish and a + * directory scan would never see. The floor guard below then checks the + * expansion found something, so a members list this parser cannot read fails + * loudly instead of yielding an empty set that passes. + */ +function workspaceMembers() { + const manifest = readFileSync(join(WORKSPACE, 'Cargo.toml'), 'utf8') + const block = /^members\s*=\s*\[([^\]]*)\]/m.exec(manifest)?.[1] ?? '' + return [...block.matchAll(/"([^"]+)"/g)] + .flatMap(([, pattern]) => + pattern.endsWith('/*') + ? readdirSync(join(WORKSPACE, pattern.slice(0, -2)), { + withFileTypes: true, + }) + .filter((entry) => entry.isDirectory()) + .map((entry) => `${pattern.slice(0, -2)}/${entry.name}`) + : [pattern], + ) + .sort() +} + +describe('cargo publish opt-out', () => { + const members = workspaceMembers() + + // The guard on the scan: a discovery test that enumerates nothing passes + // while checking nothing. + it('finds the workspace members it means to check', () => { + expect(members).toContain('crates/protect-ffi') + }) + + for (const member of members) { + it(`${member} declares publish = false unless allowlisted`, () => { + if (PUBLISHABLE.has(member)) return + const manifest = readFileSync( + join(WORKSPACE, member, 'Cargo.toml'), + 'utf8', + ) + expect(manifest).toMatch(/^publish = false$/m) + }) + } +}) diff --git a/scripts/__tests__/ffi-release-matrix.test.mjs b/scripts/__tests__/ffi-release-matrix.test.mjs new file mode 100644 index 000000000..03c1d0d77 --- /dev/null +++ b/scripts/__tests__/ffi-release-matrix.test.mjs @@ -0,0 +1,204 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { + buildFor, + releaseMatrix, + runnerFor, + triplesFromPlatformManifests, +} from '../ffi-release-matrix.mjs' +import { REPO_ROOT } from './lib/repo-root.mjs' + +/** + * The release matrix decides what each of the six platform jobs compiles and + * where it looks for the result. Every field it carries is one the upstream + * matrix got right for upstream and wrong here — see the header of + * `scripts/ffi-release-matrix.mjs`. + * + * These checks are derived from `packages/protect-ffi/package.json` rather than + * restated: the script the matrix names must exist, and the log file it names + * must be the one that script actually redirects to. Rename a script or move a + * redirect and this fails, which is the only way the matrix stays true to the + * package. + */ + +const FFI = join(REPO_ROOT, 'packages/protect-ffi') +const pkg = JSON.parse(readFileSync(join(FFI, 'package.json'), 'utf8')) + +/** + * `neon list-platforms` output, verbatim, as of the six platforms this package + * publishes. + * + * This is the INDEPENDENT copy. The matrix reads the same mapping out of each + * `platforms//package.json` (`neon.rust`) so the release job needs + * nothing installed, and the check below asserts the two agree — without it, + * deriving from the manifests would just be trusting the manifests. + */ +const TRIPLES = { + 'darwin-x64': 'x86_64-apple-darwin', + 'darwin-arm64': 'aarch64-apple-darwin', + 'win32-x64-msvc': 'x86_64-pc-windows-msvc', + 'linux-x64-gnu': 'x86_64-unknown-linux-gnu', + 'linux-arm64-gnu': 'aarch64-unknown-linux-gnu', + 'linux-x64-musl': 'x86_64-unknown-linux-musl', +} + +/** + * The log file a script's chain redirects to, resolved through one `pnpm run` + * hop: `build:native` -> `cargo-build` -> `… > cargo.log`. + */ +function logFileOf(scriptName) { + const body = pkg.scripts[scriptName] + if (body === undefined) return null + const hop = body.match(/pnpm run ([\w:-]+)/) + const resolved = hop ? pkg.scripts[hop[1]] : body + const redirect = String(resolved).match(/>\s*(\S+\.log)/) + return redirect ? redirect[1] : null +} + +const MATRIX = releaseMatrix(TRIPLES) + +describe('the platform fixture matches the package', () => { + it('covers exactly the platforms this package publishes', () => { + expect(Object.keys(TRIPLES).sort()).toEqual([...pkg.neon.platforms].sort()) + }) + + it('agrees with the triples committed in the platform packages', () => { + // The mapping the release matrix actually uses. A platform package whose + // `neon.rust` drifts from what neon computes would cross-compile for the + // wrong target under a name that says otherwise — the tarball installs and + // then fails to dlopen, which is the failure this whole matrix exists to + // prevent. + expect(triplesFromPlatformManifests()).toEqual(TRIPLES) + }) +}) + +describe('release matrix', () => { + it('produces one entry per platform', () => { + expect(MATRIX).toHaveLength(6) + expect(MATRIX.map((entry) => entry.platform).sort()).toEqual( + Object.keys(TRIPLES).sort(), + ) + }) + + it('carries the Rust triple for every platform', () => { + // Without this, cargo builds for the runner's own architecture — and both + // Darwin platforms share an arm64 runner, so `darwin-x64` would ship an ARM + // binary that installs cleanly and fails to load. + for (const entry of MATRIX) { + expect(entry.target).toBe(TRIPLES[entry.platform]) + } + }) + + it('never selects `build`, which is tsc and emits no binary', () => { + // The single most likely port defect: upstream's `build` WAS its cargo + // script. Here it is `tsc` and nothing else. + expect(pkg.scripts.build).toBe('tsc') + for (const entry of MATRIX) { + expect(entry.script).not.toBe('build') + } + }) + + it('names a script that exists', () => { + for (const entry of MATRIX) { + expect( + pkg.scripts[entry.script], + `${entry.platform} selects "${entry.script}", which packages/protect-ffi/package.json does not define`, + ).toBeDefined() + } + }) + + it('names the log file that script actually redirects to', () => { + // `neon dist` reads this file to locate the compiled artifact, so a matrix + // that names the wrong one fails after the compile has already been paid + // for — or, worse, reads a stale log from the other build. + for (const entry of MATRIX) { + expect( + entry.log, + `${entry.platform} reads ${entry.log}, but ${entry.script} redirects to ${logFileOf(entry.script)}`, + ).toBe(logFileOf(entry.script)) + } + }) + + it('routes every platform to the runner and build it needs', () => { + // A LITERAL TABLE, not the ladder re-typed. Both of these were previously + // asserted by re-implementing `runnerFor`/`buildFor` in the expectation — + // which cannot fail for the error it looks like it guards: a wrong rule in + // the script passes as long as the copy in the test is edited the same + // wrong way. Spelled as data, every field of every platform is pinned, and + // changing a rule means changing rows here. + // + // zigbuild is how the gnu glibc floor gets pinned (`--target + // .2.28`); applying it to musl or Darwin would be a different build + // entirely. Both Darwin platforms share one runner and cross-compile, which + // is why `target` is explicit. + expect(MATRIX).toEqual([ + { + platform: 'darwin-x64', + target: 'x86_64-apple-darwin', + os: 'macos-latest', + script: 'build:native', + log: 'cargo.log', + }, + { + platform: 'darwin-arm64', + target: 'aarch64-apple-darwin', + os: 'macos-latest', + script: 'build:native', + log: 'cargo.log', + }, + { + platform: 'win32-x64-msvc', + target: 'x86_64-pc-windows-msvc', + os: 'windows-latest', + script: 'build:native', + log: 'cargo.log', + }, + { + platform: 'linux-x64-gnu', + target: 'x86_64-unknown-linux-gnu', + os: 'blacksmith-4vcpu-ubuntu-2404', + script: 'zigbuild', + log: 'zig.log', + }, + { + platform: 'linux-arm64-gnu', + target: 'aarch64-unknown-linux-gnu', + os: 'blacksmith-4vcpu-ubuntu-2404', + script: 'zigbuild', + log: 'zig.log', + }, + { + platform: 'linux-x64-musl', + target: 'x86_64-unknown-linux-musl', + os: 'blacksmith-4vcpu-ubuntu-2404', + script: 'build:native', + log: 'cargo.log', + }, + ]) + }) + + it('sends a platform the table does not name to a Linux runner', () => { + // The fallback arm, which the table above cannot reach. A seventh platform + // added tomorrow gets Blacksmith unless `runnerFor` is taught otherwise. + expect(runnerFor('linux-arm64-musl')).toBe('blacksmith-4vcpu-ubuntu-2404') + }) + + it('is empty for an empty mapping, rather than inventing platforms', () => { + // The CLI turns this into a hard failure: a matrix of zero builds nothing, + // uploads nothing, and reports success. + expect(releaseMatrix({})).toEqual([]) + }) + + it('exposes the script/log pair as one decision', () => { + // Both fields come from the same call, so they cannot be edited apart. + expect(buildFor('linux-arm64-gnu')).toEqual({ + script: 'zigbuild', + log: 'zig.log', + }) + expect(buildFor('darwin-x64')).toEqual({ + script: 'build:native', + log: 'cargo.log', + }) + }) +}) diff --git a/scripts/__tests__/ffi-repository-urls.test.mjs b/scripts/__tests__/ffi-repository-urls.test.mjs new file mode 100644 index 000000000..4aa70ef20 --- /dev/null +++ b/scripts/__tests__/ffi-repository-urls.test.mjs @@ -0,0 +1,110 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' + +/** + * The seven FFI manifests must name THIS repository. + * + * npm trusted publishing matches `repository.url` against the repository the + * publish runs from, exactly: *"your package's `repository.url` field in + * `package.json` must exactly match your GitHub repository"* + * (https://docs.npmjs.com/trusted-publishers/). A stale URL does not warn and + * does not degrade — the publish is rejected. All seven still named + * `cipherstash/protectjs-ffi` after the subtree import, which was correct while + * they were still published from there and wrong the moment publishing moves. + * + * `repository.directory` is the quieter half. It resolves from the ROOT of the + * repository named in `repository.url`, so `platforms/

` addressed a real + * directory in the old repo and addresses nothing here, where the packages live + * at `packages/protect-ffi/platforms/

`. The two fields fail differently: a + * stale `url` fails the publish outright, while a `directory` that does not + * resolve publishes fine and silently breaks the source link on the package + * page. Only one of those gets noticed, which is why both are asserted. + */ + +const FFI = join(REPO_ROOT, 'packages/protect-ffi') + +/** The repository these packages publish from as of the cutover. */ +const EXPECTED = 'https://github.com/cipherstash/stack' + +// Directories only, matching how `scripts/ffi-release-matrix.mjs` enumerates +// the same folder. A stray file beside them (a .DS_Store, an editor backup) +// would otherwise be read as a platform and crash this suite on a missing +// package.json — a failure about the wrong thing entirely. +const PLATFORMS = readdirSync(join(FFI, 'platforms'), { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + +const manifests = [ + join(FFI, 'package.json'), + ...PLATFORMS.map((platform) => + join(FFI, 'platforms', platform, 'package.json'), + ), +] + +describe('FFI manifests name this repository', () => { + it('checks the wrapper and all six platform packages', () => { + expect(manifests).toHaveLength(7) + }) + + for (const path of manifests) { + const pkg = JSON.parse(readFileSync(path, 'utf8')) + it(`${pkg.name} points repository.url at cipherstash/stack`, () => { + expect(pkg.repository.url).toBe(`git+${EXPECTED}.git`) + }) + } + + it('the wrapper also updates bugs and homepage', () => { + // Not required by npm, but a published package that links its users at an + // archived repository is its own kind of wrong. + const pkg = JSON.parse(readFileSync(join(FFI, 'package.json'), 'utf8')) + expect(pkg.bugs.url).toBe(`${EXPECTED}/issues`) + expect(pkg.homepage).toBe(`${EXPECTED}#readme`) + }) + + for (const platform of PLATFORMS) { + it(`${platform} names its own path from the repo root`, () => { + // A host-only rewrite leaves this field alone and the suite would go + // green on a source link that 404s — these six assertions are what make + // skipping the `directory` fix visible. + const pkg = JSON.parse( + readFileSync(join(FFI, 'platforms', platform, 'package.json'), 'utf8'), + ) + expect(pkg.repository.directory).toBe( + `packages/protect-ffi/platforms/${platform}`, + ) + }) + } + + it('no manifest still references the old repository', () => { + for (const path of manifests) { + expect(readFileSync(path, 'utf8')).not.toMatch(/protectjs-ffi/) + } + }) + + it('the crate manifest names this repository too', () => { + // `publish = false`, so this is documentation rather than a registry + // requirement — but the crate manifest ships inside every platform tarball + // built from this tree, and a wrong URL in a shipped manifest is wrong. + const cargo = readFileSync( + join(FFI, 'crates/protect-ffi/Cargo.toml'), + 'utf8', + ) + expect(cargo).toMatch( + /^repository = "https:\/\/github\.com\/cipherstash\/stack"$/m, + ) + expect(cargo).not.toMatch(/protectjs-ffi/) + }) + + it('the crate sends bug reports here, not to the archived repository', () => { + // `InvariantViolation`'s message is the one repository URL that reaches an + // end user at runtime — it is compiled into every platform binary and + // printed when the Rust core hits a state it believes impossible. The old + // repository is archived at the end of the cutover, and an archived + // repository accepts no issues, so this link stops working for exactly the + // people it exists to help. + const lib = readFileSync(join(FFI, 'crates/protect-ffi/src/lib.rs'), 'utf8') + expect(lib).not.toMatch(/protectjs-ffi/) + }) +}) diff --git a/scripts/__tests__/fixtures/lint-no-workflow-caching/mise-default-cache.yml b/scripts/__tests__/fixtures/lint-no-workflow-caching/mise-default-cache.yml new file mode 100644 index 000000000..aa477c9aa --- /dev/null +++ b/scripts/__tests__/fixtures/lint-no-workflow-caching/mise-default-cache.yml @@ -0,0 +1,17 @@ +name: Mise Default Cache +# `jdx/mise-action`'s `cache:` input defaults to TRUE, so omitting the key is +# not "no caching" — it is caching, spelled invisibly. The `with.cache` rule +# only fires on a truthy value, so before the explicit-`false` rule covered this +# action a workflow shaped exactly like this one passed the gate. +on: + push: + branches: [main] +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: jdx/mise-action@5228313ee0372e111a38da051671ca30fc5a96db # v3.6.3 + with: + install: true + working_directory: packages/protect-ffi diff --git a/scripts/__tests__/lint-no-workflow-caching.test.mjs b/scripts/__tests__/lint-no-workflow-caching.test.mjs index a9cf8f4d8..e0fb11f34 100644 --- a/scripts/__tests__/lint-no-workflow-caching.test.mjs +++ b/scripts/__tests__/lint-no-workflow-caching.test.mjs @@ -7,8 +7,16 @@ import { describe, expect, it } from 'vitest' import { REPO_ROOT } from './lib/repo-root.mjs' // Workflows the supply-chain gate is responsible for. +// +// A SECOND COPY of the script's own default TARGETS, and it used to be an +// unchecked one: nothing compared the two, so the `actions/cache` sweep at the +// bottom of this file silently stopped covering whatever this list omitted. The +// first test below now reads the script's real target list out of its success +// output and asserts the two agree, so adding a target in one place and not the +// other is a failure rather than a quiet loss of coverage. const TARGET_WORKFLOWS = [ '.github/workflows/release.yml', + '.github/workflows/_build-ffi-artifacts.yml', '.github/workflows/tests-supply-chain.yml', ] @@ -18,8 +26,13 @@ const SCRIPT = resolve( ) function run(...targets) { try { - execFileSync('node', [SCRIPT, ...targets], { encoding: 'utf8' }) - return { exitCode: 0, output: '' } + // stdout is kept on the success path too: on a clean run the script prints + // the targets it checked, and that listing is the only way to read its + // default TARGETS without importing a file that lints on import. + const stdout = execFileSync('node', [SCRIPT, ...targets], { + encoding: 'utf8', + }) + return { exitCode: 0, output: String(stdout) } } catch (err) { return { exitCode: err.status, @@ -35,8 +48,18 @@ describe('lint-no-workflow-caching', () => { `../fixtures/lint-no-workflow-caching/${name}`, ) - it('defaults to checking release.yml and tests-supply-chain.yml', () => { - expect(run().exitCode).toBe(0) + it('checks exactly the workflows this file knows about, by default', () => { + const r = run() + expect(r.exitCode).toBe(0) + // The success epilogue lists every target it scanned, one per line. + const scanned = r.output + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.startsWith('.github/workflows/')) + expect( + scanned.sort(), + "The script's default TARGETS and this file's TARGET_WORKFLOWS have diverged. Whichever one is missing an entry, the `actions/cache` sweep below has stopped covering it — and it stopped silently.", + ).toEqual([...TARGET_WORKFLOWS].sort()) }) it('passes on a workflow with no caching', () => { @@ -78,18 +101,28 @@ describe('lint-no-workflow-caching', () => { expect(r.output).toMatch(/package-manager-cache/) }) - it('keeps release.yml free of GitHub Actions caching', () => { - expect( - run(resolve(REPO_ROOT, '.github/workflows/release.yml')).exitCode, - ).toBe(0) + it('fails on a mise-action step that omits `cache:`', () => { + // The one action on the allowlist whose caching is ON by default, so an + // omitted key is not an intent problem — it is a cache restore. The + // `with.cache` rule fires only on a TRUTHY value, so this shape passed the + // gate until the explicit-`false` rule covered the action: verified against + // this fixture, exit 0 and `OK`, while the step it describes restores the + // GitHub Actions cache in a workflow whose artifacts are published. + const r = run(fx('mise-default-cache.yml')) + expect(r.exitCode).toBe(1) + expect(r.output).toMatch(/jdx\/mise-action/) + expect(r.output).toMatch(/cache: false/) }) - it('keeps tests-supply-chain.yml free of GitHub Actions caching', () => { - expect( - run(resolve(REPO_ROOT, '.github/workflows/tests-supply-chain.yml')) - .exitCode, - ).toBe(0) - }) + // Generated from the target list rather than written out per file: a target + // added to TARGET_WORKFLOWS gets its own check for free, which is the whole + // reason the two lists are now asserted to agree. + for (const target of TARGET_WORKFLOWS) { + it(`keeps ${target.replace('.github/workflows/', '')} free of GitHub Actions caching`, () => { + const r = run(resolve(REPO_ROOT, target)) + expect(r.exitCode, r.output).toBe(0) + }) + } // 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` diff --git a/scripts/__tests__/lint-release-scope.test.mjs b/scripts/__tests__/lint-release-scope.test.mjs new file mode 100644 index 000000000..e34a5967c --- /dev/null +++ b/scripts/__tests__/lint-release-scope.test.mjs @@ -0,0 +1,69 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, WORKFLOW_DIR, workflowFiles } from './lib/workflows.mjs' + +/** + * `lint-release.yml` states its scope TWICE — once as the `pull_request` paths + * filter that decides when the job runs, once as the argument list actionlint + * is actually pointed at. Nothing bound the two. + * + * Both directions are silent. Add a release workflow, name it only in `paths:`, + * and the gate runs without ever linting it; name it only in the argument list + * and it is linted except on the pull requests that change it — which is every + * pull request that could break it. Neither shows up as a failure; both show up + * as green. + * + * `lint-no-workflow-caching.test.mjs` carries the same guard for the same + * reason, and says so: its `TARGET_WORKFLOWS` "used to be an unchecked second + * copy". + */ + +const LINT_RELEASE = `${WORKFLOW_DIR}/lint-release.yml` + +const workflow = readWorkflow(LINT_RELEASE) +// `on:` parses as the boolean `true` under YAML 1.1 — the Norway problem. +const triggers = workflow.on ?? workflow[true] + +const filtered = triggers.pull_request.paths.filter((path) => + path.startsWith(`${WORKFLOW_DIR}/`), +) + +const source = readFileSync(join(REPO_ROOT, LINT_RELEASE), 'utf8') +const linted = [ + ...source.matchAll(/^\s+(\.github\/workflows\/[\w.-]+\.ya?ml)\s*\\?$/gm), +].map((match) => match[1]) + +describe('lint-release.yml lints exactly what it triggers on', () => { + it('points actionlint at every workflow in its own paths filter', () => { + // The direction that loses coverage without looking like it: a workflow + // added to `paths:` alone runs the gate and is never linted by it. + expect([...linted].sort()).toEqual([...filtered].sort()) + }) + + it('found both lists, rather than passing on two empty ones', () => { + // A discovery test that discovers nothing passes, having checked nothing — + // the failure mode `lib/workflows.mjs` was extracted to stop. The floor is + // the four release workflows this gate was introduced for. + expect(filtered.length).toBeGreaterThanOrEqual(4) + expect(linted.length).toBeGreaterThanOrEqual(4) + }) + + it('names only workflows that exist', () => { + // A renamed or deleted workflow leaves actionlint pointed at a path that no + // longer resolves, which fails the gate mid-review rather than here. + const present = workflowFiles() + for (const relPath of new Set([...linted, ...filtered])) { + expect(present, `${relPath} is named by lint-release.yml`).toContain( + relPath, + ) + } + }) + + it('lints itself', () => { + // The gate has to be inside its own scope: a shell or syntax error + // introduced HERE is otherwise checked by nothing. + expect(linted).toContain(LINT_RELEASE) + }) +}) diff --git a/scripts/__tests__/release-gate.test.mjs b/scripts/__tests__/release-gate.test.mjs new file mode 100644 index 000000000..992ea11cc --- /dev/null +++ b/scripts/__tests__/release-gate.test.mjs @@ -0,0 +1,176 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import yaml from 'js-yaml' +import { describe, expect, it } from 'vitest' +import { + classify, + unpublished, + workspacePackagePatterns, +} from '../release-gate.mjs' +import { REPO_ROOT } from './lib/repo-root.mjs' + +/** + * The gate decides what a push to `main` still has to publish, and it is + * LOAD-BEARING rather than a cost control — see the header of + * `scripts/release-gate.mjs`. A false negative skips the native matrix, and + * `changeset publish` then packs the six platform workspaces without their + * `index.node` and publishes them. + */ + +const FFI = '@cipherstash/protect-ffi' +const PLATFORM = '@cipherstash/protect-ffi-darwin-arm64' + +describe('unpublished', () => { + it('reports a package whose committed version is not on the registry', () => { + expect( + unpublished([{ name: FFI, version: '0.32.0' }], () => ['0.31.0']), + ).toEqual([FFI]) + }) + + it('reports nothing when the committed version is already published', () => { + // The basis of the whole ordered-publisher design: a release is a no-op for + // anything already on the registry, which is why publishing the FFI + // tarballs first makes `changeset publish` skip them. + expect( + unpublished([{ name: FFI, version: '0.31.0' }], () => ['0.31.0']), + ).toEqual([]) + }) + + it('treats a registry 404 as unpublished', () => { + // A name that has never been published; `null` is the 404. + expect( + unpublished([{ name: 'new-pkg', version: '1.0.0' }], () => null), + ).toEqual(['new-pkg']) + }) + + it('skips private packages', () => { + expect( + unpublished( + [{ name: 'bench', version: '1.0.0', private: true }], + () => null, + ), + ).toEqual([]) + }) + + it('propagates a lookup error instead of reporting "nothing to publish"', () => { + // THE load-bearing case. A network, auth or rate-limit failure must fail + // the gate — reading it as "already published" skips the artifact build and + // lets changesets publish binary-less platform packages. + const boom = () => { + throw new Error('npm view failed: ETIMEDOUT') + } + expect(() => unpublished([{ name: FFI, version: '0.32.0' }], boom)).toThrow( + /ETIMEDOUT/, + ) + }) + + it('looks each package up once, and only the publishable ones', () => { + // `npm view` is a network round trip per package. The private skip has to + // happen BEFORE the lookup, not after it: a private package has no registry + // entry, so looking one up costs a request to be told 404 and then ignored. + const asked = [] + const lookup = (name) => { + asked.push(name) + return ['1.0.0'] + } + unpublished( + [ + { name: 'a', version: '1.0.0' }, + { name: 'secret', version: '1.0.0', private: true }, + { name: 'b', version: '2.0.0' }, + ], + lookup, + ) + expect(asked).toEqual(['a', 'b']) + }) +}) + +describe('workspacePackagePatterns', () => { + const SOURCE = readFileSync(join(REPO_ROOT, 'pnpm-workspace.yaml'), 'utf8') + + it('reads the same patterns a YAML parser does', () => { + // THE ORACLE. The gate parses `packages:` with node builtins so its job + // needs no install — which only holds while the hand parse and real YAML + // agree about THIS file. js-yaml is a devDependency and available here, so + // the divergence fails on the pull request instead of narrowing the gate + // during a release. + expect(workspacePackagePatterns(SOURCE)).toEqual(yaml.load(SOURCE).packages) + }) + + it('keeps the nested platform packages, which `packages/*` does not cover', () => { + // The six platform packages sit a level deeper than the glob above them. + // Losing this entry is the concrete shape of a narrowed gate: six + // unpublished packages reported as nothing to publish. + expect(workspacePackagePatterns(SOURCE)).toContain( + 'packages/protect-ffi/platforms/*', + ) + }) + + it('stops at the next top-level key', () => { + expect( + workspacePackagePatterns( + 'packages:\n - a/*\n - b\n\ncatalogs:\n repo:\n tsup: 1.0.0\n', + ), + ).toEqual(['a/*', 'b']) + }) + + it('ignores comments and quotes, inline and on their own line', () => { + expect( + workspacePackagePatterns( + 'packages:\n # why\n - \'a/*\' # trailing\n - "b"\n', + ), + ).toEqual(['a/*', 'b']) + }) + + it('throws rather than returning a short list it could not parse', () => { + // Every failure mode here fails loudly — see the script header. A pattern + // silently dropped is a package never looked up. + // Flow style is valid YAML this parse does not read, so the block header + // never matches and it throws — the direction that fails a release rather + // than narrowing one. The oracle test above is what catches the day + // pnpm-workspace.yaml is rewritten this way. + expect(() => workspacePackagePatterns('packages: [a, b]\n')).toThrow( + /no `packages:` key/, + ) + expect(() => + workspacePackagePatterns('packages:\n - a/*\n not-a-list-item\n'), + ).toThrow(/unparsable/) + expect(() => workspacePackagePatterns('catalogs:\n repo: {}\n')).toThrow( + /no `packages:` key/, + ) + expect(() => workspacePackagePatterns('packages:\ncatalogs:\n')).toThrow( + /no `packages:` patterns/, + ) + }) +}) + +describe('classify', () => { + it('flags ffi when the wrapper is unpublished', () => { + expect(classify([FFI])).toEqual({ ffi: true, js: false }) + }) + + it('flags ffi when only a platform package is unpublished', () => { + // The fixed group moves all seven together, but a partially-failed publish + // can leave one behind — that still needs the matrix. + expect(classify([PLATFORM])).toEqual({ ffi: true, js: false }) + }) + + it('flags js for an ordinary Stack release', () => { + expect(classify(['@cipherstash/stack', 'stash'])).toEqual({ + ffi: false, + js: true, + }) + }) + + it('flags both when a release spans them', () => { + expect(classify([FFI, '@cipherstash/stack'])).toEqual({ + ffi: true, + js: true, + }) + }) + + it('flags neither when nothing is unpublished', () => { + // The common case: any push to main that is not a merged Version PR. + expect(classify([])).toEqual({ ffi: false, js: false }) + }) +}) diff --git a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs index 72d807d24..9cb118142 100644 --- a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs +++ b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs @@ -55,7 +55,14 @@ const REPOSITORY = 'cipherstash/stack' * must not fail this — it must subject that workflow to the check below. */ const EXPECTED_DISPATCHABLE = [ + // The manual dry run for an FFI release. Dispatch is not a convenience here: + // it is the ONLY way to run it, and it exists to be pointed at a Version + // Packages branch before the irreversible publish. + '.github/workflows/ffi-preflight.yml', '.github/workflows/integration-protect-ffi.yml', + // Path-filtered to the release machinery, so dispatch is how it gets run + // against a branch that changed something the filter does not name. + '.github/workflows/lint-release.yml', '.github/workflows/osv-scanner.yml', '.github/workflows/tests-rust.yml', ] diff --git a/scripts/__tests__/workflow-inline-node-quoting.test.mjs b/scripts/__tests__/workflow-inline-node-quoting.test.mjs new file mode 100644 index 000000000..eb7742503 --- /dev/null +++ b/scripts/__tests__/workflow-inline-node-quoting.test.mjs @@ -0,0 +1,192 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, WORKFLOW_DIR } from './lib/workflows.mjs' + +/** + * A backtick or a `${` inside a single-quoted `node -e` argument fails the + * release gate — on whichever day the runner image happens to upgrade + * shellcheck. + * + * WHAT HAPPENED. `_build-ffi-artifacts.yml` carries a `node -e '…'` whose JS + * says, in a `//` comment, "Against the manifests own `neon.platforms`". Shell + * does not know that is a comment: the whole argument is one single-quoted + * string, and shellcheck reads a backtick in it as command substitution that + * will not expand — SC2016. Fifteen lines further down the SAME argument spells + * the rule out ("no backticks in this argument — shellcheck reads them as + * command substitution and reports SC2016") and then a later edit broke it + * anyway, because nothing was checking. + * + * WHY CI WAS GREEN. actionlint is pinned to v1.7.7 but shellcheck is NOT + * pinned — it comes from whatever the `ubuntu-latest` image ships. The image's + * shellcheck reported SC2016 only for `$`; 0.11.0 extended it to backticks. So + * this sat as a latent failure keyed to an image bump, and would have surfaced + * as a red `lint-release` on an unrelated pull request, which is precisely the + * "fails at the worst possible moment" that workflow exists to prevent. + * + * WHY A REPO-SIDE TEST AND NOT JUST THE GATE. The gate is the same check with + * an unpinned dependency; it cannot tell "clean" from "this shellcheck does not + * look for that yet". This runs in `test:scripts` on every pull request, with + * no binary to download and no version to drift. + * + * WHY SCOPED TO INLINE `node` ARGUMENTS rather than all single-quoted text. + * Shell `#` comments in this repo are full of backticks — twenty of them across + * these four workflows — and shellcheck is right to ignore every one, because a + * comment is not a string. The dangerous construct is the inverse: a JS `//` + * comment that LOOKS like a comment while sitting inside shell single quotes. + * Scanning the `node -e '…'` argument is exact, and needs no shell lexer whose + * disagreement with the real one would be the next silent hole. + * + * WHY IT STOPS SHORT OF APOSTROPHES, the third thing that must not appear in + * one of these arguments — an apostrophe does not merely trip a lint, it ENDS + * the string, and the JS after it becomes shell. That failure needs no guard + * here because it is already loud on every shellcheck: SC1011 and SC1036 are a + * warning and an ERROR, not the info-level SC2016, and `lint-release` is + * path-filtered to exactly these workflows, so any edit that introduces one + * fails the gate on the pull request that made it. The backtick was worth + * pinning precisely because it was the quiet one. + */ + +const LINT_RELEASE = `${WORKFLOW_DIR}/lint-release.yml` + +/** + * The workflows actionlint is pointed at, read out of the gate itself rather + * than copied here. `lint-release-scope.test.mjs` already binds that list to + * the workflow's own `paths:` filter, so deriving from it means this check + * covers a fifth release workflow the day one is added — and covers nothing + * silently if the list is ever emptied, which the floor below catches. + */ +const lintedWorkflows = [ + ...readFileSync(join(REPO_ROOT, LINT_RELEASE), 'utf8').matchAll( + /^\s+(\.github\/workflows\/[\w.-]+\.ya?ml)\s*\\?$/gm, + ), +].map((match) => match[1]) + +/** Every `run:` body in a workflow, as `{ job, run }`. */ +const runBodies = (relPath) => { + const doc = readWorkflow(relPath) + return Object.entries(doc?.jobs ?? {}).flatMap(([job, spec]) => + (spec?.steps ?? []) + .filter((step) => typeof step?.run === 'string') + .map((step) => ({ job, name: step.name, run: step.run })), + ) +} + +/** + * The single-quoted arguments of inline `node` scripts in one `run:` body. + * + * A shell single-quoted string has NO escape sequence — there is no way to put + * an apostrophe inside one — so the next `'` after the opening one is + * unambiguously the terminator. That is why this can be a scan rather than a + * parser. + * + * An unterminated one throws instead of being skipped: `node -e '` with no + * closing quote is a broken workflow, and reading it as "no argument to check" + * would report the broken file as clean. + */ +export const inlineNodeScripts = (run) => { + const scripts = [] + const opener = /node\s+(?:-e|-p|--eval|--print)\s+'/g + let match = opener.exec(run) + while (match !== null) { + const start = match.index + match[0].length + const end = run.indexOf("'", start) + if (end === -1) { + throw new Error( + `unterminated single-quoted node argument at offset ${match.index}`, + ) + } + scripts.push(run.slice(start, end)) + opener.lastIndex = end + 1 + match = opener.exec(run) + } + return scripts +} + +/** + * What shellcheck objects to in a single-quoted string: the two forms it reads + * as an expression that will not expand. + * + * `$` alone is not enough — `/^\d+\.\d+\.\d+/` is a legal regex in one of these + * arguments today and shellcheck says nothing about a bare `$`. It is `${` and + * the backtick that trip SC2016. + */ +export const sc2016Offenders = (script) => + script + .split('\n') + .map((line, index) => ({ line, number: index + 1 })) + .filter(({ line }) => line.includes('`') || line.includes('${')) + .map(({ line, number }) => `${number}: ${line.trim()}`) + +describe('release workflows — inline node scripts survive shellcheck', () => { + it('finds the workflows and the inline scripts, rather than passing on none', () => { + // The guard on the scan: the check below is a filter over a derived list, + // and an empty list makes it pass having read nothing. Both floors are the + // state at writing — four linted workflows, and the inline `node` scripts + // that verify the packed tarballs. + expect(lintedWorkflows.length).toBeGreaterThanOrEqual(4) + const scripts = lintedWorkflows.flatMap((file) => + runBodies(file).flatMap(({ run }) => inlineNodeScripts(run)), + ) + expect(scripts.length).toBeGreaterThanOrEqual(2) + }) + + it('carries no backtick or ${ in a single-quoted node argument', () => { + const offenders = lintedWorkflows.flatMap((file) => + runBodies(file).flatMap(({ job, name, run }) => + inlineNodeScripts(run).flatMap((script) => + sc2016Offenders(script).map( + (hit) => `${file} / ${job} / ${name ?? 'unnamed step'} — ${hit}`, + ), + ), + ), + ) + expect( + offenders, + 'Shell reads the whole argument as one string, JS comment or not: a backtick or `${` in it is SC2016, and lint-release turns that into a failed release gate.', + ).toEqual([]) + }) +}) + +describe('release workflows — how an inline node argument is found', () => { + // Against synthetic input, because the sweep above only ever sees a tree + // someone has already cleaned. A scanner that matched nothing would pass it. + it('reads the argument up to its closing quote', () => { + expect(inlineNodeScripts("node -e 'console.log(1)' && echo done")).toEqual([ + 'console.log(1)', + ]) + }) + + it('finds every inline script in one body, not just the first', () => { + expect(inlineNodeScripts("node -e 'a'\nnode -p 'b'")).toEqual(['a', 'b']) + }) + + it('ignores an argument that is not single-quoted', () => { + // Double quotes are a different question — the shell expands them, so a + // backtick there is real command substitution and shellcheck is right to + // say nothing about SC2016. `.github/actions/build-ffi-binding` uses that + // form deliberately. + expect(inlineNodeScripts('node -e "require(\'./lib\')"')).toEqual([]) + }) + + it('throws on an unterminated argument rather than reporting it clean', () => { + expect(() => inlineNodeScripts("node -e 'oops")).toThrow(/unterminated/) + }) + + it('reports the offending forms and only those', () => { + expect(sc2016Offenders('// a `backtick` comment')).toEqual([ + '1: // a `backtick` comment', + ]) + // biome-ignore-start lint/suspicious/noTemplateCurlyInString: a literal + // `${` in a single-quoted string is the input under test — the rule fires + // on exactly the shape this assertion exists to pin. + expect(sc2016Offenders('const x = "${HOME}"')).toEqual([ + '1: const x = "${HOME}"', + ]) + // biome-ignore-end lint/suspicious/noTemplateCurlyInString: as above + // A bare `$` is not one of them: shellcheck does not flag it, and a regex + // anchor in one of these arguments would otherwise be a false failure. + expect(sc2016Offenders('if (!/^\\d+\\.\\d+$/.test(v)) {}')).toEqual([]) + }) +}) diff --git a/scripts/__tests__/workflow-paths-filter-parity.test.mjs b/scripts/__tests__/workflow-paths-filter-parity.test.mjs index 1da305263..02e35016a 100644 --- a/scripts/__tests__/workflow-paths-filter-parity.test.mjs +++ b/scripts/__tests__/workflow-paths-filter-parity.test.mjs @@ -106,6 +106,16 @@ const EXPECTED_ASYMMETRIES = new Map([ // unchecked — is unreachable when `push` is unfiltered. 'push is deliberately unfiltered (branches: [main]), so main runs a superset of what PRs run', ], + [ + '.github/workflows/lint-release.yml', + // There is no `push:` trigger at all — this gate exists to catch a broken + // release workflow BEFORE it merges, and a copy running after the merge + // would report the same finding at the moment it is no longer useful. With + // one event there is no second list to drift from, and the direction that + // would hurt (a narrower `pull_request` filter, so the PR that breaks the + // release machinery never runs the check) is what the single list IS. + 'pull_request is the only trigger; a post-merge copy would report a release-blocking finding too late to act on', + ], ]) describe('paths filters are written twice, identically', () => { diff --git a/scripts/__tests__/workflow-publish-permissions.test.mjs b/scripts/__tests__/workflow-publish-permissions.test.mjs new file mode 100644 index 000000000..cd4decc8b --- /dev/null +++ b/scripts/__tests__/workflow-publish-permissions.test.mjs @@ -0,0 +1,244 @@ +import { describe, expect, it } from 'vitest' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' + +/** + * In a workflow that can mint an npm publishing credential, a job that does not + * publish must not be able to. + * + * WHAT HAPPENED. `release.yml` declared `id-token: write`, `contents: write` + * and `pull-requests: write` at the WORKFLOW level, because the two jobs that + * publish need them. Workflow-level permissions are a default, not a ceiling + * applied only where asked, so the `gate` job — a checkout and one `node` + * invocation that answers "is anything unpublished?" — inherited all three. It + * could mint the OIDC token, push to the repository and write pull requests, + * having no use for any of it. + * + * WHY THAT IS THE CREDENTIAL AND NOT JUST A SCOPE. npm trusted publishing is + * bound to a repository AND A WORKFLOW FILENAME. Once `release.yml` is the + * registered publisher, an `id-token: write` token minted by ANY job in that + * file is one npm will accept for a publish — the registry cannot tell the gate + * apart from `publish-ffi`. So the blast radius of a compromised step in the + * cheap every-push job is the whole seven-package release, not a wasted minute. + * + * THE FIX IS THE DEFAULT, NOT THE OVERRIDE. Overriding the gate alone would + * close today's hole and leave tomorrow's open: the next job added to this file + * inherits the publish credential by omission, which is the failure mode that + * produced this one. The workflow-level grant is `contents: read` instead, and + * the two publishing jobs escalate explicitly. A new job now has to ASK for the + * credential in its own diff. + * + * WHAT THIS FILE PINS is that property rather than the current text: any + * workflow where some job holds `id-token: write` must grant nothing writable + * to the jobs that do not. It scans the workflow directory, so a second + * publishing workflow is held to the same bar the day it lands. + */ + +/** + * The jobs that may hold `id-token: write`, as ` / `. + * + * AN EQUALITY, NOT A FLOOR, and that is the point of the list. A floor would + * let an eighth job start minting publish tokens without a word in review, + * which is the exact shape this file exists to stop. Adding a publisher means + * editing this line — deliberately, in the same diff. + */ +const OIDC_JOBS = [ + // Uploads the seven prebuilt FFI tarballs. Publishes, so it needs OIDC. + '.github/workflows/release.yml / publish-ffi', + // `changeset publish` for the JS packages, plus the Version Packages PR. + '.github/workflows/release.yml / release', +] + +/** + * A `permissions:` value as a scope→level map. + * + * `undefined` means the key is absent — the caller decides what that inherits + * from. Everything else normalises, including the two string forms and the + * empty map, so a workflow written as `permissions: write-all` cannot slip past + * a check that only understands the mapping form. + * + * It THROWS on a shape it does not know rather than returning an empty map: a + * permissions block this file cannot read and a permissions block that grants + * nothing must not produce the same green. + */ +const scopes = (permissions) => { + if (permissions === undefined) return undefined + // `permissions:` with an empty body parses as null and means "grant nothing". + if (permissions === null) return {} + if (permissions === 'read-all') return { 'read-all': 'read' } + if (permissions === 'write-all') return { 'write-all': 'write' } + if (typeof permissions === 'object' && !Array.isArray(permissions)) { + return permissions + } + throw new Error( + `unrecognised permissions value: ${JSON.stringify(permissions)}`, + ) +} + +/** The scopes granted at `write` level, sorted, for a readable failure. */ +const writable = (granted) => + Object.entries(granted ?? {}) + .filter(([, level]) => level === 'write') + .map(([scope]) => scope) + .sort() + +/** + * What a job actually gets: its own block if it has one, otherwise the + * workflow-level default. This is the whole defect in one function — a job with + * no `permissions:` key is not a job with no permissions. + * + * A `uses:` job needs no special case. The caller job's grant is the CEILING + * for the reusable workflow it calls — a called workflow asking for more fails + * the run rather than being given it — so a caller held to `contents: read` + * cannot hand `id-token: write` to anything downstream, whatever that file + * declares. Checking the caller is checking the whole subtree. + */ +const effective = (job, workflowLevel) => + scopes(job?.permissions) ?? workflowLevel + +const workflows = workflowFiles().map((file) => { + const doc = readWorkflow(file) + return { + file, + workflowLevel: scopes(doc?.permissions), + jobs: Object.entries(doc?.jobs ?? {}), + } +}) + +/** Is this the ` / ` of a job sanctioned to publish? */ +const sanctioned = (file, name) => OIDC_JOBS.includes(`${file} / ${name}`) + +describe('supply chain — a publishing workflow grants OIDC per job', () => { + it('discovers the jobs that hold id-token: write', () => { + // The guard on the scan. Every check below is "for each workflow that mints + // OIDC…", and a scan that finds none of them passes having verified + // nothing — the failure `integration-workflow-paths.test.mjs`'s `required` + // floor was added for, and `workflow-dispatch-job-conditions.test.mjs` + // repeats. Pin the set instead. + const holders = workflows.flatMap(({ file, workflowLevel, jobs }) => + jobs + .filter( + ([, job]) => effective(job, workflowLevel)?.['id-token'] === 'write', + ) + .map(([name]) => `${file} / ${name}`), + ) + expect(holders.sort()).toEqual([...OIDC_JOBS].sort()) + }) + + it('never grants id-token: write at the workflow level', () => { + // Workflow level is a DEFAULT: it reaches every job that does not override + // it, including the one added next month by someone who never read this + // file. Escalating per job inverts that — omission becomes the safe answer. + const offenders = workflows + .filter(({ workflowLevel }) => workflowLevel?.['id-token'] === 'write') + .map(({ file }) => file) + expect( + offenders, + 'A workflow-level `id-token: write` hands every job in the file a credential npm accepts for a publish.', + ).toEqual([]) + }) + + it('leaves the non-publishing jobs of a publishing workflow read-only', () => { + // Not just OIDC. The gate inherited `contents: write` and + // `pull-requests: write` too, so a compromised step there could rewrite the + // tree the publish jobs then build from — a publish that never needed to + // forge a token because it changed what was about to be published. + const offenders = workflows + // A workflow is a publishing one if it holds the credential ANYWHERE — + // by sanction above, or by a job that granted itself `id-token: write` + // without being listed. The second disjunct matters: an unsanctioned + // publisher must not also switch this check off for the file it is in. + .filter(({ file, workflowLevel, jobs }) => + jobs.some( + ([name, job]) => + sanctioned(file, name) || + effective(job, workflowLevel)?.['id-token'] === 'write', + ), + ) + .flatMap(({ file, workflowLevel, jobs }) => + jobs + .filter(([name]) => !sanctioned(file, name)) + .flatMap(([name, job]) => { + const writes = writable(effective(job, workflowLevel)) + return writes.length + ? [`${file} / ${name}: ${writes.join(', ')}`] + : [] + }), + ) + expect( + offenders, + 'A job in a publishing workflow that does not publish must not be able to write to the repository.', + ).toEqual([]) + }) + + it('declares workflow-level permissions in a publishing workflow', () => { + // Absent is not read-only: with no `permissions:` key at all, jobs fall back + // to the REPOSITORY default, which is settings-controlled and outside this + // tree. A publishing workflow must not have its floor set somewhere a + // reviewer of this repo cannot see. + const publishing = new Set(OIDC_JOBS.map((entry) => entry.split(' / ')[0])) + const offenders = workflows + .filter( + ({ file, workflowLevel }) => + publishing.has(file) && workflowLevel === undefined, + ) + .map(({ file }) => file) + expect(offenders).toEqual([]) + }) +}) + +/** + * The reader, against synthetic input. + * + * The sweep above only ever sees a repo someone has already made clean, and a + * clean repo says nothing about how strict the checker is — a `scopes()` that + * returned `{}` for every shape it did not recognise would pass all four checks + * above while enforcing nothing. These pin the GitHub semantics the reader + * encodes, and inheritance is the one the defect actually lived in. + */ +describe('supply chain — how a job’s permissions are read', () => { + it('treats an absent job block as the workflow-level grant, not as none', () => { + // The defect, in one assertion. `gate` had no `permissions:` key and was + // read by everyone as "this job asks for nothing". + const workflowLevel = scopes({ 'id-token': 'write', contents: 'write' }) + expect(effective({ steps: [] }, workflowLevel)).toEqual(workflowLevel) + expect(writable(effective({ steps: [] }, workflowLevel))).toEqual([ + 'contents', + 'id-token', + ]) + }) + + it('lets a job block replace the default outright, rather than merge into it', () => { + // GitHub does not merge the two: a job block is the complete grant. So + // `permissions: {contents: read}` on the gate removes id-token and + // pull-requests as well, which is why the fix needed no other line. + const workflowLevel = scopes({ 'id-token': 'write', contents: 'write' }) + expect( + writable(effective({ permissions: { contents: 'read' } }, workflowLevel)), + ).toEqual([]) + }) + + it('reads the string forms, which the mapping-only check would wave through', () => { + expect(writable(scopes('write-all'))).toEqual(['write-all']) + expect(writable(scopes('read-all'))).toEqual([]) + // `permissions:` with an empty body is the documented "grant nothing". + expect(writable(scopes(null))).toEqual([]) + }) + + it('distinguishes “absent” from “grants nothing”', () => { + // Absent inherits — from the workflow, or from the repository default when + // the workflow is silent too. `{}` inherits nothing. Collapsing the two is + // how a workflow with no `permissions:` key at all reads as hardened. + expect(scopes(undefined)).toBeUndefined() + expect(scopes({})).toEqual({}) + expect( + effective({ permissions: {} }, scopes({ contents: 'write' })), + ).toEqual({}) + }) + + it('throws on a shape it cannot read', () => { + // Rather than returning an empty grant: "this file did not understand the + // block" and "the block grants nothing" must not produce the same green. + expect(() => scopes(['contents'])).toThrow(/unrecognised/) + expect(() => scopes('read')).toThrow(/unrecognised/) + }) +}) diff --git a/scripts/ffi-release-matrix.mjs b/scripts/ffi-release-matrix.mjs new file mode 100644 index 000000000..20495eb01 --- /dev/null +++ b/scripts/ffi-release-matrix.mjs @@ -0,0 +1,130 @@ +/** + * The release build matrix for the six `@cipherstash/protect-ffi-` + * packages: one entry per platform, carrying everything the job needs that + * cannot be derived inside it. + * + * WHY THIS IS A FILE AND NOT FIFTEEN LINES OF `node -e` IN THE WORKFLOW. Three + * of the four fields are wrong if the upstream matrix is copied across + * verbatim, and each is wrong silently: + * + * target — upstream derives the Rust triple from `neon list-platforms` and + * passes it as CARGO_BUILD_TARGET, and every cross-compilation detail hangs + * off it. Omit it and cargo builds for the runner: `macos-latest` is + * arm64 today, so BOTH Darwin jobs would emit ARM64 and `darwin-x64` would + * ship a binary that installs cleanly and fails to dlopen. + * + * script — upstream's non-gnu platforms select `build`, which upstream had as + * its cargo script. HERE `build` IS `tsc` AND NOTHING ELSE: phase 1 of the + * absorption moved cargo to `build:native` deliberately, so that the + * default test and build paths stay Rust-free. Ported as-is, four of the + * six platforms would run a TypeScript compile, produce no binary, and fail + * one step later on a missing log file. + * + * log — `cargo-build` redirects to `cargo.log`, `zig-build` to `zig.log`, and + * `neon dist` reads that file to locate the artifact it just built. A + * single hardcoded `< cargo.log` is wrong for whichever half it does not + * match. + * + * All three are the kind of mistake that produces a green matrix and a broken + * tarball, so they live here with `scripts/__tests__/ffi-release-matrix.test.mjs` + * checking them against the package's actual scripts rather than against a copy + * of this reasoning. + */ +import { appendFileSync, readdirSync, readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import process from 'node:process' +import { fileURLToPath } from 'node:url' + +const REPO_ROOT = resolve(import.meta.dirname, '..') +const PLATFORMS_DIR = join(REPO_ROOT, 'packages/protect-ffi/platforms') + +/** + * Where each platform builds. + * + * Linux goes to Blacksmith (the repo's standard Linux runner); the Darwin and + * Windows builds need GitHub-hosted images for their SDKs. Both Darwin + * platforms share `macos-latest` and cross-compile — which is exactly why + * `target` is explicit. + */ +export function runnerFor(platform) { + if (platform.startsWith('win32')) return 'windows-latest' + if (platform.startsWith('darwin')) return 'macos-latest' + return 'blacksmith-4vcpu-ubuntu-2404' +} + +/** + * gnu targets cross-compile through cargo-zigbuild so the glibc floor can be + * pinned (`--target .2.28`); everything else builds with plain cargo. + * The two scripts redirect to different log files — see the header. + */ +export function buildFor(platform) { + return platform.includes('gnu') + ? { script: 'zigbuild', log: 'zig.log' } + : { script: 'build:native', log: 'cargo.log' } +} + +/** + * Platform name -> Rust target triple, read from the platform packages + * themselves: each `platforms//package.json` carries `neon.rust`, written + * there by `neon add-platform` and shipped inside the published tarball. + * + * `neon list-platforms` prints the same mapping, and the matrix used to shell + * out to it — which meant a cold full-workspace install (~1GB, no caching + * allowed in a publishing workflow) on the critical path ahead of all six + * builds, to read six fields that are committed to this repository. + * `ffi-release-matrix.test.mjs` pins this against the fixture taken from + * `neon list-platforms`, so the two cannot drift silently. + */ +export function triplesFromPlatformManifests(dir = PLATFORMS_DIR) { + const entries = readdirSync(dir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort() + return Object.fromEntries( + entries.map((platform) => { + const pkg = JSON.parse( + readFileSync(join(dir, platform, 'package.json'), 'utf8'), + ) + return [pkg.neon.node, pkg.neon.rust] + }), + ) +} + +/** + * `triples` is a platform name -> Rust target triple mapping, as + * `triplesFromPlatformManifests()` (or `neon list-platforms`) produces. + */ +export function releaseMatrix(triples) { + return Object.entries(triples).map(([platform, target]) => ({ + platform, + target, + os: runnerFor(platform), + ...buildFor(platform), + })) +} + +function main() { + const matrix = releaseMatrix(triplesFromPlatformManifests()) + + if (matrix.length === 0) { + // An empty matrix builds nothing, uploads nothing, and reports success — + // and the next job would then pack the wrapper against six platform + // packages that were never built. + console.error('release matrix is empty: no platform packages found') + process.exit(1) + } + + // Readable form to the log, machine form to the step output — and nothing to + // stdout. A third copy went there too, which obliged the only caller to + // redirect it to /dev/null and read as though something were being + // suppressed. + console.error(JSON.stringify(matrix, null, 2)) + if (process.env.GITHUB_OUTPUT) { + appendFileSync( + process.env.GITHUB_OUTPUT, + `result=${JSON.stringify(matrix)}\n`, + ) + } +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) main() diff --git a/scripts/lint-no-workflow-caching.mjs b/scripts/lint-no-workflow-caching.mjs index d5907ca66..167af61c7 100644 --- a/scripts/lint-no-workflow-caching.mjs +++ b/scripts/lint-no-workflow-caching.mjs @@ -10,17 +10,25 @@ const TARGETS = process.argv.slice(2).length ? process.argv.slice(2) : [ '.github/workflows/release.yml', + // Everything it builds is packed and published with provenance by + // release.yml's publish-ffi job, so a poisoned restore here lands in a + // tarball on npm. Reached from release.yml anyway (the traversal follows + // a job-level `uses:`), and named here so `ffi-preflight.yml` — which + // calls it too — cannot become a way to build these artifacts under + // different rules. + '.github/workflows/_build-ffi-artifacts.yml', '.github/workflows/tests-supply-chain.yml', ] // `uses:` values that pull in the GitHub Actions cache directly. const CACHE_ACTION = /^actions\/cache(\/(restore|save))?@/ -// Steps that must disable their built-in caching *explicitly* — leaving the -// key off and relying on the default is not enough: the gate asserts intent. -const PNPM_ACTION_SETUP = /^pnpm\/action-setup(@|$)/ -const SETUP_NODE = /^actions\/setup-node(@|$)/ - +// An action that caches must be told not to *explicitly* — leaving the key off +// and relying on the default is not enough, and for two of the three it is not +// even "no caching": their input defaults to true. Which input does it, per +// action, is recorded on the allowlist entry below (`cacheInput`) rather than +// in a parallel list here, so the audit and the requirement cannot drift. +// // A `uses:` naming a directory in this checkout rather than a published action. // GitHub requires the `./` prefix for those, so anything without it is an // `owner/repo@ref` or `docker://` reference with nothing here to open. @@ -89,29 +97,64 @@ const PARENT_USES = /^\.\.\// // it was added to — the person adding it is the person told to audit it, in the // same PR. A denylist's staleness is an `OK`. // -// The cost is bounded and was measured before choosing it, not assumed. The two -// targeted workflows are deliberately minimal and reach exactly four actions -// between them (all four are below); traversal is target-scoped, so nothing -// outside what release.yml and tests-supply-chain.yml reach is constrained; and -// they reach no local composite and no reusable workflow today. Adding an -// action to the npm-publishing workflow costs one line here plus a sentence -// saying why — which is the review that workflow warrants regardless. +// The cost is bounded. The three targeted workflows are deliberately minimal +// and reach seven actions between them (all seven are below); traversal is +// target-scoped, so nothing outside what they reach is constrained. Adding an +// action to the npm-publishing path costs one entry here plus a sentence saying +// why — which is the review that path warrants regardless. +// +// `release.yml` does now reach a reusable workflow (`ffi-artifacts` calls +// `_build-ffi-artifacts.yml`), and `followReusableWorkflow` walks into it, so +// its jobs are audited as though they were written inline. // // LOCAL `uses:` IS EXEMPT, and deliberately so: this gate opens a `./` action // and reads every one of its steps, so it is audited by construction rather // than trusted. Listing it here would report the composite and bury the // `actions/cache` inside it — the finding that actually matters. -const AUDITED_ACTIONS = new Set([ +// ONE ENTRY PER ACTION, carrying BOTH halves of the audit: that a human read +// it, and what they found about its caching. `cacheInput` is the input that +// turns its built-in caching off, or `null` for an action that has none. +// +// The two used to be separate lists — a bare `Set` here, and hand-written +// regex/input pairs above — which is exactly the shape this file warns about a +// few paragraphs up: "This file already carries two hand-maintained members of +// that class … they are here only because somebody happened to notice. Nothing +// would have caught the third." The third (`jdx/mise-action`, which caches BY +// DEFAULT) was indeed caught by noticing, after it had shipped. Keyed together, +// adding an entry forces answering "which input disables its caching?" at the +// moment of the audit, rather than after someone reproduces the miss against a +// probe workflow. +// +// Keyed on `actionPath(uses)` — lowercased, `@ref` stripped — so the lookup and +// the explicit-`false` requirement cannot disagree about which action a step +// is. They could before: the allowlist matched case-insensitively while the +// rules tested the raw `uses:`, so `Actions/Setup-Node@v6.5.0` was audited and +// then skipped its `package-manager-cache: false` requirement. +const AUDITED_ACTIONS = new Map([ // First-party checkout. No cache of its own. - 'actions/checkout', - // Both cache on request only, and the explicit-`false` rules above assert - // that these two are told not to — they are on this list *and* separately - // constrained. - 'actions/setup-node', - 'pnpm/action-setup', + ['actions/checkout', { cacheInput: null }], + // `package-manager-cache` DEFAULTS TO TRUE (read from action.yml at the + // pinned v6.5.0). Whether the default becomes a restore depends on + // package.json metadata the action recognises, so the explicit `false` + // removes the dependence on that detail. + ['actions/setup-node', { cacheInput: 'package-manager-cache' }], + // `cache` defaults to 'false' at the pinned v6.0.10; the explicit setting + // records intent. + ['pnpm/action-setup', { cacheInput: 'cache' }], // release.yml's publish step. Runs `pnpm run release` and talks to npm over // OIDC; no cache, no cache input. - 'changesets/action', + ['changesets/action', { cacheInput: null }], + // Artifact transport between the build matrix and the publish job. Neither + // touches the GitHub Actions cache: they use the artifact API, a different + // per-run store with no cross-run key. + ['actions/upload-artifact', { cacheInput: null }], + ['actions/download-artifact', { cacheInput: null }], + // Supplies zig + cargo-zigbuild (the glibc-pinned gnu builds) and wasm-pack, + // all pinned in packages/protect-ffi/mise.toml. `cache` DEFAULTS TO TRUE, so + // an omitted key is a cache restore — and the generic `with.cache` rule below + // fires only on a truthy value, which is how a mise-action step carrying no + // `cache:` passed this gate. SHA-pinned at every call site. + ['jdx/mise-action', { cacheInput: 'cache' }], ]) // A secondary, deliberately over-broad read of the action's name. It is NOT the @@ -135,7 +178,7 @@ const AUDITED_ACTIONS = new Set([ // done the fail-closed work. const CACHE_SHAPED_ACTION = /cache/i -for (const audited of AUDITED_ACTIONS) { +for (const audited of AUDITED_ACTIONS.keys()) { if (CACHE_SHAPED_ACTION.test(audited)) { throw new Error( `AUDITED_ACTIONS must not contain a cache action, found "${audited}". ` + @@ -260,16 +303,14 @@ function checkStep(step, at, bodyAudited = false) { const uses = usesOf(step) if (uses === null) return - // Explicit-disable assertions for the package-manager setup actions. Both are - // allowlisted, so these are the additional constraint on them, not a - // substitute for one. - if (PNPM_ACTION_SETUP.test(uses)) { - const reason = explicitFalseReason(step, 'cache') - if (reason) offenders.push(`${at}: pnpm/action-setup ${reason}`) - } - if (SETUP_NODE.test(uses)) { - const reason = explicitFalseReason(step, 'package-manager-cache') - if (reason) offenders.push(`${at}: actions/setup-node ${reason}`) + // The explicit-disable requirement, read off the allowlist entry rather than + // from a parallel list of per-action rules. An audited action that caches is + // an additional constraint on it, not a substitute for being audited. + const path = actionPath(uses) + const cacheInput = AUDITED_ACTIONS.get(path)?.cacheInput + if (cacheInput) { + const reason = explicitFalseReason(step, cacheInput) + if (reason) offenders.push(`${at}: ${path} ${reason}`) } // One verdict per `uses:`, most specific first — a step reported twice reads @@ -289,11 +330,11 @@ function checkStep(step, at, bodyAudited = false) { 'local `uses:` that starts with `./`, and this gate will not follow one ' + 'out of the workspace root, so these steps cannot be audited', ) - } else if (CACHE_SHAPED_ACTION.test(actionPath(uses))) { + } else if (CACHE_SHAPED_ACTION.test(path)) { offenders.push( `${at}: uses \`${uses}\` — a third-party cache action (GitHub Actions cache)`, ) - } else if (!AUDITED_ACTIONS.has(actionPath(uses))) { + } else if (!AUDITED_ACTIONS.has(path)) { offenders.push( `${at}: uses \`${uses}\` — not in AUDITED_ACTIONS. This gate cannot read ` + 'a published action’s steps, so it cannot prove this one does not cache', diff --git a/scripts/release-gate.mjs b/scripts/release-gate.mjs new file mode 100644 index 000000000..d6a3c3abf --- /dev/null +++ b/scripts/release-gate.mjs @@ -0,0 +1,187 @@ +/** + * Decide what a push to `main` still has to publish. + * + * THE GATE IS REGISTRY STATE, NOT CHANGESET ANALYSIS. "No unconsumed + * `.changeset/*.md`" is also true for an ordinary docs commit and for the very + * next commit after a release, so gating on it would fire the sixteen-minute + * native matrix routinely. Asking npm what is actually missing is exact, and it + * is the same question `changeset publish` asks itself, per package. + * + * THIS IS LOAD-BEARING, not a cost control, and an earlier draft of the plan had + * that backwards. A false negative — reporting `ffi=false` while those versions + * are in fact unpublished — skips the artifact build and the FFI publish, and + * `changeset publish` then finds the same versions unpublished and packs the six + * platform workspaces FROM THE WORKSPACE, where `index.node` is a build output + * nobody produced. Six binary-less tarballs go to npm and every consumer's + * install resolves a wrapper whose binding cannot load. + * + * So every failure mode here fails loudly: a lookup that errors for any reason + * other than a 404 throws rather than being read as "already published". + */ +import { execFileSync } from 'node:child_process' +import { appendFileSync, globSync, readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import process from 'node:process' +import { fileURLToPath } from 'node:url' + +const REPO_ROOT = resolve(import.meta.dirname, '..') + +/** + * The wrapper and its six platform packages all start with this, so one prefix + * separates the two publisher branches. + */ +export const FFI_PREFIX = '@cipherstash/protect-ffi' + +/** + * Names whose committed version is absent from the registry. + * + * `lookup(name)` returns the published versions, or `null` when the package + * does not exist at all (a 404 — a first publish). Anything else it throws + * propagates: see the header. + */ +export function unpublished(manifests, lookup) { + const missing = [] + for (const { name, version, private: isPrivate } of manifests) { + // Before the lookup, not after: a private package has no registry entry, so + // asking costs a round trip to be told 404 and then ignored. + if (isPrivate) continue + const published = lookup(name) + if (published === null || !published.includes(version)) missing.push(name) + } + return missing +} + +/** Which publisher branches the unpublished set requires. */ +export function classify(names) { + return { + ffi: names.some((name) => name.startsWith(FFI_PREFIX)), + js: names.some((name) => !name.startsWith(FFI_PREFIX)), + } +} + +/** + * The `packages:` globs from `pnpm-workspace.yaml`, parsed without a YAML + * library. + * + * NODE BUILTINS ONLY, DELIBERATELY. This script needs nothing installed, and + * that is the whole cost of the job it runs in: the gate fires on EVERY push to + * main, and one `import yaml from 'js-yaml'` obliges that job to do a cold + * full-workspace install (no caching is permitted in a publishing workflow) + * before it can answer a question that is sitting in the tree. An earlier + * draft argued exactly that against `pnpm ls -r` and then paid the same cost + * for the parse. + * + * The block is a flat sequence of scalars, so the parse is a block scan rather + * than a YAML implementation — and `release-gate.test.mjs` pins the result + * against `js-yaml` reading the same file, so a `pnpm-workspace.yaml` written + * in a shape this does not handle fails a unit test on the pull request rather + * than silently narrowing the gate. + * + * Narrowing is the failure that matters: a pattern dropped here is a package + * never looked up, reported as "nothing to publish", and then published + * binary-less by `changeset publish`. So an empty result throws. + */ +export function workspacePackagePatterns(source) { + const lines = source.split('\n') + const start = lines.findIndex((line) => /^packages:\s*(#.*)?$/.test(line)) + if (start === -1) + throw new Error('pnpm-workspace.yaml has no `packages:` key') + + const patterns = [] + for (const line of lines.slice(start + 1)) { + if (/^\s*(#.*)?$/.test(line)) continue + // A non-indented line ends the block: the next top-level key. + if (!/^\s/.test(line)) break + const item = line.match(/^\s+-\s+(['"]?)(.+?)\1\s*(?:#.*)?$/) + if (!item) { + throw new Error( + `unparsable \`packages:\` entry in pnpm-workspace.yaml: ${line}`, + ) + } + patterns.push(item[2]) + } + + if (patterns.length === 0) { + throw new Error('pnpm-workspace.yaml lists no `packages:` patterns') + } + return patterns +} + +/** + * Every workspace manifest, read from disk. + * + * Resolved from `pnpm-workspace.yaml`'s own globs rather than by listing + * packages here, so a package added tomorrow is gated the day it lands — and + * read directly rather than through `pnpm ls -r`, which needs an installed + * `node_modules` to answer. + */ +export function workspaceManifests() { + const patterns = workspacePackagePatterns( + readFileSync(join(REPO_ROOT, 'pnpm-workspace.yaml'), 'utf8'), + ).map((pattern) => `${pattern}/package.json`) + + const manifests = globSync(patterns, { cwd: REPO_ROOT }) + .sort() + .map((relative) => + JSON.parse(readFileSync(join(REPO_ROOT, relative), 'utf8')), + ) + .map(({ name, version, private: isPrivate }) => ({ + name, + version, + private: Boolean(isPrivate), + })) + + // Same reasoning as the empty-pattern throw: no manifests is indistinguishable + // from "everything is published" downstream, and it is the reading that skips + // the artifact build. + if (manifests.length === 0) { + throw new Error( + `no workspace manifests matched ${patterns.join(', ')} under ${REPO_ROOT}`, + ) + } + return manifests +} + +/** + * Registry lookup. `null` on a 404 so a first publish is not mistaken for + * "published"; everything else throws. + */ +export function npmVersions(name) { + try { + return JSON.parse( + execFileSync('npm', ['view', name, 'versions', '--json'], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }), + ) + } catch (err) { + const text = `${err.stdout ?? ''}${err.stderr ?? ''}` + if (text.includes('E404')) return null + throw new Error(`npm view ${name} failed: ${text.trim() || err.message}`) + } +} + +function main() { + const manifests = workspaceManifests() + const missing = unpublished(manifests, npmVersions) + const { ffi, js } = classify(missing) + + console.log( + missing.length + ? `unpublished: ${missing.join(', ')}` + : 'nothing to publish — every committed version is on the registry', + ) + console.log(`ffi=${ffi} js=${js}`) + + // `ffi` and `js` only: the unpublished list was written here too and no job + // ever declared it as an output, so it was reachable by nothing. The + // `console.log` above is where that list is actually read, in the job log. + if (process.env.GITHUB_OUTPUT) { + appendFileSync(process.env.GITHUB_OUTPUT, `ffi=${ffi}\njs=${js}\n`) + } +} + +// Importable without running: the unit tests exercise the two pure functions +// above, and neither the workspace scan nor the registry lookups may fire on +// import. +if (process.argv[1] === fileURLToPath(import.meta.url)) main() diff --git a/skills/stash-supply-chain-security/SKILL.md b/skills/stash-supply-chain-security/SKILL.md index ed4756631..2ea92d1cf 100644 --- a/skills/stash-supply-chain-security/SKILL.md +++ b/skills/stash-supply-chain-security/SKILL.md @@ -49,8 +49,13 @@ Two layers: CI uses `pnpm install --frozen-lockfile`. If `pnpm-lock.yaml` and any `package.json` drift, the install aborts — no silent registry fetches that bypass the locked versions. -- **Where**: `.github/workflows/tests.yml` -- **Test asserts**: every `pnpm install` invocation in tests.yml carries `--frozen-lockfile` +- **Where**: every workflow under `.github/workflows/`, and every local composite action under `.github/actions/` +- **Test asserts**: no `pnpm install` step anywhere in that graph is missing `--frozen-lockfile` + +The check used to read `tests.yml` alone, and `release.yml` — the one workflow +that publishes to npm — ran a bare `pnpm install` the whole time. The single +install allowed to resolve outside the lockfile was the one whose output goes to +the registry. ### 5. Cooldown'd auto-updates — practice #6 @@ -131,18 +136,81 @@ npm view @ --json | grep -A3 attestations Constraints baked into that workflow — don't undo them: -- **`permissions: id-token: write`** is what mints the OIDC token. Without it every publish fails. +- **`permissions: id-token: write`** is what mints the OIDC token. Without it every publish fails — but it belongs on the **publishing jobs**, never at the workflow level. A trusted publisher is registered against a repository *and a workflow filename*, so once `release.yml` is the registered publisher, npm accepts a token minted by **any** job in that file: the registry cannot tell the cheap every-push gate apart from the publish job. Declared at the top, it reaches every job that does not override it, including the one added next month by someone who never read this page. `release.yml` therefore grants `contents: read` at the workflow level and escalates per job, so a new job has to *ask* for the credential in its own diff. Enforced by `scripts/__tests__/workflow-publish-permissions.test.mjs`, which also holds the list of jobs allowed to hold it. - **`runs-on: ubuntu-latest`, not a self-hosted/Blacksmith runner.** npm rejects provenance from non-GitHub-hosted runners with E422. - **Never set `NPM_TOKEN`.** `changesets/action` writes a token `.npmrc` when it sees one, which shadows OIDC and fails every publish with E404 (npm/cli#8976). - **npm ≥ 11.5.1 and Node ≥ 22.14.** Node 22 ships npm 10.x, so the workflow installs `npm@^11.5.1` explicitly before publishing. - **No Actions cache in this workflow** (no `cache:`, `package-manager-cache: false`, `pnpm/action-setup` with `cache: false`). A poisoned cache entry would execute in a credential-bearing job. Enforced by `scripts/lint-no-workflow-caching.mjs`, which also follows any local composite action or reusable workflow the job reaches — the rule is about the whole call tree, not the one file. -- **Every published `uses:` must be in that script's `AUDITED_ACTIONS` allowlist.** The gate cannot open a published action to check whether it caches, and the ones that do are not all named "cache" — a `setup-` action that caches by default has no `cache:` input and no telling name. So the list is what is *permitted*, and an action it has never met fails by default. Adding a step to `release.yml` or `tests-supply-chain.yml` means auditing the action and adding it there with the reason, in the same PR. +- **Every published `uses:` must be in that script's `AUDITED_ACTIONS` allowlist.** The gate cannot open a published action to check whether it caches, and the ones that do are not all named "cache" — a `setup-` action that caches by default has no `cache:` input and no telling name. So the list is what is *permitted*, and an action it has never met fails by default. Adding a step to `release.yml`, `_build-ffi-artifacts.yml` or `tests-supply-chain.yml` means auditing the action and adding it there with the reason, in the same PR. +- **Three actions must disable caching *explicitly*, and the input differs for each.** Allowlisting an action is not the same as it being safe by default: + + | Action | Input | Its default | Required | + |---|---|---|---| + | `pnpm/action-setup` | `cache` | `false` | `cache: false` | + | `actions/setup-node` | `package-manager-cache` | **`true`** | `package-manager-cache: false` | + | `jdx/mise-action` | `cache` | **`true`** | `cache: false` | + + ```yaml + - uses: jdx/mise-action@ # v3.6.3 + with: + install: true + working_directory: packages/protect-ffi + cache: false # defaults to TRUE — omitting this restores the Actions cache + ``` + + Omitting the key is not "no caching" for the bottom two, it is caching spelled invisibly. The gate's generic rule only fires on a *truthy* `cache:` value, so a missing key is invisible to it — which is exactly how a `mise-action` step with no `cache:` passed until each action got its own explicit-`false` assertion. + +### The native-binding publish path + +`@cipherstash/protect-ffi` and its six `@cipherstash/protect-ffi-` +packages ship compiled binaries, which `changeset publish` cannot produce: it +packs from the workspace, where `index.node` is a build output. So `release.yml` +publishes them itself, before changesets runs, and the same constraints apply to +that job — GitHub-hosted runner, `id-token: write`, no `NPM_TOKEN`, +npm ≥ 11.5.1, no Actions cache. + +- `scripts/release-gate.mjs` asks the registry which committed versions are + missing. It is not a cost optimisation: if it wrongly reports nothing to + publish, changesets publishes six platform packages with no binary in them. + Every failure mode in it throws rather than answering "nothing to publish". +- `_build-ffi-artifacts.yml` is a reusable workflow, and only builds. npm + validates a trusted publish against the **entry-point** workflow's filename, + and its docs call out `workflow_call` as a known issue: *"validation checks + the calling workflow's name instead of the workflow that actually contains the + publish command, which can cause configuration mismatches"*, with + `id-token: write` required in **both** parent and child. A publish inside a + reusable workflow is therefore validated against whichever workflow called it. + Keeping it in `release.yml` — the registered filename, as an entry-point job + rather than a call — is correct whichever way that resolves. The reusable + workflow is on the no-caching gate's target list for the same reason + `release.yml` is: everything it produces gets published. +- Platform packages publish **before** the wrapper. The wrapper's six + `optionalDependencies` are exact versions, so publishing it first exposes a + version whose binaries do not exist yet. +- `ffi-preflight.yml` is the dry run — `changeset publish` has no `--dry-run`. + Dispatch it against a Version Packages branch and it builds the real tarballs, + checks each binary's architecture and libc, installs the host pair and loads + it. It cannot publish, and "no `id-token`" is only half of why: that closes + the OIDC path, while a plain `NPM_TOKEN` would still authenticate one. Both + are absent — the workflow grants `contents: read`, passes no secrets (no + `secrets: inherit` on its call into the build workflow), sets no + `registry-url` (which is what writes an `_authToken` line into `.npmrc`), and + names no `NPM_TOKEN` or `NODE_AUTH_TOKEN`. Keep it that way; adding any one of + them turns a dry run into a publisher. Trusted publishing is configured **per package** on npmjs.com (package settings → Trusted publisher → GitHub Actions): owner/repo `cipherstash/stack`, workflow filename `release.yml` (filename only, with extension — not a path), environment blank. npm does not validate this on save, so a typo only surfaces as a failed -publish. +publish. For configurations created after 2026-05-20 npm also requires an +explicit **Allowed actions** selection — pick `npm publish`. + +**`repository.url` must exactly match the publishing repository** +(`https://github.com/cipherstash/stack`). npm checks it on a trusted publish and +rejects a mismatch; nothing warns beforehand. A package moved between +repositories needs its manifest updated in the same change as its publisher — +and for a package published from a subdirectory, `repository.directory` is +resolved from that repository's root, so it moves too. ### Publishing a package name for the first time