From 4309ee12e218193ffa6f9ccddc2e20b2837479f5 Mon Sep 17 00:00:00 2001 From: Vasil Date: Sat, 24 Oct 2026 04:20:14 +0200 Subject: [PATCH 1/2] KTMS_READ_KEY: a read-only Keystone key for reads and viewers With the optional KTMS_READ_KEY (scope admin:read), Admin API reads and every call of a viewer carry it, so they cannot change Keystone even if a route's role check were wrong; changes keep KTMS_ADMIN_KEY. The sidebar shows whether each key is accepted for its job, probing writes with an empty MintAPIKey that fails validation after the scope check. The e2e covers both keys when Keystone knows admin:read. --- CHANGELOG.md | 6 ++ README.md | 4 +- env.example | 2 + internal/api/api.go | 92 ++++++++++++++++++-- internal/api/readkey_test.go | 148 +++++++++++++++++++++++++++++++++ internal/config/config.go | 4 + internal/web/static/js/main.js | 3 +- internal/web/static/style.css | 2 + scripts/e2e.sh | 22 ++++- 9 files changed, 272 insertions(+), 11 deletions(-) create mode 100644 internal/api/readkey_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b825ec1..0fdbf6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ Notable changes to ktms-admin. The format follows ## [Unreleased] +### Added +- `KTMS_READ_KEY`: an optional Keystone key with the `admin:read` scope (Keystone 1.2+). Admin API + reads and every call of a viewer go with it; changes keep `KTMS_ADMIN_KEY`. The sidebar's + health shows whether each key is accepted for its job: the admin key may change Keystone, the + read key may read and nothing more. + ## [1.0.0] - 2026-10-23 The first stable release, paired with Keystone. The Model Switchboard. (KTMS) 1.0: ktms-admin diff --git a/README.md b/README.md index ec633e7..ba2b476 100644 --- a/README.md +++ b/README.md @@ -261,6 +261,7 @@ All settings are environment variables; see [`env.example`](env.example). | `SESSION_TTL` | `12h` | Session lifetime. | | `KTMS_ADDR` | empty | `host:port` of the KTMS gateway's gRPC endpoint. (`LLM_PROXY_ADDR` still works, deprecated.) | | `KTMS_ADMIN_KEY` | – | Key with the `admin` scope. Required with `KTMS_ADDR`. (`LLM_PROXY_ADMIN_KEY` still works, deprecated.) | +| `KTMS_READ_KEY` | empty | Optional key with the `admin:read` scope (Keystone 1.2+). Admin API reads and every call of a viewer go with it, so they cannot change Keystone; changes keep `KTMS_ADMIN_KEY`. The sidebar shows whether each key is accepted for its job. | | `KTMS_TLS` | `false` | Use TLS to reach the gateway. (`LLM_PROXY_TLS` still works, deprecated.) | | `TOKENMETER_URL` | empty | Base URL of light-tokenmeter's HTTP port. | | `TOKENMETER_TOKEN` | empty | Bearer token for light-tokenmeter with the `read` scope, when its authentication is on. A missing or refused token marks the usage dot "refused" and names the setting. | @@ -335,7 +336,8 @@ shows its spend this month against the budget and its keys. ## Security model - The console holds the gateway's admin key **on the server**; it never reaches the browser, and vendor keys are - write-only. + write-only. With `KTMS_READ_KEY` (scope `admin:read`) every read and everything a viewer does goes with a key + that cannot change Keystone, so a viewer stays read-only even if a route's role check were wrong. - Session cookies are signed (HMAC-SHA256), `HttpOnly`, `SameSite=Lax` and `Secure` over HTTPS. They carry only name, e-mail, sign-in method and expiry; the role is recomputed from the allowlists on every request, and turning a sign-in method off ends its sessions. diff --git a/env.example b/env.example index ad8ccca..c8984ee 100644 --- a/env.example +++ b/env.example @@ -44,6 +44,8 @@ SESSION_TTL=12h # KTMS gateway (gRPC). The key needs the admin scope. KTMS_ADDR=ktms:9090 KTMS_ADMIN_KEY= +# Optional, Keystone 1.2+: a key with the admin:read scope for reads and viewers. +KTMS_READ_KEY= KTMS_TLS=false # light-tokenmeter (HTTP). With its auth on, a token with the read scope. TOKENMETER_URL=http://light-tokenmeter:8080 diff --git a/internal/api/api.go b/internal/api/api.go index 4276cfc..0bbc929 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -55,8 +55,8 @@ func New(cfg *config.Config, a *auth.Handler) (*Server, func(), error) { creds = nil } opts := []grpc.DialOption{ - grpc.WithUnaryInterceptor(attachKey(cfg.ProxyAdminKey)), - grpc.WithStreamInterceptor(attachStreamKey(cfg.ProxyAdminKey)), + grpc.WithUnaryInterceptor(attachKey(s.keyFor)), + grpc.WithStreamInterceptor(attachStreamKey(s.keyFor)), } if creds == nil { opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) @@ -81,20 +81,45 @@ func New(cfg *config.Config, a *auth.Handler) (*Server, func(), error) { return s, closer, nil } -func attachKey(key string) grpc.UnaryClientInterceptor { +func attachKey(key func(context.Context, string) string) grpc.UnaryClientInterceptor { return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, inv grpc.UnaryInvoker, opts ...grpc.CallOption) error { - ctx = metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+key) + ctx = metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+key(ctx, method)) return inv(ctx, method, req, reply, cc, opts...) } } -func attachStreamKey(key string) grpc.StreamClientInterceptor { +func attachStreamKey(key func(context.Context, string) string) grpc.StreamClientInterceptor { return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { - ctx = metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+key) + ctx = metadata.AppendToOutgoingContext(ctx, "authorization", "Bearer "+key(ctx, method)) return streamer(ctx, desc, cc, method, opts...) } } +type viewerCtx struct{} +type keyCtx struct{} + +// keyFor picks the Keystone key for a call. With KTMS_READ_KEY set, Admin API +// reads and anything a viewer does go with it, so a read or a viewer can +// change nothing even if a route's guard were wrong. +func (s *Server) keyFor(ctx context.Context, method string) string { + if k, ok := ctx.Value(keyCtx{}).(string); ok { + return k + } + if s.cfg.ProxyReadKey == "" { + return s.cfg.ProxyAdminKey + } + if ctx.Value(viewerCtx{}) != nil || readMethod(method) { + return s.cfg.ProxyReadKey + } + return s.cfg.ProxyAdminKey +} + +// readMethod is an Admin API call that admin:read allows. +func readMethod(method string) bool { + name, ok := strings.CutPrefix(method, "/ktms.v1.AdminService/") + return ok && (strings.HasPrefix(name, "Get") || strings.HasPrefix(name, "List") || name == "ExplainRoute") +} + func (s *Server) Handler() http.Handler { mux := http.NewServeMux() s.auth.Mount(mux) @@ -233,6 +258,9 @@ func (s *Server) guard(pol policy, h handler) http.HandlerFunc { return } } + if u.IsViewer() { + r = r.WithContext(context.WithValue(r.Context(), viewerCtx{}, true)) + } defer func() { if rec := recover(); rec != nil { log.Error().Interface("panic", rec).Str("path", r.URL.Path).Msg("handler panic") @@ -340,7 +368,51 @@ type backendStatus struct { OK bool `json:"ok"` Error string `json:"error,omitempty"` // Auth marks a refused key or token, so the console does not call a running backend down. - Auth bool `json:"auth,omitempty"` + Auth bool `json:"auth,omitempty"` + Keys []keyStatus `json:"keys,omitempty"` +} + +// keyStatus says whether Keystone takes one of the console's keys for what +// the console uses it for. +type keyStatus struct { + Name string `json:"name"` // KTMS_ADMIN_KEY or KTMS_READ_KEY + OK bool `json:"ok"` + Error string `json:"error,omitempty"` +} + +// checkKeys probes each key without changing anything: MintAPIKey with no +// name fails validation after the scope check, so its error tells whether the +// key may write. +func (s *Server) checkKeys(ctx context.Context) []keyStatus { + mayWrite := func(key string) (bool, error) { + _, err := s.admin.MintAPIKey(context.WithValue(ctx, keyCtx{}, key), &pb.MintAPIKeyRequest{}) + switch status.Code(err) { + case codes.InvalidArgument, codes.OK: + return true, nil + case codes.PermissionDenied: + return false, nil + } + return false, err + } + admin := keyStatus{Name: "KTMS_ADMIN_KEY", OK: true} + if w, err := mayWrite(s.cfg.ProxyAdminKey); err != nil { + admin.OK, admin.Error = false, status.Convert(err).Message() + } else if !w { + admin.OK, admin.Error = false, "the key cannot change Keystone: it needs the admin scope" + } + out := []keyStatus{admin} + if s.cfg.ProxyReadKey == "" { + return out + } + read := keyStatus{Name: "KTMS_READ_KEY", OK: true} + if _, err := s.admin.ListAPIKeys(context.WithValue(ctx, keyCtx{}, s.cfg.ProxyReadKey), &pb.ListAPIKeysRequest{}); err != nil { + read.OK, read.Error = false, status.Convert(err).Message() + } else if w, err := mayWrite(s.cfg.ProxyReadKey); err != nil { + read.OK, read.Error = false, status.Convert(err).Message() + } else if w { + read.OK, read.Error = false, "the key can change Keystone: mint one with only the admin:read scope" + } + return append(out, read) } func (s *Server) health(w http.ResponseWriter, r *http.Request, _ *auth.User) { @@ -354,6 +426,12 @@ func (s *Server) health(w http.ResponseWriter, r *http.Request, _ *auth.User) { c := status.Code(err) st.Auth = c == codes.Unauthenticated || c == codes.PermissionDenied } + if st.OK || st.Auth { + st.Keys = s.checkKeys(ctx) + for _, k := range st.Keys { + st.OK, st.Auth = st.OK && k.OK, st.Auth || !k.OK + } + } out["proxy"] = st } if s.usage != nil { diff --git a/internal/api/readkey_test.go b/internal/api/readkey_test.go new file mode 100644 index 0000000..37fbe6e --- /dev/null +++ b/internal/api/readkey_test.go @@ -0,0 +1,148 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + + pb "github.com/ai-process/ktms-admin/internal/gen/ktms/v1" +) + +// scopedAdmin answers as Keystone would for the key a call carries: the admin +// key may do anything, the read key only read. +type scopedAdmin struct { + *fakeAdmin + key func(context.Context, string) string + admin string + read string + used map[string]string // method → key it carried +} + +func (f *scopedAdmin) check(ctx context.Context, method string, write bool) error { + k := f.key(ctx, method) + f.used[method] = k + switch { + case k == f.admin: + return nil + case k == f.read && !write: + return nil + case k == f.read: + return status.Error(codes.PermissionDenied, "apikeys: insufficient scope: need admin") + } + return status.Error(codes.Unauthenticated, "invalid key") +} + +func (f *scopedAdmin) ListAPIKeys(ctx context.Context, r *pb.ListAPIKeysRequest, o ...grpc.CallOption) (*pb.ListAPIKeysResponse, error) { + if err := f.check(ctx, "/ktms.v1.AdminService/ListAPIKeys", false); err != nil { + return nil, err + } + return f.fakeAdmin.ListAPIKeys(ctx, r, o...) +} + +func (f *scopedAdmin) MintAPIKey(ctx context.Context, r *pb.MintAPIKeyRequest, o ...grpc.CallOption) (*pb.MintAPIKeyResponse, error) { + if err := f.check(ctx, "/ktms.v1.AdminService/MintAPIKey", true); err != nil { + return nil, err + } + if r.Name == "" { + return nil, status.Error(codes.InvalidArgument, "name is required") + } + return f.fakeAdmin.MintAPIKey(ctx, r, o...) +} + +func scoped(t *testing.T, readKey string) (*Server, *scopedAdmin) { + t.Helper() + s, fa, _ := newServer(t, "") + s.cfg.ProxyAdminKey, s.cfg.ProxyReadKey = "ktms_admin", readKey + sa := &scopedAdmin{fakeAdmin: fa, key: s.keyFor, admin: "ktms_admin", read: "ktms_read", used: map[string]string{}} + s.admin = sa + return s, sa +} + +func TestReadKeyCarriesReadsAndViewers(t *testing.T) { + s, sa := scoped(t, "ktms_read") + h := s.Handler() + viewer, boss := sessionCookie(t, "p@view.com"), sessionCookie(t, "boss@x.com") + + if w := call(h, "GET", "/api/proxy/keys", "", boss, nil); w.Code != 200 || sa.used["/ktms.v1.AdminService/ListAPIKeys"] != "ktms_read" { + t.Fatalf("an admin's read went with %q (%d)", sa.used["/ktms.v1.AdminService/ListAPIKeys"], w.Code) + } + if w := call(h, "POST", "/api/proxy/keys", `{"name":"svc","scopes":["generate"]}`, boss, csrf); w.Code != 200 || sa.used["/ktms.v1.AdminService/MintAPIKey"] != "ktms_admin" { + t.Fatalf("an admin's change went with %q (%d %s)", sa.used["/ktms.v1.AdminService/MintAPIKey"], w.Code, w.Body) + } + if w := call(h, "GET", "/api/proxy/keys", "", viewer, nil); w.Code != 200 || sa.used["/ktms.v1.AdminService/ListAPIKeys"] != "ktms_read" { + t.Fatalf("a viewer's read went with %q", sa.used["/ktms.v1.AdminService/ListAPIKeys"]) + } + + // Whatever a viewer's call is, it carries the read key. + ctx := context.WithValue(context.Background(), viewerCtx{}, true) + if k := s.keyFor(ctx, "/ktms.v1.AdminService/RevokeAPIKey"); k != "ktms_read" { + t.Fatalf("a viewer's change would go with %q", k) + } + for m, want := range map[string]string{ + "/ktms.v1.AdminService/GetConfig": "ktms_read", + "/ktms.v1.AdminService/ExplainRoute": "ktms_read", + "/ktms.v1.AdminService/ReplaceRules": "ktms_admin", + "/ktms.v1.GatewayService/GenerateText": "ktms_admin", + } { + if k := s.keyFor(context.Background(), m); k != want { + t.Errorf("%s goes with %q, want %q", m, k, want) + } + } + + // The interceptor puts the chosen key on the wire. + var got string + _ = attachKey(s.keyFor)(context.Background(), "/ktms.v1.AdminService/ListNodes", nil, nil, nil, + func(ctx context.Context, _ string, _, _ any, _ *grpc.ClientConn, _ ...grpc.CallOption) error { + md, _ := metadata.FromOutgoingContext(ctx) + got = md.Get("authorization")[0] + return nil + }) + if got != "Bearer ktms_read" { + t.Fatalf("authorization = %q", got) + } +} + +func TestWithoutReadKeyEverythingUsesTheAdminKey(t *testing.T) { + s, _ := scoped(t, "") + ctx := context.WithValue(context.Background(), viewerCtx{}, true) + for _, m := range []string{"/ktms.v1.AdminService/ListAPIKeys", "/ktms.v1.AdminService/MintAPIKey"} { + if k := s.keyFor(ctx, m); k != "ktms_admin" { + t.Errorf("%s goes with %q", m, k) + } + } +} + +func TestHealthChecksBothKeys(t *testing.T) { + health := func(s *Server) backendStatus { + t.Helper() + w := call(s.Handler(), "GET", "/api/health", "", sessionCookie(t, "boss@x.com"), nil) + var out map[string]backendStatus + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + return out["proxy"] + } + s, _ := scoped(t, "ktms_read") + if st := health(s); !st.OK || len(st.Keys) != 2 || !st.Keys[0].OK || !st.Keys[1].OK { + t.Fatalf("both keys right: %+v", st) + } + + // The read key in KTMS_ADMIN_KEY: reads work, the console cannot change anything. + s, sa := scoped(t, "") + sa.admin = "nobody" + s.cfg.ProxyAdminKey = "ktms_read" + if st := health(s); st.OK || !st.Auth || st.Keys[0].OK { + t.Fatalf("a read key as the admin key: %+v", st) + } + + // An admin key in KTMS_READ_KEY is reported: it is not read-only. + s, _ = scoped(t, "ktms_admin") + if st := health(s); st.OK || st.Keys[1].OK { + t.Fatalf("an admin key as the read key: %+v", st) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 6f74ac0..6a3ac81 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -47,6 +47,9 @@ type Config struct { ProxyAddr string ProxyAdminKey string + // ProxyReadKey, a key with the admin:read scope, carries every read and + // every viewer's call, so those cannot change Keystone. Optional. + ProxyReadKey string ProxyTLS bool TokenmeterURL string @@ -114,6 +117,7 @@ func Load() (*Config, error) { InsecureNoAuth: env("INSECURE_NO_AUTH", "") == "true", ProxyAddr: env("KTMS_ADDR", env("LLM_PROXY_ADDR", "")), ProxyAdminKey: env("KTMS_ADMIN_KEY", env("LLM_PROXY_ADMIN_KEY", "")), + ProxyReadKey: strings.TrimSpace(env("KTMS_READ_KEY", "")), ProxyTLS: env("KTMS_TLS", env("LLM_PROXY_TLS", "")) == "true", TokenmeterURL: strings.TrimRight(env("TOKENMETER_URL", ""), "/"), // Trimmed: a secret written with echo ends in a newline, which no header may carry. diff --git a/internal/web/static/js/main.js b/internal/web/static/js/main.js index dd82fe0..35eb787 100644 --- a/internal/web/static/js/main.js +++ b/internal/web/static/js/main.js @@ -67,8 +67,9 @@ function Health() { const [h, setH] = useState(session.health); useEffect(() => { api('GET', '/api/health').then((x) => { session.health = x; setH(x); }).catch(() => {}); }, []); const row = (label, st) => st?.enabled && html`
${label}${!st.ok && html`${st.auth ? 'refused' : 'down'}`}
`; + const keyRow = (k) => html`
${k.name}${!k.ok && html`refused`}
`; if (!h || (!h.proxy?.enabled && !h.usage?.enabled)) return null; - return html`
${row('KTMS gateway', h.proxy)}${row('Tokenmeter', h.usage)}
`; + return html`
${row('KTMS gateway', h.proxy)}${h.proxy?.keys?.map(keyRow)}${row('Tokenmeter', h.usage)}
`; } function RoleBadge({ role }) { diff --git a/internal/web/static/style.css b/internal/web/static/style.css index 7c511ef..976782c 100644 --- a/internal/web/static/style.css +++ b/internal/web/static/style.css @@ -90,6 +90,8 @@ code, .mono { font-family: var(--mono); font-size: 13px; } .side .grow { flex: 1; } .health { display: flex; flex-direction: column; gap: 8px; padding: 12px 10px; border: 1px solid var(--line); border-radius: 10px; font-size: 13px; } .health div { display: flex; align-items: center; gap: 8px; } +.health .key { padding-left: 14px; font-size: 12px; } +.health .key code { font-size: 11px; } .dot { width: 8px; height: 8px; border-radius: 50%; background: var(--muted); flex: none; } .dot.ok { background: var(--ok); } .dot.bad { background: var(--bad); } .who { display: flex; align-items: center; gap: 10px; padding: 4px 6px; font-size: 13px; } diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 63372a6..d823cba 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -97,12 +97,12 @@ docker run -d --name "$P-ktms" --network "$NET" \ -e BOOTSTRAP_ADMIN_KEY="$BOOTSTRAP" -e REDIS_ADDR="$P-redis:6379" -e ROUTING_RELOAD_SECONDS=1 \ -e USAGE_COLLECTOR_HOST="$P-tm:9090" -e USAGE_COLLECTOR_TOKEN="$W" "$P-ktms-img" >/dev/null -# run_console KEY TOKEN starts the console on localhost only, without sign-in. +# run_console KEY TOKEN [READ_KEY] starts the console on localhost only, without sign-in. run_console() { docker rm -f "$P-admin" >/dev/null 2>&1 || true docker run -d --name "$P-admin" --network "$NET" -p "127.0.0.1:$ADMIN_PORT:8080" \ -e PUBLIC_URL="$CONSOLE" -e INSECURE_NO_AUTH=true \ - -e KTMS_ADDR="$P-ktms:9090" -e KTMS_ADMIN_KEY="$1" \ + -e KTMS_ADDR="$P-ktms:9090" -e KTMS_ADMIN_KEY="$1" -e KTMS_READ_KEY="${3:-}" \ -e TOKENMETER_URL="http://$P-tm:8080" -e TOKENMETER_TOKEN="$2" "$P-admin-img" >/dev/null wait_for "$CONSOLE/healthz" } @@ -172,6 +172,24 @@ until api GET "/api/usage?from=$SINCE&to=$DAY&group=model" | jq -e '(.report.gro done echo "e2e: the call's usage went KTMS -> light-tokenmeter (write token) -> console (read token)" +# Keystone 1.2 knows admin:read; an older one refuses the scope, and this part +# is skipped. +if READ_KEY=$(api POST /api/proxy/keys '{"name":"console-read","scopes":["admin:read"]}' 2>/dev/null | jq -er .plain_key); then + run_console "$CONSOLE_KEY" "$R" "$READ_KEY" + api GET /api/health | jq -e '.proxy.ok and (.proxy.keys | length == 2) and all(.proxy.keys[]; .ok)' >/dev/null || + fail "health with both keys: $(api GET /api/health)" + api GET /api/proxy/config | jq -e '.models | map(.id) | index("mock-gpt")' >/dev/null || fail "a read with KTMS_READ_KEY" + api PUT /api/proxy/rules '{"rules":[{"name":"catch-all","priority":10,"use":["mock-gpt"],"enabled":true}]}' >/dev/null || + fail "a change with KTMS_ADMIN_KEY next to a read key" + # The read key as the admin key: the console sees it cannot change anything. + run_console "$READ_KEY" "$R" + api GET /api/health | jq -e '.proxy.ok == false and .proxy.auth == true' >/dev/null || + fail "a read key as KTMS_ADMIN_KEY was not reported: $(api GET /api/health)" + echo "e2e: reads go with KTMS_READ_KEY, changes with KTMS_ADMIN_KEY, and a wrong key is reported" +else + echo "e2e: ktms@$KTMS_REF has no admin:read scope; KTMS_READ_KEY not tested" +fi + # A token without the read scope must show as refused, not as down. run_console "$CONSOLE_KEY" "$W" api GET /api/health | jq -e '.usage.ok == false and .usage.auth == true' >/dev/null || fail "a write-only token was not reported as refused: $(api GET /api/health)" From b803ea44eae73364058bdd21e59ccae542869a3a Mon Sep 17 00:00:00 2001 From: Vasil Date: Sat, 24 Oct 2026 04:27:17 +0200 Subject: [PATCH 2/2] KTMS_READ_KEY: review fixes The read methods are listed, not guessed from names. Key probes run once a minute on their own context, in parallel with light-tokenmeter's check, so a slow gateway cannot mark it down; a probe that fails for other reasons is not a refused key. The write probe is doubly invalid. A refused read names KTMS_READ_KEY. The e2e proves reads carry the read key with a bogus one, and skips only when Keystone does not know the scope. --- internal/api/api.go | 162 ++++++++++++++++++++++----------- internal/api/readkey_test.go | 29 +++++- internal/config/config.go | 4 +- internal/web/static/js/main.js | 2 +- scripts/e2e.sh | 15 ++- 5 files changed, 150 insertions(+), 62 deletions(-) diff --git a/internal/api/api.go b/internal/api/api.go index 0bbc929..e7a085e 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -39,6 +39,10 @@ type Server struct { data pb.GatewayServiceClient usage *usage.Client + keyMu sync.Mutex + keyStatus []keyStatus + keyAt time.Time + priceMu sync.Mutex prices map[string]usage.Price pricesAt time.Time @@ -114,12 +118,19 @@ func (s *Server) keyFor(ctx context.Context, method string) string { return s.cfg.ProxyAdminKey } -// readMethod is an Admin API call that admin:read allows. -func readMethod(method string) bool { - name, ok := strings.CutPrefix(method, "/ktms.v1.AdminService/") - return ok && (strings.HasPrefix(name, "Get") || strings.HasPrefix(name, "List") || name == "ExplainRoute") +// readMethods are the Admin API calls Keystone lets admin:read make. Listed, +// not guessed from names, so a new RPC goes with the admin key until added. +var readMethods = map[string]bool{ + pb.AdminService_GetConfig_FullMethodName: true, pb.AdminService_ListAPIKeys_FullMethodName: true, + pb.AdminService_ListNodeKeys_FullMethodName: true, pb.AdminService_ListNodes_FullMethodName: true, + pb.AdminService_ListUserRateLimits_FullMethodName: true, pb.AdminService_ListSpendLimits_FullMethodName: true, + pb.AdminService_ListOrganizations_FullMethodName: true, pb.AdminService_GetTeam_FullMethodName: true, + pb.AdminService_ListTeams_FullMethodName: true, pb.AdminService_GetTeamSpend_FullMethodName: true, + pb.AdminService_GetSpend_FullMethodName: true, "/ktms.v1.AdminService/ExplainRoute": true, } +func readMethod(method string) bool { return readMethods[method] } + func (s *Server) Handler() http.Handler { mux := http.NewServeMux() s.auth.Mount(mux) @@ -341,7 +352,7 @@ func backendErr(w http.ResponseWriter, err error) { } msg := st.Message() if st.Code() == codes.Unauthenticated || st.Code() == codes.PermissionDenied { - msg = "the KTMS gateway rejected the console's admin key: " + msg + msg = "the KTMS gateway rejected the console's key (KTMS_ADMIN_KEY, or KTMS_READ_KEY for reads when it is set): " + msg } if st.Code() == codes.Unavailable { msg = "the KTMS gateway is unreachable: " + msg @@ -375,72 +386,117 @@ type backendStatus struct { // keyStatus says whether Keystone takes one of the console's keys for what // the console uses it for. type keyStatus struct { - Name string `json:"name"` // KTMS_ADMIN_KEY or KTMS_READ_KEY - OK bool `json:"ok"` - Error string `json:"error,omitempty"` + Name string `json:"name"` // KTMS_ADMIN_KEY or KTMS_READ_KEY + OK bool `json:"ok"` + Refused bool `json:"refused,omitempty"` // Keystone answered; the key is wrong for its job + Error string `json:"error,omitempty"` } +// keyCheckTTL spaces the key probes: each one is a refused call in +// Keystone's log, and a key's scopes rarely change. +const keyCheckTTL = time.Minute + // checkKeys probes each key without changing anything: MintAPIKey with no -// name fails validation after the scope check, so its error tells whether the -// key may write. -func (s *Server) checkKeys(ctx context.Context) []keyStatus { - mayWrite := func(key string) (bool, error) { - _, err := s.admin.MintAPIKey(context.WithValue(ctx, keyCtx{}, key), &pb.MintAPIKeyRequest{}) +// name and an unknown scope fails validation after the scope check, so its +// code tells whether the key may write. The probes run on their own context. +func (s *Server) checkKeys() []keyStatus { + s.keyMu.Lock() + defer s.keyMu.Unlock() + if s.keyStatus != nil && time.Since(s.keyAt) < keyCheckTTL { + return s.keyStatus + } + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + probe := func(name, key string, wantWrite bool) keyStatus { + st := keyStatus{Name: name, OK: true} + kctx := context.WithValue(ctx, keyCtx{}, key) + if _, err := s.admin.ListAPIKeys(kctx, &pb.ListAPIKeysRequest{}); err != nil { + return failed(st, err) + } + _, err := s.admin.MintAPIKey(kctx, &pb.MintAPIKeyRequest{Scopes: []string{"-"}}) switch status.Code(err) { - case codes.InvalidArgument, codes.OK: - return true, nil + case codes.InvalidArgument: + if !wantWrite { + st.OK, st.Refused, st.Error = false, true, "the key can change Keystone: mint one with only the admin:read scope" + } case codes.PermissionDenied: - return false, nil + if wantWrite { + st.OK, st.Refused, st.Error = false, true, "the key cannot change Keystone: it needs the admin scope" + } + case codes.OK: + log.Error().Str("key", name).Msg("Keystone accepted an invalid MintAPIKey probe") + st.OK, st.Error = false, "Keystone accepted an invalid key request" + default: + return failed(st, err) } - return false, err + return st } - admin := keyStatus{Name: "KTMS_ADMIN_KEY", OK: true} - if w, err := mayWrite(s.cfg.ProxyAdminKey); err != nil { - admin.OK, admin.Error = false, status.Convert(err).Message() - } else if !w { - admin.OK, admin.Error = false, "the key cannot change Keystone: it needs the admin scope" + var wg sync.WaitGroup + out := []keyStatus{{}} + if s.cfg.ProxyReadKey != "" { + out = append(out, keyStatus{}) + wg.Add(1) + go func() { defer wg.Done(); out[1] = probe("KTMS_READ_KEY", s.cfg.ProxyReadKey, false) }() } - out := []keyStatus{admin} - if s.cfg.ProxyReadKey == "" { - return out - } - read := keyStatus{Name: "KTMS_READ_KEY", OK: true} - if _, err := s.admin.ListAPIKeys(context.WithValue(ctx, keyCtx{}, s.cfg.ProxyReadKey), &pb.ListAPIKeysRequest{}); err != nil { - read.OK, read.Error = false, status.Convert(err).Message() - } else if w, err := mayWrite(s.cfg.ProxyReadKey); err != nil { - read.OK, read.Error = false, status.Convert(err).Message() - } else if w { - read.OK, read.Error = false, "the key can change Keystone: mint one with only the admin:read scope" - } - return append(out, read) + out[0] = probe("KTMS_ADMIN_KEY", s.cfg.ProxyAdminKey, true) + wg.Wait() + s.keyStatus, s.keyAt = out, time.Now() + return out +} + +func failed(st keyStatus, err error) keyStatus { + c := status.Code(err) + st.OK, st.Error = false, status.Convert(err).Message() + st.Refused = c == codes.Unauthenticated || c == codes.PermissionDenied + return st } func (s *Server) health(w http.ResponseWriter, r *http.Request, _ *auth.User) { ctx, cancel := context.WithTimeout(r.Context(), 4*time.Second) defer cancel() out := map[string]backendStatus{"proxy": {}, "usage": {}} + var wg sync.WaitGroup + var proxy, use backendStatus if s.admin != nil { - st := backendStatus{Enabled: true, OK: true} - if _, err := s.admin.ListAPIKeys(ctx, &pb.ListAPIKeysRequest{}); err != nil { - st.OK, st.Error = false, status.Convert(err).Message() - c := status.Code(err) - st.Auth = c == codes.Unauthenticated || c == codes.PermissionDenied - } - if st.OK || st.Auth { - st.Keys = s.checkKeys(ctx) - for _, k := range st.Keys { - st.OK, st.Auth = st.OK && k.OK, st.Auth || !k.OK + wg.Add(1) + go func() { + defer wg.Done() + proxy = backendStatus{Enabled: true, OK: true} + if _, err := s.admin.ListAPIKeys(ctx, &pb.ListAPIKeysRequest{}); err != nil { + k := failed(keyStatus{}, err) + proxy.OK, proxy.Error, proxy.Auth = false, k.Error, k.Refused } - } - out["proxy"] = st + if !proxy.OK && !proxy.Auth { + return // down: the keys cannot be judged + } + proxy.Keys = s.checkKeys() + for _, k := range proxy.Keys { + if !k.OK { + proxy.OK, proxy.Auth = false, proxy.Auth || k.Refused + if proxy.Error == "" { + proxy.Error = k.Name + ": " + k.Error + } + } + } + }() } if s.usage != nil { - st := backendStatus{Enabled: true, OK: true} - if err := s.usage.Ready(ctx); err != nil { - var authErr *usage.AuthError - st.OK, st.Error, st.Auth = false, err.Error(), errors.As(err, &authErr) - } - out["usage"] = st + wg.Add(1) + go func() { + defer wg.Done() + use = backendStatus{Enabled: true, OK: true} + if err := s.usage.Ready(ctx); err != nil { + var authErr *usage.AuthError + use.OK, use.Error, use.Auth = false, err.Error(), errors.As(err, &authErr) + } + }() + } + wg.Wait() + if s.admin != nil { + out["proxy"] = proxy + } + if s.usage != nil { + out["usage"] = use } writeJSON(w, http.StatusOK, out) } diff --git a/internal/api/readkey_test.go b/internal/api/readkey_test.go index 37fbe6e..cdb9754 100644 --- a/internal/api/readkey_test.go +++ b/internal/api/readkey_test.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "sync" "testing" "google.golang.org/grpc" @@ -20,10 +21,18 @@ type scopedAdmin struct { key func(context.Context, string) string admin string read string + mu sync.Mutex // the health probes call concurrently used map[string]string // method → key it carried + mints int + down bool } func (f *scopedAdmin) check(ctx context.Context, method string, write bool) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.down { + return status.Error(codes.Unavailable, "connection refused") + } k := f.key(ctx, method) f.used[method] = k switch { @@ -45,6 +54,9 @@ func (f *scopedAdmin) ListAPIKeys(ctx context.Context, r *pb.ListAPIKeysRequest, } func (f *scopedAdmin) MintAPIKey(ctx context.Context, r *pb.MintAPIKeyRequest, o ...grpc.CallOption) (*pb.MintAPIKeyResponse, error) { + f.mu.Lock() + f.mints++ + f.mu.Unlock() if err := f.check(ctx, "/ktms.v1.AdminService/MintAPIKey", true); err != nil { return nil, err } @@ -141,8 +153,21 @@ func TestHealthChecksBothKeys(t *testing.T) { } // An admin key in KTMS_READ_KEY is reported: it is not read-only. - s, _ = scoped(t, "ktms_admin") - if st := health(s); st.OK || st.Keys[1].OK { + s, sa = scoped(t, "ktms_admin") + if st := health(s); st.OK || st.Keys[1].OK || !st.Keys[1].Refused { t.Fatalf("an admin key as the read key: %+v", st) } + // The probes are cached: Keystone sees them once a minute, not per page. + before := sa.mints + health(s) + if sa.mints != before { + t.Fatalf("keys probed again within %s", keyCheckTTL) + } + + // A gateway that is down is down, not a refused key. + s, sa = scoped(t, "ktms_read") + sa.down = true + if st := health(s); st.OK || st.Auth || st.Error == "" { + t.Fatalf("a gateway that is down: %+v", st) + } } diff --git a/internal/config/config.go b/internal/config/config.go index 6a3ac81..21e68a7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -50,7 +50,7 @@ type Config struct { // ProxyReadKey, a key with the admin:read scope, carries every read and // every viewer's call, so those cannot change Keystone. Optional. ProxyReadKey string - ProxyTLS bool + ProxyTLS bool TokenmeterURL string // TokenmeterToken is a light-tokenmeter bearer token with the read scope. @@ -117,7 +117,7 @@ func Load() (*Config, error) { InsecureNoAuth: env("INSECURE_NO_AUTH", "") == "true", ProxyAddr: env("KTMS_ADDR", env("LLM_PROXY_ADDR", "")), ProxyAdminKey: env("KTMS_ADMIN_KEY", env("LLM_PROXY_ADMIN_KEY", "")), - ProxyReadKey: strings.TrimSpace(env("KTMS_READ_KEY", "")), + ProxyReadKey: env("KTMS_READ_KEY", ""), ProxyTLS: env("KTMS_TLS", env("LLM_PROXY_TLS", "")) == "true", TokenmeterURL: strings.TrimRight(env("TOKENMETER_URL", ""), "/"), // Trimmed: a secret written with echo ends in a newline, which no header may carry. diff --git a/internal/web/static/js/main.js b/internal/web/static/js/main.js index 35eb787..d9a0a7d 100644 --- a/internal/web/static/js/main.js +++ b/internal/web/static/js/main.js @@ -67,7 +67,7 @@ function Health() { const [h, setH] = useState(session.health); useEffect(() => { api('GET', '/api/health').then((x) => { session.health = x; setH(x); }).catch(() => {}); }, []); const row = (label, st) => st?.enabled && html`
${label}${!st.ok && html`${st.auth ? 'refused' : 'down'}`}
`; - const keyRow = (k) => html`
${k.name}${!k.ok && html`refused`}
`; + const keyRow = (k) => html`
${k.name}${!k.ok && html`${k.refused ? 'refused' : 'unchecked'}`}
`; if (!h || (!h.proxy?.enabled && !h.usage?.enabled)) return null; return html`
${row('KTMS gateway', h.proxy)}${h.proxy?.keys?.map(keyRow)}${row('Tokenmeter', h.usage)}
`; } diff --git a/scripts/e2e.sh b/scripts/e2e.sh index d823cba..a515733 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -172,9 +172,14 @@ until api GET "/api/usage?from=$SINCE&to=$DAY&group=model" | jq -e '(.report.gro done echo "e2e: the call's usage went KTMS -> light-tokenmeter (write token) -> console (read token)" -# Keystone 1.2 knows admin:read; an older one refuses the scope, and this part -# is skipped. -if READ_KEY=$(api POST /api/proxy/keys '{"name":"console-read","scopes":["admin:read"]}' 2>/dev/null | jq -er .plain_key); then +# Keystone 1.2 knows admin:read; an older one refuses the scope as unknown, +# and only then is this part skipped. +if READ_KEY=$(api POST /api/proxy/keys '{"name":"console-read","scopes":["admin:read"]}' 2>"$WORK/read-key.err" | jq -er .plain_key); then + # A bogus read key breaks reads and not changes: reads carry KTMS_READ_KEY. + run_console "$CONSOLE_KEY" "$R" "ktms_not_a_key" + if api GET /api/proxy/config >/dev/null 2>&1; then fail "a read did not go with KTMS_READ_KEY"; fi + api PUT /api/proxy/rules '{"rules":[{"name":"catch-all","priority":10,"use":["mock-gpt"],"enabled":true}]}' >/dev/null || + fail "a change went with KTMS_READ_KEY" run_console "$CONSOLE_KEY" "$R" "$READ_KEY" api GET /api/health | jq -e '.proxy.ok and (.proxy.keys | length == 2) and all(.proxy.keys[]; .ok)' >/dev/null || fail "health with both keys: $(api GET /api/health)" @@ -186,8 +191,10 @@ if READ_KEY=$(api POST /api/proxy/keys '{"name":"console-read","scopes":["admin: api GET /api/health | jq -e '.proxy.ok == false and .proxy.auth == true' >/dev/null || fail "a read key as KTMS_ADMIN_KEY was not reported: $(api GET /api/health)" echo "e2e: reads go with KTMS_READ_KEY, changes with KTMS_ADMIN_KEY, and a wrong key is reported" -else +elif grep -q 'unknown scope' "$WORK/read-key.err"; then echo "e2e: ktms@$KTMS_REF has no admin:read scope; KTMS_READ_KEY not tested" +else + fail "minting a read key: $(cat "$WORK/read-key.err")" fi # A token without the read scope must show as refused, not as down.