Skip to content
Merged
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
1 change: 0 additions & 1 deletion src/main/http/ssh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,16 @@
});

// Get config hosts
app.get('/api/ssh/config-hosts', async () => {
try {
const hosts = await connectionManager.getConfigHosts();
return { success: true, data: hosts };
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
logger.error('Failed to get SSH config hosts:', message);
return { success: true, data: [] };
}
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.

// Resolve host
app.post<{ Body: { alias: string } }>('/api/ssh/resolve-host', async (request) => {
Expand All @@ -108,7 +108,6 @@
port: config.port,
username: config.username,
authMethod: config.authMethod,
privateKeyPath: config.privateKeyPath,
},
});
return { success: true };
Expand Down
4 changes: 3 additions & 1 deletion src/main/ipc/configValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,9 @@ function isValidSshProfile(profile: unknown): boolean {
if (typeof profile.host !== 'string') return false;
if (typeof profile.port !== 'number') return false;
if (typeof profile.username !== 'string') return false;
const validMethods = ['password', 'privateKey', 'agent', 'auto'];
// Accept current values plus legacy ('auto', 'agent', 'privateKey') so older
// configs round-trip; legacy values are normalized to 'sshConfig' on read.
const validMethods = ['password', 'sshConfig', 'auto', 'agent', 'privateKey'];
if (!validMethods.includes(profile.authMethod as string)) return false;
Comment on lines +379 to 381
return true;
}
Expand Down
1 change: 0 additions & 1 deletion src/main/ipc/ssh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ export function registerSshHandlers(ipcMain: IpcMain): void {
port: config.port,
username: config.username,
authMethod: config.authMethod,
privateKeyPath: config.privateKeyPath,
},
});
return { success: true };
Expand Down
2 changes: 1 addition & 1 deletion src/main/services/infrastructure/ConfigManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export interface SshPersistConfig {
host: string;
port: number;
username: string;
authMethod: 'password' | 'privateKey' | 'agent' | 'auto';
authMethod: 'password' | 'sshConfig';
privateKeyPath?: string;
} | null;
autoReconnect: boolean;
Expand Down
196 changes: 158 additions & 38 deletions src/main/services/infrastructure/SshConfigParser.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
/**
* SshConfigParser - Parses ~/.ssh/config to resolve host aliases.
*
* Responsibilities:
* - Parse SSH config with Include directive support
* - Return all defined Host aliases (excluding wildcards)
* - Resolve alias to HostName, Port, User, IdentityFile
* - Gracefully handle missing/unreadable files
* Two responsibilities:
* - `getHosts()`: enumerate every Host alias for the dropdown autocomplete.
* Uses a line-based scanner because the `ssh-config` library was silently
* dropping hosts in some configurations (mixed indentation, comment-heavy
* sections, Include directives). For the dropdown we just need the names —
* not full config resolution — so a forgiving parser is the right tool.
* - `resolveHost()`: full per-alias resolution. Uses the `ssh-config` library
* which understands `Match`, multi-alias Host lines, and config inheritance.
*
* Both methods follow `Include` directives, expanding paths and globs.
*/

import { createLogger } from '@shared/utils/logger';
Expand All @@ -27,31 +32,17 @@ export class SshConfigParser {

/**
* Returns all defined Host aliases (excluding `*` wildcards and patterns).
*
* Uses a forgiving line-based scan instead of the `ssh-config` library so
* that an unparseable block doesn't take out the rest of the config —
* users have lots of weird stuff in their ssh_config (gcloud-generated
* sections, OrbStack Includes, comment blocks).
*/
async getHosts(): Promise<SshConfigHostEntry[]> {
try {
const config = await this.parseConfig();
if (!config) return [];

const entries: SshConfigHostEntry[] = [];

for (const section of config) {
if (section.type !== SSHConfig.DIRECTIVE) continue;
if (section.param !== 'Host') continue;

const hostValue = section.value;
if (typeof hostValue !== 'string') continue;

// Skip wildcard-only entries and patterns with * or ?
const aliases = hostValue.split(/\s+/).filter((h) => !h.includes('*') && !h.includes('?'));

for (const alias of aliases) {
const resolved = this.resolveFromConfig(config, alias);
entries.push(resolved);
}
}

return entries;
const content = await this.readExpandedConfig();
if (content === null) return [];
return parseHostListing(content);
} catch (err) {
logger.error('Failed to get SSH config hosts:', err);
return [];
Expand All @@ -70,8 +61,12 @@ export class SshConfigParser {
const resolved = this.resolveFromConfig(config, alias);

// If nothing was resolved beyond the alias itself, check if host was actually defined
if (!resolved.hostName && !resolved.user && !resolved.port && !resolved.identityFiles?.length) {
// Check if there's an explicit Host entry for this alias
if (
!resolved.hostName &&
!resolved.user &&
!resolved.port &&
!resolved.identityFiles?.length
) {
const hasEntry = config.some(
(section) =>
section.type === SSHConfig.DIRECTIVE &&
Expand Down Expand Up @@ -102,9 +97,12 @@ export class SshConfigParser {
const user = Array.isArray(rawUser) ? rawUser[0] : (rawUser ?? undefined);
const portStr = computed.Port;
const port = portStr ? parseInt(String(portStr), 10) : undefined;
// Resolve identity file paths (expand ~ to home directory)
const rawIdentityFile = computed.IdentityFile;
const rawFiles = Array.isArray(rawIdentityFile) ? rawIdentityFile : rawIdentityFile != null ? [rawIdentityFile] : [];
const rawFiles = Array.isArray(rawIdentityFile)
? rawIdentityFile
: rawIdentityFile != null
? [rawIdentityFile]
: [];
const identityFiles = rawFiles
.filter((f): f is string => typeof f === 'string')
.map((f) => f.replace(/^~(?=$|\/|\\)/, os.homedir()));
Expand All @@ -119,18 +117,25 @@ export class SshConfigParser {
}

private async parseConfig(): Promise<SSHConfig | null> {
const content = await this.readExpandedConfig();
if (content === null) return null;
try {
let content = await fs.promises.readFile(this.configPath, 'utf8');

// Process Include directives by expanding them inline
content = await this.expandIncludes(content);

return SSHConfig.parse(content);
} catch (err) {
logger.error('Failed to parse SSH config:', err);
return null;
}
}

private async readExpandedConfig(): Promise<string | null> {
try {
const content = await fs.promises.readFile(this.configPath, 'utf8');
return await this.expandIncludes(content);
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
logger.info('No SSH config file found at', this.configPath);
} else {
logger.error('Failed to parse SSH config:', err);
logger.error('Failed to read SSH config:', err);
}
return null;
}
Expand All @@ -156,7 +161,6 @@ export class SshConfigParser {
const expandedPattern = pattern.replace(/^~/, os.homedir());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

SSH Include directives with relative paths should be resolved relative to ~/.ssh/ (or /etc/ssh/ for root), but the current implementation resolves them relative to the process's current working directory. This can cause includes to fail depending on how the application was launched.

      let expandedPattern = pattern.replace(/^~/, os.homedir());
      if (!path.isAbsolute(expandedPattern)) {
        expandedPattern = path.join(path.dirname(this.configPath), expandedPattern);
      }


try {
// Handle glob-like patterns by checking if the path contains wildcards
if (expandedPattern.includes('*') || expandedPattern.includes('?')) {
const dir = path.dirname(expandedPattern);
const globPart = path.basename(expandedPattern);
Expand Down Expand Up @@ -194,3 +198,119 @@ export class SshConfigParser {
}
}
}

// =============================================================================
// Line-based host listing
// =============================================================================

/**
* Walks the (already include-expanded) config text and returns one entry per
* Host alias. Lenient: tolerates mixed indentation, `key=value` and `key value`
* forms, and comments. Unrecognized directives are silently skipped.
*/
function parseHostListing(content: string): SshConfigHostEntry[] {
const entries: SshConfigHostEntry[] = [];
// Multiple aliases on the same `Host a b c` line share the same body, so we
// accumulate to a list of "current entries" that all receive the next props.
let current: SshConfigHostEntry[] = [];

const flush = (): void => {
for (const entry of current) {
// Don't echo identity defaults that aren't explicitly set in the block.
if (entry.identityFiles?.length === 0) {
delete entry.identityFiles;
}
entries.push(entry);
}
current = [];
};

for (const rawLine of content.split('\n')) {
const line = stripComment(rawLine).trim();
if (!line) continue;

const kv = parseKeyValue(line);
if (!kv) continue;

const key = kv.key.toLowerCase();
const value = kv.value;

if (key === 'host') {
flush();
const aliases = value.split(/\s+/).filter((a) => a && !a.includes('*') && !a.includes('?'));
current = aliases.map((alias) => ({ alias }));
continue;
}

if (current.length === 0) continue;

for (const entry of current) {
applyDirective(entry, key, value);
}
}

flush();
return entries;
}

/* eslint-disable no-param-reassign --
`entry` is the in-progress builder for a Host block. Imperative mutation
is the natural shape for a line-by-line parser; the alternative (returning
a new object every line) would just churn allocations for no readability
gain. */
function applyDirective(entry: SshConfigHostEntry, key: string, value: string): void {
switch (key) {
case 'hostname':
if (value !== entry.alias) entry.hostName = value;
break;
case 'user':
entry.user = value;
break;
case 'port': {
const port = parseInt(value, 10);
if (!Number.isNaN(port) && port !== 22) entry.port = port;
break;
}
case 'identityfile': {
const expanded = value.replace(/^~(?=$|\/|\\)/, os.homedir());
if (!entry.identityFiles) entry.identityFiles = [];
entry.identityFiles.push(expanded);
break;
}
default:
// Unrecognized directives don't appear in the dropdown; ignore.
break;
}
}
/* eslint-enable no-param-reassign -- end builder mutation block */

function stripComment(line: string): string {
const idx = line.indexOf('#');
return idx === -1 ? line : line.slice(0, idx);
}

function parseKeyValue(line: string): { key: string; value: string } | null {
// Both `Key Value ...` and `Key=Value ...` are valid in OpenSSH config.
// Avoid regex to keep this linear-time on long lines.
const eqIdx = line.indexOf('=');
const wsIdx = findWhitespace(line);
if (eqIdx !== -1 && (wsIdx === -1 || eqIdx < wsIdx)) {
const key = line.slice(0, eqIdx).trim();
const value = line.slice(eqIdx + 1).trim();
if (!key) return null;
return { key, value };
}
if (wsIdx === -1) return null;
const key = line.slice(0, wsIdx);
const value = line.slice(wsIdx + 1).trim();
if (!key || !value) return null;
return { key, value };
}
Comment on lines +292 to +308
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The parseKeyValue logic incorrectly handles cases where the key and value are separated by an equals sign with a preceding space (e.g., Host = myhost). In such cases, the = is mistakenly included in the value, which will break host resolution and dropdown population. OpenSSH allows whitespace around the = separator.

function parseKeyValue(line: string): { key: string; value: string } | null {
  // Both `Key Value ...` and `Key=Value ...` are valid in OpenSSH config.
  // We need to find the first separator which can be '=' or whitespace.
  const eqIdx = line.indexOf('=');
  const wsIdx = findWhitespace(line);

  let sepIdx = -1;
  if (eqIdx !== -1 && wsIdx !== -1) {
    sepIdx = Math.min(eqIdx, wsIdx);
  } else if (eqIdx !== -1) {
    sepIdx = eqIdx;
  } else if (wsIdx !== -1) {
    sepIdx = wsIdx;
  } else {
    return null;
  }

  const key = line.slice(0, sepIdx).trim();
  let value = line.slice(sepIdx + 1).trim();

  // If we split on whitespace, check if the value starts with '=' (e.g., "Key = Value")
  if (sepIdx === wsIdx && value.startsWith('=')) {
    value = value.slice(1).trim();
  }

  if (!key || !value) return null;
  return { key, value };
}


function findWhitespace(s: string): number {
for (let i = 0; i < s.length; i += 1) {
const c = s.charCodeAt(i);
if (c === 0x20 || c === 0x09) return i;
}
return -1;
}
Loading
Loading