mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
fix(connections): retain safe broker rejection diagnostics (#14098)
Preserve fixed broker rejection reason codes within strict size/time limits while retaining public error codes and status. Unknown bodies remain generic; no raw response or credential material enters the error. Validated by 33 consumer tests, a synthetic producer HTTP contract fixture, root typecheck/build, and full CI. Greptile 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -316,6 +316,20 @@ Gmail uses the same credential ownership choice as the rest of the Apps setup:
|
||||
- Trash, spam, destructive label changes, newly discovered tools, nested
|
||||
execution, and any future send tool remain blocked until separately reviewed.
|
||||
|
||||
## Broker rejection diagnostics
|
||||
|
||||
A rejected connector request includes its operation, HTTP status, and an
|
||||
allowlisted broker reason in the server error. For example,
|
||||
`RETURN_ORIGIN_NOT_ENROLLED` means the callback origin is not enrolled for that
|
||||
instance. Compare the current page/configured origin with the instance's enrolled
|
||||
origins; do not bypass origin checks or copy credentials to a different instance.
|
||||
|
||||
Unknown codes, malformed bodies, oversized responses, and stalled diagnostic
|
||||
reads produce `UNKNOWN_BROKER_ERROR`. This is not proof of any specific rejection
|
||||
cause. Diagnostics read at most 4 KiB within 500 ms and retain no raw response
|
||||
messages, URLs, tokens, authorization state, or instance/customer identifiers.
|
||||
The existing error code, status, authorization, and retry behavior are unchanged.
|
||||
|
||||
## Verification checklist
|
||||
|
||||
### Development
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"status": 400,
|
||||
"body": {
|
||||
"error": "RETURN_ORIGIN_NOT_ENROLLED"
|
||||
}
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
type KeyObject,
|
||||
} from "node:crypto";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
|
||||
import {
|
||||
createPaperclipCloudConnector,
|
||||
@@ -24,6 +25,13 @@ const instanceId = "inst_test";
|
||||
const companyId = "company_test";
|
||||
const subject = "user_test";
|
||||
|
||||
// Captured from the producer's in-memory HTTP integration test, using synthetic
|
||||
// enrollment and a signed request with a callback outside the enrolled origins.
|
||||
// Retain only the HTTP status and exact JSON error body, never request data.
|
||||
const originRejectionContract = JSON.parse(readFileSync(
|
||||
new URL("./fixtures/cloud-connector-origin-rejection.json", import.meta.url), "utf8",
|
||||
)) as { status: number; body: { error: string } };
|
||||
|
||||
function rawPrivateKey(key: KeyObject): string {
|
||||
const jwk = key.export({ format: "jwk" }) as { d?: string };
|
||||
if (!jwk.d) throw new Error("missing private key bytes");
|
||||
@@ -46,6 +54,94 @@ function config() {
|
||||
}
|
||||
|
||||
describe("Paperclip Cloud connector", () => {
|
||||
async function rejection(response: Response) {
|
||||
const connector = createPaperclipCloudConnector({
|
||||
config: config().config,
|
||||
request: vi.fn(async () => response) as typeof fetch,
|
||||
});
|
||||
return connector.startAuthorization({
|
||||
subject, companyId, profile: "gmail.read",
|
||||
returnUri: "https://paperclip.example.test/api/tools/oauth/cloud-connector/callback",
|
||||
returnState: "private-state",
|
||||
}).catch((error: unknown) => error);
|
||||
}
|
||||
|
||||
it("accepts the producer's origin-rejection HTTP contract", async () => {
|
||||
expect(await rejection(Response.json(originRejectionContract.body, {
|
||||
status: originRejectionContract.status,
|
||||
}))).toMatchObject({
|
||||
code: "CONNECTOR_REQUEST_FAILED", status: 400,
|
||||
message: "Paperclip Cloud connector rejected the request (operation=session, status=400, reason=RETURN_ORIGIN_NOT_ENROLLED)",
|
||||
});
|
||||
});
|
||||
|
||||
it("retains an allowlisted rejection reason without broker messages or credentials", async () => {
|
||||
const error = await rejection(Response.json({
|
||||
error: "RETURN_ORIGIN_NOT_ENROLLED",
|
||||
message: "DO_NOT_REPORT private-state access-secret https://private.example.test",
|
||||
}, { status: 400 }));
|
||||
expect(error).toBeInstanceOf(PaperclipCloudConnectorError);
|
||||
expect(error).toMatchObject({
|
||||
code: "CONNECTOR_REQUEST_FAILED", status: 400,
|
||||
message: "Paperclip Cloud connector rejected the request (operation=session, status=400, reason=RETURN_ORIGIN_NOT_ENROLLED)",
|
||||
});
|
||||
expect(JSON.stringify(error)).not.toMatch(/DO_NOT_REPORT|private-state|access-secret|private\.example/);
|
||||
});
|
||||
|
||||
it.each([
|
||||
JSON.stringify({ error: "CUSTOM_SECRET_ERROR", message: "DO_NOT_REPORT" }),
|
||||
JSON.stringify({ error: { code: "RETURN_ORIGIN_NOT_ENROLLED" } }),
|
||||
"<html>DO_NOT_REPORT</html>",
|
||||
JSON.stringify({ error: "RETURN_ORIGIN_NOT_ENROLLED", padding: "x".repeat(4_096) }),
|
||||
])("drops unknown, malformed, or oversized rejection bodies", async (body) => {
|
||||
const error = await rejection(new Response(body, { status: 409 }));
|
||||
expect(error).toMatchObject({
|
||||
code: "REAUTHORIZATION_REQUIRED", status: 409,
|
||||
message: "Paperclip Cloud connector rejected the request (operation=session, status=409, reason=UNKNOWN_BROKER_ERROR)",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the original status when the error body stream fails", async () => {
|
||||
const response = new Response(new ReadableStream({
|
||||
start(controller) { controller.error(new Error("DO_NOT_REPORT")); },
|
||||
}), { status: 503 });
|
||||
expect(await rejection(response)).toMatchObject({
|
||||
code: "CONNECTOR_REQUEST_FAILED", status: 503,
|
||||
message: expect.stringContaining("reason=UNKNOWN_BROKER_ERROR"),
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the original failure when a response body is absent or already locked", async () => {
|
||||
const locked = Response.json({ error: "RETURN_ORIGIN_NOT_ENROLLED" }, { status: 401 });
|
||||
const reader = locked.body!.getReader();
|
||||
try {
|
||||
for (const response of [new Response(null, { status: 401 }), locked]) {
|
||||
expect(await rejection(response)).toMatchObject({
|
||||
code: "CONNECTOR_REQUEST_FAILED", status: 401,
|
||||
message: expect.stringContaining("reason=UNKNOWN_BROKER_ERROR"),
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
await reader.cancel();
|
||||
}
|
||||
});
|
||||
|
||||
it("bounds a stalled diagnostic read and does not wait for cancellation", async () => {
|
||||
vi.useFakeTimers();
|
||||
const cancel = vi.fn(() => new Promise<void>(() => {}));
|
||||
try {
|
||||
const pending = rejection(new Response(new ReadableStream({ cancel }), { status: 400 }));
|
||||
await vi.advanceTimersByTimeAsync(500);
|
||||
expect(await pending).toMatchObject({
|
||||
code: "CONNECTOR_REQUEST_FAILED", status: 400,
|
||||
message: expect.stringContaining("reason=UNKNOWN_BROKER_ERROR"),
|
||||
});
|
||||
expect(cancel).toHaveBeenCalledOnce();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("refreshes capabilities after enrollment and rejects stale cache writes", async () => {
|
||||
const keys = config().config;
|
||||
const env = {
|
||||
|
||||
@@ -129,7 +129,55 @@ const ED25519_PKCS8_PREFIX = Buffer.from("302e020100300506032b657004220420", "he
|
||||
const X25519_PKCS8_PREFIX = Buffer.from("302e020100300506032b656e04220420", "hex");
|
||||
const X25519_SPKI_PREFIX = Buffer.from("302a300506032b656e032100", "hex");
|
||||
|
||||
/** A stable, intentionally detail-free error for all remote broker failures. */
|
||||
// Only fixed protocol codes may enter errors/logs. Never retain the broker's
|
||||
// message, request data, return URI, or an arbitrary error/code string.
|
||||
const BROKER_REJECTION_REASONS = new Set([
|
||||
"RETURN_ORIGIN_NOT_ENROLLED", "INVALID_RETURN_URI", "INVALID_REQUEST",
|
||||
"UNKNOWN_INSTANCE", "INSTANCE_APPROVAL_REQUIRED", "INSTANCE_SUSPENDED",
|
||||
"ENVIRONMENT_MISMATCH", "PROFILE_NOT_AVAILABLE", "PROFILE_NOT_ELIGIBLE",
|
||||
"REQUEST_EXPIRED", "REQUEST_REPLAYED", "SIGNATURE_REJECTED",
|
||||
"AUDIENCE_MISMATCH", "OPERATION_MISMATCH", "PAYLOAD_MISMATCH",
|
||||
"MALFORMED_REQUEST", "UNSUPPORTED_ALGORITHM", "RATE_LIMITED",
|
||||
"PROVIDER_OPERATION_FAILED",
|
||||
]);
|
||||
const UNKNOWN_BROKER_REASON = "UNKNOWN_BROKER_ERROR";
|
||||
|
||||
async function readBrokerRejectionReason(response: Response): Promise<string> {
|
||||
if (response.bodyUsed || response.body?.locked) return UNKNOWN_BROKER_REASON;
|
||||
const reader = response.body?.getReader();
|
||||
if (!reader) return UNKNOWN_BROKER_REASON;
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
return await Promise.race([
|
||||
(async () => {
|
||||
const chunks: Uint8Array[] = [];
|
||||
let size = 0;
|
||||
while (true) {
|
||||
const { done, value } = await reader.read();
|
||||
if (done) break;
|
||||
size += value.byteLength;
|
||||
if (size > 4_096) return UNKNOWN_BROKER_REASON;
|
||||
chunks.push(value);
|
||||
}
|
||||
const body: unknown = JSON.parse(Buffer.concat(chunks).toString("utf8"));
|
||||
const reason = body && typeof body === "object" && "error" in body ? body.error : null;
|
||||
return typeof reason === "string" && BROKER_REJECTION_REASONS.has(reason)
|
||||
? reason : UNKNOWN_BROKER_REASON;
|
||||
})(),
|
||||
new Promise<string>((resolve) => {
|
||||
timer = setTimeout(() => resolve(UNKNOWN_BROKER_REASON), 500);
|
||||
}),
|
||||
]);
|
||||
} catch {
|
||||
return UNKNOWN_BROKER_REASON;
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
// Diagnostic reads and cancellation must never delay the original failure.
|
||||
void reader.cancel().catch(() => {});
|
||||
}
|
||||
}
|
||||
|
||||
/** Stable public code/status with only allowlisted broker diagnostics. */
|
||||
export class PaperclipCloudConnectorError extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
@@ -264,8 +312,9 @@ export function createPaperclipCloudConnector(input: {
|
||||
}
|
||||
if (operation === "revoke" && response.status === 204) return {};
|
||||
if (!response.ok) {
|
||||
const reason = await readBrokerRejectionReason(response);
|
||||
throw new PaperclipCloudConnectorError(
|
||||
"Paperclip Cloud connector rejected the request",
|
||||
`Paperclip Cloud connector rejected the request (operation=${operation}, status=${response.status}, reason=${reason})`,
|
||||
response.status === 409 ? "REAUTHORIZATION_REQUIRED" : "CONNECTOR_REQUEST_FAILED",
|
||||
response.status,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user