diff --git a/README.md b/README.md index b8b502b..037ac12 100644 --- a/README.md +++ b/README.md @@ -801,8 +801,10 @@ or in CI. `tokensOutPerMonth`, `costPerDayMicroUsd`, `costPerMonthMicroUsd` — alongside their configured limits, plus one extra `total` row: the synthetic all-traffic scope (see [Limits and accounting]( - #limits-and-accounting)), limits always `null`. The dashboard's Overview - and Usage views poll both every 5 seconds while open. + #limits-and-accounting)), limits always `null`. A group row also carries + its configured `providers`/`models`/`mcpServers`/`agents` access lists, + omitted when unrestricted. The dashboard's Providers and Usage views + poll both every 5 seconds while open. - **`GET /admin/api/usage/history`** returns a bucketed series for one scope/metric/window — the data source for the dashboard's Charts view (per-user/per-group/total, stacked tokens-in/tokens-out, with a diff --git a/admin_test.go b/admin_test.go index ca7d6e7..32f58f6 100644 --- a/admin_test.go +++ b/admin_test.go @@ -496,14 +496,76 @@ func findUsageEntry(t *testing.T, entries []adminUsageEntryView, id string) admi return adminUsageEntryView{} } +// accessListKeys are adminUsageEntryView's four group-access JSON keys +// (admin.go) — the set TestAdminUsage_GroupAccessLists checks both at the +// typed-struct level and, since a nil Go slice and an absent JSON key +// decode identically into a struct field either way, at the raw-JSON +// level too (assertRawAccessList below): a struct-only assertion would +// stay green even if the "omitempty" json tags were removed, since +// encoding/json would then emit "null" for an unset field and +// json.Unmarshal reads a JSON null back into a nil []string all the same. +var accessListKeys = []string{"providers", "models", "mcpServers", "agents"} + +// findRawUsageEntry finds row id's raw JSON object (one element of GET +// /admin/api/usage's users/groups array, or the total object) among rows — +// the raw-JSON counterpart to findUsageEntry, used where a typed-struct +// decode cannot distinguish "key absent" from "key present but null" (see +// accessListKeys' own doc comment). +func findRawUsageEntry(t *testing.T, rows []map[string]json.RawMessage, id string) map[string]json.RawMessage { + t.Helper() + for _, row := range rows { + var rowID string + if err := json.Unmarshal(row["id"], &rowID); err != nil { + t.Fatalf("decode id: %v", err) + } + if rowID == id { + return row + } + } + t.Fatalf("no raw usage entry for id %q", id) + return nil +} + +// assertRawAccessList asserts one access-list key's raw JSON presence and +// value on row: want == nil means the key must be ENTIRELY ABSENT from the +// JSON object (not present-as-null, not present-as-"[]") — the omitempty +// contract adminUsageEntryView's doc comment promises; a non-nil want means +// the key must be present with exactly that value. +func assertRawAccessList(t *testing.T, row map[string]json.RawMessage, key string, want []string) { + t.Helper() + raw, present := row[key] + if want == nil { + if present { + t.Errorf("key %q = %s, want entirely absent (unrestricted/non-group row)", key, raw) + } + return + } + if !present { + t.Errorf("key %q absent, want present with value %v", key, want) + return + } + var got []string + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("decode %q: %v", key, err) + } + if !slices.Equal(got, want) { + t.Errorf("key %q = %v, want %v", key, got, want) + } +} + // TestAdminUsage_GroupAccessLists proves GET /admin/api/usage's per-group // rows (group-access-display task) echo GroupConfig's Providers/Models/ // MCPServers/Agents exactly as configured — no server-side expansion to the // full provider/model catalog — and that a group with none of those set -// carries empty/omitted lists rather than a resolved "everything" set, -// matching group.allowsX's own empty-means-all contract (auth.go). A user -// row and the synthetic total row must never carry these fields at all — -// they are group-only, same convention as GroupName's own user-only field. +// carries the keys entirely absent (json:",omitempty") rather than present +// with a resolved "everything" set, matching group.allowsX's own +// empty-means-all contract (auth.go). A user row and the synthetic total +// row must never carry these fields at all — they are group-only, same +// convention as GroupName's own user-only field. Assertions run at both the +// typed-struct level (adminUsageResponse) and the raw-JSON level +// (assertRawAccessList) — the typed level alone cannot prove the field was +// actually omitted from the wire, only that it decoded to nil, which a +// present-but-null field would do identically. func TestAdminUsage_GroupAccessLists(t *testing.T) { t.Parallel() cfg := CreateConfig() @@ -537,6 +599,15 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) { t.Fatalf("decode: %v", err) } + var raw struct { + Total map[string]json.RawMessage `json:"total"` + Users []map[string]json.RawMessage `json:"users"` + Groups []map[string]json.RawMessage `json:"groups"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil { + t.Fatalf("decode raw: %v", err) + } + tests := []struct { name string group string @@ -554,7 +625,7 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) { wantAgents: []string{"agent1"}, }, { - name: "unrestricted group carries empty/omitted lists", + name: "unrestricted group carries empty or omitted lists", group: "open", }, } @@ -573,6 +644,12 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) { if !slices.Equal(entry.Agents, tc.wantAgents) { t.Errorf("agents = %v, want %v", entry.Agents, tc.wantAgents) } + + rawEntry := findRawUsageEntry(t, raw.Groups, tc.group) + assertRawAccessList(t, rawEntry, "providers", tc.wantProviders) + assertRawAccessList(t, rawEntry, "models", tc.wantModels) + assertRawAccessList(t, rawEntry, "mcpServers", tc.wantMCPServers) + assertRawAccessList(t, rawEntry, "agents", tc.wantAgents) }) } @@ -585,6 +662,13 @@ func TestAdminUsage_GroupAccessLists(t *testing.T) { t.Errorf("total entry must never carry group access lists, got providers=%v models=%v mcpServers=%v agents=%v", got.Total.Providers, got.Total.Models, got.Total.MCPServers, got.Total.Agents) } + + rawAdmin1 := findRawUsageEntry(t, raw.Users, "admin1") + rawTotal := raw.Total + for _, key := range accessListKeys { + assertRawAccessList(t, rawAdmin1, key, nil) + assertRawAccessList(t, rawTotal, key, nil) + } } // TestLimiterCurrentUsage_StoreDown drives limiter.currentUsage's diff --git a/auth.go b/auth.go index 4d6a08a..b78ac6a 100644 --- a/auth.go +++ b/auth.go @@ -82,6 +82,24 @@ func (grp *group) allowsAgent(name string) bool { return matchesGlob(grp.agents, name) } +// cloneStringSlice returns an independent copy of s, preserving nil (a nil +// s returns nil, never an empty non-nil slice — groupSummary's own +// providers/models/mcpServers/agents fields rely on that nil-ness to stay +// "omitempty" all the way to the JSON response, admin.go's +// adminUsageEntryView). Plain builtins only (make/copy, no generics, no +// non-stdlib package): the plugin runs interpreted under Yaegi (see +// tools/yaegi-check), whose stdlib symbol table does not resolve the +// generic "slices" package's type parameters — this is why a +// slices.Clone call is not used here. +func cloneStringSlice(s []string) []string { + if s == nil { + return nil + } + out := make([]string, len(s)) + copy(out, s) + return out +} + // matchesGlob reports whether candidate matches any shell-style pattern in // patterns (path.Match semantics). An empty pattern list matches anything. // A malformed pattern (path.ErrBadPattern) is treated as a non-match rather @@ -340,6 +358,13 @@ type userSummary struct { // provider/model/MCP server/agent is allowed (matchesGlob's own contract, // auth.go), never expanded to the full catalog here — the admin dashboard // echoes the configured glob list, not a resolved membership set. +// +// snapshot (below) clones all four slices rather than aliasing group's own +// — group.providers/models/mcpServers/agents are the live authorization +// data every allowsX call reads on the request path; groupSummary is a +// read-only, dashboard-facing COPY, so a future caller that sorts or +// otherwise mutates a groupSummary's list in place can never reorder or +// corrupt the slice authorization itself still relies on. type groupSummary struct { limits *LimitsConfig name string @@ -371,10 +396,10 @@ func (a *authStore) snapshot() ([]userSummary, []groupSummary) { groups = append(groups, groupSummary{ limits: grp.limits, name: name, - providers: grp.providers, - models: grp.models, - mcpServers: grp.mcpServers, - agents: grp.agents, + providers: cloneStringSlice(grp.providers), + models: cloneStringSlice(grp.models), + mcpServers: cloneStringSlice(grp.mcpServers), + agents: cloneStringSlice(grp.agents), memberCount: memberCounts[name], }) } diff --git a/webui/src/components/UsageView.vue b/webui/src/components/UsageView.vue index c2fb2fe..fcadcb0 100644 --- a/webui/src/components/UsageView.vue +++ b/webui/src/components/UsageView.vue @@ -227,6 +227,32 @@ const expandedGroupValues = computed({ watch(userQuery, (value) => { if (value.trim() === '') clearExpandOverrides(groupExpandState) }) + +// --- Access block (group-access-display task, review round) --- +// +// GLOB_HINT is the title attribute on every Access label (Providers, +// Models, MCP servers, Agents): group.allowsX (auth.go) matches every one +// of these four configured lists via matchesGlob/path.Match semantics, not +// literal equality — a heading that just says "Models" leaves that +// invisible. One constant, not four copies of the same string (vue.md: +// "if you've written it twice, you owe an abstraction"). +const GLOB_HINT = 'Glob pattern — matches like a shell wildcard (*, ?, [...])' + +/** + * isGlobPattern reports whether one configured models entry is a glob + * pattern rather than a literal model id — the same special characters + * path.Match recognizes (auth.go's matchesGlob, which group.allowsModel + * calls). Only Models needs this distinction rendered differently: a + * literal id gets the copyable ModelChip treatment (it IS a routable id a + * caller can paste into a request's `model` field); a pattern like + * "gpt-4*" is not a valid id and must not invite copying it as one, so it + * renders as a plain, non-copyable Badge instead (see the template below). + * Providers/MCPServers/Agents render every entry as a plain Badge either + * way, so they need no such split. + */ +function isGlobPattern(entry: string): boolean { + return /[*?[]/.test(entry) +}