Merge pull request #89 from copenhagentruckwash/fix-unvalidated-attachment-download-links

Validate self-serve attachment download links

src/composables/useSelfServeLogic.js

@@ -1,5 +1,6 @@
 import { computed, ref } from "vue";
 import { SessionUser } from "@/components/session/token/SessionUser.vue";
+import { safeAttachmentDownloadLink } from "@/services/attachmentDownloadLinks.js";
 
 const normalizeBooleanAnswer = (value) => {
   if (value === true || value === false) {
@@ -526,7 +527,7 @@ export function useSelfServeLogic() {
   };
 
   const downloadAttachment = async (taskId, attachmentId, attachment = null) => {
-    const existingDownloadLink = attachment?.download_link || null;
+    const existingDownloadLink = safeAttachmentDownloadLink(attachment?.download_link);
     if (existingDownloadLink) {
       window.open(existingDownloadLink, "_blank", "noopener");
       return;
@@ -534,10 +535,9 @@ export function useSelfServeLogic() {
 
     try {
       const response = await SessionUser.objects.self_serve_tasks.attachments.download(taskId, attachmentId);
-      if (response?.data?.download_link) {
-        window.open(response.data.download_link, "_blank", "noopener");
-      } else if (response?.download_link) {
-        window.open(response.download_link, "_blank", "noopener");
+      const downloadLink = safeAttachmentDownloadLink(response?.data?.download_link || response?.download_link);
+      if (downloadLink) {
+        window.open(downloadLink, "_blank", "noopener");
       }
     } catch (error) {
       console.error("Error downloading attachment:", error);

src/config.js

@@ -55,6 +55,12 @@ export const RELEASE_TRUSTED_ORIGINS = String(import.meta.env.VITE_RELEASE_TRUST
   .split(",")
   .map((origin) => origin.trim().replace(/\/+$/, ""))
   .filter(Boolean);
+export const SELF_SERVE_ATTACHMENT_TRUSTED_ORIGINS = String(
+  import.meta.env.VITE_SELF_SERVE_ATTACHMENT_TRUSTED_ORIGINS || ""
+)
+  .split(",")
+  .map((origin) => origin.trim().replace(/\/+$/, ""))
+  .filter(Boolean);
 
 // Allowed origins
 export const ALLOWED_ORIGINS = [

src/services/attachmentDownloadLinks.js

@@ -0,0 +1,55 @@
+import { SELF_SERVE_ATTACHMENT_TRUSTED_ORIGINS } from "@/config.js";
+import { trustedReleaseOrigins } from "@/services/releaseTrust.js";
+
+const normalizeOrigin = (value) => {
+  const raw = String(value || "").trim();
+  if (!raw) {
+    return "";
+  }
+  try {
+    return new URL(raw).origin;
+  } catch {
+    return "";
+  }
+};
+
+const isLocalHttpOrigin = (url) =>
+  url.protocol === "http:" && ["localhost", "127.0.0.1", "::1"].includes(url.hostname);
+
+const attachmentTrustedOrigins = () =>
+  Array.from(
+    new Set(
+      [...trustedReleaseOrigins(), ...SELF_SERVE_ATTACHMENT_TRUSTED_ORIGINS]
+        .map(normalizeOrigin)
+        .filter(Boolean)
+    )
+  );
+
+export const isSafeAttachmentDownloadLink = (value) => {
+  const raw = String(value || "").trim();
+  if (!raw || raw.startsWith("//")) {
+    return false;
+  }
+
+  if (raw.startsWith("/")) {
+    return true;
+  }
+
+  let url;
+  try {
+    url = new URL(raw);
+  } catch {
+    return false;
+  }
+
+  if (url.protocol !== "https:" && !isLocalHttpOrigin(url)) {
+    return false;
+  }
+
+  return attachmentTrustedOrigins().includes(url.origin);
+};
+
+export const safeAttachmentDownloadLink = (value) => {
+  const raw = String(value || "").trim();
+  return isSafeAttachmentDownloadLink(raw) ? raw : "";
+};

src/views/dashboards/departmentDashboard/modules/self-serve/DepartmentSelfServeStudio.vue

@@ -18,6 +18,7 @@ import {
   isSearching as isSearchingCustomers,
 } from "@/components/search/economic/customerSearch.vue";
 import { resolveReleaseApiUrl } from "@/services/releaseTimeline.js";
+import { safeAttachmentDownloadLink } from "@/services/attachmentDownloadLinks.js";
 import {
   buildSelfServeDynamicImageUrl,
   getSelfServeCompletedDynamicImageStep,
@@ -3158,14 +3159,14 @@ const inspectorTaskDynamicImageUnavailableReason = computed(() => {
 });
 
 const openTaskAttachment = async (attachment, taskId = null) => {
-  let downloadLink = String(attachment?.download_link || "").trim();
+  let downloadLink = safeAttachmentDownloadLink(attachment?.download_link);
   if (!downloadLink && taskId && attachment?.id) {
     try {
       const payload = await requestGet("/department/selfserve/tasks/attachments/download", {
         task_id: taskId,
         attachment_id: attachment.id,
       });
-      downloadLink = String(payload?.download_link || "").trim();
+      downloadLink = safeAttachmentDownloadLink(payload?.download_link);
     } catch (error) {
       withErrorToast(error, "Attachment download could not be prepared.");
       return;
@@ -7054,7 +7055,7 @@ onBeforeUnmount(() => {
                                   class="studio-debug-attachment"
                                   :disabled="!attachment.download_link"
                                   :title="taskAttachmentLabel(attachment)"
-                                  @click.stop="openTaskAttachment(attachment)"
+                                  @click.stop="openTaskAttachment(attachment, task.id)"
                                 >
                                   <span class="icon is-small"><i :class="taskAttachmentIcon(attachment)"></i></span>
                                   <span>{{ taskAttachmentLabel(attachment) }}</span>
@@ -7512,7 +7513,7 @@ onBeforeUnmount(() => {
                               class="studio-node-attachment"
                               :disabled="!attachment.download_link"
                               :title="taskAttachmentLabel(attachment)"
-                              @click.stop="openTaskAttachment(attachment)"
+                              @click.stop="openTaskAttachment(attachment, data.object_id)"
                               @mousedown.stop
                             >
                               <span class="icon is-small"><i :class="taskAttachmentIcon(attachment)"></i></span>
@@ -9838,7 +9839,7 @@ onBeforeUnmount(() => {
                             class="studio-debug-attachment"
                             :disabled="!attachment.download_link"
                             :title="taskAttachmentLabel(attachment)"
-                            @click.stop="openTaskAttachment(attachment)"
+                            @click.stop="openTaskAttachment(attachment, task.id)"
                           >
                             <span class="icon is-small"><i :class="taskAttachmentIcon(attachment)"></i></span>
                             <span>{{ taskAttachmentLabel(attachment) }}</span>

tests/unit/attachment-download-links.spec.js

@@ -0,0 +1,16 @@
+import { describe, expect, it } from "vitest";
+import { isSafeAttachmentDownloadLink, safeAttachmentDownloadLink } from "@/services/attachmentDownloadLinks.js";
+
+describe("attachmentDownloadLinks", () => {
+  it("allows relative and trusted HTTPS attachment URLs", () => {
+    expect(isSafeAttachmentDownloadLink("/department/selfserve/tasks/attachments/download?token=abc")).toBe(true);
+    expect(isSafeAttachmentDownloadLink("https://api-v2.truckwash.io/master/api/attachments/123")).toBe(true);
+  });
+
+  it("rejects scriptable, protocol-relative, and untrusted attachment URLs", () => {
+    expect(safeAttachmentDownloadLink("javascript:alert(document.domain)")).toBe("");
+    expect(safeAttachmentDownloadLink("data:text/html,<script>alert(document.domain)</script>")).toBe("");
+    expect(safeAttachmentDownloadLink("//evil.example/attachment.pdf")).toBe("");
+    expect(safeAttachmentDownloadLink("https://evil.example/attachment.pdf")).toBe("");
+  });
+});

tests/unit/self-serve-studio-task-scope.spec.js

@@ -99,7 +99,8 @@ describe("self-serve studio task editing", () => {
     expect(source).toContain('data-testid="studio-task-attachment-manager"');
     expect(source).toContain('data-testid="studio-task-attachment-upload"');
     expect(source).toContain('class="studio-debug-attachment-list"');
-    expect(source).toContain('@click.stop="openTaskAttachment(attachment)"');
+    expect(source).toContain('@click.stop="openTaskAttachment(attachment, data.object_id)"');
+    expect(source).toContain('@click.stop="openTaskAttachment(attachment, task.id)"');
     expect(source).toContain("delete data.attachments");
   });
 

tests/unit/use-self-serve-logic.spec.js

@@ -137,12 +137,32 @@ describe("useSelfServeLogic", () => {
 
     await logic.downloadAttachment(5, 202, {
       id: 202,
-      download_link: "https://cdn.example.test/photo.jpg",
+      download_link: "https://api-v2.truckwash.io/master/api/attachments/photo.jpg",
       content: { other: "photo.jpg" },
     });
 
     expect(mocks.attachmentsDownload).not.toHaveBeenCalled();
-    expect(openSpy).toHaveBeenCalledWith("https://cdn.example.test/photo.jpg", "_blank", "noopener");
+    expect(openSpy).toHaveBeenCalledWith(
+      "https://api-v2.truckwash.io/master/api/attachments/photo.jpg",
+      "_blank",
+      "noopener"
+    );
+  });
+
+  it("does not open untrusted embedded task attachment links", async () => {
+    const openSpy = vi.fn();
+    vi.stubGlobal("window", { open: openSpy });
+    mocks.attachmentsDownload.mockResolvedValue({ download_link: "javascript:alert(document.domain)" });
+    const logic = useSelfServeLogic();
+
+    await logic.downloadAttachment(5, 202, {
+      id: 202,
+      download_link: "data:text/html,<script>alert(document.domain)</script>",
+      content: { other: "photo.jpg" },
+    });
+
+    expect(mocks.attachmentsDownload).toHaveBeenCalledWith(5, 202);
+    expect(openSpy).not.toHaveBeenCalled();
   });
 
   it("falls back to lane/reg summary when session summary does not include questions", async () => {