feat(stack)!: wasm-inline takes workspaceCrn, not region (CIP-3245)#528
Closed
coderdan wants to merge 1 commit into
Closed
feat(stack)!: wasm-inline takes workspaceCrn, not region (CIP-3245)#528coderdan wants to merge 1 commit into
coderdan wants to merge 1 commit into
Conversation
Bumps @cipherstash/auth 0.38.0 → 0.40.0 and adopts the new AccessKeyStrategy.create(workspaceCrn, accessKey) signature. The @cipherstash/stack/wasm-inline client now derives the region from the workspace CRN and asserts the access-key token's workspace against it (WORKSPACE_MISMATCH), making the CRN the single source of truth — matching the Node entry. - WasmClientConfig.region (required, @deprecated) → workspaceCrn - resolveStrategy passes the CRN to AccessKeyStrategy.create - re-add CS_WORKSPACE_CRN to the supabase-worker example + wasm-e2e CI job (and its secret-presence assertion), removing the "deliberately omitted" notes - update the Deno round-trip e2e test + docs/comments Closes #523
🦋 Changeset detectedLatest commit: b6c0e91 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
📝 WalkthroughWalkthrough
ChangesworkspaceCrn Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stack/src/wasm-inline.ts (1)
318-330: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFail fast when
workspaceCrnis missing for JS callers.The runtime already compensates for plain JS bypassing the
accessKey/strategyunion, but Lines 329-330 still passcfg.workspaceCrnthrough unchecked. A caller that upgrades late and still sendsregionwill now fall into the auth factory withundefinedinstead of getting a package-level error that points at the migration.Suggested fix
if (cfg.strategy) return cfg.strategy - if (cfg.accessKey) - return AccessKeyStrategy.create(cfg.workspaceCrn, cfg.accessKey) + if (cfg.accessKey) { + if (!cfg.workspaceCrn) { + throw new Error( + '[encryption]: `config.workspaceCrn` is required when using `config.accessKey`.', + ) + } + return AccessKeyStrategy.create(cfg.workspaceCrn, cfg.accessKey) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stack/src/wasm-inline.ts` around lines 318 - 330, `resolveStrategy` currently guards the `strategy`/`accessKey` conflict, but it still forwards `cfg.workspaceCrn` unchecked when creating the access key strategy. Add a runtime validation in `resolveStrategy` so JS callers get a package-level error if `workspaceCrn` is missing before calling `AccessKeyStrategy.create`, and keep the failure message tied to the encryption config shape for easy migration debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/stack/src/wasm-inline.ts`:
- Around line 318-330: `resolveStrategy` currently guards the
`strategy`/`accessKey` conflict, but it still forwards `cfg.workspaceCrn`
unchecked when creating the access key strategy. Add a runtime validation in
`resolveStrategy` so JS callers get a package-level error if `workspaceCrn` is
missing before calling `AccessKeyStrategy.create`, and keep the failure message
tied to the encryption config shape for easy migration debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b83d6e6-bf4e-4ab4-a93e-7de82dc9bbee
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/wasm-inline-workspace-crn.md.github/workflows/tests.ymle2e/wasm/roundtrip.test.tsexamples/supabase-worker/.env.exampleexamples/supabase-worker/README.mdexamples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.tspackages/stack/src/wasm-inline.tspnpm-workspace.yaml
Contributor
Author
coderdan
deleted the
dan/cip-3245-wasm-inline-support-cs_workspace_crn-derive-region-from
branch
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
1 participant
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.
Summary
Closes #523. Brings
@cipherstash/stack/wasm-inlineto parity with the Node entry on workspace identity.The WASM-inline path previously required an explicit
regionand ignoredCS_WORKSPACE_CRN, because the underlying@cipherstash/auth/wasm-inlineAccessKeyStrategy.create()took a region argument. That blocker is now resolved:@cipherstash/auth0.39.0+ takes aworkspaceCrn, derives the region from it, and asserts the bearer token's workspace against it (WORKSPACE_MISMATCH).This PR bumps
@cipherstash/auth0.38.0 → 0.40.0 (catalog, in lockstep) and adopts the new signature:WasmClientConfig.region(required,@deprecated) →workspaceCrn— single source of truth (crn:<region>:<workspace-id>); region is derived and the access-key token's workspace is asserted against the CRN.resolveStrategypasses the CRN toAccessKeyStrategy.create(workspaceCrn, accessKey).CS_WORKSPACE_CRNto thesupabase-workerexample (.env.example, function, README) and thewasm-e2e-testsCI job (env + secret-presence assertion); removed the "deliberately omitted" notes.Breaking change for consumers of
@cipherstash/stack/wasm-inline: replaceconfig.region(e.g."ap-southeast-2.aws") withconfig.workspaceCrn(e.g."crn:ap-southeast-2.aws:<workspace-id>"). Changeset included (@cipherstash/stack, minor — pre-1.0).The
@cipherstash/authbump is low-impact elsewhere:wizard/cliuseAutoStrategy/device-flow/getToken(), whose signatures andTokenResultshape are unchanged across 0.38→0.40; theOAuthStrategy→DeviceSessionStrategyrename keeps a deprecated alias.Summary by CodeRabbit
New Features
workspaceCrnas the workspace identifier.Breaking Changes
regionwithworkspaceCrnin WASM client configuration.WORKSPACE_MISMATCH.Bug Fixes