Repository navigation
feat: resolve real Docker container names in CONTAINER column - #2125
headersalreadysent wants to merge 1 commit into
Conversation
htop's CONTAINER column is derived purely from the cgroup scope name
(linux/CGroupUtils.c). For processes inside Docker containers it has
always shown a heuristic marker !docker:<12-hex-id> - the container ID
truncated to 12 characters - and never the actual container name,
because htop never queries the container runtime.
This commit adds real name resolution for Docker containers:
* New linux/DockerMgr.{c,h}: a small unix-socket HTTP client that talks
to /var/run/docker.sock, requests GET /containers/<id>/json, extracts
the Name field, and caches results per container ID so the socket is
hit at most once per container per htop run.
* linux/LinuxProcessTable.c: when the cgroup heuristic produces a
!docker: prefix, the ID is passed to DockerMgr and, on success, the
real container name replaces the marker.
* Display names are capped at 25 characters (the cache stores the
truncated name), keeping the auto-width CONTAINER column reasonable
with long container names.
* linux/CGroupUtils.c: return empty string instead of / for processes
not in a recognized container, keeping the column clean for host
processes.
Failure behavior is intentional and graceful: if the Docker socket is
unreachable (daemon stopped, socket missing, or user not in the docker
group), DockerMgr_getContainerName() returns NULL, the call site keeps
the original !docker:<id> fallback, and nothing fails. No new crash
paths are introduced.
Requires a reachable Docker daemon and a user in the docker group (or
root). No new dependencies were added; build wiring is in Makefile.am
(autotools-only tree). Verified with targeted unit tests (name
resolution, 25-char cap, empty host-process column) against a live
Docker daemon.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)📝 WalkthroughWalkthroughThe change adds Docker container-name lookup through Suggested reviewers: Priority: ⬇️ Low Change: Feature Merge Risk: 🟡 Moderate · up to A stalled Docker connection can freeze container-name updates and the process-table refresh. Bound the socket exchange before merging; also correct the cache key so distinct containers cannot display the wrong name. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Container-name resolution now depends on Docker during process-table refresh. The change lacks request-identifier validation, bounded socket wait times, and exact container-identity matching in its cache. Ordinary lookup failures preserve the previous display, and only container names are exposed; privilege escalation or broad data disclosure has not been established. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A container name travels from the socket. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
17c2cb39-ad31-47bd-bf74-63d24729579f
📒 Files selected for processing (5)
Makefile.amlinux/CGroupUtils.clinux/DockerMgr.clinux/DockerMgr.hlinux/LinuxProcessTable.c
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| static unsigned int hashStr(const char* s) { | ||
| unsigned int h = 5381; | ||
| while (*s) | ||
| h = (h * 33) ^ (unsigned char)*s++; | ||
| return h; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '24,110p' linux/DockerMgr.c
rg -n 'typedef.*ht_key_t|Hashtable_new|Hashtable_put|Hashtable_get|Hashtable_create' Hashtable.* linux/DockerMgr.cRepository: htop-dev/htop
Length of output: 3258
🏁 Script executed:
printf '%s\n' '--- Hashtable key and declarations ---'; cat -n Hashtable.h | sed -n '1,45p'; printf '%s\n' '--- Hashtable insertion and lookup ---'; cat -n Hashtable.c | sed -n '105,135p;205,250p;275,315p'; printf '%s\n' '--- Docker ID sources and cgroup parsing ---'; rg -n -i 'docker|container.*id|cgroup|containerId|container_id' linux | head -100Repository: htop-dev/htop
Length of output: 13313
🏁 Script executed:
printf '%s\n' '--- Hashtable internals ---'; cat -n Hashtable.c | sed -n '30,105p;130,165p'; printf '%s\n' '--- Docker cgroup ID extraction ---'; cat -n linux/CGroupUtils.c | sed -n '205,280p'; printf '%s\n' '--- Cache caller ---'; cat -n linux/LinuxProcessTable.c | sed -n '1003,1018p'Repository: htop-dev/htop
Length of output: 7254
🏁 Script executed:
printf '%s\n' '--- Hashtable insertion replacement ---'; cat -n Hashtable.c | sed -n '152,205p'; printf '%s\n' '--- Docker-specific filter branch ---'; cat -n linux/CGroupUtils.c | sed -n '25,75p;280,360p'Repository: htop-dev/htop
Length of output: 6938
Use a collision-safe cache key for Docker IDs.
hashStr reduces each 12-character Docker ID to a 32-bit ht_key_t. Distinct IDs can therefore produce the same key. Hashtable_get compares only that key, so the second ID can receive the first ID's cached name.
Use a representation that preserves all 12 hexadecimal characters. Parsing into an integer requires widening ht_key_t beyond its current 32-bit type. Otherwise, retain the full ID and handle hash collisions before returning or replacing entries.
The owner-semantics warning does not apply: nameCache already uses Hashtable_new(16, true), and replacement values are freed under that setting.
| static char* queryName(const char* id) { | ||
| int fd = socket(AF_UNIX, SOCK_STREAM, 0); | ||
| if (fd < 0) | ||
| return NULL; | ||
|
|
||
| struct sockaddr_un addr; | ||
| memset(&addr, 0, sizeof(addr)); | ||
| addr.sun_family = AF_UNIX; | ||
| snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", DOCKER_SOCKET); | ||
|
|
||
| if (connect(fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { | ||
| close(fd); | ||
| return NULL; | ||
| } | ||
|
|
||
| char req[256]; | ||
| int reqLen = snprintf(req, sizeof(req), | ||
| "GET /containers/%s/json HTTP/1.1\r\n" | ||
| "Host: localhost\r\n" | ||
| "Accept: application/json\r\n" | ||
| "Connection: close\r\n\r\n", id); | ||
| if (reqLen <= 0 || reqLen >= (int)sizeof(req) || write(fd, req, (size_t)reqLen) != reqLen) { | ||
| close(fd); | ||
| return NULL; | ||
| } | ||
|
|
||
| char buf[RESPONSE_SIZE]; | ||
| size_t used = 0; | ||
| ssize_t rd; | ||
| while (used < sizeof(buf) - 1 && (rd = read(fd, buf + used, sizeof(buf) - 1 - used)) > 0) | ||
| used += (size_t)rd; | ||
| close(fd); | ||
|
|
||
| if (used == 0) | ||
| return NULL; | ||
| buf[used] = '\0'; | ||
|
|
||
| char* key = strstr(buf, "\"Name\":\""); | ||
| if (!key) | ||
| return NULL; | ||
| char* val = key + strlen("\"Name\":\""); | ||
| if (*val == '/') | ||
| val++; | ||
| char* end = strchr(val, '"'); | ||
| if (!end) | ||
| return NULL; | ||
| return xStrndup(val, (size_t)(end - val)); | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,125p' linux/DockerMgr.c
printf '\n-- socket timeout/nonblocking patterns --\n'
rg -n --glob '*.[ch]' 'SO_RCVTIMEO|SO_SNDTIMEO|poll\(|select\(|fcntl\(.*O_NONBLOCK|connect\(' linux | head -120Repository: htop-dev/htop
Length of output: 3102
Bound the complete Docker socket exchange
queryName performs synchronous connect, write, and read calls during process-table refresh. A stalled Docker socket can delay the refresh indefinitely. Bound the complete exchange: use nonblocking connect with a finite poll deadline, and apply the same deadline to request writes and response reads. SO_RCVTIMEO and SO_SNDTIMEO alone do not bound connect.
The stack-buffer and negative-cache suggestions are separate concerns and are omitted from this correction.
|
Two more points:
Overall not a huge fan; but TBH not a huge fan of Docker either … |
BenBE
added
Linux 🐧
2 participants
htop's CONTAINER column is derived purely from the cgroup scope name (linux/CGroupUtils.c). For processes inside Docker containers it has always shown a heuristic marker !docker:<12-hex-id> - the container ID truncated to 12 characters - and never the actual container name, because htop never queries the container runtime.
This commit adds real name resolution for Docker containers:
Failure behavior is intentional and graceful: if the Docker socket is unreachable (daemon stopped, socket missing, or user not in the docker group), DockerMgr_getContainerName() returns NULL, the call site keeps the original !docker: fallback, and nothing fails. No new crash paths are introduced.
Requires a reachable Docker daemon and a user in the docker group (or root). No new dependencies were added; build wiring is in Makefile.am (autotools-only tree). Verified with targeted unit tests (name resolution, 25-char cap, empty host-process column) against a live Docker daemon.