backendsrv: Implement optional URL path fetch sanitize (#106540)
* feat: Implement optional URL path sanitization in BackendSrv methods * add comment * revert * remove namespace import from backendsrv * change method to validatePath, remove query params and fragments * Moved validatePath call into fetch and make it throw an error instead * update pluginSettings tests * prettier * Update public/app/features/plugins/pluginSettings.ts Co-authored-by: Hugo Häggmark <hugo.haggmark@gmail.com> * change name to validatePath * fix other tests * rename property in backend_srv tests * rename to validatePath in backend_srv, add extra tests * Move path validation into parseUrlFromOptions * fix * Add additional check * Add test --------- Co-authored-by: joshhunt <josh.hunt@grafana.com> Co-authored-by: Hugo Häggmark <hugo.haggmark@gmail.com>
This commit is contained in:
co-authored by
Hugo Häggmark
joshhunt
parent
704d91f2be
commit
b30f501bff
@@ -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';
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<OriginalPath extends string>(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,
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -147,7 +147,14 @@ export class BackendSrv implements BackendService {
|
||||
chunked(options: BackendSrvRequest): Observable<FetchResponse<Uint8Array | undefined>> {
|
||||
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<T>(options: BackendSrvRequest): Observable<FetchResponse<T>> {
|
||||
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;
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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, any>): 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 => {
|
||||
|
||||
@@ -29,11 +29,14 @@ export class LegacyDashboardAPI implements DashboardAPI<DashboardDTO, Dashboard>
|
||||
deleteDashboard(uid: string, showSuccessAlert: boolean): Promise<DeleteDashboardResponse> {
|
||||
return getBackendSrv().delete<DeleteDashboardResponse>(`/api/dashboards/uid/${uid}`, undefined, {
|
||||
showSuccessAlert,
|
||||
validatePath: true,
|
||||
});
|
||||
}
|
||||
|
||||
async getDashboardDTO(uid: string, params?: UrlQueryMap) {
|
||||
const result = await getBackendSrv().get<DashboardDTO>(`/api/dashboards/uid/${uid}`, params);
|
||||
const result = await getBackendSrv().get<DashboardDTO>(`/api/dashboards/uid/${uid}`, params, undefined, {
|
||||
validatePath: true,
|
||||
});
|
||||
|
||||
if (result.meta.isFolder) {
|
||||
appEvents.emit(AppEvents.alertError, ['Dashboard not found']);
|
||||
|
||||
@@ -43,7 +43,7 @@ abstract class DashboardLoaderSrvBase<T> implements DashboardLoaderSrvLike<T> {
|
||||
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) => {
|
||||
|
||||
@@ -74,6 +74,7 @@ export const updateDataSource = (dataSource: DataSourceSettings) => {
|
||||
return getBackendSrv().put(`/api/datasources/uid/${dataSource.uid}`, dataSource, {
|
||||
showErrorAlert: false,
|
||||
showSuccessAlert: false,
|
||||
validatePath: true,
|
||||
});
|
||||
};
|
||||
|
||||
|
||||
@@ -83,7 +83,14 @@ export async function getLibraryPanel(uid: string, isHandled = false): Promise<L
|
||||
}
|
||||
|
||||
export async function getLibraryPanelByName(name: string): Promise<LibraryElementDTO[]> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -13,7 +13,7 @@ export function getPluginSettings(pluginId: string, options?: Partial<BackendSrv
|
||||
return Promise.resolve(v);
|
||||
}
|
||||
return getBackendSrv()
|
||||
.get(`/api/plugins/${pluginId}/settings`, undefined, undefined, options)
|
||||
.get(`/api/plugins/${pluginId}/settings`, undefined, undefined, { ...options, validatePath: true })
|
||||
.then((settings) => {
|
||||
pluginInfoCache[pluginId] = settings;
|
||||
return settings;
|
||||
|
||||
Reference in New Issue
Block a user