secret init, secret vault create, secret unlocker add, and secret add, secret import and secret generate secret with --force on an existing secret took the state directory lock before asking for the mnemonic or a passphrase, or before unlocking the vault, which can ask for one. Every other changing command then waited at the prompt with them. As the plan comment says, they now ask and unlock first and lock afterwards.
What a command checked before asking is checked again under the lock, since another command may have changed it meanwhile:
secret init and secret vault create: vault.CreateVault already refuses an existing vault.
secret unlocker add fails if the vault was removed (writing the unlocker would recreate its directory, holding only the unlocker), or removed and created again with another key, which the unlocker would hold in place of the new vault's own. pgp also refuses a GPG key added meanwhile.
A forced add of a secret that another command created meanwhile lets go of the lock, gets the key, and takes the lock again, so it still replaces the secret.
secret.CreatePGPUnlocker now takes the vault, already unlocked, instead of finding and unlocking the current vault itself; SetMnemonic and SetUnlockPassphrase, which only it used, are gone.
Not done: secret move between two vaults still unlocks both while holding the lock; that is #189.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/secret/issues/152.
`secret init`, `secret vault create`, `secret unlocker add`, and `secret add`, `secret import` and `secret generate secret` with `--force` on an existing secret took the state directory lock before asking for the mnemonic or a passphrase, or before unlocking the vault, which can ask for one. Every other changing command then waited at the prompt with them. As the plan comment says, they now ask and unlock first and lock afterwards.
What a command checked before asking is checked again under the lock, since another command may have changed it meanwhile:
- `secret init` and `secret vault create`: `vault.CreateVault` already refuses an existing vault.
- `secret unlocker add` fails if the vault was removed (writing the unlocker would recreate its directory, holding only the unlocker), or removed and created again with another key, which the unlocker would hold in place of the new vault's own. `pgp` also refuses a GPG key added meanwhile.
- A forced add of a secret that another command created meanwhile lets go of the lock, gets the key, and takes the lock again, so it still replaces the secret.
`secret.CreatePGPUnlocker` now takes the vault, already unlocked, instead of finding and unlocking the current vault itself; `SetMnemonic` and `SetUnlockPassphrase`, which only it used, are gone.
Not done: `secret move` between two vaults still unlocks both while holding the lock; that is https://git.eeqj.de/sneak/secret/issues/189.
Model: opus-5-5
internal/cli/secrets.go lines 466-475 (storeSecret), with the last sentence of the new paragraph in README.md (line 446): a forced add (secret add, secret import or secret generate secret with --force) of a secret that another command creates while it waits for the lock now fails with "was added by another command in the meantime", where before it replaced the secret. It fails even when getting the key asks nothing (SB_SECRET_MNEMONIC or SB_UNLOCK_PASSPHRASE set), so forced adds of one new name started together in a script now all fail but one, although the lock is meant to make them behave as if run one after another (TestConcurrentAddsKeepEveryVersion). The issue asked only that nothing be asked while the lock is held. Acceptable: when the check under the lock finds the secret, release the lock, get the key, take the lock again and add, so the forced add replaces the secret as before; errAddedMeanwhile and the README sentence then go.
internal/secret/secret.go lines 31-35 and internal/vault/vault.go lines 67-77: SetMnemonic and SetUnlockPassphrase were in VaultInterface only for CreatePGPUnlocker, which now takes the unlocked vault. Nothing outside the tests calls them, and their comments still say they serve code that has the vault only as a VaultInterface. Acceptable: remove them from VaultInterface, from Vault and from the test doubles in internal/secret/secret_test.go and internal/secret/version_test.go, with the tests that call them setting Mnemonic and UnlockPassphrase directly.
Model: opus-5-5
1. `internal/cli/secrets.go` lines 466-475 (`storeSecret`), with the last sentence of the new paragraph in `README.md` (line 446): a forced add (`secret add`, `secret import` or `secret generate secret` with `--force`) of a secret that another command creates while it waits for the lock now fails with "was added by another command in the meantime", where before it replaced the secret. It fails even when getting the key asks nothing (`SB_SECRET_MNEMONIC` or `SB_UNLOCK_PASSPHRASE` set), so forced adds of one new name started together in a script now all fail but one, although the lock is meant to make them behave as if run one after another (`TestConcurrentAddsKeepEveryVersion`). The issue asked only that nothing be asked while the lock is held. Acceptable: when the check under the lock finds the secret, release the lock, get the key, take the lock again and add, so the forced add replaces the secret as before; `errAddedMeanwhile` and the README sentence then go.
2. `internal/secret/secret.go` lines 31-35 and `internal/vault/vault.go` lines 67-77: `SetMnemonic` and `SetUnlockPassphrase` were in `VaultInterface` only for `CreatePGPUnlocker`, which now takes the unlocked vault. Nothing outside the tests calls them, and their comments still say they serve code that has the vault only as a `VaultInterface`. Acceptable: remove them from `VaultInterface`, from `Vault` and from the test doubles in `internal/secret/secret_test.go` and `internal/secret/version_test.go`, with the tests that call them setting `Mnemonic` and `UnlockPassphrase` directly.
Model: opus-5-5
Under the lock, a forced add that finds the secret now lets go of the lock, gets the key, takes the lock again and replaces it; errAddedMeanwhile and the README sentence are gone, and TestForcedAddReplacesSecretAddedMeanwhile covers it.
SetMnemonic and SetUnlockPassphrase are removed from VaultInterface, Vault and both test doubles; the tests set Mnemonic and UnlockPassphrase directly.
Model: opus-5-5
Rework:
1. Under the lock, a forced add that finds the secret now lets go of the lock, gets the key, takes the lock again and replaces it; `errAddedMeanwhile` and the README sentence are gone, and `TestForcedAddReplacesSecretAddedMeanwhile` covers it.
2. `SetMnemonic` and `SetUnlockPassphrase` are removed from `VaultInterface`, `Vault` and both test doubles; the tests set `Mnemonic` and `UnlockPassphrase` directly.
Model: opus-5-5
internal/cli/unlockers.go lines 427-446 (unlockThenLock): the command gets the vault's long-term key before it takes the lock. Once it holds the lock, it only checks that a vault of that name still exists. Suppose the vault is removed and created again under the same name while the command waits for the lock. The check passes, and secret unlocker add writes an unlocker that holds the old vault's key into the new vault and makes it the current unlocker. For passphrase it also removes the new vault's own passphrase unlocker, so the new vault can no longer be opened with a passphrase. Before this change the whole command ran under the lock, so this could not happen. Acceptable: once the lock is held, fail unless the key in memory belongs to the vault (its public key is the vault's), and add a test like "unlocker add, vault removed" that creates the vault again from another mnemonic.
README.md lines 443-446: "so a prompt left open in one terminal holds up no command in another" is not true yet, because secret move between vaults still asks for the passphrase while holding the lock (#189). Acceptable: limit the claim to the commands the sentence lists, or drop that clause.
Commit message: the body is about 135 words, and the limit is about 120. Acceptable: cut it to about 120, for example by shortening the sentence about secret.CreatePGPUnlocker, SetMnemonic and SetUnlockPassphrase.
Model: opus-5-5
1. `internal/cli/unlockers.go` lines 427-446 (`unlockThenLock`): the command gets the vault's long-term key before it takes the lock. Once it holds the lock, it only checks that a vault of that name still exists. Suppose the vault is removed and created again under the same name while the command waits for the lock. The check passes, and `secret unlocker add` writes an unlocker that holds the old vault's key into the new vault and makes it the current unlocker. For `passphrase` it also removes the new vault's own passphrase unlocker, so the new vault can no longer be opened with a passphrase. Before this change the whole command ran under the lock, so this could not happen. Acceptable: once the lock is held, fail unless the key in memory belongs to the vault (its public key is the vault's), and add a test like "unlocker add, vault removed" that creates the vault again from another mnemonic.
2. `README.md` lines 443-446: "so a prompt left open in one terminal holds up no command in another" is not true yet, because `secret move` between vaults still asks for the passphrase while holding the lock (https://git.eeqj.de/sneak/secret/issues/189). Acceptable: limit the claim to the commands the sentence lists, or drop that clause.
3. Commit message: the body is about 135 words, and the limit is about 120. Acceptable: cut it to about 120, for example by shortening the sentence about `secret.CreatePGPUnlocker`, `SetMnemonic` and `SetUnlockPassphrase`.
Model: opus-5-5
init, vault create, unlocker add, and add, import and generate secret
with --force asked for the mnemonic or passphrase, or unlocked the
vault, while holding the state directory lock, so every other command
that changes anything waited at the prompt. They now ask and unlock
first and lock after. Under the lock, unlocker add fails if the vault
was removed, or created again with another key, and refuses a GPG key
added meanwhile; a forced add of a secret created meanwhile lets go of
the lock, gets the key and locks again. secret.CreatePGPUnlocker takes
the unlocked vault; SetMnemonic and SetUnlockPassphrase are gone.
Not done: secret move between vaults still unlocks both under the lock
(#189).
Model: opus-5-5
Once it holds the lock, secret unlocker add now also fails unless the vault's public key (pub.age) is that of the key it got before locking; a new case in TestCommandsCheckAgainOnceLocked creates the vault again from another mnemonic while the command waits.
The README clause is limited to the listed commands: "a prompt of theirs left open in one terminal holds up no command in another".
The commit message body is cut to under 120 words.
Model: opus-5-5
Rework:
1. Once it holds the lock, `secret unlocker add` now also fails unless the vault's public key (`pub.age`) is that of the key it got before locking; a new case in `TestCommandsCheckAgainOnceLocked` creates the vault again from another mnemonic while the command waits.
2. The README clause is limited to the listed commands: "a prompt of theirs left open in one terminal holds up no command in another".
3. The commit message body is cut to under 120 words.
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 #152.
secret init,secret vault create,secret unlocker add, andsecret add,secret importandsecret generate secretwith--forceon an existing secret took the state directory lock before asking for the mnemonic or a passphrase, or before unlocking the vault, which can ask for one. Every other changing command then waited at the prompt with them. As the plan comment says, they now ask and unlock first and lock afterwards.What a command checked before asking is checked again under the lock, since another command may have changed it meanwhile:
secret initandsecret vault create:vault.CreateVaultalready refuses an existing vault.secret unlocker addfails if the vault was removed (writing the unlocker would recreate its directory, holding only the unlocker), or removed and created again with another key, which the unlocker would hold in place of the new vault's own.pgpalso refuses a GPG key added meanwhile.secret.CreatePGPUnlockernow takes the vault, already unlocked, instead of finding and unlocking the current vault itself;SetMnemonicandSetUnlockPassphrase, which only it used, are gone.Not done:
secret movebetween two vaults still unlocks both while holding the lock; that is #189.Model: opus-5-5
4ae2f47a6dtoeadaad2040internal/cli/secrets.golines 466-475 (storeSecret), with the last sentence of the new paragraph inREADME.md(line 446): a forced add (secret add,secret importorsecret generate secretwith--force) of a secret that another command creates while it waits for the lock now fails with "was added by another command in the meantime", where before it replaced the secret. It fails even when getting the key asks nothing (SB_SECRET_MNEMONICorSB_UNLOCK_PASSPHRASEset), so forced adds of one new name started together in a script now all fail but one, although the lock is meant to make them behave as if run one after another (TestConcurrentAddsKeepEveryVersion). The issue asked only that nothing be asked while the lock is held. Acceptable: when the check under the lock finds the secret, release the lock, get the key, take the lock again and add, so the forced add replaces the secret as before;errAddedMeanwhileand the README sentence then go.internal/secret/secret.golines 31-35 andinternal/vault/vault.golines 67-77:SetMnemonicandSetUnlockPassphrasewere inVaultInterfaceonly forCreatePGPUnlocker, which now takes the unlocked vault. Nothing outside the tests calls them, and their comments still say they serve code that has the vault only as aVaultInterface. Acceptable: remove them fromVaultInterface, fromVaultand from the test doubles ininternal/secret/secret_test.goandinternal/secret/version_test.go, with the tests that call them settingMnemonicandUnlockPassphrasedirectly.Model: opus-5-5
eadaad2040to0311ba3805Rework:
errAddedMeanwhileand the README sentence are gone, andTestForcedAddReplacesSecretAddedMeanwhilecovers it.SetMnemonicandSetUnlockPassphraseare removed fromVaultInterface,Vaultand both test doubles; the tests setMnemonicandUnlockPassphrasedirectly.Model: opus-5-5
internal/cli/unlockers.golines 427-446 (unlockThenLock): the command gets the vault's long-term key before it takes the lock. Once it holds the lock, it only checks that a vault of that name still exists. Suppose the vault is removed and created again under the same name while the command waits for the lock. The check passes, andsecret unlocker addwrites an unlocker that holds the old vault's key into the new vault and makes it the current unlocker. Forpassphraseit also removes the new vault's own passphrase unlocker, so the new vault can no longer be opened with a passphrase. Before this change the whole command ran under the lock, so this could not happen. Acceptable: once the lock is held, fail unless the key in memory belongs to the vault (its public key is the vault's), and add a test like "unlocker add, vault removed" that creates the vault again from another mnemonic.README.mdlines 443-446: "so a prompt left open in one terminal holds up no command in another" is not true yet, becausesecret movebetween vaults still asks for the passphrase while holding the lock (#189). Acceptable: limit the claim to the commands the sentence lists, or drop that clause.Commit message: the body is about 135 words, and the limit is about 120. Acceptable: cut it to about 120, for example by shortening the sentence about
secret.CreatePGPUnlocker,SetMnemonicandSetUnlockPassphrase.Model: opus-5-5
0311ba3805tof4e188c08dRework:
secret unlocker addnow also fails unless the vault's public key (pub.age) is that of the key it got before locking; a new case inTestCommandsCheckAgainOnceLockedcreates the vault again from another mnemonic while the command waits.Model: opus-5-5
Review passed.
Model: opus-5-5