Ask for the mnemonic and passphrase before taking the state directory lock (closes #152)
check / check (push) Waiting to run
check / check (push) Waiting to run
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
This commit was merged in pull request #188.
This commit is contained in:
+328
-14
@@ -280,9 +280,9 @@ func stateDirModTimes(t *testing.T, fs afero.Fs) map[string]int64 {
|
||||
return modTimes
|
||||
}
|
||||
|
||||
// setupEveryCommand makes what each command in
|
||||
// TestChangingCommandsWaitForLock needs: the current vault "work" with two
|
||||
// versions of "test/secret", the vault "other", and the file testInput.
|
||||
// setupEveryCommand makes what the commands that the lock tests below run
|
||||
// need: the current vault "work" with two versions of "test/secret", the
|
||||
// vault "other", and the file testInput.
|
||||
// There is no vault "default", which init creates. If withUnlocker is set,
|
||||
// it also gives "work" a passphrase unlocker, which is slow. It returns the
|
||||
// older version and the unlocker's ID.
|
||||
@@ -346,6 +346,25 @@ func waitingForLock() bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// requireWaitingForLock returns once a command waits for the state
|
||||
// directory lock. It fails the test if the command finishes first, sending
|
||||
// its error on done, or never waits.
|
||||
func requireWaitingForLock(t *testing.T, done <-chan error) {
|
||||
t.Helper()
|
||||
|
||||
timeout := time.After(lockWait)
|
||||
|
||||
for !waitingForLock() {
|
||||
select {
|
||||
case err := <-done:
|
||||
t.Fatalf("finished while the lock was held, with error %v", err)
|
||||
case <-timeout:
|
||||
t.Fatal("never waited for the lock")
|
||||
case <-time.After(time.Millisecond):
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// requireWaitsForLock runs a command, given what setupEveryCommand made,
|
||||
// while holding the state directory lock. The command must neither finish
|
||||
// nor change anything before it waits for the lock, and must succeed once
|
||||
@@ -385,17 +404,7 @@ func requireWaitsForLock(
|
||||
|
||||
go func() { done <- run(cli, olderVersion, unlockerID) }()
|
||||
|
||||
timeout := time.After(lockWait)
|
||||
|
||||
for !waitingForLock() {
|
||||
select {
|
||||
case err := <-done:
|
||||
t.Fatalf("finished while the lock was held, with error %v", err)
|
||||
case <-timeout:
|
||||
t.Fatal("never waited for the lock")
|
||||
case <-time.After(time.Millisecond):
|
||||
}
|
||||
}
|
||||
requireWaitingForLock(t, done)
|
||||
|
||||
assert.Equal(t, before, stateDirModTimes(t, fs),
|
||||
"changed the state directory before waiting for the lock")
|
||||
@@ -472,6 +481,311 @@ func TestChangingCommandsWaitForLock(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCommandsAskBeforeLocking is a regression test for
|
||||
// https://git.eeqj.de/sneak/secret/issues/152: these commands took the
|
||||
// state directory lock before asking for the mnemonic or a passphrase, so
|
||||
// while one waited at its prompt, every other command that changes anything
|
||||
// waited for it. Each runs here while the test holds the lock, given what
|
||||
// takes it to the last step at which it can ask: the passphrase prompt of
|
||||
// init and vault create, and the unlock of the vault, which asks for the
|
||||
// passphrase of its passphrase unlocker, for the others. The tests have no
|
||||
// terminal, so a prompt fails at once. Each command must get there, and
|
||||
// fail, without waiting for the lock.
|
||||
//
|
||||
//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
|
||||
func TestCommandsAskBeforeLocking(t *testing.T) {
|
||||
newTestGPGKey(t)
|
||||
|
||||
mnemonic := testMnemonicBuffer(t)
|
||||
|
||||
wrongPassphrase := memguard.NewBufferFromBytes([]byte("wrong-passphrase"))
|
||||
t.Cleanup(wrongPassphrase.Destroy)
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
run func(cli *Instance) error
|
||||
// want is wrapped in the error the command fails with
|
||||
want error
|
||||
}{
|
||||
{"init", func(cli *Instance) error {
|
||||
cli.Mnemonic = mnemonic
|
||||
|
||||
return cli.Init(cli.cmd)
|
||||
}, secret.ErrNoTerminal},
|
||||
{"vault create", func(cli *Instance) error {
|
||||
cli.Mnemonic = mnemonic
|
||||
|
||||
return cli.CreateVault(cli.cmd, "created")
|
||||
}, secret.ErrNoTerminal},
|
||||
// With no terminal to ask for the new passphrase on, the new
|
||||
// unlocker takes the one in SB_UNLOCK_PASSPHRASE, which then fails
|
||||
// to unlock the vault
|
||||
{"unlocker add passphrase", func(cli *Instance) error {
|
||||
cli.UnlockPassphrase = wrongPassphrase
|
||||
|
||||
return cli.UnlockersAdd("passphrase", cli.cmd)
|
||||
}, secret.ErrWrongPassphrase},
|
||||
{"unlocker add pgp", func(cli *Instance) error {
|
||||
cli.cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
|
||||
|
||||
return cli.UnlockersAdd("pgp", cli.cmd)
|
||||
}, secret.ErrNoTerminal},
|
||||
{"add --force", func(cli *Instance) error {
|
||||
return cli.AddSecret("test/secret", true)
|
||||
}, secret.ErrNoTerminal},
|
||||
{"import --force", func(cli *Instance) error {
|
||||
return cli.ImportSecret(cli.cmd, "test/secret", testInput, true)
|
||||
}, secret.ErrNoTerminal},
|
||||
{"generate secret --force", func(cli *Instance) error {
|
||||
return cli.GenerateSecret(cli.cmd, "test/secret", 16, "base58", true)
|
||||
}, secret.ErrNoTerminal},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
requireAsksBeforeLocking(t, tc.run, tc.want)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// startWhileLocked makes what setupEveryCommand makes, with the passphrase
|
||||
// unlocker, on fs, an empty in-memory filesystem, takes the state directory
|
||||
// lock there, and starts run, given a CLI instance on fs with "value" on
|
||||
// stdin. It returns the function that releases the lock, which does so only
|
||||
// the first time it is called, and the channel that run's error comes on.
|
||||
// The lock is also released when the test ends, so that later tests can
|
||||
// take it.
|
||||
func startWhileLocked(
|
||||
t *testing.T, fs afero.Fs, run func(cli *Instance) error,
|
||||
) (func(), <-chan error) {
|
||||
t.Helper()
|
||||
|
||||
setupEveryCommand(t, fs, true)
|
||||
|
||||
release, err := vault.LockStateDir(fs, testStateDir)
|
||||
require.NoError(t, err)
|
||||
|
||||
release = sync.OnceFunc(release)
|
||||
t.Cleanup(release)
|
||||
|
||||
cli := NewCLIInstanceWithStateDir(fs, testStateDir)
|
||||
cli.cmd = &cobra.Command{}
|
||||
cli.cmd.SetIn(strings.NewReader("value"))
|
||||
cli.cmd.SetOut(io.Discard)
|
||||
|
||||
done := make(chan error, 1)
|
||||
|
||||
go func() { done <- run(cli) }()
|
||||
|
||||
return release, done
|
||||
}
|
||||
|
||||
// requireAsksBeforeLocking starts run while holding the state directory
|
||||
// lock, as startWhileLocked does. run must fail, with an error wrapping
|
||||
// want, without waiting for the lock.
|
||||
func requireAsksBeforeLocking(
|
||||
t *testing.T, run func(cli *Instance) error, want error,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
release, done := startWhileLocked(t, afero.NewMemMapFs(), run)
|
||||
|
||||
timeout := time.After(lockWait)
|
||||
|
||||
for !waitingForLock() {
|
||||
select {
|
||||
case err := <-done:
|
||||
require.ErrorIs(t, err, want)
|
||||
|
||||
return
|
||||
case <-timeout:
|
||||
t.Fatal("neither failed nor waited for the lock")
|
||||
case <-time.After(time.Millisecond):
|
||||
}
|
||||
}
|
||||
|
||||
// Let the command go on, so that it releases the lock
|
||||
release()
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(lockWait):
|
||||
}
|
||||
|
||||
t.Fatal("waited for the lock before asking")
|
||||
}
|
||||
|
||||
// TestCommandsCheckAgainOnceLocked checks that commands which check what
|
||||
// they change before they ask for anything, and so before they take the
|
||||
// state directory lock, check again once they hold it: another command may
|
||||
// have changed it in the meantime. Each runs while the test holds the lock,
|
||||
// given what it needs to ask for nothing. Once it waits for the lock, the
|
||||
// test makes the change another command could have made, then releases the
|
||||
// lock. The command must fail, and change nothing.
|
||||
//
|
||||
//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
|
||||
func TestCommandsCheckAgainOnceLocked(t *testing.T) {
|
||||
fingerprint := newTestGPGKey(t)
|
||||
|
||||
mnemonic := testMnemonicBuffer(t)
|
||||
|
||||
otherMnemonic := memguard.NewBufferFromBytes([]byte("legal winner thank " +
|
||||
"year wave sausage worth useful legal winner thank yellow"))
|
||||
t.Cleanup(otherMnemonic.Destroy)
|
||||
|
||||
passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
|
||||
t.Cleanup(passphrase.Destroy)
|
||||
|
||||
// work returns the vault "work" on fs, opened with the mnemonic
|
||||
work := func(fs afero.Fs) *vault.Vault {
|
||||
vlt := vault.NewVault(fs, testStateDir, "work")
|
||||
vlt.Mnemonic = mnemonic
|
||||
|
||||
return vlt
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
run func(cli *Instance) error
|
||||
// change is what another command changes while run waits
|
||||
change func(fs afero.Fs) error
|
||||
// want is wrapped in the error the command fails with
|
||||
want error
|
||||
}{
|
||||
{
|
||||
"unlocker add, vault removed",
|
||||
func(cli *Instance) error {
|
||||
cli.Mnemonic, cli.UnlockPassphrase = mnemonic, passphrase
|
||||
|
||||
return cli.UnlockersAdd("passphrase", cli.cmd)
|
||||
},
|
||||
func(fs afero.Fs) error {
|
||||
return fs.RemoveAll(testStateDir + "/vaults.d/work")
|
||||
},
|
||||
vault.ErrVaultNotFound,
|
||||
},
|
||||
{
|
||||
"unlocker add, vault created again from another mnemonic",
|
||||
func(cli *Instance) error {
|
||||
cli.Mnemonic, cli.UnlockPassphrase = mnemonic, passphrase
|
||||
|
||||
return cli.UnlockersAdd("passphrase", cli.cmd)
|
||||
},
|
||||
func(fs afero.Fs) error {
|
||||
err := fs.RemoveAll(testStateDir + "/vaults.d/work")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
_, err = vault.CreateVault(fs, testStateDir, "work",
|
||||
otherMnemonic, passphrase)
|
||||
|
||||
return err
|
||||
},
|
||||
errVaultCreatedAgain,
|
||||
},
|
||||
{
|
||||
"unlocker add pgp, key added",
|
||||
func(cli *Instance) error {
|
||||
cli.Mnemonic = mnemonic
|
||||
cli.cmd.Flags().String("keyid", unreadableTestGPGUserID, "")
|
||||
|
||||
return cli.UnlockersAdd("pgp", cli.cmd)
|
||||
},
|
||||
func(fs afero.Fs) error {
|
||||
_, err := secret.CreatePGPUnlocker(work(fs),
|
||||
unreadableTestGPGUserID, fingerprint)
|
||||
|
||||
return err
|
||||
},
|
||||
errGPGKeyAlreadyUnlocker,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
requireChecksAgainOnceLocked(t, tc.run, tc.change, tc.want)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// requireChecksAgainOnceLocked starts run while holding the state directory
|
||||
// lock, as startWhileLocked does. Once run waits for the lock, change makes
|
||||
// the change that another command could have made in the meantime, and the
|
||||
// lock is released. run must then fail, with an error wrapping want, and
|
||||
// change nothing.
|
||||
func requireChecksAgainOnceLocked(
|
||||
t *testing.T, run func(cli *Instance) error,
|
||||
change func(fs afero.Fs) error, want error,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
release, done := startWhileLocked(t, fs, run)
|
||||
|
||||
requireWaitingForLock(t, done)
|
||||
require.NoError(t, change(fs))
|
||||
|
||||
// Left out: taking and releasing the lock writes the lock file
|
||||
lockFile := testStateDir + "/lock"
|
||||
|
||||
before := stateDirModTimes(t, fs)
|
||||
delete(before, lockFile)
|
||||
|
||||
release()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
require.ErrorIs(t, err, want)
|
||||
case <-time.After(lockWait):
|
||||
t.Fatal("did not finish once the lock was released")
|
||||
}
|
||||
|
||||
after := stateDirModTimes(t, fs)
|
||||
delete(after, lockFile)
|
||||
|
||||
assert.Equal(t, before, after, "changed the state directory")
|
||||
}
|
||||
|
||||
// TestForcedAddReplacesSecretAddedMeanwhile runs a forced add of a secret
|
||||
// that does not exist yet while the test holds the state directory lock.
|
||||
// Once it waits for the lock, the test adds the secret, as another command
|
||||
// could have, and releases the lock. The forced add must replace that
|
||||
// secret, as if it had run after the other command.
|
||||
//
|
||||
//nolint:paralleltest // waitingForLock sees any test's command waiting for the lock
|
||||
func TestForcedAddReplacesSecretAddedMeanwhile(t *testing.T) {
|
||||
mnemonic := testMnemonicBuffer(t)
|
||||
|
||||
fs := afero.NewMemMapFs()
|
||||
release, done := startWhileLocked(t, fs, func(cli *Instance) error {
|
||||
cli.Mnemonic = mnemonic
|
||||
|
||||
return cli.AddSecret("added", true)
|
||||
})
|
||||
|
||||
requireWaitingForLock(t, done)
|
||||
|
||||
work := vault.NewVault(fs, testStateDir, "work")
|
||||
work.Mnemonic = mnemonic
|
||||
|
||||
other := memguard.NewBufferFromBytes([]byte("other"))
|
||||
t.Cleanup(other.Destroy)
|
||||
|
||||
require.NoError(t, work.AddSecret("added", other, false))
|
||||
|
||||
release()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
require.NoError(t, err)
|
||||
case <-time.After(lockWait):
|
||||
t.Fatal("did not finish once the lock was released")
|
||||
}
|
||||
|
||||
value, err := work.GetSecret("added")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(value.Destroy)
|
||||
|
||||
assert.Equal(t, "value", string(value.Bytes()))
|
||||
}
|
||||
|
||||
// TestEncryptWithExistingKeyTakesNoLock checks that secret encrypt with a
|
||||
// key that already exists, which only reads the state directory, finishes
|
||||
// while another command holds the state directory lock.
|
||||
|
||||
Reference in New Issue
Block a user