Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
082f032
fix(daemon): publish status files atomically
gnanam1990 Aug 24, 2026
8d0f918
fix(daemon): bind status publication to directory handle
gnanam1990 Aug 24, 2026
c24a634
fix(daemon): validate status directory access
gnanam1990 Aug 24, 2026
f0103fd
fix(daemon): accept current token directory owner
gnanam1990 Aug 24, 2026
b0c83e1
fix(daemon): migrate owned runtime directories safely
gnanam1990 Aug 25, 2026
f314833
fix(daemon): open Windows security handle relatively
gnanam1990 Aug 25, 2026
bfbf7d2
fix(daemon): use current NT directory object
gnanam1990 Aug 25, 2026
33ada0f
fix(observability): harden existing crash directories
gnanam1990 Aug 25, 2026
2ff6b07
fix(security): bind crash report creation to private root
gnanam1990 Aug 28, 2026
6e7f9c4
fix(observability): atomically publish crash reports
gnanam1990 Aug 28, 2026
8549347
fix(observability): preserve committed crash reports
gnanam1990 Aug 28, 2026
ca19e2f
fix(observability): revalidate committed crash path
gnanam1990 Aug 28, 2026
f764ff0
Merge remote-tracking branch 'origin/main' into codex/pr949-followup
gnanam1990 Aug 29, 2026
a4c3cc9
fix(daemon): secure runtime lifecycle boundaries
gnanam1990 Aug 29, 2026
7eb6421
fix(daemon): bind status ownership through cleanup
gnanam1990 Aug 30, 2026
a7a4224
fix(daemon): preserve Windows status security
gnanam1990 Aug 31, 2026
ed2af6e
fix(daemon): retain trusted runtime root for lifecycle
gnanam1990 Aug 31, 2026
ff2ffe3
fix(daemon): roll back redirected socket binds
gnanam1990 Sep 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions internal/daemon/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ type ServerOptions struct {
Now func() time.Time
Log func(string)
isAlive func(int) bool // test hook for the single-instance lock
// beforeStatusReplace, replaceStatusFile, and syncStatusParent are test hooks
// for the status-file commit boundary. nil selects production behavior.
beforeStatusReplace func()
replaceStatusFile func(root *os.Root, src, dst string) error
syncStatusParent func(root *os.Root) error
}

// NewServer validates options and builds a Server.
Expand Down Expand Up @@ -80,7 +85,7 @@ func (s *Server) Serve() error {
if err := checkSocketPathLength(s.opts.Paths.Socket); err != nil {
return err
}
if err := secureSocketParent(s.opts.Paths.Socket); err != nil {
if err := secureRuntimeParents(s.opts.Paths); err != nil {
return err
}
lock, err := acquireLock(s.opts.Paths.Lock, s.opts.isAlive)
Expand Down Expand Up @@ -208,7 +213,12 @@ func (s *Server) writeStatusFile() error {
if err != nil {
return err
}
if err := os.WriteFile(s.opts.Paths.Status, data, 0o600); err != nil {
if err := writeStatusFileAtomically(s.opts.Paths.Status, data, 0o600, s.opts.beforeStatusReplace, s.opts.replaceStatusFile, s.opts.syncStatusParent); err != nil {
var committed *statusFileCommittedError
if errors.As(err, &committed) {
s.logf("daemon: %v", committed)
return nil
}
return fmt.Errorf("daemon: write status file: %w", err)
}
return nil
Expand Down
68 changes: 67 additions & 1 deletion internal/daemon/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,24 @@ import (
"path/filepath"
"testing"
"time"

"github.com/Gitlawb/zero/internal/observability"
)

func newTestServer(t *testing.T, launcher Launcher) (*Server, Paths) {
t.Helper()
dir := t.TempDir()
secureStatusTestDir(t, dir)
paths := Paths{
Socket: filepath.Join(dir, "d.sock"),
Lock: filepath.Join(dir, "d.lock"),
Status: filepath.Join(dir, "d.status"),
}
return newTestServerWithPaths(t, launcher, paths), paths
}

func newTestServerWithPaths(t *testing.T, launcher Launcher, paths Paths) *Server {
t.Helper()
pool, err := NewPool(PoolOptions{Size: 2, Launcher: launcher, KillTimeout: 200 * time.Millisecond})
if err != nil {
t.Fatalf("NewPool: %v", err)
Expand All @@ -28,7 +36,7 @@ func newTestServer(t *testing.T, launcher Launcher) (*Server, Paths) {
if err != nil {
t.Fatalf("NewServer: %v", err)
}
return srv, paths
return srv
}

func waitForFile(t *testing.T, path string) {
Expand Down Expand Up @@ -136,6 +144,64 @@ func TestServerEndToEnd(t *testing.T) {
}
}

func TestServerPublishesDefaultStatusAfterCrashReportCreatesRuntimeDirectory(t *testing.T) {
home, err := os.MkdirTemp("", "zero-home-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(home) })
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_RUNTIME_DIR", "")

if _, err := observability.WriteCrashReport(
observability.DefaultCrashDir(),
"cli",
"boom",
[]byte("stack"),
time.Now(),
); err != nil {
t.Fatalf("WriteCrashReport: %v", err)
}
paths, err := DefaultPaths()
if err != nil {
t.Fatalf("DefaultPaths: %v", err)
}
if paths.Status != filepath.Join(home, ".zero", "daemon.status") {
t.Fatalf("default status path = %q, want path beneath temporary home", paths.Status)
}

launcher, _ := seqLauncher(&fakeWorker{pid: 1})
srv := newTestServerWithPaths(t, launcher, paths)
serveErr := make(chan error, 1)
go func() { serveErr <- srv.Serve() }()

deadline := time.NewTimer(3 * time.Second)
defer deadline.Stop()
for {
if _, err := os.Stat(paths.Status); err == nil {
break
}
select {
case err := <-serveErr:
t.Fatalf("Serve returned before publishing status: %v", err)
case <-deadline.C:
t.Fatal("daemon did not publish its default status file")
case <-time.After(2 * time.Millisecond):
}
}

srv.Shutdown()
select {
case err := <-serveErr:
if err != nil {
t.Fatalf("Serve returned error: %v", err)
}
case <-time.After(3 * time.Second):
t.Fatal("Serve did not return after shutdown")
}
}

func TestServerSecondInstanceFails(t *testing.T) {
block := make(chan struct{})
defer close(block)
Expand Down
35 changes: 30 additions & 5 deletions internal/daemon/socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package daemon

import (
"fmt"
"os"
"path/filepath"

"github.com/Gitlawb/zero/internal/privatedir"
)

// maxUnixSocketPath bounds the socket path to the smallest platform sun_path
Expand All @@ -12,10 +13,34 @@ import (
// safe cross-platform ceiling.
const maxUnixSocketPath = 103

// secureSocketParent creates the socket's parent directory owner-only (0700 on
// POSIX; on Windows the per-user profile directory is already ACL-restricted).
func secureSocketParent(socketPath string) error {
return os.MkdirAll(filepath.Dir(socketPath), 0o700)
// secureRuntimeParents creates and hardens every directory that can influence
// daemon coordination. Existing directories are migrated only after ownership
// is verified through a bound handle; directories owned by another user fail
// closed.
func secureRuntimeParents(paths Paths) error {
parents := []struct {
name string
path string
}{
{name: "socket", path: filepath.Dir(paths.Socket)},
{name: "lock", path: filepath.Dir(paths.Lock)},
{name: "status", path: filepath.Dir(paths.Status)},
}
seen := make(map[string]struct{}, len(parents))
for _, parent := range parents {
absolute, err := filepath.Abs(parent.path)
if err != nil {
return fmt.Errorf("daemon: resolve %s directory: %w", parent.name, err)
}
if _, ok := seen[absolute]; ok {
continue
}
seen[absolute] = struct{}{}
if err := privatedir.Ensure(absolute); err != nil {
return fmt.Errorf("daemon: secure %s directory: %w", parent.name, err)
}
}
return nil
}

// checkSocketPathLength rejects an over-long unix socket path before bind.
Expand Down
20 changes: 20 additions & 0 deletions internal/daemon/status_dir_owner_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//go:build !windows

package daemon

import (
"fmt"
"os"
"syscall"
)

func checkStatusDirOwner(_ *os.Root, info os.FileInfo) error {
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return fmt.Errorf("status directory ownership metadata is unavailable")
}
if int(stat.Uid) != os.Geteuid() {
return fmt.Errorf("status directory is owned by uid %d, not the current user", stat.Uid)
}
return nil
}
51 changes: 51 additions & 0 deletions internal/daemon/status_dir_owner_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//go:build !windows

package daemon

import (
"os"
"strings"
"syscall"
"testing"
"time"
)

func secureStatusTestDirPlatform(t *testing.T, dir string) {
t.Helper()
if err := os.Chmod(dir, 0o700); err != nil {
t.Fatal(err)
}
}

func broadenStatusTestDirPlatform(t *testing.T, dir string) {
t.Helper()
if err := os.Chmod(dir, 0o755); err != nil {
t.Fatal(err)
}
}

func TestCheckStatusDirOwnerRejectsMissingMetadata(t *testing.T) {
err := checkStatusDirOwner(nil, statusDirOwnerTestInfo{})
if err == nil || !strings.Contains(err.Error(), "metadata is unavailable") {
t.Fatalf("checkStatusDirOwner error = %v, want unavailable metadata rejection", err)
}
}

func TestCheckStatusDirOwnerRejectsDifferentUser(t *testing.T) {
info := statusDirOwnerTestInfo{sys: &syscall.Stat_t{Uid: uint32(os.Geteuid() + 1)}}
err := checkStatusDirOwner(nil, info)
if err == nil || !strings.Contains(err.Error(), "not the current user") {
t.Fatalf("checkStatusDirOwner error = %v, want owner mismatch rejection", err)
}
}

type statusDirOwnerTestInfo struct {
sys any
}

func (statusDirOwnerTestInfo) Name() string { return "." }
func (statusDirOwnerTestInfo) Size() int64 { return 0 }
func (statusDirOwnerTestInfo) Mode() os.FileMode { return os.ModeDir | 0o700 }
func (statusDirOwnerTestInfo) ModTime() time.Time { return time.Time{} }
func (statusDirOwnerTestInfo) IsDir() bool { return true }
func (info statusDirOwnerTestInfo) Sys() any { return info.sys }
137 changes: 137 additions & 0 deletions internal/daemon/status_dir_owner_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
//go:build windows

package daemon

import (
"errors"
"fmt"
"os"
"unsafe"

"golang.org/x/sys/windows"
)

const statusDirectoryWriteAccess = windows.ACCESS_MASK(
windows.GENERIC_ALL |
windows.GENERIC_WRITE |
windows.DELETE |
windows.WRITE_DAC |
windows.WRITE_OWNER |
windows.FILE_WRITE_DATA |
windows.FILE_APPEND_DATA |
windows.FILE_WRITE_ATTRIBUTES |
windows.FILE_WRITE_EA |
0x40, // FILE_DELETE_CHILD
)

// checkStatusDirOwner validates ownership and write access through a handle
// opened beneath root. Path-based ACL inspection would recreate the ancestor
// swap race that Root is intended to close.
func checkStatusDirOwner(root *os.Root, _ os.FileInfo) (returnErr error) {
directory, err := root.Open(".")
if err != nil {
return fmt.Errorf("open status directory for access validation: %w", err)
}
defer func() {
if err := directory.Close(); err != nil {
returnErr = errors.Join(returnErr, fmt.Errorf("close status directory access handle: %w", err))
}
}()

raw, err := directory.SyscallConn()
if err != nil {
return fmt.Errorf("access status directory handle: %w", err)
}
var descriptor *windows.SECURITY_DESCRIPTOR
var queryErr error
if err := raw.Control(func(handle uintptr) {
descriptor, queryErr = windows.GetSecurityInfo(
windows.Handle(handle),
windows.SE_FILE_OBJECT,
windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION,
)
}); err != nil {
return fmt.Errorf("inspect status directory access: %w", err)
}
if queryErr != nil {
return fmt.Errorf("inspect status directory owner and DACL: %w", queryErr)
}
if descriptor == nil {
return fmt.Errorf("status directory security descriptor is unavailable")
}

token := windows.GetCurrentProcessToken()
user, err := token.GetTokenUser()
if err != nil {
return fmt.Errorf("resolve current Windows user: %w", err)
}
tokenOwner, err := currentWindowsTokenOwner(token)
if err != nil {
return fmt.Errorf("resolve current Windows token owner: %w", err)
}
owner, _, err := descriptor.Owner()
if err != nil {
return fmt.Errorf("read status directory owner: %w", err)
}
if owner == nil || (!owner.Equals(user.User.Sid) && !owner.Equals(tokenOwner)) {
return fmt.Errorf("status directory is not owned by the current Windows token")
}

dacl, _, err := descriptor.DACL()
if err != nil {
return fmt.Errorf("read status directory DACL: %w", err)
}
if dacl == nil {
return fmt.Errorf("status directory has an unrestricted Windows DACL")
}
for index := uint16(0); index < dacl.AceCount; index++ {
var ace *windows.ACCESS_ALLOWED_ACE
if err := windows.GetAce(dacl, uint32(index), &ace); err != nil {
return fmt.Errorf("read status directory DACL entry %d: %w", index, err)
}
switch ace.Header.AceType {
case windows.ACCESS_DENIED_ACE_TYPE:
continue
case windows.ACCESS_ALLOWED_ACE_TYPE:
default:
return fmt.Errorf("status directory DACL entry %d has unsupported type %d", index, ace.Header.AceType)
}
if ace.Mask&statusDirectoryWriteAccess == 0 {
continue
}
trustee := (*windows.SID)(unsafe.Pointer(&ace.SidStart))
if !allowedStatusDirectoryTrustee(trustee, user.User.Sid) {
return fmt.Errorf("status directory DACL grants write access to unexpected trustee %s", trustee.String())
}
}
return nil
}

type statusDirectoryTokenOwner struct {
owner *windows.SID
}

func currentWindowsTokenOwner(token windows.Token) (*windows.SID, error) {
var size uint32
err := windows.GetTokenInformation(token, windows.TokenOwner, nil, 0, &size)
if err != windows.ERROR_INSUFFICIENT_BUFFER {
return nil, err
}
buffer := make([]byte, size)
if err := windows.GetTokenInformation(token, windows.TokenOwner, &buffer[0], size, &size); err != nil {
return nil, err
}
owner := (*statusDirectoryTokenOwner)(unsafe.Pointer(&buffer[0])).owner
if owner == nil {
return nil, errors.New("Windows access token has no default owner")

Check failure on line 126 in internal/daemon/status_dir_owner_windows.go

View workflow job for this annotation

GitHub Actions / Smoke (windows-latest)

ST1005: error strings should not be capitalized (staticcheck)
}
return owner.Copy()
}

func allowedStatusDirectoryTrustee(trustee, user *windows.SID) bool {
return trustee != nil && (trustee.Equals(user) ||
trustee.IsWellKnown(windows.WinLocalSystemSid) ||
trustee.IsWellKnown(windows.WinBuiltinAdministratorsSid) ||
trustee.IsWellKnown(windows.WinCreatorOwnerSid) ||
trustee.IsWellKnown(windows.WinCreatorOwnerRightsSid))
}
Loading
Loading