Skip to content

feature: fail closed on caller-supplied filesystem paths in the MCP surface #32

Description

@copyleftdev

Problem

Every MCP tool in kahea-mcp accepts the store root, the configuration path, and (for
kahea_invoke) the plan reference as ordinary call arguments. A model composing a tool call
therefore chooses the filesystem locations the server reads, and kahea_invoke additionally
chooses the configuration whose policy fingerprint it will be checked against.

Public scanning already flags the narrowest edge of this. The mcpbeat listing for this server
reports kahea_invoke.plan reaching fs::read(path) at crates/kahea-mcp/src/lib.rs:661:

  • tool_invoke reads plan straight from the arguments (crates/kahea-mcp/src/lib.rs:436) and
    passes it to stored_plan_kind (:450).
  • stored_plan_kind treats any reference without a plan: / workflow-plan: /
    conformance-plan: prefix as a literal path (:659) and reads it (:661).
  • load_plan (crates/kahea-plan/src/lib.rs:2299) and load_websocket_plan (:2326) repeat the
    same branch, so the reference is read a second time and deserialized.

This is deliberate CLI parity — the input schema documents plan as "Sealed plan handle or local
sealed plan file path" (crates/kahea-mcp/src/lib.rs:142) — and it is the right contract for
kahea invoke <plan.json> on a terminal, where the operator typed the path. It is the wrong
contract for a tool argument, where the string is model-controlled.

Consequences, in order of how much they actually matter:

  1. Path oracle. operation_error returns raw io::Error and serde_json::Error text to the
    caller, so a model can probe any path the server user can read and learn whether it exists, is
    readable, and parses as JSON. No file content is disclosed: stored_plan_kind extracts only
    "kind" and swallows failure with .ok(), and load_plan requires verify_seal to pass
    before anything executes.
  2. The configuration is not an anchor. config (:438) and store (:435) are also call
    arguments. plan.policy_fingerprint is only compared against the fingerprint of whatever
    configuration the same call named (:450-456, enforced in crates/kahea-exec/src/lib.rs:119),
    and grants (:458) are supplied by the caller as well. The runtime boundary in
    evaluate_runtime_target (crates/kahea-exec/src/lib.rs:227) is sound, but it evaluates grants
    the caller passed against policy.allowed_hosts from a file the caller chose. A caller that can
    write a config.toml anywhere on disk can plan and invoke against it, and both fingerprints
    agree because both were derived from that file.
  3. The seal is integrity, not authenticity. RequestPlan::seal / verify_seal
    (crates/kahea-core/src/lib.rs:178-194) is a keyless BLAKE3 digest over the plan itself, so any
    file a caller can write is a validly sealed plan. That is fine as long as plan references cannot
    leave the operator's store; it is what makes (1) and (2) compose.

Nothing here is exploitable by Kāhea alone — the server has no write primitive. It requires an
agent holding some other filesystem-write tool in the same session, which is the normal MCP host
configuration and therefore the assumption the server should be built against.

Proposed contract

Fail closed on the MCP surface only. The CLI keeps every path it accepts today.

  1. plan must be a handle. On the MCP surface, reject any reference that is not a valid sealed
    plan handle, reusing the grammar already implemented by validate_handle
    (crates/kahea-mcp/src/lib.rs:627). Bare filesystem paths become
    McpError::Invalid before any read.
  2. store and config are process configuration, not call arguments. Pin both at server
    startup (kahea mcp --store … --config …, defaulting to .kahea and .kahea/config.toml).
    Remove them from every tool input schema so a call cannot relocate the store or swap the policy
    it is measured against. This is a breaking change to the MCP tool schemas and needs a changelog
    entry.
  3. Confine any residual path resolution. Where a path is still resolved from a handle,
    canonicalize it and assert the result is inside the pinned store root, so a handle that survives
    validation still cannot escape.
  4. Close the oracle. Collapse plan-load failures on the MCP surface to a single non-descriptive
    McpError::Invalid, so a rejected reference does not report whether the target existed, was
    unreadable, or failed to parse.

Fail-closed behavior: a call naming anything outside the pinned store is denied before the
filesystem is touched, with no network connection and no evidence record.

Out of scope for this issue, tracked separately if wanted: replacing the keyless plan digest with a
MAC under a store-local key, so verify_seal means "this store minted it" rather than "this JSON
hashes to itself."

Alternatives considered

  • Canonicalize and confine, keep paths in the schema. Preserves CLI parity on the MCP surface
    but leaves the store root and configuration path caller-chosen, so confinement is measured
    against a boundary the caller picked. Rejected as the primary fix; kept as hardening (3).
  • Allowlist a plan directory in config.toml. Still bootstraps trust from a configuration file
    the caller names, so it does not fix (2).
  • Leave as documented behavior. The schema does document that plan accepts a path, and the
    finding as scanned is low severity on its own. Rejected because the documentation describes an
    operator affordance while the consumer is a model, and because (1) and (2) compose into a policy
    bypass in any session with a write tool.

Reported publicly by mcpbeat. Filing this in
the open rather than as a private advisory because the finding is already published, no release is
exposed to a remote attacker (the server is local-only, stdio, and unpublished to registries), and
the change is a tool-schema contract change that needs a public record.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions