Skip to content

Studio: gate change-password submit on current_password length#5738

Closed
danielhanchen wants to merge 3 commits into
mainfrom
fix/change-password-submit-gating
Closed

Studio: gate change-password submit on current_password length#5738
danielhanchen wants to merge 3 commits into
mainfrom
fix/change-password-submit-gating

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Summary

  • The change-password form's submit disable gate checks new password length, confirm equality, and current != new, but skipped a current_password length check. When __UNSLOTH_BOOTSTRAP__ is injected this is harmless (currentPassword falls back to a 16+ char bootstrap value), but on the admin-forced must_change_password path no bootstrap is published, the "Current password" input is rendered, and the gate let users submit with an empty current_password.
  • The submit handler then rejected it with "Unable to initialize setup. Reload the page and try again." -- a misleading error that hints at a backend bootstrap problem when the real cause is just an empty input. Surfaced while running the autonomous Playwright probe against an isolated UNSLOTH_STUDIO_HOME build.

Fix

  • Add currentPassword.length < 8 to invalidChangePasswordForm in studio/frontend/src/features/auth/components/auth-form.tsx. The new gate is a strict superset of the submit handler's own validation, so the noisy reload-error path becomes unreachable.
  • Common bootstrap-injection flow unchanged: bootstrap pws are 16+ chars, so currentPassword.length < 8 is always false there.

Test plan

  • cd studio/frontend && npm run typecheck clean
  • On a fresh install (bootstrap injected): change-password form behaves identically -- type new + confirm (>= 8 chars), button enables, submit succeeds
  • On an admin-forced must_change_password=true reset (no bootstrap): "Current password" input rendered, button stays disabled until that field has 8+ chars, no more "Unable to initialize setup" error

The change-password form's submit-button disable logic checks new
password length, confirm equality and current!=new, but skipped a
current-password length check. When __UNSLOTH_BOOTSTRAP__ is injected
that is harmless -- currentPassword falls back to a 16+ char bootstrap
value -- but on the admin-forced must_change_password path no bootstrap
is published, the "Current password" input is rendered, and the gate
let users submit with an empty current_password. The submit handler
then rejected it with "Unable to initialize setup. Reload the page and
try again." -- a misleading error that suggests a backend bootstrap
problem.

Add the missing currentPassword.length < 8 check so the button stays
disabled until every required field is filled. The new gate is a strict
superset of the submit handler's own validation, so the noisy reload
error path becomes unreachable.

Common bootstrap-injection path is unaffected: bootstrap pws are 16+
chars, so currentPassword.length < 8 is always false there.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the password change form validation logic in auth-form.tsx to include a minimum length requirement of 8 characters for the current password field. I have no further feedback to provide as there were no review comments to evaluate.

…ndler

Reviewer round on the original PR flagged that the disable-gate update
left the submit handler asymmetric: the button predicate now requires
currentPassword.length >= 8, but handleSubmit still only does
`if (!currentPassword)`. The button is normally kept disabled in this
state, but native form submission (Enter on a focused input, browser
autofill replaying a form post) can bypass the disabled attribute and
reach handleSubmit with a 1-7 char current_password, which the backend
schema then rejects with a noisier 422.

Tighten the submit handler to the same length check so both validation
surfaces stay in lockstep. Empty input still surfaces the original
"Unable to initialize setup" reload-prompt error; short non-empty input
surfaces a more accurate "must be at least 8 characters" message.
@danielhanchen

Copy link
Copy Markdown
Member Author

Pushed 8091ea7 addressing one P1 from a fresh reviewer.py pass. Asymmetric-fix bug: the disable-gate update made the button require currentPassword.length >= 8, but handleSubmit still only did if (!currentPassword). The button is normally kept disabled in this state, but native form submission (Enter on a focused input, autofill replaying a form post) can bypass the disabled attribute and reach the handler with a 1-7 char current_password, which the backend schema then rejects with a noisier 422.

Tightened the submit handler to the same length check so both validation surfaces stay in lockstep. Empty input still surfaces the original "Unable to initialize setup" reload-prompt error; short non-empty input now surfaces a more accurate "must be at least 8 characters" message.

npm run typecheck clean.

danielhanchen added a commit that referenced this pull request May 23, 2026
Bundles three independent CI regressions hitting the maintainer PR
backlog. Each one is verified end-to-end on a staging fork against
real Ubuntu / macOS / Windows GitHub-hosted runners before this
lands.

1. Windows --no-torch install: pydantic + pydantic-core drift to
   incompatible versions under `uv pip install --no-deps -r
   no-torch-runtime.txt` because pip resolves each independently
   from latest. pydantic.VERSION 2.13.4 pins pydantic-core==2.46.4
   but pydantic-core 2.47.0 was the freshest published wheel, so
   `import pydantic` raised
   `SystemError: pydantic-core 2.47.0 is incompatible with the
   current pydantic version`. Resolve pydantic WITH deps in a
   focused pip call (install.sh, install.ps1,
   install_python_stack.py) before the --no-deps no-torch-runtime
   pass so pip pins pydantic-core to the version pydantic declares.
   pydantic's transitive deps (annotated-types, pydantic-core,
   typing-extensions, typing-inspection) are torch-free. Drop the
   redundant `Patch Studio venv with full typer / pydantic dep
   trees` workaround from the four Windows smoke YAMLs.
   Supersedes #5733 + #5734.

2. Linux Studio Update CI: upstream llama.cpp b9261+ split each
   binary's entry code into a paired `libllama-<binary>-impl.so`
   shared library. `llama-server` and `llama-quantize` NEEDED-link
   against `libllama-server-impl.so` / `libllama-quantize-impl.so`
   with RUNPATH `$ORIGIN`, so the prebuilt overlay must copy those
   alongside the binaries. Without that, ldd reports them missing,
   preflight rejects, the installer falls back to source build, and
   studio-update-smoke annotates `setup.sh idempotency regressed`.
   Add `libllama-*-impl.so*` to the Linux runtime patterns and lock
   the pattern in test_rocm_support.TestRuntimePatterns.

3. Mac Studio UI Chat: change-password submit clicked while
   disabled. The disable gate only checked new + confirm password
   length, but Playwright's first click landed before the
   current-password field's React state had committed, so the form
   was simultaneously logically-invalid (current_password empty) and
   the button was disabled. Tighten the gate to require
   `currentPassword.length >= 8` and mirror the same check in the
   submit handler so Enter / autofill cannot bypass.
   Supersedes #5738.
@danielhanchen

Copy link
Copy Markdown
Member Author

Superseded by #5741 (now merged). The pydantic-core mismatch, llama.cpp libllama-*-impl.so* overlay, change-password gate, and Windows-smoke workaround cleanup all landed in that consolidated PR.

@danielhanchen danielhanchen deleted the fix/change-password-submit-gating branch May 23, 2026 13:59
rsd-darshan pushed a commit to rsd-darshan/unsloth that referenced this pull request Jun 3, 2026
Bundles three independent CI regressions hitting the maintainer PR
backlog. Each one is verified end-to-end on a staging fork against
real Ubuntu / macOS / Windows GitHub-hosted runners before this
lands.

1. Windows --no-torch install: pydantic + pydantic-core drift to
   incompatible versions under `uv pip install --no-deps -r
   no-torch-runtime.txt` because pip resolves each independently
   from latest. pydantic.VERSION 2.13.4 pins pydantic-core==2.46.4
   but pydantic-core 2.47.0 was the freshest published wheel, so
   `import pydantic` raised
   `SystemError: pydantic-core 2.47.0 is incompatible with the
   current pydantic version`. Resolve pydantic WITH deps in a
   focused pip call (install.sh, install.ps1,
   install_python_stack.py) before the --no-deps no-torch-runtime
   pass so pip pins pydantic-core to the version pydantic declares.
   pydantic's transitive deps (annotated-types, pydantic-core,
   typing-extensions, typing-inspection) are torch-free. Drop the
   redundant `Patch Studio venv with full typer / pydantic dep
   trees` workaround from the four Windows smoke YAMLs.
   Supersedes unslothai#5733 + unslothai#5734.

2. Linux Studio Update CI: upstream llama.cpp b9261+ split each
   binary's entry code into a paired `libllama-<binary>-impl.so`
   shared library. `llama-server` and `llama-quantize` NEEDED-link
   against `libllama-server-impl.so` / `libllama-quantize-impl.so`
   with RUNPATH `$ORIGIN`, so the prebuilt overlay must copy those
   alongside the binaries. Without that, ldd reports them missing,
   preflight rejects, the installer falls back to source build, and
   studio-update-smoke annotates `setup.sh idempotency regressed`.
   Add `libllama-*-impl.so*` to the Linux runtime patterns and lock
   the pattern in test_rocm_support.TestRuntimePatterns.

3. Mac Studio UI Chat: change-password submit clicked while
   disabled. The disable gate only checked new + confirm password
   length, but Playwright's first click landed before the
   current-password field's React state had committed, so the form
   was simultaneously logically-invalid (current_password empty) and
   the button was disabled. Tighten the gate to require
   `currentPassword.length >= 8` and mirror the same check in the
   submit handler so Enter / autofill cannot bypass.
   Supersedes unslothai#5738.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant