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.

Sign up for GitHub

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

Jump to bottom

Add unsafe-no-cors mode #1533

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+57 −8
Open

Add unsafe-no-cors mode #1533

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8fcb0ee
Add unsafe-no-cors mode
bvandersloot-mozilla Nov 7, 2022
ae04837
Make first round of changes
bvandersloot-mozilla Nov 14, 2022
fbf459d
Editorial changes in the mode definition.
bvandersloot-mozilla Nov 22, 2022
9a4f343
Further refining the definition of the mode and adding an assertion
bvandersloot-mozilla Nov 30, 2022
7356349
First attempt at placing the Origin header under full control of "uns…
bvandersloot-mozilla Nov 30, 2022
ecbf1c5
s/unsafe-no-cors/user-agent-no-cors/g
bvandersloot-mozilla Jan 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions fetch.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ urlPrefix:https://w3c.github.io/hr-time/#;spec:hr-time
type:typedef;url:dom-domhighrestimestamp;text:DOMHighResTimeStamp

urlPrefix:https://tc39.es/ecma262/#;type:dfn;spec:ecma-262
url:sec-agent-clusters;text:agent cluster
url:host-environment;text:host environment
url:realm;text:realm
url:sec-list-and-record-specification-type;text:Record
</pre>
Expand Down Expand Up @@ -1773,8 +1775,8 @@ to not have to set <a for=/>request</a>'s <a for=request>referrer</a>.
<p>A <a for=/>request</a> has an associated
<dfn export for=request id=concept-request-mode>mode</dfn>, which is
"<code>same-origin</code>", "<code>cors</code>", "<code>no-cors</code>",
"<code>navigate</code>", or "<code>websocket</code>". Unless stated otherwise, it is
"<code>no-cors</code>".
"<code>navigate</code>", "<code>unsafe-no-cors</code>", or "<code>websocket</code>".
Unless stated otherwise, it is "<code>no-cors</code>".

<div class="note no-backref">
<dl>
Expand All @@ -1796,13 +1798,27 @@ to not have to set <a for=/>request</a>'s <a for=request>referrer</a>.
<dt>"<code>navigate</code>"
<dd>This is a special mode used only when <a>navigating</a> between documents.

<dt>"<code>unsafe-no-cors</code>"
Copy link
Member

Choose a reason for hiding this comment

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

Consider naming this something like "host-no-cors" or "ua-no-cors" or something else that indicates what purpose it serves. Just "unsafe" is likely to scare off people who should use it, and not dissuade overconfident people who think they know what they're doing. Since this is only for use in web standards, that'll get caught eventually in spec review, but that could be much later than we want, and a better name could move the right design earlier.

Copy link
Author

Choose a reason for hiding this comment

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

unsafe-no-cors was @annevk's name for it, so I'll defer to him for a renaming decision.

Copy link
Member

Choose a reason for hiding this comment

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

I think unsafe is exactly right given that it exposes the contents of the response in the process it was invoked in. It's a name that is supposed to make you ask your peers for advice. I don't think host or ua has that kind of effect, especially since we don't have clearly delineated "host/ua agent clusters".

Copy link
Member

Choose a reason for hiding this comment

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

IIUC, the point of this mode is that it must only be used from the browser process or an equivalently-trusted process, so the response was already exposed there. If it's invoked by that sort of caller, are there any situations where it would still be unsafe?

I have a general rule to avoid "unsafe" and "safe" in naming, since they usually refer to just one kind of safety that was salient to the original author (here, exposure of resource bytes, I think), and future users tend to assume a different kind of safety and so get into trouble.

We could be more specific about what's unsafe instead of trying to name according to the intended caller. E.g. "no-cors-override-exposure-policies". But I suspect that naming according to the intended caller will make it easier to figure out how to maintain this mode's behavior in the future.

Copy link
Member

Choose a reason for hiding this comment

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

I think I follow the policy that we use for introducing "unsafe" in various places in the web platform. Namely that it requires additional attention. Is easy to lint, etc.

It also very much depends on what happens with the response bytes, how the request is setup, etc.

One point against "unsafe" here might be that it's not useful to web developers as this value is also exposed to them. From that perspective "user-agent-no-cors" is prolly better.

Copy link

Choose a reason for hiding this comment

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

One point against "unsafe" here might be that it's not useful to web developers as this value is also exposed to them. From that perspective "user-agent-no-cors" is prolly better.

Just to double check: do I understand it correctly that this is also going to be exposed to web developers (as Sec-Fetch-Mode=unsafe-no-cors), in addition to browser engineers?

If so, the name shouldn't trigger suspicion by the web developer (who should trust that the spec was written in a careful and thoughtful way, rather than something that the developer has to re-check again), right?

From that perspective "user-agent-no-cors" is prolly better.

Right. user-agent-no-cors would make it do and I think would also address @jyasskin 's point (and @annevk 's, to trigger spec reviewers to look carefully).

FWIW, mediated-no-cors could work too for me, just as a suggestion (user-agent-no-cors works too I think).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. I think in this case since web developers are not making the unsafe decision it's indeed probably best if they don't see the word unsafe. I think there are other cases though where web developers seeing the word unsafe is good, e.g., unsafe-eval.

I like user-agent-no-cors best still.

<dd>This is a special mode for the <a>host environment</a> to use internally to wittingly make
Copy link
Member

Choose a reason for hiding this comment

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

Is "host environment" the right term? It's odd to borrow a Javascript term for something in Fetch that's not integrated in any other way with Javascript infrastructure. I might just use [=user agent=].

Copy link
Author

Choose a reason for hiding this comment

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

I reached for "host environment" because I was talking about agent clusters in the warning and also mentioned the host environment there. I'll change this reference to [=user agent=], but do you think I should leave the reference in the warning alone?

Copy link
Member

Choose a reason for hiding this comment

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

I would probably change both to "user agent" or say something about the required memory isolation in the warning, but I don't feel strongly about it. I think we really want something about how trusted the other code in that agent cluster's process is, but I don't think we have the right terms defined for that, and I don't think this change needs to block on thinking through and defining those.

Copy link
Author

Choose a reason for hiding this comment

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

Okay. I'm changing the warning to be "user agent" as well and mention memory isolation explicitly, rather than directly call in Javascript concepts.

requests that are unsafe. It restricts requests to using <a>CORS-safelisted methods</a> and
<a>CORS-safelisted request-headers</a>. However, the request will not be required to pass a
<a>cross-origin resource policy check</a> or to test if
<a>Cross-Origin-Embedder-Policy allows credentials</a>. Additionally, upon success a fetch will
return a <a>cors filtered response</a>. However, a request with this mode cannot
use <a>service-workers mode</a> "<code>all</code>".
bvandersloot-mozilla marked this conversation as resolved.
Show resolved Hide resolved

<dt>"<code>websocket</code>"
<dd>This is a special mode used only when
<a spec=websockets lt="establish a WebSocket connection">establishing a WebSocket connection</a>.
</dl>

<p>Even though the default <a for=/>request</a> <a for=request>mode</a> is "<code>no-cors</code>",
standards are highly discouraged from using it for new features. It is rather unsafe.

<p class=warning> Using <a for=/>request</a> <a for=request>mode</a> "<code>unsafe-no-cors</code>"
is even more discouraged and unsafe than "<code>no-cors</code>". Any use of this mode must be in an
<a>agent cluster</a> associated with the <a>host environment</a> itself to isolate its results from
misuse. This <a for=request>mode</a> is deliberately not exposed in the {{RequestMode}}.
bvandersloot-mozilla marked this conversation as resolved.
Show resolved Hide resolved
</div>

<p>A <a for=/>request</a> has an associated
Expand Down Expand Up @@ -4051,7 +4067,7 @@ the request.
<a>HTTP(S) scheme</a>

<li><p><var>request</var>'s <a for=request>mode</a> is "<code>same-origin</code>",
"<code>cors</code>", or "<code>no-cors</code>"
"<code>cors</code>", "<code>no-cors</code>", or "<code>unsafe-no-cors</code>"

<li><p><var>request</var>'s <a for=request>window</a> is not null

Expand Down Expand Up @@ -4302,6 +4318,15 @@ steps:
<li><p>Return <var>corsWithPreflightResponse</var>.
</ol>

<dt><var>request</var>'s <a for=request>mode</a> is "<code>unsafe-no-cors</code>"
<dd>
<ol>
<li><p>Set <var>request</var>'s <a for=request>response tainting</a> to
"<code>basic</code>".

<li><p>Return the result of running <a>HTTP fetch</a> given <var>fetchParams</var>.
</ol>

<dt>Otherwise
<dd>
<ol>
Expand Down Expand Up @@ -4788,6 +4813,8 @@ these steps:
<li><p><var>request</var>'s <a for=request>mode</a> is not "<code>no-cors</code>" and
<var>response</var>'s <a for=response>type</a> is "<code>opaque</code>"

<li><p><var>request</var>'s <a for=request>mode</a> is "<code>unsafe-no-cors</code>"

<li><var>request</var>'s <a for=request>redirect mode</a> is not "<code>manual</code>" and
<var>response</var>'s <a for=response>type</a> is "<code>opaqueredirect</code>"

Expand Down Expand Up @@ -8303,6 +8330,7 @@ Axel Rauschmayer,
Ben Kelly,
Benjamin Gruenbaum,
Benjamin Hawkes-Lewis,
Benjamin VanderSloot,
Bert Bos,
Björn Höhrmann,
Boris Zbarsky,
Expand Down