Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ import {
type LoopOverMentionCommandName,
} from "../github/commands";
import { handleGitHubWebhook, handleOrbRelay } from "../github/webhook";
import { requestAprRepoTransfer } from "../orb/apr-repo-transfer";
import { handleOrbIngest, readOrbIngestBody } from "../orb/ingest";
import { handleAmsIngest } from "../ams/ingest";
import { handleOrbWebhook } from "../orb/webhook";
Expand Down Expand Up @@ -532,6 +533,16 @@ const evaluateEscalationSchema = z.object({
killRequested: z.boolean().optional(),
});

// #7742: customer-facing APR transfer request. `ideaComplete` is the #7591 completion signal (caller-supplied
// until that signal has a persisted lookup); plan/payment fields are deliberately absent — the gate is
// completion-only. Repo identity bounds match other /v1/loop POST bodies (non-empty strings, positive install id).
const requestAprTransferSchema = z.object({
installationId: z.number().int().positive(),
repoFullName: z.string().min(1).max(200),
newOwner: z.string().min(1).max(100),
ideaComplete: z.boolean(),
});

// #6744: mirrors proposeActionShape in src/mcp/server.ts VERBATIM, minus owner/repo (they are path params), so
// POST /v1/repos/:owner/:repo/agent/pending-actions can never stage an action the loopover_propose_action MCP
// tool would reject, or vice versa. actionClass stays the 7-value propose set (a subset of AgentActionClass).
Expand Down Expand Up @@ -3727,6 +3738,20 @@ export function createApp() {
return c.json(evaluateEscalation(parsed.data));
});

// #7742: customer-facing "request transfer" for an APR repo. Request-only (nothing auto-offers); gated on
// the idea-completion signal (#7591) via requestAprRepoTransfer, which calls initiateAprRepoTransfer only
// when ideaComplete is true. A rejected gate returns 409 without touching GitHub; a successful initiation
// is still pending-acceptance (202), never "transfer done".
app.post("/v1/loop/request-apr-transfer", async (c) => {
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>

if (result.status === "rejected") return c.json(result, 409);
if (result.status === "failed") return c.json(result, 502);
return c.json(result, 202);
});

// #6752: REST mirror of the loopover_build_results_payload MCP tool, bringing it to the same REST/CLI parity
// its same-tier sibling loopover_check_slop_risk (/v1/lint/slop-risk) already has. Both are pure, source-free
// composers over caller-supplied, already-computed iteration metadata, so this route delegates to the same
Expand Down
72 changes: 69 additions & 3 deletions src/orb/apr-repo-transfer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// under a loopover-controlled GitHub org (#7637) and can later be transferred, on explicit customer request, to
// the customer's own account via GitHub's standard repository-transfer flow.
//
// This module owns ONLY the initiation call. Detecting when a pending transfer is accepted or expires, any
// customer-facing UI, and the policy of *when* a transfer should be offered are deliberately out of scope
// (separate follow-ons per #7638). No provisioning or repo-creation logic lives here.
// Initiation (#7638) lives here; the customer-facing request gate (#7742) does too. Detecting when a pending
// transfer is accepted or expires (#7741) remains out of scope. No provisioning or repo-creation logic lives
// here. Transfer is NEVER offered or nudged proactively in v1 — request-only, and only once the idea's
// completion signal (#7591) is true.

import { createInstallationToken } from "../github/app";
import { githubHeaders, timeoutFetch } from "../github/client";
Expand All @@ -22,6 +23,42 @@ export type AprRepoTransferResult =
| { initiated: true; status: number; newFullName: string | null }
| { initiated: false; status: number; error: string };

/**
* #7742 policy gate: a transfer may be requested only after the idea's completion signal (#7591) is true.
* Plan/payment tiers are deliberately NOT consulted — this stays clear of the billing track. Pure: no IO.
*/
export type AprRepoTransferRequestEligibility =
| { allowed: true }
| { allowed: false; reason: "idea_not_complete" };

export type RequestAprRepoTransferInput = {
installationId: number;
repoFullName: string;
newOwner: string;
/** The idea-completion signal from #7591 — false until that signal says the task-graph is done. */
ideaComplete: boolean;
};

/**
* Outcome of a customer-initiated transfer request (#7742).
*
* - `rejected` — the completion gate blocked the call; GitHub was never contacted.
* - `initiated` / `failed` — the gate passed and {@link initiateAprRepoTransfer} ran; `initiated` still means
* GitHub accepted a *pending* transfer (see {@link AprRepoTransferResult}), never "transfer done".
*/
export type RequestAprRepoTransferResult =
| { status: "rejected"; reason: "idea_not_complete" }
| { status: "initiated"; transfer: Extract<AprRepoTransferResult, { initiated: true }> }
| { status: "failed"; transfer: Extract<AprRepoTransferResult, { initiated: false }> };

/** Decide whether a customer may request an APR repo transfer right now (#7742). Pure and deterministic. */
export function evaluateAprRepoTransferRequestEligibility(input: {
ideaComplete: boolean;
}): AprRepoTransferRequestEligibility {
if (input.ideaComplete !== true) return { allowed: false, reason: "idea_not_complete" };
return { allowed: true };
}

/**
* Initiate a transfer of `repoFullName` (a loopover-org APR repo, `owner/name`) to the GitHub account `newOwner`,
* using the App installation token — the same token source as APR repo creation (#7637).
Expand All @@ -30,6 +67,9 @@ export type AprRepoTransferResult =
* throwing on an API error (a non-existent target account, or missing admin access to the repo, come back as a
* structured `{ initiated: false }` result), so callers get a total function they can branch on. A successful
* result models the transfer as INITIATED, not complete — see {@link AprRepoTransferResult}.
*
* Prefer {@link requestAprRepoTransfer} for the customer-facing path — it applies the #7742 completion gate
* before calling this. Direct callers are for tests / internal seams that already enforced the gate.
*/
export async function initiateAprRepoTransfer(
env: Env,
Expand All @@ -51,3 +91,29 @@ export async function initiateAprRepoTransfer(
const payload = (await response.json().catch(() => null)) as { full_name?: string } | null;
return { initiated: true, status: response.status, newFullName: payload?.full_name ?? null };
}

/**
* Customer-facing "request transfer" action (#7742): gate on the idea-completion signal, then call
* {@link initiateAprRepoTransfer}. Never initiates when the idea is incomplete — and nothing in this module
* (or its REST mirror) auto-offers or nudges a transfer.
*/
export async function requestAprRepoTransfer(
env: Env,
input: RequestAprRepoTransferInput,
options: {
initiate?: (
env: Env,
installationId: number,
repoFullName: string,
newOwner: string,
) => Promise<AprRepoTransferResult>;
} = {},
): Promise<RequestAprRepoTransferResult> {
const eligibility = evaluateAprRepoTransferRequestEligibility({ ideaComplete: input.ideaComplete });
if (!eligibility.allowed) return { status: "rejected", reason: eligibility.reason };

const initiate = options.initiate ?? initiateAprRepoTransfer;
const transfer = await initiate(env, input.installationId, input.repoFullName, input.newOwner);
if (transfer.initiated) return { status: "initiated", transfer };
return { status: "failed", transfer };
}
94 changes: 93 additions & 1 deletion test/unit/orb-apr-repo-transfer.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import { createInstallationToken } from "../../src/github/app";
import { initiateAprRepoTransfer } from "../../src/orb/apr-repo-transfer";
import {
evaluateAprRepoTransferRequestEligibility,
initiateAprRepoTransfer,
requestAprRepoTransfer,
} from "../../src/orb/apr-repo-transfer";
import { createTestEnv } from "../helpers/d1";

// The transfer initiation mints an App installation token. Mock that mint to return a plain opaque token string
Expand Down Expand Up @@ -73,4 +77,92 @@ describe("initiateAprRepoTransfer (#7638)", () => {
const result = await initiateAprRepoTransfer(createTestEnv(), 1, "loopover-repos/widgets", "customer-acct");
expect(result).toEqual({ initiated: false, status: 403, error: "transfer request failed (403)" });
});

it("falls back to a status message when response.text() rejects on a non-OK reply", async () => {
stubFetch(
() =>
({
ok: false,
status: 500,
text: async () => {
throw new Error("body unread");
},
}) as unknown as Response,
);
const result = await initiateAprRepoTransfer(createTestEnv(), 1, "loopover-repos/widgets", "customer-acct");
expect(result).toEqual({ initiated: false, status: 500, error: "transfer request failed (500)" });
});
});

describe("evaluateAprRepoTransferRequestEligibility (#7742)", () => {
it("allows a request only when the idea-completion signal is true", () => {
expect(evaluateAprRepoTransferRequestEligibility({ ideaComplete: true })).toEqual({ allowed: true });
});

it("rejects when the idea is not complete — including an explicit false", () => {
expect(evaluateAprRepoTransferRequestEligibility({ ideaComplete: false })).toEqual({
allowed: false,
reason: "idea_not_complete",
});
});
});

describe("requestAprRepoTransfer (#7742)", () => {
afterEach(() => {
vi.unstubAllGlobals();
});

it("rejects without calling initiate when the idea is incomplete", async () => {
const initiate = vi.fn();
const result = await requestAprRepoTransfer(
createTestEnv(),
{ installationId: 1, repoFullName: "loopover-repos/widgets", newOwner: "customer-acct", ideaComplete: false },
{ initiate },
);
expect(result).toEqual({ status: "rejected", reason: "idea_not_complete" });
expect(initiate).not.toHaveBeenCalled();
});

it("initiates when the idea is complete and the GitHub call succeeds", async () => {
const initiate = vi.fn().mockResolvedValue({ initiated: true, status: 202, newFullName: "customer-acct/widgets" });
const env = createTestEnv();
const result = await requestAprRepoTransfer(
env,
{ installationId: 7, repoFullName: "loopover-repos/widgets", newOwner: "customer-acct", ideaComplete: true },
{ initiate },
);
expect(initiate).toHaveBeenCalledWith(env, 7, "loopover-repos/widgets", "customer-acct");
expect(result).toEqual({
status: "initiated",
transfer: { initiated: true, status: 202, newFullName: "customer-acct/widgets" },
});
});

it("surfaces a structured failure when initiate returns initiated:false", async () => {
const initiate = vi.fn().mockResolvedValue({ initiated: false, status: 403, error: "no admin" });
const result = await requestAprRepoTransfer(
createTestEnv(),
{ installationId: 1, repoFullName: "loopover-repos/widgets", newOwner: "customer-acct", ideaComplete: true },
{ initiate },
);
expect(result).toEqual({
status: "failed",
transfer: { initiated: false, status: 403, error: "no admin" },
});
});

it("defaults to initiateAprRepoTransfer when no initiate hook is supplied", async () => {
mockedToken.mockResolvedValue("ghs_installation_token");
stubFetch(() => new Response(JSON.stringify({ full_name: "customer-acct/widgets" }), { status: 202 }));
const result = await requestAprRepoTransfer(createTestEnv(), {
installationId: 1,
repoFullName: "loopover-repos/widgets",
newOwner: "customer-acct",
ideaComplete: true,
});
expect(result).toEqual({
status: "initiated",
transfer: { initiated: true, status: 202, newFullName: "customer-acct/widgets" },
});
});
});
104 changes: 104 additions & 0 deletions test/unit/routes-request-apr-transfer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { createApp } from "../../src/api/routes";
import { createInstallationToken } from "../../src/github/app";
import { createTestEnv } from "../helpers/d1";

// #7742: POST /v1/loop/request-apr-transfer — customer-facing request-only APR transfer. Pins the ROUTE
// contract (status codes + body validation) against the real requestAprRepoTransfer wiring; GitHub is mocked.
vi.mock("../../src/github/app", async (importOriginal) => ({
...(await importOriginal<typeof import("../../src/github/app")>()),
createInstallationToken: vi.fn(),
}));
const mockedToken = vi.mocked(createInstallationToken);

function stubFetch(handler: (url: string, init: RequestInit) => Response): void {
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => handler(String(input), init ?? {}));
}

const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}`, "content-type": "application/json" });
const PATH = "/v1/loop/request-apr-transfer";

const validBody = {
installationId: 42,
repoFullName: "loopover-repos/widgets",
newOwner: "customer-acct",
ideaComplete: true,
};

const post = (env: Env, body: unknown) =>
createApp().request(PATH, { method: "POST", headers: apiHeaders(env), body: JSON.stringify(body) }, env);

describe("POST /v1/loop/request-apr-transfer (#7742)", () => {
beforeEach(() => {
mockedToken.mockReset();
mockedToken.mockResolvedValue("ghs_installation_token");
});
afterEach(() => {
vi.unstubAllGlobals();
});

it("returns 202 with the initiated transfer when the gate passes and GitHub accepts", async () => {
stubFetch(() => new Response(JSON.stringify({ full_name: "customer-acct/widgets" }), { status: 202 }));
const response = await post(createTestEnv(), validBody);
expect(response.status).toBe(202);
await expect(response.json()).resolves.toEqual({
status: "initiated",
transfer: { initiated: true, status: 202, newFullName: "customer-acct/widgets" },
});
});

it("returns 409 without contacting GitHub when the idea is not complete", async () => {
let fetchCalls = 0;
stubFetch(() => {
fetchCalls += 1;
return new Response("{}", { status: 202 });
});
const response = await post(createTestEnv(), { ...validBody, ideaComplete: false });
expect(response.status).toBe(409);
await expect(response.json()).resolves.toEqual({ status: "rejected", reason: "idea_not_complete" });
expect(fetchCalls).toBe(0);
expect(mockedToken).not.toHaveBeenCalled();
});

it("returns 502 when the gate passes but GitHub rejects the transfer", async () => {
stubFetch(() => new Response("", { status: 403 }));
const response = await post(createTestEnv(), validBody);
expect(response.status).toBe(502);
await expect(response.json()).resolves.toEqual({
status: "failed",
transfer: { initiated: false, status: 403, error: "transfer request failed (403)" },
});
});

it("rejects an invalid or unparseable body with 400 before any GitHub call", async () => {
let fetchCalls = 0;
stubFetch(() => {
fetchCalls += 1;
return new Response("{}", { status: 202 });
});
const env = createTestEnv();
for (const body of [
{},
{ ...validBody, installationId: 0 },
{ ...validBody, installationId: -1 },
{ ...validBody, repoFullName: "" },
{ ...validBody, newOwner: "" },
{ ...validBody, ideaComplete: "yes" },
{ installationId: 1, repoFullName: "a/b", newOwner: "c" }, // missing ideaComplete
]) {
const response = await post(env, body);
expect(response.status, JSON.stringify(body)).toBe(400);
await expect(response.json()).resolves.toMatchObject({ error: "invalid_request_apr_transfer_request" });
}
const malformed = await createApp().request(PATH, { method: "POST", headers: apiHeaders(env), body: "{not json" }, env);
expect(malformed.status).toBe(400);
expect(fetchCalls).toBe(0);
expect(mockedToken).not.toHaveBeenCalled();
});

it("leaks no wallet/hotkey/trust-score terms", async () => {
stubFetch(() => new Response(JSON.stringify({ full_name: "customer-acct/widgets" }), { status: 202 }));
const text = JSON.stringify(await (await post(createTestEnv(), validBody)).json());
expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward/i);
});
});