-
Notifications
You must be signed in to change notification settings - Fork 3.9k
fix(process): handle Object.prototype methods on Windows process.env #26316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Updated 8:52 PM PT - Jan 20th, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 26316That installs a local version of the PR into your bun-26316 --bun |
WalkthroughWindows-specific process.env proxy get handler now checks Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/js/builtins/ProcessObjectInternals.ts`:
- Around line 391-407: The proxy get handler currently checks internalEnv (via
upperKey) before checking Object.prototype methods, causing prototype methods
like hasOwnProperty to be shadowed by env strings; update the get handler in
ProcessObjectInternals (function get, symbols: p, upperKey, internalEnv,
objectProto, receiver) to first check if p exists on objectProto and, if so,
return the bound method or property, and only if not present fall back to
checking internalEnv[upperKey] so env vars remain accessible via uppercase but
Object.prototype methods remain callable.
- Around line 389-405: Replace uses of the public globals in the Proxy get
handler: instead of using Object.prototype (objectProto) and typeof checks,
reference the intrinsic $Object to access its prototype and use the $isCallable
helper for callable checks. Concretely, update the variable currently assigned
from Object.prototype to use $Object.prototype, and change the typeof method ===
"function" guard to use $isCallable(method); ensure you still bind the method to
receiver when callable and keep the same branch logic in the get handler for
internalEnv lookup and fallback to prototype methods.
In `@test/regression/issue/26315.test.ts`:
- Around line 7-26: Tests assume PATH exists; change both tests (the
"process.env.hasOwnProperty works" and "process.env.propertyIsEnumerable works")
to create a temporary env var (e.g., set process.env.__TEST_TMP_VAR__ = "1"),
assert hasOwnProperty/propertyIsEnumerable against that key, then delete the key
(or restore prior value) in cleanup to avoid leaking state; reference the uses
of process.env.hasOwnProperty and process.env.propertyIsEnumerable in the two
test blocks when making the change.
01a8af5 to
655c7d1
Compare
|
Addressed all review comments:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/js/builtins/ProcessObjectInternals.ts`:
- Around line 389-407: The current code uses method.bind(receiver) which can be
hijacked if Function.prototype.bind is mutated; update the return path in the
get trap so that instead of calling method.bind(receiver) you return a protected
wrapper that invokes the original method with the receiver via the intrinsic
call/apply helpers (e.g. use the existing $call or $apply primitive to invoke
method with receiver and forwarded arguments). Change the branch that checks
objectProto and $isCallable (the code referencing objectProto, $isCallable, and
method.bind(receiver)) to return a small wrapper that forwards args to
$call(method, receiver, ...args) (or the project’s equivalent protected
Function.prototype.call wrapper) to avoid relying on the untrusted bind
implementation.
On Windows, process.env is wrapped in a Proxy for case-insensitive access. The get trap previously converted all property names to uppercase and looked them up in the env map, which meant inherited methods like hasOwnProperty returned undefined (or threw an error when called). This fix checks if the property is an inherited Object.prototype method first (before looking up env vars), and returns a wrapper function that invokes the method with the proxy as `this` so it works correctly. Uses $Object.prototype, $isCallable, and $call intrinsics per code review feedback to avoid relying on potentially mutated prototypes. Fixes #26315 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
655c7d1 to
a9effc7
Compare
|
Updated to address the latest review comment:
|
Summary
process.env.hasOwnProperty()throwing "TypeError: environment.hasOwnProperty is not a function" on Windowsprocess.envProxy now correctly returns inheritedObject.prototypemethods when no env var exists with that namehasOwnPropertychecks the proxy, not the underlying object)Fixes #26315
Test plan
test/regression/issue/26315.test.tshasOwnProperty,propertyIsEnumerable,toString,valueOf, andisPrototypeOfwork correctly🤖 Generated with Claude Code