From f2001b0c5a0b35e052ba62ef4f2e19bcc64fd870 Mon Sep 17 00:00:00 2001 From: tanaka takashi Date: Mon, 10 Aug 2026 02:26:09 +0900 Subject: perf(sandbox): apply ACLs at approved roots Replace per-descendant ACL enumeration, security descriptor snapshots, journal entries, audit events, and cleanup rewrites with one inheritable ACE and one snapshot per approved directory root. Exact file approvals continue to receive non-inheritable file ACEs, and nested roots retain independent least-privilege scopes cleaned in reverse order. Keep protected descendant DACLs protected instead of disabling inheritance protection. Such paths remain OS-denied, preserving fail-safe behavior without silently expanding the SandboxPolicy. Remove the obsolete descendant normalization pass. Restoring the root security descriptor removes inherited Application SID ACEs from existing and Task-created descendants; crash recovery performs the same root descriptor restoration from the journal. Add tests for existing-child inheritance, newly-created-child inheritance, protected descendants, one journal entry per root, exact descriptor restoration, no residual Application SID ACE, and independent-process crash recovery. Extend the existing elevated recovery and backend tests to validate inherited child cleanup and one ACL apply/restore event per root. Verification: Release build passed with 0 warnings and 0 errors; Core 50/50 passed; Planner 10/10 passed; focused ACL/recovery 3/3 passed; elevated Windows Security 28/28 passed; all reported skip 0. Operational evidence: the updated Release Bundle completed MCP Task 9b4a59a9-1f9b-4362-85f4-7b858899b488. ACL apply/restore decreased from 3,583/3,583 events taking 72/57 seconds to 3/3 root_inheritance events completing within the same second. WFP residual filters were 0 and the Task journal was removed. Release: refreshed the win-x64 self-contained folder bundle, clean ZIP, manifest, and checksum while preserving the local agent.yaml. Agent.Windows.dll SHA-256 is 24FDDD5727D5455170B6742B1D21E023A0BD70F92C225A2681BB5832C7AC0824; ZIP SHA-256 is EF7684C1A5A6155B774F4DF23CB08E9D1CF676351BF2B7D0F57915304495F456. No dependencies were added. --- .../Agent.Windows.Tests/CrashScenarioBootstrap.cs | 6 + tests/Agent.Windows.Tests/SecurityPolicyTests.cs | 147 ++++++++++++++++++++- 2 files changed, 152 insertions(+), 1 deletion(-) (limited to 'tests/Agent.Windows.Tests') diff --git a/tests/Agent.Windows.Tests/CrashScenarioBootstrap.cs b/tests/Agent.Windows.Tests/CrashScenarioBootstrap.cs index 7dd3629..dd170ab 100644 --- a/tests/Agent.Windows.Tests/CrashScenarioBootstrap.cs +++ b/tests/Agent.Windows.Tests/CrashScenarioBootstrap.cs @@ -26,6 +26,12 @@ internal static class CrashScenarioBootstrap var acl = new TaskAclManager(AppContainerProfile.EnsureFixed().Sid, new MemorySecurityAuditSink(), journal); _ = acl.Grant(taskId, Path.Combine(root, "resource"), FileCapability.Read | FileCapability.Write); break; + case "acl-inheritance": + var inheritedAcl = new TaskAclManager( + new System.Security.Principal.SecurityIdentifier("S-1-15-2-330000001-330000002-330000003-330000004-330000005-330000006-330000007"), + new MemorySecurityAuditSink(), journal); + _ = inheritedAcl.Grant(taskId, Path.Combine(root, "resource"), FileCapability.Read | FileCapability.Write); + break; case "acl-before": byte[] descriptor = FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(Path.Combine(root, "resource")), AccessControlSections.Access) .GetSecurityDescriptorBinaryForm(); diff --git a/tests/Agent.Windows.Tests/SecurityPolicyTests.cs b/tests/Agent.Windows.Tests/SecurityPolicyTests.cs index d54ab13..8d920b2 100644 --- a/tests/Agent.Windows.Tests/SecurityPolicyTests.cs +++ b/tests/Agent.Windows.Tests/SecurityPolicyTests.cs @@ -191,6 +191,75 @@ public sealed class SecurityPolicyTests Assert.Contains(audit.Events, item => item.Event == "acl.restore"); } + [Fact] + public void Root_acl_inheritance_covers_existing_and_new_descendants_and_restores_exactly() + { + using TestDirectory directory = new(); + using TestDirectory state = new(); + string existingDirectory = Path.Combine(directory.Path, "existing"); + string existingFile = Path.Combine(existingDirectory, "before.txt"); + Directory.CreateDirectory(existingDirectory); + File.WriteAllText(existingFile, "before"); + byte[] rootBefore = AccessDescriptor(directory.Path); + byte[] existingBefore = AccessDescriptor(existingFile); + var sid = new System.Security.Principal.SecurityIdentifier( + "S-1-15-2-310000001-310000002-310000003-310000004-310000005-310000006-310000007"); + var audit = new MemorySecurityAuditSink(); + Guid taskId = Guid.NewGuid(); + TaskResourceJournal journal = TaskResourceJournal.Create(Path.Combine(state.Path, "journals"), taskId); + TaskAclManager manager = new(sid, audit, journal); + + AclSnapshot snapshot = manager.Grant(taskId, directory.Path, FileCapability.Read | FileCapability.Write); + JournalEntry aclEntry = Assert.Single(journal.Entries); + Assert.Equal("acl", aclEntry.Kind); + Assert.Equal(directory.Path, aclEntry.Target, ignoreCase: true); + Assert.Contains(AccessRules(directory.Path, sid), static rule => !rule.IsInherited); + Assert.Contains(AccessRules(existingFile, sid), static rule => rule.IsInherited); + + string createdDirectory = Path.Combine(directory.Path, "created"); + string createdFile = Path.Combine(createdDirectory, "during.txt"); + Directory.CreateDirectory(createdDirectory); + File.WriteAllText(createdFile, "during"); + Assert.Contains(AccessRules(createdFile, sid), static rule => rule.IsInherited); + + manager.Restore(taskId, snapshot); + journal.CompleteAndDelete(); + + Assert.Equal(Convert.ToHexString(rootBefore), Convert.ToHexString(AccessDescriptor(directory.Path))); + Assert.Equal(Convert.ToHexString(existingBefore), Convert.ToHexString(AccessDescriptor(existingFile))); + Assert.Empty(AccessRules(existingFile, sid)); + Assert.Empty(AccessRules(createdFile, sid)); + SecurityAuditEvent applied = Assert.Single(audit.Events, item => item.Event == "acl.apply"); + Assert.Equal("root_inheritance", applied.Data["scope"]); + Assert.Single(audit.Events, item => item.Event == "acl.restore"); + Assert.Empty(Directory.EnumerateFiles(Path.Combine(state.Path, "journals"))); + } + + [Fact] + public void Root_acl_inheritance_does_not_override_a_protected_descendant() + { + using TestDirectory directory = new(); + string protectedDirectory = Path.Combine(directory.Path, "protected"); + Directory.CreateDirectory(protectedDirectory); + DirectorySecurity protectedAcl = FileSystemAclExtensions.GetAccessControl( + new DirectoryInfo(protectedDirectory), AccessControlSections.Access); + protectedAcl.SetAccessRuleProtection(isProtected: true, preserveInheritance: true); + FileSystemAclExtensions.SetAccessControl(new DirectoryInfo(protectedDirectory), protectedAcl); + var sid = new System.Security.Principal.SecurityIdentifier( + "S-1-15-2-320000001-320000002-320000003-320000004-320000005-320000006-320000007"); + TaskAclManager manager = new(sid, new MemorySecurityAuditSink()); + + AclSnapshot snapshot = manager.Grant(Guid.NewGuid(), directory.Path, FileCapability.Read); + try + { + Assert.Empty(AccessRules(protectedDirectory, sid)); + } + finally + { + manager.Restore(Guid.NewGuid(), snapshot); + } + } + [Fact] public void Brokered_process_runs_in_fixed_app_container_with_task_acl() { @@ -273,14 +342,18 @@ public sealed class SecurityPolicyTests string journals = Path.Combine(directory.Path, "journals"); string resource = Path.Combine(directory.Path, "resource"); Directory.CreateDirectory(resource); + string child = Path.Combine(resource, "existing.txt"); + File.WriteAllText(child, "existing"); byte[] aclBefore = FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(resource), AccessControlSections.Access) .GetSecurityDescriptorBinaryForm(); + byte[] childAclBefore = AccessDescriptor(child); var audit = new MemorySecurityAuditSink(); Guid taskId = Guid.NewGuid(); TaskResourceJournal journal = TaskResourceJournal.Create(journals, taskId); var acl = new TaskAclManager(profile.Sid, audit, journal); _ = acl.Grant(taskId, resource, FileCapability.Read | FileCapability.Write); + Assert.Contains(AccessRules(child, profile.Sid), static rule => rule.IsInherited); Assert.False(LoopbackExemption.Contains(profile.Sid)); Guid loopbackEntry = journal.Prepare("network.loopback_exemption", profile.Sid.Value, @@ -307,6 +380,8 @@ public sealed class SecurityPolicyTests byte[] aclAfter = FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(resource), AccessControlSections.Access) .GetSecurityDescriptorBinaryForm(); Assert.Equal(Convert.ToHexString(aclBefore), Convert.ToHexString(aclAfter)); + Assert.Equal(Convert.ToHexString(childAclBefore), Convert.ToHexString(AccessDescriptor(child))); + Assert.Empty(AccessRules(child, profile.Sid)); Assert.Empty(Directory.EnumerateFiles(journals)); } @@ -324,9 +399,12 @@ public sealed class SecurityPolicyTests string resourceDirectory = Path.Combine(directory.Path, "resource"); string resourceFile = Path.Combine(directory.Path, "resource.txt"); Directory.CreateDirectory(resourceDirectory); + string childFile = Path.Combine(resourceDirectory, "child.txt"); + File.WriteAllText(childFile, "child-before-crash"); File.WriteAllText(resourceFile, "before-crash"); byte[] aclBefore = FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(resourceDirectory), AccessControlSections.Access) .GetSecurityDescriptorBinaryForm(); + byte[] childAclBefore = AccessDescriptor(childFile); Guid taskId = Guid.NewGuid(); using var child = new Process { @@ -358,6 +436,8 @@ public sealed class SecurityPolicyTests byte[] aclAfter = FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(resourceDirectory), AccessControlSections.Access) .GetSecurityDescriptorBinaryForm(); Assert.Equal(Convert.ToHexString(aclBefore), Convert.ToHexString(aclAfter)); + Assert.Equal(Convert.ToHexString(childAclBefore), Convert.ToHexString(AccessDescriptor(childFile))); + Assert.Empty(AccessRules(childFile, AppContainerProfile.EnsureFixed().Sid)); Assert.False(LoopbackExemption.Contains(AppContainerProfile.EnsureFixed().Sid)); string backups = Path.Combine(directory.Path, "backups"); Assert.True(!Directory.Exists(backups) || !Directory.EnumerateFileSystemEntries(backups).Any()); @@ -371,6 +451,51 @@ public sealed class SecurityPolicyTests } } + [Fact] + public void Independent_process_crash_recovers_root_inheritance_from_existing_child() + { + using TestDirectory directory = new(); + string resource = Path.Combine(directory.Path, "resource"); + string childFile = Path.Combine(resource, "child.txt"); + Directory.CreateDirectory(resource); + File.WriteAllText(childFile, "before-crash"); + byte[] rootBefore = AccessDescriptor(resource); + byte[] childBefore = AccessDescriptor(childFile); + var sid = new System.Security.Principal.SecurityIdentifier( + "S-1-15-2-330000001-330000002-330000003-330000004-330000005-330000006-330000007"); + Guid taskId = Guid.NewGuid(); + using var child = new Process + { + StartInfo = new ProcessStartInfo + { + FileName = Environment.ProcessPath!, + UseShellExecute = false, + CreateNoWindow = true, + Environment = + { + ["AGENT_MCP_CRASH_SCENARIO"] = "acl-inheritance", + ["AGENT_MCP_CRASH_ROOT"] = directory.Path, + ["AGENT_MCP_CRASH_TASK"] = taskId.ToString("D") + } + } + }; + + Assert.True(child.Start()); + Assert.True(child.WaitForExit(30_000)); + Assert.NotEqual(0, child.ExitCode); + Assert.False(File.Exists(Path.Combine(directory.Path, "crash-error.txt")), + File.Exists(Path.Combine(directory.Path, "crash-error.txt")) ? File.ReadAllText(Path.Combine(directory.Path, "crash-error.txt")) : string.Empty); + Assert.Contains(AccessRules(childFile, sid), static rule => rule.IsInherited); + + RecoveryResult recovered = new RecoveryManager(Path.Combine(directory.Path, "journals"), new MemorySecurityAuditSink()).RecoverAll(); + + Assert.True(recovered.IsClean, string.Join(Environment.NewLine, recovered.Errors)); + Assert.Equal(Convert.ToHexString(rootBefore), Convert.ToHexString(AccessDescriptor(resource))); + Assert.Equal(Convert.ToHexString(childBefore), Convert.ToHexString(AccessDescriptor(childFile))); + Assert.Empty(AccessRules(childFile, sid)); + Assert.Empty(Directory.EnumerateFiles(Path.Combine(directory.Path, "journals"))); + } + private static bool ProcessExists(int pid) { try { using Process process = Process.GetProcessById(pid); return !process.HasExited; } @@ -440,7 +565,8 @@ public sealed class SecurityPolicyTests string backups = Path.Combine(directory.Path, "backups"); Assert.True(!Directory.Exists(backups) || !Directory.EnumerateFileSystemEntries(backups).Any()); Assert.Contains(audit.Events, item => item.Event == "wfp.remove" && item.Data["residual_filters"] == "0"); - Assert.Contains(audit.Events, item => item.Event == "acl.restore"); + Assert.Single(audit.Events, item => item.Event == "acl.apply"); + Assert.Single(audit.Events, item => item.Event == "acl.restore"); } [Fact(Timeout = 60_000)] @@ -514,6 +640,25 @@ public sealed class SecurityPolicyTests private static RestrictedProcessResult Curl(RestrictedProcessLauncher launcher, Guid taskId, string curl, string url, string cwd) => launcher.Run(taskId, curl, ["--noproxy", "*", "--silent", "--show-error", "--max-time", "2", url], cwd, TimeSpan.FromSeconds(10)); + private static byte[] AccessDescriptor(string path) + { + FileSystemSecurity security = Directory.Exists(path) + ? FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(path), AccessControlSections.Access) + : FileSystemAclExtensions.GetAccessControl(new FileInfo(path), AccessControlSections.Access); + return security.GetSecurityDescriptorBinaryForm(); + } + + private static FileSystemAccessRule[] AccessRules(string path, System.Security.Principal.SecurityIdentifier sid) + { + FileSystemSecurity security = Directory.Exists(path) + ? FileSystemAclExtensions.GetAccessControl(new DirectoryInfo(path), AccessControlSections.Access) + : FileSystemAclExtensions.GetAccessControl(new FileInfo(path), AccessControlSections.Access); + return security.GetAccessRules(includeExplicit: true, includeInherited: true, typeof(System.Security.Principal.SecurityIdentifier)) + .OfType() + .Where(rule => rule.IdentityReference == sid && rule.AccessControlType == AccessControlType.Allow) + .ToArray(); + } + private sealed class TestDirectory : IDisposable { public TestDirectory() -- cgit v1.2.3