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

Adds rust_to_wasm and rust_to_wasm support to transpile_web_ui #27215

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

szilardszaloki
Copy link
Collaborator

@szilardszaloki szilardszaloki commented Jan 13, 2025

Resolves brave/brave-browser#43273.

Depends on https://github.com/brave/devops/issues/12869 — the PR itself can be merged, but using rust_to_wasm (wasm-pack) will require wasm32-unknown-unknown and rust-lld to be present.


rust_to_wasm example:

rust_to_wasm("build_rust_packages") {
  rust_packages = [
    rebase_path("some_resource/rust_package_1/Cargo.toml"),
    rebase_path("some_resource/rust_package_2/Cargo.toml"),
  ]
}

transpile_web_ui example:

ui/webui/resources/some_resource/rust_package_1/Cargo.toml:

[package]
name = "rust_package_1"
edition = "2018"

[lib]
crate-type = ["cdylib"]

[dependencies]
wasm-bindgen = "0.2"

ui/webui/resources/some_resource/rust_package_1/src/lib.rs:

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub fn return42() -> i32 {
    return 42;
}

ui/webui/resources/some_resource/rust_package_2/Cargo.toml:

[package]
name = "rust_package_2"
edition = "2018"

[lib]
crate-type = ["cdylib"]

[dependencies]
wasm-bindgen = "0.2"

ui/webui/resources/some_resource/rust_package_2/src/lib.rs:

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub fn return24() -> i32 {
    return 24;
}

ui/webui/resources/some_resource/index.js:

import { return42 } from 'gen/brave/ui/webui/resources/some_resource/rust_package_1/pkg'
import { return24 } from 'gen/brave/ui/webui/resources/some_resource/rust_package_2/pkg'
export { return42, return24 }

ui/webui/resources/BUILD.gn:

group("grdp") {
  public_deps = [
    # ...
    ":some_resource",
    # ...
  ]
  # ...
}
# ...
transpile_web_ui("some_resource") {
  resource_name = "some_resource"
  generate_grdp = true
  resource_path_prefix = "brave"

  entry_points = [ [
        "some_resource",
        rebase_path("some_resource/index.js"),
      ] ]

  rust_packages = [
    rebase_path("some_resource/rust_package_1/Cargo.toml"),
    rebase_path("some_resource/rust_package_2/Cargo.toml"),
  ]

  module_library_type = true
  output_module = true
  public_asset_path = "chrome://resources/brave/"
  xhr_wasm_loading = true
}
# ...

ui/webui/resources/sources.gni:

# ...
brave_resources_extra_grdps = [
  # ...
  "$root_gen_dir/brave/web-ui-some_resource/some_resource.grdp",
  # ...
]
# ...

ui/webui/resources/tools/bundle_js_excludes.gni (patch):

BUNDLE_JS_EXCLUDES = []
foreach(excluded_file,
        [
          # ...
          "resources/brave/some_resource.bundle.js",
          # ...
        ]) {
  BUNDLE_JS_EXCLUDES += [
    "chrome://${excluded_file}",
    "//${excluded_file}",
  ]
}

In the WebUI page:

// @ts-ignore
import('chrome://resources/brave/some_resource.bundle.js')
  .then(some_resource => console.log(
    some_resource.return42(),
    some_resource.return24()
  ))

Note: you'll need wasm-unsafe-eval in CSP, as by default we're not allowed to compile WASM in a WebUI (see here and here). This, however, would allow arbitrary WASM code execution in the page. Since at this time there doesn't seem to exist a CSP directive to specify same-origin WASM execution, or file hashes (i.e. script-src for WASM), the only legit way to isolate the WASM is to embed a chrome-untrusted:// frame into the WebUI, have the WASM compile and execute there, and add a communication mechanism to the chrome-untrusted:// frame.

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally:
    • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
    • npm run presubmit wiki, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@szilardszaloki szilardszaloki requested review from a team and thypon as code owners January 13, 2025 23:19
@github-actions github-actions bot added CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) CI/storybook-url Deploy storybook and provide a unique URL for each build labels Jan 13, 2025
Copy link

socket-security bot commented Jan 13, 2025

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteCI
Medium CVE npm/[email protected] ⚠︎

View full report↗︎

Next steps

What is a medium CVE?

Contains a medium severity Common Vulnerability and Exposure (CVE).

Remove or replace dependencies that include known medium severity CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@szilardszaloki szilardszaloki force-pushed the szilard/43273-adds-rust-to-wasm-support-to-transpile_web_ui branch 3 times, most recently from 12fdd7c to 3753b31 Compare January 15, 2025 19:39
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add this as a Git submodule rather than committing all the files to brave-core?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's see what @bridiver has to say about it. Last I checked with him, we wanted to add wasm-pack like cargo-audit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should keep this vendored like everything else

@szilardszaloki szilardszaloki force-pushed the szilard/43273-adds-rust-to-wasm-support-to-transpile_web_ui branch from 3753b31 to 123f48d Compare January 15, 2025 21:34
Copy link
Contributor

The security team is monitoring all repositories for certain keywords. This PR includes the word(s) "policy, csp" and so security team members have been added as reviewers to take a look.

No need to request a full security review at this stage, the security team will take a look shortly and either clear the label or request more information/changes.

Notifications have already been sent, but if this is blocking your merge feel free to reach out directly to the security team on Slack so that we can expedite this check.

@szilardszaloki szilardszaloki changed the title Adds Rust-to-WASM support to transpile_web_ui Adds rust_to_wasm and rust_to_wasm support to transpile_web_ui Jan 15, 2025
@szilardszaloki szilardszaloki force-pushed the szilard/43273-adds-rust-to-wasm-support-to-transpile_web_ui branch from 123f48d to 9a24c64 Compare January 16, 2025 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/run-audit-deps Check for known npm/cargo vulnerabilities (audit_deps) CI/storybook-url Deploy storybook and provide a unique URL for each build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Rust-to-WASM support
6 participants