-
Notifications
You must be signed in to change notification settings - Fork 4
Feat/w3ds evoting #290
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.
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
Feat/w3ds evoting #290
Conversation
Caution Review failedThe pull request is closed. WalkthroughAdds eVault monitoring to the Control Panel (API endpoints, client service, and UI). Implements server endpoints for pod details, logs, and metrics via kubectl. Enhances eID Wallet with deep-linking, auth guards, and scan/sign flows. Reworks eVoting frontend to QR/SSE auth, poll CRUD, and signing sessions; replaces evoting API with controller/service architecture, new entities, migrations, and a DB subscriber for Web3 adapter. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as eVoting Web (Login)
participant API as evoting-api
participant SSE as SSE Stream
participant Wallet as eID Wallet
Client->>API: GET /api/auth/offer
API-->>Client: { offer(w3ds://auth...), sessionId }
Client->>SSE: EventSource /api/auth/sessions/:sessionId
Note right of Client: display QR or deep-link
Wallet->>API: POST /api/auth { ename, session }
API-->>SSE: emit { user, token } on session
SSE-->>Client: message { user, token }
Client->>Client: Store token and initialize session
sequenceDiagram
participant Voter as eVoting Web (Vote page)
participant API as evoting-api
participant SSE as SSE(/signing)
participant Wallet as eID Wallet
Voter->>API: POST /api/signing/sessions { pollId, voteData, userId }
API-->>Voter: { sessionId, qrData }
Voter->>SSE: EventSource /api/signing/sessions/:id/status
Wallet->>API: POST /api/signing/callback { sessionId, signature, publicKey, message }
API-->>SSE: emit signed/completed (voteId)
SSE-->>Voter: signed/completed
Voter->>Voter: Refresh poll/results
sequenceDiagram
participant UI as Control Panel UI
participant Srv as EVaultService
participant API as /api/evaults...
participant K8s as kubectl (exec)
UI->>Srv: getEVaults()
Srv->>API: GET /api/evaults
API->>K8s: kubectl get ns/svc/pods (JSON)
K8s-->>API: JSON
API-->>Srv: { evaults: [...] }
Srv-->>UI: EVault[]
UI->>API: GET /api/evaults/:ns/:pod/logs|details|metrics
API->>K8s: kubectl logs/describe/top/get
K8s-->>API: Data
API-->>UI: JSON payloads
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 99
🔭 Outside diff range comments (7)
platforms/blabsy/src/components/login/login-main.tsx (3)
24-29
: Do not log auth tokens; add robust parsing and close SSE on successLogging tokens is sensitive. Also, JSON.parse may throw on non-token events. Close the SSE after successful sign-in to avoid duplicate events.
Apply this diff:
- eventSource.onmessage = async (e): Promise<void> => { - const data = JSON.parse(e.data as string) as { token: string }; - const { token } = data; - console.log(token); - await signInWithCustomToken(token); - }; + eventSource.onmessage = async (e): Promise<void> => { + try { + const data = JSON.parse(String(e.data)) as Partial<{ token: string }>; + if (data?.token) { + await signInWithCustomToken(data.token); + eventSource.close(); + } + } catch (err) { + console.error('Invalid SSE payload from auth session:', err); + } + };Optionally add:
eventSource.onerror = (err): void => { console.error('SSE error:', err); };
31-42
: Guard against missing or invalid session parameter in offer URInew URL(data.uri).searchParams.get('session') may return null or throw for invalid URIs. Casting to string masks this. Add proper validation to prevent bad SSE endpoints.
Apply this diff:
- setQr(data.uri); - watchEventStream( - new URL(data.uri).searchParams.get('session') as string - ); + setQr(data.uri); + let sessionId: string | null = null; + try { + sessionId = new URL(data.uri).searchParams.get('session'); + } catch { + console.error('Invalid offer URI received'); + } + if (sessionId) { + watchEventStream(sessionId); + } else { + console.error('Offer URI missing "session" query param.'); + }
44-52
: Close SSE and cancel in-flight requests on unmountAdd cleanup to avoid leaks when navigating away. Keep a ref to the EventSource and abort the axios request if the component unmounts mid-flight.
Outside the selected range, you can refactor:
// Add at top: import { useEffect, useRef, useState } from 'react'; const esRef = useRef<EventSource | null>(null); // Return EventSource from watchEventStream: function watchEventStream(id: string): EventSource { const sseUrl = new URL(`/api/auth/sessions/${id}`, process.env.NEXT_PUBLIC_BASE_URL).toString(); const eventSource = new EventSource(sseUrl, { withCredentials: true }); // ... handlers ... esRef.current = eventSource; return eventSource; } // Use AbortController for axios and cleanup: useEffect(() => { const ac = new AbortController(); (async () => { try { const { data } = await axios.get<{ uri: string }>( new URL('/api/auth/offer', process.env.NEXT_PUBLIC_BASE_URL || window.location.origin).toString(), { signal: ac.signal } ); setQr(data.uri); // ... session handling ... } catch (error) { if ((error as any).name !== 'CanceledError') { console.error('Error fetching QR code data:', error); } } })(); return () => { ac.abort(); esRef.current?.close(); }; }, []);platforms/eVoting/src/app/(app)/create/page.tsx (2)
385-390
: Fix datetime-local min to use local time (not UTC)datetime-local expects a local datetime. Using toISOString() sets UTC, which can render past/future incorrectly and block valid input.
Apply this diff:
- min={new Date().toISOString().slice(0, 16)} + min={safeMinDatetimeLocal}Add this helper inside the component (above return):
// Compute a local datetime string for <input type="datetime-local" /> const safeMinDatetimeLocal = new Date(Date.now() - new Date().getTimezoneOffset() * 60000) .toISOString() .slice(0, 16);
463-467
: Disable submit button and show progress to prevent double submitsBlock repeated submissions and provide user feedback while the request is in flight.
Apply this diff:
- <Button - type="submit" - className="flex-1 bg-(--crimson) hover:bg-(--crimson-50) hover:text-(--crimson) hover:border-(--crimson) border text-white" - > - Create Vote - </Button> + <Button + type="submit" + disabled={isSubmitting} + aria-busy={isSubmitting} + className="flex-1 bg-(--crimson) hover:bg-(--crimson-50) hover:text-(--crimson) hover:border-(--crimson) border text-white disabled:opacity-60 disabled:cursor-not-allowed" + > + {isSubmitting ? "Creating..." : "Create Vote"} + </Button>platforms/eVoting/src/app/(app)/[id]/page.tsx (1)
799-804
: Remove references to non-existent option propertiesThe code references
selectedPoll.options.find((opt) => opt.id === voter.optionId)?.text
but based on the Poll interface, options is a string array, not an array of objects with id and text properties.-Voted for:{" "} -{ - selectedPoll.options.find( - (opt) => - opt.id === - voter.optionId - )?.text -} +Voted for:{" "} +{voter.optionIndex !== undefined && selectedPoll.options[voter.optionIndex] + ? selectedPoll.options[voter.optionIndex] + : "Unknown"}platforms/eVoting/src/app/(app)/page.tsx (1)
77-77
: Fix Tailwind arbitrary value syntax: use square brackets instead of parenthesesTailwind requires bg-[--var] and border-[--var], not bg-(--var)/border-(--var). Current classes won’t compile and styles won’t apply.
Apply these diffs:
- className="w-full bg-(--crimson) text-white hover:bg-(--crimson-50) hover:text-(--crimson) hover:border-(--crimson) border transition-colors" + className="w-full bg-[--crimson] text-white hover:bg-[--crimson-50] hover:text-[--crimson] hover:border-[--crimson] border transition-colors"- <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-(--crimson)" /> + <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-[--crimson]" />- className="w-full bg-(--crimson) hover:bg-(--crimson-50) hover:text-(--crimson) hover:border-(--crimson) border text-white" + className="w-full bg-[--crimson] hover:bg-[--crimson-50] hover:text-[--crimson] hover:border-[--crimson] border text-white"- <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-(--crimson)" /> + <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-[--crimson]" />- className="w-full bg-(--crimson) hover:bg-(--crimson-50) hover:text-(--crimson) hover:border-(--crimson) border text-white" + className="w-full bg-[--crimson] hover:bg-[--crimson-50] hover:text-[--crimson] hover:border-[--crimson] border text-white"Also applies to: 99-99, 169-169, 293-293, 360-360
♻️ Duplicate comments (1)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (1)
284-291
: Replace simulated signature with actual cryptographic implementationSimilar to the sign page, this component uses a simulated signature which is a critical security issue for production.
The signing implementation must use actual cryptographic signatures from the vault:
- // In a real implementation, you would use the vault's signing capabilities - // For now, we'll simulate the signing process - await new Promise((resolve) => setTimeout(resolve, 2000)); // Simulate signing delay - - // Create the signed payload - const signedPayload = { - sessionId: signingSessionId, - signature: "simulated_signature_" + Date.now(), // In real implementation, this would be the actual signature - publicKey: vault?.ename || "unknown_public_key", // Use eName as public key for now - message: messageToSign, - }; + // Sign the message using the vault's private key + const signature = await vault.sign(messageToSign); + const publicKey = await vault.getPublicKey(); + + const signedPayload = { + sessionId: signingSessionId, + signature: signature, + publicKey: publicKey, + message: messageToSign, + };
🧹 Nitpick comments (46)
platforms/blabsy/src/lib/utils/mobile-detection.ts (1)
4-5
: Avoid viewport-width heuristic for device detectionUsing window.innerWidth <= 768 can misclassify resized desktop windows as “mobile.” Prefer feature detection (pointer/touch) and use UA only as a fallback.
Apply this diff to improve detection:
-export function isMobileDevice(): boolean { - if (typeof window === 'undefined') return false; - - return /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) || - (window.innerWidth <= 768); -} +export function isMobileDevice(): boolean { + if (typeof window === 'undefined') return false; + const isCoarsePointer = window.matchMedia?.('(pointer: coarse)').matches ?? false; + const isTouchCapable = 'ontouchstart' in window || navigator.maxTouchPoints > 0; + const uaMobile = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test( + navigator.userAgent + ); + return isCoarsePointer || isTouchCapable || uaMobile; +}Additionally, since this is not reactive, consider calling it within a client-only hook to avoid hydration mismatches (see comment in login-main.tsx).
platforms/eVoting/src/app/globals.css (1)
340-392
: Prefer modern dynamic viewport units (dvh/svh) and minimize global overflow hidingThe iOS hack is okay, but you can get more robust behavior with dvh/svh and reduce reliance on global overflow-x: hidden which can mask layout issues.
Consider augmenting with:
+/* Prefer modern dynamic viewport units where supported */ +@supports (height: 100dvh) { + .min-h-screen { + min-height: 100dvh; + } +}Also consider scoping overflow-x: hidden to affected containers instead of html/body to avoid breaking wide components (tables, code blocks) globally.
infrastructure/control-panel/README.md (1)
112-119
: Align eVault detection docs with actual implementation (namespace filter vs name keywords)The README states detection is by pod name keywords, but the implementation filters namespaces starting with “evault-” and then enumerates services/pods within them. Update the docs to match code to avoid confusion.
Apply this diff:
-### eVault Detection - -The system automatically detects eVault pods by filtering for pods with names containing: - -- `evault` -- `vault` -- `web3` - -You can modify the filter in `src/routes/api/evaults/+server.ts` to adjust detection criteria. +### eVault Detection + +By default, the system discovers eVaults by scanning Kubernetes namespaces that start with `evault-` and then enumerating their services and pods. You can modify this logic in `src/routes/api/evaults/+server.ts` (e.g., change the namespace filter, add label selectors, or adjust pod matching).infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/metrics/+server.ts (1)
11-12
: Gate verbose console logging behind a DEBUG flag.The route logs internal command outputs and lengths on every request. This is noisy and can leak operational details.
Apply this diff:
- console.log('Metrics API called with namespace:', namespace, 'pod:', pod); + const DEBUG = process.env.DEBUG?.includes('evaults'); + DEBUG && console.log('Metrics API called with namespace:', namespace, 'pod:', pod); @@ - console.log('Running kubectl top pod...'); + DEBUG && console.log('Running kubectl top pod...'); @@ - console.log('kubectl top pod output:', topOutput); + DEBUG && console.log('kubectl top pod output:', topOutput); @@ - console.log('Running kubectl describe pod...'); + DEBUG && console.log('Running kubectl describe pod...'); @@ - console.log('kubectl describe pod output length:', describeOutput?.length || 0); + DEBUG && console.log('kubectl describe pod output length:', describeOutput?.length || 0); @@ - console.log('Running kubectl logs...'); + DEBUG && console.log('Running kubectl logs...'); @@ - console.log('kubectl logs output length:', logsOutput?.length || 0); + DEBUG && console.log('kubectl logs output length:', logsOutput?.length || 0);Also applies to: 15-16, 24-25, 27-30, 32-35, 42-45, 58-59
infrastructure/control-panel/src/routes/api/evaults/+server.ts (2)
63-68
: Reduce noisy logging of cluster objects.Dumping entire services/pods JSON on every request is heavy and can reveal cluster details. Gate behind DEBUG or remove.
Apply this diff:
- console.log(`=== SERVICES FOR ${namespace} ===`); - console.log(JSON.stringify(services, null, 2)); - console.log(`=== PODS FOR ${namespace} ===`); - console.log(JSON.stringify(pods, null, 2)); - console.log(`=== END DATA ===`); + if (process.env.DEBUG?.includes('evaults')) { + console.log(`=== SERVICES FOR ${namespace} ===`); + console.log(JSON.stringify(services, null, 2)); + console.log(`=== PODS FOR ${namespace} ===`); + console.log(JSON.stringify(pods, null, 2)); + console.log(`=== END DATA ===`); + }
1-4
: Align RequestHandler import for consistency.Elsewhere in routes, RequestHandler is imported from './$types'. Consider aligning import style across API routes.
No functional change; improves consistency for SvelteKit typed routes.
infrastructure/control-panel/src/routes/monitoring/[namespace]/[service]/+page.ts (1)
1-7
: Type the load function and drop noisy console log.Add SvelteKit types for clarity and remove the debug log (or gate behind dev), since params are already available.
Apply this diff:
-export const load = ({ params }) => { - console.log('+page.ts load called with params:', params); - return { - namespace: params.namespace, - service: params.service - }; -}; +import type { PageLoad } from './$types'; + +export const load: PageLoad = ({ params }) => { + return { + namespace: params.namespace, + service: params.service + }; +};platforms/eVoting/src/lib/utils/mobile-detection.ts (1)
1-10
: Unify this utility across platforms to avoid driftThis is duplicated in blabsy and group-charter-manager. Consider extracting to a shared package (e.g., packages/web/utils/mobile) and consuming from all apps to keep behavior consistent.
platforms/eVoting/src/app/(app)/layout.tsx (1)
33-35
: Confirm desired UX: closing the disclaimer via outside click logs the user outonInteractOutside currently logs the user out, which can be surprising and destructive. If the intent is to prevent dismissing the dialog, call preventDefault() instead and keep users signed in.
Apply this diff if the goal is to block outside interaction rather than logout:
- <DialogContent - className="max-w-lg mx-auto backdrop-blur-md p-6 rounded-lg" - onInteractOutside={() => logout()} - > + <DialogContent + className="max-w-lg mx-auto backdrop-blur-md p-6 rounded-lg" + onInteractOutside={(e) => e.preventDefault()} + >Otherwise, consider showing a confirmation before logging a user out.
infrastructure/control-panel/src/routes/evaults/[namespace]/[pod]/+page.svelte (2)
14-16
: React to param changes (optional)If users navigate between pods/namespaces without a full reload, consider reacting to $page param changes to refetch automatically.
Apply this diff:
- const namespace = $page.params.namespace; - const podName = $page.params.pod; + $: namespace = $page.params.namespace; + $: podName = $page.params.pod; onMount(() => { fetchEVaultDetails(); }); + +$: if (namespace && podName) { + // Refetch when route params change + fetchEVaultDetails(); +}Also applies to: 46-49
121-124
: Consider tailing or limiting logs and virtualizing for long listsRendering an unbounded number of log lines can impact performance. Consider tail=N on the API and/or log virtualization.
platforms/eVoting/src/lib/auth-context.tsx (2)
39-57
: Avoid state updates on unmounted component during initInitializeAuth may resolve after unmount (route changes, fast logout), causing setState on unmounted component. Add an abort/flag to prevent that.
Example refactor:
useEffect(() => { - const initializeAuth = async () => { + let isMounted = true; + const initializeAuth = async () => { const token = getAuthToken(); const userId = getAuthId(); if (token && userId) { try { - const response = await apiClient.get("/api/users/me", { + const response = await apiClient.get("/api/users/me", { headers: { Authorization: `Bearer ${token}` }, }); - setUser(response.data); + if (isMounted) setUser(response.data); } catch (error) { console.error("Failed to get current user:", error); clearAuth(); } } - setIsLoading(false); + if (isMounted) setIsLoading(false); }; initializeAuth(); - }, []); + return () => { + isMounted = false; + }; + }, []);
64-66
: LocalStorage tokens increase XSS risk; prefer httpOnly cookiesStoring tokens in localStorage makes them accessible to XSS. If feasible, switch to server-set, httpOnly, sameSite cookies and CSRF defenses.
platforms/eVoting/src/app/(app)/create/page.tsx (2)
413-421
: Rely on schema validation instead of HTML required attr on controlled inputsThese inputs aren’t registered with RHF; the required attribute won’t integrate with your zod validation and may be misleading. Either register them or rely solely on schema + setValue updates.
Apply this diff to remove the redundant attribute:
- required
Alternatively, register options as fields (more invasive).
109-110
: Consider replacing push with replace after creationIf you don’t want the user to navigate back to the create form after success, use replace instead of push.
Apply this diff:
- router.push("/"); + router.replace("/");platforms/eVoting/src/components/auth/login-screen.tsx (3)
5-7
: Use authUtils helpers instead of raw localStorage writesKeep token persistence consistent and centralized.
Apply these diffs:
-import { useAuth } from "@/lib/auth-context"; +import { useAuth } from "@/lib/auth-context"; +import { setAuthToken, setAuthId } from "@/lib/authUtils";- // Store the token and user ID directly - localStorage.setItem("evoting_token", data.token); - localStorage.setItem("evoting_user_id", data.user.id); + // Store the token and user ID using helpers + setAuthToken(data.token); + setAuthId(data.user.id);Also applies to: 41-45
59-59
: Remove unused dependency to avoid unintended effect re-runslogin is not used inside the effect and can re-trigger the SSE setup unnecessarily.
Apply this diff:
- }, [sessionId, login]); + }, [sessionId]);
20-23
: Surface offer-fetch failures to the UI or auto-retryCurrently failures only log to console, leaving a blank/loading state. Consider exposing an error state and presenting a retry action.
infrastructure/control-panel/src/routes/+page.svelte (2)
15-16
: Avoid any[] for table data; type the mapped rowsTyping mappedData improves maintainability and prevents shape drift.
Example:
type TableCell = | { type: "text"; value: string; className?: string } | { type: "link"; value: string; link: string; external?: boolean }; type EVaultRow = { eName: TableCell; Uptime: TableCell; IP: TableCell; URI: TableCell; }; let mappedData = $state<EVaultRow[]>([]);
4-8
: Remove unused importsRefreshCw is imported but unused.
Apply this diff:
- import { RefreshCw } from 'lucide-svelte';
platforms/eVoting/src/app/(auth)/login/page.tsx (2)
69-69
: Remove unused dependency to avoid unnecessary SSE re-subscriptionlogin isn’t used by this effect.
Apply this diff:
- }, [sessionId, login]); + }, [sessionId]);
62-64
: Improve SSE error handling UXCurrently, the SSE error silently closes the stream. Consider surfacing an error and offering a retry (re-fetch offer, re-open SSE).
infrastructure/control-panel/src/routes/monitoring/[namespace]/[service]/+page.svelte (1)
119-120
: Optimize auto-refresh implementationThe auto-refresh logic could be optimized to prevent unnecessary API calls when the component is not visible or when the user is not actively viewing the logs.
Consider using the Page Visibility API to pause refresh when the page is not visible:
function startAutoRefresh() { - refreshInterval = setInterval(fetchLogs, 5000); + refreshInterval = setInterval(() => { + if (!document.hidden) { + fetchLogs(); + } + }, 5000); }platforms/eVoting/src/app/(app)/page.tsx (2)
23-36
: Add error state and UX for failed poll fetchesCurrently errors are only logged to console; users see empty content with no explanation.
Apply a lightweight error state:
- const [isLoading, setIsLoading] = useState(true); + const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState<string | null>(null); @@ - } catch (error) { - console.error("Failed to fetch polls:", error); + } catch (error) { + console.error("Failed to fetch polls:", error); + setError("Failed to load polls. Please try again later."); @@ - {isLoading ? ( + {isLoading ? ( <div className="flex justify-center py-8"> <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-[--crimson]" /> </div> - ) : activePolls.length === 0 ? ( + ) : error ? ( + <div className="text-center py-8 text-red-600"> + {error} + </div> + ) : activePolls.length === 0 ? ( <div className="text-center py-8 text-gray-500"> No active votes available for voting. </div>Repeat similar pattern for the “Your Votes” section if desired.
Also applies to: 97-105, 290-298
106-186
: Reduce duplication by extracting a reusable PollCard componentThe three grids render nearly identical cards with minor variations (CTA label, “View Vote” vs “View Results”). Factor a component to simplify maintenance and keep badges/labels consistent.
If helpful, I can draft a PollCard component with props: { poll, ctaLabel, ctaHref, showDeadline } and replace these map blocks.
Also applies to: 203-274, 299-376
platforms/eVoting/src/lib/pollApi.ts (2)
26-32
: Strengthen Vote.data typing with a discriminated unionany[] obscures mode-specific payloads and weakens type-safety across the app.
- data: { - mode: "normal" | "point" | "rank"; - data: string[] | any[]; - }; + data: + | { mode: "normal"; data: string } // option id + | { mode: "rank"; data: string[] } // ranked option ids + | { mode: "point"; data: Record<string, number> } // optionId -> points + ;This mirrors UI behavior in [id]/page.tsx and prevents invalid payloads at compile-time.
61-63
: Use typed apiClient generics for safer response inferenceAdding Axios generics improves intellisense and prevents accidental misuse of response shapes.
Examples:
- const response = await apiClient.get("/api/polls"); + const response = await apiClient.get<Poll[]>("/api/polls"); @@ - const response = await apiClient.get(`/api/polls/${id}`); + const response = await apiClient.get<Poll>(`/api/polls/${id}`); @@ - const response = await apiClient.get("/api/polls/my"); + const response = await apiClient.get<Poll[]>("/api/polls/my"); @@ - const response = await apiClient.post("/api/polls", pollData); + const response = await apiClient.post<Poll>("/api/polls", pollData); @@ - const response = await apiClient.put(`/api/polls/${id}`, pollData); + const response = await apiClient.put<Poll>(`/api/polls/${id}`, pollData); @@ - const response = await apiClient.post("/api/votes", { + const response = await apiClient.post<Vote>("/api/votes", { @@ - const response = await apiClient.get(`/api/polls/${pollId}/votes`); + const response = await apiClient.get<Vote[]>(`/api/polls/${pollId}/votes`); @@ - const response = await apiClient.get(`/api/polls/${pollId}/vote`); + const response = await apiClient.get<{ hasVoted: boolean; vote: Vote | null }>(`/api/polls/${pollId}/vote`); @@ - const response = await apiClient.get(`/api/polls/${pollId}/results`); + const response = await apiClient.get<PollResults>(`/api/polls/${pollId}/results`); @@ - const response = await apiClient.post("/api/signing/sessions", { + const response = await apiClient.post<SigningSession>("/api/signing/sessions", {Also applies to: 67-69, 73-75, 79-81, 85-87, 95-101, 105-107, 111-113, 117-119, 123-129
infrastructure/eid-wallet/src-tauri/capabilities/mobile.json (1)
11-12
: Add Android deep-link scheme mapping if not already presentAdding the deep-link permission is necessary, but Android also needs an intent filter declaring the scheme. If the capability does not imply “w3ds” automatically, add/verify a manifest entry (or capability configuration) that registers scheme “w3ds” so links resolve on Android.
Use the script in the Cargo.toml comment to confirm whether the Android intent filter is present.
infrastructure/eid-wallet/src/routes/(auth)/login/+page.svelte (3)
66-92
: Duplicate deep-link processing logic – extract helper to reduce driftThe pendingDeepLink handling is duplicated in both PIN and biometric flows. Extract a small helper (e.g., processPendingDeepLink()) to parse, persist deepLinkData, clear pendingDeepLink, and navigate. This lowers maintenance cost and avoids behavioral drift.
Example (apply inside onMount scope):
<script> // ... async function processPendingDeepLink() { const pendingDeepLink = sessionStorage.getItem("pendingDeepLink"); if (!pendingDeepLink) return false; try { const deepLinkData = JSON.parse(pendingDeepLink); console.log("Processing pending deep link:", deepLinkData); sessionStorage.setItem("deepLinkData", pendingDeepLink); sessionStorage.removeItem("pendingDeepLink"); await goto("/scan-qr"); return true; } catch (error) { console.error("Error processing pending deep link:", error); sessionStorage.removeItem("pendingDeepLink"); return false; } } </script>Then call
if (await processPendingDeepLink()) return;
in both flows.Also applies to: 115-141
109-147
: Biometric flow prompt text is misleadingThe prompt says “You must authenticate with PIN first” but biometric runs automatically on mount. Either defer biometric until after PIN entry or adjust the messaging to reflect the actual flow.
43-48
: Minor: pendingDeepLink banner state may linger across sessionshasPendingDeepLink is set from sessionStorage at mount. Consider clearing it after a successful flow or when the user cancels to avoid stale UX if the page is revisited.
infrastructure/eid-wallet/src/routes/(app)/sign/+page.svelte (1)
98-101
: Make redirect delay configurable and provide user feedbackThe hard-coded 3-second delay before redirect might be too long or too short depending on the use case.
// Redirect back to the main app after a short delay + const REDIRECT_DELAY_MS = 3000; // Make this configurable + // Show countdown or progress indicator to user setTimeout(() => { goto("/main"); - }, 3000); + }, REDIRECT_DELAY_MS);infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (2)
349-368
: Avoid duplicate authentication checksThe component performs authentication checks both in onMount and in the parent layout, which could lead to race conditions or duplicate redirects.
Since the parent layout already handles authentication, consider removing the duplicate check here:
onMount(async () => { console.log("Scan QR page mounted, checking authentication..."); - // Security check: Ensure user is authenticated before allowing access - try { - const vault = await globalState.vaultController.vault; - if (!vault) { - console.log("User not authenticated, redirecting to login"); - await goto("/login"); - return; - } - console.log( - "User authenticated, proceeding with scan functionality", - ); - } catch (error) { - console.log("Authentication check failed, redirecting to login"); - await goto("/login"); - return; - } + // Authentication is already handled by the parent layout + // Proceed with scan functionality console.log("Scan QR page mounted, checking for deep link data...");
473-492
: Properly type event listeners to avoid type assertionsThe event listeners use type assertions which bypass TypeScript's type checking.
+ interface DeepLinkAuthEvent extends CustomEvent { + detail: { + session: string; + platform: string; + redirect: string; + }; + } + + interface DeepLinkSignEvent extends CustomEvent { + detail: { + session: string; + data: string; + redirect_uri: string; + }; + } - const handleAuthEvent = (event: CustomEvent) => { + const handleAuthEvent = (event: DeepLinkAuthEvent) => { console.log("Received deepLinkAuth event:", event.detail); handleDeepLinkData({ type: "auth", ...event.detail, }); }; - const handleSignEvent = (event: CustomEvent) => { + const handleSignEvent = (event: DeepLinkSignEvent) => { console.log("Received deepLinkSign event:", event.detail); handleDeepLinkData({ type: "sign", ...event.detail, }); }; - window.addEventListener( - "deepLinkAuth", - handleAuthEvent as EventListener, - ); - window.addEventListener( - "deepLinkSign", - handleSignEvent as EventListener, - ); + window.addEventListener("deepLinkAuth", handleAuthEvent); + window.addEventListener("deepLinkSign", handleSignEvent); // Cleanup event listeners onDestroy(() => { - window.removeEventListener( - "deepLinkAuth", - handleAuthEvent as EventListener, - ); - window.removeEventListener( - "deepLinkSign", - handleSignEvent as EventListener, - ); + window.removeEventListener("deepLinkAuth", handleAuthEvent); + window.removeEventListener("deepLinkSign", handleSignEvent); });platforms/evoting-api/src/database/entities/MetaEnvelopeMap.ts (1)
14-14
: Consider using implicit column types.The explicit
varchar(255)
specification is redundant since TypeORM defaults tovarchar(255)
for string columns. The Cerberus platform's identical entity uses implicit column types without length specification.Apply this diff to simplify the column definitions:
- @Column("varchar", { length: 255 }) + @Column() localId!: string; - @Column("varchar", { length: 255 }) + @Column() globalId!: string; - @Column("varchar", { length: 255 }) + @Column() entityType!: string; - @Column("varchar", { length: 255 }) + @Column() platform!: string;Also applies to: 17-17, 20-20, 23-23
platforms/evoting-api/src/database/entities/Vote.ts (1)
55-57
: Document the distinction betweenuserId
andvoterId
While the comment on line 55 explains that
voterId
can be various identifiers, the relationship betweenuserId
andvoterId
could be clearer. Consider adding documentation to explain when each field is used.Add a more comprehensive comment to clarify the field usage:
+ // The authenticated user who cast this vote (foreign key to users table) @ManyToOne(() => User, (user) => user.votes) @JoinColumn({ name: "userId" }) user!: User; @Column("uuid") userId!: string; - // This can be user ID, session ID, or anonymous identifier + // Flexible identifier for the voter: can be userId for authenticated votes, + // session ID for anonymous votes, or other identifiers for external voting systems @Column("varchar", { length: 255 }) voterId!: string;platforms/evoting-api/src/services/PollService.ts (1)
10-13
: Consider lazy initialization of repositoriesThe repositories are initialized in the constructor which could cause issues if
AppDataSource
is not yet initialized when the service is instantiated.Consider using lazy initialization:
export class PollService { - private pollRepository: Repository<Poll>; - private userRepository: Repository<User>; + private pollRepository?: Repository<Poll>; + private userRepository?: Repository<User>; - constructor() { - this.pollRepository = AppDataSource.getRepository(Poll); - this.userRepository = AppDataSource.getRepository(User); - } + private getPollRepository(): Repository<Poll> { + if (!this.pollRepository) { + this.pollRepository = AppDataSource.getRepository(Poll); + } + return this.pollRepository; + } + + private getUserRepository(): Repository<User> { + if (!this.userRepository) { + this.userRepository = AppDataSource.getRepository(User); + } + return this.userRepository; + }Then update all method calls to use
this.getPollRepository()
andthis.getUserRepository()
instead of direct access.platforms/evoting-api/src/services/UserService.ts (2)
5-7
: Repository initialization without AppDataSource readiness checkSimilar to PollService, the repository is initialized immediately which could cause issues if the data source isn't ready.
Consider implementing the same lazy initialization pattern suggested for PollService, or ensure AppDataSource is initialized before service instantiation.
55-57
: Consider soft delete for user dataHard deleting users could break referential integrity if there are related votes or polls. Consider implementing soft delete.
Since the User entity already has an
isArchived
field (as shown in the relevant code snippets), consider using it for soft deletion:-async deleteUser(id: string): Promise<void> { - await this.userRepository.delete(id); +async deleteUser(id: string): Promise<void> { + await this.userRepository.update(id, { isArchived: true }); }platforms/evoting-api/src/controllers/PollController.ts (1)
11-19
: Consider using arrow functions consistentlyAll methods are defined as arrow functions except for the constructor. While this works, consider whether you need the arrow function binding here since the controller instance is already bound when instantiating.
If these methods are not passed as callbacks directly, you could use regular methods for better performance:
- getAllPolls = async (req: Request, res: Response) => { + async getAllPolls(req: Request, res: Response) {platforms/evoting-api/src/controllers/SigningController.ts (1)
17-22
: ensureService method could be more descriptiveThe method name
ensureService
doesn't clearly indicate that it throws an error if the service is not initialized.Consider renaming to be more explicit about the throwing behavior:
-private ensureService(): SigningService { +private getServiceOrThrow(): SigningService { if (!this.signingService) { throw new Error("SigningService not initialized"); } return this.signingService; }platforms/evoting-api/src/services/VoteService.ts (2)
62-73
: Hardcoded rank points should be configurableThe rank voting mode uses hardcoded point values (50, 35, 15) which reduces flexibility.
Consider making these values configurable:
// At the top of the class or in a config file private static readonly RANK_POINTS: Record<number, number> = { }; // Then in the method: const points = VoteService.RANK_POINTS[rankNum] || 0;
146-172
: Inefficient option index lookups in getPollResultsThe method uses
poll.options.indexOf()
multiple times within loops, which has O(n) complexity for each lookup.Create an options index map for O(1) lookups:
+// Create option index map for efficient lookups +const optionIndexMap = new Map<string, number>(); +poll.options.forEach((option, index) => { + optionIndexMap.set(option, index); +}); votes.forEach(vote => { if (vote.data.mode === "normal" && Array.isArray(vote.data.data)) { // ... existing logic } else if (vote.data.mode === "rank" && Array.isArray(vote.data.data)) { vote.data.data.forEach((rankData: any) => { - const optionIndex = poll.options.indexOf(rankData.option); + const optionIndex = optionIndexMap.get(rankData.option); - if (optionIndex >= 0) { + if (optionIndex !== undefined) { voteCounts[optionIndex] += rankData.points || 0; } }); } else if (vote.data.mode === "point" && Array.isArray(vote.data.data)) { vote.data.data.forEach((pointData: any) => { - const optionIndex = poll.options.indexOf(pointData.option); + const optionIndex = optionIndexMap.get(pointData.option); - if (optionIndex >= 0) { + if (optionIndex !== undefined) { voteCounts[optionIndex] += pointData.points || 0; } }); } });platforms/evoting-api/src/index.ts (1)
133-133
: Clarify auth middleware application scopeThe comment says "Apply auth middleware to all routes below" but the middleware only populates
req.user
without enforcing authentication. The actual enforcement happens viaauthGuard
.Update the comment to be more accurate:
-// Protected routes (auth required) -app.use(authMiddleware); // Apply auth middleware to all routes below +// Apply auth middleware to parse tokens and populate req.user +app.use(authMiddleware); + +// Routes below may use authGuard to enforce authenticationplatforms/evoting-api/src/services/SigningService.ts (1)
9-9
: Consider using a more specific type forvoteData
The
voteData
field is typed asany
, which reduces type safety. Based on the VoteService implementation, this should includeoptionId?: number
,points?: { [key: number]: number }
, andranks?: { [key: number]: number }
.- voteData: any; + voteData: { + optionId?: number; + points?: { [key: number]: number }; + ranks?: { [key: number]: number }; + };platforms/evoting-api/src/web3adapter/watchers/subscriber.ts (1)
45-57
: Inefficient relation loading with potential N+1 query problemThe
enrichEntity
method loadscreator
anduser
relations separately even when the IDs are already available. This creates unnecessary database queries.Since you already have the entity with the relation IDs, consider:
- If the relations are already loaded (check if they're objects vs IDs)
- Use a single query with multiple relations when needed
async enrichEntity(entity: any, tableName: string, tableTarget: any) { try { const enrichedEntity = { ...entity }; + const relationsToLoad = []; - if (entity.creator) { - const creator = await AppDataSource.getRepository( - "User" - ).findOne({ where: { id: entity.creator.id } }); - enrichedEntity.creator = creator; - } - - if (entity.user) { - const user = await AppDataSource.getRepository( - "User" - ).findOne({ where: { id: entity.user.id } }); - enrichedEntity.user = user; - } + // Check if relations need loading (are IDs instead of objects) + if (entity.creator && typeof entity.creator !== 'object') { + relationsToLoad.push('creator'); + } + if (entity.user && typeof entity.user !== 'object') { + relationsToLoad.push('user'); + } + + // Load all needed relations in one query if entity has an ID + if (relationsToLoad.length > 0 && entity.id) { + const fullEntity = await AppDataSource.getRepository(tableTarget) + .findOne({ + where: { id: entity.id }, + relations: relationsToLoad + }); + if (fullEntity) { + Object.assign(enrichedEntity, fullEntity); + } + } return this.entityToPlain(enrichedEntity); } catch (error) {
"flowbite": "^3.1.2", | ||
"flowbite-svelte": "^1.10.7", | ||
"flowbite-svelte-icons": "^2.2.1", | ||
"lucide-svelte": "^0.539.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify lucide-svelte compatibility with Svelte 5 and lock if needed
Adding lucide-svelte looks fine, but please confirm peer compatibility with your Svelte version and consider pinning if you see downstream breakage (icons libs sometimes ship breaking changes).
You can quickly check the latest version and peer deps:
🏁 Script executed:
#!/bin/bash
# Check latest lucide-svelte and peer requirements
npm view lucide-svelte version peerDependencies engines | jq .
Length of output: 121
I wasn’t able to parse the peer info from the previous command—let’s try explicitly requesting JSON for both peerDependencies and engines:
🏁 Script executed:
#!/bin/bash
# Get peerDependencies and engines for the latest lucide-svelte release
npm view lucide-svelte@latest peerDependencies --json
npm view lucide-svelte@latest engines --json
Length of output: 147
Confirm lucide-svelte peer dependency supports your Svelte version
The current lucide-svelte@0.539.0 lists
"peerDependencies": {
"svelte": "^3 || ^4 || ^5.0.0-next.42"
}
– it does not include stable Svelte 5.x releases. If this control-panel is upgraded to Svelte 5.0.0 (or higher), you’ll encounter peer-dependency warnings or runtime breakage.
Please verify your Svelte version in infrastructure/control-panel/package.json and choose one of the following:
- If you’re on Svelte 5.x stable, wait for (or upgrade to) the next lucide-svelte release that adds
"^5"
support. - If you need icons now, pin your project to Svelte 4 or the 5.0.0-next.42 prerelease.
- Lock or pin lucide-svelte to 0.539.0 and test thoroughly before upgrading Svelte.
File: infrastructure/control-panel/package.json
Line: 53
"dependencies": {
- "lucide-svelte": "^0.539.0",
+ "lucide-svelte": "^0.539.0", // peer requires Svelte "^3 || ^4 || ^5.0.0-next.42"
}
static async getEVaultDetails(namespace: string, podName: string): Promise<any> { | ||
try { | ||
const response = await fetch(`/api/evaults/${encodeURIComponent(namespace)}/${encodeURIComponent(podName)}/details`); | ||
if (!response.ok) { | ||
throw new Error('Failed to fetch eVault details'); | ||
} | ||
return await response.json(); | ||
} catch (error) { | ||
console.error('Error fetching eVault details:', error); | ||
return null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding return type definition for better type safety
The methods getEVaultDetails
and getEVaultMetrics
return Promise<any>
, which loses type safety benefits. Consider defining proper interfaces for the response data structures to ensure type safety throughout the application.
+interface EVaultDetails {
+ // Define the structure based on your API response
+ namespace: string;
+ podName: string;
+ // Add other relevant fields
+}
+
+interface EVaultMetrics {
+ resources?: {
+ cpu?: string;
+ memory?: string;
+ note?: string;
+ };
+ status?: {
+ podStatus?: string;
+ podAge?: string;
+ events?: string[];
+ conditions?: string[];
+ };
+ logs?: {
+ totalLines?: number;
+ errorCount?: number;
+ warningCount?: number;
+ lastUpdate?: string;
+ };
+}
-static async getEVaultDetails(namespace: string, podName: string): Promise<any> {
+static async getEVaultDetails(namespace: string, podName: string): Promise<EVaultDetails | null> {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
static async getEVaultDetails(namespace: string, podName: string): Promise<any> { | |
try { | |
const response = await fetch(`/api/evaults/${encodeURIComponent(namespace)}/${encodeURIComponent(podName)}/details`); | |
if (!response.ok) { | |
throw new Error('Failed to fetch eVault details'); | |
} | |
return await response.json(); | |
} catch (error) { | |
console.error('Error fetching eVault details:', error); | |
return null; | |
} | |
} | |
// Add these at the top of the file (above your service class) | |
interface EVaultDetails { | |
// Define the structure based on your API response | |
namespace: string; | |
podName: string; | |
// Add other relevant fields | |
} | |
interface EVaultMetrics { | |
resources?: { | |
cpu?: string; | |
memory?: string; | |
note?: string; | |
}; | |
status?: { | |
podStatus?: string; | |
podAge?: string; | |
events?: string[]; | |
conditions?: string[]; | |
}; | |
logs?: { | |
totalLines?: number; | |
errorCount?: number; | |
warningCount?: number; | |
lastUpdate?: string; | |
}; | |
} | |
// Then update the method signature: | |
static async getEVaultDetails(namespace: string, podName: string): Promise<EVaultDetails | null> { | |
try { | |
const response = await fetch( | |
`/api/evaults/${encodeURIComponent(namespace)}/${encodeURIComponent(podName)}/details` | |
); | |
if (!response.ok) { | |
throw new Error('Failed to fetch eVault details'); | |
} | |
return await response.json(); | |
} catch (error) { | |
console.error('Error fetching eVault details:', error); | |
return null; | |
} | |
} |
🤖 Prompt for AI Agents
in infrastructure/control-panel/src/lib/services/evaultService.ts around lines
32 to 43, the method currently returns Promise<any> which loses type safety;
define and export interfaces (e.g., EVaultDetails and EVaultMetrics) that model
the expected JSON shape, update the method signatures to return
Promise<EVaultDetails | null> (or appropriate nullable/union types),
cast/validate the fetched JSON to that interface before returning (or use a
lightweight runtime assertion/mapper), and update any callers to use the new
types; keep error handling the same but ensure the declared return type matches
what you actually return on error (null or throw).
static async getEVaultMetrics(namespace: string, podName: string): Promise<any> { | ||
try { | ||
const response = await fetch(`/api/evaults/${encodeURIComponent(namespace)}/${encodeURIComponent(podName)}/metrics`); | ||
if (!response.ok) { | ||
throw new Error('Failed to fetch metrics'); | ||
} | ||
return await response.json(); | ||
} catch (error) { | ||
console.error('Error fetching eVault metrics:', error); | ||
return null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Apply consistent return type definition
Similar to the getEVaultDetails
method, define a proper interface for the metrics response.
-static async getEVaultMetrics(namespace: string, podName: string): Promise<any> {
+static async getEVaultMetrics(namespace: string, podName: string): Promise<EVaultMetrics | null> {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
static async getEVaultMetrics(namespace: string, podName: string): Promise<any> { | |
try { | |
const response = await fetch(`/api/evaults/${encodeURIComponent(namespace)}/${encodeURIComponent(podName)}/metrics`); | |
if (!response.ok) { | |
throw new Error('Failed to fetch metrics'); | |
} | |
return await response.json(); | |
} catch (error) { | |
console.error('Error fetching eVault metrics:', error); | |
return null; | |
} | |
} | |
static async getEVaultMetrics(namespace: string, podName: string): Promise<EVaultMetrics | null> { | |
try { | |
const response = await fetch(`/api/evaults/${encodeURIComponent(namespace)}/${encodeURIComponent(podName)}/metrics`); | |
if (!response.ok) { | |
throw new Error('Failed to fetch metrics'); | |
} | |
return await response.json(); | |
} catch (error) { | |
console.error('Error fetching eVault metrics:', error); | |
return null; | |
} | |
} |
🤖 Prompt for AI Agents
in infrastructure/control-panel/src/lib/services/evaultService.ts around lines
45 to 56, the getEVaultMetrics method currently returns Promise<any>; declare
and use a proper interface (e.g., EVaultMetrics) matching the expected metrics
shape (or reuse the existing EVaultDetails-style interface), change the method
signature to Promise<EVaultMetrics | null>, cast/validate the parsed JSON to
EVaultMetrics before returning (or use a runtime check), and update any callers
to handle the typed response/null. Ensure the new interface is exported/placed
alongside other types for consistency.
import { exec } from 'child_process'; | ||
import { promisify } from 'util'; | ||
import { json } from '@sveltejs/kit'; | ||
import type { RequestHandler } from './$types'; | ||
|
||
const execAsync = promisify(exec); | ||
|
||
export const GET: RequestHandler = async ({ params }) => { | ||
const { namespace, pod } = params; | ||
|
||
try { | ||
// Get detailed pod information | ||
const { stdout: podInfo } = await execAsync(`kubectl describe pod -n ${namespace} ${pod}`); | ||
|
||
// Get pod YAML | ||
const { stdout: podYaml } = await execAsync(`kubectl get pod -n ${namespace} ${pod} -o yaml`); | ||
|
||
// Get pod metrics if available | ||
let metrics = null; | ||
try { | ||
const { stdout: metricsOutput } = await execAsync(`kubectl top pod -n ${namespace} ${pod}`); | ||
metrics = metricsOutput.trim(); | ||
} catch (metricsError) { | ||
// Metrics might not be available | ||
console.log('Metrics not available:', metricsError); | ||
} | ||
|
||
return json({ | ||
podInfo: podInfo.trim(), | ||
podYaml: podYaml.trim(), | ||
metrics | ||
}); | ||
} catch (error) { | ||
console.error('Error fetching pod details:', error); | ||
return json({ error: 'Failed to fetch pod details' }, { status: 500 }); | ||
} | ||
}; No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same command injection vector; switch to execFile and validate params across all kubectl calls
All kubectl invocations interpolate route params into shell strings. Validate inputs and use execFile with arrays to eliminate shell parsing.
Apply this diff:
-import { exec } from 'child_process';
+import { execFile } from 'child_process';
import { promisify } from 'util';
import { json } from '@sveltejs/kit';
import type { RequestHandler } from './$types';
-const execAsync = promisify(exec);
+const execFileAsync = promisify(execFile);
export const GET: RequestHandler = async ({ params }) => {
const { namespace, pod } = params;
+ // Validate k8s identifiers
+ const isValidK8sName = (v: string) => /^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$/.test(v);
+ if (!isValidK8sName(namespace) || !isValidK8sName(pod)) {
+ return json({ error: 'Invalid namespace or pod' }, { status: 400 });
+ }
+
try {
// Get detailed pod information
- const { stdout: podInfo } = await execAsync(`kubectl describe pod -n ${namespace} ${pod}`);
+ const { stdout: podInfo } = await execFileAsync('kubectl', ['describe', 'pod', '-n', namespace, pod]);
// Get pod YAML
- const { stdout: podYaml } = await execAsync(`kubectl get pod -n ${namespace} ${pod} -o yaml`);
+ const { stdout: podYaml } = await execFileAsync('kubectl', ['get', 'pod', '-n', namespace, pod, '-o', 'yaml']);
// Get pod metrics if available
let metrics = null;
try {
- const { stdout: metricsOutput } = await execAsync(`kubectl top pod -n ${namespace} ${pod}`);
+ const { stdout: metricsOutput } = await execFileAsync('kubectl', ['top', 'pod', '-n', namespace, pod]);
metrics = metricsOutput.trim();
} catch (metricsError) {
// Metrics might not be available
console.log('Metrics not available:', metricsError);
}
return json({
podInfo: podInfo.trim(),
podYaml: podYaml.trim(),
metrics
});
} catch (error) {
console.error('Error fetching pod details:', error);
return json({ error: 'Failed to fetch pod details' }, { status: 500 });
}
};
Follow up: Please apply the same hardening to the metrics endpoint (…/metrics/+server.ts), which also shells out with interpolated params.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { exec } from 'child_process'; | |
import { promisify } from 'util'; | |
import { json } from '@sveltejs/kit'; | |
import type { RequestHandler } from './$types'; | |
const execAsync = promisify(exec); | |
export const GET: RequestHandler = async ({ params }) => { | |
const { namespace, pod } = params; | |
try { | |
// Get detailed pod information | |
const { stdout: podInfo } = await execAsync(`kubectl describe pod -n ${namespace} ${pod}`); | |
// Get pod YAML | |
const { stdout: podYaml } = await execAsync(`kubectl get pod -n ${namespace} ${pod} -o yaml`); | |
// Get pod metrics if available | |
let metrics = null; | |
try { | |
const { stdout: metricsOutput } = await execAsync(`kubectl top pod -n ${namespace} ${pod}`); | |
metrics = metricsOutput.trim(); | |
} catch (metricsError) { | |
// Metrics might not be available | |
console.log('Metrics not available:', metricsError); | |
} | |
return json({ | |
podInfo: podInfo.trim(), | |
podYaml: podYaml.trim(), | |
metrics | |
}); | |
} catch (error) { | |
console.error('Error fetching pod details:', error); | |
return json({ error: 'Failed to fetch pod details' }, { status: 500 }); | |
} | |
}; | |
import { execFile } from 'child_process'; | |
import { promisify } from 'util'; | |
import { json } from '@sveltejs/kit'; | |
import type { RequestHandler } from './$types'; | |
const execFileAsync = promisify(execFile); | |
export const GET: RequestHandler = async ({ params }) => { | |
const { namespace, pod } = params; | |
// Validate k8s identifiers | |
const isValidK8sName = (v: string) => /^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$/.test(v); | |
if (!isValidK8sName(namespace) || !isValidK8sName(pod)) { | |
return json({ error: 'Invalid namespace or pod' }, { status: 400 }); | |
} | |
try { | |
// Get detailed pod information | |
const { stdout: podInfo } = await execFileAsync( | |
'kubectl', | |
['describe', 'pod', '-n', namespace, pod] | |
); | |
// Get pod YAML | |
const { stdout: podYaml } = await execFileAsync( | |
'kubectl', | |
['get', 'pod', '-n', namespace, pod, '-o', 'yaml'] | |
); | |
// Get pod metrics if available | |
let metrics = null; | |
try { | |
const { stdout: metricsOutput } = await execFileAsync( | |
'kubectl', | |
['top', 'pod', '-n', namespace, pod] | |
); | |
metrics = metricsOutput.trim(); | |
} catch (metricsError) { | |
// Metrics might not be available | |
console.log('Metrics not available:', metricsError); | |
} | |
return json({ | |
podInfo: podInfo.trim(), | |
podYaml: podYaml.trim(), | |
metrics | |
}); | |
} catch (error) { | |
console.error('Error fetching pod details:', error); | |
return json({ error: 'Failed to fetch pod details' }, { status: 500 }); | |
} | |
}; |
🤖 Prompt for AI Agents
in
infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/details/+server.ts
lines 1-37: kubectl calls build shell strings with unvalidated route params
creating a command-injection vector; replace exec(promisified) usage with
execFile (or spawn) passing kubectl as the command and each argument as an array
element (e.g. ["describe","pod","-n", namespace, pod]) and validate/sanitize
namespace and pod first (allow only expected pattern: e.g. alphanumerics,
dashes, dots, underscores, max length) and return 400 on invalid input; ensure
you handle stdout trimming and errors the same way but without shell
interpolation; apply the identical changes to the corresponding metrics endpoint
file (…/metrics/+server.ts) so all kubectl invocations use execFile with
validated params.
import { exec } from 'child_process'; | ||
import { promisify } from 'util'; | ||
import { json } from '@sveltejs/kit'; | ||
import type { RequestHandler } from './$types'; | ||
|
||
const execAsync = promisify(exec); | ||
|
||
export const GET: RequestHandler = async ({ params, url }) => { | ||
const { namespace, pod } = params; | ||
const tail = url.searchParams.get('tail') || '100'; | ||
|
||
try { | ||
const { stdout } = await execAsync(`kubectl logs -n ${namespace} ${pod} -c evault --tail=${tail}`); | ||
const logs = stdout.trim().split('\n').filter(line => line.trim()); | ||
|
||
return json({ logs }); | ||
} catch (error) { | ||
console.error('Error fetching logs:', error); | ||
return json({ error: 'Failed to fetch logs', logs: [] }, { status: 500 }); | ||
} | ||
}; No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command injection risk via unvalidated route params; use execFile/spawn with strict validation
Interpolating namespace, pod, and tail into a shell string enables command injection. Validate inputs against RFC 1123 label constraints and use execFile with arg arrays. Also coerce and clamp tail to a safe numeric range.
Apply this diff:
-import { exec } from 'child_process';
+import { execFile } from 'child_process';
import { promisify } from 'util';
import { json } from '@sveltejs/kit';
import type { RequestHandler } from './$types';
-const execAsync = promisify(exec);
+const execFileAsync = promisify(execFile);
export const GET: RequestHandler = async ({ params, url }) => {
const { namespace, pod } = params;
- const tail = url.searchParams.get('tail') || '100';
+ // Validate k8s names: RFC 1123 label (lowercase alphanumeric, '-' allowed), dots allowed for subdomains
+ const isValidK8sName = (v: string) => /^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$/.test(v);
+ if (!isValidK8sName(namespace) || !isValidK8sName(pod)) {
+ return json({ error: 'Invalid namespace or pod' }, { status: 400 });
+ }
+ const tailRaw = url.searchParams.get('tail') || '100';
+ const tailNum = Number.isFinite(Number(tailRaw)) ? Math.floor(Number(tailRaw)) : 100;
+ const tail = String(Math.min(Math.max(tailNum, 1), 2000));
try {
- const { stdout } = await execAsync(`kubectl logs -n ${namespace} ${pod} -c evault --tail=${tail}`);
+ const { stdout } = await execFileAsync(
+ 'kubectl',
+ ['logs', '-n', namespace, pod, '-c', 'evault', '--tail', tail],
+ { maxBuffer: 1024 * 1024 } // 1MB
+ );
const logs = stdout.trim().split('\n').filter(line => line.trim());
return json({ logs });
} catch (error) {
console.error('Error fetching logs:', error);
return json({ error: 'Failed to fetch logs', logs: [] }, { status: 500 });
}
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { exec } from 'child_process'; | |
import { promisify } from 'util'; | |
import { json } from '@sveltejs/kit'; | |
import type { RequestHandler } from './$types'; | |
const execAsync = promisify(exec); | |
export const GET: RequestHandler = async ({ params, url }) => { | |
const { namespace, pod } = params; | |
const tail = url.searchParams.get('tail') || '100'; | |
try { | |
const { stdout } = await execAsync(`kubectl logs -n ${namespace} ${pod} -c evault --tail=${tail}`); | |
const logs = stdout.trim().split('\n').filter(line => line.trim()); | |
return json({ logs }); | |
} catch (error) { | |
console.error('Error fetching logs:', error); | |
return json({ error: 'Failed to fetch logs', logs: [] }, { status: 500 }); | |
} | |
}; | |
import { execFile } from 'child_process'; | |
import { promisify } from 'util'; | |
import { json } from '@sveltejs/kit'; | |
import type { RequestHandler } from './$types'; | |
const execFileAsync = promisify(execFile); | |
export const GET: RequestHandler = async ({ params, url }) => { | |
const { namespace, pod } = params; | |
// Validate k8s names: RFC 1123 label (lowercase alphanumeric, '-' allowed), dots allowed for subdomains | |
const isValidK8sName = (v: string) => /^[a-z0-9]([-a-z0-9.]*[a-z0-9])?$/.test(v); | |
if (!isValidK8sName(namespace) || !isValidK8sName(pod)) { | |
return json({ error: 'Invalid namespace or pod' }, { status: 400 }); | |
} | |
const tailRaw = url.searchParams.get('tail') || '100'; | |
const tailNum = Number.isFinite(Number(tailRaw)) ? Math.floor(Number(tailRaw)) : 100; | |
const tail = String(Math.min(Math.max(tailNum, 1), 2000)); | |
try { | |
const { stdout } = await execFileAsync( | |
'kubectl', | |
['logs', '-n', namespace, pod, '-c', 'evault', '--tail', tail], | |
{ maxBuffer: 1024 * 1024 } // 1MB | |
); | |
const logs = stdout.trim().split('\n').filter(line => line.trim()); | |
return json({ logs }); | |
} catch (error) { | |
console.error('Error fetching logs:', error); | |
return json({ error: 'Failed to fetch logs', logs: [] }, { status: 500 }); | |
} | |
}; |
export function getDeepLinkUrl(qrData: string): string { | ||
return qrData; | ||
} No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Sanitize deep-link URLs to block javascript:/data: injection and constrain to allowed schemes
Returning QR data verbatim can enable javascript:/data: links to be injected into hrefs. At minimum, sanitize and allow only known-safe schemes.
Apply this diff:
export function getDeepLinkUrl(qrData: string): string {
- return qrData;
+ if (!qrData || typeof qrData !== "string") return "";
+ const trimmed = qrData.trim();
+ const lower = trimmed.toLowerCase();
+
+ // Disallow dangerous schemes outright
+ if (lower.startsWith("javascript:") || lower.startsWith("data:")) {
+ return "";
+ }
+
+ // Allow a conservative, extensible set of schemes.
+ // Extend this list if your wallet uses additional custom schemes.
+ const allowedSchemes = new Set([
+ "http:",
+ "https:",
+ "w3ds:",
+ "eudiw:",
+ "wallet:",
+ "eid:",
+ "metastate:"
+ ]);
+
+ try {
+ const url = new URL(trimmed, typeof window !== "undefined" ? window.location.origin : "http://localhost");
+ return allowedSchemes.has(url.protocol) ? url.href : "";
+ } catch {
+ // Not a valid URL; reject
+ return "";
+ }
}
Would you confirm the list of allowed deep-link schemes for EVoting so we can lock this down precisely?
🏁 Script executed:
#!/bin/bash
set -e
echo "Searching for usage of getDeepLinkUrl..."
rg -n "getDeepLinkUrl" -A 2
echo -e "\nSearching for references to known-scheme prefixes..."
for scheme in "w3ds:" "eudiw:" "wallet:" "eid:" "metastate:"; do
echo -e "\nScheme \"$scheme\" occurrences:"
rg -n "$scheme" -A 2 || echo " (none found)"
done
echo -e "\nSearching for any custom deep-link schemes in README or docs..."
rg -n "scheme" -i -A 2 README* docs/
Length of output: 12771
🏁 Script executed:
#!/bin/bash
set -e
echo "Context around getDeepLinkUrl usage in EVoting login page:"
rg -n -C 5 "getDeepLinkUrl" -C 5 "platforms/eVoting/src/app/(auth)/login/page.tsx"
Length of output: 619
Restrict deep-link schemes to only EVoting’s supported URIs
Based on a scan of the EVoting codebase, only w3ds://
, https://
, and http://
links are emitted or consumed. No references to eudiw:
, wallet:
, eid:
or metastate:
were found.
Proposed patch to platforms/eVoting/src/lib/utils/mobile-detection.ts
:
export function getDeepLinkUrl(qrData: string): string {
- return qrData;
+ if (!qrData || typeof qrData !== 'string') return '';
+ const trimmed = qrData.trim();
+ const lower = trimmed.toLowerCase();
+
+ // Block dangerous schemes
+ if (lower.startsWith('javascript:') || lower.startsWith('data:')) {
+ return '';
+ }
+
+ // Only allow EVoting-specific and web schemes
+ const allowedSchemes = new Set(['w3ds:', 'https:', 'http:']);
+
+ try {
+ const url = new URL(
+ trimmed,
+ typeof window !== 'undefined' ? window.location.origin : 'http://localhost'
+ );
+ return allowedSchemes.has(url.protocol) ? url.href : '';
+ } catch {
+ return '';
+ }
}
• File to update:
- platforms/eVoting/src/lib/utils/mobile-detection.ts
Please confirm if any additional deep-link schemes are required for EVoting beyond w3ds:
, https:
, and http:
.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export function getDeepLinkUrl(qrData: string): string { | |
return qrData; | |
} | |
export function getDeepLinkUrl(qrData: string): string { | |
if (!qrData || typeof qrData !== 'string') return ''; | |
const trimmed = qrData.trim(); | |
const lower = trimmed.toLowerCase(); | |
// Block dangerous schemes | |
if (lower.startsWith('javascript:') || lower.startsWith('data:')) { | |
return ''; | |
} | |
// Only allow EVoting-specific and web schemes | |
const allowedSchemes = new Set(['w3ds:', 'https:', 'http:']); | |
try { | |
const url = new URL( | |
trimmed, | |
typeof window !== 'undefined' ? window.location.origin : 'http://localhost' | |
); | |
return allowedSchemes.has(url.protocol) ? url.href : ''; | |
} catch { | |
return ''; | |
} | |
} |
🤖 Prompt for AI Agents
In platforms/eVoting/src/lib/utils/mobile-detection.ts around lines 8 to 10, the
getDeepLinkUrl function currently returns any input; change it to only allow
EVoting-supported schemes by validating qrData and returning it only when it
matches the allowed schemes (w3ds://, https://, http://) — implement a simple
case-insensitive check (e.g., regex or URL parsing) for those schemes and return
an empty string (or null if preferred by project conventions) for any other
scheme; update unit tests accordingly and confirm whether any additional schemes
are required before adding them.
{isMobileDevice() ? ( | ||
<div className="flex flex-col gap-4 items-center"> | ||
<a | ||
href={getDeepLinkUrl(qrData)} | ||
className="px-6 py-3 bg-blue-600 text-white rounded-lg hover:bg-blue-700 transition-colors text-center" | ||
> | ||
Login with eID Wallet | ||
</a> | ||
<div className="text-xs text-gray-500 text-center max-w-xs"> | ||
Click the button to open your eID wallet app | ||
</div> | ||
<p className="p-4 rounded-xl bg-gray-100 text-gray-700 mt-4"> | ||
You are entering Group Charter - a group charter management | ||
platform built on the Web 3.0 Data Space (W3DS) | ||
architecture. This system is designed around the principle | ||
of data-platform separation, where all your personal content | ||
is stored in your own sovereign eVault, not on centralised | ||
servers. | ||
</p> | ||
<Image | ||
src="/W3DS.svg" | ||
alt="W3DS Logo" | ||
width={50} | ||
height={20} | ||
className="mx-auto mt-5" | ||
</div> | ||
) : ( | ||
<div className="bg-white p-4 rounded-lg border"> | ||
<QRCodeSVG | ||
value={qrData} | ||
size={200} | ||
level="M" | ||
includeMargin={true} | ||
/> | ||
</div> | ||
</div> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hydration mismatch: compute “isMobile” after mount instead of calling isMobileDevice() during render.
isMobileDevice() returns false on the server (window undefined), so SSR renders the QR path while the client may swap to the mobile button post-hydration, risking warnings and UI flicker. Derive an isMobile state in an effect and use that in the JSX.
Apply this diff to the JSX:
- {isMobileDevice() ? (
+ {isMobile ? (
Then add this state setup (outside the shown range) near the other hooks:
// Add near other state hooks
const [isMobile, setIsMobile] = useState(false);
// After mount, evaluate once to avoid SSR/CSR divergence
useEffect(() => {
setIsMobile(isMobileDevice());
}, []);
Optional refinements:
- Consider a “Show QR code instead” fallback on mobile in case deep-linking fails.
- The helper text at lines 131–133 currently mentions scanning a QR code; make it conditional so mobile users see deep-link guidance instead.
🤖 Prompt for AI Agents
In platforms/group-charter-manager/src/components/auth/login-screen.tsx around
lines 105 to 126, avoid calling isMobileDevice() during render (causes SSR/CSR
hydration mismatch); instead add a local state const [isMobile, setIsMobile] =
useState(false) near the other hooks and run useEffect(() =>
setIsMobile(isMobileDevice()), [] ) to compute it once after mount, then replace
direct calls to isMobileDevice() in the JSX with the isMobile state; also make
the helper text conditional so mobile users see deep-link guidance while
non-mobile users see the QR scanning message.
export function isMobileDevice(): boolean { | ||
if (typeof window === 'undefined') return false; | ||
|
||
return /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) || | ||
(window.innerWidth <= 768); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Strengthen device detection (avoid viewport-only false positives; prefer modern signals).
Relying on window.innerWidth ≤ 768 can misclassify narrow desktop windows as “mobile.” Prefer UA-CH (navigator.userAgentData.mobile) when available, and combine touch/pointer-coarse heuristics to reduce false positives. Also guard navigator access.
Apply this diff:
-export function isMobileDevice(): boolean {
- if (typeof window === 'undefined') return false;
-
- return /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) ||
- (window.innerWidth <= 768);
-}
+export function isMobileDevice(): boolean {
+ if (typeof window === 'undefined' || typeof navigator === 'undefined') return false;
+
+ // Prefer UA-CH when available (modern browsers)
+ const uaDataMobile = (navigator as any).userAgentData?.mobile;
+ if (typeof uaDataMobile === 'boolean') return uaDataMobile;
+
+ const ua = navigator.userAgent;
+ const uaMatch = /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(ua);
+
+ const coarse = typeof window.matchMedia === 'function'
+ ? window.matchMedia('(pointer: coarse)').matches
+ : false;
+ const touch = (navigator as any).maxTouchPoints > 0;
+
+ const narrow = window.innerWidth <= 768;
+
+ // Combine signals; avoid treating a narrow desktop window as mobile unless it also appears touch/coarse
+ return uaMatch || ((touch || coarse) && narrow);
+}
🤖 Prompt for AI Agents
In platforms/group-charter-manager/src/lib/utils/mobile-detection.ts around
lines 1 to 6, replace the current heuristic that falls back to window.innerWidth
<= 768 with a modern, safer detection: first guard access to window and
navigator, then if navigator.userAgentData exists use
navigator.userAgentData.mobile; otherwise check navigator.maxTouchPoints > 0 and
matchMedia('(pointer:coarse)') as strong mobile signals; if those are absent
fall back to the UA regex; remove or deprioritize the innerWidth check to avoid
narrow-desktop false positives and ensure all navigator accesses are guarded
with typeof checks.
export function getDeepLinkUrl(qrData: string): string { | ||
return qrData; | ||
} No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Sanitize deep-link URLs to block javascript: and other unsupported schemes.
Passing qrData straight into href risks XSS via schemes like javascript:. Whitelist expected schemes and fall back safely.
Apply this diff:
-export function getDeepLinkUrl(qrData: string): string {
- return qrData;
-}
+export function getDeepLinkUrl(qrData: string): string {
+ // Allow only known safe schemes used by wallets. Adjust to your ecosystem as needed.
+ const allowedSchemes = [
+ 'https',
+ 'http',
+ 'openid',
+ 'openid-vc',
+ 'openid-credential-offer',
+ 'openid-initiate-issuance',
+ 'openid4vp',
+ 'w3ds',
+ 'eudiw',
+ ];
+ try {
+ const scheme = qrData.split(':')[0]?.toLowerCase();
+ if (!scheme || !allowedSchemes.includes(scheme)) {
+ console.warn('Blocked deep-link with unsupported scheme:', scheme);
+ return '#';
+ }
+ return qrData;
+ } catch {
+ return '#';
+ }
+}
To ensure the whitelist matches conventions elsewhere in the repo, run:
🏁 Script executed:
#!/bin/bash
# Discover deep-link schemes and related helpers used across the repo
rg -n -A2 -i $'getDeepLinkUrl\\(|deeplink|deep-link|openid|credential-offer|openid4vp|eudiw|w3ds'
Length of output: 320663
Sanitize deep-link URLs (block javascript: / restrict schemes) — fix required across frontends
Passing raw QR data into href is a real XSS risk (javascript: and other unwanted schemes). We should whitelist known deep-link schemes (w3ds + https/http and any openid/credential schemes you expect) and return a safe fallback (e.g. '#') for anything else.
Files that need this change (definitive hits from repo search):
- platforms/group-charter-manager/src/lib/utils/mobile-detection.ts
- platforms/eVoting/src/lib/utils/mobile-detection.ts
- platforms/blabsy/src/lib/utils/mobile-detection.ts
- Update callers that render hrefs to use the sanitized output (examples found):
- platforms/group-charter-manager/src/components/auth/login-screen.tsx — href={getDeepLinkUrl(qrData)}
- platforms/eVoting/src/app/(auth)/login/page.tsx — href={getDeepLinkUrl(qrData)}
- platforms/eVoting/src/components/signing-interface.tsx — href={getDeepLinkUrl(qrData)}
- platforms/blabsy/src/components/login/login-main.tsx — href={getDeepLinkUrl(qr)}
Why: the repo generates and embeds w3ds:// deep links (see controllers and test-deep-link.html). At minimum the whitelist must include 'w3ds' and usual web schemes — everything else should be rejected.
Apply this replacement in each mobile-detection.ts (example diff to replace the current trivial function):
-export function getDeepLinkUrl(qrData: string): string {
- return qrData;
-}
+export function getDeepLinkUrl(qrData: string): string {
+ // Allow only known safe schemes used by wallets / redirects.
+ // Adjust the list if you intentionally support more schemes.
+ const allowedSchemes = new Set<string>([
+ 'w3ds',
+ 'https',
+ 'http',
+ 'openid',
+ 'openid-vc',
+ 'openid-credential-offer',
+ 'openid-initiate-issuance',
+ 'openid4vp',
+ 'eudiw',
+ ]);
+
+ try {
+ const scheme = String(qrData).split(':')[0]?.toLowerCase();
+ if (!scheme || !allowedSchemes.has(scheme)) {
+ console.warn('Blocked deep-link with unsupported scheme:', scheme, qrData);
+ return '#';
+ }
+ return qrData;
+ } catch (err) {
+ console.warn('Malformed deep-link, blocking:', qrData, err);
+ return '#';
+ }
+}
Recommendations:
- Apply the same change to every copy of mobile-detection.ts (or better: centralize this helper into a shared lib and import it everywhere).
- Update any unit/UX tests that expect raw URIs.
- Audit usages where qrData comes from untrusted input and ensure callers correctly handle the '#' fallback (e.g., disable the link when fallback is returned).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export function getDeepLinkUrl(qrData: string): string { | |
return qrData; | |
} | |
export function getDeepLinkUrl(qrData: string): string { | |
// Allow only known safe schemes used by wallets / redirects. | |
// Adjust the list if you intentionally support more schemes. | |
const allowedSchemes = new Set<string>([ | |
'w3ds', | |
'https', | |
'http', | |
'openid', | |
'openid-vc', | |
'openid-credential-offer', | |
'openid-initiate-issuance', | |
'openid4vp', | |
'eudiw', | |
]); | |
try { | |
const scheme = String(qrData).split(':')[0]?.toLowerCase(); | |
if (!scheme || !allowedSchemes.has(scheme)) { | |
console.warn('Blocked deep-link with unsupported scheme:', scheme, qrData); | |
return '#'; | |
} | |
return qrData; | |
} catch (err) { | |
console.warn('Malformed deep-link, blocking:', qrData, err); | |
return '#'; | |
} | |
} |
🤖 Prompt for AI Agents
In platforms/group-charter-manager/src/lib/utils/mobile-detection.ts around
lines 8-10, the current getDeepLinkUrl returns raw qrData which allows dangerous
schemes (e.g. javascript:) and is an XSS risk; replace it with a sanitizer that
whitelists allowed schemes (at minimum "w3ds", "http", "https" and any expected
openid/credential schemes), only returns the input when it uses a whitelisted
scheme, and otherwise returns a safe fallback like "#" (callers should treat "#"
as disabled link); apply same change to the other copies of mobile-detection.ts,
update any tests expecting raw URIs, and ensure callers that render hrefs
disable the link when getDeepLinkUrl returns the fallback.
} | ||
}} | ||
></article> | ||
<a href={qrData}>{qrData}</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid exposing raw login QR URI in the DOM; gate and sanitize the link
Rendering the full QR URI (likely containing a session id/token) as a clickable link can enable session hijacking via shoulder-surfing, copy/paste leakage, or link prefetch by extensions. At minimum, validate allowed protocols, and hide behind a dev/feature flag. Prefer a “Copy link” action over in-page navigation.
Apply this diff to avoid rendering in production and ensure only safe protocols are used:
- <a href={qrData}>{qrData}</a>
+ {#if showQrLink && safeQrLink}
+ <a
+ href={safeQrLink}
+ rel="noopener"
+ target="_blank"
+ class="text-xs text-gray-400 break-all underline"
+ >Open login link</a
+ >
+ <p class="mt-1 text-[10px] text-gray-400 break-all">{safeQrLink}</p>
+ {/if}
Add the following to the <script> section to define safeQrLink and the dev-only toggle:
// Add near other imports/vars
const allowedProtocols = new Set(['https:', 'w3ds:', 'w3dsapp:']);
let safeQrLink: string | null = null;
const showQrLink = import.meta.env.MODE !== 'production';
// After setting qrData in onMount
try {
const u = new URL(qrData);
if (allowedProtocols.has(u.protocol)) safeQrLink = u.toString();
} catch {
safeQrLink = null;
}
Optional: Replace the anchor with a “Copy link” button using the Clipboard API to further reduce accidental navigations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
[dependencies] | ||
tauri = { version = "2", features = [] } | ||
tauri-plugin-opener = "2" | ||
tauri-plugin-deep-link = "2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add deep-link plugin: confirm version alignment and platform config
Good addition. Please verify:
- JS plugin version matches crate major (JS ^2.4.1 vs Rust "2" are compatible).
- Android deep-link intent filters are configured (Info.ios.plist is updated for iOS; ensure Android has scheme "w3ds" too).
Run this script to check for Android deep-link configuration and scheme usage:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for Android manifest and deep link scheme 'w3ds'..."
fd -HI "AndroidManifest.xml" -t f | while read -r f; do
echo "Found: $f"
rg -n "intent-filter|scheme|w3ds|android.intent.action.VIEW" -A 2 -B 2 "$f" || true
done
echo
echo "Searching capabilities for deep-link custom scheme config..."
rg -n "deep-link|scheme|w3ds" -A 3 -B 3 infrastructure/eid-wallet/src-tauri/capabilities || true
Length of output: 2201
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for JS deep-link plugin versions in package.json..."
fd -HI package.json -t f | while read -r f; do
echo "File: $f"
rg -n '"deep-link"' -A 1 -B 1 "$f" || true
done
echo
echo "Searching for iOS Info.plist entries for scheme 'w3ds'..."
fd -HI Info.plist -t f | while read -r f; do
echo "File: $f"
rg -n "w3ds" -A 2 -B 2 "$f" || true
done
Length of output: 4919
#!/usr/bin/env bash
set -euo pipefail
echo "Searching for tauri-plugin-deep-link references across the repo..."
rg -n "tauri-plugin-deep-link" .
Ensure JS deep-link plugin is installed and version-aligned
I’ve confirmed platform configs are in place but don’t see the JS side of the plugin yet:
- infrastructure/eid-wallet/src-tauri/gen/android/app/src/main/AndroidManifest.xml
• Intent-filter for android.intent.action.VIEW with<data android:scheme="w3ds"/>
is present. - infrastructure/eid-wallet/src-tauri/gen/apple/eid-wallet_iOS/Info.plist
• CFBundleURLSchemes includes"w3ds"
. - No
@tauri/plugin-deep-link
(or similar) in anypackage.json
.
Please add the JS plugin at major version 2 to match the Rust crate, for example:
--- package.json
+++ package.json
@@ "dependencies": {
- // existing deps...
+ "@tauri/plugin-deep-link": "^2.0.0",
// other deps...
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
tauri-plugin-deep-link = "2" | |
// package.json | |
{ | |
"dependencies": { | |
// existing deps... | |
"@tauri/plugin-deep-link": "^2.0.0", | |
// other deps... | |
} | |
} |
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src-tauri/Cargo.toml around line 23, the Rust
tauri-plugin-deep-link is set to v2 but the JS side plugin is missing; add the
corresponding JS package "@tauri/plugin-deep-link" at major version 2 (e.g.
^2.0.0) to the project's package.json dependencies, run your package manager
install (npm/yarn/pnpm) to update lockfiles, and verify the installed version is
v2 to keep Rust and JS plugin versions aligned.
onMount(async () => { | ||
// Get global state | ||
globalState = getContext<() => GlobalState>("globalState")(); | ||
// Authentication guard for all app routes | ||
try { | ||
if (!globalState) { | ||
console.log("No global state, redirecting to login"); | ||
await goto("/login"); | ||
return; | ||
} | ||
const vault = await globalState.vaultController.vault; | ||
if (!vault) { | ||
console.log("User not authenticated, redirecting to login"); | ||
await goto("/login"); | ||
return; | ||
} | ||
console.log("User authenticated, allowing access to app routes"); | ||
} catch (error) { | ||
console.log("Authentication check failed, redirecting to login"); | ||
await goto("/login"); | ||
return; | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider handling authentication state more robustly
The authentication guard implementation has several potential issues:
- Multiple redirect attempts without proper state management
- Logging sensitive authentication states
- No differentiation between different authentication failure reasons
onMount(async () => {
// Get global state
globalState = getContext<() => GlobalState>("globalState")();
// Authentication guard for all app routes
try {
if (!globalState) {
- console.log("No global state, redirecting to login");
+ console.error("Global state initialization failed");
await goto("/login");
return;
}
const vault = await globalState.vaultController.vault;
if (!vault) {
- console.log("User not authenticated, redirecting to login");
+ // Store the attempted route for redirect after login
+ sessionStorage.setItem("redirectAfterLogin", page.url.pathname);
await goto("/login");
return;
}
- console.log("User authenticated, allowing access to app routes");
+ // User authenticated successfully
} catch (error) {
- console.log("Authentication check failed, redirecting to login");
+ console.error("Authentication check failed:", error);
+ sessionStorage.setItem("authError", error instanceof Error ? error.message : "Unknown error");
await goto("/login");
return;
}
});
Committable suggestion skipped: line range outside the PR's diff.
$effect(() => { | ||
const isScanPage = currentRoute === "scan-qr"; | ||
if (isScanPage) | ||
return document.body.classList.add("custom-global-style"); | ||
return document.body.classList.remove("custom-global-style"); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Clean up global styles on component unmount
The effect adds a global CSS class but doesn't clean it up when the component unmounts, which could cause style leaks.
$effect(() => {
const isScanPage = currentRoute === "scan-qr";
if (isScanPage)
return document.body.classList.add("custom-global-style");
return document.body.classList.remove("custom-global-style");
+
+ return () => {
+ // Cleanup on unmount
+ document.body.classList.remove("custom-global-style");
+ };
});
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/+layout.svelte around lines 41 to
46, the effect adds a global CSS class to document.body but doesn't guarantee
cleanup on component unmount; update the code to always remove the
"custom-global-style" class when the component is destroyed (use Svelte's
onDestroy to call document.body.classList.remove("custom-global-style")), and
ensure the effect toggles the class idempotently (remove before add or use
add/remove as currently written) so no global style leak remains after unmount.
// Redirect back to the platform that initiated the request | ||
try { | ||
console.log( | ||
"Attempting redirect to:", | ||
data.redirect, | ||
); | ||
// Try multiple redirect methods for better compatibility | ||
try { | ||
// Method 1: Direct assignment | ||
window.location.href = data.redirect; | ||
} catch (error1) { | ||
console.log( | ||
"Method 1 failed, trying method 2:", | ||
error1, | ||
); | ||
try { | ||
// Method 2: Using assign | ||
window.location.assign(data.redirect); | ||
} catch (error2) { | ||
console.log( | ||
"Method 2 failed, trying method 3:", | ||
error2, | ||
); | ||
try { | ||
// Method 3: Using replace | ||
window.location.replace(data.redirect); | ||
} catch (error3) { | ||
console.log( | ||
"Method 3 failed, using fallback:", | ||
error3, | ||
); | ||
throw new Error( | ||
"All redirect methods failed", | ||
); | ||
} | ||
} | ||
} | ||
} catch (error) { | ||
console.error( | ||
"All redirect methods failed, staying in app:", | ||
error, | ||
); | ||
// If redirect fails, fall back to normal flow | ||
loggedInDrawerOpen = true; | ||
startScan(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify redirect logic and handle failures properly
The triple-nested try-catch blocks for redirect methods are overly complex and may mask errors. Also, falling back to scanning on redirect failure doesn't make sense UX-wise.
- // Redirect back to the platform that initiated the request
- try {
- console.log(
- "Attempting redirect to:",
- data.redirect,
- );
-
- // Try multiple redirect methods for better compatibility
- try {
- // Method 1: Direct assignment
- window.location.href = data.redirect;
- } catch (error1) {
- console.log(
- "Method 1 failed, trying method 2:",
- error1,
- );
-
- try {
- // Method 2: Using assign
- window.location.assign(data.redirect);
- } catch (error2) {
- console.log(
- "Method 2 failed, trying method 3:",
- error2,
- );
-
- try {
- // Method 3: Using replace
- window.location.replace(data.redirect);
- } catch (error3) {
- console.log(
- "Method 3 failed, using fallback:",
- error3,
- );
- throw new Error(
- "All redirect methods failed",
- );
- }
- }
- }
- } catch (error) {
- console.error(
- "All redirect methods failed, staying in app:",
- error,
- );
- // If redirect fails, fall back to normal flow
- loggedInDrawerOpen = true;
- startScan();
- }
+ // Redirect back to the platform
+ console.log("Redirecting to:", data.redirect);
+ try {
+ // Use location.href as the standard method
+ window.location.href = data.redirect;
+ } catch (error) {
+ console.error("Redirect failed:", error);
+ // Show error message to user instead of starting scan
+ errorMessage = "Failed to redirect back to the platform. Please try again.";
+ // Show error drawer or notification
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Redirect back to the platform that initiated the request | |
try { | |
console.log( | |
"Attempting redirect to:", | |
data.redirect, | |
); | |
// Try multiple redirect methods for better compatibility | |
try { | |
// Method 1: Direct assignment | |
window.location.href = data.redirect; | |
} catch (error1) { | |
console.log( | |
"Method 1 failed, trying method 2:", | |
error1, | |
); | |
try { | |
// Method 2: Using assign | |
window.location.assign(data.redirect); | |
} catch (error2) { | |
console.log( | |
"Method 2 failed, trying method 3:", | |
error2, | |
); | |
try { | |
// Method 3: Using replace | |
window.location.replace(data.redirect); | |
} catch (error3) { | |
console.log( | |
"Method 3 failed, using fallback:", | |
error3, | |
); | |
throw new Error( | |
"All redirect methods failed", | |
); | |
} | |
} | |
} | |
} catch (error) { | |
console.error( | |
"All redirect methods failed, staying in app:", | |
error, | |
); | |
// If redirect fails, fall back to normal flow | |
loggedInDrawerOpen = true; | |
startScan(); | |
} | |
// Redirect back to the platform | |
console.log("Redirecting to:", data.redirect); | |
try { | |
// Use location.href as the standard method | |
window.location.href = data.redirect; | |
} catch (error) { | |
console.error("Redirect failed:", error); | |
// Show error message to user instead of starting scan | |
errorMessage = "Failed to redirect back to the platform. Please try again."; | |
// Show error drawer or notification | |
} |
(Total: {Object.values(signingData.voteData.points).reduce( | ||
(sum, points) => | ||
(sum as number) + ((points as number) || 0), | ||
0, | ||
)}/100 points) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix potential type safety issue in points calculation
The reduce operation has unsafe type casting that could lead to runtime errors.
(Total: {Object.values(signingData.voteData.points).reduce(
- (sum, points) =>
- (sum as number) + ((points as number) || 0),
- 0,
+ (sum: number, points: unknown) =>
+ sum + (typeof points === 'number' ? points : 0),
+ 0 as number,
)}/100 points)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
(Total: {Object.values(signingData.voteData.points).reduce( | |
(sum, points) => | |
(sum as number) + ((points as number) || 0), | |
0, | |
)}/100 points) | |
(Total: {Object.values(signingData.voteData.points).reduce( | |
(sum: number, points: unknown) => | |
sum + (typeof points === 'number' ? points : 0), | |
0 as number, | |
)}/100 points) |
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte around lines
737 to 741, the reduce uses unsafe type casts (as number) which can cause
runtime errors; instead ensure the values are treated as numbers by using a
proper type-safe approach: reference the points object with optional chaining
(signingData.voteData?.points) to avoid undefined, convert or validate each
value before summing (e.g., map or guard each entry with typeof checks or
Number(...) and treat NaN/undefined as 0), and use a correctly typed initial
accumulator (0 as number) so the reduce operates on numbers only; update the
expression accordingly or move the logic into a small typed helper function in
the script block that returns the total points.
const isPollActive = (poll: Poll) => { | ||
if (!poll.isActive) return false; | ||
if (!poll.deadline) return true; | ||
return new Date() < new Date(poll.deadline); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Handle invalid deadline values in isPollActive to avoid false negatives
If deadline is an invalid date string, new Date(poll.deadline) becomes Invalid Date and comparison fails unpredictably.
- if (!poll.deadline) return true;
- return new Date() < new Date(poll.deadline);
+ if (!poll.deadline) return true;
+ const deadline = new Date(poll.deadline);
+ if (isNaN(deadline.getTime())) return true; // Treat invalid deadlines as no deadline
+ return new Date() < deadline;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const isPollActive = (poll: Poll) => { | |
if (!poll.isActive) return false; | |
if (!poll.deadline) return true; | |
return new Date() < new Date(poll.deadline); | |
}; | |
const isPollActive = (poll: Poll) => { | |
if (!poll.deadline) return true; | |
const deadline = new Date(poll.deadline); | |
if (isNaN(deadline.getTime())) return true; // Treat invalid deadlines as no deadline | |
return new Date() < deadline; | |
}; |
🤖 Prompt for AI Agents
In platforms/eVoting/src/app/(app)/page.tsx around lines 39 to 42, isPollActive
currently constructs new Date(poll.deadline) without validating it, which can
produce an Invalid Date and lead to unpredictable false negatives; update the
function to parse the deadline into a Date, check its validity (e.g., ensure
!isNaN(date.getTime())), and only perform the comparison when the date is
valid—if the deadline is missing or invalid, return true (treat poll as active).
{poll.mode === "rank" | ||
? `${poll.votes?.length || 0} points` | ||
: `${poll.votes?.length || 0} votes`} | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect “points” label uses vote count for ranked mode
For ranked polls you display “N points” but still compute N from votes length, which is misleading. Either compute actual points from results or just label them as votes consistently.
Minimal fix to label consistently:
- {poll.mode === "rank"
- ? `${poll.votes?.length || 0} points`
- : `${poll.votes?.length || 0} votes`}
+ {`${poll.votes?.length || 0} votes`}
If you want to show points, fetch them via pollApi.getPollResults(poll.id) and compute the aggregate.
useEffect(() => { | ||
return () => { | ||
// Only close SSE connection if signing is not complete | ||
if (eventSource && status !== "signed") { | ||
eventSource.close(); | ||
} | ||
}; | ||
}, [eventSource, status]); | ||
|
||
// Additional cleanup when signing is complete | ||
useEffect(() => { | ||
if (status === "signed" && eventSource) { | ||
eventSource.close(); | ||
} | ||
}, [status, eventSource]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Redundant cleanup logic for SSE connection
There are multiple useEffect
hooks managing the SSE connection cleanup, which could lead to race conditions or unexpected behavior. The cleanup logic appears in lines 60-66, 136-143, and 145-150.
Consolidate the cleanup logic into a single effect:
-// Cleanup on unmount
-useEffect(() => {
- return () => {
- if (eventSource) {
- eventSource.close();
- }
- };
-}, [eventSource]);
-// Cleanup SSE connection
-useEffect(() => {
- return () => {
- // Only close SSE connection if signing is not complete
- if (eventSource && status !== "signed") {
- eventSource.close();
- }
- };
-}, [eventSource, status]);
-
-// Additional cleanup when signing is complete
-useEffect(() => {
- if (status === "signed" && eventSource) {
- eventSource.close();
- }
-}, [status, eventSource]);
+// Consolidated SSE cleanup
+useEffect(() => {
+ // Close on status change to signed
+ if (status === "signed" && eventSource) {
+ eventSource.close();
+ setEventSource(null);
+ }
+
+ // Cleanup on unmount
+ return () => {
+ if (eventSource) {
+ eventSource.close();
+ }
+ };
+}, [eventSource, status]);
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In platforms/eVoting/src/components/signing-interface.tsx around lines 136-150,
consolidate the multiple SSE cleanup useEffects into a single effect: remove the
other duplicate effects (including those at lines ~60-66 and 145-150), create
one useEffect with dependencies [eventSource, status] that (1) if status ===
"signed" and eventSource exists, immediately closes the eventSource, and (2)
returns a cleanup function that closes eventSource on unmount only when it still
exists and signing is not complete (status !== "signed"); ensure the effect
handles null/undefined eventSource safely.
export interface SigningSession { | ||
sessionId: string; | ||
qrData: string; | ||
expiresAt: string; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SigningSession shape likely mismatches backend (“id” vs “sessionId”, missing status)
Server snippet indicates a SigningSession with id and status fields. Using sessionId here risks runtime/type mismatches.
Align interface to backend (dates as strings are fine for client):
-export interface SigningSession {
- sessionId: string;
- qrData: string;
- expiresAt: string;
-}
+export interface SigningSession {
+ id: string;
+ qrData: string;
+ status: "pending" | "signed" | "expired" | "completed";
+ expiresAt: string;
+}
Please verify usages of sessionId in consumers (e.g., signing-interface) and update to id if necessary.
🤖 Prompt for AI Agents
In platforms/eVoting/src/lib/pollApi.ts around lines 52 to 56, the
SigningSession interface doesn't match the backend shape (backend uses id and
includes status) which risks runtime/type mismatches; update the interface to
use id: string instead of sessionId: string and add status: string (keep
expiresAt and qrData as string), then search/replace any consumers that
reference sessionId (e.g., signing-interface) to use id and adjust code paths
that rely on the missing status field to handle or display it appropriately.
// Create signing session | ||
createSigningSession: async (pollId: string, voteData: any, userId: string): Promise<SigningSession> => { | ||
const response = await apiClient.post("/api/signing/sessions", { | ||
pollId, | ||
voteData, | ||
userId, | ||
}); | ||
return response.data; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Client-supplied userId in createSigningSession is a security risk unless server ignores it
Accepting userId from the client can enable impersonation if the server trusts this field. Prefer deriving the user from auth context (token/session) on the server side.
Consider removing userId from the client request and server API. If removal is viable now:
- createSigningSession: async (pollId: string, voteData: any, userId: string): Promise<SigningSession> => {
- const response = await apiClient.post("/api/signing/sessions", {
- pollId,
- voteData,
- userId,
- });
+ createSigningSession: async (pollId: string, voteData: any): Promise<SigningSession> => {
+ const response = await apiClient.post("/api/signing/sessions", {
+ pollId,
+ voteData,
+ });
Run a repo-wide check to confirm server-side ignores client userId and identify call sites to update:
🏁 Script executed:
#!/bin/bash
# Find createSigningSession usage and server handlers
rg -n "createSigningSession\\(" -A 2
rg -n "/api/signing/sessions" -A 5
Length of output: 3283
🏁 Script executed:
#!/bin/bash
# Inspect the createSigningSession implementation in the SigningController
rg -n "async createSigningSession" -A 20 platforms/evoting-api/src/controllers/SigningController.ts
Length of output: 993
Enforce server-side user identity and remove client-supplied userId
The Signing API currently trusts a client-provided userId
—both in the front-end call and in the controller—which allows any user to impersonate another. We should drop userId
from the request payload entirely and derive the caller’s identity from the authentication context.
Changes required:
- In the client library (platforms/eVoting/src/lib/pollApi.ts):
• Remove theuserId
parameter and payload field. - In the signing UI (platforms/eVoting/src/components/signing-interface.tsx):
• Stop passinguser.id
intocreateSigningSession
. - In the server controller (platforms/evoting-api/src/controllers/SigningController.ts):
• Remove{ userId } = req.body
and related validation.
• Extract the authenticated user’s ID fromreq.user
(or equivalent) before callingcreateSession
.
Example diff for the client API:
--- a/platforms/eVoting/src/lib/pollApi.ts
+++ b/platforms/eVoting/src/lib/pollApi.ts
@@ -120,7 +120,7 @@ export const pollApi = {
- createSigningSession: async (pollId: string, voteData: any, userId: string): Promise<SigningSession> => {
- const response = await apiClient.post("/api/signing/sessions", {
- pollId,
- voteData,
- userId,
- });
+ createSigningSession: async (pollId: string, voteData: any): Promise<SigningSession> => {
+ const response = await apiClient.post("/api/signing/sessions", { pollId, voteData });
return response.data;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Create signing session | |
createSigningSession: async (pollId: string, voteData: any, userId: string): Promise<SigningSession> => { | |
const response = await apiClient.post("/api/signing/sessions", { | |
pollId, | |
voteData, | |
userId, | |
}); | |
return response.data; | |
} | |
// Create signing session | |
createSigningSession: async (pollId: string, voteData: any): Promise<SigningSession> => { | |
const response = await apiClient.post("/api/signing/sessions", { pollId, voteData }); | |
return response.data; | |
} |
🤖 Prompt for AI Agents
In platforms/eVoting/src/lib/pollApi.ts around lines 121-129, the client
currently sends a userId in the createSigningSession payload which allows
client-side impersonation; remove the userId parameter from the
createSigningSession function signature and from the POST payload so the request
only sends pollId and voteData, update any call sites (e.g.,
platforms/eVoting/src/components/signing-interface.tsx) to stop passing user.id,
and ensure the server-side SigningController
(platforms/evoting-api/src/controllers/SigningController.ts) no longer reads
userId from req.body but instead derives the authenticated user ID from req.user
(or your auth context) before calling createSession.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review continued from previous batch...
createPoll = async (req: Request, res: Response) => { | ||
try { | ||
const { title, mode, visibility, options, deadline } = req.body; | ||
const creatorId = (req as any).user.id; | ||
|
||
const poll = await this.pollService.createPoll({ | ||
title, | ||
mode, | ||
visibility, | ||
options, | ||
deadline, | ||
creatorId | ||
}); | ||
|
||
res.status(201).json(poll); | ||
} catch (error) { | ||
console.error("Error creating poll:", error); | ||
res.status(500).json({ error: "Failed to create poll" }); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add input validation for poll creation
The controller doesn't validate required fields or their formats before passing them to the service. This could lead to invalid data being stored or runtime errors.
Add validation before calling the service:
createPoll = async (req: Request, res: Response) => {
try {
const { title, mode, visibility, options, deadline } = req.body;
const creatorId = (req as any).user.id;
+
+ // Validate required fields
+ if (!title || !mode || !visibility || !options) {
+ return res.status(400).json({
+ error: "Missing required fields: title, mode, visibility, options"
+ });
+ }
+
+ // Validate mode
+ if (!["normal", "point", "rank"].includes(mode)) {
+ return res.status(400).json({
+ error: "Invalid mode. Must be one of: normal, point, rank"
+ });
+ }
+
+ // Validate visibility
+ if (!["public", "private"].includes(visibility)) {
+ return res.status(400).json({
+ error: "Invalid visibility. Must be one of: public, private"
+ });
+ }
+
+ // Validate options
+ if (!Array.isArray(options) || options.length === 0) {
+ return res.status(400).json({
+ error: "Options must be a non-empty array"
+ });
+ }
+
+ // Validate deadline if provided
+ if (deadline && isNaN(new Date(deadline).getTime())) {
+ return res.status(400).json({
+ error: "Invalid deadline format"
+ });
+ }
const poll = await this.pollService.createPoll({
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
createPoll = async (req: Request, res: Response) => { | |
try { | |
const { title, mode, visibility, options, deadline } = req.body; | |
const creatorId = (req as any).user.id; | |
const poll = await this.pollService.createPoll({ | |
title, | |
mode, | |
visibility, | |
options, | |
deadline, | |
creatorId | |
}); | |
res.status(201).json(poll); | |
} catch (error) { | |
console.error("Error creating poll:", error); | |
res.status(500).json({ error: "Failed to create poll" }); | |
} | |
}; | |
createPoll = async (req: Request, res: Response) => { | |
try { | |
const { title, mode, visibility, options, deadline } = req.body; | |
const creatorId = (req as any).user.id; | |
// Validate required fields | |
if (!title || !mode || !visibility || !options) { | |
return res.status(400).json({ | |
error: "Missing required fields: title, mode, visibility, options" | |
}); | |
} | |
// Validate mode | |
if (!["normal", "point", "rank"].includes(mode)) { | |
return res.status(400).json({ | |
error: "Invalid mode. Must be one of: normal, point, rank" | |
}); | |
} | |
// Validate visibility | |
if (!["public", "private"].includes(visibility)) { | |
return res.status(400).json({ | |
error: "Invalid visibility. Must be one of: public, private" | |
}); | |
} | |
// Validate options | |
if (!Array.isArray(options) || options.length === 0) { | |
return res.status(400).json({ | |
error: "Options must be a non-empty array" | |
}); | |
} | |
// Validate deadline if provided | |
if (deadline && isNaN(new Date(deadline).getTime())) { | |
return res.status(400).json({ | |
error: "Invalid deadline format" | |
}); | |
} | |
const poll = await this.pollService.createPoll({ | |
title, | |
mode, | |
visibility, | |
options, | |
deadline, | |
creatorId | |
}); | |
res.status(201).json(poll); | |
} catch (error) { | |
console.error("Error creating poll:", error); | |
res.status(500).json({ error: "Failed to create poll" }); | |
} | |
}; |
🤖 Prompt for AI Agents
In platforms/evoting-api/src/controllers/PollController.ts around lines 38 to
57, the createPoll handler currently forwards req.body to the service without
validating required fields or formats; add input validation to ensure title,
mode, visibility, options, and deadline are present and of the expected types
(e.g., title non-empty string, mode/visibility within allowed enums, options an
array with at least two non-empty strings, deadline a valid future date if
provided), return 400 with a clear error message when validation fails, and only
call this.pollService.createPoll when validation passes; keep existing try/catch
for service errors and use concise validation error responses.
constructor() { | ||
try { | ||
this.signingService = new SigningService(); | ||
console.log("SigningController initialized successfully"); | ||
} catch (error) { | ||
console.error("Failed to initialize SigningController:", error); | ||
this.signingService = null; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconsider swallowing initialization errors
The constructor catches and swallows errors during SigningService initialization, setting the service to null. This could mask configuration issues and make debugging difficult.
Consider failing fast if the service cannot be initialized:
constructor() {
try {
this.signingService = new SigningService();
console.log("SigningController initialized successfully");
} catch (error) {
console.error("Failed to initialize SigningController:", error);
- this.signingService = null;
+ throw new Error(`SigningController initialization failed: ${error instanceof Error ? error.message : String(error)}`);
}
}
This ensures the application doesn't start with a broken signing service, making issues more visible during deployment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
constructor() { | |
try { | |
this.signingService = new SigningService(); | |
console.log("SigningController initialized successfully"); | |
} catch (error) { | |
console.error("Failed to initialize SigningController:", error); | |
this.signingService = null; | |
} | |
} | |
constructor() { | |
try { | |
this.signingService = new SigningService(); | |
console.log("SigningController initialized successfully"); | |
} catch (error) { | |
console.error("Failed to initialize SigningController:", error); | |
throw new Error( | |
`SigningController initialization failed: ${ | |
error instanceof Error ? error.message : String(error) | |
}` | |
); | |
} | |
} |
🤖 Prompt for AI Agents
In platforms/evoting-api/src/controllers/SigningController.ts around lines 7 to
15, the constructor currently catches errors from new SigningService() and sets
signingService to null, which hides initialization failures; change this to fail
fast by removing the try/catch or rethrowing the caught error so the process
fails during startup if the SigningService cannot be created. Ensure the
constructor either allows the original error to propagate or throws a new Error
with contextual details (service name and config) so the application does not
continue running with a null signingService.
// Set SSE headers | ||
res.writeHead(200, { | ||
"Content-Type": "text/event-stream", | ||
"Cache-Control": "no-cache", | ||
"Connection": "keep-alive", | ||
"Access-Control-Allow-Origin": "*", | ||
"Access-Control-Allow-Headers": "Cache-Control" | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Potential CORS issues with SSE headers
The SSE endpoint sets Access-Control-Allow-Origin: "*"
and Access-Control-Allow-Headers
directly in the response. This could conflict with the global CORS middleware configured in index.ts
.
Remove the redundant CORS headers since they're already handled by the global middleware:
// Set SSE headers
res.writeHead(200, {
"Content-Type": "text/event-stream",
"Cache-Control": "no-cache",
"Connection": "keep-alive",
- "Access-Control-Allow-Origin": "*",
- "Access-Control-Allow-Headers": "Cache-Control"
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Set SSE headers | |
res.writeHead(200, { | |
"Content-Type": "text/event-stream", | |
"Cache-Control": "no-cache", | |
"Connection": "keep-alive", | |
"Access-Control-Allow-Origin": "*", | |
"Access-Control-Allow-Headers": "Cache-Control" | |
}); | |
// Set SSE headers | |
res.writeHead(200, { | |
"Content-Type": "text/event-stream", | |
"Cache-Control": "no-cache", | |
"Connection": "keep-alive" | |
}); |
🤖 Prompt for AI Agents
In platforms/evoting-api/src/controllers/SigningController.ts around lines
69–76, remove the explicit CORS headers ("Access-Control-Allow-Origin" and
"Access-Control-Allow-Headers") from the SSE response and keep only the
necessary SSE headers ("Content-Type": "text/event-stream", "Cache-Control":
"no-cache", "Connection": "keep-alive"); rely on the global CORS middleware in
index.ts for origin/headers handling and, if the SSE endpoint requires
credentials or special headers, ensure those are configured in the global CORS
middleware rather than setting them here.
// Handle client disconnect | ||
req.on("close", () => { | ||
unsubscribe(); | ||
res.end(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for SSE stream
The SSE endpoint doesn't handle potential errors when writing to the response stream or from the subscription callback.
Add error handling for the SSE stream:
// Handle client disconnect
req.on("close", () => {
unsubscribe();
res.end();
});
+
+// Handle errors
+req.on("error", (error) => {
+ console.error("SSE stream error:", error);
+ unsubscribe();
+ res.end();
+});
+
+// Wrap the subscriber callback with error handling
+const unsubscribe = this.ensureService().subscribeToSession(sessionId, (data) => {
+ try {
+ res.write("data: " + JSON.stringify(data) + "\n\n");
+ } catch (error) {
+ console.error("Error writing SSE data:", error);
+ unsubscribe();
+ res.end();
+ }
+});
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In platforms/evoting-api/src/controllers/SigningController.ts around lines 86 to
90, the SSE connection only listens for req "close" and doesn't handle write
errors or errors raised in the subscription callback; wrap the subscription
callback body in a try/catch and on error call unsubscribe(), log the error and
end the response; before writing check res.writableEnded (or res.writable) and
handle res.write returning false by listening for 'drain' if necessary; also
attach 'error' and 'finish' handlers to both req and res to call unsubscribe(),
log the error, and ensure res.end() is invoked so the stream is cleaned up on
any error or premature termination.
import { Vote } from "./Vote"; | ||
|
||
@Entity("user") | ||
@Entity("users") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Table name change from "user" to "users"
The entity now maps to the "users" table. Ensure that database migrations handle this rename properly if upgrading from a previous version.
🏁 Script executed:
#!/bin/bash
# Check if there are migrations handling the table rename
fd -e ts -e sql migration | xargs rg -l "RENAME TABLE.*user.*TO.*users|ALTER TABLE.*user.*RENAME TO.*users"
# Check for any remaining references to the old "user" table name
rg '"user"' --type ts --glob '!**/User.ts'
Length of output: 862
Add missing migration for renaming “user” → “users”
No existing migration handles the table rename, so upgrading an existing database will fail. Please add a new migration that performs:
- ALTER TABLE "user" RENAME TO "users"
Also, double-check any raw SQL or QueryBuilder calls that might still reference the old table name.
🤖 Prompt for AI Agents
In platforms/evoting-api/src/database/entities/User.ts around line 12, the
entity decorator was changed to @Entity("users") but there is no migration to
rename the existing table, which will break upgrades; create a new migration
that executes ALTER TABLE "user" RENAME TO "users" (and the reverse in down
migration), add it to the migration list so it runs on deploy, and scan the
codebase for any raw SQL strings or QueryBuilder references still using "user"
and update them to "users" (or parameterize via the entity metadata) to ensure
consistency.
private cleanupExpiredSessions(): void { | ||
const now = new Date(); | ||
for (const [sessionId, session] of this.sessions.entries()) { | ||
if (now > session.expiresAt && session.status === "pending") { | ||
session.status = "expired"; | ||
session.updatedAt = now; | ||
this.sessions.set(sessionId, session); | ||
|
||
// Notify subscribers of expiration | ||
this.notifySubscribers(sessionId, { | ||
type: "expired", | ||
status: "expired", | ||
sessionId | ||
}); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Race condition in cleanup: Session status updates not atomic
The cleanup function updates session status but these updates aren't atomic. If a session is being processed while cleanup runs, it could lead to inconsistent state.
Consider adding a lock mechanism or using atomic operations:
private cleanupExpiredSessions(): void {
const now = new Date();
for (const [sessionId, session] of this.sessions.entries()) {
- if (now > session.expiresAt && session.status === "pending") {
+ // Use atomic check-and-set pattern
+ if (now > session.expiresAt && session.status === "pending") {
+ // Check status hasn't changed since we read it
+ const currentSession = this.sessions.get(sessionId);
+ if (currentSession && currentSession.status === "pending") {
session.status = "expired";
session.updatedAt = now;
this.sessions.set(sessionId, session);
// Notify subscribers of expiration
this.notifySubscribers(sessionId, {
type: "expired",
status: "expired",
sessionId
});
+ }
}
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
private cleanupExpiredSessions(): void { | |
const now = new Date(); | |
for (const [sessionId, session] of this.sessions.entries()) { | |
if (now > session.expiresAt && session.status === "pending") { | |
session.status = "expired"; | |
session.updatedAt = now; | |
this.sessions.set(sessionId, session); | |
// Notify subscribers of expiration | |
this.notifySubscribers(sessionId, { | |
type: "expired", | |
status: "expired", | |
sessionId | |
}); | |
} | |
} | |
} | |
private cleanupExpiredSessions(): void { | |
const now = new Date(); | |
for (const [sessionId, session] of this.sessions.entries()) { | |
// Use atomic check-and-set pattern | |
if (now > session.expiresAt && session.status === "pending") { | |
// Check status hasn't changed since we read it | |
const currentSession = this.sessions.get(sessionId); | |
if (currentSession && currentSession.status === "pending") { | |
session.status = "expired"; | |
session.updatedAt = now; | |
this.sessions.set(sessionId, session); | |
// Notify subscribers of expiration | |
this.notifySubscribers(sessionId, { | |
type: "expired", | |
status: "expired", | |
sessionId | |
}); | |
} | |
} | |
} | |
} |
let voteDataToStore; | ||
if (voteData.optionId !== undefined) { | ||
// Normal voting mode | ||
voteDataToStore = { | ||
mode: "normal" as const, | ||
data: [voteData.optionId.toString()] | ||
}; | ||
} else if (voteData.ranks) { | ||
// Ranked choice voting mode - convert to points (50, 35, 15) | ||
const rankData = Object.entries(voteData.ranks).map(([rank, optionIndex]) => { | ||
const rankNum = parseInt(rank); | ||
let points = 0; | ||
if (rankNum === 1) points = 50; | ||
else if (rankNum === 2) points = 35; | ||
else if (rankNum === 3) points = 15; | ||
|
||
return { | ||
option: poll.options[optionIndex], | ||
points: points | ||
}; | ||
}); | ||
voteDataToStore = { | ||
mode: "rank" as const, | ||
data: rankData | ||
}; | ||
} else if (voteData.points) { | ||
// Points-based voting mode | ||
const pointData = Object.entries(voteData.points) | ||
.filter(([optionIndex, points]) => { | ||
const index = parseInt(optionIndex); | ||
return index >= 0 && index < poll.options.length && points > 0; | ||
}) | ||
.map(([optionIndex, points]) => { | ||
const index = parseInt(optionIndex); | ||
return { | ||
option: poll.options[index], | ||
points: points | ||
}; | ||
}); | ||
|
||
voteDataToStore = { | ||
mode: "point" as const, | ||
data: pointData | ||
}; | ||
} else { | ||
throw new Error("Invalid vote data"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Complex voting logic should be extracted into separate methods
The createVote method contains complex branching logic for different voting modes. This makes the method long and harder to test.
Extract the vote data preparation logic into separate methods:
private prepareNormalVoteData(optionId: number): VoteDataByMode {
return {
mode: "normal" as const,
data: [optionId.toString()]
};
}
private prepareRankVoteData(ranks: { [key: number]: number }, pollOptions: string[]): VoteDataByMode {
const rankData = Object.entries(ranks).map(([rank, optionIndex]) => {
const rankNum = parseInt(rank);
let points = 0;
if (rankNum === 1) points = 50;
else if (rankNum === 2) points = 35;
else if (rankNum === 3) points = 15;
return {
option: pollOptions[optionIndex],
points: points
};
});
return {
mode: "rank" as const,
data: rankData
};
}
private preparePointVoteData(points: { [key: number]: number }, pollOptions: string[]): VoteDataByMode {
const pointData = Object.entries(points)
.filter(([optionIndex, points]) => {
const index = parseInt(optionIndex);
return index >= 0 && index < pollOptions.length && points > 0;
})
.map(([optionIndex, points]) => {
const index = parseInt(optionIndex);
return {
option: pollOptions[index],
points: points
};
});
return {
mode: "point" as const,
data: pointData
};
}
Then simplify the main method:
let voteDataToStore;
if (voteData.optionId !== undefined) {
- // Normal voting mode
- voteDataToStore = {
- mode: "normal" as const,
- data: [voteData.optionId.toString()]
- };
+ voteDataToStore = this.prepareNormalVoteData(voteData.optionId);
} else if (voteData.ranks) {
- // ... existing rank logic
+ voteDataToStore = this.prepareRankVoteData(voteData.ranks, poll.options);
} else if (voteData.points) {
- // ... existing point logic
+ voteDataToStore = this.preparePointVoteData(voteData.points, poll.options);
} else {
throw new Error("Invalid vote data");
}
🤖 Prompt for AI Agents
In platforms/evoting-api/src/services/VoteService.ts around lines 53-99, the
createVote method contains complex branching for normal, rank and point voting
modes; extract each branch into its own private helper (prepareNormalVoteData,
prepareRankVoteData, preparePointVoteData) that accept the specific input and
poll.options and return a VoteDataByMode, move the rank-to-points logic
(1→50,2→35,3→15) and the validation/filtering of point entries into those
helpers, then replace the inlined branches with calls to the new methods to
simplify createVote and make each branch unit-testable.
dbPath: path.resolve(process.env.EVOTING_MAPPING_DB_PATH as string), | ||
registryUrl: process.env.PUBLIC_REGISTRY_URL as string, | ||
platform: process.env.PUBLIC_EVOTING_BASE_URL as string, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify environment variables are properly configured
The adapter configuration relies on environment variables that must be properly set. Missing or incorrect values could cause runtime failures.
🏁 Script executed:
#!/bin/bash
# Check if required environment variables are documented and have defaults
# Check for environment variable usage in the codebase
echo "Checking for EVOTING_MAPPING_DB_PATH usage:"
rg "EVOTING_MAPPING_DB_PATH" -A 2 -B 2
echo -e "\nChecking for PUBLIC_REGISTRY_URL usage:"
rg "PUBLIC_REGISTRY_URL" -A 2 -B 2
echo -e "\nChecking for PUBLIC_EVOTING_BASE_URL usage:"
rg "PUBLIC_EVOTING_BASE_URL" -A 2 -B 2
# Check if there's an example env file
echo -e "\nChecking for .env.example or similar:"
fd -e example -e sample | grep -i env
if [ -f ".env.example" ]; then
echo "Content of .env.example:"
cat .env.example | grep -E "(EVOTING_MAPPING_DB_PATH|PUBLIC_REGISTRY_URL|PUBLIC_EVOTING_BASE_URL)"
fi
Length of output: 15313
Ensure all required environment variables are documented and validated
The subscriber.ts
adapter (and related services) assume the presence of three env vars without defaults:
- EVOTING_MAPPING_DB_PATH
- PUBLIC_REGISTRY_URL
- PUBLIC_EVOTING_BASE_URL
Our checks revealed:
.env.example
only listsPUBLIC_REGISTRY_URL
.subscriber.ts
usesprocess.env.EVOTING_MAPPING_DB_PATH as string
andprocess.env.PUBLIC_EVOTING_BASE_URL as string
with no fallback or runtime check.- Other services either throw on a missing
PUBLIC_REGISTRY_URL
or provide defaults for certain vars—but this file does neither.
Please update as follows:
- In
.env.example
, add placeholders for:- EVOTING_MAPPING_DB_PATH=your_mapping_db_path_here
- PUBLIC_EVOTING_BASE_URL=https://your.evoting.base.url
- In
platforms/evoting-api/src/web3adapter/watchers/subscriber.ts
, validate or default each var at startup, for example:const dbPathEnv = process.env.EVOTING_MAPPING_DB_PATH; if (!dbPathEnv) throw new Error('EVOTING_MAPPING_DB_PATH is required'); const registryUrl = process.env.PUBLIC_REGISTRY_URL; if (!registryUrl) throw new Error('PUBLIC_REGISTRY_URL is required'); const platformUrl = process.env.PUBLIC_EVOTING_BASE_URL; if (!platformUrl) throw new Error('PUBLIC_EVOTING_BASE_URL is required'); export const adapter = new Web3Adapter({ schemasPath: path.resolve(__dirname, "../mappings/"), dbPath: path.resolve(dbPathEnv), registryUrl, platform: platformUrl, });
- Optionally, align with other platforms’ patterns (throwing on missing vars or providing sane defaults).
🤖 Prompt for AI Agents
In platforms/evoting-api/src/web3adapter/watchers/subscriber.ts around lines
18-21, the code uses EVOTING_MAPPING_DB_PATH, PUBLIC_REGISTRY_URL, and
PUBLIC_EVOTING_BASE_URL directly from process.env with no validation or
defaults; update .env.example to add
EVOTING_MAPPING_DB_PATH=your_mapping_db_path_here and
PUBLIC_EVOTING_BASE_URL=https://your.evoting.base.url, and modify subscriber.ts
to read each env var into a local const, validate that each required var is
present (throw a clear Error if missing) or supply a sane default where
appropriate, then call path.resolve on the validated db path and pass the
validated registry and platform URLs into the Web3Adapter constructor.
event.metadata.tableName.endsWith("s") | ||
? event.metadata.tableName | ||
: event.metadata.tableName + "s" | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Inconsistent pluralization logic repeated multiple times
The table name pluralization logic is duplicated in multiple places (afterInsert, afterUpdate, afterRemove). This violates DRY principle and could lead to maintenance issues.
Extract the pluralization logic into a helper method:
+ private normalizeTableName(tableName: string): string {
+ return tableName.endsWith("s") ? tableName : tableName + "s";
+ }
+
async afterInsert(event: InsertEvent<any>) {
let entity = event.entity;
if (entity) {
entity = (await this.enrichEntity(
entity,
event.metadata.tableName,
event.metadata.target
)) as ObjectLiteral;
}
- this.handleChange(
- entity ?? event.entity,
- event.metadata.tableName.endsWith("s")
- ? event.metadata.tableName
- : event.metadata.tableName + "s"
- );
+ this.handleChange(
+ entity ?? event.entity,
+ this.normalizeTableName(event.metadata.tableName)
+ );
}
Apply similar changes to afterUpdate
and afterRemove
methods.
Also applies to: 97-100, 108-112
🤖 Prompt for AI Agents
In the file platforms/evoting-api/src/web3adapter/watchers/subscriber.ts around
lines 77 to 80, the code contains duplicated logic for pluralizing table names.
This violates the DRY (Don't Repeat Yourself) principle and could lead to
maintenance issues. To fix this, extract the pluralization logic into a helper
method and use it in the `afterInsert`, `afterUpdate`, and `afterRemove`
methods. Apply similar changes to the duplicated logic at lines 97-100 and
108-112.
if (entity[relation]) { | ||
const relatedEntity = await repository.findOne({ | ||
where: { id: entity[relation].id || entity[relation] }, | ||
relations: this.getRelationsForEntity(relation), | ||
}); | ||
enrichedEntity[relation] = relatedEntity; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug in relation loading: Wrong entity type passed to getRelationsForEntity
On line 199, getRelationsForEntity(relation)
is called with the relation name (e.g., "polls", "votes") instead of the entity type (e.g., "Poll", "Vote"). This will return an empty array since the relation map uses entity type names.
for (const relation of relations) {
if (entity[relation]) {
+ // Determine the entity type from the relation name
+ const relationEntityType = relation.charAt(0).toUpperCase() + relation.slice(1);
+ // Handle plural relations
+ const entityType = relationEntityType.endsWith('s')
+ ? relationEntityType.slice(0, -1)
+ : relationEntityType;
+
const relatedEntity = await repository.findOne({
where: { id: entity[relation].id || entity[relation] },
- relations: this.getRelationsForEntity(relation),
+ relations: this.getRelationsForEntity(entityType),
});
enrichedEntity[relation] = relatedEntity;
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (entity[relation]) { | |
const relatedEntity = await repository.findOne({ | |
where: { id: entity[relation].id || entity[relation] }, | |
relations: this.getRelationsForEntity(relation), | |
}); | |
enrichedEntity[relation] = relatedEntity; | |
} | |
for (const relation of relations) { | |
if (entity[relation]) { | |
+ // Determine the entity type from the relation name | |
+ const relationEntityType = relation.charAt(0).toUpperCase() + relation.slice(1); | |
+ // Handle plural relations | |
+ const entityType = relationEntityType.endsWith('s') | |
+ ? relationEntityType.slice(0, -1) | |
+ : relationEntityType; | |
+ | |
const relatedEntity = await repository.findOne({ | |
where: { id: entity[relation].id || entity[relation] }, | |
- relations: this.getRelationsForEntity(relation), | |
+ relations: this.getRelationsForEntity(entityType), | |
}); | |
enrichedEntity[relation] = relatedEntity; | |
} | |
} |
🤖 Prompt for AI Agents
platforms/evoting-api/src/web3adapter/watchers/subscriber.ts around lines
196-202: getRelationsForEntity is being called with the relation name (e.g.,
"polls") but the relations map is keyed by entity type names, so pass the entity
type instead; obtain the entity type from the repository metadata (e.g.,
repository.metadata.name or repository.metadata.targetName) and call
this.getRelationsForEntity(entityType) when loading the related entity so the
correct relations array is returned.
Description of change
Add w3ds connector, for eVoting app
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Documentation
Chores