Skip to content

fix(hermes): block in-container self-updates - #3162

Open
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/hermes-block-container-self-update
Open

fix(hermes): block in-container self-updates#3162
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/hermes-block-container-self-update

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Why this matters

Issue #2380 documents a production-reachable way to bypass ODS's Hermes image pin: the authenticated Hermes UI sends POST /api/hermes/update, which upgrades hermes-agent inside the container's writable virtualenv. A normal ods restart hermes preserves that mutated filesystem. In the reported macOS install, the upgraded binary enforced a newer dashboard-auth contract, stopped listening on internal port 9119, and left hermes-proxy returning 502s.

This prevention change makes the ODS-owned Caddy boundary reject exactly that mutating POST with HTTP 409 and an actionable use ods update message. Status, chat, WebSocket, auth, and every other Hermes route remain unchanged. Container/image upgrades stay owned by the ODS lifecycle, where the image pin, Compose recreation, and release checks apply.

Behavioral invariant: requests crossing the supported ODS proxy may not mutate the container-managed Hermes installation; supported Hermes APIs must continue to proxy normally.

This addresses the prevention track of #2380. Recovery for containers already mutated remains a separate independently useful lifecycle scope rather than being implied by this route guard.

Overlap check

Searched open and closed PRs for Hermes in-container self updater, Hermes update endpoint Caddy, Hermes self updater proxy, Hermes recover force-recreate, and issue #2380. No PR blocks this endpoint. Open #1976 changes Basic-auth integration for newer Hermes code and does not prevent writable-venv mutation; it also remains unmerged. The earlier #1964 auth-gate report likewise does not own the self-update path.

Changed production file searched: ods/extensions/services/hermes-proxy/Caddyfile. Existing proxy/auth/body-limit PRs were inspected through the current contract; none cover /api/hermes/update.

Regression test

tests/test-hermes-proxy-caddyfile.sh treats the shipped Caddyfile as the public routing boundary. It requires a POST-only exact-path matcher, verifies the rejection runs before forward_auth/upstream forwarding, and requires HTTP 409.

A live exact-version probe additionally started caddy:2.11.3-alpine with the candidate config and sent the reported request:

POST /api/hermes/update
HTTP/1.1 409 Conflict
Hermes self-update is disabled in ODS; use ods update.

No dashboard-api or Hermes upstream was running, so the 409 also proves the request was handled locally rather than forwarded.

Validation

  • bash tests/test-hermes-proxy-caddyfile.sh from clean LF integration worktree ? passed all 5 proxy assertions
  • python3 tests/contracts/test-network-exposure-contracts.py ? 11 passed
  • docker run --rm ... caddy:2.11.3-alpine caddy validate --config /etc/caddy/Caddyfile ? Valid configuration
  • live pinned-Caddy POST probe ? returned 409 with the ODS lifecycle message
  • git diff --check ? passed

The temporary validation container was stopped and removed. No live Hermes install was upgraded or repaired in this session; validation proves proxy prevention, not recovery of already-mutated container state.

Tradeoffs and rollback

HTTP 409 is deliberate: the endpoint exists, but conflicts with the container-managed deployment contract. Restricting the matcher to POST avoids hiding any future read-only status route at the same path. Reverting restores the UI updater but also restores the exact pin-bypass and auth-gate failure reported in #2380.

Batch compatibility

This PR was validated on synthetic integration head 9af795fc, which applies #3158 through #3167 in numeric order on upstream/main (6ff9b4fc). Combined make lint, make test, make smoke, make simulate, and all 418 BATS cases passed (one root-specific permission assertion skipped by design).

Recommended merge order: #3158 ? #3159 ? #3160 ? #3161 ? #3162 ? #3163 ? #3164 ? #3165 ? #3166 ? #3167. The only manual reconciliation observed was the adjacent Makefile test insertion shared by #3164 and #3166; retain both test-unix-restart-recreate-env.sh and test-chat-error-exit-parity.sh lines. Production code merged automatically across the full batch.

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