diff --git a/internal/sandbox/windows_acl.go b/internal/sandbox/windows_acl.go index 96a18004a..c7d47956e 100644 --- a/internal/sandbox/windows_acl.go +++ b/internal/sandbox/windows_acl.go @@ -33,8 +33,14 @@ type WindowsACLEntry struct { // down onto the target's EXISTING descendants (not just new ones it // creates going forward), which is why direct-only denies must set this // flag rather than rely on inheritance. - NoInherit bool `json:"noInherit,omitempty"` - Materialize bool `json:"materialize,omitempty"` + NoInherit bool `json:"noInherit,omitempty"` + // Anchor is the write root Path was DERIVED from, for the paths this + // package constructs rather than the operator naming. The apply requires + // the object it finally opens to still live under it, so a reparse point + // planted on the derived tail cannot walk an elevated ACL write out of the + // sandbox. Empty for an operator-named path, which has no owned tail. + Anchor string `json:"anchor,omitempty"` + Materialize bool `json:"materialize,omitempty"` } type WindowsACLPlan struct { @@ -57,10 +63,22 @@ func BuildWindowsACLPlan(config WindowsSandboxCommandConfig) (WindowsACLPlan, er Capability: capability.SID, }) for _, path := range capability.ProtectedWriteDenyPaths { + // ANCHORED ONLY WHERE THE ANCHOR IS TRUE. Most of these are derived + // from the root and sit under it, and holding those inside it is the + // point. But ReadOnlySubpaths is a profile field an operator can set to + // any path, and one deliberately placed outside the root is a + // configuration that works today; anchoring it would turn that into a + // containment refusal. A path that is not under the root gets no + // anchor and keeps the final-component guard it always had. + anchor := "" + if pathWithinRoot(capability.Root, path) { + anchor = capability.Root + } entries = append(entries, WindowsACLEntry{ Action: WindowsACLDenyWrite, Path: path, Capability: capability.SID, + Anchor: anchor, }) } } diff --git a/internal/sandbox/windows_acl_apply_windows.go b/internal/sandbox/windows_acl_apply_windows.go index dcacf68e9..e8ecb008c 100644 --- a/internal/sandbox/windows_acl_apply_windows.go +++ b/internal/sandbox/windows_acl_apply_windows.go @@ -23,7 +23,10 @@ const ( ) type windowsACLPathGroup struct { - Path string + Path string + // Anchor is the write root Path was derived from, empty when the operator + // named the path. See verifyWindowsACLHandleUnderAnchor. + Anchor string Entries []WindowsACLEntry Materialize bool } @@ -69,6 +72,12 @@ func groupWindowsACLPlanByPath(plan WindowsACLPlan) []windowsACLPathGroup { } group.Entries = append(group.Entries, entry) group.Materialize = group.Materialize || entry.Materialize + // One path is derived from at most one root, so the first anchor seen is + // the anchor. Taking it rather than overwriting keeps a later + // operator-named duplicate of the same path from clearing it. + if group.Anchor == "" { + group.Anchor = entry.Anchor + } } out := make([]windowsACLPathGroup, 0, len(byPath)) for _, group := range byPath { @@ -104,6 +113,13 @@ func applyWindowsACLPathGroup(group windowsACLPathGroup) (windowsACLSnapshot, bo } return windowsACLSnapshot{}, false, nil } + // Before creating anything: os.MkdirAll walks a pathname and follows every + // reparse point on it, so a junction on the derived tail would have this + // elevated setup create the directory outside the write root and only the + // containment check below would notice, after the fact. + if err := verifyWindowsACLPathUnderAnchor(group.Anchor, path); err != nil { + return windowsACLSnapshot{}, false, err + } if err := os.MkdirAll(path, 0o700); err != nil { return windowsACLSnapshot{}, false, fmt.Errorf("materialize windows ACL target %s: %w", path, err) } @@ -123,6 +139,15 @@ func applyWindowsACLPathGroup(group windowsACLPathGroup) (windowsACLSnapshot, bo } return windowsACLSnapshot{}, false, err } + // THE OBJECT HAS TO BE WHERE THE PLAN SAID IT WOULD BE. The open above + // refuses a reparse point at the final component and resolves every one + // above it, which is right for a path the operator named and not enough for + // one this package derived from a write root: a junction on the derived tail + // redirects the handle out of the sandbox with nothing about the final + // object looking wrong. + if err := verifyWindowsACLHandleUnderAnchor(handle, group.Anchor, path); err != nil { + return fail(err) + } descriptor, err := windows.GetSecurityInfo(handle, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) if err != nil { return fail(fmt.Errorf("read windows ACL for %s: %w", path, err)) diff --git a/internal/sandbox/windows_acl_containment_windows.go b/internal/sandbox/windows_acl_containment_windows.go new file mode 100644 index 000000000..704d46885 --- /dev/null +++ b/internal/sandbox/windows_acl_containment_windows.go @@ -0,0 +1,228 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "golang.org/x/sys/windows" +) + +// Keeping an elevated ACL write inside the tree it was planned for. +// +// FILE_FLAG_OPEN_REPARSE_POINT GUARDS ONE COMPONENT, AND A PATH HAS MANY. The +// apply opens its target with that flag, which stops the FINAL component from +// being followed and refuses the handle when it is a reparse point. Every +// component above it is resolved normally, as it must be for any absolute path +// to work at all. +// +// That is enough for a target the operator named, and not enough for one the +// sandbox derived. The write-root carveouts are derived: /.git/hooks and +// /.git/config are constructed from a root, and /.git is a name an +// unprivileged workspace writer can create. `mklink /J` needs no privilege, so +// a junction there makes the apply open /hooks instead, and +// `zero sandbox setup` writes a deny-write ACE, as Administrator, on an object +// outside the workspace. The final-component flag never fires: the object at +// the end of the walk is an ordinary directory. +// +// So the derived targets carry the root they were derived FROM, and the object +// finally opened has to still be inside it. The check is on the handle, not on +// the name: GetFinalPathNameByHandle answers where the object this process is +// holding actually lives, and both sides of the comparison go through it so the +// spelling is normalized the same way (\\?\ prefix, long names, drive letter). +// A junction anywhere along the derived tail moves the answer out of the root +// and the apply refuses. +// +// The rule is deliberately not "no reparse point anywhere in the path". Above +// the write root the path belongs to the operator, who may legitimately keep a +// workspace under a junction or a mapped directory, and refusing that would +// break setups this has nothing to say about. Only the tail the sandbox itself +// appended is held strict. + +// windowsFinalPathByHandle reports where an open handle's object actually lives. +func windowsFinalPathByHandle(handle windows.Handle) (string, error) { + buffer := make([]uint16, windows.MAX_LONG_PATH) + n, err := windows.GetFinalPathNameByHandle(handle, &buffer[0], uint32(len(buffer)), 0) + if err != nil { + return "", err + } + if int(n) > len(buffer) { + buffer = make([]uint16, n) + n, err = windows.GetFinalPathNameByHandle(handle, &buffer[0], uint32(len(buffer)), 0) + if err != nil { + return "", err + } + } + return windows.UTF16ToString(buffer[:n]), nil +} + +// windowsFinalPathOf opens path without following a final-component reparse +// point and reports where the object lives. A missing path is surfaced as +// os.ErrNotExist so callers can walk up to a parent that exists. +func windowsFinalPathOf(path string) (string, error) { + utf16Path, err := windows.UTF16PtrFromString(path) + if err != nil { + return "", fmt.Errorf("encode windows path %s: %w", path, err) + } + handle, err := windows.CreateFile( + utf16Path, + windows.READ_CONTROL, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_BACKUP_SEMANTICS|windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + if err != nil { + return "", fmt.Errorf("open windows path %s: %w", path, err) + } + defer func() { _ = windows.CloseHandle(handle) }() + final, err := windowsFinalPathByHandle(handle) + if err != nil { + return "", fmt.Errorf("resolve windows path %s: %w", path, err) + } + return final, nil +} + +// windowsACLTailUnderAnchor returns the components target adds to anchor, and +// whether target is under anchor lexically at all. +func windowsACLTailUnderAnchor(anchor, target string) (string, bool) { + relative, err := filepath.Rel(filepath.Clean(anchor), filepath.Clean(target)) + if err != nil { + return "", false + } + if relative == "." { + return "", true + } + if relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return "", false + } + return relative, true +} + +// windowsACLContainmentError reports a derived target that resolved outside the +// root it was derived from. +type windowsACLContainmentError struct { + Anchor string + Target string + Actual string +} + +func (err windowsACLContainmentError) Error() string { + return fmt.Sprintf( + "refusing to apply ACL to %s: it resolves to %s, outside the write root %s; "+ + "a reparse point on the way down redirects an elevated ACL write out of the sandbox", + err.Target, err.Actual, err.Anchor) +} + +// verifyWindowsACLHandleUnderAnchor refuses a handle whose object does not live +// where the plan said it would. +// +// anchor empty means the target was named rather than derived, and there is no +// owned tail to hold strict; the final-component check the caller already did is +// the whole guard for those. +func verifyWindowsACLHandleUnderAnchor(handle windows.Handle, anchor, target string) error { + if strings.TrimSpace(anchor) == "" { + return nil + } + tail, ok := windowsACLTailUnderAnchor(anchor, target) + if !ok { + return windowsACLContainmentError{Anchor: anchor, Target: target, Actual: target} + } + anchorFinal, err := windowsFinalPathOf(anchor) + if err != nil { + return fmt.Errorf("resolve write root %s: %w", anchor, err) + } + targetFinal, err := windowsFinalPathByHandle(handle) + if err != nil { + return fmt.Errorf("resolve windows ACL target %s: %w", target, err) + } + expected := anchorFinal + if tail != "" { + expected = filepath.Join(anchorFinal, tail) + } + if !strings.EqualFold(filepath.Clean(targetFinal), filepath.Clean(expected)) { + return windowsACLContainmentError{Anchor: anchor, Target: target, Actual: targetFinal} + } + return nil +} + +// verifyWindowsACLPathUnderAnchor makes the same check against the deepest +// component of target that exists, before anything is created. +// +// Materialization creates a missing target with os.MkdirAll, which walks a +// pathname and follows every reparse point on it, so without this an elevated +// setup creates the directory on the far side of a junction and only the +// containment check afterwards notices. The window between this check and that +// create is not closed here; closing it needs the components created relative to +// retained handles, which is the rooted descent tracked in #808. +func verifyWindowsACLPathUnderAnchor(anchor, target string) error { + if strings.TrimSpace(anchor) == "" { + return nil + } + if _, ok := windowsACLTailUnderAnchor(anchor, target); !ok { + return windowsACLContainmentError{Anchor: anchor, Target: target, Actual: target} + } + existing := filepath.Clean(target) + for { + if _, err := os.Lstat(existing); err == nil { + break + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect windows ACL target %s: %w", existing, err) + } + parent := filepath.Dir(existing) + if parent == existing { + return nil + } + existing = parent + } + tail, ok := windowsACLTailUnderAnchor(anchor, existing) + if !ok || tail == "" { + // Above or at the write root, which is the operator's to arrange. + return nil + } + handle, err := windowsACLOpenForContainment(existing) + if err != nil { + return err + } + defer func() { _ = windows.CloseHandle(handle) }() + // THE ANCESTOR ITSELF IS THE REDIRECTION, WHICH IS WHY ITS NAME LOOKS RIGHT. + // + // Asking only where this object lives answers "exactly where you asked": the + // open above does not follow a final-component reparse point, so a junction + // at /.git comes back as /.git. os.MkdirAll does follow it, and + // creates the missing components on the far side. So what disqualifies this + // ancestor is that it IS a reparse point, not where it reports living. + var info windows.ByHandleFileInformation + if err := windows.GetFileInformationByHandle(handle, &info); err != nil { + return fmt.Errorf("inspect windows ACL target %s: %w", existing, err) + } + if info.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return windowsACLContainmentError{Anchor: anchor, Target: target, Actual: existing + " (a reparse point)"} + } + return verifyWindowsACLHandleUnderAnchor(handle, anchor, existing) +} + +func windowsACLOpenForContainment(path string) (windows.Handle, error) { + utf16Path, err := windows.UTF16PtrFromString(path) + if err != nil { + return 0, fmt.Errorf("encode windows ACL target %s: %w", path, err) + } + handle, err := windows.CreateFile( + utf16Path, + windows.READ_CONTROL, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_BACKUP_SEMANTICS|windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + if err != nil { + return 0, fmt.Errorf("open windows ACL target %s: %w", path, err) + } + return handle, nil +} diff --git a/internal/sandbox/windows_acl_containment_windows_test.go b/internal/sandbox/windows_acl_containment_windows_test.go new file mode 100644 index 000000000..b5d3a15aa --- /dev/null +++ b/internal/sandbox/windows_acl_containment_windows_test.go @@ -0,0 +1,231 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// windowsACLTestDenySID is a well-known group this process is not a member of. +// +// A deny ACE is what the carveouts actually are, and denying a group the test +// runs as revokes its own WRITE_DAC: the apply succeeds and then nothing can +// put the DACL back, so the temporary tree cannot be removed. Guests keeps the +// deny semantics without locking the test out of its own fixture. +const windowsACLTestDenySID = "S-1-5-32-546" + +// makeACLJunction plants a directory junction at link pointing at target. +// +// A junction rather than a symlink on purpose: mklink /J needs no privilege, so +// it is reachable by exactly the unprivileged workspace writer this guards +// against, and the test runs on an ordinary developer or CI account. +func makeACLJunction(t *testing.T, link, target string) { + t.Helper() + output, err := exec.Command("cmd.exe", "/c", "mklink", "/J", link, target).CombinedOutput() + if err != nil { + t.Skipf("mklink /J unavailable here: %v (%s)", err, output) + } +} + +// AN ELEVATED ACL WRITE MUST NOT LEAVE THE WRITE ROOT THROUGH A JUNCTION. +// +// The apply opens its target with FILE_FLAG_OPEN_REPARSE_POINT, which refuses a +// reparse point at the FINAL component and resolves every component above it. +// The write-root carveouts are derived rather than named: /.git/hooks and +// /.git/config are constructed from the root, and /.git is a name an +// unprivileged workspace writer can create before setup runs. A junction there +// makes the apply open /hooks, an ordinary directory with +// nothing wrong about its final component, and `zero sandbox setup` writes a +// deny-write ACE on it as Administrator, outside the workspace. +func TestApplyWindowsACLRefusesACarveoutRedirectedByAJunction(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere") + if err := os.MkdirAll(filepath.Join(outside, "hooks"), 0o700); err != nil { + t.Fatal(err) + } + makeACLJunction(t, filepath.Join(root, ".git"), outside) + + carveout := filepath.Join(root, ".git", "hooks") + group := windowsACLPathGroup{ + Path: carveout, + Anchor: root, + Entries: []WindowsACLEntry{{ + Action: WindowsACLDenyWrite, + Path: carveout, + Capability: windowsACLTestDenySID, + Anchor: root, + }}, + } + + _, applied, err := applyWindowsACLPathGroup(group) + if applied { + t.Error("applied a deny-write ACE through a junction, so an elevated setup rewrote the DACL of an object outside the workspace") + } + var containment windowsACLContainmentError + if !errors.As(err, &containment) { + t.Fatalf("err = %v, want a containment refusal", err) + } + if !strings.Contains(containment.Actual, "elsewhere") { + t.Errorf("the refusal does not name where the target actually resolved: %v", containment) + } +} + +// The same carveout inside an ordinary workspace still gets its ACE, or the +// refusal above would prove only that the apply fails. +func TestApplyWindowsACLStillAppliesAnOrdinaryCarveout(t *testing.T) { + root := t.TempDir() + carveout := filepath.Join(root, ".git", "hooks") + if err := os.MkdirAll(carveout, 0o700); err != nil { + t.Fatal(err) + } + group := windowsACLPathGroup{ + Path: carveout, + Anchor: root, + Entries: []WindowsACLEntry{{ + Action: WindowsACLDenyWrite, + Path: carveout, + Capability: windowsACLTestDenySID, + Anchor: root, + }}, + } + + snapshot, applied, err := applyWindowsACLPathGroup(group) + if err != nil { + t.Fatalf("apply: %v", err) + } + // PUT IT BACK, OR THE TEMP TREE CANNOT BE REMOVED. The ACE just written + // denies write to the group this test runs as, and t.TempDir removes the + // tree when the test ends. + if applied { + t.Cleanup(func() { + if err := rollbackWindowsACLSnapshots([]windowsACLSnapshot{snapshot}); err != nil { + t.Errorf("rollback: %v", err) + } + }) + } + if !applied { + t.Fatal("an ordinary carveout inside the write root was not applied") + } + if snapshot.Path != carveout { + t.Fatalf("snapshot path = %q, want %q", snapshot.Path, carveout) + } +} + +// MATERIALIZATION MUST NOT CREATE THROUGH THE JUNCTION EITHER. os.MkdirAll +// walks a pathname and follows every reparse point on it, so without the +// pre-create check an elevated setup makes the directory on the far side and +// only notices afterwards. +func TestApplyWindowsACLRefusesToMaterializeThroughAJunction(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere") + if err := os.MkdirAll(outside, 0o700); err != nil { + t.Fatal(err) + } + makeACLJunction(t, filepath.Join(root, ".git"), outside) + + carveout := filepath.Join(root, ".git", "hooks") + if _, err := os.Lstat(filepath.Join(outside, "hooks")); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("SETUP INVALID: the target already holds hooks: %v", err) + } + group := windowsACLPathGroup{ + Path: carveout, + Anchor: root, + Materialize: true, + Entries: []WindowsACLEntry{{ + Action: WindowsACLDenyRead, + Path: carveout, + Capability: windowsACLTestDenySID, + Anchor: root, + Materialize: true, + }}, + } + + if _, applied, err := applyWindowsACLPathGroup(group); applied || err == nil { + t.Errorf("materialized through a junction: applied=%v err=%v", applied, err) + } + if _, err := os.Lstat(filepath.Join(outside, "hooks")); !errors.Is(err, os.ErrNotExist) { + t.Errorf("an elevated create landed outside the write root: %v", err) + } +} + +// An operator-named path carries no anchor and keeps the behaviour it had: the +// final component is still refused when it is a reparse point, and nothing above +// it is second-guessed, because above the write root the path belongs to the +// operator, who may keep a workspace under a junction. +func TestApplyWindowsACLLeavesAnUnanchoredPathAlone(t *testing.T) { + base := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere") + if err := os.MkdirAll(filepath.Join(outside, "leaf"), 0o700); err != nil { + t.Fatal(err) + } + makeACLJunction(t, filepath.Join(base, "link"), outside) + + named := filepath.Join(base, "link", "leaf") + group := windowsACLPathGroup{ + Path: named, + Entries: []WindowsACLEntry{{ + Action: WindowsACLDenyWrite, + Path: named, + Capability: windowsACLTestDenySID, + }}, + } + + snapshot, applied, err := applyWindowsACLPathGroup(group) + if err != nil { + t.Fatalf("an operator-named path below a junction was refused: %v", err) + } + if applied { + t.Cleanup(func() { + if err := rollbackWindowsACLSnapshots([]windowsACLSnapshot{snapshot}); err != nil { + t.Errorf("rollback: %v", err) + } + }) + } + if !applied { + t.Fatal("an operator-named path below a junction was not applied") + } +} + +// AND THE GUARD ON THE CREATE HAS TO BE PINNED WHERE IT CAN BE SEEN. +// +// Through the whole apply, dropping the pre-create check is invisible: the +// directory is made on the far side of the junction, the containment check on +// the handle then refuses, and the failure path removes what it made, so the +// filesystem afterwards looks the same either way. The transient elevated create +// outside the write root is the thing being prevented, so it is checked here, +// against the function that prevents it. +func TestVerifyWindowsACLPathUnderAnchorRefusesAMissingTargetBehindAJunction(t *testing.T) { + root := t.TempDir() + outside := filepath.Join(t.TempDir(), "elsewhere") + if err := os.MkdirAll(outside, 0o700); err != nil { + t.Fatal(err) + } + makeACLJunction(t, filepath.Join(root, ".git"), outside) + + carveout := filepath.Join(root, ".git", "hooks") + if _, err := os.Lstat(carveout); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("SETUP INVALID: the carveout already exists, so nothing would be created: %v", err) + } + + err := verifyWindowsACLPathUnderAnchor(root, carveout) + var containment windowsACLContainmentError + if !errors.As(err, &containment) { + t.Fatalf("err = %v, want a containment refusal before anything is created", err) + } + + // The ordinary case still says yes, or the refusal proves only that the + // function rejects things. + ordinary := t.TempDir() + if err := verifyWindowsACLPathUnderAnchor(ordinary, filepath.Join(ordinary, ".git", "hooks")); err != nil { + t.Errorf("an ordinary missing carveout was refused: %v", err) + } + // And an unanchored path is not this function's business. + if err := verifyWindowsACLPathUnderAnchor("", carveout); err != nil { + t.Errorf("an unanchored path was refused: %v", err) + } +} diff --git a/internal/sandbox/windows_acl_test.go b/internal/sandbox/windows_acl_test.go index 257198cd6..bcabedbdb 100644 --- a/internal/sandbox/windows_acl_test.go +++ b/internal/sandbox/windows_acl_test.go @@ -240,3 +240,117 @@ func TestDedupeWindowsACLEntriesKeepsInheritanceVariants(t *testing.T) { t.Fatalf("dedupe order/shape = %#v, want first NoInherit then inheritable", out) } } + +// THE PLAN HAS TO CARRY THE ANCHOR, OR THE APPLY HAS NOTHING TO ENFORCE. +// +// The apply-side tests hand applyWindowsACLPathGroup an anchor directly, so they +// pass whether or not anything ever sets one. This drives the real builder and +// checks which entries come out anchored: the carveouts derived from a write +// root, and not the paths the operator named, which have no owned tail and whose +// intermediates are the operator's own business. +// +// NATIVE PATHS, NOT WINDOWS LITERALS. The builder is cross-platform and so is +// this test, and the anchoring decision runs through pathWithinRoot, which is +// filepath.Rel underneath. On Linux a backslash is an ordinary character, so +// `C:\workspace\.git\hooks` is one component and is not under `C:\workspace`; +// the neighbouring tests get away with such literals because they only compare +// strings they built the same way and never ask whether one contains another. +func TestBuildWindowsACLPlanAnchorsDerivedCarveouts(t *testing.T) { + home := t.TempDir() + workspace := t.TempDir() + carveout := filepath.Join(workspace, ".git", "hooks") + metadata := filepath.Join(workspace, ".zero") + named := filepath.Join(t.TempDir(), "named-by-the-operator") + config := WindowsSandboxCommandConfig{ + SandboxHome: home, + WorkspaceRoots: []string{workspace}, + PermissionProfile: PermissionProfile{ + FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + WriteRoots: []WritableRoot{{ + Root: workspace, + ReadOnlySubpaths: []string{carveout}, + ProtectedMetadataNames: []string{".zero"}, + }}, + DenyWrite: []string{named}, + }, + Network: NetworkPolicy{Mode: NetworkDeny}, + }, + } + + plan, err := BuildWindowsACLPlan(config) + if err != nil { + t.Fatalf("BuildWindowsACLPlan: %v", err) + } + + anchors := map[string]string{} + for _, entry := range plan.Entries { + anchors[strings.ToLower(entry.Path)] = entry.Anchor + } + for _, derived := range []string{carveout, metadata} { + anchor, present := anchors[strings.ToLower(derived)] + if !present { + t.Fatalf("SETUP INVALID: the plan has no entry for the derived carveout %s", derived) + } + if anchor != workspace { + t.Errorf("derived carveout %s carries anchor %q, want the write root it came from", derived, anchor) + } + } + operatorAnchor, present := anchors[strings.ToLower(named)] + if !present { + t.Fatal("SETUP INVALID: the plan has no entry for the operator-named deny path") + } + if operatorAnchor != "" { + t.Errorf("operator-named path carries anchor %q, want none: its intermediates are not the sandbox's to police", operatorAnchor) + } +} + +// AND AN OUT-OF-ROOT READ-ONLY SUBPATH IS NOT ANCHORED AT ALL. +// +// ReadOnlySubpaths is a profile field an operator can set to any path. One +// placed outside the write root is a configuration that works today, and +// anchoring it to that root would turn it into a containment refusal: the +// object is not under the root and never was. Only the paths that are actually +// under it are held to it. +func TestBuildWindowsACLPlanLeavesAnOutOfRootSubpathUnanchored(t *testing.T) { + home := t.TempDir() + workspace := t.TempDir() + inside := filepath.Join(workspace, "vendor") + outside := filepath.Join(t.TempDir(), "shared") + plan, err := BuildWindowsACLPlan(WindowsSandboxCommandConfig{ + SandboxHome: home, + WorkspaceRoots: []string{workspace}, + PermissionProfile: PermissionProfile{ + FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + WriteRoots: []WritableRoot{{ + Root: workspace, + ReadOnlySubpaths: []string{inside, outside}, + }}, + }, + Network: NetworkPolicy{Mode: NetworkDeny}, + }, + }) + if err != nil { + t.Fatalf("BuildWindowsACLPlan: %v", err) + } + + seen := map[string]string{} + for _, entry := range plan.Entries { + seen[strings.ToLower(entry.Path)] = entry.Anchor + } + insideAnchor, present := seen[strings.ToLower(inside)] + if !present { + t.Fatal("SETUP INVALID: the plan has no entry for the in-root subpath") + } + if insideAnchor != workspace { + t.Errorf("in-root subpath carries anchor %q, want the write root", insideAnchor) + } + outsideAnchor, present := seen[strings.ToLower(outside)] + if !present { + t.Fatal("SETUP INVALID: the plan has no entry for the out-of-root subpath") + } + if outsideAnchor != "" { + t.Errorf("out-of-root subpath carries anchor %q, want none: the apply would refuse a config that works today", outsideAnchor) + } +}