From 675aeebdebb4d95d81a8541138e48c5d6c261933 Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Sat, 11 Apr 2026 11:03:05 -0700 Subject: [PATCH] Fix scoped gRPC persistence and autosave behavior --- TODO.md | 4 + internal/api/server.go | 168 ++++++++++++++++++++++-------- internal/api/server_test.go | 109 ++++++++++++++++++++ internal/appstate/state.go | 116 +++++++++++++-------- internal/appstate/state_test.go | 174 ++++++++++++++++++++++++++++++++ internal/appui/app.go | 15 +++ internal/appui/main_test.go | 34 +++++++ internal/appui/settings.go | 12 +++ internal/session/session.go | 4 + 9 files changed, 551 insertions(+), 85 deletions(-) diff --git a/TODO.md b/TODO.md index c64b47e..019f7a1 100644 --- a/TODO.md +++ b/TODO.md @@ -97,6 +97,10 @@ These should remain in the main user flow rather than being hidden behind a sett keep the split-button pattern, but reduce the visual weight of the sync controls and make advanced sync affordances clearer. - Synchronize: avoid layout-shifting success banners and keep noncritical notifications ephemeral. +- Synchronize: + define exact local-versus-remote merge semantics for cases where both sides changed, and make the user-facing action names describe the real behavior instead of ambiguous `push`/`pull` labels if those actions perform two-way reconciliation. +- Synchronize: + choose sync wording and defaults that maximize user comprehension and safety, especially around merge, overwrite, conflict, and retry behavior. - Phone layout: continue reducing header and control density so content appears sooner. - Mobile reliability: diff --git a/internal/api/server.go b/internal/api/server.go index ec9da39..6556c30 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -249,6 +249,7 @@ func (s *Server) FindBrowserLogins(ctx context.Context, req *keepassgov1.FindBro if locked { return nil, status.Error(codes.FailedPrecondition, "vault is locked") } + displayModel := visibleModel(model) token, err := s.authenticateRequest(ctx) if err != nil { return nil, err @@ -259,7 +260,7 @@ func (s *Server) FindBrowserLogins(ctx context.Context, req *keepassgov1.FindBro } var matches []rankedBrowserMatch - for _, entry := range visibleModel(model).Entries { + for _, entry := range displayModel.Entries { quality, score := classifyBrowserEntryMatch(pageHost, entry.URL) if score == 0 { continue @@ -425,11 +426,13 @@ func (s *Server) ListEntries(ctx context.Context, req *keepassgov1.ListEntriesRe if locked { return nil, status.Error(codes.FailedPrecondition, "vault is locked") } - if _, err := s.authorizePathRequest(ctx, apitokens.OperationListEntries, req.GetPath()); err != nil { + displayModel := visibleModel(model) + internalPath := expandClientPath(displayModel, req.GetPath()) + if _, err := s.authorizePathRequest(ctx, apitokens.OperationListEntries, internalPath); err != nil { return nil, err } - model = visibleModel(model) + model = displayModel var entries []vault.Entry if strings.TrimSpace(req.GetQuery()) != "" { results := model.Search(req.GetQuery()) @@ -438,14 +441,14 @@ func (s *Server) ListEntries(ctx context.Context, req *keepassgov1.ListEntriesRe entries = append(entries, result.Entry) } } else { - entries = model.EntriesInPath(req.GetPath()) + entries = model.EntriesInPath(internalPath) } resp := &keepassgov1.ListEntriesResponse{ Entries: make([]*keepassgov1.Entry, 0, len(entries)), } for _, entry := range entries { - resp.Entries = append(resp.Entries, entryToProto(entry)) + resp.Entries = append(resp.Entries, entryToProtoWithModel(model, entry)) } return resp, nil @@ -456,44 +459,50 @@ func (s *Server) ListGroups(ctx context.Context, req *keepassgov1.ListGroupsRequ if locked { return nil, status.Error(codes.FailedPrecondition, "vault is locked") } - if _, err := s.authorizePathRequest(ctx, apitokens.OperationListGroups, req.GetPath()); err != nil { + displayModel := visibleModel(model) + internalPath := expandClientPath(displayModel, req.GetPath()) + if _, err := s.authorizePathRequest(ctx, apitokens.OperationListGroups, internalPath); err != nil { return nil, err } return &keepassgov1.ListGroupsResponse{ - Names: visibleModel(model).ChildGroups(req.GetPath()), + Names: displayModel.ChildGroups(internalPath), }, nil } func (s *Server) CreateGroup(ctx context.Context, req *keepassgov1.CreateGroupRequest) (*keepassgov1.CreateGroupResponse, error) { - if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateGroup, req.GetParentPath()); err != nil { + model, locked := s.snapshotModel() + if locked { + return nil, status.Error(codes.FailedPrecondition, "vault is locked") + } + parentPath := expandClientPath(visibleModel(model), req.GetParentPath()) + if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateGroup, parentPath); err != nil { return nil, err } s.mu.Lock() defer s.mu.Unlock() - if s.locked { - return nil, status.Error(codes.FailedPrecondition, "vault is locked") - } - s.model.CreateGroup(req.GetParentPath(), req.GetName()) + s.model.CreateGroup(parentPath, req.GetName()) s.dirty = true s.syncMutationLocked() return &keepassgov1.CreateGroupResponse{}, nil } func (s *Server) RenameGroup(ctx context.Context, req *keepassgov1.RenameGroupRequest) (*keepassgov1.RenameGroupResponse, error) { - if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateGroup, req.GetPath()); err != nil { + model, locked := s.snapshotModel() + if locked { + return nil, status.Error(codes.FailedPrecondition, "vault is locked") + } + groupPath := expandClientPath(visibleModel(model), req.GetPath()) + if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateGroup, groupPath); err != nil { return nil, err } s.mu.Lock() defer s.mu.Unlock() - if s.locked { - return nil, status.Error(codes.FailedPrecondition, "vault is locked") - } - if err := s.model.RenameGroup(req.GetPath(), req.GetNewName()); err != nil { + if err := s.model.RenameGroup(groupPath, req.GetNewName()); err != nil { if errors.Is(err, vault.ErrEntryNotFound) { return nil, status.Error(codes.NotFound, err.Error()) } @@ -506,17 +515,19 @@ func (s *Server) RenameGroup(ctx context.Context, req *keepassgov1.RenameGroupRe } func (s *Server) DeleteGroup(ctx context.Context, req *keepassgov1.DeleteGroupRequest) (*keepassgov1.DeleteGroupResponse, error) { - if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateGroup, req.GetPath()); err != nil { + model, locked := s.snapshotModel() + if locked { + return nil, status.Error(codes.FailedPrecondition, "vault is locked") + } + groupPath := expandClientPath(visibleModel(model), req.GetPath()) + if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateGroup, groupPath); err != nil { return nil, err } s.mu.Lock() defer s.mu.Unlock() - if s.locked { - return nil, status.Error(codes.FailedPrecondition, "vault is locked") - } - if err := s.model.DeleteGroup(req.GetPath()); err != nil { + if err := s.model.DeleteGroup(groupPath); err != nil { switch { case errors.Is(err, vault.ErrEntryNotFound): return nil, status.Error(codes.NotFound, err.Error()) @@ -537,22 +548,22 @@ func (s *Server) UpsertEntry(ctx context.Context, req *keepassgov1.UpsertEntryRe return nil, status.Error(codes.InvalidArgument, "missing entry") } - entry := entryFromProto(req.GetEntry()) - if _, err := s.authorizeEntryRequest(ctx, apitokens.OperationMutateEntry, entry); err != nil { + model, locked := s.snapshotModel() + if locked { + return nil, status.Error(codes.FailedPrecondition, "vault is locked") + } + entry := entryFromProtoWithModel(visibleModel(model), req.GetEntry()) + if _, err := s.authorizeUpsertEntryRequest(ctx, entry); err != nil { return nil, err } s.mu.Lock() - if s.locked { - s.mu.Unlock() - return nil, status.Error(codes.FailedPrecondition, "vault is locked") - } s.model.UpsertEntry(entry) s.dirty = true s.syncMutationLocked() s.mu.Unlock() - return &keepassgov1.UpsertEntryResponse{Entry: entryToProto(entry)}, nil + return &keepassgov1.UpsertEntryResponse{Entry: entryToProtoWithModel(visibleModel(model), entry)}, nil } func (s *Server) DeleteEntry(ctx context.Context, req *keepassgov1.DeleteEntryRequest) (*keepassgov1.DeleteEntryResponse, error) { @@ -612,7 +623,7 @@ func (s *Server) RestoreEntry(ctx context.Context, req *keepassgov1.RestoreEntry s.dirty = true s.syncMutationLocked() - return &keepassgov1.RestoreEntryResponse{Entry: entryToProto(restored)}, nil + return &keepassgov1.RestoreEntryResponse{Entry: entryToProtoWithModel(visibleModel(model), restored)}, nil } func (s *Server) ListEntryHistory(ctx context.Context, req *keepassgov1.ListEntryHistoryRequest) (*keepassgov1.ListEntryHistoryResponse, error) { @@ -633,7 +644,7 @@ func (s *Server) ListEntryHistory(ctx context.Context, req *keepassgov1.ListEntr Entries: make([]*keepassgov1.Entry, 0, len(entry.History)), } for _, historical := range entry.History { - resp.Entries = append(resp.Entries, entryToProto(historical)) + resp.Entries = append(resp.Entries, entryToProtoWithModel(visibleModel(model), historical)) } return resp, nil } @@ -666,7 +677,7 @@ func (s *Server) RestoreEntryHistory(ctx context.Context, req *keepassgov1.Resto } s.dirty = true s.syncMutationLocked() - return &keepassgov1.RestoreEntryHistoryResponse{Entry: entryToProto(entry)}, nil + return &keepassgov1.RestoreEntryHistoryResponse{Entry: entryToProtoWithModel(visibleModel(s.model), entry)}, nil } func (s *Server) ListTemplates(ctx context.Context, _ *keepassgov1.ListTemplatesRequest) (*keepassgov1.ListTemplatesResponse, error) { @@ -684,7 +695,7 @@ func (s *Server) ListTemplates(ctx context.Context, _ *keepassgov1.ListTemplates Templates: make([]*keepassgov1.Entry, 0, len(s.model.Templates)), } for _, template := range s.model.Templates { - resp.Templates = append(resp.Templates, entryToProto(template)) + resp.Templates = append(resp.Templates, entryToProtoWithModel(visibleModel(s.model), template)) } return resp, nil @@ -705,12 +716,12 @@ func (s *Server) UpsertTemplate(ctx context.Context, req *keepassgov1.UpsertTemp return nil, status.Error(codes.FailedPrecondition, "vault is locked") } - entry := entryFromProto(req.GetTemplate()) + entry := entryFromProtoWithModel(visibleModel(s.model), req.GetTemplate()) s.model.UpsertTemplate(entry) s.dirty = true s.syncMutationLocked() - return &keepassgov1.UpsertTemplateResponse{Template: entryToProto(entry)}, nil + return &keepassgov1.UpsertTemplateResponse{Template: entryToProtoWithModel(visibleModel(s.model), entry)}, nil } func (s *Server) DeleteTemplate(ctx context.Context, req *keepassgov1.DeleteTemplateRequest) (*keepassgov1.DeleteTemplateResponse, error) { @@ -743,7 +754,8 @@ func (s *Server) InstantiateTemplate(ctx context.Context, req *keepassgov1.Insta if _, err := s.authorizeTemplateRequest(ctx, apitokens.OperationListTemplates, req.GetTemplateId()); err != nil { return nil, err } - if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateEntry, req.GetOverrides().GetPath()); err != nil { + overridePath := expandClientPath(visibleModel(s.model), req.GetOverrides().GetPath()) + if _, err := s.authorizePathRequest(ctx, apitokens.OperationMutateEntry, overridePath); err != nil { return nil, err } @@ -754,7 +766,8 @@ func (s *Server) InstantiateTemplate(ctx context.Context, req *keepassgov1.Insta return nil, status.Error(codes.FailedPrecondition, "vault is locked") } - entry, err := s.model.InstantiateTemplate(req.GetTemplateId(), entryFromProto(req.GetOverrides())) + overrides := entryFromProtoWithModel(visibleModel(s.model), req.GetOverrides()) + entry, err := s.model.InstantiateTemplate(req.GetTemplateId(), overrides) if err != nil { if errors.Is(err, vault.ErrEntryNotFound) { return nil, status.Error(codes.NotFound, err.Error()) @@ -764,7 +777,7 @@ func (s *Server) InstantiateTemplate(ctx context.Context, req *keepassgov1.Insta s.dirty = true s.syncMutationLocked() - return &keepassgov1.InstantiateTemplateResponse{Entry: entryToProto(entry)}, nil + return &keepassgov1.InstantiateTemplateResponse{Entry: entryToProtoWithModel(visibleModel(s.model), entry)}, nil } func (s *Server) ListAttachments(ctx context.Context, req *keepassgov1.ListAttachmentsRequest) (*keepassgov1.ListAttachmentsResponse, error) { @@ -932,7 +945,7 @@ func (s *Server) GeneratePassword(ctx context.Context, req *keepassgov1.Generate return &keepassgov1.GeneratePasswordResponse{Password: password}, nil } -func entryToProto(entry vault.Entry) *keepassgov1.Entry { +func entryToProtoWithModel(model vault.Model, entry vault.Entry) *keepassgov1.Entry { return &keepassgov1.Entry{ Id: entry.ID, Title: entry.Title, @@ -941,12 +954,12 @@ func entryToProto(entry vault.Entry) *keepassgov1.Entry { Url: entry.URL, Notes: entry.Notes, Tags: append([]string(nil), entry.Tags...), - Path: append([]string(nil), entry.Path...), + Path: collapseInternalPath(model, entry.Path), Fields: maps.Clone(entry.Fields), } } -func entryFromProto(entry *keepassgov1.Entry) vault.Entry { +func entryFromProtoWithModel(model vault.Model, entry *keepassgov1.Entry) vault.Entry { return vault.Entry{ ID: entry.GetId(), Title: entry.GetTitle(), @@ -955,11 +968,44 @@ func entryFromProto(entry *keepassgov1.Entry) vault.Entry { URL: entry.GetUrl(), Notes: entry.GetNotes(), Tags: append([]string(nil), entry.GetTags()...), - Path: append([]string(nil), entry.GetPath()...), + Path: expandClientPath(model, entry.GetPath()), Fields: maps.Clone(entry.GetFields()), } } +func hiddenVaultRoot(model vault.Model) string { + if len(model.EntriesInPath(nil)) != 0 { + return "" + } + groups := model.ChildGroups(nil) + if len(groups) != 1 { + return "" + } + return groups[0] +} + +func expandClientPath(model vault.Model, path []string) []string { + root := hiddenVaultRoot(model) + if root == "" { + return append([]string(nil), path...) + } + if len(path) == 0 { + return []string{root} + } + if path[0] == root { + return append([]string(nil), path...) + } + return append([]string{root}, path...) +} + +func collapseInternalPath(model vault.Model, path []string) []string { + root := hiddenVaultRoot(model) + if root == "" || len(path) == 0 || path[0] != root { + return append([]string(nil), path...) + } + return append([]string(nil), path[1:]...) +} + func findEntryByID(model vault.Model, id string) (vault.Entry, error) { for _, entry := range model.Entries { if entry.ID == id { @@ -1103,6 +1149,44 @@ func (s *Server) authorizeEntryRequest(ctx context.Context, op apitokens.Operati return s.authorizeResourceRequest(ctx, token, op, apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: entry.ID, Path: entry.Path}) } +func (s *Server) authorizeUpsertEntryRequest(ctx context.Context, entry vault.Entry) (apitokens.Token, error) { + token, err := s.authenticateRequest(ctx) + if err != nil { + return apitokens.Token{}, err + } + model, locked := s.snapshotModel() + if locked { + return apitokens.Token{}, status.Error(codes.FailedPrecondition, "vault is locked") + } + existing, err := findEntryByID(model, entry.ID) + switch { + case err == nil: + if _, err := s.authorizeResourceRequest(ctx, token, apitokens.OperationMutateEntry, apitokens.Resource{ + Kind: apitokens.ResourceEntry, + EntryID: existing.ID, + Path: existing.Path, + }); err != nil { + return apitokens.Token{}, err + } + if !slices.Equal(existing.Path, entry.Path) { + if _, err := s.authorizeResourceRequest(ctx, token, apitokens.OperationMutateEntry, apitokens.Resource{ + Kind: apitokens.ResourceGroup, + Path: entry.Path, + }); err != nil { + return apitokens.Token{}, err + } + } + return token, nil + case errors.Is(err, vault.ErrEntryNotFound): + return s.authorizeResourceRequest(ctx, token, apitokens.OperationMutateEntry, apitokens.Resource{ + Kind: apitokens.ResourceGroup, + Path: entry.Path, + }) + default: + return apitokens.Token{}, status.Errorf(codes.Internal, "lookup existing entry: %v", err) + } +} + func (s *Server) authorizeTemplateRequest(ctx context.Context, op apitokens.Operation, templateID string) (apitokens.Token, error) { token, err := s.authenticateRequest(ctx) if err != nil { diff --git a/internal/api/server_test.go b/internal/api/server_test.go index ea17623..1b86434 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "net" "os" + "slices" "testing" "time" @@ -225,6 +226,71 @@ func TestVaultServiceFindsBrowserLoginsWithinAuthorizedGroupScope(t *testing.T) } } +func TestVaultServiceListEntriesHidesSingleInternalVaultRoot(t *testing.T) { + t.Parallel() + + client, _, cleanup := newTestClientForModel(t, vault.Model{ + Entries: []vault.Entry{ + { + ID: "codex-nextcloud", + Title: "Nextcloud (codex)", + Username: "jjulian", + Password: "secret-1", + URL: "https://nextcloud.example.invalid", + Path: []string{"keepass", "Joe", "codex"}, + }, + testAPITokenEntry(t, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + ), + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Joe"}, + {"keepass", "Joe", "codex"}, + }, + }) + defer cleanup() + + resp, err := client.ListEntries(tokenContext(defaultTestTokenSecret), &keepassgov1.ListEntriesRequest{ + Path: []string{"Joe", "codex"}, + }) + if err != nil { + t.Fatalf("ListEntries() error = %v", err) + } + if len(resp.Entries) != 1 { + t.Fatalf("len(ListEntries().Entries) = %d, want 1", len(resp.Entries)) + } + if got := resp.Entries[0].Path; !slices.Equal(got, []string{"Joe", "codex"}) { + t.Fatalf("ListEntries().Entries[0].Path = %v, want [Joe codex]", got) + } +} + +func TestVaultServiceListGroupsHidesSingleInternalVaultRoot(t *testing.T) { + t.Parallel() + + client, _, cleanup := newTestClientForModel(t, vault.Model{ + Entries: []vault.Entry{ + testAPITokenEntry(t, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListGroups, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass"}}}, + ), + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Joe"}, + {"keepass", "Shared"}, + }, + }) + defer cleanup() + + resp, err := client.ListGroups(tokenContext(defaultTestTokenSecret), &keepassgov1.ListGroupsRequest{}) + if err != nil { + t.Fatalf("ListGroups() error = %v", err) + } + if !slices.Equal(resp.Names, []string{"Joe", "Shared"}) { + t.Fatalf("ListGroups().Names = %v, want [Joe Shared]", resp.Names) + } +} + func TestVaultServiceGetsBrowserCredentialForAuthorizedClients(t *testing.T) { t.Parallel() @@ -1036,6 +1102,49 @@ func TestVaultServiceUpsertEntryUpdatesLifecycleModel(t *testing.T) { } } +func TestVaultServiceUpsertsNewEntryWithinAuthorizedGroupScope(t *testing.T) { + t.Parallel() + + client, _, cleanup := newTestClientForModel(t, vault.Model{ + Entries: []vault.Entry{ + testAPITokenEntry(t, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationMutateEntry, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + apitokens.PolicyRule{Effect: apitokens.EffectAllow, Operation: apitokens.OperationListEntries, Resource: apitokens.Resource{Kind: apitokens.ResourceGroup, Path: []string{"keepass", "Joe", "codex"}}}, + ), + }, + Groups: [][]string{ + {"keepass"}, + {"keepass", "Joe"}, + {"keepass", "Joe", "codex"}, + }, + }) + defer cleanup() + + upserted, err := client.UpsertEntry(tokenContext(defaultTestTokenSecret), &keepassgov1.UpsertEntryRequest{ + Entry: &keepassgov1.Entry{ + Id: "codex-created", + Title: "Codex Created", + Path: []string{"Joe", "codex"}, + }, + }) + if err != nil { + t.Fatalf("UpsertEntry() error = %v", err) + } + if got := upserted.Entry.Path; !slices.Equal(got, []string{"Joe", "codex"}) { + t.Fatalf("UpsertEntry().Entry.Path = %v, want [Joe codex]", got) + } + + listed, err := client.ListEntries(tokenContext(defaultTestTokenSecret), &keepassgov1.ListEntriesRequest{ + Path: []string{"Joe", "codex"}, + }) + if err != nil { + t.Fatalf("ListEntries() error = %v", err) + } + if len(listed.Entries) != 1 || listed.Entries[0].Id != "codex-created" { + t.Fatalf("ListEntries().Entries = %#v, want created codex entry", listed.Entries) + } +} + func TestVaultServiceDeletesAndRestoresEntriesForAuthorizedClients(t *testing.T) { t.Parallel() diff --git a/internal/appstate/state.go b/internal/appstate/state.go index dec10aa..cf982e3 100644 --- a/internal/appstate/state.go +++ b/internal/appstate/state.go @@ -55,6 +55,15 @@ type SaveableSession interface { Save() error } +type AutoSaveableSession interface { + SaveableSession + HasSaveTarget() bool +} + +type RemoteAwareSession interface { + IsRemote() bool +} + type SynchronizableSession interface { CurrentSession Synchronize() error @@ -103,6 +112,7 @@ type State struct { Session CurrentSession Approvals ApprovalManager AuditLog *apiaudit.Log + AutoSaveRemote bool Section Section CurrentPath []string SearchQuery string @@ -196,7 +206,9 @@ func (s *State) IssueAPIToken(name, clientName string, expiresAt *time.Time, now } apitokens.Upsert(&model, token) session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return apitokens.Token{}, "", err + } s.recordTokenAudit(apiaudit.EventTokenIssued, token, "issued API token") return token, secret, nil } @@ -220,7 +232,9 @@ func (s *State) RotateAPIToken(id string, now time.Time) (apitokens.Token, strin } apitokens.Upsert(&model, token) session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return apitokens.Token{}, "", err + } s.recordTokenAudit(apiaudit.EventTokenRotated, token, "rotated API token") return token, secret, nil } @@ -236,7 +250,9 @@ func (s *State) UpsertAPIToken(token apitokens.Token) error { } apitokens.Upsert(&model, token) session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return err + } s.recordTokenAudit(apiaudit.EventTokenUpdated, token, "updated API token") return nil } @@ -257,7 +273,9 @@ func (s *State) DisableAPIToken(id string) error { token = apitokens.Disable(token) apitokens.Upsert(&model, token) session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return err + } s.recordTokenAudit(apiaudit.EventTokenDisabled, token, "disabled API token") return nil } @@ -278,7 +296,9 @@ func (s *State) RevokeAPIToken(id string, when time.Time) error { token = apitokens.Revoke(token, when) apitokens.Upsert(&model, token) session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return err + } s.recordTokenAudit(apiaudit.EventTokenRevoked, token, "revoked API token") return nil } @@ -300,7 +320,9 @@ func (s *State) DeleteAPIToken(id string) error { return err } session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return err + } s.recordTokenAudit(apiaudit.EventTokenDeleted, token, "deleted API token") return nil } @@ -339,8 +361,7 @@ func (s *State) ConfigureSecurity(settings vault.SecuritySettings) error { if err := security.ConfigureSecurity(settings); err != nil { return err } - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) ShowSection(section Section) { @@ -568,8 +589,7 @@ func (s *State) DeleteSelectedEntry() error { session.Replace(model) s.SelectedEntryID = "" - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) RestoreEntry(id string) error { @@ -588,8 +608,7 @@ func (s *State) RestoreEntry(id string) error { } session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) UpsertEntry(entry vault.Entry) error { @@ -606,8 +625,7 @@ func (s *State) UpsertEntry(entry vault.Entry) error { model.UpsertEntry(entry) session.Replace(model) s.SelectedEntryID = entry.ID - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) UpsertTemplate(entry vault.Entry) error { @@ -624,8 +642,7 @@ func (s *State) UpsertTemplate(entry vault.Entry) error { model.UpsertTemplate(entry) session.Replace(model) s.SelectedEntryID = entry.ID - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) InstantiateTemplate(templateID string, overrides vault.Entry) (vault.Entry, error) { @@ -646,7 +663,9 @@ func (s *State) InstantiateTemplate(templateID string, overrides vault.Entry) (v session.Replace(model) s.SelectedEntryID = entry.ID - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return vault.Entry{}, err + } return entry, nil } @@ -669,8 +688,7 @@ func (s *State) DeleteTemplate(id string) error { if s.SelectedEntryID == id { s.SelectedEntryID = "" } - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) DuplicateSelectedEntry(duplicateID string) (vault.Entry, error) { @@ -691,7 +709,9 @@ func (s *State) DuplicateSelectedEntry(duplicateID string) (vault.Entry, error) session.Replace(model) s.SelectedEntryID = duplicate.ID - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return vault.Entry{}, err + } return duplicate, nil } @@ -711,8 +731,7 @@ func (s *State) RestoreSelectedEntryVersion(historyIndex int) error { } session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) Lock() error { @@ -748,8 +767,7 @@ func (s *State) ChangeMasterKey(key vault.MasterKey) error { return err } - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) EnterGroup(name string) { @@ -776,6 +794,25 @@ func (s *State) Save() error { return nil } +func (s *State) markDirtyAndAutoSave() error { + s.Dirty = true + session, ok := s.Session.(SaveableSession) + if !ok { + return nil + } + if autosave, ok := s.Session.(AutoSaveableSession); ok && !autosave.HasSaveTarget() { + return nil + } + if remote, ok := s.Session.(RemoteAwareSession); ok && remote.IsRemote() && !s.AutoSaveRemote { + return nil + } + if err := session.Save(); err != nil { + return err + } + s.Dirty = false + return nil +} + func (s *State) Synchronize() error { session, ok := s.Session.(SynchronizableSession) if !ok { @@ -948,7 +985,9 @@ func (s *State) ConfigureRemoteBinding(input RemoteBindingInput) (RemoteBinding, } session.Replace(model) - s.Dirty = true + if err := s.markDirtyAndAutoSave(); err != nil { + return RemoteBinding{}, err + } return binding, nil } @@ -968,8 +1007,7 @@ func (s *State) RemoveRemoteBinding(binding RemoteBinding) error { } session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) CreateGroup(name string) error { @@ -985,8 +1023,7 @@ func (s *State) CreateGroup(name string) error { model.CreateGroup(s.CurrentPath, name) session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) MoveCurrentGroup(parent []string) error { @@ -1006,8 +1043,7 @@ func (s *State) MoveCurrentGroup(parent []string) error { if len(current) > 0 { s.CurrentPath = append(append([]string(nil), parent...), current[len(current)-1]) } - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) RenameCurrentGroup(newName string) error { @@ -1029,8 +1065,7 @@ func (s *State) RenameCurrentGroup(newName string) error { if len(s.CurrentPath) > 0 { s.CurrentPath = append(append([]string(nil), s.CurrentPath[:len(s.CurrentPath)-1]...), newName) } - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) MoveSelectedEntry(path []string) error { @@ -1049,8 +1084,7 @@ func (s *State) MoveSelectedEntry(path []string) error { } session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) DeleteCurrentGroup() error { @@ -1073,8 +1107,7 @@ func (s *State) DeleteCurrentGroup() error { s.CurrentPath = append([]string(nil), s.CurrentPath[:len(s.CurrentPath)-1]...) } s.SelectedEntryID = "" - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } func (s *State) AddAttachmentToSelectedEntry(name string, content []byte) error { @@ -1100,8 +1133,7 @@ func (s *State) AddAttachmentToSelectedEntry(name string, content []byte) error } model.Entries[i].Attachments[name] = append([]byte(nil), content...) session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } return vault.ErrEntryNotFound @@ -1127,8 +1159,7 @@ func (s *State) ReplaceAttachmentOnSelectedEntry(name string, content []byte) er } model.Entries[i].Attachments[name] = append([]byte(nil), content...) session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } return vault.ErrEntryNotFound @@ -1157,8 +1188,7 @@ func (s *State) DeleteAttachmentFromSelectedEntry(name string) error { model.Entries[i].Attachments = nil } session.Replace(model) - s.Dirty = true - return nil + return s.markDirtyAndAutoSave() } return vault.ErrEntryNotFound diff --git a/internal/appstate/state_test.go b/internal/appstate/state_test.go index b124ea9..5888209 100644 --- a/internal/appstate/state_test.go +++ b/internal/appstate/state_test.go @@ -184,6 +184,93 @@ func TestIssueRotateDisableRevokeAndDeleteAPIToken(t *testing.T) { } } +func TestIssueAPITokenAutoSavesWhenSessionSupportsSaving(t *testing.T) { + t.Parallel() + + session := &mutableSaveableStubSession{model: vault.Model{}, hasSaveTarget: true} + state := State{Session: session} + now := time.Date(2026, 3, 29, 12, 0, 0, 0, time.UTC) + + if _, _, err := state.IssueAPIToken("CLI", "grpc-cli", nil, now); err != nil { + t.Fatalf("IssueAPIToken() error = %v", err) + } + + if session.saveCalls != 1 { + t.Fatalf("saveCalls = %d, want 1", session.saveCalls) + } + if state.Dirty { + t.Fatal("Dirty = true, want false after autosave") + } +} + +func TestIssueAPITokenDoesNotAutoSaveWithoutSaveTarget(t *testing.T) { + t.Parallel() + + session := &mutableSaveableStubSession{model: vault.Model{}} + state := State{Session: session} + now := time.Date(2026, 3, 29, 12, 0, 0, 0, time.UTC) + + if _, _, err := state.IssueAPIToken("CLI", "grpc-cli", nil, now); err != nil { + t.Fatalf("IssueAPIToken() error = %v", err) + } + + if session.saveCalls != 0 { + t.Fatalf("saveCalls = %d, want 0", session.saveCalls) + } + if !state.Dirty { + t.Fatal("Dirty = false, want true when no save target exists") + } +} + +func TestIssueAPITokenDoesNotAutoSaveForRemoteSession(t *testing.T) { + t.Parallel() + + session := &mutableSaveableStubSession{ + model: vault.Model{}, + hasSaveTarget: true, + remote: true, + } + state := State{Session: session} + now := time.Date(2026, 3, 29, 12, 0, 0, 0, time.UTC) + + if _, _, err := state.IssueAPIToken("CLI", "grpc-cli", nil, now); err != nil { + t.Fatalf("IssueAPIToken() error = %v", err) + } + + if session.saveCalls != 0 { + t.Fatalf("saveCalls = %d, want 0", session.saveCalls) + } + if !state.Dirty { + t.Fatal("Dirty = false, want true for remote session") + } +} + +func TestIssueAPITokenAutoSavesForRemoteSessionWhenEnabled(t *testing.T) { + t.Parallel() + + session := &mutableSaveableStubSession{ + model: vault.Model{}, + hasSaveTarget: true, + remote: true, + } + state := State{ + Session: session, + AutoSaveRemote: true, + } + now := time.Date(2026, 3, 29, 12, 0, 0, 0, time.UTC) + + if _, _, err := state.IssueAPIToken("CLI", "grpc-cli", nil, now); err != nil { + t.Fatalf("IssueAPIToken() error = %v", err) + } + + if session.saveCalls != 1 { + t.Fatalf("saveCalls = %d, want 1", session.saveCalls) + } + if state.Dirty { + t.Fatal("Dirty = true, want false after remote autosave") + } +} + func TestRemoteProfilesReturnsVaultProfiles(t *testing.T) { t.Parallel() @@ -1480,6 +1567,60 @@ func TestCreateGroupPersistsGroupAndMarksDirty(t *testing.T) { } } +func TestCreateGroupAutoSavesWhenSessionSupportsSaving(t *testing.T) { + t.Parallel() + + sess := &mutableSaveableStubSession{model: testVaultModel(), hasSaveTarget: true} + state := State{ + Session: sess, + CurrentPath: []string{"Root"}, + } + + if err := state.CreateGroup("Finance"); err != nil { + t.Fatalf("CreateGroup() error = %v", err) + } + + if sess.saveCalls != 1 { + t.Fatalf("saveCalls = %d, want 1", sess.saveCalls) + } + if state.Dirty { + t.Fatal("Dirty = true, want false after autosave") + } +} + +func TestCreateGroupSaveFailureLeavesStateDirty(t *testing.T) { + t.Parallel() + + sess := &mutableSaveableStubSession{ + model: testVaultModel(), + hasSaveTarget: true, + saveErr: errors.New("save failed"), + } + state := State{ + Session: sess, + CurrentPath: []string{"Root"}, + } + + err := state.CreateGroup("Finance") + if err == nil || err.Error() != "save failed" { + t.Fatalf("CreateGroup() error = %v, want save failed", err) + } + if sess.saveCalls != 1 { + t.Fatalf("saveCalls = %d, want 1", sess.saveCalls) + } + if !state.Dirty { + t.Fatal("Dirty = false, want true after failed autosave") + } + + got, childErr := state.ChildGroups() + if childErr != nil { + t.Fatalf("ChildGroups() error = %v", childErr) + } + if !slices.Equal(got, []string{"Finance", "Internet", "Security Office"}) { + t.Fatalf("ChildGroups() = %v, want Finance, Internet, Security Office", got) + } +} + func TestCreateGroupSupportsNestedGroupPath(t *testing.T) { t.Parallel() @@ -1816,6 +1957,39 @@ func (s *saveableStubSession) Save() error { return nil } +type mutableSaveableStubSession struct { + model vault.Model + err error + saveCalls int + saveErr error + hasSaveTarget bool + remote bool +} + +func (s *mutableSaveableStubSession) Current() (vault.Model, error) { + if s.err != nil { + return vault.Model{}, s.err + } + return s.model, nil +} + +func (s *mutableSaveableStubSession) Replace(model vault.Model) { + s.model = model +} + +func (s *mutableSaveableStubSession) Save() error { + s.saveCalls++ + return s.saveErr +} + +func (s *mutableSaveableStubSession) HasSaveTarget() bool { + return s.hasSaveTarget +} + +func (s *mutableSaveableStubSession) IsRemote() bool { + return s.remote +} + type lifecycleStubSession struct { createCalls int model vault.Model diff --git a/internal/appui/app.go b/internal/appui/app.go index 9ed6b6d..1b5bbb6 100644 --- a/internal/appui/app.go +++ b/internal/appui/app.go @@ -380,6 +380,7 @@ type ui struct { settingsHistory widget.Bool settingsDenseLayout widget.Bool settingsDebugHeaderBounds widget.Bool + settingsAutoSaveRemote widget.Bool entryClicks []widget.Clickable apiTokenClicks []widget.Clickable apiPolicyEdits []widget.Clickable @@ -475,6 +476,7 @@ type ui struct { editingEntry bool syncDefaultSourceMode syncSourceMode syncDefaultDirection syncDirection + autoSaveRemote bool groupControlsHidden bool lifecycleAdvancedHidden bool historyHidden bool @@ -665,6 +667,7 @@ func newUIWithState(mode string, sess appstate.CurrentSession, paths statePaths) syncDirection: syncDirectionPull, syncDefaultSourceMode: syncSourceLocal, syncDefaultDirection: syncDirectionPull, + autoSaveRemote: false, apiPolicyGroupScope: true, autofillNoticePreference: autofillNoticeAll, vaultSharer: platform.NewVaultSharer(runtime.GOOS), @@ -678,6 +681,7 @@ func newUIWithState(mode string, sess appstate.CurrentSession, paths statePaths) u.apiPolicyAllow.Value = true u.apiPolicyGroupScopeW.Value = true u.state.Session = sess + u.state.AutoSaveRemote = u.autoSaveRemote u.phoneSplit.Value = 0.46 u.eyeIcon, _ = widget.NewIcon(icons.ActionVisibility) u.eyeOffIcon, _ = widget.NewIcon(icons.ActionVisibilityOff) @@ -1211,6 +1215,17 @@ func (u *ui) securityDialogContent(gtx layout.Context) layout.Dimensions { return syncDialogSummaryCard(gtx, u.theme, syncDialogPurposeAdvanced, u.settingsDraft.Sync.SourceDefault, u.settingsDraft.Sync.DirectionDefault) }, layout.Spacer{Height: unit.Dp(8)}.Layout, + func(gtx layout.Context) layout.Dimensions { + check := material.CheckBox(u.theme, &u.settingsAutoSaveRemote, "Auto-save remote vault edits") + return check.Layout(gtx) + }, + layout.Spacer{Height: unit.Dp(4)}.Layout, + func(gtx layout.Context) layout.Dimensions { + lbl := material.Label(u.theme, unit.Sp(12), "When enabled, edits to an already-open remote vault save immediately instead of waiting for an explicit remote save.") + lbl.Color = mutedColor + return lbl.Layout(gtx) + }, + layout.Spacer{Height: unit.Dp(8)}.Layout, func(gtx layout.Context) layout.Dimensions { lbl := material.Label(u.theme, unit.Sp(12), "Conflict handling stays retry-safe: merged entry changes keep history, while remote save conflicts still require reopening the vault and retrying the save.") lbl.Color = mutedColor diff --git a/internal/appui/main_test.go b/internal/appui/main_test.go index 54684dc..75c0c42 100644 --- a/internal/appui/main_test.go +++ b/internal/appui/main_test.go @@ -5618,6 +5618,33 @@ func TestUISyncDefaultsPersistInSettings(t *testing.T) { } } +func TestUIRemoteAutosavePersistsInSettings(t *testing.T) { + t.Parallel() + + configPath := filepath.Join(t.TempDir(), "settings.json") + + first := newUIWithSession("desktop", &session.Manager{}, statePaths{ + SettingsPath: configPath, + }) + first.autoSaveRemote = true + first.state.AutoSaveRemote = true + first.saveSettings() + + second := newUIWithSession("desktop", &session.Manager{}, statePaths{ + SettingsPath: configPath, + }) + second.autoSaveRemote = false + second.state.AutoSaveRemote = false + second.loadSettings() + + if !second.autoSaveRemote { + t.Fatal("autoSaveRemote = false, want true after reload") + } + if !second.state.AutoSaveRemote { + t.Fatal("state.AutoSaveRemote = false, want true after reload") + } +} + func TestUIDebugHeaderBoundsPersistInSettings(t *testing.T) { t.Parallel() @@ -5714,6 +5741,7 @@ func TestUISaveSecuritySettingsPersistsSyncDefaults(t *testing.T) { u.loadSettingsDraft() u.settingsDraft.Sync.SourceDefault = syncSourceRemote u.settingsDraft.Sync.DirectionDefault = syncDirectionPush + u.settingsAutoSaveRemote.Value = true if err := u.saveSecuritySettingsAction(); err != nil { t.Fatalf("saveSecuritySettingsAction() error = %v", err) @@ -5730,6 +5758,12 @@ func TestUISaveSecuritySettingsPersistsSyncDefaults(t *testing.T) { if got := reloaded.syncDefaultDirection; got != syncDirectionPush { t.Fatalf("reloaded syncDefaultDirection = %q, want push", got) } + if !reloaded.autoSaveRemote { + t.Fatal("reloaded autoSaveRemote = false, want true") + } + if !reloaded.state.AutoSaveRemote { + t.Fatal("reloaded state.AutoSaveRemote = false, want true") + } } func TestUISaveSecuritySettingsPersistsDebugHeaderBounds(t *testing.T) { diff --git a/internal/appui/settings.go b/internal/appui/settings.go index ae531ab..4c61389 100644 --- a/internal/appui/settings.go +++ b/internal/appui/settings.go @@ -44,6 +44,7 @@ type settingsFile struct { type syncSettings struct { SourceDefault string `json:"sourceDefault,omitempty"` DirectionDefault string `json:"directionDefault,omitempty"` + AutoSaveRemote bool `json:"autoSaveRemote,omitempty"` } type debugSettings struct { @@ -53,6 +54,7 @@ type debugSettings struct { type syncSettingsDraft struct { SourceDefault syncSourceMode DirectionDefault syncDirection + AutoSaveRemote bool } type settingsDraft struct { @@ -198,12 +200,14 @@ func (u *ui) loadSettingsDraft() { Sync: syncSettingsDraft{ SourceDefault: u.syncDefaultSourceMode, DirectionDefault: u.syncDefaultDirection, + AutoSaveRemote: u.autoSaveRemote, }, Debug: debugSettings{ LogHeaderBounds: u.debugLogHeaderBounds, }, } u.settingsDebugHeaderBounds.Value = u.settingsDraft.Debug.LogHeaderBounds + u.settingsAutoSaveRemote.Value = u.settingsDraft.Sync.AutoSaveRemote } func (u *ui) saveSecuritySettingsAction() error { @@ -226,9 +230,12 @@ func (u *ui) applySecuritySettingsLive() error { u.settingsDraft.Accessibility.DisplayDensity = displayDensityForDenseLayout(u.settingsDenseLayout.Value) } u.settingsDraft.Debug.LogHeaderBounds = u.settingsDebugHeaderBounds.Value + u.settingsDraft.Sync.AutoSaveRemote = u.settingsAutoSaveRemote.Value u.settingsDenseLayout.Value = u.settingsDraft.Accessibility.DisplayDensity == displayDensityDense u.syncDefaultSourceMode = sanitizeSyncSourceMode(u.settingsDraft.Sync.SourceDefault) u.syncDefaultDirection = sanitizeSyncDirection(u.settingsDraft.Sync.DirectionDefault) + u.autoSaveRemote = u.settingsDraft.Sync.AutoSaveRemote + u.state.AutoSaveRemote = u.autoSaveRemote u.debugLogHeaderBounds = u.settingsDraft.Debug.LogHeaderBounds if !u.debugLogHeaderBounds { u.lastHeaderBoundsLog = "" @@ -243,6 +250,7 @@ func (u *ui) applySecuritySettingsLive() error { func (u *ui) loadSettings() { u.syncDefaultSourceMode = syncSourceLocal u.syncDefaultDirection = syncDirectionPull + u.autoSaveRemote = false if strings.TrimSpace(u.settingsPath) != "" { content, err := os.ReadFile(u.settingsPath) @@ -251,6 +259,8 @@ func (u *ui) loadSettings() { if json.Unmarshal(content, &settings) == nil { u.syncDefaultSourceMode = sanitizeSyncSourceMode(syncSourceMode(settings.Sync.SourceDefault)) u.syncDefaultDirection = sanitizeSyncDirection(syncDirection(settings.Sync.DirectionDefault)) + u.autoSaveRemote = settings.Sync.AutoSaveRemote + u.state.AutoSaveRemote = u.autoSaveRemote u.debugLogHeaderBounds = settings.Debug.LogHeaderBounds return } @@ -258,6 +268,7 @@ func (u *ui) loadSettings() { } u.loadLegacySyncDefaultsFromUIPreferences() + u.state.AutoSaveRemote = u.autoSaveRemote } func (u *ui) loadLegacySyncDefaultsFromUIPreferences() { @@ -287,6 +298,7 @@ func (u *ui) saveSettings() { Sync: syncSettings{ SourceDefault: string(u.syncDefaultSourceMode), DirectionDefault: string(u.syncDefaultDirection), + AutoSaveRemote: u.autoSaveRemote, }, Debug: debugSettings{ LogHeaderBounds: u.debugLogHeaderBounds, diff --git a/internal/session/session.go b/internal/session/session.go index b59ebbd..b443cc3 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -93,6 +93,10 @@ func (m *Manager) HasVault() bool { return len(m.encoded) > 0 || m.path != "" || m.remotePath != "" } +func (m *Manager) HasSaveTarget() bool { + return m.path != "" || (m.remoteClient != nil && m.remotePath != "") +} + func (m *Manager) EncodedBytes() []byte { return append([]byte(nil), m.encoded...) }