Skip to content

Fix the public map and the sessions it needed (#115, #120) - #121

Merged
CaYatur merged 2 commits into
mainfrom
fix/public-map-and-sessions
Jul 28, 2026
Merged

Fix the public map and the sessions it needed (#115, #120)#121
CaYatur merged 2 commits into
mainfrom
fix/public-map-and-sessions

Conversation

@CaYatur

@CaYatur CaYatur commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Fix the public map and the sessions it needed (#115, #120)

Two reports that both ended in a visitor seeing nothing.

The public map never drew. MAP_JS is shared between the two pages and read
r.body, which is the panel's response shape; the public site's api() answers
{ok, s, j}. So every poll evaluated undefined.dimension and threw before
anything was assigned — hence the console error, and hence the status pill
sitting on "Bridge not connected" while the bridge was live, because the line
that would have set it never ran.

The store and crate modules never had this problem because they never fetch for
themselves. The map was the first shared module that did, and it inherited one
page's convention as if it were universal. It now takes mapGet/mapPost host
hooks alongside mapFeedUrl and mapAvatarUrl, and neither page's shape is
knowledge the engine holds.

Player sessions did not survive a restart. They lived in a Map while the
accounts were persisted, so restarting the manager signed out every player on
the website — except the browser still held the token and went on believing it
was signed in. Every request was then silently anonymous, and opening your own
profile told you no such player existed.

Sessions are persisted next to the accounts and expired on read. And a token
that was supplied and did not resolve now gets 401 rather than being treated
as no token at all: that is a fact about the credential, not about any name, so
the 404-vs-200 rule that closes the enumeration oracle for anonymous requests is
untouched. The site clears a rejected token and reopens the login instead of
showing a header that claims a session it does not have.

Also: the single-instance lock now means what its comment says. "Two
instances on the same launch dir = data corruption" — but Electron keys the lock
on the userData path, which was left at its default, making it app-wide. A
portable copy running from the desktop blocked a smoke run out of the repo: two
installs that share no state at all. userData now lives inside the launch dir,
which also puts Electron's cache where everything else this app keeps already
is. The renderer stores nothing in localStorage, so nothing is lost by moving
it.

The smoke seeded MAP.data and called mapDraw(), which is precisely why it
never saw the first bug. Both pages now run their own mapRefresh() against the
stub fetch and must end up with a frame and a bridge state that came from the
response — an unhandled rejection during it fails the gate.

Two reports that both ended in a visitor seeing nothing.

**The public map never drew.** `MAP_JS` is shared between the two pages and read
`r.body`, which is the panel's response shape; the public site's `api()` answers
`{ok, s, j}`. So every poll evaluated `undefined.dimension` and threw before
anything was assigned — hence the console error, and hence the status pill
sitting on "Bridge not connected" while the bridge was live, because the line
that would have set it never ran.

The store and crate modules never had this problem because they never fetch for
themselves. The map was the first shared module that did, and it inherited one
page's convention as if it were universal. It now takes `mapGet`/`mapPost` host
hooks alongside `mapFeedUrl` and `mapAvatarUrl`, and neither page's shape is
knowledge the engine holds.

**Player sessions did not survive a restart.** They lived in a Map while the
accounts were persisted, so restarting the manager signed out every player on
the website — except the browser still held the token and went on believing it
was signed in. Every request was then silently anonymous, and opening your own
profile told you no such player existed.

Sessions are persisted next to the accounts and expired on read. And a token
that was *supplied* and did not resolve now gets 401 rather than being treated
as no token at all: that is a fact about the credential, not about any name, so
the 404-vs-200 rule that closes the enumeration oracle for anonymous requests is
untouched. The site clears a rejected token and reopens the login instead of
showing a header that claims a session it does not have.

**Also: the single-instance lock now means what its comment says.** "Two
instances on the same launch dir = data corruption" — but Electron keys the lock
on the userData path, which was left at its default, making it app-wide. A
portable copy running from the desktop blocked a smoke run out of the repo: two
installs that share no state at all. userData now lives inside the launch dir,
which also puts Electron's cache where everything else this app keeps already
is. The renderer stores nothing in localStorage, so nothing is lost by moving
it.

The smoke seeded `MAP.data` and called `mapDraw()`, which is precisely why it
never saw the first bug. Both pages now run their own `mapRefresh()` against the
stub fetch and must end up with a frame and a bridge state that came from the
response — an unhandled rejection during it fails the gate.
Copilot AI review requested due to automatic review settings July 28, 2026 23:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ous test

resolvePlayerSession dropped an expired session and persisted the file. That
runs on every request carrying a token, and an expired token is exactly what a
replay holds — so a stale credential forced a serialise and two filesystem
operations per request, at ten requests a second per address. The same
amplification shape as the roster parse in #107, added while fixing something
else. The entry is dropped from memory only; the file is pruned the next time it
is loaded or written for a reason.

Sessions are capped at 2000, oldest first: fourteen-day tokens minted one per
login are unbounded within the window, and evicting the oldest logs out the
least recently used device rather than whoever just signed in.

The #107 assertion that an admin token is a stranger was guarded by
`if (status === 200)`. The 401 added here means that branch never runs, so the
test passed while checking nothing. It asserts 401 outright now, alongside a
dead player token, and separately that an anonymous read is still answered —
so the refusal cannot swallow the case the enumeration rule depends on.
@CaYatur

CaYatur commented Jul 28, 2026

Copy link
Copy Markdown
Owner Author

Self-review

1. Persisting sessions put a disk write on an unauthenticated read path

resolvePlayerSession dropped an expired session and called saveSessions().
That runs on every request carrying a token, and a token that has expired is
exactly what an attacker replaying an old one holds — so a stale token would
force a JSON serialise and two filesystem operations per request, at ten
requests a second per address.

The same amplification shape as the roster parse in #107, introduced while
fixing something else. The entry is now dropped from memory only; the file is
pruned of expired sessions the next time it is loaded or written for a real
reason, which is soon enough for something nobody can read.

Sessions are also capped at 2000, oldest first. Fourteen-day tokens with one
minted per login is unbounded within the window, and the eviction order matters:
oldest-first logs out the least recently used device rather than whoever just
signed in.

2. My own test stopped testing

The #107 assertion "an admin panel token is a stranger" was written as
if (pr.status === 200) { ...check the body... }. The 401 added in this PR
means that branch no longer runs, so the assertion passes without checking
anything.

It now asserts === 401 outright, which is the stronger statement anyway, plus
a dead player token (the actual restart case) and — separately — that an
anonymous request is still answered rather than refused, so the 401 cannot
quietly swallow the case the enumeration rule depends on.

Verified

The new map assertion was proved to catch the reported bug: restoring r.body
in the public site's mapGet gives
FAIL - the site map got no frame from its own api() — see #115, which is the
console error in the report.

Twelve gates green.

@CaYatur
CaYatur merged commit ff9e384 into main Jul 28, 2026
1 check passed
@CaYatur
CaYatur deleted the fix/public-map-and-sessions branch July 28, 2026 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants