From 24be51a608d721694f1263ada745f3b33a275701 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 28 Aug 2026 00:39:26 -0400 Subject: [PATCH 1/2] feat(store): tenant schema + identity plumbing (RIG-2861 T1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tenant-identity foundation for Compass managed multi-tenancy, per the frozen RIG-2861 design record (docs/designs/infra/runtime/compass-managed-multitenancy/design.md, §T1). This is T1 only: schema + context plumbing, no RLS (that is T2, gated on RIG-2877). - New `tenants` table (id, slug, display_name, created_at_unix_ms), folded into 0001_init.sql per the repo's pre-dogfood collapse convention (no new NNNN file). `accounts` gains a NOT NULL `tenant_id` FK + a lookup index. - `store.TenantID` newtype; `WithTenant`/`TenantFromContext` context seam mirroring the comms actor seam (context.go). - `(*Store).BootstrapTenant` — idempotent single-tenant seed mirroring BootstrapAdmin; `Open` seeds it and caches the id. `resolveTenant` stamps the context tenant when set, else the bootstrap tenant — OSS single-tenant stays degenerate with no `if multiTenant` fork. - All four account inserts stamp `tenant_id` via `resolveTenant(ctx)`. Tests (pgtest): idempotent seed, migration on fresh + existing DBs, CreateUser stamps the context tenant and falls back to the bootstrap tenant. Existing account/agent/system suites pass unchanged under the new NOT NULL column. Refs RIG-2918. --- go/internal/store/accounts.go | 16 +-- go/internal/store/accounts_test.go | 8 +- go/internal/store/context.go | 26 +++++ go/internal/store/migrations/0001_init.sql | 23 +++- go/internal/store/store.go | 11 ++ go/internal/store/tenant.go | 55 ++++++++++ go/internal/store/tenant_test.go | 119 +++++++++++++++++++++ go/internal/store/types.go | 4 + 8 files changed, 248 insertions(+), 14 deletions(-) create mode 100644 go/internal/store/context.go create mode 100644 go/internal/store/tenant.go create mode 100644 go/internal/store/tenant_test.go diff --git a/go/internal/store/accounts.go b/go/internal/store/accounts.go index 55c680848..696b82a30 100644 --- a/go/internal/store/accounts.go +++ b/go/internal/store/accounts.go @@ -26,8 +26,8 @@ func (s *Store) CreateUser(ctx context.Context, u NewUser) (Account, error) { defer func() { _ = tx.Rollback(ctx) }() if _, err := tx.Exec(ctx, - "INSERT INTO accounts (id, handle, display_name) VALUES ($1, $2, $3)", - id, u.Handle, u.DisplayName, + "INSERT INTO accounts (id, handle, display_name, tenant_id) VALUES ($1, $2, $3, $4)", + id, u.Handle, u.DisplayName, string(s.resolveTenant(ctx)), ); err != nil { if pgErrIs(err, pgUniqueViolation) { return Account{}, fmt.Errorf("%w: handle %q already taken", ErrConflict, u.Handle) @@ -78,8 +78,8 @@ func (s *Store) BootstrapAdmin(ctx context.Context, u NewUser) (Account, error) defer func() { _ = tx.Rollback(ctx) }() if _, err := tx.Exec(ctx, - "INSERT INTO accounts (id, handle, display_name) VALUES ($1, $2, $3)", - id, u.Handle, u.DisplayName, + "INSERT INTO accounts (id, handle, display_name, tenant_id) VALUES ($1, $2, $3, $4)", + id, u.Handle, u.DisplayName, string(s.resolveTenant(ctx)), ); err != nil { if pgErrIs(err, pgUniqueViolation) { // Already bootstrapped (restart): fetch and return the existing admin. @@ -172,8 +172,8 @@ func (s *Store) ensureSystemSubtypeAccount(ctx context.Context, handle, displayN defer func() { _ = tx.Rollback(ctx) }() // no-op after a successful commit; safe on every non-commit path. if _, err := tx.Exec(ctx, - "INSERT INTO accounts (id, handle, display_name) VALUES ($1, $2, $3)", - id, handle, displayName, + "INSERT INTO accounts (id, handle, display_name, tenant_id) VALUES ($1, $2, $3, $4)", + id, handle, displayName, string(s.resolveTenant(ctx)), ); err != nil { if pgErrIs(err, pgUniqueViolation) { // Already seeded (restart): fetch and return the existing system account. @@ -251,8 +251,8 @@ func (s *Store) CreateAgent(ctx context.Context, ownerUserID AccountID, a NewAge defer func() { _ = tx.Rollback(ctx) }() if _, err := tx.Exec(ctx, - "INSERT INTO accounts (id, handle, display_name) VALUES ($1, $2, $3)", - accountID, a.Handle, a.DisplayName, + "INSERT INTO accounts (id, handle, display_name, tenant_id) VALUES ($1, $2, $3, $4)", + accountID, a.Handle, a.DisplayName, string(s.resolveTenant(ctx)), ); err != nil { if pgErrIs(err, pgUniqueViolation) { return Account{}, fmt.Errorf("%w: handle %q already taken", ErrConflict, a.Handle) diff --git a/go/internal/store/accounts_test.go b/go/internal/store/accounts_test.go index 61cd90adb..f6c1c83fa 100644 --- a/go/internal/store/accounts_test.go +++ b/go/internal/store/accounts_test.go @@ -757,8 +757,8 @@ func TestEnsureSystemAccountWrongShapeSquatterConflicts(t *testing.T) { s := newTestStore(t) id := newID() if _, err := s.pool.Exec(ctx, - "INSERT INTO accounts (id, handle, display_name) VALUES ($1, $2, $3)", - id, SystemAccountHandle, "Squatter", + "INSERT INTO accounts (id, handle, display_name, tenant_id) VALUES ($1, $2, $3, $4)", + id, SystemAccountHandle, "Squatter", string(s.resolveTenant(ctx)), ); err != nil { t.Fatalf("insert squatter account: %v", err) } @@ -776,8 +776,8 @@ func TestEnsureSystemAccountWrongShapeSquatterConflicts(t *testing.T) { owner := mustUser(t, s, "owner") id := newID() if _, err := s.pool.Exec(ctx, - "INSERT INTO accounts (id, handle, display_name) VALUES ($1, $2, $3)", - id, SystemAccountHandle, "Squatter", + "INSERT INTO accounts (id, handle, display_name, tenant_id) VALUES ($1, $2, $3, $4)", + id, SystemAccountHandle, "Squatter", string(s.resolveTenant(ctx)), ); err != nil { t.Fatalf("insert squatter account: %v", err) } diff --git a/go/internal/store/context.go b/go/internal/store/context.go new file mode 100644 index 000000000..47259fbf3 --- /dev/null +++ b/go/internal/store/context.go @@ -0,0 +1,26 @@ +package store + +import "context" + +// tenantContextKey is the private key under which a request's resolved TenantID +// is carried. The auth layer sets it per request after resolving token → +// account → tenant; the store reads it per write to stamp tenancy. Unexported +// so only this package can set or read it — tenant identity can never be +// spoofed through a request field (mirrors comms.actorContextKey). +type tenantContextKey struct{} + +// WithTenant returns a context carrying t as the resolved tenant. The auth +// interceptor calls it after resolving a token to a tenant; tests call it to +// exercise a specific tenant. +func WithTenant(ctx context.Context, t TenantID) context.Context { + return context.WithValue(ctx, tenantContextKey{}, t) +} + +// TenantFromContext reports the resolved tenant set on ctx, if any. On the OSS +// single-tenant path no interceptor sets one, so the store falls back to the +// bootstrap tenant (Store.resolveTenant); the bool distinguishes an unset +// context from a deliberately-set tenant. +func TenantFromContext(ctx context.Context) (TenantID, bool) { + t, ok := ctx.Value(tenantContextKey{}).(TenantID) + return t, ok +} diff --git a/go/internal/store/migrations/0001_init.sql b/go/internal/store/migrations/0001_init.sql index 5d57e5b6f..45eda8cf7 100644 --- a/go/internal/store/migrations/0001_init.sql +++ b/go/internal/store/migrations/0001_init.sql @@ -4,8 +4,9 @@ -- channels + membership + policy, topics and topic-scoped messages, the pinned -- board, agent workspaces, delivery cursors, session ownership + placement, the -- two-tier transcript store, the secrets names registry, the fleet config --- bundle, board issues, the forge-poll fetch machinery, and forge --- authored-artifact ownership. +-- bundle, board issues, the forge-poll fetch machinery, forge authored-artifact +-- ownership, and tenants — the isolation root every tenant-owned table hangs +-- off (RIG-2861). -- -- History note: this replaces the original sequential 0001..0016 migration -- chain PLUS the two migrations added after it (the forge authored-artifact @@ -29,6 +30,20 @@ -- that references it (accounts first, then its subtypes, then everything that -- hangs off them; topics before messages; messages before channel_pins). +-- ── Tenants ───────────────────────────────────────────────────────────────── +-- One row per managed-service tenant; the isolation root every tenant-owned +-- table hangs off (RIG-2861 T1). slug is the stable idempotency key the +-- bootstrap-tenant seed finds-or-creates on (BootstrapTenant), mirroring the +-- unique-handle key BootstrapAdmin uses. created_at_unix_ms is BIGINT ms since +-- epoch, matching the newer unix-ms columns in this schema (sessions, +-- delivery), NOT a TIMESTAMPTZ. OSS single-tenant runs with exactly one row. +CREATE TABLE tenants ( + id TEXT PRIMARY KEY, + slug TEXT NOT NULL UNIQUE, + display_name TEXT NOT NULL, + created_at_unix_ms BIGINT NOT NULL +); + -- ── Accounts ──────────────────────────────────────────────────────────────── -- One row per account; the user/agent split lives in the two subtype tables -- below, mirroring the compass.v1 Account `kind` oneof. handle is globally @@ -37,9 +52,13 @@ CREATE TABLE accounts ( id TEXT PRIMARY KEY, handle TEXT NOT NULL UNIQUE, display_name TEXT NOT NULL, + tenant_id TEXT NOT NULL REFERENCES tenants (id) ON DELETE RESTRICT, created_at TIMESTAMPTZ NOT NULL DEFAULT now() ); +-- The "accounts of this tenant" lookup direction for tenant-scoped reads. +CREATE INDEX accounts_tenant_idx ON accounts (tenant_id); + -- Human accounts: a permission role (0 member, 1 admin). PK is also the FK to -- accounts, so a user row is exactly one account and cannot coexist with an -- agent row of the same id. diff --git a/go/internal/store/store.go b/go/internal/store/store.go index f043ff2de..d1652054a 100644 --- a/go/internal/store/store.go +++ b/go/internal/store/store.go @@ -51,6 +51,11 @@ type Store struct { // No lock: set once before serving, so the write happens-before the first // concurrent parent-edge write (mirrors hub.SetSettleSink). coordinationHook CoordinationHook + // bootstrapTenantID is the single OSS tenant seeded at Open, the fallback + // tenant every write is stamped with when the request context carries no + // resolved tenant (resolveTenant). Set once in Open before the store serves; + // no lock, mirroring coordinationHook's set-once-before-serving discipline. + bootstrapTenantID TenantID } // querier is the read surface shared by the pool and a transaction, so a scan @@ -90,6 +95,12 @@ func Open(ctx context.Context, dsn string) (*Store, error) { pool.Close() return nil, err } + bt, err := s.BootstrapTenant(ctx) + if err != nil { + pool.Close() + return nil, err + } + s.bootstrapTenantID = bt return s, nil } diff --git a/go/internal/store/tenant.go b/go/internal/store/tenant.go new file mode 100644 index 000000000..a3f3a14f1 --- /dev/null +++ b/go/internal/store/tenant.go @@ -0,0 +1,55 @@ +package store + +import ( + "context" + "fmt" + "time" +) + +const ( + bootstrapTenantSlug = "default" + bootstrapTenantDisplayName = "Default" +) + +// BootstrapTenant ensures the single bootstrap tenant exists and returns its id, +// idempotently — the isolation root the OSS single-tenant deployment stamps +// every account with. Mirrors BootstrapAdmin's unique-violation-means-fetch +// shape: on first boot it mints one tenants row (slug bootstrapTenantSlug); on +// every later boot the insert hits the unique slug and the existing id is +// fetched and returned. Called from Open, so a store is tenant-ready before it +// serves. +func (s *Store) BootstrapTenant(ctx context.Context) (TenantID, error) { + id := newID() + if _, err := s.pool.Exec(ctx, + "INSERT INTO tenants (id, slug, display_name, created_at_unix_ms) VALUES ($1, $2, $3, $4)", + id, bootstrapTenantSlug, bootstrapTenantDisplayName, time.Now().UnixMilli(), + ); err != nil { + if pgErrIs(err, pgUniqueViolation) { + return s.tenantIDBySlug(ctx, bootstrapTenantSlug) + } + return "", fmt.Errorf("store: insert bootstrap tenant: %w", err) + } + return TenantID(id), nil +} + +// tenantIDBySlug fetches an existing tenant id by slug, backing +// BootstrapTenant's idempotent restart path. +func (s *Store) tenantIDBySlug(ctx context.Context, slug string) (TenantID, error) { + var id string + if err := s.pool.QueryRow(ctx, "SELECT id FROM tenants WHERE slug = $1", slug).Scan(&id); err != nil { + return "", fmt.Errorf("store: resolve tenant by slug: %w", err) + } + return TenantID(id), nil +} + +// resolveTenant returns the tenant to stamp a write with: the tenant set on the +// context by the auth layer if present, else the bootstrap tenant (the OSS +// single-tenant degenerate path). Mirrors comms.actorFromContext's +// set-or-bootstrap-fallback: no `if multiTenant` fork — a single-tenant +// deployment simply always falls through to the bootstrap tenant. +func (s *Store) resolveTenant(ctx context.Context) TenantID { + if t, ok := TenantFromContext(ctx); ok && t != "" { + return t + } + return s.bootstrapTenantID +} diff --git a/go/internal/store/tenant_test.go b/go/internal/store/tenant_test.go new file mode 100644 index 000000000..ec0acdfa9 --- /dev/null +++ b/go/internal/store/tenant_test.go @@ -0,0 +1,119 @@ +//go:build pgtest + +package store + +// Tenant contracts (RIG-2861 T1): the tenants table migrates onto a fresh and an +// existing database, Open idempotently seeds exactly one bootstrap tenant, every +// account write stamps a tenant_id (the context tenant when set, else the +// bootstrap tenant — the OSS single-tenant degenerate fallback). + +import ( + "context" + "testing" +) + +// tenantOf reads an account's stamped tenant_id directly, so a test asserts the +// persisted tenancy rather than trusting the return value. +func tenantOf(t *testing.T, s *Store, id AccountID) string { + t.Helper() + var tenantID string + if err := s.pool.QueryRow(context.Background(), + "SELECT tenant_id FROM accounts WHERE id = $1", string(id), + ).Scan(&tenantID); err != nil { + t.Fatalf("read tenant_id of %q: %v", id, err) + } + return tenantID +} + +// TestBootstrapTenantSeedsOneIdempotently proves single-tenant boot seeds +// exactly one tenant and a re-run (the restart path) finds it rather than +// minting a second — mirroring TestBootstrapAdminIdempotentByHandle. +func TestBootstrapTenantSeedsOneIdempotently(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + var count int + if err := s.pool.QueryRow(ctx, "SELECT count(*) FROM tenants").Scan(&count); err != nil { + t.Fatalf("count tenants after Open: %v", err) + } + if count != 1 { + t.Fatalf("tenants after Open = %d, want exactly one bootstrap tenant", count) + } + + // A second bootstrap (the restart path) is a no-op find, not a second row: + // same id, still exactly one tenant. + again, err := s.BootstrapTenant(ctx) + if err != nil { + t.Fatalf("BootstrapTenant(restart): %v", err) + } + if again != s.bootstrapTenantID { + t.Fatalf("restart minted a new tenant %q, want the existing %q", again, s.bootstrapTenantID) + } + if err := s.pool.QueryRow(ctx, "SELECT count(*) FROM tenants").Scan(&count); err != nil { + t.Fatalf("count tenants after restart: %v", err) + } + if count != 1 { + t.Fatalf("tenants after restart = %d, want still exactly one", count) + } +} + +// TestTenantMigrationAppliesOnFreshAndExistingDB proves the tenants schema +// migrates onto a fresh database (newTestStore Opens against a reset DB) and +// that re-Opening the same DSN (the existing-DB restart path) applies cleanly +// and adds no duplicate tenant — BootstrapTenant is idempotent at Open too. +func TestTenantMigrationAppliesOnFreshAndExistingDB(t *testing.T) { + ctx := context.Background() + s, dsn := newTestStoreDSN(t) + + first := s.bootstrapTenantID + if first == "" { + t.Fatalf("fresh Open left bootstrapTenantID empty") + } + + // Re-Open against the same, already-migrated database: the existing-DB path. + reopened := reopenStore(t, dsn) + if reopened.bootstrapTenantID != first { + t.Fatalf("reopen bootstrapTenantID = %q, want the existing %q", reopened.bootstrapTenantID, first) + } + var count int + if err := reopened.pool.QueryRow(ctx, "SELECT count(*) FROM tenants").Scan(&count); err != nil { + t.Fatalf("count tenants after reopen: %v", err) + } + if count != 1 { + t.Fatalf("tenants after reopen = %d, want still exactly one", count) + } +} + +// TestCreateUserStampsTenant proves CreateUser stamps the context tenant when +// one is set (the managed multi-tenant path) and falls back to the bootstrap +// tenant when the context carries none (the OSS single-tenant degenerate path). +func TestCreateUserStampsTenant(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + // No tenant in context → the bootstrap tenant is stamped. + bootstrapUser, err := s.CreateUser(ctx, NewUser{Handle: "bootstrap", DisplayName: "Bootstrap"}) + if err != nil { + t.Fatalf("CreateUser(no tenant): %v", err) + } + if got := tenantOf(t, s, bootstrapUser.ID); got != string(s.bootstrapTenantID) { + t.Fatalf("no-tenant CreateUser stamped %q, want the bootstrap tenant %q", got, s.bootstrapTenantID) + } + + // A second tenant row, then CreateUser under its context stamps it. + if _, err := s.pool.Exec(ctx, + "INSERT INTO tenants (id, slug, display_name, created_at_unix_ms) VALUES ($1, $2, $3, $4)", + "tenant-other", "other", "Other", int64(1), + ); err != nil { + t.Fatalf("insert second tenant: %v", err) + } + otherTenant := TenantID("tenant-other") + + scopedUser, err := s.CreateUser(WithTenant(ctx, otherTenant), NewUser{Handle: "scoped", DisplayName: "Scoped"}) + if err != nil { + t.Fatalf("CreateUser(with tenant): %v", err) + } + if got := tenantOf(t, s, scopedUser.ID); got != string(otherTenant) { + t.Fatalf("tenant-context CreateUser stamped %q, want %q", got, otherTenant) + } +} diff --git a/go/internal/store/types.go b/go/internal/store/types.go index 3814853f0..4469823f2 100644 --- a/go/internal/store/types.go +++ b/go/internal/store/types.go @@ -36,6 +36,10 @@ type ( WorkspaceID string // MessageID identifies a message row. MessageID string + // TenantID identifies a managed-service tenant — the isolation root an + // account (and everything reachable through it) belongs to. OSS + // single-tenant runs with one bootstrap tenant (BootstrapTenant). + TenantID string ) // UserRole is a human account's permission role (comms.proto:127-130). The From 512b15bc112b7f4fcf4178792c67d41936a093d0 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 28 Aug 2026 01:18:46 -0400 Subject: [PATCH 2/2] test(store): assert CreateAgent stamps the context tenant (RIG-2861 T1 review) Review finding (low): only CreateUser tenant-stamping was directly asserted. CreateAgent inserts through a different transactional path, so a wrong-tenant stamp there would escape both the CreateUser test and the NOT NULL column. Add TestCreateAgentStampsTenant reading back the persisted agent tenant_id. Refs RIG-2918. --- go/internal/store/tenant_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/go/internal/store/tenant_test.go b/go/internal/store/tenant_test.go index ec0acdfa9..72564aef1 100644 --- a/go/internal/store/tenant_test.go +++ b/go/internal/store/tenant_test.go @@ -117,3 +117,35 @@ func TestCreateUserStampsTenant(t *testing.T) { t.Fatalf("tenant-context CreateUser stamped %q, want %q", got, otherTenant) } } + +// TestCreateAgentStampsTenant proves the agent-insert path also stamps the +// context tenant. CreateAgent inserts through a different (transactional) path +// than CreateUser, so a wrong-tenant stamp there would not be caught by the +// CreateUser test nor by the NOT NULL column — this asserts the persisted +// tenant_id on the agent account directly. The owning user is created under the +// same tenant context so the owner FK resolves within the tenant. +func TestCreateAgentStampsTenant(t *testing.T) { + ctx := context.Background() + s := newTestStore(t) + + if _, err := s.pool.Exec(ctx, + "INSERT INTO tenants (id, slug, display_name, created_at_unix_ms) VALUES ($1, $2, $3, $4)", + "tenant-agent", "agent-tenant", "Agent Tenant", int64(1), + ); err != nil { + t.Fatalf("insert tenant: %v", err) + } + tenant := TenantID("tenant-agent") + tctx := WithTenant(ctx, tenant) + + owner, err := s.CreateUser(tctx, NewUser{Handle: "agent-owner", DisplayName: "Owner"}) + if err != nil { + t.Fatalf("CreateUser(owner): %v", err) + } + agent, err := s.CreateAgent(tctx, owner.ID, NewAgent{Handle: "worker", DisplayName: "Worker"}) + if err != nil { + t.Fatalf("CreateAgent(with tenant): %v", err) + } + if got := tenantOf(t, s, agent.ID); got != string(tenant) { + t.Fatalf("tenant-context CreateAgent stamped %q, want %q", got, tenant) + } +}