Fix security issues from codebase audit - #14
Merged
Conversation
- Escape user/external data in Leaflet map popups/tooltips (stored XSS via place names and geocoded addresses) using a shared window.escapeHtml helper - Validate vehicleFilter against the user's own vehicles in HasVehicleFilter so an arbitrary vehicle id can't scope drive/charge reads or bulk-tag writes - Neutralize CSV formula injection in drive/charge/raw exports (cells leading with = + - @ are prefixed with a quote) - Use hash_equals for the telemetry shared-secret comparison (timing safety) - Bind TeslaFiImport::checkProcessing to the current user's cache key https://claude.ai/code/session_014qGSJ8rbKzPDPaJir25uGV
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses several security issues identified in a codebase audit, primarily around client-side HTML injection, authorization scoping via user-controlled filters, CSV formula injection, and timing-safe secret comparisons.
Changes:
- Escapes user-controlled strings before inserting them into Leaflet popups/tooltips via a shared
window.escapeHtmlhelper. - Hardens request/Livewire state handling by constraining cache keys and vehicle filters to the current user’s scope.
- Neutralizes CSV formula injection in exports and switches telemetry secret comparison to
hash_equals.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/views/livewire/places-list.blade.php | Escapes place names before inserting into Leaflet popup HTML. |
| resources/views/livewire/lifetime-map.blade.php | Escapes marker labels before inserting into tooltip HTML. |
| resources/views/livewire/charges-list.blade.php | Escapes marker labels before inserting into tooltip HTML. |
| resources/views/components/layouts/app.blade.php | Adds global window.escapeHtml helper for safe HTML injection. |
| app/Livewire/TeslaFiImport.php | Ensures processingKey is bound to the authenticated user before reading cache. |
| app/Livewire/Concerns/HasVehicleFilter.php | Restricts vehicleFilter to the authenticated user’s owned vehicle IDs. |
| app/Http/Middleware/TelemetryAuth.php | Uses timing-safe secret comparison (hash_equals). |
| app/Http/Controllers/Api/V1/ExportController.php | Adds csvSafe() and applies it to exported text fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+15
to
+19
| $owned = Auth::user()->vehicles()->pluck('id'); | ||
|
|
||
| return $this->vehicleFilter | ||
| ? collect([(int) $this->vehicleFilter]) | ||
| : $user->vehicles()->pluck('id'); | ||
| ? $owned->intersect([(int) $this->vehicleFilter])->values() | ||
| : $owned; |
Owner
Author
There was a problem hiding this comment.
Good catch — fixed in 902a38d. The owned-vehicle resolution is now extracted into a shared ResolvesVehicleFilter concern, used by both HasVehicleFilter and all five Analytics chart components (EfficiencyChart, EnergyChart, CostCharts, CalendarPanel, TempEfficiencyChart), so the reactive vehicleFilter prop is validated against the user's own vehicles everywhere. A repo-wide grep confirms no remaining instances of the unvalidated pattern.
Generated by Claude Code
…d LF to csvSafe - Extract the owned-vehicle resolution into a ResolvesVehicleFilter concern and use it from both HasVehicleFilter and the five Analytics chart components, which previously re-derived vehicle IDs from the reactive vehicleFilter prop without an ownership check - Include a leading LF in csvSafe()'s formula-injection check https://claude.ai/code/session_014qGSJ8rbKzPDPaJir25uGV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://claude.ai/code/session_014qGSJ8rbKzPDPaJir25uGV