Add typed tool arguments and the form checks for paths, hosts and URLs
Implemented brokerd::args: a pure module (no I/O, no clock) that parses tool arguments into a typed ToolArgs and checks the form of paths, hosts and URLs. Four private deny_unknown_fields structs drive parse and canonical_json; path, cwd and url are validated as written and never normalised. All 13 args tests pass and make gate prints gate: ok. Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)
This commit is contained in:
@@ -0,0 +1,428 @@
|
||||
//! Tests for tool arguments: their shape, and the form of paths, hosts and URLs. Do not edit.
|
||||
//!
|
||||
//! Every row of the "Paths" and "Hosts" tables in the M3a spec is a case here.
|
||||
|
||||
use brokerd::args::{
|
||||
ArgsError, MAX_PATH, MAX_URL, ToolArgs, ToolName, host_matches, inside, parse, url_host,
|
||||
valid_host, valid_host_pattern, valid_path,
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn the_four_tool_names() {
|
||||
let names = ["read_file", "write_file", "shell", "http_fetch"];
|
||||
for (tool, name) in ToolName::ALL.into_iter().zip(names) {
|
||||
assert_eq!(tool.as_str(), name);
|
||||
assert_eq!(ToolName::parse(name), Some(tool));
|
||||
}
|
||||
for other in [
|
||||
"",
|
||||
"echo",
|
||||
"clock",
|
||||
"call_tool",
|
||||
"Read_File",
|
||||
"read_file ",
|
||||
"readfile",
|
||||
] {
|
||||
assert_eq!(ToolName::parse(other), None, "{other:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_paths() {
|
||||
let longest = format!("/{}", "a".repeat(MAX_PATH - 1));
|
||||
assert_eq!(longest.len(), MAX_PATH);
|
||||
for path in [
|
||||
"/",
|
||||
"/etc",
|
||||
"/home/kyle/notes/a.md",
|
||||
"/home/kyle/notes",
|
||||
"/with space/and\ttab",
|
||||
"/dots.in.names/..hidden/...",
|
||||
"/unicode/\u{e9}t\u{e9}",
|
||||
longest.as_str(),
|
||||
] {
|
||||
assert!(valid_path(path), "{path:?} should be valid");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_paths() {
|
||||
let too_long = format!("/{}", "a".repeat(MAX_PATH));
|
||||
assert_eq!(too_long.len(), MAX_PATH + 1);
|
||||
for path in [
|
||||
"",
|
||||
"notes/a.md",
|
||||
"./notes",
|
||||
"~/notes",
|
||||
"/home/kyle/notes/../.ssh/id",
|
||||
"/home/kyle//notes/./a.md",
|
||||
"/home//kyle",
|
||||
"/home/./kyle",
|
||||
"/home/kyle/",
|
||||
"/home/kyle/..",
|
||||
"/..",
|
||||
"/.",
|
||||
"//",
|
||||
"/nul\0byte",
|
||||
too_long.as_str(),
|
||||
] {
|
||||
assert!(!valid_path(path), "{path:?} should be invalid");
|
||||
}
|
||||
}
|
||||
|
||||
/// The table in the spec, row by row, for the rows about form and containment.
|
||||
#[test]
|
||||
fn inside_is_by_whole_components() {
|
||||
let grant = "/home/kyle/notes";
|
||||
assert!(inside(grant, "/home/kyle/notes/a.md"));
|
||||
assert!(inside(grant, "/home/kyle/notes"));
|
||||
assert!(inside(grant, "/home/kyle/notes/deep/er/b.md"));
|
||||
assert!(!inside(grant, "/home/kyle/notes2/a.md"));
|
||||
assert!(!inside(grant, "/home/kyle/note"));
|
||||
assert!(!inside(grant, "/home/kyle"));
|
||||
assert!(!inside(grant, "/"));
|
||||
assert!(!inside(grant, "/other/home/kyle/notes/a.md"));
|
||||
// A grant of the root is refused when grants are loaded, but the function is still right.
|
||||
assert!(inside("/", "/etc/passwd"));
|
||||
assert!(inside("/", "/"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_hosts_and_patterns() {
|
||||
let label63 = "a".repeat(63);
|
||||
let long = format!("{label63}.{label63}.{label63}.{}.com", "b".repeat(57));
|
||||
assert_eq!(long.len(), 253);
|
||||
for host in [
|
||||
"example.com",
|
||||
"www.example.com",
|
||||
"a.b.example.com",
|
||||
"xn--bcher-kva.example",
|
||||
"1password.com",
|
||||
"3.example.org",
|
||||
"a-b.c-d.io",
|
||||
long.as_str(),
|
||||
] {
|
||||
assert!(valid_host(host), "{host:?} should be a valid host");
|
||||
assert!(
|
||||
valid_host_pattern(host),
|
||||
"{host:?} should be a valid pattern"
|
||||
);
|
||||
let wild = format!("*.{host}");
|
||||
assert!(!valid_host(&wild), "{wild:?} is a pattern, not a host");
|
||||
}
|
||||
assert!(valid_host_pattern("*.example.com"));
|
||||
assert!(valid_host_pattern("*.a.b.example.com"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_hosts_and_patterns() {
|
||||
let label64 = format!("{}.com", "a".repeat(64));
|
||||
let too_long = format!("{}.com", ["a".repeat(63).as_str(); 4].join("."));
|
||||
assert!(too_long.len() > 253);
|
||||
for host in [
|
||||
"",
|
||||
"localhost",
|
||||
"com",
|
||||
"Example.com",
|
||||
"example.COM",
|
||||
"example.com.",
|
||||
".example.com",
|
||||
"example..com",
|
||||
"-example.com",
|
||||
"example-.com",
|
||||
"exa_mple.com",
|
||||
"example.com:443",
|
||||
"example.com/path",
|
||||
"user@example.com",
|
||||
"exa mple.com",
|
||||
"[::1]",
|
||||
"::1",
|
||||
// Every spelling of an IPv4 address: the last label does not start with a letter.
|
||||
"127.0.0.1",
|
||||
"127.1",
|
||||
"10.0.0.0x1",
|
||||
"1.2.3.4",
|
||||
"example.123",
|
||||
"b\u{fc}cher.example",
|
||||
label64.as_str(),
|
||||
too_long.as_str(),
|
||||
] {
|
||||
assert!(!valid_host(host), "{host:?} should not be a valid host");
|
||||
assert!(
|
||||
!valid_host_pattern(host),
|
||||
"{host:?} should not be a valid pattern"
|
||||
);
|
||||
}
|
||||
for pattern in [
|
||||
"*",
|
||||
"*.",
|
||||
"*.com",
|
||||
"*example.com",
|
||||
"www.*.com",
|
||||
"*.*.example.com",
|
||||
"**.example.com",
|
||||
"*.Example.com",
|
||||
"*.127.0.0.1",
|
||||
] {
|
||||
assert!(!valid_host_pattern(pattern), "{pattern:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// The host table in the spec, row by row.
|
||||
#[test]
|
||||
fn host_matching() {
|
||||
assert!(host_matches("example.com", "example.com"));
|
||||
assert!(!host_matches("example.com", "www.example.com"));
|
||||
assert!(host_matches("*.example.com", "www.example.com"));
|
||||
assert!(host_matches("*.example.com", "a.b.example.com"));
|
||||
assert!(!host_matches("*.example.com", "example.com"));
|
||||
// A suffix is not enough: the match is by whole labels.
|
||||
assert!(!host_matches("*.example.com", "badexample.com"));
|
||||
assert!(!host_matches("*.example.com", "www.example.com.evil.org"));
|
||||
assert!(!host_matches("example.com", "example.com.evil.org"));
|
||||
assert!(!host_matches("*.example.com", ".example.com"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_urls_and_their_hosts() {
|
||||
let base = "https://example.com/";
|
||||
let longest = format!("{base}{}", "a".repeat(MAX_URL - base.len()));
|
||||
assert_eq!(longest.len(), MAX_URL);
|
||||
for (url, host) in [
|
||||
("https://example.com", "example.com"),
|
||||
("https://example.com/", "example.com"),
|
||||
("https://example.com:443", "example.com"),
|
||||
("https://example.com:443/", "example.com"),
|
||||
("https://www.example.com/a/b.html", "www.example.com"),
|
||||
("https://example.com/search?q=a+b&x=%20#frag", "example.com"),
|
||||
("https://example.com/@user", "example.com"),
|
||||
("https://example.com/a:8080/b", "example.com"),
|
||||
("https://example.com/https://other.org/", "example.com"),
|
||||
("https://example.com/back\\slash", "example.com"),
|
||||
(longest.as_str(), "example.com"),
|
||||
] {
|
||||
assert_eq!(url_host(url), Some(host), "{url}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_urls() {
|
||||
let base = "https://example.com/";
|
||||
let too_long = format!("{base}{}", "a".repeat(MAX_URL - base.len() + 1));
|
||||
assert_eq!(too_long.len(), MAX_URL + 1);
|
||||
for url in [
|
||||
"",
|
||||
"example.com",
|
||||
"http://example.com/",
|
||||
"HTTPS://example.com/",
|
||||
"https:/example.com/",
|
||||
"https://",
|
||||
"https:///path",
|
||||
"ftp://example.com/",
|
||||
"file:///etc/passwd",
|
||||
// userinfo
|
||||
"https://user@example.com/",
|
||||
"https://user:pw@example.com/",
|
||||
"https://example.com@evil.org/",
|
||||
// ports
|
||||
"https://example.com:8443/",
|
||||
"https://example.com:80/",
|
||||
"https://example.com:/",
|
||||
"https://example.com:443x/",
|
||||
"https://example.com:4433/",
|
||||
"https://example.com:443:443/",
|
||||
// what follows the host must be the end, `:443` or `/`
|
||||
"https://example.com?q=1",
|
||||
"https://example.com#frag",
|
||||
"https://example.com\\@evil.org/",
|
||||
// hosts that are not host names
|
||||
"https://localhost/",
|
||||
"https://127.0.0.1/",
|
||||
"https://127.1/",
|
||||
"https://[::1]/",
|
||||
"https://Example.com/",
|
||||
"https://example.com./",
|
||||
"https://b\u{fc}cher.example/",
|
||||
// the rest must be printable ASCII with no space
|
||||
"https://example.com/a b",
|
||||
"https://example.com/a\tb",
|
||||
"https://example.com/a\nb",
|
||||
"https://example.com/caf\u{e9}",
|
||||
"https://example.com/\u{7f}",
|
||||
" https://example.com/",
|
||||
"https://example.com/ ",
|
||||
too_long.as_str(),
|
||||
] {
|
||||
assert_eq!(url_host(url), None, "{url:?} should be invalid");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn each_tool_parses_its_own_arguments() {
|
||||
assert_eq!(
|
||||
parse(ToolName::ReadFile, r#"{"path":"/home/kyle/notes/a.md"}"#),
|
||||
Ok(ToolArgs::ReadFile {
|
||||
path: "/home/kyle/notes/a.md".to_string()
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
parse(
|
||||
ToolName::WriteFile,
|
||||
r#"{"path":"/home/kyle/notes/a.md","content":"line\n"}"#
|
||||
),
|
||||
Ok(ToolArgs::WriteFile {
|
||||
path: "/home/kyle/notes/a.md".to_string(),
|
||||
content: "line\n".to_string()
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
parse(ToolName::Shell, r#"{"command":"ls -l"}"#),
|
||||
Ok(ToolArgs::Shell {
|
||||
command: "ls -l".to_string(),
|
||||
cwd: None
|
||||
})
|
||||
);
|
||||
assert_eq!(
|
||||
parse(ToolName::Shell, r#"{"command":"ls","cwd":"/home/kyle"}"#),
|
||||
Ok(ToolArgs::Shell {
|
||||
command: "ls".to_string(),
|
||||
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()
|
||||
})
|
||||
);
|
||||
// Field order and white space in the request do not matter.
|
||||
assert_eq!(
|
||||
parse(
|
||||
ToolName::WriteFile,
|
||||
" { \"content\" : \"x\" , \"path\" : \"/a/b\" } "
|
||||
),
|
||||
Ok(ToolArgs::WriteFile {
|
||||
path: "/a/b".to_string(),
|
||||
content: "x".to_string()
|
||||
})
|
||||
);
|
||||
// `command` and `content` are not inspected.
|
||||
assert!(parse(ToolName::Shell, r#"{"command":""}"#).is_ok());
|
||||
assert!(parse(ToolName::Shell, r#"{"command":"rm -rf / ; ../../x"}"#).is_ok());
|
||||
assert!(parse(ToolName::WriteFile, r#"{"path":"/a/b","content":""}"#).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn arguments_of_the_wrong_shape_are_refused() {
|
||||
let cases: [(ToolName, &str); 17] = [
|
||||
(ToolName::ReadFile, ""),
|
||||
(ToolName::ReadFile, "null"),
|
||||
(ToolName::ReadFile, "[]"),
|
||||
(ToolName::ReadFile, r#""/etc/hosts""#),
|
||||
(ToolName::ReadFile, "{}"),
|
||||
(ToolName::ReadFile, r#"{"path":"/a","mode":"r"}"#),
|
||||
(ToolName::ReadFile, r#"{"path":"/a","path":"/b"}"#),
|
||||
(ToolName::ReadFile, r#"{"path":7}"#),
|
||||
(ToolName::ReadFile, r#"{"path":null}"#),
|
||||
(ToolName::ReadFile, r#"{"path":"/a"} trailing"#),
|
||||
(ToolName::WriteFile, r#"{"path":"/a/b"}"#),
|
||||
(ToolName::WriteFile, r#"{"content":"x"}"#),
|
||||
(
|
||||
ToolName::WriteFile,
|
||||
r#"{"path":"/a/b","content":"x","append":true}"#,
|
||||
),
|
||||
(ToolName::Shell, r#"{"cwd":"/a"}"#),
|
||||
(ToolName::Shell, r#"{"command":"ls","cwd":"/a","env":{}}"#),
|
||||
(ToolName::Shell, r#"{"command":["ls"]}"#),
|
||||
(
|
||||
ToolName::HttpFetch,
|
||||
r#"{"url":"https://example.com/","method":"POST"}"#,
|
||||
),
|
||||
];
|
||||
for (tool, text) in cases {
|
||||
match parse(tool, text) {
|
||||
Err(ArgsError::Shape(_)) => {}
|
||||
other => panic!("{tool:?} {text:?}: expected a shape error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
// One tool's arguments do not fit another tool.
|
||||
assert!(parse(ToolName::Shell, r#"{"path":"/a"}"#).is_err());
|
||||
assert!(parse(ToolName::ReadFile, r#"{"url":"https://example.com/"}"#).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_bad_path_or_url_is_refused_in_every_tool_that_takes_one() {
|
||||
for bad in ["notes/a.md", "/home/kyle/notes/../.ssh/id", "/a//b", "/a/"] {
|
||||
let quoted = serde_json::to_string(bad).unwrap();
|
||||
let read = format!(r#"{{"path":{quoted}}}"#);
|
||||
let write = format!(r#"{{"path":{quoted},"content":"x"}}"#);
|
||||
let shell = format!(r#"{{"command":"ls","cwd":{quoted}}}"#);
|
||||
assert_eq!(
|
||||
parse(ToolName::ReadFile, &read),
|
||||
Err(ArgsError::Path(bad.to_string()))
|
||||
);
|
||||
assert_eq!(
|
||||
parse(ToolName::WriteFile, &write),
|
||||
Err(ArgsError::Path(bad.to_string()))
|
||||
);
|
||||
assert_eq!(
|
||||
parse(ToolName::Shell, &shell),
|
||||
Err(ArgsError::Path(bad.to_string()))
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
parse(ToolName::HttpFetch, r#"{"url":"http://example.com/"}"#),
|
||||
Err(ArgsError::Url("http://example.com/".to_string()))
|
||||
);
|
||||
// A NUL can only arrive as a JSON escape; it is refused once decoded.
|
||||
let nul = format!("{{\"path\":\"/a{}u0000b\"}}", '\\');
|
||||
assert!(matches!(
|
||||
parse(ToolName::ReadFile, &nul),
|
||||
Err(ArgsError::Path(_))
|
||||
));
|
||||
// `cwd: null` is the same as no `cwd`.
|
||||
assert_eq!(
|
||||
parse(ToolName::Shell, r#"{"command":"ls","cwd":null}"#),
|
||||
Ok(ToolArgs::Shell {
|
||||
command: "ls".to_string(),
|
||||
cwd: None
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
/// What the owner is shown is the parsed value written out again, so two spellings of one path
|
||||
/// look the same. The escape is built from pieces so that no tool rewrites it on the way here.
|
||||
#[test]
|
||||
fn canonical_json_shows_what_was_parsed() {
|
||||
let escaped_slash = format!("{}u002f", '\\');
|
||||
let sneaky = format!("{{ \"path\" : \"{escaped_slash}etc{escaped_slash}hosts\" }}");
|
||||
assert!(sneaky.contains("u002fetc"));
|
||||
let args = parse(ToolName::ReadFile, &sneaky).unwrap();
|
||||
assert_eq!(
|
||||
args,
|
||||
ToolArgs::ReadFile {
|
||||
path: "/etc/hosts".to_string()
|
||||
}
|
||||
);
|
||||
assert_eq!(args.canonical_json(), r#"{"path":"/etc/hosts"}"#);
|
||||
|
||||
// Fields come out in the spec's order whatever order they came in.
|
||||
let write = parse(ToolName::WriteFile, r#"{"content":"x\ny","path":"/a/b"}"#).unwrap();
|
||||
assert_eq!(
|
||||
write.canonical_json(),
|
||||
r#"{"path":"/a/b","content":"x\ny"}"#
|
||||
);
|
||||
let shell = parse(ToolName::Shell, r#"{"cwd":"/a","command":"ls"}"#).unwrap();
|
||||
assert_eq!(shell.canonical_json(), r#"{"command":"ls","cwd":"/a"}"#);
|
||||
// An absent cwd is left out, and the host is never written: it is not an argument.
|
||||
let bare = parse(ToolName::Shell, r#"{"command":"ls","cwd":null}"#).unwrap();
|
||||
assert_eq!(bare.canonical_json(), r#"{"command":"ls"}"#);
|
||||
let fetch = parse(ToolName::HttpFetch, r#"{"url":"https://example.com/a"}"#).unwrap();
|
||||
assert_eq!(fetch.canonical_json(), r#"{"url":"https://example.com/a"}"#);
|
||||
assert_eq!(fetch.tool(), ToolName::HttpFetch);
|
||||
assert_eq!(write.tool(), ToolName::WriteFile);
|
||||
}
|
||||
Reference in New Issue
Block a user