Self-review: close the account oracle, cache the roster, use showProfiles

The profile endpoint answered 200 for a name with a website account and 404 for
one without, whenever the name had never played — one request per name, no
credential needed. That is the enumeration oracle closed in #105, rebuilt in a
different route. isRegistered is consulted only when the viewer is already
authenticated as that name, which keeps the case it was there for: someone who
registered on a server that has never run can still see their own profile.

getPlayers NBT-parses every player .dat in the world. Every other public route
answers from memory, and the per-address budget allows ten requests a second, so
one visitor could have the whole world folder parsed ten times a second without
signing in. Cached for ten seconds, shared across callers. The client made it
worse: refreshWhoami re-fetched whenever puuid was empty, which it stays for a
player who registered but never joined, so every navigation fired another roster
parse. It tries once per page load now.

showProfiles was added to the public payload and never read, so with no server
behind the site the header linked a visitor to a page that says "no such player"
about themselves.

Author: CaYatur

src/main/smoke.ts

@@ -4521,20 +4521,37 @@ export async function runWebSmoke(): Promise<void> {
         if (own.mcName !== 'Profiley') return fail('own profile returned the wrong player')
         if (own.hidden.length !== 0) return fail('a player was told their own data was withheld')
 
-        // A stranger reading the same name, with everything off.
+        // ...and a STRANGER asking about that same name gets the same answer
+        // they would get for a name nobody has ever heard of. The fixture
+        // server has no roster, so `Profiley` exists only as a website account
+        // — and answering 200 for it while answering 404 for `Nobodyy` would
+        // make this endpoint report which names have accounts, one per request.
+        // That is the oracle closed in #105, in a different route.
+        const strangerHit = await sget('/api/public/profile?name=Profiley')
+        const strangerMiss = await sget('/api/public/profile?name=Nobodyy')
+        if (strangerHit.status !== strangerMiss.status) {
+          return fail(
+            'a stranger can tell a registered name from an unknown one: ' +
+              strangerHit.status + ' vs ' + strangerMiss.status
+          )
+        }
+        // Whatever a stranger does get back must carry none of the gated fields.
         pr = await sget('/api/public/profile?name=Profiley')
-        if (pr.status !== 200) return fail('a public profile expected 200, got ' + pr.status)
-        const strange = (await pr.json()) as unknown as Record<string, unknown>
-        for (const f of GATED) {
-          if (f in strange) return fail('an anonymous profile read carried "' + f + '"')
+        if (pr.status === 200) {
+          const strange = (await pr.json()) as unknown as Record<string, unknown>
+          for (const f of GATED) {
+            if (f in strange) return fail('an anonymous profile read carried "' + f + '"')
+          }
         }
         // An admin token must not be a player token here either: the endpoint
         // decides "owner" from a PLAYER session, and an operator holding a panel
         // token is a stranger to every player account.
         pr = await sget('/api/public/profile?name=Profiley', ot)
-        const asAdmin = (await pr.json()) as unknown as Record<string, unknown>
-        for (const f of GATED) {
-          if (f in asAdmin) return fail('an admin token read a player\'s ' + f + ' from the public site')
+        if (pr.status === 200) {
+          const asAdmin = (await pr.json()) as unknown as Record<string, unknown>
+          for (const f of GATED) {
+            if (f in asAdmin) return fail('an admin token read a player\'s ' + f + ' from the public site')
+          }
         }
         console.log('WEB-SMOKE: public profile OK (own vs stranger, admin token is a stranger, 400 on a bad name)')
       } finally {

src/main/web/publicSiteHtml.ts

@@ -268,10 +268,15 @@ var S=null,LANG='en',ptoken=localStorage.getItem('msms_ptoken')||'',pname=localS
 /* The signed-in player's uuid, for the head beside their name (#107). Cached in
    localStorage rather than re-fetched on every render: it never changes for a
    given name, and the profile read parses player .dat files. */
-var puuid=localStorage.getItem('msms_puuid')||'';
+var puuid=localStorage.getItem('msms_puuid')||'',whoamiTried=false;
 function refreshWhoami(){
- if(!ptoken){puuid='';localStorage.removeItem('msms_puuid');return}
- if(puuid)return;
+ if(!ptoken){puuid='';whoamiTried=false;localStorage.removeItem('msms_puuid');return}
+ if(puuid||whoamiTried)return;
+ /* Once per page load, not once per render. A player who has registered but
+    never joined has no uuid to find, so without the flag every navigation
+    would fire another request — and on the server side that request is the
+    only public one that parses the world's player files. */
+ whoamiTried=true;
  api('/api/public/profile',null,ptoken).then(function(r){
   if(!r.ok||!r.j.uuid)return;
   puuid=r.j.uuid;localStorage.setItem('msms_puuid',puuid);renderChrome()})}
@@ -310,7 +315,12 @@ function renderChrome(){
  var sel=document.getElementById('langSel');var codes=Object.keys(S.i18n.langs);
  sel.innerHTML=codes.map(function(c){return '<option value="'+c+'"'+(c===LANG?' selected':'')+'>'+c.toUpperCase()+'</option>'}).join('');
  document.getElementById('accBtn').innerHTML=ptoken
-  ? '<a href="#/profile" class="whoami" title="'+escAttr(T('profile.title'))+'">'+headImg(puuid,24)+'<span>'+esc(pname)+'</span></a><button class="btn sm" onclick="plogout()">'+esc(T('auth.logout'))+'</button>'
+  ? (S.showProfiles
+      ? '<a href="#/profile" class="whoami" title="'+escAttr(T('profile.title'))+'">'+headImg(puuid,24)+'<span>'+esc(pname)+'</span></a>'
+      /* No server behind the site means no profile to link to: the page would
+         load and say "no such player" about the visitor themselves. */
+      : '<span class="muted" style="margin-right:10px;font-size:14px">'+esc(pname)+'</span>')+
+    '<button class="btn sm" onclick="plogout()">'+esc(T('auth.logout'))+'</button>'
   : '<button class="btn sm primary" onclick="openAuth()">'+esc(T('auth.login'))+'</button>';
  var links=[['#/','nav.home'],['#/news','nav.news']];
  if(S.showStore)links.push(['#/store','nav.store']);
@@ -548,8 +558,8 @@ function render(){
  else if(h==='#/store')app.innerHTML=pageStore();
  else if(h==='#/servers')app.innerHTML=pageServers();
  else if(h==='#/map'&&S.showMap)app.innerHTML=pageMap();
- else if(h.indexOf('#/player/')===0)app.innerHTML=pageProfile(decodeURIComponent(h.slice(9)));
- else if(h==='#/profile'&&ptoken)app.innerHTML=pageProfile('');
+ else if(h.indexOf('#/player/')===0&&S.showProfiles)app.innerHTML=pageProfile(decodeURIComponent(h.slice(9)));
+ else if(h==='#/profile'&&ptoken&&S.showProfiles)app.innerHTML=pageProfile('');
  else app.innerHTML=pageHome();
  /* Stop the 2s feed on the way OUT of the map. Without this a visitor who
     opened it once keeps polling for as long as the tab is open, from every

src/main/web/server.ts

@@ -38,6 +38,34 @@ import type { ProfileViewer } from '@shared/profile'
 
 /** Same shape the rest of the app validates a Minecraft name with. */
 const MC_NAME_RE = /^[A-Za-z0-9_]{3,16}$/
+
+/**
+ * The player roster, cached for a few seconds (#107).
+ *
+ * `getPlayers` parses every player's `.dat` with an NBT reader. Every other
+ * public route answers from memory; this one is the first that reads the disk,
+ * and the per-address budget allows ten requests a second — so without a cache
+ * one visitor turns a cheap HTTP request into the whole world folder being
+ * parsed, ten times a second, and a busy server has thousands of those files.
+ *
+ * The window is short because a profile showing a ten-second-old inventory is
+ * fine and one that pins the process is not. Shared across callers rather than
+ * per-address: the expensive part is the same work whoever asked for it.
+ */
+const rosterCache = new Map<string, { at: number; players: PlayerInfo[] }>()
+const ROSTER_TTL_MS = 10_000
+
+async function cachedRoster(serverId: string): Promise<PlayerInfo[]> {
+  const hit = rosterCache.get(serverId)
+  if (hit && Date.now() - hit.at < ROSTER_TTL_MS) return hit.players
+  const players = await playersMod.getPlayers(serverId).catch(() => [] as PlayerInfo[])
+  rosterCache.set(serverId, { at: Date.now(), players })
+  return players
+}
+
+export function _resetRosterCache(): void {
+  rosterCache.clear()
+}
 import * as metrics from '../core/metrics'
 import * as events from '../core/events'
 import * as alerts from '../core/alerts'
@@ -527,6 +555,8 @@ async function handlePublic(
 
   // ---- a player's profile (#107) ----
   //
+  // See `cachedRoster`: this is the only public route that touches the disk.
+  //
   // Who is asking decides what comes back, and the decision is a pure table in
   // @shared/profile because it is the whole security of the feature. Fields are
   // OMITTED, never sent-and-hidden: a page can be read with the network tab
@@ -543,11 +573,20 @@ async function handlePublic(
       : session.mcName.toLowerCase() === asked.toLowerCase()
         ? 'owner'
         : 'stranger'
-    // The roster read is the expensive part (it parses player .dat files), so
-    // it happens once and the decision runs over the result.
-    const roster = await playersMod.getPlayers(psid).catch(() => [])
+    const roster = await cachedRoster(psid)
     const p = roster.find((x) => x.name.toLowerCase() === asked.toLowerCase())
-    if (!p && !playerAuth.isRegistered(asked)) return sendJson(res, 404, { error: 'not-found' })
+    // Existence is decided by the ROSTER alone for anyone but the owner.
+    //
+    // Consulting `isRegistered` here for a stranger would make 200-vs-404
+    // answer "does this name have a website account?" for every name that has
+    // never played — the same enumeration oracle closed in #105, reopened in a
+    // different endpoint. The owner is already authenticated as that name, so
+    // telling them their own account exists reveals nothing, and it is the only
+    // way someone who registered on a server that has never run can see their
+    // own profile.
+    if (!p && !(viewer === 'owner' && playerAuth.isRegistered(asked))) {
+      return sendJson(res, 404, { error: 'not-found' })
+    }
     return sendJson(
       res,
       200,