Fix missing-key errors for restore and deep verify to name both key sources (closes #401)

errSecretKeyRequired and errDecryptionKeyRequired now share one message that says the age secret key is missing and names both places it can be set, the VAULTIK_AGE_SECRET_KEY environment variable first since it is preferred, then the age_secret_key config file field.

Model: deepseek-v4-pro

README.md

@@ -836,8 +836,6 @@ Work planned after 1.0. Loosely ordered by priority.
 Every command's user-facing output is governed by `internal/ui`, in one
 of two ways. Color is enabled when the stream written to is a TTY and
 the `NO_COLOR` environment variable is unset (https://no-color.org/).
-The same rule governs the colorized log lines `internal/log` writes to a
-terminal.
 
 * **Status, progress, warnings, and errors** go through the `internal/ui`
   message methods below: marker-prefixed, colored on a TTY, and — except

internal/log/tty_handler.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
-	"os"
 	"strconv"
 	"strings"
 	"sync"
@@ -61,12 +60,6 @@ type TTYHandler struct {
 	mu   *sync.Mutex
 	out  io.Writer
 
-	// color reports whether records are written with ANSI color. It is
-	// false when NO_COLOR is set, matching internal/ui's shouldColor
-	// test; the caller already chose this handler for a terminal, so
-	// TTY-ness is not re-checked here.
-	color bool
-
 	// attrs are the attributes accumulated through WithAttrs, emitted
 	// ahead of each record's own attributes. Their keys already carry
 	// the group path that was open when they were added, so no
@@ -79,19 +72,16 @@ type TTYHandler struct {
 	groups []string
 }
 
-// NewTTYHandler creates a new TTY handler. Color is enabled unless the
-// NO_COLOR environment variable is set to a non-empty value, the same
-// test internal/ui applies to status lines.
+// NewTTYHandler creates a new TTY handler with colored output.
 func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler {
 	if opts == nil {
 		opts = &slog.HandlerOptions{}
 	}
 
 	return &TTYHandler{
-		out:   out,
-		opts:  *opts,
-		mu:    &sync.Mutex{},
-		color: os.Getenv("NO_COLOR") == "",
+		out:  out,
+		opts: *opts,
+		mu:   &sync.Mutex{},
 	}
 }
 
@@ -134,10 +124,10 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
 	// are written around it: it can carry text from an untrusted source
 	// (a storage error, for one), and a raw control character would
 	// otherwise reach the terminal.
-	_, _ = fmt.Fprintf(h.out, "%s %s %s",
-		h.paint(colorGray, timestamp),
-		h.paint(levelColor, level),
-		h.paint(colorBold, sanitize(r.Message)))
+	_, _ = fmt.Fprintf(h.out, "%s%s%s %s%s%s %s%s%s",
+		colorGray, timestamp, colorReset,
+		levelColor, level, colorReset,
+		colorBold, sanitize(r.Message), colorReset)
 
 	// Attributes carried by the handler come first, then the record's
 	// own. Handler attributes were qualified when they were added; the
@@ -246,7 +236,6 @@ func (h *TTYHandler) clone() *TTYHandler {
 		opts:   h.opts,
 		mu:     h.mu,
 		out:    h.out,
-		color:  h.color,
 		attrs:  make([]slog.Attr, len(h.attrs), len(h.attrs)+1),
 		groups: make([]string, len(h.groups), len(h.groups)+1),
 	}
@@ -284,19 +273,9 @@ func (h *TTYHandler) writeAttr(a slog.Attr) {
 	// the snapshot database — so a control character in one of them must
 	// be rendered as an escape sequence rather than reaching the terminal,
 	// where it could move the cursor or inject its own colours.
-	_, _ = fmt.Fprintf(h.out, " %s=%s",
-		h.paint(colorCyan, sanitize(a.Key)),
-		h.paint(colorBlue, sanitize(value)))
-}
-
-// paint wraps s in the given ANSI color when color is enabled, and
-// returns s unchanged otherwise.
-func (h *TTYHandler) paint(color, s string) string {
-	if !h.color {
-		return s
-	}
-
-	return color + s + colorReset
+	_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
+		colorCyan, sanitize(a.Key), colorReset,
+		colorBlue, sanitize(value), colorReset)
 }
 
 // sanitize returns s unchanged when every rune in it is printable, and a

internal/log/tty_handler_test.go

@@ -421,19 +421,3 @@ func TestTTYHandlerEmptyGroupAndAttrsAreNoOps(t *testing.T) {
 	assert.Contains(t, plain, "kept=yes")
 	assert.NotContains(t, plain, " =")
 }
-
-// TestTTYHandlerRespectsNoColor checks that the handler writes no ANSI
-// color when NO_COLOR is set, matching internal/ui's shouldColor test.
-// The handler is only ever chosen for a terminal, so this is the one
-// remaining condition that can still leave color in log output.
-func TestTTYHandlerRespectsNoColor(t *testing.T) {
-	t.Setenv("NO_COLOR", "1")
-
-	var buf bytes.Buffer
-
-	logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
-	logger.Info("hello", "key", "value")
-
-	assert.NotContains(t, buf.String(), "\x1b[",
-		"NO_COLOR must suppress every ANSI escape in log output")
-}

internal/vaultik/restore.go

@@ -30,11 +30,7 @@ var (
 	errFilesFailedRestore    = errors.New("file(s) failed to restore")
 	errPathsNotInSnapshot    = errors.New("requested path(s) not in the snapshot")
 	errFilesFailedVerify     = errors.New("files failed verification")
-	errDecryptionKeyRequired = errors.New(
-		"decryption key required for restore\n\n" +
-			"Set the VAULTIK_AGE_SECRET_KEY environment variable to your " +
-			"age private key file:\n" +
-			"  export VAULTIK_AGE_SECRET_KEY=\"$(cat vaultik_backup_private_key.txt)\"")
+	errDecryptionKeyRequired = errors.New(ageSecretKeyMissingMessage)
 	// errInvalidAgeSecretKey is returned when the configured key does not
 	// parse as any age identity. It names the source but never the value,
 	// which is secret, so the message is safe to print and log.

internal/vaultik/restore_identity_test.go

@@ -106,3 +106,18 @@ func TestRestoreIdentitiesMissingKey(t *testing.T) {
 	_, err := v.restoreIdentities()
 	require.ErrorIs(t, err, errDecryptionKeyRequired)
 }
+
+// TestMissingKeyErrorsNameBothSources asserts the missing-key errors for
+// restore and deep verify say the key is missing and name both places it
+// can be set, the environment variable first since it is preferred, with
+// the same wording in both.
+func TestMissingKeyErrorsNameBothSources(t *testing.T) {
+	t.Parallel()
+
+	const want = "age secret key is missing; set it in " +
+		"the VAULTIK_AGE_SECRET_KEY environment variable or the " +
+		"age_secret_key config file field"
+
+	require.Equal(t, want, errSecretKeyRequired.Error())
+	require.Equal(t, want, errDecryptionKeyRequired.Error())
+}

internal/vaultik/verify.go

@@ -20,11 +20,18 @@ import (
 	"sneak.berlin/go/vaultik/internal/storage"
 )
 
+// ageSecretKeyMissingMessage is the shared text of the missing-key errors
+// for restore and deep verify. It says the key is missing and names both
+// places it can come from, the environment variable first since it is
+// preferred.
+const ageSecretKeyMissingMessage = "age secret key is missing; set it in " +
+	"the VAULTIK_AGE_SECRET_KEY environment variable or the " +
+	"age_secret_key config file field"
+
 // Sentinel errors for snapshot verification failures.
 var (
-	errVerificationFailed = errors.New("verification failed")
-	errSecretKeyRequired  = errors.New(
-		"VAULTIK_AGE_SECRET_KEY not set; required for deep verification")
+	errVerificationFailed  = errors.New("verification failed")
+	errSecretKeyRequired   = errors.New(ageSecretKeyMissingMessage)
 	errChunksOutOfOrder    = errors.New("chunks out of order")
 	errChunkHashMismatch   = errors.New("chunk hash mismatch")
 	errNegativeChunkLength = errors.New("chunk length is negative")