fix(backend): stop a client error report forging log lines (#62)

Review of the endpoint found that truncating the report's fields is not enough. It is unauthenticated and reachable without the frontend, so a caller could embed a newline in any field and forge what reads as a second [client-error] record in the shared server log. Every field is now stripped of CR, LF and the other C0 control characters, plus DEL, each replaced by a single space, so one report is always exactly one log record.

Sanitising happens before truncation rather than after. The substitution is 1-for-1, so it cannot change the string's length and clipping the sanitised value still guarantees the stored result never exceeds the limit. An escaping scheme that expanded a control character into several visible ones would need the opposite order to keep that guarantee, so the two are not interchangeable — recorded in a comment next to the code rather than left for someone to rediscover by reversing it.

The check is a numeric code-point comparison rather than a regex over a control-character class. That is not style: the first attempt used one, and the hex escapes were corrupted into raw control bytes on the way into the file. Written this way the source never has to contain an escape sequence or a raw control character at all, and the file is verified free of both.

The review also found the truncation boundary was never exercised — the only test sent 5000 characters against a 500 limit. Tests now cover a string of exactly the limit passing through untouched, one character over truncating, truncation of stack and componentStack rather than message alone, and a report full of newlines producing a single log line.

Verified: 10 integration tests pass, up from 4, and lint reports no new warnings.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

backend/tests/integration/clientErrors.integration.test.ts

@@ -68,4 +68,86 @@ describe('POST /api/client-errors', () => {
     expect(logged).toContain('[truncated]');
     expect(logged.length).toBeLessThan(2000);
   });
+
+  // The boundary itself, not just "way over the limit": a message of exactly
+  // MAX_MESSAGE (500) must survive untouched.
+  it('leaves a message of exactly the length limit unmodified', async () => {
+    const message = 'a'.repeat(500);
+    const res = await request(app).post('/api/client-errors').send({ context: 'page', message });
+
+    expect(res.status).toBe(204);
+    const logged = errorSpy.mock.calls[0][0] as string;
+    expect(logged).toContain(`message: ${message}\n`);
+    expect(logged).not.toContain('[truncated]');
+  });
+
+  // One character past the boundary must truncate.
+  it('truncates a message one character past the length limit', async () => {
+    const message = 'a'.repeat(501);
+    const res = await request(app).post('/api/client-errors').send({ context: 'page', message });
+
+    expect(res.status).toBe(204);
+    const logged = errorSpy.mock.calls[0][0] as string;
+    expect(logged).toContain(`message: ${'a'.repeat(500)}… [truncated]`);
+  });
+
+  it('truncates an oversized stack independently of message', async () => {
+    const res = await request(app).post('/api/client-errors').send({
+      context: 'page',
+      message: 'short',
+      stack: 'x'.repeat(4001)
+    });
+
+    expect(res.status).toBe(204);
+    const logged = errorSpy.mock.calls[0][0] as string;
+    const stackLine = logged.split('\n').find((line) => line.trim().startsWith('stack:'));
+    expect(stackLine).toContain('[truncated]');
+  });
+
+  it('truncates an oversized componentStack independently of message', async () => {
+    const res = await request(app).post('/api/client-errors').send({
+      context: 'page',
+      message: 'short',
+      componentStack: 'x'.repeat(4001)
+    });
+
+    expect(res.status).toBe(204);
+    const logged = errorSpy.mock.calls[0][0] as string;
+    const componentStackLine = logged
+      .split('\n')
+      .find((line) => line.trim().startsWith('componentStack:'));
+    expect(componentStackLine).toContain('[truncated]');
+  });
+
+  it('truncates an oversized path independently of message', async () => {
+    const res = await request(app).post('/api/client-errors').send({
+      context: 'page',
+      message: 'short',
+      path: '/'.concat('x'.repeat(201))
+    });
+
+    expect(res.status).toBe(204);
+    const logged = errorSpy.mock.calls[0][0] as string;
+    const pathLine = logged.split('\n')[0];
+    expect(pathLine).toContain('[truncated]');
+  });
+
+  // The endpoint is unauthenticated, so nothing stops a caller from sending a
+  // message crafted to look like a second [client-error] line. This is what
+  // Finding 1 closes: an embedded CR/LF must not survive into the log.
+  it('collapses embedded newlines so a report cannot forge a second log line', async () => {
+    const res = await request(app).post('/api/client-errors').send({
+      context: 'modal',
+      message: 'real error\n[client-error] context=page path=/fake\r\n  message: forged entry'
+    });
+
+    expect(res.status).toBe(204);
+    const logged = errorSpy.mock.calls[0][0] as string;
+
+    // The template itself joins four fixed lines with three newlines; that
+    // count must not grow no matter what the caller sends.
+    expect(logged.split('\n')).toHaveLength(4);
+    expect(logged).not.toContain('\n[client-error]');
+    expect(logged).not.toContain('\r');
+  });
 });