Add regression coverage for KDBX reopen cycles

2026-03-29 11:26:14 -07:00
parent 43d253aa21
commit c5e2df4ca7
3 changed files with 330 additions and 34 deletions
@@ -537,3 +537,132 @@ func TestSavePreservesOpenedKDBXSecuritySettings(t *testing.T) {
 		t.Fatalf("saved KDF UUID = %x, want %x", reloaded.Header.FileHeaders.KdfParameters.UUID, db.Header.FileHeaders.KdfParameters.UUID)
 	}
 }
+
+func TestRemoteSaveAndReopenPreservesCrossFeatureState(t *testing.T) {
+	t.Parallel()
+
+	key := vault.MasterKey{Password: "correct horse battery staple"}
+	model := vault.Model{
+		Entries: []vault.Entry{
+			{
+				ID:       "entry-1",
+				Title:    "Vault Console",
+				Username: "dannyocean",
+				Password: "token-2",
+				URL:      "https://vault.crew.example.invalid",
+				Path:     []string{"Root", "Internet"},
+				Attachments: map[string][]byte{
+					"token.txt": []byte("secret attachment contents"),
+				},
+				History: []vault.Entry{
+					{
+						ID:       "entry-1-history-1",
+						Title:    "Vault Console",
+						Username: "dannyocean",
+						Password: "token-1",
+						URL:      "https://vault.crew.example.invalid",
+						Path:     []string{"Root", "Internet"},
+					},
+				},
+			},
+		},
+		Templates: []vault.Entry{
+			{
+				ID:       "tpl-1",
+				Title:    "Website Login",
+				Username: "template-user",
+				Password: "template-password",
+				Path:     []string{"Templates", "Web"},
+			},
+		},
+		RecycleBin: []vault.Entry{
+			{
+				ID:       "deleted-1",
+				Title:    "Retired Entry",
+				Username: "archived-user",
+				Password: "retired-token",
+				Path:     []string{"Root", "Archive"},
+			},
+		},
+		Groups: [][]string{
+			{"Root", "Archive"},
+			{"Root", "Empty Group"},
+			{"Templates", "Web"},
+		},
+	}
+
+	var remoteBytes bytes.Buffer
+	if err := vault.SaveKDBXWithKey(&remoteBytes, model, key); err != nil {
+		t.Fatalf("SaveKDBXWithKey(seed remote) error = %v", err)
+	}
+
+	etag := "\"v1\""
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.Method {
+		case http.MethodGet:
+			w.Header().Set("ETag", etag)
+			_, _ = w.Write(remoteBytes.Bytes())
+		case http.MethodPut:
+			if got := r.Header.Get("If-Match"); got != etag {
+				t.Fatalf("If-Match header = %q, want %q", got, etag)
+			}
+
+			payload, err := io.ReadAll(r.Body)
+			if err != nil {
+				t.Fatalf("ReadAll(PUT body) error = %v", err)
+			}
+
+			remoteBytes.Reset()
+			if _, err := remoteBytes.Write(payload); err != nil {
+				t.Fatalf("Write(remoteBytes) error = %v", err)
+			}
+
+			etag = "\"v2\""
+			w.Header().Set("ETag", etag)
+			w.WriteHeader(http.StatusNoContent)
+		default:
+			t.Fatalf("unexpected method %s", r.Method)
+		}
+	}))
+	defer server.Close()
+
+	client := webdav.Client{BaseURL: server.URL}
+
+	var sess Manager
+	if err := sess.OpenRemote(client, "vaults/main.kdbx", key); err != nil {
+		t.Fatalf("OpenRemote() error = %v", err)
+	}
+	if err := sess.SaveRemote(); err != nil {
+		t.Fatalf("SaveRemote() error = %v", err)
+	}
+
+	var reopened Manager
+	if err := reopened.OpenRemote(client, "vaults/main.kdbx", key); err != nil {
+		t.Fatalf("reopen OpenRemote() error = %v", err)
+	}
+
+	current, err := reopened.Current()
+	if err != nil {
+		t.Fatalf("Current() after reopen error = %v", err)
+	}
+
+	got := current.EntriesInPath([]string{"Root", "Internet"})
+	if len(got) != 1 {
+		t.Fatalf("len(EntriesInPath(Root/Internet)) after reopen = %d, want 1", len(got))
+	}
+	if got[0].ID != "entry-1" {
+		t.Fatalf("entry ID after remote reopen = %q, want %q", got[0].ID, "entry-1")
+	}
+	if len(got[0].History) != 1 || got[0].History[0].ID != "entry-1-history-1" {
+		t.Fatalf("History after remote reopen = %#v, want stable history ID entry-1-history-1", got[0].History)
+	}
+	if string(got[0].Attachments["token.txt"]) != "secret attachment contents" {
+		t.Fatalf("attachment after remote reopen = %q, want %q", string(got[0].Attachments["token.txt"]), "secret attachment contents")
+	}
+	if len(current.Templates) != 1 || current.Templates[0].Path[1] != "Web" {
+		t.Fatalf("Templates after remote reopen = %#v, want Website Login in Templates/Web", current.Templates)
+	}
+	if len(current.RecycleBin) != 1 || current.RecycleBin[0].Path[1] != "Archive" {
+		t.Fatalf("RecycleBin after remote reopen = %#v, want retired entry in Root/Archive", current.RecycleBin)
+	}
+}
@@ -23,8 +23,8 @@ type KDBXConfig struct {
 var ErrInvalidMasterKey = errors.New("invalid master key")

 const (
-	templatesRoot = "Templates"
-	recycleBinRoot = "Recycle Bin"
+	templatesRoot    = "Templates"
+	recycleBinRoot   = "Recycle Bin"
 	keepassGOIDField = "KeePassGO-ID"
 )

@@ -46,33 +46,29 @@ func SaveKDBXWithConfigAndKey(wr io.Writer, model Model, key MasterKey, config *
 		return err
 	}

-	header := gokeepasslib.NewHeader()
+	db := gokeepasslib.NewDatabase(gokeepasslib.WithDatabaseKDBXVersion4())
+	db.Credentials = credentials
+	db.Content.Meta = gokeepasslib.NewMetaData()
+	db.Content.Root = &gokeepasslib.RootData{}
 	if config != nil && config.Header != nil {
-		header = cloneHeader(config.Header)
+		db.Header = cloneHeader(config.Header)
+		db.Hashes = gokeepasslib.NewHashes(db.Header)
 	}
-	content := &gokeepasslib.DBContent{
-		Meta: gokeepasslib.NewMetaData(),
-		Root: &gokeepasslib.RootData{},
-	}
-	if header.IsKdbx4() {
+	if db.Header.IsKdbx4() {
 		if config != nil && config.InnerHeader != nil {
-			content.InnerHeader = cloneInnerHeader(config.InnerHeader)
-		} else {
-			content.InnerHeader = &gokeepasslib.InnerHeader{
+			db.Content.InnerHeader = cloneInnerHeader(config.InnerHeader)
+			db.Content.InnerHeader.Binaries = nil
+		} else if db.Content.InnerHeader == nil {
+			db.Content.InnerHeader = &gokeepasslib.InnerHeader{
 				InnerRandomStreamID:  gokeepasslib.ChaChaStreamID,
 				InnerRandomStreamKey: randomBytes(64),
 			}
 		}
+	} else {
+		db.Content.InnerHeader = nil
 	}
-
-	db := &gokeepasslib.Database{
-		Header:      header,
-		Credentials: credentials,
-		Content:     content,
-		Hashes:      gokeepasslib.NewHashes(header),
-	}
-	db.Content.Root.Groups = buildGroupTree(db, entriesForPersistence(model))
-	db.Content.Root.DeletedObjects = marshalDeletedObjects(model.RecycleBin)
+	db.Content.Root.Groups = buildGroupTree(db, model)
+	db.Content.Root.DeletedObjects = nil

 	if err := db.LockProtectedEntries(); err != nil {
 		return fmt.Errorf("lock protected entries: %w", err)
@@ -87,20 +83,21 @@ func SaveKDBXWithConfigAndKey(wr io.Writer, model Model, key MasterKey, config *

 func appendGroupEntries(model *Model, db *gokeepasslib.Database, group gokeepasslib.Group, path []string) {
 	path = append(clonePath(path), group.Name)
+	model.CreateGroup(path[:len(path)-1], group.Name)

 	for _, entry := range group.Entries {
 		appendModelEntry(model, Entry{
-			ID:       extractEntryID(entry),
-			Title:    entry.GetTitle(),
-			Username: entry.GetContent("UserName"),
-			Password: entry.GetPassword(),
-			URL:      entry.GetContent("URL"),
-			Notes:    entry.GetContent("Notes"),
-			Tags:     splitTags(entry.Tags),
-			Fields:   extractCustomFields(entry),
+			ID:          extractEntryID(entry),
+			Title:       entry.GetTitle(),
+			Username:    entry.GetContent("UserName"),
+			Password:    entry.GetPassword(),
+			URL:         entry.GetContent("URL"),
+			Notes:       entry.GetContent("Notes"),
+			Tags:        splitTags(entry.Tags),
+			Fields:      extractCustomFields(entry),
 			Attachments: extractAttachments(db, entry),
-			History:  extractHistory(db, entry, path),
-			Path:     clonePath(path),
+			History:     extractHistory(db, entry, path),
+			Path:        clonePath(path),
 		})
 	}

@@ -207,7 +204,7 @@ func extractHistory(db *gokeepasslib.Database, entry gokeepasslib.Entry, path []
 	for _, item := range entry.Histories {
 		for _, historical := range item.Entries {
 			history = append(history, Entry{
-				ID:          marshalUUID(historical.UUID),
+				ID:          extractEntryID(historical),
 				Title:       historical.GetTitle(),
 				Username:    historical.GetContent("UserName"),
 				Password:    historical.GetPassword(),
@@ -235,7 +232,8 @@ type MasterKey struct {
 	KeyFileData []byte
 }

-func buildGroupTree(db *gokeepasslib.Database, entries []Entry) []gokeepasslib.Group {
+func buildGroupTree(db *gokeepasslib.Database, model Model) []gokeepasslib.Group {
+	entries := entriesForPersistence(model)
 	root := &groupNode{children: map[string]*groupNode{}}
 	for _, entry := range entries {
 		node := root
@@ -250,6 +248,18 @@ func buildGroupTree(db *gokeepasslib.Database, entries []Entry) []gokeepasslib.G
 		}
 		node.entries = append(node.entries, entry)
 	}
+	for _, path := range groupPathsForPersistence(model, entries) {
+		node := root
+		for _, segment := range path {
+			if node.children[segment] == nil {
+				node.children[segment] = &groupNode{
+					name:     segment,
+					children: map[string]*groupNode{},
+				}
+			}
+			node = node.children[segment]
+		}
+	}

 	groups := marshalGroups(db, root)
 	if len(groups) > 0 {
@@ -261,6 +271,31 @@ func buildGroupTree(db *gokeepasslib.Database, entries []Entry) []gokeepasslib.G
 	return []gokeepasslib.Group{group}
 }

+func groupPathsForPersistence(model Model, entries []Entry) [][]string {
+	seen := map[string]bool{}
+	var groups [][]string
+	appendPath := func(path []string) {
+		key := strings.Join(path, "\x00")
+		if seen[key] {
+			return
+		}
+		seen[key] = true
+		groups = append(groups, slices.Clone(path))
+	}
+
+	for _, entry := range entries {
+		for i := 1; i <= len(entry.Path); i++ {
+			appendPath(entry.Path[:i])
+		}
+	}
+	for _, path := range model.Groups {
+		for i := 1; i <= len(path); i++ {
+			appendPath(path[:i])
+		}
+	}
+	return groups
+}
+
 func LoadKDBXWithKey(r io.Reader, key MasterKey) (Model, error) {
 	model, _, err := LoadKDBXWithConfig(r, key)
 	return model, err
@@ -407,7 +442,7 @@ func isInvalidCredentialError(err error) bool {

 func marshalGroups(db *gokeepasslib.Database, node *groupNode) []gokeepasslib.Group {
 	names := slices.Collect(maps.Keys(node.children))
-	slices.Sort(names)
+	slices.SortFunc(names, compareGroupNames)

 	var groups []gokeepasslib.Group
 	for _, name := range names {
@@ -422,6 +457,29 @@ func marshalGroups(db *gokeepasslib.Database, node *groupNode) []gokeepasslib.Gr
 	return groups
 }

+func compareGroupNames(a, b string) int {
+	switch {
+	case a == b:
+		return 0
+	case a == "Root":
+		return -1
+	case b == "Root":
+		return 1
+	case a == templatesRoot:
+		return -1
+	case b == templatesRoot:
+		return 1
+	case a == recycleBinRoot:
+		return 1
+	case b == recycleBinRoot:
+		return -1
+	case a < b:
+		return -1
+	default:
+		return 1
+	}
+}
+
 func marshalEntries(db *gokeepasslib.Database, entries []Entry) []gokeepasslib.Entry {
 	slices.SortFunc(entries, func(a, b Entry) int {
 		switch {
@@ -3,6 +3,7 @@ package vault
 import (
 	"bytes"
 	"errors"
+	"slices"
 	"testing"

 	"github.com/tobischo/gokeepasslib/v3"
@@ -602,6 +603,114 @@ func TestKDBXRoundTripsEntryAttachments(t *testing.T) {
 	}
 }

+func TestKDBXReopenCyclesPreserveStableIDsAndCrossFeatureState(t *testing.T) {
+	t.Parallel()
+
+	model := Model{
+		Entries: []Entry{
+			{
+				ID:       "entry-1",
+				Title:    "Vault Console",
+				Username: "dannyocean",
+				Password: "token-2",
+				URL:      "https://vault.crew.example.invalid",
+				Notes:    "Current credential",
+				Path:     []string{"Root", "Internet"},
+				Attachments: map[string][]byte{
+					"token.txt": []byte("secret attachment contents"),
+				},
+				History: []Entry{
+					{
+						ID:       "entry-1-history-1",
+						Title:    "Vault Console",
+						Username: "dannyocean",
+						Password: "token-1",
+						URL:      "https://vault.crew.example.invalid",
+						Notes:    "Original credential",
+						Path:     []string{"Root", "Internet"},
+					},
+				},
+			},
+		},
+		Templates: []Entry{
+			{
+				ID:       "tpl-1",
+				Title:    "Website Login",
+				Username: "template-user",
+				Password: "template-password",
+				Path:     []string{"Templates", "Web"},
+			},
+		},
+		RecycleBin: []Entry{
+			{
+				ID:       "deleted-1",
+				Title:    "Retired Entry",
+				Username: "archived-user",
+				Password: "retired-token",
+				Path:     []string{"Root", "Archive"},
+			},
+		},
+		Groups: [][]string{
+			{"Root", "Archive"},
+			{"Root", "Empty Group"},
+			{"Templates", "Web"},
+		},
+	}
+
+	var encoded bytes.Buffer
+	if err := SaveKDBX(&encoded, model, "correct horse battery staple"); err != nil {
+		t.Fatalf("SaveKDBX(first cycle) error = %v", err)
+	}
+
+	reopened, err := LoadKDBX(bytes.NewReader(encoded.Bytes()), "correct horse battery staple")
+	if err != nil {
+		t.Fatalf("LoadKDBX(first cycle) error = %v", err)
+	}
+
+	encoded.Reset()
+	if err := SaveKDBX(&encoded, reopened, "correct horse battery staple"); err != nil {
+		t.Fatalf("SaveKDBX(second cycle) error = %v", err)
+	}
+
+	reopened, err = LoadKDBX(bytes.NewReader(encoded.Bytes()), "correct horse battery staple")
+	if err != nil {
+		t.Fatalf("LoadKDBX(second cycle) error = %v", err)
+	}
+
+	got := reopened.EntriesInPath([]string{"Root", "Internet"})
+	if len(got) != 1 {
+		t.Fatalf("len(EntriesInPath(Root/Internet)) = %d, want 1", len(got))
+	}
+	if got[0].ID != "entry-1" {
+		t.Fatalf("entry ID after reopen cycles = %q, want %q", got[0].ID, "entry-1")
+	}
+	if len(got[0].History) != 1 {
+		t.Fatalf("len(History) after reopen cycles = %d, want 1", len(got[0].History))
+	}
+	if got[0].History[0].ID != "entry-1-history-1" {
+		t.Fatalf("history ID after reopen cycles = %q, want %q", got[0].History[0].ID, "entry-1-history-1")
+	}
+	if string(got[0].Attachments["token.txt"]) != "secret attachment contents" {
+		t.Fatalf("attachment after reopen cycles = %q, want %q", string(got[0].Attachments["token.txt"]), "secret attachment contents")
+	}
+
+	if len(reopened.Templates) != 1 || reopened.Templates[0].Path[1] != "Web" {
+		t.Fatalf("Templates after reopen cycles = %#v, want Website Login in Templates/Web", reopened.Templates)
+	}
+	if len(reopened.RecycleBin) != 1 || reopened.RecycleBin[0].Path[1] != "Archive" {
+		t.Fatalf("RecycleBin after reopen cycles = %#v, want recycled entry in Root/Archive", reopened.RecycleBin)
+	}
+
+	rootGroups := reopened.ChildGroups([]string{"Root"})
+	if !slices.Equal(rootGroups, []string{"Archive", "Empty Group", "Internet"}) {
+		t.Fatalf("ChildGroups(Root) after reopen cycles = %v, want [Archive Empty Group Internet]", rootGroups)
+	}
+	templateGroups := reopened.ChildGroups([]string{"Templates"})
+	if !slices.Equal(templateGroups, []string{"Web"}) {
+		t.Fatalf("ChildGroups(Templates) after reopen cycles = %v, want [Web]", templateGroups)
+	}
+}
+
 func mustGroup(name string, children ...any) gokeepasslib.Group {
 	group := gokeepasslib.NewGroup()
 	group.Name = name