From 37485a85df8af7931e116fca4f1052a45ce3f721 Mon Sep 17 00:00:00 2001
From: beardthelion <56458543+beardthelion@users.noreply.github.com>
Date: Sun, 21 Jun 2026 13:49:32 -0500
Subject: [PATCH] fix(node): reject malformed path globs with non-trailing or
 empty-segment wildcards (#74)

validate_path_glob checked `ends_with("/**")` without first stripping it, so any
glob that still ended in `/**` slipped past the wildcard guard even when it held a
`*` elsewhere in the prefix (e.g. `/a*b/**`, `/a/**/**`). The prefix is fed to
glob_prefix/glob_matches as a literal, so such a rule matched no real path and
silently granted nobody, the exact silent-misconfiguration this validator exists
to prevent.

Strip the one permitted trailing `/**` first, then reject any remaining `*`.

While here, close a related hole in the same validator: `glob_prefix` greedily
trims all trailing `**` and `/`, so `//**` collapsed to `/` (whole repo) and
bypassed the explicit `/**` guard, and `//secret/**` became a dead rule whose
prefix never matches a real single-slash path. Reject empty path segments so the
validator and glob_prefix agree on the canonical form.

Adds `/a*b/**`, `/a/**/**`, `/**/**`, `//**`, `//secret/**` to the rejection test.
---
 crates/gitlawb-node/src/api/visibility.rs | 37 ++++++++++++++++++++---
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/crates/gitlawb-node/src/api/visibility.rs b/crates/gitlawb-node/src/api/visibility.rs
index 6665b9e8..ba94fe9f 100644
--- a/crates/gitlawb-node/src/api/visibility.rs
+++ b/crates/gitlawb-node/src/api/visibility.rs
@@ -57,9 +57,23 @@ fn validate_path_glob(path_glob: &str) -> Result<()> {
             "path_glob must not end with '/'".into(),
         ));
     }
-    if path_glob.contains('*') && !path_glob.ends_with("/**") {
+    // Reject empty path segments. `glob_prefix` collapses "//**" to "/" (whole
+    // repo), so without this an operator could bypass the "/**" guard above and
+    // grant repo-wide read with "//**", or set a dead rule like "//secret/**"
+    // whose prefix never matches a real single-slash path and silently leaves
+    // the subtree readable.
+    if path_glob.contains("//") {
         return Err(AppError::BadRequest(
-            "the only supported wildcard is a trailing '/**'".into(),
+            "path_glob must not contain empty path segments".into(),
+        ));
+    }
+    // Strip the one permitted trailing "/**" before checking for stray
+    // wildcards, otherwise a "*" elsewhere in the prefix (e.g. "/a*b/**" or
+    // "/a/**/**") slips through because the string still ends with "/**".
+    let prefix = path_glob.strip_suffix("/**").unwrap_or(path_glob);
+    if prefix.contains('*') {
+        return Err(AppError::BadRequest(
+            "the only supported wildcard is a single trailing '/**'".into(),
         ));
     }
     Ok(())
@@ -240,9 +254,22 @@ mod tests {
 
     #[test]
     fn rejects_malformed_globs() {
-        // empty, no leading slash, whole-repo via "/**", trailing slash, and
-        // non-trailing wildcards are all rejected.
-        for g in ["", "secret/**", "/**", "/secret/", "/a*b", "/*/x"] {
+        // empty, no leading slash, whole-repo via "/**", trailing slash,
+        // non-trailing wildcards, double wildcards, and empty path segments
+        // (which would collapse to whole-repo or a dead rule) are all rejected.
+        for g in [
+            "",
+            "secret/**",
+            "/**",
+            "/**/**",
+            "/secret/",
+            "/a*b",
+            "/*/x",
+            "/a*b/**",
+            "/a/**/**",
+            "//**",
+            "//secret/**",
+        ] {
             assert!(validate_path_glob(g).is_err(), "{g} should be rejected");
         }
     }