Skip to content

fix(docker): targeted chown to preserve host file ownership in HERMES_HOME (#19795)#33033

Merged
benbarclay merged 1 commit into
mainfrom
salvage-19795-targeted-chown
May 27, 2026
Merged

fix(docker): targeted chown to preserve host file ownership in HERMES_HOME (#19795)#33033
benbarclay merged 1 commit into
mainfrom
salvage-19795-targeted-chown

Conversation

@benbarclay

Copy link
Copy Markdown
Collaborator

Salvages #19795 (@ptichalouf, issue #19788).

Problem

docker/stage2-hook.sh does chown -R hermes:hermes "$HERMES_HOME" whenever
the runtime hermes UID doesn't match the directory owner (typically: on first
boot or when HERMES_UID is remapped). When $HERMES_HOME is a bind-mounted
host directory that contains files not managed by hermes, the recursive
chown silently rewrites every file's ownership — destroying host-side file
ownership permanently.

Fix

Targeted chown:

  • Top-level $HERMES_HOME itself is chowned (not its contents), so hermes
    can mkdir new subdirs.
  • Hermes-owned subdirs get recursive chown — these are the same dirs
    seeded by the s6-setuidgid mkdir -p block below
    (cron sessions logs hooks memories skills skins plans workspace home profiles).
  • Everything else in $HERMES_HOME (host files, host subdirs) keeps its
    original ownership.

Validation

Built three images: baseline (origin/main), the salvage in this PR, and ran
the same chown E2E harness against each.

The E2E harness:

  1. Creates a host dir with a pre-existing host-owned HOST_FILE.txt and a
    host-owned-subdir/note.txt
  2. Mounts it as /opt/data and runs docker run -e HERMES_UID=12345 --version
  3. Inspects post-run ownership from inside the volume (via root-privileged
    helper container — host UID can no longer stat into a 12345-owned dir)

Baseline (origin/main, current behavior)

✓ stage2 chown path triggered: [stage2] Fixing ownership of /opt/data to hermes (12345)
✗ HOST_FILE.txt UID changed (1000 → 12345) — BUG #19788 PRESENT
✗ host-owned-subdir/note.txt UID changed (1000 → 12345) — recursive chown leaked
✓ HOST_FILE.txt content preserved (only the metadata, not the bytes)
✓ $HERMES_HOME/cron owned by 12345 (the remapped hermes UID)

Result: 5 pass, 2 fail — bug reproduces cleanly

Salvage (this PR)

✓ stage2 chown path triggered: [stage2] Fixing ownership of /opt/data (targeted) to hermes (12345)
✓ HOST_FILE.txt UID preserved (1000 → 1000) — bug #19788 not present
✓ host-owned-subdir/note.txt UID preserved (1000 → 1000)
✓ HOST_FILE.txt content preserved verbatim
✓ $HERMES_HOME/cron owned by 12345 (the remapped hermes UID)
✓ top-level $HERMES_HOME chowned to 12345 (hermes can create new subdirs)

Result: 7 pass, 0 fail

Also confirmed via the standard image smoke battery (6/6 pass): --version,
hermes_cli import inside venv, _tui_need_npm_install: False, --tui
no-TTY exit, tools.lazy_deps imports, gcc still present.

Authorship

Original change by @ptichalouf in #19795. Their branch targeted
docker/entrypoint.sh (a deprecated shim since the s6-overlay rework — the
real cont-init logic moved to docker/stage2-hook.sh); retargeted the
fix accordingly. Preserved attribution via Co-authored-by:.

Closes #19795.
Fixes #19788.

…_HOME (#19795)

Replaces the recursive chown of $HERMES_HOME in stage2-hook.sh with a
targeted approach: chown the top-level dir (so hermes can create new subdirs)
plus the specific hermes-owned subdirectories (cron/, sessions/, logs/,
hooks/, memories/, skills/, skins/, plans/, workspace/, home/, profiles/) —
the same canonical list seeded by the s6-setuidgid mkdir -p block below.

Avoids clobbering host-side file ownership when $HERMES_HOME is a bind
mount that contains user-owned files not managed by hermes (issue #19788).

Original fix targeted docker/entrypoint.sh which is now a deprecated shim;
retargeted to docker/stage2-hook.sh where the recursive chown moved during
the s6-overlay rework.

Co-authored-by: Ptichalouf <1809721+ptichalouf@users.noreply.github.com>
@benbarclay benbarclay added the area/docker Docker image, Compose, packaging label May 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: salvage-19795-targeted-chown vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9400 on HEAD, 9400 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4973 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists backend/docker Docker container execution labels May 27, 2026
@benbarclay benbarclay merged commit 9eadb68 into main May 27, 2026
24 of 25 checks passed
@benbarclay benbarclay deleted the salvage-19795-targeted-chown branch May 27, 2026 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging backend/docker Docker container execution P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

default entrypoint.sh recursively chown's to 10000:10000 - good thing I never ran it on my host root

2 participants