docs(sandbox): tighten the comments added by the delete-confinement fix

One home per fact: the parent-FILE_DELETE_CHILD rationale now lives in the
acl module doc and the package README instead of being retold in
restrictTokenIntegrity, init(), and grantWrite. Drops the duplicated
inline narration; behavior unchanged.
This commit is contained in:
Huanqi Cao committed 2026-09-19 02:25:01 +08:00
1 parent 36e632751e
commit 7450e1eb24
4 files changed
+46 -91

No files matched your search

+27 -44
View File
@@ -5,17 +5,14 @@
* failure is reported with the API name, the exact Win32 code, the formatted
* system text, and the affected path.
*
* Each grant is a PAIR of edits applied in ONE SetNamedSecurityInfoW call:
* a Deny ACE removing the ambient parent-directory delete right
* (`Everyone` / `FILE_DELETE_CHILD`) plus the capability-SID allow ACE in the
* DACL, and a Low mandatory label with the no-write-up policy in the label
* ACL ({@link buildLowLabelAcl}). The deny makes the capability ACE's DELETE
* bit the only delete authority inside the granted roots — Windows otherwise
* authorizes a delete from the parent directory's `FILE_DELETE_CHILD`, which
* the token's write-restricted intersection does not reach, so one confined
* child could delete files in another granted root. The label denies
* write-class access to every object that is not labeled Low, and the kernel
* applies that policy inside the access check itself.
* Each grant applies three edits in ONE SetNamedSecurityInfoW call: the
* capability-SID allow ACE, a Deny ACE that removes the ambient
* `FILE_DELETE_CHILD` right from the world SID, and a Low no-write-up
* mandatory label ({@link buildLowLabelAcl}). The deny is what keeps one
* granted root out of another's reach: Windows also authorizes a delete from
* the parent directory's `FILE_DELETE_CHILD` right, which the token's
* write-restricted intersection does not reach, and every granted root carries
* the Low label that clears the integrity check.
*
* Concurrency: grants are read-merge-write against the directory's CURRENT
* DACL, and the whole get-merge-set sequence runs under a per-path exclusive
@@ -152,12 +149,10 @@ function readCurrentSecurity(
/**
* Build the Low mandatory label applied with every write grant: one
* SYSTEM_MANDATORY_LABEL_ACE naming `lowLabelSidPtr` with the no-write-up
* policy, inheriting to subcontainers and objects so files created later
* inside the granted tree carry the same label automatically.
*
* The caller frees the returned ACL with LocalFree; SetNamedSecurityInfoW
* copies it. Fail-closed: every Win32 call is checked and the half-built ACL
* is released before the error is thrown.
* policy, inheriting to subcontainers and objects so later children carry the
* same label. The caller frees the returned ACL with LocalFree
* (SetNamedSecurityInfoW copies it); every Win32 call is checked and a
* half-built ACL is released before the error is thrown.
* @param api - the binding table.
* @param lowLabelSidPtr - the Low integrity SID (S-1-16-4096) the label names.
* @returns the ACL carrying the single inheritable label ACE.
@@ -311,11 +306,10 @@ function hasExactGrant(oldAcl: NativePtr, sidPtr: NativePtr): boolean {
}
/**
* True when the merge's ambient-delete deny is already present: the exact
* Deny ACE for {@link abi.FILE_DELETE_CHILD} naming the world SID. Together
* with {@link hasExactGrant} and the label check this gates the idempotent
* skip, so a root granted by an earlier build receives the deny on its next
* provision instead of keeping the parent-`FILE_DELETE_CHILD` route open.
* True when the explicit DACL already carries the EXACT ambient-delete deny:
* the Deny ACE for {@link abi.FILE_DELETE_CHILD} naming the world SID. It is
* part of the idempotent skip, so a root granted by an earlier build receives
* the deny on its next provision.
* @param oldAcl - the current explicit DACL pointer (from {@link readCurrentSecurity}).
* @param worldSidPtr - the Everyone SID the deny names.
* @returns whether the exact deny ACE is already present.
@@ -326,29 +320,18 @@ function hasExactDeny(oldAcl: NativePtr, worldSidPtr: NativePtr): boolean {
/**
* Grant `GRANT_MASK` (Write+Delete, displays as "Modify") to the capability SID
* on `path`, deny the ambient `FILE_DELETE_CHILD` right to the world SID, and
* apply the Low mandatory label — all in one SetNamedSecurityInfoW call.
* on `path`, deny the world SID the ambient `FILE_DELETE_CHILD` right, and
* apply the Low mandatory label — one merge, inheriting to subcontainers and
* objects. The capability ACE's DELETE bit is then the only delete authority
* inside the root, so a file whose own DACL grants no DELETE is no longer
* deletable through its parent's rights.
*
* The deny closes the parent-directory delete route inside the granted roots:
* Windows authorizes a delete from the object's own DELETE right OR from the
* parent's `FILE_DELETE_CHILD`, and the token's write-restricted intersection
* reaches only the first, so without the deny one confined child could delete
* files in ANOTHER granted root (another workspace, a live sibling temp
* directory) whose Low label clears the integrity check. With the deny in
* place the capability ACE's DELETE bit is the only delete authority inside
* the granted roots, for the confined child and the ambient user alike; a file
* whose own DACL grants no DELETE is therefore no longer deletable by its
* parent's default rights.
*
* Idempotent: when the directory's current explicit DACL already carries the
* exact ACE and deny AND its label ACL the exact label (the per-workspace
* grant surviving from a previous server lifetime), the SetNamedSecurityInfoW
* apply is SKIPPED — it would otherwise re-propagate the identical security
* descriptor across the whole tree (eager inheritance; minutes on large
* workspaces). Otherwise read-merge-write: the entries merge into the
* directory's CURRENT explicit DACL (same shape as {@link revokeWrite}), so
* pre-existing explicit ACEs survive. Runs under the per-path lock. The
* directory must be owned by the caller (owner implicit WRITE_DAC) — same
* Idempotent: the exact ACE, deny, and label together SKIP the
* SetNamedSecurityInfoW apply, which would otherwise re-propagate the
* identical descriptor across the whole tree (eager inheritance; minutes on
* large workspaces). Otherwise read-merge-write, so pre-existing explicit ACEs
* survive (same shape as {@link revokeWrite}). Runs under the per-path lock.
* The directory must be owned by the caller (owner implicit WRITE_DAC) — same
* precondition as the POC.
* @param api - the binding table.
* @param path - the directory whose DACL and label gain the grant (the workspace or temp root).
@@ -10,14 +10,10 @@
* keep-alive group logon SID + Everyone; Authenticated Users, INTERACTIVE,
* and LOCAL are absent from both lists — see the seam's dual-list contract
* in `packages/sandbox/sandbox-local` and the package README's Modes section
* for the complete boundary). The intersection covers only the write bits of
* the object's OWN access check: Windows can also authorize a write or delete
* from the parent directory's FILE_DELETE_CHILD right, which no restricting
* SID has to co-sign. The token is therefore ALSO lowered to Low integrity
* and every granted directory carries a Low no-write-up label, so the
* kernel's mandatory-integrity check — evaluated inside the access check
* regardless of which right supplied the authority — confines the child to
* the labeled roots. The write SID is the per-WORKSPACE identity
* for the complete boundary). The intersection covers only the object's own
* access check, so the token is also lowered to Low integrity and every
* granted directory carries a Low no-write-up label and the ambient-delete
* deny the `acl` module documents. The write SID is the per-WORKSPACE identity
* ({@link workspaceWriteSid}): deterministic from the canonical workspace
* path, so the workspace-root ACE materializes once per workspace per
* machine and every later provision hits the exact-ACE skip — the
@@ -263,9 +259,7 @@ export class AclSandbox {
// provision would re-propagate the whole tree) and the temp ACE is
// REVOCABLE (dispose() removes it before the private directory is
// deleted; the ambient temp root is never granted).
// The Low integrity SID every granted directory is labeled with (and the
// confined token is lowered to) plus the world SID the grants deny the
// ambient parent-directory delete right to.
// The Low label SID and the world SID the grants deny and label with.
const lowLabelSid = makeWellKnownSid(api, abi.WinLowLabelSid)
const worldSid = makeWellKnownSid(api, abi.WinWorldSid)
this.sidAllocations.push(lowLabelSid, worldSid)
@@ -292,12 +286,6 @@ export class AclSandbox {
{ world: worldSid },
this.mode,
)
// Lower the confined token to Low integrity. The write-SID intersection
// does not reach the parent directory's FILE_DELETE_CHILD authority, so
// without the matching label a confined child could delete anything its
// ambient user SIDs may delete; the label plus this level close that
// route, and the granted directories stay writable because
// grantWrite labels them Low too.
restrictTokenIntegrity(api, restrictedToken, lowLabelSid)
this.token = restrictedToken
// The restricted token's default DACL still names only the user's
@@ -145,20 +145,10 @@ export function setTokenDefaultDaclGrant(api: Win32Bindings, token: NativePtr, s
}
/**
* Lower the restricted token's integrity level to Low (S-1-16-4096).
*
* `WRITE_RESTRICTED` intersects only the write bits of the access mask it
* evaluates against an object's DACL. Windows can also authorize a write or a
* delete from the PARENT directory's FILE_DELETE_CHILD right, and that
* authority is granted without the restricting-SID intersection — so a
* confined child could still delete files anywhere its ambient user SIDs hold
* Modify. The mandatory label closes that route: the kernel applies the
* integrity policy inside the access check, wherever the authority came from,
* so a Low child only writes where the object is labeled Low (see
* `grantWrite`).
*
* Requires TOKEN_ADJUST_DEFAULT on the token (the caller opens it with the
* right). Fails closed: a failure throws before any child is spawned.
* Lower the restricted token's integrity level to Low (S-1-16-4096), the level
* the mandatory labels `grantWrite` applies are matched against; a token left
* at Medium would ignore them. Requires TOKEN_ADJUST_DEFAULT on the token;
* fails closed before any child is spawned.
* @param api - the binding table.
* @param token - the restricted token to lower.
* @param lowLabelSidPtr - the Low integrity SID (S-1-16-4096).
@@ -400,16 +400,11 @@ describe.skipIf(!isWin32 || !pwshAvailable())('windows-acl runner', () => {
}, 30_000)
it('delete-escape regression: deletes outside the granted roots are denied in BOTH modes, through every delete authority', () => {
// Windows authorizes a delete from EITHER the object's own DELETE right OR
// the parent directory's FILE_DELETE_CHILD right, and the
// WRITE_RESTRICTED pass-2 intersection only covers the first. Every
// ordinary directory the ambient user SIDs control (a project checkout,
// %TEMP%, the profile) grants FILE_DELETE_CHILD, so `cmd /c del` deleted
// files outside the workspace even though a plain write from the same
// child was denied. The Low mandatory label closes both routes because the
// kernel applies the integrity policy inside the access check, whichever
// right supplied the authority; the remaining delete paths below are the
// ones a confined child reaches for.
// Windows authorizes a delete from the object's own DELETE right OR the
// parent directory's FILE_DELETE_CHILD, and the write-restricted
// intersection only covers the first — so `cmd /c del` used to delete
// outside the workspace. The Low label plus the grant's deny close both
// routes; these are the deleters a confined child reaches for.
for (const mode of ['read-only', 'workspace-write'] as const) {
const victims = {
cmd: join(scratchRoot, `delete-${mode}-cmd.txt`),
@@ -451,12 +446,11 @@ describe.skipIf(!isWin32 || !pwshAvailable())('windows-acl runner', () => {
}, 60_000)
it('cross-root delete regression: one session cannot delete inside ANOTHER granted root', () => {
// Both roots carry the Low label, so the integrity check alone would let a
// confined child delete in the other one: Windows authorizes a delete from
// the parent directory's FILE_DELETE_CHILD, which the write-restricted
// intersection does not reach. The grant therefore denies that right to the
// world SID, leaving the capability ACE's DELETE bit as the only delete
// authority inside a granted root.
// Both roots carry the Low label, so the integrity check alone would not
// stop this: Windows also authorizes a delete from the parent directory's
// FILE_DELETE_CHILD right, which the capability intersection never reaches.
// The grant denies that right, leaving the capability DELETE bit as the
// only delete authority inside a granted root.
const otherWorkspace = join(scratchRoot, 'other-workspace')
const otherTemp = join(scratchRoot, 'other-temp')
mkdirSync(otherWorkspace)