fix!: the authority is asked about the operation dispatched, never the decoded path (#75 review)

CHANGELOG.md

@@ -1,5 +1,38 @@
 # Changelog
 
+## Unreleased — review fixes for #75: the authority is asked about the operation dispatched (breaking)
+
+From the independent review of #75 (r1):
+
+- **The route checked is the operation dispatched (F1, security; F2).** The
+  authority was asked about `r.Method + " " + r.URL.Path`, the decoded path,
+  while the mux dispatches on the escaped one: `GET /v1/page/pages/secret%2Fhistory`
+  ran the `/pages/{id}` operation and was checked against the
+  `/pages/{id}/history` row, so a caller holding only the weaker history row
+  ran the page handler. The same root made a subtree `/x/` beside `/x/{id}`
+  (legal for the mux) a path the authority refuses as matching two routes.
+  `wrapRequest` now asks about the dispatched operation's own row,
+  `"<VERB> " + servedRoute(path)`, which the authority finds by exact name.
+- **`InterfaceInfo.BasePath` is deleted (F4).** The CLI prefixes every route
+  row with the document's base path, while the edge forwards a solution's
+  requests on `/v1/<id>` and the runtime checks routes as served, so no
+  non-empty base path ever matched a request. A backend still setting it no
+  longer compiles, naming the field: drop it and carry `/v1/<id>` in each
+  operation's path, as the migrated solutions do.
+- **A malformed request is refused with its reason (F5).** An
+  `InvalidArgument` or `OutOfRange` from the authority's `Check` is answered
+  400 (`invalid_argument` on a relayed call) with the authority's reason,
+  `ErrRequestInvalid`, never 503 "try again"; an issue refused that way is a
+  `WorkContextRefusal` of 400 with its reason.
+- **A stream's first message is bounded again (F6).** A relayed
+  server-streaming call cuts off a module that accepts the stream and sends
+  nothing at the method's `Timeout`, as `StreamFirstAnswerWithin` did before
+  the authority move, instead of holding it for the whole `MaxStreamDuration`.
+- **Open (F3):** `go.mod` pins the authority client and hop libraries at
+  commits only on the unmerged `authority/vision` branch. This closes when
+  authority#1 merges with a merge commit and this runtime re-pins; no runtime
+  release before then.
+
 ## Unreleased — the operation checked is the operation run; every served route publishes its policy (breaking)
 
 From the independent review of 2026-10-25 (F1–F4, F10, F11):

README.md

@@ -119,18 +119,23 @@ nothing:
   calling a route as a tool sends the child the authority issued it through
   its binding: `WorkContextPrincipals` names the person as owner and the module
   as current actor, and `ActorChain()` the whole chain.
-- **Which routes it reaches**: the `Check` names the route, `"<VERB> <path>"`
-  as this module serves it, and the authority applies this module's delivered
-  route policy (the `"<VERB> <template>"` rows the CLI derives from the
-  published contract's `x-codefly-authority-policy`). A route the call does
+- **Which routes it reaches**: the `Check` names the operation the request was
+  dispatched to, `"<VERB> <template>"` exactly as its row is published, and
+  the authority applies this module's delivered route policy (the
+  `"<VERB> <template>"` rows the CLI derives from the published contract's
+  `x-codefly-authority-policy`). The request's own path is never what is
+  asked about: an encoded `/` in a segment (`/pages/a%2Fhistory`) runs the
+  `/pages/{id}` operation while its decoded path reads as `/pages/{id}/history`,
+  and a subtree and a `{id}` route the mux tells apart would both match it. A route the call does
   not hold, one that declares nothing, and one not served are answered 403
   with the authority's reason. Under `hop/ENFORCEMENT=warn` the check is
   advisory: the route goes through and the refusal is logged. An authority
   that cannot be reached refused nobody: the request is answered 503, naming
   that, never 401.
 - **The operation checked is the operation run.** A request is served by the
-  operation declared for its method on its path, so the `"<VERB> <path>"` the
-  authority is asked about is the operation whose handler runs. A method the
+  operation declared for its method on its path, and the `"<VERB> <template>"`
+  the authority is asked about is that operation's, so it is the operation
+  whose handler runs. A method the
   path does not declare is answered 405, naming the methods it serves (and
   `Allow`), before the authority is asked or any handler runs. The boot
   refuses an operation with no policy, an undocumented one, one method
@@ -642,9 +647,10 @@ hold: `solution.Requires(kind, actions...)` or `solution.AnyCheckedCaller()`,
 the same shape as the authority's `authority.hop.v1.Policy`. The artifact
 writes it as the operation's `x-codefly-authority-policy`, the extension the
 CLI derives the solution's route rows from, and an operation that declares
-none, or both, is refused at generation by name. A backend renders its artifact with no `BasePath`: its operations already
-carry the `/v1/<id>` the edge forwards them on, so each route is written as
-served and matches the `"<VERB> <path>"` the runtime's `Check` names. A path
+none, or both, is refused at generation by name. The artifact has no base path
+(`InterfaceInfo` has no `BasePath`): the edge forwards a solution's requests on
+`/v1/<id>`, which its operations already carry, so each route is written as
+served and is the `"<VERB> <template>"` the runtime's `Check` names. A path
 ending in `/` serves its subtree and is written `/…/{path=**}`. The passthrough
 routes `InterfaceArtifact` publishes beside the solution's own are any checked
 caller's: see [why](#letting-the-page-call-a-consumed-module-the-passthrough).
@@ -655,6 +661,14 @@ caller's: see [why](#letting-the-page-call-a-consumed-module-the-passthrough).
 > decoding and frontend validation; sdk-go owns resolution and endpoint
 > admission; the authority client owns the authority's wire. Local source
 > composition does not require publishing a release.
+>
+> **Open: the authority pins.** `codefly-dev/authority/libraries/authority-client/go`
+> and `.../hop/go` are pinned at commits that are, today, only on the unmerged
+> `authority/vision` branch of `codefly-dev/authority`. The module is private
+> and fetched direct, so a squash merge and a branch deletion would orphan both
+> commits. This closes when authority#1 merges **with a merge commit** (which
+> keeps both commits reachable from `main`) and this runtime re-pins to that
+> merge or a tag; no runtime release is cut before then.
 ### Shared frontend validation and headless components
 
 `Manifest.Frontend == nil` declares a headless component: it boots normally and

authority.go

@@ -59,6 +59,12 @@ var ErrNoViewer = errors.New("the authority does not check the Work Context this
 // the authority's reason.
 var ErrRequestRefused = errors.New("the authority refuses this request")
 
+// ErrRequestInvalid is a request the authority refuses as malformed: the Work
+// Context or the route it was asked about is not one it can read. The refusal
+// is permanent, so it is answered 400 with the authority's reason, never as
+// an outage a retry would get past.
+var ErrRequestInvalid = errors.New("the authority refuses this request as malformed")
+
 // ErrAuthorityUnavailable is a request the authority could not be asked about:
 // it did not answer, or failed. Nobody refused it, so it is not answered as a
 // sign-in (401) but 503, by this name; the transport's own text, which can
@@ -113,17 +119,21 @@ func dialAuthority(endpoint string, transport workcontext.TransportSource, token
 
 // viewer asks the authority's Check whose the Work Context a request carries
 // is, addressed to this module, and, when route is named, whether it holds
-// what this module's delivery declares for that route: "<VERB> <path>", the
-// request as this module serves it, which the authority matches against the
-// "<VERB> <template>" rows the CLI derives from the published contract. A
+// what this module's delivery declares for that route: "<VERB> <template>",
+// the operation the request was dispatched to, spelled exactly as one of the
+// rows the CLI derives from the published contract, so the authority finds
+// that row by name rather than by matching a path against every template. A
 // context the authority does not check is no viewer, whatever it claims.
 func (h *hostAuthority) viewer(ctx context.Context, workContext, route string) (*authorityv1.WorkContextV1, error) {
 	ctx, cancel := context.WithTimeout(ctx, authorityCallTimeout)
 	defer cancel()
 	checked, err := h.authority.Check(ctx, &authorityv1.CheckRequest{Token: workContext, Audience: h.principal, Method: route, Advisory: h.advisory && route != ""})
 	if err != nil && refusalStatus(err) < http.StatusInternalServerError {
-		if status.Code(err) == codes.Unauthenticated {
+		switch refusalStatus(err) {
+		case http.StatusUnauthorized:
 			return nil, fmt.Errorf("%w: %s", ErrNoViewer, status.Convert(err).Message())
+		case http.StatusBadRequest:
+			return nil, fmt.Errorf("%w: %s", ErrRequestInvalid, status.Convert(err).Message())
 		}
 		return nil, fmt.Errorf("%w: %s", ErrRequestRefused, status.Convert(err).Message())
 	}
@@ -218,6 +228,8 @@ func refusalStatus(err error) int {
 	switch status.Code(err) {
 	case codes.Unauthenticated:
 		return http.StatusUnauthorized
+	case codes.InvalidArgument, codes.OutOfRange:
+		return http.StatusBadRequest
 	case codes.PermissionDenied, codes.NotFound, codes.FailedPrecondition:
 		return http.StatusForbidden
 	case codes.Unavailable, codes.DeadlineExceeded:

authority_test.go

@@ -2,6 +2,7 @@ package solution
 
 import (
 	"context"
+	"errors"
 	"log"
 	"net"
 	"net/http"
@@ -40,6 +41,10 @@ type interfaceAuthority struct {
 // edgeContext is the person's Work Context the edge sends the page.
 const edgeContext = "edge-context-for-the-page"
 
+// malformedContext is a Work Context the authority refuses as malformed
+// (InvalidArgument).
+const malformedContext = "malformed-context"
+
 // robinChild is a module's tool call to the page: Maya's Work Context, issued
 // by the authority as a child through robin's declared binding, robin acting,
 // addressed to the page.
@@ -50,6 +55,9 @@ func (a *interfaceAuthority) Check(_ context.Context, r *authorityv1.CheckReques
 	a.checked = append(a.checked, proto.Clone(r).(*authorityv1.CheckRequest))
 	held := a.held
 	a.mu.Unlock()
+	if r.GetToken() == malformedContext {
+		return nil, status.Error(codes.InvalidArgument, "the Work Context is not a token the authority can read")
+	}
 	var wc *authorityv1.WorkContextV1
 	switch {
 	case r.GetAudience() != "page":
@@ -62,7 +70,14 @@ func (a *interfaceAuthority) Check(_ context.Context, r *authorityv1.CheckReques
 	if wc == nil {
 		return nil, status.Error(codes.Unauthenticated, "not a context for this audience")
 	}
-	if r.GetMethod() != "" && held != nil && !slices.ContainsFunc(held, func(row string) bool { return routeRowMatches(row, r.GetMethod()) }) {
+	if r.GetMethod() != "" && held != nil {
+		matched, ambiguous := heldRoute(held, r.GetMethod())
+		if ambiguous != "" {
+			return nil, status.Error(codes.PermissionDenied, ambiguous)
+		}
+		if matched {
+			return &authorityv1.CheckResponse{Context: wc}, nil
+		}
 		refusal := "page requires pages:write for " + r.GetMethod() + ", which the call does not hold (it holds pages:read)"
 		if !r.GetAdvisory() {
 			return nil, status.Error(codes.PermissionDenied, refusal)
@@ -72,15 +87,46 @@ func (a *interfaceAuthority) Check(_ context.Context, r *authorityv1.CheckReques
 	return &authorityv1.CheckResponse{Context: wc}, nil
 }
 
-// routeRowMatches is the authority's reading of a request "<VERB> <path>"
-// against a declared row "<VERB> <template>", "{name}" standing for one
-// segment: enough of it to prove the runtime spells the request the way the
-// rows are matched.
+// heldRoute is the authority's reading (state.admitMethod) of a checked
+// method against the held rows: a row named exactly is that row; otherwise a
+// request "<VERB> <path>" is matched against every "<VERB> <template>",
+// "{name}" standing for one segment and a trailing "{name=**}" for the rest
+// of the path, and two templates matching one path is refused as ambiguous,
+// never guessed between. matched reports a held row; ambiguous, when set, is
+// the authority's refusal.
+func heldRoute(held []string, method string) (matched bool, ambiguous string) {
+	if slices.Contains(held, method) {
+		return true, ""
+	}
+	var found []string
+	for _, row := range held {
+		if routeRowMatches(row, method) {
+			found = append(found, row)
+		}
+	}
+	if len(found) > 1 {
+		return false, method + " matches more than one declared route (" + strings.Join(found, ", ") + ")"
+	}
+	return len(found) == 1, ""
+}
+
+// routeRowMatches is the authority's routeOf for one row: a request "<VERB>
+// <path>" against a declared row "<VERB> <template>", "{name}" standing for
+// one non-empty segment and a last "{name=**}" for the rest of the path.
 func routeRowMatches(row, request string) bool {
 	verb, template, _ := strings.Cut(row, " ")
 	got, path, _ := strings.Cut(request, " ")
+	if verb != got {
+		return false
+	}
 	want, have := strings.Split(template, "/"), strings.Split(path, "/")
-	if verb != got || len(want) != len(have) {
+	if last := want[len(want)-1]; strings.HasPrefix(last, "{") && strings.HasSuffix(last, "=**}") {
+		if len(have) < len(want) || strings.Join(have[len(want)-1:], "/") == "" {
+			return false
+		}
+		want, have = want[:len(want)-1], have[:len(want)-1]
+	}
+	if len(want) != len(have) {
 		return false
 	}
 	for i, segment := range want {
@@ -157,7 +203,7 @@ func authorityServer(t *testing.T) (*Server, *interfaceAuthority) {
 func TestUnderTheAuthorityTheViewerIsTheEdgesWorkContext(t *testing.T) {
 	s, _ := authorityServer(t)
 	var seen *Gateway
-	handler := s.wrapRequest(func(_ *http.Request, gw *Gateway) (any, error) {
+	handler := s.wrapRequest("GET /v1/page/types", func(_ *http.Request, gw *Gateway) (any, error) {
 		seen = gw
 		return map[string]string{"ok": "yes"}, nil
 	})
@@ -279,7 +325,7 @@ func TestUnderTheAuthorityAModulesToolCallActsForThePerson(t *testing.T) {
 	var principals WorkContextPrincipals
 	var actors []string
 	ran := 0
-	handler := s.wrapRequest(func(r *http.Request, gw *Gateway) (any, error) {
+	handler := s.wrapRequest("GET /v1/page/pages/{page_id}", func(r *http.Request, gw *Gateway) (any, error) {
 		ran++
 		var err error
 		principals, err = gw.WorkContextPrincipals(r.Context())
@@ -324,36 +370,39 @@ func TestUnderTheAuthorityAModulesToolCallActsForThePerson(t *testing.T) {
 	}
 }
 
-// Row 7: the Check names the route a request calls, "<VERB> <path>" as the
-// page serves it, the spelling the CLI's route-policy rows ("<VERB>
-// <template>") are matched against, so the authority applies the page's
-// delivered per-route policy. A route the call does not hold is refused with
-// the authority's own reason, before the handler runs.
+// Row 7: the Check names the operation a request was dispatched to, "<VERB>
+// <template>" exactly as the CLI's route-policy row for it is spelled, so the
+// authority applies the page's delivered per-route policy to the operation
+// that runs, whatever the request's own path reads as. A route the call does
+// not hold is refused with the authority's own reason, before the handler
+// runs.
 func TestUnderTheAuthorityTheCheckNamesTheRoute(t *testing.T) {
 	s, stand := authorityServer(t)
 	stand.held = []string{"GET /v1/page/pages/{page_id}"}
 	ran := 0
-	handler := s.wrapRequest(func(*http.Request, *Gateway) (any, error) {
+	handler := func(*http.Request, *Gateway) (any, error) {
 		ran++
 		return map[string]string{"ok": "yes"}, nil
-	})
+	}
+	read := s.wrapRequest("GET /v1/page/pages/{page_id}", handler)
+	write := s.wrapRequest("POST /v1/page/pages", handler)
 
 	request := httptest.NewRequest(http.MethodGet, "/v1/page/pages/p-1?revision=3", nil)
 	request.Header.Set(WorkContextHeader, robinChild)
 	answer := httptest.NewRecorder()
-	handler(answer, request)
+	read(answer, request)
 	if answer.Code != http.StatusOK || ran != 1 {
 		t.Fatalf("a held route answered %d (%s)", answer.Code, answer.Body)
 	}
 	checked := stand.checks()
-	if len(checked) != 1 || checked[0].GetMethod() != "GET /v1/page/pages/p-1" || checked[0].GetAudience() != "page" || checked[0].GetAdvisory() {
+	if len(checked) != 1 || checked[0].GetMethod() != "GET /v1/page/pages/{page_id}" || checked[0].GetAudience() != "page" || checked[0].GetAdvisory() {
 		t.Fatalf("the check asked %v", checked)
 	}
 
 	request = httptest.NewRequest(http.MethodPost, "/v1/page/pages", strings.NewReader(`{}`))
 	request.Header.Set(WorkContextHeader, robinChild)
 	answer = httptest.NewRecorder()
-	handler(answer, request)
+	write(answer, request)
 	if answer.Code != http.StatusForbidden || ran != 1 || !strings.Contains(answer.Body.String(), "page requires pages:write for POST /v1/page/pages") {
 		t.Fatalf("a route the call does not hold answered %d (%s), handler ran %d times", answer.Code, answer.Body, ran)
 	}
@@ -370,7 +419,7 @@ func TestUnderAdvisoryEnforcementARefusedRouteIsLoggedAndLetThrough(t *testing.T
 	logged := &strings.Builder{}
 	log.SetOutput(logged)
 	defer log.SetOutput(os.Stderr)
-	handler := s.wrapRequest(func(*http.Request, *Gateway) (any, error) {
+	handler := s.wrapRequest("POST /v1/page/pages", func(*http.Request, *Gateway) (any, error) {
 		return map[string]string{"ok": "yes"}, nil
 	})
 
@@ -412,3 +461,27 @@ func TestHopEnforcementWarnMakesTheRouteCheckAdvisory(t *testing.T) {
 	}
 	t.Cleanup(func() { _ = codefly.LoadEnvironmentVariables() })
 }
+
+// r1 F5: an authority status outside the sign-in and permission classes was
+// answered 503 "try again" with its reason only in the log. A malformed
+// request (InvalidArgument) is a permanent refusal: it is answered 400 with
+// the authority's reason, to a REST caller and a relayed Connect call alike,
+// and an outage is still 503.
+func TestAMalformedRequestIsRefusedWithTheAuthoritysReason(t *testing.T) {
+	s, _ := authorityServer(t)
+	ran := false
+	handler := s.wrapRequest("GET /v1/page/types", func(*http.Request, *Gateway) (any, error) { ran = true; return nil, nil })
+	request := httptest.NewRequest(http.MethodGet, "/v1/page/types", nil)
+	request.Header.Set(WorkContextHeader, malformedContext)
+	answer := httptest.NewRecorder()
+	handler(answer, request)
+	if answer.Code != http.StatusBadRequest || ran || !strings.Contains(answer.Body.String(), "not a token the authority can read") {
+		t.Fatalf("a malformed request answered %d %s (handler ran: %v)", answer.Code, answer.Body, ran)
+	}
+	if _, err := s.viewerGateway(context.Background(), malformedContext, "GET /v1/page/types"); !errors.Is(err, ErrRequestInvalid) || errors.Is(err, ErrAuthorityUnavailable) {
+		t.Fatalf("viewerGateway = %v, want ErrRequestInvalid", err)
+	}
+	if got := refusalStatus(status.Error(codes.Unavailable, "down")); got != http.StatusServiceUnavailable {
+		t.Fatalf("an outage maps to %d, want 503", got)
+	}
+}