kayg/infraternal

Watch

Fork 0

Trust host key: a repeated click must not end in an error (J419, follows #439) #449

New issue

Open

opened 2026-11-09 08:14:08 +00:00 by kayg · 7 comments

kayg commented

2026-11-09 08:14:08 +00:00

Owner

Duplicate "Trust and continue" in Add host can end in an error instead of continuing (J395b, #439 final comment). A trust must be idempotent: one setup step however many clicks.

Branch: fix-trust-key-double-click, base origin/dev 4754b14a7. Job J419.

Findings so far (code reading, not yet reproduced):

Server: TrustedOpenSsh::confirm (crates/plugins/ssh/src/trust.rs) removes the pending challenge on first use, so an identical second confirm hits denied() ("The SSH host-key confirmation needs review. Retry the host check.") even though the key was just pinned.
Server socket (bootstrap_api.rs): while one confirmation runs, any further text frame is answered {"error":"host-key confirmation busy…"}.
Client: ProvisioningRun.confirmHostKey rejects a second call while one is pending, and an error frame rejects the pending one and raises onError (the failure screen). real.ts trustHostKey only forgets the observation after the await, so a repeat after success throws "Check the host key again before trusting it."
The wizard's trustKey() has a busy guard that disables the foot buttons, but that is only a disabled state, with no pending marker.

Plan (about 2 h): red specs first (server duplicate-trust test, service-level and production-build browser double/triple click), then: server remembers a consumed challenge with its fingerprint for the observation's remaining lifetime so an identical repeat answers success while a different key is still refused; client trustHostKey joins an in-flight trust and answers a repeat for an already-trusted key; the button gets aria-busy on the first click.

Duplicate "Trust and continue" in Add host can end in an error instead of continuing (J395b, #439 final comment). A trust must be idempotent: one setup step however many clicks. **Branch:** `fix-trust-key-double-click`, base `origin/dev` 4754b14a7. Job J419. **Findings so far (code reading, not yet reproduced):** - Server: `TrustedOpenSsh::confirm` (`crates/plugins/ssh/src/trust.rs`) *removes* the pending challenge on first use, so an identical second confirm hits `denied()` ("The SSH host-key confirmation needs review. Retry the host check.") even though the key was just pinned. - Server socket (`bootstrap_api.rs`): while one confirmation runs, any further text frame is answered `{"error":"host-key confirmation busy…"}`. - Client: `ProvisioningRun.confirmHostKey` rejects a second call while one is pending, and an `error` frame rejects the pending one and raises `onError` (the failure screen). `real.ts trustHostKey` only forgets the observation after the await, so a repeat after success throws "Check the host key again before trusting it." - The wizard's `trustKey()` has a `busy` guard that disables the foot buttons, but that is only a disabled state, with no pending marker. **Plan (about 2 h):** red specs first (server duplicate-trust test, service-level and production-build browser double/triple click), then: server remembers a consumed challenge with its fingerprint for the observation's remaining lifetime so an identical repeat answers success while a different key is still refused; client trustHostKey joins an in-flight trust and answers a repeat for an already-trusted key; the button gets `aria-busy` on the first click.

kayg commented

2026-11-09 08:20:52 +00:00

Author

Owner

Progress (J419). Branch fix-trust-key-double-click, head 15ff4aba7, pushed. Nothing merged or deployed.

Cause, both sides (from the code, with one red run so far):

Server, ssh trust (crates/plugins/ssh/src/trust.rs): confirm removes the pending challenge on first use. An identical second confirmation therefore finds nothing and returns the generic denied() error ("The SSH host-key confirmation needs review. Retry the host check."), although the key was just pinned.
Server, socket (bootstrap_api.rs): while one confirmation is looking up, every other frame, including an identical repeat, gets the host-key confirmation busy error. The screen treats any error frame as a failure and rejects the pending trust.
Client, service (apps/tracer/src/canvas/addhost/real.ts trustHostKey): it forgot the observation only after the confirmation resolved, and ProvisioningRun.confirmHostKey rejects a second one while one is pending. A repeat therefore failed with "Host-key confirmation is unavailable", or, after success, with "Check the host key again before trusting it."
Client, button: the wizard's trustKey() already had a busy guard that disables the foot buttons, so a plain double-click on the button sends one request. The failures above can still reach the service from any caller or timing that gets past the button. The button had no explicit pending marker.

Red (client service), run on dev-02 against bc9b2034d with bun: real.test.ts gives 14 pass and 2 fail. The failures are the two repeat cases:

error: Host-key confirmation is unavailable.   (a second trust while the first is in flight)
error: Check the host key again before trusting it.   (a trust repeated after success)

Green (same file, head): 16 pass, 0 fail.

Fixes (atomic commits): de39de93f server remembers a confirmed challenge (destination and fingerprint, 120 s, at most 128) and answers an exact repeat with success after the live authority check, writing nothing; a different key, another destination or an unknown challenge stays refused. 1e01266ed the socket drops an identical in-flight confirmation instead of answering busy. real.ts trust joins an in-flight trust and answers a repeat after success. wizard.ts sets aria-busy on the primary button.

Still to run, queued on dev-02 behind other jobs: Rust red/green (host_trust_repeat, bootstrap_api busy tests), the production-build Playwright double and triple click, verify quick.

**Progress (J419).** Branch `fix-trust-key-double-click`, head `15ff4aba7`, pushed. Nothing merged or deployed. **Cause, both sides (from the code, with one red run so far):** 1. **Server, ssh trust** (`crates/plugins/ssh/src/trust.rs`): `confirm` removes the pending challenge on first use. An identical second confirmation therefore finds nothing and returns the generic `denied()` error ("The SSH host-key confirmation needs review. Retry the host check."), although the key was just pinned. 2. **Server, socket** (`bootstrap_api.rs`): while one confirmation is looking up, every other frame, including an identical repeat, gets the `host-key confirmation busy` error. The screen treats any error frame as a failure and rejects the pending trust. 3. **Client, service** (`apps/tracer/src/canvas/addhost/real.ts` `trustHostKey`): it forgot the observation only after the confirmation resolved, and `ProvisioningRun.confirmHostKey` rejects a second one while one is pending. A repeat therefore failed with "Host-key confirmation is unavailable", or, after success, with "Check the host key again before trusting it." 4. **Client, button:** the wizard's `trustKey()` already had a `busy` guard that disables the foot buttons, so a plain double-click on the button sends one request. The failures above can still reach the service from any caller or timing that gets past the button. The button had no explicit pending marker. **Red (client service), run on dev-02 against `bc9b2034d` with bun:** `real.test.ts` gives 14 pass and 2 fail. The failures are the two repeat cases: ```text error: Host-key confirmation is unavailable. (a second trust while the first is in flight) error: Check the host key again before trusting it. (a trust repeated after success) ``` **Green (same file, head):** 16 pass, 0 fail. **Fixes (atomic commits):** `de39de93f` server remembers a confirmed challenge (destination and fingerprint, 120 s, at most 128) and answers an exact repeat with success after the live authority check, writing nothing; a different key, another destination or an unknown challenge stays refused. `1e01266ed` the socket drops an identical in-flight confirmation instead of answering busy. `real.ts` trust joins an in-flight trust and answers a repeat after success. `wizard.ts` sets `aria-busy` on the primary button. **Still to run, queued on dev-02 behind other jobs:** Rust red/green (`host_trust_repeat`, `bootstrap_api` busy tests), the production-build Playwright double and triple click, `verify quick`.

kayg commented

2026-11-09 09:51:03 +00:00

Author

Owner

J419 finish. Not merged, not deployed.

Branch: fix-trust-key-double-click, head 6f83ee00fd218484585ed449278e227842b3e701, confirmed with git ls-remote origin fix-trust-key-double-click. Base origin/dev 4754b14a7.

Cause: see my earlier comment. In short, a trust is not idempotent in three places:

the server's confirm consumes the challenge, so an identical repeat gets the generic host-key error;
the bootstrap socket answers an identical in-flight confirmation with the "busy" error, which the screen shows as a failure;
the client's trustHostKey rejects a second concurrent call and, after success, a repeat too.

What did not reproduce: a plain double or triple click on the real button. The wizard already disables the foot buttons on the first click, so the production-build spec is green on the unfixed client (red-browser.log, run on bc9b2034d: 2 passed). It stays in as a regression guard: exactly one confirm-host-key frame goes out and the flow reaches the done row. The red specs are the server and service ones below. I did not run J395b's rig on compute01. It needs the root VM and a mesh claim, and the red/green evidence above covers the server and service paths without it. J395b's own failure therefore came through one of the paths in the cause list (a faster or non-button caller, or the socket timing). I can't say which from here.

Red then green (logs in ~/.cache/codex-scratch/j419/ on dev-02):

crates/plugins/ssh/tests/host_trust_repeat.rs: red on 8a89e8341 (red-ssh.log: 2 passed, 2 failed, confirmation 2 of the same key must answer success and the simultaneous case with two HostKey errors); green on 15ff4aba7 (green-ssh.log: 4 passed, 0 failed).
bootstrap_api_tests::an_identical_confirmation_during_a_pending_one_is_not_refused_as_busy: red on b485ea514 (red-ws.log: an identical confirmation got a reply while the first was pending … "host-key confirmation busy; retry after confirmation"); green on 15ff4aba7 (green-ws2.log: 2 passed, 0 failed, with the existing busy test).
real.test.ts service-level repeat tests: red 14 pass / 2 fail on bc9b2034d, green 16 pass / 0 fail on head.
A different key is still refused in all three layers. The ssh test also checks another destination, a revoked sign-in on a repeat, and an unknown challenge.

What changed:

trust.rs: remembers each confirmed challenge (destination and fingerprint, 120 s, at most 128) and answers an exact repeat with success after the live authority check; a tokio mutex makes simultaneous identical requests take turns. Adds the tokio sync feature to the ssh crate (no new dependency).
bootstrap_api.rs: an identical in-flight confirmation frame is dropped, so the one reply answers both; any other request still gets the busy error.
real.ts: trustHostKey joins an in-flight trust, answers a repeat after success, and forgets that on a new observation; an unseen fingerprint is still refused.
wizard.ts: aria-busy on the primary button while a request is in flight.

verify quick on 6f83ee00f (quick3.log, dev-02, VERIFY_QUICK_BASE=origin/dev), verbatim:

+---------------------------+--------------+
| Rust gate                 | Result       |
+---------------------------+--------------+
| scope guard               | PASS         |
| migration names           | PASS         |
| Nix Cargo vendoring       | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| shape lints off (D351)    | PASS         |
| doc comments on diff      | PASS         |
| fixed test waits          | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |
| ui literals               | PASS         |
| tracer typecheck          | PASS         |
+---------------------------+--------------+

QUICK pre-check passed. This is not the gate: run scripts/verify fast before reporting done.
exit=0

Two earlier quick runs failed on fmt and on missing D345 doc comments in my own files; both were fixed in 79eb2462b and 6f83ee00f. verify fast was not run (the task asked for quick); the orchestrator reruns it on merge. No screenshots: the only UI change is an aria-busy attribute.

Cleanup: the cargo target and temporary archives are deleted. The dev-02 worktree ~/worktrees/j419 is left detached at the head, and the logs are small. Nothing of mine is running.

J419 finish. **Not merged, not deployed.** **Branch:** `fix-trust-key-double-click`, **head `6f83ee00fd218484585ed449278e227842b3e701`**, confirmed with `git ls-remote origin fix-trust-key-double-click`. Base `origin/dev` 4754b14a7. **Cause:** see my earlier comment. In short, a trust is not idempotent in three places: 1. the server's `confirm` consumes the challenge, so an identical repeat gets the generic host-key error; 2. the bootstrap socket answers an identical in-flight confirmation with the "busy" error, which the screen shows as a failure; 3. the client's `trustHostKey` rejects a second concurrent call and, after success, a repeat too. **What did not reproduce:** a plain double or triple click on the real button. The wizard already disables the foot buttons on the first click, so the production-build spec is **green on the unfixed client** (`red-browser.log`, run on `bc9b2034d`: 2 passed). It stays in as a regression guard: exactly one `confirm-host-key` frame goes out and the flow reaches the done row. The red specs are the server and service ones below. I did not run J395b's rig on compute01. It needs the root VM and a mesh claim, and the red/green evidence above covers the server and service paths without it. J395b's own failure therefore came through one of the paths in the cause list (a faster or non-button caller, or the socket timing). I can't say which from here. **Red then green** (logs in `~/.cache/codex-scratch/j419/` on dev-02): - `crates/plugins/ssh/tests/host_trust_repeat.rs`: red on `8a89e8341` (`red-ssh.log`: 2 passed, 2 failed, `confirmation 2 of the same key must answer success` and the simultaneous case with two `HostKey` errors); green on `15ff4aba7` (`green-ssh.log`: 4 passed, 0 failed). - `bootstrap_api_tests::an_identical_confirmation_during_a_pending_one_is_not_refused_as_busy`: red on `b485ea514` (`red-ws.log`: `an identical confirmation got a reply while the first was pending … "host-key confirmation busy; retry after confirmation"`); green on `15ff4aba7` (`green-ws2.log`: 2 passed, 0 failed, with the existing busy test). - `real.test.ts` service-level repeat tests: red 14 pass / 2 fail on `bc9b2034d`, green 16 pass / 0 fail on head. - A different key is still refused in all three layers. The ssh test also checks another destination, a revoked sign-in on a repeat, and an unknown challenge. **What changed:** - `trust.rs`: remembers each confirmed challenge (destination and fingerprint, 120 s, at most 128) and answers an exact repeat with success after the live authority check; a tokio mutex makes simultaneous identical requests take turns. Adds the tokio `sync` feature to the ssh crate (no new dependency). - `bootstrap_api.rs`: an identical in-flight confirmation frame is dropped, so the one reply answers both; any other request still gets the busy error. - `real.ts`: `trustHostKey` joins an in-flight trust, answers a repeat after success, and forgets that on a new observation; an unseen fingerprint is still refused. - `wizard.ts`: `aria-busy` on the primary button while a request is in flight. **`verify quick` on `6f83ee00f`** (`quick3.log`, dev-02, `VERIFY_QUICK_BASE=origin/dev`), verbatim: ```text +---------------------------+--------------+ | Rust gate | Result | +---------------------------+--------------+ | scope guard | PASS | | migration names | PASS | | Nix Cargo vendoring | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | shape lints off (D351) | PASS | | doc comments on diff | PASS | | fixed test waits | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | | ui literals | PASS | | tracer typecheck | PASS | +---------------------------+--------------+ QUICK pre-check passed. This is not the gate: run scripts/verify fast before reporting done. exit=0 ``` Two earlier quick runs failed on fmt and on missing D345 doc comments in my own files; both were fixed in `79eb2462b` and `6f83ee00f`. **`verify fast` was not run** (the task asked for quick); the orchestrator reruns it on merge. No screenshots: the only UI change is an `aria-busy` attribute. **Cleanup:** the cargo target and temporary archives are deleted. The dev-02 worktree `~/worktrees/j419` is left detached at the head, and the logs are small. Nothing of mine is running.

kayg commented

2026-11-09 10:31:24 +00:00

Author

Owner

Review start (J423, independent security review). Reviewing fix-trust-key-double-click at 6f83ee00fd (base origin/dev 4754b14a7). Scratch merge with origin/dev 54a9da2a8 is clean (825a760e9, local only, on compute01 ~/worktrees/j423). Red/green runs, two reviewer probes and verify quick are queued in one build-slot job (compute01 queue is 8 deep). verify fast will not get a slot within 30 min, so it is skipped. Nothing will be pushed, merged or deployed. Evidence: ~/.cache/codex-scratch/j423/evidence/ on compute01 and on code.

**Review start (J423, independent security review).** Reviewing `fix-trust-key-double-click` at **6f83ee00fd218484585ed449278e227842b3e701** (base origin/dev 4754b14a7). Scratch merge with origin/dev 54a9da2a8 is clean (`825a760e9`, local only, on compute01 `~/worktrees/j423`). Red/green runs, two reviewer probes and `verify quick` are queued in one build-slot job (compute01 queue is 8 deep). `verify fast` will not get a slot within 30 min, so it is skipped. Nothing will be pushed, merged or deployed. Evidence: `~/.cache/codex-scratch/j423/evidence/` on compute01 and on code.

kayg commented

2026-11-09 11:22:06 +00:00

Author

Owner

J423 security review: REQUEST CHANGES. Reviewed fix-trust-key-double-click at 6f83ee00fd (base origin/dev 4754b14a7; scratch merge with origin/dev 54a9da2a8 is clean). Nothing pushed, merged or deployed.

There are two P2s. Both are small and live in confirm in crates/plugins/ssh/src/trust.rs. I found no way to replay or bypass the trust check, and the socket and client parts hold up.

P2-1: the new `confirming` lock makes every confirmation on the server wait in one line

confirming: tokio::sync::Mutex<()> is a field of TrustedOpenSsh, and there is one per server (bootstrap_runtime.rs:176). confirm holds it for the whole confirmation: ssh -G, ssh-keygen -F (up to 3 s per known-hosts path), the live authorize() lookup and the pin write. So a confirmation for host B, or for another user, waits behind any confirmation that is slow, on any host.

Evidence: probe B (review_probe.rs, debug.log). Host A's authority takes 3 s. Host B's unrelated confirmation starts 200 ms later and finishes after b_elapsed_ms=2814 (PROBE_B a=Ok(()) b=Ok(()) b_elapsed_ms=2814 total_ms=3015). On base the two run in parallel.
Why it isn't needed: two identical confirmations can't reach the transport at the same time in production. The socket runs one request at a time, and this branch now drops the identical frame. The batch belongs to one socket, and the runner checks the owner and the current last_failure challenge. The lock only serves the simultaneous_identical_confirmations_all_succeed test, which calls the transport directly.
Fix: remove the lock, or scope it to one challenge. One way: keep a per-challenge "confirming" marker next to pending/confirmed, so only an identical challenge waits. This is a latency regression under CLAUDE.md priority 1, and no benchmark comes with it.

P2-2: a repeat answers "trusted" even when the pin is gone or holds another key

The repeat path checks only (challenge, destination, fingerprint, ≤120 s) plus live authority. It never checks that the pin it wrote is still there. Within the 120 s window:

After Forget (D368, remote_removal.rs → forget_pin): a repeat returns Ok, and the socket replies host_key_confirmed {fingerprint}, but no pin exists. Evidence: probe A, PROBE_A pins_after_forget=0 repeat_result=Ok(()) (debug.log).
After another challenge replaced the pin (a changed-key confirmation, for example during a key swap): a repeat of the old challenge still says that key is trusted, although the pin now holds a different key.

Connections still fail closed, because the next connect checks the actual pin. But the trust flow reports the wrong thing about which key is trusted, which is exactly what this flow exists to show. Since the repeat path writes nothing, a lie isn't needed.

Fix: store policy and record in Confirmed. On a repeat, require self.recorded(&policy)? == Some(record) (with PUBLICATION held), and answer denied() otherwise. Add probe A as a test, plus a replaced-pin case.

P3-1 (client): a new observation can join an old trust, and the old trust then deletes the new observation

In real.ts, trusting and trusted are keyed by host\nfingerprint, without the challenge. Suppose a new observation for the same host and the same fingerprint arrives while an older trust is in flight. A trust call for it joins the old promise, which confirms the old challenge. When that finishes, observations.delete(host) removes the new observation (the unconditional delete was already there before this branch). Mitigation: the wizard is busy during a trust, so this is unlikely. Fix: put the challenge in the key, or delete only if (observations.get(host) === observed).

P3-2: the field failure is still not reproduced

The author's finish note says a real double or triple click was green on the unfixed client, because the wizard already disables the button. The fix therefore covers the likely paths (socket busy reply, server consuming the challenge, service repeat) without showing which one J395b actually hit. That is fine as defence in depth. Keep #449's root cause marked "unconfirmed" until J395b's rig or the test server shows it.

Checked and fine

Replay by another user, session or tenant: blocked before the transport. The runner's confirm_host_key requires batch.inner.authority.owner == owner, a batch that belongs to this socket, a failed and non-running state, and last_failure.host_key_challenge == challenge. An API-key socket is refused outright.
Revocation and sign-out: the repeat runs authority.authorize() live, and the author's test with a revoked authority is refused. The socket's revocation.changed() also tears the socket down.
Another key or destination: refused (same == false → denied()).
No extension past 120 s: Confirmed.observed is the observation's received time.
Memory: confirmed holds at most 128 entries and expired ones are pruned on insert. When it is full, the code falls back to the old refusal.
Socket dedupe: is_same_confirmation compares every field of ConfirmHostKey (target, challenge, fingerprint). Anything else still gets the busy reply. Frames come only from the already-authenticated owner socket, so no other party's frame can be dropped.
Client joins: callers that join share one promise, so a rejection reaches all of them, and finally clears the in-flight entry. trusted is keyed by host and cleared on a new observation and on dispose, so I found no stale success across hosts.

Runs (dev-02, scratch merge `52ae8a1f0`, build-slot)

test	red (this file's fix reverse-applied)	green
ssh `host_trust_repeat`	2 passed / 2 failed (`confirmation 2 of the same key must answer success`, simultaneous → two `HostKey` errors)	4 passed / 0 failed
socket `an_identical_confirmation_during_a_pending_one…`	0 passed / 1 failed	`bootstrap_api` filter: 19 passed / 0 failed
`real.test.ts`	14 pass / 2 fail (join in flight, repeat after success)	16 pass / 0 fail

Logs: redgreen-ssh.log, red-socket.log, green-socket.log, red-real.log, green-real.log, debug.log. Note: my first ssh run (green-ssh.log, red-ssh.log) used INFRATERNAL_TEST_STATE_ROOT=/tmp/j423s. Under that root the rig's stand-in open fails with Install, not HostKey, so those two logs say nothing about the change. The rerun with the default scratch is the valid one.

scripts/verify quick on the scratch merge (pre-check only, not the gate), from verify-quick.log:

| scope guard               | PASS         |
| migration names           | PASS         |
| Nix Cargo vendoring       | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| shape lints off (D351)    | PASS         |
| doc comments on diff      | PASS         |
| fixed test waits          | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |
| ui literals               | PASS         |
| tracer typecheck          | PASS         |
QUICK pre-check passed. This is not the gate: run scripts/verify fast before reporting done.

verify fast was not run. compute01's queue was 9 deep (oldest waiting 3 h), so no slot was free within 30 minutes.

Evidence (probe source, scripts, all logs) is at ~/.cache/codex-scratch/j423/evidence/ on code. The probe source is also attached here. The VM worktrees and the dev-02 target are deleted.

**J423 security review: REQUEST CHANGES.** Reviewed `fix-trust-key-double-click` at **6f83ee00fd218484585ed449278e227842b3e701** (base origin/dev 4754b14a7; scratch merge with origin/dev 54a9da2a8 is clean). Nothing pushed, merged or deployed. There are two P2s. Both are small and live in `confirm` in `crates/plugins/ssh/src/trust.rs`. I found no way to replay or bypass the trust check, and the socket and client parts hold up. ### P2-1: the new `confirming` lock makes every confirmation on the server wait in one line `confirming: tokio::sync::Mutex<()>` is a field of `TrustedOpenSsh`, and there is one per server (`bootstrap_runtime.rs:176`). `confirm` holds it for the whole confirmation: `ssh -G`, `ssh-keygen -F` (up to 3 s per known-hosts path), the live `authorize()` lookup and the pin write. So a confirmation for host B, or for another user, waits behind any confirmation that is slow, on any host. - **Evidence:** probe B (`review_probe.rs`, `debug.log`). Host A's authority takes 3 s. Host B's unrelated confirmation starts 200 ms later and finishes after **`b_elapsed_ms=2814`** (`PROBE_B a=Ok(()) b=Ok(()) b_elapsed_ms=2814 total_ms=3015`). On base the two run in parallel. - **Why it isn't needed:** two identical confirmations can't reach the transport at the same time in production. The socket runs one request at a time, and this branch now drops the identical frame. The batch belongs to one socket, and the runner checks the owner and the current `last_failure` challenge. The lock only serves the `simultaneous_identical_confirmations_all_succeed` test, which calls the transport directly. - **Fix:** remove the lock, or scope it to one challenge. One way: keep a per-challenge "confirming" marker next to `pending`/`confirmed`, so only an identical challenge waits. This is a latency regression under CLAUDE.md priority 1, and no benchmark comes with it. ### P2-2: a repeat answers "trusted" even when the pin is gone or holds another key The repeat path checks only `(challenge, destination, fingerprint, ≤120 s)` plus live authority. It never checks that the pin it wrote is still there. Within the 120 s window: - **After Forget (D368, `remote_removal.rs` → `forget_pin`):** a repeat returns `Ok`, and the socket replies `host_key_confirmed {fingerprint}`, but no pin exists. **Evidence:** probe A, `PROBE_A pins_after_forget=0 repeat_result=Ok(())` (`debug.log`). - **After another challenge replaced the pin** (a changed-key confirmation, for example during a key swap): a repeat of the old challenge still says that key is trusted, although the pin now holds a different key. Connections still fail closed, because the next connect checks the actual pin. But the trust flow reports the wrong thing about which key is trusted, which is exactly what this flow exists to show. Since the repeat path writes nothing, a lie isn't needed. - **Fix:** store `policy` and `record` in `Confirmed`. On a repeat, require `self.recorded(&policy)? == Some(record)` (with `PUBLICATION` held), and answer `denied()` otherwise. Add probe A as a test, plus a replaced-pin case. ### P3-1 (client): a new observation can join an old trust, and the old trust then deletes the new observation In `real.ts`, `trusting` and `trusted` are keyed by `host\nfingerprint`, without the challenge. Suppose a new observation for the same host and the same fingerprint arrives while an older trust is in flight. A trust call for it joins the old promise, which confirms the old challenge. When that finishes, `observations.delete(host)` removes the **new** observation (the unconditional delete was already there before this branch). Mitigation: the wizard is busy during a trust, so this is unlikely. **Fix:** put the challenge in the key, or delete only `if (observations.get(host) === observed)`. ### P3-2: the field failure is still not reproduced The author's finish note says a real double or triple click was green on the unfixed client, because the wizard already disables the button. The fix therefore covers the likely paths (socket busy reply, server consuming the challenge, service repeat) without showing which one J395b actually hit. That is fine as defence in depth. Keep #449's root cause marked "unconfirmed" until J395b's rig or the test server shows it. ### Checked and fine - **Replay by another user, session or tenant: blocked before the transport.** The runner's `confirm_host_key` requires `batch.inner.authority.owner == owner`, a batch that belongs to this socket, a failed and non-running state, and `last_failure.host_key_challenge == challenge`. An API-key socket is refused outright. - **Revocation and sign-out:** the repeat runs `authority.authorize()` live, and the author's test with a revoked authority is refused. The socket's `revocation.changed()` also tears the socket down. - **Another key or destination:** refused (`same == false` → `denied()`). - **No extension past 120 s:** `Confirmed.observed` is the observation's `received` time. - **Memory:** `confirmed` holds at most 128 entries and expired ones are pruned on insert. When it is full, the code falls back to the old refusal. - **Socket dedupe:** `is_same_confirmation` compares every field of `ConfirmHostKey` (target, challenge, fingerprint). Anything else still gets the busy reply. Frames come only from the already-authenticated owner socket, so no other party's frame can be dropped. - **Client joins:** callers that join share one promise, so a rejection reaches all of them, and `finally` clears the in-flight entry. `trusted` is keyed by host and cleared on a new observation and on `dispose`, so I found no stale success across hosts. ### Runs (dev-02, scratch merge `52ae8a1f0`, build-slot) | test | red (this file's fix reverse-applied) | green | |---|---|---| | ssh `host_trust_repeat` | 2 passed / 2 failed (`confirmation 2 of the same key must answer success`, simultaneous → two `HostKey` errors) | 4 passed / 0 failed | | socket `an_identical_confirmation_during_a_pending_one…` | 0 passed / 1 failed | `bootstrap_api` filter: 19 passed / 0 failed | | `real.test.ts` | 14 pass / 2 fail (join in flight, repeat after success) | 16 pass / 0 fail | Logs: `redgreen-ssh.log`, `red-socket.log`, `green-socket.log`, `red-real.log`, `green-real.log`, `debug.log`. Note: my first ssh run (`green-ssh.log`, `red-ssh.log`) used `INFRATERNAL_TEST_STATE_ROOT=/tmp/j423s`. Under that root the rig's stand-in `open` fails with `Install`, not `HostKey`, so those two logs say nothing about the change. The rerun with the default scratch is the valid one. `scripts/verify quick` on the scratch merge (pre-check only, not the gate), from `verify-quick.log`: ``` | scope guard | PASS | | migration names | PASS | | Nix Cargo vendoring | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | shape lints off (D351) | PASS | | doc comments on diff | PASS | | fixed test waits | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | | ui literals | PASS | | tracer typecheck | PASS | QUICK pre-check passed. This is not the gate: run scripts/verify fast before reporting done. ``` `verify fast` was **not run**. compute01's queue was 9 deep (oldest waiting 3 h), so no slot was free within 30 minutes. Evidence (probe source, scripts, all logs) is at `~/.cache/codex-scratch/j423/evidence/` on code. The probe source is also attached here. The VM worktrees and the dev-02 target are deleted.

j423-review_probe.rs.txt

2.9 KiB

j423-debug.log.txt

2.4 KiB

j423-redgreen-ssh.log.txt

1.5 KiB

j423-red-socket.log.txt

5.4 KiB

j423-green-socket.log.txt

15 KiB

j423-red-real.log.txt

3.5 KiB

j423-green-real.log.txt

1.9 KiB

j423-verify-quick.log.txt

96 KiB

kayg commented

2026-11-09 12:35:14 +00:00

Author

Owner

J419 round 2 progress (answers J423's REQUEST CHANGES on 6f83ee00f). Branch fix-trust-key-double-click, head 45198e736009277fb041c09bc06ba3c8b2d770d7 (fixes plus a merge of current origin/dev), pushed. Not merged or deployed. verify fast is queued on compute01 (position 10 of 15 at 12:29 IST); I'll quote it when it finishes.

Commits: d9d7422df red tests, 881a5237e server fix, 273702b0d client fix, 45198e736 merge of origin/dev.

P2-1: the server-wide confirming lock is now one lock per challenge, kept in a table whose entries are removed by a drop guard (also on cancellation). New test confirmations_for_different_hosts_do_not_wait_for_each_other holds host A inside its authority check until host B has finished, with a 10 s deadlock guard (not a speed budget). Red: host B was held up behind host A for 10.00182269s: Err(Elapsed(())). Green.
P2-2: Confirmed keeps the policy and the exact record. A repeat succeeds only while that record is still the pin on disk (checked under PUBLICATION); otherwise the usual refusal. New tests: pin deleted, pin replaced by another key. Both red before, green after.
P3-1: real.ts keys in-flight trusts by challenge; a finished trust deletes and records only the observation it confirmed. New service test with two runs. Red 16 pass / 1 fail, green 17 pass / 0 fail.

host_trust_repeat: red on d9d7422df is 4 passed / 3 failed (r2-red-ssh.log); green on 45198e736 is 7 passed / 0 failed (r3-green-ssh.log).

verify quick on 45198e736 (r3-quick.log, dev-02):

| scope guard               | PASS         |
| migration names           | PASS         |
| Nix Cargo vendoring       | PASS         |
| VT source recipe          | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| shape lints off (D351)    | PASS         |
| doc comments on diff      | PASS         |
| fixed test waits          | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |
| ui literals               | PASS         |
| tracer typecheck          | PASS         |
QUICK pre-check passed.

P3-2, which path J395b hit. I did not run the rig. It lives under /root/j395-vm, which claude cannot read, and compute01's queue is 15 deep. I read J395b's kept evidence instead (~/.cache/codex-scratch/j395/evidence/harness-trust-final-kayg-039-kayg/ and fix_trust.py):

The harness clicks "trust host key" only when the button is visible and enabled (fix_trust.py). Its log shows STEP host key trusted three times, and after the first click the page still showed "Check 127.0.0.1's key" with the trust-key button.
On success the wizard moves to the progress page and removes that button. So the first trust did not succeed: the button came back enabled, which only happens in the wizard's error path (showError, busy cleared). That means a click did go through the button; the failure came afterwards, in the confirmation. Two more clicks followed, then "Something went wrong with 127.0.0.1".
The server log has only the awaiting host key confirmation line, with no success or refusal line. Load average was 25–40.
Most likely path, not proven: the client's confirmation timer (5 s in provisioning-client.ts) ran out under load while the server was still checking, so the button re-enabled with the challenge already being consumed. The next click then hit the server's denied (challenge gone) or the busy reply, and the error frame turned the batch into the failure page. Every step of that is now handled: an identical repeat joins or is answered as already trusted, with no error frame.
I did not change the 5 s timer. If the owner wants the timer itself raised, that is a separate, one-line decision.

**J419 round 2 progress** (answers J423's REQUEST CHANGES on `6f83ee00f`). Branch `fix-trust-key-double-click`, head `45198e736009277fb041c09bc06ba3c8b2d770d7` (fixes plus a merge of current origin/dev), pushed. Not merged or deployed. `verify fast` is queued on compute01 (position 10 of 15 at 12:29 IST); I'll quote it when it finishes. **Commits:** `d9d7422df` red tests, `881a5237e` server fix, `273702b0d` client fix, `45198e736` merge of origin/dev. - **P2-1:** the server-wide `confirming` lock is now one lock per challenge, kept in a table whose entries are removed by a drop guard (also on cancellation). New test `confirmations_for_different_hosts_do_not_wait_for_each_other` holds host A inside its authority check until host B has finished, with a 10 s deadlock guard (not a speed budget). Red: `host B was held up behind host A for 10.00182269s: Err(Elapsed(()))`. Green. - **P2-2:** `Confirmed` keeps the policy and the exact record. A repeat succeeds only while that record is still the pin on disk (checked under `PUBLICATION`); otherwise the usual refusal. New tests: pin deleted, pin replaced by another key. Both red before, green after. - **P3-1:** `real.ts` keys in-flight trusts by challenge; a finished trust deletes and records only the observation it confirmed. New service test with two runs. Red 16 pass / 1 fail, green 17 pass / 0 fail. `host_trust_repeat`: red on `d9d7422df` is 4 passed / 3 failed (`r2-red-ssh.log`); green on `45198e736` is 7 passed / 0 failed (`r3-green-ssh.log`). **`verify quick` on `45198e736`** (`r3-quick.log`, dev-02): ```text | scope guard | PASS | | migration names | PASS | | Nix Cargo vendoring | PASS | | VT source recipe | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | shape lints off (D351) | PASS | | doc comments on diff | PASS | | fixed test waits | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | | ui literals | PASS | | tracer typecheck | PASS | QUICK pre-check passed. ``` **P3-2, which path J395b hit.** I did not run the rig. It lives under `/root/j395-vm`, which `claude` cannot read, and compute01's queue is 15 deep. I read J395b's kept evidence instead (`~/.cache/codex-scratch/j395/evidence/harness-trust-final-kayg-039-kayg/` and `fix_trust.py`): - The harness clicks "trust host key" only when the button is **visible and enabled** (`fix_trust.py`). Its log shows `STEP host key trusted` three times, and after the first click the page still showed "Check 127.0.0.1's key" with the `trust-key` button. - On success the wizard moves to the progress page and removes that button. So the first trust did not succeed: the button came back enabled, which only happens in the wizard's error path (`showError`, busy cleared). That means a click did go through the button; the failure came afterwards, in the confirmation. Two more clicks followed, then "Something went wrong with 127.0.0.1". - The server log has only the `awaiting host key confirmation` line, with no success or refusal line. Load average was 25–40. - **Most likely path, not proven:** the client's confirmation timer (5 s in `provisioning-client.ts`) ran out under load while the server was still checking, so the button re-enabled with the challenge already being consumed. The next click then hit the server's `denied` (challenge gone) or the busy reply, and the error frame turned the batch into the failure page. Every step of that is now handled: an identical repeat joins or is answered as already trusted, with no error frame. - I did not change the 5 s timer. If the owner wants the timer itself raised, that is a separate, one-line decision.

kayg commented

2026-11-10 04:43:02 +00:00

Author

Owner

J419 round 2 finish. Branch fix-trust-key-double-click, head 51414e63608b3b1ac56d121d1d3549068c9c0997 (confirmed with git ls-remote). Not merged or deployed.

Fixes for J423's findings were already described in my earlier round-2 comment: per-challenge confirm lock (P2-1), a repeat only while the pin on disk still matches (P2-2), and in-flight trusts keyed by challenge (P3-1). The head is those commits plus merges of current origin/dev.

verify fast on 51414e636 (compute01, build-slot, log ~/.cache/codex-scratch/j419/fast.log on compute01), verbatim:

| Nix Cargo vendoring       | PASS |
| fmt                       | PASS |
| migration names           | PASS |
| migration tooling tests   | PASS |
| VT native fmt             | PASS |
| CLI plugin compile-out    | PASS |
| clippy                    | PASS |
| SSH runtime test helper   | PASS |
| real-binary fixture paths | PASS |
| Traefik component manifest | PASS |
| release component publication | PASS |
| test                      | PASS |
| SSH inode reuse fixture   | PASS |
| shim process-per-user     | PASS |
| cargo-deny                | PASS |
| gate architecture         | PASS |
| doc coverage ratchet      | PASS |
| shape lints off (D351)    | PASS |
| doc comments on diff      | PASS |
| fixed test waits          | PASS |
| privileged integration    | PASS |
| probe release exclusion   | PASS |
| nightly fuzz reporter     | PASS |
| nightly browser suite reporter | PASS |
| static builds             | PASS |
FAST_EXIT=0

Two earlier fast runs failed for reasons outside this change: on 45198e736, cargo-deny (RUSTSEC-2026-0459, fixed on dev by #473; origin/dev itself passed deny) and one shim churn test (passed on the next run); on 899687ef0, the tracer test "a closing notice cannot hide its replacement", which passes on current origin/dev and on this head (startup-toast plus addhost: 84 pass, 0 fail). Also green earlier on the pre-merge head: host_trust_repeat 7/7, real.test.ts 17/17, verify quick, and the double/triple-click browser spec (2 passed). Red runs: host_trust_repeat 4 pass / 3 fail on d9d7422df; the cross-host test fails with host B was held up behind host A for 10.00s.

P3-2 (which path J395b hit): not reproduced with the rig (it is under /root/j395-vm, unreadable for claude). From J395b's kept evidence (run 039): the trust button reappeared after the first click, so the first confirmation did not complete. Most likely the client's 5 s confirmation timer ran out under load while the server was still checking, then the next click met the consumed challenge or the busy reply. Unproven; I did not change the 5 s timer. Keep the root cause marked "unconfirmed".

**J419 round 2 finish.** Branch `fix-trust-key-double-click`, **head `51414e63608b3b1ac56d121d1d3549068c9c0997`** (confirmed with `git ls-remote`). Not merged or deployed. Fixes for J423's findings were already described in my earlier round-2 comment: per-challenge confirm lock (P2-1), a repeat only while the pin on disk still matches (P2-2), and in-flight trusts keyed by challenge (P3-1). The head is those commits plus merges of current origin/dev. **`verify fast` on `51414e636`** (compute01, build-slot, log `~/.cache/codex-scratch/j419/fast.log` on compute01), verbatim: ```text | Nix Cargo vendoring | PASS | | fmt | PASS | | migration names | PASS | | migration tooling tests | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | doc coverage ratchet | PASS | | shape lints off (D351) | PASS | | doc comments on diff | PASS | | fixed test waits | PASS | | privileged integration | PASS | | probe release exclusion | PASS | | nightly fuzz reporter | PASS | | nightly browser suite reporter | PASS | | static builds | PASS | FAST_EXIT=0 ``` Two earlier fast runs failed for reasons outside this change: on `45198e736`, cargo-deny (RUSTSEC-2026-0459, fixed on dev by #473; origin/dev itself passed deny) and one shim churn test (passed on the next run); on `899687ef0`, the tracer test "a closing notice cannot hide its replacement", which passes on current origin/dev and on this head (`startup-toast` plus addhost: 84 pass, 0 fail). Also green earlier on the pre-merge head: `host_trust_repeat` 7/7, `real.test.ts` 17/17, `verify quick`, and the double/triple-click browser spec (2 passed). Red runs: `host_trust_repeat` 4 pass / 3 fail on `d9d7422df`; the cross-host test fails with `host B was held up behind host A for 10.00s`. **P3-2 (which path J395b hit):** not reproduced with the rig (it is under `/root/j395-vm`, unreadable for `claude`). From J395b's kept evidence (run 039): the trust button reappeared after the first click, so the first confirmation did not complete. Most likely the client's 5 s confirmation timer ran out under load while the server was still checking, then the next click met the consumed challenge or the busy reply. Unproven; I did not change the 5 s timer. Keep the root cause marked "unconfirmed".

kayg commented

2026-11-10 05:04:34 +00:00

Author

Owner

J467 security re-review (round 2): APPROVE. Reviewed fix-trust-key-double-click at 51414e6360 against origin/dev. Nothing pushed, merged or deployed. I added one adversarial test to a local VM copy only. Evidence is in ~/.cache/codex-scratch/j467/evidence/ on dev-02.

No P0, P1 or P2 findings. All three round-1 findings are fixed.

The round-1 fixes

P2-1 fixed. ChallengeTurn keeps one tokio::sync::Mutex per challenge UUID, and a challenge waits only on itself. confirmations_for_different_hosts_do_not_wait_for_each_other passes.
P2-2 fixed. A repeat needs a matching Confirmed entry (same destination, same fingerprint, observation under 120 s old) and a live authorize(). Then, under PUBLICATION, the pin on disk must equal the exact record this challenge wrote. The deleted-pin and replaced-pin tests both pass.
P3-1 fixed. trusting is keyed by challenge, and a finished trust deletes the observation only when observations.get(host) === observed. A new observation clears trusted for that host.

The security questions

Same host, two challenges, two keys. The per-challenge lock does not order these two, but it doesn't need to: the global PUBLICATION compare-and-replace (recorded == observation.previous) still does. The first write changes the pin, so the second fails its compare. A later repeat of the loser has no Confirmed entry and is refused. A repeat of the winner passes only while its record is still the pin. My adversarial test j467_two_challenges_two_keys_same_host_pin_exactly_one observes key K1 (challenge A), swaps the scanned key to K2 (challenge B), confirms A and B together with tokio::join! and checks:
- exactly one returns Ok;
- exactly one pin file exists, and it holds the winner's key;
- in 19 more joined rounds, the winner's repeat still succeeds and the loser's is still refused.
It passes, and it passed 200 out of 200 loops, together with the simultaneous-identical and cross-host tests (race-200.log).
No TOCTOU between the pin check and the success reply. The repeat path checks recorded(&policy) == record while holding PUBLICATION, does no await in between, and returns. The socket reply goes out after the lock is released. That gap is the same one the first-time path already has (write, release, reply). A Forget that lands in the gap is ordinary ordering and does not get around the check.
The lock map is bounded.
- ChallengeTurn::new clones the Arc under the table lock. Drop takes the table lock and removes the entry when strong_count <= 2 (the table's copy plus its own), so nobody can be waiting on a removed lock.
- Drop order is correct: _held is dropped before turn.
- Cancellation mid-lock().await also runs Drop.
- A random UUID fails at pending.remove and its entry goes away at once. The map never holds more than the confirmations in flight.
- confirmed is capped at 128 entries. Each lives at most 120 s, pruned when a later confirmation succeeds.
Scope. Runner::confirm_host_key (bootstrap.rs:1898) still ties the challenge to this batch's owner and to its current failed HostKey state. A repeat can't be replayed from another batch or another user.
Client.
- is_same_confirmation drops only an exact repeat of (target, challenge, fingerprint). Anything else still gets the busy error, and text that doesn't parse never counts as a repeat.
- provisioning-client.ts matches host_key_confirmed by target and fingerprint, so one reply settles the confirmation.
- setBusy disables the foot buttons and sets aria-busy.

P3 (non-blocking)

P3-a: the repeat path skips configured_trust and the effective-policy recheck. If known-hosts trust appears within 120 s, a repeat still answers success. This is harmless: the pin really is there, and connecting already refuses pin-plus-external trust as ambiguous. A one-line comment on why it is left out would help.
P3-b, related to P3-2: the client's 5 s confirm timeout (provisioning-client.ts:307) is unchanged. If the server's confirm takes longer than that under load (ssh -G, ssh-keygen -F up to 3 s per path, authority lookup), the first click still shows an error. What this branch adds is a working retry: the next click either joins the reply still in flight (the server drops the identical frame) or gets the repeat-path success. A follow-up should measure the confirm's p99 under load, then lengthen the timeout or keep the confirmation pending until the socket closes. That is out of scope for this branch.

Tests I ran (dev-02, build-slot, `CARGO_INCREMENTAL=0`, worktree at `51414e636`)

cargo test -p infraternal-plugin-ssh --test host_trust_repeat, with my test added: 8 passed, 0 failed (ssh-tests.log).
The same binary, 200 loops of j467_, simultaneous and confirmations_for: 200/200 ok, 0 failed (race-200.log).
cargo test -p infraternal-server --lib bootstrap_api: 19 passed, 0 failed, including an_identical_confirmation_during_a_pending_one_is_not_refused_as_busy and a_client_request_during_a_pending_confirmation_keeps_the_batch (rust.log).
A harness note, not a branch finding: my first ssh run set INFRATERNAL_TEST_STATE_ROOT=/tmp/j467s at mode 0755, and all 7 rig tests failed with Install, as the code should (a non-private root fails closed). Without it they pass (rust.log has the failing first attempt).
I did not rerun the client tests (dev-02 has no node_modules, and installing them would cost a slot). I read the author's fast.log on compute01 instead. It is at 51414e636, its 4 new real.test.ts cases pass, and it ends with FAST_EXIT=0. The author ran the double- and triple-click browser specs on the head before the merge (2 passed). Per the reviewer rule, I did not run verify fast or verify quick.

dev-02 cleanup: worktree ~/worktrees/j467 and its target are deleted. Evidence is kept (116K).

**J467 security re-review (round 2): APPROVE.** Reviewed `fix-trust-key-double-click` at **51414e63608b3b1ac56d121d1d3549068c9c0997** against origin/dev. Nothing pushed, merged or deployed. I added one adversarial test to a local VM copy only. Evidence is in `~/.cache/codex-scratch/j467/evidence/` on dev-02. No P0, P1 or P2 findings. All three round-1 findings are fixed. ### The round-1 fixes - **P2-1 fixed.** `ChallengeTurn` keeps one `tokio::sync::Mutex` per challenge UUID, and a challenge waits only on itself. `confirmations_for_different_hosts_do_not_wait_for_each_other` passes. - **P2-2 fixed.** A repeat needs a matching `Confirmed` entry (same destination, same fingerprint, observation under 120 s old) and a live `authorize()`. Then, under `PUBLICATION`, the pin on disk must equal the exact `record` this challenge wrote. The deleted-pin and replaced-pin tests both pass. - **P3-1 fixed.** `trusting` is keyed by challenge, and a finished trust deletes the observation only when `observations.get(host) === observed`. A new observation clears `trusted` for that host. ### The security questions 1. **Same host, two challenges, two keys.** The per-challenge lock does not order these two, but it doesn't need to: the global `PUBLICATION` compare-and-replace (`recorded == observation.previous`) still does. The first write changes the pin, so the second fails its compare. A later repeat of the loser has no `Confirmed` entry and is refused. A repeat of the winner passes only while its record is still the pin. **My adversarial test** `j467_two_challenges_two_keys_same_host_pin_exactly_one` observes key K1 (challenge A), swaps the scanned key to K2 (challenge B), confirms A and B together with `tokio::join!` and checks: - exactly one returns Ok; - exactly one pin file exists, and it holds the winner's key; - in 19 more joined rounds, the winner's repeat still succeeds and the loser's is still refused. It passes, and it passed **200 out of 200** loops, together with the simultaneous-identical and cross-host tests (`race-200.log`). 2. **No TOCTOU between the pin check and the success reply.** The repeat path checks `recorded(&policy) == record` while holding `PUBLICATION`, does no await in between, and returns. The socket reply goes out after the lock is released. That gap is the same one the first-time path already has (write, release, reply). A Forget that lands in the gap is ordinary ordering and does not get around the check. 3. **The lock map is bounded.** - `ChallengeTurn::new` clones the `Arc` under the table lock. `Drop` takes the table lock and removes the entry when `strong_count <= 2` (the table's copy plus its own), so nobody can be waiting on a removed lock. - Drop order is correct: `_held` is dropped before `turn`. - Cancellation mid-`lock().await` also runs `Drop`. - A random UUID fails at `pending.remove` and its entry goes away at once. The map never holds more than the confirmations in flight. - `confirmed` is capped at 128 entries. Each lives at most 120 s, pruned when a later confirmation succeeds. 4. **Scope.** `Runner::confirm_host_key` (`bootstrap.rs:1898`) still ties the challenge to this batch's owner and to its current failed `HostKey` state. A repeat can't be replayed from another batch or another user. 5. **Client.** - `is_same_confirmation` drops only an exact repeat of (target, challenge, fingerprint). Anything else still gets the busy error, and text that doesn't parse never counts as a repeat. - `provisioning-client.ts` matches `host_key_confirmed` by target and fingerprint, so one reply settles the confirmation. - `setBusy` disables the foot buttons and sets `aria-busy`. ### P3 (non-blocking) - **P3-a: the repeat path skips `configured_trust` and the effective-policy recheck.** If known-hosts trust appears within 120 s, a repeat still answers success. This is harmless: the pin really is there, and connecting already refuses pin-plus-external trust as ambiguous. A one-line comment on why it is left out would help. - **P3-b, related to P3-2: the client's 5 s confirm timeout (`provisioning-client.ts:307`) is unchanged.** If the server's confirm takes longer than that under load (`ssh -G`, `ssh-keygen -F` up to 3 s per path, authority lookup), the first click still shows an error. What this branch adds is a working retry: the next click either joins the reply still in flight (the server drops the identical frame) or gets the repeat-path success. A follow-up should measure the confirm's p99 under load, then lengthen the timeout or keep the confirmation pending until the socket closes. That is out of scope for this branch. ### Tests I ran (dev-02, build-slot, `CARGO_INCREMENTAL=0`, worktree at 51414e636) - `cargo test -p infraternal-plugin-ssh --test host_trust_repeat`, with my test added: **8 passed, 0 failed** (`ssh-tests.log`). - The same binary, 200 loops of `j467_`, `simultaneous` and `confirmations_for`: **200/200 ok, 0 failed** (`race-200.log`). - `cargo test -p infraternal-server --lib bootstrap_api`: **19 passed, 0 failed**, including `an_identical_confirmation_during_a_pending_one_is_not_refused_as_busy` and `a_client_request_during_a_pending_confirmation_keeps_the_batch` (`rust.log`). - A harness note, not a branch finding: my first ssh run set `INFRATERNAL_TEST_STATE_ROOT=/tmp/j467s` at mode 0755, and all 7 rig tests failed with `Install`, as the code should (a non-private root fails closed). Without it they pass (`rust.log` has the failing first attempt). - I did not rerun the client tests (dev-02 has no node_modules, and installing them would cost a slot). I read the author's `fast.log` on compute01 instead. It is at 51414e636, its 4 new `real.test.ts` cases pass, and it ends with `FAST_EXIT=0`. The author ran the double- and triple-click browser specs on the head before the merge (2 passed). Per the reviewer rule, I did not run `verify fast` or `verify quick`. dev-02 cleanup: worktree `~/worktrees/j467` and its target are deleted. Evidence is kept (116K).

No labels

launch-qa

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kayg/infraternal#449

No description provided.

Rows
Columns

Trust host key: a repeated click must not end in an error (J419, follows #439) #449

P2-1: the new confirming lock makes every confirmation on the server wait in one line

P2-2: a repeat answers "trusted" even when the pin is gone or holds another key

P3-1 (client): a new observation can join an old trust, and the old trust then deletes the new observation

P3-2: the field failure is still not reproduced

Checked and fine

Runs (dev-02, scratch merge 52ae8a1f0, build-slot)

The round-1 fixes

The security questions

P3 (non-blocking)

Tests I ran (dev-02, build-slot, CARGO_INCREMENTAL=0, worktree at 51414e636)

P2-1: the new `confirming` lock makes every confirmation on the server wait in one line

Runs (dev-02, scratch merge `52ae8a1f0`, build-slot)

Tests I ran (dev-02, build-slot, `CARGO_INCREMENTAL=0`, worktree at `51414e636`)