diff --git a/.changeset/redact-response-debug-logs.md b/.changeset/redact-response-debug-logs.md new file mode 100644 index 000000000..71c52e0c3 --- /dev/null +++ b/.changeset/redact-response-debug-logs.md @@ -0,0 +1,5 @@ +--- +"@slack/web-api": patch +--- + +fix: apply redact() to API response bodies in debug logs and recurse into nested objects, preventing tokens from leaking into logs when debug logging is enabled diff --git a/packages/web-api/src/WebClient.test.ts b/packages/web-api/src/WebClient.test.ts index 4dcd4c851..d599cebc7 100644 --- a/packages/web-api/src/WebClient.test.ts +++ b/packages/web-api/src/WebClient.test.ts @@ -118,6 +118,49 @@ describe('WebClient', () => { // Calling #setName of the given logger is destructive assert.strictEqual((logger.setName as sinon.SinonStub).called, false); }); + + it('should redact tokens in API response bodies in debug logs', async () => { + const scope = nock('https://slack.com') + .post('/api/oauth.v2.access') + .reply(200, { + ok: true, + access_token: 'xoxb-secret-token', + token_type: 'bot', + scope: 'commands,incoming-webhook', + bot_user_id: 'U0KRQLJ9H', + app_id: 'A0KRD7HC3', + expires_in: 43200, + refresh_token: 'xoxe-secret-token', + team: { + name: 'Slack Softball Team', + id: 'T9TK3CUKW', + }, + enterprise: { + name: 'slack-sports', + id: 'E12345678', + }, + authed_user: { + id: 'U1234', + scope: 'chat:write', + access_token: 'xoxe.xoxp-secret-token', + expires_in: 43200, + refresh_token: 'xoxe-secret-token', + token_type: 'user', + }, + }); + const client = new WebClient(undefined, { logLevel: LogLevel.DEBUG, logger }); + await client.apiCall('oauth.v2.access', { code: 'test-code' }); + scope.done(); + + const debugCalls = (logger.debug as sinon.SinonStub).getCalls(); + const resultLog = debugCalls.find((call) => call.args[0].includes('http request result')); + assert.ok(resultLog, 'expected a debug log for the http request result'); + assert.ok(resultLog.args[0].includes('[[REDACTED]]'), 'expected access_token to be redacted'); + assert.ok(!resultLog.args[0].includes('xoxb-secret-token'), 'expected raw bot token to not appear in logs'); + assert.ok(!resultLog.args[0].includes('xoxe-secret-token'), 'expected raw refresh token to not appear in logs'); + assert.ok(!resultLog.args[0].includes('xoxe.xoxp-secret-token'), 'expected raw user token to not appear in logs'); + assert.ok(!resultLog.args[0].includes('xoxe-secret-token'), 'expected raw user token to not appear in logs'); + }); }); describe('has an option to override the Axios timeout value', () => { diff --git a/packages/web-api/src/WebClient.ts b/packages/web-api/src/WebClient.ts index 3a5185a33..e2ab7329b 100644 --- a/packages/web-api/src/WebClient.ts +++ b/packages/web-api/src/WebClient.ts @@ -371,7 +371,7 @@ export class WebClient extends Methods { headers, ); const result = await this.buildResult(response); - this.logger.debug(`http request result: ${JSON.stringify(result)}`); + this.logger.debug(`http request result: ${JSON.stringify(redact(result))}`); // log warnings in response metadata if (result.response_metadata !== undefined && result.response_metadata.warnings !== undefined) { @@ -1072,7 +1072,7 @@ export function buildThreadTsWarningMessage(method: string): string { * @param body * @returns */ -function redact(body: Record): Record { +function redact(body: object): Record { // biome-ignore lint/suspicious/noExplicitAny: objects can be anything const flattened = Object.entries(body).map<[string, any] | []>(([key, value]) => { // no value provided @@ -1090,8 +1090,8 @@ function redact(body: Record): Record { // when value is buffer or stream we can avoid logging it if (Buffer.isBuffer(value) || isStream(value)) { serializedValue = '[[BINARY VALUE OMITTED]]'; - } else if (typeof value !== 'string' && typeof value !== 'number' && typeof value !== 'boolean') { - serializedValue = JSON.stringify(value); + } else if (typeof value === 'object') { + serializedValue = redact(value); } return [key, serializedValue]; });