2 Commits
Author SHA1 Message Date
sneak d0e2fb8b20 Ask for the passphrase on the controlling terminal, not stdin (closes #144)
check / check (push) Canceled after 0s
The passphrase and mnemonic prompts open /dev/tty and ask there, so
`echo v | secret add s1 --force`, `secret encrypt` and `secret decrypt`
with piped input ask at a terminal instead of failing. Only a process
with no controlling terminal still gets the error naming the variable to
set. `secret init` and `secret vault create` no longer write a newline
to stderr after the mnemonic.

On SIGINT or SIGTERM, Entry() restores the settings of /dev/tty instead
of stdin, since that is where the prompt turns echo off.

The two tests that pin the no-terminal message run the binary in a new
session, so it has no terminal wherever the tests run.

Unverified by a test: the echo restore after Ctrl-C, checked by hand.

Model: opus-5-5
2026-10-14 19:42:36 +00:00
clawbot 5371382cb4 A failed secret add --force leaves the secret unchanged (closes #136)
check / check (push) Canceled after 0s
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
2026-10-14 20:48:48 +02:00
12 changed files with 465 additions and 60 deletions
+5
View File
@@ -372,6 +372,11 @@ which every command except `secret vault import` offers when the variable is not
set, is the safer default; `secret vault import` has no prompt and needs both set, is the safer default; `secret vault import` has no prompt and needs both
variables. variables.
The prompt asks on the terminal the command runs in, not on stdin, so it also
asks when stdin is a pipe, as in `echo value | secret add name --force` or
`cat file | secret encrypt key`. With no controlling terminal, as under cron or
in a CI job, the command fails and names the variable to set.
## Security Features ## Security Features
### Encryption ### Encryption
+18
View File
@@ -18,6 +18,24 @@ https://git.eeqj.de/sneak/secret/milestone/12
# Completed Steps # Completed Steps
- 2026-10-14: The passphrase and mnemonic prompts ask on the controlling
terminal (`/dev/tty`) instead of stdin, so `echo v | secret add s1 --force`,
`cat file | secret encrypt key` and `cat file.age | secret decrypt key` ask
for the passphrase at a terminal instead of failing
(https://git.eeqj.de/sneak/secret/issues/144). Only a process with no
controlling terminal gets the error that names the variable to set. On SIGINT
or SIGTERM, `Entry()` restores the settings of that terminal, not of stdin,
since that is where the prompt turns echo off. `secret init` and
`secret vault create` no longer write a newline to stderr after the mnemonic
is typed, since the prompt ends that line on the terminal.
- 2026-10-14: A `secret add --force` that cannot unlock the vault, given a wrong
passphrase or mnemonic, leaves the secret as it was
(https://git.eeqj.de/sneak/secret/issues/136). The new version used to be
written before the vault was unlocked to set `notAfter` on the version it
replaces, so each failed add left one more version that `secret version list`
showed. The vault is now unlocked, and that version's metadata read, before
anything is written. `generate secret --force` and `import --force` take the
same path.
- 2026-10-12: A mnemonic typed with extra spaces, tabs or line breaks gives the - 2026-10-12: A mnemonic typed with extra spaces, tabs or line breaks gives the
same long-term key as its words separated by single spaces same long-term key as its words separated by single spaces
(https://git.eeqj.de/sneak/secret/issues/135). The key was derived from the (https://git.eeqj.de/sneak/secret/issues/135). The key was derived from the
+8 -6
View File
@@ -8,6 +8,7 @@ import (
"os/exec" "os/exec"
"slices" "slices"
"strings" "strings"
"syscall"
"testing" "testing"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
@@ -197,10 +198,10 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
// TestMnemonicNotReadNamesOnlyMnemonic is a regression test for // TestMnemonicNotReadNamesOnlyMnemonic is a regression test for
// https://git.eeqj.de/sneak/secret/issues/115: `secret init` without // https://git.eeqj.de/sneak/secret/issues/115: `secret init` without
// SB_SECRET_MNEMONIC and with a stdin that is not a terminal said "failed to // SB_SECRET_MNEMONIC and with no terminal said "failed to read mnemonic:
// read mnemonic: failed to read passphrase: ...". The error must wrap // failed to read passphrase: ...". The error must wrap
// secret.ErrMnemonicNotRead and name the mnemonic only. The message is // secret.ErrMnemonicNotRead and name the mnemonic only. The message is
// pinned on the built binary, whose stdin is surely not a terminal. // pinned on the built binary, run in a new session, which has no terminal.
func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) { func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
t.Parallel() t.Parallel()
@@ -216,14 +217,15 @@ func TestMnemonicNotReadNamesOnlyMnemonic(t *testing.T) {
"PATH=" + os.Getenv("PATH"), "PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"), "HOME=" + os.Getenv("HOME"),
} }
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
output, err := cmd.CombinedOutput() output, err := cmd.CombinedOutput()
require.Error(t, err) require.Error(t, err)
require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+ require.Equal(t, "Initialized secrets manager at: "+stateDir+"\n"+
"Error: failed to read mnemonic: stdin is not a terminal (piped input "+ "Error: failed to read mnemonic: there is no terminal to ask on. "+
"or script). Please set the SB_SECRET_MNEMONIC environment variable "+ "Please set the SB_SECRET_MNEMONIC environment variable or run "+
"or run interactively\n", string(output)) "interactively\n", string(output))
} }
// TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for // TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for
+1 -4
View File
@@ -5,7 +5,6 @@ import (
"fmt" "fmt"
"log" "log"
"log/slog" "log/slog"
"os"
"strings" "strings"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
@@ -57,13 +56,11 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) {
// Read mnemonic securely without echo // Read mnemonic securely without echo
mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ") mnemonicBuffer, err := secret.ReadMnemonic("Enter your BIP39 mnemonic phrase: ")
if err != nil { if err != nil {
secret.Debug("Failed to read mnemonic from stdin", "error", err) secret.Debug("Failed to read mnemonic", "error", err)
return nil, nil, err return nil, nil, err
} }
fmt.Fprintln(os.Stderr) // Add newline after hidden input
return mnemonicBuffer, mnemonicBuffer.Destroy, nil return mnemonicBuffer, mnemonicBuffer.Destroy, nil
} }
+204
View File
@@ -0,0 +1,204 @@
package cli_test
import (
"bufio"
"bytes"
"context"
"io"
"os"
"os/exec"
"strings"
"syscall"
"testing"
"filippo.io/age"
"github.com/awnumar/memguard"
"github.com/creack/pty"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/secret/internal/secret"
"sneak.berlin/go/secret/internal/vault"
)
// TestPassphraseAskedAtTerminalWithPipedStdin is a regression test for
// https://git.eeqj.de/sneak/secret/issues/144: run at a terminal with stdin
// a pipe, as in `echo v2 | secret add s1 --force`, a command that needed
// the passphrase failed instead of asking for it. Each command here must ask
// on the terminal and succeed once the passphrase is typed there.
func TestPassphraseAskedAtTerminalWithPipedStdin(t *testing.T) {
t.Parallel()
stateDir := t.TempDir()
passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
defer passphrase.Destroy()
vlt, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
testMnemonicBuffer(t), passphrase)
require.NoError(t, err)
value := memguard.NewBufferFromBytes([]byte("v1"))
defer value.Destroy()
require.NoError(t, vlt.AddSecret("s1", value, false))
identity, err := age.GenerateX25519Identity()
require.NoError(t, err)
key := secret.IdentityToLockedBuffer(identity)
defer key.Destroy()
require.NoError(t, vlt.AddSecret("mykey", key, false))
runAtTerminal(t, stateDir, "v2\n", "add", "s1", "--force")
vlt.UnlockPassphrase = passphrase
added, err := vlt.GetSecret("s1")
require.NoError(t, err)
defer added.Destroy()
assert.Equal(t, "v2", added.String())
encrypted := runAtTerminal(t, stateDir, "a document\n", "encrypt", "mykey")
decrypted := runAtTerminal(t, stateDir, encrypted, "decrypt", "mykey")
assert.Equal(t, "a document\n", decrypted)
}
// TestInitWritesNoBlankLineAfterMnemonic runs `secret init 2>log` at a
// terminal: the terminal is its controlling terminal and none of stdin,
// stdout and stderr. The mnemonic is typed on the terminal and the prompt
// ends its line there, so stderr must not get a newline for it as well, a
// blank line in the log. pty.Open is described above
// TestRemoveIgnoresTerminalOnStdout.
func TestInitWritesNoBlankLineAfterMnemonic(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(t.Context(), commandWait)
defer cancel()
stateDir := t.TempDir()
//nolint:gosec // G204: test executes the freshly built secret binary
cmd := exec.CommandContext(ctx, secretBinaryPath(t), "init")
cmd.Env = []string{
secret.EnvStateDir + "=" + stateDir,
secret.EnvUnlockPassphrase + "=" + testPassphrase,
"PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"),
}
ptmx, tty, err := pty.Open()
require.NoError(t, err)
defer func() { _ = ptmx.Close() }()
deadline, _ := ctx.Deadline()
require.NoError(t, ptmx.SetReadDeadline(deadline))
var stderr strings.Builder
cmd.Stderr = &stderr
cmd.ExtraFiles = []*os.File{tty}
// Ctty is a descriptor in the new process: 3, the first of ExtraFiles.
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 3}
require.NoError(t, cmd.Start())
_ = tty.Close()
var (
shown []byte
char byte
)
terminal := bufio.NewReader(ptmx)
for !bytes.HasSuffix(shown, []byte("Enter your BIP39 mnemonic phrase: ")) {
char, err = terminal.ReadByte()
require.NoError(t, err, "secret init did not ask on the terminal: %s",
shown)
shown = append(shown, char)
}
_, err = ptmx.WriteString(testMnemonic + "\n")
require.NoError(t, err)
// The read ends once the command has exited and so closed the terminal.
rest, _ := io.ReadAll(terminal)
err = cmd.Wait()
require.NoError(t, err, "secret init: %s%s%s", shown, rest, stderr.String())
assert.True(t, strings.HasPrefix(stderr.String(),
"Initialized secrets manager at: "+stateDir+"\n\n"+
"Default vault created and configured\n"),
"stderr: %q", stderr.String())
}
// runAtTerminal runs the secret binary with args and input on stdin, in a
// new session whose controlling terminal is a new terminal, which is also
// its stderr: a command typed at a terminal with its input piped. pty.Open
// is described above TestRemoveIgnoresTerminalOnStdout. runAtTerminal waits
// for the passphrase prompt on the terminal, types the passphrase there,
// and returns what the command wrote to stdout once it has succeeded.
func runAtTerminal(t *testing.T, stateDir, input string, args ...string) string {
t.Helper()
ctx, cancel := context.WithTimeout(t.Context(), commandWait)
defer cancel()
//nolint:gosec // G204: test executes the freshly built secret binary
cmd := exec.CommandContext(ctx, secretBinaryPath(t), args...)
cmd.Env = []string{
secret.EnvStateDir + "=" + stateDir,
"PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"),
}
ptmx, tty, err := pty.Open()
require.NoError(t, err)
defer func() { _ = ptmx.Close() }()
deadline, _ := ctx.Deadline()
require.NoError(t, ptmx.SetReadDeadline(deadline))
var stdout strings.Builder
cmd.Stdin = strings.NewReader(input)
cmd.Stdout = &stdout
cmd.Stderr = tty
// Ctty is a descriptor in the new process: 2, its stderr.
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 2}
require.NoError(t, cmd.Start())
_ = tty.Close()
var (
shown []byte
char byte
)
terminal := bufio.NewReader(ptmx)
for !bytes.HasSuffix(shown, []byte("Enter unlock passphrase: ")) {
char, err = terminal.ReadByte()
require.NoError(t, err, "secret %v did not ask on the terminal: %s",
args, shown)
shown = append(shown, char)
}
_, err = ptmx.WriteString(testPassphrase + "\n")
require.NoError(t, err)
// The read ends once the command has exited and so closed the terminal.
rest, _ := io.ReadAll(terminal)
require.NoError(t, cmd.Wait(), "secret %v: %s%s", args, shown, rest)
return stdout.String()
}
+21 -8
View File
@@ -15,16 +15,29 @@ import (
// but exit with the code. // but exit with the code.
func Entry() int { func Entry() int {
// On SIGINT or SIGTERM memguard runs this function, wipes every buffer // On SIGINT or SIGTERM memguard runs this function, wipes every buffer
// and exits with status 1. The passphrase prompt turns terminal echo // and exits with status 1. The passphrase prompt turns echo off on the
// off until the read finishes, so a signal there would leave echo off. // controlling terminal until the read finishes, so a signal there would
// Only a process in the terminal's foreground process group may reset // leave echo off. Only a process in the terminal's foreground process
// it: one in the background that tries is stopped instead of exiting. // group may reset it: one in the background that tries is stopped
terminalState, terminalErr := term.GetState(unix.Stdin) // instead of exiting.
terminal, terminalErr := os.Open(secret.TerminalPath)
var terminalState *term.State
if terminalErr == nil {
defer func() { _ = terminal.Close() }()
terminalState, terminalErr = term.GetState(int(terminal.Fd()))
}
memguard.CatchSignal(func(os.Signal) { memguard.CatchSignal(func(os.Signal) {
foreground, err := unix.IoctlGetInt(unix.Stdin, unix.TIOCGPGRP) if terminalErr != nil {
if terminalErr == nil && err == nil && foreground == unix.Getpgrp() { return
_ = term.Restore(unix.Stdin, terminalState) }
foreground, err := unix.IoctlGetInt(int(terminal.Fd()), unix.TIOCGPGRP)
if err == nil && foreground == unix.Getpgrp() {
_ = term.Restore(int(terminal.Fd()), terminalState)
} }
}, os.Interrupt, unix.SIGTERM) }, os.Interrupt, unix.SIGTERM)
+6 -4
View File
@@ -16,6 +16,7 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"syscall"
"testing" "testing"
"filippo.io/age" "filippo.io/age"
@@ -223,7 +224,7 @@ func TestMoveUnlockFailureNamesVault(t *testing.T) {
} }
// TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built // TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built
// binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a // binary without SB_UNLOCK_PASSPHRASE and in a new session, which has no
// terminal, so the passphrase cannot be read. The unlocker was not tried, // terminal, so the passphrase cannot be read. The unlocker was not tried,
// and adding one would need a passphrase read the same way, so the error // and adding one would need a passphrase read the same way, so the error
// is the cause alone, without the advice to use the mnemonic. // is the cause alone, without the advice to use the mnemonic.
@@ -254,15 +255,16 @@ func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) {
"PATH=" + os.Getenv("PATH"), "PATH=" + os.Getenv("PATH"),
"HOME=" + os.Getenv("HOME"), "HOME=" + os.Getenv("HOME"),
} }
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
output, err := cmd.CombinedOutput() output, err := cmd.CombinedOutput()
require.Error(t, err) require.Error(t, err)
assert.Equal(t, "Error: failed to unlock vault: "+ assert.Equal(t, "Error: failed to unlock vault: "+
"failed to get long-term key: failed to get unlocker identity: "+ "failed to get long-term key: failed to get unlocker identity: "+
"failed to read passphrase: stdin is not a terminal (piped input or "+ "failed to read passphrase: there is no terminal to ask on. Please "+
"script). Please set the SB_UNLOCK_PASSPHRASE environment variable or "+ "set the SB_UNLOCK_PASSPHRASE environment variable or run "+
"run interactively\n", string(output)) "interactively\n", string(output))
} }
// TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and // TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and
+4
View File
@@ -17,6 +17,10 @@ const (
// EnvGPGKeyID is the environment variable for providing the GPG key ID // EnvGPGKeyID is the environment variable for providing the GPG key ID
EnvGPGKeyID = "SB_GPG_KEY_ID" EnvGPGKeyID = "SB_GPG_KEY_ID"
// TerminalPath opens the controlling terminal of the process that opens
// it, where the passphrase and mnemonic prompts ask.
TerminalPath = "/dev/tty"
// UnlockerTimeFormat is the layout of the time, in UTC, in the name of a // UnlockerTimeFormat is the layout of the time, in UTC, in the name of a
// new unlocker's directory. It runs to the nanosecond, so that every new // new unlocker's directory. It runs to the nanosecond, so that every new
// unlocker, even one added right after another, gets a directory of its // unlocker, even one added right after another, gets a directory of its
+17 -26
View File
@@ -6,7 +6,6 @@ import (
"fmt" "fmt"
"io" "io"
"os" "os"
"syscall"
"unsafe" "unsafe"
"filippo.io/age" "filippo.io/age"
@@ -16,11 +15,8 @@ import (
var ( var (
errNilPassphraseBuffer = errors.New("passphrase buffer is nil") errNilPassphraseBuffer = errors.New("passphrase buffer is nil")
errStdinNotTerminal = errors.New( errNoTerminal = errors.New("there is no terminal to ask on")
"stdin is not a terminal (piped input or script)") errNothingEntered = errors.New("nothing was entered")
errStderrNotTerminal = errors.New(
"stderr is not a terminal (running in non-interactive mode)")
errNothingEntered = errors.New("nothing was entered")
) )
// ErrMnemonicNotRead is wrapped in every error of ReadMnemonic: there is no // ErrMnemonicNotRead is wrapped in every error of ReadMnemonic: there is no
@@ -193,38 +189,33 @@ func ReadMnemonic(prompt string) (*memguard.LockedBuffer, error) {
return readFromTerminal(prompt, ErrMnemonicNotRead, EnvMnemonic) return readFromTerminal(prompt, ErrMnemonicNotRead, EnvMnemonic)
} }
// readFromTerminal reads input from the terminal without echoing it. Every // readFromTerminal asks on the controlling terminal and reads the answer
// error it returns wraps notRead; without a terminal, the error says to set // there without echoing it, whatever stdin is: in
// envVar instead. // `echo value | secret add name --force`, stdin is a pipe. Every error it
// returns wraps notRead; without a terminal, the error says to set envVar
// instead.
func readFromTerminal( func readFromTerminal(
prompt string, notRead error, envVar string, prompt string, notRead error, envVar string,
) (*memguard.LockedBuffer, error) { ) (*memguard.LockedBuffer, error) {
// Check if stdin is a terminal terminal, err := os.OpenFile(TerminalPath, os.O_RDWR, 0)
if !term.IsTerminal(syscall.Stdin) { if err != nil {
// Not a terminal - never read secrets from piped input Debug("Failed to open the controlling terminal", "error", err)
// for security reasons
return nil, fmt.Errorf( return nil, fmt.Errorf(
"%w: %w. Please set the %s environment variable or run interactively", "%w: %w. Please set the %s environment variable or run interactively",
notRead, errStdinNotTerminal, envVar) notRead, errNoTerminal, envVar)
} }
defer func() { _ = terminal.Close() }()
// stdin is a terminal, check if stderr is also a terminal for _, _ = fmt.Fprint(terminal, prompt)
// interactive prompting
if !term.IsTerminal(syscall.Stderr) {
return nil, fmt.Errorf("%w: %w. Please set the %s environment variable",
notRead, errStderrNotTerminal, envVar)
}
// Both stdin and stderr are terminals - use secure password reading input, err := term.ReadPassword(int(terminal.Fd()))
fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout
input, err := term.ReadPassword(syscall.Stdin)
if err != nil { if err != nil {
return nil, fmt.Errorf("%w: %w", notRead, err) return nil, fmt.Errorf("%w: %w", notRead, err)
} }
// Print newline to stderr since ReadPassword doesn't echo // The newline typed after the input, which ReadPassword did not echo
fmt.Fprintln(os.Stderr) _, _ = fmt.Fprintln(terminal)
if len(input) == 0 { if len(input) == 0 {
return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered) return nil, fmt.Errorf("%w: %w", notRead, errNothingEntered)
+39 -12
View File
@@ -219,6 +219,15 @@ func (v *Vault) addVersion(
name, secretDir string, value *memguard.LockedBuffer, name, secretDir string, value *memguard.LockedBuffer,
previousVersion *secret.Version, previousVersion *secret.Version,
) error { ) error {
// Unlock the vault before writing anything, so that a wrong passphrase
// or mnemonic leaves the secret as it was
err := v.loadPreviousVersionMetadata(previousVersion)
if err != nil {
return err
}
// Taken after the unlock, which can wait for the user to type the
// passphrase, so that the new version starts when it is created
now := time.Now() now := time.Now()
// Create the new version and save the encrypted value // Create the new version and save the encrypted value
@@ -632,8 +641,35 @@ func (v *Vault) checkExistingSecret(
return true, previousVersion, nil return true, previousVersion, nil
} }
// loadPreviousVersionMetadata unlocks the vault and loads the metadata of
// the version being superseded. It is a no-op when previousVersion is nil.
func (v *Vault) loadPreviousVersionMetadata(
previousVersion *secret.Version,
) error {
if previousVersion == nil {
return nil
}
ltIdentity, err := v.GetOrDeriveLongTermKey()
if err != nil {
secret.Debug("Failed to get long-term key for previous version", "error", err)
return fmt.Errorf("failed to get long-term key: %w", err)
}
err = previousVersion.LoadMetadata(ltIdentity)
if err != nil {
secret.Debug("Failed to load previous version metadata", "error", err)
return fmt.Errorf("failed to load previous version metadata: %w", err)
}
return nil
}
// updatePreviousVersion sets the notAfter timestamp on the version being // updatePreviousVersion sets the notAfter timestamp on the version being
// superseded. It is a no-op when previousVersion is nil. // superseded, whose metadata loadPreviousVersionMetadata has loaded. It is
// a no-op when previousVersion is nil.
func (v *Vault) updatePreviousVersion( func (v *Vault) updatePreviousVersion(
previousVersion *secret.Version, now *time.Time, previousVersion *secret.Version, now *time.Time,
) error { ) error {
@@ -641,22 +677,13 @@ func (v *Vault) updatePreviousVersion(
return nil return nil
} }
// Get long-term key to decrypt/encrypt metadata // loadPreviousVersionMetadata unlocked the vault, so this returns the
// key it keeps in memory
ltIdentity, err := v.GetOrDeriveLongTermKey() ltIdentity, err := v.GetOrDeriveLongTermKey()
if err != nil { if err != nil {
secret.Debug("Failed to get long-term key for metadata update", "error", err)
return fmt.Errorf("failed to get long-term key: %w", err) return fmt.Errorf("failed to get long-term key: %w", err)
} }
// Load previous version metadata
err = previousVersion.LoadMetadata(ltIdentity)
if err != nil {
secret.Debug("Failed to load previous version metadata", "error", err)
return fmt.Errorf("failed to load previous version metadata: %w", err)
}
// Update notAfter timestamp // Update notAfter timestamp
previousVersion.Metadata.NotAfter = now previousVersion.Metadata.NotAfter = now
+66
View File
@@ -10,6 +10,8 @@
// version // version
// - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter) // - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter)
// across versions // across versions
// - TestVaultVersionTimestampsAfterUnlock: Tests that those timestamps are
// taken after AddSecret unlocks the vault
// - TestVaultGetNonExistentVersion: Tests error handling for invalid versions // - TestVaultGetNonExistentVersion: Tests error handling for invalid versions
// - TestUpdateVersionMetadata: Tests metadata update functionality // - TestUpdateVersionMetadata: Tests metadata update functionality
// //
@@ -295,6 +297,70 @@ func TestVaultVersionTimestamps(t *testing.T) {
assert.Nil(t, secondVersion.Metadata.NotAfter) // Current version assert.Nil(t, secondVersion.Metadata.NotAfter) // Current version
} }
// longTermKeyReadFs records when longterm.age, the last file an unlock
// reads, was last opened.
type longTermKeyReadFs struct {
afero.Fs
readAt time.Time
}
//nolint:ireturn // afero.File is the interface required by afero.Fs
func (f *longTermKeyReadFs) Open(name string) (afero.File, error) {
if filepath.Base(name) == "longterm.age" {
f.readAt = time.Now()
}
return f.Fs.Open(name)
}
// TestVaultVersionTimestampsAfterUnlock checks that when AddSecret has to
// unlock the vault to replace a version, the new version's notBefore and the
// replaced version's notAfter are taken after the unlock, which can wait for
// the user to type the passphrase.
func TestVaultVersionTimestampsAfterUnlock(t *testing.T) {
t.Parallel()
fs := &longTermKeyReadFs{Fs: afero.NewMemMapFs()}
mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
defer mnemonic.Destroy()
passphrase := memguard.NewBufferFromBytes([]byte("test-passphrase"))
defer passphrase.Destroy()
created, err := CreateVault(fs, testStateDir, "test", mnemonic, passphrase)
require.NoError(t, err)
addTestSecretToVault(t, created, testSecretPath, []byte("version-1"), false)
// A new instance is locked; AddSecret unlocks it with the passphrase
vault := NewVault(fs, testStateDir, "test")
vault.SetUnlockPassphrase(passphrase)
addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)
require.False(t, fs.readAt.IsZero(), "AddSecret did not unlock the vault")
ltIdentity, err := vault.GetOrDeriveLongTermKey()
require.NoError(t, err)
vaultDir, _ := vault.GetDirectory()
secretDir := filepath.Join(vaultDir, "secrets.d", "test%secret")
versions, err := secret.ListVersions(fs, secretDir)
require.NoError(t, err)
require.Len(t, versions, 2)
newVersion := secret.NewVersion(vault, testSecretPath, versions[0])
require.NoError(t, newVersion.LoadMetadata(ltIdentity))
replacedVersion := secret.NewVersion(vault, testSecretPath, versions[1])
require.NoError(t, replacedVersion.LoadMetadata(ltIdentity))
assert.False(t, newVersion.Metadata.NotBefore.Before(fs.readAt))
assert.False(t, replacedVersion.Metadata.NotAfter.Before(fs.readAt))
}
func TestVaultGetNonExistentVersion(t *testing.T) { func TestVaultGetNonExistentVersion(t *testing.T) {
t.Parallel() t.Parallel()
+76
View File
@@ -5,6 +5,7 @@ import (
"path/filepath" "path/filepath"
"testing" "testing"
"filippo.io/age"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -103,3 +104,78 @@ func TestAddSecretCleansUpOnFailure(t *testing.T) {
require.Len(t, entries, 1) require.Len(t, entries, 1)
assert.Equal(t, "pub.age", entries[0].Name()) assert.Equal(t, "pub.age", entries[0].Name())
} }
// TestAddSecretForceWithWrongPassphraseChangesNothing checks that adding a
// version to an existing secret, with a passphrase that does not open the
// vault, fails and leaves the secret's directory as it was.
func TestAddSecretForceWithWrongPassphraseChangesNothing(t *testing.T) {
t.Parallel()
fs := afero.NewMemMapFs()
passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
t.Cleanup(passphrase.Destroy)
created, err := vault.CreateVault(fs, testStateDir, testVaultName,
testMnemonicBuffer(t), passphrase)
require.NoError(t, err)
first := memguard.NewBufferFromBytes([]byte("first"))
t.Cleanup(first.Destroy)
require.NoError(t, created.AddSecret(testSecretName, first, false))
wrong := memguard.NewBufferFromBytes([]byte("wrong passphrase"))
t.Cleanup(wrong.Destroy)
vlt := vault.NewVault(fs, testStateDir, testVaultName)
vlt.SetUnlockPassphrase(wrong)
secretDir := filepath.Join(testStateDir, "vaults.d", testVaultName,
"secrets.d", testSecretName)
before := dirContents(t, fs, secretDir)
second := memguard.NewBufferFromBytes([]byte("second"))
t.Cleanup(second.Destroy)
err = vlt.AddSecret(testSecretName, second, true)
var noMatch *age.NoIdentityMatchError
require.ErrorAs(t, err, &noMatch)
assert.Equal(t, before, dirContents(t, fs, secretDir))
}
// dirContents maps every file under dir to its contents, and every
// directory, written with a trailing "/", to "".
func dirContents(t *testing.T, fs afero.Fs, dir string) map[string]string {
t.Helper()
contents := map[string]string{}
err := afero.Walk(fs, dir, func(
path string, info os.FileInfo, err error,
) error {
if err != nil {
return err
}
if info.IsDir() {
contents[path+"/"] = ""
return nil
}
content, err := afero.ReadFile(fs, path)
if err != nil {
return err
}
contents[path] = string(content)
return nil
})
require.NoError(t, err)
return contents
}