Fixes the first half of #42. The mnemonic half moved to #60.
PGPUnlocker.GetID() no longer panics when its metadata cannot be read or parsed, or has no GPG key ID. It warns, naming the unlocker's directory, and returns pgp-unknown. That ID cannot clash with a real one, because the stored key ID is a hex fingerprint.
Vault.ListUnlockers() used to fail when one unlocker's metadata file could not be read or was not JSON, so secret unlocker list showed nothing. It now skips that unlocker with a warning naming the directory, as it already did for a missing metadata file.
What the diff does not show:
GetID() is called several times per command, often as a debug-log argument that is evaluated even with debugging off. So if the corrupt PGP unlocker is also the current one, its warning prints more than once.
The mnemonic half would touch 13 places in 10 files that read these variables, and some commands read the same variable twice. Each variable has to be read once per command and passed down, which is beyond a tight diff. No child process needs the mnemonic in its environment.
Judgement call: PGP metadata with an empty GPG key ID now counts as corrupt. Before this change it gave the ID pgp-.
Judgement call: the not-JSON case was not a panic, but the issue's definition of done requires that one corrupt unlocker not stop the others from being listed.
Model: opus-5-5
Fixes the first half of https://git.eeqj.de/sneak/secret/issues/42. The mnemonic half moved to https://git.eeqj.de/sneak/secret/issues/60.
- `PGPUnlocker.GetID()` no longer panics when its metadata cannot be read or parsed, or has no GPG key ID. It warns, naming the unlocker's directory, and returns `pgp-unknown`. That ID cannot clash with a real one, because the stored key ID is a hex fingerprint.
- `Vault.ListUnlockers()` used to fail when one unlocker's metadata file could not be read or was not JSON, so `secret unlocker list` showed nothing. It now skips that unlocker with a warning naming the directory, as it already did for a missing metadata file.
What the diff does not show:
- `GetID()` is called several times per command, often as a debug-log argument that is evaluated even with debugging off. So if the corrupt PGP unlocker is also the current one, its warning prints more than once.
- The mnemonic half would touch 13 places in 10 files that read these variables, and some commands read the same variable twice. Each variable has to be read once per command and passed down, which is beyond a tight diff. No child process needs the mnemonic in its environment.
Judgement call: PGP metadata with an empty GPG key ID now counts as corrupt. Before this change it gave the ID `pgp-`.
Judgement call: the not-JSON case was not a panic, but the issue's definition of done requires that one corrupt unlocker not stop the others from being listed.
Model: opus-5-5
PGPUnlocker.GetID() panicked when its metadata could not be read or
parsed, which took down `secret unlocker list` for every unlocker. It
now warns with the unlocker's directory and returns `pgp-unknown`;
metadata with an empty GPG key ID counts as corrupt too.
ListUnlockers now skips, with a warning, an unlocker whose metadata
file is unreadable or not JSON, as it already did for a missing one.
This is the first half of the issue only. Passing the mnemonic in
memory moved to #60.
Model: opus-5-5
internal/vault/unlockers.go, ListUnlockers, the branch for a metadata file that cannot be read: no test covers the new skip-with-warning. The definition of done in #42 names unreadable unlockers alongside corrupt ones. Acceptable: a test where reading one unlocker's metadata file fails and the other unlocker is still listed with its real ID. A filesystem wrapper like unlockersDirFailFs in internal/cli/unlockers_list_test.go can make the read fail.
PR body, "What the diff does not show": it says warnings repeat only when the corrupt PGP unlocker is the current one. A metadata file that is not JSON also repeats. ListUnlockers warns once, then the listing's ID lookup (findUnlockerIDByMetadata in internal/cli/unlockers.go) warns again, with a different message, for each listed unlocker whose directory sorts after it. Acceptable: say so in the PR body, or make the listing warn once per directory.
Judgement call: secret unlocker select and secret unlocker remove still fail outright when a metadata file that is not JSON sorts before the target. findUnlockerByID in internal/vault/unlockers.go still returns an error where ListUnlockers now skips. The definition of done covers listing only, so this is not counted here; it needs its own issue.
Model: opus-5-5
**FAIL** (needs-rework)
1. `internal/vault/unlockers.go`, `ListUnlockers`, the branch for a metadata file that cannot be read: no test covers the new skip-with-warning. The definition of done in https://git.eeqj.de/sneak/secret/issues/42 names unreadable unlockers alongside corrupt ones. Acceptable: a test where reading one unlocker's metadata file fails and the other unlocker is still listed with its real ID. A filesystem wrapper like `unlockersDirFailFs` in `internal/cli/unlockers_list_test.go` can make the read fail.
2. PR body, "What the diff does not show": it says warnings repeat only when the corrupt PGP unlocker is the current one. A metadata file that is not JSON also repeats. `ListUnlockers` warns once, then the listing's ID lookup (`findUnlockerIDByMetadata` in `internal/cli/unlockers.go`) warns again, with a different message, for each listed unlocker whose directory sorts after it. Acceptable: say so in the PR body, or make the listing warn once per directory.
Judgement call: `secret unlocker select` and `secret unlocker remove` still fail outright when a metadata file that is not JSON sorts before the target. `findUnlockerByID` in `internal/vault/unlockers.go` still returns an error where `ListUnlockers` now skips. The definition of done covers listing only, so this is not counted here; it needs its own issue.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes the first half of #42. The mnemonic half moved to #60.
PGPUnlocker.GetID()no longer panics when its metadata cannot be read or parsed, or has no GPG key ID. It warns, naming the unlocker's directory, and returnspgp-unknown. That ID cannot clash with a real one, because the stored key ID is a hex fingerprint.Vault.ListUnlockers()used to fail when one unlocker's metadata file could not be read or was not JSON, sosecret unlocker listshowed nothing. It now skips that unlocker with a warning naming the directory, as it already did for a missing metadata file.What the diff does not show:
GetID()is called several times per command, often as a debug-log argument that is evaluated even with debugging off. So if the corrupt PGP unlocker is also the current one, its warning prints more than once.Judgement call: PGP metadata with an empty GPG key ID now counts as corrupt. Before this change it gave the ID
pgp-.Judgement call: the not-JSON case was not a panic, but the issue's definition of done requires that one corrupt unlocker not stop the others from being listed.
Model: opus-5-5
FAIL (needs-rework)
internal/vault/unlockers.go,ListUnlockers, the branch for a metadata file that cannot be read: no test covers the new skip-with-warning. The definition of done in #42 names unreadable unlockers alongside corrupt ones. Acceptable: a test where reading one unlocker's metadata file fails and the other unlocker is still listed with its real ID. A filesystem wrapper likeunlockersDirFailFsininternal/cli/unlockers_list_test.gocan make the read fail.ListUnlockerswarns once, then the listing's ID lookup (findUnlockerIDByMetadataininternal/cli/unlockers.go) warns again, with a different message, for each listed unlocker whose directory sorts after it. Acceptable: say so in the PR body, or make the listing warn once per directory.Judgement call:
secret unlocker selectandsecret unlocker removestill fail outright when a metadata file that is not JSON sorts before the target.findUnlockerByIDininternal/vault/unlockers.gostill returns an error whereListUnlockersnow skips. The definition of done covers listing only, so this is not counted here; it needs its own issue.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.