From 57f6ae658d9d61662f05323fb249c025eda30ec7 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Sun, 29 Mar 2026 11:26:14 -0700 Subject: [PATCH] Add regression coverage for KDBX reopen cycles --- session/session_test.go | 129 ++++++++++++++++++++++++++++++++++++++++ vault/kdbx.go | 126 ++++++++++++++++++++++++++++----------- vault/kdbx_test.go | 109 +++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+), 34 deletions(-) diff --git a/session/session_test.go b/session/session_test.go index 7324da5..748c009 100644 --- a/session/session_test.go +++ b/session/session_test.go @@ -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) + } +} diff --git a/vault/kdbx.go b/vault/kdbx.go index 93f4ab9..b8ffd14 100644 --- a/vault/kdbx.go +++ b/vault/kdbx.go @@ -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 { diff --git a/vault/kdbx_test.go b/vault/kdbx_test.go index a147b64..f856c72 100644 --- a/vault/kdbx_test.go +++ b/vault/kdbx_test.go @@ -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