Skip to content

Commit 3ce0758

Browse files
zerox80claude
andcommitted
test(windows-service): compare IPC ACL by canonical SID instead of SDDL alias
The hardening_script_removes_stale_aces test read the resulting DACL back via GetSecurityDescriptorSddlForm and checked for the current user's raw SID as a substring. When the test runs as the built-in Administrator (RID 500), SDDL abbreviates that account to the `LA` alias, so the raw-SID check failed even though the ACE was correct. Enumerate the access rules with explicit SecurityIdentifier identities so SYSTEM, Admins, and the current user compare by canonical SID regardless of SDDL aliasing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d4fabfe commit 3ce0758

1 file changed

Lines changed: 32 additions & 10 deletions

File tree

windows/src/bin/service/utils/tests.rs

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,27 +135,49 @@
135135

136136
// Read the DACL back via the .NET API for the same reason the script
137137
// does: the Get-Acl cmdlet's Microsoft.PowerShell.Security module is
138-
// unreliable on CI runners.
139-
let sddl_out = std::process::Command::new("powershell")
138+
// unreliable on CI runners. Enumerate the access rules with explicit
139+
// SecurityIdentifier identities rather than reading the SDDL string,
140+
// because SDDL abbreviates well-known accounts to aliases (e.g. the
141+
// built-in Administrator becomes `LA`), which a raw-SID substring
142+
// check would miss.
143+
let aces_out = std::process::Command::new("powershell")
140144
.args([
141145
"-NoProfile",
142146
"-NonInteractive",
143147
"-Command",
144148
&format!(
145-
"[System.IO.File]::GetAccessControl('{}').GetSecurityDescriptorSddlForm('Access')",
149+
"$acl = [System.IO.File]::GetAccessControl('{}'); \
150+
$acl.GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | \
151+
ForEach-Object {{ \"$($_.AccessControlType):$($_.IdentityReference.Value):$($_.FileSystemRights)\" }}",
146152
token_path.to_str().unwrap().replace('\'', "''")
147153
),
148154
])
149155
.output()
150-
.expect("read back SDDL");
151-
let sddl = String::from_utf8_lossy(&sddl_out.stdout).trim().to_string();
156+
.expect("read back ACEs");
157+
let aces = String::from_utf8_lossy(&aces_out.stdout);
158+
let allow_with = |sid: &str, right: &str| {
159+
aces.lines().any(|line| {
160+
line.starts_with("Allow:")
161+
&& line.contains(&format!(":{sid}:"))
162+
&& line.contains(right)
163+
})
164+
};
152165

153-
assert!(!sddl.contains(";;;WD)"), "Everyone ACE must be removed: {sddl}");
154-
assert!(sddl.contains("(A;;FA;;;SY)"), "SYSTEM full control expected: {sddl}");
155-
assert!(sddl.contains("(A;;FA;;;BA)"), "Admins full control expected: {sddl}");
156166
assert!(
157-
sddl.contains(&format!(";;;{current_user_sid})")),
158-
"current user read expected: {sddl}"
167+
!aces.contains("S-1-1-0"),
168+
"Everyone ACE must be removed: {aces}"
169+
);
170+
assert!(
171+
allow_with("S-1-5-18", "FullControl"),
172+
"SYSTEM full control expected: {aces}"
173+
);
174+
assert!(
175+
allow_with("S-1-5-32-544", "FullControl"),
176+
"Admins full control expected: {aces}"
177+
);
178+
assert!(
179+
allow_with(&current_user_sid, "Read"),
180+
"current user read expected: {aces}"
159181
);
160182
}
161183

0 commit comments

Comments
 (0)