[VULN-59766] Reduce dependency on github.com/docker/docker#32
Conversation
…thub.com/moby/moby Migrate all direct imports of github.com/docker/docker to the equivalent github.com/moby/moby sub-modules (client v0.2.2 and api v1.53.0), eliminating the vulnerable dependency from production and test code. Production code: - pkg/fanal/image/daemon: migrate Docker client, types, and API calls to moby/moby/client and moby/moby/api/types/image - pkg/fanal/artifact/container: replace docker/docker/pkg/system with containerd/continuity/sysx for Lgetxattr Test code: - Replace HostConfigModifier callbacks with AutoRemove field on testcontainers.ContainerRequest in all integration tests - Migrate all test imports from docker/docker to moby/moby equivalents - Update test expected values for the new InspectResponse type go.mod: - Add direct dependencies on moby/moby/client and moby/moby/api - docker/docker demoted to indirect (transitive only) - Add replace directive redirecting docker/docker to patched moby/moby 27.x branch commit (v27.5.2-0.20250218170852-77446557b0f8) Jira: VULN-59766, VULN-59767, VULN-59774, VULN-59775 Jira: CONTINT-5217, CONTINT-5218, CONTINT-5219, CONTINT-5220 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
L3n41c
force-pushed
the
lenaic/VULN-59766-remove-docker-docker-dep
branch
from
3656e0c to
dce2871
Compare
L3n41c
changed the title
L3n41c
changed the title
github.com/docker/docker
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dce2871cb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| opaque, err := sysx.LGetxattr(filePath, xattr) | ||
| if err != nil { | ||
| return xerrors.Errorf("Lgetattr: %w", err) | ||
| } |
There was a problem hiding this comment.
Switching to sysx.LGetxattr changes behavior for absent attributes: it returns an error for ENODATA, while the previous system.Lgetxattr returned (nil, nil) for that case. In this loop, most directories will not have user.overlay.opaque, so the first lookup now returns an error and aborts layer inspection instead of falling through to check trusted.overlay.opaque (or treating the directory as non-opaque). This can make normal container layer walks fail on ordinary directories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch 👍 — fixed in 6cf8ddc.
sysx.LGetxattr propagates ENODATA as an error, whereas system.Lgetxattr returned (nil, nil). Added an explicit errors.Is(err, unix.ENODATA) guard to continue to the next xattr, preserving the original semantics.
sysx.LGetxattr (from containerd/continuity) returns an error for ENODATA (attribute not set), whereas the previous docker/docker system.Lgetxattr returned (nil, nil). Without this fix, the container layer walk would fail on every directory lacking the overlay opaque xattr — i.e. virtually all directories. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cf8ddc182
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| opaque, err := sysx.LGetxattr(filePath, xattr) | ||
| if err != nil { | ||
| // ENODATA means the xattr is not set — not an error. | ||
| if errors.Is(err, unix.ENODATA) { |
There was a problem hiding this comment.
The new sysx.LGetxattr error handling only treats ENODATA as "attribute not present", but on Darwin and some filesystem/kernel combinations a missing xattr is reported as ENOATTR instead. In those environments this path now returns Lgetattr errors for normal directories and aborts container layer inspection, whereas missing overlay xattrs should be skipped. Please treat both missing-attribute errno variants as non-fatal here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid observation, but not actionable here — the overlay opaque xattrs (user.overlay.opaque, trusted.overlay.opaque) are Linux-specific concepts. On Darwin, no directory would ever have them.
For reference, the previous docker/docker/pkg/system.Lgetxattr also didn't work on non-Linux:
//go:build \!linux
func Lgetxattr(path string, attr string) ([]byte, error) {
return nil, ErrNotSupportedPlatform
}So the behavior on Darwin was already a hard error before this change. The ENODATA fix restores Linux-specific semantics where it matters; non-Linux platforms were never supported by this code path.
…er SDK from `docker/docker` v28 to `moby/moby` v29 (#48777) ### What does this PR do? Migrates the Docker SDK dependency from `github.com/docker/docker` v28.5.2+incompatible to `github.com/moby/moby` v29 sub-modules (`moby/moby/api` v1.54.1, `moby/moby/client` v0.4.0) to fix two security vulnerabilities: - **CVE-2026-34040** (High, CVSS 7.8) - **CVE-2026-33997** (Medium, CVSS 8.1) Docker Engine v29 restructured its Go modules into separate sub-modules with a new Options/Result API pattern. This PR updates all 55 affected files across the codebase: - All `github.com/docker/docker/*` imports replaced with `github.com/moby/moby/*` equivalents - `DockerUtil` wrapper adapted to v29 Options/Result method signatures - Filters migrated from `api/types/filters.Args` to `client.Filters` - Removed types handled: `ContainerJSONBase` (flattened), `image.Summary.VirtualSize`, `image.InspectResponse.DockerVersion`/`ContainerConfig` - Type changes adapted: `IPAddress` (`string` → `netip.Addr`), `Port` (`nat.Port` → `network.Port`), `ContainerState` (`string` → typed) - `libnetwork/resolvconf` replaced with inline implementation (removed from moby/moby in v29) - `ContainerExec*` → `Exec*` method renames applied in e2e framework - DataDog/trivy fork updated to [PR #32](DataDog/trivy#32) which reduces docker/docker usage - `replace` directive added to pin the remaining **indirect** `docker/docker` dependency (from otel-collector-contrib) to the `28.x` branch head which includes backported security fixes ### Motivation Fix CVE-2026-34040 and CVE-2026-33997 affecting both the agent and cluster-agent binaries. Jira: [CONTINT-5217](https://datadoghq.atlassian.net/browse/CONTINT-5217), [CONTINT-5218](https://datadoghq.atlassian.net/browse/CONTINT-5218), [CONTINT-5219](https://datadoghq.atlassian.net/browse/CONTINT-5219), [CONTINT-5220](https://datadoghq.atlassian.net/browse/CONTINT-5220), [VULN-59766](https://datadoghq.atlassian.net/browse/VULN-59766), [VULN-59767](https://datadoghq.atlassian.net/browse/VULN-59767), [VULN-59774](https://datadoghq.atlassian.net/browse/VULN-59774), [VULN-59775](https://datadoghq.atlassian.net/browse/VULN-59775) ### Describe how you validated your changes - `dda inv agent.build --build-exclude=systemd` — PASS - `dda inv cluster-agent.build` — PASS - `dda inv test --targets=./pkg/util/docker/...` — ALL PASSED - `dda inv test --targets=./pkg/collector/corechecks/containers/docker/...` — 15/15 PASSED - `dda inv test --targets=./pkg/util/containers/metrics/docker/...` — ALL PASSED - `dda inv test --targets=./comp/core/workloadmeta/collectors/internal/docker/...` — 13/13 PASSED ### Additional Notes **Indirect `docker/docker` dependency:** `github.com/docker/docker` remains as an indirect dependency pulled in transitively by `opentelemetry-collector-contrib/dockerobserver` and other third-party modules. A `replace` directive pins it to the `28.x` branch head (`31a1689cb0a1`) which includes the same security fixes backported from v29.3.1 (not yet released as a tagged v28.x version). No datadog-agent code directly imports from `docker/docker` anymore. **`docker.image.virtual_size` metric:** This metric now reports `image.Size` instead of the removed `image.VirtualSize` field. These values have been identical since Docker API v1.44 (`VirtualSize` was already deprecated). [CONTINT-5217]: https://datadoghq.atlassian.net/browse/CONTINT-5217?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: lenaic.huard <lenaic.huard@datadoghq.com>
1 participant
Summary
github.com/docker/dockerto the equivalentgithub.com/moby/mobysub-modules (clientv0.2.2 andapiv1.53.0)docker/docker/pkg/system.Lgetxattrwithcontainerd/continuity/sysx.LGetxattr(DataDog-specific container artifact)HostConfigModifiercallbacks withAutoRemovefield ontestcontainers.ContainerRequestin all integration testsreplacedirective redirecting the remaining indirectdocker/dockerto the patchedmoby/moby27.x branch commitContext
github.com/docker/dockerv27 has known security vulnerabilities. This PR eliminates all direct usage so thatDataDog/datadog-agentcan consume this trivy version without pulling in the vulnerable package.Ports the upstream migration from aquasecurity/trivy#10202 (Docker client SDK v29) adapted to the v27-era codebase of this branch.
Jira: VULN-59766, VULN-59767, VULN-59774, VULN-59775, CONTINT-5217, CONTINT-5218, CONTINT-5219, CONTINT-5220
Related: DataDog/datadog-agent#48777
Test plan
go mod tidycompletes successfullygo build ./...compiles without errors (pre-existinginternal/testutil/util.goissue excluded)go vetpasses on all modified packagesgo test ./pkg/fanal/image/daemon/...— all unit tests passgithub.com/docker/dockerimports in Go source filesdocker/dockeris now// indirectingo.mod, redirected viareplaceto patched moby/moby🤖 Generated with Claude Code