From 74504427df0b23486389e426d383263f9907ff77 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:29:30 +0100 Subject: [PATCH] Show diagnostics callers only the API tokens they may manage /api/diagnostics is readable with settings:read and lists the whole token inventory: every token's ID, name, timestamps, key hint and which agents use it, across organizations. A token bound to one organization could read the other organizations' credential metadata, which the token management endpoints already refuse to show it. The diagnostics result is computed once per organization scope and cached for every caller, so the filter runs after the cache: an API-token caller that is not instance-wide gets an API token section rebuilt over the tokens callerTokenMayManage allows, with counts, notes and agent usage computed over that view. Instance-wide tokens and browser sessions get the full section, and the cache itself still holds the full result. Tests cover the builder (tokens, counts, usage) and the handler on the cached path, including that a scoped caller served from the cache does not change what the next caller sees. Each filter fails when removed. Contract-Neutral: Behavioural fix that applies the token visibility rule to diagnostics; no new API surface or contract. --- internal/api/diagnostics.go | 52 ++++-- internal/api/diagnostics_additional_test.go | 2 +- .../api/diagnostics_token_visibility_test.go | 149 ++++++++++++++++++ 3 files changed, 192 insertions(+), 11 deletions(-) create mode 100644 internal/api/diagnostics_token_visibility_test.go diff --git a/internal/api/diagnostics.go b/internal/api/diagnostics.go index 3876201258..6220e99cea 100644 --- a/internal/api/diagnostics.go +++ b/internal/api/diagnostics.go @@ -816,7 +816,7 @@ func (r *Router) handleDiagnostics(w http.ResponseWriter, req *http.Request) { if ok && !cachedEntry.cachedAt.IsZero() && now.Sub(cachedEntry.cachedAt) <= diagnosticsCacheTTL { diagnosticsCacheHits.Inc() - writeDiagnosticsResponse(w, cachedEntry.diag, cachedEntry.cachedAt) + writeDiagnosticsResponse(w, r.diagnosticsForCaller(req, cachedEntry.diag), cachedEntry.cachedAt) return } @@ -857,7 +857,23 @@ func (r *Router) handleDiagnostics(w http.ResponseWriter, req *http.Request) { }) result := v.(diagResult) - writeDiagnosticsResponse(w, result.diag, result.cachedAt) + writeDiagnosticsResponse(w, r.diagnosticsForCaller(req, result.diag), result.cachedAt) +} + +// diagnosticsForCaller adapts a cached diagnostics result to the caller. The +// cache holds one result per organization scope, built for the instance, and +// is shared by every caller, so per-caller filtering happens here, after the +// cache. The API token section lists credentials, so an API-token caller sees +// only the tokens it may manage (see callerTokenMayManage); instance-wide +// tokens and browser sessions get the full section. +func (r *Router) diagnosticsForCaller(req *http.Request, diag DiagnosticsInfo) DiagnosticsInfo { + caller := getAPITokenRecordFromRequest(req) + if caller == nil || tokenHasInstanceWideAuthority(caller) || diag.APITokens == nil { + return diag + } + view := diag + view.APITokens = buildAPITokenDiagnostic(r.config, r.monitor, caller) + return view } func writeDiagnosticsResponse(w http.ResponseWriter, diag DiagnosticsInfo, cachedAt time.Time) { @@ -1079,7 +1095,7 @@ func (r *Router) computeDiagnostics(ctx context.Context) DiagnosticsInfo { runtime.ReadMemStats(&memStats) diag.System = buildSystemDiagnostic(memStats, currentProcessRSS(), debug.SetMemoryLimit(-1)) - diag.APITokens = buildAPITokenDiagnostic(r.config, r.monitor) + diag.APITokens = buildAPITokenDiagnostic(r.config, r.monitor, nil) diag.MetricsStore = buildMetricsStoreDiagnostic(r.monitor) // Test each configured node @@ -1322,14 +1338,27 @@ func buildDiscoveryDiagnostic(cfg *config.Config, monitor *monitoring.Monitor) * return discovery } -func buildAPITokenDiagnostic(cfg *config.Config, monitor *monitoring.Monitor) *APITokenDiagnostic { +// buildAPITokenDiagnostic summarises the token inventory. A nil caller sees +// every token; an API-token caller sees only the tokens callerTokenMayManage +// allows, and the counts, notes and usage are computed over that view. +func buildAPITokenDiagnostic(cfg *config.Config, monitor *monitoring.Monitor, caller *config.APITokenRecord) *APITokenDiagnostic { if cfg == nil { return nil } + records := make([]config.APITokenRecord, 0, len(cfg.APITokens)) + visibleIDs := make(map[string]struct{}, len(cfg.APITokens)) + for _, record := range cfg.APITokens { + if !callerTokenMayManage(caller, &record) { + continue + } + records = append(records, record) + visibleIDs[record.ID] = struct{}{} + } + diag := &APITokenDiagnostic{ Enabled: cfg.HasAPITokens(), - TokenCount: len(cfg.APITokens), + TokenCount: len(records), } appendNote := func(note string) { @@ -1339,15 +1368,15 @@ func buildAPITokenDiagnostic(cfg *config.Config, monitor *monitoring.Monitor) *A diag.Notes = append(diag.Notes, note) } - diag.RecommendTokenSetup = len(cfg.APITokens) == 0 + diag.RecommendTokenSetup = !cfg.HasAPITokens() if diag.RecommendTokenSetup { appendNote("No API tokens are configured. Open Settings → Security to generate dedicated tokens for each automation or agent.") } - tokens := make([]APITokenSummary, 0, len(cfg.APITokens)) + tokens := make([]APITokenSummary, 0, len(records)) unusedCount := 0 - for _, record := range cfg.APITokens { + for _, record := range records { summary := APITokenSummary{ ID: record.ID, Name: record.Name, @@ -1380,8 +1409,8 @@ func buildAPITokenDiagnostic(cfg *config.Config, monitor *monitoring.Monitor) *A diag.Tokens = tokens diag.UnusedTokenCount = unusedCount - if len(cfg.APITokens) > 0 { - if unusedCount == len(cfg.APITokens) { + if len(records) > 0 { + if unusedCount == len(records) { appendNote("Configured API tokens have not been used yet. Update your agents or automations to switch to the new tokens.") } else if unusedCount > 0 { appendNote(fmt.Sprintf("%d API token(s) have never been used. Remove unused tokens or update the corresponding agents.", unusedCount)) @@ -1397,6 +1426,9 @@ func buildAPITokenDiagnostic(cfg *config.Config, monitor *monitoring.Monitor) *A continue } tokenID := strings.TrimSpace(host.TokenID()) + if _, visible := visibleIDs[tokenID]; !visible { + continue + } tokenUsage[tokenID] = append(tokenUsage[tokenID], name) } } diff --git a/internal/api/diagnostics_additional_test.go b/internal/api/diagnostics_additional_test.go index 2cce32d730..df2327ebc8 100644 --- a/internal/api/diagnostics_additional_test.go +++ b/internal/api/diagnostics_additional_test.go @@ -547,7 +547,7 @@ func TestBuildAPITokenDiagnostic_WithDockerUsage(t *testing.T) { t.Fatalf("ApplyDockerReport legacy: %v", err) } - diag := buildAPITokenDiagnostic(cfg, monitor) + diag := buildAPITokenDiagnostic(cfg, monitor, nil) if diag == nil || !diag.Enabled { t.Fatalf("expected diagnostics enabled") } diff --git a/internal/api/diagnostics_token_visibility_test.go b/internal/api/diagnostics_token_visibility_test.go new file mode 100644 index 0000000000..708ebf714c --- /dev/null +++ b/internal/api/diagnostics_token_visibility_test.go @@ -0,0 +1,149 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" +) + +// Diagnostics is readable with settings:read and lists API credentials: IDs, +// names, timestamps, key hints and which agents use each token. The token +// inventory is global, so a token caller must see only the tokens it may +// manage, as on the token endpoints themselves. +type diagnosticsTokenFixture struct { + cfg *config.Config + orgA, def, orgB config.APITokenRecord +} + +func newDiagnosticsTokenFixture(t *testing.T) *diagnosticsTokenFixture { + t.Helper() + mk := func(raw, name, org string, scopes []string) config.APITokenRecord { + record, err := config.NewAPITokenRecord(raw, name, scopes) + if err != nil { + t.Fatalf("new token record %s: %v", name, err) + } + record.OrgID = org + return *record + } + f := &diagnosticsTokenFixture{ + orgA: mk("diag-org-a-wildcard.12345678", "org-a admin", "org-a", []string{config.ScopeWildcard}), + def: mk("diag-default-wildcard.12345678", "default admin", "default", []string{config.ScopeWildcard}), + orgB: mk("diag-org-b-scoped.12345678", "org-b reader", "org-b", []string{config.ScopeMonitoringRead}), + } + f.cfg = &config.Config{DataPath: t.TempDir(), APITokens: []config.APITokenRecord{f.orgA, f.def, f.orgB}} + f.cfg.SortAPITokens() + return f +} + +func tokenIDs(diag *APITokenDiagnostic) map[string]bool { + ids := map[string]bool{} + for _, token := range diag.Tokens { + ids[token.ID] = true + } + return ids +} + +func TestAPITokenDiagnosticShowsOnlyTokensTheCallerMayManage(t *testing.T) { + f := newDiagnosticsTokenFixture(t) + monitor := newMonitorForDiagnostics(t, f.cfg) + now := time.Now().UTC() + for i, token := range []config.APITokenRecord{f.orgA, f.def, f.orgB} { + report := agentsdocker.Report{ + Agent: agentsdocker.AgentInfo{ID: "agent-" + token.Name, Version: "1.0.0"}, + Host: agentsdocker.HostInfo{Hostname: "docker-" + token.Name, MachineID: "machine-" + token.Name}, + Timestamp: now.Add(time.Duration(i) * time.Second), + } + if _, err := monitor.ApplyDockerReport(report, &config.APITokenRecord{ID: token.ID}); err != nil { + t.Fatalf("ApplyDockerReport %s: %v", token.Name, err) + } + } + + all := buildAPITokenDiagnostic(f.cfg, monitor, nil) + if all.TokenCount != 3 || len(all.Usage) != 3 { + t.Fatalf("an unfiltered view should list all three tokens and usages, got %d tokens, %d usages", all.TokenCount, len(all.Usage)) + } + + view := buildAPITokenDiagnostic(f.cfg, monitor, &f.orgA) + ids := tokenIDs(view) + if len(ids) != 1 || !ids[f.orgA.ID] || view.TokenCount != 1 { + t.Fatalf("org-a caller should see only its own token, got ids %v count %d", ids, view.TokenCount) + } + if len(view.Usage) != 1 || view.Usage[0].TokenID != f.orgA.ID { + t.Fatalf("usage should be limited to the caller's token, got %+v", view.Usage) + } + if view.UnusedTokenCount != 1 { + t.Fatalf("unused count should be computed over the visible tokens, got %d", view.UnusedTokenCount) + } + // The existence of other tokens still shows in the global flag only. + if !view.Enabled || view.RecommendTokenSetup { + t.Fatalf("enabled=%v recommend=%v, want enabled and no setup recommendation", view.Enabled, view.RecommendTokenSetup) + } + + instanceWide := buildAPITokenDiagnostic(f.cfg, monitor, &f.def) + if instanceWide.TokenCount != 3 { + t.Fatalf("an instance-wide caller passed to the builder sees %d tokens, want 3 (callers filter nothing for it)", instanceWide.TokenCount) + } +} + +func TestDiagnosticsFiltersTokenSectionAfterTheSharedCache(t *testing.T) { + f := newDiagnosticsTokenFixture(t) + router := &Router{config: f.cfg} + + // The cache holds the instance-wide result and is shared across callers. + diagnosticsCacheMu.Lock() + prevCache := diagnosticsCache + diagnosticsCache = map[string]cachedDiagnosticsEntry{ + diagnosticsScopeKey(context.Background()): { + diag: DiagnosticsInfo{Version: "cached", APITokens: buildAPITokenDiagnostic(f.cfg, nil, nil)}, + cachedAt: time.Now(), + }, + } + diagnosticsCacheMu.Unlock() + t.Cleanup(func() { + diagnosticsCacheMu.Lock() + diagnosticsCache = prevCache + diagnosticsCacheMu.Unlock() + }) + + get := func(caller *config.APITokenRecord) (*httptest.ResponseRecorder, DiagnosticsInfo) { + req := httptest.NewRequest(http.MethodGet, "/api/diagnostics", nil) + if caller != nil { + attachAPITokenRecord(req, caller) + } + rec := httptest.NewRecorder() + router.handleDiagnostics(rec, req) + var payload DiagnosticsInfo + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode diagnostics: %v", err) + } + return rec, payload + } + + rec, scoped := get(&f.orgA) + if rec.Code != http.StatusOK || scoped.Version != "cached" { + t.Fatalf("status %d version %q, want a 200 served from the cache", rec.Code, scoped.Version) + } + if ids := tokenIDs(scoped.APITokens); len(ids) != 1 || !ids[f.orgA.ID] { + t.Fatalf("org-a caller saw tokens %v, want only its own", ids) + } + for _, other := range []config.APITokenRecord{f.def, f.orgB} { + if strings.Contains(rec.Body.String(), other.ID) || strings.Contains(rec.Body.String(), other.Name) { + t.Fatalf("the response leaked token %q: %s", other.Name, rec.Body.String()) + } + } + + // A caller served right after still gets its own view, not the previous one. + for name, caller := range map[string]*config.APITokenRecord{"instance-wide token": &f.def, "browser session": nil} { + _, full := get(caller) + if got := len(tokenIDs(full.APITokens)); got != 3 { + t.Fatalf("%s saw %d tokens, want all 3 after a scoped caller was served from the same cache", name, got) + } + } +}