chore: bump cipherstash-client and stack-auth to 0.37.0#100
Conversation
Update cipherstash-client and stack-auth to 0.37.0, plus the lockstep crates cts-common and stack-profile, to avoid duplicate 0.36/0.37 copies in the dependency tree. Pulls in transitive bumps of cipherstash-config, cipherstash-core (0.37.0) and zerokms-protocol (0.12.17).
|
Too many files changed? Review this PR in Change Stack to see how the pieces fit before you dive in. Warning Review limit reached
More reviews will be available in 11 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPins protect-ffi Rust dependencies to =0.37.0, updates integration-tests to use ChangesDependency pins and integration-test updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Add integration-tests/tests/oidc-federation.test.ts covering the new
stack-auth OidcFederation strategy (surfaced via @cipherstash/auth).
The FFI boundary duck-types on the AuthStrategy { getToken } contract and
never branches on strategy type, so OIDC coverage is consumer-side wiring
mirroring js-strategy.test.ts and wasm-round-trip.test.ts.
The hermetic end-to-end block is describe.skip-ped pending two upstream
additions in @cipherstash/auth (tracked in cipherstash-suite):
- OidcFederation is not exported yet (0.38.0 exposes only Auto/AccessKey/
OAuth strategies)
- MockAuthServer needs an OIDC token-exchange mock + base-URL override
A live contract test runs today (no creds/network): it asserts a rejected
getToken propagates as a client error through the exact opts.strategy path
the real OidcFederation will travel.
The OidcFederationStrategy export already shipped in @cipherstash/auth
0.39.0 (npm latest); the prior ^0.38.0 pin predated it. Bump the
integration-tests dep and replace the pending scaffold with real coverage:
- Contract tier (runs in CI, no creds): the published
OidcFederationStrategy constructs via .create and satisfies the FFI's
structural AuthStrategy contract; a rejected getToken propagates through
newClient.
- End-to-end tier (describe.skipIf on CS_OIDC_JWT + dataset creds): a real
federation round-trip — third-party JWT -> CTS token -> encrypt/decrypt.
A hermetic MockAuthServer round-trip isn't reachable through the published
consumer API (napi-only mock; no base-URL override on the public .create),
so the live exchange is env-gated rather than mocked.
0.39.0 also makes AccessKeyStrategy.create take the full workspace CRN
(was the <region>.<provider> segment). Update the existing wasm-round-trip
and js-strategy suites and the event-loop-exit fixture to pass the full CRN,
dropping the now-needless CRN-splitting.
- E2E: wrap the wasm OidcFederationStrategy in a plain { getToken } object
before handing it to the Neon newClient. The JsBacked path is proven
against this shape (js-strategy.test.ts); passing a wasm-bindgen class
instance straight through was untested and the block is skip-gated, so a
failure there would have shipped green.
- Contract: drop the vacuous expect(asStrategy).toBeDefined() (always true)
in favour of asserting getToken is callable on the contract-typed handle.
The skip-gated end-to-end round-trip relied on a static CS_OIDC_JWT, which can't work: OIDC JWTs expire in minutes, and CI sets no such secret, so the block was skipped on every run — green but inert, masking the absence of coverage. It also duplicated existing coverage: the FFI <-> wasm-strategy <-> ZeroKMS encrypt/decrypt path is already proven via AccessKeyStrategy (wasm-round-trip/js-strategy), and the OIDC-specific getJwt -> /api/authorise exchange is tested hermetically inside stack-auth (MockAuthServer + a base-URL override not exposed on the published consumer API). Keep the two contract tests — they run in CI unconditionally with no credentials and nothing skipped — and drop the now-unused E2E block, splitCrn helper, and encrypt/decrypt/isEncrypted imports.
a8535c9 to
458b398
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust-side CipherStash client/auth crate versions to 0.37.0 (plus related lockstep crates) and updates the JS integration test harness to @cipherstash/auth@^0.39.0, including new contract coverage for the OIDC federation strategy and updating AccessKeyStrategy.create call sites to use the full workspace CRN.
Changes:
- Bump Rust dependencies (
cipherstash-client,stack-auth,cts-common,stack-profile) to0.37.0and updateCargo.lockaccordingly. - Bump
@cipherstash/authto^0.39.0inintegration-testsand adjust strategy construction to pass the full workspace CRN. - Add
OidcFederationStrategycontract tests to validate structural compatibility with the FFI strategy contract and error propagation behavior.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| integration-tests/tests/wasm-round-trip.test.ts | Update wasm round-trip test to use full workspace CRN with AccessKeyStrategy.create. |
| integration-tests/tests/oidc-federation.test.ts | Add OIDC federation strategy contract tests. |
| integration-tests/tests/js-strategy.test.ts | Update JsBacked strategy tests to use full workspace CRN with AccessKeyStrategy.create. |
| integration-tests/tests/fixtures/event-loop-exit-jsbacked.cjs | Update fixture to use full workspace CRN with AccessKeyStrategy.create. |
| integration-tests/package.json | Bump @cipherstash/auth dependency to ^0.39.0. |
| integration-tests/package-lock.json | Lockfile updates for @cipherstash/auth@0.39.0 and local package version. |
| crates/protect-ffi/Cargo.toml | Bump Rust crate dependencies to =0.37.0. |
| Cargo.lock | Lockfile updates for Rust crate bumps, including zerokms-protocol transitive update. |
Files not reviewed (1)
- integration-tests/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const clientOpts: ClientOpts = { | ||
| clientId: process.env.CS_CLIENT_ID, | ||
| clientKey: process.env.CS_CLIENT_KEY, | ||
| } |
There was a problem hiding this comment.
Good catch — confirmed and fixed in a37dd25.
You're right that clientOpts from process.env made this non-deterministic. Verified empirically: with the CS_ vars unset, newClient resolves and deserializes the client key into a ProxyKeySet before it ever calls getToken, so it fails with a key-provider error (Client key not configured / Invalid client key: … expected struct ProxyKeySet) rather than the strategy error being asserted. A fabricated dummy key can't get past that eager structural check, so the test can't be made hermetic by swapping in dummy creds.
That test also used a generic { getToken } stub rather than the real OidcFederationStrategy, which made it an exact duplicate of js-strategy.test.ts's propagates a rejected getToken Promise (already runs in CI with creds) — so it added no OIDC-specific coverage.
Fix: dropped it and kept only the genuinely hermetic, OIDC-specific contract test (the real published OidcFederationStrategy constructs via .create and is structurally assignable to the FFI AuthStrategy with a callable getToken). It now passes identically with CS_ vars set or unset — no credentials, no network, no env dependence. The header points at js-strategy.test.ts for the newClient-propagation coverage.
Copilot flagged that the 'rejected getToken propagates' test sourced
clientOpts from process.env, making the rejection reason non-deterministic
when creds are absent. Confirmed: with CS_ vars unset, newClient resolves
(and deserializes) the client key into a ProxyKeySet *before* it ever calls
getToken, so it fails with a key-provider error ('Client key not configured'
/ 'Invalid client key') rather than the strategy error being asserted.
That test also used a generic { getToken } stub rather than the real
OidcFederationStrategy, making it an exact duplicate of js-strategy.test.ts's
'propagates a rejected getToken Promise' (which runs in CI with creds). It
added no OIDC-specific coverage and couldn't be made hermetic without valid
key material.
Drop it and keep only the genuinely hermetic, OIDC-specific contract test:
the real published OidcFederationStrategy constructs via .create and is
structurally assignable to the FFI AuthStrategy with a callable getToken. No
credentials, no network, no env dependence — passes identically with CS_ vars
set or unset. Header updated to point at js-strategy.test.ts for the
newClient-propagation coverage.
Summary
Updates
cipherstash-clientandstack-authto0.37.0. Also bumps the lockstep cratescts-commonandstack-profileto0.37.0so the dependency tree doesn't end up with duplicate0.36/0.37copies (cipherstash-client0.37 pulls in the 0.37 versions transitively).Transitive bumps that came along:
cipherstash-config,cipherstash-core→0.37.0,zerokms-protocol→0.12.17.This branch also includes the OIDC federation test coverage from #101 (merged in): bumps
@cipherstash/authto^0.39.0and addsOidcFederationStrategycontract tests, plus theAccessKeyStrategy.createfull-CRN update across the existing strategy suites.Testing
cargo test— 71 passednpm run build:wasm— clean build + inline shim generatedtests/keyset.test.ts.Local-only keyset failures (these pass in CI)
Running
mise run test:integration:alllocally shows 2 failures intests/keyset.test.ts(Failed to load keyset: NotFound). They reproduce only against local credentials — root cause is a workspace-permission gap, visible inbeforeAll:The local credentials can't provision/grant the
Testkeyset. CI runs with workspace credentials that can, so those tests — and the whole suite — pass green in CI. Either way it's environment/access-control related and independent of the version bump.Summary by CodeRabbit