From d097cd635e80c56096f0563965db0fd2b8b0c304 Mon Sep 17 00:00:00 2001 From: "@zimeg" Date: Mon, 24 Jun 2024 19:07:36 -0700 Subject: [PATCH 1/4] socket-mode(fix): redact ephemeral tokens and secrets from debug logs --- packages/socket-mode/package.json | 2 +- .../socket-mode/src/SocketModeClient.spec.ts | 89 +++++++++++++++++++ packages/socket-mode/src/SocketModeClient.ts | 43 +++++++-- 3 files changed, 127 insertions(+), 7 deletions(-) diff --git a/packages/socket-mode/package.json b/packages/socket-mode/package.json index 8d4ce02e9..276135e86 100644 --- a/packages/socket-mode/package.json +++ b/packages/socket-mode/package.json @@ -1,6 +1,6 @@ { "name": "@slack/socket-mode", - "version": "2.0.0", + "version": "2.0.1", "description": "Official library for using the Slack Platform's Socket Mode API", "author": "Slack Technologies, LLC", "license": "MIT", diff --git a/packages/socket-mode/src/SocketModeClient.spec.ts b/packages/socket-mode/src/SocketModeClient.spec.ts index 118d5314b..f6b08708f 100644 --- a/packages/socket-mode/src/SocketModeClient.spec.ts +++ b/packages/socket-mode/src/SocketModeClient.spec.ts @@ -275,6 +275,95 @@ describe('SocketModeClient', () => { assert.equal(passedEnvelopeId, envelopeId); }); }); + + describe('redact', () => { + let spies: sinon.SinonSpy; + + beforeEach(() => { + spies = sinon.spy(); + }); + + afterEach(() => { + sinon.reset(); + }); + + it('should remove tokens and secrets from incoming messages', async () => { + const logger = new ConsoleLogger(); + logger.debug = spies; + const client = new SocketModeClient({ + appToken: 'xapp-example-001', + logger, + }); + + const input = { + type: 'hello', + payload: { + example: '12', + token: 'xoxb-example-001', + event: { + bot_access_token: 'xwfp-redaction-001', + }, + values: { + secret: 'abcdef', + }, + inputs: [ + { id: 'example', mock: 'testing' }, + { id: 'mocking', mock: 'testure' }, + ], + }, + }; + const expected = { + type: 'hello', + payload: { + example: '12', + token: '[[REDACTED]]', + event: { + bot_access_token: '[[REDACTED]]', + }, + values: { + secret: '[[REDACTED]]', + }, + inputs: [ + { id: 'example', mock: 'testing' }, + { id: 'mocking', mock: 'testure' }, + ], + }, + }; + + client.emit('message', JSON.stringify(input)); + assert(spies.called); + assert(spies.calledWith(`Received a message on the WebSocket: ${JSON.stringify(expected)}`)); + }); + + it('should respond with undefined when attempting to redact undefined', async () => { + const logger = new ConsoleLogger(); + logger.debug = spies; + const client = new SocketModeClient({ + appToken: 'xapp-example-001', + logger, + }); + + const input = undefined; + + client.emit('message', JSON.stringify(input)); + assert(spies.called); + assert(spies.calledWith('Received a message on the WebSocket: undefined')); + }); + + it('should print the incoming data if parsing errors happen', async () => { + const logger = new ConsoleLogger(); + logger.debug = spies; + logger.error = spies; + const client = new SocketModeClient({ + appToken: 'xapp-example-001', + logger, + }); + + client.emit('message', '{"number":'); + assert(spies.called); + assert(spies.calledWith('Received a message on the WebSocket: {"number":')); + }); + }); }); }); diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index c03023094..08a72f526 100644 --- a/packages/socket-mode/src/SocketModeClient.ts +++ b/packages/socket-mode/src/SocketModeClient.ts @@ -284,8 +284,6 @@ export class SocketModeClient extends EventEmitter { this.logger.debug('Unexpected binary message received, ignoring.'); return; } - const payload = data.toString(); - this.logger.debug(`Received a message on the WebSocket: ${payload}`); // Parse message into slack event let event: { @@ -299,10 +297,13 @@ export class SocketModeClient extends EventEmitter { accepts_response_payload?: boolean; // type: events_api, slash_commands, interactive }; + const payload = data?.toString(); try { event = JSON.parse(payload); + this.logger.debug(`Received a message on the WebSocket: ${JSON.stringify(SocketModeClient.redact(event))}`); } catch (parseError) { // Prevent application from crashing on a bad message, but log an error to bring attention + this.logger.debug(`Received a message on the WebSocket: ${payload}`); this.logger.debug( `Unable to parse an incoming WebSocket message (will ignore): ${parseError}, ${payload}`, ); @@ -325,7 +326,7 @@ export class SocketModeClient extends EventEmitter { // Define Ack, a helper method for acknowledging events incoming from Slack const ack = async (response: Record): Promise => { if (this.logger.getLevel() === LogLevel.DEBUG) { - this.logger.debug(`Calling ack() - type: ${event.type}, envelope_id: ${event.envelope_id}, data: ${JSON.stringify(response)}`); + this.logger.debug(`Calling ack() - type: ${event.type}, envelope_id: ${event.envelope_id}, data: ${JSON.stringify(SocketModeClient.redact(response))}`); } await this.send(event.envelope_id, response); }; @@ -386,9 +387,8 @@ export class SocketModeClient extends EventEmitter { } else { this.emit('outgoing_message', message); - const flatMessage = JSON.stringify(message); - this.logger.debug(`Sending a WebSocket message: ${flatMessage}`); - this.websocket.send(flatMessage, (error) => { + this.logger.debug(`Sending a WebSocket message: ${JSON.stringify(SocketModeClient.redact(message))}`); + this.websocket.send(JSON.stringify(message), (error) => { if (error) { this.logger.error(`Failed to send a WebSocket message (error: ${error})`); return reject(websocketErrorWithOriginal(error)); @@ -398,6 +398,37 @@ export class SocketModeClient extends EventEmitter { } }); } + + /** + * Removes secrets and tokens from socket request and response objects + * before logging. + * @param body - the object with values for redaction. + * @returns the same object with redacted values. + */ + private static redact(body?: Record): Record | unknown[] | undefined { + if (body === undefined || body === null) { + return body; + } + const records = Object.create(body); + if (Array.isArray(body)) { + return body.map((item) => ( + (typeof item === 'object' && item !== null) ? + SocketModeClient.redact(item as Record) : + item + )); + } + Object.keys(body).forEach((key: string) => { + const value = body[key]; + if (typeof value === 'object' && value !== null) { + records[key] = SocketModeClient.redact(value as Record); + } else if (key.match(/.*token.*/) !== null || key.match(/secret/)) { + records[key] = '[[REDACTED]]'; + } else { + records[key] = value; + } + }); + return records; + } } /* Instrumentation */ From b4718d802a8133e4ddcfab30d54fa3b8d98c4b5a Mon Sep 17 00:00:00 2001 From: "@zimeg" Date: Mon, 24 Jun 2024 20:23:28 -0700 Subject: [PATCH 2/4] chore(release): revert bump to @slack/socket-mode@2.0.1 --- packages/socket-mode/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/socket-mode/package.json b/packages/socket-mode/package.json index 276135e86..8d4ce02e9 100644 --- a/packages/socket-mode/package.json +++ b/packages/socket-mode/package.json @@ -1,6 +1,6 @@ { "name": "@slack/socket-mode", - "version": "2.0.1", + "version": "2.0.0", "description": "Official library for using the Slack Platform's Socket Mode API", "author": "Slack Technologies, LLC", "license": "MIT", From e7a11660b5caf71ae9a7e71d689fcad4f6c2562c Mon Sep 17 00:00:00 2001 From: "@zimeg" Date: Mon, 24 Jun 2024 20:26:33 -0700 Subject: [PATCH 3/4] style: rename the object records to be the singular record --- packages/socket-mode/src/SocketModeClient.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index 08a72f526..786be82ad 100644 --- a/packages/socket-mode/src/SocketModeClient.ts +++ b/packages/socket-mode/src/SocketModeClient.ts @@ -409,7 +409,7 @@ export class SocketModeClient extends EventEmitter { if (body === undefined || body === null) { return body; } - const records = Object.create(body); + const record = Object.create(body); if (Array.isArray(body)) { return body.map((item) => ( (typeof item === 'object' && item !== null) ? @@ -420,14 +420,14 @@ export class SocketModeClient extends EventEmitter { Object.keys(body).forEach((key: string) => { const value = body[key]; if (typeof value === 'object' && value !== null) { - records[key] = SocketModeClient.redact(value as Record); + record[key] = SocketModeClient.redact(value as Record); } else if (key.match(/.*token.*/) !== null || key.match(/secret/)) { - records[key] = '[[REDACTED]]'; + record[key] = '[[REDACTED]]'; } else { - records[key] = value; + record[key] = value; } }); - return records; + return record; } } From 3147e08ad70d8dde91e01143be6786a9dab72173 Mon Sep 17 00:00:00 2001 From: "@zimeg" Date: Tue, 25 Jun 2024 13:43:23 -0700 Subject: [PATCH 4/4] fix: improve nested record typings and remove a casting --- packages/socket-mode/src/SocketModeClient.ts | 23 ++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/socket-mode/src/SocketModeClient.ts b/packages/socket-mode/src/SocketModeClient.ts index 786be82ad..9833c78b1 100644 --- a/packages/socket-mode/src/SocketModeClient.ts +++ b/packages/socket-mode/src/SocketModeClient.ts @@ -30,6 +30,21 @@ enum State { Authenticated = 'authenticated', } +/** + * Recursive definition for what a value might contain. + */ +interface Nesting { + [key: string]: NestedRecord | unknown; +} + +/** + * Recursive definiton for possible JSON object values. + * + * FIXME: Prefer using a circular reference if allowed: + * Record | NestedRecord[] + */ +type NestedRecord = Nesting | NestedRecord[]; + /** * A Socket Mode Client allows programs to communicate with the * [Slack Platform's Events API](https://api.slack.com/events-api) over WebSocket connections. @@ -405,22 +420,22 @@ export class SocketModeClient extends EventEmitter { * @param body - the object with values for redaction. * @returns the same object with redacted values. */ - private static redact(body?: Record): Record | unknown[] | undefined { - if (body === undefined || body === null) { + private static redact(body: NestedRecord): NestedRecord { + if (body === undefined) { return body; } const record = Object.create(body); if (Array.isArray(body)) { return body.map((item) => ( (typeof item === 'object' && item !== null) ? - SocketModeClient.redact(item as Record) : + SocketModeClient.redact(item) : item )); } Object.keys(body).forEach((key: string) => { const value = body[key]; if (typeof value === 'object' && value !== null) { - record[key] = SocketModeClient.redact(value as Record); + record[key] = SocketModeClient.redact(value as NestedRecord); } else if (key.match(/.*token.*/) !== null || key.match(/secret/)) { record[key] = '[[REDACTED]]'; } else {