Skip to content

Commit 65a1a1c

Browse files
mjerrisclaude
andcommitted
fix(logging): name stripControlChars' param for the reference, and TEST the wiring
typescript is the ONE port that already had this right: `Logger.log` calls `stripControlChars(serialized)` before merging (Logger.ts:339), so the scrub was genuinely on the emission path. rust, cpp and java each shipped the same function with ZERO call sites — public, correct, and protecting nothing. This commit does not change ts behaviour; it closes the two gaps around it. 1. PARAM NAME. The reference's public contract is `strip_control_chars(event_dict)` (logging_config.py:33); this port recorded `data`. Renamed the parameter to `eventDict` — the enumerator's camelCase->snake_case fold turns that into `event_dict`, so the recorded signature now matches the oracle with NO rename-table entry needed. Verified by regenerating port_signatures.json: signalwire.core.logging_config [('event_dict', 'any')] The alternative was a free-function param-rename table, which does not exist in enumerate-signatures.ts (its rename tables are keyed by CLASS). Renaming in the port source is the smaller, more honest change: nothing about the reference is being reconciled away, the port simply says what it means. All three call sites are positional, so nothing else moved. 2. NO TEST COVERED THE WIRING. The protection was real but unguarded — the exact state in which rust/cpp/java's scrub rotted into a no-op without any gate noticing. The new test reads what the logger ACTUALLY emitted (via the existing console spies) rather than calling stripControlChars directly, so it fails when the wiring is removed. Verified by deleting the call from Logger.log — RED: × strips control characters from emitted log data Tests 1 failed | 48 passed (49) A helper-only test passes against that same break. The second test asserts tab/newline/CR SURVIVE: a scrub that ate them would satisfy "no control chars" while mangling every multi-line message. Verified: run-tests.sh -> exit 0, 136 files / 2884 tests (2882 before; +2 new). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KKwgPbehDoAMdG3hPL79oi
1 parent 740fb79 commit 65a1a1c

3 files changed

Lines changed: 36 additions & 6 deletions

File tree

port_signatures.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5440,7 +5440,7 @@
54405440
"strip_control_chars": {
54415441
"params": [
54425442
{
5443-
"name": "data",
5443+
"name": "event_dict",
54445444
"type": "any",
54455445
"required": true
54465446
}

src/Logger.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,13 +231,16 @@ const CONTROL_CHAR_RE = /[\x00-\x08\x0b\x0c\x0e-\x1f\x7f-\x9f]/g;
231231
* log injection attacks. Mirrors Python SDK's `strip_control_chars` structlog
232232
* processor. Processes nested objects and arrays recursively.
233233
*
234-
* @param data - The record whose string values should be sanitized.
235-
* @returns A shallow copy of `data` with control characters removed from strings.
234+
* @param eventDict - The log event record whose string values should be sanitized.
235+
* Named for the reference's `event_dict` (`logging_config.py:33`) — the enumerator
236+
* camelCase->snake_case folds this to `event_dict`, so the recorded signature matches
237+
* the oracle without needing a rename-table entry.
238+
* @returns A shallow copy of `eventDict` with control characters removed from strings.
236239
*/
237-
export function stripControlChars<T extends Record<string, unknown>>(data: T): T {
240+
export function stripControlChars<T extends Record<string, unknown>>(eventDict: T): T {
238241
const result = {} as T;
239-
for (const key of Object.keys(data) as (keyof T)[]) {
240-
const value = data[key];
242+
for (const key of Object.keys(eventDict) as (keyof T)[]) {
243+
const value = eventDict[key];
241244
if (typeof value === 'string') {
242245
result[key] = value.replace(CONTROL_CHAR_RE, '') as T[keyof T];
243246
} else if (Array.isArray(value)) {

tests/Logger.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
setGlobalLogStream,
1010
resetLoggingConfiguration,
1111
getExecutionMode,
12+
stripControlChars,
1213
} from '../src/Logger.js';
1314

1415
describe('Logger', () => {
@@ -472,4 +473,30 @@ describe('Logger', () => {
472473
expect(widensToDebug(log)).toBe(false);
473474
});
474475
});
476+
// --- control-char scrub: WIRING ------------------------------------------
477+
//
478+
// typescript is the ONE port where the scrub was already on the emission path
479+
// (Logger.log calls stripControlChars before merging) — but nothing tested
480+
// that, so the protection was real yet unguarded. rust/cpp/java each shipped
481+
// the same function with ZERO call sites; the only assertion that can tell the
482+
// difference is one that reads what the logger ACTUALLY emitted.
483+
describe('control-character scrub on the emission path', () => {
484+
it('strips control characters from emitted log data', () => {
485+
getLogger('inject.test').info('msg', { field: 'user\u0000said\u001b[31mRED\u0007' });
486+
487+
expect(spyInfo).toHaveBeenCalled();
488+
const emitted = JSON.stringify(spyInfo.mock.calls);
489+
expect(emitted).not.toContain('\u0000');
490+
expect(emitted).not.toContain('\u001b');
491+
expect(emitted).not.toContain('\u0007');
492+
expect(emitted).toContain('usersaid[31mRED');
493+
});
494+
495+
it('keeps tab/newline/carriage-return, which are legal in a log line', () => {
496+
// A scrub that ate these would satisfy "no control chars" above while
497+
// mangling every multi-line message.
498+
const legal = 'line1\tcol\nline2\r end';
499+
expect(stripControlChars({ field: legal })).toEqual({ field: legal });
500+
});
501+
});
475502
});

0 commit comments

Comments
 (0)