dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(plugins): accept same-origin GET on session-authenticated webhooks (#2984)

Browse Source 
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
João Salavisa
2026-08-24 20:31:32 +00:00
committed by GitHub
parent f4bc106db5
commit 9b0f6cf809
4 changed files with 231 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/plugins/handlers.go
+63 -8
View File
@@ -455,18 +455,73 @@ func webhookCallerAuthorized(ctx *gin.Context, public bool) bool {
ctx.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "authentication required"})
return false
}
if identity.SessionID != "" {
origin := ctx.GetHeader("Origin")
if origin == "" || !commonhttpmw.AllowedOrigin(origin, ctx.Request.Host) {
ctx.AbortWithStatusJSON(http.StatusForbidden, gin.H{
"error": "an accepted Origin is required for session-authenticated webhooks",
})
return false
}
if identity.SessionID != "" && !webhookSameOriginRequest(ctx) {
ctx.AbortWithStatusJSON(http.StatusForbidden, gin.H{
"error": "a same-origin request is required for session-authenticated webhooks",
})
return false
}
return true
}
// webhookSameOriginRequest reports whether a session-authenticated webhook
// call is a genuine same-origin request rather than cross-site forgery.
//
// A session identity rides on an ambient cookie, so a page on another site can
// make the browser attach it; a PAT or the synthetic auth-disabled identity
// cannot be borrowed that way, which is why only sessions reach this check.
// The signal is the request's origin, never its method: Kandev does not
// enforce webhooks[].method and cannot see whether a plugin's GET handler has
// side effects, so exempting safe verbs would reopen exactly the hole this
// gate closes.
//
// The two signals, in order:
//
// - Origin present: decided by httpmw.AllowedOrigin, the shared origin trust
// policy. Every cross-origin request carries Origin, including the SPA's
// own fetch in a split-origin or desktop install (frontend and backend on
// different ports, see apps/web/lib/plugins/host-api.ts), which the browser
// labels Sec-Fetch-Site: same-site. Origin therefore has to decide alone
// here; also demanding same-origin fetch metadata would break those installs.
//
// - Origin absent: decided by Sec-Fetch-Site. Browsers do not send Origin on
// a same-origin GET or HEAD (per Fetch, it is attached to cross-origin
// requests and to same-origin requests whose method is neither GET nor
// HEAD), so an absent Origin is the normal state of a plugin panel polling
// its own webhook, not evidence of a cross-origin caller. Requiring Origin
// unconditionally refused every such poll on any auth-enabled instance.
//
// same-origin and none (a user-initiated request with no initiator: address
// bar, bookmark) are accepted; cross-site and same-site are refused. Refusing
// cross-site closes the one ambient-credential vector left on this route: the
// SameSite=Lax session cookie is not sent on a cross-site subresource request
// or POST, but it *is* sent on a cross-site top-level GET navigation, which
// carries no Origin either.
//
// Neither header is refused. A session cookie with no origin signal at all is
// indistinguishable from a cross-site top-level GET navigation made by a
// browser that predates Fetch Metadata (or behind something stripping it),
// which SameSite=Lax does attach the cookie to, so accepting it would reopen
// the CSRF path this gate exists to close. That costs nothing anyone has
// today: such a request is already refused before this change, so refusing it
// still is not a regression, and the deliberate non-browser caller (curl, a
// CLI, a server-side integration) has a credential built for exactly this,
// the PAT, which is not ambient and is not gated here at all.
func webhookSameOriginRequest(ctx *gin.Context) bool {
if origin := ctx.GetHeader("Origin"); origin != "" {
return commonhttpmw.AllowedOrigin(origin, ctx.Request.Host)
}
// Fetch Metadata values are lowercase per spec; match leniently, which
// cannot admit a value outside the accepted set. An absent header falls to
// the default and is refused along with cross-site and same-site.
switch strings.ToLower(strings.TrimSpace(ctx.GetHeader("Sec-Fetch-Site"))) {
case "same-origin", "none":
return true
default:
return false
}
}
// webhookStatusForResponse validates a plugin-supplied WebhookResponse.Status
// against the HTTP status code range net/http's WriteHeader accepts
// ([100, 599] — RFC 9110 informational/success/redirection/client
apps/backend/internal/plugins/handlers_webhook_auth_test.go
+133 -13
View File
@@ -134,12 +134,23 @@ func TestWebhookAuthGate_AuthenticatedNonPublicReachesRelay(t *testing.T) {
}
}
// browserSignals is the pair of headers the session CSRF gate reads. A zero
// value models a request carrying neither: a non-browser client replaying a
// session cookie, or a browser predating Fetch Metadata.
type browserSignals struct {
origin string
secFetchSite string
}
func doWebhookIdentityRequest(
router http.Handler, method, path string, identity authn.Identity, origin string,
router http.Handler, method, path string, identity authn.Identity, signals browserSignals,
) *httptest.ResponseRecorder {
req := httptest.NewRequest(method, path, strings.NewReader("{}"))
if origin != "" {
req.Header.Set("Origin", origin)
if signals.origin != "" {
req.Header.Set("Origin", signals.origin)
}
if signals.secFetchSite != "" {
req.Header.Set("Sec-Fetch-Site", signals.secFetchSite)
}
req = req.WithContext(authn.WithIdentity(req.Context(), identity))
rec := httptest.NewRecorder()
@@ -147,23 +158,132 @@ func doWebhookIdentityRequest(
return rec
}
func TestWebhookAuthGate_SessionRequiresAcceptedOrigin(t *testing.T) {
// TestWebhookAuthGate_SessionCSRFSignals is the table over
// (method, Origin, Sec-Fetch-Site, identity) for the session CSRF gate.
//
// The gate exists because a session identity is carried by an ambient cookie,
// so a cross-site page can make the browser attach it. It must therefore
// admit a genuine same-origin request and refuse a cross-site one. The
// discriminating signal is *origin*, never the verb: a plugin's GET webhook
// may have side effects and the host cannot verify that it does not, so
// exempting safe methods would reopen the hole this gate closes.
//
// Sec-Fetch-Site is the signal for the case Origin cannot answer. Browsers do
// not send Origin on a same-origin GET/HEAD (Fetch, "append a request Origin
// header": Origin is attached to cross-origin requests and to same-origin
// requests whose method is neither GET nor HEAD), so an absent Origin is the
// *normal* state of the SPA polling its own plugin webhook, not evidence of a
// cross-origin caller.
func TestWebhookAuthGate_SessionCSRFSignals(t *testing.T) {
router, svc := newTestRouter(t)
installedWebhookPlugin(t, svc, "kandev-plugin-session-origin", "key1", false)
identity := authn.Identity{UserID: "user-1", Role: authn.RoleMember, SessionID: "session-1"}
session := authn.Identity{UserID: "user-1", Role: authn.RoleMember, SessionID: "session-1"}
pat := authn.Identity{UserID: "user-1", Role: authn.RoleMember, TokenID: "token-1"}
path := "/api/plugins/kandev-plugin-session-origin/webhooks/key1"
const (
blocked = http.StatusForbidden
relayed = http.StatusServiceUnavailable // reached the relay; see reachedRelay
)
for _, tc := range []struct {
name string
origin string
want int
name string
method string
identity authn.Identity
signals browserSignals
want int
}{
{name: "missing", want: http.StatusForbidden},
{name: "foreign", origin: "https://attacker.example", want: http.StatusForbidden},
{name: "same host", origin: "https://example.com", want: http.StatusServiceUnavailable},
// The case broken in production: a plugin panel polling its own
// webhook same-origin with GET. No Origin is sent, and before this
// gate learned to read Sec-Fetch-Site every such poll was refused 403
// on any auth-enabled instance.
{
name: "same-origin GET without Origin", method: http.MethodGet, identity: session,
signals: browserSignals{secFetchSite: "same-origin"}, want: relayed,
},
{
name: "same-origin POST without Origin", method: http.MethodPost, identity: session,
signals: browserSignals{secFetchSite: "same-origin"}, want: relayed,
},
// Header values are lowercase per the Fetch Metadata spec; matching
// case-insensitively costs nothing and cannot admit a new value.
{
name: "same-origin spelled with mixed case", method: http.MethodGet, identity: session,
signals: browserSignals{secFetchSite: "Same-Origin"}, want: relayed,
},
// Sec-Fetch-Site: none is a user-initiated request with no initiator
// (address bar, bookmark). A cross-site page cannot cause a request
// to be labelled none, so it is not a CSRF vector.
{
name: "user-initiated navigation", method: http.MethodGet, identity: session,
signals: browserSignals{secFetchSite: "none"}, want: relayed,
},
// A cross-site top-level GET navigation sends no Origin, and the
// SameSite=Lax session cookie *is* attached to it — this is the one
// ambient-credential vector left on this route, and the gate refuses
// it on the fetch-metadata signal alone.
{
name: "cross-site without Origin", method: http.MethodGet, identity: session,
signals: browserSignals{secFetchSite: "cross-site"}, want: blocked,
},
{
name: "cross-site POST without Origin", method: http.MethodPost, identity: session,
signals: browserSignals{secFetchSite: "cross-site"}, want: blocked,
},
// same-site (a sibling subdomain, or the same host on another port)
// is refused too: SameSite=Lax does not stop it, and no legitimate
// caller reaches this route as same-site without also sending Origin
// (see the split-origin case below).
{
name: "same-site without Origin", method: http.MethodGet, identity: session,
signals: browserSignals{secFetchSite: "same-site"}, want: blocked,
},
// Origin present keeps deciding on its own: a cross-origin fetch
// always carries Origin, and httpmw.AllowedOrigin is the single
// shared origin trust policy.
{
name: "foreign Origin", method: http.MethodPost, identity: session,
signals: browserSignals{origin: "https://attacker.example"}, want: blocked,
},
{
name: "foreign Origin claiming same-origin", method: http.MethodGet, identity: session,
signals: browserSignals{origin: "https://attacker.example", secFetchSite: "same-origin"},
want: blocked,
},
{
name: "accepted Origin", method: http.MethodPost, identity: session,
signals: browserSignals{origin: "https://example.com"}, want: relayed,
},
// The split-origin deployment host-api.ts documents: the SPA on one
// port fetching the backend on another. That is cross-origin, so the
// browser sends Origin (which AllowedOrigin accepts — it ignores
// ports) and labels it same-site. Deciding on Origin first is what
// keeps this working; demanding same-origin whenever the header is
// present would break every split-origin and desktop install.
{
name: "accepted Origin on another port", method: http.MethodGet, identity: session,
signals: browserSignals{origin: "https://example.com:1420", secFetchSite: "same-site"},
want: relayed,
},
// Neither signal: refused. An ambient session cookie with no origin
// evidence is indistinguishable from a cross-site top-level GET
// navigation by a browser predating Fetch Metadata, which
// SameSite=Lax does attach the cookie to. A deliberate non-browser
// caller uses a PAT, which is not gated at all (next case).
{
name: "no browser signals at all", method: http.MethodGet, identity: session,
signals: browserSignals{}, want: blocked,
},
// The gate is scoped to ambient-credential identities. A PAT is sent
// deliberately by its holder, so a cross-site page cannot borrow it
// and the fetch-metadata signal is irrelevant.
{
name: "PAT identity is not gated", method: http.MethodGet, identity: pat,
signals: browserSignals{secFetchSite: "cross-site"}, want: relayed,
},
} {
t.Run(tc.name, func(t *testing.T) {
rec := doWebhookIdentityRequest(router, http.MethodGet, path, identity, tc.origin)
rec := doWebhookIdentityRequest(router, tc.method, path, tc.identity, tc.signals)
if rec.Code != tc.want {
t.Fatalf("status = %d, want %d, body=%s", rec.Code, tc.want, rec.Body.String())
}
@@ -180,7 +300,7 @@ func TestWebhookAuthGate_NonBrowserIdentitiesDoNotRequireOrigin(t *testing.T) {
{UserID: "default", Role: authn.RoleAdmin, Synthetic: true},
}
for _, identity := range identities {
rec := doWebhookIdentityRequest(router, http.MethodPost, path, identity, "")
rec := doWebhookIdentityRequest(router, http.MethodPost, path, identity, browserSignals{})
if !reachedRelay(rec.Code) {
t.Fatalf("identity %+v: status = %d, want 503, body=%s", identity, rec.Code, rec.Body.String())
}
docs/public/plugins-authoring.md
+30 -3
View File
@@ -174,6 +174,10 @@ curated React, UI, and app-store surface.
for browser UI and billable operations. Kandev does not enforce
`webhooks[].method`; public integrations must still validate the provider
signature and replay/timestamp rules before side effects.
- An authenticated webhook is reachable from your panel with any method,
`GET` included: `host.api.fetch` is same-origin (or carries an accepted
`Origin` in a split-origin install), which is what the host checks. You do
not need to force a read onto `POST`.
- capabilities.auth is the highest-risk capability. A webhook response may
assert a verified external identity with X-Kandev-Auth-Login; only assert an
email the IdP verified as owned by the subject. See [ADR 0050](../decisions/0050-plugin-external-auth-capability.md).
@@ -1984,6 +1988,25 @@ For plugin-UI operations, declare `access: authenticated`; those routes may
raise `max_body_bytes` to 16 MiB. Kandev verifies the current user but strips
its session cookie and PAT before relaying the remaining headers.
On an instance with authentication enabled, a caller identified by the session
cookie must additionally look same-origin, because that cookie is ambient and a
page on another site could otherwise make the browser send it. The host accepts
the request in exactly two cases:
- it carries an accepted `Origin`; or
- it carries no `Origin` and `Sec-Fetch-Site` is `same-origin` or `none`.
Everything else is rejected, including `Sec-Fetch-Site` of `cross-site` or
`same-site` and a request carrying neither header. Browsers omit `Origin` on
same-origin `GET` and `HEAD` requests, so the second rule is what lets a panel
poll its own webhook with `GET`. In practice `host.api.fetch` from an active
`ui.bundle` satisfies it with no work on your part, whatever method you use.
Callers identified by a PAT are not subject to this check at all, because a PAT
is not ambient. That is the supported way to call an authenticated webhook from
outside a browser (a script, a CLI, your own backend); a replayed session cookie
carries no origin signal and is refused.
```yaml
webhooks:
- key: "provider-events"
@@ -2192,9 +2215,13 @@ repackaging.
- **Trusting webhook metadata:** `webhooks[].method` is informational. Public
routes are not authenticated by Kandev, so validate method, signature, timestamp, replay
protection, and body before side effects.
- **Authenticated webhook access:** a valid Kandev session or PAT is all the
host checks. Any signed-in user reaches an authenticated webhook, so enforce
your own per-user or per-role rules when the endpoint needs them.
- **Authenticated webhook access:** a valid Kandev session or PAT, plus a
same-origin request for the session case, is all the host checks. Any
signed-in user reaches an authenticated webhook, so enforce your own
per-user or per-role rules when the endpoint needs them. It is not an
authorization boundary, and it is not a CSRF boundary for your own state
either: the host proves the request came from its own origin, not that the
user meant to trigger whatever your handler does.
- **Bundling React:** use host.React, host.jsx, and host.ui; a second React or
Radix copy breaks shared contexts and portals.
- **Shipping the wrong binary name:** every declared executable must be under
docs/specs/plugins/requirements/voice-extraction-host.md
+5 -1
View File
@@ -156,7 +156,11 @@ rpc HandleWebhook(WebhookRequest) returns (WebhookResponse)
```
`access` is enforced by the host before `HandleWebhook` runs. For `authenticated`, the normal Kandev
identity must be present and browser cookie authentication additionally requires an accepted Origin;
identity must be present, and a session-cookie identity additionally requires a same-origin request.
Exactly two signals accept it: an accepted Origin, or (when the browser sends no Origin, as it does
not on same-origin GET/HEAD) a `Sec-Fetch-Site` of `same-origin` or `none`. Everything else is
rejected, including `cross-site`, `same-site`, and a request carrying neither header. PAT and
synthetic identities are not ambient and are not subject to that check;
authentication-disabled installations use the synthetic default user. Public webhooks retain their
current external-call behavior. Request cancellation propagates through the existing RPC context.