Fix Ledger Privacy & Obfuscation - #29
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR enhances privacy and security in the Ledger List feature by implementing three key improvements: correcting the admin client configuration to use DB_SECRET_KEY, obfuscating user emails in the UI, and introducing a "whimsical location" feature that replaces precise coordinates with general location names (e.g., "Herndon") using OpenStreetMap's Nominatim API.
Key Changes:
- Admin client now prioritizes
DB_SECRET_KEYfor privileged operations - Email obfuscation changed from showing full username to showing first 3 and last 3 characters with ellipsis
- New location privacy feature using Nominatim API to fetch suburb/town names instead of displaying raw coordinates
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
lib/supabase-admin.ts |
Updated environment variable priority to use DB_SECRET_KEY first for admin operations |
lib/location-utils.ts |
New utility that integrates with Nominatim API to fetch "whimsical" location names for privacy |
app/page.tsx |
Enhanced email obfuscation to show only first 3 and last 3 characters of username |
app/api/books/generate/route.ts |
Integrated whimsical location feature and switched from admin client to route handler client |
__tests__/app/api/books/generate/route.test.ts |
Added mock for location utils in existing tests |
__tests__/api/generate-location.test.ts |
New comprehensive test suite for location privacy feature |
__tests__/api/generate-auth.test.ts |
Added location utils mock and headers to test requests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return `${lat.toFixed(2)}, ${lon.toFixed(2)}` | ||
| } | ||
|
|
||
| const data = await res.json() | ||
|
|
||
| // Extract just the suburb, neighborhood, or town as requested | ||
| // Fallback to "The Wilds" if nothing specific found, or coordinate-ish hint | ||
| const name = | ||
| data.address?.suburb || | ||
| data.address?.town || | ||
| data.address?.city || | ||
| data.address?.village || | ||
| data.address?.county || | ||
| "The Wilds" | ||
|
|
||
| return name | ||
| } catch (error) { | ||
| console.error("Failed to fetch whimsical location:", error) | ||
| // Fallback to simplified coordinates | ||
| return `${lat.toFixed(2)}, ${lon.toFixed(2)}` |
There was a problem hiding this comment.
The fallback coordinate format uses toFixed(2) which provides 2 decimal places. This level of precision still reveals location within roughly 1km, which may not align with the privacy goals of using "whimsical locations". Consider reducing precision further (e.g., toFixed(1) or even toFixed(0)) in the fallback, or use a generic message like "Unknown Location" to maintain consistency with the privacy approach.
| { | ||
| headers: { | ||
| // Nominatim requires a User-Agent. | ||
| "User-Agent": "LFL-BookTracker/1.0", |
There was a problem hiding this comment.
The User-Agent string "LFL-BookTracker/1.0" should be updated to include contact information as per Nominatim's Usage Policy. The policy states: "Valid User-Agent identifying your application. Preferably a link to your application or an email address." Consider updating this to something like "LFL-BookTracker/1.0 (contact-email@domain.com)" or include a URL to the project.
| "User-Agent": "LFL-BookTracker/1.0", | |
| "User-Agent": "LFL-BookTracker/1.0 (contact@example.com)", |
| } | ||
| ) | ||
|
|
||
| if (!res.ok) { | ||
| console.warn("Nominatim API Error:", res.status, res.statusText) | ||
| return `${lat.toFixed(2)}, ${lon.toFixed(2)}` | ||
| } |
There was a problem hiding this comment.
The getWhimsicalLocation function is called during book generation, which is a synchronous operation in the request flow. If the Nominatim API is slow or times out, this will block the entire book generation process. Consider adding a timeout to the fetch request to prevent indefinite hanging. For example, use signal: AbortSignal.timeout(5000) to timeout after 5 seconds and fall back to the coordinate format.
| } | |
| ) | |
| if (!res.ok) { | |
| console.warn("Nominatim API Error:", res.status, res.statusText) | |
| return `${lat.toFixed(2)}, ${lon.toFixed(2)}` | |
| } | |
| signal: AbortSignal.timeout(5000), | |
| } | |
| ) | |
| if (!res.ok) { | |
| console.warn("Nominatim API Error:", res.status, res.statusText) | |
| return `${lat.toFixed(2)}, ${lon.toFixed(2)}` |
| // Format: first 3 chars + ... + last 3 chars of username. | ||
| const username = fullEmail.split("@")[0] | ||
| const maskedEmail = `${username.slice(0, 3)}...${username.slice(-3)}` |
There was a problem hiding this comment.
The email obfuscation logic has a bug with short usernames. For usernames with 6 or fewer characters, the slice operations will produce overlapping or duplicate characters. For example:
- A username "abc" would become "abc...abc" (duplicating all characters)
- A username "test" would become "tes...est" (overlapping)
The logic should check the username length and handle short usernames appropriately, perhaps by showing them as "ab..." or similar for very short names.
| // Format: first 3 chars + ... + last 3 chars of username. | |
| const username = fullEmail.split("@")[0] | |
| const maskedEmail = `${username.slice(0, 3)}...${username.slice(-3)}` | |
| // For typical usernames (> 6 chars): first 3 chars + ... + last 3 chars of username. | |
| const username = fullEmail.split("@")[0] | |
| let maskedUsername: string | |
| if (username.length <= 2) { | |
| // Very short usernames: show first char only. | |
| maskedUsername = `${username.slice(0, 1)}...` | |
| } else if (username.length <= 6) { | |
| // Short usernames: avoid overlapping/duplicating characters. | |
| maskedUsername = `${username.slice(0, 2)}...${username.slice(-1)}` | |
| } else { | |
| maskedUsername = `${username.slice(0, 3)}...${username.slice(-3)}` | |
| } | |
| const maskedEmail = maskedUsername |
| export async function getWhimsicalLocation(lat: number, lon: number): Promise<string> { | ||
| try { | ||
| const res = await fetch( | ||
| `https://nominatim.openstreetmap.org/reverse?format=json&lat=${lat}&lon=${lon}&zoom=10`, |
There was a problem hiding this comment.
The zoom level of 10 in the Nominatim API request may not consistently return suburb/town level data for all locations. Zoom level 10 typically corresponds to city-level detail. For better consistency in getting neighborhood/suburb information, consider using zoom level 14-16. However, if the intention is to provide less precise information for privacy, the current level may be appropriate - just ensure this matches the privacy requirements.
| `https://nominatim.openstreetmap.org/reverse?format=json&lat=${lat}&lon=${lon}&zoom=10`, | |
| `https://nominatim.openstreetmap.org/reverse?format=json&lat=${lat}&lon=${lon}&zoom=14`, |
| const res = await fetch( | ||
| `https://nominatim.openstreetmap.org/reverse?format=json&lat=${lat}&lon=${lon}&zoom=10`, | ||
| { | ||
| headers: { | ||
| // Nominatim requires a User-Agent. | ||
| "User-Agent": "LFL-BookTracker/1.0", | ||
| }, | ||
| next: { | ||
| // Cache for a long time to avoid rate limits on known coords | ||
| revalidate: 86400, | ||
| }, | ||
| } | ||
| ) |
There was a problem hiding this comment.
The Nominatim Usage Policy requires no more than 1 request per second and recommends implementing proper caching. While the code implements caching with a 24-hour revalidation period, there's no rate limiting mechanism in place. If multiple books are generated simultaneously with different coordinates, this could violate the API's usage policy. Consider implementing a request queue or rate limiter to ensure compliance with the 1 request/second limit.
There was a problem hiding this comment.
Not considering this high-priority; if we hit a rate limit I'll be impressed.
Fixes Ledger List privacy issues by:
DB_SECRET_KEYfor admin operations.tes...ser).Also includes a regression test for the location privacy feature.