docs(auth): record final review fix round 1 evidence
This commit is contained in:
@@ -0,0 +1,162 @@
|
||||
# Final-review fix round 1 report (sanitized)
|
||||
|
||||
## Verdict
|
||||
|
||||
- Base: `fa499a9bdd37011833691b0f447470d8b7e8a3a6`.
|
||||
- Final frozen source: `10cd66fe6a5b484a4dc569326a228c1c5484a5d4` on
|
||||
`feat/thoth-auth`.
|
||||
- Authentication remediation: **PASS / ADDRESSED**. All four final-review Important findings are
|
||||
resolved relative to the remediation brief.
|
||||
- Terra Minor evidence corrections: **ADDRESSED**.
|
||||
- Branch/release readiness: **FAIL**. The completed exact-source workflow still contains executed
|
||||
baseline clone-contract, LF/Compose, and Linux Docker failures. Unavailable external/manual
|
||||
gates remain **PENDING**.
|
||||
- Source and evidence remain separate commits. No workflow was dispatched from the evidence-only
|
||||
phase.
|
||||
|
||||
## Finding disposition
|
||||
|
||||
| Finding | Disposition | Evidence |
|
||||
|---|---|---|
|
||||
| Important 1 — exhaustive Windows cleanup | RESOLVED | Cleanup now attempts close/delete/validation operations in deterministic order and returns sanitized `ErrUnsafeFile` after aggregating failures. `TestWindowsPrivateRegularCleanupClosesAfterDeleteDispositionFailure` and `TestWindowsClaimCleanupAttemptsLaterOperationsAfterEarlierFailure` cover the non-short-circuit contract. Global no-delete sharing remains unchanged. |
|
||||
| Important 2 — usable native Windows authority | RESOLVED | Owner-only descriptors use the current user SID, protected/non-defaulted DACL semantics, valid NT attributes/access masks, self-relative creation descriptors, and semantic full-control validation. Equal-or-stronger Windows fixture adaptations retain no-delete handles instead of weakening ACL/identity checks. The final native three-package gate passes. |
|
||||
| Important 3 — restore-test deadlock | RESOLVED | Lifecycle-stage release observes the buffered worker outcome, uses a bounded/cancellable release, reports premature completion directly, and never waits indefinitely on `done`. `TestReleaseLifecycleStageReturnsPrematureWorkerOutcome` and the lifecycle-lock terminal-cleanup test are green. |
|
||||
| Important 4 — complete native package gate | RESOLVED | Workflow and remediation plan both use the exact unfiltered command `go test ./internal/safeio ./internal/backup ./internal/authstorage -count=1`. Final logs prove all three packages executed natively. |
|
||||
| Minor — non-executed gate classification | RESOLVED | Non-executed/skipped commands are `NOT_RUN` / `BLOCKED`; `FAIL` is reserved for commands that ran and failed. Historical results remain separately labelled. |
|
||||
| Minor — explicit Windows StageArchive row | RESOLVED | `.artifacts/task-15/automated-gates.json` contains `windows_stagearchive_retained_capability` = PASS, bound to the final source and native backup result. |
|
||||
|
||||
Additional failures exposed by the required unfiltered gate were fixed without narrowing the
|
||||
workflow: Windows secret-bearing archive reservation is protected before use; StageArchive shares
|
||||
one retained root capability across both staged files; claim/consume transitions serialize the
|
||||
complete public validation and retained-handle operation while preserving ACL, hard-link identity,
|
||||
reparse rejection, and no-delete invariants.
|
||||
|
||||
## RED → GREEN record
|
||||
|
||||
### Initial RED
|
||||
|
||||
- Run `32122302381`:
|
||||
https://github.com/mptyl/ThothII/actions/runs/32122302381
|
||||
- Source: `b31b27e5845ffd3adf311429367319beaba263c7`.
|
||||
- Windows job: `95665197885`.
|
||||
- Result: native `safeio`/`backup` failure, including the 10-minute restore lifecycle timeout;
|
||||
`authstorage` was absent from the command. This established the RED for Important 2–4 and the
|
||||
required native authority.
|
||||
- Cleanup failure-injection tests added for Important 1 first exposed the short-circuit behavior
|
||||
before the implementation was changed.
|
||||
|
||||
### Final concurrency RED
|
||||
|
||||
- Run `32140481263`:
|
||||
https://github.com/mptyl/ThothII/actions/runs/32140481263
|
||||
- Source: `b48e9e9189dd0e8083db9bd0378704524e670edb`.
|
||||
- Windows job: `95721724645`.
|
||||
- Native results: backup PASS (`20.757s`), authstorage PASS (`104.180s`), safeio FAIL
|
||||
(`63.502s`). The only failures were:
|
||||
- `TestCanonicalPrivateClaimWaitsForRetainedRemoveOperation`: the concurrent claim returned
|
||||
`false, unsafe file` before retained removal completed;
|
||||
- `TestCanonicalPrivateClaimConsumeHasOneConcurrentWinner`: iteration 8 returned `unsafe file`.
|
||||
- Diagnosis: the process mutex started below `validateClaimPaths`; a concurrent caller could fail
|
||||
while reopening the retained no-delete directory before reaching the lock.
|
||||
|
||||
### GREEN implementation and local gates
|
||||
|
||||
The lock boundary was moved to the three public claim/read/remove APIs, covering validation,
|
||||
relative operation, and handle close. The Unix implementation uses a no-op boundary and retains its
|
||||
existing descriptor-relative semantics.
|
||||
|
||||
Final-source local commands passed:
|
||||
|
||||
```text
|
||||
go test ./internal/safeio ./internal/backup ./internal/authstorage -count=1
|
||||
go test -race ./...
|
||||
go vet ./...
|
||||
go build -o /tmp/thothii-tht-host ./cmd/tht
|
||||
GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build -o /tmp/thothii-tht-windows.exe ./cmd/tht
|
||||
GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go test -c ... ./internal/{safeio,backup,authstorage}
|
||||
```
|
||||
|
||||
- Focused host package times: safeio `8.750s`, backup `8.378s`, authstorage `8.854s`.
|
||||
- Race suite and vet: PASS.
|
||||
- Host CLI: Mach-O arm64; Windows CLI and all three Windows test binaries: PE32+ x86-64.
|
||||
- Cross-compilation remains compile-only and is not used as native proof.
|
||||
|
||||
## Exact-source native certification
|
||||
|
||||
- Run: `32141428407`
|
||||
- URL: https://github.com/mptyl/ThothII/actions/runs/32141428407
|
||||
- Event/status/conclusion: `workflow_dispatch` / `completed` / `failure`.
|
||||
- Head SHA: `10cd66fe6a5b484a4dc569326a228c1c5484a5d4` — exact final source match.
|
||||
- Windows job: `Windows clone and Compose contract`, job `95724751282`:
|
||||
https://github.com/mptyl/ThothII/actions/runs/32141428407/job/95724751282
|
||||
- Native step: `Run native Windows retained-capability tests` — **PASS**.
|
||||
- Exact command: `go test ./internal/safeio ./internal/backup ./internal/authstorage -count=1`.
|
||||
- Native package results:
|
||||
- safeio PASS (`8.230s`);
|
||||
- backup PASS (`5.195s`);
|
||||
- authstorage PASS (`8.383s`).
|
||||
- Job conclusion: `failure` only because the following `Verify Windows clone contract` baseline
|
||||
step failed with a PowerShell `ParserError` at
|
||||
`scripts/test-windows-clone-contract.ps1:208`; `$remoteYaml:` is not delimited before `:`.
|
||||
|
||||
## Remaining branch/release blockers
|
||||
|
||||
| Gate | Classification | Exact outcome |
|
||||
|---|---|---|
|
||||
| Windows native authentication packages | PASS | All three required packages executed on final source. |
|
||||
| Windows clone contract | FAIL / baseline | Executed after native PASS; PowerShell parser error at line 208. |
|
||||
| LF, Compose, docs, and TypeScript | FAIL / baseline CI contract | Job `95724751205`; unified Compose passed, then `test-no-deployment-coupling-scope.sh` failed because `TMPDIR` was unset. Downstream skipped commands are `NOT_RUN` / `BLOCKED`. |
|
||||
| Linux Docker deployment and rollback | FAIL / infrastructure prerequisite | Job `95724751356`; executed smoke stopped because `rg` was unavailable. Cleanup proof passed; no new image manifest was generated. |
|
||||
| Native Windows Docker Desktop/WSL2 startup | NOT_RUN / BLOCKED | Job `95724752028` was skipped by workflow conditions; no Docker/WSL2 command executed. |
|
||||
| Harness/Ruff/other historical baseline gates | FAIL | Retained with their recorded source and results; not rewritten as final-source proof. |
|
||||
| L2, real PSD/manual acceptance, provider readiness | PENDING | Required secrets, identity/access, or provider prerequisites remain unavailable. |
|
||||
|
||||
The historical Docker image manifest remains bound to source
|
||||
`74b062f1a737103524cbe706346cfd65f87cdfd1`; it was not reused as proof for the final source.
|
||||
|
||||
## Principal source commits
|
||||
|
||||
- `cd5f505` — exhaustive cleanup, Windows authority foundation, restore deadlock tests/fix, and
|
||||
complete workflow/plan package command.
|
||||
- `a0e05ad` through `b6396e6` — effective full-control DACL semantics, valid NT attributes/access,
|
||||
self-relative descriptors, retained no-delete fixture ordering, and Windows installation fixture
|
||||
protection.
|
||||
- `824245d` — preserve existing lifecycle ACL trees instead of mutating inherited authority.
|
||||
- `455fffb`, `2d1670e`, `c01482c`, `9fc1a15` — concurrent claim/consume and settled-loss handling.
|
||||
- `6474118` — one retained StageArchive root capability shared across staged files.
|
||||
- `feee4ee` — unified Windows path wrappers on the retained primitive.
|
||||
- `b261dd4` — bounded private-root sharing contention handling.
|
||||
- `b48e9e9` — deterministic retained-remove concurrency regression and claim-operation lock.
|
||||
- `10cd66f` — final lock boundary includes public path validation; frozen source.
|
||||
|
||||
## Files changed
|
||||
|
||||
Source changes relative to the fix-round base:
|
||||
|
||||
- `.github/workflows/deployment.yml`;
|
||||
- `docs/superpowers/plans/2026-08-18-thothii-authentication-remediation.md`;
|
||||
- `tools/tht/internal/authstorage/storage_test.go`;
|
||||
- `tools/tht/internal/backup/{create.go,create_test.go,fixture_security_unix_test.go,fixture_security_windows_test.go,preflight.go,preflight_test.go,preflight_windows_test.go,restore.go,restore_test.go}`;
|
||||
- `tools/tht/internal/safeio/{claim_unix.go,claim_windows.go,claim_windows_test.go,files.go,files_test.go,private_root_windows.go,private_windows.go,private_windows_test.go}`.
|
||||
|
||||
Evidence/status changes are restricted to:
|
||||
|
||||
- `.artifacts/task-15/automated-gates.json`;
|
||||
- `.superpowers/sdd/2026-08-18-thothii-authentication-remediation/task-4-report.md`;
|
||||
- `.superpowers/sdd/2026-08-18-thothii-authentication-remediation/fix-round-1-report.md`;
|
||||
- `.superpowers/sdd/2026-08-16-thothii-authentication/task-15-report.md`;
|
||||
- `PROJECT_STATE.md`.
|
||||
|
||||
Machine-readable evidence SHA-256:
|
||||
`5c110b7b2607693de078def441b10290c5a29024c83b7e5a0ced894b72b7507f`.
|
||||
|
||||
## Git and protection status
|
||||
|
||||
- The evidence commit contains only the five evidence/status files listed above; no source is
|
||||
changed after frozen source `10cd66fe6a5b484a4dc569326a228c1c5484a5d4`.
|
||||
- After the evidence commit and push, the intended status is synchronized
|
||||
`feat/thoth-auth...origin/feat/thoth-auth` with only protected untracked `.playwright-cli/` and
|
||||
`.thothctl/`.
|
||||
- `AGENTS.md`, `CLAUDE.md`, and `docs/agents/` are untouched. No generated `tools/tht/tht` exists.
|
||||
- Evidence commit SHA is reported externally after commit creation because a commit cannot contain
|
||||
its own final hash.
|
||||
Reference in New Issue
Block a user