diff --git a/BACKLOG.md b/BACKLOG.md index 5a2dad3..b6fb7d4 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -284,3 +284,11 @@ sourcing the function from `tools/gate.sh` is the obvious shape, and 17's PLAN d Deliberately cited by **construct, not line number**: both files change under active work, and a line citation in a file under change is the stale-citation class this repository has already committed once inside the document recording the fix for it. + +## 12. The MCP surface changed in 16j + +The MCP surface changed in slice 16j. Changes are pinned by tests in +`apps/hacktui_agent/test/mcp_stdio_framing_test.exs`. + +`ping` carrying a `_meta` revision is a known `beam_mcp` defect, tracked as SCR-257, and is +pinned by no test here because a test would pin the defect. diff --git a/README.md b/README.md index b73bb3b..8704556 100644 --- a/README.md +++ b/README.md @@ -445,7 +445,7 @@ HACKTUI_MCP_STDIO=1 ./bin/hacktui-mcp ## Quick initialize test ```bash -body='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"demo","version":"0.1"}}}' +body='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"demo","version":"0.1"}}}' printf '%s\n' "$body" | ./bin/hacktui-mcp ``` @@ -955,7 +955,7 @@ mix hacktui.tui And if you want to test MCP: ```bash -body='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"demo","version":"0.1"}}}' +body='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-11-25","capabilities":{},"clientInfo":{"name":"demo","version":"0.1"}}}' printf '%s\n' "$body" | ./bin/hacktui-mcp ``` diff --git a/apps/hacktui_agent/lib/hacktui_agent/mcp/dispatch.ex b/apps/hacktui_agent/lib/hacktui_agent/mcp/dispatch.ex index d5f2648..f990c13 100644 --- a/apps/hacktui_agent/lib/hacktui_agent/mcp/dispatch.ex +++ b/apps/hacktui_agent/lib/hacktui_agent/mcp/dispatch.ex @@ -91,8 +91,23 @@ defmodule HacktuiAgent.MCP.Dispatch do def call(:propose_action, action_spec, opts) when is_map(action_spec) do proposal_service = Keyword.get(opts, :proposal_service, ProposalService) - {:ok, proposal_service.propose_action(action_spec, opts)} + {:ok, proposal_service.propose_action(coerce_action_class(action_spec), opts)} end def call(_tool, _args, _opts), do: {:error, :unknown_tool} + + # The protocol core hands values through unchanged: turning "contain" into :contain is domain + # knowledge, and a generic MCP layer that guessed at it would be carrying this project's + # vocabulary. The schema's enum is what constrains the value; this maps the three it allows + # and leaves anything else alone for the service to reject. + defp coerce_action_class(%{action_class: value} = spec) when is_binary(value) do + %{spec | action_class: action_class_atom(value)} + end + + defp coerce_action_class(spec), do: spec + + defp action_class_atom("contain"), do: :contain + defp action_class_atom("observe"), do: :observe + defp action_class_atom("notify_export"), do: :notify_export + defp action_class_atom(other), do: other end diff --git a/apps/hacktui_agent/lib/hacktui_agent/mcp/schema.ex b/apps/hacktui_agent/lib/hacktui_agent/mcp/schema.ex deleted file mode 100644 index f7b90f7..0000000 --- a/apps/hacktui_agent/lib/hacktui_agent/mcp/schema.ex +++ /dev/null @@ -1,112 +0,0 @@ -defmodule HacktuiAgent.MCP.Schema do - @moduledoc """ - Validates `tools/call` arguments against the JSON Schema the server advertises. - - The schemas at `MCP.Server.input_schema/1` were declared and never enforced — they were - used only to build the `tools/list` response. `"additionalProperties" => false` and - `"required"` were advertisement. - - That was exploitable. `normalize_arguments/1` retained unrecognised **string** keys; - `ProposalService.propose_action/2` set its safety fields with **atom** keys via - `Map.put_new`, which cannot see them; and `to_json_value/1` stringified and collapsed - the collision with the caller's value winning. A client could send - `{"requires_approval": false, "status": "approved"}` and receive a response asserting - its own containment action was pre-approved. - - This is a deliberately small subset of JSON Schema — the keywords the server actually - uses. It is not a general validator, and it refuses rather than guesses. - """ - - @type result :: :ok | {:error, String.t()} - - @doc """ - Validates `arguments` against `schema`. - - Returns `:ok`, or `{:error, reason}` naming the offending property. - """ - @spec validate(map(), map()) :: result() - def validate(arguments, schema) when is_map(arguments) and is_map(schema) do - properties = Map.get(schema, "properties", %{}) - - with :ok <- check_required(arguments, Map.get(schema, "required", [])), - :ok <- check_additional(arguments, properties, schema) do - check_properties(arguments, properties) - end - end - - def validate(_arguments, _schema), do: {:error, "arguments must be an object"} - - defp check_required(arguments, required) when is_list(required) do - case Enum.reject(required, &Map.has_key?(arguments, &1)) do - [] -> :ok - missing -> {:error, "missing required propert#{plural(missing)}: #{join(missing)}"} - end - end - - defp check_required(_arguments, _), do: :ok - - defp check_additional(arguments, properties, schema) do - if Map.get(schema, "additionalProperties", true) == false do - case arguments |> Map.keys() |> Enum.reject(&Map.has_key?(properties, &1)) do - [] -> :ok - extra -> {:error, "unknown propert#{plural(extra)}: #{join(extra)}"} - end - else - :ok - end - end - - defp check_properties(arguments, properties) do - Enum.reduce_while(arguments, :ok, fn {key, value}, :ok -> - case Map.fetch(properties, key) do - {:ok, spec} -> - case check_value(key, value, spec) do - :ok -> {:cont, :ok} - error -> {:halt, error} - end - - :error -> - {:cont, :ok} - end - end) - end - - defp check_value(key, value, spec) do - with :ok <- check_type(key, value, Map.get(spec, "type")), - :ok <- check_enum(key, value, Map.get(spec, "enum")) do - check_range(key, value, spec) - end - end - - defp check_type(_key, _value, nil), do: :ok - defp check_type(_key, value, "string") when is_binary(value), do: :ok - defp check_type(_key, value, "integer") when is_integer(value), do: :ok - defp check_type(_key, value, "number") when is_number(value), do: :ok - defp check_type(_key, value, "boolean") when is_boolean(value), do: :ok - defp check_type(_key, value, "object") when is_map(value), do: :ok - defp check_type(_key, value, "array") when is_list(value), do: :ok - defp check_type(key, _value, type), do: {:error, "#{key} must be of type #{type}"} - - defp check_enum(_key, _value, nil), do: :ok - - defp check_enum(key, value, allowed) when is_list(allowed) do - if value in allowed, do: :ok, else: {:error, "#{key} must be one of: #{join(allowed)}"} - end - - defp check_range(key, value, spec) when is_number(value) do - min = Map.get(spec, "minimum") - max = Map.get(spec, "maximum") - - cond do - is_number(min) and value < min -> {:error, "#{key} must be >= #{min}"} - is_number(max) and value > max -> {:error, "#{key} must be <= #{max}"} - true -> :ok - end - end - - defp check_range(_key, _value, _spec), do: :ok - - defp plural([_]), do: "y" - defp plural(_), do: "ies" - defp join(values), do: Enum.map_join(values, ", ", &to_string/1) -end diff --git a/apps/hacktui_agent/lib/hacktui_agent/mcp/server.ex b/apps/hacktui_agent/lib/hacktui_agent/mcp/server.ex deleted file mode 100644 index 5a5c20c..0000000 --- a/apps/hacktui_agent/lib/hacktui_agent/mcp/server.ex +++ /dev/null @@ -1,320 +0,0 @@ -defmodule HacktuiAgent.MCP.Server do - alias HacktuiAgent.MCP.Schema - - @moduledoc false - - alias HacktuiAgent.MCP.Dispatch - alias HacktuiAgent.MCP.ToolCatalog - alias HacktuiAgent.MCP.ToolSpec - - @protocol_version "2024-11-05" - @server_name "hacktui-hermes" - @server_version "0.1.0" - - @type state :: %{ - dispatch: (atom(), map(), keyword() -> {:ok, term()} | {:error, term()}), - dispatch_opts: keyword(), - initialized?: boolean(), - shutdown?: boolean(), - tool_catalog: module() - } - - @spec new(keyword()) :: state() - def new(opts \\ []) do - %{ - dispatch: Keyword.get(opts, :dispatch, &Dispatch.safe_call/3), - dispatch_opts: Keyword.get(opts, :dispatch_opts, []), - initialized?: false, - shutdown?: false, - tool_catalog: Keyword.get(opts, :tool_catalog, ToolCatalog) - } - end - - @spec shutdown?(state()) :: boolean() - def shutdown?(state), do: state.shutdown? - - @spec handle_message(state(), map()) :: {state(), map() | nil} - def handle_message(state, %{"jsonrpc" => "2.0", "id" => id, "method" => "initialize"}) do - response = - result(id, %{ - "protocolVersion" => @protocol_version, - "capabilities" => %{"tools" => %{"listChanged" => false}}, - "serverInfo" => %{"name" => @server_name, "version" => @server_version} - }) - - {%{state | initialized?: true}, response} - end - - def handle_message(state, %{"jsonrpc" => "2.0", "method" => "notifications/initialized"}) do - {%{state | initialized?: true}, nil} - end - - def handle_message(state, %{"jsonrpc" => "2.0", "id" => id, "method" => "ping"}) do - {state, result(id, %{})} - end - - def handle_message(state, %{"jsonrpc" => "2.0", "id" => id, "method" => "tools/list"}) do - tools = Enum.map(state.tool_catalog.all(), &tool_definition/1) - {state, result(id, %{"tools" => tools})} - end - - def handle_message( - state, - %{ - "jsonrpc" => "2.0", - "id" => id, - "method" => "tools/call", - "params" => %{"name" => name} = params - } - ) do - arguments = Map.get(params, "arguments", %{}) - - case normalize_tool_name(name) do - {:ok, tool_name} -> - response = - case validate_and_dispatch(state, tool_name, arguments) do - {:ok, payload} -> - result(id, tool_success(payload)) - - {:error, reason} -> - result(id, tool_failure(reason)) - end - - {state, response} - - :error -> - {state, error(id, -32_601, "Unknown tool: #{name}")} - end - end - - def handle_message(state, %{"jsonrpc" => "2.0", "id" => id, "method" => "shutdown"}) do - {%{state | shutdown?: true}, result(id, %{})} - end - - def handle_message(state, %{"jsonrpc" => "2.0", "method" => "exit"}) do - {%{state | shutdown?: true}, nil} - end - - def handle_message(state, %{"jsonrpc" => "2.0", "id" => id, "method" => method}) do - {state, error(id, -32_601, "Method not found: #{method}")} - end - - def handle_message(state, %{"id" => id}) do - {state, error(id, -32_600, "Invalid Request")} - end - - def handle_message(state, _message) do - {state, nil} - end - - defp tool_definition(%ToolSpec{} = tool) do - %{ - "name" => Atom.to_string(tool.name), - "description" => tool.description, - "inputSchema" => input_schema(tool.name), - "annotations" => %{ - "destructiveHint" => tool.mode == :proposal, - "idempotentHint" => tool.mode == :read_only, - "openWorldHint" => false, - "readOnlyHint" => tool.mode == :read_only - } - } - end - - defp tool_success(payload) do - encoded = to_json_value(payload) - - %{ - "content" => [%{"type" => "text", "text" => Jason.encode!(encoded, pretty: true)}], - "structuredContent" => encoded, - "isError" => false - } - end - - defp tool_failure(reason) do - message = format_reason(reason) - - %{ - "content" => [%{"type" => "text", "text" => message}], - "structuredContent" => %{"error" => message}, - "isError" => true - } - end - - defp normalize_tool_name(name) when is_binary(name) do - valid_names = Enum.map(ToolCatalog.all(), &Atom.to_string(&1.name)) - - if name in valid_names do - {:ok, String.to_existing_atom(name)} - else - :error - end - end - - defp normalize_tool_name(name) when is_atom(name), do: {:ok, name} - defp normalize_tool_name(_name), do: :error - - # The advertised schema is the contract. Validate the wire form -- string keys, as the - # client sent them -- before normalising, so `required` and `additionalProperties` - # mean what tools/list says they mean. - defp validate_and_dispatch(state, tool_name, arguments) do - case Schema.validate(arguments, input_schema(tool_name)) do - :ok -> - state.dispatch.(tool_name, normalize_arguments(arguments), state.dispatch_opts) - - {:error, reason} -> - {:error, %{tool: tool_name, reason: "invalid arguments: #{reason}"}} - end - end - - # Only reached with a map: Schema.validate/2 rejects anything else first. - defp normalize_arguments(arguments) when is_map(arguments) do - Enum.reduce(arguments, %{}, fn {key, value}, acc -> - normalized_value = normalize_argument_value(key, value) - - # Only recognised keys survive, and only as atoms. Previously both the string - # and atom form were inserted, and unrecognised string keys were retained -- - # which let a caller smuggle fields past ProposalService's atom-keyed - # Map.put_new and win the collision during stringification. - case normalize_argument_key(key) do - {:ok, atom_key} -> Map.put(acc, atom_key, normalized_value) - :error -> acc - end - end) - end - - defp normalize_argument_key(key) when is_atom(key), do: {:ok, key} - - defp normalize_argument_key(key) when is_binary(key) do - case key do - "action_class" -> {:ok, :action_class} - "case_id" -> {:ok, :case_id} - "format" -> {:ok, :format} - "limit" -> {:ok, :limit} - "rationale" -> {:ok, :rationale} - "summary" -> {:ok, :summary} - "target" -> {:ok, :target} - _ -> :error - end - end - - defp normalize_argument_key(_key), do: :error - - defp normalize_argument_value("action_class", value) when is_binary(value) do - case value do - "contain" -> :contain - "observe" -> :observe - "notify_export" -> :notify_export - other -> other - end - end - - defp normalize_argument_value(_key, value) when is_map(value), do: to_json_value(value) - - defp normalize_argument_value(_key, value) when is_list(value), - do: Enum.map(value, &to_json_value/1) - - defp normalize_argument_value(_key, value), do: value - - @doc false - @spec input_schema_for(atom()) :: map() - def input_schema_for(tool_name), do: input_schema(tool_name) - - defp input_schema(:get_latest_alerts) do - %{ - "type" => "object", - "properties" => %{ - "limit" => %{ - "type" => "integer", - "description" => "Maximum number of alert queue entries to return.", - "minimum" => 1, - "maximum" => 100 - } - }, - "additionalProperties" => false - } - end - - defp input_schema(:get_case_timeline) do - %{ - "type" => "object", - "properties" => %{ - "case_id" => %{ - "type" => "string", - "description" => "Case identifier to inspect." - } - }, - "required" => ["case_id"], - "additionalProperties" => false - } - end - - defp input_schema(:draft_report) do - %{ - "type" => "object", - "properties" => %{ - "case_id" => %{ - "type" => "string", - "description" => "Case identifier to draft a report for." - }, - "format" => %{ - "type" => "string", - "description" => "Optional report format hint." - } - }, - "required" => ["case_id"], - "additionalProperties" => false - } - end - - defp input_schema(:propose_action) do - %{ - "type" => "object", - "properties" => %{ - "case_id" => %{ - "type" => "string", - "description" => "Case identifier for the action request." - }, - "action_class" => %{ - "type" => "string", - "description" => "Action class to propose.", - "enum" => ["contain", "observe", "notify_export"] - }, - "target" => %{ - "type" => "string", - "description" => "Target host, identity, or resource." - }, - "rationale" => %{ - "type" => "string", - "description" => "Why the action is being proposed." - } - }, - "required" => ["case_id", "action_class", "target"], - "additionalProperties" => false - } - end - - defp input_schema(_tool_name) do - %{"type" => "object", "properties" => %{}, "additionalProperties" => true} - end - - defp to_json_value(value) when is_map(value) do - Map.new(value, fn {key, nested_value} -> - normalized_key = if is_atom(key), do: Atom.to_string(key), else: key - {normalized_key, to_json_value(nested_value)} - end) - end - - defp to_json_value(value) when is_list(value), do: Enum.map(value, &to_json_value/1) - defp to_json_value(value) when is_atom(value), do: Atom.to_string(value) - defp to_json_value(value), do: value - - defp format_reason(reason) when is_binary(reason), do: reason - defp format_reason(reason), do: inspect(reason) - - defp result(id, payload), do: %{"jsonrpc" => "2.0", "id" => id, "result" => payload} - - defp error(id, code, message) do - %{"jsonrpc" => "2.0", "id" => id, "error" => %{"code" => code, "message" => message}} - end -end diff --git a/apps/hacktui_agent/lib/hacktui_agent/mcp/stdio.ex b/apps/hacktui_agent/lib/hacktui_agent/mcp/stdio.ex deleted file mode 100644 index c757ad4..0000000 --- a/apps/hacktui_agent/lib/hacktui_agent/mcp/stdio.ex +++ /dev/null @@ -1,146 +0,0 @@ -defmodule HacktuiAgent.MCP.Stdio do - @moduledoc false - - alias HacktuiAgent.MCP.Server - - @spec run(keyword()) :: :ok - def run(opts \\ []) do - loop(Server.new(opts)) - end - - defp loop(state) do - case read_message() do - :eof -> - :ok - - {:ok, message} -> - {next_state, response} = Server.handle_message(state, message) - - if response do - write_message(response) - end - - if Server.shutdown?(next_state) do - :ok - else - loop(next_state) - end - - {:error, reason} -> - write_message(%{ - "jsonrpc" => "2.0", - "id" => nil, - "error" => %{"code" => -32_700, "message" => "Parse error", "data" => inspect(reason)} - }) - - loop(state) - end - end - - # MCP stdio framing is newline-delimited JSON-RPC. The spec at 2024-11-05 -- the - # revision this server advertises -- and every revision since: - # - # "Messages are delimited by newlines, and MUST NOT contain embedded newlines." - # - # This previously implemented LSP framing (Content-Length headers), so no conformant - # MCP client could complete a handshake. Content-Length is still accepted on read so - # existing callers keep working, but responses are always newline-delimited. - @max_line_bytes 1_048_576 - @max_body_bytes 1_048_576 - - defp read_message do - case read_line_bounded() do - :eof -> - :eof - - {:error, reason} -> - {:error, reason} - - {:ok_line, line} -> - trimmed = String.trim_trailing(line, "\r") - - cond do - trimmed == "" -> read_message() - content_length_header?(trimmed) -> read_legacy_framed(trimmed) - true -> decode(trimmed) - end - end - end - - # Reads one newline-terminated line, refusing to allocate past @max_line_bytes. - # The cap has to bound the read, not merely inspect its result. - defp read_line_bounded(acc \\ [], size \\ 0) - - defp read_line_bounded(_acc, size) when size >= @max_line_bytes, - do: {:error, :frame_too_large} - - defp read_line_bounded(acc, size) do - case IO.binread(:stdio, 1) do - :eof when acc == [] -> :eof - :eof -> {:ok_line, acc |> Enum.reverse() |> IO.iodata_to_binary()} - {:error, reason} -> {:error, reason} - "\n" -> {:ok_line, acc |> Enum.reverse() |> IO.iodata_to_binary()} - byte -> read_line_bounded([byte | acc], size + 1) - end - end - - defp decode(body) do - case Jason.decode(body) do - {:ok, decoded} -> {:ok, decoded} - {:error, _} = error -> error - end - end - - defp content_length_header?(line) do - line |> String.downcase() |> String.starts_with?("content-length:") - end - - # Legacy LSP-style framing, retained for callers written against the old behaviour. - defp read_legacy_framed(first_line) do - with {:ok, length} <- parse_content_length(first_line), - :ok <- skip_remaining_headers(), - {:ok, body} <- read_body(length) do - decode(body) - else - {:error, :unexpected_eof} -> :eof - {:error, _} = error -> error - end - end - - defp skip_remaining_headers do - case IO.binread(:stdio, :line) do - :eof -> :ok - {:error, reason} -> {:error, reason} - line -> if String.trim(line) == "", do: :ok, else: skip_remaining_headers() - end - end - - defp parse_content_length(line) do - case String.split(line, ":", parts: 2) do - [_name, value] -> - case Integer.parse(String.trim(value)) do - {length, ""} when length >= 0 and length <= @max_body_bytes -> {:ok, length} - {length, ""} when length > @max_body_bytes -> {:error, :frame_too_large} - _ -> {:error, :invalid_content_length} - end - - _ -> - {:error, :missing_content_length} - end - end - - defp read_body(length) when is_integer(length) and length >= 0 do - case IO.binread(:stdio, length) do - :eof -> {:error, :unexpected_eof} - {:error, reason} -> {:error, reason} - body -> {:ok, body} - end - end - - # Always newline-delimited, per the MCP stdio binding. Jason never emits a raw - # newline inside a JSON scalar, so the "MUST NOT contain embedded newlines" - # requirement holds. - defp write_message(message) do - IO.binwrite(:stdio, [Jason.encode!(message), "\n"]) - end -end diff --git a/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_catalog.ex b/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_catalog.ex index cac7b43..5fa905d 100644 --- a/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_catalog.ex +++ b/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_catalog.ex @@ -1,34 +1,68 @@ defmodule HacktuiAgent.MCP.ToolCatalog do @moduledoc """ Bounded MCP tool catalog derived from the approved architecture. + + Implements `BeamMCP.ToolCatalog`. Each spec carries its own `input_schema`: the protocol core + advertises that schema in `tools/list` and enforces the same one on `tools/call`, so the + contract a client is shown and the contract it is held to cannot drift apart. + + These schemas previously lived as per-tool clauses inside the protocol server. They are + domain data — this project's six tools — and they belong beside the tools rather than inside + a generic module, which is why the extracted package does not carry them. """ - alias HacktuiAgent.MCP.ToolSpec + @behaviour BeamMCP.ToolCatalog + + alias BeamMCP.ToolSpec + + @open_object %{"type" => "object", "properties" => %{}, "additionalProperties" => true} @read_only_tools [ %ToolSpec{ name: :get_latest_alerts, command_class: :observe, mode: :read_only, - description: "Read the latest alert queue entries." + description: "Read the latest alert queue entries.", + input_schema: %{ + "type" => "object", + "properties" => %{ + "limit" => %{ + "type" => "integer", + "description" => "Maximum number of alert queue entries to return.", + "minimum" => 1, + "maximum" => 100 + } + }, + "additionalProperties" => false + } }, %ToolSpec{ name: :get_sensor_logs, command_class: :observe, mode: :read_only, - description: "Read recent sensor logs." + description: "Read recent sensor logs.", + input_schema: @open_object }, %ToolSpec{ name: :get_jido_responses, command_class: :observe, mode: :read_only, - description: "Read recent Jido agent responses." + description: "Read recent Jido agent responses.", + input_schema: @open_object }, %ToolSpec{ name: :get_case_timeline, command_class: :observe, mode: :read_only, - description: "Read the timeline for a single case." + description: "Read the timeline for a single case.", + input_schema: %{ + "type" => "object", + "properties" => %{ + "case_id" => %{"type" => "string", "description" => "Case identifier to inspect."} + }, + "required" => ["case_id"], + "additionalProperties" => false + } } ] @@ -37,13 +71,49 @@ defmodule HacktuiAgent.MCP.ToolCatalog do name: :draft_report, command_class: :notify_export, mode: :proposal, - description: "Draft a report for analyst review." + description: "Draft a report for analyst review.", + input_schema: %{ + "type" => "object", + "properties" => %{ + "case_id" => %{ + "type" => "string", + "description" => "Case identifier to draft a report for." + }, + "format" => %{"type" => "string", "description" => "Optional report format hint."} + }, + "required" => ["case_id"], + "additionalProperties" => false + } }, %ToolSpec{ name: :propose_action, command_class: :contain, mode: :proposal, - description: "Propose an approval-governed action request." + description: "Propose an approval-governed action request.", + input_schema: %{ + "type" => "object", + "properties" => %{ + "case_id" => %{ + "type" => "string", + "description" => "Case identifier for the action request." + }, + "action_class" => %{ + "type" => "string", + "description" => "Action class to propose.", + "enum" => ["contain", "observe", "notify_export"] + }, + "target" => %{ + "type" => "string", + "description" => "Target host, identity, or resource." + }, + "rationale" => %{ + "type" => "string", + "description" => "Why the action is being proposed." + } + }, + "required" => ["case_id", "action_class", "target"], + "additionalProperties" => false + } } ] @@ -53,6 +123,7 @@ defmodule HacktuiAgent.MCP.ToolCatalog do @spec proposal_tools() :: [ToolSpec.t()] def proposal_tools, do: @proposal_tools + @impl BeamMCP.ToolCatalog @spec all() :: [ToolSpec.t()] def all, do: @read_only_tools ++ @proposal_tools end diff --git a/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_spec.ex b/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_spec.ex deleted file mode 100644 index 5248e94..0000000 --- a/apps/hacktui_agent/lib/hacktui_agent/mcp/tool_spec.ex +++ /dev/null @@ -1,15 +0,0 @@ -defmodule HacktuiAgent.MCP.ToolSpec do - @moduledoc """ - Typed MCP tool contract exposed to bounded agent runs. - """ - - @enforce_keys [:name, :command_class, :mode, :description] - defstruct [:name, :command_class, :mode, :description] - - @type t :: %__MODULE__{ - name: atom(), - command_class: atom(), - mode: :read_only | :proposal, - description: String.t() - } -end diff --git a/apps/hacktui_agent/lib/mix/tasks/hacktui.mcp.ex b/apps/hacktui_agent/lib/mix/tasks/hacktui.mcp.ex index d2b2007..4401a7d 100644 --- a/apps/hacktui_agent/lib/mix/tasks/hacktui.mcp.ex +++ b/apps/hacktui_agent/lib/mix/tasks/hacktui.mcp.ex @@ -7,6 +7,11 @@ defmodule Mix.Tasks.Hacktui.Mcp do def run(_args) do :logger.remove_handler(:default) Mix.Task.run("app.start") - HacktuiAgent.MCP.Stdio.run() + + BeamMCP.Transport.Stdio.run( + tool_catalog: HacktuiAgent.MCP.ToolCatalog, + dispatch: &HacktuiAgent.MCP.Dispatch.safe_call/3, + server_name: "hacktui-hermes" + ) end end diff --git a/apps/hacktui_agent/mix.exs b/apps/hacktui_agent/mix.exs index b822787..804e9ca 100644 --- a/apps/hacktui_agent/mix.exs +++ b/apps/hacktui_agent/mix.exs @@ -24,6 +24,7 @@ defmodule HacktuiAgent.MixProject do defp deps do [ + {:beam_mcp, "~> 0.1.0"}, {:hacktui_core, in_umbrella: true}, {:hacktui_hub, in_umbrella: true}, {:jido, "~> 2.0"} diff --git a/apps/hacktui_agent/test/hacktui_agent_contracts_test.exs b/apps/hacktui_agent/test/hacktui_agent_contracts_test.exs index cbb24b1..8412d7d 100644 --- a/apps/hacktui_agent/test/hacktui_agent_contracts_test.exs +++ b/apps/hacktui_agent/test/hacktui_agent_contracts_test.exs @@ -1,7 +1,8 @@ defmodule HacktuiAgent.ContractsTest do use ExUnit.Case, async: true - alias HacktuiAgent.{HermesBoundary, MCP.ToolCatalog, MCP.ToolSpec} + alias BeamMCP.ToolSpec + alias HacktuiAgent.{HermesBoundary, MCP.ToolCatalog} test "defines a bounded MCP tool catalog" do read_only_tools = ToolCatalog.read_only_tools() diff --git a/apps/hacktui_agent/test/hacktui_agent_dispatch_test.exs b/apps/hacktui_agent/test/hacktui_agent_dispatch_test.exs index 7fc08b6..460419a 100644 --- a/apps/hacktui_agent/test/hacktui_agent_dispatch_test.exs +++ b/apps/hacktui_agent/test/hacktui_agent_dispatch_test.exs @@ -15,6 +15,10 @@ defmodule HacktuiAgent.DispatchTest do def propose_action(%{case_id: "case-1", action_class: :contain, target: "host-42"}, _opts), do: %{case_id: "case-1", action_class: :contain, target: "host-42", requires_approval: true} + + # Echoes whatever it is handed, so a test can assert what Dispatch passed on rather than + # what this fake decided to return. + def propose_action(spec, _opts), do: Map.put(spec, :requires_approval, true) end test "dispatches read-only MCP tools to the hub query service" do @@ -40,4 +44,26 @@ defmodule HacktuiAgent.DispatchTest do proposal_service: FakeProposalService ) end + + # The protocol core passes argument values through unchanged: a client sends the string + # "contain" and that is what arrives. Turning it into :contain is this project's vocabulary, + # so the coercion lives here. Covered explicitly because it used to be covered by a protocol + # test that no longer asserts it -- moving behaviour moves the burden of proving it. + test "coerces the wire's action_class string into the domain atom" do + assert {:ok, %{action_class: :contain}} = + Dispatch.call( + :propose_action, + %{case_id: "case-1", action_class: "contain", target: "host-42"}, + proposal_service: FakeProposalService + ) + end + + test "leaves an action_class it does not recognise alone, for the service to reject" do + assert {:ok, %{action_class: "teleport"}} = + Dispatch.call( + :propose_action, + %{case_id: "case-1", action_class: "teleport", target: "host-42"}, + proposal_service: FakeProposalService + ) + end end diff --git a/apps/hacktui_agent/test/hacktui_agent_mcp_server_test.exs b/apps/hacktui_agent/test/hacktui_agent_mcp_server_test.exs index 74a681b..6cd92f5 100644 --- a/apps/hacktui_agent/test/hacktui_agent_mcp_server_test.exs +++ b/apps/hacktui_agent/test/hacktui_agent_mcp_server_test.exs @@ -1,41 +1,55 @@ defmodule HacktuiAgent.MCPServerTest do use ExUnit.Case, async: true - alias HacktuiAgent.MCP.Server + alias BeamMCP.Server defmodule FakeToolCatalog do def all do [ - %HacktuiAgent.MCP.ToolSpec{ + %BeamMCP.ToolSpec{ name: :get_latest_alerts, command_class: :observe, mode: :read_only, description: "Read the latest alert queue entries." }, - %HacktuiAgent.MCP.ToolSpec{ + %BeamMCP.ToolSpec{ name: :propose_action, command_class: :contain, mode: :proposal, - description: "Propose an approval-governed action request." + description: "Propose an approval-governed action request.", + # A ToolSpec without an input_schema is a tool with no declared arguments, so the + # server derives no keys and dispatch receives nothing. That is the package working + # as designed: the schema is the argument contract, and a fake catalog that omits + # one is describing a tool that takes none. + input_schema: %{ + "type" => "object", + "properties" => %{ + "case_id" => %{"type" => "string"}, + "action_class" => %{"type" => "string"}, + "target" => %{"type" => "string"} + }, + "required" => ["case_id", "action_class", "target"], + "additionalProperties" => false + } } ] end end test "initialize advertises MCP tool capability" do - state = Server.new(tool_catalog: FakeToolCatalog) + state = Server.new(tool_catalog: FakeToolCatalog, server_name: "hacktui-hermes") {next_state, response} = Server.handle_message(state, %{"jsonrpc" => "2.0", "id" => 1, "method" => "initialize"}) assert next_state.initialized? - assert response["result"]["protocolVersion"] == "2024-11-05" + assert response["result"]["protocolVersion"] == "2025-11-25" assert response["result"]["capabilities"] == %{"tools" => %{"listChanged" => false}} assert response["result"]["serverInfo"]["name"] == "hacktui-hermes" end test "tools/list exposes MCP-compatible tool metadata" do - state = Server.new(tool_catalog: FakeToolCatalog) + state = Server.new(tool_catalog: FakeToolCatalog, server_name: "hacktui-hermes") {_next_state, response} = Server.handle_message(state, %{"jsonrpc" => "2.0", "id" => 2, "method" => "tools/list"}) @@ -56,7 +70,8 @@ defmodule HacktuiAgent.MCPServerTest do {:ok, %{received: args[:action_class], case_id: args[:case_id], target: args[:target]}} end - state = Server.new(dispatch: dispatch, tool_catalog: FakeToolCatalog) + state = + Server.new(dispatch: dispatch, tool_catalog: FakeToolCatalog, server_name: "hacktui-hermes") {_next_state, response} = Server.handle_message(state, %{ @@ -75,7 +90,10 @@ defmodule HacktuiAgent.MCPServerTest do assert_receive {:dispatch, :propose_action, args, []} assert args[:case_id] == "case-7" - assert args[:action_class] == :contain + # The protocol core normalises the KEY and passes the VALUE through. Turning "contain" + # into :contain is domain knowledge and now lives in HacktuiAgent.MCP.Dispatch, which this + # test replaces with a fake -- so the value arrives as the client sent it. + assert args[:action_class] == "contain" assert args[:target] == "host-42" assert response["result"]["isError"] == false @@ -88,7 +106,7 @@ defmodule HacktuiAgent.MCPServerTest do end test "unknown tools return a JSON-RPC error" do - state = Server.new(tool_catalog: FakeToolCatalog) + state = Server.new(tool_catalog: FakeToolCatalog, server_name: "hacktui-hermes") {_next_state, response} = Server.handle_message(state, %{ diff --git a/apps/hacktui_agent/test/mcp_boundary_test.exs b/apps/hacktui_agent/test/mcp_boundary_test.exs index 6c975f9..9e156c9 100644 --- a/apps/hacktui_agent/test/mcp_boundary_test.exs +++ b/apps/hacktui_agent/test/mcp_boundary_test.exs @@ -2,15 +2,30 @@ defmodule HacktuiAgent.MCP.BoundaryTest do @moduledoc """ The MCP boundary must enforce the contract it advertises. - The JSON Schemas at `Server.input_schema/1` were used only to build `tools/list`. - Because `normalize_arguments/1` also retained unrecognised string keys, and - `ProposalService` set its safety fields with atom keys via `Map.put_new`, a caller - could smuggle `requires_approval`/`status` past both and win the collision when - `to_json_value/1` stringified them. + The schemas were once advertised and not enforced: they built `tools/list` and nothing + checked a call against them. Because argument normalisation also retained unrecognised + string keys, and `ProposalService` set its safety fields with atom keys via `Map.put_new`, + a caller could smuggle `requires_approval`/`status` past both and win the collision when + the result was stringified. + + The schemas now live on the catalog's `BeamMCP.ToolSpec`s and the protocol core enforces + the same one it advertises. This test reads them from the catalog, so it asserts against + the schema the server actually uses rather than a copy that could drift from it. """ use ExUnit.Case, async: true - alias HacktuiAgent.MCP.{Egress, Schema, Server} + alias BeamMCP.Schema + alias HacktuiAgent.MCP.{Egress, ToolCatalog} + + # The schemas moved onto the catalog's ToolSpecs when the protocol core was extracted: + # they are this project's domain data, and a generic MCP package does not carry them. + # Server.input_schema_for/1 was removed with them and does not come back -- it existed only + # to reach schemas the server no longer holds. The catalog is where they live now, and + # reading them from there is what makes this test assert the schema the server actually + # advertises rather than a second copy. + defp advertised_schema(name) do + ToolCatalog.all() |> Enum.find(&(&1.name == name)) |> Map.fetch!(:input_schema) + end describe "schema validation" do test "rejects properties the schema does not declare" do @@ -23,7 +38,7 @@ defmodule HacktuiAgent.MCP.BoundaryTest do "requires_approval" => false, "status" => "approved" }, - Server.input_schema_for(:propose_action) + advertised_schema(:propose_action) ) assert reason =~ "unknown properties" @@ -35,19 +50,19 @@ defmodule HacktuiAgent.MCP.BoundaryTest do assert :ok = Schema.validate( %{"case_id" => "c-1", "action_class" => "contain", "target" => "h1"}, - Server.input_schema_for(:propose_action) + advertised_schema(:propose_action) ) end test "enforces required properties" do assert {:error, reason} = - Schema.validate(%{"case_id" => "c-1"}, Server.input_schema_for(:propose_action)) + Schema.validate(%{"case_id" => "c-1"}, advertised_schema(:propose_action)) assert reason =~ "missing required" end test "enforces the advertised numeric range" do - schema = Server.input_schema_for(:get_latest_alerts) + schema = advertised_schema(:get_latest_alerts) assert :ok = Schema.validate(%{"limit" => 10}, schema) assert {:error, reason} = Schema.validate(%{"limit" => 500}, schema) @@ -57,7 +72,7 @@ defmodule HacktuiAgent.MCP.BoundaryTest do test "enforces declared types" do assert {:error, reason} = - Schema.validate(%{"limit" => "ten"}, Server.input_schema_for(:get_latest_alerts)) + Schema.validate(%{"limit" => "ten"}, advertised_schema(:get_latest_alerts)) assert reason =~ "integer" end diff --git a/apps/hacktui_agent/test/mcp_stdio_framing_test.exs b/apps/hacktui_agent/test/mcp_stdio_framing_test.exs index 17cb0bd..2b61dc9 100644 --- a/apps/hacktui_agent/test/mcp_stdio_framing_test.exs +++ b/apps/hacktui_agent/test/mcp_stdio_framing_test.exs @@ -1,10 +1,11 @@ -defmodule HacktuiAgent.MCP.StdioFramingTest do +defmodule BeamMCP.Transport.StdioFramingTest do @moduledoc """ End-to-end framing tests against the real launcher. There was no test of `Stdio` at all, which is why the server shipped speaking LSP - framing (`Content-Length` headers) rather than the MCP stdio binding. The MCP spec at - `2024-11-05` -- the revision this server advertises -- and every revision since: + framing (`Content-Length` headers) rather than the MCP stdio binding. The server + advertises `2026-07-28` and `2025-11-25` and refuses anything older with `-32022`; + the stdio binding is unchanged across every revision, and has said since `2024-11-05`: "Messages are delimited by newlines, and MUST NOT contain embedded newlines." @@ -50,7 +51,7 @@ defmodule HacktuiAgent.MCP.StdioFramingTest do "id" => id, "method" => "initialize", "params" => %{ - "protocolVersion" => "2024-11-05", + "protocolVersion" => "2025-11-25", "capabilities" => %{}, "clientInfo" => %{"name" => "framing-test", "version" => "1"} } @@ -100,5 +101,129 @@ defmodule HacktuiAgent.MCP.StdioFramingTest do assert status == 0, "smoke client failed: #{out}" assert out =~ "MCP initialize ok" + + # The banner alone is not evidence. The client reads response["result"], which an + # error response does not carry, so it printed "MCP initialize ok" and exited 0 over + # a -32022 refusal (16j round 1, r1-B1 = r2-B1). These assert what the server + # actually returned, so a client that reports success on a refusal fails here. + assert out =~ "protocolVersion=2025-11-25" + assert out =~ "server=hacktui-hermes" + refute out =~ "=None" + end + + # 16j round 1, r2-B3 = r1-R2. The swap enlarged this surface and nothing exercised it. + # These pin what the launcher answers, so a later revision move cannot change the + # refusal or the discovery shape without a test going red. + test "a revision the server does not speak is refused with -32022, not served" do + request = + Jason.encode!(%{ + "jsonrpc" => "2.0", + "id" => 1, + "method" => "initialize", + "params" => %{ + "protocolVersion" => "2024-11-05", + "capabilities" => %{}, + "clientInfo" => %{"name" => "framing-test", "version" => "1"} + } + }) + + {out, 0} = mcp(request <> "\n") + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + + assert %{"error" => %{"code" => -32_022, "data" => data}} = response + assert data["requested"] == "2024-11-05" + assert "2025-11-25" in data["supported"] + refute Map.has_key?(response, "result") + end + + test "server/discover answers with no initialize, and batches are refused" do + {out, 0} = mcp(~s({"jsonrpc":"2.0","id":1,"method":"server/discover"}\n)) + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + + assert %{"result" => result} = response + assert result["serverInfo"]["name"] == "hacktui-hermes" + assert result["protocolVersions"] == ["2026-07-28", "2025-11-25"] + + {batch_out, 0} = mcp(~s([{"jsonrpc":"2.0","id":1,"method":"server/discover"}]\n)) + batch = batch_out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + assert %{"error" => %{"code" => code}} = batch + assert is_integer(code) + end + + # 16j round 1, r2-B6. structuredContent.error was a JSON string carrying Elixir + # inspect/1 output and is now an object. Nothing asserted the shape either way, which + # is why the change passed unnoticed. This pins the object so it cannot drift back. + test "a rejected tools/call returns structuredContent.error as an object, not a string" do + request = + Jason.encode!(%{ + "jsonrpc" => "2.0", + "id" => 1, + "method" => "tools/call", + "_meta" => %{"io.modelcontextprotocol/protocolVersion" => "2026-07-28"}, + "params" => %{"name" => "propose_action", "arguments" => %{}} + }) + + {out, 0} = mcp(request <> "\n") + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + + error = get_in(response, ["result", "structuredContent", "error"]) + + assert is_map(error), "expected an object, got: #{inspect(error)}" + assert error["tool"] == "propose_action" + assert is_binary(error["reason"]) + refute error["reason"] =~ "%{", "Elixir inspect/1 output must not reach the wire" + end + + # 16j rows 3, 4, 7 and 9. The §10 surface disclosure lives in these tests rather than in + # prose: a test pins one behaviour and fails when it moves, where a sentence about the + # surface can overstate what it covers. Each was proved red against base b425f63. + # + # ping carrying a _meta revision is refused, under 2026-07-28 and 2025-11-25 alike. + # Deliberately has no test here: it is a defect in beam_mcp 0.1.0 (SCR-257), and a test + # pinning it would pin the defect. + + test "row 3: initialize with no params answers 2025-11-25, not the pre-swap revision" do + {out, 0} = mcp(~s({"jsonrpc":"2.0","id":1,"method":"initialize"}\n)) + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + + assert response["result"]["protocolVersion"] == "2025-11-25" + end + + test "row 4: a modern-era request is answered with resultType and a _meta serverInfo" do + request = + ~s({"jsonrpc":"2.0","id":1,"method":"shutdown","_meta":{"io.modelcontextprotocol/protocolVersion":"2026-07-28"}}) + + {out, 0} = mcp(request <> "\n") + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + result = response["result"] + + assert result["resultType"] == "complete" + assert result["_meta"]["io.modelcontextprotocol/serverInfo"]["name"] == "hacktui-hermes" + end + + test "row 7: a rejected tools/call puts a plain sentence in content, not an Elixir map" do + request = + ~s({"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"propose_action","arguments":{}}}) + + {out, 0} = mcp(request <> "\n") + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + text = get_in(response, ["result", "content", Access.at(0), "text"]) + + assert is_binary(text) + assert text =~ "propose_action: invalid arguments" + refute text =~ "%{", "Elixir inspect/1 output must not reach content" + refute text =~ ":propose_action", "an Elixir atom literal must not reach content" + end + + test "row 9: an unsupported revision arriving via _meta is refused, not served" do + request = + ~s({"jsonrpc":"2.0","id":1,"method":"tools/list","_meta":{"io.modelcontextprotocol/protocolVersion":"2024-11-05"}}) + + {out, 0} = mcp(request <> "\n") + response = out |> String.split("\n", trim: true) |> List.last() |> Jason.decode!() + + assert %{"error" => %{"code" => -32_022, "data" => data}} = response + assert data["requested"] == "2024-11-05" + refute Map.has_key?(response, "result"), "the tool list must not be served" end end diff --git a/bin/hacktui-mcp-smoke b/bin/hacktui-mcp-smoke index 084a4d9..7efd24e 100755 --- a/bin/hacktui-mcp-smoke +++ b/bin/hacktui-mcp-smoke @@ -12,7 +12,7 @@ request = { "id": 1, "method": "initialize", "params": { - "protocolVersion": "2024-11-05", + "protocolVersion": "2025-11-25", "capabilities": {}, "clientInfo": {"name": "hacktui-mcp-smoke", "version": "0.1.0"} } @@ -61,7 +61,22 @@ try: print(err, file=sys.stderr) sys.exit(1) - result = response.get("result", {}) + # An error response carries "error" and no "result". Reading result alone printed + # the success banner over a -32022 refusal and exited 0 (16j round 1, r1-B1). + if "error" in response: + err = response["error"] + print( + f"initialize refused: {err.get('message')} " + f"(code {err.get('code')}) data={err.get('data')}", + file=sys.stderr, + ) + sys.exit(1) + + if "result" not in response: + print(f"initialize response has neither result nor error: {response}", file=sys.stderr) + sys.exit(1) + + result = response["result"] server = result.get("serverInfo", {}) print("MCP initialize ok") print(f"protocolVersion={result.get('protocolVersion')}") diff --git a/mix.lock b/mix.lock index 271174a..d89e6d5 100644 --- a/mix.lock +++ b/mix.lock @@ -1,5 +1,6 @@ %{ "abacus": {:hex, :abacus, "2.1.0", "b6db5c989ba3d9dd8c36d1cb269e2f0058f34768d47c67eb8ce06697ecb36dd4", [:mix], [], "hexpm", "255de08b02884e8383f1eed8aa31df884ce0fb5eb394db81ff888089f2a1bbff"}, + "beam_mcp": {:hex, :beam_mcp, "0.1.0", "bce9c8897302a7ffa146fe4d8b7250780e0bfe30862c7ebfaac06de5e91cada9", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "b8c351933260d90844eae1614ae4759cf979d4a524ee139fbbef1c2dfaab5e7f"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "certifi": {:hex, :certifi, "2.15.0", "0e6e882fcdaaa0a5a9f2b3db55b1394dba07e8d6d9bcad08318fb604c6839712", [:rebar3], [], "hexpm", "b147ed22ce71d72eafdad94f055165c1c182f61a2ff49df28bcc71d1d5b94a60"}, "combine": {:hex, :combine, "0.10.0", "eff8224eeb56498a2af13011d142c5e7997a80c8f5b97c499f84c841032e429f", [:mix], [], "hexpm", "1b1dbc1790073076580d0d1d64e42eae2366583e7aecd455d1215b0d16f2451b"},