Skip to content

MPP-4282: (bugfix) Removed Hardcoded Domain in PROD #5730

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

Merged
merged 1 commit into from
Jul 24, 2025

Conversation

vpremamozilla
Copy link
Collaborator

@vpremamozilla vpremamozilla commented Jul 18, 2025

This PR fixes MPP-4282

  • Removed fxa-mock.com and basket-mock.com from mock data to prevent leaking them into production.
  • Replaced those domains with safe local URLs like http://localhost/mock/fxa.
  • Changed static imports of mock files in _app.page.tsx to dynamic import() so they’re only loaded in mock mode.
  • This keeps test-only code and URLs out of the production JavaScript bundle.

How to test:

  • l10n changes have been submitted to the l10n repository, if any.
  • I've added a unit test to test for potential regressions of this bug.
  • I've added or updated relevant docs in the docs/ directory.
  • All UI revisions follow the coding standards, and use Protocol / Nebula colors where applicable (see /frontend/src/styles/colors.scss).
  • Commits in this PR are minimal and have descriptive commit messages.

@vpremamozilla vpremamozilla force-pushed the MPP-4282-bugfix-domain-mock-leak branch from 15d04ff to b9764a2 Compare July 18, 2025 17:00
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

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

On this branch, I ran:

npm run build
...
ag mock.com

And it still found this occurence of mock.com in the out/_next/static/chunks/14603.1ff5bc9cb9342e89.js output file.

... t.push(r.L.post("https://basket-mock.com/news/subscribe/" ...

Looking at the out/_next/static/chunks/14603.1ff5bc9cb9342e89.js.map file, it looks like this comes from:

handlers.push(
    http.post("https://basket-mock.com/news/subscribe/", (_info) => {

in the src/apiMocks/handlers.ts file.

What I don't understand is: why is the apiMocks directory included in our built JS file in the first place? It shouldn't need to be there right?

It seems like the full fix here is to figure out why the mock files are included in the JS build file at all?

@vpremamozilla
Copy link
Collaborator Author

On this branch, I ran:

npm run build
...
ag mock.com

And it still found this occurence of mock.com in the out/_next/static/chunks/14603.1ff5bc9cb9342e89.js output file.

... t.push(r.L.post("https://basket-mock.com/news/subscribe/" ...

Looking at the out/_next/static/chunks/14603.1ff5bc9cb9342e89.js.map file, it looks like this comes from:

handlers.push(
    http.post("https://basket-mock.com/news/subscribe/", (_info) => {

in the src/apiMocks/handlers.ts file.

What I don't understand is: why is the apiMocks directory included in our built JS file in the first place? It shouldn't need to be there right?

It seems like the full fix here is to figure out why the mock files are included in the JS build file at all?

Somewhere in the codebase, a file statically imports apiMocks/handlers.ts or something that imports it.

Webpack (and Next.js) includes everything reachable via static import in the bundle — regardless of runtime conditions.

So even if mock logic is gated behind process.env.NEXT_PUBLIC_MOCK_API, the moment you do this:

import { handlers } from "../apiMocks/handlers";

…the entire file is bundled.

let me check every place this is imported and make that fix. great callout!

@vpremamozilla vpremamozilla force-pushed the MPP-4282-bugfix-domain-mock-leak branch 2 times, most recently from 1b93d24 to 2c57ecc Compare July 21, 2025 21:09
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

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

Okay it's coming along. And we might be able to merge this as-is.

But, I'd still like to try to make sure no mocks are leaking into the production build. And now that we've changed the mock hosts to localhost, I did a search for localhost and found there are still 5 .js files in the out/ directory with localhost in them ...

npm run build
...
ag -l localhost out/

out/_next/static/chunks/8e9ea143.0567fc3a1c6cd547.js.map
out/_next/static/chunks/16659.5b061ed07f8b8f85.js
out/_next/static/chunks/14603.e5179c17c3c04895.js
out/_next/static/chunks/14596.abac252af574c9d1.js.map
out/_next/static/chunks/14603.e5179c17c3c04895.js.map
out/_next/static/chunks/14596.abac252af574c9d1.js
out/_next/static/chunks/pages/tracker-report-01173556999ea1fe.js.map
out/_next/static/chunks/16659.5b061ed07f8b8f85.js.map
out/_next/static/chunks/polyfills-42372ed130431b0a.js
out/_next/static/chunks/8e9ea143.0567fc3a1c6cd547.js

@vpremamozilla vpremamozilla force-pushed the MPP-4282-bugfix-domain-mock-leak branch from 2c57ecc to 3ff7ce2 Compare July 23, 2025 18:38
@vpremamozilla vpremamozilla force-pushed the MPP-4282-bugfix-domain-mock-leak branch from 3ff7ce2 to d2bda49 Compare July 23, 2025 21:16
price: 1.99,
currency: "EUR",
url: "https://payments-next.stage.fxa.nonprod.webservices.mozgcp.net/relay-premium-127/yearly/landing",
let mockedRuntimeData: RuntimeData;
Copy link
Member

Choose a reason for hiding this comment

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

question (non-blocking): So does moving this from export const into if (process.env.NEXT_PUBLIC_MOCK_API === "true") help prevent it from being automatically exported?

Comment on lines 20 to 21
// necessary to prevent mocks leaking into prod
// eslint-disable-next-line @typescript-eslint/no-require-imports
Copy link
Member

Choose a reason for hiding this comment

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

suggestion (non-blocking): move this docstring up to the top of this if block?

@vpremamozilla vpremamozilla force-pushed the MPP-4282-bugfix-domain-mock-leak branch from d2bda49 to 76ddecf Compare July 24, 2025 00:31
@vpremamozilla vpremamozilla force-pushed the MPP-4282-bugfix-domain-mock-leak branch from 76ddecf to 2ff879e Compare July 24, 2025 00:58
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

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

This removes the problematic hard-coded domains. Can you file a follow-up ticket to finish the work to prevent the mock & testing code from getting into the production build files?

@groovecoder groovecoder added this pull request to the merge queue Jul 24, 2025
Merged via the queue into main with commit c86eba5 Jul 24, 2025
32 checks passed
@groovecoder groovecoder deleted the MPP-4282-bugfix-domain-mock-leak branch July 24, 2025 15:28
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.

2 participants