Seal the fetch target: one value holds the URL and its host

Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)
This commit is contained in:
2026-09-22 23:03:49 -07:00
parent f095cca1ee
commit bf9e79ac21
5 changed files with 53 additions and 21 deletions
+40 -9
View File
@@ -66,10 +66,39 @@ pub enum ToolArgs {
command: String,
cwd: Option<String>,
},
HttpFetch {
url: String,
host: String,
},
HttpFetch(FetchUrl),
}
/// A URL that passed the checks, and its host. Only `parse` makes one, so the host policy matched
/// is always the host of the URL the tool fetches.
///
/// ```compile_fail
/// let _ = brokerd::args::FetchUrl {
/// url: "https://evil.example/".to_string(),
/// host: "example.com".to_string(),
/// };
/// ```
///
/// ```
/// let args = brokerd::args::parse(brokerd::args::ToolName::HttpFetch, r#"{"url":"https://example.com/a"}"#);
/// let Ok(brokerd::args::ToolArgs::HttpFetch(target)) = args else { panic!("{args:?}") };
/// assert_eq!((target.url(), target.host()), ("https://example.com/a", "example.com"));
/// ```
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FetchUrl {
url: String, // private
host: String, // private
}
impl FetchUrl {
/// The URL that passed the checks.
pub fn url(&self) -> &str {
&self.url
}
/// The host of that URL, as `parse` derived it.
pub fn host(&self) -> &str {
&self.host
}
}
impl ToolArgs {
@@ -79,7 +108,7 @@ impl ToolArgs {
ToolArgs::ReadFile { .. } => ToolName::ReadFile,
ToolArgs::WriteFile { .. } => ToolName::WriteFile,
ToolArgs::Shell { .. } => ToolName::Shell,
ToolArgs::HttpFetch { .. } => ToolName::HttpFetch,
ToolArgs::HttpFetch(_) => ToolName::HttpFetch,
}
}
@@ -107,8 +136,10 @@ impl ToolArgs {
};
serde_json::to_string(&value).unwrap_or_else(|_| "{}".to_string())
}
ToolArgs::HttpFetch { url, .. } => {
let value = HttpFetchArgs { url: url.clone() };
ToolArgs::HttpFetch(target) => {
let value = HttpFetchArgs {
url: target.url.clone(),
};
serde_json::to_string(&value).unwrap_or_else(|_| "{}".to_string())
}
}
@@ -192,10 +223,10 @@ pub fn parse(tool: ToolName, arguments: &str) -> Result<ToolArgs, ArgsError> {
Some(host) => host.to_string(),
None => return Err(ArgsError::Url(value.url)),
};
Ok(ToolArgs::HttpFetch {
Ok(ToolArgs::HttpFetch(FetchUrl {
url: value.url,
host,
})
}))
}
}
}
+2 -2
View File
@@ -404,12 +404,12 @@ fn covers(args: &ToolArgs, grant: &Grant) -> Option<Option<String>> {
}
}
ToolArgs::Shell { cwd: Some(cwd), .. } => best_path(grant, cwd, true),
ToolArgs::HttpFetch { host, .. } => {
ToolArgs::HttpFetch(target) => {
if grant
.constraints
.hosts
.iter()
.any(|pattern| host_matches(pattern, host))
.any(|pattern| host_matches(pattern, target.host()))
{
Some(None)
} else {
+1 -1
View File
@@ -105,7 +105,7 @@ pub fn run(decision: Decision, runtime: &dyn Runtime) -> proto::ToolResponse {
.collect(),
None,
),
ToolArgs::HttpFetch { .. } => (Vec::new(), Some(decision.hosts().to_vec())),
ToolArgs::HttpFetch(_) => (Vec::new(), Some(decision.hosts().to_vec())),
};
let spec = RunSpec {
tool: args.tool(),
+9 -9
View File
@@ -289,16 +289,16 @@ fn each_tool_parses_its_own_arguments() {
cwd: Some("/home/kyle".to_string())
})
);
assert_eq!(
parse(
ToolName::HttpFetch,
r#"{"url":"https://www.example.com/a"}"#
),
Ok(ToolArgs::HttpFetch {
url: "https://www.example.com/a".to_string(),
host: "www.example.com".to_string()
})
// A `FetchUrl` cannot be built outside `args`, so the parsed value is read through its getters.
let fetched = parse(
ToolName::HttpFetch,
r#"{"url":"https://www.example.com/a"}"#,
);
let Ok(ToolArgs::HttpFetch(target)) = fetched else {
panic!("{fetched:?}")
};
assert_eq!(target.url(), "https://www.example.com/a");
assert_eq!(target.host(), "www.example.com");
// Field order and white space in the request do not matter.
assert_eq!(
parse(