From f647cd60edbab0608e5bcc7de0e43f4bee160010 Mon Sep 17 00:00:00 2001 From: Cemil ILIK Date: Sat, 8 Aug 2026 13:04:59 +0300 Subject: [PATCH 01/29] docs: restructure the roadmap around the one-way-door test A four-report audit of the corpus surfaced three mechanisms that do not work as written, all inside the two subsystems the project calls non-negotiable, plus a sequencing problem: the control plane was being built before the product it controls. Correctness, moved earlier: - ADR-0003 Amendment 3 corrects the RLS policy template. It issued two permissive policies, which PostgreSQL combines with OR, so every tenant-wide row (organization_id IS NULL) was visible to every tenant. The corrected template is one AND-ed policy with FORCE ROW LEVEL SECURITY, an explicit WITH CHECK, and a four-role model. It now lives in exactly one place -- Standards 05 -- instead of four divergent copies. Isolation tests run as learnstack_app, because a test that connects as the table owner passes against inert policies. - ADR-0033 supersedes ADR-0016. MUST-class audit is written as a durable intent inside the business transaction and fails closed; SHOULD/MAY stays best-effort. This resolves the contradiction between Standards 18 and ADR-0016, and prevents the corrected RLS policy from silently rejecting every audit insert. Also corrects the audit_log DDL, which declared a primary key twice. - ADR-0034 replaces the "closed at four endpoints" rule with two enforceable invariants. The count was never true, and protecting it drove TLS private keys into the cached entitlement payload. Host resolution never calls the Hub. Additive infrastructure, moved later: - ADR-0035 codifies the one-way-door test. Dapr, Kafka, APISIX, Vault, the Hub integration, signed licence keys, custom-domain TLS automation and audit_log partitioning each ship as a port plus a working default now, and a vendor adapter on a written trigger in Phase 11. - Standards 00 gains the test itself, plus a rule that an uncontracted deployment mode may not decide a technical question. Proof, moved earlier: - Phase 02d is new: a two-tenant walking skeleton that renders two different education sites from one binary. The second tenant moves out of Phase 10 into Phase 02a Packet 7, where two tenants already exist for isolation testing, so genericity is tested continuously rather than at the end. - Phase 02a packets 4-10 are re-scoped and a Packet 3b repairs what Packets 2 and 3 left behind. Packets 0-3 are shipped; their records are unchanged. Every phase now carries a Phase Exit Decision -- nine were missing. The Hub repository owns its own roadmap; phase-02c keeps only LearnStack's side of the boundary, and phase-09b and phase-12 become pointers. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/skills/add-architecture-test/SKILL.md | 2 +- .claude/skills/code-review/SKILL.md | 3 +- .claude/skills/commit-and-pr/SKILL.md | 4 +- .claude/skills/standards-check/SKILL.md | 2 +- .github/CONTRIBUTING.md | 2 +- AGENTS.md | 2 +- CLAUDE.md | 194 +++- README.md | 159 +-- docs/architecture/01-platform-vision.md | 128 ++- docs/architecture/09-tenant-isolation.md | 53 +- docs/architecture/15-event-and-outbox.md | 466 ++++++--- docs/architecture/23-data-protection.md | 7 +- docs/architecture/24-learnstack-hub.md | 89 +- docs/architecture/25-deployment-models.md | 96 +- docs/architecture/26-hybrid-license-model.md | 232 ++++- docs/architecture/27-custom-domain-tls.md | 163 +++- .../28-platform-tenant-organization.md | 18 +- docs/architecture/30-api-gateway.md | 227 ++++- docs/architecture/31-audit-subsystem.md | 357 +++++-- .../32-tenant-customization-model.md | 293 +++++- .../architecture/33-cross-cutting-concerns.md | 27 +- .../0003-tenant-isolation-defense-in-depth.md | 122 ++- docs/decisions/0016-audit-log-subsystem.md | 19 +- .../0017-tenant-organization-hierarchy.md | 25 +- .../0018-tenant-driven-customization-model.md | 43 + docs/decisions/0019-learnstack-hub.md | 19 +- docs/decisions/0022-custom-domain-tls.md | 25 +- .../0028-audit-log-partition-management.md | 30 +- ...tion-handling-logging-and-observability.md | 24 + docs/decisions/0033-audit-durability-model.md | 198 ++++ .../0034-hub-contract-surface-invariant.md | 200 ++++ .../0035-demand-gated-infrastructure.md | 180 ++++ docs/decisions/README.md | 18 +- docs/glossary.md | 45 +- docs/roadmap/README.md | 234 +++-- docs/roadmap/phase-02a-kernel-tenancy.md | 908 ++++++++++++------ docs/roadmap/phase-02b-events-auth.md | 535 ++++++++--- docs/roadmap/phase-02c-hub-foundation.md | 579 ++++++----- docs/roadmap/phase-02d-walking-skeleton.md | 167 ++++ docs/roadmap/phase-03-identity-admin.md | 479 +++++++-- docs/roadmap/phase-04-cms-media-pages.md | 482 +++++++--- .../phase-05-education-learning-content.md | 390 ++++++-- .../roadmap/phase-06-renderer-admin-studio.md | 356 +++++-- .../phase-07-enrollment-learner-portal.md | 249 ++++- .../phase-08a-assessment-notifications.md | 223 ++++- docs/roadmap/phase-08b-scheduling.md | 31 +- docs/roadmap/phase-08c-classroom.md | 321 +++++-- ...phase-09-billing-integrations-analytics.md | 364 +++++-- docs/roadmap/phase-09b-hub-billing.md | 173 +--- docs/roadmap/phase-10-english-learning-mvp.md | 384 ++++---- docs/roadmap/phase-11-production-hardening.md | 479 +++++++-- docs/roadmap/phase-12-hub-marketplace.md | 142 ++- docs/standards/00-principles.md | 64 ++ docs/standards/01-architecture-standards.md | 6 +- docs/standards/02-backend-coding.md | 66 +- docs/standards/05-database.md | 123 ++- docs/standards/11-security.md | 150 ++- docs/standards/13-documentation.md | 2 +- docs/standards/14-git-workflow.md | 4 +- docs/standards/18-audit-coverage.md | 16 +- docs/standards/20-infrastructure-stack.md | 195 +++- .../21-architecture-tests-catalogue.md | 710 ++++++++++++-- docs/standards/README.md | 69 +- 63 files changed, 8685 insertions(+), 2688 deletions(-) create mode 100644 docs/decisions/0033-audit-durability-model.md create mode 100644 docs/decisions/0034-hub-contract-surface-invariant.md create mode 100644 docs/decisions/0035-demand-gated-infrastructure.md create mode 100644 docs/roadmap/phase-02d-walking-skeleton.md diff --git a/.claude/skills/add-architecture-test/SKILL.md b/.claude/skills/add-architecture-test/SKILL.md index a7146b1..e792e12 100644 --- a/.claude/skills/add-architecture-test/SKILL.md +++ b/.claude/skills/add-architecture-test/SKILL.md @@ -120,7 +120,7 @@ The current set lives across these files; add yours to the right one: | `AuditTests.cs` | `AuditEntry` inheritance, no direct `audit_log` writes. | | `EntitlementTests.cs` | Plan-projected vs tenant-flag separation, FeatureKey registry. | | `PermissionTests.cs` | Closed action set, scope correctness, denied-test presence. | -| `HubContractTests.cs` | No direct Hub-URL references, four-endpoint surface enforcement. | +| `HubContractTests.cs` | No direct Hub-URL references; Hub clients only inside the named adapters (ADR-0034). | | `DomainGenericTests.cs` | `Core_Modules_HaveNo_DomainSpecific_Names`, no `Verticals/`. | | `DaprDirectInjectionTests.cs` | No `IConnectionMultiplexer` / `KafkaProducer` / `VaultClient` in modules. | | `ConventionTests.cs` | Strongly-typed ids in commands, validator pairing, etc. | diff --git a/.claude/skills/code-review/SKILL.md b/.claude/skills/code-review/SKILL.md index ff0d95d..4557684 100644 --- a/.claude/skills/code-review/SKILL.md +++ b/.claude/skills/code-review/SKILL.md @@ -262,7 +262,8 @@ Author intent: `Asana`, `Kyu`, `CodeChallenge` — all forbidden; live as `TenantContentType` / `TenantLevelTaxonomy` / `TenantScoringRule` data). - **No `Verticals/` folder.** ADR-0018 superseded ADR-0011. -- **Hub HTTPS contract surface closed at four endpoints.** Adding a fifth +- **Hub HTTPS contract surface governed by two invariants** (the Hub stores no tenant + content; every crossing goes through a named adapter — ADR-0034). Adding an endpoint requires a new ADR. - **No direct `IConnectionMultiplexer` / `IDistributedCache` / `KafkaProducer` / `VaultClient` injection** (per [CLAUDE.md hard rules](../../../CLAUDE.md) and diff --git a/.claude/skills/commit-and-pr/SKILL.md b/.claude/skills/commit-and-pr/SKILL.md index a06e841..c9414e9 100644 --- a/.claude/skills/commit-and-pr/SKILL.md +++ b/.claude/skills/commit-and-pr/SKILL.md @@ -91,7 +91,7 @@ Trailers go at the **end** of the body (after a blank line). The supported set: Pick the trailer that matches the agent that did material work: - Claude Code session: - `Co-Authored-By: Claude Opus 4.7 (1M context) ` + `Co-Authored-By: Claude Opus 5 (1M context) ` - OpenAI Codex session: `Co-Authored-By: Codex Opus 4.7 (1M context) ` @@ -110,7 +110,7 @@ access shares the per-learner entitlement read path with free access. ADR: 0010 Module: Enrollment, Billing -Co-Authored-By: Claude Opus 4.7 (1M context) +Co-Authored-By: Claude Opus 5 (1M context) EOF )" ``` diff --git a/.claude/skills/standards-check/SKILL.md b/.claude/skills/standards-check/SKILL.md index 1a76c18..8db099b 100644 --- a/.claude/skills/standards-check/SKILL.md +++ b/.claude/skills/standards-check/SKILL.md @@ -332,7 +332,7 @@ checklist: `docs/modules//permissions.md` updated. - [ ] Frontend i18n key changed → `I18n:` commit trailer present. - [ ] Hub-side change in this repo → flagged for coordination with - `learnstack-hub` repo (the four-endpoint contract is shared). + `learnstack-hub` repo (the contract surface is shared; see ADR-0034). ### Step 6 — Output diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index fc38172..f539302 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -44,7 +44,7 @@ Per CLAUDE.md § Commit conventions: - **Conventional Commits**: `type(scope): subject` with subject in imperative mood, ≤ 72 chars. - AI-assisted commits carry the trailer - `Co-Authored-By: Claude Opus 4.7 (1M context) ` + `Co-Authored-By: Claude Opus 5 (1M context) ` (replace the model name when authoring with a different assistant). - `docs(scope)` for doc-only commits; scope ∈ `architecture | decisions | standards | roadmap` or omitted for cross-cutting changes. diff --git a/AGENTS.md b/AGENTS.md index c19a110..c3b1f64 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ There are no rule differences. The only thing that varies between agent runtimes the **`Co-Authored-By` commit trailer**, which names the assistant that contributed: - Claude Code sessions: - `Co-Authored-By: Claude Opus 4.7 (1M context) ` + `Co-Authored-By: Claude Opus 5 (1M context) ` - OpenAI Codex sessions: `Co-Authored-By: Codex Opus 4.7 (1M context) ` diff --git a/CLAUDE.md b/CLAUDE.md index 423134e..09c99e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,47 +6,91 @@ the conventions you must follow when contributing. ## What this is -LearnStack is a **multi-tenant core platform for building education -products** — not a single LMS. It powers arbitrarily-domained education -products (English-learning, yoga, coding bootcamps, music schools, -driving schools, …) on the **same code paths**; the difference between -them is **tenant customization data** loaded at provisioning, not code. -The first showcase tenant is an online English-learning platform -(Phase 10); the substrate-genericity proof is a second non-English -tenant running the same code paths. +LearnStack is a **white-label platform for multi-branch education +businesses that teach live** — not a single LMS, and not an education +product of its own. One binary, one schema, and one set of container +images serve a language school, a yoga studio, a music school, or a +coding bootcamp. What differs between them is **tenant customization +data** loaded at provisioning, not code +([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)). + +That claim has a stated edge, and the edge lives in exactly one place: +[Platform Vision § Genericity boundary](docs/architecture/01-platform-vision.md). +Content shape, presentation, and pure rule evaluation are tenant data. +Stateful entitlement (credit packs, session quotas) and external +capability invocation (running submitted code, scoring speech) are +platform features gated by plan — they need a release, not a +customization row. Link to that section; do not restate it. LearnStack ships in three production deployment modes — SaaS, Dedicated, -Self-Hosted — backed by the companion **`learnstack-hub`** repository -(separate repo, see [ADR-0019](docs/decisions/0019-learnstack-hub.md)). -The Hub repo lives at sibling path `../learnstack-hub` on developer -workstations; GitHub: https://github.com/cemililik/LearnStack-Hub. -Phase 02c P02c-0 (Hub repo bootstrap) shipped 2026-05-21 — see the -Phase 02c roadmap doc for the per-packet status. +Self-Hosted — backed by the companion **LearnStack Hub** control plane +(separate repository, see +[ADR-0019](docs/decisions/0019-learnstack-hub.md)). On developer +workstations the Hub repo is the sibling directory `../LearnStack-Hub`; +GitHub: https://github.com/cemililik/LearnStack-Hub. The Hub repository +**owns its own roadmap** at `../LearnStack-Hub/docs/roadmap/`; this +repository holds only LearnStack's side of the boundary, in +[Phase 02c](docs/roadmap/phase-02c-hub-foundation.md). ## What state this is in -**Phase 01 complete — repository scaffolding, local infrastructure, DX, -and CI baseline. No domain code yet — Phase 02a starts that.** - -What shipped: the .NET 10 solution scaffold under `backend/` (core + 7 -modules × 4 projects + 4 test projects including the non-skippable -`LearnStack.Tests.Architecture`), the `pnpm` frontend monorepo under -`frontend/` (`apps/web` Next.js App Router + `packages/{config,ui,sdk}`), -the full local-dev compose stack at `infra/compose/dev.yml` — PostgreSQL -18, Valkey, SeaweedFS, Mailpit, Meilisearch, Keycloak (two realms), -LiveKit OSS + Coturn, Kafka + kafka-ui, Vault, Dapr sidecar + placement, -APISIX in file-driven standalone mode — and the DX + CI surround -(repo-root `Makefile`, `.env.example` single source of truth, -`.githooks/pre-commit` formatter + Leakwatch, `infra/compose/e2e.yml` -ephemeral overlay, `.github/workflows/ci.yml` with backend + frontend + -meta + secret-scan required checks, `scripts/seed.sh`). - -Every module assembly is empty of domain code today. Module-level +**Phase 01 complete. +[Phase 02a](docs/roadmap/phase-02a-kernel-tenancy.md) in progress — +packets 0–3 shipped; packets 3b–10 were re-scoped on 2026-08-08 after a +four-report audit of the corpus.** + +**Phase 01** shipped the .NET 10 solution scaffold under `backend/` +(core + 7 modules × 4 projects + 4 test projects including the +non-skippable `LearnStack.Tests.Architecture`), the `pnpm` frontend +monorepo under `frontend/` (`apps/web` Next.js App Router + +`packages/{config,ui,sdk}`), the local-dev compose stack at +`infra/compose/dev.yml`, and the DX + CI surround (repo-root `Makefile`, +`.env.example` single source of truth, `.githooks/pre-commit` formatter + +Leakwatch, `infra/compose/e2e.yml` ephemeral overlay, +`.github/workflows/ci.yml` with backend + frontend + meta + secret-scan +required checks, `scripts/seed.sh`). + +**Phase 02a packets 0–3** shipped the decision set (Vogen, API +versioning, audit partition management), the shared kernel core +(`Result` + `LocalizedMessage`, `Entity` / `AuditableEntity`, +domain events, cursor pagination, `IClock` / `IRandom` / `IGuidFactory`), +and the [ADR-0032](docs/decisions/0032-exception-handling-logging-and-observability.md) +cross-cutting foundation (L1 `IExceptionHandler`, the eight-step MediatR +pipeline, Serilog → OTLP, `TenantContextSpanProcessor`, +`IErrorTrackingProvider`, `IProviderResilience`, the `LS0001` +analyzer). Those records are frozen delivery history. + +**The 2026-08-08 restructure** re-scoped packets 3b–10 along three lines, +all recorded in the Phase 02a Status block: + +- *Correctness moved earlier.* The Row Level Security template that four + documents carried produced two **permissive** policies, which + PostgreSQL combines with `OR` — leaking every tenant-wide row across + tenants. [ADR-0003 Amendment 3](docs/decisions/0003-tenant-isolation-defense-in-depth.md) + corrects it, and [ADR-0033](docs/decisions/0033-audit-durability-model.md) + makes MUST-class audit a durable intent inside the business + transaction — which is also what stops the corrected policy from + rejecting every audit insert. +- *Additive infrastructure moved later.* Per + [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md), Packet 5 + ships the foundation **ports and their default implementations**; the + Dapr, Kafka, APISIX and Vault adapters land in + [Phase 11](docs/roadmap/phase-11-production-hardening.md) against + written triggers. +- *Proof moved earlier.* Two seed tenants in unrelated domains land in + Packet 7, and [Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md) + renders both of them in a browser. + +**[Phase 02d: Two-Tenant Walking Skeleton](docs/roadmap/phase-02d-walking-skeleton.md) +is the next user-visible milestone** — the first phase whose output +someone who does not read C# can evaluate: two hosts, two tenants, two +education sites, one binary and one database. + +Every module assembly is still empty of domain code. Module-level references in the docs (e.g. `LearnStack.Modules.Education.Application`, `ILiveClassProvider`, `ITenantSearch`) describe **intended** shape that -the corpus anchors against — Phase 02a (Platform Kernel + -Multi-Tenancy) is where those types actually land. Phase 02c (Hub -Foundation, separate `learnstack-hub` repo) runs in parallel. +the corpus anchors against; Phase 02a packets 6–9 and Phase 02d are +where the first of those types actually land. ## Where to start @@ -55,10 +99,19 @@ For any task, read in this order: 1. [README.md](README.md) — direction at a glance. 2. [docs/architecture/01-platform-vision.md](docs/architecture/01-platform-vision.md) — what we build and why. 3. [docs/architecture/05-mvp-scope.md](docs/architecture/05-mvp-scope.md) — what is in / out / deferred. -4. [docs/roadmap/README.md](docs/roadmap/README.md) — phased plan with explicit dependencies. +4. [docs/roadmap/README.md](docs/roadmap/README.md) — phased plan with explicit dependencies and the one-way-door sequencing principle. 5. [docs/standards/00-principles.md](docs/standards/00-principles.md) — the beliefs every other standard descends from. 6. [docs/glossary.md](docs/glossary.md) — terminology; the single source of truth for project-specific terms. +Then read the two phases that are live: + +- [docs/roadmap/phase-02a-kernel-tenancy.md](docs/roadmap/phase-02a-kernel-tenancy.md) + — the current phase, with a dated Status block listing every packet. +- [docs/roadmap/phase-02d-walking-skeleton.md](docs/roadmap/phase-02d-walking-skeleton.md) + — what Phase 02a is building toward. `02d` sorts after `02b`/`02c` but + runs **before** them; the roadmap dependency map is authoritative for + order, filename order is not. + Once the high-level reading is done, pick **exactly one** skill entry point based on the user's intent. The entry point dispatches the rest internally; do not chain entry points yourself. @@ -82,8 +135,8 @@ let the entry point pick it. |-----------|---------|------------| | `docs/architecture/` | Conceptual descriptions of what we are building. Numbered `NN-topic.md` linearly. | Editable as the system evolves. | | `docs/decisions/` | ADRs — one-time decisions with status, context, decision, consequences. Redirect / superseded ADRs live under `_redirects/`. | Accepted ADRs are immutable except for dated Amendments. | -| `docs/standards/` | Engineering rules (`NN-topic.md`, 00 – 20). Each anchored standard carries a `**Derives from:** ADR-NNNN` header. | Editable as the team learns; standard changes cite an ADR. | -| `docs/roadmap/` | Phased plan (`phase-NN-topic.md`, 00 – 12 with 02a/02b/02c, 08a/08b/08c, and 09/09b splits). | Editable per phase. | +| `docs/standards/` | Engineering rules (`NN-topic.md`, 00 – 21). Each anchored standard carries a `**Derives from:** ADR-NNNN` header. | Editable as the team learns; standard changes cite an ADR. | +| `docs/roadmap/` | Phased plan (`phase-NN-topic.md`, 00 – 12 with 02a/02b/02c/**02d**, 08a/08b/08c, and 09/09b splits). Every phase doc carries the same six sections: Goal, Scope, Deliverables, Completion Criteria, Risks, Phase Exit Decision. | Editable per phase; the Status block of a shipped packet is a dated delivery record and is not rewritten. | | `docs/glossary.md` | Terminology source of truth. | Editable; new term goes here first, then used. | > `docs/analysis/` exists locally but is **gitignored** — it is a private scratchpad @@ -98,14 +151,17 @@ let the entry point pick it. - **Single source of truth.** Each piece of knowledge lives in exactly one place. The glossary holds terms. ADRs hold decisions. Standards hold ongoing rules. Architecture docs hold conceptual descriptions. Roadmap holds phases. Do not duplicate. - **ADR numbers are sequential and never reused.** Superseded ADRs become redirect stubs under `decisions/_redirects/`. Adding a new ADR uses the next free number. - **Standards changes cite an ADR.** A new standard rule or a change to an existing one is paired with an ADR when the rule is non-trivial. -- **Modular monolith with four cross-module mechanisms** ([ADR-0010](docs/decisions/0010-cross-module-communication.md)): application contract, intra-module domain event, integration event via outbox (dispatched through Dapr pub/sub per Amendment 1), read-model projection. No fifth. +- **Modular monolith with four cross-module mechanisms** ([ADR-0010](docs/decisions/0010-cross-module-communication.md)): application contract, intra-module domain event, integration event via outbox (dispatched through `IEventBus` — `InProcessEventBus` today, the Dapr/Kafka adapter on its trigger), read-model projection. No fifth. - **Tenant + organization isolation is defense-in-depth from day one** ([ADR-0003 Amendment 1](docs/decisions/0003-tenant-isolation-defense-in-depth.md), [ADR-0017](docs/decisions/0017-tenant-organization-hierarchy.md)): tenant + organization context + EF query filters + PostgreSQL RLS + architecture tests. -- **Self-hosted infrastructure preferred** for Keycloak (auth, with two realms — `learnstack` + `learnstack-hub`), LiveKit OSS (live classroom), SeaweedFS (object storage), Meilisearch (search), Kafka (pub/sub backend), Vault (secrets). See ADRs 0004, 0005, 0014. -- **The core platform stays domain-generic.** Domain-specific shapes (CEFR levels, English placement-test scoring, kyu/dan ranks, yoga asana catalogs, …) live as **tenant customization data** ([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)), never as code in any module. There is no `Verticals/` folder. ADR-0011 is superseded. -- **Foundation building blocks are Day-1, not Phase-11.** Dapr (`IEventBus`/`ICacheService`/`ISecretProvider`), APISIX gateway, audit infrastructure, organization scope, entitlement projection socket, host-to-tenant resolver, and architecture tests all ship in Phase 02a — not as later hardening. -- **Provider adapters everywhere.** Payments, auth, storage, search, live classroom, notifications, **event bus, cache, secrets, Hub HTTPS contract, entitlement source, host resolver** — all sit behind interfaces. No SaaS lock-in in `Domain` or `Application`. See [20-infrastructure-stack.md](docs/standards/20-infrastructure-stack.md). -- **Hub HTTPS contract surface is closed at four endpoints.** Adding a fifth requires a new ADR. See [ADR-0019](docs/decisions/0019-learnstack-hub.md). -- **Three deployment modes, one binary.** `SaaS` / `Dedicated` / `SelfHosted` selection happens at composition root via `DeploymentMode`; module code never branches on the mode. See [ADR-0020](docs/decisions/0020-triple-deployment-hybrid-license.md). +- **One canonical RLS template, in one file.** The corrected policy shape — one `AND`-ed policy per table, `ENABLE` **and** `FORCE ROW LEVEL SECURITY`, an explicit `WITH CHECK`, and the four-role model (`learnstack_migration` owns, `learnstack_app` connects with `NOBYPASSRLS`, `learnstack_platform` and `learnstack_outbox_admin` hold audited bypasses) — is decided in [ADR-0003 Amendment 3](docs/decisions/0003-tenant-isolation-defense-in-depth.md) and written as SQL in exactly one document: [Database Standards](docs/standards/05-database.md). Every other document links there. The superseded template lived in four documents and was wrong in all four — two *permissive* policies, which PostgreSQL combines with `OR`, so every tenant-wide row was visible across tenants. +- **Self-hosted infrastructure preferred** for Keycloak (auth, with two realms — `learnstack` + `learnstack-hub`), LiveKit OSS (live classroom), SeaweedFS (object storage), Meilisearch (search), Kafka (pub/sub backend), Vault (secrets). See ADRs 0004, 0005, 0014. **What** LearnStack uses is settled; **when** each arrives is [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)'s trigger table. +- **The core platform stays domain-generic.** Domain-specific shapes (CEFR levels, English placement-test scoring, kyu/dan ranks, yoga asana catalogs, …) live as **tenant customization data** ([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)), never as code in any module. There is no `Verticals/` folder. ADR-0011 is superseded. The boundary of that claim is in [Platform Vision § Genericity boundary](docs/architecture/01-platform-vision.md). +- **Irreversible now, additive on demand — the one-way-door test** ([ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)): *if I add this six months from now, will I have to touch code that is already written?* + - **Yes → ship it now.** Tenant + organization isolation, the corrected RLS policies, the `outbox_messages` table and its ownership, strongly-typed identifiers, the localization schema, MUST-class audit durability, module boundaries and their architecture tests. These touch every query, every migration, and every job payload. + - **No → ship the port now, the adapter on a named trigger.** Dapr pub/sub, Kafka, Valkey-backed cache, Vault, APISIX, the Hub entitlement source, signed licence keys, custom-domain TLS automation, `audit_log` partitioning. Each has a port in `LearnStack.SharedKernel`, a working default implementation (`InProcessEventBus`, `InMemoryCacheService`, `EnvironmentSecretProvider`, `NullEntitlementProvider`), an owning phase, and a written trigger condition. A building block missing any of those four is not demand-gated — it is missing. +- **Provider adapters everywhere.** Payments, auth, storage, search, live classroom, notifications, **event bus, cache, secrets, Hub contract, entitlement source, host resolver** — all sit behind interfaces. No SaaS lock-in in `Domain` or `Application`. See [20-infrastructure-stack.md](docs/standards/20-infrastructure-stack.md). +- **The Hub contract is governed by two invariants, not by a count** ([ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md)): (1) the Hub stores **no tenant content** — courses, lessons, learners, enrollments, sessions and media live only in LearnStack, and the Hub holds tenant *metadata* only; (2) **every LearnStack↔Hub crossing goes through a named adapter** — `IEntitlementProvider`, `IUsageReporter`, `IHubTenantSync`, and nothing else may hold a Hub client. Adding an endpoint still requires an ADR, because the surface is a cross-repository contract both repositories have to agree on. +- **One binary, five `DeploymentMode` values, two of them wired.** Selection happens at the composition root; module code never branches on the mode ([ADR-0020](docs/decisions/0020-triple-deployment-hybrid-license.md), enforced by `Modules_Do_Not_Reference_DeploymentMode`). `Development` and `SaaS` are wired end to end; `Dedicated`, `SelfHostedOnline` and `SelfHostedAirGapped` are **prepared seams, not supported deployments**, until [Phase 11](docs/roadmap/phase-11-production-hardening.md) builds their adapters and integration suites. ## Conventions when editing docs @@ -128,7 +184,8 @@ rules: `frontend/apps/web` with route segments ([03](docs/standards/03-frontend-coding.md), [07](docs/standards/07-frontend-architecture.md)). The operator portal - (`learnstack-hub-web`) is a separate app in the `learnstack-hub` repo. + is a separate app, `frontend/apps/operator-portal`, in the + `LearnStack-Hub` repository. - REST + RFC 7807 Problem Details + cursor pagination + idempotency keys + ETag concurrency ([04](docs/standards/04-api-design.md)). - OpenTelemetry + correlation id end to end ([10](docs/standards/10-observability.md)). @@ -136,9 +193,12 @@ rules: - Audit-coverage matrix required per module ([18](docs/standards/18-audit-coverage.md)). - Permission keys `{module}.{resource}.{action}` with closed action set + scope (Platform / Tenant / Organization) ([19](docs/standards/19-permissions.md)). -- Infrastructure-stack rules (Dapr building blocks, APISIX, Hub HTTPS contract, - outbox + inbox, entitlement projection) in - [20](docs/standards/20-infrastructure-stack.md). +- Infrastructure-stack rules (foundation ports and their default + implementations, the Hub contract surface, outbox + inbox, entitlement + projection) in [20](docs/standards/20-infrastructure-stack.md). +- The architecture-test catalogue in + [21](docs/standards/21-architecture-tests-catalogue.md) — canonical rule + names live there; do not invent a second spelling. - Zero-tolerance review blockers enumerated in [17](docs/standards/17-code-review.md). ## Commit conventions @@ -149,7 +209,7 @@ rules: `architecture`, `decisions`, `standards`, `roadmap`, or omitted for cross-cutting changes. - Commits made with AI assistance carry the trailer - `Co-Authored-By: Claude Opus 4.7 (1M context) `. + `Co-Authored-By: Claude Opus 5 (1M context) `. ## Things to never do @@ -162,9 +222,37 @@ rules: [ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md). There is no `Verticals/` folder; the architecture test `No_Source_Folder_Named_Verticals` enforces it. -- Add a fifth endpoint to the Hub HTTPS contract surface without an ADR. +- Add an endpoint to the Hub contract surface without an ADR. The count + is not the rule — [ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md)'s + two invariants are — but the surface is a cross-repository contract, so + it changes by decision record, in both repositories, or not at all. - Call Hub endpoints from anywhere except the dedicated `IEntitlementProvider` / `IUsageReporter` / `IHubTenantSync` adapters. +- Resolve a host by calling the Hub. `IHostToTenantResolver` reads + `platform_host_to_tenant` and nothing else + ([ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md)); an + anonymous page load must never depend on a control plane being + reachable. +- Carry TLS certificates or private keys in the entitlement payload. Cert + material moves by secret-store replication and is referenced by path + from `PUT /api/internal/tenants/{id}/host-mappings`, never by value + through a payload LearnStack caches, logs, audits and mirrors. +- Copy the RLS template into a second document. It lives only in + [Database Standards](docs/standards/05-database.md); everywhere else + links to it. The last duplication shipped a broken policy into four + files at once. +- Run a tenant- or organization-isolation test as the table owner or as a + `BYPASSRLS` role. Isolation tests connect as **`learnstack_app`** — a + test that runs as `learnstack_migration`, `learnstack_platform` or + `learnstack_outbox_admin` passes even when every policy is inert, and + therefore proves nothing. +- Write a MUST-class audit row outside the business transaction. MUST-class + audit is a **durable intent enrolled in the same `SaveChanges` as the + state change it describes** ([ADR-0033](docs/decisions/0033-audit-durability-model.md)), + so it commits with that change or not at all — and so it executes while + `app.tenant_id` is set and RLS accepts it. A tenant `AuditConfig` may + narrow SHOULD/MAY coverage but never removes baseline MUST coverage, and + a config-store read failure **fails closed**. - Inject `IConnectionMultiplexer` / `IDistributedCache` / `KafkaProducer` / `VaultClient` directly — use `IEventBus` / `ICacheService` / `ISecretProvider`. @@ -180,7 +268,11 @@ rules: the existing documents ambiguous about ownership; if a topic needs more space, expand the existing doc rather than splintering. - Mention a feature as "deferred to a later phase" without naming the - phase that owns it. + phase that owns it. For an infrastructure building block the bar is + higher: name the **port**, the **default implementation**, the **owning + phase**, and the **trigger condition** + ([ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)). Three + out of four is not demand-gating. - Throw `DomainException` for expected business-rule violations — use `Result.Fail(business_rule_violation, ...)`. `DomainException` is reserved for programmer errors / aggregate invariant diff --git a/README.md b/README.md index dd8b498..a1bd631 100644 --- a/README.md +++ b/README.md @@ -1,46 +1,61 @@ # LearnStack -LearnStack is a **multi-tenant core platform for building education products** — not a -single LMS. It is an education-aware CMS and platform engine that powers different -learning brands, landing pages, catalogs, portals, and **domain-agnostic** education -products. The same code paths serve an online English-learning brand, a yoga studio, -a coding bootcamp, a music school, or a driving school — the difference between them -is **tenant customization data** loaded at provisioning, not compiled code. - -The first showcase tenant happens to be an online English-learning platform (Phase 10); -the substrate-genericity proof is a second, non-English tenant running the same code -paths against its own customization data. +LearnStack is a **white-label platform for multi-branch education businesses that teach +live** — not a single LMS, and not an education product of its own. It is an +education-aware CMS, learning engine, and platform foundation that powers different +brands, landing pages, catalogs, and learner portals. The same code paths serve a +language school, a yoga studio, a music school, or a coding bootcamp — the difference +between them is **tenant customization data** loaded at provisioning, not compiled code +([ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md)). + +That claim has a stated edge: content shape, presentation and pure rule evaluation are +tenant data; stateful entitlement and external capability invocation are platform +features gated by plan. See +[Platform Vision § Genericity boundary](docs/architecture/01-platform-vision.md). + +Two tenants in unrelated domains exist from +[Phase 02a Packet 7](docs/roadmap/phase-02a-kernel-tenancy.md) onward and are both +rendered in a browser in [Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md), so +genericity is tested continuously rather than asserted and checked once at the end. LearnStack ships in three production deployment modes — SaaS, Dedicated, Self-Hosted — -backed by the companion **`learnstack-hub`** repository which provides the SaaS / +backed by the companion **LearnStack Hub** repository, which provides the SaaS / Dedicated control plane, plan editor, custom-domain admin, and license-key issuance. -The Hub repo is expected to live at `../learnstack-hub` (sibling to this repo on the -developer's workstation) so the cross-repo doc cross-links resolve. See -[learnstack-hub on GitHub](https://github.com/cemililik/LearnStack-Hub) and -[docs/roadmap/phase-02c-hub-foundation.md](docs/roadmap/phase-02c-hub-foundation.md). +The Hub repo is expected to live at `../LearnStack-Hub` (sibling to this repo on the +developer's workstation) so the cross-repo doc links resolve. The Hub repository owns +its own roadmap at `../LearnStack-Hub/docs/roadmap/`. See +[LearnStack-Hub on GitHub](https://github.com/cemililik/LearnStack-Hub) and +[docs/roadmap/phase-02c-hub-foundation.md](docs/roadmap/phase-02c-hub-foundation.md) +for LearnStack's side of the boundary. ## Status -Phase 01 complete. The repository now holds the .NET 10 solution scaffold (7 -modules × 4 projects + 4 test projects with `No_Source_Folder_Named_Verticals` -architecture test), the `pnpm` frontend monorepo (`apps/web` + -`packages/{config,ui,sdk}`), the full local-dev `docker-compose` stack — -PostgreSQL 18, Valkey, SeaweedFS, Mailpit, Meilisearch, Keycloak (two realms), -LiveKit OSS + Coturn, Kafka + kafka-ui, Vault, Dapr sidecar + placement, APISIX -(file-driven standalone) — and the DX + CI surround: repo-root `Makefile`, -`.env.example` single source of truth, `.githooks/pre-commit` formatter, -`infra/compose/e2e.yml` ephemeral overlay, `.github/workflows/ci.yml`, and -`scripts/seed.sh`. See [docs/roadmap/phase-01-repository-tooling.md](docs/roadmap/phase-01-repository-tooling.md) -for the per-packet history. Phase 02a (Platform Kernel + Multi-Tenancy) is -underway; see [Phase 02a Status & Packets](docs/roadmap/phase-02a-kernel-tenancy.md) -for the 11-packet breakdown. Packets 0 (Kickoff) and 1 (Foundation decisions -— [ADR-0023](docs/decisions/0023-strongly-typed-id-source-generator.md) Vogen, +**Phase 01 complete. [Phase 02a](docs/roadmap/phase-02a-kernel-tenancy.md) in progress — +packets 0–3 shipped; packets 3b–10 re-scoped on 2026-08-08.** + +Phase 01 shipped the .NET 10 solution scaffold, the `pnpm` frontend monorepo +(`apps/web` + `packages/{config,ui,sdk}`), the local-dev `docker-compose` stack, and the +DX + CI surround. See +[phase-01-repository-tooling.md](docs/roadmap/phase-01-repository-tooling.md) for the +per-packet history. + +Phase 02a packets 0–3 shipped the foundation decisions +([ADR-0023](docs/decisions/0023-strongly-typed-id-source-generator.md) Vogen, [ADR-0024](docs/decisions/0024-api-versioning-policy.md) API versioning, -[ADR-0028](docs/decisions/0028-audit-log-partition-management.md) audit -partition mgmt) have shipped; Packet 2 — Shared Kernel core is next. -Phase 02c (Hub Foundation, parallel, separate repo) starts once the 02a -sockets it depends on are in place. +[ADR-0028](docs/decisions/0028-audit-log-partition-management.md) audit partition +management — whose *timing* later moved to Phase 11), the shared kernel core, and the +[ADR-0032](docs/decisions/0032-exception-handling-logging-and-observability.md) +cross-cutting foundation. + +The 2026-08-08 restructure moved correctness earlier (the corrected RLS template in +[ADR-0003 Amendment 3](docs/decisions/0003-tenant-isolation-defense-in-depth.md), durable +MUST-class audit in [ADR-0033](docs/decisions/0033-audit-durability-model.md)), moved +additive infrastructure later behind its ports +([ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md)), and moved the +genericity proof earlier — two seed tenants in Packet 7, rendered in a browser in +[Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md), the next user-visible +milestone. Module assemblies hold no domain code yet. ```bash make install # one-time: deps + git hooks @@ -48,38 +63,56 @@ make dev # bring local stack up make seed # verify health + print demo credentials ``` +> `make seed`'s health gate currently times out, because three compose services declare +> no healthcheck. [Phase 02a Packet 3b](docs/roadmap/phase-02a-kernel-tenancy.md) fixes +> it along with the rest of the Phase 01 development-loop debt. + ## Direction At A Glance - **Backend:** .NET 10, ASP.NET Core, Entity Framework Core, MediatR. -- **Database:** PostgreSQL 18, with Row-Level Security from day one. Tenant + **Organization** - defense in depth ([ADR-0003 Amendment 1](docs/decisions/0003-tenant-isolation-defense-in-depth.md), - [ADR-0017](docs/decisions/0017-tenant-organization-hierarchy.md)). -- **Cache / Pub-Sub / Secrets:** Valkey 8 (RESP-protocol Linux-Foundation BSD fork - per [ADR-0030](docs/decisions/0030-redis-compatible-store-valkey.md)), Kafka, - HashiCorp Vault — all accessed via **Dapr** building blocks (`IEventBus`, - `ICacheService`, `ISecretProvider`) per - [ADR-0014](docs/decisions/0014-adopt-dapr.md). -- **API Gateway:** **APISIX** in file-driven standalone (`data_plane`) mode per - [ADR-0015](docs/decisions/0015-api-gateway-apisix.md). +- **Database:** PostgreSQL 18, with Row-Level Security from day one. Tenant + + **Organization** defense in depth + ([ADR-0003](docs/decisions/0003-tenant-isolation-defense-in-depth.md) Amendment 1 for + organization scope, **Amendment 3** for the corrected policy template and the + four-role database model, + [ADR-0017](docs/decisions/0017-tenant-organization-hierarchy.md)). The canonical SQL + lives in exactly one file: + [Database Standards](docs/standards/05-database.md). +- **Foundation ports:** `IEventBus`, `ICacheService`, `ISecretProvider`, + `IEntitlementProvider`, `IHostToTenantResolver` in `LearnStack.SharedKernel`, each + with a working default implementation. Vendor adapters — Dapr + ([ADR-0014](docs/decisions/0014-adopt-dapr.md)), Kafka, Valkey + ([ADR-0030](docs/decisions/0030-redis-compatible-store-valkey.md)), Vault, APISIX + ([ADR-0015](docs/decisions/0015-api-gateway-apisix.md)) — are **demand-gated**: each + has an owning phase and a written trigger condition in + [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md). - **Object storage:** SeaweedFS locally, S3-compatible storage in production. -- **Search:** Meilisearch initially. +- **Search:** PostgreSQL full-text search first; Meilisearch behind `ITenantSearch` when + quality or scale requires it. - **Frontend:** Next.js 15 (App Router), TypeScript, React. **One** application - (`apps/web`) with route segments for public, studio, and portal — multi-app split - inside this repo deferred. The operator portal (`learnstack-hub-web`) lives in the - separate `learnstack-hub` repository. + (`apps/web`) with route segments for public, studio, and portal; a multi-app split + inside this repo is not planned before + [Phase 11](docs/roadmap/phase-11-production-hardening.md). The operator portal + (`frontend/apps/operator-portal`) lives in the separate `LearnStack-Hub` repository. - **Identity:** Self-hosted Keycloak with **two realms** — `learnstack` for tenant users, `learnstack-hub` for operators. - **Architecture:** Modular monolith with explicit module contracts. - **Tenant customization:** Per [ADR-0018](docs/decisions/0018-tenant-driven-customization-model.md), content types, page blocks, lesson item types, level taxonomies, scoring rules, completion rules, custom fields, and notification templates are **data** authored - by tenants, not code. The core stays generic. + by tenants, not code. The core stays generic, within the boundary the ADR's 2026-08-08 + amendment draws. - **Audit:** Append-only `LearnStack.Modules.Audit` with EF interceptor + MediatR - behavior + partitioned `audit_log` table per - [ADR-0016](docs/decisions/0016-audit-log-subsystem.md). + behavior. MUST-class audit is a durable intent written inside the business + transaction per [ADR-0033](docs/decisions/0033-audit-durability-model.md), which + supersedes ADR-0016; `audit_log` partitioning and retention land in + [Phase 11](docs/roadmap/phase-11-production-hardening.md). - **Entitlements:** Feature-based projection mirrored from the Hub per [ADR-0021](docs/decisions/0021-feature-based-entitlement.md); typed - `FeatureKeys` / `LimitKeys` registries. + `FeatureKeys` / `LimitKeys` registries. The Hub contract is governed by two + invariants — the Hub stores no tenant content, and every crossing goes through a + named adapter — per + [ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md). - **Live classroom:** In-app WebRTC; **self-hosted LiveKit OSS** is the default; LiveKit Cloud available behind the same `ILiveClassProvider` interface. A custom WebRTC SFU is explicitly out of scope. @@ -131,6 +164,7 @@ Platform substrate deep dives: - [30 — API Gateway (APISIX)](docs/architecture/30-api-gateway.md) - [31 — Audit Subsystem](docs/architecture/31-audit-subsystem.md) - [32 — Tenant Customization Model](docs/architecture/32-tenant-customization-model.md) +- [33 — Cross-Cutting Concerns](docs/architecture/33-cross-cutting-concerns.md) Decision context: - [18 — WebRTC Build vs Adopt](docs/architecture/18-webrtc-build-vs-adopt.md) @@ -138,16 +172,25 @@ Decision context: ### Decisions (`docs/decisions/`) - [ADR index](docs/decisions/README.md) — accepted decisions with their reasoning and - consequences. The 2026-05-18 redesign added ADRs 0014–0022. + consequences. The 2026-05-18 redesign added ADRs 0014–0022; the 2026-08-08 + restructure added [ADR-0033](docs/decisions/0033-audit-durability-model.md) (audit + durability, supersedes ADR-0016), + [ADR-0034](docs/decisions/0034-hub-contract-surface-invariant.md) (Hub contract + invariants) and + [ADR-0035](docs/decisions/0035-demand-gated-infrastructure.md) (demand-gated + infrastructure). ### Engineering Standards (`docs/standards/`) - [Standards index](docs/standards/README.md) — rules that apply to every PR: coding, testing, security, observability, accessibility, performance, - [infrastructure-stack rules](docs/standards/20-infrastructure-stack.md), and more. + [infrastructure-stack rules](docs/standards/20-infrastructure-stack.md), and the + [architecture-test catalogue](docs/standards/21-architecture-tests-catalogue.md). ### Roadmap (`docs/roadmap/`) -- [Phased roadmap](docs/roadmap/README.md) — phases 00 through 12, including the - Phase 02c (Hub Foundation) and Phase 09b (Hub Billing) parallel tracks. +- [Phased roadmap](docs/roadmap/README.md) — phases 00 through 12, including + [Phase 02d](docs/roadmap/phase-02d-walking-skeleton.md) (Two-Tenant Walking Skeleton) + and the Phase 02c (Hub Integration) and Phase 09b (Hub Billing) parallel tracks. The + dependency map there is authoritative for order; filename order is not. ### Reference - [Glossary](docs/glossary.md) — canonical definitions. @@ -168,7 +211,11 @@ For the strategy and the headline decisions: 1. [Platform Vision](docs/architecture/01-platform-vision.md) 2. [MVP Scope](docs/architecture/05-mvp-scope.md) 3. [Roadmap overview](docs/roadmap/README.md) -4. [ADR index](docs/decisions/README.md) +4. [Phase 02a: Kernel + Tenancy](docs/roadmap/phase-02a-kernel-tenancy.md) — where the + work is now +5. [Phase 02d: Two-Tenant Walking Skeleton](docs/roadmap/phase-02d-walking-skeleton.md) + — where it is going next +6. [ADR index](docs/decisions/README.md) For the technical foundations: 1. [Technical Architecture](docs/architecture/04-technical-architecture.md) diff --git a/docs/architecture/01-platform-vision.md b/docs/architecture/01-platform-vision.md index 8a82cce..8bcbc49 100644 --- a/docs/architecture/01-platform-vision.md +++ b/docs/architecture/01-platform-vision.md @@ -1,23 +1,42 @@ # Platform Vision -LearnStack is a **PaaS for education** — a platform on which customers build their own -education platforms in arbitrary domains and disciplines. It is not itself an education -product. Customers might use LearnStack to build: +LearnStack is a **white-label platform for multi-branch education businesses that teach +live**. Its customer is an education business — a language school with three branches, a +yoga studio with four, a coding bootcamp, a music school — that sells courses, teaches +them partly or wholly in scheduled live sessions, and wants its own brand on its own +domain rather than a listing inside someone else's marketplace. LearnStack is not itself +an education product. + +Three properties define the fit. A prospect that has none of them is not the target +customer: + +- **Multi-branch.** Campuses, studios, departments, or cohorts that share one brand and + one catalog but keep their own members, staff, and schedules. This is the + `Organization` level ([ADR-0017](../decisions/0017-tenant-organization-hierarchy.md)), + and it is the reason a single-teacher course seller is a poor fit. +- **Teaches live.** Scheduling, booking, attendance, and an in-app live classroom are + first-class subsystems, not an add-on bolted to a video library. +- **Owns its brand.** Own domain, own design tokens, own locales, own notification + senders, own catalog — no LearnStack logo in the learner's browser. + +What is **not** fixed is the subject those businesses teach. The same code paths serve: - An online English-learning platform with CEFR levels and placement tests. - A yoga studio platform with asana taxonomy, sequence-based lessons, and teacher scheduling. -- A coding bootcamp with code-challenge lesson items and automatic test-runner grading. +- A coding bootcamp with programming-exercise lesson items and automatic grading. - A music school with score-reading lesson items, MIDI playback, and audio submissions. - A meditation app with timed practice sessions and habit-streak tracking. - A driving school with vehicle scheduling and progress-checkpoint workflows. - An art workshop with portfolio uploads and peer-review assessments. -- A meditation, certification, exam-prep, or any domain not anticipated above. +- A certification body, an exam-prep provider, or a domain not anticipated above. LearnStack ships **one codebase, one set of container images, one Helm chart** that serves all of these customers. The differentiator across customers is their **data** (content, content type definitions, page block schemas, scoring rules, level taxonomies, custom fields) — not their code. LearnStack engineers never write per-vertical code. +That claim holds inside a stated edge — see +[Genericity boundary](#genericity-boundary) below. ## Product thesis @@ -37,6 +56,9 @@ Education businesses need infrastructure that is: ([24-learnstack-hub.md](24-learnstack-hub.md)). - **Deployment-flexible.** Same codebase deploys as SaaS, Dedicated (LearnStack-managed single-tenant), or Self-Hosted ([25-deployment-models.md](25-deployment-models.md)). + `Development` and `SaaS` are wired end to end today; the other three `DeploymentMode` + values are prepared seams until Phase 11 builds their adapters and integration suites + ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). ## The three layers @@ -85,11 +107,56 @@ flowchart TB See [28-platform-tenant-organization.md](28-platform-tenant-organization.md) for the full conceptual model. +## Genericity boundary + +"The domain is data" is true inside a boundary, and stating the boundary is what keeps +the claim credible. The boundary is drawn in +[ADR-0018 Amendment (2026-08-08)](../decisions/0018-tenant-driven-customization-model.md); +this section is its product-facing statement. + +**Inside the boundary — tenant customization data, no LearnStack release required:** + +| Dimension | What the tenant declares | Example | +|---|---|---| +| **Content shape** | A `TenantContentType` JSON Schema | A vocabulary card, an asana pose, a grammar topic, a repertoire piece | +| **Presentation** | Which blocks compose a page, which fields a card renders, what the level taxonomy is called | CEFR A1–C2 versus Foundation–Advanced versus kyu/dan | +| **Pure rule evaluation** | A `TenantScoringRule` or `TenantCompletionRule` expression | Map a placement-test answer set to a recommended level; decide a lesson is complete from facts already recorded | + +The discriminating property is that all three are **pure functions of tenant data and +already-recorded state**. Nothing in this column holds new state or reaches outside the +process, so a schema plus an evaluator is a complete answer. + +**Outside the boundary — platform features, written by LearnStack, gated by plan:** + +| Dimension | Example | Why it cannot be a customization row | +|---|---|---| +| **Stateful entitlement** | A ten-session credit pack; "three make-up classes per term"; a per-learner session quota | The tenant needs a balance that is decremented on booking, refunded on cancellation, expired on a schedule, and reconstructible in a dispute. **A JSON Schema declares a shape; it cannot declare a ledger.** | +| **External capability invocation** | Running a learner's submitted program; scoring pronunciation from an audio clip; automated proctoring | The tenant needs a sandbox, a runtime, a resource budget, and a security boundary that survives hostile input. **A rule DSL evaluates; it does not execute arbitrary programs.** | + +Said plainly: **a tenant that needs something in the second column needs a LearnStack +release, or an integration with an external provider through an adapter. It does not +need — and cannot have — a customization row.** Sales conversations, plan design, and +roadmap intake all depend on that sentence being said out loud rather than discovered +during implementation. + +Two things this boundary does **not** change: + +- **The core stays domain-neutral either way.** A credit-pack ledger is not a yoga + feature and an execution sandbox is not a coding-bootcamp feature. Both are named for + the capability, never for the domain that first asked for it — the rule + `Core_Modules_HaveNo_DomainSpecific_Names` enforces the naming mechanically from + [Phase 02a Packet 10](../roadmap/phase-02a-kernel-tenancy.md). +- **There is still no `Verticals/` folder.** A platform feature is a generic capability + offered to every tenant and switched on by a `FeatureKey`; it is not a per-vertical + code package. [ADR-0011](../decisions/0011-extension-points.md) stays superseded. + ## Design principles -- **Generic-only core.** No domain-specific code in LearnStack modules. CEFR, asanas, - code challenges live as **tenant data** (ADR-0018), never as `LearnStack.Verticals.*` - source code. +- **Generic-only core.** No domain-specific code in LearnStack modules. CEFR levels, + asana taxonomies, and programming-exercise shapes live as **tenant data** (ADR-0018), + never as `LearnStack.Verticals.*` source code. Where a tenant need falls outside the + customization model, it becomes a **generically named platform capability** — see + [Genericity boundary](#genericity-boundary) — not a vertical package. - **Multi-tenant from day one.** Tenant isolation is non-negotiable; defense-in-depth = tenant context + organization filter + EF query filter + RLS + architecture tests (ADR-0003 Amendment 1). @@ -100,9 +167,16 @@ conceptual model. The renderer is a client of the core, not the other way around. - **Provider adapters everywhere.** Payments, auth, storage, search, live classroom, notifications, recording — all behind interfaces. No SaaS lock-in baked into core code. -- **Auditability and event tracking as platform primitives.** Domain events, integration - events (outbox → Dapr pub/sub → Kafka — ADR-0014), and audit log (ADR-0016) are designed - in, not bolted on. +- **Auditability and event tracking as platform primitives.** Domain events, the + LearnStack-owned outbox, and the audit log are designed in, not bolted on. MUST-class + audit is a durable intent written inside the business transaction + ([ADR-0033](../decisions/0033-audit-durability-model.md), superseding ADR-0016). +- **Ports on day one, adapters on demand.** The one-way-door test + ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)) separates decisions that + get more expensive every week — isolation, schema ownership, typed identifiers — from + decisions a port makes reversible. `IEventBus` ships in Phase 02a; its Dapr/Kafka + adapter ships in Phase 11 when a second process needs to consume an integration event + (ADR-0014 decides *what*, ADR-0035 decides *when*). - **Versioned publish workflows** for content and courses that affect learners. - **Hub-separated control plane.** Tenant lifecycle, billing, licensing, custom domains, compliance run in a separate codebase (`learnstack-hub`, ADR-0019). LearnStack core @@ -134,9 +208,12 @@ When the foundation is in place, LearnStack should be able to: sales-assisted for Enterprise. - **Map a custom domain end-to-end** — DNS verification → Let's Encrypt cert → APISIX hot-reload → tenant resolver mapping. All Hub-orchestrated, ADR-0022. -- **Run three radically different tenant platforms on the same binary** — English - learning + yoga studio + coding bootcamp + driving school — without LearnStack writing - any domain code. +- **Run radically different tenant platforms on the same binary** — English learning + + yoga studio + coding bootcamp + driving school — without LearnStack writing any domain + code. Two such tenants exist from + [Phase 02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md) and render side by side in + a browser from [Phase 02d](../roadmap/phase-02d-walking-skeleton.md), so the claim is + tested continuously rather than asserted until the showcase phase. - **Tenant admin defines a custom content type in Admin Studio** (e.g. "BreathTechnique" for a yoga tenant) — JSON Schema authored in a visual editor — and immediately uses it in a course lesson. @@ -148,8 +225,10 @@ When the foundation is in place, LearnStack should be able to: - **Air-gapped customer renews a license key offline** — signed `.lic` file delivered via SFTP, placed on disk, SIGHUP triggers re-read, entitlement refreshed — no outbound network required. -- **Regulator inquires about a specific tenant's history** — audit log (ADR-0016) plus - Hub audit stream answer "who did what when" with full correlation. +- **Regulator inquires about a specific tenant's history** — audit log + ([ADR-0033](../decisions/0033-audit-durability-model.md)) plus Hub audit stream answer + "who did what when" with full correlation, and the MUST-class rows are there because + the operation would have been rejected if they could not be written. - **Customer migrates from SaaS to Self-Hosted** — same binary, same data export tools, no rewrite. @@ -170,13 +249,20 @@ When the foundation is in place, LearnStack should be able to: ## References to formative decisions -- ADR-0003 Amendment 1 — Tenant Isolation + Organization scope. -- ADR-0014 — Adopt Dapr. -- ADR-0015 — APISIX gateway. -- ADR-0016 — Audit log subsystem. +- ADR-0003 Amendment 1 (Organization scope) + Amendment 3 (corrected RLS template and + database role model) — Tenant Isolation. +- ADR-0014 — Adopt Dapr (what), with ADR-0035 deciding when. +- ADR-0015 — APISIX gateway (what), with ADR-0035 deciding when. - ADR-0017 — Tenant + Organization hierarchy. -- ADR-0018 — Tenant-driven customization (supersedes ADR-0011 vertical packs). +- ADR-0018 — Tenant-driven customization (supersedes ADR-0011 vertical packs); the + 2026-08-08 Amendment draws the genericity boundary above. - ADR-0019 — LearnStack Hub. - ADR-0020 — Triple deployment + hybrid license. - ADR-0021 — Feature-based entitlement. - ADR-0022 — Custom domain & TLS. +- [ADR-0033](../decisions/0033-audit-durability-model.md) — Audit durability model + (supersedes ADR-0016). +- [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — Hub contract surface + invariant. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — Demand-gated + infrastructure. diff --git a/docs/architecture/09-tenant-isolation.md b/docs/architecture/09-tenant-isolation.md index 35b003c..4293444 100644 --- a/docs/architecture/09-tenant-isolation.md +++ b/docs/architecture/09-tenant-isolation.md @@ -66,38 +66,31 @@ sequenceDiagram ## RLS policy templates -### Tenant-only entity - -```sql -ALTER TABLE ADD COLUMN tenant_id uuid NOT NULL; -CREATE INDEX ix_
_tenant_id ON
(tenant_id); - -ALTER TABLE
ENABLE ROW LEVEL SECURITY; -CREATE POLICY
_tenant_isolation ON
- USING (tenant_id = current_setting('app.tenant_id', true)::uuid); -``` - -### Tenant + Organization entity - -```sql -ALTER TABLE
ADD COLUMN tenant_id uuid NOT NULL; -ALTER TABLE
ADD COLUMN organization_id uuid NULL; -CREATE INDEX ix_
_tenant_org ON
(tenant_id, organization_id); - -ALTER TABLE
ENABLE ROW LEVEL SECURITY; -CREATE POLICY
_tenant_isolation ON
- USING (tenant_id = current_setting('app.tenant_id', true)::uuid); -CREATE POLICY
_organization_isolation ON
- USING ( - organization_id IS NULL -- tenant-wide row, visible to all orgs in tenant - OR organization_id = current_setting('app.organization_id', true)::uuid -- org-scoped row, only matching org - OR current_setting('app.scope', true) = 'tenant' -- tenant-scope operation (admin / reporting) sees all orgs - ); -``` +The **canonical SQL template** — for both the tenant-only and the tenant + organization +shape — lives in exactly one place: +[Database Standards § Tenant-Owned and Organization-Scoped Tables](../standards/05-database.md). +It is not repeated here. Copying it into a second document is how the four divergent +copies that preceded [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) +came about. + +Three properties of that template matter to the isolation model described on this page: + +- **One policy, one `AND`-ed predicate.** Tenant and organization scope are evaluated + together. Two separate policies would both be *permissive*, and PostgreSQL combines + permissive policies with `OR` — under which a tenant-wide row (`organization_id IS + NULL`) satisfies the organization half on its own and becomes visible to every + tenant. A second policy may only ever be added `AS RESTRICTIVE`. +- **`ENABLE` and `FORCE ROW LEVEL SECURITY`.** Without `FORCE`, the table owner + bypasses its own policies — and the default Entity Framework Core arrangement makes + the application that owner. +- **An explicit `WITH CHECK`.** `USING` decides what is readable; `WITH CHECK` decides + what is writable. The `app.scope = 'tenant'` setting is set by middleware when the request comes from a -tenant-admin role with a tenant-wide operation flag (e.g. cross-org reporting). The -default scope (`null` or `'organization'`) honours both filters strictly. +tenant-admin role with a tenant-wide operation flag (e.g. cross-org reporting). It +widens **reads** across organizations within the caller's tenant; it never widens +writes, and it never crosses a tenant boundary. The default scope (`null` or +`'organization'`) restricts reads to the caller's organization plus tenant-wide rows. ## Default org semantics diff --git a/docs/architecture/15-event-and-outbox.md b/docs/architecture/15-event-and-outbox.md index 4c4cda6..ba95881 100644 --- a/docs/architecture/15-event-and-outbox.md +++ b/docs/architecture/15-event-and-outbox.md @@ -1,25 +1,52 @@ # Events and Outbox Events allow modules to collaborate without cross-module database coupling. This document -defines the event types, the outbox pattern, and how dispatch is delivered via Dapr -pub/sub to Kafka (ADR-0010 Amendment 1 + ADR-0014). +defines the event types, the outbox pattern, the claim protocol that makes concurrent +dispatch safe, and how dispatch reaches subscribers — in process today, through Dapr +pub/sub to Kafka when the trigger for that adapter fires (ADR-0010 Amendment 1 + +ADR-0014 + ADR-0035). ## Decision Use **domain events** within a module (in-process MediatR notifications, same transaction as the aggregate change) and **integration events** across modules (written to an outbox -in the same transaction; dispatched at-least-once to subscribers via Dapr pub/sub to -Kafka with retry / backoff / dead-lettering). +in the same transaction; dispatched at-least-once to subscribers with retry / backoff / +dead-lettering on both the producer and the subscriber side). -The outbox table is **LearnStack-owned** (per-module in PostgreSQL); Dapr is the -**dispatch transport**, not the durable buffer. +The outbox table is **LearnStack-owned** (a single shared table in PostgreSQL); the event +bus is the **dispatch transport**, not the durable buffer. That ownership split is what +makes the transport swappable without touching a single producer or consumer. + +## What ships when + +Three separable things are often collapsed into one; they are not the same and they do +not land together. + +| Piece | Owning phase | State | +|---|---|---| +| `outbox_messages` table, its schema, and its LearnStack ownership | [Phase 02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) | Ships even though nothing dispatches from it yet — the schema and its ownership are a one-way door | +| `IEventBus` port + `InProcessEventBus` | [Phase 02a Packet 5](../roadmap/phase-02a-kernel-tenancy.md) | The only registered implementation | +| `OutboxProcessor`, `IInboxGuard`, the claim protocol, the first real integration event | [Phase 02b](../roadmap/phase-02b-events-auth.md) | The durable dispatcher | +| `DaprEventBus` → Dapr pub/sub → Kafka | [Phase 11](../roadmap/phase-11-production-hardening.md) | Demand-gated; trigger: a second process needs to consume an integration event, or event volume / replay / cross-process ordering is required ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)) | + +**`InProcessEventBus` is a first-class transport, not a stub.** It uses the same +`IIntegrationEventHandler` interface, the same `IInboxGuard` deduplication, and the +same tenant-context restoration as the durable path. A development transport that skips +those is a development transport that never exercises the isolation code — and it forces +every consumer to carry two implementations, one of which is never tested against the +other. Everything in this document about consumer obligations applies identically to both +transports; the only difference is what carries the bytes between publish and handle. + +[ADR-0014](../decisions/0014-adopt-dapr.md) stands as the decision that Dapr is the +cross-process transport LearnStack uses. [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) +decides when it arrives, and the answer is "when a second process exists". ## Event types | Type | Scope | Transport | Example | |------|-------|-----------|---------| | **Domain event** | Inside one module | MediatR `INotification` in-process | `CourseVersionPublished` | -| **Integration event** | Cross-module | Outbox → `IEventBus.PublishAsync` → Dapr pub/sub → Kafka | `OrderPaidV1`, `EnrollmentCreated`, `TenantSuspended` | +| **Integration event** | Cross-module | Outbox → `IEventBus.PublishAsync` → transport (in-process dispatcher today; Dapr pub/sub → Kafka from Phase 11) | `OrderPaidV1`, `EnrollmentCreated`, `TenantSuspended` | | **Analytics event** | Reporting stream | Same channel as integration events | `LessonCompleted` | | **Provider event** | External callback (webhook) | Provider → API endpoint → outbox → ... | `LiveKitParticipantJoined`, `StripeInvoicePaid` | @@ -28,12 +55,11 @@ The outbox table is **LearnStack-owned** (per-module in PostgreSQL); Dapr is the ```mermaid sequenceDiagram participant Module as Owning Module - participant DB as PostgreSQL (module schema) + participant DB as PostgreSQL participant Outbox as outbox_messages participant Processor as OutboxProcessor (BackgroundService) - participant Dapr as Dapr sidecar - participant Kafka - participant ConsumerDapr as Consumer Dapr sidecar + participant Bus as IEventBus + participant Transport as Transport (in-process dispatcher, or Dapr to Kafka) participant Consumer as Consuming Module participant Inbox as inbox_messages @@ -42,60 +68,56 @@ sequenceDiagram DB->>DB: COMMIT (atomic: aggregate + outbox row) loop Polling (every 200ms; configurable) - Processor->>Outbox: SELECT WHERE processed_at IS NULL
FOR UPDATE SKIP LOCKED LIMIT @batch - Processor->>Dapr: IEventBus.PublishAsync → DaprClient.PublishEventAsync
(topic: learnstack.{module}.{aggregate}) - Dapr->>Kafka: produce - Processor->>Outbox: UPDATE processed_at = now() + Processor->>Outbox: CLAIM — UPDATE SET locked_by, locked_until
WHERE id IN (SELECT ... FOR UPDATE SKIP LOCKED)
RETURNING *; the claim survives the COMMIT + Processor->>Bus: PublishAsync(event, partitionKey)
topic learnstack.{module}.{aggregate} + Bus->>Transport: deliver + Processor->>Outbox: UPDATE processed_at = now()
WHERE id = @id AND locked_by = @me end - Kafka->>ConsumerDapr: deliver to subscribed sidecar - ConsumerDapr->>Consumer: HTTP POST /events/{topic} - Consumer->>Inbox: IsAlreadyProcessedAsync(@event.EventId) + Transport->>Consumer: deliver to subscribed handler + Consumer->>Consumer: Restore tenant context from event.TenantId + Consumer->>Inbox: IsAlreadyProcessedAsync(event.EventId) alt Already processed Inbox-->>Consumer: skip else New event Consumer->>Consumer: Handle business logic - Consumer->>Inbox: MarkAsProcessed(@event.EventId, @event.GetType().Name) + Consumer->>Inbox: MarkAsProcessed(event.EventId, event type name) Consumer->>DB: SaveChangesAsync (business write + inbox marker, atomic) end ``` +Read as text: the producer writes the aggregate change and the outbox row in one +transaction. A processor **claims** a batch of unprocessed rows by stamping a lease on +them, and the claim outlives the claiming transaction. It publishes each claimed row and +marks it processed only if it still holds the lease. The transport delivers to the +consumer, which restores tenant context, deduplicates through its inbox, and commits the +business write and the inbox marker together. + ## Outbox table -Each module owns its own outbox table inside the module's schema (or in a shared -namespace with module-prefixed key). LearnStack uses a single shared `outbox_messages` -table with `tenant_id` for RLS isolation: +LearnStack uses a **single shared `outbox_messages` table** — not one per module — with +`tenant_id` for Row Level Security isolation. The canonical DDL, its indexes, and its RLS +policy live in exactly one place: +[Database Standards § Outbox](../standards/05-database.md). Do not copy the DDL into a +third document; the last time this table's policy was duplicated, the copy carried the +superseded two-permissive-policy shape that +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) corrects. -```sql -CREATE TABLE outbox_messages ( - id uuid PRIMARY KEY, - occurred_at timestamptz NOT NULL DEFAULT now(), - tenant_id uuid NOT NULL, - correlation_id text NULL, - causation_id uuid NULL, - actor_user_id uuid NULL, - type text NOT NULL, -- assembly-qualified event type name - topic text NOT NULL, -- "learnstack.identity.user" - payload jsonb NOT NULL, - metadata jsonb NULL, - processed_at timestamptz NULL, - attempts int NOT NULL DEFAULT 0, - last_error text NULL, - available_after timestamptz NOT NULL DEFAULT now() -); +Three columns exist for reasons this document owns rather than the schema, and the two +guarantees below (ordering, single-claimant dispatch) cannot be honoured without them: -CREATE INDEX ix_outbox_pending - ON outbox_messages (available_after) - WHERE processed_at IS NULL; -CREATE INDEX ix_outbox_tenant_pending - ON outbox_messages (tenant_id, available_after) - WHERE processed_at IS NULL; - -ALTER TABLE outbox_messages ENABLE ROW LEVEL SECURITY; -CREATE POLICY outbox_messages_tenant_isolation ON outbox_messages - USING (tenant_id = current_setting('app.tenant_id', true)::uuid); --- OutboxProcessor uses learnstack_outbox_admin role that bypasses RLS to read all tenants. -``` +| Column | Why it exists | Lands with | +|---|---|---| +| `partition_key text NOT NULL` | The ordering guarantee below is expressed entirely through this value. Defaults to the aggregate id; falls back to `tenant_id` when the event names no aggregate. Set at enqueue time by `IOutbox.EnqueueAsync`, never by the transport. | The table itself, in [Phase 02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) — a producer-side column is cheaper to ship with the table than to backfill | +| `locked_by text NULL` + `locked_until timestamptz NULL` | The dispatch **lease**. A processor stamps them to claim a row, and the claim survives the claiming transaction's commit. See the claim protocol below. | The dispatcher, in [Phase 02b](../roadmap/phase-02b-events-auth.md) | +| `available_after timestamptz NOT NULL` | Retry backoff. A failed dispatch pushes this forward instead of blocking the batch. | Already in the canonical DDL | + +Both additions go into the canonical DDL in Standards 05 when their phase ships; this +document does not carry a second copy of the table. + +The pending index therefore covers `(available_after)` filtered on +`processed_at IS NULL`, and the claim predicate additionally reads +`locked_until IS NULL OR locked_until < now()`. ### Why the `learnstack_outbox_admin` role bypasses RLS — and what bounds the bypass @@ -106,8 +128,8 @@ the bypass is bounded by: - **Scope by grant**. The role has `SELECT`, `UPDATE` only on `outbox_messages` (status column transitions: `processed_at`, `attempts`, `last_error`, - `available_after`). It has **no** access to any other tenant-owned table; the - bypass cannot be used as a generic backdoor. + `available_after`, `locked_by`, `locked_until`). It has **no** access to any + other tenant-owned table; the bypass cannot be used as a generic backdoor. - **No connection sharing**. The role is used only by the `OutboxProcessor` BackgroundService DI scope; no MediatR handler or API endpoint ever runs under this role (architecture test @@ -121,10 +143,14 @@ the bypass is bounded by: the RLS bypass is the minimum privilege escalation that makes that pattern work. It is not an exception to ADR-0003 — it is the only legitimate path. -The same shape applies to `learnstack_audit_admin` (read-side cross-tenant -audit queries, see [31-audit-subsystem.md](31-audit-subsystem.md)) and any -future `learnstack_*_admin` role: bounded by grant, isolated by DI scope, -discoverable by log. +`learnstack_outbox_admin` is one of the **four** roles in the database model fixed by +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) — +`learnstack_migration` (owner, `NOBYPASSRLS`), `learnstack_app` (runtime, +`NOBYPASSRLS`), `learnstack_platform` (audited cross-tenant admin, `BYPASSRLS`), and +this one. Cross-tenant audit reads use `learnstack_platform` through the audited +`EnterPlatformAdminScope(reason)` path; there is no separate audit role. Adding a fifth +role requires an ADR — every `BYPASSRLS` role is a hole in the isolation model that has +to earn its existence by grant scope, DI-scope isolation, and log discoverability. ## Inbox table (per consumer module) @@ -166,7 +192,9 @@ public async Task> Handle(CreateEnrollmentCommand cmd, Can ``` `IOutbox.EnqueueAsync` writes to the same `DbContext` (no separate transaction); commit -is atomic with the aggregate write. +is atomic with the aggregate write. It also resolves and stores the row's +`partition_key` — here `EnrollmentId`, the aggregate this event is about. See +[Ordering](#ordering). ## Consumer pattern @@ -196,13 +224,34 @@ public sealed class CreateAuditTrailOnEnrollmentCreated( ## OutboxProcessor (BackgroundService) -Continuously polls `outbox_messages` and dispatches: +### The claim problem + +`SELECT ... FOR UPDATE SKIP LOCKED` holds its row locks **only until the transaction that +took them ends**. A processor that selects a batch, commits to release the locks, and +*then* starts publishing has released every row it is about to work on. A second +processor polling one millisecond later sees those rows with `processed_at IS NULL`, is +not blocked by any lock, and dispatches the same events. The first processor then marks +them processed, and nothing in the system records that they went out twice. + +That is why this document no longer claims "no double dispatch". The delivery contract is +**at-least-once**, and consumer-side idempotency through `IInboxGuard` is what makes it +safe — not the row lock. What the claim protocol buys is that duplicates come from crash +recovery and lease expiry, which are rare and bounded, rather than from every concurrent +poll, which is neither. + +### The claim protocol + +The row lock has to survive the transaction that takes it, which means the claim must be +**written to the row**, not held in the lock table: ```csharp public sealed class OutboxProcessor : BackgroundService { private const int BatchSize = 100; private static readonly TimeSpan PollInterval = TimeSpan.FromMilliseconds(200); + private static readonly TimeSpan LeaseDuration = TimeSpan.FromSeconds(60); + + private readonly string _processorId = $"{Environment.MachineName}:{Guid.NewGuid():N}"; protected override async Task ExecuteAsync(CancellationToken ct) { @@ -220,35 +269,39 @@ public sealed class OutboxProcessor : BackgroundService { var db = services.GetRequiredService(); var eventBus = services.GetRequiredService(); - // Connection set to learnstack_outbox_admin role; RLS bypassed. + // Connection runs as learnstack_outbox_admin; RLS bypassed by grant. - await using var tx = await db.Database.BeginTransactionAsync(ct); + // 1. CLAIM. One statement, its own short transaction. The lease is written to the + // row, so it outlives the commit that releases the physical locks. var batch = await db.Set() .FromSqlInterpolated($@" - SELECT * FROM outbox_messages - WHERE processed_at IS NULL AND available_after <= now() - ORDER BY occurred_at - LIMIT {BatchSize} - FOR UPDATE SKIP LOCKED") + UPDATE outbox_messages + SET locked_by = {_processorId}, + locked_until = now() + {LeaseDuration}, + attempts = attempts + 1 + WHERE id IN ( + SELECT id FROM outbox_messages + WHERE processed_at IS NULL + AND available_after <= now() + AND (locked_until IS NULL OR locked_until < now()) + ORDER BY occurred_at + LIMIT {BatchSize} + FOR UPDATE SKIP LOCKED) + RETURNING *") .ToListAsync(ct); - if (batch.Count == 0) - { - await tx.CommitAsync(ct); - return 0; - } - - // Commit the SELECT FOR UPDATE transaction immediately to release row locks. - await tx.CommitAsync(ct); + if (batch.Count == 0) return 0; + // 2. PUBLISH. One short transaction per message. A single poisoned message does + // not roll back the batch. foreach (var msg in batch) { await using var perMessageTx = await db.Database.BeginTransactionAsync(ct); try { var eventInstance = JsonSerializer.Deserialize(msg.Payload, Type.GetType(msg.Type)!); - await eventBus.PublishAsync((IIntegrationEvent)eventInstance!, ct); - msg.MarkProcessed(DateTimeOffset.UtcNow); + await eventBus.PublishAsync((IIntegrationEvent)eventInstance!, msg.PartitionKey, ct); + msg.MarkProcessed(_clock.UtcNow, _processorId); // no-op if the lease was lost } catch (Exception ex) { @@ -264,32 +317,84 @@ public sealed class OutboxProcessor : BackgroundService } ``` -Key invariants: +`MarkProcessed` writes `processed_at` **and** clears the lease under the predicate +`locked_by = @me`, so a processor whose lease expired mid-batch — because the transport +hung past `LeaseDuration` — cannot overwrite the state of whichever processor took the +row next. A lost lease is logged at warning level and surfaces on the +`learnstack_outbox_lease_lost_total` counter; a non-zero rate means `LeaseDuration` is +shorter than the transport's tail latency and must be raised. + +### The simpler alternative, and its cost + +Holding one transaction open across the whole batch — `BEGIN`, `SELECT ... FOR UPDATE +SKIP LOCKED`, publish each, `UPDATE processed_at`, `COMMIT` — is also correct, and is an +acceptable implementation of the same contract. Its costs are real and must be accepted +knowingly: + +- The transaction stays open for the duration of every network publish in the batch, + pinning the `VACUUM` horizon and the oldest transaction id for that long. +- One poisoned message rolls back the batch unless every publish is wrapped in a + savepoint, which reintroduces most of the complexity the approach was meant to avoid. +- A crash mid-batch republishes everything already published in it. + +The lease is the specified design. The batch-held transaction is the acceptable fallback +if a deployment cannot add the two columns. Whichever ships, the choice is made once, in +[Phase 02b](../roadmap/phase-02b-events-auth.md), and written into the dispatcher — not +left to each reader of this document. + +### What the processor guarantees + +- **At-least-once delivery.** Duplicates are expected; consumers deduplicate through + `IInboxGuard`. This is the contract, not a defect. +- **At most one active claimant per row.** Two processors cannot both hold a live lease + on the same row, so concurrent polling does not multiply dispatches. Duplicates arise + only from lease expiry and crash recovery. +- **Batch independence.** Each message gets its own transaction; one failure does not + roll back the batch. +- **Bounded retry.** Failed messages retry with exponential backoff (1s, 5s, 30s, 5min, + 1h). On reaching `MaxAttempts` the row moves to the producer-side dead-letter state + described below. +- **Horizontal scalability.** `SKIP LOCKED` plus the lease predicate lets N processors + across N pods drain the same table without coordination. + +## `IEventBus` and the partition key + +The port takes the partition key explicitly. It is not derived inside the transport, +because the transport is the one component that does not know what the event's ordering +domain is: -- `FOR UPDATE SKIP LOCKED` allows horizontal scaling (multiple OutboxProcessor instances - across pods can run concurrently without double-dispatch). -- The `SELECT FOR UPDATE` transaction commits immediately to release row locks. -- Each message gets its own transaction; one failure doesn't roll back the batch. -- Failed messages are retried with exponential backoff (1s, 5s, 30s, 5min, 1h); - permanent failures (max retries reached) move to dead-letter (manual intervention). +```csharp +public interface IEventBus +{ + Task PublishAsync(TEvent @event, string partitionKey, CancellationToken ct = default) + where TEvent : IIntegrationEvent; +} +``` -## `IEventBus` → Dapr pub/sub → Kafka +The durable implementation forwards it as Dapr's `partitionKey` metadata, which the Kafka +pub/sub component maps onto the Kafka message key: ```csharp public sealed class DaprEventBus(DaprClient daprClient) : IEventBus { - public Task PublishAsync(TEvent @event, CancellationToken ct = default) + public Task PublishAsync(TEvent @event, string partitionKey, CancellationToken ct = default) where TEvent : IIntegrationEvent - { - var topic = ConventionTopicName(@event); // "learnstack.{module}.{aggregate}" - return daprClient.PublishEventAsync("pubsub", topic, @event, ct); - } + => daprClient.PublishEventAsync( + "pubsub", + ConventionTopicName(@event), // "learnstack.{module}.{aggregate}" + @event, + new Dictionary { ["partitionKey"] = partitionKey }, + ct); private static string ConventionTopicName(IIntegrationEvent @event) => $"learnstack.{ExtractModule(@event.GetType())}.{ExtractAggregate(@event.GetType())}"; } ``` +`InProcessEventBus` honours the same key by serialising dispatch per partition key — +concurrent across keys, sequential within one — so a consumer cannot observe an ordering +in development that the durable transport would not also produce. + Topic naming convention: `learnstack.{module}.{aggregate}`. Examples: - `learnstack.identity.user` @@ -300,22 +405,46 @@ Topic naming convention: `learnstack.{module}.{aggregate}`. Examples: - `learnstack.hub.entitlement` (Hub-side) - `learnstack.cache.invalidation` (cross-instance L1 cache) -## Development mode +## `InProcessEventBus` + +Until the Dapr adapter's trigger fires, `InProcessEventBus` is the only registered +`IEventBus`. It is a **transport**, not a shortcut, and it carries the same four +obligations as the durable path: -When `DeploymentMode.Development` and Dapr sidecar isn't running, `InProcessEventBus` -replaces `DaprEventBus`: +| Obligation | Why it cannot be skipped in development | +|---|---| +| Same handler interface — `IIntegrationEventHandler`, never a bare `INotificationHandler` | Two interfaces means two implementations per consumer, and the one that runs in CI is not the one that runs in production | +| Same `IInboxGuard` deduplication | Deduplication bugs are the most common integration-event defect; a transport that never delivers a duplicate never surfaces them | +| Same tenant-context restoration from `@event.TenantId` before the handler runs | A dev path that skips it is a dev path where Row Level Security and the query filters are never exercised on the consumer side | +| Same per-partition-key ordering | Ordering assumptions that hold only in process are discovered in production | ```csharp -public sealed class InProcessEventBus(IPublisher publisher) : IEventBus +public sealed class InProcessEventBus( + IServiceScopeFactory scopeFactory, + ITenantContextAccessor tenantAccessor, + IPartitionSerializer partitions) : IEventBus { - public Task PublishAsync(TEvent @event, CancellationToken ct = default) + public Task PublishAsync(TEvent @event, string partitionKey, CancellationToken ct = default) where TEvent : IIntegrationEvent - => publisher.Publish(@event, ct); + => partitions.RunSequentiallyFor(partitionKey, async () => + { + await using var scope = scopeFactory.CreateAsyncScope(); + tenantAccessor.Set(TenantContext.FromEvent(@event)); // same restore as the durable path + foreach (var handler in scope.ServiceProvider.GetServices>()) + await handler.HandleAsync(@event, ct); // handler calls IInboxGuard itself + }); } ``` -Module subscribers (`INotificationHandler` for dev / `IIntegrationEventHandler` -for prod) handle the event. The composition root chooses based on `DeploymentMode`. +What the in-process transport genuinely does **not** provide, and what therefore +constitutes the trigger for the Dapr adapter: delivery to a second process, broker-side +replay, and durability of the in-flight message if the process dies between publish and +handle. The outbox row covers the third — an unhandled event has not been marked +processed and is redelivered — which is precisely why the outbox is LearnStack-owned and +ships before any transport. + +The composition root selects the implementation by `DeploymentMode`; modules never see +the choice. ## Rules @@ -325,25 +454,103 @@ for prod) handle the event. The composition root chooses based on `DeploymentMod - Consumers are **idempotent** via `IInboxGuard` (per-module inbox table). - Mandatory metadata on every integration event: `EventId`, `TenantId`, `OccurredAt`, `CorrelationId`. Optional but recommended: `CausationId`, `ActorUserId`. -- **Tenant context** restored on the consumer side before any business logic runs - (Dapr passes the event payload; consumer middleware sets `accessor.SetTenant(...)` - from `@event.TenantId` before invoking the handler). -- Ordering is **not** guaranteed across topics; **per-partition** ordering within a Kafka - topic is preserved by partition-keying on the aggregate id when ordering is required - (rare). -- Failed dispatches **retry with exponential backoff**; permanent failures move to a - dead-letter state (manual review via the `OutboxStatusEndpoints` admin API). +- **Tenant context** restored on the consumer side before any business logic runs. The + transport delivers the event payload; the consumer scope sets the ambient context from + `@event.TenantId` before invoking the handler, so the handler's queries carry the same + filters and the same `app.tenant_id` as an HTTP-originated command. + +## Ordering + +Ordering is **not** guaranteed across topics, and never has been. Within one topic, +ordering is guaranteed **per partition key** — and a partition key that nobody sets is a +guarantee nobody has. + +The rule, therefore, is that **every outbox row carries a non-null `partition_key`**: + +| Event shape | Partition key | +|---|---| +| Event about one aggregate instance (the normal case) | The aggregate id — `EnrollmentCreatedIntegrationEvent` keys on `EnrollmentId` | +| Event about the tenant as a whole (`TenantSuspended`, `EntitlementUpdated`) | `TenantId` | +| Event with an explicit ordering domain that is neither (rare) | Declared by the event type through `IPartitionedIntegrationEvent.PartitionKey` | + +`IOutbox.EnqueueAsync` resolves the key at enqueue time and writes it to the row; the +processor reads it from the row and passes it to `IEventBus.PublishAsync`. Nothing +downstream re-derives it, so the ordering domain is decided once, by the producer that +knows it. + +Consequences worth stating plainly: + +- Two events about the **same** aggregate arrive in publish order at every consumer. +- Two events about **different** aggregates may arrive in any relative order, even inside + one topic. A consumer that needs a cross-aggregate order needs a different design — + usually a read model, not an ordering assumption. +- Keying on `TenantId` for high-volume events serialises that tenant's whole stream onto + one partition. That is a deliberate throughput cost, paid only where the ordering is + genuinely tenant-wide. +- The column is `NOT NULL`, so the database asserts it. The catalogued + `Outbox_Row_Carries_Correlation_Context` integration test extends its assertion to + `partition_key` when the column lands, so a new enqueue path that forgets it fails in + CI rather than in a consumer. + +## Dead-letter: two sides, two failure domains + +The distinction the earlier draft of this document missed: a producer-side dispatch +success means "the transport accepted the bytes". It says nothing about whether any +consumer processed them. The two sides fail independently and need separate handling. + +### Producer side — dispatch never succeeded + +The outbox row reaches `MaxAttempts` without a successful `PublishAsync`. + +- The row stays in the table with `processed_at IS NULL`, `attempts >= MaxAttempts`, and + the final `last_error`. +- `available_after` is pushed beyond the retry horizon so the poller stops picking it up. +- It surfaces on the `OutboxStatusEndpoints` admin API and on the + `learnstack_outbox_deadletter_total{event_type}` counter; the alert is on the counter's + rate, not on the table's size. +- Recovery is a manual replay: an operator clears the lease, resets `attempts`, and + resets `available_after`. + +### Subscriber side — dispatch succeeded, handling did not + +A consumer's handler throws on every attempt: a malformed payload, a schema version it +cannot read, a permanently failing downstream dependency. The producer's row is long +since `processed_at`-stamped, and no amount of producer-side retry will help. + +- The transport retries the handler with backoff. On exhausting its retry policy it + routes the message to a **dead-letter topic**, `learnstack.dlq.{module}` — one per + subscribing module, three segments, satisfying + `Dapr_PubSub_TopicNames_FollowConvention`. Under Dapr this is the subscription's + `deadLetterTopic`; under `InProcessEventBus` the equivalent is a row in + `dead_letter_messages` carrying the same envelope. +- **The inbox row is not marked processed.** A dead-lettered message has not been + handled, and recording it as handled would make a later replay a silent no-op. +- Every module subscribes to its own dead-letter topic with a handler that does nothing + but persist the envelope, emit `learnstack_inbox_deadletter_total{module, event_type}`, + and raise an operational alert. A dead-letter topic nobody consumes is a queue that + grows until the broker's retention silently deletes the evidence. +- Recovery is an operator-initiated replay from the dead-letter store back onto the + original topic. Because the inbox never marked the event processed, the replayed + message is handled normally; because `IInboxGuard` is still in the path, a replay that + races a late success is still idempotent. +- Poison-message containment is per subscription. One module's dead-lettered event does + not block another module's consumption of the same topic — each subscription has its + own offset and its own dead-letter path. ## Service extraction readiness The outbox + `IEventBus` is the future service boundary. If a module is promoted to a separate service later: -- The module's integration events already cross a real broker (Kafka). -- The consumer-side subscription pattern (Dapr `[Topic]` attribute or programmatic - subscription) works identically across in-process and cross-service. -- The producer-side outbox is durable; service extraction doesn't require dual-write - semantics. +- The producer-side outbox is durable and LearnStack-owned; service extraction does not + require dual-write semantics. +- The consumer-side contract — `IIntegrationEventHandler`, `IInboxGuard`, tenant-context + restoration, partition key — is identical under both transports, so extraction changes + which process runs the handler, not how the handler is written. +- Swapping `InProcessEventBus` for `DaprEventBus` is a composition-root change. That is + the whole reason the transport is demand-gated: extraction gets no cheaper by having + built the adapter early, and the platform pays the operational cost of a broker every + day until then. ## Architecture tests @@ -360,23 +567,46 @@ separate service later: LearnStack-core topics remain 3-segment (`learnstack.{module}.{aggregate}`). - `OutboxProcessor_NeverBlocks_OnSingleMessageFailure` — integration test asserts one poisoned message doesn't prevent others in the batch from processing. +- `Integration_Event_Handler_Restores_Tenant_Context` — catalogued in + [Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md); runs + against whichever transport is registered, so `InProcessEventBus` is held to the same + restoration contract as the durable path. +- `Outbox_Row_Carries_Correlation_Context` — extended to assert a non-null + `partition_key`. +- Concurrent-claim integration test: two `OutboxProcessor` instances draining one table + produce **one** claim per row while both leases are live. This is the test that would + have caught the released-lock defect, and it needs two processors — a single-processor + test passes against the broken protocol. ## Observability -- Outbox lag metric: `learnstack_outbox_pending_count{tenant_id}` (gauge). +- Outbox lag: `learnstack_outbox_pending_count{tenant_id}` (gauge). - Dispatch duration: `learnstack_outbox_dispatch_duration_seconds{event_type}` (histogram). - Dispatch failures: `learnstack_outbox_dispatch_failed_total{event_type}` (counter). -- Inbox dedup count: `learnstack_inbox_dedup_total{module, event_type}` (counter). - -Grafana dashboard surfaces these alongside Dapr's own pub/sub metrics -(`dapr_component_pubsub_*`). +- Producer-side dead letter: `learnstack_outbox_deadletter_total{event_type}` (counter). +- Lost leases: `learnstack_outbox_lease_lost_total` (counter) — non-zero means the lease + is shorter than the transport's tail latency. +- Inbox dedup: `learnstack_inbox_dedup_total{module, event_type}` (counter). +- Subscriber-side dead letter: `learnstack_inbox_deadletter_total{module, event_type}` + (counter). + +The two dead-letter counters are **separate signals on separate dashboards**. A +producer-side spike means the transport is down; a subscriber-side spike means a handler +or a payload is broken. Collapsing them into one "events failed" number destroys the only +information that distinguishes the two incidents. Once the Dapr adapter lands, both sit +alongside Dapr's own `dapr_component_pubsub_*` metrics. ## References -- ADR-0006 (Amendment 1) — Events and Outbox; Dapr pub/sub dispatch transport. +- ADR-0006 (Amendment 1) — Events and Outbox; dispatch transport. - ADR-0010 (Amendment 1) — Cross-Module Communication; outbox dispatch target. -- ADR-0014 — Adopt Dapr. -- ADR-0016 — Audit Log Subsystem. +- ADR-0014 — Adopt Dapr (what LearnStack uses for cross-process pub/sub). +- [ADR-0033](../decisions/0033-audit-durability-model.md) — Audit durability model; + audit fan-out to external sinks rides this outbox, MUST-class audit does not. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — Demand-gated + infrastructure; when the Dapr and Kafka adapters arrive. +- [Database Standards § Outbox](../standards/05-database.md) — the canonical + `outbox_messages` DDL and RLS policy. - [29-dapr-integration.md](29-dapr-integration.md) — Dapr deep dive. - [10-cross-module-contracts.md](10-cross-module-contracts.md) — the four sanctioned cross-module mechanisms. diff --git a/docs/architecture/23-data-protection.md b/docs/architecture/23-data-protection.md index e9036d3..26352d2 100644 --- a/docs/architecture/23-data-protection.md +++ b/docs/architecture/23-data-protection.md @@ -145,9 +145,10 @@ not residency-bound. Two flows legitimately cross regions: - **LearnStack ↔ Hub** internal API. Hub may live in a single region (e.g. eu-west) - while serving tenants in multiple regions. The four-endpoint contract is small, - audit-friendly, and carries no tenant content — only plan / entitlement / license - metadata. Tenants whose plan forbids cross-region operator access are served by a + while serving tenants in multiple regions. The contract surface is small, + audit-friendly, and — by the first invariant of + [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — carries no tenant + content, only plan / entitlement / licence metadata. Tenants whose plan forbids cross-region operator access are served by a region-local Hub instance (Hub federation is a Phase-11+ topic, not MVP). - **Telemetry → centralised OpenTelemetry collector**. Trace and metric data can be shipped cross-region for unified observability; PII redaction in the pipeline keeps diff --git a/docs/architecture/24-learnstack-hub.md b/docs/architecture/24-learnstack-hub.md index 16f90aa..acd6e77 100644 --- a/docs/architecture/24-learnstack-hub.md +++ b/docs/architecture/24-learnstack-hub.md @@ -182,43 +182,58 @@ erDiagram ## 3. Communication contracts -### Contract surface — "closed at four" - -The **closed Hub HTTPS contract surface** is the set of endpoints that the -`LearnStack_Modules_DoNotReference_Hub` architecture test and the "no fifth without -ADR" rule (per [ADR-0019](../decisions/0019-learnstack-hub.md) and -[20-infrastructure-stack.md § Hub HTTPS Contract Surface](../standards/20-infrastructure-stack.md)) -guard. Those four endpoints are the **load-bearing entitlement + lifecycle + telemetry -contract**: - -| # | Direction | Method | Path | Purpose | -|---|-----------|--------|------|---------| -| 1 | Hub → LS | POST | `/api/internal/tenants` | Create tenant + default organization | -| 2 | Hub → LS | PUT | `/api/internal/tenants/{id}/entitlements` | Push updated entitlement projection (status, features, limits, compliance.caps) | -| 3 | LS → Hub | POST | `/api/v1/internal/license/verify` | Pull / verify entitlement (called by `HubEntitlementProvider`) | -| 4 | LS → Hub | POST | `/api/v1/usage/report` | Report usage metric (idempotent; Hub aggregates per period) | - -Adding a fifth endpoint to this closed set requires a new ADR. - -### Auxiliary lifecycle endpoints (within the closed surface) - -The operational endpoints below are **specializations of endpoint #2** in the closed -set — they share the same `/api/internal/tenants/{id}` resource, the same -mTLS + signed JWT + HMAC chain, the same `Hub → LearnStack` direction, and the same -authorization scope. They are listed separately for clarity but do not count as new -contract entries: - -| Method | Path | Purpose | Belongs to closed-set # | -|--------|------|---------|--------------------------| -| PUT | `/api/internal/tenants/{id}/status` | Suspend / activate / archive (status field of entitlement projection) | #2 | -| GET | `/api/internal/tenants/{id}/usage` | Pull aggregated usage metrics (Hub-side mirror of #4 reports) | #4 | -| DELETE | `/api/internal/tenants/{id}` | Terminate (hard delete with confirmation) | #1 (inverse) | -| POST | `/api/v1/internal/license/refresh` | Phone-home refresh used by `SelfHostedOnline` mode — a scheduled call shape of #3 | #3 | - -If any of these auxiliary endpoints needs a contract or auth model that **differs** -from its parent (different role, different rate limit, different payload shape that -isn't a subset of the parent), it must be promoted to a new ADR-registered closed-set -entry. +### Contract surface — two invariants, not a count + +Per [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md), the Hub contract +surface is governed by two properties rather than by an endpoint count: + +1. **The Hub stores no tenant content.** Courses, lessons, learners, enrollments, + classroom sessions, media and content entries live exclusively in LearnStack. The + Hub holds tenant *metadata*: plan, subscription, licence, custom domain, compliance + caps, aggregated usage. Enforced by `Hub_NeverStores_TenantData`. +2. **Every crossing goes through a named adapter** — `IEntitlementProvider`, + `IUsageReporter`, `IHubTenantSync`. No other type holds a Hub client, and nothing + resolves a host by calling the Hub. Enforced by + `LearnStack_Modules_DoNotReference_Hub` and + `Hub_Client_Referenced_Only_By_Named_Adapters`. + +This section previously declared the surface "closed at four" and then listed four +further endpoints as "specializations" that did not count. An HTTP endpoint is a path +plus a method; `DELETE /api/internal/tenants/{id}` is not `POST /api/internal/tenants` +because one is the inverse of the other. Worse, the pressure to keep the count at four +is what drove [ADR-0022](../decisions/0022-custom-domain-tls.md) Amendment 1 to tunnel +TLS private keys through the entitlement payload. The invariants above are what anyone +actually needed the count to stand for. + +**Hub → LearnStack** (`/api/internal/*`, internal listener only): + +| Method | Path | Purpose | +|--------|------|---------| +| `POST` | `/api/internal/tenants` | Create tenant + default organization | +| `PUT` | `/api/internal/tenants/{id}/entitlements` | Push the entitlement projection | +| `PUT` | `/api/internal/tenants/{id}/status` | Suspend / activate / archive | +| `DELETE` | `/api/internal/tenants/{id}` | Terminate | +| `GET` | `/api/internal/tenants/{id}/usage` | Pull aggregated usage | +| `PUT` | `/api/internal/tenants/{id}/host-mappings` | Push host → `(tenant_id, organization_id?)` mappings. Carries the tuple only — certificate material moves by secret-store replication and is referenced by path | + +**LearnStack → Hub:** + +| Method | Path | Purpose | +|--------|------|---------| +| `POST` | `/api/v1/internal/license/verify` | Verify / pull the entitlement projection | +| `POST` | `/api/v1/internal/license/refresh` | Scheduled phone-home refresh | +| `POST` | `/api/v1/usage/report` | Report a usage metric (idempotent) | + +Every one of these carries the same auth chain: mTLS with LearnStack-internal CA-signed +client certificates, an RS256 JWT with `aud=learnstack-internal` and `exp ≤ 5min` +replay-protected on `jti`, and an HMAC-SHA256 body signature in `X-Signature`. + +Adding an endpoint still requires an ADR — not because the count is sacred, but because +the surface is a cross-repository contract and both repositories have to agree. + +The Hub's own tenant-facing and operator-facing APIs (`/api/v1/tenants/*`, +`/api/v1/subscriptions/*`, `/api/v1/webhooks/*`) are **not** part of this surface; they +are the Hub's public API, governed by the Hub repository. ### Authentication chain (applies to every endpoint above) diff --git a/docs/architecture/25-deployment-models.md b/docs/architecture/25-deployment-models.md index 7a9c7c8..b42d740 100644 --- a/docs/architecture/25-deployment-models.md +++ b/docs/architecture/25-deployment-models.md @@ -4,8 +4,19 @@ [ADR-0019](../decisions/0019-learnstack-hub.md). LearnStack supports three deployment models from **one codebase, one Helm chart, one set -of container images**. The differentiator across modes is configuration + Dapr component -YAML + `IEntitlementProvider` implementation choice — never application code. +of container images**. The differentiator across modes is configuration + component +wiring + `IEntitlementProvider` implementation choice — never application code. + +> **Support state (2026-08-08).** The `DeploymentMode` enum has five values and the +> composition root branches on all five. Only **`Development`** and **`SaaS`** are wired +> and tested end to end today. `Dedicated`, `SelfHostedOnline`, and +> `SelfHostedAirGapped` are **prepared seams, not supported deployments**, until +> [Phase 11](../roadmap/phase-11-production-hardening.md) builds their adapters and +> integration suites, per +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md). See +> [§ 5](#5-deploymentmode-configuration) for what "prepared seam" means concretely. The +> rest of this document describes the target topologies; it is a design document for all +> five, and a description of running systems for two. ## 1. The three modes @@ -83,8 +94,29 @@ Internet time: < 1 minute. - **Tenant URL**: Starts on `{slug}.learnstack.app`; upgrades to custom domain on Growth+ tier. -- **Failure isolation**: RLS protects tenant rows; one tenant's burst doesn't starve - others (rate limits per remote_addr; Phase 11+ per-tenant consumer keys). +- **Failure isolation**: two different problems, and only one of them is solved today. + **Correctness** isolation — no tenant can read or write another tenant's rows — is + covered by tenant context + EF query filters + Row Level Security + architecture tests + ([ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md)). + **Resource** isolation — no tenant can starve another of connections, CPU, or job + workers — is **not** covered by any of those. Row Level Security is a visibility + predicate: it filters rows, and a filtered query consumes exactly as much of the + database as an unfiltered one. A single tenant running an unbounded report can hold a + connection, saturate the pool, and degrade every other tenant on the instance while + every isolation test stays green. + + What exists today: APISIX `limit-req` keyed on `remote_addr`, which throttles a noisy + client but not a noisy tenant — one tenant behind many IPs is unaffected, and many + tenants behind one NAT are punished together. + + What is required, and where it lives: **resource fairness is + [Phase 11](../roadmap/phase-11-production-hardening.md)** — `statement_timeout` per + connection class, per-tenant connection-pool partitioning, query cost ceilings on + report and export paths, Hangfire queue fairness so one tenant's bulk import cannot + monopolise the workers, and per-tenant rate limiting keyed on the resolved tenant + rather than on the client address. No earlier phase owns this work, and until Phase 11 + ships it, SaaS multi-tenancy is protected against **leakage** but not against + **contention**. ## 3. Dedicated mode @@ -188,17 +220,46 @@ public enum DeploymentMode } ``` -`Program.cs` wires the entitlement provider based on this value (see ADR-0020). +`Program.cs` wires the entitlement provider based on this value (see ADR-0020). Modules +never read the enum — `Modules_Do_Not_Reference_DeploymentMode` enforces that the +composition root branches once. -Other config settings switched by mode: +### Supported today versus prepared seam + +| Value | State | What that means concretely | +|---|---|---| +| `Development` | **Supported** | Wired, run daily, covered by the integration suite | +| `SaaS` | **Supported** | Wired and covered end to end from [Phase 02c](../roadmap/phase-02c-hub-foundation.md) | +| `Dedicated` | Prepared seam | The branch exists and resolves to the default implementations; no dedicated-topology integration suite, no operational runbook | +| `SelfHostedOnline` | Prepared seam | Same; the phone-home path needs the Hub adapter and its failure-mode tests | +| `SelfHostedAirGapped` | Prepared seam | Same, plus a signed-licence provider and a no-egress telemetry target that do not exist yet | + +A prepared seam is a **branch point with no adapter behind it**: the value is accepted, +the composition root routes it, and the implementations it selects are the same defaults +`Development` gets. It is honest to design for it; it is not honest to sell it. Each seam +becomes a supported mode in [Phase 11](../roadmap/phase-11-production-hardening.md) when +its trigger fires — for `SelfHostedAirGapped`, that trigger is a signed Self-Hosted +contract ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). + +The same ADR adds a rule that this document must obey: **a deployment mode without a +signed contract cannot be the deciding factor in a technical choice.** It may break a tie +between otherwise-equal options. It may not, on its own, reject a dependency or justify +an abstraction. + +### Other config settings switched by mode - **Hub URL** — pointed at LearnStack-hosted (SaaS / Dedicated / SelfHostedOnline) OR customer-hosted (rare) OR not set (SelfHostedAirGapped). -- **Default secret store** — Dapr Vault (most modes) OR file-based (air-gapped optional). +- **Secret store** — `EnvironmentSecretProvider` today in every mode; the Vault-backed + provider is demand-gated to Phase 11 behind `ISecretProvider`, triggered by secrets + needing rotation without a redeploy or by a non-development deployment existing. - **Telemetry sink** — LearnStack OTel collector (SaaS / Dedicated) OR customer OTel - (Self-Hosted). -- **Dapr pub/sub component** — Kafka backend differs by deployment (managed Kafka vs - bundled Kafka). + (Self-Hosted). `SelfHostedAirGapped` wires no network exporter at all; its file target + lands in Phase 11. +- **Event transport** — `InProcessEventBus` today in every mode; the Dapr pub/sub + component and its Kafka backend land in Phase 11, triggered by a second process needing + to consume an integration event. See + [15-event-and-outbox.md](15-event-and-outbox.md). ## 6. Same Helm chart @@ -254,12 +315,14 @@ Same chart. Same templates. Different values. ## 7. Deployment-mode-conditional behaviour -A small list of behaviours change across modes: +A small list of behaviours change across modes. This is the **target** table; the two +supported modes reach it today and the three seams reach it in Phase 11. | Behaviour | SaaS | Dedicated | Self-Hosted Online | Self-Hosted Air-Gapped | |-----------|------|-----------|--------------------|------------------------| | `IEntitlementProvider` | Hub | Hub | Hub | SignedLicenseKey | -| `ISecretProvider` | Dapr → Vault | Dapr → Vault | Dapr → Vault | Dapr → Vault OR file | +| `ISecretProvider` (target) | Vault | Vault | Vault | Vault OR file | +| `ISecretProvider` (today) | `EnvironmentSecretProvider` in every mode — the Vault adapter is demand-gated to Phase 11 | ← | ← | ← | | Phone-home enabled | Yes | Yes | Yes | No | | Outbound HTTP to Hub | Yes | Yes | Yes | No | | OTel collector endpoint | LearnStack-hosted | LearnStack-hosted | Customer-hosted | Customer-hosted | @@ -324,6 +387,9 @@ To be authored: - `docs/operations/migration-orchestration.md` — Schema migration coordination. - `docs/operations/security-incident.md` — Security incident response. - `docs/operations/data-export-and-portability.md` — GDPR data export per tenant. +- `docs/operations/resource-fairness.md` — the SaaS contention controls named in + [§ 2](#2-saas-mode): `statement_timeout` classes, pool partitioning, query cost + ceilings, Hangfire queue fairness, per-tenant rate limits. ## 12. Non-goals @@ -338,6 +404,12 @@ To be authored: - ADR-0020 — Triple Deployment + Hybrid License. - ADR-0019 — LearnStack Hub. +- [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) — what + Row Level Security does and does not guarantee. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — which modes are + supported, which are seams, and what promotes a seam. - [26-hybrid-license-model.md](26-hybrid-license-model.md) — license payload + lifecycle. - [24-learnstack-hub.md](24-learnstack-hub.md) — Hub architecture. - [04-technical-architecture.md](04-technical-architecture.md) — overall stack. +- [Phase 11: Production Hardening, Operations, and Scale](../roadmap/phase-11-production-hardening.md) + — owns resource fairness and the three prepared seams. diff --git a/docs/architecture/26-hybrid-license-model.md b/docs/architecture/26-hybrid-license-model.md index 987d57a..9426b4d 100644 --- a/docs/architecture/26-hybrid-license-model.md +++ b/docs/architecture/26-hybrid-license-model.md @@ -15,14 +15,47 @@ The model handles all three deployment modes (SaaS / Dedicated / Self-Hosted) wi common implementation. This document describes the license payload, lifecycle, signing procedure, refresh cadence, revocation flow, and operational runbook. +## 0. Canonical key vocabulary + +The corpus has carried two incompatible spellings for the same cross-repository +projection: `classroom.recording` versus `classroom.recording.enabled`, and +`tenancy.max_learners` versus `limits.max_users` versus a bare `max_users`. Since the +payload is parsed by two repositories and cached in a third place, "close enough" means a +feature silently reads `false`. + +**One form is canonical, and it is the one fixed by +[ADR-0021 Amendment 1 (2026-05-18)](../decisions/0021-feature-based-entitlement.md) and +[21-feature-flags.md](21-feature-flags.md):** + +| Rule | Canonical | Not canonical | +|---|---|---| +| Every key is `{area}.{name}`, lowercase, dot-separated, `snake_case` within a segment | `classroom.recording` | `recording`, `Classroom.Recording` | +| Feature keys carry **no** `.enabled` suffix — a `FeatureKey` is boolean by construction | `classroom.recording` | `classroom.recording.enabled` | +| Limit keys are prefixed by their **subject area**, never by the word `limits` | `tenancy.max_learners` | `limits.max_users`, `max_users` | +| The area is the capability's owner, not the payload section it appears in | `media.storage_gb` | `limits.media_storage_gb` | + +Two further rules make the vocabulary enforceable rather than aspirational: + +- **A key that is not in `FeatureKeys` / `LimitKeys` may not appear in a payload.** The + registries in [21-feature-flags.md](21-feature-flags.md) are the closed set; adding a + key is a code change in both repositories plus a plan-editor update, not a Hub-side + string. +- **`FeatureKey_AllReferences_AreInRegistry`** (catalogued in + [Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md)) fails + the build on a free-form string, and the schema snapshot test in + [§ 5](#5-entitlement-read-path-and-grace-enforcement) fails on a payload that carries + an unregistered key. + +Every older spelling elsewhere in the corpus is superseded by this table. Where a +document still shows `limits.max_users`, it is stale, not an alternative. + ## 1. License key payload The key is a JWT-style RS256-signed token with a custom header `typ: "LSL"` -("LearnStack License"). The embedded `entitlement.features` key strings follow -the typed `FeatureKey` catalog in -[21-feature-flags.md](21-feature-flags.md) and -[ADR-0021 Amendment 1](../decisions/0021-feature-based-entitlement.md) — the -trailing `.enabled` suffix used in earlier drafts has been dropped. +("LearnStack License"). Its `entitlement` object is the **same shape** the Hub pushes +over `PUT /api/internal/tenants/{id}/entitlements` and the same shape +`platform_entitlement_cache` stores — one wire contract, three carriers — and it is +pinned by `entitlement-v1.schema.json` (see [§ 5](#5-entitlement-read-path-and-grace-enforcement)). ```json { @@ -51,13 +84,13 @@ trailing `.enabled` suffix used in earlier drafts has been dropped. "compliance.data_residency": true }, "limits": { - "limits.max_users": 50000, - "limits.max_organizations": 100, - "limits.classroom_minutes_per_month": -1, - "limits.recording_storage_gb": 10000, - "limits.media_storage_gb": 50000, - "limits.media_bandwidth_gb_per_month": -1, - "limits.api_rate_per_minute": 60000 + "tenancy.max_learners": 50000, + "tenancy.max_organizations": 100, + "classroom.minutes_per_month": -1, + "classroom.recording_storage_gb": 10000, + "media.storage_gb": 50000, + "media.bandwidth_gb_per_month": -1, + "integrations.api_rate": 60000 }, "compliance": { "caps": { @@ -186,46 +219,126 @@ public sealed class LicenseRefreshJob : LearnStackJob hit Hub at the same time. The job adds a random 0-119 minute offset per tenant when first enqueued. -## 5. Grace period enforcement +## 5. Entitlement read path and grace enforcement + +### What was wrong + +The earlier `HubEntitlementProvider.GetAsync` had two defects that cancelled out the +guarantee this whole document exists to provide: + +1. **No cold-start fallback.** On a cache miss it called the Hub unguarded. A Hub outage + therefore threw a transport exception **out of a feature-flag check** — so a pod + restart during a Hub incident turned "is recording enabled?" into a 500 on an + unrelated request path. +2. **The durable table was never read.** `platform_entitlement_cache` — the table that + carries `valid_until` and `grace_until` — did not appear anywhere in the read path. + Grace was evaluated against whatever happened to be in the distributed cache, whose + TTL is ≤ 15 minutes. The advertised **30-day grace period was, in practice, a 15-minute + cache TTL**: on eviction, the only source left was the Hub, which was the thing that + was down. -`HubEntitlementProvider.GetAsync(tenantId)`: +Those are the same defect seen twice: a *freshness* mechanism (cache TTL) was being used +as an *authorisation* mechanism (grace window). They are different clocks and they need +different storage. + +### The normative order + +[ADR-0034 § The entitlement read path](../decisions/0034-hub-contract-surface-invariant.md) +fixes the order, and the order is normative — an implementation may not skip a layer: + +```text +L1 in-process cache (per pod; seconds) + → L2 distributed cache (ICacheService; ≤ 15 min TTL — freshness only) + → platform_entitlement_cache (durable table; carries valid_until + grace_until) + → Hub POST /api/v1/internal/license/verify (last resort, always guarded) +``` ```csharp -public async Task GetAsync(Guid tenantId, CancellationToken ct) +public async Task GetAsync(TenantId tenantId, CancellationToken ct) { - var cached = await _cache.GetAsync(CacheKey(tenantId), ct); - if (cached is null) - { - // First access; fetch from Hub - cached = await _hubClient.VerifyAsync(tenantId, ct); - await _cache.SetAsync(CacheKey(tenantId), cached, ct); - return cached; - } + // 1-2. L1 → L2, both pure freshness layers. + if (await _cache.GetAsync(CacheKey(tenantId), ct) is { } cached) + return Evaluate(cached, source: EntitlementSource.Cache); - var now = _clock.UtcNow; - if (now < cached.ExpiresAt) - { - return cached; // Fresh; serve as-is - } + // 3. Durable projection. This is the layer that makes grace real: it survives pod + // restarts, cache flushes and Hub outages, and it is the only place valid_until / + // grace_until are authoritative. + var durable = await _entitlementCacheStore.FindAsync(tenantId, ct); - if (cached.GraceUntil is not null && now < cached.GraceUntil) + // 4. Hub — only when the durable row is missing or stale, and never unguarded. + if (durable is null || _clock.UtcNow >= durable.ValidUntil) { - _logger.LogWarning("Entitlement for {TenantId} in grace period (expired {Expiry}, grace until {Grace})", - tenantId, cached.ExpiresAt, cached.GraceUntil); - return cached with { InGracePeriod = true }; // Serve cached; flag for UI banner + try + { + var refreshed = await _hubClient.VerifyAsync(tenantId, ct); + await _entitlementCacheStore.UpsertAsync(refreshed, ct); // durable first + await _cache.SetAsync(CacheKey(tenantId), refreshed, ct); // then fast layers + return Evaluate(refreshed, source: EntitlementSource.Hub); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // A control-plane outage must not throw out of a feature-flag check. + _logger.LogWarning(ex, "Hub verify failed for {TenantId}; falling back", tenantId); + _metrics.HubVerifyFailed(); + if (durable is null) + return EntitlementLookup.Unresolved(tenantId); // policy table below decides + } } - // Past grace; read-only mode - return Entitlement.ReadOnly(tenantId, cached.Generation); + await _cache.SetAsync(CacheKey(tenantId), durable!, ct); + return Evaluate(durable!, source: EntitlementSource.Durable); } ``` -When in grace, the Admin Studio surfaces a banner: "Your license is in a 30-day grace -period. Contact support / renew via Hub." +`Evaluate` applies the lifecycle from [§ 3](#3-lifecycle): fresh → serve; past +`valid_until` but within `grace_until` → serve with `InGracePeriod = true`; past +`grace_until` → read-only. + +The provider **never throws** out of a feature-flag or limit check. Every path returns an +`EntitlementLookup` carrying the values, the source, and whether the answer is degraded. + +### Failure policy by key class + +`EntitlementLookup.Unresolved` — no cache, no durable row, no Hub — is the only genuinely +hard case, and one blanket answer is wrong for it. Each key class declares its posture +**explicitly in the registry**, so the behaviour is a property of the key rather than of +the call site: + +| Key class | Posture when unresolved | Why | +|---|---|---| +| Compliance caps (`compliance.*`, `audit.retention.days`, `data.residency.region`) | **Fail closed** — reject the operation that depends on the cap | An unknown residency or retention cap must never be read as permissive; a wrong answer here is a regulatory finding | +| Security-surface features (`identity.sso.saml`, `identity.scim`, `audit.export`, `integrations.api_access`) | **Fail closed** — treat as disabled | An unknown answer must not open an export or an API surface | +| Product capability features (`classroom.recording`, `classroom.breakout_rooms`, `tenancy.custom_domain`, `analytics.advanced_reporting`) | **Fail closed on a cold start, fail open to the last known value otherwise** | A paying tenant mid-class should not lose recording because the Hub is down — but the platform must not invent an entitlement it has never seen | +| Numeric limits (`tenancy.*`, `classroom.*`, `media.*`, `integrations.api_rate`) | **Fall back to the built-in floor** — the Starter-tier defaults compiled into the binary. Never `-1`, never `0` | Unlimited is a gift; zero is an outage. The floor keeps a tenant working at the smallest plan's ceiling until the answer arrives | + +Two consequences worth naming: -When past grace, every feature flag returns `false`; every limit returns `0`. Writes -return `Result.Failure("license.expired_read_only")`. Reads continue (so customers don't -lose access to their own data). +- Cold-start-unresolved is **rare and loud**: it requires an empty L1, an empty L2, no + durable row, and an unreachable Hub. It is alerted on + `learnstack_entitlement_unresolved_total{tenant_id}`, not silently absorbed. +- The source of every answer is observable — + `learnstack_entitlement_source_total{source}` over `cache | durable | hub | floor`. A + rising `durable` share means the Hub is degraded; a non-zero `floor` share means + tenants are being served the fallback and someone must know. + +### The wire contract is pinned + +The projection's shape is checked into **both** repositories as +`entitlement-v1.schema.json`, and a snapshot test in each fails the build when the +serialised payload drifts from it. This is what makes the read path above safe to +evolve: a Hub-side field rename that LearnStack cannot parse breaks a test in the Hub +repository, not a tenant's feature flag in production. + +The schema also encodes the vocabulary from [§ 0](#0-canonical-key-vocabulary) — +`features` and `limits` keys are validated against the registry — so an unregistered key +cannot reach a payload that LearnStack will cache. + +### UI surface + +When in grace, Admin Studio surfaces a banner: "Your license is in a 30-day grace period. +Contact support / renew via Hub." When past grace, every feature flag returns `false` and +every limit returns `0`; writes return `Result.Fail("license.expired_read_only")` and +reads continue, so customers never lose access to their own data. ## 6. Signed-key delivery (air-gapped) @@ -306,7 +419,8 @@ Hub operator portal exposes: | Customer extending license offline | License is RSA-signed; customer cannot forge a new signature | | Customer tampering with cached entitlement | Cache table protected by RLS + DB-level read-only role for non-admin paths | | Stolen Hub API key | Per-tenant API key; rotatable; scope limited to license verify + usage report | -| Revoked license still cached | Cache TTL ≤ 15 min; eager invalidation via Dapr event when revoked online; revocation list pulled daily | +| Revoked license still cached | Revocation invalidates **all four** layers, in the order that closes the window: `platform_entitlement_cache` row first, then L2, then L1 via the invalidation event. Invalidating only the fast layers leaves the durable row to re-serve the revoked entitlement on the next miss. Cache TTL ≤ 15 min and the daily revocation-list pull are the backstops, not the mechanism | +| Grace window silently collapsing to a cache TTL | `valid_until` / `grace_until` are read from `platform_entitlement_cache`, never from a cache entry. An integration test flushes both cache layers, stops the Hub, and asserts the tenant still resolves through the durable row for the full grace window | ## 10. Architecture tests @@ -317,17 +431,32 @@ Hub operator portal exposes: 3. `NullEntitlementProvider_RejectedInProduction` — runtime startup check: in any non- Development environment, `IEntitlementProvider` is `HubEntitlementProvider` or `SignedLicenseKeyEntitlementProvider`; never `NullEntitlementProvider`. -4. `LicenseKey_Payload_MatchesSchema` — `entitlement-v1.schema.json` snapshot test; any - breaking change requires schema-version bump. +4. `LicenseKey_Payload_MatchesSchema` — `entitlement-v1.schema.json` snapshot test, run + in **both** repositories against the same checked-in schema; any breaking change + requires a schema-version bump and a coordinated change in both. A snapshot test in + only one repository proves only that that repository is self-consistent. +5. `Entitlement_Read_Path_Falls_Through_To_Durable_Row` — integration test: flush L1 and + L2, make the Hub unreachable, assert the tenant resolves from + `platform_entitlement_cache` and that no exception escapes the feature-flag call. +6. `FeatureKey_AllReferences_AreInRegistry` — catalogued in + [Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md); + backs the vocabulary rule in [§ 0](#0-canonical-key-vocabulary). ## 11. Phasing -| Phase | Deliverable | -|-------|-------------| -| 02 | `IEntitlementProvider` interface in SharedKernel. `NullEntitlementProvider` default. `platform_entitlement_cache` table. `DeploymentMode` config enum. | -| 02c | `HubEntitlementProvider` (calls Hub `/internal/license/verify`); Hub-side `Entitlement` recompute on subscription change. | -| 09b | License-key issuance UI in Hub operator portal. | -| 11 | `SignedLicenseKeyEntitlementProvider` (air-gapped). Revocation list signing + distribution. Phone-home retry / backoff tuning. Grace period enforcement integration tests. SIGHUP hot-reload. Key rotation procedure documented. | +Per [ADR-0035](../decisions/0035-demand-gated-infrastructure.md), the port ships early +and each implementation ships against a written trigger. + +| Phase | Deliverable | Trigger | +|-------|-------------|---------| +| [02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) | `platform_entitlement_cache` table; `DeploymentMode` config enum | One-way door — the durable projection's schema and ownership | +| [02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md) | `IEntitlementProvider` socket; `NullEntitlementProvider` (all features enabled, no limits) as the **only** implementation | — | +| [02c](../roadmap/phase-02c-hub-foundation.md) | `HubEntitlementProvider` with the four-layer read path above; `entitlement-v1.schema.json` in both repositories; Hub-side `Entitlement` recompute on subscription change | A tenant must be billed or plan-gated | +| [09b](../roadmap/phase-09b-hub-billing.md) | License-key issuance UI in the Hub operator portal | Commercial billing needed | +| [11](../roadmap/phase-11-production-hardening.md) | `SignedLicenseKeyEntitlementProvider` (air-gapped); revocation-list signing + distribution; phone-home retry / backoff tuning; grace-period integration tests; SIGHUP hot-reload; key-rotation procedure | A Self-Hosted contract is signed | + +`NullEntitlementProvider` must not be registered outside `Development` once Phase 02c +lands (`NullEntitlementProvider_NotRegistered_OutsideDevelopment`). ## 12. Operational runbook (Phase 11) @@ -339,8 +468,13 @@ Hub operator portal exposes: ## References - ADR-0020 — Triple Deployment + Hybrid License. -- ADR-0021 — Feature-Based Entitlement. +- ADR-0021 (Amendment 1) — Feature-Based Entitlement; the canonical key vocabulary. - ADR-0019 — LearnStack Hub. +- [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — the normative + entitlement read path and the Hub contract surface invariants. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — which entitlement + implementation ships when. +- [21-feature-flags.md](21-feature-flags.md) — the `FeatureKeys` / `LimitKeys` registries. - [25-deployment-models.md](25-deployment-models.md) — three-mode topology. - [24-learnstack-hub.md](24-learnstack-hub.md) — Hub architecture. - Nexora reference: `Nexora/docs/decisions/0030-license-hot-reload-mechanism.md`, diff --git a/docs/architecture/27-custom-domain-tls.md b/docs/architecture/27-custom-domain-tls.md index bfa6070..c4d22b6 100644 --- a/docs/architecture/27-custom-domain-tls.md +++ b/docs/architecture/27-custom-domain-tls.md @@ -40,10 +40,11 @@ sequenceDiagram HubJob->>HubJob: Status=Verifying HubJob->>LE: ACME order via DNS-01 (or HTTP-01 fallback) LE-->>HubJob: Cert issued - HubJob->>HubAPI: Store cert in Vault; update CustomDomain
Status=Active, cert_expires_at=2026-08-16 - HubAPI->>HubAPI: Publish learnstack.hub.custom-domain.activated event
via Dapr pub/sub - HubAPI->>APISIX: Hot-reload route table partial
(add SNI cert + route entry) - HubAPI->>LSApi: Cache invalidation for host→tenant lookup + HubJob->>HubAPI: Store cert + key in the Hub secret store;
update CustomDomain Status=Active, cert_expires_at + HubAPI->>HubAPI: Replicate cert + key to the LearnStack-side secret store
(secret-store replication — never over HTTP payload) + HubAPI->>HubAPI: Publish learnstack.hub.custom-domain.activated event + HubAPI->>APISIX: Hot-reload route table partial
(SNI entry referencing the secret BY PATH) + HubAPI->>LSApi: PUT /api/internal/tenants/{id}/host-mappings
(host to tenant/org mapping only — no key material) else CNAME mismatch HubJob->>HubJob: Increment attempt; retry in 60s end @@ -165,11 +166,21 @@ routes: ssl: - id: ssl-anatoliayoga sni: anatoliayoga.com - # cert/key materialized via Vault Agent sidecar in production - cert_ref: vault://learnstack-hub/certs/anatoliayoga.com/cert - key_ref: vault://learnstack-hub/certs/anatoliayoga.com/key + # Materialised from the LEARNSTACK-side secret store by the secret-agent sidecar. + # The path is the replication target of the Hub-side secret; the value never + # travels in an HTTP payload. See § 6. + cert_ref: secret://learnstack/certs/anatoliayoga.com/cert + key_ref: secret://learnstack/certs/anatoliayoga.com/key ``` +> **Phasing note.** APISIX itself is demand-gated to +> [Phase 11](../roadmap/phase-11-production-hardening.md) per +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md); until it arrives, host +> routing and TLS termination are the deployment's ingress concern and +> `platform_host_to_tenant` remains the sole authority for host → tenant resolution +> inside the application. The route-partial mechanism described here is the target +> design, not a running system. + APISIX watches the file (standalone mode); changes are picked up within seconds. In Kubernetes, the file is in a ConfigMap; rolling update of the ConfigMap propagates; @@ -179,35 +190,112 @@ Etcd-backed APISIX (Phase 11+) replaces file-write with direct Admin API call. ## 6. Tenant resolver mapping -LearnStack runtime needs to map `Host: anatoliayoga.com` → `tenant_id`. Implementation: +LearnStack runtime needs to map `Host: anatoliayoga.com` → `(tenant_id, +organization_id?)`. + +### Host resolution never calls the Hub + +An earlier version of this document showed `CachedHostToTenantResolver` taking an +`IHubClient` and calling `LookupHostAsync` on every cache miss. That broke three rules at +once, and the third is the one that matters operationally: + +1. It was an **unrecorded endpoint** — no ADR, no entry in the contract surface, and no + counterpart in the Hub's own API documentation. +2. It was a **Hub call from outside the sanctioned adapters**. Only + `IEntitlementProvider`, `IUsageReporter` and `IHubTenantSync` may hold a Hub client + ([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)). +3. It put the **Hub on the hot path of anonymous public page loads**. Every cache miss on + every marketing page of every tenant became a synchronous dependency on the control + plane. A Hub outage — or a cold cache after a deploy during one — would have taken + every tenant's public site down, for a lookup whose answer LearnStack already stores. + +**`IHubClient.LookupHostAsync` is deleted.** `IHostToTenantResolver` reads +`platform_host_to_tenant` and nothing else: ```csharp namespace LearnStack.Infrastructure.MultiTenancy; public interface IHostToTenantResolver { - Task ResolveAsync(string host, CancellationToken ct = default); + Task ResolveAsync(string host, CancellationToken ct = default); } +public sealed record HostResolution(TenantId TenantId, OrganizationId? OrganizationId); + public sealed class CachedHostToTenantResolver( ICacheService cache, - IHubClient hubClient) : IHostToTenantResolver + TenancyDbContext db) : IHostToTenantResolver { - public async Task ResolveAsync(string host, CancellationToken ct = default) - { - return await cache.GetOrSetAsync( - $"hub:host:{host}", - async _ => await hubClient.LookupHostAsync(host, _), + public Task ResolveAsync(string host, CancellationToken ct = default) + => cache.GetOrSetAsync( + $"host:{host}", + async token => await db.HostMappings + .AsNoTracking() + .Where(m => m.Host == host && m.IsActive) + .Select(m => new HostResolution(m.TenantId, m.OrganizationId)) + .SingleOrDefaultAsync(token), new CacheOptions(L1Ttl: TimeSpan.FromMinutes(2), L2Ttl: TimeSpan.FromMinutes(15)), ct); - } } ``` -Cache invalidated on `learnstack.hub.custom-domain.activated` and -`learnstack.hub.custom-domain.revoked` Dapr pub/sub events. `TenantMiddleware` calls this -resolver first (before JWT validation, since the tenant context is needed for some -public anonymous routes too). +`platform_host_to_tenant` is a **platform-level table**, not tenant-owned — the row is +what *determines* the tenant, so it cannot be filtered by a tenant context that does not +exist yet. It is read before `ITenantContext` is populated, and the read is the only +database access in the request that legitimately runs unscoped. + +Consequences of the change: + +- A Hub outage degrades **billing and provisioning**. It does not touch page loads. +- The cache in front of the table is a latency optimisation, not an availability + mechanism. Even a total cache failure leaves a single indexed primary-key lookup. +- `TenantResolverMiddleware` calls this resolver first, before JWT validation, because + anonymous public routes need a tenant context too. + +### How mappings arrive + +Hub pushes host mappings over a dedicated endpoint, **not** through the entitlement +payload: + +```text +PUT /api/internal/tenants/{id}/host-mappings +``` + +The endpoint is part of the enumerated Hub → LearnStack surface in +[ADR-0034](../decisions/0034-hub-contract-surface-invariant.md), carries the same auth +chain as every other internal call (mTLS + RS256 JWT with `aud=learnstack-internal` + +HMAC body signature + `jti` replay protection), and is handled by `IHubTenantSync`. The +handler upserts `platform_host_to_tenant` and invalidates the resolver cache for the +affected hosts on `learnstack.hub.custom-domain.activated` / +`.revoked`. + +### Certificate material never rides the mapping payload + +The host-mapping payload carries **hosts and identifiers only**. TLS certificates and +private keys are never carried in an HTTP payload that LearnStack caches, logs, audits or +mirrors — which is precisely what the superseded design did when it tunnelled cert +material through `PUT /api/internal/tenants/{id}/entitlements`, a payload that lands in +`platform_entitlement_cache`. + +Key material moves by **secret-store replication** between the Hub-owned and +LearnStack-owned secret stores, and the mapping payload references it **by path**: + +```json +{ + "host": "anatoliayoga.com", + "tenant_id": "…", + "organization_id": null, + "certificate_ref": "learnstack/certs/anatoliayoga.com", + "is_active": true +} +``` + +`certificate_ref` is a path, resolvable only by a principal that already holds read +access to that secret store. Possession of the payload — in a log line, an audit row, a +cache entry, or a support ticket screenshot — grants nothing. +[ADR-0022 Amendment 1](../decisions/0022-custom-domain-tls.md)'s step 3 is superseded +accordingly; its central guarantee, that the Hub never holds Kubernetes credentials on +the LearnStack cluster, is unchanged. ## 7. Cert renewal @@ -301,16 +389,29 @@ instead of just `tenant_id`. always derived from authenticated session, never from request body / query. - `CustomDomain_Revocation_RemovesTenantResolverMapping` — integration test ends with the resolver returning null for the revoked host. +- `Hub_Client_Referenced_Only_By_Named_Adapters` — from + [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md); a resolver, a + middleware or a controller holding an `IHubClient` fails the build. This is the + mechanical guard against the deleted `LookupHostAsync` pattern reappearing. +- `Host_Resolution_Makes_No_Outbound_Calls` — integration test resolves a host with the + Hub client registered as a throwing stub and asserts the resolution still succeeds. ## 11. Phasing -| Phase | Deliverable | -|-------|-------------| -| 02a | `IHostToTenantResolver` interface + `CachedHostToTenantResolver` implementation in LearnStack. | -| 02c | `CustomDomain` aggregate in Hub. Submission endpoint scaffolded; verification logic stubbed (always returns success in dev). | -| 04 | Admin Studio custom-domain settings page: submission UI, DNS instructions, real-time status. | -| 09b | Hub operator portal: Pending Queue, Active List, Renewal Watch dashboards. Compliance caps editor includes domain-gating policy. | -| 11 | Production hardening: cert-manager + Let's Encrypt automation finalised; DNS provider API integrations (Cloudflare, Route 53, GCP DNS, Azure DNS); HTTP-01 fallback; renewal job tested at scale; APISIX hot-reload validated. | +| Phase | Deliverable | Trigger | +|-------|-------------|---------| +| [02a Packet 6](../roadmap/phase-02a-kernel-tenancy.md) | `platform_host_to_tenant` table | One-way door — the table that determines the tenant | +| [02a Packet 7](../roadmap/phase-02a-kernel-tenancy.md) | `IHostToTenantResolver` + `CachedHostToTenantResolver` reading `platform_host_to_tenant` and nothing else; two hosts wired to two seed tenants | — | +| [02d](../roadmap/phase-02d-walking-skeleton.md) | Host-based resolution exercised end to end: two hosts, two tenants, one binary | — | +| [02c](../roadmap/phase-02c-hub-foundation.md) | LearnStack-side `PUT /api/internal/tenants/{id}/host-mappings` handler behind `IHubTenantSync`; Hub-side `CustomDomain` aggregate and submission endpoint live in the Hub repository | A tenant is provisioned through the Hub | +| [04](../roadmap/phase-04-cms-media-pages.md) | Admin Studio custom-domain settings page: submission UI, DNS instructions, real-time status | — | +| [09b](../roadmap/phase-09b-hub-billing.md) | Hub operator portal: Pending Queue, Active List, Renewal Watch dashboards; compliance-caps editor includes domain gating | Commercial billing needed | +| [11](../roadmap/phase-11-production-hardening.md) | **The TLS automation itself**: ACME client, Let's Encrypt integration, DNS provider APIs (Cloudflare, Route 53, GCP DNS, Azure DNS), HTTP-01 fallback, renewal job at scale, secret-store replication, APISIX hot-reload validation | A tenant needs its own domain in production ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)) | + +The split is deliberate: **the mapping is a one-way door and ships early; the automation +that populates it is additive and ships on demand.** A tenant can run on a custom domain +before Phase 11 by having an operator insert the mapping row and place the certificate in +the secret store by hand. What Phase 11 removes is the operator, not the capability. ## 12. Operational runbook (Phase 11) @@ -333,7 +434,13 @@ instead of just `tenant_id`. ## References -- ADR-0022 — Custom Domain & TLS. +- ADR-0022 — Custom Domain & TLS. Amendment 1's step 3 (certificate material inside the + entitlement payload) is superseded by ADR-0034. +- [ADR-0034](../decisions/0034-hub-contract-surface-invariant.md) — Hub contract surface + invariant: the `host-mappings` endpoint, key material leaving the entitlement payload, + and the rule that host resolution never calls the Hub. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — when the TLS automation + ships. - ADR-0019 — LearnStack Hub. - ADR-0021 — Feature-Based Entitlement (gate). - ADR-0015 — APISIX Gateway (hot-reload of route table + SSL config). diff --git a/docs/architecture/28-platform-tenant-organization.md b/docs/architecture/28-platform-tenant-organization.md index c6dd405..587c964 100644 --- a/docs/architecture/28-platform-tenant-organization.md +++ b/docs/architecture/28-platform-tenant-organization.md @@ -252,19 +252,21 @@ operational discipline: Both repositories carry a row for a tenant. Each field belongs to **exactly one** of the two as the source of truth; the other side mirrors read-only and is invalidated -by integration events. The closed Hub HTTPS contract surface (four endpoints, -ADR-0019) means the only legitimate cross-system writes flow through one of those -endpoints. +by integration events. The Hub HTTPS contract surface +([ADR-0034](../decisions/0034-hub-contract-surface-invariant.md)) means the only +legitimate cross-system writes flow through one of its enumerated endpoints, and every +one of them goes through a named adapter — `IEntitlementProvider`, `IUsageReporter` or +`IHubTenantSync`. | Field | Authoritative side | Mirrored side | Sync direction & event | |-------|--------------------|---------------|------------------------| | `tenant_id` (UUID) | Hub (issued on tenant create) | LearnStack | One-time push at provisioning via `POST /api/internal/tenants` | | `slug` (handle) | Hub | LearnStack | `POST /api/internal/tenants` at create; renames require an ADR (none today) | | `display_name` | **LearnStack** | Hub | LearnStack publishes `learnstack.tenancy.tenant.renamed`; Hub consumer updates its mirror | -| `status` (Trial/Active/Suspended/Archived) | Hub | LearnStack | `PUT /api/internal/tenants/{id}/entitlements` carries the status; or a dedicated lifecycle endpoint to be added by future ADR | +| `status` (Trial/Active/Suspended/Archived) | Hub | LearnStack | `PUT /api/internal/tenants/{id}/status` | | `plan_code` | Hub | LearnStack | Carried inside entitlement projection (`PUT /api/internal/tenants/{id}/entitlements`) | | `features` / `limits` / `compliance` | Hub | LearnStack (`platform_entitlement_cache`) | `PUT /api/internal/tenants/{id}/entitlements`; eagerly invalidated by Dapr event `learnstack.hub.entitlement` | -| Custom-domain `host → tenant` mapping | Hub | LearnStack (`platform_host_to_tenant`) | Hub pushes after DNS / TLS verification; Dapr event `learnstack.hub.custom-domain.activated` / `.deactivated` | +| Custom-domain `host → tenant` mapping | Hub | LearnStack (`platform_host_to_tenant`) | Hub pushes after DNS / TLS verification via `PUT /api/internal/tenants/{id}/host-mappings`, which carries the tuple only — certificate material moves by secret-store replication and is referenced by path. LearnStack resolves hosts from `platform_host_to_tenant` and **never** calls the Hub | | Branding (logo, colours, typography) | **LearnStack** | Hub (denormalised summary only, optional) | LearnStack-side write only; no sync needed today | | Organizations + memberships | **LearnStack** | — | Hub does not mirror; tenant-internal shape | | Course / content / lesson / enrollment / progress data | **LearnStack** | — | Hub never stores tenant content (invariant #3 above) | @@ -274,9 +276,9 @@ endpoints. **Rename rule.** The two fields that can change after provisioning (`display_name`, branding) are LearnStack-authoritative — there is no Hub endpoint to -write them, only a LearnStack-published event the Hub consumer mirrors. This keeps the -four-endpoint contract closed; adding a Hub-side write for `display_name` would -require a new ADR. +write them, only a LearnStack-published event the Hub consumer mirrors. Adding a +Hub-side write for `display_name` would require a new ADR, because the contract surface +is a cross-repository agreement. **Mirror staleness budget.** The entitlement projection is **eagerly** invalidated via Dapr and otherwise has a 15-min TTL. The host → tenant mapping is invalidated by Dapr diff --git a/docs/architecture/30-api-gateway.md b/docs/architecture/30-api-gateway.md index 3933652..2adf961 100644 --- a/docs/architecture/30-api-gateway.md +++ b/docs/architecture/30-api-gateway.md @@ -7,6 +7,26 @@ LearnStack uses **Apache APISIX** in standalone mode as the API gateway in front of both the LearnStack core API and the LearnStack Hub API. +> **Status (2026-08-08).** APISIX is **demand-gated to +> [Phase 11](../roadmap/phase-11-production-hardening.md)** per +> [ADR-0035](../decisions/0035-demand-gated-infrastructure.md); its trigger is a non-development +> deployment that needs edge rate limiting, host routing, or JWT pre-validation. Until +> then the same responsibilities are carried by ASP.NET middleware inside the API +> process: correlation-id assignment, CORS, JWT bearer validation, rate limiting, and +> header normalisation. [ADR-0015](../decisions/0015-api-gateway-apisix.md) stands as the +> decision that APISIX is the gateway LearnStack uses; ADR-0035 decides when. +> +> **A gap worth naming rather than discovering.** In the shipped +> `infra/apisix/apisix.yaml`, route 100's `openid-connect` block is **commented out**, so +> today every `/api/v*` request reaches the backend unauthenticated **at the gateway**. +> That is a defense-in-depth gap, not an open door: the backend re-validates every token +> independently ([§ 5](#5-defense-in-depth-jwt-validated-twice)) and rejects +> unauthenticated requests on its own. But it means the gateway is currently contributing +> nothing to authentication, and the "validated twice" property in § 5 describes the +> Phase 11 target, not the running system. The block is uncommented in +> [Phase 03](../roadmap/phase-03-identity-admin.md), when the Keycloak realm it discovers +> against exists. + ## 1. Topology ```mermaid @@ -137,72 +157,153 @@ plugins: ## 4. Route table -`apisix.yaml` declares routes with priority bands: +Three rules govern this table, and each exists because its violation has already been +found in the corpus. + +### Rule 1 — the URI wildcard is a single trailing `*` + +`lua-resty-radixtree`, the matcher APISIX uses, supports **one** wildcard and only at the +**end** of the path. There is no `**` form, and a `*` in the middle of a path is not a +wildcard — it is a literal asterisk. A route declared as `/api/v*/**` therefore does not +mean "any version, any sub-path"; it means something no client will ever send. + +Because [ADR-0024](../decisions/0024-api-versioning-policy.md) puts the version in the +URL and the version set is small and enumerable, the correct spelling is one route per +live version: + +| Wrong | Right | +|---|---| +| `/api/v*/**` | `/api/v1/*` (and `/api/v2/*` when v2 ships) | +| `/api/v*/localization/*` | `/api/v1/localization/*` | + +Adding a version adds routes. That is the intended cost of URL versioning, and it keeps +the deprecation window in ADR-0024 visible in the gateway config rather than hidden +behind a wildcard. + +### Rule 2 — every route declares an explicit `priority` + +APISIX resolves overlapping routes by `priority`, **higher wins**, and an omitted +`priority` defaults to `0`. The `id` field is an identifier, not an ordering — a fact the +earlier version of this table obscured by using `id` values that looked like priority +bands. + +The consequence of that confusion was concrete: the public `GET /api/v*/localization/*` +route carried no `priority` (so `0`) while the authenticated catch-all carried +`priority: 100`. Both matched a localization request, the catch-all won, and **a route +documented as public anonymous would have demanded a bearer token** — an anonymous +learner loading a public page would have received 401 from the gateway. + +The banding is therefore **specific-beats-general, written down**: + +| Band | Priority | Contents | +|---|---|---| +| Infrastructure | 400 | `/healthz`, `/openapi/*` (environment-gated), `/admin/hangfire*` | +| Public anonymous | 300 | Named public paths: localization, public catalog reads, auth endpoints with their own strict limits | +| CORS preflight | 200 | `OPTIONS` on the versioned prefix — must beat the authenticated band so preflight never reaches `openid-connect` | +| Authenticated catch-all | 100 | Everything else under the versioned prefix | + +**A public route always outranks the catch-all.** The invariant is asserted in CI: the +route-table lint fails when any route omits `priority`, and when any route whose plugin +set lacks `openid-connect` shares a path prefix with a higher-or-equal-priority route +that has it. + +### Rule 3 — one trust domain per credential + +Route 100 previously carried +`client_secret_ref: vault://learnstack/hub/internal-api-hmac-key`. That path is the +**Hub internal API HMAC body-signing key** — a credential from an entirely different +trust domain, whose possession lets the holder forge a signed entitlement push for **any +tenant**. Handing it to the edge gateway as an OIDC client secret placed the platform's +highest-value secret on its most exposed component. + +The line is **deleted**, and no substitute is needed: the route is `bearer_only: true`, +which means APISIX validates a presented bearer token against the realm's public JWKS and +never performs a code exchange. A `bearer_only` OIDC route has no use for a client +secret at all. ```yaml routes: - # Public anonymous — priority 1 + # ---- Infrastructure band — priority 400 ------------------------------- - id: 1 - uri: /health + uri: /healthz + methods: [GET] + priority: 400 plugins: - limit-req: - rate: 10 - burst: 5 - rejected_code: 429 - key: remote_addr + limit-req: { rate: 10, burst: 5, key: remote_addr, rejected_code: 429 } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - id: 2 - uri: /api/v*/localization/* + uri: /openapi/* + priority: 400 + upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } + # Dev-only; environment-gated to 404 in production. + + # Hangfire dashboard — defense-in-depth: gateway BasicAuth + backend role check + - id: 3 + uri: /admin/hangfire* + priority: 400 + plugins: + basic-auth: {} # consumer credentials from the secret store + upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } + + # ---- Public anonymous band — priority 300 ----------------------------- + # MUST outrank the authenticated catch-all, or these become 401s. + - id: 10 + uri: /api/v1/localization/* methods: [GET] + priority: 300 plugins: - cors: - allow_origins: "https://app.learnstack.dev,https://hub.learnstack.dev" - limit-req: { rate: 50, burst: 20, key: remote_addr } + cors: { allow_origins: "https://app.learnstack.dev,https://hub.learnstack.dev" } + limit-req: { rate: 50, burst: 20, key: remote_addr, rejected_code: 429 } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - - id: 3 - uri: /openapi/* + - id: 11 + uri: /api/v1/auth/* + methods: [POST] + priority: 300 + plugins: + cors: { allow_origins: "https://app.learnstack.dev,*.learnstack.app" } + limit-count: { count: 5, time_window: 60, key: remote_addr, rejected_code: 429 } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - # Dev-only; rejected by environment-gated NGINX rule in production. - # CORS preflight separation — priority 99 (just above main authenticated band) - # OPTIONS bypasses openid-connect (which would reject preflight without Authorization header). - - id: 99 - uri: /api/v*/** + # ---- CORS preflight band — priority 200 ------------------------------- + # OPTIONS carries no Authorization header; openid-connect would reject it. + - id: 20 + uri: /api/v1/* methods: [OPTIONS] + priority: 200 plugins: cors: allow_origins: "https://app.learnstack.dev,*.learnstack.app" allow_methods: "GET,POST,PUT,PATCH,DELETE,OPTIONS" - allow_headers: "Authorization,Content-Type,X-Tenant-Id,X-Correlation-Id" + allow_headers: "Authorization,Content-Type,X-Tenant-Id,X-Organization-Id,X-Correlation-Id" max_age: 3600 - # Authenticated band — priority 100 + # ---- Authenticated catch-all — priority 100 --------------------------- - id: 100 - uri: /api/v*/** + uri: /api/v1/* methods: [GET, POST, PUT, PATCH, DELETE] + priority: 100 plugins: openid-connect: client_id: learnstack-gateway - client_secret_ref: vault://learnstack/hub/internal-api-hmac-key # via Vault Agent sidecar discovery: http://keycloak:8080/realms/learnstack/.well-known/openid-configuration - bearer_only: true - realm: learnstack + bearer_only: true # validates a presented token; no code exchange, + realm: learnstack # therefore no client secret — see Rule 3 scope: openid set_access_token_header: true access_token_in_authorization_header: true - cors: - allow_origins: "https://app.learnstack.dev,*.learnstack.app" - limit-req: { rate: 100, burst: 50, key: remote_addr } + cors: { allow_origins: "https://app.learnstack.dev,*.learnstack.app" } + limit-req: { rate: 100, burst: 50, key: remote_addr, rejected_code: 429 } request-id: { include_in_response: true } upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } - # Hub host band — priority 200 + # ---- Hub host — same banding, scoped by host -------------------------- - id: 200 host: hub.learnstack.dev - uri: /api/v*/** + uri: /api/v1/* methods: [OPTIONS] + priority: 200 plugins: cors: allow_origins: "https://hub.learnstack.dev" @@ -211,28 +312,27 @@ routes: - id: 201 host: hub.learnstack.dev - uri: /api/v*/** + uri: /api/v1/* methods: [GET, POST, PUT, PATCH, DELETE] + priority: 100 plugins: openid-connect: client_id: learnstack-hub-gateway discovery: http://keycloak:8080/realms/learnstack-hub/.well-known/openid-configuration bearer_only: true realm: learnstack-hub - cors: - allow_origins: "https://hub.learnstack.dev" - limit-req: { rate: 60, burst: 30, key: remote_addr } # stricter on Hub + cors: { allow_origins: "https://hub.learnstack.dev" } + limit-req: { rate: 60, burst: 30, key: remote_addr, rejected_code: 429 } # stricter on Hub request-id: { include_in_response: true } upstream: { type: roundrobin, nodes: { "learnstack-hub:5000": 1 } } - - # Hangfire dashboard — defense-in-depth: gateway BasicAuth + backend role check - - id: 300 - uri: /admin/hangfire* - plugins: - basic-auth: {} # consumer credentials in Vault - upstream: { type: roundrobin, nodes: { "learnstack-api:5000": 1 } } ``` +The corrections in Rules 1–3 land in the shipped `infra/apisix/apisix.yaml` in +[Phase 03](../roadmap/phase-03-identity-admin.md), alongside uncommenting the +`openid-connect` block against the realm that exists by then. APISIX becomes a real +gateway — TLS termination, consumer-keyed limits, WAF — in +[Phase 11](../roadmap/phase-11-production-hardening.md). + ## 5. Defense-in-depth: JWT validated twice **Why validate JWT both at APISIX and at the backend?** @@ -247,6 +347,13 @@ If APISIX is misconfigured, bypassed (someone hits the backend directly on the i network), or compromised, the backend still rejects unauthenticated requests. This is non-negotiable. +That property is what makes today's state a gap rather than a breach: with route 100's +`openid-connect` block commented out, layer 1 is absent and layer 2 carries the whole +load. The system is not open — it is running on one line of defense where the design +calls for two, and the cheap fast rejection at the edge is not happening. Restoring +layer 1 is [Phase 03](../roadmap/phase-03-identity-admin.md) work; nothing about the +backend's obligations changes when it lands. + ```csharp // LearnStack.Host/Program.cs builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) @@ -274,15 +381,19 @@ builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) | Surface | Rate | Burst | Key | |---------|------|-------|-----| -| `/health` | 10/s | 5 | remote_addr | -| `/api/v*/localization/*` | 50/s | 20 | remote_addr | +| `/healthz` | 10/s | 5 | remote_addr | +| `/api/v1/localization/*` | 50/s | 20 | remote_addr | +| `/api/v1/auth/*` (login, password reset) | 5/min | 0 | remote_addr | | Authenticated GET/POST/PUT/PATCH/DELETE (main API) | 100/s | 50 | remote_addr | | Hub authenticated API | 60/s | 30 | remote_addr | -| `/api/v*/auth/*` (login, password reset) | 5/min | 0 | remote_addr | -| Write endpoints (POST/PUT/PATCH/DELETE) on authenticated routes | 60/min/token | — | consumer (when consumer-key authn enabled, Phase 11) | +| Write endpoints (POST/PUT/PATCH/DELETE) on authenticated routes | 60/min/token | — | consumer (when consumer-key authn is wired, Phase 11) | -Phase 11+ moves from `remote_addr`-keyed limits to consumer-key-keyed limits when APISIX -consumer support is wired (etcd-backed mode required for dynamic consumer issuance). +`remote_addr` keying limits a noisy **client**, not a noisy **tenant**: one tenant behind +many addresses is unaffected, and many tenants behind one NAT are throttled together. +Per-tenant fairness — at the gateway and inside the database — is +[Phase 11](../roadmap/phase-11-production-hardening.md) work, described in +[25-deployment-models.md § 2](25-deployment-models.md). Consumer-key-keyed limits require +etcd-backed mode for dynamic consumer issuance and land in the same phase. ## 7. CORS preflight separation @@ -290,9 +401,11 @@ The `openid-connect` plugin **rejects OPTIONS requests** because they don't carr `Authorization` header. This breaks browser CORS preflight unless OPTIONS is handled separately. -Solution: a higher-priority OPTIONS-only route (id 99 / 200) carries the `cors` plugin -without `openid-connect`. The browser's preflight succeeds; the subsequent actual -request (with the `Authorization` header) hits the authenticated route. +Solution: an OPTIONS-only route in the **preflight band (priority 200)** carries the +`cors` plugin without `openid-connect`. Because 200 outranks the authenticated +catch-all's 100, the browser's preflight succeeds; the subsequent actual request, with +its `Authorization` header and a non-OPTIONS method, falls through to the authenticated +route. Nexora pattern verbatim (see `Nexora/docs/decisions/0003-deployment-strategy.md` and `Nexora/docs/operations/HELM_INSTALLATION.md`). @@ -351,10 +464,18 @@ Phase 11 deliverables: ## 11. Architecture tests - `Apisix_RouteYaml_IsValid` — CI step running `apisix test` against `apisix.yaml`. +- `Apisix_Routes_Declare_Explicit_Priority` — route-table lint: every route sets + `priority`; a default-`0` route is a latent ordering bug (Rule 2). +- `Apisix_Public_Routes_Outrank_Authenticated_Catchall` — route-table lint: no route + lacking `openid-connect` shares a path prefix with a higher-or-equal-priority route + that has it (Rule 2). +- `Apisix_Uri_Patterns_Are_RadixtreeValid` — route-table lint: at most one `*`, and only + as the final path segment (Rule 1). - `Apisix_NeverFronts_InternalApi` — integration test asserts requests to `app.learnstack.dev/api/internal/*` return 404 from APISIX (route not defined). - `Backend_RequiresJwt_OnAllAuthenticatedRoutes` — `WebApplicationFactory` integration test: every endpoint outside the public allow-list returns 401 without a bearer token. + This is the test that holds the line while the gateway's OIDC block is commented out. ## 12. Non-goals @@ -367,7 +488,11 @@ Phase 11 deliverables: ## References -- ADR-0015 — API Gateway with APISIX. +- ADR-0015 — API Gateway with APISIX (what). +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — Demand-gated + infrastructure (when), and what carries the gateway's responsibilities until then. +- [ADR-0024](../decisions/0024-api-versioning-policy.md) — URL versioning; why the route + table enumerates versions instead of wildcarding them. - ADR-0004 Amendment 1 — `learnstack-hub` realm. - [11-security.md](../standards/11-security.md) — defense-in-depth. - [13-identity-and-auth.md](13-identity-and-auth.md) — Keycloak realm diff --git a/docs/architecture/31-audit-subsystem.md b/docs/architecture/31-audit-subsystem.md index 37ac7f6..b0a92e9 100644 --- a/docs/architecture/31-audit-subsystem.md +++ b/docs/architecture/31-audit-subsystem.md @@ -1,42 +1,81 @@ # Audit Subsystem -**Derives from:** [ADR-0016](../decisions/0016-audit-log-subsystem.md), +**Derives from:** [ADR-0033](../decisions/0033-audit-durability-model.md) +(supersedes [ADR-0016](../decisions/0016-audit-log-subsystem.md)), [ADR-0017 (Tenant + Organization)](../decisions/0017-tenant-organization-hierarchy.md), [18-audit-coverage.md](../standards/18-audit-coverage.md). The audit subsystem captures and persists an append-only history of security- and compliance-relevant operations across every LearnStack module. This document describes -the three-piece pipeline, data model, retention, redaction, and operational concerns. +the pipeline, the durability model, the data model, retention, redaction, and operational +concerns. -## 1. Pipeline overview +## 1. Two durability classes + +The single most important thing to understand about this subsystem is that **audit is not +one mechanism**. ADR-0016 treated it as one and inherited a contradiction: Standards 18 +required MUST-class rows to be written in the same transaction as the change they record, +while ADR-0016 required that audit never block business logic. Under the shipped MediatR +order — `Validation → Logging → AuditLog → TenantContext → Authorization → Transaction → +OutboxFlush → Handler` — `AuditLogBehavior` wraps `TransactionBehavior` from the outside, +so its write lands **after** the business transaction has already committed or rolled +back. Both requirements could not hold. + +Worse, once the corrected Row Level Security template from +[ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) lands, an +audit insert executed outside that transaction runs with no `app.tenant_id` set. The +policy's `WITH CHECK` rejects the row, and a catch-and-log posture swallows the +rejection: the audit log would record nothing while reporting success. A silent, complete +audit failure is strictly worse than a loud one. + +[ADR-0033](../decisions/0033-audit-durability-model.md) resolves it by splitting the +classes rather than reordering the pipeline: + +| Class | Where the row is written | On failure | Rationale | +|---|---|---|---| +| **MUST** — security, compliance, privileged access | **Inside the business transaction**, enrolled in the same `DbContext.SaveChanges` as the state change | **Fail closed** — the business operation is rejected | For these events the audit row *is* part of the operation's contract. "Platform admin read tenant B's learner records" with no audit row is an audit finding | +| **SHOULD / MAY** — operational, diagnostic | Outside the transaction, best-effort | Logged and dropped; the accepted loss is written down, not assumed | Losing "course renamed" costs a support conversation | + +Enrichment, redaction, projection and external fan-out always happen **after** the +commit, reading the durable row. ADR-0016's "audit never blocks business logic" is +preserved for that second stage and withdrawn for the first. + +## 2. Pipeline overview ```mermaid -flowchart LR +flowchart TB Cmd["Command / Query / Action"] --> Behavior["AuditLogBehavior
(MediatR pipeline)"] - Behavior --> Handler["Module handler
(business logic)"] - Handler --> SaveChanges["DbContext.SaveChangesAsync"] + Behavior --> Config["IAuditConfigService
classify MUST / SHOULD / MAY"] + Config -->|"lookup fails"| Closed["FAIL CLOSED
reject the operation"] + Config --> Handler["Module handler
(business logic)"] + Handler --> Enroll["MUST-class: IAuditStore enrols the row
in the SAME DbContext"] + Enroll --> SaveChanges["DbContext.SaveChangesAsync"] SaveChanges --> Interceptor["AuditChangeTrackerInterceptor
(EF SaveChangesInterceptor)"] Interceptor --> Buffer["IAuditStateCapture
(scoped buffer)"] - Handler --> ReturnsResult["Handler returns Result"] - ReturnsResult --> Behavior - Behavior --> Buffer - Behavior --> Build["Build AuditEntry"] - Build --> Store["IAuditStore.SaveAsync"] - Store --> Table[("audit_log
(partitioned by month)")] + SaveChanges --> Commit[("COMMIT — business rows + MUST audit row,
atomically, with app.tenant_id set")] + Commit --> Behavior + Behavior --> Post["After commit: enrich, redact, project,
fan out via outbox; SHOULD/MAY written here"] ``` -Three pieces, separated concerns: +Read as text: the behavior classifies the operation; a configuration-read failure rejects +the operation rather than proceeding unaudited. The handler runs, and for a MUST-class +operation the audit row is enrolled in the same `SaveChanges` as the business write — so +it commits with it or not at all, and so it executes while `app.tenant_id` is set and Row +Level Security accepts it. Everything after the commit — enrichment, redaction, external +fan-out, and SHOULD/MAY rows — is best-effort and never blocks. -1. **`AuditChangeTrackerInterceptor`** — runs inside `DbContext.SaveChangesAsync`, - walks the ChangeTracker, snapshots state for every entity inheriting `AuditableEntity`. -2. **`IAuditStateCapture`** — a scoped (per-request) buffer that holds entity snapshots +Three components, separated concerns: + +1. **`AuditChangeTrackerInterceptor`** — runs inside `DbContext.SaveChangesAsync`, walks + the ChangeTracker, snapshots state for every entity inheriting `AuditableEntity`. +2. **`IAuditStateCapture`** — a scoped (per-request) buffer holding entity snapshots until the MediatR behavior reads them. -3. **`AuditLogBehavior`** — wraps the handler, awaits it (catching - exceptions to still audit failed operations), reads the buffer, builds one `AuditEntry` - per request, writes via `IAuditStore.SaveAsync`. Failure to write never blocks the - business response. +3. **`AuditLogBehavior`** — keeps its shipped position and its + shipped responsibility: catch handler exceptions, record the outcome, rethrow via + `ExceptionDispatchInfo`. What changed is that the MUST-class row it records is + **already durable** by the time it runs. -## 2. The interceptor +## 3. The interceptor ```csharp namespace LearnStack.Infrastructure.Audit; @@ -91,7 +130,7 @@ Pattern explicitly mirrors Nexora's `AuditChangeTrackerInterceptor` (see `Nexora/docs/decisions/0009-audit-repository-pattern.md`) — verbatim port to LearnStack naming. -## 3. The scoped buffer +## 4. The scoped buffer ```csharp namespace LearnStack.SharedKernel.Abstractions.Audit; @@ -119,7 +158,7 @@ public sealed class AuditStateCapture : IAuditStateCapture Registered as **scoped** in DI (per-request lifetime). Cleared at the end of every request to prevent cross-request bleed (architecture test enforces this). -## 4. The MediatR behavior +## 5. The MediatR behavior ```csharp namespace LearnStack.Infrastructure.Behaviors; @@ -141,22 +180,35 @@ public sealed class AuditLogBehavior( if (requestKind == RequestKind.Other) return await next(); var (module, operation) = ExtractModuleAndOperation(request, requestKind); - var defaultEnabled = requestKind == RequestKind.Command; - bool auditEnabled; + // Classification is a decision, not a toggle: it returns Must | Should | May | Off. + // A tenant AuditConfig override may narrow Should/May. It can never remove + // baseline Must coverage — the catalogue's Must floor is applied after the + // override, not before it. + AuditClassification classification; try { - auditEnabled = await configService.IsEnabledAsync(module, operation, ct, defaultEnabled); + classification = await configService.ClassifyAsync(module, operation, ct); } catch (Exception ex) { - // Audit config check failed → skip audit; never block business write. - logger.LogError(ex, "Audit config check failed for {Module}.{Operation}", + // FAIL CLOSED. A single unreadable config row, or a config-store outage, + // must not silently switch off mandatory security auditing. Rejecting the + // operation is loud, recoverable, and visible; proceeding unaudited is none + // of those. Per ADR-0033. + logger.LogError(ex, "Audit classification unavailable for {Module}.{Operation}; rejecting", module, operation); - return await next(); + return Result.FailFor(AuditErrors.ConfigurationUnavailable); } - if (!auditEnabled) return await next(); + if (classification == AuditClassification.Off) return await next(); + + // MUST-class: the audit row is enrolled by IAuditStore into the SAME DbContext + // the handler writes through, so it commits with the business write or not at + // all — and so it executes with app.tenant_id set. If the enrolment or the + // commit fails, the whole operation fails; that is the point. + if (classification == AuditClassification.Must) + auditStore.EnrolDurableIntent(BuildIntent(module, operation, request)); TResponse response; bool handlerFailed = false; @@ -201,12 +253,17 @@ public sealed class AuditLogBehavior( Changes: SerializeChanges(stateCapture.Changes), Timestamp: DateTimeOffset.UtcNow); - await auditStore.SaveAsync(entry, ct); + // MUST-class: completes the already-durable row (enrichment only — the row's + // existence is not in question here). SHOULD/MAY-class: writes it now, + // outside the transaction, best-effort. + await auditStore.CompleteOrSaveAsync(entry, classification, ct); } - catch (Exception ex) + catch (Exception ex) when (classification != AuditClassification.Must) { - // Audit save failed → log, never block business write. - logger.LogError(ex, "Audit save failed for {Module}.{Operation}", module, operation); + // SHOULD/MAY only: log and drop. The accepted loss is written down in the + // module's audit-coverage matrix, not assumed. + logger.LogError(ex, "Best-effort audit save failed for {Module}.{Operation}", + module, operation); } finally { @@ -220,22 +277,32 @@ public sealed class AuditLogBehavior( } // ClassifyRequest, ExtractModuleAndOperation, DetermineOutcome, DeriveOperationType, - // DeriveOperationClass, SerializeBefore/After/Changes are private static helpers. + // BuildIntent, SerializeBefore/After/Changes are private helpers. } ``` Key invariants enforced by this behavior: -- **Audit failure never blocks business write.** The `try { auditStore.SaveAsync(...) } - catch { log }` and the surrounding `finally { stateCapture.Clear(); }` guarantee that - even a totally broken audit store doesn't reject business commands. +- **Audit configuration failure fails closed.** A configuration-store outage, or a single + unreadable `audit_config` row, rejects the operation. It does not switch off mandatory + security auditing — which is exactly what the previous `catch → return await next()` + did, and what made a config-store incident indistinguishable from a period of clean + behaviour in the log. +- **MUST-class audit failure fails the operation.** The exception filter above + deliberately excludes `Must`, so a MUST-class audit problem propagates. This is a real + availability trade-off, stated in + [ADR-0033 § Consequences](../decisions/0033-audit-durability-model.md) and required to + be visible in the operational runbooks. +- **SHOULD/MAY audit failure never blocks the business write.** The cheap path stays + cheap; the platform does not pay compliance-grade cost for "a course was renamed". - **Failed handlers still get audited.** The behavior catches the handler exception, - writes an audit entry with `IsSuccess=false`, then re-throws via `ExceptionDispatchInfo` - to preserve the original stack trace. -- **Per-(module, operation) toggling.** `IAuditConfigService.IsEnabledAsync` reads from - per-tenant config (`audit_config` table) with module-declared defaults. + records `IsSuccess=false`, then rethrows via `ExceptionDispatchInfo` so the original + stack trace survives. +- **A tenant override cannot remove MUST coverage.** `IAuditConfigService.ClassifyAsync` + applies the per-tenant `audit_config` override and then re-applies the catalogue's MUST + floor. A tenant may audit *more* than the baseline, never less. -## 5. Pipeline order +## 6. Pipeline order MediatR pipeline behaviors are registered in this order in `LearnStack.Infrastructure.DependencyInjection`: @@ -244,7 +311,7 @@ MediatR pipeline behaviors are registered in this order in services.AddTransient(typeof(IPipelineBehavior<,>), typeof(ValidationBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(LoggingBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(AuditLogBehavior<,>)); -services.AddTransient(typeof(IPipelineBehavior<,>), typeof(TenantContextCheckBehavior<,>)); +services.AddTransient(typeof(IPipelineBehavior<,>), typeof(TenantContextBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(AuthorizationBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(TransactionBehavior<,>)); services.AddTransient(typeof(IPipelineBehavior<,>), typeof(OutboxFlushBehavior<,>)); @@ -256,15 +323,30 @@ Effective execution order (outer → inner): Request → Validation (reject before any work; FluentValidation) → Logging (request scope; correlation id) - → AuditLog (wraps the rest; audit failure never blocks) - → TenantContextCheck (assert tenant_id resolved) + → AuditLog (classifies; fails closed on config failure) + → TenantContext (assert tenant_id resolved) → Authorization (resource-scoped checks beyond endpoint-level [Authorize]) - → Transaction (begin transaction; commit/rollback on outcome) + → Transaction (begin transaction; SET LOCAL app.tenant_id; commit/rollback) → OutboxFlush (flush outbox writes to DbContext before commit) → Handler (business logic) ``` -## 6. Data model +**The order did not change, and does not need to.** `AuditLogBehavior` still sits outside +`TransactionBehavior`, which is exactly why its *own* write cannot be the durable one. +The durable MUST-class row is enrolled through `IAuditStore` into the `DbContext` that +`TransactionBehavior` owns, so it is flushed and committed by that inner behavior — the +row travels inward through the pipeline even though the behavior that decided on it sits +outward. `AuditLogBehavior` then completes the row's enrichment on the way back out. + +The alternative — moving `TransactionBehavior` outward so it wraps `AuditLogBehavior` — +was considered and rejected in +[ADR-0033 § Considered Options](../decisions/0033-audit-durability-model.md): it would +also drag `TenantContext` and `Authorization` inside the transaction and open the +transaction before validation has finished, changing a shipped, test-asserted global +ordering (`MediatR_Pipeline_Order_Matches_Canonical_Sequence`) to solve a problem +belonging to one behavior. + +## 7. Data model ### `AuditEntry` aggregate @@ -325,6 +407,29 @@ public enum OperationClass { Must, Should, May } ### `audit_log` table +`audit_log` ships in [Phase 02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md) as a +**single, plain, correct table**. Monthly partitioning, the partition-management job, and +the retention purge from [ADR-0028](../decisions/0028-audit-log-partition-management.md) +move to [Phase 11](../roadmap/phase-11-production-hardening.md) per +[ADR-0035](../decisions/0035-demand-gated-infrastructure.md), against the trigger +"measured `audit_log` growth justifies partition maintenance". Audit **correctness** +cannot be added later; audit **scale** can, and the platform has no rows yet to scale. + +Two details that a partition-ready design gets wrong if it is copied carelessly: + +- The primary key is the **composite** `(id, timestamp)`. ADR-0016's DDL declared a + primary key twice — inline on `id` and again as a table constraint on `(id, timestamp)` + — and PostgreSQL rejects that table outright. When partitioning arrives, a partitioned + table must include every partition-key column in its primary key, so the composite is + the correct one and the inline declaration was the error. Shipping the composite now + means Phase 11 adds `PARTITION BY RANGE (timestamp)` without a key migration. +- Cross-tenant reads use **`learnstack_platform`**, entered through the audited + `EnterPlatformAdminScope(reason)` path. There is no separate `learnstack_audit_admin` + role: the database role model fixed by + [ADR-0003 Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) has + exactly four roles, and every additional `BYPASSRLS` role is a hole in the isolation + model that would need its own ADR. + ```sql CREATE TABLE audit_log ( id uuid NOT NULL, @@ -348,14 +453,11 @@ CREATE TABLE audit_log ( user_agent text NULL, timestamp timestamptz NOT NULL DEFAULT now(), metadata jsonb NULL, - PRIMARY KEY (id, timestamp) -) PARTITION BY RANGE (timestamp); - -CREATE TABLE audit_log_2026_05 PARTITION OF audit_log - FOR VALUES FROM ('2026-05-01') TO ('2026-06-01'); -CREATE TABLE audit_log_2026_06 PARTITION OF audit_log - FOR VALUES FROM ('2026-06-01') TO ('2026-07-01'); --- ... auto-created monthly by a Hangfire recurring job (LearnStackJob) + CONSTRAINT audit_log_pkey PRIMARY KEY (id, timestamp) +); +-- Phase 11 adds PARTITION BY RANGE (timestamp) and the monthly partitions. +-- The composite key above is already partition-compatible, so that change is +-- additive rather than a key migration. CREATE INDEX ix_audit_log_tenant_timestamp ON audit_log (tenant_id, timestamp DESC); @@ -368,13 +470,25 @@ CREATE INDEX ix_audit_log_correlation CREATE INDEX ix_audit_log_module_operation_timestamp ON audit_log (module, operation, timestamp DESC); --- RLS +-- RLS: built from the canonical template in Database Standards § Tenant-Owned and +-- Organization-Scoped Tables — one AND-ed policy, ENABLE *and* FORCE, explicit +-- WITH CHECK. Do not hand-write it here; the template is the single source of truth. ALTER TABLE audit_log ENABLE ROW LEVEL SECURITY; -CREATE POLICY audit_log_tenant_isolation ON audit_log - USING (tenant_id = current_setting('app.tenant_id', true)::uuid); --- Platform admin role bypasses RLS via SET role learnstack_audit_admin (audited). +ALTER TABLE audit_log FORCE ROW LEVEL SECURITY; +CREATE POLICY audit_log_isolation ON audit_log + USING (tenant_id = current_setting('app.tenant_id', true)::uuid) + WITH CHECK (tenant_id = current_setting('app.tenant_id', true)::uuid); +-- Cross-tenant reads run as learnstack_platform, entered through the audited +-- EnterPlatformAdminScope(reason) path. ``` +The `WITH CHECK` clause is the reason MUST-class audit **has** to be written inside the +business transaction. Outside it, `app.tenant_id` is unset, `current_setting` returns +null, the predicate fails, and the insert is rejected — which the old catch-and-log +posture would have swallowed. See +[Database Standards](../standards/05-database.md) for the template and +[ADR-0033](../decisions/0033-audit-durability-model.md) for the durability rule. + ### `audit_config` table ```sql @@ -389,14 +503,22 @@ CREATE TABLE audit_config ( UNIQUE (tenant_id, module, operation) ); ALTER TABLE audit_config ENABLE ROW LEVEL SECURITY; -CREATE POLICY audit_config_tenant_isolation ON audit_config - USING (tenant_id = current_setting('app.tenant_id', true)::uuid); +ALTER TABLE audit_config FORCE ROW LEVEL SECURITY; +CREATE POLICY audit_config_isolation ON audit_config + USING (tenant_id = current_setting('app.tenant_id', true)::uuid) + WITH CHECK (tenant_id = current_setting('app.tenant_id', true)::uuid); ``` Defaults declared in each module via `IModule.RegisterAuditDefaults()`; the table holds per-tenant overrides only. -## 7. Per-module coverage matrix (baseline) +`is_enabled` is deliberately **not** the whole story. A row here can narrow SHOULD/MAY +coverage; it cannot switch off an operation the catalogue classifies MUST. +`ClassifyAsync` applies the override and then re-applies the MUST floor, and a read +failure against this table rejects the operation rather than defaulting it — see +[§ 5](#5-the-mediatr-behavior). + +## 8. Per-module coverage matrix (baseline) Standard 18 defines the MUST / SHOULD / MAY matrix per module. Excerpt for Tier-1 core modules: @@ -417,7 +539,7 @@ modules: Hub-side modules audit to **Hub's own audit stream**, separate from LearnStack's; same shape, different table. -## 8. Retention +## 9. Retention Default retention by operation class (per-tenant overridable within plan limits): @@ -429,18 +551,22 @@ Default retention by operation class (per-tenant overridable within plan limits) | ReadSensitive | **2 years** | 6 months – 5 years | | Other Action | **1 year** | 3 months – 3 years | -Retention purge runs as a Hangfire recurring job (`LearnStackJob` analog): +Both retention jobs run **daily**. Three documents previously disagreed — daily in +[Audit Coverage Standards](../standards/18-audit-coverage.md) and +[ADR-0028](../decisions/0028-audit-log-partition-management.md), weekly here — and this +document was the outlier. Daily is correct: a weekly purge means a tenant's stated +retention can be exceeded by up to six days, which is a compliance answer nobody wants to +give. + +Both jobs land in [Phase 11](../roadmap/phase-11-production-hardening.md) alongside +partitioning; until then `audit_log` is a single table and nothing purges it. -- `learnstack:audit:partition-management` — runs daily; creates next month's partition - (if not exists); drops partitions older than max retention window across all tenants - (10y for safety; tenant-specific retention enforced by row-level deletes within - partition). -- `learnstack:audit:retention-purge` — runs weekly; deletes individual rows per tenant's - configured retention. Uses tenant-config retention values; bypasses RLS via - `learnstack_audit_admin` role; the purge itself emits a `SecurityEvent` audit row - summarising what was deleted. +| Hangfire recurring job | Cadence | What it does | +|---|---|---| +| `learnstack:audit:partition-management` | Daily | Creates next month's partition if absent; drops partitions older than the maximum retention window across all tenants (10y for safety) | +| `learnstack:audit:retention-purge` | **Daily** | Deletes individual rows per the tenant's configured retention, in batches. Runs as `learnstack_platform` through the audited platform-admin scope. The purge itself emits a `SecurityEvent` audit row summarising what was deleted | -## 9. GDPR / PII redaction +## 10. GDPR / PII redaction When a user is GDPR-erased (`UserGdprDeletedIntegrationEvent` published), audit rows containing that user's PII are **redacted in place** (not deleted — the audit row's @@ -488,7 +614,7 @@ Every module that stores user references in audit payloads must register an `IUserReferenceLocator` implementation (architecture test enforces this — same shape as Nexora's `IContactReferenceLocator`). -## 10. Querying audit log +## 11. Querying audit log The Audit module's admin API exposes: @@ -508,7 +634,7 @@ Required permission: `audit.events.read` (tenant scope). Required for export: `audit.events.export`. Required for cross-tenant query (platform admin only): `platform.audit.events.read`. -## 11. Hub-side audit stream +## 12. Hub-side audit stream Hub maintains a parallel audit stream in its own database (`hub_audit_log`), capturing operator actions: @@ -523,43 +649,66 @@ operator actions: Cross-stream correlation by `correlation_id`. A regulatory inquiry covering "what happened to tenant X on date Y" pulls from both streams and joins by correlation. -## 12. Architecture tests - -Five blocker-level tests added in Phase 02: - -1. `Every_Command_HasAuditCoverage` — auto-discovers commands by interface - (`ICommand`); for each, asserts the (module, operation) appears in - `IAuditConfigService.GetCoverageMatrix()` with at least SHOULD classification. -2. `AuditEntry_Is_AppendOnly` — `AuditEntry` does not implement `ISoftDeletable`; has no - `Update*` public methods; reflection assertion. -3. `AuditLogBehavior_NeverBlocks_BusinessWrites` — integration test asserts that when - `IAuditStore.SaveAsync` throws, the command result is still returned (handler success - path). -4. `AuditStateCapture_ClearedPerRequest` — integration test asserts that after a request - completes (success or failure), the scoped `IAuditStateCapture.Changes` collection is - empty for the next request. -5. `Every_PII_Module_RegistersUserReferenceLocator` — modules storing user references in - audit payloads (declared via `[StoresUserReference]` attribute or - `IModule.RegisterUserReferences()`) must register an `IUserReferenceLocator` impl. - -## 13. Phasing +## 13. Architecture tests + +Blocker-level tests, registered in +[Architecture Tests Catalogue](../standards/21-architecture-tests-catalogue.md) by +[Phase 02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md): + +1. `Every_TenantOwned_Command_HasAuditCoverage` — auto-discovers commands by interface; + for each, asserts the (module, operation) appears in the coverage matrix with at least + a SHOULD classification. +2. `AuditEntry_Inherits_Entity_Not_AuditableEntity` — append-only by construction: an + audit row that carries `UpdatedAt` / `DeletedAt` is a contradiction. +3. `MustClass_Audit_Writes_Share_The_Business_Transaction` — the binding test for + [ADR-0033](../decisions/0033-audit-durability-model.md). One command produces exactly + one `audit_log` row; a command whose audit write is forced to fail produces **zero** + business rows. +4. `Audit_Config_Failure_Rejects_Operation` — with `IAuditConfigService` throwing, a + MUST-class command returns a failure `Result` and writes nothing. This is the + fail-closed guarantee; without it, a config-store outage is indistinguishable from a + quiet day. +5. `AuditStateCapture_ClearedPerRequest` — after a request completes, success or failure, + the scoped `IAuditStateCapture.Changes` collection is empty for the next request. +6. `Every_Module_Has_An_AuditCoverage_Matrix` — a module without a matrix cannot classify + its operations, and under ADR-0033 classification is functional, not documentary. +7. `Every_PII_Module_RegistersUserReferenceLocator` — modules storing user references in + audit payloads must register an `IUserReferenceLocator`. + +`AuditLogBehavior_NeverBlocks_BusinessWrites` from the ADR-0016 era is **replaced**: the +property it asserted is now true only for SHOULD/MAY-class operations, and asserting it +for MUST-class would lock in exactly the defect ADR-0033 removes. + +## 14. Phasing | Phase | Deliverable | |-------|-------------| -| 02 | Infrastructure: `AuditChangeTrackerInterceptor`, `IAuditStateCapture` + impl, `AuditLogBehavior`, `IAuditStore` + `PostgresAuditStore`, `audit_log` table + first month partition, `LearnStackJob` for partition management. | -| 03 | `LearnStack.Modules.Audit`: `AuditEntry` aggregate, repository, admin API endpoints. `UserGdprDeletedIntegrationEventHandler` + per-module `IUserReferenceLocator`. | -| 06+ | Admin Studio audit UI: timeline view, filter by user/module/operation/operation_class/timestamp, diff viewer, CSV / JSON export. | -| 09 | Hub-side `hub_audit_log` + cross-stream correlation query. | -| 11 | Production: long-term partition lifecycle automation, off-archive policy for partitions > 1 year (e.g. move to cheaper storage tier), per-tenant retention enforcement under plan limits. | +| [02a Packet 9](../roadmap/phase-02a-kernel-tenancy.md) | `AuditChangeTrackerInterceptor`, `IAuditStateCapture` + impl, `AuditLogBehavior` lit up per ADR-0033, `IAuditStore` + `PostgresAuditStore`, `AuditEntry` aggregate, `AuditConfig` with the fail-closed MUST floor, `audit_log` as a **single plain table** with the composite primary key. | +| [03](../roadmap/phase-03-identity-admin.md) | Admin API endpoints over the audit stream; `UserGdprDeletedIntegrationEventHandler` + per-module `IUserReferenceLocator`. | +| [06](../roadmap/phase-06-renderer-admin-studio.md) | Admin Studio audit UI: timeline view, filters, diff viewer, CSV / JSON export. | +| [09](../roadmap/phase-09-billing-integrations-analytics.md) | Hub-side `hub_audit_log` + cross-stream correlation query. | +| [11](../roadmap/phase-11-production-hardening.md) | **Scale, not correctness**: `PARTITION BY RANGE (timestamp)` plus monthly partitions, the daily partition-management job, the daily retention purge, off-archive policy for partitions older than a year, per-tenant retention enforcement under plan limits. Trigger: measured `audit_log` growth ([ADR-0035](../decisions/0035-demand-gated-infrastructure.md)). | ## References -- ADR-0016 — Audit Log Subsystem. -- ADR-0003 Amendment 1 — Organization scope (audit row carries organization_id). +- [ADR-0033](../decisions/0033-audit-durability-model.md) — Audit Durability Model + (supersedes ADR-0016); the two durability classes, the fail-closed rules, and the + corrected `audit_log` primary key. +- [ADR-0016](../decisions/0016-audit-log-subsystem.md) — Audit Log Subsystem + (**superseded**; retained for its data model and coverage rationale). +- [ADR-0003 Amendment 1 + Amendment 3](../decisions/0003-tenant-isolation-defense-in-depth.md) + — organization scope on the audit row; the corrected RLS template and four-role model. +- [ADR-0028](../decisions/0028-audit-log-partition-management.md) — partition management + via Hangfire; lands in Phase 11. +- [ADR-0035](../decisions/0035-demand-gated-infrastructure.md) — why partitioning is + demand-gated and correctness is not. +- [Database Standards](../standards/05-database.md) — the canonical RLS template. - [18-audit-coverage.md](../standards/18-audit-coverage.md) — MUST/SHOULD/MAY matrix (standard). +- [15-event-and-outbox.md](15-event-and-outbox.md) — audit fan-out to external sinks + rides the outbox; MUST-class audit does not. - [29-dapr-integration.md](29-dapr-integration.md) — `UserGdprDeletedIntegrationEvent` - arrives via Dapr pub/sub. + transport. - Nexora reference: `Nexora/docs/modules/tier-1-core/audit/SPEC.md`, `Nexora/docs/decisions/0009-audit-repository-pattern.md`, `Nexora/docs/standards/audit-coverage.md`. diff --git a/docs/architecture/32-tenant-customization-model.md b/docs/architecture/32-tenant-customization-model.md index 099e7d4..3ffc67b 100644 --- a/docs/architecture/32-tenant-customization-model.md +++ b/docs/architecture/32-tenant-customization-model.md @@ -89,14 +89,15 @@ These map 1:1 to JSON Schema `type` + `format` combinations: ```typescript const COMPOSITE_RENDERERS = { - 'default-card': DefaultCardRenderer, // image + title + description - 'content-list': ContentListRenderer, // grid / carousel / list of items - 'media-gallery': MediaGalleryRenderer, - 'rich-page': RichPageRenderer, - 'lesson-shell': LessonShellRenderer, // standard lesson play UI - 'quiz-shell': QuizShellRenderer, - 'placement-shell': PlacementShellRenderer, - 'live-shell': LiveShellRenderer, + 'default-card': DefaultCardRenderer, // image + title + description + 'content-list': ContentListRenderer, // grid / carousel / list of items + 'media-gallery': MediaGalleryRenderer, + 'rich-page': RichPageRenderer, + 'lesson-shell': LessonShellRenderer, // standard lesson play UI + 'quiz-shell': QuizShellRenderer, + 'placement-shell': PlacementShellRenderer, + 'live-shell': LiveShellRenderer, + 'submission-shell': SubmissionShellRenderer, // prompt + authoring surface + submit + result // ... small, fixed list } as const; ``` @@ -104,6 +105,12 @@ const COMPOSITE_RENDERERS = { Adding a new primitive or composite renderer is a LearnStack release (CODEOWNERS rule on this folder). Tenants compose existing primitives — they cannot bring custom JSX. +**Every key in both registries is named for a capability, never for a domain.** A +`cefr-level-badge` or `asana-card` key would fail +`Core_Modules_HaveNo_DomainSpecific_Names`. When a tenant needs a domain-flavoured +presentation, it declares a `TenantPageBlock` row pointing at a generic composite — the +domain lives in the row's display name and its schema, not in the registry. + ## 3. Worked example: three tenants, same modules ### Example A — English learning platform @@ -253,14 +260,39 @@ Same modules. Different data. "difficulty": { "type": "string", "x-taxonomy": "coding-difficulty" } } }, - "renderer_key": "code-challenge-shell" + "renderer_key": "submission-shell" } ``` -Note: `code-challenge-shell` is a **composite renderer** that LearnStack ships (since -running test suites server-side is a paid LearnStack feature, gated by -`FeatureKeys.CodeChallengeRunner`). The composite is generic — it works for any language -declared in the schema; the tenant doesn't bring custom JSX. +**Two naming rules meet in this example, and they point in opposite directions.** + +The content type's `key` is `code-challenge`. That is fine: it is a **row in the tenant's +database**, authored by the tenant, and it may name its domain as specifically as the +tenant likes. `vocabulary-card`, `asana-pose`, `code-challenge` and `breath-technique` +are all legitimate tenant data. + +The renderer key and the feature key are **code**, and code may not name a domain. Both +were previously named for the first tenant that asked for them: + +| Was | Is | Why | +|---|---|---| +| `code-challenge-shell` (composite renderer) | **`submission-shell`** | The capability is "prompt + authoring surface + submit + evaluated result panel". It serves a code exercise, a pronunciation recording, an essay, and a portfolio upload equally | +| `FeatureKeys.CodeChallengeRunner` | **`FeatureKeys.SandboxedEvaluation`** (`assessment.sandboxed_evaluation`) | The capability is "evaluate a learner's submitted artefact in a sandbox with a resource budget". Nothing about it is specific to code | + +The rule is not stylistic. `Core_Modules_HaveNo_DomainSpecific_Names` enforces it +mechanically from +[Phase 02a Packet 10](../roadmap/phase-02a-kernel-tenancy.md) — a module type, namespace +or registry key matching `Cefr`, `Asana`, `CodeChallenge`, `English*`, `Yoga*` fails the +build. The old names would have failed it. + +`submission-shell` is also where this document meets the **genericity boundary**. The +renderer is presentation and therefore inside the boundary; the *evaluation* it triggers +is external capability invocation and therefore outside it. Running a learner's submitted +program needs a sandbox, a runtime, a resource budget and a security boundary that +survives hostile input — none of which a JSON Schema or a rule DSL can declare. It is a +**platform feature gated by plan**, written by LearnStack, not a customization row. See +[ADR-0018 Amendment (2026-08-08)](../decisions/0018-tenant-driven-customization-model.md) +and [Platform Vision § Genericity boundary](01-platform-vision.md). ## 4. Schema versioning @@ -360,7 +392,158 @@ expression: | Same sandbox engine as scoring rules (decision pending). -## 8. What cannot be customized +## 8. Runtime cost model + +Everything above describes what a tenant *can* declare. This section describes what it +*costs* to serve, because a customization model without a cost model is an invitation to +declare something the runtime cannot render inside a request budget. Every limit here is +enforced at write time, so a tenant learns about it while authoring rather than a learner +discovering it on a page load. + +### 8.1 Validation timing — write time, not read time + +| Validation | When | Failure mode | +|---|---|---| +| The `json_schema` is itself a valid JSON Schema (draft 2020-12), and every `x-renderer` / `x-taxonomy` / `x-language` extension resolves to a registry entry | **On saving the content type / block / lesson item type** | 400 with Problem Details naming the offending JSON pointer | +| A content entry conforms to its content type's schema at its pinned `schema_version` | **On saving the entry** | 400; the entry is not persisted | +| The declared limits in § 8.3 | **On saving the schema** | 400 | +| Renderer resolution: does `renderer_key` exist in `COMPOSITE_RENDERERS`? | **On saving**, again on **server render** as a cheap dictionary lookup | Save is rejected; a stale reference renders a fallback block plus a logged warning, never an exception | + +**Nothing is schema-validated on the read path.** A published content entry has already +been validated against its pinned schema version, and re-validating it on every page load +would put a JSON Schema evaluation in the hot path of anonymous traffic for a result that +cannot have changed. The consequence is stated plainly: **the schema is a write-time +contract, and the read path trusts the database.** Anything that can write a content-entry +row without going through the validating command path — a manual `INSERT`, a bad +migration, a future bulk importer — breaks that trust for every subsequent read. Bulk +import therefore goes through the same validator, at +[Phase 04](../roadmap/phase-04-cms-media-pages.md). + +The one read-time check is **structural, not semantic**: the renderer walks the entry's +JSON against the schema's field list and skips unknown fields. That is an O(fields) pass, +not a validation. + +### 8.2 Cache strategy + +Customization definitions are read on nearly every request and written a handful of times +per tenant per month. That ratio is the whole design. + +| What | Layer | Key | TTL | Invalidated by | +|---|---|---|---|---| +| `TenantContentType` set for a tenant | L1 + L2 | `cust:{tenant_id}:content-types:{generation}` | L1 60s, L2 15 min | Generation bump | +| `TenantLevelTaxonomy` by key | L1 + L2 | `cust:{tenant_id}:taxonomy:{key}:{generation}` | same | Generation bump | +| `TenantPageBlock` set | L1 + L2 | `cust:{tenant_id}:blocks:{generation}` | same | Generation bump | +| Compiled JSON Schema validator | L1 only, per pod | `(tenant_id, content_type_key, schema_version)` | Process lifetime, bounded LRU | Immutable — a schema version never changes | + +Two rules make this safe: + +- **A generation counter, not prefix eviction.** Each tenant carries a + `customization_generation` integer, bumped in the same transaction as any customization + write. Cache keys embed it, so a write makes every stale key unreachable at once, + across every pod, without enumerating keys. This is deliberate: the published + `ICacheService.RemoveByPrefixAsync` contract cannot be honoured across instances by any + candidate backend, and it is removed or redesigned to exactly this pattern before + [Phase 02a Packet 5](../roadmap/phase-02a-kernel-tenancy.md) ships. +- **Compiled validators are cached separately from definitions**, keyed by an immutable + `(key, schema_version)` tuple. Compiling a JSON Schema is the expensive part; because a + published schema version is immutable ([§ 4](#4-schema-versioning)), the compiled form + never needs invalidating — it only needs bounding, hence the LRU. + +Cache misses cost one indexed query per tenant per definition set. A cold pod serving its +first request for a tenant performs at most four such queries, not one per entry. + +### 8.3 The N+1 problem, and the limits that bound it + +The guided-sequence example in [§ 3](#3-worked-example-three-tenants-same-modules) is a +textbook N+1 and it is worth being explicit about, because it is the shape tenants will +naturally author: + +```json +"poses": { "type": "array", "items": { "$ref": "#/$defs/pose-ref" } } +``` + +Each `pose-ref` carries `{ content_type: "asana-pose", content_id: "" }`. A naive +renderer resolves each reference with its own query, so a 40-pose sequence issues 40 +round trips — and a lesson page containing three such sequences issues 120. At 2 ms per +round trip that is a quarter-second of pure latency for a page that reads four rows' +worth of actual information. + +**The resolution contract:** + +1. The renderer **collects every reference in the whole payload first**, walking the + entry's JSON once and gathering `(content_type, content_id)` pairs into a set. +2. It issues **one batched query per content type** — `WHERE tenant_id = @t AND + content_type = @ct AND id = ANY(@ids)` — not one per reference. Duplicated references + to the same entry cost nothing extra. +3. It hydrates the payload from the resulting dictionary. +4. References that do not resolve render a placeholder and emit a warning. A dangling + reference is a data problem, never an exception on a public page. + +Total cost for any entry: **one query per distinct referenced content type**, regardless +of reference count. Reference resolution is capped at **depth 2** — an entry may +reference entries, and those may not reference further. Deeper composition is a +[Phase 05](../roadmap/phase-05-education-learning-content.md) decision with its own +batching design, not something a tenant can reach by nesting `$ref`s. + +The `Customization_Reference_Resolution_Is_Batched` integration test asserts the query +count for a 40-reference entry is a small constant, not 40. Without a test that counts +queries, this contract silently regresses the first time someone writes a convenient +`foreach`. + +### 8.4 Declared limits + +Every limit is checked at write time and returns Problem Details naming the limit that +was exceeded. They are deliberately generous — they exist to stop pathological documents, +not to constrain reasonable authoring. + +| Limit | Value | Why this one | +|---|---|---| +| `$ref` / `$defs` nesting depth inside one schema | 5 | JSON Schema validators are recursive; unbounded depth is a stack-exhaustion vector on a tenant-authored document | +| Reference resolution depth (entry → entry) | 2 | Bounds the batched-query fan-out at § 8.3 to a fixed small number of round trips | +| Properties in one content type | 100 | Beyond this the authoring UI is unusable and the row is a schema smell | +| Array `maxItems` where the schema omits it | 200 (applied as a default, not a rejection) | An unbounded array in a JSONB column is an unbounded render loop | +| References in one content entry | 500 | Caps the `ANY(@ids)` parameter list and the hydration dictionary | +| Block instances on one page | 100 | Each instance is a render subtree; the page is a document, not an application | +| Serialised size of one content entry | 1 MB | PostgreSQL will happily TOAST more; the browser will not happily render it | +| Content types per tenant | Plan-gated via `FeatureKeys` / `LimitKeys` | Unlimited content types is a Growth+ feature ([ADR-0021](../decisions/0021-feature-based-entitlement.md)) | + +A schema that violates a structural limit is rejected on save. An **existing** entry that +would violate a newly tightened limit still renders — limits are enforced forward, and a +tightening ships with the migration that reports which existing rows exceed it. + +### 8.5 The `embed-html` sanitisation contract + +`embed-html` is the one primitive that takes an **HTML sink authored by a lower-trust +actor** and puts it in the tenant's page. Everything else in the primitive set renders +structured data through React text nodes and is safe by construction. This one is not +safe by construction; it is safe by contract, and the contract has to be written down — +calling the primitive set "closed and safe" without stating it is precisely the gap that +lets a reviewer approve an unsanitised path. + +**The threat model.** A tenant's content editor is not the tenant's security team. In a +multi-branch business, an organization-level editor at one studio can author an +`embed-html` block that renders on a page a learner from another organization loads. The +actor is semi-trusted, the audience is not the actor, and the two may sit in different +organizations of the same tenant. + +**The contract:** + +| Rule | Detail | +|---|---| +| Sanitise **on write and on render** | Write-time sanitisation gives the author immediate feedback and keeps the stored value clean. Render-time sanitisation is the one that actually protects the learner, because it is the only one that also covers rows written before the current allow-list | +| Allow-list, never block-list | Elements: structural and text markup plus `iframe` (see below). Attributes: `class`, `id`, `href`, `src`, `alt`, `title`, `width`, `height`, `colspan`, `rowspan`. Everything else is stripped | +| No script execution surface | `