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: handle duplicate paths granted #56591

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

src: handle duplicate paths granted #56591

wants to merge 1 commit into from
+44 −2

Conversation

RafaelGSS
Copy link
Member

This commit fixes a crash whenever someone tries to allow access to the same path twice

@RafaelGSS RafaelGSS added the permission Issues and PRs related to the Permission Model label Jan 13, 2025
@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++. needs-ci PRs that need a full CI run. labels Jan 13, 2025
@RafaelGSS
src: handle duplicate paths granted
a856e12 
This commit fixs a crash whenever someone tries to allow access to the
same path twice
@RafaelGSS RafaelGSS force-pushed the fix-pm-crash-on-duplicate-path branch from 4cd9bf7 to a856e12 Compare January 13, 2025 20:09
@codecov Codecov
Copy link

codecov bot commented Jan 13, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.19%. Comparing base (3946f16) to head (a856e12).
Report is 29 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #56591      +/-   ##
==========================================
+ Coverage   89.16%   89.19%   +0.03%     
==========================================
  Files         662      662              
  Lines      191751   191800      +49     
  Branches    36900    36923      +23     
==========================================
+ Hits       170972   171084     +112     
+ Misses      13635    13555      -80     
- Partials     7144     7161      +17
Files with missing lines Coverage Δ
src/permission/fs_permission.cc 73.64% <100.00%> (+1.98%) ⬆️

... and 39 files with indirect coverage changes

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

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

@Cxz324 Cxz324 mentioned this pull request Jan 13, 2025
Closed
@nodejs-github-bot
Copy link
Collaborator

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

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

@marco-ippolito marco-ippolito marco-ippolito left review comments

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. permission Issues and PRs related to the Permission Model
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RafaelGSS @nodejs-github-bot @jasnell @marco-ippolito