Add secret-safe copy and reveal UX coverage

This commit is contained in:
Joe Julian
2026-03-29 13:32:21 -07:00
parent 422de535af
commit c5670a9a36
5 changed files with 204 additions and 10 deletions
+18 -2
View File
@@ -2,7 +2,6 @@ package clipboard
import (
"errors"
"fmt"
systemclipboard "github.com/atotto/clipboard"
@@ -10,6 +9,7 @@ import (
)
var ErrUnsupportedTarget = errors.New("unsupported clipboard target")
var ErrWriteFailed = errors.New("clipboard write failed")
type Target string
@@ -39,7 +39,7 @@ func (s Service) Copy(model vault.Model, entryID string, target Target) error {
}
if err := s.writer().WriteText(content); err != nil {
return fmt.Errorf("write clipboard text: %w", err)
return writeError{err: err}
}
return nil
@@ -79,3 +79,19 @@ type systemWriter struct{}
func (systemWriter) WriteText(text string) error {
return systemclipboard.WriteAll(text)
}
type writeError struct {
err error
}
func (e writeError) Error() string {
return ErrWriteFailed.Error()
}
func (e writeError) Unwrap() error {
return e.err
}
func (e writeError) Is(target error) bool {
return target == ErrWriteFailed
}
+27
View File
@@ -68,6 +68,25 @@ func TestServiceRejectsUnknownEntryAndUnsupportedTarget(t *testing.T) {
}
}
func TestServiceSanitizesClipboardWriteErrors(t *testing.T) {
t.Parallel()
service := Service{Writer: failingWriter{err: errors.New("backend refused token-1")}}
model := vault.Model{
Entries: []vault.Entry{
{ID: "vault-console", Password: "token-1"},
},
}
err := service.Copy(model, "vault-console", TargetPassword)
if !errors.Is(err, ErrWriteFailed) {
t.Fatalf("Copy() write error = %v, want ErrWriteFailed", err)
}
if err.Error() != ErrWriteFailed.Error() {
t.Fatalf("Copy() write error string = %q, want %q", err.Error(), ErrWriteFailed.Error())
}
}
type memoryWriter struct {
content string
}
@@ -76,3 +95,11 @@ func (w *memoryWriter) WriteText(text string) error {
w.content = text
return nil
}
type failingWriter struct {
err error
}
func (w failingWriter) WriteText(string) error {
return w.err
}
+15 -7
View File
@@ -86,7 +86,6 @@ type ui struct {
copyUser widget.Clickable
copyPass widget.Clickable
copyURL widget.Clickable
openURL widget.Clickable
lockVault widget.Clickable
unlockVault widget.Clickable
createVault widget.Clickable
@@ -135,6 +134,7 @@ type ui struct {
eyeIcon *widget.Icon
eyeOffIcon *widget.Icon
copyIcon *widget.Icon
clipboardWriter clipboard.Writer
loadingMessage string
statusMessage string
errorMessage string
@@ -975,10 +975,7 @@ func (u *ui) detailPanel(gtx layout.Context) layout.Dimensions {
layout.Rigid(u.entryEditorPanel),
)
}
password := strings.Repeat("•", max(8, len(item.Password)))
if u.showPassword {
password = item.Password
}
password := u.detailPasswordValue()
titleSize := unit.Sp(26)
titlePad := unit.Dp(10)
sectionGap := unit.Dp(8)
@@ -1016,7 +1013,7 @@ func (u *ui) detailPanel(gtx layout.Context) layout.Dimensions {
}),
layout.Rigid(layout.Spacer{Height: unit.Dp(8)}.Layout),
layout.Rigid(func(gtx layout.Context) layout.Dimensions {
return tonedButton(gtx, u.theme, &u.openURL, "Open URL")
return tonedButton(gtx, u.theme, &u.copyURL, "Copy URL")
}),
)
}
@@ -1032,7 +1029,7 @@ func (u *ui) detailPanel(gtx layout.Context) layout.Dimensions {
}),
layout.Rigid(layout.Spacer{Width: unit.Dp(8)}.Layout),
layout.Rigid(func(gtx layout.Context) layout.Dimensions {
btn := material.Button(u.theme, &u.openURL, "Open URL")
btn := material.Button(u.theme, &u.copyURL, "Copy URL")
return btn.Layout(gtx)
}),
)
@@ -1201,6 +1198,17 @@ func (u *ui) passwordLine(label, value string) layout.Widget {
}
}
func (u *ui) detailPasswordValue() string {
item, ok := u.selectedEntry()
if !ok {
return ""
}
if u.showPassword {
return item.Password
}
return strings.Repeat("•", max(8, len(item.Password)))
}
func card(gtx layout.Context, w layout.Widget) layout.Dimensions {
return layout.Background{}.Layout(gtx, fill(panelColor), func(gtx layout.Context) layout.Dimensions {
return layout.UniformInset(unit.Dp(16)).Layout(gtx, w)
+143
View File
@@ -8,6 +8,7 @@ import (
"os"
"path/filepath"
"slices"
"strings"
"testing"
"git.julianfamily.org/keepassgo/clipboard"
@@ -891,3 +892,145 @@ func TestUIUsesKeePassGOProductCopy(t *testing.T) {
t.Fatalf("desktopSubtitle = %q, want updated product subtitle", desktopSubtitle)
}
}
func TestUICopyActionsWriteExpectedClipboardContentsAndSanitizedFeedback(t *testing.T) {
t.Parallel()
model := vault.Model{
Entries: []vault.Entry{
{
ID: "vault-console",
Title: "Vault Console",
Username: "dannyocean",
Password: "token-1",
URL: "https://vault.crew.example.invalid",
Path: []string{"Root", "Internet"},
},
},
}
tests := []struct {
name string
target clipboard.Target
label string
want string
}{
{name: "username", target: clipboard.TargetUsername, label: "copy username", want: "dannyocean"},
{name: "password", target: clipboard.TargetPassword, label: "copy password", want: "token-1"},
{name: "url", target: clipboard.TargetURL, label: "copy URL", want: "https://vault.crew.example.invalid"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
u := newUIWithModel("desktop", model)
writer := &memoryClipboardWriter{}
u.clipboardWriter = writer
u.showEntriesSection()
u.currentPath = []string{"Root", "Internet"}
u.filter()
u.state.SelectedEntryID = "vault-console"
u.runAction(tt.label, func() error { return u.copySelectedFieldAction(tt.target) })
if writer.content != tt.want {
t.Fatalf("clipboard content = %q, want %q", writer.content, tt.want)
}
if u.statusMessage != tt.label+" complete" {
t.Fatalf("statusMessage = %q, want %q", u.statusMessage, tt.label+" complete")
}
if u.errorMessage != "" {
t.Fatalf("errorMessage = %q, want empty", u.errorMessage)
}
if strings.Contains(u.statusMessage, tt.want) {
t.Fatalf("statusMessage = %q, must not contain copied secret or field value %q", u.statusMessage, tt.want)
}
})
}
}
func TestUICopyActionSanitizesClipboardBackendErrors(t *testing.T) {
t.Parallel()
u := newUIWithModel("desktop", vault.Model{
Entries: []vault.Entry{
{
ID: "vault-console",
Title: "Vault Console",
Username: "dannyocean",
Password: "token-1",
URL: "https://vault.crew.example.invalid",
Path: []string{"Root", "Internet"},
},
},
})
u.clipboardWriter = failingClipboardWriter{err: os.ErrPermission}
u.showEntriesSection()
u.currentPath = []string{"Root", "Internet"}
u.filter()
u.state.SelectedEntryID = "vault-console"
u.runAction("copy password", func() error { return u.copySelectedFieldAction(clipboard.TargetPassword) })
if u.errorMessage != clipboard.ErrWriteFailed.Error() {
t.Fatalf("errorMessage = %q, want %q", u.errorMessage, clipboard.ErrWriteFailed.Error())
}
if strings.Contains(u.errorMessage, "token-1") {
t.Fatalf("errorMessage = %q, must not contain copied password", u.errorMessage)
}
if u.statusMessage != "" {
t.Fatalf("statusMessage = %q, want empty on copy failure", u.statusMessage)
}
}
func TestUIPasswordRevealTogglesDisplayedPasswordAndLockResetsIt(t *testing.T) {
t.Parallel()
u := newUIWithModel("desktop", vault.Model{
Entries: []vault.Entry{
{
ID: "vault-console",
Title: "Vault Console",
Username: "dannyocean",
Password: "token-1",
Path: []string{"Root", "Internet"},
},
},
})
u.showEntriesSection()
u.currentPath = []string{"Root", "Internet"}
u.filter()
u.state.SelectedEntryID = "vault-console"
if got := u.detailPasswordValue(); got != "••••••••" {
t.Fatalf("detailPasswordValue() hidden = %q, want %q", got, "••••••••")
}
u.showPassword = true
if got := u.detailPasswordValue(); got != "token-1" {
t.Fatalf("detailPasswordValue() revealed = %q, want %q", got, "token-1")
}
if err := u.lockAction(); err != nil {
t.Fatalf("lockAction() error = %v", err)
}
if u.showPassword {
t.Fatal("showPassword = true after lockAction(), want false")
}
}
type memoryClipboardWriter struct {
content string
}
func (w *memoryClipboardWriter) WriteText(text string) error {
w.content = text
return nil
}
type failingClipboardWriter struct {
err error
}
func (w failingClipboardWriter) WriteText(string) error {
return w.err
}
+1 -1
View File
@@ -241,7 +241,7 @@ func (u *ui) copySelectedFieldAction(target clipboard.Target) error {
return err
}
service := clipboard.Service{}
service := clipboard.Service{Writer: u.clipboardWriter}
return service.Copy(model, u.state.SelectedEntryID, target)
}