[CWS] Add declared hardening infos - #55994
Conversation
There was a problem hiding this comment.
AI review by Codex (OpenAI) - workflow run
patch is incorrect: the declared security context cannot reach system-probe through the existing remote workloadmeta transport, so the new field remains absent in deployed Agents.
| container, err := r.wmeta.GetContainer(string(id)) | ||
| if err != nil || container == nil || container.SecurityContext == nil { |
There was a problem hiding this comment.
[P1] Transport SecurityContext to system-probe before resolving it
System-probe uses workloadmeta.Remote, but Container.SecurityContext is marked proto:"ignore" in comp/core/workloadmeta/def/types.go and is omitted by the workloadmeta protobuf conversions. Consequently, even when the core Agent's kubelet collector populates this field, system-probe receives a container with a nil security context and this resolver always returns nil. Add the field to the remote workloadmeta transport and verify the deployed pipeline so Declared can actually be populated.
|
🎯 Code Coverage (details) 🔗 Commit SHA: 26bdd40 | Docs | View more details | Give us feedback! |
Files inventory check summaryFile checks results against ancestor efcfce51: Results for datadog-agent_7.85.0~devel.git.155.26bdd40.pipeline.136688439-1_amd64.deb:No change detected Results for datadog-iot-agent_7.85.0~devel.git.155.26bdd40.pipeline.136688439-1_amd64.deb:No change detected |
Static quality checks❌ Please find below the results from static quality gates Error
Gate failure full details
Static quality gate failures prevent this PR from merging! Successful checksInfo
3 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: efcfce5 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_idle_all_features | memory utilization | +0.55 | [+0.51, +0.58] | 1 | Logs bounds checks dashboard |
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_memory | memory utilization | +0.52 | [+0.30, +0.74] | 1 | Logs |
| ➖ | quality_gate_security_idle | memory utilization | +0.42 | [+0.38, +0.46] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.41 | [+0.19, +0.64] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | +0.25 | [+0.17, +0.33] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | +0.23 | [+0.18, +0.27] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.24 | [-0.29, -0.20] | 1 | Logs bounds checks dashboard |
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_cpu | % cpu utilization | -0.40 | [-0.65, -0.15] | 1 | Logs |
| ➖ | quality_gate_private_action_runner | memory utilization | -0.96 | [-1.09, -0.83] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -2.38 | [-3.23, -1.53] | 1 | Logs bounds checks dashboard |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 ≤ 5 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 177.24MiB ≤ 179MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 746.88KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 4 ≤ 5 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 525.85MiB ≤ 537MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.14MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 20 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 210.56MiB ≤ 228MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 263.43MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 392.77 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 20 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 418.62MiB ≤ 455MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.94GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 72.39MiB ≤ 75MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 31.52 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 326.71MiB ≤ 355MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 62.63 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 299.87MiB ≤ 335MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 25.12 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 315.17MiB ≤ 345MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
CI Pass/Fail Decision
✅ Passed. All Quality Gates passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # go.sum
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # comp/core/tagger/fx-remote/go.sum # comp/core/tagger/impl-remote/go.sum # comp/logs-library/go.sum # comp/otelcol/collector-contrib/impl/go.sum # comp/otelcol/ddflareextension/impl/go.sum # comp/otelcol/logsagentpipeline/go.sum # comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.sum # comp/otelcol/otlp/components/exporter/datadogexporter/go.sum # comp/otelcol/otlp/components/exporter/logsagentexporter/go.sum # comp/otelcol/otlp/components/exporter/serializerexporter/go.sum # comp/otelcol/otlp/components/processor/infraattributesprocessor/go.sum # go.sum # internal/qbranch/anomalydetection-testbench/go.sum # pkg/config/remote/go.sum # pkg/process/util/api/go.sum # pkg/proto/go.sum # pkg/serializer/go.sum # pkg/trace/otel/go.sum # pkg/util/grpc/go.sum # test/e2e-framework/go.sum # test/fakeintake/go.sum # test/new-e2e/go.sum # test/otel/go.sum
Co-authored-by: Cursor <cursoragent@cursor.com>
The workspace-wide dda inv tidy after merging main churned 24 nested modules to propagate the agent-payload pseudo-version. That churn is unrelated to this PR's intent: our only genuine dep change is the agent-payload pin in the root go.mod. Restoring those nested files to their origin/main state. Note: this will fail the check-mod-tidy CI gate (which runs 'go work sync' + 'git diff --exit-code **/go.mod **/go.sum'). Accepted for now; will be addressed either by re-adding the sync (if we keep the agent-payload pseudo-version) or by cutting a real agent-payload release and re-pinning to it. Co-authored-by: Cursor <cursoragent@cursor.com>
f17c21f to
2c36c3e
Compare
There was a problem hiding this comment.
AI review by Codex (OpenAI) - workflow run
The patch is incorrect: a transient workloadmeta miss can permanently omit declared hardening information from an active profile. The added unit tests do not exercise this startup ordering or verify delivery through fakeintake.
| Start: eventTime, | ||
| End: eventTime, | ||
| } | ||
| secprof.Declared = m.resolveDeclared(event.ProcessContext.Process.ContainerContext.ContainerID) |
There was a problem hiding this comment.
[P2] Retry unresolved posture after profile creation
The remote tagger and remote workloadmeta populate independently, so image tags can be available before workloadmeta contains the container's security context. If this lookup returns nil, getOrCreateProfile subsequently returns the cached profile without resolving it again; the storage backfill only runs when loading a profile. Consequently, a transient startup or container-discovery delay leaves Declared absent for the profile's lifetime even after the data arrives. Retry resolution while the cached profile's Declared is nil, and cover delayed workloadmeta availability in a test.
| v := *src | ||
| return &v |
There was a problem hiding this comment.
nit: can use new(*src) instead of de-ref into a variable and reference again.
| v := *src | ||
| return &v |
There was a problem hiding this comment.
similar nit with new(*src)
| // copyBoolPtr returns a fresh *bool with src's value, or nil if src is nil, | ||
| // so wire-side callers never alias workloadmeta cache storage. | ||
| func copyBoolPtr(src *bool) *bool { |
There was a problem hiding this comment.
not a big deal but this defensive measure doesn't seem that necessary. This is used for proto streaming - not like it'll be passed around different components. Similar thought with the other copyBoolPtr .
| // copyBoolPtr returns a fresh *bool with src's value, or nil if src is nil. | ||
| func copyBoolPtr(src *bool) *bool { | ||
| if src == nil { | ||
| return nil | ||
| } | ||
| v := *src | ||
| return &v | ||
| } |
There was a problem hiding this comment.
similar nit and thoughts on this method in general.
| github.com/CycloneDX/cyclonedx-go v0.12.0 | ||
| github.com/DATA-DOG/go-sqlmock v1.5.2 | ||
| github.com/DataDog/agent-payload/v5 v5.0.209 | ||
| github.com/DataDog/agent-payload/v5 v5.0.211-0.20260910152323-44bb6d8404c1 |
There was a problem hiding this comment.
Already mentioned in the PR description, but another reminder to pin to a release version instead of a commit hash before merging.
What
Enrich CWS security profiles with the declared Kubernetes hardening posture (privileged, seccomp profile, capabilities add/drop).
How
pkg/proto/datadog/workloadmeta: addContainerSecurityContext,Capabilities,SeccompProfilemessages and wire them throughContainer; extend proto ↔ struct converters incomp/core/workloadmeta/proto. Without this,workloadmeta.Remote(used by system-probe) silently strips the field on the gRPC wire.pkg/security/resolvers/securitycontext: newWorkloadmetaResolverthat resolves a container'sDeclaredposture from workloadmeta.pkg/security/security_profile: attachDeclaredon dump creation; NewHardeningDeclaredwire message viaagent-payloadbump.TODO
New release on
agent-payloadbefore merging: DataDog/agent-payload#519