Add original_name parameter to _download_and_cache, preferring the
attachment metadata filename over the CDN URL path basename. Previously
files were cached with meaningless QQ CDN hash names (e.g.
qqdownload_...oadftnv5), causing ugly filenames when sent back to users.
Aligns with qqbot-agent-sdk's AttachmentDownloader.download_document.
1. Handle op 7 (Server Reconnect): close WS to trigger reconnect loop
while preserving session for Resume
2. Handle op 9 (Invalid Session): check d value to determine if session
is resumable; clear session only when not resumable
3. Remove 4009 from session-clearing set (connection timeout is resumable)
4. Expand fatal close codes: 4001/4002/4010-4014 now stop reconnect
immediately instead of retrying uselessly
5. Add unit tests
1. Add INTERACTION intent bit (1<<26) to _send_identify, fixing approval
button clicks not being received (INTERACTION_CREATE events were never
dispatched by the gateway)
2. Include local cached path in video/file attachment descriptions so the
LLM can reference files for re-sending to users
3. Add unit tests (TestIdentifyIntents, TestProcessAttachmentsPathExposure)
A bare except in _load_gateway_runtime_config would silently return the
unexpanded dict on any _expand_env_vars failure — masking the very bug
this helper exists to fix. Drop it; let the caller see real errors.
PR #41d2c758c ("Fix unsafe gateway media path delivery") tightened
`validate_media_delivery_path` so that artifacts emitted by the agent
must live inside `MEDIA_DELIVERY_SAFE_ROOTS` (Hermes-managed cache
dirs) or an operator-allowlisted root via `HERMES_MEDIA_ALLOW_DIRS`.
Two kanban-notifier tests put their PDFs and PNGs under pytest's
`tmp_path`, which is correctly rejected by the new validator. They
started failing on main as soon as that PR landed:
FAILED tests/hermes_cli/test_kanban_notify.py::test_notifier_uploads_artifacts_on_completion
FAILED tests/hermes_cli/test_kanban_notify.py::test_notifier_artifact_delivery_skips_missing_files
Symptom in logs: "Skipping unsafe local file path outside allowed
roots". The validator is doing exactly what it should — the tests were
relying on the looser pre-fix behaviour.
Fix: add `HERMES_MEDIA_ALLOW_DIRS=tmp_path` to the `kanban_home`
fixture so artifacts under `tmp_path` are recognised as safe. This is
the same allowlist mechanism the operator-facing env var documents.
PR infographics belong in PR descriptions, not committed to the repo.
Removes the 13 archived directories under infographic/ and adds the path
to .gitignore so future generations don't accidentally land in-tree.
The fal.media URLs embedded in each PR's body remain the canonical
artifact — those PR descriptions are the storage.
The Kimi K2 branch added in the prior commit only emitted extra_body.thinking
and dropped reasoning_effort entirely. KimiProfile (api.moonshot.ai/v1) sends
both fields, and OpenCode Go proxies to the same Moonshot backend. Mirror that
shape on the Go path so /reasoning effort actually reaches Kimi.
- low/medium/high pass through verbatim
- xhigh/max clamp to high (Moonshot's max supported value)
- minimal / unknown effort → omit reasoning_effort, keep thinking on
- disabled / no config → unchanged
- DeepSeek branch unchanged
The two ACP slash-command tests that exercise `provider:model` routing
(`test_set_session_model_accepts_provider_prefixed_choice` and
`test_model_switch_uses_requested_provider`) relied on the live
`hermes_cli.models._KNOWN_PROVIDER_NAMES` / `_PROVIDER_ALIASES` module
state to parse `anthropic:claude-sonnet-4-6` into
`("anthropic", "claude-sonnet-4-6")`. If any earlier test in the same
xdist worker registers a custom provider that shadows `anthropic` or
otherwise mutates those globals, the parser falls into the
`detect_provider_for_model` branch and resolves to `custom` instead.
Observed once in CI on run 26326728502 / job 77505732299 as
`AssertionError: assert 'custom' == 'anthropic'` — could not reproduce
locally under per-file isolation, so the failing in-file order was
specific to a particular xdist scheduling.
Monkeypatching `parse_model_input` + `detect_provider_for_model` for
both tests removes the global-catalog dependency, so the tests now only
exercise what they were written to verify (the `requested_provider ->
runtime -> AIAgent kwargs` plumbing).
The reference entry now documents the truthy set
(``1`` / ``true`` / ``yes`` / ``on``) explicitly, matches the
falsy half (``0`` / ``false`` / ``no`` / ``off`` / empty string)
that the GHSA-5qr3-c538-wm9j fix re-aligned both the agent loader
and the dashboard web server around, and points readers at the
defence-in-depth rule that project plugins never have their
Python ``api`` file auto-imported by the dashboard regardless of
the env var.
GHSA-5qr3-c538-wm9j — half two of the bypass chain.
``_mount_plugin_api_routes`` imports each dashboard plugin's
manifest ``api`` field as a Python module via
``importlib.util.spec_from_file_location`` — arbitrary code
execution by design. Two primitives in the surrounding code
turned that "by design" RCE into a usable attack:
1. Absolute paths in the manifest swallow the plugin directory.
``Path('safe/dashboard') / '/tmp/evil.py'`` resolves to
``/tmp/evil.py``, so a single manifest line
``{"api": "/tmp/payload.py"}`` was enough to redirect the
importer at any Python file on disk.
2. ``..`` traversal in the manifest climbs out of the dashboard
directory. ``Path('plugins/safe/dashboard') /
'../../../tmp/evil.py'`` lands in ``/tmp/evil.py`` after
``resolve()`` — the static-asset handler
(``serve_plugin_asset``) already defends against this via
``is_relative_to``; the api-mount path didn't.
Fix at three layers so a regression in any one can't re-open the
advisory:
* New ``_safe_plugin_api_relpath`` validator runs at *discovery*
time and stores only sanitised relative paths on the plugin
entry's ``_api_file`` field. Absolute paths, ``..`` traversal,
empty / non-string values, and paths that ``resolve()`` outside
the plugin's ``dashboard/`` directory are rejected with a
warning naming the plugin. ``has_api`` follows the sanitised
value so the dashboard frontend doesn't render a fake "Backend
API" badge for plugins whose api was scrubbed.
* ``_mount_plugin_api_routes`` re-validates the resolved path
against the live filesystem just before the import — defence in
depth in case ``_dir`` is tampered with post-cache or a future
caller bypasses the discovery-time validator.
* Project plugins (``source == "project"``) are refused outright
for backend import. ``./.hermes/plugins/`` ships with the CWD,
so any threat model that includes "user opens a malicious repo"
treats it as attacker-controlled; project plugins can still
extend the UI via static JS/CSS but their Python ``api`` is no
longer auto-imported. Combined with the truthy env-gate fix
from the previous commit, the original advisory chain now
fails at two distinct choke points.
35 new tests across 5 classes covering every layer of the
GHSA-5qr3-c538-wm9j defence. Each class corresponds to one chokepoint
so a regression in any single layer is caught by the named class:
* ``TestProjectPluginsEnvGate`` (13 cases) — parametrised over both
the documented truthy values (``1`` / ``true`` / ``yes`` / ``on``
+ uppercase variants) and the previously-bypassing falsy strings
(``0`` / ``false`` / ``no`` / ``off`` / ``""`` / ``False``). The
falsy half is the direct env-bypass repro: pre-fix any non-empty
string enabled the project source.
* ``TestApiPathSanitizer`` (16 cases) — unit-level coverage of the
new ``_safe_plugin_api_relpath`` helper. Absolute paths
(``/etc/passwd``, ``/tmp/payload.py``, ``/usr/bin/python``),
``..``-traversal payloads (including nested ``subdir/../../..``),
and non-string / empty / whitespace-only values must all return
``None``. Safe relative paths (``api.py``, ``backend/routes.py``)
round-trip unchanged so legitimate plugins keep working.
* ``TestDiscoveryScrubsApiField`` (3 cases) — end-to-end through
``_discover_dashboard_plugins`` with a real manifest on disk.
Verifies that the cached plugin entry's ``_api_file`` is
scrubbed *at discovery time* (``None`` + ``has_api: False``) so
any downstream consumer can't be tricked into re-deriving the
unsafe path from cache.
* ``TestMountApiRoutesRefusesUntrusted`` (3 cases) — pokes
synthetic plugin entries with each refusal vector directly into
the cache and patches ``importlib.util.spec_from_file_location``
to assert it is *not* invoked for project-source / traversal
payloads, and *is* invoked normally for bundled / user plugins.
* ``TestEndToEndPocBlocked`` (1 case) — reproduces the original
advisory PoC: operator sets ``HERMES_ENABLE_PROJECT_PLUGINS=0``
believing project plugins are off, attacker plants a manifest in
CWD's ``.hermes/plugins/`` with ``api`` pointing at an absolute
payload path. Asserts that the importer is never called against
the payload path *and* that ``hermes_dashboard_plugin_evil`` is
not in ``sys.modules`` after the mount routine runs.
An autouse fixture busts ``_dashboard_plugins_cache`` before and
after each test so the production cache (populated by the
import-time ``_mount_plugin_api_routes()`` call) can't bleed in.
All 12 pre-existing dashboard-plugin tests in
``test_web_server.py`` still pass unchanged.
GHSA-5qr3-c538-wm9j — half one of the bypass chain.
``_discover_dashboard_plugins`` opted into the untrusted ``./.hermes/
plugins/`` source via ``if os.environ.get("HERMES_ENABLE_PROJECT_
PLUGINS"):`` — which is True for any non-empty string. ``=0``,
``=false``, ``=no``, ``=off`` all return non-empty strings and so
*enabled* the project source even though every operator (and the
agent loader, ``hermes_cli/plugins.py`` line 815) reads those values
as "disabled". An attacker who can land a manifest under the CWD's
``.hermes/plugins/`` directory — a malicious cloned repo, a worktree
checked out from a forked PR, a CI runner workspace — was therefore
guaranteed to get their manifest discovered the moment the user ran
``hermes dashboard`` from that directory, regardless of whether the
user thought they had project plugins disabled.
Switch to the shared ``utils.env_var_enabled`` helper used by the
agent loader so the gate accepts the documented truthy set (``1`` /
``true`` / ``yes`` / ``on``, case-insensitive) and treats everything
else — including ``0`` / ``false`` / ``no`` — as off.
Half two (path-traversal + project-source ``api`` import) lands in
the next commit. Together they break the RCE chain at two distinct
choke points so a future regression in either one alone can't
re-open the advisory.
PR #30136 review item O7: the plan doc was 3,191 lines — 5x the
size of any other plan in docs/plans/ and the largest reference
document in the repo. With the implementation shipped, most of
that content is either:
* The phase-by-phase TDD walkthrough (~2,800 lines): now canonical
in the PR commit log (`git log a957ef083..a6f7171a5`).
* The v2/v3 re-validation preambles: artifacts of the planning
process, no longer load-bearing.
* The full Open Questions deliberations with options A/B/C laid
out: collapsed into the Decision Log.
* The Rollout Plan and Estimated Timeline: history.
Trim to ~430 lines covering what readers actually need going
forward: the goal, architecture, scope, key design decisions
(D1–D9), risk register (now including the three risks surfaced
in PR review — `_s6_running` detection, svscanctl FIFO perms,
supervise control FIFO perms), the decision log including the
post-merge additions, and the verification checklist (now all
boxes ticked).
Header now reads 'Status: shipped' and points at the PR. The git
history preserves the full v3 plan for anyone who needs it.
PR #30136 review item O6: test_container_restart.py used fixed
`time.sleep(8)` calls after `docker restart` to wait for the
cont-init reconciler to finish. Fixed sleeps are slow when the
event happens fast and false-fail when the event happens slow.
Replace with two polling helpers:
* `_wait_for_path(container, path, kind='f' | 'd', deadline_s=...)`
— generic `test -f/-d` poller. Returns True on success, False on
timeout; callers assert with a clear message.
* `_wait_for_reconcile_log_mention(container, profile, ...)` — the
reconciler's per-profile log line is the canonical signal that
the cont-init reconcile has finished for that profile. Poll on
it instead of a sleep that hopes 8 seconds is enough.
The fixture-level setup wait is similarly migrated: it now polls
for `profile=default` in the boot log (every container always
gets a default-slot entry per item I1) and raises a clear timeout
error from the fixture if the container never finishes cont-init —
much better diagnostics than a mid-test KeyError.
The remaining `time.sleep()` calls are all internal interval_s
between probe attempts; no fixed wait points left.
PR #30136 review item O5: docker/entrypoint.sh is now a thin shim
that forwards to stage2-hook.sh — the real ENTRYPOINT is /init plus
main-wrapper.sh. External scripts that hard-coded entrypoint.sh as
the container's ENTRYPOINT will see the cont-init bootstrap happen
but the CMD will not be exec'd (because stage2-hook only handles
bootstrap; main-wrapper.sh handles the CMD passthrough).
Add a stderr warning explaining the new contract and pointing
callers at the migration path (drop the --entrypoint override).
The shim itself stays in place for one release cycle so the
deprecation isn't a hard break — anyone still invoking it sees
the warning in their logs and has time to migrate.
PR #30136 review noted the asymmetry: `register_profile_gateway`
used tmp_dir + rename to publish a new service slot atomically,
but the boot-time reconciler wrote files into the slot directly.
Same underlying concern (a concurrent s6-svscan rescan could
observe a half-populated directory), different code path.
Rewrite `container_boot._register_service` to mirror the manager:
build everything in `<scandir>/gateway-<profile>.tmp/`, then
`Path.replace` into place. If a previous interrupted run left a
`.tmp` sibling, it's cleaned up before the new build starts. If
the target already exists, it's removed before the rename so
`Path.replace` doesn't error on a non-empty target (Linux `rename`
overwrites empty targets only).
Three new tests: atomic publication leaves no .tmp leftovers,
overwriting an existing slot still leaves no .tmp leftovers, and
a stale .tmp from an interrupted run is cleaned up automatically.
PR #30136 review noted: container-boot.log was append-only with no
rotation. On a long-lived container with frequent restarts and
many profiles it would grow unboundedly (~80 B per profile per
reconcile pass).
Add a soft cap: when the file size hits 256 KiB (`_LOG_ROTATE_BYTES`,
≈3000 reconcile lines, ≈1 year of daily reboots × 5 profiles), the
current file is renamed to `container-boot.log.1` (replacing any
existing one) before new entries are appended. Worst case is two
files at ~512 KiB — well within visibility limits for grep/cat.
Rotation is intentionally simple (no logrotate or s6-log machinery
for one append-only file). Failures during rotation are logged via
the module logger and treated as non-fatal — we keep appending to
the existing file rather than dropping the reconcile entry. Three
new unit tests cover above-threshold rotation, below-threshold
non-rotation, and overwrite of an existing .1 file.
PR #30136 review caught: three `s6-setuidgid hermes sh -c "..."`
invocations in stage2-hook.sh interpolated $HERMES_HOME into a
nested shell context. Practically low-risk (a malicious HERMES_HOME
already requires container-launch privileges) but the cleaner
pattern is to invoke commands directly so the shell isn't a second
interpreter.
* `mkdir -p` of the data subdirs now runs directly via s6-setuidgid,
one path per arg.
* The .install_method stamp is written via `printf | tee` — also no
shell wrapper.
* The skills_sync invocation uses the venv's python by absolute path
instead of sourcing activate inside a shell. skills_sync.py doesn't
need anything from activate beyond sys.path, which the bin-stub
python already provides.
No behavior change. Just a smaller attack surface and a script
that's easier to read.
PR #30136 review caught: `_allocate_gateway_port()` in profiles.py
computed a SHA-256-derived port that was threaded through
`register_profile_gateway(profile, port=N)` →
`_render_run_script(profile, port, extra_env)` → and then **ignored**.
The rendered run script picked the bind port from the profile's
config.yaml (`[gateway] port = …`), never from the allocator. So
the entire allocator + parameter chain was dead code.
Remove:
* `hermes_cli.profiles._allocate_gateway_port` (deterministic
SHA-256 → [9200, 9800) — never used).
* `port` kwarg from `ServiceManager.register_profile_gateway`
(Protocol + Mixin + S6 implementation).
* `port` positional arg from `_render_run_script(profile, port,
extra_env)` — now `_render_run_script(profile, extra_env)`.
* The pass-through call in `profiles._maybe_register_gateway_service`.
config.yaml is now the single source of truth for gateway port
selection — matches reality and reduces the API surface. Three
explanatory comments in service_manager.py / profiles.py document
the retirement so future readers don't reach for the allocator and
find a ghost.
Tests: drop the three `_allocate_gateway_port` tests; update
fakes' signatures throughout test_service_manager.py and
test_profiles_s6_hooks.py to match the new no-port API.
PR #30136 review caught: docker-compose.yml still said "If you
override entrypoint, keep /opt/hermes/docker/entrypoint.sh in the
command chain." That was true under tini; under s6-overlay the
entrypoint is /init plus main-wrapper.sh, and entrypoint.sh is now
only a backward-compat shim.
Replace with an accurate description: /init must remain first in the
chain because it's PID 1 and runs the cont-init.d scripts (chown,
profile reconcile, dashboard toggle) before any service starts.
PR #30136 review caught a false positive: when HERMES_DASHBOARD was
unset, the dashboard run script did `exec sleep infinity`, so
`s6-svstat /run/service/dashboard` reported the slot as 'up'.
`hermes doctor` and any other s6-svstat-based health check saw the
dashboard as supervised-running even though no dashboard process
existed.
Add cont-init.d/03-dashboard-toggle: writes a `down` marker file
into `/run/service/dashboard/` when HERMES_DASHBOARD is falsy,
removes any leftover marker when it's truthy. s6-supervise honors
`down` by not starting the service, so s6-svstat reports 'down' —
matching reality.
The run script's HERMES_DASHBOARD case-statement stays in place as
a belt-and-suspenders guard, so the two layers can never disagree.
Two new integration tests lock the behavior: slot reports down
when unset; slot reports up when set to 1.
PR #30136 review caught: `S6ServiceManager.start/stop/restart` called
`subprocess.run(check=True)` on `s6-svc`, so any failure surfaced as
a raw `CalledProcessError` traceback. The two cases operators
actually hit are:
1. The service slot doesn't exist — most commonly because the user
typed a profile name wrong (`hermes -p typo gateway start`).
2. s6-svc itself fails — most commonly EACCES on the supervise
control FIFO when running unprivileged.
Both deserve named errors with actionable messages, not stacktraces.
Changes:
* Add `S6Error` base + two concrete errors in `hermes_cli.service_manager`:
- `GatewayNotRegisteredError(profile)` — carries the unprefixed
profile name; message: `no such gateway 'typo': register it
with `hermes profile create typo` first, or pass an existing
profile name via `-p <name>``.
- `S6CommandError(service, action, returncode, stderr)` — carries
the s6-svc rc and stderr; message: `s6-svc start on
'gateway-coder' failed (rc=111): <stderr>`.
* Factor lifecycle dispatch through `_run_svc(flag, label, name)`:
pre-checks that the service directory exists (raises
GatewayNotRegisteredError before invoking s6-svc), then runs
s6-svc and translates any CalledProcessError into S6CommandError.
* `_dispatch_via_service_manager_if_s6` in `hermes_cli.gateway`
catches both errors and prints `✗ <message>` + `sys.exit(1)`
instead of letting the exception bubble. The dispatch path that
used to dump a traceback at the user now gives an actionable
one-liner.
Tests: 6 new tests for the error types and their CLI rendering;
existing lifecycle test pre-seeds the slot directory before calling
`mgr.start` etc.
PR #30136 review caught: `hermes gateway start` (no `-p`) inside
the container resolves `_profile_suffix() == ""` → service name
`gateway-default`, but no such slot was ever registered. The Phase 4
profile-create hook only fired on `hermes profile create <name>`,
and the root profile (which lives at the top of $HERMES_HOME, not
under `profiles/`) was never one of those. So bare `hermes gateway
start` landed on `s6-svc -u /run/service/gateway-default` →
uncaught `CalledProcessError` → traceback to the user.
Changes:
1. `reconcile_profile_gateways` now always registers a
`gateway-default` slot before iterating named profiles. Its
prior state is read from `$HERMES_HOME/gateway_state.json`
(sibling to the profile root, not under `profiles/`); stale
runtime files there are swept the same way. Auto-up only if the
prior state was `running` — same rule as named profiles.
2. `S6ServiceManager._render_run_script` special-cases
`profile == "default"` to emit `hermes gateway run` with NO
`-p` flag. Passing `-p default` would resolve to
`$HERMES_HOME/profiles/default/` — a different profile that
almost certainly doesn't exist. The empty profile-suffix
convention is the dispatcher's contract and the run script has
to match.
3. A user-created `profiles/default/` collides with the reserved
root-profile slot; the reconciler now skips it with a warning
rather than producing two registrations of the same service name.
Action-list ordering is stable: `default` first, then named
profiles in directory order. Boot-log readers can rely on this.
Tests: 8 new dedicated default-slot tests plus updates to every
existing test that asserted against the action list (via the new
`_named_actions` helper that drops the always-present default
entry).
PR #30136 review caught that website/docs/user-guide/docker.md still
said "The dashboard side-process is **not supervised** — if it
crashes, it stays down until the container restarts." That was true
under tini but is the opposite of the s6 behavior this PR ships and
`test_dashboard_restarts_after_crash` proves.
Replace with a description of what users actually see now: automatic
restart by s6-overlay, new PID after a short backoff, logs via
`docker logs`. The standalone-container caveat carries forward
unchanged.
PR #30136 review caught that `hermes gateway stop --all` and
`... restart --all` were broken under s6. The Phase 4 dispatcher was
gated on `not stop_all` (and the symmetric restart_all), so `--all`
fell through to `kill_gateway_processes(all_profiles=True)`. pkill
SIGTERMed every gateway, s6-supervise observed the crashes, and
restarted every gateway ~1s later — net effect: `--all` *kicked*
gateways instead of *stopping* them.
Add `_dispatch_all_via_service_manager_if_s6(action)` that iterates
`mgr.list_profile_gateways()` and routes stop/restart through each
service slot. s6's `want up`/`want down` flips correctly, so a
stop persists. Partial failures are surfaced per-profile with a
running success count; the host pkill path is only reached when s6
isn't in play.
`start --all` isn't a CLI surface — the helper rejects it and
returns False (host code path can take over).
PR #30136 review caught a silent regression: the smoke-test action
overrode ENTRYPOINT to `/opt/hermes/docker/entrypoint.sh`, which the
s6-overlay migration reduced to a shim that just `exec`s the stage2
hook. stage2-hook ignores its CMD args, prints "Setup complete", and
exits 0 — so `hermes --help` and `hermes dashboard --help` never
ran. The #9153 regression guard was a green-always no-op.
Drop the override so the smoke test uses the image's real ENTRYPOINT
chain (`/init` + `main-wrapper.sh`), which is the actual production
startup path. `hermes --help` and `hermes dashboard --help` now run
through the full supervision tree and exercise the real argv routing.
PR #30136 review flagged the s6-overlay install as a supply-chain
regression vs the gosu source it replaced — `tianon/gosu` was
digest-pinned via `FROM ...@sha256:...`, but the three new
ADD/curl downloads had no integrity check at all.
Pin all three tarballs (noarch, symlinks-noarch, per-arch) to
upstream-published SHA256s via ARGs. Verification happens via
`sha256sum -c` against a single checksum file (avoids a piped-shell
hadolint DL4006 warning under dash). To bump S6_OVERLAY_VERSION,
fetch the four `.sha256` files from the new release and update
the ARGs — documented inline.
If upstream artifacts are tampered with mid-build, the build now
fails loudly at the verification step instead of silently
producing a tainted image.
The Dockerfile only ADD'd `s6-overlay-x86_64.tar.xz`, so the
`build-arm64` job in docker-publish.yml — which runs on
`ubuntu-24.04-arm` and publishes by digest — produced an image whose
`/init` couldn't exec on actual arm64 hosts. Apple Silicon and ARM
server users were getting a broken container.
Map BuildKit's `TARGETARCH` (`amd64` / `arm64`) to s6's kernel-arch
naming (`x86_64` / `aarch64`) inside the RUN step and fetch the
correct tarball via `curl` (`ADD`'s URL is evaluated at parse time,
before TARGETARCH substitution, so dynamic arch selection requires
RUN). The noarch + symlinks tarballs are architecture-independent
and stay as ADDs.
The audit case is now explicit: unsupported architectures fail loudly
at build time rather than producing a silently-broken image.
PR #30136 review surfaced two issues, both rooted in the same audit gap:
docker integration tests were running as root, not the unprivileged
`hermes` user (UID 10000) that the runtime actually uses via
`s6-setuidgid hermes`. Anything that probed PID-1 state or wrote to
the s6 control surface worked as root in the tests but was inert in
production.
Fixes:
1. `_s6_running()` previously called `Path("/proc/1/exe").resolve()`,
which is root-only readable. For UID 10000 the symlink yields
PermissionError, `resolve()` silently returns the unresolved path,
and `exe.name == "exe"` — so detection always returned False, the
service-manager runtime-registration path was inert, and every
`hermes profile create` / `hermes -p X gateway start` silently
skipped the s6 hook. Replace with `/proc/1/comm` (world-readable)
+ `/run/s6/basedir` (s6-overlay-specific) — both required, fail
closed.
2. `02-reconcile-profiles` now also chowns `/run/service/.s6-svscan/`
{control,lock} to hermes so `s6-svscanctl -a/-an` works without
root. Previously the directory chown stopped at `/run/service`
and the FIFO inside stayed root-owned, so `register_profile_gateway`
from hermes failed at the rescan-trigger step with EACCES — the
wrapper in profiles.py caught the exception and printed a swallowed
warning, so profile creation appeared to succeed while the slot
was rolled back.
Audit changes to flush this class of bug next time:
- Add `docker_exec` / `docker_exec_sh` helpers to `tests/docker/conftest.py`
that default to `-u hermes`. The module docstring explains why and
flags `user="root"` as opt-in only for tests that explicitly need
root (none currently do).
- Refactor every `docker exec` call in tests/docker/ through the new
helpers (test_dashboard.py, test_zombie_reaping.py, test_profile_gateway.py,
test_container_restart.py, test_s6_profile_gateway_integration.py).
- Add 5 unit tests covering `_s6_running` under various probe states
(both signals present; comm wrong; basedir missing; PermissionError
on /proc/1/comm; missing /proc — non-Linux). The PermissionError
test is the explicit regression guard for the original bug.
Known follow-up: the per-service `supervise/control` FIFO inside each
`/run/service/gateway-<profile>/supervise/` is created root-owned by
s6-supervise (which runs as root because s6-svscan is PID 1). `s6-svc
-u/-d/-t` from the hermes user will get EACCES on those. The audit
under `-u hermes` will reveal this in lifecycle tests — surfacing the
issue cleanly so it can be fixed in a focused follow-up (likely via a
small SUID helper or a polling chown loop in cont-init.d). The
detection + svscanctl fixes here are independent and complete on
their own.
Extends @briandevans's PR #17659 from {auth.json, auth.lock,
.anthropic_oauth.json} to also cover:
- HERMES_HOME/.env (provider API keys)
- HERMES_HOME/webhook_subscriptions.json (per-route HMAC secrets)
- HERMES_HOME/mcp-tokens/ (OAuth token directory; dir
+ everything inside)
…AND iterates over both _hermes_home_path() AND _hermes_root_path()
so profile-mode runs (HERMES_HOME = <root>/profiles/<name>) also block
<root>/{auth.json, .env, mcp-tokens/, ...}. Same widening shape as the
write-deny side already does (#15981, #14157).
Explicitly NOT a security boundary. Per the personal-assistant trust
model, the terminal tool runs as the same OS user and can `cat
auth.json` directly. This read-deny exists as defense-in-depth:
- Models that respect tool denials empirically tend to stop rather
than reach for the shell.
- The denial surfaces an audit trail when something tries to read
credentials — easier to spot in logs than a generic `cat`.
Docstring + error message both flag this as defense-in-depth so future
contributors don't mistake it for a real security boundary and don't
re-decline reports that propose the same fix shape.
Absorbs the .env and mcp-tokens/ coverage from @tomqiaozc's parallel
PR #8055 (closed-as-duplicate, credited).
Co-authored-by: Tom Qiao <zqiao@microsoft.com>
read_file_tool resolves relative paths against TERMINAL_CWD (or the
task's live terminal cwd), but the prior call passed the original
unresolved string to get_read_block_error. That function's own
resolve() is anchored at the Python process cwd, so when a task's
TERMINAL_CWD pointed at HERMES_HOME and the agent issued read_file
on the relative path "auth.json", the credential-store denylist was
never reached and the file was read normally.
Pass the already-resolved absolute path string at the file_tools call
site, document the contract on get_read_block_error, and add a
read_file_tool-level regression test that pins the relative-path
case under TERMINAL_CWD == HERMES_HOME.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`get_read_block_error` previously only denied reads inside
`${HERMES_HOME}/skills/.hub`, which left `auth.json` (provider OAuth
state + plaintext API keys) and `.anthropic_oauth.json` (Anthropic PKCE
tokens) directly readable by the agent. A prompt-injection reaching
`read_file` could exfiltrate active provider credentials in plaintext.
Mode-0600 file permissions only protect against *other Unix users* —
the agent runs as the file's owner, so `read_file` is unaffected.
Extend the existing deny list with the three credential paths
identified in #17656 (`auth.json`, `auth.lock`, `.anthropic_oauth.json`).
The check uses the same `Path.resolve()` pattern as `skills/.hub`, so
symlink/path-traversal indirection is caught too. The agent doesn't
need to read these directly — `auxiliary_client` and `credential_pool`
consume them through process env / OAuth flows that bypass `read_file`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #6656 added rel_path + \x00 prefixing to ``bundle_content_hash`` so a
filename swap between two files in a bundle changes the digest. But it
only patched the in-memory side — ``content_hash`` in ``tools/skills_guard.py``
(the on-disk equivalent) still hashed file contents only.
These two functions need to stay symmetric: ``check_for_skill_updates``
compares the disk hash of an installed skill against the bundle hash
of the upstream copy. With the asymmetric fix, every clean install
showed as drifted because the digests no longer matched
(2 existing tests in ``test_skills_hub.py`` started failing as soon as
the contributor's change landed).
Apply the same ``rel_path + \x00 + content`` shape to the disk-side
function. Both functions now produce the same digest for the same skill
content laid out two ways. Documented the symmetry invariant in the
docstring so a future change to either function knows to touch both.
Also adds tests/tools/test_pr_6656_regressions.py with 10 regression
tests covering all three fixes salvaged in PR #6656:
- uninstall_skill path traversal (4 cases: parent segments, absolute
paths, symlink escape, legitimate skill)
- bundle_content_hash filename swap detection (4 cases: in-memory
swap, identity, disk-side swap, bundle↔disk symmetry)
- list_pending lock contract (2 cases: source-grep contract, smoke)
Also fixes AUTHOR_MAP entry for @aaronlab — their commit email
(1115117931@qq.com) maps to "aaronagent" which isn't a real GitHub
login, so changelog @mentions would 404.
- skills_hub: validate that uninstall_skill's install_path resolves
inside SKILLS_DIR before calling shutil.rmtree, preventing recursive
deletion of arbitrary directories via poisoned lock.json entries
- skills_hub: include file paths (not just contents) in
bundle_content_hash so swapping filenames between files changes the
hash, strengthening update-detection integrity
- pairing: wrap list_pending() in self._lock so _cleanup_expired() file
writes don't race with concurrent generate_code()/approve_code() calls
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Follow-up to PR #28832 — the dashboard plugin routes now accept slashed
names like `observability/langfuse` and `image_gen/openai`, but
`_sanitize_plugin_name` still rejected forward slash and so dashboard
update + remove on those plugins fell through to '404 not found' even
though they exist on disk.
Adds an opt-in `allow_subdir=True` flag that:
- Permits internal forward slashes (category-namespaced plugin keys
emitted by `_discover_all_plugins`).
- Strips leading and trailing slashes.
- Still rejects `..` and backslash, and still asserts the resolved
target lives inside `plugins_dir`.
Opted in at the two read-paths that operate on installed plugins:
`_require_installed_plugin` (CLI update/remove) and
`_user_installed_plugin_dir` (dashboard update/remove). The install
path keeps the default (`allow_subdir=False`) because freshly-cloned
plugins always land top-level under `~/.hermes/plugins/<name>/`.
Adds 6 targeted unit tests covering the new flag's allow/reject matrix.
Removes the global `uppercase` + `font-mondwest` from the App.tsx root
that forced every page to opt-out, replaces stacked-alpha text colors
with semantic tokens for WCAG-AA contrast across all 7 themes, and
applies the new `text-display` utility from @nous-research/ui@0.16.0
on intentional brand chrome (page titles, sidebar headings, segmented
filters) only. Bumps every sub-12px arbitrary text size to text-xs.
Also widens the dashboard plugin routes (/api/dashboard/agent-plugins/
{name:path}/...) so category-namespaced plugins like observability/
langfuse and image_gen/openai can be enable/disabled from the dashboard
— previously the FE encodeURIComponent-ed the slash and the backend
{name} route rejected it. _validate_plugin_name still blocks .. and
backslash, and strips leading/trailing slash.
Touches sessions/env/keys page chrome and adds two new i18n keys
(`overview`, `showMore`/`showLess`) across all 18 locales.
Squashes 19 commits from PR #28832.
Co-authored-by: Hermes <noreply@nousresearch.com>
- test_browser_secret_exfil: mock _run_browser_command instead of
launching real Chrome (secret check is pre-launch, browser is
irrelevant to the assertion)
- test_web_server: add time.sleep(0.05) after pub.send_text() to
yield the event loop before receive_text(). TestClient's sync mode
can race the broadcast handler otherwise, hanging the test.
walli