authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-03-01 19:01:47-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-03-01 20:06:21-08:00
logd58af33fba73936659158ba2167a00475889d1e5
tree559a9e8b6911edf3cf4968be9e2ed8123cbde5c4
parenta56418f8bc9866ab114c85b4e73a7f95987c3469

Fix attachment cleanup to match exact thread segments

- Parse thread segments from attachment IDs using a UUID-aware pattern - Prevent revert/delete cleanup from removing files belonging to similarly prefixed threads - Add tests for prefix-collision cases in attachment store and projection pipeline

4 files changed, 72 insertions(+), 9 deletions(-)

apps/server/src/attachmentStore.test.ts+14-2
...@@ -4,7 +4,11 @@ import path from "node:path";...@@ -4,7 +4,11 @@ import path from "node:path";
44
5import { describe, expect, it } from "vitest";5import { describe, expect, it } from "vitest";
66
7import { createAttachmentId, resolveAttachmentPathById } from "./attachmentStore.ts";7import {
8 createAttachmentId,
9 parseThreadSegmentFromAttachmentId,
10 resolveAttachmentPathById,
11} from "./attachmentStore.ts";
812
9describe("attachmentStore", () => {13describe("attachmentStore", () => {
10 it("sanitizes thread ids when creating attachment ids", () => {14 it("sanitizes thread ids when creating attachment ids", () => {
...@@ -14,7 +18,7 @@ describe("attachmentStore", () => {...@@ -14,7 +18,7 @@ describe("attachmentStore", () => {
14 return;18 return;
15 }19 }
1620
17 const [threadSegment] = attachmentId.split("-", 1);21 const threadSegment = parseThreadSegmentFromAttachmentId(attachmentId);
18 expect(threadSegment).toBeTruthy();22 expect(threadSegment).toBeTruthy();
19 expect(threadSegment).toMatch(/^[a-z0-9_-]+$/i);23 expect(threadSegment).toMatch(/^[a-z0-9_-]+$/i);
20 expect(threadSegment).not.toContain(".");24 expect(threadSegment).not.toContain(".");
...@@ -22,6 +26,14 @@ describe("attachmentStore", () => {...@@ -22,6 +26,14 @@ describe("attachmentStore", () => {
22 expect(threadSegment).not.toContain("/");26 expect(threadSegment).not.toContain("/");
23 });27 });
2428
29 it("parses exact thread segments from attachment ids without prefix collisions", () => {
30 const fooId = "foo-00000000-0000-4000-8000-000000000001";
31 const fooBarId = "foo-bar-00000000-0000-4000-8000-000000000002";
32
33 expect(parseThreadSegmentFromAttachmentId(fooId)).toBe("foo");
34 expect(parseThreadSegmentFromAttachmentId(fooBarId)).toBe("foo-bar");
35 });
36
25 it("resolves attachment path by id using the extension that exists on disk", () => {37 it("resolves attachment path by id using the extension that exists on disk", () => {
26 const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3code-attachment-store-"));38 const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "t3code-attachment-store-"));
27 try {39 try {
apps/server/src/attachmentStore.ts+19
...@@ -12,6 +12,13 @@ import { inferImageExtension, SAFE_IMAGE_FILE_EXTENSIONS } from "./imageMime.ts"...@@ -12,6 +12,13 @@ import { inferImageExtension, SAFE_IMAGE_FILE_EXTENSIONS } from "./imageMime.ts"
1212
13const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"];13const ATTACHMENT_FILENAME_EXTENSIONS = [...SAFE_IMAGE_FILE_EXTENSIONS, ".bin"];
14const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80;14const ATTACHMENT_ID_THREAD_SEGMENT_MAX_CHARS = 80;
15const ATTACHMENT_ID_THREAD_SEGMENT_PATTERN = "[a-z0-9_]+(?:-[a-z0-9_]+)*";
16const ATTACHMENT_ID_UUID_PATTERN =
17 "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}";
18const ATTACHMENT_ID_PATTERN = new RegExp(
19 `^(${ATTACHMENT_ID_THREAD_SEGMENT_PATTERN})-(${ATTACHMENT_ID_UUID_PATTERN})$`,
20 "i",
21);
1522
16export function toSafeThreadAttachmentSegment(threadId: string): string | null {23export function toSafeThreadAttachmentSegment(threadId: string): string | null {
17 const segment = threadId24 const segment = threadId
...@@ -35,6 +42,18 @@ export function createAttachmentId(threadId: string): string | null {...@@ -35,6 +42,18 @@ export function createAttachmentId(threadId: string): string | null {
35 return `${threadSegment}-${randomUUID()}`;42 return `${threadSegment}-${randomUUID()}`;
36}43}
3744
45export function parseThreadSegmentFromAttachmentId(attachmentId: string): string | null {
46 const normalizedId = normalizeAttachmentRelativePath(attachmentId);
47 if (!normalizedId || normalizedId.includes("/") || normalizedId.includes(".")) {
48 return null;
49 }
50 const match = normalizedId.match(ATTACHMENT_ID_PATTERN);
51 if (!match) {
52 return null;
53 }
54 return match[1]?.toLowerCase() ?? null;
55}
56
38export function attachmentRelativePath(attachment: ChatAttachment): string {57export function attachmentRelativePath(attachment: ChatAttachment): string {
39 switch (attachment.type) {58 switch (attachment.type) {
40 case "image": {59 case "image": {
apps/server/src/orchestration/Layers/ProjectionPipeline.test.ts+19-3
...@@ -734,8 +734,10 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {...@@ -734,8 +734,10 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {
734 const eventStore = yield* OrchestrationEventStore;734 const eventStore = yield* OrchestrationEventStore;
735 const now = new Date().toISOString();735 const now = new Date().toISOString();
736 const threadId = ThreadId.makeUnsafe("thread revert.files");736 const threadId = ThreadId.makeUnsafe("thread revert.files");
737 const keepAttachmentId = "thread-revert-files-keep";737 const keepAttachmentId = "thread-revert-files-00000000-0000-4000-8000-000000000001";
738 const removeAttachmentId = "thread-revert-files-remove";738 const removeAttachmentId = "thread-revert-files-00000000-0000-4000-8000-000000000002";
739 const otherThreadAttachmentId =
740 "thread-revert-files-extra-00000000-0000-4000-8000-000000000003";
739741
740 const appendAndProject = (event: Parameters<typeof eventStore.append>[0]) =>742 const appendAndProject = (event: Parameters<typeof eventStore.append>[0]) =>
741 eventStore743 eventStore
...@@ -904,8 +906,11 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {...@@ -904,8 +906,11 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {
904 fs.mkdirSync(path.join(stateDir, "attachments"), { recursive: true });906 fs.mkdirSync(path.join(stateDir, "attachments"), { recursive: true });
905 fs.writeFileSync(keepPath, Buffer.from("keep"));907 fs.writeFileSync(keepPath, Buffer.from("keep"));
906 fs.writeFileSync(removePath, Buffer.from("remove"));908 fs.writeFileSync(removePath, Buffer.from("remove"));
909 const otherThreadPath = path.join(stateDir, "attachments", `${otherThreadAttachmentId}.png`);
910 fs.writeFileSync(otherThreadPath, Buffer.from("other"));
907 assert.equal(fs.existsSync(keepPath), true);911 assert.equal(fs.existsSync(keepPath), true);
908 assert.equal(fs.existsSync(removePath), true);912 assert.equal(fs.existsSync(removePath), true);
913 assert.equal(fs.existsSync(otherThreadPath), true);
909914
910 yield* appendAndProject({915 yield* appendAndProject({
911 type: "thread.reverted",916 type: "thread.reverted",
...@@ -925,6 +930,7 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {...@@ -925,6 +930,7 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {
925930
926 assert.equal(fs.existsSync(keepPath), true);931 assert.equal(fs.existsSync(keepPath), true);
927 assert.equal(fs.existsSync(removePath), false);932 assert.equal(fs.existsSync(removePath), false);
933 assert.equal(fs.existsSync(otherThreadPath), true);
928 }).pipe(934 }).pipe(
929 (effect) => runWithProjectionPipelineLayer(stateDir, effect),935 (effect) => runWithProjectionPipelineLayer(stateDir, effect),
930 Effect.ensuring(Effect.sync(() => fs.rmSync(stateDir, { recursive: true, force: true }))),936 Effect.ensuring(Effect.sync(() => fs.rmSync(stateDir, { recursive: true, force: true }))),
...@@ -943,7 +949,9 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {...@@ -943,7 +949,9 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {
943 const eventStore = yield* OrchestrationEventStore;949 const eventStore = yield* OrchestrationEventStore;
944 const now = new Date().toISOString();950 const now = new Date().toISOString();
945 const threadId = ThreadId.makeUnsafe("thread delete.files");951 const threadId = ThreadId.makeUnsafe("thread delete.files");
946 const attachmentId = "thread-delete-files-att-1";952 const attachmentId = "thread-delete-files-00000000-0000-4000-8000-000000000001";
953 const otherThreadAttachmentId =
954 "thread-delete-files-extra-00000000-0000-4000-8000-000000000002";
947955
948 const appendAndProject = (event: Parameters<typeof eventStore.append>[0]) =>956 const appendAndProject = (event: Parameters<typeof eventStore.append>[0]) =>
949 eventStore957 eventStore
...@@ -1025,9 +1033,16 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {...@@ -1025,9 +1033,16 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {
1025 });1033 });
10261034
1027 const threadAttachmentPath = path.join(stateDir, "attachments", `${attachmentId}.png`);1035 const threadAttachmentPath = path.join(stateDir, "attachments", `${attachmentId}.png`);
1036 const otherThreadAttachmentPath = path.join(
1037 stateDir,
1038 "attachments",
1039 `${otherThreadAttachmentId}.png`,
1040 );
1028 fs.mkdirSync(path.join(stateDir, "attachments"), { recursive: true });1041 fs.mkdirSync(path.join(stateDir, "attachments"), { recursive: true });
1029 fs.writeFileSync(threadAttachmentPath, Buffer.from("delete"));1042 fs.writeFileSync(threadAttachmentPath, Buffer.from("delete"));
1043 fs.writeFileSync(otherThreadAttachmentPath, Buffer.from("other-thread"));
1030 assert.equal(fs.existsSync(threadAttachmentPath), true);1044 assert.equal(fs.existsSync(threadAttachmentPath), true);
1045 assert.equal(fs.existsSync(otherThreadAttachmentPath), true);
10311046
1032 yield* appendAndProject({1047 yield* appendAndProject({
1033 type: "thread.deleted",1048 type: "thread.deleted",
...@@ -1046,6 +1061,7 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {...@@ -1046,6 +1061,7 @@ projectionLayer("OrchestrationProjectionPipeline", (it) => {
1046 });1061 });
10471062
1048 assert.equal(fs.existsSync(threadAttachmentPath), false);1063 assert.equal(fs.existsSync(threadAttachmentPath), false);
1064 assert.equal(fs.existsSync(otherThreadAttachmentPath), true);
1049 }).pipe(1065 }).pipe(
1050 (effect) => runWithProjectionPipelineLayer(stateDir, effect),1066 (effect) => runWithProjectionPipelineLayer(stateDir, effect),
1051 Effect.ensuring(Effect.sync(() => fs.rmSync(stateDir, { recursive: true, force: true }))),1067 Effect.ensuring(Effect.sync(() => fs.rmSync(stateDir, { recursive: true, force: true }))),
apps/server/src/orchestration/Layers/ProjectionPipeline.ts+20-4
...@@ -37,7 +37,12 @@ import {...@@ -37,7 +37,12 @@ import {
37 OrchestrationProjectionPipeline,37 OrchestrationProjectionPipeline,
38 type OrchestrationProjectionPipelineShape,38 type OrchestrationProjectionPipelineShape,
39} from "../Services/ProjectionPipeline.ts";39} from "../Services/ProjectionPipeline.ts";
40import { attachmentRelativePath, toSafeThreadAttachmentSegment } from "../../attachmentStore.ts";40import {
41 attachmentRelativePath,
42 parseAttachmentIdFromRelativePath,
43 parseThreadSegmentFromAttachmentId,
44 toSafeThreadAttachmentSegment,
45} from "../../attachmentStore.ts";
4146
42export const ORCHESTRATION_PROJECTOR_NAMES = {47export const ORCHESTRATION_PROJECTOR_NAMES = {
43 projects: "projection.projects",48 projects: "projection.projects",
...@@ -198,7 +203,8 @@ function collectThreadAttachmentRelativePaths(...@@ -198,7 +203,8 @@ function collectThreadAttachmentRelativePaths(
198 if (attachment.type !== "image") {203 if (attachment.type !== "image") {
199 continue;204 continue;
200 }205 }
201 if (!attachment.id.startsWith(`${threadSegment}-`)) {206 const attachmentThreadSegment = parseThreadSegmentFromAttachmentId(attachment.id);
207 if (!attachmentThreadSegment || attachmentThreadSegment !== threadSegment) {
202 continue;208 continue;
203 }209 }
204 relativePaths.add(attachmentRelativePath(attachment));210 relativePaths.add(attachmentRelativePath(attachment));
...@@ -236,7 +242,12 @@ const runAttachmentSideEffects = Effect.fn(function* (sideEffects: AttachmentSid...@@ -236,7 +242,12 @@ const runAttachmentSideEffects = Effect.fn(function* (sideEffects: AttachmentSid
236 if (normalizedEntry.length === 0 || normalizedEntry.includes("/")) {242 if (normalizedEntry.length === 0 || normalizedEntry.includes("/")) {
237 return;243 return;
238 }244 }
239 if (!normalizedEntry.startsWith(`${threadSegment}-`)) {245 const attachmentId = parseAttachmentIdFromRelativePath(normalizedEntry);
246 if (!attachmentId) {
247 return;
248 }
249 const attachmentThreadSegment = parseThreadSegmentFromAttachmentId(attachmentId);
250 if (!attachmentThreadSegment || attachmentThreadSegment !== threadSegment) {
240 return;251 return;
241 }252 }
242 yield* fileSystem.remove(path.join(attachmentsRootDir, normalizedEntry), {253 yield* fileSystem.remove(path.join(attachmentsRootDir, normalizedEntry), {
...@@ -272,7 +283,12 @@ const runAttachmentSideEffects = Effect.fn(function* (sideEffects: AttachmentSid...@@ -272,7 +283,12 @@ const runAttachmentSideEffects = Effect.fn(function* (sideEffects: AttachmentSid
272 if (relativePath.length === 0 || relativePath.includes("/")) {283 if (relativePath.length === 0 || relativePath.includes("/")) {
273 return;284 return;
274 }285 }
275 if (!relativePath.startsWith(`${threadSegment}-`)) {286 const attachmentId = parseAttachmentIdFromRelativePath(relativePath);
287 if (!attachmentId) {
288 return;
289 }
290 const attachmentThreadSegment = parseThreadSegmentFromAttachmentId(attachmentId);
291 if (!attachmentThreadSegment || attachmentThreadSegment !== threadSegment) {
276 return;292 return;
277 }293 }
278294