feat: credential proxy daemon Phase 1 — HTTP-only broker (#4656) - #4691
feat: credential proxy daemon Phase 1 — HTTP-only broker (#4656)#4691kenantan32 wants to merge 1 commit into
Conversation
…lder substitution (NousResearch#4656) Implements Phase 1 of the zero-knowledge credential proxy daemon. Tool subprocesses receive hermes-proxy://<name> placeholders instead of real credentials. The proxy intercepts HTTP requests on a Unix socket, substitutes placeholders with real values, and forwards to upstream. The agent process structurally cannot read credential values. New files: - proxy/store.py: Thread-safe, write-only credential store - proxy/server.py: asyncio HTTP proxy with placeholder substitution - proxy/config.py: Config helpers (socket path, enabled, proxy_credentials) - proxy/daemon.py: Daemon entry point (spawned by CLI) - tools/secrets_tool.py: Write-only secrets tool (store/rotate/delete/list) - hermes_cli/cred_proxy.py: CLI lifecycle (hermes cred-proxy start|stop|status) Modified files: - tools/env_passthrough.py: register_proxy_credentials() for blocklist bypass - tools/environments/local.py: inject http_proxy when proxy socket exists - hermes_cli/main.py: wire cred-proxy subcommand Tests: 37 new tests (store, server, passthrough, secrets tool) All 4665 existing tests pass (2 pre-existing failures unrelated). Phase 2 (HTTPS MITM + local CA) and Phase 3 (encrypted persistence) are left for follow-up PRs. Addresses NousResearch#4429 (env passthrough for proxy-brokered credentials). Refs NousResearch#4656
dsr-restyn
left a comment
There was a problem hiding this comment.
Thanks for picking this up — the secrets tool integration and profile-aware config are solid design choices that my earlier PR (#4695, now corrected in dsr-restyn#10) didn't include. A few things I noticed while reading through:
Duplicate function definition
register_proxy_credentials() is defined twice in tools/env_passthrough.py — once at line 93 and again at line 115. Looks like a copy-paste artifact; only the second version has the ImportError guard.
CONNECT returns 501 — breaks all HTTPS tools
Returning 501 for CONNECT means any tool making HTTPS requests through the proxy gets a hard failure. Since most API traffic is HTTPS, this effectively blocks the proxy from being usable in practice.
A blind TCP relay (pass-through without inspection) is a better Phase 1 choice:
if method.upper() == "CONNECT":
host, _, port_str = url.rpartition(":")
port = int(port_str)
# Establish upstream connection
up_reader, up_writer = await asyncio.open_connection(host, port)
# Tell client the tunnel is established
transport.write(b"HTTP/1.1 200 Connection Established\r\n\r\n")
# Blind bidirectional relay — no credential substitution
await asyncio.gather(pipe(client, upstream), pipe(upstream, client))HTTPS tools work normally, they just don't get credential substitution — which is the correct Phase 1 limitation. Phase 2 (MITM CA) adds substitution inside the tunnel.
_start() doesn't wait for daemon readiness
proc = subprocess.Popen([sys.executable, "-m", "proxy.daemon", ...], ...)
print(f"Credential proxy started (PID {proc.pid}).")This prints "started" immediately after spawning the subprocess, before the daemon has bound the socket or written its PID file. If you run hermes cred-proxy start && hermes cred-proxy status quickly, the status check may not find the socket yet.
A readiness poll (wait up to N seconds for the PID file + socket to appear) would fix this.
No CLI path for credential ingestion
hermes cred-proxy only supports start|stop|status — there's no add or list subcommand. Credentials can only be added through the secrets tool from within a running agent session.
This means you can't pre-load credentials before starting the agent (e.g., hermes cred-proxy add cf_dns_token from a setup script). The issue spec mentions hermes cred-proxy add <name> as a CLI entry point, and it's useful for bootstrapping.
This requires some IPC mechanism from the CLI process to the daemon's store — either a management endpoint on the Unix socket or a secondary channel. I implemented this in my fork as a /_cred/ HTTP API on the same socket (management requests use relative paths like /_cred/add, proxy requests use absolute URLs like http://host/..., so they never collide).
Incomplete body handling
In _ProxyProtocol._handle_request():
if content_length is not None and len(body) < content_length:
remaining = content_length - len(body)
if remaining > 0:
logger.debug("body incomplete: have %d, need %d more", len(body), remaining)This logs that the body is incomplete but doesn't actually read the remaining data — it continues with a truncated body. For typical small API payloads this probably works (the full request often arrives in one TCP read), but it will silently corrupt larger POST bodies.
Missing hermes-proxy:// blocklist bypass in _sanitize_subprocess_env
The local.py changes only inject http_proxy in _make_run_env(). But _sanitize_subprocess_env() still strips env vars like CLOUDFLARE_API_TOKEN from the blocklist — even when the value is just a hermes-proxy:// placeholder.
The passthrough registration handles this when the daemon is running, but if someone configures CLOUDFLARE_API_TOKEN=hermes-proxy://cf_token in .env without the daemon active, the placeholder gets stripped silently. Adding or value.startswith("hermes-proxy://") as a bypass in _sanitize_subprocess_env makes placeholder values safe regardless of daemon state.
Happy to contribute fixes for any of these if you'd like — I've already implemented the CONNECT relay, readiness polling, management API, and blocklist bypass in my fork. Let me know if you'd prefer a co-authored approach or separate follow-up PRs.
1 similar comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused stdlib-only Phase 1 contribution. The security boundary needs substantial rework before this version can be salvaged.
Problems
proxy/daemon.py:61callsset_store(store)only in the detached daemon.tools/secrets_tool.py:22-30reads a process-local_store, so the agent process cannot access or mutate the daemon’s store.tests/proxy/test_secrets_tool.pysets that global directly in its own process and does not cover the daemon boundary.- The daemon registers passthrough names only in its own process (
proxy/daemon.py:43-51), but current main reads the ContextVar-backed allowlist while constructing agent subprocess environments (tools/environments/local.py:346-372,794-812). The configured placeholder variables will still be stripped. The PR also definesregister_proxy_credentialstwice intools/env_passthrough.py; the latter overwrites the former. tools/secrets_tool.pyusestoolset="secrets", but this PR does not add that tool or toolset totoolsets.py, so discovery does not expose it to the agent.proxy/server.py:64dispatches at header completion;proxy/server.py:143-151logs an incomplete body but forwards it anyway.
Suggested changes
- Add daemon IPC plus an agent-side client, register passthrough in the agent process, wire a service-gated toolset, and add daemon/socket integration coverage for storage, env filtering, and segmented request bodies.
Automated hermes-sweeper review.
|
|
||
| def _check_available() -> bool: | ||
| """Return True if the credential proxy store is available.""" | ||
| return _store is not None |
There was a problem hiding this comment.
_store is process-local. The only production set_store() call is in the detached daemon (proxy/daemon.py), so the agent process keeps _store is None and this tool is unavailable. Please replace this with an IPC client to the daemon and add a cross-process integration test.
Summary
Implements Phase 1 of the zero-knowledge credential proxy daemon proposed in #4656.
Tool subprocesses receive
hermes-proxy://<name>placeholders instead of real credentials. A lightweight HTTP proxy running on a Unix domain socket intercepts outbound requests, substitutes placeholders with real values from an in-memory store, and forwards to the upstream server. The agent process structurally cannot read credential values — not as a matter of policy, but by design.Architecture
Changes (15 files, +1147 lines)
New files
proxy/store.py_resolve()for proxy server only. No read/get API — this is the security invariant.proxy/server.pyhermes-proxy://<name>placeholders in request headers and bodies. CONNECT returns 501 (Phase 2).proxy/config.pycredential_proxysection from config.yaml. Profile-aware socket paths (uses HERMES_HOME).proxy/daemon.pyhermes cred-proxy start. Registers proxy credentials in passthrough allowlist, binds store to secrets tool.tools/secrets_tool.pysecretstool registered withtools/registry.py. Actions: store, rotate, delete, list. No read/get action exists.hermes_cli/cred_proxy.pyhermes cred-proxy start|stop|status. Mirrors gateway.py pattern.Modified files
tools/env_passthrough.pyregister_proxy_credentials()— readscredential_proxy.proxy_credentialsfrom config and registers those var names so_sanitize_subprocess_env()doesn't strip placeholder values. Addresses #4429.tools/environments/local.py_make_run_env()now injectshttp_proxy/HTTP_PROXYpointing at the proxy Unix socket when the socket file exists.hermes_cli/main.pyhermes cred-proxy start|stop|statussubcommand.Configuration
Then in
.env:The agent sees the placeholder. The proxy substitutes the real value at the HTTP transport layer.
Security invariant
There is no
secrets(action="read")orsecrets(action="get"). The store has no external read API. The only code path that resolves a credential name to its value is inside the proxy server process — never exposed to the agent.Tests
37 new tests across 4 test files:
test_store.py: CRUD, thread safety, concurrent accesstest_server.py: placeholder regex, header/body substitution, mixed resolved/unresolvedtest_passthrough.py: env var registration, blocklist bypasstest_secrets_tool.py: all actions, no-read invariant, error casesAll 4665 existing tests pass (2 pre-existing failures in
test_api_key_providersandtest_codex_execution_pathsunrelated to this PR).What's NOT in this PR (future phases)
Dependencies
This PR includes the env passthrough mechanism needed for proxy-brokered credentials (#4429), making #4592 unnecessary.
Refs #4656, addresses #4429