Repository navigation
fix(store): reject empty secrets on Linux, Windows and posixage - #694
Conversation
Save and Upsert on the Linux secret service and the Windows credential manager now return ErrEmptySecret for an empty value, matching macOS. On Linux the check runs before connecting to the secret service, so an empty value never triggers an unlock prompt. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Move ErrEmptySecret to the store package so every store can return it, keeping keychain.ErrEmptySecret as an alias. posixage Save and Upsert now return it for an empty value before taking the lock or prompting for encryption keys. Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Benehiko
changed the title
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
|
|
||
| // ErrEmptySecret is returned by Save and Upsert when the secret's value is | ||
| // empty. | ||
| var ErrEmptySecret = errors.New("secret value must not be empty") |
There was a problem hiding this comment.
ErrEmptySecret is defined in store with its own errors.New, unlike ErrCredentialNotFound above, which aliases secrets.ErrNotFound from x/secrets.
Not-found is shared because it has to be recognised outside the store: a plugin passes the store's not-found on, the resolver API maps secrets.ErrNotFound to CodeNotFound, and the client maps it back. plugin.ErrNotFound and client.ErrSecretNotFound are the same value for the same reason.
An empty secret is a store-level validation error: it is returned by Save/Upsert before anything is written, and nothing beyond the store needs to recognise it. If plugins or the engine API should ever report it specially, it can move to x/secrets and be aliased here the same way.
Written by Claude (Anthropic) through the alki coding agent, at the direction of @Benehiko.
The store now returns store.ErrEmptySecret from Save and Upsert on every platform (#694). With --force, set asked the engine to authorize the overwrite before the store could reject the value, so the user approved an overwrite that then failed. Check the value right after reading it instead, before any engine round-trip. Move the value switch into a readSecret helper to keep SetCommand under the gocyclo limit. Signed-off-by: Johannes Großmann <grossmann.johannes@t-online.de>
3 participants
Follow-up to #687, which added
ErrEmptySecretto the macOS keychain. This makesSaveandUpsertreject an empty secret value in every store: the Linux secret service, the Windows credential manager, and posixage.Stacked on #687. Until #687 merges, this PR's diff also shows its commits; only the commits after 6057f1a are new here.
store:ErrEmptySecretnow lives in thestorepackage asstore.ErrEmptySecret, so posixage can return it without importingkeychain.keychain.ErrEmptySecretfrom fix(keychain): update existing items in place on Upsert #687 is removed rather than re-exported; it has not been released.Save(whichUpsertcalls) marshals the secret first and returnsErrEmptySecretbefore connecting to the secret service, so an empty value never opens a D-Bus connection or triggers an unlock prompt.encodeSecretreturnsErrEmptySecretfor an empty value, soSaveandUpsertrefuse it before writing anything or deleting old chunk credentials.Save(whichUpsertcalls) returnsErrEmptySecretbefore taking the lock or prompting for encryption keys.Store.Save/Store.Upsertinterface docs now say an empty value is rejected withErrEmptySecret.This is a behaviour change: callers that saved empty values on Linux, Windows, or posixage now get an error. The test doubles (
store/mocksandplugins/pass/teststore) are unchanged.Testing
keychain_test.go:SaveandUpsertwith a nil and an empty value both returnErrEmptySecret, and nothing is stored.TestKeychainSaveRejectsEmptySecretagainst the fake secret service: no connection is opened and nothing is created or updated.TestSaveRejectsEmptySecret:SaveandUpsertwith a nil and an empty value returnErrEmptySecret, the encryption callback is never called, and nothing is written.go test ./...instore/on macOS, andgo vet ./...cross-compiled for Linux and Windows. The Linux and Windows keychain tests themselves only run in CI.AI disclosure: the code, tests and this description were written by Claude (Anthropic) through the alki coding agent, at my direction.