Skip to content

id(web): needsRefresh suppresses identity token refresh forever on a non-numeric stored timestamp (NaN comparison) #847

Closed

Description

@dezren39

Problem

needsRefresh() in pkgs/id/web/src/main.ts:291-296 silently suppresses identity-token refresh forever when the stored refresh timestamp is not numeric:

function needsRefresh(): boolean {
  const refreshedAt = localStorage.getItem(IDENTITY_REFRESHED_AT_KEY);
  if (!refreshedAt) return true;
  const elapsed = Date.now() - Number(refreshedAt);
  return elapsed >= IDENTITY_REFRESH_INTERVAL_MS;
}

Number("garbage") is NaN; Date.now() - NaN is NaN; NaN >= interval is false. So any non-numeric value in id_identity_refreshed_at (corrupt storage, a value written by another build/version, manual tampering) makes needsRefresh() return false on every SPA navigation, indefinitely — the exact path (main.ts, onMainSwapped) whose comment says it "keeps the 30-day token alive" never calls ensureIdentity().

Verified with node on main 4f66baa: Date.now() - Number("garbage") >= 86400000 → false. A future/garbage-but-numeric timestamp has the same shape: a far-future value yields a negative elapsed and also suppresses refresh until wall-clock time catches up.

Why it matters

The failure mode is silent and total for the affected browser profile: the token is never proactively refreshed, so the user is eventually logged out at the 30-day expiry despite using the app daily via SPA navigation. localStorage is untrusted, user-writable state — parsing it with a bare Number() and no finiteness check is the same silent-reinterpretation family the id web code has been fixing elsewhere (#707/#832 world-seed, #712 goto-line), but here the bad value disables a keep-alive instead of producing a visible error.

Acceptance criteria

  • A non-numeric or non-finite stored timestamp is treated as "needs refresh" (true), as is a timestamp implausibly far in the future.
  • Normal behavior unchanged: missing key → true; elapsed ≥ 24h interval → true; recent timestamp → false.
  • The check is covered by a test (extracting the parse into a small testable helper next to the other pkgs/id/web/src/*.test.ts suites is fine), or an equivalent verified repro is attached to the fixing PR.
  • Single-repo: needsRefresh/IDENTITY_REFRESHED_AT_KEY exist only in developing-today/code pkgs/id/web/src/main.ts.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions