From e98b62a24c5beab61ba7817b8806f08964c5190b Mon Sep 17 00:00:00 2001 From: Dara Adedeji <76637177+SunkenInTime@users.noreply.github.com> Date: Thu, 3 Sep 2026 19:25:14 -0400 Subject: [PATCH 1/2] fix(server): verify path-bound image assets --- apps/server/src/assets/AssetAccess.test.ts | 74 ++++++++++++++++++---- apps/server/src/assets/AssetAccess.ts | 74 ++++++++++++++++------ docs/internals/environment-auth.md | 17 ++--- 3 files changed, 126 insertions(+), 39 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.test.ts b/apps/server/src/assets/AssetAccess.test.ts index 251a4761705d..f40ce4d3bb1c 100644 --- a/apps/server/src/assets/AssetAccess.test.ts +++ b/apps/server/src/assets/AssetAccess.test.ts @@ -320,7 +320,39 @@ describe("AssetAccess", () => { }).pipe(Effect.provide(testLayer)), ); - it.effect("keeps in-place edits readable but requires a new URL after atomic replacement", () => + it.effect("keeps image URLs valid after atomic replacement", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-image-replacement-" }); + const filePath = path.join(root, "preview.png"); + yield* fs.writeFileString(filePath, "original"); + const result = yield* issueAssetUrl({ + resource: { + _tag: "media-file", + threadId: ThreadId.make("thread-1"), + path: filePath, + }, + }); + const suffix = result.relativeUrl.slice(`${ASSET_ROUTE_PREFIX}/`.length); + const separator = suffix.indexOf("/"); + const token = suffix.slice(0, separator); + const name = suffix.slice(separator + 1); + + const replacement = path.join(root, "replacement.png"); + yield* fs.writeFileString(replacement, "replacement"); + yield* fs.rename(replacement, filePath); + const replaced = yield* resolveAsset(token, name); + if (!replaced) throw new Error("Expected the replacement image"); + const response = HttpServerResponse.toWeb(yield* assetFileResponse(replaced)); + expect(yield* Effect.promise(() => response.text())).toBe("replacement"); + + yield* fs.remove(filePath); + expect(yield* resolveAsset(token, name)).toBeNull(); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("keeps in-place video edits readable but requires a new URL after replacement", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -733,7 +765,7 @@ describe("AssetAccess", () => { }).pipe(Effect.provide(testLayer)), ); - it.effect("issues an exact capability for a saved favicon outside the workspace", () => + it.effect("streams a saved favicon outside the workspace from its opened descriptor", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -744,6 +776,7 @@ describe("AssetAccess", () => { prefix: "t3-asset-favicon-pictures-", }); const externalPath = path.join(pictures, "custom.png"); + const savedPath = path.join(pictures, "saved.png"); const siblingPath = path.join(pictures, "sibling.png"); yield* fileSystem.writeFile(externalPath, new Uint8Array([1, 2, 3])); yield* fileSystem.writeFile(siblingPath, new Uint8Array([4, 5, 6])); @@ -759,15 +792,29 @@ describe("AssetAccess", () => { expect(result.sourcePath).toBe(externalPath); expect(result.relativeUrl).toMatch(/\/v[0-9a-f]{64}-custom\.png$/); - expect( - yield* resolveAsset(suffix.slice(0, separatorIndex), suffix.slice(separatorIndex + 1)), - ).toEqual({ kind: "file", path: canonicalPath }); + const resolved = yield* resolveAsset( + suffix.slice(0, separatorIndex), + suffix.slice(separatorIndex + 1), + ); + expect(resolved).toMatchObject({ kind: "file", path: canonicalPath }); + expect(resolved?.file).toBeDefined(); const tamperedSuffixResult = yield* resolveAsset( suffix.slice(0, separatorIndex), "sibling.png", ); - expect(tamperedSuffixResult).toEqual({ kind: "file", path: canonicalPath }); - expect(tamperedSuffixResult).not.toEqual({ kind: "file", path: canonicalSiblingPath }); + expect(tamperedSuffixResult).toMatchObject({ kind: "file", path: canonicalPath }); + expect(tamperedSuffixResult).not.toMatchObject({ kind: "file", path: canonicalSiblingPath }); + + if (!resolved) throw new Error("Expected an external favicon"); + yield* fileSystem.rename(externalPath, savedPath); + yield* fileSystem.symlink(siblingPath, externalPath); + const response = HttpServerResponse.toWeb(yield* assetFileResponse(resolved)); + expect(new Uint8Array(yield* Effect.promise(() => response.arrayBuffer()))).toEqual( + new Uint8Array([1, 2, 3]), + ); + expect( + yield* resolveAsset(suffix.slice(0, separatorIndex), suffix.slice(separatorIndex + 1)), + ).toBeNull(); }).pipe(Effect.provide(testLayer)), ); @@ -820,13 +867,16 @@ describe("AssetAccess", () => { prefix: "t3-asset-favicon-type-", }); yield* fileSystem.writeFileString(path.join(root, "secret.txt"), "not an image"); + yield* fileSystem.symlink(path.join(root, "secret.txt"), path.join(root, "disguised.png")); - const error = yield* issueAssetUrl({ - resource: { _tag: "project-favicon", cwd: root }, - projectFaviconPath: "secret.txt", - }).pipe(Effect.flip); + for (const projectFaviconPath of ["secret.txt", "disguised.png"]) { + const error = yield* issueAssetUrl({ + resource: { _tag: "project-favicon", cwd: root }, + projectFaviconPath, + }).pipe(Effect.flip); - expect(error).toBeInstanceOf(AssetPreviewTypeValidationError); + expect(error).toBeInstanceOf(AssetPreviewTypeValidationError); + } }).pipe(Effect.provide(testLayer)), ); diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index a0d849bf603a..bbec7207c07e 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -96,6 +96,12 @@ const AssetClaimsSchema = Schema.Union([ inode: Schema.String, expiresAt: Schema.Number, }), + Schema.Struct({ + version: Schema.Literal(1), + kind: Schema.Literal("media-image-path"), + filePath: Schema.String, + expiresAt: Schema.Number, + }), Schema.Struct({ version: Schema.Literal(1), kind: Schema.Literal("attachment"), @@ -262,28 +268,35 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i if (!canonicalFile) { return yield* new AssetWorkspaceAssetNotFoundError({ resource: input.resource }); } - if (hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)) === null) { + const mimeType = hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)); + if (mimeType === null) { return yield* new AssetPreviewTypeValidationError({ resource: input.resource }); } - const identity = yield* openMediaFile(canonicalFile).pipe( - Effect.map((file) => - file ? { device: file.info.dev.toString(), inode: file.info.ino.toString() } : null, - ), + const openedFileInfo = yield* openMediaFile(canonicalFile).pipe( + Effect.map((file) => file?.info ?? null), Effect.scoped, Effect.mapError( (cause) => new AssetWorkspaceAssetInspectionError({ resource: input.resource, cause }), ), ); - if (!identity) { + if (!openedFileInfo) { return yield* new AssetWorkspaceAssetNotFoundError({ resource: input.resource }); } - claims = { - version: 1, - kind: "media-file-exact", - filePath: canonicalFile, - ...identity, - expiresAt, - }; + claims = mimeType.startsWith("image/") + ? { + version: 1, + kind: "media-image-path", + filePath: canonicalFile, + expiresAt, + } + : { + version: 1, + kind: "media-file-exact", + filePath: canonicalFile, + device: openedFileInfo.dev.toString(), + inode: openedFileInfo.ino.toString(), + expiresAt, + }; fileName = path.basename(canonicalFile); break; } @@ -462,6 +475,12 @@ export const issueAssetUrl = Effect.fn("AssetAccess.issueAssetUrl")(function* (i resource: input.resource, }); } + if ( + canonicalFaviconPath && + !hostPreviewMimeTypeFromExtension(path.extname(canonicalFaviconPath))?.startsWith("image/") + ) { + return yield* new AssetPreviewTypeValidationError({ resource: input.resource }); + } claims = isExternalOverride && canonicalFaviconPath ? { @@ -616,9 +635,21 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( ), Effect.orElseSucceed(() => null), ); - return faviconPath === claims.filePath - ? ({ kind: "file", path: faviconPath } satisfies ResolvedAsset) - : null; + if (faviconPath !== claims.filePath) return null; + const path = yield* Path.Path; + if (!hostPreviewMimeTypeFromExtension(path.extname(faviconPath))?.startsWith("image/")) { + return null; + } + const file = yield* openMediaFile(faviconPath).pipe( + Effect.tapError((cause) => + Effect.logError("Failed to open external project favicon.", { + filePath: faviconPath, + cause, + }), + ), + Effect.orElseSucceed(() => null), + ); + return file ? ({ kind: "file", path: faviconPath, file } satisfies ResolvedAsset) : null; } if (claims.kind === "native-app-icon") { @@ -630,7 +661,7 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( const decodedPath = decodeRelativePath(relativePath); if (decodedPath === null) return null; const path = yield* Path.Path; - if (claims.kind === "media-file-exact") { + if (claims.kind === "media-file-exact" || claims.kind === "media-image-path") { if (decodedPath !== path.basename(claims.filePath)) return null; const canonicalFile = yield* resolveCanonicalFile(claims.filePath).pipe( Effect.tapError((cause) => @@ -643,8 +674,13 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( ); if (canonicalFile !== claims.filePath) return null; const mimeType = hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)); - if (!mimeType) return null; - const file = yield* openMediaFile(canonicalFile, claims).pipe( + if (!mimeType || (claims.kind === "media-image-path" && !mimeType.startsWith("image/"))) { + return null; + } + const file = yield* openMediaFile( + canonicalFile, + claims.kind === "media-file-exact" ? claims : undefined, + ).pipe( Effect.tapError((cause) => Effect.logError("Failed to open canonical media file.", { filePath: canonicalFile, cause }), ), diff --git a/docs/internals/environment-auth.md b/docs/internals/environment-auth.md index 771b57e14b3b..78b0845ef9ba 100644 --- a/docs/internals/environment-auth.md +++ b/docs/internals/environment-auth.md @@ -45,14 +45,15 @@ A thread ID supplies the workspace for relative paths; absolute paths refer to t host, not the client. [`AssetAccess.ts`](../../apps/server/src/assets/AssetAccess.ts) resolves symlinks, requires a regular -file, and validates the resolved file's literal extension. It opens the file and signs its canonical -path and device/inode identity for one hour. The token grants access to that exact file, not adjacent -files or its containing directory. Serving rechecks the canonical path, media type, and opened -descriptor's identity, then streams full or partial responses from that descriptor. Replacing a -file atomically requires a freshly signed URL; editing it in place does not. Because the token names -one file, an HTML document served this way cannot load sibling assets; the directory-scoped -`workspace-file` resource remains the route for HTML inside the workspace. Uploaded attachments keep -their separate asset resource. +file, and validates the resolved file's literal extension. Image tokens sign the canonical path for +one hour so an atomic image replacement at that path keeps the same URL valid. Video, HTML, and PDF +tokens also sign the file's device/inode identity, so replacing those files requires a fresh URL. +Neither token grants access to adjacent files or the containing directory. Serving rechecks the +canonical path and media type, opens the current file without following symlinks, and streams full or +partial responses from that descriptor. External project favicons use the same descriptor-backed +path check. Because the token names one file, an HTML document served this way cannot load sibling +assets; the directory-scoped `workspace-file` resource remains the route for HTML inside the +workspace. Uploaded attachments keep their separate asset resource. Signed asset URLs are bearer credentials. Anyone who obtains a URL and can reach the environment can fetch that file until it expires. Clients should copy the authored reference, not the temporary From 6d9bbbd9124e7f1951118551b1686e3b29845054 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 4 Sep 2026 21:32:20 -0400 Subject: [PATCH 2/2] refactor(server): drop redundant serve-time extension rechecks for path-bound assets The canonical path must equal the signed path, and that path's extension was validated at issuance, so rechecking the same string at serve time cannot fail. Co-Authored-By: Claude Fable 5.1 --- apps/server/src/assets/AssetAccess.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/apps/server/src/assets/AssetAccess.ts b/apps/server/src/assets/AssetAccess.ts index bbec7207c07e..2e1ba2e17ca1 100644 --- a/apps/server/src/assets/AssetAccess.ts +++ b/apps/server/src/assets/AssetAccess.ts @@ -636,10 +636,6 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( Effect.orElseSucceed(() => null), ); if (faviconPath !== claims.filePath) return null; - const path = yield* Path.Path; - if (!hostPreviewMimeTypeFromExtension(path.extname(faviconPath))?.startsWith("image/")) { - return null; - } const file = yield* openMediaFile(faviconPath).pipe( Effect.tapError((cause) => Effect.logError("Failed to open external project favicon.", { @@ -674,9 +670,7 @@ export const resolveAsset = Effect.fn("AssetAccess.resolveAsset")(function* ( ); if (canonicalFile !== claims.filePath) return null; const mimeType = hostPreviewMimeTypeFromExtension(path.extname(canonicalFile)); - if (!mimeType || (claims.kind === "media-image-path" && !mimeType.startsWith("image/"))) { - return null; - } + if (!mimeType) return null; const file = yield* openMediaFile( canonicalFile, claims.kind === "media-file-exact" ? claims : undefined,