feat: drop cookie sealing with bidirectional migration#31
Conversation
…ling Introduces SessionEncryptionAdapter that wraps iron-webcrypto and auto-detects format on read (Fe26.* prefix → iron, else → JSON). Write mode is controlled by a `sealed` flag (default: false). The factory default now writes plain JSON cookies while transparently reading legacy iron-sealed cookies, enabling zero-downtime migration. Passing `sealed: true` re-enables encryption for rollback.
Greptile SummaryThis PR replaces iron-sealed session cookies with plain JSON by default, adding a
Confidence Score: 4/5The migration logic and PKCE protection are sound, but the unsealed-by-default decision moves refresh tokens into plain-text Cookie headers where server-side logging infrastructure can capture them — this warrants explicit acknowledgment and deployment guidance before merging. The bidirectional adapter, shape validation, and PKCE sealing are correctly implemented. The main concern is that refreshToken and accessToken now travel as readable JSON in every session request's Cookie header; any CDN, proxy, WAF, or SIEM log pipeline that records raw Cookie values will capture long-lived credentials. The PR describes httpOnly/secure/sameSite as the safety justification, but those attributes only guard against client-side access, not server-side log capture. src/core/encryption/sessionEncryption.ts — the unsealed write path and its implications for server-side log exposure deserve explicit documentation or a deployment checklist note.
|
| Filename | Overview |
|---|---|
| src/core/encryption/sessionEncryption.ts | New SessionEncryptionAdapter that writes plain JSON (unsealed mode default) for session cookies and always iron-seals PKCE state; the unsealed path exposes refresh tokens in HTTP Cookie headers readable by server-side logging infrastructure |
| src/core/config/ConfigurationProvider.ts | Removes cookiePassword from required fields and derives it from apiKey+clientId when absent; validation gap — derived password length is never checked against the 32-char minimum |
| src/core/AuthKitCore.ts | Adds isSessionLike runtime shape validator in decryptSession; cleanly re-throws SessionEncryptionError to avoid double-wrapping; logic is correct |
| src/service/factory.ts | Switches default encryption to SessionEncryptionAdapter wrapping ironWebcryptoEncryption; stateless today so functional behaviour is unchanged |
| src/core/encryption/sessionEncryption.spec.ts | Comprehensive adapter tests covering both modes, cross-mode reads, PKCE TTL guard, unicode, error paths, and prefix collision safety — well structured |
Sequence Diagram
sequenceDiagram
participant Browser
participant Server
participant SessionEncryptionAdapter
participant IronWebcrypto
Note over Browser,Server: Session write (unsealed mode default)
Server->>SessionEncryptionAdapter: "sealData(session, {password, ttl:0})"
SessionEncryptionAdapter-->>Server: JSON.stringify(session)
Server->>Browser: "Set-Cookie: wos-session={...} (httpOnly, secure)"
Note over Browser,Server: Session read (bidirectional migration)
Browser->>Server: "Cookie: wos-session=value"
Server->>SessionEncryptionAdapter: "unsealData(value, {password})"
alt value starts with Fe26.
SessionEncryptionAdapter->>IronWebcrypto: unsealData(value)
IronWebcrypto-->>SessionEncryptionAdapter: session object
else plain JSON
SessionEncryptionAdapter-->>SessionEncryptionAdapter: JSON.parse(value)
end
SessionEncryptionAdapter-->>Server: session object
Server->>Server: isSessionLike(session) validation
Note over Browser,Server: PKCE flow (always sealed)
Server->>SessionEncryptionAdapter: "sealData(pkceState, {password, ttl:600})"
SessionEncryptionAdapter->>IronWebcrypto: "sealData(pkceState, {ttl:600})"
IronWebcrypto-->>SessionEncryptionAdapter: "Fe26.2*..."
Server->>Browser: "Set-Cookie: wos-auth-verifier=Fe26.2*..."
Reviews (4): Last reviewed commit: "feat: make cookiePassword optional, deri..." | Re-trigger Greptile
| encryptionFactory = () => | ||
| new SessionEncryptionAdapter(ironWebcryptoEncryption), |
There was a problem hiding this comment.
The new default creates a fresh
SessionEncryptionAdapter on every encryptionFactory() call, whereas the old default returned the shared ironWebcryptoEncryption singleton. Because SessionEncryptionAdapter is stateless this is functionally equivalent today, but it diverges from the existing pattern and would mask state bugs if the class ever acquired state. Wrapping the construction in once (already imported) matches how the rest of the factory avoids repeated construction.
| encryptionFactory = () => | |
| new SessionEncryptionAdapter(ironWebcryptoEncryption), | |
| encryptionFactory = once(() => | |
| new SessionEncryptionAdapter(ironWebcryptoEncryption)), |
… API
- Always seal when TTL > 0 so PKCE codeVerifier never appears in
plaintext in the OAuth state URL parameter
- Default mode is 'sealed' for safe rolling deploys (readers first,
flip to 'unsealed' once all nodes run the adapter)
- Replace positional boolean with options object ({ mode: 'sealed' | 'unsealed' })
- Add runtime session shape validation in decryptSession() — rejects
arrays, primitives, null, and objects missing required fields
- Add tests: empty/whitespace input, fake Fe26. prefix, invalid session
shapes, PKCE TTL guard, prefix collision safety
Session cookies are written as plain JSON by default. PKCE state
remains iron-sealed (TTL > 0 guard). Consumers can pass
{ mode: 'sealed' } to re-enable encryption.
| /** | ||
| * `'sealed'` — always encrypt via iron-webcrypto (current behavior). | ||
| * `'unsealed'` — write plain JSON for session cookies. PKCE state | ||
| * (TTL > 0) is always sealed regardless of this setting because | ||
| * the sealed blob appears in the OAuth `state` URL parameter. | ||
| * | ||
| * Default: `'sealed'` — deploy readers first, flip to `'unsealed'` | ||
| * once all nodes run the adapter. | ||
| */ | ||
| mode?: 'sealed' | 'unsealed'; |
There was a problem hiding this comment.
The JSDoc for
mode says the default is 'sealed', but the constructor assignment on line 37 (options.mode ?? 'unsealed') defaults to 'unsealed'. A consumer reading this doc to plan a rolling migration would believe they can deploy readers first with sealed behavior, then flip — but the adapter already writes JSON out of the box. The stated migration strategy ("deploy readers first, flip to unsealed once all nodes run the adapter") only makes sense if the initial default is sealed.
| /** | |
| * `'sealed'` — always encrypt via iron-webcrypto (current behavior). | |
| * `'unsealed'` — write plain JSON for session cookies. PKCE state | |
| * (TTL > 0) is always sealed regardless of this setting because | |
| * the sealed blob appears in the OAuth `state` URL parameter. | |
| * | |
| * Default: `'sealed'` — deploy readers first, flip to `'unsealed'` | |
| * once all nodes run the adapter. | |
| */ | |
| mode?: 'sealed' | 'unsealed'; | |
| /** | |
| * `'sealed'` — always encrypt via iron-webcrypto (current behavior). | |
| * `'unsealed'` — write plain JSON for session cookies. PKCE state | |
| * (TTL > 0) is always sealed regardless of this setting because | |
| * the sealed blob appears in the OAuth `state` URL parameter. | |
| * | |
| * Default: `'unsealed'` — session cookies are written as plain JSON. | |
| * Pass `{ mode: 'sealed' }` to re-enable iron-webcrypto encryption. | |
| */ | |
| mode?: 'sealed' | 'unsealed'; |
| * Default: `'sealed'` — deploy readers first, flip to `'unsealed'` | ||
| * once all nodes run the adapter. |
There was a problem hiding this comment.
🟡 JSDoc states default mode is 'sealed' but implementation defaults to 'unsealed'
The JSDoc comment on the mode property at src/core/encryption/sessionEncryption.ts:12 states Default: 'sealed', but the constructor at line 37 actually defaults to 'unsealed' (options.mode ?? 'unsealed'). This discrepancy means developers reading the documentation in their IDE will believe sessions are encrypted by default, when in reality session cookies are stored as plain JSON without encryption or integrity protection. The createAuthService factory at src/service/factory.ts:52 uses the adapter with no explicit mode, inheriting the 'unsealed' default.
| * Default: `'sealed'` — deploy readers first, flip to `'unsealed'` | |
| * once all nodes run the adapter. | |
| * Default: `'unsealed'` — session cookies are written as plain JSON. | |
| * Pass `'sealed'` to re-enable iron-webcrypto encryption. |
Was this helpful? React with 👍 or 👎 to provide feedback.
WORKOS_COOKIE_PASSWORD is no longer required. When not provided, a sealing password is derived from the already-required apiKey and clientId. This eliminates an extra env var for most setups. Explicitly setting WORKOS_COOKIE_PASSWORD still works and takes precedence — useful for cookie continuity across API key rotations.
| async sealData( | ||
| data: unknown, | ||
| options: { password: string; ttl?: number | undefined }, | ||
| ): Promise<string> { | ||
| if (this.mode === 'sealed' || (options.ttl != null && options.ttl > 0)) { | ||
| return this.ironEncryption.sealData(data, options); | ||
| } | ||
| return JSON.stringify(data); | ||
| } |
There was a problem hiding this comment.
Plain-JSON session cookies expose
refreshToken to server-side log infrastructure
The PR's safety argument (httpOnly, secure, sameSite=lax) only prevents client-side JavaScript access. It does not protect against server-side log capture: CDN edge logs, WAF/SIEM systems, reverse proxies, and load balancers routinely log raw Cookie request headers at DEBUG or AUDIT level. Once a request hits those systems, the Cookie: wos-session={"accessToken":"eyJ...","refreshToken":"rt_...","user":{...}} header is written in plain text.
refreshToken is long-lived — an attacker who extracts it from a log entry can impersonate the user until the token is revoked. Consider at minimum documenting that callers must sanitize Cookie headers from any logging infrastructure, or offer a way to enable sealed mode by default for security-sensitive deployments.
Rule Used: Do not log sensitive fields like access_token, ref... (source)
Summary
SessionEncryptionAdapterthat wraps iron-webcrypto and auto-detects format on read (Fe26.*→ iron unseal, otherwise → JSON parse)'unsealed'— session cookies written as plain JSONstateURL parameterdecryptSession()rejects malformed cookie values{ mode: 'sealed' }to re-enable encryptionMigration behavior
Seamless. The adapter reads both formats, so existing users with iron-sealed cookies are never rejected. On next token refresh, the cookie is rewritten as JSON. No re-auth, no downtime.
Why this is safe
wos-session) arehttpOnly,secure,sameSite=lax— cookie attributes prevent client-side accesscodeVerifierFe26.prefix — no collision with JSONdecryptSessionvalidates decoded shape at runtime, closing the gap left by removing HMAC integrityTest plan
wos-sessioncookie is readable JSON