fix(auth): keep stack writable on errors built from OAuth error responses

[Om-singhaI]

fix(auth): keep stack writable on errors built from OAuth error responses

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #9155 🦕

Problem

getErrorFromOAuthErrorResponse() in core/packages/google-auth-library-nodejs/src/auth/oauth2common.ts builds a new Error from an OAuth or STS error response and copies the original error's own properties onto it with Object.defineProperty(..., {writable: false, enumerable: true}). It also pushes stack onto the list of keys to copy, so the resulting error ends up with a read only, enumerable stack. A regular Error has a writable, non enumerable stack.

Both call sites (StsCredentials.exchangeToken() in stscredentials.ts and ExternalAccountAuthorizedUserHandler in externalAccountAuthorizedUserClient.ts) pass the original GaxiosError, so every token exchange failure surfaced by external_account credentials (Workload Identity Federation) has this shape.

Consumers that append causal context to error.stack hit it. Compiled TypeScript modules run under "use strict", where the assignment throws TypeError: Cannot assign to read only property 'stack' instead of being silently dropped. @google-cloud/firestore does exactly this in wrapError() (handwritten/firestore/dev/src/util.ts, err.stack += '\nCaused by: ' + stack;) from stream error handlers, so a rejected federated credential turns into an uncaughtException that names Firestore and hides the real invalid_grant message. The consumer side is tracked in #9154; this PR fixes the root cause in the auth library so the errors it produces behave like ordinary errors.

Reproduction on main (48e0941), using the compiled build/src/auth/oauth2common.js in a strict mode script:

descriptor: {"value":"<stack string>","writable":false,"enumerable":true,"configurable":true}
enumerable keys: [ 'stack' ]
strict mode append threw: TypeError: Cannot assign to read only property 'stack' of object 'Error: Error code invalid_grant: ID Token is stale to sign-in.'

Change

In the property copy loop, stack is now defined with writable: true, enumerable: false and configurable: true, which is the shape it has on a freshly constructed Error. All other copied properties (code, name, response, and so on) keep the existing writable: false, enumerable: true definition with configurable left unset (so false, as before), and message is still never overwritten. The value of stack is still copied from the original error, so the existing "should preserve the original error properties" test is unchanged and still passes.

Same script after the change:

descriptor: {"value":"<stack string>","writable":true,"enumerable":false,"configurable":true}
enumerable keys: []
strict mode append: OK

A new unit test in the getErrorFromOAuthErrorResponse block of test/test.oauth2common.ts asserts the property descriptor (writable === true, enumerable === false, configurable === true), that actualError.stack += ... does not throw (the compiled test module is strict mode), and that the appended text is present afterwards.

Why configurable is set explicitly (review follow up)

Review feedback asked for configurable to be spelled out for stack rather than relying on the descriptor default, and for the test to assert it. The attribute is now configurable: key === 'stack' next to the existing writable and enumerable lines.

For stack this makes no runtime difference: new Error() already owns a configurable stack, and Object.defineProperty on an existing property keeps the attributes that the descriptor omits, so configurable was already true. The descriptor printed by the reproduction script is identical before and after this follow up:

before: {"value":"<stack string>","writable":true,"enumerable":false,"configurable":true}
after:  {"value":"<stack string>","writable":true,"enumerable":false,"configurable":true}

Spelling it out means the behaviour no longer depends on that defineProperty rule. For every other copied key the expression evaluates to false, which is exactly what the omitted attribute meant for those new properties, so their descriptors are byte for byte the same as before (for example code is still {"writable":false,"enumerable":true,"configurable":false}). The test assertion is meaningful: with configurable: false forced for stack in the compiled output, the new test fails with AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: false !== true.

Verification

All runs used tsc -p . --sourceMap followed by mocha on the compiled output inside core/packages/google-auth-library-nodejs, with Node 25.6.1 (the outcome does not depend on the Node version: the attributes are set explicitly on the property descriptor, so 22, 24 and 26 in the CI matrix behave the same).

• Pristine main: mocha build/test/test.oauth2common.js reports 25 passing.
• New test with the source change reverted to main (test file only changed): 25 passing, 1 failing. The failure is the new test, should keep the copied stack writable, configurable and non-enumerable, with AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: false !== true on the writable assertion. The full unit suite in that state reports 936 passing, 1 failing.
• With the fix: mocha build/test/test.oauth2common.js reports 26 passing; the full unit suite (mocha build/test) reports 937 passing, 0 failing.
• After the review follow up (explicit configurable): mocha build/test/test.oauth2common.js again reports 26 passing and mocha build/test again reports 937 passing, 0 failing.

Coverage: no executable lines change (the diff adds two comment lines inside the descriptor), only the attribute values inside the existing defineProperty call change, and the new test exercises that call, so coverage does not decrease.

Lint

• node ./bin/linter.mjs from the repository root (the same script presubmit runs; it lints the changed .ts files against the root ESLint config and runs tsc --noEmit for the package) exits 0 with no findings, both for the original change and after the review follow up.
• npx gts check --no-inline-config src/auth/oauth2common.ts test/test.oauth2common.ts inside the package exits 0.
• npx prettier --check on both files passes.

Alternatives considered

The issue also suggests not copying stack at all, or attaching the original error as cause. Both would change what callers currently see in error.stack (today it is the original request error's stack, and the existing test asserts that), so this PR keeps the copied value and only fixes its attributes. Happy to switch to cause in a follow up if maintainers prefer that direction.

One observable change worth stating: the copied stack is now non enumerable as well as writable, exactly like the stack of a plain Error, so Object.keys(err), for...in and JSON.stringify(err) no longer include it. Anything that serialized these errors and relied on stack appearing as an own enumerable key would see that key disappear; a plain Error never exposed it that way, which is why the change is framed as restoring normal Error semantics.

[gemini-code-assist]

Code Review

This pull request ensures that when copying properties to a new error object in 'getErrorFromOAuthErrorResponse', the 'stack' property remains writable and non-enumerable, and adds a corresponding unit test. The review feedback recommends explicitly setting 'configurable: true' on the property descriptor to fully align with standard 'Error' behavior and prevent potential strict mode errors, as well as adding a corresponding assertion in the unit test.