google-auth-library: getErrorFromOAuthErrorResponse() copies stack as non-writable, breaking error decoration in consumers

Open Beginner friendly
#9,155 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
74/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
nodejs, typescript

Research direction

Start in core/packages/google-auth-library-nodejs/src/auth/oauth2common.ts at getErrorFromOAuthErrorResponse(), then run the reproduction against an external_account OAuth/STS error. The change is done when the resulting error keeps stack writable and non-enumerable while preserving the intended error information, and strict-mode stack decoration no longer throws.

Written by the indexing model from the issue text.

Description

Environment
  • google-auth-library: 11.0.2 (also reproduced on 10.6.2)
  • Node.js: 20.19.5
  • Credential type: external_account (Workload Identity Federation, file-sourced OIDC subject token)
Summary

getErrorFromOAuthErrorResponse()core/packages/google-auth-library-nodejs/src/auth/oauth2common.ts:253 — builds a new Error from an OAuth/STS error response and copies the original error's own properties onto it — including stack — as non-writable:

const keys = Object.keys(err);
if (err.stack) {
    // Copy error.stack if available.
    keys.push('stack');
}
keys.forEach(key => {
    if (key !== 'message') {
        Object.defineProperty(newError, key, {
            value: err[key],
            writable: false,   // <-- makes `stack` read-only
            enumerable: true,
        });
    }
});

Every error surfaced from an STS/OAuth error response therefore has stack with { writable: false, enumerable: true, configurable: true }, whereas a normal Error has a writable stack.

Why it matters

Appending to error.stack to attach causal context is a common pattern, and it is normally safe. Against these errors it throws in strict mode (and silently discards the write in sloppy mode). Both outcomes are bad, and the strict-mode one is worse: the real authentication failure is replaced by a TypeError naming an unrelated library.

A concrete case in the same ecosystem — @google-cloud/firestore's wrapError() (build/src/util.js) does:

err.stack += '\nCaused by: ' + stack;

Its module is emitted with "use strict", and it is invoked from stream 'error' handlers. So when a Firestore operation fails because a federated credential was rejected, the TypeError is thrown inside an EventEmitter emit, escapes the promise chain, and becomes an uncaughtException rather than a rejected promise:

TypeError: Cannot assign to read only property 'stack' of object 'Error: Error code invalid_grant: ID Token issued at <ts> is stale to sign-in.'
    at wrapError (/app/node_modules/@google-cloud/firestore/build/src/util.js:213:15)
    at Transform.emit (node:events:524:28)
    at emitErrorNT (node:internal/streams/destroy:169:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

The underlying problem was an expired OIDC subject token. Nothing in what the process reported says so. I've filed the counterpart issue for the Firestore half (same monorepo) asking them to guard the assignment: #9154. But the read-only stack looks worth revisiting here too, since any consumer decorating errors this way will hit it, and stack is conventionally writable.

Also note enumerable: true on the copied properties: stack is normally non-enumerable, so this additionally makes it show up in Object.keys(err), for...in, and JSON.stringify output of the error's own properties.

Reproduction
// npm i google-auth-library
const { getErrorFromOAuthErrorResponse } = require('google-auth-library/build/src/auth/oauth2common.js');

const authError = getErrorFromOAuthErrorResponse(
  { error: 'invalid_grant', error_description: 'ID Token is stale to sign-in.' },
  new Error('underlying transport failure'),
);

console.log(Object.getOwnPropertyDescriptor(authError, 'stack'));
// => { value: '...', writable: false, enumerable: true, configurable: true }

// Sloppy mode: the write is silently discarded.
(function () { authError.stack += '\nCaused by: x'; })();

// Strict mode (what compiled TS/ESM consumers run in): throws.
(function () { 'use strict'; authError.stack += '\nCaused by: x'; })();
// => TypeError: Cannot assign to read only property 'stack' of object
//    'Error: Error code invalid_grant: ID Token is stale to sign-in.'

In situ this is reached via a normal external_account credential whose subject token has expired: the STS response is an invalid_grant, getErrorFromOAuthErrorResponse() converts it, and any downstream err.stack += then crashes.

Suggested fix

Leave stack writable (and non-enumerable) when copying, so these errors behave like ordinary ones:

keys.forEach(key => {
    if (key !== 'message') {
        Object.defineProperty(newError, key, {
            value: err[key],
            writable: key === 'stack',
            enumerable: key !== 'stack',
            configurable: true,
        });
    }
});

Simply not copying stack at all would also work — newError already has its own accurate stack — though that loses the original call site. Alternatively newError.cause = err conveys the same information using the standard mechanism and keeps stack untouched.

Dominant language
TypeScript
Stars
3.2k
Forks
714
Avg merge
2d 13h
Merged PRs (30d)
106

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from googleapis/google-cloud-node

All issues in googleapis/google-cloud-node

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.