Skip to content

validate_path_glob accepts malformed globs with non-trailing wildcards #74

Description

@beardthelion

Summary

validate_path_glob in crates/gitlawb-node/src/api/visibility.rs is meant to reject malformed globs before they reach the store, but its wildcard check lets through any pattern that contains a * outside the trailing /**, as long as it still ends with /**. Values like /a*b/** and /a/**/** pass validation when they should be rejected.

The bug

if path_glob.contains('*') && !path_glob.ends_with("/**") {
    return Err(AppError::BadRequest(
        "the only supported wildcard is a trailing '/**'".into(),
    ));
}

For /a*b/**, contains('*') is true but ends_with("/**") is also true, so the guard is false and the value is accepted. The check only catches a non-trailing * when the string does not also end in /**.

Impact

This is a correctness / silent-misconfiguration bug, not an access-escalation one. Tracing the consumer:

  • glob_prefix("/a*b/**") strips the trailing ** and /, yielding the prefix /a*b (the * is left in place as a literal character).
  • glob_matches then compares literally: path == "/a*b" or path.starts_with("/a*b/").

So a malformed rule matches no real repo path and silently grants nobody. A non-trailing * only ever makes the prefix more specific, never broader, so there is no over-grant or privilege escalation; the rule just quietly does nothing. That is exactly the failure mode validate_path_glob's own doc comment says it exists to prevent ("malformed globs ... would silently misconfigure access"). An operator who typos /a*b/** instead of /ab/** gets a visibility rule that appears accepted but never matches.

Fix

Strip the trailing /** first, then reject if a * remains in the prefix:

if let Some(prefix) = path_glob.strip_suffix("/**") {
    if prefix.contains('*') {
        return Err(AppError::BadRequest(
            "the only supported wildcard is a single trailing '/**'".into(),
        ));
    }
} else if path_glob.contains('*') {
    return Err(AppError::BadRequest(
        "the only supported wildcard is a single trailing '/**'".into(),
    ));
}

Add /a*b/** and /a/**/** to the rejects_malformed_globs test.

Notes

Pre-existing on main; surfaced during review of #68 but not introduced by it (the file is byte-identical there). Filing separately to keep #68 scoped.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcrate:nodegitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorsev:mediumDegraded but workaround existssubsystem:visibilityPath-scoped visibility and content withholding

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions