fix: harden file and input handling (#1234)

* fix: harden file and input handling

* fix: keep URI scheme regex lint-safe

scripts/finalize-macos-release-assets.mjs

@@ -11,21 +11,6 @@ const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..')
 const desktopDir = path.join(rootDir, 'desktop');
 const releaseDir = path.join(desktopDir, 'release');
 const requireFromScript = createRequire(import.meta.url);
-const sensitiveArgumentFlags = new Set([
-  '--apple-id',
-  '--issuer',
-  '--key',
-  '--key-id',
-  '--password',
-  '--team-id',
-]);
-
-function sanitizeArgsForError(args) {
-  return args.map((arg, index) =>
-    index > 0 && sensitiveArgumentFlags.has(args[index - 1]) ? '<redacted>' : arg
-  );
-}
-
 function run(command, args) {
   const result = spawnSync(command, args, {
     cwd: rootDir,
@@ -34,13 +19,11 @@ function run(command, args) {
   });
 
   if (result.error) {
-    throw result.error;
+    throw new Error(`${command} failed to start`);
   }
 
   if (result.status !== 0) {
-    throw new Error(
-      `${command} ${sanitizeArgsForError(args).join(' ')} failed with exit code ${result.status}`
-    );
+    throw new Error(`${command} failed with exit code ${result.status}`);
   }
 }
 
@@ -203,6 +186,6 @@ async function main() {
 }
 
 main().catch((error) => {
-  console.error(error instanceof Error ? error.message : error);
+  console.error(error instanceof Error ? error.message : 'Unknown release finalization error');
   process.exit(1);
 });

server/src/__tests__/security-config-randomness.test.ts

@@ -0,0 +1,13 @@
+import { describe, expect, it } from 'vitest';
+import { generateRecoveryKey } from '../config/security.js';
+
+describe('security config recovery keys', () => {
+  it('generates the documented alphabet and grouping', () => {
+    const keys = Array.from({ length: 128 }, () => generateRecoveryKey());
+
+    expect(new Set(keys).size).toBe(keys.length);
+    for (const key of keys) {
+      expect(key).toMatch(/^[A-HJ-KM-NP-Z2-9]{4}(?:-[A-HJ-KM-NP-Z2-9]{4}){3}$/);
+    }
+  });
+});

server/src/__tests__/text-extraction-service.test.ts

@@ -158,6 +158,18 @@ describe('TextExtractionService', () => {
       expect(extracted).toContain('Hello World');
       expect(extracted).toContain('paragraph');
     });
+
+    it('discards malformed script and style end tags without leaving executable content', async () => {
+      const filepath = path.join(testRoot, 'malformed-end-tags.html');
+      await fs.writeFile(
+        filepath,
+        '<p>Visible</p><script>alert(1)</script ><style>body{display:none}</style >'
+      );
+
+      const extracted = await service.extractText(filepath, 'text/html');
+
+      expect(extracted).toBe('Visible');
+    });
   });
 
   describe('JSON extraction', () => {

server/src/config/security.ts

@@ -378,10 +378,9 @@ export function saveSecurityConfig(config: SecurityConfig): void {
 export function generateRecoveryKey(): string {
   const chars = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789'; // Omit confusing chars (0/O, 1/I/L)
   let key = '';
-  const bytes = crypto.randomBytes(16);
 
   for (let i = 0; i < 16; i++) {
-    key += chars[bytes[i] % chars.length];
+    key += chars[crypto.randomInt(chars.length)];
     if (i === 3 || i === 7 || i === 11) {
       key += '-';
     }

server/src/services/text-extraction-service.ts

@@ -3,6 +3,7 @@ import { extractText as unpdfExtract } from 'unpdf';
 import mammoth from 'mammoth';
 import ExcelJS from 'exceljs';
 import { createLogger } from '../lib/logger.js';
+import { stripHtml } from '../utils/sanitize.js';
 const log = createLogger('text-extraction-service');
 
 export interface TextExtractionResult {
@@ -77,7 +78,7 @@ export class TextExtractionService {
       // Unknown type
       return null;
     } catch (error) {
-      log.error({ err: error }, `Text extraction failed for ${filepath}`);
+      log.error({ err: error, filepath }, 'Text extraction failed');
       return null;
     }
   }
@@ -178,13 +179,7 @@ export class TextExtractionService {
     try {
       const html = await fs.readFile(filepath, 'utf-8');
 
-      // Simple tag stripping (for more complex HTML, consider using a library like cheerio)
-      const text = html
-        .replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '')
-        .replace(/<style\b[^<]*(?:(?!<\/style>)<[^<]*)*<\/style>/gi, '')
-        .replace(/<[^>]+>/g, ' ')
-        .replace(/\s+/g, ' ')
-        .trim();
+      const text = stripHtml(html).replace(/\s+/g, ' ').trim();
 
       return text || null;
     } catch (error) {

server/src/storage/fs-helpers.ts

@@ -113,11 +113,22 @@ export async function atomicWriteFile(
 ): Promise<void> {
   const suffix = randomBytes(6).toString('hex');
   const tmpPath = `${destPath}.tmp.${suffix}`;
+  let handle: Awaited<ReturnType<typeof openAsync>> | undefined;
 
   try {
-    await writeFileAsync(tmpPath, content, encoding);
+    const noFollow = typeof fs.constants.O_NOFOLLOW === 'number' ? fs.constants.O_NOFOLLOW : 0;
+    handle = await openAsync(
+      tmpPath,
+      fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY | noFollow,
+      0o600
+    );
+    await handle.writeFile(content, typeof content === 'string' ? { encoding } : undefined);
+    await handle.sync();
+    await handle.close();
+    handle = undefined;
     await renameAsync(tmpPath, destPath);
   } catch (err) {
+    await handle?.close().catch(() => {});
     // Best-effort cleanup; ignore errors — the original file is untouched.
     await unlinkAsync(tmpPath).catch(() => {});
     throw err;

server/src/storage/sqlite/database.ts

@@ -1,5 +1,14 @@
 import { createHash } from 'crypto';
-import { closeSync, existsSync, lstatSync, mkdirSync, openSync, readSync, statSync } from 'fs';
+import {
+  closeSync,
+  constants,
+  existsSync,
+  fstatSync,
+  lstatSync,
+  mkdirSync,
+  openSync,
+  readSync,
+} from 'fs';
 import { dirname, join, resolve } from 'path';
 import { DatabaseSync } from 'node:sqlite';
 import type { SqliteStorageDiagnostics } from '@veritas-kanban/shared';
@@ -540,13 +549,22 @@ export class SqliteDatabase {
 }
 
 function inspectExistingSqliteJournalPosture(databasePath: string): ExistingSqliteJournalPosture {
-  if (!existsSync(databasePath) || statSync(databasePath).size === 0) {
-    return 'new';
+  let file: number;
+  try {
+    const noFollow = typeof constants.O_NOFOLLOW === 'number' ? constants.O_NOFOLLOW : 0;
+    file = openSync(databasePath, constants.O_RDONLY | noFollow);
+  } catch (error) {
+    const code = (error as NodeJS.ErrnoException).code;
+    if (code === 'ENOENT') return 'new';
+    if (code === 'ELOOP') return 'unrecognized';
+    throw error;
   }
 
   const header = Buffer.alloc(20);
-  const file = openSync(databasePath, 'r');
   try {
+    const opened = fstatSync(file);
+    if (!opened.isFile()) return 'unrecognized';
+    if (opened.size === 0) return 'new';
     const bytesRead = readSync(file, header, 0, header.length, 0);
     if (bytesRead < header.length || header.subarray(0, 16).toString('utf8') !== SQLITE_HEADER) {
       return 'unrecognized';

web/src/lib/__tests__/sanitize.test.ts

@@ -63,6 +63,12 @@ describe('sanitizeHtml', () => {
     expect(result).toContain('rel="noopener noreferrer"');
   });
 
+  it('does not treat digits as URI-scheme characters', () => {
+    const result = sanitizeHtml('<a href="1javascript:alert(1)">blocked</a>');
+    expect(result).not.toContain('href=');
+    expect(result).toContain('blocked');
+  });
+
   it('preserves tables', () => {
     const input = '<table><tr><td>Cell</td></tr></table>';
     expect(sanitizeHtml(input)).toContain('<td>Cell</td>');

web/src/lib/sanitize.ts

@@ -68,7 +68,7 @@ const ALLOWED_ATTR = [
  * URI schemes allowed in href/src attributes.
  * Blocks javascript:, data:, vbscript:, etc.
  */
-const ALLOWED_URI_REGEXP = /^(?:(?:https?|mailto|tel|ftp):|[^a-z]|[a-z+.-]+(?:[^a-z+.-:]|$))/i;
+const ALLOWED_URI_REGEXP = /^(?:(?:https?|mailto|tel|ftp):|[^a-z]|[a-z+.-]+(?:[^a-z+.:-]|$))/i;
 
 /**
  * Sanitize HTML content for safe rendering.