stack/docs/reports/native-kanban-sot/kbn-101-contract-security-review-82ce325.md

KBN-101 contract independent security/architecture review

Verdict: REQUEST CHANGES

Review identity and scope

• Exact reviewed head: da742ca2da4a2ff466916c818fe275c4f7ffd384 (docs(#771): record role-split review evidence)
• Required comparison: origin/main...da742ca2da4a2ff466916c818fe275c4f7ffd384
• Range: 82ce3252df38a687c50485f8d048b53ca8db5989 is an ancestor of the reviewed head; the final head adds the scratchpad evidence commit and was reviewed.
• Changed docs: docs/PRD.md, docs/SITEMAP.md, docs/native-kanban-sot/{INDEX.md,KBN-101-DB-ROLE-SPLIT.md,SHARED-CONTRACT.md,TASKS.md}, and docs/scratchpads/771-kbn101-db-role-split.md (300 additions / 25 deletions).
• Reviewed inputs: issue #771; current DB/Gateway/storage/config/wizard/installer/compose/Portainer/CI sources; all current migration/DDL references; KBN-010, rc.4/rc.5 shared contract, requirements/canon, KBN-100 #769 branch context, and the final scratchpad.
• Repository/provider state: not modified. The pre-existing .mosaic/orchestrator/* dirt was not touched.

The role graph itself is sound in principle: a NOLOGIN platform database owner, separate NOLOGIN schema owner, NOINHERIT migrator which explicitly SET ROLEs, and runtime membership only in a capability role with SET FALSE does not create circular privilege or application-created login roles. The split of foundation certification before KBN-100 and immutable-operation certification after KBN-100 is also correctly ordered.

Findings

HIGH — DDL/migration control plane is not closed at every current entrypoint

The contract requires an explicit, locked migration phase and forbids Gateway/runtime DDL (KBN-101-DB-ROLE-SPLIT.md:34-39), but its KBN-101-02 result merely says migration-capable commands use the migration DTO (:109). It does not prohibit or route every existing bypass through that one command.

Current bypasses include:

• runMigrations() falls back from an argument to DATABASE_URL and a hard-coded URL (packages/db/src/migrate.ts:24-35), while drizzle.config.ts likewise uses DATABASE_URL plus a default (packages/db/drizzle.config.ts:3-9).
• Package scripts expose direct drizzle-kit migrate and drizzle-kit push (packages/db/package.json:23-26); db:push bypasses the planned journal/fingerprint/lock entirely.
• mosaic storage migrate --run shells out to the direct db:migrate script (packages/storage/src/cli.ts:413-452).
• The federated integration test can create types, tables, and indexes directly against DATABASE_URL and intentionally operates without a Drizzle ledger (packages/db/src/federation.integration.test.ts:28-30,46-134).

Failure mode: a runtime or CI environment with only DATABASE_URL, or an operator invoking an existing command, can apply unverified DDL outside the lock, SET ROLE preflight, exact-ledger gate, and deployment sequencing. This breaks the requested fail-closed split even if Gateway startup is repaired.

Required remediation: amend KBN-101-02/03/06 to enumerate these entrypoints and make the dedicated migrator runner the only PostgreSQL DDL path. Production-like db:push must be removed/blocked; db:migrate, storage migrate --run, and migration tests must invoke the same migration runner with DATABASE_MIGRATION_URL, lock, identity preflight, and ledger verification. Tests needing schema must consume a pre-migrated disposable database, or be explicitly run only by that migration phase. Add negative tests showing each command refuses DATABASE_URL-only execution and cannot reach DDL.

HIGH — TLS requirement has no deployable server/bootstrap contract

The contract correctly requires a mounted CA and hostname-verified TLS (KBN-101-DB-ROLE-SPLIT.md:25,28,93-95). However KBN-101-05 promises only a “migration phase and secret binding boundary” (:112), not PostgreSQL server TLS, certificate issuance/SANs, CA distribution, startup ordering, or the fresh/existing-database bootstrap trust path.

Current standalone and federated compose expose plain PostgreSQL with no server TLS configuration or CA mount (docker-compose.yml:2-14; docker-compose.federated.yml:27-44). The Portainer test stack passes a single plaintext in-network URL and uses the same database login for Gateway and database bootstrap (deploy/portainer/federated-test.stack.yml:51-60,110-117).

Failure mode: enforcing the mandatory CA makes current local standalone/federated topologies unable to start; relaxing it to make bootstrap work silently violates K101-REQ-03. A first database cannot be safely migrated until the server certificate, its SAN for the actual service/DNS name, and trusted CA are provisioned, but this lifecycle is not owned or tested.

Required remediation: add a concrete KBN-101-00/05 TLS bootstrap sub-contract: issuer/CA owner; server key/cert and SAN inputs; secure storage/mount permissions; postgresql.conf/container TLS enablement; migration and runtime CA mounts; hostname used by each compose/Swarm service; readiness only after TLS authentication; CA overlap rotation; and an existing-database transition. Require a disposable standalone and federated/Swarm test to prove verified TLS succeeds and missing CA, wrong CA, wrong SAN, and sslmode downgrade fail before readiness. Do not merge KBN-101-05 with an implicit plaintext exception.

HIGH — exact ledger fingerprint and historical 0009 repair are underspecified for existing databases

The contract requires an “ordered complete set” and rejection of out-of-order rows (KBN-101-DB-ROLE-SPLIT.md:36-38), but does not define the canonical serialized tuple, ledger ordering source, or safe upgrade rule for a historical ledger. The current ledger stores only id, hash, and created_at (packages/db/src/migrate.ts:70-82,105-107). Its journal is demonstrably non-monotonic: 0008 has when=1776822435828, followed by 0009 at 1745280000000 (packages/db/drizzle/meta/_journal.json:62-79); the existing PostgreSQL runner documents that this causes skipping (packages/db/src/migrate.ts:29-35).

Failure mode: an implementation can either reject a legitimate historical database after correcting 0009, or accept a reordered/duplicated ledger because no precise comparison rule exists. A count/hash-set implementation would fail to detect the condition that this contract explicitly calls unsafe; physical id order is not an adequate substitute after historical repair.

Required remediation: freeze a versioned manifest algorithm before implementation: canonical record fields (at least journal index/tag, corrected logical order, migration content hash, and an explicit migration-manifest version), canonical byte serialization, SHA-256 input, and exact observed-ledger mapping. State whether physical ledger insertion order is normative; if not, compare hash-to-manifest tuples rather than timestamps. Add an idempotent migrator-only 0009 existing-database remediation/reconciliation procedure with backup/rollback evidence. Require clean, pre-0009, 0009-skipped, 0009-applied-late, duplicate, unknown, missing, corrupt-pair, and stale-replica cases. No manual ledger insertion is an acceptable production recovery path.

MEDIUM — advisory-lock namespace is collision-prone and lacks a fixed identifier contract

The specified lock is pg_try_advisory_lock(hashtext('mosaic-schema-migration-v1')) (KBN-101-DB-ROLE-SPLIT.md:34). hashtext produces a 32-bit key. Session ownership/crash behavior is otherwise correctly stated (one session, same-session release, connection-close release), but an unrelated database user can accidentally collide or deliberately hold the key and force DATABASE_MIGRATION_LOCKED.

Failure mode: avoidable migration denial of service in a shared PostgreSQL database. The current repository already uses separate hashtext advisory-lock names for migrate-tier, demonstrating the need for a documented namespace rather than a collision-prone implicit one.

Required remediation: freeze a two-int advisory-lock namespace (fixed documented class/object values) or a documented 64-bit hashtextextended key with fixed seed; keep acquisition, migration, verification, and release on the single max:1 migrator session. Add tests for concurrent migration, connection loss/crash release, readiness while the lock holder is active, and an unrelated lock-key non-interference case.

MEDIUM — identifier safety and search_path verification need executable constraints

The contract rightly requires pg_catalog, <mosaic_application_schema> and rejects writable paths (KBN-101-DB-ROLE-SPLIT.md:54,70-77), but uses dynamic placeholders for database/schema and does not state how migration/bootstrap SQL will avoid identifier interpolation. Existing code has raw-SQL facilities (packages/storage/src/migrate-tier.ts uses .unsafe), so this is not merely theoretical.

Failure mode: a future operator-configured database/schema value that reaches bootstrap or SET search_path through raw string construction can inject DDL, or a pooled connection can retain a mutable search path.

Required remediation: require fixed allowlisted identifiers or server-side identifier quoting (format('%I', ...)) only; never interpolate URL/config values into SQL. Set and verify the trusted path per connection/session before any query (SET LOCAL inside transactions where applicable), forbid public/$user additions, and add injection-shaped identifier and pooled-connection reset negatives. Include this in KBN-101-00/01 tests.

Acceptance and threat traceability

Requirement / threat	Review result	Evidence or blocking finding
K101-REQ-01 / AC-K101-01 split runtime/migration URLs	Partial	Role/DTO boundary is coherent; HIGH DDL-path finding requires all current commands to be closed.
K101-REQ-02 / AC-K101-02 explicit migration/readiness	Blocked	HIGH ledger definition and HIGH DDL-bypass findings.
K101-REQ-03 / AC-K101-03 least privilege, TLS, grants	Partial	Role model, default privileges, ledger read-only, TEMP/function checks are well specified (KBN-101...:47-56,70-79); HIGH TLS bootstrap and MEDIUM identifier constraints remain.
K101-REQ-04 / AC-K101-04 immutable relations	Correctly deferred	KBN-101-09 after KBN-100 is the correct serial gate (KBN-101...:58-66,115-118); no synthetic-only certification claim found.
K101-REQ-05 / AC-K101-05 N-1, secrets, rollback	Partial	No owner-runtime exception and rollback keeps migration URL out of Gateway (:83-95); deployable TLS and full command inventory are missing.
K101-REQ-06 / AC-K101-07 KBN gates and DAG	Structurally sound	DAG is acyclic: 00→01/{03}; 02→06; 00/01/03→05; 00/04/05/06→07→08→KBN-100→09→KBN-105. KBN-100’s current branch contains docs-only baseline tracking, not schema implementation.
T: runtime DDL / migration fallback	Blocked	HIGH finding 1. Current Gateway/storage, CLI, direct Drizzle scripts, and integration DDL require explicit closure.
T: race/crash/readiness	Partial	Same-session nonblocking lock and replica-unready rules are present (:34-38); lock namespace remediation required.
T: immutable evidence rewrite	Correctly staged	Explicit INSERT/SELECT-only matrix and RESTRICT retention are retained; proof is properly after table creation.
T: secret leakage / TLS downgrade	Partial	Redaction and distinct Vault paths are specified (:93-97), but no server TLS/bootstrap implementation contract exists.

Unresolved assumptions

1. standalone and federated are the complete PostgreSQL production-like set (K101-A1).
2. Each eligible deployment can execute a dedicated migration Job/one-shot phase (K101-A2).
3. Vault path names are targets, not verified existing paths; deployment ownership remains to be established.
4. PostgreSQL 17 is available for the selected membership and advisory-lock implementation.
5. The required server-side TLS issuer/certificate lifecycle and Swarm/compose secret transport have not been decided; this is blocking, not a permissible implicit plaintext bootstrap.
6. Historical databases containing the 0009 journal/ledger anomaly have no frozen reconciliation procedure.

Independent test and consistency evidence

Read-only checks run in this review:

Check	Result
git diff --check origin/main...da742ca2...	PASS
pnpm exec prettier --check on all seven changed docs	PASS
pnpm exec tsc --noEmit -p docs/native-kanban-sot/tsconfig.json	PASS
docker compose -f docker-compose.yml config --quiet (isolated test ports)	PASS
docker compose -f docker-compose.federated.yml --profile federated config --quiet (isolated test ports)	PASS
Static journal inspection	FAILS the required monotonic ordering premise: 0008 → 0009 when decreases; current runner documents skipping behavior.
Static DDL-entrypoint inventory	Found direct Drizzle scripts, storage CLI shell-out, runtime extension/migration calls, fleet backlog migration, tier probe extension creation, and a direct-DLL federated integration test.

No live database, Vault, CI, deployment, issue, PR, or repository mutation was performed. The pass results validate documentation syntax/contract compilation and compose syntax only; they do not certify the proposed security behavior.

Conclusion

Do not merge this frozen contract as implementation-ready until the HIGH findings are corrected and independently re-reviewed. The central role ownership/default-privilege design, immutable-table staging, and KBN-100/KBN-105 serial gating should be retained; they are not the reason for this REQUEST CHANGES verdict.