~bigbes/sr-ht-spec

ref: 01999c70f928a0963f60dc1670f68e7201576b1b sr-ht-spec/graph/webhook_authz_test.go -rw-r--r-- 8.0 KiB
01999c70 — Eugene Blikh 2 days ago
graph: keep webhook management with the owner, not with any agent

Moving /query to the anonymous router admitted tokens.sr.ht working tokens, and
webhookAuthorized asked only whether the caller was AUTH_INTERNAL. coreauth maps
the owner and its agents alike to INTERNAL — deliberately, because INTERNAL is
what core-go's NewAuthConfig and FilterWebhooks demand of anyone at all — so a
token carrying nothing but spec:read could create and delete subscriptions and
read every subscription's URL, stored query and delivery bodies. A read grant
was buying a mutation, which is the one thing a grant vocabulary exists to
prevent. Measured before the fix: a spec:read token created two subscriptions
against a live daemon.

The guard now asks spec's own principal for IsOwner, which is the only value
left that still tells the owner from an agent. That restores exactly what the
daemon's ownerOnly wrapper enforced before the conversion, and it needs no new
grant string: naming a third grant beside spec:read and spec:propose is a
vocabulary decision for tokens.sr.ht to mint, not something to invent at a call
site.

One guard covers the whole webhook chapter. createUserWebhook,
deleteUserWebhook, userWebhooks and userWebhook all call it, and the two field
resolvers that carry webhook data are reachable only through an object one of
those four returned. Query.webhook has no ACL and needs none — outside a
delivery there is no payload, so it answers an error; asserted rather than
assumed.

The consequence, and it is not small: webhook management is now unreachable over
/query, because this endpoint accepts no credential that resolves to the owner —
authn produces KindOwner from the unified-login cookie alone and this endpoint
reads no cookie. The check is written against the right predicate anyway, so a
plane that does yield the owner works the moment it exists. Delivery is
untouched, and was verified end to end against a subscription inserted directly.

Also stop returning coreerrors.ErrAccessDenied itself. It is a package-level
*gqlerror.Error and gqlgen assigns the field path onto the error it is handed,
so the shared object carried the previous refusal's path: against a live daemon
a refused deleteUserWebhook, userWebhooks and userWebhook all reported
"path":["createUserWebhook"]. A wrong answer, a cross-request leak of which
field somebody else asked for, and a data race on a global. The bug predates
this endpoint but was latent while ownerOnly refused non-owners before any
resolver ran; refusing agents here makes it the common path.