refactor/simplify#9
Merged
Merged
Conversation
image-fetch.util.ts and the shared ImageUrlSchema previously allowed server-side fetches of client-supplied http/https image URLs, guarded only by a hostname-string blocklist with no DNS resolution — any attacker-registered domain resolving to an internal/metadata IP (or a handful of unblocked IPv6 ranges/0.0.0.0) reached internal infrastructure from the Cloud Functions runtime. Remove the vulnerable branch entirely instead of patching the blocklist: every OCR endpoint (readings, billing-cycles, bills, image-extraction) now only accepts inline data:image/*;base64,... payloads, so the server never issues an outbound request to a client-supplied host. Readings no longer have an image_url field at all — meter photos are used transiently for OCR suggest only, never persisted, which also removes the now-pointless photo-settings feature (per-user savePhotos toggle) and its Firestore collection entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the API's SSRF fix: the backend now only accepts inline data:image/*;base64,... payloads for OCR, so image_url is no longer a fetchable URL anywhere. - readings page: drop the Firebase Storage upload path and the savePhotos-gated persistence — the compressed data URL is now the only value ever held in image_url, used purely to drive OCR suggest. - bills page: was uploading the bill photo to Storage first and then sending that HTTPS URL to POST /bills/ocr — the same SSRF-relevant upload-then-fetch pattern, and a pointless one since the resulting URL was never attached to the created billing cycle anyway. Switched to sending a compressed base64 data URL directly, matching the billings page's existing OCR flow. - Delete the photo-settings API module/types/settings page — nothing is persisted anymore, so there's nothing left to toggle. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the API/UI SSRF fix: readings no longer have an image_url field, so there's nothing to persist. Remove the savePhotos toggle from Settings, the photo-settings API module, the gated image_url attachment in CaptureReadings' submit payload, and the photo thumbnail in ReadingHistory. A captured photo is still used in-memory to drive the OCR suggest call, sent as a base64 data: URI, then discarded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
POST /users previously only pre-seeded a Firestore profile for a uid
the client had already created via the Firebase client SDK
(createUserWithEmailAndPassword). That client SDK call immediately
signs the browser into the newly created account, silently swapping
out the acting admin's own session.
Move account creation into the endpoint itself via the Admin SDK
(admin.auth().createUser()), which never touches the caller's client
session. The DTO now takes {email, password, displayName?, role}
instead of a client-supplied {uid, role}; Admin SDK errors
(email-already-exists, invalid-password, invalid-email) are mapped to
the same response shape the UI already expected.
Also fixes an adjacent bug in the same function: the new-user branch
called userRepository.create(), which writes to an auto-generated
Firestore document ID rather than the Firebase uid. Since
requireRole's role lookup is userRepository.getById(req.user.userId)
keyed on that same uid, the pre-seeded role was never actually found —
GET /auth/me's auto-create would silently default every new user to
"landlord" on their first request, discarding the admin's chosen role.
Switched to setDocument(COLLECTIONS.USERS, uid, ...), matching how
auth.service.ts's getMe() already keys the same collection.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Create New User form no longer calls Firebase's client-SDK
createUserWithEmailAndPassword — that call signed the admin's own
browser session into the newly created account. Now sends
{email, password, displayName, role} to POST /users in one request,
which creates the Firebase Auth account server-side via the Admin SDK,
and surfaces the backend's error message directly instead of mapping
client-SDK auth error codes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Updates root CLAUDE.md, PRIVACY.md, and the api/ui/mobile CLAUDE.md navigation docs to reflect: photo-settings removal (no meter-reading photo is ever persisted — OCR now runs on base64 payloads only, never a fetchable URL), and POST /users creating accounts entirely server-side via the Admin SDK instead of relying on the client SDK. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed helpers Six near-duplicate delete/purge/restore functions for property, meter-group, and reading each repeated the same query-then-mutate pattern for cascaded readings/billings and lock release/reclaim. Extracted shared cascadeReadingsBy/cascadeBillingsByPropertyId/cascadeBillingsByReadingIds helpers, cutting the file from 615 to ~370 lines with identical behavior (same transactions, same conditional-query optimization, same cache invalidation). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
meter-group/property/tenant services each repeated `new CachedRepository(repo, userId, name, ttl)` 6-8 times and the same getById-then-404 guard before update/delete/restore. Added a shared getOrThrow() in error.util.ts and a per-file repoFor() factory to remove the repetition. Also corrected the stale @deprecated note on meterGroupService.recordReset — reading.service.ts still reads MeterGroup.current_version as the live source for new readings, so the main-meter reset path is not actually removable yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dleware `app.use(authMiddleware)` in src/index.ts is mounted ahead of every protected feature router and throws before req.user can ever be unset, making the repeated `if (!userId) throw new AppError(401, ...)` guard in ~76 handlers across 11 controllers dead code. Replaced with a direct `req.user!.userId` read and dropped now-unused AppError imports where nothing else in the file used it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
getBillingCycle, getBilling, getMeterGroup, getProperty, and formatTimestampDateTime had zero call sites anywhere in mobile/src (verified by grep before removal). Updated the module map in mobile/CLAUDE.md to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… logic The "get from cache, else fetch and store" pattern for meter groups and properties was copy-pasted across ReadingHistory, CaptureReadings, and Billings. Added sessionCache.getOrFetchMeterGroups()/getOrFetchProperties() so each screen makes one call instead of repeating the check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Object.entries(property.meter_groups).find(...) membership lookup was duplicated at two call sites in CaptureReadings.svelte's proceedToStep2. Extracted findMeterGroupEntry()/needsSeedReading() to a shared util. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaced the sequential if-chain mapping Firebase auth codes to messages with a Record lookup; also dropped an unused local `message` variable. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 5 archive pages (meter-groups, properties, tenants, readings, billings) each hand-rolled identical data/isLoading/error/restoringId/isClearing state, loadData/handleRestore/handleClearCache, and the same Clear Cache button markup, despite ArchivePageTemplate already covering the table itself. Added createArchivePageState() and a ClearCacheButton component so each page now only supplies its API functions and column definitions (94-113 lines each down to ~35-55). Also swapped each page's formatDate(toDate(v)) column formatter for the new formatFirestoreDate() helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ant stores meter-groups.svelte.ts, properties.svelte.ts, and tenants.svelte.ts were byte-for-byte identical (fetch-with-staleness caching) aside from the imported fetch function and type. Extracted createListStore() so each file is now a 3-line instantiation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
billings/+page.svelte reimplemented getCumulativeOffset/getVersionsSource/ trueReading verbatim (including comments) from lib/utils/true-reading.ts instead of importing them. Now imports the shared versions and keeps only the page-specific readingConsumption() on top. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
formatDate(toDate(x)) was composed ad hoc at call sites across dashboard, meter-groups, properties, and tenants. Added formatFirestoreDate() to format.ts and swapped in the affected call sites. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
android/ is git-ignored and regenerated locally via `npx cap add android`, so the network_security_config.xml + AndroidManifest.xml wiring documented in BUILD.md was a manual, unenforced step — nothing checked it existed before packaging, so it was easy to lose silently after a fresh checkout, a new machine, or deleting android/ to fix a sync issue. Add scripts/apply-network-security-config.mjs, wired into both `npm run cap:add:android` and `npm run cap:sync`, so the config is created and wired into the manifest automatically every time those commands run instead of relying on a human to remember BUILD.md. Idempotent — verified against both a fresh (config/attribute missing) and already-configured android/ tree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
No description provided.