Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace LearnStack.Modules.Tenancy.Infrastructure.Persistence.Migrations
/// <para>
/// Intra-tenant rather than cross-tenant — the tenant term is untouched, and no row
/// crosses a tenant boundary — so this is a write-scope correction, not an isolation
/// fix. See ADR-0003 Amendment 4 and Database Standards § Tenant-Owned and
/// fix. See ADR-0003 Amendment 5 and Database Standards § Tenant-Owned and
/// Organization-Scoped Tables, which carries the corrected template.
/// </para>
/// <para>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public void Every_Write_Port_Is_Countable_Or_Enumerated()
.Where(type => type.IsInterface)
.Where(type => type.GetMethods().Any(method =>
method.GetParameters().Any(parameter =>
domainAssemblies.Contains(parameter.ParameterType.Assembly))))
Unwrap(parameter.ParameterType).Any(inner =>
domainAssemblies.Contains(inner.Assembly)))))
.Where(type => !WriteStoreConstructions(type).Any())
.Select(type => type.Name)
.Distinct()
Expand Down Expand Up @@ -163,4 +164,39 @@ private static IEnumerable<string> ProductionAssemblies() =>
.Select(Path.GetFileNameWithoutExtension)
.Where(name => !string.IsNullOrEmpty(name))
.Select(name => name!);

/// <summary>A declared type and every type reachable through it.</summary>
/// <remarks>
/// Through wrappers, because a bulk write port takes one: <c>IEnumerable&lt;Course&gt;</c>
/// lives in <c>System.Private.CoreLib</c>, so a check on the parameter's own assembly
/// sees the wrapper and not the domain type inside it — and the port escapes the
/// enumeration while satisfying every word of what this rule claims to detect.
/// Transitive, and through arrays and by-ref, because those nest too:
/// <c>Course[]</c> and <c>in Course</c> are the same port with different syntax.
/// </remarks>
private static IEnumerable<Type> Unwrap(Type declared)
{
yield return declared;

if (declared.HasElementType && declared.GetElementType() is { } element)
{
foreach (var inner in Unwrap(element))
{
yield return inner;
}
}

if (!declared.IsGenericType || declared.IsGenericTypeDefinition)
{
yield break;
}

foreach (var argument in declared.GetGenericArguments())
{
foreach (var inner in Unwrap(argument))
{
yield return inner;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ public async Task An_Organization_Scoped_Session_Cannot_Write_A_Tenant_Wide_Row(
//
// Both AS RESTRICTIVE guards used a bare `organization_id IS NULL` first arm,
// which exists so a TENANT-scope session can write those rows — and admitted an
// org-scoped one to them as well. Measured before ADR-0003 Amendment 4: a session
// org-scoped one to them as well. Measured before ADR-0003 Amendment 5: a session
// announcing tenant A and organization A1 rewrote tenant A's tenant-wide row.
//
// The refusal is silent by construction: a RESTRICTIVE USING clause on UPDATE
Expand Down
6 changes: 4 additions & 2 deletions docs/decisions/0003-tenant-isolation-defense-in-depth.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

Accepted (Amendment 1: 2026-05-18 — adds Organization scope; Amendment 2: 2026-05-19 —
identity row terminology; **Amendment 3: 2026-08-08 — corrects the RLS policy template
and adds the database role model**; see bottom of document)
and adds the database role model**; Amendment 4: 2026-08-29 — the Phase 02a table list
has gone stale; **Amendment 5: 2026-09-04 — the write guards exclude an
organization-scoped session from tenant-wide rows**; see bottom of document)

## Decision

Expand Down Expand Up @@ -301,7 +303,7 @@ The single authority for which class a table belongs to is
into three documents drifts in three directions, and this copy already had — which
is why the assignment lives in one file and this section keeps only the *rule*.

### Amendment 4 (2026-09-04): the write guards admitted an organization-scoped session to tenant-wide rows
## Amendment 5 — The write guards admitted an organization-scoped session to tenant-wide rows (2026-09-04)

**What was wrong.** Amendment 3's template closes the `USING`-only write paths with two
`AS RESTRICTIVE` guards, one `FOR UPDATE` and one `FOR DELETE`. Both read:
Expand Down
66 changes: 63 additions & 3 deletions docs/roadmap/phase-02a-kernel-tenancy.md
Original file line number Diff line number Diff line change
Expand Up @@ -2437,9 +2437,11 @@ one — and the second round repeatedly found the first round's fix.

> **Packet 7 — Tenant and organization resolution, isolation, two tenants ✅**
>
> **Measured at close: 1187 tests green** — 1 contract, 76 architecture, 802 unit,
> 308 integration. Counted from a run, not computed from a plan; an earlier commit
> message in this packet carried an arithmetic total that was eight short.
> **Measured at merge: 1208 tests green** — 1 contract, 79 architecture, 813 unit,
> 315 integration. Counted from a run, not computed from a plan; an earlier commit
> message in this packet carried an arithmetic total that was eight short. The number
> moved after this record was first written, because the pull request's own review found
> more — see § What the pull-request review found.

### What shipped

Expand Down Expand Up @@ -2506,6 +2508,50 @@ one — and the second round repeatedly found the first round's fix.
took a raw `Guid` tenant after the ADR said it and `ITenantContext.TenantId` "both
move together". Amendment 4 records the conversion and the divergence.

### What the pull-request review found

The record above was written at packet close. Fifteen commits landed after it, from three
review rounds on the pull request itself, and the sharpest defect of the packet was among
them — so the record would be a false account without them.

- **A shipped invariant that did not hold.** Promoting a default locale through the
aggregate raised `23505` every time. `PromoteDefault` clears the incumbent and then sets
the target in memory, and EF does not preserve that order: same-table commands go out in
its comparer's order, and the composite key `(tenant_id, locale)` sorts `en-US` before
`tr-TR` — exactly the seeded pair. Nothing caught it because the cases covering that
index drive raw SQL in an order they choose, so they pin what PostgreSQL does with two
statements rather than what EF emits for one save. **This packet's own PR description
cited that invariant as the migration's safety rationale.** The store now saves in two
passes; a partial unique index permits zero defaults, so the intermediate state is one
the schema allows.
- **A canonical template that admitted the wrong writer.** Both `AS RESTRICTIVE` write
guards began with a bare `organization_id IS NULL` — the arm that lets a *tenant*-scope
session write rows belonging to no organization. It admitted an *organization*-scoped
session to them as well, so one organization could rewrite the tenant-wide fallback
every other organization reads. Measured. Corrected in the template, because every
organization-scoped table is told to copy it, and recorded as
[ADR-0003 Amendment 5](../decisions/0003-tenant-isolation-defense-in-depth.md).
- **Three credential and transaction gaps.** The host resolver announced a session
variable in a transaction four documents call read-only and that was not; the
platform-admin guard accepted `rolsuper`, which bypasses the GRANT matrix that bounds
the role; and an unparseable connection string was echoed through a redaction pattern
that could not cross a `/` or a second `@` inside a password.
- **Two architecture rules with escapes, and one blind test.** The effective-host rule
banned a header the code never reads instead of the one it does; the resolving-host rule
exempted its sole setter by filename suffix; and the host-reclaim case passed with the
hostname uniqueness dropped entirely, because nothing in it asked for a conflict.
- **Three ADR edits made outside the rules that govern them** — a stale statement rewritten
rather than amended, a normative paragraph added to an Accepted body, and an amendment
placed above the decision it amends.
- **Two findings refuted by measurement**, which is why they are worth recording: MediatR
already deduplicates pipeline behaviours, so a guard written for it was removed rather
than shipped; and the query filter's pinned `DbContext` is harmless because its
`CurrentTenantId` delegates to the process-wide accessor.

**A hook now catches what CI caught three times.** Three commits on the branch tripped the
72-character subject limit, each found after a push — and a subject is only fixable by
rewriting history. `.githooks/commit-msg` enforces exactly what CI's `meta` job enforces.

### What it did not ship, and who owns each

- **Nothing is audited.** `AuditLogBehavior` lights up in Packet 9; `TransactionBehavior`
Expand All @@ -2518,3 +2564,17 @@ one — and the second round repeatedly found the first round's fix.
- **The host-resolution cache is invalidated before the commit, not after.** The
guarantee is therefore the request *after* the write, not the one racing it; closing
the rest needs a post-commit seam on `IUnitOfWork`, whose surface ADR-0040 governs.
- **Two idempotency limits do not bound memory,** and are recorded at the line rather than
fixed. The store's admission check reads a census refreshed once per sweep interval, so
within that window its caps admit every new key; and the filter buffers a response in
full before applying the 256 KiB cap. Both are Packet 4's, both fixes revisit an
ordering [ADR-0037](../decisions/0037-idempotency-key-contract.md) chose deliberately,
and neither is reachable through an endpoint that exists — every idempotent surface
today answers with an identifier, a receipt or a status. They belong with the durable
store, on that ADR's trigger.
- **Two migrations shipped in one pull request,** which
[Git Standards § Branching](../standards/14-git-workflow.md) says should not happen.
Splitting was attempted and measured infeasible: the migration commit carries the model
change it was generated from, and cherry-picking it onto `main` conflicts through the
marker and typed-identifier commits before it — a "migration PR" would have carried
roughly the first thirty commits of the packet.
3 changes: 2 additions & 1 deletion docs/standards/05-database.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
(Amendments 1 + 2),
[ADR-0003 Tenant Isolation Defense in Depth](../decisions/0003-tenant-isolation-defense-in-depth.md)
(Amendment 1: Organization Scope; **Amendment 3: corrected RLS policy template and
database role model**),
database role model**; **Amendment 5: the write guards exclude an organization-scoped
session from tenant-wide rows**),
[ADR-0006 Events and Outbox](../decisions/0006-events-and-outbox.md)
(Amendment 1: Dapr pub/sub dispatch transport),
[ADR-0038 Cross-Cutting Port and Event Contracts](../decisions/0038-cross-cutting-port-and-event-contracts.md),
Expand Down
75 changes: 75 additions & 0 deletions docs/standards/21-architecture-tests-catalogue.md
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,81 @@ because the filters hold, and removing both turns all five red.
- **Status:** **Implemented** (Packet 7, `LearnStack.Tests.Architecture`).
- **Phase:** 02a Packet 7.

#### `Every_Write_Port_Is_Countable_Or_Enumerated`

- **Asserts:** every interface in a production assembly whose method takes a type from a
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
module's `Domain` assembly — **directly, or inside a generic, array or by-ref wrapper**
— either derives from `IAggregateWriteStore<TRoot, TId>`, and is therefore visible to
the cross-aggregate census above, or appears on a literal allow-list. The list holds one
name: `IPlatformHostMappingStore`.
- **Wrappers are unwrapped transitively,** because a bulk write port is written with one:
`IEnumerable<Course>` lives in `System.Private.CoreLib`, so a check on the parameter's
own assembly sees the wrapper rather than the domain type inside it — and the port would
escape the enumeration while satisfying every word of what this rule claims.
- **Why it exists.** The census counts derivations, so a port that does not derive is
invisible to it. One already is, deliberately: `PlatformHostMapping` is a projection
with a string key rather than an aggregate root. That exemption is fine; being *silent*
about it is not, because a second such port would join the first with nothing to notice,
and the census that keeps ADR-0042's exception at one entry would stop describing the
system.
- **Detected by shape, not by name.** "Takes a domain type" rather than "ends in `Store`":
a rule keyed on a suffix is satisfied by renaming.
- **Source:** [ADR-0042](../decisions/0042-tenant-provisioning-cross-aggregate-transaction.md).
- **Type:** xUnit + reflection. **Kind:** structural.
- **Status:** **Implemented** (Packet 7 review, `LearnStack.Tests.Architecture`,
`AggregateWriteTests`).
- **Phase:** 02a Packet 7.

#### `Out_Of_Band_Setters_Open_Read_Only_Transactions`

- **Asserts:** the two components that announce a session variable outside the ambient
unit of work — `CachedHostToTenantResolver` and `OrganizationScopeValidator` — each
contain `SET TRANSACTION READ ONLY`, and contain it **at a lower source offset than
their first `set_config(`**.
- **What the offset comparison does and does not prove.** PostgreSQL refuses
`SET TRANSACTION` after the transaction's *first statement of any kind*, and this scan
only orders it against the announcement. A setter that ran some other statement — a
`SELECT`, a second `SET` — between `BEGIN` and `SET TRANSACTION READ ONLY` would satisfy
the rule and fail at runtime. That failure is loud and immediate rather than silent,
which is why the cheap ordering check is the one that ships; the expensive alternative
is parsing the method for every command execution, and
[§ What a structural test proves](#what-a-structural-test-proves--and-what-it-does-not)
states the general limit.
- **Why the property matters at all.** Read-only is what makes an out-of-band setter of a
session variable acceptable, because `learnstack_app` holds write grants on the tables
these connections reach — so nothing but this statement stops a future edit from writing
under an announcement no request made.
- **Why a scan and not a behavioural test.** The transaction is opened, used and disposed
inside one method, so nothing outside can observe its settings. Measured: the resolver
shipped without the statement while four carriers — Database Standards, Security
Standards, the glossary and ADR-0040 — described it as read-only, and the validator two
files away had carried it since Packet 6.
- **Source:** [ADR-0040](../decisions/0040-ambient-unit-of-work.md);
[05-database.md](05-database.md); [11-security.md](11-security.md).
- **Type:** xUnit + source scan. **Kind:** structural.
- **Status:** **Implemented** (Packet 7 review, `LearnStack.Tests.Architecture`,
`TenancyConventionTests`).
- **Phase:** 02a Packet 7.

#### `Registering_The_Pipeline_Twice_Registers_It_Once`

- **Asserts:** calling `AddLearnStackMediatRPipeline` twice on one `ServiceCollection`
yields the same registrations as calling it once — the same behaviour count and the same
total.
- **The property is MediatR's, not ours.** `AddBehavior` deduplicates; measured at seven
behaviours and eleven registrations either way. It is pinned because the repository
depends on it and did not write it: every test fixture registers its probe handler by
hand specifically to avoid re-running `AddMediatR`, and if deduplication stopped
holding, that workaround would become load-bearing rather than cautious with nothing to
say so. A doubled `TransactionBehavior` is a nested frame on every request.
- **A guard of our own was written and removed.** It changed nothing under mutation, and a
guard no test can kill is a comment.
- **Source:** [ADR-0032 § Sub-decision 2](../decisions/0032-exception-handling-logging-and-observability.md).
- **Type:** xUnit + DI registration inspection. **Kind:** structural.
- **Status:** **Implemented** (Packet 7 review, `LearnStack.Tests.Architecture`,
`CrossCuttingFoundationTests`).
- **Phase:** 02a Packet 7.

#### `Tenant_Context_Guard_Fires_Only_On_An_Unmarked_Transaction`

- **Asserts:** both arms of the `DbCommandInterceptor` guard. A command a module `DbContext` issues on a transaction no sanctioned setter announced throws `TenantContextMissingException`; the same command on an announced transaction runs. One arm is not the rule: a guard keyed on `TransactionBehavior` instead of on the marker passes the first arm and rejects the writes the idempotency store and the audit store legitimately make on their own short transactions.
Expand Down
Loading