Fix/firebase#10
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>
Tenants read raw meter dial values, so a cumulative all-time total never matches what they see and photograph. Consumption math still uses the version-aware cumulative offset internally; only the displayed numbers on the readings table, batch form, and expanded billing cycle rows now show the raw reading plus a meter-version/ reset indicator instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merges the simplification/refactor pass across api, ui, and mobile (dedupe helpers, archive-page/list-store extraction, mobile cleanup) plus the true-reading display fix for tenant-facing meter readings.
…ilters + update consistency Denormalize meter_group_id and billing_period_date onto Billing (copied from current_reading, mirroring what BillingCycle already does), and add scoped query filters so the UI can ask for less instead of scanning whole collections: - Billing gains meter_group_id / billing_period_date, set at every write path via a shared deriveBillingDenormalizedFields() helper (create, batch, createFromReadings, update, updateBatch). - GET /billings gains meterGroupId / startDate / endDate filters; GET /billing-cycles gains a meterGroupId equality filter. - update()/updateBatch() re-derive the denormalized fields when current_reading_id changes, so the PATCH correction escape hatch can no longer leave them stale. - billing.validator: validateUpdate now resolves the stored billing by id and validates whenever either reading id changes (previously only when property + both readings were all present together, so a lone current_reading_id skipped the same-meter-group / rollback / main-meter checks). - billing-cycle.validator: cross-check each billing's reading meter_group_id against the cycle's declared meter_group_id, run on create and whenever a PATCH changes billing_ids or meter_group_id (resolving the missing side from the stored cycle). - UI api/types wired through (pages don't call the new params yet). Verified: tsc --noEmit clean, lint 0 errors, npm test 349/349. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Idempotent Admin SDK script that denormalizes meter_group_id / billing_period_date onto existing Billing docs, resolved from each billing's current_reading_id. --dry-run (default) writes a report and performs no writes; --apply writes in Firestore batches. Already-backfilled docs are skipped, so re-runs are safe. Excludes scripts/** from the eslint "src" scope and adds a backfill:billing-meter-group npm script. Applied against staging: 365/365 billings resolved, 0 failures (verified idempotent on re-run). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…erGroup) The "move version tracking to Property for BOTH submeters and main meters" plan was only partially carried out: submeters moved, but main meters deliberately stayed on MeterGroup.current_version/.versions (propertyService rejects main-meter resets and routes them to meterGroupService; resolveVersionsSource reads MeterGroup for main meters). No backfill script was ever written and none is pending. Runtime is self-consistent; the risk was documentation debt — a future dev trusting the @deprecated tag or "migration pending" framing could delete the still-live main-meter fields and break every main-meter consumption calculation. Corrects: - meter-group.model.ts — JSDoc now says deprecated for submeters only; main-meter fields are live and required, do not delete - root CLAUDE.md — meter-groups feature bullet (The decisions/ log files are gitignored and updated locally only.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…endpoints
The active-list in-memory date post-filters compared `new Date(billing_start_date)`
/ `new Date(billing_period_date)`, but at that layer the value is a Firestore
Timestamp (or a Redis-round-tripped {_seconds,_nanoseconds}), not an ISO string —
so `new Date()` produced `Invalid Date` and every comparison was false, making
`GET /billing-cycles?billingStartDate=`/`billingEndDate=` and
`GET /billings?startDate=`/`endDate=` return 0 rows for ANY input.
Fixed by converting with the existing `parseTimestamp(x).toDate()` helper. This
was a pre-existing bug in billing-cycle.service and the identical pattern Landing 1
copied into billing.service; the Landing 2 dashboard depends on the cycle filter.
Verified live against staging: billingStartDate 2020 -> 98, 2025-07-19 -> 33,
2099 -> 0; billings startDate 2020 -> 100, meterGroupId+startDate -> 99. Suite 349/349.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Stop pulling whole collections into memory on every visit, using the denormalized
fields + scoped filters from Landing 1.
- New ui/src/lib/stores/entity-lookup-cache.ts: persistent ID->entity caches
(getBillingsByIds / getReadingsByIds) resolving only the IDs visible cycles
reference, via the API's per-ID getById cache.
- Billings page: removed the full `readings` load. Discovery/gap-fill/overrides,
edit modal, and straggler now fetch readings scoped by meterGroupId/propertyId;
expanded/printed cycles resolve reading amounts via getReadingsByIds.
autoCalculateCycleDates now queries getBillingCycles({ meterGroupId }) — one
precise read, fixing the latent bug where it silently reset dates to the current
month once cycle volume exceeded one page. (allBillings stays a full load: the
payment-status filter spans all cycles and has no scoped server query — see
decisions/20260719_landing2-deviations-and-reports-finding.md.)
- Dashboard: cycles fetched date-bounded to a 12-month window; their billings
resolved via getBillingsByIds. Kept the existing raw-consumption stat math rather
than switching to getSummaryReport (which uses version-aware consumption and would
change the displayed totals) — behavior-preserving.
- createListStore: page at limit 100 with cursors instead of one limit:1000 request;
.data stays the complete list so consumers are unaffected.
Verified: svelte-check 0/0, eslint 0 errors, prettier-clean on touched files; every
scoped query smoke-tested live against staging.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The app currently runs with a single active account despite the role model existing in code, so cross-tenant isolation (user_id scoping) is a latent risk, not an active one. Block POST /users in both layers until that isolation work is scoped, so a second account can't be onboarded by accident and silently trigger it. Both guards reference each other in comments so a future re-enable touches both sides together.
…pagination
- GET /reports computes summary/consumption/billing-trends/collection-status
from one shared cycles->billings->properties->meter-groups->readings join
instead of four independent re-fetches; the UI now calls it in one request.
- Add requireRole("admin","landlord") to all /reports/* routes as
defense-in-depth (financial data was previously reachable by any
authenticated role, including assistant).
- fetchReportContext now loops through every billing-cycle page via
fetchAllPages instead of taking a single limit:1000 page, so a landlord
with >1000 cycles no longer gets reports silently computed from only the
newest 1000.
- 60s buildJoinedDataCached TTL cache over the join, since there's no
write-path invalidation hook for it.
- aggregateConsumption's by_property now builds from a single Map pass
instead of joinedData.find(...) inside a .map() (O(n) instead of O(n*m)).
OCR calls are cost-bearing (hit the tenant's own configured vision provider) but previously fell under the generic apiRateLimiter (1000/hr, per-IP), letting a single account burn ~1000 vision calls/hour and putting users behind a shared office IP in the same bucket. Add a new per-user ocrRateLimiter (150/hr, mirrors chatbotRateLimiter's per-user keying) sitting between the tight chatbot limit and the generic per-IP limit, and apply it to /image-extraction/*, /readings/ocr, /billing-cycles/ocr, /bills/ocr.
billingService.create() only wrote the ID cache (cacheSet), unlike createBatch() and the reading-service auto-billing path, which both update the list cache too via listAppend. A billing created through the single POST /billings path (e.g. the Billings page's gap-fill/straggler flow) was silently missing from the still-warm list cache until its TTL expired. Also give listAppend an explicit ttlSeconds parameter instead of a hardcoded 30-minute default, and pass each call site's real feature TTL (10 min for billings/readings) so the list cache and ID cache entries for the same write can't drift apart.
- handleMarkAsPaid() now calls setCachedBilling() after a successful status change — it never updated the module-level entity-lookup-cache billingCache, so visiting /dashboard after marking a billing paid on /billings still showed the stale pending amount until something else cleared the cache. - Billings page date-range "to" filter now compares against end-of-day UTC instead of UTC midnight, so a cycle dated exactly on the selected end date is no longer excluded by its own time-of-day component. - Dashboard's "This Month Billed" stat and billing-cycle.util.ts's paid/outstanding helpers now route consumption*rate through billAmount()/sumMoney() (decimal.js, round-half-up) instead of summing raw floats, matching every other money computation in the app.
…ers, clearCache factory Cleanup batch from the audit's simplification/reuse findings, no user-facing behavior change beyond the new endpoints: - New GET /billings/batch-get and /readings/batch-get (comma-separated ids, capped at 100) so the UI's entity-lookup-cache.ts can resolve a cold cache with one round-trip per collection instead of N individual GETs. Wired billingService.getByIds / readingService.getByIds on the server and rewrote entity-lookup-cache.ts's resolveByIds to batch-fetch (chunked at 100) instead of Promise.all-ing per-ID requests. - Extracted a private repoFor(userId) helper in billing-cycle/billing/ reading services, matching the pattern already used by tenant/property/ meter-group — replaces ~25 inline `new CachedRepository(...)` constructions, closing a drift risk (feature-name string or TTL could diverge across copies). - Extracted utils/date-range-filter.util.ts — the in-memory date-range post-filter logic was duplicated verbatim between billing.service.ts and billing-cycle.service.ts. - Collapsed the byte-identical `clearCache` controller handler (6 features, differing only by cache-key prefix and label) into a makeClearCacheHandler() factory in utils/clear-cache-handler.util.ts.
meter-groups.ts, properties.ts, tenants.ts, and billing-cycles.ts each hand-rolled the same URLSearchParams + `if (params?.x) query.set(...)` loop that reports.ts already had a local version of. Added a shared toQueryString() to client.ts and switched all four modules (plus reports.ts, billings.ts, and readings.ts in earlier commits) to use it.
- Contains all stored test information for staging directly.
|
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
1 participant
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.