Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enhance Error Redaction #609

Merged
merged 3 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ over other authentication methods, i.e., application default credentials.
*
* Set `false` to disable.
*
* @remarks
*
* This does not replace the requirement for an active Data Loss Prevention (DLP) provider. For DLP suggestions, see:
* - https://cloud.google.com/sensitive-data-protection/docs/redacting-sensitive-data#dlp_deidentify_replace_infotype-nodejs
* - https://cloud.google.com/sensitive-data-protection/docs/infotypes-reference#credentials_and_secrets
*
* @experimental
*/
errorRedactor?: typeof defaultErrorRedactor | false;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"karma-mocha": "^2.0.0",
"karma-remap-coverage": "^0.1.5",
"karma-sourcemap-loader": "^0.4.0",
"karma-webpack": "^5.0.0",
"karma-webpack": "5.0.0",
"linkinator": "^4.0.0",
"mocha": "^8.0.0",
"multiparty": "^4.2.1",
Expand Down
30 changes: 29 additions & 1 deletion src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,15 @@
}

export interface Headers {
[index: string]: any;

Check warning on line 121 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
}
export type GaxiosPromise<T = any> = Promise<GaxiosResponse<T>>;

Check warning on line 123 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

export interface GaxiosXMLHttpRequest {
responseURL: string;
}

export interface GaxiosResponse<T = any> {

Check warning on line 129 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
config: GaxiosOptions;
data: T;
status: number;
Expand All @@ -143,7 +143,7 @@
* Optional method to override making the actual HTTP request. Useful
* for writing tests.
*/
adapter?: <T = any>(

Check warning on line 146 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
options: GaxiosOptions,
defaultAdapter: (options: GaxiosOptions) => GaxiosPromise<T>
) => GaxiosPromise<T>;
Expand All @@ -164,8 +164,8 @@
| 'TRACE'
| 'PATCH';
headers?: Headers;
data?: any;

Check warning on line 167 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
body?: any;

Check warning on line 168 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
/**
* The maximum size of the http response content in bytes allowed.
*/
Expand All @@ -175,13 +175,13 @@
*/
maxRedirects?: number;
follow?: number;
params?: any;

Check warning on line 178 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
paramsSerializer?: (params: {[index: string]: string | number}) => string;
timeout?: number;
/**
* @deprecated ignored
*/
onUploadProgress?: (progressEvent: any) => void;

Check warning on line 184 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
responseType?:
| 'arraybuffer'
| 'blob'
Expand All @@ -195,7 +195,7 @@
retry?: boolean;
// Should be instance of https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal
// interface. Left as 'any' due to incompatibility between spec and abort-controller.
signal?: any;

Check warning on line 198 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
size?: number;
/**
* Implementation of `fetch` to use when making the API call. By default,
Expand All @@ -209,6 +209,12 @@
/**
* An experimental error redactor.
*
* @remarks
*
* This does not replace the requirement for an active Data Loss Prevention (DLP) provider. For DLP suggestions, see:
* - https://cloud.google.com/sensitive-data-protection/docs/redacting-sensitive-data#dlp_deidentify_replace_infotype-nodejs
* - https://cloud.google.com/sensitive-data-protection/docs/infotypes-reference#credentials_and_secrets
*
* @experimental
*/
errorRedactor?: typeof defaultErrorRedactor | false;
Expand All @@ -227,7 +233,7 @@
*
* @experimental
*/
export type RedactableGaxiosResponse<T = any> = Pick<

Check warning on line 236 in src/common.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
GaxiosResponse<T>,
'config' | 'data' | 'headers'
>;
Expand Down Expand Up @@ -359,6 +365,16 @@
if (/^authentication$/.test(key)) {
headers[key] = REDACT;
}

// any casing of `Authorization`
if (/^authorization$/.test(key)) {
danielbankhead marked this conversation as resolved.
Show resolved Hide resolved
headers[key] = REDACT;
}

// anything containing secret, such as 'client secret'
if (/secret/.test(key)) {
headers[key] = REDACT;
}
}
}

Expand All @@ -370,7 +386,11 @@
) {
const text = obj[key];

if (/grant_type=/.test(text) || /assertion=/.test(text)) {
if (
/grant_type=/.test(text) ||
/assertion=/.test(text) ||
/secret/.test(text)
) {
obj[key] = REDACT;
}
}
Expand All @@ -385,6 +405,10 @@
if ('assertion' in obj) {
obj['assertion'] = REDACT;
}

if ('client_secret' in obj) {
obj['client_secret'] = REDACT;
}
}
}

Expand All @@ -404,6 +428,10 @@
url.searchParams.set('token', REDACT);
}

if (url.searchParams.has('client_secret')) {
url.searchParams.set('client_secret', REDACT);
}

data.config.url = url.toString();
} catch {
// ignore error - no need to parse an invalid URL
Expand Down
12 changes: 10 additions & 2 deletions test/test.getch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -704,20 +704,23 @@ describe('🎏 data handling', () => {

const customURL = new URL(url);
customURL.searchParams.append('token', 'sensitive');
customURL.searchParams.append('client_secret', 'data');
customURL.searchParams.append('random', 'non-sensitive');

const config: GaxiosOptions = {
headers: {
authentication: 'My Auth',
authorization: 'My Auth',
'content-type': 'application/x-www-form-urlencoded',
random: 'data',
},
data: {
grant_type: 'urn:ietf:params:oauth:grant-type:jwt-bearer',
assertion: 'somesensitivedata',
unrelated: 'data',
client_secret: 'data',
},
body: 'grant_type=somesensitivedata&assertion=somesensitivedata',
body: 'grant_type=somesensitivedata&assertion=somesensitivedata&client_secret=data',
};

// simulate JSON response
Expand Down Expand Up @@ -756,13 +759,15 @@ describe('🎏 data handling', () => {
assert.deepStrictEqual(e.config.headers, {
...config.headers, // non-redactables should be present
authentication: REDACT,
authorization: REDACT,
});

// config redactions - data
assert.deepStrictEqual(e.config.data, {
...config.data, // non-redactables should be present
grant_type: REDACT,
assertion: REDACT,
client_secret: REDACT,
});

// config redactions - body
Expand All @@ -773,6 +778,7 @@ describe('🎏 data handling', () => {
const resultURL = new URL(e.config.url);
assert.notDeepStrictEqual(resultURL.toString(), customURL.toString());
customURL.searchParams.set('token', REDACT);
customURL.searchParams.set('client_secret', REDACT);
assert.deepStrictEqual(resultURL.toString(), customURL.toString());

// response redactions
Expand All @@ -781,11 +787,13 @@ describe('🎏 data handling', () => {
assert.deepStrictEqual(e.response.headers, {
...responseHeaders, // non-redactables should be present
authentication: REDACT,
authorization: REDACT,
});
assert.deepStrictEqual(e.response.data, {
...response, // non-redactables should be present
grant_type: REDACT,
assertion: REDACT,
client_secret: REDACT,
grant_type: REDACT,
});
} finally {
scope.done();
Expand Down
Loading