mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 01:46:15 +02:00
Close OAuth admission abuse paths and verify API and eval contracts
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
826de9e6c2
commit
fb7946a4b8
13 files changed
+1096
-17
No files matched your search
+9
-2
@@ -86,8 +86,8 @@ authenticated browser and the configured origin. Client registration does not
|
||||
fetch redirect URLs or accept arbitrary tool destinations. Rate limiting at the
|
||||
public edge is required in addition to the bounded per-process auth limiter.
|
||||
Registration also has a database-enforced limit of 60 new clients per minute and
|
||||
10,000 retained clients shared across replicas. Expired authorization requests
|
||||
and never-consented clients older than 24 hours are collected during
|
||||
10,000 unconsented clients shared across replicas. Expired authorization requests
|
||||
and never-consented clients older than one hour are collected during
|
||||
registration. Clients with grants are retained, preserving their connections and
|
||||
audit/mutation history.
|
||||
|
||||
@@ -168,3 +168,10 @@ CLI login and local package validation do not replace these release gates.
|
||||
External sessions acting as agents, task leases and third-party granted tools
|
||||
remain the second/third releases; do not add generic executors to this public
|
||||
catalog to implement them.
|
||||
|
||||
Registration also enforces shared source quotas (6 per minute and 30 unconsented
|
||||
clients) using a resource-bound hash of the trusted request IP; raw addresses are
|
||||
not stored. Configure trusted proxies correctly. Authorization starts atomically
|
||||
remove expired requests and enforce 10 retained requests per client and 1,000
|
||||
instance-wide, independently of further client registrations. Existing grants
|
||||
remain usable when anonymous registration or authorization is throttled.
|
||||
@@ -0,0 +1 @@
|
||||
ALTER TABLE "mcp_oauth_clients" ADD COLUMN IF NOT EXISTS "registration_source_hash" text;
|
||||
+977
-2
File diff suppressed because it is too large.
Load diff
+7
@@ -2052,6 +2052,13 @@
|
||||
"when": 1790866454887,
|
||||
"tag": "0294_nervous_caretaker",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 295,
|
||||
"version": "7",
|
||||
"when": 1790867992285,
|
||||
"tag": "0295_slimy_thunderbird",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import { companies } from "./companies.js";
|
||||
export const mcpOauthClients = pgTable("mcp_oauth_clients", {
|
||||
id: text("id").primaryKey(),
|
||||
name: text("name").notNull(),
|
||||
registrationSourceHash: text("registration_source_hash"),
|
||||
redirectUris: jsonb("redirect_uris").$type<string[]>().notNull(),
|
||||
createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(),
|
||||
});
|
||||
|
||||
@@ -61,6 +61,7 @@ const apiPrefixes: Record<string, string> = {
|
||||
"plugin-ui-static.ts": "/api",
|
||||
"plugins.ts": "/api",
|
||||
"projects.ts": "/api",
|
||||
"public-mcp.ts": "/api",
|
||||
"project-tools.ts": "/api",
|
||||
"resource-memberships.ts": "/api",
|
||||
"remote-agent-profiles.ts": "/api",
|
||||
@@ -92,6 +93,13 @@ const HTTP_METHODS = new Set([
|
||||
const explicitOpenApiCoverageExclusions = new Set<string>();
|
||||
|
||||
const explicitOpenApiOperationCoverageExclusions = new Set([
|
||||
// OAuth discovery and protocol endpoints have their own metadata contract;
|
||||
// browser connection-management operations remain documented in the board API.
|
||||
"GET /.well-known/oauth-authorization-server",
|
||||
"POST /mcp/oauth/register",
|
||||
"GET /mcp/oauth/authorize",
|
||||
"POST /mcp/oauth/token",
|
||||
"POST /mcp/oauth/revoke",
|
||||
// This endpoint is authenticated by the provider signature rather than by a
|
||||
// Paperclip board/agent credential. It intentionally stays out of the public
|
||||
// board API document, while this exact exclusion keeps route coverage honest.
|
||||
@@ -132,6 +140,7 @@ function normalizeExpressPath(routePath: string) {
|
||||
}
|
||||
|
||||
function resolveMountedPath(file: string, prefix: string, routePath: string) {
|
||||
if (file === "public-mcp.ts" && (routePath.startsWith("/mcp/oauth/") || routePath.startsWith("/.well-known/"))) return routePath;
|
||||
if (
|
||||
(file === "chat-channels.ts" || file === "email.ts") &&
|
||||
routePath.startsWith("/api/chat-webhooks/")
|
||||
|
||||
@@ -3,7 +3,7 @@ import express, { type Request } from "express";
|
||||
import request from "supertest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { createDb, authUsers, companies, companyMemberships, mcpOauthTokens, mcpOauthGrants, mcpOauthClients, mcpMutationReceipts, agents, issues, issueComments, instanceUserRoles } from "@paperclipai/db";
|
||||
import { createDb, authUsers, companies, companyMemberships, mcpOauthTokens, mcpOauthRequests, mcpOauthGrants, mcpOauthClients, mcpMutationReceipts, agents, issues, issueComments, instanceUserRoles } from "@paperclipai/db";
|
||||
import { createPublicMcpOAuth, publicMcpConfig, hashMcpSecret } from "../services/public-mcp/oauth.js";
|
||||
import { createMcpApiDispatch, createPublicMcpExecutor, publicMcpCapabilities } from "../services/public-mcp/capabilities.js";
|
||||
import { publicMcpIngressRoutes, publicMcpManagementRoutes } from "../routes/public-mcp.js";
|
||||
@@ -51,7 +51,7 @@ describe.skipIf(!support.supported)("public MCP OAuth and tool boundary", () =>
|
||||
const [company] = await db.insert(companies).values({ name: "Team", issuePrefix: "M" + randomBytes(4).toString("hex") }).returning();
|
||||
const [membership] = await db.insert(companyMemberships).values({ companyId: company!.id, principalType: "user", principalId: userId, membershipRole: role, status: "active" }).returning();
|
||||
const actor: Request["actor"] = { type: "board", source: "session", userId };
|
||||
const client = await oauth.register({ client_name: "Test client", redirect_uris: [redirectUri] });
|
||||
const client = await oauth.register({ client_name: "Test client", redirect_uris: [redirectUri] }, randomUUID());
|
||||
const url = await oauth.authorize({ client_id: client.client_id, redirect_uri: redirectUri, resource: config.resource, scope: "paperclip:read paperclip:write offline_access", state: "state", response_type: "code", code_challenge: challenge, code_challenge_method: "S256" });
|
||||
const id = url.split("/").at(-1)!;
|
||||
const consent = await oauth.consent(id, actor, { decision: "approve", companyId: company!.id, allowWrites: write });
|
||||
@@ -268,7 +268,7 @@ describe.skipIf(!support.supported)("public MCP OAuth and tool boundary", () =>
|
||||
await db.insert(mcpOauthClients).values({ id: stale, name: "Never connected", redirectUris: [redirectUri], createdAt: old });
|
||||
await db.update(mcpOauthClients).set({ createdAt: old });
|
||||
const replica = createPublicMcpOAuth(db, config);
|
||||
const registrations = await Promise.allSettled(Array.from({ length: 70 }, (_, index) => (index % 2 ? oauth : replica).register({ client_name: "Quota fixture", redirect_uris: [redirectUri] })));
|
||||
const registrations = await Promise.allSettled(Array.from({ length: 70 }, (_, index) => (index % 2 ? oauth : replica).register({ client_name: "Quota fixture", redirect_uris: [redirectUri] }, `quota-source-${index}`)));
|
||||
const accepted = registrations.filter((r): r is PromiseFulfilledResult<Awaited<ReturnType<typeof oauth.register>>> => r.status === "fulfilled");
|
||||
try {
|
||||
expect(accepted).toHaveLength(60);
|
||||
@@ -280,4 +280,33 @@ describe.skipIf(!support.supported)("public MCP OAuth and tool boundary", () =>
|
||||
}
|
||||
});
|
||||
|
||||
it("limits one registration source without denying another source", async () => {
|
||||
const source = randomUUID();
|
||||
const sourceHash = hashMcpSecret(config.resource + ":" + source);
|
||||
const replica = createPublicMcpOAuth(db, config);
|
||||
try {
|
||||
const batch = await Promise.allSettled(Array.from({ length: 8 }, (_, i) => (i % 2 ? oauth : replica).register({ client_name: "Source quota", redirect_uris: [redirectUri] }, source)));
|
||||
expect(batch.filter(r => r.status === "fulfilled")).toHaveLength(6);
|
||||
await expect(replica.register({ client_name: "Other source", redirect_uris: [redirectUri] }, randomUUID())).resolves.toHaveProperty("client_id");
|
||||
for (let batch = 0; batch < 4; batch++) {
|
||||
await db.update(mcpOauthClients).set({ createdAt: new Date(Date.now() - 2 * 60_000) }).where(eq(mcpOauthClients.registrationSourceHash, sourceHash));
|
||||
for (let i = 0; i < 6; i++) await replica.register({ client_name: "Source quota", redirect_uris: [redirectUri] }, source);
|
||||
}
|
||||
await db.update(mcpOauthClients).set({ createdAt: new Date(Date.now() - 2 * 60_000) }).where(eq(mcpOauthClients.registrationSourceHash, sourceHash));
|
||||
await expect(oauth.register({ client_name: "Source quota", redirect_uris: [redirectUri] }, source)).rejects.toMatchObject({ status: 429 });
|
||||
} finally { await db.delete(mcpOauthClients).where(eq(mcpOauthClients.registrationSourceHash, sourceHash)); }
|
||||
});
|
||||
|
||||
it("bounds authorization starts across replicas and reclaims expired requests without another registration", async () => {
|
||||
const client = await oauth.register({ client_name: "Request quota", redirect_uris: [redirectUri] }, randomUUID());
|
||||
const input = { client_id: client.client_id, redirect_uri: redirectUri, response_type: "code", resource: config.resource, code_challenge: challenge, code_challenge_method: "S256" };
|
||||
const replica = createPublicMcpOAuth(db, config);
|
||||
const starts = await Promise.allSettled(Array.from({ length: 12 }, (_, i) => (i % 2 ? oauth : replica).authorize(input)));
|
||||
expect(starts.filter(r => r.status === "fulfilled")).toHaveLength(10);
|
||||
expect(starts.filter(r => r.status === "rejected").map(r => r.reason.status)).toEqual([429, 429]);
|
||||
await db.update(mcpOauthRequests).set({ expiresAt: new Date(0) }).where(eq(mcpOauthRequests.clientId, client.client_id));
|
||||
await expect(replica.authorize(input)).resolves.toContain("/mcp-connect/");
|
||||
expect(await db.select().from(mcpOauthRequests).where(eq(mcpOauthRequests.clientId, client.client_id))).toHaveLength(1);
|
||||
});
|
||||
|
||||
});
|
||||
@@ -264,6 +264,7 @@ import {
|
||||
revokeToolTrustRuleSchema,
|
||||
unbindToolProfileBindingSchema,
|
||||
importMcpJsonSchema,
|
||||
mcpConsentSchema,
|
||||
toolPolicyTestRequestSchema,
|
||||
createToolMcpGatewaySchema,
|
||||
completeConnectionIntentSchema,
|
||||
@@ -1660,6 +1661,8 @@ function resolveOperationAuthLevel(
|
||||
path: string,
|
||||
): OpenApiAuthLevel {
|
||||
const key = operationKey(method, path);
|
||||
if (key === "GET /api/mcp/requests/{id}") return "public";
|
||||
if (path.startsWith("/api/mcp/requests/") || path.startsWith("/api/mcp/connections")) return "board";
|
||||
if (PUBLIC_OPERATIONS.has(key)) return "public";
|
||||
if (key === "POST /api/mcp/project-tools" || key === "POST /api/companies/{companyId}/slack/tasks/{issueId}/tools") return "agent_run";
|
||||
if (RUNTIME_TOOLS_OPERATIONS.has(key)) return "runtime_tools";
|
||||
@@ -11299,6 +11302,28 @@ registerCurrentRoute({
|
||||
},
|
||||
});
|
||||
|
||||
registerCurrentRoute({
|
||||
method: "get", path: "/api/mcp/requests/{id}", tags: ["tool-gateway"],
|
||||
summary: "Describe an assistant connection request and available sign-in options",
|
||||
responses: { 200: r.ok(), 404: r.notFound },
|
||||
});
|
||||
registerCurrentRoute({
|
||||
method: "post", path: "/api/mcp/requests/{id}/consent", tags: ["tool-gateway"],
|
||||
summary: "Approve or deny assistant access using a same-origin browser session",
|
||||
body: mcpConsentSchema,
|
||||
responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden },
|
||||
});
|
||||
registerCurrentRoute({
|
||||
method: "get", path: "/api/mcp/connections", tags: ["tool-gateway"],
|
||||
summary: "List the signed-in person's assistant connections",
|
||||
responses: { 200: r.ok(), 401: r.unauthorized },
|
||||
});
|
||||
registerCurrentRoute({
|
||||
method: "delete", path: "/api/mcp/connections/{id}", tags: ["tool-gateway"],
|
||||
summary: "Revoke an assistant connection using a same-origin browser session",
|
||||
responses: { 204: { description: "Connection revoked" }, 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden },
|
||||
});
|
||||
|
||||
registerCurrentRoute({
|
||||
method: "get",
|
||||
path: "/api/tool-gateway/gateways/{gatewayId}/mcp",
|
||||
|
||||
@@ -55,7 +55,7 @@ export function publicMcpIngressRoutes(oauth: PublicMcpOAuth, execute: ReturnTyp
|
||||
code_challenge_methods_supported: ["S256"], scopes_supported: PUBLIC_MCP_SCOPES,
|
||||
}));
|
||||
router.use("/mcp/oauth", authRateLimit(), express.urlencoded({ extended: false, limit: "16kb" }));
|
||||
router.post("/mcp/oauth/register", async (req, res) => res.status(201).json(await oauth.register(req.body)));
|
||||
router.post("/mcp/oauth/register", async (req, res) => res.status(201).json(await oauth.register(req.body, req.ip ?? req.socket.remoteAddress ?? "unknown")));
|
||||
router.get("/mcp/oauth/authorize", async (req, res) => res.redirect(303, await oauth.authorize(req.query)));
|
||||
router.post("/mcp/oauth/token", async (req, res) => res.json(await oauth.token(req.body ?? {})));
|
||||
router.post("/mcp/oauth/revoke", async (req, res) => {
|
||||
|
||||
@@ -98,24 +98,26 @@ export function createPublicMcpOAuth(db: Db, config: PublicMcpConfig) {
|
||||
|
||||
return {
|
||||
config,
|
||||
async register(input: unknown) {
|
||||
async register(input: unknown, source = "unknown") {
|
||||
const parsed = registrationSchema.safeParse(input);
|
||||
if (!parsed.success) throw new McpOAuthError("invalid_client_metadata", "Supply a client name, valid redirect URIs, and public-client PKCE authentication.");
|
||||
const client = { id: secret("pcmcp_client_"), name: parsed.data.client_name, redirectUris: parsed.data.redirect_uris };
|
||||
const client = { id: secret("pcmcp_client_"), name: parsed.data.client_name, registrationSourceHash: hashMcpSecret(config.resource + ":" + source), redirectUris: parsed.data.redirect_uris };
|
||||
await db.transaction(async (tx) => {
|
||||
// Bound public DCR across replicas, not only per-IP in each process.
|
||||
await tx.execute(sql`SELECT pg_advisory_xact_lock(736721042)`);
|
||||
const now = new Date();
|
||||
await tx.delete(mcpOauthRequests).where(lt(mcpOauthRequests.expiresAt, now));
|
||||
await tx.delete(mcpOauthClients).where(and(
|
||||
lt(mcpOauthClients.createdAt, new Date(now.getTime() - 24 * 60 * minute)),
|
||||
lt(mcpOauthClients.createdAt, new Date(now.getTime() - 60 * minute)),
|
||||
notExists(tx.select({ id: mcpOauthGrants.id }).from(mcpOauthGrants).where(eq(mcpOauthGrants.clientId, mcpOauthClients.id))),
|
||||
notExists(tx.select({ id: mcpOauthRequests.id }).from(mcpOauthRequests).where(eq(mcpOauthRequests.clientId, mcpOauthClients.id))),
|
||||
));
|
||||
const [counts] = await tx.select({ total: sql<number>`count(*)::int`,
|
||||
sourceTotal: sql<number>`(count(*) FILTER (WHERE ${mcpOauthClients.registrationSourceHash} = ${client.registrationSourceHash}))::int`,
|
||||
sourceRecent: sql<number>`(count(*) FILTER (WHERE ${mcpOauthClients.registrationSourceHash} = ${client.registrationSourceHash} AND ${mcpOauthClients.createdAt} > ${new Date(now.getTime() - minute).toISOString()}::timestamptz))::int`,
|
||||
recent: sql<number>`(count(*) FILTER (WHERE ${mcpOauthClients.createdAt} > ${new Date(now.getTime() - minute).toISOString()}::timestamptz))::int`,
|
||||
}).from(mcpOauthClients);
|
||||
if (!counts || counts.total >= 10_000 || counts.recent >= 60) {
|
||||
}).from(mcpOauthClients).where(notExists(tx.select({ id: mcpOauthGrants.id }).from(mcpOauthGrants).where(eq(mcpOauthGrants.clientId, mcpOauthClients.id))));
|
||||
if (!counts || counts.total >= 10_000 || counts.recent >= 60 || counts.sourceTotal >= 30 || counts.sourceRecent >= 6) {
|
||||
throw new McpOAuthError("temporarily_unavailable", "Registration capacity reached. Retry later.", 429);
|
||||
}
|
||||
await tx.insert(mcpOauthClients).values(client);
|
||||
@@ -136,9 +138,20 @@ export function createPublicMcpOAuth(db: Db, config: PublicMcpConfig) {
|
||||
throw new McpOAuthError("invalid_scope", "Unsupported Paperclip scope.");
|
||||
}
|
||||
const id = secret("pcmcp_request_");
|
||||
await db.insert(mcpOauthRequests).values({
|
||||
id, clientId: client.id, redirectUri: p.redirect_uri, resource: p.resource, scopes,
|
||||
state: p.state ?? null, challenge: p.code_challenge, expiresAt: new Date(Date.now() + 10 * minute),
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.execute(sql`SELECT pg_advisory_xact_lock(736721042)`);
|
||||
const now = new Date();
|
||||
await tx.delete(mcpOauthRequests).where(lt(mcpOauthRequests.expiresAt, now));
|
||||
const [counts] = await tx.select({ total: sql<number>`count(*)::int`,
|
||||
client: sql<number>`(count(*) FILTER (WHERE ${mcpOauthRequests.clientId} = ${client.id}))::int`,
|
||||
}).from(mcpOauthRequests);
|
||||
if (!counts || counts.total >= 1000 || counts.client >= 10) {
|
||||
throw new McpOAuthError("temporarily_unavailable", "Too many pending connection requests. Retry later.", 429);
|
||||
}
|
||||
await tx.insert(mcpOauthRequests).values({
|
||||
id, clientId: client.id, redirectUri: p.redirect_uri, resource: p.resource, scopes,
|
||||
state: p.state ?? null, challenge: p.code_challenge, expiresAt: new Date(now.getTime() + 10 * minute),
|
||||
});
|
||||
});
|
||||
return config.origin + "/mcp-connect/" + id;
|
||||
},
|
||||
|
||||
@@ -1102,7 +1102,7 @@ export const runnerSuites: readonly RunnerSuiteFixture[] = [
|
||||
extraConfig: { engine: "cli", maxTurnsPerRun: 16 },
|
||||
})),
|
||||
],
|
||||
definitionMetadata: { version: 4, grader: publicMcpGraderVersion, workflowDigest: publicMcpWorkflowDigest, workerSkillDigest: publicMcpWorkerSkillDigest, workerInstructions: publicMcpWorkerInstructions, assistantMaxRequests: 16, assistantMaxEstimatedUsd: 2, instructions: "shipped-plugin-skills", scheduling: "explicit-only" },
|
||||
definitionMetadata: { version: 5, grader: publicMcpGraderVersion, workflowDigest: publicMcpWorkflowDigest, workerSkillDigest: publicMcpWorkerSkillDigest, workerInstructions: publicMcpWorkerInstructions, assistantMaxRequests: 16, assistantMaxEstimatedUsd: 2, instructions: "shipped-plugin-skills", scheduling: "explicit-only" },
|
||||
},
|
||||
{
|
||||
id: "blocker-guidance", label: "Direct blocker handling", manualOnly: true,
|
||||
|
||||
@@ -69,4 +69,15 @@ describe("external assistant evidence and billing", () => {
|
||||
expect(snapshots).toEqual([{ calls: 0, requests: 1 }, { calls: 1, requests: 1 }, { calls: 1, requests: 1 }]);
|
||||
expect(usage.estimatedCostUsd).toBeGreaterThan(0);
|
||||
});
|
||||
it("shares the 16-request ceiling across fresh conversations in one cell", async () => {
|
||||
const fetchMock = vi.fn().mockImplementation(async () => Response.json({ model: "gpt-5.4-mini", usage: { input_tokens: 10, output_tokens: 10 }, output: [{ type: "message", content: [{ type: "output_text", text: "Done." }] }] }));
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
const usage = assistantUsage("openai", "gpt-5.4-mini");
|
||||
const input = { usage, credential: "unit-test-key", prompt: "Fresh conversation", tools: [], call: vi.fn(), deadlineAt: Date.now() + 10_000, observe: vi.fn() };
|
||||
for (let i = 0; i < 16; i++) await runAssistant(input);
|
||||
await expect(runAssistant(input)).rejects.toThrow("shared 16-request per-cell budget");
|
||||
expect(fetchMock).toHaveBeenCalledTimes(16);
|
||||
expect(usage.requests).toBe(16);
|
||||
});
|
||||
|
||||
});
|
||||
@@ -50,6 +50,7 @@ export async function runAssistant(input: {
|
||||
const responseItems: any[] = [{ role: "user", content: input.prompt }];
|
||||
try {
|
||||
for (let step = 0; step < 16; step++) {
|
||||
if (input.usage.requests >= 16) throw new Error("Assistant exceeded its shared 16-request per-cell budget");
|
||||
if (Date.now() >= input.deadlineAt) throw new Error("Assistant workflow exceeded its bounded deadline");
|
||||
const anthropic = input.usage.provider === "anthropic";
|
||||
const response = await fetch(anthropic ? "https://api.anthropic.com/v1/messages" : "https://api.openai.com/v1/responses", {
|
||||
|
||||
Reference in new issue
Block a user