Persist AI Gateway default passphrase security banner dismissal to localStorage#21292
Persist AI Gateway default passphrase security banner dismissal to localStorage#21292TomeHirata merged 3 commits intomasterfrom
Conversation
Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Persists dismissal state for the AI Gateway “default KEK passphrase” security warning banner so it remains dismissed across navigation by storing the state in localStorage.
Changes:
- Read banner dismissed state from
localStorageon mount and persist dismissal on close. - Add unit/integration tests covering render, dismissal persistence, and suppression when previously dismissed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| mlflow/server/js/src/gateway/components/DefaultPassphraseBanner.tsx | Persist banner dismissal state to localStorage and use a memoized close handler. |
| mlflow/server/js/src/gateway/components/DefaultPassphraseBanner.test.tsx | Add tests validating rendering and localStorage-backed dismissal behavior. |
| const [isDismissed, setIsDismissed] = useState( | ||
| () => localStorage.getItem(DEFAULT_PASSPHRASE_BANNER_DISMISSED_KEY) === 'true', | ||
| ); | ||
|
|
||
| const handleClose = () => { | ||
| const handleClose = useCallback(() => { | ||
| localStorage.setItem(DEFAULT_PASSPHRASE_BANNER_DISMISSED_KEY, 'true'); | ||
| setIsDismissed(true); |
There was a problem hiding this comment.
🟡 MODERATE: localStorage.getItem/setItem are called unguarded during render and in the close handler. In environments where storage is unavailable/blocked (e.g., browser privacy settings), these can throw and crash the banner (and potentially the page). Consider wrapping the reads/writes in try/catch or using the existing useLocalStorage helper (which already handles failures) to make dismissal persistence non-fatal.
| const [isDismissed, setIsDismissed] = useState( | ||
| () => localStorage.getItem(DEFAULT_PASSPHRASE_BANNER_DISMISSED_KEY) === 'true', | ||
| ); |
There was a problem hiding this comment.
can we use useLocalStorage hook instead? i discovered during backsync that direct localstorage access is banned in universe. i intend to align the lint rules some time in the next sprint
There was a problem hiding this comment.
Switched to useLocalStorage hook in 76d2112. It handles storage failures gracefully and follows the project conventions.
… localStorage access Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
…calStorage (mlflow#21292) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
…calStorage (mlflow#21292) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
…calStorage (#21292) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: TomeHirata <33407409+TomeHirata@users.noreply.github.com>
The security warning banner shown when AI Gateway uses the default KEK passphrase reappears on every navigation back to the gateway tab because dismissed state was stored only in component-local React state (
useState(false)), which resets on unmount.What changes are proposed in this pull request?
DefaultPassphraseBanner.tsx: Use the project'suseLocalStoragehook to initialize and persist dismissed state, with keymlflow.gateway.default-passphrase-warning.dismissed(stored asmlflow.gateway.default-passphrase-warning.dismissed_v1after versioning). This handles storage failures gracefully (e.g., when localStorage is blocked by browser privacy settings) and follows project conventions that ban directlocalStorageaccess.DefaultPassphraseBanner.test.tsx(new): Tests for banner render, localStorage persistence on dismiss, and suppression when already dismissed.How is this PR tested?
Does this PR require documentation update?
Does this PR require updating the MLflow Skills repository?
Release Notes
Is this a user-facing change?
AI Gateway security warning banner for default KEK passphrase now stays dismissed across navigation once a user closes it.
What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/gateway: MLflow AI Gateway client APIs, server, and third-party integrationsarea/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev serverHow should the PR be classified in the release notes? Choose one:
rn/bug-fix- A user-facing bug fix worth mentioning in the release notesShould this PR be included in the next patch release?
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.