diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 32e64e0..07a37c4 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -314,7 +314,7 @@ func (b *Broker) ServeHTTP(w http.ResponseWriter, r *http.Request) { // indistinguishable, otherwise the broker is an oracle for enumerating // other tenants' resource ids. if app.Tenancy != nil { - if _, ok := app.Tenancy.EnforceRequest(b.ownerStore(), appID, app.allowSegs, r.Method, mpath, r.URL.RawQuery, r.Header.Get("Content-Type"), body, string(caller)); !ok { + if _, ok := app.Tenancy.EnforceRequest(b.ownerStore(), appID, app.tenancyPatterns(r.Method, mpath), r.Method, mpath, r.URL.RawQuery, r.Header.Get("Content-Type"), body, string(caller)); !ok { writeJSON(w, http.StatusNotFound, map[string]string{"error": "not found"}) return } @@ -463,7 +463,7 @@ func (b *Broker) ServeHTTP(w http.ResponseWriter, r *http.Request) { if app.Tenancy != nil && resp.StatusCode/100 == 2 { os := b.ownerStore() app.Tenancy.ClaimFrom(os, appID, r.Method, mpath, rb, string(caller), b.now()) - app.Tenancy.ReleaseFrom(os, appID, app.allowSegs, r.Method, mpath) + app.Tenancy.ReleaseFrom(os, appID, app.tenancyPatterns(r.Method, mpath), r.Method, mpath) if filtered, did := app.Tenancy.FilterResponse(os, appID, r.Method, mpath, rb, string(caller), b.now()); did { rb = filtered } else if filtered, did := app.Tenancy.FilterObject(os, appID, r.Method, mpath, rb, string(caller)); did { diff --git a/internal/broker/registry.go b/internal/broker/registry.go index 462a376..b155531 100644 --- a/internal/broker/registry.go +++ b/internal/broker/registry.go @@ -306,6 +306,19 @@ func (a *AppEntry) allowed(method, path string) bool { return false } +// tenancyPatterns returns the templated allow paths a request's path params are +// bound against for tenancy. A request that hits a LITERAL allow entry is a +// static route with no resource id in its path — e.g. GET /v1/agents/voices +// declared beside GET /v1/agents/{agent_id}. Binding it against the sibling +// template would read "voices" as an agent id, find no owner, and refuse a +// route the registry explicitly allows. Query and body refs are still checked. +func (a *AppEntry) tenancyPatterns(method, path string) [][]string { + if a.allowSet[costKey(strings.ToUpper(method), path)] || a.allowSet[costKey("", path)] { + return nil + } + return a.allowSegs +} + // allowPattern is a templated allow entry; method "" matches any verb. type allowPattern struct { method string diff --git a/internal/broker/zz_literal_route_test.go b/internal/broker/zz_literal_route_test.go new file mode 100644 index 0000000..f72e6be --- /dev/null +++ b/internal/broker/zz_literal_route_test.go @@ -0,0 +1,49 @@ +package broker + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// A literal allow entry next to a templated sibling (GET /v1/agents/voices beside +// GET /v1/agents/{agent_id}) is a static route. Tenancy must not bind the literal +// segment as a resource id: doing so refused io.pilot.agentphone's list_voices +// with a 404 for every caller. +func TestTenancy_LiteralRouteBesideTemplateIsNotAnID(t *testing.T) { + var hits []string + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits = append(hits, r.Method+" "+r.URL.Path) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"data":[{"id":"voice_1"}]}`) + })) + defer up.Close() + regJSON := strings.Replace(fmt.Sprintf(tenancyRegistryJSON, up.URL), + `"GET /v1/agents/{agent_id}",`, `"GET /v1/agents/{agent_id}", "GET /v1/agents/voices",`, 1) + reg, err := ParseRegistry([]byte(regJSON), func(string) string { return "master-key" }) + if err != nil { + t.Fatalf("ParseRegistry: %v", err) + } + b := New(reg, NewMemStore()) + b.Verify = VerifyConfig{Window: time.Hour} + _, mallory := newKey(t) + + if rec := do(t, b, mallory, "GET", "/io.pilot.phone/v1/agents/voices", nil); rec.Code != 200 { + t.Fatalf("literal route: %d %s, want 200", rec.Code, rec.Body) + } + if len(hits) != 1 || hits[0] != "GET /v1/agents/voices" { + t.Fatalf("literal route should reach the partner once, got %v", hits) + } + + // The template itself is still ownership-checked: an id Mallory does not own + // is refused and never forwarded. + if rec := do(t, b, mallory, "GET", "/io.pilot.phone/v1/agents/agent_alice", nil); rec.Code != http.StatusNotFound { + t.Errorf("unowned agent read: %d, want 404", rec.Code) + } + if len(hits) != 1 { + t.Errorf("unowned agent read reached the partner: %v", hits) + } +}