From a0fa81b359bd57c82b22ff0148cde77f24ba70e5 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Fri, 24 Jul 2026 19:59:33 +0300 Subject: [PATCH] refactor(authn): one Principal.CanRead() for the read-plane ACL (spec-ejq.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit graph's gate, web's mayRead and mcpsrv's Gate each hand-spelled 'IsOwner() || IsAgent()' — three copies of the read ACL, which graph's own comment warned is how a corpus leaks when they drift. Define it once as authn.Principal.CanRead and route all three through it. coreauth.Derive keeps its own owner||agent test on purpose: it answers a different question (is this an owner-backed identity to bridge to AUTH_INTERNAL), and coupling it to the read ACL would misroute a future read-only viewer kind to the owner's UserID. Closes spec-ejq.1 --- authn/principal.go | 8 ++++++++ authn/principal_test.go | 27 +++++++++++++++++++++++++++ graph/server.go | 11 ++++------- mcpsrv/mcpsrv.go | 11 +++++------ web/handlers.go | 11 ++++------- 5 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 authn/principal_test.go diff --git a/authn/principal.go b/authn/principal.go index 5c6d954643b989c7471a7cf2ea7967b6619f1e2c..78282955a185569b074ad11309acbc4652b80e75 100644 --- a/authn/principal.go +++ b/authn/principal.go @@ -83,6 +83,14 @@ func (p Principal) IsOwner() bool { return p.Kind == KindOwner } // proposals/*. func (p Principal) IsAgent() bool { return p.Kind == KindAgent } +// CanRead reports whether this principal may read content: the owner and its +// agents may, nobody else may. This is the whole read-plane ACL — one human, no +// visibility levels, and a non-owner human already resolved to anonymous by +// authn — and it lives here, in one place, because every read surface (graph's +// /query, the web UI, the MCP tools) must apply the identical policy: two read +// surfaces with two spellings of it is how a corpus leaks. +func (p Principal) CanRead() bool { return p.IsOwner() || p.IsAgent() } + // String renders the principal for logs. It never includes the token name's // secret (there is none — the name is not the token) and never includes the // cookie value. diff --git a/authn/principal_test.go b/authn/principal_test.go new file mode 100644 index 0000000000000000000000000000000000000000..3fa5d17368f9118336dc3f1b53c9a32a347e78d6 --- /dev/null +++ b/authn/principal_test.go @@ -0,0 +1,27 @@ +package authn + +import "testing" + +// TestCanRead pins the read-plane ACL every surface shares: the owner and its +// agents may read, and every other principal — including an unrecognised or +// zero Kind — is denied rather than accidentally admitted. +func TestCanRead(t *testing.T) { + cases := []struct { + name string + p Principal + want bool + }{ + {"owner may read", Principal{Kind: KindOwner}, true}, + {"agent may read", Principal{Kind: KindAgent}, true}, + {"anonymous may not", Anonymous(), false}, + {"zero value may not", Principal{}, false}, + {"unknown kind may not", Principal{Kind: "wat"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.p.CanRead(); got != tc.want { + t.Errorf("CanRead() = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/graph/server.go b/graph/server.go index 2d0c3d7e4f95846e231551f2559ff323083e38ad..5df3e8ee2566f6b4b37c200ee7ba7b7fef59298c 100644 --- a/graph/server.go +++ b/graph/server.go @@ -146,19 +146,16 @@ func (s *Server) Endpoint() http.Handler { // gate refuses a caller with no read authority before the query is parsed. // -// One human, no visibility levels: the owner and its agents may read and nobody -// else may. A logged-in human who is not the instance owner has already been -// resolved to anonymous by authn, so this is the whole ACL — the same one web/ -// applies, spelled the same way, because two read surfaces with two policies is -// how a corpus leaks. +// The ACL is authn.Principal.CanRead — the owner and its agents may read and +// nobody else may — the same predicate web/ and mcpsrv/ apply, so the three read +// surfaces cannot drift into three policies, which is how a corpus leaks. // // The refusal is a 401 with a line of text and never a redirect to meta's // login: every caller here is a machine, and handing a bot 200 and a page of // login markup tells it nothing it can act on. func gate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - p := authn.PrincipalFromContext(r.Context()) - if !p.IsOwner() && !p.IsAgent() { + if !authn.PrincipalFromContext(r.Context()).CanRead() { w.Header().Set("Content-Type", "text/plain; charset=utf-8") http.Error(w, "authentication required", http.StatusUnauthorized) return diff --git a/mcpsrv/mcpsrv.go b/mcpsrv/mcpsrv.go index 3c7757e000145cfa3ae6a58dbf7ff18e4fcfc7f1..6b9f942385bcd937d357cab1a26777aa6d03268a 100644 --- a/mcpsrv/mcpsrv.go +++ b/mcpsrv/mcpsrv.go @@ -239,10 +239,10 @@ func allowHosts(next http.Handler, origin string) http.Handler { } // Gate refuses a caller with no read authority before any MCP method — not just -// tools/call, but initialize and tools/list too — reaches the server. It is the -// read plane's ACL for this surface: the owner and its agents may read and -// nobody else may, the same policy graph's /query and the web UI apply, spelled -// the same way. Two read surfaces with two policies is how a corpus leaks. +// tools/call, but initialize and tools/list too — reaches the server. The ACL +// is authn.Principal.CanRead — the owner and its agents may read and nobody else +// may — the same predicate graph's /query and the web UI apply, so the three +// read surfaces cannot drift into three policies, which is how a corpus leaks. // // It reads the principal the resolver middleware set, so it must be mounted // INSIDE that middleware: @@ -255,8 +255,7 @@ func allowHosts(next http.Handler, origin string) http.Handler { // line of plain text and never a login redirect: every caller here is a machine. func Gate(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - p := authn.PrincipalFromContext(r.Context()) - if !p.IsOwner() && !p.IsAgent() { + if !authn.PrincipalFromContext(r.Context()).CanRead() { w.Header().Set("Content-Type", "text/plain; charset=utf-8") http.Error(w, "authentication required", http.StatusUnauthorized) return diff --git a/web/handlers.go b/web/handlers.go index 2243bb32e32da2f1a4a79c6c9b109690deb95e1c..27174f28fa82945834a575030500c104d27a83a5 100644 --- a/web/handlers.go +++ b/web/handlers.go @@ -51,14 +51,11 @@ func splitFormat(rest string) (string, format) { // ---- authorization -------------------------------------------------------- -// mayRead reports whether a request carries authority to read content. -// -// One human, no visibility levels: the owner and its agents may read and nobody -// else may. A logged-in human who is not the instance owner has already been -// resolved to anonymous by authn, so this is the whole ACL. +// mayRead reports whether a request carries authority to read content. The ACL +// itself — owner and its agents, nobody else — is authn.Principal.CanRead, the +// one spelling every read surface shares. func mayRead(r *http.Request) bool { - p := authn.PrincipalFromContext(r.Context()) - return p.IsOwner() || p.IsAgent() + return authn.PrincipalFromContext(r.Context()).CanRead() } // denyRead answers a viewer with no read authority in the shape their client