Skip to content

fix: preserve file permissions on atomic writes (Docker/NAS fix) - #10618

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-9efbf4be
Apr 16, 2026
Merged

fix: preserve file permissions on atomic writes (Docker/NAS fix)#10618
teknium1 merged 1 commit into
mainfrom
hermes/hermes-9efbf4be

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Fixes dashboard config saves resetting file permissions in Docker/NAS deployments, causing Permission denied errors and provider auth failures.

Reported by: Cedric Weber (Docker/Portainer on NAS)

Root Cause

Three layers of permission clobbering:

  1. atomic_yaml_write() / atomic_json_write()tempfile.mkstemp() creates temp files with 0o600. After os.replace(), the original file's permissions are destroyed.
  2. _secure_file() — explicitly sets 0o600 after every config save
  3. save_env_value() — has its own inline mkstemp + replace + _secure_file + redundant os.chmod (triple enforcement)

In Docker, volume-mounted config.yaml/env files may need broader permissions (e.g. 0o666). After any dashboard save, they get hammered to 0o600, breaking multi-process access.

Changes

utils.py:

  • New _preserve_file_mode() / _restore_file_mode() helpers
  • Both atomic_yaml_write and atomic_json_write now capture original permissions before write and restore after os.replace()

hermes_cli/config.py:

  • New _is_container() — detects Docker/Podman/LXC/k8s via /.dockerenv, /proc/1/cgroup, or HERMES_SKIP_CHMOD / HERMES_CONTAINER env vars
  • _secure_file() — skips chmod in containers (matches existing is_managed() pattern for NixOS)
  • save_env_value() — preserves original permissions, removed redundant third os.chmod
  • remove_env_value() — same permission preservation

Behavior

Environment Before After
Desktop 0o600 enforced 0o600 enforced (unchanged)
NixOS managed Skipped (unchanged) Skipped (unchanged)
Docker/container 0o600 enforced (BUG) Original permissions preserved
HERMES_SKIP_CHMOD=1 N/A Original permissions preserved

Test Plan

  • All 57 existing config/atomic-write tests pass
  • E2E tested: save_config + save_env_value with 0o666 permissions in container mode — permissions preserved
  • E2E tested: desktop mode still tightens to 0o600
  • E2E tested: new file creation (no original permissions) works correctly

atomic_yaml_write() and atomic_json_write() used tempfile.mkstemp()
which creates files with 0o600 (owner-only). After os.replace(), the
original file's permissions were destroyed. Combined with _secure_file()
forcing 0o600, this broke Docker/NAS setups where volume-mounted config
files need broader permissions (e.g. 0o666).

Changes:
- atomic_yaml_write/atomic_json_write: capture original permissions
  before write, restore after os.replace()
- _secure_file: skip permission tightening in container environments
  (detected via /.dockerenv, /proc/1/cgroup, or HERMES_SKIP_CHMOD env)
- save_env_value: preserve original .env permissions, remove redundant
  third os.chmod call
- remove_env_value: same permission preservation

On desktop installs, _secure_file() still tightens to 0o600 as before.
In containers, the user's original permissions are respected.

Reported by Cedric Weber (Docker/Portainer on NAS).
@teknium1
teknium1 merged commit df714ad into main Apr 16, 2026
6 of 7 checks passed
@teknium1
teknium1 deleted the hermes/hermes-9efbf4be branch April 16, 2026 02:52
konsisumer added a commit to konsisumer/hermes-agent that referenced this pull request Apr 16, 2026
Resolve conflict in utils.py between this branch's umask-honoring fix
(issue NousResearch#9239, NixOS /save) and main's preserve-existing-mode fix
(PR NousResearch#10618, Docker/NAS).  The merged policy: when an existing file is
overwritten, restore its prior permissions (Docker/NAS); when a new
file is created, apply 0o666 & ~umask — the mode a plain open() would
yield — so NixOS managed mode (umask 0007) gets 0o660 instead of 0o600.

The overwrite-permissions test in
tests/test_utils_atomic_write_permissions.py is updated to assert the
Docker/NAS preserve semantics, matching main's policy.
aj-nt pushed a commit to aj-nt/hermes-agent that referenced this pull request May 1, 2026
…sResearch#10618)

atomic_yaml_write() and atomic_json_write() used tempfile.mkstemp()
which creates files with 0o600 (owner-only). After os.replace(), the
original file's permissions were destroyed. Combined with _secure_file()
forcing 0o600, this broke Docker/NAS setups where volume-mounted config
files need broader permissions (e.g. 0o666).

Changes:
- atomic_yaml_write/atomic_json_write: capture original permissions
  before write, restore after os.replace()
- _secure_file: skip permission tightening in container environments
  (detected via /.dockerenv, /proc/1/cgroup, or HERMES_SKIP_CHMOD env)
- save_env_value: preserve original .env permissions, remove redundant
  third os.chmod call
- remove_env_value: same permission preservation

On desktop installs, _secure_file() still tightens to 0o600 as before.
In containers, the user's original permissions are respected.

Reported by Cedric Weber (Docker/Portainer on NAS).
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…sResearch#10618)

atomic_yaml_write() and atomic_json_write() used tempfile.mkstemp()
which creates files with 0o600 (owner-only). After os.replace(), the
original file's permissions were destroyed. Combined with _secure_file()
forcing 0o600, this broke Docker/NAS setups where volume-mounted config
files need broader permissions (e.g. 0o666).

Changes:
- atomic_yaml_write/atomic_json_write: capture original permissions
  before write, restore after os.replace()
- _secure_file: skip permission tightening in container environments
  (detected via /.dockerenv, /proc/1/cgroup, or HERMES_SKIP_CHMOD env)
- save_env_value: preserve original .env permissions, remove redundant
  third os.chmod call
- remove_env_value: same permission preservation

On desktop installs, _secure_file() still tightens to 0o600 as before.
In containers, the user's original permissions are respected.

Reported by Cedric Weber (Docker/Portainer on NAS).
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…sResearch#10618)

atomic_yaml_write() and atomic_json_write() used tempfile.mkstemp()
which creates files with 0o600 (owner-only). After os.replace(), the
original file's permissions were destroyed. Combined with _secure_file()
forcing 0o600, this broke Docker/NAS setups where volume-mounted config
files need broader permissions (e.g. 0o666).

Changes:
- atomic_yaml_write/atomic_json_write: capture original permissions
  before write, restore after os.replace()
- _secure_file: skip permission tightening in container environments
  (detected via /.dockerenv, /proc/1/cgroup, or HERMES_SKIP_CHMOD env)
- save_env_value: preserve original .env permissions, remove redundant
  third os.chmod call
- remove_env_value: same permission preservation

On desktop installs, _secure_file() still tightens to 0o600 as before.
In containers, the user's original permissions are respected.

Reported by Cedric Weber (Docker/Portainer on NAS).
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…sResearch#10618)

atomic_yaml_write() and atomic_json_write() used tempfile.mkstemp()
which creates files with 0o600 (owner-only). After os.replace(), the
original file's permissions were destroyed. Combined with _secure_file()
forcing 0o600, this broke Docker/NAS setups where volume-mounted config
files need broader permissions (e.g. 0o666).

Changes:
- atomic_yaml_write/atomic_json_write: capture original permissions
  before write, restore after os.replace()
- _secure_file: skip permission tightening in container environments
  (detected via /.dockerenv, /proc/1/cgroup, or HERMES_SKIP_CHMOD env)
- save_env_value: preserve original .env permissions, remove redundant
  third os.chmod call
- remove_env_value: same permission preservation

On desktop installs, _secure_file() still tightens to 0o600 as before.
In containers, the user's original permissions are respected.

Reported by Cedric Weber (Docker/Portainer on NAS).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant