Skip to content
Merged
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
21 changes: 21 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6114,6 +6114,9 @@ function canSessionAccessPath(env: Env, identity: Extract<AuthIdentity, { kind:
if (isRepoOutcomeCalibrationPath(path)) return true;
if (isRepoGatePrecisionPath(path)) return true;
if (isRepoMaintainerNoisePath(path)) return true;
if (isRepoAutomationStatePath(path)) return true; // #8653: route's requireRepoMaintainer enforces per-repo authority
if (isRepoAmsMinerCohortPath(path)) return true; // #8653: route's requireRepoMaintainer enforces per-repo authority
if (isRepoChatQaPath(path)) return true; // #8653: route's requireRepoMaintainer enforces per-repo authority
if (isRepoSelftuneOverridesPath(path)) return true;
if (isRepoSettingsPreviewPath(path)) return true;
if (isRepoOnboardingPackPreviewPath(path)) return true;
Expand Down Expand Up @@ -6154,6 +6157,24 @@ function isRepoMaintainerNoisePath(path: string): boolean {
return /^\/v1\/repos\/[^/]+\/[^/]+\/maintainer-noise$/.test(path);
}

// #8653: three maintainer-session routes documented themselves as reachable by a maintainer's browser panel
// (automation-state "Maintainer-gated like /settings", ams-miner-cohort "mirrors maintainer-noise",
// pulls/:number/chat-qa "exposes ... to apps/loopover-ui's maintainer panel") but were missing from this
// allowlist, so a real non-operator maintainer session hit the coarse 403 before the handler's own
// requireRepoMaintainer/requireRepoWriteAccess guard could admit them. Each route's own guard still enforces
// per-repo authority (a maintainer of A reaching B → 403 forbidden_repo).
function isRepoAutomationStatePath(path: string): boolean {
return /^\/v1\/repos\/[^/]+\/[^/]+\/automation-state$/.test(path);
}

function isRepoAmsMinerCohortPath(path: string): boolean {
return /^\/v1\/repos\/[^/]+\/[^/]+\/ams-miner-cohort$/.test(path);
}

function isRepoChatQaPath(path: string): boolean {
return /^\/v1\/repos\/[^/]+\/[^/]+\/pulls\/[^/]+\/chat-qa$/.test(path);
}

// #6168: let a browser (session) maintainer reach the self-tune override admin routes; the route's own
// requireRepoMaintainer then enforces per-repo authority (a non-maintainer session → 403). Matches the
// gate-precision allowlist entry above. Covers both the audit read and the live-override delete.
Expand Down
48 changes: 47 additions & 1 deletion test/unit/access-boundary.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { createApp } from "../../src/api/routes";
import { createSessionForGitHubUser } from "../../src/auth/security";
import { upsertInstallation, upsertRepositoryFromGitHub } from "../../src/db/repositories";
import { upsertInstallation, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories";
import { createTestEnv } from "../helpers/d1";

// The miner ⊕ maintainer access boundary, locked against regression.
Expand Down Expand Up @@ -82,6 +82,52 @@ describe("access boundary: per-repo maintainer data is repo-scoped", () => {
await expect(own.json()).resolves.toMatchObject({ repoFullName: "alice/repo-a", pendingActions: [] });
});

// #8653: three maintainer-session routes were missing from canSessionAccessPath, so a maintainer's own
// browser session hit the coarse 403 before the route's own guard could admit it. Each route's guard still
// scopes per-repo (maintainer of A → 403 forbidden_repo on B).
it("a maintainer can REACH automation-state on their OWN repo, scoped per-repo (allowlist parity with /settings)", async () => {
const { app, env } = await setup();
const { token } = await createSessionForGitHubUser(env, { login: "alice", id: 101 });
const cookie = `loopover_session=${token}`;
expect((await app.request("/v1/repos/alice/repo-a/automation-state", { headers: { cookie } }, env)).status).toBe(200);
const other = await app.request("/v1/repos/bob/repo-b/automation-state", { headers: { cookie } }, env);
expect(other.status).toBe(403);
expect(await other.json()).toMatchObject({ error: "forbidden_repo" });
});

it("a maintainer can REACH ams-miner-cohort on their OWN repo, scoped per-repo (allowlist parity with maintainer-noise)", async () => {
const { app, env } = await setup();
const { token } = await createSessionForGitHubUser(env, { login: "alice", id: 101 });
const cookie = `loopover_session=${token}`;
expect((await app.request("/v1/repos/alice/repo-a/ams-miner-cohort", { headers: { cookie } }, env)).status).toBe(200);
const other = await app.request("/v1/repos/bob/repo-b/ams-miner-cohort", { headers: { cookie } }, env);
expect(other.status).toBe(403);
expect(await other.json()).toMatchObject({ error: "forbidden_repo" });
});

it("a maintainer can REACH pulls/:number/chat-qa on their OWN repo, scoped per-repo (allowlist parity with the maintainer panel)", async () => {
const { app, env } = await setup();
// chat-qa needs a real PR to resolve; the answer service returns a 200 "disabled" status by default
// (advisoryAiRouting.chatQa off with no .loopover.yml), so no AI mock is needed to prove reachability.
await upsertPullRequestFromGitHub(env, "alice/repo-a", { number: 7, title: "t", state: "open", user: { login: "someone" }, labels: [], body: "x" });
const { token } = await createSessionForGitHubUser(env, { login: "alice", id: 101 });
const cookie = `loopover_session=${token}`;
const own = await app.request(
"/v1/repos/alice/repo-a/pulls/7/chat-qa",
{ method: "POST", headers: { cookie }, body: JSON.stringify({ question: "why is this blocked?" }) },
env,
);
expect(own.status).toBe(200);
// The per-route requireRepoMaintainer still scopes: maintainer of A cannot reach B's chat-qa.
const other = await app.request(
"/v1/repos/bob/repo-b/pulls/7/chat-qa",
{ method: "POST", headers: { cookie }, body: JSON.stringify({ question: "why?" }) },
env,
);
expect(other.status).toBe(403);
expect(await other.json()).toMatchObject({ error: "forbidden_repo" });
});

it("a pure miner (no maintainer role on any repo) cannot read ANY repo's maintainer settings", async () => {
const { app, env } = await setup();
const { token } = await createSessionForGitHubUser(env, { login: "miner-only", id: 900 });
Expand Down