Skip to content

Webhook Middleware Phase 2: Validating webhook middleware#4314

Draft
Sanskarzz wants to merge 9 commits intostacklok:mainfrom
Sanskarzz:dynamicwebhook2
Draft

Webhook Middleware Phase 2: Validating webhook middleware#4314
Sanskarzz wants to merge 9 commits intostacklok:mainfrom
Sanskarzz:dynamicwebhook2

Conversation

@Sanskarzz
Copy link
Contributor

[WIP]

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
Depends on Phase 1 PR (Core Webhook Package) (This PR needs rebase onto main after Phase 1 PR gets merged.)

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 88%).

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 github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Mar 22, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Sanskarzz Sanskarzz changed the title Dynamicwebhook2 Webhook Middleware Phase 2: Validating webhook middleware Mar 22, 2026
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Mar 22, 2026
@codecov
Copy link

codecov bot commented Mar 22, 2026

Codecov Report

❌ Patch coverage is 86.91275% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.09%. Comparing base (0108b11) to head (315001a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/webhook/validating/middleware.go 84.88% 7 Missing and 6 partials ⚠️
pkg/runner/middleware.go 29.41% 11 Missing and 1 partial ⚠️
pkg/webhook/client.go 91.22% 6 Missing and 4 partials ⚠️
pkg/webhook/errors.go 93.54% 1 Missing and 1 partial ⚠️
pkg/webhook/types.go 92.30% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4314      +/-   ##
==========================================
+ Coverage   68.77%   69.09%   +0.32%     
==========================================
  Files         473      479       +6     
  Lines       47919    48184     +265     
==========================================
+ Hits        32955    33294     +339     
- Misses      12299    12300       +1     
+ Partials     2665     2590      -75     

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Webhook Middleware Phase 2: Validating webhook middleware

1 participant