fix(11-08): make transaction gates fail closed

.planning/phases/11-jobs-realtime-and-search-infrastructure/11-SECURITY-REVIEW.md

@@ -1,7 +1,7 @@
 ---
 phase: "11"
 reviewed: "2026-09-30"
-reviewer: "gsd-executor, plan 11-07 (code-and-test review of plans 11-01 to 11-07); T-11-31, T-11-32 and RC-14 added in the 11-08 close-out"
+reviewer: "gsd-executor, plan 11-07 (code-and-test review of plans 11-01 to 11-07); T-11-31, T-11-32, RC-14 and RC-15 added in the 11-08 close-out/review"
 threats_open: 0
 gate: "scripts/check-phase11.sh --all"
 removal_harness: "scripts/check-phase11.sh --removal"
@@ -53,13 +53,13 @@ Commands run from `summercms.go`. `../fonoteka.go` tests run inside that reposit
 | T-11-28 | Information Disclosure | goldens and route fixtures | high | mitigate | `tide/centrifugo.go` records only whether `Authorization` matched; fonoteka `parity/check_corpus.go` fails on the test-only Centrifugo values and on an `X-Centrifugo-Secret` that is not a `{{var}}` or the documented deny literal | `TestCentrifugoRecorder` (value never stored), `TestCentrifugoRecorderRecordsPublishAndBroadcast`, parity `TestUniqueAndSecretScan` | pass; removal check RC-12 fails TestUniqueAndSecretScan | The corpus scan knows only the test-only values; live secrets never reach the isolated PHP |
 | T-11-29 | Spoofing | recorder listener and parity:broadcasts target | medium | mitigate | `CentrifugoRecorder.ListenAndServe` and `RecordBroadcasts` require loopback addresses (tide's T-02-01 rule) | `TestCentrifugoRecorderRefusesNonLoopback`, `TestCentrifugoRecorder` (loopback serve and shutdown), `TestRecordBroadcastsStep` | pass | None known |
 | T-11-30 | Repudiation | golden normalisation hiding regressions | medium | mitigate | `NormalizePublications` masks only `+00:00` timestamps, an exact `{user_id, name}` actor and captured ids under id keys; `DiffPublications` compares count, path, authorization and body; pending goldens skip, never pass | `TestNormalizePublications`, `TestDiffPublications`, parity `TestBroadcastGoldens` (created/updated must skip with "pending: Phase 12", enforced by `--named` and `--postgres`) | pass | Phase 12 must turn created/updated into assertions |
-| T-11-31 | Information Disclosure | lagoon.AfterCommit / beachcomber sync | high | mitigate | `modules/lagoon/transaction.go` `AfterCommit` buffers inside `lagoon.Transaction` and GORM's implicit single-statement transaction, and inside a foreign plain GORM `*sql.Tx` logs a warning and skips the callback; a nested `lagoon.Transaction` given a root handle returns an error; Cabana create/update/delete/bulk-delete and relation Link/Unlink and fonoteka `SaveAlbum` run in `lagoon.Transaction` | `TestTransactionAfterCommit` (`plain_gorm_transaction_is_refused`), `TestTransactionEdges` (nested root handle), beachcomber `TestSyncAfterCommit` (`plain_gorm_transaction_rollback_sends_nothing`), fonoteka `TestAlbumSearchDeleteAndFailures` (foreign transaction rollback never reaches Typesense); stages `--go`, `--postgres` | pass; removal check RC-14 fails TestTransactionAfterCommit | Code that writes inside its own plain GORM transaction gets no search sync (warned, not silent) until it adopts `lagoon.Transaction` or calls `Sync` after commit |
+| T-11-31 | Information Disclosure | lagoon.AfterCommit / beachcomber sync | high | mitigate | `modules/lagoon/transaction.go` `AfterCommit` buffers inside `lagoon.Transaction` and GORM's implicit single-statement transaction, and inside a foreign plain GORM `*sql.Tx` logs a warning and skips the callback; nested calls must use the exact parent transaction connection, so root and unrelated transaction handles are rejected; Cabana create/update/delete/bulk-delete and relation Link/Unlink and fonoteka `SaveAlbum` run in `lagoon.Transaction` | `TestTransactionAfterCommit` (`plain_gorm_transaction_is_refused`, `nested_rejects_unrelated_transaction_handle`), `TestTransactionEdges` (nested root handle), beachcomber `TestSyncAfterCommit` (`plain_gorm_transaction_rollback_sends_nothing`), fonoteka `TestAlbumSearchDeleteAndFailures` (foreign transaction rollback never reaches Typesense); stages `--go`, `--postgres`, `--named` | pass; removal checks RC-14 and RC-15 fail TestTransactionAfterCommit | Code that writes inside its own plain GORM transaction gets no search sync (warned, not silent) until it adopts `lagoon.Transaction` or calls `Sync` after commit |
 | T-11-32 | Tampering | Cabana and SaveAlbum ordered artist pivots | medium | mitigate | The Album row and its ordered artist pivots commit in one `lagoon.Transaction` (Cabana `CRUDService.save`, `RelationService.Link`/`Unlink`, fonoteka `SaveAlbum`), and the buffered sync reloads the committed row and pivots | fonoteka `TestAlbumsAdminSearchUsesCommittedArtists` (assembled admin router; indexed `artist_ids` equal the committed pivot order), `TestSaveAlbumDefersAfterCommitUntilArtistsSync` (callback sees the committed order; none on rollback) | pass; with Cabana `save` reverted to a plain transaction the admin test fails (no import), and with `SaveAlbum` reverted the SaveAlbum test fails (no callback), both checked by hand in the 11-08 close-out | Concurrent commits for the same Album are not serialized; the Phase 12 SQL re-gate of `SearchIDs` results remains the boundary |
 | T-11-SC | Tampering | Go module installs (River v0.47.0 and sub-modules) | high | mitigate | River is pinned at v0.47.0 with go.sum checksums; no Centrifugo, Typesense or Web Push client and no cron library was added in any plan | `check-phase11.sh --hygiene` (client libraries in `go list -m all` of both repositories, direct cron requirements, River version), `--self-test` (plants) | pass; removal check RC-13 (client rule disabled) fails `--self-test` | `robfig/cron/v3` is in the module graph only as River's test dependency, not a requirement |
 
 ## Removal checks
 
-Each row is one anchor-exact mutation from `scripts/check-phase11.sh --removal`. The anchor occurs exactly once in the file; the test must fail on an assertion (not a build failure); the file is restored and compared with `cmp` against the copy saved before the mutation. All thirteen were run on 2026-09-30 and all failed as required; `git status` was clean in both repositories afterwards. RC-14 was added and run in the 11-08 close-out on 2026-09-30 and failed as required.
+Each row is one anchor-exact mutation from `scripts/check-phase11.sh --removal`. The anchor occurs exactly once in the file; the test must fail on an assertion (not a build failure); the file is restored and compared with `cmp` against the copy saved before the mutation. All fifteen were run on 2026-09-30 and all failed as required; `git status` was clean in both repositories afterwards. RC-14 and RC-15 were added and run in the 11-08 close-out/review and failed as required.
 
 | Check | Threat | File | Anchor removed or changed | Replacement | Test run | Observed |
 |-------|--------|------|---------------------------|-------------|----------|----------|
@@ -77,6 +77,7 @@ Each row is one anchor-exact mutation from `scripts/check-phase11.sh --removal`.
 | RC-12 | T-11-28 | `../fonoteka.go/parity/check_corpus.go` | `if strings.Contains(text, v) {` (centrifugo test value scan) | `if false && strings.Contains(text, v) {` | `go test ./parity -run '^TestUniqueAndSecretScan$'` | fails: TestUniqueAndSecretScan; restored, cmp ok |
 | RC-13 | T-11-SC | `scripts/check-phase11.sh` (mutated copy) | `hits="$(grep -iE "$CLIENT_RE" "$modlist" \| head -n1 \|\| true)"` | `hits=""` | `bash <copy> --self-test` | fails: "refuse: self-test hygiene_11 accepted a planted client-gocent"; original untouched, cmp ok |
 | RC-14 | T-11-31 | `modules/lagoon/transaction.go` | `if transactionalHandle(db) {` (foreign-transaction refusal in `AfterCommit`) | `if false {` | `go test ./modules/lagoon -run '^TestTransactionAfterCommit$'` | fails: TestTransactionAfterCommit/plain_gorm_transaction_is_refused, callback_handle_has_a_clean_statement; restored, cmp ok |
+| RC-15 | T-11-31 | `modules/lagoon/transaction.go` | `if !parent.owns(gdb) {` (exact parent transaction identity) | `if false {` | `go test ./modules/lagoon -run '^TestTransactionAfterCommit$'` | fails: TestTransactionAfterCommit/nested_rejects_unrelated_transaction_handle; restored, cmp ok |
 
 ## Fixes made during the review
 

scripts/check-phase10.sh

@@ -54,6 +54,7 @@ require = set(os.environ.get("PHASE10_REQUIRE", "").split())
 expect_skip = set(os.environ.get("PHASE10_EXPECT_SKIP", "").split())
 skip_text = os.environ.get("PHASE10_SKIP_TEXT", "")
 skip_output = {}
+skipped = set()
 passed = set()
 failed_tests = {}
 failed_pkgs = []
@@ -79,6 +80,7 @@ with open(path, encoding="utf-8", errors="replace") as fh:
             # A later phase's documented pending test (for example the Phase
             # 12 broadcast goldens) may skip, but only with its pending text.
             if test in expect_skip and skip_text and any(skip_text in o for o in skip_output.get(test, [])):
+                skipped.add(test)
                 continue
             print(f"refuse: skipped {pkg} {test}", file=sys.stderr)
             sys.exit(2)
@@ -117,6 +119,14 @@ missing = sorted(name for name in require if name not in passed)
 if missing:
     print("refuse: required tests did not pass: " + ", ".join(missing), file=sys.stderr)
     sys.exit(5)
+unexpected_passes = sorted(expect_skip & passed)
+missing_skips = sorted(expect_skip - skipped)
+if unexpected_passes:
+    print("refuse: expected pending tests passed: " + ", ".join(unexpected_passes), file=sys.stderr)
+    sys.exit(7)
+if missing_skips:
+    print("refuse: expected pending skips did not occur: " + ", ".join(missing_skips), file=sys.stderr)
+    sys.exit(7)
 if not passed:
     print("refuse: zero tests", file=sys.stderr)
     sys.exit(3)
@@ -185,6 +195,10 @@ run_self_test() {
 	PHASE10_EXPECT_SKIP="TestG/created" PHASE10_SKIP_TEXT="pending: Phase 12" expect_detect pending-skip-without-text 2 \
 		'{"Action":"skip","Package":"p","Test":"TestG/created"}
 {"Action":"pass","Package":"p","Test":"TestG"}'
+	PHASE10_EXPECT_SKIP="TestG/created" PHASE10_SKIP_TEXT="pending: Phase 12" expect_detect pending-skip-missing 7 \
+		'{"Action":"pass","Package":"p","Test":"TestOther"}'
+	PHASE10_EXPECT_SKIP="TestG/created" PHASE10_SKIP_TEXT="pending: Phase 12" expect_detect pending-skip-passes 7 \
+		'{"Action":"pass","Package":"p","Test":"TestG/created"}'
 	expect_detect zero 3 '{"Action":"pass","Package":"git.golem15.com/golem15/summercms/modules/cabana"}'
 	expect_detect nonjson 4 '{"Action":"pass",'
 	expect_detect build 1 '{"Action":"build-fail","ImportPath":"p"}

scripts/check-phase11.sh

@@ -335,6 +335,9 @@ removal_table() {
   "hits=\"\"", "", "--self-test"],
  ["RC-14", "T-11-31", "root", "modules/lagoon/transaction.go",
   "if transactionalHandle(db) {",
+  "if false {", "./modules/lagoon", "^TestTransactionAfterCommit$"],
+ ["RC-15", "T-11-31", "root", "modules/lagoon/transaction.go",
+  "if !parent.owns(gdb) {",
   "if false {", "./modules/lagoon", "^TestTransactionAfterCommit$"]
 ]
 EOF
@@ -646,7 +649,7 @@ run_named() {
 		TestScheduledEntryMismatchSkipped TestScheduleUniqueByPeriod TestScheduleRunForeground TestScheduleValidation \
 		TestScheduleOrdering TestScheduleMissingCatalog TestScheduleLogWriter TestScheduleDueAt
 	phase11_tests "$ROOT" ./modules/bonfire TestCall TestCallEdges
-	phase11_tests "$ROOT" ./modules/lagoon TestOnDatabaseAfterActivate TestQueueMigrationsUpDown TestTransactionAfterCommit
+	phase11_tests "$ROOT" ./modules/lagoon TestOnDatabaseAfterActivate TestQueueMigrationsUpDown TestTransactionAfterCommit TestTransactionEdges
 	phase11_tests "$ROOT" ./modules/lighthouse TestBroadcastTx TestSuppression TestBulkEmitsOnce TestBroadcastEdges \
 		TestBroadcastSwallowedReadFailure TestMountSurfaces TestChannelIDMatchesPHP TestParseChannel TestRegistry
 	phase11_tests "$ROOT" ./modules/lighthouse/centrifugo TestTokenClaims TestTokenHandler TestClientRequests TestProxy TestHealthCommand