Information disclosure / Insecure error handling

POC steps and code (conceptual): - Objective: Demonstrate the potential information disclosure via plugin settings retrieval and how the fix mitigates it. - Context: In Grafana 12.x before the fix, querying settings for a non-installed plugin could surface an error (e.g., Unknown Plugin) with a 404 status, which could be used to enumerate installed vs non-installed plugins based on error signaling in the UI. - Reproduction (vulnerable behavior): 1) Enumerate a list of plausible plugin IDs (e.g., ["grafana-piechart-panel", "myorg-test-app", "nonexistent-plugin"]). 2) From a page/component that calls usePluginSettings(pluginId), trigger a request to GET /api/plugins/{pluginId}/settings. 3) If the plugin is not installed, observe the UI showing an error or a detailed error message (containing 404 status or Unknown Plugin) that can reveal plugin absence/presence. - Reproduction (fixed behavior in this commit): 1) Repeat the steps above with Grafana 12.4.0 or later. 2) For a non-installed plugin, the request resolves to undefined with no error surfaced to the UI, preventing leakage of plugin existence. Code-style repro snippet (Node.js fetch-based, environment-agnostic): const fetch = require('node-fetch'); const pluginIds = ['grafana-piechart-panel', 'myorg-test-app', 'nonexistent-plugin']; async function probe(id) { const res = await fetch(`https://grafana.example.com/api/plugins/${id}/settings`, { headers: { Authorization: 'Bearer <token>' } }); if (res.status === 200) { const meta = await res.json(); console.log(id, 'installed', meta); } else if (res.status === 404) { // Pre-fix: would surface an error; Post-fix: treated as not installed with no error console.log(id, 'not installed (404)'); } else { console.log(id, 'other error', res.status); } } pluginIds.forEach(id => probe(id)); Notes: - In a browser UI, the PoC manifests as an observable error for 404 on the vulnerable versions; after the fix, the same 404 yields no error and value is undefined. This makes automated plugin enumeration harder and reduces UI error leakage.

diff --git a/packages/grafana-runtime/src/services/pluginSettings/hooks.test.ts b/packages/grafana-runtime/src/services/pluginSettings/hooks.test.ts index ff3372023296d..b8c47c270d3d0 100644 --- a/packages/grafana-runtime/src/services/pluginSettings/hooks.test.ts +++ b/packages/grafana-runtime/src/services/pluginSettings/hooks.test.ts @@ -1,8 +1,18 @@ import { renderHook, waitFor } from '@testing-library/react'; -import { useAppPluginEnabled } from './hooks'; +import { PluginType, type PluginMeta } from '@grafana/data'; + +import { getPluginSettings } from './getPluginSettings'; +import { useAppPluginEnabled, usePluginSettings } from './hooks'; import { getAppPluginEnabled } from './settings'; +jest.mock('./getPluginSettings', () => ({ + ...jest.requireActual('./getPluginSettings'), + getPluginSettings: jest.fn(), +})); + +const getPluginSettingsMock = jest.mocked(getPluginSettings); + jest.mock('./settings', () => ({ ...jest.requireActual('./settings.ts'), getAppPluginEnabled: jest.fn(), @@ -65,4 +75,83 @@ describe('hooks', () => { expect(result.current.value).toBeUndefined(); }); }); + + describe('usePluginSettings', () => { + const mockPluginMeta: PluginMeta = { + id: 'myorg-test-app', + name: 'Test App', + type: PluginType.app, + module: '', + baseUrl: '', + info: { + author: { name: 'John Doe' }, + description: 'This is my test app', + links: [], + logos: { large: 'logo-large.png', small: 'logo-small.png' }, + screenshots: [], + updated: new Date().toISOString(), + version: '1.0.0', + }, + }; + + it('should return loading state initially', async () => { + getPluginSettingsMock.mockResolvedValue(mockPluginMeta); + + const { result } = renderHook(() => usePluginSettings('myorg-test-app')); + + expect(result.current.loading).toEqual(true); + expect(result.current.error).toBeUndefined(); + expect(result.current.value).toBeUndefined(); + + // suppress act() warning + await waitFor(() => expect(result.current.loading).toEqual(false)); + }); + + it('should return the plugin meta after loading', async () => { + getPluginSettingsMock.mockResolvedValue(mockPluginMeta); + + const { result } = renderHook(() => usePluginSettings('myorg-test-app')); + + await waitFor(() => expect(result.current.loading).toEqual(false)); + + expect(result.current.error).toBeUndefined(); + expect(result.current.value).toStrictEqual(mockPluginMeta); + }); + + it('should return undefined value without error when pluginId is empty', async () => { + const { result } = renderHook(() => usePluginSettings('')); + + await waitFor(() => expect(result.current.loading).toEqual(false)); + + expect(result.current.error).toBeUndefined(); + expect(result.current.value).toBeUndefined(); + expect(getPluginSettingsMock).not.toHaveBeenCalled(); + }); + + it('should return undefined value without error on a 404', async () => { + // getPluginSettings wraps fetch errors as: new Error('Unknown Plugin', { cause: fetchError }) + const fetchError = { status: 404, data: {} }; + getPluginSettingsMock.mockRejectedValue(new Error('Unknown Plugin', { cause: fetchError })); + + const { result } = renderHook(() => usePluginSettings('myorg-test-app')); + + await waitFor(() => expect(result.current.loading).toEqual(false)); + + expect(result.current.error).toBeUndefined(); + expect(result.current.value).toBeUndefined(); + }); + + it('should surface non-404 errors via the error field', async () => { + const fetchError = { status: 500, data: {} }; + const thrownError = new Error('Unknown Plugin', { cause: fetchError }); + getPluginSettingsMock.mockRejectedValue(thrownError); + + const { result } = renderHook(() => usePluginSettings('myorg-test-app')); + + await waitFor(() => expect(result.current.loading).toEqual(false)); + + expect(result.current.error).toStrictEqual(thrownError); + expect(result.current.value).toBeUndefined(); + }); + }); }); diff --git a/packages/grafana-runtime/src/services/pluginSettings/hooks.ts b/packages/grafana-runtime/src/services/pluginSettings/hooks.ts index 375cf5297ae80..98a324e00ccb9 100644 --- a/packages/grafana-runtime/src/services/pluginSettings/hooks.ts +++ b/packages/grafana-runtime/src/services/pluginSettings/hooks.ts @@ -1,5 +1,10 @@ import { useAsync } from 'react-use'; +import { type PluginMeta } from '@grafana/data'; + +import { isFetchError } from '../backendSrv'; + +import { getPluginSettings } from './getPluginSettings'; import { getAppPluginEnabled } from './settings'; /** @@ -12,3 +17,30 @@ export function useAppPluginEnabled(pluginId: string) { const { loading, error, value } = useAsync(async () => getAppPluginEnabled(pluginId), [pluginId]); return { loading, error, value }; } + +/** + * Hook that fetches the full plugin settings (PluginMeta) for a given plugin ID. + * @param pluginId - The ID of the plugin. + * @returns loading, error, value where value is the PluginMeta or undefined if not found. + * A 404 (plugin not installed) resolves to value: undefined with no error. + * Other failures (auth errors, server errors) are surfaced via the error field. + */ +export function usePluginSettings(pluginId: string) { + const { loading, error, value } = useAsync(async (): Promise<PluginMeta | undefined> => { + if (!pluginId) { + return undefined; + } + try { + return await getPluginSettings(pluginId); + } catch (err) { + // getLegacySettings wraps the raw fetch error as the `cause`. A 404 means the + // plugin is simply not installed — treat it as a normal absence, not an error. + const cause = err instanceof Error ? err.cause : err; + if (isFetchError(cause) && cause.status === 404) { + return undefined; + } + throw err; + } + }, [pluginId]); + return { loading, error, value }; +} diff --git a/packages/grafana-runtime/src/services/pluginSettings/settings.test.ts b/packages/grafana-runtime/src/services/pluginSettings/settings.test.ts index f9bbaf963288b..03b649343eb4d 100644 --- a/packages/grafana-runtime/src/services/pluginSettings/settings.test.ts +++ b/packages/grafana-runtime/src/services/pluginSettings/settings.test.ts @@ -150,7 +150,7 @@ describe('settings', () => { expect(enabled).toEqual(false); expect(logger.logError).toHaveBeenCalledWith( - new Error('isAppPluginEnabled: failed because of unknonw reason'), + new Error('isAppPluginEnabled: failed because of unknown reason'), { pluginId: 'myorg-test-app' } ); expect(logger.logWarning).not.toHaveBeenCalled(); diff --git a/packages/grafana-runtime/src/services/pluginSettings/settings.ts b/packages/grafana-runtime/src/services/pluginSettings/settings.ts index 05949954d286f..4fc79f989e63f 100644 --- a/packages/grafana-runtime/src/services/pluginSettings/settings.ts +++ b/packages/grafana-runtime/src/services/pluginSettings/settings.ts @@ -5,6 +5,10 @@ import { logPluginSettingsError, logPluginSettingsWarning } from './logging'; import { isAuthError } from './utils'; export async function getAppPluginEnabled(pluginId: string): Promise<boolean> { + if (!pluginId) { + return false; + } + const app = await getPluginSettings(pluginId); if (!app) { return false; @@ -26,7 +30,7 @@ export async function isAppPluginEnabled(pluginId: string): Promise<boolean> { if (isAuthError(error)) { logPluginSettingsWarning(`isAppPluginEnabled: failed because auth denied`, { pluginId }); } else { - logPluginSettingsError(`isAppPluginEnabled: failed because of unknonw reason`, error, { pluginId }); + logPluginSettingsError(`isAppPluginEnabled: failed because of unknown reason`, error, { pluginId }); } } return false; diff --git a/packages/grafana-runtime/src/unstable.ts b/packages/grafana-runtime/src/unstable.ts index 374bcc1528ece..f31406bb94ed7 100644 --- a/packages/grafana-runtime/src/unstable.ts +++ b/packages/grafana-runtime/src/unstable.ts @@ -15,6 +15,7 @@ export { defineFeatureEvents } from './analyticsFramework/main'; export type { EventProperty, Event } from './analyticsFramework/types'; export { getPluginSettings } from './services/pluginSettings/getPluginSettings'; export { updateAppPluginSettings } from './services/pluginSettings/updateAppPluginSettings'; +export { usePluginSettings } from './services/pluginSettings/hooks'; export { getDataSourceInstanceSettings, reloadDataSourceInstanceSettings } from './services/dataSource/settings'; export { getDataSourceInstance, registerRuntimeDataSourceInstance } from './services/dataSource/dataSource'; export { diff --git a/public/app/features/alerting/unified/api/pluginsApi.ts b/public/app/features/alerting/unified/api/pluginsApi.ts deleted file mode 100644 index 8c15ed9f024c7..0000000000000 --- a/public/app/features/alerting/unified/api/pluginsApi.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { type PluginMeta } from '@grafana/data'; - -import { alertingApi } from './alertingApi'; - -export const pluginsApi = alertingApi.injectEndpoints({ - endpoints: (build) => ({ - getPluginSettings: build.query<PluginMeta, string>({ - query: (pluginId) => ({ - url: `/api/plugins/${pluginId}/settings`, - notificationOptions: { - showErrorAlert: false, - }, - }), - // Keep plugin settings cached for the entire session - keepUnusedDataFor: 3600, // 1 hour in seconds - }), - }), -}); - -export const { useGetPluginSettingsQuery } = pluginsApi; diff --git a/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx b/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx index 4bf49f2a7782a..629ed74273b83 100644 --- a/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleDetails.test.tsx @@ -15,6 +15,7 @@ jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), usePluginLinks: jest.fn(), useReturnToPrevious: jest.fn(), + useAppPluginEnabled: jest.fn().mockReturnValue({ loading: false, error: undefined, value: false }), })); jest.mock('../../hooks/useIsRuleEditable'); diff --git a/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.test.tsx b/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.test.tsx index 71bd04b670541..137198246dd9b 100644 --- a/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.test.tsx +++ b/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.test.tsx @@ -97,6 +97,9 @@ describe('useRuleAdministrationAbility', () => { // Snapshot the initial loading state before the ruler resolves expect(result.current).toMatchSnapshot(); + + // Drain the async queue so the test does not leave in-flight state updates + await waitFor(() => expect(result.current.loading).toBe(false)); }); it('returns INSUFFICIENT_PERMISSIONS when user lacks folder edit permission', async () => { @@ -410,9 +413,12 @@ function makePromRule(overrides?: Partial<GrafanaPromRuleDTO>): GrafanaPromRuleD } describe('usePromRuleAdministrationAbility', () => { - it('returns all NOT_SUPPORTED when skipToken is passed', () => { + it('returns all NOT_SUPPORTED when skipToken is passed', async () => { const { result } = renderHook(() => usePromRuleAdministrationAbility(skipToken), { wrapper: wrapper() }); + // Drain the async queue from useRulePluginImmutability's useAsync call + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(isNotSupported(result.current.update)).toBe(true); expect(isNotSupported(result.current.delete)).toBe(true); expect(isNotSupported(result.current.deletePermanently)).toBe(true); diff --git a/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.utils.ts b/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.utils.ts index 9af38fd31cbc6..14df2228ab199 100644 --- a/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.utils.ts +++ b/public/app/features/alerting/unified/hooks/abilities/rules/ruleAbilities.utils.ts @@ -1,5 +1,6 @@ import { useMemo } from 'react'; +import { useAppPluginEnabled } from '@grafana/runtime'; import { contextSrv as ctx } from 'app/core/services/context_srv'; import { useFolder } from 'app/features/alerting/unified/hooks/useFolder'; import { AlertmanagerChoice } from 'app/plugins/datasource/alertmanager/types'; @@ -7,7 +8,6 @@ import { AccessControlAction } from 'app/types/accessControl'; import { type GrafanaPromRuleDTO, type RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { alertmanagerApi } from '../../../api/alertmanagerApi'; -import { useGetPluginSettingsQuery } from '../../../api/pluginsApi'; import { getRulesPermissions } from '../../../utils/access-control'; import { isAdmin } from '../../../utils/misc'; import { getRulePluginOrigin } from '../../../utils/rules'; @@ -171,10 +171,8 @@ export function useRulePluginImmutability(rule: RulerRuleDTO | GrafanaPromRuleDT pluginLoading: boolean; } { const pluginOrigin = getRulePluginOrigin(rule); - const { data: pluginSettings, isLoading } = useGetPluginSettingsQuery(pluginOrigin?.pluginId ?? '', { - skip: !pluginOrigin?.pluginId, - }); - const isPluginInstalled = pluginSettings?.enabled ?? false; + // Empty string returns false immediately without a network call + const { value: isPluginInstalled = false, loading: isLoading } = useAppPluginEnabled(pluginOrigin?.pluginId ?? ''); return useMemo( () => ({ diff --git a/public/app/features/alerting/unified/hooks/useAbilities.ts b/public/app/features/alerting/unified/hooks/useAbilities.ts index 75f2b8f87115f..b5478875cfad3 100644 --- a/public/app/features/alerting/unified/hooks/useAbilities.ts +++ b/public/app/features/alerting/unified/hooks/useAbilities.ts @@ -1,6 +1,6 @@ import { useMemo } from 'react'; -import { config } from '@grafana/runtime'; +import { config, useAppPluginEnabled } from '@grafana/runtime'; import { contextSrv as ctx } from 'app/core/services/context_srv'; import { PERMISSIONS_CONTACT_POINTS_READ } from 'app/features/alerting/unified/components/contact-points/permissions'; import { @@ -14,7 +14,6 @@ import { type CombinedRule, type RuleGroupIdentifierV2 } from 'app/types/unified import { type GrafanaPromRuleDTO, type RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { alertmanagerApi } from '../api/alertmanagerApi'; -import { useGetPluginSettingsQuery } from '../api/pluginsApi'; import { useAlertmanager } from '../state/AlertmanagerContext'; import { getInstancesPermissions, getNotificationsPermissions, getRulesP ... [truncated]