Studio: stop embedding the seeded bootstrap password in the served page#7140
Studio: stop embedding the seeded bootstrap password in the served page#7140danielhanchen wants to merge 9 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the dynamic injection of the seeded bootstrap password into the served index.html page, simplifying the frontend setup and eliminating several helper functions and tests related to origin and loopback checks. A new test is added to verify that the bootstrap password is never embedded in the served page. The reviewer suggested optimizing _build_index_response by pre-reading and caching the static index.html content during initialization to avoid redundant disk I/O and CPU overhead on every request.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _build_index_response(request: Request) -> Response: | ||
| content = (build_path / "index.html").read_bytes() | ||
| content = _strip_crossorigin(content) | ||
| # Bootstrap pw goes only to a same-origin, direct-loopback client (or | ||
| # Colab's single-user notebook proxy): a wildcard bind must not serve it | ||
| # in-page to a LAN or proxied peer. Vary: Origin keeps caches honest. | ||
| if _should_inject_bootstrap(request): | ||
| content, nonce = _inject_bootstrap(content, app) | ||
| else: | ||
| nonce = None | ||
| headers = { | ||
| "Cache-Control": "no-cache, no-store, must-revalidate", | ||
| "Vary": "Origin", | ||
| } | ||
| if nonce: | ||
| headers[_CSP_SCRIPT_NONCE_HEADER] = nonce | ||
| # The seeded bootstrap password is never embedded in the served page: a | ||
| # loopback reverse proxy is indistinguishable from a genuine local client, | ||
| # so it cannot be scoped safely. It is printed at startup and saved to | ||
| # .bootstrap_password; the operator copies it from there on first login. | ||
| content = _strip_crossorigin((build_path / "index.html").read_bytes()) | ||
| return Response( | ||
| content = content, | ||
| media_type = "text/html", | ||
| headers = headers, | ||
| headers = {"Cache-Control": "no-cache, no-store, must-revalidate"}, | ||
| ) |
There was a problem hiding this comment.
Since the seeded bootstrap password is no longer dynamically injected into index.html on a per-request basis, the served HTML content is now completely static. Reading index.html from disk and performing regex substitution via _strip_crossorigin on every single request (including all SPA fallback routes) introduces redundant disk I/O and CPU overhead.
We can optimize this by pre-reading and processing the index.html content once during setup_frontend initialization, and then serving the cached bytes in _build_index_response.
| def _build_index_response(request: Request) -> Response: | |
| content = (build_path / "index.html").read_bytes() | |
| content = _strip_crossorigin(content) | |
| # Bootstrap pw goes only to a same-origin, direct-loopback client (or | |
| # Colab's single-user notebook proxy): a wildcard bind must not serve it | |
| # in-page to a LAN or proxied peer. Vary: Origin keeps caches honest. | |
| if _should_inject_bootstrap(request): | |
| content, nonce = _inject_bootstrap(content, app) | |
| else: | |
| nonce = None | |
| headers = { | |
| "Cache-Control": "no-cache, no-store, must-revalidate", | |
| "Vary": "Origin", | |
| } | |
| if nonce: | |
| headers[_CSP_SCRIPT_NONCE_HEADER] = nonce | |
| # The seeded bootstrap password is never embedded in the served page: a | |
| # loopback reverse proxy is indistinguishable from a genuine local client, | |
| # so it cannot be scoped safely. It is printed at startup and saved to | |
| # .bootstrap_password; the operator copies it from there on first login. | |
| content = _strip_crossorigin((build_path / "index.html").read_bytes()) | |
| return Response( | |
| content = content, | |
| media_type = "text/html", | |
| headers = headers, | |
| headers = {"Cache-Control": "no-cache, no-store, must-revalidate"}, | |
| ) | |
| try: | |
| index_content = _strip_crossorigin((build_path / "index.html").read_bytes()) | |
| except Exception: | |
| index_content = b"" | |
| def _build_index_response(request: Request) -> Response: | |
| # The seeded bootstrap password is never embedded in the served page: a | |
| # loopback reverse proxy is indistinguishable from a genuine local client, | |
| # so it cannot be scoped safely. It is printed at startup and saved to | |
| # .bootstrap_password; the operator copies it from there on first login. | |
| return Response( | |
| content = index_content, | |
| media_type = "text/html", | |
| headers = {"Cache-Control": "no-cache, no-store, must-revalidate"}, | |
| ) |
There was a problem hiding this comment.
Valid observation, but keeping the per-request read is deliberate here. It is a small static SPA shell served infrequently, so the read plus one regex is negligible; reading per request also means a rebuilt index.html is picked up without a restart, and a transient read error is not cached into a permanently empty page (as the suggested except-to-empty-bytes would do). This change is scoped to removing the seed injection, so I am leaving the serve path unchanged. Happy to revisit caching separately if it ever shows up in a profile.
|
@codex review |
1 similar comment
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55ef7a1fe1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # The seeded bootstrap password is never embedded in the served page: a | ||
| # loopback reverse proxy is indistinguishable from a genuine local client, | ||
| # so it cannot be scoped safely. It is printed at startup and saved to | ||
| # .bootstrap_password; the operator copies it from there on first login. | ||
| content = _strip_crossorigin((build_path / "index.html").read_bytes()) |
There was a problem hiding this comment.
In first-run headless, Colab, or container launches where the operator relies on stdout, this removes the last browser-delivered copy of the seeded password, but the existing startup banner only prints the path to .bootstrap_password (main.py lines 565-570), not the secret itself. Since the change-password form now requires that current password to be entered manually, these users cannot complete setup from logs alone; either print the seed as claimed or surface a clear retrieval path before withholding it from the page.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. The banner only printed the .bootstrap_password path, not the secret. Fixed in 3e03c40: the first-run banner now prints the actual seed to stdout, with the file path kept as a fallback, so headless, Colab, and container operators can complete first login from logs alone. It is intentionally not gated on isatty, since Colab and container stdout is often not a TTY. The seed still never reaches the served page, which is the point of this change.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e03c40161
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| print("DEFAULT ADMIN ACCOUNT CREATED") | ||
| print(f" username: {storage.DEFAULT_ADMIN_USERNAME}") | ||
| print(f" password saved to: {bootstrap_path}") | ||
| print(f" password: {bootstrap_pw}") |
There was a problem hiding this comment.
In deployments where stdout is collected by systemd, Docker, notebook logs, or shared support tooling, this line writes the initial admin credential into durable logs that may be readable beyond the local operator. Since the password is already persisted in .bootstrap_password with restricted permissions and removed after rotation, printing the raw value reintroduces a bootstrap-secret exposure outside the served page; keep the banner path-only or require the operator to read the file directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, this is the safer of the two options raised earlier. Reverted in 777e4ab: the banner no longer prints the raw seed, since stdout is collected by systemd, Docker, and notebook logs that are less protected than the file (CWE-532). It now points to .bootstrap_password, which is written 0600 and auto-deleted on rotation (update_password calls clear_bootstrap_password), with an explicit instruction to open it. That keeps a clear retrieval path for headless and Colab operators without writing the secret to durable logs.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 777e4abd7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # loopback reverse proxy is indistinguishable from a genuine local client, | ||
| # so it cannot be scoped safely. It is printed at startup and saved to | ||
| # .bootstrap_password; the operator copies it from there on first login. | ||
| content = _strip_crossorigin((build_path / "index.html").read_bytes()) |
There was a problem hiding this comment.
When the fresh-install UI smoke jobs run, this now serves index.html without the bootstrap script, so auth-form.tsx renders a required #current-password field and keeps the submit button disabled until it is filled. The existing Playwright flows still only fill #new-password and #confirm-password before waiting for /api/auth/change-password (e.g. tests/studio/playwright_chat_ui.py:237-252, tests/studio/playwright_extra_ui.py:135-146, tests/studio/playwright_chat_ime_i18n.py:162-172), and .github/workflows/studio-ui-smoke.yml:257-274 explicitly omits the old password for the IME job because it relied on the injected value, so these CI smoke jobs will time out without a POST unless the tests/workflows are updated to provide and fill the current bootstrap password.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 7e11dc6. With the seed no longer in the page, hasBootstrapPassword is always false, so auth-form.tsx always renders the required #current-password field and keeps submit disabled until it has 8+ chars. The three Playwright flows (playwright_chat_ui.py, playwright_extra_ui.py, playwright_chat_ime_i18n.py) now fill #current-password with the bootstrap password when the field is present, and the IME job in studio-ui-smoke.yml now passes STUDIO_OLD_PW, which it previously omitted because it relied on the injected value.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e11dc6f36
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Drive IME + multilingual paste regression with Playwright | ||
| env: | ||
| BASE_URL: http://127.0.0.1:18896 | ||
| STUDIO_OLD_PW: ${{ env.STUDIO_IME_OLD_PW }} |
There was a problem hiding this comment.
This reintroduces STUDIO_OLD_PW for the IME smoke, but tests/studio/test_composer_rtl_bidi_attribute.py still asserts that the IME workflow and script do not export or read that variable. I verified python -m pytest -q tests/studio/test_composer_rtl_bidi_attribute.py now fails three tests, so any CI path running this studio guard suite will fail until those stale assertions are updated alongside the new non-autofill flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. Those three assertions encoded the old autofill invariant (drop STUDIO_OLD_PW as dead). Since this PR removes autofill, the IME flow needs the current password again, so I inverted them in 1a2cb62: the workflow drive step, the pass-password step, and the Playwright script now assert they DO supply STUDIO_OLD_PW. Guard suite is green (23 passed).
| print(f" username: {storage.DEFAULT_ADMIN_USERNAME}") | ||
| print(f" password saved to: {bootstrap_path}") | ||
| print(" Open the Studio UI to sign in and change it.") | ||
| print(" open that file to read the password, then sign in and change it.") |
There was a problem hiding this comment.
When the default admin was created in an earlier run but still has must_change_password=true, storage.ensure_default_admin() returns false, so this banner is skipped; before this change the restarted UI still autofilled from app.state.bootstrap_password, but now the user must manually read .bootstrap_password. In that restart-before-first-login scenario, the startup output no longer tells the operator where to find the current password, so the first-login flow loses its only visible instruction unless they saved the original creation log.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in 70e4821: when ensure_default_admin returns false but the seed is still active (get_bootstrap_password is non-None, since it is only cleared on rotation), the else branch now prints a one-line pointer to .bootstrap_password. It prints the path only, never the secret, so the restart-before-first-login case keeps a visible instruction without reintroducing the CWE-532 exposure.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
# Conflicts: # studio/backend/main.py
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Follow-up to #7131. That change scoped the in-page auto-fill of the seeded admin bootstrap password to loopback clients, but the "is this a local client" detection is fundamentally unreliable behind a same-host reverse proxy. A default
nginx proxy_pass http://127.0.0.1:PORT;forwardsHost: 127.0.0.1:PORTfrom a loopback socket with no forwarding headers, which is byte-for-byte identical to what a genuine local browser sends. No request-based or bind-based check can separate the two, so the seed could still reach a remote user during the pre-first-password-change window.This removes the in-page injection entirely. The seed is already printed to the startup log and saved to
.bootstrap_password, so the operator copies it from there on first login. Removing the injection closes the LAN, tunnel, and reverse-proxy exposure classes in one step and deletes the fragile Origin/loopback/Host detection code.Changes
studio/backend/main.py:_build_index_responsenow serves the SPA page without ever embeddingwindow.__UNSLOTH_BOOTSTRAP__. Removes_inject_bootstrap,_should_inject_bootstrap,_is_local_bootstrap_request,_host_header_is_loopback,_is_loopback_ip,_PROXIED_CLIENT_HEADERS,_is_same_origin_request,_canonical_origin,_DEFAULT_PORTS, and the now-unusedipaddress/urlparseimports.test_index_no_bootstrap_injection.py, which asserts the served page never contains the seed, even with a pending password change and a seed present, via a real TestClient over the root and SPA-fallback routes.Notes
auth-form.tsxalready degrades gracefully whenwindow.__UNSLOTH_BOOTSTRAP__is absent (the change-password form simply is not pre-filled), so no frontend change is required..bootstrap_passwordsave are unchanged.Test plan
studio/backend/tests/test_index_no_bootstrap_injection.pypasses: the seed is absent for the root route, the SPA fallback, and a same-origin request.test_middleware.py,test_security_gate_consistency.py, and the studio auth suites).