Set Windows DACL on discovery directory under LOCALAPPDATA

[stantheman0128]

Summary

writeServerInfoTo creates the discovery directory under xdg.StateHome (%LOCALAPPDATA% on Windows) with os.MkdirAll + os.Chmod(0700). On NTFS those POSIX modes are advisory: inherited ACEs from the parent (Everyone, other interactive users, Administrators) remain. Combined with the npipe:// discovery URL from #5201 / #5214, a local attacker who can rewrite server.json can point the next thv serve health check at their own pipe and capture the discovery nonce.

This change sets an explicit protected DACL on Windows after MkdirAll, granting GenericAll only to the process user and SYSTEM (with OICI so server.json inherits the same restriction). Non-Windows keeps os.Chmod. Sibling paths covered: new directory create, and replace (not merge) on an existing loose directory.

Fixes #5217

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (go test ./pkg/server/discovery/ focused suite on Windows 11)
• E2E tests (task test-e2e)
• Linting (task lint-fix; task not installed locally; go vet ./pkg/server/discovery/ clean)
• Manual testing (describe below)

Verification / Evidence

HEAD bug (os.Chmod leaves Everyone):

chmod_ok
=== AFTER real os.Chmod 0700 ===
...\thv-5217-head-repro2 Everyone:(OI)(CI)(M)
...
BUG_CONFIRMED: Everyone remains after os.Chmod

Unit tests (Windows 11, go1.26.0):

=== RUN   TestWriteServerInfo_WindowsDACL_NoOtherInteractiveUsers
--- PASS: TestWriteServerInfo_WindowsDACL_NoOtherInteractiveUsers
=== RUN   TestRestrictDiscoveryDirPermissions_ReplacesExistingLooseACL
--- PASS: TestRestrictDiscoveryDirPermissions_ReplacesExistingLooseACL
=== RUN   TestRestrictDiscoveryDirPermissions_NewDirectory
--- PASS: TestRestrictDiscoveryDirPermissions_NewDirectory
... WriteRead / Remove / Overwrites PASS; POSIX mode tests SKIP on Windows
PASS
ok  	github.com/stacklok/toolhive/pkg/server/discovery

Product path (real %LOCALAPPDATA% via XDG_STATE_HOME, public discovery.WriteServerInfo):

Before (Everyone + Administrators still on the discovery dir):

C:\Users\stans\AppData\Local\toolhive-5217-evidence\toolhive\server Everyone:(OI)(CI)(M)
                                                                    NT AUTHORITY\SYSTEM:(I)(OI)(CI)(F)
                                                                    BUILTIN\Administrators:(I)(OI)(CI)(F)
                                                                    CHIAO-BASE\stans:(I)(OI)(CI)(F)
SDDL before: ...D:AI(A;OICI;0x1301bf;;;WD)...(A;OICIID;FA;;;BA)...

After WriteServerInfo:

...\toolhive\server CHIAO-BASE\stans:(F)
                    CHIAO-BASE\stans:(OI)(CI)(IO)(F)
                    NT AUTHORITY\SYSTEM:(F)
                    NT AUTHORITY\SYSTEM:(OI)(CI)(IO)(F)
SDDL after: ...D:PAI(A;OICIIO;GA;;;SY)(A;;FA;;;SY)(A;OICIIO;GA;;;<user>)(A;;FA;;;<user>)
server.json inherits user+SYSTEM only (no Everyone / Administrators)
WriteServerInfo_ok

go vet ./pkg/server/discovery/: clean.

What was not tested

• Full task test / task lint-fix (Task not installed on this machine; CI on ubuntu covers the package).
• Live multi-user login session attempting cross-user write (DACL contents verified via icacls / SDDL instead).
• Migrating unrelated pre-existing directories outside the write path (out of scope per Set explicit DACL on Windows discovery directory under %LOCALAPPDATA% #5217; every thv serve startup rewrites via WriteServerInfo).

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File	Change
pkg/server/discovery/discovery.go	Call restrictDiscoveryDirPermissions after MkdirAll
pkg/server/discovery/permissions_windows.go	Windows DACL replace via SetNamedSecurityInfo
pkg/server/discovery/permissions_other.go	Non-Windows os.Chmod
pkg/server/discovery/permissions_windows_test.go	Create / replace / writeServerInfo DACL tests
pkg/server/discovery/discovery_test.go	Skip POSIX mode assertions on Windows

Does this introduce a user-facing change?

On Windows, the discovery directory under %LOCALAPPDATA%\...\toolhive\server is locked down to the user that started thv plus SYSTEM. No CLI flag or config change.

Special notes for reviewers

• Matches the named-pipe DACL intent from Restrict Windows named-pipe DACL to creating user #5214 (user + SYSTEM only), applied to the discovery directory that stores the npipe:// URL.
• AI assistance: Cursor/Grok helped implement and verify; I reviewed the diff, ran the Windows unit tests, and captured the LOCALAPPDATA before/after icacls Evidence above.

Made with Cursor