diff --git a/packages/grafana-data/src/index.ts b/packages/grafana-data/src/index.ts index 28cb92c47d3..55380d4a746 100644 --- a/packages/grafana-data/src/index.ts +++ b/packages/grafana-data/src/index.ts @@ -98,7 +98,7 @@ export { } from './text/string'; export { type TextMatch, findHighlightChunksInText, findMatchesInText, parseFlags } from './text/text'; export { type RenderMarkdownOptions, renderMarkdown, renderTextPanelMarkdown } from './text/markdown'; -export { textUtil } from './text/sanitize'; +export { textUtil, validatePath, PathValidationError } from './text/sanitize'; // Events export { eventFactory } from './events/eventFactory'; diff --git a/packages/grafana-data/src/text/sanitize.test.ts b/packages/grafana-data/src/text/sanitize.test.ts index 37d616055a7..218e4064046 100644 --- a/packages/grafana-data/src/text/sanitize.test.ts +++ b/packages/grafana-data/src/text/sanitize.test.ts @@ -1,4 +1,4 @@ -import { sanitizeTextPanelContent, sanitizeUrl, sanitize } from './sanitize'; +import { sanitizeTextPanelContent, sanitizeUrl, sanitize, validatePath, PathValidationError } from './sanitize'; describe('sanitizeTextPanelContent', () => { it('should allow whitelisted styles in text panel', () => { @@ -56,3 +56,97 @@ describe('sanitize', () => { expect(str).toBe(''); }); }); + +describe('validatePath', () => { + describe('path traversal protection', () => { + it('should block simple path traversal attempts', () => { + expect(() => validatePath('/api/../admin')).toThrow(PathValidationError); + expect(() => validatePath('api/../admin')).toThrow(PathValidationError); + expect(() => validatePath('../admin')).toThrow(PathValidationError); + }); + + it('should block URL encoded path traversal attempts', () => { + expect(() => validatePath('/api/%2e%2e/admin')).toThrow(PathValidationError); + expect(() => validatePath('/api/%252e%252e/admin')).toThrow(PathValidationError); + }); + + it('should block double encoded traversal attempts', () => { + expect(() => validatePath('/api/%252e%252e/admin')).toThrow(PathValidationError); + }); + + it('should handle malformed URI encoding gracefully', () => { + expect(() => validatePath('/api/%/admin')).toThrow(PathValidationError); + expect(() => validatePath('/api/%2/admin')).toThrow(PathValidationError); + }); + }); + + describe('safe paths', () => { + it('should preserve safe paths', () => { + expect(validatePath('/api/users/123')).toBe('/api/users/123'); + expect(validatePath('/api/dashboard/save')).toBe('/api/dashboard/save'); + expect(validatePath('api/config')).toBe('api/config'); + }); + + it('should preserve paths with file extensions', () => { + expect(validatePath('/api/file.json')).toBe('/api/file.json'); + expect(validatePath('/static/image.png')).toBe('/static/image.png'); + }); + + it('should preserve paths with query parameters', () => { + expect(validatePath('/api/search?q=test')).toBe('/api/search?q=test'); + expect(validatePath('/api/file.json?version=1.2.3&format=compact')).toBe( + '/api/file.json?version=1.2.3&format=compact' + ); + }); + + it('should handle empty and root paths', () => { + expect(validatePath('')).toBe(''); + expect(validatePath('/')).toBe('/'); + }); + }); + + describe('full URL handling', () => { + it('should preserve safe full URLs', () => { + const safeUrl = 'https://api.example.com/users/123'; + expect(validatePath(safeUrl)).toBe(safeUrl); + }); + + it('should preserve URLs with query parameters and fragments', () => { + const urlWithQuery = 'https://api.example.com/search?q=test&limit=10#results'; + expect(validatePath(urlWithQuery)).toBe(urlWithQuery); + }); + + it('should block traversal in URL paths while preserving query params', () => { + expect(() => validatePath('https://api.example.com/api/../admin?token=abc')).toThrow(PathValidationError); + expect(() => validatePath('http://localhost:3000/api/%2e%2e/secrets?param=value')).toThrow(PathValidationError); + }); + + it('should allow legitimate dots in URL paths with query params', () => { + const urlWithDots = 'https://cdn.example.com/files/document.v1.2.pdf?download=true'; + expect(validatePath(urlWithDots)).toBe(urlWithDots); + }); + + it('should allow query parameters that contain dots', () => { + const urlWithDotsInQuery = 'https://api.example.com/search?version=1.2.3&file=../config'; + expect(validatePath(urlWithDotsInQuery)).toBe(urlWithDotsInQuery); + }); + + it('should handle malformed URLs gracefully', () => { + expect(() => validatePath('not-a-url://../admin')).toThrow(PathValidationError); + expect(validatePath('://malformed')).toBe('://malformed'); // No traversal attempt, so it's allowed + }); + + it('should handle URLs with different protocols', () => { + expect(validatePath('ftp://files.example.com/safe/path')).toBe('ftp://files.example.com/safe/path'); + expect(() => validatePath('ftp://files.example.com/../secrets')).toThrow(PathValidationError); + }); + + it('should handle URLs with backslashes', () => { + expect(() => validatePath('https://api.example.com/\\example.com')).toThrow(PathValidationError); + }); + + it('should handle URLs with backslashes and dots in the path', () => { + expect(() => validatePath('https://api.example.com/\\..\\/admin')).toThrow(PathValidationError); + }); + }); +}); diff --git a/packages/grafana-data/src/text/sanitize.ts b/packages/grafana-data/src/text/sanitize.ts index c2be3a917ee..8fed4311627 100644 --- a/packages/grafana-data/src/text/sanitize.ts +++ b/packages/grafana-data/src/text/sanitize.ts @@ -120,6 +120,54 @@ export function escapeHtml(str: string): string { .replace(/"/g, '"'); } +export class PathValidationError extends Error { + constructor(message = 'Invalid request path') { + super(message); + this.name = 'PathValidationError'; + // Maintains proper stack trace for where error was thrown (only available on V8) + if (Error.captureStackTrace) { + Error.captureStackTrace(this, PathValidationError); + } + } +} + +/** + * Validates a path or URL, protecting against path traversal attacks. + * Returns the original input if safe, or throw an error + */ +export function validatePath(path: OriginalPath): OriginalPath { + try { + let originalDecoded: string = path; // down-cast to a string to indicate this can't be returned + while (true) { + const nextDecode = decodeURIComponent(originalDecoded); + if (nextDecode === originalDecoded) { + break; // String is fully decoded. + } + originalDecoded = nextDecode; + } + + // Remove query params and fragments to check only the path portion + const cleaned = originalDecoded.split(/[\?&#]/)[0]; + originalDecoded = cleaned; + + // If the original string contains traversal attempts, block it + if (originalDecoded.includes('..') || originalDecoded.includes('/\\')) { + throw new PathValidationError(); + } + + return path; + } catch (err) { + // Rethrow the original InvalidPathError to preserve the stack trace + if (err instanceof PathValidationError) { + throw err; + } + + // A decoding error can happen with malformed URIs (e.g., % not followed by hex). + // These are suspicious, so we treat them as traversal attempts. + throw new PathValidationError('Error validating request path'); + } +} + export const textUtil = { escapeHtml, hasAnsiCodes, diff --git a/packages/grafana-runtime/src/services/backendSrv.ts b/packages/grafana-runtime/src/services/backendSrv.ts index 14352c6aabb..79447166773 100644 --- a/packages/grafana-runtime/src/services/backendSrv.ts +++ b/packages/grafana-runtime/src/services/backendSrv.ts @@ -85,6 +85,12 @@ export type BackendSrvRequest = { * @deprecated withCredentials is deprecated in favor of credentials */ withCredentials?: boolean; + + /** + * Set to true to validate the URL path to prevent path traversal attacks. + * Use this when constructing URLs from user input. + */ + validatePath?: boolean; }; /** diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 57c6dc9d862..ac0d13d1051 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -147,7 +147,14 @@ export class BackendSrv implements BackendService { chunked(options: BackendSrvRequest): Observable> { const requestId = options.requestId ?? `chunked-${this.chunkRequestId++}`; const controller = new AbortController(); - const url = parseUrlFromOptions(options); + + let url: string; + try { + url = parseUrlFromOptions(options); + } catch (error) { + return throwError(() => error); + } + const init = parseInitFromOptions({ ...options, requestId, @@ -295,9 +302,15 @@ export class BackendSrv implements BackendService { } private getFromFetchStream(options: BackendSrvRequest): Observable> { - const url = parseUrlFromOptions(options); const init = parseInitFromOptions(options); + let url: string; + try { + url = parseUrlFromOptions(options); + } catch (error) { + return throwError(() => error); + } + return this.dependencies.fromFetch(url, init).pipe( mergeMap(async (response) => { const { status, statusText, ok, headers, url, type, redirected } = response; diff --git a/public/app/core/specs/backend_srv.test.ts b/public/app/core/specs/backend_srv.test.ts index ac96cfc2572..097788913d8 100644 --- a/public/app/core/specs/backend_srv.test.ts +++ b/public/app/core/specs/backend_srv.test.ts @@ -2,7 +2,7 @@ import { Observable, of, lastValueFrom } from 'rxjs'; import { fromFetch } from 'rxjs/fetch'; import { delay } from 'rxjs/operators'; -import { AppEvents, DataQueryErrorType, EventBusExtended } from '@grafana/data'; +import { AppEvents, DataQueryErrorType, EventBusExtended, PathValidationError } from '@grafana/data'; import { BackendSrvRequest, FetchError, FetchResponse } from '@grafana/runtime'; import { TokenRevokedModal } from '../../features/users/TokenRevokedModal'; @@ -745,4 +745,142 @@ describe('backendSrv', () => { }); }); }); + + describe('validatePath functionality', () => { + describe('when validatePath is enabled in options', () => { + it.each(['get', 'post', 'put', 'patch', 'delete'] as const)( + 'should sanitize malicious paths in $method requests', + async (method) => { + const { backendSrv } = getTestContext(); + const maliciousUrl = '/api/users/%2e%2e/admin'; + + const promise = + method === 'get' + ? backendSrv[method](maliciousUrl, undefined, undefined, { validatePath: true }) + : backendSrv[method](maliciousUrl, undefined, { validatePath: true }); + + await expect(promise).rejects.toThrow(PathValidationError); + await expect(promise).rejects.toThrow('Invalid request path'); + } + ); + + it('should preserve safe paths when sanitizing', async () => { + const { backendSrv } = getTestContext(); + const safeUrl = '/api/users/123'; + + const promise = backendSrv.get(safeUrl, undefined, undefined, { validatePath: true }); + + await expect(promise).resolves.toBeDefined(); + }); + + it('should sanitise paths when calling .request', async () => { + const { backendSrv } = getTestContext(); + const maliciousUrl = '/api/users/%2e%2e/admin'; + + const promise = backendSrv.request({ url: maliciousUrl, method: 'GET', validatePath: true }); + + await expect(promise).rejects.toThrow(PathValidationError); + }); + + it('should sanitise paths when calling .fetch', (done) => { + const { backendSrv } = getTestContext(); + const maliciousUrl = '/api/users/%2e%2e/admin'; + + const observable = backendSrv.fetch({ url: maliciousUrl, method: 'GET', validatePath: true }); + + observable.subscribe({ + next: () => { + throw new Error('Should not succeed'); + }, + error: (err) => { + expect(err).toBeInstanceOf(PathValidationError); + expect(err.message).toBe('Invalid request path'); + done(); + }, + }); + }); + }); + + describe('when validatePath is disabled or not provided', () => { + it('should not sanitize paths when validatePath is false', async () => { + const { backendSrv } = getTestContext(); + const maliciousUrl = '/api/../admin/secrets'; + + const promise = backendSrv.get(maliciousUrl, undefined, undefined, { validatePath: false }); + + await expect(promise).resolves.toBeDefined(); + }); + + it('should not sanitize paths when validatePath is not provided', async () => { + const { backendSrv } = getTestContext(); + const maliciousUrl = '/api/../admin/secrets'; + + const promise = backendSrv.get(maliciousUrl); + + await expect(promise).resolves.toBeDefined(); + }); + + it('should preserve paths when only other options are provided', async () => { + const { backendSrv } = getTestContext(); + const maliciousUrl = '/api/../admin/secrets'; + + const promise = backendSrv.delete(maliciousUrl, undefined, { showErrorAlert: false }); + + await expect(promise).resolves.toBeDefined(); + }); + }); + + describe('with complex validatePath scenarios', () => { + it('should handle URL encoded traversal attacks', async () => { + const { backendSrv } = getTestContext(); + const encodedUrl = '/api/%252e%252e/admin'; + + const promise = backendSrv.get(encodedUrl, undefined, undefined, { validatePath: true }); + + await expect(promise).rejects.toThrow(PathValidationError); + }); + + it('should preserve paths with legitimate dots and query parameters', async () => { + const { backendSrv, parseRequestOptionsMock } = getTestContext(); + const safeUrl = '/api/file.json?version=1.2.3&format=compact'; + + const promise = backendSrv.get(safeUrl, undefined, undefined, { validatePath: true }); + + await expect(promise).resolves.toBeDefined(); + expect(parseRequestOptionsMock).toHaveBeenCalledWith( + expect.objectContaining({ + url: '/api/file.json?version=1.2.3&format=compact', // legitimate dots and query params should be preserved + method: 'GET', + validatePath: true, + }) + ); + }); + + it('should work with other options combined', async () => { + const { backendSrv, parseRequestOptionsMock } = getTestContext(); + const safeUrl = '/api/dashboard/save'; + + const promise = backendSrv.post( + safeUrl, + { dashboard: 'data' }, + { + validatePath: true, + showErrorAlert: false, + showSuccessAlert: true, + } + ); + + await expect(promise).resolves.toBeDefined(); + expect(parseRequestOptionsMock).toHaveBeenCalledWith( + expect.objectContaining({ + url: '/api/dashboard/save', + method: 'POST', + validatePath: true, + showErrorAlert: false, + showSuccessAlert: true, + }) + ); + }); + }); + }); }); diff --git a/public/app/core/utils/fetch.test.ts b/public/app/core/utils/fetch.test.ts index 4c05a50497c..cd613132413 100644 --- a/public/app/core/utils/fetch.test.ts +++ b/public/app/core/utils/fetch.test.ts @@ -1,3 +1,5 @@ +import { PathValidationError } from '@grafana/data'; + import { isContentTypeJson, parseBody, @@ -29,6 +31,18 @@ describe('parseUrlFromOptions', () => { expect(parseUrlFromOptions({ params, url })).toEqual(expected); } ); + + it('should validate the path if validatePath is true', () => { + expect(() => parseUrlFromOptions({ url: '/api/users/%2e%2e/admin', validatePath: true })).toThrow( + PathValidationError + ); + }); + + it('should not validate the path if validatePath is false', () => { + expect(parseUrlFromOptions({ url: '/api/users/%2e%2e/admin', validatePath: false })).toEqual( + '/api/users/%2e%2e/admin' + ); + }); }); describe('parseInitFromOptions', () => { diff --git a/public/app/core/utils/fetch.ts b/public/app/core/utils/fetch.ts index 237c5d0eb13..a456f35e066 100644 --- a/public/app/core/utils/fetch.ts +++ b/public/app/core/utils/fetch.ts @@ -1,6 +1,6 @@ import { omitBy } from 'lodash'; -import { deprecationWarning } from '@grafana/data'; +import { deprecationWarning, validatePath } from '@grafana/data'; import { BackendSrvRequest } from '@grafana/runtime'; export const parseInitFromOptions = (options: BackendSrvRequest): RequestInit => { @@ -164,10 +164,19 @@ function serializeParams(data: Record): string { .join('&'); } +/** + * Formats and validates the URL. + * If options.validatePath is true, this will throw an exception if the URL fails validation. + */ export const parseUrlFromOptions = (options: BackendSrvRequest): string => { const cleanParams = omitBy(options.params, (v) => v === undefined || (v && v.length === 0)); const serializedParams = serializeParams(cleanParams); - return options.params && serializedParams.length ? `${options.url}?${serializedParams}` : options.url; + + const url = options.validatePath // + ? validatePath(options.url) + : options.url; + + return options.params && serializedParams.length ? `${url}?${serializedParams}` : url; }; export const parseCredentials = (options: BackendSrvRequest): RequestCredentials => { diff --git a/public/app/features/dashboard/api/legacy.ts b/public/app/features/dashboard/api/legacy.ts index 8833958ec5f..5ce46cdf442 100644 --- a/public/app/features/dashboard/api/legacy.ts +++ b/public/app/features/dashboard/api/legacy.ts @@ -29,11 +29,14 @@ export class LegacyDashboardAPI implements DashboardAPI deleteDashboard(uid: string, showSuccessAlert: boolean): Promise { return getBackendSrv().delete(`/api/dashboards/uid/${uid}`, undefined, { showSuccessAlert, + validatePath: true, }); } async getDashboardDTO(uid: string, params?: UrlQueryMap) { - const result = await getBackendSrv().get(`/api/dashboards/uid/${uid}`, params); + const result = await getBackendSrv().get(`/api/dashboards/uid/${uid}`, params, undefined, { + validatePath: true, + }); if (result.meta.isFolder) { appEvents.emit(AppEvents.alertError, ['Dashboard not found']); diff --git a/public/app/features/dashboard/services/DashboardLoaderSrv.ts b/public/app/features/dashboard/services/DashboardLoaderSrv.ts index c3629145e15..22bb842a761 100644 --- a/public/app/features/dashboard/services/DashboardLoaderSrv.ts +++ b/public/app/features/dashboard/services/DashboardLoaderSrv.ts @@ -43,7 +43,7 @@ abstract class DashboardLoaderSrvBase implements DashboardLoaderSrvLike { const url = 'public/dashboards/' + file.replace(/\.(?!js)/, '/') + '?' + new Date().getTime(); return getBackendSrv() - .get(url) + .get(url, undefined, undefined, { validatePath: true }) .then(this.executeScript.bind(this)) .then( (result: any) => { diff --git a/public/app/features/datasources/api.ts b/public/app/features/datasources/api.ts index 1cec20fe125..e1ff17ff598 100644 --- a/public/app/features/datasources/api.ts +++ b/public/app/features/datasources/api.ts @@ -74,6 +74,7 @@ export const updateDataSource = (dataSource: DataSourceSettings) => { return getBackendSrv().put(`/api/datasources/uid/${dataSource.uid}`, dataSource, { showErrorAlert: false, showSuccessAlert: false, + validatePath: true, }); }; diff --git a/public/app/features/library-panels/state/api.ts b/public/app/features/library-panels/state/api.ts index 5466f0e79f6..6ae546af666 100644 --- a/public/app/features/library-panels/state/api.ts +++ b/public/app/features/library-panels/state/api.ts @@ -83,7 +83,14 @@ export async function getLibraryPanel(uid: string, isHandled = false): Promise { - const { result } = await getBackendSrv().get<{ result: LibraryElementDTO[] }>(`/api/library-elements/name/${name}`); + const { result } = await getBackendSrv().get<{ result: LibraryElementDTO[] }>( + `/api/library-elements/name/${name}`, + undefined, + undefined, + { + validatePath: true, + } + ); return result; } diff --git a/public/app/features/plugins/pluginSettings.test.ts b/public/app/features/plugins/pluginSettings.test.ts index b9dc92bf1da..02c16af039b 100644 --- a/public/app/features/plugins/pluginSettings.test.ts +++ b/public/app/features/plugins/pluginSettings.test.ts @@ -29,7 +29,9 @@ describe('PluginSettings', () => { // assert expect(response).toEqual(testPluginResponse); expect(getRequestSpy).toHaveBeenCalledTimes(1); - expect(getRequestSpy).toHaveBeenCalledWith('/api/plugins/test/settings', undefined, undefined, undefined); + expect(getRequestSpy).toHaveBeenCalledWith('/api/plugins/test/settings', undefined, undefined, { + validatePath: true, + }); }); it('should fetch settings from cache when it has a hit', async () => { diff --git a/public/app/features/plugins/pluginSettings.ts b/public/app/features/plugins/pluginSettings.ts index 135cdbb9e97..1865df5b914 100644 --- a/public/app/features/plugins/pluginSettings.ts +++ b/public/app/features/plugins/pluginSettings.ts @@ -13,7 +13,7 @@ export function getPluginSettings(pluginId: string, options?: Partial { pluginInfoCache[pluginId] = settings; return settings;