emrg: a registry key grant is enforced on Windows through its own DACL applier

emrg/sandbox/fence.py

@@ -52,7 +52,7 @@
 
 from emrg.sandbox.grants import GrantKindNotEnforced
 from emrg.sandbox.policy import DANGER_FULL_ACCESS, SandboxPolicy
-from emrg.sandbox.roots import canonical_path, writable_roots
+from emrg.sandbox.roots import canonical_path, named_grants, writable_roots
 from emrg.tools.file_policy import protected_paths
 
 
@@ -91,14 +91,21 @@ def file_refusal(file_path: str, policy: SandboxPolicy) -> str | None:
         # ``emrg/tools/file_policy.py`` because the legacy scanner reads it too.
         return f"{policy.mode} sandbox: blocked write to protected daemon file {file_path!r}"
     try:
-        roots = writable_roots(policy)
+        # The enforcement question is asked here rather than left to
+        # ``writable_roots``, which projects paths and so has no answer for a
+        # kind that contributes none. Asking it through the projection made it
+        # refuse an *enforceability* problem, and the two became inseparable the
+        # moment Windows began enforcing a key (PR #2498): the raise stopped, and
+        # with it both the refusal and the projection's own guarantee.
+        named_grants(policy)
     except GrantKindNotEnforced as refused:
         # A grant this platform cannot enforce is refused rather than dropped
         # (issue #2492, requirement 5). The fence answers in text because its
         # callers render a refusal to the model; the profile builders let the
         # same exception propagate instead, so a confined command fails closed
         # rather than running with a boundary nobody applied.
         return f"{policy.mode} sandbox: {refused.reason}"
+    roots = writable_roots(policy)
     if any(_is_within_root(target, root) for root in roots):
         return None
     granted = (

emrg/sandbox/grants.py

@@ -69,15 +69,18 @@
 
 #: The kinds **this platform's dialects implement**, per platform family.
 #:
-#: ``registry`` is deliberately in none of them: the kind is modelled and
-#: :func:`enforcement_refusal` names the reason, but the code that applies a DACL
-#: to a key does not exist yet (issue #2492 Stage 2). A set rather than a comment
-#: because two callers read it — the refusal sentence, and the guard that keeps
-#: :data:`DEFAULT_MANIFEST` from claiming a kind its platform cannot honour.
+#: ``registry`` is enforced on ``win32`` alone: a key's DACL is read and rewritten
+#: through ``RegGetKeySecurity`` / ``RegSetKeySecurity``
+#: (:mod:`emrg.sandbox.win32.registry`).  The other three families name no
+#: registry API at all, so for them the kind stays modelled and unenforced, and
+#: :func:`enforcement_refusal` says so by name (issue #2492 Stage 2).  A set
+#: rather than a comment because two callers read it — the refusal sentence, and
+#: the guard that keeps :data:`DEFAULT_MANIFEST` from claiming a kind its
+#: platform cannot honour.
 ENFORCED_KINDS: dict[str, tuple[str, ...]] = {
     "darwin": (GRANT_DIRECTORY, GRANT_FILE),
     "linux": (GRANT_DIRECTORY, GRANT_FILE),
-    "win32": (GRANT_DIRECTORY, GRANT_FILE),
+    "win32": (GRANT_DIRECTORY, GRANT_FILE, GRANT_REGISTRY),
     "other": (GRANT_DIRECTORY, GRANT_FILE),
 }
 
@@ -90,13 +93,12 @@
 #: Two properties this table owes, both asserted by tests rather than trusted:
 #:
 #: * **a row's kind is one its platform can enforce** — :data:`ENFORCED_KINDS` is
-#:   the reader, so the Windows registry keys the rant also names are absent
-#:   here until Stage 2 can enforce them. A manifest promising a grant the run
-#:   does not have would be the lying boundary this module refuses elsewhere;
-#: * **the table is data, not a code path** — an entry whose directory does not
-#:   exist on this host is simply absent from the derivation
-#:   (:func:`default_grants`), so a deployment that has never run ``lark-cli``
-#:   is not an error and not a grant.
+#:   the reader, so a row is only written for a kind that platform's dialect
+#:   implements. That is what the Windows registry rows below are held to, and
+#:   why no ``darwin``/``linux`` row names one;
+#: * **the table is data, not a code path** — an entry whose object does not exist
+#:   on this host is simply absent from the derivation (:func:`default_grants`), so
+#:   a deployment that has never run ``lark-cli`` is not an error and not a grant.
 #:
 #: ``%APPDATA%`` is expanded with :func:`os.path.expandvars`, which understands
 #: the ``%NAME%`` form on Windows only; on POSIX the two darwin/linux rows carry
@@ -113,6 +115,16 @@
     "win32": (
         (GRANT_DIRECTORY, "~/.lark-cli"),
         (GRANT_DIRECTORY, "%APPDATA%/Microsoft/Protect"),
+        # Two rows for one tool, and both are load-bearing: the **parent** is what
+        # lets the confined child create ``keychain`` in the first place (the
+        # capability ACE is inheritable), while the **key** itself is what reaches
+        # a ``keychain`` that already exists — inheritance applies when a key is
+        # created, and ``RegSetKeySecurity`` on the parent does not walk the keys
+        # already beneath it. A deployment where ``keychain`` is absent loses only
+        # the second row (:func:`default_grants` is total over a key this host does
+        # not have) and still gets the first.
+        (GRANT_REGISTRY, "HKCU\\Software\\LarkCli"),
+        (GRANT_REGISTRY, "HKCU\\Software\\LarkCli\\keychain"),
     ),
 }
 
@@ -292,10 +304,18 @@ def default_grants(platform: str | None = None) -> list[RootGrant]:
     out: list[RootGrant] = []
     for kind, spelling in DEFAULT_MANIFEST.get(family, ()):
         target = os.path.expandvars(os.path.expanduser(spelling))
-        try:
-            exists = os.path.isdir(target) if kind == GRANT_DIRECTORY else os.path.exists(target)
-        except OSError:  # pragma: no cover - both calls swallow their own errors
-            exists = False
+        if kind == GRANT_REGISTRY:
+            # Imported here rather than at module scope: the registry predicate is
+            # Windows-only machinery, and this derivation runs on every platform
+            # for every family. Off Windows it answers False without touching it.
+            from emrg.sandbox.win32.registry import key_exists  # noqa: PLC0415
+
+            exists = key_exists(target)
+        else:
+            try:
+                exists = os.path.isdir(target) if kind == GRANT_DIRECTORY else os.path.exists(target)
+            except OSError:  # pragma: no cover - both calls swallow their own errors
+                exists = False
         if not exists:
             continue
         out.append(RootGrant(kind=kind, target=target, source=SOURCE_DEFAULT))

emrg/sandbox/providers/win32.py

@@ -44,6 +44,7 @@
 from dataclasses import dataclass
 
 from emrg.sandbox.contract import Runner, RunnerFailureRule
+from emrg.sandbox.grants import GRANT_REGISTRY
 from emrg.sandbox.policy import SandboxPolicy
 from emrg.sandbox.roots import canonical_path, named_grants
 from emrg.sandbox.win32.ffi import Win32Bindings, win32
@@ -387,10 +388,12 @@ def runner_argv(policy: SandboxPolicy) -> list[str]:
             "--temp", tempfile.gettempdir(),
             "--mode", policy.mode,
         ]
-    extra_roots = _extra_root_spellings(policy)
+    path_spellings, key_spellings = _extra_grant_spellings(policy)
     extra_flags: list[str] = []
-    for root in extra_roots:
+    for root in path_spellings:
         extra_flags += ["--extra-root", root]
+    for key in key_spellings:
+        extra_flags += ["--extra-key", key]
     if policy.session_id is None:
         return [
             *runner_invocation(),
@@ -399,7 +402,7 @@ def runner_argv(policy: SandboxPolicy) -> list[str]:
             "--mode", policy.mode,
             *extra_flags,
         ]
-    capability = STORE.materialize(policy.session_id, workspace, extra_roots=extra_roots)
+    capability = STORE.materialize(policy.session_id, workspace, extra_roots=path_spellings)
     return [
         *runner_invocation(),
         "--workspace", workspace,
@@ -411,25 +414,51 @@ def runner_argv(policy: SandboxPolicy) -> list[str]:
     ]
 
 
-def _extra_root_spellings(policy: SandboxPolicy) -> tuple[str, ...]:
-    """The enforceable extra grants' paths — the manifest's and the session's.
+def _extra_grant_spellings(policy: SandboxPolicy) -> tuple[tuple[str, ...], tuple[str, ...]]:
+    """The enforceable extra grants' spellings, split by object kind.
 
     Read from the shared derivation (:func:`~emrg.sandbox.roots.named_grants`)
     rather than off ``policy.extra_grants``, because a grant the host never typed
     is still one this run must be able to write under: that is what the default
     manifest is for (issue #2492). Canonicalization and de-duplication happen in
     that derivation, once, for every dialect.
 
-    A **registry** grant never reaches this function: no platform enforces that
-    kind yet, and the derivation raises
-    :class:`~emrg.sandbox.grants.GrantKindNotEnforced` rather than handing a key
-    spelling to a path consumer. On Windows that is the loop Stage 2 closes — the
-    key needs a DACL of its own, not an ``--extra-root`` flag.
+    The split is the whole point of the pair: a path grant becomes an
+    ``--extra-root`` and a key grant becomes an ``--extra-key``, because this
+    backend applies a DACL to a *path* with ``SetNamedSecurityInfoW`` and to a
+    *key* with ``RegSetKeySecurity``. Handing a key spelling to the path half
+    would grant a filesystem object nobody named.
+
+    :param policy: the resolved per-call policy.
+    :returns: the path spellings and the key spellings.
+    """
+    named = named_grants(policy)
+    return (
+        tuple(grant.target for grant in named if grant.kind != GRANT_REGISTRY),
+        tuple(grant.target for grant in named if grant.kind == GRANT_REGISTRY),
+    )
+
+
+def _extra_root_spellings(policy: SandboxPolicy) -> tuple[str, ...]:
+    """The enforceable extra grants' paths — the manifest's and the session's.
+
+    :param policy: the resolved per-call policy.
+    :returns: the canonical path spellings.
+    """
+    return _extra_grant_spellings(policy)[0]
+
+
+def _extra_key_spellings(policy: SandboxPolicy) -> tuple[str, ...]:
+    """The enforceable extra grants' registry keys — the manifest's and the session's.
+
+    One producer for the key half too, for the reason the path half has one: the
+    ACE the seam applies and the SID the runner puts in the token are derived from
+    this same list, so a key cannot end up granted on one side only.
 
     :param policy: the resolved per-call policy.
-    :returns: the canonical spellings.
+    :returns: the canonical key spellings.
     """
-    return tuple(grant.target for grant in named_grants(policy))
+    return _extra_grant_spellings(policy)[1]
 
 
 def release_session(session_id: str) -> None:

emrg/sandbox/roots.py

@@ -14,7 +14,7 @@
 
 import os
 import tempfile
-from typing import TYPE_CHECKING, NamedTuple
+from typing import TYPE_CHECKING, Callable, NamedTuple
 
 from emrg.sandbox.grants import (
     GRANT_DIRECTORY,
@@ -192,27 +192,70 @@ def granted_grants(policy: SandboxPolicy) -> list[RootGrant]:
     """
     if policy.mode != "workspace-write":
         return []
-    return _dedupe([*_mode_grants(policy), *named_grants(policy)])
+    return _composed(policy, named_grants)
+
+
+def _composed(
+    policy: SandboxPolicy, named: Callable[[SandboxPolicy], list[RootGrant]]
+) -> list[RootGrant]:
+    """One composition of a policy's grants: the mode's sources, then the named ones.
+
+    The two named-list policies differ in exactly one respect — whether the
+    enforcement table is consulted (:func:`named_grants` asks it,
+    :func:`stored_grants` does not) — and both reach this same composition, so
+    the readers of either cannot disagree about which grants exist: "the shell
+    can write where ``write`` cannot" needs one derivation, not one call site.
+
+    ``read-only`` (and ``danger-full-access``, which never asks) grant nothing:
+    the early return is what makes requirement 4 of issue #2492 true by
+    construction rather than by a check — a default manifest cannot widen a tier
+    that grants none, because this function never consults one for that tier.
+
+    :param policy: the file-effect policy to derive the grant list from.
+    :param named: which named-list policy to append — the enforcement-checked
+        one, or the display one the path projection reads.
+    :returns: the canonical, deduplicated grants; empty for every mode but
+        ``workspace-write``.
+    """
+    if policy.mode != "workspace-write":
+        return []
+    return _dedupe([*_mode_grants(policy), *named(policy)])
 
 
 def writable_roots(policy: SandboxPolicy) -> list[str]:
-    """The **paths** among :func:`granted_grants` — the projection path-readers use.
+    """The **paths** a policy grants — the projection path-readers use.
 
-    A derived view, not a second allow-list: it is built from
-    :func:`granted_grants` and can therefore never disagree with it. It exists
+    A derived view, not a second allow-list: it is built by the same
+    :func:`_composed` composition :func:`granted_grants` reads, and applies one
+    filter to it, so it can never disagree with that derivation. It exists
     because several readers need nothing but paths — the fence's containment
     test, the confined cache relocation, and the providers' own spellings — and
     because a registry grant has no path to contribute, so a path-only reader
     must not be handed one.
 
+    **It does not ask the enforcement table**, and that is the point of the
+    split. Whether this platform can enforce a registry key is a question about
+    enforcement; a key contributes no path either way. Asking it here is what
+    once made this projection's own guarantee unmeasurable: while no platform
+    enforced a key the projection refused before it could be asked, and the day
+    one did — Windows, PR #2498 — the refusal vanished and so did the "no key
+    among the paths" claim, leaving a green reading about nothing. The loud
+    answer to a grant this platform cannot enforce is :func:`named_grants`'s,
+    and every reader that *acts* on grants reads that one: the profile builders
+    directly, and the fence by asking for the refusal itself.
+
     ``read-only`` still answers ``[]``, which is the property the host's rule
     rests on: at a tier that grants nothing, nothing is writable however the
     session or the manifest is configured.
 
     :param policy: the file-effect policy to derive the allow-list from.
     :returns: the canonical writable paths.
     """
-    return [grant.target for grant in granted_grants(policy) if grant.kind != GRANT_REGISTRY]
+    return [
+        grant.target
+        for grant in _composed(policy, stored_grants)
+        if grant.kind != GRANT_REGISTRY
+    ]
 
 
 #: The shared POSIX temp root — one spelling, named once, read through

emrg/sandbox/win32/abi.py

@@ -96,6 +96,51 @@
 #: Maximum SID allocation size in bytes.
 SECURITY_MAX_SID_SIZE = 68
 
+# --- registry keys (issue #2492 Stage 2) -----------------------------------
+
+#: Predefined ``HKEY_CURRENT_USER`` handle.
+HKEY_CURRENT_USER = 0x80000001
+#: Predefined ``HKEY_LOCAL_MACHINE`` handle.
+HKEY_LOCAL_MACHINE = 0x80000002
+#: Standard right: read an object's DACL.
+READ_CONTROL = 0x00020000
+#: Standard right: replace an object's DACL.
+WRITE_DAC = 0x00040000
+#: Key access needed to read a key's DACL and rewrite it — and nothing else.
+#:
+#: ``RegOpenKeyExW`` wants the rights to be asked for up front, so the applier
+#: asks for exactly the two the edit needs: with these, a key whose DACL the
+#: caller may not rewrite fails at the open, before any merge is attempted.
+KEY_SECURITY_ACCESS = READ_CONTROL | WRITE_DAC
+#: Key right: read a key's values.
+KEY_QUERY_VALUE = 0x0001
+#: Key right: write a key's values.
+KEY_SET_VALUE = 0x0002
+#: Key right: create a subkey.
+KEY_CREATE_SUB_KEY = 0x0004
+#: Key right: enumerate a key's subkeys.
+KEY_ENUMERATE_SUB_KEYS = 0x0008
+#: Key right: receive change notifications for a key.
+KEY_NOTIFY = 0x0010
+#: Capability-SID access mask granting read, value writes and subkey creation.
+#:
+#: The registry's counterpart of :data:`GRANT_MASK`, and it excludes the standard
+#: rights for the same reason: ``WRITE_DAC`` and ``WRITE_OWNER`` would let a
+#: confined child rewrite the key's DACL or take its ownership and walk out of the
+#: allowlist it was handed. ``READ_CONTROL`` is excluded with them — the mask
+#: grants what a tool does to a key's *contents*, never what it does to the key's
+#: own security descriptor.
+REGISTRY_GRANT_MASK = (
+    KEY_QUERY_VALUE | KEY_SET_VALUE | KEY_CREATE_SUB_KEY | KEY_ENUMERATE_SUB_KEYS | KEY_NOTIFY
+)
+#: ``SECURITY_DESCRIPTOR_REVISION``: the only revision ``InitializeSecurityDescriptor`` accepts.
+SECURITY_DESCRIPTOR_REVISION = 1
+#: The bytes an absolute ``SECURITY_DESCRIPTOR`` occupies.
+#:
+#: Named by the SDK for exactly this use — the buffer an absolute descriptor is
+#: built in before ``MakeSelfRelativeSD`` measures the form the API wants.
+SECURITY_DESCRIPTOR_MIN_LENGTH = 20
+
 # --- process creation (Win32 process owner, win32-process/src/abi.ts) -------
 
 #: STARTUPINFO.dwFlags: the std handles in the struct are meaningful.

emrg/sandbox/win32/acl.py

@@ -278,12 +278,14 @@ def merge_and_apply(
         throw_last_error(api, "LocalFree", f"{label}({path}) new ACL")
 
 
-def has_exact_grant(api: Win32Bindings, old_acl: int, sid_ptr: int) -> bool:
+def has_exact_grant(
+    api: Win32Bindings, old_acl: int, sid_ptr: int, mask: int = abi.GRANT_MASK
+) -> bool:
     """Whether the DACL already carries the exact write grant this module adds.
 
     Every field is read through bounded offset reads (Allow ACE, ``OI|CI``
-    inheritance, :data:`~emrg.sandbox.win32.abi.GRANT_MASK`, the capability
-    SID) — no pointer arithmetic, no ``memcpy``.  The ACE's SID is **inline**
+    inheritance, the access mask, the capability SID) — no pointer arithmetic,
+    no ``memcpy``.  The ACE's SID is **inline**
     (embedded after the 4-byte mask), so there is no pointer to follow: it is
     read byte-wise against the ACE's own size.  A malformed header reads as "no
     exact grant", which sends the caller down the merge path — the path that
@@ -292,6 +294,10 @@ def has_exact_grant(api: Win32Bindings, old_acl: int, sid_ptr: int) -> bool:
     :param api: the binding table.
     :param old_acl: the current explicit DACL pointer.
     :param sid_ptr: the capability SID to match.
+    :param mask: the access mask the grant's ACE carries — the file rights by
+        default, and :data:`~emrg.sandbox.win32.abi.REGISTRY_GRANT_MASK` for a
+        registry key.  A second checker was the alternative and would have been
+        this one with one name changed.
     :returns: whether the exact grant ACE is already present.
     """
     header = ACLStructure.from_address(old_acl)
@@ -307,11 +313,11 @@ def has_exact_grant(api: Win32Bindings, old_acl: int, sid_ptr: int) -> bool:
         ace_size = int(ctypes.c_uint16.from_address(old_acl + offset + 2).value)
         if ace_size < 8 or offset + ace_size > acl_size:
             return False  # implausible: fall back to the merge path
-        mask = int(ctypes.c_uint32.from_address(old_acl + offset + 4).value)
+        ace_mask = int(ctypes.c_uint32.from_address(old_acl + offset + 4).value)
         exact = (
             ace_type == abi.ACCESS_ALLOWED_ACE_TYPE
             and ace_flags == abi.SUB_CONTAINERS_AND_OBJECTS_INHERIT
-            and mask == abi.GRANT_MASK
+            and ace_mask == mask
         )
         if exact:
             inline = _inline_sid_bytes(old_acl + offset + 8, ace_size - 8)