mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 02:00:52 +01:00
fix(security): guard connection token brokers
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
bfc8fd81af
commit
b0515d65a2
4 files changed
+255
-11
No files matched your search
@@ -924,6 +924,22 @@ pnpm secrets:migrate-inline-env # dry run
|
||||
pnpm secrets:migrate-inline-env --apply # apply migration
|
||||
```
|
||||
|
||||
## Internal Connection Token Brokers
|
||||
|
||||
Connection token exchanges carry a resolved parent credential, so their URLs
|
||||
are public-only by default even in local/private deployments. To intentionally
|
||||
use an internal broker, configure an exact hostname allowlist:
|
||||
|
||||
```sh
|
||||
PAPERCLIP_TOKEN_BROKER_ALLOWED_HOSTS=broker.internal.example,10.0.0.42
|
||||
```
|
||||
|
||||
Entries are comma- or whitespace-separated exact hostnames (no wildcards). The
|
||||
host configured by `PAPERCLIP_PAGES_API_URL` is included automatically. Every
|
||||
broker hostname is resolved once and the request is pinned to the approved
|
||||
address; IPv4 and IPv6 link-local destinations remain denied even when their
|
||||
host is allowlisted.
|
||||
|
||||
## Company Deletion Toggle
|
||||
|
||||
Company deletion is intended as a dev/debug capability and can be disabled at runtime:
|
||||
|
||||
@@ -111,6 +111,10 @@ Required controls:
|
||||
first, then persist only refs and redacted account metadata on the connection.
|
||||
- Create/update routes validate same-company ownership for every referenced
|
||||
secret, app, connection, agent, user, project, routine, and issue.
|
||||
- Create/update routes validate every configured token-broker exchange URL
|
||||
against the private-host policy before persisting the connection. Minting
|
||||
repeats the check and pins the approved address before transmitting the
|
||||
parent credential, covering legacy rows and DNS rebinding.
|
||||
- Health and auth failures transition failure-closed: `missing_secret`,
|
||||
`degraded`, `failed`, `auth_required`, or disabled equivalents.
|
||||
- Error payloads and logs redact provider responses that may contain credentials.
|
||||
|
||||
@@ -254,6 +254,7 @@ async function createBrokerConnection(
|
||||
rateLimitPerHour?: number;
|
||||
healthStatus?: "unknown" | "healthy" | "degraded" | "failed" | "unchecked" | "ok" | "error" | "missing_secret";
|
||||
tokenUrl?: string;
|
||||
protocol?: "pages" | "generic" | "rfc8693";
|
||||
} = {},
|
||||
) {
|
||||
const secret = await secretService(db).create(companyId, {
|
||||
@@ -284,7 +285,8 @@ async function createBrokerConnection(
|
||||
tokenBroker: {
|
||||
enabled: true,
|
||||
path: input.path ?? "exchange",
|
||||
tokenUrl: input.tokenUrl ?? "https://pages.example.test/v1/tokens/exchange",
|
||||
tokenUrl: input.tokenUrl ?? "https://93.184.216.34/v1/tokens/exchange",
|
||||
...(input.protocol ? { protocol: input.protocol } : {}),
|
||||
parentCredentialConfigPath: "credentials.deploy_token",
|
||||
parentScopes: input.parentScopes ?? ["pages:publish:ns/dotta"],
|
||||
defaultScopes: input.defaultScopes ?? [],
|
||||
@@ -475,7 +477,7 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
const app = createRouteApp(db, agentJwtActor(company.id, agent.id, run.id));
|
||||
|
||||
const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => {
|
||||
expect(String(url)).toBe("https://pages.example.test/v1/tokens/exchange");
|
||||
expect(String(url)).toBe("https://93.184.216.34/v1/tokens/exchange");
|
||||
expect(init?.headers).toEqual(expect.objectContaining({ authorization: "Bearer parent-deploy-token" }));
|
||||
const body = JSON.parse(String(init?.body));
|
||||
expect(body).toMatchObject({
|
||||
@@ -543,6 +545,76 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
]));
|
||||
});
|
||||
|
||||
it.each([
|
||||
["generic", undefined],
|
||||
["RFC 8693", "rfc8693" as const],
|
||||
])("blocks a link-local %s token broker before credentials reach fetch", async (_label, protocol) => {
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { run } = await createIssueAndRun(db, company.id, agent.id);
|
||||
const { connection } = await createBrokerConnection(db, company.id, {
|
||||
tokenUrl: "http://169.254.169.254/latest/meta-data",
|
||||
...(protocol ? { protocol } : {}),
|
||||
});
|
||||
await allowConnectionForAgent(db, company.id, agent.id, connection.id);
|
||||
const app = createRouteApp(db, agentJwtActor(company.id, agent.id, run.id));
|
||||
const fetchMock = vi.spyOn(globalThis, "fetch").mockRejectedValue(
|
||||
new Error("the parent credential must never reach the broker"),
|
||||
);
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/agents/me/connections/${connection.id}/token`)
|
||||
.send({ scope: "pages:publish:ns/dotta" });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toMatchObject({ code: "remote_http_private_endpoint" });
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
const [issuance] = await db.select().from(connectionTokenIssuances);
|
||||
expect(issuance).toMatchObject({
|
||||
connectionId: connection.id,
|
||||
outcome: "failure",
|
||||
errorCode: "remote_http_private_endpoint",
|
||||
tokenHash: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("allows an explicitly allowlisted internal token broker through the guarded fetch", async () => {
|
||||
vi.stubEnv("PAPERCLIP_TOKEN_BROKER_ALLOWED_HOSTS", "broker.example, 127.0.0.1");
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
const { run } = await createIssueAndRun(db, company.id, agent.id);
|
||||
const { connection } = await createBrokerConnection(db, company.id, {
|
||||
tokenUrl: "http://127.0.0.1:8787/v1/tokens/exchange",
|
||||
});
|
||||
await allowConnectionForAgent(db, company.id, agent.id, connection.id);
|
||||
const app = createRouteApp(db, agentJwtActor(company.id, agent.id, run.id));
|
||||
const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => {
|
||||
expect(String(url)).toBe("http://127.0.0.1:8787/v1/tokens/exchange");
|
||||
expect(init).toMatchObject({
|
||||
method: "POST",
|
||||
redirect: "manual",
|
||||
headers: expect.objectContaining({ authorization: "Bearer parent-deploy-token" }),
|
||||
});
|
||||
return {
|
||||
ok: true,
|
||||
status: 201,
|
||||
json: async () => ({
|
||||
token: "allowlisted-child-token",
|
||||
expires_in: 600,
|
||||
scope: "pages:publish:ns/dotta",
|
||||
}),
|
||||
} as Response;
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/agents/me/connections/${connection.id}/token`)
|
||||
.send({ scope: "pages:publish:ns/dotta" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toMatchObject({ token: "allowlisted-child-token" });
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("selects scoped credentials for array scopes and fails closed for unknown selectors", async () => {
|
||||
const company = await createCompany(db);
|
||||
const agent = await createAgent(db, company.id);
|
||||
@@ -1271,6 +1343,86 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
["tokenBroker.tokenUrl", { tokenBroker: { enabled: true, tokenUrl: "http://169.254.169.254/token" } }],
|
||||
["tokenBroker.exchangeTokenUrl", { tokenBroker: { enabled: true, exchangeTokenUrl: "http://169.254.169.254/token" } }],
|
||||
["tokenExchangeUrl", { tokenExchangeUrl: "http://169.254.169.254/token" }],
|
||||
["pagesTokenExchangeUrl", { pagesTokenExchangeUrl: "http://169.254.169.254/token" }],
|
||||
])("rejects a link-local %s when a remote connection is created", async (_field, brokerConfig) => {
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db, {
|
||||
deploymentMode: "authenticated",
|
||||
deploymentExposure: "public",
|
||||
});
|
||||
|
||||
await expect(service.createConnection(company.id, {
|
||||
name: `Rejected broker ${randomUUID()}`,
|
||||
transport: "mcp_remote",
|
||||
config: { url: "https://93.184.216.34/mcp", ...brokerConfig },
|
||||
enabled: true,
|
||||
status: "active",
|
||||
})).rejects.toMatchObject({
|
||||
status: 400,
|
||||
details: { code: "remote_http_private_endpoint" },
|
||||
});
|
||||
await expect(db.select().from(toolConnections)).resolves.toHaveLength(0);
|
||||
});
|
||||
|
||||
it("rejects a link-local token broker when a remote connection is updated", async () => {
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db, {
|
||||
deploymentMode: "authenticated",
|
||||
deploymentExposure: "public",
|
||||
});
|
||||
const connection = await service.createConnection(company.id, {
|
||||
name: "Initially safe broker",
|
||||
transport: "mcp_remote",
|
||||
config: { url: "https://93.184.216.34/mcp" },
|
||||
enabled: true,
|
||||
status: "active",
|
||||
});
|
||||
|
||||
await expect(service.updateConnection(connection.id, {
|
||||
config: {
|
||||
...connection.config,
|
||||
tokenBroker: { enabled: true, tokenUrl: "http://169.254.169.254/token" },
|
||||
},
|
||||
})).rejects.toMatchObject({
|
||||
status: 400,
|
||||
details: { code: "remote_http_private_endpoint" },
|
||||
});
|
||||
await expect(service.getConnection(connection.id)).resolves.toMatchObject({
|
||||
config: { url: "https://93.184.216.34/mcp" },
|
||||
});
|
||||
});
|
||||
|
||||
it("implicitly allowlists the configured Pages API host for internal token brokers", async () => {
|
||||
vi.stubEnv("PAPERCLIP_PAGES_API_URL", "http://127.0.0.1:8787");
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db, {
|
||||
deploymentMode: "authenticated",
|
||||
deploymentExposure: "public",
|
||||
});
|
||||
|
||||
await expect(service.createConnection(company.id, {
|
||||
name: "Internal Pages broker",
|
||||
transport: "mcp_remote",
|
||||
config: {
|
||||
url: "https://93.184.216.34/mcp",
|
||||
tokenBroker: {
|
||||
enabled: true,
|
||||
tokenUrl: "http://127.0.0.1:9999/v1/tokens/exchange",
|
||||
},
|
||||
},
|
||||
enabled: true,
|
||||
status: "active",
|
||||
})).resolves.toMatchObject({
|
||||
config: {
|
||||
tokenBroker: { tokenUrl: "http://127.0.0.1:9999/v1/tokens/exchange" },
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("lists testable agents with per-connection effective access summaries", async () => {
|
||||
const company = await createCompany(db);
|
||||
const userId = `tool-tester-${randomUUID()}`;
|
||||
|
||||
@@ -1841,6 +1841,63 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
return assertRemoteHttpUrlAllowed(remoteEndpoint(config));
|
||||
}
|
||||
|
||||
function normalizeTokenBrokerAllowedHost(value: string): string | null {
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed) return null;
|
||||
try {
|
||||
const parsed = new URL(trimmed.includes("://") ? trimmed : `http://${trimmed}`);
|
||||
return parsed.hostname.replace(/^\[|\]$/g, "").replace(/\.$/, "").toLowerCase() || null;
|
||||
} catch {
|
||||
// Invalid allowlist entries grant no access. The configured broker URL is
|
||||
// still evaluated under the public-only policy below.
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function tokenBrokerAllowedPrivateHosts(): Set<string> {
|
||||
const configured = (process.env.PAPERCLIP_TOKEN_BROKER_ALLOWED_HOSTS ?? "")
|
||||
.split(/[,\s]+/)
|
||||
.map(normalizeTokenBrokerAllowedHost)
|
||||
.filter((host): host is string => host !== null);
|
||||
const pagesApiHost = normalizeTokenBrokerAllowedHost(process.env.PAPERCLIP_PAGES_API_URL ?? "");
|
||||
if (pagesApiHost) configured.push(pagesApiHost);
|
||||
return new Set(configured);
|
||||
}
|
||||
|
||||
function tokenBrokerAllowsPrivateNetwork(endpoint: URL): boolean {
|
||||
const hostname = endpoint.hostname.replace(/^\[|\]$/g, "").replace(/\.$/, "").toLowerCase();
|
||||
return tokenBrokerAllowedPrivateHosts().has(hostname);
|
||||
}
|
||||
|
||||
function tokenBrokerHttpFetchOptions(endpoint: URL): GuardedRemoteHttpFetchOptions {
|
||||
return {
|
||||
allowPrivateNetwork: tokenBrokerAllowsPrivateNetwork(endpoint),
|
||||
error: (message, code) => badRequest(message, { code }),
|
||||
};
|
||||
}
|
||||
|
||||
async function assertTokenBrokerHttpUrlAllowed(value: string): Promise<string> {
|
||||
const endpoint = parseRemoteHttpEndpoint(value, (message, code) => badRequest(message, { code }));
|
||||
await assertPublicRemoteHttpEndpoint(
|
||||
endpoint,
|
||||
{ allowPrivateNetwork: tokenBrokerAllowsPrivateNetwork(endpoint) },
|
||||
(message, code) => badRequest(message, { code }),
|
||||
);
|
||||
return endpoint.toString();
|
||||
}
|
||||
|
||||
async function assertConfiguredTokenBrokerEndpointsAllowed(config: Record<string, unknown>): Promise<void> {
|
||||
for (const url of configuredTokenBrokerExchangeUrls(config)) {
|
||||
await assertTokenBrokerHttpUrlAllowed(url);
|
||||
}
|
||||
}
|
||||
|
||||
async function assertRemoteConnectionEndpointsAllowed(config: Record<string, unknown>): Promise<string> {
|
||||
const endpoint = await assertRemoteEndpointAllowed(config);
|
||||
await assertConfiguredTokenBrokerEndpointsAllowed(config);
|
||||
return endpoint;
|
||||
}
|
||||
|
||||
/**
|
||||
* OAuth endpoint scheme/transport gate (PAP-17099).
|
||||
*
|
||||
@@ -2031,13 +2088,26 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
return [];
|
||||
}
|
||||
|
||||
function tokenBrokerConfig(connection: typeof toolConnections.$inferSelect): Record<string, unknown> {
|
||||
const config = asRecord(connection.config);
|
||||
function tokenBrokerConfigFromConnectionConfig(config: Record<string, unknown>): Record<string, unknown> {
|
||||
const broker = asRecord(config.tokenBroker);
|
||||
if (Object.keys(broker).length > 0) return broker;
|
||||
return asRecord(config.broker);
|
||||
}
|
||||
|
||||
function tokenBrokerConfig(connection: typeof toolConnections.$inferSelect): Record<string, unknown> {
|
||||
return tokenBrokerConfigFromConnectionConfig(asRecord(connection.config));
|
||||
}
|
||||
|
||||
function configuredTokenBrokerExchangeUrls(config: Record<string, unknown>): string[] {
|
||||
const broker = tokenBrokerConfigFromConnectionConfig(config);
|
||||
return [...new Set([
|
||||
readConfigString(broker, "tokenUrl"),
|
||||
readConfigString(broker, "exchangeTokenUrl"),
|
||||
readConfigString(config, "tokenExchangeUrl"),
|
||||
readConfigString(config, "pagesTokenExchangeUrl"),
|
||||
].filter((url): url is string => url !== null))];
|
||||
}
|
||||
|
||||
function connectionTokenBrokerEnabled(connection: typeof toolConnections.$inferSelect): boolean {
|
||||
const config = asRecord(connection.config);
|
||||
const tokenBroker = asRecord(config.tokenBroker);
|
||||
@@ -2421,21 +2491,23 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
body.set("requested_token_type", readConfigString(broker, "requestedTokenType") ?? "urn:ietf:params:oauth:token-type:access_token");
|
||||
body.set("actor_token", Buffer.from(JSON.stringify(actor)).toString("base64url"));
|
||||
body.set("actor_token_type", readConfigString(broker, "actorTokenType") ?? "urn:ietf:params:oauth:token-type:jwt");
|
||||
response = await fetch(url, {
|
||||
const endpoint = parseRemoteHttpEndpoint(url, (message, code) => badRequest(message, { code }));
|
||||
response = await guardedRemoteHttpFetch(endpoint, {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/x-www-form-urlencoded" },
|
||||
body,
|
||||
});
|
||||
}, tokenBrokerHttpFetchOptions(endpoint));
|
||||
} else {
|
||||
const namespace = isPages ? pagesNamespaceFromScope(input.scope) : null;
|
||||
const body = isPages && namespace
|
||||
? { namespace, ttlSeconds: input.ttlSeconds, actions: ["publish"], actor }
|
||||
: { scope: input.scope, ttlSeconds: input.ttlSeconds, actor, audience: readConfigString(broker, "audience") };
|
||||
response = await fetch(url, {
|
||||
const endpoint = parseRemoteHttpEndpoint(url, (message, code) => badRequest(message, { code }));
|
||||
response = await guardedRemoteHttpFetch(endpoint, {
|
||||
method: "POST",
|
||||
headers: { authorization: `Bearer ${parentToken}`, "content-type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
}, tokenBrokerHttpFetchOptions(endpoint));
|
||||
}
|
||||
const payload = await response.json().catch(() => ({})) as unknown;
|
||||
const record = asRecord(payload);
|
||||
@@ -6224,7 +6296,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
config = normalizeGoogleSheetsConnectionConfig(config);
|
||||
await assertGoogleSheetsSpreadsheetOwnership(companyId, config);
|
||||
}
|
||||
if (transport === "mcp_remote") await assertRemoteEndpointAllowed(config);
|
||||
if (transport === "mcp_remote") await assertRemoteConnectionEndpointsAllowed(config);
|
||||
if (transport === "local_stdio") await stdioTemplateId(companyId, config);
|
||||
assertLocalStdioCanBeEnabled(transport, false);
|
||||
|
||||
@@ -7859,7 +7931,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
const transport = input.transport;
|
||||
if (!transport) throw badRequest("Tool connection transport is required");
|
||||
const config = normalizeGoogleSheetsConnectionConfig(input.config ?? input.transportConfig ?? {});
|
||||
if (transport === "mcp_remote") await assertRemoteEndpointAllowed(config);
|
||||
if (transport === "mcp_remote") await assertRemoteConnectionEndpointsAllowed(config);
|
||||
if (transport === "local_stdio") await stdioTemplateId(companyId, config);
|
||||
assertLocalStdioCanBeEnabled(transport, input.enabled ?? false);
|
||||
await assertGoogleSheetsSpreadsheetOwnership(companyId, config);
|
||||
@@ -8095,7 +8167,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
|
||||
updateConnection: async (connectionId: string, input: UpdateToolConnection): Promise<ToolConnection> => {
|
||||
const existing = await getConnectionRow(connectionId);
|
||||
const config = normalizeGoogleSheetsConnectionConfig(input.config ?? input.transportConfig ?? existing.config);
|
||||
if (existing.transport === "mcp_remote") await assertRemoteEndpointAllowed(config);
|
||||
if (existing.transport === "mcp_remote") await assertRemoteConnectionEndpointsAllowed(config);
|
||||
if (existing.transport === "local_stdio") await stdioTemplateId(existing.companyId, config);
|
||||
assertLocalStdioCanBeEnabled(existing.transport, input.enabled ?? existing.enabled);
|
||||
await assertGoogleSheetsSpreadsheetOwnership(existing.companyId, config, { excludeConnectionId: existing.id });
|
||||
|
||||
Reference in new issue
Block a user