Skip to content

Commit dfe3f18

Browse files
committed
feat(terminal): preview images shown by codex viewed image output
Issue 261 reports that the inline terminal image preview added in PR 241 does not trigger for the "Viewed Image" output that the Codex CLI prints as a tree pointer line followed by a relative path: • Viewed Image └ app/docs/screenshots/issue-237/proof/pr238-proof-...png The detector previously only matched absolute paths starting with "/", and the api fetch validator rejected anything that was not absolute. This change adds a second detection pass anchored on the tree pointer glyphs `└` and `├` and a relative path with a supported image extension. Detection deduplicates against the existing absolute-path pass so plain absolute paths preceded by a pointer are not double-counted. The api validator `planTerminalImageFetch` now accepts an optional `baseDir` and resolves bare relative paths against it. Terminal sessions remember the project's target dir (e.g. `/home/dev/app`) and pass it as the base directory when fetching an image, so the relative path printed by codex resolves to the same file the user is looking at. All previous safety checks still apply: the base dir itself must be absolute and free of traversal/control characters, and the joined path is re-validated for traversal segments and unsafe characters before any docker exec runs. Tests cover the detection and validator changes including the issue 261 reproduction, alternative `├` pointer, multiple consecutive viewed images, no-base-dir backwards compatibility, and unsafe-base rejection. Refs #261
1 parent 80e4a5d commit dfe3f18

5 files changed

Lines changed: 153 additions & 21 deletions

File tree

packages/api/src/services/terminal-image-fetch-core.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,39 @@ const normalizeTerminalImagePath = (path: string): TerminalImagePathNormalizatio
103103
}
104104
}
105105

106-
export const planTerminalImageFetch = (path: string): TerminalImageFetchPlan => {
106+
export type TerminalImageFetchOptions = {
107+
readonly baseDir?: string
108+
}
109+
110+
const isAbsolutePosixPath = (value: string): boolean => value.startsWith("/")
111+
112+
const joinBaseDirAndRelativePath = (baseDir: string, relativePath: string): string => {
113+
const trimmedBase = baseDir.replace(/\/+$/u, "")
114+
return `${trimmedBase}/${relativePath}`
115+
}
116+
117+
export const planTerminalImageFetch = (
118+
path: string,
119+
options: TerminalImageFetchOptions = {}
120+
): TerminalImageFetchPlan => {
107121
if (typeof path !== "string" || path.length === 0) {
108122
return { _tag: "InvalidTerminalImageFetch", message: "Image path is required." }
109123
}
110124
const normalized = normalizeTerminalImagePath(path)
111125
if (normalized._tag === "InvalidTerminalImagePath") {
112126
return { _tag: "InvalidTerminalImageFetch", message: normalized.message }
113127
}
114-
const containerPath = normalized.path
115-
if (!containerPath.startsWith("/")) {
116-
return { _tag: "InvalidTerminalImageFetch", message: "Image path must be absolute." }
128+
const normalizedPath = normalized.path
129+
let containerPath = normalizedPath
130+
if (!isAbsolutePosixPath(containerPath)) {
131+
const baseDir = options.baseDir
132+
if (baseDir === undefined || !isAbsolutePosixPath(baseDir)) {
133+
return { _tag: "InvalidTerminalImageFetch", message: "Image path must be absolute." }
134+
}
135+
if (invalidCharacterPattern.test(baseDir) || traversalPattern.test(baseDir)) {
136+
return { _tag: "InvalidTerminalImageFetch", message: "Image base directory is invalid." }
137+
}
138+
containerPath = joinBaseDirAndRelativePath(baseDir, containerPath)
117139
}
118140
if (invalidCharacterPattern.test(containerPath)) {
119141
return { _tag: "InvalidTerminalImageFetch", message: "Image path contains invalid characters." }

packages/api/src/services/terminal-sessions.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ type TerminalRecord = {
5959
projectDisplayName: string
6060
projectId: string
6161
projectKey: string
62+
projectTargetDir: string
6263
prepared: ReturnType<typeof prepareProjectSsh>
6364
}
6465

@@ -581,7 +582,8 @@ const registerRecord = (
581582
projectKey: string,
582583
projectDisplayName: string,
583584
prepared: ReturnType<typeof prepareProjectSsh>,
584-
projectContainerName: string
585+
projectContainerName: string,
586+
projectTargetDir: string
585587
): TerminalSession => {
586588
const session: TerminalSession = {
587589
attachedClients: 0,
@@ -600,6 +602,7 @@ const registerRecord = (
600602
projectDisplayName,
601603
projectId,
602604
projectKey,
605+
projectTargetDir,
603606
pty: null,
604607
session,
605608
sockets: new Set<WebSocket>()
@@ -662,7 +665,8 @@ export const createTerminalSession = (
662665
project.projectKey,
663666
project.displayName,
664667
prepared,
665-
projectItem.containerName
668+
projectItem.containerName,
669+
projectItem.targetDir
666670
)
667671
yield* _(emitTerminalSessionCreated(projectId, session.id, options.requestId))
668672
return { project, session }
@@ -681,7 +685,8 @@ export const createTerminalSession = (
681685
project.projectKey,
682686
project.displayName,
683687
prepared,
684-
reachableProjectItem.containerName
688+
reachableProjectItem.containerName,
689+
reachableProjectItem.targetDir
685690
)
686691
yield* _(emitTerminalSessionCreated(projectId, session.id, options.requestId))
687692
yield* _(emitTerminalStatus(projectId, "ssh.post-start", "Post-start self-heal continues in background"))
@@ -786,7 +791,7 @@ export const readProjectTerminalImage = (
786791
Effect.fail(new ApiNotFoundError({ message: `Terminal session not found: ${sessionId}` }))
787792
)
788793
}
789-
const plan = planTerminalImageFetch(imagePath)
794+
const plan = planTerminalImageFetch(imagePath, { baseDir: record.projectTargetDir })
790795
if (plan._tag === "InvalidTerminalImageFetch") {
791796
return yield* _(Effect.fail(new ApiBadRequestError({ message: plan.message })))
792797
}

packages/api/tests/terminal-image-fetch-core.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,53 @@ describe("terminal image fetch core", () => {
103103
_tag: "InvalidTerminalImageFetch"
104104
})
105105
})
106+
107+
it("resolves a relative path against the provided absolute base directory", () => {
108+
expect(
109+
planTerminalImageFetch(
110+
"app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png",
111+
{ baseDir: "/home/dev" }
112+
)
113+
).toEqual({
114+
_tag: "ValidTerminalImageFetch",
115+
containerPath: "/home/dev/app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png",
116+
mediaType: "image/png"
117+
})
118+
})
119+
120+
it("ignores trailing slashes on the base directory when resolving a relative path", () => {
121+
expect(planTerminalImageFetch("photos/cover.jpg", { baseDir: "/home/dev/" })).toEqual({
122+
_tag: "ValidTerminalImageFetch",
123+
containerPath: "/home/dev/photos/cover.jpg",
124+
mediaType: "image/jpeg"
125+
})
126+
})
127+
128+
it("still rejects relative paths when no base directory is supplied", () => {
129+
expect(planTerminalImageFetch("app/cover.png")).toEqual({
130+
_tag: "InvalidTerminalImageFetch",
131+
message: "Image path must be absolute."
132+
})
133+
})
134+
135+
it("rejects relative paths when the supplied base directory is not absolute", () => {
136+
expect(planTerminalImageFetch("app/cover.png", { baseDir: "home/dev" })).toEqual({
137+
_tag: "InvalidTerminalImageFetch",
138+
message: "Image path must be absolute."
139+
})
140+
})
141+
142+
it("rejects an unsafe base directory containing traversal segments", () => {
143+
expect(planTerminalImageFetch("cover.png", { baseDir: "/home/dev/../etc" })).toEqual({
144+
_tag: "InvalidTerminalImageFetch",
145+
message: "Image base directory is invalid."
146+
})
147+
})
148+
149+
it("rejects relative paths that contain traversal segments after resolution", () => {
150+
expect(planTerminalImageFetch("../etc/cover.png", { baseDir: "/home/dev" })).toMatchObject({
151+
_tag: "InvalidTerminalImageFetch",
152+
message: "Image path must not contain '.' or '..' segments."
153+
})
154+
})
106155
})

packages/app/src/web/terminal-image-paths.ts

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ const imagePathPattern = new RegExp(
88
"giu"
99
)
1010

11+
const treePointerImagePathSource =
12+
String.raw`[^\s"'(<>\[\]{}|\\/][^\s"'(<>\[\]{}|\\]*\.(?:${extensionAlternation})`
13+
const treePointerImagePathPattern = new RegExp(
14+
String.raw`(?:^|\s)[└├]\s+(${treePointerImagePathSource})(?=$|[\s"')<>\[\]{}|.,;:?!])`,
15+
"giu"
16+
)
17+
1118
const escapeChar = String.fromCodePoint(0x1B)
1219
const bellChar = String.fromCodePoint(0x07)
1320

@@ -26,22 +33,38 @@ export type TerminalImagePathMatch = {
2633
export const stripTerminalAnsi = (text: string): string =>
2734
text.replace(ansiOscPattern, "").replace(ansiCsiPattern, "").replace(ansiOtherEscapePattern, "")
2835

29-
export const detectTerminalImagePathMatches = (text: string): ReadonlyArray<TerminalImagePathMatch> => {
30-
const plainText = stripTerminalAnsi(text)
31-
const matches: Array<TerminalImagePathMatch> = []
32-
for (const match of plainText.matchAll(imagePathPattern)) {
36+
const collectPatternMatches = (
37+
plainText: string,
38+
pattern: RegExp,
39+
matches: Array<TerminalImagePathMatch>,
40+
seenStartIndices: Set<number>
41+
): void => {
42+
for (const match of plainText.matchAll(pattern)) {
3343
const candidate = match[1]
34-
if (candidate !== undefined && candidate.length > 0) {
35-
const fullMatch = match[0]
36-
const fullStartIndex = match.index
37-
const startIndex = fullStartIndex + fullMatch.lastIndexOf(candidate)
38-
matches.push({
39-
endIndex: startIndex + candidate.length,
40-
path: candidate,
41-
startIndex
42-
})
44+
if (candidate === undefined || candidate.length === 0) {
45+
continue
4346
}
47+
const fullMatch = match[0]
48+
const fullStartIndex = match.index
49+
const startIndex = fullStartIndex + fullMatch.lastIndexOf(candidate)
50+
if (seenStartIndices.has(startIndex)) {
51+
continue
52+
}
53+
seenStartIndices.add(startIndex)
54+
matches.push({
55+
endIndex: startIndex + candidate.length,
56+
path: candidate,
57+
startIndex
58+
})
4459
}
60+
}
61+
62+
export const detectTerminalImagePathMatches = (text: string): ReadonlyArray<TerminalImagePathMatch> => {
63+
const plainText = stripTerminalAnsi(text)
64+
const matches: Array<TerminalImagePathMatch> = []
65+
const seenStartIndices = new Set<number>()
66+
collectPatternMatches(plainText, imagePathPattern, matches, seenStartIndices)
67+
collectPatternMatches(plainText, treePointerImagePathPattern, matches, seenStartIndices)
4568
return matches
4669
}
4770

packages/app/tests/docker-git/terminal-image-paths.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,37 @@ describe("terminal image path detection", () => {
9898
expect(isSupportedTerminalImagePath("/var/data/a.bmp")).toBe(false)
9999
expect(isSupportedTerminalImagePath("/var/data/a.txt")).toBe(false)
100100
})
101+
102+
it("detects relative image paths under a tree pointer (issue 261 viewed image format)", () => {
103+
const text = " └ app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png"
104+
expect(detectTerminalImagePaths(text)).toEqual([
105+
"app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png"
106+
])
107+
})
108+
109+
it("detects relative image paths under the alternative branch tree pointer", () => {
110+
expect(detectTerminalImagePaths(" ├ assets/cover.jpg")).toEqual(["assets/cover.jpg"])
111+
})
112+
113+
it("detects multiple viewed images across separate lines", () => {
114+
const text = [
115+
"• Viewed Image",
116+
" └ app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png",
117+
"",
118+
"• Viewed Image",
119+
" └ app/docs/screenshots/issue-237/proof/pr238-proof-09-skiller-submodule-checks.png"
120+
].join("\n")
121+
expect(detectTerminalImagePaths(text)).toEqual([
122+
"app/docs/screenshots/issue-237/proof/pr238-proof-06-skiller-submodule-status.png",
123+
"app/docs/screenshots/issue-237/proof/pr238-proof-09-skiller-submodule-checks.png"
124+
])
125+
})
126+
127+
it("does not duplicate absolute paths preceded by a tree pointer", () => {
128+
expect(detectTerminalImagePaths(" └ /var/data/foo.png")).toEqual(["/var/data/foo.png"])
129+
})
130+
131+
it("ignores tree pointers that do not precede an image", () => {
132+
expect(detectTerminalImagePaths(" └ Viewed File: notes.txt")).toEqual([])
133+
})
101134
})

0 commit comments

Comments
 (0)