Uh oh!
There was an error while loading. Please reload this page.
chore: make applications reject older wallets - #424
Conversation
WalkthroughThis PR implements app version gating for client authentication across multiple platforms. Client apps now send appVersion "0.4.0" in auth payloads; servers validate against MIN_REQUIRED_VERSION and reject older clients with a version_mismatch error. Client UIs handle version errors by displaying user-facing messages and halting login flow. Version utility modules provide semantic version comparison logic. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as Client App<br/>(v0.4.0)
participant Server as Auth Server
participant UI as UI Component
User->>Client: Initiate QR Scan
Client->>Server: POST /login with<br/>appVersion: "0.4.0"
alt Version Valid
Server->>Server: isVersionValid("0.4.0")<br/>returns true
Server->>Client: Send token via SSE
Client->>UI: Receive token event
UI->>UI: Sign in & Navigate
else Version Invalid
Server->>Server: isVersionValid(appVersion)<br/>returns false
Server->>Client: Send version_mismatch error
Client->>UI: Receive error event
UI->>UI: Set errorMessage
UI->>User: Display Error Banner
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (6)
platforms/group-charter-manager-api/src/utils/version.ts (1)
1-32: Same issues as dreamsync-api version utilities.This file is identical to
platforms/dreamsync-api/src/utils/version.ts. Please refer to the comments on that file regarding:
- Missing input validation for malformed version strings
- NaN handling in compareVersions
- Code duplication across services
platforms/group-charter-manager-api/src/controllers/AuthController.ts (2)
5-7: Version validation imports and configuration follow the same pattern.The implementation is consistent with other AuthControllers. See comments on
platforms/dreamsync-api/src/controllers/AuthController.tsregarding:
- Externalizing MIN_REQUIRED_VERSION to environment configuration
- Extracting validation logic to shared middleware
70-82: Version validation logic is correct and consistent.The validation implementation follows the same well-structured pattern as other AuthControllers. See previous comments for suggestions about extracting this to shared middleware.
platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts (1)
5-7: Version validation follows established pattern.The implementation is consistent with other AuthControllers. Refer to comments on
platforms/dreamsync-api/src/controllers/AuthController.tsfor suggestions about:
- Externalizing MIN_REQUIRED_VERSION
- Extracting to shared middleware
Also applies to: 70-82
platforms/evoting-api/src/controllers/AuthController.ts (1)
6-8: Version validation implementation is consistent and correct.This follows the same well-structured pattern as the other AuthControllers. All previous comments regarding externalization of MIN_REQUIRED_VERSION and extracting to shared middleware apply here as well.
Also applies to: 69-81
platforms/evoting-api/src/utils/version.ts (1)
1-32: Identical implementation to other version utilities.This file is identical to
platforms/dreamsync-api/src/utils/version.tsandplatforms/group-charter-manager-api/src/utils/version.ts. All previous comments about input validation, NaN handling, and code duplication apply here.
🧹 Nitpick comments (8)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/scanLogic.ts (1)
265-265: Consider extracting version to a constant.The hardcoded version string "0.4.0" could become a maintenance burden if it needs to be updated across multiple files. Consider extracting it to a constant or reading it from package.json.
// At the top of the file or in a separate config fileconstAPP_VERSION="0.4.0";// Then use it in the payloadconstauthPayload={ename: vault.ename,session: get(session),w3id: w3idResult,signature: signature,appVersion: APP_VERSION,};Alternatively, if the wallet has a package.json, consider reading the version dynamically to keep it in sync with the actual app version.
platforms/eVoting/src/components/auth/login-screen.tsx (1)
40-45: LGTM - Consider adding onopen handler.The version mismatch handling is correct. Optionally, consider adding an
onopenhandler to clear any previous error messages when the connection successfully opens, as done in the blabsy implementation:eventSource.onopen=()=>{setErrorMessage(null);};platforms/pictique-api/src/controllers/AuthController.ts (1)
81-93: LGTM - Good fail-fast version validation.The version check correctly:
- Validates before expensive operations (user lookup)
- Treats missing appVersion as outdated (secure default)
- Provides clear error messages to both SSE listeners and the HTTP response
- Uses the shared utility function for consistency
Consider extracting
MIN_REQUIRED_VERSIONto an environment variable or config file for easier updates across deployments:constMIN_REQUIRED_VERSION=process.env.MIN_WALLET_VERSION||"0.4.0";platforms/dreamsync-api/src/controllers/AuthController.ts (2)
6-8: Consider externalizing MIN_REQUIRED_VERSION to environment configuration.While hardcoding the version is acceptable, moving it to an environment variable or configuration file would allow updating the minimum version requirement without code changes or redeployment.
-const MIN_REQUIRED_VERSION = "0.4.0";+const MIN_REQUIRED_VERSION = process.env.MIN_APP_VERSION || "0.4.0";
69-81: Consider extracting version validation to shared middleware.This version validation logic is duplicated across multiple AuthControllers. Consider extracting it to a reusable middleware function to maintain consistency and reduce duplication.
Example middleware:
// utils/versionMiddleware.tsexportfunctionvalidateAppVersion(appVersion: string|undefined,minVersion: string,session: string,eventEmitter: EventEmitter): {valid: boolean;response?: any}{if(!appVersion||!isVersionValid(appVersion,minVersion)){consterrorMessage={error: true,message: `Your eID Wallet app version is outdated. Please update to version ${minVersion} or later.`,type: "version_mismatch"};eventEmitter.emit(session,errorMessage);return{valid: false,response: {error: "App version too old",message: errorMessage.message}};}return{valid: true};}platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
106-111: Error UI is clear and appropriately styled.The error display is well-positioned and clearly communicates the authentication failure to users. Consider adding a dismiss button or refresh action if users need to retry without a full page refresh.
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (2)
7-20: Recommended: Add input validation for better error handling.Consider validating the version string format before processing to provide clearer error messages:
export function compareVersions(version1: string, version2: string): number { + const versionPattern = /^\d+(\.\d+)*$/;+ if (!versionPattern.test(version1) || !versionPattern.test(version2)) {+ throw new Error('Invalid version format: expected format like "1.2.3"');+ }+ const v1Parts = version1.split('.').map(Number); const v2Parts = version2.split('.').map(Number); -- // Validate that all parts are valid numbers- if (v1Parts.some(isNaN) || v2Parts.some(isNaN)) {- throw new Error('Invalid version format: versions must contain only numeric segments');- } for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { const v1Part = v1Parts[i] || 0; const v2Part = v2Parts[i] || 0; if (v1Part < v2Part) return -1; if (v1Part > v2Part) return 1; } return 0; }This provides more specific error messages and validates format upfront, making the NaN check unnecessary.
7-20: Optional: Consider full semantic versioning support for future needs.The current implementation handles only numeric segments (
major.minor.patch). If you later need to support pre-release versions (e.g.,"1.0.0-alpha") or build metadata (e.g.,"1.0.0+20130313144700"), consider using thesemvernpm package for full SemVer 2.0 compliance. For now, the simple implementation is adequate for the app version gating use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/scanLogic.ts(1 hunks)platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts(2 hunks)platforms/blabsy-w3ds-auth-api/src/utils/version.ts(1 hunks)platforms/blabsy/src/components/common/maintenance-banner.tsx(2 hunks)platforms/blabsy/src/components/login/login-main.tsx(3 hunks)platforms/dreamSync/client/src/components/auth/login-screen.tsx(3 hunks)platforms/dreamsync-api/src/controllers/AuthController.ts(3 hunks)platforms/dreamsync-api/src/utils/version.ts(1 hunks)platforms/eVoting/package.json(1 hunks)platforms/eVoting/src/components/auth/login-screen.tsx(3 hunks)platforms/evoting-api/src/controllers/AuthController.ts(2 hunks)platforms/evoting-api/src/utils/version.ts(1 hunks)platforms/group-charter-manager-api/src/controllers/AuthController.ts(2 hunks)platforms/group-charter-manager-api/src/utils/version.ts(1 hunks)platforms/group-charter-manager/src/components/auth/login-screen.tsx(3 hunks)platforms/pictique-api/src/controllers/AuthController.ts(2 hunks)platforms/pictique-api/src/utils/version.ts(1 hunks)platforms/pictique/src/routes/(auth)/auth/+page.svelte(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
platforms/evoting-api/src/utils/version.ts (4)
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/dreamsync-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/group-charter-manager-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/pictique-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (4)
platforms/dreamsync-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/evoting-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/group-charter-manager-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/pictique-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)
platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts (1)
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (1)
isVersionValid(28-30)
platforms/pictique-api/src/utils/version.ts (4)
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/dreamsync-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/evoting-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/group-charter-manager-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)
platforms/dreamsync-api/src/utils/version.ts (4)
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/evoting-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/group-charter-manager-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/pictique-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)
platforms/group-charter-manager/src/components/auth/login-screen.tsx (1)
platforms/eVoting/src/lib/authUtils.ts (2)
setAuthId(18-20)setAuthToken(5-7)
platforms/pictique-api/src/controllers/AuthController.ts (1)
platforms/pictique-api/src/utils/version.ts (1)
isVersionValid(28-30)
platforms/group-charter-manager-api/src/controllers/AuthController.ts (1)
platforms/group-charter-manager-api/src/utils/version.ts (1)
isVersionValid(28-30)
platforms/evoting-api/src/controllers/AuthController.ts (1)
platforms/evoting-api/src/utils/version.ts (1)
isVersionValid(28-30)
platforms/dreamsync-api/src/controllers/AuthController.ts (1)
platforms/dreamsync-api/src/utils/version.ts (1)
isVersionValid(28-30)
platforms/group-charter-manager-api/src/utils/version.ts (4)
platforms/blabsy-w3ds-auth-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/dreamsync-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/evoting-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)platforms/pictique-api/src/utils/version.ts (2)
compareVersions(7-20)isVersionValid(28-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
platforms/blabsy/src/components/common/maintenance-banner.tsx (3)
1-88: This file doesn't relate to the PR objective.The PR is titled "make applications reject older wallets" and the summary describes app version gating for client authentication. However, this file only implements timeout and error handling improvements for the maintenance banner MOTD fetch, with no version checking or wallet rejection logic.
21-23: Good addition of timeout.The 5-second timeout is appropriate for a non-critical MOTD fetch and prevents the request from hanging indefinitely if the registry service is unresponsive.
31-46: Error handling logic is sound.The enhanced error handling correctly distinguishes between network failures (which are silently handled since the registry may be unavailable) and other errors like HTTP status errors, which are logged for debugging. The string comparison
error.message === 'Network Error'is a common pattern for backward compatibility with different Axios versions.platforms/dreamSync/client/src/components/auth/login-screen.tsx (2)
50-55: LGTM - Clean version mismatch handling.The version error handling correctly:
- Checks for the specific error type
- Sets a user-friendly message with a fallback
- Closes the EventSource to prevent further processing
- Returns early to avoid executing the success path
121-126: LGTM - Clear error UI feedback.The error banner provides clear visual feedback to users when authentication fails due to version mismatch, with appropriate styling and structure.
platforms/blabsy/src/components/login/login-main.tsx (2)
21-24: Good practice - Clear error on reconnection.Clearing the error message when the SSE connection opens successfully ensures users see current status and don't get confused by stale error messages.
34-42: LGTM - Consistent version mismatch handling.The error handling follows the same pattern as other login components in the PR, ensuring a consistent user experience across platforms.
platforms/group-charter-manager/src/components/auth/login-screen.tsx (1)
53-76: LGTM - Comprehensive error and success handling.The authentication flow correctly:
- Checks for version mismatch errors first with early return
- Only processes successful authentication when both user and token exist
- Clears error message on connection open
- Properly closes EventSource before page reload
The implementation is consistent with the version gating pattern across the codebase.
platforms/dreamsync-api/src/utils/version.ts (1)
28-30: Function logic is correct.The implementation correctly delegates to
compareVersions. However, it will inherit any input validation issues from that function.platforms/dreamsync-api/src/controllers/AuthController.ts (1)
69-81: Version validation logic is well-implemented.The validation correctly treats missing
appVersionas invalid, provides user-friendly error messages, and notifies both SSE clients and the HTTP response. Thetype: "version_mismatch"field enables proper client-side error handling.platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
57-64: Version mismatch error handling is well-implemented.The code properly detects version mismatch errors, sets user-facing error messages with a fallback, closes the SSE connection, and halts further processing. This aligns well with the server-side version validation.
| const { ename, session } = req.body; | ||
| const { ename, session, appVersion } = req.body; | ||
| console.log(req.body) |
There was a problem hiding this comment.
Remove debug console.log statement.
This debug logging statement should be removed before merging to production. Logging the entire request body can expose sensitive data (ename, session tokens, etc.) in production logs.
- console.log(req.body)-
if (!ename) {📝 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.
| console.log(req.body) | |
| if(!ename){ |
🤖 Prompt for AI Agents
In platforms/blabsy-w3ds-auth-api/src/controllers/AuthController.ts around line
60, remove the debug console.log(req.body) statement; if you need request
inspection keep it out of production by replacing it with a proper logger call
that only logs non-sensitive fields or use logger.debug behind an environment
check (e.g., log only when NODE_ENV !== 'production' or redact tokens/PII) so no
full request bodies are written to production logs.
| export function compareVersions(version1: string, version2: string): number { | ||
| const v1Parts = version1.split('.').map(Number); | ||
| const v2Parts = version2.split('.').map(Number); | ||
| for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { | ||
| const v1Part = v1Parts[i] || 0; | ||
| const v2Part = v2Parts[i] || 0; | ||
| if (v1Part < v2Part) return -1; | ||
| if (v1Part > v2Part) return 1; | ||
| } | ||
| return 0; | ||
| } | ||
| /** | ||
| * Checks if the app version meets the minimum required version | ||
| * @param appVersion - The version from the app (e.g., "0.4.0") | ||
| * @param minVersion - The minimum required version (e.g., "0.4.0") | ||
| * @returns true if appVersion >= minVersion, false otherwise | ||
| */ | ||
| export function isVersionValid(appVersion: string, minVersion: string): boolean { | ||
| return compareVersions(appVersion, minVersion) >= 0; | ||
| } |
There was a problem hiding this comment.
Major: Consolidate duplicated version utilities into a shared module.
This exact implementation is duplicated across 5 platform APIs (dreamsync-api, evoting-api, group-charter-manager-api, pictique-api, and blabsy-w3ds-auth-api). This violates DRY principles and creates maintenance burden—any bug fix or enhancement must be applied to all 5 copies.
Consider extracting these utilities into a shared package (e.g., @project/version-utils or a shared/utils module) that all platform APIs can import.
| export function compareVersions(version1: string, version2: string): number { | ||
| const v1Parts = version1.split('.').map(Number); | ||
| const v2Parts = version2.split('.').map(Number); | ||
| for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { | ||
| const v1Part = v1Parts[i] || 0; | ||
| const v2Part = v2Parts[i] || 0; | ||
| if (v1Part < v2Part) return -1; | ||
| if (v1Part > v2Part) return 1; | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Critical: Handle invalid version strings to prevent NaN comparison bugs.
The function uses .map(Number) without validating that version strings contain only numeric segments. If a version contains non-numeric parts (e.g., "1.2.a", "abc"), Number() returns NaN, and all comparisons with NaN evaluate to false. This causes the function to incorrectly return 0 (versions equal), which could allow malformed version strings to bypass validation in the authentication flow.
Apply this diff to add validation:
export function compareVersions(version1: string, version2: string): number {
const v1Parts = version1.split('.').map(Number);
const v2Parts = version2.split('.').map(Number);
++ // Validate that all parts are valid numbers+ if (v1Parts.some(isNaN) || v2Parts.some(isNaN)) {+ throw new Error('Invalid version format: versions must contain only numeric segments');+ }
for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) {
const v1Part = v1Parts[i] || 0;
const v2Part = v2Parts[i] || 0;
if (v1Part < v2Part) return -1;
if (v1Part > v2Part) return 1;
}
return 0;
}🤖 Prompt for AI Agents
In platforms/blabsy-w3ds-auth-api/src/utils/version.ts around lines 7 to 20, the
compareVersions implementation maps segments to Number without validating, which
yields NaN for non-numeric segments and makes comparisons always false; update
the function to first trim the input, split on '.', validate each segment with a
strict numeric check (e.g., /^\d+$/) and reject invalid version strings by
throwing a descriptive error (or returning a distinct error code) before
performing numeric comparisons, then parse segments with parseInt and proceed
with the existing numeric comparison logic.
| /** | ||
| * Compares two semantic version strings | ||
| * @param version1 - First version string (e.g., "0.4.0") | ||
| * @param version2 - Second version string (e.g., "0.3.0") | ||
| * @returns -1 if version1 < version2, 0 if equal, 1 if version1 > version2 | ||
| */ | ||
| export function compareVersions(version1: string, version2: string): number { | ||
| const v1Parts = version1.split('.').map(Number); | ||
| const v2Parts = version2.split('.').map(Number); | ||
| for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { | ||
| const v1Part = v1Parts[i] || 0; | ||
| const v2Part = v2Parts[i] || 0; | ||
| if (v1Part < v2Part) return -1; | ||
| if (v1Part > v2Part) return 1; | ||
| } | ||
| return 0; | ||
| } | ||
| /** | ||
| * Checks if the app version meets the minimum required version | ||
| * @param appVersion - The version from the app (e.g., "0.4.0") | ||
| * @param minVersion - The minimum required version (e.g., "0.4.0") | ||
| * @returns true if appVersion >= minVersion, false otherwise | ||
| */ | ||
| export function isVersionValid(appVersion: string, minVersion: string): boolean { | ||
| return compareVersions(appVersion, minVersion) >= 0; | ||
| } | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated version utilities to a shared package.
This exact implementation is duplicated across at least 5 services in the codebase. In a monorepo, shared utilities should be extracted to a common package (e.g., packages/shared-utils or packages/version-utils) and imported by each service.
Consider creating a shared package structure:
// packages/shared-utils/src/version.tsexportfunctioncompareVersions(version1: string,version2: string): number{// implementation}exportfunctionisVersionValid(appVersion: string,minVersion: string): boolean{// implementation}Then import in each service:
import{isVersionValid}from'@metastate/shared-utils/version';🤖 Prompt for AI Agents
platforms/dreamsync-api/src/utils/version.ts lines 1-32: this version utility is
duplicated across services and should be extracted to a shared package; create a
new workspace package (e.g., packages/shared-utils or packages/version-utils),
move the compareVersions and isVersionValid implementations into that package
and export them, update the monorepo package.json/workspace config to include
the new package, replace the local functions here with an import from the new
package (e.g., import { isVersionValid } from '@your-org/shared-utils/version'),
remove the duplicate copies from other services or update their imports to the
shared package, run installs/build and update any tests or type imports to
ensure the new package compiles and all services reference the single shared
implementation.
| export function compareVersions(version1: string, version2: string): number { | ||
| const v1Parts = version1.split('.').map(Number); | ||
| const v2Parts = version2.split('.').map(Number); | ||
| for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { | ||
| const v1Part = v1Parts[i] || 0; | ||
| const v2Part = v2Parts[i] || 0; | ||
| if (v1Part < v2Part) return -1; | ||
| if (v1Part > v2Part) return 1; | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Add input validation and handle malformed version strings.
The function doesn't validate inputs or handle non-numeric segments. When split('.').map(Number) encounters non-numeric segments (e.g., "1.a.0"), it produces NaN. Since NaN < x and NaN > x both return false, the function incorrectly returns 0 (equal) for malformed versions.
Apply this diff to add validation:
export function compareVersions(version1: string, version2: string): number {
+ if (!version1 || !version2) {+ throw new Error('Version strings cannot be null or empty');+ }+
const v1Parts = version1.split('.').map(Number);
const v2Parts = version2.split('.').map(Number);
++ if (v1Parts.some(isNaN) || v2Parts.some(isNaN)) {+ throw new Error('Version strings must contain only numeric segments');+ }
for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) {🤖 Prompt for AI Agents
In platforms/dreamsync-api/src/utils/version.ts around lines 7 to 20, the
compareVersions function currently maps segments with Number and fails silently
on non-numeric parts (yielding NaN and incorrect equality). Change the
implementation to validate inputs: ensure both inputs are non-empty strings,
split on '.', parse each segment with parseInt(segment, 10), and if any parsed
segment is NaN or not a valid integer (or contains non-digit characters after
trimming) throw a clear Error (or return a distinct value) indicating a
malformed version string; then compare numeric segments with missing parts
treated as 0 as before. Ensure you add unit tests or callers handle the thrown
error.
| "lucide-react": "^0.453.0", | ||
| "next": "15.4.2", | ||
| "next-qrcode": "^2.5.1", | ||
| "next.js": "^1.0.3", |
There was a problem hiding this comment.
Remove incorrect dependency - wrong package name.
The dependency "next.js" is NOT the official Next.js package. The official package is "next" (which is already present on line 47). The "next.js" package on npm is an outdated, unofficial package that was last published 10 years ago.
Apply this diff to remove the incorrect dependency:
"next": "15.4.2",
"next-qrcode": "^2.5.1",
- "next.js": "^1.0.3",
"qrcode.react": "^4.2.0",📝 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.
| "next.js": "^1.0.3", | |
| "next": "15.4.2", | |
| "next-qrcode": "^2.5.1", | |
| "qrcode.react": "^4.2.0", |
🤖 Prompt for AI Agents
In platforms/eVoting/package.json around line 49, remove the incorrect
dependency "next.js": "^1.0.3" because the official Next.js package is "next"
(already present on line 47); update package.json by deleting the "next.js"
entry from the dependencies block and run a quick npm/yarn install to ensure
lockfile and node_modules are consistent.
| export function compareVersions(version1: string, version2: string): number { | ||
| const v1Parts = version1.split('.').map(Number); | ||
| const v2Parts = version2.split('.').map(Number); | ||
| for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) { | ||
| const v1Part = v1Parts[i] || 0; | ||
| const v2Part = v2Parts[i] || 0; | ||
| if (v1Part < v2Part) return -1; | ||
| if (v1Part > v2Part) return 1; | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Add input validation and consider edge cases.
The compareVersions function lacks input validation and has limited semantic versioning support:
- No validation: Invalid inputs like empty strings, non-numeric segments, or malformed versions will cause issues.
split('.').map(Number)will produceNaNfor non-numeric parts. - Limited semver: Doesn't handle pre-release versions (
1.0.0-alpha) or build metadata (1.0.0+build123). - Code duplication: This exact implementation is duplicated across multiple platform APIs (blabsy, dreamsync, evoting, group-charter-manager, pictique). Consider extracting to a shared utility package.
Apply this diff to add basic validation:
export function compareVersions(version1: string, version2: string): number {
+ if (!version1 || !version2) {+ throw new Error('Version strings cannot be empty');+ }+
const v1Parts = version1.split('.').map(Number);
const v2Parts = version2.split('.').map(Number);
++ // Check for invalid version parts (NaN)+ if (v1Parts.some(isNaN) || v2Parts.some(isNaN)) {+ throw new Error('Version strings must contain only numeric parts separated by dots');+ }
for (let i = 0; i < Math.max(v1Parts.length, v2Parts.length); i++) {For the code duplication, consider creating a shared @metastate/version-utils package that all platforms can import.
📝 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.
| exportfunctioncompareVersions(version1: string,version2: string): number{ | |
| constv1Parts=version1.split('.').map(Number); | |
| constv2Parts=version2.split('.').map(Number); | |
| for(leti=0;i<Math.max(v1Parts.length,v2Parts.length);i++){ | |
| constv1Part=v1Parts[i]||0; | |
| constv2Part=v2Parts[i]||0; | |
| if(v1Part<v2Part)return-1; | |
| if(v1Part>v2Part)return1; | |
| } | |
| return0; | |
| } | |
| exportfunctioncompareVersions(version1: string,version2: string): number{ | |
| if(!version1||!version2){ | |
| thrownewError('Version strings cannot be empty'); | |
| } | |
| constv1Parts=version1.split('.').map(Number); | |
| constv2Parts=version2.split('.').map(Number); | |
| // Check for invalid version parts (NaN) | |
| if(v1Parts.some(isNaN)||v2Parts.some(isNaN)){ | |
| thrownewError('Version strings must contain only numeric parts separated by dots'); | |
| } | |
| for(leti=0;i<Math.max(v1Parts.length,v2Parts.length);i++){ | |
| constv1Part=v1Parts[i]||0; | |
| constv2Part=v2Parts[i]||0; | |
| if(v1Part<v2Part)return-1; | |
| if(v1Part>v2Part)return1; | |
| } | |
| return0; | |
| } |
🤖 Prompt for AI Agents
In platforms/pictique-api/src/utils/version.ts around lines 7 to 20, the
compareVersions implementation lacks input validation, mishandles non-numeric
segments, and doesn't account for basic semantic-version pre-release/build
rules; update it to first validate both inputs (non-empty strings), trim them,
and split off build metadata (+...) to ignore it, then split a core numeric
version and optional pre-release (by '-') and ensure all numeric core segments
parse to finite numbers (throw or return an error on invalid input), compare
core numeric segments as before, and if cores are equal handle pre-release
precedence (a version without a pre-release has higher precedence than one with;
otherwise compare prerelease identifiers by numeric where possible, or
lexicographically), and finally factor this logic into a shared package (e.g.,
@metastate/version-utils) to remove duplication across platforms so each API
imports the common utility.
| function getAppStoreLink(): string { | ||
| const userAgent = | ||
| navigator.userAgent || navigator.vendor || (window as { opera?: string }).opera || ''; | ||
| if (/android/i.test(userAgent)) { | ||
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | ||
| } | ||
| onMount(async () => { | ||
| getAppStoreLink = () => { | ||
| const userAgent = | ||
| navigator.userAgent || | ||
| navigator.vendor || | ||
| (window as { opera?: string }).opera || | ||
| ''; | ||
| if (/android/i.test(userAgent)) { | ||
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | ||
| } | ||
| if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) { | ||
| return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667'; | ||
| } | ||
| if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) { | ||
| return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667'; | ||
| } | ||
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | ||
| }; | ||
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Potential duplication with mobile-detection utilities.
The platform detection logic here may duplicate functionality already available in the imported mobile-detection utils (line 11). The store URLs are also hardcoded but should use the imported environment variables (PUBLIC_APP_STORE_EID_WALLET, PUBLIC_PLAY_STORE_EID_WALLET from lines 4-6).
function getAppStoreLink(): string {
- const userAgent =- navigator.userAgent || navigator.vendor || (window as { opera?: string }).opera || '';- if (/android/i.test(userAgent)) {- return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet';- }-- if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) {- return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667';- }-- return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet';+ const userAgent =+ navigator.userAgent || navigator.vendor || (window as { opera?: string }).opera || '';+ if (/android/i.test(userAgent)) {+ return PUBLIC_PLAY_STORE_EID_WALLET;+ }+ if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) {+ return PUBLIC_APP_STORE_EID_WALLET;+ }+ return PUBLIC_PLAY_STORE_EID_WALLET;
}📝 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.
| function getAppStoreLink(): string { | |
| const userAgent = | |
| navigator.userAgent || navigator.vendor || (window as { opera?:string }).opera || ''; | |
| if (/android/i.test(userAgent)) { | |
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | |
| } | |
| onMount(async () => { | |
| getAppStoreLink = () => { | |
| const userAgent = | |
| navigator.userAgent || | |
| navigator.vendor || | |
| (window as { opera?:string }).opera || | |
| ''; | |
| if (/android/i.test(userAgent)) { | |
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | |
| } | |
| if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) { | |
| return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667'; | |
| } | |
| if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) { | |
| return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667'; | |
| } | |
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | |
| }; | |
| return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; | |
| } | |
| function getAppStoreLink(): string { | |
| const userAgent = | |
| navigator.userAgent || navigator.vendor || (window as { opera?:string }).opera || ''; | |
| if (/android/i.test(userAgent)) { | |
| return PUBLIC_PLAY_STORE_EID_WALLET; | |
| } | |
| if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) { | |
| return PUBLIC_APP_STORE_EID_WALLET; | |
| } | |
| return PUBLIC_PLAY_STORE_EID_WALLET; | |
| } |
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
Release Notes