From af817788c37bc5610bfba8aaf63608d5e84dba29 Mon Sep 17 00:00:00 2001 From: Codex Date: Fri, 7 Aug 2026 01:04:02 +1000 Subject: [PATCH 1/2] fix: redact sensitive API error headers --- src/model/ApiError.ts | 24 ++++++++++++++++++++++-- src/test/apiError.spec.ts | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/model/ApiError.ts b/src/model/ApiError.ts index 8b547985..ec4bd13e 100644 --- a/src/model/ApiError.ts +++ b/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,7 +54,7 @@ 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, @@ -44,7 +62,9 @@ export class ApiError { 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 } } diff --git a/src/test/apiError.spec.ts b/src/test/apiError.spec.ts index 2e1ff9f9..a90a0186 100644 --- a/src/test/apiError.spec.ts +++ b/src/test/apiError.spec.ts @@ -1,7 +1,12 @@ import { ApiError } from '../model/ApiError'; 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 +17,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 +49,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 +68,8 @@ describe('ApiError', () => { }, headers: { 'content-type': 'application/json', + 'set-cookie': 'session=response-cookie-that-must-not-leak', + 'X-Rate-Limit-Remaining': '59', }, }, request: { @@ -63,7 +84,10 @@ 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: { @@ -71,6 +95,7 @@ describe('ApiError', () => { }, headers: { 'content-type': 'application/json', + 'X-Rate-Limit-Remaining': '59', }, request: { url: { From a644f09cf5aa1b8fc2b497d57e511284d52d4dee Mon Sep 17 00:00:00 2001 From: Ryan Duguid <152749594+ryanduguid@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:04:50 +1000 Subject: [PATCH 2/2] fix: route XeroClient token and query rejections through the redactor tokenRequest() and queryApi() rejected the raw AxiosError, bypassing the ApiError redactor entirely. AxiosError.toJSON() serialises error.config, so a caller doing JSON.stringify(err) recovered the Basic base64(clientId: clientSecret) header and the refresh_token body from the token request, and the Bearer access token from queryApi (which backs the public updateTenants()). Both catch blocks now build an ApiError and reject the redacted, serialised result, matching the pattern already used by every generated API client. Adds nock-driven integration tests that fail against the unfixed client, covering both paths with real axios failures rather than object literals. --- src/XeroClient.ts | 7 +++-- src/test/apiError.spec.ts | 65 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/XeroClient.ts b/src/XeroClient.ts index 100e1817..c933c46a 100644 --- a/src/XeroClient.ts +++ b/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())); } }); } diff --git a/src/test/apiError.spec.ts b/src/test/apiError.spec.ts index a90a0186..da820b92 100644 --- a/src/test/apiError.spec.ts +++ b/src/test/apiError.spec.ts @@ -1,4 +1,6 @@ import { ApiError } from '../model/ApiError'; +import { XeroClient } from '../XeroClient'; +import nock from 'nock'; describe('ApiError', () => { it('removes sensitive headers from serialized generated errors', () => { @@ -114,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): Promise => 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); + }); +});