fix: escape all control characters in diagnostics JSON output

[strawgate]

Fixed an issue in the diagnostics JSON escaping where ASCII control characters (0x00-0x1F) were not being escaped, leading to invalid JSON output. Added unit and integration tests to ensure all control characters are properly handled using either named escapes or the \uXXXX format.

Fixes #381

---

PR created automatically by Jules for task 13652391228897795876 started by @strawgate

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The esc() helper in crates/logfwd-io/src/diagnostics.rs was updated to escape all ASCII control characters \x00–\x1f as JSON Unicode escapes (\uXXXX), while preserving existing named escapes (\n, \r, \t, \", \\). Unit tests were added to verify escaping of specific control bytes and an integration test asserts a pipeline name containing a control character is returned as valid JSON with the escaped sequence.

Possibly related PRs

• fix: Cleanup pass — 7 audit findings #136 — Implements JSON escaping of ASCII control characters (0x00–0x1F) as \uXXXX sequences, matching the escaping behavior changed here.
• refactor: decommission shadow JSON parser in otlp.rs (#357) #370 — Adjusts OTLP/timestamp-related tests in otlp.rs; related to this repo's small test change in crates/logfwd-core/src/otlp.rs.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❓ Inconclusive	Minor Clippy warning fixes in logfwd-core (test assertions) are incidental to the main objective. Changes to structural.rs and otlp.rs are small refactoring/cleanup unrelated to issue #381 but address CI lint compliance.	Consider clarifying whether incidental Clippy fixes in unrelated files should be bundled. If they're CI blockers, document this context; otherwise isolate them in a separate PR.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All coding requirements from issue #381 are met: control characters 0x00-0x1F are escaped, named escapes preserved, \uXXXX format used, tests added covering control characters.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/logfwd-io/src/diagnostics.rs`:
- Around line 657-660: The match arm that handles control characters uses
write!(out, "\\u{:04x}", c as u32).unwrap(); — replace the bare .unwrap() with
.expect(...) to document why this is safe (e.g., .expect("writing to String is
infallible") ) so the call remains explicit and complies with the guideline;
locate the write! invocation in the control-character branch in diagnostics.rs
and change only the unwrap to an expect message.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bef6649a-9703-40ed-b712-fa9beb242006

📥 Commits

Reviewing files that changed from the base of the PR and between 6b97b86 and 0bf4324.

📒 Files selected for processing (1)

• crates/logfwd-io/src/diagnostics.rs

[github-actions]

Addressed the open review feedback with a surgical code change.

• Updated crates/logfwd-io/src/diagnostics.rs to replace the remaining production-path .unwrap() in control-character escaping with .expect("write to String is infallible").
• Resolved the corresponding review thread after pushing the fix.

Validation

• cargo fmt --check
• RUSTC_WRAPPER= cargo clippy -p logfwd-io -- -D warnings
• RUSTC_WRAPPER= cargo test -p logfwd-io (78 passed)

Also attempted full just ci, but just is not installed in this runner, so equivalent required checks were run directly.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:fix: escape all control characters in diagnostics JSON output #397 (pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)

---

What is this? | From workflow: AI: Address PR Review Feedback

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/logfwd-io/src/diagnostics.rs`:
- Around line 1004-1010: The test test_esc_control_chars should be expanded to
iterate over the full control byte range 0x00..=0x1F and assert esc produces the
exact expected escape for each value; modify the test to loop bytes b in
0u8..=0x1F, convert to a single-byte string and call esc, then compute the
expected output (use the known named escapes for 0x09->"\t", 0x0A->"\n",
0x0D->"\r" and for all other control bytes expect the unicode escape form
"\\u00XX" with uppercase hex) and assert_eq!(esc(...), expected). Keep the test
name test_esc_control_chars and update assertions to cover every control byte so
regressions for unlisted bytes are caught.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8159ea18-c750-4ca5-93f6-0302ec0e6a9f

📥 Commits

Reviewing files that changed from the base of the PR and between 44c67d2 and 09bd063.

📒 Files selected for processing (1)

• crates/logfwd-io/src/diagnostics.rs

[github-actions]

Addressed the remaining unresolved review feedback in crates/logfwd-io/src/diagnostics.rs.

• Updated test_esc_control_chars to iterate exhaustively over 0x00..=0x1F and assert expected escaping for each byte (\\t, \\n, \\r, or \\u00xx), with per-byte failure messages.
• Resolved thread PRRT_kwDORzg8fM53-DMp after pushing the fix.

Validation run

• RUSTC_WRAPPER= cargo test -p logfwd-io test_esc_control_chars ✅
• RUSTC_WRAPPER= cargo test -p logfwd-io test_pipelines_endpoint_escaping ✅
• RUSTC_WRAPPER= cargo fmt --all --check ✅
• RUSTC_WRAPPER= cargo clippy -p logfwd-io -- -D warnings ✅

just ci could not be run in this environment because just is not installed.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

• pr:fix: escape all control characters in diagnostics JSON output #397 (pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)

---

What is this? | From workflow: AI: Address PR Review Feedback

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/logfwd-io/src/diagnostics.rs`:
- Around line 1008-1015: The loop currently calls format! and .to_string() each
iteration; instead allocate a reusable String before the loop and build expected
into it with push/push_str and write! for the fallback: declare let mut expected
= String::with_capacity(8) outside the loop, inside the loop do
expected.clear(); expected.push('x'); match ch { '\t' =>
expected.push_str("\\t"), '\n' => expected.push_str("\\n"), '\r' =>
expected.push_str("\\r"), _ => write!(&mut expected, "\\u{:04x}", b).unwrap() };
expected.push('y'); and use that expected (and remove the per-iteration
format!/to_string() calls referring to input, expected_mid, expected, ch and b).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1cba2c9a-f2a4-49f5-a6d2-4e9fff79f1f8

📥 Commits

Reviewing files that changed from the base of the PR and between 09bd063 and 0088021.

📒 Files selected for processing (1)

• crates/logfwd-io/src/diagnostics.rs