Skip to content

process: disallow some uses of Object.defineProperty() on process.env#28006

Merged
nodejs-github-bot merged 4 commits intonodejs:masterfrom
himself65:27990
Apr 4, 2022
Merged

process: disallow some uses of Object.defineProperty() on process.env#28006
nodejs-github-bot merged 4 commits intonodejs:masterfrom
himself65:27990

Conversation

@himself65
Copy link
Member

@himself65 himself65 commented Jun 1, 2019

try to fix #27990

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jun 1, 2019
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m labelling this semver-major because it adds a throw, but I don’t have a strong opinion on that and we can remove it if somebody feels differently.

@addaleax addaleax added process Issues and PRs related to the process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Jun 1, 2019
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is green. (I think I'd prefer that we throw an error with a code property, but that can always be changed later.)

@nodejs-github-bot

This comment has been minimized.

@himself65 himself65 force-pushed the 27990 branch 2 times, most recently from d30c5e6 to 40bdd0c Compare June 2, 2019 03:16
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % comment.

@himself65 himself65 force-pushed the 27990 branch 13 times, most recently from 5d63629 to 6b9d9cb Compare June 3, 2019 16:54
@himself65 himself65 force-pushed the 27990 branch 2 times, most recently from 75fa512 to 59b5175 Compare June 5, 2019 15:02
@himself65
Copy link
Member Author

Ok, i will take a look

Disallow the use of Object.defineProperty() to hide entries in
process.env or make them immutable.
@himself65
Copy link
Member Author

hope the test will pass

@himself65
Copy link
Member Author

Download action repository 'actions/checkout@v2' (SHA:ec3a7ce113134d7a93b817d10a8272cb61118579)
Warning: Failed to download action 'https://v-api-github-com.286600.xyz/repos/actions/checkout/tarball/ec3a7ce113134d7a93b817d10a8272cb61118579'. Error: Response status code does not indicate success: 503 (Service Unavailable).
Warning: Back off 25.2 seconds before retry.
Warning: Failed to download action 'https://v-api-github-com.286600.xyz/repos/actions/checkout/tarball/ec3a7ce113134d7a93b817d10a8272cb61118579'. Error: Response status code does not indicate success: 503 (Service Unavailable).
Warning: Back off 12.773 seconds before retry.
Error: Response status code does not indicate success: 503 (Service Unavailable).

@himself65
Copy link
Member Author

why all ci crash and only happen in my pr?

@himself65
Copy link
Member Author

why all ci crash and only happen in my pr?

https://v-www-githubstatus-com.286600.xyz/incidents/cypv026dr23w

@himself65
Copy link
Member Author

@ljharb could u please take a look? thanks

@ljharb
Copy link
Member

ljharb commented Jan 13, 2022

LGTM!

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jan 14, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 14, 2022
@nodejs-github-bot

This comment was marked as outdated.

@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 3, 2022
@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in e6a7300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. notable-change PRs with changes that should be highlighted in changelogs. process Issues and PRs related to the process subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot delete property from process.env