fix(api): refuse artifact uploads from untrusted origins
Uploads with a text/plain, text/csv or text/markdown body are CORS "simple" requests that a browser sends cross-site without a preflight. The session cookie's SameSite default already keeps it off such requests, but a deployment that relaxes it would accept uploads from any page. The route now applies the same origin rule as CORS before reading the session. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRtGG9ARfqqhma3t85Ro2u
This commit is contained in:
parent
c008112ab4
commit
147611a8a1
@ -589,6 +589,7 @@ export async function createApp(
|
|||||||
artifacts,
|
artifacts,
|
||||||
sessionUserId: async (request) =>
|
sessionUserId: async (request) =>
|
||||||
(await auth.api.getSession({ headers: sessionHeaders(request) }))?.user?.id ?? null,
|
(await auth.api.getSession({ headers: sessionHeaders(request) }))?.user?.id ?? null,
|
||||||
|
trustedOrigin: (origin) => isTrustedOrigin(origin, env),
|
||||||
});
|
});
|
||||||
mountWebhookHttpRoutes(app, { prisma, secrets, events, jobs });
|
mountWebhookHttpRoutes(app, { prisma, secrets, events, jobs });
|
||||||
// Shared with stop so a shutdown during retry delays does not restart polling.
|
// Shared with stop so a shutdown during retry delays does not restart polling.
|
||||||
|
|||||||
@ -77,6 +77,7 @@ describe("artifact HTTP routes", () => {
|
|||||||
prisma: prisma as unknown as PrismaClient,
|
prisma: prisma as unknown as PrismaClient,
|
||||||
artifacts: artifacts as unknown as ArtifactStore,
|
artifacts: artifacts as unknown as ArtifactStore,
|
||||||
sessionUserId,
|
sessionUserId,
|
||||||
|
trustedOrigin: (origin) => origin === "http://127.0.0.1:5173",
|
||||||
});
|
});
|
||||||
return { hono, prisma, artifacts };
|
return { hono, prisma, artifacts };
|
||||||
}
|
}
|
||||||
@ -95,6 +96,19 @@ describe("artifact HTTP routes", () => {
|
|||||||
expect(artifacts.putStream).not.toHaveBeenCalled();
|
expect(artifacts.putStream).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("refuses uploads from untrusted origins before reading the session", async () => {
|
||||||
|
const sessionUserId = vi.fn(async () => "user-1");
|
||||||
|
const { hono, artifacts } = app(sessionUserId);
|
||||||
|
const response = await hono.request("/api/artifacts?botId=bot-1&name=a.txt", {
|
||||||
|
method: "POST",
|
||||||
|
headers: { "content-type": "text/plain", origin: "https://evil.example" },
|
||||||
|
body: "data",
|
||||||
|
});
|
||||||
|
expect(response.status).toBe(403);
|
||||||
|
expect(sessionUserId).not.toHaveBeenCalled();
|
||||||
|
expect(artifacts.putStream).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("answers ids that are not artifact ids with 404 without a lookup", async () => {
|
it("answers ids that are not artifact ids with 404 without a lookup", async () => {
|
||||||
const { hono, prisma } = app(async () => "user-1");
|
const { hono, prisma } = app(async () => "user-1");
|
||||||
const response = await hono.request(`/api/artifacts/${encodeURIComponent("../etc/passwd")}`);
|
const response = await hono.request(`/api/artifacts/${encodeURIComponent("../etc/passwd")}`);
|
||||||
|
|||||||
@ -20,6 +20,8 @@ export type ArtifactHttpDeps = {
|
|||||||
artifacts: ArtifactStore;
|
artifacts: ArtifactStore;
|
||||||
/** The signed-in user (cookie or bearer session), or null. */
|
/** The signed-in user (cookie or bearer session), or null. */
|
||||||
sessionUserId: (request: Request) => Promise<string | null>;
|
sessionUserId: (request: Request) => Promise<string | null>;
|
||||||
|
/** Same rule as CORS. Raw uploads can be CORS "simple" requests, so the origin is checked. */
|
||||||
|
trustedOrigin: (origin: string) => boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
/** Generated ids only; anything else is not an artifact. */
|
/** Generated ids only; anything else is not an artifact. */
|
||||||
@ -208,10 +210,12 @@ export function mountArtifactHttpRoutes(app: Hono, deps: ArtifactHttpDeps) {
|
|||||||
|
|
||||||
app.post(ARTIFACT_HTTP_PATH, async (c) => {
|
app.post(ARTIFACT_HTTP_PATH, async (c) => {
|
||||||
const request = c.req.raw;
|
const request = c.req.raw;
|
||||||
const reject = (status: 400 | 401 | 404 | 413 | 415, error: string) => {
|
const reject = (status: 400 | 401 | 403 | 404 | 413 | 415, error: string) => {
|
||||||
cancelBody(request.body);
|
cancelBody(request.body);
|
||||||
return c.json({ error }, status);
|
return c.json({ error }, status);
|
||||||
};
|
};
|
||||||
|
const origin = c.req.header("origin");
|
||||||
|
if (origin && !deps.trustedOrigin(origin)) return reject(403, "Forbidden");
|
||||||
const userId = await deps.sessionUserId(request);
|
const userId = await deps.sessionUserId(request);
|
||||||
if (!userId) return reject(401, "Unauthorized");
|
if (!userId) return reject(401, "Unauthorized");
|
||||||
const actor = await requireMembership(
|
const actor = await requireMembership(
|
||||||
|
|||||||
@ -148,6 +148,15 @@ describeRoutes("artifact HTTP routes", () => {
|
|||||||
(await upload(app, intruder, { groupId: group.id }, "x.mp4", "video/mp4", video)).status,
|
(await upload(app, intruder, { groupId: group.id }, "x.mp4", "video/mp4", video)).status,
|
||||||
).toBe(404);
|
).toBe(404);
|
||||||
|
|
||||||
|
// A cross-site page cannot upload with the owner's cookie.
|
||||||
|
expect(
|
||||||
|
(
|
||||||
|
await upload(app, owner, { botId: bot.id }, "x.txt", "text/plain", video, {
|
||||||
|
origin: "https://evil.example",
|
||||||
|
})
|
||||||
|
).status,
|
||||||
|
).toBe(403);
|
||||||
|
|
||||||
// Signed out.
|
// Signed out.
|
||||||
expect((await app.request(`/api/artifacts/${botArtifact.id}`)).status).toBe(401);
|
expect((await app.request(`/api/artifacts/${botArtifact.id}`)).status).toBe(401);
|
||||||
expect(
|
expect(
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user