Merge remote-tracking branch 'origin/feat/dashboard-agent-flows' into feat/dashboard-agent-ui

Author: kathiekiwi

apps/webapp/app/entry.server.tsx

@@ -49,12 +49,12 @@ import { workerRegionRegistry } from "./v3/workerRegions.server";
 const ABORT_DELAY = 30000;
 
 /**
- * Where a document may load images from. The agent's markdown renders no images at
- * all (`components/dashboard-agent/model-markdown.ts`); this is the backstop, so a
- * model-authored image that ever slips through still can't reach a remote host.
+ * Where a document may load images from. The markdown renderer that strips images
+ * ships in the stacked UI PR, so on this branch the policy is the only thing stopping
+ * a model- or customer-authored image from reaching a remote host.
  *
- * Only exact origins: the GitHub avatar host we store avatar URLs for, plus
- * whatever `CSP_IMG_SRC_ALLOWLIST` adds (e.g. a self-hosted SSO avatar host).
+ * The hosts we store avatar URLs for, plus whatever `CSP_IMG_SRC_ALLOWLIST` adds
+ * (e.g. a self-hosted SSO avatar host).
  */
 const IMG_SRC_DIRECTIVE = buildImgSrcDirective(
   singleton("CspImageOrigins", () => {

apps/webapp/app/utils/cspImageOrigins.test.ts

@@ -1,11 +1,26 @@
 import { describe, expect, it } from "vitest";
+import { faviconUrl } from "./favicon";
 import {
   BASE_IMG_SRC_SOURCES,
   buildImgSrcDirective,
   parseCspImageOrigins,
   withImgSrc,
 } from "./cspImageOrigins";
 
+/** True if a source expression in the directive would match the given image URL. */
+function directivePermits(directive: string, imageUrl: string): boolean {
+  const url = new URL(imageUrl);
+  return directive
+    .split(" ")
+    .slice(1)
+    .some((source) => {
+      if (!source.startsWith("http")) return false;
+      const parsed = new URL(source);
+      if (parsed.protocol !== url.protocol || parsed.host !== url.host) return false;
+      return parsed.pathname === "/" || parsed.pathname === url.pathname;
+    });
+}
+
 describe("parseCspImageOrigins", () => {
   it("accepts exact https origins, with or without a port", () => {
     const { origins, rejected } = parseCspImageOrigins(
@@ -75,12 +90,28 @@ describe("parseCspImageOrigins", () => {
 });
 
 describe("buildImgSrcDirective", () => {
-  it("is self, data, blob and the SSO avatar hosts by default", () => {
+  it("is self, data, blob, the SSO avatar hosts and the favicon endpoint by default", () => {
     expect(buildImgSrcDirective()).toBe(
-      "img-src 'self' data: blob: https://avatars.githubusercontent.com https://lh3.googleusercontent.com"
+      "img-src 'self' data: blob: https://avatars.githubusercontent.com https://lh3.googleusercontent.com https://www.google.com/s2/favicons"
     );
   });
 
+  it("permits the org avatar URL the app actually stores", () => {
+    expect(directivePermits(buildImgSrcDirective(), faviconUrl("example.com"))).toBe(true);
+  });
+
+  it("permits nothing else on the favicon host", () => {
+    expect(directivePermits(buildImgSrcDirective(), "https://www.google.com/beacon.png")).toBe(
+      false
+    );
+  });
+
+  it("permits both OAuth avatar hosts", () => {
+    const directive = buildImgSrcDirective();
+    expect(directivePermits(directive, "https://avatars.githubusercontent.com/u/1?v=4")).toBe(true);
+    expect(directivePermits(directive, "https://lh3.googleusercontent.com/a/abc=s96-c")).toBe(true);
+  });
+
   it("has no wildcard host and no bare scheme host", () => {
     const directive = buildImgSrcDirective(parseCspImageOrigins("https://sso.example.com").origins);
     expect(directive).not.toContain("*");

apps/webapp/app/utils/cspImageOrigins.ts

@@ -1,16 +1,22 @@
 /**
  * The document `img-src` allowlist. Remote images are a beacon channel: rendering
- * one is the outbound request, no click needed. So the list is exact origins only —
- * no wildcard host, no bare scheme, nothing with a path.
+ * one is the outbound request, no click needed. So no wildcard host and no bare
+ * scheme. Operator-supplied entries are exact origins; a base source may pin a path
+ * to narrow the host further.
  */
 
-/** Always allowed: own origin, inline data, object URLs, and the SSO avatar hosts. */
+/**
+ * Always allowed: own origin, inline data, object URLs, the SSO avatar hosts, and the
+ * favicon endpoint org avatars are stored as (see `utils/favicon.ts`). The path pins
+ * that one endpoint — CSP matches the path and ignores the query string.
+ */
 export const BASE_IMG_SRC_SOURCES = [
   "'self'",
   "data:",
   "blob:",
   "https://avatars.githubusercontent.com",
   "https://lh3.googleusercontent.com",
+  "https://www.google.com/s2/favicons",
 ] as const;
 
 export type RejectedOrigin = { value: string; reason: string };

apps/webapp/test/contextlessPatRoutes.test.ts

@@ -53,18 +53,40 @@ import { loader as projectsLoader } from "~/routes/api.v1.projects";
 
 const USER_ID = "usr_1";
 
-async function createOrg(cap: string[]): Promise<{ status: number; body: any }> {
+// Counts `can()` invocations without changing what the ability answers.
+function countingAbility(ability: any): { ability: any; canCalls: () => number } {
+  let canCalls = 0;
+  const wrapped = new Proxy(ability, {
+    get(target, prop, receiver) {
+      const value = Reflect.get(target, prop, receiver);
+      if (typeof value !== "function") {
+        return value;
+      }
+      if (prop === "can") {
+        return (...args: any[]) => {
+          canCalls++;
+          return value.apply(target, args);
+        };
+      }
+      return value.bind(target);
+    },
+  });
+  return { ability: wrapped, canCalls: () => canCalls };
+}
+
+async function createOrg(cap: string[]): Promise<{ status: number; body: any; canCalls: number }> {
   const token = await signUserActorToken(SESSION_SECRET, {
     userId: USER_ID,
     client: "personal-access-token",
     cap,
   });
+  const counted = countingAbility(buildJwtAbility(cap));
   mocks.authenticateUserActor.mockImplementation(async () => ({
     ok: true,
     userId: USER_ID,
     claims: { userId: USER_ID, client: "personal-access-token", cap },
     subject: { type: "userActor", userId: USER_ID, organizationId: "org_1" },
-    ability: buildJwtAbility(cap),
+    ability: counted.ability,
   }));
 
   const response = await action({
@@ -76,19 +98,26 @@ async function createOrg(cap: string[]): Promise<{ status: number; body: any }>
     params: {},
     context: {},
   } as any);
-  return { status: response.status, body: await response.json() };
+  return { status: response.status, body: await response.json(), canCalls: counted.canCalls() };
 }
 
 // An ordinary PAT, paired with an ability that denies everything. Nothing on this route may
 // consult it — the route has no org to scope a gate to, and on cloud the plugin returns a
 // deny-shaped ability when there is no org context.
-async function createOrgWithPat(): Promise<{ status: number; body: any }> {
+async function createOrgWithPat(): Promise<{ status: number; body: any; canCalls: number }> {
+  let canCalls = 0;
   mocks.authenticatePat.mockImplementation(async () => ({
     ok: true,
     userId: USER_ID,
     tokenId: "pat_1",
     lastAccessedAt: new Date(),
-    ability: { can: () => false, canSuper: () => false },
+    ability: {
+      can: () => {
+        canCalls++;
+        return false;
+      },
+      canSuper: () => false,
+    },
   }));
 
   const response = await action({
@@ -100,7 +129,7 @@ async function createOrgWithPat(): Promise<{ status: number; body: any }> {
     params: {},
     context: {},
   } as any);
-  return { status: response.status, body: await response.json() };
+  return { status: response.status, body: await response.json(), canCalls };
 }
 
 const AGENT_ENVIRONMENT_ID = "env_dev";
@@ -177,6 +206,7 @@ describe("creating an organization over the API", () => {
 
     expect(result.status).toBe(201);
     expect(result.body.slug).toBe("new-org");
+    expect(result.canCalls).toBe(0);
   });
 
   // The env gate runs before the capability gate, so an install with the API disabled tells
@@ -189,6 +219,7 @@ describe("creating an organization over the API", () => {
       const result = await createOrg(["read:all"]);
 
       expect(result.status).toBe(404);
+      expect(result.canCalls).toBe(0);
       expect(mocks.createOrganization).not.toHaveBeenCalled();
     } finally {
       mocks.env.ORG_CREATION_API_ENABLED = "1";