Skip to content

fix: reject malformed JSON containers in datadog log fast path - #299

Open
detail-app[bot] wants to merge 1 commit into
masterfrom
detail/bug-fix/fix-reject-malformed-json-containers-in-datadog-lo-977520
Open

detail-app[bot] wants to merge 1 commit into
masterfrom
detail/bug-fix/fix-reject-malformed-json-containers-in-datadog-lo-977520

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 16, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Bug

The per-record Datadog-log fast path (FieldWalker.valueEnd in src/signals/json_scan.zig, used by DatadogLog.parseRaw) matched container brackets and tracked string spans but skipped every other interior byte (else => {}), and validValueSpan accepted any {/[-leading span unconditionally. Bracket-balanced-but-structurally-malformed container values in unknown fields (e.g. [1,], {"k":}, {1:2}) were stored verbatim and never routed to the validating fallback — contradicting the scanner's stated strictness guarantee and the evalLogRecord contract that "semantics never depend on the fast path."

With a matching drop or mutating policy, the verdict diverged from the corrected (fail-open) path: a drop policy returned .drop (record discarded) and a mutating policy returned .replace (record re-serialized with the malformed span baked in), instead of .keep.

Fix

  • src/signals/json_scan.zig: replaced the container-interior byte skip in valueEnd with a per-depth position state machine that validates the JSON container grammar (commas, colons, string keys vs. values, closers, and scalar tokens via validValueSpan) as the span is scanned. Malformed containers now error Malformed and route to the validating/fail-open fallback. Whitespace and the documented raw control-byte (< 0x20) deviation are unchanged.
  • src/signals/datadog/log.zig: companion to the above — writeAnyValue previously swallowed iterator/materialization errors, leaving the JSON writer in the .the_beginning/.colon state where endObject/endArray are unreachable and would crash. Since the scanner fix now routes malformed records to this fallback, those errors are propagated so a malformed container aborts serialization cleanly and evalLogRecord fail-opens to .keep rather than panicking. The jsonStringify call site coerces the (unreachable for valid data) non-WriteFailed errors to WriteFailed to preserve the std.json.Stringify contract.

Testing

  • New regression tests inline in each file: scanner-level container-interior grammar validation and positive coverage; parseRaw rejection parity; and an evalLogRecord integration test asserting that matching drop and matching mutating policies both fail open to .keep for malformed unknown-field containers, with well-formed contrasts confirming the policy still runs on valid records.
  • Routine checks (zig build, zig build -Doptimize=ReleaseSafe, zig fmt --check, ziglint, zig build test) all pass — 523/524 tests (1 skipped, unchanged), up 4 from baseline, no regressions.
  • Verified the new scanner test fails on the reverted bug, and that the integration test crashes with panic: reached unreachable code in Stringify.endObject when the writeAnyValue companion fix is reverted — confirming both fixes are load-bearing.
  • End-to-end: the production evalLogRecord matrix benchmark builds and its verdict sanity checks pass. A temporary probe confirmed writeAnyValue now returns a clean error on malformed containers instead of panicking; it was reverted before commit. Perf cost from the added grammar work is ~2-4% on container-heavy records and flat/noise elsewhere, well under the documented 10-15% budget.
  • A live HTTP intake server was not driven end-to-end (requires the full frontend + policy stack); the production verdict path is exercised via the matrix benchmark since exec.zig's evalJsonRecord delegates 1:1 to evalLogRecord.

Fixes ENG-700


Automatic Fixes PRs can be configured here.

@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved at 2d1406b

Macroscope's review found this PR approvable — This is a localized Datadog JSON parsing bug fix that makes malformed containers fail open and preserves valid-record behavior. The added scanner logic and serialization safeguards are covered by targeted regression tests, with no schema, security, billing, or deployment changes.

You can add or adjust custom eligibility rules. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant