From 3c6ddfaef1516aa4f67984fd367a5585a04ca104 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 18 Aug 2026 10:10:37 +0200 Subject: [PATCH] docs(auth): plan important finding remediation --- ...8-18-thothii-authentication-remediation.md | 910 ++++++++++++++++++ 1 file changed, 910 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-18-thothii-authentication-remediation.md diff --git a/docs/superpowers/plans/2026-08-18-thothii-authentication-remediation.md b/docs/superpowers/plans/2026-08-18-thothii-authentication-remediation.md new file mode 100644 index 00000000..3bab5f93 --- /dev/null +++ b/docs/superpowers/plans/2026-08-18-thothii-authentication-remediation.md @@ -0,0 +1,910 @@ +# ThothII Authentication Important-Finding Remediation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Close the three remaining Important authentication findings on `feat/thoth-auth`: POSIX local-registry ownership, retained-capability restore staging, and handle-relative Windows claim removal. + +**Architecture:** POSIX registry reads bind every path and descriptor metadata observation to one validated effective UID. Restore staging creates one random private regular file directly below a retained `safeio.PrivateDirectoryHandle`, keeps that directory capability for the entire stream and cleanup lifecycle, and never authorizes cleanup through a pathname. Windows canonical claim removal reuses the already handle-relative `PrivateDirectoryHandle.RemoveClaim` implementation instead of reopening and deleting absolute paths. + +**Tech Stack:** Node.js `24.16.0`, TypeScript, Fastify auth services, Vitest, Go `1.26.5`, `golang.org/x/sys`, native Windows GitHub Actions, Docker Compose release smokes. + +**Spec:** `docs/superpowers/specs/2026-08-16-thothii-authentication-design.md` + +**Review basis:** `.superpowers/sdd/2026-08-16-thothii-authentication/task-15-report.md` and the Task 15 section of `PROJECT_STATE.md` at finding baseline `178113a`. + +## Global Constraints + +- The only host CLI is `tht`; do not add another executable or revive `thothctl`. +- Production modes are `local` and `oidc`; `none` and `mock` are development/test only, while `upstream` remains a deprecated migration adapter. +- Authentik is the first-release certified OIDC provider; the browser OIDC protocol layer must not contain Authentik-specific login logic. +- The ID token must contain direct claim `groups: string[]`; absent, malformed, indirect, or overage claims fail closed. +- Only configured groups are checked and mapped. Unmapped provider groups are ignored silently and never produce a warning. +- Every configured group must be proven to exist through the configured group-catalog adapter; first release provides `authentik`. +- Local passwords use Argon2id v19 with `m=65536,t=3,p=1`, 16-byte random salt, and 32-byte output. +- A remembered local session has a seven-day idle timeout and thirty-day absolute timeout and survives browser/backend restarts. +- No raw session cookie, password, CSRF token, OIDC token, client secret, Authentik API token, or password hash may enter logs or diagnostic output. +- Browser authentication uses an opaque `HttpOnly`, `SameSite=Lax`, path-scoped cookie; `Secure` is conditional on an HTTPS public URL so loopback HTTP remains functional. +- Every cookie-authenticated state-changing route requires a CSRF token and same-origin browser checks. +- Authentication configuration is installation-global, but static checks appear in workspace validation and live checks appear in workspace connection tests. +- Workspace document content remains in its workspace language; application chrome and new authentication UI strings are English. +- JSON CLI stdout is pristine. Prompts, progress, and human guidance go to stderr. +- Node is exactly `24.16.0`; Docker uses `sha256:40ad9f3064e67d6860b4bc3fe1880b2953934fd6320ada990e45fe0efa6badd7`. +- Existing session artifacts, workflow persistence, workspace ownership, and Pi RPC behavior must not change. +- Preserve unrelated user changes and the existing untracked `.playwright-cli/` and `.thothctl/` paths. + +--- + +## Execution Contract + +Run the tasks in numerical order. Tasks 2 and 3 both touch `safeio`; they must not be parallelized. + +| Task | Implementer | Reasoning | Mandatory reviewer | +|---|---|---:|---| +| 1 | fresh Terra | high | fresh Terra, ultra | +| 2 | fresh Terra | ultra | fresh Terra, ultra | +| 3 | fresh Terra | ultra | fresh Terra, ultra | +| 4 | fresh Terra | max | fresh Terra, ultra, whole-branch scope | + +For every task: + +1. Dispatch a new Terra implementer that has not worked on any earlier authentication task. +2. Give it only this plan, the design spec, `global-constraints.md`, the Task 15 report, and the current task's relevant files. +3. Require RED-GREEN TDD, focused tests before broad tests, and one task-scoped commit. +4. Dispatch a different, fresh Terra reviewer after the commit. The reviewer is read-only and checks the task diff, tests, security invariants, and scope. +5. Do not start the next numbered task unless the reviewer reports `CLEAN` for Critical/Important issues. A requested fix gets a new Terra implementer and, after its commit, another fresh Terra reviewer. +6. Three review/fix cycles are the hard breaker for one task. If the third review is not `CLEAN`, stop and record the exact blocker; do not weaken an invariant or recast the finding as accepted risk. + +The review of Task 4 is also the final whole-branch review over `178113a..HEAD`. A native-Windows test that was only cross-compiled is `PENDING`, never `PASS`. + +## Scope Boundaries and Design Choices + +- Do not change login, role, session, CSRF, OIDC, group-mapping, or CLI behavior except where a test proves an unintended dependency on one of the three fixes. +- Do not introduce a second filesystem abstraction. Extend `safeio.PrivateDirectoryHandle` with one streaming-file method and reuse its existing handle-relative `RemoveRegular` and `RemoveClaim` operations. +- Do not create a temporary staging subdirectory. A unique archive file directly under the retained `restore-staging` directory removes an unnecessary cleanup capability and still allows candidate and recovery archives to coexist. +- Do not test POSIX ownership by requiring root or calling `chown`. Vitest must override returned metadata while the production code continues to use the real `node:fs` API. +- Do not treat the known Ruff, MkDocs, installation-wording, Pi model-policy, deployment-coupling, L2, PSD, provider-readiness, or Windows-Docker statuses as fixed by this plan. Re-run and report them honestly where the release matrix requires them. +- Do not put credentials, internal endpoints, provider identities, registry names, raw paths containing secrets, or unsanitized Docker output into retained evidence. + +## Target File Structure + +### Backend ownership boundary + +- `backend/src/auth/local-registry.ts` — validates one effective UID across every POSIX `lstat` and `fstat` observation. +- `backend/test/local-registry.test.ts` — injects foreign UID metadata at path-file, descriptor-file, path-directory, and descriptor-directory boundaries. + +### Retained restore staging + +- `tools/tht/internal/safeio/private_root.go` — adds the cross-platform streaming regular-file interface. +- `tools/tht/internal/safeio/private_root_unix.go` — creates an exclusive owner-private stream with `openat` under the retained directory descriptor. +- `tools/tht/internal/safeio/private_root_windows.go` — creates an exclusive owner-private stream with NT `RootDirectory`-relative access. +- `tools/tht/internal/safeio/files_test.go` — proves streaming creation and handle-relative removal through the public directory capability. +- `tools/tht/internal/backup/preflight.go` — owns the retained staging directory capability until `stagedArchive.Close` finishes. +- `tools/tht/internal/backup/preflight_test.go` — updates lifecycle, failure-cleanup, and same-filesystem assertions for direct-child staging. +- `tools/tht/internal/backup/preflight_unix_test.go` — proves cleanup targets the pinned Unix directory after a lexical ancestor swap. +- `tools/tht/internal/backup/preflight_windows_test.go` — proves native Windows blocks the swap while the no-delete directory handle is retained. +- `.github/workflows/deployment.yml` — executes the security tests natively in the existing `windows-clone` job. + +### Windows claim removal + +- `tools/tht/internal/safeio/claim_windows.go` — delegates canonical removal to a retained `PrivateDirectoryHandle`. +- `tools/tht/internal/safeio/claim_windows_test.go` — proves native Windows ancestor replacement cannot redirect deletion. + +### Certification evidence + +- `.artifacts/task-15/automated-gates.json` — sanitized machine-readable results bound to the final source commit. +- `.artifacts/task-15/unified-docker-images.json` — exact final-smoke image identities without registry names. +- `.superpowers/sdd/2026-08-16-thothii-authentication/task-15-report.md` — remediation and release-gate narrative. +- `PROJECT_STATE.md` — current status, exact source SHA, review verdict, PASS/FAIL/PENDING matrix. + +--- + +### Task 1: Require Effective-UID Ownership for the POSIX Local Registry + +**Files:** +- Modify: `backend/src/auth/local-registry.ts:53-193,229-250` +- Modify: `backend/test/local-registry.test.ts:1-190` + +**Interfaces:** +- Produces: `runtimeOwner(): number`, which returns a non-negative safe integer from `process.geteuid()` or throws `local_user_registry_invalid`. +- Produces: `fileMetadata(info: Stats, owner: number): FileIdentity` and `directoryMetadata(info: Stats, owner: number): DirectoryIdentity`. +- Produces: `registryIdentity(path: string, owner: number)`, `readBounded(path: string, owner: number)`, and `load(path: string, owner: number)`. +- Preserves: `createLocalUserRegistry(usersPath, options)` and every public `LocalUserRegistry` method. + +- [ ] **Step 1: Add a controllable metadata wrapper to the existing Vitest file** + +Place a hoisted state object before the imports from `local-registry.ts`, mock only `node:fs` metadata calls, and leave all real filesystem mutation/open/read calls intact: + +```ts +type OwnershipObservation = + | "file-lstat" + | "file-fstat" + | "directory-lstat" + | "directory-fstat"; + +const ownershipOverride = vi.hoisted(() => ({ + observation: undefined as OwnershipObservation | undefined, + uid: undefined as number | undefined, +})); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + const replaceUid = (value: T, uid: number): T => new Proxy(value, { + get(target, property) { + if (property === "uid") return uid; + const member = Reflect.get(target, property, target); + return typeof member === "function" ? member.bind(target) : member; + }, + }); + const maybeReplace = ( + value: T, + source: "lstat" | "fstat", + ): T => { + const kind = value.isDirectory() ? "directory" : "file"; + return ownershipOverride.observation === `${kind}-${source}` && ownershipOverride.uid !== undefined + ? replaceUid(value, ownershipOverride.uid) + : value; + }; + return { + ...actual, + lstatSync(path: import("node:fs").PathLike) { + return maybeReplace(actual.lstatSync(path), "lstat"); + }, + fstatSync(fd: number) { + return maybeReplace(actual.fstatSync(fd), "fstat"); + }, + }; +}); +``` + +Reset both fields in the existing `afterEach` before deleting fixture files so one case cannot contaminate the next. + +- [ ] **Step 2: Add four foreign-owner rejection cases** + +Add a POSIX-only table test after the existing unsafe-metadata test: + +```ts +test.runIf(process.platform !== "win32").each([ + "file-lstat", + "file-fstat", + "directory-lstat", + "directory-fstat", +] as const)("rejects foreign ownership at the %s boundary", async (observation) => { + const fixture = writeRegistry(registryYaml(userYaml())); + const owner = process.geteuid(); + ownershipOverride.observation = observation; + ownershipOverride.uid = owner === 0 ? 1 : owner - 1; + + await expectInvalid( + createLocalUserRegistry(fixture.path).findByUsername("admin"), + ["admin", passwordHash, fixture.path], + ); +}); +``` + +Add the invalid-effective-UID case explicitly: + +```ts +test.runIf(process.platform !== "win32")("fails closed when the effective UID is invalid", async () => { + const fixture = writeRegistry(registryYaml(userYaml())); + const getuid = vi.spyOn(process, "geteuid").mockReturnValue(-1); + try { + await expectInvalid( + createLocalUserRegistry(fixture.path).findByUsername("admin"), + ["admin", passwordHash, fixture.path], + ); + } finally { + getuid.mockRestore(); + } +}); +``` + +Keep the existing Windows bridge test green to prove native Windows does not enter the POSIX owner path. + +- [ ] **Step 3: Run the focused tests and verify RED** + +```bash +cd backend +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx vitest run test/local-registry.test.ts -t "foreign ownership|effective UID" +``` + +Expected: all new POSIX ownership cases fail because `uid` is not checked; the Windows-only bridge case remains excluded or passing according to host platform. + +- [ ] **Step 4: Add one fail-closed effective-UID resolver** + +Use the same invariant already present in `backend/src/auth/config.ts`, but keep this patch local to the registry to avoid unrelated config refactoring: + +```ts +function runtimeOwner(): number { + if (process.platform === "win32" || typeof process.geteuid !== "function") throw invalid(); + const owner = process.geteuid(); + if (!Number.isSafeInteger(owner) || owner < 0) throw invalid(); + return owner; +} +``` + +Add `uid: number` to both `FileIdentity` and `DirectoryIdentity`; include it in `sameFileIdentity` and `sameDirectoryIdentity`. + +- [ ] **Step 5: Thread one owner through every POSIX observation** + +Change the metadata helpers so every path and descriptor stat uses the same owner captured at the start of `currentPosix()`: + +```ts +function fileMetadata(info: Stats, owner: number): FileIdentity { + if (!info.isFile() || info.uid !== owner || info.nlink !== 1 || (info.mode & 0o7777) !== 0o600) { + throw invalid(); + } + if (info.size < 0 || info.size > MAX_USERS_YAML_BYTES) throw invalid(); + return { dev: info.dev, ino: info.ino, uid: info.uid, size: info.size, mtimeMs: info.mtimeMs }; +} + +function directoryMetadata(info: Stats, owner: number): DirectoryIdentity { + if (!info.isDirectory() || info.uid !== owner || (info.mode & 0o7777) !== 0o700) throw invalid(); + return { dev: info.dev, ino: info.ino, uid: info.uid, mode: info.mode & 0o7777 }; +} +``` + +`directoryIdentity`, `registryIdentity`, `readBounded`, and `load` must all require `owner`. `currentPosix` captures it once and passes it through both cache probes and both load attempts: + +```ts +function currentPosix(): LocalUserRecord[] { + try { + const owner = runtimeOwner(); + const before = registryIdentity(usersPath, owner); + if (cached && sameIdentity(cached.identity, before)) return cached.records; + for (let attempt = 0; attempt < 2; attempt += 1) { + const loaded = load(usersPath, owner); + if (sameIdentity(loaded.identity, registryIdentity(usersPath, owner))) { + cached = loaded; + return loaded.records; + } + } + } catch { + throw invalid(); + } + throw invalid(); +} +``` + +Do not call `runtimeOwner()` inside individual metadata helpers: changing the expected owner between observations would weaken the snapshot invariant. + +- [ ] **Step 6: Run focused and complete Node 24 gates** + +```bash +cd backend +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx vitest run test/local-registry.test.ts +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx tsc --noEmit -p . +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx vitest run +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npm run build +``` + +Expected: focused and full backend suites pass; no error contains a user name, hash, or registry path. + +- [ ] **Step 7: Commit the ownership remediation** + +```bash +git add backend/src/auth/local-registry.ts backend/test/local-registry.test.ts +git commit -m "fix(auth): require local registry ownership" +``` + +**Mandatory Terra review gate:** Review the task commit against the finding. Confirm all eight POSIX metadata observations in `readBounded` plus the file-and-directory observations in both cache probes flow through owner-checking helpers, invalid/missing `geteuid` fails closed, and the Windows bridge remains unchanged. Verdict must be `CLEAN` before Task 2. + +--- + +### Task 2: Retain the Restore-Staging Capability Through Stream and Cleanup + +**Files:** +- Modify: `tools/tht/internal/safeio/private_root.go:8-23` +- Modify: `tools/tht/internal/safeio/private_root_unix.go:162-202` +- Modify: `tools/tht/internal/safeio/private_root_windows.go:512-539` +- Modify: `tools/tht/internal/safeio/files_test.go` +- Modify: `tools/tht/internal/backup/preflight.go:97-103,261-374` +- Modify: `tools/tht/internal/backup/preflight_test.go:60-101,403-488` +- Modify: `tools/tht/internal/backup/preflight_unix_test.go` +- Modify: `tools/tht/internal/backup/preflight_windows_test.go` +- Modify: `.github/workflows/deployment.yml:125-153` + +**Interfaces:** +- Produces: `PrivateDirectoryHandle.CreateRegularFile(name string) (*os.File, bool, error)`. +- Contract: `created=false, file=nil, err=nil` means a safe existing leaf prevented exclusive creation; any unsafe existing leaf returns `ErrUnsafeFile`. +- Contract: a successful caller owns the returned `*os.File`; the directory handle remains the sole cleanup authority through `RemoveRegular(name)`. +- Produces: `newStagingArchiveName() (string, error)` returning `archive-<32 lowercase hex>.zip` from 16 cryptographically random bytes. +- Produces: `stagedArchive{file *os.File, parent safeio.PrivateDirectoryHandle, name string, path string}`; `path` is diagnostic only and is never passed to a remove operation. + +- [ ] **Step 1: Add a failing cross-platform streaming-capability test** + +In `tools/tht/internal/safeio/files_test.go`, open a private directory, call the new method, stream bytes, rewind/read them, and remove the leaf through the same directory capability: + +```go +func TestPrivateDirectoryCreatesAndRemovesStreamingRegularFile(t *testing.T) { + temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) + if err != nil { + t.Fatal(err) + } + root, err := os.MkdirTemp(temporaryRoot, "tht-safeio-stream-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + if err := ProtectPrivateDirectory(root); err != nil { + t.Fatal(err) + } + directory, found, err := OpenPrivateDirectory(root, true) + if err != nil || !found { + t.Fatalf("OpenPrivateDirectory() = found %v, err %v", found, err) + } + defer directory.Close() + + file, created, err := directory.CreateRegularFile("archive-stream.zip") + if err != nil || !created || file == nil { + t.Fatalf("CreateRegularFile() = file %v, created %v, err %v", file, created, err) + } + if _, err := file.Write([]byte("private archive")); err != nil { + t.Fatal(err) + } + if err := file.Sync(); err != nil { + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + removed, err := directory.RemoveRegular("archive-stream.zip") + if err != nil || !removed { + t.Fatalf("RemoveRegular() = removed %v, err %v", removed, err) + } +} +``` + +Add an adjacent case that pre-creates a safe `0600` leaf and expects `(nil, false, nil)`, then replaces a different leaf with a symlink/reparse point and expects `ErrUnsafeFile`: + +```go +if created, err := directory.CreateRegular("existing.zip", []byte("existing")); err != nil || !created { + t.Fatalf("CreateRegular(existing.zip) = created %v, err %v", created, err) +} +if file, created, err := directory.CreateRegularFile("existing.zip"); err != nil || created || file != nil { + t.Fatalf("CreateRegularFile(existing.zip) = file %v, created %v, err %v", file, created, err) +} +if created, err := directory.CreateRegular("target.zip", []byte("target")); err != nil || !created { + t.Fatalf("CreateRegular(target.zip) = created %v, err %v", created, err) +} +testsupport.SymlinkOrSkip(t, filepath.Join(root, "target.zip"), filepath.Join(root, "linked.zip")) +if file, created, err := directory.CreateRegularFile("linked.zip"); !errors.Is(err, ErrUnsafeFile) || created || file != nil { + t.Fatalf("CreateRegularFile(linked.zip) = file %v, created %v, err %v", file, created, err) +} +``` + +- [ ] **Step 2: Add failing StageArchive cleanup-race tests** + +Add `TestStageArchiveCloseUsesPinnedRootAfterAncestorSwap` in both platform files. Install the existing test hook before staging and react only to `before-stage-archive-remove`. + +The Unix case must: + +1. complete `StageArchive` first; +2. rename `result.stagingRoot` to `result.stagingRoot + "-original"` inside the close hook; +3. replace the lexical root with a symlink to an owner-private outside directory containing `sentinel`; +4. call `staged.Close()`; +5. prove the random archive is absent below the moved original root and the outside sentinel is unchanged. + +The Windows case must attempt the same root rename in the close hook and require the rename to fail while the retained no-delete directory handle is live. It then requires `staged.Close()` to succeed, the archive to be absent, and an outside sentinel to remain unchanged. + +Use these hook bodies so the race point is deterministic and occurs after streaming, immediately before removal: + +```go +// Unix hook. +restoreHook := safeio.SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "before-stage-archive-remove" || swapped { + return + } + swapped = true + if err := os.Rename(result.stagingRoot, movedRoot); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, result.stagingRoot); err != nil { + t.Fatal(err) + } +}) +defer restoreHook() +``` + +```go +// Windows hook. +restoreHook := safeio.SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "before-stage-archive-remove" || attemptedSwap { + return + } + attemptedSwap = true + if err := os.Rename(result.stagingRoot, result.stagingRoot+"-moved"); err == nil { + t.Fatal("staging-root rename succeeded while Close retained its directory handle") + } +}) +defer restoreHook() +``` + +After `staged.Close`, require the hook boolean to be true. The Unix test checks `os.Stat(filepath.Join(movedRoot, stagedName))` returns `os.ErrNotExist`; the Windows test checks the original root contains no archive leaf. Both read the outside sentinel and compare its exact original bytes. + +Update existing assertions that reference `staged.directory`: direct-child staging means `filepath.Dir(staged.path) == result.stagingRoot` in the normal case. + +- [ ] **Step 3: Run focused tests and verify RED** + +```bash +cd tools/tht +go test ./internal/safeio ./internal/backup -run 'TestPrivateDirectoryCreatesAndRemovesStreamingRegularFile|TestStageArchiveCloseUsesPinnedRootAfterAncestorSwap' -count=1 +``` + +Expected: compile failure because `CreateRegularFile`, `parent`, and `name` do not exist, or the old pathname cleanup is redirected by the Unix swap. + +- [ ] **Step 4: Add the minimal streaming method to `PrivateDirectoryHandle`** + +Import `os` in `private_root.go` and add exactly one method: + +```go +type PrivateDirectoryHandle interface { + Close() error + Validate() error + OpenChild(name string, ensure bool) (PrivateDirectoryHandle, bool, error) + CreateRegular(name string, contents []byte) (bool, error) + CreateRegularFile(name string) (*os.File, bool, error) + ReadRegular(name string, maximum int64) ([]byte, bool, error) + ReplaceRegular(name string, contents []byte) error + RemoveRegular(name string) (bool, error) + ListPage(maximumEntries int, afterName string, validName func(string) bool, validLinks func(string, uint64) bool) (PrivateDirectoryPage, error) + ClaimRegular(source, claim string) (bool, error) + ReadClaim(source, claim string, maximum int64) ([]byte, bool, error) + RemoveClaim(source, claim string) (bool, error) +} +``` + +Do not add `CreateTemporaryDirectory`, `RemoveDirectory`, or pathname-returning cleanup methods. + +- [ ] **Step 5: Implement exclusive streaming creation on Unix** + +Use `unix.Openat(directory.descriptor, name, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC|O_NOFOLLOW, 0600)`. On success: + +- wrap the descriptor in `os.NewFile`; +- apply `Fchmod(0600)`; +- require `privateUnixRootRegular(stat, 1)` after `Fstat`; +- require `directory.Validate()` and `Fsync(directory.descriptor)` before return; +- on every failure, close the descriptor and `Unlinkat(directory.descriptor, name, 0)`. + +On `EEXIST`, inspect the existing leaf with `requirePrivateUnixRootRegularAt(..., 1)` and return `(nil, false, nil)` only for a safe existing regular file. Every other condition returns `ErrUnsafeFile`. + +- [ ] **Step 6: Implement exclusive streaming creation on Windows** + +Reuse `createWindowsPrivateRegularAt(directory.handle, name)` so the owner-only DACL is installed in the same NT relative create. Before detaching the handle: + +- require a one-link regular non-reparse file; +- require `directory.Validate()`; +- convert the retained file handle with `os.NewFile` and clear the wrapper handle to prevent double close; +- on failure call `closeAndDeleteWindowsPrivateRegular` while the parent handle is still retained. + +If relative open proves that a safe one-link leaf already exists, return `(nil, false, nil)`; unsafe or ambiguous failures return `ErrUnsafeFile`. + +- [ ] **Step 7: Replace StageArchive pathname ownership with the retained directory capability** + +Add the bounded, cryptographically random leaf-name helper: + +```go +func newStagingArchiveName() (string, error) { + value := make([]byte, 16) + if _, err := rand.Read(value); err != nil { + return "", err + } + return "archive-" + hex.EncodeToString(value) + ".zip", nil +} +``` + +After the capacity check, open the staging root once: + +```go +parent, found, err := safeio.OpenPrivateDirectory(result.stagingRoot, true) +if err != nil || !found { + return nil, errors.New("open private restore staging root") +} +``` + +Generate up to eight 16-byte random names. For each name, call `parent.CreateRegularFile(name)`; retry only the safe collision result. If no file is created, close the parent and return a sanitized creation error. + +Construct: + +```go +staged := &stagedArchive{ + file: file, + parent: parent, + name: name, + path: filepath.Join(result.stagingRoot, name), +} +``` + +Keep the existing source revalidation, bounded streaming, digest, `Sync`, and rewind logic unchanged. Remove `os.MkdirTemp`, `ProtectPrivateDirectory`, the nested `archive.zip`, and every `os.Remove` cleanup path from `StageArchive` and `stagedArchive.Close`. + +- [ ] **Step 8: Make Close exhaustive, capability-relative, and idempotent** + +`Close` must perform all cleanup attempts in this order even if an earlier operation fails: + +1. close `staged.file` and set it to `nil`; +2. call `safeio.NotifyPrivateDirectoryTestHookForTest("before-stage-archive-remove")`; +3. call `staged.parent.RemoveRegular(staged.name)` and require `removed=true` on the first close; +4. close `staged.parent` and set it to `nil`; +5. clear `name` and `path`; +6. return only `destroy private restore staging archive` if any operation failed. + +A second `Close()` returns `nil`. No error may contain the staging path or archive bytes. + +- [ ] **Step 9: Add the native Windows test gate** + +In the existing `windows-clone` job, after Go setup and before the clone-contract script, add: + +```yaml + - name: Run native Windows retained-capability tests + working-directory: tools/tht + run: go test ./internal/safeio ./internal/backup -count=1 +``` + +This job is the native execution authority. A Linux/macOS cross-compile only proves buildability. + +- [ ] **Step 10: Run focused, broad, race, and cross-compile gates** + +```bash +cd tools/tht +gofmt -w internal/safeio/private_root.go internal/safeio/private_root_unix.go \ + internal/safeio/private_root_windows.go internal/safeio/files_test.go \ + internal/backup/preflight.go internal/backup/preflight_test.go \ + internal/backup/preflight_unix_test.go internal/backup/preflight_windows_test.go +go test ./internal/safeio ./internal/backup -count=1 +go test -race ./internal/safeio ./internal/backup -count=1 +go vet ./internal/safeio ./internal/backup +GOOS=windows GOARCH=amd64 go test -c ./internal/safeio -o /tmp/tht-safeio-windows.test.exe +GOOS=windows GOARCH=amd64 go test -c ./internal/backup -o /tmp/tht-backup-windows.test.exe +``` + +Expected: all host tests pass and both Windows test executables compile. Record native execution as pending until the workflow job actually runs. + +- [ ] **Step 11: Commit the retained-staging remediation** + +```bash +git add tools/tht/internal/safeio/private_root.go \ + tools/tht/internal/safeio/private_root_unix.go \ + tools/tht/internal/safeio/private_root_windows.go \ + tools/tht/internal/safeio/files_test.go \ + tools/tht/internal/backup/preflight.go \ + tools/tht/internal/backup/preflight_test.go \ + tools/tht/internal/backup/preflight_unix_test.go \ + tools/tht/internal/backup/preflight_windows_test.go \ + .github/workflows/deployment.yml +git commit -m "fix(backup): retain staging cleanup capability" +``` + +**Mandatory Terra review gate:** Confirm no `StageArchive` creation or cleanup decision uses `os.MkdirTemp`, `os.Remove`, or a re-resolved staging pathname; `path` is diagnostic only; failure cleanup uses the retained parent; Unix swap cleanup removes only the moved-root archive; Windows native coverage is wired into CI; all handles close on every exit. Verdict must be `CLEAN` before Task 3. + +--- + +### Task 3: Remove Windows Claims Through the Retained Parent Handle + +**Files:** +- Modify: `tools/tht/internal/safeio/claim_windows.go:126-160` +- Create: `tools/tht/internal/safeio/claim_windows_test.go` + +**Interfaces:** +- Consumes: `OpenPrivateDirectory(path string, ensure bool) (PrivateDirectoryHandle, bool, error)`. +- Consumes: `PrivateDirectoryHandle.RemoveClaim(source, claim string) (bool, error)`. +- Preserves: `RemoveCanonicalPrivateClaim(source, claim string) (bool, error)` and its absent/orphan semantics. +- Produces test hook stage: `after-canonical-private-claim-parent-open`. + +- [ ] **Step 1: Add a native-Windows ancestor-swap test** + +Create a `//go:build windows` test in package `safeio`. It must: + +1. create and protect one private directory; +2. create `state.json` as an owner-private regular file; +3. create `state.claim` through `ClaimCanonicalPrivateRegular` so both names refer to the verified two-link file; +4. create a separate protected outside directory with a sentinel that must survive; +5. install `SetPrivateDirectoryTestHookForTest` and react to `after-canonical-private-claim-parent-open`; +6. attempt to rename the protected source directory and require Windows to reject the rename while the retained no-delete handle is live; +7. call `RemoveCanonicalPrivateClaim` and require `(true, nil)`; +8. prove both original names are absent and every outside file is unchanged. + +Use a private-file helper and the following main test shape: + +```go +func createWindowsPrivateTestFile(t *testing.T, path string, contents []byte) { + t.Helper() + file, err := CreateCanonicalNewPrivateFile(path) + if err != nil { + t.Fatal(err) + } + if _, err := file.Write(contents); err != nil { + _ = file.Close() + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } +} + +func TestRemoveCanonicalPrivateClaimRetainsParentDuringDeletion(t *testing.T) { + parent := filepath.Join(t.TempDir(), "claims") + if err := os.Mkdir(parent, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(parent); err != nil { + t.Fatal(err) + } + source := filepath.Join(parent, "state.json") + claim := filepath.Join(parent, "state.claim") + createWindowsPrivateTestFile(t, source, []byte("state")) + if claimed, err := ClaimCanonicalPrivateRegular(source, claim); err != nil || !claimed { + t.Fatalf("ClaimCanonicalPrivateRegular() = claimed %v, err %v", claimed, err) + } + + outside := filepath.Join(t.TempDir(), "outside") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(outside); err != nil { + t.Fatal(err) + } + sentinel := filepath.Join(outside, "sentinel") + createWindowsPrivateTestFile(t, sentinel, []byte("outside-safe")) + attemptedSwap := false + restoreHook := SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "after-canonical-private-claim-parent-open" || attemptedSwap { + return + } + attemptedSwap = true + if err := os.Rename(parent, parent+"-moved"); err == nil { + t.Fatal("claim parent rename succeeded while removal retained its handle") + } + }) + defer restoreHook() + + removed, err := RemoveCanonicalPrivateClaim(source, claim) + if err != nil || !removed || !attemptedSwap { + t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, attempted %v, err %v", removed, attemptedSwap, err) + } + for _, path := range []string{source, claim} { + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("removed path %q still exists: %v", filepath.Base(path), err) + } + } + if contents, err := os.ReadFile(sentinel); err != nil || string(contents) != "outside-safe" { + t.Fatalf("outside sentinel = %q, err %v", contents, err) + } +} +``` + +Add adjacent cases for an orphan one-link claim (`false, nil`, orphan preserved) and a mismatched two-file pair (`ErrUnsafeFile`, neither file removed). Build the mismatch as two independent private files, each with its own auxiliary hard link, so both source and claim have link count two but different file identities. These cases lock in current recovery semantics while changing deletion authority. + +- [ ] **Step 2: Cross-compile the new test and verify RED behavior by inspection** + +```bash +cd tools/tht +GOOS=windows GOARCH=amd64 go test -c ./internal/safeio -o /tmp/tht-safeio-claim-red.test.exe +``` + +Expected: compilation succeeds against the current API, but the new hook is never observed and the test would fail natively because the current function closes retained handles before absolute-path `DeleteFile` calls. Do not label this compile-only result as an executed RED test. + +- [ ] **Step 3: Replace absolute-path deletion with one retained directory operation** + +Implement the Windows function with a named return so a parent-close failure can fail closed: + +```go +func removeCanonicalPrivateClaim(source, claim string) (removed bool, resultErr error) { + parentPath := filepath.Dir(source) + sourceName := filepath.Base(source) + claimName := filepath.Base(claim) + if parentPath != filepath.Dir(claim) || !validPrivateLeafName(sourceName) || !validPrivateLeafName(claimName) { + return false, ErrUnsafeFile + } + directory, found, err := OpenPrivateDirectory(parentPath, false) + if err != nil || !found { + return false, ErrUnsafeFile + } + defer func() { + if closeErr := directory.Close(); closeErr != nil && resultErr == nil { + resultErr = ErrUnsafeFile + } + }() + NotifyPrivateDirectoryTestHookForTest("after-canonical-private-claim-parent-open") + return directory.RemoveClaim(sourceName, claimName) +} +``` + +Delete the old `openWindowsPrivateRegular`/close/`windows.DeleteFile` sequence from this function. Do not duplicate link-count, identity, orphan, or delete-on-close logic: `windowsPrivateDirectory.RemoveClaim` already implements those checks relative to `directory.handle`. + +- [ ] **Step 4: Run Windows compilation plus host-wide Go gates** + +```bash +cd tools/tht +gofmt -w internal/safeio/claim_windows.go internal/safeio/claim_windows_test.go +GOOS=windows GOARCH=amd64 go test -c ./internal/safeio -o /tmp/tht-safeio-claim-windows.test.exe +go test ./internal/safeio ./internal/authstorage -count=1 +go test -race ./... +go vet ./... +go build ./cmd/tht +``` + +Expected: cross-compile, safeio/authstorage semantics, race suite, vet, and CLI build pass. Native ancestor-swap execution remains pending until Task 4 runs the Windows workflow. + +- [ ] **Step 5: Commit the Windows claim remediation** + +```bash +git add tools/tht/internal/safeio/claim_windows.go \ + tools/tht/internal/safeio/claim_windows_test.go +git commit -m "fix(auth): remove Windows claims by retained handle" +``` + +**Mandatory Terra review gate:** Confirm `removeCanonicalPrivateClaim` contains no `DeleteFile` and performs no deletion after closing the parent capability; `RemoveClaim` remains the single identity/link/orphan authority; the test attempts an ancestor replacement and protects outside sentinels; Unix behavior is untouched. Verdict must be `CLEAN` before Task 4. + +--- + +### Task 4: Re-Certify the Remediation and Refresh Sanitized Evidence + +**Files:** +- Modify: `.artifacts/task-15/automated-gates.json` +- Modify: `.artifacts/task-15/unified-docker-images.json` only after a new immutable-source Docker smoke +- Modify: `.superpowers/sdd/2026-08-16-thothii-authentication/task-15-report.md` +- Modify: `PROJECT_STATE.md` + +**Interfaces:** +- Consumes every test and workflow gate from Tasks 1-3. +- Produces one source-bound PASS/FAIL/PENDING matrix with hashes for retained artifacts. +- Preserves the historical Task 15 evidence as provenance; new results supersede rather than rewrite historical source SHAs. + +- [ ] **Step 1: Freeze and record the source under test** + +After Tasks 1-3 and their reviews are clean: + +```bash +git status --short +AUTH_REMEDIATION_SOURCE="$(git rev-parse HEAD)" +printf '%s\n' "$AUTH_REMEDIATION_SOURCE" +``` + +Only `.playwright-cli/` and `.thothctl/` may be untracked. Record the resulting commit as `AUTH_REMEDIATION_SOURCE` in the operator notes. If any tracked source changes after this point, discard downstream certification results and restart this task from Step 1. + +- [ ] **Step 2: Run all Go security and build gates** + +```bash +cd tools/tht +go test ./internal/safeio ./internal/backup ./internal/authstorage -count=1 +go test -race ./... +go vet ./... +go build ./cmd/tht +GOOS=windows GOARCH=amd64 go test -c ./internal/safeio -o /tmp/tht-safeio-final-windows.test.exe +GOOS=windows GOARCH=amd64 go test -c ./internal/backup -o /tmp/tht-backup-final-windows.test.exe +GOOS=windows GOARCH=amd64 go build -o /tmp/tht-final-windows.exe ./cmd/tht +``` + +Record package counts and exact failures. Cross-compilation is a separate `PASS` row and never substitutes for native Windows execution. + +- [ ] **Step 3: Run the exact Node 24 backend and frontend gates** + +```bash +cd backend +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH node --version +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx tsc --noEmit -p . +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx vitest run +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npm run build +cd ../frontend +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx tsc -b +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npx vitest run +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npm run build +PATH=/Users/mp/.nvm/versions/node/v24.16.0/bin:$PATH npm run e2e -- --grep "authentication|F1" +``` + +The first command must print `v24.16.0`. Run the existing sentinel leak scan in `scripts/authentication-smoke.sh`; retain no browser credential or token. + +- [ ] **Step 4: Run harness and static/documentation gates without laundering baselines** + +```bash +cd ../harness +.venv/bin/pytest -q +.venv/bin/ruff check . +cd .. +bash scripts/auth-docs-smoke.sh +``` + +Record current counts. Existing baseline failures remain `FAIL` unless the exact command is now green; this remediation task does not authorize unrelated fixes. + +- [ ] **Step 5: Execute the native Windows authority** + +Push or dispatch only if the execution session has explicit repository authorization. The remote branch must contain `AUTH_REMEDIATION_SOURCE`: + +```bash +AUTH_REMEDIATION_SOURCE="$(git rev-parse HEAD)" +git push origin HEAD:feat/thoth-auth +gh workflow run deployment.yml --ref feat/thoth-auth -f windows_docker_startup=false +AUTH_WINDOWS_RUN_ID="" +for AUTH_WINDOWS_LOOKUP_ATTEMPT in {1..12}; do + AUTH_WINDOWS_RUN_ID="$(gh run list --workflow deployment.yml --branch feat/thoth-auth \ + --event workflow_dispatch --limit 10 --json databaseId,headSha \ + --jq "map(select(.headSha == \"$AUTH_REMEDIATION_SOURCE\"))[0].databaseId // empty")" + if [[ -n "$AUTH_WINDOWS_RUN_ID" ]]; then break; fi + sleep 5 +done +test -n "$AUTH_WINDOWS_RUN_ID" +gh run watch "$AUTH_WINDOWS_RUN_ID" --exit-status +gh run view "$AUTH_WINDOWS_RUN_ID" --json jobs \ + --jq '.jobs[] | select(.name == "Windows clone and Compose contract") | {name,conclusion,url}' +``` + +Wait for the matching source SHA and require the `Windows clone and Compose contract` job, including `Run native Windows retained-capability tests`, to pass. Retain the run URL/ID and the two focused test names, not raw runner logs. If dispatch or a native runner is unavailable, record `PENDING: native Windows execution unavailable` and do not mark the three-finding remediation complete. + +- [ ] **Step 6: Run shell, Compose, authentication, and final Docker gates** + +Run lightweight contracts first: + +```bash +bash -n scripts/*.sh +bash scripts/authentication-smoke.sh +bash scripts/auth-docs-smoke.sh +docker compose -f compose.yaml config --quiet +docker compose -f compose.yaml -f compose.unified.yaml config --quiet +``` + +Then run the unified Docker smoke exactly once against the frozen source: + +```bash +timeout --signal=TERM --kill-after=45s 32m bash scripts/unified-deployment-smoke.sh +``` + +Require task-scoped cleanup and five-image source traceability. A failed Docker smoke stays `FAIL`; do not rerun it against changed source without restarting at Step 1. + +- [ ] **Step 7: Run optional external gates only when their prerequisites exist** + +```bash +cd harness +.venv/bin/pytest -q -m l2 +``` + +Follow `docs/testing/authentication-manual-acceptance.md` for real PSD/Authentik acceptance only when real identity/access is available. Missing L2 secrets, Authentik access, PSD identities, or a provider port are `PENDING` with the exact prerequisite category; they are not remediation failures and are not PASS. + +- [ ] **Step 8: Refresh machine-readable and narrative evidence** + +Update `.artifacts/task-15/automated-gates.json` with: + +- `source_commit` equal to `AUTH_REMEDIATION_SOURCE`; +- UTC start/end timestamps; +- exact Node, Go, and Pi versions; +- focused ownership, StageArchive, and Windows claim test status; +- native Windows run ID/status distinct from cross-compile status; +- backend/frontend/harness counts; +- Docker run ID and cleanup status; +- unchanged known FAIL/PENDING rows where still applicable. + +Regenerate `.artifacts/task-15/unified-docker-images.json` only from the new Docker run and retain digests without registry names. Compute both SHA-256 values and place them in the Task 15 report. + +Update `PROJECT_STATE.md` so its leading Task 15 section states separately: + +- whether all three Important findings are closed by a clean final review; +- whether native Windows execution passed; +- whether authentication is implementation-complete; +- whether release acceptance remains blocked by unrelated FAIL/PENDING gates. + +- [ ] **Step 9: Commit only sanitized certification evidence** + +```bash +git add .artifacts/task-15/automated-gates.json \ + .artifacts/task-15/unified-docker-images.json \ + .superpowers/sdd/2026-08-16-thothii-authentication/task-15-report.md \ + PROJECT_STATE.md +git commit -m "docs(auth): record remediation certification" +``` + +Before committing, search the staged diff for fixture secrets, tokens, internal endpoints, user identities, and registry names. The report may contain hashes, versions, test counts, job IDs, and sanitized failure categories only. + +**Mandatory final Terra review gate:** Review `178113a..HEAD`, not only the evidence commit. Reproduce focused tests, inspect every affected security boundary, verify native-Windows evidence is executed rather than inferred, and issue separate verdicts for (a) the three Important findings and (b) overall release readiness. The remediation is complete only when verdict (a) is `CLEAN`; overall release readiness must remain `PENDING` or `FAIL` wherever unrelated gates still require it. + +--- + +## Final Acceptance Checklist + +- [ ] Every POSIX `lstat`/`fstat` metadata path for `users.yaml` and its parent requires the same valid effective UID. +- [ ] Foreign ownership at file-path, file-descriptor, directory-path, and directory-descriptor observations fails with only `local_user_registry_invalid`. +- [ ] `StageArchive` retains one `PrivateDirectoryHandle` from file creation through `Close` cleanup. +- [ ] `StageArchive` has no pathname-authorized file or directory removal and no nested temporary directory. +- [ ] Unix ancestor replacement cannot redirect staging cleanup; native Windows blocks replacement while the retained handle is live. +- [ ] Windows canonical claim removal delegates to `PrivateDirectoryHandle.RemoveClaim` and contains no post-close `DeleteFile` call. +- [ ] Native Windows executes both retained-capability race tests; cross-compilation is recorded separately. +- [ ] A fresh Terra reviewer reports no Critical/Important finding after every task. +- [ ] A fresh final Terra reviewer reports the three original Important findings `CLEAN` over `178113a..HEAD`. +- [ ] Evidence is bound to one immutable source SHA, sanitized, hashed, and honest about all remaining FAIL/PENDING release gates.