in_opentelemetry: validate log protobuf depth before unpacking - #12449
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe OpenTelemetry input plugin now validates protobuf payloads before unpacking. The validator checks wire boundaries and nested-message depth. Unit and integration tests cover malformed payloads, deep nesting, rejection, recovery, and daemon survival. ChangesOpenTelemetry protobuf validation
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant OpenTelemetryClient
participant FluentBit
participant protobuf_logs_unpack
participant opentelemetry_protobuf_validate
participant protobuf_unpack
OpenTelemetryClient->>FluentBit: Send OpenTelemetry protobuf request
FluentBit->>protobuf_logs_unpack: Process request payload
protobuf_logs_unpack->>opentelemetry_protobuf_validate: Validate payload
opentelemetry_protobuf_validate-->>protobuf_logs_unpack: Return validation result
protobuf_logs_unpack->>protobuf_unpack: Unpack valid payload
protobuf_logs_unpack-->>FluentBit: Reject invalid payload or return unpacked logs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Validate OTLP log request nesting before protobuf-c recursively unpacks or allocates the message tree. The schema-aware preflight uses an explicit stack, treats strings/bytes/unknown fields as opaque, and rejects malformed lengths and varints.
Validation: The protobuf CTest target passed, including 99/100/101 message boundaries and malformed-wire cases, and passed focused Valgrind checks. All three log ingestion/recovery cases passed normally and under strict Valgrind. The existing large-protobuf-log scenario also passed both ways.
Focused integration commands (run against the combined validation build containing these fixes):
Relevant internal CTest targets and focused Valgrind checks also passed. Full PR-range commit-prefix lint, DCO and whitespace checks passed against fetched master. Code and regression tests are separate commits.
Compatibility: Requests exceeding 100 schema messages in depth are rejected; the count includes protocol wrappers. Public interfaces and valid shallow requests are unchanged. Metrics/traces/profiles preflight fixes are maintained in their respective library PRs; this patch and its integration tests cover logs.
Summary by CodeRabbit
Bug Fixes
Tests