fix(objectql): drop the caller value MySQL inlines in its duplicate-entry diagnostic (#8823) (#9162)

The #8682 statement cut keeps the database's own diagnostic because it names
the failing identifier. MySQL's ER_DUP_ENTRY prints the conflicting VALUE in
that diagnostic instead, so it survived the cut into the server log.

The tail is still kept, index name included; only the value slot is replaced.
Also closes the fragment a value containing " - " used to leave behind.

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/mysql-duplicate-entry-log-value.md

@@ -0,0 +1,25 @@
+---
+"@objectstack/objectql": patch
+---
+
+Keep a caller's value out of the server log when MySQL reports a duplicate entry
+
+The driver-fault redaction added for #8682 replaces the bound statement in a logged
+write fault and keeps the database's own diagnostic, because that diagnostic names the
+failing identifier an operator needs. On MySQL's `ER_DUP_ENTRY` (1062) that premise does
+not hold: the template is `Duplicate entry '<value>' for key '<index>'`, so the
+conflicting value is in the diagnostic rather than in the statement and survived the cut.
+
+The tail is still kept — including `for key '<index>'`, which is the answer to "which
+constraint?" — and only the value slot is replaced:
+
+```
+before  Duplicate entry 'acme@example.com' for key 'crm_account.email' [statement and bound values redacted]
+after   Duplicate entry [value redacted] for key 'crm_account.email' [statement and bound values redacted]
+```
+
+Also closed: a value spelled with `" - "` in it used to leave a fragment behind, because
+the statement cut takes the last separator and that separator was inside the value.
+
+Identifier-bearing diagnostics on every dialect are unchanged, the rethrown error is
+untouched, and no HTTP response moves — this narrows one log slot only.

packages/objectql/src/driver-fault-redaction.test.ts

@@ -100,6 +100,107 @@ describe('redactStatementFromMessage', () => {
   });
 });
 
+// #8823 — the tail is kept because it names IDENTIFIERS, and on MySQL's
+// duplicate-entry family it does not: `ER_DUP_ENTRY` prints the conflicting
+// VALUE in the diagnostic itself. These cases pin both halves of the remedy —
+// the value goes, the index name stays — because a fix that blanked the tail
+// would trade away exactly the debuggability #8682 paid for.
+//
+// ⛔ Not a demonstrated deployment leak: no live MySQL server was measured.
+// The inputs are this repo's own recorded mysql2 phrasings
+// (`packages/types/src/unique-violation.ts`) in knex's documented shape.
+describe('#8823 — a caller value inlined in the diagnostic itself', () => {
+  const EMAIL = 'acme@example.com';
+  const BOUND_DUP_ENTRY =
+    'insert into `crm_account` (`email`, `name`) values '
+    + `('${EMAIL}', 'Acme')`
+    + ` - Duplicate entry '${EMAIL}' for key 'crm_account.email'`;
+
+  it('drops the conflicting value and keeps the index the operator needs', () => {
+    const out = redactStatementFromMessage(BOUND_DUP_ENTRY);
+
+    expect(out).not.toContain(EMAIL);
+    // The index name is the answer to "which constraint?" and survives whole.
+    expect(out).toContain("for key 'crm_account.email'");
+    // Still legibly MySQL's own diagnostic, not a blanked tail.
+    expect(out).toContain('Duplicate entry');
+    expect(out).toBe(
+      "Duplicate entry [value redacted] for key 'crm_account.email' [statement and bound values redacted]",
+    );
+  });
+
+  it('redacts a BARE diagnostic too — the shape that reaches us without a statement', () => {
+    // Before #9030 the shared leak predicate did not recognise this phrasing,
+    // so a bare `Duplicate entry …` was turned away at the door and kept its
+    // value. That limb landed for a different reason; the two compose here.
+    const out = redactStatementFromMessage(`Duplicate entry '${EMAIL}' for key 'crm_account.email'`);
+
+    expect(out).not.toContain(EMAIL);
+    expect(out).toBe("Duplicate entry [value redacted] for key 'crm_account.email'");
+    // No statement was present, so the entry must not claim one was removed.
+    expect(out).not.toContain('[statement and bound values redacted]');
+  });
+
+  it('leaves no fragment when the value itself contained " - "', () => {
+    // The statement cut takes the LAST separator, which lands INSIDE a value
+    // spelled like this — measured on the shipped function, it logged
+    // `Q3 plan' for key 't.label'`. The words are not reconstructed: an anchor
+    // is evidence about the value, not licence to assert the template.
+    const out = redactStatementFromMessage(
+      "insert into `t` (`label`) values ('2026 - Q3 plan')"
+      + " - Duplicate entry '2026 - Q3 plan' for key 't.label'",
+    );
+
+    expect(out).not.toContain('Q3 plan');
+    expect(out).not.toContain('2026');
+    expect(out).toContain("for key 't.label'");
+  });
+
+  it.each([
+    ["an unescaped quote in the value", "Duplicate entry 'O'Brien' for key 't.name'", 'Brien', "for key 't.name'"],
+    ['a composite key value', "Duplicate entry 'acme-x' for key 't.idx_a_b'", 'acme-x', "for key 't.idx_a_b'"],
+    ['the PRIMARY key', "Duplicate entry 'r1' for key 'PRIMARY'", "'r1'", "for key 'PRIMARY'"],
+    // Ambiguous: the value may itself contain the anchor. Resolving to the LAST
+    // anchor discards more, which is the only direction that cannot leak.
+    ['a value that mimics the anchor', "Duplicate entry 'a' for key 'b' for key 't.n'", "for key 'b'", "for key 't.n'"],
+  ])('%s', (_shape, diagnostic, gone, kept) => {
+    const out = redactStatementFromMessage(`insert into \`t\` (\`c\`) values ('v') - ${diagnostic}`);
+
+    expect(out).not.toContain(gone);
+    expect(out).toContain(kept);
+    expect(out).toContain('[value redacted]');
+  });
+
+  it('leaves every IDENTIFIER-bearing tail exactly as it was', () => {
+    // The other three dialect shapes the card measured, plus the MySQL family
+    // that names a column rather than a value. Redacting these would be the
+    // regression #8682's triage warned about, not a fix.
+    for (const [statement, diagnostic] of [
+      ["insert into `t` (`c`) values ('v')", "Unknown column 'zzz' in 'field list'"],
+      ["insert into `t` (`c`) values ('v')", 'UNIQUE constraint failed: crm_account.email'],
+      ['insert into "t" ("c") values (\'v\')', 'duplicate key value violates unique constraint "crm_account_email_key"'],
+      ["insert into `t` (`c`) values ('v')", 'NOT NULL constraint failed: sys_team.organization_id'],
+    ]) {
+      const out = redactStatementFromMessage(`${statement} - ${diagnostic}`);
+
+      expect(out).toBe(`${diagnostic} [statement and bound values redacted]`);
+      expect(out).not.toContain('[value redacted]');
+    }
+  });
+
+  it('does not reach into a bound value that merely looks like the template', () => {
+    // The templates are read only AFTER the cut, where nothing but the
+    // database's own words is left — so a caller storing this text in a column
+    // cannot steer what survives.
+    const out = redactStatementFromMessage(
+      "insert into `t` (`note`) values ('Duplicate entry \\'x\\' for key \\'k\\'')"
+      + ' - table t has no column named note',
+    );
+
+    expect(out).toBe('table t has no column named note [statement and bound values redacted]');
+  });
+});
+
 describe('redactBoundStatement', () => {
   it('redacts `stack` too — the statement opened it a second time', () => {
     const original = new Error(BOUND_INSERT);
@@ -167,7 +268,18 @@ describe('#8682 half B — the write-path loggers', () => {
     return logger;
   }
 
-  async function insertAgainstADriftedColumn() {
+  /**
+   * The one driver double in this file. #8823 needed a MySQL-shaped fault and
+   * the shape is a PARAMETER rather than a second double — one fake engine per
+   * file keeps the contract the double implements reviewable in one place.
+   */
+  const DRIFTED_COLUMN = {
+    name: 'SqliteError',
+    code: 'SQLITE_ERROR',
+    diagnostic: (object: string) => `table ${object} has no column named secret_note`,
+  };
+
+  async function insertAgainstADriftedColumn(fault = DRIFTED_COLUMN) {
     const logger = makeCapturingLogger();
     const engine = new ObjectQL({ logger });
     const driver: any = {
@@ -178,10 +290,11 @@ describe('#8682 half B — the write-path loggers', () => {
       async create(object: string, data: Record<string, unknown>) {
         const cols = Object.keys(data).sort();
         const stmt = `insert into \`${object}\` (${cols.map((c) => `\`${c}\``).join(', ')}) values (${cols.map((c) => `'${String(data[c])}'`).join(', ')}) returning *`;
-        const e: any = new Error(`${stmt} - table ${object} has no column named secret_note`);
-        e.name = 'SqliteError';
-        e.code = 'SQLITE_ERROR';
-        e.stack = `SqliteError: ${stmt} - table ${object} has no column named secret_note\n    at Database.prepare (/x/better-sqlite3.js:1:1)`;
+        const text = `${stmt} - ${fault.diagnostic(object)}`;
+        const e: any = new Error(text);
+        e.name = fault.name;
+        e.code = fault.code;
+        e.stack = `${fault.name}: ${text}\n    at Database.prepare (/x/better-sqlite3.js:1:1)`;
         throw e;
       },
       async update() { return {}; }, async updateMany() { return 0; },
@@ -248,4 +361,48 @@ describe('#8682 half B — the write-path loggers', () => {
     expect(String(thrown?.message)).toContain('insert into');
     expect(String(thrown?.message)).toContain(SECRET);
   });
+
+  /**
+   * [#8823] The same write path, with the fault MySQL raises instead — where
+   * the caller's value is in the DIAGNOSTIC and not only in the statement, so
+   * the statement cut alone never reached it.
+   */
+  const MYSQL_DUPLICATE_ENTRY = {
+    name: 'Error',
+    code: 'ER_DUP_ENTRY',
+    diagnostic: (object: string) => `Duplicate entry '${SECRET}' for key '${object}.secret_note'`,
+  };
+
+  it('MySQL duplicate entry — the entry survives and still names the index', async () => {
+    const { line } = await insertAgainstADriftedColumn(MYSQL_DUPLICATE_ENTRY);
+
+    expect(line).toBeDefined();
+    expect(line!.level).toBe('error');
+    expect(line!.meta).toEqual({ object: 'crm_account' });
+    // The operator's answer to "which constraint?" is kept whole.
+    expect(String(line!.err?.message)).toContain("for key 'crm_account.secret_note'");
+    expect(String(line!.err?.stack)).toContain('at Database.prepare');
+  });
+
+  it('MySQL duplicate entry — neither `message` nor `stack` carries the value', async () => {
+    const { line } = await insertAgainstADriftedColumn(MYSQL_DUPLICATE_ENTRY);
+
+    for (const field of [String(line!.err?.message), String(line!.err?.stack)]) {
+      expect(field).not.toContain(SECRET);
+      expect(field).not.toContain(DESCRIPTION);
+      expect(field).not.toContain('insert into');
+    }
+  });
+
+  it('MySQL duplicate entry — the RETHROWN error is still untouched', async () => {
+    // Same boundary as above: the log narrows, the caller's answer does not
+    // move. `isUniqueViolationError` and `uniqueViolationColumn` read this
+    // message downstream and must keep seeing the driver's own text.
+    const { thrown } = await insertAgainstADriftedColumn(MYSQL_DUPLICATE_ENTRY);
+
+    expect(String(thrown?.message)).toContain('insert into');
+    expect(String(thrown?.message)).toContain(SECRET);
+    expect(String(thrown?.message)).toContain('Duplicate entry');
+    expect((thrown as any)?.code).toBe('ER_DUP_ENTRY');
+  });
 });

packages/objectql/src/driver-fault-redaction.ts

@@ -31,12 +31,49 @@
  * The driver's own diagnostic — the tail — is the half that names the failing
  * column and the condition (`table crm_account has no column named
  * zzz_nonexistent_field`, `NOT NULL constraint failed: sys_team.name`). It is
- * kept verbatim, and the log site keeps the `object` it already carried. ⛔ The
+ * kept, and the log site keeps the `object` it already carried. ⛔ The
  * remedy for this exposure is NOT to lower the level or drop the entry: a
  * driver-level fault that logs nothing is a fault nobody can debug, which is
  * strictly worse than one logged too loudly. This narrows WHAT is written; the
  * level, the message and the entry are untouched.
  *
+ * ## [#8823] …but "the tail names identifiers" is not true of every dialect
+ *
+ * The paragraph above was written with a premise attached: that whatever the
+ * database prints after the separator names IDENTIFIERS — a column, a table, a
+ * constraint. That premise was checked against SQLite and Postgres, and it is
+ * false on MySQL for one family. MySQL's `ER_DUP_ENTRY` (1062) puts the
+ * conflicting value in the diagnostic itself rather than in the statement:
+ *
+ * ```
+ * mysql  unknown column   => "Unknown column 'zzz' in 'field list' […]"
+ * mysql  duplicate entry  => "Duplicate entry 'acme@example.com' for key 'crm_account.email' […]"
+ * sqlite unique violation => "UNIQUE constraint failed: crm_account.email […]"
+ * pg     unique violation => "duplicate key value violates unique constraint "crm_account_email_key" […]"
+ * ```
+ *
+ * Three keep an identifier; the second keeps a caller's value. Measured through
+ * this function, not predicted — and re-measured byte-identical after #9030
+ * taught the shared leak predicate this phrasing, which moves the VERDICT but
+ * not the cut.
+ *
+ * Postgres is not saved by the cut here — it is saved by an unrelated fact:
+ * its conflicting value lives on `error.detail`, a field `Logger` never
+ * serializes (it writes `message` and `stack`, nothing else). That is a
+ * coincidence, not a defence, and nothing below makes `detail` reachable.
+ *
+ * So the tail is kept, minus the spans a dialect's own template documents as a
+ * VALUE — see {@link redactDiagnosticValues}. Identifier-bearing tails are
+ * untouched, which is the property #8682 paid for on purpose: MySQL's
+ * `for key '…'` names an INDEX (`uniqueViolationColumn` in
+ * `@objectstack/types` refuses to read a column out of it for exactly that
+ * reason), and an operator debugging a duplicate needs that index name.
+ *
+ * ⛔ This is a server LOG. The rethrown error is untouched, every HTTP boundary
+ * is unaffected, and no live MySQL deployment was measured — the input strings
+ * are this repo's own recorded mysql2 phrasings (`unique-violation.ts`) driven
+ * through the real function.
+ *
  * ## Why the cut is at the separator, and not at a statement keyword
  *
  * "Is this a driver dump?" already has ONE owner in this repo —
@@ -86,6 +123,46 @@ const STATEMENT_SEPARATOR = ' - ';
 /** What replaces a statement that carried nothing but values. */
 const REDACTED_STATEMENT = '[statement and bound values redacted]';
 
+/** [#8823] What replaces one caller value inlined in the database's own diagnostic. */
+const REDACTED_VALUE = '[value redacted]';
+
+/**
+ * [#8823] MySQL/MariaDB `ER_DUP_ENTRY` (1062), whole: the template's own head,
+ * the conflicting VALUE, and the `for key <index>` tail that anchors it.
+ *
+ * `Duplicate entry '%-.192s' for key '%-.192s'` — the first slot is whatever the
+ * caller tried to write, the second is the index name. MySQL escapes neither,
+ * so the value's own closing quote is not distinguishable on sight
+ * (`Duplicate entry 'O'Brien' for key 'i'` is a real shape) and only the
+ * ` for key '…'` anchor bounds it. The key token requires quotes because that
+ * is what separates this template from prose that merely contains the words.
+ *
+ * The value is matched GREEDILY, so an ambiguous message resolves to the LAST
+ * anchor — `Duplicate entry 'a' for key 'b' for key 't.n'` reads as the value
+ * `a' for key 'b`, not as a short value with a fragment left standing. Same
+ * reasoning as the statement cut taking the last separator: when the shape is
+ * ambiguous, discarding more is the only direction that cannot leak. Safe to be
+ * greedy here precisely because this runs AFTER the cut, where no statement is
+ * left for a match to reach across.
+ *
+ * The quote class and the key token deliberately mirror the `ER_DUP_ENTRY` limb
+ * `DIALECT_LEAK_PHRASINGS` carries in `@objectstack/types` — two files reading
+ * one dialect's one template should not disagree about its shape.
+ */
+const DUPLICATE_ENTRY = /(duplicate entry\s+)["'`][\s\S]*["'`](\s+for key\s+["'`][^"'`]+["'`])/gi;
+
+/**
+ * [#8823] The same template with its head already gone — what the statement cut
+ * leaves behind when the conflicting VALUE itself contained ` - `.
+ *
+ * Measured: `insert into … values ('2026 - Q3 plan') - Duplicate entry '2026 -
+ * Q3 plan' for key 't.label'` cuts at the separator INSIDE the value and logs
+ * `Q3 plan' for key 't.label'`. Same exposure, one shape further on, so it is
+ * closed here rather than left for the next reader to rediscover. Everything
+ * before the anchor is value residue by construction and is dropped whole.
+ */
+const DUPLICATE_ENTRY_TAIL = /["'`](\s+for key\s+["'`][^"'`]+["'`])/gi;
+
 /**
  * The first stack FRAME line (`    at …`). Everything above it is the header
  * that repeats `name: message` — and therefore repeats the statement.
@@ -126,8 +203,12 @@ export function redactBoundStatement(error: unknown): unknown {
 export function redactStatementFromMessage(message: string): string {
   if (!message || !looksLikeInternalErrorLeak(message)) return message;
   const cut = message.lastIndexOf(STATEMENT_SEPARATOR);
-  if (cut === -1) return message;
-  const diagnostic = message.slice(cut + STATEMENT_SEPARATOR.length).trim();
+  // No statement to cut — but a dialect may still have inlined a value in the
+  // diagnostic itself, and since #9030 taught the shared predicate this
+  // phrasing, a BARE `Duplicate entry …` now reaches this line instead of
+  // being turned away above.
+  if (cut === -1) return redactDiagnosticValues(message);
+  const diagnostic = redactDiagnosticValues(message.slice(cut + STATEMENT_SEPARATOR.length).trim());
   // A dump whose tail is empty still had its head removed: report the
   // redaction rather than an empty message, so the entry never reads as a
   // fault with no detail at all.
@@ -136,6 +217,62 @@ export function redactStatementFromMessage(message: string): string {
     : REDACTED_STATEMENT;
 }
 
+/**
+ * [#8823] Drop the caller values a dialect inlines into its OWN diagnostic,
+ * keeping every identifier around them.
+ *
+ * Runs on the tail the statement cut already produced, never on the whole
+ * message: a bound value can contain anything, this file's own template
+ * included, and matching before the cut would let a value in the STATEMENT
+ * steer what survives. After the cut there is nothing left but the database's
+ * words, so the templates below can be read literally.
+ *
+ * ⛔ Add a dialect's spelling here only once it has been measured off a THROWN
+ * error, never from a reading of the manual — the standing rule in this
+ * neighbourhood (`unique-violation.ts`), and the reason MySQL's other
+ * value-bearing families are not listed. Over-matching is the expensive
+ * direction: it deletes the diagnostic an operator came for.
+ */
+function redactDiagnosticValues(diagnostic: string): string {
+  const whole = lastMatch(DUPLICATE_ENTRY, diagnostic);
+  if (whole) {
+    // The template's head survived, so keep it and MySQL's own wording around
+    // it — only the value slot is replaced.
+    return diagnostic.slice(0, whole.index)
+      + whole[1] + REDACTED_VALUE + whole[2]
+      + diagnostic.slice(whole.index + whole[0].length);
+  }
+
+  const tail = lastMatch(DUPLICATE_ENTRY_TAIL, diagnostic);
+  if (tail) {
+    // Head gone: everything before the anchor is what is left of the value.
+    // The words are NOT reconstructed — an anchor is evidence about the value,
+    // not licence to assert which template printed it.
+    return REDACTED_VALUE + tail[1] + diagnostic.slice(tail.index + tail[0].length);
+  }
+
+  return diagnostic;
+}
+
+/**
+ * The LAST match of a sticky-free global pattern, or `undefined`.
+ *
+ * Last, not first, for the reason the statement cut takes the last separator:
+ * a value may itself contain the anchor, and the later match is the one the
+ * database printed. `lastIndex` is reset so the shared `RegExp` objects above
+ * carry no state between calls.
+ */
+function lastMatch(pattern: RegExp, text: string): RegExpExecArray | undefined {
+  pattern.lastIndex = 0;
+  let found: RegExpExecArray | undefined;
+  for (let m = pattern.exec(text); m !== null; m = pattern.exec(text)) {
+    found = m;
+    if (m[0].length === 0) break;
+  }
+  pattern.lastIndex = 0;
+  return found;
+}
+
 /**
  * Rebuild `stack` so its header carries the redacted message instead of the
  * statement, keeping every frame.