Files
PaperClipAI/server/src/services/tool-access/oauth/client-metadata.ts
T
DottaandPaperclip 4413bb262f refactor(tools): isolate OAuth protocol boundaries (#17147)
## 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>
2026-10-21 22:21:41 -05:00

81 lines
2.9 KiB
TypeScript

import { assertPublicRemoteHttpEndpoint, type RemoteHttpEndpointLookup } from "../../remote-http-endpoint-guard.js";
/**
* Where this deployment publishes its Client ID Metadata Document. The document's
* own URL is the `client_id` Paperclip presents, so this path is a stable part of
* the deployment's public contract with every authorization server that has seen
* it — changing it invalidates existing CIMD registrations.
*/
export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_PATH =
"/api/tools/oauth/client-metadata";
/**
* Resolve the URL Paperclip would use as a CIMD client id, but only when its
* hostname is not known to resolve into a private network.
*
* An authorization server fetches this URL from outside Paperclip's network and
* will normally apply an SSRF guard. Tailscale/MagicDNS names are HTTPS but
* resolve into 100.64.0.0/10, so presenting one as a client id can only produce
* an `invalid_client` response. A local DNS failure remains inconclusive because
* split-horizon public DNS may still let the authorization server resolve it.
*/
export async function resolveOAuthClientIdMetadataDocumentUrl(
redirectUri: string,
lookup?: RemoteHttpEndpointLookup,
): Promise<string | null> {
try {
const parsed = new URL(redirectUri);
if (parsed.protocol !== "https:") return null;
const hostname = parsed.hostname.replace(/^\[|\]$/g, "").toLowerCase();
const isLoopback =
hostname === "localhost" ||
hostname.endsWith(".localhost") ||
hostname === "::1" ||
/^127(?:\.\d{1,3}){3}$/.test(hostname);
if (isLoopback) return null;
const metadataUrl = new URL(
OAUTH_CLIENT_ID_METADATA_DOCUMENT_PATH,
parsed.origin,
).toString();
try {
await assertPublicRemoteHttpEndpoint(
new URL(metadataUrl),
{ allowPrivateNetwork: false, lookup },
(message, code) => Object.assign(new Error(message), { code }),
);
} catch (error) {
if (
error instanceof Error &&
"code" in error &&
error.code === "remote_http_private_endpoint"
) {
return null;
}
}
return metadataUrl;
} catch {
return null;
}
}
/**
* Paperclip's client metadata for CIMD (RFC 7591 metadata, served rather than
* registered). Only the callback for this deployment appears in it, so an
* authorization server that fetches it can see exactly one legal redirect target.
*/
export function oauthClientIdMetadataDocument(input: {
clientId: string;
redirectUri: string;
}): Record<string, unknown> {
return {
client_id: input.clientId,
client_name: `Paperclip (${new URL(input.redirectUri).host})`,
client_uri: new URL("/", input.clientId).toString(),
redirect_uris: [input.redirectUri],
grant_types: ["authorization_code", "refresh_token"],
response_types: ["code"],
token_endpoint_auth_method: "none",
application_type: "web",
};
}