Skip to content

Outbound webhook notifications for proposal creation - #331

Open
wankhede04 wants to merge 2 commits into
Infisical:mainfrom
wankhede04:feat/proposal-webhook-notifications
Open

Outbound webhook notifications for proposal creation#331
wankhede04 wants to merge 2 commits into
Infisical:mainfrom
wankhede04:feat/proposal-webhook-notifications

Conversation

@wankhede04

Copy link
Copy Markdown

Closes #328.

What

Adds outbound webhook notifications for proposal lifecycle events, following the same "env var enables it" pattern already used for SMTP (AGENT_VAULT_SMTP_*):

  • AGENT_VAULT_NOTIFY_WEBHOOK_URL — generic webhook, POSTs a JSON payload (proposal id, vault, agent, message, approval URL) on proposal.created.
  • AGENT_VAULT_NOTIFY_FORMAT=generic|slackslack sends a {"text": ...} body compatible with Slack Incoming Webhooks; close enough to Discord's webhook API to work there too without extra config.

This is scoped as a notification side-channel only — it doesn't touch credential handling, the proxy path, or the approval/auth flow.

Why

Agent Vault's core UX loop depends on a human noticing and approving a proposal quickly. Email-only (and often not even configured) is a real gap for teams that live in Slack/Discord. The webhook channel fires independently of email — a vault with no human members (so no email recipients) still gets notified.

Changes

  • internal/notify: new WebhookConfig + Notifier.SendProposalWebhook, generic JSON and Slack-compatible payload shapes.
  • internal/server: fire the webhook on proposal.created independently of the email path; new owner-only POST /v1/admin/notify/webhook/test endpoint; webhook_configured added to GET /v1/admin/settings.
  • cmd: new agent-vault owner notify webhook test CLI command, mirroring owner email test.
  • web: matching "Test Webhook" section in the instance Settings tab.
  • Docs: .env.example, docs/self-hosting/environment-variables.mdx, docs/reference/cli.mdx, docs/learn/proposals.mdx.

Testing

  • New unit tests for WebhookConfig loading, SendProposalWebhook (generic + Slack payloads, non-2xx handling, unreachable URL), the /v1/admin/notify/webhook/test handler (owner-gating, not-configured, success, upstream failure), and a regression test proving the webhook fires even when a vault has zero email recipients.
  • go build ./..., go vet ./..., and go test ./... all pass (one pre-existing, unrelated failure in internal/isolation — a Docker-socket-path test that also fails on main in this sandbox, unrelated to this change).
  • cd web && npx tsc --noEmit passes.

Follow-ups (happy to take these on separately if useful)

  • Additional events (e.g. proposal.expiring_soon, 24h before the 7-day TTL) as a second nudge before a proposal expires.

Adds AGENT_VAULT_NOTIFY_WEBHOOK_URL / AGENT_VAULT_NOTIFY_FORMAT so teams
that don't watch email can route proposal.created events to Slack,
Discord, or a generic HTTP endpoint, following the same "env var enables
it" pattern as AGENT_VAULT_SMTP_*.

- internal/notify: new WebhookConfig + Notifier.SendProposalWebhook,
  generic JSON and Slack-compatible {"text": ...} payload shapes
- server: fire webhook independently of email on proposal creation (a
  vault with no human members still gets notified), plus an owner-only
  /v1/admin/notify/webhook/test endpoint and settings flag
- cli: `agent-vault owner notify webhook test`
- web: matching "Test Webhook" section in instance settings
- docs: .env.example, environment-variables.mdx, reference/cli.mdx,
  learn/proposals.mdx

Closes Infisical#328
@infisical-cla-app

infisical-cla-app Bot commented Jul 23, 2026

Copy link
Copy Markdown

CLA satisfied. All contributors have signed the current CLA. The cla/signed check is passing.

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds outbound webhook notifications for newly created proposals. The main changes are:

  • Generic and Slack-compatible webhook payloads.
  • Proposal notification fan-out independent of email.
  • An owner-only test endpoint and CLI command.
  • Webhook status and testing controls in instance settings.
  • Environment variable, proposal, and CLI documentation.

Confidence Score: 4/5

The outbound request policy and Unicode truncation path need fixes before merging.

  • Webhook requests can resolve or redirect to private network destinations.
  • Multibyte proposal messages can be corrupted during truncation.
  • The test endpoint, CLI wiring, and settings integration are otherwise consistent.

internal/notify/webhook.go, internal/server/handle_proposals.go, and docs/self-hosting/environment-variables.mdx

Security Review

The webhook HTTP client does not apply the repository's outbound network policy. Direct destinations, DNS resolutions, and redirects can reach private or metadata services.

Important Files Changed

Filename Overview
internal/notify/webhook.go Adds webhook configuration and delivery, but the HTTP client omits outbound destination controls.
internal/server/handle_proposals.go Separates email and webhook fan-out, but byte-based truncation can corrupt Unicode messages.
internal/server/handle_settings.go Adds the owner-checked webhook test handler and exposes webhook configuration state.
internal/server/server.go Registers the authenticated webhook test route.
cmd/notify.go Adds the nested owner command for sending a test webhook.
web/src/pages/instance/SettingsTab.tsx Adds webhook configuration status and test controls to instance settings.
docs/self-hosting/environment-variables.mdx Documents webhook configuration, but the test command omits its owner parent.

Reviews (1): Last reviewed commit: "feat: outbound webhook notifications for..." | Re-trigger Greptile

Comment thread internal/notify/webhook.go Outdated

// webhookHTTPClient is shared across sends; short timeout since this is a
// fire-and-forget best-effort notification, not on the request's critical path.
var webhookHTTPClient = &http.Client{Timeout: 10 * time.Second}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 security Webhook Bypasses Network Policy

This client uses the default transport and follows redirects without applying the existing private-range and allowlist checks. A configured endpoint that resolves or redirects to loopback, link-local, cloud metadata, or another internal address can make proposal creation send requests into the server's private network.

Context Used: Flag SSRF risks (source)

Comment thread internal/server/handle_proposals.go Outdated
Comment on lines +259 to +260
// Truncate for notification bodies.
msg := message

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Byte Truncation Corrupts Unicode

When byte 200 falls inside a multibyte character, msg[:200] produces invalid UTF-8. JSON encoding replaces the broken character and the email path receives the same malformed string, so proposal messages containing emoji or other non-ASCII text can be corrupted in both notification channels.

```

<Tip>
To verify the webhook is working, run `agent-vault notify webhook test`. It sends a synthetic `proposal.created` event to the configured URL.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Documented Command Misses Parent

The new command is registered below owner, so the documented agent-vault notify webhook test invocation fails as an unknown command. The working command is agent-vault owner notify webhook test, as shown in the CLI reference.

Suggested change
To verify the webhook is working, run `agent-vault notify webhook test`. It sends a synthetic `proposal.created` event to the configured URL.
To verify the webhook is working, run `agent-vault owner notify webhook test`. It sends a synthetic `proposal.created` event to the configured URL.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

- Apply netguard SSRF protection to the outbound webhook HTTP client so an
  operator-supplied webhook URL (or a redirect it returns) can't be used to
  reach loopback, RFC-1918, link-local, or cloud metadata addresses.
- Truncate proposal notification messages by rune count instead of byte
  count, so multi-byte UTF-8 characters (emoji, accents, CJK) aren't split
  and corrupted when a message exceeds 200 characters.
- Fix the documented webhook test command to include the `owner` parent
  command, matching the actual CLI surface.
@wankhede04

Copy link
Copy Markdown
Author

Thanks for the review, @greptile-apps! Addressed all three points in ecea411:

  1. SSRF (webhook bypasses network policy) — the webhook HTTP client now uses a transport wired through netguard.SafeDialContext, the same SSRF guard used for the OAuth token client and MITM proxy egress. Loopback/RFC-1918/link-local/metadata targets are blocked by default (only allowed via AGENT_VAULT_ALLOW_PRIVATE_RANGES=true, matching existing behavior elsewhere in the codebase). Added TestSendProposalWebhook_BlocksPrivateRangesByDefault to cover it.
  2. Byte truncation corrupts Unicode — added a rune-aware truncateRunes helper (mirrors the existing truncateDeviceLabel pattern in server.go) and swapped the msg[:200] byte slice for it. Added TestTruncateRunes_PreservesUTF8Validity.
  3. Docs command missing owner parent — fixed the environment-variables.mdx tip to say agent-vault owner notify webhook test.

All existing + new tests pass (go test ./..., excluding one pre-existing unrelated failure in internal/isolation on main).

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

All three fixes look correct and well-implemented. Here's a quick summary of what's in ecea411:

  1. SSRF fixwebhookHTTPClient() clones http.DefaultTransport, sets Proxy: nil, and wires DialContext to netguard.SafeDialContext(netguard.AllowPrivateFromEnv()). This is exactly the right pattern and matches the existing OAuth/MITM egress path.

  2. Unicode truncationtruncateRunes in handle_proposals.go correctly uses utf8.RuneCountInString and []rune conversion before slicing, appending "...". Clean and consistent with the existing truncateDeviceLabel pattern.

  3. Docs fix — straightforward one-liner update.

The notifyProposalCreated refactor is also a nice side effect — separating email and webhook into independent calls with their own guard clauses makes the intent clearer.

LGTM. Thanks for the thorough follow-up.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

@wankhede04

Copy link
Copy Markdown
Author

All Greptile review comments have been addressed and fixed (see commit ecea411 above) — SSRF protection added to the webhook client, Unicode-safe truncation, and the docs typo fixed.

This should be ready for a maintainer review whenever you get a chance. Would appreciate it if someone from the team could take a look and merge if it looks good. Thanks!

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.

Outbound notifications (Slack/Discord/webhook) for pending proposals

1 participant