Compare commits
3
Commits
5cfb9c5f46
...
97d039f1a4
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
97d039f1a4 | ||
|
|
4e562f834f | ||
|
|
641d5659ec |
+2
-1
@@ -9,7 +9,8 @@ RUN go mod download
|
|||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
RUN make fmt-check
|
RUN make fmt-check
|
||||||
RUN make lint
|
# Not make lint: script/lint is a docker build, which cannot run in here.
|
||||||
|
RUN golangci-lint run --config .golangci.yml ./...
|
||||||
|
|
||||||
# Build stage — tests and compilation
|
# Build stage — tests and compilation
|
||||||
# golang 1.24.13-alpine (2026-03-10)
|
# golang 1.24.13-alpine (2026-03-10)
|
||||||
|
|||||||
@@ -0,0 +1,19 @@
|
|||||||
|
# Lint image, built by script/lint: golangci-lint runs as a build step, so a
|
||||||
|
# successful build is a clean lint. Works where the docker daemon is remote
|
||||||
|
# and bind mounts are impossible.
|
||||||
|
|
||||||
|
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
|
||||||
|
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps
|
||||||
|
|
||||||
|
WORKDIR /src
|
||||||
|
|
||||||
|
COPY go.mod go.sum ./
|
||||||
|
RUN go mod download
|
||||||
|
|
||||||
|
# script/lint rebuilds this stage on every run, by this name; the module
|
||||||
|
# download above stays cached.
|
||||||
|
FROM deps AS lint
|
||||||
|
|
||||||
|
COPY . .
|
||||||
|
|
||||||
|
RUN golangci-lint run --config .golangci.yml ./...
|
||||||
@@ -139,6 +139,9 @@ matching.
|
|||||||
|
|
||||||
Moves or renames a secret within the current vault.
|
Moves or renames a secret within the current vault.
|
||||||
- Fails if the destination already exists
|
- Fails if the destination already exists
|
||||||
|
- Fails if the destination is the source under another name, such as `foo`
|
||||||
|
for `Foo` on a case-insensitive filesystem (the macOS default); there, to
|
||||||
|
change only the case of a name, move the secret to a third name first
|
||||||
- Preserves all versions and metadata
|
- Preserves all versions and metadata
|
||||||
|
|
||||||
### Version Management
|
### Version Management
|
||||||
@@ -496,15 +499,18 @@ standard: normalized scripts in `script/` are the entrypoints for the
|
|||||||
development workflow, and the Makefile targets are thin shims that call
|
development workflow, and the Makefile targets are thin shims that call
|
||||||
them. We provide:
|
them. We provide:
|
||||||
|
|
||||||
- `script/bootstrap` — install all dependencies (Go, golangci-lint, Go
|
- `script/bootstrap` — install all dependencies (Go, Go module
|
||||||
module download), idempotently
|
download), idempotently; golangci-lint is not installed, it runs in
|
||||||
|
docker
|
||||||
- `script/setup` — make a fresh clone ready for development: runs
|
- `script/setup` — make a fresh clone ready for development: runs
|
||||||
`script/bootstrap`, then `script/install-precommit`
|
`script/bootstrap`, then `script/install-precommit`
|
||||||
- `script/projectname` — output the project name (`secret`); used by
|
- `script/projectname` — output the project name (`secret`); used by
|
||||||
other scripts such as `script/docker`
|
other scripts such as `script/docker`
|
||||||
- `script/test` — run `go vet` and the test suite (verbose rerun on
|
- `script/test` — run `go vet` and the test suite (verbose rerun on
|
||||||
failure)
|
failure)
|
||||||
- `script/lint` — run `golangci-lint`
|
- `script/lint` — run `golangci-lint` in docker only: builds
|
||||||
|
`Dockerfile.lint`, where the linter is a build step that runs on every
|
||||||
|
call, also on an unchanged tree
|
||||||
- `script/fmt` — format all Go code (writes)
|
- `script/fmt` — format all Go code (writes)
|
||||||
- `script/fmt-check` — check formatting without writing
|
- `script/fmt-check` — check formatting without writing
|
||||||
- `script/check` — run `script/test`, `script/lint`, and
|
- `script/check` — run `script/test`, `script/lint`, and
|
||||||
|
|||||||
@@ -25,6 +25,26 @@ Bring the repo into policy compliance in one commit:
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-04: `secret mv` rejects a move whose destination is the source
|
||||||
|
under another name, such as `foo` for `Foo` on a case-insensitive
|
||||||
|
filesystem (the macOS default) or a name reached through a symbolic
|
||||||
|
link, before changing anything, with or without `--force`, within a
|
||||||
|
vault and between vaults; before, `--force` removed the destination and
|
||||||
|
so deleted the secret. A rename that changes only letter case works on a
|
||||||
|
case-sensitive filesystem as before.
|
||||||
|
- 2026-10-04: Lint runs only in docker: `script/lint` builds
|
||||||
|
`Dockerfile.lint`, where golangci-lint is a build step rebuilt on
|
||||||
|
every run (`--no-cache-filter`), so an unchanged tree is linted too;
|
||||||
|
the module download stays cached. `script/bootstrap` no longer
|
||||||
|
installs golangci-lint, and the `Dockerfile` lint stage calls it
|
||||||
|
directly instead of `make lint`. `golangci-lint config verify` is not
|
||||||
|
run: it fetches its schema live over unpinned HTTPS.
|
||||||
|
- 2026-10-04: A PGP unlocker whose metadata has no usable GPG key ID
|
||||||
|
no longer panics: `GetID()` warns with the unlocker's directory and
|
||||||
|
returns `pgp-unknown`. `ListUnlockers` skips, with a warning, an
|
||||||
|
unlocker whose metadata file cannot be checked for, read or parsed
|
||||||
|
instead of failing, so `secret unlocker list` still lists the others;
|
||||||
|
the listing's ID lookup no longer warns about that directory again.
|
||||||
- 2026-10-03: `secret mv` rejects a move whose destination is the
|
- 2026-10-03: `secret mv` rejects a move whose destination is the
|
||||||
source (`mv --force x x`, `mv --force work:x work:`, or an empty
|
source (`mv --force x x`, `mv --force work:x work:`, or an empty
|
||||||
destination, which defaults to the source name) before changing
|
destination, which defaults to the source name) before changing
|
||||||
@@ -160,8 +180,6 @@ Bring the repo into policy compliance in one commit:
|
|||||||
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
|
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
|
||||||
209-216); non-constant-time public key compare (vault.go:95-100).
|
209-216); non-constant-time public key compare (vault.go:95-100).
|
||||||
- High priority:
|
- High priority:
|
||||||
- Return errors instead of panicking on corrupted metadata
|
|
||||||
(pgpunlocker.go:116, keychainunlocker.go:141).
|
|
||||||
- Secure temporary file handling and cleanup.
|
- Secure temporary file handling and cleanup.
|
||||||
- Print cobra usage only for argument errors, not internal
|
- Print cobra usage only for argument errors, not internal
|
||||||
failures.
|
failures.
|
||||||
|
|||||||
@@ -1,9 +1,15 @@
|
|||||||
package cli_test
|
package cli_test
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"git.eeqj.de/sneak/secret/internal/cli"
|
"git.eeqj.de/sneak/secret/internal/cli"
|
||||||
|
"git.eeqj.de/sneak/secret/internal/secret"
|
||||||
|
"git.eeqj.de/sneak/secret/internal/vault"
|
||||||
|
"github.com/awnumar/memguard"
|
||||||
|
"github.com/spf13/afero"
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
@@ -97,3 +103,127 @@ func TestMoveWithinOtherVaultKeepsCurrentVault(t *testing.T) {
|
|||||||
require.Contains(t, after, workSecrets+"y/")
|
require.Contains(t, after, workSecrets+"y/")
|
||||||
require.NotContains(t, after, workSecrets+"x/")
|
require.NotContains(t, after, workSecrets+"x/")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestMoveOntoSameSecretUnderAnotherNameIsRejected is a regression test for
|
||||||
|
// https://git.eeqj.de/sneak/secret/issues/78: on a case-insensitive
|
||||||
|
// filesystem "Foo" and "foo" are one secret, and `secret mv --force Foo foo`
|
||||||
|
// removed the destination, which was the source. Symbolic links on the real
|
||||||
|
// filesystem give one secret two names here: in "default", "y" is a link to
|
||||||
|
// the secret "x", and the secrets.d of "other" is a link to that of
|
||||||
|
// "default", so other:x is default:x. Each move must be rejected and leave
|
||||||
|
// the secret and the links as they were.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // t.Setenv
|
||||||
|
func TestMoveOntoSameSecretUnderAnotherNameIsRejected(t *testing.T) {
|
||||||
|
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||||
|
|
||||||
|
const isSame = "is the same secret on this filesystem"
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
command string
|
||||||
|
source, dest string
|
||||||
|
force bool
|
||||||
|
wantErr string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
"mv --force y x", "y", "x", true,
|
||||||
|
"secret 'y' cannot be moved onto itself: 'x' " + isSame,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"mv --force x y", "x", "y", true,
|
||||||
|
"secret 'x' cannot be moved onto itself: 'y' " + isSame,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"mv x y", "x", "y", false,
|
||||||
|
"secret 'x' cannot be moved onto itself: 'y' " + isSame,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"mv --force default:x other:x", "default:x", "other:x", true,
|
||||||
|
"secret 'default:x' cannot be moved onto itself: 'other:x' " +
|
||||||
|
isSame,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"mv default:x other", "default:x", "other", false,
|
||||||
|
"secret 'default:x' cannot be moved onto itself: 'other:x' " +
|
||||||
|
isSame,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.command, func(t *testing.T) {
|
||||||
|
fs := afero.NewOsFs()
|
||||||
|
stateDir := t.TempDir()
|
||||||
|
vaultsDir := filepath.Join(stateDir, "vaults.d")
|
||||||
|
|
||||||
|
// "default" is created last, so it is the current vault.
|
||||||
|
_, err := vault.CreateVault(fs, stateDir, "other")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
vlt, err := vault.CreateVault(fs, stateDir, "default")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
defaultSecrets := filepath.Join(vaultsDir, "default", "secrets.d")
|
||||||
|
otherSecrets := filepath.Join(vaultsDir, "other", "secrets.d")
|
||||||
|
link := filepath.Join(defaultSecrets, "y")
|
||||||
|
|
||||||
|
require.NoError(t, os.Symlink("x", link))
|
||||||
|
require.NoError(t, os.Remove(otherSecrets))
|
||||||
|
require.NoError(t, os.Symlink(defaultSecrets, otherSecrets))
|
||||||
|
|
||||||
|
c := cli.NewCLIInstanceWithStateDir(fs, stateDir)
|
||||||
|
moveErr := c.MoveSecret(&cobra.Command{}, tt.source, tt.dest, tt.force)
|
||||||
|
|
||||||
|
value, err := vlt.GetSecret("x")
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, "value", string(value))
|
||||||
|
|
||||||
|
target, err := os.Readlink(link)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, "x", target)
|
||||||
|
|
||||||
|
target, err = os.Readlink(otherSecrets)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, defaultSecrets, target)
|
||||||
|
|
||||||
|
require.EqualError(t, moveErr, tt.wantErr)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem checks that where "Foo"
|
||||||
|
// and "foo" are two secrets, `secret mv --force Foo foo` still replaces "foo"
|
||||||
|
// with "Foo".
|
||||||
|
func TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem(t *testing.T) {
|
||||||
|
t.Setenv(secret.EnvMnemonic, testMnemonic)
|
||||||
|
|
||||||
|
fs := afero.NewOsFs()
|
||||||
|
stateDir := t.TempDir()
|
||||||
|
|
||||||
|
vlt, err := vault.CreateVault(fs, stateDir, "default")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
err = vlt.AddSecret("Foo", memguard.NewBufferFromBytes([]byte("upper")), false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
_, err = os.Stat(filepath.Join(stateDir, "vaults.d", "default", "secrets.d", "foo"))
|
||||||
|
if err == nil {
|
||||||
|
t.Skip("the temporary directory is on a case-insensitive filesystem")
|
||||||
|
}
|
||||||
|
|
||||||
|
err = vlt.AddSecret("foo", memguard.NewBufferFromBytes([]byte("lower")), false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
c := cli.NewCLIInstanceWithStateDir(fs, stateDir)
|
||||||
|
err = c.MoveSecret(&cobra.Command{}, "Foo", "foo", true)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
value, err := vlt.GetSecret("foo")
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, "upper", string(value))
|
||||||
|
|
||||||
|
_, err = vlt.GetSecret("Foo")
|
||||||
|
require.ErrorIs(t, err, vault.ErrSecretNotFound)
|
||||||
|
}
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"log"
|
"log"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"slices"
|
"slices"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -890,6 +891,18 @@ func (cli *Instance) moveSecretWithinVault(
|
|||||||
destEncoded := strings.ReplaceAll(dest, "/", "%")
|
destEncoded := strings.ReplaceAll(dest, "/", "%")
|
||||||
destDir := filepath.Join(vaultDir, "secrets.d", destEncoded)
|
destDir := filepath.Join(vaultDir, "secrets.d", destEncoded)
|
||||||
|
|
||||||
|
// Removing a destination that is the source under another name, such as
|
||||||
|
// "foo" for "Foo" on a case-insensitive filesystem, would delete it too.
|
||||||
|
same, err := cli.sameDirectory(sourceDir, destDir)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if same {
|
||||||
|
return fmt.Errorf("secret '%s' %w: '%s' is the same secret on "+
|
||||||
|
"this filesystem", source, errMoveOntoItself, dest)
|
||||||
|
}
|
||||||
|
|
||||||
exists, err = afero.DirExists(cli.fs, destDir)
|
exists, err = afero.DirExists(cli.fs, destDir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to check if destination secret exists: %w", err)
|
return fmt.Errorf("failed to check if destination secret exists: %w", err)
|
||||||
@@ -916,6 +929,31 @@ func (cli *Instance) moveSecretWithinVault(
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// sameDirectory reports whether the existing directory dir and the path
|
||||||
|
// other are one directory under two names, as secrets.d/Foo and
|
||||||
|
// secrets.d/foo are on a case-insensitive filesystem, or a directory and a
|
||||||
|
// symbolic link to it. Removing other to make room for dir would then delete
|
||||||
|
// dir. It is false if other does not exist, and always false on the
|
||||||
|
// in-memory filesystem, which has no such aliasing and whose files
|
||||||
|
// os.SameFile does not compare.
|
||||||
|
func (cli *Instance) sameDirectory(dir, other string) (bool, error) {
|
||||||
|
dirInfo, err := cli.fs.Stat(dir)
|
||||||
|
if err != nil {
|
||||||
|
return false, fmt.Errorf("failed to check %s: %w", dir, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
otherInfo, err := cli.fs.Stat(other)
|
||||||
|
if errors.Is(err, os.ErrNotExist) {
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
return false, fmt.Errorf("failed to check %s: %w", other, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return os.SameFile(dirInfo, otherInfo), nil
|
||||||
|
}
|
||||||
|
|
||||||
// moveSecretCrossVault handles moving between two different vaults. Its
|
// moveSecretCrossVault handles moving between two different vaults. Its
|
||||||
// caller, MoveSecret, has already checked both secret names and that both
|
// caller, MoveSecret, has already checked both secret names and that both
|
||||||
// vaults exist.
|
// vaults exist.
|
||||||
@@ -940,6 +978,27 @@ func (cli *Instance) moveSecretCrossVault(
|
|||||||
srcSecretName, errSecretNotFound, srcVault.Name)
|
srcSecretName, errSecretNotFound, srcVault.Name)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The source is removed after the copy, so a destination that is the
|
||||||
|
// source under another name would be lost with it.
|
||||||
|
destVaultDir, err := destVault.GetDirectory()
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to get destination vault directory: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
destStorageName := strings.ReplaceAll(destSecretName, "/", "%")
|
||||||
|
destSecretDir := filepath.Join(destVaultDir, "secrets.d", destStorageName)
|
||||||
|
|
||||||
|
same, err := cli.sameDirectory(srcSecretDir, destSecretDir)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if same {
|
||||||
|
return fmt.Errorf("secret '%s:%s' %w: '%s:%s' is the same secret on "+
|
||||||
|
"this filesystem", srcVault.Name, srcSecretName, errMoveOntoItself,
|
||||||
|
destVault.Name, destSecretName)
|
||||||
|
}
|
||||||
|
|
||||||
// Unlock destination vault (will fail if neither mnemonic nor unlocker available)
|
// Unlock destination vault (will fail if neither mnemonic nor unlocker available)
|
||||||
_, err = destVault.GetOrDeriveLongTermKey()
|
_, err = destVault.GetOrDeriveLongTermKey()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -349,6 +349,10 @@ func unlockerIDFromDir(
|
|||||||
// itself cannot be read. Callers must distinguish the two: an unreadable
|
// itself cannot be read. Callers must distinguish the two: an unreadable
|
||||||
// directory means the unlocker's real ID is unknowable, so the entry has
|
// directory means the unlocker's real ID is unknowable, so the entry has
|
||||||
// to be skipped rather than reported under a synthesized ID.
|
// to be skipped rather than reported under a synthesized ID.
|
||||||
|
//
|
||||||
|
// A metadata file that cannot be read or parsed is skipped without a
|
||||||
|
// warning: every caller gets metadata from vault.ListUnlockers first,
|
||||||
|
// which has already warned about that directory.
|
||||||
func findUnlockerIDByMetadata(
|
func findUnlockerIDByMetadata(
|
||||||
fs afero.Fs, unlockersDir string, metadata secret.UnlockerMetadata,
|
fs afero.Fs, unlockersDir string, metadata secret.UnlockerMetadata,
|
||||||
includeSecureEnclave bool,
|
includeSecureEnclave bool,
|
||||||
@@ -371,9 +375,6 @@ func findUnlockerIDByMetadata(
|
|||||||
// Check if this is the right unlocker by comparing metadata
|
// Check if this is the right unlocker by comparing metadata
|
||||||
metadataBytes, err := afero.ReadFile(fs, metadataPath)
|
metadataBytes, err := afero.ReadFile(fs, metadataPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
secret.Warn("Could not read unlocker metadata file",
|
|
||||||
"path", metadataPath, "error", err)
|
|
||||||
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -381,9 +382,6 @@ func findUnlockerIDByMetadata(
|
|||||||
|
|
||||||
err = json.Unmarshal(metadataBytes, &diskMetadata)
|
err = json.Unmarshal(metadataBytes, &diskMetadata)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
secret.Warn("Could not parse unlocker metadata file",
|
|
||||||
"path", metadataPath, "error", err)
|
|
||||||
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,13 +1,19 @@
|
|||||||
// Unlocker List Tests
|
// Unlocker List Tests
|
||||||
//
|
//
|
||||||
// Tests for `secret unlocker list` behavior when the unlockers.d directory
|
// Tests for `secret unlocker list` behavior when the unlockers.d directory,
|
||||||
// cannot be read while the listing is being rendered:
|
// or an unlocker's metadata in it, cannot be read while the listing is
|
||||||
|
// being rendered:
|
||||||
//
|
//
|
||||||
// - TestUnlockersListSkipsUnreadableUnlockersDir: an unreadable
|
// - TestUnlockersListSkipsUnreadableUnlockersDir: an unreadable
|
||||||
// unlockers.d yields no rows rather than rows bearing synthesized IDs.
|
// unlockers.d yields no rows rather than rows bearing synthesized IDs.
|
||||||
// - TestUnlockersListSkipsOnlyUnreadableEntries: a readable entry is
|
// - TestUnlockersListSkipsOnlyUnreadableEntries: a readable entry is
|
||||||
// still listed, with its real ID and its current-unlocker marker,
|
// still listed, with its real ID and its current-unlocker marker,
|
||||||
// when a later entry's scan fails.
|
// when a later entry's scan fails.
|
||||||
|
// - TestUnlockersListToleratesCorruptMetadata: one unlocker's corrupt
|
||||||
|
// metadata does not stop the others from being listed.
|
||||||
|
// - TestUnlockersListSkipsUnreadableMetadata: an unlocker whose metadata
|
||||||
|
// file cannot be checked for or read is left out, and the other is
|
||||||
|
// still listed.
|
||||||
//
|
//
|
||||||
// The listing resolves each unlocker's real ID by rescanning unlockers.d
|
// The listing resolves each unlocker's real ID by rescanning unlockers.d
|
||||||
// after the vault has already enumerated it. If that rescan fails the ID
|
// after the vault has already enumerated it. If that rescan fails the ID
|
||||||
@@ -22,6 +28,7 @@ import (
|
|||||||
"bytes"
|
"bytes"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
@@ -92,6 +99,49 @@ func (f *unlockersDirFailFs) Open(name string) (afero.File, error) {
|
|||||||
return f.Fs.Open(name)
|
return f.Fs.Open(name)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// errMetadataUnreadable is returned by the test filesystem in place of a
|
||||||
|
// successful open of one unlocker's metadata file.
|
||||||
|
var errMetadataUnreadable = errors.New("input/output error")
|
||||||
|
|
||||||
|
// metadataReadFailFs fails every open of the file at unreadablePath. The
|
||||||
|
// file still exists, so checking for it succeeds and only reading it fails.
|
||||||
|
type metadataReadFailFs struct {
|
||||||
|
afero.Fs
|
||||||
|
|
||||||
|
unreadablePath string
|
||||||
|
}
|
||||||
|
|
||||||
|
//nolint:ireturn // afero.File is the interface required by afero.Fs
|
||||||
|
func (f *metadataReadFailFs) Open(name string) (afero.File, error) {
|
||||||
|
if name == f.unreadablePath {
|
||||||
|
return nil, errMetadataUnreadable
|
||||||
|
}
|
||||||
|
|
||||||
|
//nolint:wrapcheck // test double must return the wrapped Fs error as-is
|
||||||
|
return f.Fs.Open(name)
|
||||||
|
}
|
||||||
|
|
||||||
|
// errMetadataUncheckable is returned by the test filesystem in place of a
|
||||||
|
// successful check for one unlocker's metadata file.
|
||||||
|
var errMetadataUncheckable = errors.New("permission denied")
|
||||||
|
|
||||||
|
// metadataStatFailFs fails every check for whether the file at
|
||||||
|
// uncheckablePath exists, as when its unlocker directory cannot be entered.
|
||||||
|
type metadataStatFailFs struct {
|
||||||
|
afero.Fs
|
||||||
|
|
||||||
|
uncheckablePath string
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *metadataStatFailFs) Stat(name string) (os.FileInfo, error) {
|
||||||
|
if name == f.uncheckablePath {
|
||||||
|
return nil, errMetadataUncheckable
|
||||||
|
}
|
||||||
|
|
||||||
|
//nolint:wrapcheck // test double must return the wrapped Fs error as-is
|
||||||
|
return f.Fs.Stat(name)
|
||||||
|
}
|
||||||
|
|
||||||
// writePGPUnlocker writes a PGP unlocker directory with metadata that
|
// writePGPUnlocker writes a PGP unlocker directory with metadata that
|
||||||
// yields the real ID "pgp-<keyID>".
|
// yields the real ID "pgp-<keyID>".
|
||||||
func writePGPUnlocker(
|
func writePGPUnlocker(
|
||||||
@@ -227,3 +277,102 @@ func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
|
|||||||
assert.True(t, unlockers[0].IsCurrent)
|
assert.True(t, unlockers[0].IsCurrent)
|
||||||
assert.False(t, unlockers[1].IsCurrent)
|
assert.False(t, unlockers[1].IsCurrent)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestUnlockersListToleratesCorruptMetadata asserts that one unlocker with
|
||||||
|
// corrupt metadata does not stop the listing. Metadata that is not JSON
|
||||||
|
// leaves that unlocker out; PGP metadata without a usable GPG key ID lists
|
||||||
|
// it as "pgp-unknown". The healthy unlocker is listed with its real ID.
|
||||||
|
func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
healthyID := "pgp-" + listTestGPGKeyID + "A"
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
metadata string
|
||||||
|
wantIDs []string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "not JSON",
|
||||||
|
metadata: "not json",
|
||||||
|
wantIDs: []string{healthyID},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "GPG key ID of the wrong type",
|
||||||
|
metadata: `{"type": "pgp", "gpgKeyId": 42}`,
|
||||||
|
wantIDs: []string{healthyID, "pgp-unknown"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "GPG key ID missing",
|
||||||
|
metadata: `{"type": "pgp"}`,
|
||||||
|
wantIDs: []string{healthyID, "pgp-unknown"},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
fs := newListTestVault(t, 2)
|
||||||
|
metadataPath := filepath.Join(listTestStateDir, "vaults.d",
|
||||||
|
listTestVaultName, listTestUnlockersDirName,
|
||||||
|
listTestUnlockerDirTwo, listTestMetadataFileName)
|
||||||
|
require.NoError(t, afero.WriteFile(
|
||||||
|
fs, metadataPath, []byte(tt.metadata), listTestFilePerm,
|
||||||
|
))
|
||||||
|
|
||||||
|
unlockers := listUnlockersJSON(t, fs)
|
||||||
|
require.Len(t, unlockers, len(tt.wantIDs))
|
||||||
|
|
||||||
|
for i, wantID := range tt.wantIDs {
|
||||||
|
assert.Equal(t, wantID, unlockers[i].ID)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestUnlockersListSkipsUnreadableMetadata asserts that an unlocker whose
|
||||||
|
// metadata file cannot be checked for or cannot be read is left out of the
|
||||||
|
// listing, and the other unlocker is still listed with its real ID. The
|
||||||
|
// failing one sorts first, so finding the other's ID has to step past it
|
||||||
|
// as well.
|
||||||
|
func TestUnlockersListSkipsUnreadableMetadata(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
failingPath := filepath.Join(listTestStateDir, "vaults.d",
|
||||||
|
listTestVaultName, listTestUnlockersDirName,
|
||||||
|
listTestUnlockerDirOne, listTestMetadataFileName)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
wrap func(base afero.Fs) afero.Fs
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "checking for the file fails",
|
||||||
|
wrap: func(base afero.Fs) afero.Fs {
|
||||||
|
return &metadataStatFailFs{Fs: base, uncheckablePath: failingPath}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "reading the file fails",
|
||||||
|
wrap: func(base afero.Fs) afero.Fs {
|
||||||
|
return &metadataReadFailFs{Fs: base, unreadablePath: failingPath}
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
fs := tt.wrap(newListTestVault(t, 2))
|
||||||
|
|
||||||
|
unlockers := listUnlockersJSON(t, fs)
|
||||||
|
|
||||||
|
require.Len(t, unlockers, 1,
|
||||||
|
"only the unlocker with usable metadata may be listed")
|
||||||
|
assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[0].ID,
|
||||||
|
"the listed row must carry the real unlocker ID")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -155,14 +155,18 @@ func (p *PGPUnlocker) GetDirectory() string {
|
|||||||
return p.Directory
|
return p.Directory
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetID implements Unlocker interface - generates ID from GPG key ID
|
// GetID implements Unlocker interface - generates ID from GPG key ID.
|
||||||
|
// If the metadata has no usable GPG key ID, it warns with the unlocker's
|
||||||
|
// directory and returns "pgp-unknown", so listing the other unlockers
|
||||||
|
// still works.
|
||||||
func (p *PGPUnlocker) GetID() string {
|
func (p *PGPUnlocker) GetID() string {
|
||||||
// Generate ID using GPG key ID: pgp-<keyid>
|
// Generate ID using GPG key ID: pgp-<keyid>
|
||||||
gpgKeyID, err := p.GetGPGKeyID()
|
gpgKeyID, err := p.GetGPGKeyID()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// The vault metadata is corrupt - this is a fatal error
|
Warn("PGP unlocker metadata is corrupt or missing its GPG key ID",
|
||||||
// We cannot continue with a fallback ID as that would mask data corruption
|
"directory", p.Directory, "error", err)
|
||||||
panic(fmt.Sprintf("PGP unlocker metadata is corrupt or missing GPG key ID: %v", err))
|
|
||||||
|
return "pgp-unknown"
|
||||||
}
|
}
|
||||||
|
|
||||||
return "pgp-" + gpgKeyID
|
return "pgp-" + gpgKeyID
|
||||||
@@ -197,6 +201,10 @@ func (p *PGPUnlocker) GetGPGKeyID() (string, error) {
|
|||||||
return "", fmt.Errorf("failed to parse PGP metadata: %w", err)
|
return "", fmt.Errorf("failed to parse PGP metadata: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if pgpMetadata.GPGKeyID == "" {
|
||||||
|
return "", fmt.Errorf("PGP metadata: %w", errGPGKeyIDEmpty)
|
||||||
|
}
|
||||||
|
|
||||||
return pgpMetadata.GPGKeyID, nil
|
return pgpMetadata.GPGKeyID, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -233,9 +233,10 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
|
|||||||
|
|
||||||
exists, err := afero.Exists(v.fs, metadataPath)
|
exists, err := afero.Exists(v.fs, metadataPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
secret.Warn("Skipping unlocker directory whose metadata file cannot be checked",
|
||||||
"failed to check if metadata exists for unlocker %s: %w",
|
"directory", file.Name(), "error", err)
|
||||||
file.Name(), err)
|
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
if !exists {
|
if !exists {
|
||||||
@@ -247,16 +248,20 @@ func (v *Vault) ListUnlockers() ([]UnlockerMetadata, error) {
|
|||||||
|
|
||||||
metadataBytes, err := afero.ReadFile(v.fs, metadataPath)
|
metadataBytes, err := afero.ReadFile(v.fs, metadataPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
secret.Warn("Skipping unlocker directory with unreadable metadata file",
|
||||||
"failed to read metadata for unlocker %s: %w", file.Name(), err)
|
"directory", file.Name(), "error", err)
|
||||||
|
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
var metadata UnlockerMetadata
|
var metadata UnlockerMetadata
|
||||||
|
|
||||||
err = json.Unmarshal(metadataBytes, &metadata)
|
err = json.Unmarshal(metadataBytes, &metadata)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
secret.Warn("Skipping unlocker directory with corrupt metadata file",
|
||||||
"failed to parse metadata for unlocker %s: %w", file.Name(), err)
|
"directory", file.Name(), "error", err)
|
||||||
|
|
||||||
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
unlockers = append(unlockers, metadata)
|
unlockers = append(unlockers, metadata)
|
||||||
|
|||||||
+1
-6
@@ -6,6 +6,7 @@
|
|||||||
# make, node, yarn, go, or python). Node is used directly if installed;
|
# make, node, yarn, go, or python). Node is used directly if installed;
|
||||||
# otherwise a pinned version is installed via nvm (installing nvm
|
# otherwise a pinned version is installed via nvm (installing nvm
|
||||||
# itself first, from a hash-verified release archive, never curl | sh).
|
# itself first, from a hash-verified release archive, never curl | sh).
|
||||||
|
# golangci-lint is never installed: script/lint runs it in docker.
|
||||||
#
|
#
|
||||||
# Uncomment the language sections in main() that apply to this repo.
|
# Uncomment the language sections in main() that apply to this repo.
|
||||||
set -eu
|
set -eu
|
||||||
@@ -136,12 +137,6 @@ main() {
|
|||||||
|
|
||||||
# ---- Go repos ----
|
# ---- Go repos ----
|
||||||
if missing go; then pkg_install go golang go go; fi
|
if missing go; then pkg_install go golang go go; fi
|
||||||
# golangci-lint: packaged in nix, brew, and apk. On apt there is no
|
|
||||||
# package: download a specific release archive from GitHub and
|
|
||||||
# verify its hash (verify_sha256), never curl | sh.
|
|
||||||
if missing golangci-lint; then
|
|
||||||
pkg_install golangci-lint golangci-lint golangci-lint golangci-lint
|
|
||||||
fi
|
|
||||||
go mod download
|
go mod download
|
||||||
|
|
||||||
# ---- Python repos ----
|
# ---- Python repos ----
|
||||||
|
|||||||
+14
-4
@@ -1,14 +1,24 @@
|
|||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
# script/lint: run the linter.
|
# script/lint: run the linter, in docker only. Builds Dockerfile.lint,
|
||||||
|
# where golangci-lint runs as a build step.
|
||||||
|
#
|
||||||
|
# A cached build lints nothing, so --no-cache-filter rebuilds the lint
|
||||||
|
# stage on every run, an unchanged tree included. It ignores a stage name
|
||||||
|
# that does not exist, so --target names the same stage: a rename then
|
||||||
|
# fails the build instead of serving the lint from cache. cacheonly keeps
|
||||||
|
# no image; only the build's success matters.
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
# CGO is required (Makefile exports this too)
|
docker build \
|
||||||
export CGO_ENABLED=1
|
--progress=plain \
|
||||||
golangci-lint run --timeout 5m
|
--target lint \
|
||||||
|
--no-cache-filter=lint \
|
||||||
|
--output=type=cacheonly \
|
||||||
|
-f Dockerfile.lint .
|
||||||
}
|
}
|
||||||
|
|
||||||
main "$@"
|
main "$@"
|
||||||
|
|||||||
Reference in New Issue
Block a user