Fix SSH Key Password (keyPassword) Field Naming Mismatch Between Frontend and Backend #375

Merged
thorved merged 3 commits from fix--sshpasskey-missmatch into dev-1.7.3 2025-10-07 21:05:34 +00:00
4 changed files with 28 additions and 27 deletions

View File

@@ -91,7 +91,7 @@ router.get("/db/host/internal", async (req: Request, res: Response) => {
username: host.username,
password: host.autostartPassword,
key: host.autostartKey,
key_password: host.autostartKeyPassword,
keyPassword: host.autostartKeyPassword,
autostartPassword: host.autostartPassword,
autostartKey: host.autostartKey,
autostartKeyPassword: host.autostartKeyPassword,
@@ -151,7 +151,7 @@ router.get("/db/host/internal/all", async (req: Request, res: Response) => {
username: host.username,
password: host.autostartPassword || host.password,
key: host.autostartKey || host.key,
key_password: host.autostartKeyPassword || host.key_password,
keyPassword: host.autostartKeyPassword || host.key_password,
autostartPassword: host.autostartPassword,
autostartKey: host.autostartKey,
autostartKeyPassword: host.autostartKeyPassword,
@@ -226,7 +226,7 @@ router.post(
authType,
credentialId,
key,
key_password,
keyPassword,
keyType,
pin,
enableTerminal,
@@ -278,7 +278,7 @@ router.post(
sshDataObj.keyType = null;
} else if (effectiveAuthType === "key") {
sshDataObj.key = key || null;
sshDataObj.key_password = key_password || null;
sshDataObj.key_password = keyPassword || null;
sshDataObj.keyType = keyType;
sshDataObj.password = null;
} else {
@@ -407,7 +407,7 @@ router.put(
authType,
credentialId,
key,
key_password,
keyPassword,
keyType,
pin,
enableTerminal,
@@ -464,8 +464,8 @@ router.put(
if (key) {
sshDataObj.key = key;
}
if (key_password !== undefined) {
sshDataObj.key_password = key_password || null;
if (keyPassword !== undefined) {
sshDataObj.key_password = keyPassword || null;
}
if (keyType) {
sshDataObj.keyType = keyType;
@@ -711,7 +711,7 @@ router.get(
authType: resolvedHost.authType,
password: resolvedHost.password || null,
key: resolvedHost.key || null,
key_password: resolvedHost.key_password || null,
keyPassword: resolvedHost.keyPassword || null,
keyType: resolvedHost.keyType || null,
folder: resolvedHost.folder,
tags:
@@ -1234,12 +1234,27 @@ async function resolveHostCredentials(host: any): Promise<any> {
authType: credential.auth_type || credential.authType,
gemini-code-assist[bot] commented 2025-10-07 20:33:05 +00:00 (Migrated from github.com)
Review

high

There's a potential bug in this conversion logic. If a host object uses a credentialId but also has a stale key_password property from a previous configuration, this logic will incorrectly overwrite the keyPassword from the credential with the stale key_password from the host object.

This happens because the credential resolution logic merges credential properties into the host object, but the original host.key_password is preserved. This block then unconditionally uses host.key_password if it exists.

To fix this, we should only perform the conversion from key_password to keyPassword if keyPassword hasn't already been set by the credential resolution logic.

    const result = { ...host };
    if (host.key_password !== undefined) {
      // If keyPassword is not already set by credential resolution, use the inline one.
      if (result.keyPassword === undefined) {
        result.keyPassword = host.key_password;
      }
      // Always remove the snake_case version to standardize the output.
      delete result.key_password;
    }
    return result;
![high](https://www.gstatic.com/codereviewagent/high-priority.svg) There's a potential bug in this conversion logic. If a `host` object uses a `credentialId` but also has a stale `key_password` property from a previous configuration, this logic will incorrectly overwrite the `keyPassword` from the credential with the stale `key_password` from the host object. This happens because the credential resolution logic merges credential properties into the `host` object, but the original `host.key_password` is preserved. This block then unconditionally uses `host.key_password` if it exists. To fix this, we should only perform the conversion from `key_password` to `keyPassword` if `keyPassword` hasn't already been set by the credential resolution logic. ```typescript const result = { ...host }; if (host.key_password !== undefined) { // If keyPassword is not already set by credential resolution, use the inline one. if (result.keyPassword === undefined) { result.keyPassword = host.key_password; } // Always remove the snake_case version to standardize the output. delete result.key_password; } return result; ```
thorved commented 2025-10-07 20:51:17 +00:00 (Migrated from github.com)
Review

Key Password Precedence Fix

This document summarizes the small but critical fix that prevents stale inline key_password values from overwriting credential-resolved keyPassword values when resolving SSH host credentials.

const result = { ...host };
if (host.key_password !== undefined) {
  // Only use the inline key_password if keyPassword hasn't been set by credential resolution
  if (result.keyPassword === undefined) {
    result.keyPassword = host.key_password;
  }
  // Always remove the snake_case version to standardize the output
  delete result.key_password;
}

Why this matters

  • Hosts can be migrated from inline credentials to credential-backed configurations. If the old key_password remains on the host row, it must not overwrite the keyPassword supplied by the referenced credential.

The fix correctly:

  1. Checks if keyPassword is already set by credential resolution (line 1245 in src/backend/database/routes/ssh.ts).
  2. Only uses host.key_password if keyPassword is undefined (line 1246).
  3. Always removes the snake_case version (key_password) to maintain a clean, camelCase API contract (line 1249).

Result: This prevents stale inline credentials from overwriting credential-resolved values and ensures proper precedence when building the API response.

# Key Password Precedence Fix This document summarizes the small but critical fix that prevents stale inline `key_password` values from overwriting credential-resolved `keyPassword` values when resolving SSH host credentials. ```typescript const result = { ...host }; if (host.key_password !== undefined) { // Only use the inline key_password if keyPassword hasn't been set by credential resolution if (result.keyPassword === undefined) { result.keyPassword = host.key_password; } // Always remove the snake_case version to standardize the output delete result.key_password; } ``` Why this matters - Hosts can be migrated from inline credentials to credential-backed configurations. If the old `key_password` remains on the host row, it must not overwrite the `keyPassword` supplied by the referenced credential. The fix correctly: 1. Checks if `keyPassword` is already set by credential resolution (line 1245 in `src/backend/database/routes/ssh.ts`). 2. Only uses `host.key_password` if `keyPassword` is `undefined` (line 1246). 3. Always removes the snake_case version (`key_password`) to maintain a clean, camelCase API contract (line 1249). Result: This prevents stale inline credentials from overwriting credential-resolved values and ensures proper precedence when building the API response.
password: credential.password,
key: credential.key,
key_password: credential.key_password || credential.key_password,
keyPassword: credential.key_password || credential.keyPassword,
keyType: credential.key_type || credential.keyType,
};
}
}
return host;
const result = { ...host };
if (host.key_password !== undefined) {
if (result.keyPassword === undefined) {
result.keyPassword = host.key_password;
}
delete result.key_password;
}
const result = { ...host };
if (host.key_password !== undefined) {
if (result.keyPassword === undefined) {
result.keyPassword = host.key_password;
}
delete result.key_password;
}
return result;
} catch (error) {
sshLogger.warn(
`Failed to resolve credentials for host ${host.id}: ${error instanceof Error ? error.message : "Unknown error"}`,

View File

@@ -478,7 +478,7 @@ async function resolveHostCredentials(
function addLegacyCredentials(baseHost: any, host: any): void {
baseHost.password = host.password || null;
baseHost.key = host.key || null;
baseHost.keyPassword = host.keyPassword || null;
baseHost.keyPassword = host.key_password || host.keyPassword || null;
baseHost.keyType = host.keyType;
}

View File

@@ -877,21 +877,6 @@ export function HostManagerEditor({
| "credential";
setAuthTab(newAuthType);
form.setValue("authType", newAuthType);
if (newAuthType === "password") {
form.setValue("key", null);
form.setValue("keyPassword", "");
form.setValue("keyType", "auto");
form.setValue("credentialId", null);
} else if (newAuthType === "key") {
form.setValue("password", "");
form.setValue("credentialId", null);
} else if (newAuthType === "credential") {
form.setValue("password", "");
form.setValue("key", null);
form.setValue("keyPassword", "");
form.setValue("keyType", "auto");
}
}}
className="flex-1 flex flex-col h-full min-h-0"
>

View File

@@ -512,7 +512,8 @@ export const Terminal = forwardRef<any, SSHTerminalProps>(function SSHTerminal(
cursorStyle: "bar",
scrollback: 10000,
fontSize: 14,
fontFamily: '"Caskaydia Cove Nerd Font Mono", "SF Mono", Consolas, "Liberation Mono", monospace',
fontFamily:
'"Caskaydia Cove Nerd Font Mono", "SF Mono", Consolas, "Liberation Mono", monospace',
theme: { background: "#18181b", foreground: "#f7f7f7" },
allowTransparency: true,
convertEol: true,