mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
## Thinking Path > - Paperclip manages AI agents and their access to external tools. > - Tool connections use OAuth to obtain credentials from remote providers. > - The tool-access service mixed provider protocol parsing with database and secret writes. > - This made protocol changes hard to review without reading the full coordinator. > - This PR gives discovery, endpoint policy, and client exchange explicit owners. > - The coordinator keeps authorization, leases, persistence, and audit decisions. > - A real local provider fixture checks the boundary through public routes. ## Linked Issues or Issue Description **What existing behavior does this improve?** The ownership of OAuth discovery, registration, token exchange, and client metadata. **Current behavior** The tool-access service contains over 20,000 lines. Provider responses, endpoint policy, and stateful operations share the same closure. **Proposed behavior** Small protocol modules accept settings and named request capabilities. They return normalized results. The service owns durable state and existing public exports. This is part 3 of the stack: #17144, then #17146, then this PR. It targets #17146's branch. Review only this PR's diff. ## What Changed - Extract endpoint policy, metadata parsing/discovery, registration/token exchange, provider error normalization, and the public client metadata document. - Validate remote registration responses in the protocol module. Return client material to the coordinator for secret and connection writes. - Remove the unused raw provider response from the internal token result. Keep every consumed field. - Preserve process-wide registration single-flight, service-local maps, refresh leases, membership checks, callback checks, and transaction ownership. - Reuse connection-method URL normalization. Keep the endpoint setting and clock callbacks live. - Add a real local HTTP OAuth/MCP route fixture for public and Basic-authenticated clients. - Reduce `tool-access.ts` from 20,917 lines / 808,777 bytes to 19,293 lines / 749,158 bytes. The seven OAuth modules total 1,697 lines; the largest has 582 lines. The stack removes 2,232 lines from the original service. - Record the next profile-resolution and token-broker boundaries in the dated plan. ## Verification - Baseline: 466 service cases and 80 protocol/provider-policy cases pass. The new two-case HTTP fixture passes before moving production code. - After: `pnpm exec vitest run server/src/__tests__/tool-access-oauth-protocol.integration.test.ts server/src/__tests__/tool-access-service.test.ts server/src/__tests__/remote-mcp-connectors.test.ts server/src/__tests__/remote-http-rebinding.test.ts server/src/__tests__/oauth-client-metadata.test.ts` — 508 tests pass with one worker and disposable PostgreSQL. - The fixture drives connect, metadata discovery, DCR, PKCE code exchange, callback, catalog review, and finish. It checks persisted active state, secret references, issuer/auth-method binding, and wire-level authentication. - `pnpm -r --workspace-concurrency=2 typecheck` passes. The sandbox-only attempt could not open a local IPC socket; the same command passed with local process permissions. - `node scripts/check-module-boundaries.mjs` and `git diff --check` pass. - AST comparison confirms 37 moved bodies are unchanged. Registration protocol and persistence statements match their original halves. Token exchange only drops its unused raw result. Preflight uses explicit defaults and a clock callback. - Initial fixture authoring used a nonexistent catalog enabled field; the corrected assertion checks persisted status and reviewer identity. An initial typecheck found optional defaults at the new boundary; the caller now passes its already-validated defaults. - Final-stack local verification: `pnpm -r --workspace-concurrency=2 typecheck` and `pnpm build` pass. Remote CI is green at `3dc46c132408556ecb31038a9b9621ac20aacefc`. Greptile is 5/5 with no actionable findings or open inline threads. - Full local-suite limitation: `pnpm test:run` was interrupted after 79 minutes during the serial general-server phase, after CI had completed the full test matrix. Before interruption, it flagged the unchanged `company-search-service.test.ts` suite (26 database cases skipped). One isolated rerun passed all 28 cases without code changes. The original log and a pre-interruption copy are preserved. The original failure cause was not reproduced. The full local suite and its later groups are not claimed as passed; the complete CI matrix is green on this stack. ## Risks - OAuth endpoint checks and issuer/resource binding are security boundaries. Existing denial, redirect, scope, DCR, secret-redaction, and refresh tests cover the preserved paths. - A moved closure could capture stale policy or split registration/refresh ownership. The new modules hold no mutable state. Policy is read through the existing callback, and state stays in the service. - The local fixture proves real HTTP and database behavior. It does not claim a live third-party provider sign-in. - No schema, API, authorization policy, or persisted-data migration changes are intended. ## Model Used OpenAI Codex, GPT-6 family, with code execution and repository tools. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
369 lines
13 KiB
TypeScript
369 lines
13 KiB
TypeScript
import { parseRemoteHttpEndpoint } from "../remote-http-endpoint-guard.js";
|
|
import type { AppDefinition, ConnectionMethodDef, ToolConnectionTransport } from "@paperclipai/shared";
|
|
import {
|
|
credentialConfigPath,
|
|
getAvailableConnectionMethod,
|
|
getAvailableConnectionMethods,
|
|
getConnectableAppDefinition,
|
|
resolveConnectionMethodServerUrl,
|
|
checkMcpRemoteHeaderName,
|
|
checkMcpRemoteHeaderValue,
|
|
mcpRemoteHeaderRejectionMessage,
|
|
} from "@paperclipai/shared";
|
|
import { badRequest, unprocessable } from "../../errors.js";
|
|
import { isPaperclipCloudConnectorStrategy } from "../paperclip-cloud-connector.js";
|
|
import { honchoManagedArguments } from "../honcho-connection.js";
|
|
import { assertGoogleChatToolArgumentsSupported, googleChatToolInputSchema } from "../google-chat-tool-policy.js";
|
|
import { gmailArchiveToolInputSchema } from "../gmail-archive-tool-policy.js";
|
|
|
|
type ConnectionSettings = {
|
|
config: Record<string, unknown>;
|
|
transport: ToolConnectionTransport;
|
|
transportConfig?: Record<string, unknown>;
|
|
};
|
|
|
|
function asRecord(value: unknown): Record<string, unknown> {
|
|
if (value && typeof value === "object" && !Array.isArray(value))
|
|
return value as Record<string, unknown>;
|
|
return {};
|
|
}
|
|
|
|
export function connectionMethodFor(app: AppDefinition, methodKey?: string | null) {
|
|
const normalizedMethodKey =
|
|
app.slug === "gmail" && methodKey === "paperclip-id-oauth"
|
|
? "paperclip-draft"
|
|
: methodKey;
|
|
// Stored managed connections must remain recognizable for callback, refresh,
|
|
// and revoke even though static definitions omit instance availability. New
|
|
// setup passes a definition filtered by signed profiles before reaching here;
|
|
// the broker independently enforces availability on authorization and refresh.
|
|
const availableMethods = new Set(getAvailableConnectionMethods(app));
|
|
const toolMethods = app.methods.filter(
|
|
(candidate) =>
|
|
candidate.purpose !== "channel" && candidate.purpose !== "ai"
|
|
&& candidate.transport !== "chat_sdk" && candidate.transport !== "runtime_auth"
|
|
&& (availableMethods.has(candidate) || isPaperclipCloudConnectorStrategy(candidate.oauthStrategy)),
|
|
);
|
|
const method = normalizedMethodKey
|
|
? (toolMethods.find((candidate) => candidate.key === normalizedMethodKey) ??
|
|
null)
|
|
: getAvailableConnectionMethod({ ...app, methods: toolMethods }, null);
|
|
if (!method)
|
|
throw unprocessable(
|
|
"This app does not have an available connection method",
|
|
);
|
|
return method;
|
|
}
|
|
|
|
export function connectionMethodForConnection(
|
|
app: AppDefinition,
|
|
connection: ConnectionSettings,
|
|
) {
|
|
const methodKey =
|
|
typeof connection.config.connectionMethodKey === "string"
|
|
? connection.config.connectionMethodKey
|
|
: null;
|
|
return connectionMethodFor(app, methodKey);
|
|
}
|
|
|
|
export function credentialFieldsFor(app: AppDefinition, methodKey?: string | null) {
|
|
const method = connectionMethodFor(app, methodKey);
|
|
return (method.credentialFields ?? []).map((field) => ({
|
|
label: field.label,
|
|
configPath: credentialConfigPath(field, method),
|
|
helpUrl: method.consoleLinks?.keys ?? method.consoleLinks?.docs ?? "",
|
|
required: field.required,
|
|
placement:
|
|
method.keyPlacement?.location === "header"
|
|
? ("header" as const)
|
|
: undefined,
|
|
key: method.keyPlacement?.name,
|
|
prefix: method.keyPlacement?.prefix,
|
|
}));
|
|
}
|
|
|
|
|
|
export function normalizeConnectionMethodConfig(
|
|
method: ConnectionMethodDef,
|
|
configValues: Record<string, unknown> | undefined,
|
|
): {
|
|
values: Record<string, string | boolean>;
|
|
url?: string;
|
|
headers?: Record<string, string>;
|
|
} {
|
|
const fields = [
|
|
...(method.tenantFields ?? []),
|
|
...(method.extensionFields ?? []),
|
|
];
|
|
const allowedKeys = new Set(fields.map((field) => field.key));
|
|
for (const key of Object.keys(configValues ?? {})) {
|
|
if (!allowedKeys.has(key))
|
|
throw badRequest(`Unknown connection setting: ${key}`);
|
|
}
|
|
|
|
const values: Record<string, string | boolean> = {};
|
|
for (const field of fields) {
|
|
const raw = configValues?.[field.key] ?? field.defaultValue;
|
|
if (field.type === "checkbox") {
|
|
if (raw !== undefined && typeof raw !== "boolean")
|
|
throw badRequest(`${field.label} must be true or false`);
|
|
if (raw !== undefined) values[field.key] = raw;
|
|
continue;
|
|
}
|
|
if (raw !== undefined && typeof raw !== "string")
|
|
throw badRequest(`${field.label} must be text`);
|
|
let value = raw?.trim() ?? "";
|
|
if (field.transport?.format === "csv") {
|
|
value = Array.from(
|
|
new Set(
|
|
value
|
|
.split(/[\n,]/g)
|
|
.map((entry) => entry.trim())
|
|
.filter(Boolean),
|
|
),
|
|
).join(",");
|
|
}
|
|
if (field.required && !value)
|
|
throw badRequest(`Missing connection setting: ${field.label}`);
|
|
if (!value) continue;
|
|
if (
|
|
field.validation?.maxLength &&
|
|
value.length > field.validation.maxLength
|
|
) {
|
|
throw badRequest(
|
|
`${field.label} must be at most ${field.validation.maxLength} characters`,
|
|
);
|
|
}
|
|
if (
|
|
field.validation?.pattern &&
|
|
!new RegExp(field.validation.pattern).test(value)
|
|
) {
|
|
throw badRequest(`${field.label} has an invalid value`);
|
|
}
|
|
if (
|
|
field.type === "select" &&
|
|
!field.options?.some((option) => option.value === value)
|
|
) {
|
|
throw badRequest(`${field.label} has an invalid option`);
|
|
}
|
|
values[field.key] = value;
|
|
}
|
|
for (const keys of method.configRequirements?.atLeastOneOf
|
|
? [method.configRequirements.atLeastOneOf]
|
|
: []) {
|
|
if (
|
|
!keys.some(
|
|
(key) => typeof values[key] === "string" && values[key].length > 0,
|
|
)
|
|
) {
|
|
throw badRequest(`Provide at least one of: ${keys.join(", ")}`);
|
|
}
|
|
}
|
|
|
|
const resolvedServerUrl = resolveConnectionMethodServerUrl(method, values);
|
|
if (method.defaults?.serverUrlTemplate && !resolvedServerUrl) {
|
|
throw badRequest(
|
|
"Missing or invalid connection settings for the server URL",
|
|
);
|
|
}
|
|
const endpoint = resolvedServerUrl ? new URL(resolvedServerUrl) : null;
|
|
if (endpoint && endpoint.protocol !== "https:") {
|
|
throw badRequest("Connection server URL must use HTTPS");
|
|
}
|
|
const headers: Record<string, string> = {};
|
|
for (const field of fields) {
|
|
const transport = field.transport;
|
|
const value = values[field.key];
|
|
if (
|
|
!transport ||
|
|
value === undefined ||
|
|
(value === false && transport.omitFalse)
|
|
)
|
|
continue;
|
|
const serialized = typeof value === "boolean" ? String(value) : value;
|
|
if (transport.location === "query")
|
|
endpoint?.searchParams.set(transport.name, serialized);
|
|
else {
|
|
const nameCheck = checkMcpRemoteHeaderName(transport.name);
|
|
const valueCheck = checkMcpRemoteHeaderValue(serialized);
|
|
if (!nameCheck.ok || !valueCheck.ok) {
|
|
throw badRequest(
|
|
mcpRemoteHeaderRejectionMessage(
|
|
transport.name,
|
|
nameCheck.reason ?? valueCheck.reason!,
|
|
),
|
|
);
|
|
}
|
|
headers[transport.name] = serialized;
|
|
}
|
|
}
|
|
return {
|
|
values,
|
|
...(endpoint ? { url: endpoint.toString() } : {}),
|
|
...(Object.keys(headers).length > 0 ? { headers } : {}),
|
|
};
|
|
}
|
|
|
|
export function projectedConnectionHeaders(
|
|
connection: ConnectionSettings,
|
|
): Record<string, string> {
|
|
const sourceTemplateKey =
|
|
typeof connection.config.sourceTemplateKey === "string"
|
|
? connection.config.sourceTemplateKey
|
|
: null;
|
|
const app = sourceTemplateKey
|
|
? getConnectableAppDefinition(sourceTemplateKey)
|
|
: null;
|
|
const headers: Record<string, string> = {};
|
|
if (app) {
|
|
const method = connectionMethodForConnection(app, connection);
|
|
// Non-transport settings do not participate in header projection. In
|
|
// particular, legacy connections can lack a newly introduced setting.
|
|
if ([...(method.tenantFields ?? []), ...(method.extensionFields ?? [])].some((field) => field.transport?.location === "header")) {
|
|
Object.assign(headers, normalizeConnectionMethodConfig(
|
|
method,
|
|
asRecord(connection.config.methodConfig),
|
|
).headers);
|
|
}
|
|
}
|
|
if (
|
|
connection.transport === "mcp_remote" &&
|
|
(sourceTemplateKey === "github" || connection.transportConfig?.sourceTemplateKey === "github")
|
|
) {
|
|
// GitHub excludes Actions from its default catalog. Project this on every
|
|
// discovery and invocation so existing connections gain the toolset too.
|
|
headers["X-MCP-Toolsets"] = "default,actions";
|
|
}
|
|
return headers;
|
|
}
|
|
|
|
function mergeManagedToolArguments(
|
|
supplied: Record<string, unknown>,
|
|
managed: Record<string, unknown>,
|
|
): Record<string, unknown> {
|
|
const merged = { ...supplied };
|
|
for (const [key, value] of Object.entries(managed)) {
|
|
const suppliedValue = merged[key];
|
|
merged[key] =
|
|
asRecord(value) === value && asRecord(suppliedValue) === suppliedValue
|
|
? mergeManagedToolArguments(
|
|
suppliedValue as Record<string, unknown>,
|
|
value as Record<string, unknown>,
|
|
)
|
|
: value;
|
|
}
|
|
return merged;
|
|
}
|
|
|
|
export function projectConnectionMethodToolArguments(
|
|
method: ConnectionMethodDef,
|
|
parameters: unknown,
|
|
): Record<string, unknown> {
|
|
const supplied = asRecord(parameters);
|
|
const managed = method.defaults?.toolArgumentDefaults;
|
|
return managed ? mergeManagedToolArguments(supplied, managed) : supplied;
|
|
}
|
|
|
|
function stripManagedToolArgumentSchema(
|
|
schema: Record<string, unknown>,
|
|
managed: Record<string, unknown>,
|
|
): Record<string, unknown> {
|
|
const properties = asRecord(schema.properties);
|
|
if (Object.keys(properties).length === 0) return schema;
|
|
const nextProperties = { ...properties };
|
|
for (const [key, managedValue] of Object.entries(managed)) {
|
|
const propertySchema = asRecord(nextProperties[key]);
|
|
const managedRecord = asRecord(managedValue);
|
|
if (
|
|
Object.keys(propertySchema).length === 0 ||
|
|
Object.keys(managedRecord).length === 0
|
|
) {
|
|
delete nextProperties[key];
|
|
continue;
|
|
}
|
|
const projectedProperty = stripManagedToolArgumentSchema(
|
|
propertySchema,
|
|
managedRecord,
|
|
);
|
|
if (Object.keys(asRecord(projectedProperty.properties)).length === 0)
|
|
delete nextProperties[key];
|
|
else nextProperties[key] = projectedProperty;
|
|
}
|
|
const nextSchema: Record<string, unknown> = {
|
|
...schema,
|
|
properties: nextProperties,
|
|
};
|
|
if (Array.isArray(schema.required)) {
|
|
const required = schema.required.filter(
|
|
(key): key is string => typeof key === "string" && key in nextProperties,
|
|
);
|
|
if (required.length > 0) nextSchema.required = required;
|
|
else delete nextSchema.required;
|
|
}
|
|
return nextSchema;
|
|
}
|
|
|
|
export function projectConnectionMethodToolInputSchema(
|
|
method: ConnectionMethodDef,
|
|
inputSchema: Record<string, unknown>,
|
|
): Record<string, unknown> {
|
|
const managed = method.defaults?.toolArgumentDefaults;
|
|
return managed
|
|
? stripManagedToolArgumentSchema(inputSchema, managed)
|
|
: inputSchema;
|
|
}
|
|
|
|
export function projectedConnectionToolArguments(
|
|
connection: ConnectionSettings,
|
|
parameters: unknown,
|
|
toolName: string,
|
|
inputSchema: Record<string, unknown> = {},
|
|
): Record<string, unknown> {
|
|
const managed = honchoManagedArguments(connection, inputSchema);
|
|
if (managed) parameters = mergeManagedToolArguments(asRecord(parameters), managed);
|
|
assertGoogleChatToolArgumentsSupported(connection, toolName, parameters);
|
|
const sourceTemplateKey =
|
|
typeof connection.config.sourceTemplateKey === "string"
|
|
? connection.config.sourceTemplateKey
|
|
: null;
|
|
const app = sourceTemplateKey
|
|
? getConnectableAppDefinition(sourceTemplateKey)
|
|
: null;
|
|
if (!app) return asRecord(parameters);
|
|
return projectConnectionMethodToolArguments(
|
|
connectionMethodForConnection(app, connection),
|
|
parameters,
|
|
);
|
|
}
|
|
|
|
export function projectedConnectionToolInputSchema(
|
|
connection: ConnectionSettings,
|
|
inputSchema: Record<string, unknown>,
|
|
toolName: string,
|
|
): Record<string, unknown> {
|
|
const managed = honchoManagedArguments(connection, inputSchema);
|
|
if (managed) inputSchema = stripManagedToolArgumentSchema(inputSchema, managed);
|
|
inputSchema = googleChatToolInputSchema(connection, toolName, inputSchema);
|
|
inputSchema = gmailArchiveToolInputSchema(connection, toolName, inputSchema);
|
|
const sourceTemplateKey =
|
|
typeof connection.config.sourceTemplateKey === "string"
|
|
? connection.config.sourceTemplateKey
|
|
: null;
|
|
const app = sourceTemplateKey
|
|
? getConnectableAppDefinition(sourceTemplateKey)
|
|
: null;
|
|
if (!app) return inputSchema;
|
|
return projectConnectionMethodToolInputSchema(
|
|
connectionMethodForConnection(app, connection),
|
|
inputSchema,
|
|
);
|
|
}
|
|
|
|
export function remoteEndpoint(config: Record<string, unknown>): string {
|
|
const value = config.url ?? config.endpoint ?? config.remoteUrl;
|
|
const parsed = parseRemoteHttpEndpoint(value, (message, code) =>
|
|
badRequest(message, { code }),
|
|
);
|
|
return parsed.toString();
|
|
}
|