Addresses planner audit #3 (SSRF via callbackUrl) and #7 (URL-param
size). Two attack surfaces hardened:
1. /new loader — \`callback\`, \`token\`, \`returnUrl\`, \`gpx\` query
params now validated:
- callbackUrl: must be a valid absolute http(s) URL ≤ 2048 chars.
If \`PLANNER_CALLBACK_ALLOWED_HOSTS\` is set (comma-separated),
the host must match — defense-in-depth SSRF guard for self-
hosted instances. Unset = no allowlist (dev / open self-host).
- token: max 2048 chars.
- returnUrl: must be a same-origin path or absolute http(s) URL
≤ 2048 chars. Rejects \`javascript:\`, \`data:\`, and
protocol-relative \`//host\` (which would resolve to a remote
origin on HTTPS pages).
- gpx: ≤ 2 MB encoded.
Invalid input throws 400 from the loader.
2. /session/:id default-export component — \`waypoints\`, \`noGoAreas\`,
\`notes\`, \`returnUrl\` URL params now bounded before
\`JSON.parse\` / use:
- waypoints / noGoAreas: ≤ 50KB each; over-cap returns undefined
(component starts with empty initial state, same as malformed).
- notes: ≤ 10KB.
- returnUrl: ≤ 2KB + same scheme rules as #1.
Pulled the URL validation into \`lib/url-validation.server.ts\` so
both routes (and any future caller) share the same rules.
Tests: \`url-validation.server.test.ts\` (14 cases — schemes,
allowlist, length caps, protocol-relative guards, env parsing).
Full repo: pnpm typecheck / lint / test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The hourly `expireSessions()` cron runs `DELETE FROM planner.sessions
WHERE last_activity < cutoff`. Without an index on `last_activity`,
that's a full table scan growing linearly with the total sessions
ever created (planner sessions are never user-deleted; expiry is the
only churn path).
`drizzle-kit push` picks this up on next deploy.
Mirrors journal PR #5. `fetchSegment()` and `computeSegmentGpx()` in
lib/brouter.ts called `fetch()` with no AbortSignal — a hung or slow
BRouter would stall the request handler indefinitely.
New `lib/http.server.ts::fetchWithTimeout()` (same as the journal's
helper) wraps fetch with `AbortSignal.timeout(30_000)`, composable
with a caller-supplied signal via `AbortSignal.any()`.
Tests: lib/http.server.test.ts (2 cases — timeout abort, happy path).
The three planner-coloring tests wait for the elevation chart's
\`<canvas>\` after seeing the Yjs \"Connected\" text. \`ElevationChart\`
returns \`null\` until \`points.length >= 2\` — i.e. until the BRouter
response (even mocked) has flowed through the Yjs round-trip and
updated \`yjs.routeData\`. The chart module is also lazy-imported
(\`Suspense\`) — cold CI runs need to fetch the chunk before mounting.
Saw the failure on PR #424's CI run: canvas \"element(s) not found\"
after the 10s window, both initial run and retry. Other planner
suites that wait on \`.leaflet-container\` (which renders
unconditionally and isn't behind a lazy boundary) don't see this.
Bumped \`CANVAS_TIMEOUT\` to 20s. Not masking a real regression — the
lazy chunk + Yjs apply is a real serial cost that the test budget
didn't reflect.
The five \`*.integration.test.ts\` files in apps/journal (explore, follow,
demo-bot, notifications, notifications-fanout — 31 tests total) were
gated behind \`EXPLORE_INTEGRATION=1\` / \`FOLLOW_INTEGRATION=1\` /
\`DEMO_BOT_INTEGRATION=1\` / \`NOTIFICATIONS_INTEGRATION=1\`. The unit-test
job doesn't have Postgres, so they correctly skipped there — but no
CI job set the env vars, so they were effectively dead code.
Added a step in the E2E job (which already has Postgres + schema
pushed) that flips all four gates and runs the integration files
serially (\`--no-file-parallelism\` — they share the schema and trip FK
constraints if run in parallel; ~2.5s sequential anyway).
Wiring them up surfaced two real issues, both fixed here:
1. **\`createRoute\` silently dropped \`input.visibility\`** — every
caller passing \`visibility: \"public\"\` (including the demo-bot)
was getting the column's \`private\` default. Spread now mirrors
\`createActivity\`'s pattern. Demo-bot routes have actually been
private in production all this time — they were rendering on the
home feed only via the \`activity_published\` fan-out from the
*activity*, not as visible *routes*.
2. **The demo-bot test's fetch stub was incomplete** — it stubbed
\`.text()\` but the planner-session preflight calls \`.json()\`. The
stub now branches on URL: \`/api/sessions\` returns
\`{ sessionId: 'test-session' }\` JSON, the BRouter call returns the
stub GPX text.
Full repo: pnpm typecheck, pnpm lint, pnpm test all green
(177 unit-test pass + 31 integration pass = 208 total, no skips).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Self-hosted instances no longer inherit the trails.cool flagship Sentry
DSNs. Code paths now read DSN strictly from env — unset = no Sentry
init, no events sent.
Flagship continues to report unchanged: cd-apps.yml and cd-staging.yml
inject the public DSNs as workflow env vars, which feed into:
- runtime via \`infrastructure/app.env\` / \`staging.env\`
(\`SENTRY_DSN_JOURNAL\` / \`SENTRY_DSN_PLANNER\` → compose env per service)
- build time via docker build-arg \`VITE_SENTRY_DSN\` (journal only;
planner has no client Sentry init).
Sentry DSNs are public-by-design (transmitted unencrypted from the
client JS bundle), so embedding them as plaintext workflow env vars
is no worse than the runtime exposure. Forks should replace these with
their own DSNs or remove the workflow env lines to ship Sentry-free.
Files changed:
- apps/journal/server.ts: \`process.env.SENTRY_DSN\` only, no fallback
- apps/planner/server.ts: same
- apps/journal/app/lib/sentry.client.ts: \`import.meta.env.VITE_SENTRY_DSN\`
only; falsy = skip init
- apps/journal/Dockerfile: new \`ARG VITE_SENTRY_DSN\` baked into build
- infrastructure/docker-compose.yml: pass \`SENTRY_DSN\` per service
- infrastructure/docker-compose.staging.yml: same
- .github/workflows/cd-apps.yml: workflow env + build-arg + app.env echo
- .github/workflows/cd-staging.yml: same
Full repo: pnpm typecheck, pnpm lint, pnpm test, journal build all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
infrastructure/docker-compose.yml had \`\${VAR:-default}\` fallbacks for
JWT_SECRET, SESSION_SECRET, and POSTGRES_PASSWORD that silently
substituted known-weak values (\`change-me-in-production\`, \`trails\`)
when the env wasn't set. A misconfigured prod deploy would happily come
up with these defaults — JWT/session forgery + a guessable Postgres
password.
Switched the four critical substitutions to \`\${VAR:?message}\` so
\`docker compose up\` fails loud instead. Mirrors the existing pattern
already used for BROUTER_URL/BROUTER_AUTH_TOKEN and (in staging) for
JWT_SECRET/SESSION_SECRET.
Sites fixed:
- journal: DATABASE_URL (POSTGRES_PASSWORD), JWT_SECRET, SESSION_SECRET
- planner: DATABASE_URL (POSTGRES_PASSWORD)
- postgres: POSTGRES_PASSWORD (container itself)
- exporter: DATA_SOURCE_NAME (POSTGRES_PASSWORD)
- staging: both DATABASE_URLs
Production already provides all three via SOPS-encrypted
secrets.app.env, so this is a defense-in-depth change with no behavior
change on a properly-configured deploy.
Updated infrastructure/.env.example to make SESSION_SECRET explicit
(was missing) and call out the trio as required.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors #422 (fail-loud DB URL) and #430 (health-check pool reuse) for
the planner app, which still had:
- two \`process.env.DATABASE_URL ?? \"postgres://trails:trails@localhost:5432/trails\"\`
call sites that would silently boot a misconfigured prod against
localhost
- a /health handler that opened a fresh postgres client + connection on
every probe (no pool, per-call TCP/TLS handshake)
Both now route through @trails-cool/db's \`getDatabaseUrl()\` (refuses to
start in production if unset / matches the dev default; E2E=true is the
opt-out) and a module-level singleton client (max: 2, idle_timeout: 30).
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors PR #429 for the planner app. Each HTTP request now gets a
requestId (inbound X-Request-Id honored, otherwise a fresh UUID),
echoed on the response, and propagated to every downstream log call via
AsyncLocalStorage + pino's \`mixin\`.
Tests: planner logger.server.test.ts (2 cases, same shape as the
journal version).
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the i18next peer dep override workaround with a proper single
TypeScript version in the workspace catalog. All 15 packages typecheck
cleanly with TypeScript 6.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mobile's TypeScript 6 devDep created a second peer-resolution variant of
i18next (i18next(typescript@6)) alongside the web apps' (i18next(typescript@5)).
Under pnpm's hoisted nodeLinker the server and client ended up with different
instances of the i18n singleton, so SSR rendered raw keys and hydration
mismatched.
Pin i18next and react-i18next TypeScript peer deps to 5.9.3 in root overrides
so the entire workspace resolves a single i18next instance regardless of which
app's TypeScript version is active.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous handler opened a fresh postgres client (max: 1) on every
call to /api/health and tore it down in the finally block. Under the
prod monitoring cadence (probes every few seconds), that's a fresh
TCP + TLS + auth handshake on every probe, plus connection-table
churn on the Postgres side — fine for the trickle of curl-ish manual
checks, slow-bleed under blackbox monitoring.
Now we cache a module-level singleton postgres client dedicated to
/api/health (max: 2, idle_timeout: 30) and reuse it across calls.
Separate from the app's main DB pool (via @trails-cool/db's createDb)
on purpose — so a starvation event on the main pool doesn't fail the
liveness check and trigger a restart loop.
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every HTTP request now gets a requestId (inbound X-Request-Id header is
honored, otherwise a fresh UUID is minted) and the value is echoed on
the response. The server wraps the request in
\`requestContext.run({ requestId }, ...)\` — an AsyncLocalStorage scope —
so pino's \`mixin\` callback can read it on every log call without the
caller threading it through.
Net effect: \`logger.info({ ... }, \"db error\")\` from a loader, action,
or downstream lib now lands in JSON with a \`requestId\` field, making
cross-handler debugging trivial (\`grep requestId=abc-123\` returns the
full request trace).
Out of scope here:
- Planner gets the same treatment (separate, smaller PR after this lands).
- BRouter / Fedify outbound calls don't propagate the requestId yet —
those are HTTP boundaries where we'd add it as a header, but the
audit value was the in-process trace.
Tests:
- logger.server.test.ts (2 cases — als-bound info tags requestId; no
context = no tag).
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Sentry DSNs were hardcoded in journal/server.ts, planner/server.ts,
and journal/app/lib/sentry.client.ts. Self-hosted instances inheriting
the trails.cool flagship DSN would silently ship their errors to our
Sentry account.
Now each init site reads its DSN from env:
- journal/server.ts: SENTRY_DSN (server runtime env)
- planner/server.ts: SENTRY_DSN (server runtime env)
- journal/app/lib/sentry.client.ts: VITE_SENTRY_DSN (build-time bake)
The flagship DSN is kept as the fallback so the production deploy keeps
reporting without an infra change — but self-hosters can:
- set SENTRY_DSN=\"\" / VITE_SENTRY_DSN=\"\" to ship their own builds
without Sentry, or
- set SENTRY_DISABLED=true to skip init entirely at runtime, or
- set SENTRY_DSN=/VITE_SENTRY_DSN= to their own DSN.
Follow-up: a future PR can remove the hardcoded fallbacks once
infrastructure/docker-compose.yml + the cd-apps.yml workflow are wired
to pass SENTRY_DSN explicitly. That requires SOPS edits + workflow
changes I want isolated from this purely-code change.
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
verifyLoginCode, verifyMagicToken, and verifyEmailChange previously did
SELECT WHERE used_at IS NULL → UPDATE … SET used_at = now. Two concurrent
verifications could both pass the SELECT and both succeed, accepting the
same single-use token twice.
Collapsed each to a single UPDATE … WHERE … RETURNING * statement.
Postgres serializes row-level locks within an UPDATE, so exactly one
concurrent caller observes a returned row; the rest see an empty array
and get \"Invalid or expired\". The token is also marked used as part of
the same statement — no second write needed.
verifyEmailChange's tertiary email-availability check now runs *after*
the consume; we keep the original semantics where the token is burned
on a clash (the previous code explicitly did the same with a separate
UPDATE).
No behavior change on the happy path. Closes a credential-reuse
window that mattered most for the 6-digit login codes (small search
space, more likely to race).
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous \`importOne\` only fetched page 1 of /v1/workouts and errored
with \"not found on page 1\" if the workout wasn't there — silently
breaking import for any workout older than roughly the most recent 30
entries (per_page default). Webhook-driven imports happen to land on
page 1 by definition, so this only bit on user-initiated catch-up
imports of older workouts.
Now we paginate forward, using the \`total / per_page\` returned by page 1
to compute a stop condition, with a \`MAX_PAGES=100\` ceiling so a
misbehaving API can't loop us. We also stop early on an empty page.
Tests:
- new pagination case (workout on page 2, expect 2 fetch calls)
- new \"not found on any page\" case
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rollup was warning on 5 modules that were both dynamically and statically
imported. With static importers in the same chunk, the dynamic forms
buy no chunking benefit — they were leftovers from earlier
cycle-avoidance workarounds that no longer apply.
Converted to static:
- @trails-cool/gpx (routes.\$id.server.ts, sync.import.\$provider.server.ts)
- logger.server (boss.server.ts — comment claimed test cycles, but tests pass)
- boss.server (activities.server.ts at two sites)
- connected-services/manager (komoot/importer.ts, wahoo/importer.ts)
- notifications.server (root.tsx)
Kept dynamic: fit-file-parser in sync.import.\$provider.server.ts — it's
heavy and only the FIT ingestion path needs it, no other static
importers exist, so the dynamic actually does chunk-split it.
Build is now warning-free.
Full repo: pnpm typecheck, pnpm lint, pnpm test, pnpm build all green.
192 tests passed (up from 181 — rate-limit test from #424 + others).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
E2E suite drives registrations from one IP at parallel volume;
production limits trip and flake tests. Same opt-out pattern as the
fail-loud secret/DB-URL guards.
expo-crypto@56 introduced a native AES class that throws in Jest's Node.js
environment on import. Mock the functions actually used (getRandomBytes,
digestStringAsync) so the api-client test suite can run.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Upgrade expo, expo-router, expo-dev-client, expo-crypto, expo-localization,
expo-location, expo-navigation-bar, expo-notifications, expo-secure-store,
expo-status-bar, expo-system-ui, expo-web-browser to SDK 56 versions
- Upgrade react-native 0.83.4 → 0.85.3
- Upgrade TypeScript 5.9 → 6.0.3 (required by SDK 56)
- Add react-native-worklets (required peer dep for react-native-reanimated)
- Remove expo-dev-menu as a direct dependency (transitive, managed by Expo)
- Move splash config from top-level ExpoConfig to expo-splash-screen plugin
(ExpoConfig.splash was removed in SDK 56 types)
- Add expo-localization and expo-splash-screen to plugins array
- Fix metro.config.js watchFolders to merge with Expo defaults instead of
overwriting them (fixes expo-doctor Metro config check)
- Add types: ["jest"] to tsconfig.json (required for jest globals in TS 6)
- Exclude @sentry/react-native from Expo version validation (bundledNativeModules
has a stale entry; Sentry 8.x officially supports Expo 51+ and RN 0.73+)
expo-doctor: 18/18 checks passed
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds per-process in-memory fixed-window rate limiting (lib/rate-limit.server.ts)
and applies it across /api/auth/login and /api/auth/register:
- All login steps: 30 attempts per IP per minute (defense against
step-fanout flooding).
- finish-passkey: 10 per IP per minute (assertions don't usefully retry
faster).
- magic-link generation: 3 per email per 5 min + 10 per IP per 5 min
(defeats inbox-spam-the-victim + cross-email IP fanout).
- verify-code: 10 per email per 15 min — makes 6-digit code brute force
(10^6 search) infeasible before code expiry.
- /api/auth/register: 10 per IP per hour (legitimate signup completes
in 2-3 requests; sustained churn from one IP is account spam).
Single-instance is fine for the flagship's current topology (one journal
container). When we horizontally scale we revisit with a Postgres- or
Redis-backed store. Buckets self-clean on next read and a 5-minute
background sweep drops stale entries.
Client IP: honors X-Forwarded-For first entry (Caddy in front of the
journal sets it), falls back to a stable "unknown" bucket so the
limiter still bites when the header is missing.
Tests: rate-limit.server.test.ts (8 cases — exhaustion, independent
scopes/keys, remaining countdown, reset window, XFF parsing, fallback,
trimming). Existing auth tests still pass; limits sized to not trip
during the ~7 test cases that all share the "unknown" IP bucket.
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Playwright runs the server via `react-router serve` with
NODE_ENV=production but against a local dev Postgres and local cookie
secrets. The guards added in 5a7bb76 refused to start under that
configuration. `E2E=true` (already set by the CI E2E job) is now the
explicit opt-out: in real production this env var is never set, so the
guard still bites.
pnpm test:e2e runs via react-router serve which boots with
NODE_ENV=production; the new requireSecret() guard refuses to start
without explicit values. CI now supplies CI-only throwaway secrets so
the guard still bites in real prod deploys without breaking the test
job.
Adds `requireSecret(name, devFallback)` in lib/config.server.ts and
`getDatabaseUrl()` in @trails-cool/db. Both:
- return the env var when set,
- fall back to the dev default in non-production,
- throw at boot in production if the env var is missing OR matches the
known dev fallback (which would otherwise silently ship a public
secret / point at localhost).
Applied to:
- JWT_SECRET (lib/jwt.server.ts) — was `?? "dev-jwt-secret-change-in-production"`
- SESSION_SECRET (lib/auth/session.server.ts) — was `?? "dev-secret-change-in-production"`
- DATABASE_URL (server.ts health + boss; packages/db migrate-data) — was
`?? "postgres://trails:trails@localhost:5432/trails"`
Why: these strings are in the repo and known to attackers. A
misconfigured prod deploy that forgot to set them would either run with
guessable signing keys (full session/JWT forgery) or connect to a
non-existent localhost DB. Better to refuse to start than to silently
operate insecurely.
Tests:
- packages/db/src/get-database-url.test.ts (5 cases)
- lib/config.server.test.ts gains `requireSecret` cases (4 new)
Full repo: pnpm typecheck, pnpm lint, pnpm test all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
