Remove github.com/DataDog/zstd_0 dependency - #512
Conversation
zstd_0 is an abandoned fork of an old zstd version with known memory safety vulnerabilities. MessageEncodingZstdPB now returns an unsupported-encoding error on encode/decode, matching the behavior already used by the no-cgo build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
| d, err = decoder.DecodeAll(body, nil) | ||
| } else { | ||
| d, err = zstd_0.Decompress(nil, body) | ||
| return fmt.Errorf("unsupported encoding: MessageEncodingZstdPB is no longer supported") |
![datadog-prod-us1-6[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The current Datadog Agent still selects MessageEncodingZstdPB for normal process payloads and drops messages when encoding fails. Adopting this library release without first migrating that caller would stop process and container submissions.
📊 Validated against 8 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit 1fcb846 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return nil, fmt.Errorf("unsupported encoding: MessageEncodingZstdPB is no longer supported") |
There was a problem hiding this comment.
Bumping the Datadog Agent to this release as planned would silently drop normal process and container payloads before submission, leaving process visibility incomplete.
Assertion details
- Input: A cgo Agent process or container payload passed through its current
api.EncodePayloadpath, which selectsMessageEncodingZstdPBfor every message type except connections and process events. - Expected:
Update the Agent payload encoder and fake-intake fixtures to a supported encoding, validate that encoding against intake, and only then adopt this library release. This coordinated cross-repository migration cannot be expressed as a safe line-bounded suggestion here without restoring the vulnerable dependency. - Actual:
EncodeMessagereturnsunsupported encoding: MessageEncodingZstdPB is no longer supportedwith a nil payload. The current Agent submitter logs this encoding error and skips the message.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
There was a problem hiding this comment.
No because the plan is to remove zstd_0 from the Agent.
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
There was a problem hiding this comment.
Overall lgtm, however, for the eventual agent side bump can you couple that with a change for our default encoding from MessageEncodingZstdPB to MessageEncodingZstd1xPB and updating the fake-intake fixtures?
- https://github.com/DataDog/datadog-agent/blob/1d86ce7580bb486ea2076294dcc6062716227901/pkg/process/util/api/payload.go#L39-L43
- https://github.com/DataDog/datadog-agent/blob/1d86ce7580bb486ea2076294dcc6062716227901/test/fakeintake/fixtures/process.go#L20
otherwise just a bump would lead to the unsupported encoding case for our payloads
Oh yeah, the plan is to remove the other calls to |
3 participants
What does this PR do?
Removes zstd_0 from this project.
Motivation
zstd_0 is an abandoned fork of an old zstd version with known memory safety vulnerabilities. MessageEncodingZstdPB now returns an unsupported-encoding error on encode/decode, matching the behavior already used by the no-cgo build.
Additional Notes
Once merged we need to bump the one found in the Datadog Agent to the latest version of this project.
Possible Drawbacks / Trade-offs
It will now error if anyone uses zstd_0.