Fix wildcard detection (#11), dead repo URLs, and duplicate CI steps - #29
Fix wildcard detection (#11), dead repo URLs, and duplicate CI steps#29Dishanth234 wants to merge 1 commit into
Conversation
|
Hi @hermanerr, @chriskrycho first-time contributor here. This bundles the |
imran-siddique
left a comment
There was a problem hiding this comment.
Thanks for picking up the URL fixes from #10 and for the credit. Reviewing the whole thing rather than just my half, and I ran it instead of reading it.
The detector gap reproduces on main. At 2b828199627e0d6f31a9ab5f1914bbf3141273c8, apply_detectors returns zero matches for all of these:
| rule | main | this PR |
|---|---|---|
Bash(*) |
no matches | OVERBROAD |
Skill(*) |
no matches | OVERBROAD |
* |
no matches | OVERBROAD |
Bash(nc:*) |
no matches | OVERBROAD |
Bash(sudo cp:*) |
no matches | OVERBROAD |
Bash(git reset:*) |
no matches | DESTRUCTIVE |
Bash(rm -rf *) matches on both, so the detector was working generally and failing on exactly the two shapes #11 named. The colon and space forms now classify identically, which is the part that matters: cmd:* is what "always allow" writes, so the colon form was the common case in a real config and it was the one silently passing. Suite on the PR head is 89 passed, 1 skipped.
The URL half is complete against current main. Six live VantaInc/grantguard references remain: CONTRIBUTING.md (1), CHANGELOG.md (2), .github/ISSUE_TEMPLATE/config.yml (1), grantguard/web/app.js (2). Those are exactly the four files here. README.md carries none any more, so dropping it from #10's file list is correct rather than a miss.
One thing that is not visible from the checks. This PR reports three checks, the two Socket ones and semgrep. A merged PR in this repo gets thirteen, including ubuntu/macos/windows on py3.10 and py3.14 and three CodeQL analyses. The CI and CodeQL runs on fix/wildcards-urls-ci are both sitting at action_required (run ids 33413300045 and 33413300037), so the repo's own CI has never executed the new detector tests, and my 89-passed number is from my machine on Python 3.12, not from here. That needs a maintainer to release before the green reads as green.
No blocking findings from me on the diff.
Summary
Detectors: colon-syntax prefix wildcards (
Bash(cmd:*)— the form ClaudeCode writes for "always allow") now classify the same as the equivalent space
form (
Bash(cmd *)), and bare tool-wide wildcards such asBash(*)areflagged OVERBROAD instead of safe.
Bash(*)reuses the existing OVERBROADcategory rather than adding a new severity tier — happy to adjust if you'd
prefer it ranked higher. Two detector tests added.
Docs/UI: dead
VantaInc/grantguardlinks (404 since the org rename) nowpoint to
OpenVanta/GrantGuard— the CONTRIBUTING clone command, thechangelog link references, the issue chooser's security-advisory link, and
both spots in the web UI share modal. Also points the oxfmt link at its live
docs page. This revives the approved-but-unmerged #10 (credit to
@imran-siddique).
CI: removes the duplicated
setup-pythonv6.3.0 /checkoutv7.0.0steps left behind by a merge conflict resolution, so each job checks out and
sets up Python exactly once. Also obsoletes the dependabot re-bump in #28.
Verified: 90/90 tests on Python 3.10 and 3.12,
compileallclean,pnpm fmt:checkpasses, no newpnpm checkerrors, every replacement URLreturns 200.
Related issue
Closes #11
Type of change
Checklist
uv run python -m compileall -q grantguard grantguard.pypassesredact()(n/a — no new secret patterns; the added detectors are non-redacting)