Skip to content

Commit

Permalink
refactor: process error stack and mask potential secrets in error mes…
Browse files Browse the repository at this point in the history
…sage in telemetry (#11290)

* fix: extract method names from error stack

* fix: extract method names from error stack

* fix: use entropy to mask secret tokens in string

* test: ut

* test: ut

* refactor: white list flag

* fix: white list

* refactor: improvement

* test: ut

* test: ut
  • Loading branch information
jayzhang authored Apr 11, 2024
1 parent e4e0a46 commit 5ddf860
Show file tree
Hide file tree
Showing 14 changed files with 265 additions and 22 deletions.
4 changes: 4 additions & 0 deletions packages/api/review/teamsfx-api.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ export interface ErrorOptionBase {
message?: string;
// (undocumented)
name?: string;
skipProcessInTelemetry?: boolean;
// (undocumented)
source?: string;
// (undocumented)
Expand Down Expand Up @@ -360,6 +361,7 @@ export interface FxError extends Error {
categories?: string[];
innerError?: any;
recommendedOperation?: string;
skipProcessInTelemetry?: boolean;
source: string;
timestamp: Date;
// (undocumented)
Expand Down Expand Up @@ -872,6 +874,7 @@ export class SystemError extends Error implements FxError {
innerError?: any;
issueLink?: string;
recommendedOperation?: string;
skipProcessInTelemetry?: boolean;
source: string;
timestamp: Date;
userData?: string;
Expand Down Expand Up @@ -1066,6 +1069,7 @@ export class UserError extends Error implements FxError {
helpLink?: string;
innerError?: any;
recommendedOperation?: string;
skipProcessInTelemetry?: boolean;
source: string;
timestamp: Date;
userData?: string;
Expand Down
20 changes: 20 additions & 0 deletions packages/api/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export interface FxError extends Error {
* e.g. "debug-in-test-tool"
*/
recommendedOperation?: string;
/**
* whether to skip process (such as mask secret tokens) in telemetry collection
*/
skipProcessInTelemetry?: boolean;
}
export interface ErrorOptionBase {
source?: string;
Expand All @@ -33,6 +37,10 @@ export interface ErrorOptionBase {
userData?: any;
displayMessage?: string;
categories?: string[];
/**
* whether to skip process (such as mask secret tokens) in telemetry collection
*/
skipProcessInTelemetry?: boolean;
}

export interface UserErrorOptions extends ErrorOptionBase {
Expand Down Expand Up @@ -73,6 +81,11 @@ export class UserError extends Error implements FxError {

categories?: string[];

/**
* whether to skip process (such as mask secret tokens) in telemetry collection
*/
skipProcessInTelemetry?: boolean;

/**
* recommended operation for user to fix the error
* e.g. "debug-in-test-tool"
Expand Down Expand Up @@ -124,6 +137,7 @@ export class UserError extends Error implements FxError {
this.displayMessage = option.displayMessage;
this.timestamp = new Date();
this.categories = option.categories;
this.skipProcessInTelemetry = option.skipProcessInTelemetry;
}
}

Expand Down Expand Up @@ -159,6 +173,11 @@ export class SystemError extends Error implements FxError {

categories?: string[];

/**
* whether to skip process (such as mask secret tokens) in telemetry collection
*/
skipProcessInTelemetry?: boolean;

/**
* recommended operation for user to fix the error
* e.g. "debug-in-test-tool"
Expand Down Expand Up @@ -210,5 +229,6 @@ export class SystemError extends Error implements FxError {
this.displayMessage = option.displayMessage;
this.timestamp = new Date();
this.categories = option.categories;
this.skipProcessInTelemetry = option.skipProcessInTelemetry;
}
}
2 changes: 2 additions & 0 deletions packages/fx-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@
"test:migration": "nyc mocha \"tests/core/middleware/migration/projectMigrationV3.test.ts\"",
"test:teamsappMgr": "nyc mocha \"tests/component/driver/teamsApp/teamsappMgr.test.ts\"",
"test:projcheck": "nyc mocha \"tests/common/projectTypeChecker.test.ts\"",
"test:telemetry": "nyc mocha \"tests/common/telemetry.test.ts\"",
"test:stringUtils": "nyc mocha \"tests/common/stringUtils.test.ts\"",
"clean": "rm -rf build",
"prebuild": "npm run gen:cli",
"build": "rimraf build && npx tsc -p ./",
Expand Down
134 changes: 134 additions & 0 deletions packages/fx-core/src/common/stringUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

const MIN_ENTROPY = 4;
const SECRET_REPLACE = "<REDACTED:secret>";
const USER_REPLACE = "<REDACTED:user>";

const WHITE_LIST = [
"user-file-path",
"publish-app,",
"X-Correlation-ID",
"innerError",
"client-request-id",
];

function getProbMap(str: string) {
const probMap = new Map<string, number>();
for (const char of str) {
probMap.set(char, (probMap.get(char) || 0) + 1);
}
for (const [char, freq] of probMap.entries()) {
const prob = freq / str.length;
probMap.set(char, prob);
}
return probMap;
}

// Measure the entropy of a string in bits per symbol.
function shannonEntropy(str: string, probMap: Map<string, number>) {
let sum = 0;
for (const char of str) {
const prob = probMap.get(char) || 0;
const delta = (prob * Math.log(prob)) / Math.log(2);
sum += delta;
}
return -sum;
}

class Token {
value: string;
splitter: boolean;
entropy?: number;
constructor(value: string, splitter: boolean) {
this.value = value;
this.splitter = splitter;
}
}

function tokenize(text: string): Token[] {
const splitterString = " '`\n\t\r\",:{}";
const splitterChars = new Set<string>();
for (const char of splitterString) {
splitterChars.add(char);
}
const tokens: Token[] = [];
let currentToken = "";
for (const char of text) {
if (splitterChars.has(char)) {
if (currentToken.length > 0) {
tokens.push(new Token(currentToken, false));
currentToken = "";
}
tokens.push(new Token(char, true));
} else {
currentToken += char;
}
}
if (currentToken.length > 0) {
tokens.push(new Token(currentToken, false));
}
return tokens;
}

function computeShannonEntropy(token: Token) {
if (!token.splitter) {
const probMap = getProbMap(token.value);
token.entropy = shannonEntropy(token.value, probMap);
}
}

export interface MaskSecretOptions {
threshold?: number;
whiteList?: string[];
}

export function maskSecret(
inputText?: string,
option = { threshold: MIN_ENTROPY, whiteList: WHITE_LIST }
): string {
if (!inputText) return "";
// mask by secret pattern
inputText = maskByPattern(inputText);
// mask by .env.xxx.user
inputText = maskSecretValues(inputText, SECRET_REPLACE);
// mask by entropy
let output = "";
const tokens = tokenize(inputText);
tokens.forEach((token) => {
computeShannonEntropy(token);
if (
option.whiteList?.includes(token.value) ||
token.splitter ||
(token.entropy || 0) <= option.threshold
) {
output += token.value;
} else {
output += SECRET_REPLACE;
}
});
for (const token of tokens) {
console.log(token);
}
return output;
}

function maskByPattern(command: string): string {
const regexU = /(-u|--username|--user) (\S+)/;
const regexP = /(-p|--password|--pwd) (\S+)/;
let output = command.replace(regexU, `$1 ${USER_REPLACE}`);
output = output.replace(regexP, `$1 ${SECRET_REPLACE}`);
return output;
}

export function maskSecretValues(stdout: string, replace = "***"): string {
for (const key of Object.keys(process.env)) {
if (key.startsWith("SECRET_")) {
const value = process.env[key];
if (value) {
stdout = stdout.replace(value, replace);
}
}
}
return stdout;
}
18 changes: 16 additions & 2 deletions packages/fx-core/src/common/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { TOOLS, globalVars } from "../core/globalVars";
import { ProjectTypeResult } from "./projectTypeChecker";
import { assign } from "lodash";
import { ProjectType } from "@microsoft/m365-spec-parser";
import { maskSecret } from "./stringUtils";

export enum TelemetryProperty {
TriggerFrom = "trigger-from",
Expand Down Expand Up @@ -241,8 +242,10 @@ export function fillInTelemetryPropsForFxError(
props[TelemetryConstants.properties.errorCode] =
props[TelemetryConstants.properties.errorCode] || errorCode;
props[TelemetryConstants.properties.errorType] = errorType;
// props[TelemetryConstants.properties.errorMessage] = error.message; // error-message is retired
// props[TelemetryConstants.properties.errorStack] = error.stack !== undefined ? error.stack : ""; // error stack will not append in error-message any more
props[TelemetryConstants.properties.errorMessage] = error.skipProcessInTelemetry
? error.message
: maskSecret(error.message);
props[TelemetryConstants.properties.errorStack] = extractMethodNamesFromErrorStack(error.stack); // error stack will not append in error-message any more
props[TelemetryConstants.properties.errorName] = error.name;

// append global context properties
Expand Down Expand Up @@ -290,3 +293,14 @@ export function fillinProjectTypeProperties(
};
assign(props, newProps);
}

export function extractMethodNamesFromErrorStack(stack?: string): string {
if (!stack) return "";
const methodNamesRegex = /at\s([\w.<>\[\]\s]+)\s\(/g;
let match;
const methodNames: string[] = [];
while ((match = methodNamesRegex.exec(stack)) !== null) {
methodNames.push(match[1]);
}
return methodNames.join(" | ");
}
4 changes: 2 additions & 2 deletions packages/fx-core/src/component/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ export const TelemetryConstants = {
success: "success",
errorCode: "error-code",
errorType: "error-type",
errorMessage: "error-message",
errorStack: "error-stack",
errorMessage: "err-message", // change the error message property key
errorStack: "err-stack", // change the error stack property key
timeCost: "time-cost",
errorName: "error-name", // need classify, keep error name as a separate property for telemetry analysis, error name should has limited set of values
innerError: "inner-error", // need classify, JSON serialized raw inner error that is caused by internal error or external call error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { TelemetryConstant } from "../../constant/commonConstant";
import { performance } from "perf_hooks";
import { TelemetryConstants } from "../../constants";
import { isExecutionResult } from "./addStartAndEndTelemetry";
import { maskSecretValues } from "../../utils/envUtil";
import { maskSecretValues } from "../../../common/stringUtils";

/**
* A special telemetry middleware for SWA deployment.
Expand Down
3 changes: 2 additions & 1 deletion packages/fx-core/src/component/driver/script/scriptDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import { ScriptExecutionError, ScriptTimeoutError } from "../../../error/script"
import { TelemetryConstant } from "../../constant/commonConstant";
import { ProgressMessages } from "../../messages";
import { getSystemEncoding } from "../../utils/charsetUtils";
import { DotenvOutput, maskSecretValues } from "../../utils/envUtil";
import { DotenvOutput } from "../../utils/envUtil";
import { DriverContext } from "../interface/commonArgs";
import { ExecutionResult, StepDriver } from "../interface/stepDriver";
import { addStartAndEndTelemetry } from "../middleware/addStartAndEndTelemetry";
import { maskSecretValues } from "../../../common/stringUtils";

const ACTION_NAME = "script";

Expand Down
12 changes: 0 additions & 12 deletions packages/fx-core/src/component/utils/envUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,15 +386,3 @@ class DotenvUtil {
}

export const dotenvUtil = new DotenvUtil();

export function maskSecretValues(stdout: string): string {
for (const key of Object.keys(process.env)) {
if (key.startsWith("SECRET_")) {
const value = process.env[key];
if (value) {
stdout = stdout.replace(value, "***");
}
}
}
return stdout;
}
1 change: 1 addition & 0 deletions packages/fx-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export * from "./common/deps-checker";
export * from "./common/featureFlags";
export * from "./common/globalState";
export * from "./common/telemetry";
export * from "./common/stringUtils";
export { jsonUtils } from "./common/jsonUtils";
export * from "./common/local";
export * from "./common/m365/constants";
Expand Down
26 changes: 26 additions & 0 deletions packages/fx-core/tests/common/stringUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { assert } from "chai";
import "mocha";
import sinon from "sinon";
import { maskSecret } from "../../src/common/stringUtils";

describe("stringUtils", () => {
const sandbox = sinon.createSandbox();
afterEach(async () => {
sandbox.restore();
});
describe("maskSecret", () => {
it("happy path", async () => {
const input =
"Bearer eyJ0eXAiOiJKV1QiLCJub25jZSI6IkZQQVpfd0ZXc2EwdFpCcGMtcXJITFBzQjd6QnJSWmpzbnFTMW";
const output = maskSecret(input);
assert.equal(output, "Bearer <REDACTED:secret>");
});
it("input undefined", async () => {
const output = maskSecret();
assert.equal(output, "");
});
});
});
Loading

0 comments on commit 5ddf860

Please sign in to comment.