Fix: guardrail payload corruption before signing + missing after_tool for complete_task

[Deez-Automations]

Summary

Two related bugs in the Labs guardrail webhook feature, both confirmed against current source before fixing.

1. Payload truncated mid-JSON before HMAC signing

GuardrailHookService.invoke() sliced the already-serialized JSON body at a raw byte offset when it exceeded the payload size limit, then signed those truncated bytes:

body_bytes = envelope.model_dump_json().encode()
if len(body_bytes) > max_payload:
    body_bytes = body_bytes[:max_payload]   # raw byte slice
signature = self._sign_payload(body_bytes, ...)  # signs the broken bytes

Cutting a multi-byte UTF-8 character or a string/object literal mid-way produces a body that is neither valid UTF-8 nor valid JSON. The HMAC signature is computed over those broken bytes and still "validates" fine on the receiver's end — there's no way to detect the corruption from the signature alone.

Fix: _truncate_envelope_for_payload_limit() now truncates the envelope's long string fields (tool_result, user_message, model_output) individually, before serialization. Python string slicing operates on Unicode code points, so cutting a string at any character offset and then letting model_dump_json() re-serialize it always produces valid JSON, regardless of where the cut lands. tool_arguments is a dict, which can't be safely character-truncated without risking invalid JSON, so it's replaced with a small marker instead — {"_truncated": true, "original_size_bytes": N, "keys": [...]}, preserving the top-level key names so a receiver can still see which arguments existed. invoke() now also sets an X-Guardrail-Truncated: true response header when truncation occurred, so the receiver isn't left guessing.

2. after_tool never fires for the complete_task control-flow tool

In finbot/agents/base.py's agent loop, before_tool fires for every tool call including the internal complete_task tool, but complete_task returned immediately on success, bypassing the after_tool invocation every other tool call gets a few lines later. Any guardrail pairing before/after events (e.g. to measure execution time or verify output) would see every complete_task call as permanently open-ended.

Fix: fires after_tool for complete_task too, immediately before the existing completion/return logic, using the same call signature as the sibling call site.

3. Found during our own review, not by the issue: invoke() could mask a successful task completion

The new after_tool call for complete_task sits inside the same try/except as the tool call it's observing — unlike the sibling call site for ordinary tools, which sits outside it. If anything in guardrail plumbing raised (config load, envelope construction, the new truncation logic, signing — none of which were previously wrapped, only the HTTP call itself was), the surrounding except Exception would have discarded the already-successful function_output, replaced it with a generic error, and let the loop continue instead of returning — silently converting a successful complete_task into a fake failure.

Fixed at the source rather than patched around the call site: invoke() is now wrapped so it can never raise under any circumstance, matching its own documented contract ("never gates execution on the verdict"). The original body moved to a private _invoke(); the public invoke() catches everything and returns HookOutcome.error on any unexpected failure. This also means the try/except placement question becomes moot — the call is now safe wherever it's placed.

A note on merge overlap with #575

This branch and the already-open #575 (DNS-based SSRF fix, same guardrail feature) both add tests to TestWebhookInvocation in tests/unit/labs/test_guardrail_service.py. Verified independently: each branch merges cleanly into current main on its own. If both get merged, whichever lands second will show a real (if trivial) conflict in that one file, purely because both are adding tests to the same class. Flagging this now so it's not a surprise at merge time — happy to rebase/resolve once the merge order between the two is decided, just didn't want to guess at that order preemptively.

Test plan

• New TestPayloadTruncation class (tests/unit/labs/test_guardrail_service.py): unchanged-under-limit, valid-JSON-under-truncation (including multi-byte content), single/multi-field truncation, dict-marker replacement with preserved keys, no-over-truncation when barely over, and the pathological case where max_payload is smaller than the envelope's own fixed-field overhead (confirms no crash, still valid JSON, documented as a best-effort limit rather than a hard guarantee)
• New integration test test_oversized_payload_sends_valid_json_and_truncated_header: proves invoke() sends a json.loads()-able body with a signature that matches it, plus the truncation header
• New test test_invoke_never_raises_even_on_internal_failure: forces an exception in previously-unwrapped code (_sign_payload) and confirms invoke() still returns HookOutcome.error rather than propagating
• New TestGuardrailHooksAroundToolCalls class (tests/unit/agents/test_base_agent.py): confirms after_tool now fires for complete_task, plus a regression guard that ordinary tool calls still get exactly one after_tool each
• pytest tests/unit/labs/test_guardrail_service.py tests/unit/agents/test_base_agent.py -v — 38/38 passing
• pytest tests/unit/agents/ tests/unit/labs/ tests/unit/mcp/ -q — 135 passed, no new failures
• pytest tests/unit/ -q — full suite, 317 passed, 4 pre-existing failures unrelated to this change (present before it too)
• Locally merge-tested against fresh origin/main — clean on its own and combined with Fix: orchestrator confirms payment to vendor even when it fails #574; the one real conflict found (with Fix: DNS-based SSRF bypass in guardrail webhook URL validation #575, noted above) was isolated and confirmed precisely rather than assumed

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.