From 92a2d9de4abe5942f107acab8bbc0640524af67e Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Tue, 22 Sep 2026 22:20:08 -0700
Subject: [PATCH] fix(acl): comma-split the handler parser so two commands on
 one line both survive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 app/src-tauri/src/command_census.rs | 73 ++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 13 deletions(-)

diff --git a/app/src-tauri/src/command_census.rs b/app/src-tauri/src/command_census.rs
index 955d59d..7eb56bc 100644
--- a/app/src-tauri/src/command_census.rs
+++ b/app/src-tauri/src/command_census.rs
@@ -14,24 +14,30 @@ use std::collections::{BTreeMap, BTreeSet};
 /// duplicates kept (the caller decides whether that is an error). `None` if the block is
 /// missing or unterminated.
 ///
-/// Line-based, not `split(',')`: the list is grouped under `// Docker` style comments, and
-/// splitting on commas glues each comment to the command that follows it. A
-/// `starts_with("//")` filter then drops that command — silently, and once per group.
+/// Comma-split, not line-split: `// Docker` style comments are stripped from every line first
+/// (a whole-line comment strips to nothing; a trailing one leaves the code before it), and the
+/// *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>> {
     let (_, rest) = lib_rs.split_once("generate_handler![")?;
     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(
-        inside
-            .lines()
+        cleaned
+            .split(',')
             .map(str::trim)
-            .filter(|l| !l.is_empty() && !l.starts_with("//"))
-            .filter_map(|l| {
-                // `a::b::name, // comment` → `name`
-                let code = l.split("//").next().unwrap_or("").trim();
-                code.trim_end_matches(',')
-                    .rsplit("::")
-                    .next()
-                    .map(|n| n.trim().to_string())
+            .filter(|s| !s.is_empty())
+            .filter_map(|s| {
+                // `a::b::name` → `name`; a bare `name` (no `::`) is its own last segment.
+                s.rsplit("::").next().map(|n| n.trim().to_string())
             })
             .filter(|n| !n.is_empty())
             .collect(),
@@ -237,6 +243,47 @@ mod tests {
         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]
     fn permission_identifiers_replace_only_underscores() {
         assert_eq!(allow_permission("check_docker"), "allow-check-docker");