From 3941dd4ecc80f3a54bd018c10140d69c1f207098 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Wed, 14 Oct 2026 08:13:04 +0800 Subject: [PATCH] fix: keep foreign keys on during user data import (#1626) Import credentials before hosts and remap the file's credential ids to the ones created here, so hosts and protocol logins no longer point at another server's ids. With rows written in dependency order, the import no longer turns off SQLite foreign keys on the shared connection. The uploaded file is now removed on every exit path. --- src/backend/database/database.ts | 435 +++++++++--------- .../repositories/credential-repository.ts | 6 +- .../repositories/sqlite-foreign-keys.ts | 40 -- .../tests/database/import-route.test.ts | 147 ++++++ .../host-credential-repositories.test.ts | 10 +- .../repositories/sqlite-foreign-keys.test.ts | 51 -- 6 files changed, 368 insertions(+), 321 deletions(-) delete mode 100644 src/backend/database/repositories/sqlite-foreign-keys.ts create mode 100644 src/backend/tests/database/import-route.test.ts delete mode 100644 src/backend/tests/database/repositories/sqlite-foreign-keys.test.ts diff --git a/src/backend/database/database.ts b/src/backend/database/database.ts index 3f7530abd..0d09b5897 100644 --- a/src/backend/database/database.ts +++ b/src/backend/database/database.ts @@ -79,7 +79,6 @@ import { createCurrentSshCredentialUsageRepository, createCurrentUserRepository, } from "./repositories/factory.js"; -import { withCurrentSqliteForeignKeysDisabled } from "./repositories/sqlite-foreign-keys.js"; import { applyPluginHostImportSettings } from "./routes/host-plugin-settings.js"; import { selectExportableSettings, @@ -1235,227 +1234,218 @@ app.post( }; try { - await withCurrentSqliteForeignKeysDisabled(async () => { - try { - const importedHosts = importDb - .prepare("SELECT * FROM ssh_data") - .all(); - const importedHostIds: number[] = []; - for (const host of importedHosts) { - try { - const hostRepository = createCurrentHostRepository(); - const exists = await hostRepository.existsForImportIdentity( - userId, - host.ip, - host.port, - host.username, - ); - - if (exists) { - result.summary.skippedItems++; - continue; - } - - const hostData = { - userId: userId, - name: host.name, - ip: host.ip, - port: host.port, - username: host.username, - folder: host.folder, - tags: host.tags, - pin: Boolean(host.pin), - authType: host.auth_type, - forceKeyboardInteractive: host.force_keyboard_interactive, - password: host.password, - key: host.key, - keyPassword: host.key_password, - keyType: host.key_type, - sudoPassword: host.sudo_password, - credentialId: host.credential_id || null, - overrideCredentialUsername: Boolean( - host.override_credential_username, - ), - jumpHosts: host.jump_hosts, - ...legacyStatusCheck(host), - // Exports from before 2.9.0 carry these in terminal_config. - sshOptions: - host.ssh_options ?? - sshOptionsForWrite({ - terminalConfig: host.terminal_config, - }) ?? - null, - notes: host.notes, - useSocks5: Boolean(host.use_socks5), - socks5Host: host.socks5_host, - socks5Port: host.socks5_port, - socks5Username: host.socks5_username, - socks5Password: host.socks5_password, - socks5ProxyChain: host.socks5_proxy_chain, - createdAt: host.created_at || new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - (hostData as Record).defaultOverrides = - JSON.stringify( - await applyHostDefaultsToWrite({ - ownerId: userId, - hostId: null, - columns: hostData as Record, - body: hostData as Record, - }), - ); - const created = await hostRepository.createEncryptedForUser( - userId, - hostData, - ); - await applyPluginHostImportSettings( - Number(created.id), - importedHostPluginSettings(importDb, host), - ); - importedHostIds.push(Number(created.id)); - await importHostProtocolLogins( - importDb, - host, - Number(created.id), - userId, - ); - result.summary.sshHostsImported++; - } catch (hostError) { - result.summary.errors.push( - `SSH host import error: ${hostError.message}`, - ); - } - } - await applyDefaultsAfterHostWrites(importedHostIds); - } catch { - apiLogger.info("ssh_data table not found in import file, skipping"); - } - - try { - const importedCreds = importDb - .prepare("SELECT * FROM ssh_credentials") - .all(); - for (const cred of importedCreds) { - try { - const credentialRepository = - createCurrentCredentialRepository(); - const exists = - await credentialRepository.existsForImportIdentity( - userId, - cred.name, - cred.username, - ); - - if (exists) { - result.summary.skippedItems++; - continue; - } - - const credData = { - userId: userId, - name: cred.name, - description: cred.description, - folder: cred.folder, - tags: cred.tags, - authType: cred.auth_type, - username: cred.username, - password: cred.password, - key: cred.key, - privateKey: cred.private_key, - publicKey: cred.public_key, - keyPassword: cred.key_password, - keyType: cred.key_type, - detectedKeyType: cred.detected_key_type, - usageCount: cred.usage_count || 0, - lastUsed: cred.last_used, - createdAt: cred.created_at || new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - - await credentialRepository.createEncryptedForUser( - userId, - credData, - ); - result.summary.sshCredentialsImported++; - } catch (credError) { - result.summary.errors.push( - `SSH credential import error: ${credError.message}`, - ); - } - } - } catch { - apiLogger.info( - "ssh_credentials table not found in import file, skipping", - ); - } - - const pluginRows = await importUserPluginRows(importDb, userId); - result.summary.pluginItemsImported += pluginRows.imported; - result.summary.skippedItems += pluginRows.skipped; - result.summary.errors.push(...pluginRows.errors); - - const hostDefaults = await importHostDefaults(importDb, userId); - result.summary.skippedItems += hostDefaults.skipped; - if (hostDefaults.imported > 0) { - await recompute({ userIds: [userId] }); - } - - const targetUser = await userRepository.findById(userId); - if (targetUser?.isAdmin) { + // The file's credential ids belong to the server that wrote it. + const credentialIds = new Map(); + try { + const importedCreds = importDb + .prepare("SELECT * FROM ssh_credentials") + .all(); + for (const cred of importedCreds) { try { - const importedSettings = readImportedSettings(importDb); - for (const setting of importedSettings) { - try { - await upsertImportedSetting(setting); - result.summary.settingsImported++; - } catch (settingError) { - result.summary.errors.push( - `Setting import error (${setting.key}): ${settingError.message}`, - ); - } + const credentialRepository = createCurrentCredentialRepository(); + const existingId = + await credentialRepository.findIdForImportIdentity( + userId, + cred.name, + cred.username, + ); + + if (existingId !== null) { + credentialIds.set(cred.id, existingId); + result.summary.skippedItems++; + continue; } - } catch { - apiLogger.info( - "settings table not found in import file, skipping", + + const credData = { + userId: userId, + name: cred.name, + description: cred.description, + folder: cred.folder, + tags: cred.tags, + authType: cred.auth_type, + username: cred.username, + password: cred.password, + key: cred.key, + privateKey: cred.private_key, + publicKey: cred.public_key, + keyPassword: cred.key_password, + keyType: cred.key_type, + detectedKeyType: cred.detected_key_type, + usageCount: cred.usage_count || 0, + lastUsed: cred.last_used, + createdAt: cred.created_at || new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + + const created = await credentialRepository.createEncryptedForUser( + userId, + credData, + ); + credentialIds.set(cred.id, Number(created.id)); + result.summary.sshCredentialsImported++; + } catch (credError) { + result.summary.errors.push( + `SSH credential import error: ${credError.message}`, ); } - } else { - apiLogger.info( - "Settings import skipped - only admin users can import settings", - ); } + } catch { + apiLogger.info( + "ssh_credentials table not found in import file, skipping", + ); + } - result.success = true; - - try { - await DatabaseSaveTrigger.forceSave("database_import"); - } catch (saveError) { - apiLogger.error( - "Failed to persist imported data to disk", - saveError, - { - operation: "import_force_save_failed", + try { + const importedHosts = importDb + .prepare("SELECT * FROM ssh_data") + .all(); + const importedHostIds: number[] = []; + for (const host of importedHosts) { + try { + const hostRepository = createCurrentHostRepository(); + const exists = await hostRepository.existsForImportIdentity( userId, - }, - ); + host.ip, + host.port, + host.username, + ); + + if (exists) { + result.summary.skippedItems++; + continue; + } + + const hostData = { + userId: userId, + name: host.name, + ip: host.ip, + port: host.port, + username: host.username, + folder: host.folder, + tags: host.tags, + pin: Boolean(host.pin), + authType: host.auth_type, + forceKeyboardInteractive: host.force_keyboard_interactive, + password: host.password, + key: host.key, + keyPassword: host.key_password, + keyType: host.key_type, + sudoPassword: host.sudo_password, + credentialId: credentialIds.get(host.credential_id) ?? null, + overrideCredentialUsername: Boolean( + host.override_credential_username, + ), + jumpHosts: host.jump_hosts, + ...legacyStatusCheck(host), + // Exports from before 2.9.0 carry these in terminal_config. + sshOptions: + host.ssh_options ?? + sshOptionsForWrite({ + terminalConfig: host.terminal_config, + }) ?? + null, + notes: host.notes, + useSocks5: Boolean(host.use_socks5), + socks5Host: host.socks5_host, + socks5Port: host.socks5_port, + socks5Username: host.socks5_username, + socks5Password: host.socks5_password, + socks5ProxyChain: host.socks5_proxy_chain, + createdAt: host.created_at || new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + + (hostData as Record).defaultOverrides = + JSON.stringify( + await applyHostDefaultsToWrite({ + ownerId: userId, + hostId: null, + columns: hostData as Record, + body: hostData as Record, + }), + ); + const created = await hostRepository.createEncryptedForUser( + userId, + hostData, + ); + await applyPluginHostImportSettings( + Number(created.id), + importedHostPluginSettings(importDb, host), + ); + importedHostIds.push(Number(created.id)); + await importHostProtocolLogins( + importDb, + host, + Number(created.id), + userId, + credentialIds, + ); + result.summary.sshHostsImported++; + } catch (hostError) { + result.summary.errors.push( + `SSH host import error: ${hostError.message}`, + ); + } } - }); + await applyDefaultsAfterHostWrites(importedHostIds); + } catch { + apiLogger.info("ssh_data table not found in import file, skipping"); + } + + const pluginRows = await importUserPluginRows(importDb, userId); + result.summary.pluginItemsImported += pluginRows.imported; + result.summary.skippedItems += pluginRows.skipped; + result.summary.errors.push(...pluginRows.errors); + + const hostDefaults = await importHostDefaults(importDb, userId); + result.summary.skippedItems += hostDefaults.skipped; + if (hostDefaults.imported > 0) { + await recompute({ userIds: [userId] }); + } + + const targetUser = await userRepository.findById(userId); + if (targetUser?.isAdmin) { + try { + const importedSettings = readImportedSettings(importDb); + for (const setting of importedSettings) { + try { + await upsertImportedSetting(setting); + result.summary.settingsImported++; + } catch (settingError) { + result.summary.errors.push( + `Setting import error (${setting.key}): ${settingError.message}`, + ); + } + } + } catch { + apiLogger.info("settings table not found in import file, skipping"); + } + } else { + apiLogger.info( + "Settings import skipped - only admin users can import settings", + ); + } + + result.success = true; + + try { + await DatabaseSaveTrigger.forceSave("database_import"); + } catch (saveError) { + apiLogger.error( + "Failed to persist imported data to disk", + saveError, + { + operation: "import_force_save_failed", + userId, + }, + ); + } } finally { if (importDb) { importDb.close(); } } - try { - fs.unlinkSync(req.file.path); - } catch { - apiLogger.warn("Failed to clean up uploaded file", { - operation: "file_cleanup_warning", - filePath: req.file.path, - }); - } - res.json({ success: result.success, message: result.success @@ -1472,17 +1462,6 @@ app.post( }); } } catch (error) { - if (req.file?.path && fs.existsSync(req.file.path)) { - try { - fs.unlinkSync(req.file.path); - } catch { - apiLogger.warn("Failed to clean up uploaded file after error", { - operation: "file_cleanup_error", - filePath: req.file.path, - }); - } - } - apiLogger.error("SQLite import failed", error, { operation: "sqlite_import_api_failed", userId: (req as AuthenticatedRequest).userId, @@ -1491,6 +1470,17 @@ app.post( error: "Failed to import SQLite data", details: getErrorMessage(error), }); + } finally { + if (req.file) { + try { + fs.rmSync(req.file.path, { force: true }); + } catch { + apiLogger.warn("Failed to clean up uploaded file", { + operation: "file_cleanup_warning", + filePath: req.file.path, + }); + } + } } }), ); @@ -2012,14 +2002,15 @@ function legacyStatusCheck(row: Record): { /** * A host's plugin protocol logins from an export. The file's credential ids - * belong to the server that wrote it, so a login keeps one only when it is - * a credential the importing user can use. + * belong to the server that wrote it, so a login keeps one only when it maps + * to a credential imported alongside it. */ async function importHostProtocolLogins( importDb: Database.Database, host: Record, hostId: number, userId: string, + credentialIds: Map, ): Promise { let rows: Array>; try { @@ -2040,7 +2031,7 @@ async function importHostProtocolLogins( } protocolAuth[String(row.protocol)] = { authType: row.auth_type, - credentialId: row.credential_id, + credentialId: credentialIds.get(row.credential_id as number) ?? null, username: row.username, password: row.password, fields, diff --git a/src/backend/database/repositories/credential-repository.ts b/src/backend/database/repositories/credential-repository.ts index db3fbd610..d5b76f353 100644 --- a/src/backend/database/repositories/credential-repository.ts +++ b/src/backend/database/repositories/credential-repository.ts @@ -103,11 +103,11 @@ export class CredentialRepository { .orderBy(desc(sshCredentials.updatedAt)); } - async existsForImportIdentity( + async findIdForImportIdentity( userId: string, name: string, username: string | null, - ): Promise { + ): Promise { const rows = await this.context.drizzle .select({ id: sshCredentials.id }) .from(sshCredentials) @@ -120,7 +120,7 @@ export class CredentialRepository { ) .limit(1); - return rows.length > 0; + return rows[0]?.id ?? null; } async findDecryptedByIdForUser( diff --git a/src/backend/database/repositories/sqlite-foreign-keys.ts b/src/backend/database/repositories/sqlite-foreign-keys.ts deleted file mode 100644 index cf4f4e5d5..000000000 --- a/src/backend/database/repositories/sqlite-foreign-keys.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { getCurrentRepositorySqlite } from "./factory.js"; -import { needsExplicitPersist, resolveDatabaseDialect } from "../db/dialect.js"; - -export interface SqliteForeignKeyClient { - exec(sql: string): unknown; -} - -export async function withSqliteForeignKeysDisabled( - sqlite: SqliteForeignKeyClient, - operation: () => Promise, -): Promise { - sqlite.exec("PRAGMA foreign_keys = OFF"); - try { - return await operation(); - } finally { - sqlite.exec("PRAGMA foreign_keys = ON"); - } -} - -/** - * Runs a bulk import with foreign keys relaxed. - * - * Backup restore writes tables in an order that is not dependency-safe, so the - * constraints have to stand down for the duration. - * - * Postgres and MySQL keep their constraints enabled. The portable importer - * writes through repositories and handles individual row failures, so it must - * still be allowed to run there; only SQLite needs this connection-local - * relaxation for legacy backups whose rows are not dependency ordered. - */ -export async function withCurrentSqliteForeignKeysDisabled( - operation: () => Promise, -): Promise { - const dialect = resolveDatabaseDialect(); - if (!needsExplicitPersist(dialect)) { - return operation(); - } - - return withSqliteForeignKeysDisabled(getCurrentRepositorySqlite(), operation); -} diff --git a/src/backend/tests/database/import-route.test.ts b/src/backend/tests/database/import-route.test.ts new file mode 100644 index 000000000..085a7e03d --- /dev/null +++ b/src/backend/tests/database/import-route.test.ts @@ -0,0 +1,147 @@ +import * as fs from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { runInNewContext } from "node:vm"; +import Database from "better-sqlite3"; +import ts from "typescript"; +import { expect, it, vi } from "vitest"; + +const source = readFileSync( + new URL("../../database/database.ts", import.meta.url), + "utf8", +); +const ast = ts.createSourceFile( + "database.ts", + source, + ts.ScriptTarget.Latest, + true, +); +const route = ast.statements.find( + (s) => + ts.isExpressionStatement(s) && + ts.isCallExpression(s.expression) && + s.expression.arguments[0]?.getText(ast) === '"/database/import"', +) as ts.ExpressionStatement; +const batched = (route.expression as ts.CallExpression) + .arguments[3] as ts.CallExpression; +const callback = batched.arguments[0]; + +const dir = mkdtempSync(join(tmpdir(), "termix-import-route-")); + +function exportFile(): string { + const path = join(dir, `export-${Math.random()}.db`); + const db = new Database(path); + db.exec(` + CREATE TABLE ssh_credentials (id INTEGER, name TEXT, username TEXT); + CREATE TABLE ssh_data (id INTEGER, ip TEXT, port INTEGER, username TEXT, credential_id INTEGER); + INSERT INTO ssh_credentials VALUES (7, 'fresh', 'root'), (8, 'kept', 'admin'); + INSERT INTO ssh_data VALUES + (1, '10.0.0.1', 22, 'root', 7), + (2, '10.0.0.2', 22, 'root', 8), + (3, '10.0.0.3', 22, 'root', 99), + (4, '10.0.0.4', 22, 'root', NULL); + `); + db.close(); + return path; +} + +async function invoke(path: string, user: unknown = { isExternal: false }) { + const writes: string[] = []; + const hosts: Array> = []; + const importHostProtocolLogins = vi.fn(); + const handler = runInNewContext( + ts.transpileModule(`(${callback.getText(ast)})`, { + compilerOptions: { target: ts.ScriptTarget.ES2022 }, + }).outputText, + { + fs, + Buffer, + Database, + parseUserAgent: () => ({ type: "web" }), + createCurrentUserRepository: () => ({ findById: async () => user }), + DataCrypto: { getUserDataKey: () => Buffer.from("key") }, + apiLogger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + }, + createCurrentCredentialRepository: () => ({ + findIdForImportIdentity: async (_: string, name: string) => + name === "kept" ? 42 : null, + createEncryptedForUser: async () => { + writes.push("credential"); + return { id: 100 }; + }, + }), + createCurrentHostRepository: () => ({ + existsForImportIdentity: async () => false, + createEncryptedForUser: async (_: string, host: never) => { + writes.push("host"); + hosts.push(host); + return { id: 500 + hosts.length }; + }, + }), + legacyStatusCheck: () => ({}), + sshOptionsForWrite: () => null, + applyHostDefaultsToWrite: async () => ({}), + applyPluginHostImportSettings: async () => {}, + importedHostPluginSettings: () => ({}), + importHostProtocolLogins, + applyDefaultsAfterHostWrites: async () => {}, + importUserPluginRows: async () => ({ + imported: 0, + skipped: 0, + errors: [], + }), + importHostDefaults: async () => ({ imported: 0, skipped: 0 }), + DatabaseSaveTrigger: { forceSave: async () => {} }, + getErrorMessage: (error: Error) => error.message, + }, + ); + const res = { json: vi.fn(), status: vi.fn() }; + res.status.mockReturnValue(res); + await handler( + { userId: "user-1", file: { path, originalname: "export.db" } }, + res, + ); + return { res, writes, hosts, importHostProtocolLogins }; +} + +it("points imported hosts at the credentials imported with them", async () => { + const { res, writes, hosts, importHostProtocolLogins } = + await invoke(exportFile()); + + expect(res.status).not.toHaveBeenCalled(); + expect(writes).toEqual(["credential", "host", "host", "host", "host"]); + expect(hosts.map((host) => host.credentialId)).toEqual([100, 42, null, null]); + expect([...importHostProtocolLogins.mock.calls[0][4]]).toEqual([ + [7, 100], + [8, 42], + ]); +}); + +it("removes the uploaded file when the user is gone", async () => { + const path = exportFile(); + const { res } = await invoke(path, null); + + expect(res.status).toHaveBeenCalledWith(404); + expect(existsSync(path)).toBe(false); +}); + +it("removes the uploaded file when it is not a SQLite database", async () => { + const path = join(dir, "not-sqlite.db"); + writeFileSync(path, "definitely not a database"); + const { res } = await invoke(path); + + expect(res.status).toHaveBeenCalledWith(400); + expect(existsSync(path)).toBe(false); +}); + +it("removes the uploaded file after a successful import", async () => { + const path = exportFile(); + await invoke(path); + + expect(existsSync(path)).toBe(false); +}); diff --git a/src/backend/tests/database/repositories/host-credential-repositories.test.ts b/src/backend/tests/database/repositories/host-credential-repositories.test.ts index c4f99b54b..7efc0ada5 100644 --- a/src/backend/tests/database/repositories/host-credential-repositories.test.ts +++ b/src/backend/tests/database/repositories/host-credential-repositories.test.ts @@ -213,7 +213,7 @@ describe("HostRepository and CredentialRepository", () => { it("checks credential import identity", async () => { const repo = await createRepositories(); - await repo.credentials.create({ + const created = await repo.credentials.create({ userId: "user-1", name: "primary", authType: "password", @@ -221,11 +221,11 @@ describe("HostRepository and CredentialRepository", () => { }); await expect( - repo.credentials.existsForImportIdentity("user-1", "primary", "root"), - ).resolves.toBe(true); + repo.credentials.findIdForImportIdentity("user-1", "primary", "root"), + ).resolves.toBe(created.id); await expect( - repo.credentials.existsForImportIdentity("user-1", "primary", "admin"), - ).resolves.toBe(false); + repo.credentials.findIdForImportIdentity("user-1", "primary", "admin"), + ).resolves.toBeNull(); }); it("renames credential folders through the write boundary", async () => { diff --git a/src/backend/tests/database/repositories/sqlite-foreign-keys.test.ts b/src/backend/tests/database/repositories/sqlite-foreign-keys.test.ts deleted file mode 100644 index 9e674ea33..000000000 --- a/src/backend/tests/database/repositories/sqlite-foreign-keys.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; - -const factory = vi.hoisted(() => ({ - getCurrentRepositorySqlite: vi.fn(), -})); - -vi.mock("../../../database/repositories/factory.js", () => factory); - -import { - withCurrentSqliteForeignKeysDisabled, - withSqliteForeignKeysDisabled, -} from "../../../database/repositories/sqlite-foreign-keys.js"; - -const previousDatabaseDialect = process.env.DATABASE_DIALECT; - -afterEach(() => { - if (previousDatabaseDialect === undefined) - delete process.env.DATABASE_DIALECT; - else process.env.DATABASE_DIALECT = previousDatabaseDialect; - vi.clearAllMocks(); -}); - -describe("withSqliteForeignKeysDisabled", () => { - it("restores foreign keys after an import", async () => { - const sqlite = { exec: vi.fn() }; - - await expect( - withSqliteForeignKeysDisabled(sqlite, async () => "imported"), - ).resolves.toBe("imported"); - expect(sqlite.exec.mock.calls).toEqual([ - ["PRAGMA foreign_keys = OFF"], - ["PRAGMA foreign_keys = ON"], - ]); - }); -}); - -describe("withCurrentSqliteForeignKeysDisabled", () => { - it.each(["postgres", "mysql"])( - "runs portable imports with constraints enabled on %s", - async (dialect) => { - process.env.DATABASE_DIALECT = dialect; - const operation = vi.fn().mockResolvedValue("imported"); - - await expect( - withCurrentSqliteForeignKeysDisabled(operation), - ).resolves.toBe("imported"); - expect(operation).toHaveBeenCalledOnce(); - expect(factory.getCurrentRepositorySqlite).not.toHaveBeenCalled(); - }, - ); -});