trails.cool · security review

Security review

Defensive review of the auth, federation, secret-management, and injection surfaces across both apps and the infrastructure — findings verified against source and severity-calibrated.

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

Method & calibration

How to read this — and what I threw out

The automated sweeps over-flagged. I verified each high-severity claim against the source and downgraded or dropped the ones that didn't hold up. That recalibration is itself a finding: don't action the raw scanner output.

Dropped / downgraded after verification

  • "Critical: inbox signatures not verified" → Fedify verifies HTTP Signatures on inbox listeners by default. Real item is only a missing regression test. downgraded → Info
  • "JWT alg-confusion / none"jose with a symmetric key already rejects alg:none and asymmetric algs. Explicit allow-list is hardening only. → Info
  • "Magic-link 6-digit brute-force" → bounded by a per-IP catch-all (30/min) and per-email verify cap (10/15min) against a 15-min-rotating code. → Info
  • ".env secrets committed".env is gitignored and never appears in history; values are dev creds. → Info

What held up

  • One High: unauthenticated SSRF via the Planner session callback URL.
  • A handful of Medium/Low hardening gaps (upload validation, log redaction, Caddy admin bind, user enumeration).
  • A broad base of genuinely strong fundamentals (next-to-last slide).

Severity legend

High Medium Low Info

Findings

What to fix, in order

finding 1 / 8

SSRF — unauthenticated Planner session callback URL High

Files

apps/planner/app/routes/api.sessions.ts · apps/planner/app/routes/api.save-to-journal.ts · helper apps/planner/app/lib/url-validation.server.ts (exists, unused here)

Problem

Verified: POST /api/sessions is anonymous (the Planner is stateless) and stores the caller-supplied callbackUrl with no validation. On save, the Planner server fetches it:

resp = await fetchWithTimeout(session.callbackUrl, {
  method: "POST",
  headers: { Authorization: `Bearer ${session.callbackToken}` },
  ...

Anyone can make the Planner backend issue POSTs to arbitrary hosts — internal services, 169.254.169.254, localhost. A SAFE-scheme/host validator (validateFetchUrl) already exists in the repo but is not applied on this path.

Fix

Validate callbackUrl at session-create against the existing allowlist helper, rejecting non-HTTP(S) schemes and private/loopback/link-local resolutions:

const v = validateFetchUrl(callbackUrl, {
  allowedHosts: getCallbackAllowedHosts(),
});
if (!v.ok) return data(
  { error: "Invalid callback URL" },
  { status: 400 },
);

Re-validate at fetch time too (defends against DNS rebinding between create and save).

finding 2 / 8

Upload validation — no content-type allowlist, raw filename in key Medium

Files

apps/journal/app/routes/api.v1.uploads.ts

Problem

Verified: the presigned-upload endpoint builds the S3 key from the raw client filename and accepts any content-type:

const key =
  `${resourceType}/${resourceId}/${randomUUID()}-${filename}`;
  • No content-type allowlist — a user can store HTML/SVG under an image resource; stored XSS if those bytes are ever served inline rather than as attachments.
  • Raw filename 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).
  • Confirm resourceId ownership is checked so a user can't mint upload URLs under another user's resource path.

Fix

  • Allowlist contentType (e.g. image/jpeg, image/png, application/gpx+xml) and bind it into the presign so the upload can't differ.
  • Sanitize filename to [A-Za-z0-9._-] or drop it from the key entirely (the UUID is enough).
  • Serve user content with Content-Disposition: attachment / from a separate origin, and verify resourceId belongs to the caller.
finding 3 / 8

OAuth callback logs the raw exception Low

Files

apps/journal/app/lib/connected-services/oauth-flow.server.ts (~line 113) · pattern also in manager.ts (markNeedsRelink reason)

Problem

Verified: the code-exchange failure path logs the whole error object:

} catch (e) {
  console.error(
    `OAuth callback failed for ${manifest.id}:`, e);

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.

Fix

console.error(
  `OAuth callback failed for ${manifest.id}:`,
  e instanceof Error ? e.message : String(e),
);

Log a redacted shape (name + truncated message), never the raw object. Apply the same to markNeedsRelink's provider-supplied reason.

finding 4 / 8

Caddy admin API bound to 0.0.0.0:2019 Low

Files

infrastructure/Caddyfile (line 3)

Problem

Verified: admin 0.0.0.0:2019. The port isn't published to the host, so it's not internet-reachable — but it is 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.

Fix

admin localhost:2019

Bind admin to loopback inside the Caddy container (or disable it with admin off if no live-reload is needed). Turns a one-step lateral move into a non-path.

finding 5 / 8

User enumeration via distinct auth error messages Low

Files

apps/journal/app/lib/auth.server.ts (register + magic-link create paths)

Problem

Verified: the API returns distinguishable messages — "Email already in use" vs "Username already taken", and magic-link create throws "No account found for this email". An attacker can probe which emails/usernames are registered.

Calibrated to Low: username availability is intentionally visible at registration anyway, and this is a privacy/info-leak issue, not an account-takeover one.

Fix

  • 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.
  • 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).
finding 6 / 8

Federation — hardening & missing regression tests Low

Files

apps/journal/app/lib/federation*.server.ts · apps/journal/app/jobs/poll-remote-*.ts

What's actually fine

Verified: Fedify verifies HTTP Signatures on inbox listeners by default; allowPrivateAddress 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.

Worth doing anyway

  • Regression test asserting an unsigned / wrongly-signed POST to /users/:u/inbox is rejected — so a future config change can't silently disable verification.
  • Response-size cap on remote actor/outbox fetches (BRouter already caps at 10 MB; federation fetches don't) — prevents an OOM from a hostile instance.
  • Treat DNS-rebinding on remote dereferences as an upstream Fedify concern to track; add resolve-then-connect guarding if it surfaces.
  • Range-check remote-supplied activity stats before storing.
finding 7 / 8

Rate limiting is in-process and trusts X-Forwarded-For Low

Files

apps/journal/app/lib/rate-limit.server.ts

Problem

  • Verified: buckets live in a per-process Map. Correct and safe for the single-instance flagship; horizontal scaling silently weakens every limit (each instance has its own counters).
  • clientIp() trusts the first X-Forwarded-For hop. Safe behind Caddy today (the app ports aren't published); becomes spoofable the moment the container is exposed directly.

Both are documented assumptions in the code, not oversights — recorded here so the assumption stays visible when the topology changes.

Fix (when scaling)

  • Move buckets to Postgres/Redis before running a second journal instance.
  • Pin the trusted-proxy hop count (or read a Caddy-set, non-spoofable header) rather than blindly taking XFF[0].
  • Keep the app containers unpublished — never expose them without Caddy in front.
finding 8 / 8

Komoot credential storage — by-design, minor edges Info

Files

apps/journal/app/lib/crypto.server.ts · connected-services/providers/komoot/* · api.sync.komoot.connect.ts

What's fine

Verified: the Komoot web-login password is encrypted at rest with AES-256-GCM, 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.

Minor edges

  • The account email is stored plaintext in the credentials JSONB next to the encrypted password — minor PII at rest; consider encrypting it too.
  • The decrypted password lives briefly as a JS string during basic-auth construction — only matters under memory-dump threat models; low priority.
  • Pin explicit scrypt cost params (N, r, p) so a future Node default change can't alter derivation.
  • Tighten the Credentials type off the Record<string, unknown> catch-all so a future kind can't accidentally store a secret in the clear.
Calibration

Strong fundamentals (verified) Good

Injection & input

  • All raw SQL is parameterized — including PostGIS: the GeoJSON is bound, table names go through sql.identifier().
  • GPX/XML parsing is XXE-safe (DOMParser / linkedom, no external-entity or DTD expansion).
  • safeReturnTo() blocks open redirects (local paths only, no //).

Secrets

  • requireSecret() refuses to boot prod on a dev-fallback secret.
  • AES-256-GCM + random IV; SOPS/age for secrets at rest; .env gitignored and never in history.
  • Sentry: sendDefaultPii: false, no session replay.

Auth & authz

  • Single-use JWT (consumed_jwt_jti, atomic ON CONFLICT) and single-use magic tokens (atomic UPDATE … RETURNING) — both race-proof.
  • Session cookies: HttpOnly, Secure in prod, SameSite=Lax, signed.
  • Branded ownership loader makes IDOR a compile error (from the prior refactor).
  • Terms gate enforced on both web and bearer-token API.

Boundary & infra

  • E2E backdoor endpoints gated on E2E env, never set in prod compose.
  • Strong headers + CSP (HSTS, nosniff, frame-deny, no unsafe-eval); Garmin webhook host allowlist; app ports unpublished behind Caddy; deploy secrets SOPS-encrypted, not echoed.
Summary

Priorities

#FindingSevDo
1SSRF via Planner session callback URLHighApply validateFetchUrl at create + fetch
2Upload content-type / filename / ownershipMediumAllowlist type, sanitize key, check owner
3OAuth callback logs raw exceptionLowLog redacted message only
4Caddy admin on 0.0.0.0:2019LowBind to localhost / admin off
5User enumeration via auth errorsLowGeneric responses on the email paths
6Federation tests + fetch capsLowAdd signature-rejection test, size cap
7In-process rate limit / XFF trustLowShared store before scaling out
8Komoot credential edgesInfoEncrypt email, pin scrypt params

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.