Noting: https://github.com/mozilla/standards-positions/issues/732

Also: https://github.com/WebKit/standards-positions/issues/28

Hi @a-sully – are there versions of the spec and explainer markdowns that can be directly linked to, rather than un-merged PRs? That would greatly help the review.

Hi @a-sully - thanks for this. We're reviewing today and noting that this is built on top of File System Access which we previously reviewed positively. Can we set up a session where y'all join us to discuss the security issues, multi-stakeholder interest, abuse cases, and potential mitigations?

I'm not sure if this is already covered, but there needs to be a limitation on how often this method can be called. Likely restricted to a single call per user activation.

Otherwise, a malicious site can use the fact that existing files can't be overwritten to probe for the existence of other files that the user has not granted access to.

Hi @a-sully – are there versions of the spec and explainer markdowns that can be directly linked to, rather than un-merged PRs? That would greatly help the review.

No, not at the moment. There had been some earlier discussions as to where these types of explainers should go but that thread was never resolved... I can attempt to merge them into whatwg/fs and cite this as justification :)

Hi @a-sully - thanks for this. We're reviewing today and noting that this is built on top of File System Access which we previously reviewed positively. Can we set up a session where y'all join us to discuss the security issues, multi-stakeholder interest, abuse cases, and potential mitigations?

Absolutely! (and sorry, I realized I had accidentally been filtering emails related to this repo. I should be more responsive going forward)

I'm not sure if this is already covered, but there needs to be a limitation on how often this method can be called. Likely restricted to a single call per user activation.

Otherwise, a malicious site can use the fact that existing files can't be overwritten to probe for the existence of other files that the user has not granted access to.

Correct! This is discussed in the explainer; both on requiring user activation if you don't have permission and overwriting existing files.

We expect this to be sufficient, but can add usage limitations later on if needed

Hi @a-sully – are there versions of the spec and explainer markdowns that can be directly linked to, rather than un-merged PRs? That would greatly help the review.

No, not at the moment. There had been some earlier discussions as to where these types of explainers should go but that thread was never resolved... I can attempt to merge them into whatwg/fs and cite this as justification :)

Following up on this - see https://github.com/whatwg/fs/pull/108

In that PR I mention "the anti-pattern of putting the useful text only in the explainers and not in the spec text itself". Unfortunately this feature is currently in a worse state than even that, since the spec PR for this feature (https://github.com/whatwg/fs/pull/10) has yet to land at all. For months, specifying move() (among many other things) was blocked on answering the fundamental question posed in https://github.com/whatwg/fs/issues/59 of what exactly is a FileSystemHandle

Fortunately, this was just recently resolved in https://github.com/whatwg/fs/pull/96. There's now only one structural blocker to getting the move() method specified, which I hope will be merged soon: https://github.com/whatwg/fs/pull/95

In the meantime, I'll be updating https://github.com/whatwg/fs/pull/10 to include more information about the restrictions discussed in the hopefully-soon-to-be-merged explainers/MovingNonOpfsFiles.md

Apologies for the chaos! The move() method has a complicated history (it's been shipped in Chromium browsers for only files in the OPFS for about a year now) that I'm still working to make right. Please don't hesitate to reach out if it would be useful for me to join a session with y'all

FYI: the explainer for this feature has a new home https://github.com/whatwg/fs/blob/main/proposals/MovingNonOpfsFiles.md

Hi - Thanks for the update on the explainer. One thing that is still blocking this from our perspective is that the Spec is not self-contained so it's difficult to understand what we're reviewing - especially the privacy & security considerations. In the explainer it says "User agents are recommended to perform security checks on files moved within the local file system" but that isn't in the linked PR. And one issue we're concerned about is the strength of this "recommendation" and whether it's appropriate to the power of this API - especially when it comes to security. If there's a self-contained spec, can you please amend the review to point to that?

Hi @torgo,

The move() spec PR has been blocked on several infrastructural changes and at this point is a bit out of date. I expect https://github.com/whatwg/fs/pull/131 is the last remaining piece we need to finally be able to specify it properly, at which point I'll go back and update the spec PR

In the explainer it says "User agents are recommended to perform security checks on files moved within the local file system" but that isn't in the linked PR. And one issue we're concerned about is the strength of this "recommendation" and whether it's appropriate to the power of this API - especially when it comes to security.

Regarding this point specifically - we recent added to the spec the ability to distinguish between files on the local file system vs. files in a Bucket File System (f.k.a. OPFS). This now allows us to specify within the move() algorithm something like:

1. If [=this=] [=FileSystemHandle/is in a bucket file system=], run [=implementation-defined=] malware checks

Hi @a-sully - can you clarify what do you mean by "distinguish"? Do you mean that the spec itself would mandate a higher security level for anything to do with local file system files vs. "bucket file system" files? Is there additional normative spec text that could be therefore added to protect users from bad outcomes (i.e. necessary security guarantees)?

can you clarify what do you mean by "distinguish"

We now have the ability to say "this FileSystemHandle corresponds to a file in a bucket file system" (as opposed to the local file system). See https://fs.spec.whatwg.org/#filesystemhandle-is-in-a-bucket-file-system.

I expected the move() method to be specified similarly to the existing specification for running malware checks when closing a FileSystemWritableFileStream:

Run implementation-defined malware scans and safe browsing checks. If these checks fail, reject closeResult with an "AbortError" DOMException and abort these steps.

"Implementation defined" is broad enough that each implementation could choose to skip malware checks for files in the bucket file system - which I expect is the case currently across all browsers. We could be explicit that these checks must not run for files in the bucket file system (as I sketched out above). That would remove some theoretical flexibility on the part of the user agent, but in practice I don't expect that to be an issue (cc @jesup @szewai @annevk)

Meanwhile, there's the question @torgo posed about running security checks for local files ("necessary security guarantees"). From my perspective (please correct me if I'm misunderstanding), there are two pieces to this:

1. Can we specify these security checks in more detail, say like downloads? (i.e. can we do better than just "implementation-defined"?)
2. Should we specify that security checks must be run when saving or moving files on the local file system?

I don't think we want (2). See the non-normative language here: https://wicg.github.io/file-system-access/#security-malware

user agents are encouraged to verify the contents of files modified by this API via malware scans and safe browsing checks, unless some kind of external strong trust relation already exists

The user agent should have the flexibility to skip security checks in some scenarios (e.g. the user agent can determine what qualifies as a "strong trust relation", how it's established, etc)

Regarding (1), I'm open to discussing adding more detail (and since this also affects the existing specification of FileSystemWritableFileStream, we may want to move this discussion to a new issue on the FS spec). But without (2) we can't guarantee any security behaviors

I've just revamped https://github.com/whatwg/fs/pull/10/ to make use of the various infrastructural changes I mentioned above and to include the restrictions discussed in the explainer for non-BucketFileSystem moves. Feel free to leave comments here or on that PR

A few things to note:

• The steps for requesting access to the destination are still subject to change depending on what we decide in https://github.com/whatwg/fs/pull/131
• No browsers yet support directory moves outside of the BucketFileSystem, so there are likely more restrictions we'll want to add that - may require some acrobatics with the https://github.com/wicg/file-system-access spec. For example, we should not allow well-known directories to be moved

Hi @a-sully thanks for that and thanks for listening to our comments and concerns. Our main concern continues to be security-related issues related allowing access to the local filesystem. In the case where different UAs have made different security choices about this capability, does the API allow for graceful fallback when the browser in question doesn't support file moves in the local file system? Is that detectable? Have you thought about how this would behave differently with isolated web apps #842 ?

does the API allow for graceful fallback when the browser in question doesn't support file moves in the local file system

On browsers for which only the Bucket File System is supported, well, there is currently no way to get a FileSystemHandle corresponding to the local file system at all. But if a browser wanted to support the local file system in a limited capacity - say, by allowing read-only access to files which are dragged-and-dropped or selected via a file picker - then the requirement of "readwrite" permission to (at least) the source file would result in the move() method reliably throwing a NotAllowedError exception

Is that detectable?

Were other browsers to implement (even partial) support for the local file system, that could include upstreaming the FileSystemHandle.queryPermission() method (and possibly also the FileSystemHandle.requestPermission() method) from the WICG spec to the whatwg spec. The website could then check queryPermission({ mode : "readwrite" }) before bothering to try to move() the file

Have you thought about how this would behave differently with isolated web apps https://github.com/w3ctag/design-reviews/issues/842 ?

The permission requirements mentioned above will also apply to IWAs. If the user agent wanted to somehow privilege IWAs - say, by allowing only IWAs to call move() - that would be facilitated by granting the appropriate File System permissions to the IWA. The means of handing out File System permissions for the local file system - to IWAs or otherwise - will presumably remain implementation-defined

Hi folks - thanks for your responses and thanks for addressing our concerns. We're happy with the proposal as it stands, however I think the main concern we have now relates to multi-stakeholder support for the file system access API itself. We encourage you to work with other stakeholders / implementers to achieve a consensus-based approach to this capability.