Skip to content

Policy: add scopes to ease usability#34552

Closed
bmeck wants to merge 1 commit intonodejs:masterfrom
bmeck:policy-scopes
Closed

Policy: add scopes to ease usability#34552
bmeck wants to merge 1 commit intonodejs:masterfrom
bmeck:policy-scopes

Conversation

@bmeck
Copy link
Member

@bmeck bmeck commented Jul 29, 2020 •
edited
Loading

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

This adds a mechanism matching package.json that allows setting default "integrity" and "dependency" values by a URL subspace. It also adds the ability to opt-in to cascading permissions by introducing "cascade": true. It uses a separate field in the policy file due to collisions of string representation of scopes and potential resources.

@bmeck bmeck added wip Issues and PRs that are still a work in progress. module Issues and PRs related to the module subsystem. experimental Issues and PRs related to experimental features. policy Issues and PRs related to the policy subsystem. labels Jul 29, 2020
@bmeck
Copy link
Member Author

bmeck commented Aug 7, 2020 •
edited
Loading

I've noted that full protocol scoping is not currently possible for https: since http requires a host, similarly file:// does not cover non-localhost file hosts.

@bmeck
Copy link
Member Author

bmeck commented Aug 7, 2020

Ugggg, blob: inherits origin from parsing in its path... mmmm I guess we should match and special case so that blob: never inherits from a protocol level scope?

@bmeck bmeck force-pushed the policy-scopes branch 3 times, most recently from e25f721 to 6233f31 Compare August 14, 2020 20:04
@bmeck
Copy link
Member Author

bmeck commented Aug 14, 2020

rebased due to this and conditions landing a bit uncleanly

@bmeck bmeck removed the wip Issues and PRs that are still a work in progress. label Aug 18, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 18, 2020

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

@bmeck
Copy link
Member Author

bmeck commented Aug 18, 2020

this is pretty much just ready for review, some odd quirks about node do leak out like how the '-e' and '-p' CLI flags treat their location as files?

@bmeck bmeck requested review from guybedford and jasnell August 18, 2020 14:36
jasnell
jasnell reviewed Aug 18, 2020
View reviewed changes
jasnell
jasnell reviewed Aug 18, 2020
View reviewed changes
doc/api/policy.md Outdated
Comment on lines 203 to 204
Copy link
Member

@jasnell jasnell Aug 18, 2020

Choose a reason for hiding this comment

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

These two sentences are a bit confusing and awkward together

Copy link
Member Author

@bmeck bmeck Aug 18, 2020

Choose a reason for hiding this comment

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

how about combining them:

If a scope or resource includes "cascade": true unknown specifiers will
be searched for in their containing scope.

Copy link
Member Author

@bmeck bmeck Aug 18, 2020

Choose a reason for hiding this comment

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

rephrased

jasnell
jasnell reviewed Aug 18, 2020
View reviewed changes
jasnell
jasnell reviewed Aug 18, 2020
View reviewed changes
jasnell
jasnell reviewed Aug 18, 2020
View reviewed changes
jasnell
jasnell reviewed Aug 18, 2020
View reviewed changes
@bmeck
Copy link
Member Author

bmeck commented Aug 19, 2020

Wrote up some attack concerns on adding this feature in some slides. I believe the risk of using this feature is acceptable.

@bmeck
Copy link
Member Author

bmeck commented Aug 20, 2020

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

@bmeck bmeck added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 20, 2020
@bmeck
Copy link
Member Author

bmeck commented Aug 21, 2020

I intend to land this on Monday.

@github-actions github-actions bot mentioned this pull request Aug 22, 2020
Open
9 tasks
This was referenced Aug 23, 2020
Open
Open
Open
@bmeck
Copy link
Member Author

bmeck commented Aug 25, 2020

rebased but waiting on ci reliability (seems unrelated?)

@github-actions github-actions bot mentioned this pull request Aug 26, 2020
Open
9 tasks
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 26, 2020

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

bmeck added a commit that referenced this pull request Aug 26, 2020
@bmeck
policy: implement scopes field
4234904 
PR-URL: #34552
Reviewed-By: James M Snell <jasnell@gmail.com>
@bmeck
Copy link
Member Author

bmeck commented Aug 26, 2020

Landed in 4234904

@bmeck bmeck closed this Aug 26, 2020
@github-actions github-actions bot mentioned this pull request Aug 27, 2020
Open
10 tasks
richardlau pushed a commit that referenced this pull request Sep 1, 2020
@bmeck @richardlau
policy: implement scopes field
18b04ab 
PR-URL: #34552
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau mentioned this pull request Sep 2, 2020
Merged
4 tasks
@bmeck bmeck deleted the policy-scopes branch February 3, 2022 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@guybedford guybedford Awaiting requested review from guybedford

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. experimental Issues and PRs related to experimental features. module Issues and PRs related to the module subsystem. policy Issues and PRs related to the policy subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bmeck @nodejs-github-bot @jasnell