Skip to content

feat(enrichment): add CODEOWNERS blast-radius analyzer - #1645

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
dale053:feat/enrichment-codeowners-blast-radius
Jun 28, 2026
Merged

feat(enrichment): add CODEOWNERS blast-radius analyzer#1645
JSONbored merged 1 commit into
JSONbored:mainfrom
dale053:feat/enrichment-codeowners-blast-radius

Conversation

@dale053

@dale053 dale053 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a CODEOWNERS + blast-radius analyzer to REES (#1515) that surfaces
changed files governed by a CODEOWNERS rule whose owner is not the PR author.

  • Fetches .github/CODEOWNERS (with fallbacks to CODEOWNERS and
    docs/CODEOWNERS) via the GitHub contents API using the installation token.
  • Parses CODEOWNERS using last-match-wins semantics; glob-to-regex conversion
    uses only [^/]*, .*, and [^/] — no catastrophic backtracking on
    attacker-controlled input.
  • Owner matching normalises the leading @ and is case-insensitive; Alice
    matches @alice.
  • Unowned files (no matching rule) are silently skipped; only files with a
    rule that excludes the author are reported.
  • Fail-safe throughout: any network error, non-ok response, or missing
    CODEOWNERS file returns [] without throwing. Caps at 20 findings.
  • Findings rendered in promptSection as a
    ### CODEOWNERS violations — N ownership domain(s) affected block with
    per-file owner lists and blast-radius count derived at render time.
  • SLUG_RE validates owner/repo segments before API URL interpolation,
    preventing path-traversal on attacker-controlled repoFullName.

Resolves #1515.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥97% coverage of the lines AND branches you changed (aim for 98%+ on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • test:workers — no Cloudflare Worker bindings touched; REES is a standalone Node.js service.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

No visible UI changes — REES is a backend enrichment service; output appears
only in the review engine's prompt section.

Notes

  • Glob matching handles ** (across separators), * (within a path
    segment), ? (single non-/ char), leading / (repo-root anchor),
    trailing / (any descendant), and interior / (implicit anchor) —
    matching GitHub's documented CODEOWNERS semantics.
  • Blast radius is the count of distinct ownership domains (unique owner
    handles/teams) across all flagged files; derived at render time from the
    full findings set so it requires no extra data in the finding struct.

@dale053
dale053 requested a review from JSONbored as a code owner June 28, 2026 02:29
@dosubot dosubot Bot added the size:L label Jun 28, 2026
@loopover-orb

loopover-orb Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review — safe to merge

4 files · 1 AI reviewers · no blockers · readiness 55/100 · CI green · clean

✅ Approved — safe to merge

Review summary
Well-structured new analyzer: type-safe, fail-safe network layer, SLUG_RE guards URL interpolation before encodeURIComponent, last-match-wins wired correctly at the query layer. Two correctness bugs undermine the CODEOWNERS semantic model — one silently drops ownership-clearing patterns, the other lets `**` match partial path-component suffixes — both generate false-positive violations on real repos.

Blockers

  • parseCodeowners (codeowners.ts:79) skips lines with zero owners instead of recording them as rules, breaking last-match-wins for explicit unowned patterns: for a CODEOWNERS file with `* @​team-a` then `*.log` (no owners), findOwners returns `["@​team-a"]` for `debug.log` instead of `[]`, producing a false-positive violation for a file that was intentionally made unowned.
  • patternToRegex (codeowners.ts:44-45) emits `.*` after consuming the `/` that follows `**`, dropping the required path-component boundary: `**/foo` compiles to `(^|/).*foo$` and incorrectly matches `src/barfoo`; the same flaw hits interior `**/` — `src/**/test.ts` compiles to `^src/.*test\.ts$` and matches `src/footest.ts`; the fix is to emit `(?:^|.+/)` (not `.*`) when `**` is followed by `/` and more pattern, so the next token must start at a component boundary.
Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ✅ Linked #1515
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Review load ❌ 8/20 Readiness component derived from cached public PR metadata and labels; size label size:L.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 40 open PR(s), 18 likely reviewable, 22 unlinked.
Contributor context ✅ Confirmed Gittensor contributor dale053; Gittensor profile; 36 PR(s), 21 issue(s).
Gate result ✅ Passing No configured blocker found.
Nits — 6 non-blocking
  • CodeownersFinding.owners carries the comment `// sorted owners` (types.ts:108) but parseCodeowners preserves source order and no sort is applied anywhere — fix the comment or sort before pushing the finding.
  • fetchCodeowners sends `Accept: "application/vnd.github.raw"` (codeowners.ts:107); GitHub REST now recommends the `+json` suffix (`application/vnd.github.raw+json`) — the legacy form still works but may emit deprecation headers.
  • authorMatchesOwner (codeowners.ts:93) normalises author to `@​author`, so email-format CODEOWNERS entries (`user@​example.com`) will never match a GitHub login; add a comment acknowledging this so future maintainers don't assume it works.
  • The MAX_FILES_REPORTED early-break (codeowners.ts:155) means blastRadius in render.ts silently undercounts ownership domains when there are >20 violations — worth a brief comment so the reader knows the count is capped.
  • Fix blocker chore(release): prepare public gittensory launch #2 in patternToRegex: when the `**` branch consumes a trailing `/`, emit `(?:^|.+/)` instead of `.*` — this forces the next literal to begin at a path-component boundary, making `**/foo` → `(?:^|.+/)foo$` (or `^foo$` when not anchored) so `src/barfoo` no longer matches.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.
Review context
Contributor next steps
  • Review top overlaps.
  • Add scope summary.
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

Well-structured new analyzer: type-safe, fail-safe network layer, SLUG_RE guards URL interpolation before encodeURIComponent, last-match-wins wired correctly at the query layer. Two correctness bugs undermine the CODEOWNERS semantic model — one silently drops ownership-clearing patterns, the other lets `**` match partial path-component suffixes — both generate false-positive violations on real repos.

Blockers

  • parseCodeowners (codeowners.ts:79) skips lines with zero owners instead of recording them as rules, breaking last-match-wins for explicit unowned patterns: for a CODEOWNERS file with `* @​team-a` then `*.log` (no owners), findOwners returns `["@​team-a"]` for `debug.log` instead of `[]`, producing a false-positive violation for a file that was intentionally made unowned.
  • patternToRegex (codeowners.ts:44-45) emits `.*` after consuming the `/` that follows `**`, dropping the required path-component boundary: `**/foo` compiles to `(^|/).*foo$` and incorrectly matches `src/barfoo`; the same flaw hits interior `**/` — `src/**/test.ts` compiles to `^src/.*test\.ts$` and matches `src/footest.ts`; the fix is to emit `(?:^|.+/)` (not `.*`) when `**` is followed by `/` and more pattern, so the next token must start at a component boundary.

Nits (5)

  • CodeownersFinding.owners carries the comment `// sorted owners` (types.ts:108) but parseCodeowners preserves source order and no sort is applied anywhere — fix the comment or sort before pushing the finding.
  • fetchCodeowners sends `Accept: "application/vnd.github.raw"` (codeowners.ts:107); GitHub REST now recommends the `+json` suffix (`application/vnd.github.raw+json`) — the legacy form still works but may emit deprecation headers.
  • authorMatchesOwner (codeowners.ts:93) normalises author to `@​author`, so email-format CODEOWNERS entries (`user@​example.com`) will never match a GitHub login; add a comment acknowledging this so future maintainers don't assume it works.
  • The MAX_FILES_REPORTED early-break (codeowners.ts:155) means blastRadius in render.ts silently undercounts ownership domains when there are >20 violations — worth a brief comment so the reader knows the count is capped.
  • Fix blocker chore(release): prepare public gittensory launch #2 in patternToRegex: when the `**` branch consumes a trailing `/`, emit `(?:^|.+/)` instead of `.*` — this forces the next literal to begin at a path-component boundary, making `**/foo` → `(?:^|.+/)foo$` (or `^foo$` when not anchored) so `src/barfoo` no longer matches.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. labels Jun 28, 2026
@JSONbored
JSONbored merged commit 2b612da into JSONbored:main Jun 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(enrichment): CODEOWNERS + blast-radius mapper

2 participants