feat(shared): redact paths from DatabaseError messages (W-3.5-SEC-L1)

code/src/shared/infrastructure/errors/database-error.ts

@@ -19,6 +19,19 @@ import { InfrastructureError } from "./infrastructure-error.ts";
  *
  * Construction is via static factories (one per `code`) so that the
  * `code` literal cannot drift from the discriminator type.
+ *
+ * Path/identifier redaction (W-3.5-SEC-L1):
+ * - Filesystem paths and similar leak-prone identifiers (e.g. the
+ *   absolute path of the SQLite file, the migrations directory) are
+ *   stored in the structured {@link DatabaseError.details} bag, NOT
+ *   concatenated into `message`. Pino's redactor walks structured keys
+ *   (see `DEFAULT_REDACT_PATHS` in `pino-logger.ts`) but does NOT
+ *   inspect message content — keeping paths out of the message is
+ *   what makes them redactable when these errors flow through the
+ *   logger.
+ * - Callers that need the path read it from `details.path` /
+ *   `details.dir`. Tests that previously asserted on `error.message`
+ *   substring should pivot to `error.details.path`.
  */
 export type DatabaseErrorCode =
   | "database.open-failed"
@@ -32,22 +45,45 @@ export type DatabaseErrorCode =
   | "database.migration-failed"
   | "database.migration-directory-invalid";
 
+/**
+ * Structured side-channel for sensitive or operationally-useful
+ * identifiers attached to a {@link DatabaseError}.
+ *
+ * The bag is populated by the static factories; callers MUST treat it
+ * as read-only. Keys are lowercase ASCII; values are JSON-serializable
+ * primitives (no nested objects yet — kept narrow on purpose so the
+ * pino redact globs `*.details.path` / `*.details.dir` match cleanly
+ * without `**` recursion).
+ */
+export type DatabaseErrorDetails = Readonly<Record<string, unknown>>;
+
 export class DatabaseError extends InfrastructureError {
   public readonly code: DatabaseErrorCode;
 
+  /**
+   * Structured fields that supplement {@link Error.message} without
+   * appearing inside the message string. Always defined (empty object
+   * when a factory has nothing to attach) so callers can dot-access
+   * `details.path` without an undefined-guard.
+   */
+  public readonly details: DatabaseErrorDetails;
+
   private constructor(
     code: DatabaseErrorCode,
     message: string,
+    details: DatabaseErrorDetails,
     cause?: unknown,
   ) {
     super(message, cause !== undefined ? { cause } : undefined);
     this.code = code;
+    this.details = details;
   }
 
   public static openFailed(path: string, cause: unknown): DatabaseError {
     return new DatabaseError(
       "database.open-failed",
-      `failed to open SQLite database at ${path}`,
+      "failed to open SQLite database",
+      { path },
       cause,
     );
   }
@@ -56,6 +92,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.encryption-key-rejected",
       "encryption key was rejected by SQLCipher (wrong key or corrupted DB header)",
+      {},
       cause,
     );
   }
@@ -67,6 +104,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.extension-load-failed",
       `failed to load SQLite extension "${extensionName}"`,
+      { extensionName },
       cause,
     );
   }
@@ -75,6 +113,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.prepare-failed",
       `failed to prepare SQL statement (sql length=${String(sql.length)})`,
+      { sqlLength: sql.length },
       cause,
     );
   }
@@ -83,6 +122,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.exec-failed",
       "failed to execute SQL batch",
+      {},
       cause,
     );
   }
@@ -91,6 +131,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.transaction-failed",
       "transaction rolled back due to a thrown exception",
+      {},
       cause,
     );
   }
@@ -99,6 +140,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.connection-closed",
       `cannot ${operation}: database connection has been closed`,
+      { operation },
     );
   }
 
@@ -109,6 +151,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.migration-ahead-of-code",
       `database schema_migrations top version is ${String(dbVersion)} but code only ships migrations up to ${String(codeMaxVersion)}; refusing to start to avoid silent corruption`,
+      { dbVersion, codeMaxVersion },
     );
   }
 
@@ -120,6 +163,7 @@ export class DatabaseError extends InfrastructureError {
     return new DatabaseError(
       "database.migration-failed",
       `migration ${String(version)} (${name}) failed and was rolled back`,
+      { version, name },
       cause,
     );
   }
@@ -130,7 +174,8 @@ export class DatabaseError extends InfrastructureError {
   ): DatabaseError {
     return new DatabaseError(
       "database.migration-directory-invalid",
-      `migrations directory ${dir} is invalid: ${reason}`,
+      `migrations directory is invalid: ${reason}`,
+      { dir, reason },
     );
   }
 }

code/src/shared/infrastructure/logger/pino-logger.ts

@@ -57,6 +57,18 @@ export const DEFAULT_REDACT_PATHS: readonly string[] = Object.freeze([
   // Two-level wildcards for request-style envelopes (e.g. headers).
   "*.headers.authorization",
   "*.headers.cookie",
+  // Filesystem paths attached to structured error envelopes
+  // (W-3.5-SEC-L1). DatabaseError stores absolute paths under
+  // `details.path` / `details.dir` instead of concatenating them into
+  // `message`; pino's redactor only walks structured keys, so these
+  // globs ensure the path never leaves the process in plaintext when
+  // an error is logged via `logger.error({ err }, "...")`. The literal
+  // top-level entries cover the rare case where details is logged
+  // standalone (e.g. `logger.error({ details })`).
+  "details.path",
+  "details.dir",
+  "*.details.path",
+  "*.details.dir",
 ]);
 
 /**

code/tests/unit/shared/infrastructure/errors/errors.test.ts

@@ -34,63 +34,109 @@ describe("InfrastructureError", () => {
 });
 
 describe("DatabaseError factories", () => {
-  it("openFailed", () => {
-    const e = DatabaseError.openFailed("/tmp/db", new Error("ENOSPC"));
+  it("openFailed keeps the absolute path out of message and exposes it via details", () => {
+    const sensitivePath = "/Users/alice/secret/workspace/recall.db";
+    const e = DatabaseError.openFailed(sensitivePath, new Error("ENOSPC"));
     expect(e.code).toBe("database.open-failed");
-    expect(e.message).toContain("/tmp/db");
+    // VALOR (W-3.5-SEC-L1): the path MUST NOT appear in `message` so
+    // that pino's redactor (which only walks structured fields) can
+    // redact it when this error is logged via `logger.error({ err })`.
+    expect(e.message).not.toContain(sensitivePath);
+    expect(e.message).not.toContain("/Users/alice");
+    expect(e.message).toBe("failed to open SQLite database");
+    // The path is still accessible via the structured side-channel
+    // for callers that need it (e.g. UI surfaces, debugging tools).
+    expect(e.details["path"]).toBe(sensitivePath);
   });
 
   it("encryptionKeyRejected", () => {
     const e = DatabaseError.encryptionKeyRejected(new Error("bad"));
     expect(e.code).toBe("database.encryption-key-rejected");
+    expect(e.details).toEqual({});
   });
 
   it("extensionLoadFailed", () => {
     const e = DatabaseError.extensionLoadFailed("sqlite-vec", new Error("x"));
     expect(e.code).toBe("database.extension-load-failed");
     expect(e.message).toContain("sqlite-vec");
+    expect(e.details["extensionName"]).toBe("sqlite-vec");
   });
 
   it("prepareFailed", () => {
     const e = DatabaseError.prepareFailed("SELECT 1", new Error("syntax"));
     expect(e.code).toBe("database.prepare-failed");
     expect(e.message).toContain("8"); // sql length
+    expect(e.details["sqlLength"]).toBe(8);
   });
 
   it("execFailed", () => {
     const e = DatabaseError.execFailed(new Error("x"));
     expect(e.code).toBe("database.exec-failed");
+    expect(e.details).toEqual({});
   });
 
   it("transactionFailed", () => {
     const e = DatabaseError.transactionFailed(new Error("x"));
     expect(e.code).toBe("database.transaction-failed");
+    expect(e.details).toEqual({});
   });
 
   it("connectionClosed", () => {
     const e = DatabaseError.connectionClosed("prepare");
     expect(e.code).toBe("database.connection-closed");
     expect(e.message).toContain("prepare");
+    expect(e.details["operation"]).toBe("prepare");
   });
 
   it("migrationAheadOfCode", () => {
     const e = DatabaseError.migrationAheadOfCode(5, 3);
     expect(e.code).toBe("database.migration-ahead-of-code");
     expect(e.message).toContain("5");
     expect(e.message).toContain("3");
+    expect(e.details["dbVersion"]).toBe(5);
+    expect(e.details["codeMaxVersion"]).toBe(3);
   });
 
   it("migrationFailed", () => {
     const e = DatabaseError.migrationFailed(2, "core", new Error("x"));
     expect(e.code).toBe("database.migration-failed");
     expect(e.message).toContain("2");
     expect(e.message).toContain("core");
+    expect(e.details["version"]).toBe(2);
+    expect(e.details["name"]).toBe("core");
   });
 
-  it("migrationDirectoryInvalid", () => {
-    const e = DatabaseError.migrationDirectoryInvalid("/tmp", "duplicate");
+  it("migrationDirectoryInvalid keeps the directory path out of message and exposes it via details", () => {
+    const sensitiveDir = "/Users/alice/secret/workspace/migrations";
+    const e = DatabaseError.migrationDirectoryInvalid(
+      sensitiveDir,
+      "duplicate migration version 1",
+    );
     expect(e.code).toBe("database.migration-directory-invalid");
-    expect(e.message).toContain("/tmp");
+    // VALOR (W-3.5-SEC-L1): the absolute path MUST NOT leak into the
+    // message — only the reason (which the call sites construct from
+    // safe content like "duplicate migration version N") is visible.
+    expect(e.message).not.toContain(sensitiveDir);
+    expect(e.message).not.toContain("/Users/alice");
+    expect(e.message).toBe(
+      "migrations directory is invalid: duplicate migration version 1",
+    );
+    expect(e.details["dir"]).toBe(sensitiveDir);
+    expect(e.details["reason"]).toBe("duplicate migration version 1");
+  });
+
+  it("backward-compat: callers pivot from message-substring to details for paths", () => {
+    // Documents the migration path for callers/tests that previously
+    // pinned the absolute path via `error.message.includes(...)`. The
+    // new contract is `error.details.path` for openFailed and
+    // `error.details.dir` for migrationDirectoryInvalid.
+    const dbPath = "/var/lib/recall/db.sqlite";
+    const openErr = DatabaseError.openFailed(dbPath, new Error("x"));
+    expect(openErr.details["path"]).toBe(dbPath);
+
+    const migDir = "/var/lib/recall/migrations";
+    const migErr = DatabaseError.migrationDirectoryInvalid(migDir, "empty");
+    expect(migErr.details["dir"]).toBe(migDir);
   });
 });
 

code/tests/unit/shared/infrastructure/logger/pino-logger.test.ts

@@ -114,6 +114,20 @@ describe("DEFAULT_REDACT_PATHS", () => {
     }
   });
 
+  it("contains structured-error path globs (W-3.5-SEC-L1)", () => {
+    // DatabaseError stows absolute paths under `details.path` /
+    // `details.dir`; the globs below ensure pino's redactor catches
+    // them when the error envelope is logged via `{ err }`.
+    for (const k of [
+      "details.path",
+      "details.dir",
+      "*.details.path",
+      "*.details.dir",
+    ]) {
+      expect(DEFAULT_REDACT_PATHS).toContain(k);
+    }
+  });
+
   it("is frozen", () => {
     expect(Object.isFrozen(DEFAULT_REDACT_PATHS)).toBe(true);
   });
@@ -258,6 +272,59 @@ describe("PinoLogger.create — redaction (security-critical)", () => {
     expect(text).toContain("[REDACTED]");
   });
 
+  it("redacts DatabaseError details.path / details.dir end-to-end (W-3.5-SEC-L1)", () => {
+    // Mirrors the canonical caller pattern:
+    //   logger.error({ err }, "open failed")
+    // where `err` is a DatabaseError whose `details.path` carries the
+    // absolute SQLite path. The redactor must walk the structured
+    // envelope and never let the path land in the JSON line.
+    const log = PinoLogger.create({ level: "info" });
+    const sensitivePath = "/Users/alice/secret/workspace/recall.db";
+    const sensitiveDir = "/Users/alice/secret/workspace/migrations";
+    log.error(
+      {
+        err: {
+          name: "DatabaseError",
+          code: "database.open-failed",
+          message: "failed to open SQLite database",
+          details: { path: sensitivePath },
+        },
+      },
+      "open failed",
+    );
+    log.error(
+      {
+        err: {
+          name: "DatabaseError",
+          code: "database.migration-directory-invalid",
+          message: "migrations directory is invalid: duplicate",
+          details: { dir: sensitiveDir, reason: "duplicate" },
+        },
+      },
+      "migration dir invalid",
+    );
+    const text = joined(captured.stdout);
+    expect(text).not.toContain(sensitivePath);
+    expect(text).not.toContain(sensitiveDir);
+    expect(text).not.toContain("/Users/alice");
+    expect(text).toContain("[REDACTED]");
+    // The non-sensitive `reason` field stays visible — proves redact is
+    // surgical, not a blanket details-bag wipeout.
+    expect(text).toContain('"reason":"duplicate"');
+  });
+
+  it("redacts top-level details.path when details is logged standalone", () => {
+    // Some callers log a `details` envelope directly (not wrapped in
+    // `err`); the literal `details.path` glob covers that shape.
+    const log = PinoLogger.create({ level: "info" });
+    const sensitivePath = "/var/lib/recall/db.sqlite";
+    log.info({ details: { path: sensitivePath, op: "open" } }, "ev");
+    const text = joined(captured.stdout);
+    expect(text).not.toContain(sensitivePath);
+    expect(text).toContain("[REDACTED]");
+    expect(text).toContain('"op":"open"');
+  });
+
   it("merges custom redact paths on top of the defaults", () => {
     const log = PinoLogger.create({
       level: "info",