Stamp Docker image version from the host; flush profiles on error exit (closes #75)

Docker images reported commit unknown because the build ran git inside the container while .dockerignore excludes .git, and VERSION was never overridden. script/docker and script/cibuild now compute version, commit and date on the host and pass them as build args; the Dockerfile runs no git and falls back to dev and unknown, never empty, on a bare docker build.

Profiling a failing command gave a truncated or missing profile: Entry and each command goroutine called os.Exit(1), skipping the deferred profile writers in main. Entry now returns a status that main exits with after its defers run, and command goroutines report failure through one RunOperation helper, which also restores PID-lock release and graceful shutdown on failure.

model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge)

Co-authored-by: clawbot <clawbot@noreply.example.org>

cmd/vaultik/dockerversion_test.go

@@ -0,0 +1,102 @@
+package main_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// This file guards the version stamping of the product image (issue
+// #75). The failure it protects against is silent: the image still
+// builds and runs, but `vaultik version` inside it reports "commit:
+// unknown", so an operator cannot tell which source produced a given
+// backup. .dockerignore excludes .git, so the build cannot derive the
+// commit itself; the values must be computed on the host and passed in.
+//
+// These are parses of the committed files, for the same reason the lint
+// guards next door are: shelling out to docker would nest a build
+// inside `make test`. That `vaultik version` in the built image really
+// prints the host's version is verified by hand and recorded on the
+// pull request.
+
+// dockerScript is script/docker, relative to the repository root.
+const dockerScript = "script/docker"
+
+// versionArgs are the ldflag targets the build stamps and, matching
+// them, the build args the host must supply. The names line up so the
+// same list checks both files.
+func versionArgs() []string {
+	return []string{"VERSION", "COMMIT", "COMMIT_DATE"}
+}
+
+// TestProductDockerfileTakesVersionAsBuildArgs fails unless the build
+// declares each version arg and stamps it into the binary by ldflag
+// reference, rather than computing it in the container.
+func TestProductDockerfileTakesVersionAsBuildArgs(t *testing.T) {
+	t.Parallel()
+
+	found := instructions(t, productDockerfile)
+
+	for _, arg := range versionArgs() {
+		require.GreaterOrEqual(t, indexOf(found, "ARG "+arg), 0,
+			"%s must declare `ARG %s` so the host can pass it in",
+			productDockerfile, arg)
+
+		assertLdflagReferences(t, found, arg)
+	}
+}
+
+// TestProductDockerfileDoesNotDeriveVersionItself is the anti-regression
+// for the original defect: the container ran `git rev-parse`, but .git
+// is not in the build context, so it always resolved to "unknown". No
+// git command may reach into a build that cannot see the history.
+func TestProductDockerfileDoesNotDeriveVersionItself(t *testing.T) {
+	t.Parallel()
+
+	text := instructionText(readRepoFile(t, productDockerfile))
+
+	assert.NotContains(t, text, "git ",
+		"%s must not run git: .git is excluded from the build context, so"+
+			" any value it derives is wrong. Pass version, commit and date"+
+			" in as build args instead.", productDockerfile)
+}
+
+// TestDockerScriptComputesVersionOnTheHost fails unless script/docker
+// derives each value where .git exists and passes it as a build arg,
+// with VERSION coming from script/version so a Docker build reports the
+// same string a local build of the same tree would.
+func TestDockerScriptComputesVersionOnTheHost(t *testing.T) {
+	t.Parallel()
+
+	script := readRepoFile(t, dockerScript)
+
+	for _, arg := range versionArgs() {
+		assert.Contains(t, script, "--build-arg "+arg+"=",
+			"%s must pass --build-arg %s to the build", dockerScript, arg)
+	}
+
+	assert.Contains(t, script, "/version",
+		"%s must take VERSION from script/version, the source of truth"+
+			" shared with the Makefile", dockerScript)
+}
+
+// assertLdflagReferences fails unless some build instruction stamps the
+// named variable from the ARG (a ${arg} reference), not from a value
+// computed inside the container.
+func assertLdflagReferences(t *testing.T, found []string, arg string) {
+	t.Helper()
+
+	for _, instruction := range found {
+		if strings.HasPrefix(instruction, "RUN ") &&
+			strings.Contains(instruction, "go build") &&
+			strings.Contains(instruction, "${"+arg+"}") {
+			return
+		}
+	}
+
+	assert.Fail(t, "version arg is declared but never stamped",
+		"the go build in %s must reference ${%s} in its ldflags, or the"+
+			" arg is passed and discarded", productDockerfile, arg)
+}

cmd/vaultik/lintdocker_test.go

@@ -304,10 +304,14 @@ func instructionText(contents string) string {
 }
 
 // indexOf returns the position of the first instruction equal to, or
-// beginning with, want; -1 if there is none.
+// beginning with, want; -1 if there is none. An `ARG NAME=default`
+// counts as beginning with `ARG NAME`, so a declared arg is found
+// whether or not it carries a default.
 func indexOf(found []string, want string) int {
 	for i, instruction := range found {
-		if instruction == want || strings.HasPrefix(instruction, want+" ") {
+		if instruction == want ||
+			strings.HasPrefix(instruction, want+" ") ||
+			strings.HasPrefix(instruction, want+"=") {
 			return i
 		}
 	}

cmd/vaultik/main.go

@@ -10,6 +10,16 @@ import (
 )
 
 func main() {
+	os.Exit(run())
+}
+
+// run sets up optional profiling, runs the CLI, and returns the process
+// exit code. os.Exit lives in main so it fires only after run's deferred
+// profile writers have flushed. cli.Entry returns a status code rather
+// than calling os.Exit itself: an os.Exit from inside it would skip
+// these defers and truncate the profile of a failing command -- exactly
+// the command one most often wants to profile.
+func run() int {
 	// CPU profiling: set VAULTIK_CPUPROFILE=/path/to/cpu.prof
 	if cpuProfile := os.Getenv("VAULTIK_CPUPROFILE"); cpuProfile != "" {
 		f, err := os.Create(cpuProfile) //nolint:gosec // G304: operator-set path
@@ -46,5 +56,5 @@ func main() {
 		}()
 	}
 
-	cli.Entry()
+	return cli.Entry()
 }