diff --git a/.task/feature-rdm-two-trees-project-audit.md b/.task/feature-rdm-two-trees-project-audit.md index 6eb2de3..04598fb 100644 --- a/.task/feature-rdm-two-trees-project-audit.md +++ b/.task/feature-rdm-two-trees-project-audit.md @@ -6,6 +6,7 @@ - Startup receipt: `/tmp/backend-w9-precheck-20261014-01.json` (SELF, exit 2, `SYNC_REQUIRED` for the separate Dev checkout `feature/rdm-increment-acceptance-note`, six commits behind Dev `origin/main`). This Backend task uses the independent-development path in Backend `AGENTS.md`; it does not write that Dev checkout or its task card. The receipt is preserved as a failure, not relabeled READY. - Approved affected design: `rdm-two-trees-stage6-audit-link-design-adjustment-r1.md`, SHA256 `385d9d7570217ddcce9fe60a60c99dc58c9a4875ad790584a724d09a210a6e52`; user approval in the task session covers this local Backend increment, not tag publication or Stage 7 acceptance. - Approved PostgreSQL cutoff adjustment: `rdm-two-trees-stage6-w9-pg-audit-cutoff-adjustment-r1.md`, SHA256 `2357b715a7c8f3451b34cc25a578d655e8a48f3f26d9cf7561ea7a13998ca857`; scope is isolated Backend implementation/testing, with no real database migration or release authorization. +- Approved all-writers affected design: `rdm-two-trees-stage6-w9-all-writers-cutoff-adjustment-r1.md`, SHA256 `44f6f7cd34bf95a9c1eb844bfdd70a5c2233e1b5ffc89b3091359dcb5fc3b5d8`; the direct user approval allows the local v34 correction and isolated tests only. ## Implemented local slice @@ -17,7 +18,9 @@ - Independent review of `a8fa2fe` found two P2 security defects: clock-based operation IDs could collide; and `NewScopedHost(owner,"rdm")` let another compiled plugin self-report the publisher's plugin ID. The new local increment uses 192 bits from `crypto/rand` for each host-generated operation ID. Both zero-offset and explicit-offset publication require a unique committed row for owner/project/operation; a historical duplicate rejects publication, including attempts to link another private command. Verifier registration returns a private `ProjectAuditPublisherFactory` for the registering plugin's background outbox; a generic scoped Host has no publisher method. Fixed-clock concurrent IDs, forged params, ambiguous private offset, forged scoped Host, and legitimate registered publisher each have focused tests. - The minted publisher exposes only `ProjectAuditPublisher`, not the full owner-scoped `Host`; tests reject attempts to use `NewScopedHost` with a self-reported `rdm` string as publisher authority. A publisher is a registration-time capability, while `IdempotencyKey` remains the separate retry identity. - Exact-source linked reads do not enumerate hidden links or reveal raw payload, private conversation ID, or content. Body disclosure and a broad project link list are not implemented; the public coverage reports the narrower capability. -- A PostgreSQL-only v33 default function now takes a table-specific transaction advisory lock before assigning `command_audit.id`, making supported implicit-ID writes commit in source-offset order. Migration and each PG append/project page verify CACHE 1, ascending non-cyclic positive IDs, the exact guarded default expression, and the next ID above the committed maximum; unsafe drift fails closed. SQLite remains single-writer. The owner-only project page reports `committed-prefix` only after the runtime check. Explicit online IDs and sequence maintenance remain prohibited; offline `storage-migrate` advances the owned sequence after copying IDs. +- A PostgreSQL-only v33 default function takes a table-specific transaction advisory lock before assigning `command_audit.id`, making supported implicit-ID writes commit in source-offset order. Migration and each PG append/project page verify CACHE 1, ascending non-cyclic positive IDs, the exact guarded default expression, and the next ID above the committed maximum; unsafe drift fails closed. SQLite remains single-writer. The owner-only project page reports `committed-prefix` only after runtime checks. v34 below closes the explicit-ID bypass; online sequence maintenance remains prohibited. +- Isolated PG RED receipt `/tmp/backend-w9-explicit-low-red-20261014-01.json` proved v33 still misreported `committed-prefix`: explicit ID 3 committed after normal ID 4 and disappeared across pages. Local v34 adds `ENABLE ALWAYS` insert and append-only mutation triggers. Every online INSERT checks READ COMMITTED, obtains the same transaction lock, and rejects a nonpositive, nonmonotone, or beyond-reserved ID. UPDATE/DELETE reject. PG append and every strong project page verify both trigger identities, exact event shape, and enablement; the existing default allocator also checks this guard. This is a local candidate, not yet independently reviewed or deployed. +- `storage-migrate` now copies `command_audit` within one ACCESS EXCLUSIVE transaction: it disables only the named insert guard, copies explicit IDs, verifies count and checksum, advances the owned sequence only forward, restores and verifies the guard, and records done before commit. Failure rolls back table data, trigger state, and done; nontransactional `setval` may leave a safe forward gap. Idempotent done checks guard health before reporting success. Other tables retain their existing path. ## Verification and remaining work @@ -31,5 +34,6 @@ - Historical pre-commit SELF load receipt `/tmp/backend-w9-pg-load-precommit-20261014-01.json` captured an initial v33 variant whose unlocked status function later produced false `cutoff unsafe` errors under concurrency; it is **not** valid performance acceptance evidence. The corrected status check reads committed MAX before non-MVCC sequence state and does not lock readers; the default ID allocator alone holds the transaction advisory lock and rechecks after `nextval`. `TestProjectAuditConcurrentReadWriteNeverReportsFalseDrift` and repeated PG load/cross-page runs protect that ordering. Three-condition pre-commit SELF raw receipts: `/tmp/backend-w9-pg-load-readlock-precommit-20261014-01.json` uses a test-only locked status wrapper, while `/tmp/backend-w9-pg-load-unlocked2-precommit-20261014-01.json` uses the final MAX-first unlocked status; each run also logs its v32 direct-INSERT baseline, PostgreSQL 18.6 version, 192 writes per mode/8 workers, elapsed time, p95, and throughput. The wrapper adds overhead and all measurements vary with hardware/load; no performance threshold has been approved, so they are Stage 6 cost evidence only. - Pre-commit SELF `/tmp/backend-w9-default-drift-red-20261014-01.json` showed that a replacement default containing the guarded function name but executing unguarded `nextval` was accepted; `/tmp/backend-w9-default-drift-green-20261014-01.json` verifies exact-expression rejection. Pre-commit SELF `/tmp/backend-w9-precommit-all-20261014-02.json` and `/tmp/backend-w9-precommit-pg-20261014-02.json` passed before this last guard tightening; fixed-HEAD runs are required below. - After the guard tightening, pre-commit SELF receipts `/tmp/backend-w9-precommit-all-final-20261014-01.json` (`go test ./... -count=1`), `/tmp/backend-w9-precommit-pg-final-20261014-01.json` (isolated PG store/storage-migrate), and `/tmp/backend-w9-precommit-pg-race-20261014-01.json` (cross-page/concurrent read-write with race detector) all exited 0. These still identify the dirty pre-commit tree, so they do not replace fixed-HEAD verification. -- Integration constraint: this branch uses schema v31 (audit attribution), v32 (links), and v33 (PG cutoff), while the independent W6 Backend branch also has a local v31. Neither branch has consumed the other's unmerged work. After the first branch lands, the later branch owner must sync the new main, renumber its unpublished migrations without rewriting an already released version, and rerun fresh/old SQLite and PG migration plus combined regression on the actual merged source. These isolated tests do not prove W6+W9 compatibility. +- v34 pre-commit SELF `/tmp/backend-w9-allwriters-load-precommit-20261014-01.json` records isolated PostgreSQL 18.6, three runs of 192 writes per mode with 8 workers, comparing v32 direct INSERT with v34 guarded `Store.AppendAudit`; it also logs three mixed runs of 180 writes with 6 writers and 160 200-row pages with 4 readers. Protected write throughput ranged about 2.7–3.0k/s, write p95 6.4–6.6ms, and mixed large-page p95 1.2–1.9ms on this one host. These are SELF cost signals with no approved threshold and no Stage 7 acceptance meaning; fixed-HEAD tests still follow. +- Integration constraint: this branch uses schema v31 (audit attribution), v32 (links), v33 (PG implicit cutoff), and v34 (all-writer guard), while the independent W6 Backend branch also has a local v31. Neither branch has consumed the other's unmerged work. After the first branch lands, the later branch owner must sync the new main, renumber its unpublished migrations without rewriting an already released version, and rerun fresh/old SQLite and PG migration plus combined regression on the actual merged source. These isolated tests do not prove W6+W9 compatibility. - Required next W9 work: real RDM committed-event verifier/outbox consumer, Backend project-event source, run ID propagation, model/tool/background receipts, body/detail disclosure, broad authorized linked-operation pagination, history/export, and combined product verification remain unfinished. A domain commit followed by a lost Backend audit append leaves the link pending; recovery must retain and reconcile that unknown state without claiming a host receipt. Exact-link and owner-only paging do not satisfy Stage 7 audit acceptance. PostgreSQL v33 adds a global command-audit append lock; its measured wait/throughput cost has no approved performance threshold and needs human Stage 6 review. diff --git a/cmd/storage-migrate/integration_test.go b/cmd/storage-migrate/integration_test.go index 57cb9f0..7df23a7 100644 --- a/cmd/storage-migrate/integration_test.go +++ b/cmd/storage-migrate/integration_test.go @@ -101,6 +101,105 @@ func TestMigrateEndToEndAndIdempotent(t *testing.T) { } } +func TestCommandAuditExclusiveCopyRollsBackAndNeverRewindsSequence(t *testing.T) { + target, cleanup := pgSchemaDSN(t) + defer cleanup() + fixture := buildFixture(t) + src, err := openSourceRO(fixture) + if err != nil { + t.Fatal(err) + } + defer src.Close() + st, err := store.OpenPostgres(target, filepath.Join(t.TempDir(), "artifacts")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + db := st.DB() + if _, err := db.Exec(controlDDL); err != nil { + t.Fatal(err) + } + cols, pk, err := targetColumns(db, "command_audit") + if err != nil { + t.Fatal(err) + } + bools, err := boolColumns(db, "command_audit", cols) + if err != nil { + t.Fatal(err) + } + count, checksum, err := sourceScan(src, "command_audit", cols, pk, bools) + if err != nil { + t.Fatal(err) + } + blocker, err := db.Begin() + if err != nil { + t.Fatal(err) + } + var existing int + if err := blocker.QueryRow(`SELECT COUNT(*) FROM command_audit`).Scan(&existing); err != nil { + t.Fatal(err) + } + timeoutDB, err := sql.Open("pgx", store.AppendPGRuntimeParam(target, "lock_timeout", "100")) + if err != nil { + t.Fatal(err) + } + if _, err := migrateCommandAudit(timeoutDB, src, "retry-audit", cols, pk, bools, count, checksum); err == nil || !strings.Contains(err.Error(), "lock timeout") { + t.Fatalf("exclusive copy did not fail on a held reader: %v", err) + } + timeoutDB.Close() + if err := blocker.Rollback(); err != nil { + t.Fatal(err) + } + if _, err := migrateCommandAudit(db, src, "retry-audit", cols, pk, bools, count+1, checksum); err == nil { + t.Fatal("mismatched copy committed despite in-transaction target count check") + } + var rows, done int + if err := db.QueryRow(`SELECT COUNT(*) FROM command_audit`).Scan(&rows); err != nil || rows != 0 { + t.Fatalf("failed copy left rows: %d %v", rows, err) + } + if err := db.QueryRow(`SELECT COUNT(*) FROM migration_table_state WHERE table_name='command_audit'`).Scan(&done); err != nil || done != 0 { + t.Fatalf("failed copy recorded done: %d %v", done, err) + } + var enabled string + if err := db.QueryRow(`SELECT tgenabled FROM pg_trigger WHERE tgrelid='command_audit'::regclass AND tgname='project_audit_insert_guard_v1'`).Scan(&enabled); err != nil || enabled != "A" { + t.Fatalf("failed copy left guard disabled: %q %v", enabled, err) + } + var seq string + if err := db.QueryRow(`SELECT pg_get_serial_sequence('command_audit','id')`).Scan(&seq); err != nil { + t.Fatal(err) + } + // setval is non-transactional; an interrupted offline attempt can leave a + // forward gap even though its rows and trigger-state changes roll back. + tx, err := db.Begin() + if err != nil { + t.Fatal(err) + } + if _, err := tx.Exec(`LOCK TABLE command_audit IN ACCESS EXCLUSIVE MODE`); err != nil { + t.Fatal(err) + } + if _, err := tx.Exec(`ALTER TABLE command_audit DISABLE TRIGGER project_audit_insert_guard_v1`); err != nil { + t.Fatal(err) + } + if _, err := tx.Exec(`SELECT setval($1::regclass,50,true)`, seq); err != nil { + t.Fatal(err) + } + if err := tx.Rollback(); err != nil { + t.Fatal(err) + } + if _, err := migrateCommandAudit(db, src, "retry-audit", cols, pk, bools, count, checksum); err != nil { + t.Fatalf("retry after rollback: %v", err) + } + var last int64 + if err := db.QueryRow(`SELECT last_value FROM ` + seq).Scan(&last); err != nil || last < 50 { + t.Fatalf("copy rewound sequence: last=%d err=%v", last, err) + } + var nextID int64 + if err := db.QueryRow(`INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload) + VALUES ('2026-10-14T00:00:00Z','after-copy','','','committed','','','alice','{}') RETURNING id`).Scan(&nextID); err != nil || nextID <= 50 { + t.Fatalf("post-copy ID did not advance: id=%d err=%v", nextID, err) + } +} + func TestMigrateResumeAfterInterrupt(t *testing.T) { target, cleanup := pgSchemaDSN(t) defer cleanup() diff --git a/cmd/storage-migrate/main.go b/cmd/storage-migrate/main.go index 3a4f366..65b571c 100644 --- a/cmd/storage-migrate/main.go +++ b/cmd/storage-migrate/main.go @@ -508,9 +508,18 @@ func migrateTable(db *sql.DB, src *sql.DB, migrationID, table string) (map[strin if countNow != srcCount || sumNow != srcChecksum { return nil, fmt.Errorf("源与登记不符(疑似换快照):count %d/%d checksum %s…/%s…", countNow, srcCount, sumNow[:12], srcChecksum[:12]) } + if table == "command_audit" { + var guarded bool + if err := db.QueryRow(`SELECT project_audit_cutoff_safe('command_audit'::regclass) AND project_audit_writers_guarded('command_audit'::regclass)`).Scan(&guarded); err != nil || !guarded { + return nil, fmt.Errorf("command_audit idempotent guard is unsafe: safe=%v err=%v", guarded, err) + } + } entry := map[string]any{"table": table, "source_count": countNow, "target_count": dstCountStored, "checksum": sumNow, "state": "done(idempotent)"} return entry, nil } + if table == "command_audit" { + return migrateCommandAudit(db, src, migrationID, cols, pk, bools, countNow, sumNow) + } if err := copyRows(db, src, table, cols); err != nil { return nil, err } @@ -663,7 +672,11 @@ func sourceScan(src *sql.DB, table string, cols, pk []string, bools map[string]b return scanChecksum(rows, cols, bools) } -func targetScan(db *sql.DB, table string, cols, pk []string, bools map[string]bool) (int64, string, error) { +type rowsQuerier interface { + Query(string, ...any) (*sql.Rows, error) +} + +func targetScan(db rowsQuerier, table string, cols, pk []string, bools map[string]bool) (int64, string, error) { rows, err := db.Query(`SELECT ` + quoteJoin(cols, `"`) + ` FROM "` + table + `" ORDER BY ` + quoteJoin(pk, `"`)) if err != nil { return 0, "", err @@ -686,6 +699,18 @@ func copyRows(db *sql.DB, src *sql.DB, table string, cols []string) error { if err != nil { return err } + tx, err := db.Begin() + if err != nil { + return err + } + defer tx.Rollback() + if err := copyRowsInTx(tx, src, table, cols, bools); err != nil { + return err + } + return tx.Commit() +} + +func copyRowsInTx(tx *sql.Tx, src *sql.DB, table string, cols []string, bools map[string]bool) error { colList := quoteJoin(cols, `"`) ph := make([]string, len(cols)) for i := range ph { @@ -702,11 +727,6 @@ func copyRows(db *sql.DB, src *sql.DB, table string, cols []string) error { for i := range vals { ptrs[i] = &vals[i] } - tx, err := db.Begin() - if err != nil { - return err - } - defer tx.Rollback() stmt, err := tx.Prepare(fmt.Sprintf(`INSERT INTO "%s" (%s) VALUES (%s) ON CONFLICT DO NOTHING`, table, colList, placeholders)) if err != nil { return err @@ -734,7 +754,76 @@ func copyRows(db *sql.DB, src *sql.DB, table string, cols []string) error { if err := srcRows.Err(); err != nil { return err } - return tx.Commit() + return nil +} + +// command_audit alone needs an offline transaction: explicit source IDs do not +// pass the online monotone guard, and disabling it must never become visible +// separately from the copied rows, sequence advance, or done marker. +func migrateCommandAudit(db *sql.DB, src *sql.DB, migrationID string, cols, pk []string, bools map[string]bool, sourceCount int64, sourceChecksum string) (map[string]any, error) { + tx, err := db.Begin() + if err != nil { + return nil, err + } + defer tx.Rollback() + if _, err := tx.Exec(`LOCK TABLE command_audit IN ACCESS EXCLUSIVE MODE`); err != nil { + return nil, err + } + if _, err := tx.Exec(`ALTER TABLE command_audit DISABLE TRIGGER project_audit_insert_guard_v1`); err != nil { + return nil, err + } + if err := copyRowsInTx(tx, src, "command_audit", cols, bools); err != nil { + return nil, err + } + targetCount, targetChecksum, err := targetScan(tx, "command_audit", cols, pk, bools) + if err != nil { + return nil, err + } + if targetCount != sourceCount || targetChecksum != sourceChecksum { + return nil, fmt.Errorf("command_audit copy mismatch: count %d/%d checksum %s/%s", targetCount, sourceCount, targetChecksum, sourceChecksum) + } + if err := advanceCommandAuditSequence(tx); err != nil { + return nil, err + } + if _, err := tx.Exec(`ALTER TABLE command_audit ENABLE ALWAYS TRIGGER project_audit_insert_guard_v1`); err != nil { + return nil, err + } + var guarded bool + if err := tx.QueryRow(`SELECT project_audit_cutoff_safe('command_audit'::regclass) AND project_audit_writers_guarded('command_audit'::regclass)`).Scan(&guarded); err != nil || !guarded { + return nil, fmt.Errorf("command_audit guard not restored: safe=%v err=%v", guarded, err) + } + if _, err := tx.Exec(`INSERT INTO migration_table_state (migration_id, table_name, source_count, target_count, checksum, state, finished_at) + VALUES ($1,'command_audit',$2,$3,$4,'done',$5) + ON CONFLICT(migration_id, table_name) DO UPDATE SET source_count=excluded.source_count, target_count=excluded.target_count, checksum=excluded.checksum, state=excluded.state, finished_at=excluded.finished_at`, + migrationID, sourceCount, targetCount, sourceChecksum, nowISO()); err != nil { + return nil, err + } + if err := tx.Commit(); err != nil { + return nil, err + } + return map[string]any{"table": "command_audit", "source_count": sourceCount, "target_count": targetCount, "checksum": sourceChecksum, "state": "done"}, nil +} + +func advanceCommandAuditSequence(tx *sql.Tx) error { + var seq string + if err := tx.QueryRow(`SELECT pg_get_serial_sequence('command_audit','id')`).Scan(&seq); err != nil || seq == "" { + return fmt.Errorf("command_audit sequence missing: %v", err) + } + var last int64 + var called bool + if err := tx.QueryRow(fmt.Sprintf(`SELECT last_value,is_called FROM %s`, seq)).Scan(&last, &called); err != nil { + return err + } + var maxID sql.NullInt64 + if err := tx.QueryRow(`SELECT MAX(id) FROM command_audit`).Scan(&maxID); err != nil { + return err + } + if maxID.Valid && (maxID.Int64 > last || (maxID.Int64 == last && !called)) { + if _, err := tx.Exec(`SELECT setval($1::regclass,$2,true)`, seq, maxID.Int64); err != nil { + return err + } + } + return nil } func sqliteBool(v any) bool { diff --git a/internal/app/project_audit_test.go b/internal/app/project_audit_test.go index 3640da9..7bda52b 100644 --- a/internal/app/project_audit_test.go +++ b/internal/app/project_audit_test.go @@ -2,8 +2,10 @@ package app import ( "context" + "database/sql" "encoding/json" "errors" + "os" "path/filepath" "strings" "testing" @@ -99,3 +101,138 @@ func TestProjectAuditHostChecksDomainReadAndProjectsStoredReceipts(t *testing.T) t.Fatalf("revoked audit.read: %v", err) } } + +// A direct explicit ID bypasses the v33 default allocator. This must never +// yield a committed-prefix page that permanently skips the late lower row. +func TestProjectAuditPostgresExplicitLowIDCannotHideAcrossPages(t *testing.T) { + dsn := os.Getenv("AGENT_PG_TEST_DSN") + if dsn == "" { + t.Skip("AGENT_PG_TEST_DSN unset: isolated PostgreSQL check NOT_RUN") + } + admin, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { admin.Close() }) + if _, err := admin.Exec(`CREATE EXTENSION IF NOT EXISTS pg_trgm`); err != nil { + t.Fatal(err) + } + schema := "w9_" + strings.ToLower(string(store.NewID("audit"))) + if _, err := admin.Exec(`CREATE SCHEMA ` + schema); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { admin.Exec(`DROP SCHEMA IF EXISTS ` + schema + ` CASCADE`) }) + st, err := store.OpenPostgres(store.AppendPGRuntimeParam(dsn, "search_path", schema+",public"), filepath.Join(t.TempDir(), "artifacts")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + if err := st.EnsureUser("alice", "Alice"); err != nil { + t.Fatal(err) + } + project, err := st.CreateProject("alice", "A") + if err != nil { + t.Fatal(err) + } + conversation, err := st.CreateConversation("alice", string(project.ID), "work", "") + if err != nil { + t.Fatal(err) + } + for _, command := range []string{"first", "second"} { + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: command, ConversationID: string(conversation.ID), State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + } + insert := `INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload,actor_name,project_id) + VALUES ($1,$2,'','','committed','',$3,'alice',$4,'Alice',$5) RETURNING id` + gap, err := st.DB().Begin() + if err != nil { + t.Fatal(err) + } + defer gap.Rollback() + var gapID int64 + if err := gap.QueryRow(insert, time.Now().UTC().Format(time.RFC3339Nano), "rolled-back-gap", string(conversation.ID), `{}`, string(project.ID)).Scan(&gapID); err != nil { + t.Fatal(err) + } + if gapID != 3 { + t.Fatalf("rollback gap ID=%d want=3", gapID) + } + if err := gap.Rollback(); err != nil { + t.Fatal(err) + } + a, err := st.DB().Begin() + if err != nil { + t.Fatal(err) + } + defer a.Rollback() + env := agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "explicit-late", ConversationID: string(conversation.ID), State: agentcommand.StateCommitted} + raw, err := json.Marshal(env) + if err != nil { + t.Fatal(err) + } + _, err = a.Exec(`INSERT INTO command_audit(id,time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload,actor_name,project_id) + VALUES (3,$1,$2,'','','committed','',$3,'alice',$4,'Alice',$5)`, env.Time.UTC().Format(time.RFC3339Nano), env.CommandID, string(conversation.ID), string(raw), string(project.ID)) + if err != nil { + // Rejecting the online explicit ID is also a safe outcome. + t.Logf("online explicit low ID rejected: %v", err) + return + } + slot := &projectAuditSlot{} + caps := hostcaps{st: st, projectAudit: slot} + reg := pluginRegistry{pluginID: "rdm", caps: caps} + if err := reg.RegisterProjectAuditAuthorizer(func(actor string, projectID core.ID) (bool, error) { + return actor == "alice" && projectID == project.ID, nil + }); err != nil { + t.Fatal(err) + } + h := inprocHost{caps: caps, owner: "alice", actor: "alice", pluginID: "rdm"} + committed := make(chan error, 1) + started := make(chan struct{}) + go func() { + close(started) + committed <- st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "implicit-high", ConversationID: string(conversation.ID), State: agentcommand.StateCommitted}) + }() + <-started + var first extension.ProjectAuditPage + highCommittedEarly := false + select { + case err := <-committed: + if err != nil { + t.Fatal(err) + } + highCommittedEarly = true + case <-time.After(500 * time.Millisecond): + } + if highCommittedEarly { + first, err = h.QueryProjectAudit(context.Background(), extension.ProjectAuditQuery{ProjectID: project.ID, Limit: 2}) + if err != nil { + t.Fatal(err) + } + if len(first.Receipts) != 2 || first.Receipts[0].SourceOffset != 4 || first.Receipts[1].SourceOffset != 2 || first.CutoffOffset != 4 || first.NextOffset != 2 || first.Coverage.CutoffConsistency != "committed-prefix" { + t.Fatalf("counterexample first page changed: %+v", first) + } + } + if err := a.Commit(); err != nil { + t.Fatal(err) + } + if !highCommittedEarly { + if err := <-committed; err != nil { + t.Fatal(err) + } + first, err = h.QueryProjectAudit(context.Background(), extension.ProjectAuditQuery{ProjectID: project.ID, Limit: 2}) + if err != nil { + t.Fatal(err) + } + } + second, err := h.QueryProjectAudit(context.Background(), extension.ProjectAuditQuery{ProjectID: project.ID, BeforeOffset: first.NextOffset, CutoffOffset: first.CutoffOffset, Limit: 2}) + if err != nil { + t.Fatal(err) + } + seen := map[int64]bool{} + for _, receipt := range append(first.Receipts, second.Receipts...) { + seen[receipt.SourceOffset] = true + } + if !seen[3] || len(seen) != 4 { + t.Fatalf("committed-prefix omitted explicit low ID 3: early=%v first=%+v second=%+v", highCommittedEarly, first, second) + } +} diff --git a/internal/store/audit.go b/internal/store/audit.go index 6894c02..9b41a1c 100644 --- a/internal/store/audit.go +++ b/internal/store/audit.go @@ -38,7 +38,8 @@ type ProjectCommandAuditRow struct { } // ErrAuditCutoffUnsafe prevents a PostgreSQL page from claiming a fixed cutoff -// when the installed ID generator or sequence no longer proves commit order. +// when the ID generator, sequence, or all-writer database guard cannot prove +// commit order. var ErrAuditCutoffUnsafe = errors.New("store: project audit cutoff unsafe") func (s *Store) requireProjectAuditCommittedPrefix() error { @@ -46,7 +47,7 @@ func (s *Store) requireProjectAuditCommittedPrefix() error { return nil } // SQLite serializes writers. var safe bool - if err := s.db.QueryRow(`SELECT project_audit_cutoff_safe('command_audit'::regclass)`).Scan(&safe); err != nil { + if err := s.db.QueryRow(`SELECT project_audit_cutoff_safe('command_audit'::regclass) AND project_audit_writers_guarded('command_audit'::regclass)`).Scan(&safe); err != nil { return fmt.Errorf("%w: %v", ErrAuditCutoffUnsafe, err) } if !safe { diff --git a/internal/store/migrate.go b/internal/store/migrate.go index 24a9763..2ab5a1c 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -919,6 +919,84 @@ func init() { `DROP FUNCTION project_audit_cutoff_safe(regclass)`, }, }}, + migration{version: 34, name: "project_audit_all_writers_guard", + up: map[string][]string{ + "sqlite": {`SELECT 1`}, + "postgres": { + `LOCK TABLE command_audit IN ACCESS EXCLUSIVE MODE`, + `CREATE FUNCTION project_audit_insert_guard_v1() RETURNS trigger LANGUAGE plpgsql VOLATILE AS $$ + DECLARE seq regclass; last_number numeric; was_called boolean; max_id bigint; + BEGIN + IF current_setting('transaction_isolation') <> 'read committed' THEN + RAISE EXCEPTION 'command audit insert requires read committed isolation'; + END IF; + PERFORM pg_advisory_xact_lock(715328491,TG_RELID::int4); + IF NOT project_audit_cutoff_safe(TG_RELID) OR NOT project_audit_writers_guarded(TG_RELID) THEN + RAISE EXCEPTION 'command audit write guard is unsafe'; + END IF; + seq := pg_get_serial_sequence(TG_RELID::regclass::text,'id')::regclass; + EXECUTE format('SELECT last_value,is_called FROM %s',seq) INTO last_number,was_called; + EXECUTE format('SELECT COALESCE(MAX(id),0) FROM %s',TG_RELID::regclass) INTO max_id; + IF NEW.id IS NULL OR NEW.id<=0 OR NEW.id<=max_id OR NOT was_called OR NEW.id>last_number THEN + RAISE EXCEPTION 'command audit ID is not commit-monotone'; + END IF; + RETURN NEW; + END $$`, + `CREATE FUNCTION project_audit_mutation_guard_v1() RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN RAISE EXCEPTION 'command audit is append-only'; END $$`, + `CREATE TRIGGER project_audit_insert_guard_v1 BEFORE INSERT ON command_audit FOR EACH ROW EXECUTE FUNCTION project_audit_insert_guard_v1()`, + `CREATE TRIGGER project_audit_mutation_guard_v1 BEFORE UPDATE OR DELETE ON command_audit FOR EACH ROW EXECUTE FUNCTION project_audit_mutation_guard_v1()`, + `ALTER TABLE command_audit ENABLE ALWAYS TRIGGER project_audit_insert_guard_v1`, + `ALTER TABLE command_audit ENABLE ALWAYS TRIGGER project_audit_mutation_guard_v1`, + `CREATE FUNCTION project_audit_writers_guarded(target regclass) RETURNS boolean LANGUAGE plpgsql VOLATILE AS $$ + DECLARE insert_ok boolean; mutation_ok boolean; + BEGIN + SELECT EXISTS (SELECT 1 FROM pg_trigger WHERE tgrelid=target AND tgname='project_audit_insert_guard_v1' + AND NOT tgisinternal AND tgenabled='A' AND tgtype=7 + AND tgfoid=to_regprocedure('project_audit_insert_guard_v1()')) INTO insert_ok; + SELECT EXISTS (SELECT 1 FROM pg_trigger WHERE tgrelid=target AND tgname='project_audit_mutation_guard_v1' + AND NOT tgisinternal AND tgenabled='A' AND tgtype=27 + AND tgfoid=to_regprocedure('project_audit_mutation_guard_v1()')) INTO mutation_ok; + RETURN insert_ok AND mutation_ok; + END $$`, + `CREATE OR REPLACE FUNCTION next_command_audit_id(target regclass) RETURNS bigint LANGUAGE plpgsql VOLATILE AS $$ + DECLARE seq regclass; allocated bigint; max_id bigint; + BEGIN + PERFORM pg_advisory_xact_lock(715328491,target::oid::int4); + IF NOT project_audit_cutoff_safe(target) OR NOT project_audit_writers_guarded(target) THEN + RAISE EXCEPTION 'project audit cutoff guard is unsafe'; + END IF; + seq := pg_get_serial_sequence(target::text,'id')::regclass; + allocated := nextval(seq); + EXECUTE format('SELECT COALESCE(MAX(id),0) FROM %s',target) INTO max_id; + IF allocated<=0 OR allocated<=max_id THEN RAISE EXCEPTION 'project audit ID did not advance beyond committed rows'; END IF; + RETURN allocated; + END $$`, + `DO $$ BEGIN IF NOT project_audit_cutoff_safe('command_audit'::regclass) OR NOT project_audit_writers_guarded('command_audit'::regclass) THEN RAISE EXCEPTION 'project audit writer guard migration precondition failed'; END IF; END $$`, + }, + }, + down: map[string][]string{ + "sqlite": {`SELECT 1`}, + "postgres": { + `LOCK TABLE command_audit IN ACCESS EXCLUSIVE MODE`, + `CREATE OR REPLACE FUNCTION next_command_audit_id(target regclass) RETURNS bigint LANGUAGE plpgsql VOLATILE AS $$ + DECLARE seq regclass; allocated bigint; max_id bigint; + BEGIN + PERFORM pg_advisory_xact_lock(715328491,target::oid::int4); + IF NOT project_audit_cutoff_safe(target) THEN RAISE EXCEPTION 'project audit cutoff sequence is unsafe'; END IF; + seq := pg_get_serial_sequence(target::text,'id')::regclass; + allocated := nextval(seq); + EXECUTE format('SELECT COALESCE(MAX(id),0) FROM %s',target) INTO max_id; + IF allocated<=0 OR allocated<=max_id THEN RAISE EXCEPTION 'project audit ID did not advance beyond committed rows'; END IF; + RETURN allocated; + END $$`, + `DROP TRIGGER project_audit_insert_guard_v1 ON command_audit`, + `DROP TRIGGER project_audit_mutation_guard_v1 ON command_audit`, + `DROP FUNCTION project_audit_insert_guard_v1()`, + `DROP FUNCTION project_audit_mutation_guard_v1()`, + `DROP FUNCTION project_audit_writers_guarded(regclass)`, + }, + }}, ) } diff --git a/internal/store/migrate_compat.go b/internal/store/migrate_compat.go index a653f3b..a4f7766 100644 --- a/internal/store/migrate_compat.go +++ b/internal/store/migrate_compat.go @@ -38,6 +38,7 @@ var migrationBreaksOldReaders = map[int]bool{ 31: false, // command_audit_attribution_snapshot:仅加列/索引,旧读者不受影响 32: false, // project_audit_committed_links:新表,旧读者不受影响 33: false, // project_audit_committed_prefix:PG 审计 ID 默认函数,旧读者兼容 + 34: false, // project_audit_all_writers_guard:PG 在线审计写入门禁,旧读者兼容 } // SchemaTarget 是本二进制会迁移到的 schema 版本。 diff --git a/internal/store/project_audit_sequence_test.go b/internal/store/project_audit_sequence_test.go index 022f2b8..5742ffc 100644 --- a/internal/store/project_audit_sequence_test.go +++ b/internal/store/project_audit_sequence_test.go @@ -2,6 +2,7 @@ package store import ( "context" + "database/sql" "encoding/json" "fmt" "os" @@ -26,6 +27,88 @@ func openProjectAuditPG(t *testing.T) *Store { return st } +func TestProjectAuditAllWritersRejectUnsafeOnlineChanges(t *testing.T) { + st := openProjectAuditPG(t) + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "first", State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + row := `INSERT INTO command_audit(id,time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload) + VALUES ($1,'2026-10-14T00:00:00Z','explicit','','','committed','','','alice','{}')` + if _, err := st.db.Exec(row, 100); err == nil { + t.Fatal("online explicit ID above reserved sequence position was accepted") + } + if _, err := st.db.Exec(`UPDATE command_audit SET command_id='changed' WHERE id=1`); err == nil { + t.Fatal("online audit UPDATE was accepted") + } + if _, err := st.db.Exec(`DELETE FROM command_audit WHERE id=1`); err == nil { + t.Fatal("online audit DELETE was accepted") + } + for _, level := range []sql.IsolationLevel{sql.LevelRepeatableRead, sql.LevelSerializable} { + tx, err := st.db.BeginTx(context.Background(), &sql.TxOptions{Isolation: level}) + if err != nil { + t.Fatal(err) + } + _, err = tx.Exec(`INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload) + VALUES ('2026-10-14T00:00:00Z','stale-snapshot','','','committed','','','alice','{}')`) + tx.Rollback() + if err == nil || !strings.Contains(err.Error(), "read committed") { + t.Fatalf("isolation %v did not fail closed: %v", level, err) + } + } + seq := commandAuditSequenceName(t, st) + if _, err := st.db.Exec(`SELECT nextval($1::regclass)`, seq); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec(`INSERT INTO command_audit(id,time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload) + VALUES (4,'2026-10-14T00:00:00Z','first-of-batch','','','committed','','','alice','{}'), + (1,'2026-10-14T00:00:00Z','decreasing','','','committed','','','alice','{}')`); err == nil { + t.Fatal("unordered multirow audit INSERT was accepted") + } + if _, err := st.db.Exec(row+` ON CONFLICT DO NOTHING`, 1); err == nil { + t.Fatal("online replay bypassed the INSERT guard through ON CONFLICT") + } + var count int + if err := st.db.QueryRow(`SELECT COUNT(*) FROM command_audit`).Scan(&count); err != nil || count != 1 { + t.Fatalf("unsafe writes persisted: count=%d err=%v", count, err) + } +} + +func TestProjectAuditAllWritersGuardDriftFailsClosed(t *testing.T) { + st := openProjectAuditPG(t) + for _, trigger := range []string{"project_audit_insert_guard_v1", "project_audit_mutation_guard_v1"} { + if _, err := st.db.Exec(`ALTER TABLE command_audit DISABLE TRIGGER ` + trigger); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatalf("read accepted disabled trigger %s", trigger) + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "drift"}); err == nil { + t.Fatalf("append accepted disabled trigger %s", trigger) + } + if _, err := st.db.Exec(`ALTER TABLE command_audit ENABLE ALWAYS TRIGGER ` + trigger); err != nil { + t.Fatal(err) + } + } + if _, err := st.db.Exec(`CREATE OR REPLACE TRIGGER project_audit_insert_guard_v1 BEFORE INSERT ON command_audit FOR EACH ROW EXECUTE FUNCTION project_audit_mutation_guard_v1()`); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec(`ALTER TABLE command_audit ENABLE ALWAYS TRIGGER project_audit_insert_guard_v1`); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatal("read accepted rebound insert trigger") + } + if _, err := st.db.Exec(`CREATE OR REPLACE TRIGGER project_audit_insert_guard_v1 BEFORE INSERT ON command_audit FOR EACH ROW EXECUTE FUNCTION project_audit_insert_guard_v1()`); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec(`ALTER TABLE command_audit ENABLE ALWAYS TRIGGER project_audit_insert_guard_v1`); err != nil { + t.Fatal(err) + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "restored"}); err != nil { + t.Fatalf("restored guard: %v", err) + } +} + func commandAuditSequenceName(t *testing.T, st *Store) string { t.Helper() var name string @@ -335,6 +418,12 @@ func TestProjectAuditSequenceLockTimeoutAndRollback(t *testing.T) { if _, err := conn.ExecContext(ctx, `RESET lock_timeout`); err != nil { t.Fatal(err) } + cancelCtx, cancel := context.WithTimeout(ctx, 150*time.Millisecond) + defer cancel() + var canceledID int64 + if err := conn.QueryRowContext(cancelCtx, insert, args...).Scan(&canceledID); err == nil { + t.Fatalf("canceled writer passed advisory lock: id=%d", canceledID) + } if err := tx.Rollback(); err != nil { t.Fatal(err) } @@ -426,7 +515,7 @@ func TestProjectAuditSequenceConcurrentLoad(t *testing.T) { statusMode = "test-only locked status wrapper" } newElapsed, newP95 := run("guarded", func(env agentcommand.AuditEnvelope) error { return st.AppendAudit("alice", env) }) - t.Logf("isolated PostgreSQL %s audit append: %d writes per mode / %d workers; v32 direct INSERT=%s p95=%s (%.0f/s), v33 Store.AppendAudit with %s=%s p95=%s (%.0f/s); hardware/load dependent SELF only", + t.Logf("isolated PostgreSQL %s audit append: %d writes per mode / %d workers; v32 direct INSERT=%s p95=%s (%.0f/s), v34 Store.AppendAudit with %s=%s p95=%s (%.0f/s); hardware/load dependent SELF only", serverVersion, writers*each, writers, oldElapsed, oldP95, float64(writers*each)/oldElapsed.Seconds(), statusMode, newElapsed, newP95, float64(writers*each)/newElapsed.Seconds()) seen := map[string]bool{} var before, cutoff int64 @@ -464,8 +553,14 @@ func TestProjectAuditConcurrentReadWriteNeverReportsFalseDrift(t *testing.T) { if err != nil { t.Fatal(err) } + for i := 0; i < 210; i++ { + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: fmt.Sprintf("seed-%d", i), ConversationID: string(c.ID), State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + } const writers, writes, readers, reads = 6, 30, 4, 40 errs := make(chan error, writers*writes+readers*reads) + readDurations := make(chan time.Duration, readers*reads) var wg sync.WaitGroup for worker := 0; worker < writers; worker++ { wg.Add(1) @@ -485,10 +580,17 @@ func TestProjectAuditConcurrentReadWriteNeverReportsFalseDrift(t *testing.T) { go func() { defer wg.Done() for i := 0; i < reads; i++ { - if _, _, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10); err != nil { + started := time.Now() + page, _, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 200) + if err != nil { errs <- fmt.Errorf("read: %w", err) return } + if len(page) != 200 { + errs <- fmt.Errorf("large page rows=%d want=200", len(page)) + return + } + readDurations <- time.Since(started) } }() } @@ -497,4 +599,12 @@ func TestProjectAuditConcurrentReadWriteNeverReportsFalseDrift(t *testing.T) { for err := range errs { t.Fatal(err) } + close(readDurations) + var samples []time.Duration + for duration := range readDurations { + samples = append(samples, duration) + } + sort.Slice(samples, func(i, j int) bool { return samples[i] < samples[j] }) + t.Logf("isolated PG mixed load: %d append writes / %d writers and %d 200-row pages / %d readers; page p95=%s, SELF cost signal only", + writers*writes, writers, readers*reads, readers, samples[len(samples)*95/100]) } diff --git a/pkg/extension/extension.go b/pkg/extension/extension.go index 173e5f0..0287fc8 100644 --- a/pkg/extension/extension.go +++ b/pkg/extension/extension.go @@ -44,7 +44,9 @@ import ( // ProjectAuditPublisherFactory; scoped Host string IDs cannot publish links. // 2.17.0-experimental: PostgreSQL project audit pages require a verified // committed-prefix ID generator; unsafe sequence/default drift rejects reads. -const SurfaceVersion = "2.17.0-experimental" +// 2.18.0-experimental: all PostgreSQL audit writes require the database +// insert guard and append-only policy before a committed-prefix claim. +const SurfaceVersion = "2.18.0-experimental" // ErrNotLinked 表示进程没有空白导入 link 包,Main 无法启动服务。 var ErrNotLinked = errors.New("extension: runner not linked; blank-import agent-system-backend/pkg/extension/link") diff --git a/pkg/extension/project_audit.go b/pkg/extension/project_audit.go index d1592fe..c05d6dc 100644 --- a/pkg/extension/project_audit.go +++ b/pkg/extension/project_audit.go @@ -103,7 +103,8 @@ type ProjectAuditPublisher interface { // ProjectAuditQuery reads one project-bound command-audit slice. Offset is a // host source offset, not a cross-domain event sequence. A nonzero BeforeOffset // must be paired with the CutoffOffset returned on the first page. PostgreSQL -// pages are served only while the installed generator and sequence prove a +// pages are served only while the installed generator, sequence, and all-writer +// database guards prove a // committed prefix; otherwise the host rejects the read instead of claiming a // fixed snapshot. See Coverage.CutoffConsistency. type ProjectAuditQuery struct {