Skip to content

emrg: a registry key can be typed, validated and withdrawn - #2512

Merged
argszero merged 3 commits into
masterfrom
feature/a-registry-key-can-be-typed
Oct 23, 2026
Merged

argszero merged 3 commits into
masterfrom
feature/a-registry-key-can-be-typed

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2511

A registry key was modelled (kind="registry") and enforceable (the DACL applier landed with #2498), but no command could reach either: /sandbox add had no way to say that the argument is a key, so the daemon always received kind=None, applied the path rules to HKCU\Software\..., and answered "relative spellings are not roots". The enforcement existed and was unreachable.

What changed

The kind is stated, never inferred. /sandbox add|remove --registry <key> — one mapping (grants.KIND_FLAGS) is the single producer of the flag, read by the client that parses the command and by the daemon that judges the kind. A key has nothing on a POSIX disk to read a kind off, which is the identity defect this whole issue exists to remove; a key is granted as itself.

Validation runs through the applier's own parser. judge_registry_key calls registry.split_key, the same function apply_dacl uses, so a key the writer accepts is one the enforcement can address, and there is no second parser with a second wording. It is judged before the platform question: a malformed key is malformed everywhere, and telling a host "this platform has no registry" when their spelling names no key sends them to fix the wrong thing.

The stored target is canonical. A capability SID is derived from the spelling (registry_write_sid), so HKCU and HKEY_CURRENT_USER — both accepted, both one hive — would otherwise derive two identities for one key. canonical_key_spelling converges the root; the subkey keeps the host's case because the namespace is case-insensitive anyway. The duplicate check then compares case-insensitively, so naming a key twice is answered rather than stored twice.

Withdrawal has the key's own branch. A key is not absolute, is not a filesystem root, and has nothing to stat, so judge_root_removal skips the path rules when the kind is stated and validates only the spelling — never the policy, because a withdrawal is not a grant.

Two defects found while doing it

A key named at read-only would have been stored twice. The duplicate check initially read granted_grants, which answers [] for every mode but workspace-write — so at the lower tier nothing was found and the same key went on the list again. It now reads stored_grants, the mode-independent list: the session's list follows the session, not the tier, exactly as it does for a path. test_a_key_named_twice_at_read_only_is_not_stored_twice pins it and reddens when the reader is put back.

enforcement_refusal still promised the enforcement would ship. Its sentence closed with "issue #2492 Stage 2 adds registry enforcement", which #2498 had already falsified: on a platform that cannot apply a key, the host was told the capability was coming to their platform, when the table says win32 alone. The clause is now derived from ENFORCED_KINDS, so it names where the kind is enforced and cannot drift from the row it describes. Three carriers of the old promise were corrected: the function, the module comment, and two tests that asserted the literal "Stage 2".

Guard

test_every_stated_flag_is_named_in_the_command_help holds /sandbox's help line — the only place a host learns the syntax, and prose, the carrier that goes stale in silence — to KIND_FLAGS, so a flag cannot ship undiscoverable.

Verification

  • uv run pytest tests/ — 5701 passed, 35 skipped
  • the three touched files in isolation — 145 passed, 10 skipped
  • uv run python -c "from emrg.client.app import run_client", uv run python -m emrg --help — both clean
  • both new guards armed in the other direction and observed red

Not in this PR

The Electron SandboxRootsDialog still sends no kind, so a key typed there meets the path rules and is refused as a relative spelling — the pre-existing behaviour, not a regression. A key is typed in the TUI for now.

CI on this head: pending at the time of opening.

The model carried kind=registry and Stage 2 landed the enforcement, but nothing could reach it: /sandbox add had no flag to state a kind, so the daemon always received None and applied the path rules to a key. VALIDATION runs through split_key — the parser the DACL applier itself uses — so an accepted key is one enforcement can address; the stored target is canonical, because a capability SID is derived from the spelling and HKCU/HKEY_CURRENT_USER would otherwise be two identities on one key. The duplicate check reads stored_grants rather than granted_grants: the second answers [] for every mode but workspace-write, so a key named at read-only would have been stored twice. enforcement_refusal's trailing promise that a stage would add the enforcement was falsified by #2498; it now derives which family does enforce the kind from ENFORCED_KINDS, so no sentence can outlive its row. The /sandbox help line names the flag, held to KIND_FLAGS by a test.
EMRG Evolution added 2 commits October 23, 2026 09:18
…e/a-registry-key-can-be-typed

# Conflicts:
#	tests/test_bash_v2_win32.py
Ten tests in the registry-judge section spelled "/tmp" as their policy's workspace root and the Windows leg refused every one of them inside SandboxPolicy.__post_init__ — os.path.isabs("/tmp") is False there, because a drive-less rooted spelling does not resolve under the cwd, so the subject never ran. The Linux leg passed, so nothing here saw it; run 39881732802 is the reading (10 failed, 5223 passed). The registry judges read no workspace at all — a key is granted as itself — so the value is irrelevant to what is measured, which is exactly why it must not be a literal. It is now os.path.abspath(os.sep), so the host's own os.path answers and no spelling is written down for a platform nobody measured. test_the_key_judges_never_spell_a_posix_workspace_root pins the region; it keeps its own section because written inside §5b it failed on itself — the spelling it forbids is quoted in the sentence forbidding it — and it reddens when a spelled root is put back, measured.
@argszero

Copy link
Copy Markdown
Owner Author

The test-windows leg was red, not pending — read to a cause and fixed in 2f7e7b6e (run 39881732802: 10 failed, 5223 passed).

Cause. All ten failures were mine, and none of them reached the subject: every one is ValueError: sandbox policy: workspace_root must be an absolute execution-world path (got '/tmp') raised in SandboxPolicy.__post_init__. The ten registry-judge tests spelled "/tmp" as their policy's workspace root, and os.path.isabs("/tmp") is False on Windows, because a drive-less rooted spelling does not resolve under the cwd. The Linux leg passed, which is why nothing here saw it before the push.

Fix. The judges read no workspace at all — a key is granted as itself — so the value is irrelevant to what those tests measure, and that is exactly why it must not be a literal. They now take os.path.abspath(os.sep): the host's own os.path answers, so it is / here and C:\ there, and no spelling is written down for a platform nobody measured.

Guard. test_the_key_judges_never_spell_a_posix_workspace_root reads the §5b region between its §5b and §5c markers and refuses a spelled POSIX root. It keeps its own section for a reason it measured first: written inside §5b it failed on itself, because the spelling it forbids is quoted in the sentence forbidding it. Armed — putting one _ws_policy("/tmp") back reddens it.

Master is merged in (#2510, aea5ed96) — the conflict was two pure additions to test_bash_v2_win32.py and both guards are kept. Merged tree: 5703 passed / 35 skipped. CI on this head: pending.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261023-094325

Read the head 2f7e7b6e itself rather than an earlier record of it, in a detached worktree.

  • Ancestry: git merge-base --is-ancestor aea5ed96 FETCH_HEAD holds, so master's current
    head is contained and this lands on the tree CI measured.
  • CI on this head: run 39883632292 — test pass in 3m28s, test-windows pass in 5m22s.
    Both legs, not a local-only reading.
  • The kind is stated, never inferred: grants.split_kind_flag is the one producer and
    KIND_FLAGS is a mapping, so the client cannot spell a kind the grant model lacks; the removal
    carries the kind through instead of re-reading every remaining grant off the disk.
  • An empty target is refused with a sentence: probed on this head — split_kind_flag("--registry")
    answers ("registry", ""), and canonical_key_spelling("") refuses with "a registry grant needs a
    key spelling, not an empty string", so /sandbox add --registry cannot store a grant with no target.
  • Both accepted root spellings converge: HKEY_CURRENT_USER\Software\LarkCli and
    HKCU\Software\LarkCli both canonicalize to the short form — which matters because the capability
    SID is derived from the spelling — and a whole-hive spelling (HKCU) is refused by name.
  • The refusal sentence is derived, not written: enforcement_refusal reads ENFORCED_KINDS to say
    where a kind is enforceable, so the clause that kept promising a future stage would add enforcement
    after #2498 had shipped it cannot come back.
  • The arms are named by tests: split_kind_flag, judge_registry_key and canonical_key_spelling
    each appear in tests/, so deleting any of them reddens rather than passing quietly.

One thing I checked and do not count against it: removal matches on the canonical target alone, not
kind-and-target. A key's canonical spelling always carries a hive root while a path grant's is an absolute
path, so the two cannot collide — worth the comment it already has, not a change here.

This is one vote of the three R3 asks for; I have not merged it.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261023-094800

Read head 2f7e7b6e: emrg/sandbox/roots.py (judge_registry_key, and the registry branch of
judge_root_removal), the three small hunks it needs elsewhere (client/app.py routing the
target through split_kind_flag, the /sandbox help line, and the daemon passing kind into
the removal judge), and tests/test_sandbox_extra_roots.py §5b/§5c.

The fix replaces a spelled "/tmp" workspace root with os.path.abspath(os.sep), so I armed
that derived root in both directions, in a detached worktree at this head, with the import
confirmed to be the worktree's own emrg:

  • relative root (_ANY_WS = "tmp"): 10 of the 11 selected nodes fail, and the reason is the
    policy's own — ValueError: sandbox policy: workspace_root must be an absolute execution-world path (got 'tmp') at emrg/sandbox/policy.py:103. The single survivor is
    test_a_typed_registry_kind_is_answered_without_a_stale_sentence, the one node in the set
    that never reads _ANY_WS. Every node that depends on the derived root therefore depends on
    it through the precondition the fix exists to clear, so these arms are not passing because
    the predicate went vacuous.
  • restored from a saved copy, not with git checkout --: sha256 2d201b82…5b675da9c
    identical to the original, and 11 passed again.

One observation, not a blocker, and it is the bound the guard's own docstring already states:
with _ANY_WS = "/tmp" put back, this file is green on this host — 11 passed — because the
definition line sits outside the §5b–§5c region the source-text assertion scans, and
os.path.isabs("/tmp") is True on POSIX. That regression is caught only by the Windows leg
(the isabs assertion, and the policy refusal that follows). Naming it so the bound stays a
stated one: the text half protects the region, the platform half protects the definition.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261023-095635

Read head 2f7e7b6e and then ran the one arm no earlier reading of this head covered: the Windows
half
, which is where this PR's first CI run died and where the fix actually has to hold.

Which sha each run answered about. The red run (39881732802) ran e35f65f2 — the predecessor —
and failed test-windows only (test succeeded), with 20 rows of the same sentence:
sandbox policy: workspace_root must be an absolute execution-world path (got '/tmp'). The green run
(39883632292) ran 2f7e7b6e, this head, with both legs success (test 3m28s, test-windows 5m22s).
So the pass is this head's own, not an inherited one.

The guard really ran on Windows, and was not skipped. The Windows leg's log carries the node twice
(collected, then):

tests/test_sandbox_extra_roots.py::test_the_key_judges_never_spell_a_posix_workspace_root
[gw0] [ 37%] PASSED tests/test_sandbox_extra_roots.py::test_the_key_judges_never_spell_a_posix_workspace_root

and the file's only skipif is test_bwrap_really_binds_a_named_file_writable (Linux bwrap), so §5c's
guard is platform-ungated and passed on the platform it exists for.

Why the derived root is the only spelling that can hold on both. Evaluated with ntpath — the very
module os.path is on Windows — the predicate the policy applies answers:

  • ntpath.isabs("/tmp") → False (drive-less rooted spelling: refused before the subject runs — the
    measured cause of the red leg), ntpath.isabs("tmp") → False, "" → False;
  • ntpath.isabs("C:\\") → True, and os.path.abspath(os.sep) on Windows is exactly a drive root
    (os.sep is \), while on POSIX it is /. One expression, absolute on both, no literal written down
    for a platform nobody measured.

I did try to simulate abspath here as well and it does not transfer: ntpath.abspath("\\") on this host
answers "\\" because ntpath calls the POSIX getcwd(). That reading is this host's artefact, not a
Windows fact, so the claim above rests on isabs plus the red leg's own failure text and the green leg's
own log — not on it.

The §5c bound stated in the docstring holds and I confirmed it is stated rather than hidden: with the
literal put back, the file is green on POSIX (the definition line sits outside the text-scanned
region and isabs("/tmp") is true there), and the Windows leg is the half that catches it. Naming it
here so the bound stays a known one; it does not weaken the fix.

@argszero
argszero merged commit 62d6ced into master Oct 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: a host cannot type a registry key — /sandbox add has no --registry flag and the writer refuses the kind Stage 2 made enforceable

1 participant