diff --git a/packages/api/src/services/terminal-image-fetch-core.ts b/packages/api/src/services/terminal-image-fetch-core.ts index 5eb53c6c..b68d17ec 100644 --- a/packages/api/src/services/terminal-image-fetch-core.ts +++ b/packages/api/src/services/terminal-image-fetch-core.ts @@ -103,7 +103,21 @@ const normalizeTerminalImagePath = (path: string): TerminalImagePathNormalizatio } } -export const planTerminalImageFetch = (path: string): TerminalImageFetchPlan => { +export type TerminalImageFetchOptions = { + readonly baseDir?: string +} + +const isAbsolutePosixPath = (value: string): boolean => value.startsWith("/") + +const joinBaseDirAndRelativePath = (baseDir: string, relativePath: string): string => { + const trimmedBase = baseDir.replace(/\/+$/u, "") + return `${trimmedBase}/${relativePath}` +} + +export const planTerminalImageFetch = ( + path: string, + options: TerminalImageFetchOptions = {} +): TerminalImageFetchPlan => { if (typeof path !== "string" || path.length === 0) { return { _tag: "InvalidTerminalImageFetch", message: "Image path is required." } } @@ -111,9 +125,17 @@ export const planTerminalImageFetch = (path: string): TerminalImageFetchPlan => if (normalized._tag === "InvalidTerminalImagePath") { return { _tag: "InvalidTerminalImageFetch", message: normalized.message } } - const containerPath = normalized.path - if (!containerPath.startsWith("/")) { - return { _tag: "InvalidTerminalImageFetch", message: "Image path must be absolute." } + const normalizedPath = normalized.path + let containerPath = normalizedPath + if (!isAbsolutePosixPath(containerPath)) { + const baseDir = options.baseDir + if (baseDir === undefined || !isAbsolutePosixPath(baseDir)) { + return { _tag: "InvalidTerminalImageFetch", message: "Image path must be absolute." } + } + if (invalidCharacterPattern.test(baseDir) || traversalPattern.test(baseDir)) { + return { _tag: "InvalidTerminalImageFetch", message: "Image base directory is invalid." } + } + containerPath = joinBaseDirAndRelativePath(baseDir, containerPath) } if (invalidCharacterPattern.test(containerPath)) { return { _tag: "InvalidTerminalImageFetch", message: "Image path contains invalid characters." } diff --git a/packages/api/src/services/terminal-sessions.ts b/packages/api/src/services/terminal-sessions.ts index 021bf14d..de8f9472 100644 --- a/packages/api/src/services/terminal-sessions.ts +++ b/packages/api/src/services/terminal-sessions.ts @@ -59,6 +59,7 @@ type TerminalRecord = { projectDisplayName: string projectId: string projectKey: string + projectTargetDir: string prepared: ReturnType } @@ -581,7 +582,8 @@ const registerRecord = ( projectKey: string, projectDisplayName: string, prepared: ReturnType, - projectContainerName: string + projectContainerName: string, + projectTargetDir: string ): TerminalSession => { const session: TerminalSession = { attachedClients: 0, @@ -600,6 +602,7 @@ const registerRecord = ( projectDisplayName, projectId, projectKey, + projectTargetDir, pty: null, session, sockets: new Set() @@ -662,7 +665,8 @@ export const createTerminalSession = ( project.projectKey, project.displayName, prepared, - projectItem.containerName + projectItem.containerName, + projectItem.targetDir ) yield* _(emitTerminalSessionCreated(projectId, session.id, options.requestId)) return { project, session } @@ -681,7 +685,8 @@ export const createTerminalSession = ( project.projectKey, project.displayName, prepared, - reachableProjectItem.containerName + reachableProjectItem.containerName, + reachableProjectItem.targetDir ) yield* _(emitTerminalSessionCreated(projectId, session.id, options.requestId)) yield* _(emitTerminalStatus(projectId, "ssh.post-start", "Post-start self-heal continues in background")) @@ -786,7 +791,7 @@ export const readProjectTerminalImage = ( Effect.fail(new ApiNotFoundError({ message: `Terminal session not found: ${sessionId}` })) ) } - const plan = planTerminalImageFetch(imagePath) + const plan = planTerminalImageFetch(imagePath, { baseDir: record.projectTargetDir }) if (plan._tag === "InvalidTerminalImageFetch") { return yield* _(Effect.fail(new ApiBadRequestError({ message: plan.message }))) } diff --git a/packages/api/tests/terminal-image-fetch-core.test.ts b/packages/api/tests/terminal-image-fetch-core.test.ts index a9f38b39..eebe5764 100644 --- a/packages/api/tests/terminal-image-fetch-core.test.ts +++ b/packages/api/tests/terminal-image-fetch-core.test.ts @@ -103,4 +103,53 @@ describe("terminal image fetch core", () => { _tag: "InvalidTerminalImageFetch" }) }) + + it("resolves a relative path against the provided absolute base directory", () => { + expect( + planTerminalImageFetch( + "app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png", + { baseDir: "/home/dev" } + ) + ).toEqual({ + _tag: "ValidTerminalImageFetch", + containerPath: "/home/dev/app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png", + mediaType: "image/png" + }) + }) + + it("ignores trailing slashes on the base directory when resolving a relative path", () => { + expect(planTerminalImageFetch("photos/cover.jpg", { baseDir: "/home/dev/" })).toEqual({ + _tag: "ValidTerminalImageFetch", + containerPath: "/home/dev/photos/cover.jpg", + mediaType: "image/jpeg" + }) + }) + + it("still rejects relative paths when no base directory is supplied", () => { + expect(planTerminalImageFetch("app/cover.png")).toEqual({ + _tag: "InvalidTerminalImageFetch", + message: "Image path must be absolute." + }) + }) + + it("rejects relative paths when the supplied base directory is not absolute", () => { + expect(planTerminalImageFetch("app/cover.png", { baseDir: "home/dev" })).toEqual({ + _tag: "InvalidTerminalImageFetch", + message: "Image path must be absolute." + }) + }) + + it("rejects an unsafe base directory containing traversal segments", () => { + expect(planTerminalImageFetch("cover.png", { baseDir: "/home/dev/../etc" })).toEqual({ + _tag: "InvalidTerminalImageFetch", + message: "Image base directory is invalid." + }) + }) + + it("rejects relative paths that contain traversal segments after resolution", () => { + expect(planTerminalImageFetch("../etc/cover.png", { baseDir: "/home/dev" })).toMatchObject({ + _tag: "InvalidTerminalImageFetch", + message: "Image path must not contain '.' or '..' segments." + }) + }) }) diff --git a/packages/app/src/web/terminal-image-paths.ts b/packages/app/src/web/terminal-image-paths.ts index 903ddf23..dc8be54c 100644 --- a/packages/app/src/web/terminal-image-paths.ts +++ b/packages/app/src/web/terminal-image-paths.ts @@ -8,6 +8,13 @@ const imagePathPattern = new RegExp( "giu" ) +const treePointerImagePathSource = + String.raw`[^\s"'(<>\[\]{}|\\/][^\s"'(<>\[\]{}|\\]*\.(?:${extensionAlternation})` +const treePointerImagePathPattern = new RegExp( + String.raw`(?:^|\s)[└├]\s+(${treePointerImagePathSource})(?=$|[\s"')<>\[\]{}|.,;:?!])`, + "giu" +) + const escapeChar = String.fromCodePoint(0x1B) const bellChar = String.fromCodePoint(0x07) @@ -26,22 +33,38 @@ export type TerminalImagePathMatch = { export const stripTerminalAnsi = (text: string): string => text.replace(ansiOscPattern, "").replace(ansiCsiPattern, "").replace(ansiOtherEscapePattern, "") -export const detectTerminalImagePathMatches = (text: string): ReadonlyArray => { - const plainText = stripTerminalAnsi(text) - const matches: Array = [] - for (const match of plainText.matchAll(imagePathPattern)) { +const collectPatternMatches = ( + plainText: string, + pattern: RegExp, + matches: Array, + seenStartIndices: Set +): void => { + for (const match of plainText.matchAll(pattern)) { const candidate = match[1] - if (candidate !== undefined && candidate.length > 0) { - const fullMatch = match[0] - const fullStartIndex = match.index - const startIndex = fullStartIndex + fullMatch.lastIndexOf(candidate) - matches.push({ - endIndex: startIndex + candidate.length, - path: candidate, - startIndex - }) + if (candidate === undefined || candidate.length === 0) { + continue } + const fullMatch = match[0] + const fullStartIndex = match.index + const startIndex = fullStartIndex + fullMatch.lastIndexOf(candidate) + if (seenStartIndices.has(startIndex)) { + continue + } + seenStartIndices.add(startIndex) + matches.push({ + endIndex: startIndex + candidate.length, + path: candidate, + startIndex + }) } +} + +export const detectTerminalImagePathMatches = (text: string): ReadonlyArray => { + const plainText = stripTerminalAnsi(text) + const matches: Array = [] + const seenStartIndices = new Set() + collectPatternMatches(plainText, imagePathPattern, matches, seenStartIndices) + collectPatternMatches(plainText, treePointerImagePathPattern, matches, seenStartIndices) return matches } diff --git a/packages/app/tests/docker-git/terminal-image-paths.test.ts b/packages/app/tests/docker-git/terminal-image-paths.test.ts index ec30dc3b..f999ac27 100644 --- a/packages/app/tests/docker-git/terminal-image-paths.test.ts +++ b/packages/app/tests/docker-git/terminal-image-paths.test.ts @@ -98,4 +98,37 @@ describe("terminal image path detection", () => { expect(isSupportedTerminalImagePath("/var/data/a.bmp")).toBe(false) expect(isSupportedTerminalImagePath("/var/data/a.txt")).toBe(false) }) + + it("detects relative image paths under a tree pointer (issue 261 viewed image format)", () => { + const text = " └ app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png" + expect(detectTerminalImagePaths(text)).toEqual([ + "app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png" + ]) + }) + + it("detects relative image paths under the alternative branch tree pointer", () => { + expect(detectTerminalImagePaths(" ├ assets/cover.jpg")).toEqual(["assets/cover.jpg"]) + }) + + it("detects multiple viewed images across separate lines", () => { + const text = [ + "• Viewed Image", + " └ app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png", + "", + "• Viewed Image", + " └ app/docs/screenshots/issue-237/proof/pr238-proof-09-skiller-submodule-checks.png" + ].join("\n") + expect(detectTerminalImagePaths(text)).toEqual([ + "app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png", + "app/docs/screenshots/issue-237/proof/pr238-proof-09-skiller-submodule-checks.png" + ]) + }) + + it("does not duplicate absolute paths preceded by a tree pointer", () => { + expect(detectTerminalImagePaths(" └ /var/data/foo.png")).toEqual(["/var/data/foo.png"]) + }) + + it("ignores tree pointers that do not precede an image", () => { + expect(detectTerminalImagePaths(" └ Viewed File: notes.txt")).toEqual([]) + }) })