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

scope="../" should be invalid #551

Closed
pkotwicz opened this issue Feb 6, 2017 · 13 comments
Closed

scope="../" should be invalid #551

pkotwicz opened this issue Feb 6, 2017 · 13 comments
Assignees
Labels
scope member Related to scope member

Comments

@pkotwicz
Copy link

pkotwicz commented Feb 6, 2017

From https://crbug.com/688889

When a user adds a page which contains a Web Manifest to the homescreen in Chrome all of the pages within the Web Manifests' scope get special treatment.

It is currently valid for a web manifest in http://rootsweb.ancestry.com/~peter/manifest.json to have scope="../"
In my opinion this is suboptimal because this would apply the special treatment specified by http://rootsweb.ancestry.com/~peter/manifest.json to all of http://rootsweb.ancestry.com/

http://rootsweb.ancestry.com/ is a web hosting service which grants each user hosting within a different subdirectory within http://rootsweb.ancestry.com/
There does not seem to be many web hosting services which grant users a sub directory but they seem to still exist http://www.free-webhosts.com/free-web-hosts.php

I suggest requiring a Web Manifest scope to be within the directory which contains the Web Manifest
Examples:
Manifest URL https://www.example.com/subdir/manifest.json
Scope '.' Valid
Scope 'deeper_subdir' Valid
Scope '../' Invalid
Scope '../other_subdir' Invalid
Scope 'deeper_subdir/../../other_subdir' Invalid

@kenchris
Copy link
Collaborator

kenchris commented Feb 6, 2017

That on the other hand kind of requires having the manifest file live in the root. I guess that might be OK, but I have seem multiple examples where that was not the case (To be honest, most of these were for sites using favicon generators).

We haven't told people to keep the manifest in the app root (not necessarily domain root!) so would be nice to get the input from @PaulKinlan .

Or does it makes more sense to make scope relative to start_url instead of manifest location? @marcoscaceres

@RobDolinMS
Copy link
Contributor

There are a number of cases where an app may want to support multiple scopes (see #449.)

To address @pkotwicz's concern, what about:
the (primary) "scope" should be within the same directory as the manifest, but allow for "additional_scopes" to be more broad; possibly with limitation on how these could intercept navigations.

@kenchris
Copy link
Collaborator

kenchris commented Feb 8, 2017

I think it is nice to be a bit flexible and not having to have the manifest file in the root. I talked to @PaulKinlan and as far as I understood, he agrees with me.

For solving the case where you are not the owner of the domain, I think we could allow the domain some power to restrict what others can use as scope, something like the robots.txt to ignore seach engines, we might be able to have a /.well-known/manifest-policy.json [1] imposing restrictions.

[1] https://www.mnot.net/blog/2010/04/07/well-known

@kenchris
Copy link
Collaborator

kenchris commented Feb 8, 2017

/.well-known/manifest-policy.json could list paths from root which are acting like scope borders, like

E.g. for https://github.com:

https://github.com/.well-known/manifest-policy.json contains "scope_borders": ["/"], which means e.g. that the widest scope of https://github.com/kenchris/manifest.json would be https://github.com/kenchris/ etc

Similarly for https://inventeddomain.com:

"scope_borders": ["/sites", "/"] would mean that for https://inventedsite.com/sites/coolsite/data/manifest.json the widest scope would be https://inventedsite.com/sites/coolsite, but for https://inventedsite.com/food/manifest.json it would be https://inventedsite.com/food/

For "additional_scopes" we would also need a way for a site to say that it allows itself to become an additional scope of another manifest. /.well-known/manifest-policy.json could be the right place to define that.

@PaulKinlan
Copy link

PaulKinlan commented Feb 8, 2017

What @pkotwicz describes is the exact challenges that Service Worker also had for their scope. I know the two are different, but given that the scope affects the navigational context, the app gets special privileges not normally afforded to web apps (URL interception in particular).

It gets a little harder because the location of the manifest could reasonably be on a CDN and the manifest scope processing doesn't include the document URL other than for checking the scope is on the same origin as the document. Step 4 and 6 are at conflict in this process.

I'm not clear on the interplay between the scope and the start_url, I've (maybe mistakenly) been thinking that the base url gets affected based on the scope whichs means the start_url is relative to that to the scope and the scope can be either defined or inferred by the location of the manifest. I think there needs to be some clarity about what defines a web app.

There is a note that implies this, but I don't read it that clearly in the spec

For example, if the value of start_url is ../start_point.html, and the manifest's URL is https://example.com/resources/manifest.webmanifest, then the result of URL parsing would be https://example.com/start_point.html.

I've tried to document combinations of start url and manifest url and the base to make it clearer in my head about what is desirable, note: I am assuming that the start url is calculated from the manifest_url which means that I am assuming that the manifest url is the base and the thing that drives everything that makes an app an app.

manifest url start_url scope attr calculated scope caluclated start_url valid?
/tech-today/manifest.json ./index.html undefined /tech-today/ /tech-today/index.html valid
/tech-today/manifest.json ./index.html ../ / /index.html valid - but scope leaks to higher level
/tech-today/manifest.json / / / /index.html valid - but scope leaks to higher level
/tech-today/manifest.json / undefined /tech-today/ / invalid - start url out of scope
/tech-today/manifest.json ./tech-today/index.html undefined /tech-today/ /tech-today/tech-today/index.html valid - but undesired
/manifest.json ./tech-today/index.html undefined / /tech-today/index.html valid - broad scope
/manifest.json /tech-today/index.html tech-today /tech-today/ /tech-today/index.html valid - tight scope

Note 2: I am not including additional scopes in this thread at the moment.

@kenchris
Copy link
Collaborator

kenchris commented Feb 8, 2017

  1. We are defining things inside a manifest which is in one location, so making things relative to that location makes sense (like for any HTML file etc). That is how it is defined today, thus icons and start_url are relative to manifest path.

  2. As there is a strong relation between scope and start_url (the latter has to be within scope), it could make sense to divert from the above and let start_url be relative to scope.

  3. I think it "makes sense" to have the default scope be "." instead of unbounded. Unbounded seems a bit unsafe and might shock people. On the other hand, if it defaults to "." people will probably start getting their "scope" set :-) Most people use manifest.json in root today so it would make little difference, but this would break sites that are currently not having manifest.json in root, nor having a scope defined.

  4. With the above, if "scope" by default is "." that would mean that start_url is also relative to the manifest location, like spec'ed today

@kenchris
Copy link
Collaborator

kenchris commented Feb 8, 2017

So the way it works today (as specced):

Every single URL within the manifest file is relative to the location of that file (like it would be in a JS, HTML or CSS file).

If that is cumbersome in some situations, we could consider added a "base" for setting base url, like https://developer.mozilla.org/en-US/docs/Web/HTML/Element/base

| manifest url | start_url | scope attr | calculated scope | calculated start_url | valid |
|
| ------ | ------ | ------ | ------ | ------ | ------ |
| /tech-today/manifest.json | ./index.html | undefined | / | /tech-today/index.html | YES (broader scope) |
| /tech-today/manifest.json | ./index.html | ../ | / | /tech-today/index.html | YES (broader scope) |
| /tech-today/manifest.json | / | / | / | /index.html | YES |
| /tech-today/manifest.json | / | ./ | /tech-today/ | /index.html | NO |
| /tech-today/manifest.json | / | undefined | / | / | YES |
| /tech-today/manifest.json | ./tech-today/index.html | undefined | / | /tech-today/tech-today/index.html | YES (but wrong intent) |
| /manifest.json | ./tech-today/index.html | undefined | / | /tech-today/index.html | YES (broader scope) |
| /manifest.json | /tech-today/index.html | tech-today | /tech-today/ | /tech-today/index.html | YES |

@pkotwicz
Copy link
Author

pkotwicz commented Feb 8, 2017

  • It feels like the ideal solution would be disallow scopes which are wider than the manifest directory. (Scope="../" would be illegal). Chrome disallows registering a service worker whose scope is larger than the directory containing the page that the service worker is registered from (I tested with https://gauntface.github.io/simple-push-demo/)
  • Supporting multiple scopes in the Web Manifest is a hard problem. The website owner would need a mechanism to prove that they own all of the scopes.

kenchris@ is right that if we made scope="../" illegal we would break some sites. A coworker ran a script on all of the Web Manifests on the web. There are 26 Web Manifests on the Web whose scope is wider than the directory containing the manifest. There's ~ 35,000 Web Manifests total. Unfortunately, one of those 26 is https://www.nfl.com/manifest/super-bowl.json I have no idea whether it is possible to convince all of the website owners to move the manifest.json file on their site

@kenchris
Copy link
Collaborator

kenchris commented Feb 9, 2017

Yes, the additional scopes somehow would need to whitelist the manifest who is referring to them.

One way could be

In https://mysite.com/manifest.json, have:

"includes": ["https://otherofmysites.com/manifest.json"]

Then in https://otherofmysites.com/manifest.json, have:

"owner": "https://mysite.com/manifest.json",
"scope": "/"

Adding "owner" to a manifest means that it will refer to another manifest file as the data source for adding to homescreen, and only a few of the properties will be used (ie "scope" for now).

@kenchris
Copy link
Collaborator

kenchris commented May 2, 2017

So any update on what you changed in Chrome? Like would it be possible for you to submit a PR if you made any changes so that we can review it and move this forward?

@pkotwicz
Copy link
Author

pkotwicz commented May 2, 2017

We ended up not making any changes in Chrome. It is safe to assume that we will likely not make any changes in the foreseeable future. I don't think anyone on the Chrome team has given this topic much thought in the previous month.

@marcoscaceres marcoscaceres added the scope member Related to scope member label Sep 8, 2019
@marcoscaceres
Copy link
Member

Ok, we need to probably good to check and confirm with a test of some sort... adding to CR list for scope.

@mgiuca
Copy link
Collaborator

mgiuca commented Jul 6, 2020

It sounds like from the original report that the reason to prevent this would be to prevent escaping out of a subdirectory (e.g., if you were given ownership over a subdirectory on a shared origin with other untrusted parties).

On the web, paths are not a security boundary. That battle is far too lost; there are heaps of things that let you do anything you want at the origin scope -- even in the manifest you could set a scope of "/". The origin is the security boundary, and web hosts should give each person their own suborigin, not a sub-path.

Therefore, closing as WAI.

@mgiuca mgiuca closed this as completed Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope member Related to scope member
Projects
None yet
Development

No branches or pull requests

7 participants