fix(observability): reject malformed JSON-looking alert evidence - #480
Conversation
|
Warning Review limit reached
Next review available in: 16 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (/^\s*[\[{]/.test(line)) { | ||
| console.error(`Malformed JSON in observability log line ${index + 1}.`); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🟡 Behavior change missing from changelog
The alert script now exits with an error on malformed JSON-looking lines instead of dropping them, a behavior change, yet CHANGELOG.md's ## Unreleased section gets no entry. CONTRIBUTING.md and CLAUDE.md require a changelog entry for every behavior change.
Prompt for agents
CONTRIBUTING.md and CLAUDE.md require that CHANGELOG.md's ## Unreleased section be updated with every behavior change. This PR changes the behavior of scripts/evaluate-observability-alerts.mjs so that lines beginning with { or [ (after optional whitespace) that fail to parse now cause a fail-closed exit(1) with a 'Malformed JSON' diagnostic, rather than being silently tolerated. Add a corresponding entry (in Korean, matching the existing changelog style) to the ## Unreleased section of CHANGELOG.md describing this fail-closed hardening.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (/^\s*[\[{]/.test(line)) { | ||
| console.error(`Malformed JSON in observability log line ${index + 1}.`); | ||
| process.exit(1); | ||
| } | ||
| // Wrangler tail can include non-JSON diagnostic lines. Preserve that noise tolerance. |
There was a problem hiding this comment.
🔍 Bracket-prefixed diagnostic lines now fail closed
The new guard exits with an error on any unparseable line starting with { or [. The retained comment states diagnostic noise from wrangler tail must stay tolerated, yet some diagnostic lines start with [ (e.g. [wrangler:inf] ..., [mf:inf] ...). If any such line reaches the log file, a valid run fails closed. Confirm the redirected tail output never contains bracket-prefixed diagnostics.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Purpose
Test-first repair of an observability-evidence integrity gap. Protected source intentionally tolerates plain Wrangler diagnostic noise, but its broad per-line
catchalso silently drops malformed JSON-looking NDJSON. A truncated/exchangefailure record can therefore disappear from alert aggregation instead of failing closed.RED → candidate
3d2425fc908ef221d55be958a66d26fb2ea8b94ad35baf78ed34853821487cd11eba8e89477c55cb32608420133, job97117236832, exact checkoutd35baf78...;release testsfailed exactly because the new realistic CLI test expected exit 1 but protected production returned exit 0 after silently dropping the truncated JSON-looking/exchangerecordb734a2a208fadd57821b64c0428963139318d15dscripts/evaluate-observability-alerts.mjsonly, +4/-0The production repair keeps the existing tolerance for plain Wrangler diagnostic noise, but a failed parse for a line that begins as structured JSON (
{or[, allowing leading whitespace) now exits fail-closed with line-specificMalformed JSONdiagnostics. Existing fatal UTF-8, duplicate decoded-key, timestamp/status/latency validation and alert aggregation remain unchanged.An intermediate candidate moved the entire aggregation block outside the parser
try/catch; it was deliberately tightened back to the four-line causal repair so the final candidate has the smallest blast radius.Exact-head evidence boundary
Fresh candidate application CI
32608613022, reviewer-ci32608612902, and central Security Scan32608612995are non-passing until terminal-success on unchangedb734a2a.... Keep Draft until all applicable exact-head gates pass and zero valid unresolved findings remain.Authority boundary
This hardens local retained observability evidence only. It does not manufacture production KPI, deployment, customer, release, legal/IP, or acquisition-ready evidence. No central
.githubor other dedicated-writer source is modified.