fix(acl): comma-split the handler parser so two commands on one line both survive
Review round 1 (Minor promoted to required fix): the parser applied
rsplit("::").next() once per line, so `a::x, b::y,` on a single line
collapsed to one item and silently dropped a::x — a denied command at
runtime with nothing flagging it. Strip `//` comments per line first (a
whole-line comment strips to nothing, a trailing one leaves the code before
it), then split the cleaned text on `,` so every grant is its own item
regardless of how many share a line.
Adds two_commands_on_one_line_are_both_kept (the regression case) and
a_fixture_shaped_like_the_real_handler_list_parses_every_command (section
comments plus 1-, 2- and 3-segment paths, mirroring lib.rs's real shape).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -14,24 +14,30 @@ use std::collections::{BTreeMap, BTreeSet};
|
|||||||
/// duplicates kept (the caller decides whether that is an error). `None` if the block is
|
/// duplicates kept (the caller decides whether that is an error). `None` if the block is
|
||||||
/// missing or unterminated.
|
/// missing or unterminated.
|
||||||
///
|
///
|
||||||
/// Line-based, not `split(',')`: the list is grouped under `// Docker` style comments, and
|
/// Comma-split, not line-split: `// Docker` style comments are stripped from every line first
|
||||||
/// splitting on commas glues each comment to the command that follows it. A
|
/// (a whole-line comment strips to nothing; a trailing one leaves the code before it), and the
|
||||||
/// `starts_with("//")` filter then drops that command — silently, and once per group.
|
/// *cleaned* text is then split on `,` so each grant is its own item regardless of how many
|
||||||
|
/// share a line. A line-split version of this parser shipped first and used
|
||||||
|
/// `rsplit("::").next()` once *per line*: two commands on one line (`a::x, b::y,`) collapsed to
|
||||||
|
/// a single item, silently dropping `a::x` — a denied command at runtime with nothing flagging
|
||||||
|
/// it. Comma-splitting fixes that because it no longer assumes one item per line.
|
||||||
pub fn registered_commands(lib_rs: &str) -> Option<Vec<String>> {
|
pub fn registered_commands(lib_rs: &str) -> Option<Vec<String>> {
|
||||||
let (_, rest) = lib_rs.split_once("generate_handler![")?;
|
let (_, rest) = lib_rs.split_once("generate_handler![")?;
|
||||||
let (inside, _) = rest.split_once("])")?;
|
let (inside, _) = rest.split_once("])")?;
|
||||||
|
let cleaned: String = inside
|
||||||
|
.lines()
|
||||||
|
// Strip a trailing `//` comment (and a whole-line one, which strips to "").
|
||||||
|
.map(|l| l.split("//").next().unwrap_or(""))
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join("\n");
|
||||||
Some(
|
Some(
|
||||||
inside
|
cleaned
|
||||||
.lines()
|
.split(',')
|
||||||
.map(str::trim)
|
.map(str::trim)
|
||||||
.filter(|l| !l.is_empty() && !l.starts_with("//"))
|
.filter(|s| !s.is_empty())
|
||||||
.filter_map(|l| {
|
.filter_map(|s| {
|
||||||
// `a::b::name, // comment` → `name`
|
// `a::b::name` → `name`; a bare `name` (no `::`) is its own last segment.
|
||||||
let code = l.split("//").next().unwrap_or("").trim();
|
s.rsplit("::").next().map(|n| n.trim().to_string())
|
||||||
code.trim_end_matches(',')
|
|
||||||
.rsplit("::")
|
|
||||||
.next()
|
|
||||||
.map(|n| n.trim().to_string())
|
|
||||||
})
|
})
|
||||||
.filter(|n| !n.is_empty())
|
.filter(|n| !n.is_empty())
|
||||||
.collect(),
|
.collect(),
|
||||||
@@ -237,6 +243,47 @@ mod tests {
|
|||||||
assert_eq!(registered_commands("generate_handler![ a::b, "), None, "unterminated");
|
assert_eq!(registered_commands("generate_handler![ a::b, "), None, "unterminated");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The bug this regression-tests: a line-split parser applies `rsplit("::").next()` once
|
||||||
|
/// per *line*, so two commands sharing a line collapse into one item and the first is
|
||||||
|
/// silently dropped. Comma-splitting must keep both regardless of layout.
|
||||||
|
#[test]
|
||||||
|
fn two_commands_on_one_line_are_both_kept() {
|
||||||
|
let lib_rs = "generate_handler![\n a::x, b::y,\n])";
|
||||||
|
assert_eq!(registered_commands(lib_rs).unwrap(), cmds(&["x", "y"]));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Mirrors the real `lib.rs` handler list's shape: `// Section` comments between groups,
|
||||||
|
/// and command paths one (`open_url_external`), two (`url_open::open_url_external`) and
|
||||||
|
/// three (`commands::docker_commands::check_docker`) segments deep, all ending in a comma
|
||||||
|
/// except the last entry before `])`.
|
||||||
|
#[test]
|
||||||
|
fn a_fixture_shaped_like_the_real_handler_list_parses_every_command() {
|
||||||
|
let lib_rs = r#"
|
||||||
|
.invoke_handler(tauri::generate_handler![
|
||||||
|
// Docker
|
||||||
|
commands::docker_commands::check_docker,
|
||||||
|
commands::docker_commands::build_image,
|
||||||
|
// Opening a link in the host browser
|
||||||
|
url_open::open_url_external,
|
||||||
|
// Bare, module-less command
|
||||||
|
open_help,
|
||||||
|
// Terminal file viewer
|
||||||
|
commands::file_viewer_commands::viewer_read_file
|
||||||
|
])
|
||||||
|
.run(tauri::generate_context!())
|
||||||
|
"#;
|
||||||
|
assert_eq!(
|
||||||
|
registered_commands(lib_rs).unwrap(),
|
||||||
|
cmds(&[
|
||||||
|
"check_docker",
|
||||||
|
"build_image",
|
||||||
|
"open_url_external",
|
||||||
|
"open_help",
|
||||||
|
"viewer_read_file",
|
||||||
|
])
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn permission_identifiers_replace_only_underscores() {
|
fn permission_identifiers_replace_only_underscores() {
|
||||||
assert_eq!(allow_permission("check_docker"), "allow-check-docker");
|
assert_eq!(allow_permission("check_docker"), "allow-check-docker");
|
||||||
|
|||||||
Reference in New Issue
Block a user