Skip to content

feat(orb): gate APR repo-transfer requests on idea completion - #8000

Closed
jsdevninja wants to merge 1 commit into
JSONbored:mainfrom
jsdevninja:feat/apr-request-transfer
Closed

feat(orb): gate APR repo-transfer requests on idea completion#8000
jsdevninja wants to merge 1 commit into
JSONbored:mainfrom
jsdevninja:feat/apr-request-transfer

Conversation

@jsdevninja

Copy link
Copy Markdown
Contributor

Closes #7742

Summary

Test plan

  • Unit: eligibility true/false; request rejects without calling initiate when incomplete; initiates on success; surfaces structured failure; default initiate hook path; response.text() rejection fallback
  • Route: 202 / 409 / 502 status mapping; 400 on invalid/malformed body with no GitHub call; no wallet/hotkey/trust-score leakage
  • npm run typecheck
  • Focused vitest: orb-apr-repo-transfer + routes-request-apr-transfer (17 passing)

Adds the customer-facing request-only path for APR transfers (JSONbored#7742):
evaluateAprRepoTransferRequestEligibility + requestAprRepoTransfer wrap
initiateAprRepoTransfer so GitHub is only contacted when the JSONbored#7591
completion signal is true, and expose POST /v1/loop/request-apr-transfer.
No automatic or proactive offer.
@jsdevninja
jsdevninja requested a review from JSONbored as a code owner July 22, 2026 05:40

@superagent-security superagent-security 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.

Superagent found 1 security concern(s).

Comment thread src/api/routes.ts
const body = await c.req.json().catch(() => null);
const parsed = requestAprTransferSchema.safeParse(body);
if (!parsed.success) return c.json({ error: "invalid_request_apr_transfer_request", issues: parsed.error.issues }, 400);
const result = await requestAprRepoTransfer(c.env, parsed.data);

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.

P1: ideaComplete security gate is trivially bypassable because it is caller-supplied

The ideaComplete boolean comes from the request body, so any caller can set it to true and bypass the completion gate.

Look up idea completion from a server-side persisted source instead of trusting caller input.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/api/routes.ts">
<violation number="1" location="src/api/routes.ts:3749">
<priority>P1</priority>
<title>`ideaComplete` security gate is trivially bypassable because it is caller-supplied</title>
<evidence>The `ideaComplete` boolean is accepted directly from the request body (`ideaComplete: z.boolean()`) and passed to `requestAprRepoTransfer`, which uses it as the sole input to `evaluateAprRepoTransferRequestEligibility`. Since the caller controls this value, they can set `ideaComplete: true` to bypass the completion gate entirely, defeating the #7742 policy that repo transfers should only be available after idea completion.</evidence>
<recommendation>Look up the idea completion status from a trusted server-side source (#7591) instead of trusting the caller. If a persisted lookup is not yet available, reject all transfer requests or verify completion via an internal service call that the caller cannot influence. The route should not accept `ideaComplete` from the request body.</recommendation>
</violation>
</file>

@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jul 22, 2026
@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 22, 2026
@loopover-orb

loopover-orb Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-22 05:50:38 UTC

4 files · 1 AI reviewer · no blockers · CI failing · blocked

🛑 Suggested Action - Fix Blockers

Review summary
The AI review returned non-blocking notes for this change but did not include a separate narrative summary. Review the nits below before deciding this PR.

Nits — 5 non-blocking
  • src/orb/apr-repo-transfer.ts:43 / src/api/routes.ts:3741 — `ideaComplete` is entirely caller-supplied with no server-side lookup of the actual Design spec: idea-completion determination + customer-facing status for APR (no GitHub UI exposure) #7591 completion signal; confirm the private-API auth on this route means only a trusted internal caller (which itself checks the real signal) can hit it, otherwise any bearer-token holder can pass `ideaComplete: true` and bypass the policy gate the PR is meant to enforce.
  • External brief flags magic numbers (200/400/409/502/202 status codes) at src/api/routes.ts — these match the existing status-code style used throughout the file for other routes, so no named constants are needed here.
  • src/api/routes.ts:3742 — the error detail from GitHub (`transfer.error`) is echoed straight into the 502 JSON body; worth double-checking it can't ever surface GitHub-internal wording you don't want customer-facing (low risk given it's truncated to 200 chars in apr-repo-transfer.ts).
  • If `ideaComplete` is meant to eventually come from a persisted Design spec: idea-completion determination + customer-facing status for APR (no GitHub UI exposure) #7591 lookup rather than the request body, consider tracking that as a fast follow-up issue so the gate doesn't silently stay client-trusted long-term.
  • Consider adding a short note to the route handler doc comment (src/api/routes.ts:3738) clarifying which internal caller is expected to supply `ideaComplete`, mirroring the clarity already in the module doc comment.

CI checks failing

  • Superagent Security Scan — PR requires security review

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 #7742
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: 210 registered-repo PR(s), 132 merged, 37 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jsdevninja; Gittensor profile; 210 PR(s), 37 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR adds a gated, customer-request-only `requestAprRepoTransfer`/`evaluateAprRepoTransferRequestEligibility` pair and a REST route that only calls the existing `initiateAprRepoTransfer` when `ideaComplete` is true, with no automatic/proactive path, matching the ratified policy's two deliverables.

Review context
  • Author: jsdevninja
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: JavaScript, Swift, C, CSS, MDX, Python, TypeScript, Vue
  • Official Gittensor activity: 210 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 &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; 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 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (Superagent Security Scan)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.79%. Comparing base (24ddd19) to head (19b18eb).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8000      +/-   ##
==========================================
- Coverage   91.92%   89.79%   -2.14%     
==========================================
  Files         740       96     -644     
  Lines       75933    24341   -51592     
  Branches    23040     4578   -18462     
==========================================
- Hits        69803    21857   -47946     
+ Misses       5037     2214    -2823     
+ Partials     1093      270     -823     
Flag Coverage Δ
shard-1 28.10% <100.00%> (-30.32%) ⬇️
shard-2 44.31% <11.11%> (-8.19%) ⬇️
shard-3 72.09% <11.11%> (+22.75%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/api/routes.ts 95.18% <100.00%> (+0.01%) ⬆️
src/orb/apr-repo-transfer.ts 100.00% <100.00%> (ø)

... and 644 files with indirect coverage changes

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. pr:flagged PR flagged for review by security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decide policy for when an APR repo-transfer offer is surfaced to a customer (deferred from #7638)

1 participant