test: pin fingerprint capture cleanliness contract

[AlexKantor87]

Summary

Adds three tests that pin the customer-facing contract for kosli fingerprint: stdout is exactly the fingerprint, stderr is empty on the success path, and stderr remains a functional channel for opt-in --debug output.

Closeskosli-dev/server#5564. Follow-up to #840, which fixed the most recent regression but did not pin the underlying contract.

Why

The version-notice-on-stderr bug has shipped three times in two weeks:

Round	PR	Commit	What broke
1	#781	3097f68	Background goroutine wrote update notice to stderr on every command
2	#799	d76c084	Tried to fix it by skipping when --output=json; missed fingerprint (no --output flag)
3	#840	eb93aaa	Removed the goroutine entirely

Each round added a test for the case the author was thinking about. None pinned the customer contract:

FP=$(kosli fingerprint <artifact> 2>&1) must produce a parseable fingerprint.

@tooky asked in the issue: "With my customer's hat on I would really appreciate it if, before this bug is fixed, a failing test is added so that it cannot reappear." This PR addresses that — and not just for the version-check bug. The contract is cause-agnostic.

What the tests assert

TestFingerprintFile_CaptureCleanliness

Stubs the update check to return a notice (so the test fires deterministically), runs fingerprint --artifact-type=file testdata/file1, asserts:

• stdout == "<sha256>\n" exactly — anything else breaks $(...) capture
• stderr == "" exactly — anything else breaks 2>&1 capture
• combined stdout+stderr == the fingerprint — what the customer's bash actually evaluates

The exact-empty assertion on stderr is the key generalisation. The previous test used NotContains "A new version", which only catches the version-notice string. Equal-empty catches anything — a deprecation warning, a telemetry log, a future framework upgrade that adds startup output.

TestFingerprintDir_CaptureCleanliness

Same contract for --artifact-type=dir. The directory and OCI paths are slow enough for a background goroutine to complete and pollute stderr — this is the path that triggered the cyber-dojo failure.

TestFingerprintFile_DebugModeIsAllowedToWriteStderr

Pins the inverse: with --debug=true, stderr MUST contain "calculated fingerprint". Stops a future contributor from "fixing" a CaptureCleanliness regression by silencing the logger inside the fingerprint code path. Asserting on the fingerprint-specific debug line (not just NotEmpty(stderr)) ensures earlier framework debug logs from PreRunE don't mask a real silencing regression.

How the test fails on the bug (proven locally)

Reverting eb93aaa4 and re-running:

--- FAIL: TestFingerprintCaptureTestSuite/TestFingerprintFile_CaptureCleanliness
expected: "7509...e6ca9\n"
actual : "7509...e6ca9\n\nA new version of the Kosli CLI is available: v9.99.0..."
Messages: combined output (the 2>&1 capture pattern) must be exactly the fingerprint

The "actual" line is the same shape of output cyber-dojo's CI captured.

How the test catches future regressions (also proven locally)

Adding a stray logger.Warn(...) to fingerprint.go's run() — a totally different cause from the version check — fails with:

--- FAIL: TestFingerprintCaptureTestSuite/TestFingerprintFile_CaptureCleanliness
actual : "[warning] the --artifact-type=file flag will be renamed to --type=file in v3.0\n"
Messages: stderr must be empty — any output here pollutes 2>&1 capture pipelines

A call-site-restriction test on version.CheckForUpdate would not have caught this. The contract test does.

What this test does not block

The test runs with no --debug flag, no deprecated flags, no config file, no auth. So it doesn't fire on:

• logger.Debug output (gated on --debug=true)
• Deprecation warnings (only fire when the user uses a deprecated flag)
• Plain-text-token warnings (only fire if ~/.kosli.yml has plain text)
• Error output (failure path is a different contract)

The line it draws: the happy path of kosli fingerprint with default flags must produce no stderr. Anything firing unconditionally on every invocation has to actively justify itself by updating these tests in code review — which is exactly the conversation that wasn't happening for the last three rounds.

Test plan

• go test ./cmd/kosli/ -run TestFingerprintCaptureTestSuite -v — all three pass
• make lint vet fmt — green (golangci-lint 2.11.4)
• Reproduced the original bug locally with a stamped binary; confirmed test fails on the buggy root.go, passes after fix: remove version checks on all commands #840's fix
• Simulated an unrelated future regression (stray logger.Warn in fingerprint path); confirmed test catches it
• Simulated an over-correction (silencing the logger in fingerprint path); confirmed the debug-mode test catches it
• CI integration suite — full make test_integration not run locally (requires KOSLI_API_TOKEN_PROD + AWS CLI access). The new file is pure in-process Go with no server, network, or auth dependency, so the risk surface is low. Letting CI validate.

🤖 Generated with Claude Code

[claude]

Claude finished @AlexKantor87's task in 2m 47s —— View job

---

PR Review: Fingerprint Capture Cleanliness Tests

• Phase 1: Static analysis (go build, go vet, go mod tidy)
• Phase 2: Read and review the changed files
• Phase 3: Cross-reference with existing test patterns
• Phase 4: Post review feedback

---

Static Analysis

All three checks pass cleanly — go build ./..., go vet ./..., and go mod tidy produce no errors or diffs.

Review Summary

This is a well-designed PR. The contract tests are cause-agnostic (asserting stderr == "" rather than NotContains "specific string"), which is exactly the right generalization for preventing the class of bug described in the PR body. The test structure follows established repo patterns (suite.Suite, executeCommandC, SetCheckForUpdateOverride), and the test fixtures (testdata/file1, testdata/folder1, alpine-by-digest) are all pre-existing.

What works well

• The Equal("", stderr) assertion is the key insight — it catches any future stderr pollution, not just the version-notice string. This is materially stronger than the previous NotContains approach.
• The debug-mode inverse test (TestFingerprintFile_DebugModeIsAllowedToWriteStderr) is a smart guard against over-correction. Asserting on "calculated fingerprint" specifically (which maps to cli_utils.go:348) ensures the fingerprint code path's own debug output is tested, not just framework-level PreRunE logs.
• The defer version.SetCheckForUpdateOverride(...)() pattern correctly restores the original state after each test, matching the pattern in root_test.go.
• Docker variant addition (from the latest commit) addresses @JonJagger's review feedback and covers a genuinely different code path (internal/docker.GetImageFingerprint).

Nits (non-blocking)

Two minor items posted as inline comments:

1. Duplicated fake notice string — fakeUpdateNotice is identical to fakeNotice in root_test.go (declared twice there, once here). Not a problem today, but three places to update if the format changes.
2. alpineFingerprint scope — declared as a local const in the docker test method, while file1Fingerprint and folder1Fingerprint are package-level. Minor inconsistency.

No issues found with

• Correctness: All asserted values cross-check against existing golden values in fingerprint_test.go.
• Test isolation: Each test sets up and tears down its own SetCheckForUpdateOverride stub via defer. The mutex-protected override (sync.RWMutex in update_check.go) is safe for parallel test execution.
• Security: No security concerns — pure in-process test file with no auth, network, or external dependencies.
• Performance: SetupSuite pulls the Docker image once for the entire suite, not per-test. The file/dir tests are fast in-process operations.

Verdict: Looks good to merge. The two inline nits are optional polish — neither affects correctness.

[mbevc1]

Do we expect this to be different than file test? 🤔

[AlexKantor87]

Good catch — the dir fingerprint of testdata/folder1 is already pinned in fingerprint_test.go:35 as c43808cb..., so no reason to use Regexp here. Just pushed 4faf6f1 aligning the dir test with the file test — same shape, same three contracts (exact stdout, empty stderr, exact combined). Thanks for the prompt.

[JonJagger]

These tests are good. The one checking for no extra output even when --debug=true in particular.
I think it is worth also testing that you also get no extra output when the --artifact-type is docker or oci. The code paths for those flag values could try to print debug info. It may be trickier for these because generating a fingerprint is a path for all the kosli-attest commands as well as for kosli-fingerprint.

[AlexKantor87]

Thanks @JonJagger — both fair points. Just pushed 4faf6f1 which adds TestFingerprintDocker_CaptureCleanliness mirroring the existing docker test pattern (alpine pinned by digest, pulled in SetupSuite).

For the other two pieces I've raised follow-ups rather than expand this PR, since both involve real engineering work that warrants a separate review pass:

• OCI variant → #848. There's no OCI test scaffolding in fingerprint_test.go today, so it needs either standing up the existing cli_registry container with a known image, or pinning a stable public OCI image. Worth doing properly rather than improvising on this PR.
• Attest-command surface → #849. You're right that the same fingerprint code path is reused across kosli attest *, so the contract logically extends there. Trickier because attest commands need auth + server (the contract tests in this PR are pure in-process and need neither). Best handled by extending existing attest test files in a focused PR.

Capture cleanliness contract now covers file, dir, docker, plus the debug-mode invariant. Re-review when you have a moment 🙏