Skip to content

feat(gateway): HERMES_MEDIA_EXTRA_ROOTS to widen /api/media read roots - #42778

Closed
maxtrigify wants to merge 1 commit into
NousResearch:mainfrom
maxtrigify:feat/media-extra-roots
Closed

feat(gateway): HERMES_MEDIA_EXTRA_ROOTS to widen /api/media read roots#42778
maxtrigify wants to merge 1 commit into
NousResearch:mainfrom
maxtrigify:feat/media-extra-roots

Conversation

@maxtrigify

Copy link
Copy Markdown

Summary

GET /api/media (remote image display) is confined to ~/.hermes/{images,screenshots,cache}. That's the right default, but it means a desktop client connected to a remote gateway can't display images an agent generated outside that tree — e.g. into its project/output directory. Those requests return 403 {"detail":"Path outside media roots"} and render as dead "Open …png" links instead of inline images.

This adds an opt-in HERMES_MEDIA_EXTRA_ROOTS env var (an os.pathsep-separated list of absolute directories) that operators can set to allow additional read roots.

Why

Common multi-host setup: gateways on one machine, desktop on another. The agent writes generated images (charts, cards, screenshots) into its working/output dir — outside ~/.hermes — so they can't be displayed remotely today. There's no way to widen the allowlist without editing source.

Safety

The widening is deliberate and scoped:

  • Each extra root is resolved symlink-safe (same .resolve() + root in target.parents check as the built-in roots).
  • The image-extension allowlist and size cap are unchanged — this only adds directories, it doesn't relax the other guards.
  • Default behavior is identical when the env var is unset (single-profile / single-host users are unaffected).

Changes

  • hermes_cli/web_server.py_media_serve_roots() appends HERMES_MEDIA_EXTRA_ROOTS entries.
  • tests/hermes_cli/test_web_server.py — new test: a dir is 403 by default, served once opted in via the env var, and a sibling dir not listed stays 403 (scoping).

Verified: pytest tests/hermes_cli/test_web_server.py -k get_media → 6 passed.

🤖 Generated with Claude Code

GET /api/media is confined to ~/.hermes/{images,screenshots,cache}, so a remote
desktop client can't display images an agent generates outside that tree — e.g.
into a project/output dir — which 403 "Path outside media roots" and render as
dead links. Let operators opt extra directories in via HERMES_MEDIA_EXTRA_ROOTS
(os.pathsep-separated absolute paths). Each is still resolved symlink-safe and
the suffix/size guards are unchanged, so this widens the allowlist deliberately
without bypassing the other protections.

Common multi-host case: gateways on one machine serving a desktop on another,
where the agent writes generated images into its working/output directory.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels Jun 9, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified — no issues found.

Security review notes on the HERMES_MEDIA_EXTRA_ROOTS widening:

  • The existing _path_is_within boundary check and suffix/size guards remain in effect, so the widening is additive-only — operators explicitly opt in directories, and files outside those directories remain 403.
  • Symlinks are resolved before the boundary check (line 830+), so a symlink inside an extra root that points outside is correctly rejected.
  • The test coverage confirms scoped widening: an opted-in directory is served, a sibling directory stays forbidden.

The opt-in model (explicit env var, no wildcard expansion, default empty) is the right trade-off for a media-serving endpoint.

LGTM on the security posture.

@liuhao1024

Copy link
Copy Markdown
Contributor

Positive Verification — Media Serving Security Review

Reviewed the full diff in hermes_cli/web_server.py and test.

Changes verified:

  • Extra roots from HERMES_MEDIA_EXTRA_ROOTS are appended before the existing symlink-safe resolution loop — no path traversal bypass
  • The sibling-dir test confirms the widening is strictly scoped (only opted-in dirs are served)
  • raw.strip() handles whitespace in env var values correctly
  • os.pathsep is the correct cross-platform separator for path lists
  • Operator-controlled env var (not user-facing web input) — appropriate threat model

No issues found. Clean opt-in mechanism for operators needing wider media serving roots.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the scoped implementation and the preserved symlink, suffix, and size checks. This is an automated hermes-sweeper review.


Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants