feat: self-signed HTTPS for goosed server#7126
Conversation
|
looks promising, if I read correctly, it is trying to let it slide for localhost, i wonder if there is a way to add the self cert to the trust store (just a thought if this doesn't work) but keep pushing! |
In the last commit, I switched from the process-wide Re: adding the self-signed cert to the trust store — that's tricky since goosed regenerates the cert on every launch, so we'd need keychain writes on each startup. Happy to revisit if the current approach causes issues though. |
Generate an ephemeral self-signed TLS certificate at startup so goosed serves over HTTPS. This gives MCP app iframes a secure context, enabling browser APIs (crypto.subtle, clipboard, etc.) and making 3rd-party iframe injection viable. Changes: - Add tls module that generates a self-signed cert for 127.0.0.1/localhost - Switch axum::serve to axum_server::bind_rustls for TLS termination - Update Electron baseUrl from http to https - Add certificate-error handler in Electron for localhost self-signed cert - Set NODE_TLS_REJECT_UNAUTHORIZED=0 for main-process health checks - Update CSP connect-src to allow https://127.0.0.1:*
MCP app guest iframes previously used srcdoc to inject HTML, which gave
them a URL of about:srcdoc. This caused third-party SDKs (like Square
Web Payments SDK) to fail because they check window.location.protocol
and reject anything that isn't https: or localhost.
This commit adds a /mcp-app-guest endpoint that stores guest HTML
server-side and serves it from a real https://localhost:{port} URL:
Server changes (mcp_app_proxy.rs):
- Added POST /mcp-app-guest to store HTML with a unique nonce
- Added GET /mcp-app-guest to retrieve and serve stored HTML with
proper CSP headers (script-src, connect-src, frame-src, style-src)
- In-memory HashMap<String, StoredHtml> with 5-minute TTL and
automatic cleanup on each request
- HTML is consumed on first retrieval (one-time use)
Proxy template changes (mcp_app_proxy.html):
- On receiving guest HTML via postMessage, the proxy now POSTs it
to /mcp-app-guest to get a nonce, then sets the guest iframe src
to GET /mcp-app-guest?secret=...&nonce=...
- Falls back to srcdoc if the endpoint is unavailable
- createGuestIframe is now async to support the fetch workflow
Auth changes (auth.rs):
- Added /mcp-app-guest to the auth exemption list (it uses its own
secret query parameter for authentication, same as /mcp-app-proxy)
Desktop changes (goosed.ts, main.ts):
- Switched baseUrl from https://127.0.0.1:{port} to
https://localhost:{port} so that SDKs with built-in localhost
exceptions (like Square) recognize the origin
- Added http://localhost:* and https://localhost:* to the Electron
CSP connect-src allowlist
- Updated default GOOSE_API_HOST to http://localhost
Replace process-wide NODE_TLS_REJECT_UNAUTHORIZED=0 with Electron's net.fetch for the goosed API client. net.fetch uses Chromium's network stack which respects the certificate-error handler already scoped to localhost/127.0.0.1, so external fetch calls (fetch-metadata, getAllowList) retain strict TLS verification.
The handler was inside appMain() but net.fetch needs it registered before any createChat() call. Code paths like open-url and second-instance can call createChat() before appMain() runs.
The certificate-error handler only applies to webContents (renderer) requests. net.fetch in the main process needs setCertificateVerifyProc on the default session to accept self-signed certs from localhost. Registered via app.whenReady() before appMain() to ensure it's active before any health check.
aharvard
force-pushed
the
aharvard/self-signed-https
branch
from
e9929cc to
4ece61f
Compare
|
@michaelneale I think we're ready for a review on this. Got anything else before I open it to broader feedback? |
|
Looking good, also from research, this looks like http/2 which supports tons of streams and sse won't block, so this should I think help performance a lot as a side effect (what @DOsinga mentioned). some things to consider: (notes from my agents which I think I agree with): File: let url = format!("http://127.0.0.1:{}{}", port, message.path);The tunnel proxy receives requests via WebSocket from Cloudflare and forwards them to the local goosed server using Impact: The entire mobile tunnel feature breaks. All remote requests through the Cloudflare relay will fail with connection errors. Fix needed: Either:
External Backend Path Still Uses HTTP: File: return connectToExternalBackend(dir, `http://127.0.0.1:${port}`);When I think these are only unexpected side effects, not sure of best way - perhaps we need a way to start goosed without self signed then both of these could work? Very cool otherwise! |
|
@michaelneale Great catches on the tunnel and external backend — took your feedback and worked through the options with my agent. Here are the three paths we see: Option 1:
|
The tunnel proxy (lapstone) now builds a shared reqwest::Client with danger_accept_invalid_certs(true) for localhost-only connections to the self-signed goosed server. Introduced ProxyContext struct to bundle per-connection state and fix clippy too-many-arguments lint. The external backend path in goosed.ts now defaults to https:// to match the normal startup path.
Resolved conflicts: - Cargo.lock: took main's version, regenerated with cargo update - agent.rs: kept OTLP shutdown logic from main - goosed.ts: took main's refactored version, applied http->https fix - main.ts: kept net.fetch client override for self-signed cert support, added createClient/createConfig imports
The tunnel proxy now uses HTTPS when connecting to the local goosed server (which serves self-signed TLS), while tests use HTTP since the mock server is plain HTTP. The reqwest client conditionally accepts invalid certs only when scheme is https.
|
Went with Option 3 (HTTPS-only, fix the callsites) — commits What changed:
No |
The goosed integration tests run in plain Node.js (not Electron), so they need NODE_TLS_REJECT_UNAUTHORIZED=0 to accept the self-signed TLS cert from the local goosed server. In the Electron app this is handled by setCertificateVerifyProc.
aharvard
requested review from
DOsinga,
alexhancock,
Copilot,
jamadeo and
katzdave
There was a problem hiding this comment.
Pull request overview
This PR adds self-signed HTTPS support to the goosed server to provide a secure context for MCP app iframes. By serving over HTTPS, browser APIs like crypto.subtle and clipboard access become available to third-party iframe content, enabling richer MCP app functionality.
Changes:
- Added TLS certificate generation at startup using
rcgenandrustls - Migrated from
axum::servetoaxum_server::bind_rustlsfor HTTPS serving - Updated tunnel proxy to connect via HTTPS with self-signed certificate acceptance
- Configured Electron to trust localhost self-signed certificates via
setCertificateVerifyProc - Added server-side storage for guest HTML to provide real HTTPS URLs instead of
srcdociframes
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
crates/goose-server/src/tls.rs |
New module that generates ephemeral self-signed certificates for localhost |
crates/goose-server/src/commands/agent.rs |
Switched to HTTPS serving using axum_server::bind_rustls with graceful shutdown |
crates/goose-server/src/tunnel/lapstone.rs |
Made proxy scheme configurable, accepts self-signed certs for HTTPS connections |
crates/goose-server/src/tunnel/mod.rs |
Passes "https" scheme to tunnel proxy |
crates/goose-server/src/tunnel/lapstone_test.rs |
Uses "http" scheme for plain HTTP test servers |
crates/goose-server/src/routes/mcp_app_proxy.rs |
Added endpoints to store/serve guest HTML with real HTTPS URLs |
crates/goose-server/src/routes/templates/mcp_app_proxy.html |
Client-side logic to store guest HTML server-side and load via real URL |
crates/goose-server/src/auth.rs |
Added /mcp-app-guest to auth bypass list (auth still checked in handler) |
crates/goose-server/src/lib.rs |
Exported new tls module |
crates/goose-server/Cargo.toml |
Added rcgen and axum-server dependencies |
ui/desktop/src/main.ts |
Added certificate trust for localhost, recreated client with net.fetch, updated CSP |
ui/desktop/src/goosed.ts |
Changed base URLs from http:// to https:// |
ui/desktop/tests/integration/setup.ts |
Set NODE_TLS_REJECT_UNAUTHORIZED=0 for integration tests |
Cargo.lock |
Dependency lock file updates |
Adds ensure_extensions_loaded() helper that awaits any in-flight background extension loading task before serving read_resource or call_tool requests. This prevents 500s when the UI hits these endpoints before start_agent's background spawn has finished registering extensions. Also adds error logging to read_resource so failures aren't silently swallowed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
crates/goose-server/src/tunnel/lapstone.rs:276
- The error message "failed to build reqwest client" doesn't provide context about what failed or why. Consider using
context()from anyhow to provide more details, or handle the error properly and propagate it to the caller.
.expect("failed to build reqwest client");
crates/goose-server/src/routes/templates/mcp_app_proxy.html:71
- The function signature changed from synchronous to asynchronous but the function name wasn't updated to reflect this. Consider renaming to indicate the async nature, or document the change with a comment explaining why the async conversion was necessary.
async function createGuestIframe(html, permissions) {
ui/desktop/src/main.ts
Outdated
| // setCertificateVerifyProc handles net.fetch requests (main process). | ||
| app.on('certificate-error', (event, _webContents, url, _error, _certificate, callback) => { | ||
| const parsed = new URL(url); | ||
| if (parsed.hostname === '127.0.0.1' || parsed.hostname === 'localhost') { |
There was a problem hiding this comment.
This seems risky if MCP apps will be used for anything sensitive. (Will they?) A malicious local process could race to serve on the same port and would be accepted no matter what cert it presents.
This is no worse than before, when we didn't use HTTPS at all, but there's a reason browsers only enable certain features when HTTPS is used, and we are circumventing that protection. I think the improvement would be reasonably simple: goosed could emit the generated cert's fingerprint to stdout and we could grab it and accept only that cert here. If we choose not to do this, I think a comment acknowledging the decision to leave the cert validation open would be a good idea.
There was a problem hiding this comment.
The main driver of this change was to enable MCP Apps to accept payments via the Square Web Payments SDK, So yeah, I think that's sensitive. :)
Great idea. I'll get that set up!
There was a problem hiding this comment.
Done — goosed now emits GOOSED_CERT_FINGERPRINT=<sha256-hex> to stdout at startup. The Electron app captures it and pins it in both certificate-error and setCertificateVerifyProc. Before the fingerprint arrives during early startup, localhost certs are accepted to allow the initial health check; once pinned, only the exact cert is accepted.
There was a problem hiding this comment.
Sorry to nitpick but maybe we could use the url passed to the certificate-error event to just always allow the health check through with any cert, and not have it fail open if anything goes wrong with passing the fingerprint across?
There was a problem hiding this comment.
Good call. Updated — certificate-error now always accepts localhost (URL-only check), so renderer page loads and iframe resources can never fail due to fingerprint timing. Fingerprint pinning is scoped to setCertificateVerifyProc only, which handles net.fetch from the main process where the security benefit actually applies.
crates/goose-server/src/tls.rs
Outdated
| /// return an `axum_server::tls_rustls::RustlsConfig` ready to use. | ||
| pub async fn self_signed_config() -> Result<RustlsConfig> { | ||
| // rustls 0.23+ requires an explicit crypto provider installation. | ||
| let _ = rustls::crypto::ring::default_provider().install_default(); |
There was a problem hiding this comment.
nit: I think we're using awc_lc_rs elsewhere; using it for rustls as well may possibly cut down the crate graph a little
There was a problem hiding this comment.
Good call — switched both the rustls crypto provider and the digest to aws_lc_rs, and removed the direct ring dependency
| return Ok(()); | ||
| } | ||
| }; | ||
| let mut client_builder = reqwest::Client::builder(); |
There was a problem hiding this comment.
Can we re-use the reqwest::Client rather than building a new one for every request?
There was a problem hiding this comment.
Fixed — the client is now built once in run_single_connection and stored in ProxyContext so it's reused across all requests for the lifetime of the tunnel connection.
| /// In-memory store for guest HTML content. | ||
| /// Maps nonce -> (html_content, csp_string) | ||
| /// Entries are consumed on first read (one-time use). | ||
| type GuestHtmlStore = Arc<RwLock<HashMap<String, (String, String)>>>; |
There was a problem hiding this comment.
As far as I can see there's no bound on this store; if the client doesn't read the response body the entry will be left behind. If every possible client who can cause an entry to be written here is trusted then this is just a nit.
There was a problem hiding this comment.
Added a 5-minute TTL and 64-entry cap. Expired entries are evicted on each write, and if still at capacity the oldest entry is dropped.
goosed now emits GOOSED_CERT_FINGERPRINT=<sha256-hex> to stdout at startup. The Electron app captures it and pins it in both certificate-error and setCertificateVerifyProc handlers, so only the exact cert generated by this goosed instance is accepted. Before the fingerprint arrives (during early startup), localhost certs are accepted to allow the initial health check. Once pinned, any other self-signed cert from localhost is rejected. Addresses jh-block review feedback on PR #7126.
- Switch from ring to aws_lc_rs for rustls crypto provider and SHA-256 digest, reducing duplicate crypto backends in the crate graph - Reuse reqwest::Client in tunnel proxy by storing it in ProxyContext instead of building a new one per request - Bound the guest HTML store with a 5-minute TTL and 64-entry cap, evicting expired/oldest entries on each write
|
@jh-block thanks for the review, can you PTAL at my updates? Want to make sure I'm on track before merge. |
|
All looks great but I had one further comment on the certificate verification part, I think it's also fine to punt that especially if it's not as easy as I imagined |
…ly in setCertificateVerifyProc Per jh-block's follow-up: certificate-error (renderer webContents) now always accepts localhost so page loads and iframe resources never fail. Fingerprint pinning is scoped to setCertificateVerifyProc (main-process net.fetch) where the security benefit applies.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
crates/goose-server/src/commands/agent.rs:52
- The
tls_setup.fingerprintfield is returned but never used in this function. The fingerprint is printed to stdout (line 41 in tls.rs) for the Electron process to capture, so this field in the returned struct serves no purpose here.
let tls_setup = self_signed_config().await?;
seemed pretty easy to implement @jh-block, whatcha think? |
|
sorry took a minute to circle back here. The implementation is slightly different than what I had in mind. It's not clear to me that the renderer is lower-risk than the main process (an attacker being able to cause different content to be rendered is certainly a problem). So always accepting any cert from localhost on the renderer seems risky. I was imagining we would only accept any cert for the health check, and keep the pin in place for everything else, to close the hole for both main-process stuff and for the renderer. Otherwise everything looks great! |
The certificate-error handler (renderer) was accepting any self-signed cert from localhost, while setCertificateVerifyProc (main process) was already pinning to the exact fingerprint. An attacker who could MITM localhost with a different self-signed cert could serve malicious content in the renderer. Now both handlers share the same logic: accept any localhost cert only during the brief bootstrap window before the fingerprint is known, then pin to the exact cert goosed generated. Also adds missing Buffer import from node:buffer to fix pre-existing eslint no-undef error in normalizeFingerprint.
Keep both the gateway_manager auto-start spawn (from main) and the axum_server::bind_rustls TLS binding (from this branch).
PR #7126 hardcoded self-signed HTTPS as the only server mode, breaking headless deployments (K8s health probes, sidecar proxies, plain HTTP clients) that expect HTTP. TLS is now controlled by the GOOSE_TLS env var (default: true), so existing desktop deployments are unaffected and server deployments can opt out with GOOSE_TLS=false.
Summary
Generate an ephemeral self-signed TLS certificate at startup so goosed serves over HTTPS. This gives MCP app iframes a secure context, enabling browser APIs (
crypto.subtle,clipboard, etc.) and making 3rd-party iframe injection viable.Changes
Rust (
goose-server)tls.rs(new) — Generates a self-signed cert for127.0.0.1+localhostusingrcgen, installs theringcrypto provider, returnsRustlsConfigagent.rs— Switched fromaxum::serve(plain TCP) toaxum_server::bind_rustls(TLS). UsesHandlefor graceful shutdown.lib.rs— Addedpub mod tls;Cargo.toml— Addedrcgen0.13 andaxum-server0.8 withtls-rustlsfeaturemcp_app_proxy.rs/mcp_app_proxy.html— Serve MCP app guest HTML from a real HTTPS URL instead ofsrcdoc, giving iframes a proper secure origintunnel/lapstone.rs— Made the proxy scheme configurable (httpsin production,httpin tests). The reqwest client conditionally accepts self-signed certs when using HTTPS. Removed shared client fromProxyContextin favor of per-request client construction.tunnel/mod.rs— Passes"https"scheme tolapstone::startElectron (
ui/desktop)goosed.ts— ChangedbaseUrlfromhttp://tohttps://(including external backend connections)main.ts— AddedsetCertificateVerifyProcto accept self-signed certs from localhost, scoped trust vianet.fetchinstead of process-wideNODE_TLS_REJECT_UNAUTHORIZED=0, updated CSPconnect-srcto allowhttps://127.0.0.1:*Tests
tests/integration/setup.ts— SetNODE_TLS_REJECT_UNAUTHORIZED=0so Node.js integration tests accept the self-signed cert (in Electron this is handled bysetCertificateVerifyProc)lapstone_test.rs— Pass"http"scheme since the mock test server is plain HTTPHow It Works
tls::self_signed_config()which generates an ephemeral RSA key pair and X.509 cert with SANs for127.0.0.1andlocalhostaxum_server::bind_rustls(addr, tls_config)serves all routes over HTTPSsetCertificateVerifyProcaccepts the self-signed cert for localhost originsnet.fetchwith scoped certificate trust for health checkshttps://127.0.0.1:{port}/mcp-app-proxy?secret=..., giving them a secure context ✅danger_accept_invalid_certs(true)(safe: only connects to our own127.0.0.1)Approach: HTTPS-only (Option 3)
Per review discussion, this PR implements Option 3 — HTTPS everywhere with scoped certificate acceptance. All callsites (tunnel proxy, external backend, Electron health checks, Node.js integration tests) were updated to use HTTPS. No
--no-tlsflag or dual HTTP+HTTPS stack needed.Testing
cargo build -p goose-server✅cargo fmt✅cargo clippy -p goose-server✅npm run typecheck(ui/desktop) ✅httpscheme for plain HTTP mock server ✅NODE_TLS_REJECT_UNAUTHORIZED=0✅