Repository navigation
session: one random row per login - #83
Merged
Merged
Conversation
Each login writes its own row holding 3$ and 64 random hex chars instead of the constant 3.0.0, which v2 accepted as a session from anyone. The 3$ shape marks v3's rows the way a stored password hash names its own format. A login sweeps that user's v3 sessions idle past the token's max age, and leaves v2's to its own timeout. The token's exp now matches that max age. The mysql store selects last_access, so its activity throttle applies.
The token lifetime follows main's 8 hours: maxAgeSec in lib/session/v3.js is now 8 * 60 * 60, and the expire test's stale row sits past that cutoff.
The JWT validate accepted any well-signed token, so DELETE /session left the token working until it expired. A token is now valid only while its session row exists for that user. v2's timeout_sessions deletes rows idle past its session_timeout (45 min by default) on a shared db, so every authenticated request now refreshes last_access, not only GET /session.
# Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
🟡 Changes recommended
Existing predictable session rows remain exploitable through v2 until explicitly invalidated.
1 open finding
What changed in this PR
Improves session isolation and revocation for NicTool’s shared v2/v3 database.
Changes:
- Creates a random, version-marked session row per login.
- Checks session existence and refreshes activity on authenticated requests.
- Adds expiry cleanup and session regression tests.
| File | Description |
|---|---|
| routes/session.test.js | Tests revocation, activity, and write failures. |
| routes/session.js | Creates random sessions and expires stale rows. |
| routes/index.js | Validates session existence and updates activity. |
| lib/session/v3.test.js | Tests session value generation and identification. |
| lib/session/v3.js | Defines session markers and lifetime constants. |
| lib/session/test/index.js | Tests distinct rows, activity updates, and cleanup. |
| lib/session/store/mysql.js | Adds expiry cleanup and reads activity timestamps. |
| lib/session/store/file.js | Creates distinct rows and adds expiry cleanup. |
| CHANGELOG.md | Documents session security and lifecycle changes. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Login wrote the constant
3.0.0intont_user_session. v2 looks a session up by that column alone, so on a shared db, sendingnt_user_session=3.0.0to v2 logged in as a recent v3 user.Each login now gets its own row holding
3$and 64 random hex chars, marked the way #56 marks password hashes. A login deletes that user's expired v3 rows and leaves v2's alone. The token'sexpnow matches the 4 hour max age it was already refused at. The mysql store now readslast_access, so its one-minute touch throttle works.