2 Commits
Author SHA1 Message Date
sneak a8282ccd8b Make secret unlocker add pgp work on Linux (closes #88)
check / check (push) Waiting to run
CreatePGPUnlocker got the vault's long-term key from the keychain
unlocker's helper, which on every platform but macOS is a stub that
always fails. It now calls the vault's GetOrDeriveLongTermKey, as adding
a passphrase unlocker does: from the mnemonic, checked against the
vault, or else from the current unlocker. That method joins
VaultInterface. The test GPG key gains an encryption subkey, and a new
test adds a PGP unlocker with the long-term key from the mnemonic and
from a passphrase unlocker, then reads a secret through it.

Model: opus-5-5
2026-10-04 12:26:07 +00:00
clawbot 007254a1f0 Check vault names in every command that takes one (closes #68)
check / check (push) Waiting to run
A vault name may use only lowercase ASCII letters, digits, `.`, `-` and
`_`, and must not be empty, `.` or `..`; the error now states that rule.
`vault create`, `vault import`, `vault select`, `vault remove`, both
vault names of `mv` and shell completion of a `vault:secret` argument
check the name as typed before building any path from it. Before,
`vault import ..` wrote a long-term key and an unlocker into the state
directory itself, and `vault select ..` made that the current vault.

Model: opus-5-5
2026-10-04 13:58:46 +02:00
10 changed files with 172 additions and 34 deletions
+3
View File
@@ -91,6 +91,9 @@ Lists all available vaults. The current vault is marked.
Creates a new vault with the specified name.
**Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
are allowed, and a name must not be empty, `.` or `..`.
#### `secret vault select <name>`
Switches to the specified vault for subsequent operations.
+9
View File
@@ -34,6 +34,15 @@ Bring the repo into policy compliance in one commit:
but macOS always failed. A test adds a PGP unlocker for a throwaway GPG
key, getting the long-term key once from the mnemonic and once from a
passphrase unlocker, and reads a secret through the new unlocker.
- 2026-10-04: A vault name may use only lowercase ASCII letters, digits,
`.`, `-` and `_`, and must not be empty, `.` or `..`
(https://git.eeqj.de/sneak/secret/issues/68); the error and `README.md`
state the rule. `vault create`, `vault import`, `vault select`,
`vault remove`, both vault names of `mv` and shell completion of a
`vault:secret` argument check the name as typed with
`vault.ValidateVaultName` before building any path from it. Before,
`vault import ..` wrote a long-term key and an unlocker into the state
directory itself, and `vault select ..` made that the current vault.
- 2026-10-04: `script/cibuild` runs the checks again on an unchanged
tree (https://git.eeqj.de/sneak/secret/issues/54). It passes the
current time as the `CHECK_EPOCH` build argument, which both the lint
+7 -1
View File
@@ -123,7 +123,9 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
}
// completeVaultQualifiedSecrets completes "vault:secret" references once a
// colon is present in the input
// colon is present in the input. It completes nothing when the vault part
// is not a valid vault name, so that a name such as ".." cannot list a
// directory outside vaults.d.
func completeVaultQualifiedSecrets(
fs afero.Fs, stateDir, toComplete string,
) []string {
@@ -134,6 +136,10 @@ func completeVaultQualifiedSecrets(
vaultName := parts[0]
secretPrefix := parts[1]
if vault.ValidateVaultName(vaultName) != nil {
return nil
}
vlt := vault.NewVault(fs, stateDir, vaultName)
secrets, err := vlt.ListSecrets()
+41
View File
@@ -0,0 +1,41 @@
//nolint:testpackage // white-box test of unexported internals
package cli
import (
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestVaultSecretCompletionRejectsInvalidVaultName is a regression test for
// https://git.eeqj.de/sneak/secret/issues/68: completing a `vault:secret`
// argument lists nothing when the vault part is not a valid vault name, even
// where that name, joined onto vaults.d, leads to a secrets.d directory.
func TestVaultSecretCompletionRejectsInvalidVaultName(t *testing.T) {
t.Parallel()
const (
stateDir = "/state"
dirPerm = 0o700
)
fs := afero.NewMemMapFs()
// The vault "work" holds the secret "x". So does every directory an
// invalid name below would lead to from vaults.d.
for _, vaultName := range []string{"work", ".", "..", "a/b"} {
secretDir := filepath.Join(stateDir, "vaults.d", vaultName, "secrets.d", "x")
require.NoError(t, fs.MkdirAll(secretDir, dirPerm))
}
assert.Equal(t, []string{"work:x"},
completeVaultQualifiedSecrets(fs, stateDir, "work:"))
for _, toComplete := range []string{".:", "..:", "a/b:"} {
assert.Empty(t, completeVaultQualifiedSecrets(fs, stateDir, toComplete),
"completing %q", toComplete)
}
}
+9 -10
View File
@@ -48,26 +48,25 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
"mv work:nosuch work:y", "work:nosuch", "work:y", false,
"secret 'nosuch' not found",
},
// Only an existing vault is used, so ".." cannot reach the state
// directory itself.
// Only an existing vault is used.
{
"mv --force ..:x ..:y", "..:x", "..:y", true,
"vault '..' does not exist",
"mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true,
"vault 'nosuch' does not exist",
},
// Each of these spells "work" a second way. The spelling is not an
// existing vault name, so the move is not taken for a move between
// two vaults, which would delete the destination, here the source.
// Each of these spells "work" a second way. The spelling is not a
// valid vault name, so the move is not taken for a move between two
// vaults, which would delete the destination, here the source.
{
"mv --force work:x work/:x", workX, "work/:x", true,
"vault 'work/' does not exist",
vault.ValidateVaultName("work/").Error(),
},
{
"mv --force work/:x work:", "work/:x", "work:", true,
"vault 'work/' does not exist",
vault.ValidateVaultName("work/").Error(),
},
{
"mv --force work:x ./work:x", workX, "./work:x", true,
"vault './work' does not exist",
vault.ValidateVaultName("./work").Error(),
},
}
+52
View File
@@ -292,6 +292,58 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
}
}
// TestInvalidVaultNameLeavesStateUnchanged is a regression test for
// https://git.eeqj.de/sneak/secret/issues/68, where
// `secret vault import ..` wrote a long-term key and an unlocker into the
// state directory itself, and `secret vault select ..` made it the current
// vault. Each command that takes a vault name must reject an invalid one
// before building a path from it. The mnemonic and the passphrase are set,
// and moves and removals use --force, so that only the name check stands
// in the way.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) {
before := snapshotStateDir(t, newTwoVaultFs(t))
t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
cmd := &cobra.Command{}
// Each command is a format with %q where the vault name goes.
commands := []struct {
command string
run func(c *cli.Instance, name string) error
}{
{"vault create %q", func(c *cli.Instance, name string) error {
return c.CreateVault(cmd, name)
}},
{"vault import %q", func(c *cli.Instance, name string) error {
return c.VaultImport(cmd, name)
}},
{"vault select %q", func(c *cli.Instance, name string) error {
return c.SelectVault(cmd, name)
}},
{"vault remove --force %q", func(c *cli.Instance, name string) error {
return c.RemoveVault(cmd, name, true)
}},
{"mv --force %q:x work:x", func(c *cli.Instance, name string) error {
return c.MoveSecret(cmd, name+":x", "work:x", true)
}},
{"mv --force default:x %q:x", func(c *cli.Instance, name string) error {
return c.MoveSecret(cmd, "default:x", name+":x", true)
}},
}
for _, tt := range commands {
for _, name := range []string{"", ".", "..", "a/b"} {
t.Run(fmt.Sprintf(tt.command, name), func(t *testing.T) {
requireRejectedAndUnchanged(t, before, vault.ValidateVaultName(name),
func(c *cli.Instance) error { return tt.run(c, name) })
})
}
}
}
// TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
// with a version that is not the current one removes that version and
// changes nothing else.
+11 -5
View File
@@ -811,9 +811,9 @@ func (cli *Instance) moveSecret(
cmd, vlt, srcSecretName, destSecretName, force)
}
// Both vaults must be existing vaults by exact name, so that two
// spellings of one vault, such as "work" and "work/", are never taken for
// two vaults. A named vault does not become the current vault.
// Both vault names must be valid and name existing vaults exactly, so
// that two spellings of one vault, such as "work" and "work/", are never
// taken for two vaults. A named vault does not become the current vault.
srcVault, err := cli.existingVault(srcVaultName)
if err != nil {
return err
@@ -833,9 +833,15 @@ func (cli *Instance) moveSecret(
cmd, srcVault, srcSecretName, destVault, destSecretName, force)
}
// existingVault returns the vault with the given name, or an error if there
// is none. Unlike vault.SelectVault, it leaves the current vault as it is.
// existingVault returns the vault with the given name, or an error if the
// name is not a valid vault name or there is no such vault. Unlike
// vault.SelectVault, it leaves the current vault as it is.
func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
err := vault.ValidateVaultName(name)
if err != nil {
return nil, err
}
vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
if err != nil {
return nil, fmt.Errorf("failed to list vaults: %w", err)
+10
View File
@@ -462,6 +462,11 @@ func updateVaultImportMetadata(
// VaultImport imports a mnemonic into a specific vault, holding the state
// directory lock while importMnemonic runs
func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
err := vault.ValidateVaultName(vaultName)
if err != nil {
return err
}
release, err := vault.LockStateDir(cli.fs, cli.stateDir)
if err != nil {
return err
@@ -617,6 +622,11 @@ func (cli *Instance) switchAwayFromVault(
// RemoveVault removes a vault, holding the state directory lock while
// removeVault runs
func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
err := vault.ValidateVaultName(name)
if err != nil {
return err
}
release, err := vault.LockStateDir(cli.fs, cli.stateDir)
if err != nil {
return err
+4 -3
View File
@@ -17,9 +17,10 @@ var (
"derived public key does not match vault: mnemonic may be incorrect",
)
// ErrInvalidVaultName indicates a vault name that does not match the
// allowed pattern [a-z0-9.\-_]+. Composed as
// "invalid vault name '<name>': must match pattern [a-z0-9.\-_]+".
// ErrInvalidVaultName indicates a vault name that breaks the naming
// rule: only lowercase ASCII letters, digits, '.', '-' and '_'; not
// empty, "." or "..". Composed by ValidateVaultName as
// "invalid vault name '<name>': <the rule>".
ErrInvalidVaultName = errors.New("invalid vault name")
// ErrVaultNotFound indicates the named vault does not exist. Composed
+26 -15
View File
@@ -24,10 +24,12 @@ func init() {
})
}
// isValidVaultName validates vault names according to the format [a-z0-9\.\-\_]+
// Note: We don't allow slashes in vault names unlike secret names
// isValidVaultName reports whether name is a valid vault name: only
// lowercase ASCII letters, digits, '.', '-' and '_', and not empty, "." or
// "..". With no path separator allowed, a vault is always one directory
// directly under vaults.d.
func isValidVaultName(name string) bool {
if name == "" {
if name == "" || name == "." || name == ".." {
return false
}
@@ -36,6 +38,21 @@ func isValidVaultName(name string) bool {
return matched
}
// ValidateVaultName returns an error wrapping ErrInvalidVaultName when name
// is not a valid vault name. Call it on the name exactly as the user gave it,
// before building any path from it.
func ValidateVaultName(name string) error {
if !isValidVaultName(name) {
return fmt.Errorf(
"%w '%s': only lowercase ASCII letters, digits, '.', '-' and '_' "+
"are allowed, and a name must not be empty, '.' or '..'",
ErrInvalidVaultName, name,
)
}
return nil
}
// ResolveVaultSymlink reads the currentvault file to get the path to the current vault
// The file contains just the vault name (e.g., "default")
func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
@@ -199,14 +216,11 @@ func processMnemonicForVault(
func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
// Validate vault name
if !isValidVaultName(name) {
err := ValidateVaultName(name)
if err != nil {
secret.Debug("Invalid vault name provided", "vault_name", name)
return nil, fmt.Errorf(
"%w '%s': must match pattern [a-z0-9.\\-_]+",
ErrInvalidVaultName, name,
)
return nil, err
}
secret.Debug("Vault name validation passed", "vault_name", name)
@@ -285,14 +299,11 @@ func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
func SelectVault(fs afero.Fs, stateDir string, name string) error {
secret.Debug("Selecting vault", "vault_name", name, "state_dir", stateDir)
// Validate vault name
if !isValidVaultName(name) {
err := ValidateVaultName(name)
if err != nil {
secret.Debug("Invalid vault name provided", "vault_name", name)
return fmt.Errorf(
"%w '%s': must match pattern [a-z0-9.\\-_]+",
ErrInvalidVaultName, name,
)
return err
}
secret.Debug("Vault name validation passed", "vault_name", name)