fix(frontend): encode artifact URL path segments (#4278)

* fix(frontend): encode artifact URL path segments

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* fix(frontend): preserve markdown artifact URL suffixes

---------

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

frontend/src/components/workspace/messages/markdown-link.tsx

@@ -1,6 +1,6 @@
 import type { AnchorHTMLAttributes } from "react";
 
-import { resolveArtifactURL } from "@/core/artifacts/utils";
+import { resolveMarkdownArtifactURL } from "@/core/artifacts/utils";
 import { cn } from "@/lib/utils";
 
 import { CitationLink } from "../citations/citation-link";
@@ -34,7 +34,7 @@ export function createMarkdownLinkComponent(threadId?: string) {
       return (
         <a
           {...props}
-          href={resolveArtifactURL(href, threadId)}
+          href={resolveMarkdownArtifactURL(href, threadId)}
           target="_blank"
           rel="noopener noreferrer"
         />

frontend/src/core/artifacts/utils.ts

@@ -4,6 +4,33 @@ import type { AgentThreadState } from "../threads";
 
 const EMPTY_ARTIFACT_PATHS: readonly string[] = [];
 
+function decodePathSegment(segment: string) {
+  try {
+    return decodeURIComponent(segment);
+  } catch {
+    return segment;
+  }
+}
+
+function splitPathSuffix(src: string) {
+  const [path = ""] = src.split(/[?#]/, 1);
+  return {
+    path,
+    suffix: src.slice(path.length),
+  };
+}
+
+function encodeArtifactPath(filepath: string) {
+  return filepath
+    .split("/")
+    .map((segment) => encodeURIComponent(decodePathSegment(segment)))
+    .join("/");
+}
+
+function decodeRelativeArtifactPath(filepath: string) {
+  return filepath.split("/").map(decodePathSegment).join("/");
+}
+
 export function urlOfArtifact({
   filepath,
   threadId,
@@ -18,10 +45,12 @@ export function urlOfArtifact({
   if (isStaticWebsiteOnly()) {
     return staticDemoArtifactURL({ filepath, threadId, download });
   }
+  const encodedThreadId = encodeURIComponent(threadId);
+  const encodedFilepath = encodeArtifactPath(filepath);
   if (isMock) {
-    return `${getBackendBaseURL()}/mock/api/threads/${threadId}/artifacts${filepath}${download ? "?download=true" : ""}`;
+    return `${getBackendBaseURL()}/mock/api/threads/${encodedThreadId}/artifacts${encodedFilepath}${download ? "?download=true" : ""}`;
   }
-  return `${getBackendBaseURL()}/api/threads/${threadId}/artifacts${filepath}${download ? "?download=true" : ""}`;
+  return `${getBackendBaseURL()}/api/threads/${encodedThreadId}/artifacts${encodedFilepath}${download ? "?download=true" : ""}`;
 }
 
 export function extractArtifactsFromThread(thread: {
@@ -34,7 +63,12 @@ export function resolveArtifactURL(absolutePath: string, threadId: string) {
   if (isStaticWebsiteOnly()) {
     return staticDemoArtifactURL({ filepath: absolutePath, threadId });
   }
-  return `${getBackendBaseURL()}/api/threads/${threadId}/artifacts${absolutePath}`;
+  return `${getBackendBaseURL()}/api/threads/${encodeURIComponent(threadId)}/artifacts${encodeArtifactPath(absolutePath)}`;
+}
+
+export function resolveMarkdownArtifactURL(src: string, threadId: string) {
+  const { path, suffix } = splitPathSuffix(src);
+  return `${resolveArtifactURL(path, threadId)}${suffix}`;
 }
 
 export function resolveMessageImageURL(
@@ -43,11 +77,12 @@ export function resolveMessageImageURL(
   artifactPaths: readonly string[],
 ) {
   if (src.startsWith("/mnt/")) {
-    return resolveArtifactURL(src, threadId);
+    return resolveMarkdownArtifactURL(src, threadId);
   }
 
-  const [relativePath = ""] = src.split(/[?#]/, 1);
+  const { path: relativePath, suffix } = splitPathSuffix(src);
   const normalizedPath = relativePath.replace(/^(?:\.\/)+/, "");
+  const decodedNormalizedPath = decodeRelativeArtifactPath(normalizedPath);
   if (
     !normalizedPath ||
     normalizedPath.startsWith("/") ||
@@ -59,13 +94,13 @@ export function resolveMessageImageURL(
   }
 
   const matches = artifactPaths.filter((path) =>
-    path.endsWith(`/${normalizedPath}`),
+    path.endsWith(`/${decodedNormalizedPath}`),
   );
   if (matches.length !== 1) {
     return src;
   }
 
-  return `${resolveArtifactURL(matches[0]!, threadId)}${src.slice(relativePath.length)}`;
+  return `${resolveArtifactURL(matches[0]!, threadId)}${suffix}`;
 }
 
 function staticDemoArtifactURL({
@@ -77,6 +112,6 @@ function staticDemoArtifactURL({
   threadId: string;
   download?: boolean;
 }) {
-  const demoPath = filepath.replace(/^\/mnt\//, "/");
-  return `${getBackendBaseURL()}/demo/threads/${threadId}${demoPath}${download ? "?download=true" : ""}`;
+  const demoPath = encodeArtifactPath(filepath.replace(/^\/mnt\//, "/"));
+  return `${getBackendBaseURL()}/demo/threads/${encodeURIComponent(threadId)}${demoPath}${download ? "?download=true" : ""}`;
 }

frontend/tests/unit/core/artifacts/utils.test.ts

@@ -74,6 +74,79 @@ describe("artifact URL helpers", () => {
     ).toBe("/demo/threads/thread-1/user-data/outputs/style.css");
   });
 
+  test("encodes reserved characters in artifact URL path segments", async () => {
+    const { resolveArtifactURL, urlOfArtifact } =
+      await loadFreshArtifactUtils();
+
+    expect(
+      urlOfArtifact({
+        filepath: "/mnt/user-data/outputs/a#b?.txt",
+        threadId: "thread #1",
+        download: true,
+      }),
+    ).toBe(
+      "/api/threads/thread%20%231/artifacts/mnt/user-data/outputs/a%23b%3F.txt?download=true",
+    );
+    expect(
+      urlOfArtifact({
+        filepath: "/mnt/user-data/outputs/a#b?.txt",
+        threadId: "thread #1",
+        isMock: true,
+      }),
+    ).toBe(
+      "/mock/api/threads/thread%20%231/artifacts/mnt/user-data/outputs/a%23b%3F.txt",
+    );
+    expect(
+      resolveArtifactURL("/mnt/user-data/outputs/中 文#?.png", "thread #1"),
+    ).toBe(
+      "/api/threads/thread%20%231/artifacts/mnt/user-data/outputs/%E4%B8%AD%20%E6%96%87%23%3F.png",
+    );
+    expect(
+      resolveArtifactURL("/mnt/user-data/outputs/a%23b%3F.txt", "thread-1"),
+    ).toBe(
+      "/api/threads/thread-1/artifacts/mnt/user-data/outputs/a%23b%3F.txt",
+    );
+  });
+
+  test("preserves markdown query and fragment suffixes on artifact URLs", async () => {
+    const { resolveMarkdownArtifactURL, resolveMessageImageURL } =
+      await loadFreshArtifactUtils();
+
+    expect(
+      resolveMarkdownArtifactURL(
+        "/mnt/user-data/outputs/chart.png?v=2#detail",
+        "thread-1",
+      ),
+    ).toBe(
+      "/api/threads/thread-1/artifacts/mnt/user-data/outputs/chart.png?v=2#detail",
+    );
+    expect(
+      resolveMessageImageURL(
+        "/mnt/user-data/outputs/a%23b%3F.png?v=2#detail",
+        "thread-1",
+        [],
+      ),
+    ).toBe(
+      "/api/threads/thread-1/artifacts/mnt/user-data/outputs/a%23b%3F.png?v=2#detail",
+    );
+  });
+
+  test("encodes reserved characters in static demo artifact URLs", async () => {
+    setEnv("NEXT_PUBLIC_STATIC_WEBSITE_ONLY", "true");
+
+    const { urlOfArtifact } = await loadFreshArtifactUtils();
+
+    expect(
+      urlOfArtifact({
+        filepath: "/mnt/user-data/outputs/a#b?.txt",
+        threadId: "thread #1",
+        download: true,
+      }),
+    ).toBe(
+      "/demo/threads/thread%20%231/user-data/outputs/a%23b%3F.txt?download=true",
+    );
+  });
+
   test("returns stable artifact path references", async () => {
     const { extractArtifactsFromThread } = await loadFreshArtifactUtils();
     const threadWithoutArtifacts = { values: {} };
@@ -93,6 +166,7 @@ describe("artifact URL helpers", () => {
       "/mnt/user-data/outputs/aws-agent-overview.png",
       "/mnt/user-data/outputs/aws-agent-console-config.png",
       "/mnt/user-data/outputs/chart.png",
+      "/mnt/user-data/outputs/a#b?.png",
     ];
 
     expect(
@@ -119,6 +193,11 @@ describe("artifact URL helpers", () => {
     expect(
       resolveMessageImageURL("outputs/chart.png", "thread-1", artifacts),
     ).toBe("/api/threads/thread-1/artifacts/mnt/user-data/outputs/chart.png");
+    expect(
+      resolveMessageImageURL("a%23b%3F.png#detail", "thread-1", artifacts),
+    ).toBe(
+      "/api/threads/thread-1/artifacts/mnt/user-data/outputs/a%23b%3F.png#detail",
+    );
   });
 
   test("does not rewrite unregistered, ambiguous, or external message images", async () => {