Alerting: Encode path separators to side-step proxies (#58141)

public/app/features/alerting/unified/utils/rule-id.test.ts

@@ -6,7 +6,7 @@ import {
   RulerRecordingRuleDTO,
 } from 'app/types/unified-alerting-dto';
 
-import { hashRulerRule } from './rule-id';
+import { hashRulerRule, parse, stringifyIdentifier } from './rule-id';
 
 describe('hashRulerRule', () => {
   it('should not hash unknown rule types', () => {
@@ -59,4 +59,52 @@ describe('hashRulerRule', () => {
 
     expect(hashRulerRule(grafanaRule)).toBe(RULE_UID);
   });
+
+  it('should correctly encode and decode unix-style path separators', () => {
+    const identifier = {
+      ruleSourceName: 'my-datasource',
+      namespace: 'folder1/folder2',
+      groupName: 'group1/group2',
+      ruleHash: 'abc123',
+    };
+
+    const encodedIdentifier = encodeURIComponent(stringifyIdentifier(identifier));
+
+    expect(encodedIdentifier).toBe('pri%24my-datasource%24folder1%1Ffolder2%24group1%1Fgroup2%24abc123');
+    expect(encodedIdentifier).not.toContain('%2F');
+    expect(parse(encodedIdentifier, true)).toStrictEqual(identifier);
+  });
+
+  it('should correctly decode regular encoded path separators (%2F)', () => {
+    const identifier = {
+      ruleSourceName: 'my-datasource',
+      namespace: 'folder1/folder2',
+      groupName: 'group1/group2',
+      ruleHash: 'abc123',
+    };
+
+    expect(parse('pri%24my-datasource%24folder1%2Ffolder2%24group1%2Fgroup2%24abc123', true)).toStrictEqual(identifier);
+  });
+
+  it('should correctly encode and decode windows-style path separators', () => {
+    const identifier = {
+      ruleSourceName: 'my-datasource',
+      namespace: 'folder1\\folder2',
+      groupName: 'group1\\group2',
+      ruleHash: 'abc123',
+    };
+
+    const encodedIdentifier = encodeURIComponent(stringifyIdentifier(identifier));
+
+    expect(encodedIdentifier).toBe('pri%24my-datasource%24folder1%1Efolder2%24group1%1Egroup2%24abc123');
+    expect(parse(encodedIdentifier, true)).toStrictEqual(identifier);
+  });
+
+  it('should correctly decode a Grafana managed rule id', () => {
+    expect(parse('abc123', false)).toStrictEqual({ uid: 'abc123', ruleSourceName: 'grafana' });
+  });
+
+  it('should throw for malformed identifier', () => {
+    expect(() => parse('foo$bar$baz', false)).toThrow(/failed to parse/i);
+  });
 });

public/app/features/alerting/unified/utils/rule-id.ts

@@ -91,10 +91,25 @@ function escapeDollars(value: string): string {
   return value.replace(/\$/g, '_DOLLAR_');
 }
 
-function unesacapeDollars(value: string): string {
+function unescapeDollars(value: string): string {
   return value.replace(/\_DOLLAR\_/g, '$');
 }
 
+/**
+ * deal with Unix-style path separators "/" (replaced with \x1f – unit separator)
+ * and Windows-style path separators "\" (replaced with \x1e – record separator)
+ * we need this to side-step proxies that automatically decode %2F to prevent path traversal attacks
+ * we'll use some non-printable characters from the ASCII table that will get encoded properly but very unlikely
+ * to ever be used in a rule name or namespace
+ */
+function escapePathSeparators(value: string): string {
+  return value.replace(/\//g, '\x1f').replace(/\\/g, '\x1e');
+}
+
+function unescapePathSeparators(value: string): string {
+  return value.replace(/\x1f/g, '/').replace(/\x1e/g, '\\');
+}
+
 export function parse(value: string, decodeFromUri = false): RuleIdentifier {
   const source = decodeFromUri ? decodeURIComponent(value) : value;
   const parts = source.split('$');
@@ -104,7 +119,7 @@ export function parse(value: string, decodeFromUri = false): RuleIdentifier {
   }
 
   if (parts.length === 5) {
-    const [prefix, ruleSourceName, namespace, groupName, hash] = parts.map(unesacapeDollars);
+    const [prefix, ruleSourceName, namespace, groupName, hash] = parts.map(unescapeDollars).map(unescapePathSeparators);
 
     if (prefix === cloudRuleIdentifierPrefix) {
       return { ruleSourceName, namespace, groupName, rulerRuleHash: hash };
@@ -145,6 +160,7 @@ export function stringifyIdentifier(identifier: RuleIdentifier): string {
     ]
       .map(String)
       .map(escapeDollars)
+      .map(escapePathSeparators)
       .join('$');
   }
 
@@ -157,6 +173,7 @@ export function stringifyIdentifier(identifier: RuleIdentifier): string {
   ]
     .map(String)
     .map(escapeDollars)
+    .map(escapePathSeparators)
     .join('$');
 }