fix: redact sensitive API error headers

src/XeroClient.ts

@@ -1,5 +1,6 @@
 import { Client, Issuer, TokenSet, TokenSetParameters, custom } from 'openid-client';
 import * as xero from './gen/api';
+import { ApiError } from './model/ApiError';
 const axios = require('axios');
 import http = require('http');
 
@@ -220,7 +221,8 @@ export class XeroClient {
         }
       }
       catch (error) {
-        reject(error)
+        const errorResponse = new ApiError(error);
+        reject(JSON.stringify(errorResponse.generateError()));
       }
     });
   }
@@ -262,7 +264,8 @@ export class XeroClient {
         }
       }
       catch (error) {
-        reject(error)
+        const errorResponse = new ApiError(error);
+        reject(JSON.stringify(errorResponse.generateError()));
       }
     });
   }

src/model/ApiError.ts

@@ -23,6 +23,24 @@ interface ErrorResponse {
 	body: any
 }
 
+const SENSITIVE_HEADER_NAME = /authorization|cookie|api[-_]?key|token|secret|password/i;
+
+function sanitizeHeaders(headers: any): any {
+	const safeHeaders: any = {};
+
+	if (!headers || typeof headers !== 'object') {
+		return safeHeaders;
+	}
+
+	Object.keys(headers).forEach((headerName) => {
+		if (!SENSITIVE_HEADER_NAME.test(headerName)) {
+			safeHeaders[headerName] = headers[headerName];
+		}
+	});
+
+	return safeHeaders;
+}
+
 export class ApiError {
 
 	statusCode: number
@@ -36,15 +54,17 @@ export class ApiError {
 
 		this.statusCode = response.status || 0;
 		this.body = response.data ?? axiosError.message;
-		this.headers = response.headers || {};
+		this.headers = sanitizeHeaders(response.headers);
 		this.request = {
 			url: {
 				protocol: request.protocol,
 				port: request.agent?.defaultPort || request.socket?.localPort,
 				host: request.host,
 				path: request.path,
 			},
-			headers: typeof request.getHeaders === 'function' ? request.getHeaders() : {},
+			headers: sanitizeHeaders(
+				typeof request.getHeaders === 'function' ? request.getHeaders() : undefined
+			),
 			method: request.method
 		}
 	}

src/test/apiError.spec.ts

@@ -1,7 +1,14 @@
 import { ApiError } from '../model/ApiError';
+import { XeroClient } from '../XeroClient';
+import nock from 'nock';
 
 describe('ApiError', () => {
-	it('handles axios errors without a response', () => {
+	it('removes sensitive headers from serialized generated errors', () => {
+		const authorization = 'Bearer access-token-that-must-not-leak';
+		const cookie = 'session=session-cookie-that-must-not-leak';
+		const proxyAuthorization = 'Basic proxy-credentials-that-must-not-leak';
+		const apiKey = 'api-key-that-must-not-leak';
+		const clientSecret = 'client-secret-that-must-not-leak';
 		const apiError = new ApiError({
 			message: 'Network Error',
 			request: {
@@ -12,13 +19,26 @@ describe('ApiError', () => {
 				host: 'api.xero.com',
 				path: '/api.xro/2.0/Invoices',
 				getHeaders: () => ({
-					authorization: 'Bearer token',
+					authorization,
+					Cookie: cookie,
+					'Proxy-Authorization': proxyAuthorization,
+					'X-API-Key': apiKey,
+					'X-Client-Secret': clientSecret,
+					Accept: 'application/json',
+					'X-Request-Id': 'request-id',
 				}),
 				method: 'GET',
 			},
 		});
 
-		expect(apiError.generateError()).toEqual({
+		const serializedError = JSON.stringify(apiError.generateError());
+
+		expect(serializedError).not.toContain(authorization);
+		expect(serializedError).not.toContain(cookie);
+		expect(serializedError).not.toContain(proxyAuthorization);
+		expect(serializedError).not.toContain(apiKey);
+		expect(serializedError).not.toContain(clientSecret);
+		expect(JSON.parse(serializedError)).toEqual({
 			response: {
 				statusCode: 0,
 				body: 'Network Error',
@@ -31,7 +51,8 @@ describe('ApiError', () => {
 						path: '/api.xro/2.0/Invoices',
 					},
 					headers: {
-						authorization: 'Bearer token',
+						Accept: 'application/json',
+						'X-Request-Id': 'request-id',
 					},
 					method: 'GET',
 				},
@@ -49,6 +70,8 @@ describe('ApiError', () => {
 				},
 				headers: {
 					'content-type': 'application/json',
+					'set-cookie': 'session=response-cookie-that-must-not-leak',
+					'X-Rate-Limit-Remaining': '59',
 				},
 			},
 			request: {
@@ -63,14 +86,18 @@ describe('ApiError', () => {
 			},
 		});
 
-		expect(apiError.generateError()).toEqual({
+		const serializedError = JSON.stringify(apiError.generateError());
+
+		expect(serializedError).not.toContain('response-cookie-that-must-not-leak');
+		expect(JSON.parse(serializedError)).toEqual({
 			response: {
 				statusCode: 401,
 				body: {
 					error: 'invalid_token',
 				},
 				headers: {
 					'content-type': 'application/json',
+					'X-Rate-Limit-Remaining': '59',
 				},
 				request: {
 					url: {
@@ -89,3 +116,66 @@ describe('ApiError', () => {
 		});
 	});
 });
+
+describe('ApiError integration with XeroClient', () => {
+	const clientId = 'client-id-that-must-not-leak';
+	const clientSecret = 'client-secret-that-must-not-leak';
+	const refreshToken = 'refresh-token-that-must-not-leak';
+	const accessToken = 'access-token-that-must-not-leak';
+
+	const buildClient = () => new XeroClient({
+		clientId,
+		clientSecret,
+		redirectUris: ['http://localhost:5000/callback'],
+		scopes: 'openid profile email offline_access'.split(' ')
+	});
+
+	const rejectionOf = (promise: Promise<any>): Promise<any> => promise.then(
+		() => { throw new Error('expected the request to reject'); },
+		(error) => error
+	);
+
+	afterEach(() => {
+		nock.cleanAll();
+	});
+
+	it('redacts the basic auth credentials and refresh token when the token request fails', async () => {
+		nock('https://identity.xero.com')
+			.post('/connect/token')
+			.reply(401, { error: 'invalid_grant' });
+
+		const rejection = await rejectionOf(
+			buildClient().refreshWithRefreshToken(clientId, clientSecret, refreshToken)
+		);
+
+		const serializedError = JSON.stringify(rejection);
+		const basicCredentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64');
+
+		expect(serializedError).not.toContain(basicCredentials);
+		expect(serializedError).not.toContain(clientSecret);
+		expect(serializedError).not.toContain(refreshToken);
+		expect(JSON.parse(rejection).response.statusCode).toEqual(401);
+	});
+
+	it('redacts the bearer token when the connections query fails', async () => {
+		nock('https://api.xero.com')
+			.get('/connections')
+			.reply(401, { Title: 'Unauthorized' });
+
+		const client = buildClient();
+		client.setTokenSet({
+			access_token: accessToken,
+			refresh_token: refreshToken,
+			token_type: 'Bearer',
+			expires_at: 1231231234
+		});
+
+		const rejection = await rejectionOf(client.updateTenants());
+
+		const serializedError = JSON.stringify(rejection);
+
+		expect(serializedError).not.toContain(accessToken);
+		expect(serializedError).not.toContain(refreshToken);
+		expect(JSON.parse(rejection).response.statusCode).toEqual(401);
+	});
+});