diff --git a/public/app/features/alerting/unified/api/ruler.ts b/public/app/features/alerting/unified/api/ruler.ts index 331d45154d0..df265d53bad 100644 --- a/public/app/features/alerting/unified/api/ruler.ts +++ b/public/app/features/alerting/unified/api/ruler.ts @@ -5,7 +5,7 @@ import { FetchResponse, getBackendSrv } from '@grafana/runtime'; import { RulerDataSourceConfig } from 'app/types/unified-alerting'; import { PostableRulerRuleGroupDTO, RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; -import { checkForPathSeparator } from '../components/rule-editor/util'; +import { containsPathSeparator } from '../components/rule-editor/util'; import { RULER_NOT_SUPPORTED_MSG } from '../utils/constants'; import { getDatasourceAPIUid, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; @@ -73,11 +73,15 @@ interface RulerQueryDetailsProvider { group: (group: string) => GroupUrlParams; } +// some gateways (like Istio) will decode "/" and "\" characters – this will cause 404 errors for any API call +// that includes these values in the URL (ie. /my/path%2fto/resource -> /my/path/to/resource) +// +// see https://istio.io/latest/docs/ops/best-practices/security/#customize-your-system-on-path-normalization function getQueryDetailsProvider(rulerConfig: RulerDataSourceConfig): RulerQueryDetailsProvider { const isGrafanaDatasource = rulerConfig.dataSourceName === GRAFANA_RULES_SOURCE_NAME; const groupParamRewrite = (group: string): GroupUrlParams => { - if (checkForPathSeparator(group) !== true) { + if (containsPathSeparator(group) === true) { return { group: QUERY_GROUP_TAG, searchParams: { group } }; } return { group, searchParams: {} }; @@ -93,7 +97,7 @@ function getQueryDetailsProvider(rulerConfig: RulerDataSourceConfig): RulerQuery return { namespace: (namespace: string): NamespaceUrlParams => { - if (checkForPathSeparator(namespace) !== true) { + if (containsPathSeparator(namespace) === true) { return { namespace: QUERY_NAMESPACE_TAG, searchParams: { namespace } }; } return { namespace, searchParams: {} }; diff --git a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx index a10eb9f7f19..c8e84b38183 100644 --- a/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx @@ -23,7 +23,6 @@ import { evaluateEveryValidationOptions } from '../rules/EditRuleGroupModal'; import { EvaluationGroupQuickPick } from './EvaluationGroupQuickPick'; import { containsSlashes, Folder, RuleFolderPicker } from './RuleFolderPicker'; -import { checkForPathSeparator } from './util'; export const MAX_GROUP_RESULTS = 1000; @@ -175,9 +174,6 @@ export function FolderAndGroup({ name="folder" rules={{ required: { value: true, message: 'Select a folder' }, - validate: { - pathSeparator: (folder: Folder) => checkForPathSeparator(folder.uid), - }, }} /> or @@ -251,9 +247,6 @@ export function FolderAndGroup({ control={control} rules={{ required: { value: true, message: 'Must enter a group name' }, - validate: { - pathSeparator: (group_: string) => checkForPathSeparator(group_), - }, }} /> diff --git a/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx b/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx index 541a7b6780a..eb3532f954a 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx @@ -10,8 +10,6 @@ import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelect import { fetchRulerRulesAction } from '../../state/actions'; import { RuleFormValues } from '../../types/rule-form'; -import { checkForPathSeparator } from './util'; - interface Props { rulesSourceName: string; } @@ -74,9 +72,6 @@ export const GroupAndNamespaceFields = ({ rulesSourceName }: Props) => { control={control} rules={{ required: { value: true, message: 'Required.' }, - validate: { - pathSeparator: checkForPathSeparator, - }, }} /> @@ -98,9 +93,6 @@ export const GroupAndNamespaceFields = ({ rulesSourceName }: Props) => { control={control} rules={{ required: { value: true, message: 'Required.' }, - validate: { - pathSeparator: checkForPathSeparator, - }, }} /> diff --git a/public/app/features/alerting/unified/components/rule-editor/util.test.ts b/public/app/features/alerting/unified/components/rule-editor/util.test.ts index ec51a60f31e..b5ffc31189c 100644 --- a/public/app/features/alerting/unified/components/rule-editor/util.test.ts +++ b/public/app/features/alerting/unified/components/rule-editor/util.test.ts @@ -3,7 +3,7 @@ import { ClassicCondition, ExpressionQuery } from 'app/features/expressions/type import { AlertQuery } from 'app/types/unified-alerting-dto'; import { - checkForPathSeparator, + containsPathSeparator, findRenamedDataQueryReferences, getThresholdsForQueries, queriesWithUpdatedReferences, @@ -229,17 +229,15 @@ describe('rule-editor', () => { }); }); -describe('checkForPathSeparator', () => { - it('should not allow strings with /', () => { - expect(checkForPathSeparator('foo / bar')).not.toBe(true); - expect(typeof checkForPathSeparator('foo / bar')).toBe('string'); +describe('containsPathSeparator', () => { + it('should return true for strings with /', () => { + expect(containsPathSeparator('foo / bar')).toBe(true); }); - it('should not allow strings with \\', () => { - expect(checkForPathSeparator('foo \\ bar')).not.toBe(true); - expect(typeof checkForPathSeparator('foo \\ bar')).toBe('string'); + it('should return true for strings with \\', () => { + expect(containsPathSeparator('foo \\ bar')).toBe(true); }); - it('should allow anything without / or \\', () => { - expect(checkForPathSeparator('foo bar')).toBe(true); + it('should return false for strings without / or \\', () => { + expect(containsPathSeparator('foo !@#$%^&*() <> [] {} bar')).toBe(false); }); }); diff --git a/public/app/features/alerting/unified/components/rule-editor/util.ts b/public/app/features/alerting/unified/components/rule-editor/util.ts index 7c09766a2db..716c9f2fbbc 100644 --- a/public/app/features/alerting/unified/components/rule-editor/util.ts +++ b/public/app/features/alerting/unified/components/rule-editor/util.ts @@ -1,5 +1,4 @@ import { xor } from 'lodash'; -import { ValidateResult } from 'react-hook-form'; import { DataFrame, @@ -89,17 +88,8 @@ export function refIdExists(queries: AlertQuery[], refId: string | null): boolea return queries.find((query) => query.refId === refId) !== undefined; } -// some gateways (like Istio) will decode "/" and "\" characters – this will cause 404 errors for any API call -// that includes these values in the URL (ie. /my/path%2fto/resource -> /my/path/to/resource) -// -// see https://istio.io/latest/docs/ops/best-practices/security/#customize-your-system-on-path-normalization -export function checkForPathSeparator(value: string): ValidateResult { - const containsPathSeparator = value.includes('/') || value.includes('\\'); - if (containsPathSeparator) { - return 'Cannot contain "/" or "\\" characters'; - } - - return true; +export function containsPathSeparator(value: string): boolean { + return value.includes('/') || value.includes('\\'); } // this function assumes we've already checked if the data passed in to the function is of the alert condition diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx index 7569cb33d18..65c65e07685 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, userEvent } from 'test/test-utils'; +import { render } from 'test/test-utils'; import { byLabelText, byTestId, byText, byTitle } from 'testing-library-selector'; import { CombinedRuleNamespace } from 'app/types/unified-alerting'; @@ -158,12 +158,4 @@ describe('EditGroupModal component on grafana-managed alert rules', () => { expect(await ui.input.namespace.find()).toHaveValue('namespace1'); expect(ui.folderLink.query()).not.toBeInTheDocument(); }); - - it('does not allow slashes in the group name', async () => { - const user = userEvent.setup(); - renderWithGrafanaGroup(); - await user.type(await ui.input.group.find(), 'group/with/slashes'); - await user.click(ui.input.interval.get()); - expect(await screen.findByText(/cannot contain \"\/\"/i)).toBeInTheDocument(); - }); }); diff --git a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx index 883fd11c733..2ff9d57baaf 100644 --- a/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx +++ b/public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx @@ -28,7 +28,6 @@ import { EvaluationIntervalLimitExceeded } from '../InvalidIntervalWarning'; import { decodeGrafanaNamespace, encodeGrafanaNamespace } from '../expressions/util'; import { EvaluationGroupQuickPick } from '../rule-editor/EvaluationGroupQuickPick'; import { MIN_TIME_RANGE_STEP_S } from '../rule-editor/GrafanaEvaluationBehavior'; -import { checkForPathSeparator } from '../rule-editor/util'; const ITEMS_PER_PAGE = 10; @@ -300,11 +299,6 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { readOnly={intervalEditOnly || isGrafanaManagedGroup} {...register('namespaceName', { required: 'Namespace name is required.', - validate: { - // for Grafana-managed we do not validate the name of the folder because we use the UID anyway - pathSeparator: (namespaceName) => - isGrafanaManagedGroup ? true : checkForPathSeparator(namespaceName), - }, })} /> @@ -337,9 +331,6 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement { readOnly={intervalEditOnly} {...register('groupName', { required: 'Evaluation group name is required.', - validate: { - pathSeparator: (namespace) => checkForPathSeparator(namespace), - }, })} />