Repository navigation
Session: a memory store never needs the secret; safer encryptor cache - #10
Merged
eliasjpr merged 1 commit intoOct 27, 2026
Merged
Conversation
MemoryStore#encrypts_with_secret? is false. The :production, :high_security and :clustered presets turn encrypt_redis_data on, and the Store default follows it, so a MemoryStore under those presets without SESSION_SECRET failed Session.configure and every request although it never encrypts. Configuration#encryptor publishes its cache through Atomic(EncryptorCache?) and reads the secret once per call. The default-secret messages suggest SESSION_SECRET and `openssl rand -hex 32` instead of a too-short literal. Specs: the Session.configure specs use a store that encrypts, cover the branch without a store, and reset_config restores every setting a preset changes. UPGRADE.md: expiry next to the secret section, presets that set sliding_expiration, and Store#encrypts_with_secret? for store authors. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HT5gvYGR83U2M4DyA6NA6f
Up to standards ✅🟢 Issues
|
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
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.
Follow-up to #8 and #9. Fixes these findings on the merged code:
Store#encrypts_with_secret?returnedencrypt_redis_datafor every store. The:production,:high_securityand:clusteredpresets turn that on, soMemoryStore+ one of those presets + noSESSION_SECRETin production madeSession.configureraise, andSessionHandler#callreturned a 500 on every request, even thoughMemoryStorenever encrypts. This setup worked on the previous master. NowMemoryStore#encrypts_with_secret?isfalse.ClusteredRedisStoreandPooledRedisStorekeep the base default, which is correct because they encrypt throughRedisStore, andCookieStorestaystrue.Session.configurespecs that only passed becausereset_configsets aMemoryStorenow use a stub store that encrypts the wayRedisStoredoes (RedisLikeStore).CRYSTAL_ENV=productionand no secret, aMemoryStoreboots and the handler answers 200. Also covered: theelsif config.encrypt_redis_databranch with no store (raises; does not raise when the flag is off), andencrypts_with_secret?for the memory, cookie and Redis stores.Configuration#encryptorpublishes its cache throughAtomic(EncryptorCache?)(acquire/release) and readssecretonce per call (validate_secret!(key)). The existing cache specs still pass. New specs check that the secret is read once and that fibers can use the encryptor while settings change.c.secret = "your-secure-secret"(18 bytes), which the 32-byte check now refuses. They now point toSESSION_SECRET/openssl rand -hex 32/c.secret = ENV["SESSION_SECRET"].reset_confignow restores every setting a preset or a spec changes (retry and circuit-breaker config,compression_threshold,log_errors,kdf_salt, cookie options,fail_fast_on_corruption,trust_proxy_headers,session_key) from a freshConfiguration. A spec covers this.:development,:production,:high_securityand:clusteredpresets already setsliding_expiration = true, and documentsStore#encrypts_with_secret?for store authors (also added todocs/api/store.md;docs/security/encryption.mdis updated to match).Each new spec was first run against the code without its fix and failed there: the
MemoryStoreboot/handler specs raisedInsecureSecretException, and the read-once, message andreset_configspecs failed.Checks (Crystal 1.22.1, with Redis):
crystal tool format --check src spec scripts examples: cleancrystal spec: 560 examples, 0 failures, 0 errors, 0 pendingcrystal spec -Dpreview_mton the configuration, presets, cookie_store, request_isolation and session_handler_error_handling specs: 115 examples, 0 failuresscripts/check_docs.cron README.md, UPGRADE.md and docs/**/*.md: 157 snippets, 0 failedRelease note: this is meant as a patch release. The release job is currently failing because tag
v1.0.32points at a PR merge-ref commit (7440c2c), so merging this will not publish a release until that tag is fixed.Generated by Claude Code