Skip to content
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.

Sign up for GitHub

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

Jump to bottom

src,permission: throw async errors on async APIs #52730

Merged
merged 2 commits into from Apr 30, 2024
Merged

src,permission: throw async errors on async APIs #52730

merged 2 commits into from Apr 30, 2024

Conversation

RafaelGSS
Copy link
Member

Refs: nodejs/security-wg#898

With this PR, we throw access denied asynchronously for async APIs.

fs.readFile('restricted-file.md', (error) => {
  // error is ERR_ACCESS_DENIED
})

cc/ @nodejs/security-wg

@RafaelGSS RafaelGSS added security Issues and PRs related to security. permission Issues and PRs related to the Permission Model labels Apr 28, 2024
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 28, 2024
@RafaelGSS RafaelGSS added the dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. label Apr 28, 2024
benjamingr
benjamingr approved these changes Apr 28, 2024
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

LGTM (this is a big break, and I'm concerned about breakage but it's better to do now and not when it's no longer experimental)

@RafaelGSS
Copy link
Member Author

RafaelGSS commented Apr 28, 2024 •
edited

LGTM (this is a big break, and I'm concerned about breakage but it's better to do now and not when it's no longer experimental)

Yes, that's a "expected" breakage to Permission Model users

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

CI: https://ci.nodejs.org/job/node-test-pull-request/58784/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/58797/

@RafaelGSS
Copy link
Member Author

Failing due to #52751

@RafaelGSS RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 29, 2024
@RafaelGSS RafaelGSS added commit-queue Add this label to land a pull request using GitHub Actions. notable-change PRs with changes that should be highlighted in changelogs. labels Apr 29, 2024
@github-actions GitHub Actions
Copy link
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @RafaelGSS.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@RafaelGSS
Copy link
Member Author

I'm adding the notable change label as it's a breaking change to Permission Model users.

@RafaelGSS RafaelGSS mentioned this pull request Apr 29, 2024
Closed
7 tasks
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 30, 2024
@nodejs-github-bot nodejs-github-bot merged commit 4487e37 into nodejs:main Apr 30, 2024
66 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 4487e37

aduh95 pushed a commit that referenced this pull request Apr 30, 2024
@RafaelGSS @aduh95
src,permission: throw async errors on async APIs
c6fe433 
PR-URL: #52730
Refs: nodejs/security-wg#898
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 added a commit that referenced this pull request Apr 30, 2024
@aduh95
2024-05-02, Version 22.1.0 (Current)
bbeff10 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) #52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) #52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280

PR-URL: TODO
@aduh95 aduh95 mentioned this pull request Apr 30, 2024
Merged
aduh95 added a commit that referenced this pull request May 1, 2024
@aduh95
2024-05-02, Version 22.1.0 (Current)
12b0dab 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) #52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) #52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280

PR-URL: #52768
aduh95 added a commit that referenced this pull request May 1, 2024
@aduh95
2024-05-02, Version 22.1.0 (Current)
488a3f2 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) #52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) #52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280

PR-URL: #52768
aduh95 added a commit that referenced this pull request May 1, 2024
@aduh95
2024-05-02, Version 22.1.0 (Current)
7e00ff2 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) #52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) #52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280

PR-URL: #52768
aduh95 added a commit that referenced this pull request May 2, 2024
@aduh95
2024-05-02, Version 22.1.0 (Current)
87b87a8 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) #52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) #52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280

PR-URL: #52768
targos pushed a commit that referenced this pull request May 2, 2024
@aduh95 @targos
2024-05-02, Version 22.1.0 (Current)
08c3256 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) #52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) #52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280

PR-URL: #52768
Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024
@RafaelGSS @Ch3nYuY
src,permission: throw async errors on async APIs
9f73cdf 
PR-URL: nodejs#52730
Refs: nodejs/security-wg#898
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this pull request May 8, 2024
@aduh95 @Ch3nYuY
2024-05-02, Version 22.1.0 (Current)
9017ca8 
Notable changes:

buffer:
  * improve `base64` and `base64url` performance (Yagiz Nizipli) nodejs#52428
dns:
  * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) nodejs#52492
events,doc:
  * mark CustomEvent as stable (Daeyeon Jeong) nodejs#52618
lib, url:
  * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) nodejs#52509
module:
  * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) nodejs#52535
net:
  * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) nodejs#52474
src:
  * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) nodejs#52595
src,permission:
  * throw async errors on async APIs (Rafael Gonzaga) nodejs#52730
test_runner:
  * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) nodejs#52529
url:
  * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) nodejs#52280

PR-URL: nodejs#52768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingr benjamingr benjamingr approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

Assignees
No one assigned
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++. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. permission Issues and PRs related to the Permission Model security Issues and PRs related to security.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@RafaelGSS @nodejs-github-bot @benjamingr @marco-ippolito