MULTI_TENANT: an opt-in mode serving many isolated workspaces from one PostgreSQL-backed process - #31
Open
distronode-com wants to merge 52 commits into
Open
MULTI_TENANT: an opt-in mode serving many isolated workspaces from one PostgreSQL-backed process#31distronode-com wants to merge 52 commits into
distronode-com wants to merge 52 commits into
Conversation
Calnode's SQL is hand-written and portable apart from placeholder syntax, so supporting a second engine is mostly a matter of not spreading "? vs $n" across 763 call sites. This adds the layer that hides it: - Dialect (sqlite, postgres) selected from the DATABASE_URL scheme, with Dialect.SQL for the few statements that genuinely cannot be shared. - Rebind: a small lexer, not a string replace. A ? inside a string literal, a quoted identifier or a comment is data, and renumbering one corrupts the statement in a way that only shows up at runtime. - DB/Tx wrapping sql.DB/sql.Tx, rebinding every statement on the way through. Query, Exec, QueryRow, their Context forms, Prepare and Begin/BeginTx are all covered, so a call site moving onto the wrapper is a type change and nothing else. Begin returns *db.Tx rather than *sql.Tx deliberately: a transaction that quietly stopped rebinding is the easiest way to reintroduce ? on Postgres. - OpenDB returns the wrapper; Open still returns the bare *sql.DB so nothing outside this package changes yet. The SQLite path is untouched: same pragmas, and the one-connection pool stays because it is the correctness guarantee behind the booking overlap check (ARCHITECTURE section 17), not a tuning choice. Postgres gets an ordinary pool, and openPostgres records what that costs so the gap is not rediscovered later. Migrations move to migrations/sqlite/ unchanged (git detects all 57 as renames); the postgres set lands in the next commit. is_applied is now tested for truth rather than compared to 1, which is the one spelling both engines accept -- goose stores it as INTEGER on SQLite and BOOLEAN on Postgres. pgx v5.10.0 is the driver, via database/sql (stdlib), so nothing here depends on the pgx-native API.
57 files in migrations/postgres, one per SQLite migration and numbered
identically, embedded alongside them. Open picks the directory and the goose
dialect from the DSN, so nothing else has to know which set is in use.
What the translation does, and what it deliberately does not:
- Flag columns stay integers (INTEGER -> SMALLINT), never BOOLEAN. Those columns
are scanned into Go ints across the codebase, so changing the type would break
every one of those scans. Columns that hold real numbers stay INTEGER. A test
reads information_schema and fails on a boolean.
- Timestamp defaults keep TEXT and keep SQLite's exact string format:
strftime('%Y-%m-%dT%H:%M:%fZ','now') becomes
to_char(now() AT TIME ZONE 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"') and
datetime('now') becomes the seconds-precision equivalent. TIMESTAMPTZ was the
alternative and is wrong here: every time value in this schema is written,
compared, sorted and paginated as a string, and database/sql would hand those
columns back formatted as RFC3339Nano, which sorts differently and would not
match a stored value. A test asserts both formats against the shapes SQLite
writes.
- Partial unique indexes carry over verbatim; a test proves idx_bookings_no_double
still refuses a second booking at the same start time and still allows re-booking
a cancelled slot.
- crypto_keystore.id relied on SQLite's rowid (keyvault.go inserts without an id),
so it becomes BIGINT GENERATED BY DEFAULT AS IDENTITY. server_settings.id does
not: it is seeded explicitly and pinned by CHECK(id = 1), where a sequence would
only mislead. BLOB becomes BYTEA.
- The three table rebuilds become what Postgres can do directly: 00012 drops the
four columns, 00014 and 00042 replace a constraint in place. 00042 therefore
needs neither NO TRANSACTION nor the foreign_keys PRAGMA toggle, both of which
existed only to make SQLite's rebuild safe.
- lower(hex(randomblob(16))) becomes replace(gen_random_uuid()::text, '-', ''),
the same 32 lowercase hex characters with no extension to install.
INSERT OR IGNORE becomes ON CONFLICT DO NOTHING.
- Downs that are no-ops on SQLite stay no-ops here even where Postgres could drop
the column. A down that lands on a different schema per engine is worse than one
that lands on none, and it would make the following up fail on one engine only.
Tests are opt-in on CALNODE_TEST_POSTGRES_DSN and skip cleanly when it is unset,
so upstream CI and any contributor running SQLite is unaffected. Each test runs in
a schema of its own, created and dropped around it, so runs neither collide nor
leave anything behind in an operator's database.
Every other cross-engine test asserts that one specific thing survived the translation. This one asserts that nothing else changed: it migrates SQLite and Postgres to 57 in the same process and compares the table list and each table's column names, so a column quietly dropped, renamed or added in one set fails here rather than in whichever handler reads it. Names only, deliberately. TEXT versus text and SMALLINT versus integer are the translation doing its job, and the types that do matter are already pinned by TestPostgres_flagColumnsStayIntegers. The table-count floor is there because an empty map on either side would satisfy every comparison in the test.
The dialect-aware wrapper rebinds ? to $n on Postgres and is a no-op on SQLite,
so the only way the rest of Calnode benefits is to hold the wrapper rather than
the bare *sql.DB. Method names match database/sql, so this is a type change at
the declarations and almost nothing at the 763 call sites.
Three places did need a decision rather than a substitution:
- internal/connstore.Execer is an interface over QueryRowContext, so both the
wrapper and a bare *sql.DB satisfy it; only its doc comment changed.
destination_test.go opens the sqlite driver directly against a bespoke
fragment schema, and stays on *sql.DB for that reason.
- handler.Readyz passes h.db.DB to db.SchemaReady, which takes a bare pool.
Safe: that statement has no placeholders, so there is nothing to rebind.
- cmd/calnode moved from db.Open + db.Migrate(database) to db.OpenDB and the
handle's own Migrate method, which is what carries the dialect.
SQLite behaviour is unchanged: Rebind returns its input untouched there.
Timestamps first, because they are 38 of the 44 sites. datetime('now') and
strftime('%Y-%m-%dT%H:%M:%fZ','now') are now computed in Go and bound as
parameters, which removes the dialect difference rather than encoding it and
makes the value something a test can predict.
internal/dbtime holds the two layouts. Two, not one: the schema already stores
both shapes, and they are load-bearing. recordings.created_at is the space-
separated form and meeting_consents.decided_at the millisecond form, and
consentWindow converts between them to build a lexicographic BETWEEN; notes'
updated_at is handed to a client verbatim; jobs.run_at written by enqueueJob is
deliberately the space form, which sorts before every T-separated run_at and is
what makes those jobs due immediately. Normalising would have been tidier and
would have quietly changed all three. dbtime_test asserts both layouts against
SQLite's own output rather than against a reading of its documentation.
The rest:
- INSERT OR IGNORE becomes ON CONFLICT DO NOTHING with no conflict target,
which both engines accept and which keeps OR IGNORE's "any unique
constraint" scope.
- COLLATE NOCASE becomes LOWER(col) = LOWER(?). No dialect pair, because
nothing indexes booking_attendees.email: there is no collation for an index
to depend on, so there is no plan to preserve.
- demo.Reset's PRAGMA foreign_keys is SQLite-only now. Postgres has no
equivalent without superuser rights, so that branch wipes with a single
TRUNCATE ... CASCADE over every table, which needs no delete ordering at
all. sqlite_master becomes pg_tables scoped to current_schema(), the one
Dialect.SQL pair added here — current_schema() also keeps it right inside an
isolated test schema.
RETURNING is left alone: both engines support it. No UPDATE or DELETE carries a
LIMIT; that was checked by scanning every backquoted string in the tree rather
than grepping single lines.
SQLite behaviour is unchanged, stored bytes included.
ARCHITECTURE §17 says the app-level double-booking check is free of TOCTOU races because SetMaxOpenConns(1) serialises every transaction. That is true and it is a SQLite property. On a Postgres pool two overlapping bookings can both read "host is free" and both insert, and idx_bookings_no_double is UNIQUE(host_id, start_at) so it only catches an IDENTICAL start time — 10:00-10:30 against 10:15-10:45 are two distinct keys and both inserts satisfy it. lockHosts takes pg_advisory_xact_lock on every host whose availability the transaction is about to decide, before the first hostBusy read. The lock is released by the transaction ending, so there is no unlock to forget and no leak down the several early-return paths these functions have. Per host rather than global, so bookings for different hosts stay concurrent — otherwise this would just reinstate the single writer it replaces. On SQLite it returns immediately and nothing changes. The key is SHA-256 of "calnode:booking:host:" + id, first eight bytes big-endian as int64. Derived in Go rather than with hashtext() so it is readable and testable from Go, and domain-separated so a later advisory lock on some other entity cannot collide by hashing the same raw id. Ids are sorted and deduplicated on a COPY before locking: two transactions needing the same two hosts in opposite orders would deadlock, and Create's HostIDs arrive in round-robin priority order, which decides who gets the booking. Three call sites, where the packet named two. ReassignHost has the identical check-then-write shape — hostBusy's own doc comment names all three write paths — and leaving it out would have left a known double-booking hole. Reschedule locks the primary host BEFORE reading booking_hosts rather than after, because a concurrent ReassignHost holds that same key, and that ordering is what stops a reassignment committing between the host-list read and the UPDATE. isUniqueViolation now matches SQLSTATE 23505 as well as SQLite's message text. It was a substring match on "UNIQUE constraint failed", so on Postgres the index backstop would have surfaced as a 500 rather than ErrDoubleBooked. SQLSTATE because the message is localised by the server's lc_messages. Measured against PostgreSQL 17, 40 rounds of two goroutines racing overlapping slots: with the lock, 40 created / 40 conflicts / 0 overlapping pairs in the database. With the lock disabled, 79 created / 1 conflict / 39 overlapping pairs. The unlocked run is why the index cannot be described as a sufficient guard: it caught one race in forty. internal/dbtest arrives with this rather than with the docs commit, because the committed version of that test needs it. Each test gets its own calnode_test_<random> schema via search_path in the DSN, dropped on cleanup; the test that asserts the isolation actually reaches the server is in that package, because a search_path that silently stopped being forwarded would migrate every package into public and read as flakiness.
22 test helpers across 18 files moved from db.OpenDB("sqlite://:memory:") +
Migrate() to dbtest.Open(t), so which engine the suite exercises is an
environment variable rather than 22 hardcoded DSNs. With
CALNODE_TEST_POSTGRES_DSN unset — every local run, and the existing CI job —
nothing changes: it is the same in-memory SQLite as before.
Four helpers stay on SQLite deliberately: dbtime_test pins SQLite's own timestamp
output, hostlock_internal_test asserts the advisory lock is a no-op there,
connstore/destination_test opens the bare driver against a bespoke fragment
schema, and two health_test cases want an UNMIGRATED database. keyvault_test
keeps its shared-cache DSN, which it has for a reason.
ARCHITECTURE §4 is amended rather than extended, because a new section would have
sat next to prose asserting the opposite. The heading covers both engines; the
pool difference is stated along with the way the wrapper can be bypassed
silently; the rows-cursor gotcha is scoped to SQLite together with the reason the
materialise-first pattern has to stay regardless; and a new subsection states the
double-booking guarantee per engine — the single connection on SQLite, the
advisory lock on Postgres — plus what the partial unique index actually covers,
with the measured 39-double-bookings-without-the-lock figure. §17's first gotcha
and one now-engine-conditional aside in §8 were pulled into line with it.
The new CI job runs the Go half against a postgres:17 service. Separate job, not
a matrix, because only Go is engine-dependent and svelte-check would otherwise
run twice. It carries a pg_isready health check: without one the first connection
races the server's startup and fails as "connection refused", which reads like a
bad DSN rather than a timing problem.
That job will be RED until internal/db/migrations/postgres exists — the database
layer is another worker's file set and it has only the sqlite directory today.
Measured rather than assumed: with the DSN set, dbtest.Open reports "run
migrations: migrations/postgres directory does not exist".
Thirteen sites decided between 409, 400, 404 and 500 by substring-matching
SQLite's error text. On PostgreSQL none of them matched, so a duplicate slug, a
replayed idempotency key and a bad enum value all fell through to
{"error":"internal error"} with a 500. Seven Postgres test failures were this.
internal/db now exports IsUniqueViolation, IsCheckViolation and
IsForeignKeyViolation. PostgreSQL is matched on SQLSTATE (23505 / 23514 / 23503)
because the message is localised by the server's lc_messages, so a server running
in a non-English locale would defeat any text match no matter how carefully it was
written. SQLite keeps a text match, because modernc.org/sqlite does not expose a
code on its error value — but it is now in exactly one place instead of thirteen,
and it is pinned by a test.
A *pgconn.PgError whose code does not match returns false rather than falling
through to the text comparison. Falling through would classify a PostgreSQL error
by whether its message happened to contain another engine's English phrase, which
is the fragility being removed.
The test provokes a REAL violation of each class against the real migrated schema
on whichever engine dbtest is configured for — a duplicate users.email, a booking
with a status the CHECK forbids, a booking referencing a nonexistent event type —
rather than constructing an error value, because a constructed error only proves
the predicate agrees with what the author believed the driver returns, and that
belief being wrong is why this commit exists. It also asserts exactly ONE of the
three predicates matches each violation, and that a missing-table error and a
plain errors.New match none: a predicate that answered true for everything would
have made all thirteen call sites pass and been badly wrong.
internal/booking and internal/handler keep thin local wrappers where a local name
read better at the call site; their bodies now delegate.
grep -rn "constraint failed" --include='*.go' internal/ cmd/ returns three hits,
all of them the constants inside internal/db/constraint.go.
With the constraint predicates in, eight Postgres failures were left, and they were
four unrelated causes rather than one.
Computed booleans. "(user_id = ?) AS owned" and "(archived_at IS NOT NULL) AS
archived" are 0/1 on SQLite and a boolean on PostgreSQL, and the boolean does not
scan into the int the real 0/1 columns beside them use — "converting driver.Value
type bool to a int". Both are CASE WHEN ... THEN 1 ELSE 0 END now: portable, and
the same 0/1 convention the schema uses for stored flags, so the != 0 idiom in the
scanner is untouched.
json_extract in production. replaceReminderJobs deleted a booking's reminder jobs
by filtering on json_extract(payload, '$.booking_id'), a SQLite JSON1 function. On
PostgreSQL the DELETE errored, so rescheduling silently left the old reminder in
place — a real bug, not just a red test. No portable spelling exists, so this is a
Dialect.SQL pair: payload::json ->> 'booking_id', with the cast because the column
is TEXT rather than json.
ORDER BY rowid. webhook_deliveries had no timestamp of its own, so "the 50 most
recent" was rowid DESC. PostgreSQL has no rowid, and the query was not strictly
correct on SQLite either: rowid tracks insertion order only until something
renumbers it, and VACUUM may. Migration 00058 adds created_at to both dirs, the
writer binds it, and the order is created_at DESC, id DESC — the tiebreak because
two deliveries written in the same millisecond would otherwise come back in
whatever order the engine felt like. The default is a constant '' rather than a
timestamp expression because SQLite's ALTER TABLE ADD COLUMN forbids a
parenthesised DEFAULT; rows predating the migration sort last, which is where the
oldest deliveries belong.
randomblob in a gcal test helper: SQLite's id generator, swapped for uid.New(),
which is what every other row in those tests already used.
Two test changes, neither of them an assertion:
- reschedule_test held its own copy of the json_extract expression AND discarded
the Scan error, so "function json_extract does not exist" surfaced two seconds
later as time.Parse failing on "". It now uses the same dialect pair and
reports the query error, so the next engine difference names itself.
- postgres_test pinned the migration count as the literal 57 in two places. It is
now one named constant at 58, so adding a migration is a single edit that
cannot be half-done.
go test -count=1 ./... is green on both engines from this commit: 28 of 28 packages
on SQLite, 28 of 28 on PostgreSQL.
The comment in constraint.go claimed modernc.org/sqlite exposes no error codes. That was wrong. It defines type Error with Code(), populated for every constraint class, and the codes are SQLite's extended result codes. Measured against a real in-memory database rather than read off a doc page: UNIQUE 2067, PRIMARY KEY 1555, CHECK 275, FOREIGN KEY 787, NOT NULL 1299 ⛔ The reason this is worth a commit rather than a tidy-up: a PRIMARY KEY collision reports 1555, NOT 2067, while still saying "UNIQUE constraint failed" in its message. The text match caught both by accident. Switching to Code() == 2067 alone would silently stop recognising primary-key collisions — and Calnode has one that matters: idempotency_keys.idempotency_key is a bare PRIMARY KEY, so every idempotent replay arrives as 1555. Verified by removing 1555 and re-running: the new subtest fails naming the code and the table, and TestCreateBooking_idempotentReplay goes to "replay: 500". So the regression was real, not theoretical. IsUniqueViolation therefore matches BOTH 2067 and 1555. PostgreSQL needs no equivalent change: a primary-key collision is 23505 like any other unique violation, which is why the trap exists on only one side. Constants are named after the SQLite symbols so the numbers are greppable. The text comparison stays as a fallback, and only as a fallback. It covers an error that arrives without its concrete driver type attached — a driver release that changes the type, a layer that reformats rather than wraps — where the message is the only signal left and answering from it beats returning a 500. TestConstraintTextFallback exercises that branch directly, so it is not unexecuted code that reads like an accident. A *pgconn.PgError or a *sqlite.Error whose code does not match is a DEFINITE no and does not fall through: falling through would reintroduce this very trap in reverse, readmitting a 1555 by its message after excluding it by code. Also fixes a flake in the dbtest harness, which is what the two intermittent handler failures under `go test ./...` actually were — not an application fault. Calnode's handlers do several things fire-and-forget (notify hosts, enqueue webhook, enqueue reminders), those goroutines outlive the test body, and closing the pool does not stop an in-flight statement. DROP SCHEMA CASCADE needs an exclusive lock on every object, meets them, and PostgreSQL reports "deadlock detected" (40P01) — visible only under the full run, where packages compete and everything is slower, in a different pair of tests each time. The drop now runs on a pinned connection with lock_timeout set so an attempt fails fast instead of deadlocking, and retries within a bounded budget; a schema that still cannot be dropped is reported, because a leaked schema accumulates on a shared server.
Includes the measured code table, the primary-key trap and how it was proved, and the dbtest teardown deadlock that the two intermittent handler failures actually were.
Timestamps are TEXT and compared lexicographically on purpose: the worker
claims with run_at <= ?, sessions and tokens expire on expires_at > ?, the
consent window brackets decided_at, booking overlap compares start_at/end_at
against bound strings, and several lists are ORDER BY created_at. On SQLite
that is memcmp. On Postgres it is the column's collation, which by default is
the database's.
Migration 00059 alters 54 columns across 27 tables to COLLATE "C" on Postgres
(every *_at, jobs.locked_until, and the availability HH:MM / YYYY-MM-DD
columns, which are ordered as times too). Fixed in the schema rather than in
40-odd predicates that would each have to remember a COLLATE clause. The
SQLite half is a no-op file: BINARY is already memcmp, and the file exists so
the two directories keep one file per version.
internal/handler/notetaker.go is the site that makes this load-bearing rather
than theoretical: it writes datetime('now')'s space-separated shape precisely
because it sorts before any T-separated stamp, which is what makes a notetaker
job due immediately.
collation_test.go holds it three ways:
- an audit over information_schema.columns, matching by NAME so a timestamp
column added by a later migration is caught without anyone remembering;
- a positive control that skips, rather than passing vacuously, if the server's
own default already orders byte-wise;
- an ordering test on jobs.run_at through the worker's real claim predicate.
Measured, not assumed: the two shapes the schema actually stores do NOT flip
under this server's en_US.utf8 default, nor under any of the other 878
collations installed on it - glibc ignores the space at the primary level but
still sorts a digit before 'T'. The control therefore also carries RFC 3339's
lower-case t/z spelling (permitted by RFC 3339 section 5.6, so an importer can
hand it to us), where en_US.utf8 puts lower case first and memcmp puts upper
case first. Proved failable by dropping jobs.run_at from the migration: the
audit reports "jobs.run_at = <database default>" and the ordering test reports
the two spellings transposed.
Boundary 2 ported the tree's one RETURNING clause (question_handler.go's auto-position INSERT) and left it verified on SQLite and unverified on PostgreSQL. It runs unchanged on both: no d.SQL pair, no rewrite. The test goes through the HTTP handler on dbtest.Open(t), so it follows CALNODE_TEST_POSTGRES_DSN like the rest of the suite, and it asserts the value the handler SCANNED from RETURNING as well as the row left behind - a RETURNING that quietly produced a zero would still leave a correct row, so checking the table alone (which TestCreateQuestion_autoPosition already did) cannot see it. The explicit-position case is in the same test on purpose: after a pinned position 9, the next auto position must be 10, which is what shows the COALESCE(MAX(position)+1, 0) subselect inside VALUES is really evaluated by the engine rather than the sequence being an artifact of insertion order. Measured: 0, 1, 2, then 9 explicit, then 10, identical on sqlite and postgres.
Open("...") returned a plain *sql.DB and was documented as being "for callers
that have not moved to OpenDB yet". Nothing outside the package's own tests
used it, and it could only ever do harm: a statement issued through that handle
is not rebound, so every ? in it is a Postgres syntax error discovered at
runtime, a long way from the call. It was also the shape most likely to be
copied by the next person adding a call site.
/readyz was the one production caller reaching into the exported embedded field
(db.SchemaReady(ctx, h.db.DB)). *DB now carries SchemaReady and AppliedVersion,
so the handler asks the handle. The package-level functions stay for the
bare-pool cases that really do need one - goose's own bookkeeping, and the
tests that open an unmigrated pool - and db_test.go passes database.DB to them
explicitly, which says at the call site that the bare pool is deliberate.
No test asserts the absence of the symbol: the build is the proof, and a
reflective check on a package's exported names is not cheap enough to be worth
the fixture. TestOpen_inMemory is renamed TestOpenDB_inMemory rather than left
named after something that no longer exists.
SetMaxOpenConns(10) / SetMaxIdleConns(5) were literals in openPostgres. The number that fits is a property of the server, not of Calnode: PostgreSQL's max_connections is shared with every other client, and an instance behind PgBouncer wants a different figure from one talking to a 200-connection server directly. DB_MAX_OPEN_CONNS and DB_MAX_IDLE_CONNS now carry it, defaults unchanged at 10/5. Validation lives in config.PoolFromEnv: unset, unparsable or non-positive falls back to the default with a warning (matching getBool/getDuration, which also refuse to fail a boot over a typo in an optional knob), and an idle limit above the open limit is clamped - database/sql silently reduces it anyway, so the clamped pair is the honest description of what the pool will do. The clamp applies to the DEFAULT idle too, which is the case a test pins: DB_MAX_OPEN_CONNS=2 alone must give 2/2, not 2/5. OpenDB reads PoolFromEnv itself rather than taking the numbers as an argument, so every entry point (server, mcp, rotate_key, recover_key, reset_admin) picks them up without five identical edits and without one of them silently keeping the defaults. db -> config is not a cycle; config imports only the standard library. A caller that must not follow the environment passes db.WithPool. SQLite stays pinned at 1/1 and ignores both the environment and WithPool. That is a correctness guarantee, not a preference: the single connection is what serialises write transactions (it is why booking.lockHosts is a no-op there), and the pragmas are connection-scoped, so a second connection would not have them. TestOpenDB_sqlitePoolIsNotConfigurable asserts it with both knobs set to 40/20, for both the :memory: and file forms. The idle half cannot be read back from database/sql (DBStats exposes the open limit only), so it is measured instead: four live transactions force four connections, and after committing them all the pool keeps 1 with WithPool(4, 1). Measured on the live server, "idle after release 1".
GET /v1/auth/sso?token=<jwt> takes a short-lived HS256 token from an external identity system that has already authenticated the person and starts an ordinary Calnode session, so they are not asked to log in a second time. Off unless CALNODE_SSO_SHARED_SECRET is set, and 404 when it is not: an instance that has not configured this should be indistinguishable from one that does not implement it, and a feature that can mint a session must not be reachable by default. The token is verified in-tree with crypto/hmac rather than by a JWT library, for the reason internal/livekit signs its own: one algorithm, one key, a fixed claim set, nothing to keep current. HS256 is checked before the signature is compared, because accepting the token's own choice of algorithm is the alg:none downgrade. Two properties carry the security, and both are cheap only because the token is a redirect the browser follows immediately. It may live at most 60s after iat (30s of skew either way, since the two systems are separate hosts), and its jti is claimed in the new sso_nonces table BEFORE the session is created, so a replay inside the window collides on a primary key instead of racing a read-then-write. The worker purges expired nonces in the GC pass it already runs; a row past its expires_at can no longer refuse anything, so keeping it is pure growth. aud must equal BASE_URL. That is what stops a token minted for staging being spent on production when someone shares a secret between the two by mistake. This is the one path that creates a user without an invite, and that is the trade the shared secret buys. On creation the claimed role is applied. On someone who already exists it is not: a workspace's roles are the workspace's business, and a hand-off rewriting them on every sign-in would make the admin UI's role controls advisory. The single exception is bootstrapping an instance with no owner, where the one-owner invariant means there is nothing to displace. An archived account is refused here as it is everywhere else. ?next= is honoured only for a same-origin absolute path, and a bad one is refused rather than cleaned up, because a redirect built from a partially sanitised value is how open redirects survive their own fix. It is also checked before the nonce is claimed, so the caller's own bug does not burn the token. wid is parsed and ignored. A multi-workspace mode will use it to choose which workspace the hand-off lands in; accepting it now means a caller written against that does not need changing to work today.
POST /v1/auth/sessions/revoke-all. With no body it drops every session the
caller has except the one that made the request, which is the action people
actually want when a laptop goes missing: Logout already ends the current
session, and a 30-day cookie on a machine you no longer hold is the thing with
no answer. An API-key caller has no current session, so for them every row goes.
With {"user_id": "..."} it becomes an offboarding tool, on the same tiers
roles.go already enforces: an admin may revoke a member, only the owner may
revoke another admin, and the owner's sessions can be ended only by the owner
(there is exactly one owner, so that is the self branch). The actor's tier is
checked before the target is loaded, so this endpoint's 404 cannot be used to
enumerate user ids.
It also deletes the target's oauth_access_tokens rows. That is what makes it an
offboarding tool rather than a convenience: an MCP connector authenticates with a
bearer token, not the session cookie, so revoking sessions alone would leave an
agent connected with exactly the authority just withdrawn. Both deletes share one
transaction, because a caller told "revoked" must not keep a token on account of
the second statement failing after the first committed.
The four remaining items and what was measured for each: 54 columns across 27 tables pinned to COLLATE "C", the server collation the control fired against (en_US.utf8, libc) and the correction that the two shapes the schema really stores do not flip under any of the 878 collations installed there, RETURNING position giving 0/1/2 then 9 then 10 identically on both engines, and the pool keeping 1 idle connection with WithPool(4, 1). Open item 2 is marked closed in place rather than deleted, since Boundary 6 is where it was raised.
Rate limits key on the TCP peer, and that is correct for a directly reachable instance and useless behind a fronting CDN, where every visitor arrives from the same few addresses and shares one bucket. TRUSTED_PROXY_CIDRS opts in per network: for a peer inside one of them the client IP comes from CF-Connecting-IP, else from X-Forwarded-For, else the peer. The default does not move, and cannot be moved by a header. A peer that is not in the list has its headers ignored before they are read, so an instance that sets nothing behaves exactly as it did. Two details are the difference between this and a spoofable version: The X-Forwarded-For walk goes RIGHT TO LEFT past trusted hops and returns the first untrusted address, not the leftmost entry. The left of that header is whatever the original client sent, and every well-behaved proxy prepends to it and preserves it, so "leftmost" is a value the client picks. The rightmost non-trusted hop is the last address one of our own proxies actually observed. A hop that does not parse ends the walk and falls back to the peer instead of being skipped. Skipping would let a client inject one malformed entry to push the walk past the real hop onto a value it chose. The cost is that a proxy appending "ip:port" reads as malformed and lands on the peer, which is the safe direction. Resolution is a middleware rather than a package-level setting, computed once in the outermost layer and carried in the request context: a mutable global would leak between server instances in a test binary and would make "which requests are affected" unanswerable from the wiring. remoteIP reads the context when it is there, so the untrusted path is byte-for-byte the old function. A CIDR that does not parse is logged and dropped rather than fatal, and the other entries survive it. The consequence of dropping one is that that hop's headers are not believed, which costs shared rate-limit buckets and never a trusted forgery. audit/claims.yaml's rate-limit-keys-on-tcp-source-address said the headers are "never read", which stops being true the moment this ships. Rewritten to state the condition rather than left to rot into a false claim.
FRAME_ANCESTORS (space-separated, so it reads the same in the env var as in the header) puts Content-Security-Policy: frame-ancestors <list> on the handler under /admin/. Wanted by anyone embedding the console in their own tooling; the alternative today is a reverse proxy rewriting headers. The middleware wraps frontend.Handler() and nothing else. The public booking pages set frame-ancestors 'none' and X-Frame-Options: DENY in their own handlers and must keep doing so unconditionally: they are unauthenticated pages that collect names, emails and card details, and clickjacking one of those is worth more to an attacker than framing a console nobody can reach without a session. No X-Frame-Options is sent beside the CSP. That header has no allow-list form (ALLOW-FROM was implemented by one browser and is dead), so the only value it could carry here is SAMEORIGIN, which the browsers that read it apply INSTEAD of honouring frame-ancestors, breaking the embedding this exists to enable. Unset changes nothing, and what "nothing" is here is worth writing down: /admin/ sends no frame header at all today, so the SPA is framable by default. This does not add a default deny, because an opt-in setting must not smuggle in a behaviour change, and TestAdminSPA_sendsNoFrameHeadersWhenUnset pins the current answer so that the next person to change it does so on purpose. An entry that is not https://host[:port] or 'self' fails config.Validate and the process exits rather than starting. A browser drops a source list it cannot parse, so a typo would leave /admin/ MORE embeddable than the setting unset, with nothing in the response to say so. Wildcards are refused too even though CSP allows them: https://*.example.com trusts every host that name ever points at, including one taken over later, and an operator who needs two hosts can name two hosts.
GET /metrics exposes build identity, requests by surface and status, a duration histogram, job-queue depth, booking lifecycle counts, process start time and two Go runtime gauges. internal/metrics writes the exposition by hand: the format is one page of stable text, and a scrape endpoint is not worth a dependency tree in a binary an operator self-hosts. Same trade as hand-signing LiveKit tokens. It answers 404, not 401, when METRICS_TOKEN is unset or the bearer is wrong, and the body is byte-identical to the mux's own not-found. A 401 confirms the endpoint exists and invites a guess, and what is behind it is a business feed: bookings per hour, request volume by surface, queue depth, on an instance whose whole point is being publicly reachable. An operator who has not set a token has not agreed to publish any of that, so there is nothing to advertise. The token compare is over SHA-256 digests rather than the raw strings, because ConstantTimeCompare returns early on a length mismatch and would otherwise leak the token's length. No rate limit: a scrape runs every few seconds by design, and a limiter tuned for humans would drop samples and leave gaps that read as downtime. The class label is derived from the path PREFIX only, so the label set is closed at five values and a request cannot mint a new series. That is the usual way a metrics endpoint turns into an out-of-memory vector. It costs some precision and the docs say so: POST /v1/bookings is public and unauthenticated and still counts as api, because the alternative is a per-route table that goes stale the first time someone adds a route without noticing. Requests are counted in the existing Logging middleware rather than a wrapper of their own: that is already the one place holding the final status and the elapsed time, and a second measurement of the same request would drift from the log line it is supposed to corroborate. Job depth is read from the jobs table per scrape, because any instance can claim any job, so it is not this process's counter to keep. Booking counts sit in the three shared side-effect functions, which covers the REST, MCP and manage-link paths at once, and they are incremented next to the webhook enqueue but not inside its nil check: an instance with no webhooks configured still has bookings worth counting. A host reassignment is deliberately not counted as a reschedule, even though it does fire booking.rescheduled, because it does not move the meeting and counting it would make the series answer a question nobody asked. All three booking series are emitted at zero rather than appearing on first use. A series that springs into existence makes rate() over a quiet window return nothing instead of 0, which a dashboard renders as "no data" rather than "nothing happened".
The Deepgram host was a hardcoded constant, so meeting audio always went to the provider's global endpoint. STT_BASE_URL points it elsewhere: a regional endpoint, so recordings are transcribed inside one jurisdiction, or a self-hosted deployment of the same API. The default is unchanged and a test pins the exact URL, query included, because a dropped diarize=true returns a transcript with no speaker labels rather than an error. Only the host is configurable. The path, model and options stay in listenPath, so an operator picks a region and not a different request; a trailing slash is trimmed so both spellings of a host behave the same. The base URL is a NewDeepgram parameter rather than a package variable, because configuration reaching a client through a global is configuration nobody can see at the call site. GET /v1/settings/notetaker now reports the effective value as stt_base_url, resolved on read so a Handler built without the setter still names the real default instead of an empty string. It is read-only and env-only, unlike the API key beside it: an admin should be able to see where recording audio is sent without shelling into a container, and should not be able to repoint it from a browser session.
Reminders were email-only, so an integration could hear that a booking was made, moved or cancelled but not that the nudge before it had been sent. reminder.send now enqueues booking.reminder, booking-shaped like its siblings plus hours_before, which the job payload has always carried: an event type can configure several reminders, so a subscriber that cannot tell 24h from 1h cannot act on the event. Fired after the email and only when the email succeeded. The event means "the attendee has been reminded", so emitting it beside a failed send would say something untrue, and the job retries, which would deliver it twice for one reminder. The mirror image also matters: an enqueue failure does NOT fail the job, because the email has already gone and a retry would send a second one. Logged and dropped, the trade every webhook enqueue after a committed side effect makes here. sendReminder's existing early returns are all "no reminder happened" - booking deleted, no longer confirmed, host has reminder emails off - so none of them reaches this either. A test pins the suppressed-email case, since "a job ran" and "someone was reminded" are easy to conflate later. The payload needs the BOOKING's host_id, not the event type's owner: a rotation or a reassignment moves it, and Enqueue selects a subscriber's webhooks by that id. The query gained b.host_id and b.created_at for this and nothing else. hours_before goes through the same per-webhook field selection as everything else rather than being bolted onto the envelope, so it appears in the admin field list and an operator can decline it. It is in defaultFields on the same bargain as the payment fields: omitted at zero, so every other event's payload is byte-identical to before, and a webhook created before this event existed is unchanged. Payment fields are deliberately absent from this event. paymentStatusForWebhook's mapping lives in the handler package, and a reminder is a time notification; payment state is what create and cancel are for. The SPA's event list and field catalog gain the two strings. The embedded build is not rebuilt here, so the admin UI will not offer booking.reminder until someone runs pnpm build - the API accepts it now either way.
A visitor asking for fr-CA got the France copy. fr-CA.json is the first regional locale here, and it is a file rather than a fallback because the differences are real rather than cosmetic: - courriel, not e-mail. The OQLF rejects the France-ism and it reads as foreign. - reporter / report, not reprogrammer / reprogrammation. - renseignements personnels, never données personnelles. That is the statutory term in Quebec, so it is the one a reader expects to see. - no space before ! ? ;, where France puts one. The space before : stays. - conflit d'horaire rather than conflit d'agenda. - month_short_jul is juill., against juil. in France. CLDR itself disagrees on exactly this one abbreviation, which is what TestDateTablesMatchCLDR is for; had the file been a copy of fr it would have failed on that line and nothing else. The 24-hour clock and the day-month date_format are shared with fr. Canadian French also puts a non-breaking space before $ and %, and no key in the file carries either today, so that rule is written into ARCHITECTURE §23 rather than applied to nothing. Generated from fr.json with explicit per-key overrides and verified to have the same keys, the same key order and the same printf verbs, which is what the three guards check. Every other value is inherited verbatim, so a later correction to fr does not silently diverge here. TestResolve gains four cases, and two of them are the ones that matter: fr and fr-FR must be unaffected by fr-CA existing, because that is the half that would break silently for every existing French visitor. The wording is an unreviewed LLM draft, as every non-English locale in this repository is. Structure is verified, the copy is not, and the CHANGELOG says so before anyone markets the language.
postgres_test.go asserts the fully-migrated version twice through this constant, and its own doc comment says it moves with every migration added. This branch adds one, so it is 59. Worth knowing for the next person: those two tests skip when CALNODE_TEST_POSTGRES_DSN is unset, so a migration added and verified on the SQLite lane alone leaves this red and nothing says so until someone runs the Postgres lane.
Boundary 1 of multi-tenant mode. Migration 00060 in both dirs adds
`workspaces`, a `workspace_id` on all 32 application tables, the composite
uniqueness that was global, and one row-level-security policy per table.
`MULTI_TENANT` / `DATABASE_ADMIN_URL` / `CALNODE_PLATFORM_TOKEN` arrive with a
`Validate` that refuses the five combinations that cannot work.
Numbered 00060, not the packet's 00061: `feat/platform-hooks` is not in this
branch, the 59 existing files are contiguous, and migrations_internal_test.go
asserts target version == file count.
ENABLE / FORCE ROW LEVEL SECURITY is deliberately NOT in the migration. FORCE
applies a policy to the table's owner, and in single-tenant mode DATABASE_URL is
the owner: measured on PostgreSQL 17.11 with a NOBYPASSRLS owner and one row,
ENABLE+FORCE with no binding returns 0 rows, ENABLE alone returns 1, and a
non-owner app role under ENABLE alone already returns 0. A superuser DSN hides
all of it, and the suite's DSN is a superuser, so an unconditional FORCE would
have gone green while blinding every real single-tenant deployment. The policies
stay in the migration as reviewable SQL (a policy is inert while RLS is off,
verified); the two ALTERs move to db.EnableRLS, which runs at boot only under
MULTI_TENANT, on the platform handle, idempotently, and exits 1 on failure.
The column default is COALESCE(current_setting('app.workspace_id', true),
'default') rather than the bare current_setting, which RAISES when unset and
would fail every single-tenant INSERT. It fails closed the other way too: an
unbound multi-tenant write would be written as 'default' and the policy's WITH
CHECK refuses it with 42501 instead of landing it in the wrong tenant.
rls_proof_test.go is the gate. It creates a NOBYPASSRLS calnode_app_<hex> role
per test schema and skips LOUDLY if it cannot, or if the role turns out to
bypass, or owns a table. Unbound reads 0 of 2 and its INSERT is refused with
nothing left in 'default'; bound to ws-a it reads 1, inserts naming no
workspace_id and lands as ws-a, cannot see ws-b, and is refused 42501 when it
names ws-b. Proved failable by stubbing EnableRLS: all four subtests fail.
jobs keeps workspace-free copies of its two partial indexes. It is the one table
worked across tenants, and the worker's claim orders by run_at globally, which a
workspace-leading index cannot serve.
SQLite gets the same columns with a literal default and no foreign key: ADD
COLUMN rejects a REFERENCES column with a non-NULL default outright. Only
idempotency_keys and meeting_consents are rebuilt, for their primary keys; with
one workspace a global unique and a (workspace_id, x) unique admit the same rows.
demo.Reset now holds workspaces back with goose_db_version. Truncating the tenant
root took the 'default' row with it and the re-seed failed on the foreign key.
gofmt -l empty, go vet clean, go test ./... rc=0 on SQLite (28/28) and rc=0 on
PostgreSQL (28/28). Negative control with the password changed to wrong: the new
tests FAIL rather than skip, with "failed SASL auth: FATAL: password
authentication failed for user \"postgres\" (SQLSTATE 28P01)".
…nection Boundary 2. OpenPair(appURL, adminURL) returns the application handle and the platform handle; app.Platform() answers with the second, so nothing downstream carries both. ForWorkspace(id) returns a cheap value sharing the pool, which is what makes it safe in the fire-and-forget goroutines this codebase is full of: it holds a pool and a string, not a session. A test builds a handle inside a function, lets that function return, and reads through it from four goroutines. Each statement on a bound handle takes a pooled connection, sets app.workspace_id on it, runs there, and gives it back — Exec at once, Row on Scan/Err, Rows on Close, Tx on Commit/Rollback, the last idempotently because `defer tx.Rollback()` after a successful Commit is the pattern here. Begin uses SET LOCAL, so the connection returns carrying nothing. Because every statement sets the parameter itself, a value left on a pooled connection cannot leak into a later one. The release is asserted with Stats().InUse returning to 0 after every shape, and with a positive control on the same number: 1 while a cursor or transaction is open. Proved failable by skipping the release — InUse climbs to 2, 3, 4 and the "with a cursor open" control fails too. Prepare is refused on a bound handle. A *sql.Stmt is re-prepared on an arbitrary pooled connection with no hook to bind first, so it would run unbound, which is silently empty rather than an error. Nothing in the tree prepares a statement. VerifyRoles is new, because D4 was otherwise documentation only. Both halves of a bad configuration are silent: an application role that is superuser, has BYPASSRLS, or owns any table in the schema is not constrained by the policies and can read every workspace while nothing appears wrong; a platform role that does not bypass makes its '' binding match no row, so the worker claims nothing and the reconciler enumerates nothing. It runs at boot after EnableRLS and the boot exits 1 if it fails. It is also what makes binding '' on the platform handle safe rather than a gamble. connstore.Execer now returns *db.Row. Go has no covariant return types and the interface is satisfied by both *db.DB and *db.Tx, so it had to move with them. destination_test.go's deliberately-bare sql.Open therefore becomes db.OpenDB — a correction rather than a concession, since a bare *sql.DB does not rebind placeholders either. The compiler found the only three other sites: two *sql.Rows declarations in internal/handler and one test helper. Boot opens the pair under MULTI_TENANT and one handle otherwise, migrates and enables RLS on the platform handle, and verifies roles on the application one. gofmt -l empty, go vet clean, go test ./... rc=0 on SQLite (28/28) and rc=0 on PostgreSQL (28/28).
…t per request
Boundary 3, part one of two. This lands the foundation; the 163 mux registrations
in internal/server/server.go, the route-classification test, the five end-to-end
request tests and the /v1/bookings/{id} tenant check follow in part two.
Handler was 30 fields including six sync.RWMutex. Giving 314 methods a
tenant-bound h.db without editing any of them means handing them a receiver that
differs in that field, so Handler is copied per request — and a struct holding a
mutex cannot be copied. So `shared` holds everything the process has one of and
Handler is { *shared; db; ws; bookingSvc; webhookSvc }. Embedding as *shared
rather than naming it is what keeps h.logger, h.livekitMu and h.mailer = m
compiling: the split needed zero edits across the 61 files of the package, and go
vet copylocks is clean. bookingSvc and webhookSvc each wrap a *db.DB, so
forWorkspace rebuilds them through new (*Service).ForDB methods.
RequireAuth's api_keys and sessions reads move to the platform handle. On the
application handle they run bound to the workspace of the request, and the
workspace of the request is what they exist to discover — so they would find
nothing and report a valid API key invalid. Both now select u.workspace_id into
the new AuthUser.WorkspaceID. This is why D9 keeps those two uniques global.
Four refusals, none of which is "carry on unscoped": unknown host 404, suspended
503, mismatch 403 {"error":"workspace mismatch"}, unresolved 500. There is
deliberately no fallback to the default workspace on an unrecognised host — that
would serve one tenant's booking page on any domain pointed at the instance — and
00060 seeds public_host empty for `default` so no request can land on it either.
Proved failable by making HostWorkspace fall back: three assertions catch it,
including "the method ran 1 times; wantReach=false".
Scoped takes a method EXPRESSION, (*Handler).Method. A bound method value would
capture the unscoped receiver, which is the bug this wrapper exists to prevent,
and it would compile silently. Platform(method) is the sibling for routes that
belong to no workspace, so "unscoped, on purpose" is stated at registration
rather than implied by omission.
publicURL() returns https://<ws.public_host> in multi-tenant mode and ignores
PUBLIC_BASE_URL entirely; single-tenant is unchanged.
Not implemented and not faked, both blocked on feat/platform-hooks being absent
from this clone: the OAuth callback SSO hand-off (D11) and the
(workspace_id, client_ip) rate-limit keys (D14). Each is a TODO(integration) in
PROGRESS.md naming the function expected. Noted with them: the platform /metrics
endpoint reads the jobs table and must use Platform() at integration, since jobs
is a tenant table.
gofmt -l empty, go vet clean, go test ./... rc=0 on SQLite (28/28) and rc=0 on
PostgreSQL (28/28).
Boundary 3, part two. 160 registrations rewritten through Scoped/Platform, the
MCP mount scoped per workspace, and a gate that fails on a route added without a
wrapper. The end-to-end request tests through a real OpenPair are the one thing
part two still owes and land next, before B4.
169 routes: 31 host-scoped, 106 credential-scoped, 24 platform, 8 allowlisted.
type H = handler.Handler so a registration reads (*H).ListBookings. The brevity
is incidental; the method expression is the point. Scoped takes
func(*Handler, http.ResponseWriter, *http.Request), so passing h.ListBookings — a
bound value on the unscoped handler, the exact bug Scoped prevents — does not
compile. Credential routes are RequireAuth(Scoped(CredentialWorkspace, ...)) in
that order, since CredentialWorkspace reads the caller from context; a test checks
the nesting on all 106.
The gate reads server.go rather than walking the mux, because http.ServeMux
exposes no way to enumerate patterns and the thing to catch is a registration
written without a wrapper, which is a property of the text. Three guards against a
vacuous pass: a floor of 150 registrations, a floor of 3 per bucket, and a check
that every allowlist entry is still a live route. The allowlist's 8 entries are
all handlers that are not *handler.Handler methods: two empty CORS preflights, the
embedded favicon/SPA/redirects, the MCP mount. A second test pins the 24-member
platform set exactly against D11 with a reason per group, so a route moving on or
off the identity host cannot happen silently.
The gate earned itself on its first run: POST /v1/livekit/egress-webhook carries a
trailing // legacy alias comment, so the scripted rewrite's line pattern missed it
and it stayed unwrapped.
The MCP tools close over their handler, so the single server instance built at
boot would have run every workspace's tool calls on whichever handler built it.
/mcp is on the identity host and carries no tenant Host, so the credential is the
only source: MCPCallerMiddleware now also reads users.workspace_id (through the
platform handle, same reasoning as RequireAuth) and the factory becomes
MCPServerForRequest, which caches one server per workspace on shared. One entry
keyed "" in single-tenant mode, so that path is unchanged behaviour with a map in
front of it.
GET /v1/bookings/{id} is now host-scoped, so a booking id from another workspace
is not visible and the 404 comes from the policy rather than a predicate the
handler has to remember. It still has no auth middleware, alone among its
siblings; that is reported, not changed — adding auth to a route the booking page
may depend on is a product decision. What changes is the blast radius.
gofmt -l empty, go vet clean, go test ./... rc=0 on SQLite (28/28) and rc=0 on
PostgreSQL (28/28). Negative control, unwrapping GET /v1/bookings: 'route "GET
/v1/bookings" is unclassified: h.RequireAuth(h.ListBookings)'.
… OpenPair
Two workspaces on distinct public hosts, one process, and an application handle
that is a NOBYPASSRLS role owning nothing. Eight assertions, each in both
directions: event types list, bookings list, slots, the public booking page,
GET /v1/bookings/{id} (200 for its own, 404 for B's), a public create that lands
in acme and leaves B's count unmoved, an MCP list_bookings over the streamable
HTTP transport, and A's key on B's host refused 403 {"error":"workspace
mismatch"} while the same key on the identity host still works.
The MCP call is asserted in both directions, A then B, because that is what
catches a cached server built for whichever workspace called first.
dbtest.RequireTenantPair is the harness, now reusable: it creates the role, grants
the schema, enables RLS, opens the pair and runs VerifyRoles. It SKIPS LOUDLY
rather than falling back if the role cannot be created or turns out to bypass,
because every assertion in this file passes through a superuser handle whether the
policies exist or not.
VerifyMCPBearer was reading credentials on the tenant handle — all four of its
statements. /mcp is on the identity host so no workspace is bound at all, and the
test found it as "connect to /mcp: Unauthorized": on a multi-tenant instance every
valid bearer token would have been rejected. Now on platformDB, the third place
where a global-unique credential lookup had to move. The pattern is that any read
whose purpose is to discover the tenant cannot be bound to it.
Two controls. Stubbing the binding off fails everything at the fixture with
SQLSTATE 42501, which proves the binding is load-bearing and nothing more. Giving
the application handle the bypassing owner role — the "a superuser DSN proves
nothing" case made concrete — is the one that shows the assertions working: A's
booking list contains B's booking, B's slots render on A's host, B's booking page
returns 200, B's booking id returns 200 with B's data, and both MCP directions
leak.
A trap the fixture paid for: server.New's drain blocks until the worker finishes
its cycle, and the worker stops only when its context is done, so passing
context.Background() gives a green body and a hang in Worker.Wait that reads as a
deadlock in the code under test.
PROGRESS.md carries two notes into B6: vendor webhooks must resolve their
workspace from the row they name on the platform handle and then hand off to
forWorkspace, and every INSERT through a Platform route or the platform API must
name workspace_id explicitly, because the platform handle binds '' and the column
default would land the row in 'default' silently.
…re wrong
Three of these had been fixed one at a time, each found by a failing test
(RequireAuth, MCPCallerMiddleware, VerifyMCPBearer). This enumerates all eighteen
sites so the fourth is not found the same way. The table is in PROGRESS.md.
The rule: a read whose job is to discover the tenant cannot be bound to it. The
corollary is what the two findings are: a WRITE on a Platform-wrapped route must
name workspace_id, because the platform handle binds '' and the column default is
COALESCE(current_setting('app.workspace_id', true), 'default') — so an omitted
column does not fail, it lands the row in the default workspace.
Fifteen sites were already right. Six are on platformDB or are Platform-wrapped
routes where h.db already IS the platform handle. Nine are correctly on the bound
handle: magic links, invites and manage tokens are read on host-scoped routes and
the email that carried them was sent from that workspace's own public host, so a
token of B presented on A's host resolves to nothing — which is the right answer
and is now tested.
Two were wrong.
The OAuth grant inserts (oauth_auth_codes, oauth_access_tokens) named no
workspace_id, so every MCP Connect grant landed in the default workspace. It still
worked, which is why nothing caught it: VerifyMCPBearer reads on the platform
handle and bypasses the policies, so agents connected and called tools normally.
What broke was ownership — the workspace's Connected-apps page could neither list
nor revoke the grant, and deleting the workspace would not have cascaded it. A
revocation UI that silently cannot revoke is worse than none. Fixed by naming
workspace_id from a new workspaceOfUser helper on the platform handle, and tested
by driving the real flow: register, consent with its CSRF cookie, token exchange
with PKCE S256, then assert the row's workspace through the platform handle and
that A's connections page lists it while B's does not.
POST /v1/setup is Platform-wrapped and creates the first user plus a live API key;
both would land in the default workspace, a working credential in a tenant nobody
owns and no host reaches. It now 404s under MULTI_TENANT. Narrow in practice —
setup's own 409 guard covers it once any user exists — so this is the fresh-database
window plus defence in depth, and PROGRESS.md says so rather than overselling it.
One assertion had to be corrected and it is a trap worth keeping: the manage page
answers 200 with TokenInvalid for an unknown token, by design, because it is a
booker-facing page and not an API. A status-code assertion against a surface that
renders its own errors proves nothing; the test now asserts on content.
gofmt -l empty, go vet clean.
Boundary 4. The five Set/get singleton pairs on shared — mailer, LLM, Zoom, Stripe, LiveKit — each behind its own RWMutex, become five tenantCache[T]. Every value is built lazily from THAT workspace's server_settings row through the bound handle. The settings-save handlers needed no edit: they already call SetX after writing, and SetX now writes at h.cacheKey(), so a save on a credential-scoped handler primes its own key and nobody else's. Three deliberate things in the cache. The key is "" in single-tenant mode rather than "default", so the map cannot grow past one entry and nothing that calls ForWorkspace can make it. present is tracked separately from entries because nil is a meaningful value — a workspace with no Stripe credentials caches a nil client, and conflating nil with absent would rebuild it on every request. And the builder runs outside the lock, because every builder reads server_settings and holding the write lock across a round trip would serialise every tenant behind the slowest; the first store wins, so 32 concurrent gets ran 1 builder and returned one value. mailer.From() is new on *SMTP, *Resend and *Live. The sender address is the only per-workspace value a built mailer exposes, so without it the test could only assert two mailers are different pointers, which a shared cache also satisfies. The negative control is in the tree rather than run by hand: the same two mailers built under the real keys and under a key stubbed to "", with both halves asserted. It logs the bug in one line — with the key stubbed, B would send as bookings@acme.example instead of hello@globex.example. A single handle cannot tell two workspaces apart: db.ForWorkspace is the identity function on a handle that did not come from OpenPair, so the first draft of these tests compared a value with itself and failed as "mailer *mailer.Noop exposes no From address" — which reads like a builder bug rather than a harness one. The four workspace-distinguishing cases now take a real pair and skip loudly on SQLite. One documented gap: getCal is unchanged. The calendar Service is a registry of providers keyed by the instance's OAuth app, which D7 keeps platform-level, and all three providers capture a *db.DB at construction — so making it per-tenant needs a ForDB on the Service and each provider plus config plumbing. Today the captured handle is the unbound one, which matches no row, so on a multi-tenant instance calendar sync is inert rather than cross-tenant. Safe, but not working; it belongs with B5, which touches the reconciler anyway. PROGRESS.md says so. gofmt -l empty, go vet clean, all seven cases pass under -race.
…g what cannot be primed Boundary 5, part one: deliverables 1 and 4. The worker, the reconciler and the CLI subcommands follow. B4 left getCal as a process-wide singleton and called it "inert rather than cross-tenant". Inert is not shippable — calendar connections are the product — so calendar.Provider gains ForDB(handle) Provider, implemented by gcal, microsoft and caldav as a shallow copy with the handle replaced. No import cycle: all three already import internal/calendar for CalendarInfo and calendar imports none of them. Service.ForDB rebuilds the provider map through each provider's own ForDB and carries primary over, so which backend claims a new connection does not change per workspace. The OAuth app configuration stays platform-level per D7 — client id, secret, redirect and encryption key identify the instance to Google and Microsoft, not the tenant — so ForDB is deliberately shallow. shared now holds calBase, the registry boot installs, and calCache, the per-workspace bound copies, and getCal is the only reader: returning calBase directly is the bug this splits apart. Single-tenant returns calBase itself, because rebinding would allocate a second Service and every provider in it for no behaviour change. SetCalendar invalidates, so SetCalendar(nil) is not shadowed by a cached service. The test uses CalDAV, which needs no instance-level OAuth app, so it is about the handle and not about credentials: two workspaces with one connection row each, and per workspace Connected and HasDestination true for its own user and false for the other's, with a platform-handle control that both rows exist so a false is the policy rather than an empty table. Disconnect is the write half — A asking to disconnect B's user leaves B's row intact. The negative control is in the tree and logs the leak in one line: with the key stubbed, B's service sees A's connection and not its own. That is one tenant's free/busy deciding another's availability, which is worse than the mailer case because it is silent. Boot priming is made single-tenant-only rather than removed. Every LoadXSettingsFromDB reads server_settings through the unbound handle, which in multi-tenant mode matches no row — so it installed nothing and logged "not configured" for an instance whose tenants are all configured. Gated rather than deleted because it is the only path that seeds env-var SMTP into the database on first boot, and on a single-tenant instance it is what makes the first request fast instead of lazy. gofmt -l empty, go vet clean, four calendar cases pass under -race.
Boundary 5, deliverable 2. The reconciler and the CLI subcommands follow.
The claim loop and the work are on opposite sides of the tenancy boundary, and
getting either side wrong is silent. Claiming has to see every workspace's jobs —
the queue is one queue ordered by run_at globally — so it runs on the platform
handle and the claim query carries no workspace predicate, which is what migration
00060's workspace-free idx_jobs_pending_global exists for. Doing the work has to see
exactly one workspace, so it runs on ForWorkspace(the job's workspace_id) with that
workspace's mailer and webhook service. A bound claim loop would serve one tenant and
starve the rest; a platform-handle worker would read across them.
worker.TenantDeps{DB, Mailer, Webhook} is what a job is processed with, supplied by
WithTenantResolver. Unset — single-tenant — depsFor returns the Worker's own handle
and mailer, so nothing changes. sendReminder and deliverWebhook take deps rather
than reading the Worker's fields.
RegisterHandler becomes func(ctx, workspaceID, payload) error. The packet offered a
context value or a signature change and asked for the smaller: the signature is two
call sites and two method heads, against a context key every future handler author
would have to know exists. Each notetaker job now starts with
h = h.workspaceForJob(workspaceID), before any row is read, because the receiver the
worker calls it on is unbound. handler.TenantRuntime returns the three per-workspace
values and server.New adapts them; the handler does not import the worker package.
Three tests. One reminder each, both due, one Poll: both reach done, each sent with
its own workspace's mailer to its own attendee, with a cross-check that neither
mailer saw the other's. Each workspace registers a webhook at its own path on one
httptest server so the arriving signature can be attributed: the two signatures must
differ and so must the two secrets — a shared service would sign one tenant's
payloads with another's secret, which gets reported as "your webhooks are broken"
rather than as a tenancy bug. And the custom-handler signature is asserted by job.
The negative control is the old shape: the same due job run through the platform
handle sends 1 reminder, then requeued and run through the application handle sends
0 and stays pending. jobs is a tenant table and the application handle is unbound,
so it matches no row — the loop runs, finds nothing, logs nothing. That is a third
failure mode, distinct from leakage and starvation.
The Poll housekeeping sweeps stay cross-tenant on the platform handle, deliberately:
they are retention rules, not tenant logic. The Stripe payment-hold backstop is
called out in PROGRESS.md because it writes across tenants, keyed on a property of
the row rather than of the tenant.
gofmt -l empty, go vet clean, the whole worker package passes under -race.
…m handle Boundary 5, deliverable 3. reconcileCalendar becomes a dispatcher: reconcileTargets enumerates active workspaces on the platform handle and each reconcileCalendarPass runs on a handler bound to one of them. Every query in a pass reads bookings, booking_hosts and calendar_connections, which are tenant tables, so a pass on the platform handle would reconcile every workspace's bookings against whichever connections it resolved, and a pass on the unbound application handle reads nothing and heals nothing. It was the second of those. activeWorkspaceIDs sits with the other workspace reads and filters status='active' there rather than at each call site, so the next periodic loop cannot forget. Suspended workspaces are skipped as a decision, not an optimisation: a suspended tenant answers 503 on its own surfaces, so healing its calendar in the background would be work on its behalf that it can neither see nor stop. Single-tenant short-circuits before the query — one target, the handler itself. The enumeration test asserts the target set and that each target's handle is bound to the workspace it is labelled with, because a right list over wrong handles would pass a weaker check. The effect test uses a cancelled booking that still carries a calendar event id in one active and one suspended workspace, with a control that both rows start stale: after the sweep the active one is cleared and the suspended one is byte-identical. The negative control runs one pass on the unbound handle — 0 of 1 healed, no error — then the fixed sweep over the same fixture and asserts the row is healed, so the difference is the binding and not the fixture. gofmt -l empty, go vet clean, four cases pass under -race.
…y what stays global Boundary 5, deliverable 5, plus the cross-tenant inventory B7 will assert against. Which of the four subcommands needs what follows from what it touches, so the classification is data (platformWideCLI) with a test asserting it. rotate-key and recover-key operate only on crypto_keystore, which 00060 leaves exempt from tenancy because there is one DEK per process — it has no policies at all, so DATABASE_URL's handle reaches it either way. Both now carry a comment saying that, and saying that per-tenant DEKs would make it wrong. reset-admin was a real hole rather than a hygiene issue. Since D9 the unique on users is (workspace_id, email), so UPDATE users SET password_hash = ? WHERE email = ? on an unscoped handle matches every workspace that has a user with that address — and a shared address across an agency's client workspaces is the ordinary case. The negative control measures it: one unscoped statement reset 2 workspaces' owners. A recovery tool that quietly hands out access to tenants nobody asked about is worse than one that refuses. --workspace is accepted in both forms, anywhere among the positional arguments, validated against the same shape db.ForWorkspace enforces, and in single-tenant mode accepted and ignored so a fleet script keeps working. mcp stdio is refused: the transport carries no credential and no Host, so there is nothing to resolve a tenant from and the tools would run unbound, which matches no row — an empty workspace with no explanation. The message names POST /mcp with a workspace credential, and a test asserts it does, because a refusal that does not say what to do instead is a dead end. PROGRESS.md now inventories every remaining cross-tenant read and write with the reason each is correct: the tenant-root and credential reads, the exempt tables, the worker's single queue and its five retention sweeps, and the one cross-tenant WRITE left — the Stripe payment-hold backstop, flagged as the entry a reviewer should push back on and the one that has to move if hold timeouts ever become per-workspace. The three sites that are not yet correct are listed separately and owed to B6. gofmt -l empty, go vet clean, nine cases pass.
Six conflicted files, ten hunks, plus three routes and one migration that had to be
classified and renumbered on the way in. Every route the platform branch adds now goes
through this branch's wrappers, and every write it makes names its workspace.
Conflicts and how each was resolved
-----------------------------------
internal/config/config.go (3 hunks)
Both sides added fields and both rewrote Validate's doc comment. Kept both field sets
(SSOSharedSecret, MetricsToken, STTBaseURL, TrustedProxyCIDRs, FrameAncestors from the
platform branch; MultiTenant, DatabaseAdminURL, PlatformToken from B1) and merged the
doc into the two families it now holds: settings whose malformed value weakens a
defence silently, and multi-tenant combinations whose only other outcome is silent
cross-tenant exposure. Validate runs the FRAME_ANCESTORS loop first, then returns early
when MultiTenant is unset, so a single-tenant instance still gets the CSP check.
internal/config/config_test.go (2 hunks)
Additive on both sides; kept both test groups.
internal/db/postgres_test.go (1 hunk)
knownMigrationCount 60 -> 61 for the renumbered sso_nonces migration.
internal/handler/handler.go (1 hunk)
The platform branch's SetSTTBaseURL/sttBaseURL landed beside B3's publicURL. The three
new settings (ssoSecret, metricsToken, sttBaseURLCfg) live on `shared`, not on the
per-request Handler value: each is one process-wide env-var setting, none is per
workspace, and putting them on the value would copy three strings per request for
nothing. SetSSOSecret/SetMetricsToken keep working unchanged.
internal/server/server.go (2 hunks)
BuildHandler: kept both blocks of setters and added SetMultiTenantLimits(cfg.MultiTenant)
before any RateLimit call, since the bucket key depends on it (D14).
New: the three new routes are classified through this branch's wrappers -
GET /metrics -> h.Platform, and its jobs read goes through
Platform() inside the handler
GET /v1/auth/sso -> h.Platform (no tenant Host, no credential; the
token IS the credential and the workspace is
its wid claim)
POST /v1/auth/sessions/revoke-all -> h.RequireAuth(h.Scoped(CredentialWorkspace,..))
The revoke-all registration is on ONE line: routes_classified_test.go scans this file
line by line, so the platform branch's wrapped continuation line read as an
unclassified route and redded the gate. 172 routes: 31 host-scoped, 107
credential-scoped, 26 platform, 8 allowlisted.
Kept the platform branch's FrameAncestors wrap of the admin SPA and its
TrustClientIP/ParseTrustedProxies outermost middleware, which is what makes the D14
key's IP half the client's own address rather than the proxy's.
internal/worker/worker.go (1 hunk)
Kept both: the sso_nonces GC sweep (platform handle, alongside the other retention
deletes - a nonce is global, see below) and B5's per-workspace reminder path. The
platform branch's booking.reminder webhook fires through deps.Webhook, not w.svc, so it
is signed with the job's own workspace's secret.
Not conflicted but required by the merge
----------------------------------------
Migration: 00059_sso_nonces -> 00061_sso_nonces in both dirs (00060 is tenancy).
sso_nonces is EXEMPT, not a tenant table. A jti is global: the question it answers is
"has this exact token been spent", and the answer must not depend on which workspace
the token names - per workspace the same jti could be replayed once per tenant. It also
has no owner at the moment the row is written, because the wid claim has not been
trusted yet. db.ExemptTables and TestTenancy_tableListsCoverTheSchema updated.
The postgres file declares expires_at COLLATE "C": the worker purges it with a
lexicographic expires_at < ?, which is exactly what migration 00059 pinned the other
54 TEXT timestamps for, and collation_test.go audits by column name so it would have
failed the moment the table existed. wantTimestampColumns 56 -> 57.
internal/handler/sso.go
The wid claim was parsed and deliberately ignored ("an instance is a single workspace
today"). It is now load-bearing in multi-tenant mode: required, validated, and read
back through the platform handle so a deleted, suspended or public-host-less workspace
is refused rather than seating a user nobody can reach. The audience is the WORKSPACE's
public host, not BASE_URL (D11) - a token for tenant A must not be spendable on B's
host. Single-tenant behaviour is unchanged: wid is ignored and aud is BASE_URL.
Every statement now names the workspace. /v1/auth/sso is a Platform route, so h.db
bypasses the policies and binds '': since D9 the unique on users is
(workspace_id, email), an unqualified WHERE email = ? resolves an arbitrary one of the
workspaces holding that address and hands the token a session on somebody else's
tenant. Same fix for the owner count, which counted owners instance-wide.
internal/handler/session.go
createSessionIn(..., workspaceID) so the hand-off can name the session's workspace. An
empty workspaceID keeps the original statement, so the other seven callers - all on
handles already bound to the request's workspace - are untouched. It matters beyond
bookkeeping: every later request for that user runs on a BOUND handle, which could
neither read nor delete a session filed in the wrong workspace.
internal/server/middleware.go
D14 wired now that TRUSTED_PROXY_CIDRS exists: rateLimitKey is (workspace host, client
IP) when SetMultiTenantLimits is on, the IP alone otherwise. The workspace comes from
the Host rather than a resolved *Workspace because the limiter runs before any handler
and rejecting must not cost a database read; an unrecognised host keys on the host
string, so a Host-rotating attacker gets a bucket per value and spends no tenant's
allowance.
New tests
---------
internal/handler/sso_tenancy_test.go one email handed to two workspaces yields two
users and two sessions, each in its own
workspace; bad wid, wrong audience and a
suspended workspace are refused
internal/server/ratelimit_tenancy_test.go two workspaces behind one proxy IP do not
share a bucket; two bookers of one workspace
keep their own; single-tenant ignores the Host
Gates
-----
gofmt -l . empty, go vet ./... clean, go build ./... clean
go test ./... rc=0 on SQLite (30/30) and rc=0 on PostgreSQL (30/30)
go test -race -count=1 ./internal/handler/ ./internal/worker/ rc=0 on PostgreSQL
negative control, wrong password: internal/db FAILS with SQLSTATE 28P01 rather than
skipping, and so does the new SSO tenancy test
…ceded it
⛔ A booking created and rescheduled inside the same second could keep its reminder
pinned to the ORIGINAL time, permanently, with no error logged anywhere.
Two detached goroutines write the same row. The CREATE path
(enqueueBookingReminders -> enqueueReminder) and the RESCHEDULE path
(rescheduleSideEffects -> replaceReminderJobs) both marshal the identical payload
{booking_id, hours_before}, and jobs carries a unique on (workspace_id, type, payload),
so exactly one row can exist per (booking, hours_before). replaceReminderJobs deletes
the booking's non-running reminder rows and re-inserts them at the new time. The losing
interleaving is:
reschedule: DELETE ... (create's row not yet committed, so invisible)
create: INSERT at the OLD time
reschedule: INSERT at the NEW time -> conflict -> ON CONFLICT DO NOTHING -> dropped
The reschedule's own row is the one discarded, and the attendee is then reminded about a
time the meeting no longer has - or not at all, if the old run_at is already past.
Fix: the reschedule INSERT upserts.
ON CONFLICT (workspace_id, type, payload) DO UPDATE
SET run_at = excluded.run_at, status = 'pending', attempts = 0
WHERE jobs.status <> 'running'
The create side deliberately KEEPS DO NOTHING: a create must never overwrite a
reschedule, and by the time the two can collide the reschedule is the later fact. With
DO UPDATE on one side only, all three interleavings agree on the reschedule's run_at.
The arbiter is the index's exact column set, read from migration 00060 rather than
assumed: both engines declare (workspace_id, type, payload) after it (the (type, payload)
lines at sqlite:161 and postgres:268 are the Down sections). On PostgreSQL a conflict
target that matches no index is a runtime error, not a compile error.
What it does to a `done` row for the same payload - i.e. a reschedule after the reminder
has already fired: nothing, because such a row never reaches this statement. The DELETE
above spares only 'running', so an already-fired reminder is removed and re-inserted
fresh at the new time. That is the intended semantics: the attendee was reminded about a
time that has since moved, so they are owed another reminder at the new one. The
alternative - leaving the done row and skipping the new reminder - would silently drop a
notification for a meeting whose time changed after it was announced.
WHERE jobs.status <> 'running' preserves the invariant the DELETE already encodes: a job
the worker has claimed is executing now, and resetting it to pending underneath would
either double-send or be clobbered by the worker's own completion write. A running
conflict leaves the row alone, exactly as today.
Tests
-----
internal/handler/reminder_race_test.go forces the losing interleaving rather than waiting
for it: the create-side row is inserted inside an UNCOMMITTED transaction, so it is
invisible to the DELETE and already owns the key, which makes the reschedule's INSERT
block until that transaction commits. The test waits on pg_stat_activity for a backend to
be genuinely lock-waiting before committing, and FAILS if none ever is - a run that never
reached the interleaving would otherwise pass whatever ON CONFLICT does. PostgreSQL only,
with a loud skip: SQLite runs on one connection by design, so neither the interleaving nor
the race can be constructed there.
Negative control, DO NOTHING put back:
--- FAIL: TestReplaceReminderJobs_upsertSurvivesTheLosingInterleaving
surviving reminder run_at = 2026-09-15T10:00:00Z; want 2026-09-18T09:00:00Z
(the rescheduled time). 2026-09-15T10:00:00Z is the ORIGINAL time, which is what
ON CONFLICT DO NOTHING leaves behind
TestRescheduleBooking_updatesReminderJob is fixed as a test too, because it reported this
bug as something else entirely: it polled for "any run_at that is not the old string",
kept the last value read when its 2s deadline expired, and then asserted on it - so a
real failure surfaced as "run_at is three days wrong" rather than as a timeout. It now
polls for the WANTED run_at, fails naming the timeout and both values, and reads with
ORDER BY run_at DESC LIMIT 1 (an event type may have several reminder offsets, so
QueryRow without an order returns an arbitrary one on PostgreSQL and a stable one on
SQLite - a test that passes on one engine and flakes on the other).
⚠️ This is a PRE-EXISTING upstream race, not a regression from the platform-hooks merge.
Both writers already produced the identical payload before it. My earlier attribution
(0 failures in 60 runs before, 2 in 60 after) was weak evidence: at a ~3% rate a clean
run of 60 is unremarkable.
Gates
-----
gofmt -l . empty, go vet ./... clean, go build ./... clean
go test -count=30 (reschedule + race tests): 0 failures of 30 on PostgreSQL, 0 of 30
on SQLite (was 2 of 60 on PostgreSQL)
go test -count=1 ./... rc=0 on SQLite (30/30)
go test -count=1 ./internal/handler/ ./internal/worker/ rc=0 on PostgreSQL
…pace (D12)
internal/handler/platform.go. Four routes on the identity host, all Platform-wrapped,
bearer CALNODE_PLATFORM_TOKEN with a constant-time compare:
POST /v1/platform/workspaces 201 {api_key, webhook_secret}, once
GET /v1/platform/workspaces/{id}
PATCH /v1/platform/workspaces/{id} public_host, status (active|suspended), slug
DELETE /v1/platform/workspaces/{id} cascade, 200 {recording_object_keys: [...]}
Two gates that answer differently on purpose: with the token unset, or on a single-tenant
instance, every route 404s (the API does not exist here, and a prober should not learn
which of the two reasons applies - it is Setup's mirror image, which 404s in multi-tenant
mode). With the token set, a wrong token is 401, so an operator with a typo can tell that
apart from a missing feature.
Provisioning is ONE transaction: the workspaces row, server_settings (id = 1 per
workspace, D8), the owner user, that owner's first cno_ key, the webhook subscription, and
the default event type with its availability rules. Either the tenant exists complete or
it does not exist - a half-provisioned workspace answers requests with no owner, or serves
a booking page with no availability. The 409 test asserts the rollback, not just the code.
⛔ Every INSERT names workspace_id. h.db here is the platform handle: it bypasses the
policies and binds '', so an unnamed column resolves to '' and the row fails its foreign
key to workspaces(id) with SQLSTATE 23503. Reads are equally unscoped and each carries its
own workspace_id predicate - there is no policy behind this file to catch a forgotten
WHERE.
⚠️ owner_timezone is stored as the owner's iana_timezone, NOT UTC. availability_rules holds
local HH:MM with no zone of its own, so defaulting the owner's zone would silently move
the workspace's working hours.
Migration 00062: server_settings gains embed_allowed_origins and stt_base_url
------------------------------------------------------------------------------
Both are in the settled contract's `defaults` and neither had a column. Both are
per-TENANT facts the environment cannot express: one process-wide EMBED_ALLOWED_ORIGINS
would let one tenant's allowlist govern another's embed, and the STT host is a residency
knob, which is a property of the tenant. Default '' means "fall back to the process-wide
value", so single-tenant and every existing row behave as before. knownMigrationCount
61 -> 62.
⚠️ They are WRITTEN and not yet READ: the embed CORS check still uses
config.EmbedAllowedOrigins and the notetaker still uses config.STTBaseURL, so a
multi-tenant deployment currently shares one embed allowlist and one STT host. Storing
them now means a provisioned workspace does not silently lose what the caller sent; wiring
the readers needs the per-workspace settings cache (D7) and is separate work.
webhook.NewSecret
-----------------
The webhook secret's encoding gets exactly one implementation. The column holds the
AES-GCM of the RAW secret bytes, Sign uses those bytes as the HMAC key, and the string
handed to the subscriber is their hex. A second implementation that encrypted the hex
string would store a key the subscriber cannot reproduce, and every delivery would fail
its signature check with nothing in the logs to point at. A caller-supplied secret must
be hex and at least 16 bytes; absent, one is generated. The INSERT stays in the
provisioning transaction, where it can name workspace_id.
⛔ Decision: the `default` workspace is SUSPENDED at multi-tenant boot
---------------------------------------------------------------------
db.SuspendDefaultWorkspace, called from main right after EnableRLS, idempotent, and
non-fatal (a sweep over an empty workspace is waste, not damage).
Migration 00060 seeds `default` because it is the workspace every single-tenant row
belongs to and the one SQLite's column default names. On a multi-tenant instance it is a
tenant nobody owns - no public_host, no users, no settings - and while it is active every
background sweep enumerates it, because activeWorkspaceIDs filters on status = 'active'.
NOT in the migration, which is the point: a migration cannot see MULTI_TENANT, and in
single-tenant mode `default` IS the workspace, so a suspended row there would make Scoped
answer 503 to every request on the instance. Suspension rather than deletion because the
row is referenced by server_settings and by whatever single-tenant data a converted
instance still holds, and because one status flip removes it from every sweep at once
instead of adding a second exclusion rule a new loop could forget.
Two things the tests paid for
-----------------------------
⚠️ httptest.NewRequest does not populate mux path values - they come from the pattern
ServeMux matched, and these tests call the handler directly. Every {id} route therefore
read an empty id and answered 404, which looks exactly like a missing workspace.
⚠️ location_type and routing_mode carry CHECK constraints (migration 00001) admitting no
'none' or 'single'. The seeded event type is 'link' / 'fixed', the schema's own defaults.
The first run failed with 23514 as a bare 500, visible only with a live logger.
Gates
-----
gofmt -l . empty, go vet ./... clean, go build ./... clean
go test -count=1 ./... rc=0 on SQLite (30/30)
PostgreSQL: internal/handler rc=0 (410.2s), internal/db rc=0, internal/server rc=0,
internal/webhook rc=0
--- PASS: TestPlatform_createProvisionsTheWholeWorkspace (1.04s)
--- PASS: TestPlatform_createdWorkspacesAreIsolated (1.25s)
--- PASS: TestPlatform_duplicateIDOrHostIs409 (1.10s)
--- PASS: TestPlatform_getPatchDelete (1.08s)
--- PASS: TestPlatform_tokenGate (1.08s)
--- PASS: TestPlatform_404WithoutATokenConfigured (0.73s)
--- PASS: TestPlatform_404OnASingleTenantInstance (0.74s)
--- PASS: TestPlatform_createValidation (11.47s)
--- PASS: TestPostgres_defaultWorkspaceIsSuspendedAtMultiTenantBoot (0.76s)
--- PASS: TestEveryRouteIsClassified (176 routes: 31 host, 107 credential, 30 platform, 8 allowlisted)
…wo columns nothing reads yet
internal/handler/platform_data.go. Three more Platform routes behind the same token gate:
POST /v1/platform/workspaces/{id}/export one JSON document, replay-ordered
POST /v1/platform/workspaces/{id}/import 409 unless the workspace is empty
DELETE /v1/platform/workspaces/{id}/attendees?email=
The replay order is data, not convention
----------------------------------------
exportTableOrder is hand-ordered parents-before-children and the document carries its tables
as an ordered ARRAY, because import replays them in the order it receives. Alphabetical -
db.TenantTables' order - would put booking_answers before bookings and event_type_hosts
before event_types, which is a foreign-key violation.
⛔ exportCoversEveryTenantTable runs at REQUEST time, not only in a test: the export fails if
any tenant table is missing from the order, or if the order names a table that is not a tenant
table. TestTenancy_tableListsCoverTheSchema already forces a new table to be classified; this
carries that guarantee into the backups, so a table added by a later migration cannot be
silently absent from every tenant's export. Being wrong here produces data that was never
backed up, which nobody notices until they need it.
Rows are read with SELECT * plus rows.Columns() rather than 32 hand-written column lists, for
the same reason: a migration that added a column would otherwise stop exporting it, and the
loss would surface only as data missing after an import. ORDER BY 1 makes two exports of one
workspace byte-comparable, which is what the round-trip test rests on.
⛔ Decision: the DEK does not travel; a fingerprint does
-------------------------------------------------------
The instruction for this group said the crypto_keystore row for the workspace travels with it
because the DEK is per tenant. There is no such row: crypto_keystore has no workspace_id at
all - it is exempt (D2) and holds one wrapped DEK per PROCESS (D3), labelled primary /
recovery. Read from the schema rather than assumed.
Manufacturing one would have been worse than useless: an export of a SINGLE tenant would then
contain the key that decrypts EVERY tenant's secrets on that instance, which is the opposite
of what the isolation is for.
So the document carries dek_fingerprint - SHA-256 over the already-encrypted wrapped_dek,
which cannot be reversed to the key. Equal fingerprints mean the two instances share a data
key and the _enc columns will decrypt there; different ones make import refuse with 409. That
refusal is the point. Without it the rows import perfectly and then every secret in them (SMTP
password, LLM key, LiveKit secret, calendar tokens) fails at first use, one integration at a
time, long after anyone is watching the response. Import is the only moment the two keys can
be compared, and the message names CALNODE_ENCRYPTION_KEY because moving it with the data is
the operator's real remedy.
A per-tenant DEK would change this, D3, and the schema. Separate packet.
Secrets and API-key hashes otherwise travel verbatim, per the contract: a tenant whose keys and
manage links stopped working on migration has not been migrated. The document is therefore as
sensitive as the database.
⛔ Import forces the target workspace
------------------------------------
Every row is inserted with workspace_id = the id in the URL; the document's own value is
discarded. The endpoint is authorised by the platform token, so trusting the document would
make an export of any workspace a way to write rows into any other.
⚠️ Ids are GLOBAL primary keys, so import is a MOVE, not a copy. Replaying a document into a
second workspace while the first still holds its rows collides on users_pkey. The supported
operation is export -> delete -> import, usually into another region's instance where those
ids do not exist; the test performs exactly that inside one database, which is the only place
the workspace_id question can be asked. Worth knowing before anyone tries to clone a tenant
for staging with it.
UseNumber on the decoder, and importValue converts a json.Number to int64 when it is one:
without it every numeric column round-trips through float64 and a large id loses its low bits
silently.
Erasure
-------
⚠️ booking_answers carries no attendee - it is keyed (booking_id, question_id) - so "their
answers" has to be derived. Answers are erased only for bookings where the erased person was
the ONLY attendee. With anyone else still on the booking the answers cannot be attributed to
them, and deleting them would erase a third party's data to satisfy someone else's request.
Both halves are tested: two attendees gives 1 attendee row and 0 answers erased; sole attendee
gives 1 and 1.
Nothing is cancelled - the host's calendar and the other attendees' records are not the erased
person's data - and the same address in another workspace is untouched, because one tenant's
erasure request is not consent to delete another's records.
Gates
-----
gofmt -l . empty, go vet ./... clean, go build ./... clean
go test -count=1 ./... rc=0 on SQLite (30/30)
PostgreSQL: internal/handler rc=0 (329.5s), internal/server rc=0, internal/db rc=0
--- PASS: TestPlatformData_exportDeleteImportRoundTrip (1.31s)
--- PASS: TestPlatformData_importIntoAPopulatedWorkspaceIs409 (1.29s)
--- PASS: TestPlatformData_importForcesTheTargetWorkspace (1.33s)
--- PASS: TestPlatformData_eraseAttendee (1.31s)
--- PASS: TestPlatformData_eraseTakesAnswersWhenNobodyElseIsOnTheBooking (1.27s)
--- PASS: TestPlatformData_eraseRequiresAnEmail (1.22s)
--- PASS: TestPlatformData_routesRefuseWithoutTheToken (1.27s)
--- PASS: TestPlatformData_importRefusesAForeignDEK (1.28s)
--- PASS: TestEveryRouteIsClassified (179 routes: 31 host, 107 credential, 33 platform, 8 allowlisted)
Negative control, the workspace-forcing removed so the document's workspace_id is trusted:
--- FAIL: TestPlatformData_importForcesTheTargetWorkspace
import into acme: 400 — violates foreign key constraint
"server_settings_workspace_id_fkey" (SQLSTATE 23503)
⚠️ Not the failure predicted, and stated as it is: it proves the forcing is load-bearing
(unfixed 400, fixed 200 with the rows in acme), but it fails at the foreign key rather than by
writing rows into the wrong tenant, because the source workspace has been deleted by then. The
leak shape is unreachable on one instance for the same reason ids are global; the guard matters
for a destination where the named workspace does exist.
…ot four of them The platform API's create wrote a subscription for the booking four (created/cancelled/rescheduled/reminder). This codebase emits SEVEN - recording.completed, transcript.ready and notes.ready as well - and a receiver on the other side of a provisioned tenancy handles all of them. ⛔ The four-event subscription is not a smaller feature, it is a silent gap: the three media events would never be delivered, and that surfaces weeks later as "recordings never appear", with nothing in either system to point at. The subscription exists, the events fire, and the rows simply never match. The three media events fire only when recording or the notetaker is switched on, so a tenancy without them never receives them anyway. Subscribing is free; not subscribing is a decision the operator cannot see, and can only undo with an API call nobody knows to make. provisionedWebhookEvents is one named list rather than an inline marshal, so the set is reviewable. Enumerated from the tree rather than from memory: every Enqueue call site under internal/ emits one of these seven and nothing else. TestPlatform_createSubscribesToEveryEmittedEvent pins the whole list in stored order, so adding an event to the codebase without adding it here fails. --- PASS: TestPlatform_createSubscribesToEveryEmittedEvent (0.88s)
A `-race -count=30` run with two other workers on the machine failed 2 times in 30 at the POSITIVE CONTROL - `no backend ever blocked on a lock` - and never at the run_at assertion. The fix is holding; the detection was not. Why `wait_event_type = 'Lock'` alone was not enough: the Lock wait is a STATE THE POLL HAS TO CATCH THE BACKEND IN, and pg_stat_activity is a sampled view of it. Under -race (several times slower on the Go side) plus CPU contention, the goroutine issuing the INSERT can still be inside the driver, or between statements of its transaction, for the entire window the loop looks at. From that one column, "not blocked yet" and "never going to block" are the same answer - so the control failed a test whose subject was fine.⚠️ It is NOT driver-side connection queueing. The PostgreSQL pool defaults to 10 open / 5 idle (config.PoolFromEnv) and this test uses two connections, so nothing is waiting for one. It is -race plus load, which is why the remedy is a better signal rather than a bigger pool. Hardened: - a backend in state 'active' running the reschedule's own upsert (matched on its query text) counts as well. Such a backend is either about to block on the key or already past it; either way the interleaving has been reached, which is all this control claims. - deadline 10s -> 30s, so it outlasts a slow loaded box. It is only ever paid when something is genuinely wrong. - the loud failure stays, and now names both signals, so a run that really never reaches the interleaving still fails instead of passing vacuously. Measured after the change, both 0 failures: go test -race -count=30 ... rc=0 fails=0 go test -race -count=30 ... with 4 spinners rc=0 fails=0
The verify half came with the platform-hooks merge. This is the mint half, plus the
correction that makes the verify half safe on a public host.
⛔ The workspace comes from the HOST; wid is CHECKED against it
--------------------------------------------------------------
The endpoint is reached at https://<public_host>/v1/auth/sso, because landing the cookie on
the tenant's own domain is the entire reason the hand-off exists. Resolving the workspace from
the token's `wid` - which is what the merge left - is therefore wrong: a token for workspace A
presented on B's host has a good signature, a wid that resolves and an aud that matches A's
host, so it would quietly create A's session on B's domain.
Now: workspaceByHost, then wid == that workspace's id (403 {"error":"workspace mismatch"}) and
aud == "https://" + that public host, no trailing slash. Unknown host 404, suspended 503.
Single-tenant is untouched: wid ignored, aud is BASE_URL.
Negative control, resolution put back on wid:
--- FAIL: TestSSOHandoff_tokenForAnotherWorkspaceIsRefusedOnThisHost
status = 302; want 403 — <a href="/admin/">Found</a>.
--- FAIL: TestOAuthHandoff_mintedTokenIsRefusedOnAnotherWorkspacesHost
status = 302; want 403
That 302 is the bug: one tenant's session on another tenant's domain.
The state cookie carries the workspace; the URL carries only the nonce
---------------------------------------------------------------------
newOAuthState(w, workspaceID) writes <nonce>|<workspace_id> into the HttpOnly
calnode_oauth_state cookie and sends only the nonce to the provider. verifyOAuthState
compares the nonce and READS the workspace.
⛔ That split is the security property. A visitor can rewrite the state query parameter, and
doing so produces a failed login; the value that decides which tenant a Google identity is
admitted to never left this server's cookie. In the URL instead, anyone could choose the tenant
they are let into.
The workspace is resolved at the login START (GET /v1/auth/login, from the Host the person
clicked on), because the callback arrives on the identity host and cannot know it. That route is
Platform-wrapped - it has to be, since its callback is - so it calls workspaceByHost explicitly
and 404s an unrecognised host, doing by hand what HostWorkspace would.
finishOAuthLogin: two bugs, one of them live before this
-------------------------------------------------------
1. ⛔ The lookup was `WHERE email = ?` on the platform handle. Since D9 the unique on users is
(workspace_id, email), so one address in several workspaces is ordinary - and that statement
resolves an ARBITRARY one of them and starts a session for a stranger. Now scoped by
workspace_id in both modes ('default' in single-tenant, where every row carries it), so
there is one statement and no branch to drift.
2. The session is no longer set here in multi-tenant mode. This callback runs on the identity
host, and a cookie for it is no use to someone whose admin UI is on their own domain. It
mints a 30s HS256 token (iss = BASE_URL, aud = https://<public_host>, wid, role = member,
jti) and redirects to that workspace's /v1/auth/sso.
role = member always: the callback knows only that Google or Microsoft vouched for an address,
which says nothing about what that person may do here. ssoResolveUser leaves an existing user's
role alone, so the value only applies to a user it creates.
⛔ No shared secret means refuse. With CALNODE_SSO_SHARED_SECRET unset the hand-off endpoint
404s, so there is nowhere to land, and setting a cookie on the identity host instead would
produce a session the person's own admin UI cannot see. error=sso, no session.
⚠️ The MCP Connect return keeps its identity-host cookie tail, deliberately: /oauth/authorize is
an identity-host endpoint whose consent-step cookies were set there, so handing it to a tenant's
public host would arrive with none of them.
The calendar callback resolves its workspace from the state's USER
-----------------------------------------------------------------
p.Exchange writes calendar_connections through a provider that captured a handle at
construction. On this Platform route that is the UNBOUND handle, so connecting appeared to
succeed and no row existed. Now the workspace comes from workspaceOfUser on the state's user id
- the state is encrypted, so the id cannot be forged - the exchange runs through
forWorkspace(ws).getCal()'s provider (B5's Provider.ForDB), and the redirect goes to that
workspace's publicURL() rather than h.baseURL, which would have sent the person somewhere their
session does not exist.
⚠️ The workspace is deliberately not ALSO carried in the calendar state: it would be a second
copy of a fact the user id already settles, and two sources of one truth is how they come to
disagree.
⚠️ A trap the test bridge paid for
---------------------------------
Driving finishOAuthLogin on the bare handler reported no_account for a user that exists: the
tail's lookup runs on whatever handle the route gives it, and on the bare handler that is the
unbound application handle, which matches no row. The bridge goes through h.Platform(...) as
server.New does. A bridge that had accepted the bare handler would have been asserting against
the wrong handle.
Gates
-----
gofmt -l . empty, go vet ./... clean, go build ./... clean
go test -count=1 ./... rc=0 on SQLite (30/30)
PostgreSQL: internal/handler rc=0 (302.5s), internal/server rc=0
--- PASS: TestSSOHandoff_multiTenantLandsInTheTokensWorkspace (0.96s)
--- PASS: TestSSOHandoff_tokenForAnotherWorkspaceIsRefusedOnThisHost (0.90s)
--- PASS: TestSSOHandoff_multiTenantRefusesABadWID (3.49s)
--- PASS: TestSSOHandoff_multiTenantAudienceIsThePublicHost (2.40s)
--- PASS: TestSSOHandoff_multiTenantReplayIsRefused (0.77s)
--- PASS: TestSSOHandoff_unknownHostIs404 (0.73s)
--- PASS: TestSSOHandoff_multiTenantRefusesASuspendedWorkspace (0.71s)
--- PASS: TestOAuthHandoff_callbackMintsATokenTheSSOEndpointAccepts (0.76s)
--- PASS: TestOAuthHandoff_mintedTokenIsRefusedOnAnotherWorkspacesHost (0.72s)
--- PASS: TestOAuthHandoff_refusesWithoutTheSharedSecret (0.76s)
and the platform branch's own 11 SSO cases still pass on both lanes
…les stop riding the platform pool POST /v1/livekit/webhook (+ legacy alias) and POST /v1/stripe/webhook. Both stay Platform- classified: the caller is a vendor, so there is no tenant Host and no credential of ours. The workspace comes from OUR row, keyed on an id WE issued --------------------------------------------------------- internal/handler/vendor_webhook.go. LiveKit resolves in three steps - the egress id on a recordings row, then the room's recordings row, then the booking the room name encodes (booking-<id>, the only room name this application creates, which is what a room_finished for a never-recorded meeting needs). Stripe resolves from bookings.stripe_session_id, falling back to the metadata booking id and even then taking workspace_id from the ROW. ⛔ No resolver consults a workspace_id in a vendor payload: that would be a tenant selector supplied by whoever can forge a body. Every read names workspace_id in its own SELECT list. An event no row owns is 2xx and ignored - a 4xx would make LiveKit and Stripe retry for days, and no retry can make a row exist that never did.⚠️ Decision: in multi-tenant mode the RESOLVE precedes the VERIFY ---------------------------------------------------------------- The instruction asked for verify-then-resolve. That is impossible here: both vendors' credentials live in server_settings, i.e. per workspace (D7), and on a Platform-wrapped route the handle bypasses the policies - so LoadLiveKitSettingsFromDB's `WHERE id = 1` matches every tenant's row and returns an arbitrary one. Verifying against a randomly chosen tenant's secret is not verification, and there is no process-wide credential to use instead, because boot priming is single-tenant-only (B5 deliverable 4). So: resolve (one keyed SELECT), verify with THAT workspace's secret, then act. Two properties make it safe, both load-bearing: nothing is written before the signature verifies, and nothing is disclosed (empty body either way).⚠️ Residual: an existence oracle - an unsigned request naming a real egress id gets 403, an unknown one 200. The ids are opaque and a caller with a valid signature already knows the id it sent. Closing it means 403 for an unknown row, i.e. the retry storm this design avoids. Single-tenant keeps the original order exactly: one workspace, one set of credentials, nothing to resolve. ⛔ THE HOLE THIS EXPOSED: forWorkspace was binding onto the PLATFORM pool ----------------------------------------------------------------------- Platform() replaces h.db with the platform handle, whose role BYPASSES row-level security. forWorkspace built its scoped copy with h.db.ForWorkspace(ws.ID) - so the result NAMED a tenant and was not CONFINED to it. A `WHERE id = 1` read through such a handle matches every workspace's server_settings row and returns an arbitrary one; a write lands wherever the statement says. Both silent. The symptom was oblique: the LiveKit webhook's hand-off to forWorkspace(ws).getLiveKit() read the DEFAULT workspace's empty settings row instead of the resolved tenant's, so the client was nil and the handler answered 200 having verified nothing. No error, no log line. Fixed by holding the application handle on shared (appDB, set once in handler.New) and binding every scoped copy from that: h.appBase().ForWorkspace(ws.ID). A tenant-scoped handler now always rides the role the policies constrain, whichever route it came from.⚠️ Why nothing else caught it: every other Platform-route hand-off either names workspace_id explicitly on its writes (the platform API, the SSO endpoint - which is why those tests pass either way) or runs on the worker's path, where the base handler's db IS the application handle. The vendor webhooks are the first code to depend on a scoped handle's READS being filtered. Negative control, the bind put back on h.db: --- FAIL: TestVendorWebhook_livekitBadSignatureWritesNothing status = 200; want 403 200 means it never reached verification, having loaded the wrong tenant's absent credentials. Gates ----- gofmt -l . empty, go vet ./... clean, go build ./... clean go test -count=1 ./... rc=0 on SQLite (30/30) PostgreSQL: internal/handler rc=0 (321.2s), internal/server rc=0 (9.8s) - including the whole B3/B4/B5 tenancy suite, which is what says the appDB change moved nothing else --- PASS: TestVendorWebhook_livekitResolvesTheOwningWorkspace [7 cases] --- PASS: TestVendorWebhook_livekitEventWritesOnlyItsOwnWorkspace --- PASS: TestVendorWebhook_livekitUnknownRoomIs2xxWithNoWrite --- PASS: TestVendorWebhook_livekitBadSignatureWritesNothing --- PASS: TestVendorWebhook_stripeResolvesTheOwningWorkspace [6 cases] --- PASS: TestVendorWebhook_stripeUnknownSessionIs2xxWithNoWrite --- PASS: TestVendorWebhook_stripeBadSignatureWritesNothing PROGRESS.md closes B6: D11-D14 all done, the bring-up env table, and the two known follow-ups (per-tenant DEKs; the D7 reader wiring for the two columns nothing reads yet).
…ot enumerated (B7)
internal/server/tenancy_proof_test.go. Everything before this tested a mechanism: the handle
binds, the resolver resolves, this route is classified. This tests the PROPERTY those mechanisms
exist for, across the whole surface at once: as workspace A, nothing of workspace B is reachable.
⛔ The three lists are DERIVED. A copied list goes stale the first time somebody adds a route,
and the copy that went stale would be the one claiming isolation.
routes server.ScanClassifiedRoutes over server.go — the same scan the classification
gate uses, extracted so there is one implementation rather than two
MCP tools a live tools/list against the running server
job types worker.go's `case "<type>":` dispatch AND server.go's RegisterHandler calls
138 tenant routes exercised as A (31 host-scoped on A's public host, 107 credential-scoped with
A's API key on the identity host), 10 MCP tools, 4 job types.
⚠️ Most synthetic requests answer 4xx — an empty body where one is required, a path value naming
nothing — and that is not the point: a 404 cannot leak, and the table's value is that a NEW route
is covered the moment it is registered. What stops it being vacuous is a floor on how many routes
must answer 2xx (61 do), so a refactor that made everything 404 fails here.
⛔ The derivation caught a real error on its first run. The job-type scan originally read only
worker.go's switch, which covers webhook.deliver and reminder.send; the coverage map I had written
by hand claimed a `notetaker.run` that does not exist, while the two that do —
notetaker.transcribe and notetaker.summarize, registered through RegisterHandler — were uncovered
and unnoticed. Scanning both sources found all four. That is precisely the failure a hand-kept
list produces, and it happened inside the change meant to prevent it.
Both notetaker handlers were then read rather than assumed: each begins with
h = h.workspaceForJob(workspaceID), before any row, and the coverage map records why each type is
or is not driven here rather than leaving it absent.
The unscoped-query control, both halves required
------------------------------------------------
A query that FORGETS its workspace predicate returns 0 rows through the application handle and
every workspace's rows through the platform handle, over six tables. The zero alone would also be
true of an empty database; the two alone would also be true with the policies disabled. It is the
pair that says the policies produced the zero. A's bound handle then sees exactly its own 1.
Negative control: every scoped handler bound to B instead of A
-------------------------------------------------------------
tenancy_proof_test.go:177: GET /v1/users (credential) leaked B's user id ("globex-user") to workspace A
tenancy_proof_test.go:177: GET /v1/users (credential) leaked B's owner email ("globex@example.com")
tenancy_proof_test.go:177: GET /v1/recordings (credential) leaked B's booking id ("globex-booking")
… 7 leak reports in total
Gates
-----
gofmt -l . empty, go vet ./... clean, go build ./... clean
go test -count=1 ./... rc=0 on SQLite (30/30) — the proof skips loudly there, because the
isolation guarantee is row-level security and there is nothing to assert without it
PostgreSQL:
--- PASS: TestProof_noRouteLeaksAnotherWorkspace (2.31s) 138 routes, 61 answered 2xx
--- PASS: TestProof_everyMCPToolIsScoped (1.50s) 10 tools
--- PASS: TestProof_everyWorkerJobTypeIsCoveredAndScoped (1.28s)
--- PASS: TestProof_unscopedQueryReturnsNothing (1.31s) 6 tables
go test -race -count=1 ./internal/server/ -run TestProof rc=0, no data race
internal/server/rss_proof_test.go, opt-in behind CALNODE_RSS_PROOF=1. It provisions 200 workspaces through the real platform API in one process, serves one request to each on its own public host, and reads VmRSS from /proc/self/status either side. A test rather than a script under scripts/ deliberately: the number wanted is the RSS of a process that has the handler, the per-workspace caches and the pool in it, and a separate script would measure a different process. Measured on the dev PostgreSQL: baseline (handler + worker, no tenants) 28 936 KB after provisioning 200 workspaces 33 676 KB after one request to each 35 860 KB growth 6 924 KB -> 34.6 KB per tenant runtime.MemStats.Sys 39 354 KB -> 43 722 KB pool application open=1 idle=1, platform open=1 idle=1 ⛔ This CORRECTS the design's claim of "a few MB per tenant". It is a few MB for HUNDREDS: ~7 MB bought 200 workspaces including their settings rows, owners, API keys, webhooks, event types and availability rules. The per-tenant cost is cache entries plus the workspace row, and ForWorkspace returning a value over a shared pool is why the connection count does not move at all. Two ceilings are asserted rather than the numbers themselves, since RSS is noisy and the GC is not forced: 1 MB per tenant (a per-workspace allocation that never came back would breach it) and 20 pool connections (ForWorkspace must never open one per tenant). --- PASS: TestRSS_200Workspaces (2.58s)
The operator-facing description of the mode: what it requires (Postgres, the two roles, the paired DSNs), the three-layer isolation model and the one rule about the platform handle, the route classes, the platform API contract as settled, the SSO hand-off, the export/import/erasure semantics including the DEK fingerprint rule, the vendor-webhook resolve-then-verify order, what is deliberately NOT per tenant, the checklist and the measured cost (34.6 KB per tenant over 200 workspaces). PROGRESS.md records the B7 gate lines in one place and marks boundaries 1–7 complete.
…the process values (D7) Migration 00062 gave server_settings `embed_allowed_origins` and `stt_base_url`, the platform API has filled them since, and nothing read them: every workspace on a multi-tenant instance shared the process-wide EMBED_ALLOWED_ORIGINS and STT_BASE_URL. The first is an isolation gap (tenant A's embed origins were honoured on tenant B's booking endpoints), the second a routing one (an EU tenant's recordings went to whichever host the process booted with). Both now go through a per-workspace settingsCache built from the bound handle (internal/handler/tenant_settings.go): - The CORS wrapper is PublicCORSFor(originsFor), with PublicCORS(list) kept as the single-tenant case. In multi-tenant mode server.New passes h.EmbedOriginsFor, which resolves the request host to its workspace on the platform handle and answers that workspace's list. An unknown host answers known=false and the middleware then sends NO Access-Control-Allow-Origin at all — never `*`, because "no workspace" must not read as "any origin". An empty per-tenant list keeps the single-tenant meaning (any origin). - sttBaseURL() gains one rung above its existing ladder: the workspace column when multi-tenant and non-empty, then STT_BASE_URL, then the provider default. Single-tenant mode consults neither column, so its behaviour is byte-identical; the cors unit tests it already had pass unchanged through the new constructor. Negative control, the multi-tenant branch in server.New disabled: the end-to-end test answers `200 "*"` on the tenant's own origin, on a foreign origin and on an unknown host, which is the shared-allowlist bug made visible.
The upload directory (avatars, branding assets) was the literal relative "data" in server.BuildHandler, which every deployment has been writing to since the beginning and which a read-only container image running as an unprivileged user cannot create: the first avatar upload fails with EACCES and nothing at boot says why. DATA_DIR sets it, defaulting to the same relative "data", so nothing moves for anyone who never sets it. BuildHandler repeats the fallback because tests build a Config literal that skips Load, and an empty directory would put uploads beside the binary.
PROGRESS.md is the working log of the fork this branch was developed on. It is not part of the change and does not belong upstream; the reasoning it holds is in the commit messages.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
Isolation is strong on tenant tables, but the workspaces root policy never turns on, and platform-token auth is weaker than the metrics path already in this PR. Address those before treating multi-tenant as production-ready.
Reviewed changes
Initial review of the full multi-tenant stack on head 7299edd (297 files / 52 commits vs main), including dual-engine Postgres plumbing, RLS/pair handles, request tenancy, platform API, worker claim/work split, SSO hand-off, vendor webhooks, and the derived isolation proofs.
- Opt-in multi-tenant mode —
MULTI_TENANT+ app/platform DSNs; single-tenant paths stay the default. - RLS + per-statement binding — migration
00060,EnableRLS/VerifyRoles,OpenPair/ForWorkspaceGUC binding, Prepare refused on bound handles. - Request tenancy — host/credential/platform route classes,
Scoped/Platform, workspace mismatch and suspended semantics. - Platform control plane — provision/patch/delete, export/import/erasure, DEK fingerprint, bearer
CALNODE_PLATFORM_TOKEN. - Background work — platform claim loop, per-workspace mailer/webhook deps, calendar reconcile per tenant.
- SSO / OAuth hand-off — host-resolved workspace with
widcheck; OAuth mints short-lived HS256 hand-off tokens. - Vendor webhooks — resolve tenant from our rows before signature verify in multi-tenant mode.
- Proofs — route classification scan, RLS proof, end-to-end tenancy and derived surface isolation tests.
⚠️ Process-wide DEK remains the hard isolation ceiling
Row isolation is database-side; ciphertext is not. One wrapped DEK per process means a DB reader can decrypt every workspace, and import correctly refuses a fingerprint mismatch. That matches the stated non-goal, but operators will read “RLS isolation” as stronger than the crypto story. Keep the DEK limit loud in deploy docs and any marketing of multi-tenant.
Technical details
# Process-wide DEK vs row isolation
## Affected sites
- `docs/MULTI_TENANT.md` “What is NOT per tenant”
- `internal/handler/platform_data.go` export/import `dek_fingerprint`
- `crypto_keystore` left exempt in `00060`
## Required outcome
- Operators understand that RLS does not equal cryptographic tenant separation.
- Any public claim of multi-tenant isolation names the shared-DEK limit.
## Open questions for the human
- Is per-tenant DEK work scheduled before anyone runs this with untrusted co-tenants on one DB?ℹ️ Empty embed allowlist still means any origin
EmbedOriginsFor treats an empty per-workspace list as Access-Control-Allow-Origin: *, same as single-tenant. That is documented, but on a multi-tenant fleet “forgot to set origins” is a silent open embed for that booking host. Consider defaulting provisioned workspaces to a deny-empty policy, or requiring at least one origin when multi-tenant is on.
ℹ️ Nitpicks
cmd/calnode/main.gocallscfg.Validate()twice in a row (before and after the start log).- Migration
00062still comments thatembed_allowed_origins/stt_base_urlare “not yet READ”; commit85550e8wires the readers.
Grok ｜ 𝕏
| if _, err := h.DB.ExecContext(ctx, `ALTER TABLE `+table+` FORCE ROW LEVEL SECURITY`); err != nil { | ||
| return fmt.Errorf("force row level security on %s: %w", table, err) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
EnableRLS only walks TenantTables, so the workspaces_read SELECT-only policy from migration 00060 never activates (workspaces is exempt). Without ENABLE ROW LEVEL SECURITY on workspaces, that policy is inert.
The operator checklist (and the RLS proof harness) grants DML on all schema tables; with the policy dead, the application role can INSERT/UPDATE/DELETE the tenant root, which the migration text says only the platform role should do.
Technical details
# Enable RLS on workspaces
## Affected sites
- `internal/db/tenancy.go` `EnableRLS` — loop is TenantTables only
- `internal/db/migrations/postgres/00060_multi_tenant.sql` — `CREATE POLICY workspaces_read ... FOR SELECT`
- `internal/dbtest/tenant.go` / `rls_proof_test.go` — `GRANT ... ON ALL TABLES` includes workspaces
- `TestPostgres_rlsIsOffUntilEnabled` — currently asserts exempt tables stay RLS-off (locks the gap in)
## Required outcome
- After multi-tenant boot, `workspaces` has RLS enabled so non-owner roles get SELECT-only (no write policies).
- Platform owner still writes (owner bypass without FORCE, or BYPASSRLS).
- App role can still SELECT for any legitimate app-handle reads; writes fail under RLS.
## Suggested approach
- `ENABLE ROW LEVEL SECURITY` on `workspaces` in `EnableRLS` (ENABLE only is enough if the app role does not own the table; avoid FORCE if it would break a mis-owned role).
- Adjust the exempt-table RLS test to expect `workspaces` enabled, other exempts still off.| return false | ||
| } | ||
| presented := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ") | ||
| if subtle.ConstantTimeCompare([]byte(presented), []byte(h.platformToken)) != 1 { |
Contributor
There was a problem hiding this comment.
subtle.ConstantTimeCompare on the raw bearer and token returns early when lengths differ, so a wrong-length probe can learn the platform token length. The metrics path in this same PR already avoids that by comparing SHA-256 digests (metricsAuthorized).
Technical details
# Platform token compare should match metrics
## Affected sites
- `internal/handler/platform.go` `platformAuthorized`
- `internal/handler/metrics.go` `metricsAuthorized` (good pattern)
## Required outcome
- Authorization compare does not short-circuit on unequal lengths.
- Still require a proper `Bearer ` prefix before comparing.
## Suggested approach
- Reuse the metrics pattern: hash both sides, then ConstantTimeCompare the digests.| return errors.New("DEMO_MODE and MULTI_TENANT are mutually exclusive — " + | ||
| "demo mode periodically wipes the entire database") | ||
| } | ||
| return nil |
Contributor
There was a problem hiding this comment.
Validate refuses SQLite, a missing/same admin DSN, and demo mode, but not “multi-tenant + Google/Microsoft OAuth without CALNODE_SSO_SHARED_SECRET”. Docs say the secret is required when social login is configured; without it the process boots and OAuth only fails later in mintSSOToken.
Technical details
# Refuse multi-tenant OAuth without SSO secret at boot
## Affected sites
- `internal/config/config.go` `Validate`
- `internal/handler/auth_oauth.go` `mintSSOToken` (runtime failure)
- `docs/MULTI_TENANT.md` environment table
## Required outcome
- If `MultiTenant` and either Google or Microsoft client id is set, missing `SSOSharedSecret` is a fatal Validate error with an actionable message.|
|
||
| // Config whose wrong value is worse than its absence is checked here rather than | ||
| // tolerated at request time — see (*config.Config).Validate. | ||
| if err := cfg.Validate(); err != nil { |
Contributor
There was a problem hiding this comment.
Second identical cfg.Validate() call; the first already ran just above. Drop one of them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
An opt-in
MULTI_TENANTmode. With the variable unset, Calnode is unchanged: SQLite and single-tenant PostgreSQL behave exactly as before and every existing test passes without modification. With it set, one process serves many workspaces, each with its own public hostname, users, event types, bookings, credentials and vendor integrations, and PostgreSQL row-level security is what keeps one workspace out of another's rows rather than the query author.The full design, the operator checklist, the platform API contract and the measured cost are in
docs/MULTI_TENANT.md(added by this pull request). The short version:workspacestable is the tenant root; every application table gainsworkspace_id; every unique that was global becomes composite where it must (users(workspace_id, email),event_types(workspace_id, slug), …).NOBYPASSRLS, owns nothing) for requests and a platform role (schema owner,BYPASSRLS) for migrations, cross-tenant reads and the worker's claim loop. Startup refuses the misconfigurations that would silently disable isolation (same role for both, a superuser application role, a platform role that cannot bypass).Hostheader or from the credential it carries, and every route registration declares which; a source-scanning test fails on a route that does not.*db.DBbound to a workspace runsset_config('app.workspace_id', …)per statement on a pooled connection, so a handle can be copied into goroutines that outlive their request./v1/platform/workspaces, bearer-authenticated, 404 when unconfigured) to provision, read, patch, delete, export, import and erase.Stacked on #29 and #30; the commits unique to this one begin after the merge of
feat/platform-hooks.Cost, measured
200 workspaces provisioned through the API in one process: RSS 28.9 MB → 35.9 MB, about 35 KB per tenant, with the connection pool unchanged (
ForWorkspacereturns a value over a shared pool).What is deliberately not in this pull request
The data-encryption key stays one per process (documented, with an export fingerprint that refuses a mismatched instance); per-tenant keys are a follow-up. The SQLite path has no multi-tenant mode, by design: there is no row-level security to express it with.
On the size
It is large because it is one mode, and a partial tenancy would be worse than none. Every commit is self-contained with its reasoning in the message; the natural review order is the commit order. Happy to split along boundary lines if that helps.
🤖 Generated with Claude Code