Skip to content

Commit 2d23dde

Browse files
committed
fix(webapp): keep the retired chats.messages column, and allow Google SSO avatars
Dropping the column was irreversible and blocked on a production row count; retiring it is not. The avatar host is an exact origin the app already knows, like the GitHub one.
1 parent 5126bae commit 2d23dde

7 files changed

Lines changed: 29 additions & 14 deletions

File tree

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,17 @@ describe("parseCspImageOrigins", () => {
7575
});
7676

7777
describe("buildImgSrcDirective", () => {
78-
it("is self, data, blob and the GitHub avatar host by default", () => {
78+
it("is self, data, blob and the SSO avatar hosts by default", () => {
7979
expect(buildImgSrcDirective()).toBe(
80-
"img-src 'self' data: blob: https://avatars.githubusercontent.com"
80+
"img-src 'self' data: blob: https://avatars.githubusercontent.com https://lh3.googleusercontent.com"
8181
);
8282
});
8383

8484
it("has no wildcard host and no bare scheme host", () => {
8585
const directive = buildImgSrcDirective(parseCspImageOrigins("https://sso.example.com").origins);
8686
expect(directive).not.toContain("*");
87-
expect(directive).not.toContain("googleusercontent.com");
87+
// The avatar hosts are exact origins; a wildcard over them would not be.
88+
expect(directive).not.toContain("*.googleusercontent.com");
8889
expect(directive).not.toMatch(/(^|\s)https?:(\s|$)/);
8990
});
9091

apps/webapp/app/utils/cspImageOrigins.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@
44
* no wildcard host, no bare scheme, nothing with a path.
55
*/
66

7-
/** Always allowed: own origin, inline data, object URLs, and the GitHub avatar host. */
7+
/** Always allowed: own origin, inline data, object URLs, and the SSO avatar hosts. */
88
export const BASE_IMG_SRC_SOURCES = [
99
"'self'",
1010
"data:",
1111
"blob:",
1212
"https://avatars.githubusercontent.com",
13+
"https://lh3.googleusercontent.com",
1314
] as const;
1415

1516
export type RejectedOrigin = { value: string; reason: string };

apps/webapp/test/dashboardAgentImageCsp.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ describe("document image CSP", () => {
3636

3737
it("builds the directive from the configured allowlist, not a wildcard literal", () => {
3838
expect(source).toContain("parseCspImageOrigins(env.CSP_IMG_SRC_ALLOWLIST");
39-
expect(source).not.toContain("googleusercontent.com");
39+
expect(source).not.toContain("*.googleusercontent.com");
4040
});
4141

4242
it("keeps a route's own img-src", () => {

apps/webapp/test/dashboardAgentLegacyMessagesColumn.test.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ import path from "node:path";
33
import { describe, expect, it } from "vitest";
44

55
/**
6-
* `chats.messages` is gone: the transcript lives in `chat_messages`, one row per message.
6+
* `chats.messages` is retired, not dropped: the transcript lives in `chat_messages`, one
7+
* row per message, and the column stays declared so whatever a deployed environment
8+
* already wrote remains readable.
79
*
8-
* TypeScript already catches a reference through the Drizzle schema — the column isn't
9-
* there, so it doesn't compile. A raw-SQL reference compiles fine and fails at runtime,
10-
* which is the hole this scan covers. Zero hits today is the point; the test exists so a
11-
* reintroduction is caught rather than deployed.
10+
* Retired means nothing reads or writes it. The column being declared is exactly why this
11+
* scan matters — the compiler no longer refuses a reference to it, in Drizzle or in raw
12+
* SQL. Zero hits today is the point; the test exists so a reintroduction is caught rather
13+
* than deployed.
1214
*/
1315

1416
const ROOT = path.resolve(__dirname, "../../..");
@@ -25,7 +27,7 @@ const SCANNED = [
2527
* aliased table (`from chats c … c.messages`). Migrations are skipped on purpose.
2628
*/
2729

28-
/** A qualified reference to the dropped column, in any of the spellings Postgres accepts. */
30+
/** A qualified reference to the retired column, in any of the spellings Postgres accepts. */
2931
const QUALIFIED = /"?\bchats"?\s*\.\s*"?messages"?/i;
3032

3133
/** An unqualified one, inside a literal that is plainly SQL against `chats`. */
@@ -74,7 +76,7 @@ function offences(file: string): string[] {
7476
return offencesForText(readFileSync(file, "utf8"), path.relative(ROOT, file));
7577
}
7678

77-
describe("the dropped chats.messages column", () => {
79+
describe("the retired chats.messages column", () => {
7880
it("is not referenced by any production source, including in raw SQL", () => {
7981
const files = SCANNED.flatMap((dir) => sourceFiles(path.join(ROOT, dir)));
8082
// A scan that found nothing to read would pass vacuously.

internal-packages/dashboard-agent-db/drizzle/0002_watches_and_chat_messages.sql

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@ CREATE TABLE "trigger_dashboard_agent"."watches" (
8686
"cadence_minutes" integer GENERATED ALWAYS AS (((spec ->> 'checkEveryMinutes')::int)) STORED
8787
);
8888
--> statement-breakpoint
89-
ALTER TABLE "trigger_dashboard_agent"."chats" DROP COLUMN IF EXISTS "messages";
90-
--> statement-breakpoint
9189
ALTER TABLE "trigger_dashboard_agent"."chats" ADD COLUMN IF NOT EXISTS "last_read_at" timestamp with time zone;
9290
--> statement-breakpoint
9391
ALTER TABLE "trigger_dashboard_agent"."chats" ADD COLUMN IF NOT EXISTS "next_message_position" integer DEFAULT 1 NOT NULL;

internal-packages/dashboard-agent-db/drizzle/meta/0002_snapshot.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,13 @@
490490
"primaryKey": false,
491491
"notNull": true,
492492
"default": "now()"
493+
},
494+
"messages": {
495+
"name": "messages",
496+
"type": "jsonb",
497+
"primaryKey": false,
498+
"notNull": true,
499+
"default": "'[]'::jsonb"
493500
}
494501
},
495502
"indexes": {

internal-packages/dashboard-agent-db/src/schema.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ export const chats = dashboardAgentSchema.table(
3030
userId: text("user_id").notNull(),
3131
title: text("title").notNull().default("New chat"),
3232
metadata: jsonb("metadata").$type<Record<string, unknown>>().notNull().default({}),
33+
/**
34+
* @deprecated The transcript lives in `chat_messages`. Declared so drizzle doesn't
35+
* offer to drop it: whatever a deployed environment already wrote stays readable
36+
* until someone decides it isn't needed. Nothing reads or writes it.
37+
*/
38+
messages: jsonb("messages").$type<unknown[]>().notNull().default([]),
3339
pinnedAt: timestamp("pinned_at", { withTimezone: true }),
3440
// NULL means never read, so everything in the chat counts as unread.
3541
lastReadAt: timestamp("last_read_at", { withTimezone: true }),

0 commit comments

Comments
 (0)