Skip to content

Commit 3d880cd

Browse files
authored
fix(node): reject malformed path globs with non-trailing or empty-segment wildcards (#74) (#75)
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.
1 parent 0a15e76 commit 3d880cd

1 file changed

Lines changed: 32 additions & 5 deletions

File tree

‎crates/gitlawb-node/src/api/visibility.rs‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,23 @@ fn validate_path_glob(path_glob: &str) -> Result<()> {
5656
"path_glob must not end with '/'".into(),
5757
));
5858
}
59-
if path_glob.contains('*') && !path_glob.ends_with("/**") {
59+
// Reject empty path segments. `glob_prefix` collapses "//**" to "/" (whole
60+
// repo), so without this an operator could bypass the "/**" guard above and
61+
// grant repo-wide read with "//**", or set a dead rule like "//secret/**"
62+
// whose prefix never matches a real single-slash path and silently leaves
63+
// the subtree readable.
64+
if path_glob.contains("//") {
6065
return Err(AppError::BadRequest(
61-
"the only supported wildcard is a trailing '/**'".into(),
66+
"path_glob must not contain empty path segments".into(),
67+
));
68+
}
69+
// Strip the one permitted trailing "/**" before checking for stray
70+
// wildcards, otherwise a "*" elsewhere in the prefix (e.g. "/a*b/**" or
71+
// "/a/**/**") slips through because the string still ends with "/**".
72+
let prefix = path_glob.strip_suffix("/**").unwrap_or(path_glob);
73+
if prefix.contains('*') {
74+
return Err(AppError::BadRequest(
75+
"the only supported wildcard is a single trailing '/**'".into(),
6276
));
6377
}
6478
Ok(())
@@ -239,9 +253,22 @@ mod tests {
239253

240254
#[test]
241255
fn rejects_malformed_globs() {
242-
// empty, no leading slash, whole-repo via "/**", trailing slash, and
243-
// non-trailing wildcards are all rejected.
244-
for g in ["", "secret/**", "/**", "/secret/", "/a*b", "/*/x"] {
256+
// empty, no leading slash, whole-repo via "/**", trailing slash,
257+
// non-trailing wildcards, double wildcards, and empty path segments
258+
// (which would collapse to whole-repo or a dead rule) are all rejected.
259+
for g in [
260+
"",
261+
"secret/**",
262+
"/**",
263+
"/**/**",
264+
"/secret/",
265+
"/a*b",
266+
"/*/x",
267+
"/a*b/**",
268+
"/a/**/**",
269+
"//**",
270+
"//secret/**",
271+
] {
245272
assert!(validate_path_glob(g).is_err(), "{g} should be rejected");
246273
}
247274
}

0 commit comments

Comments
 (0)