diff --git a/TODO.md b/TODO.md index 02f2cf9..37ea5a0 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,22 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-06: The tests run quickly with the race detector on + (https://git.eeqj.de/sneak/secret/issues/120). Most of their time went to + deriving keys from passphrases with scrypt, which is slow on purpose. The new + `secret.ScryptWorkFactor`, when not zero, replaces age's scrypt work factor + when a passphrase encrypts; the tests of `internal/secret`, `internal/vault` + and `internal/cli` set it to 1 before any test runs, and the program never + sets it; `TestGetCommandOutputsToStdout` checks that the passphrase unlocker + the built binary's `secret init` writes names age's work factor, 18. + `TestRemovalAsksWithoutHoldingLock` and `TestFailedCommandReleasesLock` no + longer run in parallel with other tests: each waits at most 10 seconds for the + in-memory lock that every test in the package shares, and other tests' + commands held it longer. `TestConcurrentAddsKeepEveryVersion`, which times + nothing, and `TestGetCommandOutputsToStdout`, which no longer sets an + environment variable its commands do not read, now run in parallel. The + `script/cibuild` comment no longer says that tests are skipped without its + memlock ulimit. - 2026-10-05: No test stores a secret larger than 1 MiB (https://git.eeqj.de/sneak/secret/issues/52). The size tests for `secret add`, `secret import` and the stdin buffer no longer try 2 MB, 10 MB, 99 MB, 100 MB diff --git a/internal/cli/confirm_test.go b/internal/cli/confirm_test.go index 32a56b8..af2ec91 100644 --- a/internal/cli/confirm_test.go +++ b/internal/cli/confirm_test.go @@ -353,9 +353,9 @@ func TestRemovalWithoutTerminalFailsAtOnce(t *testing.T) { // for its answer, another command can take the state directory lock and // change the secret, and that the removal then removes nothing, since the // secret is no longer what the question named. +// +//nolint:paralleltest // times commands against the in-memory lock all tests share func TestRemovalAsksWithoutHoldingLock(t *testing.T) { - t.Parallel() - r := newRemoval(t, "rm") answers, answerWriter := io.Pipe() diff --git a/internal/cli/integration_test.go b/internal/cli/integration_test.go index d572e5c..9e66629 100644 --- a/internal/cli/integration_test.go +++ b/internal/cli/integration_test.go @@ -52,8 +52,12 @@ func runSecretWithStdin(stdin string, env map[string]string, args ...string) (st return cli.ExecuteCommandInProcess(args, stdin, env) } -// TestMain runs before all tests and ensures the binary is built +// TestMain runs before all tests and ensures the binary is built. It also +// makes passphrase encryption in the tests cheap (see +// secret.ScryptWorkFactor); the binary keeps age's work factor. func TestMain(m *testing.M) { + secret.ScryptWorkFactor = 1 + // Get the current working directory wd, err := os.Getwd() if err != nil { diff --git a/internal/cli/lock_test.go b/internal/cli/lock_test.go index 502553f..caa2ee3 100644 --- a/internal/cli/lock_test.go +++ b/internal/cli/lock_test.go @@ -94,9 +94,9 @@ func numbered(prefix string, count int) []string { // lock, adds of a new secret all find it absent and replace each other, and // forced adds read the same highest version number and overwrite each // other's version. With it they behave as if run one after another. -// -//nolint:paralleltest // times commands against the in-memory lock all tests share func TestConcurrentAddsKeepEveryVersion(t *testing.T) { + t.Parallel() + mnemonic := testMnemonicBuffer(t) const adds = 8 @@ -110,6 +110,8 @@ func TestConcurrentAddsKeepEveryVersion(t *testing.T) { {"real", afero.NewOsFs(), t.TempDir()}, } { t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := vault.CreateVault(tc.fs, tc.stateDir, "default", mnemonic, nil) require.NoError(t, err) @@ -235,9 +237,9 @@ func TestEncryptPipedIntoAdd(t *testing.T) { // TestFailedCommandReleasesLock checks that a command failing after it // took the state directory lock leaves the lock free for the next command. +// +//nolint:paralleltest // times commands against the in-memory lock all tests share func TestFailedCommandReleasesLock(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() cli := NewCLIInstanceWithStateDir(fs, testStateDir) diff --git a/internal/cli/stdout_stderr_test.go b/internal/cli/stdout_stderr_test.go index a0916ce..f1937fc 100644 --- a/internal/cli/stdout_stderr_test.go +++ b/internal/cli/stdout_stderr_test.go @@ -15,11 +15,11 @@ import ( // TestGetCommandOutputsToStdout tests that 'secret get' outputs the secret // value to stdout, not stderr func TestGetCommandOutputsToStdout(t *testing.T) { - // Create a temporary directory for our vault - tempDir := t.TempDir() + t.Parallel() - // Set environment variables for the test - t.Setenv(secret.EnvStateDir, tempDir) + // Create a temporary directory for our vault; each command is given it + // in its environment + tempDir := t.TempDir() // Find the secret binary path wd, err := filepath.Abs("../..") @@ -41,6 +41,18 @@ func TestGetCommandOutputsToStdout(t *testing.T) { output, err := cmd.CombinedOutput() require.NoError(t, err, "init should succeed: %s", string(output)) + // The binary, unlike these tests, encrypts the passphrase unlocker's key + // at age's scrypt work factor, 18. age writes the work factor last on the + // second line of priv.age: "-> scrypt ". + vaultDir := filepath.Join(tempDir, "vaults.d", "default") + unlockerName := readFile(t, filepath.Join(vaultDir, "current-unlocker")) + unlockerDir := filepath.Join(vaultDir, "unlockers.d", string(unlockerName)) + privAge := readFile(t, filepath.Join(unlockerDir, "priv.age")) + header := strings.SplitN(string(privAge), "\n", 3) + require.Len(t, header, 3, "priv.age should start with an age header") + assert.Regexp(t, `^-> scrypt \S+ 18$`, header[1], + "the passphrase unlocker should be encrypted at scrypt work factor 18") + // Add a secret //nolint:gosec // G204: test executes the freshly built secret binary cmd = exec.CommandContext(t.Context(), secretPath, "add", "test/secret") diff --git a/internal/secret/crypto.go b/internal/secret/crypto.go index 9c017a1..f6005f0 100644 --- a/internal/secret/crypto.go +++ b/internal/secret/crypto.go @@ -28,6 +28,17 @@ var ( // terminal to read the mnemonic from, reading it failed, or it was empty. var ErrMnemonicNotRead = errors.New("failed to read mnemonic") +// ScryptWorkFactor is, when not zero, the scrypt work factor that +// EncryptWithPassphrase uses instead of age's, 18: log2 of scrypt's cost +// parameter N. Deriving a key with age's takes about a second and 256 MiB, on +// purpose, since so does every guess at the passphrase. Only tests set it, +// lower, before any test runs, so that the passphrase unlockers they create +// cost nothing; the program leaves it zero. Decryption takes the work factor +// from the encrypted data, so it needs no setting. +// +//nolint:gochecknoglobals // set by the tests of the packages that use this one +var ScryptWorkFactor int + // EncryptToRecipient encrypts data to a recipient using age // The data parameter should be a LockedBuffer for secure memory handling func EncryptToRecipient( @@ -142,6 +153,10 @@ func EncryptWithPassphrase( return nil, fmt.Errorf("failed to create scrypt recipient: %w", err) } + if ScryptWorkFactor != 0 { + recipient.SetWorkFactor(ScryptWorkFactor) + } + return EncryptToRecipient(data, recipient) } diff --git a/internal/secret/secret_test.go b/internal/secret/secret_test.go index e5dae85..94bd5d4 100644 --- a/internal/secret/secret_test.go +++ b/internal/secret/secret_test.go @@ -25,6 +25,14 @@ var ( errNotImplementedInMock = errors.New("not implemented in mock") ) +// TestMain makes passphrase encryption in the tests cheap; see +// ScryptWorkFactor. +func TestMain(m *testing.M) { + ScryptWorkFactor = 1 + + os.Exit(m.Run()) +} + // MockVault is a test implementation of the VaultInterface type MockVault struct { name string diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index 2b6967d..72e0243 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -3,6 +3,7 @@ package vault_test import ( "bytes" "errors" + "os" "path/filepath" "slices" "testing" @@ -28,6 +29,14 @@ const ( testPassphrase = "test-passphrase" ) +// TestMain makes passphrase encryption in the tests cheap; see +// secret.ScryptWorkFactor. +func TestMain(m *testing.M) { + secret.ScryptWorkFactor = 1 + + os.Exit(m.Run()) +} + // testMnemonicBuffer returns testMnemonic in a locked buffer that is // destroyed when the test ends. func testMnemonicBuffer(t *testing.T) *memguard.LockedBuffer { diff --git a/script/cibuild b/script/cibuild index 58bf15f..bb21515 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,9 +1,9 @@ #!/bin/sh # script/cibuild: run the CI build. The Dockerfile runs script/check # (via make check), so a successful build implies all checks pass. -# The Gitea workflow runs this on push. The memlock ulimit lets the tests -# that lock large secrets in memory (memguard mlocks them) run; under the -# lower limit of a plain `docker build .` they are skipped. +# The Gitea workflow runs this on push. The memlock ulimit lifts the limit +# on memory the tests lock (memguard mlocks secrets); they also pass under +# the lower limit of a plain `docker build .`. # A cached build checks nothing: a new CHECK_EPOCH on every run makes the # Dockerfile's check steps run again on an unchanged tree, while its base # images and module downloads stay cached.