The LLP 0063 D4 exclusivity gate fails closed on an unreadable central layer (#623)

[philcunliffe]

Root cause

The D4 exclusivity gate ("one org enrollment per machine", LLP 0063 D4) decides a
permission from an input it could not tell apart from its own absence.

1. src/core/runtime/boot.js resolveLayeredConfigFromDisk maps an unparseable
   central layer to null without throwing:
   const centralConfig = centralLoaded?.ok ? centralLoaded.config : null. A
   parse failure and "no central layer at all" become the same value.
2. src/core/remote/gateway_seed.js readCentralSinkOrigins consumed that
   directly (centralConfig?.sinks ?? {}), so a corrupt layer returned [].
3. src/core/cli/remote_commands.js gated on
   if (!alreadyEnrolled && connectedOrigins.length > 0). With [] the
   rejection branch is skipped and the login proceeds.

Verified against origin/master by test, not by reading: with a central seed on
disk containing { "version": 2, "sinks": {, hyp remote login other returned 0
and the browser flow was invoked.

The fail-open direction

A machine in this state is enrolled by every other definition the codebase uses:
hyp leave and resolveCentralLayerPath key on the central layer file, not on
whether it parses. The gate alone read it as "not enrolled" and permitted a second
org's enrollment, which is precisely what D4 exists to prevent. A gate that cannot
read its own input must refuse, not permit.

The fix

Surface the third state rather than collapsing it into an empty list.

• resolveLayeredConfigFromDisk now returns centralLoaded raw alongside the
  localLoaded it already returned. Boot itself keeps collapsing the two states,
  correctly: either way there is nothing to merge. Only a caller deciding a
  permission needs them apart.
• New readCentralEnrollment returns { origins, unreadable }
  (CentralEnrollment in src/core/remote/types.d.ts). A config_missing load
  failure is deliberately not unreadable: the layer path can come from an
  active-slot pointer naming a since-removed file, and "the file is gone" is the
  absent case, not the ambiguous one.
• readCentralSinkOrigins survives as that reduced to its origin list, for the
  one caller that is not making a permission decision and has its own documented
  answer for an unreadable layer (evaluateCwdClassification, deliberately inert
  on anything it cannot read: LLP 0106 #interactive). Unchanged behaviour there.
• Both halves of the D4 gate refuse on unreadable: the pre-auth check in
  runBrowserLogin (exit 2, before any auth) and the seed-time recheck in
  enrollCentralSink, which D4 specifies as part of the same gate. Leaving the
  recheck fail-open would have made its @ref LLP 0063#d4 dishonest; nothing is
  written at that point, so it throws and the caller's existing "signed in, but
  enrollment failed" path reports it.

The rejection names the real problem:

hyp remote login: this machine's central config layer (<path>) cannot be read, so its enrollment cannot be verified
  config is not valid JSON: ...
  repair that file, or disconnect this machine ('hyp leave'), then log in again

A same-origin re-login is refused too: with no parse there is no origin to compare
against. The advice stays actionable because hyp leave clears the layer by path,
not by contents.

Regression test

test/core/remote-login-command.test.js, four tests, all under LLP 0063 D4:

• an unreadable central layer fails the D4 gate CLOSED: login to a different server is rejected - the failing-then-passing one. On origin/master it fails
  (code 0, the browser flow ran, no rejection); with the fix it returns 2, the
  flow is never invoked, and the message names the unreadable layer rather than
  "this machine is connected to".
• an unreadable central layer also refuses a same-origin re-login - also
  fails on master, passes after.
• an ABSENT central layer is not an enrollment and still permits login -
  passes before and after; proves the fail-closed branch does not overshoot.
• a PARSEABLE central layer keeps its D4 behavior - passes before and after;
  same-origin re-login allowed, different origin rejected with the existing
  message.

Run on origin/master with only the tests applied: 2 failed, 2 passed. After the
fix: 65/65 in that file.

status.js neighbour: out of scope

src/core/daemon/status.js:266-268 collapses the same two states for hasCentral.
Left alone deliberately. Every consumer (buildClientActionsReport, the layered
report block, the wizard's managed hint) is display and reporting; none gates an
action. Changing what hyp status shows for a corrupt layer is a reporting design
question with its own message and its own tests, not part of restoring this
permission decision. Worth its own issue.

Checks

• npm test: 3381 tests, 3379 pass, 1 skipped, 1 fail. The single failure,
  a corrupt marker fails open ... LLP 0101 fail-open polarity, is environmental
  (a daemon_loaded_no_pid systemd diagnostic from the host) and reproduces
  identically on origin/master with this branch stashed.
• npm run typecheck: clean. test/core/classify-inactive-state.test.js needed
  centralLoaded: null in its hand-rolled resolveLayeredConfigFromDisk fixture.

LLP

No LLP edit. LLP 0063 D4 is Active and this changes nothing it settled: D4 states
the gate is total and re-checked at seed time, and never contemplated a layer it
could not read. Failing closed is that decision implemented correctly, not a new
one. The @ref LLP 0063#d4 annotations moved with the code they annotate and
still hold.

Fixes #623

[philcunliffe]

Neutral review, round 1 of 2

Verdict: approve after one fix, which I landed on the branch. The hole is closed and the fix does not overshoot. I verified both empirically rather than by reading, by driving runRemoteLogin against eleven hand-built central-layer states. One residual instance of the same fail-open survived the fix (the config_missing carve-out); it is fixed in a77efde + a814be6 and re-verified.

1. Is the hole closed? Yes, and pre-auth

Probed on 8f28f33 with the real runRemoteLogin and a login stub that records whether it was ever entered:

central layer state	exit	auth invoked
corrupt JSON seed	2	no	closed, pre-auth
empty file	2	no	closed
directory where a file is expected	2	no	closed
chmod 000 (EACCES)	2	no	closed
valid JSON, invalid shape (version: 1)	2	no	closed
active slot -> corrupt config.a.json	2	no	closed

loadConfigFile maps every non-ENOENT readFile throw to config_unreadable, so EACCES and EISDIR both land in unreadable; an empty file is config_invalid_json. None of them read as absent. The rejection returns before login() is entered in every case (authCalled=false), so no auth is spent.

2. Does it overshoot? No

• Absent layer (no seed, no pointer): exit 0, login proceeds. resolveCentralLayerPath returns null, so centralLoaded is null and unreadable is null.
• Parseable layer, same origin: exit 0, re-login proceeds, no "cannot be read".
• Parseable layer, different origin: exit 2 with the pre-existing this machine is connected to <A> message, unchanged.
• Active slot -> good config.a.json, different origin: exit 2 with the existing message. The applied-slot route to the gate still works.

3. Finding (Medium): config_missing was not the absent case, and it kept a live fail-open

src/core/remote/gateway_seed.js:125 (pre-fix) carved config_missing out of unreadable, justified as "the layer path can come from an active-slot pointer naming a since-removed file, so that is the absent case".

Probed: an active-slot pointer (config-control/active -> config.a.json) with the slot file removed permitted hyp remote login to a different origin on 8f28f33 (exit 0, browser flow invoked). Same shape as #623, one step further in.

The justification is backwards, and the surrounding code says so:

• resolveCentralLayerPath (src/core/config/apply.js:91) returns null for a machine with no layer, and that null never reaches a load. "Absent" is already fully expressed by the path not resolving.
• The seed branch existsSync-checks before returning the path (apply.js:96), so an ENOENT there is only a live race with a concurrent removal.
• The slot branch resolves a pointer that commit only ever flips after fs.writeFileSync(slotPath(target), ...) (apply.js:630-645), and recoverBadActiveEtag / resetCentralLayerToSeed remove the pointer, never the file alone. So the app never produces pointer-without-file: reaching that state means an applied-to (i.e. enrolled) machine whose layer was removed out of band.
• hyp leave calls exactly that machine connected: its "nothing to do" test is centralLayerPath === null (src/core/commands/central.js:371), and it tears the machine down. The PR's own principle ("hyp leave and the apply engine key on the file") therefore argues against the carve-out.

Fixed in a77efde: unreadable is now every centralLoaded.ok === false, with the doc rewritten to state the rule as "absent is resolveCentralLayerPath returning null, and only that". a814be6 brings the CentralEnrollment interface doc in line. src/core/cli/remote_commands.js:586 comment updated to match.

Lockout risk checked, not assumed: I ran runLeave against the stale-pointer state. It reports leaving the central server / removed the central config layer, clears the pointer, exits 0, and a subsequent login is then permitted. The message's advice clears the state it names.

New regression test: an active-slot pointer naming a file that is gone is unreadable, not absent (LLP 0063 D4). Post-fix, all eleven probed states behave: only genuinely-absent and parseable-same-origin permit.

4. Seed-time recheck: safe, verified

Called enrollCentralSink directly against an unreadable layer with a directory snapshot around it. It throws before touching disk (config-control|config-control/seed.json, identical before and after), and remote_commands.js:722 catches it into signed in, but enrollment failed: <message> returning 1. The author's claim holds.

5. Test strength: independently confirmed

Copied the PR's test/core/remote-login-command.test.js onto a clean origin/master worktree (fb60a9f) and ran it: 63 pass, 2 fail, and the two failures are exactly the two claimed (fails the D4 gate CLOSED and also refuses a same-origin re-login). The absent-layer and parseable-layer tests pass on master, which is what makes them useful as overshoot guards.

6. @ref and LLP

@ref LLP 0063#d4 at gateway_seed.js:130 moved with the function it annotates and still describes it; remote_commands.js:583 likewise. Both anchors resolve (<a id="d4"> in llp/0063-login-enrollment.decision.md), as do #d2, #d3, #d5, #login-config-pull, #prerequisites, #connection-levels. The prose reference to LLP 0106 #interactive resolves too ({#interactive}, line 43), and evaluateCwdClassification is genuinely documented inert on an unreadable layer, with behaviour unchanged via the origins-only readCentralSinkOrigins (now its only caller).

No LLP edit needed: agreed. D4 says the gate is total ("at most one server per machine") and is re-checked at seed time. Refusing a state the gate cannot evaluate is that decision implemented faithfully, not a new one, so under the repo's "Accepted docs are settled" rule this is correctly a code change. Worth flagging that D4's text does describe same-origin re-login as falling through to idempotent re-seeding, which the fix now refuses when the layer is unreadable; that reads as a defensible answer to an unforeseen state rather than a contradiction, and the code comment says why.

Style

No semicolons, no U+2014 anywhere in the diff. Type-import specifiers are root-anchored .js ('../../../src/core/config/types.js', '../../../src/core/remote/types.js'), the shared type is an interface in a .d.ts, no @typedef, no inline import('...').

Checks

• npm test on the branch with my fixes: 3382 tests, 3380 pass, 1 skipped, 1 fail. The single failure is a corrupt marker fails open (absent, no diagnostic, no degrade) - LLP 0101 fail-open polarity, which fails identically on a clean origin/master worktree in this sandbox (3377 tests, 3375 pass, 1 skipped, 1 fail, same test). Environmental, not this PR.
• npm run typecheck: clean.

Residual findings

• (Low, out of scope, agreed) src/core/daemon/status.js:266-268 still collapses "no central layer" and "unparseable central layer" into hasCentral: false. Every consumer is display, not a gate, so this is a reporting question and deserves its own issue. The PR body already says so.
• (Low, pre-existing) The seed-time refusal throws after markFirstSyncHoldBestEffort has written the first-sync hold marker (src/core/cli/remote_commands.js:715), so an aborted enrollment leaves a hold marker until its bounded deadline. This is true of every existing throw out of enrollCentralSink (identity-seed failure, rollback), it is bounded by design (LLP 0101 no-release), and a non-enrolled machine has no forward sink to hold. Not introduced here; noted only so it is not rediscovered as new.
• (Informational) The D4 gate is a hyp remote login gate only; hyp join keeps its documented overwrite semantics (D4: "the guard is for the attended accidental case"). Unchanged by this PR and correct per the LLP, but worth knowing the exclusivity property is not machine-wide against an operator-driven join.

[philcunliffe]

Round 2 (of 2), security-relevant: one High finding, fixed in 70b9cd1

Round 1 closed the config_missing carve-out by widening "unreadable" to every
centralLoaded.ok === false. That is the right condition for loading. But the
fix still keys a permission on a named state as a proxy for a property, one level
up: resolveCentralLayerPath() === null is the stand-in for "not enrolled",
and resolution is lossy in exactly the way the loader was.

readActiveSlot (src/core/config/apply.js:701) swallows every readlink
error into null, and resolveCentralLayerPath's seed branch uses existsSync,
which reports EACCES as "not there". So a machine whose enrollment is fully
intact on disk resolves to null, and the gate reads null as a free machine.

High - the D4 gate still failed OPEN on a central layer whose path would not resolve

src/core/remote/gateway_seed.js:132 (readCentralEnrollment) via
src/core/config/apply.js:91 (resolveCentralLayerPath).

Probed, not reasoned: the real runRemoteLogin driven against 16 constructed
states with an auth stub recording whether auth was entered. Target is a
different origin than the one on disk.

state	before this round	after
C1 no central layer at all	permitted (correct)	permitted
C2 parseable seed -> org A	refused	refused
C3 corrupt seed (invalid JSON)	refused	refused
C4 pointer -> missing slot file	refused	refused
E1 pointer replaced by a regular file (readlink EINVAL); config.a.json still names org A	PERMITTED, auth entered	refused (2)
E2 pointer -> config.c.json (neither slot); config.a.json still names org A	PERMITTED, auth entered	refused (2)
E3 pointer is a symlink loop (active -> active); config.a.json still names org A	PERMITTED, auth entered	refused (2)
E4 pointer is an absolute symlink to slot a	refused (correct)	refused
E5 config-control/ EACCES, seed inside names org A	PERMITTED, auth entered	refused (2)
E6 config-control/ EACCES, pointer + slot a name org A	PERMITTED, auth entered	refused (2)
E7 state dir hypaware/ EACCES, seed inside names org A	PERMITTED, auth entered	refused (2)
E8 seed.json EACCES, dir readable	refused	refused
E9 seed.json is a directory (EISDIR)	refused	refused
E10 slot a is a dangling symlink	refused	refused
E11 slot a is a symlink loop (ELOOP)	refused	refused
E12 seed parses but is shape-invalid	refused	refused

Six live fail-opens. In every one the enrollment is completely intact: the slot
file or seed still holds org A's @hypaware/central sink verbatim, and a chmod
or a repaired symlink brings it straight back. That is a strictly worse state than
the config_missing one round 1 closed, where the layer's contents were at least
gone.

The fix (70b9cd1)

centralLayerResolutionFailure({ stateRoot }), new in
src/core/config/apply.js:133, re-checks a null path against the control
directory itself, reading no file contents:

• the directory cannot be listed (anything but ENOENT/ENOTDIR) -> cannot tell;
• the directory still holds active, seed.json, config.a.json or
  config.b.json that the resolution did not manage to name -> cannot tell;
• otherwise the absence is verified -> genuinely not enrolled.

resolveCentralLayerPath is untouched, so boot and hyp status keep the one
null they are right to see. Only the permission decision asks the second
question, exactly as round 1 made only it ask about load failure.
readCentralEnrollment (src/core/remote/gateway_seed.js:142) folds it into
unreadable, so both halves of the D4 gate (pre-auth, and the
enrollCentralSink seed-time recheck) pick it up together.

No overshoot, probed: an empty control directory (what hyp leave and
resetCentralLayerToSeed leave behind), a directory holding only state.json
and an orphan config.a.etag, a state dir with no config-control/, and
config-control being a file (ENOTDIR) all still permit login. Only
layer-bearing names count as evidence.

Medium (introduced by the fix above, fixed in the same commit) - hyp leave was the same dead end

hyp leave keyed on the same lossy resolution
(src/core/commands/central.js:363), so every state the widened gate refuses on
was one leave called "not connected to a central server - nothing to do" -
returning 0 while config.a.json and the damaged pointer stayed on disk. The
gate's own advice would have been a permanent lockout. Verified by probe, then
fixed:

• leave now proceeds when centralLayerResolutionFailure reports residue. The
  teardown was already the right one: resetCentralLayerToSeed force-removes the
  pointer and both slots by name, no resolution required.
• its central-layer step is counted-and-reported like every other step rather than
  an uncaught EACCES that aborted before the attach reversal in step 3.

Escape verified end-to-end for every fail-closed state - leave exits 0, leaves
config-control/ empty, and a subsequent login to the other org is permitted:
damaged pointer (3 shapes), corrupt seed, pointer-without-slot. For an
EACCES control directory leave now exits 1 with
✗ could not remove the central config layer: EACCES ... and later steps still
run; login stays refused, correctly - the user owns the directory, chmod is the
real repair, and the gate's message names the exact path and errno.

Re-assessed from round 1

• enrollCentralSink seed-time recheck (src/core/commands/central.js:208):
  correct after the widening. It runs before any write, and a fresh enrolling
  login sees no control directory (ENOENT -> absent), so it does not refuse
  itself. Its comment is updated to stay honest about what it now covers.
• evaluateCwdClassification (src/core/usage-policy/classification.js:190):
  unchanged. It still calls readCentralSinkOrigins, which discards unreadable;
  deliberately inert on anything it cannot read (LLP 0106 #interactive).
• hyp join overwrite semantics and the first-sync hold marker ordering:
  unchanged, out of scope, as round 1 judged.

Checks

• npm test on this head: 3387 tests, 3385 pass, 1 skipped, 1 fail.
  The single failure is a corrupt marker fails open ... LLP 0101 fail-open polarity (test/core/status-first-sync-hold.test.js:99). It is not this
  PR's, and it is not permanently environmental either: this branch is based on
  fb60a9f, and master has since landed db33f04 ("npm test no longer drives
  the host's real service manager", npm test kills the real daemon on macOS: attach tests reach the real launchctl through the label namespace #602/npm test no longer drives the host's real service manager (#602) #606), which fixes exactly it. Verified
  both ways - a clean origin/master worktree passes that file, and
  origin/master merged into this head runs 3418 tests, 3417 pass, 1 skipped,
  0 fail with no conflicts. The branch just needs master merged in before
  landing.
• npm run typecheck: clean.
• Regression tests added: 5 in test/core/remote-login-command.test.js (three
  pointer shapes, the unlistable directory, and the no-overshoot cases) and 2 in
  test/core/leave-command.test.js (teardown through an unresolvable pointer,
  and the reported-not-thrown EACCES). The login ones fail on a814be6 and pass
  on 70b9cd1.

Residual findings

1. Low, display-only. src/core/daemon/status.js:265-268 now collapses
   three states into hasCentral: false - no layer, a layer that will not
   load, and a layer whose path will not resolve. A user hitting the gate's
   refusal sees hyp status report no central layer, which contradicts it.
   Every consumer is reporting, not gating, so it is still out of scope for a
   permission fix, but it is a more visible inconsistency than round 1 judged.
   Worth its own issue.
2. Low, unchanged from round 1. The seed-time refusal in enrollCentralSink
   throws after the first-sync hold marker is written. Pre-existing, bounded by
   LLP 0101.
3. Informational. readActiveSlot and resolveCentralLayerPath stay lossy by
   design; centralLayerResolutionFailure is the compensating read, and the D4
   gate is its only caller. Any future permission decision keyed on
   resolveCentralLayerPath() === null reopens this hole. The new function's doc
   comment says so.
4. Stale base, above: merge origin/master before landing, or the one red
   test travels with the PR.

[philcunliffe]

Neutral triage (LLP 0017) on this PR at head 70b9cd1a02e052c8ffb58384132e5750d4357ff1, after the review fix-loop hit its 2-round bound.

Divergent-edge check on round 2's fix. The task for this triage pass was specifically to check whether centralLayerResolutionFailure() (src/core/config/apply.js:133) has the same recurring divergent-edge defect its own predecessors had. I drove the real runRemoteLogin (with a stubbed auth callback, matching the existing test harness) across several additional states beyond the 16 round 2 already probed: active replaced by a directory, active a symlink escaping the control directory via an absolute path, a control directory holding only unrelated files (client-actions.json, junk), the state root itself unreadable with a real enrollment present, and the state root unreadable with no control directory at all. All refused correctly (fail-closed) except the "only unrelated files" case, which correctly permitted login (matching the existing "does not overshoot" test) since no CENTRAL_LAYER_BASENAMES file is present. I could not construct a state where the machine is genuinely enrolled but none of active/seed.json/config.a.json/config.b.json are present in the control directory (the write path only ever creates enrollment evidence under those exact names, and resetCentralLayerToSeed's teardown loop leaves whatever it didn't reach, which the residue check re-derives fresh from a live readdirSync rather than trusting any prior step's success). I found no further fail-open.

Residual findings, classified:

1. src/core/daemon/status.js:265-268 collapses "no central layer" / "unreadable" / "unresolvable" into one hasCentral: false, which now visibly disagrees with the D4 gate's refusal message. Display-only, no permission decision reads it. Non-blocking.
2. enrollCentralSink's seed-time recheck (src/core/commands/central.js:209-212) can throw after the first-sync hold marker (LLP 0101) is already written. Pre-existing (true of every throw in that function), and bounded by LLP 0101's own design (best-effort, absolute deadline, delays exports rather than leaking). Non-blocking.
3. The new helper's JSDoc already warns that any future permission check keyed on resolveCentralLayerPath() === null reopens this hole. Informational, already documented in-code. Non-blocking.
4. Stale base: this branch is based on a master older than npm test no longer drives the host's real service manager (#602) #606 (the systemd-diagnostic test fix), which is the one local test failure (npm test: 3387 tests, 3385 pass, 1 fail, 1 skipped on this head alone). I independently merged current master into this head in a scratch worktree: the merge is conflict-free and the merged tree is green — 3418 tests, 3417 pass, 0 fail, 1 skipped — confirming this is the known stale-base artifact, not a regression. GitHub's own mergeable field already reports MERGEABLE.

All four are non-blocking; none is a production defect, security hole, or lockout with no recovery. Filed the deferrable ones (findings 1-3) as #628 for follow-up; nothing here needs to hold this PR further.

[philcunliffe]

Re-triaged at 3a09236 - all residuals non-blocking, PR can ship

The head moved only because neutral merged current master in to resolve the conflict PR #629's merge created. The PR's own content is unchanged: git diff origin/master HEAD --stat is still 9 files, 468 insertions, 19 deletions, byte-identical to the pre-merge diffstat.

I re-derived the classification from the merged code rather than carrying the earlier verdict forward, and re-checked the two things the merge could have broken.

The fail-closed property survives. readCentralEnrollment (src/core/remote/gateway_seed.js:138-152) still computes unreadable from both centralLoaded.ok === false and centralLayerResolutionFailure(...) when centralLoaded === null; the pre-auth gate (src/core/cli/remote_commands.js:601-606) still returns 2 before any auth call; and the seed-time recheck (src/core/commands/central.js:219-224) still throws before any write. #629 only inserted its client-sync stamp after that gate, never before it.

#629's new wizard join lane does not bypass the gate. wizard/join.js's defaultRunLogin delegates to runRemoteLogin, so it inherits the gate. The only production caller of enrollCentralSink is inside runBrowserLogin, strictly after the gate passes.

Residuals, re-verified at this head:

1. src/core/daemon/status.js:274-275 - hasCentral still collapses "no layer", "unreadable layer", and "unresolvable path" into one false. Preference: display and reporting only, not a gate; every consumer is informational.
2. src/core/commands/central.js - the seed-time D4 recheck throws after the hold marker is written. Preference: pre-existing rather than introduced here, bounded by LLP 0101's absolute deadline, no data leak or loss.
3. resolveCentralLayerPath/readActiveSlot stay deliberately lossy, so a future gate keyed on === null would reopen the hole. Informational, already documented in centralLayerResolutionFailure's own JSDoc.

All three remain tracked in #628, which covers them accurately. Nothing new surfaced.