Skip to content

Commit ab0ab6f

Browse files
rbrenopenhands-agent
authored and
mamoodi
committed
dont return asterisks for api key (#7654)
Co-authored-by: openhands <[email protected]>
1 parent 0a4307a commit ab0ab6f

File tree

14 files changed

+39
-32
lines changed

14 files changed

+39
-32
lines changed

‎frontend/__tests__/hooks/mutation/use-save-settings.test.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ describe("useSaveSettings", () => {
1818
),
1919
});
2020

21-
result.current.mutate({ LLM_API_KEY: "" });
21+
result.current.mutate({ llm_api_key: "" });
2222
await waitFor(() => {
2323
expect(saveSettingsSpy).toHaveBeenCalledWith(
2424
expect.objectContaining({
@@ -27,7 +27,7 @@ describe("useSaveSettings", () => {
2727
);
2828
});
2929

30-
result.current.mutate({ LLM_API_KEY: null });
30+
result.current.mutate({ llm_api_key: null });
3131
await waitFor(() => {
3232
expect(saveSettingsSpy).toHaveBeenCalledWith(
3333
expect.objectContaining({

‎frontend/__tests__/routes/settings.test.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ describe("Settings Screen", () => {
395395
it("should render an indicator if the LLM API key is set", async () => {
396396
getSettingsSpy.mockResolvedValueOnce({
397397
...MOCK_DEFAULT_USER_SETTINGS,
398-
llm_api_key: "**********",
398+
llm_api_key_set: true,
399399
});
400400

401401
renderSettingsScreen();
@@ -416,7 +416,7 @@ describe("Settings Screen", () => {
416416
it("should set '<hidden>' placeholder if the LLM API key is set", async () => {
417417
getSettingsSpy.mockResolvedValueOnce({
418418
...MOCK_DEFAULT_USER_SETTINGS,
419-
llm_api_key: "**********",
419+
llm_api_key_set: true,
420420
});
421421

422422
renderSettingsScreen();
@@ -971,7 +971,7 @@ describe("Settings Screen", () => {
971971
const user = userEvent.setup();
972972
getSettingsSpy.mockResolvedValue({
973973
...MOCK_DEFAULT_USER_SETTINGS,
974-
llm_api_key: "**********",
974+
llm_api_key_set: true,
975975
});
976976

977977
renderSettingsScreen();

‎frontend/src/components/shared/modals/settings/settings-form.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function SettingsForm({ settings, models, onClose }: SettingsFormProps) {
5050

5151
posthog.capture("settings_saved", {
5252
LLM_MODEL: newSettings.LLM_MODEL,
53-
LLM_API_KEY: newSettings.LLM_API_KEY ? "SET" : "UNSET",
53+
LLM_API_KEY_SET: newSettings.LLM_API_KEY_SET ? "SET" : "UNSET",
5454
REMOTE_RUNTIME_RESOURCE_FACTOR:
5555
newSettings.REMOTE_RUNTIME_RESOURCE_FACTOR,
5656
});
@@ -74,7 +74,7 @@ export function SettingsForm({ settings, models, onClose }: SettingsFormProps) {
7474
}
7575
};
7676

77-
const isLLMKeySet = settings.LLM_API_KEY === "**********";
77+
const isLLMKeySet = settings.LLM_API_KEY_SET;
7878

7979
return (
8080
<div>

‎frontend/src/hooks/mutation/use-save-settings.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const saveSettingsMutationFn = async (
1313
return;
1414
}
1515

16+
console.log("Save settings", settings);
1617
const apiSettings: Partial<PostApiSettings> = {
1718
llm_model: settings.LLM_MODEL,
1819
llm_base_url: settings.LLM_BASE_URL,
@@ -21,9 +22,9 @@ const saveSettingsMutationFn = async (
2122
confirmation_mode: settings.CONFIRMATION_MODE,
2223
security_analyzer: settings.SECURITY_ANALYZER,
2324
llm_api_key:
24-
settings.LLM_API_KEY === ""
25+
settings.llm_api_key === ""
2526
? ""
26-
: settings.LLM_API_KEY?.trim() || undefined,
27+
: settings.llm_api_key?.trim() || undefined,
2728
remote_runtime_resource_factor: settings.REMOTE_RUNTIME_RESOURCE_FACTOR,
2829
enable_default_condenser: settings.ENABLE_DEFAULT_CONDENSER,
2930
enable_sound_notifications: settings.ENABLE_SOUND_NOTIFICATIONS,

‎frontend/src/hooks/query/use-settings.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const getSettingsQueryFn = async () => {
1515
LANGUAGE: apiSettings.language,
1616
CONFIRMATION_MODE: apiSettings.confirmation_mode,
1717
SECURITY_ANALYZER: apiSettings.security_analyzer,
18-
LLM_API_KEY: apiSettings.llm_api_key,
18+
LLM_API_KEY_SET: apiSettings.llm_api_key_set,
1919
REMOTE_RUNTIME_RESOURCE_FACTOR: apiSettings.remote_runtime_resource_factor,
2020
PROVIDER_TOKENS_SET: apiSettings.provider_tokens_set,
2121
ENABLE_DEFAULT_CONDENSER: apiSettings.enable_default_condenser,
@@ -45,10 +45,10 @@ export const useSettings = () => {
4545
});
4646

4747
React.useEffect(() => {
48-
if (query.isFetched && query.data?.LLM_API_KEY) {
48+
if (query.isFetched && query.data?.LLM_API_KEY_SET) {
4949
posthog.capture("user_activated");
5050
}
51-
}, [query.data?.LLM_API_KEY, query.isFetched]);
51+
}, [query.data?.LLM_API_KEY_SET, query.isFetched]);
5252

5353
React.useEffect(() => {
5454
if (query.data?.PROVIDER_TOKENS_SET) {

‎frontend/src/mocks/handlers.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import { GitUser } from "#/types/git";
1212
export const MOCK_DEFAULT_USER_SETTINGS: ApiSettings | PostApiSettings = {
1313
llm_model: DEFAULT_SETTINGS.LLM_MODEL,
1414
llm_base_url: DEFAULT_SETTINGS.LLM_BASE_URL,
15-
llm_api_key: DEFAULT_SETTINGS.LLM_API_KEY,
15+
llm_api_key: null,
16+
llm_api_key_set: DEFAULT_SETTINGS.LLM_API_KEY_SET,
1617
agent: DEFAULT_SETTINGS.AGENT,
1718
language: DEFAULT_SETTINGS.LANGUAGE,
1819
confirmation_mode: DEFAULT_SETTINGS.CONFIRMATION_MODE,

‎frontend/src/routes/account-settings.tsx

+7-7
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ function AccountSettings() {
7777
providerTokensSet.includes(ProviderOptions.github) || false;
7878
const isGitLabTokenSet =
7979
providerTokensSet.includes(ProviderOptions.gitlab) || false;
80-
const isLLMKeySet = settings?.LLM_API_KEY === "**********";
80+
const isLLMKeySet = settings?.LLM_API_KEY_SET;
8181
const isAnalyticsEnabled = settings?.USER_CONSENTS_TO_ANALYTICS;
8282
const isAdvancedSettingsSet = determineWhetherToToggleAdvancedSettings();
8383

@@ -120,11 +120,11 @@ function AccountSettings() {
120120
const enableSoundNotifications =
121121
formData.get("enable-sound-notifications-switch")?.toString() === "on";
122122
const llmBaseUrl = formData.get("base-url-input")?.toString() || "";
123+
const inputApiKey = formData.get("llm-api-key-input")?.toString() || "";
123124
const llmApiKey =
124-
formData.get("llm-api-key-input")?.toString() ||
125-
(isLLMKeySet
126-
? undefined // don't update if it's already set
127-
: ""); // reset if it's first time save to avoid 500 error
125+
inputApiKey === "" && isLLMKeySet
126+
? undefined // don't update if it's already set and input is empty
127+
: inputApiKey; // otherwise use the input value
128128

129129
const githubToken = formData.get("github-token-input")?.toString();
130130
const gitlabToken = formData.get("gitlab-token-input")?.toString();
@@ -151,7 +151,7 @@ function AccountSettings() {
151151
ENABLE_SOUND_NOTIFICATIONS: enableSoundNotifications,
152152
LLM_MODEL: finalLlmModel,
153153
LLM_BASE_URL: finalLlmBaseUrl,
154-
LLM_API_KEY: finalLlmApiKey,
154+
llm_api_key: finalLlmApiKey,
155155
AGENT: formData.get("agent-input")?.toString(),
156156
SECURITY_ANALYZER:
157157
formData.get("security-analyzer-input")?.toString() || "",
@@ -277,10 +277,10 @@ function AccountSettings() {
277277
label="API Key"
278278
type="password"
279279
className="w-[680px]"
280+
placeholder={isLLMKeySet ? "<hidden>" : ""}
280281
startContent={
281282
isLLMKeySet && <KeyStatusIcon isSet={isLLMKeySet} />
282283
}
283-
placeholder={isLLMKeySet ? "<hidden>" : ""}
284284
/>
285285
)}
286286

‎frontend/src/services/settings.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export const DEFAULT_SETTINGS: Settings = {
77
LLM_BASE_URL: "",
88
AGENT: "CodeActAgent",
99
LANGUAGE: "en",
10-
LLM_API_KEY: null,
10+
LLM_API_KEY_SET: false,
1111
CONFIRMATION_MODE: false,
1212
SECURITY_ANALYZER: "",
1313
REMOTE_RUNTIME_RESOURCE_FACTOR: 1,

‎frontend/src/types/config-type.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ enum ArgConfigType {
22
LLM_MODEL = "LLM_MODEL",
33
AGENT = "AGENT",
44
LANGUAGE = "LANGUAGE",
5-
LLM_API_KEY = "LLM_API_KEY",
5+
LLM_API_KEY_SET = "LLM_API_KEY_SET",
66
}
77

88
const SupportedSettings: string[] = [
99
ArgConfigType.LLM_MODEL,
1010
ArgConfigType.AGENT,
1111
ArgConfigType.LANGUAGE,
12-
ArgConfigType.LLM_API_KEY,
12+
ArgConfigType.LLM_API_KEY_SET,
1313
];
1414

1515
export { ArgConfigType, SupportedSettings };

‎frontend/src/types/core/variances.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export interface InitConfig {
1717
AGENT: string;
1818
CONFIRMATION_MODE: boolean;
1919
LANGUAGE: string;
20-
LLM_API_KEY: string;
20+
LLM_API_KEY_SET: boolean;
2121
LLM_MODEL: string;
2222
};
2323
token?: string;

‎frontend/src/types/settings.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export type Settings = {
1010
LLM_BASE_URL: string;
1111
AGENT: string;
1212
LANGUAGE: string;
13-
LLM_API_KEY: string | null;
13+
LLM_API_KEY_SET: boolean;
1414
CONFIRMATION_MODE: boolean;
1515
SECURITY_ANALYZER: string;
1616
REMOTE_RUNTIME_RESOURCE_FACTOR: number | null;
@@ -28,6 +28,7 @@ export type ApiSettings = {
2828
agent: string;
2929
language: string;
3030
llm_api_key: string | null;
31+
llm_api_key_set: boolean;
3132
confirmation_mode: boolean;
3233
security_analyzer: string;
3334
remote_runtime_resource_factor: number | null;
@@ -41,6 +42,7 @@ export type ApiSettings = {
4142
export type PostSettings = Settings & {
4243
provider_tokens: Record<Provider, string>;
4344
user_consents_to_analytics: boolean | null;
45+
llm_api_key?: string | null;
4446
};
4547

4648
export type PostApiSettings = ApiSettings & {

‎frontend/src/utils/settings-utils.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ const extractAdvancedFormData = (formData: FormData) => {
4747
};
4848
};
4949

50-
export const extractSettings = (formData: FormData): Partial<Settings> => {
50+
export const extractSettings = (
51+
formData: FormData,
52+
): Partial<Settings> & { llm_api_key?: string | null } => {
5153
const { LLM_MODEL, LLM_API_KEY, AGENT, LANGUAGE } =
5254
extractBasicFormData(formData);
5355

@@ -73,13 +75,14 @@ export const extractSettings = (formData: FormData): Partial<Settings> => {
7375

7476
return {
7577
LLM_MODEL: CUSTOM_LLM_MODEL || LLM_MODEL,
76-
LLM_API_KEY,
78+
LLM_API_KEY_SET: !!LLM_API_KEY,
7779
AGENT,
7880
LANGUAGE,
7981
LLM_BASE_URL,
8082
CONFIRMATION_MODE,
8183
SECURITY_ANALYZER,
8284
ENABLE_DEFAULT_CONDENSER,
8385
PROVIDER_TOKENS: providerTokens,
86+
llm_api_key: LLM_API_KEY,
8487
};
8588
};

‎openhands/server/routes/settings.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from openhands.server.shared import SettingsStoreImpl, config, server_config
1111
from openhands.server.types import AppMode
1212

13-
1413
app = APIRouter(prefix='/api')
1514

1615

@@ -27,10 +26,10 @@ async def load_settings(request: Request) -> GETSettingsModel | JSONResponse:
2726
)
2827

2928
provider_tokens_set = {}
30-
29+
3130
if bool(user_id):
3231
provider_tokens_set[ProviderType.GITHUB.value] = True
33-
32+
3433
provider_tokens = get_provider_tokens(request)
3534
if provider_tokens:
3635
all_provider_types = [provider.value for provider in ProviderType]
@@ -43,10 +42,10 @@ async def load_settings(request: Request) -> GETSettingsModel | JSONResponse:
4342

4443
settings_with_token_data = GETSettingsModel(
4544
**settings.model_dump(exclude='secrets_store'),
45+
llm_api_key_set=settings.llm_api_key is not None,
4646
provider_tokens_set=provider_tokens_set,
4747
)
48-
49-
settings_with_token_data.llm_api_key = settings.llm_api_key
48+
settings_with_token_data.llm_api_key = None
5049
return settings_with_token_data
5150
except Exception as e:
5251
logger.warning(f'Invalid token: {e}')

‎openhands/server/settings.py

+1
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,4 @@ class GETSettingsModel(Settings):
121121
"""
122122

123123
provider_tokens_set: dict[str, bool] | None = None
124+
llm_api_key_set: bool

0 commit comments

Comments
 (0)