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

15 KiB
Raw Blame History

Task 2 report — protected atomic registry

Scope and commit

  • Commit: 541ef45 feat: add protected DWH credential registry
  • Committed files only:
    • tools/dwh-auth/internal/securefile/securefile_linux.go
    • tools/dwh-auth/internal/securefile/securefile_linux_test.go
    • tools/dwh-auth/internal/registry/store.go
    • tools/dwh-auth/internal/registry/store_test.go
  • No server, Nginx, systemd, Docker stack, real registry, secrets, or legacy ThothII files were read or changed. Tests use t.TempDir and synthetic record digests only.

TDD evidence

All Go commands ran in the required official golang:1.26.5 container with only this linked worktree bind-mounted at /work. The container image reports go version go1.26.5 linux/amd64.

RED

Before either Task 2 production file existed, the focused command was run inside the container:

go test ./internal/securefile ./internal/registry -count=1

It failed non-zero for the expected absent implementation symbols, including undefined: OpenDir, undefined: ReadSecret, undefined: Open, undefined: State, undefined: PublicRecord, and undefined: Store.

GREEN

After the minimal implementation and formatting:

go test ./internal/securefile ./internal/registry -count=1

Result:

ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry

Race verification

The required race command completed successfully:

go test -race ./internal/securefile ./internal/registry -count=1

Result:

ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile 1.027s
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 1.179s

Additional scoped verification:

go vet ./internal/securefile ./internal/registry
go test ./... -count=1
git diff --cached --check

The Task 2 vet command completed with no findings; all four DWH-auth packages passed the full module test run; the staged-diff check completed with no output.

Delivered behavior

  • securefile is Linux-only and traverses absolute paths through descriptor-anchored syscall.Open/Openat calls with O_NOFOLLOW|O_CLOEXEC; protected roots, child directories, records, and secret files are regular/directories only and are checked against Lstat after Fstat.
  • Protected reads reject special, group-writable, or world-writable modes, cap record reads at 4096 bytes, read at most one extra byte, and reject file-size changes or short/partial reads. Secret ingress additionally requires exact 0600.
  • Secret output uses O_CREAT|O_EXCL|O_NOFOLLOW, exact 0600, and an absolute protected parent.
  • registry.Open creates protected active and revoked subdirectories under an existing safe root. Record enumeration rejects unexpected entries, unsafe files, symlinks, oversized files, bad filenames, malformed JSON, unknown JSON fields, duplicate JSON fields, and trailing JSON.
  • Add validates Task 1 records, writes canonical JSON plus one newline through an exclusive temporary file, sets final mode 0640, syncs the file, renames under a protected per-root writer lock, then syncs the directory.
  • Revoke writes and syncs a valid revoked record before unlinking and syncing the active record. Find checks revoked first; List resolves an active/revoked overlap to the revoked public record. FindLegacy scans fail-closed and permits only the reserved legacy record state.
  • PublicRecord deliberately omits secret_sha256; the redaction is regression-tested.

Security-test coverage

  • protected normal files and canonical record publication;
  • symlinked roots, registry directories, records, and secret input;
  • unsafe root/directory/record/secret modes;
  • bounded/oversized record input;
  • unknown, duplicate, trailing, and partial JSON;
  • filename mismatch and multiple legacy-record integrity failures;
  • revoked-state precedence when both active and revoked files exist;
  • concurrent adds and concurrent reads during revocation, including the race detector.

Self-review

Reviewed all syscall, path, mode, and error paths after the final race run:

  • Directory traversal never follows a supplied component; later operations use retained directory descriptors, not re-opened untrusted prefixes.
  • Fstat validates the opened object and Lstat must identify the same inode/device; the direct child name grammar refuses separators, dot components, and NUL.
  • File validation occurs before and after reads; mode/type/size checks fail closed. Directory listing obtains a fresh openat(dirfd, ".") descriptor so scans do not share a mutable directory offset.
  • Writer serialization protects the check-then-rename no-replace sequence. Failed temporary cleanup leaves an unexpected entry that later scans reject rather than silently accepting it.
  • State-specific validation rejects revocation metadata in active records and requires it in revoked records. Revoked files are consulted before active files so interruption after revoked publication cannot reactivate a credential.
  • All functionality uses only Go standard-library packages and Linux syscall; no CGO, SQLite, or third-party module was added.

Concerns

  • The optional whole-module go vet ./... reports a pre-existing Task 1 test warning at internal/credential/credential_test.go:86 (append with no variadic values). The identical line is present in approved HEAD 1e82fd3, outside this task’s authorized files. Focused Task 2 vet passes, and all module tests pass.
  • The official image's login shell resets PATH and hides /usr/local/go/bin; all evidence uses direct go/gofmt container entrypoints, which preserves the image’s Go 1.26.5 environment.
  • The generic apply_patch helper intermittently failed before file access with a sandbox network namespace error. Exact scoped corrections were applied through the shared worktree workflow; this did not affect the final staged file set or verification evidence.

Review remediation — 2026-08-21

Scope and fix commit

  • Review-fix commit: 971a0e6 fix: harden DWH credential registry reads.
  • Committed files only:
    • tools/dwh-auth/internal/registry/store.go
    • tools/dwh-auth/internal/registry/store_test.go
  • The separate Task 1 vet correction is the independent preceding commit d2415b5; it is not included in this Task 2 fix commit. No filesystem primitive, server, Nginx, service, registry, secret, Docker stack, or legacy ThothII file was changed.

Strict TDD evidence

All commands again used the official golang:1.26.5 image with only this linked worktree mounted at /work.

RED

The first focused command was run after the new regression tests and before production changes:

go test ./internal/securefile ./internal/registry -count=1

It failed as intended. The three case-variant aliases (SECRET_SHA256, Secret_SHA256, and Schema_Version) were accepted; past expiry returned active records from both Find and FindLegacy; a revocation snapshot let readers return active data before publication; a temporary file let List, Check, and FindLegacy observe false integrity failures; and the original concurrent-read regression observed ErrNotFound during revocation.

The deterministic exact-expiry test was then added before the clock implementation. Its focused run failed as intended with:

internal/registry/store_test.go:572:10: store.now undefined

GREEN and verification

After the minimum implementation and gofmt:

go test ./internal/securefile ./internal/registry -count=1
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile 0.014s
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 0.684s

go test -race ./internal/securefile ./internal/registry -count=1
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile 1.022s
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 1.711s

go vet ./internal/securefile ./internal/registry

The focused vet output was empty (success). Additional final checks passed:

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

go vet ./...
go test -race ./internal/registry -count=10
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 8.127s
git diff --cached --check

Remediated security invariants

  • Find and FindLegacy now deny an active record when ExpiresAt <= now.UTC(), returning the existing non-disclosing ErrNotFound. The unexported per-Store now function is the minimal deterministic clock seam; past, exact-equality, and future cases are covered for v1 and legacy records. A revoked record is still consulted before expiry and therefore remains authoritative.
  • One per-Store sync.RWMutex creates an in-process consistent snapshot. Add and Revoke hold it exclusively for their full writer-lock lifetime, including temporary-file publication and revoked-then-active removal. Find, FindLegacy, List, and Check hold a shared lock; their bodies delegate only to unlocked helpers, preventing nested-lock deadlocks. Close also takes the exclusive lock before closing descriptors.
  • Deterministic regression tests hold the writer path at the revocation publication/unlink and temporary-file stages. They prove public readers wait, then see either the final revoked state or a clean directory, eliminating the Names-to-load/unlink and temporary-entry false failures within the Store contract.
  • Before struct decoding, the outer record JSON object now requires exactly spelled keys from the schema allowlist and rejects duplicate literal keys. Decoder.DisallowUnknownFields, recursive duplicate detection, trailing-value rejection, record validation, filename matching, no-follow reads, modes, and durability ordering remain intact.

Self-review and concerns

  • Reviewed the new lock boundaries, error returns, revoked-first ordering, clock fallback, JSON-token consumption, and every unchanged securefile syscall/path/mode boundary. The change adds only standard-library sync; it does not relax existing fail-closed behavior.
  • The synchronized snapshot is intentionally per Store, matching the requested in-process contract. The existing protected advisory lock continues to serialize writers across Store instances/processes; no cross-process reader snapshot is claimed by this fix.
  • The historical whole-module vet concern in the original Task 2 report is now resolved by the independent Task 1 commit d2415b5; complete module vet passes in the final evidence above.

Cross-Store snapshot remediation — 2026-08-21

Scope and TDD evidence

This third Task 2 fix wave changes only the protected lock primitive and registry snapshot code:

  • tools/dwh-auth/internal/securefile/securefile_linux.go
  • tools/dwh-auth/internal/securefile/securefile_linux_test.go
  • tools/dwh-auth/internal/registry/store.go
  • tools/dwh-auth/internal/registry/store_test.go

All commands used the official golang:1.26.5 image with only this linked worktree mounted at /work.

The test-only red patch initially tried to inspect the unexported securefile.Dir.fd through the registry package and therefore did not compile. That assertion was removed without production changes: the registry tests still create writer Store A and reader Store B through two independent Open(root) calls, while the securefile test proves separate descriptors directly in its own package. The subsequent behavioral RED run, before the production change, was:

go test ./internal/securefile ./internal/registry -count=1
FAIL TestLockSharedAllowsReadersAndBlocksExclusiveWriter: Dir lacks shared advisory locking
FAIL TestCrossStoreReadersWaitAcrossRevokePublicationAndUnlink:
  Find, List, Check, and FindLegacy completed during Store A's revocation snapshot
FAIL TestCrossStoreScanReadersWaitForWriterTemporaryFile:
  Store B's List, Check, and FindLegacy observed `.tmp-regression`

Delivered synchronization contract

  • securefile.Dir.LockShared now acquires LOCK_SH on the same protected, no-follow, exact-0600 root lock file used by Lock, which continues to acquire LOCK_EX. The lock file is still opened/created, mode-validated, inode-checked, and closed through the existing Linux syscall path.
  • Every public snapshot reader (Find, FindLegacy, List, and Check) takes its Store RLock, then a shared advisory lock on root .writer.lock, and retains both through the whole revoked/active lookup or directory scan/load. Add and Revoke retain Store Lock, then the same root lock under LOCK_EX, over their full operation.
  • The lock order is universally Store mutex then root advisory lock. Public methods delegate only to unlocked helpers, so neither reader nor writer paths recursively acquire the Store mutex. Close retains its exclusive Store mutex, preventing descriptor closure from racing any locked reader or writer.
  • Revoked-first precedence, expiry denial, exact JSON validation, no-follow checks, record modes, temporary-file durability, and all previous behavior remain unchanged.

GREEN and repeated verification

go test ./internal/securefile ./internal/registry -count=1
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile 0.019s
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 0.711s

go test -race ./internal/securefile ./internal/registry -count=1
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile 1.032s
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 1.748s

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

go test -race ./internal/registry \
  -run 'TestCrossStoreReadersWaitAcrossRevokePublicationAndUnlink|TestCrossStoreScanReadersWaitForWriterTemporaryFile' -count=20
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/registry 9.880s

go test -race ./internal/securefile \
  -run TestLockSharedAllowsReadersAndBlocksExclusiveWriter -count=20
ok github.com/aritmolab/thothii/tools/dwh-auth/internal/securefile 1.063s

git diff --check

Both vet commands and the whitespace check produced no output. The securefile regression opens three protected directory descriptors, proves they are distinct, permits two independent shared holders, proves a third descriptor cannot take LOCK_EX|LOCK_NB, then proves exclusive acquisition succeeds after shared release. The registry regressions deterministically block Store B readers while Store A holds the exclusive root lock and verify only final revoked/clean states afterward.

Self-review and concerns

  • Reviewed lock creation/reopen races, no-follow flags, exact lock-file mode validation, lock release, descriptor lifetime, lock ordering, error wrapping, and the unlocked-helper call graph. No public reader invokes another public reader or writer while holding a Store lock.
  • Advisory synchronization necessarily covers cooperating registry Store instances/processes; arbitrary external filesystem mutation remains fail-closed through the existing integrity checks rather than being silently accepted.
  • No known concerns within the registry's cooperating-process contract.