fix(cookies): keep a cookie named __proto__ in getCookies

[luantaraschi]

This relates to...

No open issue. Found while reading lib/web/cookies/.

Rationale

getCookies() builds its result on a plain object literal:

const out = {}
for (const piece of cookie.split(';')) {
  const [name, ...value] = piece.split('=')
  out[name.trim()] = value.join('=')
}

__proto__ is a valid cookie name (it is a token per RFC 6265), but assigning it on a plain object reaches the Object.prototype setter instead of creating an own property. The setter only accepts an object or null, and the assigned value here is always a string, so it does nothing and the cookie is dropped:

const headers = new Headers()
headers.set('Cookie', '__proto__=foo; bar=baz')

getCookies(headers)              // { bar: 'baz' }, the first cookie is gone
getCookies(headers).__proto__    // Object.prototype, not the string 'foo'

The second line is the part that bothered me more than the missing key. getCookies is typed Record<string, string>, so a caller that indexes it by an attacker-influenced name gets an object back where the types promise a string.

There is no prototype pollution here: the setter refuses a string, so Object.prototype is untouched. The bug is silent data loss plus a broken type contract.

Changes

getCookies now builds its record with { __proto__: null }, which is already the pattern used in lib/core/util.js, lib/util/runtime-features.js, lib/web/eventsource/eventsource.js and lib/web/fetch/formdata.js.

The existing tests all use assert.deepEqual, which does not compare prototypes, so none of them needed changing. All 92 tests under test/cookie/ pass.

Features

N/A

Bug Fixes

A cookie named __proto__ is now returned by getCookies() like any other name.

Breaking Changes and Deprecations

The returned object no longer inherits from Object.prototype, so calling cookies.hasOwnProperty(name) on it stops working. Object.hasOwn(cookies, name) and name in cookies still work, as does every form of indexing and iteration.

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[mcollina]

LGTM

(However, I think this is a bad idea. We should throw hard when proto appears).

[luantaraschi]

Thanks for the review.

The three red jobs are all the same thing and it is not this change. test/fetch/content-length.js sat at the borp ceiling and got killed:

✖ /Users/runner/work/undici/undici/test/fetch/content-length.js (180006.261333ms)

That is a hang, not an assertion. Locally on this branch the file passes in 2 seconds:

ℹ tests 2
ℹ pass 2
ℹ fail 0
ℹ duration_ms 2024.4792

The diff only touches getCookies in lib/web/cookies/index.js, which nothing in that file reaches.

main has not moved since I branched, so a rebase would only be an empty commit on an approved PR. A re-run should clear it, but say the word if you would rather I push something to retrigger.

[luantaraschi]

Thanks for the review.

The three red jobs are not from this change. They fail in test/fetch/content-length.js (macOS, Node 22, times out at the 180s cap) and test/http2-request-never-settles.js (ubuntu, Node 24, both jobs). This PR only touches lib/web/cookies/index.js. test/http2-request-never-settles.js is also red right now on your own #5643, on the Node 25 WASM-SIMD-disabled job.

On throwing hard: the repo already goes the other way in two places, and both are yours.

• b64e7e4, "fix: avoid prototype collisions in parseHeaders (fix: avoid prototype collisions in parseHeaders #4923)"
• 90a1609, "fix: preserve response trailers that shadow Object.prototype"

Both take a key that came off the wire and write it with Object.defineProperty instead of rejecting it. This PR does the same thing one layer up, which is why I kept that shape rather than inventing a third one.

There is one asymmetry worth naming. __proto__ is a legal cookie-name: RFC 6265 section 4.1.1 defines cookie-name as a token, and _ is not a separator. So if getCookies throws, Cookie: __proto__=1 takes down whatever called it, and the peer picks when that happens. parseHeaders and the trailers path have the same property, which I assume is part of why they preserve.

That said, it is your call. If you do want to throw, I would rather it be one decision applied to all three sites than cookies diverging from the two already fixed, and it is a behavior change for anyone parsing untrusted cookies today. I am happy to write that version instead if you prefer it.