fix(constants): stop is_container() false positives on hosts that run containers (#65051)

[ayushnangia]

Summary

Stop is_container() reporting "inside a container" on hosts that merely run containers, by replacing the whole-file mountinfo substring scan with a structural check of the mounts that actually prove containment (#65051).

Changes

• hermes_constants.py: is_container() — the cgroup-v2 last-resort now parses /proc/self/mountinfo per-line and only inspects container-conclusive mounts: the ROOT mount (/) whose fs-root path / source / super options reference a runtime (docker, containerd, crio, podman, kubepods, kubelet, buildkit), or /etc/resolv.conf|hostname|hosts bind-mounted from kubelet/runtime-managed dirs (how runtimes inject network identity — a host never bind-mounts its own /etc identity files from those paths). /.dockerenv, /run/.containerenv, KUBERNETES_SERVICE_HOST, and the /proc/1/cgroup markers are untouched
• tests/test_hermes_constants.py: 3 tests — bare-VPS host running docker containers (containerd shim mounts + overlay mounts in the host table → False; the terminal.backend (docker AND ssh) silently forced to local - false "running inside a container" self-detection on bare VM hosts #65051 shape), WSL2 + Docker Desktop snapshot mounts (→ False; the fix(checkpoints): tolerate invalid timestamps #51930 shape), and kubelet /etc/hosts bind mount (→ True; new detection coverage). The two False cases fail on unfixed code; all 8 existing detection tests pass unchanged, including the is_container() misses containerd + cgroup v2 (k8s/k3s) → s6 gateway supervision silently disabled → gateway double-start crash loop #47111 cgroup-v2 containerd-pod case

Root cause

The mountinfo fallback added in #47144 (for #47111: cgroup-v2 pods where /proc/1/cgroup is a markerless 0::/) scans the entire file:

if any(marker in mountinfo for marker in ("kubepods", "containerd", "crio")):

But container runtime mounts are visible in the host's mount namespace too: running any Docker container puts /run/containerd/io.containerd.runtime.v2.task/moby/<id>/rootfs shim mounts in the host's table, so "containerd" in mountinfo is true on exactly the machines most likely to configure terminal.backend: docker for isolation. #51930 hit the same class from the WSL side (Docker Desktop snapshot mounts); #51935 proposed skipping the mountinfo check on WSL — this change fixes the class structurally instead, covering bare hosts and WSL with one rule, and keeps mountinfo-based detection working where it is needed.

Distinguishing signal: what matters is not whether runtime paths appear in the table, but which mount they provide. Inside a container the root mount itself comes from the runtime (the existing cgroup-v2 test's fixture — ... /containerd/.../rootfs / ... overlay — is exactly this shape and passes unchanged); on a host the root is a real device and the runtime paths hang off non-identity mount points.

The false positive is user-visible beyond hermes doctor's misleading "Running inside a container — using local terminal backend" line: is_container() also gates voice-mode audio, config-file permission hardening (_is_container chmod behavior), gateway service-manager detection, and the startup security audit.

Note on the issue's second claim ("backend silently forced to local at dispatch time"): I could not find such a code path — terminal_tool._get_env_config() reads TERMINAL_ENV with no is_container() gate, and the doctor message is display-only. Posted the dispatch-chain trace on the issue; the observed host execution is most likely the systemd service user resolving a different HERMES_HOME than the config file being edited. This PR fixes the verified detection bug.

Validation

Environment	Before	After
Bare KVM host, Docker containers running (#65051)	container=True (shim mounts match "containerd")	container=False
WSL2 + Docker Desktop (#51930)	container=True (snapshot mounts)	container=False
containerd/CRI-O pod, cgroup v2 (#47111)	container=True	container=True (unchanged)
kubelet /etc/hosts bind mount, unmarked root	container=False (missed)	container=True
/.dockerenv, podman, k8s env, cgroup-v1 markers	detected	detected (untouched)

• scripts/run_tests.sh tests/test_hermes_constants.py — 95 passed, 0 failed
• Consumers: scripts/run_tests.sh tests/tools/test_voice_mode.py tests/hermes_cli/test_gateway.py tests/hermes_cli/test_container_aware_cli.py — 140 passed, 0 failed

Fixes #65051. Refs #47111/#47144 (origin of the fallback), #51930/#51935 (WSL variant of the same class — the WSL skip becomes unnecessary with this), #34397, #25402.

[ayushnangia]

Two follow-ups after a hostile self-audit:

1. Hardening commit (038be2a): Podman and CRI-O keep container roots under the shared containers/storage tree (rootful /var/lib/containers/storage/..., rootless ~/.local/share/containers/storage/...) — paths that spell neither podman nor crio, so the root-mount check could miss them where /run/.containerenv isn't present (CRI-O never writes it). Added containers/storage to the marker set + a rootless-podman-shaped test. 12/12 detection tests, 96/96 file.

2. E2E against a real container (python:3.11-slim under Docker Desktop), specifically exercising the new mountinfo-only path by disabling the earlier signals (/.dockerenv, /run/.containerenv masked; /proc/1/cgroup forced to a markerless 0::/):

E2E in real container, all signals   : True
E2E mountinfo-only path (cgroup 0::/): True
real container root mount line: 136 84 0:63 / / rw,relatime - overlay overlay
  rw,lowerdir=/var/lib/desktop-containerd/daemon/io.containerd.snapshotter.v1.overlayfs/snapshots/...

The real root mount carries the runtime marker in its super options — exactly the field the structural parser inspects — so cgroup-v2 detection keeps working end-to-end while the host-side false positive (shim/snapshot mounts at non-identity mount points) is gone.

[teknium1]

Thanks for narrowing the cgroup-v2 fallback and adding host and WSL regressions. The underlying issue remains on current main: hermes_constants.py:1131-1135 scans every mountinfo line for runtime substrings.

Problems

• hermes_constants.py:975 builds probe_fields from fields[3] plus all fields after -, which includes the filesystem source. The generic "docker" marker at hermes_constants.py:956 would classify a non-container host whose root source is named, for example, /dev/mapper/docker--vg-root. That source name is not container-conclusive, so the new fallback retains a false-positive path.

Suggested changes

• Limit matching to runtime-specific path shapes in the root field and overlay super-options, or exclude generic sources; add a negative root-source regression alongside tests/test_hermes_constants.py:418.

Automated hermes-sweeper review.

[teknium1]

fields[sep + 1:] includes the filesystem source. Combined with the generic docker marker, a host root source such as /dev/mapper/docker--vg-root is detected as a container even though it proves no containment. Restrict the probe to runtime-specific root/super-option paths (or exclude generic sources) and add a negative regression.

[ayushnangia]

Right — the source field proves device provenance, not containment. Fixed in e2145c4: the probe is now fields[3] (fs-root path) + super options only (fields[sep + 3:]), so fstype and source never participate. Added the exact negative regression (/dev/mapper/docker--vg-root as host root source → False), 13/13 detection tests, and re-ran the real-container E2E with /.dockerenv//containerenv/cgroup signals masked — mountinfo-only detection still returns True (marker lives in the overlay super options, which stay probed).

[ayushnangia]

Premise re-anchor — #65051 (this PR's title issue) was closed by its reporter after he traced his remaining symptom to an unrelated hermes config set full-file-rewrite footgun (comment/formatting loss is tracked in #63039; the list-coercion sibling is #64323). The detection defect this PR fixes is independent of that closure and still live on current main (d71033a40):

# hermes_constants.py:1144-1148
with open("/proc/self/mountinfo", "r", encoding="utf-8") as f:
    mountinfo = f.read()
    if any(marker in mountinfo for marker in ("kubepods", "containerd", "crio")):
        _container_detected = True

The cgroup-v2 fallback still substring-scans the entire mount table, so a host merely running containers matches its own containers' overlay/shim mounts. The same defect is independently reported and still open as #58135 (bare Linux host, containerd image store — this PR fixes that report too), and in WSL-shaped form as #51935.

Sibling-PR map, since the sweeper reviewed several in this class:

• fix(cli): stop is_container() false-positive on hosts running containers #58145 restricts the scan to the root-mount line — fixes the reported shape, but matches the whole line including the mount source field, the class @teknium1 flagged on this PR (device provenance ≠ containment; a root LV/device path spelling a runtime name still false-positives). It also doesn't cover rootless-podman/CRI-O containers/storage roots.
• fix: skip mountinfo container check on WSL (Docker Desktop false positive) #51935 / fix: skip containerd mountinfo marker on WSL2 to avoid false-positive… #51968 gate the fallback on is_wsl() — fixes only the WSL shape; the bare-host false positive of [Bug]: is_container() false-positives on hosts running Docker containers (containerd markers in mountinfo), making home_mode: auto subprocess HOME non-deterministic — breaks browser auto-launch with "Chrome not found" #58135/terminal.backend (docker AND ssh) silently forced to local - false "running inside a container" self-detection on bare VM hosts #65051 survives.

This PR probes structural fields only (fs-root + super options — source and fstype never participate), extends the marker set to containers/storage, and carries negative regressions for host-root sources plus a real-container E2E with the other signals masked. If maintainers prefer landing one of the minimal variants instead, the negative-regression matrix here should transfer directly.

Rebased onto current main (d71033a40, head ba7c537dd): scripts/run_tests.sh tests/test_hermes_constants.py — 131 passed, 0 failed.

[GottZ]

This was generated by AI during triage.

Summary

Three open PRs address the container-detection defect: #58141 and #58145 both restrict mountinfo matching to the root mount, while #65060 uses a broader structural parser that also covers WSL, kubelet identity mounts, and Podman/CRI-O storage paths. Each diff excludes the reported non-root containerd mounts that falsely classify a host as containerized.

Related pull requests

• fix(agent): scan only root mount in is_container() cgroup-v2 fallback (#58135) #58141 best fix — (+54/-7) — n/a: Restricts the cgroup-v2 fallback to the / mount and adds a negative regression for a host running containers, directly addressing the reported whole-table containerd match. The keep_open verdict identifies this earlier PR as the canonical, highly salvageable focused fix.
• fix(cli): stop is_container() false-positive on hosts running containers #58145 fixes — (+74/-9) — n/a: Implements the same root-mount-only boundary and host regression as fix(agent): scan only root mount in is_container() cgroup-v2 fallback (#58135) #58141, plus an explicit positive root-overlay test. Despite its keep_open verdict, contributor triage identifies fix(cli): stop is_container() false-positive on hosts running containers #58145 as a duplicate of the earlier canonical fix(agent): scan only root mount in is_container() cgroup-v2 fallback (#58135) #58141, with the same function and mechanism.
• fix(constants): stop is_container() false positives on hosts that run containers (#65051) #65060 best fix — (+179/-5) — n/a: Replaces whole-table matching with a broader structural check over root and runtime-managed identity mounts, adding host, WSL, kubelet, LVM, and Podman/CRI-O regressions. Its contributor keep_open review flagged matching a docker-named root source, and the shown diff addresses that concern by excluding the source field and adding the /dev/mapper/docker--vg-root negative test.

Duplicates

#58141 and #58145 are substantive duplicates of the same root-mount-only correction; #58141 is the earlier canonical implementation. #65060 overlaps that correction but is not an exact duplicate because its parser and runtime coverage are broader.

Suggested consolidation

Keep #58141 open with the focused root-mount filter and host regression as its salvage path, and close #58145 as duplicate of #58141; this differs from #58145's keep_open verdict because the visible diffs use the same production mechanism and contributor triage explicitly designates #58141 as canonical. Keep #65060 open with its broader structural parser and additional WSL, kubelet, LVM, and Podman/CRI-O regressions as the salvage path, since the current diff explicitly addresses its contributor review's root-source false-positive concern.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I58135(["issue #58135 (open)"])
    I65051(["issue #65051 (closed)"])
    P65060["PR #65060 (open)"]
    P65060 -->|fixes| I58135
    P65060 -->|best fix| I65051
    class I58135 open
    class I65051 closed
    class P65060 open
    class P65060 best
    class P65060 target
    click I58135 "https://github.com/NousResearch/hermes-agent/issues/58135"
    click I65051 "https://github.com/NousResearch/hermes-agent/issues/65051"
    click P65060 "https://github.com/NousResearch/hermes-agent/pull/65060"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 20 kB of PR diffs, 21 kB of issue/PR text, 17 kB of discussion (16 comments), 13 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.