feat(protect-ffi): absorb protectjs-ffi into the monorepo (phases 1–2) - #858
Closed
tobyhede wants to merge 597 commits into
Closed
feat(protect-ffi): absorb protectjs-ffi into the monorepo (phases 1–2)#858tobyhede wants to merge 597 commits into
tobyhede wants to merge 597 commits into
Conversation
…client-follow-up Refactor/upgrade cipherstash client follow up
protect-ffi's hand-written TypeScript collapsed two distinct Rust types into one Encrypted: the storage payload (EqlCiphertext) and the query payload (EqlOutput/EqlQueryPayload). encryptQuery was mistyped, c-less query payloads type-checked as decrypt input, and isEncrypted's arg was typed Encrypted. Encrypted is now storage-only (c required); new EncryptedQuery covers query payloads; encryptQuery/encryptQueryBulk return Encrypted | EncryptedQuery; isEncrypted takes unknown. Types only, no runtime change.
EncryptedScalar (storage) and EncryptedScalarQuery (query) share the `k: 'ct'` discriminant. Mark `c` as `c?: never` on the query variant so `Encrypted | EncryptedQuery` discriminates cleanly via `'c' in payload` and no object can structurally satisfy both variants.
fix(types): split storage Encrypted from query payload types
Path-deps to cipherstash-suite for cipherstash-client/cts-common/stack-profile and a new stack-auth dep, bump vitaminc to 0.2.0-pre, and target-split tokio + neon so a future wasm32 target can be added cleanly. Fix the API drift from cipherstash-client alpha.2 → alpha.4: ColumnType/Plaintext::Utf8Str → Text, ::JsonB → ::Json, new EqlEncryptOpts.decryption_policy field, IndexType::Ope. Native build + 84 unit tests pass.
- Gate the `use neon::*` import, `Finalize for Client` impl, all 10 `#[neon::export]` fns, the `#[neon::main]`, the `RUNTIME` static, and `build_key_provider` (which depends on the native-only `stack-profile`) behind `#[cfg(not(target_arch = "wasm32"))]`. - Native build unchanged; `cargo check --target wasm32-unknown-unknown` now succeeds (zero errors, ~50 unused-warnings expected — the wasm-bindgen module that will consume the shared types lands next). Macro-coexistence checkpoint from the plan: neon's macros stay cleanly gated out on wasm32, so single-crate-with-cfg is workable. No need to fall back to sibling crates.
10 wasm-bindgen exports mirroring the Neon side:
- newClient(strategy, opts) - constructs a WasmClient. Accepts a
@cipherstash/auth-shaped JS strategy (anything with getToken()) and an
inline clientId + clientKey hex pair (no filesystem fallback on wasm).
- WasmClient.{encrypt, encryptBulk, encryptQuery, encryptQueryBulk,
decrypt, decryptBulk, decryptBulkFallible} - data plane.
- isEncrypted(raw) - sync ciphertext shape check.
JsAuthStrategy wraps a JS Function into stack_auth::AuthStrategy by
calling .call0() and awaiting the returned Promise via JsFuture, then
extracting the .token field as a SecretToken. Inline rather than going
through AuthStrategyFn because the JS callable type is hard to express
as a stored Rust closure.
Native + wasm32 both compile. ensure_keyset is intentionally skipped on
wasm (it's a setup-time helper that needs the management-only ZeroKMS
client; will be added if a clear consumer asks).
PR #92 deleted crates/protect-ffi/src/encrypt_config.rs and moved config handling onto cipherstash-client's canonical types. Port the wasm bindings the same way the Neon path in lib.rs was: import CanonicalEncryptionConfig, ColumnConfig and Identifier from cipherstash_client::schema and drop the crate::encrypt_config import. The into_config_map() call is unchanged.
- wasm: preserve the JS receiver when calling getToken() so class-based auth strategies see the right `this` instead of null. - wasm: rework decryptBulkFallible so a single malformed ciphertext is reported as a per-item DecryptResult::Error instead of aborting the whole batch — matches the *Fallible contract. - js_plaintext: switch the user-facing type names in coercion errors from `Utf8Str`/`JsonB` to `Text`/`Json` so they line up with the ColumnType migration.
Addresses three review comments on PR #87: - newClient now takes `clientId: Uuid` instead of `String` so a malformed value fails at deserialization with a clear error rather than later inside the hex decoder. - Switch from `ClientKey::from_hex_v1` + `with_client_key` to `SecretKey::from_hex` + `with_key_provider`. `SecretKey` is the zeroize-on-drop wrapper from cipherstash-client; `from_hex` also zeroizes the incoming hex string while decoding. The in-memory key material is now wiped when `new_client` returns. - Document why `unsafe impl Send/Sync for JsAuthStrategy` is required (cipherstash-client's `ScopedCipher` / `ZeroKMSWithClientKey` carry a blanket `C: Send + Sync + 'static` bound on their methods, inherited from the native build). Removing the unsafe needs an upstream relax of those bounds on wasm32, parallel to the `AuthStrategy` split that `stack-auth` already does.
…ization Addresses follow-up review comments on PR #87 — the previous pass moved the in-memory key material into `SecretKey` (zeroize-on-drop) once `new_client` got that far, but the raw hex still lived in a bare `String` field of `NewClientOpts` from the moment serde produced it until `SecretKey::from_hex` consumed it. A panic or early-return in that window (e.g. the strategy validation just above) would leave the key bytes in the heap, unzeroized. - Add a small `HexSecret(String)` newtype, `Zeroize + ZeroizeOnDrop` + `#[serde(transparent)]`. JS still passes a bare hex string; serde deposits it directly into a zeroize-on-drop wrapper. Any error path between deserialization and key consumption now zeroizes the hex buffer on stack unwind. - Decode the hex inline and call `SecretKey::new(uuid, ViturKeyMaterial)` instead of `SecretKey::from_hex(client_id.to_string(), hex)`. This drops the `Uuid -> String -> Uuid` round-trip (the `to_string()` allocation was itself a non-zeroized copy) and also avoids the upstream bug where `from_hex` doesn't zeroize the hex buffer on the `Uuid::parse_str` error path. - Add `zeroize = "1.8"` as a direct dep (already a transitive dep) for the derive macros.
- **Neon `decrypt_bulk_fallible`: per-item error semantics.** Previously a single invalid mp_base85 ciphertext aborted the whole batch via `collect::<Result<Vec<_>, Error>>()?`, diverging from the `*Fallible` contract. Port the wasm shape (decode each ciphertext independently, route decode failures to per-index `DecryptResult::Error`, only send valid records to `decrypt_fallible`) back to the Neon path so both runtimes have identical semantics. (PR #87 review item 1) - **Drop `column_config.clone()` in wasm bulk encrypt paths.** `do_encrypt_bulk` / `do_encrypt_query_bulk` were cloning `ColumnConfig` for every payload despite the Neon path borrowing for the same lifetime — `column_config` comes from `client.encrypt_config.get(&ident)` and lives for the duration of the `prepared_plaintexts` vec. `ColumnConfig` carries the index list, so the clone gets expensive on rich schemas. (item 2) - **Install `console_error_panic_hook` at module start.** Wires the unused `web-sys = { features = ["console"] }` to something useful so Rust panics surface as a JS `Error` in the browser / Node console instead of a bare `RuntimeError: unreachable executed`. (item 3) - **Revert `pub(crate)` on `into_store_ciphertext` to private.** `mod wasm` is a child of the crate root, so it already sees private items via `use crate::...` (same as `find_index_for_type`, `parse_query_op`, `to_query_plaintext`, `encrypted_record_from_mp_base85`). The visibility bump was unnecessary. (item 4) - **Document wasm surface omissions and auth caching policy.** Module doc-comment now states explicitly that (a) `ensureKeyset` and other admin ops are intentionally not exported (provisioning belongs in your server) and (b) `JsAuthStrategy::get_token` is invoked per ZeroKMS request with no Rust-side caching — the JS strategy owns the refresh / persistence policy via cookies / localStorage / etc. (items 7 + 8) Refs: cipherstash/protectjs-ffi#87 (review)
feat(wasm): add wasm-bindgen bindings (1/4 — core surface)
Wire wasm-pack into the npm scripts and the release pipeline: - npm script `build:wasm` runs `wasm-pack build crates/protect-ffi --target bundler --out-dir dist/wasm`, with a `postbuild:wasm` step that runs scripts/inline-wasm.mjs to base64-embed the .wasm into a sibling JS module (zero-config consumption on Supabase Edge / Cloudflare Workers, same pattern as @cipherstash/auth's `./wasm-inline`). - Conditional `exports` map gains `.` (node→Neon CJS, default→bundler wasm), `./wasm` (raw bundler output), and `./wasm-inline` (inline shim). `files` list extended so `npm pack` includes the wasm assets. - `.gitignore` excludes `dist/`. - Release pipeline (build.yml) gains a `wasm` job that builds the wasm, uploads it as a workflow artifact; the existing `main` job depends on the artifact so every published tarball ships the wasm alongside the Neon binaries. - PR pipeline (test.yml) gains a cheap `cargo check --target wasm32-unknown-unknown` step so regressions to the wasm path are caught at PR time without the cost of a full wasm-pack build. Verified locally: `npm run build:wasm` produces dist/wasm/ with the expected six artifacts (`.wasm`, `_bg.js`, `.d.ts`, inline shim, etc).
Biome lint flagged two issues on the same console.log call: - lint/style/useTemplate (string concat across two template literals) - lint/style/noUnusedTemplateLiteral (the prefix half had no interpolation) Both fixes converge on a single template literal.
Repo config (biome.json) uses single quotes and "asNeeded" semicolons. The new script was authored with the cipherstash-suite style (double quotes + always-on semicolons) and slipped through PR #88's prior fixup because that one only ran biome lint, not biome format. Verified with `biome format scripts/inline-wasm.mjs` — clean.
Four hardening fixes from CodeRabbit's review, plus one pre-existing
defect in the same file picked up for consistency:
1. **Pin GitHub Actions to immutable SHAs.** `baptiste0928/cargo-install`,
`actions/upload-artifact`, `actions/download-artifact`, and the newly
added `actions/checkout` references all swap from tag pins (`@v2` /
`@v4`) to full 40-char commit SHAs with `# vN` trailing comments —
matches the convention already in use for `softprops/action-gh-release`
in the same file.
2. **Upgrade `actions/checkout@v3` → `@v4` in the wasm job.** v3 uses the
deprecated Node 16 runtime; v4 uses Node 20. CodeRabbit flagged this
on the new wasm job only — the existing `main` job at line 192 had the
same defect, fixed in the same pass since it's a one-line change in
the same file.
3. **Disable credential persistence on checkout.** `persist-credentials:
false` on both wasm and main jobs. The default is `true`, which leaves
the token in `.git/config` for subsequent steps — unnecessary here
since neither job pushes back to the repo.
4. **Validate `inputs.version` before `npm version`.** The previous
`npm version ${{ inputs.version }}` expanded the workflow input
directly into the shell, an injection sink. Now binds the input to a
`VERSION_INPUT` env var so it never expands into the shell body, then
gates it with a `case` allowlist: only npm bump keywords
(`patch|minor|major|prepatch|preminor|premajor|prerelease`) or strict
semver (with optional `v` prefix and pre-release suffix) are accepted;
anything else exits non-zero before `npm version` runs.
5. **Fix ESM module semantics for `./wasm` and `./wasm-inline` exports.**
`wasm-pack --target bundler` emits ESM (`import`/`export`) and the
inline shim is ESM too. The root `package.json` has no
`"type": "module"`, so without a scoped marker Node parses these
`.js` files as CJS and fails to load the subpath exports. Have
`scripts/inline-wasm.mjs` write a sibling `dist/wasm/package.json`
with `{"type":"module"}` so only `dist/wasm/**` is treated as ESM;
the root package and Neon path stay CJS. Add the marker to the
`files` whitelist so it ships in the published tarball.
Verified locally:
- `node scripts/inline-wasm.mjs` produces both `protect_ffi_inline.js`
and `package.json` ({"type":"module"}) under `dist/wasm/`.
- `npx @biomejs/biome lint scripts/inline-wasm.mjs` clean.
- `npx @biomejs/biome format scripts/inline-wasm.mjs` clean.
- Structural check of `build.yml` confirms 7 SHA-pinned action refs,
`persist-credentials: false` present, allowlist case present.
Refs: cipherstash/protectjs-ffi#88
Replace the WasmClient class-method API (`client.encrypt(opts)`) with top-level wasm-bindgen functions taking `&WasmClient` as their first argument (`encrypt(client, opts)`). This matches the existing `@cipherstash/protect-ffi` Neon-side JS API exactly, so the conditional `exports` map (`node` → CJS, `default` → wasm) can resolve to the wasm output without consumers having to rewrite call sites between native and Edge runtimes. Also make `scripts/inline-wasm.mjs` derive the re-export list dynamically from the wasm-pack bundler stub (`protect_ffi.js`) instead of hardcoding it — avoids drift when wasm-bindgen exports are added or renamed. Verified by replaying \`npm test\` against a clean tree (typecheck → unit → lint → format → rust): all green. \`wasm-pack build\` produces 14 top-level names (\`encrypt\`, \`encryptBulk\`, \`encryptQuery\`, \`encryptQueryBulk\`, \`decrypt\`, \`decryptBulk\`, \`decryptBulkFallible\`, \`isEncrypted\`, \`newClient\`, plus \`WasmClient\` handle, \`init\` panic-hook entry, and wasm-bindgen's auto \`IntoUnderlying*\` stream types) and the inline shim picks them up automatically.
Supabase Edge's module-graph resolver treats wasm-bindgen's `@ts-self-types="./protect_ffi.d.ts"` directive as a hard external import and refuses to boot with "Module not found protect_ffi.d.ts", even when the .d.ts file is present alongside the inline JS. The inline build is the path used by sandboxed Edge runtimes that consume this entry directly via an import-map alias — there's no package-resolution machinery to wire types in via the package.json exports map. Drop the directive. Bundler/Node consumers still get types through the `exports` map's `types` condition pointing at protect_ffi.d.ts.
Convert the prior \`_spikes/node-spike\` scaffold into a proper vitest integration test under \`integration-tests/tests/wasm-round-trip.test.ts\`. The spike was only ever meant for local poking and shouldn't have been committed in that shape — this gives the wasm path the same kind of coverage the Neon-side tests already have. The new test: - Lives alongside the existing Neon-side integration tests (same vitest config, same dotenv-loaded credentials, same docker-compose harness). - Uses \`describe.skipIf\` on the four required env vars (\`CS_REGION\`, \`CS_CLIENT_ACCESS_KEY\`, \`CS_CLIENT_ID\`, \`CS_CLIENT_KEY\`) so it's a clean no-op when credentials aren't configured — matches how the existing tests behave without \`~/.cipherstash/secretkey.json\`. - Throws a clear "run \`npm run build:wasm\` from the repo root" error if \`dist/wasm/protect_ffi_inline.js\` is missing when the suite actually executes, rather than failing with an opaque ESM module-not-found. - Pulls \`@cipherstash/auth@0.37.0-alpha.8\` as a real dependency of \`integration-tests/\` and constructs \`AccessKeyStrategy\` exactly the way a wasm consumer would. Imports the \`/wasm-inline\` subpath so the test runs anywhere with a working WebAssembly engine without needing the platform-specific napi peer dep installed. - Asserts the full round-trip: \`newClient\` → \`encrypt\` → \`isEncrypted(...)\` is true → \`decrypt\` returns the original plaintext. Uses dynamic \`import()\` on a path resolved from \`__dirname\` (the integration-tests package is CJS via \`module: "node16"\` + no \`"type": "module"\`). Verified: - \`npx vitest run tests/wasm-round-trip.test.ts\` from \`integration-tests/\` with no env: 1 test, 1 skipped (as designed). - \`npm test\` from the repo root against a clean tree (typecheck → unit → biome lint → biome format → cargo test): all green. The \`_spikes/\` directory is removed; the \`@cipherstash/auth/wasm-inline\` dep ports across to \`integration-tests/package.json\` as the only durable artifact from the spike.
feat(wasm): node + Edge smoke tests for end-to-end testing (4/4)
Unifies the newClient signature so callers can pass an @cipherstash/auth-shaped strategy on either target. WASM previously took strategy as a separate first arg; Node had no way to supply a JS-backed strategy at all. - WASM: newClient(opts) with required opts.strategy (extracted via Reflect::get before serde — JS functions don't survive serde_wasm_bindgen). - Node: newClient(opts) with optional opts.strategy. Adds NeonJsAuthStrategy (Root<JsFunction> + Channel captured at module init) and a NodeAuthStrategy enum that wraps either AutoStrategy or the JS- backed variant, so the Client type stays concrete and #[neon::export] signatures don't need to become generic. Falls back to AutoStrategy from opts.clientOpts.creds when strategy is absent. - TS shim and wasm-round-trip test updated for the new signature. getToken is called on every ZeroKMS request on both targets — caching is the JS strategy's responsibility.
Absorbing protect-ffi turned `lib/`, `index.node` and `dist/wasm/**` from
tarball contents into build outputs, and nothing in CI produced them. Seven
jobs failed for three distinct reasons, all the same root cause:
- `Run Tests (Node 22/24)` — 4 TypeCheckErrors, because `tests.yml` calls
`pnpm --filter … run test:types` directly and turbo's `^build` never runs,
so protect-ffi's declarations resolved to nothing.
- Drizzle x2, Supabase, prisma-next — `Cannot find module
'.../protect-ffi-linux-x64-gnu/index.node'`.
- `Run WASM E2E Tests (Deno)` — `Module not found
'.../dist/wasm/protect_ffi_inline.js'`.
Adds `.github/actions/build-ffi-binding`, a composite that builds those
artifacts and then proves they load. A composite rather than a reusable
workflow with an artifact: the integration jobs own their database service and
credentials, so the build has to happen inside them, and artifacts do not cross
workflow files anyway.
It caches `index.node` (13MB) on a content hash of the Rust inputs rather than
cargo's `target/`, which runs to gigabytes and is slower to save and restore
than the compile it saves. The verification step runs on both the hit and miss
paths — a cache that restores nothing otherwise surfaces as dozens of unrelated
encryption failures deep in a credentialed suite instead of one legible error.
`wasm: 'true'` for exactly two jobs: the Deno smoke test, and the Drizzle job,
whose CS_IT_SUITE includes `integration/wasm/**`. Every `wasm-inline` unit test
either mocks the module or asserts on the bundle graph, so the unit suite does
not need it. wasm-pack is pinned in mise.toml at the version upstream used,
spelled with its full backend id because the short name is not in mise's
registry and resolves to nothing.
Also adds `packages/protect-ffi/crates/**` and `src/**` to the three
integration path filters: a crate change can now break those suites in a PR
that touches no TypeScript, and without this it would skip them.
AGENTS.md promised contributors that cargo stays off every PR job. That is now
true of the scripts only, and it says so.
The ported matrix selected `build` for the four non-gnu platforms. Upstream's `build` was its cargo script; here it is `tsc` and nothing else, so those jobs would have produced no binary and failed a step later on a missing cargo.log. The two cargo scripts also write different logs (cargo.log vs zig.log) and `neon dist` reads one of them, so both the script and its log are matrix fields now. Also: - `package-manager-cache: false` on every setup-node in the artifact workflow, which Task 5 puts on the caching lint's target list, and setup-node pinned to v6.5.0 — the input does not exist before v5. - `working_directory: packages/protect-ffi` on both mise steps. The mise config is nested and there is no root one, so from the repo root the action installs nothing. zig/cargo-zigbuild are now scoped to the gnu targets and wasm-pack to the wrapper job. - Dropped the unused `workflow_call` output; a reusable workflow's output has to map to a job output, and no caller read it. - Tag creation verifies the existing tag points at GITHUB_SHA, and the release attaches to the wrapper's own tag with `--verify-tag` (no eighth tag) and uploads assets unconditionally with `--clobber`. - Pre-flight distinguishes the two linux-x64 binaries by ABI (`readelf -d`); `file` reports both identically. - Task 8's dispatch named a stale branch and ran before the workflow existed on the default branch, where `workflow_dispatch` is resolved from. - Added `.github/actionlint.yaml` for the Blacksmith label, and made the snippets shellcheck-clean: Task 6 makes actionlint a gate over these files and actionlint has never run in this repo. - Dropped the pass-through `version:` hook task; it duplicates the action's default and the existing `changeset:version` script, and belongs in the EQL absorption where a Cargo.toml version actually flows through it. Verified by extracting every workflow snippet and running actionlint (with shellcheck) and scripts/lint-no-workflow-caching.mjs over all four.
Since the absorption these checks ran NOWHERE. Phase 1 deliberately moved
`cargo test` + `cargo fmt --check` behind `test:cargo` and clippy behind
`mise run lint:rust`, to keep cargo off every contributor's default
`pnpm test` — but no root workflow picked them back up, and GitHub only reads
workflows from the repository root, so the deposited
`packages/protect-ffi/.github/workflows/test.yml` never executed.
`lintWiring.test.ts` asserted against that deposited copy and said so in a
CAVEAT: it was "the specification the phase-3 pipeline port has to satisfy",
vacuous until one existed. It now reads `.github/workflows/tests-rust.yml`, so
a failure there means the Rust checks have stopped running — the exact
condition that held silently for the whole import.
Verified locally, all three arms:
cargo test 310 passed, 0 failed
cargo fmt --check clean
clippy host clean
clippy wasm32 clean
Two deviations from the plan's draft:
- `jdx/mise-action` needs `working_directory: packages/protect-ffi`. mise
reads config from the current directory and its PARENTS, so an action at
the repo root never sees the nested mise.toml — it would install nothing
and leave the config untrusted, and `mise run lint:rust` would then fail
with "Config files ... are not trusted", which reads as a toolchain
problem rather than a trust one. Fixed here and in the plan snippet.
- `git rm -r packages/protect-ffi/.github` is deferred. Task 4 still cites
that directory's `build.yml` and `actions/setup/action.yml` as its
reference material and is unwritten, so deleting now would remove the
source for it. The plan's Step 5 is gated on Task 4 accordingly.
Caching is left enabled: this workflow publishes nothing, so
scripts/lint-no-workflow-caching.mjs does not cover it, and
`cargo:cargo-zigbuild` builds from source — the cache makes that a one-off.
The `build-ffi-binding` step landed ahead of `require-cs-secrets` in all seven
jobs that use both. The secrets action exists to be cheap — it reads four
inputs and fails in seconds when a CS_* secret is rotated, cleared, or absent
because the PR came from a fork, and every workflow carrying it says so
("Fast pre-flight: fail in seconds ... before the docker pull"). Putting a cold
Rust compile in front of it means a job with no usable credentials pays minutes
before learning it was never going to encrypt anything, which is the same as
having no pre-flight.
Six jobs had the two steps adjacent, so the build moved down. The seventh did
not, and moving it down there would have BROKEN the job: in tests.yml's
`wasm-e2e-tests` a `Build stack` step sits between them and consumes
`dist/wasm/**`, so the build must stay ahead of it. There the pre-flight moved
UP instead — same resulting order, and the reason is recorded in both the
workflow comment and the test header so the next edit does not reverse it.
Guarded by scripts/__tests__/ffi-binding-step-order.test.mjs. It discovers the
jobs by scanning `.github/workflows/` rather than from a list, so a new
workflow pairing the two actions is covered the day it lands, and carries a
minimum-set guard because a discovery test that matches zero files passes and
proves nothing — the failure mode `lintWiring.test.ts` and
`lint-no-hardcoded-runners.mjs` both exist to prevent.
Also aligns `pnpm/action-setup` in integration-setup to v6.0.9. It was the only
v6.0.8 call site out of twelve.
**The WASM cache key omitted the tracked declaration files.** `dist/wasm` is
cached under a hash of the Rust inputs only, but three files in that directory
are tracked in git (`protect_ffi.d.ts`, `protect_ffi_bg.wasm.d.ts`,
`errors.d.ts`). An entry saved before a `.d.ts` edit restored over that edit
silently.
Excluding them from the archive does not work, and reads as a fix while
changing nothing. `actions/cache` resolves `path:` through
`glob.create(..., {implicitDescendants: false})`, so a bare directory yields
ONE glob result — the directory — and tar recurses on its own. A
`!.../*.d.ts` line subtracts from a set the files were never in. Verified
against actions/toolkit `cacheUtils.ts`, not from memory.
So the key hashes `dist/wasm/*.d.ts` too. The glob rather than three literal
paths is what makes it structural: an entry is only restorable if its key
matches, which embeds the current declarations' hash, so any entry that hits
was saved from a byte-identical checkout and the overwrite is a no-op by
construction — and a fourth tracked declaration file is covered automatically.
The native cache is deliberately unchanged: `index.node` is gitignored, so it
has no tracked content to clobber. That asymmetry is now in the comment.
**`jdx/mise-action` was pinned to a mutable major tag.** It is a third-party
trust dependency this absorption introduced, and it runs in jobs holding live
CipherStash credentials. Now `@5228313ee0372e111a38da051671ca30fc5a96db # v3.6.3`,
matching the `@<sha> # <tag>` convention the upstream workflows already used for
`baptiste0928/cargo-install`. The SHA was verified three ways: `v3` is a
lightweight tag resolving to that commit, the commit exists
(`chore: release v3.6.3`), and the tag listing maps both `v3` and `v3.6.3` to it.
Guarded by scripts/__tests__/ffi-binding-action.test.mjs. It derives the
tracked-file list from `git ls-files` at test time rather than hardcoding it,
models the `implicitDescendants: false` behaviour so it will not accept a
non-functional `!` exclusion as a fix, and was mutation-checked against a
fourth staged `.d.ts` — passing with the glob key, failing with literal paths.
Follow-ups from reviewing the review. Each is the same shape as the bug it sits next to: a check that looks like it runs, or a key that looks like it covers its inputs. **The CS_CLIENT_KEY hex guard was bypassable.** `printf '%s' "$KEY" | grep -qE '^[0-9a-fA-F]+$'` matches per LINE and `-q` succeeds when ANY line does, so a value of "deadbeef\n<anything>" passed whenever its total length was even — the one shape the step exists to reject. Verified both spellings against that input. Now `[[ =~ ]]`, where bash anchors the whole string and `$` is end-of-string rather than end-of-line. **`test:typecheck:wasm` ran nowhere.** It was carved out of protect-ffi's `test` entry point with the prose reason "run by the wasm job" — and the only jobs running it were the upstream copies under packages/protect-ffi/.github/, which GitHub does not execute. Same root cause as the Rust checks in bc0cb13, found because that fix did not generalise. tests.yml now runs it in the job that has both halves of the build, and lintWiring.test.ts grew a test asserting every ENTRY_POINT_EXEMPT script appears in a ROOT workflow — the exemption list was otherwise a way to launder an orphan into an intention, since the reason string is prose and prose does not fail. **Both cache keys missed build inputs.** They hashed the Rust only, but `build:native` is `cargo-build` plus a `postcargo-build` hook, both defined in package.json, and mise.toml pins the toolchain. `build:wasm` additionally runs `tsc -p tsconfig.wasm-errors.json` and a `postbuild:wasm` hook executing scripts/inline-wasm.mjs — which emits the protect_ffi_inline.js that stack's wasm-inline entry imports. An edit to the inliner with no Rust change is the likely miss, and it is precisely what a Rust-only hash cannot see: the hit skips the build and the job proceeds on a stale artifact. **lint-no-ffi-changeset printed a path that need not exist.** The offender list hardcoded a `.changeset/` prefix while the directory is argv-overridable for the self-tests. Now relative to the repo root. Also: the workflow path filters, the changeset, and skills/stash-auth follow the same edits through.
`packages/protect-ffi/integration-tests/` came across the absorption intact —
19 files of live coverage over encrypt/decrypt, lock context, keysets, JS auth
strategies, JSON SteVec, Postgres on EQL v2 *and* v3, and a WASM round trip —
and then ran nowhere at all. Upstream drove it from `test.yml` on every PR; that
workflow was deposited under `packages/protect-ffi/.github/`, a directory GitHub
never reads. Several of those paths are exercised in no other suite.
`integration-protect-ffi.yml` runs it again: path-filtered to this package,
credentialed, fork-PR-skipped like the other `integration-*.yml` jobs, and
running the FULL suite including `tests/lock-context.test.ts` (upstream CI ran
`:all`, not the lock-context-excluding variant).
Two departures from upstream, both deliberate:
- It builds via `.github/actions/build-ffi-binding` with `wasm: 'true'` rather
than `mise run build:debug`. `src/load.cts`'s fallback is
`require('../index.node')` — the package root, which is where bare `neon dist`
writes — so a *release* binding satisfies the loader, and that action caches
it. Rebuilding in the debug profile would recompile the cipherstash client
graph over an artifact the rest of CI already paid for.
- It calls vitest directly instead of `mise run test:integration:all`, for the
same reason: that task rebuilds the binding and the WASM itself.
It does NOT use `.github/actions/integration-db`. That action installs no EQL,
and `tasks.toml` pipes both EQL bundles through `docker exec -i
protect-ffi-postgres` — a container only this suite's own compose file produces.
Nothing else in the repo installs EQL v2, which `tests/postgres.test.ts`
requires.
`PGPORT` is 5436, not upstream's 5432: `docker-compose.yml` publishes
`5436:5432`. Upstream's 5432 was inert because every invocation went through
mise, whose `[env]` overrides an inherited value. The vitest step here runs
outside mise, so the job env has to be right on its own.
`src/integrationSuiteCi.test.ts` is the regression test. It requires a root
workflow that both names the suite directory AND starts vitest — a bare `paths:`
mention is the false positive it exists to reject — and asserts the binding
build, the secrets pre-flight and the absence of a lock-context exclusion.
`mise.toml`'s `build:debug` also drops its `npm run` spelling, a leftover from
the npm-based upstream.
`ENTRY_POINT_EXEMPT` excuses a script from the reachability check with a prose reason, and prose does not fail. `test:typecheck:wasm` carried the reason "run by the wasm job" while no root workflow ran it — the jobs its exemption named were the upstream copies under `packages/protect-ffi/.github/`. The guard that was supposed to catch a check going quiet was itself the place one hid. The workflow scan now reads the root workflow DIRECTORY rather than two hardcoded filenames. A hardcoded list has to be maintained in step with a set of files nothing forces it to track: move a job to a new workflow and the check reads "this script runs nowhere", and the tempting repair is widening the list until it admits the dead package-local path. A directory cannot drift out of date with itself. Both directory-based checks pass by finding nothing to contradict, so `reads the files it means to read` now also pins that the scan resolved. Second defect, second test: `release` and `dryrun` were `gh workflow run release.yml -f dryrun=… -f version=…`, written for the standalone repo's `workflow_dispatch` release. This repo has a root `release.yml` too — Changesets-driven, `on: push` to main, no inputs — so an existence check on the path is GREEN while both scripts exit non-zero. The name survived the absorption and nothing else about the workflow did. The test checks the trigger and the declared inputs, not just the path, and both scripts are deleted. A package-level `release` script is also a `turbo run release` target, so a stale one could dispatch a real workflow from an unrelated command. `scripts/changelog-extract.mjs` stays: the deposited `release.yml` is retained as the reference for the phase-3 release port and is its only caller. The README's Available Scripts section was stale in the same way, and prose drift is not caught by anything. It documented `npm run build` as producing `index.node` (it is `tsc` now — `build:native` carries the cargo release build), `npm test` as running the Rust tests (moved to `test:cargo`), and the two deleted scripts as live. `build:native`, `test:cargo` and `test:typecheck:wasm` were undocumented. Worst, it taught `npm run build -- --feature=beetle`: pnpm forwards `--` verbatim where npm strips it, and these scripts end in `> cargo.log`, so the flag lands after the redirect and cargo rejects it — the exact anti-pattern lintWiring asserts against.
Absorbing protect-ffi brought a 494-crate `Cargo.lock` in-tree. osv-scanner already saw it — `--recursive ./` extracts every lockfile it recognises, and it reports seven crates.io advisories today — so known vulnerabilities were visible from day one, but nothing proposed routine version updates. That is the gap this closes, and the only one. The test is the point, not the entry. It derives the required ecosystems from the lockfiles actually present (`git ls-files --cached --others --exclude-standard`, which delegates the node_modules/target/dist exclusions to git and fails on an uncommitted lockfile), matches them by SHAPE, then maps basename to ecosystem. A lockfile matching the shape with no mapping fails as `unrecognised` — so the next language someone adds fails the suite instead of going silently unmonitored. `cargo` is not hardcoded as an expectation: delete `Cargo.lock` and the test stops demanding it. A second test asserts every entry's `directory` actually contains the manifest its ecosystem reads, because a misaimed `directory` fails silently — Dependabot records "no manifest found" on a log page nobody visits and the only symptom is that no PR ever arrives. Two lockfiles are exempt with their reasons named in the test: `e2e/wasm/deno.lock` (JSR specifiers; no Dependabot Deno ecosystem, and the npm versions that matter are in `pnpm-lock.yaml`) and `.flox/env/manifest.lock` (Nix; not an ecosystem). Monthly, where npm and actions are weekly — a bump here is validated by `tests-rust.yml` (one Blacksmith job, cargo test plus clippy on host AND wasm32) and by a six-platform cross-build at release. Security updates are alert-driven, so nothing urgent is delayed by the cadence. The six CipherStash crates are ignored because they are pinned with exact `=` requirements and share a release train with the `@cipherstash/auth` catalog entries — `cipherstash-client`, `cts-common`, `stack-auth` and `stack-profile` all sit at `=0.42.0` against the catalog's 0.42.0. A subset bump is the Rust version of the rc.2 B1 skew the file already documents. `ignore` also suppresses Dependabot SECURITY PRs for those names; osv-scanner is the compensating control and the entry says so. `packages/protect-ffi/integration-tests/package-lock.json` is covered at the ecosystem level but gets no PRs — the npm entry follows the pnpm workspace and that directory is not in it. Left deliberately: absorbing it into the workspace deletes that lockfile, so monitoring it would be churn on a file we intend to remove. Recorded as follow-up. The bundled skill claimed two monitored ecosystems, so it needed a `stash` patch changeset. Two pre-existing untruths went with it: majors do not "stay un-grouped — one PR each" (every entry ignores `version-update:semver-major`, so none are proposed), and `ignore` suppressing security PRs was unstated.
The Status table stopped at phase 2 while six phase-3 commits had landed. Adds a progress table, and records the audit of the absorbed tree against upstream `ce820bb` (v0.31.0) — the import is faithful, 200 tracked files in and out with the full 613-commit history grafted, and every content diff reduces to a deliberate commit. The three defects the audit found shared one shape, which is the part worth keeping: a check that came across as files and then ran nowhere, because what used to invoke it was the upstream workflow deposited under `packages/protect-ffi/.github/` — a directory GitHub never reads. `test:typecheck:wasm` had an exemption claiming a job ran it; the 19-file integration suite had no claim at all. Prose in a guard is not a guard. Marks the Rust checks, the WASM typecheck, the integration suite, the workflow-dispatch check and Dependabot ecosystem coverage done. Adds one deliberately unchecked item: `integration-protect-ffi.yml`'s first run is unproven, because a dev machine has neither Docker nor credentials — with the five things to watch, in order of likelihood. Also records six deferred follow-ups with their reasons, the load-bearing one being that pnpm-absorbing `integration-tests/` changes its dependency pins, and only a credentialed run can show that is neutral. Doing it in the same breath as the wiring fix would confound the two.
…sses Mutation-tested every assertion in the four workflow guard suites by breaking what each one guards and checking it went red. Eighteen held. Three did not, and all three failed in the same direction — passing while the thing they describe was gone. **The step-order guard counted files, not jobs.** Deleting the `Require CipherStash secrets` step from tests.yml's `wasm-e2e-tests` job left the suite GREEN: 9 tests became 8 passing with nothing red. The per-job checks are generated by discovery, so removing the pre-flight did not fail a check, it deleted one — and tests.yml stayed in PAIRED_FILES through `run-tests` while the `PAIRED.length >= 6` floor had three jobs of slack. The victim is the most expensive job in CI, the one that builds with `wasm: 'true'`. Now pinned to eight `file / job` ids with no count floor, which also closed a live hole: integration-protect-ffi.yml paired the two actions but was absent from the file list, so its check could vanish the same way. **A derived requirement with no premise assertion.** integration-workflow-paths builds `required` from the suites' `from '@/…'` imports, so rewriting them to `@cipherstash/stack/…` — an ordinary "test the built package" refactor — empties the set and the check then passes with `packages/stack/src/dynamodb/**` deleted from the filter. That is verbatim the #815 gap the file's own header cites. The sibling manifest check already asserted `required.size > 0`; this one did not. **Two real Changesets spellings nothing covered.** The parser handles both; no test would have noticed if it stopped. Appending `.slice(0, 1)` to the frontmatter split left 10/10 green, because every fixture named one package on line one — while the likeliest real offender is a single `pnpm changeset` run selecting your package and protect-ffi together. Dropping both `\r?` also left it green, so a Windows checkout (`core.autocrlf=true`, and this repo has no .gitattributes) would sail through. The CRLF case is generated in a tmpdir rather than committed: a CRLF fixture in git is one autocrlf commit away from being normalised to LF and silently disarmed. No changeset — test-only repo tooling.
…ve citation Mutation-tested the three protect-ffi wiring guards. nativeLoading held on all seven. lintWiring held on twelve and failed on three; integrationSuiteCi held on four and failed on four. **Cargo could reach the default test path two ways the walk could not see.** This is the load-bearing half of the entry-point split — root `pnpm test` runs `turbo test --filter './packages/*'`, so cargo there is a Rust toolchain on every contributor's machine. `"pretest": "cargo test"` passed: pnpm 10.33.2 runs pre/post hooks with no opt-in, nothing names them, and this manifest already leans on the behaviour (`postcargo-build`, `postbuild:wasm`, `prepack`), so it is an edit in keeping with the file. `"test": "… && mise run lint:rust"` also passed: `mise run` leaves package.json entirely, and all three `lint:rust` arms are cargo, so that is cargo on every `pnpm test` with no `cargo` token in the manifest at all. `reachableFrom` now follows the hooks, and `taskReachesCargo` resolves mise hops including the two-hop `build:debug` → `pnpm run debug`. **The check against #145 was itself the #145 failure.** `runs the lint entry point in CI` asserted `toContain('mise run lint:rust')`, which `mise run lint:rust:host` satisfies as a prefix — green for a workflow running one arm and skipping the other two, which is the orphaned-arm bug the assertion exists to catch. Now terminated with `(?![\w:-])`. Under a second layer: tests-rust.yml's header COMMENT contains the same string, so even an anchored regex passed until comments were stripped. Same trap nativeLoading.test.ts already documents for the emitted entry, and integrationSuiteCi had all four of its failures for this one reason — `build-ffi-binding` survives in six comments and two `paths:` entries after the step itself is deleted. Both files now match against executable content only. **mise.toml cited a step that runs nowhere.** It told contributors CI installs the wasm32 target "in the `Add wasm32 target` step of test.yml". The step that runs is in the root tests-rust.yml; the only file named test.yml is the inert upstream copy under packages/protect-ffi/.github/. Second instance of the same false justification after `test:typecheck:wasm`'s "run by the wasm job", so it is now guarded rather than only fixed: lintWiring fails on any citation naming a workflow that exists ONLY in that directory. It reads the directory via existsSync, so the phase-4 cutover deleting it retires the guard automatically. Those three files stay. docs/plans/2026-08-04 defers the deletion and records that the phase-4 port still cites build.yml's per-platform CARGO_BUILD_TARGET matrix — omitting it ships an ARM binary as darwin-x64 — so deleting now would remove the spec for unfinished work. No changeset — test-only, plus a comment correction.
`.github/actions/build-ffi-binding/action.yml` opens with "DO NOT USE FROM A
PUBLISHING WORKFLOW. It restores the GitHub Actions cache, which
`scripts/lint-no-workflow-caching.mjs` forbids anywhere an artifact gets
published." That was documentation, not enforcement.
The script tested each step's own top-level `uses:` against
`/^actions\/cache(\/(restore|save))?@/` and stopped dead at one level of
indirection. Confirmed against a copy of the real release.yml with
`uses: ./.github/actions/build-ffi-binding` spliced in and a copy of the real
composite alongside it: exit 0, no output, while the composite it never opened
restores two caches into the credential-bearing publishing job. The same class
of failure the absorption work has been closing all week — a check that never
runs reads exactly like a check that passes — sitting inside the guard itself.
`walkSteps` now follows any `uses:` matching `^\.{1,2}/` into the action's
`runs.steps` (`runs`, not `jobs` — an action manifest has a different shape),
accepts `action.yml` or `action.yaml`, and recurses. Messages render the whole
trail; one naming only the workflow step sends the reader to a file with no
`actions/cache` anywhere in it.
**The `pnpm/action-setup` / `actions/setup-node` explicit-`false` rules apply
inside composites too.** The action runs in the same job, with the same
credentials, defaulting the same way. Exempting composites would make "move the
step into a composite" a supported way out of the rule — the bug this traversal
closes, one level down. The cost is bounded: traversal is target-scoped, so only
composites reachable from release.yml and tests-supply-chain.yml are constrained.
`visited` is per job, not per run — a job is the unit of credential exposure, so
one report per job, while a composite shared by two jobs is still named in each.
It also breaks the `A uses B uses A` cycle. An unresolvable local `uses:` exits
**2**, matching lint-no-hardcoded-runners.mjs: nothing was found caching, the
linter could not look, and skipping silently would turn a typo'd path into a
permanent exemption.
Thirteen tests, all confirmed failing first (exit 0 where 1 or 2 expected).
The last is a live citation: it asserts the real build-ffi-binding still has an
`actions/cache` step before asserting the linter flags a workflow using it, so
it cannot go vacuous the day that action stops caching.
Neither target workflow uses a local composite today, so this is a forward gate,
not a live regression. `lint:workflow-cache` still passes on both.
The Dependabot `directory` check added with the cargo entry treated every configured location as a literal path. Dependabot's options reference is explicit that it is not: "The `directories` key supports globbing and the wildcard character `*`. These features are not supported by the `directory` key." No entry uses `directories` today, so the check was correct as committed — but the first person to write `directories: ["/packages/*"]` would have been failed by a message reporting no package.json at a path never meant to be read literally. **The two keys are now checked differently, on purpose.** Glob-expanding both uniformly would have been the obvious fix and the wrong one: because `directory` does not glob, a `*` written there is a literal path segment to Dependabot and the entry monitors nothing — which is the exact failure this check exists to catch. Expanding it here would hide it. The regression test asserts both halves: `/packages/*` passes under `directories` and fails under `directory`. A glob is satisfied by matching **at least one** directory holding the manifest. An every-match rule would fail against this tree today: `/packages/*` covers `packages/utils/`, which holds only `config/` and `logger/` and no package.json of its own. `globSync` from `node:fs` — Node 22 is already the floor in `engines`, and a check on dependency policy is a poor place to add a dependency. Also fails `directories: []`. It has no entry to be wrong about, so a per-directory loop reports nothing and an entry monitoring nothing goes green — the same defect arriving as an absence rather than a wrong value. Mutation-tested: forcing the glob branch true and false each fails the new test, and breaking `MANIFEST_BY_ECOSYSTEM.cargo` still fails the live check, so the extraction into `unmonitoredDirectories` did not defang it.
`nativeLoading.test.ts` asserts against the EMITTED entry, so it carries a prerequisite the package's `test` chain satisfies (`test:typecheck` emits `lib/` before `test:unit`) and a bare `test:unit` does not. Reproduced with `lib/` moved aside: the whole file fails as `ENOENT: no such file or directory, open '.../lib/index.cjs'` pointed at the readFileSync — which names the missing file but not the command that produces it, and lands on whoever runs `test:unit` directly. Now throws first, naming both recovery paths. Verified by running it with `lib/` moved aside rather than assumed: the developer-visible vitest output is the written message, not the ENOENT. `lib/` restored and checksummed byte-identical across all 226 files afterwards. No assertion changed.
Phase 3 Task 2 repoints the seven FFI manifests at this repository. Step 3 is a blanket `perl` substitution of the host; Step 4 separately rewrites the six platform `repository.directory` values from `platforms/<name>` to `packages/protect-ffi/platforms/<name>`. The Step 1 test asserted `repository.url`, the wrapper's `bugs`/`homepage`, and "no `protectjs-ffi` substring" — and `"directory": "platforms/darwin-arm64"` contains no such substring. Ran the plan's verbatim test against a Step-3-only tree: **10 passed (10)**, byte-identical to the result Step 6 told you to expect, with all six directories still wrong. Skip Step 4 and the plan's own verification reports success. Same defect as a check nothing invokes, one level up in the process. Now 16 assertions. Re-derived every count the plan states against three tree states: pristine `15 failed | 1 passed`, Step-3-only `6 failed | 10 passed`, Step 3+4 `16 passed`. Also records why the current values are not simply a bug: `repository.directory` resolves from the root of the repository named in `repository.url`, and the old repo's root really does hold `platforms/`. Both fields are correct as published today and wrong the moment publishing moves — and they fail differently. A stale `url` fails the OIDC publish outright; a `directory` that does not resolve publishes fine and silently breaks the source link on the package page. That asymmetry is why the `url` half is the blocker and the `directory` half is the one nothing would have caught.
`@cipherstash/stack` is 1.0.0, and a `clientKey` in base64 — which worked on 1.x — now fails at client construction. Stack pins `@cipherstash/protect-ffi` exactly, so upgrading stack forces the new FFI: there is no version of this a caller opts into separately, and no supported way to stay on the old decoder. That hex was always the documented encoding describes intent, not the behaviour anyone was running against, and semver contracts on the latter. The fixed group takes `stash`, `wizard` and the three adapters to 2.0.0 with it; that is a release-management cost, not an argument about what the version number means. Reasoning recorded in the changeset body so it reaches the CHANGELOG rather than living only in PR discussion.
402af3f taught this gate to follow `uses: ./.github/actions/<name>` into a composite and flagged, but did not close, the second indirection: a job whose body is `uses: ./.github/workflows/x.yml`. Such a job has no `steps:` at all, so `Array.isArray(job?.steps) ? … : []` handed `walkSteps` an empty list and skipped the job entire — every step of the called workflow, and any composite it reaches, invisible. Confirmed against a caller whose only job was `uses: ./.github/workflows/reusable.yml` with `secrets: inherit`, the called workflow holding an `actions/cache@v4`: `OK`, exit 0, nothing scanned. `walkJob` now dispatches on the two job shapes and recurses mutually with `followReusableWorkflow`, sharing the existing per-top-level-job `visited` set so `a.yml -> b.yml -> a.yml` terminates the way composite cycles already do. The two mechanisms compose: workflow -> composite -> cache is one of the tests. **`secrets:` deliberately does not change the verdict.** It is not the only credential channel — `permissions:` is inherited independently, and that is what mints the OIDC token npm trusted publishing signs with, so a call passing no secrets can still publish. A restore also does not need credentials in its own job to be the attack: poisoned bytes landing in a build job that hands an artifact to a publish job is the canonical shape. Conditioning on `secrets:` would prevent no failure and would hand an attacker a phrasing that evades it. **A remote reusable workflow is reported, not skipped**, where a remote *step* action is skipped. That is coverage, not depth: a marketplace step sits inside a job whose step list this gate read end to end, and flagging every `actions/checkout@v6` would make it exit 2 forever and mean nothing. A remote job-level `uses:` is the whole job — no steps read, no verdict reached, `OK` printed anyway. Exit 2 keeps the "could not look" contract distinct from "found caching", per lint-no-hardcoded-runners.mjs. A job carrying both `steps:` and `uses:` is invalid to GitHub's schema, which rejects the file outright — but this gate runs on files GitHub has not validated yet, so both halves are checked rather than one being trusted. The job object is never passed to `checkStep`: at job level `with:` is inputs to the called workflow, not action inputs. No live instance. The repo's only job-level `uses:` is osv-scanner.yml's remote `google/osv-scanner-action/...@v2.3.8`, and that workflow is not a target of this gate; there are zero `workflow_call` triggers anywhere. Forward gate, like the composite traversal was. Twelve tests; the ten traversal cases confirmed failing first, the two over-trigger guards vacuous by design. `lint:workflow-cache` still passes on both real targets. test:scripts 238.
Mutation-testing 08cc337 found one surviving mutation out of seventeen: delete the `prefix ? \`${prefix}/${manifest}\` : manifest` ternary and all 21 tests still pass. That ternary exists for `directory: '/'` — which is what BOTH live entries, npm and github-actions, actually use — and nothing exercised it. It is load-bearing, not decoration: `globSync('/package.json', {cwd: REPO_ROOT})` returns `[]`, because a leading slash is absolute to globSync. Without the ternary the repo root reports as monitoring nothing. Proved with a two-file mutation — npm entry rewritten to `directories: ["/"]` passes on pristine code and fails with the ternary spliced out, while the splice alone changes nothing. One assertion, and it is the only one in the block that fails when the branch is removed.
829d7ae added a guard that names the build step when `lib/index.cjs` is absent. An interrupted `tsc` leaves the file present and empty, where `existsSync` is satisfied and the guard does not fire — and what the developer sees instead is five assertions reporting `expected '' to match /…/`, none of which says "run build". That is the one state where the guard's entire purpose silently fails. Not a vacuity fix. `reads the emitted entry, not the source` already goes red on an empty read — verified, 5 of 6 assertions fail — which is exactly why that assertion exists. This is so the failure names its own cure. Only the empty case. A partially written entry is not detectable by size and needs no separate handling: the content assertions cannot go quietly green on a degraded emit.
…audited `CACHE_ACTION` matched `actions/cache` and nothing else, so a third-party cache action with no `cache:` input was invisible to every rule in the file. Reproduced: a composite reached from a targeted workflow holding `useblacksmith/cache@v5` and `Swatinem/rust-cache@v2` exits 0 with `OK`. Not hypothetical — 11 jobs here run on `blacksmith-*` runners, where the first is a documented drop-in for `actions/cache`, and `packages/protect-ffi` has just made this a Cargo repo, where the second is the conventional choice. **Fixed by inverting the posture, not by lengthening a list.** Any rule that enumerates cache actions fails OPEN on the one nobody has met, in the direction that prints `OK` — which is this repo's own standing criticism of its own checks. `AUDITED_ACTIONS` is now what is permitted: four entries, which is the entire surface both targets reach (`actions/checkout`, `actions/setup-node`, `pnpm/action-setup`, `changesets/action`). Staleness is now a build failure naming the exact action, and the person adding it is the person told to audit it. The decisive case is a `setup-<tool>` action that caches BY DEFAULT: no `cache:` input, no "cache" in its name. No enumeration by name or by shape can see it; an allowlist catches it by construction. That class is already in this file as two hand-maintained entries that exist only because someone noticed. Local `uses:` stays exempt — the gate opens `./` actions and reads every step, so they are audited by construction, and reporting them would bury the real `actions/cache` finding under one about the wrapper. A name heuristic is kept, but demoted to two narrow jobs: wording a finding as "a third-party cache action" rather than "not on a list" (which invites adding it to the list), and a startup assertion that no `AUDITED_ACTIONS` entry is cache-shaped, so the one careless edit that could re-open this is impossible. Substring rather than segment-equality, verified not assumed: `Swatinem/rust-cache` has no path segment equal to `cache`. Also trims `uses:` once, so a quoted value with leading whitespace is followed into its composite the way GitHub follows it, instead of being the one unfollowable local reference that exited 0. **One existing test changed contract.** `skips uses: values that are not local paths` asserted exit 0; a non-local `uses:` is now a finding, so it is `never tries to open a uses: that is not a local path` and asserts the property directly rather than through the exit code. Same for the build-ffi-binding fixture, which now reports `jdx/mise-action` alongside its two caches — correct, since that action's own header forbids publishing workflows from using it. Both live targets still pass. test:scripts 247, eight new tests confirmed failing first.
`skills/*/SKILL.md` ships inside the `stash` tarball and is copied into customer repos, so per AGENTS.md a rule change has to reach it in the same PR. The release-workflow section still described the gate as three properties of release.yml itself, which stopped being the whole rule twice over: it now follows local composite actions and reusable workflows through the entire call tree, and every published `uses:` must be in the script's AUDITED_ACTIONS allowlist. `stash` patch changeset, since skills ship in that tarball.
`integration-protect-ffi.yml` declares `workflow_dispatch:` and then gated its only job on `github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository`. On a manual dispatch the event name is neither `push` nor `pull_request`, and the payload carries no `pull_request` object to reach through — GitHub returns null for a path through a missing key, and `==` coerces, so `null == 'owner/repo'` is `0 == NaN`, false. Both operands false: the run was created, the only job skipped, and the run reported SUCCESS having executed nothing. Worst shape a manual trigger can take, and it mattered most here — this is the credentialed suite with no other on-demand path, since its `paths:` filter means an unrelated commit will not start it either. Now stated as "not a fork PR" rather than as a list of the events allowed through. The comment above it already said that was the intent: fork PRs have no secrets, skip those, run everything else. An allowlist of event names fails shut on the event nobody enumerated — the same defect as `AUDITED_ACTIONS` in 233f3ee, inverted the other way, and it would break again on `schedule:`. **All six copies normalised, not just the broken one.** The other five are verbatim copies of a form that is wrong by construction; their only defence is that nobody has added `workflow_dispatch` to them yet, which is a fact about the calendar rather than about the code. The next fork-guarded workflow gets written by copying one of these, so five broken exemplars beside one fixed one reproduces the bug rather than retires it. The edit is meaning-preserving by proof, not by assertion: the guard evaluates every fork-guarded job across four event contexts and pins `{push: true, workflow_dispatch: true, same-repo PR: true, fork PR: false}`. The guard does not pattern-match the bad string, which would only catch the bug already fixed. It evaluates the expression — a recursive-descent reader for the subset job conditions use, with GitHub's loose-equality coercion — and THROWS on anything outside that grammar, so "the evaluator did not understand it" can never produce the same green as "the job runs". Job-level `if:` only, so step-level `always()` never reaches it; a test pins that it would throw. Three non-vacuity floors: the set of dispatchable workflows, the count of fork-guarded jobs held as equality, and an assertion that at least one condition was actually evaluated. Mutation-tested four ways, including a *different* bad shape and adding the trigger to a latent copy — verified independently that reverting the condition fails three tests. No changeset: CI-only, no published surface. No skill documents job-level `if:` or dispatchability. test:scripts 262.
…tions
`build:wasm` is `wasm-pack build … && tsc -p tsconfig.wasm-errors.json`, and
that tsconfig is `"files": ["src/errors.ts"]` emitting dist/wasm/errors.js.
`src/errors.ts` was not in the cache key.
Hashing the emitted `dist/wasm/*.d.ts` does not stand in for it. Rewriting the
body of `isProtectErrorCode` leaves errors.d.ts BYTE-IDENTICAL while errors.js
changes — verified by compiling both revisions — so the key hits, the step is
skipped, and the restored errors.js is the previous implementation. Nothing
tracked forces a miss either: errors.js is gitignored.
Not inert output. scripts/inline-wasm.mjs appends
`export { PROTECT_ERROR_CODES, isProtectErrorCode } from "./errors.js"` to the
generated entry, and package.json's `files` ships it — so the WASM integration
suites would exercise an implementation the release build does not ship, in the
one direction CI cannot notice, since a release build does not use this cache.
**The guard derives the input set rather than restating it.** It parses
`build:wasm` out of package.json, takes the `-p <tsconfig>` argument, reads that
tsconfig's `files`/`include` minus `exclude`, follows the transitive local
import graph, and asserts every file is covered by one of the wasm key's
`hashFiles` globs. So the fix stays correct when the tsconfig changes, and the
day `src/errors.ts` gains an import the test names the new file and says widen
the key.
That is why not an assertion that `src/errors.ts` has no imports: a proxy
constraint that forbids a legitimate refactor and answers "you may not", where
the graph walk states the real property and answers "then widen the key".
Mutation-tested both ways — reverting the key fails the property test, and an
added import resolving to a real file fails it naming that file.
And why not `src/**`: 104 of the 122 files under src/ are the eql-v3-types
declarations that tsconfig never reads, src/ has moved 13 times to crates/'s 3
on this branch, and each needless miss buys a wasm-pack install plus a cargo
wasm32 build of the cipherstash-client graph inside credentialed jobs. The key
must restate — `hashFiles()` takes literal globs and cannot read a tsconfig — so
the derivation lives in the test and the two are compared.
Native key checked and left alone: its `tsc` half emits `lib/`, which does not
reach `index.node` (that comes from `neon dist` reading cargo's log), there are
no build.rs or `include_str!`/`include_bytes!` sites, and `lib/` is rebuilt
unconditionally a step later. Reasoning recorded in the new describe block so
the next reader need not redo it.
No changeset: CI-only, no published surface. No skill documents this action.
test:scripts 264.
… jobs
node-pty is the repo's one entry in `pnpm.onlyBuiltDependencies`, so it is
the one package permitted to run a lifecycle script — and its script is
`node scripts/prebuild.js || node-gyp rebuild`, where prebuild.js is a bare
existsSync on `prebuilds/<platform>-<arch>`. The 1.1.0 tarball ships
darwin-arm64, darwin-x64, win32-arm64 and win32-x64, and no linux at all,
so on a Linux runner that fallback is not an edge case: every workspace
install in this repo compiles node-pty from source. npm bundles node-gyp,
pnpm does not, and pnpm/action-setup v6 no longer puts it on PATH — hence
the step the other eight installing workflows carry.
tests-rust.yml and integration-protect-ffi.yml copied the pnpm + Node
preamble without it. Both died in `Install dependencies` with
`sh: 1: node-gyp: not found` on every run since they landed — 11 and 8
runs, none of which reached cargo or vitest. Neither job uses a pty, which
is exactly why the omission read as harmless: the dependency is on the
workspace install, not on anything the job does. It is also invisible
locally, since macOS has a matching prebuild.
Adds the missing `cache: 'pnpm'` to both while there. Permitted here —
scripts/lint-no-workflow-caching.mjs covers release.yml and
tests-supply-chain.yml only.
Guarded by scripts/__tests__/workflow-node-gyp.test.mjs, because nothing
enumerated this. It flattens local composite actions in place before
checking order: four of the sixteen installing jobs reach `pnpm install`
only through ./.github/actions/integration-setup, so a scan stopping at a
workflow's own step list would report those four as violations and let a
future composite that installs without node-gyp through. Fail-closed
throughout — an unresolvable local `uses:` is a failure rather than a skip,
and an unrecognised `runs-on` (including a `${{ matrix.os }}` expression)
is treated as needing node-gyp, since only macOS and Windows runners have a
prebuild to find.
Mutation-tested both ways: reverting the workflow fix turns exactly these
two jobs red, and deleting the step from integration-setup's action.yml
while leaving its comment turns exactly the four composite-fed jobs red —
so the check requires the step, not a mention of it.
This was referenced Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Absorbs
cipherstash/protectjs-ffiinto this repo aspackages/protect-ffiplus its sixplatforms/*packages, so a change spanning the Rust core and the JS SDK is one PR instead of a coordinated release across two repos.Draft: phases 1 and 2 only. Source moves; publishing does not. The release pipeline (phase 3) is specified as nine executable tasks in the plan but not built, so the seven FFI packages are still published from the old repo.
scripts/lint-no-ffi-changeset.mjsenforces that.Plan:
docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md.What's here
0b605912is a puregit subtreeimport — 613 commits, 200 tracked files, no other change. Everything else is a reviewable commit on top:1e922ec0267ba11dplatforms/*glob, threeworkspace:*pins, sixoptionalDependenciesa0236fa476696dab29af450dpackages/stackconsumes protect-ffi 0.31.09b7983ced7128724CS_CLIENT_KEYhex guard6c1f641ac0bfe6b5AGENTS.md,SECURITY.md,skills/stash-authb99cbd92assertNativeBindingAvailable()a182644cThe import is also a 0.30 → 0.31 upgrade
0.31 is a breaking release, and adopting it needed five source changes in
packages/stack— one more than the plan costed, plus one it did not anticipate:ProtectErroris gone, replaced by anisProtectErrorCodeguard. Checking the code's value rather than the presence of acodeproperty fixes a pre-existing bug indynamodb/helpers.ts, where any string-valuedcode— anECONNRESETfrom the AWS SDK, say — was reported as an encryption error code.newClientmoved credentials intoclientOptsand renamedstrategytoauthStrategy. Credentials left at the top level now fail loudly; akeysetleft there would be silently ignored and bind the client to the default keyset. The test assertsclientOptsas a whole so one landing elsewhere is caught.as neverdeleted. 0.30 typed the wasm options asany, so the cast was load-bearing. Removing it immediately surfaced (4).encryptConfigno longer needsnormalizeCastAs— 0.31 normalises at the Rust boundary on both bindings. Verified against the 0.31 wasm build:cast_as: 'string'and'text'both reach authentication, where 0.30 rejected the former.idto every bulk payload that 0.30 silently dropped. Nothing used it; results correlate positionally.Cargo stays off the default paths
Root
pnpm testreaches this package, so cargo there would put the Rust toolchain on every contributor and every PR job for one package out of eighteen.testis now the JS chain;test:cargocollects the Rust checks. Verified with acargotrap onPATH: zero invocations from rootpnpm testandturbo build;test:cargocorrectly exits 97.src/lintWiring.test.tskeeps it that way — it exists becausecargo fmt --checkonce sat in the manifest with no caller for months, and a check that never runs reads exactly like a check that passes.Three WASM declaration files (11.3KB) are tracked so stack's declaration build resolves
@cipherstash/protect-ffi/wasm-inlinewithout Rust. Verified by deleting every.jsand.wasmand running stack's build,test:types:dist, type tests (59) and unit suite (1064) — all green.Verification
1064 stack tests, 79 protect-ffi tests (on the vitest 3.2.7 downgrade and Biome 2.5.3), 190 script tests, 18 supply-chain e2e. Packed tarball matches published 0.31.0 exactly — 12 files, 226 under
lib/,workspace:*rewritten to concrete versions — and./wasm/./wasm-inlineboth resolve from it in a scratch install.CS_CLIENT_KEYmust now be hex; the local key is (credentialed suites pass), but the repo secret is write-only, sorequire-cs-secretsasserts the charset without echoing the value across all six credentialed workflows.Known gaps, all tracked in the plan
stash doctor's encryption probe degrades once the laziness change ships — it relies onawait import()forcing binary resolution.assertNativeBindingAvailable()is exported but unconsumed, because stack must not consume an API absent from published 0.31.0. Phase 5 wires it.@cipherstash/stackdepends on protect-ffi atworkspace:*, so FFI releases patch-bump the whole Stack fixed group. That is deliberate — an exact pin is how a wrapper/binary mismatch is made impossible — but it is not what an earlier draft claimed.Needs a decision
The Stack changeset is
minor. A 1.0 package where a previously-working credential encoding stops working argues formajor; against it, hex was always the documented encoding, and the fixed group would takestash,wizardand three adapters to 2.0.0.