Skip to content

fix(api-manager): show validation errors inline in modals, not behind #1920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 2 commits into from
May 4, 2026
Merged

fix(api-manager): show validation errors inline in modals, not behind #1920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b64786c
fix(api-manager): show validation errors inline in modals, not behind…
andrewmunsell May 4, 2026
e2e9344
Merge release/v3.7.9 into PR branch
diegosouzapw May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion open-sse/services/tokenRefresh.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,12 @@ export async function getAccessToken(provider, credentials, log, proxyConfig: un
}

const entry = { promise: null, waiters: 0 };
entry.promise = _getAccessTokenWithStalenessCheck(provider, credentials, log, proxyConfig).finally(() => {
entry.promise = _getAccessTokenWithStalenessCheck(
provider,
credentials,
log,
proxyConfig
).finally(() => {
connectionRefreshMutex.delete(connectionId);
});
connectionRefreshMutex.set(connectionId, entry);
Expand Down
125 changes: 96 additions & 29 deletions src/app/(dashboard)/dashboard/api-manager/ApiManagerPageClient.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ export default function ApiManagerPageClient() {
const [editingKey, setEditingKey] = useState<ApiKey | null>(null);
const [showPermissionsModal, setShowPermissionsModal] = useState(false);
const [searchModel, setSearchModel] = useState("");
const [error, setError] = useState<string | null>(null);
const [pageError, setPageError] = useState<string | null>(null);
const [nameError, setNameError] = useState<string | null>(null);
const [createError, setCreateError] = useState<string | null>(null);
const [isSubmitting, setIsSubmitting] = useState(false);
const [usageStats, setUsageStats] = useState<Record<string, KeyUsageStats>>({});
const [sessionCounts, setSessionCounts] = useState<Record<string, number>>({});
Expand Down Expand Up @@ -226,19 +228,20 @@ export default function ApiManagerPageClient() {
}
};

const clearError = useCallback(() => setError(null), []);
const clearPageError = useCallback(() => setPageError(null), []);

const handleCreateKey = async () => {
// Validate raw input first, then sanitize
const validation = validateKeyName(newKeyName, t);
if (!validation.valid) {
setError(validation.error || t("invalidKeyName"));
setNameError(validation.error || t("invalidKeyName"));
return;
}
const sanitizedName = sanitizeInput(newKeyName);

setIsSubmitting(true);
clearError();
setNameError(null);
setCreateError(null);

try {
const res = await fetch("/api/keys", {
Expand All @@ -254,11 +257,11 @@ export default function ApiManagerPageClient() {
setNewKeyName("");
setShowAddModal(false);
} else {
setError(data.error || t("failedCreateKey"));
setCreateError(data.error || t("failedCreateKey"));
}
} catch (error) {
console.error("Error creating key:", error);
setError(t("failedCreateKeyRetry"));
setCreateError(t("failedCreateKeyRetry"));
} finally {
setIsSubmitting(false);
}
Expand All @@ -267,26 +270,26 @@ export default function ApiManagerPageClient() {
const handleDeleteKey = async (id: string) => {
// Validate ID format to prevent injection
if (!id || typeof id !== "string" || !/^[a-zA-Z0-9_-]+$/.test(id)) {
setError(t("invalidKeyId"));
setPageError(t("invalidKeyId"));
return;
}

if (!confirm(t("deleteConfirm"))) return;

setIsSubmitting(true);
clearError();
clearPageError();

try {
const res = await fetch(`/api/keys/${encodeURIComponent(id)}`, { method: "DELETE" });
if (res.ok) {
setKeys((prev) => prev.filter((k) => k.id !== id));
} else {
const data = await res.json();
setError(data.error || t("failedDeleteKey"));
setPageError(data.error || t("failedDeleteKey"));
}
} catch (error) {
console.error("Error deleting key:", error);
setError(t("failedDeleteKeyRetry"));
setPageError(t("failedDeleteKeyRetry"));
} finally {
setIsSubmitting(false);
}
Expand Down Expand Up @@ -329,23 +332,15 @@ export default function ApiManagerPageClient() {
) => {
if (!editingKey || !editingKey.id) return;

// Validate raw input first, then sanitize
const nameValidation = validateKeyName(name, t);
if (!nameValidation.valid) {
setError(nameValidation.error || t("invalidKeyName"));
return;
}
const sanitizedName = sanitizeInput(name);

// Validate models array
if (!Array.isArray(allowedModels)) {
setError(t("invalidModelsSelection"));
return;
Comment thread
andrewmunsell marked this conversation as resolved.
}

// Limit number of selected models to prevent abuse
if (allowedModels.length > MAX_SELECTED_MODELS) {
setError(t("cannotSelectMoreThanModels", { max: MAX_SELECTED_MODELS }));
return;
Comment thread
andrewmunsell marked this conversation as resolved.
}

Expand All @@ -364,7 +359,7 @@ export default function ApiManagerPageClient() {
: 0;

setIsSubmitting(true);
clearError();
clearPageError();

try {
const res = await fetch(`/api/keys/${encodeURIComponent(editingKey.id)}`, {
Expand All @@ -388,11 +383,11 @@ export default function ApiManagerPageClient() {
setEditingKey(null);
} else {
const data = await res.json();
setError(data.error || t("failedUpdatePermissions"));
setPageError(data.error || t("failedUpdatePermissions"));
}
} catch (error) {
console.error("Error updating permissions:", error);
setError(t("failedUpdatePermissionsRetry"));
setPageError(t("failedUpdatePermissionsRetry"));
} finally {
setIsSubmitting(false);
}
Expand Down Expand Up @@ -441,12 +436,12 @@ export default function ApiManagerPageClient() {
return (
<div className="flex flex-col gap-8">
{/* Error Banner */}
{error && (
{pageError && (
<div className="flex items-center gap-3 p-4 bg-red-500/10 border border-red-500/30 rounded-lg">
<span className="material-symbols-outlined text-red-500">error</span>
<p className="text-sm text-red-700 dark:text-red-300 flex-1">{error}</p>
<p className="text-sm text-red-700 dark:text-red-300 flex-1">{pageError}</p>
<button
onClick={clearError}
onClick={clearPageError}
className="text-red-500 hover:text-red-700 transition-colors"
>
<span className="material-symbols-outlined">close</span>
Expand Down Expand Up @@ -520,7 +515,15 @@ export default function ApiManagerPageClient() {
<h2 className="text-lg font-semibold">{t("keyManagement")}</h2>
<p className="text-sm text-text-muted">{t("keyManagementDesc")}</p>
</div>
<Button icon="add" onClick={() => setShowAddModal(true)}>
<Button
icon="add"
onClick={() => {
setNameError(null);
setCreateError(null);
clearPageError();
setShowAddModal(true);
}}
Comment thread
andrewmunsell marked this conversation as resolved.
>
{t("createKey")}
</Button>
</div>
Expand Down Expand Up @@ -554,7 +557,14 @@ export default function ApiManagerPageClient() {
</div>
<p className="text-text-main font-medium mb-2">{t("noKeys")}</p>
<p className="text-sm text-text-muted mb-4">{t("noKeysDesc")}</p>
<Button icon="add" onClick={() => setShowAddModal(true)}>
<Button
icon="add"
onClick={() => {
setNameError(null);
setCreateError(null);
setShowAddModal(true);
}}
>
{t("createFirstKey")}
</Button>
</div>
Expand Down Expand Up @@ -759,6 +769,8 @@ export default function ApiManagerPageClient() {
onClose={() => {
setShowAddModal(false);
setNewKeyName("");
setNameError(null);
setCreateError(null);
}}
>
<div className="flex flex-col gap-4">
Expand All @@ -768,25 +780,42 @@ export default function ApiManagerPageClient() {
</label>
<Input
value={newKeyName}
onChange={(e) => setNewKeyName(e.target.value)}
onChange={(e) => {
setNewKeyName(e.target.value);
setNameError(null);
}}
placeholder={t("keyNamePlaceholder")}
maxLength={MAX_KEY_NAME_LENGTH}
error={nameError}
autoFocus
/>
<p className="text-xs text-text-muted mt-1.5">{t("keyNameDesc")}</p>
</div>
{createError && (
<div className="flex items-center gap-2 px-3 py-2 rounded-lg bg-red-500/10 border border-red-500/30">
<span className="material-symbols-outlined text-red-500 text-sm">error</span>
<p className="text-sm text-red-700 dark:text-red-300 flex-1">{createError}</p>
</div>
)}
<div className="flex gap-2">
<Button
onClick={() => {
setShowAddModal(false);
setNewKeyName("");
setNameError(null);
setCreateError(null);
}}
variant="ghost"
fullWidth
>
{tc("cancel")}
</Button>
<Button onClick={handleCreateKey} fullWidth disabled={!newKeyName.trim()}>
<Button
onClick={handleCreateKey}
fullWidth
disabled={!newKeyName.trim()}
loading={isSubmitting}
>
{t("createKey")}
</Button>
</div>
Expand Down Expand Up @@ -905,6 +934,8 @@ const PermissionsModal = memo(function PermissionsModal({
const [scheduleTz, setScheduleTz] = useState(
apiKey?.accessSchedule?.tz ?? Intl.DateTimeFormat().resolvedOptions().timeZone
);
const [nameError, setNameError] = useState<string | null>(null);
const [saveError, setSaveError] = useState<string | null>(null);
const [selectedConnections, setSelectedConnections] = useState<string[]>(initialConnections);
const [allowAllConnections, setAllowAllConnections] = useState(initialConnections.length === 0);
const [expandedProviders, setExpandedProviders] = useState<Set<string>>(() => {
Expand Down Expand Up @@ -992,6 +1023,29 @@ const PermissionsModal = memo(function PermissionsModal({
);

const handleSave = useCallback(() => {
// Clear previous inline errors
setNameError(null);
setSaveError(null);

// Validate name inline before calling onSave
const validation = validateKeyName(keyName, t);
if (!validation.valid) {
setNameError(validation.error || t("invalidKeyName"));
return;
}

// Validate models selection
if (!allowAll && !Array.isArray(selectedModels)) {
setSaveError(t("invalidModelsSelection"));
return;
}

// Limit number of selected models to prevent abuse
if (!allowAll && selectedModels.length > MAX_SELECTED_MODELS) {
setSaveError(t("cannotSelectMoreThanModels", { max: MAX_SELECTED_MODELS }));
return;
}

const schedule: AccessSchedule | null = scheduleEnabled
? {
enabled: true,
Expand Down Expand Up @@ -1027,6 +1081,7 @@ const PermissionsModal = memo(function PermissionsModal({
scheduleUntil,
scheduleDays,
scheduleTz,
t,
]);

const selectedCount = selectedModels.length;
Expand All @@ -1048,13 +1103,25 @@ const PermissionsModal = memo(function PermissionsModal({
<div className="w-48 shrink-0">
<Input
value={keyName}
onChange={(e) => setKeyName(e.target.value)}
onChange={(e) => {
setKeyName(e.target.value);
setNameError(null);
}}
placeholder={t("keyNamePlaceholder")}
maxLength={MAX_KEY_NAME_LENGTH}
error={nameError}
/>
</div>
</div>

{/* Inline save error */}
{saveError && (
<div className="flex items-center gap-2 px-3 py-2 rounded-lg bg-red-500/10 border border-red-500/30">
<span className="material-symbols-outlined text-red-500 text-sm">error</span>
<p className="text-sm text-red-700 dark:text-red-300 flex-1">{saveError}</p>
</div>
)}

{/* Access Mode Toggle */}
<div className="flex gap-2 p-1 bg-surface rounded-lg">
<button
Expand Down
7 changes: 3 additions & 4 deletions src/lib/db/migrationRunner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,10 +777,9 @@ export function runMigrations(db: Database.Database, options?: { isNewDb?: boole
// "duplicate column name" means the column already exists — end state achieved, mark applied.
if (message.includes("duplicate column name")) {
const applyMarkerOnly = db.transaction(() => {
db.prepare("INSERT OR IGNORE INTO _omniroute_migrations (version, name) VALUES (?, ?)").run(
migration.version,
migration.name
);
db.prepare(
"INSERT OR IGNORE INTO _omniroute_migrations (version, name) VALUES (?, ?)"
).run(migration.version, migration.name);
});
applyMarkerOnly();
count++;
Expand Down
Loading