l7: bound ClickHouse parser allocations by the captured payload size

[tmeaney-qumulo]

Problem

ParseClickhouse feeds a truncated, untrusted eBPF payload capture into ch-go's protocol decoder. ch-go's Reader.Str (via Buffer.Ensure) allocates make([]byte, n) from the decoded varint length prefix before validating n against the bytes actually available — it only checks n >= 0.

Against a well-behaved ClickHouse server that's harmless. Against a truncated capture, misaligned parses are routine — especially when the wire format is newer than the vendored ch-go v0.62.0 understands (we run ClickHouse 26.x) — and garbage bytes get read as a multi-gigabyte string length. The allocation fires even though the read then immediately fails; the recover() catches panics, not allocations.

Production impact

On our EKS cluster the agent OOM-crash-looped on every node hosting ClickHouse native-protocol clients, dying at any memory limit we tried (1Gi → 8Gi, up to 9 kills per pod, one 59-second life). Heap profile of a dying agent (Coroot's own Go heap profiling):

total                                                    (3.0 GB, 100%)
containers.(*Registry).handleEvents                      (2.2 GB,  72%)
  containers.(*Container).onL7Request
    ebpftracer/l7.ParseClickhouse
      ch-go/proto.(*Setting).Decode
        ch-go/proto.(*Reader).Str → StrAppend → StrRaw   ← make([]byte, n), n unvalidated

Nodes without CH clients ran flat at ~400Mi throughout. This is very likely the cause of #242 as well (undiagnosed OOM against a 9GB limit).

Fix

Replace the ch-go ClientInfo.DecodeAware / Setting.Decode calls with a field-identical bounded walk (same field order, same version feature gates, using ch-go's exported primitives and feature constants): every length-prefixed read is checked against the payload size before allocating. A claimed length beyond the capture is a misparse by construction.

• Well-formed frames parse exactly as before — the existing TestParseClickHouse payloads pass unchanged
• The new TestParseClickhouseGarbageLength demonstrates the bug: a 6-byte payload that previously allocated 4.3GB (measured via runtime.MemStats in the test) now allocates nothing

The deeper fix belongs in ch-go (StrRaw should validate the length against the source before Ensure), but bounding at this untrusted boundary is correct regardless of what ch-go does, and keeps the agent safe against any future protocol drift.

Happy to adjust the approach if you'd prefer a different shape (e.g. a max-string-length constant instead of the payload-size bound).

[tmeaney-qumulo]

@def Hey would you mind me asking why you labeled this AI Slop?
This is something I have been digging into this evening to try and solve a problem with using Coroot in our production EKS cluster, if this fix isn't correct then no worries, but we cannot continue to use Coroot if we cannot have L7 tracing turned on and will need to look for other solutions.

[def]

@tmeaney-qumulo sure, there are a few reasons:

• The amount of AI-generated code is quite large, so reviewing it would likely take longer than writing it from scratch.
• The code is clearly AI-generated.
• The issue you mentioned is most likely unrelated to this PR.

A much more helpful contribution, if you run into an issue with Coroot in your environment, is to open a detailed issue here. For example, in this case, attaching a memory profile would be far more valuable than submitting a large code change.

Thanks for understanding.

[tmeaney-qumulo]

Ok but in this case it's 144 lines (including tests proving the issue) over 2 files and the profile is in the description, but no problem, if coroot cannot work for our enterprise requirements (L7 tracing is table stakes), we can find an alternative.

Thanks

[def]

@tmeaney-qumulo, the issue is real and already known, we discussed it in #319. I'm happy to fix it, but I'd rather do it properly. A 144-line AI-generated patch, even with tests, doesn't give me much confidence that it won't introduce panics or other regressions.

Let's close this PR and track the work in an issue instead. That way we can implement and review the fix properly.

[def]

@tmeaney-qumulo BTW which version of the node-agent are you using?

[tmeaney-qumulo]

@def v1.35.0 for the node agent, and thank you for the links, those were useful.
One thing, they speak to misclassification of Clickhouse traffic whereas mine is actual Clickhouse traffic.

We are on Clickhouse v26.6, clickhouse-go v2.46.0 (ch-go v0.71.0) running a high throughout ingestion pipeline

[def]

@tmeaney-qumulo thanks for the details, it's really helpful. I'll try to reproduce the issue and get a fix out soon. If I end up with a similar implementation, I'll take a closer look at your changes. Let's keep this PR open for now.

[tmeaney-qumulo]

@def Thank you! If you need anything more from me let me know

[def]

@tmeaney-qumulo I managed to reproduce the issue. It turned out to be caused by the per-query settings. As a fix, I implemented a simple ClickHouse protocol parser manually instead of using ch-go, which gives us better control over the behavior. Thanks again for the detailed report!

[tmeaney-qumulo]

@def Amazing, thank you, I see you also released a new version of the agent, that's a quick turnaround time!