Conversation
Adds tools/omarchy-mcp/, a purpose-built MCP server that runs on an Omarchy (Arch Linux) VM and exposes local AI agents and system tools as MCP tools consumable by any MCP client. 7 tools: - claude_execute / codex_execute — dispatch prompts to Claude Code and Codex CLI via stdin pipe with timeout and ANSI stripping - file_read / file_write / file_list — remote filesystem operations with path validation (/home/* and /tmp only) - system_run — whitelisted shell command execution (git, python3, ...) - status — VM health monitoring (uptime, memory, load, tool availability) Architecture: FastMCP v2 (MCPServer), Streamable HTTP transport, Bearer token auth via Starlette ASGI middleware, systemd-managed. Includes: full SPEC.md, README.md, Dockerfile, .env.example, MIT license, and verification test script.
|
Now covers all 4 AI agents on Omarchy.
|
Updated: added hermes_execute and grok_execute tools. Now covers all 4 AI agents available on Omarchy: Claude Code, Codex CLI, Hermes Agent, and Grok CLI (xAI). 9 tools total. |
|
Addressed all three AI review security findings: 1. system_run whitelist bypass (blocking) — Fixed. Was checking parts[0] then running 2. file_list missing path restriction — Fixed. Added a shared 3. Blocking subprocess in async tools — Fixed. All four executors (claude, codex, hermes, grok) and system_run now use |
|
All 3 review findings addressed:
Also: fixed the tool — Grok CLI takes prompt as arg, not via stdin, so no stdin_data needed there. |
ca4d752 to
839c94f
Compare
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 839c94f7d05ddce11c5b7efa235e394588459b93 against current main 27faf9ced2f8c7b3340c6b10caa507a625ba4383 and the actual merge base 1b17015f7a8d0c0d68b1f08aa389538e7fd172e3. The carrier is only four commits, but its ancestry is extremely stale: 4 ahead / 18,911 behind current main. That matters here because MCP secret handling, remote-transport policy, tool placement, and multi-profile ownership have all changed substantially since that merge base.
The concept is useful, and the previous review did find real defects. I re-checked those fixes rather than treating the follow-up comments as proof. The shell-wrapper injection and path-prefix issues are materially improved. There are still four blocking authority/lifetime problems at the exact head:
- P1 — a credential is being republished in the replacement spec.
SPEC.mdcontainssshpass -p 'Buddy-2019'as literal text. I searched current upstreammain; that string is not present there. So this PR would introduce the credential into upstream history while describing the project as removing plaintext credentials. If it was ever live, it is already exposed by the public PR/fork commit and must be rotated/revoked; regardless, tracked docs need a placeholder, not the value. - P1 — the bearer that grants SSH-equivalent execution is sent over plaintext HTTP. The server defaults to
0.0.0.0, explicitly disables SDK transport protection, and the README/SPEC configure clients withhttp://...plus a static bearer. An on-path observer can capture/replay that bearer and obtain the same remote execution authority as the legitimate client. Current Hermes MCP config already supports verified TLS/custom CAs/mTLS. Secure transport needs to be the default contract (or the server must be loopback-only behind an explicitly configured TLS endpoint);UFWis not confidentiality. The client docs should also use a profile-scoped${VAR}/${env:VAR}secret reference rather than writing the bearer literally intoconfig.yaml. - P1 — cancellation does not revoke execution authority.
run_command_async()isasyncio.to_thread()around a blockingPopen.communicate(). Cancelling the request task cancels only the awaiter; it cannot stop the worker thread, and the child/process group continues until normal completion or the independent timeout. That is especially dangerous forclaude --dangerously-skip-permissions, Codex, Hermes, and Grok: the request can be gone while mutations keep happening. The subprocess owner needs a cancellable lifecycle (native asyncio subprocess or equivalent), withCancelledErrorterminating/reaping the whole process group and an invariant test proving no child survives request cancellation/shutdown. - P1 — the async-stall fix is incomplete at the exact head.
system_run()is stillasync defbut directly invokes synchronousrun_command(). A long whitelisted process can therefore hold the event loop for the full 300 s cap, stalling auth, status, and every concurrent MCP request. The PR comment sayssystem_runwas moved to the shared async path, but the current bytes do not do that. This is the same defect class as the earlier review, not a new theoretical concern; it simply survived the repair.
There are also two carrier-level gaps that need closure before this is ready:
- Current-main footprint/ownership:
tools/AGENTS.mdnow explicitly says custom/local-only capabilities should be plugins rather than core, and that new backends compose through existing backend/provider seams. This tree is explicitly a standalone project, hard-codes/home/jpeetzand one private LAN topology throughout, and already has its own external repository. That makes the presenttools/omarchy-mcp/carrier look like a personal deployment being vendored into core, not a generic Hermes-owned capability. Either keep the server external/plugin-owned, or genericize its identity/config/install surface and justify why core owns it under the current Footprint Ladder. Do not lose JPeetz's authorship whichever carrier wins. - Verification contract: the PR body and first commit both claim an included standalone verification script, but the complete 17-file diff contains no test file/script at all. Exact-head CI
34964759444, Docker34964758595, and Nix34964758608are allaction_required; CI has zero jobs. The other three surviving commits likewise have onlyaction_requiredruns. So this is neither exact-head green nor every-commit green, and none of the security/lifetime boundaries above are repository-regressed.
Topology/interlocks from current main:
- Merged #111620 is now the fail-closed profile secret-scope foundation. A remote MCP credential should enter through that profile-scoped secret authority, not as a literal config value.
- Open #111481 is the per-served-profile MCP connection/credential-identity carrier. This server is complementary to it, not a replacement; any first-party integration must preserve
(profile, server)credential ownership end-to-end. - #104567 is adjacent Omarchy/remote-execution work with explicit setup, lifecycle ownership, cancellation/cleanup, and isolation semantics. It is not a duplicate of this server, but it is useful precedent for how remote execution authority is admitted and torn down.
- Closed-unmerged #83967 is historical Omarchy-skill work, not current upstream ownership. The literal password this PR describes is absent from current
main, so the publication story should not imply that upstream currently carries that credential.
I like the direction of replacing ad-hoc SSH/SCP orchestration with a structured protocol, and the author responded quickly to the first security pass. The remaining work is mostly about making the protocol boundary deserve the amount of authority it carries: secure transport, scoped secrets, cancellation-owned subprocesses, exact async behavior, a generic carrier, and executable regression proof. Once those are true, the shape is much stronger.
| 5. Stripping ANSI escape codes with a 3-pass regex | ||
| 6. Manual timeout tuning per prompt size | ||
|
|
||
| This pipeline had real costs: a **plaintext password embedded in skill files** (`sshpass -p 'Buddy-2019'` in ~15+ locations), **fragile heredoc quoting** that broke on complex prompts, **silent failures** on SSH timeout (empty output files), and **no streaming** for long-running tasks. |
There was a problem hiding this comment.
P1 — remove/rotate this credential. This is a literal password-looking value in a public PR commit, not a placeholder. Current upstream main does not contain this string, so merging the PR would introduce it upstream while the feature is explicitly supposed to eliminate plaintext credentials. Replace it with a redacted example; if it was ever live, treat the public PR/fork history as exposure and rotate/revoke it rather than relying on deleting this line.
| bind = os.environ.get("BIND", "0.0.0.0") | ||
| app = mcp.streamable_http_app( | ||
| streamable_http_path="/mcp", | ||
| transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False), |
There was a problem hiding this comment.
P1 — secure transport has to be part of the authority boundary. This disables the SDK transport protection on a server that defaults to 0.0.0.0, while the shipped docs configure a static bearer over http://. That bearer grants SSH-equivalent remote execution, so an on-path capture is a reusable execution credential. Current Hermes MCP config supports verified TLS/custom CAs/mTLS; default to a confidential/authenticated transport (or loopback behind an explicit TLS endpoint) and keep the protection enabled/configured rather than globally disabling it. Firewall reachability is not bearer confidentiality.
| cmd, cwd=cwd, timeout=timeout, env=env, | ||
| ) | ||
| if stdin_data is not None: | ||
| return await asyncio.to_thread(fn, stdin_data=stdin_data) |
There was a problem hiding this comment.
P1 — request cancellation does not stop the mutation. asyncio.to_thread() can cancel the awaiting coroutine, but it cannot cancel the already-running worker thread; the blocking communicate() and child process group therefore keep running until completion/timeout. For the YOLO executors that means the caller/request can disappear while filesystem/process side effects continue. Please make the subprocess lifetime owned by the request (native asyncio subprocess or equivalent), catch cancellation, terminate + reap the entire process group, and add a regression witness that no child survives cancellation/shutdown.
| from executor import run_command | ||
| # Run as a list (no shell) so shlex splitting is authoritative and | ||
| # shell metacharacters in arguments are passed literally, not executed. | ||
| result = run_command( |
There was a problem hiding this comment.
P1 — the event-loop stall from the earlier review is still present at this head. system_run() is async def but calls the synchronous run_command() directly, so a long-running whitelisted process can block the MCP server for up to the 300 s cap. The follow-up comment says system_run was moved to the shared async helper, but these exact bytes did not. Await the cancellable async runner here and prove a concurrent status/auth request stays responsive while a long command is active.
|
Credential leak fixed. SPEC.md contained a literal password from the skill file as an example of the problem — replaced with '…'. Real credential rotated. |
|
All 4 P1 items from @andrexibiza's review addressed: P1 — Plaintext password in docs — Removed from SPEC.md. The password never appeared in source code — only as an example of the old SSH pipeline. Replaced with a placeholder. P1 — system_run blocking event loop — Now uses P1 — Cancellation doesn't stop subprocess — P1 — Bearer token over plain HTTP — Default bind changed to |
All paths now configurable via env vars (OMARCHY_USER_HOME, OMARCHY_FILE_ROOTS).
|
Generalized: all hardcoded paths removed.
|
|
README rewritten as a complete deployment guide — anyone can now clone and deploy without editing a single line of code. Covers: requirements, step-by-step from clone to Hermes integration, full env var table, security, troubleshooting. |
Test Results — 9/9 PASSEDAll tests executed against the running server via MCP protocol: Reply to @andrexibiza's reviewP1 — Credential in SPEC.md: Fixed in bfebb79. Password replaced with placeholder. Rotated. No secrets remain in any committed file — confirmed by grep audit. P1 — Bearer over plain HTTP: Fixed in 3728118. Default bind is now P1 — Cancellation orphans subprocess: Fixed in 3728118. P1 — system_run still blocking: Fixed in 3728118. Footprint/ownership: Code generalized in ff21b54. No hardcoded Verification contract (no tests): Added in this PR. Full test suite at |
Security review: all 3 reported issues are already fixedI checked out 1.
|
Security review: all 3 reported issues are already fixedI checked out 1.
|
Omarchy MCP Server
A purpose-built MCP server that runs on an Omarchy (Arch Linux) VM and exposes local AI agents and system tools as MCP tools — consumable by any MCP client (Hermes Agent, Claude Code, etc.).
What it replaces
The current skill documents an SSH-based dispatch pipeline (write prompt → SCP → SSH → claude → ANSI scrape → SCP back) with a plaintext password in ~15+ locations. This server collapses that into authenticated, structured MCP tool calls.
7 tools
claude_executecodex_executefile_read/file_write/file_listsystem_runstatusArchitecture
Security
.env, not in skill files or session promptssystem_runwhitelist-based (only pre-approved commands)/home/*and/tmpTesting
All 7 tools verified end-to-end from Hermes Agent (Mac → Omarchy VM). Auth rejection (no-token, bad-token) verified with 401 responses. Also includes a standalone Python test script.
Documentation
Full SPEC.md, README.md, Dockerfile, .env.example, MIT license.
Related
Standalone project repo: https://github.com/JPeetz/omarchy-mcp