fix(api): allow authenticated sessions to reach registration-readiness / gittensor-config-recommendation (#8654) - #8703
Closed
RealDiligent wants to merge 2 commits into
Conversation
…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}.
Contributor
|
LoopOver closed this because @RealDiligent has 3 open pull requests, above this repository's configured limit of 2. Close or merge an existing one to open a new one. This is an automated maintenance action. |
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8703 +/- ##
==========================================
- Coverage 93.76% 91.02% -2.75%
==========================================
Files 797 98 -699
Lines 79454 24920 -54534
Branches 24070 4666 -19404
==========================================
- Hits 74504 22684 -51820
+ Misses 3565 1971 -1594
+ Partials 1385 265 -1120
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Closes #8654.
src/api/routes.ts'sregistration-readinessandgittensor-config-recommendationroute handlers have no internal auth/role check — they are advisory, any-authenticated-user lookups. The owner panel (owner-panel.tsx) calls both viauseApiResource(which always sendscredentials: "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-to-server tokens (which bypass the allowlist) worked.Fix — unblock only, no new restriction
Added the two missing path predicates (
isRepoRegistrationReadinessPath,isRepoGittensorConfigRecommendationPath) and their allowlist entries incanSessionAccessPath, 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 (not a forgotten guard):
buildRegistrationReadinessResponsedeliberately strips owner-private context viastripOwnerPolicyContextbefore returning — i.e. the payload is designed for public/any-authenticated reads.Tests
A new test in
test/unit/access-boundary.test.ts(using its existing session-cookie template) 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 --checkclean.