16 Commits
Author SHA1 Message Date
bvandeusen 1b3e29e4f6 0.2.0 — a notebook in your pocket, ready to be hosted (#3)
Android / Kotlin + Rust (APK) (push) Successful in 7m45s
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 11s
CI & Build / integration (push) Successful in 16s
CI & Build / Build & push image (push) Successful in 15s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m18s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m18s
Desktop (Tauri) / Update manifest (push) Successful in 5s
2026-08-23 16:38:00 -04:00
bvandeusenandClaude Opus 5 c851b901df The proxy-hops test still read the value from Config
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 8s
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Successful in 14s
CI & Build / Build & push image (push) Successful in 15s
09b5f87 moved trusted_proxy_hops out of the environment and into the
settings registry, but tests/test_proxy.py kept asserting against
Config.trusted_proxy_hops() — which no longer exists. The unit lane has
been red since that commit.

Assert through live() instead. That is what proxy.py actually calls, and
it is seeded from the defaults at import time, so the test covers the
case that matters: a boot that has not reached the database yet still
counts one hop rather than zero.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 16:31:21 -04:00
bvandeusen abe01da5f7 compose: say which of the three deployment shapes you're in
The operator asked why `THOUGHTSYNC_BIND` isn't just defaulted to the safe value.
Fair question, and the answer exposed that my own advice was incomplete: I told
them to set it to 127.0.0.1 without asking where their proxy runs, and for a
proxy inside Docker that is the wrong fix.

There are three shapes, not two:

1. **LAN, no proxy** — the default. Binds every interface so a phone and a desktop
   can reach the server. This is why the default is NOT the locked-down value: a
   server reachable only from the machine it runs on isn't hardened, it's broken,
   and that is the primary documented use of this app.
2. **Proxy in Docker** — delete the `ports:` block entirely. The proxy reaches the
   app over the compose network; publishing a host port is a second,
   unauthenticated way in that bypasses whatever the proxy does about TLS. Safer
   than 127.0.0.1, because there is no host port to reach even from the host.
3. **Proxy on the host** — `THOUGHTSYNC_BIND=127.0.0.1`.

The compose file now spells out all three where the decision is made, and
`docs/public-hosting.md` item 4 asks where your proxy runs before telling you what
to do, plus how to check: `curl http://<lan-ip>:5000/api/health` from another
machine should NOT answer once you're proxied.

No default changed. Changing it would silently break every LAN install on the next
`docker compose pull` — the phone would just stop syncing, with nothing saying why.
2026-08-23 15:30:01 -04:00
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 2141a0ac45 Registration closes itself once the instance has an owner
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 11s
CI & Build / integration (push) Successful in 17s
CI & Build / Build & push image (push) Successful in 28s
Operator: *"registration should be open only for the first user and they get
granted admin privileges. then registration is closed."*

The old shape had a window in it. The first account was always allowed and became
admin; every account after that was gated by `allow_registration` — which
defaulted to ON. So the door stayed open between "my account exists" and "I
remembered to turn it off in Settings", and on a public host that gap is the
entire exposure: it starts the moment DNS resolves and lasts until someone
remembers.

Now the door shuts as a CONSEQUENCE of the admin account existing, in the same
transaction that creates it. Not "defaults closed" — that would still need the
first person to get in somehow. There is no window to remember, because there is
no window.

Re-opening it is a deliberate act in Settings → Access: turn it on, have the
person register, turn it off. Crude, and it is the only mechanism there is —
**there is no invite system**, not even a stub. That is real work (a token table,
admin create/revoke, a redemption flow, expiry) and is filed as later work rather
than smuggled into a release.

An integration test covers it, because it is the interaction between two writes
in one transaction: first register → 201 and `is_admin: true`; the setting is
then false; a second register → 403; re-open deliberately and a third → 201, not
admin.

**This does not retroactively close an instance that already has users.** The
close fires on first-account creation, so a server whose admin predates this
keeps whatever the setting was — which was on. `docs/public-hosting.md` now says
so explicitly, and step 1 of the checklist is "check" rather than "do" for
exactly that reason.
2026-08-23 14:18:58 -04:00
bvandeusen 1aca294b95 Bump to 0.2.0 — a release at 0.1.0 would have been a downgrade
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 4s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Build & push image (push) Skipped
CI & Build / Python tests (push) Successful in 10s
CI & Build / integration (push) Successful in 14s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m59s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 5m9s
Desktop (Tauri) / Update manifest (push) Successful in 4s
Android / Kotlin + Rust (APK) (push) Successful in 8m0s
Found while preparing the release, and it would have quietly defeated the point
of cutting one.

The version does NOT come from the tag. `desktop/packaging/build-version.sh`
reads `desktop/src-tauri/Cargo.toml`, and on `dev` it appends the CI run number
(`0.1.269`) while on a tag or `main` it ships the file's value verbatim — which
was still `0.1.0`.

So tagging today would have published a "release" numbered BELOW every dev build
already out there, and below the 0.1.227 on the operator's phone. The desktop
updater compares semver: an installed build would have read the stable manifest,
seen a version older than its own, and correctly concluded it was already
current. The APK would have installed (versionCode is the run number and keeps
rising) while displaying a version that reads as going backwards.

build-version.sh predicted this in its own comment: "Bumping the minor in
Cargo.toml still wins over any dev build on the old line, which is the ordering
you want: 0.2.0 > 0.1.2932."

Bumped in four places, which is every one that can be read by something:
- `desktop/src-tauri/Cargo.toml` — the actual source; everything else derives
- `tauri.conf.json` — overridden at build time by `--config`, but a checked-in
  value that lies is exactly how issue 2183 happened
- `pyproject.toml` + `__init__.py` — the server's APP_VERSION fallback when no
  BUILD_VERSION is injected

`core` and `android/ffi` stay at 0.1.0 deliberately: internal library crates whose
version reaches no surface, and versioning workspace libs independently of the
app is normal.

Cargo.lock regenerated with `cargo fetch` per ci-requirements — one line, the
version itself. Verified: a tag build now yields 0.2.0 and a dev build 0.2.270,
so stable is an upgrade for every existing install and dev stays ahead of stable.
2026-08-23 14:02:02 -04:00
bvandeusen 7033995975 search is a facet on the board, not a place you go
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 4s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Python tests (push) Successful in 16s
CI & Build / integration (push) Successful in 18s
CI & Build / Build & push image (push) Successful in 37s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m3s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m13s
Desktop (Tauri) / Update manifest (push) Successful in 5s
Operator (note 2930): tags exist so you can *"filter during a search"*. The
server has always been able to do that — `GET /api/notes` composes `?q=` with
`?label=` and the rest into one AND-ed query. The frontend never reached it.

The header search box navigated to `/search`, and that view called a DIFFERENT
endpoint — `GET /api/notes/search?q=`, full text only, no facets at all. So the
one screen you landed on when you searched was the one screen where you could not
narrow by tag. Tag filtering lived on the board's FilterBar, which is where you
weren't searching. Two search boxes, two endpoints, and only the hidden one did
what tags are for.

Now the header box writes `?q=` into the board's URL beside whatever labels are
already there, and stays on the lens you're in — searching while looking at Trash
searches Trash. The box READS from the URL rather than holding its own copy, so
it stays in step with the Filters panel's Clear and with a saved view opened from
the sidebar.

Deleted: `SearchView.vue`, its route, `GET /api/notes/search`, `repo.notes.search`
and both adapter implementations, and the `notes_search` Tauri command whose only
caller was the adapter entry. FilterBar loses its own "Search text…" input — it
was the same facet, hidden behind a collapsed panel, duplicating a box that is
always on screen. Filters now does what its name says: narrowing. The header does
searching.

`core::store::search` STAYS. Android calls it through the FFI (`search_notes`) and
has its own search surface — which has the same no-tag-filter gap the web just
lost, and deserves the same fix on its own terms rather than as a rider here.
2026-08-23 10:58:23 -04:00
bvandeusen de72d27bd4 URLs unfurl on their own, and a lone link becomes the note
CI & Build / Python lint (push) Successful in 2s
CI & Build / Python tests (push) Successful in 9s
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / integration (push) Successful in 14s
CI & Build / Build & push image (push) Successful in 37s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m6s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m16s
Desktop (Tauri) / Update manifest (push) Successful in 6s
Operator: *"I'd like for URLs to unfurl. To be the whole note when the note is a
single URL, and to be a compact slot on the bottom of the note when the URL is
inline. We also need to support multiple URLs in a single note."*

Less new machinery than it sounds: `unfurl.py` already fetched and parsed OG
tags, SSRF-hardened, and `note_link_previews` was already `UNIQUE(note_id, url)`
— so several URLs per note has worked at the storage layer all along. What was
missing was that it needed a button, had one size, and drew that size in the
wrong place.

**Automatic, and never in the way.** New `unfurl_queue.py` detects a body's URLs
and fetches them on a background task AFTER the note is committed. Capture speed
is the product: an unfurl is a five-second timeout against a host nobody
controls, and a note has to persist the instant someone stops typing. Scheduled
from create, from a body edit, and from a synced push — so a linked desktop or
Android client gets previews too, on its next pull. An unlinked one has no server
to ask and simply has none, which is the honest consequence of being offline.

Safe to call on every save: it re-reads what's cached and does nothing when
nothing is new. Capped at five URLs per note, silent on every failure (a link
that won't fetch isn't an error the person needs — the note is fine, the link is
still there), and it re-checks before storing, so a slow fetch can't resurrect a
preview for a URL that was deleted while it was in flight.

**Two presentations.** A note whose body is nothing but a URL renders as its
preview and nothing else — printing the raw URL under a card that already says
where it goes is saying the same thing twice, badly. Until the fetch lands, or if
it never does, the URL stands in, so the card is never blank. Anything else gets
a compact strip.

**And the strip moved.** Previews were rendered ABOVE the body, which put a
stranger's headline where the note's own first line should be — worse now that
the first line IS the note's name. They sit at the foot of the card now, under
the note's own words.

The editor's "Preview example.com" button is gone with the manual path; removing
an unwanted preview stays, and stays editor-only.

Nine tests: three on detection (order, dedupe, sentence-punctuation trimming,
non-http rejection) in the unit lane, and three in the integration lane for what
only a real database shows — the upsert landing on the right row, a second pass
fetching nothing, and a preview NOT being stored for a URL that left the body.
2026-08-23 01:05:37 -04:00
bvandeusen c99cbb3e14 cards: clamp the web note preview, as Android always has
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 6s
CI & Build / Python tests (push) Successful in 12s
CI & Build / integration (push) Successful in 13s
CI & Build / Build & push image (push) Successful in 29s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m16s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m24s
Desktop (Tauri) / Update manifest (push) Successful in 4s
M13 step 4 asked for no bold first line, and step 3 already delivered that —
removing `note.title` took the card's <h3> and the Android editor's bold field
with it. What step 4 also asked for, and hadn't been done, was the other half:
"be willing to spend something small on legibility that isn't weight on the
first line."

The web card rendered the entire body. Android has always clamped to eight lines
(`MAX_PREVIEW_LINES`), so one long note produced a card taller than the screen on
the web and pushed the rest of the board off it — a real asymmetry between two
surfaces that are supposed to be peers.

It matters more without a title. The first line used to be what your eye caught;
with one weight throughout, an unbounded card is just a wall, and the note beside
it is the one you were actually looking for.

Clamped in the STRING, not with CSS `line-clamp` — that needs a `-webkit-box` and
behaves unreliably around the block elements MarkdownText emits (lists, quotes,
fenced code). Doing it before the parse is deterministic, matches Android's
semantics exactly, and skips parsing a body the card was never going to show.
2026-08-23 01:00:39 -04:00
bvandeusen 6f21db85a1 ci: an integration lane, so the migrations are finally run by something
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 8s
CI & Build / integration (push) Successful in 14s
CI & Build / Build & push image (push) Successful in 25s
26 Alembic revisions and none had ever been executed by CI. `alembic upgrade
head` ran for the first time when the operator's container started, and the
schema the migrations build had never been checked against the models that read
it. M13 dropped three columns and rebuilt a STORED GENERATED column with nothing
watching but a server boot.

Copied from FabledScribe's `integration` job, which had already solved the parts
that are easy to get wrong — and which are family rules precisely because they
were: a separator-free job key with no `name:` (act_runner derives the service
container name from the truncated display name, and the discovery step filters
`docker ps` by it), bridge-IP resolution because service hostnames aren't
routable on this runner, and a Python readiness wait because `run:` is busybox
sh with no `/dev/tcp`.

`postgres:16-alpine` to match the production compose. The schema is built by
real migrations, never metadata.create_all — that step IS the migration test.

Six tests, each pinning something that has only ever been checked by hand:

- an ORM insert against the migrated schema, which is the model/migration
  agreement nothing has verified until now;
- `notes.title`, `notes.kind` and `note_revisions.title` are actually gone, and
  `note_links` with them — a silently no-op migration shows up here;
- the rebuilt `search_vector` indexes both the name and the body, which matters
  because 0026 had to DROP and recreate a generated column rather than alter it;
- a note keeps its body AND its items, the shape step 2 made normal;
- `_apply_note_items` leaves items alone when a change doesn't mention them —
  the data-loss path step 2 removed, pinned so its return would be caught;
- a note with no body is still named by its first item, the hole that made
  removing the title unsafe until checklists stopped being their own kind.

Runs for visibility; does not gate the build, matching `test` and Scribe.

No local equivalent: running it means standing up Postgres on the workstation,
which rule 12 reserves for an explicit request. Documented in ci-requirements
alongside the Rust, Kotlin and frontend gates.
2026-08-23 00:24:57 -04:00
bvandeusen 924ddb20db notes: saveEdit still asked for a title
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 6s
CI & Build / Python tests (push) Successful in 8s
CI & Build / Build & push image (push) Successful in 35s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m0s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m10s
Desktop (Tauri) / Update manifest (push) Successful in 4s
The one thing step 3 missed, and the typecheck lane caught it: `saveEdit`'s
parameter type still declared `title`, so the editor's call — correctly no
longer passing one — didn't match.

I gated Rust locally and not the frontend. Both are now in ci-requirements,
including WHY the frontend one has to be `npm run build` rather than
`vue-tsc --noEmit`: the typecheck only reads the script block, so a malformed
template sails past it and fails `vite build` in a different workflow, which is
exactly how the stray `</div>` got two commits away from where it was written.
2026-08-22 21:38:13 -04:00
bvandeusen 95aa10c2c3 Remove the title field — a note is named by its first line
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) Failing after 7s
CI & Build / Build & push image (push) Skipped
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 7s
CI & Build / Python tests (push) Successful in 11s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Failing after 31s
Desktop (Tauri) / Update manifest (push) Skipped
Android / Kotlin + Rust (APK) (push) Successful in 6m45s
Operator (note 2897): "notes shouldn't have a title field." The concept of a NAME
stays — search results, export filenames and the command palette all need one —
but nothing is typed into it any more. `display_title` is now the first non-empty
line of the body, falling back to the first checklist item.

That fallback is what step 2 bought, and the reason this could not go first: a
checklist had no body to be named from, so the title was its only name. Now every
note has a body, and a note that is only a checklist is named by its first item.

Gone everywhere: the column and note_revisions.title (0026), the field on the
core's Note/NoteCreateInput/NoteRevision and its SQLite columns (user_version 7),
`normalize_title`, the wire field, the FFI record and `NoteEdit::Title` /
`ClearTitle`, the web editor's "Title (optional)" input and the card's <h3>, and
the Android title field in both the compose sheet and the editor.

**The search vector had to be rebuilt, not just left alone.** `notes.search_vector`
is a STORED GENERATED column whose expression names `title` — Postgres refuses to
drop a column another generated column depends on. It is dropped and recreated over
`display_title` at weight A, which keeps the original intent: a note's NAME ranks
above the rest of its body.

**An imported title becomes the note's first body line.** Keep notes carry one, and
so does any ThoughtSync export taken before this. Dropping it would silently lose
text someone wrote; folding it in puts it exactly where a name now lives, so the
note arrives named as it was. Skipped when the body already opens with that line,
so re-importing an export this code produced doesn't stack duplicates.

Two smaller things fell out. The Android editor loses its bold first field — one
weight throughout, because the first line is the note's name but not a different
KIND of text, which is most of step 4 arriving early. And `ClearTitle`'s
justification comment moved to `ClearRemindAt`, which is now the surviving example
of why NoteEdit is a list rather than a struct of options.

Protocol note corrected to say what actually shipped: v2 is "no kind, no title",
one bump for the pair.

Verified with the local Rust gate this time, not by CI: fmt, clippy and 116 tests
all green before pushing. It caught four things — orphaned serde attributes where
fields were removed, a `wire::Preview.title` I deleted by mistake (a link preview
still has one), nine retention fixtures inserting a dropped column, and four
rustfmt diffs.
2026-08-22 19:33:57 -04:00
bvandeusen 6d778f26a7 Fix the ktlint and compat-test failures, and start using the Rust gate
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m44s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m46s
Desktop (Tauri) / Update manifest (push) Successful in 5s
Android / Kotlin + Rust (APK) (push) Successful in 7m47s
Two more from the step-2 removals:

**Two unused Kotlin imports** — `FilterChip` (the Note/List switch) and
`Icons.Filled.Create` (the "switch to a note" icon), both orphaned when their
callers went. ktlint treats them as errors.

**`server_info_tolerates_unknown_and_absent_fields`** pinned
`sync_protocol_version: 1` as a literal, so bumping the protocol to v2 made it
fail for a reason that has nothing to do with what it tests. It is about unknown
FIELDS; the versions now come from `CLIENT_PROTOCOL_VERSION`, like every other
test in that file already did.

The bigger fix is the habit. `ci-requirements.md` has documented since
2026-08-18 that the operator authorised running fmt/clippy/test against the CI
image locally, and I had not been doing it. All three now pass here — 116 tests,
clippy clean, fmt clean — and every Rust failure in this milestone so far would
have been caught by them in under a minute instead of by CI, several commits
downstream. Noted in ci-requirements so the next session doesn't relearn it: a
removal is exactly the change that looks too safe to check.
2026-08-22 14:51:38 -04:00
bvandeusen 33e9278975 Fix three breaks the removals left behind
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Build & push image (push) Skipped
CI & Build / Python tests (push) Successful in 8s
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 2m53s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 3m4s
Desktop (Tauri) / Update manifest (push) Skipped
Android / Kotlin + Rust (APK) (push) Failing after 4m27s
**`snapshot_revision` was deleted with `create_titled`** (bc22f8e). I sliced the
function out by scanning to the next `pub fn`, and the private `fn` sitting
between them went too. Nothing in Python or TypeScript compiles Rust, so it sat
undetected until the first lane that does. Restored verbatim.

**An orphaned serde attribute** in push.rs: removing `pub kind: Option<String>`
left its `#[serde(skip_serializing_if)]` behind, which then stacked onto the
next field's. That failed the derive, which is why three follow-on errors all
said `Change: Serialize is not satisfied` — one cause, four messages.

**An unbalanced `</div>`** in NoteEditor.vue, orphaned when the "Links / Linked
from" footer was cut. `vue-tsc --noEmit` type-checks the SCRIPT block and never
parses the template, so the typecheck lane passed it and `vite build` caught it
two workflows later. Worth remembering: a green typecheck says nothing about
template structure.

I also pushed step 2 without waiting for ad21eac to go terminal, which is what
let the Rust break travel a commit further than it should have.

Each fix comes with the check that would have caught it: a scan for stacked
serde attributes and called-but-undefined fns across every .rs, and a tag
balance pass over every .vue. Both are clean.
2026-08-22 13:25:26 -04:00
bvandeusen 867405fae2 M12 — the Android client, end to end (#2)
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 7s
CI & Build / Build & push image (push) Skipped
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m47s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 5m8s
Desktop (Tauri) / Update manifest (push) Skipped
Android / Kotlin + Rust (APK) (push) Successful in 7m13s
CI & Build / Python tests (push) Successful in 11s
86 commits from dev. Native Kotlin/Compose Android client over the shared Rust
core, the server-served distribution path, signing, and self-update.

Known and recorded rather than fixed: #2810 (release APK carries a debug-profile
.so), allowBackup still true now that the store holds a device token, and
cleartext HTTP enabled app-wide for self-hosted LAN servers.
2026-08-21 08:53:57 -04:00
62 changed files with 1798 additions and 738 deletions
+10
View File
@@ -39,6 +39,16 @@ POSTGRES_PASSWORD=
# 127.0.0.1 so only the proxy can talk to it.
#THOUGHTSYNC_BIND=0.0.0.0
# NOTE: how many proxies sit in front of this app is a SETTING, not an env var —
# Settings → Security → "Trusted proxy hops" in the admin UI. It defaults to 1 (one
# reverse proxy terminating HTTPS) and belongs there because it is something you may
# need to change while the server is running, alongside the sign-in limits.
# How much the app says. Credential events (sign-ins, failures, throttles, new
# accounts, device tokens issued) are logged at INFO and read with
# `docker compose logs app`.
#THOUGHTSYNC_LOG_LEVEL=INFO
# Database identity. Changing these AFTER the first start does not rename anything
# that already exists — the volume keeps whatever the first run created.
#POSTGRES_USER=thoughtsync
+74 -1
View File
@@ -184,7 +184,80 @@ jobs:
run: uv pip install --python /opt/venv/bin/python -e ".[dev]"
- name: Run tests
run: /opt/venv/bin/python -m pytest tests/ -q
# DB-free by design. Anything needing a real Postgres is marked `integration`
# and runs in the job below.
run: /opt/venv/bin/python -m pytest tests/ -q -m "not integration"
# Real-Postgres lane (family rule 6). Until this existed, `alembic upgrade head` ran
# for the first time when the operator's container started — 26 revisions, none of
# them ever executed by CI — and the schema the migrations build had never been
# checked against the models that read it.
#
# Runs for visibility and does NOT gate the build, matching the `test` lane and
# FabledScribe's equivalent job.
#
# Job key stays separator-free ("integration") with no `name:` — rule 80. act_runner
# derives the service-container name from the truncated job display name, and the
# discovery step below filters `docker ps` by it. Service hostnames are not routable
# on this runner (rule 79), so the step resolves the container's bridge IP.
integration:
if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v')
runs-on: python-ci
container:
image: git.fabledsword.com/bvandeusen/ci-python:3.14
services:
postgres:
# Same image the production compose runs, so the schema is proven against the
# Postgres it will actually meet.
image: postgres:16-alpine
env:
POSTGRES_USER: thoughtsync
POSTGRES_PASSWORD: ci_integration
POSTGRES_DB: thoughtsync_test
options: >-
--health-cmd "pg_isready -U thoughtsync"
--health-interval 10s
--health-timeout 5s
--health-retries 10
steps:
- uses: actions/checkout@v6
- name: Create virtual environment
run: uv venv /opt/venv
# Same install as the unit lane — the two must agree on versions, or
# "unit green, integration red" stops being a signal about the code.
- name: Install package with dev deps
run: uv pip install --python /opt/venv/bin/python -e ".[dev]"
- name: Integration suite (resolve service IP, migrate, test)
run: |
set -eux
echo "=== container landscape (diagnostic for the name filter) ==="
docker ps -a --format '{{.ID}} {{.Image}} -> {{.Names}}'
PG=$(docker ps --filter "name=integration" --filter "ancestor=postgres:16-alpine" -q | head -n1)
test -n "$PG"
PG_IP=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$PG")
test -n "$PG_IP"
export THOUGHTSYNC_DATABASE_URL="postgresql+asyncpg://thoughtsync:ci_integration@${PG_IP}:5432/thoughtsync_test"
# Wait for Postgres to accept connections. `run:` is busybox sh (rule 81) —
# no bash /dev/tcp — so use the Python that is always present here.
/opt/venv/bin/python - "$PG_IP" <<'PY'
import socket, sys, time
for _ in range(30):
try:
socket.create_connection((sys.argv[1], 5432), timeout=2).close()
break
except OSError:
time.sleep(1)
else:
sys.exit("postgres did not become reachable")
PY
# Real migrations build the schema, never metadata.create_all (rule 82) —
# testing a schema no deployment has ever seen would prove nothing. This
# step IS the migration test: a broken revision fails the job here.
/opt/venv/bin/alembic upgrade head
/opt/venv/bin/python -m pytest tests/ -v -m integration
build:
name: Build & push image
Generated
+1 -1
View File
@@ -4189,7 +4189,7 @@ dependencies = [
[[package]]
name = "thoughtsync-desktop"
version = "0.1.0"
version = "0.2.0"
dependencies = [
"log",
"serde",
+82
View File
@@ -0,0 +1,82 @@
"""drop notes.title and note_revisions.title — a note's name is its first line
Revision ID: 0026
Revises: 0025
Create Date: 2026-08-22
M13 step 3. A note is a body plus optional checkable items; its NAME is the first
non-empty line of that body, falling back to its first checklist item. There is no
separate field to type into, and `display_title` (already persisted, already what
search results and export filenames read) carries the name.
## The search vector has to be rebuilt, not just left alone
`notes.search_vector` is a STORED GENERATED column whose expression names `title`
(migration 0005, weight A) — Postgres will refuse to drop a column another generated
column depends on, and even if it didn't, the weighting would be wrong. So it is
dropped and recreated over `display_title` instead, which keeps the original
intent: the note's NAME ranks above the rest of its body.
Rebuilding a stored generated column re-computes every row, and the GIN index is
rebuilt with it. On a personal instance that is milliseconds; it is worth knowing
before running this against something large.
## What happens to existing titles
Nothing preserves them, deliberately: `display_title` was already derived from the
title when one was set, so every note keeps the NAME it had. What is lost is the
distinction between "this note has an explicit title" and "this note's first line is
its name" — which is the distinction being removed.
Imports are the exception and are handled in code, not here: a Keep note's title, or
one in an export taken before this, is folded in as the note's first body line rather
than dropped (see `_create_imported_note`).
"""
from alembic import op
import sqlalchemy as sa
revision = "0026"
down_revision = "0025"
branch_labels = None
depends_on = None
def upgrade() -> None:
# Order matters: the generated column depends on `title`, so it goes first.
op.execute("DROP INDEX IF EXISTS ix_notes_search")
op.execute("ALTER TABLE notes DROP COLUMN IF EXISTS search_vector")
op.drop_column("notes", "title")
op.drop_column("note_revisions", "title")
op.execute(
"""
ALTER TABLE notes ADD COLUMN search_vector tsvector
GENERATED ALWAYS AS (
setweight(to_tsvector('english', coalesce(display_title, '')), 'A') ||
setweight(to_tsvector('english', coalesce(body, '')), 'B')
) STORED
"""
)
op.execute("CREATE INDEX ix_notes_search ON notes USING GIN (search_vector)")
def downgrade() -> None:
op.execute("DROP INDEX IF EXISTS ix_notes_search")
op.execute("ALTER TABLE notes DROP COLUMN IF EXISTS search_vector")
# Comes back empty. The text is not gone — it is the first line of every body —
# but which notes once had an explicit title is not recorded anywhere.
op.add_column("notes", sa.Column("title", sa.Text(), nullable=True))
op.add_column("note_revisions", sa.Column("title", sa.Text(), nullable=True))
op.execute(
"""
ALTER TABLE notes ADD COLUMN search_vector tsvector
GENERATED ALWAYS AS (
setweight(to_tsvector('english', coalesce(title, '')), 'A') ||
setweight(to_tsvector('english', coalesce(body, '')), 'B')
) STORED
"""
)
op.execute("CREATE INDEX ix_notes_search ON notes USING GIN (search_vector)")
@@ -226,8 +226,8 @@ private fun App(
ComposeSheet(
saving = board.state.saving,
onDismiss = { composing = false },
onSave = { title, content ->
board.create(title, content)
onSave = { content ->
board.create(content)
composing = false
},
)
@@ -194,19 +194,15 @@ class BoardViewModel(
* Blank input is ignored rather than rejected: an empty save is a slip, not a
* mistake worth interrupting someone over.
*/
fun create(
title: String,
content: String,
) {
val cleanTitle = title.trim()
fun create(content: String) {
val cleanContent = content.trim()
if (cleanTitle.isEmpty() && cleanContent.isEmpty()) return
if (cleanContent.isEmpty()) return
viewModelScope.launch {
state = state.copy(saving = true)
state =
try {
val created = withContext(Dispatchers.IO) { core.createNote(draft(cleanTitle, cleanContent)) }
val created = withContext(Dispatchers.IO) { core.createNote(draft(cleanContent)) }
// Prepend rather than reload: the new note belongs at the top
// of the board, and a full re-query would cost a round trip to
// tell us what we already know. Skipped when the board is not
@@ -277,28 +273,10 @@ class BoardViewModel(
EditorAction.Close -> state = state.copy(editing = null)
EditorAction.DismissError -> dismissError()
// Text is the only edit that batches: title and body are typed
// together and saved together on close, so they cost one write and
// one revision snapshot rather than two of each.
// Saved on close rather than per keystroke, so a session of typing
// costs one write and one revision snapshot.
is EditorAction.SaveText ->
mutate {
it.updateNote(
id,
listOf(
// An emptied title CLEARS the column rather than
// storing "". The core derives `display_title` from
// the first body line when the title is null, so the
// difference is whether an untitled note is nameable
// or blank — exactly what `ClearTitle` exists for.
if (action.title.isBlank()) {
NoteEdit.ClearTitle
} else {
NoteEdit.Title(action.title.trim())
},
NoteEdit.Body(action.body),
),
)
}
mutate { it.updateNote(id, listOf(NoteEdit.Body(action.body))) }
is EditorAction.SetColor -> edit(id, NoteEdit.Color(action.color))
@@ -464,12 +442,8 @@ private fun query(
labelId: String? = null,
) = NoteQuery(view = view, labelId = labelId, sort = null, facets = null)
private fun draft(
title: String,
content: String,
): NoteDraft =
// Body carries the text; the core derives display_title from its first line when
// no title was given, so a captured thought is nameable without making the user
// name it. A checklist is added afterwards, in the editor — it is something a note
// HAS, not a different thing to capture (M13 step 2).
NoteDraft(title = title, body = content, color = DEFAULT_COLOR, items = null)
private fun draft(content: String): NoteDraft =
// The core names the note from the body's first line, so a captured thought is
// findable without anyone being asked to name it. A checklist is added afterwards,
// in the editor — it is something a note HAS, not a different thing to capture.
NoteDraft(body = content, color = DEFAULT_COLOR, items = null)
@@ -9,7 +9,6 @@ import androidx.compose.foundation.layout.navigationBarsPadding
import androidx.compose.foundation.layout.padding
import androidx.compose.material3.Button
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.FilterChip
import androidx.compose.material3.ModalBottomSheet
import androidx.compose.material3.Text
import androidx.compose.material3.TextButton
@@ -58,27 +57,26 @@ import com.fabledsword.thoughtsync.R
fun ComposeSheet(
saving: Boolean,
onDismiss: () -> Unit,
onSave: (String, String) -> Unit,
onSave: (String) -> Unit,
) {
val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true)
// Saveable, not just remembered: a rotation mid-sentence is the same lost
// thought as a discarded one, and it was losing it before this.
var title by rememberSaveable { mutableStateOf("") }
var content by rememberSaveable { mutableStateOf("") }
val contentFocus = remember { FocusRequester() }
val written = title.isNotBlank() || content.isNotBlank()
val leave = { if (written) onSave(title, content) else onDismiss() }
val written = content.isNotBlank()
val leave = { if (written) onSave(content) else onDismiss() }
// Land in the body, not the title. Most captures are a thought, not a titled
// document, and making someone tab past an optional field is the difference
// between "under a second" and not.
// Straight into the one field there is. A capture is a thought, and every field
// someone has to tab past is the difference between "under a second" and not —
// which is why the title field is gone rather than merely skipped (M13 step 3).
LaunchedEffect(Unit) { contentFocus.requestFocus() }
// Backgrounding PERSISTS but does not close an empty sheet. Someone who tapped
// + and then got distracted should find the composer where they left it; the
// only reason to act here is that there is something to lose.
FlushOnStop { if (written) onSave(title, content) }
FlushOnStop { if (written) onSave(content) }
ModalBottomSheet(onDismissRequest = leave, sheetState = sheetState) {
Column(
@@ -92,13 +90,6 @@ fun ComposeSheet(
) {
// No note/list switch any more: there is one thing to capture. A
// checklist is added to a note in the editor, once there is a note.
PlainTextField(
value = title,
onValueChange = { title = it },
hint = R.string.compose_title_hint,
singleLine = true,
)
PlainTextField(
value = content,
onValueChange = { content = it },
@@ -110,7 +101,7 @@ fun ComposeSheet(
SheetActions(
canSave = !saving && written,
onDiscard = onDismiss,
onSave = { onSave(title, content) },
onSave = { onSave(content) },
)
}
}
@@ -22,7 +22,6 @@ sealed interface EditorAction {
data object DismissError : EditorAction
data class SaveText(
val title: String,
val body: String,
) : EditorAction
@@ -14,7 +14,6 @@ import androidx.compose.foundation.shape.CircleShape
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.filled.List
import androidx.compose.material.icons.filled.Close
import androidx.compose.material.icons.filled.Create
import androidx.compose.material.icons.filled.MoreVert
import androidx.compose.material.icons.filled.Notifications
import androidx.compose.material3.BottomAppBar
@@ -21,7 +21,6 @@ import androidx.compose.ui.draw.clip
import androidx.compose.ui.res.pluralStringResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.font.FontStyle
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.style.TextDecoration
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.unit.dp
@@ -50,21 +49,9 @@ fun NoteCard(
.border(1.dp, tint.border(dark), RoundedCornerShape(CARD_RADIUS))
.padding(12.dp),
) {
// A title only renders when one was actually set. `displayTitle` is
// derived from the first body line when it wasn't, so printing both would
// show the same text twice.
note.title?.takeIf { it.isNotBlank() }?.let { title ->
Text(
text = title,
style = MaterialTheme.typography.titleSmall,
fontWeight = FontWeight.SemiBold,
maxLines = 2,
overflow = TextOverflow.Ellipsis,
)
Spacer(Modifier.height(4.dp))
}
// Both, in order — a note can carry a body AND a checklist (M13 step 2).
// Body then checklist, in order — a note can carry both (M13 step 2), and
// nothing above them: the first line of the body IS the note's name, at the
// same weight as the rest of it (M13 steps 3 and 4).
if (note.body.isNotBlank()) {
Text(
text = note.body,
@@ -78,9 +65,9 @@ fun NoteCard(
Checklist(items = note.items)
}
// A note with no title, no body and no items still has to occupy the
// board legibly — otherwise it reads as a rendering bug.
if (note.title.isNullOrBlank() && note.body.isBlank() && note.items.isEmpty()) {
// A note with no body and no items still has to occupy the board legibly —
// otherwise it reads as a rendering bug.
if (note.body.isBlank() && note.items.isEmpty()) {
Text(
text = stringResource(R.string.board_empty_note),
style = MaterialTheme.typography.bodyMedium,
@@ -30,7 +30,6 @@ import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.unit.dp
import com.fabledsword.thoughtsync.R
import com.fabledsword.thoughtsync.core.Label
@@ -62,7 +61,6 @@ fun NoteEditorScreen(
// Keyed by note id: the editor is reused across notes, and without the key the
// second note opened would show the first one's text.
var title by remember(note.id) { mutableStateOf(note.title.orEmpty()) }
var body by remember(note.id) { mutableStateOf(note.body) }
var picker by remember(note.id) { mutableStateOf(Picker.NONE) }
var confirmingDelete by remember(note.id) { mutableStateOf(false) }
@@ -77,8 +75,8 @@ fun NoteEditorScreen(
// would bump `updated_at`, mark the note dirty for sync, and snapshot a
// revision identical to the one before it.
val flush = {
if (!readOnly && (title != note.title.orEmpty() || body != note.body)) {
onAction(EditorAction.SaveText(title, body))
if (!readOnly && body != note.body) {
onAction(EditorAction.SaveText(body))
}
}
val leave = {
@@ -142,14 +140,9 @@ fun NoteEditorScreen(
ErrorBanner(message = message, onDismiss = { onAction(EditorAction.DismissError) })
}
EditorField(
value = title,
onValueChange = { title = it },
hint = R.string.editor_title_hint,
enabled = !readOnly,
bold = true,
)
// One field. A note is its body; its NAME is that body's first line, so
// there is nothing separate to type into and nothing to render bolder
// than the line beneath it (M13 steps 3 and 4).
EditorField(
value = body,
onValueChange = { body = it },
@@ -252,11 +245,15 @@ private fun EditorOverlays(
}
/**
* The title and body fields.
* The note's body field.
*
* Undecorated, via the shared [PlainTextField]: the screen is already painted in
* the note's colour, and a filled field would draw a second surface over the first
* and turn a note into a form.
*
* One weight throughout. The first line is the note's name, but it is not a
* different KIND of text from the line after it, and typing it should not feel like
* filling in a header.
*/
@Composable
private fun EditorField(
@@ -264,7 +261,6 @@ private fun EditorField(
onValueChange: (String) -> Unit,
@StringRes hint: Int,
enabled: Boolean,
bold: Boolean = false,
minLines: Int = 1,
) {
PlainTextField(
@@ -272,16 +268,8 @@ private fun EditorField(
onValueChange = onValueChange,
hint = hint,
enabled = enabled,
// The title is one line by contract — it is a name, and a name that wraps
// has become a body. The body itself never is.
singleLine = bold,
minLines = minLines,
textStyle =
if (bold) {
MaterialTheme.typography.titleLarge.copy(fontWeight = FontWeight.SemiBold)
} else {
MaterialTheme.typography.bodyLarge
},
textStyle = MaterialTheme.typography.bodyLarge,
)
}
@@ -10,7 +10,6 @@
<!-- Compose sheet -->
<string name="compose_open">New note</string>
<string name="compose_title_hint">Title</string>
<string name="compose_body_hint">Take a note…</string>
<string name="compose_discard">Discard</string>
<string name="compose_save">Save</string>
@@ -38,7 +37,6 @@
<!-- Editor -->
<string name="board_open_note">Open note</string>
<string name="editor_back">Back to notes</string>
<string name="editor_title_hint">Title</string>
<string name="editor_add_checklist">Add a checklist</string>
<string name="editor_body_hint">Note</string>
<string name="editor_add_item">Add item</string>
+25 -43
View File
@@ -568,9 +568,8 @@ mod tests {
dir.to_string_lossy().into_owned()
}
fn draft(title: &str, body: &str) -> NoteDraft {
fn draft(body: &str) -> NoteDraft {
NoteDraft {
title: title.to_string(),
body: body.to_string(),
color: "default".to_string(),
items: None,
@@ -587,62 +586,50 @@ mod tests {
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let created = app
.create_note(draft("Groceries", "milk"))
.create_note(draft("Groceries\nmilk"))
.expect("create should succeed");
assert_eq!(created.title.as_deref(), Some("Groceries"));
assert_eq!(created.body, "milk");
assert_eq!(created.body, "Groceries\nmilk");
let fetched = app
.get_note(created.id.clone())
.expect("get should succeed");
assert_eq!(fetched.id, created.id);
// The NAME is the first line — there is no title field to have set (M13 step 3).
assert_eq!(fetched.display_title, "Groceries");
std::fs::remove_dir_all(&dir).ok();
}
/// A body-only note still has to be nameable — that is what `display_title` is
/// for, and the Android board relies on it exactly as the desktop does.
/// Every note has to be nameable — that is what `display_title` is for, and the
/// Android board relies on it exactly as the desktop does.
#[test]
fn body_only_notes_still_have_a_display_title() {
fn a_note_is_named_by_its_first_line() {
let dir = scratch_dir();
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let created = app
.create_note(draft("", "just a thought"))
.create_note(draft("just a thought"))
.expect("create should succeed");
assert_eq!(created.title, None);
assert_eq!(created.display_title, "just a thought");
std::fs::remove_dir_all(&dir).ok();
}
/// Clearing a field and setting one are different edits, and the difference has
/// to survive the trip through the patch object.
/// The hole that made removing the title unsafe until checklists stopped being
/// their own kind of thing: a note with no body text still needs a name.
#[test]
fn edits_can_both_set_and_clear_a_title() {
fn a_note_with_only_items_is_named_by_its_first_item() {
let dir = scratch_dir();
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let note = app.create_note(draft("First", "body")).expect("create");
let renamed = app
.update_note(
note.id.clone(),
vec![NoteEdit::Title {
value: "Second".to_string(),
}],
)
.expect("rename");
assert_eq!(renamed.title.as_deref(), Some("Second"));
let cleared = app
.update_note(note.id.clone(), vec![NoteEdit::ClearTitle])
.expect("clear");
assert_eq!(
cleared.title, None,
"ClearTitle must null the column, not set it to an empty string — the \
distinction is why NoteEdit is a list rather than a struct of options"
);
let created = app
.create_note(NoteDraft {
body: String::new(),
color: "default".to_string(),
items: Some(vec!["milk".to_string(), "eggs".to_string()]),
})
.expect("create should succeed");
assert_eq!(created.display_title, "milk");
std::fs::remove_dir_all(&dir).ok();
}
@@ -673,8 +660,7 @@ mod tests {
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let note = app
.create_note(NoteDraft {
title: "Packing".to_string(),
body: String::new(),
body: "Packing".to_string(),
color: "default".to_string(),
items: Some(vec!["socks".to_string()]),
})
@@ -728,7 +714,7 @@ mod tests {
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let note = app
.create_note(draft("Trip", "book the ferry #travel"))
.create_note(draft("Trip\nbook the ferry #travel"))
.expect("create");
assert_eq!(
note.labels.len(),
@@ -767,7 +753,7 @@ mod tests {
fn deleting_forever_removes_the_note() {
let dir = scratch_dir();
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let note = app.create_note(draft("Ephemeral", "body")).expect("create");
let note = app.create_note(draft("Ephemeral\nbody")).expect("create");
app.delete_note_forever(note.id.clone())
.expect("delete forever");
@@ -784,7 +770,7 @@ mod tests {
fn reminders_can_be_snoozed_and_completed() {
let dir = scratch_dir();
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let note = app.create_note(draft("Call back", "")).expect("create");
let note = app.create_note(draft("Call back")).expect("create");
assert_eq!(note.remind_at, None);
let snoozed = app.snooze_reminder(note.id.clone(), 60).expect("snooze");
@@ -810,9 +796,7 @@ mod tests {
fn completing_a_recurring_reminder_moves_it_rather_than_ending_it() {
let dir = scratch_dir();
let app = ThoughtSync::new(dir.clone()).expect("a fresh data dir should open");
let note = app
.create_note(draft("Water the plants", ""))
.expect("create");
let note = app.create_note(draft("Water the plants")).expect("create");
let armed = app
.update_note(
@@ -849,9 +833,7 @@ mod tests {
// A one-off clears BOTH fields, so an unrecognised rule cannot linger
// invisibly on a note with no reminder.
let once = app
.create_note(draft("Post the letter", ""))
.expect("create");
let once = app.create_note(draft("Post the letter")).expect("create");
app.update_note(
once.id.clone(),
vec![NoteEdit::RemindAt {
+19 -37
View File
@@ -29,9 +29,8 @@ use thoughtsync_core::sync::state as core_state;
#[derive(Debug, Clone, uniffi::Record)]
pub struct Note {
pub id: String,
pub title: Option<String>,
/// Title if set, else the first body line — always present, so a body-only note
/// is still nameable. Derived by the core, never stored.
/// The note's NAME: its first non-blank body line, else its first checklist item.
/// Always present. Derived by the core, never stored.
pub display_title: String,
pub body: String,
pub color: String,
@@ -129,7 +128,6 @@ impl From<core_models::Note> for Note {
// Exhaustive on purpose — see the module header.
let core_models::Note {
id,
title,
display_title,
body,
color,
@@ -149,7 +147,6 @@ impl From<core_models::Note> for Note {
} = value;
Note {
id,
title,
display_title,
body,
color,
@@ -341,7 +338,6 @@ impl From<NoteFacets> for core_models::Facets {
/// A new note.
#[derive(Debug, Clone, uniffi::Record)]
pub struct NoteDraft {
pub title: String,
pub body: String,
/// "default" unless the user picked a colour.
pub color: String,
@@ -352,18 +348,8 @@ pub struct NoteDraft {
impl From<NoteDraft> for core_models::NoteCreateInput {
fn from(value: NoteDraft) -> Self {
let NoteDraft {
title,
body,
color,
items,
} = value;
core_models::NoteCreateInput {
title,
body,
color,
items,
}
let NoteDraft { body, color, items } = value;
core_models::NoteCreateInput { body, color, items }
}
}
@@ -371,14 +357,12 @@ impl From<NoteDraft> for core_models::NoteCreateInput {
///
/// A LIST of these rather than a struct of optional fields, because the core's patch
/// semantics distinguish three states — leave alone, set to a value, and clear to
/// null — and Kotlin has no way to express the third with a nullable field. `title:
/// null` in a data class is indistinguishable from `title` unset, so the editor
/// could never clear a title. Explicit `Clear*` variants say it out loud, and Kotlin
/// gets a sealed class it can `when` over exhaustively.
/// null — and Kotlin has no way to express the third with a nullable field.
/// `remindAt: null` in a data class is indistinguishable from `remindAt` unset, so
/// the editor could never clear a reminder. Explicit `Clear*` variants say it out
/// loud, and Kotlin gets a sealed class it can `when` over exhaustively.
#[derive(Debug, Clone, uniffi::Enum)]
pub enum NoteEdit {
Title { value: String },
ClearTitle,
Body { value: String },
Color { value: String },
Pinned { value: bool },
@@ -399,8 +383,6 @@ impl NoteEdit {
fn entry(self) -> (&'static str, serde_json::Value) {
use serde_json::Value;
match self {
NoteEdit::Title { value } => ("title", Value::String(value)),
NoteEdit::ClearTitle => ("title", Value::Null),
NoteEdit::Body { value } => ("body", Value::String(value)),
NoteEdit::Color { value } => ("color", Value::String(value)),
NoteEdit::Pinned { value } => ("pinned", Value::Bool(value)),
@@ -415,8 +397,8 @@ impl NoteEdit {
/// Fold a list of edits into the single patch object the store applies.
///
/// Later edits win on a repeated key, which is what a caller batching "set title,
/// then clear title" would expect.
/// Later edits win on a repeated key, which is what a caller batching "set a
/// reminder, then clear it" would expect.
pub fn patch_from(edits: Vec<NoteEdit>) -> serde_json::Value {
let mut map = serde_json::Map::new();
for edit in edits {
@@ -697,14 +679,14 @@ mod tests {
#[test]
fn a_set_and_a_clear_are_different_patch_entries() {
let set = patch_from(vec![NoteEdit::Title {
value: "x".to_string(),
let set = patch_from(vec![NoteEdit::RemindAt {
value: "2026-01-01T00:00:00Z".to_string(),
}]);
assert_eq!(set["title"], serde_json::json!("x"));
assert_eq!(set["remind_at"], serde_json::json!("2026-01-01T00:00:00Z"));
let cleared = patch_from(vec![NoteEdit::ClearTitle]);
let cleared = patch_from(vec![NoteEdit::ClearRemindAt]);
assert!(
cleared["title"].is_null(),
cleared["remind_at"].is_null(),
"a clear must reach the store as JSON null — an absent key means \
'leave alone', which is a different instruction"
);
@@ -720,11 +702,11 @@ mod tests {
#[test]
fn later_edits_win_on_a_repeated_field() {
let patch = patch_from(vec![
NoteEdit::Title {
value: "first".to_string(),
NoteEdit::RemindAt {
value: "2026-01-01T00:00:00Z".to_string(),
},
NoteEdit::ClearTitle,
NoteEdit::ClearRemindAt,
]);
assert!(patch["title"].is_null());
assert!(patch["remind_at"].is_null());
}
}
+63
View File
@@ -73,6 +73,43 @@ entirely on `ci-python:3.14`.
`…/actions/artifacts/{id}/zip`. Note the workstation has no `unzip` — use
`python3 -m zipfile -e`.
## The integration lane
Added 2026-08-23. Before it, `alembic upgrade head` ran for the first time when the
operator's container started — 26 revisions, none of them ever executed by CI — and
the schema the migrations build had never been checked against the models that read
it. M13 dropped three columns and rebuilt a STORED GENERATED column with nothing
watching.
Copied from FabledScribe's `integration` job, which had already solved the awkward
parts. Three of them are family rules for a reason:
- **Job key `integration`, no `name:`** (rule 80). act_runner derives the service
container's name from the truncated job DISPLAY name, and the discovery step filters
`docker ps` by it. A spaced or underscored name breaks the filter.
- **Service hostnames are not routable** on this runner (rule 79), so the step resolves
the Postgres container's bridge IP with `docker ps --filter` + `docker inspect` and
builds `THOUGHTSYNC_DATABASE_URL` from it. `postgres:5432` will not connect.
- **`run:` is busybox sh** (rule 81) — no `/dev/tcp` — so the readiness wait is a small
Python heredoc. Its terminator must dedent to column 0 after YAML strips the block
indent; check with `yaml.safe_load` and print the `run` string if you edit it.
`postgres:16-alpine`, matching the production compose, so the schema is proven against
the Postgres it will actually meet. The schema comes from **real migrations, never
`metadata.create_all`** (rule 82): testing a schema no deployment has ever seen proves
nothing, and that `alembic upgrade head` step IS the migration test — a broken revision
fails the job there, before it can fail a container start.
Tests are marked `integration` (registered in `pyproject.toml`); the unit lane runs
`-m "not integration"` and stays DB-free. Data resets with `TRUNCATE ... CASCADE`
BEFORE each test rather than after, so a failure leaves its rows behind to look at.
Like `test`, it runs for visibility and does **not** gate the build.
There is no local way to run it — that would mean standing up Postgres on the
workstation, which rule 12 reserves for an explicit request. This lane is verified in
CI.
## Desktop (Tauri) lane — separate workflow
The Tauri desktop client (`desktop/`) builds in its own workflow,
@@ -305,11 +342,37 @@ matched CI run 3931's byte for byte. Same image, same lockfile, same units.
take seconds (~30s for clippy). It is gitignored and reaches ~1.4 GB; delete it
whenever the space is wanted.
**Run these on every Rust-touching push, not just the ones that feel risky.** Four
consecutive failures across M13's removals — a private `fn` deleted along with the
`pub fn` above it, an orphaned `#[serde]` attribute left where a field was removed,
and a test pinning a protocol version literal — were all caught by these three
commands in under a minute each, after CI had already found them the slow way. A
removal is exactly the kind of change that looks safe and isn't: nothing in the
Python or TypeScript lanes compiles Rust, so a break can travel several commits
before the first lane that does gets to it.
**Don't infer formatting from existing code.** Several lines in `local/store.rs`
exceed 100 characters and survive only because rustfmt cannot break a string
literal — copying that shape caused one of four consecutive fmt-only CI failures,
which is what this whole section exists to prevent.
## Checking the frontend lane before pushing
Same technique, same authorisation, same reason — and it covers a gap the Rust gate
cannot: `vue-tsc --noEmit` type-checks only the SCRIPT block, so a malformed TEMPLATE
passes the typecheck lane and fails `vite build` in a different workflow. `npm run
build` runs both, which is exactly what the desktop lanes run.
```
docker run --rm --user "$(id -u):$(id -g)" -e HOME=/tmp -v "$PWD:/w" -w /w/frontend \
git.fabledsword.com/bvandeusen/ci-python:3.14 sh -c "npm ci --silent && npm run build"
```
The typecheck lane uses the `ci-python` image too — it is the node the frontend jobs
already run on, not a separate one. Delete `frontend/node_modules` and `frontend/dist`
afterwards; both are gitignored, but neither belongs in a working tree that never
builds locally otherwise.
## The desktop lockfile
`Cargo.lock` is **committed** at the workspace root, per Cargo's own guidance for
+3 -6
View File
@@ -8,9 +8,9 @@ use serde::{Deserialize, Serialize};
#[derive(Serialize)]
pub struct Note {
pub id: String,
pub title: Option<String>,
/// title if set, else the note's first body line — always present, so body-only
/// notes still have something to be called. Derived, never stored.
/// The note's NAME: its first non-blank body line, else its first checklist item.
/// Always present, so every note has something to be called. Derived at read time,
/// never stored.
pub display_title: String,
pub body: String,
pub color: String,
@@ -71,7 +71,6 @@ pub struct LinkPreview {
#[derive(Serialize)]
pub struct NoteRevision {
pub id: String,
pub title: Option<String>,
pub body: String,
pub created_at: Option<String>,
}
@@ -128,8 +127,6 @@ fn default_color() -> String {
#[derive(Deserialize)]
pub struct NoteCreateInput {
#[serde(default)]
pub title: String,
#[serde(default)]
pub body: String,
#[serde(default = "default_color")]
+8 -8
View File
@@ -93,8 +93,8 @@ mod tests {
let when = Utc::now() - age;
let stamped = when.to_rfc3339_opts(chrono::SecondsFormat::Millis, true);
conn.execute(
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed, trashed_at)
VALUES (?1, 'T', 'B', ?2, ?2, 1, ?2)",
"INSERT INTO notes (id, body, created_at, updated_at, trashed, trashed_at)
VALUES (?1, 'B', ?2, ?2, 1, ?2)",
rusqlite::params![id, stamped],
)
.expect("insert");
@@ -149,8 +149,8 @@ mod tests {
fn an_untrashed_note_is_never_swept() {
let conn = db();
conn.execute(
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed)
VALUES ('live', 'T', 'B', '2020-01-01T00:00:00.000Z', '2020-01-01T00:00:00.000Z', 0)",
"INSERT INTO notes (id, body, created_at, updated_at, trashed)
VALUES ('live', 'B', '2020-01-01T00:00:00.000Z', '2020-01-01T00:00:00.000Z', 0)",
[],
)
.expect("insert");
@@ -163,8 +163,8 @@ mod tests {
// "Age unknown" must never resolve to "delete it".
let conn = db();
conn.execute(
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed, trashed_at)
VALUES ('weird', 'T', 'B', '2020-01-01T00:00:00.000Z', '2020-01-01T00:00:00.000Z', 1, 'not a date')",
"INSERT INTO notes (id, body, created_at, updated_at, trashed, trashed_at)
VALUES ('weird', 'B', '2020-01-01T00:00:00.000Z', '2020-01-01T00:00:00.000Z', 1, 'not a date')",
[],
)
.expect("insert");
@@ -179,8 +179,8 @@ mod tests {
let conn = db();
let stamped = (Utc::now() - Duration::days(40)).to_rfc3339();
conn.execute(
"INSERT INTO notes (id, title, body, created_at, updated_at, trashed, trashed_at)
VALUES ('server', 'T', 'B', ?1, ?1, 1, ?1)",
"INSERT INTO notes (id, body, created_at, updated_at, trashed, trashed_at)
VALUES ('server', 'B', ?1, ?1, 1, ?1)",
rusqlite::params![stamped],
)
.expect("insert");
+14
View File
@@ -170,6 +170,16 @@ const SCHEMA_V6: &str = r#"
ALTER TABLE notes DROP COLUMN kind;
"#;
// v7 (M13 step 3): the title field is gone. A note is a body plus optional items, and
// its NAME is the first non-empty line of that body, falling back to its first item —
// derived at read time, never stored (see store::display_title).
//
// note_revisions loses its copy for the same reason: a revision snapshots a body.
const SCHEMA_V7: &str = r#"
ALTER TABLE notes DROP COLUMN title;
ALTER TABLE note_revisions DROP COLUMN title;
"#;
/// Bring the database up to the latest schema. Idempotent.
pub fn migrate(conn: &Connection) -> rusqlite::Result<()> {
conn.execute_batch("PRAGMA foreign_keys = ON;")?;
@@ -198,5 +208,9 @@ pub fn migrate(conn: &Connection) -> rusqlite::Result<()> {
conn.execute_batch(SCHEMA_V6)?;
conn.execute_batch("PRAGMA user_version = 6;")?;
}
if version < 7 {
conn.execute_batch(SCHEMA_V7)?;
conn.execute_batch("PRAGMA user_version = 7;")?;
}
Ok(())
}
+72 -67
View File
@@ -24,30 +24,26 @@ fn new_id() -> String {
Uuid::new_v4().to_string()
}
/// title if non-empty, else the first non-blank body line — always a string.
fn display_title(title: Option<&str>, body: &str) -> String {
if let Some(t) = title {
let t = t.trim();
if !t.is_empty() {
return t.to_string();
/// The note's NAME: its first non-blank body line, else its first checklist item.
///
/// Mirrors `derive_display_title` in the server's notes/helpers.py — one rule written
/// twice, and they have to agree or a synced note is called different things on either
/// side of the wire.
///
/// Pure, and given the items rather than fetching them: every caller has already
/// loaded them, so a query here would be a second trip for something already in hand.
fn display_title(body: &str, items: &[ChecklistItem]) -> String {
if let Some(line) = body.lines().map(str::trim).find(|l| !l.is_empty()) {
return line.to_string();
}
}
body.lines()
.map(str::trim)
.find(|l| !l.is_empty())
items
.iter()
.map(|i| i.text.trim())
.find(|t| !t.is_empty())
.unwrap_or("")
.to_string()
}
fn normalize_title(raw: &str) -> Option<String> {
let t = raw.trim();
if t.is_empty() {
None
} else {
Some(t.to_string())
}
}
fn escape_like(s: &str) -> String {
s.replace('\\', "\\\\")
.replace('%', "\\%")
@@ -139,32 +135,29 @@ fn load_previews(conn: &Connection, note_id: &str) -> rusqlite::Result<Vec<LinkP
fn load_note(conn: &Connection, id: &str) -> rusqlite::Result<Note> {
let mut note = conn.query_row(
"SELECT id, title, body, color, position, pinned, archived, trashed, remind_at, recurrence, created_at, updated_at, trashed_at
"SELECT id, body, color, position, pinned, archived, trashed, remind_at, recurrence, created_at, updated_at, trashed_at
FROM notes WHERE id = ?1",
[id],
|r| {
let title: Option<String> = r.get(1)?;
let body: String = r.get(2)?;
let dt = display_title(title.as_deref(), &body);
let body: String = r.get(1)?;
Ok(Note {
id: r.get(0)?,
title,
display_title: dt,
display_title: String::new(), // filled below — it may need a query
body,
color: r.get(3)?,
position: r.get(4)?,
pinned: r.get(5)?,
archived: r.get(6)?,
trashed: r.get(7)?,
deleted_at: r.get(12)?,
remind_at: r.get(8)?,
recurrence: r.get(9)?,
color: r.get(2)?,
position: r.get(3)?,
pinned: r.get(4)?,
archived: r.get(5)?,
trashed: r.get(6)?,
deleted_at: r.get(11)?,
remind_at: r.get(7)?,
recurrence: r.get(8)?,
labels: Vec::new(),
items: Vec::new(),
attachments: Vec::new(),
previews: Vec::new(),
created_at: r.get(10)?,
updated_at: r.get(11)?,
created_at: r.get(9)?,
updated_at: r.get(10)?,
})
},
)?;
@@ -172,6 +165,8 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result<Note> {
note.items = load_items(conn, id)?;
note.attachments = load_attachments(conn, id)?;
note.previews = load_previews(conn, id)?;
// After the items, because a body-only-empty note is named by its first one.
note.display_title = display_title(&note.body, &note.items);
Ok(note)
}
@@ -267,7 +262,7 @@ pub fn list_notes(conn: &Connection, q: &ListQuery) -> rusqlite::Result<Vec<Note
if let Some(f) = &q.facets {
if let Some(text) = f.q.as_deref().filter(|s| !s.is_empty()) {
sql.push_str(" AND (title LIKE ? ESCAPE '\\' OR body LIKE ? ESCAPE '\\')");
sql.push_str(" AND body LIKE ? ESCAPE '\\'");
let pat = format!("%{}%", escape_like(text));
binds.push(pat.clone());
binds.push(pat);
@@ -321,23 +316,31 @@ pub fn reminders(conn: &Connection) -> rusqlite::Result<Vec<Note>> {
}
pub fn titles(conn: &Connection) -> rusqlite::Result<Vec<TitleEntry>> {
let mut stmt = conn.prepare("SELECT id, title, body FROM notes WHERE trashed = 0")?;
let rows = stmt.query_map([], |r| {
let title: Option<String> = r.get(1)?;
let body: String = r.get(2)?;
// Names come from `load_note` rather than from a bare row, because a note whose
// body is empty is named by its first checklist item — which a row here doesn't
// have. The command palette reads this; correctness beats one query per note at
// personal scale.
let ids: Vec<String> = {
let mut stmt = conn.prepare("SELECT id FROM notes WHERE trashed = 0")?;
let rows = stmt.query_map([], |r| r.get(0))?;
rows.collect::<rusqlite::Result<Vec<String>>>()?
};
ids.iter()
.map(|id| {
let note = load_note(conn, id)?;
Ok(TitleEntry {
id: r.get(0)?,
title: display_title(title.as_deref(), &body),
id: note.id,
title: note.display_title,
})
})?;
rows.collect()
})
.collect()
}
pub fn search(conn: &Connection, q: &str) -> rusqlite::Result<Vec<Note>> {
let pat = format!("%{}%", escape_like(q));
let ids: Vec<String> = {
let mut stmt = conn.prepare(
"SELECT id FROM notes WHERE trashed = 0 AND (title LIKE ?1 ESCAPE '\\' OR body LIKE ?1 ESCAPE '\\') ORDER BY updated_at DESC",
"SELECT id FROM notes WHERE trashed = 0 AND body LIKE ?1 ESCAPE '\\' ORDER BY updated_at DESC",
)?;
let rows = stmt.query_map([&pat], |r| r.get::<_, String>(0))?;
rows.collect::<rusqlite::Result<Vec<String>>>()?
@@ -350,16 +353,15 @@ pub fn search(conn: &Connection, q: &str) -> rusqlite::Result<Vec<Note>> {
pub fn create_note(conn: &Connection, input: &NoteCreateInput) -> rusqlite::Result<Note> {
let id = new_id();
let ts = now();
let title = normalize_title(&input.title);
let position: i64 = conn.query_row(
"SELECT COALESCE(MAX(position), 0) + 1 FROM notes",
[],
|r| r.get(0),
)?;
conn.execute(
"INSERT INTO notes (id, title, body, color, position, created_at, updated_at, dirty)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?6, 1)",
params![id, title, input.body, input.color, position, ts],
"INSERT INTO notes (id, body, color, position, created_at, updated_at, dirty)
VALUES (?1, ?2, ?3, ?4, ?5, ?5, 1)",
params![id, input.body, input.color, position, ts],
)?;
if let Some(items) = &input.items {
for (i, text) in items.iter().enumerate() {
@@ -373,25 +375,29 @@ pub fn create_note(conn: &Connection, input: &NoteCreateInput) -> rusqlite::Resu
load_note(conn, &id)
}
fn snapshot_revision(conn: &Connection, id: &str) -> rusqlite::Result<()> {
let body: String =
conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))?;
conn.execute(
"INSERT INTO note_revisions (id, note_id, body, created_at) VALUES (?1, ?2, ?3, ?4)",
params![new_id(), id, body, now()],
)?;
Ok(())
}
/// PATCH semantics: apply exactly the fields present in `changes`.
pub fn update_note(conn: &Connection, id: &str, changes: &Value) -> rusqlite::Result<Note> {
let obj = changes
.as_object()
.ok_or_else(|| rusqlite::Error::InvalidParameterName("changes must be an object".into()))?;
// Snapshot the pre-edit title/body once if either is being changed (version history).
if obj.contains_key("title") || obj.contains_key("body") {
// Snapshot the pre-edit body before changing it (version history).
if obj.contains_key("body") {
snapshot_revision(conn, id)?;
}
for (k, v) in obj {
match k.as_str() {
"title" => {
let norm = v.as_str().and_then(normalize_title);
conn.execute(
"UPDATE notes SET title = ?1 WHERE id = ?2",
params![norm, id],
)?;
}
"body" => {
let body = v.as_str().unwrap_or("");
conn.execute(
@@ -640,28 +646,27 @@ pub fn set_pref(conn: &Connection, key: &str, value: &str) -> rusqlite::Result<(
pub fn revisions(conn: &Connection, id: &str) -> rusqlite::Result<Vec<NoteRevision>> {
let mut stmt = conn
.prepare("SELECT id, title, body, created_at FROM note_revisions WHERE note_id = ?1 ORDER BY created_at DESC")?;
.prepare("SELECT id, body, created_at FROM note_revisions WHERE note_id = ?1 ORDER BY created_at DESC")?;
let rows = stmt.query_map([id], |r| {
Ok(NoteRevision {
id: r.get(0)?,
title: r.get(1)?,
body: r.get(2)?,
created_at: r.get(3)?,
body: r.get(1)?,
created_at: r.get(2)?,
})
})?;
rows.collect()
}
pub fn restore_revision(conn: &Connection, id: &str, rev_id: &str) -> rusqlite::Result<Note> {
let (title, body): (Option<String>, String) = conn.query_row(
"SELECT title, body FROM note_revisions WHERE id = ?1 AND note_id = ?2",
let body: String = conn.query_row(
"SELECT body FROM note_revisions WHERE id = ?1 AND note_id = ?2",
params![rev_id, id],
|r| Ok((r.get(0)?, r.get(1)?)),
|r| r.get(0),
)?;
snapshot_revision(conn, id)?;
conn.execute(
"UPDATE notes SET title = ?1, body = ?2 WHERE id = ?3",
params![title, body, id],
"UPDATE notes SET body = ?1 WHERE id = ?2",
params![body, id],
)?;
sync_tags(conn, id, &body)?;
touch(conn, id)?;
+10 -6
View File
@@ -344,13 +344,17 @@ mod tests {
fn server_info_tolerates_unknown_and_absent_fields() {
// Forward compatibility: a NEWER server sending fields we've never heard of
// must not break the handshake.
let info: ServerInfo = serde_json::from_str(
r#"{"site_name":"S","sync_protocol_version":1,
"min_client_protocol_version":1,
// Versions come from the constants, not literals: this test is about unknown
// FIELDS, and pinning the numbers made it fail the moment the protocol moved
// to v2 — for a reason that has nothing to do with what it checks.
let body = format!(
r#"{{"site_name":"S","sync_protocol_version":{v},
"min_client_protocol_version":{v},
"sync_features":["notes","labels","attachments","tombstones","revisions"],
"some_future_field":{"nested":true}}"#,
)
.expect("unknown fields are ignored");
"some_future_field":{{"nested":true}}}}"#,
v = CLIENT_PROTOCOL_VERSION,
);
let info: ServerInfo = serde_json::from_str(&body).expect("unknown fields are ignored");
assert_eq!(evaluate(&info), Compatibility::Ok);
}
+2 -5
View File
@@ -240,12 +240,11 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> {
// `created_at` is deliberately absent from the UPDATE clause: a note's birth time
// never changes, and the server's copy is the same value anyway.
conn.execute(
"INSERT INTO notes (id, title, body, color, position, pinned, archived,
"INSERT INTO notes (id, body, color, position, pinned, archived,
trashed, remind_at, recurrence, created_at, updated_at,
sync_revision, trashed_at, dirty)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, 0)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, 0)
ON CONFLICT(id) DO UPDATE SET
title = excluded.title,
body = excluded.body,
color = excluded.color,
position = excluded.position,
@@ -260,7 +259,6 @@ fn upsert_note(conn: &Connection, note: &wire::Note) -> rusqlite::Result<()> {
dirty = 0",
params![
note.id,
note.title,
note.body,
note.color,
note.position,
@@ -496,7 +494,6 @@ mod tests {
fn note(id: &str, revision: i64) -> wire::Note {
wire::Note {
id: id.to_string(),
title: Some("Title".into()),
body: "Body".into(),
color: "default".into(),
position: 0,
+13 -21
View File
@@ -62,14 +62,11 @@ pub struct Change {
pub op: &'static str,
pub edited_at: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub title: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub body: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub color: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(skip_serializing_if = "Option::is_none")]
pub pinned: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub archived: Option<bool>,
@@ -98,7 +95,6 @@ impl Change {
id,
op: "delete",
edited_at,
title: None,
body: None,
color: None,
pinned: None,
@@ -198,7 +194,6 @@ fn collect_labels(conn: &Connection, out: &mut Vec<Change>, limit: usize) -> rus
name: Some(r.get(1)?),
color: Some(r.get(2)?),
edited_at: r.get(3)?,
title: None,
body: None,
pinned: None,
archived: None,
@@ -234,7 +229,6 @@ fn collect_notes(conn: &Connection, out: &mut Vec<Change>, limit: usize) -> rusq
/// The note's own columns. A named struct rather than a twelve-wide tuple so the
/// field-to-column mapping stays readable at the call site.
struct NoteRow {
title: Option<String>,
body: String,
color: String,
position: i64,
@@ -249,23 +243,22 @@ struct NoteRow {
fn note_row(conn: &Connection, id: &str) -> rusqlite::Result<NoteRow> {
conn.query_row(
"SELECT title, body, color, position, pinned, archived, trashed,
"SELECT body, color, position, pinned, archived, trashed,
remind_at, recurrence, created_at, updated_at
FROM notes WHERE id = ?1",
params![id],
|r| {
Ok(NoteRow {
title: r.get(0)?,
body: r.get(1)?,
color: r.get(2)?,
position: r.get(3)?,
pinned: r.get::<_, i64>(4)? != 0,
archived: r.get::<_, i64>(5)? != 0,
trashed: r.get::<_, i64>(6)? != 0,
remind_at: r.get(7)?,
recurrence: r.get(8)?,
created_at: r.get(9)?,
updated_at: r.get(10)?,
body: r.get(0)?,
color: r.get(1)?,
position: r.get(2)?,
pinned: r.get::<_, i64>(3)? != 0,
archived: r.get::<_, i64>(4)? != 0,
trashed: r.get::<_, i64>(5)? != 0,
remind_at: r.get(6)?,
recurrence: r.get(7)?,
created_at: r.get(8)?,
updated_at: r.get(9)?,
})
},
)
@@ -304,7 +297,6 @@ fn note_change(conn: &Connection, id: &str) -> rusqlite::Result<Change> {
// The local `updated_at` IS the client's edit time, which is what the
// server's last-write-wins comparison runs against.
edited_at: row.updated_at,
title: row.title,
body: Some(row.body),
color: Some(row.color),
pinned: Some(row.pinned),
@@ -529,9 +521,9 @@ mod tests {
fn seed_note(conn: &Connection, id: &str, dirty: i64) {
conn.execute(
"INSERT INTO notes (id, title, body, color, position, pinned, archived,
"INSERT INTO notes (id, body, color, position, pinned, archived,
trashed, created_at, updated_at, sync_revision, dirty)
VALUES (?1, 'T', 'B', 'default', 0, 0, 0, 0,
VALUES (?1, 'B', 'default', 0, 0, 0, 0,
'2026-07-26T00:00:00.000Z', '2026-07-26T00:00:00.000Z', 3, ?2)",
params![id, dirty],
)
-2
View File
@@ -24,8 +24,6 @@ pub struct ChangesPage {
pub struct Note {
pub id: String,
#[serde(default)]
pub title: Option<String>,
#[serde(default)]
pub body: String,
#[serde(default = "default_color")]
pub color: String,
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "thoughtsync-desktop"
version = "0.1.0"
version = "0.2.0"
description = "ThoughtSync desktop — local-first Keep-style thought capture"
authors = ["bvandeusen"]
edition = "2021"
-6
View File
@@ -190,12 +190,6 @@ pub fn notes_titles(db: State<'_, Db>) -> Result<Vec<TitleEntry>, String> {
store::titles(&conn).map_err(|e| e.to_string())
}
#[tauri::command]
pub fn notes_search(q: String, db: State<'_, Db>) -> Result<Vec<Note>, String> {
let conn = db.0.lock().map_err(|e| e.to_string())?;
store::search(&conn, &q).map_err(|e| e.to_string())
}
#[tauri::command]
pub fn labels_list(db: State<'_, Db>) -> Result<Vec<Label>, String> {
let conn = db.0.lock().map_err(|e| e.to_string())?;
-1
View File
@@ -119,7 +119,6 @@ pub fn run() {
commands::local::notes_restore_revision,
commands::local::notes_reminders,
commands::local::notes_titles,
commands::local::notes_search,
commands::local::labels_list,
commands::local::labels_create,
commands::local::labels_rename,
+1 -1
View File
@@ -2,7 +2,7 @@
"$schema": "https://schema.tauri.app/config/2",
"productName": "ThoughtSync",
"mainBinaryName": "thoughtsync",
"version": "0.1.0",
"version": "0.2.0",
"identifier": "com.fabledsword.thoughtsync",
"build": {
"frontendDist": "../../frontend/dist",
+21 -3
View File
@@ -71,10 +71,28 @@ services:
# Uploaded attachments. /var/thoughtsync is fixed in the app (Config.DATA_DIR),
# not configurable — mount it or lose every image on container recreation.
- thoughtsync-data:/var/thoughtsync
# WHERE THE APP IS REACHABLE FROM. Three shapes, and the right answer is
# different for each — the default serves the first.
#
# 1. LAN, no proxy (the default). Binds every interface so your phone and your
# desktop can reach the server. This is what makes a self-hosted install work
# out of the box, and it is why the default is NOT the locked-down value: a
# server only reachable from the machine it runs on is not hardened, it is
# broken.
#
# 2. Reverse proxy in Docker, on this network (Traefik discovering the container,
# an nginx container, etc). DELETE the `ports:` block below entirely. The proxy
# reaches the app over the compose network without any port being published,
# and publishing one is a second, unauthenticated way in that bypasses the
# proxy — including whatever the proxy is doing about TLS and auth.
#
# 3. Reverse proxy on the HOST (not in Docker). Set THOUGHTSYNC_BIND=127.0.0.1 in
# .env, so the port exists but only the host itself can reach it.
#
# If you are exposing this to the internet, you want 2 or 3. Leaving it at 1
# means the app is reachable directly on port 5000, past everything your proxy
# does.
ports:
# Default binds every interface, which is what lets desktop clients on the LAN
# reach it. Behind a reverse proxy, set THOUGHTSYNC_BIND=127.0.0.1 so only the
# proxy can talk to it.
- "${THOUGHTSYNC_BIND:-0.0.0.0}:${THOUGHTSYNC_PORT:-5000}:5000"
healthcheck:
# python rather than curl: the runtime image is python:3.12-slim and carries no
+55 -19
View File
@@ -7,16 +7,21 @@ one guessed password away from someone's whole note history.
This is what the app does about that on its own, and the four things it cannot do for
you.
## Do these four things first
## Do these five things first
**1. Close registration.** `allow_registration` defaults to **on**, because the first
run of a fresh instance has to be able to create the admin account. It stays on
afterwards. Once your own account exists, turn it off in **Settings → Access → Allow
new registrations**, or the first stranger to find the hostname can open an account on
your server.
**1. Check registration is closed.** On a fresh instance this now takes care of
itself: the first account created becomes the admin *and* closes registration behind
it, so there is no window between "my account exists" and "I remembered to turn it
off". A brand-new instance is never locked out of itself, and never left open either.
The first account created is always the admin, regardless of this setting — so a
brand-new instance is never locked out of itself.
**Instances that predate this still need one manual flip.** The close fires when the
first account is created, so a server whose admin already existed keeps whatever
`allow_registration` was set to — which was **on** by default. Check **Settings →
Access → Allow new registrations** before exposing an instance you have been running
on a LAN.
To let someone else in, turn it back on, have them register, turn it off. There is no
invite system yet, so that is the mechanism.
**2. Terminate TLS in front of it, and forward the scheme.** The app marks the
session cookie `Secure` and sends HSTS only when it can tell the request arrived over
@@ -36,15 +41,38 @@ Once a browser has seen HSTS from your hostname it will refuse plain HTTP there
year, even if the header stops. That is the point of it, but it is worth knowing
before you put a hostname behind TLS temporarily.
**3. Stop publishing the app port.** The default compose binds `0.0.0.0:5000` so LAN
clients can reach it directly. Behind a proxy that is a second, unprotected front
door. In `.env`:
**3. Tell it how many proxies are in front of it.** **Settings → Security → Trusted
proxy hops**, which defaults to `1` — one reverse proxy terminating TLS. Behind a CDN
as well (Cloudflare in front of your proxy) set it to `2`. It applies immediately; no
restart.
```
THOUGHTSYNC_BIND=127.0.0.1
```
This decides which entry of `X-Forwarded-For` is believed, and it is a security
setting rather than a preference. The header grows left to right as a request
traverses, so the rightmost entries are the ones your own infrastructure wrote and
anything a caller forged sits to the left of them. Counting in from the right by the
number of proxies you actually run means a forged prefix can never be selected. Set it
too HIGH and it starts trusting entries no proxy of yours wrote; too low and several
callers share one rate-limit bucket, which is merely inconvenient.
**4. Have a backup that includes the files.** Attachments are files on the
**4. Stop reaching the app except through the proxy.** The default compose binds
`0.0.0.0:5000` so LAN clients can reach it directly — which is right for a LAN install
and wrong the moment there is a proxy in front, because it leaves a second way in that
bypasses everything the proxy does.
Which fix depends on where your proxy runs:
- **Proxy in Docker** (Traefik discovering the container, an nginx container): delete
the `ports:` block from `docker-compose.yml`. The proxy reaches the app over the
compose network; no published port is needed at all, and this is the safest of the
two because there is no host port to reach even from the host.
- **Proxy on the host**: set `THOUGHTSYNC_BIND=127.0.0.1` in `.env`, so the port
exists but only the host itself can use it.
To check which you have: `docker compose ps` shows the published ports, and
`curl http://<your-lan-ip>:5000/api/health` from another machine tells you whether the
app is still answering around the proxy. It should not be.
**5. Have a backup that includes the files.** Attachments are files on the
`thoughtsync-data` volume, not rows — a `pg_dump` restores notes whose images are all
gone. Back up both:
@@ -57,9 +85,10 @@ docker run --rm -v thoughtsync-data:/d -v "$PWD":/out alpine tar czf /out/media.
- **The credential endpoints are throttled.** `/api/auth/login`, `/api/auth/register`
and `/api/auth/device-login` count attempts against both the account and the calling
address, and answer `429` with a `Retry-After` once either is over budget — ten
failed sign-ins per account per fifteen minutes, five registrations per address per
hour. The account-keyed limit is the one that holds when the address is forged.
address, and answer `429` with a `Retry-After` once either is over budget. The
numbers live in **Settings → Security** — ten failed sign-ins per account per
fifteen minutes and five sign-ups per address per hour by default — and a change
applies to the next attempt rather than the next deploy. The account-keyed limit is the one that holds when the address is forged.
Checked *before* the password is verified, so a throttled attempt costs no bcrypt:
hashing is deliberately slow, and an unauthenticated caller who can trigger it
without limit has a CPU-exhaustion primitive as well as a guessing one.
@@ -87,7 +116,14 @@ Know these before you decide who gets an account.
- **No second factor.** A password is the whole of it.
- **No per-user storage quota.** Any account can upload attachments until the volume
is full. `max_attachment_mb` caps a single file, not a total.
- **No audit log.** Device tokens record `last_used_at`; sign-ins are not recorded.
- **No audit TABLE.** Credential events — sign-ins, failures, throttle trips, new
accounts, device tokens issued — are written to the application log and readable
with `docker compose logs app`, which is enough to see whether anyone is knocking.
They are not queryable, not retained beyond the container's log rotation, and not
attributable after the fact.
- **No invites.** Adding a second person means re-opening registration while they
sign up, then closing it again. There is no per-person token, no expiry, and no
record of who invited whom.
None of these are hard blockers for an instance whose accounts are you and people you
know. They are the reason not to hand out open registration to strangers.
+1 -2
View File
@@ -6,7 +6,7 @@
// parameters (e.g. labelIds -> label_ids). A few operations have no offline meaning
// yet (account auth, device linking, attachment upload, URL unfurl, file import) —
// those reject with a clear message rather than silently failing; the board, editor,
// capture, search, filters, labels, checklists and reminders all work fully offline.
// capture, filters, labels, checklists and reminders all work fully offline.
import { invoke } from "../desktop/bridge";
import type { Note, NoteRevision } from "../stores/notes";
@@ -71,7 +71,6 @@ export const local: Repo = {
restoreRevision: (id, revId) => invoke<Note>("notes_restore_revision", { id, revId }),
reminders: () => invoke<Note[]>("notes_reminders"),
titles: () => invoke<TitleEntry[]>("notes_titles"),
search: (q) => invoke<Note[]>("notes_search", { q }),
},
savedFilters: {
+1 -3
View File
@@ -32,7 +32,6 @@ export interface NoteListQuery {
}
export interface NoteCreateInput {
title: string;
body: string;
color: NoteColor;
items?: string[];
@@ -40,7 +39,7 @@ export interface NoteCreateInput {
// The mutable subset of a note (PATCH /api/notes/:id).
export type NoteChanges = Partial<
Pick<Note, "title" | "body" | "color" | "pinned" | "archived" | "remind_at" | "recurrence">
Pick<Note, "body" | "color" | "pinned" | "archived" | "remind_at" | "recurrence">
>;
export interface ChecklistItemChanges {
@@ -112,7 +111,6 @@ export interface NotesRepo {
restoreRevision(id: string, revId: string): Promise<Note>;
reminders(): Promise<Note[]>;
titles(): Promise<TitleEntry[]>;
search(q: string): Promise<Note[]>;
}
export interface SavedFiltersRepo {
-1
View File
@@ -99,7 +99,6 @@ export const rest: Repo = {
restoreRevision: (id, revId) => api.post<Note>(`/api/notes/${id}/revisions/${revId}/restore`),
reminders: async () => (await api.get<{ notes: Note[] }>("/api/notes/reminders")).notes,
titles: async () => (await api.get<{ titles: TitleEntry[] }>("/api/notes/titles")).titles,
search: async (q) => (await api.get<{ notes: Note[] }>(`/api/notes/search?q=${encodeURIComponent(q)}`)).notes,
},
savedFilters: {
+36 -11
View File
@@ -1,6 +1,6 @@
<script setup lang="ts">
import { computed, nextTick, onBeforeUnmount, onMounted, ref, watch } from "vue";
import { useRoute, useRouter } from "vue-router";
import { useRoute, useRouter, type LocationQueryRaw } from "vue-router";
import { useSessionStore } from "../stores/session";
import { useConfigStore } from "../stores/config";
import { useLabelsStore } from "../stores/labels";
@@ -177,21 +177,48 @@ function labelDot(color: string): string {
return NOTE_SWATCH_CLASSES[color as NoteColor] ?? NOTE_SWATCH_CLASSES.default;
}
// The board lenses — the routes a search can happen *within*. Searching while looking
// at Trash should search Trash, not silently move you.
const BOARD_ROUTES = new Set(["board", "archive", "trash", "label"]);
/**
* Search is a FACET, not a destination.
*
* It used to navigate to a `/search` view backed by a different endpoint with no
* facets at all — so the one screen you landed on when you searched was the one
* screen where you could not also narrow by tag, which is precisely what tags are
* for (note 2930). Now it writes `?q=` into the board's URL, beside any labels
* already there, and the same AND-ed query serves both.
*
* Existing facets are preserved, so "filter by #grocery, then search" and the reverse
* both work.
*/
function onSearch(value: string) {
searchText.value = value;
clearTimeout(searchTimer);
searchTimer = setTimeout(() => {
const q = searchText.value.trim();
if (q) router.push({ name: "search", query: { q } });
else if (route.name === "search") router.push("/");
const onBoard = BOARD_ROUTES.has(String(route.name));
const query: LocationQueryRaw = onBoard ? { ...route.query } : {};
if (q) query.q = q;
else delete query.q;
void router.push({ path: onBoard ? route.path : "/", query });
}, 250);
}
// Clear the search box when navigating to a non-search view.
// The URL is the filter state (see notes/facets.ts), so the box READS from it rather
// than holding its own copy — which is also what keeps it in step with the Filters
// panel's Clear button and with a saved view opened from the sidebar.
watch(
() => route.query.q,
(q) => {
searchText.value = typeof q === "string" ? q : "";
},
{ immediate: true },
);
watch(
() => route.name,
(name) => {
if (name !== "search") searchText.value = "";
() => {
drawer.value = false;
},
);
@@ -200,7 +227,7 @@ watch(
* What to call the lens currently in view.
*
* Keyed off the route name rather than each view declaring its own title, so the
* label sits in one place and can't go missing (the board and search never had one)
* label sits in one place and can't go missing (the board never had one)
* or drift in styling (timeline and reminders each had their own h1).
*
* A label lens is named by the label itself — "Groceries" is what the user came
@@ -212,8 +239,6 @@ const lensName = computed<string>(() => {
return "Archive";
case "trash":
return "Trash";
case "search":
return "Search";
case "timeline":
return "Timeline";
case "reminders":
@@ -286,7 +311,7 @@ async function signOut() {
page you navigated to — so it sits in the bar that never moves, beside
the app name, and stays in one place while everything beneath it
re-filters. Replaces the per-view <h1>s, which sat in a different spot
in each view and were absent entirely on the board and in search. -->
in each view and were absent entirely on the board. -->
<span aria-live="polite" class="flex min-w-0 shrink items-center gap-2 text-sm text-neutral-400">
<!-- The separator only makes sense next to the app name, which is itself
hidden on narrow screens. There, the lens name simply takes the space
@@ -511,7 +536,7 @@ async function signOut() {
BoardView, so keying on the route would remount it blanking the board
and refetching, which is precisely the page-change feeling this is meant
to remove. Unkeyed, Vue only transitions when the component TYPE changes
(board search timeline), and moving between the board's own
(board timeline reminders), and moving between the board's own
lenses stays an in-place reflow that NoteGrid animates. -->
<main id="main" tabindex="-1" class="min-w-0 flex-1 focus:outline-none">
<RouterView v-slot="{ Component }">
+1 -16
View File
@@ -10,7 +10,7 @@ import { addLocalDays, formatLocalDay, parseLocalDate } from "../notes/datetime"
import { NOTE_COLOR_KEYS, NOTE_COLOR_LABELS, NOTE_SWATCH_CLASSES, type NoteColor } from "../notes/colors";
import Icon from "./Icon.vue";
// A dead-simple facet bar over the board: text search + color + labels + has-reminder
// A dead-simple facet bar over the board: color + labels + has-reminder
// + has-attachment + created-date range. The URL query IS the state, so a
// filtered board is a shareable lens and a saved view is just a link.
const route = useRoute();
@@ -47,13 +47,6 @@ function toggleAttachment() {
patch({ has_attachment: facets.value.has_attachment ? undefined : true });
}
let qTimer: ReturnType<typeof setTimeout> | undefined;
function onQ(e: Event) {
const v = (e.target as HTMLInputElement).value;
clearTimeout(qTimer);
qTimer = setTimeout(() => patch({ q: v.trim() || undefined }), 300);
}
function onFrom(e: Event) {
const v = (e.target as HTMLInputElement).value;
patch({ created_after: v ? `${v}T00:00:00` : undefined });
@@ -118,14 +111,6 @@ const chipOff = "border-neutral-300 text-neutral-600 hover:bg-neutral-100 dark:b
v-if="open"
class="mt-2 flex flex-col gap-3 rounded-xl border border-neutral-200 p-3 dark:border-neutral-800"
>
<input
type="search"
:value="facets.q ?? ''"
placeholder="Search text…"
class="w-full rounded-lg border border-neutral-300 bg-white px-3 py-1.5 text-sm outline-none focus-visible:ring-2 focus-visible:ring-brand dark:border-neutral-700 dark:bg-neutral-900"
@input="onQ"
/>
<div class="flex flex-wrap items-center gap-1.5">
<span class="w-16 shrink-0 text-xs text-neutral-400">Color</span>
<button
+31 -7
View File
@@ -1,12 +1,24 @@
<script setup lang="ts">
import type { LinkPreview } from "../stores/notes";
defineProps<{ preview: LinkPreview; removable?: boolean }>();
/**
* A fetched link preview, in one of two sizes.
*
* `compact` is a single row — favicon-less, one line of title, the site name — for a
* URL mentioned *inside* a note that has its own text. The note is the thing; the
* link is a footnote to it.
*
* Full size is for a note that is NOTHING but a URL. There the link IS the note, and
* a compact strip would be a card with nothing on it.
*/
defineProps<{ preview: LinkPreview; removable?: boolean; compact?: boolean }>();
defineEmits<{ (e: "remove"): void }>();
</script>
<template>
<div class="group/lp relative overflow-hidden rounded-lg border border-neutral-200 dark:border-neutral-700">
<div
class="group/lp relative overflow-hidden rounded-lg border border-neutral-200 dark:border-neutral-700"
>
<a
:href="preview.url"
target="_blank"
@@ -19,16 +31,28 @@ defineEmits<{ (e: "remove"): void }>();
alt=""
loading="lazy"
decoding="async"
class="h-auto w-24 shrink-0 self-stretch object-cover"
class="h-auto shrink-0 self-stretch object-cover"
:class="compact ? 'w-12' : 'w-24'"
/>
<div class="min-w-0 flex-1 px-3 py-2">
<p v-if="preview.site_name" class="truncate text-[11px] uppercase tracking-wide text-neutral-400">
<div class="min-w-0 flex-1" :class="compact ? 'px-2 py-1.5' : 'px-3 py-2'">
<p
v-if="preview.site_name"
class="truncate uppercase tracking-wide text-neutral-400"
:class="compact ? 'text-[10px]' : 'text-[11px]'"
>
{{ preview.site_name }}
</p>
<p class="truncate text-sm font-medium text-neutral-800 dark:text-neutral-100">
<p
class="truncate font-medium text-neutral-800 dark:text-neutral-100"
:class="compact ? 'text-xs' : 'text-sm'"
>
{{ preview.title || preview.url }}
</p>
<p v-if="preview.description" class="mt-0.5 line-clamp-2 text-xs text-neutral-500 dark:text-neutral-400">
<!-- The description is the first thing to go when there is no room for it. -->
<p
v-if="preview.description && !compact"
class="mt-0.5 line-clamp-2 text-xs text-neutral-500 dark:text-neutral-400"
>
{{ preview.description }}
</p>
</div>
+55 -10
View File
@@ -54,6 +54,46 @@ const trashUrgent = computed(() => trashDays.value !== null && trashDays.value <
const firstImage = computed(() => props.note.attachments.find((a) => a.mime.startsWith("image/")));
const otherAttachments = computed(() => props.note.attachments.filter((a) => !a.mime.startsWith("image/")));
// How much of a note the CARD shows. Android has always clamped to 8
// (`MAX_PREVIEW_LINES`); the web rendered the whole body, so one long note could
// produce a card taller than the screen and push everything else off the board.
//
// It matters more now that the title is gone (M13 step 4). The first line used to be
// the thing your eye caught; with one weight throughout, an unbounded card is just a
// wall, and the note next to it is the one you were looking for.
//
// Clamped in the STRING rather than with CSS `line-clamp`, which needs a
// `-webkit-box` and behaves unreliably around the block elements MarkdownText emits
// (lists, quotes, fenced code). This is deterministic, matches Android's semantics
// exactly, and skips parsing a body the card was never going to show.
const PREVIEW_LINES = 8;
// --- Links ------------------------------------------------------------------
//
// A note that is NOTHING but a URL is a link, and its preview is the whole card —
// showing the raw URL underneath a card that already says where it goes is saying the
// same thing twice, badly. A URL mentioned *inside* a note is a footnote to it, and
// gets a compact strip at the bottom instead.
//
// Whitespace either side still counts as lone: someone pasting a link rarely trims it.
const LONE_URL_RE = /^\s*(https?:\/\/[^\s<>"'\])]+)\s*$/;
const isLoneUrl = computed(() => LONE_URL_RE.test(props.note.body) && !props.note.items.length);
/** The preview for a lone-URL note — null while it is still being fetched, or if it
* could never be fetched at all. */
const loneUrlPreview = computed(() => {
if (!isLoneUrl.value) return null;
const url = props.note.body.trim();
return props.note.previews.find((p) => p.url === url) ?? null;
});
const bodyPreview = computed(() => {
const lines = props.note.body.split("\n");
if (lines.length <= PREVIEW_LINES) return props.note.body;
return lines.slice(0, PREVIEW_LINES).join("\n") + "\n…";
});
const root = ref<HTMLElement | null>(null);
// --- Drag-to-reorder. Pointer Events, gated behind an explicit grip handle so a
@@ -206,9 +246,6 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown))
</span>
</div>
<div v-if="note.previews.length" class="mb-2 flex flex-col gap-2">
<LinkPreview v-for="p in note.previews" :key="p.id" :preview="p" />
</div>
<!-- One render path: every note is a body plus, maybe, checkable items.
A focusable div rather than a <button>, because a checklist nests interactive
@@ -221,22 +258,30 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown))
@click="emit('open', note)"
@keydown.enter="emit('open', note)"
>
<h3 v-if="note.title" class="mb-1 break-words text-sm font-semibold text-neutral-900 dark:text-neutral-100">
{{ note.title }}
</h3>
<div v-if="note.body" class="text-sm text-neutral-700 dark:text-neutral-300">
<MarkdownText :text="note.body" />
<!-- A lone URL renders as its preview and nothing else. Until the fetch lands
or if it never does the URL itself stands in, so the card is never
blank and the link is never unreachable. -->
<LinkPreview v-if="loneUrlPreview" :preview="loneUrlPreview" />
<div v-else-if="note.body" class="text-sm text-neutral-700 dark:text-neutral-300">
<MarkdownText :text="bodyPreview" />
</div>
<p
v-if="!note.title && !note.body && !note.items.length && !note.attachments.length"
v-if="!note.body && !note.items.length && !note.attachments.length"
class="text-sm italic text-neutral-400"
>
Empty note
</p>
</div>
<!-- Inline links: a compact strip at the FOOT of the card, under the note's own
words rather than stacked on top of them. They were above the body until
M13 — which put a stranger's headline where the note's first line should be. -->
<div v-if="!isLoneUrl && note.previews.length" class="mt-2 flex flex-col gap-1">
<LinkPreview v-for="p in note.previews" :key="p.id" :preview="p" compact />
</div>
<NoteChecklist
v-if="note.items.length"
:class="note.body || note.title ? 'mt-2' : ''"
:class="note.body ? 'mt-2' : ''"
:note-id="note.id"
:items="note.items"
@click="emit('open', note)"
+18 -90
View File
@@ -1,7 +1,6 @@
<script setup lang="ts">
import { computed, nextTick, onMounted, ref, watch } from "vue";
import { useNotesStore } from "../stores/notes";
import { useConfigStore } from "../stores/config";
import ColorPicker from "./ColorPicker.vue";
import Icon from "./Icon.vue";
import LabelPicker from "./LabelPicker.vue";
@@ -24,10 +23,8 @@ const props = withDefaults(defineProps<{ note?: Note | null; initialBody?: strin
});
const emit = defineEmits<{ (e: "close"): void; (e: "navigate", id: string): void }>();
const notes = useNotesStore();
const config = useConfigStore();
const noteId = ref<string | null>(props.note?.id ?? null);
const title = ref(props.note?.title ?? "");
const body = ref(props.note?.body ?? props.initialBody);
const color = ref<NoteColor>(props.note?.color ?? "default");
const labelList = ref<NoteLabel[]>(props.note ? [...props.note.labels] : []);
@@ -42,14 +39,13 @@ const fileInput = ref<HTMLInputElement | null>(null);
const uploadError = ref("");
// Baseline for edit-mode change detection (save only when text actually changed).
const baseline = ref<{ title: string | null; body: string; color: NoteColor }>({
title: props.note?.title ?? null,
const baseline = ref<{ body: string; color: NoteColor }>({
body: props.note?.body ?? "",
color: (props.note?.color ?? "default") as NoteColor,
});
const isCreate = computed(() => noteId.value === null);
const hasContent = computed(() => title.value.trim() !== "" || body.value.trim() !== "");
const hasContent = computed(() => body.value.trim() !== "");
// Rich features need a saved note; in compose they light up once there's content.
const richEnabled = computed(() => !isCreate.value || hasContent.value);
@@ -57,7 +53,6 @@ const richEnabled = computed(() => !isCreate.value || hasContent.value);
// template can read attachments/items/remind_at uniformly.
const draftNote = computed<Note>(() => ({
id: "",
title: title.value.trim() || null,
display_title: "",
body: body.value,
color: color.value,
@@ -96,19 +91,18 @@ watch(
() => props.note,
(n) => {
noteId.value = n?.id ?? null;
title.value = n?.title ?? "";
body.value = n?.body ?? "";
color.value = (n?.color ?? "default") as NoteColor;
labelList.value = n ? [...n.labels] : [];
baseline.value = { title: n?.title ?? null, body: n?.body ?? "", color: (n?.color ?? "default") as NoteColor };
baseline.value = { body: n?.body ?? "", color: (n?.color ?? "default") as NoteColor };
},
);
// ---- persistence ----
async function createFromFields(): Promise<void> {
const created = await notes.create({ title: title.value, body: body.value, color: color.value });
const created = await notes.create({ body: body.value, color: color.value });
noteId.value = created.id;
baseline.value = { title: created.title, body: created.body, color: created.color as NoteColor };
baseline.value = { body: created.body, color: created.color as NoteColor };
}
// Ensure a persisted note exists (for rich actions mid-compose). Returns its id, or
@@ -135,12 +129,12 @@ async function flush(): Promise<void> {
}
const b = baseline.value;
const nextBody = body.value;
const changed = (title.value.trim() || null) !== b.title || nextBody !== b.body || color.value !== b.color;
const changed = nextBody !== b.body || color.value !== b.color;
if (!changed) return;
saving.value = true;
try {
await notes.saveEdit(noteId.value as string, { title: title.value, body: nextBody, color: color.value });
baseline.value = { title: title.value.trim() || null, body: nextBody, color: color.value };
await notes.saveEdit(noteId.value as string, { body: nextBody, color: color.value });
baseline.value = { body: nextBody, color: color.value };
} finally {
saving.value = false;
}
@@ -148,12 +142,11 @@ async function flush(): Promise<void> {
function resetCompose(): void {
noteId.value = null;
title.value = "";
body.value = "";
color.value = "default";
labelList.value = [];
checklistOpen.value = false;
baseline.value = { title: null, body: "", color: "default" };
baseline.value = { body: "", color: "default" };
uploadError.value = "";
}
@@ -249,12 +242,6 @@ function onBodyKeydown(e: KeyboardEvent) {
}
}
function onTitleEnter(e: KeyboardEvent) {
e.preventDefault();
if (e.shiftKey && isCreate.value) void commitAndContinue();
else bodyInput.value?.focus();
}
// ---- reminder ----
const reminderLocal = computed(() => toLocalInput(liveNote.value.remind_at));
async function onReminderChange(e: Event) {
@@ -334,41 +321,12 @@ async function uploadFile(file: File) {
}
}
// ---- link previews (URL unfurl) ----
const unfurling = ref<string | null>(null); // the URL currently being fetched
const unfurlError = ref("");
// Bare http(s) URLs in the body; trailing sentence punctuation trimmed.
const URL_RE = /(https?:\/\/[^\s<>"'\])]+)/g;
const detectedUrls = computed(() => {
const out: string[] = [];
for (const m of body.value.matchAll(URL_RE)) {
const u = m[1].replace(/[.,;:!?]+$/, "");
if (!out.includes(u)) out.push(u);
}
return out;
});
const previewedUrls = computed(() => new Set(liveNote.value.previews.map((p) => p.url)));
const unpreviewedUrls = computed(() => detectedUrls.value.filter((u) => !previewedUrls.value.has(u)));
async function addPreview(url: string) {
const id = await ensureDraft();
if (!id) return;
unfurling.value = url;
unfurlError.value = "";
try {
await notes.unfurl(id, url);
} catch (e) {
unfurlError.value = (e as { error?: string }).error ?? "Couldn't fetch a preview for that link.";
} finally {
unfurling.value = null;
}
}
function shortUrl(url: string): string {
try {
return new URL(url).hostname.replace(/^www\./, "");
} catch {
return url;
}
}
// ---- link previews ----
//
// Nothing to trigger any more: the server unfurls a note's URLs in the background
// after each save (`unfurl_queue.py`) and the preview arrives on a later read. What
// is left here is removing one you don't want — the editor is the only place with
// room to offer that, and the card deliberately doesn't.
async function onFileChange(e: Event) {
const input = e.target as HTMLInputElement;
const file = input.files?.[0];
@@ -413,10 +371,9 @@ async function restoreRevisionAt(revId: string) {
const id = noteId.value;
if (!id) return;
const updated = await notes.restoreRevision(id, revId);
title.value = updated.title ?? "";
body.value = updated.body;
color.value = updated.color;
baseline.value = { title: updated.title, body: updated.body, color: updated.color };
baseline.value = { body: updated.body, color: updated.color };
void loadRevisions(); // the pre-restore state became a new revision
}
function revLabel(iso: string | null): string {
@@ -424,9 +381,7 @@ function revLabel(iso: string | null): string {
return new Date(iso).toLocaleString(undefined, { month: "short", day: "numeric", hour: "numeric", minute: "2-digit" });
}
function revPreview(rev: NoteRevision): string {
const t = (rev.title ?? "").trim();
const b = rev.body.trim().replace(/\s+/g, " ");
const s = t && b ? `${t}${b}` : t || b;
const s = rev.body.trim().replace(/\s+/g, " ");
if (!s) return "(empty)";
return s.length > 80 ? `${s.slice(0, 80)}` : s;
}
@@ -508,7 +463,7 @@ function revPreview(rev: NoteRevision): string {
</div>
<p v-if="uploadError" class="text-xs text-red-600 dark:text-red-400">{{ uploadError }}</p>
<!-- Link previews: stored preview cards + one "Preview <domain>" per detected URL -->
<!-- Fetched automatically after each save; removable here and nowhere else. -->
<div v-if="liveNote.previews.length" class="flex flex-col gap-2">
<LinkPreview
v-for="p in liveNote.previews"
@@ -518,31 +473,6 @@ function revPreview(rev: NoteRevision): string {
@remove="notes.deletePreview(liveNote.id, p.id)"
/>
</div>
<div
v-if="config.enableUrlUnfurl && !liveNote.trashed && unpreviewedUrls.length"
class="flex flex-wrap gap-1.5"
>
<button
v-for="u in unpreviewedUrls"
:key="u"
type="button"
class="inline-flex items-center gap-1 rounded-full border border-neutral-200 px-2 py-0.5 text-xs text-neutral-500 hover:bg-neutral-50 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand disabled:opacity-60 dark:border-neutral-700 dark:hover:bg-neutral-800"
:disabled="unfurling === u"
@click="addPreview(u)"
>
<Icon name="link" />
{{ unfurling === u ? "Fetching…" : `Preview ${shortUrl(u)}` }}
</button>
</div>
<p v-if="unfurlError" class="text-xs text-red-600 dark:text-red-400">{{ unfurlError }}</p>
<input
v-model="title"
type="text"
placeholder="Title (optional)"
class="w-full bg-transparent text-base font-semibold outline-none placeholder:text-neutral-400"
@keydown.enter="onTitleEnter"
/>
<textarea
ref="bodyInput"
@@ -628,7 +558,6 @@ function revPreview(rev: NoteRevision): string {
</div>
</div>
</div>
<div
v-if="!isCreate && showHistory"
@@ -750,6 +679,5 @@ function revPreview(rev: NoteRevision): string {
</div>
</div>
</div>
</div>
</Transition>
</template>
-1
View File
@@ -20,7 +20,6 @@ const router = createRouter({
{ path: "archive", name: "archive", component: () => import("../views/BoardView.vue") },
{ path: "trash", name: "trash", component: () => import("../views/BoardView.vue") },
{ path: "label/:id", name: "label", component: () => import("../views/BoardView.vue") },
{ path: "search", name: "search", component: () => import("../views/SearchView.vue") },
{ path: "reminders", name: "reminders", component: () => import("../views/RemindersView.vue") },
{ path: "timeline", name: "timeline", component: () => import("../views/TimelineView.vue") },
],
+5 -8
View File
@@ -52,19 +52,17 @@ export interface LinkPreview {
site_name: string | null;
}
// A past version of a note's title+body (version history).
// A past version of a note's body (version history).
export interface NoteRevision {
id: string;
title: string | null;
body: string;
created_at: string | null;
}
export interface Note {
id: string;
title: string | null;
// The note's display NAME: explicit title, else its first body line (server-derived).
// Every note has one, so a body-only note still has something to be called.
// The note's NAME: its first body line, else its first checklist item
// (server-derived). Every note has one, so every note has something to be called.
display_title: string;
body: string;
color: NoteColor;
@@ -134,7 +132,6 @@ export const useNotesStore = defineStore("notes", () => {
}
async function create(input: {
title: string;
body: string;
color: NoteColor;
items?: string[];
@@ -147,7 +144,7 @@ export const useNotesStore = defineStore("notes", () => {
async function mutate(
id: string,
changes: Partial<
Pick<Note, "title" | "body" | "color" | "pinned" | "archived" | "remind_at" | "recurrence">
Pick<Note, "body" | "color" | "pinned" | "archived" | "remind_at" | "recurrence">
>,
): Promise<void> {
reconcile(await repo.notes.update(id, changes));
@@ -162,7 +159,7 @@ export const useNotesStore = defineStore("notes", () => {
const setColor = (id: string, color: NoteColor) => mutate(id, { color });
const setReminder = (id: string, remindAt: string | null) => mutate(id, { remind_at: remindAt });
const setRecurrence = (id: string, recurrence: string | null) => mutate(id, { recurrence });
const saveEdit = (id: string, changes: { title: string; body: string; color: NoteColor }) => mutate(id, changes);
const saveEdit = (id: string, changes: { body: string; color: NoteColor }) => mutate(id, changes);
async function completeReminder(id: string): Promise<void> {
reconcile(await repo.notes.completeReminder(id));
-48
View File
@@ -1,48 +0,0 @@
<script setup lang="ts">
import { computed, watch } from "vue";
import { useRoute } from "vue-router";
import { repo } from "../adapters";
import { useNoteList } from "../composables/useNoteList";
import { useNoteEditor } from "../composables/useNoteEditor";
import AsyncState from "../components/AsyncState.vue";
import EmptyState from "../components/EmptyState.vue";
import NoteGrid from "../components/NoteGrid.vue";
import NoteEditor from "../components/NoteEditor.vue";
const route = useRoute();
const query = computed(() => (typeof route.query.q === "string" ? route.query.q : ""));
const noMatchSubtitle = computed(() => `Nothing found for "${query.value}".`);
const { items: results, loading, error, load: run } = useNoteList(async () => {
const q = query.value.trim();
if (!q) return [];
return repo.notes.search(q);
}, "Search failed.");
const { editing, open: openEditor, close: closeEditor, navigate: onNavigate } = useNoteEditor({
list: () => results.value,
onClose: run, // reflect any edits made from a result
});
watch(query, run, { immediate: true });
</script>
<template>
<div class="mx-auto w-full max-w-6xl px-4 py-6">
<p class="mb-4 text-sm text-neutral-500 dark:text-neutral-400">
<template v-if="query"
>Results for <span class="font-semibold text-neutral-800 dark:text-neutral-200">{{ query }}</span></template
>
<template v-else>Type in the search box to find your notes.</template>
</p>
<AsyncState :loading="loading" :error="error || undefined" error-title="Couldn't search" @retry="run">
<EmptyState v-if="query && results.length === 0" title="No matches" :subtitle="noMatchSubtitle" />
<NoteGrid v-else-if="results.length" :notes="results" @open="openEditor" />
</AsyncState>
</div>
<template v-if="editing">
<NoteEditor :note="editing" @close="closeEditor" @navigate="onNavigate" />
</template>
</template>
+10
View File
@@ -12,6 +12,10 @@ interface SettingItem {
label: string;
description: string;
group: string;
// Ints only, and nullable: the server sends the registry's bounds so the number
// input can refuse an out-of-range value before the round trip.
minimum: number | null;
maximum: number | null;
}
const config = useConfigStore();
@@ -131,10 +135,16 @@ onMounted(load);
:checked="Boolean(it.value)"
@change="it.value = ($event.target as HTMLInputElement).checked"
/>
<!-- min/max come from the registry. The server rejects out-of-range
values regardless — this is so the browser says so first, rather than
letting someone type a hop count that would disable a protection and
only learn about it from an error banner. -->
<input
v-else-if="it.type === 'int'"
:id="it.key"
type="number"
:min="it.minimum ?? undefined"
:max="it.maximum ?? undefined"
class="w-28 rounded-lg border border-neutral-300 bg-white px-3 py-2 text-sm text-neutral-900 shadow-sm focus:outline-none focus-visible:ring-2 focus-visible:ring-brand dark:border-neutral-700 dark:bg-neutral-800 dark:text-neutral-100"
:value="Number(it.value)"
@input="it.value = Number(($event.target as HTMLInputElement).value)"
+7 -1
View File
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
[project]
name = "thoughtsync"
version = "0.1.0"
version = "0.2.0"
description = "Self-hosted personal thought-capture web app (FabledSword family)"
requires-python = ">=3.12"
dependencies = [
@@ -29,6 +29,12 @@ where = ["src"]
[tool.pytest.ini_options]
asyncio_mode = "auto"
testpaths = ["tests"]
# The unit lane runs `-m "not integration"`; the integration lane runs `-m integration`
# against a real Postgres. Registered here so an unmarked typo fails loudly instead of
# quietly landing a test in neither lane.
markers = [
"integration: needs a live Postgres — CI's integration job, not the unit lane",
]
[tool.ruff]
line-length = 120
+1 -1
View File
@@ -1,3 +1,3 @@
"""ThoughtSync — self-hosted personal thought-capture web app (FabledSword family)."""
__version__ = "0.1.0"
__version__ = "0.2.0"
+23 -17
View File
@@ -1,13 +1,14 @@
from __future__ import annotations
import asyncio
import logging
import mimetypes
import os
import secrets
from contextlib import suppress
from datetime import timedelta
from quart import Quart, has_request_context, jsonify, request, send_from_directory
from quart import Quart, jsonify, send_from_directory
from quart.sessions import SecureCookieSessionInterface
from . import __version__
@@ -17,12 +18,25 @@ from .config import Config
from .db import session_scope
from .labels import bp as labels_bp
from .notes import bp as notes_bp
from .proxy import is_https
from .retention import run_sweeper
from .saved_filters import bp as saved_filters_bp
from .settings import get_public_config, get_setting, load_or_create_secret_key
from .settings import get_public_config, get_setting, load_or_create_secret_key, refresh_live
from .settings_api import bp as settings_bp
from .sync import bp as sync_bp, protocol_advertisement
# Without this, `logger.info` from this package goes nowhere: hypercorn configures its
# own access/error loggers and leaves the root logger at WARNING, so the credential
# events in auth.py would be invisible in `docker compose logs` — which is exactly
# where they are meant to be read until an audit table exists (task 2939).
#
# `force=False` (the default) so a host that has already configured logging keeps its
# own setup; LOG_LEVEL lets an operator turn it up without a code change.
logging.basicConfig(
level=os.environ.get("THOUGHTSYNC_LOG_LEVEL", "INFO").upper(),
format="%(asctime)s %(levelname)s %(name)s %(message)s",
)
STATIC_DIR = os.path.join(os.path.dirname(__file__), "static")
# `.webmanifest` isn't in every base image's mime map; register it so the PWA
@@ -30,19 +44,6 @@ STATIC_DIR = os.path.join(os.path.dirname(__file__), "static")
mimetypes.add_type("application/manifest+json", ".webmanifest")
def _is_https() -> bool:
"""Whether this request reached us over TLS — directly, or through a proxy that
terminated it and said so in X-Forwarded-Proto.
Shared by the session cookie's Secure flag and by HSTS, because they are the same
question and answering it twice is how the two drift apart.
"""
if not has_request_context():
return False
forwarded = request.headers.get("X-Forwarded-Proto", "").split(",")[0].strip().lower()
return forwarded == "https" or request.is_secure
class _AutoSecureSessionInterface(SecureCookieSessionInterface):
"""Mark the session cookie `Secure` whenever the request arrived over HTTPS —
directly, or via a TLS-terminating reverse proxy that sets X-Forwarded-Proto.
@@ -54,7 +55,7 @@ class _AutoSecureSessionInterface(SecureCookieSessionInterface):
"""
def get_cookie_secure(self, app: Quart) -> bool:
return _is_https()
return is_https()
def create_app() -> Quart:
@@ -93,6 +94,11 @@ def create_app() -> Quart:
app.config["PERMANENT_SESSION_LIFETIME"] = timedelta(days=days)
except (ValueError, TypeError, KeyError):
pass
# The security settings the throttle and the proxy trust read on hot
# paths. Cached rather than queried per request; until this runs they
# hold their registry defaults, which is the correct behaviour for a
# server that has not finished starting.
await refresh_live(db)
# Expire old trash in the background (retention.py). One task per process is
# correct because the image serves with a single hypercorn worker (Dockerfile);
# if that ever gains `--workers`, this needs a lock so N workers don't each
@@ -168,7 +174,7 @@ def create_app() -> Quart:
# commit domains this app does not own. A browser still remembers the policy
# for up to a year after the header stops being sent, which is the point of
# it — worth knowing before putting a hostname behind TLS temporarily.
if _is_https():
if is_https():
response.headers.setdefault("Strict-Transport-Security", "max-age=31536000")
return response
+42 -2
View File
@@ -1,6 +1,7 @@
from __future__ import annotations
import functools
import logging
import uuid
from datetime import datetime, timezone
@@ -11,17 +12,27 @@ from .common import iso
from .db import session_scope
from .models.device_token import DeviceToken
from .models.user import User
from .proxy import client_address
from .ratelimit import (
client_address,
register_by_address,
sign_in_by_account,
sign_in_by_address,
)
from .security import dummy_verify, generate_token, hash_password, hash_token, verify_password
from .settings import get_setting
from .settings import get_setting, set_settings
bp = Blueprint("auth", __name__, url_prefix="/api/auth")
# Every credential event goes to the app log — there is no audit TABLE yet (see task
# 2939), and until there is, `docker compose logs` is the only way to know whether
# anyone is knocking. That matters most in exactly the window this was written for: a
# freshly-exposed instance.
#
# The attempted email is included deliberately. It is the operator's own server, and
# "somebody failed a login" without saying against WHICH account tells you nothing you
# can act on. Passwords, obviously, never appear.
logger = logging.getLogger(__name__)
SESSION_KEY = "user_id"
MIN_PASSWORD_LEN = 8
DEVICE_NAME_CAP = 100
@@ -120,6 +131,7 @@ def _throttled(retry_after: int):
`Retry-After` is standard and is the one thing a legitimate client (or person)
genuinely needs.
"""
logger.warning("throttled credential attempt from=%s retry_after=%ss", client_address(), retry_after)
return (
jsonify({"error": "too many attempts — try again shortly"}),
429,
@@ -186,6 +198,7 @@ async def register():
# The first account bootstraps the admin and is always allowed, even when
# registration is otherwise closed.
if not is_first and not await get_setting(db, "allow_registration"):
logger.warning("registration refused (closed) email=%s from=%s", email, client_address())
return jsonify({"error": "registration is closed"}), 403
existing = await db.scalar(select(User).where(User.email == email))
if existing is not None:
@@ -197,10 +210,27 @@ async def register():
is_admin=is_first,
)
db.add(user)
if is_first:
# Registration CLOSES the moment the instance has an owner.
#
# Not "defaults closed" — that would still need the first person to get in
# somehow. Closed as a CONSEQUENCE of the admin account existing, which is
# the only formulation with no open window in it. Leaving the setting on
# meant the gap between "my account exists" and "I remembered to turn it
# off in Settings" was wide open, and on a public host that gap is the
# entire exposure — it starts the moment DNS resolves.
#
# An admin who wants a second person turns it back on in Settings → Access,
# adds them, and turns it off. Crude until invites exist, but it is a
# deliberate act rather than a default nobody chose.
await set_settings(db, {"allow_registration": False})
await db.commit()
await db.refresh(user)
session[SESSION_KEY] = str(user.id)
session.permanent = True
logger.info(
"account created email=%s admin=%s from=%s", email, is_first, client_address()
)
return jsonify(_serialize_user(user)), 201
@@ -222,13 +252,16 @@ async def login():
# difference is a reliable oracle for which emails have accounts here.
dummy_verify(password)
_sign_in_failed(email)
logger.warning("sign-in failed (no such account) email=%s from=%s", email, client_address())
return jsonify({"error": "invalid email or password"}), 401
if not verify_password(password, user.password_hash):
_sign_in_failed(email)
logger.warning("sign-in failed (bad password) email=%s from=%s", email, client_address())
return jsonify({"error": "invalid email or password"}), 401
_sign_in_succeeded(email)
session[SESSION_KEY] = str(user.id)
session.permanent = True
logger.info("sign-in ok email=%s from=%s", email, client_address())
return jsonify(_serialize_user(user))
@@ -296,12 +329,19 @@ async def device_login():
if user is None or not user.password_hash:
dummy_verify(password)
_sign_in_failed(email)
logger.warning("device-login failed (no such account) email=%s from=%s", email, client_address())
return jsonify({"error": "invalid email or password"}), 401
if not verify_password(password, user.password_hash):
_sign_in_failed(email)
logger.warning("device-login failed (bad password) email=%s from=%s", email, client_address())
return jsonify({"error": "invalid email or password"}), 401
_sign_in_succeeded(email)
row, token = await _issue_device_token(db, user.id, data.get("name") or "")
# A device token outlives the session that made it, so its creation is the
# most consequential thing on this blueprint.
logger.info(
"device token issued email=%s device=%s from=%s", email, row.name, client_address()
)
await db.commit()
return jsonify({"token": token, "device": _serialize_device(row), "user": _serialize_user(user)}), 201
+1
View File
@@ -48,3 +48,4 @@ class Config:
def secret_key_env(cls) -> str | None:
"""Optional break-glass override for the cookie-signing secret."""
return os.environ.get("THOUGHTSYNC_SECRET_KEY") or None
+5 -6
View File
@@ -38,11 +38,11 @@ class Note(Base):
owner_id: Mapped[uuid.UUID] = mapped_column(
UUID(as_uuid=True), ForeignKey("users.id", ondelete="CASCADE"), nullable=False
)
title: Mapped[str | None] = mapped_column(Text(), nullable=True)
# The note's display NAME: explicit title if set, else the first non-empty body
# line (see notes.derive_display_title). Persisted so every note — even a body-only
# one — has something to be called in search results and in an export filename,
# without forcing the user to type a title.
# The note's NAME: its first non-empty body line, else its first checklist item
# (see notes.derive_display_title). There is no title field to prefer — a note is
# a body plus optional items, and this is simply the first thing written in it.
# Persisted so search results and export filenames have something to say, and so
# the full-text vector can weight it above the rest of the body.
display_title: Mapped[str] = mapped_column(Text(), nullable=False, server_default="")
body: Mapped[str] = mapped_column(Text(), nullable=False, server_default="")
color: Mapped[str] = mapped_column(Text(), nullable=False, server_default="default")
@@ -73,7 +73,6 @@ class Note(Base):
def serialize(self) -> dict:
return {
"id": str(self.id),
"title": self.title,
"display_title": self.display_title,
"body": self.body,
"color": self.color,
+3 -4
View File
@@ -11,9 +11,9 @@ from . import Base
class NoteRevision(Base):
"""A point-in-time snapshot of a note's title+body, written on each edit that
changes either — so an accidental overwrite can be viewed and restored. Only
title+body are versioned in v1 (not items/attachments/labels)."""
"""A point-in-time snapshot of a note's body, written on each edit that changes
it — so an accidental overwrite can be viewed and restored. Only the body is
versioned (not items/attachments/labels)."""
__tablename__ = "note_revisions"
__table_args__ = (Index("ix_note_revisions_note_created", "note_id", "created_at"),)
@@ -22,6 +22,5 @@ class NoteRevision(Base):
note_id: Mapped[uuid.UUID] = mapped_column(
UUID(as_uuid=True), ForeignKey("notes.id", ondelete="CASCADE"), nullable=False
)
title: Mapped[str | None] = mapped_column(Text(), nullable=True)
body: Mapped[str] = mapped_column(Text(), nullable=False, server_default="")
created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False, server_default=func.now())
+39 -52
View File
@@ -37,6 +37,7 @@ from ..models.note_revision import NoteRevision
from ..responses import json_error, not_found, parse_uuid
from ..retention import purge_note
from ..settings import get_setting
from ..unfurl_queue import schedule as schedule_unfurls
from ..unfurl import UnfurlError, unfurl
from ._bp import bp
from .helpers import (
@@ -139,8 +140,10 @@ async def list_notes():
return json_error("invalid created_before", 400)
stmt = stmt.where(Note.created_at < before_dt)
if query_text:
# Full-text match over title+body (generated tsvector, migration 0005),
# ranked — so the facet bar's text box searches, not just filters.
# Full-text match over the note's name + body (generated tsvector,
# migrations 0005/0026), ranked. This is the ONLY text search now: the
# separate facet-less `/search` route was removed because landing on it
# was the one place you could not also narrow by tag (note 2930).
tsquery = func.websearch_to_tsquery("english", query_text)
search_col = literal_column("notes.search_vector")
stmt = stmt.where(search_col.op("@@")(tsquery)).order_by(
@@ -154,30 +157,6 @@ async def list_notes():
return jsonify({"notes": await _serialize_notes(db, notes)})
@bp.get("/search")
@login_required
async def search_notes():
q = (request.args.get("q") or "").strip()
if not q:
return jsonify({"notes": []})
async with session_scope() as db:
tsquery = func.websearch_to_tsquery("english", q)
# search_vector is a generated column (migration 0005), not mapped on the ORM.
search_col = literal_column("notes.search_vector")
stmt = (
select(Note)
.where(
visible_to_user("note", Note.owner_id, Note.id, g.user_id),
Note.deleted_at.is_(None),
search_col.op("@@")(tsquery),
)
.order_by(func.ts_rank(search_col, tsquery).desc(), Note.updated_at.desc())
.limit(100)
)
notes = (await db.scalars(stmt)).all()
return jsonify({"notes": await _serialize_notes(db, notes)})
@bp.get("/reminders")
@login_required
async def list_reminders():
@@ -273,7 +252,6 @@ async def export_notes():
payload["notes"].append(
{
"id": str(n.id),
"title": n.title,
"display_title": n.display_title,
"body": n.body,
"color": n.color,
@@ -406,17 +384,32 @@ async def reorder_notes():
return jsonify({"ok": True})
async def _name_for(db, note: Note, item_texts: list[str] | None = None) -> str:
"""The note's display name, consulting its checklist only when the body is silent.
`item_texts` short-circuits the query for callers that already hold the items
(create, import). Everyone else pays one narrow SELECT, and only when the body
produced nothing — which is the uncommon case.
"""
name = derive_display_title(note.body)
if name:
return name
if item_texts is not None:
return derive_display_title("", item_texts[0] if item_texts else None)
first = await db.scalar(
select(NoteItem.text).where(NoteItem.note_id == note.id).order_by(NoteItem.position).limit(1)
)
return derive_display_title("", first)
@bp.post("")
@login_required
async def create_note():
data = await request.get_json(silent=True) or {}
title = data.get("title") if isinstance(data.get("title"), str) else ""
body = data.get("body") if isinstance(data.get("body"), str) else ""
# Items are accepted on ANY note now — a checklist is something a note HAS.
# Items are accepted on ANY note — a checklist is something a note HAS.
item_texts = parse_list_items(data.get("items"))
# "Empty" therefore means all three are empty, not just the two that used to
# matter for whichever kind this was.
if is_empty_note(title, body) and not item_texts:
if is_empty_note(body, item_texts):
return json_error("note is empty", 400)
async with session_scope() as db:
# New notes go to the top of the manual order.
@@ -425,11 +418,9 @@ async def create_note():
Note.owner_id == g.user_id, Note.deleted_at.is_(None)
)
)
clean_title = title.strip() or None
note = Note(
owner_id=g.user_id,
title=clean_title,
display_title=derive_display_title(clean_title, body),
display_title=derive_display_title(body, item_texts[0] if item_texts else None),
body=body,
color=normalize_color(data.get("color")),
position=int(max_pos) + 1,
@@ -441,6 +432,9 @@ async def create_note():
await _reconcile_tags(db, note)
await db.commit()
await db.refresh(note)
# After the commit, never before it: the note is saved and the response is
# about to go out. Any link previews arrive on a later read.
schedule_unfurls(note.id, note.body)
return jsonify(await _serialize_note(db, note)), 201
@@ -467,11 +461,7 @@ async def update_note(note_id: str):
note = await _get_owned(db, note_id)
if note is None:
return not_found()
old_title = note.title
old_body = note.body
if "title" in data:
title = data["title"] if isinstance(data["title"], str) else ""
note.title = title.strip() or None
if "body" in data and isinstance(data["body"], str):
note.body = data["body"]
if "color" in data:
@@ -492,24 +482,22 @@ async def update_note(note_id: str):
note.remind_at = remind_dt
if "recurrence" in data:
note.recurrence = normalize_recurrence(data["recurrence"])
# Recompute the display name (explicit title, else first body line) whenever
# the title or body may have changed.
if "title" in data or "body" in data:
note.display_title = derive_display_title(note.title, note.body)
if "body" in data:
note.display_title = await _name_for(db, note)
await _reconcile_tags(db, note)
# Version history: snapshot the PRE-edit title+body whenever either changed.
if note.title != old_title or note.body != old_body:
db.add(NoteRevision(note_id=note.id, title=old_title, body=old_body))
# Version history: snapshot the PRE-edit body whenever it changed.
if note.body != old_body:
db.add(NoteRevision(note_id=note.id, body=old_body))
await db.commit()
await db.refresh(note)
if note.body != old_body:
schedule_unfurls(note.id, note.body)
return jsonify(await _serialize_note(db, note))
def _serialize_revision(rev: NoteRevision) -> dict:
return {
"id": str(rev.id),
"title": rev.title,
"body": rev.body,
"created_at": iso(rev.created_at),
}
@@ -546,14 +534,13 @@ async def restore_revision(note_id: str, rev_id: str):
rev = await db.scalar(select(NoteRevision).where(NoteRevision.id == rid, NoteRevision.note_id == note.id))
if rev is None:
return not_found()
if note.title == rev.title and note.body == rev.body:
if note.body == rev.body:
return jsonify(await _serialize_note(db, note)) # already at this version — no-op
# Snapshot the CURRENT state first, so restoring is itself undoable, then apply
# the revision — with the same title/body ripple as a normal edit.
db.add(NoteRevision(note_id=note.id, title=note.title, body=note.body))
note.title = rev.title
# the revision — with the same body ripple as a normal edit.
db.add(NoteRevision(note_id=note.id, body=note.body))
note.body = rev.body
note.display_title = derive_display_title(note.title, note.body)
note.display_title = await _name_for(db, note)
await _reconcile_tags(db, note)
await db.commit()
await db.refresh(note)
+18 -10
View File
@@ -19,22 +19,30 @@ VALID_FILTERS = {"active", "archived", "trash"}
DISPLAY_TITLE_CAP = 200
def derive_display_title(title: str | None, body: str | None) -> str:
"""The note's display NAME: the explicit title if set, else the first non-empty
line of the body (trimmed, length-capped). Persisted as notes.display_title so a
body-only note is still nameable/searchable/linkable — the user never has to type
a title. Deterministic (literal first line, no AI)."""
if title and title.strip():
return title.strip()[:DISPLAY_TITLE_CAP]
def derive_display_title(body: str | None, first_item: str | None = None) -> str:
"""The note's display NAME: the first non-empty line of the body, else the first
checklist item's text (both trimmed and length-capped).
There is no explicit title to prefer any more (M13 step 3) — a note is a body plus
optional items, and its name is simply the first thing written in it. Persisted as
notes.display_title so search results and export filenames have something to say.
The item fallback is what step 2 bought: a note that is only a checklist would
otherwise have no name at all, which is exactly the hole that made removing the
title unsafe before checklists stopped being their own kind of thing.
Deterministic — a literal first line, never generated.
"""
for line in (body or "").splitlines():
stripped = line.strip()
if stripped:
return stripped[:DISPLAY_TITLE_CAP]
return ""
return (first_item or "").strip()[:DISPLAY_TITLE_CAP]
def is_empty_note(title: str | None, body: str | None) -> bool:
return not (title or "").strip() and not (body or "").strip()
def is_empty_note(body: str | None, items: list | None = None) -> bool:
"""Nothing worth keeping: no body text and no checklist items."""
return not (body or "").strip() and not items
def parse_list_items(raw: object) -> list[str]:
+17 -9
View File
@@ -36,8 +36,6 @@ def _note_markdown(note: Note, labels: list, items: list) -> str:
"""One note as a human-readable Markdown file with a small frontmatter block.
The authoritative machine format is notes.json; this is for reading/portability."""
fm = ["---"]
if note.title:
fm.append(f"title: {note.title}")
fm.append(f"display_name: {note.display_title}")
if labels:
fm.append("labels: [" + ", ".join(lb["name"] for lb in labels) + "]")
@@ -276,19 +274,29 @@ async def _create_imported_note(
db, owner_id, spec: dict, zf: zipfile.ZipFile, position: int, budget: _ImportBudget
) -> bool:
"""Insert one imported note plus its items/labels/attachments, reusing the same
display-title derivation + tag/link reconciliation as create_note. Returns False
(nothing written) when the spec is empty."""
title = (spec.get("title") or "").strip() or None
name derivation + tag reconciliation as create_note. Returns False (nothing
written) when the spec is empty."""
body = spec.get("body") or ""
# An imported title becomes the note's FIRST BODY LINE.
#
# ThoughtSync has no title field any more (M13 step 3), but the things people
# import from do — Keep notes carry one, and so does any export taken before this.
# Dropping it would silently lose text someone wrote; folding it into the body puts
# it exactly where a name now lives, so the note comes in named the way it was.
# Skipped when the body already opens with that line, so re-importing an export
# this code produced doesn't stack duplicates.
title = (spec.get("title") or "").strip()
if title and body.lstrip().split("\n", 1)[0].strip() != title:
body = f"{title}\n{body}" if body.strip() else title
items = spec.get("items") or []
has_items = any((it.get("text") or "").strip() for it in items)
if is_empty_note(title, body) and not has_items:
item_texts = [t for t in ((it.get("text") or "").strip() for it in items) if t]
if is_empty_note(body, item_texts):
return False
note = Note(
owner_id=owner_id,
title=title,
display_title=derive_display_title(title, body),
display_title=derive_display_title(body, item_texts[0] if item_texts else None),
body=body,
color=normalize_color(spec.get("color")),
pinned=bool(spec.get("pinned")),
+74
View File
@@ -0,0 +1,74 @@
"""Reading what the proxies in front of this app say about a request.
Two headers carry information the app cannot see for itself — who the client is
(`X-Forwarded-For`) and whether they arrived over TLS (`X-Forwarded-Proto`) — and both
are trusted by the same rule, so the rule lives in one place. Writing it twice is
precisely how issue 2183 happened: two places holding one decision, and only one of
them updated.
## The rule
A forwarding header grows LEFT to RIGHT. Each hop appends what IT saw, so the
rightmost entries are the ones our own infrastructure wrote, and anything a caller
sent arrives to the LEFT of those.
That inverts the intuitive reading. The leftmost entry is nominally "the original
client" — and is exactly the one a caller can forge, by sending the header themselves.
So we count in from the right by the number of proxies we actually run
(the **Trusted proxy hops** setting, default 1), and a forged prefix can never be
selected no matter how much of it there is.
Too HIGH a hop count is the dangerous direction: it starts believing entries no proxy
of ours wrote. Too low just means several callers share a bucket. So when the header
is shorter than configured — fewer proxies than expected — we fall back to the socket
address rather than reaching further left.
"""
from __future__ import annotations
from quart import has_request_context, request
from .settings import live
def trusted_entry(header: str, hops: int) -> str | None:
"""The nth-from-the-right entry of a forwarding header, or None if there isn't one.
Pure, so the trust boundary is testable without a request context.
"""
if hops <= 0:
return None
entries = [part.strip() for part in header.split(",") if part.strip()]
if len(entries) < hops:
return None
return entries[-hops]
def forwarded_for(header: str, remote_addr: str | None, hops: int) -> str:
"""The client address a proxy chain vouches for, else this connection's peer."""
entry = trusted_entry(header, hops)
return (entry or remote_addr or "unknown")[:64] # bounded: becomes a dict key
def client_address() -> str:
"""The caller's address, as far as the deployment's own proxies vouch for it."""
return forwarded_for(
request.headers.get("X-Forwarded-For", ""),
request.remote_addr,
live("trusted_proxy_hops"),
)
def is_https() -> bool:
"""Whether this request reached us over TLS — directly, or via a trusted proxy.
Shared by the session cookie's `Secure` flag and by HSTS, because they are the same
question. Read with the same hop count as the address: a caller who sets
`X-Forwarded-Proto: https` on a plain-HTTP request puts it to the left of whatever
our proxy appended, so it is not what gets read.
"""
if not has_request_context():
return False
if request.is_secure:
return True
entry = trusted_entry(request.headers.get("X-Forwarded-Proto", ""), live("trusted_proxy_hops"))
return (entry or "").lower() == "https"
+40 -46
View File
@@ -22,9 +22,11 @@ came from, and either one can refuse it:
is what stops credential stuffing against one known email, no matter how many
addresses the attempts arrive from.
- **The address** bounds the damage from one source spraying many accounts. It is
best-effort by nature — behind a reverse proxy the client address is read from
``X-Forwarded-For``, which a caller can set to anything if the app is exposed
directly. That is precisely why it is not the only key.
read from ``X-Forwarded-For``, counting in from the RIGHT by
the **Trusted proxy hops** setting so that only entries our own proxies wrote are
believed — a forged header lands to the left of those and is never selected. It is
still the weaker of the two keys, because it depends on that setting matching the
deployment; the account key depends on nothing.
Counting is by failure for the sign-in routes and by attempt for registration: a
correct password should never move someone closer to being locked out, but every
@@ -34,30 +36,18 @@ from __future__ import annotations
import time
from collections import deque
from collections.abc import Callable
from quart import request
from .settings import live
# Failed sign-ins tolerated per account before it stops answering, and for how long.
# Ten is comfortably above a person mistyping a password and far below anything that
# makes a dictionary worth running.
ACCOUNT_LIMIT = 10
ACCOUNT_WINDOW_S = 15 * 60
# Wider, because one address is legitimately many people: a household, an office
# behind NAT, a phone on carrier-grade NAT.
ADDRESS_LIMIT = 50
ADDRESS_WINDOW_S = 15 * 60
# Registration is scarcer than a sign-in — it creates a row, and on a private
# instance the honest number of accounts anyone needs to make is one.
REGISTER_LIMIT = 5
REGISTER_WINDOW_S = 60 * 60
# Never let the bookkeeping become the denial of service: an attacker rotating a
# forged X-Forwarded-For could otherwise mint an unbounded number of buckets. Well
# above any real deployment's distinct-caller count, so a legitimate instance never
# reaches it; when it is reached the oldest buckets are dropped, which at worst
# forgives some attempts.
# Never let the bookkeeping become the denial of service: an attacker rotating an
# address could otherwise mint an unbounded number of buckets. Well above any real
# deployment's distinct-caller count, so a legitimate instance never reaches it; when
# it is reached the oldest buckets are dropped, which at worst forgives some attempts.
#
# Not a setting: it protects the limiter from itself rather than the app from a
# caller, and there is no operator judgment to apply to it.
MAX_BUCKETS = 10_000
@@ -67,13 +57,25 @@ class SlidingWindow:
Sliding rather than a fixed window because a fixed one lets twice the limit
through across a boundary — 10 at 14:59 and 10 at 15:00 — which for a login
limiter is the difference between the number meaning something and not.
The limit and window are SUPPLIERS, not values, so an admin saving a new number in
Settings takes effect on the next attempt instead of the next deploy. They are read
per call, which is a dict lookup — the settings cache never touches the database.
"""
def __init__(self, limit: int, window_s: float) -> None:
self.limit = limit
self.window_s = window_s
def __init__(self, limit: Callable[[], int], window_s: Callable[[], float]) -> None:
self._limit = limit
self._window_s = window_s
self._hits: dict[str, deque[float]] = {}
@property
def limit(self) -> int:
return self._limit()
@property
def window_s(self) -> float:
return self._window_s()
def _prune(self, key: str, now: float) -> deque[float]:
hits = self._hits.get(key)
if hits is None:
@@ -111,27 +113,19 @@ class SlidingWindow:
self._hits.clear()
sign_in_by_account = SlidingWindow(ACCOUNT_LIMIT, ACCOUNT_WINDOW_S)
sign_in_by_address = SlidingWindow(ADDRESS_LIMIT, ADDRESS_WINDOW_S)
register_by_address = SlidingWindow(REGISTER_LIMIT, REGISTER_WINDOW_S)
def _minutes(key: str) -> Callable[[], float]:
return lambda: float(live(key)) * 60.0
def client_address() -> str:
"""The caller's address, as well as it can be known.
``X-Forwarded-For`` is a list appended to by each hop, so the leftmost entry is
the original client — and also the only entry a client can choose for itself.
It is trusted here anyway, because the alternative behind a reverse proxy is to
see the proxy's address for every request on earth and rate-limit the entire
internet as one caller. The account-keyed limit is the one that holds when this
one is lied to.
"""
forwarded = request.headers.get("X-Forwarded-For", "")
if forwarded:
first = forwarded.split(",")[0].strip()
if first:
return first[:64] # bounded: this becomes a dict key
return (request.remote_addr or "unknown")[:64]
sign_in_by_account = SlidingWindow(
lambda: live("signin_limit_per_account"), _minutes("signin_window_minutes")
)
sign_in_by_address = SlidingWindow(
lambda: live("signin_limit_per_address"), _minutes("signin_window_minutes")
)
register_by_address = SlidingWindow(
lambda: live("register_limit_per_address"), _minutes("register_window_minutes")
)
def reset_all() -> None:
-1
View File
@@ -89,7 +89,6 @@ async def purge_note(db, note: Note, edited_at: datetime | None = None) -> None:
await db.execute(sa_delete(NoteLabel).where(NoteLabel.note_id == note.id))
await db.execute(sa_delete(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))
await db.execute(sa_delete(NoteRevision).where(NoteRevision.note_id == note.id))
note.title = None
note.body = ""
note.display_title = ""
# `deleted_at` deliberately SURVIVES. It's still true — that is when the note was
+133 -2
View File
@@ -19,6 +19,13 @@ class SettingDef:
label: str
description: str
group: str
# Ints only. Enforced server-side in validate_updates and passed to the UI so the
# number input carries them too. These exist because several of the security
# values have ranges where a typo is not merely wrong but dangerous — a proxy hop
# count of 50 would trust anything a caller sent, and a sign-in limit of 0 would
# lock every account out permanently.
minimum: int | None = None
maximum: int | None = None
# The source of truth for every user-facing setting. Add a row here and it appears
@@ -32,7 +39,8 @@ REGISTRY: list[SettingDef] = [
"bool",
True,
"Allow new registrations",
"When off, only existing users can sign in. The first account is always allowed.",
"When off, only existing users can sign in. Closes itself once the first "
"account exists — turn it back on only while you're adding someone.",
"Access",
),
SettingDef(
@@ -69,6 +77,80 @@ REGISTRY: list[SettingDef] = [
"The server contacts the linked site; private/internal addresses are always blocked.",
"Links",
),
# --- Security -----------------------------------------------------------------
#
# Read on paths too hot for a database round trip (the credential throttle checks
# them BEFORE opening a connection, which is the point of checking a throttle
# before doing expensive work), so they are cached — see `live()` below.
SettingDef(
"trusted_proxy_hops",
"int",
1,
"Trusted proxy hops",
"How many proxies sit in front of this server. 1 for a single reverse proxy "
"terminating HTTPS; 2 if a CDN like Cloudflare sits in front of that; 0 if "
"the app is exposed directly. This decides which entry of X-Forwarded-For is "
"believed — set it TOO HIGH and a visitor can forge their own address and "
"slip the sign-in limits below.",
"Security",
minimum=0,
maximum=10,
),
SettingDef(
"signin_limit_per_account",
"int",
10,
"Failed sign-ins per account",
"How many failures one account tolerates within the window before it stops "
"answering. Comfortably above mistyping a password, far below anything that "
"makes guessing worth attempting.",
"Security",
minimum=1,
maximum=1000,
),
SettingDef(
"signin_limit_per_address",
"int",
50,
"Failed sign-ins per address",
"The same, counted per visitor address instead of per account — it bounds one "
"source trying many accounts. Wider, because one address is legitimately many "
"people: a household, an office, a phone on carrier NAT.",
"Security",
minimum=1,
maximum=10000,
),
SettingDef(
"signin_window_minutes",
"int",
15,
"Sign-in window (minutes)",
"The trailing period both sign-in limits are counted over.",
"Security",
minimum=1,
maximum=1440,
),
SettingDef(
"register_limit_per_address",
"int",
5,
"Sign-ups per address",
"How many accounts one address may create within its window. Counted per "
"attempt rather than per failure — each one is a row either way.",
"Security",
minimum=1,
maximum=1000,
),
SettingDef(
"register_window_minutes",
"int",
60,
"Sign-up window (minutes)",
"The trailing period the sign-up limit is counted over.",
"Security",
minimum=1,
maximum=10080,
),
]
_BY_KEY: dict[str, SettingDef] = {d.key: d for d in REGISTRY}
@@ -152,11 +234,52 @@ async def get_admin_settings(db) -> list[dict]:
"label": d.label,
"description": d.description,
"group": d.group,
"minimum": d.minimum,
"maximum": d.maximum,
}
)
return result
# Settings the app must be able to read WITHOUT awaiting a database.
#
# The credential throttle consults these before opening a connection — deliberately,
# because a refused attempt is supposed to cost nothing, and the proxy hop count is
# needed to know who is even asking. A per-request query would undo both.
#
# Seeded from the registry defaults so the app works before (and without) a database —
# unit tests construct it with no Postgres at all — then refreshed from the DB at boot
# and again whenever an admin saves. Same live-update contract `session_ttl_days`
# already has in settings_api.py.
_LIVE_KEYS = (
"trusted_proxy_hops",
"signin_limit_per_account",
"signin_limit_per_address",
"signin_window_minutes",
"register_limit_per_address",
"register_window_minutes",
)
_live: dict[str, Any] = {k: _BY_KEY[k].default for k in _LIVE_KEYS}
def live(key: str) -> Any:
"""The cached value of a hot setting. Synchronous, never touches the database."""
return _live[key]
async def refresh_live(db) -> None:
"""Re-read the hot settings into the cache. Called at boot and after every save."""
for key in _LIVE_KEYS:
_live[key] = await get_setting(db, key)
def reset_live() -> None:
"""Back to registry defaults. For tests — nothing in the app calls this."""
for key in _LIVE_KEYS:
_live[key] = _BY_KEY[key].default
def validate_updates(updates: dict) -> tuple[dict, str | None]:
"""Coerce/validate a {key: value} dict against the registry. Returns
(clean_values, error_message). An unknown key or a bad int is rejected."""
@@ -167,9 +290,17 @@ def validate_updates(updates: dict) -> tuple[dict, str | None]:
return {}, f"unknown setting: {key}"
if defn.type == "int":
try:
clean[key] = int(val)
n = int(val)
except (ValueError, TypeError):
return {}, f"{defn.label} must be a whole number"
# Rejected rather than clamped: silently accepting a number and storing a
# different one is how somebody ends up believing a protection is set to
# something it is not.
if defn.minimum is not None and n < defn.minimum:
return {}, f"{defn.label} must be at least {defn.minimum}"
if defn.maximum is not None and n > defn.maximum:
return {}, f"{defn.label} must be at most {defn.maximum}"
clean[key] = n
elif defn.type == "bool":
clean[key] = _coerce_bool(val)
else:
+5 -1
View File
@@ -6,7 +6,7 @@ from quart import Blueprint, current_app, jsonify, request
from .auth import require_admin
from .db import session_scope
from .settings import get_admin_settings, set_settings, validate_updates
from .settings import get_admin_settings, refresh_live, set_settings, validate_updates
bp = Blueprint("settings", __name__, url_prefix="/api/settings")
@@ -35,6 +35,10 @@ async def update_settings():
async with session_scope() as db:
await set_settings(db, clean)
await db.commit()
# Re-read the cached security values so a saved limit or hop count applies to
# the very next request. Unconditional: cheap, and a conditional here would be
# one more place that has to know which keys are hot.
await refresh_live(db)
result = await get_admin_settings(db)
# Apply the live-tunable knob without a restart (rule 25).
+34 -13
View File
@@ -35,6 +35,7 @@ from .notes import (
)
from .retention import purge_note
from .serialize import serialize_label_sync
from .unfurl_queue import schedule as schedule_unfurls
bp = Blueprint("sync", __name__, url_prefix="/api/sync")
@@ -55,13 +56,12 @@ MAX_PUSH = 1000 # per-batch change cap
# Bump SYNC_PROTOCOL_VERSION for ANY wire change. Raise
# MIN_CLIENT_PROTOCOL_VERSION only for a genuinely BREAKING one: it is the switch
# that hard-blocks older clients, so additive changes must leave it alone.
# v2 (M13): `kind` left the wire. Dropping a field a v1 client sends and expects back
# is breaking, so the FLOOR moves too — a v1 client would keep pushing a `kind` the
# server no longer stores, and would read back notes without one.
# v2 (M13): `kind` and `title` both left the wire. Dropping a field a v1 client sends
# and expects back is breaking, so the FLOOR moves too — a v1 client would keep pushing
# both and would read back notes carrying neither.
#
# `title` goes the same way in step 3. It lands in this same protocol generation, so
# it needs no further bump — v2 means "no kind, no title", and nothing has run against
# a half-applied v2.
# One bump for the pair: they landed in the same protocol generation, and nothing ever
# ran against a half-applied v2.
SYNC_PROTOCOL_VERSION = 2
MIN_CLIENT_PROTOCOL_VERSION = 2
@@ -197,8 +197,6 @@ def client_wins(client_edited_at: datetime | None, server_edited_at: datetime |
def _assign_note_fields(note: Note, ch: dict) -> None:
"""Overwrite a note's scalar fields from a client's FULL-state change (sync is
whole-note, not a partial patch — the client sends its authoritative version)."""
title = ch.get("title")
note.title = (title or "").strip() or None if isinstance(title, str) else None
note.body = ch["body"] if isinstance(ch.get("body"), str) else ""
note.color = normalize_color(ch.get("color"))
note.pinned = bool(ch.get("pinned"))
@@ -214,6 +212,24 @@ def _assign_note_fields(note: Note, ch: dict) -> None:
note.position = ch["position"]
def _first_item_text(ch: dict) -> str:
"""The first non-blank checklist item in a pushed change, or "".
Read straight from the payload rather than the database because the note's name is
computed BEFORE `_apply_note_items` has written anything — and a note whose body is
empty is named by its first item (M13 step 3).
"""
items = ch.get("items")
if not isinstance(items, list):
return ""
for it in items:
if isinstance(it, dict):
text = (it.get("text") or "").strip()
if text:
return text
return ""
async def _apply_note_items(db, note: Note, ch: dict) -> None:
"""Replace the note's checklist items with the client's (items sync inline).
@@ -296,20 +312,25 @@ async def _apply_note(db, ch: dict) -> dict:
elif note.purged_at is not None:
note.purged_at = None # client re-created/edited → clear the tombstone
old_title, old_body = note.title, note.body
old_body = note.body
_assign_note_fields(note, ch)
note.display_title = derive_display_title(note.title, note.body)
note.display_title = derive_display_title(note.body, _first_item_text(ch))
if edited_at is not None:
note.updated_at = edited_at
# Non-destructive LWW: snapshot the overwritten server title+body into history.
if not creating and (note.title != old_title or note.body != old_body):
db.add(NoteRevision(note_id=note.id, title=old_title, body=old_body))
# Non-destructive LWW: snapshot the overwritten server body into history.
if not creating and note.body != old_body:
db.add(NoteRevision(note_id=note.id, body=old_body))
await db.flush() # assign note.id before items/labels/links
await _apply_note_items(db, note, ch)
await _reconcile_tags(db, note)
await _apply_note_manual_labels(db, note, ch)
await db.flush()
await db.refresh(note, ["sync_revision"])
# A note pushed from a linked client gets the same link previews as one typed into
# the web app — the client picks them up on its next pull. Scheduled rather than
# awaited: a push batch must not wait on somebody else's website.
if creating or note.body != old_body:
schedule_unfurls(note.id, note.body)
return {
"id": str(nid),
"entity": "note",
+127
View File
@@ -0,0 +1,127 @@
"""Unfurling a note's URLs in the background, after the note is already saved.
## Why this is not done inline
Capture speed is the product. Unfurling is a 5-second-timeout network call to a host
nobody controls, and a note must persist the instant someone stops typing — so the
save returns first and the preview catches up. A person who pastes a link and closes
the composer has already done the thing they came to do.
## Why it is on the server rather than in each client
The server sees every note that reaches it, from all three surfaces, so detection and
fetching live in one place instead of three. A linked desktop or Android client pushes
its note and picks the preview up on the next pull; an unlinked one has no server to
ask and simply has no preview until it links, which is the honest consequence of being
offline rather than a gap to paper over.
## What it deliberately does not do
Fail loudly. A preview that could not be fetched is not an error the person needs —
the note is fine, it just has no card. The link is still in the body, still clickable,
still searchable.
"""
from __future__ import annotations
import asyncio
import logging
import re
import uuid
from sqlalchemy import select
from .db import session_scope
from .models.note import Note
from .models.note_link_preview import NoteLinkPreview
from .settings import get_setting
from .unfurl import UnfurlError, unfurl
logger = logging.getLogger(__name__)
# Matches the frontend's detector (NoteEditor.vue) so both surfaces agree on what
# counts as a link. Trailing sentence punctuation is stripped below rather than in the
# pattern — a URL can legitimately end in most of these characters, just not when the
# sentence does.
_URL_RE = re.compile(r"(https?://[^\s<>\"'\])]+)")
# Per note, per save. A body pasted full of links should not turn into a burst of
# outbound requests; nobody is reading forty preview cards on one card anyway.
MAX_URLS_PER_NOTE = 5
# Background tasks are only weakly referenced by the event loop, so without a strong
# reference here a task can be garbage-collected mid-flight. Discarded on completion.
_running: set[asyncio.Task] = set()
def detect_urls(body: str | None) -> list[str]:
"""Distinct http(s) URLs in a note body, in order, trailing punctuation trimmed."""
out: list[str] = []
for match in _URL_RE.finditer(body or ""):
url = match.group(1).rstrip(".,;:!?")
if url and url not in out:
out.append(url)
return out
async def _fetch_and_store(note_id: uuid.UUID, url: str) -> None:
"""Unfurl one URL and cache it against the note. Silent on every failure."""
try:
preview = await unfurl(url)
except UnfurlError as e:
# Expected and uninteresting: a dead link, a private address, a non-page.
logger.debug("no preview for %s: %s", url, e)
return
except Exception:
logger.warning("unexpected failure unfurling %s", url, exc_info=True)
return
async with session_scope() as db:
# The note may have been deleted or the URL removed while the fetch was in
# flight, so re-check rather than assuming the world held still.
note = await db.scalar(select(Note).where(Note.id == note_id, Note.deleted_at.is_(None)))
if note is None or url not in detect_urls(note.body):
return
row = await db.scalar(
select(NoteLinkPreview).where(NoteLinkPreview.note_id == note_id, NoteLinkPreview.url == url)
)
if row is None:
row = NoteLinkPreview(note_id=note_id, url=url)
db.add(row)
row.title = preview["title"]
row.description = preview["description"]
row.image_url = preview["image_url"]
row.site_name = preview["site_name"]
await db.commit()
async def _unfurl_new_urls(note_id: uuid.UUID, body: str) -> None:
async with session_scope() as db:
if not await get_setting(db, "enable_url_unfurl"):
return
cached = set(
(
await db.scalars(select(NoteLinkPreview.url).where(NoteLinkPreview.note_id == note_id))
).all()
)
fresh = [u for u in detect_urls(body) if u not in cached][:MAX_URLS_PER_NOTE]
for url in fresh:
await _fetch_and_store(note_id, url)
def schedule(note_id: uuid.UUID, body: str | None) -> None:
"""Queue an unfurl pass for a note that was just written. Returns immediately.
Safe to call on every save: it re-reads what is already cached and does nothing
when there is nothing new, so an edit that doesn't touch the links costs one
cheap query on a background task rather than a fetch.
"""
if not body or not detect_urls(body):
return
try:
task = asyncio.create_task(_unfurl_new_urls(note_id, body))
except RuntimeError:
# No running loop — a script or a test calling the write path directly. The
# note is saved either way; only the preview is skipped.
return
_running.add(task)
task.add_done_callback(_running.discard)
+416
View File
@@ -0,0 +1,416 @@
"""The real-Postgres lane (family rule 6).
Everything else in this suite is deliberately DB-free, which means the schema the
migrations build has never been checked against the models that read it. That gap is
what this file closes, and it is not theoretical: M13 dropped three columns and
rebuilt a generated column, and until now `alembic upgrade head` ran for the first
time when the operator's container started.
Marked `integration` and excluded from the unit lane by `-m "not integration"`, so a
workstation without Postgres runs the rest of the suite unchanged.
The schema comes from real migrations, never `metadata.create_all` (rule 82) — the
point is to test what actually ships, and `create_all` would build a schema no
deployment has ever seen.
"""
from __future__ import annotations
import uuid
import pytest
import pytest_asyncio
from sqlalchemy import select, text
from thoughtsync import ratelimit
from thoughtsync.app import create_app
from thoughtsync.db import dispose_engine, session_scope
from thoughtsync.models.note import Note
from thoughtsync.models.note_item import NoteItem
from thoughtsync.models.user import User
from thoughtsync.settings import get_setting, live, refresh_live, reset_live, set_settings
from thoughtsync.notes.helpers import derive_display_title
from thoughtsync.models.note_link_preview import NoteLinkPreview
from thoughtsync.sync import _apply_note_items
from thoughtsync.unfurl_queue import _unfurl_new_urls, detect_urls
pytestmark = pytest.mark.integration
# Every table the tests touch, child-first so FKs never block the truncate.
# RESTART IDENTITY + CASCADE keeps this honest if a table gains children later.
_TABLES = "notes, note_items, note_revisions, note_labels, note_link_previews, labels, users"
@pytest_asyncio.fixture
async def db():
"""A session against the migrated database, wiped before each test.
Wiped BEFORE rather than after so a failed test leaves its rows behind to look at.
"""
async with session_scope() as session:
await session.execute(text(f"TRUNCATE {_TABLES} RESTART IDENTITY CASCADE"))
await session.commit()
yield session
await dispose_engine()
@pytest_asyncio.fixture
async def app_client(db):
"""A test client against the real app, over the migrated database.
The credential throttle is process-global and its counters outlive a single
test, so they are cleared here — otherwise a suite that registers a few times
starts handing out 429s for reasons that have nothing to do with the test.
"""
ratelimit.reset_all()
yield create_app().test_client()
ratelimit.reset_all()
@pytest_asyncio.fixture
async def owner(db):
"""A user to hang notes off — `notes.owner_id` is a real foreign key."""
user = User(email=f"{uuid.uuid4().hex}@example.test", display_name="Integration")
db.add(user)
await db.commit()
await db.refresh(user)
return user
async def test_the_migrated_schema_matches_the_models(db, owner):
"""The check that has never run: insert through the ORM, read it back.
A column the models expect and the migrations never created — or the reverse —
fails right here, instead of when a container starts.
"""
note = Note(owner_id=owner.id, body="a thought", display_title="a thought")
db.add(note)
await db.commit()
await db.refresh(note)
found = await db.scalar(select(Note).where(Note.id == note.id))
assert found is not None
assert found.body == "a thought"
assert found.display_title == "a thought"
async def test_the_dropped_columns_are_actually_gone(db):
"""M13 dropped three. If a migration silently no-opped, this is where it shows."""
cols = set(
(
await db.execute(
text("SELECT column_name FROM information_schema.columns WHERE table_name = 'notes'")
)
)
.scalars()
.all()
)
assert "title" not in cols, "notes.title should have gone in 0026"
assert "kind" not in cols, "notes.kind should have gone in 0025"
assert "display_title" in cols and "body" in cols
rev_cols = set(
(
await db.execute(
text("SELECT column_name FROM information_schema.columns WHERE table_name = 'note_revisions'")
)
)
.scalars()
.all()
)
assert "title" not in rev_cols, "note_revisions.title should have gone in 0026"
tables = set(
(await db.execute(text("SELECT table_name FROM information_schema.tables WHERE table_schema = 'public'")))
.scalars()
.all()
)
assert "note_links" not in tables, "note_links should have gone in 0024"
async def test_the_search_vector_was_rebuilt_over_the_name(db, owner):
"""0026 had to drop and recreate a STORED GENERATED column.
Postgres refuses to drop a column another generated column depends on, so getting
this wrong doesn't produce a subtly wrong ranking — it produces a migration that
won't run at all. Worth proving the replacement actually indexes something.
"""
note = Note(owner_id=owner.id, body="ferry tickets\nbook before friday", display_title="ferry tickets")
db.add(note)
await db.commit()
hit = await db.scalar(
text(
"SELECT count(*) FROM notes "
"WHERE search_vector @@ websearch_to_tsquery('english', :q)"
).bindparams(q="ferry")
)
assert hit == 1
# The NAME is weight A and the body weight B, which is what makes a name match
# rank above a body-only one. Both must be in the vector at all.
body_only = await db.scalar(
text(
"SELECT count(*) FROM notes "
"WHERE search_vector @@ websearch_to_tsquery('english', :q)"
).bindparams(q="friday")
)
assert body_only == 1
async def test_a_note_keeps_both_its_body_and_its_items(db, owner):
"""The shape M13 step 2 made normal: a note HAS a checklist, it isn't one."""
note = Note(owner_id=owner.id, body="weekend shop", display_title="weekend shop")
db.add(note)
await db.flush()
db.add_all(
[
NoteItem(note_id=note.id, text="milk", position=0),
NoteItem(note_id=note.id, text="eggs", position=1),
]
)
await db.commit()
items = (
await db.scalars(select(NoteItem).where(NoteItem.note_id == note.id).order_by(NoteItem.position))
).all()
assert [i.text for i in items] == ["milk", "eggs"]
assert (await db.scalar(select(Note.body).where(Note.id == note.id))) == "weekend shop"
async def test_sync_no_longer_deletes_items_from_a_note_with_a_body(db, owner):
"""The data-loss path step 2 removed, pinned against a real database.
`_apply_note_items` used to delete every item when the note wasn't `kind = "list"`.
Nothing can produce that state any more, but this is the regression that would
have silently eaten a checklist, and it deserves a test that would catch its
return.
"""
note = Note(owner_id=owner.id, body="packing", display_title="packing")
db.add(note)
await db.flush()
db.add(NoteItem(note_id=note.id, text="socks", position=0))
await db.commit()
# A change that says nothing about items must LEAVE them alone — absent means
# "not telling us", not "empty".
await _apply_note_items(db, note, {"body": "packing"})
await db.commit()
assert (await db.scalar(select(NoteItem.text).where(NoteItem.note_id == note.id))) == "socks"
# An explicit list replaces them.
await _apply_note_items(db, note, {"items": [{"text": "charger", "checked": True}]})
await db.commit()
rows = (await db.scalars(select(NoteItem).where(NoteItem.note_id == note.id))).all()
assert [(r.text, r.checked) for r in rows] == [("charger", True)]
async def test_a_note_with_only_items_still_has_a_name(db, owner):
"""The hole that made removing the title unsafe until step 2 closed it."""
note = Note(owner_id=owner.id, body="", display_title="")
db.add(note)
await db.flush()
db.add(NoteItem(note_id=note.id, text="milk", position=0))
await db.commit()
first = await db.scalar(
select(NoteItem.text).where(NoteItem.note_id == note.id).order_by(NoteItem.position).limit(1)
)
note.display_title = derive_display_title(note.body, first)
await db.commit()
assert (await db.scalar(select(Note.display_title).where(Note.id == note.id))) == "milk"
async def test_auto_unfurl_stores_a_preview_and_skips_what_is_cached(db, owner, monkeypatch):
"""The background pass, run inline so the assertions are deterministic.
The network is stubbed — this is about what reaches the DATABASE, not about
parsing someone's OpenGraph tags (unfurl.py's own tests cover that). What matters
here is the part only a real database can show: the unique constraint holding, the
upsert going to the right row, and a second pass not re-fetching.
"""
note = Note(
owner_id=owner.id,
body="read https://example.com/a and https://example.com/b",
display_title="read https://example.com/a and https://example.com/b",
)
db.add(note)
await db.commit()
calls: list[str] = []
async def fake_unfurl(url):
calls.append(url)
return {"url": url, "title": f"T {url}", "description": None, "image_url": None, "site_name": "example.com"}
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
await _unfurl_new_urls(note.id, note.body)
assert sorted(calls) == ["https://example.com/a", "https://example.com/b"]
rows = (await db.scalars(select(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))).all()
assert {r.url for r in rows} == {"https://example.com/a", "https://example.com/b"}
assert all(r.title.startswith("T ") for r in rows)
# A second pass over an unchanged body fetches nothing — the whole reason
# `schedule` is safe to call on every save.
calls.clear()
await _unfurl_new_urls(note.id, note.body)
assert calls == []
async def test_auto_unfurl_drops_a_preview_whose_url_left_the_body(db, owner, monkeypatch):
"""A slow fetch must not resurrect a link the person deleted mid-flight."""
note = Note(owner_id=owner.id, body="https://example.com/gone", display_title="x")
db.add(note)
await db.commit()
async def fake_unfurl(url):
# Simulate the body changing while the request was in the air.
return {"url": url, "title": "T", "description": None, "image_url": None, "site_name": None}
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
note.body = "changed my mind"
await db.commit()
await _unfurl_new_urls(note.id, "https://example.com/gone")
rows = (await db.scalars(select(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))).all()
assert rows == [], "a preview was stored for a URL the note no longer contains"
async def test_detection_agrees_with_what_gets_stored(db, owner, monkeypatch):
"""The detector and the storage path read the same body the same way."""
body = "one https://example.com/x. two (https://example.com/y) three"
assert detect_urls(body) == ["https://example.com/x", "https://example.com/y"]
note = Note(owner_id=owner.id, body=body, display_title="one")
db.add(note)
await db.commit()
async def fake_unfurl(url):
return {"url": url, "title": "T", "description": None, "image_url": None, "site_name": None}
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
await _unfurl_new_urls(note.id, body)
stored = {
r for r in (await db.scalars(select(NoteLinkPreview.url).where(NoteLinkPreview.note_id == note.id))).all()
}
assert stored == set(detect_urls(body))
async def test_registration_closes_itself_once_an_admin_exists(app_client, db):
"""The gap this removes: registration was open between "my account exists" and
"I remembered to turn it off", and on a public host that gap starts at DNS.
Runs against a real database because it is the interaction between two writes —
the user row and the settings row — inside one transaction.
"""
# The instance is empty (the fixture truncated it), so this is the first account:
# allowed unconditionally, and it becomes the admin.
first = await app_client.post(
"/api/auth/register",
json={"email": "owner@example.test", "password": "a-long-enough-password"},
)
assert first.status_code == 201
assert (await first.get_json())["is_admin"] is True
# …and the door shut behind it.
async with session_scope() as fresh:
assert await get_setting(fresh, "allow_registration") is False
second = await app_client.post(
"/api/auth/register",
json={"email": "stranger@example.test", "password": "a-long-enough-password"},
)
assert second.status_code == 403
# Re-opening it deliberately still works — that is how a second person gets in
# until invites exist.
async with session_scope() as fresh:
await set_settings(fresh, {"allow_registration": True})
await fresh.commit()
third = await app_client.post(
"/api/auth/register",
json={"email": "invited@example.test", "password": "a-long-enough-password"},
)
assert third.status_code == 201
assert (await third.get_json())["is_admin"] is False
async def test_security_settings_are_live_and_bounded(app_client, db):
"""The security values are settings now, not constants — so saving one has to take
effect without a restart, and a dangerous value has to be refused.
Real database because the whole point is the round trip: write through the admin
API, re-read into the cache the throttle consults, observe the new number.
"""
# An admin to authenticate as. First account, so it is allowed and becomes admin.
reset_live()
created = await app_client.post(
"/api/auth/register",
json={"email": "admin@example.test", "password": "a-long-enough-password"},
)
assert created.status_code == 201
# Defaults are what the registry says.
async with session_scope() as fresh:
await refresh_live(fresh)
assert live("trusted_proxy_hops") == 1
assert live("signin_limit_per_account") == 10
# A value that would disable the protection is REFUSED, not clamped — storing a
# different number than the one typed is how somebody ends up believing a limit
# is set to something it is not.
bad = await app_client.patch("/api/settings", json={"signin_limit_per_account": 0})
assert bad.status_code == 400
assert "at least" in (await bad.get_json())["error"]
# …and so is a hop count that would trust anything a caller sent.
bad_hops = await app_client.patch("/api/settings", json={"trusted_proxy_hops": 99})
assert bad_hops.status_code == 400
# A legitimate change applies to the cache the throttle reads, immediately.
ok = await app_client.patch(
"/api/settings", json={"signin_limit_per_account": 3, "trusted_proxy_hops": 2}
)
assert ok.status_code == 200
assert live("signin_limit_per_account") == 3
assert live("trusted_proxy_hops") == 2
# And it is persisted, not just cached.
async with session_scope() as fresh:
assert await get_setting(fresh, "trusted_proxy_hops") == 2
reset_live()
async def test_the_security_group_reaches_the_admin_ui(app_client, db):
"""Every security value has to be visible and editable, which is the whole reason
they moved out of the environment."""
created = await app_client.post(
"/api/auth/register",
json={"email": "admin2@example.test", "password": "a-long-enough-password"},
)
assert created.status_code == 201
resp = await app_client.get("/api/settings")
assert resp.status_code == 200
rows = (await resp.get_json())["settings"]
security = {r["key"]: r for r in rows if r["group"] == "Security"}
assert set(security) == {
"trusted_proxy_hops",
"signin_limit_per_account",
"signin_limit_per_address",
"signin_window_minutes",
"register_limit_per_address",
"register_window_minutes",
}
# The UI renders a number input from these, and it cannot offer a safe range it
# was never told about.
for row in security.values():
assert row["type"] == "int"
assert row["minimum"] is not None and row["maximum"] is not None
assert row["description"], f"{row['key']} has no description to explain itself"
+49 -22
View File
@@ -5,6 +5,7 @@ import pytest
from thoughtsync.app import create_app
from thoughtsync.common import coerce_bool, parse_dt
from thoughtsync.models.note import NOTE_COLORS, Note
from thoughtsync.unfurl_queue import detect_urls
from thoughtsync.notes import (
_attachment_ext,
_header_filename,
@@ -36,7 +37,7 @@ def test_all_note_routes_registered(app):
expected = {
f"notes.{name}"
for name in (
"list_notes", "search_notes", "list_reminders", "complete_reminder",
"list_notes", "list_reminders", "complete_reminder",
"snooze_reminder", "export_notes", "import_notes", "list_titles",
"reorder_notes", "create_note",
"get_note", "update_note", "list_revisions", "restore_revision",
@@ -51,9 +52,10 @@ def test_all_note_routes_registered(app):
def test_is_empty_note():
assert is_empty_note(None, None)
assert is_empty_note("", " ")
assert not is_empty_note("title", "")
assert not is_empty_note("", "body")
assert is_empty_note(" ", [])
assert not is_empty_note("body")
# A note that is only a checklist is not empty — it just has nothing in its body.
assert not is_empty_note("", ["milk"])
def test_normalize_color():
@@ -69,9 +71,9 @@ def test_palette_has_core_colors():
def test_serialize_shape():
n = Note(title="t", body="b", color="blue", pinned=True, archived=False)
n = Note(body="b", color="blue", pinned=True, archived=False)
s = n.serialize()
assert s["title"] == "t"
assert "title" not in s # there is no title field any more (M13 step 3)
assert s["body"] == "b"
assert s["color"] == "blue"
assert s["pinned"] is True
@@ -122,30 +124,34 @@ async def test_reorder_requires_auth(app):
# notice a route coming back, and the removal is one commit rather than a fossil.
def test_derive_display_title_explicit_wins():
assert derive_display_title("My Title", "some body line") == "My Title"
assert derive_display_title(" Padded ", "body") == "Padded"
def test_derive_display_title_from_first_body_line():
assert derive_display_title(None, "first line\nsecond line") == "first line"
assert derive_display_title("", " spaced first \nnext") == "spaced first"
def test_derive_display_title_is_the_first_body_line():
assert derive_display_title("first line\nsecond line") == "first line"
assert derive_display_title(" spaced first \nnext") == "spaced first"
# leading blank/whitespace lines are skipped to the first line with content
assert derive_display_title(None, "\n \nreal line\nmore") == "real line"
# a whitespace-only title falls through to the body
assert derive_display_title(" ", "body wins") == "body wins"
assert derive_display_title("\n \nreal line\nmore") == "real line"
def test_derive_display_title_falls_back_to_the_first_item():
# What step 2 bought: a note that is only a checklist still has a name. Without
# this it would have none at all, which is why the title could not go first.
assert derive_display_title("", "milk") == "milk"
assert derive_display_title(" \n ", " eggs ") == "eggs"
# The body still wins when it has anything to say.
assert derive_display_title("shopping", "milk") == "shopping"
def test_derive_display_title_empty():
assert derive_display_title(None, None) == ""
assert derive_display_title("", "") == ""
assert derive_display_title(" ", " \n ") == ""
assert derive_display_title(None) == ""
assert derive_display_title("") == ""
assert derive_display_title(" \n ", None) == ""
assert derive_display_title(" \n ", " ") == ""
def test_derive_display_title_caps_length():
long = "x" * 300
assert derive_display_title(None, long) == "x" * 200
assert derive_display_title(long, "body") == "x" * 200
assert derive_display_title(long) == "x" * 200
# the item fallback is capped on the same rule
assert derive_display_title("", long) == "x" * 200
def test_parse_tags():
@@ -371,6 +377,8 @@ def test_native_spec_roundtrip_fields():
"attachments": [{"file": "attachments/ab/img.png", "mime": "image/png"}],
}
spec = _native_spec(n)
# The spec still CARRIES a title — an export taken before M13 has one, and
# _create_imported_note folds it into the body rather than dropping it.
assert spec["title"] == "T"
assert spec["body"] == "b"
assert spec["color"] == "blue"
@@ -379,3 +387,22 @@ def test_native_spec_roundtrip_fields():
assert spec["created_at"].year == 2026
assert spec["labels"] == ["x"]
assert spec["attachments"] == [{"file": "attachments/ab/img.png", "mime": "image/png"}]
def test_detect_urls_finds_each_link_once_in_order():
body = "see https://example.com/a and https://example.com/b\nand https://example.com/a again"
assert detect_urls(body) == ["https://example.com/a", "https://example.com/b"]
def test_detect_urls_trims_sentence_punctuation():
# A URL can end in most punctuation; a SENTENCE containing one usually doesn't.
assert detect_urls("read https://example.com/page.") == ["https://example.com/page"]
assert detect_urls("(see https://example.com/x)") == ["https://example.com/x"]
# …but a path that legitimately ends in a slash or a dash keeps it.
assert detect_urls("https://example.com/dir/") == ["https://example.com/dir/"]
def test_detect_urls_ignores_non_http():
assert detect_urls("ftp://example.com and mailto:a@b.c and bare example.com") == []
assert detect_urls(None) == []
assert detect_urls("") == []
+73
View File
@@ -0,0 +1,73 @@
"""The proxy trust boundary.
The whole security property is "a caller cannot forge their own address", and it rests
on counting in from the RIGHT of the header rather than the left. These are the cases
that tell the two apart — pure functions, no request context, no database.
"""
from thoughtsync.proxy import forwarded_for, trusted_entry
from thoughtsync.settings import live
PEER = "10.0.0.1" # the socket address: our own proxy, or the caller when unproxied
def test_default_is_one_hop():
# One reverse proxy terminating TLS — this deployment, and the only shape that is
# safe to assume. A wrong default here is a silent security bug, not a preference.
#
# Read through live() rather than the registry: live() is what proxy.py actually
# calls, and it is seeded from the defaults at import time so the value is right
# before the first database read. A boot that never reached the DB must still
# count one hop, not zero.
assert live("trusted_proxy_hops") == 1
def test_no_proxy_ignores_the_header_entirely():
# hops=0 says nothing in front of us appends anything, so the header can only be
# something a caller invented.
assert forwarded_for("1.2.3.4", PEER, 0) == PEER
def test_one_hop_reads_what_our_proxy_wrote():
assert forwarded_for("203.0.113.7", PEER, 1) == "203.0.113.7"
def test_a_forged_prefix_is_never_selected():
# THE test. A caller sends `X-Forwarded-For: 1.2.3.4`; our proxy appends the
# address it actually saw. Reading from the left would hand the caller a fresh
# rate-limit bucket for every value they invent.
assert forwarded_for("1.2.3.4, 203.0.113.7", PEER, 1) == "203.0.113.7"
# …and padding it doesn't help either.
assert forwarded_for("a, b, c, d, 203.0.113.7", PEER, 1) == "203.0.113.7"
def test_two_hops_sees_past_a_cdn():
# Cloudflare appended the real client; our proxy appended Cloudflare.
assert forwarded_for("203.0.113.7, 172.16.0.5", PEER, 2) == "203.0.113.7"
assert forwarded_for("1.2.3.4, 203.0.113.7, 172.16.0.5", PEER, 2) == "203.0.113.7"
def test_a_short_header_falls_back_rather_than_reaching_left():
# Fewer proxies than configured. Reaching further left would start believing
# entries no proxy of ours wrote, so the safe direction is the socket address —
# at worst several callers share one bucket.
assert forwarded_for("203.0.113.7", PEER, 2) == PEER
assert forwarded_for("", PEER, 1) == PEER
def test_malformed_headers_do_not_crash_or_leak_empties():
assert forwarded_for(",,,", PEER, 1) == PEER
assert forwarded_for(" , 203.0.113.7 , ", PEER, 1) == "203.0.113.7"
def test_the_key_is_length_bounded():
# It becomes a dict key in the limiter; an unbounded header must not become an
# unbounded allocation.
assert len(forwarded_for("x" * 5000, PEER, 1)) <= 64
def test_trusted_entry_reports_absence_rather_than_guessing():
# `is_https` needs to tell "no trusted entry" apart from "an entry saying http",
# which is why this returns None rather than a default.
assert trusted_entry("", 1) is None
assert trusted_entry("https", 0) is None
assert trusted_entry("http, https", 1) == "https"
+19 -22
View File
@@ -13,10 +13,18 @@ import time
import pytest
from thoughtsync import ratelimit
from thoughtsync.settings import live
from thoughtsync.app import create_app
from thoughtsync.ratelimit import SlidingWindow
def window(limit: int, window_s: float) -> SlidingWindow:
"""A fixed-value window. The real ones read their numbers from the settings cache
so an admin's change applies immediately; these tests are about the counting, not
about where the numbers come from."""
return SlidingWindow(lambda: limit, lambda: window_s)
@pytest.fixture(autouse=True)
def _clean_counters():
ratelimit.reset_all()
@@ -30,7 +38,7 @@ def app():
def test_under_the_limit_is_not_blocked():
w = SlidingWindow(limit=3, window_s=60)
w = window(3, 60)
for i in range(3):
assert w.retry_after("k", now=i) is None
w.record("k", now=i)
@@ -38,7 +46,7 @@ def test_under_the_limit_is_not_blocked():
def test_window_slides_rather_than_resetting():
w = SlidingWindow(limit=2, window_s=60)
w = window(2, 60)
w.record("k", now=0)
w.record("k", now=30)
assert w.retry_after("k", now=31) is not None
@@ -50,7 +58,7 @@ def test_window_slides_rather_than_resetting():
def test_retry_after_points_past_the_oldest_hit():
w = SlidingWindow(limit=1, window_s=100)
w = window(1, 100)
w.record("k", now=10)
wait = w.retry_after("k", now=40)
# The hit at t=10 leaves the window at t=110, i.e. 70s away. Rounded up, never
@@ -62,14 +70,14 @@ def test_retry_after_points_past_the_oldest_hit():
def test_keys_are_counted_separately():
w = SlidingWindow(limit=1, window_s=60)
w = window(1, 60)
w.record("a", now=0)
assert w.retry_after("a", now=1) is not None
assert w.retry_after("b", now=1) is None
def test_forget_clears_one_key():
w = SlidingWindow(limit=1, window_s=60)
w = window(1, 60)
w.record("a", now=0)
w.record("b", now=0)
w.forget("a")
@@ -81,7 +89,7 @@ def test_bucket_count_is_bounded(monkeypatch):
# An attacker rotating a forged X-Forwarded-For must not be able to grow this
# dict without limit — the limiter cannot become the exhaustion it prevents.
monkeypatch.setattr(ratelimit, "MAX_BUCKETS", 8)
w = SlidingWindow(limit=5, window_s=60)
w = window(5, 60)
for i in range(50):
w.record(f"addr-{i}", now=i)
assert len(w._hits) <= 8
@@ -97,7 +105,7 @@ async def test_login_starts_refusing(app):
# at t=0..9 are fifteen minutes stale the moment the route reads
# `time.monotonic()` and get pruned before they can refuse anything.
now = time.monotonic()
for _ in range(ratelimit.ACCOUNT_LIMIT):
for _ in range(live("signin_limit_per_account")):
ratelimit.sign_in_by_account.record("someone@example.com", now=now)
resp = await client.post("/api/auth/login", json=body)
assert resp.status_code == 429
@@ -109,7 +117,7 @@ async def test_login_starts_refusing(app):
async def test_device_login_shares_the_account_counter(app):
client = app.test_client()
now = time.monotonic()
for _ in range(ratelimit.ACCOUNT_LIMIT):
for _ in range(live("signin_limit_per_account")):
ratelimit.sign_in_by_account.record("someone@example.com", now=now)
resp = await client.post(
"/api/auth/device-login",
@@ -123,26 +131,15 @@ async def test_device_login_shares_the_account_counter(app):
async def test_register_is_throttled_by_address(app):
client = app.test_client()
now = time.monotonic()
for _ in range(ratelimit.REGISTER_LIMIT):
for _ in range(live("register_limit_per_address")):
ratelimit.register_by_address.record("203.0.113.9", now=now)
resp = await client.post(
"/api/auth/register",
json={"email": "new@example.com", "password": "a-long-enough-password"},
# One entry, so with the default single trusted hop this IS the address the
# limiter keys on. The forged-prefix cases live in test_proxy.py.
headers={"X-Forwarded-For": "203.0.113.9"},
)
assert resp.status_code == 429
async def test_client_address_prefers_the_forwarded_client(app):
# Behind a reverse proxy, remote_addr is the PROXY for every request on earth —
# keying on it would rate-limit the entire internet as one caller. The leftmost
# X-Forwarded-For entry is the original client.
async with app.test_request_context("/", headers={"X-Forwarded-For": "198.51.100.4, 10.0.0.1"}):
assert ratelimit.client_address() == "198.51.100.4"
async def test_client_address_falls_back_to_the_peer(app):
async with app.test_request_context("/"):
# No proxy header: whatever the peer address is, it must be a usable key
# rather than an empty string sharing one bucket with everyone.
assert ratelimit.client_address()