Files
ThothII/.superpowers/sdd/task-3-report.md
T

6.0 KiB
Raw Blame History

Task 3 report — secret-safe dwh-auth administrative CLI

Scope

  • Added tools/dwh-auth/internal/command/command.go, its command tests, and tools/dwh-auth/cmd/dwh-auth/main.go.
  • The CLI accepts only the frozen Task 3 grammar: key create/import/list/status/revoke, registry check, and the reserved serve invocation.
  • Existing Task 1–2 APIs are consumed without modifying their files.
  • No server, Nginx, systemd, Compose, portable tht, real registry, real secret, or legacy stack was accessed or changed.

TDD evidence

Tests were written before Run existed. In the official golang:1.26.5 container, mounted against only the dedicated worktree, the focused RED run was:

go test ./internal/command -count=1
internal/command/command_test.go:214:10: undefined: Run
FAIL

After implementation and formatting:

go test ./internal/command -count=1
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/command

go test -race ./internal/command -count=1
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/command

go test ./... -count=1
ok internal/command, internal/credential, internal/record, internal/registry, internal/securefile

go test -race ./... -count=1
ok internal/command, internal/credential, internal/record, internal/registry, internal/securefile

go vet ./...

Contract coverage

  • Create generates the Task 1 canonical credential, writes it once through the protected exclusive 0600 output primitive, syncs/closes it before registry publication, and emits only created key_id=... installation_id=... output=....
  • Existing output is never overwritten. Publication failure attempts compensating removal; cleanup uncertainty returns exit 4 and reports only the output path.
  • Legacy import requires --legacy-raw, the reserved legacy-shared installation ID, and an absolute exact-0600 source. It verifies the opaque value, never changes its source, and stores only its digest.
  • List/status expose PublicRecord data only; JSON is written as pristine JSON with no digest. Revoke requires a non-empty reason and reports only its public key ID.
  • Relative paths, malformed/unknown flags, duplicate options, invalid IDs/metadata/expiry, and missing required values return exit 2. Missing status/revoke keys return exit 3. Registry/filesystem/integrity failures return exit 4.
  • Diagnostics are fixed redacted strings. Tests use a sentinel secret and assert it is absent from stdout/stderr, list/status/check JSON, import output, and unsafe/integrity failures.

Secret-redaction evidence

The command never prints credential contents or digests. It does not use environment fallback, interactive stdin, or flag diagnostics that echo argument values. The sentinel appears only in synthetic temporary test input and an integrity-fixture file; all command output assertions confirm it is absent. The registry’s existing PublicRecord contract omits secret_sha256.

Concerns

  • serve is grammar-reserved and returns a redacted exit-4 unavailable response; Task 4 owns the Unix-socket service implementation and will wire this dispatch.
  • The earlier concern about UTF-8 metadata hardening is superseded by 055dcab: metadata now rejects invalid UTF-8 and Unicode controls before generation/output. The nil-safe cleanup note remains non-blocking and outside this review wave.

Review-fix wave

Review findings were addressed in separate commit 055dcab. Regression tests were added first. The focused RED run in the official Go 1.26.5 container failed on intentionally absent seams:

undefined: nowUTC
undefined: addRecord
undefined: closeStore
FAIL github.com/aritmolab/thothii/tools/dwh-auth/internal/command

The fix rejects embedded canonical v1 credentials in description/revocation reason without echoing metadata, validates UTF-8/Unicode controls and expiry against one captured UTC creation time before generation/output, reserves exactly serve --registry-root ABS --socket ABS, and makes publication cleanup depend on a definitive registry lookup. Output is retained after publication or close ambiguity, with path-only recovery guidance.

Review-fix verification in Go 1.26.5:

go test ./internal/command -count=1       PASS
go test -race ./internal/command -count=1 PASS
go test ./... -count=1                    PASS
go test -race ./... -count=1              PASS
go vet ./...                              PASS
git diff --check                          PASS

New tests cover synthetic canonical credentials embedded with prefix/suffix, invalid UTF-8, C1 Unicode controls, past/equal/future expiry, exact serve ordering, deterministic pre-/post-publication and close-failure seams, and sentinel absence from stdout/stderr/list/status JSON.

Cleanup snapshot review-fix wave

The second re-review added two regression tests before implementation. The RED run in the official Go 1.26.5 container showed the old Find proof incorrectly treated both cases as cleanup-safe:

FAIL TestCreateRetainsOutputWhenSnapshotFindsUnrelatedIntegrityFailure
  corrupt snapshot result = (4, "", "integrity failure\n")
FAIL TestCreateRetainsOutputWhenFailedPublicationRecordIsExpired
  expired publication result = (4, "", "integrity failure\n")

Commit b1079bd fix: retain DWH key output on ambiguous publication replaces the Find proof with a complete Store.List() snapshot. It removes generated output only when the snapshot succeeds, the generated key ID is absent, and Store.Close() succeeds. Any unrelated integrity error, active/revoked/expired record, or close error retains the output and emits only path-based recovery guidance. The clean pre-publication failure path still removes the output.

Final cleanup-wave verification in Go 1.26.5:

go test ./internal/command -count=1       PASS
go test -race ./internal/command -count=1 PASS
go test ./... -count=1                    PASS
go test -race ./... -count=1              PASS
go vet ./...                              PASS
git diff --check                          PASS