Webhook Middleware Phase 2: Validating webhook middleware

[Sanskarzz]

Overview

This PR implements Phase 2 of the Dynamic Webhook Middleware feature by introducing the Validating Webhook Middleware. Validating webhooks allow ToolHive to call external HTTP services (such as policy engines, bespoke approval workflows, or rate limiters) to strictly evaluate, approve, or deny MCP requests before they reach backend tools.

Fixes #3397

Key Changes

1. pkg/webhook/validating Package

• Configuration (config.go): Added MiddlewareParams struct supporting a chain of webhook.Config elements. Includes setup validation requiring >0 webhooks to be explicitly declared.
• Middleware Handler (middleware.go):
  • Implementation of the ToolHive types.Middleware interface factory.
  • Automatically intercepts MCP POST requests (post-parsing).
  • Composes the HTTP evaluation payload, embedding the original raw JSON-RPC MCPRequest, extracting User Principal attributes directly from the auth.Identity context, and recording the request Origin Context (SourceIP, Transport, ServerName).
  • Evaluation Engine: Invokes all configured webhooks sequentially. It eagerly denies the entire request (HTTP 403) providing an optional custom error message as soon as any webhook responds with allowed: false.
  • Failure Policies: Accurately respects FailurePolicyFail (fail-closed, blocks request on network/server errors) and FailurePolicyIgnore (fail-open, logs a warning on exception but continues pipeline).
• Test Suite (middleware_test.go): Complete parallelized test-suite covering Allowed=true paths, denial paths, both failure policies, connection errors, and safe bypass for non-MCP calls. (Test Coverage sits above 75%).

2. Runner Integration (pkg/runner)

• middleware.go:
  • Registered validating.CreateMiddleware inside GetSupportedMiddlewareFactories.
  • Added dedicated configuration wiring (addValidatingWebhookMiddleware) securely positioning the validating evaluation block sequentially after mcp-parser but precisely before auditing (telemetry, authz). Thus blocking unverified telemetry pollution or unauthorized execution.
• config.go:
  • Expanded the global RunConfig exposing the ValidatingWebhooks []webhook.Config slice.

Testing Performed

• Run go test ./pkg/webhook/validating/... ./pkg/runner/... (All unit tests passing).
• Run task lint / task lint-fix against the overall project (clean).

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)
• Linting (task lint-fix)
• Manual testing (describe below)

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 70.17544% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.48%. Comparing base (9f7d1a8) to head (2994c76).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/webhook/validating/middleware.go	75.55%	17 Missing and 5 partials ⚠️
pkg/runner/middleware.go	29.41%	11 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #4314    +/-   ##
========================================
  Coverage   69.48%   69.48%            
========================================
  Files         480      482     +2     
  Lines       49032    49146   +114     
========================================
+ Hits        34069    34149    +80     
- Misses      12329    12361    +32     
- Partials     2634     2636     +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

[github-actions]

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

[github-actions]

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!

[JAORMX]

@Sanskarzz I think you need to run task docs given the swagger docs failure.

[Sanskarzz]

@Sanskarzz I think you need to run task docs given the swagger docs failure.

@JAORMX The CI is failing because swagger type fix (string -> primitive,integer). I mentioned this in the review comment discussion above.

The Fix depends on this PR merge

[JAORMX]

@Sanskarzz I missed that, let me check it out

[JAORMX]

Previous review comments have all been addressed — nice work on the updates. One new suggestion for this round regarding JSON-RPC spec compliance in error responses.

Also noting the "Docs / Verify Swagger Documentation" CI check is still failing — as discussed, this likely needs PR #4354 to merge first and then a task docs regeneration.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

[github-actions]

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!

[Sanskarzz]

@Sanskarzz I missed that, let me check it out

@JAORMX I have closed the PR, and the CI swag doc error has been fixed.

[JAORMX]

Thanks for working through all the previous feedback, @Sanskarzz! All previous blockers and suggestions are resolved. A few new things for this round... nothing blocking.

RFC/Issue consistency: Middleware chain position, failure policies, sequential eval with short-circuit, and credential-free PrincipalInfo all match the RFC. The mcp_request sends raw JSON-RPC body instead of the parsed format the RFC shows, but I think that's actually better for webhook authors. We should update the RFC to match.

Open items:

• The "integration test with full middleware chain" acceptance criterion from #3397 isn't met (tests exercise the handler directly, none go through PopulateMiddlewareConfigs -> runner). Could be a fast follow-up.
• PR description claims >88% coverage but Codecov shows 75.55% for middleware.go. Worth correcting.
• 9 commits with fix: prefixes... a squash into 1-2 clean commits (no conventional commit prefixes per repo guidelines) would be nice before merge.

[JAORMX]

This looks good to merge. The inline comments are all minor (nitpicks + a follow-up suggestion). Up to you whether you want to address them in this PR or as a follow-up... either way works for me.

[Sanskarzz]

This looks good to merge. The inline comments are all minor (nitpicks + a follow-up suggestion). Up to you whether you want to address them in this PR or as a follow-up... either way works for me.

@JAORMX I have squashed the commits into one, and I've already created the draft PR for phase 3 changes. I will address the above review comments and suggestions in Phase 3.
If this works for you, I think the PR is ready to merge.