fix: stop falsely rejecting owner-password-only PDFs as protected (#9953)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

backend/onyx/db/llm.py

@@ -236,14 +236,15 @@ def upsert_llm_provider(
         db_session.add(existing_llm_provider)
 
     # Filter out empty strings and None values from custom_config to allow
-    # providers like Bedrock to fall back to IAM roles when credentials are not provided
+    # providers like Bedrock to fall back to IAM roles when credentials are not provided.
+    # NOTE: An empty dict ({}) is preserved as-is — it signals that the provider was
+    # created via the custom modal and must be reopened with CustomModal, not a
+    # provider-specific modal. Only None means "no custom config at all".
     custom_config = llm_provider_upsert_request.custom_config
     if custom_config:
         custom_config = {
             k: v for k, v in custom_config.items() if v is not None and v.strip() != ""
         }
-        # Set to None if the dict is empty after filtering
-        custom_config = custom_config or None
 
     api_base = llm_provider_upsert_request.api_base or None
     existing_llm_provider.provider = llm_provider_upsert_request.provider
@@ -303,16 +304,7 @@ def upsert_llm_provider(
         ).delete(synchronize_session="fetch")
         db_session.flush()
 
-    # Import here to avoid circular imports
-    from onyx.llm.utils import get_max_input_tokens
-
     for model_config in llm_provider_upsert_request.model_configurations:
-        max_input_tokens = model_config.max_input_tokens
-        if max_input_tokens is None:
-            max_input_tokens = get_max_input_tokens(
-                model_name=model_config.name,
-                model_provider=llm_provider_upsert_request.provider,
-            )
 
         supported_flows = [LLMModelFlowType.CHAT]
         if model_config.supports_image_input:
@@ -325,7 +317,7 @@ def upsert_llm_provider(
                 model_configuration_id=existing.id,
                 supported_flows=supported_flows,
                 is_visible=model_config.is_visible,
-                max_input_tokens=max_input_tokens,
+                max_input_tokens=model_config.max_input_tokens,
                 display_name=model_config.display_name,
             )
         else:
@@ -335,7 +327,7 @@ def upsert_llm_provider(
                 model_name=model_config.name,
                 supported_flows=supported_flows,
                 is_visible=model_config.is_visible,
-                max_input_tokens=max_input_tokens,
+                max_input_tokens=model_config.max_input_tokens,
                 display_name=model_config.display_name,
             )
 

backend/onyx/file_processing/extract_file_text.py

@@ -205,18 +205,26 @@ def read_pdf_file(
     try:
         pdf_reader = PdfReader(file)
 
-        if pdf_reader.is_encrypted and pdf_pass is not None:
+        if pdf_reader.is_encrypted:
+            # Try the explicit password first, then fall back to an empty
+            # string.  Owner-password-only PDFs (permission restrictions but
+            # no open password) decrypt successfully with "".
+            # See https://github.com/onyx-dot-app/onyx/issues/9754
+            passwords = [p for p in [pdf_pass, ""] if p is not None]
             decrypt_success = False
-            try:
-                decrypt_success = pdf_reader.decrypt(pdf_pass) != 0
-            except Exception:
-                logger.error("Unable to decrypt pdf")
+            for pw in passwords:
+                try:
+                    if pdf_reader.decrypt(pw) != 0:
+                        decrypt_success = True
+                        break
+                except Exception:
+                    pass
 
             if not decrypt_success:
+                logger.error(
+                    "Encrypted PDF could not be decrypted, returning empty text."
+                )
                 return "", metadata, []
-        elif pdf_reader.is_encrypted:
-            logger.warning("No Password for an encrypted PDF, returning empty text.")
-            return "", metadata, []
 
         # Basic PDF metadata
         if pdf_reader.metadata is not None:

backend/onyx/file_processing/password_validation.py

@@ -33,8 +33,20 @@ def is_pdf_protected(file: IO[Any]) -> bool:
 
     with preserve_position(file):
         reader = PdfReader(file)
+        if not reader.is_encrypted:
+            return False
 
-    return bool(reader.is_encrypted)
+        # PDFs with only an owner password (permission restrictions like
+        # print/copy disabled) use an empty user password — any viewer can open
+        # them without prompting.  decrypt("") returns 0 only when a real user
+        # password is required.  See https://github.com/onyx-dot-app/onyx/issues/9754
+        try:
+            return reader.decrypt("") == 0
+        except Exception:
+            logger.exception(
+                "Failed to evaluate PDF encryption; treating as password protected"
+            )
+            return True
 
 
 def is_docx_protected(file: IO[Any]) -> bool:

backend/onyx/server/manage/llm/models.py

@@ -79,7 +79,9 @@ class LLMProviderDescriptor(BaseModel):
             provider=provider,
             provider_display_name=get_provider_display_name(provider),
             model_configurations=filter_model_configurations(
-                llm_provider_model.model_configurations, provider
+                llm_provider_model.model_configurations,
+                provider,
+                use_stored_display_name=llm_provider_model.custom_config is not None,
             ),
         )
 
@@ -156,7 +158,9 @@ class LLMProviderView(LLMProvider):
             personas=personas,
             deployment_name=llm_provider_model.deployment_name,
             model_configurations=filter_model_configurations(
-                llm_provider_model.model_configurations, provider
+                llm_provider_model.model_configurations,
+                provider,
+                use_stored_display_name=llm_provider_model.custom_config is not None,
             ),
         )
 
@@ -198,13 +202,13 @@ class ModelConfigurationView(BaseModel):
         cls,
         model_configuration_model: "ModelConfigurationModel",
         provider_name: str,
+        use_stored_display_name: bool = False,
     ) -> "ModelConfigurationView":
-        # For dynamic providers (OpenRouter, Bedrock, Ollama), use the display_name
-        # stored in DB from the source API. Skip LiteLLM parsing entirely.
+        # For dynamic providers (OpenRouter, Bedrock, Ollama) and custom-config
+        # providers, use the display_name stored in DB. Skip LiteLLM parsing.
         if (
-            provider_name in DYNAMIC_LLM_PROVIDERS
-            and model_configuration_model.display_name
-        ):
+            provider_name in DYNAMIC_LLM_PROVIDERS or use_stored_display_name
+        ) and model_configuration_model.display_name:
             # Extract vendor from model name for grouping (e.g., "Anthropic", "OpenAI")
             vendor = extract_vendor_from_model_name(
                 model_configuration_model.name, provider_name

backend/onyx/server/manage/llm/utils.py

@@ -308,12 +308,15 @@ def should_filter_as_dated_duplicate(
 def filter_model_configurations(
     model_configurations: list,
     provider: str,
+    use_stored_display_name: bool = False,
 ) -> list:
     """Filter out obsolete and dated duplicate models from configurations.
 
     Args:
         model_configurations: List of ModelConfiguration DB models
         provider: The provider name (e.g., "openai", "anthropic")
+        use_stored_display_name: If True, prefer the display_name stored in the
+            DB over LiteLLM enrichments. Set for custom-config providers.
 
     Returns:
         List of ModelConfigurationView objects with obsolete/duplicate models removed
@@ -333,7 +336,9 @@ def filter_model_configurations(
         if should_filter_as_dated_duplicate(model_configuration.name, all_model_names):
             continue
         filtered_configs.append(
-            ModelConfigurationView.from_model(model_configuration, provider)
+            ModelConfigurationView.from_model(
+                model_configuration, provider, use_stored_display_name
+            )
         )
 
     return filtered_configs

backend/tests/unit/onyx/file_processing/fixtures/owner_protected.pdf

@@ -0,0 +1,76 @@
+%PDF-1.3
+%<25><><EFBFBD><EFBFBD>
+1 0 obj
+<<
+/Producer <1083d595b1>
+>>
+endobj
+2 0 obj
+<<
+/Type /Pages
+/Count 1
+/Kids [ 4 0 R ]
+>>
+endobj
+3 0 obj
+<<
+/Type /Catalog
+/Pages 2 0 R
+>>
+endobj
+4 0 obj
+<<
+/Type /Page
+/Resources <<
+/Font <<
+/F1 <<
+/Type /Font
+/Subtype /Type1
+/BaseFont /Helvetica
+>>
+>>
+>>
+/MediaBox [ 0.0 0.0 200 200 ]
+/Contents 5 0 R
+/Parent 2 0 R
+>>
+endobj
+5 0 obj
+<<
+/Length 42
+>>
+stream
+,N<><6~<7E>)<29><><EFBFBD><EFBFBD><EFBFBD>u<EFBFBD><0C><><EFBFBD>Zc'<27><>>8g<38><67><EFBFBD>n<EFBFBD><6E><EFBFBD><EFBFBD><EFBFBD>9"
+endstream
+endobj
+6 0 obj
+<<
+/V 2
+/R 3
+/Length 128
+/P 4294967292
+/Filter /Standard
+/O <6a340a292629053da84a6d8b19a5d505953b8b3fdac3d2d389fde0e354528d44>
+/U <d6f0dc91c7b9de264a8d708515468e6528bf4e5e4e758a4164004e56fffa0108>
+>>
+endobj
+xref
+0 7
+0000000000 65535 f 
+0000000015 00000 n 
+0000000059 00000 n 
+0000000118 00000 n 
+0000000167 00000 n 
+0000000348 00000 n 
+0000000440 00000 n 
+trailer
+<<
+/Size 7
+/Root 3 0 R
+/Info 1 0 R
+/ID [ <6364336635356135633239323638353039306635656133623165313637366430> <6364336635356135633239323638353039306635656133623165313637366430> ]
+/Encrypt 6 0 R
+>>
+startxref
+655
+%%EOF

backend/tests/unit/onyx/file_processing/test_pdf.py

@@ -54,6 +54,12 @@ class TestReadPdfFile:
         text, _, _ = read_pdf_file(_load("encrypted.pdf"), pdf_pass="wrong")
         assert text == ""
 
+    def test_owner_password_only_pdf_extracts_text(self) -> None:
+        """A PDF encrypted with only an owner password (no user password)
+        should still yield its text content. Regression for #9754."""
+        text, _, _ = read_pdf_file(_load("owner_protected.pdf"))
+        assert "Hello World" in text
+
     def test_empty_pdf(self) -> None:
         text, _, _ = read_pdf_file(_load("empty.pdf"))
         assert text.strip() == ""
@@ -117,6 +123,12 @@ class TestIsPdfProtected:
     def test_protected_pdf(self) -> None:
         assert is_pdf_protected(_load("encrypted.pdf")) is True
 
+    def test_owner_password_only_is_not_protected(self) -> None:
+        """A PDF with only an owner password (permission restrictions) but no
+        user password should NOT be considered protected — any viewer can open
+        it without prompting for a password."""
+        assert is_pdf_protected(_load("owner_protected.pdf")) is False
+
     def test_preserves_file_position(self) -> None:
         pdf = _load("simple.pdf")
         pdf.seek(42)

web/lib/opal/src/core/animations/components.tsx

@@ -1,48 +1,34 @@
 "use client";
 
 import "@opal/core/animations/styles.css";
-import React, { createContext, useContext, useState, useCallback } from "react";
+import React from "react";
 import { cn } from "@opal/utils";
 import type { WithoutStyles, ExtremaSizeVariants } from "@opal/types";
 import { widthVariants } from "@opal/shared";
 
-// ---------------------------------------------------------------------------
-// Context-per-group registry
-// ---------------------------------------------------------------------------
-
-/**
- * Lazily-created map of group names to React contexts.
- *
- * Each group gets its own `React.Context<boolean | null>` so that a
- * `Hoverable.Item` only re-renders when its *own* group's hover state
- * changes — not when any unrelated group changes.
- *
- * The default value is `null` (no provider found), which lets
- * `Hoverable.Item` distinguish "no Root ancestor" from "Root says
- * not hovered" and throw when `group` was explicitly specified.
- */
-const contextMap = new Map<string, React.Context<boolean | null>>();
-
-function getOrCreateContext(group: string): React.Context<boolean | null> {
-  let ctx = contextMap.get(group);
-  if (!ctx) {
-    ctx = createContext<boolean | null>(null);
-    ctx.displayName = `HoverableContext(${group})`;
-    contextMap.set(group, ctx);
-  }
-  return ctx;
-}
-
 // ---------------------------------------------------------------------------
 // Types
 // ---------------------------------------------------------------------------
 
+type HoverableInteraction = "rest" | "hover";
+
 interface HoverableRootProps
   extends WithoutStyles<React.HTMLAttributes<HTMLDivElement>> {
   children: React.ReactNode;
   group: string;
   /** Width preset. @default "auto" */
   widthVariant?: ExtremaSizeVariants;
+  /**
+   * JS-controllable interaction state override.
+   *
+   * - `"rest"` (default): items are shown/hidden by CSS `:hover`.
+   * - `"hover"`: forces items visible regardless of hover state. Useful when
+   *   a hoverable action opens a modal — set `interaction="hover"` while the
+   *   modal is open so the user can see which element they're interacting with.
+   *
+   * @default "rest"
+   */
+  interaction?: HoverableInteraction;
   /** Ref forwarded to the root `<div>`. */
   ref?: React.Ref<HTMLDivElement>;
 }
@@ -65,12 +51,10 @@ interface HoverableItemProps
 /**
  * Hover-tracking container for a named group.
  *
- * Wraps children in a `<div>` that tracks mouse-enter / mouse-leave and
- * provides the hover state via a per-group React context.
- *
- * Nesting works because each `Hoverable.Root` creates a **new** context
- * provider that shadows the parent — so an inner `Hoverable.Item group="b"`
- * reads from the inner provider, not the outer `group="a"` provider.
+ * Uses a `data-hover-group` attribute and CSS `:hover` to control
+ * descendant `Hoverable.Item` visibility. No React state or context —
+ * the browser natively removes `:hover` when modals/portals steal
+ * pointer events, preventing stale hover state.
  *
  * @example
  * ```tsx
@@ -87,70 +71,20 @@ function HoverableRoot({
   group,
   children,
   widthVariant = "full",
+  interaction = "rest",
   ref,
-  onMouseEnter: consumerMouseEnter,
-  onMouseLeave: consumerMouseLeave,
-  onFocusCapture: consumerFocusCapture,
-  onBlurCapture: consumerBlurCapture,
   ...props
 }: HoverableRootProps) {
-  const [hovered, setHovered] = useState(false);
-  const [focused, setFocused] = useState(false);
-
-  const onMouseEnter = useCallback(
-    (e: React.MouseEvent<HTMLDivElement>) => {
-      setHovered(true);
-      consumerMouseEnter?.(e);
-    },
-    [consumerMouseEnter]
-  );
-
-  const onMouseLeave = useCallback(
-    (e: React.MouseEvent<HTMLDivElement>) => {
-      setHovered(false);
-      consumerMouseLeave?.(e);
-    },
-    [consumerMouseLeave]
-  );
-
-  const onFocusCapture = useCallback(
-    (e: React.FocusEvent<HTMLDivElement>) => {
-      setFocused(true);
-      consumerFocusCapture?.(e);
-    },
-    [consumerFocusCapture]
-  );
-
-  const onBlurCapture = useCallback(
-    (e: React.FocusEvent<HTMLDivElement>) => {
-      if (
-        !(e.relatedTarget instanceof Node) ||
-        !e.currentTarget.contains(e.relatedTarget)
-      ) {
-        setFocused(false);
-      }
-      consumerBlurCapture?.(e);
-    },
-    [consumerBlurCapture]
-  );
-
-  const active = hovered || focused;
-  const GroupContext = getOrCreateContext(group);
-
   return (
-    <GroupContext.Provider value={active}>
-      <div
-        {...props}
-        ref={ref}
-        className={cn(widthVariants[widthVariant])}
-        onMouseEnter={onMouseEnter}
-        onMouseLeave={onMouseLeave}
-        onFocusCapture={onFocusCapture}
-        onBlurCapture={onBlurCapture}
-      >
-        {children}
-      </div>
-    </GroupContext.Provider>
+    <div
+      {...props}
+      ref={ref}
+      className={cn(widthVariants[widthVariant])}
+      data-hover-group={group}
+      data-interaction={interaction !== "rest" ? interaction : undefined}
+    >
+      {children}
+    </div>
   );
 }
 
@@ -162,13 +96,10 @@ function HoverableRoot({
  * An element whose visibility is controlled by hover state.
  *
  * **Local mode** (`group` omitted): the item handles hover on its own
- * element via CSS `:hover`. This is the core abstraction.
+ * element via CSS `:hover`.
  *
- * **Group mode** (`group` provided): visibility is driven by a matching
- * `Hoverable.Root` ancestor's hover state via React context. If no
- * matching Root is found, an error is thrown.
- *
- * Uses data-attributes for variant styling (see `styles.css`).
+ * **Group mode** (`group` provided): visibility is driven by CSS `:hover`
+ * on the nearest `Hoverable.Root` ancestor via `[data-hover-group]:hover`.
  *
  * @example
  * ```tsx
@@ -184,8 +115,6 @@ function HoverableRoot({
  *   </Hoverable.Item>
  * </Hoverable.Root>
  * ```
- *
- * @throws If `group` is specified but no matching `Hoverable.Root` ancestor exists.
  */
 function HoverableItem({
   group,
@@ -194,17 +123,6 @@ function HoverableItem({
   ref,
   ...props
 }: HoverableItemProps) {
-  const contextValue = useContext(
-    group ? getOrCreateContext(group) : NOOP_CONTEXT
-  );
-
-  if (group && contextValue === null) {
-    throw new Error(
-      `Hoverable.Item group="${group}" has no matching Hoverable.Root ancestor. ` +
-        `Either wrap it in <Hoverable.Root group="${group}"> or remove the group prop for local hover.`
-    );
-  }
-
   const isLocal = group === undefined;
 
   return (
@@ -213,9 +131,6 @@ function HoverableItem({
       ref={ref}
       className={cn("hoverable-item")}
       data-hoverable-variant={variant}
-      data-hoverable-active={
-        isLocal ? undefined : contextValue ? "true" : undefined
-      }
       data-hoverable-local={isLocal ? "true" : undefined}
     >
       {children}
@@ -223,9 +138,6 @@ function HoverableItem({
   );
 }
 
-/** Stable context used when no group is specified (local mode). */
-const NOOP_CONTEXT = createContext<boolean | null>(null);
-
 // ---------------------------------------------------------------------------
 // Compound export
 // ---------------------------------------------------------------------------
@@ -233,18 +145,16 @@ const NOOP_CONTEXT = createContext<boolean | null>(null);
 /**
  * Hoverable compound component for hover-to-reveal patterns.
  *
- * Provides two sub-components:
+ * Entirely CSS-driven — no React state or context. The browser's native
+ * `:hover` pseudo-class handles all state, which means hover is
+ * automatically cleared when modals/portals steal pointer events.
  *
- * - `Hoverable.Root` — A container that tracks hover state for a named group
- *   and provides it via React context.
+ * - `Hoverable.Root` — Container with `data-hover-group`. CSS `:hover`
+ *   on this element reveals descendant `Hoverable.Item` elements.
  *
- * - `Hoverable.Item` — The core abstraction. On its own (no `group`), it
- *   applies local CSS `:hover` for the variant effect. When `group` is
- *   specified, it reads hover state from the nearest matching
- *   `Hoverable.Root` — and throws if no matching Root is found.
- *
- * Supports nesting: a child `Hoverable.Root` shadows the parent's context,
- * so each group's items only respond to their own root's hover.
+ * - `Hoverable.Item` — Hidden by default. In group mode, revealed when
+ *   the ancestor Root is hovered. In local mode (no `group`), revealed
+ *   when the item itself is hovered.
  *
  * @example
  * ```tsx
@@ -276,4 +186,5 @@ export {
   type HoverableRootProps,
   type HoverableItemProps,
   type HoverableItemVariant,
+  type HoverableInteraction,
 };

web/lib/opal/src/core/animations/styles.css

@@ -7,8 +7,20 @@
   opacity: 0;
 }
 
-/* Group mode — Root controls visibility via React context */
-.hoverable-item[data-hoverable-variant="opacity-on-hover"][data-hoverable-active="true"] {
+/* Group mode — Root :hover controls descendant item visibility via CSS.
+   Exclude local-mode items so they aren't revealed by an ancestor root. */
+[data-hover-group]:hover
+  .hoverable-item[data-hoverable-variant="opacity-on-hover"]:not(
+    [data-hoverable-local]
+  ) {
+  opacity: 1;
+}
+
+/* Interaction override — force items visible via JS */
+[data-hover-group][data-interaction="hover"]
+  .hoverable-item[data-hoverable-variant="opacity-on-hover"]:not(
+    [data-hoverable-local]
+  ) {
   opacity: 1;
 }
 
@@ -17,7 +29,16 @@
   opacity: 1;
 }
 
-/* Focus — item (or a focusable descendant) receives keyboard focus */
+/* Group focus — any focusable descendant of the Root receives keyboard focus,
+   revealing all group items (same behavior as hover). */
+[data-hover-group]:focus-within
+  .hoverable-item[data-hoverable-variant="opacity-on-hover"]:not(
+    [data-hoverable-local]
+  ) {
+  opacity: 1;
+}
+
+/* Local focus — item (or a focusable descendant) receives keyboard focus */
 .hoverable-item[data-hoverable-variant="opacity-on-hover"]:has(:focus-visible) {
   opacity: 1;
 }

web/src/refresh-pages/admin/LLMConfigurationPage.tsx

@@ -169,7 +169,10 @@ function ExistingProviderCard({
         </ConfirmationModalLayout>
       )}
 
-      <Hoverable.Root group="ExistingProviderCard">
+      <Hoverable.Root
+        group="ExistingProviderCard"
+        interaction={deleteModal.isOpen ? "hover" : "rest"}
+      >
         <SelectCard
           state="filled"
           padding="sm"

web/src/sections/modals/llmConfig/CustomModal.test.tsx

@@ -70,7 +70,9 @@ describe("Custom LLM Provider Configuration Workflow", () => {
     }
   ) {
     const nameInput = screen.getByPlaceholderText("Display Name");
-    const providerInput = screen.getByPlaceholderText("Provider Name");
+    const providerInput = screen.getByPlaceholderText(
+      "Provider Name as shown on LiteLLM"
+    );
 
     await user.type(nameInput, options.name);
     await user.type(providerInput, options.provider);
@@ -490,7 +492,9 @@ describe("Custom LLM Provider Configuration Workflow", () => {
     const nameInput = screen.getByPlaceholderText("Display Name");
     await user.type(nameInput, "Cloudflare Provider");
 
-    const providerInput = screen.getByPlaceholderText("Provider Name");
+    const providerInput = screen.getByPlaceholderText(
+      "Provider Name as shown on LiteLLM"
+    );
     await user.type(providerInput, "cloudflare");
 
     // Click "Add Line" button for custom config (aria-label from KeyValueInput)
@@ -500,9 +504,7 @@ describe("Custom LLM Provider Configuration Workflow", () => {
     await user.click(addLineButton);
 
     // Fill in custom config key-value pair
-    const keyInputs = screen.getAllByPlaceholderText(
-      "e.g. api_base, api_version, api_key"
-    );
+    const keyInputs = screen.getAllByRole("textbox", { name: /Key \d+/ });
     const valueInputs = screen.getAllByRole("textbox", { name: /Value \d+/ });
 
     await user.type(keyInputs[0]!, "CLOUDFLARE_ACCOUNT_ID");

web/src/sections/modals/llmConfig/CustomModal.tsx

@@ -14,6 +14,7 @@ import {
   submitOnboardingProvider,
 } from "@/sections/modals/llmConfig/svc";
 import {
+  APIKeyField,
   DisplayNameField,
   FieldSeparator,
   ModelsAccessField,
@@ -182,28 +183,14 @@ function ModelConfigurationList({ formikProps }: ModelConfigurationListProps) {
 
 // ─── Custom Config Processing ─────────────────────────────────────────────────
 
-// Keys that the backend accepts as dedicated fields on the LLM provider model
-// (alongside `name`, `provider`, etc.) rather than inside the freeform
-// `custom_config` dict. When the user enters one of these in the key-value
-// list, we pull it out and send it as a top-level field in the upsert request
-// so the backend stores and validates it properly.
-const FIRST_CLASS_KEYS = ["api_key", "api_base", "api_version"] as const;
-
-function extractFirstClassFields(items: KeyValue[]) {
-  const firstClass: Record<string, string | undefined> = {};
-  const remaining: { [key: string]: string } = {};
-
+function keyValueListToDict(items: KeyValue[]): Record<string, string> {
+  const result: Record<string, string> = {};
   for (const { key, value } of items) {
-    if ((FIRST_CLASS_KEYS as readonly string[]).includes(key)) {
-      if (value.trim() !== "") {
-        firstClass[key] = value;
-      }
-    } else {
-      remaining[key] = value;
+    if (key.trim() !== "") {
+      result[key] = value;
     }
   }
-
-  return { firstClass, customConfig: remaining };
+  return result;
 }
 
 export default function CustomModal({
@@ -229,6 +216,9 @@ export default function CustomModal({
     ),
     ...(isOnboarding ? buildOnboardingInitialValues() : {}),
     provider: existingLlmProvider?.provider ?? "",
+    api_key: existingLlmProvider?.api_key ?? "",
+    api_base: existingLlmProvider?.api_base ?? "",
+    api_version: existingLlmProvider?.api_version ?? "",
     model_configurations: existingLlmProvider?.model_configurations.map(
       (mc) => ({
         name: mc.name,
@@ -244,16 +234,11 @@ export default function CustomModal({
         supports_image_input: false,
       },
     ],
-    custom_config_list: [
-      ...FIRST_CLASS_KEYS.filter(
-        (k) => existingLlmProvider?.[k] != null && existingLlmProvider[k] !== ""
-      ).map((k) => ({ key: k, value: String(existingLlmProvider![k]) })),
-      ...(existingLlmProvider?.custom_config
-        ? Object.entries(existingLlmProvider.custom_config).map(
-            ([key, value]) => ({ key, value: String(value) })
-          )
-        : []),
-    ],
+    custom_config_list: existingLlmProvider?.custom_config
+      ? Object.entries(existingLlmProvider.custom_config).map(
+          ([key, value]) => ({ key, value: String(value) })
+        )
+      : [],
   };
 
   const modelConfigurationSchema = Yup.object({
@@ -302,16 +287,16 @@ export default function CustomModal({
           return;
         }
 
-        const { firstClass, customConfig } = extractFirstClassFields(
-          values.custom_config_list
-        );
+        // Always send custom_config as a dict (even empty) so the backend
+        // preserves it as non-null — this is the signal that the provider was
+        // created via CustomModal.
+        const customConfig = keyValueListToDict(values.custom_config_list);
 
         if (isOnboarding && onboardingState && onboardingActions) {
           await submitOnboardingProvider({
             providerName: values.provider,
             payload: {
               ...values,
-              ...firstClass,
               model_configurations: modelConfigurations,
               custom_config: customConfig,
             },
@@ -326,23 +311,18 @@ export default function CustomModal({
             (config) => config.name
           );
 
-          const {
-            firstClass: initialFirstClass,
-            customConfig: initialCustomConfig,
-          } = extractFirstClassFields(initialValues.custom_config_list);
-
           await submitLLMProvider({
             providerName: values.provider,
             values: {
               ...values,
-              ...firstClass,
               selected_model_names: selectedModelNames,
               custom_config: customConfig,
             },
             initialValues: {
               ...initialValues,
-              ...initialFirstClass,
-              custom_config: initialCustomConfig,
+              custom_config: keyValueListToDict(
+                initialValues.custom_config_list
+              ),
             },
             modelConfigurations,
             existingLlmProvider,
@@ -366,35 +346,54 @@ export default function CustomModal({
           isSubmitting={formikProps.isSubmitting}
         >
           {!isOnboarding && (
-            <Section gap={0}>
-              <DisplayNameField disabled={!!existingLlmProvider} />
-
-              <FieldWrapper>
-                <InputLayouts.Vertical
+            <FieldWrapper>
+              <InputLayouts.Vertical
+                name="provider"
+                title="Provider Name"
+                subDescription={markdown(
+                  "Should be one of the providers listed at [LiteLLM](https://docs.litellm.ai/docs/providers)."
+                )}
+              >
+                <InputTypeInField
                   name="provider"
-                  title="Provider Name"
-                  subDescription={markdown(
-                    "Should be one of the providers listed at [LiteLLM](https://docs.litellm.ai/docs/providers)."
-                  )}
-                >
-                  <InputTypeInField
-                    name="provider"
-                    placeholder="Provider Name"
-                    variant={existingLlmProvider ? "disabled" : undefined}
-                  />
-                </InputLayouts.Vertical>
-              </FieldWrapper>
-            </Section>
+                  placeholder="Provider Name as shown on LiteLLM"
+                  variant={existingLlmProvider ? "disabled" : undefined}
+                />
+              </InputLayouts.Vertical>
+            </FieldWrapper>
           )}
 
-          <FieldSeparator />
+          <FieldWrapper>
+            <InputLayouts.Vertical
+              name="api_base"
+              title="API Base URL"
+              suffix="optional"
+            >
+              <InputTypeInField name="api_base" placeholder="https://" />
+            </InputLayouts.Vertical>
+          </FieldWrapper>
+
+          <FieldWrapper>
+            <InputLayouts.Vertical
+              name="api_version"
+              title="API Version"
+              suffix="optional"
+            >
+              <InputTypeInField name="api_version" />
+            </InputLayouts.Vertical>
+          </FieldWrapper>
+
+          <APIKeyField
+            optional
+            subDescription="Paste your API key if your model provider requires authentication."
+          />
 
           <FieldWrapper>
             <Section gap={0.75}>
               <Content
-                title="Provider Configs"
+                title="Additional Configs"
                 description={markdown(
-                  "Add properties as needed by the model provider. This is passed to LiteLLM's `completion()` call as [arguments](https://docs.litellm.ai/docs/completion/input#input-params-1) (e.g. API base URL, API version, API key). See [documentation](https://docs.onyx.app/admins/ai_models/custom_inference_provider) for more instructions."
+                  "Add extra properties as needed by the model provider. These are passed to LiteLLM's `completion()` call as [environment variables](https://docs.litellm.ai/docs/set_keys#environment-variables). See [documentation](https://docs.onyx.app/admins/ai_models/custom_inference_provider) for more instructions."
                 )}
                 widthVariant="full"
                 variant="section"
@@ -406,7 +405,6 @@ export default function CustomModal({
                 onChange={(items) =>
                   formikProps.setFieldValue("custom_config_list", items)
                 }
-                keyPlaceholder="e.g. api_base, api_version, api_key"
                 addButtonLabel="Add Line"
               />
             </Section>
@@ -414,6 +412,12 @@ export default function CustomModal({
 
           <FieldSeparator />
 
+          {!isOnboarding && (
+            <DisplayNameField disabled={!!existingLlmProvider} />
+          )}
+
+          <FieldSeparator />
+
           <Section gap={0.5}>
             <FieldWrapper>
               <Content

web/src/sections/modals/llmConfig/getModal.tsx

@@ -11,15 +11,6 @@ import LMStudioForm from "@/sections/modals/llmConfig/LMStudioForm";
 import LiteLLMProxyModal from "@/sections/modals/llmConfig/LiteLLMProxyModal";
 import BifrostModal from "@/sections/modals/llmConfig/BifrostModal";
 
-function detectIfRealOpenAIProvider(provider: LLMProviderView) {
-  return (
-    provider.provider === LLMProviderName.OPENAI &&
-    provider.api_key &&
-    !provider.api_base &&
-    Object.keys(provider.custom_config || {}).length === 0
-  );
-}
-
 export function getModalForExistingProvider(
   provider: LLMProviderView,
   onOpenChange?: (open: boolean) => void,
@@ -31,26 +22,44 @@ export function getModalForExistingProvider(
     defaultModelName,
   };
 
+  const hasCustomConfig = provider.custom_config != null;
+
   switch (provider.provider) {
+    // These providers don't use custom_config themselves, so a non-null
+    // custom_config means the provider was created via CustomModal.
     case LLMProviderName.OPENAI:
-      // "openai" as a provider name can be used for litellm proxy / any OpenAI-compatible provider
-      if (detectIfRealOpenAIProvider(provider)) {
-        return <OpenAIModal {...props} />;
-      } else {
-        return <CustomModal {...props} />;
-      }
+      return hasCustomConfig ? (
+        <CustomModal {...props} />
+      ) : (
+        <OpenAIModal {...props} />
+      );
     case LLMProviderName.ANTHROPIC:
-      return <AnthropicModal {...props} />;
+      return hasCustomConfig ? (
+        <CustomModal {...props} />
+      ) : (
+        <AnthropicModal {...props} />
+      );
+    case LLMProviderName.AZURE:
+      return hasCustomConfig ? (
+        <CustomModal {...props} />
+      ) : (
+        <AzureModal {...props} />
+      );
+    case LLMProviderName.OPENROUTER:
+      return hasCustomConfig ? (
+        <CustomModal {...props} />
+      ) : (
+        <OpenRouterModal {...props} />
+      );
+
+    // These providers legitimately store settings in custom_config,
+    // so always use their dedicated modals.
     case LLMProviderName.OLLAMA_CHAT:
       return <OllamaModal {...props} />;
-    case LLMProviderName.AZURE:
-      return <AzureModal {...props} />;
     case LLMProviderName.VERTEX_AI:
       return <VertexAIModal {...props} />;
     case LLMProviderName.BEDROCK:
       return <BedrockModal {...props} />;
-    case LLMProviderName.OPENROUTER:
-      return <OpenRouterModal {...props} />;
     case LLMProviderName.LM_STUDIO:
       return <LMStudioForm {...props} />;
     case LLMProviderName.LITELLM_PROXY:

web/src/sections/modals/llmConfig/shared.tsx

@@ -17,7 +17,7 @@ import Switch from "@/refresh-components/inputs/Switch";
 import Text from "@/refresh-components/texts/Text";
 import { Button, LineItemButton, Tag } from "@opal/components";
 import { BaseLLMFormValues } from "@/sections/modals/llmConfig/utils";
-import { WithoutStyles } from "@opal/types";
+import { RichStr, WithoutStyles } from "@opal/types";
 import Separator from "@/refresh-components/Separator";
 import { Section } from "@/layouts/general-layouts";
 import { Hoverable } from "@opal/core";
@@ -49,7 +49,7 @@ import {
 } from "@/lib/llmConfig/providers";
 
 export function FieldSeparator() {
-  return <Separator noPadding className="px-2" />;
+  return <Separator noPadding className="p-2" />;
 }
 
 export type FieldWrapperProps = WithoutStyles<
@@ -89,11 +89,13 @@ export function DisplayNameField({ disabled = false }: DisplayNameFieldProps) {
 export interface APIKeyFieldProps {
   optional?: boolean;
   providerName?: string;
+  subDescription?: string | RichStr;
 }
 
 export function APIKeyField({
   optional = false,
   providerName,
+  subDescription,
 }: APIKeyFieldProps) {
   return (
     <FieldWrapper>
@@ -101,13 +103,15 @@ export function APIKeyField({
         name="api_key"
         title="API Key"
         subDescription={
-          providerName
-            ? `Paste your API key from ${providerName} to access your models.`
-            : "Paste your API key to access your models."
+          subDescription
+            ? subDescription
+            : providerName
+              ? `Paste your API key from ${providerName} to access your models.`
+              : "Paste your API key to access your models."
         }
         suffix={optional ? "optional" : undefined}
       >
-        <PasswordInputTypeInField name="api_key" placeholder="API Key" />
+        <PasswordInputTypeInField name="api_key" />
       </InputLayouts.Vertical>
     </FieldWrapper>
   );
@@ -689,7 +693,7 @@ export function LLMConfigurationModalWrapper({
             description={description}
             onClose={onClose}
           />
-          <Modal.Body padding={0.5} gap={0.5}>
+          <Modal.Body padding={0.5} gap={0}>
             {children}
           </Modal.Body>
           <Modal.Footer>