From 3d5c54c01b10d93d803479471f289c67cd6419e7 Mon Sep 17 00:00:00 2001 From: Mathias Picker <48158184+MathiasWP@users.noreply.github.com> Date: Thu, 27 Aug 2026 14:22:37 +0200 Subject: [PATCH] Let a sign-in reach a containerised MCP server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under ToolHive the server read its credentials from FIBER_SECRETS once at startup, so a container held whatever was true when it began: you signed in, the keychain got the new token, and the server went on presenting the expired one until someone re-exported the secrets and replaced the workload. Credentials can now travel through the collections directory the container already mounts. The app rewrites that file whenever a credential changes and the server re-reads it, so the next call picks the new value up. The file is sealed with XChaCha20-Poly1305 and the key stays out of the mount — keychain on the app's side, ToolHive's encrypted store on the container's — so a copy of it alone is inert. Its existence is the opt-in: toolhive.sh creates it, the app writes to it only if it is already there, and deleting it opts back out. Bearer collections needed a second fix to benefit. A static token cannot be refreshed by replaying a request, so a 401 never dropped it, and a zero-TTL cache entry has nothing else to expire it — a container would have presented its startup token for the life of the workload even with a live file. A rejected credential is now dropped whenever it came from a source that can change underneath the process. The desktop app is unaffected: it has no such source, and the same line there would cost a keychain prompt per 401. --- .changeset/live-container-credentials.md | 30 +++ Dockerfile | 2 +- README.md | 6 +- deploy/toolhive.md | 108 +++++--- scripts/toolhive.sh | 41 ++- src-tauri/Cargo.lock | 120 ++++++++- src-tauri/Cargo.toml | 1 + src-tauri/src/lib.rs | 33 ++- src-tauri/src/main.rs | 34 ++- src-tauri/src/mcp.rs | 260 ++++++++++++++++++- src-tauri/src/secrets.rs | 315 ++++++++++++++++++++--- src-tauri/src/send.rs | 117 ++++++++- 12 files changed, 968 insertions(+), 99 deletions(-) create mode 100644 .changeset/live-container-credentials.md diff --git a/.changeset/live-container-credentials.md b/.changeset/live-container-credentials.md new file mode 100644 index 0000000..4c4cb17 --- /dev/null +++ b/.changeset/live-container-credentials.md @@ -0,0 +1,30 @@ +--- +"fiber": minor +--- + +Signing in again now reaches a running containerised MCP server. + +Under ToolHive the server took its credentials from `FIBER_SECRETS`, read once +at startup, so a container held whatever was true when it began: you signed in, +the keychain got the new token, and the server went on presenting the expired +one until someone re-exported the secrets and replaced the workload. + +Credentials can now travel through the collections directory the container +already mounts. The app rewrites that file whenever a credential changes, the +server re-reads it, and the 401 retry that was already there picks the new value +up — no re-export, no restart. + +The file is sealed with XChaCha20-Poly1305 and the key stays out of the mount: +in the keychain on the app's side, in ToolHive's encrypted store on the +container's. Its existence is the opt-in, so a desktop-only install never has +credentials on disk. New: `fiber mcp file-key` and `fiber mcp export-secrets +--to `; `scripts/toolhive.sh` wires both up for you. + +Bearer collections needed a second fix to benefit: a static token cannot be +refreshed by replaying a request, so a 401 never dropped it, and a zero-TTL +cache entry has nothing else to expire it — a container would have presented +the token it started with for the life of the workload. A rejected credential +is now dropped from the cache whenever it came from a source that can change +underneath the process, so the next call reads the new one. The desktop app is +unaffected: it has no such source, and the same line there would have cost a +keychain prompt per 401. diff --git a/Dockerfile b/Dockerfile index f012ef0..a7f99a3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -29,7 +29,7 @@ ARG TARGETARCH # --no-default-features drops Tauri entirely (see Cargo.toml [features]) — and # with it the Linux keychain, whose D-Bus backend needs libdbus here and a # session bus at runtime, neither of which a container has. Secrets come from -# FIBER_SECRETS instead. The musl target links the CRT statically by default, so +# FIBER_SECRETS, or from the FIBER_SECRETS_FILE the app keeps current, instead. The musl target links the CRT statically by default, so # the result needs no libc. # # The binary is copied to a fixed path because the target triple is not known to diff --git a/README.md b/README.md index cac268c..dce0358 100644 --- a/README.md +++ b/README.md @@ -112,8 +112,10 @@ in memory from then on. The single exception is `fiber mcp export-secrets`, which exists so that a containerised copy can be given the credentials it cannot fetch itself — see [`deploy/toolhive.md`](deploy/toolhive.md). It covers only the collections you -have shared over MCP, it writes to a pipe and refuses a terminal, and you have -to run it deliberately. +have shared over MCP, it writes to a pipe or an encrypted file and refuses a +terminal, and you have to run it deliberately. Once you have set that up, the +app keeps the file current as you sign in, so a container stops going stale; +without it, nothing is ever written to disk. A header you type on a request beats the collection's auth — for "just this once, use a different token". `Cookie` is the exception, and has to be: cookies diff --git a/deploy/toolhive.md b/deploy/toolhive.md index b38b327..bb0ff1f 100644 --- a/deploy/toolhive.md +++ b/deploy/toolhive.md @@ -21,10 +21,10 @@ rather than the desktop app's own collections, name it: curl -fsSL .../toolhive.sh | bash -s -- ~/work/api-collections ``` -The script finds your collections directory, moves the credentials for the -collections you have shared into ToolHive's secret store, and starts the server. -Rerunning it replaces the workload and refreshes the credentials, which is what -you want after signing in again. +The script finds your collections directory, sets up the credentials for the +collections you have shared, and starts the server. Signing in again in Fiber +reaches the running server on its own — see [Credentials](#credentials) — so +rerunning this is for changing what you serve, not for refreshing a token. The image is published by the release workflow for `linux/amd64` and `linux/arm64`, so there is nothing to build and nothing to push. ToolHive pulls @@ -64,6 +64,9 @@ section files. Two things a section needs to be usable over MCP: - for authenticated sections, a `secretRef` — the app writes `":auth"`. That exact string is the key you provide below. +`/data` is also where the credentials file lives when the app is keeping one +current, which is why that half needs no separate mount. + `/data` should be **writable and persistent**: loader caches, request history and spilled response bodies are written there, and `query_response` reads a stored body back, so it needs to survive between tool calls. The image runs as the @@ -81,58 +84,97 @@ image. The desktop app keeps secrets in the OS keychain and the section file holds only a reference. A container can reach neither, so the headless build takes them -from the environment instead: `FIBER_SECRETS` is a JSON object of -`reference → value`, and `FIBER_SECRETS_FILE` is a path to a file holding the -same. Both are unset in the desktop app, which still uses only the keychain. +from the environment instead. There are two ways in, and they differ in one +thing that matters a lot in practice: whether signing in again reaches a server +that is already running. + +### The file the app keeps current (what the script sets up) -Building that map by hand is the one genuinely tedious part, so the app will -write it for you: +`FIBER_SECRETS_FILE` points at a file of `reference → value`, and the server +re-reads it whenever it needs a credential. Put that file in the directory you +already mount and the desktop app will keep it up to date as you work: sign in +again, and the next tool call picks the new token up. No re-export, no restart. + +That mount is the only channel the two halves share — the app cannot write to +ToolHive's secret store, and the container cannot read the keychain — so the +file is encrypted rather than plain, with `FIBER_SECRETS_KEY`. The key stays out +of the mount: in the keychain on the app's side, in ToolHive's encrypted store +on the container's. A copy of the file on its own is inert, and a tampered one +fails to open rather than decrypting to something else. ```sh -/Applications/Fiber.app/Contents/MacOS/fiber mcp export-secrets | - thv secret set fiber-secrets +/Applications/Fiber.app/Contents/MacOS/fiber mcp file-key | thv secret set fiber-key +/Applications/Fiber.app/Contents/MacOS/fiber mcp export-secrets --to \ + "$HOME/Library/Application Support/dev.fiber.app/mcp-secrets.enc" + +thv run --name fiber --transport stdio -v /path/to/your/collections:/data \ + --secret fiber-key,target=FIBER_SECRETS_KEY \ + --env FIBER_SECRETS_FILE=/data/mcp-secrets.enc \ + ghcr.io/mathiaswp/fiber-mcp:latest ``` -It emits `{"": ""}` for every collection you have shared over -MCP — and only those, so it hands out nothing an agent could not already use. It -writes to stdout and refuses to run into a terminal, so the credentials go down -the pipe into ToolHive's encrypted store without touching a file, a shell -variable or your scrollback. It is the only thing in Fiber that reads a secret -back out of the keychain; macOS may ask you to approve each one. +`file-key` creates the key on first use and returns the same one thereafter, so +rerunning any of this is safe: the key is long-lived and the values rotate +underneath it. -If you have no app on the machine, the same map typed by hand does the same job: +**The file's existence is the opt-in.** The app writes to it only if it is +already there, so a desktop-only user never has credentials on disk, and +deleting the file opts back out. -```sh -thv secret set fiber-secrets -# paste, e.g.: {"acme-api:auth":"eyJhbGciOi...","stripe:auth":"sk_live_..."} -``` +Both commands refuse to run into a terminal, so the key and the credentials go +down a pipe or into a `0600` file rather than into your scrollback. Reading +secrets back out of the keychain is the one thing nothing else in Fiber does; +macOS may ask you to approve each one. -Either way, one flag on the run command uses it: +### The snapshot (`FIBER_SECRETS`) + +`FIBER_SECRETS` is a JSON object of `reference → value` in the environment. It +is simpler, and it is what to use when there is no app on the machine to keep a +file current — a collections repo on a server, say. ```sh +/Applications/Fiber.app/Contents/MacOS/fiber mcp export-secrets | + thv secret set fiber-secrets + thv run --name fiber --transport stdio -v /path/to/your/collections:/data \ --secret fiber-secrets,target=FIBER_SECRETS \ ghcr.io/mathiaswp/fiber-mcp:latest ``` +A process's environment cannot change under it, so this is a **snapshot taken +when the workload started**. Sign in again and the container will go on +presenting the old credential until you re-export and replace the workload — +rerunning `toolhive.sh` does both. That is the behaviour the file above exists +to avoid. + +If you have no app on the machine, the same map typed by hand does the same job: + +```sh +thv secret set fiber-secrets +# paste, e.g.: {"acme-api:auth":"eyJhbGciOi...","stripe:auth":"sk_live_..."} +``` + For a login-request section the value is the request body (`{"user":"…","password":"…"}`); for bearer/browser sections it's the token or cookie string — exactly what the app would have put in the keychain. -If you'd rather not manage a JSON blob, mount a file and set -`--env FIBER_SECRETS_FILE=/run/secrets/fiber.json` instead of the `--secret` -line. +`FIBER_SECRETS` wins over the file if you somehow set both. An unencrypted +`FIBER_SECRETS_FILE` still works when `FIBER_SECRETS_KEY` is unset, for a file +you manage yourself; setting the key and pointing it at a plaintext file is an +error rather than a silent downgrade, and so is an encrypted file with no key. ### Why this is not as good as the keychain -Inside a container, injected secrets live in the process environment (or a -mounted file) rather than the OS keychain — that's the unavoidable cost of a +Inside a container, injected secrets live in the process environment or a +mounted file rather than the OS keychain — that's the unavoidable cost of a container that can't reach the keychain, and it's the standard container -pattern. ToolHive's encrypted secret store decrypts and injects them at runtime, -which is why `--secret` is preferable to a plain `--env`. The redaction guarantee -still holds: `authorization`, `cookie`, `set-cookie`, `proxy-authorization` and -`x-api-key` are stripped from every response the server returns, so an injected -credential can't be laundered back out through a tool result. +pattern. Encrypting the file narrows the gap: what is at rest in the mount is +ciphertext, and the key is held by ToolHive's encrypted secret store, which is +why `--secret` is preferable to a plain `--env` for it. The redaction guarantee +still holds either way: `authorization`, `cookie`, `set-cookie`, +`proxy-authorization` and `x-api-key` are stripped from every response the +server returns, so an injected credential can't be laundered back out through a +tool result. ## Building the image yourself diff --git a/scripts/toolhive.sh b/scripts/toolhive.sh index b682815..98281ee 100755 --- a/scripts/toolhive.sh +++ b/scripts/toolhive.sh @@ -18,7 +18,10 @@ set -euo pipefail # Overridable for a mirror, a pinned version, or a local build under test. IMAGE="${FIBER_IMAGE:-ghcr.io/mathiaswp/fiber-mcp:latest}" NAME="fiber" -SECRET="fiber-secrets" +# The sealing key lives in ToolHive's store; the credentials it seals live in +# the mounted collections directory, so the app can keep them current. +SECRET_KEY="fiber-key" +SECRETS_FILE="mcp-secrets.enc" die() { echo "$*" >&2 @@ -71,27 +74,39 @@ fi secret_args=() if [ -x "$app" ]; then echo "Copying credentials out of the keychain..." - # Straight down a pipe into ToolHive's encrypted store — the JSON never - # reaches a file, a shell variable or the terminal. macOS may ask for - # permission once per credential; that prompt is the keychain doing its job. + # Two pieces, and the split is the point. The *key* goes into ToolHive's + # encrypted store, where it sits unchanged for the life of the workload. The + # *credentials* go into a file inside the collections directory we are about + # to mount, sealed with that key — so signing in again in Fiber rewrites the + # file, the running container reads it on its next 401, and nothing has to be + # re-exported or restarted. Before this, a container held whatever was true + # when it started. # - # FIBER_DATA_DIR so it reports on the collections we are about to serve, - # which is not the app's own directory when a repo was named. + # The key never reaches the mount and the credentials never reach the + # terminal: each goes straight down a pipe or straight to a 0600 file. # # `< /dev/null` is not decoration. Under `curl | bash` this script *is* # bash's stdin, so a child inherits the rest of it — and a copy of Fiber too - # old to know `export-secrets` would take that for MCP traffic and sit there + # old to know these commands would take that for MCP traffic and sit there # reading. With stdin closed the worst case is an immediate empty result, # which the check below turns into an explanation. - if FIBER_DATA_DIR="$data" "$app" mcp export-secrets < /dev/null | - thv secret set "$SECRET" > /dev/null; then - secret_args=(--secret "$SECRET,target=FIBER_SECRETS") - else - echo "Could not store the credentials." >&2 + if ! "$app" mcp file-key < /dev/null | thv secret set "$SECRET_KEY" > /dev/null; then + echo "Could not store the sealing key." >&2 echo " - if ToolHive has no secrets provider yet: run 'thv secret setup'" >&2 - echo " - if Fiber said nothing about export-secrets: update it, that command is newer" >&2 + echo " - if Fiber said nothing about file-key: update it, that command is newer" >&2 + exit 1 + fi + # FIBER_DATA_DIR so it reports on the collections we are about to serve, + # which is not the app's own directory when a repo was named. + if ! FIBER_DATA_DIR="$data" "$app" mcp export-secrets --to "$data/$SECRETS_FILE" \ + < /dev/null; then + echo "Could not write the credentials file." >&2 exit 1 fi + secret_args=( + --secret "$SECRET_KEY,target=FIBER_SECRETS_KEY" + --env "FIBER_SECRETS_FILE=/data/$SECRETS_FILE" + ) else echo "Fiber is not installed here, so there are no credentials to copy." echo "Authenticated collections will need FIBER_SECRETS — see deploy/toolhive.md." diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 504ac2e..0303e28 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -8,6 +8,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "aead" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1973cfbc1a2daf9cf550e74e1f088c28e7f7d8c1e1418fb6c9dc5184b7e84c99" +dependencies = [ + "crypto-common 0.2.2", + "inout 0.2.2", +] + [[package]] name = "aes" version = "0.8.4" @@ -15,7 +25,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b169f7a6d4742236a0a00c541b845991d0ac43e546831af1249753ab4c3aa3a0" dependencies = [ "cfg-if", - "cipher", + "cipher 0.4.4", "cpufeatures 0.2.17", ] @@ -353,6 +363,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "block-buffer" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" +dependencies = [ + "hybrid-array", +] + [[package]] name = "block-padding" version = "0.3.3" @@ -525,7 +544,7 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26b52a9543ae338f279b96b0b9fed9c8093744685043739079ce85cd58f289a6" dependencies = [ - "cipher", + "cipher 0.4.4", ] [[package]] @@ -586,10 +605,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" dependencies = [ "cfg-if", + "cipher 0.5.2", "cpufeatures 0.3.0", "rand_core", ] +[[package]] +name = "chacha20poly1305" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b89e1c441e926b9c82a8d023f6e1b7ae0adcfaa7d621814e4d60789bac751cb" +dependencies = [ + "aead", + "chacha20", + "cipher 0.5.2", + "poly1305", +] + [[package]] name = "chrono" version = "0.4.45" @@ -608,8 +640,19 @@ version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" dependencies = [ - "crypto-common", - "inout", + "crypto-common 0.1.7", + "inout 0.1.4", +] + +[[package]] +name = "cipher" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8cf2a2c93cd704877c0858356ed03480ff301ee950b43f1cbe4573b088bfa6c" +dependencies = [ + "block-buffer 0.12.1", + "crypto-common 0.2.2", + "inout 0.2.2", ] [[package]] @@ -621,6 +664,12 @@ dependencies = [ "cc", ] +[[package]] +name = "cmov" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c9ea0ac24bc397ab3c98583a3c9ba74fa56b09a4449bbe172b9b1ddb016027a" + [[package]] name = "combine" version = "4.6.7" @@ -791,6 +840,17 @@ dependencies = [ "typenum", ] +[[package]] +name = "crypto-common" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453" +dependencies = [ + "getrandom 0.4.3", + "hybrid-array", + "rand_core", +] + [[package]] name = "cssparser" version = "0.36.0" @@ -830,6 +890,15 @@ version = "0.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52560adf09603e58c9a7ee1fe1dcb95a16927b17c127f0ac02d6e768a0e25bc1" +[[package]] +name = "ctutils" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e" +dependencies = [ + "cmov", +] + [[package]] name = "darling" version = "0.23.0" @@ -987,8 +1056,8 @@ version = "0.10.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ - "block-buffer", - "crypto-common", + "block-buffer 0.10.4", + "crypto-common 0.1.7", "subtle", ] @@ -1288,6 +1357,7 @@ name = "fiber" version = "0.14.10" dependencies = [ "base64 0.23.1", + "chacha20poly1305", "dirs", "futures-util", "jaq-core", @@ -1958,6 +2028,15 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" +[[package]] +name = "hybrid-array" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +dependencies = [ + "typenum", +] + [[package]] name = "hyper" version = "1.11.0" @@ -2205,6 +2284,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "inout" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4250ce6452e92010fdf7268ccc5d14faa80bb12fc741938534c58f16804e03c7" +dependencies = [ + "hybrid-array", +] + [[package]] name = "inventory" version = "0.3.24" @@ -3332,6 +3420,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "poly1305" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e2d0073b297041425c7c3df6eb4792d598a15323fe63346852b092eca02904c" +dependencies = [ + "cpufeatures 0.3.0", + "universal-hash", +] + [[package]] name = "portable-atomic" version = "1.15.0" @@ -5437,6 +5535,16 @@ version = "1.13.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c6f5d3c3b1bf09027a88a6bc961fc00497d651009560b5463668dc81b0fa87a8" +[[package]] +name = "universal-hash" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4987bdc12753382e0bec4a65c50738ffaabc998b9cdd1f952fb5f39b0048a96" +dependencies = [ + "crypto-common 0.2.2", + "ctutils", +] + [[package]] name = "unsafe-libyaml-norway" version = "0.2.15" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 8700d44..aef2199 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -78,6 +78,7 @@ dirs = "6.0.0" schemars = "1.2.2" serde_norway = "0.9.42" serde_json_path = "0.7.2" +chacha20poly1305 = { version = "0.11.0", features = ["getrandom"] } # Asking whether a keychain item exists, without reading it. keyring has no such # call — its `has` fetches the password — and on macOS that difference is a diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 7e75306..8552cf0 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -40,6 +40,7 @@ mod gui { use crate::history::{self, HistoryError, HistoryRecord, HistoryStore}; use crate::http::{BodyEvent, ChunkSink, HttpError, HttpState, RequestSpec, ResponseData}; use crate::loader::{self, LoaderError, LoaderRun}; + use crate::mcp; use crate::openapi; use crate::secrets::{self, SecretError}; use crate::send::{send_authenticated, send_authenticated_streaming}; @@ -59,6 +60,10 @@ mod gui { struct Paths { sections: PathBuf, loaders: PathBuf, + /// The root, not just the two directories under it: the credential + /// file a containerised MCP server reads lives here too. See + /// mcp::sync_secrets_file. + data: PathBuf, } /// Parsed loader caches, keyed by section id. @@ -279,8 +284,16 @@ mod gui { // call on the thread that pumps the event loop is a frozen window. Nothing // here needs to await; `async` is what moves the work off that thread. #[tauri::command] - async fn set_secret(reference: String, value: String) -> Result<(), SecretError> { - secrets::set(&reference, &value) + async fn set_secret( + paths: State<'_, Paths>, + reference: String, + value: String, + ) -> Result<(), SecretError> { + secrets::set(&reference, &value)?; + // The keychain is the record; this only mirrors the change into the + // file a containerised server reads, and only if one has been set up. + mcp::sync_secrets_file(&paths.data, &reference, Some(&value)); + Ok(()) } /// The UI can ask whether a secret exists; it can never read one back. @@ -290,8 +303,10 @@ mod gui { } #[tauri::command] - async fn delete_secret(reference: String) -> Result<(), SecretError> { - secrets::delete(&reference) + async fn delete_secret(paths: State<'_, Paths>, reference: String) -> Result<(), SecretError> { + secrets::delete(&reference)?; + mcp::sync_secrets_file(&paths.data, &reference, None); + Ok(()) } /// Forces the next send for this section to log in again. @@ -352,6 +367,10 @@ mod gui { if let Some(reference) = section.auth.secret_ref() { secrets::set(reference, &value).map_err(|err| BrowserError::Eval(err.to_string()))?; + // Signing in again is exactly the case a container used to miss: + // the keychain got the new credential and the running server went + // on presenting the expired one. + mcp::sync_secrets_file(&paths.data, reference, Some(&value)); } auth_state.invalidate(§ion_id); crate::browser::close(&app, §ion_id); @@ -648,6 +667,7 @@ mod gui { section: Section, ) -> Result<(), StoreError> { store::save(&paths.sections, §ion)?; + mcp::sync_section_sharing(&paths.data, §ion); sections.remember(section); Ok(()) } @@ -661,6 +681,10 @@ mod gui { ) -> Result<(), StoreError> { // The loader cache is derived data; it has no business outliving its section. loader::forget_cache(&paths.loaders, &id); + // Same for the credential file a container reads: the app writes + // `:auth`, so the reference is derivable without the section + // that is about to go. + mcp::sync_secrets_file(&paths.data, &format!("{id}:auth"), None); mem.forget(&id); sections.forget(&id); store::delete(&paths.sections, &id) @@ -771,6 +795,7 @@ mod gui { app.manage(Paths { sections: store::sections_dir(&app_data_dir), loaders: loader::loaders_dir(&app_data_dir), + data: app_data_dir.clone(), }); app.manage(LoaderMem::new()); app.manage(SectionMem::new()); diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index b49dbe2..ce2705f 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -19,7 +19,39 @@ fn main() { // protocol. See mcp::export_secrets for why it is allowed to do the one // thing nothing else in the app does. if std::env::args().nth(2).as_deref() == Some("export-secrets") { - if let Err(err) = fiber_lib::mcp::export_secrets() { + // `--to ` writes the sealed file a container reads live + // instead of printing the map. Same credentials, but the app can + // then keep it current as you sign in, rather than the container + // holding whatever was true when it started. + let target = match std::env::args().nth(3).as_deref() { + Some("--to") => match std::env::args().nth(4) { + Some(path) => Some(std::path::PathBuf::from(path)), + None => { + eprintln!("--to needs a path."); + std::process::exit(1); + } + }, + Some(other) => { + eprintln!("Unknown option {other}. The only one is --to ."); + std::process::exit(1); + } + None => None, + }; + let result = match &target { + Some(path) => fiber_lib::mcp::export_secrets_to(path), + None => fiber_lib::mcp::export_secrets(), + }; + if let Err(err) = result { + eprintln!("{err}"); + std::process::exit(1); + } + return; + } + + // The key that seals that file. It stays out of the mounted directory: + // the app keeps it in the keychain, the container gets it injected. + if std::env::args().nth(2).as_deref() == Some("file-key") { + if let Err(err) = fiber_lib::mcp::print_file_key() { eprintln!("{err}"); std::process::exit(1); } diff --git a/src-tauri/src/mcp.rs b/src-tauri/src/mcp.rs index e402b82..f96bc15 100644 --- a/src-tauri/src/mcp.rs +++ b/src-tauri/src/mcp.rs @@ -762,11 +762,21 @@ impl FiberMcp { hints.push("The response could not be persisted, so query_response is unavailable."); log::warn!("could not record MCP response {id}: {err}"); } - if response.status == 401 && matches!(section.auth, crate::auth::AuthConfig::Browser { .. }) - { - hints.push( - "Browser credentials cannot refresh headlessly. Sign in again in Fiber, then restart the MCP server or re-export container secrets.", - ); + // A 401 that survived the retry means re-authenticating did not help, + // and the reason depends on where the credential came from. The retry + // re-reads it (`send::send_authenticated_streaming` invalidates first), + // so a credential file that the app keeps current has already been + // consulted — which is why the advice is no longer "restart the server". + if response.status == 401 && section.auth.secret_ref().is_some() { + let browser = matches!(section.auth, crate::auth::AuthConfig::Browser { .. }); + hints.push(if browser { + "Browser credentials cannot be re-captured headlessly. Sign in again in Fiber \ + — if this server reads a credential file the app keeps current, the next call \ + picks it up; otherwise re-export its secrets." + } else { + "Re-authenticating did not help, so the stored credential is being rejected. \ + Check it in Section settings." + }); } #[derive(Serialize)] @@ -1081,6 +1091,192 @@ fn collect_secrets( exported } +/// Where the app keeps the credential file a containerised server reads. +/// +/// Deliberately inside the data directory, because that is the directory +/// ToolHive already mounts at `/data` — the app and the container have no other +/// channel, and the mount is already how collection edits reach a running +/// server. See `secrets::file_secrets` for the reading half. +pub fn secrets_file(data: &std::path::Path) -> std::path::PathBuf { + data.join(secrets::FILE_NAME) +} + +/// The key that seals that file, created on first use. +/// +/// Cached for the life of the process: on an ad-hoc signed build every keychain +/// read is a password prompt, and the app rewrites the file on every sign-in. +/// Reading once per run rather than once per write is the difference between +/// one prompt and one per credential change. +fn file_key() -> Result { + static KEY: std::sync::OnceLock> = std::sync::OnceLock::new(); + KEY.get_or_init(|| { + if let Some(existing) = crate::secrets::get(secrets::KEY_REF) { + return Ok(existing); + } + let key = secrets::new_key()?; + secrets::set(secrets::KEY_REF, &key).map_err(|err| err.to_string())?; + Ok(key) + }) + .clone() +} + +/// Prints the sealing key, for `thv secret set`. +/// +/// Same terminal guard as `export_secrets`, and for the same reason: this is a +/// key to credentials, so it goes down a pipe or nowhere. +pub fn print_file_key() -> Result<(), Box> { + use std::io::{IsTerminal, Write}; + + if std::io::stdout().is_terminal() { + return Err("This prints a key, so it only writes to a pipe.\n\ + Try: fiber mcp file-key | thv secret set fiber-key" + .into()); + } + let key = file_key()?; + let mut out = std::io::stdout(); + out.write_all(key.as_bytes())?; + out.flush()?; + Ok(()) +} + +/// Replaces the file, sealed, in one step that a reader cannot catch half-done. +/// +/// `0600` before the rename rather than after: a credential file that is +/// world-readable for even a moment is world-readable. +fn write_sealed( + path: &std::path::Path, + secrets: &serde_json::Map, + key: &str, +) -> Result<(), String> { + let sealed = + crate::secrets::seal(key, &serde_json::Value::Object(secrets.clone()).to_string())?; + + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|err| format!("{}: {err}", parent.display()))?; + } + let temporary = path.with_extension("tmp"); + std::fs::write(&temporary, sealed).map_err(|err| format!("{}: {err}", temporary.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&temporary, std::fs::Permissions::from_mode(0o600)) + .map_err(|err| format!("{}: {err}", temporary.display()))?; + } + std::fs::rename(&temporary, path).map_err(|err| format!("{}: {err}", path.display())) +} + +fn read_sealed( + path: &std::path::Path, + key: &str, +) -> Result, String> { + let raw = match std::fs::read_to_string(path) { + Ok(raw) => raw, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Default::default()), + Err(err) => return Err(format!("{}: {err}", path.display())), + }; + let plain = crate::secrets::open(key, &raw)?; + serde_json::from_str(&plain).map_err(|err| format!("{}: {err}", path.display())) +} + +/// Writes the whole map to `path`, sealed — the setup half of `export_secrets`. +/// +/// Reads every shared collection's credential out of the keychain, so it is a +/// deliberate one-off rather than something the app does as you work. The +/// as-you-work path is `sync_secrets_file`, which touches one reference. +pub fn export_secrets_to(path: &std::path::Path) -> Result<(), Box> { + let sections = store::load_all(&store::sections_dir(&app_data_dir()))?; + let exported = collect_secrets(§ions, crate::secrets::get); + let shared = sections + .iter() + .filter(|section| section.mcp.enabled) + .count(); + write_sealed(path, &exported, &file_key()?)?; + // How many, not which — the same reticence as the piped form. + eprintln!( + "Wrote {} credential(s) from {} shared collection(s) to {}.", + exported.len(), + shared, + path.display() + ); + Ok(()) +} + +/// Keeps one reference in the credential file current, if that file exists. +/// +/// This is what makes signing in again reach a running container without a +/// re-export or a restart. The file's *existence* is the opt-in: `toolhive.sh` +/// creates it, and a desktop-only user never has one, so nothing is written +/// behind their back. Deleting it opts back out. +/// +/// Surgical on purpose. Rebuilding the whole map would re-read every shared +/// collection's credential from the keychain on every sign-in — on an ad-hoc +/// signed build, a prompt each. The new value is already in hand at every call +/// site, so only the key that changed is touched. +pub fn sync_secrets_file(data: &std::path::Path, reference: &str, value: Option<&str>) { + let path = secrets_file(data); + if !path.exists() { + return; + } + if let Err(err) = file_key().and_then(|key| sync_inner(&path, reference, value, &key)) { + // Never fails the action the user actually took — saving a credential + // has already succeeded by this point, and the keychain is the record. + // A stale container is recoverable; a sign-in that reports failure + // because a container it knows nothing about could not be updated is + // just confusing. + log::warn!("could not update {}: {err}", path.display()); + } +} + +fn sync_inner( + path: &std::path::Path, + reference: &str, + value: Option<&str>, + key: &str, +) -> Result<(), String> { + let mut current = read_sealed(path, key)?; + let changed = match value { + Some(value) => { + let value = serde_json::Value::String(value.to_string()); + current.insert(reference.to_string(), value.clone()) != Some(value) + } + None => current.remove(reference).is_some(), + }; + // Rewriting an unchanged file would still bump its mtime, and every running + // server would re-read and re-decrypt it for nothing. + if changed { + write_sealed(path, ¤t, key)?; + } + Ok(()) +} + +/// Brings a section's presence in the credential file in line with whether it +/// is shared, after a save that may have toggled either. +/// +/// The keychain is read only when a section has just been shared and its +/// credential is not in the file yet — one read, not one per collection. +pub fn sync_section_sharing(data: &std::path::Path, section: &Section) { + let Some(reference) = section.auth.secret_ref() else { + return; + }; + let path = secrets_file(data); + if !path.exists() { + return; + } + if !section.mcp.enabled { + sync_secrets_file(data, reference, None); + return; + } + match file_key().and_then(|key| read_sealed(&path, &key)) { + Ok(current) if current.contains_key(reference) => {} + Ok(_) => { + if let Some(value) = crate::secrets::get(reference) { + sync_secrets_file(data, reference, Some(&value)); + } + } + Err(err) => log::warn!("could not read {}: {err}", path.display()), + } +} + /// Serves MCP over stdio until the client disconnects. pub async fn serve() -> Result<(), Box> { let data = app_data_dir(); @@ -1324,6 +1520,60 @@ mod tests { assert!(cut); } + /// The bug this exists for: sign in again, and a running container has to + /// see the new credential without anyone re-exporting or restarting it. + #[test] + fn signing_in_again_rewrites_the_credential_the_container_reads() { + let dir = std::env::temp_dir().join(format!("fiber-sync-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = secrets_file(&dir); + let key = crate::secrets::new_key().unwrap(); + + write_sealed(&path, &Default::default(), &key).unwrap(); + sync_inner(&path, "sec-1:auth", Some("first"), &key).unwrap(); + assert_eq!( + read_sealed(&path, &key).unwrap()["sec-1:auth"], + serde_json::json!("first") + ); + + sync_inner(&path, "sec-1:auth", Some("second"), &key).unwrap(); + assert_eq!( + read_sealed(&path, &key).unwrap()["sec-1:auth"], + serde_json::json!("second"), + "the second sign-in must replace the first" + ); + + // And the file on disk never holds either in the clear. + let raw = std::fs::read_to_string(&path).unwrap(); + assert!(crate::secrets::is_sealed(&raw)); + assert!(!raw.contains("second"), "credential in the clear: {raw}"); + + sync_inner(&path, "sec-1:auth", None, &key).unwrap(); + assert!( + !read_sealed(&path, &key).unwrap().contains_key("sec-1:auth"), + "deleting a credential must take it out of the file too" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// The file's existence is the opt-in. A desktop-only user never has one, + /// and must never have credentials written to disk behind their back. + #[test] + fn without_a_file_nothing_is_written() { + let dir = std::env::temp_dir().join(format!("fiber-sync-none-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + // No keychain is touched either — `file_key` is never reached, which is + // what keeps this off the password-prompt path for ordinary users. + sync_secrets_file(&dir, "sec-1:auth", Some("tok")); + assert!(!secrets_file(&dir).exists()); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn response_ids_are_unique_inside_one_millisecond() { let dir = scratch("response-ids"); diff --git a/src-tauri/src/secrets.rs b/src-tauri/src/secrets.rs index 1de7109..5121c2a 100644 --- a/src-tauri/src/secrets.rs +++ b/src-tauri/src/secrets.rs @@ -17,6 +17,14 @@ use std::sync::OnceLock; const SERVICE: &str = "dev.fiber.app"; +/// The app-managed credential file, in the data directory a containerised +/// server already mounts. See `vault` for why it is sealed rather than plain. +pub const FILE_NAME: &str = "mcp-secrets.enc"; + +/// Keychain reference for the key that seals that file. Not a section's +/// credential, so it is namespaced away from the `:auth` references. +pub const KEY_REF: &str = "mcp:file-key"; + #[derive(Debug, thiserror::Error)] #[error("keychain: {0}")] pub struct SecretError(#[from] keyring::Error); @@ -36,50 +44,239 @@ pub fn set(reference: &str, value: &str) -> Result<(), SecretError> { Ok(()) } +/// Authenticated encryption for the credential file. +/// +/// The file lives in the mounted collections directory, which is the only +/// channel the desktop app and a containerised server both reach — the app +/// cannot write to ToolHive's secret store, and the container cannot read the +/// keychain. Putting credentials there in the clear would undo the point of +/// keeping them in a keychain at all, so the file carries ciphertext and the +/// key stays out of the mount: in the keychain on the app's side, in ToolHive's +/// encrypted store on the container's. A copy of the file on its own is inert. +/// +/// The key is long-lived and the values rotate underneath it, which is the +/// whole trick: signing in again rewrites the file, and nothing has to re-issue +/// a secret or restart a workload. +mod vault { + use base64::Engine as _; + use chacha20poly1305::aead::{Aead, Generate, KeyInit}; + use chacha20poly1305::{Key, XChaCha20Poly1305, XNonce}; + + /// Version-tagged so a future format change is a clear error rather than a + /// decryption failure, and so a plaintext file is recognisably not this. + const MAGIC: &str = "FIBER-SECRETS-1 "; + const NONCE_LEN: usize = 24; + + fn b64() -> &'static base64::engine::general_purpose::GeneralPurpose { + &base64::engine::general_purpose::STANDARD + } + + /// A fresh 32-byte key, base64 for a pipe and an environment variable. + pub fn new_key() -> Result { + let key = Key::try_generate().map_err(|err| format!("no system randomness: {err}"))?; + Ok(b64().encode(key.as_slice())) + } + + fn cipher(key: &str) -> Result { + let raw = b64() + .decode(key.trim()) + .map_err(|_| "the secrets key is not valid base64".to_string())?; + XChaCha20Poly1305::new_from_slice(&raw) + .map_err(|_| format!("the secrets key must be 32 bytes, got {}", raw.len())) + } + + pub fn is_sealed(document: &str) -> bool { + document.trim_start().starts_with(MAGIC) + } + + pub fn seal(key: &str, plaintext: &str) -> Result { + let nonce = XNonce::try_generate().map_err(|err| format!("no system randomness: {err}"))?; + let sealed = cipher(key)? + .encrypt(&nonce, plaintext.as_bytes()) + .map_err(|_| "could not encrypt the secrets file".to_string())?; + let mut body = Vec::with_capacity(NONCE_LEN + sealed.len()); + body.extend_from_slice(nonce.as_slice()); + body.extend_from_slice(&sealed); + Ok(format!("{MAGIC}{}\n", b64().encode(&body))) + } + + pub fn open(key: &str, document: &str) -> Result { + let body = document + .trim() + .strip_prefix(MAGIC.trim_end()) + .ok_or("the secrets file is not in FIBER-SECRETS-1 format")?; + let body = b64() + .decode(body.trim()) + .map_err(|_| "the secrets file is not valid base64".to_string())?; + if body.len() <= NONCE_LEN { + return Err("the secrets file is truncated".into()); + } + let (nonce, sealed) = body.split_at(NONCE_LEN); + let nonce = XNonce::try_from(nonce).map_err(|_| "bad nonce".to_string())?; + let plain = cipher(key)?.decrypt(&nonce, sealed).map_err(|_| { + "could not decrypt the secrets file — FIBER_SECRETS_KEY does not match it".to_string() + })?; + String::from_utf8(plain).map_err(|_| "the secrets file did not decrypt to text".into()) + } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn a_sealed_document_round_trips() { + let key = new_key().unwrap(); + let sealed = seal(&key, r#"{"sec-1:auth":"tok"}"#).unwrap(); + assert!(is_sealed(&sealed), "it should announce its own format"); + assert!( + !sealed.contains("tok"), + "the token must not survive in the clear: {sealed}" + ); + assert_eq!(open(&key, &sealed).unwrap(), r#"{"sec-1:auth":"tok"}"#); + } + + /// The file sits in a directory that is mounted, synced and backed up. + /// A copy of it without the key has to be worthless, and a copy that + /// someone has edited has to fail loudly rather than decrypt to + /// something else. + #[test] + fn another_key_or_a_tampered_body_is_rejected() { + let key = new_key().unwrap(); + let sealed = seal(&key, r#"{"sec-1:auth":"tok"}"#).unwrap(); + + assert!(open(&new_key().unwrap(), &sealed).is_err(), "wrong key"); + + // Flip a byte in the middle of the ciphertext. + let mut tampered: Vec = sealed.chars().collect(); + let at = tampered.len() / 2; + tampered[at] = if tampered[at] == 'A' { 'B' } else { 'A' }; + let tampered: String = tampered.into_iter().collect(); + assert!( + open(&key, &tampered).is_err(), + "authentication must catch it" + ); + } + + /// Plaintext must not be mistaken for a sealed file — that is the + /// downgrade this format exists to make visible. + #[test] + fn plaintext_is_not_mistaken_for_a_sealed_file() { + assert!(!is_sealed(r#"{"sec-1:auth":"tok"}"#)); + assert!(open(&new_key().unwrap(), r#"{"sec-1:auth":"tok"}"#).is_err()); + } + } +} + +pub use vault::{is_sealed, new_key, open, seal}; + /// Secrets supplied through the environment, for headless runs where the OS /// keychain isn't reachable — most importantly the MCP server inside a /// container, where a manager like ToolHive injects them. `FIBER_SECRETS` holds /// a JSON object of `reference -> value`; `FIBER_SECRETS_FILE` points at a file -/// with the same. Both are unset in the desktop app, which uses only the -/// keychain, so its behaviour is unchanged. -fn injected_result() -> &'static Result, String> { +/// with the same, optionally sealed with `FIBER_SECRETS_KEY`. +/// +/// Both are unset in the desktop app, which uses only the keychain, so its +/// behaviour is unchanged. +fn env_result() -> &'static Result, String> { + // A process's own environment cannot change under it, so this is read once. + // The *file* is the live half — see `file_secrets`. static INJECTED: OnceLock, String>> = OnceLock::new(); - INJECTED.get_or_init(|| { - let raw = if let Ok(raw) = std::env::var("FIBER_SECRETS") { - Some(raw) - } else if let Some(path) = std::env::var_os("FIBER_SECRETS_FILE") { - Some( - std::fs::read_to_string(&path) - .map_err(|err| format!("could not read FIBER_SECRETS_FILE: {err}"))?, - ) - } else { - None - }; - raw.as_deref() - .map(parse_injected) - .transpose() - .map(Option::unwrap_or_default) + INJECTED.get_or_init(|| match std::env::var("FIBER_SECRETS") { + Ok(raw) => parse_injected(&raw), + Err(_) => Ok(HashMap::new()), }) } -/// The injected-secrets document is a flat JSON object of string values; -/// anything else is a startup error in headless mode. Silently treating a typo -/// as "no credentials" makes every authenticated call fail for an unrelated, -/// invisible reason. +/// The credential file, read afresh every time it is asked for. +/// +/// This is what makes signing in again take effect in a running containerised +/// server. The old behaviour read `FIBER_SECRETS` once into a `OnceLock`, so a +/// container held whatever was true when it started: the user signed in, the +/// keychain got the new token, and the server went on presenting the expired +/// one until someone re-exported the secrets and restarted the workload. A +/// mounted file changes under a running process, so re-reading it is half the +/// fix; the other half is `send::send_authenticated_streaming` dropping the +/// cached token on a 401, which is what sends anyone back here to look. +/// +/// Deliberately uncached, unlike the mtime+size stamp `mcp::all_sections` uses +/// for collections. Two reasons. It is not hot: `auth::header_for` only reaches +/// a lookup when `AuthState` has no live token, which is once per collection +/// per run plus each 401 — everything else is answered from memory. And a +/// stamp would be *wrong* here in a way it is not for collections: one token +/// replaced by another of the same length within a single mtime tick is the +/// ordinary case for a refreshed JWT or session cookie, and a bind mount can +/// coarsen mtime to the second. Missing that change is the bug this function +/// exists to fix, so it is not worth reintroducing to save a kilobyte read. +fn file_secrets() -> Result, String> { + let Some(path) = std::env::var_os("FIBER_SECRETS_FILE") else { + return Ok(HashMap::new()); + }; + // Absent is not an error. The app only writes the file once someone has set + // a container up, and a server pointed at a path that isn't there yet + // should say "no credentials", not refuse to start. + let raw = match std::fs::read_to_string(&path) { + Ok(raw) => raw, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(HashMap::new()), + Err(err) => return Err(format!("could not read FIBER_SECRETS_FILE: {err}")), + }; + decode_file(&raw, std::env::var("FIBER_SECRETS_KEY").ok().as_deref()) +} + +/// The key is passed in rather than read here, so the format rules — including +/// the two ways a file and a key can disagree — are testable without setting +/// process-wide environment variables. +fn decode_file(raw: &str, key: Option<&str>) -> Result, String> { + match (key, is_sealed(raw)) { + (Some(key), true) => parse_injected(&open(key.trim(), raw)?), + // With a key set, plaintext is refused rather than accepted. Otherwise + // swapping the file for an unencrypted one of an attacker's choosing + // would be a silent downgrade, and encrypting it would buy nothing. + (Some(_), false) => Err("FIBER_SECRETS_KEY is set but the secrets file is not \ + encrypted. Rewrite it with `fiber mcp export-secrets --to `." + .into()), + (None, true) => Err( + "the secrets file is encrypted but FIBER_SECRETS_KEY is not set. \ + Pass it, e.g. `--secret fiber-key,target=FIBER_SECRETS_KEY`." + .into(), + ), + (None, false) => parse_injected(raw), + } +} + +/// The injected-secrets document — from either source — is a flat JSON object +/// of string values; anything else is a startup error in headless mode. +/// Silently treating a typo as "no credentials" makes every authenticated call +/// fail for an unrelated, invisible reason. fn parse_injected(raw: &str) -> Result, String> { serde_json::from_str(raw) - .map_err(|err| format!("FIBER_SECRETS must be a JSON object of string values: {err}")) + .map_err(|err| format!("injected secrets must be a JSON object of string values: {err}")) } pub fn validate_injected() -> Result<(), String> { - injected_result().as_ref().map(|_| ()).map_err(Clone::clone) + env_result().as_ref().map(|_| ()).map_err(Clone::clone)?; + file_secrets().map(|_| ()) } -fn injected() -> &'static HashMap { - static EMPTY: OnceLock> = OnceLock::new(); - injected_result() - .as_ref() - .unwrap_or_else(|_| EMPTY.get_or_init(HashMap::new)) +/// Injected values, environment first. Returns nothing on a malformed file +/// rather than propagating: `validate_injected` has already refused to start +/// the server for that, and a file that goes bad while running should fail the +/// request that needed it, not every lookup that didn't. +fn injected(reference: &str) -> Option { + if let Some(value) = env_result().as_ref().ok()?.get(reference) { + return Some(value.clone()); + } + file_secrets().ok()?.get(reference).cloned() +} + +/// Whether credentials are coming from somewhere that can change underneath a +/// running process — which in practice means "is this the containerised server". +/// +/// The desktop app has neither variable set, so callers can use this to take a +/// step that would be wrong there. See `send::send_authenticated_streaming`: +/// dropping a cached token on a 401 costs a container one file read and costs +/// the app a keychain prompt. +pub fn has_injected_source() -> bool { + std::env::var_os("FIBER_SECRETS").is_some() || std::env::var_os("FIBER_SECRETS_FILE").is_some() } /// `None` when absent, which is not an error — an unconfigured section is a @@ -87,8 +284,8 @@ fn injected() -> &'static HashMap { pub fn get(reference: &str) -> Option { // Injected secrets win, so a containerised MCP server never has to reach a // keychain it can't see. - if let Some(value) = injected().get(reference) { - return Some(value.clone()); + if let Some(value) = injected(reference) { + return Some(value); } entry(reference).ok()?.get_password().ok() } @@ -105,7 +302,7 @@ pub fn has(reference: &str) -> bool { use security_framework::item::{ItemClass, ItemSearchOptions, Limit}; // Injected secrets never touch the keychain, so answer for them first. - if injected().contains_key(reference) { + if injected(reference).is_some() { return true; } @@ -160,4 +357,58 @@ mod tests { assert!(parse_injected(r#"{"sec-1:auth":123}"#).is_err()); assert!(parse_injected("[]").is_err()); } + + /// The whole point of the file: a value replaced on disk is the value the + /// next lookup gets. A cache keyed on mtime+size would pass a test that + /// changed the length and fail this one, which is why there isn't one. + #[test] + fn a_rewritten_file_is_read_again() { + let key = new_key().unwrap(); + let before = decode_file( + &seal(&key, r#"{"s:auth":"old-token"}"#).unwrap(), + Some(&key), + ); + assert_eq!(before.unwrap().get("s:auth").unwrap(), "old-token"); + + // Same length, as a refreshed JWT or session cookie usually is. + let after = decode_file( + &seal(&key, r#"{"s:auth":"new-token"}"#).unwrap(), + Some(&key), + ); + assert_eq!(after.unwrap().get("s:auth").unwrap(), "new-token"); + } + + /// A key without an encrypted file is a downgrade — someone swapping the + /// sealed file for one of their own — and an encrypted file without a key + /// is a container missing its `--secret`. Both have to be named, because + /// "no credentials" would send every request out unauthenticated instead. + #[test] + fn a_file_and_a_key_that_disagree_are_both_refused() { + let key = new_key().unwrap(); + let sealed = seal(&key, r#"{"s:auth":"tok"}"#).unwrap(); + let plain = r#"{"s:auth":"tok"}"#; + + assert!( + decode_file(plain, Some(&key)).is_err(), + "plaintext with a key" + ); + assert!(decode_file(&sealed, None).is_err(), "sealed without a key"); + // And the two that agree still work, in both directions. + assert!(decode_file(plain, None).is_ok()); + assert!(decode_file(&sealed, Some(&key)).is_ok()); + } + + /// A path that is not there yet is the normal state before anyone has set a + /// container up, and must not stop the server from starting. + #[test] + fn a_missing_secrets_file_is_not_an_error() { + let path = std::env::temp_dir().join("fiber-secrets-absent.json"); + let _ = std::fs::remove_file(&path); + // SAFETY: single-threaded test setup; nothing else reads this variable + // until `file_secrets` does, on the next line. + unsafe { std::env::set_var("FIBER_SECRETS_FILE", &path) }; + let found = file_secrets(); + unsafe { std::env::remove_var("FIBER_SECRETS_FILE") }; + assert!(found.unwrap().is_empty()); + } } diff --git a/src-tauri/src/send.rs b/src-tauri/src/send.rs index 599d27a..c837ca2 100644 --- a/src-tauri/src/send.rs +++ b/src-tauri/src/send.rs @@ -79,8 +79,24 @@ where let prepared = apply_auth(http_state, auth_state, section, spec, lookup).await?; let first = http::send_streaming(http_state, prepared, sink).await; - let should_retry = - matches!(&first, Ok(response) if response.status == 401) && retry_spec.is_some(); + let rejected = matches!(&first, Ok(response) if response.status == 401); + + // A static token cannot be refreshed by replaying anything, so there is no + // retry to make — but where the credential comes from a source that changes + // underneath the process, the cached copy is still worth dropping so the + // *next* send reads the new one. That is a containerised server whose + // credential file the app has just rewritten: without this, a bearer + // collection would present the token it started with for the life of the + // workload, because nothing else ever expires a zero-TTL entry. + // + // Conditioned on there being such a source, because in the desktop app the + // same line would buy nothing and cost a keychain prompt per 401 — see + // `auth::header_for` on why reads are lazy. + if rejected && retry_spec.is_none() && crate::secrets::has_injected_source() { + auth_state.invalidate(§ion.id); + } + + let should_retry = rejected && retry_spec.is_some(); if !should_retry { return first; } @@ -352,6 +368,103 @@ mod tests { ); } + /// An API that only accepts one exact token, for the container story: the + /// credential is not refreshed by replaying a request, it is *replaced on + /// disk* by the desktop app when someone signs in again. + async fn fixed_token_api(accepted: &'static str) -> (String, Arc) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let calls = Arc::new(Calls { + logins: AtomicUsize::new(0), + protected: AtomicUsize::new(0), + }); + + let counters = calls.clone(); + tokio::spawn(async move { + loop { + let Ok((mut socket, _)) = listener.accept().await else { + return; + }; + let counters = counters.clone(); + tokio::spawn(async move { + let mut buf = [0u8; 4096]; + let read = socket.read(&mut buf).await.unwrap_or(0); + let request = String::from_utf8_lossy(&buf[..read]).to_string(); + counters.protected.fetch_add(1, Ordering::SeqCst); + let ok = request.contains(&format!("Bearer {accepted}")); + let status = if ok { "200 OK" } else { "401 Unauthorized" }; + let _ = socket + .write_all( + format!("HTTP/1.1 {status}\r\nContent-Length: 0\r\n\r\n").as_bytes(), + ) + .await; + let _ = socket.flush().await; + }); + } + }); + + (format!("http://{addr}"), calls) + } + + /// The whole containerised story, end to end: a bearer collection whose + /// credential arrives through `FIBER_SECRETS_FILE`, a server holding the + /// token it started with, and a sign-in in the desktop app that rewrites + /// the file underneath it. + /// + /// Before this worked, the second send below returned 401 forever — the + /// file was read once into a `OnceLock` at startup, and a zero-TTL bearer + /// entry was never dropped because bearer auth cannot be refreshed. + #[tokio::test] + async fn a_rewritten_credential_file_reaches_a_running_server() { + let (base, _calls) = fixed_token_api("new-token").await; + let dir = std::env::temp_dir().join(format!("fiber-live-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("secrets.json"); + std::fs::write(&path, r#"{"sec-1:auth":"old-token"}"#).unwrap(); + + // SAFETY: no other test reads this variable, and the reads it causes all + // happen on this task before it is removed at the end. + unsafe { std::env::set_var("FIBER_SECRETS_FILE", &path) }; + + let section = Section { + id: "sec-1".into(), + name: "Test".into(), + base_url: base.clone(), + auth: AuthConfig::Bearer { + secret_ref: "sec-1:auth".into(), + }, + ..Default::default() + }; + let http_state = HttpState::default(); + let auth_state = AuthState::default(); + + let send = || { + send_authenticated( + &http_state, + &auth_state, + Some(§ion), + spec_for(&base), + &crate::secrets::get, + None, + ) + }; + + // The token the workload started with. Rejected, and now also dropped + // from the cache, which is the half that used to be missing. + assert_eq!(send().await.unwrap().status, 401); + + // Signing in again in Fiber. Same length, as a refreshed token usually + // is — an mtime+size stamp would not have noticed this. + std::fs::write(&path, r#"{"sec-1:auth":"new-token"}"#).unwrap(); + + let after = send().await.unwrap().status; + unsafe { std::env::remove_var("FIBER_SECRETS_FILE") }; + let _ = std::fs::remove_dir_all(&dir); + + assert_eq!(after, 200, "the next send should use the rewritten token"); + } + /// A genuine 401 must not loop: exactly one retry, then give up. #[tokio::test] async fn retries_at_most_once() {