feat: improve languages support #295
Conversation
WalkthroughThe changes in this pull request involve extensive updates across multiple Vue components and localization files to enhance internationalization (i18n) support. Key modifications include replacing static text in form fields, headers, and UI elements with dynamic translation functions. This ensures that user-facing text can be translated based on the user's language preferences. The overall structure and logic of the components remain unchanged, focusing solely on improving the localization capabilities of the application. Changes
Possibly related PRs
Suggested reviewers
Security Recommendations
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 13
🧹 Outside diff range and nitpick comments (16)
frontend/pages/home/statistics.ts (1)
Line range hint
13-17: Add error handling for API failures.The statistics fetching should handle API errors gracefully to prevent exposing sensitive information in error messages.
const { data: statistics } = useAsyncData(async () => { - const { data } = await api.stats.group(); - return data; + try { + const { data } = await api.stats.group(); + return data; + } catch (error) { + console.error('Failed to fetch statistics:', error); + return null; + } });frontend/components/Location/Selector.vue (2)
Line range hint
28-32: Security: Strengthen type safety in cast functionThe current implementation of the
castfunction bypasses TypeScript's type checking withany, which could lead to runtime errors if the data structure doesn't match expectations. This is a potential security concern as it might allow malformed data to propagate through the application.Consider implementing runtime type checking:
function cast(value: any): FlatTreeItem { + // Add runtime type validation + if (!value || typeof value.name !== 'string' || typeof value.treeString !== 'string') { + throw new Error('Invalid FlatTreeItem structure'); + } return value as FlatTreeItem; }
Based on the findings, I can now provide a final response about the security concerns.
Security: Location data appears to be properly typed but requires additional XSS protection
The investigation reveals:
- Location data is properly typed through TypeScript interfaces (
LocationOut,LocationSummary, etc.)- The
castfunction is used to enforce type safety by casting toFlatTreeItem- While type safety is in place, there's no explicit HTML sanitization for the displayed values
Recommendations:
- Add HTML escaping for
nameandtreeStringvalues before display:{{ htmlEscape(cast(item.value).name) }} {{ htmlEscape(cast(item.value).treeString) }}
- Consider using Vue's built-in
v-textdirective instead of mustache syntax to automatically escape HTML- Add input validation at the API level to reject malicious input containing HTML/script tags
🔗 Analysis chain
Line range hint
11-14: Security: Consider implementing XSS protection and data sanitizationThe direct rendering of location data (
nameandtreeString) in the template could be vulnerable to XSS attacks if the data isn't properly sanitized. Additionally, the location hierarchy might expose sensitive information.Recommendations:
- Implement input sanitization for location names and tree strings
- Consider adding a content security policy (CSP)
- Review if the full location hierarchy needs to be exposed to all users
Let's check if there's any input sanitization in place:
Also applies to: 16-18
Let me try different commands to verify the security concerns.
This will help us understand:
- If there's any sanitization in place
- If CSP is implemented
- How the cast function handles data
- How location data is processed before display
Let me check how the location data is processed before being displayed.
This will help us understand:
- The structure of location data
- Any validation or processing before display
- How location data is fetched and handled
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for sanitization implementations rg -l "sanitize|escape|purify" --type vue # Check for CSP implementations rg -l "content-security-policy" --type vue --type htmlLength of output: 154
Script:
#!/bin/bash # Search for sanitization implementations with correct file extension rg -l "sanitize|escape|purify" -g "*.vue" # Search for CSP implementations with correct file extension rg -l "content-security-policy" -g "*.vue" -g "*.html" # Check the cast function implementation rg -A 5 "function cast" -g "*.vue" -g "*.ts" # Look for location data handling rg -A 5 "item\.value" -g "*.vue"Length of output: 18238
Script:
#!/bin/bash # Check FlatTreeItem type definition rg -A 5 "type FlatTreeItem" -g "*.ts" -g "*.vue" # Check if there's any data processing/validation before display rg -A 5 "interface Location|type Location" -g "*.ts" # Look for any API response processing rg -A 5 "useQuery.*location" -g "*.vue"Length of output: 2783
frontend/pages/locations.vue (3)
Line range hint
24-35: Security: Add validation for URL query parameters.The direct parsing of URL query parameters using
JSON.parsewithout validation could lead to XSS vulnerabilities if malicious JSON is injected. Consider adding type validation and sanitization.Apply this diff to add validation:
onMounted(() => { const query = route.currentRoute.value.query; if (query && query[locationTreeId]) { console.debug("setting tree state from query params"); - const data = JSON.parse(query[locationTreeId] as string); + try { + const data = JSON.parse(query[locationTreeId] as string); + // Validate the parsed data is an object with boolean values + if (typeof data === 'object' && data !== null) { + const isValid = Object.values(data).every(v => typeof v === 'boolean'); + if (!isValid) { + console.warn('Invalid tree state data in URL'); + return; + } + for (const key in data) { + treeState.value[key] = data[key]; + } + } + } catch (e) { + console.warn('Failed to parse tree state from URL:', e); + } - for (const key in data) { - treeState.value[key] = data[key]; - } } });
Line range hint
37-45: Security: Sanitize tree state before URL serialization.When pushing state to URL, ensure the data structure contains only safe values to prevent potential XSS via URL manipulation.
Apply this diff to add sanitization:
watch( treeState, () => { + // Create a sanitized copy with only boolean values + const sanitizedState = Object.fromEntries( + Object.entries(treeState.value).filter(([_, v]) => typeof v === 'boolean') + ); // Push the current state to the URL - route.replace({ query: { [locationTreeId]: JSON.stringify(treeState.value) } }); + route.replace({ query: { [locationTreeId]: JSON.stringify(sanitizedState) } }); }, { deep: true } );
🛡️ Security Enhancement: Rate Limiting Implementation Recommended
Based on the codebase analysis, there appears to be no server-side rate limiting implemented for the locations API endpoint. To enhance security and prevent potential DoS attacks, consider implementing the following:
- Add a rate limiting middleware to restrict the number of requests per client
- Implement token bucket or sliding window rate limiting algorithm
- Configure appropriate limits based on your application's requirements:
- Suggested starting point: 100 requests per minute per IP
- Add burst allowance for legitimate high-frequency access
- Consider using Redis or similar for distributed rate limiting if running multiple instances
Example implementation using a popular rate limiting library:
import rateLimit from 'express-rate-limit' const apiLimiter = rateLimit({ windowMs: 60 * 1000, // 1 minute max: 100, // limit each IP to 100 requests per windowMs message: 'Too many requests from this IP, please try again later' }); app.use('/api/locations', apiLimiter);🔗 Analysis chain
Line range hint
13-22: Security: Consider adding rate limiting for API calls.While the error handling is good, consider implementing rate limiting for the location tree API endpoint to prevent potential DoS attacks.
Let's check if rate limiting is implemented in the API:
Let me search for more context about the API implementation and rate limiting.
The results will help us determine if there's server-side rate limiting in place for the API endpoints.
Let me check for any API-related configurations or server setup files that might contain rate limiting settings.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for rate limiting middleware or configurations rg -i "rate.*limit|throttle" --type tsLength of output: 396
Script:
#!/bin/bash # Search for API middleware and server-side rate limiting rg -i "middleware.*rate|rate.*middleware|limiter" -A 5 # Look for any express-rate-limit or similar implementations rg -i "express.*rate|rate.*limit.*middleware" -A 5 # Check for API route definitions ast-grep --pattern 'router.$_(locations.$_)'Length of output: 654
Script:
#!/bin/bash # Look for API configuration files fd -e ts -e js "api|server|config" --type f --exec rg -l "express|fastify|koa|nest" # Check for security-related configurations rg -i "security|protect|limit" -g "*.{ts,js}" --type-add "config:*.{config,conf}*" -t config # Look for backend route handlers for locations ast-grep --pattern 'class LocationsController { $$$ }'Length of output: 339
frontend/plugins/i18n.ts (1)
Line range hint
35-42: LGTM! Type inference provides good safety.The removal of explicit Object type in favor of type inference with
Record<string, any>is a good change. However, there are some security considerations for message handling:Consider implementing these security measures:
- Sanitize loaded messages to prevent XSS attacks
- Add content security policy headers to restrict loading of external resources
- Implement message validation before compilation to prevent template injection
Example validation:
function validateMessage(message: any): boolean { // Ensure message is a string and doesn't contain suspicious patterns if (typeof message !== 'string') return false; const suspicious = /(<script|javascript:|data:)/i; return !suspicious.test(message); } export const messages = () => { const messages: Record<string, any> = {}; const modules = import.meta.glob("~//locales/**.json", { eager: true }); for (const path in modules) { const key = path.slice(9, -5); const message = modules[path]; if (validateMessage(message)) { messages[key] = message; } else { console.error(`Invalid message format in ${key}`); } } return messages; };frontend/components/global/DetailsSection/DetailsSection.vue (2)
Line range hint
15-19: Add security headers for external links.When opening external links with
target="_blank", addrel="noopener noreferrer"to prevent potential security vulnerabilities:- <a class="btn btn-primary btn-xs" :href="detail.href" target="_blank"> + <a class="btn btn-primary btn-xs" :href="detail.href" target="_blank" rel="noopener noreferrer">
Line range hint
27-36: Review data exposure in copy functionality.The copy feature could potentially expose sensitive information. Consider:
- Adding a whitelist of copyable fields
- Implementing data masking for sensitive content
- Adding audit logging for copy actions
frontend/components/Label/CreateModal.vue (1)
Line range hint
89-89: Validate navigation parametersWhile the ID comes from the API response, consider adding type validation before using it in navigation to prevent potential path traversal.
Consider adding:
- navigateTo(`/label/${data.id}`); + if (typeof data.id === 'string' || typeof data.id === 'number') { + navigateTo(`/label/${data.id}`); + }frontend/components/Location/CreateModal.vue (2)
Line range hint
4-16: Consider adding input sanitization for location data.While Vue provides basic XSS protection, consider adding additional sanitization for the location name and description before submission.
[security]Add sanitization before form submission:
<form @submit.prevent="create()"> <FormTextField ref="locationNameRef" - v-model="form.name" + v-model="sanitizeInput(form.name)" :trigger-focus="focused" :autofocus="true" :required="true" :label="$t('components.location.create_modal.location_name')" :max-length="255" :min-length="1" /> <FormTextArea - v-model="form.description" + v-model="sanitizeInput(form.description)" :label="$t('components.location.create_modal.location_description')" :max-length="1000" />Add the sanitization function to the script:
function sanitizeInput(input: string): string { return input.replace(/[<>]/g, ''); // Basic HTML tag prevention }
Line range hint
89-108: Add rate limiting to location creation.The create function should implement rate limiting to prevent abuse.
[security]Add basic rate limiting:
+ const rateLimiter = { + lastCall: 0, + minInterval: 1000, // 1 second + }; async function create(close = true) { + const now = Date.now(); + if (now - rateLimiter.lastCall < rateLimiter.minInterval) { + toast.error("Please wait before creating another location"); + return; + } if (loading.value) { toast.error("Already creating a location"); return; } loading.value = true; + rateLimiter.lastCall = now;frontend/components/Item/CreateModal.vue (1)
Based on the codebase analysis, I can now provide a comprehensive security recommendation.
🔒 Implement comprehensive file upload security measures
The current implementation lacks essential security validations for file uploads. Here's a secure implementation that addresses the concerns:
<input id="photo" class="hidden" type="file" accept="image/png,image/jpeg,image/gif,image/avif,image/webp" + @change="validateAndPreviewImage" @change="previewImage" />+ const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + const VALID_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif', 'image/avif', 'image/webp']; - function previewImage(event: Event) { + async function validateAndPreviewImage(event: Event) { const input = event.target as HTMLInputElement; if (input.files && input.files.length > 0) { const file = input.files[0]; + + // Size validation + if (file.size > MAX_FILE_SIZE) { + toast.error("File size must be less than 5MB"); + input.value = ''; // Clear the input + return; + } + + // MIME type validation + if (!VALID_MIME_TYPES.includes(file.type)) { + toast.error("Invalid file type"); + input.value = ''; // Clear the input + return; + } + + // Additional security check: Verify file header + try { + const arrayBuffer = await file.slice(0, 4).arrayBuffer(); + const header = new Uint8Array(arrayBuffer); + const isValidImage = validateImageHeader(header); + if (!isValidImage) { + toast.error("Invalid image file"); + input.value = ''; + return; + } + } catch (e) { + toast.error("Error validating file"); + input.value = ''; + return; + } const reader = new FileReader(); + reader.onerror = () => { + toast.error("Error reading file"); + input.value = ''; + }; reader.onload = e => { form.preview = e.target?.result as string; + form.photo = file; }; - const file = input.files[0]; - form.photo = file; reader.readAsDataURL(file); } } + function validateImageHeader(header: Uint8Array): boolean { + // Common image format signatures + const signatures = { + jpeg: [0xFF, 0xD8, 0xFF], + png: [0x89, 0x50, 0x4E, 0x47], + gif: [0x47, 0x49, 0x46, 0x38] + }; + + return ( + signatures.jpeg.every((byte, i) => header[i] === byte) || + signatures.png.every((byte, i) => header[i] === byte) || + signatures.gif.every((byte, i) => header[i] === byte) + ); + }Security enhancements:
- File size limit (5MB) to prevent memory exhaustion
- MIME type validation using both file extension and content type
- Image header validation to prevent malicious file uploads
- Proper error handling and user feedback
- Input clearing on validation failures
- Asynchronous file processing with error handling
🔗 Analysis chain
Line range hint
22-28: Add security measures for file upload handling.The file upload implementation could benefit from additional security measures:
- Add file size validation
- Implement proper content-type validation beyond extension checking
- Consider adding client-side image compression
Consider implementing these security enhancements:
<input id="photo" class="hidden" type="file" accept="image/png,image/jpeg,image/gif,image/avif,image/webp" + :max-size="5242880" @change="previewImage" />function previewImage(event: Event) { const input = event.target as HTMLInputElement; if (input.files && input.files.length > 0) { + const file = input.files[0]; + if (file.size > 5242880) { + toast.error("File size must be less than 5MB"); + return; + } + // Validate actual content type + const validTypes = ['image/jpeg', 'image/png', 'image/gif', 'image/avif', 'image/webp']; + if (!validTypes.includes(file.type)) { + toast.error("Invalid file type"); + return; + } const reader = new FileReader(); reader.onload = e => { form.preview = e.target?.result as string; }; - const file = input.files[0]; form.photo = file; reader.readAsDataURL(file); } }
Let me search for any existing file size validations in the codebase.
Let me check for any existing image handling or file upload implementations to understand the current validation patterns.
Let me check how files are handled on the backend to understand the complete validation flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if file size limits are implemented elsewhere rg -A 3 'maxFileSize|fileSize|sizeLimit'Length of output: 40615
Script:
#!/bin/bash # Check for file size limits and image validations rg -A 3 'maxSize|fileSize|maxFileSize|validateImage|validateFile'Length of output: 67
Script:
#!/bin/bash # Search for file upload and image handling patterns rg -A 5 'FileReader|createObjectURL|previewImage|handleFile|uploadFile'Length of output: 1209
Script:
#!/bin/bash # Search for file handling in backend rg -A 5 'multipart|FormFile|file.Size|content.*type|mime.*type' backend/Length of output: 3492
frontend/pages/profile.vue (1)
450-450: Consider using non-sensitive values in examplesWhile the translation implementation is correct, using a fixed amount (1000) in the currency example could potentially reveal information about typical transaction values in your system.
Consider using a smaller, non-business-representative amount like 123.45:
- <p class="m-2 text-sm">{{$t("profile.example")}}: {{ currencyExample }}</p> + <p class="m-2 text-sm">{{$t("profile.example")}}: {{ formatCurrency(123.45) }}</p>frontend/pages/item/[id]/index.vue (2)
Line range hint
312-358: Enhance currency value handlingThe current implementation directly converts currency values to strings without proper formatting or sanitization.
Consider implementing a currency formatter:
- text: String(item.value?.purchasePrice) || "", + text: formatCurrency(item.value?.purchasePrice) || "", type: "currency",Add a utility function:
function formatCurrency(value: number | undefined): string { if (value === undefined) return ''; return new Intl.NumberFormat(locale.value, { style: 'currency', currency: 'USD' // or from user preferences }).format(value); }
Line range hint
512-609: Enhance security for image handlingWhile the i18n changes look good, the image handling could benefit from additional security measures:
- Add Content Security Policy (CSP) headers for image sources
- Implement image sanitization
- Add loading="lazy" attribute for performance and security
- <img class="max-h-[200px] rounded" :src="img.src" /> + <img + class="max-h-[200px] rounded" + :src="img.src" + loading="lazy" + @error="handleImageError" + />Add to the script section:
function handleImageError(event: Event) { const img = event.target as HTMLImageElement; img.src = '/placeholder.png'; // fallback image console.error('Failed to load image:', img.src); }
🛑 Comments failed to post (13)
frontend/pages/home/statistics.ts (2)
11-12: 🛠️ Refactor suggestion
Consider moving i18n initialization.
The i18n initialization inside the function will create a new instance on every call. Consider moving it outside the function for better performance.
+const { t } = useI18n(); + export function statCardData(api: UserClient) { - const { t } = useI18n(); - const { data: statistics } = useAsyncData(async () => {Committable suggestion was skipped due to low confidence.
21-24: 💡 Codebase verification
Based on the findings, I can now provide a final response:
HTML content in translations requires proper sanitization
The codebase shows that:
- Some translation strings contain HTML content (links, spans, code blocks)
- The i18n configuration doesn't explicitly enable HTML escaping
- There's a DOMPurify.sanitize() usage in Markdown.vue, but it's not consistently applied across all HTML content
Security recommendations:
- Use DOMPurify to sanitize all HTML content from translations before rendering
- Add the following configuration to i18n:
const i18n = createI18n({ fallbackLocale: "en", globalInjection: true, legacy: false, + escapeParameters: true, + useScope: 'global', locale: preferences.value.language || checkDefaultLanguage() || "en", messageCompiler, });For the specific statistics values:
return [ { - label: t("home.total_value"), + label: DOMPurify.sanitize(t("home.total_value")), value: Math.max(0, Number(statistics.value?.totalItemPrice) || 0), type: "currency", },🔗 Analysis chain
Ensure translation keys are properly sanitized.
The translation system needs proper XSS protection for user-facing content. Verify that the vue-i18n configuration has HTML escaping enabled.
Additionally, consider adding input validation for the statistics values:
return [ { label: t("home.total_value"), - value: statistics.value?.totalItemPrice || 0, + value: Math.max(0, Number(statistics.value?.totalItemPrice) || 0), type: "currency", },[security]
Also applies to: 26-29, 31-34, 36-39
Let me gather more information about the i18n configuration and message compiler to verify the HTML escaping settings.
This will help us understand:
- How messages are compiled and if HTML escaping is enabled
- If there are any HTML sanitization utilities in use
- If any translation strings contain HTML tags that need escaping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check i18n configuration for security settings rg -A 5 "createI18n|i18n\.global" # Verify translation keys don't contain HTML rg -g "*.json" --json "total_value|total_items|total_locations|total_labels" frontend/locales/Length of output: 2413
Script:
#!/bin/bash # Check the message compiler implementation and HTML escaping settings rg -A 10 "messageCompiler" frontend/plugins/i18n.ts # Look for any HTML sanitization or escaping utilities rg -g "*.ts" -g "*.vue" "escapeHtml|sanitize|xss" frontend/ # Check if there are any HTML content in translations rg -g "*.json" "<[^>]+>" frontend/locales/Length of output: 19915
frontend/pages/home/index.vue (1)
31-31: 💡 Codebase verification
Based on the findings, I can now provide a final response about the security implications:
XSS Protection: Translation strings containing HTML require sanitization
The translation system is using Vue's
v-htmldirective to render HTML content from translation strings in several places:
- tools.import_export_set.import_sub
- tools.actions_sub
- tools.actions_set.zero_datetimes_sub
- tools.actions_set.set_primary_photo_sub
While this allows for rich text formatting in translations, it bypasses Vue's built-in HTML escaping and could expose the application to XSS attacks if malicious HTML/JavaScript is injected into the translation files.
Recommendations:
- Add HTML sanitization for translation strings that need HTML rendering
- Consider using markdown or safer alternatives for rich text formatting in translations
- If HTML is absolutely required, implement strict sanitization rules to only allow safe HTML tags and attributes
🔗 Analysis chain
Verify XSS protection in translation system
While the localization implementation looks correct, ensure that the translation system properly escapes HTML entities to prevent XSS attacks through malicious translation strings.
Run this script to check for HTML escaping in translation files:
Let me gather more information about the translation system and its security measures.
These scripts will help us understand:
- How the i18n system is configured
- Whether there are any explicit security measures
- The structure of translation files
- Vue's template compilation settings that might affect escaping
Let me check the Vue i18n configuration and usage to understand how text interpolation is handled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potentially dangerous HTML content in translation files rg -g '*.json' '<[^>]*>|&[^;]+;' frontend/locales/Length of output: 162
Script:
#!/bin/bash # Check translation setup and usage patterns # Look for Vue i18n configuration rg -l "createI18n|i18n" frontend/ # Check for translation files fd -e json . frontend/locales/ # Look for sanitization or escaping in i18n setup rg -A 5 "escapeParameter|escape|sanitize|html" frontend/Length of output: 14610
Script:
#!/bin/bash # Check Vue configuration and plugins fd -e js -e ts "main|plugins|i18n" frontend/ # Look for security related configurations rg -A 3 "useI18n|i18nInstance|i18n\.global" frontend/Length of output: 3098
Script:
#!/bin/bash # Check for any HTML content in translation files fd -e json . frontend/locales/ -x cat {} # Look for Vue's template compilation options rg "compilerOptions|template" frontend/vite.config.ts frontend/nuxt.config.tsLength of output: 191649
Script:
#!/bin/bash # Check Vue i18n configuration and initialization rg -A 5 "createI18n|i18n\.global|useI18n" frontend/plugins/i18n.ts # Check for any HTML escaping or sanitization configuration rg -A 5 "escapeHTML|sanitize|v-html" frontend/plugins/i18n.ts frontend/nuxt.config.tsLength of output: 560
Script:
#!/bin/bash # Check for any v-html usage in templates rg -A 2 "v-html" frontend/Length of output: 1232
Script:
#!/bin/bash # Look for Vue template compilation options in Nuxt config cat frontend/nuxt.config.tsLength of output: 1409
frontend/pages/location/[id].vue (2)
112-112: 💡 Codebase verification
Security Alert: XSS Vulnerability Risk in Translation Rendering
The verification reveals two security concerns that need attention:
- Several instances of
v-htmlwith$t()translations were found intools.vue, indicating potential XSS vulnerabilities- The i18n configuration uses
IntlMessageFormatbut lacks explicit HTML sanitizationSecurity recommendations:
- Replace
v-htmlwith{{ }}interpolation when using$t()translations- If HTML is required in translations, implement a sanitization layer using a library like DOMPurify
- Add input validation for user-provided translations if applicable
Locations requiring attention:
frontend/pages/tools.vue: Multiple instances ofv-htmlwith$t()frontend/plugins/i18n.ts: Add sanitization middleware for translation processing🔗 Analysis chain
Verify XSS protection in translation rendering.
The translation implementation looks good, but ensure that the translation system has proper XSS protection for user-provided translations.
Let's verify the translation system's security:
Also applies to: 114-115, 118-118
Let me check the i18n configuration to understand the sanitization settings.
The previous results showed some v-html usage with translations, so we need to verify if proper sanitization is in place. Let's examine the i18n setup and any security-related utilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any raw v-html usage with translations rg -g '*.vue' -g '*.ts' 'v-html.*\$t\(' # Check translation sanitization implementation rg -g '*.ts' -g '*.js' 'createI18n|i18n.*config'Length of output: 597
Script:
#!/bin/bash # Check i18n plugin implementation cat frontend/plugins/i18n.ts # Look for any sanitization utilities rg -g '*.ts' -g '*.js' 'sanitize|escape|xss'Length of output: 2254
155-155: 💡 Codebase verification
Based on the gathered information, I can now provide the final response:
⚠️ Additional security measures recommended for delete operationThe delete operation is currently protected by:
- Authentication token via Authorization header
- User confirmation dialog before deletion
However, for enhanced security, recommend implementing:
- CSRF token validation for state-changing operations
- Rate limiting on delete endpoints
- Additional server-side ownership validation
Points requiring attention:
- Add CSRF token to the frontend API client in
frontend/lib/requests/requests.ts- Include CSRF middleware configuration in
nuxt.config.ts- Consider implementing request signing for delete operations
🔗 Analysis chain
Ensure CSRF protection for delete action.
The localization changes look good. However, since this component handles sensitive delete operations, verify that CSRF protection is in place.
Let's check the API security implementation:
Also applies to: 165-165, 170-170
Let me gather more information about the API security implementation.
The previous results showed basic Authorization header usage, but we need to verify the complete security implementation including CSRF protection and delete endpoint handling.
Let me check one more aspect of the security implementation, specifically looking at the HTTP client setup and authentication handling.
This will help us understand how the API requests are secured at the HTTP client level, including any authentication tokens or security headers being set.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for CSRF token handling in API calls rg -g '*.ts' -g '*.js' 'csrf|xsrf' # Check for secure headers in API configuration rg -g '*.ts' -g '*.js' 'headers.*Authorization'Length of output: 204
Script:
#!/bin/bash # Check API request implementation ast-grep --pattern 'function $_(req, $$$) { $$$ }' # Look for delete endpoint implementation rg -g '*.ts' -g '*.js' -g '*.vue' 'delete.*location' -A 5 # Check for security middleware rg -g '*.ts' -g '*.js' 'middleware.*security' # Check Nuxt config for security headers fd 'nuxt.config' --exec cat {}Length of output: 5480
Script:
#!/bin/bash # Check HTTP client implementation for security headers rg -g '*.ts' -g '*.js' 'class.*Http' -A 10 # Look for authentication implementation rg -g '*.ts' -g '*.js' 'auth.*token' -A 5 # Check for any security related middleware or interceptors ast-grep --pattern 'interceptor($$$) { $$$ }'Length of output: 583
frontend/components/Item/View/Table.vue (2)
168-174: 🛠️ Refactor suggestion
Consider refactoring the header mapping implementation.
The current implementation is marked as a "hack" and could be improved for better maintainability. Consider implementing a proper header synchronization mechanism.
Here's a suggested refactor that makes the intent clearer:
- // this is a hack to make sure that any changes to the defaultHeaders are reflected in the preferences - .map(h => ( - { - ...defaultHeaders.find(h2 => h2.value === h.value) as TableHeader, - enabled: h.enabled - } - )) + // Sync header definitions while preserving user preferences + .map(h => { + const defaultHeader = defaultHeaders.find(h2 => h2.value === h.value); + if (!defaultHeader) return h; + return { + ...defaultHeader, + enabled: h.enabled + }; + })This refactor:
- Makes the code's purpose clearer
- Handles the case where a header might not exist in defaults
- Improves readability with explicit return statements
📝 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.// Sync header definitions while preserving user preferences .map(h => { const defaultHeader = defaultHeaders.find(h2 => h2.value === h.value); if (!defaultHeader) return h; return { ...defaultHeader, enabled: h.enabled }; })
152-161: 💡 Codebase verification
Security Alert: Translation keys need XSS protection measures
The verification reveals two important security considerations:
The table headers are properly using Vue's automatic HTML escaping with
{{ $t(h.text) }}, which is safe.However, there are several instances of
v-htmlbeing used with translations in tools.vue:
tools.import_export_set.import_subtools.actions_subtools.actions_set.zero_datetimes_subtools.actions_set.set_primary_photo_subSecurity Recommendations:
- Replace
v-htmlwith{{ }}interpolation where possible- If HTML is required in translations, implement a sanitization layer before rendering
- Add translation content security policy that forbids script-like content
- Document the trusted translation keys that are allowed to contain HTML
🔗 Analysis chain
LGTM: Well-structured header translations with proper type safety.
The translation keys follow a consistent naming pattern and maintain type safety with
satisfies TableHeader[].Security Note: Ensure that translation files are properly sanitized and don't contain any malicious HTML/JavaScript, as they will be rendered directly in the table headers.
The previous search didn't find the translation files. Let's verify if they exist and check how translations are used in Vue files to better understand the security context.
Let's verify how translations are used with v-html and specifically check the Table.vue component's translation usage to ensure proper security practices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify translation files for potential XSS payloads rg -g 'locales/*.json' -e '<script|javascript:|data:text/html|onerror=|onclick='Length of output: 17041
Script:
#!/bin/bash # Check for translation files and their content fd -e json -e yaml -e yml . locales/ -x cat {} 2>/dev/null # Check Vue files for translation usage patterns rg -g '*.vue' -e '\$t\(' -A 1 -B 1Length of output: 54572
Script:
#!/bin/bash # Check for v-html usage in tools.vue and any potential XSS risks rg -g '*.vue' -B 2 -A 2 'v-html="\$t\(' # Check translation key usage in Table.vue specifically rg -g 'Table.vue' '\$t\(' -A 1 -B 1Length of output: 3031
frontend/pages/profile.vue (1)
381-386:
⚠️ Potential issueSanitize language selection input
The language selection handler directly uses the event target's value without validation. While Vue provides some XSS protection by default, it's better to explicitly validate the language code.
Consider adding validation:
- <select v-model="$i18n.locale" @change="(event) => {setLanguage((event.target as HTMLSelectElement).value )}" + <select v-model="$i18n.locale" @change="(event) => { + const lang = (event.target as HTMLSelectElement).value; + if ($i18n.availableLocales.includes(lang)) { + setLanguage(lang); + } + }}"📝 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.<select v-model="$i18n.locale" @change="(event) => { const lang = (event.target as HTMLSelectElement).value; if ($i18n.availableLocales.includes(lang)) { setLanguage(lang); } }}" class="select select-bordered"> <option v-for="lang in $i18n.availableLocales" :key="lang" :value="lang"> {{ $t(`languages.${lang}`) }} </option> </select>frontend/pages/item/[id]/index.vue (1)
156-184: 💡 Codebase verification
Based on the analysis of the codebase, I can now provide a comprehensive security recommendation regarding the copyable fields.
Implement secure clipboard handling for sensitive fields
The application already has a robust notification system in place (
useNotifier). Let's leverage this for secure clipboard operations:
The serial number, model number, and manufacturer fields being copyable is acceptable for user experience, but should be:
- Logged when copied (for audit purposes)
- Accompanied by a notification to inform users about copying sensitive data
- Rate limited to prevent automated scraping
Implementation recommendations:
- Use the existing notification system to inform users when sensitive data is copied
- Add copy events to the existing event bus system for audit logging
- Implement rate limiting using the existing debounce utilities
Security considerations:
- Clipboard operations are already properly scoped to specific fields
- The application has existing infrastructure for user notifications
- No evidence of clipboard abuse prevention in current implementation
🔗 Analysis chain
Review security implications of copyable fields
The serial number, model number, and manufacturer fields are marked as copyable. While this improves user experience, consider the security implications of exposing these identifiers through the clipboard.
Consider implementing:
- Clipboard access notifications
- Logging of copy actions for sensitive fields
- Rate limiting for copy operations
Let me gather more information about the security implications and clipboard usage patterns in the codebase.
Let me check for any existing notification or toast systems that could be used for clipboard notifications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other instances of copyable sensitive fields rg -l 'copyable:\s*true' | xargs rg 'serial|model|manufacturer'Length of output: 397
Script:
#!/bin/bash # Check for clipboard-related security measures rg -A 2 'onCopy|@copy|v-clipboard' # Check for any existing security or logging implementations around copying rg -A 2 'copy.*log|clipboard.*log' # Look for any existing rate limiting implementations rg -A 2 'rateLimit|throttle|debounce' # Check for any security-related utilities or composables fd -e ts -e js -e vue "security|clipboard"Length of output: 2547
Script:
#!/bin/bash # Check for notification/toast systems rg -A 2 'toast|notify|notification' # Check for any existing composables or utilities that might be handling UI feedback fd composable util -t f | xargs rg 'toast|notify|alert' # Look for any existing event tracking or analytics rg -A 2 'track|analytics|event'Length of output: 62687
frontend/pages/item/[id]/index/edit.vue (4)
251-253:
⚠️ Potential issueSecure price handling implementation
The price fields need additional validation to prevent financial data manipulation.
Add validation for price fields:
{ type: "number", label: "items.purchase_price", ref: "purchasePrice", + validate: (value) => { + if (value < 0) throw new Error("Price cannot be negative"); + if (!Number.isFinite(value)) throw new Error("Invalid price"); + return Number(value.toFixed(2)); // Prevent floating-point precision issues + } },Also applies to: 291-293
522-527:
⚠️ Potential issueEnhance deletion security
Consider adding additional verification for item deletion to prevent accidental or malicious deletions.
Add a more secure confirmation process:
-async function deleteItem() { - const confirmed = await confirm.open("Are you sure you want to delete this item?"); +async function deleteItem() { + const itemName = item.value.name; + const confirmed = await confirm.open({ + title: "Delete Confirmation", + message: `Please type "${itemName}" to confirm deletion`, + input: true, + validator: (input) => input === itemName + });Committable suggestion was skipped due to low confidence.
185-237:
⚠️ Potential issueEnhance input validation for form fields
While maxLength constraints are good, consider adding input sanitization to prevent XSS attacks.
Add sanitization for user inputs:
{ type: "text", label: "items.name", ref: "name", maxLength: 255, minLength: 1, + validate: (value) => DOMPurify.sanitize(value) },Committable suggestion was skipped due to low confidence.
629-633:
⚠️ Potential issueAdd security measures for file uploads
The file upload functionality lacks crucial security checks.
Consider implementing:
- File type validation
- File size limits
- Malware scanning
<DropZone @drop="dropPhoto">{{ $t("items.photos") }}</DropZone> +<script> +const ALLOWED_TYPES = ['image/jpeg', 'image/png']; +const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB + +function validateFile(file) { + if (!ALLOWED_TYPES.includes(file.type)) { + throw new Error('Invalid file type'); + } + if (file.size > MAX_FILE_SIZE) { + throw new Error('File too large'); + } +} +</script>Committable suggestion was skipped due to low confidence.
|
For use time ago, we should be able to insert the i18n service and basically just insert the translation key in. On the translation side we'll of course have to make sure that people are aware of the correct formatting (probably via some sort of note or something in Weblate) I think it should work based on some limited experience with this kind of thing in the past. But we'll have to experiment with it a bit. |
…age in language dropdown
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
frontend/composables/use-formatters.ts (2)
43-92: LGTM: Well-structured i18n implementation with security considerationsThe implementation properly handles all time units and their variations while using the safe
t()function for translations.Security note: The implementation safely handles user input through the i18n system's built-in sanitization.
Consider these improvements:
- Memoize the messages object to avoid recreating it on every call:
const getI18nMessages = memoize((t: any) => ({ justNow: t("components.global.date_time.just-now"), // ... rest of the messages }));
- Extract translation keys as constants:
const TIME_TRANSLATIONS = { JUST_NOW: "components.global.date_time.just-now", AGO: "components.global.date_time.ago", // ... etc } as const;
Line range hint
94-124: Enhance date parsing securityWhile the implementation is good, the date parsing could be strengthened to prevent potential injection or DOS attacks through malformed date strings.
Consider adding stricter date validation:
- const dt = typeof value === "string" ? new Date(value) : value; + const dt = typeof value === "string" + ? (/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d{3})?Z$/.test(value) + ? new Date(value) + : null) + : value;The function correctly uses the new localized time formatting while maintaining compatibility with other formats.
🛑 Comments failed to post (1)
frontend/locales/en.json (1)
18-40: 💡 Codebase verification
Time-related translations are well-structured, but security improvements recommended
Based on the verification results, I found that:
- The date/time translations in
en.jsonare properly structured with placeholders- Some translations are rendered using
v-htmldirective in Vue templates- Date formatting appears to be handled through ISO string conversion
Security recommendations:
- Avoid using
v-htmlfor rendering time-related translations as it can lead to XSS vulnerabilities- Consider using Vue's built-in text interpolation
{{ }}orv-textdirective instead- Ensure all dynamic time values are properly sanitized before rendering
- Use a dedicated time formatting library (like Day.js or date-fns) with proper XSS protection
🔗 Analysis chain
LGTM! Time-related translations are well-structured.
The date/time translations are comprehensive and follow i18n best practices. The placeholders {0} are correctly used for dynamic values.
For security, verify that time-related translations are properly sanitized when rendered:
Let me generate another script to specifically look for time-related rendering patterns:
This will help us verify how time values are being rendered in the templates and check for any potential security concerns.
Let me check one more thing to see how the time formatting is actually implemented:
This will help us understand how the time-related translations are being used in the actual formatting logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for raw time string interpolation in Vue templates rg -g '*.vue' '\{\{.*(?:date|time|ago).*\}\}'Length of output: 1602
Script:
#!/bin/bash # Search for time-related rendering in Vue files rg -g '*.vue' -A 2 'v-text|v-html' --context 2Length of output: 2056
Script:
#!/bin/bash # Search for time/date formatting implementation rg -g '*.{vue,ts,js}' -A 3 'formatRelative|formatDate|dateTime'Length of output: 653
What type of PR is this?
(REQUIRED)
What this PR does / why we need it:
(REQUIRED)
Improves the support for languages, this includes adding a lot of missing strings to translations and making the language select save on change.
Which issue(s) this PR fixes:
(REQUIRED)
Does not currently deal with translating useTimeAgo, this could be done like this:
https://github.com/sysadminsmedia/homebox/blob/main/frontend/composables/use-formatters.ts#L67
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores