diff --git a/.changes/database-redact-sql-traces.md b/.changes/database-redact-sql-traces.md new file mode 100644 index 000000000..89d7c484c --- /dev/null +++ b/.changes/database-redact-sql-traces.md @@ -0,0 +1,6 @@ +--- +type: fix +area: database +--- + +SQLite diagnostics now record only statement types, preventing playlist credentials and other private values from appearing in trace logs. diff --git a/apps/electron-backend/src/app/services/debug-trace.spec.ts b/apps/electron-backend/src/app/services/debug-trace.spec.ts index 91da7daed..c9346f2fd 100644 --- a/apps/electron-backend/src/app/services/debug-trace.spec.ts +++ b/apps/electron-backend/src/app/services/debug-trace.spec.ts @@ -1,4 +1,4 @@ -import { trace } from './debug-trace'; +import { trace, traceSqlStatement } from './debug-trace'; describe('debug trace redaction', () => { afterEach(() => { @@ -44,4 +44,25 @@ describe('debug trace redaction', () => { expect(output).toContain('[Redacted]'); expect(output).toContain('get_profile'); }); + + it('records only the statement type for expanded worker SQL', () => { + jest.spyOn(console, 'log').mockImplementation(() => undefined); + const secrets = [ + 'worker-user-secret', + 'worker-password-secret', + 'https://worker-user:worker-password@example.com/live?token=worker-token-secret', + ]; + + traceSqlStatement( + 'sql-worker', + `INSERT INTO playlists (username, password, url) VALUES ('${secrets[0]}', '${secrets[1]}', '${secrets[2]}')` + ); + + const output = (console.log as jest.Mock).mock.calls.flat().join('\n'); + expect(output).toContain('"statementType":"INSERT"'); + expect(output).not.toContain('INSERT INTO'); + for (const secret of secrets) { + expect(output).not.toContain(secret); + } + }); }); diff --git a/apps/electron-backend/src/app/services/debug-trace.ts b/apps/electron-backend/src/app/services/debug-trace.ts index 6b926a6c3..87c08da74 100644 --- a/apps/electron-backend/src/app/services/debug-trace.ts +++ b/apps/electron-backend/src/app/services/debug-trace.ts @@ -1,4 +1,7 @@ -import { redactSensitiveData } from '@iptvnator/shared/logging'; +import { + redactSensitiveData, + summarizeSqlStatementForTrace, +} from '@iptvnator/shared/logging'; const TRACE_ENV_TRUE_VALUES = new Set(['1', 'true', 'yes', 'on']); const TRACE_PREFIX = '[IPTVnator Trace]'; @@ -84,10 +87,6 @@ export function roundTraceDuration(durationMs: number): number { return Math.round(durationMs * 10) / 10; } -export function compactSqlForTrace(sql: string): string { - return truncateString(sql.replace(/\s+/g, ' ').trim()); -} - export function summarizeForTrace(value: unknown, depth = 0): unknown { if ( value == null || @@ -176,3 +175,7 @@ export function trace(scope: string, message: string, payload?: unknown): void { )}` ); } + +export function traceSqlStatement(scope: string, sql: unknown): void { + trace(scope, 'query', summarizeSqlStatementForTrace(sql)); +} diff --git a/apps/electron-backend/src/app/workers/database.worker-connection.ts b/apps/electron-backend/src/app/workers/database.worker-connection.ts index 9bc8b1bb7..122213d7a 100644 --- a/apps/electron-backend/src/app/workers/database.worker-connection.ts +++ b/apps/electron-backend/src/app/workers/database.worker-connection.ts @@ -10,9 +10,9 @@ import { registerNativeModuleSearchPaths, } from './worker-runtime-paths'; import { - compactSqlForTrace, isSqlTraceEnabled, trace, + traceSqlStatement, } from '../services/debug-trace'; let drizzleFactory: @@ -67,11 +67,7 @@ export async function getWorkerDatabase(): Promise { const filePath = getIptvnatorDatabasePath(); sqlite = new Database(filePath, { verbose: isSqlTraceEnabled() - ? (sql: string) => { - trace('sql-worker', 'query', { - sql: compactSqlForTrace(sql), - }); - } + ? (sql: string) => traceSqlStatement('sql-worker', sql) : undefined, }); sqlite.pragma('foreign_keys = ON'); diff --git a/libs/shared/database/src/lib/connection.spec.ts b/libs/shared/database/src/lib/connection.spec.ts index 09d00ee75..031faf932 100644 --- a/libs/shared/database/src/lib/connection.spec.ts +++ b/libs/shared/database/src/lib/connection.spec.ts @@ -18,6 +18,10 @@ function createdObjectNames(prefix: string, statements: readonly string[]) { } describe('database schema statements', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + const { createTableStatements, columnMigrationStatements, @@ -37,6 +41,26 @@ describe('database schema statements', () => { } ).ensureDownloadsPauseResumeSchema; + it('records only the statement type for expanded main-process SQL', () => { + jest.spyOn(console, 'log').mockImplementation(() => undefined); + const secrets = [ + 'main-user-secret', + 'main-password-secret', + 'https://main-user:main-password@example.com/live?token=main-token-secret', + ]; + + __databaseConnectionTestHooks.traceSqlStatement( + `UPDATE playlists SET username = '${secrets[0]}', password = '${secrets[1]}', url = '${secrets[2]}'` + ); + + const output = (console.log as jest.Mock).mock.calls.flat().join('\n'); + expect(output).toContain('"statementType":"UPDATE"'); + expect(output).not.toContain('UPDATE playlists'); + for (const secret of secrets) { + expect(output).not.toContain(secret); + } + }); + function createRebuildSqlite(legacyTableSql: string | undefined) { const statements: string[] = []; const transaction = jest.fn((callback: () => void) => callback); diff --git a/libs/shared/database/src/lib/connection.ts b/libs/shared/database/src/lib/connection.ts index c4c664b0e..0721cf11f 100644 --- a/libs/shared/database/src/lib/connection.ts +++ b/libs/shared/database/src/lib/connection.ts @@ -12,6 +12,10 @@ import Database from 'better-sqlite3'; import type { BetterSQLite3Database } from 'drizzle-orm/better-sqlite3'; import { drizzle } from 'drizzle-orm/better-sqlite3'; +import { + redactSensitiveData, + summarizeSqlStatementForTrace, +} from '@iptvnator/shared/logging'; import * as schema from './schema'; import { getIptvnatorDatabasePath } from './path-utils'; @@ -49,13 +53,6 @@ function isSqlTraceEnabled(): boolean { ); } -function compactSqlForTrace(sql: string): string { - const compactSql = sql.replace(/\s+/g, ' ').trim(); - return compactSql.length <= 180 - ? compactSql - : `${compactSql.slice(0, 177)}...`; -} - function traceSql(scope: string, message: string, payload?: unknown): void { if (payload === undefined) { console.log(`[IPTVnator Trace][${scope}] ${message}`); @@ -63,10 +60,16 @@ function traceSql(scope: string, message: string, payload?: unknown): void { } console.log( - `[IPTVnator Trace][${scope}] ${message} ${JSON.stringify(payload)}` + `[IPTVnator Trace][${scope}] ${message} ${JSON.stringify( + redactSensitiveData(payload) + )}` ); } +function traceSqlStatement(sql: unknown): void { + traceSql('sql-main', 'query', summarizeSqlStatementForTrace(sql)); +} + /** * Get the database file path */ @@ -402,6 +405,7 @@ export const __databaseConnectionTestHooks = { backfillEpgProgramSourceUrls, cleanupLegacyTmdbSearchCache, runMigrations, + traceSqlStatement, } as const; /** @@ -1119,11 +1123,7 @@ export async function initDatabase( sqlite = new Database(filePath, { readonly, verbose: isSqlTraceEnabled() - ? (message?: unknown) => { - traceSql('sql-main', 'query', { - sql: compactSqlForTrace(String(message ?? '')), - }); - } + ? (message?: unknown) => traceSqlStatement(message) : undefined, }); diff --git a/libs/shared/logging/package.json b/libs/shared/logging/package.json new file mode 100644 index 000000000..dc1a82d44 --- /dev/null +++ b/libs/shared/logging/package.json @@ -0,0 +1,11 @@ +{ + "name": "@iptvnator/shared/logging", + "version": "0.0.1", + "private": true, + "type": "commonjs", + "main": "./src/index.js", + "types": "./src/index.d.ts", + "dependencies": { + "tslib": "^2.3.0" + } +} diff --git a/libs/shared/logging/project.json b/libs/shared/logging/project.json index 031ec83b4..083c8ab35 100644 --- a/libs/shared/logging/project.json +++ b/libs/shared/logging/project.json @@ -5,6 +5,16 @@ "projectType": "library", "tags": ["scope:shared", "domain:shared-runtime", "type:util"], "targets": { + "build": { + "executor": "@nx/js:tsc", + "outputs": ["{options.outputPath}"], + "options": { + "outputPath": "dist/libs/shared/logging", + "main": "libs/shared/logging/src/index.ts", + "tsConfig": "libs/shared/logging/tsconfig.lib.json", + "assets": [] + } + }, "test": { "executor": "@nx/jest:jest", "outputs": ["{workspaceRoot}/coverage/{projectRoot}"], diff --git a/libs/shared/logging/src/index.ts b/libs/shared/logging/src/index.ts index 99feec49b..7a64299d0 100644 --- a/libs/shared/logging/src/index.ts +++ b/libs/shared/logging/src/index.ts @@ -3,6 +3,14 @@ export { redactSensitiveData, } from './lib/redact-sensitive-data'; export type { RedactionOptions } from './lib/redact-sensitive-data'; +export { + SQL_TRACE_STATEMENT_TYPE, + summarizeSqlStatementForTrace, +} from './lib/sql-trace-summary'; +export type { + SqlTraceStatementType, + SqlTraceSummary, +} from './lib/sql-trace-summary'; export { measureRendererPerformancePhase, RENDERER_PERFORMANCE_PHASE, diff --git a/libs/shared/logging/src/lib/sql-trace-summary.spec.ts b/libs/shared/logging/src/lib/sql-trace-summary.spec.ts new file mode 100644 index 000000000..bc72bf49b --- /dev/null +++ b/libs/shared/logging/src/lib/sql-trace-summary.spec.ts @@ -0,0 +1,58 @@ +import { summarizeSqlStatementForTrace } from './sql-trace-summary'; + +describe('summarizeSqlStatementForTrace', () => { + it.each([ + [ + 'SELECT', + `SELECT * FROM playlists WHERE username = 'trace-user-secret' AND password = 'trace-password-secret' AND token = 'trace-token-secret'`, + ], + [ + 'INSERT', + `INSERT INTO playlists (name) VALUES ('O''Brien-secret')`, + ], + [ + 'UPDATE', + `UPDATE playlists SET url = 'https://url-user-secret:url-password-secret@example.com/live?token=url-token-secret'`, + ], + [ + 'DELETE', + `DELETE FROM content WHERE id = 987654321 AND payload = X'7365637265742D626C6F62'`, + ], + [ + 'SELECT', + ` \n\tSeLeCt * FROM content WHERE rating = 12345.6789`, + ], + [ + 'WITH', + `WITH credentials AS (SELECT 'with-secret') SELECT * FROM credentials`, + ], + ])('returns only the %s statement type', (statementType, sql) => { + const summary = summarizeSqlStatementForTrace(sql); + const serialized = JSON.stringify(summary); + + expect(summary).toEqual({ statementType }); + expect(serialized).toBe(`{"statementType":"${statementType}"}`); + expect(serialized).not.toContain('secret'); + expect(serialized).not.toContain(`O''Brien-secret`); + expect(serialized).not.toContain('987654321'); + expect(serialized).not.toContain('7365637265742D626C6F62'); + expect(serialized).not.toContain('12345.6789'); + expect(serialized).not.toContain('length'); + }); + + it.each([ + ` \n-- SELECT 'comment-secret'\nSELECT * FROM playlists`, + `\t/* INSERT 'comment-secret' */ SELECT * FROM playlists`, + `VACUUMINTO 'malicious-secret'`, + `SELECTpassword FROM credentials`, + `DO 'unrecognized-secret'`, + `'; DROP TABLE playlists; -- malicious-secret`, + '', + undefined, + null, + ])('maps comments and unrecognized input to OTHER', (sql) => { + expect(summarizeSqlStatementForTrace(sql)).toEqual({ + statementType: 'OTHER', + }); + }); +}); diff --git a/libs/shared/logging/src/lib/sql-trace-summary.ts b/libs/shared/logging/src/lib/sql-trace-summary.ts new file mode 100644 index 000000000..60f4995b7 --- /dev/null +++ b/libs/shared/logging/src/lib/sql-trace-summary.ts @@ -0,0 +1,58 @@ +export const SQL_TRACE_STATEMENT_TYPE = { + SELECT: 'SELECT', + INSERT: 'INSERT', + UPDATE: 'UPDATE', + DELETE: 'DELETE', + REPLACE: 'REPLACE', + CREATE: 'CREATE', + ALTER: 'ALTER', + DROP: 'DROP', + PRAGMA: 'PRAGMA', + WITH: 'WITH', + BEGIN: 'BEGIN', + COMMIT: 'COMMIT', + ROLLBACK: 'ROLLBACK', + SAVEPOINT: 'SAVEPOINT', + RELEASE: 'RELEASE', + VACUUM: 'VACUUM', + ANALYZE: 'ANALYZE', + REINDEX: 'REINDEX', + ATTACH: 'ATTACH', + DETACH: 'DETACH', + EXPLAIN: 'EXPLAIN', + OTHER: 'OTHER', +} as const; + +export type SqlTraceStatementType = + (typeof SQL_TRACE_STATEMENT_TYPE)[keyof typeof SQL_TRACE_STATEMENT_TYPE]; + +export interface SqlTraceSummary { + statementType: SqlTraceStatementType; +} + +const ALLOWED_STATEMENT_TYPES = new Set( + Object.values(SQL_TRACE_STATEMENT_TYPE).filter( + (statementType) => + statementType !== SQL_TRACE_STATEMENT_TYPE.OTHER + ) as SqlTraceStatementType[] +); + +const SQL_STATEMENT_PREFIX = /^\s*([A-Za-z]+)(?=[\s(;]|$)/; + +export function summarizeSqlStatementForTrace(sql: unknown): SqlTraceSummary { + if (typeof sql !== 'string') { + return { statementType: SQL_TRACE_STATEMENT_TYPE.OTHER }; + } + + const match = SQL_STATEMENT_PREFIX.exec(sql); + const candidate = match?.[1]?.toUpperCase() as + | SqlTraceStatementType + | undefined; + + return { + statementType: + candidate && ALLOWED_STATEMENT_TYPES.has(candidate) + ? candidate + : SQL_TRACE_STATEMENT_TYPE.OTHER, + }; +}