feat: read-only web dashboard for monitoring sessions, messages, and cron

[heyalchang]

Summary

Adds hermes dashboard — an aiohttp web UI for observing gateway activity. 10 tabs: Overview, Messages, Sessions, Usage, Cron Jobs, Logs, Search, Memory, Plugins, Checkpoints.

• Read-only: all data via fresh SQLite connections in ro mode
• Schema-adaptive: PRAGMA table_info detects available columns, works across DB versions
• Single-file SPA: vanilla HTML/JS, no build step, no framework deps
• Separate process: hermes dashboard [--port PORT], never embedded in gateway
• 34 tests covering data layer, schema detection, search, path traversal blocking, secret redaction

Intended for upstream submission to NousResearch/hermes-agent after review here.

Test plan

• Review diff for upstream readiness — no local-only code, no hardcoded paths
• Run python -m pytest tests/test_dashboard_data.py -v (34 tests)
• Start with hermes dashboard --port 18808 and verify all 10 tabs render
• Check that Payload/Activity tabs are stripped (local-only, JSONL-dependent)
• Verify secret redaction in Memory tab .env display
• Test with a fresh/empty state.db (schema detection graceful degradation)

🤖 Generated with Claude Code

[ccreview]

prbot Review (Claude)

Now I have a thorough understanding. Here's my review:

---

Code Review: PR #2 — hermes dashboard web UI

📋 Overview

This PR adds a read-only web dashboard (hermes dashboard) for monitoring Hermes gateway activity. It includes 10 tabs (Overview, Messages, Sessions, Usage, Cron Jobs, Logs, Search, Memory, Plugins, Checkpoints) served as a single-file SPA via aiohttp.

However, the PR title is misleading. While titled as a dashboard feature, this PR actually contains 609 files changed (+135k / −11k lines) across 100 commits — encompassing a massive amount of unrelated work: ACP adapters, new gateway platforms (Matrix, Mattermost, DingTalk, SMS, webhook), voice tools, blockchain skills, Honcho integration, anthropic adapter, smart model routing, usage pricing, and much more.

---

🟢 Strengths

Dashboard Implementation (core feature)

• Clean architecture: Clear separation between data layer (data.py), HTTP layer (server.py), and presentation (index.html)
• Read-only safety: All DB access uses SQLite ?mode=ro URI — good defensive practice
• Schema-adaptive queries: PRAGMA table_info detection handles DB version drift gracefully — well-tested
• No build step: Vanilla HTML/JS SPA with no framework dependencies keeps things simple
• Good test coverage: 34 tests in test_dashboard_data.py covering data layer, schema detection, FTS search, path traversal blocking, and secret redaction
• Path traversal protection: _safe_id() in server.py strips .. and / from URL params, read_cron_output() uses Path.name sanitization
• Secret redaction: .env display masks secrets (shows first 4 + **** + last 4 chars)

Code Quality

• Functions are well-documented with docstrings
• Consistent error handling with graceful degradation (empty lists/dicts on failure)
• _int_param() helper prevents bad query param crashes
• Log line limits capped (min(lines, 1000), min(limit, 500))

---

🟡 Suggestions for Improvement

1. XSS Risk in innerHTML Usage (Medium severity)

The SPA uses innerHTML ~30+ times with template literals. While there's an esc() function (used ~45 times), any missed spot where user-controlled data (message content, session titles, tool names) flows into innerHTML without esc() is an XSS vector.

Recommendation: Audit every innerHTML assignment to ensure all dynamic data passes through esc(). Consider using textContent where HTML structure isn't needed (already done in some places — good).

2. No CORS Configuration

The aiohttp server has no CORS middleware. While this is fine for same-origin access, if the intent is localhost-only monitoring, explicitly binding to 127.0.0.1 (already done ✅) is good, but consider adding a note about this design choice.

3. Module-level Global State in data.py

_sessions_columns: Optional[set] = None

The cached column set is process-global and never invalidated. If the DB schema changes while the dashboard is running (e.g., after an upgrade), the cache will serve stale data until restart.

Recommendation: Add a TTL-based cache (e.g., re-detect every 60 seconds) or invalidate on error.

4. tail_log() Reads Entire File

with open(LOG_PATH, "r", errors="replace") as f:
    all_lines = f.readlines()
    return "".join(all_lines[-lines:])

This reads the entire log file into memory to return the last N lines. For large log files this is wasteful.

Recommendation: Use collections.deque(f, maxlen=lines) or seek from the end of the file.

5. Subprocess Calls in get_checkpoints()

Runs git log + git diff --shortstat per snapshot (up to 50 × N checkpoints). This could be slow with many checkpoints.

Recommendation: Consider batching git commands or caching results.

6. Secret Redaction is Fragile

if len(v) > 8:
    v = v[:4] + "****" + v[-4:]

Short secrets (≤ 8 chars) are shown in full. Values like password or short API keys leak completely.

Recommendation: Redact all values regardless of length, or mask everything beyond the first 2 characters.

7. FTS Search: No Input Sanitization

The search_transcripts() function passes the user query directly to MATCH ?. While parameterized, FTS5 MATCH syntax itself can cause errors with special characters (*, ", OR, NOT).

Recommendation: Wrap the query in double quotes or escape FTS5 operators for robustness.

---

🔴 Significant Concerns

1. PR Scope is Massively Overloaded

This PR claims to be a dashboard feature but contains 609 files, 135k additions across dozens of unrelated features. This makes it:

• Impossible to review thoroughly — the diff exceeds GitHub's 300-file limit
• High risk for regressions — so many changes bundled together
• Difficult to revert if issues are found

Strong recommendation: Split this into separate PRs per feature area (dashboard, ACP adapter, new platforms, voice tools, etc.).

2. No Authentication on the Dashboard

The dashboard exposes session transcripts, memory files, config, (redacted) env vars, and message content over HTTP with zero authentication. Even on localhost, any local process or browser tab could read this data.

Recommendation: At minimum, add a bearer token or basic auth option. Document the security model clearly.

---

📊 Summary

Aspect	Rating
Dashboard code quality	⭐⭐⭐⭐ Good
Test coverage (dashboard)	⭐⭐⭐⭐ Good (34 tests)
Security	⭐⭐⭐ Needs auth + XSS audit
PR scope / reviewability	⭐ Way too large
Upstream readiness	⭐⭐ Needs splitting

Verdict: The dashboard feature itself is well-built and well-tested. However, this PR needs to be split into manageable, reviewable chunks before it can be properly evaluated for upstream submission. The dashboard-specific files (dashboard/, tests/test_dashboard_data.py) could be extracted into a clean, focused PR of ~3,700 lines that would be straightforward to review and merge.

[ccreview]

prbot Review (Claude)

Now I have a thorough understanding of this PR. Here's my review:

---

Code Review: PR #2 — hermes dashboard (Read-only Web Dashboard)

Overview

This PR adds a hermes dashboard command that launches a read-only aiohttp web UI for monitoring gateway activity. The dashboard is a single-file SPA (vanilla HTML/JS) with 10 tabs: Overview, Messages, Sessions, Usage, Cron Jobs, Logs, Search, Memory, Plugins, and Checkpoints.

Core files (5 files, ~3,727 lines added):

File	Lines	Purpose
dashboard/data.py	720	Read-only data access layer (SQLite, filesystem)
dashboard/server.py	186	aiohttp route handlers + app factory
dashboard/static/index.html	2,413	Single-file SPA (HTML + CSS + JS)
tests/test_dashboard_data.py	408	34 tests for the data layer
Integration	~25	CLI subcommand + pyproject.toml extras

Note: The PR shows 600 files / 135K additions because the upstream/dashboard branch has diverged significantly from main. Only the 5 files above are the actual dashboard contribution.

---

👍 Strengths

• Clean architecture — Strict separation between data access (data.py), HTTP layer (server.py), and UI (index.html). No state leaks between layers.
• Read-only by design — All SQLite connections use ?mode=ro, no write endpoints exist, and cron controls are intentionally disabled.
• Schema-adaptive — PRAGMA table_info detects available columns so it works across DB versions. This is thoughtful for upstream compatibility.
• Good test coverage on the data layer — path traversal blocking, secret redaction, schema degradation, and cron parsing are all tested.
• Zero build step — No npm/webpack/framework dependencies. Just a single HTML file with vanilla JS.
• Optional dependency — pip install hermes-agent[dashboard] via pyproject extras.

---

🔴 Security Issues

1. XSS via onclick inline handler (Medium Severity)

// In loadFeed():
onclick="jumpToSession('${esc(m.session_id)}')"

The esc() function escapes <, >, &, " but does NOT escape single quotes ('). Since the JS string is delimited by single quotes inside the onclick attribute, a malicious session_id containing ');alert(1);// would break out of the string context.

Fix: Use escAttr() (which already exists and escapes quotes), or better yet, use data-session-id attributes with event delegation (which is already the pattern used for cron job rows).

2. Secret redaction leaks short secrets

if len(v) > 4:
    v = v[:2] + "****" + v[-2:]
elif v:
    v = "****"

A 5-character secret like abcde becomes ab****de — revealing 4 of 5 characters (80%). Consider using a higher threshold (e.g., len(v) > 8) or always masking all but the last 2 characters.

3. No API error handling in api() fetch

async function api(path) {
  const r = await fetch('/api/' + path);
  return r.json();
}

Non-200 responses (404, 500) will attempt r.json() which may throw on non-JSON error pages. This silently fails. Add a status check:

async function api(path) {
  const r = await fetch('/api/' + path);
  if (!r.ok) throw new Error(`API error: ${r.status}`);
  return r.json();
}

4. _safe_id() is incomplete sanitization

def _safe_id(raw: str) -> str:
    return raw.replace("/", "").replace("..", "").replace("\\", "")

This strips characters but doesn't validate the result is a legitimate job ID format. Consider allowlisting: re.sub(r'[^a-zA-Z0-9_-]', '', raw).

---

🟡 Code Quality & Maintainability

5. Global mutable state for schema cache

_sessions_columns: Optional[set] = None

Once cached, this is never invalidated. If the DB schema is altered (e.g., migration during hermes upgrade), the dashboard will serve stale column metadata until restart. Consider a TTL-based cache or re-check every N minutes.

6. get_checkpoints() has O(n×m) subprocess calls

For each checkpoint directory, it runs git log (1 subprocess), then git diff --shortstat per commit (up to 50 subprocesses). With 10 checkpoint dirs × 50 snapshots = 500 subprocess calls per request. This could easily take 30+ seconds.

Fix: Use git log --stat --format=... in a single call per checkpoint, or add pagination and lazy-load stats.

7. Hardcoded platform list in get_feed()

sources = [
    "telegram", "whatsapp", "discord", "slack", "signal",
    "homeassistant", "email", "webhook", "matrix", "mattermost",
    "dingtalk", "sms",
]

This will miss any new platforms added upstream. Consider SELECT DISTINCT source FROM sessions WHERE source != 'cli' AND source != 'cron' instead.

8. 2,400-line single HTML file

While intentional (no build step), this makes the frontend very difficult to review and maintain. At minimum, consider splitting the <script> block into a separate dashboard.js file served from the same static directory.

9. json.dumps(obj, default=str) silently converts errors

In json_response(), default=str silently converts any non-serializable object to its string representation. This can mask bugs where unexpected types slip through. Consider being explicit about which types get converted.

---

🟡 Test Coverage Gaps

What's tested ✅	What's missing ❌
Data layer (34 tests)	Server route handlers (0 tests)
Schema detection	get_checkpoints() with actual git data
Path traversal blocking	get_insights() with real data
Secret redaction	Short secret edge cases
Cron parsing	Error response codes (404, malformed params)
FTS search	Frontend rendering (even basic smoke tests)

Adding a few aiohttp test client tests for server.py would significantly increase confidence, especially for the input sanitization in route handlers.

---

🔵 Minor Nits

• tail_log() imports deque inside the function — move to module level
• No CORS headers — fine for same-origin use, but worth a comment explaining this is intentional
• Missing __all__ in dashboard/data.py — would clarify the public API
• get_cron_status() counts disabled = total - enabled - paused which could be negative if a job is both enabled: False AND state: 'paused' (should that be possible?)

---

Summary

This is a well-structured, thoughtfully designed dashboard with good separation of concerns and defensive read-only patterns. The main areas needing attention before merge:

Priority	Issue	Effort
🔴 High	XSS in onclick handler — use escAttr() or data attributes	5 min
🔴 Medium	Secret redaction threshold too low	5 min
🟡 Medium	get_checkpoints() performance (subprocess per commit)	30 min
🟡 Medium	Add server route tests	1-2 hr
🟡 Low	Hardcoded platform list in get_feed()	10 min
🟡 Low	API error handling in JS api()	5 min

The XSS fix is the only blocker — everything else can be addressed in follow-ups.

[ccreview]

prbot Review (Codex)

Summary

This PR adds a new local aiohttp dashboard, wires it into hermes, and includes a substantial read-only data layer plus frontend. It is not ready to merge yet: the packaged install is missing the dashboard’s static assets, so hermes dashboard breaks outside a source checkout, and the dashboard reimplements a couple of existing status/discovery paths in ways that can report incorrect state.

Findings

P0 — Must fix before merge

• pyproject.toml:98 / dashboard/server.py:16 / dashboard/server.py:177 The wheel does not include dashboard/static/index.html or any other static assets. I verified this by building the wheel locally with --no-build-isolation and inspecting its contents: only dashboard/__init__.py, dashboard/data.py, and dashboard/server.py were packaged. In an installed build, STATIC_DIR therefore does not exist, so create_app() registers a missing static directory and the new hermes dashboard command fails outside a git checkout. Add package-data for dashboard/static/* and cover it with a packaging test.

P1 — Strong recommendation

• dashboard/data.py:66 get_gateway_status() treats any live PID in gateway.pid as “gateway running” by doing only os.kill(pid, 0). Hermes already has canonical logic in gateway/status.py that also checks process start time and whether the process still looks like Hermes before accepting the PID. If the real gateway exits and the OS later reuses that PID for another process, the dashboard will falsely show the gateway as running and surface stale platform info. Reuse get_running_pid() / read_runtime_status() instead of duplicating the PID-file logic here.

P2 — Worth raising

• dashboard/data.py:521 The plugin view is not reporting actual Hermes plugin state; it is re-scanning manifests and marking every readable manifest or entry point as enabled=True. That disagrees with the real plugin manager, which only sets enabled after import and register() succeed, and it also discovers opt-in project plugins when HERMES_ENABLE_PROJECT_PLUGINS is set (hermes_cli/plugins.py:181, hermes_cli/plugins.py:410, tests/test_plugins.py:148). As written, a broken plugin with no __init__.py or no register() will still show up as enabled in the dashboard, which defeats the point of a monitoring page. Use PluginManager.list_plugins() or relabel this tab as manifest discovery only.

Tool Notes

semgrep could not run in this environment because its binary failed to initialize CA trust anchors.

The repo’s default pytest addopts uses -n auto, but pytest-xdist is not installed here; rerunning python -m pytest -o addopts='' tests/test_dashboard_data.py -q passed.