Skip to content

fix(session): raise default seal PBKDF2 iterations to 8192#1457

Merged
pi0 merged 2 commits into
mainfrom
fix/session-crypto
Jul 11, 2026
Merged

fix(session): raise default seal PBKDF2 iterations to 8192#1457
pi0 merged 2 commits into
mainfrom
fix/session-crypto

Conversation

@pi0x

@pi0x pi0x commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Hardens session sealing by raising the default PBKDF2 iteration count from 1 to 8192 (KDF stretching as defense-in-depth; secret entropy remains the primary boundary).

  • Existing sessions sealed with the legacy default keep working via a transitional fallback unseal and are transparently resealed with the new default.
  • Documents that the session password must be generated from a cryptographically random source.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Security

    • Strengthened the default protection for encrypted session cookies by increasing PBKDF2 iterations (encryption and integrity).
    • Expanded session security guidance, including clearer explanation of the real password boundary and a command to generate a cryptographically secure secret.
  • Bug Fixes

    • Added transparent acceptance and upgrade of legacy sealed session tokens when cookies are refreshed.
    • Added legacy-seal fallback behavior control (legacySealFallback) to reject older tokens when disabled.
  • Tests

    • Extended test coverage for legacy session sealing/unsealing and updated iron-crypto test-vector behavior.

Adds a transitional fallback that unseals existing sessions sealed with
the legacy default (iterations: 1) and transparently reseals them, to be
removed before v2 stable.

Co-Authored-By: Claude Fable 5 <[email protected]>
@pi0x
pi0x requested a review from pi0 as a code owner July 11, 2026 18:43
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Session seal upgrade

Layer / File(s) Summary
Seal defaults and security guidance
src/utils/internal/iron-crypto.ts, src/utils/session.ts, src/utils/internal/session.ts, docs/4.examples/handle-session.md
PBKDF2 defaults increase to 8192 iterations, a legacy-seal marker is added, and documentation describes strong session-password requirements.
Legacy session compatibility
src/utils/session.ts
Legacy tokens using one iteration are retried, marked, and resealed when loaded from cookies, with fallback controlled by legacySealFallback.
Legacy seal validation
test/session.test.ts, test/unit/iron-crypto.test.ts
Tests cover legacy-cookie migration, disabled fallback behavior, and explicitly configured one-iteration Iron Crypto vectors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

Poem

I’m a bunny with a stronger seal,
Guarding cookies with cryptographic zeal.
Old tokens hop in, then renew,
New secrets keep the burrow true.
One small retry, a safer trail—
Encryption wins with every grail!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: increasing the default session seal PBKDF2 iterations to 8192.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/session-crypto

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pi0
pi0 merged commit 284c216 into main Jul 11, 2026
8 of 9 checks passed
@pi0
pi0 deleted the fix/session-crypto branch July 11, 2026 18:49

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/utils/session.ts (1)

271-271: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider avoiding as any for the kLegacySeal marker.

A typed intersection keeps the symbol property visible to the compiler while preserving the same runtime behavior.

♻️ Optional refactor
-    (unsealed as any)[kLegacySeal] = true;
+    (unsealed as Partial<Session> & Record<typeof kLegacySeal, boolean>)[kLegacySeal] = true;
🤖 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 `@src/utils/session.ts` at line 271, Update the assignment to kLegacySeal in
the unsealed session flow to avoid the any cast. Use a typed intersection that
explicitly includes the symbol property, preserving the existing marker
assignment and runtime behavior.
🤖 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.

Nitpick comments:
In `@src/utils/session.ts`:
- Line 271: Update the assignment to kLegacySeal in the unsealed session flow to
avoid the any cast. Use a typed intersection that explicitly includes the symbol
property, preserving the existing marker assignment and runtime behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f8556454-9b2e-484f-bc1e-1a48978ac5cd

📥 Commits

Reviewing files that changed from the base of the PR and between 2aff5df and 69253f5.

📒 Files selected for processing (2)
  • src/utils/session.ts
  • test/session.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants