proposal: `sandbox` feature improvements

[lowlighter]

Granular permissions

Instead of just sandbox: true, I think we should rely on Deno.PermissionOptionsObject, which would make it closer to what Deno.test and deno WebWorker offer in terms of API, in addition to offer more flexibility.

Also maybe a prompt option could be added to decide whether to use Deno.permissions.request or Deno.permissions.query ? Though I don't think there's many use case where someone would want to run some pages in interactive permissions and other in non-interactives). So maybe we can scrap this one.

Example:

await browser.newPage("https://example.com", {
 sandbox: { 
   prompt: false, 
   permissions: { net: ["google.com"], read: true }
 } 
});

Realm-aware permissions

If the above gets implemented, the #validateRequest method will need to be refactored.

Currently, we use the main realm Deno.permissions object, which also mean that any permissions accepted within sandbox also "leaks" to the main realm and could be see as a security issue.

To avoid re-implementing deno's logic of validating permissions, maybe the easier way to would be to spawn a new WebWorker with the provided permissions descriptor (new WebWorker(..., { deno: { permissions } }) and launch the validation requests from there.

It'd probably create a small overhead, but I think the tradeoff is acceptable as it's an advanced feature and improve security too.

Use the new --allow-imports for <script>

Currently we treat imports from script tag as "net" (like deno used to for dynamic imports), but they have now introduced a new "imports" permissions (though the query descriptor has been forgotten upstream: https://github.com/denoland/deno/issues/27050)

The fetch paused event of CDP is able to know what is trying to be loaded (https://chromedevtools.github.io/devtools-protocol/tot/Network/#type-ResourceType), in particular "Script" element.

So to match the new deno behavior, it may makes more sense to align too on this, since the <script> are in a sense dynamic imports.

It would also be a nice QOL, because you'd be able to distinguish scripts from other resources (images, etc.)

[lino-levan]

I'm a fan. This would be really cool. We should still allow an inherit mode or something like that but more granular control is cool.

[lowlighter]

The Deno.PermissionOptionsObject actually already allows the "inherit" (both at granular and global levels), and supports booleans too.

The only thing is that maybe "true" should be replaced by "inherit" by astral for QOL, because passing true when permissions are restricted in main realm throw escalation errors since you're trying to grant more permissions than allowed

[lowlighter]

I started some work about this, but unfortunately I'm blocked for now because of upstream issues

Blockers:

• https://github.com/denoland/deno/issues/29509

Blockers for using --import permissions for <script>:

• https://github.com/denoland/deno/issues/29503
• https://github.com/denoland/deno/issues/27050

Proposed API:

export type SandboxOptions = {
  sandbox?: boolean | { permissions: "inherit" | "none" | Pick<Deno.PermissionOptionsObject, "read" | "net"> };
};

The advantage of the type above is that we can later on add --import support without any breaking changes by just patching the typing

I intend to perform the the granular permissions checks with this code (when true/"inherit", it'll use directly the main thread like currently to avoid the overhead of spawning a worker)

const { promise, resolve } = Promise.withResolvers<Deno.PermissionStatus>()
const worker = new Worker(`data:,self.close(postMessage(Deno.permissions.requestSync(${JSON.stringify(descriptor)})))`, { type: "module", deno: { permissions } })
worker.onmessage = (({data:status}) => resolve(status))
return promise

Some nice related additions:

• https://github.com/denoland/deno/issues/29499

[lino-levan]

Thanks for the update. Really cool direction for Astral to go in imo.

[lino-levan]

Part one of this issue is now done with #161, I guess part 2 is blocked on upstream.

[lowlighter]

Opened upstream:

• https://github.com/denoland/deno/pull/29610

This should be enough to unblock (https://github.com/denoland/deno/issues/29503 is not actually required)

[lino-levan]

Landed in Deno 2.4! This is great.