From 51aa6e325830d2a99e7b70c52a15f6413efb47dc Mon Sep 17 00:00:00 2001 From: smartass Date: Thu, 11 Jun 2026 23:26:54 +0500 Subject: [PATCH] fix: harden store writes, strict schemas, null cfg --- src/config/sources.ts | 6 +++++ src/config/store.ts | 25 +++++++++++++---- src/config/types.ts | 46 +++++++++++++++++--------------- test/unit/config/sources.test.ts | 14 ++++++++++ test/unit/config/store.test.ts | 7 +++++ test/unit/config/types.test.ts | 13 +++++++++ test/unit/format.test.ts | 11 ++++++++ 7 files changed, 96 insertions(+), 26 deletions(-) diff --git a/src/config/sources.ts b/src/config/sources.ts index d778f4b..18555d7 100644 --- a/src/config/sources.ts +++ b/src/config/sources.ts @@ -36,6 +36,12 @@ export const readConfigFile = (path: string | undefined): ConnectionConfig[] => console.error('dbmole: config file ' + path + ' is not valid JSON, ignoring') return [] } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { + console.error( + 'dbmole: config file ' + path + ' must contain { "connections": [...] }, ignoring' + ) + return [] + } const connections = (parsed as { connections?: unknown }).connections if (!Array.isArray(connections)) { console.error( diff --git a/src/config/store.ts b/src/config/store.ts index 39ea9e3..ae5b828 100644 --- a/src/config/store.ts +++ b/src/config/store.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs' import { homedir } from 'node:os' import { dirname, join } from 'node:path' import { type ConnectionConfig, connectionConfigSchema } from './types.js' @@ -47,10 +47,25 @@ let writeSeq = 0 export const writeStore = (path: string, connections: ConnectionConfig[]): void => { mkdirSync(dirname(path), { recursive: true, mode: 0o700 }) - writeSeq += 1 - const tmp = path + '.' + process.pid + '.' + writeSeq + '.tmp' const items = connections.map((c) => JSON.stringify(c, null, 4)) const content = items.length === 0 ? '[]\n' : '[\n' + items.join(',\n') + '\n]\n' - writeFileSync(tmp, content, { mode: 0o600 }) - renameSync(tmp, path) + let tmp: string + for (;;) { + writeSeq += 1 + tmp = path + '.' + process.pid + '.' + writeSeq + '.tmp' + try { + writeFileSync(tmp, content, { mode: 0o600, flag: 'wx' }) + break + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') { + throw error + } + } + } + try { + renameSync(tmp, path) + } catch (error) { + rmSync(tmp, { force: true }) + throw error + } } diff --git a/src/config/types.ts b/src/config/types.ts index ae0552c..872a2c1 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -1,27 +1,31 @@ import * as z from 'zod' -export const sshConfigSchema = z.object({ - host: z.string().min(1), - port: z.number().int().positive().default(22), - user: z.string().min(1), - password: z.string().optional(), - privateKey: z.string().optional(), - privateKeyPath: z.string().optional(), - passphrase: z.string().optional(), - agent: z.boolean().optional() -}) +export const sshConfigSchema = z + .object({ + host: z.string().min(1), + port: z.number().int().positive().default(22), + user: z.string().min(1), + password: z.string().optional(), + privateKey: z.string().optional(), + privateKeyPath: z.string().optional(), + passphrase: z.string().optional(), + agent: z.boolean().optional() + }) + .strict() -export const connectionConfigSchema = z.object({ - name: z.string().regex(/^[a-zA-Z0-9_-]+$/, 'name must match [a-zA-Z0-9_-]+'), - type: z.enum(['postgres', 'mysql']), - host: z.string().min(1), - port: z.number().int().positive().optional(), - user: z.string().min(1), - password: z.string().optional(), - database: z.string().optional(), - readonly: z.boolean().default(false), - ssh: sshConfigSchema.optional() -}) +export const connectionConfigSchema = z + .object({ + name: z.string().regex(/^[a-zA-Z0-9_-]+$/, 'name must match [a-zA-Z0-9_-]+'), + type: z.enum(['postgres', 'mysql']), + host: z.string().min(1), + port: z.number().int().positive().optional(), + user: z.string().min(1), + password: z.string().optional(), + database: z.string().optional(), + readonly: z.boolean().default(false), + ssh: sshConfigSchema.optional() + }) + .strict() export type SshConfig = z.infer export type ConnectionConfig = z.infer diff --git a/test/unit/config/sources.test.ts b/test/unit/config/sources.test.ts index 8be6ba5..4cdc0fe 100644 --- a/test/unit/config/sources.test.ts +++ b/test/unit/config/sources.test.ts @@ -69,4 +69,18 @@ describe('readConfigFile', () => { expect(readConfigFile(path)).toEqual([]) expect(console.error).toHaveBeenCalled() }) + + it('warns on null config', () => { + const path = join(dir, 'config.json') + writeFileSync(path, 'null') + expect(readConfigFile(path)).toEqual([]) + expect(console.error).toHaveBeenCalled() + }) + + it('warns on primitive config', () => { + const path = join(dir, 'config.json') + writeFileSync(path, '"just a string"') + expect(readConfigFile(path)).toEqual([]) + expect(console.error).toHaveBeenCalled() + }) }) diff --git a/test/unit/config/store.test.ts b/test/unit/config/store.test.ts index 062babb..11cfafd 100644 --- a/test/unit/config/store.test.ts +++ b/test/unit/config/store.test.ts @@ -78,6 +78,13 @@ describe('store', () => { writeStore(path, [connection]) expect(readFileSync(path, 'utf8')).toContain('\n "name": "lab"') }) + + it('cleans up the tmp file when rename fails', () => { + mkdirSync(path, { recursive: true }) + expect(() => writeStore(path, [connection])).toThrow() + const leftovers = readdirSync(join(dir, 'nested')).filter((f) => f.includes('.tmp')) + expect(leftovers).toEqual([]) + }) }) describe('defaultStorePath', () => { diff --git a/test/unit/config/types.test.ts b/test/unit/config/types.test.ts index fd9f5b9..bd49b1a 100644 --- a/test/unit/config/types.test.ts +++ b/test/unit/config/types.test.ts @@ -40,6 +40,19 @@ describe('connectionConfigSchema', () => { it('rejects empty host', () => { expect(() => connectionConfigSchema.parse({ ...minimal, host: '' })).toThrow() }) + + it('rejects unknown top-level keys', () => { + expect(() => connectionConfigSchema.parse({ ...minimal, readOnly: true })).toThrow() + }) + + it('rejects unknown ssh keys', () => { + expect(() => + connectionConfigSchema.parse({ + ...minimal, + ssh: { host: 'b', user: 'u', password: 'p', prot: 22 } + }) + ).toThrow() + }) }) describe('defaultPort', () => { diff --git a/test/unit/format.test.ts b/test/unit/format.test.ts index 726f008..fc5c4aa 100644 --- a/test/unit/format.test.ts +++ b/test/unit/format.test.ts @@ -74,6 +74,17 @@ describe('formatDbError', () => { ) }) + it('joins detail and hint', () => { + const error = Object.assign(new Error('boom'), { + code: '23505', + detail: 'Key (id)=(1) already exists.', + hint: 'use upsert' + }) + expect(formatDbError('postgres', error)).toBe( + '[postgres 23505] boom (Key (id)=(1) already exists. | use upsert)' + ) + }) + it('handles non-Error values', () => { expect(formatDbError('mysql', 'boom')).toBe('[mysql] boom') })