From b047b37941e155fa8ad552113e3412f8a19d25a5 Mon Sep 17 00:00:00 2001 From: Harsha Nalluru Date: Tue, 16 Apr 2024 16:36:59 -0700 Subject: [PATCH] [Recorder] Central sanitizers (#29331) ### Packages impacted by this PR `@azure-tools/test-recorder` - Adding the central sanitizers ### Issues associated with this PR **References:** - https://github.com/Azure/azure-sdk-for-java/pull/39700 - https://github.com/Azure/azure-sdk-for-python/pull/35196 - And the patterns found ### Describe the problem that is addressed by this PR - Introducing fallback sanitizers into the test recorder to handle potential secret leaks. - The new sanitizers are designed to work in conjunction with the existing `handleEnvSetup` mechanism and the fake secrets. - The sanitizers include: - `BodyKeySanitizers` that redact sensitive information in the JSON body of the requests. - `FindReplaceSanitizers` that redact sensitive information based on provided regular expressions. - `HeaderSanitizers` that redact sensitive information in the headers of the requests. ## Tests I've ran the tests for the following and they work fine - [x] recorder - [x] template - [x] notification-hubs (needed to make a few fixes for browser tests in notification hubs which do feel like unrelated to this PR, but fixing them here anyway.) ___Currently only these three packages depend on recorder v4.___ ## Future work (future PRs) - Once this PR is merged, cherrypick the commit and release a hotfix 3.x version - Add more tests at some point --- .../notification-hubs/assets.json | 2 +- .../notification-hubs/test.env | 2 +- .../test/public/utils/recordedClient.ts | 19 +++ sdk/test-utils/recorder/CHANGELOG.md | 11 ++ sdk/test-utils/recorder/src/recorder.ts | 4 + .../recorder/src/utils/fallbackSanitizers.ts | 134 ++++++++++++++++++ sdk/test-utils/recorder/src/utils/utils.ts | 2 +- 7 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 sdk/test-utils/recorder/src/utils/fallbackSanitizers.ts diff --git a/sdk/notificationhubs/notification-hubs/assets.json b/sdk/notificationhubs/notification-hubs/assets.json index 54a733710a5..180ac50ef39 100644 --- a/sdk/notificationhubs/notification-hubs/assets.json +++ b/sdk/notificationhubs/notification-hubs/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "js", "TagPrefix": "js/notificationhubs/notification-hubs", - "Tag": "js/notificationhubs/notification-hubs_44f522d99a" + "Tag": "js/notificationhubs/notification-hubs_0718648c50" } diff --git a/sdk/notificationhubs/notification-hubs/test.env b/sdk/notificationhubs/notification-hubs/test.env index ef01cb86bf5..8be0bc8d8d6 100644 --- a/sdk/notificationhubs/notification-hubs/test.env +++ b/sdk/notificationhubs/notification-hubs/test.env @@ -1,5 +1,5 @@ # Used in most samples. Retrieve these values from a Azure Notification Hub in the Azure Portal. -NOTIFICATIONHUBS_CONNECTION_STRING="" +NOTIFICATION_HUB_CONNECTION_STRING="" # Used in most samples. Provide the name of a Notification Hub within a namespace. NOTIFICATION_HUB_NAME="" diff --git a/sdk/notificationhubs/notification-hubs/test/public/utils/recordedClient.ts b/sdk/notificationhubs/notification-hubs/test/public/utils/recordedClient.ts index 56028f08e32..d862ef86424 100644 --- a/sdk/notificationhubs/notification-hubs/test/public/utils/recordedClient.ts +++ b/sdk/notificationhubs/notification-hubs/test/public/utils/recordedClient.ts @@ -3,6 +3,7 @@ import { Recorder, RecorderStartOptions, env } from "@azure-tools/test-recorder"; import { NotificationHubsClientContext, createClientContext } from "../../../src/api/index.js"; +import { isBrowser } from "@azure/core-util"; const replaceableVariables: { [k: string]: string } = { // Used in record and playback modes @@ -30,6 +31,24 @@ export async function createRecordedClientContext( recorder: Recorder, ): Promise { await recorder.start(recorderOptions); + if (isBrowser) { + // there are timestamps in the body, so do not match body + await recorder.setMatcher("BodilessMatcher"); + await recorder.addSanitizers( + { + // looks like the registration id is dynamic, redacting it instead + generalSanitizers: [ + { + regex: true, + target: "registrations/(?.*?)?api-version=", + value: "registration-id-redacted", + groupForReplace: "secret", + }, + ], + }, + ["record", "playback"], + ); + } if (!env.NOTIFICATION_HUB_CONNECTION_STRING || !env.NOTIFICATION_HUB_NAME) { throw new Error( diff --git a/sdk/test-utils/recorder/CHANGELOG.md b/sdk/test-utils/recorder/CHANGELOG.md index b6de81799b2..11fcc6c1b1c 100644 --- a/sdk/test-utils/recorder/CHANGELOG.md +++ b/sdk/test-utils/recorder/CHANGELOG.md @@ -1,5 +1,16 @@ # Release History +## 4.1.0 + +### Features Added + +- Introduced fallback sanitizers into the test recorder to handle potential secret leaks. + - The new sanitizers are designed to work in conjunction with the existing `handleEnvSetup` mechanism and the fake secrets. + - The sanitizers include: + - `BodyKeySanitizers` that redact sensitive information in the JSON body of the requests. + - `FindReplaceSanitizers` that redact sensitive information based on provided regular expressions. + - `HeaderSanitizers` that redact sensitive information in the headers of the requests. + ## 4.0.0 (2024-04-09) ### Features Added diff --git a/sdk/test-utils/recorder/src/recorder.ts b/sdk/test-utils/recorder/src/recorder.ts index 11e2ee5c3b3..00e1a9ab78c 100644 --- a/sdk/test-utils/recorder/src/recorder.ts +++ b/sdk/test-utils/recorder/src/recorder.ts @@ -35,6 +35,7 @@ import { decodeBase64 } from "./utils/encoding.js"; import { AdditionalPolicyConfig } from "@azure/core-client"; import { isVitestTestContext, TestInfo, VitestSuite } from "./testInfo.js"; import { env } from "./utils/env.js"; +import { fallbackSanitizers } from "./utils/fallbackSanitizers.js"; /** * Caculates session file path and JSON assets path from test context @@ -355,6 +356,9 @@ export class Recorder { options.envSetupForPlayback, ); + // Fallback sanitizers to be added in both record/playback modes + await fallbackSanitizers(this.httpClient, Recorder.url, this.recordingId); + // Sanitizers to be added only in record mode if (isRecordMode() && options.sanitizerOptions) { // Makes a call to the proxy-tool to add the sanitizers for the current recording id diff --git a/sdk/test-utils/recorder/src/utils/fallbackSanitizers.ts b/sdk/test-utils/recorder/src/utils/fallbackSanitizers.ts new file mode 100644 index 00000000000..9f7bee2f754 --- /dev/null +++ b/sdk/test-utils/recorder/src/utils/fallbackSanitizers.ts @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { HttpClient } from "@azure/core-rest-pipeline"; +import { addSanitizers } from "../sanitizer.js"; +import { BodyKeySanitizer, FindReplaceSanitizer, HeaderSanitizer } from "./utils.js"; + +const JSON_BODY_KEYS_TO_REDACT = [ + "authHeader", + "accountKey", + "accessToken", + "accountName", + "applicationId", + "apiKey", + "client_secret", + "connectionString", + "url", + "host", + "password", + "userName", + "applicationSecret", + "aliasSecondaryConnectionString", + "aliasPrimaryConnectionString", + "primaryKey", + "secondaryKey", + "adminPassword.value", + "administratorLoginPassword", + "runAsPassword", + "adminPassword", + "accessSAS", + "WEBSITE_AUTH_ENCRYPTION_KEY", + "decryptionKey", + "primaryMasterKey", + "primaryReadonlyMasterKey", + "secondaryMasterKey", + "secondaryReadonlyMasterKey", + "certificatePassword", + "clientSecret", + "keyVaultClientSecret", + "authHeader", + "httpHeader", + "encryptedCredential", + "appkey", + "functionKey", + "atlasKafkaPrimaryEndpoint", + "atlasKafkaSecondaryEndpoint", + "certificatePassword", + "storageAccountPrimaryKey", + "privateKey", + "fencingClientPassword", + "acrToken", + "scriptUrlSasToken", + "azureBlobSource.containerUrl", + "properties.DOCKER_REGISTRY_SEVER_PASSWORD", +]; + +const BODY_REGEXES_TO_REDACT = [ + "(?:(Password|User ID)=)(?.*)(?:;)", + "client_secret=(?[^&]+)", + "(?.*?)", + "(?.*?)", + ".*?(?.*?).*?", + ".*?(?.*?).*?", + ".*?(?.*?).*?", + 'SharedAccessKey=(?[^;\\"]+)', + 'AccountKey=(?[^;\\"]+)', + 'accesskey=(?[^;\\"]+)', + 'AccessKey=(?[^;\\"]+)', + 'Secret=(?[^;\\"]+)', + "access_token=(?.*?)(?=&|$)", + "refresh_token=(?.*?)(?=&|$)", + '(?:(sv|sig|se|srt|ss|sp)=)(?[^&\\"]*)', +]; + +const URL_REGEX = "(?<=http://|https://)([^/?]+)"; + +const HEADER_KEYS_TO_REDACT = [ + "Ocp-Apim-Subscription-Key", + "api-key", + "x-api-key", + "subscription-key", + "x-ms-encryption-key", + "sshPassword", +]; + +export async function fallbackSanitizers( + httpClient: HttpClient, + url: string, + recordingId: string, +): Promise { + const bodyKeySanitizers: BodyKeySanitizer[] = JSON_BODY_KEYS_TO_REDACT.map((prop) => ({ + jsonPath: `$..${prop}`, // Handles the request body + value: "REDACTED", + })); + + const generalSanitizers: FindReplaceSanitizer[] = BODY_REGEXES_TO_REDACT.map((regex) => ({ + value: "REDACTED", + regex: true, + groupForReplace: "secret", + target: regex, + })); + + const headerSanitizers: HeaderSanitizer[] = [ + { + key: "Operation-location", + groupForReplace: "secret", + regex: true, + target: URL_REGEX, + value: "REDACTED", + }, + { + key: "ServiceBusDlqSupplementaryAuthorization", + groupForReplace: "secret", + regex: true, + target: '(?:(sv|sig|se|srt|ss|sp)=)(?[^&\\"]+)', + value: "REDACTED", + }, + { + key: "ServiceBusSupplementaryAuthorization", + groupForReplace: "secret", + regex: true, + target: '(?:(sv|sig|se|srt|ss|sp)=)(?[^&\\"]+)', + value: "REDACTED", + }, + ]; + + const headersForRemoval: string[] = HEADER_KEYS_TO_REDACT; + await addSanitizers(httpClient, url, recordingId, { + bodyKeySanitizers, + generalSanitizers, + removeHeaderSanitizer: { headersForRemoval }, + headerSanitizers, + }); +} diff --git a/sdk/test-utils/recorder/src/utils/utils.ts b/sdk/test-utils/recorder/src/utils/utils.ts index 5dd14742668..572e3f75286 100644 --- a/sdk/test-utils/recorder/src/utils/utils.ts +++ b/sdk/test-utils/recorder/src/utils/utils.ts @@ -133,7 +133,7 @@ export function isStringSanitizer(sanitizer: FindReplaceSanitizer): sanitizer is * * If the body is NOT a JSON object, this sanitizer will NOT be applied. */ -type BodyKeySanitizer = { +export type BodyKeySanitizer = { regex?: string; value?: string;