fix: op:// secret references rejected as ssh keys on credentials (#1394)

src/backend/database/routes/credential-deploy-routes.ts

@@ -9,6 +9,9 @@ import { createCurrentHostResolutionRepository } from "../repositories/factory.j
 import { connectHost } from "../../hosts/connect/connect-host.js";
 import type { SshConnectHost } from "../../hosts/connect/types.js";
 import { resolveHostById } from "../../hosts/host-resolver.js";
+import { isSecretReference } from "../../hosts/external-secrets.js";
+import { resolveKeyReferences } from "../../hosts/stored-ssh-key.js";
+import { parseSSHKey } from "../../utils/ssh-key-utils.js";
 
 function describeDeployError(err: unknown): string {
   const message = getErrorMessage(err, "Connection failed");
@@ -363,7 +366,20 @@ export function registerCredentialDeployRoutes(
           });
         }
 
-        const publicKey = credData.publicKey;
+        let publicKey = credData.publicKey;
+        const storedKey = credData.privateKey || credData.key;
+        if (!publicKey && isSecretReference(storedKey)) {
+          // A key kept in a secret source has no stored public key; derive
+          // it from the resolved key.
+          const resolved = await resolveKeyReferences(
+            userId,
+            storedKey,
+            (credData as { keyPassword?: string | null }).keyPassword,
+          );
+          publicKey =
+            parseSSHKey(resolved.key, resolved.passphrase).publicKey ||
+            undefined;
+        }
         if (!publicKey) {
           return res.status(400).json({
             success: false,

src/backend/database/routes/credential-key-routes.ts

@@ -8,6 +8,8 @@ import {
   parseSSHKey,
   validateKeyPair,
 } from "../../utils/ssh-key-utils.js";
+import { resolveKeyReferences } from "../../hosts/stored-ssh-key.js";
+import type { AuthenticatedRequest } from "../../../types/index.js";
 
 const { utils: ssh2Utils } = ssh2Pkg;
 
@@ -99,13 +101,17 @@ export function registerCredentialKeyRoutes(
     requireCredentialViewPermission,
     requireDataAccess,
     async (req: Request, res: Response) => {
-      const { privateKey, keyPassword } = req.body;
-
-      if (!privateKey || typeof privateKey !== "string") {
+      if (!req.body?.privateKey || typeof req.body.privateKey !== "string") {
         return res.status(400).json({ error: "Private key is required" });
       }
 
       try {
+        const { key: privateKey, passphrase: keyPassword } =
+          await resolveKeyReferences(
+            (req as AuthenticatedRequest).userId,
+            req.body.privateKey,
+            req.body.keyPassword,
+          );
         const keyInfo = parseSSHKey(privateKey, keyPassword);
 
         const response = {
@@ -352,13 +358,17 @@ export function registerCredentialKeyRoutes(
     requireCredentialViewPermission,
     requireDataAccess,
     async (req: Request, res: Response) => {
-      const { privateKey, keyPassword } = req.body;
-
-      if (!privateKey || typeof privateKey !== "string") {
+      if (!req.body?.privateKey || typeof req.body.privateKey !== "string") {
         return res.status(400).json({ error: "Private key is required" });
       }
 
       try {
+        const { key: privateKey, passphrase: keyPassword } =
+          await resolveKeyReferences(
+            (req as AuthenticatedRequest).userId,
+            req.body.privateKey,
+            req.body.keyPassword,
+          );
         let privateKeyObj;
         const parseAttempts = [];
 

src/backend/database/routes/credentials.ts

@@ -7,7 +7,7 @@ import express, { type Request, type Response } from "express";
 import { authLogger } from "../../utils/logger.js";
 import { PermissionManager } from "../../utils/permission-manager.js";
 import { AuthManager } from "../../utils/auth-manager.js";
-import { parseSSHKey } from "../../utils/ssh-key-utils.js";
+import { parseKeyForStorage } from "../../hosts/stored-ssh-key.js";
 import { registerCredentialKeyRoutes } from "./credential-key-routes.js";
 import { registerCredentialDeployRoutes } from "./credential-deploy-routes.js";
 import { registerCredentialBulkRoutes } from "./credential-bulk-routes.js";
@@ -165,7 +165,7 @@ router.post(
 
       let keyInfo = null;
       if (authType === "key" && plainKey) {
-        keyInfo = parseSSHKey(plainKey, plainKeyPassword);
+        keyInfo = parseKeyForStorage(plainKey, plainKeyPassword);
         if (!keyInfo.success) {
           authLogger.warn("SSH key parsing failed", {
             operation: "credential_create",
@@ -192,7 +192,11 @@ router.post(
         password: plainPassword,
         key: plainKey,
         privateKey: keyInfo?.privateKey || plainKey,
-        publicKey: keyInfo?.publicKey || null,
+        publicKey:
+          keyInfo?.publicKey ||
+          (keyInfo?.reference && typeof req.body?.publicKey === "string"
+            ? req.body.publicKey.trim() || null
+            : null),
         keyPassword: plainKeyPassword,
         keyType: keyType || null,
         detectedKeyType: keyInfo?.keyType || null,
@@ -502,7 +506,7 @@ router.post(
 
       let keyInfo = null;
       if (authType === "key" && plainKey) {
-        keyInfo = parseSSHKey(plainKey, plainKeyPassword);
+        keyInfo = parseKeyForStorage(plainKey, plainKeyPassword);
         if (!keyInfo.success) {
           return res.status(400).json({
             error: keyInfo.error
@@ -524,7 +528,11 @@ router.post(
         password: authType === "password" ? plainPassword : null,
         key: authType === "key" ? plainKey : null,
         privateKey: authType === "key" ? keyInfo?.privateKey || plainKey : null,
-        publicKey: authType === "key" ? keyInfo?.publicKey || null : null,
+        publicKey:
+          authType === "key"
+            ? keyInfo?.publicKey ||
+              (keyInfo?.reference ? source.publicKey || null : null)
+            : null,
         keyPassword: authType === "key" ? plainKeyPassword : null,
         keyType: source.keyType,
         detectedKeyType: authType === "key" ? keyInfo?.keyType || null : null,
@@ -688,8 +696,14 @@ router.put(
       if (updateData.key !== undefined) {
         updateFields.key = updateData.key || null;
 
-        if (updateData.key && existingCredential.authType === "key") {
-          const keyInfo = parseSSHKey(updateData.key, updateData.keyPassword);
+        if (
+          updateData.key &&
+          (updateData.authType ?? existingCredential.authType) === "key"
+        ) {
+          const keyInfo = parseKeyForStorage(
+            updateData.key,
+            updateData.keyPassword,
+          );
           if (!keyInfo.success) {
             authLogger.warn("SSH key parsing failed during update", {
               operation: "credential_update",
@@ -704,8 +718,10 @@ router.put(
             });
           }
           updateFields.privateKey = keyInfo.privateKey;
-          updateFields.publicKey = keyInfo.publicKey;
-          updateFields.detectedKeyType = keyInfo.keyType;
+          updateFields.publicKey = keyInfo.reference
+            ? updateData.publicKey?.trim() || null
+            : keyInfo.publicKey;
+          updateFields.detectedKeyType = keyInfo.keyType || null;
         }
       }
       if (updateData.keyPassword !== undefined) {

src/backend/database/routes/host.ts

@@ -11,7 +11,7 @@ import { pluginEvents, TOPICS } from "../../plugins/events.js";
 import { AuthManager } from "../../utils/auth-manager.js";
 import { PermissionManager } from "../../utils/permission-manager.js";
 import { DataCrypto } from "../../utils/data-crypto.js";
-import { parseSSHKey } from "../../utils/ssh-key-utils.js";
+import { parseKeyForStorage } from "../../hosts/stored-ssh-key.js";
 import {
   pickResolvedPassword,
   pickResolvedUsername,
@@ -358,7 +358,7 @@ router.post(
       sshDataObj.keyType = null;
     } else if (effectiveAuthType === "key") {
       if (key && typeof key === "string") {
-        const keyValidation = parseSSHKey(
+        const keyValidation = parseKeyForStorage(
           key,
           typeof keyPassword === "string" ? keyPassword : undefined,
         );
@@ -940,7 +940,7 @@ router.put(
       sshDataObj.keyType = null;
     } else if (effectiveAuthType === "key") {
       if (key && typeof key === "string") {
-        const keyValidation = parseSSHKey(
+        const keyValidation = parseKeyForStorage(
           key,
           typeof keyPassword === "string" ? keyPassword : undefined,
         );

src/backend/hosts/stored-ssh-key.ts

@@ -0,0 +1,43 @@
+import { parseSSHKey, type KeyInfo } from "../utils/ssh-key-utils.js";
+import {
+  isSecretReference,
+  resolveSecretReference,
+} from "./external-secrets.js";
+
+/**
+ * Checks a private key before it is saved. A secret reference (op://...) is
+ * resolved when connecting, so it is stored as given: there is nothing to
+ * parse yet, and parsing the reference itself always failed (#1394).
+ */
+export function parseKeyForStorage(
+  key: string,
+  passphrase?: string | null,
+): KeyInfo & { reference: boolean } {
+  if (isSecretReference(key) || isSecretReference(passphrase)) {
+    return {
+      success: true,
+      privateKey: key.trim(),
+      publicKey: "",
+      keyType: "",
+      reference: true,
+    };
+  }
+  return {
+    ...parseSSHKey(key, passphrase ?? undefined),
+    reference: false,
+  };
+}
+
+/** The real key and passphrase behind any references, for key helpers. */
+export async function resolveKeyReferences(
+  userId: string,
+  key: string,
+  passphrase?: string | null,
+): Promise<{ key: string; passphrase?: string }> {
+  const resolve = async (value: string) =>
+    isSecretReference(value) ? resolveSecretReference(userId, value) : value;
+  return {
+    key: await resolve(key),
+    passphrase: passphrase ? await resolve(passphrase) : undefined,
+  };
+}

src/backend/plugins/user-data.ts

@@ -107,7 +107,7 @@ interface SqliteLike {
     run(...params: unknown[]): unknown;
     all(...params: unknown[]): unknown[];
   };
-  transaction?<T extends (...args: never[]) => unknown>(fn: T): T;
+  transaction?(fn: (rows: Row[]) => void): (rows: Row[]) => unknown;
 }
 
 const INSERT_CHUNK = 2000;
@@ -148,7 +148,7 @@ export async function writeUserPluginTables(
       : writeChunk;
     for (let start = 0; start < rows.length; start += INSERT_CHUNK) {
       const chunk = rows.slice(start, start + INSERT_CHUNK);
-      runChunk(chunk as never);
+      runChunk(chunk);
       written += chunk.length;
       await yieldToEventLoop();
     }

src/backend/tests/database/routes/credentials-secret-reference.test.ts

@@ -0,0 +1,193 @@
+/**
+ * A key credential whose private key is a secret reference (op://...) is
+ * saved as given and resolved when connecting (#1394). Parsing the reference
+ * as a key used to reject it, and an update kept the old private key.
+ */
+
+import http from "node:http";
+import type { AddressInfo } from "node:net";
+import express from "express";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+const h = vi.hoisted(() => ({
+  created: [] as Array<Record<string, unknown>>,
+  updated: [] as Array<Record<string, unknown>>,
+  existing: null as Record<string, unknown> | null,
+}));
+
+const pass = (_req: unknown, _res: unknown, next: () => void) => next();
+
+vi.mock("../../../utils/auth-manager.js", () => ({
+  AuthManager: {
+    getInstance: () => ({
+      createAuthMiddleware:
+        () => (req: { userId?: string }, _res: unknown, next: () => void) => {
+          req.userId = "u1";
+          next();
+        },
+      createDataAccessMiddleware: () => pass,
+    }),
+  },
+}));
+vi.mock("../../../utils/permission-manager.js", () => ({
+  PermissionManager: {
+    getInstance: () => ({ requirePermission: () => pass }),
+  },
+}));
+vi.mock("../../../sync/shared-copy-guard.js", () => ({
+  rejectSharedCopyWrites: () => pass,
+}));
+vi.mock("../../../utils/audit-logger.js", () => ({
+  logAudit: async () => {},
+  getAuditUsername: async () => "u1",
+  getRequestMeta: () => ({ ipAddress: null, userAgent: null }),
+}));
+vi.mock("../../../utils/logger.js", () => {
+  const log = {
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    success: vi.fn(),
+    debug: vi.fn(),
+  };
+  return { authLogger: log, sshLogger: log, databaseLogger: log };
+});
+vi.mock("../../../database/routes/credential-key-routes.js", () => ({
+  registerCredentialKeyRoutes: () => {},
+}));
+vi.mock("../../../database/routes/credential-deploy-routes.js", () => ({
+  registerCredentialDeployRoutes: () => {},
+}));
+vi.mock("../../../database/routes/credential-bulk-routes.js", () => ({
+  registerCredentialBulkRoutes: () => {},
+}));
+vi.mock("../../../hosts/delete-credential.js", () => ({
+  deleteOwnedCredential: async () => {},
+}));
+vi.mock("../../../hosts/connect/core-providers.js", () => ({
+  ensureCoreSshAuthProviders: () => {},
+}));
+vi.mock("../../../hosts/connect/auth-provider-registry.js", () => ({
+  listCredentialTypes: () => ["password", "key"],
+}));
+vi.mock("../../../utils/shared-host-secrets-manager.js", () => ({
+  SharedHostSecretsManager: {
+    getInstance: () => ({ resyncHostsForCredential: async () => {} }),
+  },
+}));
+vi.mock("../../../utils/shared-credential-secrets-manager.js", () => ({
+  SharedCredentialSecretsManager: {
+    getInstance: () => ({ resyncCredential: async () => {} }),
+  },
+}));
+vi.mock("../../../database/repositories/factory.js", () => ({
+  createCurrentCredentialRepository: () => ({
+    createEncryptedForUser: async (
+      userId: string,
+      data: Record<string, unknown>,
+    ) => {
+      h.created.push(data);
+      return { id: 1, userId, ...data };
+    },
+    findById: async () => (h.existing ? { id: 1, userId: "u1" } : null),
+    findDecryptedByIdForUser: async () => h.existing,
+    updateEncryptedForUser: async (
+      _userId: string,
+      _id: number,
+      fields: Record<string, unknown>,
+    ) => {
+      h.updated.push(fields);
+      return { ...h.existing, ...fields };
+    },
+  }),
+  createCurrentUserRepository: () => ({}),
+  createCurrentCredentialAccessRepository: () => ({}),
+  createCurrentRoleRepository: () => ({}),
+  createCurrentHostResolutionRepository: () => ({}),
+  createCurrentHostRepository: () => ({}),
+}));
+
+const { default: router } =
+  await import("../../../database/routes/credentials.js");
+
+// Never parsed here: only the update replacing it matters.
+const REAL_KEY = "stored-key-material";
+
+let server: http.Server;
+let base: string;
+
+beforeEach(async () => {
+  h.created = [];
+  h.updated = [];
+  h.existing = null;
+  const app = express();
+  app.use(express.json());
+  app.use("/credentials", router);
+  server = http.createServer(app);
+  await new Promise<void>((resolve) => server.listen(0, resolve));
+  base = `http://127.0.0.1:${(server.address() as AddressInfo).port}`;
+});
+
+afterEach(async () => {
+  await new Promise<void>((resolve) => server.close(() => resolve()));
+});
+
+function send(method: string, path: string, body: unknown) {
+  return fetch(`${base}/credentials${path}`, {
+    method,
+    headers: { "content-type": "application/json" },
+    body: JSON.stringify(body),
+  });
+}
+
+describe("credentials with a secret reference key", () => {
+  it("saves an op:// private key as given", async () => {
+    const response = await send("POST", "", {
+      name: "vault key",
+      authType: "key",
+      username: "root",
+      key: "op://vault/item/private_key",
+    });
+
+    expect(response.status).toBe(201);
+    expect(h.created[0]).toMatchObject({
+      key: "op://vault/item/private_key",
+      privateKey: "op://vault/item/private_key",
+      publicKey: null,
+      detectedKeyType: null,
+    });
+  });
+
+  it("still rejects a key that is neither a key nor a reference", async () => {
+    const response = await send("POST", "", {
+      name: "bad",
+      authType: "key",
+      key: "not a key",
+    });
+    expect(response.status).toBe(400);
+  });
+
+  it("replaces the stored private key when updating to a reference", async () => {
+    h.existing = {
+      id: 1,
+      userId: "u1",
+      name: "k",
+      authType: "key",
+      key: REAL_KEY,
+      privateKey: REAL_KEY,
+      publicKey: "ssh-ed25519 AAAA old",
+    };
+
+    const response = await send("PUT", "/1", {
+      key: "op://vault/item/private_key",
+    });
+
+    expect(response.status).toBe(200);
+    expect(h.updated[0]).toMatchObject({
+      key: "op://vault/item/private_key",
+      privateKey: "op://vault/item/private_key",
+      publicKey: null,
+      detectedKeyType: null,
+    });
+  });
+});

src/backend/tests/hosts/stored-ssh-key.test.ts

@@ -0,0 +1,43 @@
+import { describe, expect, it, vi } from "vitest";
+
+const resolve = vi.fn(async (_userId: string, ref: string) => `real:${ref}`);
+vi.mock("../../hosts/external-secrets.js", async (importOriginal) => ({
+  ...(await importOriginal<object>()),
+  resolveSecretReference: (userId: string, ref: string) => resolve(userId, ref),
+}));
+
+const { parseKeyForStorage, resolveKeyReferences } =
+  await import("../../hosts/stored-ssh-key.js");
+
+describe("parseKeyForStorage", () => {
+  it("accepts a secret reference without parsing it", () => {
+    expect(parseKeyForStorage(" op://v/i/private_key ")).toEqual({
+      success: true,
+      privateKey: "op://v/i/private_key",
+      publicKey: "",
+      keyType: "",
+      reference: true,
+    });
+  });
+
+  it("does not try to open a real key with a referenced passphrase", () => {
+    const result = parseKeyForStorage("-----BEGIN...", "op://v/i/pass");
+    expect(result).toMatchObject({ success: true, reference: true });
+  });
+
+  it("parses anything else as a key", () => {
+    const result = parseKeyForStorage("not a key");
+    expect(result.success).toBe(false);
+    expect(result.reference).toBe(false);
+  });
+});
+
+describe("resolveKeyReferences", () => {
+  it("resolves only the values that are references", async () => {
+    expect(await resolveKeyReferences("u1", "op://v/i/key", "plain")).toEqual({
+      key: "real:op://v/i/key",
+      passphrase: "plain",
+    });
+    expect(resolve).toHaveBeenCalledTimes(1);
+  });
+});