fix(auth): fail closed when a session's stored expiry is unparseable

[nickmopen]

Summary

authenticateSessionToken (src/auth/security.ts) gates a session on:

if (session.revokedAt || Date.parse(session.expiresAt) <= Date.now()) return null;

A session whose stored expires_at is malformed or empty parses to NaN, and NaN <= Date.now() is false. Combined with a null revokedAt, the session is then treated as valid and never-expiring — a fail-open on the expiry check.

Problem

expires_at is normally written by our own session-creation path, but the auth check should not depend on that invariant holding for every row (DB drift, a partial write, a manually inserted row). An unparseable expiry must be rejected, not silently honored forever. This matches the project's fail-closed posture (e.g. #1368 "fail closed when … unreadable").

Fix

Parse once and guard with Number.isFinite, so an unparseable expiry fails closed exactly like an expired one. Revoked and past-dated behavior is unchanged.

Validation

Run from repo root (Node 24):

npm run test:ci   # GATE_EXIT=0 — 4607 passed / 4 skipped; typecheck, coverage, workers, MCP, UI all green

Added a regression assertion alongside the existing expired-session test in test/unit/auth.test.ts: a session with expires_at = "not-a-date" now authenticates to null (it would have returned a valid identity before this fix).

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.37%. Comparing base (b5d575e) to head (6f49da6).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1498   +/-   ##
=======================================
  Coverage   95.37%   95.37%           
=======================================
  Files         199      199           
  Lines       21546    21547    +1     
  Branches     7791     7791           
=======================================
+ Hits        20550    20551    +1     
  Misses        416      416           
  Partials      580      580           

Files with missing lines	Coverage Δ
src/auth/security.ts	98.68% <100.00%> (+0.01%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gittensory-orb]

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review — blocked

2 files · 1 AI reviewers · no blockers · readiness 66/100 · CI failing · blocked

🛑 Blocked

Review summary
The change correctly patches a fail-open in `authenticateSessionToken` (src/auth/security.ts:121–124): by extracting the `Date.parse` result and guarding with `Number.isFinite`, a malformed or empty `expires_at` stored in the DB is now rejected rather than silently authenticated as a never-expiring session. The logic is minimal and correct — `Number.isFinite(NaN)` is `false`, the short-circuit fires, and revoked/legitimately-expired session paths are unchanged. The regression test in the diff directly covers the new behavior, though it is nested inside an existing `it()` case.

CI checks failing

• validate
• lint

Signal	Result	Evidence
Code review	✅ No blockers	1 reviewers, synthesized
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Review load	✅ 20/20	Readiness component derived from cached public PR metadata and labels; size label size:XS.
Validation evidence	❌ 5/25	Cached preflight status is hold.
Open PR queue	❌ 3/10	20 open PR(s), 9 likely reviewable, 11 unlinked.
Contributor context	✅ Confirmed Gittensor contributor	nickmopen; Gittensor profile; 46 PR(s), 1 issue(s).
Gate result	✅ Passing	No configured blocker found.

Nits — 6 non-blocking

• The new comment at src/auth/security.ts:119 uses a non-ASCII `→` (U+2192); if the project enforces ASCII-only source content this is a plausible cause of the failed `lint` check — replacing it with `->` or plain prose eliminates the risk.
• The malformed-expiry assertion is appended inside the existing expired-session `it()` block in test/unit/auth.test.ts rather than its own `it('fails closed when expiresAt is unparseable')` block; a future failure there will surface under the wrong test label and be harder to triage.
• Move the `malformed-expiry-user` assertions in test/unit/auth.test.ts into a dedicated `it()` block so test output unambiguously identifies this specific regression path.
• Replace `→` in the src/auth/security.ts comment with `->` or rewrite without the symbol to stay lint-clean regardless of charset rules.
• Repository config was not parsed
• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

Review context

• Author: nickmopen
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository registration is not available in the local Gittensory cache.
• Public profile languages: TypeScript
• Official Gittensor activity: 46 PR(s), 1 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Explain no-issue PR.
• Fix blocker.
• Expect slower review.
• Refresh registry data or choose a registered active repo.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
• Open PR queue = repo-wide review pressure; it is not a PR quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The change correctly patches a fail-open in `authenticateSessionToken` (src/auth/security.ts:121–124): by extracting the `Date.parse` result and guarding with `Number.isFinite`, a malformed or empty `expires_at` stored in the DB is now rejected rather than silently authenticated as a never-expiring session. The logic is minimal and correct — `Number.isFinite(NaN)` is `false`, the short-circuit fires, and revoked/legitimately-expired session paths are unchanged. The regression test in the diff directly covers the new behavior, though it is nested inside an existing `it()` case.

Nits (4)

• The new comment at src/auth/security.ts:119 uses a non-ASCII `→` (U+2192); if the project enforces ASCII-only source content this is a plausible cause of the failed `lint` check — replacing it with `->` or plain prose eliminates the risk.
• The malformed-expiry assertion is appended inside the existing expired-session `it()` block in test/unit/auth.test.ts rather than its own `it('fails closed when expiresAt is unparseable')` block; a future failure there will surface under the wrong test label and be harder to triage.
• Move the `malformed-expiry-user` assertions in test/unit/auth.test.ts into a dedicated `it()` block so test output unambiguously identifies this specific regression path.
• Replace `→` in the src/auth/security.ts comment with `->` or rewrite without the symbol to stay lint-clean regardless of charset rules.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review