Merge pull request #1233 from PerpetualSoftware/fix/bug-2810-nul-repair

fix(store,server,cli): count and repair the legacy NUL population (BUG-2810)

cmd/pad/cmd_db.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"database/sql"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -408,6 +409,23 @@ Steps:
 			}
 			defer dstStore.Close()
 
+			// PREFLIGHT: refuse BEFORE moving anything (DOC-2823 S3).
+			//
+			// The reason this is a preflight and not an error mid-copy is the
+			// shape of the failure it replaces. A legacy row carrying a NUL
+			// reaches PostgreSQL's jsonb parser during ImportWorkspace and
+			// fails there — at a point where earlier workspaces have already
+			// been written, so the operator is left with a half-moved
+			// database and an error naming a driver, not a cause. The scan is
+			// read-only and cheap next to the copy it guards.
+			//
+			// It does NOT repair. Dave's day-54 ruling: a migration that
+			// rewrites user content decides consent for the operator, so this
+			// prints the exact command that asks for it.
+			if err := preflightNULForMigration(srcStore, dstStore, fromPath); err != nil {
+				return err
+			}
+
 			// List workspaces from source
 			workspaces, err := srcStore.ListWorkspaces()
 			if err != nil {
@@ -482,3 +500,247 @@ func maskPassword(pgURL string) string {
 }
 
 // --- audit-log ---
+
+// preflightNULForMigration refuses a SQLite-to-PostgreSQL migration whose
+// source carries values PostgreSQL will not accept, listing them and naming the
+// repair command.
+//
+// Nothing has been written to the destination when this runs — it sits above
+// the workspace loop, which is the whole point: the failure it replaces
+// happened partway through the copy.
+//
+// TWO CHECKS, because our predicate alone cannot answer the question the
+// migration is actually asking (day-54 lead ruling on PR #1233).
+//
+// The first is the scan's violations: values every layer refuses. The second is
+// the SUSPECT class — pre-filter matches the predicate did not refuse. Most of
+// those are harmless doubled-backslash literals, but one shape in the set is
+// fatal here and invisible to every layer of ours: a NUL in a value shadowed by
+// a LITERAL duplicate key, which a map-model decode drops (textguard.KnownGaps,
+// which DOC-2823 forbids closing in a single layer).
+//
+// Dropping that class silently was the defect: the scan already HELD those rows
+// as candidates and threw them away, then promised the migration would go
+// through. So each suspect is cast on the DESTINATION connection —
+// `SELECT $1::jsonb`, side-effect-free, and the very cast an INSERT performs.
+// The database that is about to refuse the value is the oracle, which is exact
+// in both directions: no over-refusal on a literal, no miss on a shadowed one.
+func preflightNULForMigration(src *store.Store, dst *store.Store, fromPath string) error {
+	report, err := src.ScanNUL()
+	if err != nil {
+		return fmt.Errorf("NUL preflight: %w", err)
+	}
+	if !report.Applicable {
+		return nil
+	}
+
+	// THE TABLE FILTER COMES FIRST, before the destination is asked anything
+	// (codex round 10). The fail-closed rule refuses on a suspect that could
+	// not be verified, and running it over suspects from tables the migration
+	// never copies meant an unreadable row in `users` or `sessions` blocked a
+	// copy that would not have touched it — the same over-refusal round 9
+	// fixed for violations, reintroduced through the suspect path.
+	//
+	// Filtering here also stops the oracle making round trips about rows whose
+	// answer cannot matter.
+	migrated := store.MigratedTables()
+	var migratedSuspects []store.NULSuspect
+	var suspectsElsewhere []store.NULSuspect
+	for _, sus := range report.Suspects {
+		if migrated[sus.Table] {
+			migratedSuspects = append(migratedSuspects, sus)
+			continue
+		}
+		suspectsElsewhere = append(suspectsElsewhere, sus)
+	}
+
+	// COUNTED, NOT PROBED, and not silently dropped either (codex round 11).
+	// Whether one of these is actually fatal can only be answered by the
+	// destination, and asking would put them back inside the fail-closed rule
+	// this filter exists to keep them out of. So they are named, with the
+	// command that examines them properly — the alternative is a comment
+	// claiming they are reported while the code drops them, which is what the
+	// first version of this filter did.
+	if len(suspectsElsewhere) > 0 {
+		// NAMED, not just counted (codex round 12). The rows are already in
+		// hand; printing a bare number makes the operator run a second command
+		// to learn something this one could have told them.
+		fmt.Fprintf(os.Stderr,
+			"  NOTE: %d value(s) mentioning a NUL escape are in tables this migration does not copy.\n"+
+				"  They cannot block it and were not checked against the destination:\n",
+			len(suspectsElsewhere))
+		for _, sus := range suspectsElsewhere {
+			fmt.Fprintf(os.Stderr, "    %s\n", sus)
+		}
+	}
+
+	// A nil destination means the oracle is unavailable. That never happens on
+	// the real path — migrate-to-pg has connected to the target by the time
+	// this runs — but it must be SAID rather than skipped, because silently
+	// dropping the suspect class is the exact defect this check was added to
+	// correct.
+	var refusedSuspects []store.NULSuspect
+	var otherFailures []suspectFailure
+	if dst == nil {
+		if len(migratedSuspects) > 0 {
+			fmt.Fprintf(os.Stderr,
+				"  NOTE: %d suspect value(s) could not be checked — no destination to ask.\n",
+				len(migratedSuspects))
+		}
+	} else {
+		var unverified []suspectFailure
+		refusedSuspects, otherFailures, unverified = checkSuspectsAgainstDestination(src, dst, migratedSuspects)
+
+		// FAIL CLOSED. A suspect the destination never rendered a verdict on —
+		// a dropped connection, a timeout, a row that could not be read back —
+		// is not a pass. Letting it through would be the preflight promising a
+		// migration it did not check, which is the defect the suspect class was
+		// added to correct, arriving by a different route (codex round 5).
+		if len(unverified) > 0 {
+			fmt.Fprintf(os.Stderr,
+				"\nPreflight could not check %d suspect value(s) against the destination:\n\n",
+				len(unverified))
+			for _, f := range unverified {
+				fmt.Fprintf(os.Stderr, "  %s\n    %v\n", f.suspect, f.err)
+			}
+			fmt.Fprintln(os.Stderr,
+				"\nNothing has been migrated. These values may or may not be acceptable to the\n"+
+					"destination; the check did not complete, so this refuses rather than guessing.\n"+
+					"Re-run once the destination is reachable.")
+			return fmt.Errorf("%d suspect value(s) could not be checked; nothing was migrated", len(unverified))
+		}
+	}
+
+	// Cast failures for reasons OTHER than a NUL are reported and not refused
+	// on. They mean the destination will reject that row too, but a NUL
+	// preflight that silently grew into a general one would start refusing
+	// migrations that have nothing to do with this bug. Naming them beats
+	// discarding them, which is the mistake this whole check exists to correct.
+	for _, f := range otherFailures {
+		fmt.Fprintf(os.Stderr,
+			"  NOTE: %s was rejected by the destination for a non-NUL reason, which this preflight does "+
+				"not refuse on: %v\n", f.suspect, f.err)
+	}
+
+	// REFUSE only on rows the migration will actually copy. It reads six tables
+	// (store.MigratedTables); a NUL in users, platform settings, sessions or the
+	// oauth tables cannot break a copy that never touches them, and blocking on
+	// one would demand the operator rewrite content unrelated to the migration
+	// they asked for (codex round 9).
+	//
+	// The others are still REPORTED, below — as are the suspects from those
+	// tables, counted above. They are real, `pad db scan-nul` lists them, and
+	// staying silent about a broken row because this particular command does
+	// not care about it would be the information-discarding this preflight
+	// already had to be corrected for once.
+	var blocking []store.NULViolation
+	var elsewhere []store.NULViolation
+	for _, v := range report.Violations {
+		if migrated[v.Table] {
+			blocking = append(blocking, v)
+		} else {
+			elsewhere = append(elsewhere, v)
+		}
+	}
+	// refusedSuspects is already table-filtered: the oracle was only asked about
+	// migrated ones.
+	blockingSuspects := refusedSuspects
+
+	if n := len(elsewhere); n > 0 {
+		fmt.Fprintf(os.Stderr,
+			"\nNOTE: %d value(s) carrying a NUL are in tables this migration does not copy\n"+
+				"(users, platform settings, auth data). They do not block it, and\n"+
+				"'%s' will repair them:\n\n", n, repairNULCommandHint)
+		for _, v := range elsewhere {
+			fmt.Fprintf(os.Stderr, "  %s\n", v)
+		}
+	}
+
+	if len(blocking) == 0 && len(blockingSuspects) == 0 {
+		return nil
+	}
+
+	total := len(blocking) + len(blockingSuspects)
+	fmt.Fprintf(os.Stderr, "\nPreflight found %d stored value(s) in %s that PostgreSQL will not accept:\n\n",
+		total, fromPath)
+	for _, v := range blocking {
+		fmt.Fprintf(os.Stderr, "  %s\n", v)
+	}
+	for _, sus := range blockingSuspects {
+		// Named apart, because these were found by ASKING the destination
+		// rather than by our own predicate — an operator comparing this list
+		// against `pad db scan-nul`'s violations should be able to see why the
+		// two differ.
+		fmt.Fprintf(os.Stderr, "  %s (destination refused it; no layer of ours sees this one)\n", sus)
+	}
+	fmt.Fprintf(os.Stderr, "\nEach carries a NUL, which PostgreSQL refuses in a text or jsonb value —\n"+
+		"SQLSTATE 22021 and 22P05. Migrating risks failing partway through the copy, after\n"+
+		"some workspaces have already moved, so it is refused up front.\n\n"+
+		"Nothing has been migrated. Repair them first:\n\n    %s\n\n"+
+		"then re-run this command. To see the same list without migrating: pad db scan-nul\n",
+		repairNULCommandHint)
+
+	// `total`, not report.Total(). The first version returned the VIOLATION
+	// count here while the listing above showed violations plus refused
+	// suspects, so a preflight that refused one suspect and nothing else
+	// announced "0 stored value(s) carry a NUL; nothing was migrated" — a
+	// refusal whose own reason says there was nothing to refuse. Found by
+	// running the command against a real Postgres, not by a test: the tests
+	// asserted the message CONTAINED "nothing was migrated" and never read the
+	// number.
+	return fmt.Errorf("%d stored value(s) carry a NUL; nothing was migrated", total)
+}
+
+// suspectFailure pairs a suspect with the destination's complaint.
+type suspectFailure struct {
+	suspect store.NULSuspect
+	err     error
+}
+
+// checkSuspectsAgainstDestination asks the target database about each suspect.
+//
+// Returns the ones it refused for a NUL reason (which the preflight refuses on)
+// and the ones it refused for any other reason (which it reports).
+// THREE outcomes, not two, and the third is the one codex round 5 found missing:
+//
+//   - refused    — the destination answered, with a NUL code. The preflight
+//     refuses on these.
+//   - other      — the destination answered, with some other complaint about
+//     the value. Reported, not refused on: a NUL preflight that quietly grew
+//     into a general one would block migrations unrelated to this bug.
+//   - unverified — the destination did not answer, or the value could not be
+//     read back. The caller refuses on these, because an unchecked suspect
+//     treated as a pass is exactly what this whole check exists to stop.
+func checkSuspectsAgainstDestination(
+	src *store.Store, dst *store.Store, suspects []store.NULSuspect,
+) (refused []store.NULSuspect, other []suspectFailure, unverified []suspectFailure) {
+	for _, sus := range suspects {
+		if sus.KeyIncomplete {
+			unverified = append(unverified, suspectFailure{sus,
+				fmt.Errorf("row has a NULL key column, so its value cannot be read back")})
+			continue
+		}
+		value, rerr := src.ReadNULTargetValue(sus.Table, sus.Column, sus.Key)
+		if rerr != nil {
+			// Including "the row no longer exists". The scan and this check are
+			// separate statements, so a row can legitimately vanish between
+			// them — but a row that vanished is also a row whose value nobody
+			// verified, and re-running the preflight costs nothing next to a
+			// half-finished migration.
+			unverified = append(unverified, suspectFailure{sus, rerr})
+			continue
+		}
+		cerr := dst.CheckJSONBAcceptable(value)
+		switch {
+		case cerr == nil:
+			// The common case: a harmless literal the destination accepts.
+		case errors.Is(cerr, store.ErrNULDestinationRefused):
+			refused = append(refused, sus)
+		case errors.Is(cerr, store.ErrDestinationCheckUnavailable):
+			unverified = append(unverified, suspectFailure{sus, cerr})
+		default:
+			other = append(other, suspectFailure{sus, cerr})
+		}
+	}
+	return refused, other, unverified
+}

cmd/pad/cmd_db_nul.go

@@ -0,0 +1,423 @@
+package main
+
+import (
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"sort"
+
+	"github.com/spf13/cobra"
+
+	"github.com/PerpetualSoftware/pad/internal/cli"
+	"github.com/PerpetualSoftware/pad/internal/config"
+	"github.com/PerpetualSoftware/pad/internal/store"
+)
+
+// The operator-facing half of DOC-2823 S3 (BUG-2810): find the legacy rows the
+// two enforcement layers cannot retroactively fix, and repair them on an
+// explicit instruction.
+//
+// TWO SIBLING COMMANDS RATHER THAN `repair --nul`, which is how DOC-2823 first
+// spelled it. A `repair` verb that errors when given no flag is a worse shape
+// than two honest siblings, and there is no second kind of repair for it to
+// share a namespace with today. `scan-nul` also matches `migrate-to-pg`, the
+// existing hyphenated compound in this group.
+//
+// `scan-nul` IS the dry run, so `repair-nul` grows no --dry-run of its own.
+
+// repairNULCommandHint is the exact command the migrate-to-pg preflight prints
+// and this command's own help names. It is the STORE's constant rather than a
+// second spelling of the same words: the server quotes that same string in the
+// import's strict refusal, and a remedy naming a command that has been renamed
+// is worse than no remedy at all.
+//
+// TestRepairNULHintNamesARealCommand pins it against the cobra command tree, so
+// a rename that misses one of the three call sites fails a test rather than an
+// operator.
+const repairNULCommandHint = store.RepairNULCommand
+
+func dbScanNULCmd() *cobra.Command {
+	var fromPath string
+
+	cmd := &cobra.Command{
+		Use:   "scan-nul",
+		Short: "Report stored values carrying a NUL (read-only)",
+		Long: `Counts and locates every stored value that violates Pad's NUL invariant:
+a real NUL byte in any protected column, or a JSON escape in a JSON column
+that a JSON parser would decode to one.
+
+Such rows can only have been written by a binary older than the enforcement
+that now refuses them. They are not cosmetic: their workspace exports fine and
+re-imports with a 400, and 'pad db migrate-to-pg' fails partway through the
+copy against PostgreSQL's jsonb parser.
+
+The scan itself only reads. Opening the database does apply any pending schema
+migrations, exactly as starting the server does — pass --from to inspect a
+backup file instead of the live database.
+
+Nothing is repaired. Run '` + repairNULCommandHint + `' for that.`,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			proceed, err := resolveNULToolsTarget(&fromPath)
+			if err != nil {
+				return err
+			}
+			if !proceed {
+				return nil // Postgres: reported and nothing to do.
+			}
+			s, err := openNULToolsStore(fromPath)
+			if err != nil {
+				return err
+			}
+			defer s.Close()
+
+			report, err := s.ScanNUL()
+			if err != nil {
+				return fmt.Errorf("scan: %w", err)
+			}
+			printNULScanReport(os.Stdout, report, fromPath)
+			return nil
+		},
+	}
+
+	cmd.Flags().StringVar(&fromPath, "from", "", "SQLite database path (default: server-resolved — PAD_DB_PATH > PAD_DATA_DIR/pad.db > ~/.pad/pad.db)")
+	return cmd
+}
+
+func dbRepairNULCmd() *cobra.Command {
+	var fromPath string
+	var force bool
+
+	cmd := &cobra.Command{
+		Use:   "repair-nul",
+		Short: "Replace stored NULs with U+FFFD (rewrites user content)",
+		Long: `Rewrites every stored value 'pad db scan-nul' reports, replacing each NUL
+with U+FFFD (the Unicode replacement character) and leaving the rest of the
+value byte for byte as it was.
+
+THIS CHANGES USER CONTENT. It is a separate command, and never part of a
+migration, for that reason: a migration that rewrote stored text would decide
+on the operator's behalf. Every value it changes is listed.
+
+Run 'pad db scan-nul' first to see what would change — that is the dry run, so
+this command has no --dry-run of its own. Running it twice is safe: the second
+pass finds nothing to do.
+
+A row whose PRIMARY KEY carries the NUL is reported and left alone, because
+repairing it would change the row's identity and could collide with another
+row.`,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			proceed, err := resolveNULToolsTarget(&fromPath)
+			if err != nil {
+				return err
+			}
+			if !proceed {
+				return nil
+			}
+
+			// Refuse while the server is up, on the same reasoning as
+			// 'pad db restore': the report is a claim about a database, and a
+			// database somebody else is concurrently writing makes it a claim
+			// about a moment that has passed.
+			//
+			// The check is on the RESOLVED PATH, not on whether --from was
+			// given. Skipping it whenever --from was passed made the guard
+			// opt-out by accident: `--from` pointing at the live database —
+			// which is exactly what an operator copying the path out of
+			// `pad db scan-nul`'s output would type — repaired underneath a
+			// running server with no warning. A --from that names an unrelated
+			// backup is still unguarded, and should be: nothing is writing it.
+			if !force {
+				if err := refuseIfServerOwns(fromPath); err != nil {
+					return err
+				}
+			}
+
+			s, err := openNULToolsStore(fromPath)
+			if err != nil {
+				return err
+			}
+			defer s.Close()
+
+			// Show the operator what is about to change BEFORE asking. A
+			// confirmation prompt for an unnamed set of rows is not consent.
+			scan, err := s.ScanNUL()
+			if err != nil {
+				return fmt.Errorf("scan: %w", err)
+			}
+			printNULScanReport(os.Stdout, scan, fromPath)
+			if scan.Total() == 0 && len(scan.Suspects) == 0 {
+				return nil
+			}
+
+			if !force {
+				fmt.Fprintf(os.Stderr, "\nThis will rewrite the %d value(s) above, replacing each NUL with "+
+					"U+FFFD, and inspect %d suspect value(s) — rewriting only those that hide a NUL behind "+
+					"a repeated key.\n", scan.Total(), len(scan.Suspects))
+				fmt.Fprintf(os.Stderr, "Run with --force to skip this confirmation, or press Ctrl+C to abort.\n")
+				fmt.Fprintf(os.Stderr, "Continue? [y/N] ")
+				var confirm string
+				fmt.Scanln(&confirm)
+				if confirm != "y" && confirm != "Y" {
+					fmt.Fprintln(os.Stderr, "Aborted. Nothing was changed.")
+					return nil
+				}
+			}
+
+			report, err := s.RepairNUL()
+			if err != nil {
+				return fmt.Errorf("repair: %w", err)
+			}
+
+			// Only printed when there is something to say. A bare
+			// "Repaired 0 value(s)." above a suspect section that DID repair
+			// something reads as a contradiction.
+			if n := len(report.Repaired); n > 0 {
+				fmt.Fprintf(os.Stdout, "\nRepaired %d value(s).\n", n)
+				for _, v := range report.Repaired {
+					fmt.Fprintf(os.Stdout, "  %s\n", v)
+				}
+			}
+			if n := len(report.SuspectsRepaired); n > 0 {
+				fmt.Fprintf(os.Stdout, "\nRepaired %d value(s) from the suspect list — "+
+					"a NUL hidden behind a repeated JSON key:\n", n)
+				for _, sus := range report.SuspectsRepaired {
+					fmt.Fprintf(os.Stdout, "  %s\n", sus)
+				}
+			}
+			if n := len(report.SuspectsClean); n > 0 {
+				fmt.Fprintf(os.Stdout, "\n%d suspect value(s) needed nothing — they mention the escape "+
+					"without using it.\n", n)
+			}
+			if n := len(report.SuspectsSkipped); n > 0 {
+				fmt.Fprintf(os.Stdout, "\n%d suspect value(s) could not be addressed (NULL key column).\n", n)
+			}
+			if len(report.Skipped) > 0 {
+				fmt.Fprintf(os.Stdout, "\nSkipped %d value(s):\n", len(report.Skipped))
+				for _, sk := range report.Skipped {
+					fmt.Fprintf(os.Stdout, "  %s\n    %s\n", sk.Violation, sk.Reason)
+				}
+			}
+			if n := len(report.SuspectsFailed); n > 0 {
+				fmt.Fprintf(os.Stdout, "\nFailed on %d suspect value(s):\n", n)
+				for _, f := range report.SuspectsFailed {
+					fmt.Fprintf(os.Stdout, "  %s\n    %v\n", f.Suspect, f.Err)
+				}
+			}
+			if len(report.Failed) > 0 {
+				fmt.Fprintf(os.Stdout, "\nFailed on %d value(s):\n", len(report.Failed))
+				for _, f := range report.Failed {
+					fmt.Fprintf(os.Stdout, "  %s\n    %v\n", f.Violation, f.Err)
+				}
+			}
+			return nulRepairExitError(report)
+		},
+	}
+
+	cmd.Flags().StringVar(&fromPath, "from", "", "SQLite database path (default: server-resolved)")
+	cmd.Flags().BoolVar(&force, "force", false, "skip the confirmation prompt and the running-server check")
+	return cmd
+}
+
+// resolveNULToolsTarget works out which database file the two commands act on,
+// WITHOUT opening it, and reports whether there is anything to do.
+//
+// Resolution is separated from opening because opening is not free of
+// consequence: store.New runs any pending schema migrations, exactly as
+// starting the server does. The repair must be able to REFUSE — because the
+// server is running and owns this file — before that happens, and an earlier
+// version opened first and checked second, which made the refusal arrive after
+// the thing it was protecting against.
+//
+// It writes the resolved path back through fromPath so the caller can report
+// which database it looked at. A report that does not name its subject is the
+// kind an operator can act on against the wrong instance.
+func resolveNULToolsTarget(fromPath *string) (proceed bool, err error) {
+	if *fromPath == "" {
+		// PAD_DB_DRIVER ALONE decides, and PAD_DATABASE_URL deliberately does
+		// not (codex round 9). cmd_server.go opens PostgreSQL only when
+		// PAD_DB_DRIVER=postgres; PAD_DATABASE_URL is also migrate-to-pg's
+		// TARGET, and its default at that. Treating the URL as proof of a
+		// PostgreSQL deployment broke the exact flow this unit prescribes: the
+		// preflight refuses, tells the operator to run `pad db repair-nul`,
+		// and — with the target URL still exported in their shell — that
+		// command announced there was nothing to repair and exited 0.
+		if os.Getenv("PAD_DB_DRIVER") == "postgres" {
+			fmt.Fprintln(os.Stderr,
+				"This deployment is PostgreSQL, which refuses these values natively (SQLSTATE 22021 for a\n"+
+					"NUL in text, 22P05 for the escape reaching jsonb), so no stored row can carry one.\n"+
+					"Nothing to scan or repair.")
+			return false, nil
+		}
+		resolved, rerr := resolveSQLiteDBPath()
+		if rerr != nil {
+			return false, rerr
+		}
+		*fromPath = resolved
+	}
+	if _, serr := os.Stat(*fromPath); os.IsNotExist(serr) {
+		return false, fmt.Errorf("SQLite database not found: %s", *fromPath)
+	}
+	return true, nil
+}
+
+// openNULToolsStore opens the resolved database. Opening applies any pending
+// schema migrations, which is why the caller does its refusing first.
+func openNULToolsStore(path string) (*store.Store, error) {
+	s, err := store.New(path)
+	if err != nil {
+		return nil, fmt.Errorf("open SQLite: %w", err)
+	}
+	return s, nil
+}
+
+// printNULScanReport renders a scan for a human.
+//
+// ON STDOUT, unlike the rest of this command group. `pad db backup` and
+// `pad db restore` keep their progress on stderr because stdout may carry the
+// backup itself; these two commands emit no data at all, and their REPORT is
+// the whole point — an operator piping `pad db scan-nul > affected.txt` should
+// get the list, not an empty file. Only the confirmation prompt stays on
+// stderr, where a prompt belongs.
+func printNULScanReport(w io.Writer, report *store.NULScanReport, dbPath string) {
+	if !report.Applicable {
+		fmt.Fprintf(w, "Not applicable: %s.\n", report.Reason)
+		return
+	}
+
+	fmt.Fprintf(w, "Scanned %d protected column(s) in %s.\n", report.ColumnsScanned, dbPath)
+	if len(report.ColumnsAbsent) > 0 {
+		// Not an error — an older schema legitimately lacks later columns —
+		// but a census that quietly skipped part of its population is not one.
+		fmt.Fprintf(w, "  (%d listed column(s) absent from this schema: %v)\n",
+			len(report.ColumnsAbsent), report.ColumnsAbsent)
+	}
+
+	if report.Total() == 0 {
+		fmt.Fprintln(w, "No values carrying a NUL were found.")
+		printNULSuspects(w, report)
+		return
+	}
+
+	fmt.Fprintf(w, "\nFound %d value(s) carrying a NUL:\n\n", report.Total())
+
+	byColumn := report.ByColumn()
+	for _, key := range sortedCountKeys(byColumn) {
+		fmt.Fprintf(w, "  %-44s %d\n", key, byColumn[key])
+	}
+
+	byWorkspace := report.ByWorkspace()
+	fmt.Fprintln(w, "\nBy workspace:")
+	for _, id := range sortedCountKeys(byWorkspace) {
+		label := id
+		if label == "" {
+			// Sixteen of the protected tables carry no workspace_id — users,
+			// sessions, the oauth tables, platform settings. Saying so beats
+			// an empty column.
+			label = "(instance-wide tables, no workspace)"
+		}
+		fmt.Fprintf(w, "  %-44s %d\n", label, byWorkspace[id])
+	}
+
+	fmt.Fprintln(w, "\nRows:")
+	for _, v := range report.Violations {
+		fmt.Fprintf(w, "  %s\n", v)
+	}
+
+	printNULSuspects(w, report)
+}
+
+// printNULSuspects renders the suspect class under its own heading.
+//
+// SEPARATE FROM THE VIOLATIONS, deliberately. These are not values Pad refuses
+// — most are ordinary text that merely contains the escape's leading characters
+// — so listing them among the violations would tell an operator their database
+// is more broken than it is. But one shape in the set is fatal to a PostgreSQL
+// migration and invisible to every check Pad makes, and the honest thing is to
+// say so rather than to drop the whole class silently (day-54 lead ruling on
+// PR #1233).
+//
+// The heading names the resolution rather than leaving the operator to guess:
+// migrate-to-pg decides these by asking the destination, and repair-nul fixes
+// the fatal shape without touching the harmless ones.
+func printNULSuspects(w io.Writer, report *store.NULScanReport) {
+	if len(report.Suspects) == 0 {
+		return
+	}
+	fmt.Fprintf(w, "\nAlso found %d value(s) that MENTION a NUL escape without carrying one:\n\n",
+		len(report.Suspects))
+	for _, sus := range report.Suspects {
+		fmt.Fprintf(w, "  %s\n", sus)
+	}
+	fmt.Fprintln(w, "\n  These are almost always harmless — text that writes about the escape rather than")
+	fmt.Fprintln(w, "  using it. They are listed because ONE shape in this set is not: a NUL hidden behind")
+	fmt.Fprintln(w, "  a repeated JSON key, which PostgreSQL refuses and no check here can see.")
+	fmt.Fprintln(w, "  'pad db migrate-to-pg' resolves each one by asking the destination database, and")
+	fmt.Fprintln(w, "  '"+repairNULCommandHint+"' fixes the fatal shape while leaving the rest alone.")
+}
+
+func sortedCountKeys(m map[string]int) []string {
+	out := make([]string, 0, len(m))
+	for k := range m {
+		out = append(out, k)
+	}
+	sort.Strings(out)
+	return out
+}
+
+// refuseIfServerOwns errors when dbPath is the database a running server is
+// serving.
+//
+// Paths are compared after EvalSymlinks and Abs, because "the same file"
+// reached by two spellings is the case the comparison exists for — a symlinked
+// data directory, or a relative --from typed from a different working
+// directory. A path that cannot be resolved falls back to its cleaned absolute
+// form rather than being treated as different, so the guard errs toward
+// refusing.
+func refuseIfServerOwns(dbPath string) error {
+	cfg, err := config.Load()
+	if err != nil {
+		return fmt.Errorf("load config: %w", err)
+	}
+	if !cli.IsServerRunning(cfg) {
+		return nil
+	}
+	if !sameFilePath(dbPath, cfg.DBPath) {
+		return nil
+	}
+	return fmt.Errorf("the Pad server appears to be running at %s:%d and is serving %s — stop it first "+
+		"('pad server stop') so the rows cannot change under the repair, or re-run with --force to override",
+		cfg.Host, cfg.Port, cfg.DBPath)
+}
+
+// sameFilePath reports whether two paths name the same file.
+func sameFilePath(a, b string) bool {
+	return resolvePathForCompare(a) == resolvePathForCompare(b)
+}
+
+func resolvePathForCompare(p string) string {
+	if abs, err := filepath.Abs(p); err == nil {
+		p = abs
+	}
+	if real, err := filepath.EvalSymlinks(p); err == nil {
+		return real
+	}
+	return filepath.Clean(p)
+}
+
+// nulRepairExitError turns a repair report into the command's exit status.
+//
+// BOTH failure buckets count. The first version printed suspect failures and
+// then returned nil, so a repair that left data unrepaired exited 0 — invisible
+// to any script, and to an operator who trusts the status (codex round 5).
+//
+// Extracted so the decision is testable without a database: the bug was in the
+// decision, not in the repair, and a test that needed a fixture to reach it is
+// a test nobody writes.
+func nulRepairExitError(report *store.NULRepairReport) error {
+	n := len(report.Failed) + len(report.SuspectsFailed)
+	if n == 0 {
+		return nil
+	}
+	return fmt.Errorf("%d value(s) could not be repaired", n)
+}

cmd/pad/cmd_db_nul_test.go

@@ -0,0 +1,655 @@
+package main
+
+import (
+	"database/sql"
+	"errors"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+	"github.com/PerpetualSoftware/pad/internal/store"
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// TestRepairNULHintNamesARealCommand is the guard the hint's own comment
+// promises.
+//
+// Three surfaces quote this command at an operator — the migrate-to-pg
+// preflight, the workspace import's strict refusal, and scan-nul's help — and
+// each of them is a claim that typing it does something. Walking the real cobra
+// tree is what makes a rename fail here rather than in front of a user, which
+// is the difference between a cited convention and a consulted one.
+func TestRepairNULHintNamesARealCommand(t *testing.T) {
+	root := newRootCmd()
+
+	// The hint is a full command line ("pad db repair-nul"); resolve it as a
+	// path through the tree rather than by string comparison against a second
+	// spelling, which would only prove two constants agree.
+	fields := strings.Fields(repairNULCommandHint)
+	if len(fields) < 2 || fields[0] != "pad" {
+		t.Fatalf("the hint is not a 'pad ...' command line: %q", repairNULCommandHint)
+	}
+	cmd, _, err := root.Find(fields[1:])
+	if err != nil {
+		t.Fatalf("the hint names a command that does not exist: %q (%v)", repairNULCommandHint, err)
+	}
+	// Find falls back to the closest ancestor rather than failing, so the
+	// resolved command must actually BE the leaf named — otherwise "pad db
+	// repair-nonsense" resolves to "db" and passes.
+	if cmd.Name() != fields[len(fields)-1] {
+		t.Fatalf("the hint %q resolves to %q, not to a command of its own name",
+			repairNULCommandHint, cmd.CommandPath())
+	}
+	if cmd.RunE == nil && cmd.Run == nil {
+		t.Errorf("%q exists but does nothing when run", repairNULCommandHint)
+	}
+
+	// And the store's constant — which the SERVER quotes in the import
+	// refusal — is the same string, so all three surfaces move together.
+	if repairNULCommandHint != store.RepairNULCommand {
+		t.Errorf("the CLI hint (%q) and the string the server quotes (%q) have drifted apart",
+			repairNULCommandHint, store.RepairNULCommand)
+	}
+}
+
+// TestMigrateToPgPreflightRefusesAndNamesTheRepair covers the preflight's whole
+// contract: it refuses, it names the rows, it names the command, and — the part
+// that matters most — it does so BEFORE anything has moved.
+func TestMigrateToPgPreflightRefusesAndNamesTheRepair(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "preflight.db")
+	s, err := store.New(dbPath)
+	if err != nil {
+		t.Fatalf("open store: %v", err)
+	}
+	defer s.Close()
+
+	ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Preflight"})
+	if err != nil {
+		t.Fatalf("create workspace: %v", err)
+	}
+
+	// CONTROL FIRST: a clean database passes the preflight. Without this, a
+	// preflight that refused everything would satisfy the assertion below.
+	//
+	// A nil destination is the "no oracle" path: this leg is about the
+	// VIOLATION half, which needs no Postgres, and the suspect half has its own
+	// test that does. The function says so in its output rather than skipping
+	// silently.
+	if err := preflightNULForMigration(s, nil, dbPath); err != nil {
+		t.Fatalf("preflight refused a clean database: %v", err)
+	}
+
+	plantNULInWorkspaceName(t, dbPath, ws.ID, "bad"+textguard.NUL+"name")
+
+	err = preflightNULForMigration(s, nil, dbPath)
+	if err == nil {
+		t.Fatal("the preflight accepted a database carrying a value PostgreSQL will refuse — the migration " +
+			"would fail partway through the copy, which is the failure this replaces")
+	}
+	if !strings.Contains(err.Error(), "nothing was migrated") {
+		t.Errorf("the refusal does not say the migration did not start: %v", err)
+	}
+}
+
+// TestScanAndRepairAgreeThroughTheCommandPath drives the store API the two
+// commands call, so the CLI's promise — scan-nul is the dry run for
+// repair-nul — is measured rather than asserted in help text.
+func TestScanAndRepairAgreeThroughTheCommandPath(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "scanrepair.db")
+	s, err := store.New(dbPath)
+	if err != nil {
+		t.Fatalf("open store: %v", err)
+	}
+	defer s.Close()
+
+	ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "ScanRepair"})
+	if err != nil {
+		t.Fatalf("create workspace: %v", err)
+	}
+	plantNULInWorkspaceName(t, dbPath, ws.ID, "bad"+textguard.NUL+"name")
+
+	scan, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan: %v", err)
+	}
+	if scan.Total() != 1 {
+		t.Fatalf("scan found %d violations, want 1: %v", scan.Total(), scan.Violations)
+	}
+
+	report, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+	// The dry run's count and the repair's count are the same number, which is
+	// the entire reason scan-nul is offered instead of a --dry-run flag.
+	if len(report.Repaired) != scan.Total() {
+		t.Errorf("scan promised %d change(s), repair made %d", scan.Total(), len(report.Repaired))
+	}
+
+	var name string
+	if err := s.DB().QueryRow(`SELECT name FROM workspaces WHERE id = ?`, ws.ID).Scan(&name); err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if want := "bad" + textguard.Replacement + "name"; name != want {
+		t.Errorf("repaired name = %q, want %q", name, want)
+	}
+}
+
+// plantNULInWorkspaceName writes the legacy state through a raw handle with the
+// NUL triggers dropped — which is what a pre-enforcement binary was.
+func plantNULInWorkspaceName(t *testing.T, dbPath, wsID, value string) {
+	t.Helper()
+
+	raw, err := sql.Open("sqlite", dbPath+"?_pragma=busy_timeout(30000)")
+	if err != nil {
+		t.Fatalf("open raw: %v", err)
+	}
+	defer raw.Close()
+
+	rows, err := raw.Query(
+		`SELECT name FROM sqlite_master WHERE type = 'trigger' AND name GLOB 'pad_nul_workspaces_name_*'`)
+	if err != nil {
+		t.Fatalf("list triggers: %v", err)
+	}
+	var names []string
+	for rows.Next() {
+		var n string
+		if err := rows.Scan(&n); err != nil {
+			rows.Close()
+			t.Fatalf("scan: %v", err)
+		}
+		names = append(names, n)
+	}
+	rows.Close()
+	if len(names) == 0 {
+		t.Fatal("no workspaces.name triggers found — the fixture would plant nothing and the test " +
+			"would pass for the wrong reason")
+	}
+	for _, n := range names {
+		if _, err := raw.Exec(`DROP TRIGGER IF EXISTS "` + n + `"`); err != nil {
+			t.Fatalf("drop %s: %v", n, err)
+		}
+	}
+	if _, err := raw.Exec(`UPDATE workspaces SET name = ? WHERE id = ?`, value, wsID); err != nil {
+		t.Fatalf("plant: %v", err)
+	}
+}
+
+// TestSameFilePathIdentifiesTheServersDatabase covers the comparison the
+// running-server guard is built on.
+//
+// The guard used to be skipped whenever --from was given, which made it
+// opt-out by accident: the most natural --from an operator types is the path
+// `pad db scan-nul` just printed, which IS the live database. The fix compares
+// resolved paths, so the cases that matter are the ones where two spellings
+// name one file.
+func TestSameFilePathIdentifiesTheServersDatabase(t *testing.T) {
+	dir := t.TempDir()
+	live := filepath.Join(dir, "pad.db")
+	if err := os.WriteFile(live, []byte("x"), 0o600); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	other := filepath.Join(dir, "backup.db")
+	if err := os.WriteFile(other, []byte("x"), 0o600); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	link := filepath.Join(dir, "linked.db")
+	if err := os.Symlink(live, link); err != nil {
+		t.Skipf("symlinks unavailable: %v", err)
+	}
+
+	cases := []struct {
+		name string
+		a, b string
+		want bool
+		why  string
+	}{
+		{"identical paths", live, live, true, "the plain case: --from naming the live database."},
+		{"a symlink to it", link, live, true,
+			"a symlinked data directory is the shape where two spellings name one file, and the one a " +
+				"string comparison misses."},
+		{"an unrelated file", other, live, false,
+			"the control. A guard that answered true for everything would also pass every case above, " +
+				"and would refuse repairing a backup for no reason."},
+		{"a path with redundant segments", filepath.Join(dir, ".", "pad.db"), live, true,
+			"Clean/Abs normalisation, so a path typed from a different working directory still matches."},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := sameFilePath(tc.a, tc.b); got != tc.want {
+				t.Errorf("sameFilePath(%q, %q) = %v, want %v — %s", tc.a, tc.b, got, tc.want, tc.why)
+			}
+		})
+	}
+}
+
+// TestPreflightAsksTheDestinationAboutSuspects is the day-54 ruling's whole
+// point, end to end: the preflight refuses a row NO CHECK IN PAD CAN SEE,
+// because it asks the database that is about to reject it.
+//
+// Both legs matter and they are opposites. The literal-only leg is the
+// over-refusal control — a preflight that refused every suspect would block
+// migrations over prose that merely writes about this bug, and would pass the
+// refusal leg while doing it.
+func TestPreflightAsksTheDestinationAboutSuspects(t *testing.T) {
+	dsn := os.Getenv("PAD_TEST_POSTGRES_URL")
+	if dsn == "" {
+		t.Skip("the preflight's oracle needs a real PostgreSQL destination (set PAD_TEST_POSTGRES_URL)")
+	}
+
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+
+	newSource := func(t *testing.T, blob string) (*store.Store, string) {
+		t.Helper()
+		dbPath := filepath.Join(t.TempDir(), "src.db")
+		s, err := store.New(dbPath)
+		if err != nil {
+			t.Fatalf("open source: %v", err)
+		}
+		t.Cleanup(func() { s.Close() })
+
+		ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Suspect"})
+		if err != nil {
+			t.Fatalf("create workspace: %v", err)
+		}
+		plantWorkspaceSettings(t, dbPath, ws.ID, blob)
+		return s, dbPath
+	}
+
+	dst, err := store.NewPostgres(dsn)
+	if err != nil {
+		t.Fatalf("open destination: %v", err)
+	}
+	defer dst.Close()
+
+	t.Run("a NUL behind a repeated key refuses the migration", func(t *testing.T) {
+		src, path := newSource(t, `{"a":"`+esc+`","a":"clean"}`)
+
+		// The premise, asserted so this cannot quietly become a case we catch
+		// ourselves: our own scan finds NO violation here.
+		scan, err := src.ScanNUL()
+		if err != nil {
+			t.Fatalf("scan: %v", err)
+		}
+		if scan.Total() != 0 {
+			t.Fatalf("the scan now reports this as a violation, so the oracle is not what refuses it: %v",
+				scan.Violations)
+		}
+		if len(scan.Suspects) != 1 {
+			t.Fatalf("expected exactly one suspect, got %d", len(scan.Suspects))
+		}
+
+		err = preflightNULForMigration(src, dst, path)
+		if err == nil {
+			t.Fatal("the preflight accepted a value PostgreSQL refuses — this is the row the ruling " +
+				"exists for, and it is invisible to every check Pad makes")
+		}
+		if !strings.Contains(err.Error(), "nothing was migrated") {
+			t.Errorf("the refusal does not say the migration did not start: %v", err)
+		}
+		// THE COUNT, not just the phrase. The first version of this assertion
+		// read only the phrase, and the message shipped saying "0 stored
+		// value(s) carry a NUL; nothing was migrated" — a refusal whose reason
+		// says there was nothing to refuse, because it counted violations while
+		// the listing counted violations plus refused suspects.
+		if !strings.HasPrefix(err.Error(), "1 stored value") {
+			t.Errorf("the refusal miscounts what it refused on: %v", err)
+		}
+	})
+
+	t.Run("a harmless literal does NOT refuse the migration", func(t *testing.T) {
+		src, path := newSource(t, `{"note":"x`+backslash+esc+`y"}`)
+
+		scan, err := src.ScanNUL()
+		if err != nil {
+			t.Fatalf("scan: %v", err)
+		}
+		if len(scan.Suspects) != 1 {
+			t.Fatalf("expected the literal to be a suspect, got %d", len(scan.Suspects))
+		}
+
+		if err := preflightNULForMigration(src, dst, path); err != nil {
+			t.Fatalf("the preflight refused a value PostgreSQL accepts, so every suspect would block a "+
+				"migration: %v", err)
+		}
+	})
+}
+
+// plantWorkspaceSettings writes a settings blob through a raw handle with the
+// relevant triggers dropped — the pre-enforcement binary again.
+func plantWorkspaceSettings(t *testing.T, dbPath, wsID, blob string) {
+	t.Helper()
+
+	raw, err := sql.Open("sqlite", dbPath+"?_pragma=busy_timeout(30000)")
+	if err != nil {
+		t.Fatalf("open raw: %v", err)
+	}
+	defer raw.Close()
+
+	rows, err := raw.Query(
+		`SELECT name FROM sqlite_master WHERE type='trigger' AND name GLOB 'pad_nul_workspaces_settings_*'`)
+	if err != nil {
+		t.Fatalf("list triggers: %v", err)
+	}
+	var names []string
+	for rows.Next() {
+		var n string
+		if err := rows.Scan(&n); err != nil {
+			rows.Close()
+			t.Fatalf("scan: %v", err)
+		}
+		names = append(names, n)
+	}
+	rows.Close()
+	if len(names) == 0 {
+		t.Fatal("no workspaces.settings triggers found; the fixture would prove nothing")
+	}
+	for _, n := range names {
+		if _, err := raw.Exec(`DROP TRIGGER IF EXISTS "` + n + `"`); err != nil {
+			t.Fatalf("drop %s: %v", n, err)
+		}
+	}
+	if _, err := raw.Exec(`UPDATE workspaces SET settings = ? WHERE id = ?`, blob, wsID); err != nil {
+		t.Fatalf("plant: %v", err)
+	}
+}
+
+// TestRepairExitStatusCountsBothFailureBuckets pins the exit code.
+//
+// A repair that leaves data unrepaired and exits 0 is invisible: a script sees
+// success, and an operator who trusts the status moves on. The first version
+// checked only the violation bucket, so a failed SUSPECT repair — the shape
+// that needs the most attention, since no other check sees those values —
+// exited cleanly (codex round 5).
+func TestRepairExitStatusCountsBothFailureBuckets(t *testing.T) {
+	boom := errors.New("nope")
+
+	cases := []struct {
+		name    string
+		report  store.NULRepairReport
+		wantErr bool
+		why     string
+	}{
+		{
+			name:    "nothing failed",
+			report:  store.NULRepairReport{Repaired: []store.NULViolation{{Table: "items"}}},
+			wantErr: false,
+			why:     "the control: a clean run must not report failure.",
+		},
+		{
+			name: "a violation failed",
+			report: store.NULRepairReport{
+				Failed: []store.NULRepairFailure{{Err: boom}},
+			},
+			wantErr: true,
+			why:     "the case the first version did catch.",
+		},
+		{
+			name: "only a SUSPECT failed",
+			report: store.NULRepairReport{
+				SuspectsFailed: []store.NULSuspectFailure{{Err: boom}},
+			},
+			wantErr: true,
+			why:     "the case it did not. These are the values no other check in Pad can see.",
+		},
+		{
+			name: "skips are not failures",
+			report: store.NULRepairReport{
+				Skipped:       []store.NULRepairSkip{{Reason: "primary key"}},
+				SuspectsClean: []store.NULSuspect{{Table: "items"}},
+			},
+			wantErr: false,
+			why: "a deliberate skip is a reported outcome, not an error; exiting non-zero on it would " +
+				"make every run with an email_optouts row look broken.",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := nulRepairExitError(&tc.report)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("%s\n  got err=%v, want error=%v", tc.why, err, tc.wantErr)
+			}
+		})
+	}
+}
+
+// TestPreflightIgnoresTablesTheMigrationDoesNotCopy is codex round 9's second
+// finding.
+//
+// `migrate-to-pg` copies workspace content and nothing else — its own help says
+// users, platform settings and auth data are not migrated. A NUL in one of
+// those tables therefore cannot break the copy, and refusing on it demanded the
+// operator rewrite content unrelated to the migration they asked for.
+//
+// The row is still REPORTED. Staying silent about a broken row because this
+// command does not care about it would be the same information-discarding this
+// preflight already had to be corrected for once.
+func TestPreflightIgnoresTablesTheMigrationDoesNotCopy(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "src.db")
+	s, err := store.New(dbPath)
+	if err != nil {
+		t.Fatalf("open source: %v", err)
+	}
+	defer s.Close()
+
+	// platform_settings.value is protected and is NOT one of the six tables
+	// ImportWorkspace writes.
+	plantPlatformSetting(t, dbPath, "branding", "site"+textguard.NUL+"name")
+
+	scan, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan: %v", err)
+	}
+	// The premise: the SCAN does see it. If it did not, this test would pass
+	// for the wrong reason.
+	if scan.Total() != 1 {
+		t.Fatalf("the scan should still report the row; got %d violations: %v", scan.Total(), scan.Violations)
+	}
+	if store.MigratedTables()["platform_settings"] {
+		t.Fatal("platform_settings is listed as migrated; pick a table the migration really skips")
+	}
+
+	if err := preflightNULForMigration(s, nil, dbPath); err != nil {
+		t.Errorf("the preflight blocked a migration over a table it does not copy: %v", err)
+	}
+
+	// CONTROL: the same value in a table the migration DOES copy must still
+	// refuse. Without this, a preflight that refused nothing would pass above.
+	ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "Blocking"})
+	if err != nil {
+		t.Fatalf("create workspace: %v", err)
+	}
+	plantNULInWorkspaceName(t, dbPath, ws.ID, "bad"+textguard.NUL+"name")
+
+	if err := preflightNULForMigration(s, nil, dbPath); err == nil {
+		t.Error("the preflight accepted a NUL in a table the migration DOES copy")
+	}
+}
+
+// plantPlatformSetting writes a settings row through a raw handle with the
+// relevant triggers dropped.
+func plantPlatformSetting(t *testing.T, dbPath, key, value string) {
+	t.Helper()
+
+	raw, err := sql.Open("sqlite", dbPath+"?_pragma=busy_timeout(30000)")
+	if err != nil {
+		t.Fatalf("open raw: %v", err)
+	}
+	defer raw.Close()
+
+	rows, err := raw.Query(
+		`SELECT name FROM sqlite_master WHERE type='trigger' AND name GLOB 'pad_nul_platform_settings_*'`)
+	if err != nil {
+		t.Fatalf("list triggers: %v", err)
+	}
+	var names []string
+	for rows.Next() {
+		var n string
+		if err := rows.Scan(&n); err != nil {
+			rows.Close()
+			t.Fatalf("scan: %v", err)
+		}
+		names = append(names, n)
+	}
+	rows.Close()
+	if len(names) == 0 {
+		t.Fatal("no platform_settings triggers found; the fixture would prove nothing")
+	}
+	for _, n := range names {
+		if _, err := raw.Exec(`DROP TRIGGER IF EXISTS "` + n + `"`); err != nil {
+			t.Fatalf("drop %s: %v", n, err)
+		}
+	}
+	if _, err := raw.Exec(
+		`INSERT INTO platform_settings (key, value, updated_at) VALUES (?, ?, datetime('now'))`,
+		key, value); err != nil {
+		t.Fatalf("plant: %v", err)
+	}
+}
+
+// TestPreflightDoesNotFailClosedOnUnmigratedSuspects is codex round 10, and it
+// is round 9's over-refusal reintroduced through the other path.
+//
+// The fail-closed rule refuses when a suspect cannot be VERIFIED. Applied
+// before the table filter, an unverifiable suspect in a table the migration
+// never copies blocked the copy.
+//
+// It needs a REAL DESTINATION: the fail-closed branch only runs once there is
+// something to ask, so a nil-destination fixture passes whether or not the
+// ordering is right. The first version of this test made exactly that mistake
+// and proved nothing.
+//
+// "Unverifiable" here is a NULL primary key — SQLite permits one in a declared
+// TEXT PRIMARY KEY, which no other engine does — so the row's value genuinely
+// cannot be read back to be cast.
+func TestPreflightDoesNotFailClosedOnUnmigratedSuspects(t *testing.T) {
+	dsn := os.Getenv("PAD_TEST_POSTGRES_URL")
+	if dsn == "" {
+		t.Skip("the fail-closed branch needs a real destination to be reachable at all")
+	}
+
+	dbPath := filepath.Join(t.TempDir(), "src.db")
+	s, err := store.New(dbPath)
+	if err != nil {
+		t.Fatalf("open source: %v", err)
+	}
+	defer s.Close()
+
+	dst, err := store.NewPostgres(dsn)
+	if err != nil {
+		t.Fatalf("open destination: %v", err)
+	}
+	defer dst.Close()
+
+	// activities.metadata is JSON-classed (so the escape makes it a SUSPECT
+	// rather than a violation) and its table is NOT migrated.
+	plantActivitySuspect(t, dbPath, `{"a":"`+textguard.EscNUL+`","a":"clean"}`)
+
+	scan, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan: %v", err)
+	}
+	if scan.Total() != 0 {
+		t.Fatalf("the fixture should be a suspect, not a violation: %v", scan.Violations)
+	}
+	// The premise, asserted so the test cannot pass because the fixture stopped
+	// being unverifiable: the scan sees the row and cannot address it.
+	if len(scan.Suspects) != 1 {
+		t.Fatalf("expected one suspect, got %d", len(scan.Suspects))
+	}
+	if !scan.Suspects[0].KeyIncomplete {
+		t.Fatalf("the fixture is addressable, so it would be verified rather than failing closed: %v",
+			scan.Suspects[0].Key)
+	}
+
+	// The preflight writes its notes to stderr; capture them so the advisory
+	// below is asserted rather than assumed.
+	stderr := os.Stderr
+	r, w, perr := os.Pipe()
+	if perr != nil {
+		t.Fatalf("pipe: %v", perr)
+	}
+	os.Stderr = w
+	err = preflightNULForMigration(s, dst, dbPath)
+	w.Close()
+	os.Stderr = stderr
+	out, _ := io.ReadAll(r)
+
+	if err != nil {
+		t.Errorf("the preflight failed closed over an unverifiable suspect in a table the migration "+
+			"does not copy: %v", err)
+	}
+
+	// AND it says so. Excluding the row from the destination probe must not
+	// turn into dropping it from the output: a comment that claims these are
+	// reported while the code goes quiet is exactly what the first version of
+	// this filter shipped (codex round 11).
+	if !strings.Contains(string(out), "does not copy") {
+		t.Errorf("the suspect was filtered out of the check AND out of the report; an operator sees "+
+			"nothing about it. stderr was:\n%s", out)
+	}
+	// And the ROW is named, not just counted: a bare number makes the operator
+	// run a second command to learn what this one already knew.
+	if !strings.Contains(string(out), "activities.metadata") {
+		t.Errorf("the advisory does not name the affected table.column. stderr was:\n%s", out)
+	}
+}
+
+// plantActivitySuspect writes a suspect value into a non-migrated table.
+func plantActivitySuspect(t *testing.T, dbPath, value string) {
+	t.Helper()
+
+	raw, err := sql.Open("sqlite", dbPath+"?_pragma=busy_timeout(30000)")
+	if err != nil {
+		t.Fatalf("open raw: %v", err)
+	}
+	defer raw.Close()
+
+	if store.MigratedTables()["activities"] {
+		t.Fatal("activities is listed as migrated; pick a table the migration really skips")
+	}
+	// The triggers have to go first, and that is itself worth recording: Layer B
+	// REFUSES this value. SQLite's json_tree walks tokens rather than building a
+	// map, so it sees the NUL in the shadowed member that our Go predicate
+	// cannot — the database is stricter than the shared predicate for exactly
+	// this shape. Such a row can therefore only be LEGACY data, written before
+	// the triggers existed, which is precisely the population BUG-2810 is about.
+	rows, err := raw.Query(
+		`SELECT name FROM sqlite_master WHERE type='trigger' AND name GLOB 'pad_nul_activities_metadata_*'`)
+	if err != nil {
+		t.Fatalf("list triggers: %v", err)
+	}
+	var names []string
+	for rows.Next() {
+		var n string
+		if err := rows.Scan(&n); err != nil {
+			rows.Close()
+			t.Fatalf("scan: %v", err)
+		}
+		names = append(names, n)
+	}
+	rows.Close()
+	if len(names) == 0 {
+		t.Fatal("no activities.metadata triggers found; the fixture would prove nothing")
+	}
+	for _, n := range names {
+		if _, derr := raw.Exec(`DROP TRIGGER IF EXISTS "` + n + `"`); derr != nil {
+			t.Fatalf("drop %s: %v", n, derr)
+		}
+	}
+
+	// id NULL, deliberately: SQLite permits a NULL in a declared TEXT PRIMARY
+	// KEY, which is what makes this row unaddressable and therefore
+	// unverifiable. workspace_id is omitted too — an instance-wide row.
+	if _, err := raw.Exec(
+		`INSERT INTO activities (id, action, actor, source, metadata, created_at)
+		 VALUES (NULL, 'created', 'agent', 'cli', ?, datetime('now'))`, value); err != nil {
+		t.Fatalf("plant: %v", err)
+	}
+}

cmd/pad/cmd_workspace.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -17,6 +19,7 @@ import (
 	"github.com/PerpetualSoftware/pad/internal/cli"
 	"github.com/PerpetualSoftware/pad/internal/collections"
 	"github.com/PerpetualSoftware/pad/internal/config"
+	"github.com/PerpetualSoftware/pad/internal/server"
 
 	"github.com/PerpetualSoftware/pad/internal/models"
 	"golang.org/x/term"
@@ -1032,6 +1035,7 @@ Both formats can be re-imported via 'pad workspace import'.`,
 
 func importCmd() *cobra.Command {
 	var nameFlag string
+	var repairNUL bool
 	cmd := &cobra.Command{
 		Use:   "import <file>",
 		Short: "Import workspace from JSON export or tar.gz bundle",
@@ -1048,9 +1052,16 @@ Format is detected by file extension. Override workspace name with --name.`,
 			client, _ := getClient()
 			filePath := args[0]
 
-			path := "/workspaces/import"
+			q := url.Values{}
 			if nameFlag != "" {
-				path += "?name=" + nameFlag
+				q.Set("name", nameFlag)
+			}
+			if repairNUL {
+				q.Set(server.NULRepairQueryParam, "true")
+			}
+			path := "/workspaces/import"
+			if len(q) > 0 {
+				path += "?" + q.Encode()
 			}
 
 			// Detect bundle by extension. .tar.gz / .tgz route through
@@ -1074,7 +1085,8 @@ Format is detected by file extension. Override workspace name with --name.`,
 			defer f.Close()
 
 			var ws models.Workspace
-			if err := client.PostStreamWithContentType(path, f, contentType, &ws); err != nil {
+			header, err := client.PostStreamWithContentTypeHeaders(path, f, contentType, &ws)
+			if err != nil {
 				return fmt.Errorf("import: %w", err)
 			}
 
@@ -1085,10 +1097,20 @@ Format is detected by file extension. Override workspace name with --name.`,
 				fmt.Printf("  Attachments: rehydrated from bundle\n")
 			}
 			fmt.Printf("  All IDs regenerated\n")
+			if repairNUL {
+				// Report what the consent flag actually did. "Repaired" with no
+				// number is the kind of reassurance an operator cannot check,
+				// and zero is a genuinely useful answer: it means the export was
+				// clean and the flag was not needed.
+				fmt.Printf("  Values repaired (each NUL replaced with U+FFFD): %s\n",
+					repairedNULCount(header))
+			}
 			return nil
 		},
 	}
 	cmd.Flags().StringVar(&nameFlag, "name", "", "override workspace name")
+	cmd.Flags().BoolVar(&repairNUL, "repair-nul", false,
+		"replace NULs carried by the export with U+FFFD instead of refusing it (rewrites content; default is strict)")
 	return cmd
 }
 
@@ -1185,3 +1207,21 @@ func auditLogCmd() *cobra.Command {
 
 	return cmd
 }
+
+// repairedNULCount reads the count the server reports for a --repair-nul
+// import.
+//
+// An ABSENT header is reported as unknown rather than as zero. A server older
+// than this flag ignores the query parameter entirely and imports strictly, so
+// printing "0" there would tell the operator the export was clean when in fact
+// nothing was even asked.
+func repairedNULCount(header http.Header) string {
+	if header == nil {
+		return "unknown (no response headers)"
+	}
+	v := header.Get(server.NULRepairHeader)
+	if v == "" {
+		return "unknown (this server does not report the count; it may predate --repair-nul)"
+	}
+	return v
+}

cmd/pad/groups.go

@@ -192,13 +192,15 @@ func agentCmd() *cobra.Command {
 func dbCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "db",
-		Short: "Database backup, restore, and migration tools",
+		Short: "Database backup, restore, repair, and migration tools",
 		RunE:  unknownSubcommandRun,
 	}
 	cmd.AddCommand(
 		dbBackupCmd(),
 		dbRestoreCmd(),
 		dbMigrateToPgCmd(),
+		dbScanNULCmd(),
+		dbRepairNULCmd(),
 	)
 	return cmd
 }

docs/backup.md

@@ -153,11 +153,13 @@ For portable workspace backups that work across SQLite and PostgreSQL:
 # Export a workspace to JSON
 pad workspace export > my-workspace.json
 
-# Import into any Pad instance (SQLite or PostgreSQL)
-pad workspace import < my-workspace.json
+# Import into any Pad instance (SQLite or PostgreSQL).
+# The file is an ARGUMENT, not stdin — `pad workspace import < file` fails
+# with "accepts 1 arg(s), received 0".
+pad workspace import my-workspace.json
 
 # Import with a new name
-pad workspace import --name "imported-workspace" < my-workspace.json
+pad workspace import --name "imported-workspace" my-workspace.json
 ```
 
 ### One case where an export is not importable
@@ -170,35 +172,99 @@ application rule, not a universal storage fact — PostgreSQL does refuse a NUL
 outright, but SQLite accepts one in a TEXT column, which is why the rule has to
 be enforced rather than assumed, and why the paragraphs below matter.
 
-**The rule lives in the binary, not in the database**, so "before the rule
-existed" is a statement about which build served the write, not about a date.
-On SQLite, any window in which an older binary serves the same database can
-still create such rows: a rollback to the previous version, a staged rollout
-where an old and a new instance share a database, or a second older instance
-pointed at the same file. Once that window closes the guard is back, but the
-rows are already stored, and they behave exactly like genuinely old ones.
+**The rule is now enforced by the database as well as by the binary.** It used
+to live only in the running build, which meant any window where an older binary
+served the same SQLite database could still create such rows — a rollback, a
+staged rollout, a second old instance pointed at the same file. A schema
+migration now installs triggers that refuse the write in the database itself,
+so an older binary writing to an upgraded file is refused too (BUG-2813). The
+window that remains is a SQLite database an upgraded binary has never opened:
+until its migrations run, it has no triggers.
 
-Only SQLite is affected. PostgreSQL refuses a NUL in a text or JSON column
+Only SQLite ever needed this. PostgreSQL refuses a NUL in a text or JSON column
 itself, at every binary version, so a PostgreSQL instance never stored such a
 value regardless of which build wrote it.
 
-If you want the guarantee rather than the guard, drain writes from older
-binaries before the new one starts serving, or roll forward rather than back.
-Enforcing the invariant below the HTTP layer, so the running build stops
-mattering, is tracked as BUG-2813.
+None of that helps a row that was **already** stored, which is what the two
+commands below are for.
 
-The same limitation applies to `pad db migrate-to-pg`, which copies rows
-directly and does not go through the import guard: a row carrying a NUL will
-fail against PostgreSQL's JSONB parser during the copy rather than being
-reported up front.
+### Finding and repairing affected rows
 
-If you hit either, the affected value has to be repaired at the source before
-the import or migration will go through — the export itself succeeds either
-way, as described above. A preflight check and a repair path are tracked as
-BUG-2810. Until then, neither error names the exact row: the import answers
-400 naming the rule it refused on, and `pad db migrate-to-pg` reports which
-workspace's copy failed — locating the offending value inside it is manual
-today.
+`pad db scan-nul` reports every stored value carrying a NUL — which table and
+column, which row, and which workspace — and changes nothing:
+
+```bash
+pad db scan-nul                       # the live database
+pad db scan-nul --from /backups/pad-20260901.db   # or a backup file
+```
+
+`pad db repair-nul` rewrites those values, replacing each NUL with U+FFFD (the
+Unicode replacement character) and leaving the rest of the value byte for byte
+as it was. **It changes stored content**, which is why it is a separate command
+and never part of a migration — running a schema upgrade should not rewrite
+your text on your behalf. Run the scan first; it is the dry run. Running the
+repair twice is safe.
+
+```bash
+pad server stop
+pad db repair-nul                     # lists what it will change, then asks
+pad server start
+```
+
+A row whose **primary key** is the value carrying the NUL is reported and left
+alone: repairing it would change the row's identity and could collide with
+another row. `email_optouts` is the only table where that can happen today.
+
+### Migrating to PostgreSQL
+
+`pad db migrate-to-pg` now runs the same scan as a **preflight**. If the source
+database carries any affected rows it lists them, prints the repair command,
+and exits without moving anything — rather than failing partway through the
+copy against PostgreSQL's JSONB parser, which is what it used to do.
+
+One shape is checked differently, and it is worth knowing why. A JSON value
+with LITERAL duplicate keys — `{"a":"...","a":"..."}` — hides anything in the
+shadowed copy from every check Pad makes, because the JSON decoder keeps only
+the last. PostgreSQL still refuses it. Rather than let such a row through, the
+preflight asks the destination directly: any value that merely *mentions* a NUL
+escape is cast on the target database before anything moves, and the migration
+is refused if PostgreSQL rejects it. That check is exact in both directions — a
+document that only writes *about* the escape is accepted, as it should be.
+
+`pad db scan-nul` lists those values under a separate heading, and
+`pad db repair-nul` fixes the fatal shape while leaving the harmless ones byte
+for byte as they were.
+
+Two things to know about that check:
+
+- **It errs toward refusing.** If the destination cannot be reached, or a listed
+  row cannot be read back, the migration is refused rather than attempted — an
+  unchecked value is not a passed one. Re-run once the destination is reachable.
+- **It can refuse a migration that would have worked.** The check casts the
+  value as it is stored, and one column — a workspace's `settings` — is
+  normalised on the way in, which happens to drop the hidden value. Such a row
+  is still a value Pad refuses to write today, so `pad db repair-nul` clears it
+  and the migration proceeds.
+
+### Importing an export that predates the rule
+
+If you have an export file taken from an affected database, the import still
+refuses it by default and the 400 names the remedy. Passing `--repair-nul`
+applies the same U+FFFD substitution to the payload as it is imported:
+
+```bash
+pad workspace import --repair-nul my-workspace.json
+```
+
+The default stays strict, and the flag is your consent to the rewrite; the
+command reports how many values it changed. It repairs the payload the way the
+server reads it, so it reaches a NUL wherever an export can carry one —
+including inside an item's `fields` blob, which travels through an export as a
+quoted document rather than as plain text.
+
+Repairing the source database with `pad db repair-nul` and re-exporting gives
+the same result without a rewrite at import time, and is the better option when
+you still have the source instance.
 
 
 This format is database-agnostic and can be used to:

internal/cli/client.go

@@ -1180,17 +1180,34 @@ func (c *Client) PostRawWithContentType(path string, data []byte, contentType st
 // import path — together they keep import memory bounded by the
 // largest single blob (~25 MiB) rather than the full bundle size.
 func (c *Client) PostStreamWithContentType(path string, body io.Reader, contentType string, result interface{}) error {
+	_, err := c.PostStreamWithContentTypeHeaders(path, body, contentType, result)
+	return err
+}
+
+// PostStreamWithContentTypeHeaders is PostStreamWithContentType, also returning
+// the response headers.
+//
+// The workspace import reports what its --repair-nul flag changed in a response
+// header rather than in the body, because the success body is the created
+// workspace and its shape is a public contract. A caller that does not need the
+// count keeps using the wrapper above.
+//
+// Headers are captured BEFORE handleResponse, which reads and closes the body;
+// on an error path they are returned alongside the error rather than dropped,
+// so a caller can still read a diagnostic header from a failed request.
+func (c *Client) PostStreamWithContentTypeHeaders(path string, body io.Reader, contentType string, result interface{}) (http.Header, error) {
 	req, err := c.newRequest("POST", path, body)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	req.Header.Set("Content-Type", contentType)
 	resp, err := c.streamClient.Do(req)
 	if err != nil {
-		return fmt.Errorf("request failed: %w", err)
+		return nil, fmt.Errorf("request failed: %w", err)
 	}
 	defer resp.Body.Close()
-	return c.handleResponse(resp, result)
+	header := resp.Header
+	return header, c.handleResponse(resp, result)
 }
 
 // --- Auth API ---

internal/server/handlers_import_bundle.go

@@ -124,7 +124,13 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
 	newName := r.URL.Query().Get("name")
 	userID := currentUserID(r)
 
-	ws, err := s.importBundle(r.Context(), gz, newName, userID)
+	// The bundle door gets the same --repair-nul treatment as the JSON one:
+	// a gzip import is the same import reached by a different Content-Type,
+	// and BUG-2803's round 3 already learned that giving the two doors
+	// different answers is how one of them keeps being forgotten.
+	repair := &nulRepairTally{Enabled: wantsNULRepair(r)}
+
+	ws, err := s.importBundle(r.Context(), gz, newName, userID, repair)
 	if err != nil {
 		// Errors from importBundle are already shaped with status hints —
 		// surface as 400 unless the underlying error wraps an http hint.
@@ -184,6 +190,7 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
 				"workspace_id", ws.ID, "user_id", userID, "error", err)
 		}
 	}
+	repair.SetHeader(w)
 	writeJSON(w, http.StatusCreated, ws)
 }
 
@@ -206,7 +213,7 @@ func (s *Server) handleImportWorkspaceBundle(w http.ResponseWriter, r *http.Requ
 // Split out from the handler so tests can drive it with a tar.Reader
 // over an in-memory bundle and assert on the resulting state without
 // a live HTTP server.
-func (s *Server) importBundle(ctx context.Context, r io.Reader, newName, ownerID string) (*models.Workspace, error) {
+func (s *Server) importBundle(ctx context.Context, r io.Reader, newName, ownerID string, repair *nulRepairTally) (*models.Workspace, error) {
 	tr := tar.NewReader(r)
 	blobCap := s.effectiveBlobMaxBytes()
 
@@ -280,10 +287,12 @@ func (s *Server) importBundle(ctx context.Context, r io.Reader, newName, ownerID
 			// the same door as the JSON one, reached by a different
 			// Content-Type, so it gets the same answer rather than a 500
 			// from Postgres further down (codex round 3).
+			buf = repair.Apply(buf)
 			if bodyDecodesNUL(buf) {
 				return nil, &importStatusError{
 					status: http.StatusBadRequest, code: "bad_bundle",
-					message: "Bundle pad-export.json could not be decoded: " + errJSONBodyNUL.Error(),
+					message: "Bundle pad-export.json could not be decoded: " + errJSONBodyNUL.Error() +
+						nulRepairRemedy(repair),
 				}
 			}
 			var export models.WorkspaceExport
@@ -358,8 +367,10 @@ func (s *Server) importBundle(ctx context.Context, r io.Reader, newName, ownerID
 			// the bundle-import contract rather than a fix to this bug. The
 			// resulting state is pinned by a test and stated in the release
 			// note instead of being left incidental.
+			buf = repair.Apply(buf)
 			if bodyDecodesNUL(buf) {
-				return ws, fmt.Errorf("manifest decode: %w (workspace created but attachments not restored)", errJSONBodyNUL)
+				return ws, fmt.Errorf("manifest decode: %w (workspace created but attachments not restored)%s",
+					errJSONBodyNUL, nulRepairRemedy(repair))
 			}
 			var manifest models.AttachmentManifest
 			if err := json.Unmarshal(buf, &manifest); err != nil {

internal/server/handlers_workspace_import_nul_test.go

@@ -0,0 +1,363 @@
+package server
+
+import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// The workspace import's --repair-nul flag, measured AT THE DOOR (DOC-2823 S3,
+// BUG-2810).
+//
+// The unit-level tests in nul_repair_differential_test.go measure the repair
+// and the remedy string. These drive the handler, because the defect BUG-2810
+// describes is a round trip — "the server emits a payload it will then refuse"
+// — and the flag's whole job is to close it. A door-level test is also what
+// catches the wiring mistake this file's first version had: the JSON path
+// repaired the body and then discarded the count, so the header reported 0
+// while the import had rewritten a value.
+
+// nulExportBody builds the payload an export of an AFFECTED database produces.
+//
+// The escape is not typed: a real NUL is put in a Go string and json.Marshal
+// writes it out as the six-character escape, which is exactly how the export
+// endpoint produces one. Typing the escape here would make the fixture a
+// statement about this file rather than about the export.
+func nulExportBody(t *testing.T, content string) []byte {
+	t.Helper()
+	b, err := json.Marshal(models.WorkspaceExport{
+		Version:   1,
+		Workspace: models.WorkspaceExportMeta{Name: "Affected", Slug: "affected"},
+		Collections: []models.CollectionExport{{
+			ID: "col-1", Name: "Tasks", Slug: "tasks", Schema: "{}", Settings: "{}",
+		}},
+		Items: []models.ItemExport{{
+			ID: "item-1", CollectionID: "col-1", Title: "Subject", Slug: "subject",
+			Content: content, Fields: "{}", Tags: "[]", ItemNumber: 1,
+		}},
+	})
+	if err != nil {
+		t.Fatalf("marshal export: %v", err)
+	}
+	return b
+}
+
+func importWithQuery(t *testing.T, srv *Server, query string, body []byte) *httptest.ResponseRecorder {
+	t.Helper()
+	r := httptest.NewRequest("POST", "/api/v1/workspaces/import"+query, bytes.NewReader(body))
+	r.Header.Set("Content-Type", "application/json")
+	r.RemoteAddr = "192.0.2.1:1234"
+	rr := httptest.NewRecorder()
+	srv.handleImportWorkspace(rr, r)
+	return rr
+}
+
+// TestImportOfAnAffectedExportIsRefusedAndTheRemedyWorks is BUG-2810's filed
+// symptom and its fix, in one test, in that order.
+func TestImportOfAnAffectedExportIsRefusedAndTheRemedyWorks(t *testing.T) {
+	affected := nulExportBody(t, "before"+textguard.NUL+"after")
+
+	// The fixture must actually carry the escape, or everything below passes
+	// while measuring nothing.
+	if !bytes.Contains(affected, []byte(textguard.EscNUL)) {
+		t.Fatalf("fixture does not carry the escape: %s", affected)
+	}
+
+	t.Run("strict refuses it and names the flag", func(t *testing.T) {
+		srv := testServer(t)
+		rr := importWithQuery(t, srv, "", affected)
+
+		if rr.Code != http.StatusBadRequest {
+			t.Fatalf("strict import returned %d, want 400: %s", rr.Code, rr.Body.String())
+		}
+		body := rr.Body.String()
+		if !strings.Contains(body, "--repair-nul") {
+			t.Errorf("the refusal does not name the remedy: %s", body)
+		}
+		if !strings.Contains(body, "pad db repair-nul") {
+			t.Errorf("the refusal does not name the database-side repair: %s", body)
+		}
+	})
+
+	t.Run("the named flag accepts the same body and reports the count", func(t *testing.T) {
+		srv := testServer(t)
+		rr := importWithQuery(t, srv, "?repair_nul=true", affected)
+
+		if rr.Code != http.StatusCreated {
+			t.Fatalf("import with --repair-nul returned %d, want 201: %s", rr.Code, rr.Body.String())
+		}
+		if got := rr.Header().Get(NULRepairHeader); got != "1" {
+			t.Errorf("%s = %q, want \"1\" — the operator is told what was rewritten, and a count that "+
+				"is always 0 is how the repair silently stops being reported", NULRepairHeader, got)
+		}
+
+		// And the stored value is the repaired one, not the original and not
+		// something blanked. This is the assertion that makes the 201 mean
+		// something.
+		var ws models.Workspace
+		if err := json.Unmarshal(rr.Body.Bytes(), &ws); err != nil {
+			t.Fatalf("decode response: %v", err)
+		}
+		items, err := srv.store.ListItems(ws.ID, models.ItemListParams{})
+		if err != nil {
+			t.Fatalf("list items: %v", err)
+		}
+		if len(items) != 1 {
+			t.Fatalf("imported %d items, want 1", len(items))
+		}
+		if want := "before" + textguard.Replacement + "after"; items[0].Content != want {
+			t.Errorf("imported content = %q, want %q", items[0].Content, want)
+		}
+	})
+
+	t.Run("a clean export reports zero rather than nothing", func(t *testing.T) {
+		srv := testServer(t)
+		rr := importWithQuery(t, srv, "?repair_nul=true", nulExportBody(t, "ordinary content"))
+
+		if rr.Code != http.StatusCreated {
+			t.Fatalf("clean import with the flag returned %d, want 201: %s", rr.Code, rr.Body.String())
+		}
+		// Zero is a real answer: it tells the operator the export did not need
+		// the flag. An absent header would be indistinguishable from an old
+		// server that ignored it.
+		if got := rr.Header().Get(NULRepairHeader); got != "0" {
+			t.Errorf("%s = %q, want \"0\"", NULRepairHeader, got)
+		}
+	})
+
+	t.Run("the flag does not repair without being asked", func(t *testing.T) {
+		// The control for the whole feature: an import that repaired by
+		// default would pass every assertion above and break the strict
+		// posture Dave's ruling keeps.
+		srv := testServer(t)
+		rr := importWithQuery(t, srv, "?repair_nul=false", affected)
+		if rr.Code != http.StatusBadRequest {
+			t.Fatalf("repair_nul=false returned %d, want 400 — the default must stay strict: %s",
+				rr.Code, rr.Body.String())
+		}
+	})
+}
+
+// TestImportBundle_RepairFlagCoversTheBundleDoorToo is the parity leg.
+//
+// handleImportWorkspaceBundle is reachable only through the Content-Type
+// dispatch in handleImportWorkspace, so a flag honoured below that dispatch
+// would work on the JSON path and silently do nothing on the tar.gz one — which
+// is the path a real `pad workspace export --bundle` produces. BUG-2803's round
+// 3 found the strict check missing on exactly this door for the same reason,
+// and the plan-limit unit found its gate missing there too.
+func TestImportBundle_RepairFlagCoversTheBundleDoorToo(t *testing.T) {
+	src, srcSlug := testServerWithAttachments(t)
+	rr := doRequest(src, "GET", "/api/v1/workspaces/"+srcSlug+"/export", nil)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("export src: %d %s", rr.Code, rr.Body.String())
+	}
+	clean := rr.Body.String()
+
+	// Same fixture shape as TestImportBundle_RefusesNULInExport: the escape
+	// goes into the exported workspace NAME, because a bundle is bytes on the
+	// wire and that is the shape an affected export carries.
+	withNUL := strings.Replace(clean, `"name":"`, `"name":"a`+textguard.EscNUL+`b `, 1)
+	if withNUL == clean {
+		t.Fatal("fixture did not modify the export; the probe would be vacuous")
+	}
+
+	bundle := func(exportJSON string) []byte {
+		var buf bytes.Buffer
+		gzw := gzip.NewWriter(&buf)
+		tw := tar.NewWriter(gzw)
+		if err := tw.WriteHeader(&tar.Header{
+			Name: "pad-export.json", Mode: 0o644, Size: int64(len(exportJSON)),
+		}); err != nil {
+			t.Fatalf("write header: %v", err)
+		}
+		if _, err := tw.Write([]byte(exportJSON)); err != nil {
+			t.Fatalf("write export: %v", err)
+		}
+		tw.Close()
+		gzw.Close()
+		return buf.Bytes()
+	}
+
+	post := func(query string, body []byte) *httptest.ResponseRecorder {
+		dest, _ := testServerWithAttachments(t)
+		req := httptest.NewRequest("POST", "/api/v1/workspaces/import"+query, bytes.NewReader(body))
+		req.Header.Set("Content-Type", "application/gzip")
+		req.RemoteAddr = "127.0.0.1:1234"
+		rec := httptest.NewRecorder()
+		dest.ServeHTTP(rec, req)
+		return rec
+	}
+
+	// Strict still refuses, and names the remedy.
+	strict := post("?name=Strict", bundle(withNUL))
+	if strict.Code != http.StatusBadRequest {
+		t.Fatalf("strict bundle import returned %d, want 400: %s", strict.Code, strict.Body.String())
+	}
+	if !strings.Contains(strict.Body.String(), "--repair-nul") {
+		t.Errorf("the bundle refusal does not name the remedy: %s", strict.Body.String())
+	}
+
+	// And the named flag accepts the same bytes.
+	repaired := post("?name=Repaired&repair_nul=true", bundle(withNUL))
+	if repaired.Code != http.StatusOK && repaired.Code != http.StatusCreated {
+		t.Fatalf("bundle import with --repair-nul returned %d, want 201 — the flag is honoured on the "+
+			"JSON door and not this one: %s", repaired.Code, repaired.Body.String())
+	}
+	if got := repaired.Header().Get(NULRepairHeader); got != "1" {
+		t.Errorf("%s = %q, want \"1\"", NULRepairHeader, got)
+	}
+}
+
+// TestImportRepairsANULInsideAFieldsBlob is BUG-2810's actual shape, end to
+// end, and it is the leg the first version of this file did not have.
+//
+// An item's `fields` is stored as JSON TEXT and exported as a STRING, so an
+// affected row arrives in the import body as a doubled-backslash escape inside
+// a quoted document — which the gate refuses (it re-parses that string) and
+// which a raw-byte repair cannot touch (at the body's own layer it is literal
+// text). Every fixture that put the NUL in `content` passed while this did not
+// work, which is how the gap survived to the first review round.
+func TestImportRepairsANULInsideAFieldsBlob(t *testing.T) {
+	// A JSON column has TWO at-rest shapes and they export DIFFERENTLY, which
+	// is the distinction this fixture exists to hold:
+	//
+	//   - a raw NUL byte in the blob text marshals to a SINGLE-backslash
+	//     escape, so the decoded `fields` string contains a real NUL and the
+	//     gate's plain check sees it;
+	//   - a LIVE ESCAPE in the blob text — the 22P05 shape, six characters at
+	//     rest — marshals to a DOUBLED backslash, and only the gate's re-parse
+	//     of that string finds it.
+	//
+	// The second is the one a raw-byte repair cannot reach, so it is the one
+	// under test. Getting this wrong is easy and silent: the first draft of
+	// this fixture used a real NUL and produced the single-backslash form,
+	// which is the case that already worked.
+	storedBlob := `{"note":"x` + textguard.EscNUL + `y"}`
+	body, err := json.Marshal(models.WorkspaceExport{
+		Version:   1,
+		Workspace: models.WorkspaceExportMeta{Name: "Affected", Slug: "affected"},
+		Collections: []models.CollectionExport{{
+			ID: "col-1", Name: "Tasks", Slug: "tasks", Schema: "{}", Settings: "{}",
+		}},
+		Items: []models.ItemExport{{
+			ID: "item-1", CollectionID: "col-1", Title: "Subject", Slug: "subject",
+			Content: "clean", Fields: storedBlob, Tags: "[]", ItemNumber: 1,
+		}},
+	})
+	if err != nil {
+		t.Fatalf("marshal export: %v", err)
+	}
+
+	// The fixture must carry the DOUBLED form, or it is the easy case again.
+	doubled := textguard.EscNUL[:1] + textguard.EscNUL
+	if !bytes.Contains(body, []byte(doubled)) {
+		t.Fatalf("fixture does not carry a doubled-backslash escape; it is not the shape under test: %s", body)
+	}
+
+	t.Run("strict refuses it", func(t *testing.T) {
+		srv := testServer(t)
+		rr := importWithQuery(t, srv, "", body)
+		if rr.Code != http.StatusBadRequest {
+			t.Fatalf("strict import returned %d, want 400: %s", rr.Code, rr.Body.String())
+		}
+	})
+
+	t.Run("the flag repairs it and the stored blob is clean", func(t *testing.T) {
+		srv := testServer(t)
+		rr := importWithQuery(t, srv, "?repair_nul=true", body)
+		if rr.Code != http.StatusCreated {
+			t.Fatalf("import with --repair-nul returned %d, want 201: %s", rr.Code, rr.Body.String())
+		}
+		if got := rr.Header().Get(NULRepairHeader); got != "1" {
+			t.Errorf("%s = %q, want \"1\"", NULRepairHeader, got)
+		}
+
+		var ws models.Workspace
+		if err := json.Unmarshal(rr.Body.Bytes(), &ws); err != nil {
+			t.Fatalf("decode response: %v", err)
+		}
+		items, err := srv.store.ListItems(ws.ID, models.ItemListParams{})
+		if err != nil {
+			t.Fatalf("list items: %v", err)
+		}
+		if len(items) != 1 {
+			t.Fatalf("imported %d items, want 1", len(items))
+		}
+
+		// The stored blob carries U+FFFD where the NUL was, and is still the
+		// same document otherwise. Asserting the CONTENT is what distinguishes
+		// a repair from a blank.
+		//
+		// As the ESCAPE, not the character: the JSON arm rewrites `\u0000` to
+		// `\ufffd` in place, six characters for six, so nothing around it
+		// shifts. The two decode identically — checked below, because a stored
+		// blob that merely LOOKS right and parses to something else would
+		// satisfy the byte comparison alone.
+		if want := `{"note":"x` + textguard.ReplacementEscape + `y"}`; items[0].Fields != want {
+			t.Errorf("stored fields = %q, want %q", items[0].Fields, want)
+		}
+		var blob map[string]string
+		if err := json.Unmarshal([]byte(items[0].Fields), &blob); err != nil {
+			t.Fatalf("stored blob is not valid JSON: %v", err)
+		}
+		if want := "x" + textguard.Replacement + "y"; blob["note"] != want {
+			t.Errorf("stored blob decodes to %q, want %q", blob["note"], want)
+		}
+	})
+}
+
+// TestImportRepairsARawNULInsideAFieldsBlob is the OTHER at-rest shape of the
+// same column, and it is here because the two are repaired by different passes.
+//
+// A raw NUL byte in the blob marshals to a single-backslash escape, so the
+// decoded `fields` string carries a real NUL and the repair's raw pass fixes
+// it — leaving the character rather than the escape, since there is no escape
+// to rewrite. Without this leg the JSON column would only ever be tested in the
+// shape the document pass handles.
+func TestImportRepairsARawNULInsideAFieldsBlob(t *testing.T) {
+	storedBlob := `{"note":"x` + textguard.NUL + `y"}`
+	body, err := json.Marshal(models.WorkspaceExport{
+		Version:   1,
+		Workspace: models.WorkspaceExportMeta{Name: "RawBlob", Slug: "rawblob"},
+		Collections: []models.CollectionExport{{
+			ID: "col-1", Name: "Tasks", Slug: "tasks", Schema: "{}", Settings: "{}",
+		}},
+		Items: []models.ItemExport{{
+			ID: "item-1", CollectionID: "col-1", Title: "Subject", Slug: "subject",
+			Content: "clean", Fields: storedBlob, Tags: "[]", ItemNumber: 1,
+		}},
+	})
+	if err != nil {
+		t.Fatalf("marshal export: %v", err)
+	}
+
+	srv := testServer(t)
+	rr := importWithQuery(t, srv, "?repair_nul=true", body)
+	if rr.Code != http.StatusCreated {
+		t.Fatalf("import returned %d, want 201: %s", rr.Code, rr.Body.String())
+	}
+
+	var ws models.Workspace
+	if err := json.Unmarshal(rr.Body.Bytes(), &ws); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+	items, err := srv.store.ListItems(ws.ID, models.ItemListParams{})
+	if err != nil {
+		t.Fatalf("list items: %v", err)
+	}
+	if len(items) != 1 {
+		t.Fatalf("imported %d items, want 1", len(items))
+	}
+	if want := `{"note":"x` + textguard.Replacement + `y"}`; items[0].Fields != want {
+		t.Errorf("stored fields = %q, want %q", items[0].Fields, want)
+	}
+}

internal/server/handlers_workspaces.go

@@ -880,8 +880,27 @@ func (s *Server) handleImportWorkspace(w http.ResponseWriter, r *http.Request) {
 	// the default 2 MiB decodeJSON cap. 64 MiB is well above any realistic
 	// single-workspace backup while still far from the heap-exhaustion
 	// range the default cap protects against.
-	if err := decodeJSONWithLimit(r, &data, 64<<20); err != nil {
-		writeError(w, http.StatusBadRequest, "bad_request", "invalid export data: "+err.Error())
+	//
+	// --repair-nul (DOC-2823 S3 / BUG-2810). The default is strict; the flag
+	// buys the body ONE repair attempt and then runs the same gate on the
+	// repaired bytes, so this is not a decode path that skips the check.
+	repair := &nulRepairTally{Enabled: wantsNULRepair(r)}
+	var decodeErr error
+	if repair.Enabled {
+		decodeErr = decodeJSONRepairingNUL(r, &data, 64<<20, repair)
+	} else {
+		decodeErr = decodeJSONWithLimit(r, &data, 64<<20)
+	}
+	if decodeErr != nil {
+		msg := "invalid export data: " + decodeErr.Error()
+		if errors.Is(decodeErr, errJSONBodyNUL) {
+			// The strict refusal NAMES the remedy, per Dave's day-54 ruling —
+			// and TestImportStrictRefusalNamesTheWorkingRemedy drives the named
+			// flag against this exact failing body, because a suggested remedy
+			// is an untested contract claim until it has been run (PATTE-135).
+			msg += nulRepairRemedy(repair)
+		}
+		writeError(w, http.StatusBadRequest, "bad_request", msg)
 		return
 	}
 
@@ -908,5 +927,10 @@ func (s *Server) handleImportWorkspace(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	if repair.Enabled && repair.Replaced > 0 {
+		slog.Info("workspace import repaired NUL escapes on the operator's instruction",
+			"workspace_id", ws.ID, "replaced", repair.Replaced)
+	}
+	repair.SetHeader(w)
 	writeJSON(w, http.StatusCreated, ws)
 }

internal/server/import_nul_repair.go

@@ -0,0 +1,131 @@
+package server
+
+import (
+	"net/http"
+	"strconv"
+
+	"github.com/PerpetualSoftware/pad/internal/store"
+)
+
+// The workspace import's --repair-nul consent flag (DOC-2823 S3, BUG-2810).
+//
+// WHY IT EXISTS. A self-hoster whose database predates the NUL enforcement can
+// EXPORT a workspace and then not import it back: the server emits a payload it
+// will refuse. That is the sentence in BUG-2810 that made it an item rather
+// than a note — the product has no path forward for somebody restoring their
+// own backup.
+//
+// WHAT IT IS NOT. It is not an exemption from the gate. The gate still runs, on
+// the repaired bytes, and still decides; the flag buys the body one repair
+// attempt first. BUG-2803's filing named this endpoint as carrying the largest
+// attacker-controlled body in the product and explicitly ruled out exempting
+// it, so nothing here may become a decode path that skips the check.
+//
+// Dave's day-54 ruling: the flag ships, the default stays strict, the strict
+// refusal names the flag, and a test drives that named remedy against the exact
+// failing fixture.
+
+// NULRepairQueryParam is how the flag reaches the server. The CLI's
+// `pad workspace import --repair-nul` sets it.
+//
+// EXPORTED so the CLI sends the name this package reads, rather than a second
+// spelling of it. A query parameter and a response header are a two-sided
+// contract, and the failure mode of the two sides drifting is silent: the
+// import simply stops repairing, or stops reporting, with nothing to see.
+const NULRepairQueryParam = "repair_nul"
+
+// NULRepairHeader reports how many VALUES were rewritten, so the operator is
+// told what changed rather than only that something did. A header rather than a
+// response field because the success body is the created workspace and its
+// shape is a public contract.
+//
+// Values, not escapes: the repair works on the DECODED body, where the escape
+// form has already been resolved and one nested document may have carried
+// several. "Two values were rewritten" is also the sentence an operator can
+// check against the rows they end up with.
+const NULRepairHeader = "X-Pad-Repaired-NUL-Values"
+
+// nulRepairTally carries the flag through an import, counts what it changed,
+// and records any reason it declined to act.
+type nulRepairTally struct {
+	Enabled  bool
+	Replaced int
+	// Declined explains why the repair did not run on a body it was asked to
+	// repair — today, only a payload with duplicate object members, where
+	// repairing would change which value is imported. Empty when the repair
+	// ran, whether or not it found anything.
+	Declined string
+}
+
+// wantsNULRepair reports whether the request asked for the repair.
+func wantsNULRepair(r *http.Request) bool {
+	switch r.URL.Query().Get(NULRepairQueryParam) {
+	case "1", "true", "yes":
+		return true
+	}
+	return false
+}
+
+// Apply repairs a JSON body's NUL-carrying values when the flag is set, and
+// returns the bytes the gate should judge.
+//
+// A nil tally, or one with the flag unset, returns the input untouched — so a
+// caller that forgets to thread the flag gets the strict behaviour, which is
+// the safe direction for this particular forgetting.
+func (t *nulRepairTally) Apply(raw []byte) []byte {
+	if t == nil || !t.Enabled {
+		return raw
+	}
+	repaired, n, declined := repairBodyNULEscapes(raw)
+	t.Replaced += n
+	if declined != "" {
+		t.Declined = declined
+	}
+	return repaired
+}
+
+// SetHeader reports the count on a successful import.
+func (t *nulRepairTally) SetHeader(w http.ResponseWriter) {
+	if t == nil || !t.Enabled {
+		return
+	}
+	w.Header().Set(NULRepairHeader, strconv.Itoa(t.Replaced))
+}
+
+// nulRepairRemedy is the sentence appended to a strict refusal, naming the flag
+// that would have accepted it.
+//
+// It says something DIFFERENT when the flag was already given, because at that
+// point the value is one the repair could not fix, and telling an operator to
+// re-run with a flag they just used is worse than saying nothing (PATTE-135 in
+// the other direction: a remedy that does not work is still a contract claim).
+func nulRepairRemedy(t *nulRepairTally) string {
+	if t != nil && t.Declined != "" {
+		// The one case where the repair KNOWS why it did nothing, so it says so
+		// rather than falling through to the message below, which would tell an
+		// operator the repair ran when it deliberately did not.
+		return ". The import's NUL repair did not run: " + t.Declined +
+			". Repair the source database with '" + store.RepairNULCommand + "' and export again"
+	}
+	if t != nil && t.Enabled {
+		// DELIBERATELY DOES NOT NAME A CAUSE, and this branch should now be
+		// unreachable in practice: the repair walks the decoded body with the
+		// same classing the gate uses, so a body the gate refuses afterwards
+		// means the two walks have diverged — which is a defect here, not a
+		// property of the operator's data. Guessing at a cause would send them
+		// to a fix for a problem they do not have.
+		//
+		// (Two earlier wordings named causes that were wrong. The first said
+		// "a raw NUL byte rather than an escape"; the second said "not a plain
+		// NUL escape in the document". Both stopped being true when the repair
+		// moved from scanning raw bytes to walking the decoded body.)
+		return ". The import's NUL repair ran and the value is still refused. Repair the source database" +
+			" with '" + store.RepairNULCommand + "' and export again"
+	}
+	// Worded for BOTH doors. The web UI posts to this same endpoint, and a
+	// browser user has no command line to add a flag to — so the message names
+	// the option and then where to find it, rather than assuming a terminal.
+	return ". This export was written by a Pad older than the check that now refuses it. Re-run the import" +
+		" with the NUL repair option (--repair-nul on the CLI) to replace each NUL with U+FFFD, or repair" +
+		" the source database first with '" + store.RepairNULCommand + "'"
+}

internal/server/nul_repair_differential_test.go

@@ -0,0 +1,465 @@
+package server
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/store"
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// The HTTP gate's REPAIR leg (DOC-2823 S3). The other three are in
+// internal/store, beside their own corpus legs.
+
+// TestHTTPGateAcceptsEveryRepairedCorpusValue drives each repaired value
+// through the gate exactly as the refusal leg drives the original, with the
+// same key-derived classing.
+func TestHTTPGateAcceptsEveryRepairedCorpusValue(t *testing.T) {
+	for _, c := range textguard.Corpus {
+		t.Run(c.Name, func(t *testing.T) {
+			repaired := textguard.Repair(c.Value, c.IsJSON)
+
+			key := "content"
+			if c.IsJSON {
+				key = "fields"
+			}
+			body, err := json.Marshal(map[string]any{key: repaired})
+			if err != nil {
+				t.Fatalf("marshal: %v", err)
+			}
+			if bodyDecodesNUL(body) {
+				t.Errorf("the gate refuses a REPAIRED value\n  original: %q\n  repaired: %q\n  body: %s\n"+
+					"  why this case exists: %s", c.Value, repaired, body, c.Why)
+			}
+		})
+	}
+}
+
+// TestImportRepairFlagRepairsExactlyTheEscape pins what the flag does to a
+// body, at the level the handler consumes.
+//
+// The two negative halves are the load-bearing ones. A raw NUL BYTE makes the
+// document invalid JSON, so repairing one would turn a body the decoder rejects
+// into one it accepts — and widening what parses is not this flag's job. And a
+// doubled-backslash literal decodes to no NUL at all, so rewriting it would
+// corrupt a legitimate value.
+func TestImportRepairFlagRepairsExactlyTheEscape(t *testing.T) {
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+
+	cases := []struct {
+		name        string
+		body        string
+		wantChanged bool
+		wantCount   int
+		why         string
+	}{
+		{
+			name:        "a live escape is replaced",
+			body:        `{"content":"x` + esc + `y"}`,
+			wantChanged: true, wantCount: 1,
+			why: "the shape a pre-enforcement export actually carries.",
+		},
+		{
+			name:        "two live escapes are both replaced and counted",
+			body:        `{"content":"x` + esc + `y","title":"a` + esc + `b"}`,
+			wantChanged: true, wantCount: 2,
+			why: "the count is reported to the operator, so it has to be a count and not a flag.",
+		},
+		{
+			name:        "a doubled-backslash literal is untouched",
+			body:        `{"content":"x` + backslash + esc + `y"}`,
+			wantChanged: false, wantCount: 0,
+			why: "literal text after an escaped backslash. Rewriting it would corrupt a valid value.",
+		},
+		{
+			name:        "a raw NUL byte is left to fail the decode",
+			body:        `{"content":"x` + textguard.NUL + `y"}`,
+			wantChanged: false, wantCount: 0,
+			why: "invalid JSON. Repairing it would make a body parse that previously did not.",
+		},
+		{
+			name:        "a clean body is untouched",
+			body:        `{"content":"ordinary"}`,
+			wantChanged: false, wantCount: 0,
+			why: "the control. Without it a repair that rewrites everything passes every other case.",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, n, _ := repairBodyNULEscapes([]byte(tc.body))
+			changed := string(got) != tc.body
+			if changed != tc.wantChanged {
+				t.Errorf("%s\n  body:     %q\n  repaired: %q\n  changed=%t, want %t",
+					tc.why, tc.body, got, changed, tc.wantChanged)
+			}
+			if n != tc.wantCount {
+				t.Errorf("replaced %d, want %d (%s)", n, tc.wantCount, tc.why)
+			}
+			// Whatever came back, the gate must accept it when the repair
+			// claims to have fixed something — that is the whole contract.
+			if tc.wantChanged && bodyDecodesNUL(got) {
+				t.Errorf("the repaired body is still refused by the gate: %q", got)
+			}
+		})
+	}
+}
+
+// TestImportStrictRefusalNamesTheWorkingRemedy is Dave's day-54 ruling 2 in
+// test form, and PATTE-135's rule: a suggested remedy is an untested contract
+// claim until something runs it.
+//
+// So this does not merely assert that the refusal message mentions a flag. It
+// takes the EXACT body that produced the refusal, applies the named remedy, and
+// asserts the gate then accepts it — which is the only version of this test
+// that would fail if the message named a flag that did not work.
+func TestImportStrictRefusalNamesTheWorkingRemedy(t *testing.T) {
+	failing := []byte(`{"items":[{"content":"x` + textguard.EscNUL + `y"}]}`)
+
+	// Precondition: strict really does refuse this body. Without it the test
+	// could pass against a fixture that was never refused in the first place.
+	if !bodyDecodesNUL(failing) {
+		t.Fatalf("fixture does not reproduce the refusal: %s", failing)
+	}
+
+	strict := &nulRepairTally{Enabled: false}
+	msg := nulRepairRemedy(strict)
+	if !strings.Contains(msg, "--repair-nul") {
+		t.Fatalf("the strict refusal does not name the flag: %q", msg)
+	}
+	if !strings.Contains(msg, "pad db repair-nul") {
+		t.Errorf("the refusal does not name the database-side repair either: %q", msg)
+	}
+
+	// Now RUN the named remedy against the same body.
+	withFlag := &nulRepairTally{Enabled: true}
+	repaired := withFlag.Apply(failing)
+	if bodyDecodesNUL(repaired) {
+		t.Fatalf("the remedy the refusal names does not accept the body it was suggested for\n"+
+			"  body:     %s\n  repaired: %s", failing, repaired)
+	}
+	if withFlag.Replaced != 1 {
+		t.Errorf("replaced %d escapes, want 1", withFlag.Replaced)
+	}
+
+	// And the message an operator sees AFTER using the flag must not tell them
+	// to use it again — at that point the repair has already been tried and did
+	// not fix the value.
+	afterMsg := nulRepairRemedy(withFlag)
+	if strings.Contains(afterMsg, "Re-run the import with --repair-nul") {
+		t.Errorf("the post-flag message repeats a remedy the caller already used: %q", afterMsg)
+	}
+	if !strings.Contains(afterMsg, "pad db repair-nul") {
+		t.Errorf("the post-flag message gives no remaining course of action: %q", afterMsg)
+	}
+}
+
+// TestNULRepairTallyDefaultsToStrict pins the direction a mistake falls in.
+//
+// A nil tally, or one nobody set the flag on, must leave the body alone — so a
+// call site that forgets to thread the flag gets the strict behaviour rather
+// than a silently repairing import.
+func TestNULRepairTallyDefaultsToStrict(t *testing.T) {
+	body := []byte(`{"content":"x` + textguard.EscNUL + `y"}`)
+
+	var nilTally *nulRepairTally
+	if got := nilTally.Apply(body); string(got) != string(body) {
+		t.Errorf("a nil tally repaired the body: %q", got)
+	}
+
+	off := &nulRepairTally{}
+	if got := off.Apply(body); string(got) != string(body) {
+		t.Errorf("a disabled tally repaired the body: %q", got)
+	}
+	if off.Replaced != 0 {
+		t.Errorf("a disabled tally counted %d replacements", off.Replaced)
+	}
+}
+
+// TestRepairFlagReachesTheNestedAndObliqueForms is the codex round-1 finding
+// turned into a test, and it is the reason the repair walks the DECODED body
+// rather than scanning raw bytes.
+//
+// Both fixtures are shapes a REAL export carries and a raw scan cannot touch:
+//
+//   - `items.fields` travels as a STRING. The stored blob's live escape is
+//     written into the body with a DOUBLED backslash, which at the body's own
+//     layer is literal text — correctly left alone by a raw scan, and refused
+//     by the gate anyway, because the gate re-parses that string as the
+//     document it is. This is the most common carrier in a real export and the
+//     first version of the flag could not repair it.
+//   - The oblique spelling puts the backslash itself in as an escape, so the
+//     six characters do not appear in the raw bytes at all (BUG-2803 round 4).
+//     The decode resolves it; a scan never sees it.
+//
+// Each leg asserts the gate ACCEPTS the repaired body, which is the property
+// the flag exists to deliver, and that the count is reported.
+func TestRepairFlagReachesTheNestedAndObliqueForms(t *testing.T) {
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+
+	cases := []struct {
+		name string
+		body string
+		why  string
+	}{
+		{
+			name: "a live escape inside a fields blob carried as a string",
+			body: `{"fields":"{\"a\":\"x` + backslash + esc + `y\"}"}`,
+			why:  "what an export of an affected items.fields row actually looks like on the wire.",
+		},
+		{
+			name: "the obliquely spelled escape inside a fields blob",
+			body: `{"fields":"{\"a\":\"x` + backslash + `u005c` + `u0000y\"}"}`,
+			why:  "the backslash written as its own escape; the six characters never appear in the body.",
+		},
+		{
+			name: "a NUL in a KEY of a nested document",
+			body: `{"fields":"{\"k` + backslash + esc + `ey\":\"v\"}"}`,
+			why:  "keys are as fatal as values, and a value-only repair leaves the body refused.",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			raw := []byte(tc.body)
+
+			// Precondition: the gate refuses it. A fixture the gate accepts
+			// would make every assertion below vacuous.
+			if !bodyDecodesNUL(raw) {
+				t.Fatalf("the gate does not refuse this fixture, so it measures nothing: %s", raw)
+			}
+
+			tally := &nulRepairTally{Enabled: true}
+			repaired := tally.Apply(raw)
+
+			if bodyDecodesNUL(repaired) {
+				t.Fatalf("%s\n  the repair did not make the body acceptable\n  in:  %s\n  out: %s",
+					tc.why, raw, repaired)
+			}
+			if tally.Replaced != 1 {
+				t.Errorf("reported %d repaired value(s), want 1: %s", tally.Replaced, repaired)
+			}
+		})
+	}
+}
+
+// TestRepairFlagLeavesALiteralAlone is the negative half of the test above, and
+// the reason the repair cannot simply rewrite every `\u0000`-shaped run of
+// characters it finds.
+//
+// A doubled backslash in a field the gate does NOT re-parse is six literal
+// characters — corpus case 3 — and an accepted value. Rewriting it would
+// corrupt a document that merely writes ABOUT this bug, which in a
+// documentation tool is not hypothetical.
+func TestRepairFlagLeavesALiteralAlone(t *testing.T) {
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+	body := []byte(`{"content":"x` + backslash + esc + `y"}`)
+
+	if bodyDecodesNUL(body) {
+		t.Fatalf("the gate refuses a literal; the control is broken: %s", body)
+	}
+
+	tally := &nulRepairTally{Enabled: true}
+	got := tally.Apply(body)
+	if string(got) != string(body) {
+		t.Errorf("an accepted value was rewritten\n  in:  %s\n  out: %s", body, got)
+	}
+	if tally.Replaced != 0 {
+		t.Errorf("reported %d repaired value(s) for a body with nothing to repair", tally.Replaced)
+	}
+}
+
+// TestBodyRepairMirrorsTheGateOverTheCorpus is the guard on the one risk the
+// decoded walk introduces: repairDecodedNULs and bodyDecodesNUL are two
+// traversals of the same shape with the same classing, and a divergence between
+// them is invisible to review.
+//
+// So it is measured rather than reviewed. Every corpus case is put into a body
+// the way the gate's own leg puts it — classed by KEY — then repaired by the
+// BODY path and handed back to the gate. Refused cases must come back accepted;
+// accepted cases must come back byte-identical, which is what catches a walk
+// that rewrites something nobody complained about.
+func TestBodyRepairMirrorsTheGateOverTheCorpus(t *testing.T) {
+	for _, c := range textguard.Corpus {
+		t.Run(c.Name, func(t *testing.T) {
+			key := "content"
+			if c.IsJSON {
+				key = "fields"
+			}
+			body, err := json.Marshal(map[string]any{key: c.Value})
+			if err != nil {
+				t.Fatalf("marshal: %v", err)
+			}
+
+			// The corpus's own verdict must be what the gate says about this
+			// body, or the case is being measured in the wrong shape.
+			if got := bodyDecodesNUL(body); got != c.Refused {
+				t.Fatalf("gate refused=%t, corpus says %t — the fixture shape is wrong, not the repair",
+					got, c.Refused)
+			}
+
+			repaired, n, _ := repairBodyNULEscapes(body)
+
+			if bodyDecodesNUL(repaired) {
+				t.Errorf("the body repair left a value the gate still refuses\n  in:  %s\n  out: %s\n"+
+					"  why this case exists: %s", body, repaired, c.Why)
+			}
+			if c.Refused {
+				if n == 0 {
+					t.Errorf("a refused body reported no repairs: %s", body)
+				}
+			} else {
+				if n != 0 {
+					t.Errorf("an accepted body reported %d repair(s): %s", n, repaired)
+				}
+				if string(repaired) != string(body) {
+					t.Errorf("an accepted body was rewritten\n  in:  %s\n  out: %s", body, repaired)
+				}
+			}
+		})
+	}
+}
+
+// TestBodyRepairPreservesEverythingElse pins the parts of a body the repair
+// must not disturb when it DOES re-encode.
+//
+// Re-encoding is the cost of walking the decoded body, and it is only paid on
+// bodies that carry a NUL. What it must not cost is the rest of the payload: an
+// import body is full of numbers (item_number, sort_order) and text, and an
+// integer that came back as 1e+06 would be a silent data change nobody asked
+// for. json.Number is why this passes; without UseNumber it does not.
+func TestBodyRepairPreservesEverythingElse(t *testing.T) {
+	body := []byte(`{"version":1,"big":9007199254740993,"ratio":1.5,"flag":true,"nothing":null,` +
+		`"text":"a < b & c > d","list":[1,2,3],"content":"x` + textguard.EscNUL + `y"}`)
+
+	repaired, n, _ := repairBodyNULEscapes(body)
+	if n != 1 {
+		t.Fatalf("repaired %d values, want 1", n)
+	}
+
+	var got map[string]any
+	dec := json.NewDecoder(strings.NewReader(string(repaired)))
+	dec.UseNumber()
+	if err := dec.Decode(&got); err != nil {
+		t.Fatalf("repaired body does not decode: %v (%s)", err, repaired)
+	}
+
+	// The integer wider than float64 is the one that fails without UseNumber:
+	// it comes back as 9007199254740992, off by one, with nothing to see.
+	if s, _ := got["big"].(json.Number); s.String() != "9007199254740993" {
+		t.Errorf("big = %v, want 9007199254740993 — the number went through float64", got["big"])
+	}
+	if s, _ := got["ratio"].(json.Number); s.String() != "1.5" {
+		t.Errorf("ratio = %v, want 1.5", got["ratio"])
+	}
+	if got["flag"] != true {
+		t.Errorf("flag = %v, want true", got["flag"])
+	}
+	if v, present := got["nothing"]; !present || v != nil {
+		t.Errorf("nothing = %v (present=%v), want a present null", v, present)
+	}
+	// HTML-ish characters survive as themselves rather than as < escapes.
+	// Both decode the same, but SetEscapeHTML(false) keeps the payload legible
+	// for anyone who looks at it.
+	if got["text"] != "a < b & c > d" {
+		t.Errorf("text = %q, want the original", got["text"])
+	}
+	if !strings.Contains(string(repaired), "a < b & c > d") {
+		t.Errorf("HTML-ish characters were escaped on re-encode: %s", repaired)
+	}
+	if want := "x" + textguard.Replacement + "y"; got["content"] != want {
+		t.Errorf("content = %q, want %q", got["content"], want)
+	}
+}
+
+// TestRepairDeclinesADuplicateMemberBody is codex round 4's finding.
+//
+// The repair decodes into map[string]any, where a repeated member keeps only
+// the LAST value. The TYPED decode that runs next does not agree: it unmarshals
+// members in order into the same struct field, so two `workspace` objects merge
+// there and would collapse here. Repairing such a body would change what gets
+// imported, which is outside what a flag called --repair-nul may do.
+//
+// So it declines, the body is returned untouched, and the refusal says why. A
+// real export cannot contain duplicate members — json.Marshal does not emit
+// them — so this costs nothing an operator meets by accident.
+func TestRepairDeclinesADuplicateMemberBody(t *testing.T) {
+	esc := textguard.EscNUL
+	body := []byte(`{"content":"x` + esc + `y","workspace":{"name":"a"},"workspace":{"slug":"b"}}`)
+
+	// Precondition: without the duplicate this body IS repaired, so the
+	// difference below is the duplicate and not the fixture.
+	plain := []byte(`{"content":"x` + esc + `y","workspace":{"name":"a"}}`)
+	if _, n, declined := repairBodyNULEscapes(plain); n != 1 || declined != "" {
+		t.Fatalf("control: the same body without a duplicate was not repaired (n=%d declined=%q)", n, declined)
+	}
+
+	got, n, declined := repairBodyNULEscapes(body)
+	if declined == "" {
+		t.Fatalf("the repair acted on a duplicate-member body; it must decline")
+	}
+	if !strings.Contains(declined, "workspace") {
+		t.Errorf("the reason does not name the repeated member: %q", declined)
+	}
+	if n != 0 {
+		t.Errorf("reported %d repair(s) while declining", n)
+	}
+	if string(got) != string(body) {
+		t.Errorf("the body was modified while declining\n  in:  %s\n  out: %s", body, got)
+	}
+
+	// And the operator is told THAT, rather than being told the repair ran.
+	tally := &nulRepairTally{Enabled: true}
+	tally.Apply(body)
+	msg := nulRepairRemedy(tally)
+	if !strings.Contains(msg, "did not run") {
+		t.Errorf("the refusal implies the repair ran: %q", msg)
+	}
+	if !strings.Contains(msg, store.RepairNULCommand) {
+		t.Errorf("the refusal leaves no course of action: %q", msg)
+	}
+}
+
+// TestFirstDuplicateJSONKey covers the detector on its own, because the case it
+// exists for is one a decode cannot show you: by the time there is a map, the
+// duplicate is gone.
+func TestFirstDuplicateJSONKey(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+		dup  bool
+		why  string
+	}{
+		{"no duplicates", `{"a":1,"b":2}`, "", false, "the control."},
+		{"top-level duplicate", `{"a":1,"a":2}`, "a", true, "the plain case."},
+		{"duplicate nested in an object", `{"x":{"a":1,"a":2}}`, "a", true,
+			"depth matters: a nested fields object is where an import body's real content lives."},
+		{"duplicate nested in an array element", `{"items":[{"id":1},{"a":1,"a":2}]}`, "a", true,
+			"array frames must not reset the object frame's bookkeeping."},
+		{"same name in SIBLING objects is not a duplicate", `{"x":{"a":1},"y":{"a":2}}`, "", false,
+			"the false positive a single shared set of names would produce, which would decline every " +
+				"real export — items all carry `id`, `title`, `slug`.",
+		},
+		{"a name reused as a VALUE is not a key", `{"a":"a","b":"a"}`, "", false,
+			"key/value alternation: counting every string would call this a duplicate."},
+		{"a name after a nested container closes", `{"a":{"z":1},"a":2}`, "a", true,
+			"the parent's key/value alternation has to resume correctly after a nested value ends."},
+		{"array of scalars", `{"a":[1,2,3],"b":4}`, "", false, "arrays of non-objects must not confuse the walk."},
+		{"malformed input", `{"a":`, "", false,
+			"json.Valid runs first, so this answers false rather than duplicating the caller's error."},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, dup := firstDuplicateJSONKey([]byte(tc.in))
+			if dup != tc.dup || got != tc.want {
+				t.Errorf("%s\n  in: %s\n  got (%q, %v), want (%q, %v)", tc.why, tc.in, got, dup, tc.want, tc.dup)
+			}
+		})
+	}
+}

internal/server/server.go

@@ -37,6 +37,7 @@ import (
 	"github.com/PerpetualSoftware/pad/internal/models"
 	"github.com/PerpetualSoftware/pad/internal/oauth"
 	"github.com/PerpetualSoftware/pad/internal/store"
+	"github.com/PerpetualSoftware/pad/internal/textguard"
 	"github.com/PerpetualSoftware/pad/internal/watchevents"
 	"github.com/PerpetualSoftware/pad/internal/webhooks"
 )
@@ -2274,6 +2275,272 @@ func decodeJSONWithLimit(r *http.Request, v interface{}, maxBytes int64) error {
 	if err != nil {
 		return fmt.Errorf("invalid JSON: %w", err)
 	}
+	return decodeJSONBytes(raw, v)
+}
+
+// decodeJSONRepairingNUL is decodeJSONWithLimit with exactly one difference,
+// and the difference is deliberately NOT a bypass (DOC-2823 S3, BUG-2810).
+//
+// The workspace import's --repair-nul flag exists because a self-hoster whose
+// database predates the enforcement can EXPORT a workspace and then not import
+// it back: the server emits a payload it will refuse. Dave's day-54 ruling
+// ships the flag with the default staying strict.
+//
+// What the flag does is repair the body and then run the SAME gate on the
+// repaired bytes. It does not skip the gate, and it must not: a decode path
+// that does is precisely the door BUG-2803 spent thirty rounds closing, and it
+// would be reachable from the endpoint carrying the largest attacker-controlled
+// body in the product. So a value the repair cannot fix is still refused, by
+// the same function, with the same error.
+//
+// A body that is not valid JSON is left alone, so its own decode error is what
+// the caller reports: a raw NUL BYTE inside a JSON string makes the document
+// invalid, and replacing one would turn a body the decoder rejects into one it
+// accepts. Widening what parses is not this flag's job.
+//
+// The count of rewritten values, and any reason the repair declined to act,
+// are recorded on the tally the caller passes in.
+func decodeJSONRepairingNUL(r *http.Request, v interface{}, maxBytes int64, t *nulRepairTally) error {
+	raw, err := readBodyForDecode(r, maxBytes)
+	if err != nil {
+		return fmt.Errorf("invalid JSON: %w", err)
+	}
+	// The tally owns the repair, so the count and the "could not repair, and
+	// why" both come back through one object rather than through a return
+	// value the caller has to remember to record. The first version returned
+	// the count and one caller dropped it, which is how the header reported 0
+	// for an import that had rewritten a value.
+	return decodeJSONBytes(t.Apply(raw), v)
+}
+
+// repairBodyNULEscapes repairs a JSON body the way the GATE reads it, and
+// returns how many values it changed.
+//
+// IT MIRRORS bodyDecodesNUL's WALK, and the first version did not — it scanned
+// the raw bytes for a live escape, which is right for a value the gate reads at
+// the top level and wrong for the one that actually matters. An item's `fields`
+// blob travels through an export as a STRING: the stored text
+// `{"a":"x\u0000y"}` is marshalled into the body as `"{\"a\":\"x\\u0000y\"}"`,
+// with a DOUBLED backslash, which a raw scan must leave alone because at that
+// layer it is literal text. The gate refuses it anyway, because it decodes the
+// body first and re-parses that string as the document it is. So a raw-byte
+// repair left `--repair-nul` unable to fix the single most common carrier in a
+// real export, while passing every test whose fixture put the NUL in `content`
+// (codex round 1).
+//
+// The walk below is therefore the same traversal, with the same classing, one
+// verb changed: where bodyDecodesNUL asks textguard whether a value decodes to
+// a NUL, this asks textguard to repair it. Two walks of one shape in one
+// package is a risk, and the mitigation is that they are measured against the
+// same corpus in both directions rather than reviewed for similarity.
+//
+// A body that is not valid JSON is returned untouched, so its own decode error
+// is what the caller reports and a malformed body cannot be made to parse.
+// A body with nothing to repair is returned BYTE-IDENTICAL — the re-encode
+// happens only when something actually changed.
+func repairBodyNULEscapes(raw []byte) (out []byte, replaced int, declined string) {
+	if !json.Valid(raw) {
+		return raw, 0, ""
+	}
+
+	// DUPLICATE MEMBERS ARE A REFUSAL, NOT A REPAIR (codex round 4).
+	//
+	// The walk below decodes into map[string]any, where a repeated key keeps
+	// only the LAST value. The typed decode that follows does not agree: it
+	// unmarshals members in order into the same struct field, so two
+	// `"workspace"` objects MERGE there and collapse here. Repairing such a
+	// body would therefore change what gets imported, which is outside what
+	// this flag is allowed to do — the contract is "replace the NULs and
+	// nothing else".
+	//
+	// Refusing to act is the safe half of that: the body is returned untouched
+	// and the gate judges it exactly as it would without the flag. A real
+	// export cannot contain duplicate members (json.Marshal does not emit
+	// them), so this costs nothing an operator will meet by accident. Rewriting
+	// such a body faithfully needs a token-preserving pass, which is BUG-2812's
+	// token-walk and not a rider on this.
+	if key, dup := firstDuplicateJSONKey(raw); dup {
+		return raw, 0, "the payload repeats the member " + strconv.Quote(key) +
+			", and repairing it would change which value is imported"
+	}
+
+	// UseNumber, so a number wider than float64 is not silently re-emitted in
+	// scientific notation on its way back out.
+	dec := json.NewDecoder(bytes.NewReader(raw))
+	dec.UseNumber()
+	var decoded any
+	if err := dec.Decode(&decoded); err != nil {
+		return raw, 0, ""
+	}
+
+	repaired, n := repairDecodedNULs(decoded, false)
+	if n == 0 {
+		return raw, 0, ""
+	}
+
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(false)
+	if err := enc.Encode(repaired); err != nil {
+		// Re-encoding a value that came out of a decode should not fail. If it
+		// somehow does, returning the ORIGINAL leaves the gate to refuse it,
+		// which is the safe direction.
+		return raw, 0, ""
+	}
+	return bytes.TrimRight(buf.Bytes(), "\n"), n, ""
+}
+
+// firstDuplicateJSONKey reports the first object member name that appears twice
+// in the same object, at any depth.
+//
+// A token walk rather than a decode, because a decode is exactly what loses the
+// information: by the time there is a map, the duplicate is gone.
+//
+// Malformed input answers false — the caller has already checked json.Valid,
+// and a decode error there is the caller's to report, not this function's to
+// duplicate.
+func firstDuplicateJSONKey(raw []byte) (string, bool) {
+	type frame struct {
+		isObject  bool
+		seen      map[string]bool
+		expectKey bool
+	}
+	var stack []*frame
+	top := func() *frame {
+		if len(stack) == 0 {
+			return nil
+		}
+		return stack[len(stack)-1]
+	}
+
+	dec := json.NewDecoder(bytes.NewReader(raw))
+	for {
+		tok, err := dec.Token()
+		if err != nil {
+			return "", false // EOF, or malformed — nothing to report either way.
+		}
+		if d, isDelim := tok.(json.Delim); isDelim {
+			switch d {
+			case '{':
+				stack = append(stack, &frame{isObject: true, seen: map[string]bool{}, expectKey: true})
+			case '[':
+				stack = append(stack, &frame{})
+			case '}', ']':
+				if len(stack) > 0 {
+					stack = stack[:len(stack)-1]
+				}
+				// The container that just closed WAS a value of its parent, so
+				// the parent's next token is a key again.
+				if f := top(); f != nil && f.isObject {
+					f.expectKey = true
+				}
+			}
+			continue
+		}
+		f := top()
+		if f == nil || !f.isObject {
+			continue
+		}
+		if f.expectKey {
+			if name, isString := tok.(string); isString {
+				if f.seen[name] {
+					return name, true
+				}
+				f.seen[name] = true
+			}
+			f.expectKey = false
+			continue
+		}
+		// A scalar value; the next token in this object is a key.
+		f.expectKey = true
+	}
+}
+
+// repairDecodedNULs walks a decoded body, repairing every string the gate would
+// refuse, and counts the VALUES it changed.
+//
+// The count is values rather than escapes because at this layer an escape is
+// not a thing that exists any more — the outer decode has already resolved it,
+// and a nested document may carry several. "Three values were rewritten" is
+// also the sentence an operator can check against the report.
+//
+// inUserData carries the same meaning as in bodyDecodesNUL: below a
+// JSON-encoded field key nothing re-parses the strings, so they are ordinary
+// text and only a raw NUL matters.
+func repairDecodedNULs(v any, inUserData bool) (any, int) {
+	switch t := v.(type) {
+	case string:
+		// Both arms of the gate check exactly ContainsNUL here, so both repair
+		// exactly raw NULs. The escape form is only meaningful one level up,
+		// where a string is re-parsed as a document.
+		repaired := textguard.Repair(t, false)
+		if repaired == t {
+			return t, 0
+		}
+		return repaired, 1
+
+	case map[string]any:
+		out := make(map[string]any, len(t))
+		count := 0
+		for k, sub := range t {
+			// KEYS TOO. The gate refuses a NUL in a key, so a repair that only
+			// touched values would leave the body refused with nothing to show
+			// for it.
+			key := textguard.Repair(k, false)
+			if key != k {
+				count++
+			}
+
+			if !inUserData && isJSONEncodedFieldKey(k) {
+				if str, isString := sub.(string); isString {
+					// The one place the ESCAPE form is repaired: this string is
+					// re-parsed as a document, so it gets both checks, exactly
+					// as the gate gives it both.
+					repaired := textguard.Repair(str, true)
+					if repaired != str {
+						count++
+					}
+					out[key] = repaired
+					continue
+				}
+				// The field's natural shape — an object or array the server
+				// marshals itself. Everything below is caller data.
+				sr, n := repairDecodedNULs(sub, true)
+				out[key] = sr
+				count += n
+				continue
+			}
+
+			sr, n := repairDecodedNULs(sub, inUserData)
+			out[key] = sr
+			count += n
+		}
+		return out, count
+
+	case []any:
+		out := make([]any, len(t))
+		count := 0
+		for i, sub := range t {
+			sr, n := repairDecodedNULs(sub, inUserData)
+			out[i] = sr
+			count += n
+		}
+		return out, count
+
+	default:
+		// Numbers, booleans, null. json.Number is deliberately carried through
+		// untouched so it re-encodes as the literal it arrived as.
+		return v, 0
+	}
+}
+
+// decodeJSONBytes is everything decodeJSONWithLimit does once the body has been
+// read: the empty-body contract, the NUL gate, and the unmarshal.
+//
+// Extracted so the --repair-nul path can insert a repair between the read and
+// the gate WITHOUT reimplementing any of the three, which is what keeps the
+// gate the single decider.
+func decodeJSONBytes(raw []byte, v interface{}) error {
 	// An EMPTY (or whitespace-only) body must keep returning a wrapped
 	// io.EOF. json.Decoder.Decode answered io.EOF there and at least one
 	// caller depends on it — handlers_playbooks.go treats

internal/store/nul_repair_differential_test.go

@@ -0,0 +1,161 @@
+package store
+
+import (
+	"database/sql"
+	"encoding/json"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// The REPAIR legs of the four-way differential test (DOC-2823 S3).
+//
+// The four existing legs measure what each layer REFUSES. These measure that
+// every value the repair produces is one all four layers ACCEPT — which is the
+// property `pad db repair-nul` exists to deliver, and the one a repair tested
+// only against its own package could satisfy while still leaving rows that the
+// database, or Postgres, will not have.
+//
+// Three of the four live here (Layer A, Layer B, native Postgres); the HTTP
+// gate's is in internal/server, beside its own corpus leg.
+//
+// EACH LEG DRIVES THE SAME textguard.Repair the command calls. A leg that
+// repaired values with a local helper would be measuring a repair nobody ships.
+
+// TestLayerAAcceptsEveryRepairedCorpusValue — the driver guard.
+func TestLayerAAcceptsEveryRepairedCorpusValue(t *testing.T) {
+	s := testStore(t)
+	ws := createTestWorkspace(t, s, "RepairLayerA")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	item := createTestItem(t, s, ws.ID, col.ID, "Repair subject", "")
+
+	for _, c := range textguard.Corpus {
+		t.Run(c.Name, func(t *testing.T) {
+			repaired := textguard.Repair(c.Value, c.IsJSON)
+
+			// Same routing rule the refusal legs use: a JSON-classed value that
+			// is not valid JSON goes to the text column, or the fields column
+			// rejects it as malformed before the guard is consulted and the
+			// case measures SQLite's JSON parser instead.
+			var err error
+			if c.IsJSON && json.Valid([]byte(strings.TrimSpace(repaired))) {
+				_, err = s.UpdateItem(item.ID, models_ItemUpdateFields(repaired))
+			} else {
+				_, err = s.UpdateItem(item.ID, models_ItemUpdateContent(repaired))
+			}
+			if err != nil {
+				t.Fatalf("Layer A refused a REPAIRED value — the repair does not satisfy the guard it is "+
+					"meant to satisfy\n  original: %q\n  repaired: %q\n  err: %v\n  why this case exists: %s",
+					c.Value, repaired, err, c.Why)
+			}
+		})
+	}
+}
+
+// TestLayerBAcceptsEveryRepairedCorpusValue — the SQLite triggers, through a
+// raw handle so what is measured is the DATABASE's verdict rather than Layer
+// A's reflected back.
+func TestLayerBAcceptsEveryRepairedCorpusValue(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("Layer B is SQLite-only")
+	}
+	ws := createTestWorkspace(t, s, "RepairLayerB")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	item := createTestItem(t, s, ws.ID, col.ID, "Repair subject", "")
+
+	raw, err := sql.Open("sqlite", s.dbPath+"?_pragma=busy_timeout(30000)")
+	if err != nil {
+		t.Fatalf("open raw: %v", err)
+	}
+	defer raw.Close()
+
+	for _, c := range textguard.Corpus {
+		t.Run(c.Name, func(t *testing.T) {
+			repaired := textguard.Repair(c.Value, c.IsJSON)
+
+			var werr error
+			if c.IsJSON && json.Valid([]byte(strings.TrimSpace(repaired))) {
+				werr = execRaw(raw, `UPDATE workspaces SET settings = ? WHERE id = ?`, repaired, ws.ID)
+			} else {
+				werr = execRaw(raw, `UPDATE items SET content = ? WHERE id = ?`, repaired, item.ID)
+			}
+			if werr != nil {
+				t.Fatalf("Layer B refused a REPAIRED value\n  original: %q\n  repaired: %q\n  err: %v",
+					c.Value, repaired, werr)
+			}
+
+			// And it PERSISTED intact, for the reason the refusal leg reads its
+			// values back: a trigger that discarded the row would also produce
+			// no error.
+			var stored string
+			var rerr error
+			if c.IsJSON && json.Valid([]byte(strings.TrimSpace(repaired))) {
+				rerr = raw.QueryRow(`SELECT settings FROM workspaces WHERE id = ?`, ws.ID).Scan(&stored)
+			} else {
+				rerr = raw.QueryRow(`SELECT content FROM items WHERE id = ?`, item.ID).Scan(&stored)
+			}
+			if rerr != nil {
+				t.Fatalf("read back: %v", rerr)
+			}
+			if stored != repaired {
+				t.Errorf("the repaired value did not survive the round trip\n  wrote: %q\n  read:  %q",
+					repaired, stored)
+			}
+		})
+	}
+}
+
+// TestNativePostgresAcceptsEveryRepairedCorpusValue — the leg that makes the
+// repair worth running.
+//
+// BUG-2810's filing is that an affected workspace exports and will not import,
+// and that `pad db migrate-to-pg` fails partway through the copy against
+// PostgreSQL's own parser. That claim is only discharged by putting the
+// repaired values in front of a real Postgres.
+func TestNativePostgresAcceptsEveryRepairedCorpusValue(t *testing.T) {
+	dsn := os.Getenv("PAD_TEST_POSTGRES_URL")
+	if dsn == "" {
+		t.Skip("PAD_TEST_POSTGRES_URL not set; the native-Postgres leg needs a real server")
+	}
+
+	// The RAW pgx driver, not the guarded name — otherwise this measures the
+	// guard agreeing with itself.
+	db, err := sql.Open("pgx", dsn)
+	if err != nil {
+		t.Fatalf("open raw pgx: %v", err)
+	}
+	defer db.Close()
+
+	if _, err := db.Exec(`DROP TABLE IF EXISTS nul_repair_differential`); err != nil {
+		t.Fatalf("drop: %v", err)
+	}
+	if _, err := db.Exec(
+		`CREATE TABLE nul_repair_differential (id TEXT PRIMARY KEY, txt TEXT, doc JSONB)`,
+	); err != nil {
+		t.Fatalf("create: %v", err)
+	}
+	defer func() { _, _ = db.Exec(`DROP TABLE IF EXISTS nul_repair_differential`) }()
+
+	for i, c := range textguard.Corpus {
+		t.Run(c.Name, func(t *testing.T) {
+			repaired := textguard.Repair(c.Value, c.IsJSON)
+			id := "repaired-" + strings.ReplaceAll(c.Name, " ", "-") + "-" + string(rune('a'+i%26))
+
+			var err error
+			if c.IsJSON && json.Valid([]byte(strings.TrimSpace(repaired))) {
+				_, err = db.Exec(
+					`INSERT INTO nul_repair_differential (id, doc) VALUES ($1, $2)`, id, repaired)
+			} else {
+				_, err = db.Exec(
+					`INSERT INTO nul_repair_differential (id, txt) VALUES ($1, $2)`, id, repaired)
+			}
+			if err != nil {
+				t.Fatalf("PostgreSQL refused a REPAIRED value — the repair does not unblock the migration "+
+					"it exists to unblock\n  original: %q\n  repaired: %q\n  err: %v", c.Value, repaired, err)
+			}
+		})
+	}
+}

internal/store/nulrepair.go

@@ -0,0 +1,277 @@
+package store
+
+import (
+	"database/sql"
+	"errors"
+	"fmt"
+
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// The explicit operator repair for the legacy NUL population (DOC-2823 S3).
+//
+// NEVER A MIGRATION, on Dave's day-54 ruling: a migration that rewrites user
+// content decides consent for the operator. This runs only when somebody types
+// `pad db repair-nul`, and `pad db migrate-to-pg` refuses and prints that
+// command rather than repairing on the operator's behalf.
+//
+// IN GO, NOT IN SQL, for the reason TASK-2824 measured and this unit measured
+// again on the read path: SQLite's own string functions disagree about a
+// NUL-bearing value (`length()` answers 3 for an 8-byte value), so no SQL-side
+// transform can be trusted to leave the rest of the value intact. The rewrite
+// is textguard.Repair, which is also what the four enforcement layers are
+// measured against.
+
+// RepairNULCommand is the exact command an operator runs to fix the rows this
+// file repairs.
+//
+// It lives here, in the package that implements the repair, because three
+// places quote it at an operator — the migrate-to-pg preflight, the import's
+// strict refusal, and the CLI's own help — and a remedy naming a command that
+// has been renamed is worse than no remedy at all.
+const RepairNULCommand = "pad db repair-nul"
+
+// NULRepairSkip is a violation the repair deliberately did not touch.
+type NULRepairSkip struct {
+	Violation NULViolation
+	Reason    string
+}
+
+// NULRepairFailure is a violation the repair tried and could not complete.
+type NULRepairFailure struct {
+	Violation NULViolation
+	Err       error
+}
+
+// NULRepairReport is what the repair returns. Every violation the scan found
+// ends up in exactly one of the three buckets, which is what lets the CLI
+// report a total that adds up.
+type NULRepairReport struct {
+	Scan     *NULScanReport
+	Repaired []NULViolation
+	Skipped  []NULRepairSkip
+	Failed   []NULRepairFailure
+
+	// The suspect class (see NULSuspect), kept in its own buckets so the
+	// violation counts still match what `pad db scan-nul` promised.
+	//
+	// SuspectsClean is the common and boring outcome: the value carried a
+	// literal the scanner had no reason to touch.
+	SuspectsRepaired []NULSuspect
+	SuspectsClean    []NULSuspect
+	SuspectsSkipped  []NULSuspect
+	SuspectsFailed   []NULSuspectFailure
+}
+
+// NULSuspectFailure is a suspect the repair tried and could not complete.
+type NULSuspectFailure struct {
+	Suspect NULSuspect
+	Err     error
+}
+
+// RepairNUL rewrites every offending stored value, replacing each NUL with
+// U+FFFD, and reports what it changed.
+//
+// PER-ROW TRANSACTIONS, not one big one. The repair is idempotent — running it
+// twice changes nothing the second time, pinned in textguard — so a partial run
+// is a resumable state rather than a corrupt one, and that is worth more here
+// than atomicity across an unbounded number of rows: a single transaction over
+// a large database holds SQLite's write lock for the whole sweep, which on the
+// one deployment shape this exists for (a self-hoster's live instance) is the
+// difference between a repair and an outage.
+//
+// Each row IS read and written inside its own transaction, so the value cannot
+// change between the read and the rewrite.
+func (s *Store) RepairNUL() (*NULRepairReport, error) {
+	scan, err := s.ScanNUL()
+	if err != nil {
+		return nil, err
+	}
+	report := &NULRepairReport{Scan: scan}
+	if !scan.Applicable {
+		return report, nil
+	}
+
+	for _, v := range scan.Violations {
+		// THE PRIMARY KEY IS NOT REWRITTEN. Repairing a column that is part of
+		// its own row's key changes the row's identity, and U+FFFD substitution
+		// can land it on top of an existing row — for email_optouts(email), the
+		// only such column today, that would silently merge two opt-out records
+		// and could un-suppress mail to somebody. Refusing to guess is the
+		// posture the cross-workspace copy takes when a destination field needs
+		// a value; the operator is told which rows and why.
+		//
+		// It is also mechanically impossible through this handle: Layer A
+		// inspects every bound parameter, so a WHERE clause carrying the
+		// NUL-bearing key would be refused along with the write.
+		if v.KeyIncomplete {
+			report.Skipped = append(report.Skipped, NULRepairSkip{
+				Violation: v,
+				Reason: "one of the row's primary-key columns is NULL, so there is no WHERE clause that " +
+					"selects exactly this row",
+			})
+			continue
+		}
+
+		if _, isKey := v.Key[v.Column]; isKey {
+			report.Skipped = append(report.Skipped, NULRepairSkip{
+				Violation: v,
+				Reason: "the column is part of the row's primary key; repairing it would change the row's " +
+					"identity and could collide with an existing row",
+			})
+			continue
+		}
+
+		// A NUL in a key column the list does NOT protect, on a row whose
+		// violation is elsewhere. The address is then unusable for a different
+		// reason than the case above: Layer A inspects every bound parameter,
+		// including the ones in a WHERE clause, so the lookup is refused before
+		// SQLite is asked to find the row.
+		//
+		// Detected here rather than left to the driver. Without this the row
+		// lands in Failed carrying "invalid text parameter: parameter 2", which
+		// says nothing an operator can act on — the same information, phrased as
+		// a fault in the repair rather than as a property of the row (codex
+		// round 3).
+		if key, bad := nulBearingKey(v); bad {
+			report.Skipped = append(report.Skipped, NULRepairSkip{
+				Violation: v,
+				Reason: "the row's " + key + " value itself contains a NUL, so no query can address this " +
+					"row; repair or remove it by hand",
+			})
+			continue
+		}
+
+		repaired, err := s.repairOneNUL(v)
+		switch {
+		case err != nil:
+			report.Failed = append(report.Failed, NULRepairFailure{Violation: v, Err: err})
+		case repaired:
+			report.Repaired = append(report.Repaired, v)
+		default:
+			// The value was clean by the time the transaction read it —
+			// somebody else repaired it, or the row changed. Not a failure and
+			// not a repair.
+			report.Skipped = append(report.Skipped, NULRepairSkip{
+				Violation: v,
+				Reason:    "the value no longer violates the invariant; nothing to do",
+			})
+		}
+	}
+
+	// SUSPECTS, per the day-54 ruling. Most carry only a harmless literal and
+	// come back unchanged; the one shape that matters — a NUL behind a literal
+	// duplicate key — is fixed here and nowhere else, because the predicate
+	// that gates the ordinary repair cannot see it.
+	//
+	// Reported separately from Repaired so the two counts stay honest: the
+	// scan's violation count is what `pad db scan-nul` promised to change, and
+	// folding suspects into it would make the dry run disagree with the run.
+	for _, sus := range scan.Suspects {
+		if sus.KeyIncomplete {
+			report.SuspectsSkipped = append(report.SuspectsSkipped, sus)
+			continue
+		}
+		changed, err := s.RepairSuspectValue(sus)
+		switch {
+		case err != nil:
+			report.SuspectsFailed = append(report.SuspectsFailed, NULSuspectFailure{Suspect: sus, Err: err})
+		case changed:
+			report.SuspectsRepaired = append(report.SuspectsRepaired, sus)
+		default:
+			report.SuspectsClean = append(report.SuspectsClean, sus)
+		}
+	}
+	return report, nil
+}
+
+// repairOneNUL reads, rewrites and writes one value inside one transaction.
+// It reports whether it actually changed anything.
+func (s *Store) repairOneNUL(v NULViolation) (bool, error) {
+	tx, err := s.db.Begin()
+	if err != nil {
+		return false, fmt.Errorf("begin: %w", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	where, args := nulRowPredicate(v)
+	qc := quoteIdent(v.Column)
+	qt := quoteIdent(v.Table)
+
+	var value string
+	err = tx.QueryRow(fmt.Sprintf(`SELECT %s FROM %s WHERE %s`, qc, qt, where), args...).Scan(&value)
+	if errors.Is(err, sql.ErrNoRows) {
+		return false, nil
+	}
+	if err != nil {
+		return false, fmt.Errorf("read %s.%s: %w", v.Table, v.Column, err)
+	}
+
+	isJSON := nulColumnIsJSON(v.Table, v.Column)
+	if !textguard.ParameterRefused(value, isJSON) {
+		return false, nil
+	}
+
+	repaired := textguard.Repair(value, isJSON)
+	if repaired == value {
+		// textguard.Repair is required to change any refused value; a no-op
+		// here would mean the predicate and the repair disagree, and looping
+		// on it or reporting success would both be lies.
+		return false, fmt.Errorf("repair produced no change for a refused value in %s.%s", v.Table, v.Column)
+	}
+
+	updateArgs := append([]any{repaired}, args...)
+	res, err := tx.Exec(fmt.Sprintf(`UPDATE %s SET %s = ? WHERE %s`, qt, qc, where), updateArgs...)
+	if err != nil {
+		return false, fmt.Errorf("update %s.%s: %w", v.Table, v.Column, err)
+	}
+
+	// THE ROW COUNT IS CHECKED, and it is not defensive padding. The WHERE
+	// clause is built from values the scan read back out of the database, and
+	// one of them is a `rowid` for the single protected table that declares no
+	// primary key — an INTEGER column addressed with the TEXT the scan scanned
+	// it into. If any such binding ever stopped matching, this function would
+	// commit an UPDATE that touched nothing and report the value as repaired.
+	// A repair that reports success for a row it did not change is the one
+	// failure mode an operator cannot detect from the output.
+	n, err := res.RowsAffected()
+	if err != nil {
+		return false, fmt.Errorf("rows affected for %s.%s: %w", v.Table, v.Column, err)
+	}
+	if n != 1 {
+		return false, fmt.Errorf("repairing %s.%s matched %d rows, want exactly 1 — the row address the "+
+			"scan produced does not select it", v.Table, v.Column, n)
+	}
+	if err := tx.Commit(); err != nil {
+		return false, fmt.Errorf("commit: %w", err)
+	}
+	return true, nil
+}
+
+// nulRowPredicate renders the WHERE clause addressing one row, plus its args.
+func nulRowPredicate(v NULViolation) (string, []any) { return nulKeyPredicate(v.Key) }
+
+// nulColumnIsJSON answers the classing question from the shared list, so the
+// repair and the predicate cannot disagree about a column.
+func nulColumnIsJSON(table, column string) bool {
+	for _, c := range NULProtectedColumns() {
+		if c.Table == table && c.Column == column {
+			return c.Class == classJSON
+		}
+	}
+	return false
+}
+
+// nulBearingKey reports the first key column whose VALUE carries a NUL.
+//
+// Such a value cannot be bound: the store's write guard checks every parameter,
+// not only the ones being written, so a WHERE clause carrying one is refused
+// along with the statement it belongs to.
+func nulBearingKey(v NULViolation) (string, bool) {
+	for _, k := range sortedKeys(v.Key) {
+		if textguard.ContainsNUL(v.Key[k]) {
+			return k, true
+		}
+	}
+	return "", false
+}

internal/store/nulscan.go

@@ -0,0 +1,504 @@
+package store
+
+import (
+	"database/sql"
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// The counter and the repair for the NUL invariant's LEGACY population
+// (DOC-2823 S3, closing BUG-2810).
+//
+// Layers A and B stop the value being WRITTEN. Neither of them makes a row that
+// already carries one go away, and such a row is not merely untidy: its
+// workspace exports fine and re-imports 400, and `pad db migrate-to-pg` fails
+// partway through the copy against PostgreSQL's jsonb parser. This file is the
+// read-only census of that population and the explicit operator repair.
+//
+// THREE PROPERTIES, and each of them was a decision:
+//
+// ONE PREDICATE. The decision about a value is textguard.ParameterRefused with
+// isJSON taken from nulColumns — the same core the HTTP gate, Layer A and Layer
+// B share, with Layer B's column-derived classing. The SQL below narrows, it
+// never decides. A fourth implementation of "decodes to a NUL" is the thing
+// this whole cluster exists because of.
+//
+// THE COUNT IS COMPUTED IN GO, NOT IN SQL. Measured on the read path in this
+// worktree: a row planted with `bad<NUL>name` reads back into a Go string with
+// all 8 bytes and the NUL intact, while `length(name)` in the same database
+// answers 3. TASK-2824 found that C-truncation in SQLite's string functions and
+// concluded no DB-side REPAIR could be trusted; the same measurement on the
+// read path says no DB-side COUNT can be either.
+//
+// SQLITE ONLY, and not by omission. PostgreSQL cannot hold either defect —
+// SQLSTATE 22021 for a raw NUL in text, 22P05 for the escape reaching jsonb —
+// which is why Layer B was ruled not to apply there, and the four-way
+// differential test pins that native refusal. A scan of a Postgres database
+// would be a full table scan of every protected column to prove a theorem.
+// ScanNUL says so and returns an empty report rather than pretending it looked.
+
+// NULViolation is one stored value that violates the invariant.
+type NULViolation struct {
+	Table  string
+	Column string
+	// Key addresses the row: the declared primary-key columns and their
+	// values, or `rowid` for the one table that declares no primary key.
+	Key map[string]string
+	// WorkspaceID is the owning workspace, or "" for the 16 protected tables
+	// that carry no workspace_id column — see the census in nulscan_test.go.
+	WorkspaceID string
+	// RawNUL and EscapedNUL record WHICH defect the value carries. They are
+	// not exclusive: a JSON blob can hold both, and the repair has a separate
+	// pass for each.
+	RawNUL     bool
+	EscapedNUL bool
+	// KeyIncomplete marks a row one of whose key columns is NULL, so Key does
+	// not address it. SQLite permits NULL in a declared PRIMARY KEY that is
+	// neither INTEGER PRIMARY KEY nor NOT NULL, which no other engine does.
+	// The scan still REPORTS such a row — it is real and it is broken — and
+	// the repair skips it rather than issuing a WHERE that matches nothing.
+	KeyIncomplete bool
+}
+
+// String renders a violation the way the CLI reports it.
+func (v NULViolation) String() string {
+	parts := make([]string, 0, len(v.Key))
+	for _, k := range sortedKeys(v.Key) {
+		parts = append(parts, k+"="+v.Key[k])
+	}
+	kind := "raw NUL"
+	switch {
+	case v.RawNUL && v.EscapedNUL:
+		kind = "raw NUL + escaped NUL"
+	case v.EscapedNUL:
+		kind = "escaped NUL"
+	}
+	out := fmt.Sprintf("%s.%s [%s] (%s)", v.Table, v.Column, strings.Join(parts, ", "), kind)
+	if v.WorkspaceID != "" {
+		out += " workspace=" + v.WorkspaceID
+	}
+	return out
+}
+
+// NULSuspect is a value the SQL pre-filter matched and the predicate did NOT
+// refuse.
+//
+// WHY THESE ARE REPORTED AT ALL, rather than dropped as the pre-filter's
+// expected over-match (day-54 lead ruling on PR #1233). Most of them ARE that:
+// a doubled-backslash literal contains the escape's four leading characters and
+// decodes to no NUL, and refusing it is the false positive this whole predicate
+// family exists to avoid. But one shape in that set is genuinely fatal to a
+// migration and invisible to every layer — a NUL in a value shadowed by a
+// LITERAL duplicate key, which the decode drops (textguard.KnownGaps).
+//
+// Dropping the whole class silently meant the preflight was discarding
+// information it already held and then promising a migration would go through.
+// So the class is surfaced, and the DESTINATION decides: migrate-to-pg casts
+// each suspect on the target connection and refuses on the NUL SQLSTATEs. That
+// oracle is exact in both directions — no over-refusal on a literal, no miss on
+// a shadowed one — precisely because it is not a fourth opinion of ours.
+//
+// It carries NO VALUE, like NULViolation: the report travels into terminals and
+// logs, and the caller that needs the bytes reads them back by address.
+type NULSuspect struct {
+	Table       string
+	Column      string
+	Key         map[string]string
+	WorkspaceID string
+	// KeyIncomplete has the same meaning as on NULViolation: a NULL key column,
+	// so this row cannot be addressed for a read-back or a repair.
+	KeyIncomplete bool
+}
+
+// String renders a suspect the way the CLI reports it.
+func (v NULSuspect) String() string {
+	parts := make([]string, 0, len(v.Key))
+	for _, k := range sortedKeys(v.Key) {
+		parts = append(parts, k+"="+v.Key[k])
+	}
+	out := fmt.Sprintf("%s.%s [%s]", v.Table, v.Column, strings.Join(parts, ", "))
+	if v.WorkspaceID != "" {
+		out += " workspace=" + v.WorkspaceID
+	}
+	return out
+}
+
+// NULScanReport is what the counter returns.
+type NULScanReport struct {
+	// Applicable is false on PostgreSQL, where the state cannot exist. Reason
+	// says why nothing was scanned, so a zero report is never mistaken for a
+	// scan that found nothing.
+	Applicable bool
+	Reason     string
+
+	// Violations is every offending value, in table/column order. The
+	// population this exists for is legacy rows on a single self-hosted
+	// database; it is not bounded, because an operator deciding whether to
+	// repair needs the whole list and a truncated one would understate it.
+	Violations []NULViolation
+
+	// Suspects are pre-filter matches the predicate did not refuse. Most are
+	// harmless literals; one shape in the set is a migration-breaking value no
+	// layer can see. See NULSuspect.
+	Suspects []NULSuspect
+
+	// ColumnsScanned is how many of the protected columns actually exist in
+	// this database's schema, and ColumnsAbsent lists any that do not.
+	// A database at an older migration is not an error, but a scan that
+	// silently skipped columns is not a census.
+	ColumnsScanned int
+	ColumnsAbsent  []string
+}
+
+// Total is the number of offending values.
+func (r *NULScanReport) Total() int { return len(r.Violations) }
+
+// ByWorkspace groups the violation count by workspace id, with "" collecting
+// the tables that have no workspace column.
+func (r *NULScanReport) ByWorkspace() map[string]int {
+	out := map[string]int{}
+	for _, v := range r.Violations {
+		out[v.WorkspaceID]++
+	}
+	return out
+}
+
+// ByColumn groups the violation count by "table.column".
+func (r *NULScanReport) ByColumn() map[string]int {
+	out := map[string]int{}
+	for _, v := range r.Violations {
+		out[v.Table+"."+v.Column]++
+	}
+	return out
+}
+
+// ScanNUL counts and locates every stored value violating the NUL invariant.
+//
+// Read-only: it issues SELECTs and nothing else, so it is safe to run against a
+// live server and safe to run repeatedly. `pad db migrate-to-pg` calls it as a
+// preflight for exactly that reason.
+//
+// WHAT IT CANNOT DECIDE, IT REPORTS. The predicate shares the HTTP gate's
+// map-model blind spots until BUG-2812's token-walk replaces the decode —
+// today that is a JSON document with LITERAL duplicate keys, where the decode
+// keeps the last one and a NUL in a shadowed value is never seen
+// (textguard.KnownGaps). Closing that HERE is what DOC-2823 forbids: Layer A
+// "must NOT quietly fix either gap on its own", because layers disagreeing
+// about one value is the defect this whole cluster is made of, and
+// TestScanNULInheritsTheRecordedKnownGaps pins the miss.
+//
+// But the SQL pre-filter matches such a row before the predicate drops it, and
+// an earlier version of this comment recorded that as an accepted residual —
+// the scan discarding information it already held, while migrate-to-pg went on
+// promising the migration would go through. The day-54 lead ruling on PR #1233
+// corrected it: those rows become the SUSPECT class (see NULSuspect), reported
+// under their own heading, and migrate-to-pg resolves each one by casting it on
+// the DESTINATION. Nothing about what any layer REFUSES changed.
+//
+// COST, stated because an operator should not be surprised by it: one
+// unindexed scan per protected column — 131 of them today (24 JSON-classed,
+// 107 text), measured from NULProtectedColumns rather than counted by hand.
+// There is no index that would help, since the predicate is a substring search
+// over the value. That is cheap next to the migration it guards, which reads
+// every row of every table anyway.
+func (s *Store) ScanNUL() (*NULScanReport, error) {
+	if s.dialect.Driver() != DriverSQLite {
+		return &NULScanReport{
+			Applicable: false,
+			Reason: "PostgreSQL refuses these values natively (SQLSTATE 22021 for a NUL in text, " +
+				"22P05 for the escape reaching jsonb), so no stored row can carry one",
+		}, nil
+	}
+
+	live, err := s.liveColumnTypes()
+	if err != nil {
+		return nil, err
+	}
+
+	report := &NULScanReport{Applicable: true}
+	cols := NULProtectedColumns()
+	sort.Slice(cols, func(i, j int) bool {
+		if cols[i].Table != cols[j].Table {
+			return cols[i].Table < cols[j].Table
+		}
+		return cols[i].Column < cols[j].Column
+	})
+
+	addressing := map[string]tableAddressing{}
+	for _, c := range cols {
+		if !live[c.Table+"."+c.Column] {
+			report.ColumnsAbsent = append(report.ColumnsAbsent, c.Table+"."+c.Column)
+			continue
+		}
+		report.ColumnsScanned++
+
+		addr, ok := addressing[c.Table]
+		if !ok {
+			addr, err = s.addressingFor(c.Table)
+			if err != nil {
+				return nil, err
+			}
+			addressing[c.Table] = addr
+		}
+
+		found, suspects, err := s.scanColumn(c, addr)
+		if err != nil {
+			return nil, err
+		}
+		report.Violations = append(report.Violations, found...)
+		report.Suspects = append(report.Suspects, suspects...)
+	}
+	sort.Strings(report.ColumnsAbsent)
+	return report, nil
+}
+
+// tableAddressing is how a row in one table is named and attributed.
+type tableAddressing struct {
+	// KeyColumns are the declared primary-key columns, or {"rowid"} for a
+	// table that declares none. item_wiki_links is the only such table today
+	// (measured); rowid is a correct address for it because it is not
+	// declared WITHOUT ROWID.
+	KeyColumns []string
+	// HasWorkspace says whether the table carries a workspace_id.
+	HasWorkspace bool
+	// KeyIsProtected marks a table whose PRIMARY KEY is itself a protected
+	// column. Repairing such a row rewrites its identity — and can collide
+	// with an existing row — so the repair refuses it and says so.
+	// email_optouts(email) is the only one today.
+	KeyIsProtected bool
+}
+
+// addressingFor reads a table's primary key and workspace column from the live
+// schema rather than from a list, because a hand-kept table→key mapping is the
+// enumeration this cluster keeps proving unmaintainable.
+func (s *Store) addressingFor(table string) (tableAddressing, error) {
+	rows, err := s.db.Query(`SELECT name, pk FROM pragma_table_info(?)`, table)
+	if err != nil {
+		return tableAddressing{}, fmt.Errorf("table_info %s: %w", table, err)
+	}
+	defer rows.Close()
+
+	var addr tableAddressing
+	type pkCol struct {
+		name string
+		pos  int
+	}
+	var pks []pkCol
+	for rows.Next() {
+		var name string
+		var pos int
+		if err := rows.Scan(&name, &pos); err != nil {
+			return tableAddressing{}, err
+		}
+		if name == "workspace_id" {
+			addr.HasWorkspace = true
+		}
+		if pos > 0 {
+			pks = append(pks, pkCol{name, pos})
+		}
+	}
+	if err := rows.Err(); err != nil {
+		return tableAddressing{}, err
+	}
+
+	// pragma_table_info's `pk` is the 1-based position WITHIN the key, so a
+	// composite key must be ordered by it rather than by column order.
+	sort.Slice(pks, func(i, j int) bool { return pks[i].pos < pks[j].pos })
+	for _, p := range pks {
+		addr.KeyColumns = append(addr.KeyColumns, p.name)
+	}
+	if len(addr.KeyColumns) == 0 {
+		addr.KeyColumns = []string{"rowid"}
+	}
+
+	protected := map[string]bool{}
+	for _, c := range NULProtectedColumns() {
+		if c.Table == table {
+			protected[c.Column] = true
+		}
+	}
+	for _, k := range addr.KeyColumns {
+		if protected[k] {
+			addr.KeyIsProtected = true
+		}
+	}
+	return addr, nil
+}
+
+// scanColumn narrows in SQL and decides in Go.
+//
+// The WHERE clause is a PRE-FILTER on the two byte patterns a violating value
+// must contain, and it is the same pre-filter textguard applies internally
+// before it pays for a decode. It can only ever return a superset:
+//
+//   - `instr(col, char(0))` finds a raw NUL. Measured (TASK-2824, and again on
+//     the read path here): instr searches the whole stored value, unlike
+//     length(), which stops at the NUL.
+//   - `instr(col, '\u00')` finds the four characters every NUL escape starts
+//     with. Only JSON-classed columns get it: in a text column those six
+//     characters are six characters, and refusing them there is the false
+//     positive that made BUG-2803's parity pre-filter unsound.
+//
+// Whether the match MEANS anything is then textguard's call, on the value read
+// into Go — which is what keeps `{"a":"x\\u0000y"}` out of the report.
+func (s *Store) scanColumn(c nulColumn, addr tableAddressing) ([]NULViolation, []NULSuspect, error) {
+	qt := quoteIdent(c.Table)
+	qc := quoteIdent(c.Column)
+
+	sel := make([]string, 0, len(addr.KeyColumns)+2)
+	for _, k := range addr.KeyColumns {
+		sel = append(sel, quoteIdent(k))
+	}
+	if addr.HasWorkspace {
+		sel = append(sel, `"workspace_id"`)
+	}
+	sel = append(sel, qc)
+
+	where := fmt.Sprintf(`%s IS NOT NULL AND (instr(%s, char(0)) > 0`, qc, qc)
+	args := []any{}
+	if c.Class == classJSON {
+		where += fmt.Sprintf(` OR instr(%s, ?) > 0`, qc)
+		args = append(args, nulEscapePrefix)
+	}
+	where += `)`
+
+	q := fmt.Sprintf(`SELECT %s FROM %s WHERE %s`, strings.Join(sel, ", "), qt, where)
+	rows, err := s.db.Query(q, args...)
+	if err != nil {
+		return nil, nil, fmt.Errorf("scan %s.%s: %w", c.Table, c.Column, err)
+	}
+	defer rows.Close()
+
+	var out []NULViolation
+	var suspects []NULSuspect
+	for rows.Next() {
+		// EVERY column is scanned as a NULLABLE string, and that is not
+		// defensive padding — a plain *string fails with "converting NULL to
+		// string is unsupported" on the first row it meets, and the whole scan
+		// (and therefore the repair, and the migrate-to-pg preflight) fails
+		// with it. Two of the three can be NULL on an ordinary database:
+		// workspace_id is nullable on several protected tables, and SQLite —
+		// unlike every other engine — permits NULL in a PRIMARY KEY column
+		// that is not INTEGER PRIMARY KEY or explicitly NOT NULL. Only the
+		// value column is guaranteed non-NULL, by the query's own WHERE.
+		dest := make([]any, 0, len(sel))
+		keyVals := make([]sql.NullString, len(addr.KeyColumns))
+		for i := range keyVals {
+			dest = append(dest, &keyVals[i])
+		}
+		var wsID sql.NullString
+		if addr.HasWorkspace {
+			dest = append(dest, &wsID)
+		}
+		var value string
+		dest = append(dest, &value)
+
+		if err := rows.Scan(dest...); err != nil {
+			return nil, nil, fmt.Errorf("scan %s.%s row: %w", c.Table, c.Column, err)
+		}
+
+		isJSON := c.Class == classJSON
+		if !textguard.ParameterRefused(value, isJSON) {
+			// The pre-filter matched and the predicate did not. Usually that is
+			// the doubled-backslash literal, and it is EXPECTED — the
+			// pre-filter is allowed to be a superset and is worthless if it is
+			// not.
+			//
+			// It is RECORDED rather than dropped because one member of that set
+			// is not harmless, and no layer of ours can tell which (see
+			// NULSuspect). Only JSON-classed columns can produce one: a text
+			// column's pre-filter is the raw-NUL check alone, and a raw NUL is
+			// always a violation, never a suspect.
+			if isJSON {
+				sus := NULSuspect{
+					Table:         c.Table,
+					Column:        c.Column,
+					Key:           map[string]string{},
+					WorkspaceID:   wsID.String,
+					KeyIncomplete: false,
+				}
+				for i, k := range addr.KeyColumns {
+					if !keyVals[i].Valid {
+						sus.KeyIncomplete = true
+						continue
+					}
+					sus.Key[k] = keyVals[i].String
+				}
+				suspects = append(suspects, sus)
+			}
+			continue
+		}
+
+		v := NULViolation{
+			Table:       c.Table,
+			Column:      c.Column,
+			Key:         map[string]string{},
+			WorkspaceID: wsID.String,
+			RawNUL:      textguard.ContainsNUL(value),
+		}
+		v.EscapedNUL = isJSON && textguard.DocumentDecodesNULAnyShape(value)
+		for i, k := range addr.KeyColumns {
+			if !keyVals[i].Valid {
+				// A NULL key column cannot address the row for an UPDATE
+				// (`WHERE k = NULL` matches nothing), so the repair must not
+				// be handed one. Reported, with the address it could build, so
+				// the operator sees the row exists rather than having it
+				// vanish from a census.
+				v.KeyIncomplete = true
+				continue
+			}
+			v.Key[k] = keyVals[i].String
+		}
+		out = append(out, v)
+	}
+	return out, suspects, rows.Err()
+}
+
+// nulEscapePrefix is the four characters every NUL escape begins with, built
+// rather than typed: typing them produces the CHARACTER in a Go source file,
+// which is the decay corpus.go records and which happened twice while writing
+// this unit.
+var nulEscapePrefix = textguard.EscNUL[:4]
+
+// liveColumnTypes reports which table.column pairs actually exist, so a
+// database at an older migration is scanned for what it has rather than
+// erroring on a column the list knows about and the schema does not.
+func (s *Store) liveColumnTypes() (map[string]bool, error) {
+	rows, err := s.db.Query(`
+		SELECT m.name, ti.name
+		FROM sqlite_master m, pragma_table_info(m.name) ti
+		WHERE m.type = 'table'
+	`)
+	if err != nil {
+		return nil, fmt.Errorf("read live schema: %w", err)
+	}
+	defer rows.Close()
+	out := map[string]bool{}
+	for rows.Next() {
+		var table, col string
+		if err := rows.Scan(&table, &col); err != nil {
+			return nil, err
+		}
+		out[table+"."+col] = true
+	}
+	return out, rows.Err()
+}
+
+// quoteIdent quotes a SQL identifier. The names come from this package's own
+// list rather than from user input, but the trigger restoration learned that
+// quoting is worth having anyway when identifiers are interpolated at all.
+func quoteIdent(name string) string {
+	return `"` + strings.ReplaceAll(name, `"`, `""`) + `"`
+}
+
+func sortedKeys(m map[string]string) []string {
+	out := make([]string, 0, len(m))
+	for k := range m {
+		out = append(out, k)
+	}
+	sort.Strings(out)
+	return out
+}

internal/store/nulscan_test.go

@@ -0,0 +1,774 @@
+package store
+
+import (
+	"database/sql"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// dropAllNULTriggers turns the store's database into a pre-S2 one for the
+// duration of a test, so a raw handle can plant the LEGACY rows this unit
+// exists to find. Restores them before returning to the caller's control.
+//
+// A raw *sql.DB opened on the same file is what an old binary is: no Layer A
+// wrapper. With the triggers gone too, neither enforcement layer is present,
+// which is exactly the window BUG-2813 describes and the state BUG-2810's rows
+// were written in.
+func plantLegacyRows(t *testing.T, s *Store, plant func(raw *sql.DB)) {
+	t.Helper()
+
+	raw, err := sql.Open("sqlite", s.dbPath+"?_pragma=busy_timeout(30000)")
+	if err != nil {
+		t.Fatalf("open raw: %v", err)
+	}
+	defer raw.Close()
+
+	names, err := nulTriggersIn(raw)
+	if err != nil {
+		t.Fatalf("list triggers: %v", err)
+	}
+	for name := range names {
+		if _, err := raw.Exec(`DROP TRIGGER IF EXISTS "` + name + `"`); err != nil {
+			t.Fatalf("drop %s: %v", name, err)
+		}
+	}
+
+	plant(raw)
+
+	// Put the database back the way a real one is BEFORE the scan runs. A scan
+	// measured against a database with no triggers would not be measuring the
+	// deployment it exists for, and the repair's writes have to pass Layer B.
+	if _, err := s.ensureNULTriggersReporting(); err != nil {
+		t.Fatalf("restore triggers: %v", err)
+	}
+}
+
+// TestScanNULFindsThePlantedPopulation drives every shape the counter has to
+// tell apart, including the two it must NOT report.
+func TestScanNULFindsThePlantedPopulation(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("the scan is SQLite-only; Postgres cannot hold the state")
+	}
+
+	ws := createTestWorkspace(t, s, "ScanWS")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	dirtyText := createTestItem(t, s, ws.ID, col.ID, "clean title", "clean body")
+	dirtyJSON := createTestItem(t, s, ws.ID, col.ID, "second", "body")
+	literalOnly := createTestItem(t, s, ws.ID, col.ID, "third", "body")
+	cleanItem := createTestItem(t, s, ws.ID, col.ID, "fourth", "body")
+
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		// (1) raw NUL in a TEXT column.
+		mustExec(t, raw, `UPDATE items SET title = ? WHERE id = ?`,
+			"bad"+textguard.NUL+"title", dirtyText.ID)
+		// (2) live escape in a JSON column.
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`,
+			`{"note":"x`+esc+`y"}`, dirtyJSON.ID)
+		// (3) raw NUL inside a JSON column — a defect of the stored bytes
+		//     rather than of the document, and a different repair pass.
+		mustExec(t, raw, `UPDATE items SET tags = ? WHERE id = ?`,
+			`["a`+textguard.NUL+`b"]`, dirtyJSON.ID)
+		// (4) THE NEGATIVE CONTROL. A doubled backslash makes the six
+		//     characters literal text: the SQL pre-filter matches it and the
+		//     predicate must not. A scan that reports this row has a false
+		//     positive of exactly the kind BUG-2803's parity filter had.
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`,
+			`{"note":"x`+backslash+esc+`y"}`, literalOnly.ID)
+		// (5) THE OTHER NEGATIVE CONTROL. The escape in a TEXT column is six
+		//     ordinary characters and is not a violation anywhere.
+		mustExec(t, raw, `UPDATE items SET content = ? WHERE id = ?`,
+			"writing about "+esc+" in a doc", literalOnly.ID)
+		// (6) A table with NO declared primary key, addressed by rowid.
+		mustExec(t, raw,
+			`INSERT INTO item_wiki_links (source_item_id, target_kind, target_title, position)
+			 VALUES (?, 'title', ?, 0)`,
+			cleanItem.ID, "link"+textguard.NUL+"title")
+	})
+
+	report, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan: %v", err)
+	}
+	if !report.Applicable {
+		t.Fatalf("scan reported not applicable on SQLite: %s", report.Reason)
+	}
+
+	// Every protected column must exist in a freshly migrated database. A
+	// non-empty ColumnsAbsent here means the list names something the schema
+	// does not have, which the scan would silently skip.
+	if len(report.ColumnsAbsent) != 0 {
+		t.Errorf("protected columns missing from the live schema: %v", report.ColumnsAbsent)
+	}
+	if want := len(NULProtectedColumns()); report.ColumnsScanned != want {
+		t.Errorf("scanned %d columns, list has %d", report.ColumnsScanned, want)
+	}
+
+	found := map[string]NULViolation{}
+	for _, v := range report.Violations {
+		found[v.Table+"."+v.Column] = v
+	}
+
+	for _, want := range []struct {
+		key        string
+		rawNUL     bool
+		escapedNUL bool
+		workspace  string
+	}{
+		{"items.title", true, false, ws.ID},
+		{"items.fields", false, true, ws.ID},
+		{"items.tags", true, false, ws.ID},
+		{"item_wiki_links.target_title", true, false, ""},
+	} {
+		v, ok := found[want.key]
+		if !ok {
+			t.Errorf("%s: not reported", want.key)
+			continue
+		}
+		if v.RawNUL != want.rawNUL || v.EscapedNUL != want.escapedNUL {
+			t.Errorf("%s: kind mismatch — raw=%v escaped=%v, want raw=%v escaped=%v",
+				want.key, v.RawNUL, v.EscapedNUL, want.rawNUL, want.escapedNUL)
+		}
+		if v.WorkspaceID != want.workspace {
+			t.Errorf("%s: workspace %q, want %q", want.key, v.WorkspaceID, want.workspace)
+		}
+	}
+
+	// items.content carried the escape as ordinary text and must not appear;
+	// nor may the doubled-backslash document, which lives in items.fields and
+	// would show up as a SECOND items.fields violation.
+	if _, reported := found["items.content"]; reported {
+		t.Error("items.content reported: the escape in a TEXT column is six ordinary characters")
+	}
+	fieldsCount := report.ByColumn()["items.fields"]
+	if fieldsCount != 1 {
+		t.Errorf("items.fields reported %d times, want exactly 1 — the doubled-backslash document "+
+			"matches the SQL pre-filter and must be dropped by the predicate", fieldsCount)
+	}
+
+	// The rowid-addressed row must carry a usable address.
+	if v, ok := found["item_wiki_links.target_title"]; ok {
+		if _, hasRowid := v.Key["rowid"]; !hasRowid {
+			t.Errorf("item_wiki_links declares no primary key; expected a rowid address, got %v", v.Key)
+		}
+	}
+
+	if got := report.ByWorkspace()[ws.ID]; got != 3 {
+		t.Errorf("workspace %s has %d violations, want 3", ws.ID, got)
+	}
+}
+
+// TestRepairNULRepairsThePopulationAndIsIdempotent is the other half: after the
+// repair the database satisfies the invariant, and the values that were never
+// violating are byte-identical.
+func TestRepairNULRepairsThePopulationAndIsIdempotent(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	ws := createTestWorkspace(t, s, "RepairWS")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	dirty := createTestItem(t, s, ws.ID, col.ID, "clean", "body")
+	literalOnly := createTestItem(t, s, ws.ID, col.ID, "second", "body")
+
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+	literalDoc := `{"note":"x` + backslash + esc + `y"}`
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		mustExec(t, raw, `UPDATE items SET title = ? WHERE id = ?`, "bad"+textguard.NUL+"title", dirty.ID)
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`, `{"note":"x`+esc+`y"}`, dirty.ID)
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`, literalDoc, literalOnly.ID)
+	})
+
+	report, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+	if len(report.Failed) != 0 {
+		t.Fatalf("repair failures: %+v", report.Failed)
+	}
+	if len(report.Repaired) != 2 {
+		t.Errorf("repaired %d values, want 2 (%+v)", len(report.Repaired), report.Repaired)
+	}
+
+	// The database now satisfies the invariant.
+	after, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("re-scan: %v", err)
+	}
+	if after.Total() != 0 {
+		t.Errorf("scan after repair still reports %d violations: %v", after.Total(), after.Violations)
+	}
+
+	// The repaired values carry U+FFFD where the NUL was, and nothing else
+	// changed. Asserting the CONTENT rather than only the count is what stops a
+	// repair that blanks the column from passing.
+	var title, fields string
+	if err := s.db.QueryRow(`SELECT title, fields FROM items WHERE id = ?`, dirty.ID).
+		Scan(&title, &fields); err != nil {
+		t.Fatalf("read repaired row: %v", err)
+	}
+	if want := "bad" + textguard.Replacement + "title"; title != want {
+		t.Errorf("title = %q, want %q", title, want)
+	}
+	if want := `{"note":"x` + textguard.ReplacementEscape + `y"}`; fields != want {
+		t.Errorf("fields = %q, want %q", fields, want)
+	}
+
+	// The row that only ever held literal text is untouched, BYTE FOR BYTE.
+	var untouched string
+	if err := s.db.QueryRow(`SELECT fields FROM items WHERE id = ?`, literalOnly.ID).Scan(&untouched); err != nil {
+		t.Fatalf("read literal row: %v", err)
+	}
+	if untouched != literalDoc {
+		t.Errorf("a value nobody complained about was rewritten\n  before: %q\n  after:  %q", literalDoc, untouched)
+	}
+
+	// Running it again does nothing, which is what makes a partial run
+	// resumable rather than a state somebody has to reason about.
+	second, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("second repair: %v", err)
+	}
+	if len(second.Repaired) != 0 || len(second.Failed) != 0 {
+		t.Errorf("second pass was not a no-op: repaired=%d failed=%+v", len(second.Repaired), second.Failed)
+	}
+}
+
+// TestRepairNULRefusesToRewriteAPrimaryKey pins the one shape the repair
+// deliberately leaves alone.
+//
+// email_optouts(email) is both the primary key and a protected column. A
+// U+FFFD substitution there rewrites the row's identity and can land on top of
+// an existing row — silently merging two opt-out records, which in this table
+// means somebody starts receiving mail again.
+func TestRepairNULRefusesToRewriteAPrimaryKey(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	bad := "someone" + textguard.NUL + "@example.com"
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		mustExec(t, raw, `INSERT INTO email_optouts (email) VALUES (?)`, bad)
+	})
+
+	report, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+
+	var skipped *NULRepairSkip
+	for i := range report.Skipped {
+		if report.Skipped[i].Violation.Table == "email_optouts" {
+			skipped = &report.Skipped[i]
+		}
+	}
+	if skipped == nil {
+		t.Fatalf("email_optouts.email was not skipped; buckets: repaired=%+v skipped=%+v failed=%+v",
+			report.Repaired, report.Skipped, report.Failed)
+	}
+	if !strings.Contains(skipped.Reason, "primary key") {
+		t.Errorf("skip reason does not name the cause: %q", skipped.Reason)
+	}
+
+	// And the row is genuinely untouched, rather than reported as skipped
+	// while having been written anyway.
+	var count int
+	if err := s.db.QueryRow(
+		`SELECT COUNT(*) FROM email_optouts WHERE instr(email, char(0)) > 0`,
+	).Scan(&count); err != nil {
+		t.Fatalf("verify: %v", err)
+	}
+	if count != 1 {
+		t.Errorf("the skipped row was modified: %d rows still carry the NUL, want 1", count)
+	}
+}
+
+// TestScanNULOnPostgresSaysWhyItDidNotLook guards the one thing a zero report
+// must never be mistaken for.
+func TestScanNULOnPostgresSaysWhyItDidNotLook(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() == DriverSQLite {
+		t.Skip("this leg is about the Postgres arm")
+	}
+	report, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan: %v", err)
+	}
+	if report.Applicable {
+		t.Error("the scan claims to have scanned a Postgres database")
+	}
+	if report.Reason == "" {
+		t.Error("a not-applicable report with no reason is indistinguishable from a clean one")
+	}
+	if report.Total() != 0 {
+		t.Errorf("not-applicable report carries %d violations", report.Total())
+	}
+}
+
+func mustExec(t *testing.T, db *sql.DB, query string, args ...any) {
+	t.Helper()
+	if _, err := db.Exec(query, args...); err != nil {
+		t.Fatalf("exec %s: %v", query, err)
+	}
+}
+
+// TestScanCostFiguresMatchTheList keeps ScanNUL's doc comment honest.
+//
+// That comment tells an operator how much work the scan is — one unindexed
+// scan per protected column — and quotes the number. A figure in prose that
+// nothing checks is a figure that silently stops being true the next time a
+// column joins the list, which in this cluster has happened in almost every
+// round. If this fails, update the comment rather than the numbers here: the
+// list is the source and the comment is the copy.
+func TestScanCostFiguresMatchTheList(t *testing.T) {
+	const (
+		wantTotal = 131
+		wantJSON  = 24
+	)
+
+	cols := NULProtectedColumns()
+	gotJSON := 0
+	for _, c := range cols {
+		if c.Class == classJSON {
+			gotJSON++
+		}
+	}
+
+	if len(cols) != wantTotal || gotJSON != wantJSON {
+		t.Errorf("ScanNUL's doc comment says %d protected columns (%d JSON, %d text); the list now has "+
+			"%d (%d JSON, %d text). Update the comment.",
+			wantTotal, wantJSON, wantTotal-wantJSON, len(cols), gotJSON, len(cols)-gotJSON)
+	}
+}
+
+// TestRepairNULAddressesARowidOnlyTable covers the one row shape whose ADDRESS
+// is not an id.
+//
+// item_wiki_links declares no primary key, so the scan addresses it by `rowid`
+// — an INTEGER column, scanned into a Go string and bound back as one. That
+// round trip is the only place in this unit where the address could silently
+// stop selecting the row, and an UPDATE matching nothing would otherwise commit
+// and be reported as a repair.
+//
+// Driven separately from the main repair test because the main one plants only
+// id-addressed rows, and a leg that never exercises the rowid path would let
+// that binding break without a failure.
+func TestRepairNULAddressesARowidOnlyTable(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	ws := createTestWorkspace(t, s, "RowidWS")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	item := createTestItem(t, s, ws.ID, col.ID, "linked", "body")
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		// Two rows, so a repair that matched the WRONG one — or all of them —
+		// is distinguishable from a repair that matched its own.
+		mustExec(t, raw,
+			`INSERT INTO item_wiki_links (source_item_id, target_kind, target_title, position)
+			 VALUES (?, 'title', ?, 0)`, item.ID, "bad"+textguard.NUL+"link")
+		mustExec(t, raw,
+			`INSERT INTO item_wiki_links (source_item_id, target_kind, target_title, position)
+			 VALUES (?, 'title', ?, 1)`, item.ID, "innocent bystander")
+	})
+
+	report, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+	if len(report.Failed) != 0 {
+		t.Fatalf("repair failures: %+v", report.Failed)
+	}
+	if len(report.Repaired) != 1 {
+		t.Fatalf("repaired %d values, want 1: %+v", len(report.Repaired), report.Repaired)
+	}
+	if _, addressed := report.Repaired[0].Key["rowid"]; !addressed {
+		t.Errorf("the row was addressed by %v, not by rowid", report.Repaired[0].Key)
+	}
+
+	var repaired, bystander string
+	if err := s.db.QueryRow(
+		`SELECT target_title FROM item_wiki_links WHERE position = 0`).Scan(&repaired); err != nil {
+		t.Fatalf("read repaired: %v", err)
+	}
+	if err := s.db.QueryRow(
+		`SELECT target_title FROM item_wiki_links WHERE position = 1`).Scan(&bystander); err != nil {
+		t.Fatalf("read bystander: %v", err)
+	}
+	if want := "bad" + textguard.Replacement + "link"; repaired != want {
+		t.Errorf("repaired title = %q, want %q", repaired, want)
+	}
+	if bystander != "innocent bystander" {
+		t.Errorf("the neighbouring row was rewritten too: %q", bystander)
+	}
+}
+
+// TestScanNULSurvivesANullWorkspaceID is a regression test for a scan that
+// could not run on the databases it exists for.
+//
+// Several protected tables carry a NULLABLE workspace_id — activities,
+// api_tokens, mcp_audit_log — and the scan selected it into a plain *string,
+// which fails with "converting NULL to string is unsupported" and takes the
+// whole scan down with it, along with the repair and the migrate-to-pg
+// preflight that call it. The failure needs a VIOLATING row in such a table, so
+// it was invisible to every fixture that planted its rows in items.
+//
+// Verified to fail against the unfixed code: the scan returned
+// `scan activities.actor row: sql: Scan error ... converting NULL to string`.
+func TestScanNULSurvivesANullWorkspaceID(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		// workspace_id omitted, so it is NULL — the shape an instance-wide
+		// activity row has.
+		mustExec(t, raw,
+			`INSERT INTO activities (id, action, actor, source, metadata, created_at)
+			 VALUES ('act-nul', 'created', ?, 'cli', '{}', '2026-01-01')`,
+			"agent"+textguard.NUL+"name")
+	})
+
+	report, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan failed on a row with a NULL workspace_id: %v", err)
+	}
+
+	var found *NULViolation
+	for i := range report.Violations {
+		if report.Violations[i].Table == "activities" && report.Violations[i].Column == "actor" {
+			found = &report.Violations[i]
+		}
+	}
+	if found == nil {
+		t.Fatalf("activities.actor not reported: %v", report.Violations)
+	}
+	if found.WorkspaceID != "" {
+		t.Errorf("workspace attribution = %q, want empty for a NULL workspace_id", found.WorkspaceID)
+	}
+	if found.Key["id"] != "act-nul" {
+		t.Errorf("row address = %v, want id=act-nul", found.Key)
+	}
+
+	// And it repairs, so the NULL only affected attribution rather than
+	// addressing.
+	rep, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+	if len(rep.Failed) != 0 {
+		t.Fatalf("repair failures: %+v", rep.Failed)
+	}
+	var actor string
+	if err := s.db.QueryRow(`SELECT actor FROM activities WHERE id = 'act-nul'`).Scan(&actor); err != nil {
+		t.Fatalf("read back: %v", err)
+	}
+	if want := "agent" + textguard.Replacement + "name"; actor != want {
+		t.Errorf("actor = %q, want %q", actor, want)
+	}
+}
+
+// TestRepairNULSkipsARowItCannotAddress covers the second way a row's address
+// can be unusable: a NUL in a key column the LIST does not protect, on a row
+// whose violation is somewhere else.
+//
+// The address is then unbindable — Layer A checks every parameter, including a
+// WHERE clause's — so the repair explains it instead of letting the driver
+// return "invalid text parameter: parameter 2", which is the same information
+// phrased as a fault in the repair rather than a property of the row.
+//
+// platform_settings is the fixture because its key IS its primary key and is
+// NOT in the protected list, while its `value` column is: exactly the shape.
+func TestRepairNULSkipsARowItCannotAddress(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		mustExec(t, raw,
+			`INSERT INTO platform_settings (key, value, updated_at) VALUES (?, ?, '2026-01-01')`,
+			"branding"+textguard.NUL+"key", "site"+textguard.NUL+"name")
+	})
+
+	report, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+
+	if len(report.Failed) != 0 {
+		t.Errorf("the row was reported as a FAILURE rather than an explained skip: %+v", report.Failed)
+	}
+
+	var skipped *NULRepairSkip
+	for i := range report.Skipped {
+		if report.Skipped[i].Violation.Table == "platform_settings" {
+			skipped = &report.Skipped[i]
+		}
+	}
+	if skipped == nil {
+		t.Fatalf("platform_settings.value not skipped; buckets: repaired=%+v skipped=%+v failed=%+v",
+			report.Repaired, report.Skipped, report.Failed)
+	}
+	if !strings.Contains(skipped.Reason, "contains a NUL") || !strings.Contains(skipped.Reason, "address") {
+		t.Errorf("the skip reason does not explain what an operator is looking at: %q", skipped.Reason)
+	}
+
+	// And it is genuinely untouched, rather than reported as skipped while
+	// having been written anyway.
+	var count int
+	if err := s.db.QueryRow(
+		`SELECT COUNT(*) FROM platform_settings WHERE instr(value, char(0)) > 0`).Scan(&count); err != nil {
+		t.Fatalf("verify: %v", err)
+	}
+	if count != 1 {
+		t.Errorf("the skipped row was modified: %d rows still carry the NUL, want 1", count)
+	}
+}
+
+// TestScanNULInheritsTheRecordedKnownGaps pins a MISS, on purpose.
+//
+// textguard.KnownGaps are values every layer currently answers wrong together —
+// today, a JSON document with LITERAL duplicate keys, where the decode keeps
+// the last one and a NUL in the first is never seen. DOC-2823 requires the
+// layers to share that blind spot rather than diverge, and says Layer A "must
+// NOT quietly fix either gap on its own". The scan shares the same predicate,
+// so it inherits it, and that is the designed behaviour rather than an
+// oversight (raised as a finding in codex round 3).
+//
+// The CONSEQUENCE is handled elsewhere rather than accepted here. PostgreSQL
+// refuses such a value, so letting the class fall on the floor would have meant
+// the migrate-to-pg preflight promising a migration that then failed mid-copy —
+// which is what an earlier version of this unit did, until the day-54 ruling on
+// PR #1233. Those rows are now the SUSPECT class, and the destination decides
+// them. What stays true, and is what this test asserts, is that the PREDICATE
+// still does not see them: the fix is a second mechanism beside the predicate,
+// not a change to it.
+//
+// This test fails when the gap CLOSES, which is the signal that BUG-2812 has
+// landed and this file should move the case into the covered set — the same
+// direction, and the same reason, as textguard's TestKnownGapsStillGap. See
+// also TestSuspectsCollapseWhenBUG2812Lands, which names what to delete.
+func TestScanNULInheritsTheRecordedKnownGaps(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+	if len(textguard.KnownGaps) == 0 {
+		t.Skip("no recorded gaps")
+	}
+
+	ws := createTestWorkspace(t, s, "GapWS")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+
+	for _, gap := range textguard.KnownGaps {
+		if !gap.IsJSON {
+			continue
+		}
+		t.Run(gap.Name, func(t *testing.T) {
+			item := createTestItem(t, s, ws.ID, col.ID, "gap subject", "")
+			plantLegacyRows(t, s, func(raw *sql.DB) {
+				if _, err := raw.Exec(
+					`UPDATE items SET fields = ? WHERE id = ?`, gap.Value, item.ID); err != nil {
+					t.Skipf("the gap value is not storable in this column: %v", err)
+				}
+			})
+
+			report, err := s.ScanNUL()
+			if err != nil {
+				t.Fatalf("scan: %v", err)
+			}
+			for _, v := range report.Violations {
+				if v.Table == "items" && v.Column == "fields" && v.Key["id"] == item.ID {
+					t.Fatalf("the scan now DETECTS a recorded known gap (%s). That is good news and this "+
+						"test is the notification: BUG-2812 has landed, so move this case into the "+
+						"covered set and drop the residual note from ScanNUL's doc comment and "+
+						"docs/backup.md.\n  why the gap exists: %s", gap.Name, gap.Why)
+				}
+			}
+		})
+	}
+}
+
+// TestScanNULReportsSuspectsSeparately covers the class the day-54 ruling
+// added: pre-filter matches the predicate does not refuse.
+//
+// The two members that matter are opposites, and the whole point is that NO
+// CHECK HERE can tell them apart — a doubled-backslash literal and a NUL hidden
+// behind a repeated key both decode to no NUL. Both are listed; the destination
+// decides. This test pins that both LAND in the suspect list and neither lands
+// in the violations, which is what makes the preflight's oracle reachable.
+func TestScanNULReportsSuspectsSeparately(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	ws := createTestWorkspace(t, s, "SuspectWS")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	literal := createTestItem(t, s, ws.ID, col.ID, "literal", "body")
+	shadowed := createTestItem(t, s, ws.ID, col.ID, "shadowed", "body")
+	clean := createTestItem(t, s, ws.ID, col.ID, "clean", "body")
+
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`,
+			`{"note":"x`+backslash+esc+`y"}`, literal.ID)
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`,
+			`{"a":"`+esc+`","a":"clean"}`, shadowed.ID)
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`,
+			`{"note":"ordinary"}`, clean.ID)
+	})
+
+	report, err := s.ScanNUL()
+	if err != nil {
+		t.Fatalf("scan: %v", err)
+	}
+
+	if report.Total() != 0 {
+		t.Errorf("a suspect was reported as a VIOLATION — the predicate has changed, which is what "+
+			"DOC-2823 forbids doing in one layer: %v", report.Violations)
+	}
+
+	got := map[string]bool{}
+	for _, sus := range report.Suspects {
+		got[sus.Key["id"]] = true
+		if sus.Table != "items" || sus.Column != "fields" {
+			t.Errorf("unexpected suspect column %s.%s", sus.Table, sus.Column)
+		}
+		if sus.WorkspaceID != ws.ID {
+			t.Errorf("suspect %v has workspace %q, want %q", sus.Key, sus.WorkspaceID, ws.ID)
+		}
+	}
+	if !got[literal.ID] {
+		t.Error("the doubled-backslash literal is not listed as a suspect")
+	}
+	if !got[shadowed.ID] {
+		t.Error("the shadowed-duplicate row is not listed as a suspect — this is the row the preflight " +
+			"exists to catch, and dropping it here is the defect the ruling corrects")
+	}
+	if got[clean.ID] {
+		t.Error("a value with no escape at all was listed as a suspect; the pre-filter is over-matching")
+	}
+}
+
+// TestRepairSuspectFixesOnlyTheFatalShape is the measurement the ruling asked
+// for, turned into a guard.
+//
+// MEASURED FIRST, and the answer decided the design: `textguard.Repair` leaves
+// the shadowed-duplicate value completely untouched, because its scanner is
+// gated on a map-model question that answers false for exactly this shape. So a
+// preflight that refused the row and pointed at `pad db repair-nul` would have
+// been pointing at a command that does nothing to it. The repair reaches the
+// class through the token-level scanner instead.
+//
+// The literal leg is the other half and the one that would fail a careless fix:
+// a repair broad enough to catch the shadowed value must still leave a value
+// that merely writes ABOUT the escape byte-identical.
+func TestRepairSuspectFixesOnlyTheFatalShape(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverSQLite {
+		t.Skip("SQLite only")
+	}
+
+	ws := createTestWorkspace(t, s, "SuspectRepairWS")
+	col := createTestCollection(t, s, ws.ID, "Tasks")
+	literal := createTestItem(t, s, ws.ID, col.ID, "literal", "body")
+	shadowed := createTestItem(t, s, ws.ID, col.ID, "shadowed", "body")
+
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+	literalDoc := `{"note":"x` + backslash + esc + `y"}`
+	shadowedDoc := `{"a":"` + esc + `","a":"clean"}`
+
+	// The premise, asserted rather than assumed: the ordinary repair does
+	// nothing to the shadowed value. If this ever stops being true, the whole
+	// suspect-repair path is redundant and should go.
+	if got := textguard.Repair(shadowedDoc, true); got != shadowedDoc {
+		t.Fatalf("textguard.Repair now changes the shadowed value (%q). The predicate has gained "+
+			"duplicate-key awareness — BUG-2812 has landed. Collapse suspects into violations and "+
+			"delete this path.", got)
+	}
+
+	plantLegacyRows(t, s, func(raw *sql.DB) {
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`, literalDoc, literal.ID)
+		mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`, shadowedDoc, shadowed.ID)
+	})
+
+	report, err := s.RepairNUL()
+	if err != nil {
+		t.Fatalf("repair: %v", err)
+	}
+	if len(report.SuspectsFailed) != 0 {
+		t.Fatalf("suspect failures: %+v", report.SuspectsFailed)
+	}
+	if len(report.SuspectsRepaired) != 1 {
+		t.Fatalf("repaired %d suspects, want 1: %+v", len(report.SuspectsRepaired), report.SuspectsRepaired)
+	}
+	if report.SuspectsRepaired[0].Key["id"] != shadowed.ID {
+		t.Errorf("the wrong suspect was repaired: %v", report.SuspectsRepaired[0].Key)
+	}
+	if len(report.SuspectsClean) != 1 || report.SuspectsClean[0].Key["id"] != literal.ID {
+		t.Errorf("the literal was not reported as needing nothing: %+v", report.SuspectsClean)
+	}
+
+	var gotLiteral, gotShadowed string
+	if err := s.db.QueryRow(`SELECT fields FROM items WHERE id = ?`, literal.ID).Scan(&gotLiteral); err != nil {
+		t.Fatalf("read literal: %v", err)
+	}
+	if err := s.db.QueryRow(`SELECT fields FROM items WHERE id = ?`, shadowed.ID).Scan(&gotShadowed); err != nil {
+		t.Fatalf("read shadowed: %v", err)
+	}
+	if gotLiteral != literalDoc {
+		t.Errorf("a value that merely writes about the escape was rewritten\n  before: %q\n  after:  %q",
+			literalDoc, gotLiteral)
+	}
+	want := `{"a":"` + textguard.ReplacementEscape + `","a":"clean"}`
+	if gotShadowed != want {
+		t.Errorf("shadowed value = %q, want %q", gotShadowed, want)
+	}
+
+	// And the violation counts are untouched: the suspect work must not make
+	// the dry run disagree with the run.
+	if len(report.Repaired) != 0 || report.Scan.Total() != 0 {
+		t.Errorf("suspects leaked into the violation buckets: repaired=%d scanTotal=%d",
+			len(report.Repaired), report.Scan.Total())
+	}
+}
+
+// TestSuspectsCollapseWhenBUG2812Lands is the notification test the ruling
+// asked for.
+//
+// The suspect class exists ONLY because the shared predicate cannot see a NUL
+// behind a repeated key. When BUG-2812's token-walk lands, that value becomes
+// an ordinary violation, the destination oracle becomes redundant, and this
+// whole path — the suspect bucket, the preflight cast, RepairSuspectValue —
+// should be deleted rather than left as a second mechanism nobody needs.
+//
+// Nothing would otherwise tell anyone. This fails at that moment and says what
+// to remove.
+func TestSuspectsCollapseWhenBUG2812Lands(t *testing.T) {
+	shadowed := `{"a":"` + textguard.EscNUL + `","a":"clean"}`
+
+	if textguard.ParameterRefused(shadowed, true) {
+		t.Fatalf("the shared predicate now refuses a NUL behind a repeated key, so the SUSPECT class is " +
+			"obsolete. BUG-2812 has landed. Remove: NULSuspect and the Suspects bucket in nulscan.go, " +
+			"CheckJSONBAcceptable and RepairSuspectValue in nulsuspect.go, the destination cast in " +
+			"cmd/pad/cmd_db.go's preflight, the suspect heading in cmd_db_nul.go, and the residual " +
+			"paragraphs in ScanNUL's doc comment and docs/backup.md.")
+	}
+}

internal/store/nulsuspect.go

@@ -0,0 +1,324 @@
+package store
+
+import (
+	"database/sql"
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// The suspect class's two operations (DOC-2823 S3, the day-54 lead ruling on
+// PR #1233): ask the DESTINATION whether a suspect is really fatal, and repair
+// the one shape that is.
+//
+// The ruling's shape, and why it is not a fourth predicate: our layers cannot
+// tell a harmless doubled-backslash literal from a NUL hidden behind a literal
+// duplicate key, because both look identical to a map-model decode — that is
+// textguard.KnownGaps and DOC-2823 forbids closing it in one layer. But the
+// migration has something no layer has: the actual PostgreSQL that is about to
+// refuse the value. Casting it there is not an opinion, it is the oracle.
+//
+// It is exact about THE VALUE. It is not a perfect model of the MIGRATION, and
+// the difference is measured rather than hand-waved: one write path normalises
+// before writing, so a value the cast refuses can still import through that
+// column. See the KNOWN OVER-REFUSAL note on CheckJSONBAcceptable.
+
+// ReadNULTargetValue reads back the value at a suspect's address.
+//
+// The scan report deliberately carries no user content, so anything that needs
+// the bytes — the destination cast, the repair — fetches them by address. Few
+// suspects and one row each, so the extra read is not worth avoiding.
+func (s *Store) ReadNULTargetValue(table, column string, key map[string]string) (string, error) {
+	if len(key) == 0 {
+		return "", fmt.Errorf("no address for %s.%s", table, column)
+	}
+	where, args := nulKeyPredicate(key)
+	var value string
+	err := s.db.QueryRow(
+		fmt.Sprintf(`SELECT %s FROM %s WHERE %s`, quoteIdent(column), quoteIdent(table), where),
+		args...,
+	).Scan(&value)
+	if errors.Is(err, sql.ErrNoRows) {
+		return "", fmt.Errorf("row %s.%s no longer exists", table, column)
+	}
+	if err != nil {
+		return "", fmt.Errorf("read %s.%s: %w", table, column, err)
+	}
+	return value, nil
+}
+
+// ErrNULDestinationRefused is the sentinel behind a destination cast that
+// failed for a NUL reason.
+var ErrNULDestinationRefused = errors.New("store: destination refused the value")
+
+// ErrDestinationCheckUnavailable is the sentinel for a check that did not
+// COMPLETE — a dropped connection, a timeout, anything that is not the
+// database's verdict on the value.
+//
+// It exists so a caller can fail CLOSED. An unverified suspect treated as a
+// pass is the preflight promising a migration it did not check, which is the
+// defect the suspect class was added to correct, arriving by a different route
+// (codex round 5).
+var ErrDestinationCheckUnavailable = errors.New("store: could not ask the destination")
+
+// CheckJSONBAcceptable asks THIS store's database whether a value is a jsonb
+// document it will accept, without writing anything.
+//
+// `SELECT $1::jsonb` is side-effect-free: no table is touched, no transaction
+// state changes, and the cast is exactly the one an INSERT into a jsonb column
+// performs. That makes it a faithful oracle rather than a model of one.
+//
+// The verdict is narrow ON PURPOSE. Only the two NUL SQLSTATEs — 22P05 for an
+// escape decoding to NUL inside jsonb, 22021 for a NUL in text — wrap
+// ErrNULDestinationRefused. Another cast failure that the SERVER answered comes
+// back as a plain error: the destination will reject the row too, but for a
+// reason outside this preflight's remit, and a NUL preflight that silently grew
+// into a general one would refuse migrations that have nothing to do with this
+// bug. Anything else — no SQLSTATE, or an OPERATIONAL one such as a cancelled
+// query or a terminated connection — wraps ErrDestinationCheckUnavailable, and
+// the caller refuses on those. See classifyDestinationError for why the test is
+// "is this class 22" rather than "did the server answer".
+//
+// KNOWN OVER-REFUSAL, measured rather than reasoned about (codex round 5).
+// This casts the value AS STORED, and one write path normalises before writing:
+// CreateWorkspace runs models.NormalizeWorkspaceSettings, a map round-trip, so
+// a shadowed-duplicate in workspaces.settings collapses to its surviving member
+// and imports cleanly. Measured against a real server by importing the same
+// value into three columns:
+//
+//	workspaces.settings  -> import SUCCEEDS, stored as {"a": "clean"}
+//	items.fields         -> import FAILS, SQLSTATE 22P05
+//	collections.schema   -> import FAILS, SQLSTATE 22P05
+//
+// So for that one column the preflight refuses a migration that would have gone
+// through. Left as-is deliberately: the row is a violation of the invariant
+// wherever it sits — Layer B refuses that value on every write today, and it
+// exists only because it predates enforcement — so surviving the migration is
+// an accident of one column's normaliser rather than a property worth
+// preserving, and `pad db repair-nul` clears it in one command. Deriving
+// "would this column's writer normalise it" is a per-column enumeration, which
+// is the shape this cluster keeps proving unmaintainable. Flagged to the lead
+// rather than decided here; the REFUSAL WORDING no longer claims PostgreSQL
+// would reject the row, only that the value carries a NUL jsonb refuses.
+//
+// SQLSTATE by string extraction rather than a pgconn type assertion, following
+// isDeadlockError in this package: internal/store keeps both drivers behind
+// database/sql, and pgx puts the code verbatim in the error text
+// ("... (SQLSTATE 22P05)"). TestDestinationOracleClassifiesRealPostgresErrors
+// pins the extraction against a real server rather than assuming the wording.
+func (s *Store) CheckJSONBAcceptable(value string) error {
+	if s.dialect.Driver() != DriverPostgres {
+		return fmt.Errorf("the jsonb cast oracle needs a PostgreSQL destination")
+	}
+	var out []byte
+	err := s.db.QueryRow(`SELECT $1::jsonb`, value).Scan(&out)
+	if err == nil {
+		return nil
+	}
+	switch classifyDestinationError(err) {
+	case destinationRefusedNUL:
+		return fmt.Errorf("%w: %v", ErrNULDestinationRefused, err)
+	case destinationRejectedValue:
+		return err
+	default:
+		return fmt.Errorf("%w: %v", ErrDestinationCheckUnavailable, err)
+	}
+}
+
+// destinationVerdict is what a failed cast tells us.
+type destinationVerdict int
+
+const (
+	// destinationUnavailable: the server did not render a verdict about the
+	// value. No SQLSTATE at all, or an OPERATIONAL one.
+	destinationUnavailable destinationVerdict = iota
+	// destinationRejectedValue: the server judged the value and refused it, for
+	// a reason that is not a NUL.
+	destinationRejectedValue
+	// destinationRefusedNUL: the server judged the value and refused it for a
+	// NUL.
+	destinationRefusedNUL
+)
+
+// classifyDestinationError decides which of the three a cast failure is.
+//
+// THE TEST IS INVERTED FROM THE OBVIOUS ONE, and that inversion is the fix
+// (codex round 6). The first version asked "did the server answer at all",
+// treating every SQLSTATE-bearing error as a verdict about the value — but
+// 57014 (query cancelled), 57P01 (terminated by administrator), the 08 class
+// (connection exception) and the 53 class (out of resources) all carry
+// SQLSTATEs and say nothing about the value. Classified as verdicts, they let
+// the preflight proceed with an unverified suspect, which is the fail-open this
+// whole three-way split exists to close, one level deeper.
+//
+// So only CLASS 22 — data exception, PostgreSQL's class for "this value is
+// wrong" — counts as a verdict. `SELECT $1::jsonb` produces 22P02 for
+// malformed JSON and 22P05 / 22021 for the NUL cases; everything else means the
+// question was not answered, and the caller refuses rather than guessing.
+//
+// Erring toward "unavailable" is the safe direction: its cost is a refused
+// migration an operator re-runs, against a half-finished one they have to
+// unpick.
+func classifyDestinationError(err error) destinationVerdict {
+	code := sqlStateOf(err)
+	switch code {
+	case "22P05", "22021":
+		return destinationRefusedNUL
+	case "":
+		return destinationUnavailable
+	}
+	if strings.HasPrefix(code, "22") {
+		return destinationRejectedValue
+	}
+	return destinationUnavailable
+}
+
+// sqlStateOf extracts the five-character error code pgx renders into a
+// server-side error's message, or "" when there is none.
+//
+// String extraction rather than a pgconn type assertion, following
+// isDeadlockError in this package: internal/store keeps both drivers behind
+// database/sql. The RENDERING is pinned by tests that provoke real errors from
+// a real server rather than by assuming the wording.
+func sqlStateOf(err error) string {
+	if err == nil {
+		return ""
+	}
+	// ONE string for both the search and the slice. The first version indexed
+	// into strings.ToUpper(msg) and then sliced the ORIGINAL, which is only
+	// safe while every byte before the marker is ASCII: Unicode case mapping
+	// changes byte LENGTH for some runes, so a localised server message —
+	// lc_messages is a per-server setting — shifts the offset and the five
+	// bytes taken are the wrong five (codex round 7).
+	const marker = "SQLSTATE "
+	msg := strings.ToUpper(err.Error())
+	i := strings.Index(msg, marker)
+	if i < 0 {
+		return ""
+	}
+	rest := msg[i+len(marker):]
+	if len(rest) < 5 {
+		return ""
+	}
+	return rest[:5]
+}
+
+// RepairSuspectValue rewrites a suspect's NUL escapes with U+FFFD, using the
+// TOKEN-level scanner rather than the predicate-gated repair.
+//
+// MEASURED, because the ruling asked and because the answer decides whether the
+// preflight's remedy works: `textguard.Repair` leaves the shadowed-duplicate
+// shape completely untouched. Its scanner is gated on
+// DocumentDecodesNULAnyShape, which is a map-model question and answers false
+// for exactly this value, so the scanner never runs. Probed on the branch:
+//
+//	Repair(v, true)        -> unchanged
+//	the scanner directly   -> the shadowed escape rewritten, replaced=1
+//
+// Without this, `pad db migrate-to-pg` would refuse a suspect and print a
+// repair command that does nothing to it — a remedy that is a contract claim
+// nobody ran (PATTE-135). So the repair reaches the suspect class through the
+// scanner, which is sound here for the same reason it is sound everywhere: it
+// rewrites only escapes a JSON parser would decode, and consumes escapes in
+// order, so a doubled-backslash literal is left exactly as it was.
+//
+// This does NOT widen what any layer REFUSES. KnownGaps is untouched; what
+// changed is what an operator's explicit repair is allowed to FIX.
+func (s *Store) RepairSuspectValue(sus NULSuspect) (repaired bool, err error) {
+	if sus.KeyIncomplete || len(sus.Key) == 0 {
+		return false, fmt.Errorf("no address for %s.%s", sus.Table, sus.Column)
+	}
+
+	tx, err := s.db.Begin()
+	if err != nil {
+		return false, fmt.Errorf("begin: %w", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	where, args := nulKeyPredicate(sus.Key)
+	qc := quoteIdent(sus.Column)
+	qt := quoteIdent(sus.Table)
+
+	var value string
+	err = tx.QueryRow(fmt.Sprintf(`SELECT %s FROM %s WHERE %s`, qc, qt, where), args...).Scan(&value)
+	if errors.Is(err, sql.ErrNoRows) {
+		return false, nil
+	}
+	if err != nil {
+		return false, fmt.Errorf("read %s.%s: %w", sus.Table, sus.Column, err)
+	}
+
+	next, n := textguard.RepairJSONEscapes(value)
+	if n == 0 || next == value {
+		// A harmless literal — the common case. Nothing to do, and saying so is
+		// not a failure.
+		return false, nil
+	}
+
+	res, err := tx.Exec(fmt.Sprintf(`UPDATE %s SET %s = ? WHERE %s`, qt, qc, where),
+		append([]any{next}, args...)...)
+	if err != nil {
+		return false, fmt.Errorf("update %s.%s: %w", sus.Table, sus.Column, err)
+	}
+	rows, err := res.RowsAffected()
+	if err != nil {
+		return false, fmt.Errorf("rows affected for %s.%s: %w", sus.Table, sus.Column, err)
+	}
+	if rows != 1 {
+		return false, fmt.Errorf("repairing %s.%s matched %d rows, want exactly 1", sus.Table, sus.Column, rows)
+	}
+	if err := tx.Commit(); err != nil {
+		return false, fmt.Errorf("commit: %w", err)
+	}
+	return true, nil
+}
+
+// nulKeyPredicate renders a WHERE clause addressing one row from its key map.
+func nulKeyPredicate(key map[string]string) (string, []any) {
+	keys := sortedKeys(key)
+	clauses := make([]string, 0, len(keys))
+	args := make([]any, 0, len(keys))
+	for _, k := range keys {
+		clauses = append(clauses, quoteIdent(k)+" = ?")
+		args = append(args, key[k])
+	}
+	return strings.Join(clauses, " AND "), args
+}
+
+// MigratedTables names the tables `pad db migrate-to-pg` actually copies.
+//
+// The migration is application-level: it walks workspaces and runs
+// ExportWorkspace / ImportWorkspace on each. That reads six tables and no
+// others — the command's own help says users, platform settings and auth data
+// are NOT migrated — so a NUL in users.name, platform_settings.value,
+// sessions.user_agent or any oauth table cannot break it.
+//
+// The preflight uses this to decide what to REFUSE on, not what to REPORT
+// (codex round 9). Scanning everything is right: `pad db scan-nul` is about the
+// database, and an operator should hear about every affected row. Blocking a
+// migration over a row it will never touch is not — it demands the operator
+// rewrite content that has nothing to do with the copy they asked for.
+//
+// TestMigratedTablesCoversTheExport pins this against models.WorkspaceExport's
+// own shape, so a new export section fails here rather than silently making the
+// preflight miss a table.
+//
+// KNOWN RESIDUAL, stated because it is the same class of over-refusal one size
+// smaller: the export also skips SOFT-DELETED collections and items
+// (`deleted_at IS NULL`), and this filter is per-table, not per-row. A NUL in a
+// soft-deleted item still blocks a migration that would not have carried it.
+// Narrowing that needs a per-row deleted_at check at every candidate, which is
+// more machinery than the remaining over-refusal costs — the operator's way out
+// is the same single repair command either way.
+func MigratedTables() map[string]bool {
+	return map[string]bool{
+		"workspaces":    true,
+		"collections":   true,
+		"items":         true,
+		"comments":      true,
+		"item_links":    true,
+		"item_versions": true,
+	}
+}

internal/store/nulsuspect_pg_test.go

@@ -0,0 +1,314 @@
+package store
+
+import (
+	"errors"
+	"os"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+	"github.com/PerpetualSoftware/pad/internal/textguard"
+)
+
+// TestDestinationOracleClassifiesRealPostgresErrors is the test
+// CheckJSONBAcceptable's doc comment promises.
+//
+// The whole suspect design rests on the claim that PostgreSQL can tell apart
+// two values our own layers cannot, and that we can read its verdict. Both
+// halves are assumptions until something runs them against a real server: the
+// first about the database, the second about pgx's error TEXT, since the
+// SQLSTATE is matched by string here (following isDeadlockError in this
+// package). A wrong assumption on either half fails in the worst direction —
+// silently accepting a value the migration will choke on.
+func TestDestinationOracleClassifiesRealPostgresErrors(t *testing.T) {
+	s := testStore(t)
+	if s.dialect.Driver() != DriverPostgres {
+		t.Skip("the destination oracle needs a real PostgreSQL (set PAD_TEST_POSTGRES_URL)")
+	}
+
+	esc := textguard.EscNUL
+	backslash := esc[:1]
+
+	t.Run("a clean document is accepted", func(t *testing.T) {
+		// The control. Without it an oracle that refused everything would pass
+		// every case below and refuse every migration.
+		if err := s.CheckJSONBAcceptable(`{"a":"ordinary"}`); err != nil {
+			t.Fatalf("a clean document was refused: %v", err)
+		}
+	})
+
+	t.Run("a doubled-backslash literal is accepted", func(t *testing.T) {
+		// The over-refusal leg, and the reason the oracle is the DESTINATION
+		// rather than a broader predicate of ours. This value is in the suspect
+		// list, it decodes to no NUL, and PostgreSQL stores it happily — a
+		// preflight that refused it would block migrations over prose that
+		// merely writes about this bug.
+		doc := `{"note":"x` + backslash + esc + `y"}`
+		if err := s.CheckJSONBAcceptable(doc); err != nil {
+			t.Fatalf("the destination refused a harmless literal, so the preflight would over-refuse: %v", err)
+		}
+	})
+
+	t.Run("a NUL behind a repeated key is refused", func(t *testing.T) {
+		// The whole point. Our predicate accepts this — asserted here so the
+		// case cannot silently become one we catch ourselves — and PostgreSQL
+		// does not.
+		doc := `{"a":"` + esc + `","a":"clean"}`
+		if textguard.ParameterRefused(doc, true) {
+			t.Fatal("the shared predicate now refuses this; the suspect class is obsolete (BUG-2812)")
+		}
+		err := s.CheckJSONBAcceptable(doc)
+		if err == nil {
+			t.Fatal("PostgreSQL accepted a NUL hidden behind a repeated key — the premise this whole " +
+				"design rests on is wrong and the suspect path should be removed")
+		}
+		if !errors.Is(err, ErrNULDestinationRefused) {
+			t.Fatalf("the refusal was not classified as a NUL refusal, so the preflight would report it "+
+				"as an unrelated note instead of refusing: %v", err)
+		}
+		// And the classification came from the code we claim to match, not
+		// from some other part of the message.
+		if !strings.Contains(strings.ToUpper(err.Error()), "22P05") {
+			t.Errorf("expected SQLSTATE 22P05 in the error text: %v", err)
+		}
+	})
+
+	t.Run("a non-JSON value is refused but NOT as a NUL refusal", func(t *testing.T) {
+		// The bucket separation. This value will also break the migration, but
+		// for a reason outside this preflight's remit, and a NUL preflight that
+		// refused on it would start blocking migrations unrelated to this bug.
+		err := s.CheckJSONBAcceptable("not json at all")
+		if err == nil {
+			t.Fatal("a non-JSON value was accepted as jsonb")
+		}
+		if errors.Is(err, ErrNULDestinationRefused) {
+			t.Errorf("a syntax failure was classified as a NUL refusal: %v", err)
+		}
+		// AND it is a completed verdict, not an unavailable check. This is the
+		// real-server half of classifyDestinationError's rule: it trusts that a
+		// bad VALUE produces SQLSTATE class 22, and here is a real one doing so.
+		// Without this the class-22 rule would rest entirely on documentation.
+		if errors.Is(err, ErrDestinationCheckUnavailable) {
+			t.Errorf("a verdict about the value was classified as an unavailable check: %v", err)
+		}
+		if code := sqlStateOf(err); !strings.HasPrefix(code, "22") {
+			t.Errorf("a malformed value produced SQLSTATE %q, not class 22 — classifyDestinationError's "+
+				"rule is built on that class being what a bad value yields", code)
+		}
+	})
+}
+
+// TestDestinationOracleFailsClosedOnAnUnusableConnection is the test
+// hasSQLState's doc comment promises, and it pins the distinction the preflight
+// refuses on.
+//
+// "The server said no" and "the server never answered" must not look alike. The
+// first is a verdict about the value; the second is a check that did not
+// happen, and treating it as a pass would let the preflight promise a migration
+// it never verified — the same defect the suspect class was added to correct,
+// arriving by a different route.
+//
+// A CLOSED POOL rather than a fabricated error string, so what is measured is
+// what pgx actually produces when the database cannot be reached.
+func TestDestinationOracleFailsClosedOnAnUnusableConnection(t *testing.T) {
+	dsn := os.Getenv("PAD_TEST_POSTGRES_URL")
+	if dsn == "" {
+		t.Skip("needs a real PostgreSQL (set PAD_TEST_POSTGRES_URL)")
+	}
+
+	dead, err := NewPostgres(dsn)
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+
+	// CONTROL FIRST: while it is open, the oracle answers normally. Without
+	// this, a check that returned "unavailable" for everything would pass the
+	// assertion below and refuse every migration.
+	if err := dead.CheckJSONBAcceptable(`{"a":"ordinary"}`); err != nil {
+		t.Fatalf("the control failed before the pool was closed: %v", err)
+	}
+
+	if err := dead.Close(); err != nil {
+		t.Fatalf("close: %v", err)
+	}
+
+	err = dead.CheckJSONBAcceptable(`{"a":"ordinary"}`)
+	if err == nil {
+		t.Fatal("a closed pool answered the cast successfully")
+	}
+	if !errors.Is(err, ErrDestinationCheckUnavailable) {
+		t.Fatalf("a connection failure was not classified as an unavailable CHECK, so the preflight "+
+			"would treat an unverified suspect as a pass: %v", err)
+	}
+	if errors.Is(err, ErrNULDestinationRefused) {
+		t.Errorf("a connection failure was classified as a verdict about the value: %v", err)
+	}
+}
+
+// TestClassifyDestinationErrorTreatsOperationalCodesAsUnverified is codex round
+// 6's finding, and it is the fail-open one level below the one round 5 found.
+//
+// An operational SQLSTATE — a cancelled query, an administrator terminating the
+// backend, a connection exception — is the server answering that it could not
+// do the work, not a verdict about the value. Classified as a verdict, the
+// preflight proceeds with an UNVERIFIED suspect, which is exactly what the
+// three-way split exists to stop.
+//
+// SPLIT DELIBERATELY: the codes below are from PostgreSQL's error-code table
+// and are formatted the way pgx renders them, because provoking an
+// administrator shutdown inside a unit test is not worth it. The RENDERING half
+// — that pgx really does write "(SQLSTATE nnnnn)" and that class 22 really is
+// what a bad value produces — is measured against a real server in
+// TestDestinationOracleClassifiesRealPostgresErrors, and the no-code half in
+// TestDestinationOracleFailsClosedOnAnUnusableConnection. Neither half is
+// assumed alone.
+func TestClassifyDestinationErrorTreatsOperationalCodesAsUnverified(t *testing.T) {
+	pgxish := func(code, text string) error {
+		return errors.New("ERROR: " + text + " (SQLSTATE " + code + ")")
+	}
+
+	cases := []struct {
+		name string
+		err  error
+		want destinationVerdict
+		why  string
+	}{
+		{"NUL in jsonb", pgxish("22P05", "unsupported Unicode escape sequence"), destinationRefusedNUL,
+			"the code the whole preflight refuses on."},
+		{"NUL in text", pgxish("22021", "invalid byte sequence"), destinationRefusedNUL,
+			"the text-column counterpart."},
+		{"malformed JSON", pgxish("22P02", "invalid input syntax for type json"), destinationRejectedValue,
+			"class 22 and not a NUL: the destination judged the VALUE, so it is reported, not refused on."},
+		{"cancelled query", pgxish("57014", "canceling statement due to statement timeout"),
+			destinationUnavailable,
+			"THE FINDING. A timeout carries a SQLSTATE and says nothing about the value; treating it as a " +
+				"verdict lets an unverified suspect through."},
+		{"terminated by administrator", pgxish("57P01", "terminating connection due to administrator command"),
+			destinationUnavailable, "same class of mistake, different code."},
+		{"connection exception", pgxish("08006", "connection failure"), destinationUnavailable,
+			"class 08 is transport, not data."},
+		{"out of resources", pgxish("53200", "out of memory"), destinationUnavailable,
+			"class 53 is the server's own state."},
+		{"no code at all", errors.New("dial tcp: connection refused"), destinationUnavailable,
+			"the driver never reached a server; pinned for real in the closed-pool test."},
+		{"nil", nil, destinationUnavailable,
+			"boundary: a classifier that answered 'verdict' for nil would make a successful cast look " +
+				"like a rejection at any call site that forgot to check err first."},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := classifyDestinationError(tc.err); got != tc.want {
+				t.Errorf("%s\n  err:  %v\n  got %v, want %v", tc.why, tc.err, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestSQLStateExtractionEdges covers the parser the classification rests on.
+func TestSQLStateExtractionEdges(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+		want string
+		why  string
+	}{
+		{"pgx rendering", errors.New("ERROR: boom (SQLSTATE 22P05)"), "22P05", "the shape pgx produces."},
+		{"no marker", errors.New("connection refused"), "", "nothing to extract."},
+		{"truncated code", errors.New("... SQLSTATE 22"), "",
+			"a marker with fewer than five characters after it must not yield a partial code that then " +
+				"matches a class prefix."},
+		{"lowercase marker", errors.New("... sqlstate 22p05)"), "22P05",
+			"the search is case-insensitive, so the extraction must be too."},
+		{"nil", nil, "", "boundary."},
+		{
+			// THE OFFSET BUG. U+0131 (dotless i) is two bytes and uppercases to
+			// a one-byte "I", so searching an uppercased copy and slicing the
+			// original takes the wrong five bytes. PostgreSQL renders messages
+			// in lc_messages, so a non-English server is not hypothetical.
+			name: "a message whose case mapping changes byte length",
+			err:  errors.New("HATA: ge\u00e7ersiz \u0131\u0131\u0131 (SQLSTATE 22P05)"),
+			want: "22P05",
+			why:  "the code must survive a localised message; a shifted slice misclassifies a real NUL refusal.",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := sqlStateOf(tc.err); got != tc.want {
+				t.Errorf("%s\n  got %q, want %q", tc.why, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestMigratedTablesCoversTheExport pins MigratedTables against the export's
+// own SHAPE rather than against a reading of its SQL.
+//
+// The preflight refuses only on tables in that set, so a section added to
+// WorkspaceExport without a matching entry would make the preflight quietly
+// stop guarding a table the migration now copies — a miss, in the direction
+// that ends in a half-finished migration. A source-regex over ExportWorkspace's
+// queries would not catch it either: this codebase has already learned that
+// multi-line and Sprintf-composed SQL are invisible to any such instrument
+// (TASK-2825).
+//
+// Reflection over the struct is the maintainable version of the question: every
+// slice section, plus the workspace row itself, must map to a listed table.
+func TestMigratedTablesCoversTheExport(t *testing.T) {
+	// Section field name -> the table it is read from. Adding a section to
+	// WorkspaceExport fails the loop below until it is named here AND in
+	// MigratedTables, which is the point.
+	sectionTables := map[string]string{
+		"Collections":  "collections",
+		"Items":        "items",
+		"Comments":     "comments",
+		"ItemLinks":    "item_links",
+		"ItemVersions": "item_versions",
+	}
+
+	migrated := MigratedTables()
+
+	// The workspace row is carried by the Workspace field rather than a slice,
+	// and ImportWorkspace writes it through CreateWorkspace.
+	if !migrated["workspaces"] {
+		t.Error("workspaces is not listed, but ImportWorkspace creates the workspace row")
+	}
+
+	typ := reflect.TypeOf(models.WorkspaceExport{})
+	for i := 0; i < typ.NumField(); i++ {
+		f := typ.Field(i)
+		if f.Type.Kind() != reflect.Slice {
+			continue
+		}
+		table, known := sectionTables[f.Name]
+		if !known {
+			t.Errorf("WorkspaceExport has a section %q that this test does not map to a table. The "+
+				"migration now copies something new: add it here AND to MigratedTables, or the "+
+				"preflight stops guarding it.", f.Name)
+			continue
+		}
+		if !migrated[table] {
+			t.Errorf("export section %q reads %q, which MigratedTables does not list — the preflight "+
+				"would not refuse on a NUL there", f.Name, table)
+		}
+	}
+
+	// And the set contains nothing SPURIOUS, which would be an over-refusal
+	// rather than a miss but is still wrong.
+	for table := range migrated {
+		if table == "workspaces" {
+			continue
+		}
+		found := false
+		for _, mapped := range sectionTables {
+			if mapped == table {
+				found = true
+			}
+		}
+		if !found {
+			t.Errorf("MigratedTables lists %q, which no export section reads; the preflight would "+
+				"refuse a migration over a table it does not copy", table)
+		}
+	}
+}

internal/textguard/repair.go

@@ -0,0 +1,176 @@
+package textguard
+
+import "strings"
+
+// Repair is the ONE implementation of "make this value satisfy the invariant",
+// and it lives beside the predicate for the same reason the predicate is
+// shared: four layers that agree about what is REFUSED but disagree about what
+// a repair PRODUCES is this bug family arriving one step later.
+//
+// DOC-2823 S3, on Dave's day-54 ruling: the replacement character is U+FFFD.
+// Visible, greppable, and — because the JSON arm emits it as an escape — the
+// same six characters wide as the escape it replaces.
+//
+// isJSON is the caller's classification, exactly as in ParameterRefused, and
+// the two must be given the SAME value for the same column. The scan derives it
+// from the column list in internal/store/nulcolumns.go, which is Layer B's
+// classing.
+//
+// THE PROPERTY THIS MUST HAVE, in both directions, is what corpus_test.go pins:
+//
+//   - For every value the layers REFUSE, Repair produces one they ACCEPT.
+//   - For every value the layers ACCEPT, Repair is the IDENTITY.
+//
+// The second half is not politeness. A repair that "tidies" values nobody
+// complained about is a repair that rewrites `{"a":"x\\u0000y"}` — six literal
+// characters after a doubled backslash, an accepted value — and corrupts it.
+// That case is in the corpus precisely because every cheap approach fails it.
+func Repair(value string, isJSON bool) string {
+	// Raw NULs first, as TEXT. This is byte-preserving everywhere else and is
+	// the whole repair for a text-classed column; it is also the right repair
+	// for a raw NUL sitting inside a JSON-classed blob, which is a defect of
+	// the stored bytes rather than of the document.
+	out := strings.ReplaceAll(value, NUL, Replacement)
+
+	// The escape form only matters where something parses the value as a JSON
+	// document. In a text column the six characters are six characters, and
+	// rewriting them there is the false positive the corpus's third case
+	// exists to catch.
+	if isJSON && DocumentDecodesNULAnyShape(out) {
+		out, _ = RepairJSONEscapes(out)
+	}
+	return out
+}
+
+// RepairJSONEscapes rewrites every NUL escape a JSON parser would DECODE in an
+// already-valid JSON document, and reports how many it replaced.
+//
+// IT IS DELIBERATELY BROADER THAN Repair, and that difference is the whole
+// reason it is exported (DOC-2823 S3, the day-54 suspect ruling).
+//
+// Repair only reaches the scanner for a document DocumentDecodesNULAnyShape
+// answers true for — a map-model question, which cannot see a NUL in a value
+// shadowed by a LITERAL duplicate key, because the decode keeps the last one.
+// This function is a token-level walk over string literals, so it rewrites the
+// shadowed escape too. Measured: `Repair` leaves
+// `{"a":"<escape>","a":"clean"}` untouched; this returns
+// `{"a":"<U+FFFD escape>","a":"clean"}` with a count of 1.
+//
+// NEVER USE IT AS A PREDICATE. "Would this rewrite something" is not "does a
+// layer refuse this", and answering the second question with the first is the
+// layer-confusion the whole cluster is made of — textguard.KnownGaps stays a
+// recorded shared gap in what is REFUSED. This is only about what a REPAIR,
+// asked for explicitly by an operator, is allowed to fix.
+func RepairJSONEscapes(s string) (string, int) {
+	return repairJSONNULEscapes(s)
+}
+
+const (
+	// Replacement is U+FFFD as text, for a raw NUL in a stored value.
+	Replacement = "�"
+
+	// ReplacementEscape is U+FFFD spelled as a JSON escape, for a live NUL
+	// escape inside a JSON document. Six characters replacing six, so the
+	// document's byte length is unchanged and nothing around it shifts.
+	ReplacementEscape = "\\u" + "fffd"
+
+	// nulEscapeLen is the length of a \uXXXX escape.
+	nulEscapeLen = 6
+)
+
+// repairJSONNULEscapes rewrites every NUL escape that a JSON parser would
+// DECODE, and nothing else.
+//
+// WHY THIS IS A SCANNER AND NOT decode-walk-remarshal, which was the shape the
+// S3 recon write-up proposed before this was written. Re-marshalling a document
+// changes four things nobody asked to change: object key order (Go sorts map
+// keys), insignificant whitespace, integers wider than float64 (unless the
+// decoder is told to use json.Number), and HTML-ish characters (unless the
+// encoder is told not to escape them). Worse, a document with LITERAL duplicate
+// keys silently loses one on the way through a map — which is one of the two
+// gaps BUG-2812 owns, and a repair is the last place that should quietly drop
+// user data.
+//
+// Scanning the raw text has none of those failure modes: every byte the repair
+// does not deliberately rewrite is copied verbatim, so a document with no live
+// escape comes out byte-identical without that having to be argued.
+//
+// WHY IT IS NOT A SUBSTRING REPLACE, which is the version that looks equivalent
+// and is not. `{"a":"x\\u0000y"}` contains the six characters and decodes to no
+// NUL at all, because the doubled backslash makes them literal. Only a scanner
+// that consumes escapes IN ORDER can tell the two apart — which is the same
+// layer-relativity that made BUG-2803's parity pre-filter unsound, met again in
+// the write direction.
+//
+// The input is a value DocumentDecodesNULAnyShape has already accepted as
+// valid JSON, so the scanner has no malformed-input arm: an unterminated string
+// or a stray backslash cannot occur. It still copies anything it does not
+// recognise rather than assuming, so a future caller passing something else
+// gets its bytes back unchanged instead of a mangled document.
+func repairJSONNULEscapes(s string) (string, int) {
+	var b strings.Builder
+	b.Grow(len(s))
+
+	replaced := 0
+	inString := false
+	for i := 0; i < len(s); {
+		c := s[i]
+
+		if !inString {
+			if c == '"' {
+				inString = true
+			}
+			b.WriteByte(c)
+			i++
+			continue
+		}
+
+		switch c {
+		case '"':
+			inString = false
+			b.WriteByte(c)
+			i++
+		case '\\':
+			// An escape. Consume it WHOLE, so its second character can never
+			// be read as the start of another one — that is the entire
+			// difference between this and a substring replace.
+			if i+1 >= len(s) {
+				b.WriteByte(c)
+				i++
+				continue
+			}
+			if s[i+1] == 'u' && i+nulEscapeLen <= len(s) && isNULEscapeHex(s[i+2:i+nulEscapeLen]) {
+				b.WriteString(ReplacementEscape)
+				i += nulEscapeLen
+				replaced++
+				continue
+			}
+			// Any other escape, including `\\`: copy both bytes and move on.
+			b.WriteString(s[i : i+2])
+			i += 2
+		default:
+			b.WriteByte(c)
+			i++
+		}
+	}
+	return b.String(), replaced
+}
+
+// isNULEscapeHex reports whether the four hex digits of a \uXXXX escape name
+// U+0000.
+//
+// No case folding, and that is not an oversight: the only spelling of this
+// code point is four ASCII zeros, which have no case. Every OTHER code point
+// would need it, so a caller widening this function to a general hex compare
+// must add it then.
+func isNULEscapeHex(hex string) bool {
+	if len(hex) != 4 {
+		return false
+	}
+	for i := 0; i < 4; i++ {
+		if hex[i] != '0' {
+			return false
+		}
+	}
+	return true
+}

internal/textguard/repair_test.go

@@ -0,0 +1,242 @@
+package textguard
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// The repair's contract is a property over the SAME corpus the four
+// enforcement layers are measured against (DOC-2823 S3). Driving the repair
+// through the corpus rather than through examples of its own is what stops it
+// from being correct about a set of values nobody enforces.
+
+// TestRepairSatisfiesTheCorpusBothWays is the whole contract in one test.
+//
+// Both directions matter and they fail differently. A repair that is not
+// ACCEPTED afterwards has not repaired anything; a repair that is not the
+// IDENTITY on accepted values has corrupted something nobody complained about,
+// which is the harder failure to notice because every layer stays green.
+func TestRepairSatisfiesTheCorpusBothWays(t *testing.T) {
+	for _, c := range Corpus {
+		t.Run(c.Name, func(t *testing.T) {
+			got := Repair(c.Value, c.IsJSON)
+
+			if ParameterRefused(got, c.IsJSON) {
+				t.Errorf("repaired value is still refused\n  in:  %q\n  out: %q", c.Value, got)
+			}
+
+			if c.Refused {
+				// A repair that returned its input unchanged would satisfy the
+				// check above only by accident on the accepted cases, and
+				// would silently do nothing here. Asserting the change is what
+				// makes this leg fail for a no-op implementation.
+				if got == c.Value {
+					t.Errorf("refused value came back unchanged: %q", c.Value)
+				}
+			} else if got != c.Value {
+				t.Errorf("accepted value was rewritten\n  in:  %q\n  out: %q", c.Value, got)
+			}
+		})
+	}
+}
+
+// TestRepairAcceptsTheStoreOverRefusals covers the values only Layer A refuses.
+//
+// They are outside Corpus because the layers disagree about them, but the
+// repair still has to produce something the store will accept — otherwise
+// `pad db repair-nul` leaves behind exactly the rows the store's own classing
+// would refuse to rewrite.
+func TestRepairAcceptsTheStoreOverRefusals(t *testing.T) {
+	for _, c := range StoreOverRefusals {
+		t.Run(c.Name, func(t *testing.T) {
+			// The store classes by COLUMN; these are text columns whose value
+			// happens to parse as a document, so the store checks them as JSON.
+			got := Repair(c.Value, true)
+			if ParameterRefused(got, true) {
+				t.Errorf("still refused by the store's classing\n  in:  %q\n  out: %q", c.Value, got)
+			}
+		})
+	}
+}
+
+// TestRepairIsIdempotent pins that a second pass changes nothing.
+//
+// The scan and the repair are separate commands, so an operator re-running the
+// repair after a partial run is expected, not exceptional; and a repair whose
+// output it would itself flag is a repair that never terminates.
+func TestRepairIsIdempotent(t *testing.T) {
+	for _, c := range Corpus {
+		once := Repair(c.Value, c.IsJSON)
+		twice := Repair(once, c.IsJSON)
+		if once != twice {
+			t.Errorf("%s: not idempotent\n  once:  %q\n  twice: %q", c.Name, once, twice)
+		}
+	}
+}
+
+// TestRepairPreservesTheDocumentModuloNUL is the faithfulness half.
+//
+// "Accepted afterwards" is satisfied by a repair that returns `{}` for every
+// document. This asserts the repaired document decodes to the SAME structure as
+// the original, with every NUL — in a value or in a key — replaced by U+FFFD
+// and nothing else touched.
+func TestRepairPreservesTheDocumentModuloNUL(t *testing.T) {
+	for _, c := range Corpus {
+		if !c.IsJSON || !json.Valid([]byte(strings.TrimSpace(c.Value))) {
+			continue
+		}
+		t.Run(c.Name, func(t *testing.T) {
+			var before, after any
+			if err := json.Unmarshal([]byte(c.Value), &before); err != nil {
+				t.Fatalf("decode original: %v", err)
+			}
+			repaired := Repair(c.Value, true)
+			if err := json.Unmarshal([]byte(repaired), &after); err != nil {
+				t.Fatalf("repaired value is not valid JSON: %v (%q)", err, repaired)
+			}
+			if !equalModuloNUL(before, after) {
+				t.Errorf("repair changed more than the NULs\n  before: %#v\n  after:  %#v", before, after)
+			}
+		})
+	}
+}
+
+// equalModuloNUL compares two decoded JSON values, treating a NUL on the left
+// as equal to U+FFFD on the right and requiring everything else to match.
+func equalModuloNUL(before, after any) bool {
+	switch b := before.(type) {
+	case string:
+		a, ok := after.(string)
+		return ok && strings.ReplaceAll(b, NUL, Replacement) == a
+	case map[string]any:
+		a, ok := after.(map[string]any)
+		if !ok || len(a) != len(b) {
+			return false
+		}
+		for k, bv := range b {
+			av, present := a[strings.ReplaceAll(k, NUL, Replacement)]
+			if !present || !equalModuloNUL(bv, av) {
+				return false
+			}
+		}
+		return true
+	case []any:
+		a, ok := after.([]any)
+		if !ok || len(a) != len(b) {
+			return false
+		}
+		for i := range b {
+			if !equalModuloNUL(b[i], a[i]) {
+				return false
+			}
+		}
+		return true
+	default:
+		return before == after
+	}
+}
+
+// TestRepairScannerEdges covers the shapes the corpus does not carry, because
+// the corpus is about what the LAYERS disagree on and these are about what the
+// SCANNER can misread.
+func TestRepairScannerEdges(t *testing.T) {
+	// Built, never typed — see corpus.go. A case that means the escape while
+	// carrying the character is vacuous.
+	esc := EscNUL
+	backslash := esc[:1]
+
+	cases := []struct {
+		name   string
+		in     string
+		isJSON bool
+		want   string
+		why    string
+	}{
+		{
+			name: "escape in a key AND a value", isJSON: true,
+			in:   `{"k` + esc + `":"v` + esc + `"}`,
+			want: `{"k` + ReplacementEscape + `":"v` + ReplacementEscape + `"}`,
+			why:  "keys and values take the same path; a value-only rewrite leaves the document refused.",
+		},
+		{
+			name: "doubled backslash then the escape text", isJSON: true,
+			in:   `{"a":"x` + backslash + esc + `y"}`,
+			want: `{"a":"x` + backslash + esc + `y"}`,
+			why:  "literal text after an escaped backslash. This is the case a substring replace corrupts.",
+		},
+		{
+			name: "escape immediately after another escape", isJSON: true,
+			in:   `{"a":"` + backslash + `n` + esc + `"}`,
+			want: `{"a":"` + backslash + `n` + ReplacementEscape + `"}`,
+			why:  "the scanner must resume at the right offset after consuming a two-byte escape.",
+		},
+		{
+			// THE DISCRIMINATING CASE, and the one the first draft of this test
+			// did not have. A doubled-backslash literal ALONE never reaches the
+			// scanner: Repair's guard skips a document that decodes to no NUL,
+			// so a naive substring replace survived every leg here. It is only
+			// when the same document ALSO carries a live escape that the
+			// scanner runs over the literal — and that is the row an operator
+			// actually has, since the literal is what a document about this bug
+			// contains. Found by mutating the scanner to strings.ReplaceAll and
+			// watching the suite stay green.
+			name: "a live escape and a doubled-backslash literal in ONE document", isJSON: true,
+			in:   `{"a":"x` + esc + `y","b":"lit` + backslash + esc + `eral"}`,
+			want: `{"a":"x` + ReplacementEscape + `y","b":"lit` + backslash + esc + `eral"}`,
+			why:  "the live escape is rewritten and the literal is not. A substring replace corrupts the second.",
+		},
+		{
+			name: "the escape text OUTSIDE any string", isJSON: true,
+			in:   `{"a":1}`,
+			want: `{"a":1}`,
+			why:  "control: structural bytes are copied verbatim and no rewrite fires.",
+		},
+		{
+			name: "raw NUL and a live escape in one JSON value", isJSON: true,
+			in:   `{"a":"x` + NUL + `y` + esc + `z"}`,
+			want: `{"a":"x` + Replacement + `y` + ReplacementEscape + `z"}`,
+			why:  "both defects in one value: the raw pass and the document pass must BOTH fire.",
+		},
+		{
+			name: "escape inside a string that is not classed as JSON", isJSON: false,
+			in:   `{"a":"x` + esc + `y"}`,
+			want: `{"a":"x` + esc + `y"}`,
+			why:  "classing decides. In a text column these are six characters and rewriting them is the false positive.",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := Repair(tc.in, tc.isJSON)
+			if got != tc.want {
+				t.Errorf("%s\n  in:   %q\n  got:  %q\n  want: %q", tc.why, tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestRepairConstantsDidNotDecay is the same guard corpus_test.go keeps over
+// NUL and EscNUL, extended to the two this file adds.
+//
+// It compares against values it BUILDS rather than ones it types, because
+// typing the six characters is what produces the character (measured: it
+// happened twice while writing this unit).
+func TestRepairConstantsDidNotDecay(t *testing.T) {
+	if []rune(Replacement)[0] != 0xFFFD || len([]rune(Replacement)) != 1 {
+		t.Errorf("Replacement is not a single U+FFFD: %q", Replacement)
+	}
+	if len(ReplacementEscape) != 6 ||
+		ReplacementEscape[0] != '\\' || ReplacementEscape[1] != 'u' ||
+		ReplacementEscape[2:] != "fffd" {
+		t.Errorf("ReplacementEscape decayed: %q", ReplacementEscape)
+	}
+	// And the two must mean the same thing to a JSON parser.
+	var viaEscape string
+	if err := json.Unmarshal([]byte(`"`+ReplacementEscape+`"`), &viaEscape); err != nil {
+		t.Fatalf("ReplacementEscape is not a valid JSON escape: %v", err)
+	}
+	if viaEscape != Replacement {
+		t.Errorf("the escape and the character disagree: %q vs %q", viaEscape, Replacement)
+	}
+}