fix(install): make the installer's account of itself true (0.8.2, PR-1)#86
Merged
Conversation
A first-time install of 0.8.1 by someone with no context, plus a read-only audit, found the same class of defect throughout: the script asserts things it does not do. None of it broke an install; all of it misinformed the person running one. Detection is now two states, keyed on whether cosign RUNS. `command -v` answers a different question and answers it differently per shell: for a present-but-non-executable file /bin/sh says no while bash says yes. Since we ship as `curl … | sh`, keying on it meant the most common breakage — a download that lost its +x bit — printed "cosign is not installed" about a file sitting in the user's PATH, and the branch written for that case was unreachable on the delivery path. Nor is there a code that separates "missing" from "broken": an executable the kernel refuses exits 2, 126 or 127 depending on its bytes (measured: garbage 2, truncated ELF 126, near-text 127). Both states lead to the same action, so the script stops pretending to tell them apart and names both possibilities in one message. Three filesystem writes were unguarded, unlike every network call around them: - an unwritable profile killed the run AFTER the shim was fetched, made executable and installed — reporting a failed install that had succeeded. It is now non-fatal: the shim stays, the manual PATH line is printed, exit 0. - a failed chmod left ~/.local/bin/rustok.rustok-tmp behind, the exact leftover the temp-file-then-atomic-mv dance exists to prevent. It now cleans up. - a failed mkdir surfaced as a raw shell error after the image had already been pulled. It now fails in the installer's own voice. Success is now reported from the outcome, not from the write's status: a failed redirection on a compound command does not reliably return non-zero, and the first version of this fix cheerfully printed "added … to PATH" over a file the shell had just refused to open. The script reads the file back instead. Also true now where it was not: - the PATH block is gated on the profile's contents, not on this session's PATH (a directory exported by a parent shell is gone in the next terminal, and skipping the edit for that user would cost them `rustok` everywhere); - fish and csh get their own syntax and an untouched config, instead of a ~/.profile they never read plus "open a new shell", which was simply false; - replacing an existing `rustok` is announced; - the header's example URL uses the tag namespace that exists (`wallet-tui-vX.Y.Z` resolves; the bare `vX.Y.Z` it showed 404s); - the refusal links an absolute URL — over a pipe there is no local docs/ tree; - an unset HOME fails in our voice, not the shell's. Tests: ten new cases, every one proven red first, all under `sh` — the delivery path, not the shell that happened to make the old bug visible. Mutation controls included: restoring the three-state detector with separate messages turns the sh-unrunnable case red; breaking the profile-content gate, the replacement notice or the mkdir wording each turns its own case red. Filesystem failures are now injectable at all, which they were not — the reason three unguarded writes shipped through a green suite. Spec: .claude/specs/2026-07-22-0.8.2-honesty-batch.md. Version bump and the shim fixes land in PR-2; the RPC-in-`podman inspect` leak is text-only in 0.8.2 by ratification and recorded there as deferred.
temrjan
added a commit
that referenced
this pull request
Jul 22, 2026
> Stacked on #86 — base retargets to `main` once that merges. ## The one that matters: the purge gate ran after the teardown `uninstall --purge-keys` through a pipe or an agent **refused correctly — after** deregistering the clients, stopping the wallets, deleting the secrets, removing the PATH block, deleting the shim itself and wiping the config dir. The keys survived. The installation did not, and the user was left without the command that manages them. A gate that fires after the damage is not a gate. The tty gate and the volume listing now run **first**, before a single mutation. Deletion itself stays last — the data-safe order is unchanged, only the gate moved. Acceptance is the tester's `probe-03-purgekeys-order.sh`: **4/4**, was 1/4. ## Teardown no longer reports work it failed to do | before | now | |---|---| | `stop … \|\| true` + unconditional "stopped N running wallet(s)" | counts what actually stopped, names what did not — and a wallet still running is exactly what makes a later volume removal fail | | `volume rm … && echo` — a refusal printed **nothing** and exited **0** | each surviving volume named, command fails | | docker password files claimed removal whether any existed or the removal worked | reports what actually happened | ## The RPC guarantee is stated as it is `--help` promised a keyed URL *"stays out of argv, agent configs and `podman inspect`"*. That holds **only after `connect`** has stored it as a secret. Before that — and on every docker run — the value is forwarded by name and the engine records it in container metadata, readable by anything running as the user. All three cases are now spelled out, and `start` **warns at the moment it takes the exposed road**. Closing the leak in code is deferred to the next release by ratification; the text stops overclaiming today.⚠️ Landmine worth a reviewer's eye: `wallet_run_args` emits argv on **stdout**, so the warning had to go to stderr — a `say` there would have become an argument to `podman run`. There is a test asserting both the warning text and that argv still carries only `-e NAME`. ## Quoting, at the point the words are built The password mount was a word-split fragment carrying a real filesystem path. A `$HOME` or `$XDG_CONFIG_HOME` containing a space tore it into three argv tokens — corrupting `init`, `start`, and **permanently freezing a broken argv into the client's MCP config** on `connect`. The helper is gone; the three call sites append quoted words. The **101 byte-exact argv tests passed unchanged** through this refactor, which is what made it safe to do at all. ## Diagnostics stopped writing `doctor` and `status` created the engine config as a side effect of being run. They no longer persist anything, and `doctor` no longer calls the engine "pinned" when nothing has pinned it. Both now distinguish *"no wallet has ever been created"* from *"one exists but is not running"* and point a fresh install at `rustok init` — previously six green `ok:` lines told a new user nothing about the next step. `--help` also stops advertising a "leftovers" sweep; what doctor really checks is stray containers, so that is what it says. ## `update` says what it is not doing One stderr line at pull time: it pulls by **mutable tag** and does **not** re-verify the signature — the installer is the verified path. Same "tell the truth at the moment the guarantee drops" pattern as the start warning. Deep verify-on-update stays in the backlog. ## Versions 0.8.2 across the six points, plus the `SKILL.md` body tags and the install-URL tag. The shim's own axis goes **0.3.0 → 0.3.1** — its commands changed behaviour, and that axis has no guard test, so it is called out explicitly. ## Tests 13 new cases, **each proven red first**. Two pre-existing tests were updated because the contract deliberately changed, and I am flagging that rather than burying it: - engine pinning is asserted through a **state-changing** command instead of `status` (diagnostics no longer pin); - the empty-machine `status` text is the new wording. Also fixed two of my own test defects mid-flight: an assertion that checked for a prefix already present in earlier output (it could not fail), and a `--help` grep that missed the overclaim because the promise is split across a line break. ## Gates All nine locally: ruff · mypy · pytest 188 · shellcheck ×2 · shim **114/114** · install **28/28** · `bash -n` + `sh -n` · hadolint. Deferred and recorded in the spec with test locations: the RPC leak in code, per-subcommand `--help` (12 commands, plus `connect` swallowing `--help` as an agent name), and an early exit for `update` with no registrations — that last one needs the client-discovery loop duplicated before the pull, which is surgery, not a tweak.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Where this came from
0.8.1 shipped, and then two things happened: a fresh session installed the wallet from the public page with no prior context (a real first-user probe), and a read-only audit went through
install.shlooking for anything that would force an 0.8.3.The install worked — one command, exit 0, no intervention, the cosign wall gone. What both found was a class of defect underneath: the script asserts things it does not do. Nothing here breaks an installation; all of it misinforms the person running one.
Detection: two states, keyed on whether cosign runs
The probe now decides, and
command -vis gone.It answers a different question — "is there a file by that name" — and answers it differently per shell: for a present-but-non-executable file
/bin/shsays no,bashsays yes. We ship ascurl … | sh, so keying on it meant the most common breakage (a download that lost its+x) printed "cosign is not installed" about a file sitting in the user'sPATH, and the branch written for exactly that case was unreachable on the delivery path. That is what the tester hit: a truncated 18 MB cosign, reported as absent.There is also no exit code that separates "missing" from "broken". Measured on executables the kernel refuses:
+xAny single-code rule misfiles one of them. Both states lead to the same action anyway — install by digest, provenance unchecked — so the script stops pretending to distinguish them and names both possibilities in one message.
works + verify failsstill refuses, fail-closed, untouched.Three unguarded writes
Every network call was checked; the filesystem writes were not.
chmod +x'd and installed — a failed install that had in fact succeeded. Now non-fatal: shim stays, manual PATH line printed, exit 0.chmodleft~/.local/bin/rustok.rustok-tmpbehind — the exact leftover the temp-file-then-atomic-mvpattern exists to prevent. Now cleaned up.mkdirsurfaced as a raw shell error after the image had been pulled. Now fails in the installer's own voice.if ! { …; } >>"$prof"does not reliably catch a failed redirection, so it printed "added … to PATH" over a file the shell had just refused to open. Success is now read back from the file's contents. The test caught it.Also true now where it was not
PATH— a directory exported by a parent shell is gone in the next terminal, and gating on it would cost that userrustokeverywhere.~/.profilethey never read plus "open a new shell", which was false for them.rustokis announced (the tester's stale symlink was overwritten silently).wallet-tui-vX.Y.Z→ 200, the barevX.Y.Zit showed → 404.docs/tree to open.HOMEfails in our voice, not the shell's.Tests
Ten new cases, every one proven red before the fix, and all under
sh— the delivery path, not the shell that happened to make the old bug visible. That distinction is the point: the previous suite tested the no-+xcase underbash, which is precisely why it encoded the defect as expected behaviour and stayed green.Filesystem failures are now injectable at all (
break_tool), which they were not — the reason three unguarded writes passed a green suite.Mutation controls, each verified to apply before being trusted:
Honest note: reintroducing
command -vin front of the probe leaves the suite green — under two states with one message it changes nothing observable. What guards the regression is the unified message, not the absence of that call.Gates
All nine locally: ruff · mypy · pytest 188 · shellcheck ×2 · shim 101/101 · install 28/28 ·
bash -n+sh -n· hadolint. shellcheck/hadolint containerised.Scope note: no version bump here — that lands with PR-2 so the six version points move once. The RPC-URL-in-
podman inspectleak is text-only in 0.8.2 by the Captain's ratification and is recorded as deferred in the spec, with its test location.