mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-11 03:40:05 +00:00
Merge remote-tracking branch 'origin/main'
This commit is contained in:
commit
14aaf4f1d7
17 changed files with 593 additions and 140 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -2310,6 +2310,7 @@ dependencies = [
|
|||
"tempfile",
|
||||
"toml 0.8.23",
|
||||
"ulid",
|
||||
"url",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
|
|
|||
|
|
@ -510,4 +510,75 @@ describe("InstallApp", () => {
|
|||
console.error = originalConsoleError;
|
||||
}
|
||||
});
|
||||
|
||||
test("shows the GitHub App callback URL on the review step", async () => {
|
||||
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
const originalConsoleError = console.error;
|
||||
console.error = ((...args: unknown[]) => {
|
||||
if (
|
||||
typeof args[0] === "string" &&
|
||||
args[0].startsWith("react-test-renderer is deprecated")
|
||||
) {
|
||||
return;
|
||||
}
|
||||
originalConsoleError(...args);
|
||||
}) as typeof console.error;
|
||||
try {
|
||||
const fetchMock = mock((input: RequestInfo | URL) => {
|
||||
expect(String(input)).toBe("/install/session");
|
||||
return Promise.resolve(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
completed_steps: ["server", "object_store", "llm", "github"],
|
||||
llm: {
|
||||
providers: [{ provider: "anthropic" }],
|
||||
},
|
||||
server: { canonical_url: "https://fabro.example.com" },
|
||||
object_store: { provider: "local" },
|
||||
github: {
|
||||
strategy: "app",
|
||||
owner: { kind: "personal" },
|
||||
app_name: "octocat-fabro",
|
||||
slug: "octocat-fabro",
|
||||
allowed_username: "octocat",
|
||||
},
|
||||
prefill: INSTALL_PREFILL,
|
||||
}),
|
||||
{
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
globalThis.fetch = fetchMock as typeof fetch;
|
||||
|
||||
const testWindow = createTestWindow("https://fabro.example.com/install/review");
|
||||
testWindow.sessionStorage.setItem("fabro-install-token", "test-install-token");
|
||||
(globalThis as { window?: unknown }).window = testWindow;
|
||||
|
||||
let renderer: TestRenderer.ReactTestRenderer | null = null;
|
||||
await act(async () => {
|
||||
renderer = TestRenderer.create(
|
||||
<MemoryRouter initialEntries={["/install/review"]}>
|
||||
<Routes>
|
||||
<Route path="/install/*" element={<InstallApp />} />
|
||||
</Routes>
|
||||
</MemoryRouter>,
|
||||
);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
const text = renderTreeText(renderer!.toJSON());
|
||||
expect(text).toContain("GitHub callback URL");
|
||||
expect(text).toContain("https://fabro.example.com/auth/callback/github");
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
renderer?.unmount();
|
||||
});
|
||||
} finally {
|
||||
console.error = originalConsoleError;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1166,7 +1166,7 @@ function ReviewScreen({
|
|||
/>
|
||||
{renderObjectStoreSummaryRows(session?.object_store)}
|
||||
<SummaryRow label="LLM providers" value={providers || "Not configured"} />
|
||||
{renderGithubSummaryRows(session?.github)}
|
||||
{renderGithubSummaryRows(session?.github, serverUrl)}
|
||||
</dl>
|
||||
{error ? <ErrorMessage message={error} /> : null}
|
||||
<div className="flex items-center justify-between gap-3 pt-2">
|
||||
|
|
@ -1727,6 +1727,7 @@ function describeProvider(id: string): string {
|
|||
|
||||
function renderGithubSummaryRows(
|
||||
github: InstallSessionResponse["github"],
|
||||
serverUrl: string,
|
||||
): ReactNode {
|
||||
if (!github) {
|
||||
return <SummaryRow label="GitHub" value="Not configured" />;
|
||||
|
|
@ -1741,6 +1742,11 @@ function renderGithubSummaryRows(
|
|||
value={github.allowed_username ? `@${github.allowed_username}` : "Not set"}
|
||||
mono={Boolean(github.allowed_username)}
|
||||
/>
|
||||
<SummaryRow
|
||||
label="GitHub callback URL"
|
||||
value={githubCallbackUrl(serverUrl)}
|
||||
mono
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
|
@ -1756,6 +1762,10 @@ function renderGithubSummaryRows(
|
|||
);
|
||||
}
|
||||
|
||||
function githubCallbackUrl(serverUrl: string): string {
|
||||
return `${serverUrl.replace(/\/+$/, "")}/auth/callback/github`;
|
||||
}
|
||||
|
||||
function renderObjectStoreSummaryRows(
|
||||
objectStore: InstallSessionResponse["object_store"],
|
||||
): ReactNode {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use anyhow::Result;
|
||||
use fabro_api::types as api_types;
|
||||
use fabro_config::user::active_settings_path;
|
||||
use fabro_types::settings::replace_wildcard_host;
|
||||
pub(crate) use fabro_util::check_report::{
|
||||
CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus,
|
||||
};
|
||||
|
|
@ -19,15 +20,30 @@ pub(crate) fn check_config(settings_path: Option<PathBuf>) -> CheckResult {
|
|||
match settings_path {
|
||||
Some(path) => {
|
||||
let display = contract_tilde(&path);
|
||||
CheckResult {
|
||||
name: "Configuration".to_string(),
|
||||
status: CheckStatus::Pass,
|
||||
summary: display.display().to_string(),
|
||||
details: vec![CheckDetail::new(format!(
|
||||
"Loaded from {}",
|
||||
display.display()
|
||||
))],
|
||||
remediation: None,
|
||||
let wildcard_urls = wildcard_public_url_details(&path);
|
||||
let mut details = vec![CheckDetail::new(format!(
|
||||
"Loaded from {}",
|
||||
display.display()
|
||||
))];
|
||||
if wildcard_urls.is_empty() {
|
||||
CheckResult {
|
||||
name: "Configuration".to_string(),
|
||||
status: CheckStatus::Pass,
|
||||
summary: display.display().to_string(),
|
||||
details,
|
||||
remediation: None,
|
||||
}
|
||||
} else {
|
||||
details.extend(wildcard_urls);
|
||||
CheckResult {
|
||||
name: "Configuration".to_string(),
|
||||
status: CheckStatus::Warning,
|
||||
summary: "wildcard public URL configured".to_string(),
|
||||
details,
|
||||
remediation: Some(
|
||||
"Replace wildcard public URLs with loopback or proxy URLs, then update the GitHub App callback URL.".to_string(),
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
None => CheckResult {
|
||||
|
|
@ -42,6 +58,94 @@ pub(crate) fn check_config(settings_path: Option<PathBuf>) -> CheckResult {
|
|||
}
|
||||
}
|
||||
|
||||
struct WildcardPublicUrl {
|
||||
field: &'static str,
|
||||
value: String,
|
||||
suggestion: String,
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "Doctor synchronously reads one small local settings file while assembling a CLI report."
|
||||
)]
|
||||
fn wildcard_public_url_details(path: &Path) -> Vec<CheckDetail> {
|
||||
let Ok(contents) = std::fs::read_to_string(path) else {
|
||||
return Vec::new();
|
||||
};
|
||||
let Ok(doc) = contents.parse::<toml::Value>() else {
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
let mut bad_urls = Vec::new();
|
||||
for (field, value) in [
|
||||
(
|
||||
"server.web.url",
|
||||
toml_string_at(&doc, &["server", "web", "url"]),
|
||||
),
|
||||
(
|
||||
"server.api.url",
|
||||
toml_string_at(&doc, &["server", "api", "url"]),
|
||||
),
|
||||
(
|
||||
"cli.target.url",
|
||||
toml_string_at(&doc, &["cli", "target", "url"]),
|
||||
),
|
||||
] {
|
||||
let Some(value) = value else {
|
||||
continue;
|
||||
};
|
||||
let Some(suggestion) = replace_wildcard_host(value, "127.0.0.1") else {
|
||||
continue;
|
||||
};
|
||||
bad_urls.push(WildcardPublicUrl {
|
||||
field,
|
||||
value: value.to_string(),
|
||||
suggestion,
|
||||
});
|
||||
}
|
||||
|
||||
if bad_urls.is_empty() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let mut details = bad_urls
|
||||
.iter()
|
||||
.map(|entry| CheckDetail {
|
||||
text: format!(
|
||||
"{} uses wildcard host {}; set it to {}",
|
||||
entry.field, entry.value, entry.suggestion
|
||||
),
|
||||
warn: true,
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let callback_base = bad_urls
|
||||
.iter()
|
||||
.find(|entry| entry.field == "server.web.url")
|
||||
.unwrap_or(&bad_urls[0])
|
||||
.suggestion
|
||||
.as_str();
|
||||
let github_settings_url = toml_string_at(&doc, &["server", "integrations", "github", "slug"])
|
||||
.map_or_else(
|
||||
|| "the GitHub App settings page".to_string(),
|
||||
|slug| format!("https://github.com/settings/apps/{slug}"),
|
||||
);
|
||||
details.push(CheckDetail {
|
||||
text: format!(
|
||||
"Update the GitHub App Callback URL at {github_settings_url} -> General -> Callback URL to {callback_base}/auth/callback/github"
|
||||
),
|
||||
warn: true,
|
||||
});
|
||||
|
||||
details
|
||||
}
|
||||
|
||||
fn toml_string_at<'a>(doc: &'a toml::Value, path: &[&str]) -> Option<&'a str> {
|
||||
path.iter()
|
||||
.try_fold(doc, |value, key| value.get(*key))
|
||||
.and_then(toml::Value::as_str)
|
||||
}
|
||||
|
||||
fn check_version_parity(server_version: &str) -> CheckResult {
|
||||
let cli_version = FABRO_VERSION;
|
||||
if server_version == cli_version {
|
||||
|
|
@ -333,6 +437,51 @@ mod tests {
|
|||
assert!(result.remediation.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "unit test stages a temporary settings.toml fixture with sync std::fs"
|
||||
)]
|
||||
fn check_config_warns_about_wildcard_public_urls() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let settings_path = dir.path().join("settings.toml");
|
||||
std::fs::write(
|
||||
&settings_path,
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[server.web]
|
||||
url = "http://0.0.0.0:32276"
|
||||
|
||||
[server.api]
|
||||
url = "http://0.0.0.0:32276"
|
||||
|
||||
[server.integrations.github]
|
||||
slug = "octocat-fabro"
|
||||
|
||||
[cli.target]
|
||||
type = "http"
|
||||
url = "http://0.0.0.0:32276"
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let result = check_config(Some(settings_path));
|
||||
assert_eq!(result.status, CheckStatus::Warning);
|
||||
let details = result
|
||||
.details
|
||||
.iter()
|
||||
.map(|detail| detail.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
|
||||
assert!(details.contains("server.web.url"));
|
||||
assert!(details.contains("server.api.url"));
|
||||
assert!(details.contains("cli.target.url"));
|
||||
assert!(details.contains("http://127.0.0.1:32276/auth/callback/github"));
|
||||
assert!(details.contains("https://github.com/settings/apps/octocat-fabro"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn check_version_parity_warns_on_mismatch() {
|
||||
let result = check_version_parity("0.0.0-test");
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ use fabro_server::serve;
|
|||
use fabro_store::ArtifactStore;
|
||||
use fabro_types::ServerSettings;
|
||||
use fabro_types::settings::server::ServerAuthMethod;
|
||||
use fabro_types::settings::validate_public_url_with_label;
|
||||
use fabro_util::printer::Printer;
|
||||
use fabro_util::terminal::Styles;
|
||||
use fabro_util::version::FABRO_VERSION;
|
||||
|
|
@ -122,6 +123,10 @@ fn merge_server_settings(doc: &mut toml::Value, web_url: &str) -> Result<()> {
|
|||
)
|
||||
}
|
||||
|
||||
fn validate_web_url_arg(value: &str) -> Result<String> {
|
||||
validate_public_url_with_label(value, "--web-url").map_err(anyhow::Error::msg)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn format_config_toml() -> String {
|
||||
let mut doc = toml::Value::Table(toml::Table::default());
|
||||
|
|
@ -1453,6 +1458,7 @@ async fn run_install_github_inner(
|
|||
printer: Printer,
|
||||
) -> Result<()> {
|
||||
let s = Styles::detect_stderr();
|
||||
let web_url = validate_web_url_arg(&args.web_url)?;
|
||||
let fabro_dir = fabro_util::Home::from_env().root().to_path_buf();
|
||||
let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME);
|
||||
if !config_path.exists() {
|
||||
|
|
@ -1498,7 +1504,7 @@ async fn run_install_github_inner(
|
|||
]);
|
||||
let registration = setup_github_app(
|
||||
&s,
|
||||
&args.web_url,
|
||||
&web_url,
|
||||
&owner,
|
||||
username.as_deref(),
|
||||
if args.non_interactive {
|
||||
|
|
@ -1601,7 +1607,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
|
|||
let _cli = &ctx.user_settings().cli;
|
||||
let printer = ctx.printer();
|
||||
let json = ctx.json_output();
|
||||
let web_url = &args.web_url;
|
||||
let web_url = validate_web_url_arg(&args.web_url)?;
|
||||
let s = Styles::detect_stderr();
|
||||
let emoji = console::Emoji("⚒️ ", "");
|
||||
let local_config =
|
||||
|
|
@ -1686,7 +1692,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
|
|||
)?;
|
||||
let registration = setup_github_app(
|
||||
&s,
|
||||
web_url,
|
||||
&web_url,
|
||||
&owner,
|
||||
username.as_deref(),
|
||||
if args.non_interactive {
|
||||
|
|
@ -1740,7 +1746,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
|
|||
);
|
||||
}
|
||||
ServerConfigSelection::Write => {
|
||||
merge_server_settings(&mut doc, web_url)?;
|
||||
merge_server_settings(&mut doc, &web_url)?;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1821,7 +1827,7 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<(
|
|||
)
|
||||
.await?;
|
||||
if let Some(token) = dev_token_for_auth_store {
|
||||
let target = ServerTarget::http_url(&args.web_url)?;
|
||||
let target = ServerTarget::http_url(&web_url)?;
|
||||
if let Err(err) = AuthStore::default().put(
|
||||
&target,
|
||||
AuthEntry::DevToken(DevTokenEntry {
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ pub(crate) mod start;
|
|||
pub(crate) mod status;
|
||||
pub(crate) mod stop;
|
||||
|
||||
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
|
|
@ -303,10 +304,24 @@ fn install_url_hint(bind: &Bind, token: &str) -> Option<String> {
|
|||
return Some(format!("https://{domain}/install?token={token}"));
|
||||
}
|
||||
|
||||
match bind {
|
||||
Bind::Tcp(addr) => Some(format!("http://{addr}/install?token={token}")),
|
||||
Bind::Unix(_) => None,
|
||||
}
|
||||
bind_to_browser_url(bind).map(|url| format!("{url}/install?token={token}"))
|
||||
}
|
||||
|
||||
pub(super) fn bind_to_browser_url(bind: &Bind) -> Option<String> {
|
||||
let Bind::Tcp(addr) = bind else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let browser_addr = match addr.ip() {
|
||||
IpAddr::V4(ip) if ip.is_unspecified() => {
|
||||
SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), addr.port())
|
||||
}
|
||||
IpAddr::V6(ip) if ip.is_unspecified() => {
|
||||
SocketAddr::new(IpAddr::V6(Ipv6Addr::LOCALHOST), addr.port())
|
||||
}
|
||||
_ => *addr,
|
||||
};
|
||||
Some(format!("http://{browser_addr}"))
|
||||
}
|
||||
|
||||
fn default_install_bind_request() -> BindRequest {
|
||||
|
|
@ -345,7 +360,9 @@ fn generate_install_token() -> Result<String> {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::install_mode_next_step_message;
|
||||
use fabro_config::bind::Bind;
|
||||
|
||||
use super::{bind_to_browser_url, install_mode_next_step_message};
|
||||
|
||||
#[test]
|
||||
fn install_mode_next_step_message_recommends_manual_restart_locally() {
|
||||
|
|
@ -362,4 +379,24 @@ mod tests {
|
|||
" After install, the server should restart automatically."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bind_to_browser_url_uses_loopback_for_ipv4_wildcard_bind() {
|
||||
let bind = Bind::Tcp("0.0.0.0:32276".parse().unwrap());
|
||||
|
||||
assert_eq!(
|
||||
bind_to_browser_url(&bind).as_deref(),
|
||||
Some("http://127.0.0.1:32276")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bind_to_browser_url_uses_loopback_for_ipv6_wildcard_bind() {
|
||||
let bind = Bind::Tcp("[::]:32276".parse().unwrap());
|
||||
|
||||
assert_eq!(
|
||||
bind_to_browser_url(&bind).as_deref(),
|
||||
Some("http://[::1]:32276")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -363,8 +363,7 @@ async fn execute_daemon(
|
|||
pid,
|
||||
daemon.bind
|
||||
);
|
||||
if let Bind::Tcp(addr) = &daemon.bind {
|
||||
let url = format!("http://{addr}");
|
||||
if let Some(url) = super::bind_to_browser_url(&daemon.bind) {
|
||||
let styled = match styles {
|
||||
Some(s) => format!("{}", s.cyan.apply_to(&url)),
|
||||
None => url,
|
||||
|
|
|
|||
|
|
@ -143,6 +143,29 @@ fn non_interactive_without_inputs_prints_scripted_usage_and_fails() {
|
|||
assert!(stderr.contains("--github-strategy"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_rejects_wildcard_web_url_before_collecting_inputs() {
|
||||
let context = test_context!();
|
||||
let output = context
|
||||
.command()
|
||||
.args([
|
||||
"install",
|
||||
"--web-url",
|
||||
"http://0.0.0.0:32276",
|
||||
"--non-interactive",
|
||||
])
|
||||
.output()
|
||||
.expect("command should run");
|
||||
|
||||
assert!(!output.status.success());
|
||||
let stderr = String::from_utf8(output.stderr).unwrap();
|
||||
assert!(stderr.contains("--web-url must not use a wildcard host"));
|
||||
assert!(
|
||||
!stderr.contains("Non-interactive install requires additional flags"),
|
||||
"wildcard web URL should be rejected before scripted input validation: {stderr}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hidden_non_interactive_args_require_non_interactive() {
|
||||
let context = test_context!();
|
||||
|
|
|
|||
|
|
@ -3,8 +3,7 @@
|
|||
reason = "Canonical origin validation handles the public server origin; it is not credential-bearing log output."
|
||||
)]
|
||||
|
||||
use fabro_types::settings::ServerNamespace;
|
||||
use url::Url;
|
||||
use fabro_types::settings::{ServerNamespace, validate_public_url};
|
||||
|
||||
use crate::server::EnvLookup;
|
||||
|
||||
|
|
@ -19,12 +18,7 @@ pub(crate) fn resolve_canonical_origin(
|
|||
.map_err(|_| canonical_origin_error(&resolved.web.url.as_source()))?
|
||||
.value;
|
||||
|
||||
let parsed = Url::parse(&value).map_err(|_| canonical_origin_error(&value))?;
|
||||
if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() {
|
||||
return Err(canonical_origin_error(&value));
|
||||
}
|
||||
|
||||
Ok(value)
|
||||
validate_public_url(&value).map_err(|_| canonical_origin_error(&value))
|
||||
}
|
||||
|
||||
fn canonical_origin_error(value: &str) -> String {
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ use fabro_store::ArtifactStore;
|
|||
use fabro_types::ServerSettings;
|
||||
use fabro_types::settings::interp::InterpString;
|
||||
use fabro_types::settings::server::ObjectStoreSettings;
|
||||
use fabro_types::settings::{is_wildcard_host, validate_public_url_with_label};
|
||||
use fabro_util::version::FABRO_VERSION;
|
||||
use fabro_util::{Home, dev_token, session_secret};
|
||||
use fabro_vault::SecretType as VaultSecretType;
|
||||
|
|
@ -768,10 +769,11 @@ async fn put_install_server(
|
|||
.into_response();
|
||||
}
|
||||
|
||||
if let Err(err) = validate_canonical_url(canonical_url) {
|
||||
return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err);
|
||||
}
|
||||
input.canonical_url = canonical_url.to_string();
|
||||
let canonical_url = match validate_public_url_with_label(canonical_url, "canonical_url") {
|
||||
Ok(value) => value,
|
||||
Err(err) => return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err),
|
||||
};
|
||||
input.canonical_url = canonical_url;
|
||||
|
||||
lock_unpoisoned(&state.pending_install, "install session").server = Some(input);
|
||||
info!(step = "server", "install step completed");
|
||||
|
|
@ -1620,7 +1622,34 @@ fn detect_canonical_url(headers: &HeaderMap) -> String {
|
|||
.filter(|value| !value.is_empty())
|
||||
.unwrap_or("127.0.0.1:32276");
|
||||
|
||||
format!("{scheme}://{host}")
|
||||
format!("{scheme}://{}", sanitize_client_facing_host(host))
|
||||
}
|
||||
|
||||
fn sanitize_client_facing_host(host: &str) -> String {
|
||||
let host = host.trim();
|
||||
if let Some(end) = host
|
||||
.strip_prefix('[')
|
||||
.and_then(|rest| rest.find(']').map(|end| end + 1))
|
||||
{
|
||||
let address = &host[1..end];
|
||||
let suffix = &host[end + 1..];
|
||||
if is_wildcard_host(address) {
|
||||
return format!("localhost{suffix}");
|
||||
}
|
||||
return host.to_string();
|
||||
}
|
||||
|
||||
if let Some((address, port)) = host.rsplit_once(':') {
|
||||
if !address.contains(':') && is_wildcard_host(address) {
|
||||
return format!("localhost:{port}");
|
||||
}
|
||||
}
|
||||
|
||||
if is_wildcard_host(host) {
|
||||
return "localhost".to_string();
|
||||
}
|
||||
|
||||
host.to_string()
|
||||
}
|
||||
|
||||
fn completed_steps(pending_install: &PendingInstall) -> Vec<&'static str> {
|
||||
|
|
@ -1692,35 +1721,6 @@ fn install_error_response(status: StatusCode, message: impl Into<String>) -> Res
|
|||
ApiError::new(status, message).into_response()
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::disallowed_types,
|
||||
reason = "Install canonical_url validation parses a public origin and rejects query/fragment credentials before storage."
|
||||
)]
|
||||
fn validate_canonical_url(value: &str) -> Result<(), String> {
|
||||
let trimmed = value.trim();
|
||||
let parsed = fabro_http::Url::parse(trimmed).map_err(|err| err.to_string())?;
|
||||
match parsed.scheme() {
|
||||
"http" | "https" => {}
|
||||
other => return Err(format!("canonical_url must use http or https, got {other}")),
|
||||
}
|
||||
if parsed.host_str().is_none() {
|
||||
return Err("canonical_url must include a host".to_string());
|
||||
}
|
||||
if trimmed.ends_with('/') {
|
||||
return Err("canonical_url must not end with a trailing slash".to_string());
|
||||
}
|
||||
if parsed.path() != "/" {
|
||||
return Err("canonical_url must not include a path".to_string());
|
||||
}
|
||||
if parsed.query().is_some() {
|
||||
return Err("canonical_url must not include a query string".to_string());
|
||||
}
|
||||
if parsed.fragment().is_some() {
|
||||
return Err("canonical_url must not include a fragment".to_string());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn generate_ephemeral_secret() -> String {
|
||||
URL_SAFE_NO_PAD.encode(rand::random::<[u8; 32]>())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8244,7 +8244,12 @@ url = "{url}"
|
|||
|
||||
#[test]
|
||||
fn replace_settings_rejects_invalid_canonical_origin_and_keeps_previous_settings() {
|
||||
for invalid in ["", "/relative/path", "ftp://fabro.example.com"] {
|
||||
for invalid in [
|
||||
"",
|
||||
"/relative/path",
|
||||
"ftp://fabro.example.com",
|
||||
"http://0.0.0.0:32276",
|
||||
] {
|
||||
let state = create_app_state_with_env_lookup(
|
||||
canonical_origin_settings("http://valid.example.com"),
|
||||
RunLayer::default(),
|
||||
|
|
|
|||
|
|
@ -262,6 +262,27 @@ async fn install_session_requires_valid_install_token() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_session_sanitizes_wildcard_host_prefill() {
|
||||
let app = build_install_router(InstallAppState::for_test("test-install-token")).await;
|
||||
|
||||
let response = app
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("GET")
|
||||
.uri("/install/session")
|
||||
.header("authorization", "Bearer test-install-token")
|
||||
.header("host", "0.0.0.0:32276")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let body = response_json(response, StatusCode::OK, "GET /install/session").await;
|
||||
assert_eq!(body["prefill"]["canonical_url"], "http://localhost:32276");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_endpoints_reject_missing_and_wrong_tokens() {
|
||||
let app = build_install_router(InstallAppState::for_test("test-install-token")).await;
|
||||
|
|
@ -1719,6 +1740,44 @@ async fn install_server_rejects_trailing_slash_canonical_urls() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_server_rejects_wildcard_canonical_urls() {
|
||||
let app = build_install_router(InstallAppState::for_test("test-install-token")).await;
|
||||
|
||||
for canonical_url in [
|
||||
"http://0.0.0.0:32276",
|
||||
"http://[::]:32276",
|
||||
"http://0:32276",
|
||||
] {
|
||||
let response = app
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("PUT")
|
||||
.uri("/install/server")
|
||||
.header("authorization", "Bearer test-install-token")
|
||||
.header("content-type", "application/json")
|
||||
.body(Body::from(format!(
|
||||
r#"{{"canonical_url":"{canonical_url}"}}"#
|
||||
)))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let body = response_json(
|
||||
response,
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
"PUT /install/server",
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
body["errors"][0]["detail"],
|
||||
"canonical_url must not use a wildcard host"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys() {
|
||||
let temp_dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
2
lib/crates/fabro-spa/assets/index.html
generated
2
lib/crates/fabro-spa/assets/index.html
generated
|
|
@ -59,7 +59,7 @@
|
|||
<script type="module" src="/assets/chunk-nkw6kj41.js"></script>
|
||||
<script type="module" src="/assets/chunk-71s03bbh.js"></script>
|
||||
<script type="module" src="/assets/chunk-ept66kdn.js"></script>
|
||||
<script type="module" src="/assets/entry-zcpgp9fa.js"></script>
|
||||
<script type="module" src="/assets/entry-xf46xn8z.js"></script>
|
||||
<script type="module" src="/assets/chunk-dep0g6mr.js"></script>
|
||||
<script type="module" src="/assets/chunk-c8zhk10v.js"></script>
|
||||
<script type="module" src="/assets/chunk-n9tc6j4j.js"></script>
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ sha2.workspace = true
|
|||
strum.workspace = true
|
||||
toml.workspace = true
|
||||
ulid.workspace = true
|
||||
url.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
tempfile = "3"
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ pub mod features;
|
|||
pub mod interp;
|
||||
pub mod model_ref;
|
||||
pub mod project;
|
||||
pub mod public_url;
|
||||
pub mod run;
|
||||
pub mod server;
|
||||
pub mod size;
|
||||
|
|
@ -31,6 +32,9 @@ pub use model_ref::{
|
|||
AmbiguousModelRef, ModelRef, ModelRegistry, ParseModelRefError, ResolvedModelRef,
|
||||
};
|
||||
pub use project::ProjectNamespace;
|
||||
pub use public_url::{
|
||||
is_wildcard_host, replace_wildcard_host, validate_public_url, validate_public_url_with_label,
|
||||
};
|
||||
pub use run::{
|
||||
ArtifactsSettings, DaytonaSettings, DaytonaSnapshotSettings, DockerfileSource,
|
||||
GitAuthorSettings, HookDefinition, HookType, InterviewProviderSettings, McpServerSettings,
|
||||
|
|
|
|||
94
lib/crates/fabro-types/src/settings/public_url.rs
Normal file
94
lib/crates/fabro-types/src/settings/public_url.rs
Normal file
|
|
@ -0,0 +1,94 @@
|
|||
#![expect(
|
||||
clippy::disallowed_types,
|
||||
reason = "Public URL validation parses raw configured URLs before storage or display; callers do not log credentials from parsed URLs."
|
||||
)]
|
||||
|
||||
use std::net::IpAddr;
|
||||
|
||||
use url::Url;
|
||||
|
||||
pub fn validate_public_url(value: &str) -> Result<String, String> {
|
||||
validate_public_url_with_label(value, "public URL")
|
||||
}
|
||||
|
||||
pub fn validate_public_url_with_label(value: &str, label: &str) -> Result<String, String> {
|
||||
let trimmed = value.trim();
|
||||
let parsed = Url::parse(trimmed).map_err(|err| err.to_string())?;
|
||||
match parsed.scheme() {
|
||||
"http" | "https" => {}
|
||||
other => return Err(format!("{label} must use http or https, got {other}")),
|
||||
}
|
||||
let host = parsed
|
||||
.host_str()
|
||||
.ok_or_else(|| format!("{label} must include a host"))?;
|
||||
if is_wildcard_host(host) {
|
||||
return Err(format!("{label} must not use a wildcard host"));
|
||||
}
|
||||
if trimmed.ends_with('/') {
|
||||
return Err(format!("{label} must not end with a trailing slash"));
|
||||
}
|
||||
if parsed.path() != "/" {
|
||||
return Err(format!("{label} must not include a path"));
|
||||
}
|
||||
if parsed.query().is_some() {
|
||||
return Err(format!("{label} must not include a query string"));
|
||||
}
|
||||
if parsed.fragment().is_some() {
|
||||
return Err(format!("{label} must not include a fragment"));
|
||||
}
|
||||
Ok(trimmed.to_string())
|
||||
}
|
||||
|
||||
pub fn is_wildcard_host(host: &str) -> bool {
|
||||
let host = host.trim().trim_start_matches('[').trim_end_matches(']');
|
||||
host == "0"
|
||||
|| host
|
||||
.parse::<IpAddr>()
|
||||
.is_ok_and(|addr| addr.is_unspecified())
|
||||
}
|
||||
|
||||
pub fn replace_wildcard_host(value: &str, replacement_host: &str) -> Option<String> {
|
||||
let parsed = Url::parse(value.trim()).ok()?;
|
||||
let host = parsed.host_str()?;
|
||||
if !is_wildcard_host(host) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let port = parsed
|
||||
.port()
|
||||
.map(|port| format!(":{port}"))
|
||||
.unwrap_or_default();
|
||||
Some(format!(
|
||||
"{}://{}{}",
|
||||
parsed.scheme(),
|
||||
replacement_host,
|
||||
port
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{replace_wildcard_host, validate_public_url_with_label};
|
||||
|
||||
#[test]
|
||||
fn validate_public_url_rejects_wildcard_hosts() {
|
||||
for value in [
|
||||
"http://0.0.0.0:32276",
|
||||
"http://[::]:32276",
|
||||
"http://0:32276",
|
||||
] {
|
||||
assert_eq!(
|
||||
validate_public_url_with_label(value, "canonical_url").unwrap_err(),
|
||||
"canonical_url must not use a wildcard host"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replace_wildcard_host_preserves_scheme_and_port() {
|
||||
assert_eq!(
|
||||
replace_wildcard_host("http://0.0.0.0:32276", "127.0.0.1").as_deref(),
|
||||
Some("http://127.0.0.1:32276")
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue