docs: archive the 2026-06-10 architecture + security review decks

Both were sitting untracked in the repo root. They're the source for the "candidate N from the 2026-06-10 review" references in this batch of follow-up PRs, so keeping them in-repo makes those references resolvable. Self-contained HTML; a short README indexes them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-06-11 08:33:48 +02:00 · 2026-06-11 08:33:48 +02:00 · 10a3a6f1c4
commit 10a3a6f1c4
parent b4067301cc
3 changed files with 1058 additions and 0 deletions
--- a/docs/reviews/README.md
+++ b/docs/reviews/README.md
@ -0,0 +1,17 @@
+# Reviews
+
+Point-in-time review decks. Self-contained HTML — open directly in a
+browser (arrow keys / click to navigate). Each is a snapshot of its
+date; findings were triaged and tracked through PRs, so the deck reads
+as the "why" behind a batch of follow-up work rather than a live
+checklist.
+
+- **`architecture-review-2026-06-10.html`** — deepening opportunities
+  (shallow modules, implicit interfaces, duplicated choreography). The
+  10 candidates were implemented across PRs that reference them as
+  "candidate N from the 2026-06-10 review".
+
+- **`security-review-2026-06-10.html`** — defensive review of the auth,
+  federation, secrets, and injection surfaces. Severity-calibrated
+  (several scanner false-positives were verified and dropped). All
+  actionable findings were fixed in follow-up PRs.
--- a/docs/reviews/architecture-review-2026-06-10.html
+++ b/docs/reviews/architecture-review-2026-06-10.html
@ -0,0 +1,586 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+<meta charset="UTF-8">
+<meta name="viewport" content="width=device-width, initial-scale=1.0">
+<title>trails.cool — Architecture Review (2026-06-10)</title>
+<style>
+  :root {
+    --bg: #11151c;
+    --slide-bg: #181e28;
+    --text: #e8ecf2;
+    --muted: #9aa7b8;
+    --accent: #5fb88a;
+    --accent2: #6ea8d8;
+    --warn: #e0a85c;
+    --danger: #d87a6e;
+    --code-bg: #232b38;
+    --border: #2c3645;
+  }
+  * { margin: 0; padding: 0; box-sizing: border-box; }
+  html, body { height: 100%; background: var(--bg); color: var(--text);
+    font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif;
+    overflow: hidden; }
+  .slide {
+    display: none; position: absolute; inset: 0;
+    padding: 56px 80px 80px;
+    background: var(--slide-bg);
+    flex-direction: column;
+    overflow-y: auto;
+  }
+  .slide.active { display: flex; }
+  .kicker {
+    font-size: 14px; letter-spacing: 0.14em; text-transform: uppercase;
+    color: var(--accent); font-weight: 600; margin-bottom: 14px;
+  }
+  .kicker .num { color: var(--muted); font-weight: 400; }
+  h1 { font-size: 52px; font-weight: 750; line-height: 1.1; margin-bottom: 18px; letter-spacing: -0.01em; }
+  h2 { font-size: 36px; font-weight: 700; line-height: 1.15; margin-bottom: 24px; letter-spacing: -0.01em; }
+  h3 { font-size: 17px; font-weight: 700; text-transform: uppercase; letter-spacing: 0.08em;
+       margin: 18px 0 6px; }
+  p, li { font-size: 19px; line-height: 1.55; color: var(--text); }
+  .sub { color: var(--muted); font-size: 22px; max-width: 900px; }
+  ul { padding-left: 24px; margin: 6px 0 8px; }
+  li { margin-bottom: 7px; }
+  code {
+    font-family: "SF Mono", ui-monospace, Menlo, Consolas, monospace;
+    font-size: 0.86em; background: var(--code-bg); padding: 2px 7px;
+    border-radius: 5px; color: #c9d6e8; white-space: nowrap;
+  }
+  .files { margin: 4px 0 4px; }
+  .files code { display: inline-block; margin: 2px 6px 2px 0; white-space: normal; }
+  .problem h3 { color: var(--danger); }
+  .solution h3 { color: var(--accent); }
+  .benefits h3 { color: var(--accent2); }
+  .filesblock h3 { color: var(--warn); }
+  .tag {
+    display: inline-block; font-size: 13px; font-weight: 700; letter-spacing: 0.06em;
+    text-transform: uppercase; padding: 4px 12px; border-radius: 999px;
+    margin-left: 14px; vertical-align: middle;
+  }
+  .tag.defect { background: rgba(216,122,110,.16); color: var(--danger); border: 1px solid rgba(216,122,110,.4); }
+  .tag.leverage { background: rgba(95,184,138,.14); color: var(--accent); border: 1px solid rgba(95,184,138,.4); }
+  .tag.testability { background: rgba(110,168,216,.14); color: var(--accent2); border: 1px solid rgba(110,168,216,.4); }
+  .tag.cleanup { background: rgba(224,168,92,.14); color: var(--warn); border: 1px solid rgba(224,168,92,.4); }
+  .cols { display: grid; grid-template-columns: 1fr 1fr; gap: 12px 48px; flex: 1; align-content: start; }
+  .cols .full { grid-column: 1 / -1; }
+  table { border-collapse: collapse; margin-top: 18px; width: 100%; }
+  th, td { text-align: left; padding: 9px 16px; font-size: 17px; border-bottom: 1px solid var(--border); }
+  th { color: var(--muted); font-weight: 600; text-transform: uppercase; font-size: 13px; letter-spacing: 0.07em; }
+  td.n { color: var(--accent); font-weight: 700; }
+  .footer {
+    position: fixed; bottom: 0; left: 0; right: 0; height: 44px;
+    display: flex; align-items: center; justify-content: space-between;
+    padding: 0 28px; font-size: 13px; color: var(--muted);
+    background: var(--bg); border-top: 1px solid var(--border); z-index: 10;
+  }
+  .progress { position: fixed; top: 0; left: 0; height: 3px; background: var(--accent);
+    transition: width .25s ease; z-index: 11; }
+  .hint kbd {
+    font-family: inherit; background: var(--code-bg); border: 1px solid var(--border);
+    border-radius: 4px; padding: 1px 7px; font-size: 12px; margin: 0 2px;
+  }
+  .titleslide { justify-content: center; }
+  .titleslide h1 { font-size: 64px; }
+  .toc li { margin-bottom: 4px; font-size: 18px; }
+  .toc .sec { color: var(--muted); font-size: 14px; text-transform: uppercase; letter-spacing: .08em;
+    margin: 14px 0 6px; list-style: none; margin-left: -24px; font-weight: 700; }
+  blockquote {
+    border-left: 3px solid var(--border); padding: 6px 0 6px 18px; margin: 12px 0;
+    color: var(--muted); font-size: 18px; line-height: 1.5;
+  }
+  .verified { color: var(--accent); font-weight: 600; }
+</style>
+</head>
+<body>
+
+<div class="progress" id="progress"></div>
+
+<!-- 1: Title -->
+<section class="slide titleslide active">
+  <div class="kicker">trails.cool · architecture review</div>
+  <h1>Deepening opportunities</h1>
+  <p class="sub">A broad repo scan for architectural friction — shallow modules, implicit interfaces, duplicated choreography — with sketches for turning them into deep modules.</p>
+  <p class="sub" style="margin-top:28px; font-size:17px;">2026-06-10 · four parallel codebase surveys (Journal, Planner, packages, cross-cutting) · key claims verified against source · checked against CONTEXT.md and ADR-0001…0006</p>
+</section>
+
+<!-- 2: Method / vocabulary -->
+<section class="slide">
+  <div class="kicker">Method</div>
+  <h2>How to read these slides</h2>
+  <div class="cols">
+    <div>
+      <h3 style="color:var(--accent2)">Vocabulary</h3>
+      <ul>
+        <li><strong>Module</strong> — anything with an interface and an implementation.</li>
+        <li><strong>Deep module</strong> — a lot of behaviour behind a small interface.</li>
+        <li><strong>Shallow module</strong> — interface nearly as complex as the implementation.</li>
+        <li><strong>Seam</strong> — where an interface lives; behaviour can change without editing in place.</li>
+        <li><strong>Locality</strong> — change, bugs, and knowledge concentrated in one place.</li>
+        <li><strong>Leverage</strong> — what callers get from depth.</li>
+      </ul>
+    </div>
+    <div>
+      <h3 style="color:var(--accent)">The deletion test</h3>
+      <p>Imagine deleting the module. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep.</p>
+      <h3 style="color:var(--warn)">Already settled (not re-proposed)</h3>
+      <ul>
+        <li>No unified <code>SyncProvider</code> (ADR-0002)</li>
+        <li>No <code>AuthMethod</code> polymorphism (ADR-0005)</li>
+        <li>gpx-save owns geometry atomically (ADR-0006)</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 3: Overall map -->
+<section class="slide">
+  <div class="kicker">Overall map</div>
+  <h2>Where the codebase is deep — and where it isn't</h2>
+  <div class="cols">
+    <div>
+      <h3 style="color:var(--accent)">Deep, earning their keep</h3>
+      <ul>
+        <li><strong>Journal core seams</strong> — gpx-save, <code>completeAuth</code>, ConnectedServiceManager, capability seams (Importer / RoutePusher / WebhookReceiver).</li>
+        <li><strong>Planner pure modules</strong> — BRouter client, route-merge, segment-cache, host election. Well tested.</li>
+        <li><strong>packages/gpx &amp; packages/db</strong> — load-bearing, most-imported, best-tested.</li>
+        <li><strong>Federation</strong> — ~3,200 lines of deliberate Fedify isolation.</li>
+      </ul>
+    </div>
+    <div>
+      <h3 style="color:var(--danger)">Shallow, causing friction</h3>
+      <ul>
+        <li><strong>Journal route-handler layer</strong> — 110 handlers, ~5 tests; scattered ownership checks, hand-rolled form parsing.</li>
+        <li><strong>Planner Yjs document schema</strong> — the app's most important interface, enforced by convention only (~30 raw call sites).</li>
+        <li><strong>Domain types</strong> — three drifting sources of truth (types / api / db).</li>
+        <li><strong>Jobs, SSE, notifications</strong> — interfaces discovered by grep, not by type.</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 4: TOC -->
+<section class="slide">
+  <div class="kicker">Agenda</div>
+  <h2>Ten candidates</h2>
+  <div class="cols">
+    <ul class="toc">
+      <li class="sec">Closest to a real defect</li>
+      <li><strong>1.</strong> Komoot bulk import smuggles credentials past the manager</li>
+      <li class="sec">Highest-leverage refactors</li>
+      <li><strong>2.</strong> A typed job seam — payloads are <code>unknown</code> end-to-end</li>
+      <li><strong>3.</strong> Planner: the Yjs routeData schema is an implicit interface</li>
+      <li><strong>5.</strong> Journal: route/activity saves duplicate GPX choreography</li>
+      <li><strong>6.</strong> Three drifting sources of truth for Route/Activity</li>
+    </ul>
+    <ul class="toc">
+      <li class="sec">Testability &amp; safety</li>
+      <li><strong>7.</strong> Ownership checks scattered across handlers and lib</li>
+      <li><strong>8.</strong> OAuth connect→callback→resume spread over three routes</li>
+      <li class="sec">Smaller wins</li>
+      <li><strong>4.</strong> Planner GPX assembly duplicated (Save vs Export)</li>
+      <li><strong>9.</strong> Shim &amp; dead-weight packages: <code>map</code>, <code>ui</code></li>
+      <li><strong>10.</strong> E2E suite: 23 spec files re-derive their own setup</li>
+    </ul>
+  </div>
+</section>
+
+<!-- 5: Candidate 1 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 1 / 10</span></div>
+  <h2>Komoot bulk import smuggles credentials past the&nbsp;manager <span class="tag defect">defect-adjacent</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>apps/journal/app/routes/api.sync.komoot.import.ts:34</code> <code>apps/journal/app/jobs/komoot-bulk-import.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> the route enqueues <code>creds: service.credentials</code> — the raw JSONB — in the pg-boss payload. This bypasses <code>withFreshCredentials</code>, the ConnectedServiceManager's whole reason to exist:</p>
+      <ul>
+        <li>Credentials sit at rest in the jobs table.</li>
+        <li>No refresh if they go stale between enqueue and execution.</li>
+        <li><code>markNeedsRelink</code> never fires on failure.</li>
+      </ul>
+      <p>The Wahoo path goes through the manager correctly; Komoot is the one defector from the seam.</p>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>Jobs carry only the <code>serviceId</code>. The handler resolves fresh credentials through ConnectedServiceManager at execution time — identical to every other capability caller.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li><strong>Locality:</strong> all credential-lifecycle bugs live in one module again.</li>
+          <li>No credentials-at-rest in the jobs table.</li>
+          <li>Handler becomes testable with a fake manager instead of fixture credential blobs.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 6: Candidate 2 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 2 / 10</span></div>
+  <h2>A typed job seam — payloads are <code>unknown</code> end-to-end <span class="tag leverage">high leverage</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>packages/jobs/src/types.ts</code> <code>apps/journal/app/jobs/</code> (~14 handlers) · ~7 enqueue sites</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> <code>JobDefinition.handler</code> is <code>(jobs: Job&lt;unknown&gt;[]) =&gt; Promise&lt;unknown&gt;</code>.</p>
+      <ul>
+        <li>Every handler opens with <code>item.data as SomePayload</code>; every enqueue passes a bare string queue name + unchecked object.</li>
+        <li>A typo or payload-shape change is a runtime failure in a background worker.</li>
+        <li><code>unknown</code> exists for a documented contravariance reason — but that's an implementation constraint leaking into the interface.</li>
+        <li><code>enqueueOptional</code> swallows queue failures: "saved but fan-out never queued" looks like success.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>A <code>JobPayloads</code> map type (<code>'notifications-fanout': { activityId: string }</code>, …) with typed <code>enqueue&lt;K&gt;</code> and typed handler registration. The contravariance cast happens <em>once, inside the package</em> — not fourteen times at the edges.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li><strong>Leverage:</strong> enqueue call sites can't drift from handlers.</li>
+          <li><strong>Locality:</strong> retry/backoff policy and queue-name validity in one registry.</li>
+          <li>Handlers become unit-testable with typed fixtures.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 7: Candidate 3 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 3 / 10</span></div>
+  <h2>Planner: the Yjs routeData schema is an implicit interface <span class="tag leverage">high leverage</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>apps/planner/app/lib/use-yjs.ts</code> <code>use-routing.ts</code> <code>use-waypoint-manager.ts</code> <code>use-elevation-data.ts</code> + components (<code>SessionView</code>, <code>ProfileSelector</code>, <code>ElevationChart</code>, …)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> ~15 string-keyed entries (<code>geojson</code>, <code>coordinates</code>, <code>segmentBoundaries</code>, <code>surfaces</code>, <code>highways</code>, <code>maxspeeds</code>, …) read at ~30 raw call sites.</p>
+      <ul>
+        <li>No module owns the schema — consumers do <code>routeData.get("surfaces") as string | undefined</code> and re-parse JSON.</li>
+        <li><code>parseJsonArray</code> copy-pasted in two hooks with different signatures; waypoint extraction written four times.</li>
+        <li>Adding one road attribute touches ~7 files (enrichment → storage → parsing → rendering).</li>
+      </ul>
+      <p>The document schema is the Planner's most important interface — enforced by convention only.</p>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>A routeData schema module: typed read/write of the whole document state, JSON encoding internal — analogous to what <code>waypoint-ymap.ts</code> already does well for waypoints. Hooks and components consume a <code>RouteDataState</code>, never raw <code>Y.Map</code> keys.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li><strong>Locality:</strong> schema changes become one-file edits.</li>
+          <li>Hooks testable against a plain object instead of a live Yjs doc.</li>
+          <li>Deletion test passes clearly: delete it and key-string knowledge reappears at 30 call sites.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 8: Candidate 4 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 4 / 10</span></div>
+  <h2>Planner GPX assembly duplicated between Save and&nbsp;Export <span class="tag cleanup">small win</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>apps/planner/app/components/SaveToJournalButton.tsx</code> <code>apps/planner/app/components/ExportButton.tsx</code></p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li>Both buttons independently extract geojson / waypoints / no-go areas / notes from Yjs and build GPX.</li>
+        <li>ExportButton additionally handles multi-day splitting.</li>
+        <li>Same extraction, two implementations — a change to what gets persisted (e.g. waypoint notes in GPX extensions) needs two synchronized edits.</li>
+        <li>Currently zero tests on either path.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>One <code>buildGpxData(yjs)</code> module — naturally layered on candidate&nbsp;3 — consumed by both buttons and any future export path.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li>The Planner→Journal handoff payload and the file export can no longer diverge.</li>
+          <li>The assembly becomes a pure, testable function.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 9: Candidate 5 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 5 / 10</span></div>
+  <h2>Journal: <code>createRoute</code> / <code>createActivity</code> duplicate the GPX choreography <span class="tag leverage">high leverage</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>apps/journal/app/lib/routes.server.ts</code> (~24–78) <code>apps/journal/app/lib/activities.server.ts</code> (~68–122) <code>apps/journal/app/lib/gpx-save.server.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li>Both callers independently run validateGpx → extract distance/elevation stats → transaction → writeGeom, in near-identical ~45-line blocks.</li>
+        <li>Includes the "stats may be pre-supplied" handling the demo bot relies on — duplicated too.</li>
+        <li>The gpx-save module owns validation and geometry but <em>not</em> the stat extraction between them, so the choreography is copied at every save entry point. A third entity type would copy it again.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>Deepen the gpx-save module to own the full validate-and-derive step: one function returning parsed GPX + stats (accepting precomputed stats), leaving callers with only entity-specific row shapes. <strong>Extends ADR-0006 rather than contradicting it.</strong></p>
+        <blockquote>Worth grilling: should basic validation move down into <code>packages/gpx</code>, which already owns parsing and stat computation?</blockquote>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li>New validation rules or stat definitions become one edit.</li>
+          <li>"Every saved GPX has consistent stats" gets a single testable home.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 10: Candidate 6 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 6 / 10</span></div>
+  <h2>Three drifting sources of truth for Route/Activity shapes <span class="tag leverage">high leverage</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>packages/types/src/index.ts</code> <code>packages/api/src/routes.ts</code> (Zod) <code>packages/db/src/schema/journal.ts</code> (Drizzle)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li><code>Route</code> exists three times: hand-written interface (types), Zod <code>RouteSummary</code>/<code>RouteDetail</code> (api), Drizzle columns (db) — each with different fields and nullability.</li>
+        <li>Route handlers hand-roll the mapping (e.g. <code>api.v1.routes._index.ts</code> constructs response JSON manually, not through the schema).</li>
+        <li>The api package's contracts are <em>advisory</em>: nothing fails if a handler's response drifts from <code>RouteListResponse</code>.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>Pick one canonical source — likely derive TS types from the Drizzle schema, have the api Zod schemas reference them — and validate responses against the contract at the seam (typed response helpers, or validation in the v1 handlers).</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li><strong>Leverage</strong> for both apps and mobile: one definition of Route.</li>
+          <li>The 312 schema tests in <code>packages/api</code> start guarding real traffic instead of just the schemas themselves.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 11: Candidate 7 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 7 / 10</span></div>
+  <h2>Ownership checks scattered across handlers and lib modules <span class="tag testability">testability</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>routes.$id.edit.server.ts</code> <code>push-action.server.ts:54</code> <code>activities.server.ts</code> (mutators that <em>assume</em> the caller checked) · others</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li><code>route.ownerId !== user.id</code> is re-checked in some loaders, re-checked in some lib functions, absent in others where the precondition is implicit.</li>
+        <li>Nothing in the types distinguishes "a Route" from "a Route this user may mutate" — every new call site re-derives, or forgets, the check.</li>
+        <li>Contrast: the Terms gate <em>is</em> properly centralized. Ownership deserves the same treatment.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>A <code>loadOwnedRoute(routeId, userId)</code> (and activity twin) as the single enforcement point, returning a branded type that mutators require — making bypass a compile error rather than a code-review catch.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li>Authorization gets the locality that <code>completeAuth</code> gave session minting.</li>
+          <li>The 403 behavior becomes testable once, instead of per-handler.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 12: Candidate 8 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 8 / 10</span></div>
+  <h2>OAuth connect → callback → resume spread over three routes <span class="tag testability">testability</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>api.sync.connect.$provider.ts</code> <code>api.sync.callback.$provider.ts</code> <code>api.sync.push.$provider.$routeId.ts</code> <code>oauth-state.server.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li>PKCE verifier cookies, state encoding, and push-resumption are coordinated across three route handlers, each knowing part of the protocol (which cookie, when it's set, what's in the state blob).</li>
+        <li>State encode/decode is extracted — but the <em>lifecycle</em> isn't.</li>
+        <li>Adding Garmin or Strava OAuth means re-learning the choreography from the existing routes. CONTEXT.md forecasts three more OAuth providers.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>An OAuth flow module owning <strong>initiate</strong> (returns redirect with cookies set) and <strong>complete</strong> (consumes request, returns exchange result + decoded state). Routes shrink to thin adapters.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li>Each future provider reuses the flow, not the pattern.</li>
+          <li>The PKCE handshake becomes testable without a browser.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 13: Candidate 9 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 9 / 10</span></div>
+  <h2>Shim &amp; dead-weight packages: <code>map</code> and <code>ui</code> <span class="tag cleanup">cleanup</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>packages/map</code> (79 lines: re-exports + a 35-line MapView) <code>packages/ui</code> (162 lines)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p>Both fail the deletion test in the telling direction — deleting them moves almost nothing:</p>
+      <ul>
+        <li><code>map</code> adds an import-ambiguity tax: apps import from <code>map</code> and <code>map-core</code> inconsistently (~21 vs ~19 sites).</li>
+        <li><span class="verified">Verified:</span> <code>@trails-cool/ui</code> is imported <strong>only</strong> by the two apps' <code>root.tsx</code> — for the stylesheet. Button/Input/Card have zero consumers; both apps roll their own buttons inline.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <ul>
+          <li>Fold MapView into a clearly-React entry of map-core, <em>or</em> make <code>map</code> the only import surface for apps.</li>
+          <li>Either delete <code>ui</code> or commit to it as a real design system — the half-state is the worst option.</li>
+        </ul>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li>Less code falsely suggesting reuse.</li>
+          <li>One obvious import path for map code.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 14: Candidate 10 -->
+<section class="slide">
+  <div class="kicker"><span class="num">candidate 10 / 10</span></div>
+  <h2>E2E suite: 23 spec files re-derive their own setup <span class="tag testability">testability</span></h2>
+  <div class="cols">
+    <div class="full filesblock">
+      <h3>Files</h3>
+      <p class="files"><code>e2e/</code> — seed-route boilerplate in 5+ files · auth helpers defined inside <code>auth.test.ts</code> · <code>http://localhost:3000/3001</code> hardcoded in 10+ files</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li>The helpers module is nearly empty, so each spec carries its own fixtures.</li>
+        <li>Virtual-authenticator and register/login utilities live in a test file other specs can't cleanly import.</li>
+        <li>The stated e2e strategy (shared setup helpers, BRouter mocked by default) is documented — but not embodied in a module.</li>
+      </ul>
+    </div>
+    <div>
+      <div class="solution">
+        <h3>Solution</h3>
+        <p>A real <code>e2e/lib/</code> fixtures module: <code>seedRoute()</code>, <code>registerAndLogin()</code>, base-URL constants, authenticator setup — imported by every spec.</p>
+      </div>
+      <div class="benefits">
+        <h3>Benefits</h3>
+        <ul>
+          <li>New specs start at the interesting assertion, not at boilerplate.</li>
+          <li>URL/port changes become one edit.</li>
+        </ul>
+      </div>
+    </div>
+  </div>
+</section>
+
+<!-- 15: Dropped -->
+<section class="slide">
+  <div class="kicker">Scope discipline</div>
+  <h2>Noted but deliberately dropped</h2>
+  <ul style="max-width:980px">
+    <li style="margin-bottom:18px"><strong>A <code>Result&lt;T,E&gt;</code>-everywhere error-handling rewrite</strong> — too sweeping; the narrower wins live inside candidates 5 and 7.</li>
+    <li style="margin-bottom:18px"><strong>An event-driven capability plugin system for providers</strong> — this re-litigates ADR-0002's "capabilities are separate seams" without new friction to justify reopening it.</li>
+    <li style="margin-bottom:18px"><strong>ADR check:</strong> nothing in this deck conflicts with ADR-0001…0006. Candidate 5 deepens ADR-0006's module in place.</li>
+  </ul>
+</section>
+
+<!-- 16: Summary -->
+<section class="slide">
+  <div class="kicker">Summary</div>
+  <h2>Where to start</h2>
+  <table>
+    <tr><th>#</th><th>Candidate</th><th>Why it's first-tier</th></tr>
+    <tr><td class="n">1</td><td>Komoot credentials bypass</td><td>Closest to a real defect — credentials at rest, no refresh, seam violated</td></tr>
+    <tr><td class="n">3</td><td>Planner routeData schema module</td><td>Highest-leverage refactor — 30 call sites, ~7-file blast radius per attribute</td></tr>
+    <tr><td class="n">2</td><td>Typed job seam</td><td>Best testability-per-effort — one registry, fourteen handlers gain types</td></tr>
+    <tr><td class="n">7</td><td>Branded ownership loading</td><td>Best safety-per-effort — bypass becomes a compile error</td></tr>
+  </table>
+  <p style="margin-top:32px; color:var(--muted)">Then: 5 &amp; 6 (one source of truth for GPX stats and domain types), 8 (before the next OAuth provider lands), 4 / 9 / 10 as opportunistic cleanups.</p>
+</section>
+
+<div class="footer">
+  <span>trails.cool architecture review · 2026-06-10</span>
+  <span id="counter"></span>
+  <span class="hint"><kbd>←</kbd><kbd>→</kbd> or click · <kbd>Home</kbd>/<kbd>End</kbd></span>
+</div>
+
+<script>
+  const slides = Array.from(document.querySelectorAll('.slide'));
+  let i = 0;
+  function show(n) {
+    i = Math.max(0, Math.min(slides.length - 1, n));
+    slides.forEach((s, j) => s.classList.toggle('active', j === i));
+    document.getElementById('counter').textContent = (i + 1) + ' / ' + slides.length;
+    document.getElementById('progress').style.width = ((i + 1) / slides.length * 100) + '%';
+    location.hash = i + 1;
+  }
+  document.addEventListener('keydown', (e) => {
+    if (e.key === 'ArrowRight' || e.key === ' ' || e.key === 'PageDown') show(i + 1);
+    else if (e.key === 'ArrowLeft' || e.key === 'PageUp') show(i - 1);
+    else if (e.key === 'Home') show(0);
+    else if (e.key === 'End') show(slides.length - 1);
+  });
+  document.addEventListener('click', (e) => {
+    if (window.getSelection().toString()) return;
+    show(e.clientX > window.innerWidth / 2 ? i + 1 : i - 1);
+  });
+  show(parseInt(location.hash.slice(1), 10) - 1 || 0);
+</script>
+</body>
+</html>
--- a/docs/reviews/security-review-2026-06-10.html
+++ b/docs/reviews/security-review-2026-06-10.html
@ -0,0 +1,455 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+<meta charset="UTF-8">
+<meta name="viewport" content="width=device-width, initial-scale=1.0">
+<title>trails.cool — Security Review (2026-06-10)</title>
+<style>
+  :root {
+    --bg: #11151c;
+    --slide-bg: #181e28;
+    --text: #e8ecf2;
+    --muted: #9aa7b8;
+    --accent: #5fb88a;
+    --accent2: #6ea8d8;
+    --warn: #e0a85c;
+    --danger: #d87a6e;
+    --code-bg: #232b38;
+    --border: #2c3645;
+  }
+  * { margin: 0; padding: 0; box-sizing: border-box; }
+  html, body { height: 100%; background: var(--bg); color: var(--text);
+    font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif;
+    overflow: hidden; }
+  .slide {
+    display: none; position: absolute; inset: 0;
+    padding: 52px 76px 76px;
+    background: var(--slide-bg);
+    flex-direction: column;
+    overflow-y: auto;
+  }
+  .slide.active { display: flex; }
+  .kicker {
+    font-size: 14px; letter-spacing: 0.14em; text-transform: uppercase;
+    color: var(--accent2); font-weight: 600; margin-bottom: 14px;
+  }
+  .kicker .num { color: var(--muted); font-weight: 400; }
+  h1 { font-size: 52px; font-weight: 750; line-height: 1.1; margin-bottom: 18px; letter-spacing: -0.01em; }
+  h2 { font-size: 33px; font-weight: 700; line-height: 1.15; margin-bottom: 20px; letter-spacing: -0.01em; }
+  h3 { font-size: 16px; font-weight: 700; text-transform: uppercase; letter-spacing: 0.08em; margin: 16px 0 6px; }
+  p, li { font-size: 18px; line-height: 1.55; color: var(--text); }
+  .sub { color: var(--muted); font-size: 22px; max-width: 920px; }
+  ul { padding-left: 24px; margin: 6px 0 8px; }
+  li { margin-bottom: 6px; }
+  code {
+    font-family: "SF Mono", ui-monospace, Menlo, Consolas, monospace;
+    font-size: 0.85em; background: var(--code-bg); padding: 2px 7px;
+    border-radius: 5px; color: #c9d6e8; white-space: nowrap;
+  }
+  pre { background: var(--code-bg); border: 1px solid var(--border); border-radius: 8px;
+    padding: 14px 16px; margin: 8px 0; overflow-x: auto; }
+  pre code { background: none; padding: 0; white-space: pre; font-size: 14px; line-height: 1.5; }
+  .files { margin: 4px 0; }
+  .files code { white-space: normal; }
+  .problem h3 { color: var(--danger); }
+  .solution h3 { color: var(--accent); }
+  .why h3 { color: var(--warn); }
+  .tag {
+    display: inline-block; font-size: 12px; font-weight: 700; letter-spacing: 0.05em;
+    text-transform: uppercase; padding: 4px 11px; border-radius: 999px;
+    margin-left: 12px; vertical-align: middle;
+  }
+  .sev-high { background: rgba(216,122,110,.16); color: var(--danger); border: 1px solid rgba(216,122,110,.45); }
+  .sev-med { background: rgba(224,168,92,.15); color: var(--warn); border: 1px solid rgba(224,168,92,.45); }
+  .sev-low { background: rgba(110,168,216,.14); color: var(--accent2); border: 1px solid rgba(110,168,216,.4); }
+  .sev-info { background: rgba(154,167,184,.12); color: var(--muted); border: 1px solid rgba(154,167,184,.35); }
+  .sev-good { background: rgba(95,184,138,.14); color: var(--accent); border: 1px solid rgba(95,184,138,.4); }
+  .cols { display: grid; grid-template-columns: 1fr 1fr; gap: 10px 48px; flex: 1; align-content: start; }
+  .cols .full { grid-column: 1 / -1; }
+  table { border-collapse: collapse; margin-top: 14px; width: 100%; }
+  th, td { text-align: left; padding: 8px 14px; font-size: 16px; border-bottom: 1px solid var(--border); vertical-align: top; }
+  th { color: var(--muted); font-weight: 600; text-transform: uppercase; font-size: 12px; letter-spacing: 0.07em; }
+  td.sev { font-weight: 700; white-space: nowrap; }
+  .s-high { color: var(--danger); } .s-med { color: var(--warn); } .s-low { color: var(--accent2); } .s-info { color: var(--muted); } .s-good { color: var(--accent); }
+  .footer {
+    position: fixed; bottom: 0; left: 0; right: 0; height: 42px;
+    display: flex; align-items: center; justify-content: space-between;
+    padding: 0 26px; font-size: 13px; color: var(--muted);
+    background: var(--bg); border-top: 1px solid var(--border); z-index: 10;
+  }
+  .progress { position: fixed; top: 0; left: 0; height: 3px; background: var(--accent2); transition: width .25s ease; z-index: 11; }
+  .hint kbd { font-family: inherit; background: var(--code-bg); border: 1px solid var(--border); border-radius: 4px; padding: 1px 7px; font-size: 12px; margin: 0 2px; }
+  .titleslide { justify-content: center; }
+  .titleslide h1 { font-size: 60px; }
+  .toc li { margin-bottom: 5px; font-size: 17px; }
+  .toc .sec { color: var(--muted); font-size: 13px; text-transform: uppercase; letter-spacing: .08em; margin: 14px 0 5px; list-style: none; margin-left: -24px; font-weight: 700; }
+  blockquote { border-left: 3px solid var(--border); padding: 6px 0 6px 18px; margin: 10px 0; color: var(--muted); font-size: 17px; line-height: 1.5; }
+  .verified { color: var(--accent); font-weight: 600; }
+  .recal { color: var(--warn); font-weight: 600; }
+  .lead { font-size: 20px; max-width: 940px; margin-bottom: 6px; }
+</style>
+</head>
+<body>
+
+<div class="progress" id="progress"></div>
+
+<!-- 1: Title -->
+<section class="slide titleslide active">
+  <div class="kicker">trails.cool · security review</div>
+  <h1>Security review</h1>
+  <p class="sub">Defensive review of the auth, federation, secret-management, and injection surfaces across both apps and the infrastructure — findings verified against source and severity-calibrated.</p>
+  <p class="sub" style="margin-top:26px; font-size:17px;">2026-06-10 · four parallel review sweeps (auth/session/JWT · federation/SSRF · secrets/crypto · injection/uploads/infra) · every load-bearing claim re-checked against the code before inclusion</p>
+</section>
+
+<!-- 2: Method & honesty -->
+<section class="slide">
+  <div class="kicker">Method &amp; calibration</div>
+  <h2>How to read this — and what I threw out</h2>
+  <p class="lead">The automated sweeps over-flagged. I verified each high-severity claim against the source and <span class="recal">downgraded or dropped</span> the ones that didn't hold up. That recalibration is itself a finding: don't action the raw scanner output.</p>
+  <div class="cols" style="margin-top:8px">
+    <div class="problem">
+      <h3>Dropped / downgraded after verification</h3>
+      <ul>
+        <li><strong>"Critical: inbox signatures not verified"</strong> → Fedify verifies HTTP Signatures on inbox listeners <em>by default</em>. Real item is only a missing regression test. <span class="sev sev-info" style="margin:0">downgraded → Info</span></li>
+        <li><strong>"JWT alg-confusion / none"</strong> → <code>jose</code> with a symmetric key already rejects <code>alg:none</code> and asymmetric algs. Explicit allow-list is hardening only. <span class="sev sev-info" style="margin:0">→ Info</span></li>
+        <li><strong>"Magic-link 6-digit brute-force"</strong> → bounded by a per-IP catch-all (30/min) <em>and</em> per-email verify cap (10/15min) against a 15-min-rotating code. <span class="sev sev-info" style="margin:0">→ Info</span></li>
+        <li><strong>".env secrets committed"</strong> → <code>.env</code> is gitignored and never appears in history; values are dev creds. <span class="sev sev-info" style="margin:0">→ Info</span></li>
+      </ul>
+    </div>
+    <div class="solution">
+      <h3>What held up</h3>
+      <ul>
+        <li>One <strong>High</strong>: unauthenticated SSRF via the Planner session callback URL.</li>
+        <li>A handful of <strong>Medium/Low</strong> hardening gaps (upload validation, log redaction, Caddy admin bind, user enumeration).</li>
+        <li>A broad base of <strong>genuinely strong</strong> fundamentals (next-to-last slide).</li>
+      </ul>
+      <h3 style="color:var(--muted)">Severity legend</h3>
+      <p><span class="tag sev-high" style="margin:0">High</span> <span class="tag sev-med" style="margin:0">Medium</span> <span class="tag sev-low" style="margin:0">Low</span> <span class="tag sev-info" style="margin:0">Info</span></p>
+    </div>
+  </div>
+</section>
+
+<!-- 3: TOC -->
+<section class="slide">
+  <div class="kicker">Findings</div>
+  <h2>What to fix, in order</h2>
+  <div class="cols">
+    <ul class="toc">
+      <li class="sec">Act on these</li>
+      <li><strong>1.</strong> SSRF — Planner session callback URL <span class="tag sev-high" style="margin-left:6px">High</span></li>
+      <li><strong>2.</strong> Upload validation — content-type &amp; key <span class="tag sev-med" style="margin-left:6px">Medium</span></li>
+      <li><strong>3.</strong> OAuth callback logs the raw exception <span class="tag sev-low" style="margin-left:6px">Low</span></li>
+      <li><strong>4.</strong> Caddy admin API on <code>0.0.0.0</code> <span class="tag sev-low" style="margin-left:6px">Low</span></li>
+      <li><strong>5.</strong> User enumeration via auth errors <span class="tag sev-low" style="margin-left:6px">Low</span></li>
+    </ul>
+    <ul class="toc">
+      <li class="sec">Know about / track</li>
+      <li><strong>6.</strong> Federation hardening (tests, fetch caps) <span class="tag sev-low" style="margin-left:6px">Low</span></li>
+      <li><strong>7.</strong> Rate limiting is in-process / XFF-trusting <span class="tag sev-low" style="margin-left:6px">Low</span></li>
+      <li><strong>8.</strong> Komoot credential storage <span class="tag sev-info" style="margin-left:6px">Info</span></li>
+      <li class="sec">Calibration</li>
+      <li><strong>9.</strong> Strong fundamentals (the good news)</li>
+      <li><strong>10.</strong> Summary &amp; priorities</li>
+    </ul>
+  </div>
+</section>
+
+<!-- 4: Finding 1 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 1 / 8</span></div>
+  <h2>SSRF — unauthenticated Planner session callback URL <span class="tag sev-high">High</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/planner/app/routes/api.sessions.ts</code> · <code>apps/planner/app/routes/api.save-to-journal.ts</code> · helper <code>apps/planner/app/lib/url-validation.server.ts</code> (exists, unused here)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> <code>POST /api/sessions</code> is anonymous (the Planner is stateless) and stores the caller-supplied <code>callbackUrl</code> with no validation. On save, the Planner server fetches it:</p>
+      <pre><code>resp = await fetchWithTimeout(session.callbackUrl, {
+  method: "POST",
+  headers: { Authorization: `Bearer ${session.callbackToken}` },
+  ...</code></pre>
+      <p>Anyone can make the Planner backend issue POSTs to arbitrary hosts — internal services, <code>169.254.169.254</code>, localhost. A SAFE-scheme/host validator (<code>validateFetchUrl</code>) already exists in the repo but is not applied on this path.</p>
+    </div>
+    <div class="solution">
+      <h3>Fix</h3>
+      <p>Validate <code>callbackUrl</code> at session-create against the existing allowlist helper, rejecting non-HTTP(S) schemes and private/loopback/link-local resolutions:</p>
+      <pre><code>const v = validateFetchUrl(callbackUrl, {
+  allowedHosts: getCallbackAllowedHosts(),
+});
+if (!v.ok) return data(
+  { error: "Invalid callback URL" },
+  { status: 400 },
+);</code></pre>
+      <p>Re-validate at fetch time too (defends against DNS rebinding between create and save).</p>
+    </div>
+  </div>
+</section>
+
+<!-- 5: Finding 2 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 2 / 8</span></div>
+  <h2>Upload validation — no content-type allowlist, raw filename in key <span class="tag sev-med">Medium</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/journal/app/routes/api.v1.uploads.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> the presigned-upload endpoint builds the S3 key from the raw client filename and accepts any content-type:</p>
+      <pre><code>const key =
+  `${resourceType}/${resourceId}/${randomUUID()}-${filename}`;</code></pre>
+      <ul>
+        <li>No content-type allowlist — a user can store HTML/SVG under an image resource; <strong>stored XSS</strong> if those bytes are ever served inline rather than as attachments.</li>
+        <li>Raw <code>filename</code> in the key lets a caller shape arbitrary key prefixes (the UUID prevents collision/overwrite, and S3 keys aren't a filesystem, so this is shaping not traversal).</li>
+        <li>Confirm <code>resourceId</code> ownership is checked so a user can't mint upload URLs under another user's resource path.</li>
+      </ul>
+    </div>
+    <div class="solution">
+      <h3>Fix</h3>
+      <ul>
+        <li>Allowlist <code>contentType</code> (e.g. <code>image/jpeg</code>, <code>image/png</code>, <code>application/gpx+xml</code>) and bind it into the presign so the upload can't differ.</li>
+        <li>Sanitize <code>filename</code> to <code>[A-Za-z0-9._-]</code> or drop it from the key entirely (the UUID is enough).</li>
+        <li>Serve user content with <code>Content-Disposition: attachment</code> / from a separate origin, and verify <code>resourceId</code> belongs to the caller.</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 6: Finding 3 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 3 / 8</span></div>
+  <h2>OAuth callback logs the raw exception <span class="tag sev-low">Low</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/journal/app/lib/connected-services/oauth-flow.server.ts</code> (~line 113) · pattern also in <code>manager.ts</code> (<code>markNeedsRelink</code> reason)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> the code-exchange failure path logs the whole error object:</p>
+      <pre><code>} catch (e) {
+  console.error(
+    `OAuth callback failed for ${manifest.id}:`, e);</code></pre>
+      <p>If a provider's error response embeds the authorization code, a token, or other sensitive context in the thrown error, it lands in server logs and Sentry. The blast radius is small (only the failure branch, and the affected user is the token's owner), hence Low — but credentials in logs are worth closing.</p>
+    </div>
+    <div class="solution">
+      <h3>Fix</h3>
+      <pre><code>console.error(
+  `OAuth callback failed for ${manifest.id}:`,
+  e instanceof Error ? e.message : String(e),
+);</code></pre>
+      <p>Log a redacted shape (name + truncated message), never the raw object. Apply the same to <code>markNeedsRelink</code>'s provider-supplied <code>reason</code>.</p>
+    </div>
+  </div>
+</section>
+
+<!-- 7: Finding 4 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 4 / 8</span></div>
+  <h2>Caddy admin API bound to <code>0.0.0.0:2019</code> <span class="tag sev-low">Low</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>infrastructure/Caddyfile</code> (line 3)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> <code>admin 0.0.0.0:2019</code>. The port isn't published to the host, so it's not internet-reachable — but it <em>is</em> reachable by every container on the Docker network. The Caddy admin API can rewrite routes and reverse-proxy targets, so an RCE in the journal or planner container becomes "redirect all traffic" with no extra auth.</p>
+    </div>
+    <div class="solution">
+      <h3>Fix</h3>
+      <pre><code>admin localhost:2019</code></pre>
+      <p>Bind admin to loopback inside the Caddy container (or disable it with <code>admin off</code> if no live-reload is needed). Turns a one-step lateral move into a non-path.</p>
+    </div>
+  </div>
+</section>
+
+<!-- 8: Finding 5 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 5 / 8</span></div>
+  <h2>User enumeration via distinct auth error messages <span class="tag sev-low">Low</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/journal/app/lib/auth.server.ts</code> (register + magic-link create paths)</p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <p><span class="verified">Verified:</span> the API returns distinguishable messages — <code>"Email already in use"</code> vs <code>"Username already taken"</code>, and magic-link create throws <code>"No account found for this email"</code>. An attacker can probe which emails/usernames are registered.</p>
+      <p>Calibrated to Low: username availability is intentionally visible at registration anyway, and this is a privacy/info-leak issue, not an account-takeover one.</p>
+    </div>
+    <div class="solution">
+      <h3>Fix</h3>
+      <ul>
+        <li>Magic-link request: return the same "if an account exists, we've sent a link" response whether or not the email matches; only send mail when it does.</li>
+        <li>Registration: keep username-availability UX, but make the email-taken path return a generic message (or fold into the same response and notify the existing owner by email instead).</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 9: Finding 6 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 6 / 8</span></div>
+  <h2>Federation — hardening &amp; missing regression tests <span class="tag sev-low">Low</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/journal/app/lib/federation*.server.ts</code> · <code>apps/journal/app/jobs/poll-remote-*.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>What's actually fine</h3>
+      <p><span class="verified">Verified:</span> Fedify verifies HTTP Signatures on inbox listeners by default; <code>allowPrivateAddress</code> is gated behind an env flag used only in e2e; private profiles don't federate (checked in every dispatcher); inbox replay is guarded via the Postgres KV store. The "unsigned activities accepted" alarm does not hold.</p>
+    </div>
+    <div class="why">
+      <h3>Worth doing anyway</h3>
+      <ul>
+        <li><strong>Regression test</strong> asserting an unsigned / wrongly-signed POST to <code>/users/:u/inbox</code> is rejected — so a future config change can't silently disable verification.</li>
+        <li><strong>Response-size cap</strong> on remote actor/outbox fetches (BRouter already caps at 10&nbsp;MB; federation fetches don't) — prevents an OOM from a hostile instance.</li>
+        <li>Treat <strong>DNS-rebinding</strong> on remote dereferences as an upstream Fedify concern to track; add resolve-then-connect guarding if it surfaces.</li>
+        <li>Range-check remote-supplied activity stats before storing.</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 10: Finding 7 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 7 / 8</span></div>
+  <h2>Rate limiting is in-process and trusts <code>X-Forwarded-For</code> <span class="tag sev-low">Low</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/journal/app/lib/rate-limit.server.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>Problem</h3>
+      <ul>
+        <li><span class="verified">Verified:</span> buckets live in a per-process <code>Map</code>. Correct and safe for the single-instance flagship; <strong>horizontal scaling silently weakens every limit</strong> (each instance has its own counters).</li>
+        <li><code>clientIp()</code> trusts the first <code>X-Forwarded-For</code> hop. Safe behind Caddy today (the app ports aren't published); becomes spoofable the moment the container is exposed directly.</li>
+      </ul>
+      <p>Both are documented assumptions in the code, not oversights — recorded here so the assumption stays visible when the topology changes.</p>
+    </div>
+    <div class="solution">
+      <h3>Fix (when scaling)</h3>
+      <ul>
+        <li>Move buckets to Postgres/Redis before running a second journal instance.</li>
+        <li>Pin the trusted-proxy hop count (or read a Caddy-set, non-spoofable header) rather than blindly taking <code>XFF[0]</code>.</li>
+        <li>Keep the app containers unpublished — never expose them without Caddy in front.</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 11: Finding 8 -->
+<section class="slide">
+  <div class="kicker"><span class="num">finding 8 / 8</span></div>
+  <h2>Komoot credential storage — by-design, minor edges <span class="tag sev-info">Info</span></h2>
+  <div class="cols">
+    <div class="full files">
+      <h3 style="color:var(--warn)">Files</h3>
+      <p><code>apps/journal/app/lib/crypto.server.ts</code> · <code>connected-services/providers/komoot/*</code> · <code>api.sync.komoot.connect.ts</code></p>
+    </div>
+    <div class="problem">
+      <h3>What's fine</h3>
+      <p><span class="verified">Verified:</span> the Komoot web-login password is encrypted at rest with <strong>AES-256-GCM</strong>, random 12-byte IV per encryption, scrypt-derived key. It must be reversibly stored because re-login replays it — that's inherent to web-login providers (ADR-recorded), not a flaw. The cipher usage is correct.</p>
+    </div>
+    <div class="why">
+      <h3>Minor edges</h3>
+      <ul>
+        <li>The account <strong>email is stored plaintext</strong> in the credentials JSONB next to the encrypted password — minor PII at rest; consider encrypting it too.</li>
+        <li>The decrypted password lives briefly as a JS string during basic-auth construction — only matters under memory-dump threat models; low priority.</li>
+        <li>Pin explicit <code>scrypt</code> cost params (N, r, p) so a future Node default change can't alter derivation.</li>
+        <li>Tighten the <code>Credentials</code> type off the <code>Record&lt;string, unknown&gt;</code> catch-all so a future kind can't accidentally store a secret in the clear.</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 12: Good practices -->
+<section class="slide">
+  <div class="kicker">Calibration</div>
+  <h2>Strong fundamentals (verified) <span class="tag sev-good">Good</span></h2>
+  <div class="cols">
+    <div>
+      <h3 style="color:var(--accent)">Injection &amp; input</h3>
+      <ul>
+        <li>All raw SQL is parameterized — including PostGIS: the GeoJSON is bound, table names go through <code>sql.identifier()</code>.</li>
+        <li>GPX/XML parsing is XXE-safe (DOMParser / linkedom, no external-entity or DTD expansion).</li>
+        <li><code>safeReturnTo()</code> blocks open redirects (local paths only, no <code>//</code>).</li>
+      </ul>
+      <h3 style="color:var(--accent)">Secrets</h3>
+      <ul>
+        <li><code>requireSecret()</code> refuses to boot prod on a dev-fallback secret.</li>
+        <li>AES-256-GCM + random IV; SOPS/age for secrets at rest; <code>.env</code> gitignored and never in history.</li>
+        <li>Sentry: <code>sendDefaultPii: false</code>, no session replay.</li>
+      </ul>
+    </div>
+    <div>
+      <h3 style="color:var(--accent)">Auth &amp; authz</h3>
+      <ul>
+        <li>Single-use JWT (<code>consumed_jwt_jti</code>, atomic <code>ON CONFLICT</code>) and single-use magic tokens (atomic <code>UPDATE … RETURNING</code>) — both race-proof.</li>
+        <li>Session cookies: HttpOnly, Secure in prod, SameSite=Lax, signed.</li>
+        <li>Branded ownership loader makes IDOR a compile error (from the prior refactor).</li>
+        <li>Terms gate enforced on both web and bearer-token API.</li>
+      </ul>
+      <h3 style="color:var(--accent)">Boundary &amp; infra</h3>
+      <ul>
+        <li>E2E backdoor endpoints gated on <code>E2E</code> env, never set in prod compose.</li>
+        <li>Strong headers + CSP (HSTS, nosniff, frame-deny, no <code>unsafe-eval</code>); Garmin webhook host allowlist; app ports unpublished behind Caddy; deploy secrets SOPS-encrypted, not echoed.</li>
+      </ul>
+    </div>
+  </div>
+</section>
+
+<!-- 13: Summary -->
+<section class="slide">
+  <div class="kicker">Summary</div>
+  <h2>Priorities</h2>
+  <table>
+    <tr><th>#</th><th>Finding</th><th>Sev</th><th>Do</th></tr>
+    <tr><td>1</td><td>SSRF via Planner session callback URL</td><td class="sev s-high">High</td><td>Apply <code>validateFetchUrl</code> at create + fetch</td></tr>
+    <tr><td>2</td><td>Upload content-type / filename / ownership</td><td class="sev s-med">Medium</td><td>Allowlist type, sanitize key, check owner</td></tr>
+    <tr><td>3</td><td>OAuth callback logs raw exception</td><td class="sev s-low">Low</td><td>Log redacted message only</td></tr>
+    <tr><td>4</td><td>Caddy admin on <code>0.0.0.0:2019</code></td><td class="sev s-low">Low</td><td>Bind to localhost / <code>admin off</code></td></tr>
+    <tr><td>5</td><td>User enumeration via auth errors</td><td class="sev s-low">Low</td><td>Generic responses on the email paths</td></tr>
+    <tr><td>6</td><td>Federation tests + fetch caps</td><td class="sev s-low">Low</td><td>Add signature-rejection test, size cap</td></tr>
+    <tr><td>7</td><td>In-process rate limit / XFF trust</td><td class="sev s-low">Low</td><td>Shared store before scaling out</td></tr>
+    <tr><td>8</td><td>Komoot credential edges</td><td class="sev s-info">Info</td><td>Encrypt email, pin scrypt params</td></tr>
+  </table>
+  <p style="margin-top:22px; color:var(--muted)">No Critical issues. One High, externally-reachable and unauthenticated — fix first. Everything else is hardening on an already-solid base. Four scanner "Critical/High/Medium" alarms were verified false and dropped; trust the code, not the raw sweep.</p>
+</section>
+
+<div class="footer">
+  <span>trails.cool security review · 2026-06-10 · defensive, authorized</span>
+  <span id="counter"></span>
+  <span class="hint"><kbd>←</kbd><kbd>→</kbd> or click · <kbd>Home</kbd>/<kbd>End</kbd></span>
+</div>
+
+<script>
+  const slides = Array.from(document.querySelectorAll('.slide'));
+  let i = 0;
+  function show(n) {
+    i = Math.max(0, Math.min(slides.length - 1, n));
+    slides.forEach((s, j) => s.classList.toggle('active', j === i));
+    document.getElementById('counter').textContent = (i + 1) + ' / ' + slides.length;
+    document.getElementById('progress').style.width = ((i + 1) / slides.length * 100) + '%';
+    location.hash = i + 1;
+  }
+  document.addEventListener('keydown', (e) => {
+    if (e.key === 'ArrowRight' || e.key === ' ' || e.key === 'PageDown') show(i + 1);
+    else if (e.key === 'ArrowLeft' || e.key === 'PageUp') show(i - 1);
+    else if (e.key === 'Home') show(0);
+    else if (e.key === 'End') show(slides.length - 1);
+  });
+  document.addEventListener('click', (e) => {
+    if (window.getSelection().toString()) return;
+    show(e.clientX > window.innerWidth / 2 ? i + 1 : i - 1);
+  });
+  show(parseInt(location.hash.slice(1), 10) - 1 || 0);
+</script>
+</body>
+</html>