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.

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

Policy: add scopes to ease usability #34552

Closed
wants to merge 1 commit into from
Closed

Conversation

bmeck
Copy link
Member

@bmeck bmeck commented Jul 29, 2020

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

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

@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?

doc/api/policy.md Outdated Show resolved Hide resolved
Comment on lines 203 to 204
If a dependency is not found, the resource may include `"cascade": true` which
will delegate to a scope. Scopes may also use `"cascade": true`. The scope for
Copy link
Member

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

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

Choose a reason for hiding this comment

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

rephrased

doc/api/policy.md Outdated Show resolved Hide resolved
@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

@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.

@bmeck
Copy link
Member Author

bmeck commented Aug 25, 2020

rebased but waiting on ci reliability (seems unrelated?)

@nodejs-github-bot
Copy link
Collaborator

bmeck added a commit that referenced this pull request Aug 26, 2020
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
richardlau pushed a commit that referenced this pull request Sep 1, 2020
PR-URL: #34552
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau mentioned this pull request Sep 2, 2020
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
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
Development

Successfully merging this pull request may close these issues.

3 participants