Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions packages/api/src/services/terminal-image-fetch-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,39 @@ 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." }
}
const normalized = normalizeTerminalImagePath(path)
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." }
Expand Down
13 changes: 9 additions & 4 deletions packages/api/src/services/terminal-sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type TerminalRecord = {
projectDisplayName: string
projectId: string
projectKey: string
projectTargetDir: string
prepared: ReturnType<typeof prepareProjectSsh>
}

Expand Down Expand Up @@ -581,7 +582,8 @@ const registerRecord = (
projectKey: string,
projectDisplayName: string,
prepared: ReturnType<typeof prepareProjectSsh>,
projectContainerName: string
projectContainerName: string,
projectTargetDir: string
): TerminalSession => {
const session: TerminalSession = {
attachedClients: 0,
Expand All @@ -600,6 +602,7 @@ const registerRecord = (
projectDisplayName,
projectId,
projectKey,
projectTargetDir,
pty: null,
session,
sockets: new Set<WebSocket>()
Expand Down Expand Up @@ -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 }
Expand All @@ -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"))
Expand Down Expand Up @@ -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 })))
}
Expand Down
49 changes: 49 additions & 0 deletions packages/api/tests/terminal-image-fetch-core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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."
})
})
})
49 changes: 36 additions & 13 deletions packages/app/src/web/terminal-image-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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<TerminalImagePathMatch> => {
const plainText = stripTerminalAnsi(text)
const matches: Array<TerminalImagePathMatch> = []
for (const match of plainText.matchAll(imagePathPattern)) {
const collectPatternMatches = (
plainText: string,
pattern: RegExp,
matches: Array<TerminalImagePathMatch>,
seenStartIndices: Set<number>
): 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<TerminalImagePathMatch> => {
const plainText = stripTerminalAnsi(text)
const matches: Array<TerminalImagePathMatch> = []
const seenStartIndices = new Set<number>()
collectPatternMatches(plainText, imagePathPattern, matches, seenStartIndices)
collectPatternMatches(plainText, treePointerImagePathPattern, matches, seenStartIndices)
return matches
}

Expand Down
33 changes: 33 additions & 0 deletions packages/app/tests/docker-git/terminal-image-paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([])
})
})