From 1680e284e5cc3f7e0a7ff747cbc007f966fce830 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Fri, 4 Feb 2022 15:04:11 +0100 Subject: [PATCH] Alerting: validate namespace and groupname for Grafana manages and Lotex alerts (#44872) --- .../components/rule-editor/AlertTypeStep.tsx | 16 +++++++++++++++- .../rule-editor/GroupAndNamespaceFields.tsx | 7 +++++++ .../unified/components/rule-editor/util.test.ts | 16 +++++++++++++++- .../unified/components/rule-editor/util.ts | 14 ++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx index e2065b510aa..4a96553d037 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertTypeStep.tsx @@ -5,10 +5,11 @@ import { css } from '@emotion/css'; import { RuleEditorSection } from './RuleEditorSection'; import { useFormContext } from 'react-hook-form'; import { RuleFormType, RuleFormValues } from '../../types/rule-form'; -import { RuleFolderPicker } from './RuleFolderPicker'; +import { Folder, RuleFolderPicker } from './RuleFolderPicker'; import { GroupAndNamespaceFields } from './GroupAndNamespaceFields'; import { contextSrv } from 'app/core/services/context_srv'; import { CloudRulesSourcePicker } from './CloudRulesSourcePicker'; +import { checkForPathSeparator } from './util'; interface Props { editingExistingRule: boolean; @@ -72,6 +73,16 @@ export const AlertTypeStep: FC = ({ editingExistingRule }) => { {...register('name', { required: { value: true, message: 'Must enter an alert name' }, pattern: ruleFormType === RuleFormType.cloudRecording ? recordingRuleNameValidationPattern : undefined, + validate: { + pathSeparator: (value: string) => { + // we use the alert rule name as the "groupname" for Grafana managed alerts, so we can't allow path separators + if (ruleFormType === RuleFormType.grafana) { + return checkForPathSeparator(value); + } + + return true; + }, + }, })} autoFocus={true} /> @@ -148,6 +159,9 @@ export const AlertTypeStep: FC = ({ editingExistingRule }) => { name="folder" rules={{ required: { value: true, message: 'Please select a folder' }, + validate: { + pathSeparator: (folder: Folder) => checkForPathSeparator(folder.title), + }, }} /> 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 a4b5022e6b9..16cb1fa0ac4 100644 --- a/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/GroupAndNamespaceFields.tsx @@ -8,6 +8,7 @@ import { GrafanaTheme2, SelectableValue } from '@grafana/data'; import { SelectWithAdd } from './SelectWIthAdd'; import { Field, InputControl, useStyles2 } from '@grafana/ui'; import { css } from '@emotion/css'; +import { checkForPathSeparator } from './util'; interface Props { rulesSourceName: string; @@ -75,6 +76,9 @@ export const GroupAndNamespaceFields: FC = ({ rulesSourceName }) => { control={control} rules={{ required: { value: true, message: 'Required.' }, + validate: { + pathSeparator: checkForPathSeparator, + }, }} /> @@ -87,6 +91,9 @@ export const GroupAndNamespaceFields: FC = ({ rulesSourceName }) => { 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 7b62497f9c8..33643e09b65 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 @@ -1,6 +1,6 @@ import { ClassicCondition, ExpressionQuery } from 'app/features/expressions/types'; import { AlertQuery } from 'app/types/unified-alerting-dto'; -import { queriesWithUpdatedReferences, updateMathExpressionRefs } from './util'; +import { checkForPathSeparator, queriesWithUpdatedReferences, updateMathExpressionRefs } from './util'; import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend'; describe('rule-editor', () => { @@ -191,3 +191,17 @@ 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'); + }); + it('should not allow strings with \\', () => { + expect(checkForPathSeparator('foo \\ bar')).not.toBe(true); + expect(typeof checkForPathSeparator('foo \\ bar')).toBe('string'); + }); + it('should allow anything without / or \\', () => { + expect(checkForPathSeparator('foo bar')).toBe(true); + }); +}); 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 04e488b2d1f..a6921c956f6 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,6 @@ import { isExpressionQuery } from 'app/features/expressions/guards'; import { AlertQuery } from 'app/types/unified-alerting-dto'; +import { ValidateResult } from 'react-hook-form'; export function queriesWithUpdatedReferences( queries: AlertQuery[], @@ -64,3 +65,16 @@ export function updateMathExpressionRefs(expression: string, previousRefId: stri return expression.replace(oldExpression, newExpression); } + +// 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; +}