Skip to content

ESD-1624: Stop trusting the page-writable apiURL global in WASM login#513

Merged
Phil-Browne merged 5 commits into
mainfrom
esd-1624-wasm-apiurl-trust
Jul 15, 2026
Merged

ESD-1624: Stop trusting the page-writable apiURL global in WASM login#513
Phil-Browne merged 5 commits into
mainfrom
esd-1624-wasm-apiurl-trust

Conversation

@Phil-Browne

Copy link
Copy Markdown
Contributor

The WASM login path read the outbound API base URL from window.megaportToken.apiURL, a page-writable JS global. Any other script on the page (XSS, a compromised widget, a browser extension) could overwrite it after login and redirect the bearer token to an attacker host, defeating the hostname validation setAuthToken already does.

Both login call sites now read the URL from the MEGAPORT_API_URL env var that setAuthToken sets after validating the portal hostname. The apiURL field on the JS global stays for UI display only, since nothing security-sensitive reads it back.

  • loginFunc (token path) and newUnauthenticatedClientFunc now source the base URL from os.Getenv("MEGAPORT_API_URL").
  • New js,wasm tests assert both paths route to the validated host even when the JS global is tampered to an attacker URL.
  • Added a wasm.yml step so the config package's wasm tests actually run under node in CI (previously compiled but never executed).

Copilot AI review requested due to automatic review settings July 10, 2026 00:26
@Phil-Browne Phil-Browne requested review from a team and penzeliz-megaport as code owners July 10, 2026 00:26
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.29%. Comparing base (e2074d8) to head (3c8573c).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #513   +/-   ##
=======================================
  Coverage   79.29%   79.29%           
=======================================
  Files         193      193           
  Lines       18683    18683           
=======================================
  Hits        14814    14814           
  Misses       2820     2820           
  Partials     1049     1049           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the WASM authentication flow by ensuring the API base URL used for authenticated and unauthenticated clients is sourced from the hostname-validated MEGAPORT_API_URL env var (set by setAuthToken) rather than a page-writable JS global, reducing risk of token exfiltration via URL redirection.

Changes:

  • Updated WASM login and unauthenticated client creation to read the API base URL from os.Getenv("MEGAPORT_API_URL") instead of window.megaportToken.apiURL.
  • Added js,wasm unit tests to ensure tampering with the JS global cannot redirect API traffic away from the validated host.
  • Updated CI workflow to actually execute the config package’s WASM tests under node.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/commands/config/login_wasm.go Stops trusting page-writable apiURL JS global; uses validated env-var API base URL for client routing.
internal/commands/config/login_wasm_test.go Adds WASM tests to assert routing remains pinned to the validated env-var host even if JS globals are tampered.
.github/workflows/wasm.yml Runs the new WASM config tests in CI so the regression coverage executes.

Comment thread internal/commands/config/login_wasm.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@penzeliz-megaport penzeliz-megaport left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed for correctness; no findings. LGTM.

…trust

# Conflicts:
#	internal/commands/config/login_wasm.go
#	internal/commands/config/login_wasm_test.go
@Phil-Browne

Copy link
Copy Markdown
Contributor Author

Merge conflict resolved in c3d43a4

projectsuperba
projectsuperba previously approved these changes Jul 15, 2026
Comment thread internal/commands/config/login_wasm.go Outdated
@Phil-Browne Phil-Browne merged commit dee5b8d into main Jul 15, 2026
10 checks passed
@Phil-Browne Phil-Browne deleted the esd-1624-wasm-apiurl-trust branch July 15, 2026 20:41
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.

4 participants