fix: close Windows workspace output sharing gap
This commit is contained in:
@@ -14,7 +14,14 @@ import (
|
||||
"golang.org/x/sys/windows"
|
||||
)
|
||||
|
||||
const windowsRetainedHandleShareMode uint32 = windows.FILE_SHARE_READ | windows.FILE_SHARE_WRITE
|
||||
const (
|
||||
// Retained input handles deny delete sharing while permitting ordinary reads
|
||||
// and writes by trusted callers.
|
||||
windowsRetainedHandleShareMode uint32 = windows.FILE_SHARE_READ | windows.FILE_SHARE_WRITE
|
||||
// Outputs are opened for exclusive publication: deny write/delete sharing,
|
||||
// but permit the exact-identity read recheck below.
|
||||
windowsOutputHandleShareMode uint32 = windows.FILE_SHARE_READ
|
||||
)
|
||||
|
||||
// ReadCanonicalRegular opens each component with FILE_FLAG_OPEN_REPARSE_POINT and rejects a
|
||||
// reparse point on the opened handle before opening the next component. Retained handles allow
|
||||
@@ -136,7 +143,7 @@ func writeCanonicalExclusive(path string, contents []byte, mode fs.FileMode) err
|
||||
if err != nil {
|
||||
return ErrUnsafeFile
|
||||
}
|
||||
h, err := windows.CreateFile(windows.StringToUTF16Ptr(filepath.Join(parent, filepath.Base(path))), windows.GENERIC_WRITE, 0, securityAttributes, windows.CREATE_NEW, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT, 0)
|
||||
h, err := windows.CreateFile(windows.StringToUTF16Ptr(filepath.Join(parent, filepath.Base(path))), windows.GENERIC_WRITE, windowsOutputHandleShareMode, securityAttributes, windows.CREATE_NEW, windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT, 0)
|
||||
if err != nil {
|
||||
return ErrUnsafeFile
|
||||
}
|
||||
|
||||
@@ -3,19 +3,24 @@
|
||||
package safeio
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/aritmolab/thothii/tools/thothctl/internal/testsupport"
|
||||
"golang.org/x/sys/windows"
|
||||
)
|
||||
|
||||
const expectedWindowsRetainedHandleShareMode = windows.FILE_SHARE_READ | windows.FILE_SHARE_WRITE
|
||||
const expectedWindowsOutputHandleShareMode = windows.FILE_SHARE_READ
|
||||
|
||||
// Keep this contract compile-enforced so Windows cross-test compilation catches a future
|
||||
// FILE_SHARE_DELETE regression even when the tests are compiled on a non-Windows host.
|
||||
var _ [windowsRetainedHandleShareMode - expectedWindowsRetainedHandleShareMode]struct{}
|
||||
var _ [expectedWindowsRetainedHandleShareMode - windowsRetainedHandleShareMode]struct{}
|
||||
var _ [windowsOutputHandleShareMode - expectedWindowsOutputHandleShareMode]struct{}
|
||||
var _ [expectedWindowsOutputHandleShareMode - windowsOutputHandleShareMode]struct{}
|
||||
|
||||
func TestOpenWindowsComponentBlocksMutationWhileHandleIsRetained(t *testing.T) {
|
||||
t.Run("parent rename", func(t *testing.T) {
|
||||
@@ -98,3 +103,41 @@ func TestWriteCanonicalExclusiveCreatesProtectedOwnerOnlyDACL(t *testing.T) {
|
||||
t.Fatalf("output DACL = %#v, err=%v; want one owner ACE", acl, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteCanonicalExclusiveAllowsOwnerOnlyWriteAndIdentityRecheck(t *testing.T) {
|
||||
root, err := filepath.EvalSymlinks(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
path := filepath.Join(root, "candidate.yaml")
|
||||
if err := writeCanonicalExclusive(path, []byte("candidates: []\n"), 0o600); err != nil {
|
||||
t.Fatalf("owner-only output write/recheck failed: %v", err)
|
||||
}
|
||||
contents, err := os.ReadFile(path)
|
||||
if err != nil || string(contents) != "candidates: []\n" {
|
||||
t.Fatalf("output = %q, err=%v", contents, err)
|
||||
}
|
||||
if err := writeCanonicalExclusive(path, []byte("replacement\n"), 0o600); !errors.Is(err, ErrUnsafeFile) {
|
||||
t.Fatalf("existing output replacement = %v, want ErrUnsafeFile", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteCanonicalExclusiveRejectsReparseParent(t *testing.T) {
|
||||
root, err := filepath.EvalSymlinks(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
realParent := filepath.Join(root, "real-parent")
|
||||
if err := os.Mkdir(realParent, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
linkedParent := filepath.Join(root, "linked-parent")
|
||||
testsupport.SymlinkOrSkip(t, realParent, linkedParent)
|
||||
path := filepath.Join(linkedParent, "candidate.yaml")
|
||||
if err := writeCanonicalExclusive(path, []byte("unsafe\n"), 0o600); !errors.Is(err, ErrUnsafeFile) {
|
||||
t.Fatalf("reparse parent output = %v, want ErrUnsafeFile", err)
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(realParent, "candidate.yaml")); !os.IsNotExist(err) {
|
||||
t.Fatalf("reparse parent write created target: stat err=%v", err)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user