Skip to content

Commit fbcf3d0

Browse files
committed
fix(groom): enforce path scope after the verifier + close four scoping holes (BE-4757)
Cursor-review panel follow-ups on the `path` input, in severity order. - The scope filter ran only on the FINDER's output, but a `DOWNGRADE` verdict explicitly reshapes a finding — so a cross-boundary candidate that legitimately passed that filter could be narrowed onto its OUT-of-scope half and filed under a directory it no longer belongs to. `verifier.md` now emits `sites`, and `scope.py verify` re-applies the same ANY-in-scope rule to what the verifier confirmed. A finding with no LOCATABLE site is KEPT there (the opposite of the finder-side rule): `sites` is advisory on that schema, so dropping on it would discard every survivor and read as an honest "nothing survived verification". - `canonicalize_signature` forces the dedup key's scope component back to the literal the brief handed out, so signature scope-independence no longer rests on an untrusted model obeying `{{SIG_SCOPE}}`. Left alone when `scope_label` itself contains a colon — component boundaries are ambiguous there, and corrupting a working dedup key is worse than the double-filing it guards. - `normalize_site` no longer relativizes an absolute path that is NOT under the clone; it returns "" (unlocatable). Stripping the leading slash silently reinterpreted `/services/api/x.go` as repo-relative, so an out-of-tree site could satisfy a matching scope. - `resolve_within` rejects a SYMLINKED scope even when it points inside the repo. Verified against git: `git ls-files -- <link>` returns the link entry alone, so groom.yml's non-empty guard passed while the finder audited nothing and reported clean. The error names the fix (pass the real directory). - `list_files` reads bytes and decodes with surrogateescape: one non-UTF-8 tracked filename previously raised UnicodeDecodeError and killed the scoped audit before the finder ran. `printable_path` drops surrogates too, and the omitted count is now WARNED rather than silently shortening the list. - `run_audited` compares the `(scoped: <path>)` marker case-SENSITIVELY, so `services/api` and `services/API` keep separate cadence clocks instead of one silently suppressing the other's due tick. - Every caller-controlled value reaches argparse as `--flag=value`: a directory named `-foo` is valid per the charset and the bare form reads it as an option.
1 parent f297fcc commit fbcf3d0

7 files changed

Lines changed: 572 additions & 30 deletions

File tree

.github/groom/README.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,24 @@ rather than buried in a runner script.
3939
| Phase | Brief | Input | Output (JSON) |
4040
|---|---|---|---|
4141
| 1. Find | [`finder.md`](finder.md) | clean `origin/main` checkout + scan scope | `{repo, scope, findings:[{title, dimension, sites, evidence, proposed, value, risk, confidence, steelman}]}` at `{{FINDER_OUT}}` |
42-
| 2. Verify | [`verifier.md`](verifier.md) | the finder's JSON + the code | `{repo, scope, summary, findings:[{title, verdict, security, signature, body}]}` at `{{VERIFIER_OUT}}` |
42+
| 2. Verify | [`verifier.md`](verifier.md) | the finder's JSON + the code | `{repo, scope, summary, findings:[{title, verdict, security, sites, signature, body}]}` at `{{VERIFIER_OUT}}` |
4343
| 3. Build (opt-in) | [`builder.md`](builder.md) | ONE verified finding `{title, body, signature}` at `{{FINDING_IN}}` + the code | edits in the checkout + a control file `{status: patched\|bail, summary}` at `{{BUILDER_OUT}}` |
4444

4545
- **`verdict`** is `CONFIRM` \| `DOWNGRADE` (real but narrow the scope) \|
4646
`REJECT` (premature / overstated / not worth it).
4747
- **`security: true`** marks any auth/permission/security-adjacent finding —
4848
those are filed as investigations, **never** auto-implemented.
49+
- **`sites`** is the `file:line` evidence the verdict actually rests on — the
50+
NARROWED set on a `DOWNGRADE`. On a path-scoped run `scope.py verify` re-applies
51+
the directory filter to it, because a downgrade may narrow a cross-boundary
52+
finding onto its out-of-scope half.
4953
- **`signature`** is a stable dedup key (`<repo-basename>:<scope>:<slug>`) whose
5054
`<slug>` is derived **deterministically** from the finding's core subject, so it
5155
stays identical across re-runs of the same finding and a consumer never re-files
52-
a finding it has already seen.
56+
a finding it has already seen. The `<scope>` component is the caller's own
57+
`scope_label`, never the audited directory — and `scope.py verify` rewrites it
58+
back to that value, so scope-independence does not depend on the model obeying
59+
the brief.
5360

5461
## How a consumer uses these briefs
5562

.github/groom/interval.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,25 @@ def run_audited(jobs, scope_path: str = "") -> bool:
243243
marker and so cannot be ATTRIBUTED to a scope at all. Both fall through to
244244
"no prior run", which fails open.
245245
"""
246-
want = scoped_job_marker(scope_path).lower() if scope_path else ""
246+
want = scoped_job_marker(scope_path) if scope_path else ""
247247
for job in jobs or []:
248-
name = (job.get("name") or "").lower()
248+
raw_name = job.get("name") or ""
249+
name = raw_name.lower()
249250
if not any(hint in name for hint in _FINDER_JOB_HINTS):
250251
continue
251252
if job.get("conclusion") not in _AUDITED_CONCLUSIONS:
252253
continue
253254
if want:
254-
if want in name:
255+
# The MARKER is compared case-SENSITIVELY (against the raw job name)
256+
# while the surrounding prose hints stay case-insensitive. Paths on
257+
# the Linux runner are case-sensitive and `_COMPONENT_RE` admits both
258+
# cases, so `services/api` and `services/API` are two distinct scopes
259+
# with two distinct clocks; folding case would collapse them onto one
260+
# and let a run of either silently suppress the other's due tick —
261+
# the silent under-run this module refuses. A case MISMATCH now reads
262+
# as "no prior run of this scope", i.e. fail-open, the same collision
263+
# direction as every other branch here.
264+
if want in raw_name:
255265
return True
256266
elif _SCOPED_MARKER_PREFIX not in name and any(h in name for h in _DISPLAY_NAME_HINTS):
257267
return True

0 commit comments

Comments
 (0)