Skip to content

fix(control-plane): require signed approval webhooks#504

Merged
AbirAbbas merged 4 commits intoAgent-Field:mainfrom
Luffy2208:fix/237-webhook-approval-hmac-auth
Apr 28, 2026
Merged

fix(control-plane): require signed approval webhooks#504
AbirAbbas merged 4 commits intoAgent-Field:mainfrom
Luffy2208:fix/237-webhook-approval-hmac-auth

Conversation

@Luffy2208
Copy link
Copy Markdown
Contributor

Summary

Phase 1 migrates the approval webhook (POST /api/v1/webhooks/approval-response) to HMAC-only authentication, removing reliance on global API key auth.

  • Bypass global API key auth for the webhook route to prevent API key exposure in callback URLs and logs.
  • Require agentfield.approval.webhook_secret (or AGENTFIELD_APPROVAL_WEBHOOK_SECRET) and reject requests if missing or invalid.
  • Enforce HMAC-SHA256 signature validation for all webhook callbacks.
  • Introduce a dedicated webhook auth level in the API catalog.

The endpoint remains registered but:

  • Returns 503 if webhook_secret is not configured (feature effectively disabled).
  • Returns 401 for missing or invalid signatures.
  • Accepts only valid HMAC-signed requests.

File-level Changes

  • control-plane/internal/server/routes_middleware.go: Skip API key auth for webhook path via SkipPaths (added helper uniqueStrings).
  • control-plane/internal/server/routes_middleware_additional_test.go: Add test ensuring webhook bypasses API key auth while other routes still enforce it.
  • control-plane/internal/handlers/webhook_approval.go: Enforce HMAC-only auth (503 if secret missing, 401 for invalid or missing signatures). Update verifySignature to return false when secret is empty.
  • control-plane/internal/handlers/webhook_approval_test.go: Update tests to include signed requests and add coverage for missing secret, missing signature, invalid signature, and t=...,v1=... signature format.
  • control-plane/internal/config/config.go: Update ApprovalConfig.WebhookSecret comment (now required; empty disables endpoint).
  • control-plane/config/agentfield.yaml: Update approval.webhook_secret documentation.
  • control-plane/internal/server/apicatalog/catalog.go: Add webhook auth level.
  • control-plane/internal/server/apicatalog/catalog_entries.go: Mark endpoint as AuthLevel: "webhook" and clarify description.
  • control-plane/internal/server/apicatalog/catalog_test.go: Extend tests for webhook auth accessibility.

Testing

  • ./scripts/test-all.sh

  • Additional verification:

    Ran targeted Go tests:

  cd control-plane && PATH=/usr/local/go/bin:$PATH go test ./internal/handlers ./internal/server/... ./internal/config

Verified:

  • Webhook rejects unsigned requests (401).
  • Webhook rejects when secret is not configured (503).
  • Valid HMAC-signed requests succeed.
  • API key auth still enforced on non-webhook routes.

Checklist

  • I updated documentation where applicable.
  • I added or updated tests (or none were needed).
  • I updated CHANGELOG.md (or this change does not warrant a changelog entry).

Screenshots

N/A (no UI changes)

Related Issues

#237

@Luffy2208 Luffy2208 requested review from a team and AbirAbbas as code owners April 27, 2026 18:34
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 86%, aggregate ≥ 88%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 87.40% 87.30% ↑ +0.10 pp 🟡
sdk-go 91.60% 90.70% ↑ +0.90 pp 🟢
sdk-python 93.63% 93.63% ↑ +0.00 pp 🟢
sdk-typescript 92.63% 92.56% ↑ +0.07 pp 🟢
web-ui 90.03% 90.01% ↑ +0.02 pp 🟢
aggregate 89.06% 89.01% ↑ +0.05 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 40 90.00%
sdk-go 0 ➖ no changes
sdk-python 0 ➖ no changes
sdk-typescript 0 ➖ no changes
web-ui 0 ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@AbirAbbas
Copy link
Copy Markdown
Contributor

thanks for the pr, will review after tests pass (currently failing)🙏

@Luffy2208
Copy link
Copy Markdown
Contributor Author

Summary

Fixed failing functional tests by updating the test harness to comply with the new
security requirement on the approval webhook endpoint:
/api/v1/webhooks/approval-response now requires an HMAC signature and a
configured secret.


Changes

tests/functional/docker/agentfield-test.yaml

  • Added agentfield.approval.webhook_secret: "test-approval-webhook-secret"
  • Added default_expiry_hours configuration

tests/functional/docker/docker-compose.local.yml & docker-compose.postgres.yml

  • Added AGENTFIELD_APPROVAL_WEBHOOK_SECRET=test-approval-webhook-secret to the
    control-plane service environment
  • Ensures the endpoint is not in "disabled" mode during functional runs

tests/functional/tests/test_waiting_state.py

  • Updated webhook calls to compute HMAC-SHA256(secret, raw_json_body)
  • Added X-Webhook-Signature: sha256=<hex> header to all webhook requests
  • Applied to both the normal flat payload helper and the hax-sdk envelope-format testSonnet 4.6

@AbirAbbas AbirAbbas enabled auto-merge April 28, 2026 12:31
@AbirAbbas AbirAbbas added this pull request to the merge queue Apr 28, 2026
Merged via the queue into Agent-Field:main with commit 002dc5a Apr 28, 2026
18 checks passed
@Luffy2208
Copy link
Copy Markdown
Contributor Author

Can i try working on phase 2?

@AbirAbbas
Copy link
Copy Markdown
Contributor

Can i try working on phase 2?

yup go for it!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants