-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Remove support for native file system API #11407
Comments
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
Fixes brave/brave-browser#11133 Fixes brave/brave-browser#11407 Fixes brave/brave-browser#11546 Fixes brave/brave-browser#11547 - Disables features via Chromium features: * Direct Sockets * Lang Client Hint Header * Signed Exchange Prefetch Cache For Navigations * Subresource Web Bundles - Disabled the following blink features in BraveContentRendererClient::SetRuntimeFeaturesDefaultsBeforeBlinkInitialization since they don't have Chromium features: * Digital Goods * Native File System (File System Access) - Added browser tests for these features - Modified redirect-cc.py to allow overriding files under out/XXX/gen - Added an override for blink::origin_trials::IsTrialValid and overrides for blink::OriginTrialContext methods AddFeature and AddForceEnabledTrials for trials: * NativeFileSystem2 * SignedExchangeSubresourcePrefetch * SubresourceWebBundles - Added browser tests for trials disablement.
@pes10k can you please provide reasons as for why this API is considered a security risk? A vague statement like the above doesn't help. If you have specific and actionable security concerns, I'd advise to voice them in the spec repo so they can be addressed. |
@dwelle the concern is just that this is an extremely powerful but risky feature, and so a potential footgun. High risk features carry the cost of requiring users to be further responsible for their own privacy, and so we're cautious about including or enabling such features. For many, the constrained, sandboxed nature of the web is part of the appeal. Brave doesn't have a strict policy about this, we do our best to make a case by case determination when weighing the benefits and costs of enabling. |
I just went to https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/webkitdirectory#result, which has the following innocent-looking code on it: <input type="file" id="filepicker" name="fileList" webkitdirectory multiple /> Brave (and all other browsers really) will happily let the website know about every. single. file. I have ever downloaded since purging my The File System Access API has some built-in constraints that make this sort of attacks less likely. |
@tomayac That seems like a nasty aspect of the HTML spec, agreed, but I don't follow the connection between the above and the Native File System API. They pose different risks; being able to overwrite or modify the users existing files seems like an important new privacy harm |
It's definitely an API design that shows its age, we both agree.
The File System Access API (it was renamed) prevents users from from uploading their whole
That's absolutely right, saving files to arbitrary locations is a different threat, but the built-in constraints make it impossible that someone overwrites important system files. The browser will also prompt before (over-)writing an existing file in the user-accessible part of the file system. |
I would add that not only is it opt-in (under a confirmation dialog) as @tomayac has pointed out, but it's also not a privacy risk, it's a data-loss risk. One that is inherent to doing anything meaningful with the file system, and it doesn't matter whether that file system resides on user's own drive, or in the cloud. If anything, by blocking your users to use local file system, you're pushing them to host their files remotely, which then does have a privacy risk.
I've not heard a sentiment saying that being unable to allow web apps to be more integrated with the OS is useful. On the contrary, our users at @excalidraw want and benefit from such integration. If you're giving users such agency so as to have opinions on the sandboxed nature of the web, you should give them agency to make their own decisions on whether they want to grant FS access to apps they're using. |
I hear what you all are saying, but allowing a page to maintain ongoing read and / or write handles to files on disk (even keeping in mind the files ruled out by the built-in constraints) is a category of privacy and data-loss risk that the platform currently doesn't allow. For example, currently, if I share a bunch of files with a site doing what @tomayac suggests in #11407 (comment), the site isn't going to learn about changes i make to those files until I go through another browser prompt; i can edit / modify / etc them as I please knowing the site wont see those changes. Under this proposal, a site will (or at least, can) see every change I make until I revoke permissions (if im reading the spec right, this is true even if I come back to the site in the future). This is an obvious, large difference in privacy risk!
The spec does not do this. The spec suggests to browser vendors that browser vendors determine which files are sensitive, and provides a list of possible suggestions. But the spec just doesn't itself define any files as off limits or sensitive. This is a common, maddening anti-pattern in specs, where specs both want to reserve to vendors the ability to apply arbitrary policies, but also claim that they spec is private/secure/etc because of this undefined behavior. |
Anyway, we may revisit our decision, but while the proposal is still being worked out, and for the time being, we think the feature is on the wrong side of the cost/benefit line for most of our users. I can say what we are considering more seriously, but haven't made a decision on, is whether to ship the proposal default off, and allow folks to enable the proposal through an experts-only flag. No decisions have been made there yet, but we're considering that as a general approach to non-standard (yet?) features implemented upstream |
Will also say, to take the temp down a bit from my previous message, that if Im misreading or misunderstanding the spec from the above discussion, I'd be very grateful for corrections! Because these not-yet-standardized features often change (both the text and in implementation), we tend to take a more conservative approach to these pre-standardized features. And, last, just wanted to say that even if we don't currently have the outcome / decision ya'll prefer, I appreciate that thats frustrating, but that I appreciate everyone in the thread taking the time to voice their opinions and force us to revisit and reconsider our decisions. |
We're seeing steady growth of the feature in Chrome. Applications from different genres start using the API:
For the permissions point: the spec leaves UAs a lot of choice, for example, UAs could implement a directory picker in such a way that the user would pick a directory, but then one-by-one green-light the actual files in a secondary, UA-specific picker. In the extreme case, UAs could even ask before every single read or write operation as to making it super transparent to the user when disk access happens. |
This is unfortunate as this causes many devs to tell their users to install Chrome which has even more privacy issues. We're planning a PWA now and I was hoping to be able to tell users to use Brave but it seems like we might have to encourage them to use Chrome instead for full functionality. A shame, really. |
I understand your frustration but the ability to set permissions that allow for long lived disk access, even when revisiting a site is a serious privacy harm, and an enormous footgun we're not going to load and hand to our users. I appreciate @tomayac point, that the spec is sufficiently vague in privacy protections that one could maybe implement it in such away that it was privacy protecting. But vagueness in privacy protections in a spec is not a feature, its a bug and a sign of a proposal that isn't solving the privacy risks it introduces. I'll mention again what i said in #11407 (comment), that we're considering having these default off, and expert enable-able through flags, but we don't have any immediate plans to implement such a policy. TL;DR; this is a spec with serious privacy risks, and protecting our users from those risks is higher priority right now than either 1) dedicating developer time to figure out how to implement this not-yet-standard in a privacy preserving way, or 2) supporting these use cases that we don't think are core to why folks want to use Brave. As always, decision is subject to change at any point, but #11407 (comment) is the current position. If sites need to use privacy-risking features, thats (at least for now) what other browsers are for ;) |
OK! Well we are looking forward to the day we can have more full-featured local-first applications in Brave! I will be following this thread in case things change and we can start recommending brave instead of chrome for this work. |
@pes10k This might be different for PWA apps. |
@samueljim, I think I explained my concerns poorly. On my initial read through the spec, I have two concerns:
And then, finally, I'm very nervous about APIs that are not (yet?) standards (and so may change out from underneath us at any time) that do very privacy sensitive things. Thats not a controlling concern, but its a reason we're extremely wary about enabling non-standard, under development features that are privacy risky. |
Maybe @mkruisselbrink as the spec author can speak to this. |
Valid concerns on non-standard api. I hope it will soon become standard with privacy/security issues solved. Until that day, I don't see anything wrong with flag. We shouldn't choose what is correct, people should. Concerns can be explained in flags page. |
I might be missing smth, but what flag can I use to enable this feature? |
Wondering if there's a flag as well? I remember this being supported previously, so was surprised when it didn't work. Tried enabling a few promising-looking flags, but none seemed to work. Thanks. |
@pes10k this is incorrect, pages can store a file handle but if a page is closed (including all the tabs of that subdomain), browser prevents access to the handle and shows a user a prompt to regrant the permission. Here is an example from my note taking app https://bangle.io , which shows explicitly the prompt message for the first time a user selects a directory: And once you close the website and visit again, the browser shows this prompt: This feature is not a dangerous feature and should not be dismissed just because the highlight of this feature is the ability to read users files. As I mentioned above there are proper safe guards in the spec which give the power to the user to decide whether they trust the website or not and also prevent accessing sensitive directories. I kindly request you to rethink this, as it a monumental step in giving power to the web apps and enabling things that weren't possible before. With this said, I am curious to hear what your thought process is like for enabling this feature for brave users. |
Thank you for the notes @kepta ! Folks can find more details for how we're considering dealing with non-standardized, privacy-risking, still changing Chrome features here: #9586 (comment). But the TL;DR; here is:
|
@pes10k, I'm just a nobody but I really think that while you and the rest of your team handle Brave's adoption of Chrome features amazingly well, disabling the file system access api is overly protective. The guards in place are enough and the power and consistency that it affords developers as a superior alternative to indexedDB and localStorage are easily worth it, even on the balance of 'millions of users'. Your second point is really weak and the first and third are just the reasonable amount of bias I expect anyone to have with the Chrome team. You can serialize a handle into storage but how is the 3rd party script going to read it? IndexedDB? LocalStorage? Neither are available cross origin. If you're really worried about other attack avenues just lock it down to one directory that you specify. Please prioritize researching the api more thoroughly because right now it looks like your conclusion isn't well supported. I don't know what else you have on your plate but this one shouldn't take long. |
To be blatantly honest, it looks like a decision was made first and then you all decided to look for ways to support that decision.
The spec is pretty spec-d out, as far as reading and writing files is considered with millions of chrome devices out in the wild using it.
There is a fine line between thinking that you are protecting your users and actually protecting your users. Limiting the advancement of the web ecosystem on the pretense of security, "oh that poor user doesn't know what they are doing" and "oh this sounds privacy invasive and is coming from google, lets block it without proper research". This API by design is 'opt in' --- the user has to take an action (clicking 'yes' on a prompt) to even make a malicious attack feasible in the first place.
I would really be interested in hearing how you think this is privacy risky. |
I'm locking this conversation. Brave issues are for keeping track of bugs and things Brave plans on implementing, not to debate Brave product decisions. I've already shared Brave's plans for features like this. As mentioned before, this plan does not currently have an implementation timeline, but when implemented would allow users to enable the feature through brave://flags. Further, the conversation has drifted from the substance of the issue, to presuming motives or assuming ignorance, which is definitely not what these issues are for. Ya'll, I appreciate we disagree. Thats okay and inevitable given the size of the Web. But, without any snark or sarcasm meant, it shouldn't be a surprise to anyone that Brave considers some things privacy risks that other browsers (including Chrome) don't consider privacy risks. A partial list of Brave's concerns (already discussed above) include:
Next, even though I expect they don't like or approve of the decision here, I'll just say again that I appreciate the testing and additional information provided by @kepta in #11407 (comment). Finally, a correction:
I think there is a misunderstanding here. 3p scripts can read the storage of the frame they're executing in. If a 1p script writes something into storage (say… a file handle), nothing prevents a 3p script (or any other script), executing in the same frame, from reading that value out of storage. The "cross origin" limitation mentioned above prevents a script (3p or otherwise) from accessing storage for a eTLD+1 other than the eTLD+1 of the frame the script is being executed in. With storage, as with most other parts of the Web platform, access is scoped to the eTLD+1 of the frame code is executing in, not the eTLD+1 that the code came from. |
Chromium is shipping a native file system API. The privacy / security model is extemely not fleshed out, but its shipping enabled in chromium. We should disable
cc @jumde
Spec: https://github.com/WICG/native-file-system/blob/master/EXPLAINER.md
The text was updated successfully, but these errors were encountered: