Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 338be75

Browse filesBrowse files
jakehwlljakehwll
committed
🤖 fix(site/src/pages/AISettingsPage): never round-trip the saved credential mask
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.
1 parent aa9f084 commit 338be75
Copy full SHA for 338be75

2 files changed

+52-11Lines changed: 52 additions & 11 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx‎

Copy file name to clipboardExpand all lines: site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx
+31-5Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ const providerNameErrorMessage =
4848
* server. Focusing the input clears it, so we never have to round-trip the
4949
* mask to the API.
5050
*/
51-
const SAVED_CREDENTIAL_MASK = "********";
51+
export const SAVED_CREDENTIAL_MASK = "********";
5252

5353
const defaultInitialValues: ProviderFormValues = {
5454
type: "anthropic",
@@ -77,6 +77,15 @@ const makeOpenAiAnthropicSchema = (editing: boolean) =>
7777
enabled: Yup.boolean(),
7878
});
7979

80+
// Treat the saved-credential mask as empty: a value matching the placeholder
81+
// should never be treated as a real, user-supplied credential during
82+
// validation.
83+
const credentialFilled = (value: string | undefined): boolean => {
84+
if (!value) return false;
85+
const trimmed = value.trim();
86+
return trimmed !== "" && trimmed !== SAVED_CREDENTIAL_MASK;
87+
};
88+
8089
const makeBedrockSchema = (editing: boolean) =>
8190
Yup.object({
8291
type: Yup.string()
@@ -95,12 +104,29 @@ const makeBedrockSchema = (editing: boolean) =>
95104
apiKey: Yup.string(),
96105
model: Yup.string().required("Model is required"),
97106
smallFastModel: Yup.string().required("Small fast model is required"),
98-
accessKey: editing
107+
accessKey: (editing
99108
? Yup.string()
100-
: Yup.string().required("Access key is required"),
101-
accessKeySecret: editing
109+
: Yup.string().required("Access key is required")
110+
).test(
111+
"access-key-paired",
112+
"Enter both access key and secret to rotate credentials.",
113+
function (value) {
114+
const secret = (this.parent as { accessKeySecret?: string })
115+
.accessKeySecret;
116+
return !(credentialFilled(secret) && !credentialFilled(value));
117+
},
118+
),
119+
accessKeySecret: (editing
102120
? Yup.string()
103-
: Yup.string().required("Access key secret is required"),
121+
: Yup.string().required("Access key secret is required")
122+
).test(
123+
"access-key-secret-paired",
124+
"Enter both access key and secret to rotate credentials.",
125+
function (value) {
126+
const accessKey = (this.parent as { accessKey?: string }).accessKey;
127+
return !(credentialFilled(accessKey) && !credentialFilled(value));
128+
},
129+
),
104130
enabled: Yup.boolean(),
105131
});
106132

Collapse file

‎site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts‎

Copy file name to clipboardExpand all lines: site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts
+21-6Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,19 @@ import type {
44
CreateAIProviderRequest,
55
UpdateAIProviderRequest,
66
} from "#/api/typesGenerated";
7-
import type { ProviderFormValues } from "./ProviderForm";
7+
import { type ProviderFormValues, SAVED_CREDENTIAL_MASK } from "./ProviderForm";
8+
9+
/**
10+
* Treat the saved-credential mask the same as an empty value: never round-trip
11+
* the placeholder back to the API.
12+
*/
13+
const sanitizeCredential = (value: string): string => {
14+
const trimmed = value.trim();
15+
if (trimmed === "" || trimmed === SAVED_CREDENTIAL_MASK) {
16+
return "";
17+
}
18+
return trimmed;
19+
};
820

921
/**
1022
* The wire API only knows about `openai` and `anthropic`; AWS Bedrock is a
@@ -60,8 +72,8 @@ export const providerFormValuesToCreate = (
6072
const settings: AIProviderSettings = {
6173
bedrock_model: values.model.trim(),
6274
bedrock_small_fast_model: values.smallFastModel.trim(),
63-
bedrock_access_key: values.accessKey.trim(),
64-
bedrock_access_key_secret: values.accessKeySecret.trim(),
75+
bedrock_access_key: sanitizeCredential(values.accessKey),
76+
bedrock_access_key_secret: sanitizeCredential(values.accessKeySecret),
6577
};
6678
return {
6779
request: {
@@ -83,7 +95,7 @@ export const providerFormValuesToCreate = (
8395
base_url: baseUrl,
8496
enabled: values.enabled,
8597
},
86-
apiKey: values.apiKey.trim() || undefined,
98+
apiKey: sanitizeCredential(values.apiKey) || undefined,
8799
};
88100
};
89101

@@ -108,8 +120,11 @@ export const providerFormValuesToUpdate = (
108120
return base;
109121
}
110122

111-
const newAccessKey = values.accessKey.trim();
112-
const newAccessKeySecret = values.accessKeySecret.trim();
123+
const newAccessKey = sanitizeCredential(values.accessKey);
124+
const newAccessKeySecret = sanitizeCredential(values.accessKeySecret);
125+
// Yup enforces that access key and secret are entered together before we
126+
// reach this point; if both survived the mask filter, the user wants to
127+
// rotate credentials.
113128
const credentialsChanged = newAccessKey !== "" && newAccessKeySecret !== "";
114129

115130
const settings: AIProviderSettings = {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.