fix: harden MCP HTTP transport — bearer auth, loopback default, cleanup confirm gate (#13) - #28
Merged
Merged
Conversation
…up gate (#13) The HTTP transport had no authentication and bound to 0.0.0.0 by default, so any network-adjacent client could read/write/delete every agent's memory and scrape /metrics. `memory_cleanup` was also an unconfirmed, fleet-wide DELETE any client could trigger. - HEXUS_API_TOKEN: when set, an ASGI wrapper requires `Authorization: Bearer <token>` on every HTTP request (MCP calls + /metrics), constant-time compared. lifespan/websocket scopes pass through untouched so the streamable-http session manager still starts. When unset, a prominent WARNING is logged at startup. - Secure default: CLI `--host` and the FastMCP instance now default to 127.0.0.1 instead of 0.0.0.0. Docker is unaffected (the entrypoint passes `--host 0.0.0.0` explicitly). Serving on a non-loopback host without a token logs a loud warning. - memory_cleanup now defaults to a dry run: without `confirm=true` it reports the counts it WOULD delete (via new cleanup_stale_records(dry_run=True)) and deletes nothing. The scheduled background cleanup is unchanged (it calls the store method directly). Adds tests/test_http_auth.py covering the wrapper (401 without/with wrong token, pass-through with the right one, www-authenticate header, and lifespan pass-through). Note: agent identity is still client-asserted, and by-id reads/mutations (fetch_full/confirm/reject) remain unscoped — binding identity to the authenticated credential and agent-scoping those ops are larger, behavior-changing follow-ups tracked in #13 and #19. Refs #13 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| limit_date = datetime.now(timezone.utc) - timedelta(days=ttl) | ||
| if dry_run: | ||
| # table/ts_col are internal literals (not caller-supplied). | ||
| cur.execute( |
There was a problem hiding this comment.
SUGGESTION: f-string SQL interpolation of table/column names, currently safe but fragile.
table and ts_col are interpolated directly into SQL via f-strings on lines 1778 and 1784. While currently safe because the values come from a hardcoded targets list defined at L1764-1768, this pattern bypasses psycopg2's parameterized query protection. If the targets list is ever extended to accept any form of external input (e.g., config-driven table names), this becomes a SQL injection vector.
Consider adding a whitelist guard (e.g., assert table in {"conversations", "memory_entries", "delegations"}) before the cur.execute call to make the invariant explicit and auditable.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit 1a8484a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1a8484a)Status: No Issues Found | Recommendation: Merge Files Reviewed (6 files)
Previous review (commit 30f38c3)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Reviewed by deepseek-v4-pro · Input: 18.1K · Output: 4K · Cached: 120.1K |
This was referenced Jul 7, 2026
Configurable isolation policy, SQL-safety fixes, by-id scoping, and server-derived caller identity. Stacked on #28 (fix/mcp-http-auth). SQL safety (unconditional): - increment_recall_counts: allowlist the interpolated table identifier ({memory_entries, conversations, delegations}); reject anything else. - replace()/remove(): escape LIKE metacharacters (\ % _) + ESCAPE '\', so remove(old_text="%") no longer wipes the whole (agent,target) scope. Isolation policy (HEXUS_MEMORY_ISOLATION, default "shared"): - shared: reads/recall/search span all agents (trusted-fleet default). - strict: reads scoped to the caller's identity. - Cross-agent MUTATIONS (confirm/reject/remove/forget/summarize by id) are always caller-scoped, in both modes. - Resolves the empty-agent_identity asymmetry (item C): empty means "all agents" in shared / "this agent" in strict, consistently. - list_entries now treats agent_identity=None as all-agents, matching search()/count(). By-id scoping (item B): - fetch_full/confirm_entry/reject_entry/summarize_session take an optional agent_identity and filter on it; fetch_full bypasses the id-keyed CCRCache when strict-scoped so it can't leak another agent's row. Threaded through the MCP tools and the hermes handlers. Server-derived identity (item A): - HTTP transport publishes the authenticated X-Hermes-Session-Key as the caller identity (tools.current_caller ContextVar); it overrides the client-supplied agent_identity for writes/mutations, so an authenticated client can no longer act as another agent. stdio / in-process callers keep the prior behavior. Tests: SQL-safety + isolation unit tests (no DB) in test_http_auth.py; DB-backed cross-agent scoping tests in test_mcp_server.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… bump version to 0.9.2
…gation warning, update CI packaging check, and bump version to 0.9.2
fix: multi-agent isolation + SQL safety at the data layer (#19)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the highest-impact parts of #13: the open network surface and the fleet-wide delete path.
Problem
0.0.0.0by default (server.py,cli.py), so any client that could reach the port could call every tool — read all agents' memory, write poisoned entries, delete data, and scrape/metrics.memory_cleanupwas an unconfirmed, unscoped, fleet-wide DELETE any client (or a confused LLM) could trigger.Changes
1. Bearer-token auth on the HTTP transport
HEXUS_API_TOKEN, when set, requiresAuthorization: Bearer <token>on every HTTP request (MCP calls and/metrics), compared withhmac.compare_digest. Implemented as a thin ASGI wrapper around the streamable-http app so it also covers/metricsand — importantly — only gateshttpscopes, lettinglifespan/websocketpass through so the session manager still starts. When the token is unset, a prominentWARNINGis logged.2. Secure-by-default bind address
CLI
--host(and theFastMCPinstance) now default to127.0.0.1instead of0.0.0.0. Docker is unaffected —docker/entrypoint.shpasses--host 0.0.0.0explicitly. Serving on a non-loopback host without a token logs a loud warning. (Behavior change for directhexus-mcp serveusers who relied on the old 0.0.0.0 default — now opt-in.)3.
memory_cleanupconfirm gateDefaults to a dry run: without
confirm=trueit returns the counts it would delete (newcleanup_stale_records(dry_run=True)runsSELECT count(*)instead ofDELETE) and deletes nothing. Mirrorsmemory_forget's dry-run-by-default. The scheduled background cleanup is unchanged (it calls the store method directly, not the tool).Testing
tests/test_http_auth.pycovers the wrapper: 401 with no/wrong token (downstream never reached), pass-through with the correct token,WWW-Authenticateon 401, and lifespan pass-through. I validated the exact wrapper logic with a standalone async harness (5/5 pass); the suite runs under the dockertestprofile in CI. Ruff format + lint clean.Explicitly out of scope (larger follow-ups, still tracked in #13 / #19)
agent_identity. Binding identity to the credential is a bigger design change.fetch_full/confirm_entry/reject_entry) remain unscoped byagent_identity(see SQL safety: unallowlisted table identifier + unescaped LIKE wildcards + unscoped by-id ops #19).These are noted so reviewers know this PR closes the network-surface hole and the fleet-wipe path without claiming to make identity tamper-proof.
🤖 Generated with Claude Code