fix(docker): targeted chown to preserve host file ownership in HERMES_HOME (#19795)#33033
Merged
Conversation
…_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>
Contributor
🔎 Lint report:
|
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvages #19795 (@ptichalouf, issue #19788).
Problem
docker/stage2-hook.shdoeschown -R hermes:hermes "$HERMES_HOME"wheneverthe runtime hermes UID doesn't match the directory owner (typically: on first
boot or when
HERMES_UIDis remapped). When$HERMES_HOMEis a bind-mountedhost 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:
$HERMES_HOMEitself is chowned (not its contents), so hermescan
mkdirnew subdirs.seeded by the
s6-setuidgid mkdir -pblock below(
cron sessions logs hooks memories skills skins plans workspace home profiles).$HERMES_HOME(host files, host subdirs) keeps itsoriginal ownership.
Validation
Built three images: baseline (
origin/main), the salvage in this PR, and ranthe same chown E2E harness against each.
The E2E harness:
HOST_FILE.txtand ahost-owned-subdir/note.txt/opt/dataand runsdocker run -e HERMES_UID=12345 --versionhelper container — host UID can no longer stat into a 12345-owned dir)
Baseline (
origin/main, current behavior)Salvage (this PR)
Also confirmed via the standard image smoke battery (6/6 pass):
--version,hermes_cliimport inside venv,_tui_need_npm_install: False,--tuino-TTY exit,
tools.lazy_depsimports,gccstill present.Authorship
Original change by @ptichalouf in #19795. Their branch targeted
docker/entrypoint.sh(a deprecated shim since the s6-overlay rework — thereal cont-init logic moved to
docker/stage2-hook.sh); retargeted thefix accordingly. Preserved attribution via
Co-authored-by:.Closes #19795.
Fixes #19788.