Add remote SSH environment launch support#2022
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: SSH 401 retry path is unreachable dead code
- Encoded HTTP status as
[ssh_http:<status>]infetchLoopbackSshJsonerror messages so it survives IPC serialization, and addedisSshHttpAuthError()helper in service.ts to detect SSH 401 errors, making the retry path reachable for SSH environments.
- Encoded HTTP status as
- ✅ Fixed: Duplicate serialization function across catalog and service
- Exported
toPersistedSavedEnvironmentRecordfromcatalog.tsand replaced the duplicateserializeSavedEnvironmentRecordinservice.tswith the shared function.
- Exported
Or push these changes by commenting:
@cursor push aa396c60b9
Preview (aa396c60b9)
diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts
--- a/apps/desktop/src/main.ts
+++ b/apps/desktop/src/main.ts
@@ -500,12 +500,11 @@
}
if (!response.ok) {
- throw new Error(
- await readRemoteFetchErrorMessage(
- response,
- `SSH forwarded request failed (${response.status}).`,
- ),
+ const message = await readRemoteFetchErrorMessage(
+ response,
+ `SSH forwarded request failed (${response.status}).`,
);
+ throw new Error(`[ssh_http:${response.status}] ${message}`);
}
return (await response.json()) as T;
diff --git a/apps/web/src/environments/runtime/catalog.ts b/apps/web/src/environments/runtime/catalog.ts
--- a/apps/web/src/environments/runtime/catalog.ts
+++ b/apps/web/src/environments/runtime/catalog.ts
@@ -35,7 +35,7 @@
let savedEnvironmentRegistryHydrated = false;
let savedEnvironmentRegistryHydrationPromise: Promise<void> | null = null;
-function toPersistedSavedEnvironmentRecord(
+export function toPersistedSavedEnvironmentRecord(
record: SavedEnvironmentRecord,
): PersistedSavedEnvironmentRecord {
return {
diff --git a/apps/web/src/environments/runtime/service.ts b/apps/web/src/environments/runtime/service.ts
--- a/apps/web/src/environments/runtime/service.ts
+++ b/apps/web/src/environments/runtime/service.ts
@@ -49,6 +49,7 @@
removeSavedEnvironmentBearerToken,
type SavedEnvironmentRecord,
useSavedEnvironmentRegistryStore,
+ toPersistedSavedEnvironmentRecord,
useSavedEnvironmentRuntimeStore,
waitForSavedEnvironmentRegistryHydration,
writeSavedEnvironmentBearerToken,
@@ -392,16 +393,14 @@
return new Date().toISOString();
}
-function serializeSavedEnvironmentRecord(record: SavedEnvironmentRecord) {
- return {
- environmentId: record.environmentId,
- label: record.label,
- httpBaseUrl: record.httpBaseUrl,
- wsBaseUrl: record.wsBaseUrl,
- createdAt: record.createdAt,
- lastConnectedAt: record.lastConnectedAt,
- ...(record.desktopSsh ? { desktopSsh: record.desktopSsh } : {}),
- } as const;
+const SSH_HTTP_STATUS_RE = /^\[ssh_http:(\d+)\] /;
+
+function isSshHttpAuthError(error: unknown, status: number): boolean {
+ if (!(error instanceof Error)) {
+ return false;
+ }
+ const match = SSH_HTTP_STATUS_RE.exec(error.message);
+ return match !== null && Number(match[1]) === status;
}
function isDesktopSshTargetEqual(
@@ -436,7 +435,7 @@
async function persistSavedEnvironmentRegistryRollback(): Promise<void> {
await ensureLocalApi().persistence.setSavedEnvironmentRegistry(
- listSavedEnvironmentRecords().map((entry) => serializeSavedEnvironmentRecord(entry)),
+ listSavedEnvironmentRecords().map((entry) => toPersistedSavedEnvironmentRecord(entry)),
);
}
@@ -1049,11 +1048,10 @@
options?.serverConfig ?? null,
);
} catch (error) {
- if (
- !record.desktopSsh ||
- !isRemoteEnvironmentAuthHttpError(error) ||
- error.status !== 401
- ) {
+ const is401 = record.desktopSsh
+ ? isSshHttpAuthError(error, 401)
+ : isRemoteEnvironmentAuthHttpError(error) && error.status === 401;
+ if (!is401) {
throw error;
}You can send follow-ups to the cloud agent here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review Unable to check for correctness in 286ab84. This PR introduces a substantial new feature (remote SSH environment launch) with ~1,200 lines of new code handling SSH tunneling, credential management, and password prompts. Multiple unresolved review comments identify high and medium severity bugs affecting core functionality, including user-provided credentials being discarded and parsing failures on hosts with MOTD banners. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Non-SSH 401 retry incorrectly enters SSH recovery path
- Added
!activeRecord.desktopSshto the throw condition so non-SSH environments that hit a 401 re-throw the original error instead of falling through toissueDesktopSshBearerSession.
- Added
- ✅ Fixed: SSH config parser incorrectly strips inline comments
- Changed
stripInlineCommentto only treat lines starting with#as comments (returning empty string), matching the OpenSSH spec which has no inline comment syntax.
- Changed
Or push these changes by commenting:
@cursor push 19ad5e12e4
Preview (19ad5e12e4)
diff --git a/apps/desktop/src/sshEnvironment.ts b/apps/desktop/src/sshEnvironment.ts
--- a/apps/desktop/src/sshEnvironment.ts
+++ b/apps/desktop/src/sshEnvironment.ts
@@ -65,8 +65,8 @@
const NO_HOSTS = [] as const;
function stripInlineComment(line: string): string {
- const hashIndex = line.indexOf("#");
- return (hashIndex >= 0 ? line.slice(0, hashIndex) : line).trim();
+ const trimmed = line.trim();
+ return trimmed.startsWith("#") ? "" : trimmed;
}
function splitDirectiveArgs(value: string): ReadonlyArray<string> {
diff --git a/apps/web/src/environments/runtime/service.ts b/apps/web/src/environments/runtime/service.ts
--- a/apps/web/src/environments/runtime/service.ts
+++ b/apps/web/src/environments/runtime/service.ts
@@ -1079,7 +1079,7 @@
const isAuthError = activeRecord.desktopSsh
? isSshHttpAuthError(error, 401)
: isRemoteEnvironmentAuthHttpError(error) && error.status === 401;
- if (!isAuthError) {
+ if (!isAuthError || !activeRecord.desktopSsh) {
throw error;
}You can send follow-ups to the cloud agent here.
| function stripInlineComment(line: string): string { | ||
| const hashIndex = line.indexOf("#"); | ||
| return (hashIndex >= 0 ? line.slice(0, hashIndex) : line).trim(); | ||
| } |
There was a problem hiding this comment.
SSH config parser incorrectly strips inline comments
Medium Severity
stripInlineComment strips everything after # on every line, but SSH config files only treat # as a comment character when it appears at the start of a line. A Host alias or Include path containing # (e.g., Host myhost#1 or Include config.d/team#2.conf) would be silently truncated, causing hosts to be missed or included files not to be found.
Reviewed by Cursor Bugbot for commit 0cd33d0. Configure here.
juliusmarminge
force-pushed
the
t3code/remote-ssh-launch
branch
from
0cd33d0 to
1e34360
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale
bearerTokenvariable captured in WsTransport closure- Deferred registerConnection until after the metadata refresh succeeds, so a connection capturing a stale activeBearerToken is never registered in the global map; in the 401 retry path the unregistered connection is disposed directly instead of via removeConnection.
Or push these changes by commenting:
@cursor push 02d5557ab2
Preview (02d5557ab2)
diff --git a/apps/web/src/environments/runtime/service.ts b/apps/web/src/environments/runtime/service.ts
--- a/apps/web/src/environments/runtime/service.ts
+++ b/apps/web/src/environments/runtime/service.ts
@@ -1064,8 +1064,6 @@
...createEnvironmentConnectionHandlers(),
});
- registerConnection(connection);
-
try {
try {
await refreshSavedEnvironmentMetadata(
@@ -1087,7 +1085,7 @@
activeRecord = issued.record;
bearerToken = issued.bearerToken;
roleHint = issued.role;
- await removeConnection(activeRecord.environmentId).catch(() => false);
+ await connection.dispose();
pendingSavedEnvironmentConnections.delete(activeRecord.environmentId);
return await ensureSavedEnvironmentConnection(activeRecord, {
bearerToken,
@@ -1095,10 +1093,14 @@
serverConfig: options?.serverConfig ?? null,
});
}
+ registerConnection(connection);
return connection;
} catch (error) {
setRuntimeError(activeRecord.environmentId, error);
- await removeConnection(activeRecord.environmentId).catch(() => false);
+ const removed = await removeConnection(activeRecord.environmentId).catch(() => false);
+ if (!removed) {
+ await connection.dispose().catch(() => {});
+ }
throw error;
}
})();You can send follow-ups to the cloud agent here.
- Discover SSH hosts and persist SSH targets - Bootstrap tunneled SSH sessions with desktop password prompts - Extend IPC and storage tests for SSH metadata
- Validate SSH targets and known-host parsing more strictly - Retry desktop SSH session refresh on auth failures - Preserve saved registry state when bearer persistence fails
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
t3code/remote-ssh-launch
branch
from
2462e77 to
ad7c23b
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
| resolvedTarget: DesktopSshEnvironmentTarget, | ||
| packageSpec?: string, | ||
| ): Promise<SshTunnelEntry> { | ||
| let entry = this.tunnels.get(key) ?? null; |
There was a problem hiding this comment.
🟡 Medium src/sshEnvironment.ts:1001
Concurrent callers in ensureTunnelEntry race against each other. The second caller checks this.tunnels.get(key) at line 1001 before checking this.pendingTunnelEntries.get(key) at line 1014. If the first caller has set the tunnel at line 1092 but hasn't completed tunnelReady, the second caller's 2-second health check fails and calls stopTunnel, killing the SSH process the first caller is still waiting on. Both callers then fail. Consider checking pendingTunnelEntries first, before validating this.tunnels.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/desktop/src/sshEnvironment.ts around line 1001:
Concurrent callers in `ensureTunnelEntry` race against each other. The second caller checks `this.tunnels.get(key)` at line 1001 before checking `this.pendingTunnelEntries.get(key)` at line 1014. If the first caller has set the tunnel at line 1092 but hasn't completed `tunnelReady`, the second caller's 2-second health check fails and calls `stopTunnel`, killing the SSH process the first caller is still waiting on. Both callers then fail. Consider checking `pendingTunnelEntries` first, before validating `this.tunnels`.
Evidence trail:
apps/desktop/src/sshEnvironment.ts lines 1001-1017 (tunnel check before pendingTunnelEntries check), line 1092 (tunnels.set before tunnelReady), line 1094 (await tunnelReady), line 1104 (pendingTunnelEntries.set). The race occurs when Caller B enters between lines 1092 and tunnelReady completion, finds the tunnel, fails the 2-second health check, and kills the SSH process at line 1008.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Auth failure regex misses single-method SSH errors
- Changed
[^)]+to[^)]*in theisSshAuthFailureregex so single-method SSH error messages likePermission denied (password).are correctly matched.
- Changed
- ✅ Fixed: Unbounded recursion in SSH 401 retry path
- Added a
_sshAuthRetryCountoption with a max of 2 retries to the recursiveensureSavedEnvironmentConnectioncall, preventing unbounded recursion on persistent 401 errors.
- Added a
Or push these changes by commenting:
@cursor push 59e44475a5
Preview (59e44475a5)
diff --git a/apps/desktop/src/sshEnvironment.ts b/apps/desktop/src/sshEnvironment.ts
--- a/apps/desktop/src/sshEnvironment.ts
+++ b/apps/desktop/src/sshEnvironment.ts
@@ -509,7 +509,7 @@
const message = error instanceof Error ? error.message : String(error);
const normalized = message.toLowerCase();
return (
- /permission denied \((?:publickey|password|keyboard-interactive|hostbased|gssapi-with-mic)[^)]+\)/u.test(
+ /permission denied \((?:publickey|password|keyboard-interactive|hostbased|gssapi-with-mic)[^)]*\)/u.test(
normalized,
) ||
/authentication failed/u.test(normalized) ||
diff --git a/apps/web/src/environments/runtime/service.ts b/apps/web/src/environments/runtime/service.ts
--- a/apps/web/src/environments/runtime/service.ts
+++ b/apps/web/src/environments/runtime/service.ts
@@ -1003,6 +1003,7 @@
readonly bearerToken?: string;
readonly role?: AuthSessionRole | null;
readonly serverConfig?: ServerConfig | null;
+ readonly _sshAuthRetryCount?: number;
},
): Promise<EnvironmentConnection> {
const existing = environmentConnections.get(record.environmentId);
@@ -1105,6 +1106,13 @@
});
}
+ const retryCount = options?._sshAuthRetryCount ?? 0;
+ if (retryCount >= 2) {
+ throw new Error("SSH authentication failed after retrying with fresh tokens.", {
+ cause: error,
+ });
+ }
+
const issued = await issueDesktopSshBearerSession(activeRecord);
activeRecord = issued.record;
bearerToken = issued.bearerToken;
@@ -1115,6 +1123,7 @@
bearerToken,
role: roleHint,
serverConfig: options?.serverConfig ?? null,
+ _sshAuthRetryCount: retryCount + 1,
});
}
registerConnection(connection);You can send follow-ups to the cloud agent here.
| const message = error instanceof Error ? error.message : String(error); | ||
| const normalized = message.toLowerCase(); | ||
| return ( | ||
| /permission denied \((?:publickey|password|keyboard-interactive|hostbased|gssapi-with-mic)[^)]+\)/u.test( |
There was a problem hiding this comment.
Auth failure regex misses single-method SSH errors
Medium Severity
The isSshAuthFailure regex uses [^)]+ (one or more) after the auth method alternation, which requires at least one additional character before the closing parenthesis. This means SSH error messages with a single auth method like "Permission denied (password)." won't match, because there are no characters between password and ). Servers configured with only password authentication would produce this message, and the password prompt flow would never trigger — the error would propagate as a non-auth failure instead. Changing [^)]+ to [^)]* (zero or more) would fix this.
Reviewed by Cursor Bugbot for commit 2e90005. Configure here.
| bearerToken, | ||
| role: roleHint, | ||
| serverConfig: options?.serverConfig ?? null, | ||
| }); |
There was a problem hiding this comment.
Unbounded recursion in SSH 401 retry path
Medium Severity
When refreshSavedEnvironmentMetadata fails with a 401 for an SSH connection, the code issues a fresh bearer token via issueDesktopSshBearerSession and recursively calls ensureSavedEnvironmentConnection with no retry limit. If the remote server consistently rejects fresh tokens with 401, this creates unbounded recursion — each recursive call triggers the same 401 → reissue → recurse cycle indefinitely, eventually causing a stack overflow or hanging the app.
Reviewed by Cursor Bugbot for commit 2e90005. Configure here.
- Resolve dev remote package specs to `t3@nightly` - Cover the dev fallback in sshEnvironment tests
- surface stdout when remote launch or pairing fails - report parse errors and invalid remote port or credential values
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: User-provided SSH username and port silently discarded
- ensureEnvironment now overlays the caller-supplied username and port (when non-null) onto the ssh -G resolved target instead of discarding them.
- ✅ Fixed: SSH config Include paths resolved relative to parent file, not ~/.ssh
- resolveSshConfigIncludePattern now resolves relative Include patterns against ~/.ssh/ (matching OpenSSH ssh_config(5) behavior) instead of the parent file's directory, with homeDir threaded through collectSshConfigAliasesFromFile.
Or push these changes by commenting:
@cursor push d65b7e1602
Preview (d65b7e1602)
diff --git a/apps/desktop/src/sshEnvironment.ts b/apps/desktop/src/sshEnvironment.ts
--- a/apps/desktop/src/sshEnvironment.ts
+++ b/apps/desktop/src/sshEnvironment.ts
@@ -86,13 +86,13 @@
function resolveSshConfigIncludePattern(
includePattern: string,
- directory: string,
+ _directory: string,
homeDir: string = OS.homedir(),
): string {
const expandedPattern = expandHomePath(includePattern, homeDir);
return Path.isAbsolute(expandedPattern)
? expandedPattern
- : Path.resolve(directory, expandedPattern);
+ : Path.resolve(Path.join(homeDir, ".ssh"), expandedPattern);
}
function hasSshPattern(value: string): boolean {
@@ -132,6 +132,7 @@
function collectSshConfigAliasesFromFile(
filePath: string,
visited = new Set<string>(),
+ homeDir: string = OS.homedir(),
): ReadonlyArray<string> {
const resolvedPath = Path.resolve(filePath);
if (visited.has(resolvedPath) || !FS.existsSync(resolvedPath)) {
@@ -153,9 +154,9 @@
const normalizedDirective = directive.toLowerCase();
if (normalizedDirective === "include") {
for (const includePattern of rawArgs) {
- const resolvedPattern = resolveSshConfigIncludePattern(includePattern, directory);
+ const resolvedPattern = resolveSshConfigIncludePattern(includePattern, directory, homeDir);
for (const includedPath of expandGlob(resolvedPattern)) {
- for (const alias of collectSshConfigAliasesFromFile(includedPath, visited)) {
+ for (const alias of collectSshConfigAliasesFromFile(includedPath, visited, homeDir)) {
aliases.add(alias);
}
}
@@ -884,8 +885,13 @@
export async function discoverDesktopSshHosts(input?: {
readonly homeDir?: string;
}): Promise<readonly DesktopDiscoveredSshHost[]> {
- const sshDirectory = Path.join(input?.homeDir ?? OS.homedir(), ".ssh");
- const configAliases = collectSshConfigAliasesFromFile(Path.join(sshDirectory, "config"));
+ const homeDir = input?.homeDir ?? OS.homedir();
+ const sshDirectory = Path.join(homeDir, ".ssh");
+ const configAliases = collectSshConfigAliasesFromFile(
+ Path.join(sshDirectory, "config"),
+ new Set<string>(),
+ homeDir,
+ );
const knownHosts = readKnownHostsHostnames(Path.join(sshDirectory, "known_hosts"));
const discovered = new Map<string, DesktopDiscoveredSshHost>();
@@ -1002,7 +1008,12 @@
target: DesktopSshEnvironmentTarget,
options?: { readonly issuePairingToken?: boolean },
): Promise<DesktopSshEnvironmentBootstrap> {
- const resolvedTarget = await resolveDesktopSshTarget(target.alias || target.hostname);
+ const baseResolved = await resolveDesktopSshTarget(target.alias || target.hostname);
+ const resolvedTarget: DesktopSshEnvironmentTarget = {
+ ...baseResolved,
+ ...(target.username !== null ? { username: target.username } : {}),
+ ...(target.port !== null ? { port: target.port } : {}),
+ };
const key = targetConnectionKey(resolvedTarget);
const packageSpec = this.options.resolveCliPackageSpec?.();
const entry = await this.ensureTunnelEntry(key, resolvedTarget, packageSpec);You can send follow-ups to the cloud agent here.
| target: DesktopSshEnvironmentTarget, | ||
| options?: { readonly issuePairingToken?: boolean }, | ||
| ): Promise<DesktopSshEnvironmentBootstrap> { | ||
| const resolvedTarget = await resolveDesktopSshTarget(target.alias || target.hostname); |
There was a problem hiding this comment.
User-provided SSH username and port silently discarded
High Severity
ensureEnvironment calls resolveDesktopSshTarget(target.alias || target.hostname) which completely re-resolves the SSH target by running ssh -G, discarding the user-supplied username and port from the incoming target. When a user manually enters SSH connection details with a specific username or non-default port in the UI, those values are silently overridden by whatever ssh -G returns (typically SSH config defaults or Host * fallbacks).
Reviewed by Cursor Bugbot for commit d2cd406. Configure here.
| return Path.isAbsolute(expandedPattern) | ||
| ? expandedPattern | ||
| : Path.resolve(directory, expandedPattern); | ||
| } |
There was a problem hiding this comment.
SSH config Include paths resolved relative to parent file, not ~/.ssh
Low Severity
resolveSshConfigIncludePattern resolves non-absolute, non-tilde Include patterns relative to the including file's directory, but OpenSSH specifies that relative Include paths are resolved relative to ~/.ssh/. For the top-level config file this happens to work (since it lives in ~/.ssh/), but any included file from a subdirectory that itself uses a relative Include would resolve against the wrong base directory.
Reviewed by Cursor Bugbot for commit d2cd406. Configure here.
- Add a capped scroll area for discovered SSH hosts - Keep the manual SSH form always visible and simplify the dialog layout - Ensure the scroll area viewport respects inherited max height
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Pairing token parsing uses full stdout instead of last line
- Replaced
result.stdout.trim()withgetLastNonEmptyOutputLine(result.stdout)inissueRemotePairingTokento correctly extract only the last JSON line, matching the pattern already used inlaunchOrReuseRemoteServer.
- Replaced
Or push these changes by commenting:
@cursor push baafca331a
Preview (baafca331a)
diff --git a/apps/desktop/src/sshEnvironment.ts b/apps/desktop/src/sshEnvironment.ts
--- a/apps/desktop/src/sshEnvironment.ts
+++ b/apps/desktop/src/sshEnvironment.ts
@@ -805,8 +805,8 @@
...(input?.batchMode === undefined ? {} : { batchMode: input.batchMode }),
...(input?.interactiveAuth === undefined ? {} : { interactiveAuth: input.interactiveAuth }),
});
- const stdout = result.stdout.trim();
- if (!stdout) {
+ const line = getLastNonEmptyOutputLine(result.stdout);
+ if (!line) {
throw new Error(
`SSH pairing did not return a credential. stdout=${JSON.stringify(result.stdout)}`,
);
@@ -814,10 +814,10 @@
let parsed: { credential?: unknown };
try {
- parsed = JSON.parse(stdout) as { credential?: unknown };
+ parsed = JSON.parse(line) as { credential?: unknown };
} catch (cause) {
throw new Error(
- `SSH pairing returned unparseable output. stdout=${JSON.stringify(result.stdout)}`,
+ `SSH pairing returned unparseable output. line=${JSON.stringify(line)} stdout=${JSON.stringify(result.stdout)}`,
{ cause },
);
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 286ab84. Configure here.
|
|
||
| let parsed: { credential?: unknown }; | ||
| try { | ||
| parsed = JSON.parse(stdout) as { credential?: unknown }; |
There was a problem hiding this comment.
Pairing token parsing uses full stdout instead of last line
High Severity
issueRemotePairingToken calls JSON.parse on the entire trimmed stdout, while the analogous launchOrReuseRemoteServer correctly uses getLastNonEmptyOutputLine to extract only the final JSON line. SSH sessions can produce MOTD banners, shell initialization output, or other text before the actual command output, causing JSON.parse to fail on hosts with any preamble text.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 286ab84. Configure here.
1 participant
Summary
Testing
bun fmt,bun lint,bun typecheck, andbun run testshould pass before merge.Note
High Risk
Introduces new IPC surfaces and spawns/manages SSH child processes while also modifying saved-environment auth/credential handling (including recovery/rollback paths), making failures potentially impact security and connection reliability.
Overview
Adds SSH-managed remote environments end-to-end: the desktop main process can now discover SSH hosts, establish/reuse SSH tunnels, and proxy auth/session/WebSocket-token requests to a loopback-forwarded server via new IPC channels.
Saved environment persistence is extended to carry optional
desktopSshmetadata (desktop + browser storage) and the web runtime now supports connecting/reconnecting SSH environments (including concurrency de-dupe, 401 credential recovery by re-issuing bearer tokens, and registry rollback on persistence failures).UI updates add an SSH tab to “Add environment” (host discovery + manual entry) plus a modal
SshPasswordPromptDialogthat services main-process password requests; includes new contracts/types and extensive unit tests for SSH parsing, tunnel lifecycle edge cases, and the new runtime flows.Reviewed by Cursor Bugbot for commit 286ab84. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add remote SSH environment launch support to the desktop app
sshEnvironmentmodule (sshEnvironment.ts) that discovers SSH hosts from~/.ssh/configandknown_hosts, establishes tunnels, launches remote T3 server processes, and manages lifecycle of SSH processes.DesktopBridgeand renderer preload to expose SSH methods (discoverSshHosts,ensureSshEnvironment,bootstrapSshBearerSession, etc.) and an SSH password prompt dialog (SshPasswordPromptDialog.tsx) mounted in the root view.desktopSshfield, with fixes to prevent the field being dropped during registry/secret writes.Macroscope summarized 286ab84.