fix(node): reject malformed path globs in visibility validation (#74)

[beardthelion]

Summary

validate_path_glob accepted malformed visibility globs that silently misconfigure access. It now rejects a wildcard anywhere but the single trailing /**, and rejects empty path segments.

Motivation & context

Closes #74

The wildcard guard checked path_glob.ends_with("/**") without first stripping it, so any glob that still ended in /** slipped through even with a * earlier in the prefix (e.g. /a*b/**, /a/**/**). glob_prefix/glob_matches treat that prefix as a literal, so the rule matched no real path and granted nobody, the exact silent misconfiguration this validator exists to prevent.

Reviewing the fix surfaced a second hole in the same function: 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. Both are fail-closed and owner-only (only the repo owner can set rules, and a bad rule grants nobody), so this is hardening, not a live exploit, but the //** case is a one-character bypass of the whole-repo guard and worth closing alongside.

Kind of change

• Bug fix
• Feature
• Security fix
• Docs
• Tests / CI
• Refactor (no behavior change)
• Breaking or protocol change (issue required first)

What changed

Crate: gitlawb-node

• Strip the one permitted trailing /** before checking for stray *, so a non-trailing wildcard is rejected instead of slipping through.
• Reject path globs containing // (empty path segments), keeping the validator and glob_prefix agreed on the canonical form and closing the //** whole-repo bypass.
• Added /a*b/**, /a/**/**, /**/**, //**, //secret/** to the rejects_malformed_globs test.

How a reviewer can verify

cargo test -p gitlawb-node glob

Both accepts_supported_globs (/, /secret, /secret/**, /a/b/c, /a/b/** still pass) and rejects_malformed_globs (the five added cases now rejected) cover the change.

Before you request review

• Scope is one logical change; no unrelated churn
• cargo test --workspace passes locally
• New behavior is covered by tests (required for fixes)
• cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings are clean
• Commit titles use Conventional Commits (feat(...), fix(...), docs(...))
• Docs / .env.example updated if behavior or config changed (or N/A)
• Checked existing PRs so this isn't a duplicate

Notes for reviewers

remove_visibility still does not run validate_path_glob, but deleting a never-matching rule is a no-op, so that asymmetry is harmless and left out of scope.

Summary by CodeRabbit

• Bug Fixes
  • Path pattern validation now strictly rejects malformed patterns containing consecutive slashes, preventing invalid inputs from bypassing existing validation rules and security checks. This improves error handling, protects against problematic patterns, and ensures more robust validation across the system. The change maintains full backward compatibility with all valid glob patterns currently in use.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d31fe471-0bb6-408e-a211-a46ab61f8627

📥 Commits

Reviewing files that changed from the base of the PR and between e37ea7f and 37485a8.

📒 Files selected for processing (1)

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

---

📝 Walkthrough

Walkthrough

validate_path_glob gains a new early-exit guard that returns BadRequest for any path glob containing "//". The rejects_malformed_globs test is expanded to cover /**/**, /a*b/**, /a/**/**, //**, and //secret/** as invalid inputs.

Changes

Path Glob Validation Hardening

Layer / File(s)	Summary
Double-slash guard and expanded malformed-glob tests crates/gitlawb-node/src/api/visibility.rs	validate_path_glob rejects any glob containing "//" via a new early-return before the existing wildcard checks. rejects_malformed_globs adds /**/**, /a*b/**, /a/**/**, //**, and //secret/** to its invalid-input set.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A double slash sneaks through the gate?
Not on my watch — I'll validate!
"//" in your glob? That's a BadRequest today,
And new tests stand guard to keep malformed paths at bay.
Hop hop, no wildcards shall silently stray! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: rejecting malformed path globs in the visibility validation module.
Description check	✅ Passed	The PR description fully follows the template with all required sections completed: summary, motivation, kind of change, what changed, verification instructions, pre-review checklist, and notes.
Linked Issues check	✅ Passed	The PR fully addresses issue #74 by implementing both core fixes: stripping trailing '/**' before wildcard checks and rejecting double-slash patterns, with comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are scoped to validating malformed path globs in visibility.rs; no unrelated churn or out-of-scope modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/validate-path-glob-nontrailing-wildcard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[kevincodex1]

LGTM