Skip to content

Commit eff212d

Browse files
committed
fix: plug remaining injection and UX gaps
isSafeEnvValue: - Reject '$' β€” dotenv / @next/env expand `${OTHER_VAR}` on re-parse, which would let a submitted value exfiltrate or overwrite sibling env contents. - Reject whitespace β€” writeEnvFile doesn't quote, so an unquoted `KEY=a b` gets parsed as just `a` on the next read. Rejecting here is simpler than asymmetric quoting and no legitimate key/URL value needs spaces or tabs. settings-panel: - fetchKeys now returns "ok" | "rejected" | "error" so saveAdminToken can tell a 401 apart from a network blip. A network failure after a save no longer falsely claims the token was rejected. - clearAdminToken and the open-effect mark fetchKeys/fetchSkills as void to silence @typescript-eslint/no-floating-promises.
1 parent 77b45d2 commit eff212d

2 files changed

Lines changed: 44 additions & 25 deletions

File tree

β€Žagent-templates/next/app/(agent)/_components/settings-panel.tsxβ€Ž

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -425,31 +425,39 @@ export default function SettingsPanel({ config, onChange }: { config: AgentConfi
425425
setTokenSet(getConfigToken() !== "");
426426
}, [open]);
427427

428-
// Returns true if the fetch authenticated (or no auth was needed) so
429-
// callers like saveAdminToken can surface a "rejected" message.
430-
const fetchKeys = useCallback(async (): Promise<boolean> => {
428+
// Distinguishes the three outcomes so callers can show an accurate
429+
// message β€” a network blip should not look like "token rejected".
430+
type FetchKeysResult = "ok" | "rejected" | "error";
431+
const fetchKeys = useCallback(async (): Promise<FetchKeysResult> => {
432+
let res: Response;
431433
try {
432434
// On hosted deployments the GET is guarded too β€” attach token if we have one.
433-
const res = await fetch("/api/config", { headers: configHeaders(false) });
434-
if (res.status === 401 || res.status === 403) {
435-
// The token (if any) is rejected. Drop it so the user doesn't
436-
// stay locked out on the next render with stale creds in storage.
437-
window.localStorage.removeItem(CONFIG_TOKEN_KEY);
438-
setTokenSet(false);
439-
setAuthRequired(true);
440-
setHosted(true);
441-
return false;
442-
}
443-
if (res.ok) {
435+
res = await fetch("/api/config", { headers: configHeaders(false) });
436+
} catch {
437+
return "error";
438+
}
439+
if (res.status === 401 || res.status === 403) {
440+
// The token (if any) is rejected. Drop it so the user doesn't
441+
// stay locked out on the next render with stale creds in storage.
442+
window.localStorage.removeItem(CONFIG_TOKEN_KEY);
443+
setTokenSet(false);
444+
setAuthRequired(true);
445+
setHosted(true);
446+
return "rejected";
447+
}
448+
if (res.ok) {
449+
try {
444450
const data: ConfigResponse = await res.json();
445451
setKeyStatuses(data.keys);
446452
setValueStatuses(data.values ?? {});
447453
setHosted(data.hosted);
448454
setAuthRequired(false);
449-
return true;
455+
return "ok";
456+
} catch {
457+
return "error";
450458
}
451-
} catch { /* noop */ }
452-
return false;
459+
}
460+
return "error";
453461
}, []);
454462

455463
const saveAdminToken = async () => {
@@ -458,8 +466,12 @@ export default function SettingsPanel({ config, onChange }: { config: AgentConfi
458466
window.localStorage.setItem(CONFIG_TOKEN_KEY, t);
459467
setTokenSet(true);
460468
setTokenDraft("");
461-
const ok = await fetchKeys();
462-
setSaveMsg(ok ? "Admin token saved" : "Token rejected by server");
469+
const result = await fetchKeys();
470+
const msg =
471+
result === "ok" ? "Admin token saved"
472+
: result === "rejected" ? "Token rejected by server"
473+
: "Network error β€” token saved locally, retry when reachable";
474+
setSaveMsg(msg);
463475
setTimeout(() => setSaveMsg(""), 3000);
464476
};
465477

@@ -468,7 +480,7 @@ export default function SettingsPanel({ config, onChange }: { config: AgentConfi
468480
setTokenSet(false);
469481
setSaveMsg("Admin token cleared");
470482
setTimeout(() => setSaveMsg(""), 3000);
471-
fetchKeys();
483+
void fetchKeys();
472484
};
473485

474486
const fetchSkills = useCallback(async () => {
@@ -484,7 +496,7 @@ export default function SettingsPanel({ config, onChange }: { config: AgentConfi
484496
}, []);
485497

486498
useEffect(() => {
487-
if (open) { fetchKeys(); fetchSkills(); }
499+
if (open) { void fetchKeys(); void fetchSkills(); }
488500
}, [open, fetchKeys, fetchSkills]);
489501

490502
const saveKey = async (id: string) => {

β€Žagent-templates/next/app/(agent)/_lib/config/keys.tsβ€Ž

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,19 @@ export function requireConfigReadAuth(req: Request): Response | null {
118118
* parser (e.g. `@next/env`, `dotenv`) re-reads the file
119119
* \ β€” backslash escapes are parser-dependent; reject to keep
120120
* round-trip predictable
121-
* # β€” leading/mid-line `#` is treated as a comment by most
122-
* dotenv parsers and would truncate the value silently
123-
* API keys never legitimately contain any of these, so rejecting is safe.
121+
* # β€” a mid-line `#` is treated as a comment by most dotenv
122+
* parsers and would truncate the value silently
123+
* $ β€” `${OTHER_VAR}` triggers variable expansion in dotenv /
124+
* @next/env and would let a submitted value exfiltrate
125+
* other env contents on re-parse
126+
* whitespace β€” unquoted space/tab in `KEY=a b` gets parsed as just
127+
* `a`; writeEnvFile doesn't quote, so reject here instead
128+
* of adding asymmetric quoting logic
129+
* API keys, base URLs, OAuth/JWT tokens, and base64 strings never
130+
* legitimately contain any of these, so rejecting is safe.
124131
*/
125132
export function isSafeEnvValue(v: unknown): v is string {
126-
return typeof v === "string" && !/[\r\n\0"'\\#]/.test(v);
133+
return typeof v === "string" && !/[\r\n\0"'\\#$\s]/.test(v);
127134
}
128135

129136
// Strip one layer of balanced surrounding quotes (" or ') from a .env value.

0 commit comments

Comments
Β (0)