Skip to content

fix(gateway): map Docker MEDIA paths through bind mounts - #27779

Closed
LEAFERx wants to merge 2 commits into
NousResearch:mainfrom
LEAFERx:fix/outbound-docker-media-path
Closed

fix(gateway): map Docker MEDIA paths through bind mounts#27779
LEAFERx wants to merge 2 commits into
NousResearch:mainfrom
LEAFERx:fix/outbound-docker-media-path

Conversation

@LEAFERx

@LEAFERx LEAFERx commented May 18, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes outbound MEDIA:/container/path delivery when Hermes uses a Docker terminal backend but the messaging gateway sends files from the host process.

Before this PR, an agent running in Docker could create a file under a bind-mounted container path (for example /agent-artifacts/report.pdf) and reply with MEDIA:/agent-artifacts/report.pdf, but the host gateway would look for that container path on the host and fail. This PR maps configured Docker bind mounts back to host-readable paths before platform delivery.

The approach is centralized and conservative:

  • Parse Docker bind mounts from terminal.docker_volumes / TERMINAL_DOCKER_VOLUMES.
  • Ignore named volumes and unsafe/unusable mount specs that do not map to a host path.
  • Translate only paths under a configured non-root container mount.
  • Prefer the longest matching container mount for nested bind mounts.
  • Preserve existing fallback behavior when translation is unavailable.
  • Keep non-Docker and already host-readable MEDIA: paths unchanged.

Related Issue

No dedicated tracking issue.

Related PRs:

Duplicate check found no upstream PR implementing automatic outbound bind-mount mapping for MEDIA: delivery.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/media_paths.py
    • Adds Docker bind-mount parsing and host-path resolution for outbound media paths.
    • Rejects named volumes / root container mounts for this translation path.
    • Uses realpath/commonpath containment checks so traversal and symlink escapes do not map outside the configured host mount.
  • gateway/run.py, gateway/platforms/base.py, gateway/platforms/telegram.py
    • Applies outbound media path resolution before delivery and improves Docker-path diagnostics.
  • hermes_cli/config.py
    • Updates docker_volumes inline guidance to document mounted container paths for gateway media delivery.
  • website/docs/user-guide/configuration.md
    • Documents the host-visible bind mount pattern and clarifies that /output is a convention, not a requirement.
  • website/docs/user-guide/messaging/telegram.md
    • Updates Telegram media guidance for Docker-backed agents.
  • tests/gateway/test_send_image_file.py
    • Adds regression coverage for bind-mount translation, nested/longest-prefix selection, named-volume rejection, traversal, symlink escape, and platform delivery behavior.
  • tests/gateway/test_runner_startup_failures.py
    • Updates gateway startup/diagnostic expectations.

How to Test

Manual reproduction path:

  1. Configure Docker terminal backend with a host-visible bind mount, for example:
    terminal:
      backend: docker
      docker_volumes:
        - "/home/user/.hermes/cache/documents:/agent-artifacts"
  2. From inside the Docker-backed agent/tool environment, create /agent-artifacts/report.pdf.
  3. Reply with MEDIA:/agent-artifacts/report.pdf.
  4. Confirm the host gateway sends /home/user/.hermes/cache/documents/report.pdf.
  5. Confirm unmapped container-only paths still fall back to the existing missing-file warning.

Commands run locally:

git diff --check upstream/main...HEAD
# passed

python -m pytest tests/gateway/test_send_image_file.py tests/gateway/test_runner_startup_failures.py -q
# 55 passed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 14.6.1 (Darwin 23.6.0, arm64)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
    • N/A: no new config key or default was added; only existing docker_volumes docs/comments changed.
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
    • N/A: no contributor workflow/architecture instructions changed.
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A
    • N/A: no tool schema changed.

For New Skills

N/A — this PR does not add or modify a skill.

Screenshots / Logs

Focused regression tests:

55 passed in 3.62s

Independent read-only Codex review verdict: accept, no blocking issues.

server added 2 commits May 17, 2026 22:35
Resolve outbound MEDIA file paths emitted from Docker terminal sandboxes by translating configured bind-mounted container paths back to host-visible paths before gateway delivery.

Add regression coverage for mapped paths, common Docker volume options, path-boundary handling, existing host-visible files, and malformed volume config.
@LEAFERx
LEAFERx force-pushed the fix/outbound-docker-media-path branch from 3fcea68 to e0faa47 Compare May 18, 2026 05:35
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery backend/docker Docker container execution platform/telegram Telegram bot adapter labels May 18, 2026
@teknium1

teknium1 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused bug fix. I verified the premise still holds on current main: gateway MEDIA extraction currently appends the literal expanded path at gateway/platforms/base.py:2972, while the Docker gateway startup path only warns about risky media delivery at gateway/run.py:2274-2318.

Problems

  • Current main now deliberately skips crafted MEDIA paths when expansion fails at gateway/platforms/base.py:2971-2976. The PR resolver falls back to returning the original path on resolver failure at gateway/media_paths.py:186-191, so a direct salvage would need to preserve the current skip behavior.
  • The extract_media hunk is stale against current main. Current main uses shared MEDIA_TAG_CLEANUP_RE plus protected-span and JSON masking at gateway/platforms/base.py:2952-2994; the PR-head hunk around gateway/platforms/base.py:2140-2153 should not replace that newer logic wholesale.

Suggested changes

  • Keep the Docker bind-mount resolver, but wire it into current main’s existing extract_media loop without removing the masking/cleanup invariants.
  • Add/adjust tests on the current MEDIA test surface for Docker bind-mount mapping and for the existing crafted null-path case.

This is an automated hermes-sweeper review.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused Docker gateway fix. The underlying gap is still present on current main: extract_media() retains the literal container path at gateway/platforms/base.py:3658, and dispatch validates that path on the host at gateway/platforms/base.py:4927.

Problems

  • The extraction hunk predates the current parser. Current main masks protected/JSON spans and has a separate extensionless-MEDIA path at gateway/platforms/base.py:3645-3696; salvage must preserve those flows rather than replace the old extraction line directly.
  • Current main deliberately skips crafted paths when expanduser() fails at gateway/platforms/base.py:3657-3662. The resolver fallback in the proposed gateway/media_paths.py returns the malformed original path instead, conflicting with the regression contract at tests/gateway/test_platform_base.py:1801-1806.

Suggested changes

  • Wire bind-mount translation into both current extraction paths before host validation, while retaining the current masking and failure behavior.
  • Add the Docker mapping cases to the current parser test surface in tests/gateway/test_platform_base.py, including extensionless directives and the existing crafted-null-path invariant.

This is an automated hermes-sweeper review.

Comment thread gateway/platforms/base.py
path = path.lstrip("`\"'").rstrip("`\"',.;:)}]")
if path:
media.append((os.path.expanduser(path), has_voice_tag))
media.append((resolve_outbound_media_path(path), has_voice_tag))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replacement targets the pre-current extraction loop. Please salvage the resolver into current main's masked known-extension and extensionless passes (gateway/platforms/base.py:3645-3696) while preserving the current skip-on-expanduser()-failure behavior.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

teknium1 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closing with credit: you were the FIRST to submit a fix for the outbound container-path MEDIA delivery bug (May 18, well before the tracking issues existed). The fix merged via #81746 (merge commit 238351a) used #37207's mount-derived approach as the salvage base — it translates at the existing validate_media_delivery_path chokepoint without new config keys, which fit the config-surface constraints better than the allowlist design here. Your first-submitter credit is acknowledged in the merged PR and in #37207's close. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/docker Docker container execution comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants