feat(protect-ffi): absorb protectjs-ffi into the monorepo (phases 1–2) - #858
Draft
tobyhede wants to merge 577 commits into
Draft
feat(protect-ffi): absorb protectjs-ffi into the monorepo (phases 1–2)#858tobyhede wants to merge 577 commits into
tobyhede wants to merge 577 commits into
Conversation
Bump cipherstash-client, cts-common, and stack-profile from 0.34.1-alpha.2 to 0.34.1-alpha.4 and adapt protect-ffi to the breaking API changes: - ColumnType::Utf8Str -> ColumnType::Text, ColumnType::JsonB -> ColumnType::Json - Plaintext::Utf8Str -> Plaintext::Text, Plaintext::JsonB -> Plaintext::Json - EqlEncryptOpts gained a required decryption_policy field - IndexType gained an Ope variant; handle it alongside Ore in the index-name helpers
…ent-0.34.1-alpha.4 chore(deps): upgrade cipherstash-client to 0.34.1-alpha.4
Bumps cipherstash-client, cts-common, and stack-profile from 0.34.1-alpha.4 to 0.34.1-alpha.5 to pick up shared schema and config types ahead of the CanonicalEncryptionConfig migration. `IndexType::SteVec` gained a required `mode: SteVecMode` field in this release; the FFI sets it to `SteVecMode::default()` (`Compat` at alpha.5) so existing behaviour is preserved. No public TypeScript API change.
Surfaces the `mode` option on SteVec indexes through the config API, letting callers choose between `compat` and `standard` encoding. Previously the FFI hard-coded `SteVecMode::default()`, giving callers no way to opt in to the newer encoding. How: - Add the `SteVecMode` TypeScript type (`'compat' | 'standard'`). - Add a `mode` field to the Rust `SteVecIndexOpts` struct with `#[serde(default)]` so configs that omit it continue to parse. - Thread the parsed value through to `IndexType::SteVec` instead of always falling back to `SteVecMode::default()`. - Document the option in the JSONB API reference. Backwards compatibility: configs that omit `mode` keep using the upstream library default, which at this commit (cipherstash-client alpha.5) is still `Compat`. The follow-up alpha.7 bump flips the upstream default to `Standard` — see the CanonicalEncryptionConfig refactor commit for the user-visible breaking change that introduces.
Adds `.worktrees/` to .gitignore so scratch worktrees created under the repo root don't show up as untracked. Personal workflow convention; not used by CI or other contributors.
Bumps cipherstash-client, cipherstash-config, cipherstash-core, cts-common, stack-auth, stack-profile, and zerokms-protocol from alpha.5 to alpha.7. Required so the FFI can deserialize encrypt configs directly into `CanonicalEncryptionConfig` and reuse upstream validation (config version check, ste_vec/match plaintext-type rules). BREAKING (latent at this commit, becomes user-visible once the follow-up CanonicalEncryptionConfig refactor swaps the FFI types over): - `SteVecMode::default()` flips from `Compat` to `Standard` at the library level. Because the FFI currently parses `mode` through its own struct that falls back to `SteVecMode::default()` when the field is omitted, any ste_vec index that omits `mode` will now resolve to `Standard` instead of `Compat`. The two encodings are NOT cross-compatible — stored data indexed under `Compat` cannot be queried under `Standard`. Callers that need to preserve the previous behaviour must pin `mode: 'compat'` explicitly. The migration docs commit later in this branch records the full set of breaking changes and the caller-facing migration recipes.
Adds a TypeScript translation layer that converts the public, JS-friendly EncryptConfig vocabulary into the canonical vocabulary that cipherstash-config's CanonicalEncryptionConfig expects. Not yet wired into `newClient`; the wiring lands in the follow-up "normalize encrypt config vocabulary at the FFI boundary" commit. Why translate in TypeScript rather than rename in Rust: - Keeps the public TypeScript API stable for existing callers (`cast_as: 'string' | 'number' | 'bigint' | ...`). - Lets the native config adopt upstream's canonical names (`text`, `float`, `big_int`) without leaking them into the JS interface. - Future vocabulary tweaks can ship as TS-only changes without another Rust release. How: - Remap `cast_as` values that have no canonical equivalent: `string` → `text`, `number` → `float`, `bigint` → `big_int`. All other values pass through unchanged. - Inject `array_index_mode: 'none'` on any `ste_vec` index that omits the field. The upstream library defaults to `'all'`, so without this we would silently change array-indexing behaviour for existing configs (see the migration design doc earlier in this branch). - Leave `mode` untouched. Omitted `mode` follows the upstream default (`Standard` at alpha.7) — this is a documented breaking change, surfaced once the wiring lands. - Never mutate the caller's config object; build a fresh `NativeEncryptConfig` and return it. Includes unit tests covering each remapped value, the `array_index_mode` default injection, pass-through of canonical values, immutability of the input, and the no-op cases.
…Config
Removes the 688-line `encrypt_config.rs` module and deserializes
the encrypt config directly into cipherstash-config's
`CanonicalEncryptionConfig`. Eliminates duplicate type definitions
across the FFI and the shared schema crate, picks up upstream
validation (version check, ste_vec/match plaintext-type rules)
for free, and lets future schema changes propagate without an FFI
patch.
How:
- Drop `mod encrypt_config` and import `CanonicalEncryptionConfig`
and `Identifier` from `cipherstash_client::schema` directly.
- Change `NewClientOptions::encrypt_config` to
`CanonicalEncryptionConfig`.
- Replace the bespoke `Error::SteVecRequiresJsonCastAs` and
`Error::Config(String)` variants with
`Error::Config(#[from] ConfigError)`, surfacing the upstream
error verbatim.
- Rename the old `Error::Config(String)` to
`Error::Credentials(String)` to reflect its actual usage
(only emitted from SecretKey hex parsing). Note: the
`#[error("Configuration error: {0}")]` display template is
left unchanged for this variant — a follow-up to tighten the
wording is captured in the migration doc.
BREAKING CHANGES (visible to TS callers — see
docs/canonical-encryption-config-migration.md for migration
recipes):
1. SteVec `mode` default: `Compat` → `Standard`. Any ste_vec
config that omits `mode` now indexes new writes under
`Standard` encoding. The two encodings are NOT
cross-compatible: data indexed under `Compat` cannot be
queried under `Standard`, and vice versa. Pin `mode: 'compat'`
explicitly to preserve the pre-migration behaviour for stored
data, or plan a re-encryption of affected columns.
2. `match` index now requires a text-family `cast_as` (`'text'`
or `'string'`). Previously unvalidated; now fails at
`newClient` (mapped to `MATCH_REQUIRES_TEXT` by the follow-up
FFI-boundary wiring commit).
3. Config `v` must equal `1`. Previously unchecked; other values
now fail at `newClient` (mapped to `UNSUPPORTED_CONFIG_VERSION`
by the follow-up FFI-boundary wiring commit).
4. Config-validation error message text now comes from upstream
`ConfigError` and is worded differently. `ProtectError.code`
values are preserved, so consumers branching on `code` are
unaffected; consumers string-matching on `err.message` for
config-validation errors must update.
BREAKING CHANGE: ste_vec indexes that omit `mode` now use
`Standard` encoding instead of `Compat`. Pin `mode: 'compat'` or
plan re-encryption of stored data.
BREAKING CHANGE: `match` indexes now require a text-family
`cast_as` (`'text'` or `'string'`); previously unvalidated configs
will fail at `newClient`.
BREAKING CHANGE: encrypt config `v` must equal `1`; other values
fail at `newClient` instead of being silently accepted.
BREAKING CHANGE: config-validation error message wording changed
(error codes preserved); consumers string-matching on
`err.message` must update.
Wires the `normalizeEncryptConfig` helper (added earlier in this
branch) into the public `newClient` entry point so callers keep
using the JS-friendly `EncryptConfig` vocabulary while the native
side receives the canonical `CanonicalEncryptionConfig` shape
produced by the preceding refactor.
How:
- Pipe `opts.encryptConfig` through `normalizeEncryptConfig`
before passing it to `native.newClient`.
- Introduce an internal `NativeNewClientOptions` type so the
native module declaration reflects the post-normalization shape
(`NativeEncryptConfig`) without leaking it from the public API.
Error-code mapping:
- `inferErrorCode` now recognises three message fragments produced
by upstream `ConfigError`:
- `'requires plaintext_type: json'` → `STE_VEC_REQUIRES_JSON_CAST_AS`
(existing code; substring updated to match the new wording).
- `'requires plaintext_type: text'` → `MATCH_REQUIRES_TEXT`
(new code).
- `'unsupported config version'` → `UNSUPPORTED_CONFIG_VERSION`
(new code).
- Adds `MATCH_REQUIRES_TEXT` and `UNSUPPORTED_CONFIG_VERSION` to
the exported `ProtectErrorCode` union.
BREAKING:
- `ProtectError` messages for config-validation errors are now
worded as `ConfigError` emits them. Consumers branching on
`ProtectError.code` are unaffected; consumers string-matching
on `err.message` must update their match strings.
- The exported `ProtectErrorCode` union gains two new values
(`MATCH_REQUIRES_TEXT`, `UNSUPPORTED_CONFIG_VERSION`).
Exhaustive switches over `ProtectErrorCode` will need
additional cases to stay exhaustive (TS will flag missing
cases when `--strict` is on).
BREAKING CHANGE: two new `ProtectErrorCode` values exist
(`MATCH_REQUIRES_TEXT`, `UNSUPPORTED_CONFIG_VERSION`); exhaustive
switches over the union need additional cases.
BREAKING CHANGE: config-validation error message text is now
sourced from upstream `ConfigError`; consumers string-matching on
`err.message` must update.
Adds `docs/canonical-encryption-config-migration.md` describing the four breaking changes that ship with the CanonicalEncryptionConfig migration, with migration recipes for each: 1. SteVec `mode` default: `Compat` → `Standard` (most impactful; existing data must be re-encrypted or `mode: 'compat'` pinned). Calls out explicitly that the two encodings are not cross-compatible. 2. `match` index now requires text-family `cast_as`. 3. Config `v` must equal `1`. 4. ConfigError message text differs from the old hand-rolled error wording. `ProtectError.code` values are preserved. Also expands `docs/jsonb-api-reference.md` with: - The full `cast_as` vocabulary table showing the public ↔ canonical mapping (so callers can debug error messages that reference canonical names). - Validation rules for `v`, `ste_vec`, and `match` together with the error codes they emit. - A SteVec `mode` section warning that re-encryption is required when changing modes. - The two new `ProtectErrorCode` values (`MATCH_REQUIRES_TEXT`, `UNSUPPORTED_CONFIG_VERSION`). Includes a "explicitly not changed" section calling out that `array_index_mode` still defaults to `'none'` at the FFI boundary (the TS helper injects it), and a "follow-ups" section recording two non-blocking polish items (unit tests for `inferErrorCode` and the `Error::Credentials` display template). Documentation only.
Fixes two doc-drift issues caught after the migration docs landed: - The JSDoc on `SteVecMode` (src/index.cts) still claimed `compat` was the default. After the alpha.7 bump the runtime default is `standard`. Update the comment to match runtime behaviour and add a hint that callers should pin `compat` explicitly to preserve pre-alpha.7 encoding for stored data. - The `cast_as` union snippet in docs/jsonb-api-reference.md was missing `'text'` and `'timestamp'` (added by the migration). Update the snippet to mirror the public `CastAs` union exported from src/index.cts. Documentation only.
Adds an integration test exercising newClient with legacy cast_as values plus an ste_vec config without mode, and three negative cases asserting the ProtectError codes MATCH_REQUIRES_TEXT, UNSUPPORTED_CONFIG_VERSION, and STE_VEC_REQUIRES_JSON_CAST_AS — replacing the manual verification steps from the migration PR.
The cipherstash-client 0.34.1-alpha.7 default flipped SteVec encoding from `Compat` to `Standard`, which collapses the old `b3`/`ocf`/`ocv` SteVec entry fields into two: - Scalar strings and numbers share a single orderable field `oc` (CLLW ORE with tagged-plaintext domain separation). - Booleans, null, arrays, and objects produce an `hm` HMAC-SHA256. Update the integration tests to assert against `oc`/`hm` instead of the retired `b3`/`ocf`/`ocv` names. Restructure the `unique index field (b3)` block as `HMAC index field (hm)` and exercise the non-orderable types (root object, booleans) that actually produce HMAC entries under Standard mode.
The cipherstash-client 0.34.1-alpha.7 default flipped SteVec encoding from `Compat` to `Standard`, collapsing the old `b3`/`ocf`/`ocv` fields. Update the JSONB API reference to match the runtime: - Replace `b3`/`ocf`/`ocv` in the EqlCiphertext / EqlCiphertextBody type snippets with the current fields (`oc` for Standard SteVec, `op` for Compat SteVec, `opf`/`opv` for non-SteVec OPE indexes). - Note that `hm` now also covers SteVec MAC entries (objects, arrays, booleans, null), not just the standalone `unique` index. - Add a small table summarising which entry field each JSON value type produces, plus a mention of the Compat-mode `op` variant. - Update the storage and term-query example outputs accordingly. Documentation only.
Root-level `hm` is HMAC-SHA256 for unique (exact) indexes; SteVec MAC entries live under `sv`, not at the root.
Collapses three near-identical remap tests and the canonical-values loop into table-driven `it.each` blocks.
…ipherstash-client refactor: use OPE & consistent config types from `cipherstash-client`
Adopts the EQL v2.3 storage schema: EqlCiphertext is now a discriminated
union keyed on `k` ("ct" for scalar payloads, "sv" for SteVec), and
encrypt_eql returns a Vec<EqlOutput> separating storage ciphertexts from
query payloads. Storage SteVec payloads now place the root ciphertext at
`sv[0].c` rather than at the root.
`encrypt` / `encrypt_bulk` unwrap `EqlOutput::Store` via a new
`into_store_ciphertext` helper (they always run with `EqlOperation::Store`).
`encrypt_query` / `encrypt_query_bulk` return `EqlOutput` directly —
upstream now derives `Serialize` / `Deserialize` and is `#[serde(untagged)]`,
so the wire shape is determined by the inner `EqlCiphertext` / `EqlQueryPayload`
`k` tag.
Updates the FFI call sites, the TS Encrypted type, integration tests, and
the JSONB API reference to match the new wire format.
cipherstash-client 0.35.0 moved the SteVec storage root ciphertext from the top-level `c` to `sv[0].c`. Update the three remaining assertions in json-containment.test.ts (and the structural-comparison log lines) so they match the new wire format.
…client refactor: upgrade cipherstash-client to 0.35.0
…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.
The two gaps this closes are the same gap: a check nothing invokes reads exactly like a check that passes. `mise run lint:rust` is now an aggregate over three arms — clippy for the host, clippy for wasm32, and `cargo fmt --check`. It keeps the name CI already called, so the step gets strictly more coverage without a rename. The wasm arm lints the lib only; the unit tests are host-run. `npm test` now reaches `test:format:rust`, which has sat in package.json with no caller. That also makes the README's claim about `npm test` true again — it said it formatted and linted Rust, and it did neither. Drops the `cargo check --target wasm32-unknown-unknown` step from test.yml: clippy checks as it lints, so it was doing that work twice. `src/lintWiring.test.ts` guards the call graph rather than the checks. Its general form — no `test:*` script unreachable from `npm test`, no `lint:rust:*` task the aggregate skips — is what catches the next orphan, not just this one. Exemptions have to name a reason. Verified it fails on each regression it claims to catch: re-orphaning `test:format:rust`, dropping the wasm arm from `depends`, and CI calling clippy directly instead of the entry point. No changelog entry: nothing here changes the published surface. Claude-Session: https://claude.ai/code/session_01YJekcEBAsUg8qJoBcqyzBx
`Error` is a 14-variant enum, several carrying structured fields. All of it was discarded at the FFI boundary: Neon exports returned `extract::Error`, whose `TryIntoJs` is `cx.error(cause.to_string())`, and `wasm.rs` did the same via `js_error(&e.to_string())`. Only the message crossed. Each variant that JS can act on now carries `#[diagnostic(code(..))]`, and both boundaries read it onto `err.code`. Values are unchanged, so this half is additive on its own — the JS side that stops inferring them is the next commit. Notes on the shape, since two parts of the issue's proposal did not survive contact: - `#[diagnostic(transparent)]` on the `#[error(transparent)]` variants buys nothing. cipherstash-client, stack-auth, cipherstash-config and eql-bindings contain zero `#[diagnostic(code(..))]` and no manual `code` impls between them — they use `Diagnostic` for `help()` text only — so inheriting would inherit `None`. Six of the eleven wrapped types do not implement `Diagnostic` at all. The codes are therefore ours, which also settles the issue's worry about coupling to upstream naming: there is nothing to couple to. - `Error::Config` is split into four variants. Three published codes (`STE_VEC_REQUIRES_JSON_CAST_AS`, `MATCH_REQUIRES_TEXT`, `UNSUPPORTED_CONFIG_VERSION`) are sub-variants of one upstream `ConfigError`, and the derive cannot compute a code from inner state. `From<ConfigError> for Error` routes them by variant, so an upstream rename is a compile error where the substring match it replaces would have silently degraded to `UNKNOWN`. `#[error(transparent)]` on all four keeps the message identical. The Neon exports had to move their bodies into `do_*` helpers returning `Result<_, Error>`, mirroring what `wasm.rs` already does. `TryIntoJs` is sealed behind a private module, so no type declared here can implement it, and `extract::with` — which defers conversion until the JS thread and hands it a `Cx` — is the only hook for setting a property on the thrown error. That opaque return type is not something `?` can convert into, hence the split. Two things the wasm entry gains beyond the code itself: - `newClient` routed `into_config_map`, `ZeroKMSBuilder::build` and `ScopedCipher::init` through `js_error` rather than `error_to_js`. The divergence was invisible while no code was being carried; it meant the three config codes arrived bare on this entry. - `WasmDecryptResult` is gone. It existed only to describe the missing `code` — the field was synthesised by the Neon JS wrapper, which this build has no equivalent of. Both entries now name one `DecryptResult`. `UNKNOWN_QUERY_OP` is the one code that could not be derived from the variant the error was built as. #143 moved `queryOp` parsing into `query_op.rs`, where an unknown value is rejected inside `Deserialize` — which is what makes the failure name the field rather than surfacing later from query preparation — and serde's `de::Error::custom` takes a `Display`, so nothing typed reaches the boundary. `Error::unknown_query_op` recovers it from the message prefix, and `From<serde_json::Error>` / `wasm::from_js_value` route both entries through it. That is the same prefix match `src/errors.ts` was doing, moved rather than removed, and worth being explicit about in a commit whose point is that codes stop coming from prose. What moving it buys: it sits beside `UNKNOWN_QUERY_OP_PREFIX`, the constant that defines the message, in the same crate and the same review diff, and the prefix is pinned from both sides — `query_op`'s `an_unknown_value_keeps_the_prefix_the_error_routing_matches` and `error_codes::an_unknown_query_op_is_routed_off_the_serde_message`. A change that breaks the mapping fails `cargo test` instead of silently degrading a caller's `code` to `UNKNOWN`. `other_deserialization_failures_stay_uncoded` pins the other side, since a prefix match that over-captured would be worse than none. Rebase note: this series was written against a tree where `UnknownQueryOp` was still a plain `Error` variant, and #143 landed on main in between. `integration-tests/tests/wasm-error-codes.test.ts` covers the wasm entry. It needs no credentials, unlike the round-trip suite (#149) — every case is config validation, which fails before any network I/O. Claude-Session: https://claude.ai/code/session_01BYfRbVEWtYXMG5SPBNdX6m
`inferErrorCode` is deleted. It matched the message against fourteen
prefixes and substrings to recover what Rust had just thrown away — the
same process serialising structure to prose and then parsing the prose
back.
It worked, and it was fragile in a way nothing tested. Three of those
patterns matched wording owned by cipherstash-config, not this repo:
if (message.includes('requires plaintext_type: json'))
return 'STE_VEC_REQUIRES_JSON_CAST_AS'
if (message.includes('unsupported config version'))
return 'UNSUPPORTED_CONFIG_VERSION'
An upstream reword would silently downgrade a caller's error to
`UNKNOWN` — the call still fails, just less usefully, and nothing here
would have failed to say so. Three of fourteen understates it, because
the table gave no way to tell which three: `' index configured'` reads
exactly like an upstream phrase and is this repo's own `MissingIndex`.
`docs/canonical-encryption-config-migration.md` had already flagged the
gap as a follow-up, proposing tests for the substrings. This closes it by
removing them instead.
`normalizeError` reads `err.code` and validates it against the declared
set. Validation is the point: Node puts a `code` on its own errors, so a
bare structural read would let an `ECONNRESET` through as a
`ProtectErrorCode`. `isProtectErrorCode` exports that check, for callers
who cannot rely on `instanceof ProtectError` — the wasm entry has no JS
wrapper to construct one.
`PROTECT_ERROR_CODES` is now the single declaration, with the union
derived from it, and `errorCodes.test.ts` reads the Rust attributes and
proves the two sets agree. That is the one remaining way for this to go
wrong, and it is silent: a code TypeScript does not declare still arrives
at runtime and still fails the predicate.
BREAKING CHANGE: a failed `decryptBulkFallible` item with no code omits
`code` rather than setting `'UNKNOWN'`. The declared type has always been
`code?: ProtectErrorCode`, but the field was in practice always present on
the Neon entry, because the wrapper stored whatever the inference returned.
Test for absence instead.
The api reference stopped restating the union — its copy was already
missing `SHORT_MATCH_NEEDLE`, which is the argument.
Claude-Session: https://claude.ai/code/session_01YJekcEBAsUg8qJoBcqyzBx
The previous commit stopped `src/errors.ts` inferring a code from the message, but left the layer that existed to carry it: every export ran through `wrapAsync`/`wrapSync`, which caught each failure and re-threw it as a `ProtectError`. Once Rust sets `code` on the error it builds, that layer adds nothing. It was not free. It made the two bindings throw different things — wasm has no JS wrapper, so its errors stayed plain. It re-based the stack trace onto the wrapper, demoting the real one to `cause`. And the check it existed to provide, `instanceof`, is false across duplicate copies of a package, which is the failure the issue's payoff section already called out. So the exports return what the binding threw. `newClient` on a bad config now produces a byte-identical error object on Neon and wasm, which is what this PR series has been converging on since #142. No replacement guard is shipped, and the `isProtectError` I first reached for is not there. Narrowing is not a neon limitation — TypeScript types every `catch` variable as `unknown` (TS18046), so a caller narrows once no matter what Rust throws, and neon has no class API, so Rust could not throw an `instanceof`-able class unless JS handed it one. Branching needs nothing from this package: if (err instanceof Error && 'code' in err && err.code === 'MISSING_INDEX') and `isProtectErrorCode`, already exported for validating a code value, narrows `err.code` to a typed `ProtectErrorCode` for callers that want to store it. A second predicate would have been API replacing API. Every export that returns a promise is now `async`, and that keyword is load-bearing rather than stylistic: neon extracts arguments SYNCHRONOUSLY. A bad client handle, an options object serde rejects (every unknown-key rejection from #144), or an out-of-range bigint threw from the call itself, and `wrapAsync` was quietly converting those into rejections. Verified against the built addon that all four still reject rather than throw, and that an out-of-range bigint is still a `RangeError` — which the README and the `JsPlaintext` JSDoc promise, and which previously survived only because the inference table happened not to match its message. BREAKING CHANGE: `ProtectError` is no longer exported and nothing throws it. Both entries throw an ordinary `Error` with a `code` property. Replace `err instanceof ProtectError && err.code === X` with `err instanceof Error && 'code' in err && err.code === X`, or use `isProtectErrorCode` where the code is wanted as a typed value. Claude-Session: https://claude.ai/code/session_01YJekcEBAsUg8qJoBcqyzBx
ci: lint the wasm32 target, and give the Rust checks one entry point
feat!: derive error codes in Rust instead of string-matching Display output (#146)
Follows the pure subtree import. Disposition per the absorption plan: - Delete `package-lock.json` — this is a pnpm workspace. - Delete `CODE_OF_CONDUCT.md` — byte-identical to the root copy. - Delete the nested `biome.json` (schema 1.8.3) and inherit root Biome 2.5.3. The formatter and javascript blocks were already identical; what the nested config carried that root does not is three ignores, added to root `files.includes` as negations scoped to this package. They are scoped rather than repo-wide because `lib/` is a real source directory elsewhere (`packages/cli/src/commands/*/lib/`), so a bare `!**/lib` would silently drop CLI source from linting. - Apply Biome 2.5.3 across the package: import organisation (`organizeImports` is an assist action in 2.x) and four genuinely-unused type imports. The public surface is unchanged — `CredentialOpts` and `EncryptedV3` were both imported *and* separately re-exported, so only the redundant import went. Kept deliberately: - `LICENSE.md`. The plan said dedupe it against root, but the published 0.31.0 tarball contains it (npm auto-includes LICENSE regardless of `files`), so deleting it would strip the license from a published artifact. No other package here carries one; this one has a reason to. - `.github/` (workflows, composite action, `.env`). Inert — GitHub does not read workflows from a subdirectory, and every workflow-scanning script in `scripts/` is rooted at `REPO_ROOT/.github/workflows`. Kept as the reference for the phase-3 pipeline port, and deleted there, not left behind. - `mise.toml` — pins the Rust toolchain and is itself part of the Rust path filter. - `Cargo.toml` / `Cargo.lock` / `crates/`, the nested `.gitignore`, `tsconfig*.json`, `vitest.config.ts`. Not carried: upstream's untracked `integration-tests/tests/ json-array-docs-validation.test.ts`. A subtree imports tracked history only, and that file is uncommitted work in progress upstream.
Source moves; publishing does not (the seven packages go on the changesets ignore list in the next commit). - `pnpm-workspace.yaml` gains `packages/protect-ffi/platforms/*`. The existing `packages/*` glob already covers the wrapper; the platform packages sit a level deeper. - All three exact `0.30.0` pins become `workspace:*` — stack, stack-drizzle and stack-supabase. Leaving any one on npm would build and test that adapter against the published FFI instead of the absorbed source. - `minimumReleaseAgeExclude` drops its two protect-ffi entries and the two comment lines describing them: a workspace link has no release age. - `.github/dependabot.yml` drops its `@cipherstash/protect-ffi` ignore and the comment explaining the lockstep pin. Dependabot does not update workspace protocol deps, so the entry is dead — and the hazard it guarded against (a grouped bump skewing the three consumers) cannot occur once they share one workspace version. The wrapper also gains its six `optionalDependencies` at `workspace:*`. These were NOT tracked upstream — `neon update` injects them during `prepack`, which is how published 0.31.0 has them and the repo does not. That matters because the next commit drops `neon update` from `prepack` (it overwrites the versions Changesets writes); without the deps tracked, dropping it would publish a wrapper with no platform dependencies at all and the binding would never install. Tracking them at `workspace:*` gives the fixed group something to bump and pack-time rewriting something to resolve. Verified: `pnpm install` links all seven; stack resolves the wrapper through the symlink; the five non-host platform packages warn `Unsupported platform` rather than failing, which is the documented behaviour for linked (not installed) packages. `scripts/` self-tests (180), all four repo linters, and `e2e/tests/supply-chain.e2e.test.ts` (18) pass.
Two independent fixed groups, as planned: Stack's six packages keep theirs,
and the seven protect-ffi packages get their own. The wrapper and its platform
binaries must move in lockstep — a binary published at a version its wrapper
does not expect is unloadable — but a Rust patch has no business republishing
the CLI, the core library, the wizard and three adapters. A single shared
group would also have jumped protect-ffi 0.31.0 -> 1.0.1 on its next patch,
since a fixed group adopts its highest member version.
The plan's publication guard does not survive contact with changesets.
`ignore` was to hold the seven packages back until trusted publishing is
repointed; changesets rejects the config outright:
The package "@cipherstash/stack" depends on the skipped package
"@cipherstash/protect-ffi", but "@cipherstash/stack" is not being
skipped. Please add "@cipherstash/stack" to the `ignore` option.
An ignored package's dependents must be ignored too, and all three consumers
now depend on the wrapper at `workspace:*`. Following that through reaches
`@cipherstash/stack`, `stack-drizzle` and `stack-supabase`, then — via the
Stack fixed group — `stash`, `stack-prisma` and `wizard`. That is a total
release freeze, the alternative the plan considered and rejected because this
repo releases actively and the freeze has no known end date.
So the guard is a check instead, and it turns out to need less than `ignore`
did. All seven packages are already on npm at 0.31.0, the exact version the
workspace carries, and `changeset publish` only publishes versions absent from
the registry. A release is therefore already a no-op for all seven. The one
thing that would change that is a changeset naming one of them — which bumps
all seven to 0.32.0 through the fixed group and sends a publish at an npm
trusted publisher that still names cipherstash/protectjs-ffi.
`scripts/lint-no-ffi-changeset.mjs` is exactly that check, wired into tests.yml
beside the other lint gates.
It is narrower than `ignore` in the way that matters: protect-ffi source can
change freely during the window, and its changeset waits for the cutover PR —
which is where the phase-2 laziness changeset now lands too, rather than
sitting pending under an ignore list that cannot exist.
Consequences for the plan, both handled at cutover: phase 2 must not add its
own changeset, and phase 4 step 1 deletes this script, its self-test, its
fixtures and the `lint:ffi-changeset` entry instead of removing ignore entries.
The self-test covers the shape a Stack changeset actually takes — the 0.31
adoption note names protect-ffi in its prose and quotes frontmatter to show a
shape, and must not trip the guard — and asserts the guarded list matches the
fixed group so the two cannot drift.
**Keeping cargo off the default test path.** `test` was `test:typecheck && test:unit && test:lint && test:format && test:rust`, where `test:format` itself expanded to `test:format:ts && test:format:rust` — two cargo processes. Root `pnpm test` runs `turbo test --filter './packages/*'`, which reaches this package's `test`, so that put the Rust toolchain on every contributor's machine and every PR job for one package out of eighteen. The default is now the JS chain only, and a new `test:cargo` collects `test:rust` and `test:format:rust` for the path-filtered Rust job phase 3 adds. `src/lintWiring.test.ts` is what makes the split safe. It exists because `cargo fmt --check` once sat in package.json with no caller for months — a check that never runs reads exactly like a check that passes (#145). Splitting one root into two is the obvious way to recreate that, so the test now takes both roots and asserts three things: nothing under `test:*` is unreachable from either; nothing cargo is reachable from `test`; and every cargo check IS reachable from `test:cargo`. **`build` cannot be a no-op.** `lib/` is the package `main` and is untracked (upstream ships it in the tarball, so npm consumers never notice; workspace consumers resolve an empty package). So `build` is `tsc` — 226 files, matching the published tarball exactly — and cargo moves to `build:native`. `turbo.json` gains a `@cipherstash/protect-ffi#build` override for the same reason. The repo-wide task declares `outputs: ["dist/**"]`; this package emits `lib/**`. Left alone, Turbo would cache an empty output set and a cache HIT would restore nothing while reporting success — leaving `lib/` absent and the package unresolvable, intermittently, depending on cache state. **`pnpm run x -- --release` does not survive the move off npm.** npm strips the `--` separator; pnpm forwards it verbatim. Since the cargo scripts end in `> cargo.log`, the forwarded args land after the redirect, so pnpm produced cargo build --message-format=… > cargo.log -- --release and cargo rejected `--release` as a positional argument. Dropping the separator fixes it, and lintWiring now asserts no script reuses the npm spelling — the release matrix passes `--target "${CARGO_BUILD_TARGET}.2.28"` this way to pin glibc, so phase 3's port depends on it. **Publishing hooks.** `neon update` leaves `prepack`: Changesets writes real versions into the six platform `optionalDependencies` and `neon update` regenerates them, which is the single most likely cause of a silently-wrong publish. The `version` script goes entirely — `neon bump` is coupled to the npm version lifecycle Changesets does not use, and has an arg-parsing bug (`options[0]` where it means `options._unknown[0]`, yielding `npm version --force undefined`). `scripts/changelog-release.mjs` was its only other caller and is deleted with it; Changesets owns `CHANGELOG.md` now, so the file's `[Unreleased]` preamble and the README's `workflow_dispatch` release instructions are rewritten rather than left describing a process that no longer exists. **Toolchain.** Biome 1.9.4 -> ^2.5.3, `@tsconfig/node20` -> node22, `@types/node` ^20 -> ^22.20.1 (matching `packages/bench` and the root), typescript and vitest onto `catalog:repo` — vitest ^4.1.0 -> 3.2.7 is a downgrade, and all 73 tests pass on it. The npm-only `overrides: { vite }` is dropped: pnpm ignores `overrides` outside the workspace root, so it never took effect here, and root already carries vite in `catalog:security`. `mise.toml` loses `node = "latest"`, which resolved to 25.x and meant entering this directory silently switched Node relative to the rest of the monorepo — whose lockfile and `@types/node` are resolved for 22. The Rust pins stay; that is what the file is for. Verified: 73 tests pass; `turbo test`/`turbo build --dry` show cargo-free commands and `outputs: ['lib/**']`; `build:native` compiles and the binding loads and dispatches into Rust (`newClient({})` fails in serde, not in the loader). One note for phase 3: bare `neon dist` writes `./index.node` — the `debug:` fallback in `load.cts` — and needs `-o platforms/<p>/index.node` to populate a platform package.
The import is also an upgrade: this repo pinned 0.30.0 and the absorbed source is 0.31.0, a release with a `Breaking` heading. Four incompatibilities, one more than the plan costed. **1. The `ProtectError` class is gone**, replaced by an `isProtectErrorCode` guard. Both bindings now throw an ordinary `Error` with `code` set by Rust, so there is no class to match — and `instanceof` was unreliable regardless, being false across duplicate copies of a package. Every type-only import is unaffected; the two value sites move to a value check. That check is on the code's VALUE, not the presence of a `code` property, which fixes a pre-existing bug in `dynamodb/helpers.ts`. Its fallback branch accepted any string-valued `code` and asserted it into `ProtectErrorCode`, so a Node error — `ECONNRESET` from the DynamoDB client, say — was reported as an encryption error code. The two branches collapse into one correct one. **2. The wasm `newClient` moved credentials into `clientOpts`** and renamed `strategy` to `authStrategy`. Credentials left at the top level are now rejected outright, so that half fails loudly; a `keyset` left there would be silently ignored and bind the client to the DEFAULT keyset, encrypting under the wrong keys. This config forwards no keyset, and the test now asserts `clientOpts` as a whole so one landing elsewhere is caught. The `as never` is deleted. 0.30 typed the wasm options as `any`, so the cast was load-bearing; 0.31 types them properly. Removing it immediately surfaced `encryptConfig`, below — which is the argument for removing it. **3. `encryptConfig` no longer needs normalising.** 0.30's wasm binding accepted EQL-native `cast_as` only, so the factory ran `normalizeCastAs` first. 0.31 normalizes at the Rust deserialization boundary on both bindings and types the result as `CanonicalEncryptConfig`, documented as a shape nothing asks you to build — so it is not assignable to the `EncryptConfig` `newClient` declares, and keeping the call would need an assertion that misdescribes the value. Verified against the 0.31 wasm build: `cast_as: 'string'` and `'text'` both get past config parsing to authentication, where 0.30 rejected the former with ``unknown variant `string` ``. `normalizeCastAs` is deprecated rather than deleted — it is the only exhaustive consumer of `toEqlCastAs`, and removing both is a deliberate cleanup, not a side effect of a dependency bump. **4. Unknown payload keys now reach Rust and are rejected.** The plan listed this as "lower-risk but worth knowing"; it broke 61 tests. Stack attaches a correlation `id` to every bulk encrypt/decrypt payload, and protect-ffi's `EncryptPayload` / `BulkDecryptPayload` have never declared one — 0.30 dropped it silently, 0.31 fails the whole call with ``unknown field `id` ``. Nothing was using it: results are correlated positionally, by `keyMap` index in the model helpers and against the original array in `mapEncryptedDataToResult` / `mapDecryptedDataToResult`. The id is stripped at the FFI boundary and stays on stack's own side of it. Verified: 65 test files, 1064 tests pass, including the credentialed suites against the live service — which also settles the open question about `CS_CLIENT_KEY`. 0.31 decodes an explicit `clientKey` hex-only, and a base64 value would have failed every credentialed test at client construction; they pass, so the key in use is hex. Build, `test:types` (59) and `test:types:dist` all clean.
Settles the plan's last provisional finding: ordinary CI needs no Rust. The problem. `dist/wasm/**` is generated by wasm-pack + `inline-wasm.mjs`, needs Rust, the wasm32 target and wasm-pack, and was untracked upstream — invisible there because npm consumers install a packed tarball that contains it. Workspace consumers get the source tree instead, and `packages/stack/src/wasm-inline.ts` value-imports `@cipherstash/protect-ffi/wasm-inline`, so with the directory absent stack's declaration build cannot resolve the module. That would put the Rust toolchain on every contributor and every PR job in order to typecheck. Option (3) from the plan, now verified rather than assumed. Export maps resolve `types` and `default` independently, so the declarations alone are enough. Deleted every `.js` and `.wasm` under `dist/wasm`, leaving only these three, and ran: stack `build` (tsup + dts) clean, `test:types:dist` clean, `test:types` 59 passed, unit suite 1064 passed. `dist/wasm/package.json` was also removed during the test and the build still passed, so it stays untracked — it matters at runtime, not to the typechecker. 11.3KB total, hand-reviewable, changing only when the Rust surface does. The runtime output — 7.4MB regenerated per release — stays ignored, which is why option (2), committing all of it, was rejected. Three `.gitignore` layers had to agree, and two of them were not obvious: - The re-inclusion has to start at the ROOT `.gitignore`. Its bare `dist` excludes the directory itself, and git cannot re-include a file whose parent directory is excluded — so negations in `packages/protect-ffi/.gitignore` had no effect on their own, which `git check-ignore -v` is what shows. - wasm-pack writes its own `dist/wasm/.gitignore` containing `*`. The deepest `.gitignore` wins, so that one file overrode every negation above it. `inline-wasm.mjs` now removes it in `postbuild:wasm`, beside where it already writes the ESM marker `package.json`. Negating around it is not possible; deleting it is safe because the parent rules still ignore the runtime output. Verified from a clean `rm -rf dist` + `build:wasm`: the three declarations are visible to git and the six runtime files are ignored. Also fixes two tsconfigs the earlier toolchain commit missed — `tsconfig.wasm-errors.json` and `type-tests/tsconfig.json` still extended `@tsconfig/node20`, which failed `build:wasm` at its declaration typecheck.
protect-ffi 0.31.0 narrowed an explicit `clientKey` to hex only. It used to be decoded by `SecretKey::from_hex`, which falls back to standard padded base64 — the encoding `~/.cipherstash/secretkey.json` stores on disk — so a base64 value pasted into the secret worked. The Neon entry forwards `CS_CLIENT_KEY` straight through as `clientKey`, so this is the exact value that now has to be hex. Without a check, a base64 secret presents as all six credentialed workflows failing simultaneously at client construction, with `invalid clientKey: expected a hex-encoded key` and nothing else — protect-ffi discards the decode error on purpose, because hex's own message names the offending character and its offset, which would put part of a live key into logs and error trackers. Six unrelated-looking red jobs and a message that does not mention encoding is a bad afternoon. It goes in `require-cs-secrets` because every workflow that sets CS_CLIENT_KEY already calls that action — verified across all six — so this is one edit rather than six, and it sits next to the existing presence check it naturally follows. The key is never echoed: the check is a charset and even-length test, and both the success and failure messages report only the length. Evidence on the actual value, for what it is worth: the local `.env` key is hex. `packages/stack`'s credentialed suites all pass against the live service under 0.31, which they could not do if the key were base64. That is not proof about the repository secret — GitHub secrets are write-only, so no tool can read it back — which is precisely why this check exists rather than a note.
Separate from any protect-ffi changeset, and deliberately so — the seven FFI packages must not be versioned before the trusted-publishing cutover, and `scripts/lint-no-ffi-changeset.mjs` enforces that. Verified: `changeset status` bumps the six Stack fixed-group packages and no FFI package, which is the ignore-window invariant holding. Bump level is **minor**, and it is a judgement call worth stating rather than burying. The hex-only client key is a runtime behaviour that used to work and now does not, which on a 1.0 package argues for major. Against that: hex is what the documented encoding has always been, base64 was undocumented tolerance from a decoder that happened to accept both, and the Stack fixed group means a major here takes `stash`, `wizard` and all three adapters to 2.0.0 with it. Minor plus a changelog entry that leads with the breakage seemed the better trade. Change the one word if you disagree — the blast radius of being wrong in the cautious direction is six version numbers. The entry leads with the client key because that is the failure people will actually hit, and it fails at client construction with a message that deliberately withholds detail (the underlying decode error would print part of a live key). Second is the DynamoDB error-code fix, which is the one change that could quietly alter a consumer's control flow: a `code` branch that was matching transport errors like ECONNRESET will stop matching. The rest — credential nesting on the WASM entry, dropped cast_as pre-normalisation, stripped bulk correlation ids — need no reader action and are summarised last.
`AGENTS.md` step 7 requires this in the same PR as the package move, because
these files have drifted badly before.
- **Repository Layout** gains `packages/protect-ffi`, naming the two things a
reader will not guess: it holds a nested Cargo workspace, and its six
platform packages are separate published packages linked at `workspace:*`.
- **A "Working on protect-ffi" section.** The layout entry says what the
package is; this says what will bite. Cargo is off the default `test` and
`build` paths on purpose, `lintWiring.test.ts` is what keeps it that way,
`pnpm run x -- --flag` is broken by pnpm's `--` forwarding, `lib/` is
generated and needs the Turbo outputs override, three WASM declarations are
tracked through a three-layer `.gitignore` chain, and publishing has not
moved yet so an FFI changeset fails CI. Each of those cost time to find.
- **`SECURITY.md`** lists all seven newly-published-from-here packages. The
six platform binaries are grouped in one row — separately versioned but
never separately chosen, since the loader picks one for the host.
Skills: the existing protect-ffi mentions are all about the npm dependency —
bundler externalisation, version-specific behaviour — and stay accurate; the
package name and its role are unchanged, and none claimed it lived in another
repo.
`skills/stash-auth` does need one addition. It is the canonical source for the
`CS_*` variables and already documented `CS_CLIENT_KEY` as hex, which is what
`stash env` emits — but older clients also accepted the base64 spelling from
`~/.cipherstash/secretkey.json`, so a key copied out of that file worked.
0.31 rejects it at construction with a message that deliberately withholds
detail, so the skill now names the symptom ("every operation fails at
construction after an upgrade") and both fixes. Skills ship inside the `stash`
tarball and are copied into customer repos, hence the `stash` patch changeset.
One line, and it changes when the platform binary is resolved from "whenever
anything imports this package" to "on first actual use".
`import * as native from './load.cjs'` compiles to
`__importStar(require("./load.cjs"))`, and `__importStar` ENUMERATES the
module's properties in order to copy them — which forces `@neon-rs/load`'s
proxy to resolve the binary at module-evaluation time. `import native =
require('./load.cjs')` emits a bare `require`, leaving the proxy untouched
until a wrapper body reads a property off it.
Measured, with the binary removed: `require('@cipherstash/protect-ffi')` threw
`MODULE_NOT_FOUND` before; it now returns 14 exports, and the same
`MODULE_NOT_FOUND` — identical `code` and `message` — is raised on first use
instead. The ESM entry behaves the same way (15 exports, no throw), since it
re-exports the CJS one.
Who this helps: callers that never encrypt anything. `@cipherstash/migrate`
imports one pure-JS type guard from the root entry and pays an ~8.7MB dlopen
for it. `@cipherstash/stack-prisma` reaches this package through one of its
fifteen stack imports; the other fourteen resolve to chunks with no cipherstash
dependency at all. Bundlers cannot observe the change — `__importStar` is a
runtime wrapper, the module graph is identical.
`assertNativeBindingAvailable()` is added as the counterpart, exported and NOT
yet consumed. Laziness costs `stash doctor` its probe: it relies on
`await import(pkg)` forcing resolution, which is exactly what no longer
happens. There is no fix available to the consumer — the proxy is unreachable
from outside (`require('@cipherstash/protect-ffi/lib/load.cjs')` gives
`ERR_PACKAGE_PATH_NOT_EXPORTED`), touching an export does not reach it (these
are this module's own wrappers), and calling through a real wrapper means
picking one whose argument validation does not reject first (`encryptBulk({})`
throws a plain validation error before reaching native). So the package has to
offer an explicit, named, stable way to force it. It forwards to `isEncrypted`
— pure, synchronous, no client — and lets the loader's error propagate
unwrapped so existing classification keeps working.
Wiring it to doctor is phase 5, deliberately: `@cipherstash/stack` must not
consume an API that published 0.31.0 lacks while the seven FFI packages are
still published from the old repository. Doctor's probe degrades quietly for
one release, and it is a diagnostic command, not the encryption path.
`src/nativeLoading.test.ts` guards the regression, which is otherwise silent
for everyone who has a binary installed. It asserts against the EMITTED
`lib/index.cjs` — comments stripped first, because the warning comment on the
import quotes the exact `__importStar` form it exists to prevent, and tsc
carries comments through to the emit. Mutation-checked: reverting the import
form fails the test.
No changeset. The guard added with the fixed group forbids one until npm
trusted publishing is repointed; this change's changeset lands in that cutover
PR, which is also where it becomes releasable.
Phases 1-2 are done; this rewrites the remainder as nine executable tasks and records what execution and review disproved. Every claim in the review was verified before being acted on: - `pnpm pack` takes NO positional directory. Reproduced: `pnpm pack --pack-destination out ./platforms/darwin-arm64` silently packed the WRAPPER. All six binary jobs would have uploaded wrapper tarballs. Fixed with `pnpm --dir`, and each job now asserts the packed name. - The matrix had no Rust target selection. Upstream derives it from `neon list-platforms` into `CARGO_BUILD_TARGET`, and the OpenSSL, aarch64 linker, musl and glibc-pinned zigbuild steps all hang off it. `macos-latest` is `macos-15-arm64`, so darwin-x64 would have shipped an ARM binary. - `needs.publish-ffi.result != 'failure'` passes when publish-ffi is SKIPPED, which is what happens if ffi-artifacts FAILS. Changesets would then publish the platform workspaces with no index.node. Now an explicit gate-outputs/success pair. - All seven manifests still named cipherstash/protectjs-ffi. npm requires repository.url to match the publishing repo exactly, so OIDC would have been rejected. New Task 2 fixes it before cutover, with a guard test. - Changesets tags only what it published (`tagPublish` receives `filter(result === "published")`), and it skips the seven as already-published. The pipeline now tags and releases them itself. Also corrected: the gate is load-bearing, not a cost optimisation (a false negative publishes binary-less packages); platform tarballs publish before the wrapper, since a lexicographic glob put the wrapper first; the pre-flight now checks binary ARCHITECTURE via `file`, not just size; release.yml's bare `pnpm install` becomes `--frozen-lockfile`; and phase 4 covers npm's post-2026-05-20 required "Allowed actions" selection. Two findings of my own carried in: release lines are coupled by PINNING, not by fixed groups — measured, with a table — and the Rust checks have run nowhere since the import, which Task 9 fixes. The previous rewrite of this file was lost by being left uncommitted across a branch switch. Hence this commit.
🦋 Changeset detectedLatest commit: 1556240 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
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.