From a425724719a3a5839d7ff403bfa48f10d3f4c2b9 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 3 May 2026 18:27:34 -0400 Subject: [PATCH] refactor(server): reuse canonical_origin in run_web_url Delegate to the existing AppState::canonical_origin helper instead of re-resolving server.web.url and re-checking emptiness inline. The helper already validates the URL via validate_public_url, so a misconfigured non-http(s) origin no longer leaks through into run_web_url's output. Also pass web_url into create_run_input directly rather than constructing with None and immediately patching the field at the call site. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/crates/fabro-server/src/run_manifest.rs | 3 ++- lib/crates/fabro-server/src/server.rs | 17 ++++++----------- .../fabro-server/src/server/handler/runs.rs | 4 ++-- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index 30fd1ffed..5aeea806b 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -161,6 +161,7 @@ pub(crate) fn validate_prepared_manifest( pub(crate) fn create_run_input( prepared: PreparedManifest, configured_providers: Vec, + web_url: Option, ) -> CreateRunInput { CreateRunInput { workflow: WorkflowInput::Bundled(prepared.workflow_input), @@ -176,7 +177,7 @@ pub(crate) fn create_run_input( in_place: prepared.in_place, provenance: None, configured_providers, - web_url: None, + web_url, } } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index a3eb2e57b..0fe52a578 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -590,20 +590,15 @@ impl AppState { ) } - /// Public web UI URL for a run, when the web UI is enabled and configured. - /// Returns `None` when `server.web.enabled = false`, when the configured - /// URL is empty, or when InterpString resolution fails. + /// Snapshotted at create-time so attach replays surface the same link + /// even if `server.web.url` is later changed. `None` when the UI is + /// turned off or `server.web.url` is unset/invalid. pub(crate) fn run_web_url(&self, run_id: &fabro_types::RunId) -> Option { - let web = &self.server_settings().server.web; - if !web.enabled { + if !self.server_settings().server.web.enabled { return None; } - let base = resolve_interp_string(&web.url).ok()?; - let trimmed = base.trim_end_matches('/'); - if trimmed.is_empty() { - return None; - } - Some(format!("{trimmed}/runs/{run_id}")) + let base = self.canonical_origin().ok()?; + Some(format!("{}/runs/{run_id}", base.trim_end_matches('/'))) } pub(crate) async fn resolve_llm_client(&self) -> anyhow::Result { diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 675367ba7..ff03b55e5 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -363,11 +363,11 @@ async fn create_run( let web_url = state.run_web_url(&run_id); let configured_providers = state.llm_source.configured_providers().await; - let mut create_input = run_manifest::create_run_input(prepared.clone(), configured_providers); + let mut create_input = + run_manifest::create_run_input(prepared.clone(), configured_providers, web_url.clone()); create_input.run_id = Some(run_id); create_input.provenance = Some(run_provenance(&headers, &subject)); create_input.submitted_manifest_bytes = Some(body.to_vec()); - create_input.web_url = web_url.clone(); let storage_root = match resolve_interp_string(&state.server_settings().server.storage.root) { Ok(path) => PathBuf::from(path),