Skip to content

Commit ad32eaf

Browse files
author
gluck
committed
middleware: one generic Chain, and interception stops being three shapes
Review asked what a more middlewarish pattern would look like, and whether it could be generic. Both, and the answer changed more than the guard. THE NAME. Go splits the word by transport: net/http says middleware, gRPC and connectrpc say interceptor. A gRPC interceptor is NOT a decorator -- its signature is (ctx, req, info, handler) and the next handler arrives as an argument -- so borrowing that word for a func(H) H would import the name without the convention. go-kit generalised middleware off HTTP to exactly this shape (type Middleware func(Endpoint) Endpoint), so: Middleware. THE GENERIC. Not over the payload. In/Out would have exactly one instantiation, because the wire is always (json.RawMessage -> any), and a generic with one instantiation is inference noise that buys nothing. What wants a type parameter is COMPOSITION: type Middleware[H any] func(next H) H func Chain[H any](ms ...Middleware[H]) Middleware[H] Ten lines, no dependencies, no domain knowledge. One Chain for every decorator in the tree -- MethodHandler on the hub, http.Handler on the gateway, whatever comes next. A test instantiates it at http.Handler purely to keep that claim honest. WHAT IT UNIFIED. rpc.MethodHandler carries the method as an ARGUMENT rather than as N closures that each captured their own name; without that the unit of interception is a map, which is why the guard added last commit had to be a map transformer. With it, the two cross-cutting concerns on the aria endpoint are both middleware: hb.dispatch = middleware.Chain( authz.Middleware(authn, policy), dressing(h.dressParams), )(hb.route) authz.Guard survives as an ADAPTER onto authz.Middleware for the map-shaped angelus door, so there is one policy decision rather than two that can disagree. Dressing was four lines hand-inlined at the top of route(); route() is now only what its name says. ORDER IS TESTED, NOT COMMENTED. Chain(a, b, c) means a is outermost. Reversed, authorization would run after dressing -- and dressing resolves outfit names by READING FILES FROM DISK, so an unauthorized caller would reach the filesystem before anything refused them. Two tests: one on the combinator, one on the real hub chain, because they regress independently (reorder two lines in hubFor and every combinator test still passes). No nilable middleware anywhere. Chain() of nothing is the identity and authz.AllowAll() must be written, so "this door is open" is always a sentence someone typed. That is the same rule the last commit's fail-closed work rests on. Full suite green.
1 parent 3019796 commit ad32eaf

12 files changed

Lines changed: 457 additions & 88 deletions

File tree

‎api/rpc/handler.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package rpc
2+
3+
// THE SERVING SHAPE: one handler that is TOLD which method.
4+
//
5+
// The alternative -- and what figaro had -- is a map of per-method closures,
6+
// each having captured its own name. That shape cannot be decorated: to wrap
7+
// it you must transform the whole map, which makes the unit of interception a
8+
// map when the concern is a call. Every cross-cutting concern in the daemon
9+
// (authorization, dressing, and the gateway's envelope rewriting) then grows
10+
// its own spelling, and one of them ends up hand-inlined at the top of a
11+
// dispatch function.
12+
//
13+
// With the method in the signature, all three are middleware.Middleware
14+
// values over this one type, composed by one Chain, in an order that is
15+
// readable at the call site.
16+
17+
import (
18+
"context"
19+
"encoding/json"
20+
)
21+
22+
// MethodHandler serves one RPC call. It lives here, beside the method names
23+
// it dispatches on, because both the authorization seam and the daemon need
24+
// it and neither should have to import the other to say what a handler is.
25+
type MethodHandler func(ctx context.Context, method string, params json.RawMessage) (any, error)

‎internal/angelus/chain_order_test.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package angelus
2+
3+
// THE ORDERING INVARIANT, pinned on the real hub chain.
4+
//
5+
// middleware.Chain has its own order test, but that one proves the combinator
6+
// composes left-to-right. This one proves the thing that MATTERS: on an aria
7+
// endpoint, authorization runs before dressing, so a caller the policy refuses
8+
// never reaches the step that reads outfit files off disk.
9+
//
10+
// It is a separate test from the Chain test because it can regress
11+
// independently -- someone reorders two lines in hubFor and every Chain test
12+
// still passes.
13+
14+
import (
15+
"context"
16+
"encoding/json"
17+
"testing"
18+
19+
"github.com/jack-work/figaro/api/rpc"
20+
"github.com/jack-work/figaro/internal/authz"
21+
"github.com/jack-work/figaro/internal/middleware"
22+
)
23+
24+
func TestAuthzRunsBeforeDressing(t *testing.T) {
25+
dressed := false
26+
routed := false
27+
28+
dispatch := middleware.Chain(
29+
authz.Middleware(
30+
authz.AriaHeader{Enabled: false},
31+
authz.PolicyFunc(func(authz.Request) authz.Decision {
32+
return authz.Deny("no")
33+
}),
34+
),
35+
dressing(func(_ string, p json.RawMessage) (json.RawMessage, error) {
36+
dressed = true
37+
return p, nil
38+
}),
39+
)(func(context.Context, string, json.RawMessage) (any, error) {
40+
routed = true
41+
return nil, nil
42+
})
43+
44+
_, err := dispatch(context.Background(), rpc.MethodQua, json.RawMessage(`{}`))
45+
if err == nil {
46+
t.Fatal("a denied request was not refused")
47+
}
48+
if dressed {
49+
t.Fatal("DRESSING RAN FOR A REFUSED CALLER. " +
50+
"Dressing resolves outfit names by reading files from disk, so it must sit " +
51+
"INSIDE the authorization middleware, never outside or beside it. " +
52+
"Check the Chain() argument order in hubFor.")
53+
}
54+
if routed {
55+
t.Fatal("a denied request reached route()")
56+
}
57+
}
58+
59+
// And the allowed path must still dress: an ordering fix that refused
60+
// everything would pass the test above and break the daemon.
61+
func TestAllowedRequestIsStillDressed(t *testing.T) {
62+
var sawParams string
63+
dispatch := middleware.Chain(
64+
authz.Middleware(authz.AriaHeader{Enabled: false}, authz.AllowAll()),
65+
dressing(func(_ string, _ json.RawMessage) (json.RawMessage, error) {
66+
return json.RawMessage(`{"dressed":true}`), nil
67+
}),
68+
)(func(_ context.Context, _ string, p json.RawMessage) (any, error) {
69+
sawParams = string(p)
70+
return nil, nil
71+
})
72+
73+
if _, err := dispatch(context.Background(), rpc.MethodQua, json.RawMessage(`{}`)); err != nil {
74+
t.Fatalf("allowed request refused: %v", err)
75+
}
76+
if sawParams != `{"dressed":true}` {
77+
t.Fatalf("route saw %q: dressing did not reach it", sawParams)
78+
}
79+
}
80+
81+
// A dressing failure must surface as the caller's error, not be swallowed.
82+
// This is the `fig form outfit test` case: a spec naming nothing on disk has
83+
// to fail at the boundary rather than land as a literal on a board.
84+
func TestDressingFailureStopsTheChain(t *testing.T) {
85+
routed := false
86+
dispatch := middleware.Chain(
87+
authz.Middleware(authz.AriaHeader{Enabled: false}, authz.AllowAll()),
88+
dressing(func(_ string, _ json.RawMessage) (json.RawMessage, error) {
89+
return nil, errNoSuchOutfit
90+
}),
91+
)(func(context.Context, string, json.RawMessage) (any, error) {
92+
routed = true
93+
return nil, nil
94+
})
95+
96+
_, err := dispatch(context.Background(), rpc.MethodQua, json.RawMessage(`{}`))
97+
if err != errNoSuchOutfit {
98+
t.Fatalf("dressing error not surfaced: %v", err)
99+
}
100+
if routed {
101+
t.Fatal("a request with an unresolvable outfit still reached route()")
102+
}
103+
}
104+
105+
var errNoSuchOutfit = &outfitTestError{}
106+
107+
type outfitTestError struct{}
108+
109+
func (*outfitTestError) Error() string { return "no such outfit" }

‎internal/angelus/dressing_mw.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package angelus
2+
3+
// DRESSING AS MIDDLEWARE.
4+
//
5+
// Resolving a request's outfit NAMES into keys is the API boundary's one
6+
// materialization point: whatever the request reaches next -- a live agent's
7+
// inbox, the store's agentless writer -- receives pure data, and no writer
8+
// below this line reads a file.
9+
//
10+
// It used to be four lines at the top of ariaHub.route, which made it
11+
// invisible as a concern and impossible to order relative to anything else.
12+
// As middleware, "authorization runs before dressing" is a fact about the
13+
// Chain() argument list rather than a fact about where a call happens to sit
14+
// inside a function -- and that ordering matters, because dressing is the
15+
// step that touches the filesystem.
16+
17+
import (
18+
"context"
19+
"encoding/json"
20+
21+
"github.com/jack-work/figaro/api/rpc"
22+
"github.com/jack-work/figaro/internal/middleware"
23+
)
24+
25+
// DressFunc resolves outfit names in a request's params. A request naming no
26+
// outfit is returned byte for byte.
27+
type DressFunc func(method string, params json.RawMessage) (json.RawMessage, error)
28+
29+
// dressing rewrites params before the request is routed. A spec that names
30+
// nothing on disk fails HERE, at the boundary, rather than landing as a
31+
// literal on a board -- which is exactly what `fig form outfit test` did
32+
// while the hub's writer applied patches verbatim.
33+
func dressing(dress DressFunc) middleware.Middleware[rpc.MethodHandler] {
34+
return func(next rpc.MethodHandler) rpc.MethodHandler {
35+
if dress == nil {
36+
return next
37+
}
38+
return func(ctx context.Context, method string, params json.RawMessage) (any, error) {
39+
dressed, err := dress(method, params)
40+
if err != nil {
41+
return nil, err
42+
}
43+
return next(ctx, method, dressed)
44+
}
45+
}
46+
}

‎internal/angelus/hub.go‎

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,6 @@ type ariaHub struct {
3636
// writer per node whether an agent is live or not (the agent itself
3737
// writes through the backend), so this is the same writer, earlier.
3838
write func(id, method string, params json.RawMessage) (v any, ok bool, err error)
39-
// dress resolves a request's outfit NAMES into keys before it is routed
40-
// anywhere. It is the API boundary's one materialization point: whatever
41-
// the request reaches next, a live agent's inbox, the store's agentless
42-
// writer: receives pure data, and no writer below this line reads a
43-
// file. A request naming no outfit is returned byte for byte.
44-
dress func(method string, params json.RawMessage) (json.RawMessage, error)
4539
// kind is the node's species from its figwal marker ("conversation",
4640
// "form", …). A form has no agent to wake, ever: methods that reach
4741
// the wake path on a form node get a named refusal instead of a
@@ -52,17 +46,17 @@ type ariaHub struct {
5246
ln net.Listener
5347
conns map[*jkrpc.Server]struct{}
5448
agent figaro.AgentServer
55-
// guard wraps the served handler map with authentication and policy. It
56-
// is a field rather than a call in handlers() because the hub is built in
57-
// internal/angelus and the policy is assembled beside the angelus map:
58-
// one guard, both doors.
49+
// dispatch is the hub's decorated handler: authorization, then dressing,
50+
// then route. It is set by hubFor and is NEVER nil in production --
51+
// handlers() falls back to bare route only so a test can build a hub
52+
// without a policy, and hub_guard_test pins that distinction.
5953
//
60-
// A NIL GUARD SERVES UNGUARDED, and that is what this endpoint did for
61-
// its whole life before 2026-08-25: figaro.qua, figaro.set, study/cast/
62-
// drop, interrupt and the queue verbs were reachable on an aria socket
63-
// with no policy consulted, while authz.Guard wrapped only the angelus.
64-
// The guarded set must equal the served set, so hubs.go always sets this.
65-
guard func(map[string]jkrpc.HandlerFunc) map[string]jkrpc.HandlerFunc
54+
// Before this was a chain, the two concerns it composes were expressed
55+
// two different ways: dressing was hand-inlined at the top of route(),
56+
// and authorization did not exist here at all, so figaro.qua, figaro.set,
57+
// study/cast/drop, interrupt and the queue verbs were served with no
58+
// policy consulted while authz.Guard wrapped only the angelus door.
59+
dispatch rpc.MethodHandler
6660
// Nothing is buffered on purpose: a frame produced while no client is
6761
// attached is owed to nobody, and a client that attaches catches up with
6862
// a read rather than a replay.
@@ -238,34 +232,32 @@ type subscribableAgent interface {
238232
var errDormantMethod = errors.New("aria is dormant and this method needs a live agent")
239233

240234
func (hb *ariaHub) handlers() map[string]jkrpc.HandlerFunc {
235+
// One decorated handler, adapted to the map jkrpc wants. The method name
236+
// travels as an ARGUMENT rather than as N captured closures, which is
237+
// what makes the chain above possible at all.
238+
dispatch := hb.dispatch
239+
if dispatch == nil {
240+
dispatch = hb.route
241+
}
241242
methods := figaro.AgentMethods()
242243
out := make(map[string]jkrpc.HandlerFunc, len(methods))
243244
for _, m := range methods {
244245
method := m
245246
out[method] = func(ctx context.Context, params json.RawMessage) (any, error) {
246-
return hb.route(ctx, method, params)
247+
return dispatch(ctx, method, params)
247248
}
248249
}
249-
if hb.guard != nil {
250-
return hb.guard(out)
251-
}
252250
return out
253251
}
254252

255253
// route sends a request to the agent, waking the aria when the method needs
256254
// a turn loop and answering from the store when it does not.
255+
//
256+
// It is the INNERMOST handler of the chain hubFor builds: everything
257+
// cross-cutting -- authorization, and the outfit dressing that used to be
258+
// hand-inlined right here -- is middleware above it now. What is left is
259+
// routing, which is the only thing this function was ever named for.
257260
func (hb *ariaHub) route(ctx context.Context, method string, params json.RawMessage) (any, error) {
258-
// Dress FIRST, before any of the three destinations. A spec that names
259-
// nothing on disk fails here, at the boundary, rather than landing as a
260-
// literal on a board (which is exactly what `fig form outfit test` did
261-
// while the hub's writer applied patches verbatim).
262-
if hb.dress != nil {
263-
dressed, err := hb.dress(method, params)
264-
if err != nil {
265-
return nil, err
266-
}
267-
params = dressed
268-
}
269261
if agent := hb.boundAgent(); agent != nil {
270262
return agent.Handle(ctx, method, params)
271263
}

‎internal/angelus/hub_guard_test.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/jack-work/figaro/api/rpc"
2121
"github.com/jack-work/figaro/internal/authz"
2222
"github.com/jack-work/figaro/internal/figaro"
23+
"github.com/jack-work/figaro/internal/middleware"
2324
"github.com/jack-work/jkrpc"
2425
)
2526

@@ -29,13 +30,13 @@ import (
2930
func TestHubHandlersAreGuarded(t *testing.T) {
3031
var seen []string
3132
hb := newAriaHub("test", "")
32-
hb.guard = func(m map[string]jkrpc.HandlerFunc) map[string]jkrpc.HandlerFunc {
33-
return authz.Guard(m, authz.AriaHeader{Enabled: false},
33+
hb.dispatch = middleware.Chain(
34+
authz.Middleware(authz.AriaHeader{Enabled: false},
3435
authz.PolicyFunc(func(r authz.Request) authz.Decision {
3536
seen = append(seen, r.Method)
3637
return authz.Deny("policy consulted")
37-
}))
38-
}
38+
})),
39+
)(hb.route)
3940

4041
served := hb.handlers()
4142
methods := figaro.AgentMethods()
@@ -95,13 +96,13 @@ func TestAgencyMethodsAreOnTheGuardedDoor(t *testing.T) {
9596
}
9697
}
9798

98-
// A nil guard serves unguarded. That is the pre-fix behaviour, kept only so
99-
// the hub can be built in tests without a policy -- and pinned here so nobody
100-
// mistakes it for the production path. hubFor always sets a guard.
101-
func TestNilGuardServesUnguarded(t *testing.T) {
99+
// A nil dispatch falls back to bare route. That is kept ONLY so a test can
100+
// build a hub without a policy, and it is pinned here so nobody mistakes it
101+
// for the production path: hubFor always composes a chain.
102+
func TestNilDispatchFallsBackToBareRoute(t *testing.T) {
102103
hb := newAriaHub("test", "")
103-
if hb.guard != nil {
104-
t.Fatal("a freshly built hub should carry no guard until hubFor sets one")
104+
if hb.dispatch != nil {
105+
t.Fatal("a freshly built hub should carry no dispatch until hubFor sets one")
105106
}
106107
if len(hb.handlers()) != len(figaro.AgentMethods()) {
107108
t.Fatal("an unguarded hub should still serve the full method set")

‎internal/angelus/hubs.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import (
99
"time"
1010

1111
"github.com/jack-work/figaro/api/rpc"
12+
"github.com/jack-work/figaro/internal/authz"
1213
"github.com/jack-work/figaro/internal/figaro"
14+
"github.com/jack-work/figaro/internal/middleware"
1315
"github.com/jack-work/figaro/internal/store"
1416
)
1517

@@ -71,12 +73,21 @@ func (h *handlers) hubFor(id string) (*ariaHub, error) {
7173
hb.wake = h.wakeForHub
7274
hb.read = h.readFromStore
7375
hb.write = h.writeForHub
74-
hb.dress = h.dressParams
76+
// THE ORDER IS THE POINT. Authorization is outermost, so an unauthorized
77+
// caller is refused before dressing resolves an outfit name by reading
78+
// files from disk. Written as a chain, that is a property of this
79+
// argument list; written as calls inside route(), it was a property of
80+
// where somebody happened to put a line.
81+
//
7582
// The aria socket serves the AGENCY methods -- qua, set, study, cast,
76-
// interrupt, the queue verbs -- and until this line existed it served
77-
// them with no policy at all, while the angelus door beside it was
78-
// guarded. One guard, both doors: the guarded set is now the served set.
79-
hb.guard = h.guardHandlers
83+
// interrupt, the queue verbs -- and until this existed it served them
84+
// with no policy at all, while the angelus door beside it was guarded.
85+
// One decision, both doors: authz.Middleware is what authz.Guard is
86+
// built from.
87+
hb.dispatch = middleware.Chain(
88+
authz.Middleware(h.authenticator(), h.policy()),
89+
dressing(h.dressParams),
90+
)(hb.route)
8091
if h.angelus.Backend != nil {
8192
if n, ok := h.angelus.Backend.Node(id); ok {
8293
hb.kind = n.Kind

‎internal/angelus/protocol.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,6 @@ func (h *handlers) authenticator() authz.Authenticator {
134134
return authz.AriaHeader{Enabled: h.config != nil && h.config.CallerIdentityEnabled()}
135135
}
136136

137-
// guardHandlers wraps a handler map with THIS daemon's authn and policy. It is
138-
// handed to every aria hub so the agency methods are gated by the same seam
139-
// that gates the angelus door: see ariaHub.guard for what it was like before.
140-
func (h *handlers) guardHandlers(m map[string]jkrpc.HandlerFunc) map[string]jkrpc.HandlerFunc {
141-
return authz.Guard(m, h.authenticator(), h.policy())
142-
}
143-
144137
// policy builds the configured authorization policy. The turn-active predicate
145138
// is read off the live registry: a dormant or unknown aria has no turn in
146139
// flight, so it cannot be mid-turn, and a fork of it is free to proceed.

0 commit comments

Comments
 (0)