Commit Graph
18 Commits
Author SHA1 Message Date
kevinveenbirkenbachandClaude Opus 5.5 3610e7b9ef fix(code-scanning): resolve the alerts that describe real code
Of the 76 open code scanning alerts, 39 point at code that can change.

svc-ai-mcp-adapter:
- A path argument that is empty or made only of dots is refused.
  Percent-encoding keeps a separator inside its segment, but "." and ".."
  need none, so an upstream that resolves dot segments could answer from a
  path the contract does not name.
- parse_duration splits on the units. The pattern it used before rescanned
  a long run of digits from every start position.

Tests and the README generators:
- Every Jinja environment built with autoescape=False and a "noqa: S701"
  uses select_autoescape now, as six test modules and
  utils/templating/jinja.py already did. It escapes templates named
  *.html, *.htm and *.xml, which none of these call sites loads.
- Dead code goes: a doubled "rel" block in the email lint, an unread
  _SKIP_DIRS, and a module that _resolve returned to no caller.
- Two "except ...: pass" blocks become contextlib.suppress and
  assertRaises.
- Nine test modules import unittest.mock one way instead of two, and one
  lookup test imports its module once.

Closes #807

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-06 09:18:00 +02:00
kevinveenbirkenbachandClaude Opus 5 791eee7c9f refactor(mcp)!: prove the endpoint contract with a CLI test, not a deploy probe
The contract check ran as an Ansible task in the middle of every provider deploy, and the two client checks ran the same way. A test that is part of the deploy cannot be run without deploying, cannot be re-run on its own, and reports its verdict as a deploy failure. Twenty-seven task files lose 491 lines to this.

It is now what it always was. Each provider ships files/test/test.sh, templates/test.env.j2 and meta/tests.yml, and test-e2e-cli stages one shared harness beside it: run.sh drives contract.sh in a throwaway container on the provider's own network, then isolation.sh for adapter-fronted providers. The read probe each provider offers moved from the calling task into meta/mcp.yml next to the allowlist it must satisfy, and the schema now rejects a read probe naming a tool outside that allowlist.

The bearer the test presents is the one a client resolves through mcp.credential, via the new mcp_client_bearer lookup over resolve_credential, which moved to utils.roles.applications.mcp so it is the same function mcp_servers renders every client connection with. The probe presented whatever the calling role passed, which for the fifteen adapter-fronted providers is a different secret than any client holds.

Sidecar isolation is read from the engine rather than measured inside the container, so it no longer needs a delegation to the node holding the task: attachment is compared by network name, which drops the gwbridge special case, and a swarm worker says it cannot measure instead of skipping silently.

Retrying is bounded by wall clock against MCP_DEADLINE_SECONDS, which scales through the same lookup('timeout') as the harness budget it has to stay under. A flag that is neither true nor false aborts rather than reading as off.

mcp-provider-probe becomes mcp-provider-cli-test: an enabled provider must ship the test rather than include the task. mcp-probe-bearer becomes mcp-adapter-bearer, tying ADAPTER_BEARER to the declared credential key now that no probe include carries it. A new integration test renders the shared env template for every provider through Ansible's own templar, so a later change to it breaks visibly instead of poisoning twenty deploys quietly.

Hermes and OpenClaw keep their client-side proof as CLI tests of their own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-01 20:46:26 +02:00
kevinveenbirkenbach 23688c6850 feat(svc-ai-mcp-adapter): bound a range query by the results it can return
A client picks start, end and step freely, so nothing limited how much
the upstream scanned. Against the live Prometheus contract a day at
minute resolution asks for 1441 points while the contract returns 100:
the server computed all of them and the adapter discarded 1341, with the
timeout bounding only how long it did that, four times over.

The ceiling is result_items, which the adapter already truncates to, so
the request and the response now mean the same number and no eighth
limit key was needed across seventeen roles. The refusal names the
smallest step that fits. Both boundary forms Prometheus accepts are
understood, and one that cannot be parsed is refused rather than waved
through, since a range nobody can measure is a range nobody can bound.

The rule keys on a tool declaring the three range arguments, not on a
role name, so a future range-shaped upstream inherits it.

938426da01 closed the time half of the same criterion with
--query.timeout; this is the sample half.

Also repairs one dead link the docs move left in the README.
2026-08-29 10:51:35 +02:00
kevinveenbirkenbach 88be326eb5 fix(svc-ai-mcp-adapter): ship sse.py to the container that imports it
The module was added while nothing selected it, so neither the Dockerfile
COPY nor the build-context staging listed it and nothing noticed. The
first provider to declare upstream_transport: sse would have crashed the
sidecar at import instead of serving.

Two places have to agree, because the context is assembled before it is
built: instance.yml puts the file next to the Dockerfile, the COPY takes
it into the image, and either one alone is inert.

The lint parses server.py, resolves every import that names a sibling
module, and requires each in both places, plus the reverse for a module
nobody imports.
2026-08-29 10:49:16 +02:00
kevinveenbirkenbachandClaude Opus 5 b18c507cf6 feat(svc-ai-mcp-adapter): speak the classic HTTP+SSE transport
Streamable HTTP answers a POST on the same request; classic SSE splits the channels, so a GET opens the response stream and that stream announces a second URL as the request channel. The adapter only ever spoke the former, which is what reverted the Baserow migration twice. A contract now selects the transport and defaults to streamable_http, so no existing provider changes, and an unknown value is refused at load rather than at the first tool call.

The stream is opened once and held, because reopening it per call would lose whatever answer is already in flight. A notification carries no id, so it posts without awaiting a response the protocol never sends. A dead stream is now reset alongside the session it belongs to, since rebuilding the handshake over a dead stream would keep using it.

Two things this cleaned up on the way: NoRedirect and OPENER had been copied rather than reused, so server.py now takes the one in sse.py, and decode_jsonrpc moved to passthrough.py where it belongs, since it unwraps MCP responses rather than HTTP, which also brought server.py back under the file-size limit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 00:41:55 +02:00
kevinveenbirkenbachandClaude Opus 5 63a37dac45 fix(svc-ai-mcp-adapter): enforce the contract at runtime, not only at load
Four guarantees the contract made were never applied. concurrent_requests and stream_seconds appeared nowhere but the required-key list: ThreadingHTTPServer starts a thread per connection and asks no contract, so the ceiling only holds if the handler refuses the surplus itself, which it now does with 503 and gives the slot back on the failure path as well. The upstream was opened with urlopen, which follows redirects, and urllib copies the request headers onto the redirect target, so an upstream answering 302 with an off-host location would have handed the adapter's upstream credential to whoever it named.

input_schema was published to clients and never checked, which made it a suggestion: on the openapi path an argument the contract does not name is appended to the upstream request as a query parameter, so a client could add parameters the reviewed operation never granted, and a tool declaring no schema accepted anything. A tool that declares no inputs now takes none, a required input must be present, and the check runs after the size ceiling so a large payload is still refused for being large. No shipped contract regresses: the read tool every deploy probe calls declares no required argument, measured across all fourteen contracts.

sse.py adds a client for the classic HTTP+SSE transport, which splits the response and request channels across two URLs and needs a reader thread because the answer to a POST regularly arrives before the POST returns. It has no caller yet. The deploy probe keeps its own SSE implementation deliberately: it guards every enabled surface today and fails by exit code rather than by exception.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 00:26:11 +02:00
kevinveenbirkenbachandClaude Opus 5 dfe7a7fdd4 refactor(roles): sort role scripts into per-language folders
The merged branch moved every role-local script out of files/ into a folder
named for its language (files/python/, files/php/, files/shell/), but the
moves lost their staging and never landed. HEAD still carried the old paths
while the worktree had the new ones.

Move the 24 remaining scripts and update the six places that resolve them:
the mcp-adapter instance/isolation/probe tasks, the litellm key task, the
adapter README and the endpoint-path lint. Every changed line in those
files is a path.

Also drop roles/sys-svc-compose/files/swarm/stack_ready.sh, a stale
duplicate of the copy already tracked under files/shell/swarm/.

The added comments are the branch's own `nocheck: mirrored-unit-test`
directives, which state why a script whose measurement only exists inside
its container cannot have a mirrored unit test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-28 12:00:59 +02:00
kevinveenbirkenbachandClaude Opus 5 af62d5721b fix(mcp): prove refusal on initialize, not tools/list
The negative control probed tools/list, which gitea answers with 404 "Invalid session ID" when no session exists - indistinguishable from a real refusal, while the same server serves every caller that runs initialize first. An open endpoint therefore passed as guarded. initialize is the only method that needs no session, so it reaches the auth layer.

A 4xx now refuses on its own regardless of the body; previously a clear HTTP refusal without a JSON-RPC body was classed unreachable and kept the caller retrying. 5xx stays retryable.

The unauthenticated handshake no longer leaks its session id into the authenticated pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-15 13:40:52 +02:00
kevinveenbirkenbachandClaude Opus 5 700d537855 refactor(mcp): pin the adapter base by tag where the mirror looks
The adapter was the only image in the tree pinned by a hand-maintained digest,
and the pin was never recomputed by anything. It drifted twice: run 31374959721
lost seven of eight jobs to mirror/docker.io/python@sha256:6771159c... not
found, and the renewal to the newest upstream digest earlier today would have
reproduced it, because builds resolve FROM against the GHCR mirror while the
mirror is copied by tag and lags upstream by design.

The base is now FROM <image>:<version>, taken from the sidecar service in
meta/services.yml - the only file iter_role_images scans, so the tag the build
asks for is by construction the tag the mirror was told to copy. The version
moves from the moving 3.13-slim to 3.13.15-slim, which upstream stops rewriting
once published, and which the existing version drift job renews like every other
image.

With the build reading services.yml, the adapter block's digest, image and
version had no consumer left. They are removed together with the schema check
for the digest form, the warn-only drift test, and the manifest_digest helper
that existed only for it. What remains in the block describes the adapter rather
than the image it is built from.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 23:38:53 +02:00
kevinveenbirkenbachandClaude Opus 5 f72402cbd3 fix(mcp): tell an unserved endpoint apart from an unguarded one
The unauthenticated probe read a PHP fatal rendered as HTTP 200 as proof
that the endpoint serves anyone, wrote the REJECTED marker and thereby
settled probe.yml's retry condition on the first attempt. A body that is
not JSON-RPC now counts as not-yet-serving, which is what the existing
thirty retries are for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 13:01:31 +02:00
kevinveenbirkenbach 5fa47cff67 feat(mcp): proxy native upstream sessions 2026-08-12 01:55:48 +02:00
kevinveenbirkenbachandClaude Opus 5 71d4b02e78 fix(mcp): tell upstream drift apart from a policy mismatch
The probe compared the served tools against the allowlist and nothing else,
so a vendor growing the tool surface produced the same failure text as a
provider serving the wrong set. The August image bumps therefore read as
contract violations, which sent them to the wrong reader: the contract was
fine, the recorded surface had gone stale.

Compare against upstream_serves first and report what appeared and what
disappeared, then compare against the allowlist. Two failures, two audiences.

Both transports carried the identical comparison, so the check lived twice;
it is now one function.

A provider that records no differing surface, or records it as dynamic,
passes an empty list and is judged on policy alone. Without that, nextcloud's
runtime-composed surface would report drift on every deploy.

Resolving the value needs its own task: within one set_fact a variable is not
guaranteed to see another.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-11 22:02:10 +02:00
kevinveenbirkenbachandClaude Opus 5 db982dcaed feat(svc-ai-mcp-adapter): front an upstream that already speaks MCP
The adapter could only translate REST to MCP, so a provider whose upstream
already serves MCP tools had no way to sit behind it. Those providers are
exactly the ones with no enforcement point: their declared allowlist is a
statement about the upstream rather than a constraint on it, and a client
can reach any operation the upstream happens to serve by naming it.

Add the policy layer for that case. An MCP upstream exposes no HTTP method,
so a tool's blast radius cannot be inferred the way the REST layer infers
it from GET/HEAD; every tool must declare whether it mutates, and one that
does not say is refused at startup rather than assumed harmless.

Forwarding builds its own JSON-RPC envelope with a fixed method and a fresh
id. The client's envelope is never passed through, or a caller could reach
another method by nesting it in params. A single SSE frame is unwrapped,
because a streamable-http server may answer one call that way and plain
JSON parsing would choke on the data prefix.

Routing is deliberately total: a contract that omits upstream_kind is a
REST one, which is what every adapter renders today, so the ten existing
instances are untouched.

Report an upstream that grew a tool instead of filtering it forever, and
one that dropped a contracted tool, so drift is visible either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-11 21:13:46 +02:00
kevinveenbirkenbach cc0a85b750 docs(mcp): record why the tool-set assertion cannot be relaxed to a subset
Six enabled providers carry a non-empty allowlist, declare
mutating_tools_enabled: false and have no adapter in front of them. That
flag is enforced only inside the adapter, so for those six the exact
match is the only runtime evidence that nothing beyond the declared
read-only surface is served. An upstream release adding a mutating tool
would otherwise deploy green against metadata promising none.
2026-08-10 13:33:56 +02:00
kevinveenbirkenbach 09d4e496cc fix(mcp): build the adapter against the base image digest it declares
The metadata named infinito/mcp-adapter:1.0.0 with an all-zero digest,
an image that does not exist, while the build actually used
python:3.13-slim. Declaration and artifact disagreed, so the declaration
proved nothing. Both now name the real base image and pin it by digest,
which a tag cannot guarantee across a rebuild.

The adapter base moves from the provider's service version to
mcp.adapter.digest, alongside the tool specification and its checksum.
2026-08-09 23:20:58 +02:00
kevinveenbirkenbach e49e940019 fix(mcp): verify sidecar isolation by identity instead of by count
Counting interfaces held only while a sidecar reached its provider alone.
Once it also reaches an admitted client, a count cannot tell that second
attachment from an unrelated network someone joined it to. The probe now
compares each interface address against the declared subnets and rejects
both an address outside the set and a declared subnet with no address,
so an admitted client that cannot reach the sidecar fails loudly too.

In swarm the runtime attaches every task to docker_gwbridge for ingress
and egress. That subnet joins the expected set, because otherwise the
check rejects every sidecar there while proving nothing.
2026-08-09 23:19:01 +02:00
kevinveenbirkenbach f4398e1c58 feat(inventory): pin a password on every user the roles declare
A user definition that declares no password falls back to
{{ 42 | strong_password }}. That is a template, not a value: it is re-rendered
at every use site, so the task that creates an account and the task that later
authenticates as it read two different secrets. The account exists and the
login fails. Inventory creation now decides the value once, after the roles are
resolved and next to the role credentials that were already generated there.
The scope follows the deployment: only roles resolved into this inventory
contribute users, an existing password is never overwritten, and a second run
writes nothing.

Building it duplicated what the package already carried three to seven times
over: an ensure-map helper defined three times, the same fifteen-line
round-trip load repeated seven times, and the vault encrypt-and-reparse dance
twice. They now live in one module, the round-trip counterpart to the existing
read-only yaml_io. host_vars drops from 213 to 130 lines, and its two override
entry points share the merge they used to copy. Behaviour is unchanged, and
that is measured rather than asserted: the committed versions and the new ones
were driven against the same host_vars and produced byte-identical output for
all six writers, comment placement and vault block layout included.

The adapter probes move into their own folder because a shared file-name prefix
reads as a folder in disguise, and the lint that enforces it scans the tracked
file list, so the repository was failing on it. A test that was defined at
module level instead of inside its class, and therefore never ran, is indented
back in.
2026-08-06 19:54:19 +02:00
kevinveenbirkenbachandClaude Opus 5 683de0ab9a feat(svc-ai-mcp-adapter): add the reusable per-provider MCP adapter
One audited implementation, instantiated once per provider application rather
than deployed as a single process holding every application's credential. Each
instance joins only its provider's network, carries only that provider's
least-privileged credential and serves a distinct endpoint.

Every decision lives in the policy layer, which validates the contract at
startup rather than at first call, so an instance missing a limit, an
allowlist or the schema hash never reaches a client. The server itself only
moves bytes over three JSON-RPC methods, with no stream state to get wrong.
Its upstream header is configurable because only half the providers accept an
RFC 6750 bearer.

The contract probe runs at deploy time rather than in Playwright, because it
needs the provider's real credential and a Playwright trace would capture it.
It proves, over both transports, that an unauthenticated caller is refused,
that a wrongly authenticated one is refused, that the handshake completes,
that the served tool list is exactly the contracted one, and that one
deterministic read answers without an error. Steps one and two are what
separate a guarded endpoint from one that merely happens to be unreachable.

The isolation probe counts the sidecar's network interfaces from inside it.
Reachability cannot be used here: an unknown name does not fail to resolve in
this environment, it falls through to a wildcard resolver that answers every
name with one reachable address.

The role is inert until a provider includes it; those inclusions follow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-06 14:04:08 +02:00