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

Enable get() in fenced frames with network access revoked. #220

Merged
merged 12 commits into from
Feb 21, 2025

Conversation

VergeA
Copy link
Contributor

@VergeA VergeA commented Jan 21, 2025

After script running in a fenced frame successfully resolves a call to window.fence.disableUntrustedNetwork(), the fenced frame gains access to Shared Storage via get().

This patch refactors the get() algorithm to be accessible from Window and SharedStorageWorklet scopes, but the Window branch will fail outside of a fenced frame tree with network disabled.

This patch also specifies a new Permissions Policy, fenced-unpartitioned-storage-read, which can be used to disable access to get() in fenced frames. Its default allowlist is *.


Preview | Diff

After script running in a fenced frame successfully resolves a call to `window.fence.disableUntrustedNetwork()`, the fenced frame gains access to Shared Storage via `get()`.

This patch refactors the `get()` algorithm to be accessible from `Window` and `SharedStorageWorklet` scopes, but the `Window` branch will fail outside of a fenced frame tree with network disabled.
Merge main into read2
@VergeA VergeA marked this pull request as ready for review January 21, 2025 20:08
@VergeA VergeA changed the title [WIP] Enable get() in fenced frames with network access revoked. Enable get() in fenced frames with network access revoked. Jan 23, 2025
@VergeA
Copy link
Contributor Author

VergeA commented Jan 23, 2025

I think this should be ready for a first review. The spec roughly traces the path of the Chromium renderer code here, into the browser code here.

@xyaoinum, are you able to take a look at this? I don't have repo permissions to add reviewers directly. If not, I can find another reviewer.

@VergeA
Copy link
Contributor Author

VergeA commented Feb 13, 2025

Hi @xyaoinum, just wanted to follow up on this, can you give this patch a review when you get a chance?

Thanks!

@xyaoinum
Copy link
Collaborator

@VergeA My apologies for the delayed review! This LGTM!

@VergeA
Copy link
Contributor Author

VergeA commented Feb 13, 2025

Thank you! This is my first Shared Storage spec PR; is there anyone else who I should normally add as a reviewer before we merge?

@xyaoinum xyaoinum requested a review from wanderview February 13, 2025 18:06
@xyaoinum
Copy link
Collaborator

@wanderview Could you please take a look as well? Thanks!

On the other hand, methods for getting data from the [=shared storage database=] are exposed to the {{SharedStorageWorklet}} only, in order to carefully control the flow of data read from the [=shared storage database|database=].
On the other hand, methods for getting data from the [=shared storage database=] are exposed to the {{SharedStorageWorklet}} only, in order to carefully control the flow of data read from the [=shared storage database|database=]. The only exception is that {{SharedStorage/get()}} is exposed to {{Window}}, but will only succeed if the result of the [=determine if a navigable has fully revoked network=] algorithm is true.

Note: The [=determine if a navigable has fully revoked network=] algorithm ensures that {{SharedStorage/get()}} only succeeds for [=fenced frames=] that have successfully resolved a call to {{Fence/disableUntrustedNetwork()}}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is the "fully revoked" algorithm defined? I don't see it in this PR or in the existing spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's defined as a standalone algorithm in the Fenced Frames spec: https://wicg.github.io/fenced-frame/#gating-methods-on-network-revocation

I assumed since the build succeeds that the reference is being linked properly. Is there anything else I should do to clarify where this algo comes from?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. I was looking in the shared storage spec!

@VergeA
Copy link
Contributor Author

VergeA commented Feb 21, 2025

Thanks for the review Ben!

I don't have merge permissions on this repo, so could one of you (@wanderview, @xyaoinum) please press the button for me?

@wanderview wanderview merged commit a1bffbb into WICG:main Feb 21, 2025
2 checks passed
github-actions bot added a commit that referenced this pull request Feb 21, 2025
SHA: a1bffbb
Reason: push, by wanderview

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@VergeA VergeA deleted the read2 branch February 21, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants