🔐 feat: Use SecretInput for Sensitive Fields - #12955
Conversation
🚨 Unused i18next Keys DetectedThe following translation keys are defined in
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes handling of sensitive/secret fields across the client UI by adopting the shared SecretInput component (reveal + optional copy) in auth flows and settings dialogs, while keeping non-sensitive fields on the standard Input.
Changes:
- Enhanced
SecretInputstyling and added optional internal label support (for floating-label auth forms) with generic “Show secret/Hide secret” reveal labels. - Migrated multiple sensitive inputs (auth passwords, agent API key creation display, 2FA secret display, MCP custom user vars, builder action auth fields, and web-search provider key inputs) to use
SecretInput. - Updated Set Key dialog inputs to support a new
secretflag, usingSecretInputonly for key/secret fields.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client/src/components/SecretInput.tsx | Adds label support, updates border class, and standardizes reveal aria-label text. |
| client/src/components/SidePanel/Builder/ActionsAuth.tsx | Uses SecretInput for API key / OAuth client id/secret fields and refactors conditional rendering. |
| client/src/components/SidePanel/Agents/Search/InputSection.tsx | Replaces custom password-visibility logic with SecretInput for secret inputs. |
| client/src/components/Nav/SettingsTabs/Data/AgentApiKeys.tsx | Uses SecretInput to reveal/copy newly created agent API keys. |
| client/src/components/Nav/SettingsTabs/Account/TwoFactorPhases/QRPhase.tsx | Uses SecretInput to reveal/copy the 2FA secret. |
| client/src/components/MCP/CustomUserVarsSection.tsx | Uses SecretInput for MCP auth vars and improves react-hook-form typing. |
| client/src/components/MCP/tests/CustomUserVarsSection.test.tsx | Updates expectations to match SecretInput behavior (type="password"). |
| client/src/components/Input/SetKeyDialog/OtherConfig.tsx | Marks “Other” endpoint key input as secret via secret prop. |
| client/src/components/Input/SetKeyDialog/OpenAIConfig.tsx | Marks OpenAI/Azure API key inputs as secret via secret prop. |
| client/src/components/Input/SetKeyDialog/InputWithLabel.tsx | Adds secret prop and renders SecretInput when enabled; fixes sublabel rendering. |
| client/src/components/Input/SetKeyDialog/GoogleConfig.tsx | Marks Google API key input as secret via secret prop. |
| client/src/components/Input/SetKeyDialog/CustomEndpoint.tsx | Marks custom endpoint API key input as secret via secret prop. |
| client/src/components/Auth/ResetPassword.tsx | Migrates password + confirm password fields to SecretInput while preserving floating-label styling. |
| client/src/components/Auth/Registration.tsx | Uses SecretInput for password fields and keeps floating-label behavior. |
| client/src/components/Auth/LoginForm.tsx | Migrates login password field to SecretInput with floating label. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {label != null && ( | ||
| <label htmlFor={id} className={cn(labelClassName ?? '')}> | ||
| {label} | ||
| </label> | ||
| )} |
| @@ -24,6 +24,7 @@ const OpenAIConfig = ({ | |||
| label={`${isAzure ? 'Azure q' : ''}OpenAI API Key`} | |||
🚨 Unused i18next Keys DetectedThe following translation keys are defined in
|
|
@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: f4814359cb
ℹ️ 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".
| <SecretInput | ||
| placeholder={config.placeholder} | ||
| autoComplete="one-time-code" | ||
| data-lpignore="true" | ||
| data-1p-ignore="true" | ||
| controlsOnHover | ||
| className="mb-2" | ||
| {...register(name as keyof SearchApiKeyFormData)} | ||
| /> |
There was a problem hiding this comment.
Replacing the password field with SecretInput here removes the previously localized toggle labels (com_ui_show_password / com_ui_hide_password) and falls back to SecretInput’s hardcoded English aria-labels (Show secret / Hide secret). In non-English locales this regresses screen-reader UX for this form, so this call site should pass localized labels (or SecretInput should localize them internally) to keep accessibility text translated.
Useful? React with 👍 / 👎.
|
Thanks for this PR. One downstream integration note: MCP With this change, all dynamic MCP variables render as Would you be open to an optional per-field hint in |
|
@berry-13 I think @jumasheff's comment is worth implementing before merge |
agree, working on this now |
The wrapper was a flex container, so passing 'mb-2' on the input made it contribute its margin to the wrapper's cross-axis size — the controls overlay spanned the inflated height and centered the toggle 4px below the input's true center. Switching the wrapper to a plain relative block collapses height back to the input. Also tightens the toggle/copy buttons (size-7 rounded-md with hover:bg-surface-hover) and adds a focus ring on the input. Auth pages still override className/buttonClassName so login/register styling is unchanged.
SecretInput's modernized default uses focus-visible:border-border-heavy and hover:border-border-medium, which Tailwind emits after the auth pages' focus: rules and overrides them. Auth pages now also declare focus-visible:border-green-500 and hover:border-border-light so cn()/twMerge resolves them as the winners when classes are concatenated.
Dynamic MCP credential fields all rendered as masked SecretInputs, which also hid non-secret setup values like usernames, project keys, and URLs. Add an optional `sensitive` flag to customUserVars and the plugin auth config. It defaults to masked when omitted, so existing configs keep the safe-by-default behavior; set `sensitive: false` to render a field as plain text. The flag is display-only — values remain encrypted at rest.
61406e3 to
6474c84
Compare
|
@codex review |
danny-avila
changed the title
SecretInput for Sensitive Fields
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ 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". |
* feat: use SecretInput for sensitive fields * fix: align auth SecretInput styles * chore: remove unused password i18n keys * fix: align SecretInput controls * fix: use SecretInput for dynamic credentials * fix: reveal SecretInput controls on hover * fix: align SecretInput eye icon and modernize controls The wrapper was a flex container, so passing 'mb-2' on the input made it contribute its margin to the wrapper's cross-axis size — the controls overlay spanned the inflated height and centered the toggle 4px below the input's true center. Switching the wrapper to a plain relative block collapses height back to the input. Also tightens the toggle/copy buttons (size-7 rounded-md with hover:bg-surface-hover) and adds a focus ring on the input. Auth pages still override className/buttonClassName so login/register styling is unchanged. * fix: remove focus ring from SecretInput * fix: keep green focus border on auth secret inputs SecretInput's modernized default uses focus-visible:border-border-heavy and hover:border-border-medium, which Tailwind emits after the auth pages' focus: rules and overrides them. Auth pages now also declare focus-visible:border-green-500 and hover:border-border-light so cn()/twMerge resolves them as the winners when classes are concatenated. * feat: add optional sensitive flag to MCP customUserVars Dynamic MCP credential fields all rendered as masked SecretInputs, which also hid non-secret setup values like usernames, project keys, and URLs. Add an optional `sensitive` flag to customUserVars and the plugin auth config. It defaults to masked when omitted, so existing configs keep the safe-by-default behavior; set `sensitive: false` to render a field as plain text. The flag is display-only — values remain encrypted at rest.
* feat: use SecretInput for sensitive fields * fix: align auth SecretInput styles * chore: remove unused password i18n keys * fix: align SecretInput controls * fix: use SecretInput for dynamic credentials * fix: reveal SecretInput controls on hover * fix: align SecretInput eye icon and modernize controls The wrapper was a flex container, so passing 'mb-2' on the input made it contribute its margin to the wrapper's cross-axis size — the controls overlay spanned the inflated height and centered the toggle 4px below the input's true center. Switching the wrapper to a plain relative block collapses height back to the input. Also tightens the toggle/copy buttons (size-7 rounded-md with hover:bg-surface-hover) and adds a focus ring on the input. Auth pages still override className/buttonClassName so login/register styling is unchanged. * fix: remove focus ring from SecretInput * fix: keep green focus border on auth secret inputs SecretInput's modernized default uses focus-visible:border-border-heavy and hover:border-border-medium, which Tailwind emits after the auth pages' focus: rules and overrides them. Auth pages now also declare focus-visible:border-green-500 and hover:border-border-light so cn()/twMerge resolves them as the winners when classes are concatenated. * feat: add optional sensitive flag to MCP customUserVars Dynamic MCP credential fields all rendered as masked SecretInputs, which also hid non-secret setup values like usernames, project keys, and URLs. Add an optional `sensitive` flag to customUserVars and the plugin auth config. It defaults to masked when omitted, so existing configs keep the safe-by-default behavior; set `sensitive: false` to render a field as plain text. The flag is display-only — values remain encrypted at rest.
Summary
Adopts the shared
SecretInputacross every place we render a sensitive value in the client (API keys, OAuth client secrets, MCP variables, 2FA seed, generated agent keys, auth password fields), then irons out the styling so the component looks the same everywhere it appears.Closes #11801. Builds on #11582.
What's in here
SecretInput rollout
SecretInputin the Set Key dialogs (Custom, Google, OpenAI, Other), MCP custom user vars + MCP server config dialog, MCP builder auth section, agent builder Action auth (basic + bearer + custom OAuth fields), web-search provider keys (Search, Scraper, Reranker), Plugin store auth form, generated agent API keys list, and the 2FA secret display.SecretInputso the reveal/copy controls match the rest of the app.Inputso the reveal toggle isn't shown where it makes no sense.Component polish
border-border-light; addedtransition-colors,hover:border-border-medium, andfocus-visible:border-border-heavyfor clearer keyboard / pointer feedback. Auth pages opt back intofocus:border-green-500+focus-visible:border-green-500so the green focus border is preserved.size-7 rounded-mdwithhover:bg-surface-hover, giving a visible affordance on hover and a tighter shape that fits inside theh-10input.controlsOnHover. Wired into the side-panel and dialog usages where the row is otherwise visually busy.aria-labels ("Show secret" / "Hide secret" / "Copy to clipboard"),autoComplete="off", andspellCheck={false}by default.Alignment fix
flex items-center; flex containers include child margins in the cross-axis size, so passingclassName="mb-2"(e.g.InputSection.tsx) made the wrapper 48px while the input was 40px. The absolutely-positioned controls inherited the inflated 48px and centered the eye 4px below the input's true center. Wrapper is now a plainrelativeblock — height collapses back to the input and the icon sits dead-center. Verified via DOM measurement: button center Y matches input center Y exactly (was off by 4px).Files touched
packages/client/src/components/SecretInput.tsx,client/src/components/Auth/{LoginForm,Registration,ResetPassword}.tsx,client/src/components/Chat/Input/MCPConfigDialog.tsx,client/src/components/Input/SetKeyDialog/{InputWithLabel,CustomEndpoint,GoogleConfig,OpenAIConfig,OtherConfig}.tsx,client/src/components/MCP/CustomUserVarsSection.tsx,client/src/components/Nav/SettingsTabs/Account/TwoFactorPhases/QRPhase.tsx,client/src/components/Nav/SettingsTabs/Data/AgentApiKeys.tsx,client/src/components/Plugins/Store/PluginAuthForm.tsx,client/src/components/SidePanel/Agents/Search/InputSection.tsx,client/src/components/SidePanel/Builder/ActionsAuth.tsx,client/src/components/SidePanel/MCPBuilder/MCPServerDialog/sections/AuthSection.tsx, plus the unused password i18n keys removed fromclient/src/locales/en/translation.json.Change Type
Testing
Manual pass against the dev frontend (Vite on
:3090with the local backend on:3080), driving the UI through Chromium. For each affected surface I checked: empty state, with a value, focus, hover, reveal toggle, and copy (where applicable), in both light and dark mode.Surfaces exercised:
Show secret/Hide secrettoggle).Updated unit tests:
packages/client/src/components/MCP/__tests__/CustomUserVarsSection.test.tsxclient/src/components/Plugins/Store/__tests__/PluginAuthForm.spec.tsxTest Configuration
:3090), Express backend (:3080)Checklist