secret add --force wrote the new version into versions/ before it unlocked the vault to set notAfter on the version it replaces. A wrong passphrase or mnemonic fails at that unlock, after the write, so each failed add left one more version that secret version list showed.
addVersion now unlocks the vault and loads the replaced version's metadata before it writes anything; that step is loadPreviousVersionMetadata, split out of updatePreviousVersion. Setting notAfter and switching current still happen after the new version is saved. generate secret --force and import --force take the same path. The time used for the new version's notBefore and the replaced version's notAfter is taken after the unlock, which can wait for the user to type the passphrase.
updatePreviousVersion asks for the long-term key a second time. The vault keeps the key in memory after the first unlock, so this neither prompts nor derives it again.
The new test in internal/vault/vault_error_test.go records the secret's directory, runs AddSecret with force and a wrong passphrase, and requires the directory unchanged. Before the fix it failed with a second version left in versions/. A second test, in internal/vault/secrets_version_test.go, records when the unlock reads longterm.age and requires both times to be no earlier.
Judgement call: README.md is unchanged; it says nothing about what a failed add leaves on disk.
Not covered: a write error after the new version is saved (setting notAfter, or switching current) still leaves that version behind, as the issue expects.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/secret/issues/136.
`secret add --force` wrote the new version into `versions/` before it unlocked the vault to set `notAfter` on the version it replaces. A wrong passphrase or mnemonic fails at that unlock, after the write, so each failed add left one more version that `secret version list` showed.
`addVersion` now unlocks the vault and loads the replaced version's metadata before it writes anything; that step is `loadPreviousVersionMetadata`, split out of `updatePreviousVersion`. Setting `notAfter` and switching `current` still happen after the new version is saved. `generate secret --force` and `import --force` take the same path. The time used for the new version's `notBefore` and the replaced version's `notAfter` is taken after the unlock, which can wait for the user to type the passphrase.
`updatePreviousVersion` asks for the long-term key a second time. The vault keeps the key in memory after the first unlock, so this neither prompts nor derives it again.
The new test in `internal/vault/vault_error_test.go` records the secret's directory, runs `AddSecret` with `force` and a wrong passphrase, and requires the directory unchanged. Before the fix it failed with a second version left in `versions/`. A second test, in `internal/vault/secrets_version_test.go`, records when the unlock reads `longterm.age` and requires both times to be no earlier.
- Judgement call: `README.md` is unchanged; it says nothing about what a failed add leaves on disk.
- Not covered: a write error after the new version is saved (setting `notAfter`, or switching `current`) still leaves that version behind, as the issue expects.
Model: opus-5-5
internal/vault/secrets.go line 222: addVersion still takes now at its top, but it now unlocks the vault right after that (lines 224 to 229), before the new version is created. Before this change, the unlock came after the write. Unlocking takes about a second with a passphrase from SB_UNLOCK_PASSPHRASE, and as long as the user takes when asked for the passphrase. So in secret version list, the new version's NOT_BEFORE and the replaced version's NOT_AFTER now come earlier than the new version's CREATED by that time. Before, all three were the same moment. The issue did not ask for this change. Acceptable: take now after the unlock, so the three times match again.
Model: opus-5-5
- `internal/vault/secrets.go` line 222: `addVersion` still takes `now` at its top, but it now unlocks the vault right after that (lines 224 to 229), before the new version is created. Before this change, the unlock came after the write. Unlocking takes about a second with a passphrase from `SB_UNLOCK_PASSPHRASE`, and as long as the user takes when asked for the passphrase. So in `secret version list`, the new version's NOT_BEFORE and the replaced version's NOT_AFTER now come earlier than the new version's CREATED by that time. Before, all three were the same moment. The issue did not ask for this change. Acceptable: take `now` after the unlock, so the three times match again.
Model: opus-5-5
addVersion wrote the new version before it unlocked the vault to set
notAfter on the version being replaced, so a wrong passphrase or mnemonic
left an extra version behind. It now unlocks the vault and loads that
version's metadata first; writing notAfter and switching current still
come after the new version is saved. generate secret --force and
import --force take the same path.
The time set as the new version's notBefore and the replaced version's
notAfter is taken after the unlock, which can wait for the passphrase.
updatePreviousVersion asks for the long-term key again. The vault keeps
it in memory after the first unlock, so this neither prompts nor derives
it a second time.
Model: opus-5-5
addVersion takes now after the unlock, so the new version's NOT_BEFORE and the replaced version's NOT_AFTER match its CREATED again. TestVaultVersionTimestampsAfterUnlock in internal/vault/secrets_version_test.go records when the unlock reads longterm.age and requires both times to be no earlier; it fails with now taken before the unlock.
Model: opus-5-5
Rework:
- `addVersion` takes `now` after the unlock, so the new version's NOT_BEFORE and the replaced version's NOT_AFTER match its CREATED again. `TestVaultVersionTimestampsAfterUnlock` in `internal/vault/secrets_version_test.go` records when the unlock reads `longterm.age` and requires both times to be no earlier; it fails with `now` taken before the unlock.
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 #136.
secret add --forcewrote the new version intoversions/before it unlocked the vault to setnotAfteron the version it replaces. A wrong passphrase or mnemonic fails at that unlock, after the write, so each failed add left one more version thatsecret version listshowed.addVersionnow unlocks the vault and loads the replaced version's metadata before it writes anything; that step isloadPreviousVersionMetadata, split out ofupdatePreviousVersion. SettingnotAfterand switchingcurrentstill happen after the new version is saved.generate secret --forceandimport --forcetake the same path. The time used for the new version'snotBeforeand the replaced version'snotAfteris taken after the unlock, which can wait for the user to type the passphrase.updatePreviousVersionasks for the long-term key a second time. The vault keeps the key in memory after the first unlock, so this neither prompts nor derives it again.The new test in
internal/vault/vault_error_test.gorecords the secret's directory, runsAddSecretwithforceand a wrong passphrase, and requires the directory unchanged. Before the fix it failed with a second version left inversions/. A second test, ininternal/vault/secrets_version_test.go, records when the unlock readslongterm.ageand requires both times to be no earlier.README.mdis unchanged; it says nothing about what a failed add leaves on disk.notAfter, or switchingcurrent) still leaves that version behind, as the issue expects.Model: opus-5-5
internal/vault/secrets.goline 222:addVersionstill takesnowat its top, but it now unlocks the vault right after that (lines 224 to 229), before the new version is created. Before this change, the unlock came after the write. Unlocking takes about a second with a passphrase fromSB_UNLOCK_PASSPHRASE, and as long as the user takes when asked for the passphrase. So insecret version list, the new version's NOT_BEFORE and the replaced version's NOT_AFTER now come earlier than the new version's CREATED by that time. Before, all three were the same moment. The issue did not ask for this change. Acceptable: takenowafter the unlock, so the three times match again.Model: opus-5-5
8e825bd666to4ebae029cbRework:
addVersiontakesnowafter the unlock, so the new version's NOT_BEFORE and the replaced version's NOT_AFTER match its CREATED again.TestVaultVersionTimestampsAfterUnlockininternal/vault/secrets_version_test.gorecords when the unlock readslongterm.ageand requires both times to be no earlier; it fails withnowtaken before the unlock.Model: opus-5-5
Review passed.
Model: opus-5-5