Skip to content

permission: support URL and Uint8Array as has()/drop() reference - #65492

Open
nhjbest22 wants to merge 1 commit into
nodejs:mainfrom
nhjbest22:permission-reference-types
Open

permission: support URL and Uint8Array as has()/drop() reference#65492
nhjbest22 wants to merge 1 commit into
nodejs:mainfrom
nhjbest22:permission-reference-types

Conversation

@nhjbest22

Copy link
Copy Markdown
Contributor

process.permission.has()/drop() only accepted a string or Buffer for
the reference argument. This adds support for a WHATWG URL, resolved
via fileURLToPath() for fs.* scopes since those are the only scopes
that actually use the reference value, and a plain Uint8Array.

Also switches permission.cc from Utf8Value to BufferValue when reading
a Buffer/TypedArray reference, since Utf8Value forces a UTF-8 string
conversion that can silently corrupt a path that isn't valid UTF-8.

@nodejs-github-bot

Copy link
Copy Markdown
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. process Issues and PRs related to the process subsystem. labels Aug 22, 2026
@codecov

codecov Bot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.69767% with 4 lines in your changes missing coverage. Please review.
βœ… Project coverage is 90.14%. Comparing base (cf30b2e) to head (42241f8).
⚠️ Report is 194 commits behind head on main.

Files with missing lines Patch % Lines
src/permission/permission.cc 72.72% 0 Missing and 3 partials ⚠️
lib/internal/process/permission.js 96.87% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65492      +/-   ##
==========================================
+ Coverage   90.11%   90.14%   +0.03%     
==========================================
  Files         752      751       -1     
  Lines      251861   252647     +786     
  Branches    47365    47549     +184     
==========================================
+ Hits       226955   227745     +790     
+ Misses      16238    16192      -46     
- Partials     8668     8710      +42     
Files with missing lines Coverage Ξ”
lib/internal/process/permission.js 92.39% <96.87%> (+10.68%) ⬆️
src/permission/permission.cc 75.98% <72.72%> (+0.76%) ⬆️

... and 108 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nhjbest22
nhjbest22 force-pushed the permission-reference-types branch from 6d518fc to 72717a5 Compare August 23, 2026 12:59
normalizeReference() adds URL and Uint8Array support to has()/drop();
the existing string/Buffer behavior is unchanged. BufferValue replaces
Utf8Value so a Buffer/TypedArray reference is copied as raw bytes
instead of a lossy UTF-8 conversion.

Signed-off-by: seungmin Nam <nhjbest22@g.skku.edu>
@nhjbest22
nhjbest22 force-pushed the permission-reference-types branch from 72717a5 to 42241f8 Compare August 23, 2026 13:09
@RafaelGSS RafaelGSS added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs with CI started, the required approvals, and no outstanding review comments. permission Issues and PRs related to the Permission Model. labels Aug 24, 2026
@github-actions github-actions Bot added request-ci-failed Starting CI with the request-ci label failed and requires manual intervention. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 24, 2026
@github-actions

This comment was marked as outdated.

@RafaelGSS RafaelGSS added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed Starting CI with the request-ci label failed and requires manual intervention. labels Aug 24, 2026
@github-actions github-actions Bot added request-ci-failed Starting CI with the request-ci label failed and requires manual intervention. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 24, 2026
@github-actions

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@MikeMcC399 MikeMcC399 removed the request-ci-failed Starting CI with the request-ci label failed and requires manual intervention. label Aug 24, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@MikeMcC399 MikeMcC399 removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Aug 25, 2026
@MikeMcC399

This comment was marked as resolved.

@MikeMcC399

This comment was marked as resolved.

@MikeMcC399 MikeMcC399 added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2026
@nodejs-github-bot

This comment was marked as outdated.

@MikeMcC399

This comment was marked as outdated.

@MikeMcC399

This comment was marked as resolved.

@nhjbest22

Copy link
Copy Markdown
Contributor Author

@MikeMcC399
Thank you for keeping an eye on this and trying to rerun CI! I also suspect it's an issue with Jenkins CI itself. I noticed many people working hard on Slack to resolve the CI issues as well.

I'll just sit tight and wait for the CI to be resolved.

@MikeMcC399

This comment was marked as resolved.

@MikeMcC399

This comment was marked as resolved.

@MikeMcC399

This comment was marked as outdated.

@MikeMcC399

This comment was marked as outdated.

@MikeMcC399 MikeMcC399 added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2026
@nhjbest22

Copy link
Copy Markdown
Contributor Author

@MikeMcC399

Thank you so much for constantly keeping an eye on my PR and explaining the CI status! I'm truly amazed by all the care and effort you've put into this.

Based on your explanation, it sounds like we might see a green CI soon! I'll sit tight and wait a bit longer.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 27, 2026
@nodejs-github-bot

This comment was marked as outdated.

@MikeMcC399

Copy link
Copy Markdown
Contributor

@nhjbest22

Thank you so much for constantly keeping an eye on my PR and explaining the CI status! I'm truly amazed by all the care and effort you've put into this.

Based on your explanation, it sounds like we might see a green CI soon! I'll sit tight and wait a bit longer.

I didn't expect to be spending so much effort surrounding your PR, however it has got caught up in several generic issues, so that's why I'm continuing to follow it and get it resolved together with others on the team.

So far none of the issues seem to come from changes you've proposed in this PR, so I am hopeful to see a green CI soon! Thanks for your patience!

@nodejs-github-bot

This comment was marked as outdated.

@MikeMcC399 MikeMcC399 added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2026
@MikeMcC399

This comment was marked as outdated.

@sxa

This comment was marked as resolved.

@MikeMcC399 MikeMcC399 removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2026
@nodejs-github-bot

This comment was marked as outdated.

@MikeMcC399

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@MikeMcC399

This comment was marked as resolved.

@MikeMcC399

MikeMcC399 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Jenkins is now green after many attempts πŸŽ‰. The issues it found had nothing to do with the contents of this PR and these are hidden now as resolved / outdated in the history.

Marking as author ready PRs with CI started, the required approvals, and no outstanding review comments. and handing back to the subject matter experts who approved the PR.

@MikeMcC399 MikeMcC399 added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants