Skip to content

Commit f9e5a02

Browse files
konardkonard
committed
Merge remote-tracking branch 'upstream/main' into renovate/all
2 parents d003e25 + 69f5b64 commit f9e5a02

15 files changed

Lines changed: 540 additions & 114 deletions

‎docs/pr-screenshots/issue-250/terminal-local-image-proof.png‎

35.5 KB
Loading

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

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { fileURLToPath } from "node:url"
2+
13
export type TerminalImageFetchPlan =
24
| {
35
readonly _tag: "InvalidTerminalImageFetch"
@@ -23,6 +25,21 @@ const controlCharRange = `${String.fromCodePoint(0)}-${String.fromCodePoint(0x1F
2325
const deleteChar = String.fromCodePoint(0x7F)
2426
const invalidCharacterPattern = new RegExp(String.raw`[\s${controlCharRange}${deleteChar}]`, "u")
2527
const traversalPattern = /(?:^|\/)(?:\.|\.\.)(?=\/|$)/u
28+
const urlSchemePattern = /^[A-Za-z][A-Za-z0-9+.-]*:/u
29+
const fileUrlPattern = /^file:\/\//iu
30+
const encodedPathSeparatorPattern = /%(?:2f|5c)/iu
31+
const fileUrlBackslashPattern = /\\/u
32+
const fileUrlTraversalPattern = /(?:^|[\\/])(?:\.|%2e)(?:(?:\.|%2e))?(?=[\\/]|$)/iu
33+
34+
type TerminalImagePathNormalization =
35+
| {
36+
readonly _tag: "InvalidTerminalImagePath"
37+
readonly message: string
38+
}
39+
| {
40+
readonly _tag: "ValidTerminalImagePath"
41+
readonly path: string
42+
}
2643

2744
const lowercaseExtension = (path: string): string | null => {
2845
const lastDot = path.lastIndexOf(".")
@@ -32,26 +49,85 @@ const lowercaseExtension = (path: string): string | null => {
3249
return path.slice(lastDot + 1).toLowerCase()
3350
}
3451

52+
const rawFileUrlPathname = (path: string): string => {
53+
const withoutScheme = path.slice("file://".length)
54+
const pathStart = withoutScheme.indexOf("/")
55+
if (pathStart < 0) {
56+
return ""
57+
}
58+
const pathAndSuffix = withoutScheme.slice(pathStart)
59+
const queryStart = pathAndSuffix.indexOf("?")
60+
const hashStart = pathAndSuffix.indexOf("#")
61+
if (queryStart < 0 && hashStart < 0) {
62+
return pathAndSuffix
63+
}
64+
if (queryStart < 0) {
65+
return pathAndSuffix.slice(0, hashStart)
66+
}
67+
if (hashStart < 0) {
68+
return pathAndSuffix.slice(0, queryStart)
69+
}
70+
return pathAndSuffix.slice(0, Math.min(queryStart, hashStart))
71+
}
72+
73+
const normalizeTerminalImagePath = (path: string): TerminalImagePathNormalization => {
74+
if (!urlSchemePattern.test(path)) {
75+
return { _tag: "ValidTerminalImagePath", path }
76+
}
77+
if (!fileUrlPattern.test(path)) {
78+
return { _tag: "InvalidTerminalImagePath", message: "Only file:// image URLs are supported." }
79+
}
80+
81+
const rawPathname = rawFileUrlPathname(path)
82+
if (fileUrlTraversalPattern.test(rawPathname)) {
83+
return { _tag: "InvalidTerminalImagePath", message: "Image path must not contain '.' or '..' segments." }
84+
}
85+
if (encodedPathSeparatorPattern.test(rawPathname) || fileUrlBackslashPattern.test(rawPathname)) {
86+
return {
87+
_tag: "InvalidTerminalImagePath",
88+
message: "Image file URL must not contain encoded or backslash path separators."
89+
}
90+
}
91+
92+
try {
93+
const url = new URL(path)
94+
if (url.protocol !== "file:" || (url.hostname !== "" && url.hostname !== "localhost")) {
95+
return { _tag: "InvalidTerminalImagePath", message: "Image file URL must point to a local path." }
96+
}
97+
if (url.search.length > 0 || url.hash.length > 0) {
98+
return { _tag: "InvalidTerminalImagePath", message: "Image file URL must not include query or fragment." }
99+
}
100+
return { _tag: "ValidTerminalImagePath", path: fileURLToPath(url, { windows: false }) }
101+
} catch {
102+
return { _tag: "InvalidTerminalImagePath", message: "Image file URL is invalid." }
103+
}
104+
}
105+
35106
export const planTerminalImageFetch = (path: string): TerminalImageFetchPlan => {
36107
if (typeof path !== "string" || path.length === 0) {
37108
return { _tag: "InvalidTerminalImageFetch", message: "Image path is required." }
38109
}
39-
if (!path.startsWith("/")) {
110+
const normalized = normalizeTerminalImagePath(path)
111+
if (normalized._tag === "InvalidTerminalImagePath") {
112+
return { _tag: "InvalidTerminalImageFetch", message: normalized.message }
113+
}
114+
const containerPath = normalized.path
115+
if (!containerPath.startsWith("/")) {
40116
return { _tag: "InvalidTerminalImageFetch", message: "Image path must be absolute." }
41117
}
42-
if (invalidCharacterPattern.test(path)) {
118+
if (invalidCharacterPattern.test(containerPath)) {
43119
return { _tag: "InvalidTerminalImageFetch", message: "Image path contains invalid characters." }
44120
}
45-
if (traversalPattern.test(path)) {
121+
if (traversalPattern.test(containerPath)) {
46122
return { _tag: "InvalidTerminalImageFetch", message: "Image path must not contain '.' or '..' segments." }
47123
}
48-
const extension = lowercaseExtension(path)
124+
const extension = lowercaseExtension(containerPath)
49125
if (extension === null) {
50126
return { _tag: "InvalidTerminalImageFetch", message: "Image path must include a file extension." }
51127
}
52128
const mediaType = supportedExtensionMediaTypes.get(extension)
53129
if (mediaType === undefined) {
54130
return { _tag: "InvalidTerminalImageFetch", message: `Unsupported image extension: .${extension}` }
55131
}
56-
return { _tag: "ValidTerminalImageFetch", containerPath: path, mediaType }
132+
return { _tag: "ValidTerminalImageFetch", containerPath, mediaType }
57133
}

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,22 @@ import { describe, expect, it } from "@effect/vitest"
33
import { planTerminalImageFetch } from "../src/services/terminal-image-fetch-core.js"
44

55
describe("terminal image fetch core", () => {
6-
it("accepts an absolute path with a supported image extension", () => {
6+
it("continues to accept an absolute path with a supported image extension", () => {
77
expect(planTerminalImageFetch("/tmp/issue232-main.png")).toEqual({
88
_tag: "ValidTerminalImageFetch",
99
containerPath: "/tmp/issue232-main.png",
1010
mediaType: "image/png"
1111
})
1212
})
1313

14+
it("accepts a file URL and normalizes it to an absolute container path", () => {
15+
expect(planTerminalImageFetch("file:///tmp/phantom-e2e.tuhl98/wallet-step-after-password.png")).toEqual({
16+
_tag: "ValidTerminalImageFetch",
17+
containerPath: "/tmp/phantom-e2e.tuhl98/wallet-step-after-password.png",
18+
mediaType: "image/png"
19+
})
20+
})
21+
1422
it("maps each supported extension to its media type", () => {
1523
expect(planTerminalImageFetch("/a.jpg")).toMatchObject({ mediaType: "image/jpeg" })
1624
expect(planTerminalImageFetch("/a.jpeg")).toMatchObject({ mediaType: "image/jpeg" })
@@ -33,6 +41,13 @@ describe("terminal image fetch core", () => {
3341
})
3442
})
3543

44+
it("rejects non-file URLs", () => {
45+
expect(planTerminalImageFetch("https://example.com/tmp/photo.png")).toEqual({
46+
_tag: "InvalidTerminalImageFetch",
47+
message: "Only file:// image URLs are supported."
48+
})
49+
})
50+
3651
it("rejects whitespace and control characters", () => {
3752
expect(planTerminalImageFetch("/tmp/has space.png")).toMatchObject({
3853
_tag: "InvalidTerminalImageFetch"
@@ -51,6 +66,32 @@ describe("terminal image fetch core", () => {
5166
})
5267
})
5368

69+
it("rejects traversal segments in file URLs before URL normalization", () => {
70+
expect(planTerminalImageFetch("file:///tmp/../etc/photo.png")).toMatchObject({
71+
_tag: "InvalidTerminalImageFetch",
72+
message: "Image path must not contain '.' or '..' segments."
73+
})
74+
expect(planTerminalImageFetch("file:///tmp/%2E%2E/etc/photo.png")).toMatchObject({
75+
_tag: "InvalidTerminalImageFetch",
76+
message: "Image path must not contain '.' or '..' segments."
77+
})
78+
})
79+
80+
it("rejects unsafe file URL forms", () => {
81+
expect(planTerminalImageFetch("file://example.com/tmp/photo.png")).toMatchObject({
82+
_tag: "InvalidTerminalImageFetch",
83+
message: "Image file URL must point to a local path."
84+
})
85+
expect(planTerminalImageFetch("file:///tmp/photo.png?download=1")).toMatchObject({
86+
_tag: "InvalidTerminalImageFetch",
87+
message: "Image file URL must not include query or fragment."
88+
})
89+
expect(planTerminalImageFetch("file:///tmp/%2Fetc/photo.png")).toMatchObject({
90+
_tag: "InvalidTerminalImageFetch",
91+
message: "Image file URL must not contain encoded or backslash path separators."
92+
})
93+
})
94+
5495
it("rejects unsupported extensions", () => {
5596
expect(planTerminalImageFetch("/tmp/file.bmp")).toMatchObject({
5697
_tag: "InvalidTerminalImageFetch"

‎packages/app/src/web/actions-event-payload.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import type { ApiEvent } from "./api.js"
2+
3+
export const readEventPayloadString = (
4+
event: ApiEvent,
5+
key: string
6+
): string | null => {
7+
const payload = event.payload
8+
if (payload === null || typeof payload !== "object" || Array.isArray(payload)) {
9+
return null
10+
}
11+
const value = Object.entries(payload).find(([name]) => name === key)?.[1]
12+
return typeof value === "string" ? value : null
13+
}

‎packages/app/src/web/actions-project-create.ts‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,14 @@ import { Either } from "effect"
33

44
import { createProjectDraftFromInputs } from "../docker-git/menu-create-shared.js"
55
import type { CreateInputs } from "../docker-git/menu-types.js"
6+
import { readEventPayloadString } from "./actions-event-payload.js"
67
import { appendOutputLine, appendOutputLineHandler, notifyProjectEventRateLimit } from "./actions-output.js"
78
import { type BrowserActionContext, withBusy } from "./actions-shared.js"
89
import { ProjectDetailsSchema } from "./api-schema.js"
910
import { type ApiEvent, loadProjectDetails, type ProjectDetails, startCreateProject } from "./api.js"
1011
import { openProjectEventStream } from "./project-events.js"
1112
import { outputScreen, projectPickerScreen } from "./screen.js"
1213

13-
const readEventPayloadString = (
14-
event: ApiEvent,
15-
key: string
16-
): string | null => {
17-
const payload = event.payload
18-
if (payload === null || typeof payload !== "object" || Array.isArray(payload)) {
19-
return null
20-
}
21-
const value = Object.entries(payload).find(([name]) => name === key)?.[1]
22-
return typeof value === "string" ? value : null
23-
}
24-
2514
const readCreatedProjectId = (event: ApiEvent): string | null =>
2615
event.type === "project.created" ? readEventPayloadString(event, "projectId") : null
2716

‎packages/app/src/web/actions-projects.ts‎

Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { openSelectedProjectBrowser } from "./actions-browser.js"
22
import { openSelectedProjectDatabaseEditor } from "./actions-databases.js"
3+
import { readEventPayloadString } from "./actions-event-payload.js"
34
import { appendOutputLine, appendOutputLineHandler, notifyProjectEventRateLimit } from "./actions-output.js"
45
import { openSelectedProjectPort } from "./actions-port-forwards.js"
56
import {
@@ -13,9 +14,9 @@ import {
1314
} from "./actions-shared.js"
1415
import { loadSelectedProjectTasks } from "./actions-tasks.js"
1516
import {
17+
type ApiEvent,
1618
applyAllProjects,
1719
applyProject,
18-
type ApiEvent,
1920
deleteProject,
2021
downAllProjects,
2122
downProject,
@@ -79,41 +80,33 @@ const resolveProjectTerminalKey = (
7980
}
8081

8182
const randomHex = (bytes: number): string => {
82-
const getRandomValues = globalThis.crypto?.getRandomValues
83-
if (typeof getRandomValues === "function") {
84-
const values = new Uint8Array(bytes)
85-
getRandomValues.call(globalThis.crypto, values)
86-
return Array.from(values, (value) => value.toString(16).padStart(2, "0")).join("")
87-
}
88-
89-
let fallback = ""
90-
while (fallback.length < bytes * 2) {
91-
fallback += Math.floor(Math.random() * 0x1_0000_0000)
92-
.toString(16)
93-
.padStart(8, "0")
94-
}
95-
return fallback.slice(0, bytes * 2)
83+
const values = new Uint8Array(bytes)
84+
globalThis.crypto.getRandomValues(values)
85+
return Array.from(values, (value) => value.toString(16).padStart(2, "0")).join("")
9686
}
9787

9888
const createPendingTerminalSessionId = (): string => {
99-
const randomUUID = globalThis.crypto?.randomUUID
100-
if (typeof randomUUID === "function") {
101-
return randomUUID.call(globalThis.crypto)
89+
if (Reflect.has(globalThis.crypto, "randomUUID")) {
90+
return globalThis.crypto.randomUUID()
10291
}
10392

10493
return `pending-${Date.now().toString(16)}-${randomHex(8)}`
10594
}
10695

107-
const readEventPayloadString = (
108-
event: ApiEvent,
109-
key: string
110-
): string | null => {
111-
const payload = event.payload
112-
if (payload === null || typeof payload !== "object" || Array.isArray(payload)) {
113-
return null
114-
}
115-
const value = Object.entries(payload).find(([name]) => name === key)?.[1]
116-
return typeof value === "string" ? value : null
96+
type ProjectActiveTerminalSessionArgs = Omit<
97+
Parameters<typeof buildProjectActiveTerminalSession>[0],
98+
"onExit" | "onReady"
99+
>
100+
101+
const addProjectTerminalSession = (
102+
context: BrowserActionContext,
103+
args: ProjectActiveTerminalSessionArgs
104+
) => {
105+
context.addTerminalSession(buildProjectActiveTerminalSession({
106+
...args,
107+
onExit: context.reloadDashboard,
108+
onReady: context.reloadDashboard
109+
}))
117110
}
118111

119112
const readTerminalSessionCreatedId = (
@@ -188,6 +181,11 @@ export const connectProjectById = (
188181
stream?.close()
189182
stream = null
190183
}
184+
const showPendingTerminalError = (error: string) => {
185+
pendingSessionFinalized = true
186+
appendOutputLine(context, `[error] ${error}`)
187+
context.addTerminalSession(renderPendingTerminalSession(error, "error"))
188+
}
191189
const attachCreatedSession = (sessionId: string) => {
192190
if (attachedSessionId !== null) {
193191
return
@@ -198,23 +196,19 @@ export const connectProjectById = (
198196
effect: loadProjectTerminalSession(resolvedProjectKey, sessionId),
199197
label: "Attaching SSH terminal",
200198
onFailure: (error) => {
201-
pendingSessionFinalized = true
202-
appendOutputLine(context, `[error] ${error}`)
203-
context.addTerminalSession(renderPendingTerminalSession(error, "error"))
199+
showPendingTerminalError(error)
204200
closeStream()
205201
},
206202
onSuccess: (session) => {
207203
pendingSessionFinalized = true
208204
context.reloadDashboard()
209205
context.closeTerminalSession(pendingSessionId)
210-
context.addTerminalSession(buildProjectActiveTerminalSession({
211-
onExit: context.reloadDashboard,
212-
onReady: context.reloadDashboard,
206+
addProjectTerminalSession(context, {
213207
projectDisplayName,
214208
projectId,
215209
projectKey: resolvedProjectKey,
216210
session
217-
}))
211+
})
218212
context.setMessage(`Project is ready. SSH terminal is connecting for ${projectDisplayName}.`)
219213
closeStream()
220214
}
@@ -225,9 +219,7 @@ export const connectProjectById = (
225219
effect: startProjectTerminalSession(resolvedProjectKey, pendingSessionId),
226220
label: "Opening SSH terminal",
227221
onFailure: (error) => {
228-
pendingSessionFinalized = true
229-
appendOutputLine(context, `[error] ${error}`)
230-
context.addTerminalSession(renderPendingTerminalSession(error, "error"))
222+
showPendingTerminalError(error)
231223
},
232224
onSuccess: (accepted) => {
233225
appendOutputLine(context, `[ssh.prepare] SSH terminal request accepted (${accepted.requestId})`)
@@ -237,9 +229,7 @@ export const connectProjectById = (
237229
onEvent: (event) => {
238230
const failure = readTerminalStartupFailure(event, accepted.requestId)
239231
if (failure !== null) {
240-
pendingSessionFinalized = true
241-
appendOutputLine(context, `[error] ${failure}`)
242-
context.addTerminalSession(renderPendingTerminalSession(failure, "error"))
232+
showPendingTerminalError(failure)
243233
context.setMessage(failure)
244234
closeStream()
245235
return
@@ -306,14 +296,12 @@ export const attachProjectTerminalById = (
306296
effect: loadProjectTerminalSession(resolvedProjectKey, sessionId),
307297
label: "Attaching SSH terminal",
308298
onSuccess: (session) => {
309-
context.addTerminalSession(buildProjectActiveTerminalSession({
310-
onExit: context.reloadDashboard,
311-
onReady: context.reloadDashboard,
299+
addProjectTerminalSession(context, {
312300
projectDisplayName,
313301
projectId,
314302
projectKey: resolvedProjectKey,
315303
session
316-
}))
304+
})
317305
context.setMessage(`Attached SSH terminal for ${projectDisplayName}.`)
318306
}
319307
})

‎packages/app/src/web/api-types.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ import type {
2323
export type ProjectSummary = Schema.Schema.Type<typeof ProjectSummarySchema>
2424
export type ProjectDetails = Schema.Schema.Type<typeof ProjectDetailsSchema>
2525
export type CreateProjectAcceptedResponse = Schema.Schema.Type<typeof CreateProjectAcceptedResponseSchema>
26-
export type StartProjectTerminalSessionAccepted =
27-
Schema.Schema.Type<typeof StartProjectTerminalSessionAcceptedResponseSchema>
26+
export type StartProjectTerminalSessionAccepted = Schema.Schema.Type<
27+
typeof StartProjectTerminalSessionAcceptedResponseSchema
28+
>
2829
export type ProjectPortForward = Schema.Schema.Type<typeof ProjectPortForwardSchema>
2930
export type ProjectBrowserSession = Schema.Schema.Type<typeof ProjectBrowserSessionSchema>
3031
export type ProjectDatabaseForward = Schema.Schema.Type<typeof ProjectDatabaseForwardSchema>

0 commit comments

Comments
 (0)