Skip to content

fix(cli): stop a stale admin.token bricking the CLI; be admin on the first run - #101

Closed
V3RON wants to merge 17 commits into
fix/0003-07-client-abortfrom
fix/0003-08-cli-credential
Closed

fix(cli): stop a stale admin.token bricking the CLI; be admin on the first run#101
V3RON wants to merge 17 commits into
fix/0003-07-client-abortfrom
fix/0003-08-cli-credential

Conversation

@V3RON

@V3RON V3RON commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Stacked on #100. Three blocking defects from the adversarial review.

A stale admin.token bricked every command, including agent-role ones

The daemon removes admin.token only on a graceful stop, so a kill -9 or a power loss leaves it behind. The next invocation read the stale secret, auto-launched a fresh daemon that minted a new one, and hello failed ADMIN_AUTHENTICATION_FAILED — the daemon closed the socket, and nothing in the CLI handled that code. Every subsequent command died, including plain simlock lease, which needs no admin at all. The user was locked out until they deleted the file by hand.

ADR §5 designs for graceful degradation — "falling back to an agent session with a stderr notice" — but that fallback existed only for "file absent", never "file present but wrong". Now an ADMIN_AUTHENTICATION_FAILED retries once with no credential and emits the notice, naming admin.token specifically when that was the rejected source.

The CLI was never admin on a cold start

The credential was resolved before connecting, and the daemon is auto-launched inside connect. So on a fresh machine, or after simlock daemon stop, the file could not exist yet — the 3×50 ms retry was racing a daemon that had not been spawned. Every admin command (nuke, list, cleanup, config get, events, token *, doctor --fix, release --all) got FORBIDDEN on its first invocation. ADR §5's headline — "the CLI connects as admin whenever the local file is readable" — held only from the second run onward.

Credential resolution now happens after the socket is reachable: the environment takes a resolveCredential thunk, the raw connection (including any auto-launch) is established first, and only then is the file read. The brief retry now races only the narrow socket-claim-to-persist window that §5 actually describes.

config set accepted any mistyped key

validateConfigLayer warns and drops unknown keys rather than throwing, and no warn collector was passed — so simlock config set lease.heldTtlBackstop 60000 (missing Ms) validated clean, wrote the file, printed "Updated …", and the daemon ignored the key forever. ADR §11 requires validation through the config loader before writing; the mechanism was present but toothless. It now collects warnings and rejects on any unknown key.

The validation pass also moves off the sentinel path inside the real data directory and onto an in-memory filesystem, so a stray file there can no longer cause a false pass or fail — and the "never read" comment is now true.

Both existing config set tests injected a fake validator, which is why this survived; they now exercise the real one. New coverage includes a cold-start test against a real in-memory daemon with a genuine credential handshake, asserting admin on the very first invocation.

Note for reviewers

The stale-token retry degrades to agent uniformly, including for a mistyped --token. A user-supplied credential that is wrong could arguably deserve a hard failure rather than a quiet downgrade; the notice is worded differently for that case, but the behaviour is the same. Worth a second opinion.