Dev #72
No files matched your search
@@ -28,8 +28,15 @@ A tool invocation payload records:
|
||||
- the HTTP status when Gitea answered, and the outcome: success, or the error kind
|
||||
(`not_found`, `forbidden`, `invalid_argument`, `internal_error`, ...);
|
||||
- the duration in milliseconds and the correlation id;
|
||||
- the argument names, sorted. Argument values are never logged, so request bodies, secret values
|
||||
and file contents do not reach the log.
|
||||
- who called: the MCP client id of the verified token (`aegis-static-bearer` for the static
|
||||
bearer), a 12 character hash of the OAuth grant's session id (`session`, only for OAuth
|
||||
tokens, never the id or a token) and the source address (`client_ip`, HTTP only; behind a
|
||||
proxy it is the forwarded address only when the proxy is in `FORWARDED_ALLOW_IPS`);
|
||||
- the argument names, sorted, and only names that are properties of the tool's schema; any other
|
||||
name is counted in `rejected_argument_count`. Argument values are never logged, so request
|
||||
bodies, secret values and file contents do not reach the log. A call to a tool that does not
|
||||
exist is recorded as `(unknown tool, N chars)`: the caller's string is neither logged nor
|
||||
echoed in the error.
|
||||
|
||||
All payloads also pass through secret masking before they are written
|
||||
(`SECRET_DETECTION_MODE` other than `off`).
|
||||
|
||||
@@ -129,13 +129,17 @@ class AuditLogger:
|
||||
identity_login: str | None = None,
|
||||
identity_mode: str | None = None,
|
||||
sudo: str | None = None,
|
||||
client_id: str | None = None,
|
||||
session: str | None = None,
|
||||
client_ip: str | None = None,
|
||||
outcome: str | None = None,
|
||||
duration_ms: float | None = None,
|
||||
) -> str:
|
||||
"""Log an MCP tool invocation.
|
||||
|
||||
The keyword-only fields describe a server tool call (tool domain, HTTP
|
||||
method, path template, identity, sudo target, outcome and duration). They
|
||||
method, path template, identity, sudo target, the MCP client id, a short hash
|
||||
of the grant's session and the source address, outcome and duration). They
|
||||
are only written when given. Callers must pass argument names, never
|
||||
argument values, in ``params``.
|
||||
"""
|
||||
@@ -154,6 +158,9 @@ class AuditLogger:
|
||||
"identity_login": identity_login,
|
||||
"identity_mode": identity_mode,
|
||||
"sudo": sudo,
|
||||
"client_id": client_id,
|
||||
"session": session,
|
||||
"client_ip": client_ip,
|
||||
"outcome": outcome,
|
||||
"duration_ms": duration_ms,
|
||||
}
|
||||
|
||||
@@ -11,6 +11,8 @@ values are never logged or echoed in validation errors.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import ipaddress
|
||||
import json
|
||||
import logging
|
||||
import time
|
||||
@@ -78,7 +80,7 @@ _MAX_ALLOWED_NAMES = 40
|
||||
_MAX_NAME_CHARS = 64
|
||||
_MAX_AUDIT_ARGUMENT_NAMES = 64
|
||||
_MAX_AUDIT_PATH_CHARS = 200
|
||||
_MAX_UNKNOWN_NAME_CHARS = 128
|
||||
_MAX_CLIENT_ID_CHARS = 128
|
||||
#: Slack added to the upload-derived HTTP body cap for JSON framing and other arguments.
|
||||
BODY_OVERHEAD_BYTES = 1024 * 1024
|
||||
|
||||
@@ -357,6 +359,34 @@ class _Record:
|
||||
is_error: bool = True
|
||||
http_status: int | None = None
|
||||
argument_names: list[str] = field(default_factory=list)
|
||||
rejected_arguments: int = 0
|
||||
client_id: str | None = None
|
||||
session: str | None = None
|
||||
client_ip: str | None = None
|
||||
|
||||
|
||||
def _source_ip(ctx: ServerRequestContext[dict[str, Any]]) -> str | None:
|
||||
"""Client address of an HTTP request (None on stdio); only a valid IP address is recorded."""
|
||||
client = getattr(getattr(ctx, "request", None), "client", None)
|
||||
host = getattr(client, "host", None)
|
||||
if not isinstance(host, str):
|
||||
return None
|
||||
try:
|
||||
return str(ipaddress.ip_address(host.split("%")[0]))
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
def _caller_fields(record: _Record, ctx: ServerRequestContext[dict[str, Any]]) -> None:
|
||||
"""Fill the client, session and source address of the call from the verified token."""
|
||||
access = get_access_token()
|
||||
if access is not None:
|
||||
record.client_id = access.client_id[:_MAX_CLIENT_ID_CHARS]
|
||||
session_id = (access.claims or {}).get("session_id")
|
||||
if isinstance(session_id, str) and session_id:
|
||||
# A short hash: enough to correlate entries of one grant, useless as a credential.
|
||||
record.session = hashlib.sha256(session_id.encode()).hexdigest()[:12]
|
||||
record.client_ip = _source_ip(ctx)
|
||||
|
||||
|
||||
def _error_payload(kind: str, message: str, **extra: str) -> dict[str, Any]:
|
||||
@@ -409,8 +439,12 @@ class _Dispatcher:
|
||||
request_id = uuid.uuid4().hex
|
||||
set_request_id(request_id)
|
||||
arguments = params.arguments or {}
|
||||
record = _Record(tool=params.name[:_MAX_UNKNOWN_NAME_CHARS], request_id=request_id)
|
||||
record.argument_names = sorted(_clip(key) for key in arguments)[:_MAX_AUDIT_ARGUMENT_NAMES]
|
||||
# The tool name and the argument names come from the caller: only names the
|
||||
# registry knows are recorded, as the audit entry must never hold a secret a
|
||||
# model pasted into a name.
|
||||
record = _Record(tool=f"(unknown tool, {len(params.name)} chars)", request_id=request_id)
|
||||
record.rejected_arguments = len(arguments)
|
||||
_caller_fields(record, ctx)
|
||||
# Fail closed: the audit logger is obtained BEFORE anything runs, so a call is
|
||||
# never executed (and its side effects never happen) without a way to record it.
|
||||
try:
|
||||
@@ -428,8 +462,14 @@ class _Dispatcher:
|
||||
entry = self._registry.get(params.name)
|
||||
if entry is None:
|
||||
record.outcome = "unknown_tool"
|
||||
raise MCPError(types.INVALID_PARAMS, f"Unknown tool: {record.tool}")
|
||||
raise MCPError(types.INVALID_PARAMS, "Unknown tool")
|
||||
record.tool = entry.tool.name
|
||||
record.domain = entry.domain
|
||||
known = set(entry.tool.input_schema.get("properties", {}))
|
||||
record.argument_names = sorted(key for key in arguments if key in known)[
|
||||
:_MAX_AUDIT_ARGUMENT_NAMES
|
||||
]
|
||||
record.rejected_arguments = len(arguments) - len(record.argument_names)
|
||||
# Write-ahead entry: a call whose start cannot be recorded does not run.
|
||||
try:
|
||||
audit.log_tool_call_started(entry.tool.name, request_id, entry.domain)
|
||||
@@ -529,6 +569,8 @@ class _Dispatcher:
|
||||
execution, in :meth:`call_tool`.
|
||||
"""
|
||||
params: dict[str, Any] = {"argument_names": record.argument_names}
|
||||
if record.rejected_arguments:
|
||||
params["rejected_argument_count"] = record.rejected_arguments
|
||||
if record.http_status is not None:
|
||||
params["http_status"] = record.http_status
|
||||
try:
|
||||
@@ -543,6 +585,9 @@ class _Dispatcher:
|
||||
identity_login=record.identity.login if record.identity else None,
|
||||
identity_mode=record.identity.mode if record.identity else None,
|
||||
sudo=record.sudo,
|
||||
client_id=record.client_id,
|
||||
session=record.session,
|
||||
client_ip=record.client_ip,
|
||||
outcome=record.outcome,
|
||||
duration_ms=round(duration_ms, 2),
|
||||
)
|
||||
|
||||
@@ -3,13 +3,16 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from mcp import MCPError
|
||||
from pytest_httpx import HTTPXMock
|
||||
|
||||
from aegis_gitea_mcp.audit import get_audit_logger
|
||||
from tests.test_server_helpers import API, connected, mock_user
|
||||
from tests.test_oauth_helpers import BEARER_TOKEN, browser, build_oauth_app, mcp_client, sign_in
|
||||
from tests.test_server_helpers import API, audit_entries, connected, mock_user
|
||||
|
||||
REPO_ARGS = {"owner": "octo", "repo": "demo"}
|
||||
|
||||
@@ -77,3 +80,72 @@ async def test_the_refusal_does_not_leak_the_failure_reason() -> None:
|
||||
|
||||
text = result.content[0].text # type: ignore[union-attr]
|
||||
assert "No space left" not in text and "OSError" not in text
|
||||
|
||||
|
||||
# SEC-12: who made the call
|
||||
|
||||
|
||||
async def test_oauth_call_entry_names_the_client_and_never_a_token_or_session_id(
|
||||
httpx_mock: HTTPXMock,
|
||||
) -> None:
|
||||
oauth = build_oauth_app()
|
||||
flow = sign_in(browser(oauth.app), httpx_mock)
|
||||
session_ids = list(oauth.store.sessions)
|
||||
httpx_mock.add_response(url=f"{API}/repos/octo/demo", json={"full_name": "octo/demo"})
|
||||
access = flow.tokens["access_token"]
|
||||
|
||||
async with mcp_client(oauth.app, access) as mcp:
|
||||
result = await mcp.call_tool("gitea_repo_get", REPO_ARGS)
|
||||
assert not result.is_error
|
||||
|
||||
audit_text = get_audit_logger().log_path.read_text(encoding="utf-8")
|
||||
(entry,) = audit_entries()
|
||||
assert entry["client_id"] == flow.client_id
|
||||
assert re.fullmatch(r"[0-9a-f]{12}", entry["session"])
|
||||
assert entry["client_ip"] == "127.0.0.1"
|
||||
assert entry["identity_login"] == "alice"
|
||||
assert access not in audit_text
|
||||
assert flow.tokens["refresh_token"] not in audit_text
|
||||
assert all(session_id not in audit_text for session_id in session_ids)
|
||||
|
||||
|
||||
async def test_static_bearer_call_entry_names_the_static_client(httpx_mock: HTTPXMock) -> None:
|
||||
mock_user(httpx_mock)
|
||||
httpx_mock.add_response(url=f"{API}/repos/octo/demo", json={})
|
||||
oauth = build_oauth_app()
|
||||
async with mcp_client(oauth.app, BEARER_TOKEN) as mcp:
|
||||
await mcp.call_tool("gitea_repo_get", REPO_ARGS)
|
||||
|
||||
(entry,) = audit_entries()
|
||||
assert entry["client_id"] == "aegis-static-bearer"
|
||||
assert "session" not in entry
|
||||
assert BEARER_TOKEN not in get_audit_logger().log_path.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
# SEC-18: caller-chosen names
|
||||
|
||||
TOKEN_LIKE = "0123456789abcdef0123456789abcdef01234567"
|
||||
|
||||
|
||||
async def test_extra_argument_names_are_counted_not_recorded() -> None:
|
||||
async with connected() as client:
|
||||
result = await client.call_tool("gitea_repo_get", {**REPO_ARGS, TOKEN_LIKE: "x"})
|
||||
|
||||
assert result.is_error
|
||||
assert TOKEN_LIKE not in result.content[0].text # type: ignore[union-attr]
|
||||
(entry,) = audit_entries()
|
||||
assert entry["params"] == {"argument_names": ["owner", "repo"], "rejected_argument_count": 1}
|
||||
assert TOKEN_LIKE not in get_audit_logger().log_path.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
async def test_unknown_tool_names_are_neither_echoed_nor_recorded() -> None:
|
||||
name = "gitea_" + TOKEN_LIKE
|
||||
async with connected() as client:
|
||||
with pytest.raises(MCPError) as caught:
|
||||
await client.call_tool(name, {TOKEN_LIKE: 1})
|
||||
|
||||
assert TOKEN_LIKE not in caught.value.message
|
||||
(entry,) = audit_entries()
|
||||
assert entry["tool_name"] == f"(unknown tool, {len(name)} chars)"
|
||||
assert entry["params"] == {"argument_names": [], "rejected_argument_count": 1}
|
||||
assert TOKEN_LIKE not in get_audit_logger().log_path.read_text(encoding="utf-8")
|
||||
@@ -354,9 +354,9 @@ async def test_unknown_tool_is_a_protocol_error_and_is_audited() -> None:
|
||||
await client.call_tool("gitea_does_not_exist", {"a": 1})
|
||||
|
||||
assert caught.value.code == -32602
|
||||
assert "gitea_does_not_exist" in caught.value.message
|
||||
assert "gitea_does_not_exist" not in caught.value.message
|
||||
entry = audit_entries()[-1]
|
||||
assert entry["tool_name"] == "gitea_does_not_exist"
|
||||
assert entry["tool_name"] == "(unknown tool, 20 chars)"
|
||||
assert entry["outcome"] == "unknown_tool"
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user