fix(connections): commit a Paperclip page before the OAuth callback's slow work

Railway's consent page navigates to the callback and, if it is still on
screen ~2s later, replaces itself with its own 'you can close this window'
screen. Code exchange plus tool discovery takes longer than that, so the
browser never returned to Paperclip even though the connection completed.

A cross-site browser navigation to the callback now gets an immediate
same-origin interstitial that repeats the request; the repeat does the work.

Also updates three callback tests to the armed ask-first write default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
scotttongandClaude Opus 5.5 committed 2026-09-30 18:02:31 -07:00
1 parent 673b03496b
commit 69c0a8541c
2 files changed
+87 -20

No files matched your search

@@ -8048,7 +8048,7 @@ describeEmbeddedPostgres("tool access service", () => {
}
});
it("activates allowed Drive write actions without approval defaults after a managed callback", async () => {
it("activates Drive write actions behind ask-first defaults after a managed callback", async () => {
const company = await createCompany(db);
const userId = `drive-write-member-${randomUUID()}`;
await grantBoardUser(db, company.id, userId, [], "owner");
@@ -8148,8 +8148,9 @@ describeEmbeddedPostgres("tool access service", () => {
const createEntry = callback.body.catalog.find(
(entry: { toolName: string }) => entry.toolName === "create_file",
);
await expect(
db
// The armed default: writes ask first, reads stay allowed.
const drivePolicies = JSON.stringify(
await db
.select()
.from(toolPolicies)
.where(
@@ -8158,7 +8159,9 @@ describeEmbeddedPostgres("tool access service", () => {
eq(toolPolicies.enabled, true),
),
),
).resolves.toEqual([]);
);
expect(drivePolicies).toContain(createEntry.id);
expect(drivePolicies).not.toContain(searchEntry.id);
await expect(
db
.select()
@@ -8273,12 +8276,20 @@ describeEmbeddedPostgres("tool access service", () => {
).resolves.toEqual([
expect.objectContaining({ targetType: "agent", targetId: agent.id }),
]);
await expect(
db
.select()
.from(toolPolicies)
.where(eq(toolPolicies.companyId, company.id)),
).resolves.toEqual([]);
// Reconnecting neither adds nor drops the ask-first defaults.
expect(
JSON.stringify(
await db
.select()
.from(toolPolicies)
.where(
and(
eq(toolPolicies.companyId, company.id),
eq(toolPolicies.enabled, true),
),
),
),
).toBe(drivePolicies);
} finally {
driveDefinition.ownershipAvailability = previousOwnershipAvailability;
}
@@ -9495,8 +9506,9 @@ describeEmbeddedPostgres("tool access service", () => {
const sendMessageEntry = completed.catalog.find(
(entry) => entry.toolName === "send_message",
)!;
await expect(
db
// The armed default: writes ask first, reads stay allowed.
const callbackPolicies = JSON.stringify(
await db
.select()
.from(toolPolicies)
.where(
@@ -9505,7 +9517,9 @@ describeEmbeddedPostgres("tool access service", () => {
eq(toolPolicies.enabled, true),
),
),
).resolves.toEqual([]);
);
expect(callbackPolicies).toContain(sendMessageEntry.id);
expect(callbackPolicies).not.toContain(searchMessagesEntry.id);
const callbackPolicy = toolAccessPolicyService(db);
const decide = (
entry: (typeof completed.catalog)[number],
@@ -9521,13 +9535,14 @@ describeEmbeddedPostgres("tool access service", () => {
arguments: {},
},
});
for (const entry of [searchMessagesEntry, sendMessageEntry]) {
await expect(decide(entry)).resolves.toMatchObject(
host === "task"
? { decision: "deny" }
: { decision: "allow", reasonCode: "allow_profile" },
);
}
await expect(decide(searchMessagesEntry)).resolves.toMatchObject(
host === "task"
? { decision: "deny" }
: { decision: "allow", reasonCode: "allow_profile" },
);
await expect(decide(sendMessageEntry)).resolves.toMatchObject(
host === "task" ? { decision: "deny" } : { decision: "require_approval" },
);
if (host === "task") {
await expect(
db
@@ -10115,9 +10130,31 @@ describeEmbeddedPostgres("tool access service", () => {
);
expect(state).toBeTruthy();
// The provider's redirect is a cross-site navigation: Paperclip commits a
// page at once (Railway's consent page otherwise replaces itself after ~2s)
// and leaves the state unconsumed for the same-origin repeat.
const interstitialRes = await request(app)
.get("/api/tools/oauth/callback")
.set("Accept", "text/html")
.set("Sec-Fetch-Site", "cross-site")
.set("Sec-Fetch-Mode", "navigate")
.query({ state, code: "notion-choice-code" });
expect(interstitialRes.status).toBe(200);
expect(interstitialRes.headers["cache-control"]).toBe("no-store");
expect(interstitialRes.text).toContain(
`<meta http-equiv="refresh" content="0;url=/api/tools/oauth/callback?state=${state}&amp;code=notion-choice-code">`,
);
const [pendingConnection] = await db
.select()
.from(toolConnections)
.where(eq(toolConnections.id, connectRes.body.connectionId));
expect(pendingConnection?.status).not.toBe("active");
const callbackRes = await request(app)
.get("/api/tools/oauth/callback")
.set("Accept", "text/html")
.set("Sec-Fetch-Site", "same-origin")
.set("Sec-Fetch-Mode", "navigate")
.query({ state, code: "notion-choice-code" });
expect(callbackRes.status).toBe(303);
+30
View File
@@ -191,6 +191,29 @@ export function connectionIntentOAuthOutcomeHtml(input: {
return `<!doctype html><html><head><meta charset="utf-8"><title>Connection authorization</title></head><body><p>Returning to Paperclip…</p><script>const message=${message};const targetOrigin=${targetOrigin}||window.location.origin;if(window.opener&&window.opener!==window){window.opener.postMessage(message,targetOrigin);window.close();}else{window.location.replace(${fallback});}</script></body></html>`;
}
// Some providers' consent pages navigate to this callback and, if their own
// page is still on screen ~2s later, replace it with a "you can close this
// window" screen (Railway does exactly this). Exchanging the code and
// discovering the tool catalog routinely takes longer than that, so the
// provider's timer wins and the browser never lands back in Paperclip even
// though the connection completed. For a cross-site browser navigation, commit
// a Paperclip document immediately and repeat the same request from it; the
// repeat is same-origin and does the slow work.
export function isCrossSiteOAuthCallbackNavigation(req: Request): boolean {
return req.get("sec-fetch-site") === "cross-site"
&& req.get("sec-fetch-mode") === "navigate";
}
export function oauthCallbackInterstitialHtml(continuePath: string): string {
const attribute = continuePath
.replaceAll("&", "&amp;")
.replaceAll("\"", "&quot;")
.replaceAll("<", "&lt;");
// A meta refresh alone, not a script as well: the OAuth code is single-use, so
// two racing follow-ups would let the loser render an expired-state error.
return `<!doctype html><html><head><meta charset="utf-8"><meta name="referrer" content="no-referrer"><meta http-equiv="refresh" content="0;url=${attribute}"><title>Finishing connection</title></head><body><p>Finishing your connection…</p></body></html>`;
}
function normalizeCloudConnectorEnrollmentReturnTo(returnTo?: string | null): string | null {
if (!returnTo || returnTo.length > 2_048) return null;
try {
@@ -1331,6 +1354,13 @@ function connectorEnrollmentPrincipal(req: Request): string {
await assertToolConnectionConfigureAccess(req, pendingConnection);
}
const acceptsHtml = req.get("accept")?.includes("text/html") === true;
if (acceptsHtml && isCrossSiteOAuthCallbackNavigation(req)) {
// State is only peeked above, so the same-origin repeat still owns it.
res.set("Cache-Control", "no-store");
res.set("Referrer-Policy", "no-referrer");
res.type("html").send(oauthCallbackInterstitialHtml(req.originalUrl));
return;
}
let result: Awaited<ReturnType<typeof svc.completeOAuthCallback>>;
try {
result = await svc.completeOAuthCallback({