🤖 fix(site/src/pages/AISettingsPage): never round-trip the saved credential mask

jakehwll · jakehwll · commit 338be75de950 · 2026-05-14T13:52:39.000Z
Two related correctness gaps in the providers form let the masked
placeholder ("********") reach the API as a real secret:

* Path A: Bedrock credential inputs are seeded with the mask when the
  server already has values on file. The mask is cleared on focus, but
  the user can edit unrelated fields and submit without ever focusing
  the credential inputs, so values.accessKey is still "********" at
  submit time. The mapper trimmed and forwarded that as the new key.

* Path B: a partial Bedrock credential update (one of the two AWS
  fields filled, the other left as the mask or empty) was silently
  dropped because the create/update mapper required both fields to be
  non-empty to send anything. The user thought they rotated the key;
  the API never saw a change.

This commit:

* Exports SAVED_CREDENTIAL_MASK from ProviderForm and uses it in a
  shared sanitizeCredential helper inside providerFormApiMap, so any
  credential value that is empty *or matches the mask* is treated as
  "keep the existing value" and is omitted from the request.

* Adds a paired Yup .test on the Bedrock schema's accessKey /
  accessKeySecret fields. When one is filled (mask-aware), the other
  is required, so a partial rotation now surfaces a validation error
  instead of being silently dropped.
diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx
@@ -48,7 +48,7 @@ const providerNameErrorMessage =
  * server. Focusing the input clears it, so we never have to round-trip the
  * mask to the API.
  */
-const SAVED_CREDENTIAL_MASK = "********";
+export const SAVED_CREDENTIAL_MASK = "********";
 
 const defaultInitialValues: ProviderFormValues = {
 	type: "anthropic",
@@ -77,6 +77,15 @@ const makeOpenAiAnthropicSchema = (editing: boolean) =>
 		enabled: Yup.boolean(),
 	});
 
+// Treat the saved-credential mask as empty: a value matching the placeholder
+// should never be treated as a real, user-supplied credential during
+// validation.
+const credentialFilled = (value: string | undefined): boolean => {
+	if (!value) return false;
+	const trimmed = value.trim();
+	return trimmed !== "" && trimmed !== SAVED_CREDENTIAL_MASK;
+};
+
 const makeBedrockSchema = (editing: boolean) =>
 	Yup.object({
 		type: Yup.string()
@@ -95,12 +104,29 @@ const makeBedrockSchema = (editing: boolean) =>
 		apiKey: Yup.string(),
 		model: Yup.string().required("Model is required"),
 		smallFastModel: Yup.string().required("Small fast model is required"),
-		accessKey: editing
+		accessKey: (editing
 			? Yup.string()
-			: Yup.string().required("Access key is required"),
-		accessKeySecret: editing
+			: Yup.string().required("Access key is required")
+		).test(
+			"access-key-paired",
+			"Enter both access key and secret to rotate credentials.",
+			function (value) {
+				const secret = (this.parent as { accessKeySecret?: string })
+					.accessKeySecret;
+				return !(credentialFilled(secret) && !credentialFilled(value));
+			},
+		),
+		accessKeySecret: (editing
 			? Yup.string()
-			: Yup.string().required("Access key secret is required"),
+			: Yup.string().required("Access key secret is required")
+		).test(
+			"access-key-secret-paired",
+			"Enter both access key and secret to rotate credentials.",
+			function (value) {
+				const accessKey = (this.parent as { accessKey?: string }).accessKey;
+				return !(credentialFilled(accessKey) && !credentialFilled(value));
+			},
+		),
 		enabled: Yup.boolean(),
 	});
 
diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts b/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts
@@ -4,7 +4,19 @@ import type {
 	CreateAIProviderRequest,
 	UpdateAIProviderRequest,
 } from "#/api/typesGenerated";
-import type { ProviderFormValues } from "./ProviderForm";
+import { type ProviderFormValues, SAVED_CREDENTIAL_MASK } from "./ProviderForm";
+
+/**
+ * Treat the saved-credential mask the same as an empty value: never round-trip
+ * the placeholder back to the API.
+ */
+const sanitizeCredential = (value: string): string => {
+	const trimmed = value.trim();
+	if (trimmed === "" || trimmed === SAVED_CREDENTIAL_MASK) {
+		return "";
+	}
+	return trimmed;
+};
 
 /**
  * The wire API only knows about `openai` and `anthropic`; AWS Bedrock is a
@@ -60,8 +72,8 @@ export const providerFormValuesToCreate = (
 		const settings: AIProviderSettings = {
 			bedrock_model: values.model.trim(),
 			bedrock_small_fast_model: values.smallFastModel.trim(),
-			bedrock_access_key: values.accessKey.trim(),
-			bedrock_access_key_secret: values.accessKeySecret.trim(),
+			bedrock_access_key: sanitizeCredential(values.accessKey),
+			bedrock_access_key_secret: sanitizeCredential(values.accessKeySecret),
 		};
 		return {
 			request: {
@@ -83,7 +95,7 @@ export const providerFormValuesToCreate = (
 			base_url: baseUrl,
 			enabled: values.enabled,
 		},
-		apiKey: values.apiKey.trim() || undefined,
+		apiKey: sanitizeCredential(values.apiKey) || undefined,
 	};
 };
 
@@ -108,8 +120,11 @@ export const providerFormValuesToUpdate = (
 		return base;
 	}
 
-	const newAccessKey = values.accessKey.trim();
-	const newAccessKeySecret = values.accessKeySecret.trim();
+	const newAccessKey = sanitizeCredential(values.accessKey);
+	const newAccessKeySecret = sanitizeCredential(values.accessKeySecret);
+	// Yup enforces that access key and secret are entered together before we
+	// reach this point; if both survived the mask filter, the user wants to
+	// rotate credentials.
 	const credentialsChanged = newAccessKey !== "" && newAccessKeySecret !== "";
 
 	const settings: AIProviderSettings = {
