From a91e3d5cc32dde97ade5acb13b502a95ff285bbc Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 18:39:02 -0500 Subject: [PATCH 01/10] Restrict current-user files cross-platform Callers that inherit or reopen sensitive runtime files need to repair access on the verified handle, not assume Unix mode bits or a private parent are sufficient. This gives them one fail-closed primitive that preserves the current-user ownership check and installs native owner-only permissions, including a protected Windows DACL. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/AGENTS.md | 2 ++ safefileio/open_file_other.go | 9 +++++++ safefileio/open_file_unix.go | 9 +++++++ safefileio/open_file_windows.go | 13 ++++++++++ safefileio/private_dir_unix_test.go | 14 +++++++++++ safefileio/private_dir_windows_test.go | 33 ++++++++++++++++++++++++++ 6 files changed, 80 insertions(+) diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index 579eb1b..73832aa 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -20,6 +20,8 @@ callers responsible for their own file formats and higher-level policy. trusted user, token-owner, system, and administrator principals. Use SID semantics rather than username string comparisons. - If ownership or file type cannot be established, return an error. +- Restrict an open file only after validating that same handle's regular-file + type and current-user ownership; never repair an unverified path. ## Tests diff --git a/safefileio/open_file_other.go b/safefileio/open_file_other.go index c64b853..d8f0b42 100644 --- a/safefileio/open_file_other.go +++ b/safefileio/open_file_other.go @@ -25,3 +25,12 @@ func ValidateCurrentUserFile(*os.File) error { runtime.GOOS, ) } + +// RestrictCurrentUserFile fails closed when the platform cannot enforce +// current-user-only file access. +func RestrictCurrentUserFile(*os.File) error { + return fmt.Errorf( + "safefileio: current-user file restriction is unsupported on %s", + runtime.GOOS, + ) +} diff --git a/safefileio/open_file_unix.go b/safefileio/open_file_unix.go index 4eb50d9..2799d3b 100644 --- a/safefileio/open_file_unix.go +++ b/safefileio/open_file_unix.go @@ -53,3 +53,12 @@ func ValidateCurrentUserFile(file *os.File) error { } return nil } + +// RestrictCurrentUserFile validates an open handle and makes it readable and +// writable only by its current-user owner. +func RestrictCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + return file.Chmod(0o600) +} diff --git a/safefileio/open_file_windows.go b/safefileio/open_file_windows.go index 690f894..cdaaab0 100644 --- a/safefileio/open_file_windows.go +++ b/safefileio/open_file_windows.go @@ -55,6 +55,19 @@ func ValidateCurrentUserFile(file *os.File) error { return validateWindowsFileHandle(file.Name(), windows.Handle(file.Fd())) } +// RestrictCurrentUserFile validates an open handle and installs a protected +// DACL limited to the current user and Windows administrative principals. +func RestrictCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + userSID, err := currentWindowsUserSID() + if err != nil { + return err + } + return restrictWindowsDir(windows.Handle(file.Fd()), userSID) +} + func validateWindowsFileHandle(path string, handle windows.Handle) error { var info windows.ByHandleFileInformation if err := windows.GetFileInformationByHandle(handle, &info); err != nil { diff --git a/safefileio/private_dir_unix_test.go b/safefileio/private_dir_unix_test.go index 3682cd3..acf9724 100644 --- a/safefileio/private_dir_unix_test.go +++ b/safefileio/private_dir_unix_test.go @@ -95,3 +95,17 @@ func TestOpenCurrentUserFileRejectsNonRegularFile(t *testing.T) { require.Error(err) require.Nil(file) } + +func TestRestrictCurrentUserFileRepairsPublicMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + require.NoError(t, os.Chmod(path, 0o666)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.NoError(t, safefileio.RestrictCurrentUserFile(file)) + info, err := file.Stat() + require.NoError(t, err) + require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) +} diff --git a/safefileio/private_dir_windows_test.go b/safefileio/private_dir_windows_test.go index ecc968c..8ddcdc3 100644 --- a/safefileio/private_dir_windows_test.go +++ b/safefileio/private_dir_windows_test.go @@ -97,6 +97,39 @@ func TestOpenCurrentUserFileAcceptsCurrentTokenOwner(t *testing.T) { require.NoError(t, file.Close()) } +func TestRestrictCurrentUserFileRepairsBroadDACL(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + handle := windows.Handle(file.Fd()) + userSID, err := currentWindowsUserSID() + require.NoError(t, err) + ownerSID, err := currentWindowsOwnerSID() + require.NoError(t, err) + world, err := windows.CreateWellKnownSid(windows.WinWorldSid) + require.NoError(t, err) + acl, err := windows.ACLFromEntries([]windows.EXPLICIT_ACCESS{ + allowFullControl(userSID, windows.TRUSTEE_IS_USER), + allowFullControl(world, windows.TRUSTEE_IS_WELL_KNOWN_GROUP), + }, nil) + require.NoError(t, err) + require.NoError(t, windows.SetSecurityInfo( + handle, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, + nil, + acl, + nil, + )) + require.Error(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) + + require.NoError(t, RestrictCurrentUserFile(file)) + require.NoError(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) +} + func TestWindowsOwnerMatchesCurrentUserAndTokenOwner(t *testing.T) { require := require.New(t) assert := assert.New(t) From 0eb523797a1e820bdde934aa25be078400091601 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 19:43:23 -0500 Subject: [PATCH 02/10] Reopen Windows files with DACL access An ordinary data handle does not imply authority to replace a Windows DACL, so permission repair could fail before securing the file. Reopening from the already-validated handle requests the exact ACL rights without reintroducing a path substitution race, and the Windows regression now uses the same capability boundary. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/open_file_windows.go | 32 ++++++++++++++++++++++++-- safefileio/private_dir_windows_test.go | 4 +++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/safefileio/open_file_windows.go b/safefileio/open_file_windows.go index cdaaab0..e486340 100644 --- a/safefileio/open_file_windows.go +++ b/safefileio/open_file_windows.go @@ -9,6 +9,8 @@ import ( "golang.org/x/sys/windows" ) +var reOpenFile = windows.NewLazySystemDLL("kernel32.dll").NewProc("ReOpenFile") + // OpenCurrentUserFile opens path without following reparse points and verifies // the opened handle is a regular file owned by the current token user or token // owner. @@ -58,14 +60,40 @@ func ValidateCurrentUserFile(file *os.File) error { // RestrictCurrentUserFile validates an open handle and installs a protected // DACL limited to the current user and Windows administrative principals. func RestrictCurrentUserFile(file *os.File) error { - if err := ValidateCurrentUserFile(file); err != nil { + handle, err := reopenWindowsFileForDACL(file) + if err != nil { return err } + defer func() { _ = windows.CloseHandle(handle) }() userSID, err := currentWindowsUserSID() if err != nil { return err } - return restrictWindowsDir(windows.Handle(file.Fd()), userSID) + return restrictWindowsDir(handle, userSID) +} + +func reopenWindowsFileForDACL(file *os.File) (windows.Handle, error) { + if err := ValidateCurrentUserFile(file); err != nil { + return 0, err + } + result, _, callErr := reOpenFile.Call( + file.Fd(), + uintptr(windows.READ_CONTROL|windows.WRITE_DAC), + uintptr(windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE), + 0, + ) + handle := windows.Handle(result) + if handle == windows.InvalidHandle { + if callErr != windows.ERROR_SUCCESS { + return 0, callErr + } + return 0, windows.ERROR_INVALID_HANDLE + } + if err := validateWindowsFileHandle(file.Name(), handle); err != nil { + _ = windows.CloseHandle(handle) + return 0, err + } + return handle, nil } func validateWindowsFileHandle(path string, handle windows.Handle) error { diff --git a/safefileio/private_dir_windows_test.go b/safefileio/private_dir_windows_test.go index 8ddcdc3..216a8f7 100644 --- a/safefileio/private_dir_windows_test.go +++ b/safefileio/private_dir_windows_test.go @@ -103,7 +103,9 @@ func TestRestrictCurrentUserFileRepairsBroadDACL(t *testing.T) { file, err := os.OpenFile(path, os.O_RDWR, 0) require.NoError(t, err) defer func() { _ = file.Close() }() - handle := windows.Handle(file.Fd()) + handle, err := reopenWindowsFileForDACL(file) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(handle) }() userSID, err := currentWindowsUserSID() require.NoError(t, err) ownerSID, err := currentWindowsOwnerSID() From d6234ca54de812105691118cab5f1570582e05ad Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 20:03:05 -0500 Subject: [PATCH 03/10] Remove access ACLs before restricting files Mode bits alone do not revoke extended ACL grants, so a successful restriction could leave another principal with access. The safe-file contract now removes ACL policy through the verified handle on macOS and Linux, while unsupported Unix platforms fail instead of claiming privacy they cannot establish. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- go.mod | 2 +- safefileio/open_file_unix.go | 9 -- safefileio/restrict_file_darwin.go | 88 ++++++++++++++++++++ safefileio/restrict_file_darwin_test.go | 32 +++++++ safefileio/restrict_file_linux.go | 28 +++++++ safefileio/restrict_file_unsupported_unix.go | 21 +++++ 6 files changed, 170 insertions(+), 10 deletions(-) create mode 100644 safefileio/restrict_file_darwin.go create mode 100644 safefileio/restrict_file_darwin_test.go create mode 100644 safefileio/restrict_file_linux.go create mode 100644 safefileio/restrict_file_unsupported_unix.go diff --git a/go.mod b/go.mod index e823b76..91b4895 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/s3 v1.106.2 github.com/aws/smithy-go v1.27.5 github.com/charmbracelet/x/ansi v0.11.7 + github.com/ebitengine/purego v0.10.0 github.com/gofrs/flock v0.13.0 github.com/klauspost/compress v1.18.6 github.com/leanovate/gopter v0.2.11 @@ -63,7 +64,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/dnephin/pflag v1.0.7 // indirect github.com/dustin/go-humanize v1.0.1 // indirect - github.com/ebitengine/purego v0.10.0 // indirect github.com/fatih/color v1.18.0 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-logr/logr v1.4.3 // indirect diff --git a/safefileio/open_file_unix.go b/safefileio/open_file_unix.go index 2799d3b..4eb50d9 100644 --- a/safefileio/open_file_unix.go +++ b/safefileio/open_file_unix.go @@ -53,12 +53,3 @@ func ValidateCurrentUserFile(file *os.File) error { } return nil } - -// RestrictCurrentUserFile validates an open handle and makes it readable and -// writable only by its current-user owner. -func RestrictCurrentUserFile(file *os.File) error { - if err := ValidateCurrentUserFile(file); err != nil { - return err - } - return file.Chmod(0o600) -} diff --git a/safefileio/restrict_file_darwin.go b/safefileio/restrict_file_darwin.go new file mode 100644 index 0000000..b5e4e2b --- /dev/null +++ b/safefileio/restrict_file_darwin.go @@ -0,0 +1,88 @@ +package safefileio + +import ( + "errors" + "fmt" + "os" + "sync" + "syscall" + + "github.com/ebitengine/purego" +) + +const darwinACLExtended = 0x00000100 + +type darwinACLAPI struct { + init uintptr + set uintptr + free uintptr +} + +var ( + darwinACLOnce sync.Once + darwinACL darwinACLAPI + darwinACLErr error +) + +// RestrictCurrentUserFile validates an open handle, removes its macOS extended +// ACL, and makes it readable and writable only by its current-user owner. +func RestrictCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + if err := removeDarwinExtendedACL(file); err != nil { + return err + } + return file.Chmod(0o600) +} + +func removeDarwinExtendedACL(file *os.File) error { + darwinACLOnce.Do(loadDarwinACL) + if darwinACLErr != nil { + return darwinACLErr + } + acl, _, callErr := purego.SyscallN(darwinACL.init, 0) + if acl == 0 { + return darwinACLCallError("initialize empty ACL", callErr) + } + defer func() { _, _, _ = purego.SyscallN(darwinACL.free, acl) }() + result, _, callErr := purego.SyscallN( + darwinACL.set, + file.Fd(), + acl, + darwinACLExtended, + ) + if result == ^uintptr(0) { + return darwinACLCallError("remove extended ACL", callErr) + } + return nil +} + +func loadDarwinACL() { + handle, err := purego.Dlopen( + "/usr/lib/libSystem.B.dylib", + purego.RTLD_NOW|purego.RTLD_LOCAL, + ) + if err != nil { + darwinACLErr = fmt.Errorf("load macOS ACL API: %w", err) + return + } + for name, target := range map[string]*uintptr{ + "acl_init": &darwinACL.init, + "acl_set_fd_np": &darwinACL.set, + "acl_free": &darwinACL.free, + } { + *target, err = purego.Dlsym(handle, name) + if err != nil { + darwinACLErr = fmt.Errorf("load macOS ACL function %s: %w", name, err) + return + } + } +} + +func darwinACLCallError(operation string, value uintptr) error { + if value != 0 { + return fmt.Errorf("%s: %w", operation, syscall.Errno(value)) + } + return errors.New(operation + " failed") +} diff --git a/safefileio/restrict_file_darwin_test.go b/safefileio/restrict_file_darwin_test.go new file mode 100644 index 0000000..695c2cc --- /dev/null +++ b/safefileio/restrict_file_darwin_test.go @@ -0,0 +1,32 @@ +package safefileio_test + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" +) + +func TestRestrictCurrentUserFileRemovesExtendedACL(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + output, err := exec.Command( + "chmod", + "+a", + "everyone allow read", + path, + ).CombinedOutput() + require.NoError(t, err, string(output)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.NoError(t, safefileio.RestrictCurrentUserFile(file)) + listing, err := exec.Command("ls", "-lde", path).CombinedOutput() + require.NoError(t, err, string(listing)) + assert.NotContains(t, string(listing), "everyone allow read") +} diff --git a/safefileio/restrict_file_linux.go b/safefileio/restrict_file_linux.go new file mode 100644 index 0000000..6a64a9d --- /dev/null +++ b/safefileio/restrict_file_linux.go @@ -0,0 +1,28 @@ +package safefileio + +import ( + "errors" + "fmt" + "os" + + "golang.org/x/sys/unix" +) + +// RestrictCurrentUserFile validates an open handle, removes access ACLs, and +// makes the file readable and writable only by its current-user owner. +func RestrictCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + for _, attribute := range []string{ + "system.posix_acl_access", + "system.nfs4_acl", + } { + err := unix.Fremovexattr(int(file.Fd()), attribute) + if err != nil && !errors.Is(err, unix.ENODATA) && + !errors.Is(err, unix.ENOTSUP) { + return fmt.Errorf("remove access ACL %s: %w", attribute, err) + } + } + return file.Chmod(0o600) +} diff --git a/safefileio/restrict_file_unsupported_unix.go b/safefileio/restrict_file_unsupported_unix.go new file mode 100644 index 0000000..dc18541 --- /dev/null +++ b/safefileio/restrict_file_unsupported_unix.go @@ -0,0 +1,21 @@ +//go:build unix && !darwin && !linux + +package safefileio + +import ( + "fmt" + "os" + "runtime" +) + +// RestrictCurrentUserFile fails closed on Unix platforms where Kit cannot +// remove access-control lists through the verified file handle. +func RestrictCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + return fmt.Errorf( + "safefileio: current-user file restriction is unsupported on %s", + runtime.GOOS, + ) +} From b0979b7e3e5babc242fe937c4e71658cabb48666 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 20:14:18 -0500 Subject: [PATCH 04/10] Reject every failed macOS ACL update The macOS ACL setter returns a C int, so a failure may appear in the uintptr syscall result as a zero-extended 32-bit minus one rather than an all-bits-set machine word. Treating zero as the only success value keeps file restriction fail-closed across ABI return representations. Validation: reproduced the old false-success result by invoking ACL removal on a closed descriptor. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/restrict_file_darwin.go | 2 +- safefileio/restrict_file_internal_darwin_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 safefileio/restrict_file_internal_darwin_test.go diff --git a/safefileio/restrict_file_darwin.go b/safefileio/restrict_file_darwin.go index b5e4e2b..d91cad3 100644 --- a/safefileio/restrict_file_darwin.go +++ b/safefileio/restrict_file_darwin.go @@ -52,7 +52,7 @@ func removeDarwinExtendedACL(file *os.File) error { acl, darwinACLExtended, ) - if result == ^uintptr(0) { + if result != 0 { return darwinACLCallError("remove extended ACL", callErr) } return nil diff --git a/safefileio/restrict_file_internal_darwin_test.go b/safefileio/restrict_file_internal_darwin_test.go new file mode 100644 index 0000000..c5bbe07 --- /dev/null +++ b/safefileio/restrict_file_internal_darwin_test.go @@ -0,0 +1,16 @@ +package safefileio + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRemoveDarwinExtendedACLRejectsFailedSet(t *testing.T) { + file, err := os.CreateTemp(t.TempDir(), "record-*.json") + require.NoError(t, err) + require.NoError(t, file.Close()) + + require.Error(t, removeDarwinExtendedACL(file)) +} From 6fd027fabd8d6f85cf8048803a87dfa17658f21a Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 20:51:18 -0500 Subject: [PATCH 05/10] Narrow Unix modes before removing ACLs Removing a deny ACL while group or other mode bits remain open creates a brief access window even if the final state is private. Supported Unix implementations now share a fail-closed sequence that narrows mode bits before ACL work and reapplies them afterward.\n\nPlatform-specific tests also distinguish Darwin/Linux enforcement from the intentional unsupported-Unix result instead of imposing one contract across incompatible implementations. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/AGENTS.md | 3 ++ safefileio/private_dir_unix_test.go | 14 --------- safefileio/restrict_file_darwin.go | 12 ++----- safefileio/restrict_file_linux.go | 13 ++++---- safefileio/restrict_file_supported_unix.go | 21 +++++++++++++ ...trict_file_supported_unix_external_test.go | 26 ++++++++++++++++ .../restrict_file_supported_unix_test.go | 31 +++++++++++++++++++ .../restrict_file_unsupported_unix_test.go | 23 ++++++++++++++ 8 files changed, 114 insertions(+), 29 deletions(-) create mode 100644 safefileio/restrict_file_supported_unix.go create mode 100644 safefileio/restrict_file_supported_unix_external_test.go create mode 100644 safefileio/restrict_file_supported_unix_test.go create mode 100644 safefileio/restrict_file_unsupported_unix_test.go diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index 73832aa..b257768 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -22,6 +22,9 @@ callers responsible for their own file formats and higher-level policy. - If ownership or file type cannot be established, return an error. - Restrict an open file only after validating that same handle's regular-file type and current-user ownership; never repair an unverified path. +- On Unix platforms that support ACL removal, narrow mode bits before removing + ACLs and reapply the private mode afterward; never create a broader-access + interval while changing access-control policy. ## Tests diff --git a/safefileio/private_dir_unix_test.go b/safefileio/private_dir_unix_test.go index acf9724..3682cd3 100644 --- a/safefileio/private_dir_unix_test.go +++ b/safefileio/private_dir_unix_test.go @@ -95,17 +95,3 @@ func TestOpenCurrentUserFileRejectsNonRegularFile(t *testing.T) { require.Error(err) require.Nil(file) } - -func TestRestrictCurrentUserFileRepairsPublicMode(t *testing.T) { - path := filepath.Join(t.TempDir(), "record.json") - require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) - require.NoError(t, os.Chmod(path, 0o666)) - file, err := os.OpenFile(path, os.O_RDWR, 0) - require.NoError(t, err) - defer func() { _ = file.Close() }() - - require.NoError(t, safefileio.RestrictCurrentUserFile(file)) - info, err := file.Stat() - require.NoError(t, err) - require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) -} diff --git a/safefileio/restrict_file_darwin.go b/safefileio/restrict_file_darwin.go index d91cad3..cb426e6 100644 --- a/safefileio/restrict_file_darwin.go +++ b/safefileio/restrict_file_darwin.go @@ -24,16 +24,10 @@ var ( darwinACLErr error ) -// RestrictCurrentUserFile validates an open handle, removes its macOS extended -// ACL, and makes it readable and writable only by its current-user owner. +// RestrictCurrentUserFile validates an open handle, narrows its mode, removes +// its macOS extended ACL, and keeps it readable and writable only by its owner. func RestrictCurrentUserFile(file *os.File) error { - if err := ValidateCurrentUserFile(file); err != nil { - return err - } - if err := removeDarwinExtendedACL(file); err != nil { - return err - } - return file.Chmod(0o600) + return restrictCurrentUserFile(file, removeDarwinExtendedACL) } func removeDarwinExtendedACL(file *os.File) error { diff --git a/safefileio/restrict_file_linux.go b/safefileio/restrict_file_linux.go index 6a64a9d..3510b91 100644 --- a/safefileio/restrict_file_linux.go +++ b/safefileio/restrict_file_linux.go @@ -8,12 +8,13 @@ import ( "golang.org/x/sys/unix" ) -// RestrictCurrentUserFile validates an open handle, removes access ACLs, and -// makes the file readable and writable only by its current-user owner. +// RestrictCurrentUserFile validates an open handle, narrows its mode, removes +// access ACLs, and keeps it readable and writable only by its current-user owner. func RestrictCurrentUserFile(file *os.File) error { - if err := ValidateCurrentUserFile(file); err != nil { - return err - } + return restrictCurrentUserFile(file, removeLinuxAccessACLs) +} + +func removeLinuxAccessACLs(file *os.File) error { for _, attribute := range []string{ "system.posix_acl_access", "system.nfs4_acl", @@ -24,5 +25,5 @@ func RestrictCurrentUserFile(file *os.File) error { return fmt.Errorf("remove access ACL %s: %w", attribute, err) } } - return file.Chmod(0o600) + return nil } diff --git a/safefileio/restrict_file_supported_unix.go b/safefileio/restrict_file_supported_unix.go new file mode 100644 index 0000000..93556c4 --- /dev/null +++ b/safefileio/restrict_file_supported_unix.go @@ -0,0 +1,21 @@ +//go:build darwin || linux + +package safefileio + +import "os" + +func restrictCurrentUserFile( + file *os.File, + removeACL func(*os.File) error, +) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + if err := file.Chmod(0o600); err != nil { + return err + } + if err := removeACL(file); err != nil { + return err + } + return file.Chmod(0o600) +} diff --git a/safefileio/restrict_file_supported_unix_external_test.go b/safefileio/restrict_file_supported_unix_external_test.go new file mode 100644 index 0000000..2149fb0 --- /dev/null +++ b/safefileio/restrict_file_supported_unix_external_test.go @@ -0,0 +1,26 @@ +//go:build darwin || linux + +package safefileio_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" +) + +func TestRestrictCurrentUserFileRepairsPublicMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + require.NoError(t, os.Chmod(path, 0o666)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.NoError(t, safefileio.RestrictCurrentUserFile(file)) + info, err := file.Stat() + require.NoError(t, err) + require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) +} diff --git a/safefileio/restrict_file_supported_unix_test.go b/safefileio/restrict_file_supported_unix_test.go new file mode 100644 index 0000000..24027b7 --- /dev/null +++ b/safefileio/restrict_file_supported_unix_test.go @@ -0,0 +1,31 @@ +//go:build darwin || linux + +package safefileio + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRestrictCurrentUserFileNarrowsModeAroundACLRemoval(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + require.NoError(t, os.Chmod(path, 0o666)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + err = restrictCurrentUserFile(file, func(file *os.File) error { + info, statErr := file.Stat() + require.NoError(t, statErr) + require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + return file.Chmod(0o666) + }) + require.NoError(t, err) + info, err := file.Stat() + require.NoError(t, err) + require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) +} diff --git a/safefileio/restrict_file_unsupported_unix_test.go b/safefileio/restrict_file_unsupported_unix_test.go new file mode 100644 index 0000000..8439316 --- /dev/null +++ b/safefileio/restrict_file_unsupported_unix_test.go @@ -0,0 +1,23 @@ +//go:build unix && !darwin && !linux + +package safefileio_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" +) + +func TestRestrictCurrentUserFileFailsClosedWhenUnsupported(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + err = safefileio.RestrictCurrentUserFile(file) + require.ErrorContains(t, err, "current-user file restriction is unsupported") +} From 8c0a56529ec8f5d76ffbc54692c6e95ae8fd548c Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 21:01:49 -0500 Subject: [PATCH 06/10] Reject unverifiable Unix file restrictions A successful local chmod is not sufficient evidence that a filesystem enforced private access, and SMB-family mounts retain a server-side DACL that these helpers cannot safely rewrite. Fail before mutation on those Linux filesystems and require an exact mode readback around ACL removal so callers never receive a success-shaped result for ambiguous permissions. Validation: executed the complete safefileio test binary inside a Linux container in addition to the repository and cross-build suites. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/AGENTS.md | 3 ++ safefileio/restrict_file_darwin.go | 2 +- safefileio/restrict_file_linux.go | 29 +++++++++++++++- safefileio/restrict_file_linux_test.go | 21 ++++++++++++ safefileio/restrict_file_supported_unix.go | 33 +++++++++++++++++-- .../restrict_file_supported_unix_test.go | 13 +++++++- 6 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 safefileio/restrict_file_linux_test.go diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index b257768..86436d0 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -25,6 +25,9 @@ callers responsible for their own file formats and higher-level policy. - On Unix platforms that support ACL removal, narrow mode bits before removing ACLs and reapply the private mode afterward; never create a broader-access interval while changing access-control policy. +- Verify the exact private mode through the open handle after each chmod. Reject + Linux SMB/CIFS filesystems because their server DACL cannot be secured by + local mode and access-ACL operations. ## Tests diff --git a/safefileio/restrict_file_darwin.go b/safefileio/restrict_file_darwin.go index cb426e6..078f38c 100644 --- a/safefileio/restrict_file_darwin.go +++ b/safefileio/restrict_file_darwin.go @@ -27,7 +27,7 @@ var ( // RestrictCurrentUserFile validates an open handle, narrows its mode, removes // its macOS extended ACL, and keeps it readable and writable only by its owner. func RestrictCurrentUserFile(file *os.File) error { - return restrictCurrentUserFile(file, removeDarwinExtendedACL) + return restrictCurrentUserFile(file, nil, removeDarwinExtendedACL) } func removeDarwinExtendedACL(file *os.File) error { diff --git a/safefileio/restrict_file_linux.go b/safefileio/restrict_file_linux.go index 3510b91..811f8b1 100644 --- a/safefileio/restrict_file_linux.go +++ b/safefileio/restrict_file_linux.go @@ -11,7 +11,34 @@ import ( // RestrictCurrentUserFile validates an open handle, narrows its mode, removes // access ACLs, and keeps it readable and writable only by its current-user owner. func RestrictCurrentUserFile(file *os.File) error { - return restrictCurrentUserFile(file, removeLinuxAccessACLs) + return restrictCurrentUserFile( + file, + validateLinuxRestrictionFilesystem, + removeLinuxAccessACLs, + ) +} + +func validateLinuxRestrictionFilesystem(file *os.File) error { + var status unix.Statfs_t + if err := unix.Fstatfs(int(file.Fd()), &status); err != nil { + return fmt.Errorf("inspect file filesystem: %w", err) + } + if linuxFilesystemRequiresServerACL(int64(status.Type)) { + return errors.New( + "safefileio: current-user file restriction is unsupported " + + "on SMB/CIFS filesystems", + ) + } + return nil +} + +func linuxFilesystemRequiresServerACL(filesystemType int64) bool { + switch filesystemType { + case unix.CIFS_SUPER_MAGIC, unix.SMB_SUPER_MAGIC, unix.SMB2_SUPER_MAGIC: + return true + default: + return false + } } func removeLinuxAccessACLs(file *os.File) error { diff --git a/safefileio/restrict_file_linux_test.go b/safefileio/restrict_file_linux_test.go new file mode 100644 index 0000000..d4882c3 --- /dev/null +++ b/safefileio/restrict_file_linux_test.go @@ -0,0 +1,21 @@ +package safefileio + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" +) + +func TestLinuxFilesystemRequiresServerACL(t *testing.T) { + for name, filesystemType := range map[string]int64{ + "CIFS": unix.CIFS_SUPER_MAGIC, + "SMB": unix.SMB_SUPER_MAGIC, + "SMB2": unix.SMB2_SUPER_MAGIC, + } { + t.Run(name, func(t *testing.T) { + assert.True(t, linuxFilesystemRequiresServerACL(filesystemType)) + }) + } + assert.False(t, linuxFilesystemRequiresServerACL(unix.EXT4_SUPER_MAGIC)) +} diff --git a/safefileio/restrict_file_supported_unix.go b/safefileio/restrict_file_supported_unix.go index 93556c4..8656832 100644 --- a/safefileio/restrict_file_supported_unix.go +++ b/safefileio/restrict_file_supported_unix.go @@ -2,20 +2,47 @@ package safefileio -import "os" +import ( + "fmt" + "os" +) func restrictCurrentUserFile( file *os.File, + preflight func(*os.File) error, removeACL func(*os.File) error, ) error { if err := ValidateCurrentUserFile(file); err != nil { return err } - if err := file.Chmod(0o600); err != nil { + if preflight != nil { + if err := preflight(file); err != nil { + return err + } + } + if err := setPrivateFileMode(file); err != nil { return err } if err := removeACL(file); err != nil { return err } - return file.Chmod(0o600) + return setPrivateFileMode(file) +} + +func setPrivateFileMode(file *os.File) error { + if err := file.Chmod(0o600); err != nil { + return err + } + return verifyPrivateFileMode(file) +} + +func verifyPrivateFileMode(file *os.File) error { + info, err := file.Stat() + if err != nil { + return err + } + if mode := info.Mode().Perm(); mode != 0o600 { + return fmt.Errorf("safefileio: file mode is %04o, not 0600", mode) + } + return nil } diff --git a/safefileio/restrict_file_supported_unix_test.go b/safefileio/restrict_file_supported_unix_test.go index 24027b7..b79e494 100644 --- a/safefileio/restrict_file_supported_unix_test.go +++ b/safefileio/restrict_file_supported_unix_test.go @@ -18,7 +18,7 @@ func TestRestrictCurrentUserFileNarrowsModeAroundACLRemoval(t *testing.T) { require.NoError(t, err) defer func() { _ = file.Close() }() - err = restrictCurrentUserFile(file, func(file *os.File) error { + err = restrictCurrentUserFile(file, nil, func(file *os.File) error { info, statErr := file.Stat() require.NoError(t, statErr) require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) @@ -29,3 +29,14 @@ func TestRestrictCurrentUserFileNarrowsModeAroundACLRemoval(t *testing.T) { require.NoError(t, err) require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) } + +func TestVerifyPrivateFileModeRejectsPublicMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + require.NoError(t, os.Chmod(path, 0o666)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.ErrorContains(t, verifyPrivateFileMode(file), "mode is 0666, not 0600") +} From 8101d25a2df4988cb43f6f6f5fbf1ff28ba7d1d0 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 21:16:29 -0500 Subject: [PATCH 07/10] Validate private files without repairing access Permission repair cannot revoke a descriptor another process already obtained while a file was broad. Replace the unreleased mutation API with handle-bound validation so broad mode bits, access ACLs, Windows DACLs, and SMB-family filesystems fail without changing the object; callers must create a private replacement instead.\n\nNormalizing Linux filesystem magic through uint32 also keeps CIFS and SMB2 rejection effective when 32-bit statfs values are sign-extended. Validation: reproduced the sign-extension bypass in a Linux test binary and executed the complete updated safefileio suite inside Linux. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/AGENTS.md | 17 ++-- safefileio/open_file_other.go | 6 +- safefileio/open_file_windows.go | 14 +-- safefileio/private_dir_windows.go | 15 +++- safefileio/private_dir_windows_test.go | 30 ++++++- safefileio/private_file_darwin.go | 86 +++++++++++++++++++ ...in_test.go => private_file_darwin_test.go} | 6 +- ...o => private_file_internal_darwin_test.go} | 4 +- safefileio/private_file_linux.go | 58 +++++++++++++ ...nux_test.go => private_file_linux_test.go} | 10 ++- safefileio/private_file_supported_unix.go | 32 +++++++ ...vate_file_supported_unix_external_test.go} | 20 ++--- ...go => private_file_supported_unix_test.go} | 10 +-- safefileio/private_file_unsupported_unix.go | 21 +++++ ... => private_file_unsupported_unix_test.go} | 6 +- safefileio/restrict_file_darwin.go | 82 ------------------ safefileio/restrict_file_linux.go | 56 ------------ safefileio/restrict_file_supported_unix.go | 48 ----------- safefileio/restrict_file_unsupported_unix.go | 21 ----- 19 files changed, 283 insertions(+), 259 deletions(-) create mode 100644 safefileio/private_file_darwin.go rename safefileio/{restrict_file_darwin_test.go => private_file_darwin_test.go} (76%) rename safefileio/{restrict_file_internal_darwin_test.go => private_file_internal_darwin_test.go} (62%) create mode 100644 safefileio/private_file_linux.go rename safefileio/{restrict_file_linux_test.go => private_file_linux_test.go} (58%) create mode 100644 safefileio/private_file_supported_unix.go rename safefileio/{restrict_file_supported_unix_test.go => private_file_supported_unix_external_test.go} (52%) rename safefileio/{restrict_file_supported_unix_external_test.go => private_file_supported_unix_test.go} (56%) create mode 100644 safefileio/private_file_unsupported_unix.go rename safefileio/{restrict_file_unsupported_unix_test.go => private_file_unsupported_unix_test.go} (64%) delete mode 100644 safefileio/restrict_file_darwin.go delete mode 100644 safefileio/restrict_file_linux.go delete mode 100644 safefileio/restrict_file_supported_unix.go delete mode 100644 safefileio/restrict_file_unsupported_unix.go diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index 86436d0..b3af9ef 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -20,14 +20,15 @@ callers responsible for their own file formats and higher-level policy. trusted user, token-owner, system, and administrator principals. Use SID semantics rather than username string comparisons. - If ownership or file type cannot be established, return an error. -- Restrict an open file only after validating that same handle's regular-file - type and current-user ownership; never repair an unverified path. -- On Unix platforms that support ACL removal, narrow mode bits before removing - ACLs and reapply the private mode afterward; never create a broader-access - interval while changing access-control policy. -- Verify the exact private mode through the open handle after each chmod. Reject - Linux SMB/CIFS filesystems because their server DACL cannot be secured by - local mode and access-ACL operations. +- Private-file validation must inspect the same open handle used by the caller + and must never mutate permissions. Existing broad access may already have + produced handles that no in-place repair can revoke. +- On supported Unix platforms, require exact mode 0600 and no access ACL. + Reject Linux SMB/CIFS filesystems because their server DACL cannot be verified + through local mode and access-ACL operations. +- On Windows, require a DACL that grants access only to the current user and + trusted administrative principals. Callers recovering a broad file must + create a private replacement rather than repair it in place. ## Tests diff --git a/safefileio/open_file_other.go b/safefileio/open_file_other.go index d8f0b42..a160c74 100644 --- a/safefileio/open_file_other.go +++ b/safefileio/open_file_other.go @@ -26,11 +26,11 @@ func ValidateCurrentUserFile(*os.File) error { ) } -// RestrictCurrentUserFile fails closed when the platform cannot enforce +// ValidatePrivateCurrentUserFile fails closed when the platform cannot verify // current-user-only file access. -func RestrictCurrentUserFile(*os.File) error { +func ValidatePrivateCurrentUserFile(*os.File) error { return fmt.Errorf( - "safefileio: current-user file restriction is unsupported on %s", + "safefileio: private current-user file validation is unsupported on %s", runtime.GOOS, ) } diff --git a/safefileio/open_file_windows.go b/safefileio/open_file_windows.go index e486340..552394e 100644 --- a/safefileio/open_file_windows.go +++ b/safefileio/open_file_windows.go @@ -57,9 +57,9 @@ func ValidateCurrentUserFile(file *os.File) error { return validateWindowsFileHandle(file.Name(), windows.Handle(file.Fd())) } -// RestrictCurrentUserFile validates an open handle and installs a protected -// DACL limited to the current user and Windows administrative principals. -func RestrictCurrentUserFile(file *os.File) error { +// ValidatePrivateCurrentUserFile verifies that an open current-user-owned file +// grants access only to the current user and Windows administrative principals. +func ValidatePrivateCurrentUserFile(file *os.File) error { handle, err := reopenWindowsFileForDACL(file) if err != nil { return err @@ -69,7 +69,11 @@ func RestrictCurrentUserFile(file *os.File) error { if err != nil { return err } - return restrictWindowsDir(handle, userSID) + ownerSID, err := currentWindowsOwnerSID() + if err != nil { + return err + } + return verifyWindowsFileDACL(file.Name(), handle, userSID, ownerSID) } func reopenWindowsFileForDACL(file *os.File) (windows.Handle, error) { @@ -78,7 +82,7 @@ func reopenWindowsFileForDACL(file *os.File) (windows.Handle, error) { } result, _, callErr := reOpenFile.Call( file.Fd(), - uintptr(windows.READ_CONTROL|windows.WRITE_DAC), + uintptr(windows.READ_CONTROL), uintptr(windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE), 0, ) diff --git a/safefileio/private_dir_windows.go b/safefileio/private_dir_windows.go index 50f0ccc..9089b7c 100644 --- a/safefileio/private_dir_windows.go +++ b/safefileio/private_dir_windows.go @@ -196,6 +196,19 @@ func verifyWindowsDirectoryOwner(path string, owner, userSID, ownerSID *windows. } func verifyWindowsDirDACL(path string, handle windows.Handle, userSID, ownerSID *windows.SID) error { + return verifyWindowsDACL(path, handle, userSID, ownerSID, true) +} + +func verifyWindowsFileDACL(path string, handle windows.Handle, userSID, ownerSID *windows.SID) error { + return verifyWindowsDACL(path, handle, userSID, ownerSID, false) +} + +func verifyWindowsDACL( + path string, + handle windows.Handle, + userSID, ownerSID *windows.SID, + requireProtected bool, +) error { descriptor, err := windows.GetSecurityInfo( handle, windows.SE_FILE_OBJECT, @@ -208,7 +221,7 @@ func verifyWindowsDirDACL(path string, handle windows.Handle, userSID, ownerSID if err != nil { return err } - if control&windows.SE_DACL_PROTECTED == 0 { + if requireProtected && control&windows.SE_DACL_PROTECTED == 0 { return fmt.Errorf("%s DACL is not protected", path) } dacl, _, err := descriptor.DACL() diff --git a/safefileio/private_dir_windows_test.go b/safefileio/private_dir_windows_test.go index 216a8f7..383e75c 100644 --- a/safefileio/private_dir_windows_test.go +++ b/safefileio/private_dir_windows_test.go @@ -97,13 +97,23 @@ func TestOpenCurrentUserFileAcceptsCurrentTokenOwner(t *testing.T) { require.NoError(t, file.Close()) } -func TestRestrictCurrentUserFileRepairsBroadDACL(t *testing.T) { +func TestValidatePrivateCurrentUserFileRejectsBroadDACL(t *testing.T) { path := filepath.Join(t.TempDir(), "record.json") require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) file, err := os.OpenFile(path, os.O_RDWR, 0) require.NoError(t, err) defer func() { _ = file.Close() }() - handle, err := reopenWindowsFileForDACL(file) + path16, err := windows.UTF16PtrFromString(path) + require.NoError(t, err) + handle, err := windows.CreateFile( + path16, + windows.READ_CONTROL|windows.WRITE_DAC, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) require.NoError(t, err) defer func() { _ = windows.CloseHandle(handle) }() userSID, err := currentWindowsUserSID() @@ -128,8 +138,20 @@ func TestRestrictCurrentUserFileRepairsBroadDACL(t *testing.T) { )) require.Error(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) - require.NoError(t, RestrictCurrentUserFile(file)) - require.NoError(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) + require.Error(t, ValidatePrivateCurrentUserFile(file)) + require.Error(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) +} + +func TestValidatePrivateCurrentUserFileAcceptsPrivateDACL(t *testing.T) { + dir := filepath.Join(t.TempDir(), "private") + require.NoError(t, EnsurePrivateDir(dir)) + path := filepath.Join(dir, "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.NoError(t, ValidatePrivateCurrentUserFile(file)) } func TestWindowsOwnerMatchesCurrentUserAndTokenOwner(t *testing.T) { diff --git a/safefileio/private_file_darwin.go b/safefileio/private_file_darwin.go new file mode 100644 index 0000000..db58755 --- /dev/null +++ b/safefileio/private_file_darwin.go @@ -0,0 +1,86 @@ +package safefileio + +import ( + "errors" + "fmt" + "os" + "sync" + "syscall" + + "github.com/ebitengine/purego" +) + +const ( + darwinACLExtended = 0x00000100 + darwinACLFirstEntry = 0 +) + +type darwinACLAPI struct { + getFD func(int32, int32) uintptr + getEntry func(uintptr, int32, *uintptr) int32 + free func(uintptr) int32 + errno func() *int32 +} + +var ( + darwinACLOnce sync.Once + darwinACL darwinACLAPI + darwinACLErr error +) + +// ValidatePrivateCurrentUserFile verifies that an open current-user-owned file +// has private mode bits and no macOS extended ACL. +func ValidatePrivateCurrentUserFile(file *os.File) error { + return validatePrivateCurrentUserFile(file, validateDarwinExtendedACL) +} + +func validateDarwinExtendedACL(file *os.File) error { + darwinACLOnce.Do(loadDarwinACL) + if darwinACLErr != nil { + return darwinACLErr + } + errno := darwinACL.errno() + *errno = 0 + acl := darwinACL.getFD(int32(file.Fd()), darwinACLExtended) + if acl == 0 { + callErr := syscall.Errno(*errno) + if callErr == syscall.ENOENT { + return nil + } + return fmt.Errorf("read extended ACL: %w", callErr) + } + defer func() { _ = darwinACL.free(acl) }() + var entry uintptr + result := darwinACL.getEntry(acl, darwinACLFirstEntry, &entry) + switch result { + case 0: + return errors.New("safefileio: file has a macOS extended ACL") + default: + return errors.New("read extended ACL entry failed") + } +} + +func loadDarwinACL() { + handle, err := purego.Dlopen( + "/usr/lib/libSystem.B.dylib", + purego.RTLD_NOW|purego.RTLD_LOCAL, + ) + if err != nil { + darwinACLErr = fmt.Errorf("load macOS ACL API: %w", err) + return + } + for name, target := range map[string]any{ + "acl_get_fd_np": &darwinACL.getFD, + "acl_get_entry": &darwinACL.getEntry, + "acl_free": &darwinACL.free, + "__error": &darwinACL.errno, + } { + symbol, symbolErr := purego.Dlsym(handle, name) + err = symbolErr + if err != nil { + darwinACLErr = fmt.Errorf("load macOS ACL function %s: %w", name, err) + return + } + purego.RegisterFunc(target, symbol) + } +} diff --git a/safefileio/restrict_file_darwin_test.go b/safefileio/private_file_darwin_test.go similarity index 76% rename from safefileio/restrict_file_darwin_test.go rename to safefileio/private_file_darwin_test.go index 695c2cc..6afa90f 100644 --- a/safefileio/restrict_file_darwin_test.go +++ b/safefileio/private_file_darwin_test.go @@ -11,7 +11,7 @@ import ( "go.kenn.io/kit/safefileio" ) -func TestRestrictCurrentUserFileRemovesExtendedACL(t *testing.T) { +func TestValidatePrivateCurrentUserFileRejectsExtendedACL(t *testing.T) { path := filepath.Join(t.TempDir(), "record.json") require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) output, err := exec.Command( @@ -25,8 +25,8 @@ func TestRestrictCurrentUserFileRemovesExtendedACL(t *testing.T) { require.NoError(t, err) defer func() { _ = file.Close() }() - require.NoError(t, safefileio.RestrictCurrentUserFile(file)) + require.Error(t, safefileio.ValidatePrivateCurrentUserFile(file)) listing, err := exec.Command("ls", "-lde", path).CombinedOutput() require.NoError(t, err, string(listing)) - assert.NotContains(t, string(listing), "everyone allow read") + assert.Contains(t, string(listing), "everyone allow read") } diff --git a/safefileio/restrict_file_internal_darwin_test.go b/safefileio/private_file_internal_darwin_test.go similarity index 62% rename from safefileio/restrict_file_internal_darwin_test.go rename to safefileio/private_file_internal_darwin_test.go index c5bbe07..66de9a2 100644 --- a/safefileio/restrict_file_internal_darwin_test.go +++ b/safefileio/private_file_internal_darwin_test.go @@ -7,10 +7,10 @@ import ( "github.com/stretchr/testify/require" ) -func TestRemoveDarwinExtendedACLRejectsFailedSet(t *testing.T) { +func TestValidateDarwinExtendedACLRejectsFailedInspection(t *testing.T) { file, err := os.CreateTemp(t.TempDir(), "record-*.json") require.NoError(t, err) require.NoError(t, file.Close()) - require.Error(t, removeDarwinExtendedACL(file)) + require.Error(t, validateDarwinExtendedACL(file)) } diff --git a/safefileio/private_file_linux.go b/safefileio/private_file_linux.go new file mode 100644 index 0000000..bd194e9 --- /dev/null +++ b/safefileio/private_file_linux.go @@ -0,0 +1,58 @@ +package safefileio + +import ( + "errors" + "fmt" + "os" + + "golang.org/x/sys/unix" +) + +// ValidatePrivateCurrentUserFile verifies that an open current-user-owned file +// has private mode bits and no access ACL that could grant another principal. +func ValidatePrivateCurrentUserFile(file *os.File) error { + return validatePrivateCurrentUserFile(file, validateLinuxPrivateAccess) +} + +func validateLinuxPrivateAccess(file *os.File) error { + var status unix.Statfs_t + if err := unix.Fstatfs(int(file.Fd()), &status); err != nil { + return fmt.Errorf("inspect file filesystem: %w", err) + } + if linuxFilesystemRequiresServerACL(int64(status.Type)) { + return errors.New( + "safefileio: private current-user file validation is unsupported " + + "on SMB/CIFS filesystems", + ) + } + return validateLinuxAccessACLs(file) +} + +func linuxFilesystemRequiresServerACL(filesystemType int64) bool { + switch uint32(filesystemType) { + case uint32(unix.CIFS_SUPER_MAGIC), + uint32(unix.SMB_SUPER_MAGIC), + uint32(unix.SMB2_SUPER_MAGIC): + return true + default: + return false + } +} + +func validateLinuxAccessACLs(file *os.File) error { + for _, attribute := range []string{ + "system.posix_acl_access", + "system.nfs4_acl", + "system.cifs_acl", + } { + _, err := unix.Fgetxattr(int(file.Fd()), attribute, nil) + if err == nil { + return fmt.Errorf("safefileio: file has access ACL %s", attribute) + } + if errors.Is(err, unix.ENODATA) || errors.Is(err, unix.ENOTSUP) { + continue + } + return fmt.Errorf("inspect access ACL %s: %w", attribute, err) + } + return nil +} diff --git a/safefileio/restrict_file_linux_test.go b/safefileio/private_file_linux_test.go similarity index 58% rename from safefileio/restrict_file_linux_test.go rename to safefileio/private_file_linux_test.go index d4882c3..0b66b73 100644 --- a/safefileio/restrict_file_linux_test.go +++ b/safefileio/private_file_linux_test.go @@ -8,10 +8,14 @@ import ( ) func TestLinuxFilesystemRequiresServerACL(t *testing.T) { + cifs := uint32(unix.CIFS_SUPER_MAGIC) + smb2 := uint32(unix.SMB2_SUPER_MAGIC) for name, filesystemType := range map[string]int64{ - "CIFS": unix.CIFS_SUPER_MAGIC, - "SMB": unix.SMB_SUPER_MAGIC, - "SMB2": unix.SMB2_SUPER_MAGIC, + "CIFS": int64(cifs), + "CIFS sign-extended": int64(int32(cifs)), + "SMB": unix.SMB_SUPER_MAGIC, + "SMB2": int64(smb2), + "SMB2 sign-extended": int64(int32(smb2)), } { t.Run(name, func(t *testing.T) { assert.True(t, linuxFilesystemRequiresServerACL(filesystemType)) diff --git a/safefileio/private_file_supported_unix.go b/safefileio/private_file_supported_unix.go new file mode 100644 index 0000000..f3f2f88 --- /dev/null +++ b/safefileio/private_file_supported_unix.go @@ -0,0 +1,32 @@ +//go:build darwin || linux + +package safefileio + +import ( + "fmt" + "os" +) + +func validatePrivateCurrentUserFile( + file *os.File, + validatePlatformAccess func(*os.File) error, +) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + if err := verifyPrivateFileMode(file); err != nil { + return err + } + return validatePlatformAccess(file) +} + +func verifyPrivateFileMode(file *os.File) error { + info, err := file.Stat() + if err != nil { + return err + } + if mode := info.Mode().Perm(); mode != 0o600 { + return fmt.Errorf("safefileio: file mode is %04o, not 0600", mode) + } + return nil +} diff --git a/safefileio/restrict_file_supported_unix_test.go b/safefileio/private_file_supported_unix_external_test.go similarity index 52% rename from safefileio/restrict_file_supported_unix_test.go rename to safefileio/private_file_supported_unix_external_test.go index b79e494..afb417a 100644 --- a/safefileio/restrict_file_supported_unix_test.go +++ b/safefileio/private_file_supported_unix_external_test.go @@ -1,6 +1,6 @@ //go:build darwin || linux -package safefileio +package safefileio_test import ( "os" @@ -8,9 +8,10 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" ) -func TestRestrictCurrentUserFileNarrowsModeAroundACLRemoval(t *testing.T) { +func TestValidatePrivateCurrentUserFileRejectsPublicMode(t *testing.T) { path := filepath.Join(t.TempDir(), "record.json") require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) require.NoError(t, os.Chmod(path, 0o666)) @@ -18,25 +19,18 @@ func TestRestrictCurrentUserFileNarrowsModeAroundACLRemoval(t *testing.T) { require.NoError(t, err) defer func() { _ = file.Close() }() - err = restrictCurrentUserFile(file, nil, func(file *os.File) error { - info, statErr := file.Stat() - require.NoError(t, statErr) - require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) - return file.Chmod(0o666) - }) - require.NoError(t, err) + require.Error(t, safefileio.ValidatePrivateCurrentUserFile(file)) info, err := file.Stat() require.NoError(t, err) - require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + require.Equal(t, os.FileMode(0o666), info.Mode().Perm()) } -func TestVerifyPrivateFileModeRejectsPublicMode(t *testing.T) { +func TestValidatePrivateCurrentUserFileAcceptsPrivateMode(t *testing.T) { path := filepath.Join(t.TempDir(), "record.json") require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) - require.NoError(t, os.Chmod(path, 0o666)) file, err := os.OpenFile(path, os.O_RDWR, 0) require.NoError(t, err) defer func() { _ = file.Close() }() - require.ErrorContains(t, verifyPrivateFileMode(file), "mode is 0666, not 0600") + require.NoError(t, safefileio.ValidatePrivateCurrentUserFile(file)) } diff --git a/safefileio/restrict_file_supported_unix_external_test.go b/safefileio/private_file_supported_unix_test.go similarity index 56% rename from safefileio/restrict_file_supported_unix_external_test.go rename to safefileio/private_file_supported_unix_test.go index 2149fb0..8f542e0 100644 --- a/safefileio/restrict_file_supported_unix_external_test.go +++ b/safefileio/private_file_supported_unix_test.go @@ -1,6 +1,6 @@ //go:build darwin || linux -package safefileio_test +package safefileio import ( "os" @@ -8,10 +8,9 @@ import ( "testing" "github.com/stretchr/testify/require" - "go.kenn.io/kit/safefileio" ) -func TestRestrictCurrentUserFileRepairsPublicMode(t *testing.T) { +func TestVerifyPrivateFileModeRejectsPublicMode(t *testing.T) { path := filepath.Join(t.TempDir(), "record.json") require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) require.NoError(t, os.Chmod(path, 0o666)) @@ -19,8 +18,5 @@ func TestRestrictCurrentUserFileRepairsPublicMode(t *testing.T) { require.NoError(t, err) defer func() { _ = file.Close() }() - require.NoError(t, safefileio.RestrictCurrentUserFile(file)) - info, err := file.Stat() - require.NoError(t, err) - require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + require.ErrorContains(t, verifyPrivateFileMode(file), "mode is 0666, not 0600") } diff --git a/safefileio/private_file_unsupported_unix.go b/safefileio/private_file_unsupported_unix.go new file mode 100644 index 0000000..db0d985 --- /dev/null +++ b/safefileio/private_file_unsupported_unix.go @@ -0,0 +1,21 @@ +//go:build unix && !darwin && !linux + +package safefileio + +import ( + "fmt" + "os" + "runtime" +) + +// ValidatePrivateCurrentUserFile fails closed on Unix platforms where Kit +// cannot inspect access-control lists through the verified file handle. +func ValidatePrivateCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + return fmt.Errorf( + "safefileio: private current-user file validation is unsupported on %s", + runtime.GOOS, + ) +} diff --git a/safefileio/restrict_file_unsupported_unix_test.go b/safefileio/private_file_unsupported_unix_test.go similarity index 64% rename from safefileio/restrict_file_unsupported_unix_test.go rename to safefileio/private_file_unsupported_unix_test.go index 8439316..064b022 100644 --- a/safefileio/restrict_file_unsupported_unix_test.go +++ b/safefileio/private_file_unsupported_unix_test.go @@ -11,13 +11,13 @@ import ( "go.kenn.io/kit/safefileio" ) -func TestRestrictCurrentUserFileFailsClosedWhenUnsupported(t *testing.T) { +func TestValidatePrivateCurrentUserFileFailsClosedWhenUnsupported(t *testing.T) { path := filepath.Join(t.TempDir(), "record.json") require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) file, err := os.OpenFile(path, os.O_RDWR, 0) require.NoError(t, err) defer func() { _ = file.Close() }() - err = safefileio.RestrictCurrentUserFile(file) - require.ErrorContains(t, err, "current-user file restriction is unsupported") + err = safefileio.ValidatePrivateCurrentUserFile(file) + require.ErrorContains(t, err, "private current-user file validation is unsupported") } diff --git a/safefileio/restrict_file_darwin.go b/safefileio/restrict_file_darwin.go deleted file mode 100644 index 078f38c..0000000 --- a/safefileio/restrict_file_darwin.go +++ /dev/null @@ -1,82 +0,0 @@ -package safefileio - -import ( - "errors" - "fmt" - "os" - "sync" - "syscall" - - "github.com/ebitengine/purego" -) - -const darwinACLExtended = 0x00000100 - -type darwinACLAPI struct { - init uintptr - set uintptr - free uintptr -} - -var ( - darwinACLOnce sync.Once - darwinACL darwinACLAPI - darwinACLErr error -) - -// RestrictCurrentUserFile validates an open handle, narrows its mode, removes -// its macOS extended ACL, and keeps it readable and writable only by its owner. -func RestrictCurrentUserFile(file *os.File) error { - return restrictCurrentUserFile(file, nil, removeDarwinExtendedACL) -} - -func removeDarwinExtendedACL(file *os.File) error { - darwinACLOnce.Do(loadDarwinACL) - if darwinACLErr != nil { - return darwinACLErr - } - acl, _, callErr := purego.SyscallN(darwinACL.init, 0) - if acl == 0 { - return darwinACLCallError("initialize empty ACL", callErr) - } - defer func() { _, _, _ = purego.SyscallN(darwinACL.free, acl) }() - result, _, callErr := purego.SyscallN( - darwinACL.set, - file.Fd(), - acl, - darwinACLExtended, - ) - if result != 0 { - return darwinACLCallError("remove extended ACL", callErr) - } - return nil -} - -func loadDarwinACL() { - handle, err := purego.Dlopen( - "/usr/lib/libSystem.B.dylib", - purego.RTLD_NOW|purego.RTLD_LOCAL, - ) - if err != nil { - darwinACLErr = fmt.Errorf("load macOS ACL API: %w", err) - return - } - for name, target := range map[string]*uintptr{ - "acl_init": &darwinACL.init, - "acl_set_fd_np": &darwinACL.set, - "acl_free": &darwinACL.free, - } { - *target, err = purego.Dlsym(handle, name) - if err != nil { - darwinACLErr = fmt.Errorf("load macOS ACL function %s: %w", name, err) - return - } - } -} - -func darwinACLCallError(operation string, value uintptr) error { - if value != 0 { - return fmt.Errorf("%s: %w", operation, syscall.Errno(value)) - } - return errors.New(operation + " failed") -} diff --git a/safefileio/restrict_file_linux.go b/safefileio/restrict_file_linux.go deleted file mode 100644 index 811f8b1..0000000 --- a/safefileio/restrict_file_linux.go +++ /dev/null @@ -1,56 +0,0 @@ -package safefileio - -import ( - "errors" - "fmt" - "os" - - "golang.org/x/sys/unix" -) - -// RestrictCurrentUserFile validates an open handle, narrows its mode, removes -// access ACLs, and keeps it readable and writable only by its current-user owner. -func RestrictCurrentUserFile(file *os.File) error { - return restrictCurrentUserFile( - file, - validateLinuxRestrictionFilesystem, - removeLinuxAccessACLs, - ) -} - -func validateLinuxRestrictionFilesystem(file *os.File) error { - var status unix.Statfs_t - if err := unix.Fstatfs(int(file.Fd()), &status); err != nil { - return fmt.Errorf("inspect file filesystem: %w", err) - } - if linuxFilesystemRequiresServerACL(int64(status.Type)) { - return errors.New( - "safefileio: current-user file restriction is unsupported " + - "on SMB/CIFS filesystems", - ) - } - return nil -} - -func linuxFilesystemRequiresServerACL(filesystemType int64) bool { - switch filesystemType { - case unix.CIFS_SUPER_MAGIC, unix.SMB_SUPER_MAGIC, unix.SMB2_SUPER_MAGIC: - return true - default: - return false - } -} - -func removeLinuxAccessACLs(file *os.File) error { - for _, attribute := range []string{ - "system.posix_acl_access", - "system.nfs4_acl", - } { - err := unix.Fremovexattr(int(file.Fd()), attribute) - if err != nil && !errors.Is(err, unix.ENODATA) && - !errors.Is(err, unix.ENOTSUP) { - return fmt.Errorf("remove access ACL %s: %w", attribute, err) - } - } - return nil -} diff --git a/safefileio/restrict_file_supported_unix.go b/safefileio/restrict_file_supported_unix.go deleted file mode 100644 index 8656832..0000000 --- a/safefileio/restrict_file_supported_unix.go +++ /dev/null @@ -1,48 +0,0 @@ -//go:build darwin || linux - -package safefileio - -import ( - "fmt" - "os" -) - -func restrictCurrentUserFile( - file *os.File, - preflight func(*os.File) error, - removeACL func(*os.File) error, -) error { - if err := ValidateCurrentUserFile(file); err != nil { - return err - } - if preflight != nil { - if err := preflight(file); err != nil { - return err - } - } - if err := setPrivateFileMode(file); err != nil { - return err - } - if err := removeACL(file); err != nil { - return err - } - return setPrivateFileMode(file) -} - -func setPrivateFileMode(file *os.File) error { - if err := file.Chmod(0o600); err != nil { - return err - } - return verifyPrivateFileMode(file) -} - -func verifyPrivateFileMode(file *os.File) error { - info, err := file.Stat() - if err != nil { - return err - } - if mode := info.Mode().Perm(); mode != 0o600 { - return fmt.Errorf("safefileio: file mode is %04o, not 0600", mode) - } - return nil -} diff --git a/safefileio/restrict_file_unsupported_unix.go b/safefileio/restrict_file_unsupported_unix.go deleted file mode 100644 index dc18541..0000000 --- a/safefileio/restrict_file_unsupported_unix.go +++ /dev/null @@ -1,21 +0,0 @@ -//go:build unix && !darwin && !linux - -package safefileio - -import ( - "fmt" - "os" - "runtime" -) - -// RestrictCurrentUserFile fails closed on Unix platforms where Kit cannot -// remove access-control lists through the verified file handle. -func RestrictCurrentUserFile(file *os.File) error { - if err := ValidateCurrentUserFile(file); err != nil { - return err - } - return fmt.Errorf( - "safefileio: current-user file restriction is unsupported on %s", - runtime.GOOS, - ) -} From 07828b581914b1f56e580e67584d59b39f425e50 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 21:23:08 -0500 Subject: [PATCH 08/10] Keep Darwin ACL errno thread-local The ACL probe and libc errno pointer must refer to the same OS thread; otherwise goroutine migration can turn an inspection failure into a false no-ACL result. Pinning the short native-call sequence preserves the validation-only contract while retaining typed PureGo function signatures. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/private_file_darwin.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/safefileio/private_file_darwin.go b/safefileio/private_file_darwin.go index db58755..23b0860 100644 --- a/safefileio/private_file_darwin.go +++ b/safefileio/private_file_darwin.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "runtime" "sync" "syscall" @@ -39,6 +40,8 @@ func validateDarwinExtendedACL(file *os.File) error { if darwinACLErr != nil { return darwinACLErr } + runtime.LockOSThread() + defer runtime.UnlockOSThread() errno := darwinACL.errno() *errno = 0 acl := darwinACL.getFD(int32(file.Fd()), darwinACLExtended) From 9ac7634d42f9fe80f713d245b3436fe2fe361d71 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 21:33:09 -0500 Subject: [PATCH 09/10] Require protected Windows file DACLs A DACL limited to trusted principals is still mutable through inheritance while it remains unprotected. Treat inheritable files as unsafe so validation cannot be invalidated by later ACE propagation from an attacker-controlled parent; recovery continues to require a separately created protected replacement. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/AGENTS.md | 7 ++-- safefileio/open_file_windows.go | 3 +- safefileio/private_dir_windows.go | 7 ++-- safefileio/private_dir_windows_test.go | 58 +++++++++++++++++++++++++- 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index b3af9ef..1e2c01e 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -26,9 +26,10 @@ callers responsible for their own file formats and higher-level policy. - On supported Unix platforms, require exact mode 0600 and no access ACL. Reject Linux SMB/CIFS filesystems because their server DACL cannot be verified through local mode and access-ACL operations. -- On Windows, require a DACL that grants access only to the current user and - trusted administrative principals. Callers recovering a broad file must - create a private replacement rather than repair it in place. +- On Windows, require a protected DACL that grants access only to the current + user and trusted administrative principals. Callers recovering a broad or + inheritable file must create a private replacement rather than repair it in + place. ## Tests diff --git a/safefileio/open_file_windows.go b/safefileio/open_file_windows.go index 552394e..0c4e602 100644 --- a/safefileio/open_file_windows.go +++ b/safefileio/open_file_windows.go @@ -58,7 +58,8 @@ func ValidateCurrentUserFile(file *os.File) error { } // ValidatePrivateCurrentUserFile verifies that an open current-user-owned file -// grants access only to the current user and Windows administrative principals. +// has a protected DACL granting access only to the current user and Windows +// administrative principals. func ValidatePrivateCurrentUserFile(file *os.File) error { handle, err := reopenWindowsFileForDACL(file) if err != nil { diff --git a/safefileio/private_dir_windows.go b/safefileio/private_dir_windows.go index 9089b7c..cb27da7 100644 --- a/safefileio/private_dir_windows.go +++ b/safefileio/private_dir_windows.go @@ -196,18 +196,17 @@ func verifyWindowsDirectoryOwner(path string, owner, userSID, ownerSID *windows. } func verifyWindowsDirDACL(path string, handle windows.Handle, userSID, ownerSID *windows.SID) error { - return verifyWindowsDACL(path, handle, userSID, ownerSID, true) + return verifyWindowsDACL(path, handle, userSID, ownerSID) } func verifyWindowsFileDACL(path string, handle windows.Handle, userSID, ownerSID *windows.SID) error { - return verifyWindowsDACL(path, handle, userSID, ownerSID, false) + return verifyWindowsDACL(path, handle, userSID, ownerSID) } func verifyWindowsDACL( path string, handle windows.Handle, userSID, ownerSID *windows.SID, - requireProtected bool, ) error { descriptor, err := windows.GetSecurityInfo( handle, @@ -221,7 +220,7 @@ func verifyWindowsDACL( if err != nil { return err } - if requireProtected && control&windows.SE_DACL_PROTECTED == 0 { + if control&windows.SE_DACL_PROTECTED == 0 { return fmt.Errorf("%s DACL is not protected", path) } dacl, _, err := descriptor.DACL() diff --git a/safefileio/private_dir_windows_test.go b/safefileio/private_dir_windows_test.go index 383e75c..991713c 100644 --- a/safefileio/private_dir_windows_test.go +++ b/safefileio/private_dir_windows_test.go @@ -142,7 +142,7 @@ func TestValidatePrivateCurrentUserFileRejectsBroadDACL(t *testing.T) { require.Error(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) } -func TestValidatePrivateCurrentUserFileAcceptsPrivateDACL(t *testing.T) { +func TestValidatePrivateCurrentUserFileRejectsUnprotectedPrivateDACL(t *testing.T) { dir := filepath.Join(t.TempDir(), "private") require.NoError(t, EnsurePrivateDir(dir)) path := filepath.Join(dir, "record.json") @@ -150,6 +150,62 @@ func TestValidatePrivateCurrentUserFileAcceptsPrivateDACL(t *testing.T) { file, err := os.OpenFile(path, os.O_RDWR, 0) require.NoError(t, err) defer func() { _ = file.Close() }() + path16, err := windows.UTF16PtrFromString(path) + require.NoError(t, err) + handle, err := windows.CreateFile( + path16, + windows.READ_CONTROL|windows.WRITE_DAC, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(handle) }() + userSID, err := currentWindowsUserSID() + require.NoError(t, err) + acl, err := windows.ACLFromEntries([]windows.EXPLICIT_ACCESS{ + allowFullControl(userSID, windows.TRUSTEE_IS_USER), + }, nil) + require.NoError(t, err) + require.NoError(t, windows.SetSecurityInfo( + handle, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.UNPROTECTED_DACL_SECURITY_INFORMATION, + nil, + nil, + acl, + nil, + )) + + require.Error(t, ValidatePrivateCurrentUserFile(file)) +} + +func TestValidatePrivateCurrentUserFileAcceptsProtectedPrivateDACL(t *testing.T) { + dir := filepath.Join(t.TempDir(), "private") + require.NoError(t, EnsurePrivateDir(dir)) + path := filepath.Join(dir, "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + path16, err := windows.UTF16PtrFromString(path) + require.NoError(t, err) + handle, err := windows.CreateFile( + path16, + windows.READ_CONTROL|windows.WRITE_DAC, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(handle) }() + userSID, err := currentWindowsUserSID() + require.NoError(t, err) + require.NoError(t, restrictWindowsDir(handle, userSID)) require.NoError(t, ValidatePrivateCurrentUserFile(file)) } From e4927353fc7a8c5ea807d2db00c0475a21b48b60 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Fri, 7 Aug 2026 21:42:13 -0500 Subject: [PATCH 10/10] Reject Linux filesystems with external access policy AFS and similar network or user-space filesystems can grant access outside local file modes and access ACLs, so treating them as supported can falsely validate secret-bearing files as private. Classify these filesystems through statfs and fail closed while retaining uint32 normalization for 32-bit Linux. Validated with the safefileio suite in a Linux container, native vet and tests, Linux 386/arm cross-builds, and Windows/Darwin cross-builds. Generated with OpenAI Codex Co-authored-by: OpenAI Codex --- safefileio/AGENTS.md | 4 ++-- safefileio/private_file_linux.go | 19 +++++++++++++----- safefileio/private_file_linux_test.go | 28 ++++++++++++++++----------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index 1e2c01e..671880b 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -24,8 +24,8 @@ callers responsible for their own file formats and higher-level policy. and must never mutate permissions. Existing broad access may already have produced handles that no in-place repair can revoke. - On supported Unix platforms, require exact mode 0600 and no access ACL. - Reject Linux SMB/CIFS filesystems because their server DACL cannot be verified - through local mode and access-ACL operations. + Reject Linux network and user-space filesystems whose effective access policy + cannot be verified through local mode and access-ACL operations. - On Windows, require a protected DACL that grants access only to the current user and trusted administrative principals. Callers recovering a broad or inheritable file must create a private replacement rather than repair it in diff --git a/safefileio/private_file_linux.go b/safefileio/private_file_linux.go index bd194e9..2f612c0 100644 --- a/safefileio/private_file_linux.go +++ b/safefileio/private_file_linux.go @@ -19,20 +19,29 @@ func validateLinuxPrivateAccess(file *os.File) error { if err := unix.Fstatfs(int(file.Fd()), &status); err != nil { return fmt.Errorf("inspect file filesystem: %w", err) } - if linuxFilesystemRequiresServerACL(int64(status.Type)) { + if linuxFilesystemHasExternalAccessPolicy(int64(status.Type)) { return errors.New( "safefileio: private current-user file validation is unsupported " + - "on SMB/CIFS filesystems", + "on filesystems with external access policy", ) } return validateLinuxAccessACLs(file) } -func linuxFilesystemRequiresServerACL(filesystemType int64) bool { +func linuxFilesystemHasExternalAccessPolicy(filesystemType int64) bool { switch uint32(filesystemType) { - case uint32(unix.CIFS_SUPER_MAGIC), + case uint32(unix.AAFS_MAGIC), + uint32(unix.AFS_FS_MAGIC), + uint32(unix.AFS_SUPER_MAGIC), + uint32(unix.CEPH_SUPER_MAGIC), + uint32(unix.CIFS_SUPER_MAGIC), + uint32(unix.CODA_SUPER_MAGIC), + uint32(unix.FUSE_SUPER_MAGIC), + uint32(unix.NCP_SUPER_MAGIC), + uint32(unix.NFS_SUPER_MAGIC), uint32(unix.SMB_SUPER_MAGIC), - uint32(unix.SMB2_SUPER_MAGIC): + uint32(unix.SMB2_SUPER_MAGIC), + uint32(unix.V9FS_MAGIC): return true default: return false diff --git a/safefileio/private_file_linux_test.go b/safefileio/private_file_linux_test.go index 0b66b73..e40ad80 100644 --- a/safefileio/private_file_linux_test.go +++ b/safefileio/private_file_linux_test.go @@ -7,19 +7,25 @@ import ( "golang.org/x/sys/unix" ) -func TestLinuxFilesystemRequiresServerACL(t *testing.T) { - cifs := uint32(unix.CIFS_SUPER_MAGIC) - smb2 := uint32(unix.SMB2_SUPER_MAGIC) - for name, filesystemType := range map[string]int64{ - "CIFS": int64(cifs), - "CIFS sign-extended": int64(int32(cifs)), - "SMB": unix.SMB_SUPER_MAGIC, - "SMB2": int64(smb2), - "SMB2 sign-extended": int64(int32(smb2)), +func TestLinuxFilesystemHasExternalAccessPolicy(t *testing.T) { + for name, magic := range map[string]uint32{ + "Andrew FS": uint32(unix.AAFS_MAGIC), + "AFS fs": uint32(unix.AFS_FS_MAGIC), + "AFS": uint32(unix.AFS_SUPER_MAGIC), + "Ceph": uint32(unix.CEPH_SUPER_MAGIC), + "CIFS": uint32(unix.CIFS_SUPER_MAGIC), + "Coda": uint32(unix.CODA_SUPER_MAGIC), + "FUSE": uint32(unix.FUSE_SUPER_MAGIC), + "NCP": uint32(unix.NCP_SUPER_MAGIC), + "NFS": uint32(unix.NFS_SUPER_MAGIC), + "SMB": uint32(unix.SMB_SUPER_MAGIC), + "SMB2": uint32(unix.SMB2_SUPER_MAGIC), + "Plan 9 filesystem": uint32(unix.V9FS_MAGIC), } { t.Run(name, func(t *testing.T) { - assert.True(t, linuxFilesystemRequiresServerACL(filesystemType)) + assert.True(t, linuxFilesystemHasExternalAccessPolicy(int64(magic))) + assert.True(t, linuxFilesystemHasExternalAccessPolicy(int64(int32(magic)))) }) } - assert.False(t, linuxFilesystemRequiresServerACL(unix.EXT4_SUPER_MAGIC)) + assert.False(t, linuxFilesystemHasExternalAccessPolicy(unix.EXT4_SUPER_MAGIC)) }