Information disclosure

Reproduction outline (requires an unpatched Grafana 12.x instance): 1) Create a dashboard scene that includes a system variable (ScopesVariable) alongside a user-defined variable (CustomVariable). 2) Edit a dashboard panel that uses conditional rendering and click to add a rule, selecting "Template variable". 3) In the variable dropdown, observe that system variables (e.g., a scopes variable) are present in the list and can be selected to form the condition. 4) If the condition references a system variable, the dashboard UI can reveal internal configuration/state associated with that variable (e.g., access to scopes or internal naming/values) through the rendered UI elements or condition previews. Expected behavior after the patch (Grafana 12.4.0+): the variable selection dropdown for conditional rendering lists only user-defined variables; system variables are filtered out, preventing exposure of internal state through the UI. Note: The PoC here is illustrative; exact UI rendering and data exposure depend on Grafana’s frontend rendering in your environment. The key proof-of-concept is that the vulnerable version surfaces system-variable options in the variable picker and could reveal internal state, whereas the fixed version hides them.

diff --git a/public/app/features/dashboard-scene/conditional-rendering/conditions/ConditionalRenderingVariable.tsx b/public/app/features/dashboard-scene/conditional-rendering/conditions/ConditionalRenderingVariable.tsx index da30a38c4db78..174b849096966 100644 --- a/public/app/features/dashboard-scene/conditional-rendering/conditions/ConditionalRenderingVariable.tsx +++ b/public/app/features/dashboard-scene/conditional-rendering/conditions/ConditionalRenderingVariable.tsx @@ -18,7 +18,7 @@ import { Box, Combobox, type ComboboxOption, Field, Input, Stack } from '@grafan import { ALL_VARIABLE_TEXT } from 'app/features/variables/constants'; import { dashboardEditActions } from '../../edit-pane/shared'; -import { getDashboardSceneFor } from '../../utils/utils'; +import { useUserDefinedVariables } from '../../utils/variables'; import { getLowerTranslatedObjectType } from '../object'; import { ConditionalRenderingConditionWrapper } from './ConditionalRenderingConditionWrapper'; @@ -228,11 +228,11 @@ function ConditionalRenderingVariableRenderer({ model }: SceneComponentProps<Con useEffect(() => setNewValue(value), [value]); - const variables = sceneGraph.getVariables(getDashboardSceneFor(model)); + const variables = useUserDefinedVariables(model); const variableNames: ComboboxOption[] = useMemo( - () => variables.state.variables.map((v) => ({ value: v.state.name, label: v.state.label ?? v.state.name })), - [variables.state.variables] + () => variables.map((v) => ({ value: v.state.name, label: v.state.label ?? v.state.name })), + [variables] ); const operatorOptions: Array<ComboboxOption<VariableConditionValueOperator>> = useMemo( diff --git a/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.test.tsx b/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.test.tsx new file mode 100644 index 0000000000000..a05fcba639b61 --- /dev/null +++ b/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.test.tsx @@ -0,0 +1,155 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { comboboxTestSetup } from 'test/helpers/comboboxTestSetup'; + +import { selectors } from '@grafana/e2e-selectors'; +import { CustomVariable, SceneVariableSet, ScopesVariable, type SceneVariable } from '@grafana/scenes'; + +import { DashboardScene } from '../../scene/DashboardScene'; +import { AutoGridLayoutManager } from '../../scene/layout-auto-grid/AutoGridLayoutManager'; +import { RowItem } from '../../scene/layout-rows/RowItem'; +import { RowsLayoutManager } from '../../scene/layout-rows/RowsLayoutManager'; +import { ConditionalRenderingVariable } from '../conditions/ConditionalRenderingVariable'; + +import { ConditionalRenderingGroup } from './ConditionalRenderingGroup'; + +function buildScene(variables: SceneVariable[]) { + const model = ConditionalRenderingGroup.createEmpty(); + + const row = new RowItem({ + conditionalRendering: model, + layout: AutoGridLayoutManager.createEmpty(), + }); + + new DashboardScene({ + $variables: new SceneVariableSet({ variables }), + body: new RowsLayoutManager({ rows: [row] }), + }); + + return model; +} + +function buildSceneWithCondition(variables: SceneVariable[], condition: ConditionalRenderingVariable) { + const model = new ConditionalRenderingGroup({ + condition: 'and', + visibility: 'show', + conditions: [condition], + result: true, + renderHidden: false, + }); + + const row = new RowItem({ + conditionalRendering: model, + layout: AutoGridLayoutManager.createEmpty(), + }); + + new DashboardScene({ + $variables: new SceneVariableSet({ variables }), + body: new RowsLayoutManager({ rows: [row] }), + }); + + return model; +} + +describe('ConditionalRenderingGroupRenderer', () => { + describe('variables list filtering', () => { + it('does not create a variable condition when only system variables are present', async () => { + const user = userEvent.setup(); + const model = buildScene([new ScopesVariable({ enable: true })]); + const createConditionSpy = jest.spyOn(model, 'createCondition'); + + render(<ConditionalRenderingGroup.Component model={model} />); + + await user.click(screen.getByRole('button', { name: /add rule/i })); + await user.click(await screen.findByText('Template variable')); + + expect(createConditionSpy).not.toHaveBeenCalled(); + }); + + it('creates a variable condition when a user-defined variable is present', async () => { + const user = userEvent.setup(); + const model = buildScene([new CustomVariable({ name: 'myVar', query: 'a,b' })]); + const createConditionSpy = jest.spyOn(model, 'createCondition'); + + render(<ConditionalRenderingGroup.Component model={model} />); + + await user.click(screen.getByRole('button', { name: /add rule/i })); + await user.click(await screen.findByText('Template variable')); + + expect(createConditionSpy).toHaveBeenCalledWith('variable'); + }); + + it('creates a variable condition when both system and user-defined variables are present', async () => { + const user = userEvent.setup(); + const model = buildScene([ + new ScopesVariable({ enable: true }), + new CustomVariable({ name: 'myVar', query: 'a,b' }), + ]); + const createConditionSpy = jest.spyOn(model, 'createCondition'); + + render(<ConditionalRenderingGroup.Component model={model} />); + + await user.click(screen.getByRole('button', { name: /add rule/i })); + await user.click(await screen.findByText('Template variable')); + + expect(createConditionSpy).toHaveBeenCalledWith('variable'); + }); + }); + + describe('createCondition variable default', () => { + it('uses the first user-defined variable as the default variable name', () => { + const model = buildScene([new CustomVariable({ name: 'myVar', query: 'a,b' })]); + + const condition = model.createCondition('variable'); + + expect(condition).toBeInstanceOf(ConditionalRenderingVariable); + expect((condition as ConditionalRenderingVariable).state.variable).toBe('myVar'); + }); + + it('skips system variables (ScopesVariable) when choosing the default variable name', () => { + const model = buildScene([ + new ScopesVariable({ enable: true }), + new CustomVariable({ name: 'userVar', query: 'a,b' }), + ]); + + const condition = model.createCondition('variable'); + + expect(condition).toBeInstanceOf(ConditionalRenderingVariable); + expect((condition as ConditionalRenderingVariable).state.variable).toBe('userVar'); + }); + + it('defaults to an empty variable name when only system variables are present', () => { + const model = buildScene([new ScopesVariable({ enable: true })]); + + const condition = model.createCondition('variable'); + + expect(condition).toBeInstanceOf(ConditionalRenderingVariable); + expect((condition as ConditionalRenderingVariable).state.variable).toBe(''); + }); + }); + + describe('variable name options in the variable condition Combobox', () => { + beforeAll(() => { + comboboxTestSetup(); + }); + + it('lists only user-defined variables — not system variables — as selectable options', async () => { + const user = userEvent.setup({ applyAccept: false }); + + const condition = ConditionalRenderingVariable.createEmpty('myVar'); + const model = buildSceneWithCondition( + [new ScopesVariable({ name: '__scopes', enable: true }), new CustomVariable({ name: 'myVar', query: 'a,b' })], + condition + ); + + render(<ConditionalRenderingGroup.Component model={model} />); + + await user.click( + screen.getByTestId(selectors.pages.Dashboard.Sidebar.conditionalRendering.variable.variableSelection) + ); + + expect(await screen.findByRole('option', { name: 'myVar' })).toBeInTheDocument(); + expect(screen.queryByRole('option', { name: '__scopes' })).not.toBeInTheDocument(); + }); + }); +}); diff --git a/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.tsx b/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.tsx index 32132148c218f..96ade865866c1 100644 --- a/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.tsx +++ b/public/app/features/dashboard-scene/conditional-rendering/group/ConditionalRenderingGroup.tsx @@ -4,7 +4,6 @@ import { useMemo } from 'react'; import { t } from '@grafana/i18n'; import { type SceneComponentProps, - sceneGraph, type SceneObject, SceneObjectBase, type SceneObjectRef, @@ -14,7 +13,7 @@ import { type ConditionalRenderingGroupKind } from '@grafana/schema/apis/dashboa import { Stack } from '@grafana/ui'; import { ConditionalRenderingChangedEvent, dashboardEditActions } from '../../edit-pane/shared'; -import { getDashboardSceneFor } from '../../utils/utils'; +import { getUserDefinedVariables, useUserDefinedVariables } from '../../utils/variables'; import { ConditionalRenderingData } from '../conditions/ConditionalRenderingData'; import { ConditionalRenderingTimeRangeSize } from '../conditions/ConditionalRenderingTimeRangeSize'; import { ConditionalRenderingVariable } from '../conditions/ConditionalRenderingVariable'; @@ -121,9 +120,10 @@ export class ConditionalRenderingGroup extends SceneObjectBase<ConditionalRender return ConditionalRenderingTimeRangeSize.createEmpty(); case 'variable': - return ConditionalRenderingVariable.createEmpty( - sceneGraph.getVariables(getDashboardSceneFor(this)).state.variables[0].state.name - ); + const variables = getUserDefinedVariables(this); + // The code should not be hit when variables.length === 0 because we grey out the form if there are no variables + // but was added to avoid potential runtime errors if the UX changes and the section is not disabled. + return ConditionalRenderingVariable.createEmpty(variables.length ? variables[0].state.name : ''); } } @@ -207,7 +207,8 @@ export class ConditionalRenderingGroup extends SceneObjectBase<ConditionalRender function ConditionalRenderingGroupRenderer({ model }: SceneComponentProps<ConditionalRenderingGroup>) { const { condition, visibility, conditions } = model.useState(); - const { variables } = sceneGraph.getVariables(model).useState(); + const variables = useUserDefinedVariables(model); + const objectType = useMemo(() => extractObjectType(model.parent), [model]); return ( diff --git a/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.test.ts b/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.test.ts index b762847904c66..e479b1db0681f 100644 --- a/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.test.ts +++ b/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.test.ts @@ -1,4 +1,4 @@ -import { CustomVariable, SceneGridLayout, SceneVariableSet, VizPanel } from '@grafana/scenes'; +import { CustomVariable, SceneGridLayout, SceneVariableSet, ScopesVariable, VizPanel } from '@grafana/scenes'; import { DashboardScene } from '../scene/DashboardScene'; import { DashboardGridItem } from '../scene/layout-default/DashboardGridItem'; @@ -62,6 +62,17 @@ describe('collectAncestorSceneVariables', () => { expect(dupInstance).toBe(sectionDup); }); + it('excludes system variables (keepOnlyUserDefinedVariables)', () => { + const userVar = new CustomVariable({ name: 'userVar', query: 'x', value: 'x', text: 'x' }); + const scopesVar = new ScopesVariable({ enable: true }); + const dashboard = new DashboardScene({ + $variables: new SceneVariableSet({ variables: [userVar, scopesVar] }), + }); + + const merged = collectAncestorSceneVariables(dashboard); + expect(merged.map((v) => v.state.name)).toEqual(['userVar']); + }); + it('excludes the starting row section variables (walk starts at parent when present)', () => { const dashVar = new CustomVariable({ name: 'dashVar', query: 'd', value: 'd', text: 'd' }); const sectionVar = new CustomVariable({ name: 'sectionVar', query: 's', value: 's', text: 's' }); diff --git a/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.ts b/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.ts index 655296a1f0ac5..e3efefd875f28 100644 --- a/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.ts +++ b/public/app/features/dashboard-scene/utils/collectAncestorSceneVariables.ts @@ -1,5 +1,7 @@ import { type SceneObject, type SceneVariable, SceneVariableSet } from '@grafana/scenes'; +import { keepOnlyUserDefinedVariables } from './variables'; + /** * Resolves variables visible when editing from `sceneObject` (e.g. repeat options). * @@ -15,7 +17,8 @@ export function collectAncestorSceneVariables(sceneObject: SceneObject): SceneVa while (current) { if (current.state.$variables instanceof SceneVariableSet) { - for (const variable of current.state.$variables.state.variables) { + const variables = current.state.$variables.state.variables.filter(keepOnlyUserDefinedVariables); + for (const variable of variables) { const name = variable.state.name; if (!seenNames.has(name)) { seenNames.add(name); diff --git a/public/app/features/dashboard-scene/utils/variables.test.ts b/public/app/features/dashboard-scene/utils/variables.test.ts index 1e4b0528b9846..da0ea37b932a2 100644 --- a/public/app/features/dashboard-scene/utils/variables.test.ts +++ b/public/app/features/dashboard-scene/utils/variables.test.ts @@ -15,10 +15,14 @@ import { AdHocFiltersVariable, CustomVariable, DataSourceVariable, + EmbeddedScene, GroupByVariable, QueryVariable, + SceneFlexLayout, SceneVariableSet, + ScopesVariable, SwitchVariable, + TestVariable, } from '@grafana/scenes'; import { defaultDashboard, defaultTimePickerConfig, type VariableType } from '@grafana/schema'; import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; @@ -26,7 +30,7 @@ import { DashboardModel } from 'app/features/dashboard/state/DashboardModel'; import { SnapshotVariable } from '../serialization/custom-variables/SnapshotVariable'; import { NEW_LINK } from '../settings/links/utils'; -import { createSceneVariableFromVariableModel, createVariablesForSnapshot } from './variables'; +import { createSceneVariableFromVariableModel, createVariablesForSnapshot, getUserDefinedVariables } from './variables'; // mock getDataSourceSrv.getInstanceSettings() jest.mock('@grafana/runtime', () => ({ @@ -1023,3 +1027,19 @@ describe('when creating snapshot variables from dashboard model', ( ... [truncated]