feat: comprehensive code review fixes — security, performance, quality
Backend security: - Reject Gitea webhooks when webhook_secret is empty (was silently skipping) - Add slowapi rate limiting on login (5/min) and setup (3/min) endpoints - Add CORS middleware with configurable origins - Mask telegram_webhook_secret in settings API response - Protect system-owned command template configs from regular user modification - Increase minimum password length to 8 characters Backend performance: - Batch queries in _resolve_command_context (3 queries instead of 3N) - Concurrent album fetching with asyncio.gather in immich commands - Singleton Jinja2 SandboxedEnvironment (reuse instead of per-render creation) - TTLCache for rate limits (bounded memory, auto-eviction) - Optional aiohttp session reuse in send_reply/send_media_group Backend code quality: - Extract dispatch_helpers.py (shared link_data loading + event filtering) - Extract database/seeds.py from main.py (490 lines → dedicated module) - Split immich_handler.py (415 lines) into commands/immich/ subpackage - Replace bare except blocks with logged warnings - Add per-provider config validation (Pydantic models) - Truncate command input to 512 chars - Expose usage_* and desc_* slots in capabilities and variables API Frontend security: - CSS.escape() for user-controlled querySelector in highlight.ts - Client-side password min 8 chars validation on setup and password change Frontend code quality: - Replace any types with proper interfaces across top files - Decompose targets/+page.svelte into TargetForm + ReceiverSection - Fix $derived.by usage, $state mutation patterns - Add console.warn to empty catch blocks Frontend UX: - Auth redirect via goto() with "Redirecting..." state - Platform-aware Ctrl/Cmd K keyboard hint - Remove stat-card hover transform Frontend accessibility: - Modal: role=dialog, aria-modal, focus trap, restore focus - EntitySelect/IconGridSelect: listbox/option roles, aria-selected/disabled
This commit is contained in:
@@ -11,9 +11,10 @@
|
||||
import ConfirmModal from '$lib/components/ConfirmModal.svelte';
|
||||
import IconButton from '$lib/components/IconButton.svelte';
|
||||
import CrossLink from '$lib/components/CrossLink.svelte';
|
||||
import EntitySelect from '$lib/components/EntitySelect.svelte';
|
||||
import { snackSuccess, snackError } from '$lib/stores/snackbar.svelte';
|
||||
import { highlightFromUrl } from '$lib/highlight';
|
||||
import type { Tracker, TrackingConfig, TemplateConfig } from '$lib/types';
|
||||
import type { Tracker, TrackerTarget, TrackingConfig, TemplateConfig, NotificationTarget } from '$lib/types';
|
||||
|
||||
import TrackerForm from './TrackerForm.svelte';
|
||||
import LinkedTargetsSection from './LinkedTargetsSection.svelte';
|
||||
@@ -34,7 +35,7 @@
|
||||
let targets = $derived(targetsCache.items);
|
||||
let trackingConfigs = $derived(trackingConfigsCache.items);
|
||||
let templateConfigs = $derived(templateConfigsCache.items);
|
||||
let collections = $state<any[]>([]);
|
||||
let collections = $state<Record<string, any>[]>([]);
|
||||
let showForm = $state(false);
|
||||
let editing = $state<number | null>(null);
|
||||
let collectionFilter = $state('');
|
||||
@@ -44,7 +45,7 @@
|
||||
let ttTesting = $state<Record<string, string>>({});
|
||||
|
||||
// Shared link validation
|
||||
let linkWarning = $state<{ albums: any[], providerId: number } | null>(null);
|
||||
let linkWarning = $state<{ albums: { id: string; name: string; issue: string }[], providerId: number } | null>(null);
|
||||
let linkCheckLoading = $state(false);
|
||||
let linkCreating = $state(false);
|
||||
let previousCollectionIds = $state<string[]>([]);
|
||||
@@ -83,7 +84,7 @@
|
||||
];
|
||||
|
||||
let testMenuTrackerId = $state<number | null>(null);
|
||||
let testTypes = $derived(() => {
|
||||
let testTypes = $derived.by(() => {
|
||||
if (!testMenuTrackerId) return defaultTestTypes;
|
||||
const tracker = notificationTrackers.find(t => t.id === testMenuTrackerId);
|
||||
if (!tracker) return defaultTestTypes;
|
||||
@@ -98,7 +99,7 @@
|
||||
loadError = '';
|
||||
try {
|
||||
[allNotificationTrackers] = await Promise.all([
|
||||
api('/notification-trackers'),
|
||||
api<Tracker[]>('/notification-trackers'),
|
||||
providersCache.fetch(), targetsCache.fetch(),
|
||||
trackingConfigsCache.fetch(), templateConfigsCache.fetch(),
|
||||
]);
|
||||
@@ -110,7 +111,7 @@
|
||||
|
||||
async function loadCollections() {
|
||||
if (!form.provider_id) return;
|
||||
try { collections = await api(`/providers/${form.provider_id}/collections`); } catch { collections = []; }
|
||||
try { collections = await api(`/providers/${form.provider_id}/collections`); } catch (e) { console.warn('Failed to load collections:', e); collections = []; }
|
||||
}
|
||||
|
||||
let _prevProviderId = 0;
|
||||
@@ -123,7 +124,7 @@
|
||||
|
||||
function openNew() { form = defaultForm(); editing = null; showForm = true; collections = []; previousCollectionIds = []; }
|
||||
|
||||
async function edit(trk: any) {
|
||||
async function edit(trk: Tracker) {
|
||||
form = {
|
||||
name: trk.name, icon: trk.icon || '', provider_id: trk.provider_id,
|
||||
collection_ids: [...(trk.collection_ids || [])],
|
||||
@@ -143,13 +144,14 @@
|
||||
if (newAlbumIds.length > 0 && form.provider_id) {
|
||||
linkCheckLoading = true;
|
||||
try {
|
||||
const missingAlbums: any[] = [];
|
||||
const missingAlbums: { id: string; name: string; issue: string }[] = [];
|
||||
for (const albumId of newAlbumIds) {
|
||||
const links = await api(`/providers/${form.provider_id}/albums/${albumId}/shared-links`);
|
||||
const validLink = (links as any[]).find((l: any) => l.is_accessible && !l.is_expired);
|
||||
interface SharedLink { is_accessible: boolean; is_expired: boolean; has_password: boolean }
|
||||
const links = await api<SharedLink[]>(`/providers/${form.provider_id}/albums/${albumId}/shared-links`);
|
||||
const validLink = links.find((l) => l.is_accessible && !l.is_expired);
|
||||
if (!validLink) {
|
||||
const album = collections.find(c => c.id === albumId);
|
||||
const problematicLink = (links as any[]).find((l: any) => l.is_expired || l.has_password);
|
||||
const problematicLink = links.find((l) => l.is_expired || l.has_password);
|
||||
missingAlbums.push({
|
||||
id: albumId,
|
||||
name: album?.albumName || album?.name || albumId,
|
||||
@@ -164,7 +166,7 @@
|
||||
linkCheckLoading = false;
|
||||
return;
|
||||
}
|
||||
} catch { /* Proceed if check fails */ }
|
||||
} catch (e) { console.warn('Shared link check failed, proceeding:', e); }
|
||||
linkCheckLoading = false;
|
||||
}
|
||||
|
||||
@@ -210,7 +212,7 @@
|
||||
await doSave();
|
||||
}
|
||||
|
||||
async function toggle(tracker: any) {
|
||||
async function toggle(tracker: Tracker) {
|
||||
if (toggling[tracker.id]) return;
|
||||
toggling = { ...toggling, [tracker.id]: true };
|
||||
try {
|
||||
@@ -220,7 +222,7 @@
|
||||
} catch (err: any) { snackError(err.message); } finally { toggling = { ...toggling, [tracker.id]: false }; }
|
||||
}
|
||||
|
||||
function startDelete(tracker: any) { confirmDelete = tracker; }
|
||||
function startDelete(tracker: Tracker) { confirmDelete = tracker; }
|
||||
|
||||
async function doDelete() {
|
||||
if (!confirmDelete) return;
|
||||
@@ -243,7 +245,7 @@
|
||||
try {
|
||||
const d = new Date(dateStr);
|
||||
return d.toLocaleDateString(undefined, { month: 'short', day: 'numeric', year: 'numeric' });
|
||||
} catch { return ''; }
|
||||
} catch (e) { console.warn('Date format error:', e); return ''; }
|
||||
}
|
||||
|
||||
// --- Linked Targets helpers ---
|
||||
@@ -252,7 +254,7 @@
|
||||
expandedTracker = trackerId;
|
||||
}
|
||||
|
||||
function getProviderType(tracker: any): string {
|
||||
function getProviderType(tracker: Tracker): string {
|
||||
const p = providers.find(p => p.id === tracker.provider_id);
|
||||
return p?.type || '';
|
||||
}
|
||||
@@ -262,13 +264,13 @@
|
||||
return p?.name || `#${id}`;
|
||||
}
|
||||
|
||||
function configsForTracker(tracker: any, configs: (TrackingConfig | TemplateConfig)[]): any[] {
|
||||
function configsForTracker(tracker: Tracker, configs: (TrackingConfig | TemplateConfig)[]): (TrackingConfig | TemplateConfig)[] {
|
||||
const pt = getProviderType(tracker);
|
||||
return pt ? configs.filter((c: any) => c.provider_type === pt) : configs;
|
||||
return pt ? configs.filter((c) => c.provider_type === pt) : configs;
|
||||
}
|
||||
|
||||
function getUnlinkedTargets(tracker: any): any[] {
|
||||
const linkedIds = new Set((tracker.tracker_targets || []).map((tt: any) => tt.target_id));
|
||||
function getUnlinkedTargets(tracker: Tracker): NotificationTarget[] {
|
||||
const linkedIds = new Set((tracker.tracker_targets || []).map((tt: TrackerTarget) => tt.target_id));
|
||||
return targets.filter(t => !linkedIds.has(t.id));
|
||||
}
|
||||
|
||||
@@ -302,7 +304,7 @@
|
||||
} catch (err: any) { snackError(err.message); }
|
||||
}
|
||||
|
||||
async function updateTargetLink(trackerId: number, tt: any, field: string, value: any) {
|
||||
async function updateTargetLink(trackerId: number, tt: TrackerTarget, field: string, value: string | number | boolean | null) {
|
||||
try {
|
||||
await api(`/notification-trackers/${trackerId}/targets/${tt.id}`, {
|
||||
method: 'PUT',
|
||||
@@ -331,12 +333,12 @@
|
||||
const btn = event.currentTarget as HTMLElement;
|
||||
const rect = btn.getBoundingClientRect();
|
||||
testMenuStyle = `position:fixed; z-index:9999; top:${rect.bottom + 4}px; right:${window.innerWidth - rect.right}px;`;
|
||||
testMenuTrackerId = notificationTrackers.find(t => t.tracker_targets?.some((x: any) => String(x.id) === String(ttId)))?.id ?? null;
|
||||
testMenuTrackerId = notificationTrackers.find(t => t.tracker_targets?.some((x: TrackerTarget) => String(x.id) === String(ttId)))?.id ?? null;
|
||||
testMenuOpen = testMenuOpen === String(ttId) ? null : String(ttId);
|
||||
}
|
||||
|
||||
function handleTestFromMenu(ttId: number, testType: string) {
|
||||
const trackerId = notificationTrackers.find(t => t.tracker_targets?.some((x: any) => String(x.id) === String(ttId)))?.id;
|
||||
const trackerId = notificationTrackers.find(t => t.tracker_targets?.some((x: TrackerTarget) => String(x.id) === String(ttId)))?.id;
|
||||
if (trackerId) testTrackerTarget(trackerId, ttId, testType);
|
||||
}
|
||||
</script>
|
||||
@@ -451,7 +453,7 @@
|
||||
{testMenuOpen}
|
||||
{testMenuStyle}
|
||||
{ttTesting}
|
||||
testTypes={testTypes()}
|
||||
testTypes={testTypes}
|
||||
ontest={handleTestFromMenu}
|
||||
onclose={() => testMenuOpen = null}
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user