Skip to content

fix(shim): stop the shim claiming work it did not do (0.8.2, PR-2)#87

Merged
temrjan merged 1 commit into
mainfrom
fix/0.8.2-shim-honesty
Jul 22, 2026
Merged

fix(shim): stop the shim claiming work it did not do (0.8.2, PR-2)#87
temrjan merged 1 commit into
mainfrom
fix/0.8.2-shim-honesty

Conversation

@temrjan

@temrjan temrjan commented Jul 22, 2026

Copy link
Copy Markdown
Member

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.

@temrjan
temrjan force-pushed the fix/0.8.2-shim-honesty branch from 07af104 to 594ed7e Compare July 22, 2026 11:07
Base automatically changed from fix/0.8.2-installer-honesty to main July 22, 2026 11:08
The purge gate ran after the teardown it was guarding.
`uninstall --purge-keys` through a pipe or an agent refused correctly — but only
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. 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.

The tests that were supposed to catch this could not. They asserted exit 1, the
word "terminal" and intact volumes — all three of which were true while the gate
still sat at the end of the function. They now also assert that nothing happened
before the refusal: the shim file is still there, the PATH block is still in the
profile, and the log carries no deregistration, no stop and no secret removal.
Removing the early gate turns them red; before, it did not.

Teardown no longer reports work it failed to do:
- `stop … || true` followed by an unconditional "stopped N running wallet(s)"
  now counts what actually stopped and names what did not — and a wallet still
  running is exactly what makes a later volume removal fail;
- `volume rm … && echo` swallowed a refusal entirely: the one command whose job
  is to answer "are my keys gone?" printed nothing and exited 0. Each surviving
  volume is now named and the command fails;
- the docker password files claimed removal whether or not any existed, and
  whether or not the removal worked.

The RPC guarantee is now stated as it actually 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 spelled out, and
`start` warns at the moment it takes the exposed road. The warning branches by
engine: pointing a docker user at `rustok connect` would have been advice that
cannot work, since on docker `connect` freezes a literal -e rather than storing
a secret. Closing the leak in code is deferred to the next release by
ratification; the text stops overclaiming now.

The password mount is quoted at the point it is built. It was emitted as a
word-split fragment carrying a real filesystem path, so a $HOME or
$XDG_CONFIG_HOME with a space tore it into three argv tokens — corrupting
`init`, `start`, and permanently freezing a broken argv into the client config
on `connect`. The helper is gone; the three call sites append quoted words.

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 claims the engine is "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 what to do next.

`--help` also stops advertising a "leftovers" sweep; what doctor really checks
is stray containers, so that is what it says.

`update` now says, at pull time, that it pulls by mutable tag and does not
re-verify the signature — the same "tell the truth at the moment the guarantee
drops" pattern as the start warning.

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 because its commands
changed behaviour.

Tests: 15 new or strengthened cases, each proven red first. Two pre-existing
tests were updated because the contract deliberately changed — engine pinning is
now asserted through a state-changing command rather than `status`, and the
empty-machine status text is the new wording. The 101 byte-exact argv tests
passed unchanged through the password-args refactor, which is what made that
change safe to do.
@temrjan
temrjan force-pushed the fix/0.8.2-shim-honesty branch from 594ed7e to b13177c Compare July 22, 2026 11:09
@temrjan
temrjan merged commit 11275b2 into main Jul 22, 2026
9 checks passed
@temrjan
temrjan deleted the fix/0.8.2-shim-honesty branch July 22, 2026 11:10
temrjan added a commit that referenced this pull request Jul 22, 2026
## What

The 0.8.2 publish ran
([29914731366](https://github.com/rustok-org/mcp/actions/runs/29914731366))
— the guard printed `v0.8.2 is unpublished — proceeding`, the image was
built, signed and self-verified. The two fail-closed placeholders become
real:

| pin | value |
|---|---|
| `WALLET_DIGEST` |
`sha256:6090b0ca16534f8fea4f6e3f89202ef754575befb386d365cd9c8bafcc5897d0`
|
| `SHIM_COMMIT` | `11275b299d1f351f704433a30382030d9bfd232d` — merge
commit of PR-2 (#87), carrying the 0.3.1 shim |

Nothing else changes. **The `wallet-tui-v0.8.2` tag is cut from this
commit.**

## Proof, not assertion

**The pins resolve** — a well-formed pin that 404s is exactly what this
step exists to prevent:
- the raw URL at `11275b2` returns 58 608 bytes, **byte-identical** to
`cli/rustok` on `main`;
- `podman pull …@sha256:6090b0ca…` succeeds.

**Verified from outside CI**, not only by the workflow's self-check:

```
cosign v3.0.3 verify …@sha256:6090b0ca… \
  --certificate-identity …/wallet-publish.yml@refs/heads/main   → EXIT 0
```

Bite control: the same command with the identity pinned to a **tag** is
refused → the check is not vacuous.

**Gates**: pytest 188 · install 28/28 · shim 114/114 · shellcheck ·
`bash -n`.

## Reminder for the release step

Unchanged from 0.8.1 and still true: **cosign 2.x cannot see these
signatures** (stored as OCI referrers). Users on 2.x reach the
fail-closed branch and are told both ways forward — upgrade to cosign
3+, or install without cosign. Worth repeating in the release notes.
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.

1 participant