Skip to content

Commit 9132fdc

Browse files
skulidropekclaude
skulidropek
and
claude
committed
fix(pr-434): address CodeRabbit review issues
- contracts.ts: add sshPassword: string | null to ShareLinkInfo type - ssh-project-tunnels.ts: exit waitForHostname early when processClosed; propagate enableContainerPasswordAuth failure - ssh-password-setup.ts: pass password via SSHPW env var instead of shell interpolation to prevent injection - app-ready-terminal-pane.tsx: add cancelled flag to polling useEffect to prevent stale setState - panel-share.tsx: replace eslint-disable with useCallback + requestId guard for stale request prevention - app-share-link.tsx: use RegExp.exec() instead of String.match(); extract nested template literal; async copyText Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ef931d8 commit 9132fdc

6 files changed

Lines changed: 334 additions & 114 deletions

File tree

‎packages/api/src/api/contracts.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,7 @@ export type ShareLinkInfo = {
821821
readonly vscodeUri: string
822822
readonly cfVscodeUri: string | null
823823
readonly workspacePath: string
824+
readonly sshPassword: string | null
824825
readonly createdAt: string
825826
readonly expiresAt: string
826827
}

‎packages/api/src/services/ssh-password-setup.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,14 @@ export const generateSshPassword = (): string => {
2929

3030
const dockerExec = (
3131
containerName: string,
32+
env: Record<string, string>,
3233
script: string
3334
): Effect.Effect<string, ApiInternalError> =>
3435
Effect.tryPromise({
3536
catch: (cause) => new ApiInternalError({ message: `docker exec ${containerName} failed`, cause }),
3637
try: async () => {
37-
const { stdout } = await execFileAsync("docker", ["exec", containerName, "sh", "-c", script])
38+
const envArgs = Object.entries(env).flatMap(([k, v]) => ["-e", `${k}=${v}`])
39+
const { stdout } = await execFileAsync("docker", ["exec", ...envArgs, containerName, "sh", "-c", script])
3840
return stdout
3941
}
4042
})
@@ -54,14 +56,11 @@ export const enableContainerPasswordAuth = (
5456
password: string
5557
): Effect.Effect<void, ApiInternalError> => {
5658
const script = [
57-
// Enable password auth in docker-git's custom sshd config
5859
"sed -i 's/PasswordAuthentication no/PasswordAuthentication yes/' /etc/ssh/sshd_config.d/dev.conf",
59-
// Set password for dev user (chpasswd reads user:password from stdin)
60-
`echo 'dev:${password}' | chpasswd`,
61-
// Reload sshd without dropping existing connections
60+
"printf 'dev:%s' \"$SSHPW\" | chpasswd",
6261
"kill -HUP $(pgrep -xo sshd) 2>/dev/null || true"
6362
].join(" && ")
64-
return dockerExec(containerName, script).pipe(Effect.asVoid)
63+
return dockerExec(containerName, { SSHPW: password }, script).pipe(Effect.asVoid)
6564
}
6665

6766
/**
@@ -82,7 +81,7 @@ export const disableContainerPasswordAuth = (
8281
"passwd -l dev",
8382
"kill -HUP $(pgrep -xo sshd) 2>/dev/null || true"
8483
].join(" && ")
85-
return dockerExec(containerName, script).pipe(
84+
return dockerExec(containerName, {}, script).pipe(
8685
Effect.asVoid,
8786
Effect.orElse(() => Effect.void)
8887
)

‎packages/api/src/services/ssh-project-tunnels.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ const waitForHostname = (
171171
remainingAttempts: number
172172
): Effect.Effect<string | null> =>
173173
Effect.gen(function*(_) {
174-
if (record.hostname !== null || record.stopping || remainingAttempts <= 0) {
174+
if (record.hostname !== null || record.stopping || record.processClosed || remainingAttempts <= 0) {
175175
return record.hostname
176176
}
177177
yield* _(Effect.sleep(Duration.millis(250)))
@@ -209,7 +209,7 @@ export const startSshProjectTunnel = (
209209
}
210210

211211
const sshPassword = generateSshPassword()
212-
yield* _(enableContainerPasswordAuth(containerName, sshPassword).pipe(Effect.orElse(() => Effect.void)))
212+
yield* _(enableContainerPasswordAuth(containerName, sshPassword))
213213

214214
const localhostHost = yield* _(defaultLocalhostHost())
215215
const sshUrl = `ssh://${localhostHost}:${sshPort}`

‎packages/app/src/web/app-ready-terminal-pane.tsx‎

Lines changed: 158 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { Effect } from "effect"
22
import { type CSSProperties, type JSX, useEffect, useState } from "react"
33

4-
import { deleteTerminalSessionByPath } from "./api.js"
54
import { startProjectSshTunnel } from "./api-share-links.js"
5+
import { deleteTerminalSessionByPath } from "./api.js"
66
import { canOpenProjectBrowser } from "./app-ready-browser-openable.js"
77
import { TerminalTaskManagerBody } from "./app-ready-terminal-task-manager.js"
88
import type { TerminalPaneProps } from "./app-ready-terminal-types.js"
@@ -174,7 +174,13 @@ const hostSshConfig = (hostname: string, sshUser: string): string =>
174174
const directSshConfig = (host: string, sshPort: number, sshUser: string): string =>
175175
`Host ${host}-ssh\n HostName ${host}\n Port ${sshPort}\n User ${sshUser}\n StrictHostKeyChecking no\n UserKnownHostsFile /dev/null`
176176

177-
const copyText = (text: string): void => { void navigator.clipboard.writeText(text).catch(() => {}) }
177+
const copyText = async (text: string): Promise<void> => {
178+
try {
179+
await navigator.clipboard.writeText(text)
180+
} catch {
181+
// ignore clipboard errors
182+
}
183+
}
178184

179185
const vsCodePanelCodeStyle: CSSProperties = {
180186
background: "#0b1017",
@@ -222,29 +228,40 @@ const VsCodeAccessPanel = (
222228
? `ssh -o "ProxyCommand=cloudflared access ssh --hostname %h" ${info.sshUser}@${cfState.hostname}`
223229
: null
224230
const cfVscodeUri = cfState.tag === "ready"
225-
? `vscode://ms-vscode-remote.remote-ssh/open?hostName=${encodeURIComponent(`${info.sshUser}@${cfState.hostname}`)}&folder=${encodeURIComponent(info.targetDir)}`
231+
? `vscode://ms-vscode-remote.remote-ssh/open?hostName=${
232+
encodeURIComponent(`${info.sshUser}@${cfState.hostname}`)
233+
}&folder=${encodeURIComponent(info.targetDir)}`
226234
: null
227-
const directHost = window.location.hostname
235+
const directHost = location.hostname
228236
const directConfig = directSshConfig(directHost, info.sshPort, info.sshUser)
229-
const directCommand = `ssh -p ${info.sshPort} -t ${info.sshUser}@${directHost} "cd ${info.targetDir} && exec \\$SHELL"`
230-
const directVscodeUri = `vscode://ms-vscode-remote.remote-ssh/open?hostName=${encodeURIComponent(`${directHost}-ssh`)}&folder=${encodeURIComponent(info.targetDir)}`
237+
const directCommand = String
238+
.raw`ssh -p ${info.sshPort} -t ${info.sshUser}@${directHost} "cd ${info.targetDir} && exec \$SHELL"`
239+
const directVscodeUri = `vscode://ms-vscode-remote.remote-ssh/open?hostName=${
240+
encodeURIComponent(`${directHost}-ssh`)
241+
}&folder=${encodeURIComponent(info.targetDir)}`
231242
return (
232-
<div style={{
233-
background: "#0d1520",
234-
border: "1px solid #2a4060",
235-
borderRadius: "4px",
236-
boxSizing: "border-box",
237-
height: "100%",
238-
overflowY: "auto",
239-
padding: "12px 16px"
240-
}}>
243+
<div
244+
style={{
245+
background: "#0d1520",
246+
border: "1px solid #2a4060",
247+
borderRadius: "4px",
248+
boxSizing: "border-box",
249+
height: "100%",
250+
overflowY: "auto",
251+
padding: "12px 16px"
252+
}}
253+
>
241254
<div style={{ alignItems: "center", display: "flex", justifyContent: "space-between", marginBottom: "10px" }}>
242255
<div style={{ color: "#8be9fd", fontWeight: "bold" }}>VS Code / SSH access</div>
243256
<div style={{ display: "flex", gap: "4px" }}>
244257
{cfState.tag === "ready" && (
245-
<button onClick={onRefresh} style={{ ...vsCodePanelCopyBtnStyle, color: "#7fdfff" }} type="button">↻ refresh</button>
258+
<button onClick={onRefresh} style={{ ...vsCodePanelCopyBtnStyle, color: "#7fdfff" }} type="button">
259+
↻ refresh
260+
</button>
246261
)}
247-
<button onClick={onClose} style={{ ...vsCodePanelCopyBtnStyle, color: "#f87171" }} type="button">✕ close</button>
262+
<button onClick={onClose} style={{ ...vsCodePanelCopyBtnStyle, color: "#f87171" }} type="button">
263+
✕ close
264+
</button>
248265
</div>
249266
</div>
250267

@@ -255,30 +272,76 @@ const VsCodeAccessPanel = (
255272
{cfState.tag === "failed" && (
256273
<div style={{ marginTop: "8px" }}>
257274
<div style={{ color: "#f87171" }}>Tunnel failed to start.</div>
258-
<button onClick={onRetry} style={{ ...vsCodePanelCopyBtnStyle, color: "#7fdfff", marginTop: "4px" }} type="button">Retry</button>
275+
<button
276+
onClick={onRetry}
277+
style={{ ...vsCodePanelCopyBtnStyle, color: "#7fdfff", marginTop: "4px" }}
278+
type="button"
279+
>
280+
Retry
281+
</button>
259282
</div>
260283
)}
261284

262285
{cfState.tag === "ready" && (
263286
<>
264287
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold" }}>Add to ~/.ssh/config</div>
265-
<div style={{ color: "#8fa6c4", fontSize: "0.78em" }}>requires <code style={{ color: "#a8c8f0" }}>cloudflared</code> installed on your machine</div>
288+
<div style={{ color: "#8fa6c4", fontSize: "0.78em" }}>
289+
requires <code style={{ color: "#a8c8f0" }}>cloudflared</code> installed on your machine
290+
</div>
266291
<code style={vsCodePanelCodeStyle}>{cfSshConfig}</code>
267-
<button onClick={() => { copyText(cfSshConfig as string) }} style={vsCodePanelCopyBtnStyle} type="button">copy</button>
268-
269-
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>Connect via SSH</div>
292+
<button
293+
onClick={() => {
294+
copyText(cfSshConfig as string)
295+
}}
296+
style={vsCodePanelCopyBtnStyle}
297+
type="button"
298+
>
299+
copy
300+
</button>
301+
302+
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>
303+
Connect via SSH
304+
</div>
270305
<code style={vsCodePanelCodeStyle}>{cfSshCommand}</code>
271-
<button onClick={() => { copyText(cfSshCommand as string) }} style={vsCodePanelCopyBtnStyle} type="button">copy</button>
306+
<button
307+
onClick={() => {
308+
copyText(cfSshCommand as string)
309+
}}
310+
style={vsCodePanelCopyBtnStyle}
311+
type="button"
312+
>
313+
copy
314+
</button>
272315

273316
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>SSH password</div>
274317
<code style={vsCodePanelCodeStyle}>{cfState.sshPassword}</code>
275-
<button onClick={() => { copyText(cfState.sshPassword) }} style={vsCodePanelCopyBtnStyle} type="button">copy</button>
318+
<button
319+
onClick={() => {
320+
copyText(cfState.sshPassword)
321+
}}
322+
style={vsCodePanelCopyBtnStyle}
323+
type="button"
324+
>
325+
copy
326+
</button>
276327

277328
{cfVscodeUri !== null && (
278329
<>
279-
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>Open in VS Code</div>
330+
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>
331+
Open in VS Code
332+
</div>
280333
<div style={{ marginTop: "4px" }}>
281-
<a href={cfVscodeUri} style={{ color: "#56f39a", cursor: "pointer", fontFamily: "inherit", fontSize: "inherit", fontWeight: "bold", textDecoration: "none" }}>
334+
<a
335+
href={cfVscodeUri}
336+
style={{
337+
color: "#56f39a",
338+
cursor: "pointer",
339+
fontFamily: "inherit",
340+
fontSize: "inherit",
341+
fontWeight: "bold",
342+
textDecoration: "none"
343+
}}
344+
>
282345
open in VS Code (CF tunnel)
283346
</a>
284347
</div>
@@ -293,24 +356,58 @@ const VsCodeAccessPanel = (
293356
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold" }}>Add to ~/.ssh/config</div>
294357
<div style={{ color: "#8fa6c4", fontSize: "0.78em" }}>no cloudflared needed — works on same LAN</div>
295358
<code style={vsCodePanelCodeStyle}>{directConfig}</code>
296-
<button onClick={() => { copyText(directConfig) }} style={vsCodePanelCopyBtnStyle} type="button">copy</button>
359+
<button
360+
onClick={() => {
361+
copyText(directConfig)
362+
}}
363+
style={vsCodePanelCopyBtnStyle}
364+
type="button"
365+
>
366+
copy
367+
</button>
297368

298369
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>Connect via SSH</div>
299370
<code style={vsCodePanelCodeStyle}>{directCommand}</code>
300-
<button onClick={() => { copyText(directCommand) }} style={vsCodePanelCopyBtnStyle} type="button">copy</button>
371+
<button
372+
onClick={() => {
373+
copyText(directCommand)
374+
}}
375+
style={vsCodePanelCopyBtnStyle}
376+
type="button"
377+
>
378+
copy
379+
</button>
301380

302381
{cfState.tag === "ready" && (
303382
<>
304383
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>SSH password</div>
305384
<code style={vsCodePanelCodeStyle}>{cfState.sshPassword}</code>
306-
<button onClick={() => { copyText(cfState.sshPassword) }} style={vsCodePanelCopyBtnStyle} type="button">copy</button>
385+
<button
386+
onClick={() => {
387+
copyText(cfState.sshPassword)
388+
}}
389+
style={vsCodePanelCopyBtnStyle}
390+
type="button"
391+
>
392+
copy
393+
</button>
307394
</>
308395
)}
309396

310397
<div style={{ color: "#8be9fd", fontSize: "0.9em", fontWeight: "bold", marginTop: "10px" }}>Open in VS Code</div>
311398
<div style={{ color: "#8fa6c4", fontSize: "0.78em" }}>requires config entry above in ~/.ssh/config</div>
312399
<div style={{ marginTop: "4px" }}>
313-
<a href={directVscodeUri} style={{ color: "#56f39a", cursor: "pointer", fontFamily: "inherit", fontSize: "inherit", fontWeight: "bold", textDecoration: "none" }}>
400+
<a
401+
href={directVscodeUri}
402+
style={{
403+
color: "#56f39a",
404+
cursor: "pointer",
405+
fontFamily: "inherit",
406+
fontSize: "inherit",
407+
fontWeight: "bold",
408+
textDecoration: "none"
409+
}}
410+
>
314411
open in VS Code (direct)
315412
</a>
316413
</div>
@@ -409,12 +506,14 @@ const startTunnel = (
409506
void Effect.runPromise(
410507
startProjectSshTunnel(projectKey).pipe(
411508
Effect.match({
412-
onFailure: () => { setCfState({ tag: "failed" }) },
509+
onFailure: () => {
510+
setCfState({ tag: "failed" })
511+
},
413512
onSuccess: ({ hostname, sshPassword }) => {
414513
setCfState(
415-
hostname !== null
416-
? { tag: "ready", hostname, sshPassword }
417-
: { tag: "failed" }
514+
hostname === null
515+
? { tag: "failed" }
516+
: { tag: "ready", hostname, sshPassword }
418517
)
419518
}
420519
})
@@ -423,45 +522,59 @@ const startTunnel = (
423522
}
424523

425524
export const TerminalPane = (props: TerminalPaneProps): JSX.Element => {
426-
const [vsCodePanelOpen, setVsCodePanelOpen] = useState(false)
525+
const [isVsCodePanelOpen, setVsCodePanelOpen] = useState(false)
427526
const [cfState, setCfState] = useState<CfTunnelState>({ tag: "idle" })
428527
const runtime = resolveTerminalPaneRuntime(props)
429528

430529
useEffect(() => {
431-
if (!vsCodePanelOpen || runtime.browserProjectKey === undefined) return
530+
if (!isVsCodePanelOpen || runtime.browserProjectKey === undefined) return
432531
if (cfState.tag === "idle") {
433532
startTunnel(runtime.browserProjectKey, setCfState)
434533
}
435-
}, [vsCodePanelOpen, runtime.browserProjectKey, cfState.tag])
534+
}, [isVsCodePanelOpen, runtime.browserProjectKey, cfState.tag])
436535

437536
// Poll every 30s when panel is open: restart tunnel if process died
438537
useEffect(() => {
439-
if (!vsCodePanelOpen || cfState.tag !== "ready" || runtime.browserProjectKey === undefined) return
538+
if (!isVsCodePanelOpen || cfState.tag !== "ready" || runtime.browserProjectKey === undefined) return
440539
const projectKey = runtime.browserProjectKey
540+
let isCancelled = false
441541
const id = setInterval(() => {
442542
void Effect.runPromise(
443543
startProjectSshTunnel(projectKey).pipe(
444544
Effect.match({
445-
onFailure: () => { setCfState({ tag: "failed" }) },
545+
onFailure: () => {
546+
if (!isCancelled) setCfState({ tag: "failed" })
547+
},
446548
onSuccess: ({ hostname, sshPassword }) => {
447-
if (hostname === null) { setCfState({ tag: "failed" }); return }
549+
if (isCancelled) return
550+
if (hostname === null) {
551+
setCfState({ tag: "failed" })
552+
return
553+
}
448554
setCfState({ tag: "ready", hostname, sshPassword })
449555
}
450556
})
451557
)
452558
)
453559
}, 30_000)
454-
return () => { clearInterval(id) }
455-
}, [vsCodePanelOpen, cfState.tag, runtime.browserProjectKey])
560+
return () => {
561+
isCancelled = true
562+
clearInterval(id)
563+
}
564+
}, [isVsCodePanelOpen, cfState.tag, runtime.browserProjectKey])
456565

457566
const vsCodeInfo = buildVsCodeAccessInfo(props.project)
458-
const onOpenVsCode = vsCodeInfo !== null ? () => { setVsCodePanelOpen(true) } : undefined
459-
const vsCodeBodyContent = vsCodePanelOpen && vsCodeInfo !== null
567+
const onOpenVsCode = vsCodeInfo === null ? undefined : () => {
568+
setVsCodePanelOpen(true)
569+
}
570+
const vsCodeBodyContent = isVsCodePanelOpen && vsCodeInfo !== null
460571
? (
461572
<VsCodeAccessPanel
462573
cfState={cfState}
463574
info={vsCodeInfo}
464-
onClose={() => { setVsCodePanelOpen(false) }}
575+
onClose={() => {
576+
setVsCodePanelOpen(false)
577+
}}
465578
onRefresh={() => {
466579
if (runtime.browserProjectKey !== undefined) {
467580
startTunnel(runtime.browserProjectKey, setCfState)

0 commit comments

Comments
 (0)