Skip to content

Commit 57c50ac

Browse files
zahclaude
andauthored
fix(elevation): ACL principal alias + scheduledTask probe full path (#18)
Two digest-comparison bugs surfaced by L3 production-profile validation after PR #17 unblocked the apply side. Both manifest as ``post-apply observation disagrees with desired`` failures even when the underlying icacls / scheduledTask calls succeeded — the DACL and the task ARE the operator-declared shape on disk; the canonical-state string the digest is taken over diverges only because of alias / path-projection mismatches. 1. ACL: icacls accepts well-known short names (``SYSTEM``, ``NetworkService``, ``Administrators``) as INPUT but emits the domain-qualified form (``NT AUTHORITY\SYSTEM``, ``NT AUTHORITY\NETWORK SERVICE``, ``BUILTIN\Administrators``) on OUTPUT. ``normalizeDirAclEntry`` only collapsed whitespace, so a recipe declaring ``aclEntry(principal = "SYSTEM", ...)`` produced a digest with ``SYSTEM:(F)`` while the observed digest had ``NT AUTHORITY\SYSTEM:(F)`` — they never converged. Add ``canonicalizeAclPrincipal`` (case-insensitive collapse of the seven well-known aliases to their icacls-emitted form) and thread it through ``normalizeDirAclEntry`` (split the entry at the first ``:`` outside any ``(...)``; rewrite the principal prefix; preserve the rest verbatim). 2. scheduledTask: the probe's ``'TaskName=' + $t.TaskName`` line returned the LEAF name only (``WindowsRunner-EnvBootstrap``) while ``canonicalScheduledTaskDesired`` carried the full path (``\Reprobuild\WindowsRunner-EnvBootstrap``). The post-apply re-probe digests disagreed even when the task WAS registered in the operator-declared folder. Emit ``$t.TaskPath + $t.TaskName`` (``$t.TaskPath`` already ends in ``\``) so the observed canonical string carries the same shape as the desired one. Update the ``normalizeDirAclEntry`` smoke test to cover the new canonicalization branches (the previous test assumed the no-op SYSTEM:(F) shape). --no-verify: pre-existing prek migration-mode failure (no .pre-commit-config.yaml in repo). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9b115ba commit 57c50ac

3 files changed

Lines changed: 81 additions & 14 deletions

File tree

libs/repro_elevation/src/repro_elevation/posix_system_parse.nim

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,14 +1204,43 @@ proc renderIcaclsGrantCommandArgs*(path, entry: string): seq[string] =
12041204
of icaclsDenyVerb:
12051205
@["icacls", path, "/deny", split.spec]
12061206

1207+
proc canonicalizeAclPrincipal*(p: string): string =
1208+
## Collapse the well-known Windows principal aliases ``icacls``
1209+
## accepts as INPUT to the domain-qualified form it emits as
1210+
## OUTPUT, so a desired entry written ``SYSTEM:(F)`` matches an
1211+
## observed entry written ``NT AUTHORITY\SYSTEM:(F)``.
1212+
## Case-insensitive on the alias side (``System``, ``SYSTEM``,
1213+
## ``system`` are all icacls-equivalent); the canonical form keeps
1214+
## the exact spelling icacls emits (mixed-case on the
1215+
## Network/Local Service family, all-caps on SYSTEM).
1216+
##
1217+
## Without this collapse the post-apply digest of a
1218+
## ``fs.systemDirectory`` recipe declaring
1219+
## ``aclEntry(principal = "SYSTEM", ...)`` disagrees with the
1220+
## observed ACL even when ``icacls`` happily granted exactly that
1221+
## ACE — operators would have to hand-write
1222+
## ``NT AUTHORITY\SYSTEM`` to make the digest converge.
1223+
case p.toLowerAscii
1224+
of "system": "NT AUTHORITY\\SYSTEM"
1225+
of "localservice", "local service": "NT AUTHORITY\\LOCAL SERVICE"
1226+
of "networkservice", "network service": "NT AUTHORITY\\NETWORK SERVICE"
1227+
of "administrators": "BUILTIN\\Administrators"
1228+
of "users": "BUILTIN\\Users"
1229+
of "everyone": "Everyone"
1230+
of "authenticated users": "NT AUTHORITY\\Authenticated Users"
1231+
else: p
1232+
12071233
proc normalizeDirAclEntry*(ace: string): string =
1208-
## Stable rendering of an ACE for digest purposes: trim outer
1209-
## whitespace and collapse all internal whitespace runs to single
1210-
## spaces. icacls permits extra whitespace inside the perm groups;
1211-
## the canonical form collapses these so two visually-equivalent
1212-
## ACE strings hash to the same digest. Mirrors the
1213-
## `normalizeAclEntry` shape in `windows_system_parse.nim` so the
1214-
## two ACL kinds digest equivalent entries identically.
1234+
## Stable rendering of an ACE for digest purposes: collapse
1235+
## internal whitespace and canonicalise the principal alias
1236+
## (``SYSTEM`` ↔ ``NT AUTHORITY\SYSTEM`` etc.). icacls permits
1237+
## extra whitespace inside the perm groups and accepts well-known
1238+
## short names as input but always emits the domain-qualified form
1239+
## on output; the canonical form collapses both axes so two
1240+
## visually-equivalent ACE strings hash to the same digest.
1241+
## Mirrors the ``normalizeAclEntry`` shape in
1242+
## ``windows_system_parse.nim`` so the two ACL kinds digest
1243+
## equivalent entries identically.
12151244
var collapsed = ""
12161245
var prevWasSpace = false
12171246
for ch in ace.strip():
@@ -1222,7 +1251,25 @@ proc normalizeDirAclEntry*(ace: string): string =
12221251
else:
12231252
collapsed.add(ch)
12241253
prevWasSpace = false
1225-
collapsed
1254+
# Split principal:perm-spec at the FIRST ``:`` that is NOT inside a
1255+
# parenthesised group (an ACE shape is ``Principal[\Name]:(perms)``).
1256+
# The principal canonicalisation only rewrites the prefix.
1257+
var depth = 0
1258+
var splitIdx = -1
1259+
for i, ch in collapsed:
1260+
case ch
1261+
of '(': inc depth
1262+
of ')': dec depth
1263+
of ':':
1264+
if depth == 0:
1265+
splitIdx = i
1266+
break
1267+
else: discard
1268+
if splitIdx < 0:
1269+
return collapsed
1270+
let principal = collapsed[0 ..< splitIdx]
1271+
let rest = collapsed[splitIdx .. ^1]
1272+
canonicalizeAclPrincipal(principal) & rest
12261273

12271274
proc canonicalDirAclDesired*(present: bool; owner: string;
12281275
entries: seq[string];

libs/repro_elevation/src/repro_elevation/windows_system_driver.nim

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1473,7 +1473,12 @@ when defined(windows):
14731473
" -TaskName " & psQuote(parts.leaf) &
14741474
" -ErrorAction SilentlyContinue; " &
14751475
"if ($null -eq $t) { 'Missing=1' } else { " &
1476-
"'TaskName=' + $t.TaskName; " &
1476+
# Emit the FULL path (``$t.TaskPath`` already ends with ``\``)
1477+
# so the canonical observed-state string matches the desired
1478+
# one (which carries the full ``\Folder\Leaf`` path). Without
1479+
# this the digest disagrees even when Register/Get/Unregister
1480+
# all targeted the same folder.
1481+
"'TaskName=' + $t.TaskPath + $t.TaskName; " &
14771482
"'Executable=' + $t.Actions[0].Execute; " &
14781483
"'Arguments=' + $t.Actions[0].Arguments; " &
14791484
"'WorkingDirectory=' + $t.Actions[0].WorkingDirectory; " &

libs/repro_elevation/tests/t_smoke_repro_elevation.nim

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,11 +1175,26 @@ suite "repro_elevation: fs.systemDirectory Phase D — drift digest":
11751175
observedInheritanceDisabled = false,
11761176
desiredInheritance = "enabled")
11771177

1178-
test "normalizeDirAclEntry collapses internal whitespace":
1179-
check normalizeDirAclEntry("SYSTEM:(F)") == "SYSTEM:(F)"
1180-
check normalizeDirAclEntry(" SYSTEM:(F) ") == "SYSTEM:(F)"
1181-
check normalizeDirAclEntry("SYSTEM:(F) (OI)") == "SYSTEM:(F) (OI)"
1182-
check normalizeDirAclEntry("SYSTEM:(F)\t (CI)") == "SYSTEM:(F) (CI)"
1178+
test "normalizeDirAclEntry collapses internal whitespace + canonicalises principal":
1179+
# The principal-alias collapse (added alongside the whitespace
1180+
# pass) rewrites ``SYSTEM`` to ``NT AUTHORITY\SYSTEM`` so the
1181+
# desired digest matches the icacls-emitted observed digest.
1182+
check normalizeDirAclEntry("SYSTEM:(F)") == "NT AUTHORITY\\SYSTEM:(F)"
1183+
check normalizeDirAclEntry(" SYSTEM:(F) ") ==
1184+
"NT AUTHORITY\\SYSTEM:(F)"
1185+
check normalizeDirAclEntry("SYSTEM:(F) (OI)") ==
1186+
"NT AUTHORITY\\SYSTEM:(F) (OI)"
1187+
check normalizeDirAclEntry("SYSTEM:(F)\t (CI)") ==
1188+
"NT AUTHORITY\\SYSTEM:(F) (CI)"
1189+
# A principal already in domain-qualified form is left alone.
1190+
check normalizeDirAclEntry("NT AUTHORITY\\SYSTEM:(F)") ==
1191+
"NT AUTHORITY\\SYSTEM:(F)"
1192+
# NetworkService -> NT AUTHORITY\NETWORK SERVICE.
1193+
check normalizeDirAclEntry("NetworkService:(RX)") ==
1194+
"NT AUTHORITY\\NETWORK SERVICE:(RX)"
1195+
# An unknown principal is left alone (the canonicaliser only
1196+
# rewrites the documented well-known set).
1197+
check normalizeDirAclEntry("DOMAIN\\Alice:(F)") == "DOMAIN\\Alice:(F)"
11831198

11841199
test "fsSystemDirectoryDigestPayload back-compat for ACL-unmanaged":
11851200
# The aclPresent==false branch MUST produce a string

0 commit comments

Comments
 (0)