- e2e/activity-sport-type.test.ts: register → create activity with a sport →
assert the sport badge renders on the detail page. Passes locally against an
E2E=true server.
- playwright.config.ts: add the `activity-sport-type` project so the spec
actually runs (specs only execute if a project testMatch matches them).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements the activity-sport-type change (specs/activity-sport-type):
- db: nullable `sport_type` column on journal.activities + SportType /
SPORT_TYPES (text().$type<> convention).
- api: optional sportType on the activity read + create schemas (mirrored
SPORT_TYPES; @trails-cool/api stays zod-only).
- write: ActivityInput + createActivity persist it; mapSportType() normalizes
provider strings (Komoot bulk import passes tour.sport; Garmin unset);
threaded through the unified importActivity.
- read/display: sportType added to the detail/feed/profile loaders and the v1
REST endpoints; shared SportBadge (glyph + i18n label) on detail, feed, and
profile; sport-aware feed verb; create-form <select>.
- i18n: journal.activities.sport.* (labels + verbs) in en + de.
- federation: `sport` PropertyValue on the Note when set.
Tests: mapSportType unit table; federation asserts the sport attachment is
present when set and omitted when unset. typecheck + lint + unit all green.
E2E (create→badge) still to add.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three OpenSpec proposals to bring the journal's route/activity detail and feed
closer to best-in-class outdoor apps, in the order we'll implement them:
- activity-sport-type: nullable sport enum on activities (hike/walk/run/
ride/gravel/mtb/ski/other), set on create or normalized on import,
shown as a badge + feed verb, federated as a Note PropertyValue.
- activity-stats: one reusable StatRow + canonical metric set, sport-aware
avg pace/speed, moving-vs-elapsed time. Derive-only, no schema change.
- journal-elevation-profile: elevation profile chart + chart<->map
"active distance" hover/click sync on the read-only detail pages.
All three validate with `openspec validate --strict`. Specs only; no code.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
docs/reviews/internal/ holds internal working notes we keep out of the
published repo. Commit the ignore rule so the folder is excluded for everyone,
not just on the local working tree.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Diagnosis (reproduced by looping cold-server runs): the recurring
"cold start" failures in the auth/follow specs were a hydration race,
not a Vite dep-reload (the server.warmup config already handles that —
no reload events in the logs). FollowButton is an onClick button, and
Playwright considers it actionable (visible + enabled) before React
attaches the handler on a freshly-navigated page. A click in that
window is dropped (or triggers a native form submit), so the expected
state change never happens — e.g. notifications.test.ts:59 clicked
"Request to follow" with no hydration wait and the button never
flipped to "Requested".
The existing waitForHydration guard was applied per-interaction and so
was easy to forget (test 1 had it, test 3 didn't). Fix it at the
navigation instead:
- add gotoHydrated(page, url) = goto + waitForHydration, documented as
the default for "navigate then interact with a React control"
- use it before every FollowButton interaction in notifications +
social
- consolidate the setProfileVisibility helper (copy-pasted in three
specs, two missing the hydration wait) into e2e/helpers/profile.ts,
built on gotoHydrated
Validation against a fresh Postgres, cold servers: notifications 0/10
failures (was the proven flaker); notifications+social+explore 0/5
(55 tests); planner unaffected (25/25).
Note: two other "cold-start"-attributed flakes are NOT this race and
are out of scope here — planner map-load latency (mitigated by warmup
+ CI retries) and the /explore directory assertion (a local artifact
of the scratch DB accumulating >20 public users; CI's fresh DB stays
under the page size, so it doesn't bite there).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
settings.test.ts, explore.test.ts, and social.test.ts weren't matched
by any Playwright project, so they had silently never run — which is
how they rotted. Register them (one project each) and repair the
selectors against the current UI:
- settings: the page was split into sibling sections
(/settings/{profile,security,account}); the spec assumed one page.
Navigate to the right sub-page per test, use the stable section-nav
links + #id input locators (the Vite dev server transiently
double-renders the profile form during hydration, breaking
getByLabel), and wait for hydration before interacting with
fetcher-backed forms and the avatar dropdown.
- explore: setProfileVisibility now targets /settings/profile and
waits for hydration so the visibility save uses the fetcher.
- social: already green once registered.
Also fixes an app bug the specs surfaced: deleting a passkey redirected
to /settings#security, a stale anchor that now resolves to
/settings/profile — so you'd land on the wrong section. It now
redirects to /settings/security.
Verified locally against Postgres/BRouter: green under CI-style
retries (the residual registration flake is the same cold-start class
the rest of the suite has, which is why CI runs retries=2).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The flagship Caddy ran its admin API on 0.0.0.0:2019, reachable by
every container on the Docker network. The admin API can rewrite
routes and proxy targets, so a journal/planner RCE could repoint
traffic with no further auth. (The brouter-host Caddyfile already
binds admin to localhost; the flagship didn't.)
The admin endpoint also served Prometheus metrics (global `metrics`
option → admin endpoint), and Prometheus scrapes caddy:2019
cross-container — so admin couldn't just move to loopback without
breaking metrics. Split them:
- admin localhost:2019 (loopback only). All reloads are in-container
(`docker compose exec caddy caddy reload`) so they use this endpoint
unaffected.
- a dedicated `:2020` server exposes the read-only metrics handler;
prometheus.yml now scrapes caddy:2020.
Validated with `caddy validate` + `caddy adapt` against caddy:2:
admin.listen=localhost:2019, srv on :2020 carries the metrics handler,
the :443 site is unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three Low/Info hardening items from the 2026-06-10 security review.
OAuth/credential log redaction:
- oauth-flow.server.ts logged the raw exception on code-exchange
failure; a provider error can embed the auth code / token response,
which would land in logs + Sentry. Log only e.message now.
- manager.markNeedsRelink bounded the provider-supplied reason string
to 200 chars before logging.
Magic-link account enumeration:
- createMagicToken now returns null (instead of throwing "No account
found for this email") when no account matches; the login route
always responds { step: "magic-link-sent" }, minting a token and
sending mail only for a real account. The public login form can no
longer be used to probe which emails are registered. Registration's
email/username "already in use/taken" messages are intentionally
unchanged — standard signup UX, and the passkey ceremony can't be
made to fake-succeed.
Federation remote-document size cap:
- assertRemoteDocSize rejects an actor/outbox document over 4 MB once
serialized, applied in the ingest fetchJson seam. Fedify owns the
transfer (with its own SSRF + redirect limits) and our poll uses the
authenticated loader for secure-mode instances, so this is a
downstream guard on iteration/persistence, complementing the existing
per-poll item cap.
Tests: assertRemoteDocSize unit cases; a gated (FEDERATION_INTEGRATION=1)
integration test asserting an unsigned POST to /users/:u/inbox is
rejected with 401 — a regression guard for Fedify's signature
verification.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
POST /api/v1/uploads minted an upload key from caller input with no
ownership check, no content-type allowlist, and the raw filename
interpolated into the S3 key.
- Ownership: a caller may only mint upload URLs for a route/activity
they own, enforced via the branded loadOwnedRoute/loadOwnedActivity
(404 on miss, no existence leak). Closes writing into another
user's resource key-space.
- Content type: the request schema now constrains contentType to an
image/gpx allowlist, so active content (HTML/SVG/JS) that would
execute if served inline is rejected at the request boundary.
- Filename: sanitizeUploadFilename() reduces the client filename to a
safe basename (charset-restricted, no path components, no leading
dots, length-bounded) before it becomes part of the key.
Tests: schema allow/deny + filename sanitization in @trails-cool/api;
handler tests covering owned-success, not-owner 404, disallowed
content-type, and the route-vs-activity ownership branch.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The session callbackUrl becomes a server-side fetch target in
api.save-to-journal (POSTed with the callback bearer token, and the
journal's response is reflected to the caller). The /new query-param
loader already validated it, but the programmatic POST /api/sessions
entry point — anonymous, since the Planner is stateless — stored it
unvalidated. An attacker could make the Planner backend POST to
arbitrary hosts, including 169.254.169.254 and other internal targets.
- validateFetchUrl now blocks private / loopback / link-local /
CGNAT / cloud-metadata hosts (IPv4, IPv6, IPv4-mapped) when an
explicit allowlist isn't set. Gated on NODE_ENV=production && !E2E
(the requireSecret idiom) so the dev/e2e journal-on-localhost save
flow is unaffected. An explicit PLANNER_CALLBACK_ALLOWED_HOSTS still
takes precedence and remains the full-closure control (it also stops
DNS-name-to-private rebinding, which literal blocking does not).
- POST /api/sessions now validates callbackUrl exactly as /new does.
- api.save-to-journal re-validates immediately before the fetch
(defense in depth: covers sessions persisted before this change and
narrows the create→save rebinding window).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both failed the deletion test in the telling direction:
- @trails-cool/ui: Button/Input/Card had zero consumers — both apps
roll their own elements inline. The only live part was a 6-line
styles.css (the Tailwind entry + one keyframe), which now lives in
each app as app/styles.css.
- @trails-cool/map: MapView and RouteLayer had zero consumers; the
package was otherwise a re-export of two map-core constants, and the
two import sites now use @trails-cool/map-core directly. The
"map components go in @trails-cool/map" convention had drifted long
ago — the real map components live in apps/planner/app/components.
CLAUDE.md's repository structure and conventions updated to match
reality (including pointing shared-type guidance at the post-#515
sources: db row types, api contracts, Waypoint in types). Dockerfiles
no longer COPY the deleted package manifests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PKCE verifier cookie, state encoding, redirect-URI construction,
and code exchange were coordinated across three route handlers that
each knew part of the protocol. Two latent bugs lived in the gaps: a
push-initiated re-authorization skipped PKCE entirely (harmless today
only because the sole pusher, Wahoo, is not a PKCE provider), and the
push-resume callback path never cleared the spent verifier cookie.
oauth-flow.server.ts now owns the lifecycle: initiateOAuthFlow builds
the provider redirect (state + verifier cookie, on every entry path),
and completeOAuthFlow consumes the callback — state decode, verifier
recovery, code exchange, connection linking — returning a
discriminated result the route maps to redirects. The three routes are
thin adapters; the next OAuth provider reuses the flow, not the
pattern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Route and Activity existed three times: hand-written interfaces in
packages/types, Zod contracts in packages/api, and Drizzle columns in
packages/db — each with different fields and nullability. The
hand-written ones had drifted so far they had zero importers; the Zod
contracts were advisory because v1 handlers hand-rolled Response.json
shapes nothing validated.
- packages/types keeps only what both apps actually share (Waypoint,
WaypointPoiTags) and documents where row types and wire contracts
live; the dead Route/RouteMetadata/RouteVersion/Activity interfaces
are gone
- packages/db exports canonical inferred row types (RouteRow,
ActivityRow, RouteVersionRow, UserRow)
- packages/api contracts are reconciled with the real wire format
(RouteVersionSchema gains the id and createdBy fields the endpoint
has always returned) and gain Create*ResponseSchemas
- apiJson(schema, payload) in api-guard parses every v1 response
through its contract: drift is now a thrown ZodError in tests/CI,
unknown keys are stripped, and payloads are compile-checked as
z.input of the schema
Enforcement immediately caught two real drifts: nullable DB
descriptions could ship null where the contract promises string (now
coalesced at the boundary), and GET /api/v1/activities/:id was missing
the routeName and photos fields its contract declares.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
