[fix] the migration defects one real machine move turned up, and the panel refresh button - #26
Open
YJack0000 wants to merge 6 commits into
Open
[fix] the migration defects one real machine move turned up, and the panel refresh button#26YJack0000 wants to merge 6 commits into
YJack0000 wants to merge 6 commits into
Conversation
The header button only ever re-fetched the tool board, so on the key vault and the MCP matrix it spun, moved the timestamp and left the table that was loaded on mount. A header that already names the view you are in has to refresh that view, so the button and the 30s poll both bump a counter every view reads. A failed re-read of the MCP matrix also used to replace a working board with an error banner; it now keeps the matrix and puts the banner above it, which matters more once a re-read happens on a timer. The panel had no tests at all. vitest + Testing Library, mocked at `invoke` because that is the one thing genuinely absent from a test process, with four regressions that fail if either fix is reverted.
A real move of 64 keys onto a second Mac wrote all 53 credential files, had every one of the 64 keychain writes refused because the session had no desktop login, and exited 0. The reason was recorded per key and dropped by the printer, so the output was a column of bare `skip` with nothing that explained it. `pb import` now probes the keystore before it copies a file and refuses the whole import if a write will not be taken; a refused key prints its reason like a file always did, plus a summary saying the bundle still holds the values; and an import that dropped secrets exits 1. `--keys-only` finishes the vault half once the session is fixed. Two more things the same move turned up. The bundle`s SETUP.md explains how to install patchbay on a machine that has none, from inside the encrypted payload — so `pb export` now writes a cleartext `<bundle>-SETUP.md` beside it, carrying the install step and nothing about the machine; it takes no manifest, which is why no inventory can leak into it. And `pb plan` now re-derives the `KUBECONFIG` line for kubeconfigs an import left unmerged, which the import knew and the plan had been throwing away. `--passphrase-file` and `--passphrase-fd` replace the refusal to read a passphrase from anything but a terminal. The reason behind that refusal — argv is visible to `ps` and lands in shell history — is kept, but a TTY test is the wrong instrument for it: it stops no attacker, and it pushed a real move into a pty wrapper whose echo wrote the passphrase into a log, which is the leak the rule exists to prevent. A passphrase file the group or the world can read is refused, because with that flag the file is the secret.
…not a verdict Cloudflare`s /user/tokens/verify only answers for user-owned tokens. An account-owned or scoped token gets the same "Invalid API Token" rejection whether it is live or revoked, and patchbay believed it — telling the user to replace nine working keys in one real vault, which is the mistake the verdict enum`s own doc comment warns about. A rejection now falls back to listing accounts with the token: if that works, the token demonstrably does. If it does not, the answer is the new `inconclusive` verdict, because at that point a revoked token and a narrowly scoped one are the same HTTP response and asserting either would be a guess. A GitHub App private key had the same false alarm from `GET /user`, which can never authenticate a PEM; a PEM-shaped value is now `unsupported` before any request is made. `pb key verify` also took exactly one id, which in a vault of 64 means nobody checks. It now takes a list or `--all`, sweeps bounded-concurrent, and exits 1 for bad news, 2 for a provider it could not reach, 0 otherwise — `inconclusive` and `unsupported` are in that 0, because neither is a fact about the key.
✅ SonarQube Quality Gate passed — pathorsAI_patchbay0 open issues on this PR. |
…lexity ceiling Sonar failed the gate on five new violations. Two were the panel props not being marked read-only. The other three were functions this PR grew past a cognitive complexity of 15, each along a seam that was already visible: `verify` served two jobs that print differently — one named key and an --all sweep — with resolution, the bounded-concurrent run, the write-back and the exit code tangled between them; those are five helpers now and it is a dispatcher (21 -> 5). `print_import` was a run of independent sections, so it is a run of section printers (17 -> 0). The refused-key count is returned by the only section that can count it rather than declared at the top and threaded through. `tool_items` asked four separate questions about one tool — is it installed, is it logged in, what did the old machine have, is it logged in as somebody else — and each is a builder now (23 -> 2), following the precedent the kubeconfig helpers already set. No behaviour changed anywhere: same output bytes, same item ids and order, same exit codes. Nothing was suppressed with #[allow] and sonar-project.properties is untouched.
It went red on the Linux runner, and it turns out to fail 15 times in 40 runs locally. Nothing in this PR touches it: the assertion was always racy. `lookup_latest` reads the rate-limit gate when a lookup starts, and the run dispatches up to four at once, so three GitHub tools can all pass the gate before the first 403 arms it. "Total calls < 3" is therefore not a property the code has — the number depends on what was already in flight. The old test just usually got lucky. So it now asserts what is actually guaranteed and is deterministic: no tool asks twice, every entry explains the rate limit, and — in a new test at the `lookup_latest` level — a lookup that starts after the limit was seen makes no request at all and says it was skipped. That last one is the half that matters anyway, since it is what stops the next run from spending a budget that is already gone. Renamed the old test, whose name claimed the guarantee that moved to the new one. Confirmed 60/60 green after the change.
This was referenced Sep 13, 2026
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.
Everything here came out of one real task: moving 64 keys and 53 credential files from this laptop to a second Mac. Every defect below was hit in that hour, in this order.
What was wrong
The import produced a half-migrated machine and called it success. All 53 credential files landed, all 64 keychain writes were refused (the session had no desktop login), and it exited 0. The per-key reason existed in the report and the printer dropped it, so the output was a column of bare
skipwith nothing explaining it.Fixed in three places, because the failure had three halves:
pb importprobes the keystore before copying a file and refuses the import if a write will not be taken. 53 files followed by 64 dropped secrets was the worst reachable outcome; it is no longer reachable.pb import && ./somethingwas the one caller that could not tell.pb import <bundle> --keys-onlyfinishes the vault half once the session is fixed.The install instructions were encrypted with the tool they install. A bundle's
SETUP.mdsays how to install patchbay on a machine that has none, from inside the encrypted payload. On this move the receiving Mac had neither, and the command was looked up on the releases page by hand.pb exportnow writes a cleartext<bundle>-SETUP.mdbeside the bundle: install step andpb importline, nothing about the machine. It takes noManifestargument at all, which is the structural reason no inventory can leak into it.The TTY gate on the passphrase caused the leak it exists to prevent.
pb exportrefused a passphrase from anything but a terminal. The reason behind it (argv is visible topsand lands in shell history) is right and is kept — but a TTY test is the wrong instrument for it. It stops no attacker, and here it pushed the work into a pty wrapper whose echo wrote the passphrase into a log.--passphrase-fileand--passphrase-fdnow exist, the way gpg, restic, borg and age do it. A passphrase file the group or the world can read is refused: with that flag, the file is the secret. Still no passphrase as an argument.Two working keys were reported dead. Cloudflare's
/user/tokens/verifyonly answers for user-owned tokens, so an account-owned or scoped token gets the same "Invalid API Token" rejection whether it is live or revoked, and patchbay believed it — nine keys in this vault were red for no reason. A rejection now falls back to listing accounts with the token; if that also fails, the answer is the newinconclusiveverdict rather than a death sentence, since at that point a revoked token and a narrowly scoped one are the same HTTP response. A GitHub App PEM had the same false alarm fromGET /user, which can never authenticate a PEM: PEM-shaped values are nowunsupportedwithout making the request.pb key verifytook exactly one id, which in a vault of 64 means nobody checks. It now takes a list or--all, sweeps bounded-concurrent, and exits 1 for bad news / 2 for unreachable / 0 otherwise, withinconclusiveandunsupportedin that 0 because neither is a fact about the key.The panel's refresh button did nothing on two of its three pages. It only re-fetched the tool board, so on the key vault and the MCP matrix it spun, moved the timestamp and left the table loaded on mount. A failed re-read of the MCP matrix also replaced a working board with a banner; it now keeps the board and puts the banner above it, which matters more now that a re-read happens on a timer.
pb planthrew away the one fix the import knew — theKUBECONFIGline for kubeconfigs that landed unmerged. The item now re-derives it from the files, so nothing has to be persisted to stay true.Plus one wart found on the way:
pb key verifyoffered Grafana's--endpointadvice to everyunsupportedprovider, including the 52 here with no endpoint to set.Verification
cargo test --release: 100 (cli) + 663 (core) + 57 (mcp) + 1 doc-test, 0 failed.cargo fmt --all --checkandcargo clippy --workspace --all-targetsclean.invoke, with four regressions that were confirmed to fail when either panel fix is reverted.$HOMEs: both passphrase sources, dry and real imports, a mode-644 passphrase file refused, and a fake home with two loose kubeconfigs producing the realexport KUBECONFIG=…line.pb key verify --allagainst the live vault: 64 keys swept in seconds.Follow-ups, not in this PR
SecurityCliKeystore::getusesfind-generic-password -w, andsecurityprints hex whenever the password contains a newline. Confirmed on 4 of 64 keys (three Apple ASC.p8files and a GitHub App PEM): whatpb key runandpb key copyhand out is 3350 hex characters, not the 1675-byte PEM. This is the real reason that key verifies as dead, and it is worse than a verify problem — anything spending those keys gets a corrupted value. Not fixed here on purpose: blind hex-decoding on read is unsafe, since a genuine 64-character hex API key is indistinguishable fromsecurity's output (six keys in this vault are legitimately hex-shaped). The fix is thesecurity-frameworkrewrite theTODOinkeystore.rsalready calls for, which also removes the secret from theadd-generic-password -w <secret>argv it currently passes.pb key run/getover a session with no desktop login fails withkeychain lookup … failed: no output, which is the same condition the import now names properly.