hub v0.86.0 — Copy works without revealing, and every copy branch reports itself

Found by the operator, in the way that matters: it cost a real login.

The v0.84.0 Console access card shipped its Copy button DISABLED until a Reveal.
Clicking it did nothing, silently, so the clipboard kept whatever was already in
it — another host's console password from an earlier reveal. That got pasted into
demo-hp's PVE login, which failed with no explanation: the box logged a plain
`password check failed for user (root)`, the credential was never at fault, and
nothing on screen said the copy had not happened.

A copy button that silently no-ops is worse than no copy button. The operator
cannot tell "copied" from "did nothing", and the stale value left behind is a
VALID secret for a DIFFERENT machine — so the failure looks like a stale
credential and sends you diagnosing the wrong thing.

Copy now works without revealing, and that is the safer default rather than a
concession: the secret goes straight to the clipboard and never renders on
screen, so it cannot be shoulder-surfed or caught in a screenshot. Reveal remains
for when it must be read.

Three silent-failure branches closed, all in the same eight-line function:
  - not yet revealed        -> was a disabled no-op; now fetches and copies
  - navigator.clipboard absent -> was silently skipped; now shows it and says why
  - writeText() REJECTED    -> promise was ignored, so the operator believed it
                               copied; now shows it and reports the refusal

The success path names the host ("Copied demo-hp-bb76ea's root@pam password"),
because the clipboard is fleet-wide and every box has a different console
password — "copied" alone cannot say for WHICH box, which is the confusion that
produced the incident.

One retrieval path, shared: the endpoint is defined once (data-reveal-url) and
read back with getAttribute, so Copy cannot drift onto a different, unaudited URL
than Reveal. Server-side is unchanged — both buttons hit the same CSRF-gated
endpoint and both write the same recovery_credential_revealed event, which is
correct: the register records accesses, and a copy is an access.

Tests 566 -> 568. Red-proof: re-adding `disabled` reproduces the shipped bug.
This commit is contained in:
2026-07-31 09:22:11 +02:00
parent 9e079c7883
commit 670ec35ece
3 changed files with 181 additions and 23 deletions
@@ -130,6 +130,88 @@ func TestReveal_A_PageNeverCarriesTheSecret(t *testing.T) {
}
}
// --- Copy must work WITHOUT a reveal (regression, 2026-07-31) ---
//
// THE INCIDENT: Copy shipped `disabled` until a Reveal. Clicking it did nothing, silently, so the
// operator's clipboard kept its previous contents — ANOTHER HOST'S console password — which was then
// pasted into a PVE login that failed with no explanation. The box logged a plain
// `password check failed for user (root)` and the credential was never at fault.
//
// Copying without revealing is also the SAFER path: the secret never renders on screen, so it cannot
// be shoulder-surfed or captured in a screenshot.
//
// RED-PROOF: restore `disabled` on the Copy button → this test goes red.
func TestReveal_CopyIsNotGatedOnReveal(t *testing.T) {
s, st, _ := newRevealServer(t)
cookie, _ := newRevealSession(t, s)
seedRevealHost(t, st, "demo-hp-bb76ea", "demo-hp", revealCanary)
req := httptest.NewRequest(http.MethodGet, "/hosts/demo-hp-bb76ea", nil)
req.AddCookie(cookie)
body := serveReveal(t, s, req).Body.String()
// Locate the Copy button and assert it ships ENABLED.
i := strings.Index(body, `id="console-copy-demo-hp-bb76ea"`)
if i < 0 {
t.Fatal("no Copy button on the card")
}
end := strings.Index(body[i:], ">")
if end < 0 {
t.Fatal("malformed Copy button tag")
}
tag := body[i : i+end]
if strings.Contains(tag, "disabled") {
t.Fatalf("the Copy button ships DISABLED — clicking it is a silent no-op that leaves a stale "+
"secret in the clipboard: %s", tag)
}
// It must still be the case that no secret is in the document.
if strings.Contains(body, revealCanary) {
t.Fatal("SECRET LEAK: enabling Copy put the password in the page")
}
// And the copy path must be wired to the SAME audited endpoint, not a second one.
if !strings.Contains(body, "copyConsolePassword('demo-hp-bb76ea')") {
t.Error("the Copy button is not wired to a handler")
}
// The endpoint URL must be DEFINED exactly once (the data-reveal-url attribute); the script
// reads it back with getAttribute rather than rebuilding it, so Copy cannot drift onto a
// different — unaudited — path than Reveal.
if n := strings.Count(body, "/hosts/demo-hp-bb76ea/reveal-recovery-credential"); n != 1 {
t.Errorf("the retrieval URL is written %d times; it must be defined once and read back", n)
}
}
// Every failure branch of the copy path must report itself. A disabled button, a missing clipboard
// API and a refused clipboard write all previously ended in silence.
func TestReveal_CopyPathHasNoSilentFailureBranch(t *testing.T) {
s, st, _ := newRevealServer(t)
cookie, _ := newRevealSession(t, s)
seedRevealHost(t, st, "demo-hp-bb76ea", "demo-hp", revealCanary)
req := httptest.NewRequest(http.MethodGet, "/hosts/demo-hp-bb76ea", nil)
req.AddCookie(cookie)
body := serveReveal(t, s, req).Body.String()
for _, want := range []struct{ frag, why string }{
{"will not give the page clipboard access", "no clipboard API → must say so, not no-op"},
{"clipboard write was refused", "a rejected writeText → must never claim success"},
{"Could not copy the credential", "a failed fetch → must surface the status"},
{"Copied ", "a SUCCESSFUL copy must confirm, or the operator cannot tell it worked"},
} {
if !strings.Contains(body, want.frag) {
t.Errorf("missing outcome message %q (%s)", want.frag, want.why)
}
}
// The confirmation must NAME the host: the clipboard is fleet-wide and every box has a different
// console password, so "copied" alone cannot say copied for WHICH box — the exact confusion that
// produced the incident.
if !strings.Contains(body, "' + hostID + '") {
t.Error("the copy confirmation does not name the host it copied for")
}
}
// --- Scenario B: reveal delivers the secret, records exactly one event, and never logs it ---
// RED-PROOF B: delete the SaveEvent call in handleHostRevealRecoveryCredential → the event
// assertion goes RED.