mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 12:04:41 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents receive reviewed actions discovered from their connections. > - Transport, discovery, risk classification, publication and account inventory lived in the same service. > - Their ownership differs: transport guards requests, discovery reads providers, and publication changes the catalog under locks. > - This pull request gives those phases separate modules with explicit capabilities. > - The benefit is a visible discovery-to-publication path that preserves permission and quarantine behavior. ## Linked Issues or Issue Description Builds on merged #17162 and preserves the connector policy from #17141. Earlier stack: #17144 → #17146 → #17147 → #17155 → #17157 → #17159 → #17161 → #17162 → this PR. **What existing behavior does this improve?** Maintenance of tool discovery, catalog publication, health and upstream account inventory. **Current behavior** The 8,988-line service handles HTTP guards, MCP sessions, provider probes, risk classification, quarantine, cached reads and account sync. **Proposed behavior** Discovery returns descriptors through the existing credential and transport owners. Publication owns catalog writes and default access. A reader owns refresh deduplication. Health and aggregator inventory have separate owners. **Reason and benefit** The entry point falls to 6,419 lines, a reduction of 2,569. Provider discovery no longer shares a closure with connection setup and teardown. ## What Changed - Extract guarded remote HTTP, descriptor policy, discovery, publication, default access, cached reads, health, attention and aggregator account sync. - Keep the public descriptor/risk exports and existing service methods. - Preserve database locks, transaction-scoped default-access writes, quarantine decisions, current grant checks and failure classification. - Preserve per-service catalog flights and aggregator jobs, lease ownership and live remote-request options. - New modules range from 39 to 492 lines. ## Verification - Before and after the move, 470 public service/HTTP integration cases and 44 publication, HTTP rebinding and aggregator sync cases pass. - Server typecheck, module-boundary gate and whitespace checks pass. - All 157 pre-existing function/method bodies match by normalized syntax comparison. - Before merge integration, the completed stack passed 525 targeted cases, repository-wide typecheck and build. Its stronger rotating-token fixture is included. - After replaying onto master, the integrated stack passes 601 cases across nine suites, including Braintrust lifecycle and connector-policy coverage. Server typecheck and module-boundary checks pass. All 157 moved bodies still match the current master versions. - The earlier local full-suite run was interrupted after 79 minutes and remains unverified. All current-head CI gates pass on `a21595f20809a514b7619d8a8748c56f8e546e2f`, including the full sharded test matrix, browser suites, typecheck, build and canary dry run. Greptile is 5/5 with no open review threads after merge integration. ## Risks - Discovery must keep its current grant and session behavior; publication must retain its locks and quarantine rules. Existing authority, schema-drift and publication-race tests exercise these paths. - Aggregator account observations must not become authorization. Lease, freshness, failure and credential-fingerprint behavior stay unchanged. - Live transport options must not be snapshotted. The HTTP owner retains the same options object. - No schema, public API, access-policy or scheduling change is intended. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. 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>
130 lines
4.7 KiB
TypeScript
130 lines
4.7 KiB
TypeScript
import { assertConfiguredTokenBrokerEndpointsAllowed } from "./token-broker/endpoint-policy.js";
|
|
import { remoteEndpoint } from "./connection-methods.js";
|
|
import type { DeploymentExposure, DeploymentMode } from "@paperclipai/shared";
|
|
import { badRequest, HttpError } from "../../errors.js";
|
|
import { assertPublicRemoteHttpEndpoint, parseRemoteHttpEndpoint, type RemoteHttpEndpointLookup } from "../remote-http-endpoint-guard.js";
|
|
import { guardedRemoteHttpFetch, type GuardedRemoteHttpFetchOptions } from "../remote-http-fetch.js";
|
|
|
|
const REMOTE_HTTP_REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]);
|
|
|
|
const MAX_REMOTE_HTTP_REDIRECTS = 5;
|
|
|
|
type RemoteHttpOptions = {
|
|
deploymentMode?: DeploymentMode;
|
|
deploymentExposure?: DeploymentExposure;
|
|
remoteHttpEndpointLookup?: RemoteHttpEndpointLookup;
|
|
remoteHttpRequest?: (url: string, init: RequestInit) => Promise<Response>;
|
|
};
|
|
|
|
export function createToolRemoteHttp(options: RemoteHttpOptions) {
|
|
function allowPrivateRemoteEndpoints() {
|
|
return (
|
|
options.deploymentMode !== "authenticated" ||
|
|
options.deploymentExposure !== "public"
|
|
);
|
|
}
|
|
|
|
async function assertRemoteHttpUrlAllowed(value: string): Promise<string> {
|
|
const endpoint = parseRemoteHttpEndpoint(value, (message, code) =>
|
|
badRequest(message, { code }),
|
|
);
|
|
await assertPublicRemoteHttpEndpoint(
|
|
endpoint,
|
|
{
|
|
allowPrivateNetwork: allowPrivateRemoteEndpoints(),
|
|
lookup: options.remoteHttpEndpointLookup,
|
|
},
|
|
(message, code) => badRequest(message, { code }),
|
|
);
|
|
return endpoint.toString();
|
|
}
|
|
|
|
function remoteHttpFetchOptions(): GuardedRemoteHttpFetchOptions {
|
|
return {
|
|
allowPrivateNetwork: allowPrivateRemoteEndpoints(),
|
|
lookup: options.remoteHttpEndpointLookup,
|
|
error: (message, code) => badRequest(message, { code }),
|
|
};
|
|
}
|
|
|
|
async function requestRemoteHttpEndpoint(
|
|
endpoint: URL,
|
|
init: RequestInit,
|
|
): Promise<Response> {
|
|
return options.remoteHttpRequest
|
|
? options.remoteHttpRequest(endpoint.toString(), {
|
|
...init,
|
|
redirect: "manual",
|
|
})
|
|
: guardedRemoteHttpFetch(endpoint, init, remoteHttpFetchOptions());
|
|
}
|
|
|
|
/**
|
|
* Fetch an operator-supplied remote URL with the egress guard bound to the
|
|
* connection itself.
|
|
*
|
|
* `guardedRemoteHttpFetch` resolves the hostname once and dials the approved
|
|
* address, so a name server that answers public-then-private cannot move the
|
|
* connection onto a loopback or metadata address after validation
|
|
* (PAP-17098). Redirects stay manual and run the full guard again on the next
|
|
* hop, because a `Location` is just as attacker-controlled as the first URL.
|
|
*/
|
|
async function fetchRemoteHttpUrl(
|
|
value: string,
|
|
init: RequestInit = {},
|
|
): Promise<Response> {
|
|
let currentUrl = value;
|
|
const method = (init.method ?? "GET").toUpperCase();
|
|
for (
|
|
let redirectCount = 0;
|
|
redirectCount <= MAX_REMOTE_HTTP_REDIRECTS;
|
|
redirectCount += 1
|
|
) {
|
|
const endpoint = parseRemoteHttpEndpoint(currentUrl, (message, code) =>
|
|
badRequest(message, { code }),
|
|
);
|
|
const response = await requestRemoteHttpEndpoint(endpoint, init);
|
|
const location = REMOTE_HTTP_REDIRECT_STATUSES.has(response.status)
|
|
? (response.headers?.get?.("location") ?? null)
|
|
: null;
|
|
if (!location) return response;
|
|
if (method !== "GET" && method !== "HEAD") {
|
|
throw new HttpError(
|
|
502,
|
|
"Remote OAuth endpoint redirected unexpectedly",
|
|
{ code: "oauth_redirect_rejected" },
|
|
);
|
|
}
|
|
if (redirectCount >= MAX_REMOTE_HTTP_REDIRECTS) {
|
|
throw new HttpError(
|
|
502,
|
|
"Remote OAuth endpoint redirected too many times",
|
|
{ code: "oauth_redirect_limit" },
|
|
);
|
|
}
|
|
currentUrl = new URL(location, endpoint).toString();
|
|
}
|
|
throw new HttpError(
|
|
502,
|
|
"Remote OAuth endpoint redirected too many times",
|
|
{ code: "oauth_redirect_limit" },
|
|
);
|
|
}
|
|
|
|
async function assertRemoteEndpointAllowed(
|
|
config: Record<string, unknown>,
|
|
): Promise<string> {
|
|
return assertRemoteHttpUrlAllowed(remoteEndpoint(config));
|
|
}
|
|
|
|
async function assertRemoteConnectionEndpointsAllowed(
|
|
config: Record<string, unknown>,
|
|
): Promise<string> {
|
|
const endpoint = await assertRemoteEndpointAllowed(config);
|
|
await assertConfiguredTokenBrokerEndpointsAllowed(config);
|
|
return endpoint;
|
|
}
|
|
return { allowPrivateRemoteEndpoints, assertRemoteHttpUrlAllowed, requestRemoteHttpEndpoint, fetchRemoteHttpUrl, assertRemoteEndpointAllowed, assertRemoteConnectionEndpointsAllowed };
|
|
}
|
|
export type ToolRemoteHttp = ReturnType<typeof createToolRemoteHttp>;
|