diff --git a/sei-db/state_db/sc/memiavl/db.go b/sei-db/state_db/sc/memiavl/db.go index 6e9339c799..b342cbfd76 100644 --- a/sei-db/state_db/sc/memiavl/db.go +++ b/sei-db/state_db/sc/memiavl/db.go @@ -1,9 +1,11 @@ package memiavl import ( + "bytes" "context" "errors" "fmt" + "io/fs" "math" "os" "path/filepath" @@ -11,6 +13,7 @@ import ( "strconv" "strings" "sync" + "syscall" "time" "github.com/alitto/pond" @@ -27,6 +30,13 @@ const LockFileName = "LOCK" var errReadOnly = errors.New("db is read-only") +// errCorruptedSnapshot classifies snapshot data that is structurally invalid, +// as opposed to environmental failures (cancellation, file-handle or memory +// exhaustion) that say nothing about the data. Replacing a snapshot directory +// is justified only for errors carrying this sentinel, so every structural +// check on the snapshot load path must wrap it. +var errCorruptedSnapshot = errors.New("corrupted snapshot") + // DB implements DB-like functionalities on top of MultiTree: // - async snapshot rewriting // - Write-ahead-log @@ -766,19 +776,36 @@ func (db *DB) RewriteSnapshot(ctx context.Context) error { snapshotDir := snapshotName(db.lastCommitInfo.Version) targetPath := filepath.Clean(filepath.Join(db.dir, snapshotDir)) - // Check if snapshot already exists - if info, err := os.Stat(targetPath); err == nil { - if info.IsDir() { - logger.Info("snapshot already exists, skipping", - "snapshot_dir", snapshotDir, - "version", db.lastCommitInfo.Version) - return nil - } else { + // A directory left by a prior attempt at this height is adopted when it + // holds the state this DB would publish; a corrupted one is rewritten. + if info, statErr := os.Stat(targetPath); statErr == nil { + if !info.IsDir() { // targetPath exists but is not a directory - this is unexpected logger.Error("snapshot path exists but is not a directory", "path", targetPath) return fmt.Errorf("snapshot path exists but is not a directory: %s", targetPath) } + validationErr := db.validateSnapshot(ctx, targetPath) + if validationErr == nil { + logger.Info("snapshot already exists, skipping", + "snapshot_dir", snapshotDir, + "version", db.lastCommitInfo.Version) + return nil + } + if !errors.Is(validationErr, errCorruptedSnapshot) { + // Cancellation and resource exhaustion say nothing about the + // directory's contents; deleting it here could strand the current + // symlink on a directory that was perfectly good. + return fmt.Errorf("validate existing snapshot %q: %w", targetPath, validationErr) + } + // Fall through and rewrite. The corrupted directory — possibly what the + // current symlink points at — is not deleted here: publishSnapshot + // replaces it only after a freshly written, validated temp exists, so a + // failed write or a crash in the window cannot leave current dangling. + logger.Error("existing snapshot is corrupted, rewriting", + "path", targetPath, + "error", validationErr, + ) } tmpDir := snapshotDir + "-tmp" @@ -789,50 +816,139 @@ func (db *DB) RewriteSnapshot(ctx context.Context) error { writeElapsed := time.Since(writeStart).Seconds() if err != nil { - logger.Error("snapshot write failed, cleaning up temporary directory", - "tmpDir", tmpDir, - "error", err, - ) - cleanupErr := os.RemoveAll(path) - if cleanupErr != nil { - logger.Error("failed to clean up temporary snapshot directory", - "tmpDir", tmpDir, - "cleanup_error", cleanupErr, - ) - } else { - logger.Debug("temporary snapshot directory cleaned up successfully", - "tmpDir", tmpDir, - ) - } - return errorutils.Join(err, cleanupErr) + return cleanupFailedSnapshotRewrite(path, tmpDir, "snapshot write failed", err) + } + + if err := db.publishSnapshot(ctx, path, targetPath, snapshotDir); err != nil { + return cleanupFailedSnapshotRewrite(path, tmpDir, "snapshot publication failed", err) } logger.Info("snapshot rewrite completed", "duration_sec", writeElapsed) + return nil +} + +// publishSnapshot validates and publishes a completed snapshot. +func (db *DB) publishSnapshot(ctx context.Context, path, targetPath, snapshotDir string) error { + if err := db.validateSnapshot(ctx, path); err != nil { + return fmt.Errorf("validate temporary snapshot: %w", err) + } - // Rename temporary directory to final location if err := os.Rename(path, targetPath); err != nil { - logger.Error("failed to rename snapshot directory, cleaning up", - "tmpDir", tmpDir, - "targetDir", snapshotDir, - "error", err, - ) - // Clean up temporary directory on rename failure - if cleanupErr := os.RemoveAll(path); cleanupErr != nil { - logger.Error("failed to clean up temporary snapshot directory after rename failure", - "tmpDir", tmpDir, - "cleanup_error", cleanupErr, + // An existing snapshot- directory (from a prior atomic rename) can be + // used; drop our redundant temp rather than failing this rewrite. Only a + // directory is a valid prior snapshot -- a non-directory at the path is + // corruption/external interference and must not be adopted. + if errors.Is(err, fs.ErrExist) || errors.Is(err, syscall.ENOTEMPTY) { + if info, statErr := os.Stat(targetPath); statErr != nil || !info.IsDir() { + return fmt.Errorf("snapshot path %q exists but is not a usable directory: %w", targetPath, err) + } + if validationErr := db.validateSnapshot(ctx, targetPath); validationErr != nil { + if !errors.Is(validationErr, errCorruptedSnapshot) { + // Cancellation and resource exhaustion say nothing about + // the directory's contents; deleting it here could strand + // the current symlink on a directory that was perfectly good. + return fmt.Errorf("validate existing snapshot %q: %w", targetPath, validationErr) + } + // The freshly written temp already passed validation; it + // replaces the corrupted directory. + logger.Error("existing snapshot is corrupted, replacing with freshly written snapshot", + "path", targetPath, + "error", validationErr, + ) + if rmErr := os.RemoveAll(targetPath); rmErr != nil { + return fmt.Errorf("existing snapshot %q is corrupted and could not be removed: %w", + targetPath, errorutils.Join(validationErr, rmErr)) + } + if renameErr := os.Rename(path, targetPath); renameErr != nil { + return fmt.Errorf("rename snapshot directory to %q: %w", targetPath, renameErr) + } + return updateCurrentSymlink(db.dir, snapshotDir) + } + logger.Info("reusing existing snapshot directory, dropping redundant temp", + "snapshotDir", snapshotDir, ) - return errorutils.Join(err, cleanupErr) + if rmErr := os.RemoveAll(path); rmErr != nil { + return rmErr + } + return updateCurrentSymlink(db.dir, snapshotDir) } - logger.Info("temporary snapshot directory cleaned up after rename failure", - "tmpDir", tmpDir, - ) - return err + return fmt.Errorf("rename snapshot directory to %q: %w", targetPath, err) } return updateCurrentSymlink(db.dir, snapshotDir) } +// validateSnapshot loads the snapshot at path and verifies it holds the state +// this DB would publish: the recorded multi-tree version and every store's +// version and root hash must match lastCommitInfo. File sizes and record +// alignment are checked by the load; interior nodes and key data are not +// otherwise verified. Failures that prove the data is bad carry +// errCorruptedSnapshot; environmental failures do not. +func (db *DB) validateSnapshot(ctx context.Context, path string) (returnErr error) { + opts := db.opts + opts.SnapshotPrefetchThreshold = 0 + mtree, err := LoadMultiTree(ctx, path, opts) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + // A published snapshot contains every file it references; a missing + // one is an incomplete or mangled directory, not a transient failure. + return fmt.Errorf("%w: %w", errCorruptedSnapshot, err) + } + return err + } + defer func() { + returnErr = errorutils.Join(returnErr, mtree.Close()) + }() + + if mtree.Version() != db.lastCommitInfo.Version { + return fmt.Errorf("%w: snapshot version %d does not match expected version %d", + errCorruptedSnapshot, mtree.Version(), db.lastCommitInfo.Version) + } + loaded := mtree.Trees() + if len(loaded) != len(db.lastCommitInfo.StoreInfos) { + return fmt.Errorf("%w: snapshot has %d stores, expected %d", + errCorruptedSnapshot, len(loaded), len(db.lastCommitInfo.StoreInfos)) + } + loadedTrees := make(map[string]*Tree, len(loaded)) + for _, entry := range loaded { + loadedTrees[entry.Name] = entry.Tree + } + for _, info := range db.lastCommitInfo.StoreInfos { + tree, ok := loadedTrees[info.Name] + if !ok { + return fmt.Errorf("%w: snapshot is missing store %q", errCorruptedSnapshot, info.Name) + } + if tree.Version() != info.CommitId.Version { + return fmt.Errorf("%w: snapshot store %q version %d does not match expected version %d", + errCorruptedSnapshot, info.Name, tree.Version(), info.CommitId.Version) + } + if !bytes.Equal(tree.RootHash(), info.CommitId.Hash) { + return fmt.Errorf("%w: snapshot store %q root hash does not match the expected commit hash", + errCorruptedSnapshot, info.Name) + } + } + return nil +} + +func cleanupFailedSnapshotRewrite(path, tmpDir, operation string, err error) error { + logger.Error(operation+", cleaning up temporary directory", + "tmpDir", tmpDir, + "error", err, + ) + cleanupErr := os.RemoveAll(path) + if cleanupErr != nil { + logger.Error("failed to clean up temporary snapshot directory", + "tmpDir", tmpDir, + "cleanup_error", cleanupErr, + ) + } else { + logger.Debug("temporary snapshot directory cleaned up successfully", + "tmpDir", tmpDir, + ) + } + return errorutils.Join(err, cleanupErr) +} + func (db *DB) Reload() error { db.mtx.Lock() defer db.mtx.Unlock() @@ -1231,6 +1347,11 @@ func initEmptyDB(dir string, initialVersion uint32) error { // it could fail under concurrent usage for tmp file conflicts. func updateCurrentSymlink(dir, snapshot string) error { tmpPath := currentTmpPath(dir) + // A crash between Symlink and Rename can leave current-tmp behind; remove it + // so a re-offered restore is idempotent rather than failing with EEXIST. + if err := os.Remove(tmpPath); err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } if err := os.Symlink(snapshot, tmpPath); err != nil { return err } @@ -1290,10 +1411,19 @@ func atomicRemoveDir(path string) error { // createDBIfNotExist detects if db does not exist and try to initialize an empty one. func createDBIfNotExist(dir string, initialVersion uint32) error { _, err := os.Stat(filepath.Join(dir, "current", MetadataFileName)) - if err != nil && os.IsNotExist(err) { - return initEmptyDB(dir, initialVersion) + if err == nil || !os.IsNotExist(err) { + return nil } - return nil + // Stat through a dangling symlink reports not-exist, which is + // indistinguishable from a directory that was never initialized. A current + // link pointing at a missing snapshot is evidence of data loss; initializing + // here would silently reset the store to an empty version 0. + if _, lstatErr := os.Lstat(currentPath(dir)); lstatErr == nil { + return fmt.Errorf("current link at %q points to a missing snapshot; refusing to initialize an empty db over it", currentPath(dir)) + } else if !os.IsNotExist(lstatErr) { + return lstatErr + } + return initEmptyDB(dir, initialVersion) } func isSnapshotName(name string) bool { diff --git a/sei-db/state_db/sc/memiavl/db_rewrite_test.go b/sei-db/state_db/sc/memiavl/db_rewrite_test.go index 7cc75446d9..461592feda 100644 --- a/sei-db/state_db/sc/memiavl/db_rewrite_test.go +++ b/sei-db/state_db/sc/memiavl/db_rewrite_test.go @@ -86,3 +86,333 @@ func TestLoadMultiTreeWithPrefetchDisabled(t *testing.T) { tree := db2.TreeByName("test") require.NotNil(t, tree) } + +func TestPublishSnapshotRejectsCorruptCandidate(t *testing.T) { + dir := t.TempDir() + db, err := OpenDB(0, Options{ + Dir: dir, + CreateIfMissing: true, + InitialStores: []string{"test"}, + }) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + + require.NoError(t, db.ApplyChangeSets([]*proto.NamedChangeSet{{ + Name: "test", + Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{{ + Key: []byte("key"), + Value: []byte("value"), + }}}, + }})) + _, err = db.Commit() + require.NoError(t, err) + + snapshotDir := snapshotName(db.Version()) + tmpPath := filepath.Join(dir, snapshotDir+"-tmp") + targetPath := filepath.Join(dir, snapshotDir) + require.NoError(t, db.MultiTree.WriteSnapshot(context.Background(), tmpPath, db.snapshotWriterPool)) + + leavesPath := filepath.Join(tmpPath, "test", FileNameLeaves) + leaves, err := os.OpenFile(leavesPath, os.O_WRONLY|os.O_APPEND, 0) + require.NoError(t, err) + _, err = leaves.Write(make([]byte, SizeLeafWithoutHash)) + require.NoError(t, err) + require.NoError(t, leaves.Close()) + + err = db.publishSnapshot(context.Background(), tmpPath, targetPath, snapshotDir) + require.ErrorContains(t, err, "corrupted snapshot, leaves file size") + require.NoDirExists(t, targetPath) + + current, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.Equal(t, snapshotName(0), current) +} + +// openCommittedDB opens a fresh DB with one store and one committed change, +// which is the smallest state that exercises the snapshot rewrite paths. +func openCommittedDB(t *testing.T) (*DB, string) { + t.Helper() + dir := t.TempDir() + db, err := OpenDB(0, Options{ + Dir: dir, + CreateIfMissing: true, + InitialStores: []string{"test"}, + }) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, db.Close()) }) + + require.NoError(t, db.ApplyChangeSets([]*proto.NamedChangeSet{{ + Name: "test", + Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{{ + Key: []byte("key"), + Value: []byte("value"), + }}}, + }})) + _, err = db.Commit() + require.NoError(t, err) + return db, dir +} + +// writeCorruptSnapshotDir plants a directory at path that fails validation. +func writeCorruptSnapshotDir(t *testing.T, path string) { + t.Helper() + require.NoError(t, os.MkdirAll(path, 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(path, MetadataFileName), []byte("garbage"), 0o600)) +} + +func TestRewriteSnapshotSkipsValidExistingSnapshot(t *testing.T) { + db, dir := openCommittedDB(t) + require.NoError(t, db.RewriteSnapshot(context.Background())) + + targetPath := filepath.Join(dir, snapshotName(db.Version())) + before, err := os.Stat(filepath.Join(targetPath, MetadataFileName)) + require.NoError(t, err) + + require.NoError(t, db.RewriteSnapshot(context.Background())) + + after, err := os.Stat(filepath.Join(targetPath, MetadataFileName)) + require.NoError(t, err) + require.Equal(t, before.ModTime(), after.ModTime(), "a valid existing snapshot must be adopted, not rewritten") + require.NoDirExists(t, targetPath+"-tmp") +} + +func TestRewriteSnapshotReplacesCorruptExistingSnapshot(t *testing.T) { + db, dir := openCommittedDB(t) + snapshotDir := snapshotName(db.Version()) + targetPath := filepath.Join(dir, snapshotDir) + writeCorruptSnapshotDir(t, targetPath) + + require.NoError(t, db.RewriteSnapshot(context.Background())) + + require.NoError(t, db.validateSnapshot(context.Background(), targetPath)) + current, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.Equal(t, snapshotDir, current) +} + +func TestPublishSnapshotReplacesCorruptExistingTarget(t *testing.T) { + db, dir := openCommittedDB(t) + snapshotDir := snapshotName(db.Version()) + tmpPath := filepath.Join(dir, snapshotDir+"-tmp") + targetPath := filepath.Join(dir, snapshotDir) + require.NoError(t, db.MultiTree.WriteSnapshot(context.Background(), tmpPath, db.snapshotWriterPool)) + writeCorruptSnapshotDir(t, targetPath) + + require.NoError(t, db.publishSnapshot(context.Background(), tmpPath, targetPath, snapshotDir)) + + require.NoError(t, db.validateSnapshot(context.Background(), targetPath)) + require.NoDirExists(t, tmpPath) + current, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.Equal(t, snapshotDir, current) +} + +func TestPublishSnapshotAdoptsValidExistingTarget(t *testing.T) { + db, dir := openCommittedDB(t) + snapshotDir := snapshotName(db.Version()) + tmpPath := filepath.Join(dir, snapshotDir+"-tmp") + targetPath := filepath.Join(dir, snapshotDir) + require.NoError(t, db.MultiTree.WriteSnapshot(context.Background(), tmpPath, db.snapshotWriterPool)) + require.NoError(t, db.MultiTree.WriteSnapshot(context.Background(), targetPath, db.snapshotWriterPool)) + + require.NoError(t, db.publishSnapshot(context.Background(), tmpPath, targetPath, snapshotDir)) + + require.NoDirExists(t, tmpPath, "the redundant temp must be dropped when the target is adopted") + current, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.Equal(t, snapshotDir, current) +} + +// TestRewriteSnapshotKeepsSnapshotOnCancelledValidation pins the boundary of +// the self-heal: a validation failure that says nothing about the directory's +// contents, such as a cancelled context at shutdown, must not delete a +// published snapshot out from under the current symlink. +func TestRewriteSnapshotKeepsSnapshotOnCancelledValidation(t *testing.T) { + db, dir := openCommittedDB(t) + require.NoError(t, db.RewriteSnapshot(context.Background())) + snapshotDir := snapshotName(db.Version()) + + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + err := db.RewriteSnapshot(cancelled) + + require.ErrorIs(t, err, context.Canceled) + require.DirExists(t, filepath.Join(dir, snapshotDir)) + current, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.Equal(t, snapshotDir, current) +} + +// TestRewriteSnapshotKeepsCorruptTargetUntilReplacementExists pins the +// replacement ordering: a corrupted directory — possibly what current points +// at — must survive until a freshly written, validated temp exists to take its +// place. A write that fails after corruption was detected must leave it alone. +func TestRewriteSnapshotKeepsCorruptTargetUntilReplacementExists(t *testing.T) { + db, dir := openCommittedDB(t) + snapshotDir := snapshotName(db.Version()) + targetPath := filepath.Join(dir, snapshotDir) + writeCorruptSnapshotDir(t, targetPath) + + // Corruption detection reads the planted metadata before any context + // check, so the cancelled context fails the snapshot write that follows, + // not the validation. + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + err := db.RewriteSnapshot(cancelled) + + require.Error(t, err) + require.DirExists(t, targetPath, "the corrupt directory must survive until a validated replacement exists") +} + +// TestPublishSnapshotKeepsTargetOnEnvironmentalValidationFailure mirrors +// TestRewriteSnapshotKeepsSnapshotOnCancelledValidation for the rename-conflict +// branch: an existing target whose validation fails for a reason that says +// nothing about its contents must be kept, not replaced. +func TestPublishSnapshotKeepsTargetOnEnvironmentalValidationFailure(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses file permissions, so an unreadable target cannot be simulated") + } + db, dir := openCommittedDB(t) + snapshotDir := snapshotName(db.Version()) + tmpPath := filepath.Join(dir, snapshotDir+"-tmp") + targetPath := filepath.Join(dir, snapshotDir) + require.NoError(t, db.MultiTree.WriteSnapshot(context.Background(), tmpPath, db.snapshotWriterPool)) + require.NoError(t, db.MultiTree.WriteSnapshot(context.Background(), targetPath, db.snapshotWriterPool)) + // An unreadable target fails validation with permission denied, which is + // environmental, not proof of corruption. + require.NoError(t, os.Chmod(targetPath, 0)) + t.Cleanup(func() { _ = os.Chmod(targetPath, 0o750) }) + + err := db.publishSnapshot(context.Background(), tmpPath, targetPath, snapshotDir) + + require.Error(t, err) + require.NotErrorIs(t, err, errCorruptedSnapshot) + require.DirExists(t, targetPath) +} + +// TestOpenDBRefusesDanglingCurrentLink pins startup behavior when the current +// link's snapshot has gone missing: that is evidence of data loss, and opening +// with CreateIfMissing must fail loudly rather than silently reinitialize an +// empty store at version 0. +func TestOpenDBRefusesDanglingCurrentLink(t *testing.T) { + dir := t.TempDir() + db, err := OpenDB(0, Options{ + Dir: dir, + CreateIfMissing: true, + InitialStores: []string{"test"}, + }) + require.NoError(t, err) + require.NoError(t, db.Close()) + + target, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.NoError(t, os.RemoveAll(filepath.Join(dir, target))) + + _, err = OpenDB(0, Options{ + Dir: dir, + CreateIfMissing: true, + InitialStores: []string{"test"}, + }) + require.ErrorContains(t, err, "refusing to initialize") +} + +// TestLoadMultiTreeRejectsNegativeInitialVersion covers corrupted metadata +// carrying a negative initial version, which panics in setInitialVersion if it +// survives the load. It must fail as corruption instead. +func TestLoadMultiTreeRejectsNegativeInitialVersion(t *testing.T) { + dir := t.TempDir() + metadata := proto.MultiTreeMetadata{ + CommitInfo: &proto.CommitInfo{Version: 1}, + InitialVersion: -1, + } + bz, err := metadata.Marshal() + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, MetadataFileName), bz, 0o600)) + + _, err = LoadMultiTree(context.Background(), dir, Options{}) + + require.ErrorIs(t, err, errCorruptedSnapshot) + require.ErrorContains(t, err, "out of uint32 range") +} + +// TestLoadMultiTreeRejectsMetadataWithoutCommitInfo covers the metadata shape +// an unclean shutdown leaves behind: a file that unmarshals successfully but +// carries no commit info. Loading it must fail as corruption, not panic. +func TestLoadMultiTreeRejectsMetadataWithoutCommitInfo(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, MetadataFileName), []byte{}, 0o600)) + + _, err := LoadMultiTree(context.Background(), dir, Options{}) + + require.ErrorIs(t, err, errCorruptedSnapshot) + require.ErrorContains(t, err, "no commit info") +} + +func TestRewriteSnapshotReplacesSnapshotWithEmptyMetadata(t *testing.T) { + db, dir := openCommittedDB(t) + snapshotDir := snapshotName(db.Version()) + targetPath := filepath.Join(dir, snapshotDir) + require.NoError(t, os.MkdirAll(targetPath, 0o750)) + require.NoError(t, os.WriteFile(filepath.Join(targetPath, MetadataFileName), []byte{}, 0o600)) + + require.NoError(t, db.RewriteSnapshot(context.Background())) + + require.NoError(t, db.validateSnapshot(context.Background(), targetPath)) + current, err := os.Readlink(currentPath(dir)) + require.NoError(t, err) + require.Equal(t, snapshotDir, current) +} + +// TestValidateSnapshotRejectsStoreVersionSkew pins the per-store version +// comparison. An empty commit advances every version while leaving root hashes +// unchanged, so a directory mixing an old store with a new multi-tree metadata +// is caught only by comparing each store's own version. +func TestValidateSnapshotRejectsStoreVersionSkew(t *testing.T) { + db, dir := openCommittedDB(t) + require.NoError(t, db.RewriteSnapshot(context.Background())) + oldSnapshot := filepath.Join(dir, snapshotName(db.Version())) + + _, err := db.Commit() + require.NoError(t, err) + require.NoError(t, db.RewriteSnapshot(context.Background())) + newSnapshot := filepath.Join(dir, snapshotName(db.Version())) + + skewed := filepath.Join(dir, "skewed-snapshot") + require.NoError(t, os.CopyFS(skewed, os.DirFS(oldSnapshot))) + metadata, err := os.ReadFile(filepath.Join(newSnapshot, MetadataFileName)) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(skewed, MetadataFileName), metadata, 0o600)) + + err = db.validateSnapshot(context.Background(), skewed) + require.ErrorIs(t, err, errCorruptedSnapshot) + require.ErrorContains(t, err, "does not match expected version") +} + +func TestValidateSnapshotComparesCommitInfo(t *testing.T) { + db, dir := openCommittedDB(t) + require.NoError(t, db.RewriteSnapshot(context.Background())) + staleTarget := filepath.Join(dir, snapshotName(db.Version())) + + // Another commit moves lastCommitInfo past the published snapshot. + require.NoError(t, db.ApplyChangeSets([]*proto.NamedChangeSet{{ + Name: "test", + Changeset: proto.ChangeSet{Pairs: []*proto.KVPair{{ + Key: []byte("key"), + Value: []byte("value2"), + }}}, + }})) + _, err := db.Commit() + require.NoError(t, err) + + err = db.validateSnapshot(context.Background(), staleTarget) + require.ErrorContains(t, err, "does not match expected version") + + // A snapshot at the right version but holding different content must be + // rejected on its root hash. + require.NoError(t, db.RewriteSnapshot(context.Background())) + freshTarget := filepath.Join(dir, snapshotName(db.Version())) + require.NoError(t, db.validateSnapshot(context.Background(), freshTarget)) + db.lastCommitInfo.StoreInfos[0].CommitId.Hash = []byte("not the real root hash") + err = db.validateSnapshot(context.Background(), freshTarget) + require.ErrorContains(t, err, "root hash does not match") +} diff --git a/sei-db/state_db/sc/memiavl/layout_native.go b/sei-db/state_db/sc/memiavl/layout_native.go index 46a5bc88a5..2ca2c38a0f 100644 --- a/sei-db/state_db/sc/memiavl/layout_native.go +++ b/sei-db/state_db/sc/memiavl/layout_native.go @@ -3,7 +3,7 @@ package memiavl import ( - "errors" + "fmt" "unsafe" ) @@ -27,11 +27,11 @@ func NewNodes(buf []byte) (Nodes, error) { // check alignment and size of the buffer p := unsafe.Pointer(unsafe.SliceData(buf)) if uintptr(p)%unsafe.Alignof(nodeLayout{}) != 0 { - return Nodes{}, errors.New("input buffer is not aligned") + return Nodes{}, fmt.Errorf("%w: nodes buffer is not aligned", errCorruptedSnapshot) } size := int(unsafe.Sizeof(nodeLayout{})) if len(buf)%size != 0 { - return Nodes{}, errors.New("input buffer length is not correct") + return Nodes{}, fmt.Errorf("%w: nodes buffer length is not correct", errCorruptedSnapshot) } nodes := unsafe.Slice((*nodeLayout)(p), len(buf)/size) return Nodes{nodes}, nil @@ -82,11 +82,11 @@ func NewLeaves(buf []byte) (Leaves, error) { // check alignment and size of the buffer p := unsafe.Pointer(unsafe.SliceData(buf)) if uintptr(p)%unsafe.Alignof(leafLayout{}) != 0 { - return Leaves{}, errors.New("input buffer is not aligned") + return Leaves{}, fmt.Errorf("%w: leaves buffer is not aligned", errCorruptedSnapshot) } size := int(unsafe.Sizeof(leafLayout{})) if len(buf)%size != 0 { - return Leaves{}, errors.New("input buffer length is not correct") + return Leaves{}, fmt.Errorf("%w: leaves buffer length is not correct", errCorruptedSnapshot) } leaves := unsafe.Slice((*leafLayout)(p), len(buf)/size) return Leaves{leaves}, nil diff --git a/sei-db/state_db/sc/memiavl/multitree.go b/sei-db/state_db/sc/memiavl/multitree.go index 6e9101e339..25bd5815d3 100644 --- a/sei-db/state_db/sc/memiavl/multitree.go +++ b/sei-db/state_db/sc/memiavl/multitree.go @@ -88,10 +88,20 @@ func LoadMultiTree(ctx context.Context, dir string, opts Options) (*MultiTree, e treeMap := make(map[string]*Tree, len(entries)) treeNames := make([]string, 0, len(entries)) + // closeOpened releases the snapshots already opened when the load cannot + // complete, so a failed load does not leak their mmaps and file handles. + closeOpened := func() { + for _, tree := range treeMap { + if closeErr := tree.Close(); closeErr != nil { + logger.Error("failed to close partially loaded tree", "error", closeErr) + } + } + } for _, e := range entries { // Check for cancellation select { case <-ctx.Done(): + closeOpened() return nil, ctx.Err() default: } @@ -102,6 +112,7 @@ func LoadMultiTree(ctx context.Context, dir string, opts Options) (*MultiTree, e treeNames = append(treeNames, name) snapshot, err := OpenSnapshot(filepath.Join(dir, name), opts) if err != nil { + closeOpened() return nil, err } treeMap[name] = NewFromSnapshot(snapshot, opts) @@ -608,13 +619,22 @@ func readMetadata(dir string) (*proto.MultiTreeMetadata, error) { } var metadata proto.MultiTreeMetadata if err := metadata.Unmarshal(bz); err != nil { - return nil, err - } - if metadata.CommitInfo.Version > math.MaxUint32 { - return nil, fmt.Errorf("commit info version overflows uint32: %d", metadata.CommitInfo.Version) - } - if metadata.InitialVersion > math.MaxUint32 { - return nil, fmt.Errorf("initial version overflows uint32: %d", metadata.InitialVersion) + return nil, fmt.Errorf("%w: unmarshal metadata: %w", errCorruptedSnapshot, err) + } + if metadata.CommitInfo == nil { + // An empty or truncated metadata file unmarshals successfully but + // carries no commit info; reject it here so every load path returns a + // validation failure instead of dereferencing nil. + return nil, fmt.Errorf("%w: metadata has no commit info", errCorruptedSnapshot) + } + // Both fields are int64 varints on disk, so corrupted metadata can carry + // negative values as easily as oversized ones; setInitialVersion panics on + // a negative initial version if it gets that far. + if metadata.CommitInfo.Version < 0 || metadata.CommitInfo.Version > math.MaxUint32 { + return nil, fmt.Errorf("%w: commit info version %d out of uint32 range", errCorruptedSnapshot, metadata.CommitInfo.Version) + } + if metadata.InitialVersion < 0 || metadata.InitialVersion > math.MaxUint32 { + return nil, fmt.Errorf("%w: initial version %d out of uint32 range", errCorruptedSnapshot, metadata.InitialVersion) } return &metadata, nil diff --git a/sei-db/state_db/sc/memiavl/snapshot.go b/sei-db/state_db/sc/memiavl/snapshot.go index 97417f1b19..d339382b86 100644 --- a/sei-db/state_db/sc/memiavl/snapshot.go +++ b/sei-db/state_db/sc/memiavl/snapshot.go @@ -169,16 +169,16 @@ func OpenSnapshot(snapshotDir string, opts Options) (*Snapshot, error) { return nil, err } if len(bz) != SizeMetadata { - return nil, fmt.Errorf("wrong metadata file size, expcted: %d, found: %d", SizeMetadata, len(bz)) + return nil, fmt.Errorf("%w: wrong metadata file size, expcted: %d, found: %d", errCorruptedSnapshot, SizeMetadata, len(bz)) } magic := binary.LittleEndian.Uint32(bz) if magic != SnapshotFileMagic { - return nil, fmt.Errorf("invalid metadata file magic: %d", magic) + return nil, fmt.Errorf("%w: invalid metadata file magic: %d", errCorruptedSnapshot, magic) } format := binary.LittleEndian.Uint32(bz[4:]) if format != SnapshotFormat { - return nil, fmt.Errorf("unknown snapshot format: %d", format) + return nil, fmt.Errorf("%w: unknown snapshot format: %d", errCorruptedSnapshot, format) } version := binary.LittleEndian.Uint32(bz[8:]) @@ -216,12 +216,12 @@ func OpenSnapshot(snapshotDir string, opts Options) (*Snapshot, error) { // validate nodes length if len(nodes)%SizeNode != 0 { return nil, cleanupHandles( - fmt.Errorf("corrupted snapshot, nodes file size %d is not a multiple of %d", len(nodes), SizeNode), + fmt.Errorf("%w, nodes file size %d is not a multiple of %d", errCorruptedSnapshot, len(nodes), SizeNode), ) } if len(leaves)%SizeLeaf != 0 { return nil, cleanupHandles( - fmt.Errorf("corrupted snapshot, leaves file size %d is not a multiple of %d", len(leaves), SizeLeaf), + fmt.Errorf("%w, leaves file size %d is not a multiple of %d", errCorruptedSnapshot, len(leaves), SizeLeaf), ) } @@ -229,7 +229,7 @@ func OpenSnapshot(snapshotDir string, opts Options) (*Snapshot, error) { leavesLen := len(leaves) / SizeLeaf if (leavesLen > 0 && nodesLen+1 != leavesLen) || (leavesLen == 0 && nodesLen != 0) { return nil, cleanupHandles( - fmt.Errorf("corrupted snapshot, branch nodes size %d don't match leaves size %d", nodesLen, leavesLen), + fmt.Errorf("%w, branch nodes size %d don't match leaves size %d", errCorruptedSnapshot, nodesLen, leavesLen), ) } @@ -946,8 +946,21 @@ func (w *snapshotWriter) writeLeaf(version uint32, key, value, hash []byte) erro } } +// checkNodeHash rejects a hash whose length is not the fixed 32-byte record +// size, so a malformed hash cannot produce a misaligned snapshot file. +func checkNodeHash(kind string, hash []byte) error { + if len(hash) != SizeHash { + return fmt.Errorf("invalid %s hash size %d, expected %d", kind, len(hash), SizeHash) + } + return nil +} + // writeLeafDirect performs the actual leaf write (called by writer goroutine) func (w *snapshotWriter) writeLeafDirect(version uint32, keyLen uint32, keyOffset uint64, hash []byte) error { + if err := checkNodeHash("leaf", hash); err != nil { + return err + } + var buf [SizeLeafWithoutHash]byte binary.LittleEndian.PutUint32(buf[OffsetLeafVersion:], version) binary.LittleEndian.PutUint32(buf[OffsetLeafKeyLen:], keyLen) @@ -990,6 +1003,10 @@ func (w *snapshotWriter) writeBranch(version, size uint32, height, preTrees uint // writeBranchDirect performs the actual branch write (called by writer goroutine) func (w *snapshotWriter) writeBranchDirect(version, size uint32, height, preTrees uint8, keyLeaf uint32, hash []byte) error { + if err := checkNodeHash("branch", hash); err != nil { + return err + } + var buf [SizeNodeWithoutHash]byte buf[OffsetHeight] = height buf[OffsetPreTrees] = preTrees diff --git a/sei-db/state_db/sc/memiavl/snapshot_pipeline_test.go b/sei-db/state_db/sc/memiavl/snapshot_pipeline_test.go index 891d585ada..a63a50da01 100644 --- a/sei-db/state_db/sc/memiavl/snapshot_pipeline_test.go +++ b/sei-db/state_db/sc/memiavl/snapshot_pipeline_test.go @@ -210,6 +210,28 @@ func TestSnapshotWriterErrorHandling(t *testing.T) { require.Error(t, err) } +func TestSnapshotWriterRejectsInvalidHashLengths(t *testing.T) { + t.Run("leaf", func(t *testing.T) { + var leaves bytes.Buffer + writer := &snapshotWriter{leavesWriter: &leaves} + + err := writer.writeLeafDirect(1, 1, 0, nil) + + require.ErrorContains(t, err, "invalid leaf hash size 0, expected 32") + require.Empty(t, leaves.Bytes()) + }) + + t.Run("branch", func(t *testing.T) { + var nodes bytes.Buffer + writer := &snapshotWriter{nodesWriter: &nodes} + + err := writer.writeBranchDirect(1, 2, 1, 0, 1, make([]byte, SizeHash-1)) + + require.ErrorContains(t, err, "invalid branch hash size 31, expected 32") + require.Empty(t, nodes.Bytes()) + }) +} + // TestEmptySnapshotWrite tests writing an empty snapshot func TestEmptySnapshotWrite(t *testing.T) { tree := New(0)