mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
fix(auth): harden loopback checks and align CSP tests
Tighten CLI loopback target classification to use literal host checks, update explicit local TCP auth coverage to match the remote-target contract, and align server CSP assertions with the current external-script SPA bundle. Also enable reqwest cookies in fabro-http so package-scoped server tests compile without relying on workspace feature unification.
This commit is contained in:
parent
323c797e0f
commit
ed6fecfc5a
5 changed files with 132 additions and 28 deletions
|
|
@ -32,29 +32,34 @@ pub(crate) fn is_loopback_or_unix_socket(
|
|||
}
|
||||
|
||||
fn classify_http_target(api_url: &str) -> Result<LoopbackClassification, TargetSchemeError> {
|
||||
let url = fabro_http::Url::parse(user_config::normalized_http_base_url(api_url)).map_err(
|
||||
|source| TargetSchemeError::InvalidUrl {
|
||||
let normalized = user_config::normalized_http_base_url(api_url);
|
||||
let url =
|
||||
fabro_http::Url::parse(normalized).map_err(|source| TargetSchemeError::InvalidUrl {
|
||||
value: api_url.to_string(),
|
||||
reason: source.to_string(),
|
||||
},
|
||||
)?;
|
||||
})?;
|
||||
|
||||
match url.scheme() {
|
||||
"https" => Ok(LoopbackClassification::Https),
|
||||
"http" => {
|
||||
let Some(host) = url.host_str() else {
|
||||
if url.host_str().is_none() {
|
||||
return Err(TargetSchemeError::MissingHost {
|
||||
value: api_url.to_string(),
|
||||
});
|
||||
}
|
||||
if !url.username().is_empty() || url.password().is_some() {
|
||||
return Ok(LoopbackClassification::Rejected);
|
||||
}
|
||||
let Some(authority) = raw_authority(normalized) else {
|
||||
return Err(TargetSchemeError::MissingHost {
|
||||
value: api_url.to_string(),
|
||||
});
|
||||
};
|
||||
let classification = host
|
||||
.parse::<IpAddr>()
|
||||
.ok()
|
||||
.filter(ip_is_loopback)
|
||||
.map_or(LoopbackClassification::Rejected, |_| {
|
||||
LoopbackClassification::LoopbackHttp
|
||||
});
|
||||
Ok(classification)
|
||||
Ok(if raw_host_is_loopback_literal(authority) {
|
||||
LoopbackClassification::LoopbackHttp
|
||||
} else {
|
||||
LoopbackClassification::Rejected
|
||||
})
|
||||
}
|
||||
scheme => Err(TargetSchemeError::UnsupportedScheme {
|
||||
scheme: scheme.to_string(),
|
||||
|
|
@ -62,6 +67,47 @@ fn classify_http_target(api_url: &str) -> Result<LoopbackClassification, TargetS
|
|||
}
|
||||
}
|
||||
|
||||
fn raw_authority(url: &str) -> Option<&str> {
|
||||
let (_, remainder) = url.split_once("://")?;
|
||||
let end = remainder
|
||||
.find(|ch| ['/', '?', '#'].contains(&ch))
|
||||
.unwrap_or(remainder.len());
|
||||
Some(&remainder[..end])
|
||||
}
|
||||
|
||||
fn raw_host_is_loopback_literal(authority: &str) -> bool {
|
||||
if authority.contains('@') {
|
||||
return false;
|
||||
}
|
||||
let Some(host) = raw_host(authority) else {
|
||||
return false;
|
||||
};
|
||||
match host.parse::<IpAddr>().ok() {
|
||||
Some(IpAddr::V4(ipv4)) => host.contains('.') && ipv4.is_loopback(),
|
||||
Some(ip @ IpAddr::V6(_)) => ip_is_loopback(&ip),
|
||||
None => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn raw_host(authority: &str) -> Option<&str> {
|
||||
if authority.is_empty() {
|
||||
return None;
|
||||
}
|
||||
if authority.starts_with('[') {
|
||||
let end = authority.find(']')?;
|
||||
let remainder = &authority[end + 1..];
|
||||
if !remainder.is_empty() && !remainder.starts_with(':') {
|
||||
return None;
|
||||
}
|
||||
return Some(&authority[1..end]);
|
||||
}
|
||||
|
||||
let host = authority
|
||||
.split_once(':')
|
||||
.map_or(authority, |(host, _)| host);
|
||||
if host.is_empty() { None } else { Some(host) }
|
||||
}
|
||||
|
||||
fn ip_is_loopback(ip: &IpAddr) -> bool {
|
||||
match ip {
|
||||
IpAddr::V4(ipv4) => ipv4.is_loopback(),
|
||||
|
|
@ -125,6 +171,7 @@ mod tests {
|
|||
let cases = [
|
||||
"http://fabro.example.com",
|
||||
"http://127.0.0.1.evil.com",
|
||||
"http://127.0.0.1:1@attacker.com",
|
||||
"http://localhost",
|
||||
"http://localhost.evil.com",
|
||||
"http://2130706433",
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use fabro_test::{fabro_snapshot, test_context};
|
|||
use httpmock::MockServer;
|
||||
use serde_json::Value;
|
||||
|
||||
use super::support::{setup_completed_fast_dry_run, setup_created_fast_dry_run};
|
||||
use super::support::{local_dev_token, setup_completed_fast_dry_run, setup_created_fast_dry_run};
|
||||
use crate::support::unique_run_id;
|
||||
|
||||
#[test]
|
||||
|
|
@ -36,7 +36,7 @@ fn help() {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn ps_accepts_local_tcp_server_target() {
|
||||
fn ps_explicit_local_tcp_server_target_requires_explicit_auth() {
|
||||
let context = test_context!();
|
||||
let storage_root = tempfile::tempdir_in("/tmp").unwrap();
|
||||
let storage_dir = storage_root.path().join("storage");
|
||||
|
|
@ -77,9 +77,66 @@ fn ps_accepts_local_tcp_server_target() {
|
|||
.assert()
|
||||
.success();
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"ps against an explicit local TCP target should require explicit auth:\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert!(
|
||||
String::from_utf8_lossy(&output.stderr).contains("Authentication required."),
|
||||
"explicit local TCP target should fail with an auth error:\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ps_explicit_local_tcp_server_target_accepts_explicit_dev_token() {
|
||||
let context = test_context!();
|
||||
let storage_root = tempfile::tempdir_in("/tmp").unwrap();
|
||||
let storage_dir = storage_root.path().join("storage");
|
||||
std::fs::create_dir_all(&storage_dir).unwrap();
|
||||
|
||||
context
|
||||
.command()
|
||||
.env("FABRO_STORAGE_DIR", &storage_dir)
|
||||
.args(["server", "start", "--bind", "127.0.0.1"])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
let status_output = context
|
||||
.command()
|
||||
.env("FABRO_STORAGE_DIR", &storage_dir)
|
||||
.args(["server", "status", "--json"])
|
||||
.assert()
|
||||
.success()
|
||||
.get_output()
|
||||
.stdout
|
||||
.clone();
|
||||
let status_json: Value = serde_json::from_slice(&status_output).unwrap();
|
||||
let bind = status_json["bind"]
|
||||
.as_str()
|
||||
.expect("bind should be present");
|
||||
let token = local_dev_token(&storage_dir).expect("local dev token should exist");
|
||||
|
||||
let output = context
|
||||
.command()
|
||||
.env("FABRO_DEV_TOKEN", &token)
|
||||
.args(["ps", "-a", "--json", "--server", &format!("http://{bind}")])
|
||||
.output()
|
||||
.expect("ps should run");
|
||||
|
||||
context
|
||||
.command()
|
||||
.env("FABRO_STORAGE_DIR", &storage_dir)
|
||||
.args(["server", "stop"])
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"ps against local TCP target failed:\nstdout:\n{}\nstderr:\n{}",
|
||||
"ps against local TCP target with explicit auth failed:\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ doctest = false
|
|||
workspace = true
|
||||
|
||||
[dependencies]
|
||||
reqwest = { workspace = true, features = ["blocking"] }
|
||||
reqwest = { workspace = true, features = ["blocking", "cookies"] }
|
||||
thiserror.workspace = true
|
||||
|
||||
[dev-dependencies]
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
//! Content Security Policy generation.
|
||||
//!
|
||||
//! The policy is built once at server startup from the embedded SPA
|
||||
//! `index.html` so inline `<script>` hashes don't drift from the
|
||||
//! `index.html` so any inline `<script>` hashes don't drift from the
|
||||
//! template. Third-party sources are enumerated explicitly — the only
|
||||
//! outside origins the UI depends on today are Google Fonts.
|
||||
|
||||
|
|
@ -160,13 +160,14 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn embedded_spa_index_yields_at_least_one_hash() {
|
||||
// Guards against the template silently losing its inline
|
||||
// theme-bootstrap script, which would silently break CSP.
|
||||
let hashes = inline_script_hashes_from_embedded_index();
|
||||
fn embedded_spa_index_builds_a_policy() {
|
||||
// Guards against the embedded asset going missing or becoming
|
||||
// unreadable in a way that would leave the CSP header blank.
|
||||
let policy = build_policy();
|
||||
assert!(
|
||||
!hashes.is_empty(),
|
||||
"expected the embedded SPA index.html to contain at least one inline <script>"
|
||||
policy.contains("script-src 'self'"),
|
||||
"embedded SPA policy should contain a script-src directive"
|
||||
);
|
||||
assert!(policy.contains("'wasm-unsafe-eval'"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -309,6 +309,8 @@ async fn security_headers_are_applied_to_all_responses() {
|
|||
// CSP is shipped in Report-Only mode and must cover the sources the
|
||||
// embedded SPA actually loads: same-origin scripts, Google Fonts,
|
||||
// WASM instantiation (viz-js), data: and blob: images, blob: workers.
|
||||
// Inline hashes are optional because the current SPA ships only
|
||||
// external module scripts.
|
||||
let csp = spa_response
|
||||
.headers()
|
||||
.get("content-security-policy-report-only")
|
||||
|
|
@ -316,11 +318,8 @@ async fn security_headers_are_applied_to_all_responses() {
|
|||
.to_str()
|
||||
.expect("CSP should be ASCII");
|
||||
assert!(csp.contains("default-src 'self'"), "got: {csp}");
|
||||
assert!(csp.contains("script-src 'self'"), "got: {csp}");
|
||||
assert!(csp.contains("'wasm-unsafe-eval'"), "got: {csp}");
|
||||
assert!(
|
||||
csp.contains("'sha256-"),
|
||||
"inline-script hash missing: {csp}"
|
||||
);
|
||||
assert!(
|
||||
csp.contains("style-src 'self' https://fonts.googleapis.com 'unsafe-inline'"),
|
||||
"got: {csp}"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue