Skip to content

Commit 70f126d

Browse files
committed
Keep the API token out of the errors the client throws
`ApiError` stored the options of the failed query verbatim, so `error.options.token` held the API token in clear text — and travelled with the error wherever it went: into `console.error()` output shipped to log aggregators, into error trackers, and into any HTTP handler that caught it and echoed it back to its caller. Our own tech starters did the latter, which turned a 500 into a way to read the token. The token is now replaced by `[REDACTED, ending in abcd]`, which still tells two tokens apart while debugging; the real one only ever reaches the `Authorization` header. For the same reason `query`, `options` and `response` are no longer enumerable: reading `error.options` explicitly works exactly as before, as the README documents, but the details of the failed query stop travelling through `JSON.stringify()`, object spread or `serialize-error` by accident. Claude-Session: https://claude.ai/code/session_01VQGrgtFYHo3vSXkifJ1bom
1 parent f61c569 commit 70f126d

4 files changed

Lines changed: 76 additions & 3 deletions

File tree

.changeset/quiet-tokens-hide.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@datocms/cda-client': patch
3+
---
4+
5+
Keep the API token out of the errors the client throws
6+
7+
`ApiError` stored the options of the failed query verbatim, so `error.options.token`
8+
held the API token in clear text — and travelled with the error into
9+
`console.error()` output shipped to log aggregators, into error trackers, and
10+
into any HTTP handler that echoed the error back to its caller.
11+
12+
The token is now replaced by `[REDACTED, ending in abcd]`, which still tells two
13+
tokens apart while debugging; the real one only ever reaches the `Authorization`
14+
header. For the same reason `query`, `options` and `response` are now
15+
non-enumerable: reading `error.options` explicitly works exactly as before, but
16+
the details of the failed query no longer travel through `JSON.stringify()`,
17+
object spread or `serialize-error` by accident.

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,9 @@ try {
260260
console.log(result);
261261
} catch (e) {
262262
if (e instanceof ApiError) {
263-
// Information about the failed request
263+
// Information about the failed request. The API token is redacted from
264+
// `e.options`: an error tends to end up in logs and error trackers, which
265+
// are no place for a token.
264266
console.log(e.query);
265267
console.log(e.options);
266268

src/ApiError.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,41 @@ export type ApiErrorResponse = {
1717
*/
1818
const API_ERROR = Symbol.for('@datocms/cda-client:ApiError');
1919

20+
/**
21+
* Returns a copy of the options with the API token blanked out.
22+
*
23+
* Errors travel: they get logged, serialized, sent to error trackers, and
24+
* occasionally echoed back to an HTTP client by a route handler that catches
25+
* them. None of those places is a good home for a token. The real one only ever
26+
* reaches the `Authorization` header of the request.
27+
*/
28+
function redactToken<Options extends BuildRequestHeadersOptions>(
29+
options: Options,
30+
): Options {
31+
if (!options.token) {
32+
return options;
33+
}
34+
35+
return {
36+
...options,
37+
// The last 4 characters are enough to tell two tokens apart while
38+
// debugging, and useless to whoever gets hold of the log.
39+
token: `[REDACTED, ending in ${options.token.slice(-4)}]`,
40+
};
41+
}
42+
43+
/**
44+
* Hides properties from anything that walks own enumerable keys —
45+
* `console.error()`, `JSON.stringify()`, object spread, `serialize-error`,
46+
* error trackers. Reading `error.options` explicitly keeps working exactly as
47+
* before; the data just stops travelling by accident.
48+
*/
49+
function hideProperties(target: object, keys: string[]) {
50+
for (const key of keys) {
51+
Object.defineProperty(target, key, { enumerable: false });
52+
}
53+
}
54+
2055
export class ApiError extends Error {
2156
public query: string;
2257
public options: BuildRequestHeadersOptions;
@@ -85,6 +120,8 @@ export class ApiError extends Error {
85120
this.name = 'ApiError';
86121
this.response = response;
87122
this.query = query;
88-
this.options = options;
123+
this.options = redactToken(options);
124+
125+
hideProperties(this, ['query', 'options', 'response']);
89126
}
90127
}

src/__test__/ApiError.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,24 @@ describe('ApiError', () => {
4848

4949
expect(error.response.status).toBe(422);
5050
expect(error.query).toBe('{ allArticles { id } }');
51-
expect(error.options.token).toBe('fake-token');
51+
});
52+
53+
it('keeps the API token out of the error', () => {
54+
const error = buildError();
55+
56+
expect(error.options.token).toBe('[REDACTED, ending in oken]');
57+
expect(JSON.stringify(error)).not.toContain('fake-token');
58+
});
59+
60+
// `console.error()`, `serialize-error` and most error trackers walk own
61+
// enumerable keys: the details of the failed request must not be among them.
62+
it('does not travel through incidental serialization', () => {
63+
const error = buildError();
64+
65+
expect(Object.keys(error)).not.toContain('options');
66+
expect(Object.keys(error)).not.toContain('query');
67+
expect(Object.keys(error)).not.toContain('response');
68+
expect(JSON.stringify({ ...error })).not.toContain('allArticles');
5269
});
5370

5471
// The package ships parallel CJS and ESM builds, so a bundler can load two

0 commit comments

Comments
 (0)