Repository navigation
fix: generate session IDs from 128 cryptographically random bits - #222
Merged
Merged
Conversation
Member
Author
|
Review notes before merging:
|
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.
Session IDs came from
Math.random().toString(36).substring(2, 15). That generator is not designed for identifiers: it carries at most about 52 bits of state per call, is predictable from earlier outputs, and the string drops trailing zero digits, so IDs vary in length (the new test caught an 11 character one on its first run). Inside the module the ID only separates sessions within one browser, butgetAttribution()payloads exist to be sent to a backend or CRM, where sessions from many visitors are compared and a collision would merge them.generateSessionIdnow encodes 16 bytes fromcrypto.getRandomValuesas 32 lowercase hex characters.crypto.randomUUIDwould be shorter to write, but browsers only expose it in secure contexts, and sites served over plain HTTP should keep tracking. Collection only runs in the browser, andcrypto.getRandomValuesis available in every browser Nuxt supports.Existing sessions keep their stored ID until the tab session ends, so deduplication within a session is unaffected. New IDs are longer than before; nothing in the module depends on the length, but backends that validate the old format would need to accept 32 hex characters.
test/session-id.test.tsgenerates 100 IDs and checks that each is 32 hex characters and that all are distinct. Checking many IDs makes it fail reliably when a byte loses its leading zero or the generator stops being random, which a single sample would miss most of the time. It failed on the old generator (expected '45s5g9vhehe' to match /^[0-9a-f]{32}$/) and passes now.Validation:
pnpm lint --max-warnings 0, Prettier on the changed files,pnpm test:types,pnpm dev:build,pnpm test(82 tests), andpnpm test:packagepass.