Pressure-test hardening: report_id/order_id validation, groups-claim robustness, error sanitization

[MattJColes]

Summary

I ran the server and pressure-tested every surface (auth/access middleware, the S3 file-delivery path, the audit trail, the agent-behind-a-tool, and the composition/proxy wiring) using both live in-memory Client probes and a fan-out of focused review agents. The core two-layer access control is genuinely sound and fail-closed — I could not find a privilege-escalation bypass. What I did find were five real robustness/security defects around input validation and internal-detail leakage. Each fix ships with regression tests.

Baseline was 54 tests passing; now 77 passing (23 new).

Bugs fixed

1. export_report — weak report_id validation + error leakage (domains/reports.py)

   • Denylist (/, \, ..) let surprising ids through: surrounding whitespace landed in the S3 key verbatim (blank-check used .strip() but the key used the raw value), control bytes (NUL/CR/LF) reached the key and any key-logging pipeline, and over-length ids failed deep in S3. It also false-rejected legitimate dotted ids like 2024.q1.
   • A boto ClientError propagated verbatim, leaking the bucket name, S3 operation, and AWS error code into the caller's (and model's) context.
   • Fix: strict allowlist ([A-Za-z0-9._-], length-bounded) so prefix escape stays impossible while dotted ids work again; wrap the S3 calls and return an opaque report export failed, logging detail server-side.

2. allowed_tags — crash and lockout on real IdP tokens (auth.py)

   • groups: null in the claims → TypeError: 'NoneType' object is not iterable surfaced to the caller (dict.get(..., []) doesn't cover explicit null).
   • groups: "admin" (single group as a bare string) → iterated per-character, matched nothing, silently locked the caller out.
   • Fix: _normalize_groups() coerces the claim (None/missing → none, bare string → one group, list/tuple/set → its string members, else → none). Every path fails closed.

3. on_call_tool — get_tool failure leaks internal detail (access.py)

   • get_tool(name) was unwrapped. In-process tools return None on an unknown name, but a proxied tool (the shipped config mounts a remote analytics proxy) can raise a backend/connection error at lookup, whose raw text names internal hosts/operations and defeats the uniform "Unknown tool" response.
   • Fix: wrap the lookup; treat any failure as unknown so the answer stays uniform and the gate stays closed. Also corrected the module docstring, which claimed support can't see finance tools — the real policy (and README) grant support the billing tag.

4. draft_refund_email — prompt injection via order_id (domains/support.py)

   • A caller-controlled order_id was interpolated straight into the prompt handed to the injected inner agent, with no validation. An id like A1. Ignore prior instructions and ... rode verbatim into the instructions a production model would execute; the same unbounded input inflated the prompt (token-cost/DoS) and pushed control chars into the ctx.info audit line.
   • Fix: validate order_id against a tight allowlist and reject before the agent is called; reword the docstring to describe the real boundary.

5. issue_refund — misleading bool-guard comment (domains/admin.py)

   • The comment claimed isinstance(amount, bool) stops True/False passing as 1/0, but over the MCP wire the float schema coerces JSON true/false to 1.0/0.0 before the body runs, so amount=true silently records a 1.0 refund. Documented that the guard is defense-in-depth for direct callers and that the real guarantees are the positive-and-finite checks.

Testing

python -m pytest -q → 77 passed. New tests cover: unsafe/whitespace/control-char/over-length/dotted report_id, masked storage errors, groups: null/scalar-string/odd-type normalization, get_tool-raises sanitization, and order_id injection rejection.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F5dytZCaHHE432vqopou4h

---

Generated by Claude Code