From dee3db5fad09da9989df82a2231f5b995d964bee Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 20 Oct 2026 18:55:59 -0700 Subject: [PATCH 01/31] docs: plan local Worker development --- plans/104387-local-cloudflare-worker.md | 160 ++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 plans/104387-local-cloudflare-worker.md diff --git a/plans/104387-local-cloudflare-worker.md b/plans/104387-local-cloudflare-worker.md new file mode 100644 index 000000000000..90c71431ce47 --- /dev/null +++ b/plans/104387-local-cloudflare-worker.md @@ -0,0 +1,160 @@ +# Run the Cloudflare Worker during local web development + + + +## Goal and scope + +Implement [Expensify/App#104387](https://github.com/Expensify/App/issues/104387): `npm run web` should run the actual NewDot Worker in local workerd through Wrangler/Miniflare, in front of the live Rsbuild server. Developers should encounter CSP, nonce, header, and routing problems locally, without sacrificing hot reload or requiring a Cloudflare account, a built `dist/`, or ngrok. + +This document is a plan, not an implementation or a claim that the proposed topology has already passed browser testing. + +- Base: freshly fetched App `origin/main`, `bc54f602df745cd1104297e7e1e230f8f130f1e8`. +- Branch: `plan/104387-local-cloudflare-worker`. +- Worktree: `/Users/rory/Expensidev/App-Worktrees/104387-local-cloudflare-worker`. +- Keep all implementation in App. Do not change live Cloudflare routes, deploy anything, modify the legacy Worker repository, or alter unrelated worktrees. + +## Findings that shape the solution + +1. Root `package.json` currently starts `web/proxy.ts` and Rsbuild concurrently; Rsbuild opens its own HTTPS origin. The Worker is a separately locked npm package, not a root workspace. +2. `web/cloudflare-worker/src/index.ts` already owns security headers, CSP nonces, HTML cache invalidation, AASA handling, missing-asset handling, and immutable caching. Use that implementation, not a separate imitation of the Worker in Rsbuild middleware. +3. The current Worker config requires `../../dist` and defaults to the staging CSP. Its static-assets/R2 tests and built-site preview commands must continue to work unchanged in meaning. +4. Rsbuild currently selects an available port starting at 8082 and embeds it as `process.env.PORT`; `CONFIG.DEV_PORT` uses that value. Moving the browser-facing origin without updating this contract can break links even if the first page loads. +5. `USE_WEB_PROXY=false` already disables the API proxy. Preserve that meaning. Add a separate Worker opt-out rather than silently repurposing this flag. +6. The old [local-testing instructions](https://github.com/Expensify/Cloudflare-Workers/tree/main/new.expensify.com#testing-locally) use ngrok to reach Rsbuild. [Cloudflare-Workers#188](https://github.com/Expensify/Cloudflare-Workers/pull/188) explicitly describes a Wrangler/custom-HTTP-port problem, linking [workers-sdk#5774](https://github.com/cloudflare/workers-sdk/issues/5774#issuecomment-2779972187). Separately, [App#38072](https://github.com/Expensify/App/issues/38072#issuecomment-2113231238) records an older `NODE_EXTRA_CA_CERTS` workaround. These are historical clues, not proof that the same problems affect the currently pinned Wrangler 4.144.0. + +Reference code at the base revision: + +- [Web scripts and installation](https://github.com/Expensify/App/blob/bc54f602df745cd1104297e7e1e230f8f130f1e8/package.json#L12-L35) +- [Rsbuild development configuration](https://github.com/Expensify/App/blob/bc54f602df745cd1104297e7e1e230f8f130f1e8/config/rsbuild/rsbuild.config.ts#L51-L89) +- [Worker response pipeline](https://github.com/Expensify/App/blob/bc54f602df745cd1104297e7e1e230f8f130f1e8/web/cloudflare-worker/src/index.ts#L83-L130) + +## Preferred architecture: HTTPS outside, loopback HTTP inside + +```text +Browser: https://dev.new.expensify.com:8082 + -> Wrangler / local workerd (existing mkcert certificate) + -> http://127.0.0.1: (Rsbuild, live in-memory output) + -> existing API proxy on port 9000, when enabled + -> configured backend + +Browser HMR: wss://dev.new.expensify.com:8082/rsbuild-hmr + -> Worker WebSocket pass-through -> Rsbuild +``` + +Use 8082 as the preferred public port, preserving the existing hostname, browser storage, secure-context behavior, and familiar setup instructions. Select a distinct internal Rsbuild port. Share the resolved public and internal ports with all children; never let Rsbuild independently increment away from the Worker's configured upstream. On conflicts, select a coordinated available pair or fail with an actionable error, never attach to another worktree's server. Embed the **public** port in `CONFIG.DEV_PORT`. + +The Worker must construct an explicit upstream URL with scheme, loopback address, port, path, and query. Do not depend on the old `dev.host`/implicit-origin forwarding behavior. Loopback HTTP avoids Worker-to-Rsbuild certificate validation entirely; the browser still uses trusted HTTPS and WSS. It does not justify allowing HTTP assets or insecure WebSockets in the browser. + +### First implementation gate: prove the transport before plumbing everything in + +- With pinned Wrangler and a minimal loopback HTTP/WebSocket upstream, verify custom-port fetch, streaming, POST forwarding, and a 101 upgrade through local workerd. +- Start Wrangler HTTPS using `--local-protocol https`, `--https-key-path`, and `--https-cert-path` pointing to the existing `config/rsbuild` mkcert files. Verify certificate trust in the browser, not just `curl -k`. +- Then repeat against real Rsbuild, checking its Host/Origin validation, HMR token/query handling, and emitted asset URLs. +- If WebSocket pass-through is incompatible with the current runtime, evaluate a direct **WSS** HMR connection to an HTTPS Rsbuild listener as the narrow fallback; keep page/assets behind the Worker and document the exact development-only CSP addition. Do not use `ws:` from an HTTPS page. +- Only retain ngrok as an explicitly documented troubleshooting fallback if a reproducible pinned-version issue remains. Do not require a tunnel for the default workflow, globally disable TLS verification, or clear `NODE_EXTRA_CA_CERTS` as a blanket fix. + +## Implementation sequence + +### 1. Install Worker tooling during normal setup + +Keep the Worker as an independently locked package for minimal CI/deploy risk. Add a deterministic `npm ci --prefix web/cloudflare-worker --include=dev` step to root `scripts/postInstall.sh`, with clear failure output. Regenerate the Worker lockfile only if dependencies actually change; no Wrangler upgrade is necessary just to implement this issue. + +Verify root `npm install`, root `npm ci`, and standalone setup install the Worker without recursive root lifecycle execution. Inspect HybridApp/CI callers before finalizing the hook; account for environments that intentionally omit development dependencies. Avoid assuming an npm install should be network-free, but ensure launching an already-installed `npm run web` never installs packages or asks for Cloudflare login. If inspection shows the hook is unsuitable, choose workspace integration explicitly and update all standalone Worker CI/deploy installs and locks together, rather than leaving mixed ownership. + +### 2. Add local-only live-upstream configuration + +- Add a dedicated development Wrangler config for live Rsbuild mode, using the same `src/index.ts` entry point and compatibility settings, but **no static-assets directory, prepare-dist build, remote bindings, or R2 dependency**. Factor shared config only if needed to prevent drift; do not duplicate Worker application logic. +- Select `ENVIRONMENT=development` explicitly and pass a local upstream binding only in this mode. Do not change the existing default built-site staging preview into live development mode. +- Preserve Wrangler's native source watching and automatic Worker reload for both the entry point and imported modules (including `src/csp.ts`). Editing Worker code must take effect on subsequent requests without restarting `npm run web`, rebuilding the app, or restarting Rsbuild/the API proxy. This is a separate requirement from React Fast Refresh; it does not require automatically refreshing an already-loaded browser document to replace its CSP headers. +- Regenerate/check Worker types for both configurations. Model the development upstream and production `ASSETS` binding without weakening typing to `any` or accidentally making deployment bindings optional everywhere. +- Select live forwarding only when the explicit development environment and validated loopback upstream are both present. Staging/production must never proxy to an arbitrary configured origin, even if an upstream variable is accidentally supplied. + +### 3. Reuse the Worker response pipeline + +- Separate response acquisition (Rsbuild fetch versus `ASSETS.fetch`) from the shared security-header/nonce transformation. +- Preserve request method, body, query, relevant headers, status, and streaming. Avoid double body consumption. Handle redirects deliberately so internal loopback URLs are not exposed to the browser. +- Pass WebSocket upgrades through without reconstructing the 101 response as an ordinary `Response` or feeding it into `HTMLRewriter`. +- Route API/attachment requests through Rsbuild's existing proxy table before any SPA/missing-static-asset logic; JSON API errors, receipt downloads, and non-GET requests must never become the app shell. Preserve existing backend response and caching semantics for these routes. +- For application assets/HTML, keep shared security headers and per-response nonce replacement. In development, disable immutable caching for changing bundles/hot-update files; maintain the HTML no-validator rule. Handle conditional upstream responses so a 304 cannot pair old HTML with a new CSP nonce. +- Preserve SPA navigation versus missing-asset distinctions, reserved-character deep links, AASA aliases/content type, HEAD behavior, and normal upstream errors. Return a useful local error when Rsbuild is unavailable, not a stale `dist/` fallback. +- Keep static-assets and previous-build R2 behavior intact for built-site previews and deploy environments. Document that live mode tests the common Worker pipeline, not the deployed asset router/R2 itself; the built-output test pass covers those separately. + +### 4. Add the smallest development-only CSP delta + +- Extend the existing CSP builder rather than copying staging/production policies. Keep their serialized directives unchanged apart from the existing per-request nonce. +- Add only explicit local origins required by observed requests: the chosen HMR WSS origin and, where needed, local API/backend origins such as `http://localhost:9000`. Normally the browser should use same-origin API routes through the Worker, not connect directly to port 9000. +- Verify browser mixed-content rules separately: putting an origin in CSP does not make every insecure request legal. +- Inspect actual development HTML for Rsbuild-injected inline scripts/React Refresh bootstrap. Supply nonces or narrowly scoped hashes when required; do not solve startup failures by adding blanket `script-src 'unsafe-inline'`, `connect-src *`, or disabling enforcement. Preserve the production template's existing hashes/nonce contract. +- Exercise internal `.env` local-backend overrides as well as contributor defaults. If configurable origins are needed, validate them and scope them to development rather than hard-coding one engineer's hostname or permitting all domains. + +### 5. Wire startup, browser opening, and opt-outs + +- Add a small typed launcher/config helper, following existing scripts/tooling conventions, to resolve environment precedence, the coordinated ports, readiness, and the browser origin once. +- Have `npm run web` retain Pusher setup and run the API proxy, Rsbuild, and Worker under `concurrently`. Suppress Rsbuild's `--open` in Worker mode and open exactly one browser tab at the Worker's root after the app is ready. Provide a no-open path for automation. +- Add `USE_CLOUDFLARE_WORKER=false` as the independent escape hatch: run/open Rsbuild directly over its existing trusted HTTPS setup. Keep `USE_WEB_PROXY=false` solely for disabling the API proxy. +- Load `.env` consistently with shell overrides taking precedence. Confirm the launcher, Rsbuild, API proxy, and bundled `CONFIG.IS_USING_WEB_PROXY` agree. Do not assume shell variables automatically override the serialized `__REACT_WEB_CONFIG__` object. +- On Ctrl-C or failure of a required long-running child, stop siblings and release ports. An intentionally disabled API proxy must not be treated as a crash that kills the other services. Surface missing certificates and occupied API/inspector ports clearly. + +### 6. Document the supported workflows + +Update root `README.md`, `contributingGuides/SETUP_WEB.md`, and `web/cloudflare-worker/README.md` with: + +- One normal install/start path, public versus internal origin, existing mkcert/hosts prerequisites, and no ngrok/Cloudflare credentials requirement. +- Independent Worker and API-proxy flags, shell/`.env` precedence, readiness/port-conflict behavior, and how to test with a local backend. +- The difference between live Worker development and the retained `dist/` staging/production CSP preview commands. +- HMR/nonce/cache exceptions confined to development; troubleshooting for certificate trust, stale service workers, upstream failure, and optional ngrok fallback only if needed. +- Replace the overly broad claim that local sign-in is impossible: it applies to a built-site preview without an API proxy, not necessarily live development with the existing API proxy. Do not promise third-party OAuth works on arbitrary local origins. + +## Validation plan (required before calling implementation complete) + +### Automated checks + +Extend existing `node:test`/workerd integration tests with a real ephemeral loopback upstream fixture, rather than mocking only `fetch`. Retain all existing static-assets tests. Document new tests with Given/When/Then comments. + +| Area | Required assertions | +| --- | --- | +| Live transport | Path/query/method/body preservation, streaming, redirects, HEAD, upstream failures, WebSocket round trip and query/token preservation | +| Headers/nonces | Header set present; nonce matches allowed script and differs between loads; no placeholder remains; HTML cannot return a mismatched 304 | +| Routing | Root/deep links/reserved characters work; missing JS is 404 rather than HTML; API errors/downloads bypass SPA fallback; AASA aliases remain correct | +| Development cache | Edited bundles/hot updates are not immutable or stale; HTML validators removed | +| Worker reload | Under the real launcher, entry-point and imported CSP-module edits trigger Wrangler reload; subsequent requests reflect changes without restarting the command or other services; React HMR still works afterward | +| Isolation | Local origins allowed only in development; staging/production unchanged and cannot activate upstream forwarding; existing R2/static-asset tests pass | +| Launcher | Default/opt-out combinations, `.env` versus shell precedence, public port injection, coordinated port conflict, readiness, child failure and teardown | +| Installation | Fresh root/standalone install brings in Wrangler; existing independent Worker install/CI remains valid | + +Run the Worker package's `npm test` and `npm run typecheck`; run focused Bun tooling tests for launcher/config helpers, `npm run lint-changed`, `npm run spell-changed`, and root `npm run typecheck` for changed TypeScript configuration/scripts. Run shellcheck if installation shell scripts change. React Compiler checks are needed only if React components/hooks change, which is not expected. + +Expand `.github/workflows/cloudflareWorkerTests.yml` path filters if shared launcher/config/CSP inputs outside the Worker package now affect the tests. Keep its standalone install working. Run `wrangler deploy --dry-run` for `test-staging`, `staging`, and `production` against a real/fixture build to verify deploy configurations exclude development upstream/CSP settings; do not deploy. + +### Real local end-to-end pass, including Playwright MCP + +1. **Prepare an isolated environment.** Use this dedicated worktree and repo-pinned Node/npm/Bun versions. Ensure mkcert files and the hosts entry are present without copying private configuration into Git. Use a disposable browser profile because the Worker sends HSTS. Record chosen ports and confirm existing servers/worktrees are not being reused. No `dist/`, Worker `.dev.vars`, Cloudflare credentials, or ngrok should be required for this pass. +2. **Start the real command.** Run `npm run web` in the worktree with automation browser-opening disabled. Capture startup logs; verify all expected services run and the frontend readiness check passes at the Worker origin. Check HTTPS with certificate verification enabled. Confirm an ordinary interactive launch opens only the Worker URL. +3. **Attach Playwright MCP to the Worker origin**, not the internal Rsbuild port. Capture console messages, page errors, failed requests, and `securitypolicyviolation` events from before navigation using the browser tooling. Use observable UI/network readiness, not arbitrary sleeps. MCP is a required test tool, not a replacement for automated regression tests. +4. **Prove App startup.** Navigate to `/`, wait for the splash to disappear and the usable sign-in form to appear, verify scripts/CSS/fonts/images load, and save a screenshot plus console/network evidence. A 200 response or a rendered splash alone is not a pass. Compare CSP headers with script `.nonce` properties and check fresh navigations use fresh nonces. +5. **Prove the app runs beyond startup.** Use an approved disposable account/backend. For a local development backend, follow its test-account/code rules; do not assume the local `000000` code works against production. Complete sign-in/onboarding, navigate to settings and a report, open an existing attachment if available, refresh a deep link, and verify authenticated API calls and realtime connectivity. Prefer a local backend fixture for mutations; never create test financial data on production. Record any unavailable backend/account prerequisites as blockers rather than claiming end-to-end success. +6. **Prove both independent reload loops work.** First observe the successful WSS connection, make a temporary reversible React UI edit in this worktree, and confirm Fast Refresh applies it without a full-page reload and without losing entered text or the session. Check hot-update JS/JSON responses and CSP events, then restore only this test edit. Next, with the same `npm run web` session still running, make a harmless temporary Worker entry-point edit (such as a diagnostic response header), observe Wrangler reload, and verify subsequent requests expose the change. Separately edit the imported `src/csp.ts` module with a harmless development-only policy change, and verify a fresh Playwright navigation receives the updated enforced CSP. Do not restart the command, rebuild the app, or restart Rsbuild/the API proxy. Verify the authenticated session remains usable and React HMR works again after the Worker reload (allowing automatic WebSocket reconnection). Restore only these test edits and verify Wrangler reloads the original behavior. Record reload logs and before/after headers; an already-loaded page need not automatically acquire new document headers. +7. **Prove enforcement, not just absence of complaints.** In the disposable browser, attempt a harmless inline script without a nonce with a unique marker and a connection to an unapproved test origin. Verify CSP blocks them and produces the expected violation events. Allowed legitimate code/HMR must still work; expected negative-test violations must be distinguished from unexpected app violations. +8. **Check routing/cache/failure behavior.** Use browser network inspection and HTTP assertions for `/index.html`, a real deep link, a missing JS asset, AASA, and conditional HTML requests. Restart Rsbuild to check useful failure/recovery, then test fresh load and warm reload. Check for stale service-worker interference; live dev need not acquire a production service worker, but built-output preview must retain its service-worker behavior. +9. **Run the opt-out matrix below**, restarting cleanly for each case. Verify the chosen origin and running processes, not merely command exit status. Exercise both shell flags and `.env` configuration. Include an internal-backend configuration with the API proxy disabled. +10. **Validate built output separately.** Build staging and production output and use the retained Worker preview commands over HTTPS. Repeat startup/header/nonce/deep-link/cache/service-worker checks and run existing header-comparison tooling where appropriate. This validates real static-assets behavior absent from live Rsbuild mode. Built-preview sign-in/OAuth CORS limitations are distinct from live-mode regressions. +11. **Teardown and evidence.** Stop only processes started for this test, verify ports are released, remove temporary UI edits, and retain reproducible commands, test results, screenshots, and sanitized console/network logs. Do not retain auth tokens, test-account credentials, or certificate keys in committed artifacts. Report any pre-existing third-party network errors separately, with no unexplained new app/CSP failures accepted. + +| Worker | API proxy | Expected result | +| --- | --- | --- | +| Default on | Default on | Worker HTTPS origin, real app load/sign-in, API forwarding, React Fast Refresh, and automatic Worker source/module reload | +| Off | On | Existing direct HTTPS Rsbuild experience; no Worker process | +| On | Off | Worker remains active; configured local backend works; no port-9000 proxy | +| Off | Off | Direct Rsbuild plus configured backend; neither proxy process | + +## Acceptance and residual risks + +- A fresh normal setup plus `npm run web` loads and operates the app through the real local Worker, with functional HMR and no mandatory ngrok, `dist/`, extra install, or Cloudflare authentication. +- Both reload loops are required: React edits apply through Fast Refresh, and Worker entry-point/imported-module edits apply through Wrangler's native reload on subsequent requests without manually restarting the development stack. Worker reload must not leave React HMR disconnected or the app unusable. +- Local security enforcement is verified positively and negatively in Playwright MCP, including authenticated interaction; HTTP-only fixtures or sign-in-page screenshots are insufficient evidence. +- Static-assets/deploy behavior and staging/production CSP are unchanged; development allowances cannot leak into deploy configurations. +- Opt-outs are independent and documented; process lifecycle, ports, certificate trust, and environment precedence are covered by tests. +- The transport spike is an explicit prerequisite: custom-port loopback fetch and WebSocket behavior have not yet been demonstrated in this planning task. If the preferred path fails, document the precise pinned-version failure and narrow fallback before expanding scope. +- Potential gaps that local workerd cannot fully reproduce include Cloudflare edge routing/cache behavior and provider-controlled OAuth origin restrictions. Do not weaken CSP or claim production parity for those merely to obtain a green local smoke test. + +Official implementation references: [Wrangler dev options](https://developers.cloudflare.com/workers/wrangler/commands/workers/#dev), [Rsbuild HMR client configuration](https://rsbuild.dev/config/dev/client), and [Workers local development](https://developers.cloudflare.com/workers/local-development/). Verify flags against the pinned CLI's `--help` during implementation. From 780e8d3f60cd97078ed79f47da5e2017569d9405 Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 20 Oct 2026 18:56:32 -0700 Subject: [PATCH 02/31] feat(web): run local sessions through the Worker Catch CSP and security-header regressions before staging while preserving React Fast Refresh and Wrangler source reload. Use trusted local HTTPS and loopback HTTP instead of requiring ngrok. --- .github/workflows/cloudflareWorkerTests.yml | 6 + README.md | 6 + config/rsbuild/rsbuild.common.ts | 25 +- config/rsbuild/rsbuild.config.ts | 25 +- contributingGuides/SETUP_WEB.md | 21 ++ package.json | 2 +- scripts/postInstall.sh | 4 + scripts/startWeb.ts | 106 ++++++++ scripts/utils/checkWebServer.ts | 19 ++ scripts/utils/webDevConfig.ts | 52 ++++ tests/tooling/checkWebServer.test.ts | 74 +++++ tests/tooling/webDevConfig.test.ts | 60 ++++ web/cloudflare-worker/.gitignore | 1 + web/cloudflare-worker/README.md | 18 +- web/cloudflare-worker/package-lock.json | 14 +- web/cloudflare-worker/package.json | 6 +- web/cloudflare-worker/src/csp.ts | 17 +- .../src/fetchDevelopmentResponse.ts | 59 ++++ web/cloudflare-worker/src/index.ts | 27 +- .../tests/createWorkerHarness.ts | 4 +- .../tests/development.test.ts | 257 ++++++++++++++++++ web/cloudflare-worker/tests/worker.test.ts | 3 +- web/cloudflare-worker/tsconfig.json | 2 +- web/cloudflare-worker/wrangler.dev.jsonc | 14 + 24 files changed, 790 insertions(+), 32 deletions(-) create mode 100644 scripts/startWeb.ts create mode 100644 scripts/utils/checkWebServer.ts create mode 100644 scripts/utils/webDevConfig.ts create mode 100644 tests/tooling/checkWebServer.test.ts create mode 100644 tests/tooling/webDevConfig.test.ts create mode 100644 web/cloudflare-worker/src/fetchDevelopmentResponse.ts create mode 100644 web/cloudflare-worker/tests/development.test.ts create mode 100644 web/cloudflare-worker/wrangler.dev.jsonc diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index aff7d9d745ed..7f650947f6b5 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -6,6 +6,12 @@ on: branches-ignore: [staging, production] paths: - 'web/cloudflare-worker/**' + - 'scripts/startWeb.ts' + - 'scripts/utils/webDevConfig.ts' + - 'scripts/utils/checkWebServer.ts' + - 'scripts/postInstall.sh' + - 'config/rsbuild/**' + - 'package.json' # The tests check its inline scripts against the CSP hashes. - 'web/index.html' - '.nvmrc' diff --git a/README.md b/README.md index 5e737b99952d..a61b8b618da6 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,8 @@ These instructions should get you set up ready to work on New Expensify 🙌 3. Install dependencies: `npm install` 4. Run the specific platform with the following command: `npm run `, e.g. `npm run web` +For web, normal setup also installs the Cloudflare Worker tooling. After the [HTTPS/hosts setup](contributingGuides/SETUP_WEB.md), `npm run web` serves the app through the local Worker at `https://dev.new.expensify.com:8082/`, with CSP, security headers, React Fast Refresh, and automatic Worker source reload. No ngrok, Cloudflare account, or web build is required. + You can use any IDE or code editing tool for developing on any platform. Use your favorite! ## Recommended `node` setup @@ -78,6 +80,10 @@ variables referenced here get updated since your local `.env` file is ignored. - `USE_WEB_PROXY`⚠️- Used in web development, it starts a server along the local development server to proxy requests to the backend. External contributors should set this to `true` otherwise they'll have CORS errors. If you don't want to start the proxy server set this explicitly to `false` +- `USE_CLOUDFLARE_WORKER` - Defaults to `true` for `npm run web`. Set to `false` to serve Rsbuild directly over HTTPS. This does not disable the API proxy. +- `WEB_PORT` - Browser-facing development port (default `8082`). If occupied, choose a different port; the launcher will not reuse another session's server. +- `WEB_INTERNAL_PORT` - Internal loopback HTTP Rsbuild port when the Worker is enabled (default `WEB_PORT + 1`). +- `WEB_OPEN` - Set to `false` to suppress automatic browser opening, for example in browser automation. - `CAPTURE_METRICS` (optional) - Set this to `true` to capture performance metrics and see them in Flipper see [PERFORMANCE.md](contributingGuides/PERFORMANCE.md#performance-metrics-opt-in-on-local-release-builds) for more information - `ONYX_METRICS` (optional) - Set this to `true` to capture even more performance metrics and see them in Flipper diff --git a/config/rsbuild/rsbuild.common.ts b/config/rsbuild/rsbuild.common.ts index caf4082088e8..71711eda1118 100644 --- a/config/rsbuild/rsbuild.common.ts +++ b/config/rsbuild/rsbuild.common.ts @@ -133,6 +133,26 @@ function mapEnvironmentToLogoSuffix(environmentFile: string): string { */ function getDefineValues(file: string): DefinePluginOptions { const isDevelopmentFile = file === '.env' || file === '.env.development'; + const parsedConfig = dotenv.config({path: file}).parsed; + // Forward only public development settings, never arbitrary shell variables (which may contain secrets). + const developmentOverrides = isDevelopmentFile + ? Object.fromEntries( + [ + 'USE_WEB_PROXY', + 'USE_NGROK', + 'EXPENSIFY_URL', + 'SECURE_EXPENSIFY_URL', + 'NEW_EXPENSIFY_URL', + 'STAGING_EXPENSIFY_URL', + 'STAGING_SECURE_EXPENSIFY_URL', + 'NGROK_URL', + 'SECURE_NGROK_URL', + 'SKIP_ONBOARDING', + 'PUSHER_APP_KEY', + 'PUSHER_DEV_SUFFIX', + ].flatMap((key) => (process.env[key] === undefined ? [] : [[key, process.env[key]]])), + ) + : {}; /* eslint-disable @typescript-eslint/naming-convention */ return { process: {env: {}}, @@ -142,7 +162,10 @@ function getDefineValues(file: string): DefinePluginOptions { global: 'globalThis', // Define EXPO_OS for web platform to fix expo-modules-core warning 'process.env.EXPO_OS': JSON.stringify('web'), - __REACT_WEB_CONFIG__: JSON.stringify(dotenv.config({path: file}).parsed), + __REACT_WEB_CONFIG__: JSON.stringify({ + ...parsedConfig, + ...developmentOverrides, + }), // React Native JavaScript environment requires the global __DEV__ variable to be accessible. // react-native-render-html uses variable to log exclusively during development. diff --git a/config/rsbuild/rsbuild.config.ts b/config/rsbuild/rsbuild.config.ts index d5140d759494..0c4c389184e6 100644 --- a/config/rsbuild/rsbuild.config.ts +++ b/config/rsbuild/rsbuild.config.ts @@ -48,9 +48,11 @@ export default defineConfig(async ({command}) => { }; } - const port = await portfinder.getPortPromise({port: BASE_PORT}); + const useWorker = process.env.USE_CLOUDFLARE_WORKER === 'true'; + const publicPort = process.env.WEB_PORT ? Number(process.env.WEB_PORT) : await portfinder.getPortPromise({port: BASE_PORT}); + const port = useWorker ? Number(process.env.WEB_INTERNAL_PORT) : publicPort; // Check if the USE_WEB_PROXY variable has been provided and rewrite any requests to the local proxy server - const proxy = process.env.USE_WEB_PROXY === 'false' ? undefined : [{pathFilter: ['/api', '/staging', '/chat-attachments', '/receipts'], target: 'http://[::1]:9000'}]; + const proxy = process.env.USE_WEB_PROXY === 'false' ? undefined : [{pathFilter: ['/api', '/staging', '/staging-secure', '/chat-attachments', '/receipts'], target: 'http://[::1]:9000'}]; const commonRspackTool = common.tools?.rspack; @@ -58,8 +60,10 @@ export default defineConfig(async ({command}) => { ...common, dev: { client: {overlay: false}, + assetPrefix: '/', lazyCompilation: false, }, + security: useWorker ? {nonce: 'nonce-random-value'} : common.security, source: { ...common.source, define: { @@ -67,20 +71,23 @@ export default defineConfig(async ({command}) => { // The dev server's resolved port can differ from BASE_PORT if 8082 is already in use. // CONFIG.DEV_PORT (src/CONFIG.ts) reads this at runtime to build correct dev environment URLs. // eslint-disable-next-line @typescript-eslint/naming-convention - 'process.env.PORT': port, + 'process.env.PORT': publicPort, }, }, server: { proxy, port, - host: 'dev.new.expensify.com', + strictPort: !!process.env.WEB_PORT, + host: useWorker ? '127.0.0.1' : 'dev.new.expensify.com', // Rsbuild's --open flag defaults to the first entry's route, e.g. "/main" for our "main" entry key, // which 404s since the app has no route by that name. Open the actual app root instead. - open: '/', - https: { - key: fs.readFileSync(path.join(dirname, 'key.pem')), - cert: fs.readFileSync(path.join(dirname, 'certificate.pem')), - }, + open: process.env.WEB_PORT ? false : '/', + https: useWorker + ? undefined + : { + key: fs.readFileSync(path.join(dirname, 'key.pem')), + cert: fs.readFileSync(path.join(dirname, 'certificate.pem')), + }, headers: { // HTTP header names aren't valid camelCase identifiers. // eslint-disable-next-line @typescript-eslint/naming-convention diff --git a/contributingGuides/SETUP_WEB.md b/contributingGuides/SETUP_WEB.md index 5383fb997109..cfd1c23fcf86 100644 --- a/contributingGuides/SETUP_WEB.md +++ b/contributingGuides/SETUP_WEB.md @@ -30,6 +30,24 @@ If you already generated certificates before the Webpack → Rsbuild migration, - To run the **development web app**: `npm run web` - Changes applied to Javascript will be applied automatically via Rsbuild as configured in `config/rsbuild/rsbuild.config.ts` +`npm run web` starts the API proxy, Rsbuild, and the Cloudflare Worker in local workerd through Wrangler. It opens `https://dev.new.expensify.com:8082/` after the Worker serves the app. The Worker uses your existing mkcert certificate and forwards to a private loopback HTTP Rsbuild listener; you do not need ngrok, a Cloudflare account, or a `dist/` build. + +React edits use Fast Refresh through the Worker's WSS connection. Worker entry-point and imported module edits (including CSP changes) use Wrangler's native reload without restarting the command or the other services. A new document request/reload is needed to receive changed CSP headers; existing pages do not automatically acquire new response headers. + +Optional shell or `.env` settings (shell values take precedence): + +```bash +USE_CLOUDFLARE_WORKER=false npm run web # direct HTTPS Rsbuild, API proxy still enabled +USE_WEB_PROXY=false npm run web # Worker still enabled; use your configured local backend +WEB_PORT=8102 WEB_OPEN=false npm run web # isolated session suitable for Playwright +``` + +`WEB_PORT` defaults to `8082`; `WEB_INTERNAL_PORT` defaults to the next port. Occupied ports fail startup rather than attaching to another worktree or silently changing the upstream. The API proxy retains port `9000`, so only one proxy-enabled session can own it. Stop the process that owns a needed port, or disable the API proxy and configure your backend. Ctrl-C or a required service exiting stops the remaining services. + +Set `EXPENSIFY_URL` and `SECURE_EXPENSIFY_URL` for your local backend and disable `USE_WEB_PROXY` for direct backend requests. Explicit backend origins are added only to the development connection policy. Backend CORS rules and TLS trust still apply; CSP allowances do not bypass them. Third-party OAuth may require an approved origin. + +For exact built-asset caching, service-worker, and staging/production CSP checks, use the separate [Worker build preview](../web/cloudflare-worker/README.md#running-a-web-build-locally). Live Rsbuild mode uses development caching and does not exercise Cloudflare's asset store or previous-build R2 fallback. + ### Production Build To build and run the production web build locally: @@ -143,5 +161,8 @@ You probably have a misconfigured `.env` file - remove it (`rm .env`) and try ag 1. If you are having issues with **_Getting Started_**, please reference [React Native's Documentation](https://reactnative.dev/docs/environment-setup) 2. Make sure you have the correct Node version installed as specified in `.nvmrc` 3. Ensure HTTPS is properly configured if you're having certificate issues +4. If Worker setup is missing, rerun `npm install`; launching the server never installs dependencies. Missing certificates require `npm run setup-https`. Trust the mkcert CA in the browser rather than disabling certificate checks. +5. If the Worker returns `503`, inspect the Rsbuild process output. There is no fallback to an old `dist/` build. If a page appears stale, check for an old service worker in the disposable browser profile. +6. The old ngrok workaround for Wrangler upstream ports is not required: live mode explicitly fetches a loopback HTTP origin. Do not clear CA settings or disable TLS verification to work around unrelated certificate errors. **Note:** Expensify engineers that will be testing with the API in your local dev environment please refer to [these additional instructions](https://stackoverflow.com/c/expensify/questions/7699/7700). diff --git a/package.json b/package.json index 2023e9cdc908..b50521f1a58f 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "ipad-sm": "concurrently \"./scripts/run-build.sh --ipad-sm\"", "ipad-sm-standalone": "concurrently \"STANDALONE_NEW_DOT=true ./scripts/run-build.sh --ipad-sm\"", "start": "npx rock start", - "web": "./scripts/set-pusher-suffix.sh && concurrently npm:web-proxy npm:web-server", + "web": "./scripts/set-pusher-suffix.sh && bun scripts/startWeb.ts", "web-proxy": "bun web/proxy.ts", "web-server": "node ./node_modules/.bin/rsbuild dev --open --config config/rsbuild/rsbuild.config.ts", "build": "ENV=production bun ./node_modules/.bin/rsbuild build --config config/rsbuild/rsbuild.config.ts && bun ./scripts/combine-web-sourcemaps.ts", diff --git a/scripts/postInstall.sh b/scripts/postInstall.sh index bd95fa016e40..d3337eac04b4 100755 --- a/scripts/postInstall.sh +++ b/scripts/postInstall.sh @@ -39,3 +39,7 @@ bun scripts/applyPatches.ts # files, and project mode does not follow `references`. `npx tsc` at the repo root therefore checks # nothing and exits 0. Use `npx tsc --build`, which walks the references, or `npm run typecheck`. ln -sfn ../typescript/bin/tsc "$ROOT_DIR/node_modules/.bin/tsc" + +# Keep the Worker independently installable for deploy CI, but include it in normal App setup. +echo "Installing local Cloudflare Worker tooling" +npm ci --prefix "$ROOT_DIR/web/cloudflare-worker" --include=dev diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts new file mode 100644 index 000000000000..60c4c671e907 --- /dev/null +++ b/scripts/startWeb.ts @@ -0,0 +1,106 @@ +/** Start the live Worker and Rsbuild together, keeping browser-facing URLs independent of the upstream. */ +import concurrently from 'concurrently'; +import dotenv from 'dotenv'; +import {execFileSync, spawn} from 'node:child_process'; +import {existsSync, readFileSync} from 'node:fs'; +import path from 'node:path'; +import {setTimeout as delay} from 'node:timers/promises'; + +import checkWebServer from './utils/checkWebServer'; +import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; + +async function startWeb() { + dotenv.config(); + const {useWorker, useProxy, openBrowser} = getWebDevFlags(process.env); + const {publicPort, internalPort} = await getWebDevPorts(process.env, useWorker); + if (useProxy) { + await assertWebPortAvailable(9000, '::'); + } + for (const file of ['key.pem', 'certificate.pem']) { + if (!existsSync(`config/rsbuild/${file}`)) { + throw new Error('Missing local HTTPS certificate. Run npm run setup-https first.'); + } + } + if (useWorker && !existsSync('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js')) { + throw new Error('Missing Worker dependencies. Run npm install to finish App setup.'); + } + const origin = `https://dev.new.expensify.com:${publicPort}`; + process.env.WEB_PORT = String(publicPort); + process.env.WEB_INTERNAL_PORT = String(internalPort); + process.env.USE_CLOUDFLARE_WORKER = String(useWorker); + process.env.USE_WEB_PROXY = String(useProxy); + const workerDir = path.resolve('web/cloudflare-worker'); + const ca = readFileSync(path.join(execFileSync('mkcert', ['-CAROOT'], {encoding: 'utf8'}).trim(), 'rootCA.pem')); + // Values include local paths and JSON; neither should be expanded by the child shell. + const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; + const commands = [ + ...(useProxy ? [{name: 'api', command: 'npm run web-proxy'}] : []), + {name: 'rsbuild', command: 'node node_modules/@rsbuild/core/bin/rsbuild.js dev --config config/rsbuild/rsbuild.config.ts'}, + ...(useWorker + ? [ + { + name: 'worker', + cwd: workerDir, + command: [ + 'node node_modules/wrangler/bin/wrangler.js dev --local --config wrangler.dev.jsonc', + `--ip 127.0.0.1 --port ${publicPort} --inspector-port 0 --local-protocol https`, + `--https-key-path ${quote(path.resolve('config/rsbuild/key.pem'))}`, + `--https-cert-path ${quote(path.resolve('config/rsbuild/certificate.pem'))}`, + `--var ${quote(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, + `--var ${quote(`DEV_CONNECT_ORIGINS:${JSON.stringify(getDevelopmentConnectOrigins(process.env))}`)}`, + ].join(' '), + }, + ] + : []), + ]; + const controller = new AbortController(); + const {commands: children, result} = concurrently(commands, {killOthersOn: ['success', 'failure'], killTimeout: 3000}); + const stop = () => { + controller.abort(); + for (const child of children) { + child.kill('SIGTERM'); + } + }; + process.once('SIGINT', stop); + process.once('SIGTERM', stop); + // Verify the browser-facing listener and certificate, not just the internal compiler's listener. + const readiness = (async () => { + const deadline = Date.now() + 180_000; + while (!controller.signal.aborted) { + try { + const ready = await checkWebServer(origin, ca, useWorker, controller.signal); + if (ready) { + console.log(`Web app ready at ${origin}/ (${useWorker ? 'Cloudflare Worker' : 'direct Rsbuild'})`); + if (openBrowser && !controller.signal.aborted) { + const browserCommands: Partial> = {darwin: 'open', win32: 'cmd'}; + const command = browserCommands[process.platform] ?? 'xdg-open'; + const args = process.platform === 'win32' ? ['/c', 'start', '', `${origin}/`] : [`${origin}/`]; + const browser = spawn(command, args, {stdio: 'ignore'}); + browser.on('error', (error) => console.error(`Open ${origin}/ in your browser: ${error.message}`)); + } + return; + } + } catch { + // The dev server can be between starting its listener and completing its first compilation. + } + if (Date.now() > deadline) { + throw new Error(`Timed out waiting for ${origin}. Check the dev server output.`); + } + await delay(250, undefined, {signal: controller.signal}); + } + })(); + try { + await Promise.race([result, readiness]); + await result; + } finally { + stop(); + await readiness.catch(() => {}); + process.removeListener('SIGINT', stop); + process.removeListener('SIGTERM', stop); + } +} + +startWeb().catch((error: unknown) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/scripts/utils/checkWebServer.ts b/scripts/utils/checkWebServer.ts new file mode 100644 index 000000000000..b29589bfb1b0 --- /dev/null +++ b/scripts/utils/checkWebServer.ts @@ -0,0 +1,19 @@ +/** Probe the public HTTPS listener without disabling certificate verification. */ +import https from 'node:https'; + +function checkWebServer(origin: string, ca: Buffer, useWorker: boolean, signal: AbortSignal): Promise { + return new Promise((resolve) => { + const request = https.get(origin, {ca, timeout: 2000, signal}, (response) => { + response.resume(); + resolve(response.statusCode === 200 && (!useWorker || !!response.headers['content-security-policy'])); + }); + request.on('timeout', () => { + // Bun's ClientRequest.destroy() need not emit an error after a timeout. + resolve(false); + request.destroy(); + }); + request.on('error', () => resolve(false)); + }); +} + +export default checkWebServer; diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts new file mode 100644 index 000000000000..2ca0f402147d --- /dev/null +++ b/scripts/utils/webDevConfig.ts @@ -0,0 +1,52 @@ +/** Resolve one set of ports and flags for every process in a local web session. */ +import net from 'node:net'; + +const LOCAL_BACKEND_KEYS = ['EXPENSIFY_URL', 'SECURE_EXPENSIFY_URL', 'STAGING_EXPENSIFY_URL', 'STAGING_SECURE_EXPENSIFY_URL', 'NGROK_URL', 'SECURE_NGROK_URL']; + +function getWebDevFlags(env: NodeJS.ProcessEnv) { + return {useWorker: env.USE_CLOUDFLARE_WORKER !== 'false', useProxy: env.USE_WEB_PROXY !== 'false', openBrowser: env.WEB_OPEN !== 'false'}; +} + +function getDevelopmentConnectOrigins(env: NodeJS.ProcessEnv): string[] { + return [ + ...new Set( + LOCAL_BACKEND_KEYS.flatMap((key) => { + const value = env[key]; + if (!value) { + return []; + } + const url = new URL(value); + if (!['http:', 'https:'].includes(url.protocol) || url.username || url.password) { + throw new Error(`${key} must be an HTTP(S) URL without credentials`); + } + return [url.origin]; + }), + ), + ]; +} + +/** Fail on conflicts rather than letting a child silently attach to another worktree's server. */ +async function assertWebPortAvailable(port: number, host = '127.0.0.1'): Promise { + const server = net.createServer(); + await new Promise((resolve, reject) => { + server.once('error', () => reject(new Error(`Port ${port} on ${host} is in use. Stop its owner or change this session's port/proxy settings.`))); + server.listen(port, host, () => server.close(() => resolve())); + }); +} + +async function getWebDevPorts(env: NodeJS.ProcessEnv, useWorker: boolean) { + const publicPort = Number(env.WEB_PORT ?? 8082); + const internalPort = Number(env.WEB_INTERNAL_PORT ?? publicPort + 1); + if (useWorker && publicPort === internalPort) { + throw new Error('WEB_PORT and WEB_INTERNAL_PORT must differ'); + } + for (const port of useWorker ? [publicPort, internalPort] : [publicPort]) { + if (!Number.isInteger(port) || port < 1024 || port > 65535) { + throw new Error('WEB_PORT and WEB_INTERNAL_PORT must be integers between 1024 and 65535'); + } + await assertWebPortAvailable(port); + } + return {publicPort, internalPort}; +} + +export {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts}; diff --git a/tests/tooling/checkWebServer.test.ts b/tests/tooling/checkWebServer.test.ts new file mode 100644 index 000000000000..fc04251a3ede --- /dev/null +++ b/tests/tooling/checkWebServer.test.ts @@ -0,0 +1,74 @@ +import {afterEach, describe, expect, mock, spyOn, test} from 'bun:test'; + +/** A timed-out probe must not prevent the launcher from retrying or shutting down. */ +import type {ClientRequest} from 'node:http'; + +import {EventEmitter} from 'node:events'; +import {IncomingMessage} from 'node:http'; +import https from 'node:https'; +import {Socket} from 'node:net'; + +import checkWebServer from '../../scripts/utils/checkWebServer'; + +describe('public web server readiness', () => { + afterEach(() => mock.restore()); + + test('settles a timeout even when destroy emits no error', async () => { + // Given Bun can destroy a timed-out request without emitting an error + const request = Object.assign(new EventEmitter(), {destroy: mock(() => {})}); + // This fixture implements only the request events the readiness probe uses, without a real TLS connection. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const get = spyOn(https, 'get').mockReturnValue(request as unknown as ClientRequest); + const ca = Buffer.from('test CA'); + const signal = new AbortController().signal; + const result = checkWebServer('https://dev.new.expensify.com:8082', ca, true, signal); + // When the first compilation takes longer than the probe timeout + request.emit('timeout'); + // Then the launcher can retry and does not disable certificate checks + await expect(result).resolves.toBe(false); + expect(request.destroy).toHaveBeenCalledTimes(1); + expect(get).toHaveBeenCalledWith('https://dev.new.expensify.com:8082', {ca, timeout: 2000, signal}, expect.any(Function)); + }); + + test('settles connection errors so startup can retry or cancel', async () => { + // Given a request created before the Worker starts listening + const request = Object.assign(new EventEmitter(), {destroy: mock(() => {})}); + // This fixture deliberately contains only the event API used by the probe. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + spyOn(https, 'get').mockReturnValue(request as unknown as ClientRequest); + const result = checkWebServer('https://dev.new.expensify.com:8082', Buffer.from('CA'), false, new AbortController().signal); + // When the connection fails + request.emit('error', new Error('Connection refused')); + // Then it is a failed readiness probe, not an unhandled rejection or a pending promise + await expect(result).resolves.toBe(false); + }); + + test.each([ + {status: 200, csp: true, worker: true, ready: true}, + {status: 200, csp: false, worker: true, ready: false}, + {status: 503, csp: true, worker: true, ready: false}, + {status: 200, csp: false, worker: false, ready: true}, + ])('requires a usable response from the selected public listener: %j', async ({status, csp, worker, ready}) => { + // Given either the direct server or the Worker listener + const request = Object.assign(new EventEmitter(), {destroy: mock(() => {})}); + // Only the client request event API is needed by this fixture. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const get = spyOn(https, 'get').mockReturnValue(request as unknown as ClientRequest); + const result = checkWebServer('https://dev.new.expensify.com:8082', Buffer.from('CA'), worker, new AbortController().signal); + const onResponse = get.mock.calls.at(0)?.[2]; + if (!onResponse) { + throw new Error('Expected an HTTPS response callback'); + } + const response = new IncomingMessage(new Socket()); + response.statusCode = status; + response.headers = {}; + if (csp) { + response.headers['content-security-policy'] = "default-src 'self'"; + } + // When the listener returns its response + onResponse(response); + response.push(null); + // Then only a successful response with the expected Worker headers can open the browser + await expect(result).resolves.toBe(ready); + }); +}); diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts new file mode 100644 index 000000000000..2955f91a1e0d --- /dev/null +++ b/tests/tooling/webDevConfig.test.ts @@ -0,0 +1,60 @@ +/** Keep local Worker/API opt-outs and coordinated port selection independent. */ +import {describe, expect, test} from 'bun:test'; + +import net from 'node:net'; + +import {getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from '../../scripts/utils/webDevConfig'; + +describe('web development configuration', () => { + test('defaults both services on and allows independent opt-outs', () => { + // Given contributor defaults + // When flags are resolved + const defaults = getWebDevFlags({}); + // Then neither opt-out changes the meaning of the other + expect(defaults).toEqual({useWorker: true, useProxy: true, openBrowser: true}); + expect(getWebDevFlags({USE_WEB_PROXY: 'false'})).toMatchObject({useWorker: true, useProxy: false}); + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', WEB_OPEN: 'false'})).toEqual({useWorker: false, useProxy: true, openBrowser: false}); + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'})).toMatchObject({useWorker: false, useProxy: false}); + }); + + test('validates and deduplicates explicit backend origins', () => { + // Given backend settings with paths + // When CSP origins are resolved + const origins = getDevelopmentConnectOrigins({EXPENSIFY_URL: 'https://api.example.test/path', SECURE_EXPENSIFY_URL: 'https://api.example.test/secure'}); + // Then only exact origins survive and credentials/non-HTTP schemes cannot enter the policy + expect(origins).toEqual(['https://api.example.test']); + expect(() => getDevelopmentConnectOrigins({EXPENSIFY_URL: 'file:///etc/hosts'})).toThrow(); + expect(() => getDevelopmentConnectOrigins({EXPENSIFY_URL: 'https://user:pass@example.test'})).toThrow(); + }); + + test('rejects invalid and overlapping ports before starting children', async () => { + // Given invalid coordinated ports + // When the launcher checks them + // Then startup fails instead of changing the upstream port behind the Worker's back + await expect(getWebDevPorts({WEB_PORT: 'invalid'}, true)).rejects.toThrow(); + await expect(getWebDevPorts({WEB_PORT: '1023'}, true)).rejects.toThrow(); + await expect(getWebDevPorts({WEB_PORT: '65536'}, true)).rejects.toThrow(); + await expect(getWebDevPorts({WEB_PORT: '11000', WEB_INTERNAL_PORT: '11000'}, true)).rejects.toThrow('must differ'); + }); + + test('fails clearly rather than reusing an occupied worktree origin', async () => { + // Given another server owns a port + const server = net.createServer(); + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve); + }); + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Missing test port'); + } + try { + // When this session tries to use it + // Then no child is started against that unrelated listener + await expect(getWebDevPorts({WEB_PORT: String(address.port)}, false)).rejects.toThrow('in use'); + } finally { + await new Promise((resolve) => { + server.close(() => resolve()); + }); + } + }); +}); diff --git a/web/cloudflare-worker/.gitignore b/web/cloudflare-worker/.gitignore index b2f02dedeefe..8ede4ac745c9 100644 --- a/web/cloudflare-worker/.gitignore +++ b/web/cloudflare-worker/.gitignore @@ -1,3 +1,4 @@ .wrangler/ .dev.vars* worker-configuration.d.ts +worker-development.d.ts diff --git a/web/cloudflare-worker/README.md b/web/cloudflare-worker/README.md index c2cd4be84b00..66978ae8a74e 100644 --- a/web/cloudflare-worker/README.md +++ b/web/cloudflare-worker/README.md @@ -22,13 +22,23 @@ Every request runs the Worker first (`run_worker_first: true`), which reads the The CSP lives in [`src/csp.ts`](src/csp.ts). The `ENVIRONMENT` variable in [`wrangler.jsonc`](wrangler.jsonc) selects the staging or production policy. -## Running it locally +## Live development + +From the App root, run `npm install`, complete the [web HTTPS/hosts setup](../../contributingGuides/SETUP_WEB.md), then run `npm run web`. The root install includes this package's independently locked dependencies. + +The browser opens the Worker at `https://dev.new.expensify.com:8082/`. `wrangler.dev.jsonc` runs the same entry point against live Rsbuild output over loopback HTTP, with no `dist/`, R2, Cloudflare credentials, or ngrok dependency. React Fast Refresh uses WSS through the Worker; Wrangler automatically reloads Worker code and imported modules on edits. Reload the document to test changed CSP headers. + +Only the explicit `development` environment can forward to `DEV_UPSTREAM`, which must be an HTTP `127.0.0.1` origin with a port. The launcher supplies that port, and development CSP adds the public HMR origin, existing local backend endpoints, and explicit configured API origins. Live assets are not cached immutably. Staging/production still use static assets, their original CSP, and normal caching/R2 behavior. + +`USE_CLOUDFLARE_WORKER=false npm run web` bypasses the Worker but leaves the API proxy enabled. `USE_WEB_PROXY=false` disables only the API proxy. `WEB_PORT`, `WEB_INTERNAL_PORT`, and `WEB_OPEN=false` control isolated sessions and browser automation. Shell values take precedence over `.env`; occupied ports fail clearly. See the web setup guide for local backend settings and troubleshooting. + +## Running a web build locally `wrangler dev` runs the Worker in workerd, the same runtime Cloudflare uses, and serves `dist/` exactly as a deploy would. No Cloudflare account or credentials are needed. ### 1. Install -This package has its own dependencies. They are not installed by the root `npm install`. +Root `npm install` installs these dependencies too. For an independent Worker checkout or CI job, install just this package: ```bash cd web/cloudflare-worker @@ -72,7 +82,7 @@ Over plain HTTP, a few third-party scripts that load protocol-relative URLs (Con ### What you can't test locally -The API at `www.expensify.com`, and Google Sign-In, only accept requests from known origins. The app loads to the sign-in page, but signing in fails with CORS errors. Everything the Worker is responsible for can be tested locally; signing in is verified on a deployed test hostname. +This built-site preview has no API proxy and uses API endpoints and third-party OAuth providers that accept only known origins, so sign-in can fail with CORS errors. This restriction is distinct from live `npm run web`, which retains the existing API proxy or a configured local backend. Do not infer that a successful built-site startup validates authentication, or that an API proxy makes third-party OAuth work on arbitrary origins. ## Checking the behavior @@ -83,6 +93,8 @@ npm test # integration tests: the real wrangler.jsonc in workerd, again npm run typecheck # regenerates worker-configuration.d.ts with `wrangler types`, then type-checks src/ and tests/ ``` +The live-mode tests also launch the real `wrangler dev` CLI against an ephemeral HTTP/WebSocket upstream, exercising the same transport used by `npm run web`. Worker source watching is verified separately with the real app and browser: edit the entry point and `src/csp.ts`, verify subsequent responses change without restarting the command, then verify React Fast Refresh still works. + ### Comparing headers with a live host ```bash diff --git a/web/cloudflare-worker/package-lock.json b/web/cloudflare-worker/package-lock.json index fb1610bf4aca..61ff7b42ad00 100644 --- a/web/cloudflare-worker/package-lock.json +++ b/web/cloudflare-worker/package-lock.json @@ -9,8 +9,10 @@ "version": "0.0.0", "devDependencies": { "@types/node": "24.19.0", + "@types/ws": "8.18.1", "typescript": "7.0.2", - "wrangler": "4.144.0" + "wrangler": "4.144.0", + "ws": "8.21.0" } }, "node_modules/@cloudflare/kv-asset-handler": { @@ -1252,6 +1254,16 @@ "undici-types": ">=7.24.0 <7.24.7" } }, + "node_modules/@types/ws": { + "version": "8.18.1", + "resolved": "https://registry.npmjs.org/@types/ws/-/ws-8.18.1.tgz", + "integrity": "sha512-ThVF6DCVhA8kUGy+aazFQ4kXQ7E1Ty7A3ypFOe0IcJV8O/M511G99AW24irKrW56Wt44yG9+ij8FaqoBGkuBXg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/node": "*" + } + }, "node_modules/@typescript/typescript-aix-ppc64": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/@typescript/typescript-aix-ppc64/-/typescript-aix-ppc64-7.0.2.tgz", diff --git a/web/cloudflare-worker/package.json b/web/cloudflare-worker/package.json index 9fc3c6cece9b..6095b637b835 100644 --- a/web/cloudflare-worker/package.json +++ b/web/cloudflare-worker/package.json @@ -8,14 +8,16 @@ "prepare-dist": "./scripts/prepareDist.sh", "dev": "wrangler dev", "dev:production": "wrangler dev --env production", - "types": "wrangler types", + "types": "wrangler types && wrangler types worker-development.d.ts --config wrangler.dev.jsonc --env-interface DevelopmentEnv --include-runtime false", "typecheck": "npm run types > /dev/null && tsc -p . && tsc -p tests", "test": "node --test 'tests/**/*.test.ts'", "compare-headers": "node scripts/compareHeaders.ts" }, "devDependencies": { "@types/node": "24.19.0", + "@types/ws": "8.18.1", "typescript": "7.0.2", - "wrangler": "4.144.0" + "wrangler": "4.144.0", + "ws": "8.21.0" } } diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index 32866baf0d55..3238f3957527 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -5,6 +5,7 @@ type CSPOptions = { fontSrcHosts?: string[]; frameSrcHosts?: string[]; scriptSrcHashes?: string[]; + connectSrcHosts?: string[]; }; /** Inline scripts allowed without a nonce. Tests check that every such script in `web/index.html` is listed. */ @@ -17,7 +18,7 @@ const INLINE_SCRIPT_HASHES = [ 'sha256-ZC4Ihfl+1sv3E25DQh090ITQKwffxiocyA9C1vaePKU=', ]; -function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = []}: CSPOptions): string { +function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = [], connectSrcHosts = []}: CSPOptions): string { const csp: Record = { 'default-src': ["'self'", 'https://assets.onfido.com'], 'script-src': [ @@ -53,6 +54,7 @@ function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = []}: ], 'connect-src': [ "'self'", + ...connectSrcHosts, 'blob: *.onfido.com', 'data:', 'https://*.analytics.google.com', @@ -167,8 +169,17 @@ const PRODUCTION_CSP = buildCSP({ scriptSrcHashes: INLINE_SCRIPT_HASHES, }); -function getCSP(environment: Env['ENVIRONMENT'], nonce: string): string { - const csp = environment === 'staging' ? STAGING_CSP : PRODUCTION_CSP; +function getCSP(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, connectSrcHosts: string[] = []): string { + const csp = + environment === 'development' + ? buildCSP({ + fontSrcHosts: ["'self'"], + scriptSrcHashes: INLINE_SCRIPT_HASHES, + connectSrcHosts: ['https://www.expensify.com.dev', 'https://secure.expensify.com.dev', 'http://localhost:9000', ...connectSrcHosts], + }) + : environment === 'staging' + ? STAGING_CSP + : PRODUCTION_CSP; return csp.replace(NONCE_PLACEHOLDER, `nonce-${nonce}`); } diff --git a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts new file mode 100644 index 000000000000..603fe35c11c6 --- /dev/null +++ b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts @@ -0,0 +1,59 @@ +/** Forward live requests without relying on Wrangler's implicit upstream host/port handling. */ +function getDevelopmentUpstream(env: DevelopmentEnv): URL { + const upstream = new URL(env.DEV_UPSTREAM); + if ( + upstream.protocol !== 'http:' || + upstream.hostname !== '127.0.0.1' || + !upstream.port || + upstream.pathname !== '/' || + upstream.search || + upstream.hash || + upstream.username || + upstream.password + ) { + throw new Error('DEV_UPSTREAM must be an HTTP loopback origin with an explicit port'); + } + return upstream; +} + +/** API and attachment responses must never enter the SPA fallback or HTML nonce/cache pipeline. */ +function isAPIRequest(pathname: string): boolean { + return /^\/(?:api|staging|staging-secure|chat-attachments|receipts)(?:\/|$)/.test(pathname); +} + +async function fetchDevelopmentResponse(request: Request, env: DevelopmentEnv): Promise { + const upstream = getDevelopmentUpstream(env); + const publicURL = new URL(request.url); + const url = new URL(request.url); + url.protocol = upstream.protocol; + url.host = upstream.host; + if (url.pathname === '/apple-app-site-association') { + url.pathname = '/.well-known/apple-app-site-association'; + } + const forwarded = new Request(url, request); + if (!isAPIRequest(url.pathname)) { + // Never let an upstream 304 pair a previously cached HTML body with a fresh CSP nonce. + forwarded.headers.delete('If-None-Match'); + forwarded.headers.delete('If-Modified-Since'); + } + let response: Response; + try { + response = await fetch(forwarded, {redirect: 'manual'}); + } catch { + return new Response('The local Rsbuild server is unavailable. Check its output in npm run web.', {status: 503}); + } + if (response.status === 101) { + return response; + } + const location = response.headers.get('Location'); + if (location && new URL(location, url).origin === upstream.origin) { + const redirect = new URL(location, url); + redirect.protocol = publicURL.protocol; + redirect.host = publicURL.host; + response = new Response(response.body, response); + response.headers.set('Location', redirect.href); + } + return response; +} + +export {fetchDevelopmentResponse, getDevelopmentUpstream, isAPIRequest}; diff --git a/web/cloudflare-worker/src/index.ts b/web/cloudflare-worker/src/index.ts index 605990253d84..1a390f89c930 100644 --- a/web/cloudflare-worker/src/index.ts +++ b/web/cloudflare-worker/src/index.ts @@ -1,4 +1,5 @@ import {getCSP, NONCE_PLACEHOLDER} from './csp'; +import {fetchDevelopmentResponse, isAPIRequest} from './fetchDevelopmentResponse'; /** Content-hashed build output (`main-<16 hex>.bundle.js`, `main.<10 hex>.css`, ...), emitted at the root of `dist/`. */ const HASHED_ASSET_PATH = /^\/[^/]+(?:-[0-9a-f]{16}\.bundle\.js|\.[0-9a-f]{10}\.[a-z0-9]+)$/; @@ -42,9 +43,9 @@ function buildAssetRequest(request: Request, url: URL): Request { return new Request(assetURL, request); } -function setSecurityHeaders(headers: Headers, environment: Env['ENVIRONMENT'], nonce: string) { +function setSecurityHeaders(headers: Headers, environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, connectSrcHosts: string[] = []) { headers.set('Strict-Transport-Security', 'max-age=31536000; includeSubDomains; preload'); - headers.set('Content-Security-Policy', getCSP(environment, nonce)); + headers.set('Content-Security-Policy', getCSP(environment, nonce, connectSrcHosts)); headers.set('X-Frame-Options', 'SAMEORIGIN'); headers.set('X-Content-Type-Options', 'nosniff'); headers.set('Referrer-Policy', 'origin-when-cross-origin'); @@ -83,31 +84,41 @@ async function getPreviousBuildFile(env: Env, url: URL, method: string): Promise export default { async fetch(request, env): Promise { const url = new URL(request.url); + const isDevelopment = env.ENVIRONMENT === 'development'; const nonce = generateNonce(); const isNavigation = request.headers.get('Sec-Fetch-Mode') === 'navigate'; const isAASAPath = url.pathname === AASA_PATH || url.pathname === LEGACY_AASA_PATH; - let assetResponse = await env.ASSETS.fetch(buildAssetRequest(request, url)); + let assetResponse = isDevelopment ? await fetchDevelopmentResponse(request, env) : await env.ASSETS.fetch(buildAssetRequest(request, url)); + + if (assetResponse.status === 101 || (isDevelopment && isAPIRequest(url.pathname))) { + return assetResponse; + } + const connectSrcHosts = isDevelopment ? [url.origin.replace(/^https:/, 'wss:').replace(/^http:/, 'ws:'), ...(JSON.parse(env.DEV_CONNECT_ORIGINS) as string[])] : []; // The SPA fallback answers every miss with index.html. A browser asking for a missing chunk needs the file from // an earlier build, or a real 404. if (isHTMLResponse(assetResponse) && (isAASAPath || (!isNavigation && STATIC_FILE_PATH.test(url.pathname)))) { - const previousBuildFile = isAASAPath ? null : await getPreviousBuildFile(env, url, request.method); + const previousBuildFile = isAASAPath || isDevelopment ? null : await getPreviousBuildFile(env, url, request.method); if (!previousBuildFile) { const notFound = new Response('Not Found', {status: 404, headers: {'Content-Type': 'text/plain; charset=utf-8'}}); - setSecurityHeaders(notFound.headers, env.ENVIRONMENT, nonce); + setSecurityHeaders(notFound.headers, env.ENVIRONMENT, nonce, connectSrcHosts); return notFound; } assetResponse = previousBuildFile; } const response = new Response(assetResponse.body, assetResponse); - setSecurityHeaders(response.headers, env.ENVIRONMENT, nonce); + setSecurityHeaders(response.headers, env.ENVIRONMENT, nonce, connectSrcHosts); if (isAASAPath) { response.headers.set('Content-Type', 'application/json'); } - if (HASHED_ASSET_PATH.test(url.pathname)) { + if (isDevelopment) { + response.headers.set('Cache-Control', 'no-store'); + response.headers.delete('ETag'); + response.headers.delete('Last-Modified'); + } else if (HASHED_ASSET_PATH.test(url.pathname)) { response.headers.set('Cache-Control', 'public, max-age=31536000, immutable'); } @@ -127,4 +138,4 @@ export default { }) .transform(response); }, -} satisfies ExportedHandler; +} satisfies ExportedHandler; diff --git a/web/cloudflare-worker/tests/createWorkerHarness.ts b/web/cloudflare-worker/tests/createWorkerHarness.ts index 41e3fd4fb4b2..b33f311468a7 100644 --- a/web/cloudflare-worker/tests/createWorkerHarness.ts +++ b/web/cloudflare-worker/tests/createWorkerHarness.ts @@ -65,7 +65,7 @@ function writeFakeWebBuild(assetsDir: string, omittedFiles: string[]) { * override the assets directory, so this writes a temporary config that changes `main` and `assets.directory`, and * copies .assetsignore itself in place of the `build` command. */ -function createWorkerHarness(env?: 'test-staging' | 'staging' | 'production', {omittedFiles = []}: {omittedFiles?: string[]} = {}): WorkerHarness { +function createWorkerHarness(env?: 'test-staging' | 'staging' | 'production', {omittedFiles = [], vars = {}}: {omittedFiles?: string[]; vars?: Record} = {}): WorkerHarness { const tempDir = mkdtempSync(path.join(tmpdir(), 'new-expensify-worker-')); const assetsDir = path.join(tempDir, 'dist'); writeFakeWebBuild(assetsDir, omittedFiles); @@ -82,7 +82,7 @@ function createWorkerHarness(env?: 'test-staging' | 'staging' | 'production', {o }), ); - const server = createTestHarness({workers: [{configPath, env}]}); + const server = createTestHarness({workers: [{configPath, env, vars}]}); const workerName = (env ? rawConfig.env?.[env]?.name : undefined) ?? rawConfig.name ?? ''; return { server, diff --git a/web/cloudflare-worker/tests/development.test.ts b/web/cloudflare-worker/tests/development.test.ts new file mode 100644 index 000000000000..13ed820bf88d --- /dev/null +++ b/web/cloudflare-worker/tests/development.test.ts @@ -0,0 +1,257 @@ +/** Exercise the live forwarding path in real workerd against a loopback HTTP/WebSocket server. */ +import assert from 'node:assert/strict'; +import {spawn} from 'node:child_process'; +import {once} from 'node:events'; +import {cpSync, mkdtempSync, readFileSync, rmSync, writeFileSync} from 'node:fs'; +import http from 'node:http'; +import {tmpdir} from 'node:os'; +import path from 'node:path'; +import {after, before, describe, test} from 'node:test'; +import {setTimeout as delay} from 'node:timers/promises'; +import {experimental_readRawConfig} from 'wrangler'; +import {WebSocket, WebSocketServer} from 'ws'; + +describe('live development worker', () => { + const tempDir = mkdtempSync(path.join(tmpdir(), 'live-worker-')); + const upstream = http.createServer(async (request, response) => { + if (request.url?.startsWith('/echo')) { + const chunks = []; + for await (const chunk of request) { + chunks.push(chunk); + } + response.setHeader('Content-Type', 'application/json'); + response.end(JSON.stringify({method: request.method, url: request.url, body: Buffer.concat(chunks).toString(), validator: request.headers['if-none-match'] ?? null})); + } else if (request.url === '/redirect') { + const address = upstream.address(); + assert.ok(address && typeof address !== 'string'); + response.writeHead(302, {Location: `http://127.0.0.1:${address.port}/target?next=1`}); + response.end(); + } else if (request.url === '/stream.txt') { + response.setHeader('Content-Type', 'text/plain'); + response.write('first '); + setImmediate(() => response.end('second')); + } else if (request.url?.startsWith('/api/') || request.url?.startsWith('/receipts/')) { + response.writeHead(403, {'Content-Type': 'text/html', 'Cache-Control': 'private'}); + response.end('backend error, not the app shell'); + } else if (request.url === '/.well-known/apple-app-site-association') { + response.end('{"applinks":{}}'); + } else if (request.url === '/main-0123456789abcdef.bundle.js') { + response.writeHead(200, {'Content-Type': 'application/javascript', ETag: 'bundle'}); + response.end('console.log("dev bundle");'); + } else { + response.writeHead(200, {'Content-Type': 'text/html', ETag: 'html', 'Last-Modified': new Date().toUTCString()}); + response.end('
app shell
'); + } + }); + const sockets = new WebSocketServer({server: upstream}); + sockets.on('connection', (socket, request) => { + socket.on('message', (message) => socket.send(`${request.url}:${message.toString()}`)); + }); + let origin: string; + let worker: ReturnType; + const server = {fetch: (url: string, init?: RequestInit) => fetch(`${origin}${url}`, init)}; + before(async () => { + upstream.listen(0, '127.0.0.1'); + await once(upstream, 'listening'); + const address = upstream.address(); + assert.ok(address && typeof address !== 'string'); + const portReservation = http.createServer(); + portReservation.listen(0, '127.0.0.1'); + await once(portReservation, 'listening'); + const reservedAddress = portReservation.address(); + assert.ok(reservedAddress && typeof reservedAddress !== 'string'); + const port = reservedAddress.port; + await new Promise((resolve) => portReservation.close(() => resolve())); + origin = `http://127.0.0.1:${port}`; + cpSync(path.resolve(import.meta.dirname, '../src'), path.join(tempDir, 'src'), {recursive: true}); + const {rawConfig} = experimental_readRawConfig({config: path.resolve(import.meta.dirname, '../wrangler.dev.jsonc')}); + const {rawConfig: deployedConfig} = experimental_readRawConfig({config: path.resolve(import.meta.dirname, '../wrangler.jsonc')}); + assert.equal(rawConfig.compatibility_date, deployedConfig.compatibility_date, 'Live and deployed Workers must use the same runtime compatibility date'); + const configPath = path.join(tempDir, 'wrangler.json'); + writeFileSync(configPath, JSON.stringify({...rawConfig, main: './src/index.ts'})); + worker = spawn( + process.execPath, + [ + 'node_modules/wrangler/bin/wrangler.js', + 'dev', + '--local', + '--config', + configPath, + '--ip', + '127.0.0.1', + '--port', + String(port), + '--inspector-port', + '0', + '--var', + `DEV_UPSTREAM:http://127.0.0.1:${address.port}`, + '--var', + 'DEV_CONNECT_ORIGINS:["https://www.expensify.com.dev"]', + ], + {cwd: path.resolve(import.meta.dirname, '..'), env: {...process.env, WRANGLER_SEND_METRICS: 'false'}}, + ); + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error('Wrangler did not start')), 20000); + worker.stdout?.on('data', (data) => { + if (String(data).includes('Ready on')) { + clearTimeout(timeout); + resolve(); + } + }); + worker.stderr?.on('data', (data) => process.stderr.write(data)); + worker.once('exit', (code) => { + clearTimeout(timeout); + reject(new Error(`Wrangler exited before readiness: ${code}`)); + }); + }); + }); + after(async () => { + if (worker && worker.exitCode === null && worker.signalCode === null) { + worker.kill('SIGTERM'); + await once(worker, 'exit'); + } + for (const socket of sockets.clients) { + socket.terminate(); + } + await new Promise((resolve) => sockets.close(() => resolve())); + await new Promise((resolve) => upstream.close(() => resolve())); + rmSync(tempDir, {recursive: true, force: true}); + }); + + test('forwards POST bodies, queries, and HEAD without consuming the response twice', async () => { + // Given a real HTTP upstream + // When a POST with a query passes through the Worker + const response = await server.fetch('/echo?q=reserved%40value', {method: 'POST', body: 'posted payload'}); + // Then the request reaches the upstream unchanged and HEAD has no body + assert.deepEqual(await response.json(), {method: 'POST', url: '/echo?q=reserved%40value', body: 'posted payload', validator: null}); + assert.equal(await (await server.fetch('/echo', {method: 'HEAD'})).text(), ''); + }); + + test('transforms HTML and removes validators, including on conditional requests', async () => { + // Given cached HTML + // When a conditional navigation reaches the live Worker + const response = await server.fetch('/settings/profile', {headers: {'If-None-Match': 'html'}}); + const html = await response.text(); + const nonce = /nonce="([^"]+)"/.exec(html)?.[1]; + // Then the returned body and CSP agree, no validator survives, and a new load has a new nonce + assert.equal(response.status, 200); + assert.ok(nonce && nonce !== 'nonce-random-value'); + assert.ok(response.headers.get('Content-Security-Policy')?.includes(`'nonce-${nonce}'`)); + assert.equal(response.headers.get('ETag'), null); + assert.equal(response.headers.get('Last-Modified'), null); + assert.equal(response.headers.get('X-Content-Type-Options'), 'nosniff'); + assert.doesNotMatch(await (await server.fetch('/')).text(), new RegExp(nonce)); + assert.equal(((await (await server.fetch('/echo', {headers: {'If-None-Match': 'html'}})).json()) as {validator: string | null}).validator, null); + }); + + test('allows only explicit development connections alongside the normal CSP', async () => { + // Given explicit local backend settings + // When the Worker serves a page + const csp = (await server.fetch('/')).headers.get('Content-Security-Policy') ?? ''; + // Then HMR and that backend are allowed without weakening script enforcement + assert.ok(csp.includes(origin.replace('http:', 'ws:'))); + assert.ok(csp.includes('https://www.expensify.com.dev')); + assert.doesNotMatch(csp, /script-src[^;]*'unsafe-inline'|connect-src \*/); + }); + + test('does not apply immutable caching to live bundles or mistake missing files for the shell', async () => { + // Given a live bundle and an unknown static asset + // When both are fetched + const bundle = await server.fetch('/main-0123456789abcdef.bundle.js'); + const missing = await server.fetch('/missing.js'); + // Then edits cannot be hidden behind immutable caching and missing JS is not HTML + assert.equal(bundle.headers.get('Cache-Control'), 'no-store'); + assert.equal(missing.status, 404); + assert.match(missing.headers.get('Content-Type') ?? '', /text\/plain/); + }); + + test('preserves backend errors/downloads without treating HTML errors as SPA responses', async () => { + // Given API and receipt paths + // When the backend returns its own HTML error + for (const url of ['/api/Test', '/receipts/test.pdf']) { + const response = await server.fetch(url); + // Then status, body and private caching survive without Worker HTML rewriting + assert.equal(response.status, 403); + assert.equal(await response.text(), 'backend error, not the app shell'); + assert.equal(response.headers.get('Cache-Control'), 'private'); + } + }); + + test('streams responses, rewrites internal redirects and preserves AASA aliases', async () => { + // Given live streaming, a redirect and the legacy AASA path + // When they pass through the Worker + const stream = await server.fetch('/stream.txt'); + const redirect = await fetch(`${origin}/redirect`, {redirect: 'manual'}); + const aasa = await server.fetch('/apple-app-site-association'); + // Then the full stream arrives, redirects stay public, and AASA is JSON + assert.equal(await stream.text(), 'first second'); + assert.equal(redirect.headers.get('Location'), `${origin}/target?next=1`, `status=${redirect.status} body=${await redirect.text()}`); + assert.equal(aasa.headers.get('Content-Type'), 'application/json'); + assert.deepEqual(await aasa.json(), {applinks: {}}); + }); + + test('passes WebSocket upgrades, messages and HMR token queries through', {timeout: 10000}, async () => { + // Given a real WebSocket client connecting through the Worker + // When it sends a message + const socket = new WebSocket(`${origin.replace('http:', 'ws:')}/rsbuild-hmr?token=test`); + await once(socket, 'open'); + socket.send('hot update'); + const [message] = await once(socket, 'message'); + // Then the real upstream sees the HMR query and its reply arrives through the upgraded connection + assert.equal(message.toString(), '/rsbuild-hmr?token=test:hot update'); + socket.close(); + await once(socket, 'close'); + }); + + test('automatically reloads both the Worker entry point and imported CSP modules', {timeout: 15000}, async () => { + // Given the CLI is watching an isolated copy of the actual Worker source + const entryPath = path.join(tempDir, 'src/index.ts'); + const cspPath = path.join(tempDir, 'src/csp.ts'); + const entry = readFileSync(entryPath, 'utf8'); + const csp = readFileSync(cspPath, 'utf8'); + const waitForResponse = async (matches: (response: Response) => boolean) => { + const deadline = Date.now() + 5000; + while (Date.now() < deadline) { + const response = await server.fetch('/'); + await response.arrayBuffer(); + if (matches(response)) { + return; + } + await delay(50); + } + assert.fail('Wrangler did not reload the edited source'); + }; + try { + // When the entry point and an imported module change, without restarting Wrangler + writeFileSync( + entryPath, + entry.replace( + 'const response = new Response(assetResponse.body, assetResponse);', + 'const response = new Response(assetResponse.body, assetResponse); response.headers.set("X-Reload-Proof", "entry");', + ), + ); + await waitForResponse((response) => response.headers.get('X-Reload-Proof') === 'entry'); + writeFileSync(cspPath, csp.replace("'http://localhost:9000'", "'http://localhost:9000', 'https://module-reload.invalid'")); + // Then both changes are served on subsequent requests, including the new CSP + await waitForResponse((response) => response.headers.get('Content-Security-Policy')?.includes('https://module-reload.invalid') ?? false); + } finally { + writeFileSync(entryPath, entry); + writeFileSync(cspPath, csp); + } + await waitForResponse((response) => response.headers.get('X-Reload-Proof') === null && !response.headers.get('Content-Security-Policy')?.includes('https://module-reload.invalid')); + }); + + test('reports an unavailable upstream without serving stale built output', async () => { + // Given the live upstream has stopped + for (const socket of sockets.clients) { + socket.terminate(); + } + upstream.closeAllConnections(); + await new Promise((resolve) => upstream.close(() => resolve())); + // When a new page is requested + const response = await server.fetch('/'); + // Then the response explains the local failure rather than falling back to dist + assert.equal(response.status, 503); + assert.match(await response.text(), /Rsbuild server is unavailable/); + }); +}); diff --git a/web/cloudflare-worker/tests/worker.test.ts b/web/cloudflare-worker/tests/worker.test.ts index 09997a2c11b4..9d0b51f25b13 100644 --- a/web/cloudflare-worker/tests/worker.test.ts +++ b/web/cloudflare-worker/tests/worker.test.ts @@ -335,7 +335,7 @@ describe('test-staging worker', () => { }); describe('production worker', () => { - const {server, cleanup} = createWorkerHarness('production'); + const {server, cleanup} = createWorkerHarness('production', {vars: {DEV_UPSTREAM: 'http://127.0.0.1:1', DEV_CONNECT_ORIGINS: '["https://dev-only.invalid"]'}}); before(async () => { await server.listen(); @@ -354,6 +354,7 @@ describe('production worker', () => { assert.match(csp, /new-expensify:\/\/new\.expensify\.com/); assert.match(csp, /font-src data: 'self' https:\/\/www\.expensify\.com/); assert.doesNotMatch(csp, /new-expensify:\/\/staging/); + assert.doesNotMatch(csp, /localhost|127\.0\.0\.1|expensify\.com\.dev|dev-only\.invalid/); assertTemplateInlineScriptsAllowed(csp); }); diff --git a/web/cloudflare-worker/tsconfig.json b/web/cloudflare-worker/tsconfig.json index 8e3b138af7e9..c0e7a049bfa1 100644 --- a/web/cloudflare-worker/tsconfig.json +++ b/web/cloudflare-worker/tsconfig.json @@ -11,5 +11,5 @@ "isolatedModules": true, "verbatimModuleSyntax": true }, - "include": ["src", "worker-configuration.d.ts"] + "include": ["src", "worker-configuration.d.ts", "worker-development.d.ts"] } diff --git a/web/cloudflare-worker/wrangler.dev.jsonc b/web/cloudflare-worker/wrangler.dev.jsonc new file mode 100644 index 000000000000..845a0082377a --- /dev/null +++ b/web/cloudflare-worker/wrangler.dev.jsonc @@ -0,0 +1,14 @@ +{ + "$schema": "node_modules/wrangler/config-schema.json", + "name": "new-expensify-live-local", + "main": "src/index.ts", + "compatibility_date": "2026-09-26", + "workers_dev": false, + "preview_urls": false, + // Live development needs neither a dist/ directory nor Cloudflare bindings. + "vars": { + "ENVIRONMENT": "development", + "DEV_UPSTREAM": "http://127.0.0.1:8083", + "DEV_CONNECT_ORIGINS": "[]", + }, +} From 456facc0562ab28e4bcc6d003e6015e0672ea25e Mon Sep 17 00:00:00 2001 From: rory Date: Tue, 20 Oct 2026 22:56:39 -0700 Subject: [PATCH 03/31] refactor(web): use Bun-native launcher APIs --- scripts/startWeb.ts | 50 +++++++------ scripts/utils/checkWebServer.ts | 24 +++---- scripts/utils/webDevConfig.ts | 31 ++++---- tests/tooling/checkWebServer.test.ts | 104 ++++++++++++++------------- tests/tooling/webDevConfig.test.ts | 63 +++++++++++----- 5 files changed, 152 insertions(+), 120 deletions(-) diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 60c4c671e907..91ed89a4610e 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -1,38 +1,35 @@ /** Start the live Worker and Rsbuild together, keeping browser-facing URLs independent of the upstream. */ +import {$} from 'bun'; import concurrently from 'concurrently'; -import dotenv from 'dotenv'; -import {execFileSync, spawn} from 'node:child_process'; -import {existsSync, readFileSync} from 'node:fs'; -import path from 'node:path'; -import {setTimeout as delay} from 'node:timers/promises'; import checkWebServer from './utils/checkWebServer'; import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; async function startWeb() { - dotenv.config(); - const {useWorker, useProxy, openBrowser} = getWebDevFlags(process.env); - const {publicPort, internalPort} = await getWebDevPorts(process.env, useWorker); + // Bun loads .env automatically, preserving explicit shell overrides. + const {useWorker, useProxy, openBrowser} = getWebDevFlags(Bun.env); + const {publicPort, internalPort} = getWebDevPorts(Bun.env, useWorker); if (useProxy) { - await assertWebPortAvailable(9000, '::'); + assertWebPortAvailable(9000, '::'); } for (const file of ['key.pem', 'certificate.pem']) { - if (!existsSync(`config/rsbuild/${file}`)) { + if (!(await Bun.file(`config/rsbuild/${file}`).exists())) { throw new Error('Missing local HTTPS certificate. Run npm run setup-https first.'); } } - if (useWorker && !existsSync('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js')) { + if (useWorker && !(await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists())) { throw new Error('Missing Worker dependencies. Run npm install to finish App setup.'); } const origin = `https://dev.new.expensify.com:${publicPort}`; - process.env.WEB_PORT = String(publicPort); - process.env.WEB_INTERNAL_PORT = String(internalPort); - process.env.USE_CLOUDFLARE_WORKER = String(useWorker); - process.env.USE_WEB_PROXY = String(useProxy); - const workerDir = path.resolve('web/cloudflare-worker'); - const ca = readFileSync(path.join(execFileSync('mkcert', ['-CAROOT'], {encoding: 'utf8'}).trim(), 'rootCA.pem')); - // Values include local paths and JSON; neither should be expanded by the child shell. - const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; + Bun.env.WEB_PORT = String(publicPort); + Bun.env.WEB_INTERNAL_PORT = String(internalPort); + Bun.env.USE_CLOUDFLARE_WORKER = String(useWorker); + Bun.env.USE_WEB_PROXY = String(useProxy); + const root = process.cwd(); + const workerDir = `${root}/web/cloudflare-worker`; + const caRoot = (await $`mkcert -CAROOT`.text()).trim(); + const ca = await Bun.file(`${caRoot}/rootCA.pem`).text(); + // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. const commands = [ ...(useProxy ? [{name: 'api', command: 'npm run web-proxy'}] : []), {name: 'rsbuild', command: 'node node_modules/@rsbuild/core/bin/rsbuild.js dev --config config/rsbuild/rsbuild.config.ts'}, @@ -44,10 +41,10 @@ async function startWeb() { command: [ 'node node_modules/wrangler/bin/wrangler.js dev --local --config wrangler.dev.jsonc', `--ip 127.0.0.1 --port ${publicPort} --inspector-port 0 --local-protocol https`, - `--https-key-path ${quote(path.resolve('config/rsbuild/key.pem'))}`, - `--https-cert-path ${quote(path.resolve('config/rsbuild/certificate.pem'))}`, - `--var ${quote(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, - `--var ${quote(`DEV_CONNECT_ORIGINS:${JSON.stringify(getDevelopmentConnectOrigins(process.env))}`)}`, + `--https-key-path ${$.escape(`${root}/config/rsbuild/key.pem`)}`, + `--https-cert-path ${$.escape(`${root}/config/rsbuild/certificate.pem`)}`, + `--var ${$.escape(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, + `--var ${$.escape(`DEV_CONNECT_ORIGINS:${JSON.stringify(getDevelopmentConnectOrigins(Bun.env))}`)}`, ].join(' '), }, ] @@ -75,8 +72,9 @@ async function startWeb() { const browserCommands: Partial> = {darwin: 'open', win32: 'cmd'}; const command = browserCommands[process.platform] ?? 'xdg-open'; const args = process.platform === 'win32' ? ['/c', 'start', '', `${origin}/`] : [`${origin}/`]; - const browser = spawn(command, args, {stdio: 'ignore'}); - browser.on('error', (error) => console.error(`Open ${origin}/ in your browser: ${error.message}`)); + $`${command} ${args}`.quiet().catch((error: unknown) => { + console.error(`Open ${origin}/ in your browser:`, error); + }); } return; } @@ -86,7 +84,7 @@ async function startWeb() { if (Date.now() > deadline) { throw new Error(`Timed out waiting for ${origin}. Check the dev server output.`); } - await delay(250, undefined, {signal: controller.signal}); + await Bun.sleep(250); } })(); try { diff --git a/scripts/utils/checkWebServer.ts b/scripts/utils/checkWebServer.ts index b29589bfb1b0..6718d497b17b 100644 --- a/scripts/utils/checkWebServer.ts +++ b/scripts/utils/checkWebServer.ts @@ -1,19 +1,15 @@ /** Probe the public HTTPS listener without disabling certificate verification. */ -import https from 'node:https'; - -function checkWebServer(origin: string, ca: Buffer, useWorker: boolean, signal: AbortSignal): Promise { - return new Promise((resolve) => { - const request = https.get(origin, {ca, timeout: 2000, signal}, (response) => { - response.resume(); - resolve(response.statusCode === 200 && (!useWorker || !!response.headers['content-security-policy'])); - }); - request.on('timeout', () => { - // Bun's ClientRequest.destroy() need not emit an error after a timeout. - resolve(false); - request.destroy(); +async function checkWebServer(origin: string, ca: string, useWorker: boolean, signal: AbortSignal): Promise { + try { + const response = await fetch(origin, { + tls: {ca}, + signal: AbortSignal.any([signal, AbortSignal.timeout(2000)]), }); - request.on('error', () => resolve(false)); - }); + await response.body?.cancel(); + return response.status === 200 && (!useWorker || response.headers.has('Content-Security-Policy')); + } catch { + return false; + } } export default checkWebServer; diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index 2ca0f402147d..0936b8b3c0b8 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -1,13 +1,11 @@ /** Resolve one set of ports and flags for every process in a local web session. */ -import net from 'node:net'; - const LOCAL_BACKEND_KEYS = ['EXPENSIFY_URL', 'SECURE_EXPENSIFY_URL', 'STAGING_EXPENSIFY_URL', 'STAGING_SECURE_EXPENSIFY_URL', 'NGROK_URL', 'SECURE_NGROK_URL']; -function getWebDevFlags(env: NodeJS.ProcessEnv) { +function getWebDevFlags(env: Record) { return {useWorker: env.USE_CLOUDFLARE_WORKER !== 'false', useProxy: env.USE_WEB_PROXY !== 'false', openBrowser: env.WEB_OPEN !== 'false'}; } -function getDevelopmentConnectOrigins(env: NodeJS.ProcessEnv): string[] { +function getDevelopmentConnectOrigins(env: Record): string[] { return [ ...new Set( LOCAL_BACKEND_KEYS.flatMap((key) => { @@ -26,15 +24,24 @@ function getDevelopmentConnectOrigins(env: NodeJS.ProcessEnv): string[] { } /** Fail on conflicts rather than letting a child silently attach to another worktree's server. */ -async function assertWebPortAvailable(port: number, host = '127.0.0.1'): Promise { - const server = net.createServer(); - await new Promise((resolve, reject) => { - server.once('error', () => reject(new Error(`Port ${port} on ${host} is in use. Stop its owner or change this session's port/proxy settings.`))); - server.listen(port, host, () => server.close(() => resolve())); - }); +function assertWebPortAvailable(port: number, host = '127.0.0.1'): void { + try { + const listener = Bun.listen({ + hostname: host, + port, + socket: { + data: (socket) => { + socket.end(); + }, + }, + }); + listener.stop(true); + } catch { + throw new Error(`Port ${port} on ${host} is in use. Stop its owner or change this session's port/proxy settings.`); + } } -async function getWebDevPorts(env: NodeJS.ProcessEnv, useWorker: boolean) { +function getWebDevPorts(env: Record, useWorker: boolean) { const publicPort = Number(env.WEB_PORT ?? 8082); const internalPort = Number(env.WEB_INTERNAL_PORT ?? publicPort + 1); if (useWorker && publicPort === internalPort) { @@ -44,7 +51,7 @@ async function getWebDevPorts(env: NodeJS.ProcessEnv, useWorker: boolean) { if (!Number.isInteger(port) || port < 1024 || port > 65535) { throw new Error('WEB_PORT and WEB_INTERNAL_PORT must be integers between 1024 and 65535'); } - await assertWebPortAvailable(port); + assertWebPortAvailable(port); } return {publicPort, internalPort}; } diff --git a/tests/tooling/checkWebServer.test.ts b/tests/tooling/checkWebServer.test.ts index fc04251a3ede..5920ce675d28 100644 --- a/tests/tooling/checkWebServer.test.ts +++ b/tests/tooling/checkWebServer.test.ts @@ -1,46 +1,61 @@ +/** Readiness probes must time out or cancel without blocking the development launcher. */ import {afterEach, describe, expect, mock, spyOn, test} from 'bun:test'; -/** A timed-out probe must not prevent the launcher from retrying or shutting down. */ -import type {ClientRequest} from 'node:http'; - -import {EventEmitter} from 'node:events'; -import {IncomingMessage} from 'node:http'; -import https from 'node:https'; -import {Socket} from 'node:net'; - import checkWebServer from '../../scripts/utils/checkWebServer'; describe('public web server readiness', () => { afterEach(() => mock.restore()); - test('settles a timeout even when destroy emits no error', async () => { - // Given Bun can destroy a timed-out request without emitting an error - const request = Object.assign(new EventEmitter(), {destroy: mock(() => {})}); - // This fixture implements only the request events the readiness probe uses, without a real TLS connection. - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - const get = spyOn(https, 'get').mockReturnValue(request as unknown as ClientRequest); - const ca = Buffer.from('test CA'); - const signal = new AbortController().signal; - const result = checkWebServer('https://dev.new.expensify.com:8082', ca, true, signal); - // When the first compilation takes longer than the probe timeout - request.emit('timeout'); - // Then the launcher can retry and does not disable certificate checks + test.each([new DOMException('Probe timed out', 'TimeoutError'), new TypeError('Connection refused'), new Error('Certificate verification failed')])( + 'treats fetch failures as an unsuccessful probe: %s', + async (error) => { + // Given a listener that is starting up, unresponsive, or has an untrusted certificate + const get = spyOn(globalThis, 'fetch').mockRejectedValue(error); + const signal = new AbortController().signal; + // When the launcher probes the public origin + const ready = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', true, signal); + // Then it can retry without disabling TLS verification + expect(ready).toBe(false); + const [url, init] = get.mock.calls.at(0) ?? []; + expect(url).toBe('https://dev.new.expensify.com:8082'); + expect(init).toMatchObject({tls: {ca: 'test CA'}}); + expect(init?.signal).toBeInstanceOf(AbortSignal); + }, + ); + + test('cancels an outstanding probe when the launcher stops', async () => { + // Given a probe waiting for its response + const waitingFetch = Object.assign( + (_url: Parameters[0], init?: Parameters[1]) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason), {once: true}); + }), + {preconnect: fetch.preconnect}, + ); + spyOn(globalThis, 'fetch').mockImplementation(waitingFetch); + const controller = new AbortController(); + const result = checkWebServer('https://dev.new.expensify.com:8082', 'test CA', true, controller.signal); + // When the session is stopped + controller.abort(); + // Then readiness finishes without waiting for the timeout await expect(result).resolves.toBe(false); - expect(request.destroy).toHaveBeenCalledTimes(1); - expect(get).toHaveBeenCalledWith('https://dev.new.expensify.com:8082', {ca, timeout: 2000, signal}, expect.any(Function)); }); - test('settles connection errors so startup can retry or cancel', async () => { - // Given a request created before the Worker starts listening - const request = Object.assign(new EventEmitter(), {destroy: mock(() => {})}); - // This fixture deliberately contains only the event API used by the probe. - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - spyOn(https, 'get').mockReturnValue(request as unknown as ClientRequest); - const result = checkWebServer('https://dev.new.expensify.com:8082', Buffer.from('CA'), false, new AbortController().signal); - // When the connection fails - request.emit('error', new Error('Connection refused')); - // Then it is a failed readiness probe, not an unhandled rejection or a pending promise - await expect(result).resolves.toBe(false); + test('times out a real listener that does not return response headers', async () => { + // Given a real local server that cannot finish its first response yet + const server = Bun.serve({ + hostname: '127.0.0.1', + port: 0, + fetch: () => new Promise(() => {}), + }); + try { + // When the launcher probes it without an external cancellation + const result = checkWebServer(server.url.href, 'test CA', false, new AbortController().signal); + // Then the built-in request timeout releases the probe rather than hanging startup + await expect(result).resolves.toBe(false); + } finally { + await server.stop(true); + } }); test.each([ @@ -50,25 +65,16 @@ describe('public web server readiness', () => { {status: 200, csp: false, worker: false, ready: true}, ])('requires a usable response from the selected public listener: %j', async ({status, csp, worker, ready}) => { // Given either the direct server or the Worker listener - const request = Object.assign(new EventEmitter(), {destroy: mock(() => {})}); - // Only the client request event API is needed by this fixture. - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - const get = spyOn(https, 'get').mockReturnValue(request as unknown as ClientRequest); - const result = checkWebServer('https://dev.new.expensify.com:8082', Buffer.from('CA'), worker, new AbortController().signal); - const onResponse = get.mock.calls.at(0)?.[2]; - if (!onResponse) { - throw new Error('Expected an HTTPS response callback'); - } - const response = new IncomingMessage(new Socket()); - response.statusCode = status; - response.headers = {}; + const headers = new Headers(); if (csp) { - response.headers['content-security-policy'] = "default-src 'self'"; + headers.set('Content-Security-Policy', "default-src 'self'"); } + const response = new Response('app shell', {status, headers}); + spyOn(globalThis, 'fetch').mockResolvedValue(response); // When the listener returns its response - onResponse(response); - response.push(null); + const result = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', worker, new AbortController().signal); // Then only a successful response with the expected Worker headers can open the browser - await expect(result).resolves.toBe(ready); + expect(result).toBe(ready); + expect(response.bodyUsed).toBe(true); }); }); diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts index 2955f91a1e0d..ad5ace73464e 100644 --- a/tests/tooling/webDevConfig.test.ts +++ b/tests/tooling/webDevConfig.test.ts @@ -1,8 +1,6 @@ /** Keep local Worker/API opt-outs and coordinated port selection independent. */ import {describe, expect, test} from 'bun:test'; -import net from 'node:net'; - import {getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from '../../scripts/utils/webDevConfig'; describe('web development configuration', () => { @@ -27,34 +25,61 @@ describe('web development configuration', () => { expect(() => getDevelopmentConnectOrigins({EXPENSIFY_URL: 'https://user:pass@example.test'})).toThrow(); }); - test('rejects invalid and overlapping ports before starting children', async () => { + test('rejects invalid and overlapping ports before starting children', () => { // Given invalid coordinated ports // When the launcher checks them // Then startup fails instead of changing the upstream port behind the Worker's back - await expect(getWebDevPorts({WEB_PORT: 'invalid'}, true)).rejects.toThrow(); - await expect(getWebDevPorts({WEB_PORT: '1023'}, true)).rejects.toThrow(); - await expect(getWebDevPorts({WEB_PORT: '65536'}, true)).rejects.toThrow(); - await expect(getWebDevPorts({WEB_PORT: '11000', WEB_INTERNAL_PORT: '11000'}, true)).rejects.toThrow('must differ'); + expect(() => getWebDevPorts({WEB_PORT: 'invalid'}, true)).toThrow(); + expect(() => getWebDevPorts({WEB_PORT: '1023'}, true)).toThrow(); + expect(() => getWebDevPorts({WEB_PORT: '65536'}, true)).toThrow(); + expect(() => getWebDevPorts({WEB_PORT: '11000', WEB_INTERNAL_PORT: '11000'}, true)).toThrow('must differ'); }); - test('fails clearly rather than reusing an occupied worktree origin', async () => { + test('fails clearly rather than reusing an occupied worktree origin', () => { // Given another server owns a port - const server = net.createServer(); - await new Promise((resolve) => { - server.listen(0, '127.0.0.1', resolve); + const listener = Bun.listen({ + hostname: '127.0.0.1', + port: 0, + socket: { + data: (socket) => { + socket.end(); + }, + }, }); - const address = server.address(); - if (!address || typeof address === 'string') { - throw new Error('Missing test port'); - } try { // When this session tries to use it // Then no child is started against that unrelated listener - await expect(getWebDevPorts({WEB_PORT: String(address.port)}, false)).rejects.toThrow('in use'); + expect(() => getWebDevPorts({WEB_PORT: String(listener.port)}, false)).toThrow('in use'); } finally { - await new Promise((resolve) => { - server.close(() => resolve()); - }); + listener.stop(true); } }); + + test('releases the availability probe so the selected port can be bound', () => { + // Given an available local port + const initial = Bun.listen({ + hostname: '127.0.0.1', + port: 0, + socket: { + data: (socket) => { + socket.end(); + }, + }, + }); + const port = initial.port; + initial.stop(true); + // When a session resolves and probes that port + expect(getWebDevPorts({WEB_PORT: String(port)}, false).publicPort).toBe(port); + // Then its actual server can bind the port immediately + const listener = Bun.listen({ + hostname: '127.0.0.1', + port, + socket: { + data: (socket) => { + socket.end(); + }, + }, + }); + listener.stop(true); + }); }); From 67ac29dad9caf4edbf38afca1c1bf660190c0798 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 22 Oct 2026 06:08:54 -0700 Subject: [PATCH 04/31] style(web): prefix boolean variable names --- config/rsbuild/rsbuild.config.ts | 10 +++++----- scripts/startWeb.ts | 24 ++++++++++++------------ scripts/utils/checkWebServer.ts | 4 ++-- scripts/utils/webDevConfig.ts | 8 ++++---- tests/tooling/checkWebServer.test.ts | 20 ++++++++++---------- tests/tooling/webDevConfig.test.ts | 8 ++++---- 6 files changed, 37 insertions(+), 37 deletions(-) diff --git a/config/rsbuild/rsbuild.config.ts b/config/rsbuild/rsbuild.config.ts index 0c4c389184e6..4242403ec167 100644 --- a/config/rsbuild/rsbuild.config.ts +++ b/config/rsbuild/rsbuild.config.ts @@ -48,9 +48,9 @@ export default defineConfig(async ({command}) => { }; } - const useWorker = process.env.USE_CLOUDFLARE_WORKER === 'true'; + const isWorkerEnabled = process.env.USE_CLOUDFLARE_WORKER === 'true'; const publicPort = process.env.WEB_PORT ? Number(process.env.WEB_PORT) : await portfinder.getPortPromise({port: BASE_PORT}); - const port = useWorker ? Number(process.env.WEB_INTERNAL_PORT) : publicPort; + const port = isWorkerEnabled ? Number(process.env.WEB_INTERNAL_PORT) : publicPort; // Check if the USE_WEB_PROXY variable has been provided and rewrite any requests to the local proxy server const proxy = process.env.USE_WEB_PROXY === 'false' ? undefined : [{pathFilter: ['/api', '/staging', '/staging-secure', '/chat-attachments', '/receipts'], target: 'http://[::1]:9000'}]; @@ -63,7 +63,7 @@ export default defineConfig(async ({command}) => { assetPrefix: '/', lazyCompilation: false, }, - security: useWorker ? {nonce: 'nonce-random-value'} : common.security, + security: isWorkerEnabled ? {nonce: 'nonce-random-value'} : common.security, source: { ...common.source, define: { @@ -78,11 +78,11 @@ export default defineConfig(async ({command}) => { proxy, port, strictPort: !!process.env.WEB_PORT, - host: useWorker ? '127.0.0.1' : 'dev.new.expensify.com', + host: isWorkerEnabled ? '127.0.0.1' : 'dev.new.expensify.com', // Rsbuild's --open flag defaults to the first entry's route, e.g. "/main" for our "main" entry key, // which 404s since the app has no route by that name. Open the actual app root instead. open: process.env.WEB_PORT ? false : '/', - https: useWorker + https: isWorkerEnabled ? undefined : { key: fs.readFileSync(path.join(dirname, 'key.pem')), diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 91ed89a4610e..b9f57a3cd6b3 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -7,9 +7,9 @@ import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, ge async function startWeb() { // Bun loads .env automatically, preserving explicit shell overrides. - const {useWorker, useProxy, openBrowser} = getWebDevFlags(Bun.env); - const {publicPort, internalPort} = getWebDevPorts(Bun.env, useWorker); - if (useProxy) { + const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); + const {publicPort, internalPort} = getWebDevPorts(Bun.env, isWorkerEnabled); + if (isProxyEnabled) { assertWebPortAvailable(9000, '::'); } for (const file of ['key.pem', 'certificate.pem']) { @@ -17,23 +17,23 @@ async function startWeb() { throw new Error('Missing local HTTPS certificate. Run npm run setup-https first.'); } } - if (useWorker && !(await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists())) { + if (isWorkerEnabled && !(await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists())) { throw new Error('Missing Worker dependencies. Run npm install to finish App setup.'); } const origin = `https://dev.new.expensify.com:${publicPort}`; Bun.env.WEB_PORT = String(publicPort); Bun.env.WEB_INTERNAL_PORT = String(internalPort); - Bun.env.USE_CLOUDFLARE_WORKER = String(useWorker); - Bun.env.USE_WEB_PROXY = String(useProxy); + Bun.env.USE_CLOUDFLARE_WORKER = String(isWorkerEnabled); + Bun.env.USE_WEB_PROXY = String(isProxyEnabled); const root = process.cwd(); const workerDir = `${root}/web/cloudflare-worker`; const caRoot = (await $`mkcert -CAROOT`.text()).trim(); const ca = await Bun.file(`${caRoot}/rootCA.pem`).text(); // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. const commands = [ - ...(useProxy ? [{name: 'api', command: 'npm run web-proxy'}] : []), + ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy'}] : []), {name: 'rsbuild', command: 'node node_modules/@rsbuild/core/bin/rsbuild.js dev --config config/rsbuild/rsbuild.config.ts'}, - ...(useWorker + ...(isWorkerEnabled ? [ { name: 'worker', @@ -65,10 +65,10 @@ async function startWeb() { const deadline = Date.now() + 180_000; while (!controller.signal.aborted) { try { - const ready = await checkWebServer(origin, ca, useWorker, controller.signal); - if (ready) { - console.log(`Web app ready at ${origin}/ (${useWorker ? 'Cloudflare Worker' : 'direct Rsbuild'})`); - if (openBrowser && !controller.signal.aborted) { + const isReady = await checkWebServer(origin, ca, isWorkerEnabled, controller.signal); + if (isReady) { + console.log(`Web app ready at ${origin}/ (${isWorkerEnabled ? 'Cloudflare Worker' : 'direct Rsbuild'})`); + if (shouldOpenBrowser && !controller.signal.aborted) { const browserCommands: Partial> = {darwin: 'open', win32: 'cmd'}; const command = browserCommands[process.platform] ?? 'xdg-open'; const args = process.platform === 'win32' ? ['/c', 'start', '', `${origin}/`] : [`${origin}/`]; diff --git a/scripts/utils/checkWebServer.ts b/scripts/utils/checkWebServer.ts index 6718d497b17b..22c5c17168d7 100644 --- a/scripts/utils/checkWebServer.ts +++ b/scripts/utils/checkWebServer.ts @@ -1,12 +1,12 @@ /** Probe the public HTTPS listener without disabling certificate verification. */ -async function checkWebServer(origin: string, ca: string, useWorker: boolean, signal: AbortSignal): Promise { +async function checkWebServer(origin: string, ca: string, isWorkerEnabled: boolean, signal: AbortSignal): Promise { try { const response = await fetch(origin, { tls: {ca}, signal: AbortSignal.any([signal, AbortSignal.timeout(2000)]), }); await response.body?.cancel(); - return response.status === 200 && (!useWorker || response.headers.has('Content-Security-Policy')); + return response.status === 200 && (!isWorkerEnabled || response.headers.has('Content-Security-Policy')); } catch { return false; } diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index 0936b8b3c0b8..b69e83070b1a 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -2,7 +2,7 @@ const LOCAL_BACKEND_KEYS = ['EXPENSIFY_URL', 'SECURE_EXPENSIFY_URL', 'STAGING_EXPENSIFY_URL', 'STAGING_SECURE_EXPENSIFY_URL', 'NGROK_URL', 'SECURE_NGROK_URL']; function getWebDevFlags(env: Record) { - return {useWorker: env.USE_CLOUDFLARE_WORKER !== 'false', useProxy: env.USE_WEB_PROXY !== 'false', openBrowser: env.WEB_OPEN !== 'false'}; + return {isWorkerEnabled: env.USE_CLOUDFLARE_WORKER !== 'false', isProxyEnabled: env.USE_WEB_PROXY !== 'false', shouldOpenBrowser: env.WEB_OPEN !== 'false'}; } function getDevelopmentConnectOrigins(env: Record): string[] { @@ -41,13 +41,13 @@ function assertWebPortAvailable(port: number, host = '127.0.0.1'): void { } } -function getWebDevPorts(env: Record, useWorker: boolean) { +function getWebDevPorts(env: Record, isWorkerEnabled: boolean) { const publicPort = Number(env.WEB_PORT ?? 8082); const internalPort = Number(env.WEB_INTERNAL_PORT ?? publicPort + 1); - if (useWorker && publicPort === internalPort) { + if (isWorkerEnabled && publicPort === internalPort) { throw new Error('WEB_PORT and WEB_INTERNAL_PORT must differ'); } - for (const port of useWorker ? [publicPort, internalPort] : [publicPort]) { + for (const port of isWorkerEnabled ? [publicPort, internalPort] : [publicPort]) { if (!Number.isInteger(port) || port < 1024 || port > 65535) { throw new Error('WEB_PORT and WEB_INTERNAL_PORT must be integers between 1024 and 65535'); } diff --git a/tests/tooling/checkWebServer.test.ts b/tests/tooling/checkWebServer.test.ts index 5920ce675d28..6becc77f575b 100644 --- a/tests/tooling/checkWebServer.test.ts +++ b/tests/tooling/checkWebServer.test.ts @@ -13,9 +13,9 @@ describe('public web server readiness', () => { const get = spyOn(globalThis, 'fetch').mockRejectedValue(error); const signal = new AbortController().signal; // When the launcher probes the public origin - const ready = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', true, signal); + const isReady = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', true, signal); // Then it can retry without disabling TLS verification - expect(ready).toBe(false); + expect(isReady).toBe(false); const [url, init] = get.mock.calls.at(0) ?? []; expect(url).toBe('https://dev.new.expensify.com:8082'); expect(init).toMatchObject({tls: {ca: 'test CA'}}); @@ -59,22 +59,22 @@ describe('public web server readiness', () => { }); test.each([ - {status: 200, csp: true, worker: true, ready: true}, - {status: 200, csp: false, worker: true, ready: false}, - {status: 503, csp: true, worker: true, ready: false}, - {status: 200, csp: false, worker: false, ready: true}, - ])('requires a usable response from the selected public listener: %j', async ({status, csp, worker, ready}) => { + {status: 200, hasCSP: true, isWorkerEnabled: true, isReady: true}, + {status: 200, hasCSP: false, isWorkerEnabled: true, isReady: false}, + {status: 503, hasCSP: true, isWorkerEnabled: true, isReady: false}, + {status: 200, hasCSP: false, isWorkerEnabled: false, isReady: true}, + ])('requires a usable response from the selected public listener: %j', async ({status, hasCSP, isWorkerEnabled, isReady}) => { // Given either the direct server or the Worker listener const headers = new Headers(); - if (csp) { + if (hasCSP) { headers.set('Content-Security-Policy', "default-src 'self'"); } const response = new Response('app shell', {status, headers}); spyOn(globalThis, 'fetch').mockResolvedValue(response); // When the listener returns its response - const result = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', worker, new AbortController().signal); + const isServerReady = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', isWorkerEnabled, new AbortController().signal); // Then only a successful response with the expected Worker headers can open the browser - expect(result).toBe(ready); + expect(isServerReady).toBe(isReady); expect(response.bodyUsed).toBe(true); }); }); diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts index ad5ace73464e..3f94eeeba143 100644 --- a/tests/tooling/webDevConfig.test.ts +++ b/tests/tooling/webDevConfig.test.ts @@ -9,10 +9,10 @@ describe('web development configuration', () => { // When flags are resolved const defaults = getWebDevFlags({}); // Then neither opt-out changes the meaning of the other - expect(defaults).toEqual({useWorker: true, useProxy: true, openBrowser: true}); - expect(getWebDevFlags({USE_WEB_PROXY: 'false'})).toMatchObject({useWorker: true, useProxy: false}); - expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', WEB_OPEN: 'false'})).toEqual({useWorker: false, useProxy: true, openBrowser: false}); - expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'})).toMatchObject({useWorker: false, useProxy: false}); + expect(defaults).toEqual({isWorkerEnabled: true, isProxyEnabled: true, shouldOpenBrowser: true}); + expect(getWebDevFlags({USE_WEB_PROXY: 'false'})).toMatchObject({isWorkerEnabled: true, isProxyEnabled: false}); + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', WEB_OPEN: 'false'})).toEqual({isWorkerEnabled: false, isProxyEnabled: true, shouldOpenBrowser: false}); + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'})).toMatchObject({isWorkerEnabled: false, isProxyEnabled: false}); }); test('validates and deduplicates explicit backend origins', () => { From fd2a2f1cc1c78a6e7242fce0d6d37363f054517b Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 22 Oct 2026 11:13:21 -0700 Subject: [PATCH 05/31] fix(web): wait for Rsbuild before starting Worker --- .github/workflows/cloudflareWorkerTests.yml | 1 + scripts/startWeb.ts | 1 + scripts/waitForWebServer.ts | 45 ++++++++++++++++++ tests/tooling/waitForWebServer.test.ts | 52 +++++++++++++++++++++ web/cloudflare-worker/README.md | 16 +++++++ 5 files changed, 115 insertions(+) create mode 100644 scripts/waitForWebServer.ts create mode 100644 tests/tooling/waitForWebServer.test.ts diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index 7f650947f6b5..91380d66819b 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -7,6 +7,7 @@ on: paths: - 'web/cloudflare-worker/**' - 'scripts/startWeb.ts' + - 'scripts/waitForWebServer.ts' - 'scripts/utils/webDevConfig.ts' - 'scripts/utils/checkWebServer.ts' - 'scripts/postInstall.sh' diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index b9f57a3cd6b3..84bd97e6013a 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -39,6 +39,7 @@ async function startWeb() { name: 'worker', cwd: workerDir, command: [ + `bun ../../scripts/waitForWebServer.ts http://127.0.0.1:${internalPort}/ &&`, 'node node_modules/wrangler/bin/wrangler.js dev --local --config wrangler.dev.jsonc', `--ip 127.0.0.1 --port ${publicPort} --inspector-port 0 --local-protocol https`, `--https-key-path ${$.escape(`${root}/config/rsbuild/key.pem`)}`, diff --git a/scripts/waitForWebServer.ts b/scripts/waitForWebServer.ts new file mode 100644 index 000000000000..38f201382f21 --- /dev/null +++ b/scripts/waitForWebServer.ts @@ -0,0 +1,45 @@ +/** Gate dependent dev processes on an HTTP response, not a listening socket or a compiler log message. */ +async function waitForWebServer(origin: string, signal: AbortSignal, {timeout = 180_000, interval = 250} = {}): Promise { + const deadline = Date.now() + timeout; + while (!signal.aborted && Date.now() < deadline) { + try { + const response = await fetch(origin, {signal: AbortSignal.any([signal, AbortSignal.timeout(Math.min(2000, Math.max(1, deadline - Date.now())))]), redirect: 'manual'}); + await response.body?.cancel(); + if (response.status === 200) { + signal.throwIfAborted(); + return; + } + } catch { + // The upstream may not have a listener yet, or may still be compiling its first response. + } + if (!signal.aborted) { + await Bun.sleep(Math.min(interval, Math.max(0, deadline - Date.now()))); + } + } + signal.throwIfAborted(); + throw new Error(`Timed out waiting for Rsbuild at ${origin}. Check its output above.`); +} + +if (import.meta.main) { + const controller = new AbortController(); + const stop = () => controller.abort(); + process.once('SIGINT', stop); + process.once('SIGTERM', stop); + try { + const origin = Bun.argv.at(2); + if (!origin) { + throw new Error('Missing Rsbuild readiness URL'); + } + await waitForWebServer(origin, controller.signal); + } catch (error) { + if (!controller.signal.aborted) { + console.error(error instanceof Error ? error.message : error); + } + process.exitCode = 1; + } finally { + process.removeListener('SIGINT', stop); + process.removeListener('SIGTERM', stop); + } +} + +export default waitForWebServer; diff --git a/tests/tooling/waitForWebServer.test.ts b/tests/tooling/waitForWebServer.test.ts new file mode 100644 index 000000000000..86fb4117ae4d --- /dev/null +++ b/tests/tooling/waitForWebServer.test.ts @@ -0,0 +1,52 @@ +/** Startup dependencies should wait for usable HTTP responses and stop cleanly on failure or cancellation. */ +import {describe, expect, test} from 'bun:test'; + +import waitForWebServer from '../../scripts/waitForWebServer'; + +describe('Rsbuild startup gate', () => { + test('waits through unavailable responses before allowing the dependent command to start', async () => { + // Given an upstream whose first responses are not ready + let requests = 0; + const server = Bun.serve({port: 0, hostname: '127.0.0.1', fetch: () => new Response('compiling', {status: ++requests < 3 ? 503 : 200})}); + try { + // When the gate waits for the server + await waitForWebServer(server.url.href, new AbortController().signal, {timeout: 1000, interval: 5}); + // Then it returns only after a successful response + expect(requests).toBe(3); + } finally { + await server.stop(true); + } + }); + + test('reports compilation/startup timeouts instead of starting the Worker', async () => { + // Given an upstream that never becomes ready + const server = Bun.serve({port: 0, hostname: '127.0.0.1', fetch: () => new Response(null, {status: 503})}); + try { + // When the startup deadline expires + // Then the gate rejects, so the shell never starts Wrangler + await expect(waitForWebServer(server.url.href, new AbortController().signal, {timeout: 50, interval: 5})).rejects.toThrow('Timed out waiting for Rsbuild'); + } finally { + await server.stop(true); + } + }); + + test('cancels an in-flight request when the session stops', async () => { + // Given a request that cannot yet return headers + const controller = new AbortController(); + const server = Bun.serve({ + port: 0, + hostname: '127.0.0.1', + fetch: () => { + controller.abort(); + return new Promise(() => {}); + }, + }); + try { + // When the session is cancelled + // Then readiness rejects promptly rather than starting a dependent process + await expect(waitForWebServer(server.url.href, controller.signal)).rejects.toThrow(); + } finally { + await server.stop(true); + } + }); +}); diff --git a/web/cloudflare-worker/README.md b/web/cloudflare-worker/README.md index 51bad166e51a..8ae769d002e0 100644 --- a/web/cloudflare-worker/README.md +++ b/web/cloudflare-worker/README.md @@ -32,6 +32,22 @@ Only the explicit `development` environment can forward to `DEV_UPSTREAM`, which `USE_CLOUDFLARE_WORKER=false npm run web` bypasses the Worker but leaves the API proxy enabled. `USE_WEB_PROXY=false` disables only the API proxy. `WEB_PORT`, `WEB_INTERNAL_PORT`, and `WEB_OPEN=false` control isolated sessions and browser automation. Shell values take precedence over `.env`; occupied ports fail clearly. See the web setup guide for local backend settings and troubleshooting. +### Startup and logs + +Wrangler starts only after Rsbuild returns its first successful HTTP response, so normal compilation does not produce Worker `503` request logs. The browser opens after the public HTTPS Worker is ready. + +Worker `console.log`, `console.warn`, and `console.error` output appears in the terminal running `npm run web`, prefixed with `[worker]`. This is separate from the application's browser console. + +Wrangler also exposes a local observability API on the same listener. For the default public port, query recent captured request and console logs with: + +```bash +curl https://127.0.0.1:8082/cdn-cgi/local/explorer/api/local/observability/query \ + -H 'Content-Type: application/json' \ + --data '{"sql":"SELECT ts_ms, level, message FROM logs ORDER BY ts_ms DESC LIMIT 20"}' +``` + +Use your chosen `WEB_PORT` if different. This is a local API, not a separate hosted Miniflare service; no Cloudflare credentials are required. + ## Running a web build locally `wrangler dev` runs the Worker in workerd, the same runtime Cloudflare uses, and serves `dist/` exactly as a deploy would. No Cloudflare account or credentials are needed. From 7ab263d557641ec5efe41f21ef634e1c744afeb0 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 22 Oct 2026 11:15:38 -0700 Subject: [PATCH 06/31] docs: keep implementation plan local --- plans/104387-local-cloudflare-worker.md | 160 ------------------------ 1 file changed, 160 deletions(-) delete mode 100644 plans/104387-local-cloudflare-worker.md diff --git a/plans/104387-local-cloudflare-worker.md b/plans/104387-local-cloudflare-worker.md deleted file mode 100644 index 90c71431ce47..000000000000 --- a/plans/104387-local-cloudflare-worker.md +++ /dev/null @@ -1,160 +0,0 @@ -# Run the Cloudflare Worker during local web development - - - -## Goal and scope - -Implement [Expensify/App#104387](https://github.com/Expensify/App/issues/104387): `npm run web` should run the actual NewDot Worker in local workerd through Wrangler/Miniflare, in front of the live Rsbuild server. Developers should encounter CSP, nonce, header, and routing problems locally, without sacrificing hot reload or requiring a Cloudflare account, a built `dist/`, or ngrok. - -This document is a plan, not an implementation or a claim that the proposed topology has already passed browser testing. - -- Base: freshly fetched App `origin/main`, `bc54f602df745cd1104297e7e1e230f8f130f1e8`. -- Branch: `plan/104387-local-cloudflare-worker`. -- Worktree: `/Users/rory/Expensidev/App-Worktrees/104387-local-cloudflare-worker`. -- Keep all implementation in App. Do not change live Cloudflare routes, deploy anything, modify the legacy Worker repository, or alter unrelated worktrees. - -## Findings that shape the solution - -1. Root `package.json` currently starts `web/proxy.ts` and Rsbuild concurrently; Rsbuild opens its own HTTPS origin. The Worker is a separately locked npm package, not a root workspace. -2. `web/cloudflare-worker/src/index.ts` already owns security headers, CSP nonces, HTML cache invalidation, AASA handling, missing-asset handling, and immutable caching. Use that implementation, not a separate imitation of the Worker in Rsbuild middleware. -3. The current Worker config requires `../../dist` and defaults to the staging CSP. Its static-assets/R2 tests and built-site preview commands must continue to work unchanged in meaning. -4. Rsbuild currently selects an available port starting at 8082 and embeds it as `process.env.PORT`; `CONFIG.DEV_PORT` uses that value. Moving the browser-facing origin without updating this contract can break links even if the first page loads. -5. `USE_WEB_PROXY=false` already disables the API proxy. Preserve that meaning. Add a separate Worker opt-out rather than silently repurposing this flag. -6. The old [local-testing instructions](https://github.com/Expensify/Cloudflare-Workers/tree/main/new.expensify.com#testing-locally) use ngrok to reach Rsbuild. [Cloudflare-Workers#188](https://github.com/Expensify/Cloudflare-Workers/pull/188) explicitly describes a Wrangler/custom-HTTP-port problem, linking [workers-sdk#5774](https://github.com/cloudflare/workers-sdk/issues/5774#issuecomment-2779972187). Separately, [App#38072](https://github.com/Expensify/App/issues/38072#issuecomment-2113231238) records an older `NODE_EXTRA_CA_CERTS` workaround. These are historical clues, not proof that the same problems affect the currently pinned Wrangler 4.144.0. - -Reference code at the base revision: - -- [Web scripts and installation](https://github.com/Expensify/App/blob/bc54f602df745cd1104297e7e1e230f8f130f1e8/package.json#L12-L35) -- [Rsbuild development configuration](https://github.com/Expensify/App/blob/bc54f602df745cd1104297e7e1e230f8f130f1e8/config/rsbuild/rsbuild.config.ts#L51-L89) -- [Worker response pipeline](https://github.com/Expensify/App/blob/bc54f602df745cd1104297e7e1e230f8f130f1e8/web/cloudflare-worker/src/index.ts#L83-L130) - -## Preferred architecture: HTTPS outside, loopback HTTP inside - -```text -Browser: https://dev.new.expensify.com:8082 - -> Wrangler / local workerd (existing mkcert certificate) - -> http://127.0.0.1: (Rsbuild, live in-memory output) - -> existing API proxy on port 9000, when enabled - -> configured backend - -Browser HMR: wss://dev.new.expensify.com:8082/rsbuild-hmr - -> Worker WebSocket pass-through -> Rsbuild -``` - -Use 8082 as the preferred public port, preserving the existing hostname, browser storage, secure-context behavior, and familiar setup instructions. Select a distinct internal Rsbuild port. Share the resolved public and internal ports with all children; never let Rsbuild independently increment away from the Worker's configured upstream. On conflicts, select a coordinated available pair or fail with an actionable error, never attach to another worktree's server. Embed the **public** port in `CONFIG.DEV_PORT`. - -The Worker must construct an explicit upstream URL with scheme, loopback address, port, path, and query. Do not depend on the old `dev.host`/implicit-origin forwarding behavior. Loopback HTTP avoids Worker-to-Rsbuild certificate validation entirely; the browser still uses trusted HTTPS and WSS. It does not justify allowing HTTP assets or insecure WebSockets in the browser. - -### First implementation gate: prove the transport before plumbing everything in - -- With pinned Wrangler and a minimal loopback HTTP/WebSocket upstream, verify custom-port fetch, streaming, POST forwarding, and a 101 upgrade through local workerd. -- Start Wrangler HTTPS using `--local-protocol https`, `--https-key-path`, and `--https-cert-path` pointing to the existing `config/rsbuild` mkcert files. Verify certificate trust in the browser, not just `curl -k`. -- Then repeat against real Rsbuild, checking its Host/Origin validation, HMR token/query handling, and emitted asset URLs. -- If WebSocket pass-through is incompatible with the current runtime, evaluate a direct **WSS** HMR connection to an HTTPS Rsbuild listener as the narrow fallback; keep page/assets behind the Worker and document the exact development-only CSP addition. Do not use `ws:` from an HTTPS page. -- Only retain ngrok as an explicitly documented troubleshooting fallback if a reproducible pinned-version issue remains. Do not require a tunnel for the default workflow, globally disable TLS verification, or clear `NODE_EXTRA_CA_CERTS` as a blanket fix. - -## Implementation sequence - -### 1. Install Worker tooling during normal setup - -Keep the Worker as an independently locked package for minimal CI/deploy risk. Add a deterministic `npm ci --prefix web/cloudflare-worker --include=dev` step to root `scripts/postInstall.sh`, with clear failure output. Regenerate the Worker lockfile only if dependencies actually change; no Wrangler upgrade is necessary just to implement this issue. - -Verify root `npm install`, root `npm ci`, and standalone setup install the Worker without recursive root lifecycle execution. Inspect HybridApp/CI callers before finalizing the hook; account for environments that intentionally omit development dependencies. Avoid assuming an npm install should be network-free, but ensure launching an already-installed `npm run web` never installs packages or asks for Cloudflare login. If inspection shows the hook is unsuitable, choose workspace integration explicitly and update all standalone Worker CI/deploy installs and locks together, rather than leaving mixed ownership. - -### 2. Add local-only live-upstream configuration - -- Add a dedicated development Wrangler config for live Rsbuild mode, using the same `src/index.ts` entry point and compatibility settings, but **no static-assets directory, prepare-dist build, remote bindings, or R2 dependency**. Factor shared config only if needed to prevent drift; do not duplicate Worker application logic. -- Select `ENVIRONMENT=development` explicitly and pass a local upstream binding only in this mode. Do not change the existing default built-site staging preview into live development mode. -- Preserve Wrangler's native source watching and automatic Worker reload for both the entry point and imported modules (including `src/csp.ts`). Editing Worker code must take effect on subsequent requests without restarting `npm run web`, rebuilding the app, or restarting Rsbuild/the API proxy. This is a separate requirement from React Fast Refresh; it does not require automatically refreshing an already-loaded browser document to replace its CSP headers. -- Regenerate/check Worker types for both configurations. Model the development upstream and production `ASSETS` binding without weakening typing to `any` or accidentally making deployment bindings optional everywhere. -- Select live forwarding only when the explicit development environment and validated loopback upstream are both present. Staging/production must never proxy to an arbitrary configured origin, even if an upstream variable is accidentally supplied. - -### 3. Reuse the Worker response pipeline - -- Separate response acquisition (Rsbuild fetch versus `ASSETS.fetch`) from the shared security-header/nonce transformation. -- Preserve request method, body, query, relevant headers, status, and streaming. Avoid double body consumption. Handle redirects deliberately so internal loopback URLs are not exposed to the browser. -- Pass WebSocket upgrades through without reconstructing the 101 response as an ordinary `Response` or feeding it into `HTMLRewriter`. -- Route API/attachment requests through Rsbuild's existing proxy table before any SPA/missing-static-asset logic; JSON API errors, receipt downloads, and non-GET requests must never become the app shell. Preserve existing backend response and caching semantics for these routes. -- For application assets/HTML, keep shared security headers and per-response nonce replacement. In development, disable immutable caching for changing bundles/hot-update files; maintain the HTML no-validator rule. Handle conditional upstream responses so a 304 cannot pair old HTML with a new CSP nonce. -- Preserve SPA navigation versus missing-asset distinctions, reserved-character deep links, AASA aliases/content type, HEAD behavior, and normal upstream errors. Return a useful local error when Rsbuild is unavailable, not a stale `dist/` fallback. -- Keep static-assets and previous-build R2 behavior intact for built-site previews and deploy environments. Document that live mode tests the common Worker pipeline, not the deployed asset router/R2 itself; the built-output test pass covers those separately. - -### 4. Add the smallest development-only CSP delta - -- Extend the existing CSP builder rather than copying staging/production policies. Keep their serialized directives unchanged apart from the existing per-request nonce. -- Add only explicit local origins required by observed requests: the chosen HMR WSS origin and, where needed, local API/backend origins such as `http://localhost:9000`. Normally the browser should use same-origin API routes through the Worker, not connect directly to port 9000. -- Verify browser mixed-content rules separately: putting an origin in CSP does not make every insecure request legal. -- Inspect actual development HTML for Rsbuild-injected inline scripts/React Refresh bootstrap. Supply nonces or narrowly scoped hashes when required; do not solve startup failures by adding blanket `script-src 'unsafe-inline'`, `connect-src *`, or disabling enforcement. Preserve the production template's existing hashes/nonce contract. -- Exercise internal `.env` local-backend overrides as well as contributor defaults. If configurable origins are needed, validate them and scope them to development rather than hard-coding one engineer's hostname or permitting all domains. - -### 5. Wire startup, browser opening, and opt-outs - -- Add a small typed launcher/config helper, following existing scripts/tooling conventions, to resolve environment precedence, the coordinated ports, readiness, and the browser origin once. -- Have `npm run web` retain Pusher setup and run the API proxy, Rsbuild, and Worker under `concurrently`. Suppress Rsbuild's `--open` in Worker mode and open exactly one browser tab at the Worker's root after the app is ready. Provide a no-open path for automation. -- Add `USE_CLOUDFLARE_WORKER=false` as the independent escape hatch: run/open Rsbuild directly over its existing trusted HTTPS setup. Keep `USE_WEB_PROXY=false` solely for disabling the API proxy. -- Load `.env` consistently with shell overrides taking precedence. Confirm the launcher, Rsbuild, API proxy, and bundled `CONFIG.IS_USING_WEB_PROXY` agree. Do not assume shell variables automatically override the serialized `__REACT_WEB_CONFIG__` object. -- On Ctrl-C or failure of a required long-running child, stop siblings and release ports. An intentionally disabled API proxy must not be treated as a crash that kills the other services. Surface missing certificates and occupied API/inspector ports clearly. - -### 6. Document the supported workflows - -Update root `README.md`, `contributingGuides/SETUP_WEB.md`, and `web/cloudflare-worker/README.md` with: - -- One normal install/start path, public versus internal origin, existing mkcert/hosts prerequisites, and no ngrok/Cloudflare credentials requirement. -- Independent Worker and API-proxy flags, shell/`.env` precedence, readiness/port-conflict behavior, and how to test with a local backend. -- The difference between live Worker development and the retained `dist/` staging/production CSP preview commands. -- HMR/nonce/cache exceptions confined to development; troubleshooting for certificate trust, stale service workers, upstream failure, and optional ngrok fallback only if needed. -- Replace the overly broad claim that local sign-in is impossible: it applies to a built-site preview without an API proxy, not necessarily live development with the existing API proxy. Do not promise third-party OAuth works on arbitrary local origins. - -## Validation plan (required before calling implementation complete) - -### Automated checks - -Extend existing `node:test`/workerd integration tests with a real ephemeral loopback upstream fixture, rather than mocking only `fetch`. Retain all existing static-assets tests. Document new tests with Given/When/Then comments. - -| Area | Required assertions | -| --- | --- | -| Live transport | Path/query/method/body preservation, streaming, redirects, HEAD, upstream failures, WebSocket round trip and query/token preservation | -| Headers/nonces | Header set present; nonce matches allowed script and differs between loads; no placeholder remains; HTML cannot return a mismatched 304 | -| Routing | Root/deep links/reserved characters work; missing JS is 404 rather than HTML; API errors/downloads bypass SPA fallback; AASA aliases remain correct | -| Development cache | Edited bundles/hot updates are not immutable or stale; HTML validators removed | -| Worker reload | Under the real launcher, entry-point and imported CSP-module edits trigger Wrangler reload; subsequent requests reflect changes without restarting the command or other services; React HMR still works afterward | -| Isolation | Local origins allowed only in development; staging/production unchanged and cannot activate upstream forwarding; existing R2/static-asset tests pass | -| Launcher | Default/opt-out combinations, `.env` versus shell precedence, public port injection, coordinated port conflict, readiness, child failure and teardown | -| Installation | Fresh root/standalone install brings in Wrangler; existing independent Worker install/CI remains valid | - -Run the Worker package's `npm test` and `npm run typecheck`; run focused Bun tooling tests for launcher/config helpers, `npm run lint-changed`, `npm run spell-changed`, and root `npm run typecheck` for changed TypeScript configuration/scripts. Run shellcheck if installation shell scripts change. React Compiler checks are needed only if React components/hooks change, which is not expected. - -Expand `.github/workflows/cloudflareWorkerTests.yml` path filters if shared launcher/config/CSP inputs outside the Worker package now affect the tests. Keep its standalone install working. Run `wrangler deploy --dry-run` for `test-staging`, `staging`, and `production` against a real/fixture build to verify deploy configurations exclude development upstream/CSP settings; do not deploy. - -### Real local end-to-end pass, including Playwright MCP - -1. **Prepare an isolated environment.** Use this dedicated worktree and repo-pinned Node/npm/Bun versions. Ensure mkcert files and the hosts entry are present without copying private configuration into Git. Use a disposable browser profile because the Worker sends HSTS. Record chosen ports and confirm existing servers/worktrees are not being reused. No `dist/`, Worker `.dev.vars`, Cloudflare credentials, or ngrok should be required for this pass. -2. **Start the real command.** Run `npm run web` in the worktree with automation browser-opening disabled. Capture startup logs; verify all expected services run and the frontend readiness check passes at the Worker origin. Check HTTPS with certificate verification enabled. Confirm an ordinary interactive launch opens only the Worker URL. -3. **Attach Playwright MCP to the Worker origin**, not the internal Rsbuild port. Capture console messages, page errors, failed requests, and `securitypolicyviolation` events from before navigation using the browser tooling. Use observable UI/network readiness, not arbitrary sleeps. MCP is a required test tool, not a replacement for automated regression tests. -4. **Prove App startup.** Navigate to `/`, wait for the splash to disappear and the usable sign-in form to appear, verify scripts/CSS/fonts/images load, and save a screenshot plus console/network evidence. A 200 response or a rendered splash alone is not a pass. Compare CSP headers with script `.nonce` properties and check fresh navigations use fresh nonces. -5. **Prove the app runs beyond startup.** Use an approved disposable account/backend. For a local development backend, follow its test-account/code rules; do not assume the local `000000` code works against production. Complete sign-in/onboarding, navigate to settings and a report, open an existing attachment if available, refresh a deep link, and verify authenticated API calls and realtime connectivity. Prefer a local backend fixture for mutations; never create test financial data on production. Record any unavailable backend/account prerequisites as blockers rather than claiming end-to-end success. -6. **Prove both independent reload loops work.** First observe the successful WSS connection, make a temporary reversible React UI edit in this worktree, and confirm Fast Refresh applies it without a full-page reload and without losing entered text or the session. Check hot-update JS/JSON responses and CSP events, then restore only this test edit. Next, with the same `npm run web` session still running, make a harmless temporary Worker entry-point edit (such as a diagnostic response header), observe Wrangler reload, and verify subsequent requests expose the change. Separately edit the imported `src/csp.ts` module with a harmless development-only policy change, and verify a fresh Playwright navigation receives the updated enforced CSP. Do not restart the command, rebuild the app, or restart Rsbuild/the API proxy. Verify the authenticated session remains usable and React HMR works again after the Worker reload (allowing automatic WebSocket reconnection). Restore only these test edits and verify Wrangler reloads the original behavior. Record reload logs and before/after headers; an already-loaded page need not automatically acquire new document headers. -7. **Prove enforcement, not just absence of complaints.** In the disposable browser, attempt a harmless inline script without a nonce with a unique marker and a connection to an unapproved test origin. Verify CSP blocks them and produces the expected violation events. Allowed legitimate code/HMR must still work; expected negative-test violations must be distinguished from unexpected app violations. -8. **Check routing/cache/failure behavior.** Use browser network inspection and HTTP assertions for `/index.html`, a real deep link, a missing JS asset, AASA, and conditional HTML requests. Restart Rsbuild to check useful failure/recovery, then test fresh load and warm reload. Check for stale service-worker interference; live dev need not acquire a production service worker, but built-output preview must retain its service-worker behavior. -9. **Run the opt-out matrix below**, restarting cleanly for each case. Verify the chosen origin and running processes, not merely command exit status. Exercise both shell flags and `.env` configuration. Include an internal-backend configuration with the API proxy disabled. -10. **Validate built output separately.** Build staging and production output and use the retained Worker preview commands over HTTPS. Repeat startup/header/nonce/deep-link/cache/service-worker checks and run existing header-comparison tooling where appropriate. This validates real static-assets behavior absent from live Rsbuild mode. Built-preview sign-in/OAuth CORS limitations are distinct from live-mode regressions. -11. **Teardown and evidence.** Stop only processes started for this test, verify ports are released, remove temporary UI edits, and retain reproducible commands, test results, screenshots, and sanitized console/network logs. Do not retain auth tokens, test-account credentials, or certificate keys in committed artifacts. Report any pre-existing third-party network errors separately, with no unexplained new app/CSP failures accepted. - -| Worker | API proxy | Expected result | -| --- | --- | --- | -| Default on | Default on | Worker HTTPS origin, real app load/sign-in, API forwarding, React Fast Refresh, and automatic Worker source/module reload | -| Off | On | Existing direct HTTPS Rsbuild experience; no Worker process | -| On | Off | Worker remains active; configured local backend works; no port-9000 proxy | -| Off | Off | Direct Rsbuild plus configured backend; neither proxy process | - -## Acceptance and residual risks - -- A fresh normal setup plus `npm run web` loads and operates the app through the real local Worker, with functional HMR and no mandatory ngrok, `dist/`, extra install, or Cloudflare authentication. -- Both reload loops are required: React edits apply through Fast Refresh, and Worker entry-point/imported-module edits apply through Wrangler's native reload on subsequent requests without manually restarting the development stack. Worker reload must not leave React HMR disconnected or the app unusable. -- Local security enforcement is verified positively and negatively in Playwright MCP, including authenticated interaction; HTTP-only fixtures or sign-in-page screenshots are insufficient evidence. -- Static-assets/deploy behavior and staging/production CSP are unchanged; development allowances cannot leak into deploy configurations. -- Opt-outs are independent and documented; process lifecycle, ports, certificate trust, and environment precedence are covered by tests. -- The transport spike is an explicit prerequisite: custom-port loopback fetch and WebSocket behavior have not yet been demonstrated in this planning task. If the preferred path fails, document the precise pinned-version failure and narrow fallback before expanding scope. -- Potential gaps that local workerd cannot fully reproduce include Cloudflare edge routing/cache behavior and provider-controlled OAuth origin restrictions. Do not weaken CSP or claim production parity for those merely to obtain a green local smoke test. - -Official implementation references: [Wrangler dev options](https://developers.cloudflare.com/workers/wrangler/commands/workers/#dev), [Rsbuild HMR client configuration](https://rsbuild.dev/config/dev/client), and [Workers local development](https://developers.cloudflare.com/workers/local-development/). Verify flags against the pinned CLI's `--help` during implementation. From 97588debc126b3d02878115c5e4e4c8032033391 Mon Sep 17 00:00:00 2001 From: rory Date: Thu, 22 Oct 2026 14:38:58 -0700 Subject: [PATCH 07/31] refactor(web): simplify local Worker setup --- config/rsbuild/rsbuild.config.ts | 3 +- contributingGuides/SETUP_WEB.md | 2 +- scripts/utils/webDevConfig.ts | 28 +++++++++---------- web/cloudflare-worker/src/csp.ts | 18 ++++++------ .../src/fetchDevelopmentResponse.ts | 6 ++-- 5 files changed, 26 insertions(+), 31 deletions(-) diff --git a/config/rsbuild/rsbuild.config.ts b/config/rsbuild/rsbuild.config.ts index 4242403ec167..60bf8f600c8d 100644 --- a/config/rsbuild/rsbuild.config.ts +++ b/config/rsbuild/rsbuild.config.ts @@ -68,8 +68,7 @@ export default defineConfig(async ({command}) => { ...common.source, define: { ...common.source?.define, - // The dev server's resolved port can differ from BASE_PORT if 8082 is already in use. - // CONFIG.DEV_PORT (src/CONFIG.ts) reads this at runtime to build correct dev environment URLs. + // CONFIG.DEV_PORT builds browser-facing links, so it must not use the private Rsbuild port. // eslint-disable-next-line @typescript-eslint/naming-convention 'process.env.PORT': publicPort, }, diff --git a/contributingGuides/SETUP_WEB.md b/contributingGuides/SETUP_WEB.md index cfd1c23fcf86..96cf800dbc00 100644 --- a/contributingGuides/SETUP_WEB.md +++ b/contributingGuides/SETUP_WEB.md @@ -30,7 +30,7 @@ If you already generated certificates before the Webpack → Rsbuild migration, - To run the **development web app**: `npm run web` - Changes applied to Javascript will be applied automatically via Rsbuild as configured in `config/rsbuild/rsbuild.config.ts` -`npm run web` starts the API proxy, Rsbuild, and the Cloudflare Worker in local workerd through Wrangler. It opens `https://dev.new.expensify.com:8082/` after the Worker serves the app. The Worker uses your existing mkcert certificate and forwards to a private loopback HTTP Rsbuild listener; you do not need ngrok, a Cloudflare account, or a `dist/` build. +`npm run web` starts the API proxy, Rsbuild, and the Cloudflare Worker in local workerd through Wrangler. It opens `https://dev.new.expensify.com:8082/` after the Worker serves the app. The Worker uses your existing mkcert certificate and forwards to a private loopback HTTP Rsbuild listener. React edits use Fast Refresh through the Worker's WSS connection. Worker entry-point and imported module edits (including CSP changes) use Wrangler's native reload without restarting the command or the other services. A new document request/reload is needed to receive changed CSP headers; existing pages do not automatically acquire new response headers. diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index b69e83070b1a..ea7b7adc1ffa 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -6,21 +6,19 @@ function getWebDevFlags(env: Record) { } function getDevelopmentConnectOrigins(env: Record): string[] { - return [ - ...new Set( - LOCAL_BACKEND_KEYS.flatMap((key) => { - const value = env[key]; - if (!value) { - return []; - } - const url = new URL(value); - if (!['http:', 'https:'].includes(url.protocol) || url.username || url.password) { - throw new Error(`${key} must be an HTTP(S) URL without credentials`); - } - return [url.origin]; - }), - ), - ]; + const origins = new Set(); + for (const key of LOCAL_BACKEND_KEYS) { + const value = env[key]; + if (!value) { + continue; + } + const url = new URL(value); + if (!['http:', 'https:'].includes(url.protocol) || url.username || url.password) { + throw new Error(`${key} must be an HTTP(S) URL without credentials`); + } + origins.add(url.origin); + } + return [...origins]; } /** Fail on conflicts rather than letting a child silently attach to another worktree's server. */ diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index 3238f3957527..8ba126d23400 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -170,16 +170,14 @@ const PRODUCTION_CSP = buildCSP({ }); function getCSP(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, connectSrcHosts: string[] = []): string { - const csp = - environment === 'development' - ? buildCSP({ - fontSrcHosts: ["'self'"], - scriptSrcHashes: INLINE_SCRIPT_HASHES, - connectSrcHosts: ['https://www.expensify.com.dev', 'https://secure.expensify.com.dev', 'http://localhost:9000', ...connectSrcHosts], - }) - : environment === 'staging' - ? STAGING_CSP - : PRODUCTION_CSP; + let csp = environment === 'staging' ? STAGING_CSP : PRODUCTION_CSP; + if (environment === 'development') { + csp = buildCSP({ + fontSrcHosts: ["'self'"], + scriptSrcHashes: INLINE_SCRIPT_HASHES, + connectSrcHosts: ['https://www.expensify.com.dev', 'https://secure.expensify.com.dev', 'http://localhost:9000', ...connectSrcHosts], + }); + } return csp.replace(NONCE_PLACEHOLDER, `nonce-${nonce}`); } diff --git a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts index 603fe35c11c6..e0311f0132f5 100644 --- a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts +++ b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts @@ -46,8 +46,8 @@ async function fetchDevelopmentResponse(request: Request, env: DevelopmentEnv): return response; } const location = response.headers.get('Location'); - if (location && new URL(location, url).origin === upstream.origin) { - const redirect = new URL(location, url); + const redirect = location ? new URL(location, url) : null; + if (redirect?.origin === upstream.origin) { redirect.protocol = publicURL.protocol; redirect.host = publicURL.host; response = new Response(response.body, response); @@ -56,4 +56,4 @@ async function fetchDevelopmentResponse(request: Request, env: DevelopmentEnv): return response; } -export {fetchDevelopmentResponse, getDevelopmentUpstream, isAPIRequest}; +export {fetchDevelopmentResponse, isAPIRequest}; From 727fe41b9d467fa068e0f244b9de0a157008115b Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 13:32:08 -0700 Subject: [PATCH 08/31] fix(web): allow local backend media and frames in dev CSP The dev CSP only widened connect-src, so proxied and local-backend videos hit media-src and the OldDot statement iframe hit frame-src. Allow 'self' and every local backend origin for media, and the backends for frames. Staging and production are unchanged. --- web/cloudflare-worker/src/csp.ts | 26 ++++++++++++++++--- web/cloudflare-worker/src/index.ts | 14 +++++----- .../tests/development.test.ts | 23 +++++++++++++--- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index 8ba126d23400..0b249c8ab2e7 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -6,8 +6,21 @@ type CSPOptions = { frameSrcHosts?: string[]; scriptSrcHashes?: string[]; connectSrcHosts?: string[]; + mediaSrcHosts?: string[]; }; +/** Origins only the live development Worker allows, beyond what the deployed CSP does. */ +type DevelopmentOrigins = { + /** The public Worker origin as a WebSocket origin, for Rsbuild HMR. */ + hmrOrigin: string; + + /** Local backends from `.env`, which serve API responses, attachments, and OldDot pages. */ + backendOrigins: string[]; +}; + +/** The default local OldDot backend, which the app calls, plays videos from, and frames (e.g. wallet statements). */ +const DEFAULT_DEVELOPMENT_BACKEND_ORIGINS = ['https://www.expensify.com.dev', 'https://secure.expensify.com.dev']; + /** Inline scripts allowed without a nonce. Tests check that every such script in `web/index.html` is listed. */ const INLINE_SCRIPT_HASHES = [ 'sha256-hK550RmP3t+9myNCpVty39wCs9CDUTXjWUP30rrXvD0=', @@ -18,7 +31,7 @@ const INLINE_SCRIPT_HASHES = [ 'sha256-ZC4Ihfl+1sv3E25DQh090ITQKwffxiocyA9C1vaePKU=', ]; -function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = [], connectSrcHosts = []}: CSPOptions): string { +function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = [], connectSrcHosts = [], mediaSrcHosts = []}: CSPOptions): string { const csp: Record = { 'default-src': ["'self'", 'https://assets.onfido.com'], 'script-src': [ @@ -149,7 +162,7 @@ function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = [], ], 'worker-src': ["'self'", 'blob:'], 'child-src': ["'self'", 'blob:'], - 'media-src': ['blob:', 'https://assets.onfido.com', 'https://d2k5nsl2zxldvw.cloudfront.net', 'https://staging.expensify.com', 'https://www.expensify.com'], + 'media-src': [...mediaSrcHosts, 'blob:', 'https://assets.onfido.com', 'https://d2k5nsl2zxldvw.cloudfront.net', 'https://staging.expensify.com', 'https://www.expensify.com'], }; return Object.entries(csp) @@ -169,16 +182,21 @@ const PRODUCTION_CSP = buildCSP({ scriptSrcHashes: INLINE_SCRIPT_HASHES, }); -function getCSP(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, connectSrcHosts: string[] = []): string { +function getCSP(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, developmentOrigins?: DevelopmentOrigins): string { let csp = environment === 'staging' ? STAGING_CSP : PRODUCTION_CSP; if (environment === 'development') { + const backendOrigins = [...new Set([...DEFAULT_DEVELOPMENT_BACKEND_ORIGINS, ...(developmentOrigins?.backendOrigins ?? [])])]; csp = buildCSP({ fontSrcHosts: ["'self'"], + frameSrcHosts: backendOrigins, scriptSrcHashes: INLINE_SCRIPT_HASHES, - connectSrcHosts: ['https://www.expensify.com.dev', 'https://secure.expensify.com.dev', 'http://localhost:9000', ...connectSrcHosts], + connectSrcHosts: [...backendOrigins, 'http://localhost:9000', ...(developmentOrigins ? [developmentOrigins.hmrOrigin] : [])], + // 'self' covers attachments the Rsbuild proxy serves from the app origin. + mediaSrcHosts: ["'self'", ...backendOrigins], }); } return csp.replace(NONCE_PLACEHOLDER, `nonce-${nonce}`); } export {NONCE_PLACEHOLDER, getCSP}; +export type {DevelopmentOrigins}; diff --git a/web/cloudflare-worker/src/index.ts b/web/cloudflare-worker/src/index.ts index cbaca7671be2..2fe5a0f03a7e 100644 --- a/web/cloudflare-worker/src/index.ts +++ b/web/cloudflare-worker/src/index.ts @@ -1,3 +1,5 @@ +import type {DevelopmentOrigins} from './csp'; + import {getCSP, NONCE_PLACEHOLDER} from './csp'; import {fetchDevelopmentResponse, isAPIRequest} from './fetchDevelopmentResponse'; @@ -85,9 +87,9 @@ function buildAssetRequest(request: Request, url: URL, pathSuffix = ''): Request return assetRequest; } -function setSecurityHeaders(headers: Headers, environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, connectSrcHosts: string[] = []) { +function setSecurityHeaders(headers: Headers, environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, developmentOrigins?: DevelopmentOrigins) { headers.set('Strict-Transport-Security', 'max-age=31536000; includeSubDomains; preload'); - headers.set('Content-Security-Policy', getCSP(environment, nonce, connectSrcHosts)); + headers.set('Content-Security-Policy', getCSP(environment, nonce, developmentOrigins)); headers.set('X-Frame-Options', 'SAMEORIGIN'); headers.set('X-Content-Type-Options', 'nosniff'); headers.set('Referrer-Policy', 'origin-when-cross-origin'); @@ -193,13 +195,13 @@ export default { const isPrecompressed = !isDevelopment && PRECOMPRESSED_PATH.test(url.pathname); // Cloudflare presents every request with `Accept-Encoding: br, gzip` and decodes the twin for clients without Brotli. let served: ServedFile; - let connectSrcHosts: string[] = []; + let developmentOrigins: DevelopmentOrigins | undefined; if (isDevelopment) { const assetResponse = await fetchDevelopmentResponse(request, env); if (assetResponse.status === 101 || isAPIRequest(url.pathname)) { return assetResponse; } - connectSrcHosts = [url.origin.replace(/^https:/, 'wss:').replace(/^http:/, 'ws:'), ...(JSON.parse(env.DEV_CONNECT_ORIGINS) as string[])]; + developmentOrigins = {hmrOrigin: url.origin.replace(/^https:/, 'wss:').replace(/^http:/, 'ws:'), backendOrigins: JSON.parse(env.DEV_CONNECT_ORIGINS) as string[]}; served = {response: assetResponse, isBrotli: false}; } else { served = await getAsset(env, request, url, isPrecompressed); @@ -212,7 +214,7 @@ export default { if (!previousBuildFile) { await served.response.body?.cancel(); const notFound = new Response('Not Found', {status: 404, headers: {'Content-Type': 'text/plain; charset=utf-8'}}); - setSecurityHeaders(notFound.headers, env.ENVIRONMENT, nonce, connectSrcHosts); + setSecurityHeaders(notFound.headers, env.ENVIRONMENT, nonce, developmentOrigins); return notFound; } await served.response.body?.cancel(); @@ -221,7 +223,7 @@ export default { const {response: file, isBrotli} = served; const headers = new Headers(file.headers); - setSecurityHeaders(headers, env.ENVIRONMENT, nonce, connectSrcHosts); + setSecurityHeaders(headers, env.ENVIRONMENT, nonce, developmentOrigins); if (isAASAPath) { headers.set('Content-Type', 'application/json'); diff --git a/web/cloudflare-worker/tests/development.test.ts b/web/cloudflare-worker/tests/development.test.ts index 1f75fa2f0f67..0c13e4c0e3ec 100644 --- a/web/cloudflare-worker/tests/development.test.ts +++ b/web/cloudflare-worker/tests/development.test.ts @@ -86,7 +86,7 @@ describe('live development worker', () => { '--var', `DEV_UPSTREAM:http://127.0.0.1:${address.port}`, '--var', - 'DEV_CONNECT_ORIGINS:["https://www.expensify.com.dev"]', + 'DEV_CONNECT_ORIGINS:["https://www.expensify.com.dev","http://backend.test:8080"]', ], {cwd: path.resolve(import.meta.dirname, '..'), env: {...process.env, WRANGLER_SEND_METRICS: 'false'}}, ); @@ -154,6 +154,23 @@ describe('live development worker', () => { assert.doesNotMatch(csp, /script-src[^;]*'unsafe-inline'|connect-src \*/); }); + test('allows local backends to serve videos and the OldDot statement iframe', async () => { + // Given proxied attachments, the default local OldDot and a configured backend + // When the Worker serves a page + const csp = (await server.fetch('/')).headers.get('Content-Security-Policy') ?? ''; + const getSources = (directive: string) => new RegExp(`(?:^|; )${directive} ([^;]*)`).exec(csp)?.[1].split(' ') ?? []; + // Then videos load from the app origin and every local backend, which can also be framed + for (const source of ["'self'", 'https://www.expensify.com.dev', 'http://backend.test:8080']) { + assert.ok(getSources('media-src').includes(source), `media-src is missing ${source}`); + } + for (const source of ['https://www.expensify.com.dev', 'http://backend.test:8080']) { + assert.ok(getSources('frame-src').includes(source), `frame-src is missing ${source}`); + } + // And the HMR WebSocket stays a connect-only source + const hmrOrigin = origin.replace('http:', 'ws:'); + assert.ok(!getSources('media-src').includes(hmrOrigin) && !getSources('frame-src').includes(hmrOrigin)); + }); + test('does not apply immutable caching to live bundles or mistake missing files for the shell', async () => { // Given a live bundle and an unknown static asset // When both are fetched @@ -226,8 +243,8 @@ describe('live development worker', () => { writeFileSync( entryPath, entry.replace( - 'setSecurityHeaders(headers, env.ENVIRONMENT, nonce, connectSrcHosts);', - 'setSecurityHeaders(headers, env.ENVIRONMENT, nonce, connectSrcHosts); headers.set("X-Reload-Proof", "entry");', + 'setSecurityHeaders(headers, env.ENVIRONMENT, nonce, developmentOrigins);', + 'setSecurityHeaders(headers, env.ENVIRONMENT, nonce, developmentOrigins); headers.set("X-Reload-Proof", "entry");', ), ); await waitForResponse((response) => response.headers.get('X-Reload-Proof') === 'entry'); From 26aa6d5adde5fc595c00e17f7e810e642f44e095 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 13:32:46 -0700 Subject: [PATCH 09/31] refactor(web): reuse createNotFoundResponse for dev 404s Pass the development origins through instead of inlining the 404 response. --- web/cloudflare-worker/src/index.ts | 8 +++----- web/cloudflare-worker/tests/development.test.ts | 2 ++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/web/cloudflare-worker/src/index.ts b/web/cloudflare-worker/src/index.ts index 2fe5a0f03a7e..c5b0a0978107 100644 --- a/web/cloudflare-worker/src/index.ts +++ b/web/cloudflare-worker/src/index.ts @@ -97,9 +97,9 @@ function setSecurityHeaders(headers: Headers, environment: Env['ENVIRONMENT'] | headers.set('Document-Policy', 'js-profiling'); } -function createNotFoundResponse(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string): Response { +function createNotFoundResponse(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, developmentOrigins?: DevelopmentOrigins): Response { const notFound = new Response('Not Found', {status: 404, headers: {'Content-Type': 'text/plain; charset=utf-8'}}); - setSecurityHeaders(notFound.headers, environment, nonce); + setSecurityHeaders(notFound.headers, environment, nonce, developmentOrigins); return notFound; } @@ -213,9 +213,7 @@ export default { const previousBuildFile = isAASAPath || isDevelopment ? null : await getPreviousBuildFile(env, url, request.method, isPrecompressed); if (!previousBuildFile) { await served.response.body?.cancel(); - const notFound = new Response('Not Found', {status: 404, headers: {'Content-Type': 'text/plain; charset=utf-8'}}); - setSecurityHeaders(notFound.headers, env.ENVIRONMENT, nonce, developmentOrigins); - return notFound; + return createNotFoundResponse(env.ENVIRONMENT, nonce, developmentOrigins); } await served.response.body?.cancel(); served = previousBuildFile; diff --git a/web/cloudflare-worker/tests/development.test.ts b/web/cloudflare-worker/tests/development.test.ts index 0c13e4c0e3ec..61d86776dfce 100644 --- a/web/cloudflare-worker/tests/development.test.ts +++ b/web/cloudflare-worker/tests/development.test.ts @@ -180,6 +180,8 @@ describe('live development worker', () => { assert.equal(bundle.headers.get('Cache-Control'), 'no-store'); assert.equal(missing.status, 404); assert.match(missing.headers.get('Content-Type') ?? '', /text\/plain/); + // And the 404 keeps the development CSP, like every other live response + assert.ok(missing.headers.get('Content-Security-Policy')?.includes(origin.replace('http:', 'ws:'))); }); test('preserves backend errors/downloads without treating HTML errors as SPA responses', async () => { From cf6b80fa045acb62b128f9475bcdf11995d77be3 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 13:37:04 -0700 Subject: [PATCH 10/31] fix(web): stop requiring mkcert to start the dev server The readiness probe read the mkcert root CA via `mkcert -CAROOT`, so `npm run web` failed without mkcert on PATH, even with the Worker off. `npm run setup-https` already trusts that CA in the OS store, so verify against the system CAs instead. --- scripts/startWeb.ts | 4 +--- scripts/types/tls.d.ts | 6 ++++++ scripts/utils/checkWebServer.ts | 11 ++++++++--- tests/tooling/checkWebServer.test.ts | 14 ++++++++------ 4 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 scripts/types/tls.d.ts diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 84bd97e6013a..29658858609c 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -27,8 +27,6 @@ async function startWeb() { Bun.env.USE_WEB_PROXY = String(isProxyEnabled); const root = process.cwd(); const workerDir = `${root}/web/cloudflare-worker`; - const caRoot = (await $`mkcert -CAROOT`.text()).trim(); - const ca = await Bun.file(`${caRoot}/rootCA.pem`).text(); // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. const commands = [ ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy'}] : []), @@ -66,7 +64,7 @@ async function startWeb() { const deadline = Date.now() + 180_000; while (!controller.signal.aborted) { try { - const isReady = await checkWebServer(origin, ca, isWorkerEnabled, controller.signal); + const isReady = await checkWebServer(origin, isWorkerEnabled, controller.signal); if (isReady) { console.log(`Web app ready at ${origin}/ (${isWorkerEnabled ? 'Cloudflare Worker' : 'direct Rsbuild'})`); if (shouldOpenBrowser && !controller.signal.aborted) { diff --git a/scripts/types/tls.d.ts b/scripts/types/tls.d.ts new file mode 100644 index 000000000000..2e5ecd06bf46 --- /dev/null +++ b/scripts/types/tls.d.ts @@ -0,0 +1,6 @@ +/** Bun and Node 26 implement tls.getCACertificates, but the repo's @types/node predates it. */ +declare module 'tls' { + function getCACertificates(type?: 'default' | 'system' | 'bundled' | 'extra'): string[]; +} + +export {}; diff --git a/scripts/utils/checkWebServer.ts b/scripts/utils/checkWebServer.ts index 22c5c17168d7..50f4eb2adfa7 100644 --- a/scripts/utils/checkWebServer.ts +++ b/scripts/utils/checkWebServer.ts @@ -1,8 +1,13 @@ -/** Probe the public HTTPS listener without disabling certificate verification. */ -async function checkWebServer(origin: string, ca: string, isWorkerEnabled: boolean, signal: AbortSignal): Promise { +import {getCACertificates} from 'node:tls'; + +/** + * Probe the public HTTPS listener without disabling certificate verification. `npm run setup-https` trusts the mkcert CA + * in the OS store, so reading that store avoids requiring mkcert on PATH whenever the server starts. + */ +async function checkWebServer(origin: string, isWorkerEnabled: boolean, signal: AbortSignal): Promise { try { const response = await fetch(origin, { - tls: {ca}, + tls: {ca: getCACertificates('system')}, signal: AbortSignal.any([signal, AbortSignal.timeout(2000)]), }); await response.body?.cancel(); diff --git a/tests/tooling/checkWebServer.test.ts b/tests/tooling/checkWebServer.test.ts index 6becc77f575b..e7039f5cd233 100644 --- a/tests/tooling/checkWebServer.test.ts +++ b/tests/tooling/checkWebServer.test.ts @@ -1,6 +1,8 @@ /** Readiness probes must time out or cancel without blocking the development launcher. */ import {afterEach, describe, expect, mock, spyOn, test} from 'bun:test'; +import {getCACertificates} from 'node:tls'; + import checkWebServer from '../../scripts/utils/checkWebServer'; describe('public web server readiness', () => { @@ -13,12 +15,12 @@ describe('public web server readiness', () => { const get = spyOn(globalThis, 'fetch').mockRejectedValue(error); const signal = new AbortController().signal; // When the launcher probes the public origin - const isReady = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', true, signal); - // Then it can retry without disabling TLS verification + const isReady = await checkWebServer('https://dev.new.expensify.com:8082', true, signal); + // Then it can retry, verifying TLS against the OS trust store that `npm run setup-https` adds the mkcert CA to expect(isReady).toBe(false); const [url, init] = get.mock.calls.at(0) ?? []; expect(url).toBe('https://dev.new.expensify.com:8082'); - expect(init).toMatchObject({tls: {ca: 'test CA'}}); + expect(init).toMatchObject({tls: {ca: getCACertificates('system')}}); expect(init?.signal).toBeInstanceOf(AbortSignal); }, ); @@ -34,7 +36,7 @@ describe('public web server readiness', () => { ); spyOn(globalThis, 'fetch').mockImplementation(waitingFetch); const controller = new AbortController(); - const result = checkWebServer('https://dev.new.expensify.com:8082', 'test CA', true, controller.signal); + const result = checkWebServer('https://dev.new.expensify.com:8082', true, controller.signal); // When the session is stopped controller.abort(); // Then readiness finishes without waiting for the timeout @@ -50,7 +52,7 @@ describe('public web server readiness', () => { }); try { // When the launcher probes it without an external cancellation - const result = checkWebServer(server.url.href, 'test CA', false, new AbortController().signal); + const result = checkWebServer(server.url.href, false, new AbortController().signal); // Then the built-in request timeout releases the probe rather than hanging startup await expect(result).resolves.toBe(false); } finally { @@ -72,7 +74,7 @@ describe('public web server readiness', () => { const response = new Response('app shell', {status, headers}); spyOn(globalThis, 'fetch').mockResolvedValue(response); // When the listener returns its response - const isServerReady = await checkWebServer('https://dev.new.expensify.com:8082', 'test CA', isWorkerEnabled, new AbortController().signal); + const isServerReady = await checkWebServer('https://dev.new.expensify.com:8082', isWorkerEnabled, new AbortController().signal); // Then only a successful response with the expected Worker headers can open the browser expect(isServerReady).toBe(isReady); expect(response.bodyUsed).toBe(true); From 9d524e6279051c5cb1559a479d26efce4ae166b1 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 13:38:51 -0700 Subject: [PATCH 11/31] test(web): release the pending handler before stopping the test server Bun's server.stop(true) waits for a handler that never settles, so the cancellation test always hit the 5s test timeout during cleanup. --- tests/tooling/waitForWebServer.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/tooling/waitForWebServer.test.ts b/tests/tooling/waitForWebServer.test.ts index 86fb4117ae4d..fec58825deca 100644 --- a/tests/tooling/waitForWebServer.test.ts +++ b/tests/tooling/waitForWebServer.test.ts @@ -33,12 +33,13 @@ describe('Rsbuild startup gate', () => { test('cancels an in-flight request when the session stops', async () => { // Given a request that cannot yet return headers const controller = new AbortController(); + const {promise: response, resolve: sendResponse} = Promise.withResolvers(); const server = Bun.serve({ port: 0, hostname: '127.0.0.1', fetch: () => { controller.abort(); - return new Promise(() => {}); + return response; }, }); try { @@ -46,6 +47,8 @@ describe('Rsbuild startup gate', () => { // Then readiness rejects promptly rather than starting a dependent process await expect(waitForWebServer(server.url.href, controller.signal)).rejects.toThrow(); } finally { + // Bun cannot stop a server while a handler is still pending. + sendResponse(new Response('ready')); await server.stop(true); } }); From fec21798b3c02833d9b46c6dff3f93af2bcac930 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 13:40:35 -0700 Subject: [PATCH 12/31] fix(web): don't stop the dev session on a slow first compile Both readiness loops gave up after 180s, which killed every service when the first compile ran long. The Rsbuild gate now waits until Rsbuild responds or the session stops, and the launcher only warns once when startup is slow. --- .github/workflows/cloudflareWorkerTests.yml | 1 + scripts/startWeb.ts | 39 +++++++---------- scripts/utils/waitForPublicWebServer.ts | 36 ++++++++++++++++ scripts/waitForWebServer.ts | 7 ++- tests/tooling/waitForPublicWebServer.test.ts | 45 ++++++++++++++++++++ tests/tooling/waitForWebServer.test.ts | 26 ++++++++++- 6 files changed, 127 insertions(+), 27 deletions(-) create mode 100644 scripts/utils/waitForPublicWebServer.ts create mode 100644 tests/tooling/waitForPublicWebServer.test.ts diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index 91380d66819b..edf5daa8bd1d 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -10,6 +10,7 @@ on: - 'scripts/waitForWebServer.ts' - 'scripts/utils/webDevConfig.ts' - 'scripts/utils/checkWebServer.ts' + - 'scripts/utils/waitForPublicWebServer.ts' - 'scripts/postInstall.sh' - 'config/rsbuild/**' - 'package.json' diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 29658858609c..d3eef9689ea2 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -2,7 +2,7 @@ import {$} from 'bun'; import concurrently from 'concurrently'; -import checkWebServer from './utils/checkWebServer'; +import waitForPublicWebServer from './utils/waitForPublicWebServer'; import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; async function startWeb() { @@ -61,29 +61,20 @@ async function startWeb() { process.once('SIGTERM', stop); // Verify the browser-facing listener and certificate, not just the internal compiler's listener. const readiness = (async () => { - const deadline = Date.now() + 180_000; - while (!controller.signal.aborted) { - try { - const isReady = await checkWebServer(origin, isWorkerEnabled, controller.signal); - if (isReady) { - console.log(`Web app ready at ${origin}/ (${isWorkerEnabled ? 'Cloudflare Worker' : 'direct Rsbuild'})`); - if (shouldOpenBrowser && !controller.signal.aborted) { - const browserCommands: Partial> = {darwin: 'open', win32: 'cmd'}; - const command = browserCommands[process.platform] ?? 'xdg-open'; - const args = process.platform === 'win32' ? ['/c', 'start', '', `${origin}/`] : [`${origin}/`]; - $`${command} ${args}`.quiet().catch((error: unknown) => { - console.error(`Open ${origin}/ in your browser:`, error); - }); - } - return; - } - } catch { - // The dev server can be between starting its listener and completing its first compilation. - } - if (Date.now() > deadline) { - throw new Error(`Timed out waiting for ${origin}. Check the dev server output.`); - } - await Bun.sleep(250); + const isReady = await waitForPublicWebServer(origin, isWorkerEnabled, controller.signal, { + onSlow: () => console.warn(`Still waiting for ${origin}/. Check the dev server output. If it compiled, check that its certificate is trusted (npm run setup-https).`), + }); + if (!isReady) { + return; + } + console.log(`Web app ready at ${origin}/ (${isWorkerEnabled ? 'Cloudflare Worker' : 'direct Rsbuild'})`); + if (shouldOpenBrowser) { + const browserCommands: Partial> = {darwin: 'open', win32: 'cmd'}; + const command = browserCommands[process.platform] ?? 'xdg-open'; + const args = process.platform === 'win32' ? ['/c', 'start', '', `${origin}/`] : [`${origin}/`]; + $`${command} ${args}`.quiet().catch((error: unknown) => { + console.error(`Open ${origin}/ in your browser:`, error); + }); } })(); try { diff --git a/scripts/utils/waitForPublicWebServer.ts b/scripts/utils/waitForPublicWebServer.ts new file mode 100644 index 000000000000..f9cdf081a54a --- /dev/null +++ b/scripts/utils/waitForPublicWebServer.ts @@ -0,0 +1,36 @@ +import checkWebServer from './checkWebServer'; + +type WaitForPublicWebServerOptions = { + interval?: number; + + /** How long startup can take before `onSlow` reports it. */ + slowAfter?: number; + onSlow?: () => void; +}; + +/** + * Wait until the browser-facing listener serves the app. A slow first compile is reported, not fatal: concurrently + * already stops the session if a required service exits. Resolves false if the session stops first. + */ +async function waitForPublicWebServer( + origin: string, + isWorkerEnabled: boolean, + signal: AbortSignal, + {interval = 250, slowAfter = 180_000, onSlow = () => {}}: WaitForPublicWebServerOptions = {}, +): Promise { + const slowAt = Date.now() + slowAfter; + let hasReportedSlow = false; + while (!signal.aborted) { + if (await checkWebServer(origin, isWorkerEnabled, signal)) { + return !signal.aborted; + } + if (!hasReportedSlow && Date.now() > slowAt) { + hasReportedSlow = true; + onSlow(); + } + await Bun.sleep(interval); + } + return false; +} + +export default waitForPublicWebServer; diff --git a/scripts/waitForWebServer.ts b/scripts/waitForWebServer.ts index 38f201382f21..51b75d95d441 100644 --- a/scripts/waitForWebServer.ts +++ b/scripts/waitForWebServer.ts @@ -1,5 +1,8 @@ -/** Gate dependent dev processes on an HTTP response, not a listening socket or a compiler log message. */ -async function waitForWebServer(origin: string, signal: AbortSignal, {timeout = 180_000, interval = 250} = {}): Promise { +/** + * Gate dependent dev processes on an HTTP response, not a listening socket or a compiler log message. There is no default + * deadline, because a failure here stops the session, and concurrently already stops this gate if Rsbuild exits. + */ +async function waitForWebServer(origin: string, signal: AbortSignal, {timeout = Infinity, interval = 250} = {}): Promise { const deadline = Date.now() + timeout; while (!signal.aborted && Date.now() < deadline) { try { diff --git a/tests/tooling/waitForPublicWebServer.test.ts b/tests/tooling/waitForPublicWebServer.test.ts new file mode 100644 index 000000000000..5259d095903f --- /dev/null +++ b/tests/tooling/waitForPublicWebServer.test.ts @@ -0,0 +1,45 @@ +/** The launcher should wait for the public listener as long as the session runs, without stopping it on a slow start. */ +import {afterEach, describe, expect, mock, setSystemTime, spyOn, test} from 'bun:test'; + +import waitForPublicWebServer from '../../scripts/utils/waitForPublicWebServer'; + +function mockFetch(respond: () => Response) { + spyOn(globalThis, 'fetch').mockImplementation(Object.assign(async () => respond(), {preconnect: fetch.preconnect})); +} + +describe('public web server startup', () => { + afterEach(() => { + setSystemTime(); + mock.restore(); + }); + + test('keeps waiting past a slow first compile and reports the delay once', async () => { + // Given a public listener that only becomes ready long after startup + let requests = 0; + mockFetch(() => { + if (++requests === 2) { + setSystemTime(new Date(Date.now() + 10 * 60_000)); + } + return new Response('app shell', {status: requests < 5 ? 503 : 200}); + }); + const onSlow = mock(() => {}); + // When the launcher waits for it + const isReady = await waitForPublicWebServer('https://dev.new.expensify.com:8082', false, new AbortController().signal, {interval: 1, onSlow}); + // Then the session is not stopped, and the delay is reported once rather than on every retry + expect(isReady).toBe(true); + expect(requests).toBe(5); + expect(onSlow).toHaveBeenCalledTimes(1); + }); + + test('stops waiting without readiness when the session stops', async () => { + // Given a listener that never becomes ready + const controller = new AbortController(); + mockFetch(() => { + controller.abort(); + return new Response(null, {status: 503}); + }); + // When the session is stopped + // Then waiting ends without reporting the server as ready + expect(await waitForPublicWebServer('https://dev.new.expensify.com:8082', false, controller.signal, {interval: 1})).toBe(false); + }); +}); diff --git a/tests/tooling/waitForWebServer.test.ts b/tests/tooling/waitForWebServer.test.ts index fec58825deca..550be20b9d52 100644 --- a/tests/tooling/waitForWebServer.test.ts +++ b/tests/tooling/waitForWebServer.test.ts @@ -1,5 +1,5 @@ /** Startup dependencies should wait for usable HTTP responses and stop cleanly on failure or cancellation. */ -import {describe, expect, test} from 'bun:test'; +import {describe, expect, setSystemTime, test} from 'bun:test'; import waitForWebServer from '../../scripts/waitForWebServer'; @@ -18,6 +18,30 @@ describe('Rsbuild startup gate', () => { } }); + test('waits through a slow first compile by default rather than stopping the session', async () => { + // Given an upstream whose first compile outlasts any fixed startup deadline + let requests = 0; + const server = Bun.serve({ + port: 0, + hostname: '127.0.0.1', + fetch: () => { + if (++requests === 2) { + setSystemTime(new Date(Date.now() + 10 * 60_000)); + } + return new Response('compiling', {status: requests < 4 ? 503 : 200}); + }, + }); + try { + // When the gate waits with its default settings + await waitForWebServer(server.url.href, new AbortController().signal, {interval: 5}); + // Then it starts the Worker once Rsbuild responds + expect(requests).toBe(4); + } finally { + setSystemTime(); + await server.stop(true); + } + }); + test('reports compilation/startup timeouts instead of starting the Worker', async () => { // Given an upstream that never becomes ready const server = Bun.serve({port: 0, hostname: '127.0.0.1', fetch: () => new Response(null, {status: 503})}); From 8cc1d51515c9e1a9857666aa53fd01fad12a2e90 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 13:42:05 -0700 Subject: [PATCH 13/31] fix(web): only install the Worker tooling when its lockfile changes postinstall ran the Worker's npm ci on every root install, so CI downloaded wrangler and workerd on every setupNode cache miss and local installs always reinstalled them. Skip CI, where the Worker jobs install it themselves, and skip local installs whose Worker lockfile is unchanged. --- .github/workflows/cloudflareWorkerTests.yml | 1 + scripts/installCloudflareWorker.ts | 34 ++++++++++ scripts/postInstall.sh | 5 +- tests/tooling/installCloudflareWorker.test.ts | 67 +++++++++++++++++++ 4 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 scripts/installCloudflareWorker.ts create mode 100644 tests/tooling/installCloudflareWorker.test.ts diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index edf5daa8bd1d..97dd314d7831 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -12,6 +12,7 @@ on: - 'scripts/utils/checkWebServer.ts' - 'scripts/utils/waitForPublicWebServer.ts' - 'scripts/postInstall.sh' + - 'scripts/installCloudflareWorker.ts' - 'config/rsbuild/**' - 'package.json' # The tests check its inline scripts against the CSP hashes. diff --git a/scripts/installCloudflareWorker.ts b/scripts/installCloudflareWorker.ts new file mode 100644 index 000000000000..ccadcad5094e --- /dev/null +++ b/scripts/installCloudflareWorker.ts @@ -0,0 +1,34 @@ +/** + * Install the independently locked Worker tooling during local App setup, so `npm run web` can run it. CI jobs that need + * the Worker install it themselves, and local installs skip it until its lockfile changes. + */ +import {$} from 'bun'; +import path from 'node:path'; + +/** Kept inside node_modules, so deleting the Worker dependencies also invalidates it. */ +const INSTALLED_LOCKFILE_HASH_PATH = 'node_modules/.app-installed-lockfile-hash'; + +async function runNpmCI(workerDir: string): Promise { + await $`npm ci --include=dev`.cwd(workerDir); +} + +async function installCloudflareWorker(workerDir: string, env: Record = Bun.env, install = runNpmCI): Promise { + if (env.CI) { + return false; + } + const lockfileHash = new Bun.CryptoHasher('sha256').update(await Bun.file(path.join(workerDir, 'package-lock.json')).text()).digest('hex'); + const installedLockfileHash = Bun.file(path.join(workerDir, INSTALLED_LOCKFILE_HASH_PATH)); + if ((await installedLockfileHash.exists()) && (await installedLockfileHash.text()) === lockfileHash) { + return false; + } + console.log('Installing local Cloudflare Worker tooling'); + await install(workerDir); + await Bun.write(installedLockfileHash, lockfileHash); + return true; +} + +if (import.meta.main) { + await installCloudflareWorker(path.resolve(import.meta.dir, '../web/cloudflare-worker')); +} + +export default installCloudflareWorker; diff --git a/scripts/postInstall.sh b/scripts/postInstall.sh index d3337eac04b4..d55013d920e5 100755 --- a/scripts/postInstall.sh +++ b/scripts/postInstall.sh @@ -40,6 +40,5 @@ bun scripts/applyPatches.ts # nothing and exits 0. Use `npx tsc --build`, which walks the references, or `npm run typecheck`. ln -sfn ../typescript/bin/tsc "$ROOT_DIR/node_modules/.bin/tsc" -# Keep the Worker independently installable for deploy CI, but include it in normal App setup. -echo "Installing local Cloudflare Worker tooling" -npm ci --prefix "$ROOT_DIR/web/cloudflare-worker" --include=dev +# Keep the Worker independently installable for deploy CI, but include it in normal local App setup. +bun scripts/installCloudflareWorker.ts diff --git a/tests/tooling/installCloudflareWorker.test.ts b/tests/tooling/installCloudflareWorker.test.ts new file mode 100644 index 000000000000..a3de781c8273 --- /dev/null +++ b/tests/tooling/installCloudflareWorker.test.ts @@ -0,0 +1,67 @@ +/** Root installs should set up the Worker tooling locally, but only when its lockfile changes, and never in CI. */ +import {afterEach, beforeEach, describe, expect, mock, test} from 'bun:test'; + +import {mkdtempSync, rmSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import path from 'node:path'; + +import installCloudflareWorker from '../../scripts/installCloudflareWorker'; + +describe('Cloudflare Worker install during App setup', () => { + let workerDir: string; + const install = mock(async () => { + await Bun.write(path.join(workerDir, 'node_modules/wrangler/package.json'), '{}'); + }); + + beforeEach(async () => { + workerDir = mkdtempSync(path.join(tmpdir(), 'worker-install-')); + await Bun.write(path.join(workerDir, 'package-lock.json'), '{"lockfileVersion": 3}'); + install.mockClear(); + }); + + afterEach(() => rmSync(workerDir, {recursive: true, force: true})); + + test('installs once, then skips root installs until the Worker lockfile changes', async () => { + // Given a fresh checkout + // When the root install runs twice + expect(await installCloudflareWorker(workerDir, {}, install)).toBe(true); + expect(await installCloudflareWorker(workerDir, {}, install)).toBe(false); + // Then the Worker tooling is installed only once + expect(install).toHaveBeenCalledTimes(1); + + // When the Worker lockfile changes + await Bun.write(path.join(workerDir, 'package-lock.json'), '{"lockfileVersion": 3, "packages": {}}'); + // Then the next root install updates the Worker tooling + expect(await installCloudflareWorker(workerDir, {}, install)).toBe(true); + expect(install).toHaveBeenCalledTimes(2); + }); + + test('reinstalls when the Worker node_modules was removed', async () => { + // Given an installed Worker whose dependencies were then deleted + await installCloudflareWorker(workerDir, {}, install); + rmSync(path.join(workerDir, 'node_modules'), {recursive: true, force: true}); + // When the root install runs + // Then the tooling is restored + expect(await installCloudflareWorker(workerDir, {}, install)).toBe(true); + expect(install).toHaveBeenCalledTimes(2); + }); + + test('retries an install that failed', async () => { + // Given an install that fails partway + const failingInstall = mock(async () => { + throw new Error('network error'); + }); + // When the root install runs + // Then it fails, and the next root install tries again + await expect(installCloudflareWorker(workerDir, {}, failingInstall)).rejects.toThrow('network error'); + expect(await installCloudflareWorker(workerDir, {}, install)).toBe(true); + }); + + test('skips CI, where only the Worker jobs need the tooling and install it themselves', async () => { + // Given a CI root install + // When postinstall runs + // Then wrangler and workerd are not downloaded + expect(await installCloudflareWorker(workerDir, {CI: 'true'}, install)).toBe(false); + expect(install).not.toHaveBeenCalled(); + }); +}); From 3b51c6275bd1a27e73c4abcc05dda346742f550f Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:04:45 -0700 Subject: [PATCH 14/31] test(web): fail fast when a reload test edit misses its target A silently unchanged source surfaced 5s later as a misleading reload timeout. Assert each edit applies, and anchor them on stable lines. --- web/cloudflare-worker/tests/development.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/web/cloudflare-worker/tests/development.test.ts b/web/cloudflare-worker/tests/development.test.ts index 61d86776dfce..00b1f8c37020 100644 --- a/web/cloudflare-worker/tests/development.test.ts +++ b/web/cloudflare-worker/tests/development.test.ts @@ -228,6 +228,11 @@ describe('live development worker', () => { const cspPath = path.join(tempDir, 'src/csp.ts'); const entry = readFileSync(entryPath, 'utf8'); const csp = readFileSync(cspPath, 'utf8'); + // A missing target would otherwise leave the source unchanged, and fail later as a misleading reload timeout. + const replaceOnce = (source: string, target: string, replacement: string) => { + assert.ok(source.includes(target), `Reload test target is missing from the Worker source: ${target}`); + return source.replace(target, replacement); + }; const waitForResponse = async (matches: (response: Response) => boolean) => { const deadline = Date.now() + 5000; while (Date.now() < deadline) { @@ -244,13 +249,10 @@ describe('live development worker', () => { // When the entry point and an imported module change, without restarting Wrangler writeFileSync( entryPath, - entry.replace( - 'setSecurityHeaders(headers, env.ENVIRONMENT, nonce, developmentOrigins);', - 'setSecurityHeaders(headers, env.ENVIRONMENT, nonce, developmentOrigins); headers.set("X-Reload-Proof", "entry");', - ), + replaceOnce(entry, "headers.set('X-Content-Type-Options', 'nosniff');", "headers.set('X-Content-Type-Options', 'nosniff'); headers.set('X-Reload-Proof', 'entry');"), ); await waitForResponse((response) => response.headers.get('X-Reload-Proof') === 'entry'); - writeFileSync(cspPath, csp.replace("'http://localhost:9000'", "'http://localhost:9000', 'https://module-reload.invalid'")); + writeFileSync(cspPath, replaceOnce(csp, "'worker-src': [", "'worker-src': ['https://module-reload.invalid', ")); // Then both changes are served on subsequent requests, including the new CSP await waitForResponse((response) => response.headers.get('Content-Security-Policy')?.includes('https://module-reload.invalid') ?? false); } finally { From ce6c4df794811d0a1127471fc317a3e399f04bdf Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:05:59 -0700 Subject: [PATCH 15/31] fix(web): drop redundant hardcoded dev CSP origins Local backends arrive through DEV_CONNECT_ORIGINS, and the browser never calls the server-side API proxy. Keep only www.expensify.com.dev, which the app hardcodes as its development OldDot URL. --- web/cloudflare-worker/src/csp.ts | 11 +++++++---- web/cloudflare-worker/tests/development.test.ts | 12 +++++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index 0b249c8ab2e7..26c03d4fc9df 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -18,8 +18,11 @@ type DevelopmentOrigins = { backendOrigins: string[]; }; -/** The default local OldDot backend, which the app calls, plays videos from, and frames (e.g. wallet statements). */ -const DEFAULT_DEVELOPMENT_BACKEND_ORIGINS = ['https://www.expensify.com.dev', 'https://secure.expensify.com.dev']; +/** + * The app hardcodes this development OldDot origin (CONST.INTERNAL_DEV_EXPENSIFY_URL) for links and the wallet statement + * iframe, so it is allowed even when no local backend is configured. + */ +const DEVELOPMENT_OLDDOT_ORIGIN = 'https://www.expensify.com.dev'; /** Inline scripts allowed without a nonce. Tests check that every such script in `web/index.html` is listed. */ const INLINE_SCRIPT_HASHES = [ @@ -185,12 +188,12 @@ const PRODUCTION_CSP = buildCSP({ function getCSP(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], nonce: string, developmentOrigins?: DevelopmentOrigins): string { let csp = environment === 'staging' ? STAGING_CSP : PRODUCTION_CSP; if (environment === 'development') { - const backendOrigins = [...new Set([...DEFAULT_DEVELOPMENT_BACKEND_ORIGINS, ...(developmentOrigins?.backendOrigins ?? [])])]; + const backendOrigins = [...new Set([DEVELOPMENT_OLDDOT_ORIGIN, ...(developmentOrigins?.backendOrigins ?? [])])]; csp = buildCSP({ fontSrcHosts: ["'self'"], frameSrcHosts: backendOrigins, scriptSrcHashes: INLINE_SCRIPT_HASHES, - connectSrcHosts: [...backendOrigins, 'http://localhost:9000', ...(developmentOrigins ? [developmentOrigins.hmrOrigin] : [])], + connectSrcHosts: [...backendOrigins, ...(developmentOrigins ? [developmentOrigins.hmrOrigin] : [])], // 'self' covers attachments the Rsbuild proxy serves from the app origin. mediaSrcHosts: ["'self'", ...backendOrigins], }); diff --git a/web/cloudflare-worker/tests/development.test.ts b/web/cloudflare-worker/tests/development.test.ts index 00b1f8c37020..25d8fb3219ab 100644 --- a/web/cloudflare-worker/tests/development.test.ts +++ b/web/cloudflare-worker/tests/development.test.ts @@ -86,7 +86,7 @@ describe('live development worker', () => { '--var', `DEV_UPSTREAM:http://127.0.0.1:${address.port}`, '--var', - 'DEV_CONNECT_ORIGINS:["https://www.expensify.com.dev","http://backend.test:8080"]', + 'DEV_CONNECT_ORIGINS:["http://backend.test:8080"]', ], {cwd: path.resolve(import.meta.dirname, '..'), env: {...process.env, WRANGLER_SEND_METRICS: 'false'}}, ); @@ -154,6 +154,16 @@ describe('live development worker', () => { assert.doesNotMatch(csp, /script-src[^;]*'unsafe-inline'|connect-src \*/); }); + test('adds only configured backends and the OldDot origin the app hardcodes', async () => { + // Given a backend from .env, and the app's hardcoded development OldDot URL (CONST.INTERNAL_DEV_EXPENSIFY_URL) + // When the Worker serves a page + const csp = (await server.fetch('/')).headers.get('Content-Security-Policy') ?? ''; + // Then both are allowed, and unconfigured or server-side-only origins are not + assert.ok(csp.includes('http://backend.test:8080')); + assert.match(csp, /frame-src [^;]*https:\/\/www\.expensify\.com\.dev/); + assert.doesNotMatch(csp, /secure\.expensify\.com\.dev|localhost:9000/); + }); + test('allows local backends to serve videos and the OldDot statement iframe', async () => { // Given proxied attachments, the default local OldDot and a configured backend // When the Worker serves a page From 49ab6112ff61cdd3b899d6d98f148ab1113530be Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:07:31 -0700 Subject: [PATCH 16/31] fix(web): validate live Worker vars once with named errors DEV_CONNECT_ORIGINS was cast from JSON.parse on every request, and DEV_UPSTREAM was re-validated each time. Validate both on first use, and name the variable when either is malformed. --- .../src/developmentConfig.ts | 62 +++++++++++++++++++ .../src/fetchDevelopmentResponse.ts | 22 +------ web/cloudflare-worker/src/index.ts | 6 +- .../tests/developmentConfig.test.ts | 37 +++++++++++ 4 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 web/cloudflare-worker/src/developmentConfig.ts create mode 100644 web/cloudflare-worker/tests/developmentConfig.test.ts diff --git a/web/cloudflare-worker/src/developmentConfig.ts b/web/cloudflare-worker/src/developmentConfig.ts new file mode 100644 index 000000000000..7fed71591dd9 --- /dev/null +++ b/web/cloudflare-worker/src/developmentConfig.ts @@ -0,0 +1,62 @@ +/** Validated settings that `npm run web` passes to the live development Worker as vars. */ +type DevelopmentConfig = { + /** The private loopback Rsbuild origin the Worker forwards to. */ + upstream: URL; + + /** Local backends from `.env`, which serve API responses, attachments, and OldDot pages. */ + backendOrigins: string[]; +}; + +type DevelopmentVars = { + DEV_UPSTREAM: string; + DEV_CONNECT_ORIGINS: string; +}; + +function parseUpstream(value: string): URL { + const upstream = URL.parse(value); + if ( + upstream?.protocol !== 'http:' || + upstream.hostname !== '127.0.0.1' || + !upstream.port || + upstream.pathname !== '/' || + upstream.search || + upstream.hash || + upstream.username || + upstream.password + ) { + throw new Error(`DEV_UPSTREAM must be an HTTP loopback origin with an explicit port, got ${value}`); + } + return upstream; +} + +function isHTTPOrigin(value: unknown): value is string { + const url = typeof value === 'string' ? URL.parse(value) : null; + return (url?.protocol === 'http:' || url?.protocol === 'https:') && url.origin === value; +} + +function parseBackendOrigins(value: string): string[] { + let origins: unknown; + try { + origins = JSON.parse(value); + } catch { + origins = undefined; + } + if (!Array.isArray(origins) || !origins.every(isHTTPOrigin)) { + throw new Error(`DEV_CONNECT_ORIGINS must be a JSON array of HTTP(S) origins, got ${value}`); + } + return origins; +} + +let cached: {vars: DevelopmentVars; config: DevelopmentConfig} | undefined; + +/** Vars are fixed for the life of the Worker, so they are validated on first use rather than on every request. */ +function getDevelopmentConfig(vars: DevelopmentVars): DevelopmentConfig { + if (cached?.vars.DEV_UPSTREAM !== vars.DEV_UPSTREAM || cached.vars.DEV_CONNECT_ORIGINS !== vars.DEV_CONNECT_ORIGINS) { + const config = {upstream: parseUpstream(vars.DEV_UPSTREAM), backendOrigins: parseBackendOrigins(vars.DEV_CONNECT_ORIGINS)}; + cached = {vars: {DEV_UPSTREAM: vars.DEV_UPSTREAM, DEV_CONNECT_ORIGINS: vars.DEV_CONNECT_ORIGINS}, config}; + } + return cached.config; +} + +export default getDevelopmentConfig; +export type {DevelopmentConfig}; diff --git a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts index e0311f0132f5..d40cb52b9110 100644 --- a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts +++ b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts @@ -1,28 +1,10 @@ -/** Forward live requests without relying on Wrangler's implicit upstream host/port handling. */ -function getDevelopmentUpstream(env: DevelopmentEnv): URL { - const upstream = new URL(env.DEV_UPSTREAM); - if ( - upstream.protocol !== 'http:' || - upstream.hostname !== '127.0.0.1' || - !upstream.port || - upstream.pathname !== '/' || - upstream.search || - upstream.hash || - upstream.username || - upstream.password - ) { - throw new Error('DEV_UPSTREAM must be an HTTP loopback origin with an explicit port'); - } - return upstream; -} - /** API and attachment responses must never enter the SPA fallback or HTML nonce/cache pipeline. */ function isAPIRequest(pathname: string): boolean { return /^\/(?:api|staging|staging-secure|chat-attachments|receipts)(?:\/|$)/.test(pathname); } -async function fetchDevelopmentResponse(request: Request, env: DevelopmentEnv): Promise { - const upstream = getDevelopmentUpstream(env); +/** Forward live requests without relying on Wrangler's implicit upstream host/port handling. */ +async function fetchDevelopmentResponse(request: Request, upstream: URL): Promise { const publicURL = new URL(request.url); const url = new URL(request.url); url.protocol = upstream.protocol; diff --git a/web/cloudflare-worker/src/index.ts b/web/cloudflare-worker/src/index.ts index c5b0a0978107..a70211d07fab 100644 --- a/web/cloudflare-worker/src/index.ts +++ b/web/cloudflare-worker/src/index.ts @@ -1,6 +1,7 @@ import type {DevelopmentOrigins} from './csp'; import {getCSP, NONCE_PLACEHOLDER} from './csp'; +import getDevelopmentConfig from './developmentConfig'; import {fetchDevelopmentResponse, isAPIRequest} from './fetchDevelopmentResponse'; /** Content-hashed build output (`main-<16 hex>.bundle.js`, `main.<10 hex>.css`, ...), emitted at the root of `dist/`. */ @@ -197,11 +198,12 @@ export default { let served: ServedFile; let developmentOrigins: DevelopmentOrigins | undefined; if (isDevelopment) { - const assetResponse = await fetchDevelopmentResponse(request, env); + const {upstream, backendOrigins} = getDevelopmentConfig(env); + const assetResponse = await fetchDevelopmentResponse(request, upstream); if (assetResponse.status === 101 || isAPIRequest(url.pathname)) { return assetResponse; } - developmentOrigins = {hmrOrigin: url.origin.replace(/^https:/, 'wss:').replace(/^http:/, 'ws:'), backendOrigins: JSON.parse(env.DEV_CONNECT_ORIGINS) as string[]}; + developmentOrigins = {hmrOrigin: url.origin.replace(/^https:/, 'wss:').replace(/^http:/, 'ws:'), backendOrigins}; served = {response: assetResponse, isBrotli: false}; } else { served = await getAsset(env, request, url, isPrecompressed); diff --git a/web/cloudflare-worker/tests/developmentConfig.test.ts b/web/cloudflare-worker/tests/developmentConfig.test.ts new file mode 100644 index 000000000000..16ea94bc5535 --- /dev/null +++ b/web/cloudflare-worker/tests/developmentConfig.test.ts @@ -0,0 +1,37 @@ +/** The live Worker's launcher-provided vars should be validated once, with errors that name the bad variable. */ +import assert from 'node:assert/strict'; +import {describe, test} from 'node:test'; + +import getDevelopmentConfig from '../src/developmentConfig.ts'; + +const VALID_VARS = {DEV_UPSTREAM: 'http://127.0.0.1:8083', DEV_CONNECT_ORIGINS: '["https://www.expensify.com.dev","http://backend.test:8080"]'}; + +describe('live development Worker configuration', () => { + test('parses the upstream and backend origins', () => { + // Given vars from the launcher + // When the Worker reads them + const config = getDevelopmentConfig(VALID_VARS); + // Then it gets a loopback upstream and exact backend origins + assert.equal(config.upstream.href, 'http://127.0.0.1:8083/'); + assert.deepEqual(config.backendOrigins, ['https://www.expensify.com.dev', 'http://backend.test:8080']); + }); + + test('validates the vars once rather than on every request', () => { + // Given the same vars on every request + // When the Worker reads them repeatedly + // Then it reuses the validated configuration + assert.equal(getDevelopmentConfig({...VALID_VARS}), getDevelopmentConfig({...VALID_VARS})); + }); + + for (const DEV_UPSTREAM of ['not a url', 'https://127.0.0.1:8083', 'http://example.test:8083', 'http://127.0.0.1', 'http://127.0.0.1:8083/path']) { + test(`names DEV_UPSTREAM when it is not an HTTP loopback origin: ${DEV_UPSTREAM}`, () => { + assert.throws(() => getDevelopmentConfig({...VALID_VARS, DEV_UPSTREAM}), /DEV_UPSTREAM/); + }); + } + + for (const DEV_CONNECT_ORIGINS of ['not json', '"https://example.test"', '[1]', '["https://example.test/path"]', '["file:///etc/hosts"]', '["https://user:pass@example.test"]']) { + test(`names DEV_CONNECT_ORIGINS when it is not a JSON array of HTTP(S) origins: ${DEV_CONNECT_ORIGINS}`, () => { + assert.throws(() => getDevelopmentConfig({...VALID_VARS, DEV_CONNECT_ORIGINS}), /DEV_CONNECT_ORIGINS/); + }); + } +}); From 428600fa9cce17eb41fdc12d04fe799803d82a82 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:09:50 -0700 Subject: [PATCH 17/31] refactor(web): share the nonce placeholder with the Rsbuild config Rsbuild copied the Worker's placeholder as a literal, so renaming it would silently drop the nonce from dev HTML. --- config/rsbuild/rsbuild.config.ts | 6 +++++- web/cloudflare-worker/src/csp.ts | 3 +-- web/cloudflare-worker/src/noncePlaceholder.ts | 7 +++++++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 web/cloudflare-worker/src/noncePlaceholder.ts diff --git a/config/rsbuild/rsbuild.config.ts b/config/rsbuild/rsbuild.config.ts index 60bf8f600c8d..b5824dce2105 100644 --- a/config/rsbuild/rsbuild.config.ts +++ b/config/rsbuild/rsbuild.config.ts @@ -9,6 +9,10 @@ import {fileURLToPath} from 'url'; import type Environment from './types.ts'; +// The Worker is a separately installed package that the App does not depend on, so it can only be reached by path. +// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig +// eslint-disable-next-line import/no-relative-packages +import NONCE_PLACEHOLDER from '../../web/cloudflare-worker/src/noncePlaceholder.ts'; // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig import getCommonConfiguration from './rsbuild.common.ts'; @@ -63,7 +67,7 @@ export default defineConfig(async ({command}) => { assetPrefix: '/', lazyCompilation: false, }, - security: isWorkerEnabled ? {nonce: 'nonce-random-value'} : common.security, + security: isWorkerEnabled ? {nonce: NONCE_PLACEHOLDER} : common.security, source: { ...common.source, define: { diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index 26c03d4fc9df..d418c1ff00f4 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -1,5 +1,4 @@ -/** Placeholder in the CSP and in `web/index.html` script tags that the Worker replaces with a per-request nonce. */ -const NONCE_PLACEHOLDER = 'nonce-random-value'; +import NONCE_PLACEHOLDER from './noncePlaceholder'; type CSPOptions = { fontSrcHosts?: string[]; diff --git a/web/cloudflare-worker/src/noncePlaceholder.ts b/web/cloudflare-worker/src/noncePlaceholder.ts new file mode 100644 index 000000000000..ea071795dad3 --- /dev/null +++ b/web/cloudflare-worker/src/noncePlaceholder.ts @@ -0,0 +1,7 @@ +/** + * Placeholder in the CSP and in script tags (`web/index.html`, and Rsbuild's dev output) that the Worker replaces with a + * per-request nonce. Kept free of Worker types, so the Rsbuild config can import it. + */ +const NONCE_PLACEHOLDER = 'nonce-random-value'; + +export default NONCE_PLACEHOLDER; From 320f270357c7265fb3e5536fcb9f22c5ce0dc3a6 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:13:42 -0700 Subject: [PATCH 18/31] refactor(web): define the API proxy paths and port once The Worker regex, the Rsbuild pathFilter, the launcher, and the proxy each repeated the proxied paths or port 9000. A path added to only one would give API responses the SPA CSP, nonce, and 404 handling. --- config/rsbuild/rsbuild.config.ts | 5 +++- scripts/startWeb.ts | 5 +++- .../src/developmentAPIProxy.ts | 14 ++++++++++ .../src/fetchDevelopmentResponse.ts | 7 ++--- web/cloudflare-worker/src/index.ts | 3 ++- .../tests/developmentAPIProxy.test.ts | 27 +++++++++++++++++++ web/proxy.ts | 7 +++-- 7 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 web/cloudflare-worker/src/developmentAPIProxy.ts create mode 100644 web/cloudflare-worker/tests/developmentAPIProxy.test.ts diff --git a/config/rsbuild/rsbuild.config.ts b/config/rsbuild/rsbuild.config.ts index b5824dce2105..36d0e551e281 100644 --- a/config/rsbuild/rsbuild.config.ts +++ b/config/rsbuild/rsbuild.config.ts @@ -12,6 +12,9 @@ import type Environment from './types.ts'; // The Worker is a separately installed package that the App does not depend on, so it can only be reached by path. // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig // eslint-disable-next-line import/no-relative-packages +import {API_PROXY_PATHS, API_PROXY_PORT} from '../../web/cloudflare-worker/src/developmentAPIProxy.ts'; +// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig +// eslint-disable-next-line import/no-relative-packages import NONCE_PLACEHOLDER from '../../web/cloudflare-worker/src/noncePlaceholder.ts'; // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig import getCommonConfiguration from './rsbuild.common.ts'; @@ -56,7 +59,7 @@ export default defineConfig(async ({command}) => { const publicPort = process.env.WEB_PORT ? Number(process.env.WEB_PORT) : await portfinder.getPortPromise({port: BASE_PORT}); const port = isWorkerEnabled ? Number(process.env.WEB_INTERNAL_PORT) : publicPort; // Check if the USE_WEB_PROXY variable has been provided and rewrite any requests to the local proxy server - const proxy = process.env.USE_WEB_PROXY === 'false' ? undefined : [{pathFilter: ['/api', '/staging', '/staging-secure', '/chat-attachments', '/receipts'], target: 'http://[::1]:9000'}]; + const proxy = process.env.USE_WEB_PROXY === 'false' ? undefined : [{pathFilter: API_PROXY_PATHS, target: `http://[::1]:${API_PROXY_PORT}`}]; const commonRspackTool = common.tools?.rspack; diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index d3eef9689ea2..00277cdc2c24 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -2,6 +2,9 @@ import {$} from 'bun'; import concurrently from 'concurrently'; +// The Worker is a separately installed package that the App does not depend on, so it can only be reached by path. +// eslint-disable-next-line import/no-relative-packages +import {API_PROXY_PORT} from '../web/cloudflare-worker/src/developmentAPIProxy'; import waitForPublicWebServer from './utils/waitForPublicWebServer'; import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; @@ -10,7 +13,7 @@ async function startWeb() { const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); const {publicPort, internalPort} = getWebDevPorts(Bun.env, isWorkerEnabled); if (isProxyEnabled) { - assertWebPortAvailable(9000, '::'); + assertWebPortAvailable(API_PROXY_PORT, '::'); } for (const file of ['key.pem', 'certificate.pem']) { if (!(await Bun.file(`config/rsbuild/${file}`).exists())) { diff --git a/web/cloudflare-worker/src/developmentAPIProxy.ts b/web/cloudflare-worker/src/developmentAPIProxy.ts new file mode 100644 index 000000000000..03b081673d28 --- /dev/null +++ b/web/cloudflare-worker/src/developmentAPIProxy.ts @@ -0,0 +1,14 @@ +/** + * The local API proxy (`web/proxy.ts`) that Rsbuild forwards these paths to. The live Worker passes the same paths through + * untouched, so this stays free of Worker types for the Rsbuild config, launcher, and proxy to import. + */ +const API_PROXY_PATHS = ['/api', '/staging', '/staging-secure', '/chat-attachments', '/receipts']; + +const API_PROXY_PORT = 9000; + +/** API and attachment responses must never enter the SPA fallback or HTML nonce/cache pipeline. */ +function isAPIRequest(pathname: string): boolean { + return API_PROXY_PATHS.some((prefix) => pathname === prefix || pathname.startsWith(`${prefix}/`)); +} + +export {API_PROXY_PATHS, API_PROXY_PORT, isAPIRequest}; diff --git a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts index d40cb52b9110..12d61281f408 100644 --- a/web/cloudflare-worker/src/fetchDevelopmentResponse.ts +++ b/web/cloudflare-worker/src/fetchDevelopmentResponse.ts @@ -1,7 +1,4 @@ -/** API and attachment responses must never enter the SPA fallback or HTML nonce/cache pipeline. */ -function isAPIRequest(pathname: string): boolean { - return /^\/(?:api|staging|staging-secure|chat-attachments|receipts)(?:\/|$)/.test(pathname); -} +import {isAPIRequest} from './developmentAPIProxy'; /** Forward live requests without relying on Wrangler's implicit upstream host/port handling. */ async function fetchDevelopmentResponse(request: Request, upstream: URL): Promise { @@ -38,4 +35,4 @@ async function fetchDevelopmentResponse(request: Request, upstream: URL): Promis return response; } -export {fetchDevelopmentResponse, isAPIRequest}; +export default fetchDevelopmentResponse; diff --git a/web/cloudflare-worker/src/index.ts b/web/cloudflare-worker/src/index.ts index a70211d07fab..f718917bf461 100644 --- a/web/cloudflare-worker/src/index.ts +++ b/web/cloudflare-worker/src/index.ts @@ -1,8 +1,9 @@ import type {DevelopmentOrigins} from './csp'; import {getCSP, NONCE_PLACEHOLDER} from './csp'; +import {isAPIRequest} from './developmentAPIProxy'; import getDevelopmentConfig from './developmentConfig'; -import {fetchDevelopmentResponse, isAPIRequest} from './fetchDevelopmentResponse'; +import fetchDevelopmentResponse from './fetchDevelopmentResponse'; /** Content-hashed build output (`main-<16 hex>.bundle.js`, `main.<10 hex>.css`, ...), emitted at the root of `dist/`. */ const HASHED_ASSET_PATH = /^\/[^/]+(?:-[0-9a-f]{16}\.bundle\.js|\.[0-9a-f]{10}\.[a-z0-9]+)$/; diff --git a/web/cloudflare-worker/tests/developmentAPIProxy.test.ts b/web/cloudflare-worker/tests/developmentAPIProxy.test.ts new file mode 100644 index 000000000000..74938715ec17 --- /dev/null +++ b/web/cloudflare-worker/tests/developmentAPIProxy.test.ts @@ -0,0 +1,27 @@ +/** The Worker must pass through exactly the paths Rsbuild proxies to the API, or API responses get SPA handling. */ +import assert from 'node:assert/strict'; +import {describe, test} from 'node:test'; + +import {API_PROXY_PATHS, isAPIRequest} from '../src/developmentAPIProxy.ts'; + +describe('live development API passthrough', () => { + test('passes through every path Rsbuild proxies to the API', () => { + // Given the shared list Rsbuild also proxies + for (const prefix of API_PROXY_PATHS) { + // When the Worker classifies the prefix and paths beneath it + // Then they bypass the SPA fallback and HTML handling + assert.ok(isAPIRequest(prefix), prefix); + assert.ok(isAPIRequest(`${prefix}/Command?x=1`), prefix); + } + }); + + test('does not treat app routes that only share a prefix as API requests', () => { + // Given app paths that start with the same letters + // When the Worker classifies them + // Then they still get the app shell + for (const prefix of API_PROXY_PATHS) { + assert.equal(isAPIRequest(`${prefix}x`), false, prefix); + } + assert.equal(isAPIRequest('/settings'), false); + }); +}); diff --git a/web/proxy.ts b/web/proxy.ts index 101a859abb7f..b0133af7da5e 100644 --- a/web/proxy.ts +++ b/web/proxy.ts @@ -5,6 +5,9 @@ import http from 'http'; import https from 'https'; import proxyConfig from '../config/proxyConfig'; +// The Worker is a separately installed package that the App does not depend on, so it can only be reached by path. +// eslint-disable-next-line import/no-relative-packages +import {API_PROXY_PORT} from './cloudflare-worker/src/developmentAPIProxy'; dotenv.config(); @@ -87,7 +90,7 @@ const server = http.createServer((request: IncomingMessage, response: ServerResp }); }); -server.listen(9000, () => { +server.listen(API_PROXY_PORT, () => { /* eslint-disable-next-line no-console */ - console.log('Proxy server listening at http://localhost:9000'); + console.log(`Proxy server listening at http://localhost:${API_PROXY_PORT}`); }); From 95ac108bcd9332d67af6c31df76c74e149e13b0e Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:16:06 -0700 Subject: [PATCH 19/31] refactor(web): define the local backend env keys once The launcher's CSP backends and Rsbuild's forwarded app settings listed the backend keys separately, so a key added to one could leave the app and CSP out of sync. --- config/rsbuild/developmentEnvKeys.ts | 7 +++++++ config/rsbuild/rsbuild.common.ts | 20 +++----------------- scripts/utils/webDevConfig.ts | 4 ++-- tests/tooling/developmentEnvKeys.test.ts | 24 ++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 config/rsbuild/developmentEnvKeys.ts create mode 100644 tests/tooling/developmentEnvKeys.test.ts diff --git a/config/rsbuild/developmentEnvKeys.ts b/config/rsbuild/developmentEnvKeys.ts new file mode 100644 index 000000000000..33b1fc992681 --- /dev/null +++ b/config/rsbuild/developmentEnvKeys.ts @@ -0,0 +1,7 @@ +/** Local backends from `.env`, which the app calls and the launcher adds to the live Worker's CSP. */ +const LOCAL_BACKEND_URL_KEYS = ['EXPENSIFY_URL', 'SECURE_EXPENSIFY_URL', 'STAGING_EXPENSIFY_URL', 'STAGING_SECURE_EXPENSIFY_URL', 'NGROK_URL', 'SECURE_NGROK_URL']; + +/** Public development settings that shell variables can override in the app config. Never forward arbitrary shell variables, which may contain secrets. */ +const FORWARDED_DEVELOPMENT_ENV_KEYS = [...LOCAL_BACKEND_URL_KEYS, 'USE_WEB_PROXY', 'USE_NGROK', 'NEW_EXPENSIFY_URL', 'SKIP_ONBOARDING', 'PUSHER_APP_KEY', 'PUSHER_DEV_SUFFIX']; + +export {FORWARDED_DEVELOPMENT_ENV_KEYS, LOCAL_BACKEND_URL_KEYS}; diff --git a/config/rsbuild/rsbuild.common.ts b/config/rsbuild/rsbuild.common.ts index 71711eda1118..274479e5a52e 100644 --- a/config/rsbuild/rsbuild.common.ts +++ b/config/rsbuild/rsbuild.common.ts @@ -26,6 +26,8 @@ import BrotliCompressionPlugin from './BrotliCompressionPlugin.ts'; // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig import CustomVersionFilePlugin from './CustomVersionFilePlugin.ts'; // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig +import {FORWARDED_DEVELOPMENT_ENV_KEYS} from './developmentEnvKeys.ts'; +// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig import ModuleInitTimingPlugin from './ModuleInitTimingPlugin.ts'; // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig import RspackPreloadPlugin from './RspackPreloadPlugin.ts'; @@ -134,24 +136,8 @@ function mapEnvironmentToLogoSuffix(environmentFile: string): string { function getDefineValues(file: string): DefinePluginOptions { const isDevelopmentFile = file === '.env' || file === '.env.development'; const parsedConfig = dotenv.config({path: file}).parsed; - // Forward only public development settings, never arbitrary shell variables (which may contain secrets). const developmentOverrides = isDevelopmentFile - ? Object.fromEntries( - [ - 'USE_WEB_PROXY', - 'USE_NGROK', - 'EXPENSIFY_URL', - 'SECURE_EXPENSIFY_URL', - 'NEW_EXPENSIFY_URL', - 'STAGING_EXPENSIFY_URL', - 'STAGING_SECURE_EXPENSIFY_URL', - 'NGROK_URL', - 'SECURE_NGROK_URL', - 'SKIP_ONBOARDING', - 'PUSHER_APP_KEY', - 'PUSHER_DEV_SUFFIX', - ].flatMap((key) => (process.env[key] === undefined ? [] : [[key, process.env[key]]])), - ) + ? Object.fromEntries(FORWARDED_DEVELOPMENT_ENV_KEYS.flatMap((key: string) => (process.env[key] === undefined ? [] : [[key, process.env[key]]]))) : {}; /* eslint-disable @typescript-eslint/naming-convention */ return { diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index ea7b7adc1ffa..9899cea9460f 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -1,5 +1,5 @@ /** Resolve one set of ports and flags for every process in a local web session. */ -const LOCAL_BACKEND_KEYS = ['EXPENSIFY_URL', 'SECURE_EXPENSIFY_URL', 'STAGING_EXPENSIFY_URL', 'STAGING_SECURE_EXPENSIFY_URL', 'NGROK_URL', 'SECURE_NGROK_URL']; +import {LOCAL_BACKEND_URL_KEYS} from '../../config/rsbuild/developmentEnvKeys'; function getWebDevFlags(env: Record) { return {isWorkerEnabled: env.USE_CLOUDFLARE_WORKER !== 'false', isProxyEnabled: env.USE_WEB_PROXY !== 'false', shouldOpenBrowser: env.WEB_OPEN !== 'false'}; @@ -7,7 +7,7 @@ function getWebDevFlags(env: Record) { function getDevelopmentConnectOrigins(env: Record): string[] { const origins = new Set(); - for (const key of LOCAL_BACKEND_KEYS) { + for (const key of LOCAL_BACKEND_URL_KEYS) { const value = env[key]; if (!value) { continue; diff --git a/tests/tooling/developmentEnvKeys.test.ts b/tests/tooling/developmentEnvKeys.test.ts new file mode 100644 index 000000000000..b3b2cbc99543 --- /dev/null +++ b/tests/tooling/developmentEnvKeys.test.ts @@ -0,0 +1,24 @@ +/** The app and the Worker CSP must see the same local backends, so the backend keys are defined once. */ +import {describe, expect, test} from 'bun:test'; + +import {FORWARDED_DEVELOPMENT_ENV_KEYS, LOCAL_BACKEND_URL_KEYS} from '../../config/rsbuild/developmentEnvKeys'; +import {getDevelopmentConnectOrigins} from '../../scripts/utils/webDevConfig'; + +describe('development env keys', () => { + test('forwards every backend the CSP allows to the app', () => { + // Given the backend keys the launcher adds to the Worker CSP + // When Rsbuild forwards development settings to the app + // Then the app sees every one of those backends + for (const key of LOCAL_BACKEND_URL_KEYS) { + expect(FORWARDED_DEVELOPMENT_ENV_KEYS).toContain(key); + } + }); + + test('allows each backend key in the CSP', () => { + // Given a distinct origin for every backend key + const env = Object.fromEntries(LOCAL_BACKEND_URL_KEYS.map((key, index) => [key, `https://backend-${index}.test/`])); + // When the launcher resolves the CSP origins + // Then none is missing + expect(getDevelopmentConnectOrigins(env)).toHaveLength(LOCAL_BACKEND_URL_KEYS.length); + }); +}); From 0cd9db19fc639070c741494860bd9070e8d7157e Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:18:13 -0700 Subject: [PATCH 20/31] refactor(web): share one readiness loop between the launcher and Rsbuild gate Both had their own polling, cancellation, and slow-start handling. waitForWebServer now takes a probe, and each caller supplies its own. --- .github/workflows/cloudflareWorkerTests.yml | 1 - scripts/startWeb.ts | 6 +- scripts/utils/waitForPublicWebServer.ts | 36 ------ scripts/waitForWebServer.ts | 59 +++++++--- tests/tooling/waitForPublicWebServer.test.ts | 45 -------- tests/tooling/waitForWebServer.test.ts | 110 +++++++++++-------- 6 files changed, 109 insertions(+), 148 deletions(-) delete mode 100644 scripts/utils/waitForPublicWebServer.ts delete mode 100644 tests/tooling/waitForPublicWebServer.test.ts diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index 97dd314d7831..a09a05f0ce6d 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -10,7 +10,6 @@ on: - 'scripts/waitForWebServer.ts' - 'scripts/utils/webDevConfig.ts' - 'scripts/utils/checkWebServer.ts' - - 'scripts/utils/waitForPublicWebServer.ts' - 'scripts/postInstall.sh' - 'scripts/installCloudflareWorker.ts' - 'config/rsbuild/**' diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 00277cdc2c24..ab7bafbe04b6 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -5,8 +5,9 @@ import concurrently from 'concurrently'; // The Worker is a separately installed package that the App does not depend on, so it can only be reached by path. // eslint-disable-next-line import/no-relative-packages import {API_PROXY_PORT} from '../web/cloudflare-worker/src/developmentAPIProxy'; -import waitForPublicWebServer from './utils/waitForPublicWebServer'; +import checkWebServer from './utils/checkWebServer'; import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; +import waitForWebServer from './waitForWebServer'; async function startWeb() { // Bun loads .env automatically, preserving explicit shell overrides. @@ -64,7 +65,8 @@ async function startWeb() { process.once('SIGTERM', stop); // Verify the browser-facing listener and certificate, not just the internal compiler's listener. const readiness = (async () => { - const isReady = await waitForPublicWebServer(origin, isWorkerEnabled, controller.signal, { + const isReady = await waitForWebServer((signal) => checkWebServer(origin, isWorkerEnabled, signal), controller.signal, { + slowAfter: 180_000, onSlow: () => console.warn(`Still waiting for ${origin}/. Check the dev server output. If it compiled, check that its certificate is trusted (npm run setup-https).`), }); if (!isReady) { diff --git a/scripts/utils/waitForPublicWebServer.ts b/scripts/utils/waitForPublicWebServer.ts deleted file mode 100644 index f9cdf081a54a..000000000000 --- a/scripts/utils/waitForPublicWebServer.ts +++ /dev/null @@ -1,36 +0,0 @@ -import checkWebServer from './checkWebServer'; - -type WaitForPublicWebServerOptions = { - interval?: number; - - /** How long startup can take before `onSlow` reports it. */ - slowAfter?: number; - onSlow?: () => void; -}; - -/** - * Wait until the browser-facing listener serves the app. A slow first compile is reported, not fatal: concurrently - * already stops the session if a required service exits. Resolves false if the session stops first. - */ -async function waitForPublicWebServer( - origin: string, - isWorkerEnabled: boolean, - signal: AbortSignal, - {interval = 250, slowAfter = 180_000, onSlow = () => {}}: WaitForPublicWebServerOptions = {}, -): Promise { - const slowAt = Date.now() + slowAfter; - let hasReportedSlow = false; - while (!signal.aborted) { - if (await checkWebServer(origin, isWorkerEnabled, signal)) { - return !signal.aborted; - } - if (!hasReportedSlow && Date.now() > slowAt) { - hasReportedSlow = true; - onSlow(); - } - await Bun.sleep(interval); - } - return false; -} - -export default waitForPublicWebServer; diff --git a/scripts/waitForWebServer.ts b/scripts/waitForWebServer.ts index 51b75d95d441..60d1bed0f6f9 100644 --- a/scripts/waitForWebServer.ts +++ b/scripts/waitForWebServer.ts @@ -1,26 +1,47 @@ +type WaitForWebServerOptions = { + interval?: number; + + /** How long startup can take before `onSlow` reports it. */ + slowAfter?: number; + onSlow?: () => void; +}; + /** - * Gate dependent dev processes on an HTTP response, not a listening socket or a compiler log message. There is no default - * deadline, because a failure here stops the session, and concurrently already stops this gate if Rsbuild exits. + * Gate dev processes on a usable HTTP response, not a listening socket or a compiler log message. Resolves false if the + * session stops first. There is no deadline: concurrently already stops the session if a required service exits, so a + * slow first compile is only reported. */ -async function waitForWebServer(origin: string, signal: AbortSignal, {timeout = Infinity, interval = 250} = {}): Promise { - const deadline = Date.now() + timeout; - while (!signal.aborted && Date.now() < deadline) { +async function waitForWebServer( + probe: (signal: AbortSignal) => Promise, + signal: AbortSignal, + {interval = 250, slowAfter = Infinity, onSlow = () => {}}: WaitForWebServerOptions = {}, +): Promise { + const slowAt = Date.now() + slowAfter; + let hasReportedSlow = false; + while (!signal.aborted) { try { - const response = await fetch(origin, {signal: AbortSignal.any([signal, AbortSignal.timeout(Math.min(2000, Math.max(1, deadline - Date.now())))]), redirect: 'manual'}); - await response.body?.cancel(); - if (response.status === 200) { - signal.throwIfAborted(); - return; + if (await probe(signal)) { + return !signal.aborted; } } catch { - // The upstream may not have a listener yet, or may still be compiling its first response. + // The server may not have a listener yet, or may still be compiling its first response. + } + if (!hasReportedSlow && Date.now() > slowAt) { + hasReportedSlow = true; + onSlow(); } if (!signal.aborted) { - await Bun.sleep(Math.min(interval, Math.max(0, deadline - Date.now()))); + await Bun.sleep(interval); } } - signal.throwIfAborted(); - throw new Error(`Timed out waiting for Rsbuild at ${origin}. Check its output above.`); + return false; +} + +/** Whether the private Rsbuild listener can serve the Worker yet. */ +async function isRsbuildReady(origin: string, signal: AbortSignal): Promise { + const response = await fetch(origin, {signal: AbortSignal.any([signal, AbortSignal.timeout(2000)]), redirect: 'manual'}); + await response.body?.cancel(); + return response.status === 200; } if (import.meta.main) { @@ -33,11 +54,12 @@ if (import.meta.main) { if (!origin) { throw new Error('Missing Rsbuild readiness URL'); } - await waitForWebServer(origin, controller.signal); - } catch (error) { - if (!controller.signal.aborted) { - console.error(error instanceof Error ? error.message : error); + // A non-zero exit keeps the shell from starting the Worker. + if (!(await waitForWebServer((signal) => isRsbuildReady(origin, signal), controller.signal))) { + process.exitCode = 1; } + } catch (error) { + console.error(error instanceof Error ? error.message : error); process.exitCode = 1; } finally { process.removeListener('SIGINT', stop); @@ -46,3 +68,4 @@ if (import.meta.main) { } export default waitForWebServer; +export {isRsbuildReady}; diff --git a/tests/tooling/waitForPublicWebServer.test.ts b/tests/tooling/waitForPublicWebServer.test.ts deleted file mode 100644 index 5259d095903f..000000000000 --- a/tests/tooling/waitForPublicWebServer.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -/** The launcher should wait for the public listener as long as the session runs, without stopping it on a slow start. */ -import {afterEach, describe, expect, mock, setSystemTime, spyOn, test} from 'bun:test'; - -import waitForPublicWebServer from '../../scripts/utils/waitForPublicWebServer'; - -function mockFetch(respond: () => Response) { - spyOn(globalThis, 'fetch').mockImplementation(Object.assign(async () => respond(), {preconnect: fetch.preconnect})); -} - -describe('public web server startup', () => { - afterEach(() => { - setSystemTime(); - mock.restore(); - }); - - test('keeps waiting past a slow first compile and reports the delay once', async () => { - // Given a public listener that only becomes ready long after startup - let requests = 0; - mockFetch(() => { - if (++requests === 2) { - setSystemTime(new Date(Date.now() + 10 * 60_000)); - } - return new Response('app shell', {status: requests < 5 ? 503 : 200}); - }); - const onSlow = mock(() => {}); - // When the launcher waits for it - const isReady = await waitForPublicWebServer('https://dev.new.expensify.com:8082', false, new AbortController().signal, {interval: 1, onSlow}); - // Then the session is not stopped, and the delay is reported once rather than on every retry - expect(isReady).toBe(true); - expect(requests).toBe(5); - expect(onSlow).toHaveBeenCalledTimes(1); - }); - - test('stops waiting without readiness when the session stops', async () => { - // Given a listener that never becomes ready - const controller = new AbortController(); - mockFetch(() => { - controller.abort(); - return new Response(null, {status: 503}); - }); - // When the session is stopped - // Then waiting ends without reporting the server as ready - expect(await waitForPublicWebServer('https://dev.new.expensify.com:8082', false, controller.signal, {interval: 1})).toBe(false); - }); -}); diff --git a/tests/tooling/waitForWebServer.test.ts b/tests/tooling/waitForWebServer.test.ts index 550be20b9d52..5da01f73bb6a 100644 --- a/tests/tooling/waitForWebServer.test.ts +++ b/tests/tooling/waitForWebServer.test.ts @@ -1,54 +1,72 @@ -/** Startup dependencies should wait for usable HTTP responses and stop cleanly on failure or cancellation. */ -import {describe, expect, setSystemTime, test} from 'bun:test'; +/** Startup dependencies should wait for usable HTTP responses for as long as the session runs, and stop cleanly on cancellation. */ +import {afterEach, describe, expect, mock, setSystemTime, test} from 'bun:test'; -import waitForWebServer from '../../scripts/waitForWebServer'; +import waitForWebServer, {isRsbuildReady} from '../../scripts/waitForWebServer'; -describe('Rsbuild startup gate', () => { - test('waits through unavailable responses before allowing the dependent command to start', async () => { - // Given an upstream whose first responses are not ready - let requests = 0; - const server = Bun.serve({port: 0, hostname: '127.0.0.1', fetch: () => new Response('compiling', {status: ++requests < 3 ? 503 : 200})}); - try { - // When the gate waits for the server - await waitForWebServer(server.url.href, new AbortController().signal, {timeout: 1000, interval: 5}); - // Then it returns only after a successful response - expect(requests).toBe(3); - } finally { - await server.stop(true); - } - }); +describe('dev server readiness', () => { + afterEach(() => setSystemTime()); - test('waits through a slow first compile by default rather than stopping the session', async () => { - // Given an upstream whose first compile outlasts any fixed startup deadline - let requests = 0; - const server = Bun.serve({ - port: 0, - hostname: '127.0.0.1', - fetch: () => { - if (++requests === 2) { - setSystemTime(new Date(Date.now() + 10 * 60_000)); - } - return new Response('compiling', {status: requests < 4 ? 503 : 200}); - }, + test('retries failed and throwing probes until the server is ready', async () => { + // Given a server that is not listening, then compiling, then ready + const results = [new TypeError('Connection refused'), false, true]; + const probe = mock(async () => { + const result = results.shift(); + if (result instanceof Error) { + throw result; + } + return result ?? true; }); - try { - // When the gate waits with its default settings - await waitForWebServer(server.url.href, new AbortController().signal, {interval: 5}); - // Then it starts the Worker once Rsbuild responds - expect(requests).toBe(4); - } finally { - setSystemTime(); - await server.stop(true); - } + // When the launcher waits for it + // Then it is ready only after the successful probe + expect(await waitForWebServer(probe, new AbortController().signal, {interval: 1})).toBe(true); + expect(probe).toHaveBeenCalledTimes(3); + }); + + test('waits through a slow first compile, reporting the delay once rather than stopping the session', async () => { + // Given a server whose first compile outlasts any fixed startup deadline + let probes = 0; + const probe = async () => { + if (++probes === 2) { + setSystemTime(new Date(Date.now() + 10 * 60_000)); + } + return probes >= 5; + }; + const onSlow = mock(() => {}); + // When the launcher waits for it + const isReady = await waitForWebServer(probe, new AbortController().signal, {interval: 1, slowAfter: 180_000, onSlow}); + // Then it keeps waiting until the server is ready, and reports the delay once + expect(isReady).toBe(true); + expect(probes).toBe(5); + expect(onSlow).toHaveBeenCalledTimes(1); }); - test('reports compilation/startup timeouts instead of starting the Worker', async () => { - // Given an upstream that never becomes ready - const server = Bun.serve({port: 0, hostname: '127.0.0.1', fetch: () => new Response(null, {status: 503})}); + test('stops without readiness when the session stops', async () => { + // Given a server that never becomes ready + const controller = new AbortController(); + const probe = async () => { + controller.abort(); + return false; + }; + // When the session is stopped + // Then waiting ends without starting dependent processes + expect(await waitForWebServer(probe, controller.signal, {interval: 1})).toBe(false); + }); +}); + +describe('Rsbuild startup gate', () => { + test('requires a successful response, not a listening socket', async () => { + // Given Rsbuild compiling, redirecting, then serving the app + const statuses = [503, 302, 200]; + const server = Bun.serve({port: 0, hostname: '127.0.0.1', fetch: () => new Response(null, {status: statuses.shift() ?? 200, headers: {Location: '/'}})}); try { - // When the startup deadline expires - // Then the gate rejects, so the shell never starts Wrangler - await expect(waitForWebServer(server.url.href, new AbortController().signal, {timeout: 50, interval: 5})).rejects.toThrow('Timed out waiting for Rsbuild'); + // When the gate probes it + const signal = new AbortController().signal; + // Then only the 200 starts the Worker + expect([await isRsbuildReady(server.url.href, signal), await isRsbuildReady(server.url.href, signal), await isRsbuildReady(server.url.href, signal)]).toEqual([ + false, + false, + true, + ]); } finally { await server.stop(true); } @@ -68,8 +86,8 @@ describe('Rsbuild startup gate', () => { }); try { // When the session is cancelled - // Then readiness rejects promptly rather than starting a dependent process - await expect(waitForWebServer(server.url.href, controller.signal)).rejects.toThrow(); + // Then readiness ends promptly rather than starting a dependent process + expect(await waitForWebServer((signal) => isRsbuildReady(server.url.href, signal), controller.signal)).toBe(false); } finally { // Bun cannot stop a server while a handler is still pending. sendResponse(new Response('ready')); From ed8a288c46ec6d43b089a75319f1224783740344 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:19:07 -0700 Subject: [PATCH 21/31] refactor(web): keep getWebDevPorts free of port probes It bound real sockets despite its name, so parsing tests needed live listeners. The launcher now checks availability itself. --- scripts/startWeb.ts | 5 ++++- scripts/utils/webDevConfig.ts | 11 +++++------ tests/tooling/webDevConfig.test.ts | 13 +++++++------ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index ab7bafbe04b6..529453e2ae31 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -12,7 +12,10 @@ import waitForWebServer from './waitForWebServer'; async function startWeb() { // Bun loads .env automatically, preserving explicit shell overrides. const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); - const {publicPort, internalPort} = getWebDevPorts(Bun.env, isWorkerEnabled); + const {publicPort, internalPort, ports} = getWebDevPorts(Bun.env, isWorkerEnabled); + for (const port of ports) { + assertWebPortAvailable(port); + } if (isProxyEnabled) { assertWebPortAvailable(API_PROXY_PORT, '::'); } diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index 9899cea9460f..05a5f9123b05 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -39,19 +39,18 @@ function assertWebPortAvailable(port: number, host = '127.0.0.1'): void { } } +/** Parse and validate the session's ports. Use `assertWebPortAvailable` to check that they are free. */ function getWebDevPorts(env: Record, isWorkerEnabled: boolean) { const publicPort = Number(env.WEB_PORT ?? 8082); const internalPort = Number(env.WEB_INTERNAL_PORT ?? publicPort + 1); if (isWorkerEnabled && publicPort === internalPort) { throw new Error('WEB_PORT and WEB_INTERNAL_PORT must differ'); } - for (const port of isWorkerEnabled ? [publicPort, internalPort] : [publicPort]) { - if (!Number.isInteger(port) || port < 1024 || port > 65535) { - throw new Error('WEB_PORT and WEB_INTERNAL_PORT must be integers between 1024 and 65535'); - } - assertWebPortAvailable(port); + const ports = isWorkerEnabled ? [publicPort, internalPort] : [publicPort]; + if (ports.some((port) => !Number.isInteger(port) || port < 1024 || port > 65535)) { + throw new Error('WEB_PORT and WEB_INTERNAL_PORT must be integers between 1024 and 65535'); } - return {publicPort, internalPort}; + return {publicPort, internalPort, ports}; } export {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts}; diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts index 3f94eeeba143..723eb6db704c 100644 --- a/tests/tooling/webDevConfig.test.ts +++ b/tests/tooling/webDevConfig.test.ts @@ -1,7 +1,7 @@ /** Keep local Worker/API opt-outs and coordinated port selection independent. */ import {describe, expect, test} from 'bun:test'; -import {getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from '../../scripts/utils/webDevConfig'; +import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from '../../scripts/utils/webDevConfig'; describe('web development configuration', () => { test('defaults both services on and allows independent opt-outs', () => { @@ -47,9 +47,10 @@ describe('web development configuration', () => { }, }); try { - // When this session tries to use it - // Then no child is started against that unrelated listener - expect(() => getWebDevPorts({WEB_PORT: String(listener.port)}, false)).toThrow('in use'); + // When this session resolves and checks its ports + // Then resolving them stays side-effect free, and the availability check stops startup before any child starts + expect(getWebDevPorts({WEB_PORT: String(listener.port)}, false).publicPort).toBe(listener.port); + expect(() => assertWebPortAvailable(listener.port)).toThrow('in use'); } finally { listener.stop(true); } @@ -68,8 +69,8 @@ describe('web development configuration', () => { }); const port = initial.port; initial.stop(true); - // When a session resolves and probes that port - expect(getWebDevPorts({WEB_PORT: String(port)}, false).publicPort).toBe(port); + // When a session probes that port + assertWebPortAvailable(port); // Then its actual server can bind the port immediately const listener = Bun.listen({ hostname: '127.0.0.1', From 222c691df0683b3a23e128635ea42e2e27431e44 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 14:22:43 -0700 Subject: [PATCH 22/31] fix(web): pass launcher settings to Rsbuild explicitly The launcher wrote its settings into its own env for children to inherit, and Rsbuild treated any WEB_PORT, including one from .env, as the launcher running. Pass them through each command's env with an explicit WEB_LAUNCHER flag. --- config/rsbuild/getDevServerSettings.ts | 29 +++++++ config/rsbuild/rsbuild.config.ts | 12 ++- scripts/startWeb.ts | 88 ++++++++++++++-------- tests/tooling/getDevServerSettings.test.ts | 37 +++++++++ tests/tooling/startWeb.test.ts | 41 ++++++++++ 5 files changed, 168 insertions(+), 39 deletions(-) create mode 100644 config/rsbuild/getDevServerSettings.ts create mode 100644 tests/tooling/getDevServerSettings.test.ts create mode 100644 tests/tooling/startWeb.test.ts diff --git a/config/rsbuild/getDevServerSettings.ts b/config/rsbuild/getDevServerSettings.ts new file mode 100644 index 000000000000..eaf04424030d --- /dev/null +++ b/config/rsbuild/getDevServerSettings.ts @@ -0,0 +1,29 @@ +type DevServerSettings = { + isWorkerEnabled: boolean; + + /** The browser-facing port, which links built by the app use. */ + publicPort: number; + + /** The port Rsbuild binds: private behind the Worker, otherwise the public port. */ + port: number; + strictPort: boolean; + open: string | false; +}; + +/** + * `npm run web` owns the ports and opening the browser, and says so with WEB_LAUNCHER. Without it, Rsbuild runs on its + * own, so launcher settings that `.env` may contain do not apply. + */ +async function getDevServerSettings(env: Record, findPort: () => Promise): Promise { + if (env.WEB_LAUNCHER !== 'true') { + const port = await findPort(); + // Rsbuild's --open flag defaults to the first entry's route, e.g. "/main" for our "main" entry key, which 404s + // since the app has no route by that name. Open the actual app root instead. + return {isWorkerEnabled: false, publicPort: port, port, strictPort: false, open: '/'}; + } + const isWorkerEnabled = env.USE_CLOUDFLARE_WORKER === 'true'; + const publicPort = Number(env.WEB_PORT); + return {isWorkerEnabled, publicPort, port: isWorkerEnabled ? Number(env.WEB_INTERNAL_PORT) : publicPort, strictPort: true, open: false}; +} + +export default getDevServerSettings; diff --git a/config/rsbuild/rsbuild.config.ts b/config/rsbuild/rsbuild.config.ts index 36d0e551e281..e766d5a4e98f 100644 --- a/config/rsbuild/rsbuild.config.ts +++ b/config/rsbuild/rsbuild.config.ts @@ -17,6 +17,8 @@ import {API_PROXY_PATHS, API_PROXY_PORT} from '../../web/cloudflare-worker/src/d // eslint-disable-next-line import/no-relative-packages import NONCE_PLACEHOLDER from '../../web/cloudflare-worker/src/noncePlaceholder.ts'; // @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig +import getDevServerSettings from './getDevServerSettings.ts'; +// @ts-expect-error -- Can't use .ts extensions without allowImportingTsExtensions in tsconfig import getCommonConfiguration from './rsbuild.common.ts'; const filename = fileURLToPath(import.meta.url); @@ -55,9 +57,7 @@ export default defineConfig(async ({command}) => { }; } - const isWorkerEnabled = process.env.USE_CLOUDFLARE_WORKER === 'true'; - const publicPort = process.env.WEB_PORT ? Number(process.env.WEB_PORT) : await portfinder.getPortPromise({port: BASE_PORT}); - const port = isWorkerEnabled ? Number(process.env.WEB_INTERNAL_PORT) : publicPort; + const {isWorkerEnabled, publicPort, port, strictPort, open} = await getDevServerSettings(process.env, () => portfinder.getPortPromise({port: BASE_PORT})); // Check if the USE_WEB_PROXY variable has been provided and rewrite any requests to the local proxy server const proxy = process.env.USE_WEB_PROXY === 'false' ? undefined : [{pathFilter: API_PROXY_PATHS, target: `http://[::1]:${API_PROXY_PORT}`}]; @@ -83,11 +83,9 @@ export default defineConfig(async ({command}) => { server: { proxy, port, - strictPort: !!process.env.WEB_PORT, + strictPort, host: isWorkerEnabled ? '127.0.0.1' : 'dev.new.expensify.com', - // Rsbuild's --open flag defaults to the first entry's route, e.g. "/main" for our "main" entry key, - // which 404s since the app has no route by that name. Open the actual app root instead. - open: process.env.WEB_PORT ? false : '/', + open, https: isWorkerEnabled ? undefined : { diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 529453e2ae31..625460118172 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -9,35 +9,33 @@ import checkWebServer from './utils/checkWebServer'; import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; import waitForWebServer from './waitForWebServer'; -async function startWeb() { - // Bun loads .env automatically, preserving explicit shell overrides. - const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); - const {publicPort, internalPort, ports} = getWebDevPorts(Bun.env, isWorkerEnabled); - for (const port of ports) { - assertWebPortAvailable(port); - } - if (isProxyEnabled) { - assertWebPortAvailable(API_PROXY_PORT, '::'); - } - for (const file of ['key.pem', 'certificate.pem']) { - if (!(await Bun.file(`config/rsbuild/${file}`).exists())) { - throw new Error('Missing local HTTPS certificate. Run npm run setup-https first.'); - } - } - if (isWorkerEnabled && !(await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists())) { - throw new Error('Missing Worker dependencies. Run npm install to finish App setup.'); - } - const origin = `https://dev.new.expensify.com:${publicPort}`; - Bun.env.WEB_PORT = String(publicPort); - Bun.env.WEB_INTERNAL_PORT = String(internalPort); - Bun.env.USE_CLOUDFLARE_WORKER = String(isWorkerEnabled); - Bun.env.USE_WEB_PROXY = String(isProxyEnabled); - const root = process.cwd(); +type WebCommandSettings = { + root: string; + isWorkerEnabled: boolean; + isProxyEnabled: boolean; + publicPort: number; + internalPort: number; + connectOrigins: string[]; +}; + +/** Each child gets the settings it needs in its own env, so the launcher's environment stays as the user set it. */ +function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins}: WebCommandSettings) { const workerDir = `${root}/web/cloudflare-worker`; // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. - const commands = [ - ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy'}] : []), - {name: 'rsbuild', command: 'node node_modules/@rsbuild/core/bin/rsbuild.js dev --config config/rsbuild/rsbuild.config.ts'}, + return [ + ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy', env: {USE_WEB_PROXY: 'true'}}] : []), + { + name: 'rsbuild', + command: 'node node_modules/@rsbuild/core/bin/rsbuild.js dev --config config/rsbuild/rsbuild.config.ts', + // WEB_LAUNCHER tells Rsbuild these settings came from this launcher, not from a user's .env. + env: { + WEB_LAUNCHER: 'true', + WEB_PORT: String(publicPort), + WEB_INTERNAL_PORT: String(internalPort), + USE_CLOUDFLARE_WORKER: String(isWorkerEnabled), + USE_WEB_PROXY: String(isProxyEnabled), + }, + }, ...(isWorkerEnabled ? [ { @@ -50,12 +48,34 @@ async function startWeb() { `--https-key-path ${$.escape(`${root}/config/rsbuild/key.pem`)}`, `--https-cert-path ${$.escape(`${root}/config/rsbuild/certificate.pem`)}`, `--var ${$.escape(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, - `--var ${$.escape(`DEV_CONNECT_ORIGINS:${JSON.stringify(getDevelopmentConnectOrigins(Bun.env))}`)}`, + `--var ${$.escape(`DEV_CONNECT_ORIGINS:${JSON.stringify(connectOrigins)}`)}`, ].join(' '), }, ] : []), ]; +} + +async function startWeb() { + // Bun loads .env automatically, preserving explicit shell overrides. + const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); + const {publicPort, internalPort, ports} = getWebDevPorts(Bun.env, isWorkerEnabled); + for (const port of ports) { + assertWebPortAvailable(port); + } + if (isProxyEnabled) { + assertWebPortAvailable(API_PROXY_PORT, '::'); + } + for (const file of ['key.pem', 'certificate.pem']) { + if (!(await Bun.file(`config/rsbuild/${file}`).exists())) { + throw new Error('Missing local HTTPS certificate. Run npm run setup-https first.'); + } + } + if (isWorkerEnabled && !(await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists())) { + throw new Error('Missing Worker dependencies. Run npm install to finish App setup.'); + } + const origin = `https://dev.new.expensify.com:${publicPort}`; + const commands = getWebCommands({root: process.cwd(), isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins: getDevelopmentConnectOrigins(Bun.env)}); const controller = new AbortController(); const {commands: children, result} = concurrently(commands, {killOthersOn: ['success', 'failure'], killTimeout: 3000}); const stop = () => { @@ -96,7 +116,11 @@ async function startWeb() { } } -startWeb().catch((error: unknown) => { - console.error(error); - process.exitCode = 1; -}); +if (import.meta.main) { + startWeb().catch((error: unknown) => { + console.error(error); + process.exitCode = 1; + }); +} + +export default getWebCommands; diff --git a/tests/tooling/getDevServerSettings.test.ts b/tests/tooling/getDevServerSettings.test.ts new file mode 100644 index 000000000000..81ebde137979 --- /dev/null +++ b/tests/tooling/getDevServerSettings.test.ts @@ -0,0 +1,37 @@ +/** Rsbuild should only adopt launcher-owned settings when the launcher says it started Rsbuild. */ +import {describe, expect, mock, test} from 'bun:test'; + +import getDevServerSettings from '../../config/rsbuild/getDevServerSettings'; + +describe('Rsbuild dev server settings', () => { + test('uses the launcher ports exactly when the launcher started it', async () => { + // Given the launcher's explicit settings + const findPort = mock(async () => 8090); + // When Rsbuild resolves its server settings + const settings = await getDevServerSettings({WEB_LAUNCHER: 'true', WEB_PORT: '8102', WEB_INTERNAL_PORT: '8103', USE_CLOUDFLARE_WORKER: 'true'}, findPort); + // Then it binds the private port behind the Worker, never another port, and leaves opening the browser to the launcher + expect(settings).toEqual({isWorkerEnabled: true, publicPort: 8102, port: 8103, strictPort: true, open: false}); + expect(findPort).not.toHaveBeenCalled(); + }); + + test('serves the public port directly when the launcher runs without the Worker', async () => { + // Given a launcher session with the Worker off + // When Rsbuild resolves its server settings + // Then it binds the public port itself + expect(await getDevServerSettings({WEB_LAUNCHER: 'true', WEB_PORT: '8102', WEB_INTERNAL_PORT: '8103', USE_CLOUDFLARE_WORKER: 'false'}, async () => 8090)).toEqual({ + isWorkerEnabled: false, + publicPort: 8102, + port: 8102, + strictPort: true, + open: false, + }); + }); + + test('ignores launcher settings from .env when Rsbuild runs on its own', async () => { + // Given WEB_PORT and the Worker flag set in .env, which docs allow for the launcher + // When Rsbuild runs directly + const settings = await getDevServerSettings({WEB_PORT: '8102', USE_CLOUDFLARE_WORKER: 'true'}, async () => 8090); + // Then it keeps its standalone behavior: a free port, direct HTTPS, and opening the app itself + expect(settings).toEqual({isWorkerEnabled: false, publicPort: 8090, port: 8090, strictPort: false, open: '/'}); + }); +}); diff --git a/tests/tooling/startWeb.test.ts b/tests/tooling/startWeb.test.ts new file mode 100644 index 000000000000..a0d55d4abfea --- /dev/null +++ b/tests/tooling/startWeb.test.ts @@ -0,0 +1,41 @@ +/** The launcher should hand its resolved settings to each child explicitly, without mutating its own environment. */ +import {describe, expect, test} from 'bun:test'; + +import getWebCommands from '../../scripts/startWeb'; + +const SETTINGS = {root: '/repo', publicPort: 8102, internalPort: 8103, connectOrigins: ['https://www.expensify.com.dev']}; + +describe('web launcher commands', () => { + test('passes the resolved settings and an explicit launcher flag to Rsbuild', () => { + // Given a session with the Worker and the API proxy enabled + // When the launcher builds its commands + const commands = getWebCommands({...SETTINGS, isWorkerEnabled: true, isProxyEnabled: true}); + // Then Rsbuild learns that the launcher started it, and which ports to use, from its own env + expect(commands.find(({name}) => name === 'rsbuild')?.env).toEqual({ + WEB_LAUNCHER: 'true', + WEB_PORT: '8102', + WEB_INTERNAL_PORT: '8103', + USE_CLOUDFLARE_WORKER: 'true', + USE_WEB_PROXY: 'true', + }); + expect(commands.map(({name}) => name)).toEqual(['api', 'rsbuild', 'worker']); + }); + + test('does not change its own environment, which every child would inherit', () => { + // Given the launcher's environment + const before = {...process.env}; + // When it builds its commands + getWebCommands({...SETTINGS, isWorkerEnabled: false, isProxyEnabled: false}); + // Then nothing leaks into it + expect(process.env).toEqual(before); + }); + + test('starts only the selected services', () => { + // Given both services opted out + // When the launcher builds its commands + const commands = getWebCommands({...SETTINGS, isWorkerEnabled: false, isProxyEnabled: false}); + // Then only Rsbuild runs, and it knows the Worker and proxy are off + expect(commands.map(({name}) => name)).toEqual(['rsbuild']); + expect(commands.at(0)?.env).toMatchObject({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'}); + }); +}); From c3194a6aa820b646bb2e839bb76e103a72a94ff1 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 23:28:33 -0700 Subject: [PATCH 23/31] style(web): list Wrangler arguments one per line --- scripts/startWeb.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 625460118172..d35df50d6c0c 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -43,8 +43,13 @@ function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, inte cwd: workerDir, command: [ `bun ../../scripts/waitForWebServer.ts http://127.0.0.1:${internalPort}/ &&`, - 'node node_modules/wrangler/bin/wrangler.js dev --local --config wrangler.dev.jsonc', - `--ip 127.0.0.1 --port ${publicPort} --inspector-port 0 --local-protocol https`, + 'node node_modules/wrangler/bin/wrangler.js dev', + '--local', + '--config wrangler.dev.jsonc', + '--ip 127.0.0.1', + `--port ${publicPort}`, + '--inspector-port 0', + '--local-protocol https', `--https-key-path ${$.escape(`${root}/config/rsbuild/key.pem`)}`, `--https-cert-path ${$.escape(`${root}/config/rsbuild/certificate.pem`)}`, `--var ${$.escape(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, From bec3811da18de8f3b787426a5f4102bad9297822 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 23:28:34 -0700 Subject: [PATCH 24/31] refactor(web): launch Rsbuild and Wrangler with npx --- scripts/startWeb.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index d35df50d6c0c..0993c4ef2e45 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -26,7 +26,7 @@ function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, inte ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy', env: {USE_WEB_PROXY: 'true'}}] : []), { name: 'rsbuild', - command: 'node node_modules/@rsbuild/core/bin/rsbuild.js dev --config config/rsbuild/rsbuild.config.ts', + command: 'npx rsbuild dev --config config/rsbuild/rsbuild.config.ts', // WEB_LAUNCHER tells Rsbuild these settings came from this launcher, not from a user's .env. env: { WEB_LAUNCHER: 'true', @@ -43,7 +43,7 @@ function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, inte cwd: workerDir, command: [ `bun ../../scripts/waitForWebServer.ts http://127.0.0.1:${internalPort}/ &&`, - 'node node_modules/wrangler/bin/wrangler.js dev', + 'npx wrangler dev', '--local', '--config wrangler.dev.jsonc', '--ip 127.0.0.1', From 26dc1498f3afdd93ff7223209454ad6acd6df287 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 23 Oct 2026 23:33:21 -0700 Subject: [PATCH 25/31] chore(deps): bump @types/node to match the Node 26 runtime Node 26 types declare tls.getCACertificates, so the local module augmentation is no longer needed. Their stricter exec and webcrypto overloads needed an explicit encoding type in Git.ts, and Node buffer types in the CloudflareAccess test mocks. --- package-lock.json | 12 ++++++++---- package.json | 2 +- scripts/types/tls.d.ts | 6 ------ scripts/utils/Git.ts | 3 ++- tests/unit/CloudflareAccessTest.ts | 4 ++-- 5 files changed, 13 insertions(+), 14 deletions(-) delete mode 100644 scripts/types/tls.d.ts diff --git a/package-lock.json b/package-lock.json index 8945f71935ee..ab20b644cda4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -220,7 +220,7 @@ "@types/lodash-es": "4.17.12", "@types/mapbox-gl": "^2.7.13", "@types/mime-db": "^1.43.5", - "@types/node": "^20.11.5", + "@types/node": "^26.6.5", "@types/pako": "^2.0.3", "@types/pusher-js": "^5.1.0", "@types/react": "19.2.3", @@ -19635,10 +19635,12 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "20.19.9", + "version": "26.6.5", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.6.5.tgz", + "integrity": "sha512-uMzsv9stDdofIZBx4UGh4Z9ZgXnGC9seRDscilBcTHAeOFk67MZg/k9bWVfO+bk4qZmG+l52hRnLwUiH2LnJTg==", "license": "MIT", "dependencies": { - "undici-types": "~6.21.0" + "undici-types": "~8.9.0" } }, "node_modules/@types/pako": { @@ -41926,7 +41928,9 @@ } }, "node_modules/undici-types": { - "version": "6.21.0", + "version": "8.9.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.9.0.tgz", + "integrity": "sha512-KTDyRTYX8sWmKXAikPHHSyc63CRPETMctyjKFupcC6OBLXT3xsN0e9aF7m+mIXutFWpUXuedtowG7iLOzp0kQg==", "license": "MIT" }, "node_modules/unicode-canonical-property-names-ecmascript": { diff --git a/package.json b/package.json index f561711368df..a29156f06091 100644 --- a/package.json +++ b/package.json @@ -297,7 +297,7 @@ "@types/lodash-es": "4.17.12", "@types/mapbox-gl": "^2.7.13", "@types/mime-db": "^1.43.5", - "@types/node": "^20.11.5", + "@types/node": "^26.6.5", "@types/pako": "^2.0.3", "@types/pusher-js": "^5.1.0", "@types/react": "19.2.3", diff --git a/scripts/types/tls.d.ts b/scripts/types/tls.d.ts deleted file mode 100644 index 2e5ecd06bf46..000000000000 --- a/scripts/types/tls.d.ts +++ /dev/null @@ -1,6 +0,0 @@ -/** Bun and Node 26 implement tls.getCACertificates, but the repo's @types/node predates it. */ -declare module 'tls' { - function getCACertificates(type?: 'default' | 'system' | 'bundled' | 'extra'): string[]; -} - -export {}; diff --git a/scripts/utils/Git.ts b/scripts/utils/Git.ts index 48dcbb6ccebb..7a17ca2850b8 100644 --- a/scripts/utils/Git.ts +++ b/scripts/utils/Git.ts @@ -13,7 +13,8 @@ import {error as logError, warn as logWarn} from './Logger'; type ExecOptions = Omit & {cwd?: ExecWithCallbackOptions['cwd']}; function exec(command: string, options?: ExecOptions) { - const optionsWithEncoding = { + // The explicit encoding type selects the overload that resolves stdout and stderr as strings. + const optionsWithEncoding: ExecWithCallbackOptions & {encoding: BufferEncoding} = { encoding: 'utf8', cwd: process.cwd(), ...options, diff --git a/tests/unit/CloudflareAccessTest.ts b/tests/unit/CloudflareAccessTest.ts index 63e9cd957339..082f528e1423 100644 --- a/tests/unit/CloudflareAccessTest.ts +++ b/tests/unit/CloudflareAccessTest.ts @@ -56,8 +56,8 @@ function resetQAAuthConfig() { beforeEach(() => { jest.clearAllMocks(); resetQAAuthConfig(); - getWebCrypto.getRandomValues.mockImplementation((array: Uint8Array) => webcrypto.getRandomValues(array)); - getWebCrypto.sha256.mockImplementation((data: BufferSource) => webcrypto.subtle.digest('SHA-256', data)); + getWebCrypto.getRandomValues.mockImplementation((array: Uint8Array) => webcrypto.getRandomValues(array)); + getWebCrypto.sha256.mockImplementation((data: NodeJS.BufferSource) => webcrypto.subtle.digest('SHA-256', data)); jest.mocked(getAuthServerEndpoints).mockResolvedValue({ authorizationEndpoint: 'https://team.cloudflareaccess.com/cdn-cgi/access/oauth/authorization', tokenEndpoint: 'https://team.cloudflareaccess.com/cdn-cgi/access/oauth/token', From 1d87d41022acfae701244aed9fcfaad990401d0f Mon Sep 17 00:00:00 2001 From: rory Date: Sat, 24 Oct 2026 00:41:08 -0700 Subject: [PATCH 26/31] refactor(web): move getWebCommands into its own module startWeb.ts default-exported a different function so a test could import it, which breaks the rule that files are named after what they export. --- scripts/startWeb.ts | 65 ++----------------- scripts/utils/getWebCommands.ts | 55 ++++++++++++++++ ...tartWeb.test.ts => getWebCommands.test.ts} | 2 +- 3 files changed, 61 insertions(+), 61 deletions(-) create mode 100644 scripts/utils/getWebCommands.ts rename tests/tooling/{startWeb.test.ts => getWebCommands.test.ts} (96%) diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 0993c4ef2e45..0b02d60e6a93 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -6,61 +6,10 @@ import concurrently from 'concurrently'; // eslint-disable-next-line import/no-relative-packages import {API_PROXY_PORT} from '../web/cloudflare-worker/src/developmentAPIProxy'; import checkWebServer from './utils/checkWebServer'; +import getWebCommands from './utils/getWebCommands'; import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; import waitForWebServer from './waitForWebServer'; -type WebCommandSettings = { - root: string; - isWorkerEnabled: boolean; - isProxyEnabled: boolean; - publicPort: number; - internalPort: number; - connectOrigins: string[]; -}; - -/** Each child gets the settings it needs in its own env, so the launcher's environment stays as the user set it. */ -function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins}: WebCommandSettings) { - const workerDir = `${root}/web/cloudflare-worker`; - // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. - return [ - ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy', env: {USE_WEB_PROXY: 'true'}}] : []), - { - name: 'rsbuild', - command: 'npx rsbuild dev --config config/rsbuild/rsbuild.config.ts', - // WEB_LAUNCHER tells Rsbuild these settings came from this launcher, not from a user's .env. - env: { - WEB_LAUNCHER: 'true', - WEB_PORT: String(publicPort), - WEB_INTERNAL_PORT: String(internalPort), - USE_CLOUDFLARE_WORKER: String(isWorkerEnabled), - USE_WEB_PROXY: String(isProxyEnabled), - }, - }, - ...(isWorkerEnabled - ? [ - { - name: 'worker', - cwd: workerDir, - command: [ - `bun ../../scripts/waitForWebServer.ts http://127.0.0.1:${internalPort}/ &&`, - 'npx wrangler dev', - '--local', - '--config wrangler.dev.jsonc', - '--ip 127.0.0.1', - `--port ${publicPort}`, - '--inspector-port 0', - '--local-protocol https', - `--https-key-path ${$.escape(`${root}/config/rsbuild/key.pem`)}`, - `--https-cert-path ${$.escape(`${root}/config/rsbuild/certificate.pem`)}`, - `--var ${$.escape(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, - `--var ${$.escape(`DEV_CONNECT_ORIGINS:${JSON.stringify(connectOrigins)}`)}`, - ].join(' '), - }, - ] - : []), - ]; -} - async function startWeb() { // Bun loads .env automatically, preserving explicit shell overrides. const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); @@ -121,11 +70,7 @@ async function startWeb() { } } -if (import.meta.main) { - startWeb().catch((error: unknown) => { - console.error(error); - process.exitCode = 1; - }); -} - -export default getWebCommands; +startWeb().catch((error: unknown) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/scripts/utils/getWebCommands.ts b/scripts/utils/getWebCommands.ts new file mode 100644 index 000000000000..79db05284923 --- /dev/null +++ b/scripts/utils/getWebCommands.ts @@ -0,0 +1,55 @@ +import {$} from 'bun'; + +type WebCommandSettings = { + root: string; + isWorkerEnabled: boolean; + isProxyEnabled: boolean; + publicPort: number; + internalPort: number; + connectOrigins: string[]; +}; + +/** Each child gets the settings it needs in its own env, so the launcher's environment stays as the user set it. */ +function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins}: WebCommandSettings) { + const workerDir = `${root}/web/cloudflare-worker`; + // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. + return [ + ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy', env: {USE_WEB_PROXY: 'true'}}] : []), + { + name: 'rsbuild', + command: 'npx rsbuild dev --config config/rsbuild/rsbuild.config.ts', + // WEB_LAUNCHER tells Rsbuild these settings came from this launcher, not from a user's .env. + env: { + WEB_LAUNCHER: 'true', + WEB_PORT: String(publicPort), + WEB_INTERNAL_PORT: String(internalPort), + USE_CLOUDFLARE_WORKER: String(isWorkerEnabled), + USE_WEB_PROXY: String(isProxyEnabled), + }, + }, + ...(isWorkerEnabled + ? [ + { + name: 'worker', + cwd: workerDir, + command: [ + `bun ../../scripts/waitForWebServer.ts http://127.0.0.1:${internalPort}/ &&`, + 'npx wrangler dev', + '--local', + '--config wrangler.dev.jsonc', + '--ip 127.0.0.1', + `--port ${publicPort}`, + '--inspector-port 0', + '--local-protocol https', + `--https-key-path ${$.escape(`${root}/config/rsbuild/key.pem`)}`, + `--https-cert-path ${$.escape(`${root}/config/rsbuild/certificate.pem`)}`, + `--var ${$.escape(`DEV_UPSTREAM:http://127.0.0.1:${internalPort}`)}`, + `--var ${$.escape(`DEV_CONNECT_ORIGINS:${JSON.stringify(connectOrigins)}`)}`, + ].join(' '), + }, + ] + : []), + ]; +} + +export default getWebCommands; diff --git a/tests/tooling/startWeb.test.ts b/tests/tooling/getWebCommands.test.ts similarity index 96% rename from tests/tooling/startWeb.test.ts rename to tests/tooling/getWebCommands.test.ts index a0d55d4abfea..c1e922566b69 100644 --- a/tests/tooling/startWeb.test.ts +++ b/tests/tooling/getWebCommands.test.ts @@ -1,7 +1,7 @@ /** The launcher should hand its resolved settings to each child explicitly, without mutating its own environment. */ import {describe, expect, test} from 'bun:test'; -import getWebCommands from '../../scripts/startWeb'; +import getWebCommands from '../../scripts/utils/getWebCommands'; const SETTINGS = {root: '/repo', publicPort: 8102, internalPort: 8103, connectOrigins: ['https://www.expensify.com.dev']}; From 2782299f72160efb3c10c317388def4da9b3d77b Mon Sep 17 00:00:00 2001 From: rory Date: Sat, 24 Oct 2026 00:44:26 -0700 Subject: [PATCH 27/31] fix(web): allow the QA backend and its Cloudflare Access issuer in dev CSP With QA configured, the app calls the QA hosts directly and the Access flow calls https://$QA_CF_TEAM_DOMAIN, but dev connect-src allowed neither, so the QA auth probe and sign-in failed. Add the QA URLs to the backend keys and allow the team domain, which must be a bare hostname. --- config/rsbuild/developmentEnvKeys.ts | 27 +++++++++++++++++++++--- scripts/utils/webDevConfig.ts | 10 ++++++++- tests/tooling/developmentEnvKeys.test.ts | 6 +++--- tests/tooling/webDevConfig.test.ts | 21 ++++++++++++++++++ 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/config/rsbuild/developmentEnvKeys.ts b/config/rsbuild/developmentEnvKeys.ts index 33b1fc992681..4816802b58e1 100644 --- a/config/rsbuild/developmentEnvKeys.ts +++ b/config/rsbuild/developmentEnvKeys.ts @@ -1,7 +1,28 @@ /** Local backends from `.env`, which the app calls and the launcher adds to the live Worker's CSP. */ -const LOCAL_BACKEND_URL_KEYS = ['EXPENSIFY_URL', 'SECURE_EXPENSIFY_URL', 'STAGING_EXPENSIFY_URL', 'STAGING_SECURE_EXPENSIFY_URL', 'NGROK_URL', 'SECURE_NGROK_URL']; +const LOCAL_BACKEND_URL_KEYS = [ + 'EXPENSIFY_URL', + 'SECURE_EXPENSIFY_URL', + 'STAGING_EXPENSIFY_URL', + 'STAGING_SECURE_EXPENSIFY_URL', + 'QA_EXPENSIFY_URL', + 'QA_SECURE_EXPENSIFY_URL', + 'NGROK_URL', + 'SECURE_NGROK_URL', +]; + +/** The Cloudflare Access team for the QA backend: a bare hostname whose token endpoint the app calls over HTTPS. */ +const QA_ACCESS_TEAM_DOMAIN_KEY = 'QA_CF_TEAM_DOMAIN'; /** Public development settings that shell variables can override in the app config. Never forward arbitrary shell variables, which may contain secrets. */ -const FORWARDED_DEVELOPMENT_ENV_KEYS = [...LOCAL_BACKEND_URL_KEYS, 'USE_WEB_PROXY', 'USE_NGROK', 'NEW_EXPENSIFY_URL', 'SKIP_ONBOARDING', 'PUSHER_APP_KEY', 'PUSHER_DEV_SUFFIX']; +const FORWARDED_DEVELOPMENT_ENV_KEYS = [ + ...LOCAL_BACKEND_URL_KEYS, + QA_ACCESS_TEAM_DOMAIN_KEY, + 'USE_WEB_PROXY', + 'USE_NGROK', + 'NEW_EXPENSIFY_URL', + 'SKIP_ONBOARDING', + 'PUSHER_APP_KEY', + 'PUSHER_DEV_SUFFIX', +]; -export {FORWARDED_DEVELOPMENT_ENV_KEYS, LOCAL_BACKEND_URL_KEYS}; +export {FORWARDED_DEVELOPMENT_ENV_KEYS, LOCAL_BACKEND_URL_KEYS, QA_ACCESS_TEAM_DOMAIN_KEY}; diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index 05a5f9123b05..709cd77c33f8 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -1,5 +1,5 @@ /** Resolve one set of ports and flags for every process in a local web session. */ -import {LOCAL_BACKEND_URL_KEYS} from '../../config/rsbuild/developmentEnvKeys'; +import {LOCAL_BACKEND_URL_KEYS, QA_ACCESS_TEAM_DOMAIN_KEY} from '../../config/rsbuild/developmentEnvKeys'; function getWebDevFlags(env: Record) { return {isWorkerEnabled: env.USE_CLOUDFLARE_WORKER !== 'false', isProxyEnabled: env.USE_WEB_PROXY !== 'false', shouldOpenBrowser: env.WEB_OPEN !== 'false'}; @@ -18,6 +18,14 @@ function getDevelopmentConnectOrigins(env: Record): } origins.add(url.origin); } + const teamDomain = env[QA_ACCESS_TEAM_DOMAIN_KEY]; + if (teamDomain) { + const issuer = URL.parse(`https://${teamDomain}`); + if (issuer?.host !== teamDomain.toLowerCase()) { + throw new Error(`${QA_ACCESS_TEAM_DOMAIN_KEY} must be a bare hostname, like team.cloudflareaccess.com`); + } + origins.add(issuer.origin); + } return [...origins]; } diff --git a/tests/tooling/developmentEnvKeys.test.ts b/tests/tooling/developmentEnvKeys.test.ts index b3b2cbc99543..2f14a10dca07 100644 --- a/tests/tooling/developmentEnvKeys.test.ts +++ b/tests/tooling/developmentEnvKeys.test.ts @@ -1,15 +1,15 @@ /** The app and the Worker CSP must see the same local backends, so the backend keys are defined once. */ import {describe, expect, test} from 'bun:test'; -import {FORWARDED_DEVELOPMENT_ENV_KEYS, LOCAL_BACKEND_URL_KEYS} from '../../config/rsbuild/developmentEnvKeys'; +import {FORWARDED_DEVELOPMENT_ENV_KEYS, LOCAL_BACKEND_URL_KEYS, QA_ACCESS_TEAM_DOMAIN_KEY} from '../../config/rsbuild/developmentEnvKeys'; import {getDevelopmentConnectOrigins} from '../../scripts/utils/webDevConfig'; describe('development env keys', () => { test('forwards every backend the CSP allows to the app', () => { // Given the backend keys the launcher adds to the Worker CSP // When Rsbuild forwards development settings to the app - // Then the app sees every one of those backends - for (const key of LOCAL_BACKEND_URL_KEYS) { + // Then the app sees every one of those backends, and the QA Access issuer + for (const key of [...LOCAL_BACKEND_URL_KEYS, QA_ACCESS_TEAM_DOMAIN_KEY]) { expect(FORWARDED_DEVELOPMENT_ENV_KEYS).toContain(key); } }); diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts index 723eb6db704c..78bcd7e7500e 100644 --- a/tests/tooling/webDevConfig.test.ts +++ b/tests/tooling/webDevConfig.test.ts @@ -25,6 +25,27 @@ describe('web development configuration', () => { expect(() => getDevelopmentConnectOrigins({EXPENSIFY_URL: 'https://user:pass@example.test'})).toThrow(); }); + test('allows the QA backend and its Cloudflare Access team domain', () => { + // Given a QA backend behind Cloudflare Access, whose token endpoint the app calls on the bare team domain + // When CSP origins are resolved + const origins = getDevelopmentConnectOrigins({ + QA_EXPENSIFY_URL: 'https://qa.example.test/', + QA_SECURE_EXPENSIFY_URL: 'https://secure-qa.example.test/', + QA_CF_TEAM_DOMAIN: 'team.cloudflareaccess.com', + }); + // Then the app can reach the QA hosts and the Access issuer + expect(origins).toEqual(['https://qa.example.test', 'https://secure-qa.example.test', 'https://team.cloudflareaccess.com']); + }); + + test('names QA_CF_TEAM_DOMAIN when it is not a bare hostname', () => { + // Given a team domain written as a URL, which the app would prefix with https:// again + // When CSP origins are resolved + // Then startup explains the setting rather than allowing an origin the app never calls + for (const QA_CF_TEAM_DOMAIN of ['https://team.cloudflareaccess.com', 'team.cloudflareaccess.com/path', 'user@team.cloudflareaccess.com']) { + expect(() => getDevelopmentConnectOrigins({QA_CF_TEAM_DOMAIN})).toThrow('QA_CF_TEAM_DOMAIN'); + } + }); + test('rejects invalid and overlapping ports before starting children', () => { // Given invalid coordinated ports // When the launcher checks them From 5763cd7de85888c4d2ad7c5bc0829112c1741cbf Mon Sep 17 00:00:00 2001 From: rory Date: Sat, 24 Oct 2026 00:45:54 -0700 Subject: [PATCH 28/31] fix(web): serve Rsbuild directly when the Worker tooling is missing npm install skips the Worker when CI is set, so npm run web failed in containers, cloud agents, and CI-run browser tests that worked on main. Warn and fall back to direct Rsbuild instead. --- contributingGuides/SETUP_WEB.md | 2 +- scripts/startWeb.ts | 9 +++++---- scripts/utils/webDevConfig.ts | 11 +++++++++-- tests/tooling/webDevConfig.test.ts | 26 ++++++++++++++++++++------ 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/contributingGuides/SETUP_WEB.md b/contributingGuides/SETUP_WEB.md index 96cf800dbc00..d8ca2dbbd752 100644 --- a/contributingGuides/SETUP_WEB.md +++ b/contributingGuides/SETUP_WEB.md @@ -161,7 +161,7 @@ You probably have a misconfigured `.env` file - remove it (`rm .env`) and try ag 1. If you are having issues with **_Getting Started_**, please reference [React Native's Documentation](https://reactnative.dev/docs/environment-setup) 2. Make sure you have the correct Node version installed as specified in `.nvmrc` 3. Ensure HTTPS is properly configured if you're having certificate issues -4. If Worker setup is missing, rerun `npm install`; launching the server never installs dependencies. Missing certificates require `npm run setup-https`. Trust the mkcert CA in the browser rather than disabling certificate checks. +4. If the Worker tooling is missing, `npm run web` warns and serves Rsbuild directly. Rerun `npm install` to restore the Worker; it skips the Worker when `CI` is set, and launching the server never installs dependencies. Missing certificates require `npm run setup-https`. Trust the mkcert CA in the browser rather than disabling certificate checks. 5. If the Worker returns `503`, inspect the Rsbuild process output. There is no fallback to an old `dist/` build. If a page appears stale, check for an old service worker in the disposable browser profile. 6. The old ngrok workaround for Wrangler upstream ports is not required: live mode explicitly fetches a loopback HTTP origin. Do not clear CA settings or disable TLS verification to work around unrelated certificate errors. diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 0b02d60e6a93..9144ccb7fc72 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -12,7 +12,11 @@ import waitForWebServer from './waitForWebServer'; async function startWeb() { // Bun loads .env automatically, preserving explicit shell overrides. - const {isWorkerEnabled, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env); + const isWorkerInstalled = await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists(); + const {isWorkerEnabled, isWorkerMissing, isProxyEnabled, shouldOpenBrowser} = getWebDevFlags(Bun.env, isWorkerInstalled); + if (isWorkerMissing) { + console.warn('Cloudflare Worker tooling is not installed, so serving Rsbuild directly without the Worker. Run npm install (it skips the Worker when CI is set).'); + } const {publicPort, internalPort, ports} = getWebDevPorts(Bun.env, isWorkerEnabled); for (const port of ports) { assertWebPortAvailable(port); @@ -25,9 +29,6 @@ async function startWeb() { throw new Error('Missing local HTTPS certificate. Run npm run setup-https first.'); } } - if (isWorkerEnabled && !(await Bun.file('web/cloudflare-worker/node_modules/wrangler/bin/wrangler.js').exists())) { - throw new Error('Missing Worker dependencies. Run npm install to finish App setup.'); - } const origin = `https://dev.new.expensify.com:${publicPort}`; const commands = getWebCommands({root: process.cwd(), isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins: getDevelopmentConnectOrigins(Bun.env)}); const controller = new AbortController(); diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index 709cd77c33f8..3fe338ddf723 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -1,8 +1,15 @@ /** Resolve one set of ports and flags for every process in a local web session. */ import {LOCAL_BACKEND_URL_KEYS, QA_ACCESS_TEAM_DOMAIN_KEY} from '../../config/rsbuild/developmentEnvKeys'; -function getWebDevFlags(env: Record) { - return {isWorkerEnabled: env.USE_CLOUDFLARE_WORKER !== 'false', isProxyEnabled: env.USE_WEB_PROXY !== 'false', shouldOpenBrowser: env.WEB_OPEN !== 'false'}; +/** A session whose install skipped or failed to install the Worker tooling still starts, serving Rsbuild directly. */ +function getWebDevFlags(env: Record, isWorkerInstalled: boolean) { + const isWorkerRequested = env.USE_CLOUDFLARE_WORKER !== 'false'; + return { + isWorkerEnabled: isWorkerRequested && isWorkerInstalled, + isWorkerMissing: isWorkerRequested && !isWorkerInstalled, + isProxyEnabled: env.USE_WEB_PROXY !== 'false', + shouldOpenBrowser: env.WEB_OPEN !== 'false', + }; } function getDevelopmentConnectOrigins(env: Record): string[] { diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts index 78bcd7e7500e..5c6a56a327ba 100644 --- a/tests/tooling/webDevConfig.test.ts +++ b/tests/tooling/webDevConfig.test.ts @@ -5,14 +5,28 @@ import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, ge describe('web development configuration', () => { test('defaults both services on and allows independent opt-outs', () => { - // Given contributor defaults + // Given contributor defaults, with the Worker tooling installed // When flags are resolved - const defaults = getWebDevFlags({}); + const defaults = getWebDevFlags({}, true); // Then neither opt-out changes the meaning of the other - expect(defaults).toEqual({isWorkerEnabled: true, isProxyEnabled: true, shouldOpenBrowser: true}); - expect(getWebDevFlags({USE_WEB_PROXY: 'false'})).toMatchObject({isWorkerEnabled: true, isProxyEnabled: false}); - expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', WEB_OPEN: 'false'})).toEqual({isWorkerEnabled: false, isProxyEnabled: true, shouldOpenBrowser: false}); - expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'})).toMatchObject({isWorkerEnabled: false, isProxyEnabled: false}); + expect(defaults).toEqual({isWorkerEnabled: true, isWorkerMissing: false, isProxyEnabled: true, shouldOpenBrowser: true}); + expect(getWebDevFlags({USE_WEB_PROXY: 'false'}, true)).toMatchObject({isWorkerEnabled: true, isProxyEnabled: false}); + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', WEB_OPEN: 'false'}, true)).toEqual({ + isWorkerEnabled: false, + isWorkerMissing: false, + isProxyEnabled: true, + shouldOpenBrowser: false, + }); + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'}, true)).toMatchObject({isWorkerEnabled: false, isProxyEnabled: false}); + }); + + test('serves Rsbuild directly when the Worker tooling is not installed', () => { + // Given a checkout whose install skipped or failed to install the Worker, e.g. with CI set + // When flags are resolved + // Then the session still starts without the Worker, and says why, rather than failing + expect(getWebDevFlags({}, false)).toMatchObject({isWorkerEnabled: false, isWorkerMissing: true}); + // And a session that opted out of the Worker has nothing to report + expect(getWebDevFlags({USE_CLOUDFLARE_WORKER: 'false'}, false)).toMatchObject({isWorkerEnabled: false, isWorkerMissing: false}); }); test('validates and deduplicates explicit backend origins', () => { From 97c80326e724adc8c37fa32247ac7cdb3bea83b6 Mon Sep 17 00:00:00 2001 From: rory Date: Sat, 24 Oct 2026 00:46:31 -0700 Subject: [PATCH 29/31] fix(web): warn instead of failing npm install when the Worker install fails postInstall runs under set -e, so an offline machine or a failed workerd download failed every App install, including for developers who never run the Worker. npm run web now serves Rsbuild directly without it, and the next install retries. --- scripts/installCloudflareWorker.ts | 8 +++++++- tests/tooling/installCloudflareWorker.test.ts | 19 +++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/scripts/installCloudflareWorker.ts b/scripts/installCloudflareWorker.ts index ccadcad5094e..d28df501ea60 100644 --- a/scripts/installCloudflareWorker.ts +++ b/scripts/installCloudflareWorker.ts @@ -22,7 +22,13 @@ async function installCloudflareWorker(workerDir: string, env: Record { expect(install).toHaveBeenCalledTimes(2); }); - test('retries an install that failed', async () => { - // Given an install that fails partway + test('warns instead of failing the App install, and retries next time', async () => { + // Given an install that fails partway, e.g. offline or a failed workerd download const failingInstall = mock(async () => { throw new Error('network error'); }); - // When the root install runs - // Then it fails, and the next root install tries again - await expect(installCloudflareWorker(workerDir, {}, failingInstall)).rejects.toThrow('network error'); + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + // When the root install runs + // Then it still succeeds for developers who never run the Worker, and explains what is missing + expect(await installCloudflareWorker(workerDir, {}, failingInstall)).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + } finally { + warn.mockRestore(); + } + // And the next root install tries again expect(await installCloudflareWorker(workerDir, {}, install)).toBe(true); }); From c46878f435ebc433e785daacced86807e09f0136 Mon Sep 17 00:00:00 2001 From: rory Date: Sat, 24 Oct 2026 00:46:56 -0700 Subject: [PATCH 30/31] ci: only run the Worker tests for changes they cover The job only typechecks and tests web/cloudflare-worker, which reads no launcher, Rsbuild, or root package files. Those have their own tests under tests/tooling. --- .github/workflows/cloudflareWorkerTests.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/cloudflareWorkerTests.yml b/.github/workflows/cloudflareWorkerTests.yml index a09a05f0ce6d..aff7d9d745ed 100644 --- a/.github/workflows/cloudflareWorkerTests.yml +++ b/.github/workflows/cloudflareWorkerTests.yml @@ -6,14 +6,6 @@ on: branches-ignore: [staging, production] paths: - 'web/cloudflare-worker/**' - - 'scripts/startWeb.ts' - - 'scripts/waitForWebServer.ts' - - 'scripts/utils/webDevConfig.ts' - - 'scripts/utils/checkWebServer.ts' - - 'scripts/postInstall.sh' - - 'scripts/installCloudflareWorker.ts' - - 'config/rsbuild/**' - - 'package.json' # The tests check its inline scripts against the CSP hashes. - 'web/index.html' - '.nvmrc' From f4a652c5b423e984dab3f9acc342c042aa146da3 Mon Sep 17 00:00:00 2001 From: rory Date: Sat, 24 Oct 2026 00:49:41 -0700 Subject: [PATCH 31/31] fix(web): let a second session pick free ports and share the API proxy On main, a second worktree's npm run web moved to the next free port and shared the first session's proxy on 9000. The launcher instead failed if 8082, 8083, or 9000 was busy, and nothing could get a second proxy-enabled session past 9000. Pick the first free ports unless WEB_PORT or WEB_INTERNAL_PORT is set, and reuse a running proxy. --- README.md | 4 +- contributingGuides/SETUP_WEB.md | 2 +- scripts/startWeb.ts | 22 +++++--- scripts/utils/getWebCommands.ts | 7 ++- scripts/utils/webDevConfig.ts | 63 ++++++++++++++++------ tests/tooling/getWebCommands.test.ts | 15 ++++-- tests/tooling/webDevConfig.test.ts | 79 ++++++++++++++++------------ 7 files changed, 126 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index a61b8b618da6..1a0b7ad99a4b 100644 --- a/README.md +++ b/README.md @@ -81,8 +81,8 @@ variables referenced here get updated since your local `.env` file is ignored. requests to the backend. External contributors should set this to `true` otherwise they'll have CORS errors. If you don't want to start the proxy server set this explicitly to `false` - `USE_CLOUDFLARE_WORKER` - Defaults to `true` for `npm run web`. Set to `false` to serve Rsbuild directly over HTTPS. This does not disable the API proxy. -- `WEB_PORT` - Browser-facing development port (default `8082`). If occupied, choose a different port; the launcher will not reuse another session's server. -- `WEB_INTERNAL_PORT` - Internal loopback HTTP Rsbuild port when the Worker is enabled (default `WEB_PORT + 1`). +- `WEB_PORT` - Browser-facing development port (default: the first free port from `8082`). An explicit port must be free. +- `WEB_INTERNAL_PORT` - Internal loopback HTTP Rsbuild port when the Worker is enabled (default: the next free port after `WEB_PORT`). - `WEB_OPEN` - Set to `false` to suppress automatic browser opening, for example in browser automation. - `CAPTURE_METRICS` (optional) - Set this to `true` to capture performance metrics and see them in Flipper see [PERFORMANCE.md](contributingGuides/PERFORMANCE.md#performance-metrics-opt-in-on-local-release-builds) for more information diff --git a/contributingGuides/SETUP_WEB.md b/contributingGuides/SETUP_WEB.md index d8ca2dbbd752..373f9ee69aac 100644 --- a/contributingGuides/SETUP_WEB.md +++ b/contributingGuides/SETUP_WEB.md @@ -42,7 +42,7 @@ USE_WEB_PROXY=false npm run web # Worker still enabled; use your configur WEB_PORT=8102 WEB_OPEN=false npm run web # isolated session suitable for Playwright ``` -`WEB_PORT` defaults to `8082`; `WEB_INTERNAL_PORT` defaults to the next port. Occupied ports fail startup rather than attaching to another worktree or silently changing the upstream. The API proxy retains port `9000`, so only one proxy-enabled session can own it. Stop the process that owns a needed port, or disable the API proxy and configure your backend. Ctrl-C or a required service exiting stops the remaining services. +`WEB_PORT` defaults to the first free port from `8082`, and `WEB_INTERNAL_PORT` to the next free port after it, so a second worktree's session starts alongside the first. A port you set explicitly must be free, or startup fails. The API proxy keeps port `9000`: if another session already runs it, the new session reuses it, and its API requests stop working when that session stops. Ctrl-C or a required service exiting stops the remaining services. Set `EXPENSIFY_URL` and `SECURE_EXPENSIFY_URL` for your local backend and disable `USE_WEB_PROXY` for direct backend requests. Explicit backend origins are added only to the development connection policy. Backend CORS rules and TLS trust still apply; CSP allowances do not bypass them. Third-party OAuth may require an approved origin. diff --git a/scripts/startWeb.ts b/scripts/startWeb.ts index 9144ccb7fc72..794c62d3f397 100644 --- a/scripts/startWeb.ts +++ b/scripts/startWeb.ts @@ -7,7 +7,7 @@ import concurrently from 'concurrently'; import {API_PROXY_PORT} from '../web/cloudflare-worker/src/developmentAPIProxy'; import checkWebServer from './utils/checkWebServer'; import getWebCommands from './utils/getWebCommands'; -import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from './utils/webDevConfig'; +import {findWebDevPorts, getDevelopmentConnectOrigins, getWebDevFlags, isWebPortAvailable} from './utils/webDevConfig'; import waitForWebServer from './waitForWebServer'; async function startWeb() { @@ -17,12 +17,10 @@ async function startWeb() { if (isWorkerMissing) { console.warn('Cloudflare Worker tooling is not installed, so serving Rsbuild directly without the Worker. Run npm install (it skips the Worker when CI is set).'); } - const {publicPort, internalPort, ports} = getWebDevPorts(Bun.env, isWorkerEnabled); - for (const port of ports) { - assertWebPortAvailable(port); - } - if (isProxyEnabled) { - assertWebPortAvailable(API_PROXY_PORT, '::'); + const {publicPort, internalPort} = findWebDevPorts(Bun.env, isWorkerEnabled, isWebPortAvailable); + const shouldStartProxy = isProxyEnabled && isWebPortAvailable(API_PROXY_PORT, '::'); + if (isProxyEnabled && !shouldStartProxy) { + console.log(`Reusing the API proxy already on port ${API_PROXY_PORT}, e.g. from another worktree's session. API requests stop working if that session stops.`); } for (const file of ['key.pem', 'certificate.pem']) { if (!(await Bun.file(`config/rsbuild/${file}`).exists())) { @@ -30,7 +28,15 @@ async function startWeb() { } } const origin = `https://dev.new.expensify.com:${publicPort}`; - const commands = getWebCommands({root: process.cwd(), isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins: getDevelopmentConnectOrigins(Bun.env)}); + const commands = getWebCommands({ + root: process.cwd(), + isWorkerEnabled, + isProxyEnabled, + shouldStartProxy, + publicPort, + internalPort, + connectOrigins: getDevelopmentConnectOrigins(Bun.env), + }); const controller = new AbortController(); const {commands: children, result} = concurrently(commands, {killOthersOn: ['success', 'failure'], killTimeout: 3000}); const stop = () => { diff --git a/scripts/utils/getWebCommands.ts b/scripts/utils/getWebCommands.ts index 79db05284923..83537f08a879 100644 --- a/scripts/utils/getWebCommands.ts +++ b/scripts/utils/getWebCommands.ts @@ -4,17 +4,20 @@ type WebCommandSettings = { root: string; isWorkerEnabled: boolean; isProxyEnabled: boolean; + + /** False when another session already runs the API proxy, which this session's Rsbuild then shares. */ + shouldStartProxy: boolean; publicPort: number; internalPort: number; connectOrigins: string[]; }; /** Each child gets the settings it needs in its own env, so the launcher's environment stays as the user set it. */ -function getWebCommands({root, isWorkerEnabled, isProxyEnabled, publicPort, internalPort, connectOrigins}: WebCommandSettings) { +function getWebCommands({root, isWorkerEnabled, isProxyEnabled, shouldStartProxy, publicPort, internalPort, connectOrigins}: WebCommandSettings) { const workerDir = `${root}/web/cloudflare-worker`; // concurrently accepts command strings, so escape its dynamic paths and JSON bindings. return [ - ...(isProxyEnabled ? [{name: 'api', command: 'npm run web-proxy', env: {USE_WEB_PROXY: 'true'}}] : []), + ...(isProxyEnabled && shouldStartProxy ? [{name: 'api', command: 'npm run web-proxy', env: {USE_WEB_PROXY: 'true'}}] : []), { name: 'rsbuild', command: 'npx rsbuild dev --config config/rsbuild/rsbuild.config.ts', diff --git a/scripts/utils/webDevConfig.ts b/scripts/utils/webDevConfig.ts index 3fe338ddf723..c618f8f61b89 100644 --- a/scripts/utils/webDevConfig.ts +++ b/scripts/utils/webDevConfig.ts @@ -36,10 +36,10 @@ function getDevelopmentConnectOrigins(env: Record): return [...origins]; } -/** Fail on conflicts rather than letting a child silently attach to another worktree's server. */ -function assertWebPortAvailable(port: number, host = '127.0.0.1'): void { +/** Whether a session could bind the port. Releases its probe immediately, so the session's own server can bind it. */ +function isWebPortAvailable(port: number, host = '127.0.0.1'): boolean { try { - const listener = Bun.listen({ + Bun.listen({ hostname: host, port, socket: { @@ -47,25 +47,56 @@ function assertWebPortAvailable(port: number, host = '127.0.0.1'): void { socket.end(); }, }, - }); - listener.stop(true); + }).stop(true); + return true; } catch { - throw new Error(`Port ${port} on ${host} is in use. Stop its owner or change this session's port/proxy settings.`); + return false; } } -/** Parse and validate the session's ports. Use `assertWebPortAvailable` to check that they are free. */ -function getWebDevPorts(env: Record, isWorkerEnabled: boolean) { - const publicPort = Number(env.WEB_PORT ?? 8082); - const internalPort = Number(env.WEB_INTERNAL_PORT ?? publicPort + 1); - if (isWorkerEnabled && publicPort === internalPort) { +const DEFAULT_WEB_PORT = 8082; +const MIN_WEB_PORT = 1024; +const MAX_WEB_PORT = 65535; + +function parseWebPort(env: Record, key: 'WEB_PORT' | 'WEB_INTERNAL_PORT'): number | undefined { + const value = env[key]; + if (value === undefined) { + return undefined; + } + const port = Number(value); + if (!Number.isInteger(port) || port < MIN_WEB_PORT || port > MAX_WEB_PORT) { + throw new Error(`${key} must be an integer between ${MIN_WEB_PORT} and ${MAX_WEB_PORT}`); + } + return port; +} + +/** + * Ports the user set must be free, since they asked for exactly those. Otherwise take the first free ports from 8082 up, + * so a second worktree's session starts alongside the first, as it did before the Worker. + */ +function findWebDevPorts(env: Record, isWorkerEnabled: boolean, isPortAvailable: (port: number) => boolean) { + const explicitPublicPort = parseWebPort(env, 'WEB_PORT'); + const explicitInternalPort = isWorkerEnabled ? parseWebPort(env, 'WEB_INTERNAL_PORT') : undefined; + if (explicitPublicPort !== undefined && explicitPublicPort === explicitInternalPort) { throw new Error('WEB_PORT and WEB_INTERNAL_PORT must differ'); } - const ports = isWorkerEnabled ? [publicPort, internalPort] : [publicPort]; - if (ports.some((port) => !Number.isInteger(port) || port < 1024 || port > 65535)) { - throw new Error('WEB_PORT and WEB_INTERNAL_PORT must be integers between 1024 and 65535'); + for (const port of [explicitPublicPort, explicitInternalPort]) { + if (port !== undefined && !isPortAvailable(port)) { + throw new Error(`Port ${port} is in use. Stop its owner, or unset WEB_PORT and WEB_INTERNAL_PORT to use free ports.`); + } } - return {publicPort, internalPort, ports}; + const findFreePort = (from: number, taken: number | undefined) => { + for (let port = from; port <= MAX_WEB_PORT; port++) { + if (port !== taken && isPortAvailable(port)) { + return port; + } + } + throw new Error(`No free port from ${from} up for the web dev server`); + }; + const publicPort = explicitPublicPort ?? findFreePort(DEFAULT_WEB_PORT, explicitInternalPort); + // Rsbuild ignores the internal port without the Worker. + const internalPort = isWorkerEnabled ? (explicitInternalPort ?? findFreePort(publicPort + 1, publicPort)) : publicPort + 1; + return {publicPort, internalPort}; } -export {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts}; +export {findWebDevPorts, getDevelopmentConnectOrigins, getWebDevFlags, isWebPortAvailable}; diff --git a/tests/tooling/getWebCommands.test.ts b/tests/tooling/getWebCommands.test.ts index c1e922566b69..e2285f2be036 100644 --- a/tests/tooling/getWebCommands.test.ts +++ b/tests/tooling/getWebCommands.test.ts @@ -9,7 +9,7 @@ describe('web launcher commands', () => { test('passes the resolved settings and an explicit launcher flag to Rsbuild', () => { // Given a session with the Worker and the API proxy enabled // When the launcher builds its commands - const commands = getWebCommands({...SETTINGS, isWorkerEnabled: true, isProxyEnabled: true}); + const commands = getWebCommands({...SETTINGS, isWorkerEnabled: true, isProxyEnabled: true, shouldStartProxy: true}); // Then Rsbuild learns that the launcher started it, and which ports to use, from its own env expect(commands.find(({name}) => name === 'rsbuild')?.env).toEqual({ WEB_LAUNCHER: 'true', @@ -21,11 +21,20 @@ describe('web launcher commands', () => { expect(commands.map(({name}) => name)).toEqual(['api', 'rsbuild', 'worker']); }); + test('reuses an API proxy another session already runs', () => { + // Given a proxy-enabled session whose proxy port another worktree's session owns + // When the launcher builds its commands + const commands = getWebCommands({...SETTINGS, isWorkerEnabled: true, isProxyEnabled: true, shouldStartProxy: false}); + // Then it does not start a second proxy, but Rsbuild still forwards API requests to the running one + expect(commands.map(({name}) => name)).toEqual(['rsbuild', 'worker']); + expect(commands.at(0)?.env).toMatchObject({USE_WEB_PROXY: 'true'}); + }); + test('does not change its own environment, which every child would inherit', () => { // Given the launcher's environment const before = {...process.env}; // When it builds its commands - getWebCommands({...SETTINGS, isWorkerEnabled: false, isProxyEnabled: false}); + getWebCommands({...SETTINGS, isWorkerEnabled: false, isProxyEnabled: false, shouldStartProxy: false}); // Then nothing leaks into it expect(process.env).toEqual(before); }); @@ -33,7 +42,7 @@ describe('web launcher commands', () => { test('starts only the selected services', () => { // Given both services opted out // When the launcher builds its commands - const commands = getWebCommands({...SETTINGS, isWorkerEnabled: false, isProxyEnabled: false}); + const commands = getWebCommands({...SETTINGS, isWorkerEnabled: false, isProxyEnabled: false, shouldStartProxy: false}); // Then only Rsbuild runs, and it knows the Worker and proxy are off expect(commands.map(({name}) => name)).toEqual(['rsbuild']); expect(commands.at(0)?.env).toMatchObject({USE_CLOUDFLARE_WORKER: 'false', USE_WEB_PROXY: 'false'}); diff --git a/tests/tooling/webDevConfig.test.ts b/tests/tooling/webDevConfig.test.ts index 5c6a56a327ba..2dc097c5d41c 100644 --- a/tests/tooling/webDevConfig.test.ts +++ b/tests/tooling/webDevConfig.test.ts @@ -1,7 +1,9 @@ /** Keep local Worker/API opt-outs and coordinated port selection independent. */ import {describe, expect, test} from 'bun:test'; -import {assertWebPortAvailable, getDevelopmentConnectOrigins, getWebDevFlags, getWebDevPorts} from '../../scripts/utils/webDevConfig'; +import {findWebDevPorts, getDevelopmentConnectOrigins, getWebDevFlags, isWebPortAvailable} from '../../scripts/utils/webDevConfig'; + +const isFree = () => true; describe('web development configuration', () => { test('defaults both services on and allows independent opt-outs', () => { @@ -62,16 +64,43 @@ describe('web development configuration', () => { test('rejects invalid and overlapping ports before starting children', () => { // Given invalid coordinated ports - // When the launcher checks them + // When the launcher resolves them // Then startup fails instead of changing the upstream port behind the Worker's back - expect(() => getWebDevPorts({WEB_PORT: 'invalid'}, true)).toThrow(); - expect(() => getWebDevPorts({WEB_PORT: '1023'}, true)).toThrow(); - expect(() => getWebDevPorts({WEB_PORT: '65536'}, true)).toThrow(); - expect(() => getWebDevPorts({WEB_PORT: '11000', WEB_INTERNAL_PORT: '11000'}, true)).toThrow('must differ'); + expect(() => findWebDevPorts({WEB_PORT: 'invalid'}, true, isFree)).toThrow(); + expect(() => findWebDevPorts({WEB_PORT: '1023'}, true, isFree)).toThrow(); + expect(() => findWebDevPorts({WEB_PORT: '65536'}, true, isFree)).toThrow(); + expect(() => findWebDevPorts({WEB_PORT: '11000', WEB_INTERNAL_PORT: '11000'}, true, isFree)).toThrow('must differ'); + }); + + test('uses the default ports when they are free', () => { + // Given a machine with no other web session + // When the launcher resolves its ports + // Then the app is at its usual origin + expect(findWebDevPorts({}, true, isFree)).toEqual({publicPort: 8082, internalPort: 8083}); + }); + + test('moves a second session to the next free ports rather than failing', () => { + // Given another worktree's session on the default ports + const busy = new Set([8082, 8083, 8085]); + // When this session resolves its ports + // Then it takes the next free ones, as portfinder did before the Worker + expect(findWebDevPorts({}, true, (port) => !busy.has(port))).toEqual({publicPort: 8084, internalPort: 8086}); + expect(findWebDevPorts({}, false, (port) => !busy.has(port)).publicPort).toBe(8084); }); - test('fails clearly rather than reusing an occupied worktree origin', () => { - // Given another server owns a port + test('fails fast on a busy port the user chose explicitly', () => { + // Given explicitly requested ports that another server owns + const busy = new Set([9100, 9201]); + // When the launcher resolves them + // Then it says which one is taken rather than silently moving the session + expect(() => findWebDevPorts({WEB_PORT: '9100'}, true, (port) => !busy.has(port))).toThrow('Port 9100 is in use'); + expect(() => findWebDevPorts({WEB_PORT: '9200', WEB_INTERNAL_PORT: '9201'}, true, (port) => !busy.has(port))).toThrow('Port 9201 is in use'); + // And a free explicit choice is used as is, with the other port found around it + expect(findWebDevPorts({WEB_INTERNAL_PORT: '8082'}, true, isFree)).toEqual({publicPort: 8083, internalPort: 8082}); + }); + + test('detects a port another server owns, and releases its own probe', () => { + // Given another server on one port const listener = Bun.listen({ hostname: '127.0.0.1', port: 0, @@ -82,40 +111,22 @@ describe('web development configuration', () => { }, }); try { - // When this session resolves and checks its ports - // Then resolving them stays side-effect free, and the availability check stops startup before any child starts - expect(getWebDevPorts({WEB_PORT: String(listener.port)}, false).publicPort).toBe(listener.port); - expect(() => assertWebPortAvailable(listener.port)).toThrow('in use'); + // When the launcher probes it, and a port it freed itself + // Then the owned port is unavailable + expect(isWebPortAvailable(listener.port)).toBe(false); } finally { listener.stop(true); } - }); - - test('releases the availability probe so the selected port can be bound', () => { - // Given an available local port - const initial = Bun.listen({ + // And once free, the probe does not keep it from the session's own server + expect(isWebPortAvailable(listener.port)).toBe(true); + Bun.listen({ hostname: '127.0.0.1', - port: 0, + port: listener.port, socket: { data: (socket) => { socket.end(); }, }, - }); - const port = initial.port; - initial.stop(true); - // When a session probes that port - assertWebPortAvailable(port); - // Then its actual server can bind the port immediately - const listener = Bun.listen({ - hostname: '127.0.0.1', - port, - socket: { - data: (socket) => { - socket.end(); - }, - }, - }); - listener.stop(true); + }).stop(true); }); });