fix(mcp): webhook envelope profile falls back to active profile (KLA-420)#39
Open
jklaassenjc wants to merge 1 commit into
Open
fix(mcp): webhook envelope profile falls back to active profile (KLA-420)#39jklaassenjc wants to merge 1 commit into
jklaassenjc wants to merge 1 commit into
Conversation
…420)
In 1.17.0, server.go wired stepUpConfig.Profile = opts.SigningProfile
directly. The signer block beneath it falls back to
config.ActiveProfile() when SigningProfile is empty, but the
webhook path didn't. Result: an operator who enables
mcp:
require_step_up_for_destructive: true
step_up_authenticator: webhook
approval_webhook_url: ...
without also enabling mcp.sign_destructive_ops gets `profile: ""`
in every outbound approval envelope. Receivers keying on profile
(Slack bot routing, audit dashboards) see empty strings instead
of the active profile name.
Fix: resolve the profile once at the server level, before building
both stepUpConfig and the signer. SigningProfile remains the
explicit opt-in; empty means "use the active profile" and now
applies uniformly to both consumers.
Tests:
- TestNewServer_WebhookProfileFallsBackToActiveProfile (empty
SigningProfile → webhook authenticator profile = "default")
- TestNewServer_WebhookProfileHonorsExplicitSigningProfile
("staging" → webhook authenticator profile = "staging")
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
KLA-420 — surfaced during the 1.17.0 code-quality review.
In 1.17.0 (KLA-413),
server.gowiredstepUpConfig.Profile = opts.SigningProfiledirectly. The signer block beneath it falls back toconfig.ActiveProfile()whenSigningProfile == "", but the webhook path didn't. Result: an operator who enables```yaml
mcp:
require_step_up_for_destructive: true
step_up_authenticator: webhook
approval_webhook_url: ...
```
without also enabling
mcp.sign_destructive_opsgets"profile": ""in every outbound approval envelope. Receivers keying on profile (Slack bot routing, audit dashboards) see empty strings instead of the active profile name.Fix
Resolve the profile once at the server level, before building both
stepUpConfigand the signer:```go
profile := opts.SigningProfile
if profile == "" {
profile = config.ActiveProfile()
}
```
SigningProfileremains the explicit opt-in; empty means "use the active profile" and now applies uniformly to both consumers.Tests
TestNewServer_WebhookProfileFallsBackToActiveProfile— emptySigningProfile→ webhook authenticator profile ="default"TestNewServer_WebhookProfileHonorsExplicitSigningProfile—"staging"→ webhook authenticator profile ="staging"Test plan
go test ./internal/mcp/ ./internal/cmd/greengo vet ./...clean🤖 Generated with Claude Code
Note
Low Risk
Small, localized server bootstrap change with regression tests; no auth or approval-gate logic changes beyond metadata in outbound envelopes.
Overview
KLA-420: Webhook step-up approval envelopes now get the same profile resolution as the destructive-op signer.
NewServerresolvesprofileonce (SigningProfile, orconfig.ActiveProfile()when empty) and passes it into bothstepUpConfigand the signer. Previously only the signer path fell back to the active profile, so webhook-only destructive approval (require_step_up+ webhook, withoutsign_destructive_ops) posted"profile": ""—breaking Slack routing and audit views that key on profile.Tests assert empty
SigningProfile→ active profile (default) onwebhookStepUp, and explicitSigningProfile→ that value unchanged.Reviewed by Cursor Bugbot for commit 6b7fc55. Bugbot is set up for automated code reviews on this repo. Configure here.