fix(collector): surface unreadable docker sockets as DEGRADED and make doctor evaluate the service user (#580)

[evertramos]

What & why

Closes #580

A kind: docker collector whose Engine socket is unreadable by the service user failed silently: runAPI retried inside its own loop forever, so runCollector never saw an error, collectors_state stayed OK, no audit row or notification was produced, and doctor printed a green docker: socket access because it dialed as the invoking user (root under sudo, or an operator in the docker group).

Acceptance criteria restated, and where each is satisfied:

Collector

• A docker collector or exec watcher whose connect / first request fails persistently (permission denied, HTTP 403/404) returns the error from Run, so the supervisor's backoff, collectors_state = DEGRADED, the collector_degraded audit row and the one-shot critical notification all fire exactly as they do for journald. Retrying stays in-loop only for stream drops after a successful connect.
• EACCES is reported as a permission error naming the service user and the three access paths — never as "engine socket unavailable, falling back to filesystem tail". The filesystem fallback is not attempted on a permission failure (/var/lib/docker is closed for the same reason).
• ezyshield status shows the existing DEGRADED banner naming the collector within one supervisor cycle (unchanged path — it is now actually reached).

Doctor (works without root: pure arithmetic over stat + /etc/passwd + /etc/group, all world-readable; nothing is dialed or executed)

• docker: socket access evaluates the service user: FAIL when a docker collector or docker_exec is configured and ezyshield cannot read+write the socket; PASS when it can (group membership, owner, or world bits — so a socket proxy running as ezyshield passes too); N/A when nothing docker-related is configured or Docker is absent. The hint names the three access paths and their privilege cost.
• service user: docker group keeps WARN for members; "not a member" no longer reads as PASS when a docker collector is configured — it defers to the socket-access verdict, which is the check that knows whether another path grants the access.
• New collectors: observation state asks the running daemon for collectors_state over the control socket and FAILs on DEGRADED, naming the collector and its last error (mirror of checkEnforcementState).
• Field scenario (docker collector configured, service user not in the group, daemon running) now prints socket access FAIL + observation state FAIL.

Non-goal respected: no new access path is added; the configurable docker.host transport is #579. This PR only makes the failure visible and the doctor truthful.

Changes

• internal/collector/docker_api.go: streamAPILogs now returns (connected bool, err error) — connected is true only after a 200, so a dial failure or a 403/404 is distinguishable from a dropped live stream. runAPI tracks everConnected plus a consecutive-failure streak and calls the new fatalAPIError, which returns the error on errors.Is(err, fs.ErrPermission) immediately, after 3 attempts when the collector never connected in this Run, and after 10 consecutive failed reconnects once a stream had worked.
• internal/collector/docker.go: isUnixSocket returns the stat error; Run returns a permission error (no filesystem fallback) when the socket path is closed with EACCES. New shared dockerPermissionError names the service user and the three access paths with their cost.
• internal/collector/dockerexec.go: same connect/drop split for the events stream; persistent connect failures and permission denials are returned from Run.
• internal/daemon/execactivity.go: the injected exec watcher is now supervised through runCollector via a small sdk.Collector adapter (docker-exec-watch), so a watcher that stops gets the same backoff, DEGRADED state, audit row and critical alert. cmd/ezyshield/run.go is untouched (its closure already logs the underlying error).
• internal/daemon/collhealth.go: collectorsState reports DEGRADED ahead of NONE — supervised observation sources are not all in d.collectors.
• cmd/ezyshield/doctor_dockeraccess.go (+ _linux.go / _other.go stat split, same pattern as checkConfigOwnership): the service-user socket-access check.
• cmd/ezyshield/doctor_collstate.go: the observation-state check.
• cmd/ezyshield/doctor.go: two registration lines; the old checkDockerSocket (dialed as the caller) is deleted.
• cmd/ezyshield/doctor_dockergroup.go: !member + docker configured → N/A pointing at docker: socket access.
• docs/content/{en,pt-br}/guides/troubleshooting.md: new "collector configured but cannot read its source" section with the exact doctor output.

Tests

Written first; each new behavioural test fails on origin/dev (verified by stashing the source changes: Run never returned on a permission-denied socket, Run never returned for a container the engine does not serve, exec watcher never returned…, it fell back instead of failing honestly) and passes here.

• internal/collector/docker_visibility_test.go: socket present but mode 0000 → Run returns a permission error naming the user and the three paths (skipped as root, where mode bits do not deny); parent dir denying traversal → permission error, no filesystem fallback; HTTP 404 → error after a bounded retry (≥2 attempts, so a blip is still absorbed); stream drop after a successful connect still retries in-loop and shutdown still returns nil; exec watcher on an unreadable socket returns the same error.

• internal/daemon/collector_supervise_test.go: a collector failing on connect flips collectors_state to DEGRADED with the collector and error named, writes exactly one collector_degraded audit row and sends exactly one critical notification; a stopped exec watcher is recorded as DEGRADED under docker-exec-watch.

• cmd/ezyshield/doctor_dockeraccess_test.go: decision table over passwd/group/socket fixtures (FAIL when configured and inaccessible, PASS via group membership, PASS when the socket is owned by the service user, N/A when unconfigured / Docker absent / service user unknown, FAIL when the path itself is closed), the kernel permission arithmetic including the no-fallthrough property, and the passwd/group parsing.

• cmd/ezyshield/doctor_collstate_test.go: checkCollectorsState against a fake daemon reporting DEGRADED / OK / NONE / not running.

• Unit tests added/updated

• New parser/rule? fixture added in fixtures/ — N/A, no parser or rule changed

• Parser change? fuzz test present — N/A, no parser touched

• make lint test green locally (-race)

gofmt -l .                         (clean)
go build ./...                     ok
go vet ./...                       ok
go test -race ./...                exit 0
golangci-lint run ./...            no findings in this worktree
scripts/ip-hygiene-gate.sh --self-test / origin/dev   no non-example IP literals added
scripts/spdx-gate.sh               all .go files carry the correct SPDX header
scripts/docs-placeholder-gate.sh   no raw placeholder tags in docs/content
scripts/docs-order-gate.sh         every page has a unique order: within its section

Security review (per docs/internal/SECURITY-REVIEW.md)

• §1 Input handling (hostile logs): OK — no parsing changed. The Docker API path still drains a bounded 4 KiB of a non-200 body into io.Discard and never puts it in an error; the new error strings carry only resp.Status (bounded by net/http), the operator-configured socket path, the validated container name (reDockerContainerName) and integers. The exec watcher's caps (maxExecEventLineBytes, capExecField) are untouched. The doctor check parses /etc/passwd and /etc/group as data with a 1 MiB scanner cap, comparing fields and converting ids with strconv.ParseUint — no field is ever interpolated into a command, query or prompt.
• §2 Decision engine (lock-out / false-ban): N/A — no ban, strike, allowlist or anti-lockout path is touched. The change only ends a collector's Run; the supervisor restarts it, so a failing source cannot manufacture events.
• §3 Privilege separation / enforcer: OK — no firewall mutation, no new privilege. The doctor probe is deliberately arithmetic instead of a dial: it needs no root, does not setuid, and does not open the Docker socket, so doctor gains no capability the daemon has. The PR adds no access path (that is feat(collector): docker_host tcp:// transport so container logs can flow through a read-only socket proxy instead of the docker group #579); it only reports that one is missing.
• §4 Secrets: OK — no token, key or credential is read, logged or formatted. The /etc/passwd fixture in the test uses the standard x shadow marker (no hash), with a //nolint:gosec note explaining why G101 does not apply.
• §5 AI / prompt-injection boundary: N/A — no AI provider, prompt or normalizer path is touched.
• §6 Control surfaces (socket/dashboard): OK — no new listener. checkCollectorsState is an outbound client call on the existing daemon control socket using the existing status verb and daemon.Call, with a 2 s timeout, exactly like checkEnforcementState; the status payload already carried collectors_state / collectors_detail.
• §7 Plugins: N/A — no plugin loading or manifest path touched.
• §8 Edge / external APIs: N/A — no Cloudflare/Bunny/AWS call. The Docker Engine call is local and unchanged in shape (same URL, same method).
• §9 Dependencies / supply chain: OK — no new dependency; only stdlib (errors, io/fs, slices, strconv, bufio, syscall behind the existing Linux build tag).
• §10 Logging / audit / fail-safe: the core of this PR. The failure mode fixed is a fail-silent one: protection was claimed while nothing was observed. Now the collector's Run returns, so the existing daemon/status: collector death or persistent read failure is invisible — status keeps reporting ACTIVE with zero observation #456 path fires — collectors_state = DEGRADED, one collector_degraded audit row on the transition (not per retry), one critical notification at the alert threshold — and doctor reports both the cause (service user cannot reach the socket) and the effect (the daemon is not reading). Retry is still bounded on both sides: capped exponential backoff in-loop, then the supervisor's own capped backoff, so nothing hot-loops. Nothing new is written to the audit log or logs beyond the existing transition records, and no error carries a response body.

Self-assessment: No. The change removes no check and grants no access — the docker socket is neither opened nor made more reachable by doctor, and the docker group is still described as root-equivalent rather than handed out as a command. It strictly increases what an operator can see: a source that stops being read now surfaces as DEGRADED instead of a WARN in the journal. The one behavioural risk considered is availability of detection: a collector that previously retried forever can now end its Run — but only after a bounded retry, and the supervisor restarts it, so a docker restart or container restart still recovers on its own (guarded by a test asserting stream drops keep reconnecting in-loop). An attacker who could make the Engine API fail could make the collector cycle through supervisor restarts; that is loud (DEGRADED, audit, critical alert) instead of silent, which is the point of the issue.

Checklist

• Follows AGENTS.md Hard Rules (no new listeners, allowlist supremacy, dry-run default, secrets out of code)
• No hardening systemd directive removed (none touched)
• Docs updated (troubleshooting guide, en + pt-br)
• New dependency justified (none added)

🤖 Generated with Claude Code

https://claude.ai/code/session_01DUE7UrgAffrSvdMVMp7UN3

[strix-security]

Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here.

So far, Strix has reviewed 49 pull requests, surfaced 8 security issues (1 critical/high) and blocked 3 risky merges across this workspace.

[amazon-q-developer]

Review Summary

This PR effectively addresses issue #580 by surfacing silent Docker collector failures as DEGRADED states. The implementation is comprehensive, well-tested, and properly documented.

Key Improvements

• Docker collectors now return errors on persistent connection failures instead of retrying forever
• Permission denied errors are immediately fatal with actionable operator guidance
• The doctor check now evaluates the service user's socket access (not the invoking user)
• Exec watcher failures are now supervised with the same visibility as collectors
• Comprehensive test coverage validates all behavioral changes

Critical Finding

1 issue requires attention - A logic error in dockerexec.go where permission errors should be checked within the else block after incrementing failures, ensuring consistent failure tracking.

The security review is thorough, the retry bounds are appropriate, and the visibility improvements align perfectly with the stated goals. Once the noted issue is addressed, this will significantly improve operational visibility for Docker-based observation paths.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[amazon-q-developer]

🛑 Logic Error: The failure counter is incremented when connected is false, but permission errors should increment failures before checking the limit. The current logic at lines 107-109 returns immediately on permission errors without considering whether it needs to increment the failure counter, potentially causing inconsistent behavior compared to the API collector.

The sequence should be: increment failures → check permission error → check limit. Currently, permission errors bypass the failure increment that occurred only when !connected, which happens before the permission check. This creates a race condition where the failure count may not reflect the actual number of permission denials.

Suggested change

	} else {
	failures++
	}
	if errors.Is(err, fs.ErrPermission) {
	return dockerPermissionError(sock, err)
	}
	if connected {
	everConnected = true
	failures = 0
	} else {
	failures++
	// Permission errors are always fatal and should return immediately
	if errors.Is(err, fs.ErrPermission) {
	return dockerPermissionError(sock, err)
	}
	}

[Copilot]

🟡 Changes recommended

There is at least one misleading new/updated operator-facing log message that should be corrected before merge to avoid obscuring “never connected” vs “stream dropped” behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes a long-standing “fail-silent” gap in Docker observation by ensuring Docker collectors and the docker exec watcher surface persistent startup/connect failures to the daemon supervisor (so collectors_state becomes DEGRADED with audit + one-shot critical notification), and updates ezyshield doctor to evaluate Docker socket access for the service user (not the invoking user) plus adds a new doctor check that asks the running daemon for collectors_state.

Changes:

• Docker Engine API collectors/watchers now distinguish “never connected / first request failed” from “live stream dropped”, and return persistent connect failures (incl. permissions) from Run to trigger supervision + degraded observation state.
• The injected docker exec activity watcher is supervised through runCollector, and aggregate collectorsState() prioritizes DEGRADED over NONE so injected supervised sources can’t be hidden.
• Doctor gains docker: socket access (service-user, pure stat+passwd/group arithmetic) and collectors: observation state (control-socket status), with docs updated to guide remediation.

File summaries

File	Description
internal/daemon/execactivity.go	Wraps injected exec watcher as a supervised collector so failures flip observation health.
internal/daemon/collhealth.go	Ensures DEGRADED outranks NONE so supervised non-d.collectors sources are still reported.
internal/daemon/collector_supervise_test.go	Adds regression tests for degraded state + one-shot audit/critical alert; covers exec watcher supervision.
internal/collector/docker_api.go	Bounds in-loop retry and returns persistent connect failures from Run while keeping live-stream reconnects in-loop.
internal/collector/docker.go	Makes EACCES on socket stat a first-class permission error (no filesystem fallback).
internal/collector/dockerexec.go	Applies the same connected-vs-drop split and bounded retry/return behavior for the events stream.
internal/collector/docker_visibility_test.go	New Linux-only tests reproducing unreadable socket and bounded retry behavior.
cmd/ezyshield/doctor.go	Registers new docker socket-access and collectors-state checks; removes old caller-user dial check.
cmd/ezyshield/doctor_test.go	Updates smoke test to the new socket-access check entry point.
cmd/ezyshield/doctor_dockergroup.go	Defers “not in docker group” verdict to socket-access when docker is configured.
cmd/ezyshield/doctor_dockergroup_test.go	Adjusts expectations for docker-configured non-membership case.
cmd/ezyshield/doctor_dockeraccess.go	Implements service-user Docker socket permission arithmetic and passwd/group parsing.
cmd/ezyshield/doctor_dockeraccess_linux.go	Linux-only uid/gid extraction from syscall.Stat_t.
cmd/ezyshield/doctor_dockeraccess_other.go	Non-Linux stub returning N/A for ownership-based checks.
cmd/ezyshield/doctor_dockeraccess_test.go	Decision-table tests over fixtures for socket ownership + passwd/group parsing and permission arithmetic.
cmd/ezyshield/doctor_collstate.go	Adds doctor check that queries daemon status and FAILs on DEGRADED collectors_state.
cmd/ezyshield/doctor_collstate_test.go	Tests for OK/DEGRADED/NONE/not-running mappings.
docs/content/en/guides/troubleshooting.md	Documents the new “configured but cannot read source” scenario and doctor output.
docs/content/pt-br/guides/troubleshooting.md	Portuguese version of the same troubleshooting guidance.

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.