Add a usable local Cloud development workflow - #1168
Conversation
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request for security, code quality, performance, duplication, architecture, and end-to-end behavior. |
| "exec", | ||
| "vite", | ||
| "dev", | ||
| "--host", |
There was a problem hiding this comment.
🚨 slopcop/review — P1: The seeded account is open on every network interface.
This listener uses 0.0.0.0, while line 502 enables DEV_AUTH_USER_ID. The auth boundary checks the configured URL, not the request source. A LAN or Tailscale client can change account state without authentication.
Bind seeded mode to 127.0.0.1. Disable seeded auth when remote access is necessary. Add a request test from a non-loopback host.
| { | ||
| BB_CLOUD_DEV_APP_URL: `http://127.0.0.1:${WEB_PORT}`, | ||
| BB_CLOUD_DEV_BASE_DOMAIN: BASE_DOMAIN, | ||
| BB_CLOUD_DEV_CONNECT_SERVER_URL_TEMPLATE: `http://{label}.${BASE_DOMAIN}:${CONNECT_PORT}`, |
There was a problem hiding this comment.
🚨 slopcop/review — P1: The GitHub-auth mode cannot authorize this local gate.
Better Auth creates a host-only cookie for 127.0.0.1. This gate uses <label>.localhost, and the return URL check rejects that different base host. The desktop fallback also installs an __Secure- cookie with secure: false for this HTTP URL, which Chromium rejects.
Use a shared development parent domain or mint a gate-scoped signed session. Add a browser test for the complete --github-auth return flow.
| store, | ||
| shares, | ||
| getLoopbackBaseUrl: () => bb.server.loopbackBaseUrl, | ||
| getLoopbackBaseUrl, |
There was a problem hiding this comment.
🚨 slopcop/review — P1: The account-origin override does not reach machine creation or revocation.
Pairing and the dashboard use this override. However, both machine operations still derive http://localhost:<connect-port> from the gate credential. Their routes run on the separate web port, so both calls fail.
Pass the account origin to both machine helpers. Add tests with different account and gate ports.
| code, | ||
| expiresInMs: CONNECT_CODE_TTL_MS, | ||
| serverUrl: `https://${srv.subdomain}.${baseDomain}`, | ||
| serverUrl: serverUrlForLabel(srv.subdomain, serverUrlTemplate), |
There was a problem hiding this comment.
🚨 slopcop/review — P1: The returned HTTP gate URL cannot complete the local machine lifecycle.
The installer and host daemon still require HTTPS. The daemon always creates a wss: tunnel. Machine share URLs also use HTTPS, and the machine page omits the local port.
Permit HTTP only for validated loopback domains. Derive the HTTP and WebSocket schemes from the trusted gate URL. Add an end-to-end machine test.
| const response = await fetch(url, { | ||
| signal: AbortSignal.timeout(1_000), | ||
| }); | ||
| if (response.status < 500) return; |
There was a problem hiding this comment.
🚨 slopcop/review — P2: An HTTP 500 response causes a tight readiness loop.
The delay only occurs when fetch throws. A fast 500 response causes immediate retries for 30 seconds, and the response body remains unread. This can create thousands of requests and consume sockets.
Consume or cancel each body. Add the 250 ms delay after every result that is not ready. Test a server that always returns 500.
| connectProxy.closeAllConnections(); | ||
| stopService(connect); | ||
| stopService(web); | ||
| setTimeout(() => process.exit(exitCode), 100); |
There was a problem hiding this comment.
🚨 slopcop/review — P2: Startup and shutdown can leave detached services active.
A proxy bind failure occurs after Connect starts but before cleanup exists. Normal shutdown sends SIGTERM and exits the parent after only 100 ms. Wrangler or Vite can keep a port and the shared database open.
Register cleanup before the first spawn. Await both child exits, then use SIGKILL after a deadline. Test bind failure and slow shutdown.
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: This pull request adds one command for local Cloud work. The main server flow starts, but several security and machine paths still fail.
I found six issues:
- P1: Seeded authentication is reachable from LAN and Tailscale clients because Vite listens on every interface.
- P1: The
--github-authmode cannot authorize an HTTP<label>.localhostgate. - P1: Machine creation and revocation call the Connect port instead of the separate account port.
- P1: Machine enrollment, tunnels, shares, and the machine page still require the production HTTPS topology.
- P2: HTTP 500 readiness responses cause a tight retry loop and leave response bodies unread.
- P2: Bind failures and the 100 ms shutdown path can leave detached services active.
The architecture still infers an account origin from a gate origin in several places. Carry both origins explicitly through each contract. Centralize loopback validation so IPv4, IPv6, protocol, and port rules cannot drift.
Validation passed for all affected Turbo tests and type checks. The app ran 2,415 tests, and the desktop ran 231 tests. The Cloud launcher started successfully in a real browser smoke test. The dashboard claimed a handle, generated a pairing code, and rendered the offline gate.
A LAN browser also received the seeded account state without authentication. Chromium rejected the insecure __Secure- desktop cookie for the HTTP gate.
I left a comment-only review, as required.
bbf0b34 to
be8e538
Compare
2 participants
Summary
pnpm cloud:devas the one-command local workflow for the Cloud dashboard and Connect worker<handle>.localhostthrough the Connect worker, including WebSocket tunnel upgradesDeliberate scope
This is only a local Cloud test loop. It does not add a local GitHub OAuth mode, configurable session-cookie names, a new Connect URL configuration surface, or mutation/restoration of a running bb server's environment.
OPENAI_API_KEYis optional and only needed when exercising the AI gateway. Local D1 state is stored under.wrangler/cloud-dev.Verification