From cf40b8680bb63b7faf98f3f4879cabbd8e68a969 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Thu, 3 Sep 2026 18:17:48 +0530 Subject: [PATCH 1/2] fix(sandbox): take Everyone out of the restricted-SID list A profile that sets denyRead drops WRITE_RESTRICTED, and the token that replaces it carried the World SID. Every principal carries Everyone, so the restricted-SID check passed for free on any path whose DACL grants Everyone write, and the workspace write jail fell back to the caller's own permissions, which is the boundary the token exists to be stricter than. No privilege, no symlink and no race: an Everyone-writable directory was enough, and share roots opened Everyone:F and loose installer trees supply them. #865 closed this for the WRITE_RESTRICTED token and left this half open. It could not simply be dropped. Without WRITE_RESTRICTED the restricted-SID check covers reads as well, and default Windows DACLs grant BUILTIN\Users rather than anything in the list, so a token without Everyone cannot open cmd.exe and dies at launch with a bare access denial. Confirmed by impersonating both token shapes against C:\Windows\System32\cmd.exe. So the read capability takes its place. BuildWindowsACLPlan already grants that SID on every read root and denies it on every denyRead path under exactly the same condition, so the read allowance and the restriction become one decision, and it names only what setup granted rather than every principal on the machine. The principal path already worked this way. Regressions drive a real restricted token through a real impersonated write: an Everyone-only DACL is refused, a directory the capability grants is written, and the token's own restricted-SID list is read back from the token. A separate test pins the plan and the token to the same answer, because the two halves are decided in different files off the same field. Closes #869 --- .../sandbox/windows_command_runner_windows.go | 11 +- .../windows_restricted_sid_read_test.go | 132 +++++++++++ internal/sandbox/windows_runner.go | 25 +++ internal/sandbox/windows_token_windows.go | 59 ++--- .../windows_world_sid_bypass_windows_test.go | 210 ++++++++++++++++++ 5 files changed, 399 insertions(+), 38 deletions(-) create mode 100644 internal/sandbox/windows_restricted_sid_read_test.go create mode 100644 internal/sandbox/windows_world_sid_bypass_windows_test.go diff --git a/internal/sandbox/windows_command_runner_windows.go b/internal/sandbox/windows_command_runner_windows.go index 6d9235874..3b2e1236f 100644 --- a/internal/sandbox/windows_command_runner_windows.go +++ b/internal/sandbox/windows_command_runner_windows.go @@ -71,7 +71,7 @@ func runWindowsSandboxCommand(config WindowsSandboxCommandConfig, stderr io.Writ // granting only the user, Administrators, and SYSTEM (msys2-runtime // sigproc.cc sigproc_init -> sec_user_nih -> __sec_user), and a // WRITE_RESTRICTED write check must ALSO match one of the token's - // restricted SIDs (logon SID, Everyone, capability SIDs). None of the + // restricted SIDs (logon SID and capability SIDs). None of the // granted SIDs can be added to the restricted list without collapsing the // write jail (each has write access nearly everywhere), so MSYS startup // dies with "couldn't create signal pipe" or "CreateFileMapping .1", @@ -88,6 +88,15 @@ func runWindowsSandboxCommand(config WindowsSandboxCommandConfig, stderr io.Writ // restricted token, trading spawn capability for read-deny enforcement. writeRestricted := len(config.PermissionProfile.FileSystem.DenyRead) == 0 + // That strict token then needs the read capability in its SID list, because + // the restricted-SID check covers reads once WRITE_RESTRICTED is gone. See + // windowsRestrictedTokenSIDsForProfile. + tokenSIDs, err = windowsRestrictedTokenSIDsForProfile(tokenSIDs, config.SandboxHome, writeRestricted) + if err != nil { + fmt.Fprintln(stderr, WindowsSandboxCommandRunnerName+": "+err.Error()) + return 1 + } + // A provisioned sandbox principal replaces the restricted token entirely: it // is a separate account, so reads outside its granted roots are denied by the // filesystem rather than left open the way a same-user restricted token has diff --git a/internal/sandbox/windows_restricted_sid_read_test.go b/internal/sandbox/windows_restricted_sid_read_test.go new file mode 100644 index 000000000..849a0845b --- /dev/null +++ b/internal/sandbox/windows_restricted_sid_read_test.go @@ -0,0 +1,132 @@ +package sandbox + +import ( + "path/filepath" + "strings" + "testing" +) + +// THE STRICT TOKEN MUST CARRY THE GRANT ITS READ ROOTS NAME. +// +// A profile with DenyRead drops WRITE_RESTRICTED, which puts reads under the +// restricted-SID check as well. Everyone used to satisfy that check, and taking +// it away without putting the read capability in its place leaves a token that +// cannot open cmd.exe: the command dies at launch with an access denial that +// says nothing about the profile. +// +// The ACL plan grants this same capability on every read root, so the two halves +// have to be decided together. This pins the token half; the plan half is pinned +// by the plan tests, and the kernel behaviour by the impersonation test beside +// this one. +func TestTheStrictTokenCarriesTheReadCapability(t *testing.T) { + home := t.TempDir() + want, err := WindowsReadAllowSID(home) + if err != nil { + t.Fatalf("SETUP INVALID: no read capability for %s: %v", home, err) + } + + base := []string{"S-1-15-3-1024-workspace"} + got, err := windowsRestrictedTokenSIDsForProfile(base, home, false) + if err != nil { + t.Fatalf("windowsRestrictedTokenSIDsForProfile: %v", err) + } + if !containsSID(got, want) { + t.Fatalf("the strict token's restricted SIDs %v omit the read capability %s, so the command cannot open its own executable", got, want) + } + if !containsSID(got, base[0]) { + t.Fatalf("the workspace capability was dropped: %v", got) + } +} + +// And the WRITE_RESTRICTED token does not get it: reads are already unrestricted +// there, so adding a SID would widen the write jail for nothing. +func TestTheWriteRestrictedTokenDoesNotCarryTheReadCapability(t *testing.T) { + home := t.TempDir() + readSID, err := WindowsReadAllowSID(home) + if err != nil { + t.Fatalf("SETUP INVALID: no read capability for %s: %v", home, err) + } + got, err := windowsRestrictedTokenSIDsForProfile([]string{"S-1-15-3-1024-workspace"}, home, true) + if err != nil { + t.Fatalf("windowsRestrictedTokenSIDsForProfile: %v", err) + } + if containsSID(got, readSID) { + t.Fatalf("the WRITE_RESTRICTED token carries the read capability %s, widening the write jail to every read root", readSID) + } +} + +func containsSID(values []string, want string) bool { + for _, value := range values { + if strings.EqualFold(value, want) { + return true + } + } + return false +} + +// THE PLAN AND THE TOKEN DECIDE THE READ CAPABILITY SEPARATELY. +// +// The ACL plan asks whether the profile configures DenyRead; the runner asks +// whether the token keeps WRITE_RESTRICTED. Both are read off the same field +// today, in two files, with nothing tying them together. Drift either way is +// silent and bad: a token carrying a SID no DACL names cannot open its own +// executable, and a plan granting a SID no token carries leaves reads confined +// by nothing. +// +// So this pins the equivalence rather than either half. +func TestThePlanAndTheTokenAgreeOnTheReadCapability(t *testing.T) { + for _, testCase := range []struct { + name string + denyRead []string + }{ + {name: "with denyRead", denyRead: []string{filepath.Join("secrets")}}, + {name: "without denyRead"}, + } { + t.Run(testCase.name, func(t *testing.T) { + workspace := t.TempDir() + config := WindowsSandboxCommandConfig{ + SandboxHome: t.TempDir(), + CommandCWD: workspace, + WorkspaceRoots: []string{workspace}, + PermissionProfile: PermissionProfile{ + FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + WriteRoots: []WritableRoot{{Root: workspace}}, + ReadRoots: []string{workspace}, + DenyRead: prefixEach(workspace, testCase.denyRead), + }, + }, + } + + planned, err := windowsReadAllowCapabilitySID(config) + if err != nil { + t.Fatalf("windowsReadAllowCapabilitySID: %v", err) + } + writeRestricted := len(config.PermissionProfile.FileSystem.DenyRead) == 0 + tokenSIDs, err := windowsRestrictedTokenSIDsForProfile(nil, config.SandboxHome, writeRestricted) + if err != nil { + t.Fatalf("windowsRestrictedTokenSIDsForProfile: %v", err) + } + + inPlan := planned != "" + inToken := len(tokenSIDs) > 0 + if inPlan != inToken { + t.Fatalf("the plan grants the read capability = %t but the token carries it = %t; one side confines reads the other side never checks", inPlan, inToken) + } + if inPlan && !containsSID(tokenSIDs, planned) { + t.Fatalf("the plan grants %s but the token carries %v, so the command cannot read what setup allowed", planned, tokenSIDs) + } + }) + } +} + +func prefixEach(root string, relatives []string) []string { + if len(relatives) == 0 { + return nil + } + out := make([]string, 0, len(relatives)) + for _, relative := range relatives { + out = append(out, filepath.Join(root, relative)) + } + return out +} diff --git a/internal/sandbox/windows_runner.go b/internal/sandbox/windows_runner.go index 6b1ca42fe..7c6dbb1f9 100644 --- a/internal/sandbox/windows_runner.go +++ b/internal/sandbox/windows_runner.go @@ -727,6 +727,31 @@ func randomWindowsCapabilitySID() string { return fmt.Sprintf("S-1-5-21-%d-%d-%d-%d", words[0], words[1], words[2], words[3]) } +// windowsRestrictedTokenSIDsForProfile adds the read capability to a restricted +// token's SID list when the profile selected the strict token. +// +// WRITE_RESTRICTED scopes the restricted-SID check to writes. Without it the +// check applies to reads as well, so the token needs a restricted SID that the +// read roots' DACLs name or the command cannot open its own executable. It used +// to be Everyone, which is a key to every object whose DACL grants Everyone +// write and so handed back the write jail (#869). BuildWindowsACLPlan grants +// this capability on every read root and denies it on every DenyRead path, so +// the read allowance and the restriction are the same decision. +// +// Only elevated setup can grant it on the volume root the production profile +// seeds; the unelevated tier refuses a DenyRead profile up front for that +// reason, so nothing reaches here expecting a grant nobody applied. +func windowsRestrictedTokenSIDsForProfile(tokenSIDs []string, sandboxHome string, writeRestricted bool) ([]string, error) { + if writeRestricted { + return tokenSIDs, nil + } + readSID, err := WindowsReadAllowSID(sandboxHome) + if err != nil { + return nil, fmt.Errorf("resolve the sandbox read capability: %w", err) + } + return append(tokenSIDs, readSID), nil +} + // WindowsReadAllowSID returns the sandbox home's read-capability SID, minting and // persisting it on first use. Both halves of the setup protocol ask for it: the // capability ACL plan grants it on every read root, and the principal's strict diff --git a/internal/sandbox/windows_token_windows.go b/internal/sandbox/windows_token_windows.go index 77c8863bb..0a7c30a45 100644 --- a/internal/sandbox/windows_token_windows.go +++ b/internal/sandbox/windows_token_windows.go @@ -142,50 +142,35 @@ func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []w } entries = append(entries, windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)}) - // The World SID (S-1-1-0, Everyone) is added ONLY to the token that does not - // carry WRITE_RESTRICTED, and putting it back unconditionally would reopen a - // write-jail bypass. + // NO UNIVERSAL GROUP IS EVER A RESTRICTED SID HERE (#869). // - // A restricted SID is a key to every object whose DACL names it. That is why - // the runner refuses to add the user SID, Administrators or SYSTEM — "each - // has write access nearly everywhere". Everyone is the broadest of the lot: - // every principal carries it, so under WRITE_RESTRICTED the second - // (restricted-SID) check passes for free on any path whose DACL grants - // Everyone write, and confinement falls back to the ordinary user's own - // permissions — the exact boundary this token exists to be stricter than. It - // needs no privilege, no symlink and no race: an Everyone-writable directory - // is enough, and share roots opened with Everyone:F and loose installer ACLs - // supply them. It was present from the original sandbox baseline, - // uncommented, and under WRITE_RESTRICTED nothing depends on it, because that - // flag already exempts reads from the restricted-SID check. + // A restricted SID is a key to every object whose DACL names it, which is why + // the runner refuses to add the user SID, Administrators or SYSTEM: each has + // write access nearly everywhere. The World SID (S-1-1-0, Everyone) was the + // broadest of the lot and used to be added to the token that does NOT carry + // WRITE_RESTRICTED. Every principal carries Everyone, so the restricted-SID + // check passed for free on any path whose DACL grants Everyone write, and the + // workspace write jail fell back to the ordinary user's own permissions. No + // privilege, no symlink and no race was needed: an Everyone-writable directory + // was enough, and share roots opened with Everyone:F and loose installer ACLs + // supply them. // - // Without the flag it IS load-bearing and cannot simply be dropped. The - // restricted-SID check then applies to READS too, and default Windows DACLs - // grant BUILTIN\Users rather than anything in this list, so a token without - // Everyone cannot open cmd.exe — the process dies at launch with - // STATUS_ACCESS_DENIED (0xC0000022) before it runs anything. That path is - // only taken when the profile configures DenyRead, which is already the - // posture that trades capability for read-deny enforcement (#612). - // - // So the bypass survives for DenyRead profiles, deliberately and narrowly, - // rather than being traded for a sandbox that cannot start a command. Closing - // it there needs a different mechanism (a read-side grant that is not a - // universal group), tracked separately. + // It could not simply be dropped while it was load-bearing. Without + // WRITE_RESTRICTED the restricted-SID check applies to reads as well, and + // default Windows DACLs grant BUILTIN\Users rather than anything in this + // list, so a token without Everyone could not open cmd.exe and died at launch + // with STATUS_ACCESS_DENIED. The caller now supplies the read capability for + // exactly that case, which names only the roots setup granted, so the read + // side is satisfied without handing out a key to every Everyone-writable + // object on the machine. // // The logon SID above stays in both modes: it is this token's own rather than // a broad group, and broadenWindowsRestrictedTokenDefaultDacl depends on it so // the process can use pipes and events it creates for itself. // - // Anything added here needs the same scrutiny — Authenticated Users, Users, - // INTERACTIVE and BATCH would each produce this bypass on a DACL naming them. - if !writeRestricted { - worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid) - if err != nil { - return 0, fmt.Errorf("create world SID: %w", err) - } - entries = append(entries, windows.SIDAndAttributes{Sid: worldSID}) - } - + // Anything added here needs the same scrutiny. Authenticated Users, Users, + // INTERACTIVE and BATCH would each reintroduce this bypass on a DACL naming + // them. // WRITE_RESTRICTED scopes the restricted-SID check to write-type accesses: // reads use only the normal token identity, so the sandboxed process can // open executables, DLLs, and per-user config the user can read, while diff --git a/internal/sandbox/windows_world_sid_bypass_windows_test.go b/internal/sandbox/windows_world_sid_bypass_windows_test.go new file mode 100644 index 000000000..f2c2c2563 --- /dev/null +++ b/internal/sandbox/windows_world_sid_bypass_windows_test.go @@ -0,0 +1,210 @@ +//go:build windows + +package sandbox + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "unsafe" + + "golang.org/x/sys/windows" +) + +var procImpersonateLoggedOnUser = windows.NewLazySystemDLL("advapi32.dll").NewProc("ImpersonateLoggedOnUser") + +// tokenRestrictedSIDs reads the restricting SIDs the token actually carries. +// Read back from the token rather than from the list handed to the builder, +// because the list is not what the kernel consults. +func tokenRestrictedSIDs(t *testing.T, token windows.Token) []string { + t.Helper() + var size uint32 + err := windows.GetTokenInformation(token, windows.TokenRestrictedSids, nil, 0, &size) + if err != nil && err != windows.ERROR_INSUFFICIENT_BUFFER { + t.Fatalf("size the restricted-SID list: %v", err) + } + buffer := make([]byte, size) + if err := windows.GetTokenInformation(token, windows.TokenRestrictedSids, &buffer[0], size, &size); err != nil { + t.Fatalf("read the restricted-SID list: %v", err) + } + groups := (*windows.Tokengroups)(unsafe.Pointer(&buffer[0])) + out := make([]string, 0, groups.GroupCount) + for _, group := range groups.AllGroups() { + out = append(out, group.Sid.String()) + } + runtime.KeepAlive(buffer) + return out +} + +// protectDirectoryFor replaces the directory's DACL with exactly the grants +// named, detached from inheritance. Everyone-writable share roots and loose +// installer trees look like this, and the inherited ACEs have to go: otherwise +// the caller's own rights answer the first access check for reasons that have +// nothing to do with the restricted-SID list under test. +func protectDirectoryFor(t *testing.T, path string, sids []string) { + t.Helper() + entries := make([]windows.EXPLICIT_ACCESS, 0, len(sids)) + for _, value := range sids { + sid, err := windows.StringToSid(value) + if err != nil { + t.Fatalf("parse %q: %v", value, err) + } + entries = append(entries, windows.EXPLICIT_ACCESS{ + AccessPermissions: windows.GENERIC_ALL, + AccessMode: windows.GRANT_ACCESS, + Inheritance: windows.SUB_CONTAINERS_AND_OBJECTS_INHERIT, + Trustee: windows.TRUSTEE{ + TrusteeForm: windows.TRUSTEE_IS_SID, + TrusteeType: windows.TRUSTEE_IS_GROUP, + TrusteeValue: windows.TrusteeValueFromSID(sid), + }, + }) + } + dacl, err := windows.ACLFromEntries(entries, nil) + if err != nil { + t.Fatalf("build a DACL for %s: %v", path, err) + } + if err := windows.SetNamedSecurityInfo( + path, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, nil, dacl, nil, + ); err != nil { + t.Skipf("cannot rewrite the DACL of %s here: %v", path, err) + } +} + +// writeAsToken attempts a create inside dir while impersonating token, which is +// the access check this is all about. DISABLE_MAX_PRIVILEGE keeps +// SeChangeNotifyPrivilege, so traversal to dir is bypassed and the answer comes +// from that directory's own DACL rather than from its ancestors'. +func writeAsToken(t *testing.T, token windows.Token, dir string) error { + t.Helper() + var impersonation windows.Token + if err := windows.DuplicateTokenEx(token, windows.TOKEN_ALL_ACCESS, nil, windows.SecurityImpersonation, windows.TokenImpersonation, &impersonation); err != nil { + t.Fatalf("duplicate the token for impersonation: %v", err) + } + defer impersonation.Close() + + runtime.LockOSThread() + defer runtime.UnlockOSThread() + if result, _, callErr := procImpersonateLoggedOnUser.Call(uintptr(impersonation)); result == 0 { + t.Fatalf("impersonate the restricted token: %v", callErr) + } + defer func() { + if err := windows.RevertToSelf(); err != nil { + t.Fatalf("revert to self: %v", err) + } + }() + return os.WriteFile(filepath.Join(dir, "escaped.txt"), []byte("x"), 0o600) +} + +// EVERYONE AS A RESTRICTING SID IS A KEY TO EVERY EVERYONE-WRITABLE OBJECT. +// +// A profile with DenyRead selects the token without WRITE_RESTRICTED, and that +// token used to carry the World SID so it could still open its own executable. +// Every principal carries Everyone, so the restricted-SID check then passed for +// free on any path whose DACL grants Everyone write, and the workspace write jail +// fell back to the caller's own permissions. That is the boundary the token +// exists to be stricter than, and share roots opened Everyone:F supply the +// directories. +// +// Driven through a real restricted token and a real impersonated write, because +// the question is what the kernel decides, not what the SID list looks like. +// Rewriting the DACL of a directory this test created needs no Administrator +// rights, so it runs unelevated. +func TestTheStrictTokenCannotWriteAnEveryoneWritableDirectory(t *testing.T) { + workspace := t.TempDir() + config := WindowsSandboxCommandConfig{ + SandboxHome: t.TempDir(), + CommandCWD: workspace, + WorkspaceRoots: []string{workspace}, + PermissionProfile: PermissionProfile{ + FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + WriteRoots: []WritableRoot{{Root: workspace}}, + }, + }, + } + capability, err := windowsCapabilitySIDForWriteRoot(config, workspace) + if err != nil { + t.Fatalf("resolve the workspace capability SID: %v", err) + } + + // writeRestricted false: the token a DenyRead profile selects. + token, err := createWindowsRestrictedTokenForCapabilitySIDs([]string{capability}, false) + if err != nil { + t.Fatalf("build the strict restricted token: %v", err) + } + defer token.Close() + + // The direct fact, before any access check: no universal group is a key. + restricted := tokenRestrictedSIDs(t, token) + for _, sid := range restricted { + if strings.EqualFold(sid, "S-1-1-0") { + t.Fatalf("the restricted-SID list %v carries Everyone, so every Everyone-writable object on the machine is inside the write jail", restricted) + } + } + if len(restricted) == 0 { + t.Fatal("SETUP INVALID: the token carries no restricting SIDs at all, so the refusal below would hold for a token that is not restricted") + } + + // SETUP: the token writes where the capability IS granted, or the refusal + // below would be satisfied by a token that can write nowhere. + granted := filepath.Join(workspace, "granted") + if err := os.MkdirAll(granted, 0o700); err != nil { + t.Fatal(err) + } + protectDirectoryFor(t, granted, []string{"S-1-1-0", capability}) + if err := writeAsToken(t, token, granted); err != nil { + t.Fatalf("SETUP INVALID: the token cannot write a directory its own capability grants: %v", err) + } + + // And the bypass: Everyone full control, nothing for the capability. + everyone := filepath.Join(workspace, "everyone-writable") + if err := os.MkdirAll(everyone, 0o700); err != nil { + t.Fatal(err) + } + protectDirectoryFor(t, everyone, []string{"S-1-1-0"}) + if err := writeAsToken(t, token, everyone); err == nil { + t.Fatal("the sandboxed token wrote a directory outside every write root, because its DACL grants Everyone and Everyone is one of the token's restricting SIDs") + } +} + +// The same directory, and the WRITE_RESTRICTED token a profile without DenyRead +// selects: also refused. Both tokens hold the jail, and the flag only scopes +// which accesses the restricted-SID check covers. +func TestTheWriteRestrictedTokenCannotWriteAnEveryoneWritableDirectory(t *testing.T) { + workspace := t.TempDir() + config := WindowsSandboxCommandConfig{ + SandboxHome: t.TempDir(), + CommandCWD: workspace, + WorkspaceRoots: []string{workspace}, + PermissionProfile: PermissionProfile{ + FileSystem: FileSystemPolicy{ + Kind: FileSystemRestricted, + WriteRoots: []WritableRoot{{Root: workspace}}, + }, + }, + } + capability, err := windowsCapabilitySIDForWriteRoot(config, workspace) + if err != nil { + t.Fatalf("resolve the workspace capability SID: %v", err) + } + token, err := createWindowsRestrictedTokenForCapabilitySIDs([]string{capability}, true) + if err != nil { + t.Fatalf("build the WRITE_RESTRICTED token: %v", err) + } + defer token.Close() + + everyone := filepath.Join(workspace, "everyone-writable") + if err := os.MkdirAll(everyone, 0o700); err != nil { + t.Fatal(err) + } + protectDirectoryFor(t, everyone, []string{"S-1-1-0"}) + if err := writeAsToken(t, token, everyone); err == nil { + t.Fatal("the sandboxed token wrote a directory outside every write root through an Everyone grant") + } +} From 759133cdd3bcc256222687699e7a5374b8b56a4b Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Thu, 3 Sep 2026 18:19:59 +0530 Subject: [PATCH 2/2] test(sandbox): keep the SID helper from colliding with #886 --- internal/sandbox/windows_restricted_sid_read_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/sandbox/windows_restricted_sid_read_test.go b/internal/sandbox/windows_restricted_sid_read_test.go index 849a0845b..80a11cea3 100644 --- a/internal/sandbox/windows_restricted_sid_read_test.go +++ b/internal/sandbox/windows_restricted_sid_read_test.go @@ -30,10 +30,10 @@ func TestTheStrictTokenCarriesTheReadCapability(t *testing.T) { if err != nil { t.Fatalf("windowsRestrictedTokenSIDsForProfile: %v", err) } - if !containsSID(got, want) { + if !carriesSID(got, want) { t.Fatalf("the strict token's restricted SIDs %v omit the read capability %s, so the command cannot open its own executable", got, want) } - if !containsSID(got, base[0]) { + if !carriesSID(got, base[0]) { t.Fatalf("the workspace capability was dropped: %v", got) } } @@ -50,12 +50,14 @@ func TestTheWriteRestrictedTokenDoesNotCarryTheReadCapability(t *testing.T) { if err != nil { t.Fatalf("windowsRestrictedTokenSIDsForProfile: %v", err) } - if containsSID(got, readSID) { + if carriesSID(got, readSID) { t.Fatalf("the WRITE_RESTRICTED token carries the read capability %s, widening the write jail to every read root", readSID) } } -func containsSID(values []string, want string) bool { +// Not containsSID: #886 adds a package-level helper by that name, and the two +// branches would stop compiling the moment both land. +func carriesSID(values []string, want string) bool { for _, value := range values { if strings.EqualFold(value, want) { return true @@ -113,7 +115,7 @@ func TestThePlanAndTheTokenAgreeOnTheReadCapability(t *testing.T) { if inPlan != inToken { t.Fatalf("the plan grants the read capability = %t but the token carries it = %t; one side confines reads the other side never checks", inPlan, inToken) } - if inPlan && !containsSID(tokenSIDs, planned) { + if inPlan && !carriesSID(tokenSIDs, planned) { t.Fatalf("the plan grants %s but the token carries %v, so the command cannot read what setup allowed", planned, tokenSIDs) } })