diff --git a/pkg/appview/db/manifest_key_swap_test.go b/pkg/appview/db/manifest_key_swap_test.go index f703cae..1234751 100644 --- a/pkg/appview/db/manifest_key_swap_test.go +++ b/pkg/appview/db/manifest_key_swap_test.go @@ -375,3 +375,184 @@ func seedPublicHold(t *testing.T, database *sql.DB) { t.Fatalf("seed hold: %v", err) } } + +// reshapeToPre0034Unenforced builds the same pre-0034 shape as +// reshapeToPre0034WithChildren, but without the foreign keys on the two child +// tables. That is the state a real database can be in: libSQL enables foreign +// keys by default, mattn/go-sqlite3 did not, and rows written under the old +// driver were never checked against the constraint the DDL declared. +func reshapeToPre0034Unenforced(t *testing.T, database *sql.DB) { + t.Helper() + stmts := []string{ + `DROP TABLE layers`, + `DROP TABLE manifest_references`, + `DROP TABLE manifests`, + `CREATE TABLE manifests ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + did TEXT NOT NULL, + repository TEXT NOT NULL, + digest TEXT NOT NULL, + hold_endpoint TEXT NOT NULL, + schema_version INTEGER NOT NULL, + media_type TEXT NOT NULL, + config_digest TEXT, + config_size INTEGER, + artifact_type TEXT NOT NULL DEFAULT 'container-image', + subject_digest TEXT, + created_at TIMESTAMP NOT NULL, + manifest_key TEXT, + UNIQUE(did, repository, digest), + FOREIGN KEY(did) REFERENCES users(did) ON DELETE CASCADE + )`, + `CREATE TABLE layers ( + manifest_id INTEGER NOT NULL, + digest TEXT NOT NULL, + size INTEGER NOT NULL, + media_type TEXT NOT NULL, + layer_index INTEGER NOT NULL, + annotations TEXT, + PRIMARY KEY(manifest_id, layer_index) + )`, + `CREATE TABLE manifest_references ( + manifest_id INTEGER NOT NULL, + digest TEXT NOT NULL, + media_type TEXT NOT NULL, + size INTEGER NOT NULL, + platform_architecture TEXT, + platform_os TEXT, + platform_variant TEXT, + platform_os_version TEXT, + is_attestation BOOLEAN DEFAULT FALSE, + reference_index INTEGER NOT NULL, + PRIMARY KEY(manifest_id, reference_index) + )`, + } + for _, stmt := range stmts { + if _, err := database.Exec(stmt); err != nil { + t.Fatalf("reshape: %v\n%s", err, stmt) + } + } +} + +// TestMigration0034DropsOrphanedChildren covers the case the sibling test +// cannot reach, and the one the production database actually presents. +// +// The sibling seeds no orphan because the live foreign key refuses to create +// one, and reasons that the migration's join is "insurance for a database whose +// foreign keys were off at some point". Production is exactly that database: +// 160 layers and 22 manifest_references there point at manifests.id values that +// no longer exist, left behind by deletes performed under mattn/go-sqlite3 +// where the constraint was not enforced. So the insurance is load-bearing on +// the only database that matters, and nothing tested it. +// +// What must hold is narrow and total: orphans are discarded, live rows survive +// attached to the right parent, and nothing lands with an empty key. That last +// assertion is the one with teeth — carrying an orphan forward under a nullable +// key produces a row that satisfies every count and joins to nothing. +func TestMigration0034DropsOrphanedChildren(t *testing.T) { + database := revTestDB(t) + reshapeToPre0034Unenforced(t, database) + + const did = "did:plc:orphans" + if _, err := database.Exec( + `INSERT INTO users (did, handle, pds_endpoint, last_seen) VALUES (?, ?, ?, ?)`, + did, "orphans.example.com", "https://pds.example.com", time.Now(), + ); err != nil { + t.Fatalf("seed user: %v", err) + } + + res, err := database.Exec(` + INSERT INTO manifests (did, repository, digest, hold_endpoint, schema_version, media_type, artifact_type, created_at, manifest_key) + VALUES (?, 'myapp', 'sha256:live', 'did:web:hold', 2, 'application/vnd.oci.image.manifest.v1+json', 'container-image', ?, ?) + `, did, time.Now(), ManifestKey(did, "myapp", "sha256:live")) + if err != nil { + t.Fatalf("seed manifest: %v", err) + } + liveID, err := res.LastInsertId() + if err != nil { + t.Fatalf("last insert id: %v", err) + } + + for i := range 2 { + if _, err := database.Exec(` + INSERT INTO layers (manifest_id, digest, size, media_type, layer_index) + VALUES (?, 'sha256:livelayer', 100, 'application/vnd.oci.image.layer.v1.tar+gzip', ?) + `, liveID, i); err != nil { + t.Fatalf("seed live layer: %v", err) + } + } + if _, err := database.Exec(` + INSERT INTO manifest_references (manifest_id, digest, media_type, size, platform_architecture, platform_os, reference_index) + VALUES (?, 'sha256:liveref', 'application/vnd.oci.image.manifest.v1+json', 500, 'amd64', 'linux', 0) + `, liveID); err != nil { + t.Fatalf("seed live reference: %v", err) + } + + // The orphans: children of a manifest id that does not exist. + const goneID = 999999 + for i := range 3 { + if _, err := database.Exec(` + INSERT INTO layers (manifest_id, digest, size, media_type, layer_index) + VALUES (?, 'sha256:orphanlayer', 100, 'application/vnd.oci.image.layer.v1.tar+gzip', ?) + `, goneID, i); err != nil { + t.Fatalf("seed orphan layer: %v", err) + } + } + for i := range 2 { + if _, err := database.Exec(` + INSERT INTO manifest_references (manifest_id, digest, media_type, size, platform_architecture, platform_os, reference_index) + VALUES (?, 'sha256:orphanref', 'application/vnd.oci.image.manifest.v1+json', 500, 'amd64', 'linux', ?) + `, goneID, i); err != nil { + t.Fatalf("seed orphan reference: %v", err) + } + } + + if got := countRows(t, database, `SELECT COUNT(*) FROM layers`); got != 5 { + t.Fatalf("fixture: layer count = %d, want 5 (2 live + 3 orphaned)", got) + } + + applyMigration(t, database, 34) + + liveKey := ManifestKey(did, "myapp", "sha256:live") + + if got := countRows(t, database, `SELECT COUNT(*) FROM layers`); got != 2 { + t.Errorf("layer count = %d, want 2: the three orphans must not survive", got) + } + if got := countRows(t, database, `SELECT COUNT(*) FROM layers WHERE manifest_key = ?`, liveKey); got != 2 { + t.Errorf("layers attached to the live manifest = %d, want 2", got) + } + if got := countRows(t, database, `SELECT COUNT(*) FROM manifest_references`); got != 1 { + t.Errorf("manifest_references count = %d, want 1: both orphans must not survive", got) + } + if got := countRows(t, database, `SELECT COUNT(*) FROM manifest_references WHERE manifest_key = ?`, liveKey); got != 1 { + t.Errorf("references attached to the live manifest = %d, want 1", got) + } + + // Nothing may land keyless. A carried-forward orphan under a nullable key + // still counts, still selects, and joins to no manifest ever again. + if got := countRows(t, database, + `SELECT COUNT(*) FROM layers WHERE manifest_key IS NULL OR manifest_key = ''`); got != 0 { + t.Errorf("%d layers have no manifest_key", got) + } + if got := countRows(t, database, + `SELECT COUNT(*) FROM manifest_references WHERE manifest_key IS NULL OR manifest_key = ''`); got != 0 { + t.Errorf("%d manifest_references have no manifest_key", got) + } + + // And the rebuild must leave the database referentially clean. + rows, err := database.Query(`PRAGMA foreign_key_check`) + if err != nil { + t.Fatalf("foreign_key_check: %v", err) + } + defer rows.Close() + var violations int + for rows.Next() { + violations++ + } + if err := rows.Err(); err != nil { + t.Fatalf("foreign_key_check rows: %v", err) + } + if violations != 0 { + t.Errorf("foreign_key_check reports %d violations after the swap", violations) + } +}