diff --git a/src/auth/rate-limit.ts b/src/auth/rate-limit.ts index 7402f3d70b..207c87562f 100644 --- a/src/auth/rate-limit.ts +++ b/src/auth/rate-limit.ts @@ -116,6 +116,7 @@ export function routeClassForPath(path: string): RateLimitClass { path.includes("/decision-pack") || path.includes("/miner-dashboard/refresh") || path.includes("/open-pr-monitor") || + path === "/v1/opportunities/find" || // Maintainer BYOK config: POST /ai-key and /linear-key both run PBKDF2 (100k iters) + an encrypted D1 // upsert per request. /\/(?:ai-(?:key|review)|linear-key)$/.test(path) || diff --git a/src/mcp/find-opportunities.ts b/src/mcp/find-opportunities.ts index 0e6a310f31..8f5a68e58a 100644 --- a/src/mcp/find-opportunities.ts +++ b/src/mcp/find-opportunities.ts @@ -55,6 +55,11 @@ export type FindOpportunitiesResult = { const DEFAULT_LIMIT = 5; const MAX_LIMIT = 50; +export const MAX_FIND_OPPORTUNITIES_TARGETS = 25; +export const MAX_FIND_OPPORTUNITIES_OWNER_LENGTH = 39; +export const MAX_FIND_OPPORTUNITIES_REPO_LENGTH = 100; +export const MAX_FIND_OPPORTUNITIES_LANGUAGES = 20; +export const MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH = 30; function clamp01(value: number): number { if (!Number.isFinite(value)) return 0; @@ -81,14 +86,34 @@ export function validateFindOpportunitiesInput( if (!hasTargets && !hasSearch) { return { ok: false, reason: "targets_or_search_query_required" }; } + let normalizedTargets: FindOpportunitiesTarget[] | undefined; if (hasTargets) { + if (targets!.length > MAX_FIND_OPPORTUNITIES_TARGETS) return { ok: false, reason: "too_many_targets" }; + const seenTargets = new Set(); + normalizedTargets = []; for (const target of targets!) { const owner = typeof target?.owner === "string" ? target.owner.trim() : ""; const repo = typeof target?.repo === "string" ? target.repo.trim() : ""; if (!owner || !repo) return { ok: false, reason: "invalid_target" }; + if (owner.length > MAX_FIND_OPPORTUNITIES_OWNER_LENGTH) return { ok: false, reason: "owner_too_long" }; + if (repo.length > MAX_FIND_OPPORTUNITIES_REPO_LENGTH) return { ok: false, reason: "repo_too_long" }; + const key = `${owner.toLowerCase()}/${repo.toLowerCase()}`; + if (seenTargets.has(key)) continue; + seenTargets.add(key); + normalizedTargets.push({ owner, repo }); } } if (hasSearch && searchQuery.length > 500) return { ok: false, reason: "search_query_too_long" }; + const languages = input.goalSpec?.languages; + if (languages !== undefined) { + if (!Array.isArray(languages) || languages.length > MAX_FIND_OPPORTUNITIES_LANGUAGES) { + return { ok: false, reason: "invalid_languages" }; + } + for (const language of languages) { + const value = typeof language === "string" ? language.trim() : ""; + if (!value || value.length > MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH) return { ok: false, reason: "invalid_languages" }; + } + } const minRankScore = input.goalSpec?.minRankScore; if (minRankScore !== undefined && (!Number.isFinite(minRankScore) || minRankScore < 0 || minRankScore > 100)) { return { ok: false, reason: "invalid_min_rank_score" }; @@ -96,7 +121,7 @@ export function validateFindOpportunitiesInput( return { ok: true, value: { - ...(hasTargets ? { targets } : {}), + ...(normalizedTargets ? { targets: normalizedTargets } : {}), ...(hasSearch ? { searchQuery } : {}), ...(input.goalSpec ? { goalSpec: input.goalSpec } : {}), ...(input.limit !== undefined ? { limit: input.limit } : {}), diff --git a/src/mcp/server.ts b/src/mcp/server.ts index c4653c3444..762bb66d37 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -4,7 +4,15 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { RequestHandlerExtra } from "@modelcontextprotocol/sdk/shared/protocol.js"; import { ElicitResultSchema, type ServerNotification, type ServerRequest } from "@modelcontextprotocol/sdk/types.js"; import { z } from "zod"; -import { runFindOpportunities, validateFindOpportunitiesInput } from "./find-opportunities"; +import { + MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH, + MAX_FIND_OPPORTUNITIES_LANGUAGES, + MAX_FIND_OPPORTUNITIES_OWNER_LENGTH, + MAX_FIND_OPPORTUNITIES_REPO_LENGTH, + MAX_FIND_OPPORTUNITIES_TARGETS, + runFindOpportunities, + validateFindOpportunitiesInput, +} from "./find-opportunities"; import { authenticatePrivateToken, extractBearerToken, @@ -216,17 +224,18 @@ const findOpportunitiesShape = { targets: z .array( z.object({ - owner: z.string().min(1), - repo: z.string().min(1), + owner: z.string().min(1).max(MAX_FIND_OPPORTUNITIES_OWNER_LENGTH), + repo: z.string().min(1).max(MAX_FIND_OPPORTUNITIES_REPO_LENGTH), }), ) + .max(MAX_FIND_OPPORTUNITIES_TARGETS) .optional(), searchQuery: z.string().min(1).max(500).optional(), goalSpec: z .object({ lane: z.string().min(1).optional(), minRankScore: z.number().min(0).max(100).optional(), - languages: z.array(z.string().min(1)).optional(), + languages: z.array(z.string().min(1).max(MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH)).max(MAX_FIND_OPPORTUNITIES_LANGUAGES).optional(), }) .optional(), limit: z.number().int().min(1).max(50).optional(), diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index 11aef8ec8a..9aab779309 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -111,6 +111,7 @@ describe("private-beta auth and rate limiting", () => { expect(routeClassForPath("/v1/contributors/jsonbored/decision-pack")).toBe("expensive"); expect(routeClassForPath("/v1/app/miner-dashboard/refresh")).toBe("expensive"); expect(routeClassForPath("/v1/contributors/jsonbored/open-pr-monitor")).toBe("expensive"); + expect(routeClassForPath("/v1/opportunities/find")).toBe("expensive"); expect(routeClassForPath("/v1/installations/999/repair/refresh")).toBe("expensive"); expect(routeClassForPath("/v1/internal/jobs/generate-signal-snapshots")).toBe("expensive"); expect(routeClassForPath("/v1/internal/jobs/build-contributor-decision-packs")).toBe("expensive"); diff --git a/test/unit/find-opportunities.test.ts b/test/unit/find-opportunities.test.ts index e921719c65..0a98654216 100644 --- a/test/unit/find-opportunities.test.ts +++ b/test/unit/find-opportunities.test.ts @@ -3,6 +3,11 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { afterEach, describe, expect, it, vi } from "vitest"; import { + MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH, + MAX_FIND_OPPORTUNITIES_LANGUAGES, + MAX_FIND_OPPORTUNITIES_OWNER_LENGTH, + MAX_FIND_OPPORTUNITIES_REPO_LENGTH, + MAX_FIND_OPPORTUNITIES_TARGETS, normalizeFindOpportunitiesLimit, publicRankScore, runFindOpportunities, @@ -61,10 +66,33 @@ describe("validateFindOpportunitiesInput", () => { it("rejects invalid targets and oversized search queries", () => { expect(validateFindOpportunitiesInput({ targets: [{ owner: "", repo: "demo" }] })).toEqual({ ok: false, reason: "invalid_target" }); + expect(validateFindOpportunitiesInput({ targets: [{ owner: 123 as unknown as string, repo: "demo" }] })).toEqual({ + ok: false, + reason: "invalid_target", + }); + expect(validateFindOpportunitiesInput({ targets: [{ owner: "acme", repo: 456 as unknown as string }] })).toEqual({ + ok: false, + reason: "invalid_target", + }); + expect( + validateFindOpportunitiesInput({ + targets: Array.from({ length: MAX_FIND_OPPORTUNITIES_TARGETS + 1 }, () => ({ owner: "acme", repo: "demo" })), + }), + ).toEqual({ ok: false, reason: "too_many_targets" }); + expect( + validateFindOpportunitiesInput({ targets: [{ owner: "x".repeat(MAX_FIND_OPPORTUNITIES_OWNER_LENGTH + 1), repo: "demo" }] }), + ).toEqual({ ok: false, reason: "owner_too_long" }); + expect( + validateFindOpportunitiesInput({ targets: [{ owner: "acme", repo: "x".repeat(MAX_FIND_OPPORTUNITIES_REPO_LENGTH + 1) }] }), + ).toEqual({ ok: false, reason: "repo_too_long" }); expect(validateFindOpportunitiesInput({ searchQuery: "x".repeat(501) })).toEqual({ ok: false, reason: "search_query_too_long" }); expect( validateFindOpportunitiesInput({ searchQuery: "docs", goalSpec: { minRankScore: 101 } }), ).toEqual({ ok: false, reason: "invalid_min_rank_score" }); + expect(validateFindOpportunitiesInput({ searchQuery: "docs", goalSpec: { languages: [""] } })).toEqual({ + ok: false, + reason: "invalid_languages", + }); }); it("accepts trimmed targets and search queries", () => { @@ -75,11 +103,75 @@ describe("validateFindOpportunitiesInput", () => { }); expect(parsed.ok).toBe(true); if (parsed.ok) { - expect(parsed.value.targets?.[0]).toEqual({ owner: " acme ", repo: " widgets " }); + expect(parsed.value.targets?.[0]).toEqual({ owner: "acme", repo: "widgets" }); expect(parsed.value.goalSpec).toEqual({ lane: "docs", minRankScore: 40 }); expect(parsed.value.limit).toBe(3); } }); + + it("deduplicates targets before downstream authorization and lookup work", () => { + const parsed = validateFindOpportunitiesInput({ + targets: [ + { owner: " acme ", repo: " widgets " }, + { owner: "ACME", repo: "widgets" }, + { owner: "acme", repo: "other" }, + ], + }); + expect(parsed.ok).toBe(true); + if (parsed.ok) { + expect(parsed.value.targets).toEqual([ + { owner: "acme", repo: "widgets" }, + { owner: "acme", repo: "other" }, + ]); + } + }); + + it("accepts exactly MAX_FIND_OPPORTUNITIES_TARGETS targets (boundary, not just the +1 overflow)", () => { + const parsed = validateFindOpportunitiesInput({ + targets: Array.from({ length: MAX_FIND_OPPORTUNITIES_TARGETS }, (_, i) => ({ owner: "acme", repo: `demo${i}` })), + }); + expect(parsed.ok).toBe(true); + if (parsed.ok) expect(parsed.value.targets).toHaveLength(MAX_FIND_OPPORTUNITIES_TARGETS); + }); + + it("rejects a non-array goalSpec.languages", () => { + expect( + validateFindOpportunitiesInput({ searchQuery: "docs", goalSpec: { languages: "typescript" as unknown as string[] } }), + ).toEqual({ ok: false, reason: "invalid_languages" }); + }); + + it("rejects a non-string language entry", () => { + expect( + validateFindOpportunitiesInput({ searchQuery: "docs", goalSpec: { languages: [123 as unknown as string] } }), + ).toEqual({ ok: false, reason: "invalid_languages" }); + }); + + it("rejects more than MAX_FIND_OPPORTUNITIES_LANGUAGES languages", () => { + expect( + validateFindOpportunitiesInput({ + searchQuery: "docs", + goalSpec: { languages: Array.from({ length: MAX_FIND_OPPORTUNITIES_LANGUAGES + 1 }, (_, i) => `lang${i}`) }, + }), + ).toEqual({ ok: false, reason: "invalid_languages" }); + }); + + it("rejects a language entry longer than MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH", () => { + expect( + validateFindOpportunitiesInput({ + searchQuery: "docs", + goalSpec: { languages: ["x".repeat(MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH + 1)] }, + }), + ).toEqual({ ok: false, reason: "invalid_languages" }); + }); + + it("accepts a valid languages list at or under the boundary", () => { + const parsed = validateFindOpportunitiesInput({ + searchQuery: "docs", + goalSpec: { languages: ["typescript", "x".repeat(MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH)] }, + }); + expect(parsed.ok).toBe(true); + if (parsed.ok) expect(parsed.value.goalSpec).toEqual({ languages: ["typescript", "x".repeat(MAX_FIND_OPPORTUNITIES_LANGUAGE_LENGTH)] }); + }); }); describe("find-opportunities helpers", () => { @@ -132,6 +224,38 @@ describe("runFindOpportunities", () => { }); }); + it("checks access only once for duplicate targets", async () => { + const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "test-token" }); + const allowedPolicy = readFixture("allowed-silent.md"); + const accessChecks: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/repos/acme/allowed/contents/AI-USAGE.md")) return jsonResponse({}, { status: 404 }); + if (url.includes("/repos/acme/allowed/contents/CONTRIBUTING.md")) return contentResponse(allowedPolicy); + if (url.includes("/repos/acme/allowed/issues?")) return jsonResponse([issue(5)]); + return jsonResponse({}, { status: 404 }); + }); + + const result = await runFindOpportunities( + env, + { + targets: [ + { owner: "acme", repo: "allowed" }, + { owner: "ACME", repo: "allowed" }, + ], + }, + { + canAccessRepo: async (repoFullName) => { + accessChecks.push(repoFullName); + return true; + }, + }, + ); + + expect(result.status).toBe("ok"); + expect(accessChecks).toEqual(["acme/allowed"]); + }); + it("filters inaccessible targets via canAccessRepo", async () => { const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "test-token" }); await upsertRepositoryFromGitHub(env, { name: "allowed", full_name: "acme/allowed" }); diff --git a/test/unit/mcp-find-opportunities.test.ts b/test/unit/mcp-find-opportunities.test.ts index 9ffeb49b6c..c7b8b17f62 100644 --- a/test/unit/mcp-find-opportunities.test.ts +++ b/test/unit/mcp-find-opportunities.test.ts @@ -115,6 +115,19 @@ describe("MCP gittensory_find_opportunities", () => { expect(JSON.stringify(data)).not.toMatch(/wallet|hotkey|reward estimate|trust score/i); }); + it("rejects oversized target lists before authorization", async () => { + const env = createTestEnv(); + const client = await connect(env); + + const result = await client.callTool({ + name: "gittensory_find_opportunities", + arguments: { targets: Array.from({ length: 26 }, () => ({ owner: "acme", repo: "allowed" })) }, + }); + + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/Too big|maximum|25/i); + }); + it("rejects cross-repo search for non-operator sessions", async () => { const env = createTestEnv(); const { session } = await createSessionForGitHubUser(env, { login: "miner1", id: 1 });