Skip to content

feat: surface loopback sidecar diagnostics#4612

Closed
santastabber wants to merge 1 commit into
nesquena:masterfrom
santastabber:extension-sidecar-monitor-clean
Closed

feat: surface loopback sidecar diagnostics#4612
santastabber wants to merge 1 commit into
nesquena:masterfrom
santastabber:extension-sidecar-monitor-clean

Conversation

@santastabber

@santastabber santastabber commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Manifest-bundled extensions can already declare local assets and run trusted JavaScript in the WebUI browser origin.
  • Sidecar-class extensions need an operator-visible way to confirm that their declared local companion service was parsed and reachable.
  • This keeps the boundary narrow: it extends the existing read-only extension diagnostics/status surface and Settings panel, but does not add backend proxying, install/manage mutation, or new extension-owned server routes.

What Changed

  • Parse optional per-extension sidecar declarations from enabled manifest extension entries:
    • supports only type: "loopback";
    • accepts only http(s) origins on 127.0.0.1, localhost, or [::1];
    • rejects userinfo, paths, queries, fragments, unsafe characters, external hosts, and malformed ports;
    • defaults missing health_path to /health and rejects unsafe explicit health paths.
  • Add sanitized sidecar diagnostics to GET /api/extensions/status:
    • sidecars[] with id, name, type, origin, health_path, and health_url;
    • counts.sidecars and manifest.sidecar_count;
    • stable warning codes for rejected/unsupported/truncated sidecars without returning raw rejected origins or paths.
  • Render a read-only Loopback sidecars card in Settings → Extensions.
  • Check declared sidecar health directly from the browser with fetch(health_url, { credentials: 'omit', cache: 'no-store' }) and a short timeout.
  • Document the sidecar declaration shape, sanitization rules, and no-proxy/no-cookie boundary.

Why It Matters

Operators can now see whether sidecar-class extensions such as Desktop Companion declared a local companion service correctly and whether the local health endpoint is reachable, without WebUI becoming a sidecar proxy or forwarding authenticated WebUI cookies to local helper processes.

Verification

  • ./scripts/test.sh tests/test_extension_status_endpoint.py tests/test_extensions_settings_panel.py -q
    • Result: 33 passed
  • python3 -m py_compile api/extensions.py tests/test_extension_status_endpoint.py tests/test_extensions_settings_panel.py
  • node --check static/panels.js
  • git diff --check origin/master...HEAD
  • Isolated API/browser smoke with temporary WebUI state and a mock loopback sidecar:
    • /health returned 200;
    • /api/extensions/status returned the accepted Desktop Companion sidecar with the expected health URL;
    • an external sidecar origin was rejected with only sidecar_origin_rejected / manifest:sidecars and no raw host/port/path leakage;
    • Settings → Extensions rendered the Loopback sidecars card and the health badge changed to healthy;
    • browser console probe reported no captured errors.

Risks / Follow-ups

  • Browser health checks depend on the sidecar allowing the browser request through CORS. A CORS/network/timeout failure is shown as unreachable/blocked; WebUI intentionally does not proxy the request.
  • This does not start, install, enable, disable, or uninstall sidecars or extensions.
  • The sidecar list is capped at the same defensive bound as extension asset URLs to keep diagnostics small.

Contract Routing

Task type: extension diagnostics / trusted loopback sidecar visibility.

Touched areas:

  • extension manifest diagnostics
  • Settings → Extensions read-only diagnostics panel
  • extension documentation
  • extension security/static regression tests

Relevant public docs:

  • AGENTS.md
  • CONTRIBUTING.md
  • docs/CONTRACTS.md
  • docs/EXTENSIONS.md

Scope boundaries:

  • Does not add extension backend routes.
  • Does not proxy sidecar traffic through WebUI.
  • Does not send WebUI credentials or cookies to sidecars.
  • Does not add install, enable/disable, uninstall, marketplace, or dependency-system behavior.
  • Does not expose local filesystem paths, raw env values, rejected sidecar origins, rejected health paths, or health response bodies.

Evidence needed before claiming done:

  • focused extension endpoint/panel tests pass;
  • syntax/static checks pass;
  • isolated API/browser smoke confirms sanitized status and sidecar health rendering.

@santastabber santastabber marked this pull request as ready for review June 21, 2026 10:02
@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR surfaces read-only loopback sidecar diagnostics in the extensions system. The server parses optional sidecar declarations from manifest extension entries, sanitizes them (loopback-only origins, safe health paths, no raw rejected values in output), and exposes them through GET /api/extensions/status. The browser-side panel then performs health checks directly from the client with credentials: 'omit'.

  • API (api/extensions.py): Two new sanitizers (_normalize_loopback_sidecar_origin, _normalize_sidecar_health_path) gate what appears in the diagnostics response, with stable warning codes replacing any rejected raw values. The existing early-loop-break (if scripts_full and stylesheets_full: break) is removed so sidecar processing continues for all entries even after asset lists fill up — correct given the 64 KB manifest cap.
  • Frontend (static/panels.js): loadExtensionsPanel captures the sidecar monitor sequence at increment time and threads it through _renderExtensionsPanel_monitorExtensionSidecars_checkExtensionSidecarHealth, with stale-check guards at both the render boundary and after each async fetch, which correctly addresses the race noted in the prior review.
  • Tests: 33 tests cover accepted sidecars, disabled entry skipping, rejection of non-loopback origins/invalid paths/unsupported types, truncation, and UI code shape assertions.

Confidence Score: 5/5

Safe to merge; the sidecar surface is strictly read-only, all sanitization paths reject external hosts and unsafe content before anything reaches diagnostics or the browser fetch, and credentials are omitted from health checks.

The origin/health-path sanitizers are thorough (loopback-only allowlist, full percent-encoding expansion, no raw rejected values in output). The browser health check correctly threads the seq counter through the entire async chain, and the stale-check guards at both the render boundary and post-fetch prevent stale callbacks from updating a newer panel. Tests cover the full rejection matrix and confirm no leakage of rejected values.

No files require special attention.

Important Files Changed

Filename Overview
api/extensions.py Adds loopback sidecar parsing with two thorough sanitizers (origin and health path), warning-code-only rejection, and the sidecar list threaded through the existing diagnostics tuple. Early-break removal is intentional and bounded by the 64 KB manifest cap.
static/panels.js Adds sidecar card rendering and browser-side health monitor; seq is captured at increment time in loadExtensionsPanel and threaded explicitly through _renderExtensionsPanel/_monitorExtensionSidecars/_checkExtensionSidecarHealth, with stale-check guards at the render boundary and post-fetch — the concern from the prior review is addressed.
static/style.css Adds scoped sidecar list/row/badge CSS; responsive overrides included; no existing rules overridden.
tests/test_extension_status_endpoint.py Adds 8 focused tests covering accepted sidecars (127.0.0.1, localhost, [::1]), disabled entry skipping, non-loopback rejection without raw value leakage, invalid health path rejection, unsupported type rejection, and truncation; existing tests updated for new tuple unpacking.
tests/test_extensions_settings_panel.py Adds panel shape assertions (sidecar card, esc() usage, fetch options, seq threading, no response body reading) plus docs content assertions; good static verification of security-sensitive rendering patterns.
docs/EXTENSIONS.md Documents sidecar declaration shape, sanitization rules, no-proxy/no-cookie boundary, and updated diagnostics field descriptions; multi-line bash example collapsed to single line (cosmetic).

Reviews (4): Last reviewed commit: "feat: surface loopback sidecar diagnosti..." | Re-trigger Greptile

@cutter-sh

cutter-sh Bot commented Jun 21, 2026

Copy link
Copy Markdown

🎬 Cutter preview — PR #4612

/settings/extensions
/settings/extensions — Extensions diagnostics now include a Loopback sidecars card listing declared local companions and their health.

@santastabber santastabber force-pushed the extension-sidecar-monitor-clean branch from b06cf40 to 16b7bc4 Compare June 21, 2026 10:17
@santastabber santastabber marked this pull request as draft June 21, 2026 10:18
@santastabber santastabber marked this pull request as ready for review June 21, 2026 15:02
@nesquena-hermes nesquena-hermes added the size:L Large PR (>10 files or >250 LOC) label Jun 21, 2026
@santastabber

santastabber commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current origin/master and refreshed the branch.

New head: 7080929f3727cedb4e56433b0a71cf446f8f12a5

Verification after rebase:

  • ./scripts/test.sh tests/test_extension_status_endpoint.py tests/test_extensions_settings_panel.py -q33 passed
  • python3 -m py_compile api/extensions.py tests/test_extension_status_endpoint.py tests/test_extensions_settings_panel.py
  • node --check static/panels.js
  • git diff --check origin/master...HEAD

Greptile's previous P2 seq-race finding was addressed before this rebase: the sidecar monitor seq is captured at loadExtensionsPanel start and passed through render/monitor callbacks, with a static regression pinning that shape.

@santastabber santastabber force-pushed the extension-sidecar-monitor-clean branch from 16b7bc4 to 7080929 Compare June 21, 2026 15:19
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Heads-up @franksong2702 — this PR adds the loopback "sidecar" diagnostics surface to Settings → Extensions (manifest extensions declaring a type: loopback companion service, surfaced as a read-only health card with a browser-side health probe). Since you're driving the Desktop Companion direction, flagging it so the sidecar manifest shape here (type / origin / health_path validation, loopback-only) stays consistent with where the Companion is headed. Nathan has concept-approved it; it's in the release pipeline now. If the manifest contract should evolve to fit the Companion, easiest to land that as a follow-up so this diagnostics surface isn't blocked.

nesquena-hermes pushed a commit that referenced this pull request Jun 21, 2026
The CSP-loopback-alignment fix added http(s)://[::1]:* to connect-src, but CSP
host-source grammar can't express a port wildcard on a bracketed IPv6 literal —
Chromium rejects '[::1]:*' as an invalid source, which browser-smoke flags as a
console error (CI fail). Keep the valid https://127.0.0.1:*/localhost:* additions;
drop the IPv6 wildcard entries. IPv6 [::1] sidecars are still accepted by the
validator (displayed in the card); their browser health probe surfaces as
'blocked' under CSP unless an operator adds a specific-port [::1]:<port> via
HERMES_WEBUI_CSP_CONNECT_EXTRA. Tests updated to match + assert [::1]:* absent.
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.564 🚀 — thank you @santastabber.

The loopback sidecar diagnostics surface landed. Review applied three hardening fixes during the gate:

  1. Encoded query/fragment: _normalize_sidecar_health_path now re-rejects decoded ?/# so /health%3Ftoken=… can't survive past the documented query ban into the probed URL.
  2. CSP alignment: added https://127.0.0.1:* / https://localhost:* to connect-src so accepted https loopback sidecars aren't CSP-blocked.
  3. Browser-smoke fix: the initial CSP attempt added [::1]:*, which Chromium rejects as an invalid CSP source (caught by the browser-smoke CI). Dropped it — IPv6 [::1] sidecars are still accepted and displayed, but since CSP host-source grammar can't wildcard a bracketed-IPv6 port, the IPv6 health probe surfaces as "blocked" unless an operator adds a specific-port entry via HERMES_WEBUI_CSP_CONNECT_EXTRA.

Review trail: Codex regression gate SAFE TO SHIP (after fixes), Opus advisor SAFE — ship (allowlist validation, rebuild-not-echo, no server SSRF, uniform escaping), full suite 9987 passed, browser-smoke green. Regression tests added for all three findings.

github-actions Bot pushed a commit to Ulysses0706/hermes-webui that referenced this pull request Jun 21, 2026
…quena#4632)

* stage nesquena#4612 (santastabber): surface loopback sidecar diagnostics + CHANGELOG

Settings -> Extensions surfaces manifest-declared loopback sidecar companion services
(type:loopback, http(s) on 127.0.0.1/localhost/::1 only, sanitized origin+health_path)
as read-only health cards w/ a browser-side credentials-omitted health probe. Backend
hard-whitelists scheme + loopback host, rejects userinfo/path/query/traversal, rebuilds
output from parsed components (no echo of rejected input); frontend esc()'s every field,
whitelists badge status, fetch never reads body. Purely additive to GET /api/extensions/status.
Code applied byte-faithful from PR head 7080929. 33 own tests pass.
Nathan concept-approved (loopback-companion direction).

Co-authored-by: santastabber <santastabber@users.noreply.github.com>

* fix nesquena#4612 Codex gate findings: reject encoded query/fragment in health_path + align CSP loopback allowlist

Codex SHIP-WITH-FIXES (2 reproduced SILENT findings):
1. _normalize_sidecar_health_path percent-decoded AFTER the raw query/fragment ban,
   so /health%3Ftoken=abc -> ?token=abc survived into the probed URL. Now re-rejects
   decoded ? and # + regression test (%3F/%23 skipped with sidecar_health_path_rejected).
2. The origin validator accepts https:// and [::1], but CSP connect-src only listed
   http 127.0.0.1/localhost -> accepted https/IPv6 loopback sidecars would be CSP-blocked.
   Aligned _CSP_CONNECT_BASE (added https v4/localhost + http(s) [::1]) + updated the 4
   exact-string CSP tests + a regression test asserting all loopback forms are allowed.

Co-authored-by: santastabber <santastabber@users.noreply.github.com>

* Release v0.51.564 — Release TW (loopback sidecar diagnostics; nesquena#4612)

* fix(nesquena#4612): drop browser-invalid [::1]:* CSP source (browser-smoke fail)

The CSP-loopback-alignment fix added http(s)://[::1]:* to connect-src, but CSP
host-source grammar can't express a port wildcard on a bracketed IPv6 literal —
Chromium rejects '[::1]:*' as an invalid source, which browser-smoke flags as a
console error (CI fail). Keep the valid https://127.0.0.1:*/localhost:* additions;
drop the IPv6 wildcard entries. IPv6 [::1] sidecars are still accepted by the
validator (displayed in the card); their browser health probe surfaces as
'blocked' under CSP unless an operator adds a specific-port [::1]:<port> via
HERMES_WEBUI_CSP_CONNECT_EXTRA. Tests updated to match + assert [::1]:* absent.

---------

Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
Co-authored-by: santastabber <santastabber@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L Large PR (>10 files or >250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants