fix(expo): strip base64 padding from useLocalCredentials store keys - #10035
RaphaelFakhri wants to merge 1 commit into
Conversation
|
@RaphaelFakhri is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: c254796 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthrough
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Padded publishable keys now produce SecureStore-safe names, while unpadded keys retain their existing names. No concrete merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change consistently updates credential storage names without weakening biometric protection or changing the sign-in flow. Remaining uncertainty concerns storage compatibility and rollback for padded keys, rather than an established new attack path. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-biometrics
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/mosaic
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Description
useLocalCredentialsbuilds its secure-store keys from the raw publishable key. A publishable key ends with=when the base64 of<frontend host>$needs padding.expo-secure-storeonly accepts keys that match/^[\w.-]+$/, so the synchronousgetItem(key)call in theuseStateinitializer throws during render and takes down the whole screen.This change removes the
=characters from the publishable key before it builds the two store keys, the same way the offline resource cache already does. Keys without padding don't change, so credentials that are already stored stay readable.The new test mocks
expo-secure-storewith the same key validation as the real module. It fails on the parent commit and passes with this change.Fixes #10033
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change