Commit Graph
4 Commits
Author SHA1 Message Date
bvandeusen 09b5f874b6 Security values move into the Settings UI
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Failing after 9s
CI & Build / Build now, or wait for Android? (push) Successful in 2s
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Failing after 12s
CI & Build / Build & push image (push) Successful in 32s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m17s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m14s
Desktop (Tauri) / Update manifest (push) Successful in 5s
Operator: *"proxy hops defaults to 1 and should be in the settings UI not in the
envs, we need the security values to be in the UI."* Overrules the call I made
yesterday, and rule 25 is on your side — I argued deployment-topology, but the
operator has to be able to SEE what protects them, and reading a container's
environment is not seeing.

Six new settings in a **Security** group: trusted proxy hops (default 1), the
per-account and per-address sign-in limits with their shared window, and the
sign-up limit with its own. `THOUGHTSYNC_TRUSTED_PROXY_HOPS` is gone; the rate
limits are no longer hardcoded constants.

**The hard part was keeping the throttle cheap.** It consults these BEFORE opening
a database connection — deliberately, because a refused attempt is meant to cost
nothing, and the hop count is needed to know who is even asking. A query per
attempt would undo both. So there is a small cache seeded from the registry
defaults (the app works with no database at all, which is what the DB-free unit
lane relies on), loaded at boot, and refreshed on every settings save — the same
live-update contract `session_ttl_days` already had.

`SlidingWindow` now takes its limit and window as SUPPLIERS rather than values, so
a saved number applies to the next attempt instead of the next deploy.

**Bounds are rejected, not clamped.** A hop count of 99 would trust anything a
caller sent; a sign-in limit of 0 would lock every account out permanently. Both
now fail validation with a message naming the range, and the number input carries
min/max so the browser objects first. Silently storing a different number than the
one typed is how somebody ends up believing a protection is set to something it is
not.

`MAX_BUCKETS` stays a constant on purpose: it protects the limiter from itself
rather than the app from a caller, and there is no operator judgment to apply.

Two integration tests, because the whole point is the round trip: a dangerous
value refused, a legitimate one reaching the cache the throttle reads and
persisting; and every Security row reaching the admin payload with bounds and a
description that explains itself.
2026-08-23 15:24:15 -04:00
bvandeusen a85c53ba2c Trust proxy headers by hop count, and log every credential event
CI & Build / Build now, or wait for Android? (push) Successful in 2s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 6s
CI & Build / Python tests (push) Successful in 9s
CI & Build / integration (push) Successful in 12s
CI & Build / Build & push image (push) Successful in 32s
Operator, before exposing the instance: *"I'd expect that we should have a proxy
hops setting for how many proxy hops we should trust a shared real-ip at… and is
there any session logging."* Neither existed, and the first one was a real hole.

**The address was forgeable.** `client_address()` read the LEFTMOST
`X-Forwarded-For` entry — nominally "the original client", and precisely the one a
caller controls, because anything they send arrives before what proxies append. So
`curl -H "X-Forwarded-For: 1.2.3.4"`, rotated per request, minted a fresh
rate-limit bucket every time.

Concretely: stuffing ONE account stayed limited (the account key is unforgeable
and that is why it exists), but spraying MANY accounts from one source was not —
each account got its own budget, and the per-address cap meant to bound the total
was defeated by a header. On a LAN that is nothing. It is not nothing on a public
host.

Now it counts in from the RIGHT by `THOUGHTSYNC_TRUSTED_PROXY_HOPS`, default 1.
Each hop appends what it saw, so the rightmost entries are the ones our own
infrastructure wrote and a forged prefix lands to the left of them where it can
never be selected — proven for the honest, forged, padded, CDN and
shorter-than-configured cases. 0 ignores the header entirely; 2 is Cloudflare in
front of a proxy. Too high is the dangerous direction, so a header shorter than
configured falls back to the socket address rather than reaching further left.

`X-Forwarded-Proto` had the same bug and now shares the same rule. Both live in a
new `proxy.py` rather than being written twice — two places holding one decision
is how issue 2183 happened, and this is the same decision.

Env rather than the Settings UI, against rule 25's usual pull: it is deployment
topology rather than preference, and the limiter consults it BEFORE opening a
database connection, which is the entire point of checking a throttle before doing
expensive work. Easy to move if that reads wrong.

**And there was no logging at all** — `auth.py` had no logger, and the only record
of anything was `device_tokens.last_used_at`. Sign-ins, failures, throttle trips,
new accounts and device-token issuance now all log, with the attempted email and
the trusted address. Deliberately including the email: it is the operator's own
server, and "somebody failed a login" without saying against which account is not
actionable.

`basicConfig` at INFO in `create_app`, because hypercorn configures its own loggers
and leaves the root at WARNING — without it every line above would have gone
nowhere, which is a worse failure than not writing them.

This is the app log, not an audit table. Not queryable, not retained past log
rotation. The table is task 2939; this is what makes the next few days observable.
2026-08-23 15:12:14 -04:00
bvandeusen bacedea8a3 tests: seed the throttle counters on the clock the routes actually read
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 13s
CI & Build / Build & push image (push) Successful in 19s
The three route tests stamped their pre-loaded hits at t=0..9 through the
injected clock, then called a route that reads `time.monotonic()`. Against a
trailing window those hits are fifteen minutes stale on arrival, so they were
pruned before they could refuse anything, the request carried on to the database
that this suite doesn't have, and the assertion read `500 == 429`.

The window's own tests keep the injected clock — they pass the same one to both
sides, which is what makes them deterministic and instant. Only the tests that
hand off to a route need the real one.
2026-08-21 22:03:19 -04:00
bvandeusen b6152ec18b server: harden the surfaces a public deployment leaves exposed
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 9s
CI & Build / Python tests (push) Failing after 11s
CI & Build / Build & push image (push) Successful in 33s
On a LAN the login form is reachable by people you already trust. Exposed, it is
reachable by everyone, and nothing in front of it was counting.

Three credential routes — /login, /register and /device-login — now throttle.
Every attempt is counted against BOTH the account and the calling address, and
either can refuse it. The account key is the one that matters and the one that
cannot be forged: it stops stuffing against a known email no matter how many
addresses the attempts arrive from. The address key bounds one source spraying
many accounts, and is best-effort by nature — behind a proxy it comes from
X-Forwarded-For, which a caller can set to anything if the app is exposed
directly. That is exactly why it isn't the only key.

The check runs BEFORE the password is verified, which is the other half of what
this protects. bcrypt is deliberately slow; an unauthenticated caller who can
trigger it without limit has a CPU exhaustion primitive as well as a guessing
one. Sliding rather than fixed windows, because a fixed one lets twice the limit
through across a boundary. Bucket count is capped so a rotating forged header
can't turn the limiter into the exhaustion it prevents.

A sign-in against an email with no account now spends a real bcrypt against a
throwaway hash first. Without it "no such account" returned in microseconds
while a wrong password took ~100ms, which is a reliable oracle for which emails
are registered here.

Every response carries a CSP with script-src 'self', object-src 'none' and
frame-ancestors 'none', plus nosniff, a referrer policy and a permissions
policy. The app has no inline and no third-party scripts, so this concedes
nothing; the exceptions are honest — inline STYLE (Vue writes it itself for
v-show and the FLIP), and remote images (a link preview renders the og:image of
an arbitrary host, over either scheme, since a LAN install is served over http).
HSTS only where the request already arrived over TLS, and scoped to the one
host: no includeSubDomains, no preload, neither of which is this app's to
commit.

X-Forwarded-Proto detection moved into one `_is_https()` — the session cookie's
Secure flag and HSTS are the same question, and answering it twice is how the
two drift apart.

docs/public-hosting.md is the rest of it: the four things only the operator can
do (close registration, terminate TLS and forward the scheme, stop publishing
the app port, back up the attachment volume as well as the database), and an
honest list of what the app does NOT have — no email verification, no password
reset, no second factor, no per-user quota, no audit log. Those aren't blockers
for an instance whose accounts are people you know. They're the reason not to
leave signups open to strangers.
2026-08-21 22:01:37 -04:00