diff --git a/CHANGELOG.md b/CHANGELOG.md index ad65028..8bfb556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **The panel writes to the key vault** — `add key` opens a form (id, provider, + label, a masked secret field, with purpose, scopes, expiry, endpoint and the + rotation checkbox folded away), and every row gets a trash affordance behind + an inline confirm that says what removing does *not* do: the entry and its + keychain value go, the credential keeps working until you revoke it at the + provider. Both commands (`key_add`, `key_remove`) are thin wrappers over the + same `KeyRegistry` calls the CLI makes, so the registry's rules — duplicate id + refused unless you are rotating, empty secret refused, both-or-neither writes — + and its error strings reach the panel verbatim. The vault view no longer tells + you to go and use the command line. The secret exists in the field, the invoke + payload and `KeyRegistry::add`, and nowhere else: it is cleared on submit, + never logged, never echoed back. The panel still cannot *read* a value — + `get_secret` is not wired up, there is no reveal and no copy, and `pb key copy` + remains the only way one leaves the vault. The old CLI-only rule was about + argv (`ps`, shell history), and a password field in a native window has + neither. + ## [0.2.0] - 2026-08-13 ### Added diff --git a/app/src-tauri/Cargo.toml b/app/src-tauri/Cargo.toml index 25b0def..2dd15f1 100644 --- a/app/src-tauri/Cargo.toml +++ b/app/src-tauri/Cargo.toml @@ -23,3 +23,7 @@ tauri = { version = "2", features = [] } serde = { version = "1", features = ["derive"] } serde_json = "1" tokio = { version = "1", features = ["rt-multi-thread"] } + +# Standalone: keeps cargo from adopting an outer checkout's workspace when this +# app is built from a nested git worktree (.claude/worktrees/...). +[workspace] diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index dbddc83..41c1b69 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -4,6 +4,8 @@ //! and (tier 2) spawn the tools' own CLIs, so each call runs on the blocking //! pool — the webview never waits on the async runtime's worker threads. +use chrono::{DateTime, NaiveDate, NaiveTime, TimeZone, Utc}; +use patchbay_core::keys::NewKey; use patchbay_core::{ KeyEntry, KeyExpiryState, KeyRegistry, McpClient, McpClientRegistry, PermissionsReport, Registry, SwitchOutcome, ToolStatus, VerifyOutcome, @@ -80,8 +82,16 @@ async fn permissions(tool: String) -> CmdResult { /// markers on the board can never disagree about what "expiring soon" means. /// /// Metadata only, by construction: [`KeyEntry`] has never carried the secret -/// value — only its `last4`. [`KeyRegistry::get_secret`] is deliberately *not* -/// wired up as a command; the panel displays keys, it never needs to hold one. +/// value — only its `last4`. +/// +/// The panel takes a secret exactly once — [`key_add`], from a masked field, +/// held in memory for the length of one call and handed straight to the +/// keychain. It never reads one back: [`KeyRegistry::get_secret`] is +/// deliberately *not* wired up as a command, there is no reveal and no copy in +/// this window, and `pb key copy` stays the only way a value gets out of the +/// vault. Writing is easy, reading is not — that asymmetry is the design, and +/// the CLI-only rule it replaced was about argv (`ps`, shell history), which a +/// masked field in a native app does not touch. #[derive(serde::Serialize)] struct KeyRow { #[serde(flatten)] @@ -106,6 +116,109 @@ async fn keys_list() -> CmdResult> { .await } +/// What [`key_remove`] reports back: enough to name the key that is gone in a +/// confirmation, and nothing else. `last4` is the same four characters the +/// table was already showing. +#[derive(serde::Serialize)] +struct RemovedKey { + id: String, + last4: String, +} + +/// A date the panel's form accepts: `YYYY-MM-DD` (UTC midnight) or a full +/// timestamp. Reimplemented rather than shared with the CLI's `--expires` +/// parser — the panel does not depend on `patchbay-cli` and is not about to +/// start for six lines. Both go through `patchbay_core::util::parse_timestamp`, +/// which is where the timestamp knowledge actually lives. +fn parse_expiry(raw: &str) -> anyhow::Result> { + let raw = raw.trim(); + if let Ok(date) = NaiveDate::parse_from_str(raw, "%Y-%m-%d") { + return Ok(Utc.from_utc_datetime(&date.and_time(NaiveTime::MIN))); + } + patchbay_core::util::parse_timestamp(raw) + .ok_or_else(|| anyhow::anyhow!("could not read `{raw}` as a date; try `2027-01-01`")) +} + +/// Trimmed, or `None` when the field was left blank — an empty optional field +/// is an absent one, not an empty string in the registry. +fn some_text(raw: Option) -> Option { + raw.map(|s| s.trim().to_string()).filter(|s| !s.is_empty()) +} + +/// Register a key: metadata to `keys.json`, value to the keychain, both or +/// neither — [`KeyRegistry::add`] owns that guarantee and its rules (duplicate +/// id refused unless `overwrite`, empty secret refused) are surfaced to the +/// panel verbatim rather than re-litigated here. +/// +/// `secret` is the one piece of credential material that crosses this boundary. +/// It is moved into the blocking closure, borrowed once by `add`, and dropped +/// there. It is never logged, never stored, and never put in an error: every +/// message the panel can show comes from the registry, which only ever knew the +/// metadata. +#[tauri::command] +#[allow(clippy::too_many_arguments)] +async fn key_add( + id: String, + provider: Option, + label: Option, + purpose: Option, + scopes: Vec, + expires: Option, + endpoint: Option, + secret: String, + overwrite: bool, +) -> CmdResult { + off_thread(move || { + let id = id.trim().to_string(); + let expires_at = some_text(expires) + .as_deref() + .map(parse_expiry) + .transpose()?; + let new = NewKey::new( + id.as_str(), + // Where the entry came from, for the `source` column. The vault has + // said `"gui"` since it was written; this is the first thing to use it. + "gui", + ) + .provider(some_text(provider).unwrap_or_else(|| "unknown".to_string())) + .label(some_text(label).unwrap_or_else(|| id.clone())) + .purpose(some_text(purpose)) + .scopes( + scopes + .into_iter() + .filter_map(|s| some_text(Some(s))) + .collect(), + ) + .expires_at(expires_at) + .endpoint(some_text(endpoint)); + + let registry = KeyRegistry::detect()?; + let entry = registry.add(new, &secret, overwrite)?; + drop(secret); + + Ok(KeyRow { + expiry_state: entry.expiry_state(Utc::now()), + entry, + }) + }) + .await +} + +/// Unregister a key: the metadata row and the keychain item both go. Removing +/// is not revoking — the credential itself keeps working, which is why the +/// panel says so before it asks. +#[tauri::command] +async fn key_remove(id: String) -> CmdResult { + off_thread(move || { + let entry = KeyRegistry::detect()?.remove(id.trim())?; + Ok(RemovedKey { + id: entry.id, + last4: entry.last4, + }) + }) + .await +} + /// Every MCP client patchbay knows about, present or not — the absent ones are /// the point of the matrix as much as the present ones. Server entries carry /// env var *names* and header *names* only; core never reads the values. @@ -143,6 +256,8 @@ pub fn run() { verify_profile, permissions, keys_list, + key_add, + key_remove, mcp_list ]) .run(tauri::generate_context!()) diff --git a/app/src/api.ts b/app/src/api.ts index 67a7fca..fca376f 100644 --- a/app/src/api.ts +++ b/app/src/api.ts @@ -2,7 +2,9 @@ import { invoke } from "@tauri-apps/api/core"; import type { KeyRow, McpClient, + NewKeyInput, PermissionsReport, + RemovedKey, SwitchOutcome, ToolStatus, VerifyOutcome, @@ -26,7 +28,22 @@ export const verifyProfile = (tool: string, profile: string) => export const permissions = (tool: string) => invoke("permissions", { tool }); -/** Vault metadata. Read-only, and there is no command that returns a value. */ +/** Vault metadata. There is no command that returns a value — see `keyAdd`. */ export const keysList = () => invoke("keys_list"); +/** + * Register a key. The secret is a separate argument on purpose: it belongs to + * no object, is never held in state alongside the metadata, and exists only + * between the form field and this call. The backend hands it to the same + * `KeyRegistry::add` the CLI uses and drops it. + * + * Values only ever travel in this direction. Nothing in the panel reads one + * back — `pb key copy ` remains the only way out of the vault. + */ +export const keyAdd = (key: NewKeyInput, secret: string) => + invoke("key_add", { ...key, secret }); + +/** Unregister a key: metadata row and keychain item both. Not a revocation. */ +export const keyRemove = (id: string) => invoke("key_remove", { id }); + export const mcpList = () => invoke("mcp_list"); diff --git a/app/src/components/Glyphs.tsx b/app/src/components/Glyphs.tsx index a8b79ef..cc4c17f 100644 --- a/app/src/components/Glyphs.tsx +++ b/app/src/components/Glyphs.tsx @@ -12,7 +12,7 @@ */ /** Registered vault keys. */ -export function KeyGlyph({ size = 11 }: { size?: number }) { +export function KeyGlyph({ size = 11 }: Readonly<{ size?: number }>) { return ( ) { + return ( + + ); +} + /** The MCP servers-against-clients matrix. */ -export function MatrixGlyph({ size = 11 }: { size?: number }) { +export function MatrixGlyph({ size = 11 }: Readonly<{ size?: number }>) { return ( ` is still the only way a value + * leaves the vault — the asymmetry the CLI was built around survives intact. + * + * The old rule was "adding happens on the command line", and its reason was + * argv: a secret passed as an argument is visible in `ps` and written verbatim + * into shell history. Neither is true of a password field in a native window, + * so the rule was protecting nothing here and cost the panel the one action a + * key vault is for. */ export function KeysView() { const [rows, setRows] = useState(null); const [error, setError] = useState(null); + const [adding, setAdding] = useState(false); + /** The row whose delete is armed. One at a time, and never pre-armed. */ + const [confirming, setConfirming] = useState(null); + const [removing, setRemoving] = useState(null); + /** The outcome of the last write, in the panel's usual note voice. */ + const [note, setNote] = useState<{ text: string; bad: boolean } | null>(null); - useEffect(() => { - let live = true; - keysList() - .then((r) => live && setRows(r)) - .catch((e) => live && setError(String(e))); - return () => { - live = false; - }; + const load = useCallback(async () => { + try { + setRows(await keysList()); + setError(null); + } catch (e) { + setError(String(e)); + } }, []); - if (error) { + useEffect(() => { + void load(); + }, [load]); + + const remove = async (id: string) => { + setRemoving(id); + setNote(null); + try { + const gone = await keyRemove(id); + setNote({ + text: `removed ${gone.id} (··${gone.last4}) — patchbay has forgotten it; the credential itself keeps working until you revoke it at the provider`, + bad: false, + }); + setConfirming(null); + await load(); + } catch (e) { + // Core's errors say what half-state the vault is in; do not paraphrase. + setNote({ text: String(e), bad: true }); + } finally { + setRemoving(null); + } + }; + + const added = async (row: KeyRow) => { + setAdding(false); + setNote({ text: `registered ${row.id} (··${row.last4}) — value in the OS keychain`, bad: false }); + await load(); + }; + + if (error && !rows) { return (
△ @@ -37,30 +79,6 @@ export function KeysView() { if (!rows) return

reading the vault…

; - if (rows.length === 0) { - return ( -
-

no keys registered

-

- The vault holds API keys that no CLI tracks — a Cloudflare token used from a script, a - provider key an agent was handed. Metadata lives in a JSON file; the value goes straight - to the OS keychain. -

- {/* patchbay runs its own actions rather than handing out commands, and - this is the deliberate exception: registering a key means typing the - secret somewhere, and the panel is not that somewhere. Nothing here - ever accepts or displays a value, which is the whole point. */} -
- - Adding a key has to happen on the command line: it means typing the secret itself, and - the panel deliberately never takes one. - - -
-
- ); - } - return (
@@ -68,42 +86,392 @@ export function KeysView() { {rows.length} {rows.length === 1 ? "key" : "keys"} · metadata only +
-
- - - - - - - - - - - - - {rows.map((k) => ( - - - - - {/* The only thing on this page derived from a secret value. */} - - - + {error && ( +
+ △ + {error} +
+ )} + + {note && ( +
+ {note.text} +
+ )} + + {rows.length === 0 ? ( +
+

no keys registered

+

+ The vault holds API keys that no CLI tracks — a Cloudflare token used from a script, a + provider key an agent was handed. Metadata lives in a JSON file; the value goes straight + to the OS keychain, and nothing in this window can read it back. +

+ +

+ Already in a terminal? The same registration, with the secret on stdin so it never + reaches argv or your shell history: +

+ +
+ ) : ( +
+
idproviderlabellast 4expirypurpose
{k.id}{k.provider}{k.label}··{k.last4} - - {KEY_EXPIRY_LABEL[k.expiry_state]} - - {k.purpose ?? —}
+ + + + + + + + + - ))} - -
idproviderlabellast 4expirypurpose
-
+ + + {rows.map((k) => ( + + + {k.id} + {k.provider} + {k.label} + {/* The only thing on this page derived from a secret value. */} + ··{k.last4} + + + {KEY_EXPIRY_LABEL[k.expiry_state]} + + + {k.purpose ?? —} + + + + + {confirming === k.id && ( + + +
+ + Remove {k.id}? This removes the entry and its keychain value; the + credential itself keeps working — revoke it at the provider. + + + +
+ + + )} + + ))} + + +
+ )} + + {adding && setAdding(false)} onAdded={added} />}
); } + +/** + * The one place in patchbay's UI that takes a secret. + * + * It lives in component state for as long as the drawer is open and is cleared + * on every submit; closing the drawer unmounts the component and takes it with + * it. It is never logged, never put in the success note, and never written + * anywhere but the `key_add` payload. + * + * Validation is core's job — the registry's messages about slugs, duplicate ids + * and empty secrets are written to be read, so they are shown verbatim rather + * than pre-empted. The only client-side rules are the two that decide whether + * pressing the button could possibly work. + */ +function AddKeyForm({ + onClose, + onAdded, +}: Readonly<{ + onClose: () => void; + onAdded: (row: KeyRow) => void | Promise; +}>) { + const [id, setId] = useState(""); + const [provider, setProvider] = useState(""); + const [label, setLabel] = useState(""); + const [secret, setSecret] = useState(""); + const [more, setMore] = useState(false); + const [purpose, setPurpose] = useState(""); + const [scopes, setScopes] = useState(""); + const [expires, setExpires] = useState(""); + const [endpoint, setEndpoint] = useState(""); + const [overwrite, setOverwrite] = useState(false); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + const onKey = (e: globalThis.KeyboardEvent) => e.key === "Escape" && onClose(); + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [onClose]); + + const ready = id.trim().length > 0 && secret.length > 0 && !busy; + + const submit = async (e: FormEvent) => { + e.preventDefault(); + if (!ready) return; + setBusy(true); + setError(null); + // Taken out of state before the await, so the value the request carries is + // a local that dies with this function whatever the backend answers. + const value = secret; + setSecret(""); + try { + const row = await keyAdd( + { + id: id.trim(), + provider: provider.trim(), + label: label.trim(), + purpose: purpose.trim() || null, + scopes: scopes + .split(",") + .map((s) => s.trim()) + .filter(Boolean), + expires: expires.trim() || null, + endpoint: endpoint.trim() || null, + overwrite, + }, + value, + ); + await onAdded(row); + } catch (err) { + setError(String(err)); + } finally { + setBusy(false); + } + }; + + return ( + <> + {/* Click-outside is a real button, not a div with a handler: it is a + control, so it answers to the keyboard and screen readers like one. + `.scrim` resets a button's chrome, and the div ToolDetail uses is + unaffected by the reset. */} + + + +
void submit(e)}> + + + + + + + + +
+ +
+ + {more && ( + <> + + + + + + + + + + + )} + + {/* Core's own sentence, unedited: it names the id, says what it + refused and what to do instead. */} + {error && ( +
+
+ △ + {error} +
+ + the secret field was cleared — paste it again to retry + +
+ )} + +
+ + +
+
+ + + ); +} diff --git a/app/src/styles.css b/app/src/styles.css index 5e4f374..e437bc2 100644 --- a/app/src/styles.css +++ b/app/src/styles.css @@ -719,10 +719,25 @@ button { /* ---------- tool detail ---------- */ +/* A wash over the board while a drawer is open. The add-key drawer's scrim is + a real