Skip to content

fix(api): allow authenticated sessions to reach registration-readiness / gittensor-config-recommendation (#8654) - #8706

Merged
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-session-allowlist-readiness-8654
Jul 26, 2026
Merged

fix(api): allow authenticated sessions to reach registration-readiness / gittensor-config-recommendation (#8654)#8706
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
RealDiligent:fix/critical-issue-session-allowlist-readiness-8654

Conversation

@RealDiligent

Copy link
Copy Markdown
Contributor

Problem

Closes #8654.

src/api/routes.ts's registration-readiness and gittensor-config-recommendation route handlers have no internal auth/role check — they are advisory, any-authenticated-user lookups. The owner panel calls both via useApiResource (always credentials: "include") and takes a free-text repo name rather than scoping to the caller's own repos.

Neither path had a predicate in canSessionAccessPath, so a real non-operator browser session got 403 on the owner panel's only two data calls, while operators / server tokens (which bypass the allowlist) worked.

Fix — unblock only, no new restriction

Added the two missing path predicates and their allowlist entries in canSessionAccessPath, so any authenticated session may reach both routes.

This unblocks the existing intended open access; it adds no authorization restriction. Evidence the open access is intentional:

  • The handlers carry no per-repo ownership check at all.
  • buildRegistrationReadinessResponse deliberately strips owner-private context via stripOwnerPolicyContext before returning — the payload is designed for public/any-authenticated reads.
  • The owner panel intentionally accepts an arbitrary free-text repo name.

Tests

A new test in test/unit/access-boundary.test.ts asserts a logged-in user who maintains no repo here now gets 200 (previously 403) on both routes for an arbitrary repo. Reverting the allowlist entries makes it fail. git diff --check clean.

(Supersedes #8703, which the bot auto-closed under the 2-open-PR limit; re-filed now that a slot is free.)

…cal path-matchers

src/review/merge-train.ts hand-rolled its own low-signal file/dir detection that had drifted from the
canonical set its sibling files already use:

- LOW_SIGNAL_FILENAME_RE matched only package-lock.json/yarn.lock/pnpm-lock.yaml/Cargo.lock (4 names),
  so two PRs sharing only a poetry.lock, go.sum, npm-shrinkwrap.json, etc. were treated as a real
  overlap and forced into a spurious merge-train wait.
- LOW_SIGNAL_DIR_RE matched only dist/build/coverage/node_modules, missing 'out' and the vendored-code
  family (vendor/vendored/third_party/bower_components/jspm_packages).

Replace the filename regex with the canonical isLockfile() helper from path-matchers.ts (the same
dependency-free leaf utility review-diff.ts and review-grounding.ts already delegate to, covering all
24+ lockfile formats), and extend the directory regex to the canonical generated/vendored set. Two
new tests prove PRs sharing only poetry.lock/go.sum or vendor/third_party paths now decide {wait:false}
where they previously decided {wait:true}.
@RealDiligent
RealDiligent requested a review from JSONbored as a code owner July 25, 2026 23:55
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

…s and gittensor-config-recommendation

src/api/routes.ts's registration-readiness and gittensor-config-recommendation route handlers carry
no per-repo ownership guard by design -- they are advisory, any-authenticated-user lookups (the owner
panel takes a free-text repo name, and buildRegistrationReadinessResponse already strips owner-private
context via stripOwnerPolicyContext before returning). But neither path had a predicate in
canSessionAccessPath, so a real non-operator browser session got 403 on the owner panel's only two
data calls, while operators/server tokens (which bypass the allowlist) worked.

Add the two missing path predicates to the session allowlist so any authenticated session may reach
them, matching the routes' existing intended open access. This unblocks the existing behavior only --
it adds no new authorization restriction (none was found to be intended: the handlers have no
per-repo check, and the readiness payload is deliberately owner-context-stripped for public reads).

Test: a session-cookie-authenticated user who maintains no repo here now gets 200 (not 403) on both
routes for an arbitrary repo, in test/unit/access-boundary.test.ts.
@loopover-orb loopover-orb Bot added gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier. labels Jul 26, 2026
@loopover-orb

loopover-orb Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-26 00:07:12 UTC

4 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds two missing path predicates (registration-readiness, gittensor-config-recommendation) to canSessionAccessPath so authenticated non-operator sessions can reach the owner panel's two advisory, per-repo-scope-free data endpoints, matching the existing pattern used by sibling routes in this same function. It also (in a separately-scoped hunk) swaps merge-train.ts's hand-rolled 4-name lockfile regex for the canonical isLockfile helper and broadens the low-signal directory set, with new tests for both changes and the access-boundary fix. The two changes are unrelated to each other (auth allowlist fix vs. merge-train overlap classification) but both are narrow, well-tested, and each traces to a linked issue (#8654, #8647).

Nits — 4 non-blocking

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #8654
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 327 registered-repo PR(s), 134 merged, 37 issue(s).
Contributor context ✅ Confirmed Gittensor contributor RealDiligent; Gittensor profile; 327 PR(s), 37 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds the two missing predicates to canSessionAccessPath, explains explicitly (with evidence) that this unblocks existing intended open access rather than adding a restriction, and includes the required session-cookie-based test in access-boundary.test.ts asserting 200 for a non-maintaining user on both routes.

Review context
  • Author: RealDiligent
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, Ruby, TypeScript, Svelte, Cuda, JavaScript, Markdown, MDX
  • Official Gittensor activity: 327 PR(s), 37 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 9e6e57d into JSONbored:main Jul 26, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. gittensor:priority Maintainer-selected Gittensor priority — scores a 1.5x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(orb): registration-readiness and gittensor-config-recommendation routes are unreachable from a real browser session

1 participant