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

socket-mode(fix): redact ephemeral tokens and secrets from debug logs #1831

Open
wants to merge 4 commits into
base: socket-mode-1.3.x
Choose a base branch
from
Open
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
93 changes: 93 additions & 0 deletions packages/socket-mode/src/SocketModeClient.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const { assert } = require('chai');
const sinon = require('sinon');
const { ConsoleLogger } = require('@slack/logger');
const { SocketModeClient } = require('./SocketModeClient');

describe('SocketModeClient', () => {
Expand Down Expand Up @@ -250,6 +252,97 @@ describe('SocketModeClient', () => {
assert.equal(passedEnvelopeId, '57d6a792-4d35-4d0b-b6aa-3361493e1caf');
});
});

describe('redact', () => {
/** @type sinon.SinonSpy */
let spies;

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.onWebSocketMessage({ data: 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;
logger.error = spies;
const client = new SocketModeClient({
appToken: 'xapp-example-001',
logger,
});

const input = undefined;

client.onWebSocketMessage({ data: 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.onWebSocketMessage({ data: `{"number":` });
assert(spies.called);
assert(spies.calledWith(`Received a message on the WebSocket: {"number":`));
});
});
});
});

Expand Down
57 changes: 51 additions & 6 deletions packages/socket-mode/src/SocketModeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,21 @@ enum Event {
UnableToSocketModeStart = 'unable_to_socket_mode_start',
}

/**
* 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<string, NestedRecord> | NestedRecord[]
*/
type NestedRecord = Nesting | NestedRecord[];

/**
* An Socket Mode Client allows programs to communicate with the
* [Slack Platform's Events API](https://api.slack.com/events-api) over WebSocket connections.
Expand Down Expand Up @@ -417,9 +432,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 !== undefined && error !== null) {
this.logger.error(`Failed to send a WebSocket message (error: ${error.message})`);
return reject(websocketErrorWithOriginal(error));
Expand Down Expand Up @@ -700,8 +714,6 @@ export class SocketModeClient extends EventEmitter {
* This will parse the payload and dispatch the relevant events for each incoming message.
*/
protected async onWebSocketMessage({ data }: { data: string }): Promise<void> {
this.logger.debug(`Received a message on the WebSocket: ${data}`);

// Parse message into slack event
let event: {
type: string;
Expand All @@ -716,9 +728,11 @@ export class SocketModeClient extends EventEmitter {

try {
event = JSON.parse(data);
this.logger.debug(`Received a message on the WebSocket: ${JSON.stringify(SocketModeClient.redact(event))}`);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (parseError: any) {
// Prevent application from crashing on a bad message, but log an error to bring attention
this.logger.debug(`Received a message on the WebSocket: ${data}`);
this.logger.error(
`Unable to parse an incoming WebSocket message: ${parseError.message}`,
);
Expand All @@ -741,7 +755,7 @@ export class SocketModeClient extends EventEmitter {
// Define Ack
const ack = async (response: Record<string, unknown>): Promise<void> => {
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);
};
Expand Down Expand Up @@ -779,6 +793,37 @@ export class SocketModeClient extends EventEmitter {
accepts_response_payload: event.accepts_response_payload,
});
}

/**
* 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: 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) :
item
));
}
Object.keys(body).forEach((key: string) => {
const value = body[key];
if (typeof value === 'object' && value !== null) {
record[key] = SocketModeClient.redact(value as NestedRecord);
} else if (key.match(/.*token.*/) !== null || key.match(/secret/)) {
record[key] = '[[REDACTED]]';
} else {
record[key] = value;
}
});
return record;
}
}

/* Instrumentation */
Expand Down
Loading