Lantern Venue API โ QA Audit Report โ
Date: February 10, 2026 Auditor: Claude Sonnet 4.5 (claude.ai/code) Scope: Venue API Cloud Run service (services/api/venues/) + client integration (apps/web/src/lib/venue*.js) + Firestore security rules for venue collections Branch: cattreedev/issue262
Executive Summary โ
Overall Assessment: โญโญโญโญ (4/5 โ Production-capable with blockers to address)
The Venue API is a well-structured Express.js service with strong authentication, clean separation of concerns, comprehensive OpenAPI documentation, and thoughtful integration with the shared @lantern/shared package. The architecture is sound and the service is ready for dev, but three findings represent pre-production blockers: a Firestore rule allows any authenticated user to write arbitrary activeLanternCount values to any venue; client-side OSM venue creation remains possible despite the server-side API being in place (Issue #224 is not fully closed); and in-memory rate limiting will not survive Cloud Run scale-out.
Findings at a Glance โ
| Severity | Count |
|---|---|
| ๐ด Critical | 3 |
| โ ๏ธ High | 4 |
| ๐ก Medium | 7 |
| โน๏ธ Low | 6 |
1. Architecture Overview โ
Client (apps/web)
โ
โโ venueRefreshService.js โ staleness check (reads venueRefreshMetadata from Firestore)
โ โ
โ โโ> triggerAreaRefresh()
โ โ
โ โโ> venueApiClient.js โโ POST /venues/import/osm โโโโโโโโโโโโโโโ
โ โ
โโ venueService.js โ direct Firestore reads (geohash range queries) โ
โ โ
โโ getNearbyVenues() โ Firestore `venues` collection โ
โโ enrichVenueAddress() โ Cloud Function (direct) / venue-api (future) โ
โผ
โโโโ Venue API (Cloud Run) โโโโ
โ services/api/venues/ โ
โ โ
โ middleware/ โ
โ verifyFirebaseToken โ
โ requireRole โ
โ rateLimitMiddleware โ
โ โ
โ routes/ โ
โ import.js (OSM import) โ
โ refresh.js (enrichment) โ
โ utility.js (list/nearby) โ
โ admin.js (admin CRUD) โ
โ โ
โ services/ โ
โ osm.service.js โ
โ nominatim.service.js โ
โ venue.service.js โ
โโโโโโโโโโโโฌโโโโโโโโโโโโโโโโโโโ
โ
โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโผโโโโโโโโโโโโโโโโโโโ
โผ โผ โผ
Overpass API Nominatim API Firestore
(OSM data) (reverse geocoding) (venues, metadata)Shared configuration in @lantern/shared/venues is the single source of truth for tier filters, category maps, overpass endpoints, and refresh thresholds โ consumed by both the Cloud Run service and the web client. @lantern/shared/services owns CORS allowed origins and rate limit constants.
Production status: The service is deployed to dev only. prodUrl: null in packages/shared/services/index.js.
2. Security Findings โ
2.1 Authentication & Authorization โ
Auth Middleware (services/api/venues/src/middleware/auth.js) โ Verifies Firebase ID tokens via the Admin SDK. Standard Bearer scheme. Implementation is solid.
RBAC Middleware (services/api/venues/src/middleware/rbac.js):
โ ๏ธ H2 โ RBAC silently defaults to 'user' when role field is absent โ
File: services/api/venues/src/middleware/rbac.js:33
const userRole = userData.role || 'user';If a user document exists in Firestore but has no role field (e.g., a legacy record or partially-created account), this silently grants 'user' access. For most routes this is harmless, but it means a misconfigured document won't surface an error โ the request proceeds as a regular user instead of being rejected. This masks misconfiguration rather than failing loudly.
Recommendation: Return 403 FORBIDDEN when role is absent, rather than defaulting.
๐ก M4a โ Error message leaks current user role โ
File: services/api/venues/src/middleware/rbac.js:39
message: `This action requires '${minRole}' role. You have '${userRole}'.`The response reveals the authenticated user's current role to the caller, which is information that should be opaque outside admin interfaces.
Recommendation: Use a generic message: "Insufficient permissions for this action".
๐ก M4b โ "User not found" message leaks account existence โ
File: services/api/venues/src/middleware/rbac.js:28
message: 'User not found in system'A caller with a valid Firebase token but no Firestore document gets a distinguishable error. This confirms that the authenticated account exists in Firebase Auth but not in the app's user collection โ useful information for enumeration.
Recommendation: Return the same generic 403 FORBIDDEN response for both "not found" and "insufficient role".
๐ก M (Cloud Scheduler bypass in dev) โ
File: services/api/venues/src/routes/refresh.js (Cloud Scheduler header check)
The POST /venues/refresh/scheduled endpoint validates the X-CloudScheduler-JobName header in production but skips this check entirely in dev mode. This is correct dev-mode behavior, but the pattern means developers can test the endpoint without the header, potentially masking issues if the check is accidentally made environment-dependent in a future refactor.
Recommendation: Add a comment noting this explicitly, and consider an integration test that verifies the header is enforced in non-dev mode.
2.2 Rate Limiting โ
๐ด C3 โ In-memory rate limiting fails under Cloud Run multi-instance scale-out โ
File: services/api/venues/src/middleware/rateLimiter.js
/** @type {Map<string, number[]>} source โ array of timestamps */
const sourceWindows = new Map();
/** @type {Map<string, number[]>} userId โ array of timestamps */
const userWindows = new Map();All rate limit state is stored in module-level Map objects. This means:
- State is lost on every pod restart or deployment โ rate limits effectively reset.
- Multi-instance deployments multiply effective limits โ if Cloud Run scales to 3 pods, the effective user limit for
importis5 ร 3 = 15requests/minute. - Cloud Run's default concurrency is 80 requests/instance; a single user can drive scale-out and then have 80ร the intended rate limit.
This is particularly risky for Nominatim (1 req/sec policy): violating Nominatim's usage policy risks IP banning the entire Cloud Run service.
Recommendation: Replace in-memory state with Firestore counters or Memorystore (Redis) before scaling beyond a single instance. Until then, set --max-instances=1 on the Cloud Run service as a mitigation.
โ ๏ธ H3 โ Unauthenticated requests bypass user rate limiting โ
File: services/api/venues/src/middleware/rateLimiter.js:119
export function rateLimitMiddleware(action) {
return (req, res, next) => {
const userId = req.user?.uid;
if (!userId) return next(); // โ bypasses rate limiting entirelyIf req.user is not set (unauthenticated request), the middleware calls next() immediately. In practice, routes using this middleware also require verifyFirebaseToken, which should block unauthenticated requests first. However, the ordering dependence is fragile โ if rateLimitMiddleware is ever applied to a route before auth middleware, or to a route that relaxes auth requirements, rate limiting is silently skipped.
Recommendation: Add a defensive check: if userId is absent, either reject with 401 or use an IP-based fallback key.
2.3 Firestore Security Rules โ Venue Collections โ
These findings are in firestore.rules and affect both the Cloud Run service and direct client access.
๐ด C2 โ Any authenticated user can set activeLanternCount to any value โ
File: firestore.rules:308-311
allow update: if isAuthenticated()
&& (resource.data.merchantId == request.auth.uid
|| resource.data.ownerId == request.auth.uid
|| request.resource.data.diff(resource.data).affectedKeys().hasOnly(['activeLanternCount', 'updatedAt']));The third branch allows any authenticated user to update activeLanternCount and updatedAt on any venue, with no constraint on the value being set. A user can:
- Set
activeLanternCountto9999on any venue (artificial popularity inflation) - Set it to
-1or0to suppress a venue's visibility in the feed - Race with legitimate lantern lighting to create count inconsistencies
The field should only be updated by Cloud Functions using the Admin SDK (which bypasses these rules). Client-side writes to this field should be removed entirely.
Recommendation: Remove the third branch. Lantern count updates should be atomic Cloud Function increments only.
๐ด C1 โ Client-side openstreetmap venue creation is still allowed (Issue #224 not closed) โ
File: firestore.rules:291-305
allow create: if isAuthenticated()
&& request.resource.data.keys().hasAll([...])
&& request.resource.data.source in ['openstreetmap', 'manual', 'google_places'];Although the Venue API Cloud Run service now handles OSM imports server-side using the Admin SDK (which bypasses these rules), this rule still permits any authenticated user to write a venue document directly to Firestore with source: 'openstreetmap'. This means:
- A malicious user can flood the
venuescollection with fabricated OSM venues - They bypass all server-side deduplication, rate limiting, and validation in the Cloud Run service
- No OSM ID validation occurs client-side
Issue #224 (Move OSM Import to server-side) is referenced in the rule comment but the rules were not updated to close the client pathway.
Recommendation: Restrict source: 'openstreetmap' creation to Cloud Functions (Admin SDK). Client-side creates should only be permitted with source: 'manual'. Update the rule to:
allow create: if isAuthenticated()
&& request.resource.data.source == 'manual';โ ๏ธ H1 โ Any authenticated user can write venueRefreshMetadata documents โ
File: firestore.rules:397
match /venueRefreshMetadata/{geohashPrefix} {
allow read: if true;
allow write: if isAuthenticated();
}The comment acknowledges that server-side updates use the Admin SDK, but the rule allows any authenticated user to write these documents. An attacker could:
- Set
inProgress: trueon a geohash prefix to indefinitely block venue imports for an area (denial-of-venue-data attack) - Clear
inProgress: falseprematurely, causing race conditions during active imports - Falsify
lastRefreshedAtto trick all users into thinking an area is fresh and suppress background refreshes
Recommendation: Restrict writes to Cloud Functions / Admin SDK only. Since the Venue API uses Admin SDK and the client only reads this collection for staleness checks, client write access can be removed entirely.
โน๏ธ (Design note) โ Venue collection is world-readable without authentication โ
File: firestore.rules:283
allow read: if true;This is an intentional design decision to support guest browsing. However, it means:
- The full venue catalog (name, lat/lng, address, phone, website, OSM ID) is queryable by anyone
activeLanternCount(real-time popularity signal) is publicly readable, allowing competitive intelligence gathering- No rate limiting applies to Firestore reads from unauthenticated clients
For the current use case (public venue directory with no user PII) this is an acceptable trade-off, but it should be a documented and deliberate choice, especially if sensitive categories are ever added to the venue schema.
2.4 Input Validation Gaps โ
๐ก M1 โ GET /venues has no Zod schema for query parameters โ
File: services/api/venues/src/routes/utility.js:53-73
router.get('/', async (req, res, next) => {
const { limit, startAfter, category, city, name, orderBy, order } = req.query;
const result = await listVenues({
limit: limit ? parseInt(limit, 10) : undefined, // no validation
...
});All other routes use Zod schemas to validate inputs before they reach the service layer. This route does not. Problems:
limit="abc"โparseIntreturnsNaN, which propagates toMath.min(Math.max(1, NaN), 100)โNaN, then Firestore may throw or behave unexpectedlyorderBy="__injected__"โ Firestore will reject invalid field names but the error will surface as a 500 rather than a clean 400categoryandcityaccept arbitrary strings that flow into Firestore queries without sanitization
Recommendation: Add a Zod schema for all query params, matching the pattern used in the /nearby route.
๐ก M7 โ Admin endpoints accept unvalidated body parameters โ
File: services/api/venues/src/routes/admin.js:30-31
const dryRun = req.body.dryRun !== false;
const daysInactive = req.body.daysInactive || 90;daysInactive is used directly in a date calculation:
cutoffDate.setDate(cutoffDate.getDate() - daysInactive);If daysInactive is a string, the arithmetic will produce unexpected results. If it's 0 or negative, all venues become "orphaned." No upper bound prevents purging recent venues.
Recommendation: Add a Zod schema: daysInactive: z.number().int().min(1).max(3650).default(90).
3. Bug Findings โ
3.1 Venue API Service โ
๐ก M2 โ getVenuesNeedingEnrichment query may miss unenriched venues โ
File: services/api/venues/src/services/venue.service.js:178
.where('addressComponents', '==', null)Firestore treats a document with a field explicitly set to null differently from a document that does not have the field at all. OSM import creates venue documents without the addressComponents field (it is simply absent, not null). This query will only find venues where addressComponents was explicitly written as null, missing the majority of unenriched venues where the field is simply absent.
Recommendation: Query for venues where addressComponents does not exist using a sentinel value, or use where('enriched', '==', false) with a dedicated boolean field set at import time.
๐ก M3 โ Cursor pagination re-fetches the cursor document on every page โ
File: services/api/venues/src/services/venue.service.js:304-307
if (startAfter) {
const cursorDoc = await db.collection(VENUES_COLLECTION).doc(startAfter).get();
if (cursorDoc.exists) {
query = query.startAfter(cursorDoc);Each paginated request fetches the cursor document from Firestore before executing the actual query. This doubles the read cost for every page after the first. On a busy endpoint, this adds up to meaningful additional Firestore read operations.
Recommendation: Pass the document snapshot value (e.g., the orderBy field value) in the cursor instead of the document ID, allowing startAfter(value) without an extra read. Alternatively, encode the cursor as a base64 JSON of the cursor field values.
โน๏ธ L5 โ No max-length check on Overpass query before encoding โ
File: services/api/venues/src/services/osm.service.js (fetchOverpassWithRetry)
Overpass QL queries are constructed from tier filter arrays and passed directly to encodeURIComponent(). For large radii or many tiers, the query string could exceed HTTP limits or Overpass's own query size limits. The error would manifest as a network failure rather than a clear validation error.
Recommendation: Add a query string length check (e.g., reject if > 64KB) before the HTTP call.
3.2 Client-Side โ
โน๏ธ L4 โ Module-scoped deduplication Sets are cleared on HMR in development โ
File: apps/web/src/lib/venueService.js (around line 945)
const enrichedThisSession = new Set()
const enrichmentInProgress = new Set()These module-level Sets persist across re-renders but are cleared when Vite's Hot Module Replacement reloads the module during development. After an HMR reload, venues that were already enriched will re-trigger enrichment calls until the Sets are repopulated. The code has a dev log noting this, which is the right acknowledgment โ just documenting it here for completeness.
No change needed โ acknowledged in code, non-issue in production.
๐ก (Naming concern) โ isCoalTierCategory uses raw OSM tag values, not Lantern category names โ
File: packages/shared/venues/index.js:303-307
export function isCoalTierCategory(category) {
if (!category) return false
const c = category.toLowerCase()
return COAL_TIER_FILTERS.amenity.includes(c) || COAL_TIER_FILTERS.leisure.includes(c)
}COAL_TIER_FILTERS contains raw OSM tag values (e.g., 'dog_park', 'playground'). The function name implies it checks Lantern's internal category names (e.g., 'coffee_shop', 'gym'), but it actually checks OSM tags. This works correctly because the Firestore category field stores the raw OSM category from the import pipeline โ but normalizeCategory() in osm.service.js maps some tags (e.g., 'cafe' โ 'coffee_shop'), creating a subtle gap: a category that passed through normalizeCategory() may no longer match the raw OSM tag in COAL_TIER_FILTERS.
In practice the coal-tier categories don't overlap with normalizeCategory()'s mapping, so no venues are currently misclassified. But the assumption is fragile.
Recommendation: Document the expected input type on the function (raw OSM tag), add a test case, or rename to isCoalTierOsmTag.
3.3 Refresh Threshold Alignment โ
File: packages/shared/venues/index.js:268-277 vs apps/web/src/lib/venueRefreshService.js
The shared config defines three thresholds:
export const VENUE_REFRESH_THRESHOLDS = {
minimumRefreshDays: 14, // Minimum interval between API calls
moderateStaleDays: 30, // Refresh in background, return cached
veryStaleDays: 90, // Block and wait for fresh data
}The comment on moderateStaleDays says "refresh in background" but the client's venueRefreshService.js uses VERY_STALE_DAYS = 90 as the blocking threshold and MODERATE_STALENESS_DAYS = 30 as the background threshold. These values match. However, the shared config comment on minimumRefreshDays says "rate limiting across all users" โ the client uses this as the fresh/background boundary, not as a minimum API call interval. The naming and description should be verified against client usage to prevent future misalignment.
No action required immediately โ values align in practice. Worth a documentation clarification.
4. Privacy Concerns โ
4.1 Venue Data Sensitivity โ
All venue data originates from OpenStreetMap (public domain) or user-reported closures. No Lantern user PII is stored in venue documents. The fields that could be considered sensitive:
| Field | Source | Sensitivity | Notes |
|---|---|---|---|
lat/lng | OSM | None | Public map data |
phoneNumber | OSM | Low | Business phone, not personal |
website | OSM | None | Public |
email | OSM | Low | Business contact |
instagram/facebook | OSM | None | Public handles |
activeLanternCount | Lantern runtime | Low | Reveals real-time venue popularity |
addressComponents | Nominatim (enriched) | None | Street addresses of businesses |
The main privacy concern is activeLanternCount as a behavioral signal: an aggregated count of people who have "lit a lantern" at a venue, visible in real time to anyone (no auth required). This is an acceptable product trade-off, but venues with high counts could attract unwanted attention or be targeted for competitive analysis.
4.2 Closure Reports โ
The venueClosureReports collection stores reportedBy: uid (the Firebase user UID). This links a specific account to a specific venue at a specific time. The rule correctly restricts read access to admins and the reporter themselves. No additional concern.
One gap: There is no client-side deduplication check before submitting a closure report. A user can submit multiple reports for the same venue. This isn't a privacy issue but creates noise in the admin review queue.
Recommendation: Before creating a new report, check if the user already has a pending report for the same venue and surface a notice rather than a duplicate submission.
4.3 localStorage Cache โ
Venue objects (stripped of non-serializable icon fields) are persisted to localStorage under the key lantern_venue_cache. On shared devices (family computer, public kiosk), this leaks the last user's browsed venues to the next user. The data is low-sensitivity (public OSM venues), but the browsing location pattern is meaningful.
Recommendation: Add a note in the privacy policy. Consider clearing the cache on explicit logout.
4.4 Nominatim Enriched Addresses โ
When POST /venues/refresh/enrich/:venueId runs, the full addressComponents object from Nominatim is written to the venue document in Firestore. These are business addresses (not user addresses), publicly queryable. Acceptable.
5. Performance & Optimization โ
5.1 Rate Limiting at Scale โ
Issue: See C3 above. Beyond correctness, the in-memory implementation has a specific Nominatim risk. The nominatim source limit is 1 request / 1100ms. If Cloud Run scales to 2+ instances under load, each instance tracks its own window independently, potentially sending 2 requests/second to Nominatim. Nominatim's fair-use policy may result in an IP block if this occurs frequently.
Short-term mitigation: Set --max-instances=1 on the Cloud Run service until distributed rate limiting is implemented.
5.2 Geohash Query Performance โ
The findNearbyVenues() function in venue.service.js runs multiple Firestore geohash range queries in parallel (one per bounding box segment), then merges and deduplicates results. This is the standard geofire-common pattern and performs well for typical use cases.
However, the client's venueService.js performs name/search filtering after Firestore returns results:
// Client: apps/web/src/lib/venueService.js (around line 323-327)
if (searchQuery) {
filtered = filtered.filter(v =>
v.searchTerms?.some(t => t.includes(q))
)
}For large venue sets, this client-side scan is fine. For future server-side list endpoints, the GET /venues?name=... filter in utility.js performs the same client-side scan in listVenues() (venue.service.js). Firestore doesn't support case-insensitive prefix search natively, so this is a reasonable approach โ but it means the server must fetch all matching-category venues before filtering by name.
No immediate action needed โ document the limitation in the venue service if full-text search becomes a requirement.
5.3 Pagination Cursor Cost โ
See M3 above. At current scale this is unlikely to matter, but is worth fixing before the service sees production traffic.
5.4 Sequential Nominatim Prefetch โ
The client's prefetchVenueAddresses() in venueService.js queues Nominatim enrichment calls with 1100ms delays between each. This is correct behavior to respect Nominatim's 1 req/sec policy, but it means prefetching 10 venues takes ~11 seconds. The Cloud Run /venues/refresh/batch endpoint exists specifically for this use case and respects the same rate limiting server-side.
Optimization opportunity: Route prefetch calls through the batch enrichment API rather than individual Cloud Function calls, consolidating rate limit tracking to the server.
5.5 Missing Observability โ
| Gap | Impact |
|---|---|
| No Cloud Monitoring custom metrics | Can't alert on high import failure rates or Nominatim errors |
| No Cloud Trace integration | Can't measure latency breakdown across Overpass โ Firestore |
No HEALTHCHECK in Dockerfile | Cloud Run falls back to TCP checks; unhealthy pod may serve traffic longer |
Recommendation (Low priority): Add a HEALTHCHECK to the Dockerfile. Add a /metrics endpoint or structured metric logging for import success/failure rates.
6. Code Quality & Architecture โ
6.1 Strengths โ
- Clean layering: Routes never touch Firestore directly โ all DB operations go through
venue.service.js. No business logic in route handlers. - Zod validation coverage: ~80% of endpoints validate inputs with Zod schemas. Error responses include field-level details.
- Structured logging: Pino + pino-http throughout. All errors include
uidin context for audit tracing. - Shared config:
@lantern/shared/venuesand@lantern/shared/servicesprevent duplication between the API service and the web client. Tier filters, category maps, CORS origins, and rate limit values are all centralized. - OpenAPI spec: All 21 endpoints documented with request/response schemas and examples. Scalar API reference mounted at
/api-docs. - Docker security: Non-root
nodeuser, production deps only, Node 22 slim base. - Dry-run defaults: Destructive admin operations default to
dryRun: trueโ an operator must explicitly opt into execution.
6.2 Concerns โ
๐ก M5 โ openapi.json hardcodes the dev Cloud Run URL โ
File: services/api/venues/openapi.json (line 14)
"url": "https://venue-api-531553779372.us-central1.run.app"This URL is the dev deployment. When the service is promoted to production, the spec will point to the wrong environment. The URL should be injected at build time or derived from the service registry in @lantern/shared/services.
Recommendation: Replace the hardcoded URL with a template variable ({serverUrl}) and inject it during deployment, or generate the OpenAPI spec from the service registry.
๐ก M6 โ No .env.example file for the venue-api service โ
File: services/api/venues/ (missing)
The service requires FIREBASE_PROJECT_ID and uses NODE_ENV to gate dev-only behavior. There is no .env.example or documentation of required environment variables. A new developer or a new deployment environment has no obvious reference.
Recommendation: Create services/api/venues/.env.example:
# Required: Firebase project ID for Admin SDK initialization
FIREBASE_PROJECT_ID=lantern-app-dev
# Set to "production" in Cloud Run to suppress debug output
NODE_ENV=development
# HTTP port (Cloud Run sets this automatically)
PORT=8080โน๏ธ L6 โ Venue API has no production URL in the service registry โ
File: packages/shared/services/index.js:21
prodUrl: null,The admin portal API Reference page auto-discovers services from this registry. Once a production deployment exists, this should be updated so the admin can test against prod from the portal.
Reminder: Update before prod launch.
6.3 Test Coverage โ
| Module | Unit Tests | Notes |
|---|---|---|
osm.service.js | โ None | Core import logic, Overpass query building, lifecycle detection |
nominatim.service.js | โ None | Address parsing, enrichment pipeline |
venue.service.js | โ None | Firestore CRUD, geohash queries, pagination |
rateLimiter.js | โ None | Sliding window logic, edge cases |
auth.js (middleware) | โ None | Token extraction, error handling |
rbac.js (middleware) | โ None | Role check, default fallback |
venueApiClient.js | โ None | HTTP wrapper, auth injection, error handling |
venueCacheManager.js | โ None | Cache validation, location check, serialization |
venueRefreshService.js | โ None | Staleness checks, trigger logic |
The venueService.test.js in apps/web/src/lib/__tests__/ tests calculateDistance() and formatDistance() only. No integration tests exist for the Cloud Run endpoints.
The absence of server-side tests is the most significant pre-production gap. A regression in osm.service.js (e.g., the filter for coal-tier venues silently breaking) would import excluded venue types into the database with no automated detection.
Minimum recommended test targets before prod launch:
osm.service.jsโisLikelyClosed(),normalizeCategory(),buildVenueFromOSM(), coal-tier exclusionrateLimiter.jsโ limit enforcement, window cleanup, multi-user isolationvenue.service.jsโgetExistingOsmIds()deduplication,importVenuesToFirestore()batch logic- Route-level tests โ at least one happy-path + one error-path test per route
7. Firestore Security Rules โ Deep Dive โ
| Collection | Read | Write | Assessment |
|---|---|---|---|
venues | true (public) | isAuthenticated() + field checks | โ ๏ธ C1, C2 โ see above |
venueRefreshMetadata | true (public) | isAuthenticated() | โ ๏ธ H1 โ any auth user can write |
venueClosureReports | Admin or own | isAuthenticated() + field validation | โ Correct |
Summary of venue rule issues โ
C1 โ OSM source allowed client-side:request.resource.data.source in ['openstreetmap', 'manual', 'google_places'] permits a raw Firestore write with source: 'openstreetmap', bypassing the Cloud Run service entirely.
C2 โ activeLanternCount freely writable: The hasOnly(['activeLanternCount', 'updatedAt']) branch has no value constraint. Set to any integer.
H1 โ venueRefreshMetadata freely writable:allow write: if isAuthenticated() โ no restriction to Admin SDK or Cloud Functions. Client code currently only reads this collection, so the write permission is unnecessarily broad.
Positive notes:
- Catch-all deny rule at the bottom is correct and prevents accidental exposure of new collections.
venueClosureReportsrules are well-designed: status must be'pending'on create,reportedBymust match auth UID, reason must be in the allowed enum.venuescannot be deleted (allow delete: if false), preserving data integrity.
8. End-to-End Flow Audit โ
8.1 OSM Import (Happy Path) โ โ
User geolocation change
โ venueRefreshService.checkStaleness(geohashPrefix)
โ Reads venueRefreshMetadata/{prefix} from Firestore
โ If stale: venueRefreshService.triggerAreaRefresh()
โ Sets inProgress: true in venueRefreshMetadata
โ venueApiClient.importVenuesFromApi(lat, lng, radius, tiers)
โ POST /venues/import/osm
โ verifyFirebaseToken โ
โ Zod validation (lat/lng ranges, radius 100-50000m) โ
โ rateLimitMiddleware('import') (5 req/min per user) โ
โ fetchOSMVenues()
โ checkSourceLimit('osm') โ
โ fetchOverpassWithRetry() (3 retries, 3 endpoint fallbacks) โ
โ buildVenueFromOSM() for each element
โ isLikelyClosed() check โ
โ Coal-tier exclusion โ
โ importVenuesToFirestore()
โ getExistingOsmIds() (deduplication) โ
โ Batch writes (500-op chunks) โ
โ updateRefreshMetadata() โ
โ { success, imported, skipped, total }
โ Sets inProgress: false
โ Fresh venues appear in next geohash query8.2 Error Path Analysis โ
| Scenario | Behavior | Assessment |
|---|---|---|
| Overpass API timeout | Retry with exponential backoff (1s, 2s, 4s), 3 endpoint fallbacks | โ Resilient |
| Overpass all endpoints down | Returns HTTP 500 to client | โ Clean failure |
| Nominatim unavailable | Venue stored without address, enrichment skipped | โ Graceful degradation |
| Nominatim rate limit hit | Returns 429 with retryAfterMs, client retries | โ Good |
| Firebase Admin write failure | Returns HTTP 500, not retried by client | โ ๏ธ Client should surface this clearly |
| User hits import rate limit | Returns 429 with retryAfterMs | โ Good |
| Invalid coordinates | Zod rejects with 400 VALIDATION_ERROR | โ Good |
| inProgress flag not cleared on error | updateRefreshMetadata called in finally-like pattern | โ Good |
One gap: If the Cloud Run service crashes mid-import (pod restart, OOM), inProgress remains true in Firestore. The client code has a 5-minute timeout after which inProgress is ignored, which is a reasonable mitigation.
9. Prioritized Findings โ
Critical โ Pre-production blockers โ
| ID | Finding | File | Impact |
|---|---|---|---|
| C1 | Client-side venue CREATE with source: 'openstreetmap' still allowed | firestore.rules:291 | Spam/flood of fake OSM venues |
| C2 | activeLanternCount writable to any value by any auth user | firestore.rules:308-311 | Data integrity, popularity manipulation |
| C3 | In-memory rate limiting fails under multi-instance Cloud Run | rateLimiter.js | Nominatim IP ban risk, rate limit bypass |
High โ Address before scaling โ
| ID | Finding | File | Impact |
|---|---|---|---|
| H1 | Any auth user can write venueRefreshMetadata (lock manipulation) | firestore.rules:397 | API abuse, DoS via lock poisoning |
| H2 | RBAC silently defaults to 'user' when role field absent | rbac.js:33 | Masks misconfigured accounts |
| H3 | Unauthenticated requests bypass user rate limiting | rateLimiter.js:119 | DoS vector on protected routes |
| H4 | No unit or integration tests for the venue-api service | โ | Silent regressions in import/enrichment |
Medium โ Address in next sprint โ
| ID | Finding | File | Impact |
|---|---|---|---|
| M1 | GET /venues has no Zod query param validation | utility.js:53-73 | Malformed inputs reach Firestore |
| M2 | getVenuesNeedingEnrichment may miss unenriched venues | venue.service.js:178 | Enrichment silently skipped |
| M3 | Cursor pagination re-fetches cursor doc each page | venue.service.js:304-307 | Extra Firestore read per page |
| M4a | Error message exposes current user role | rbac.js:39 | Minor info leakage |
| M4b | "User not found" message leaks account existence | rbac.js:28 | Minor info leakage |
| M5 | Dev Cloud Run URL hardcoded in openapi.json | openapi.json (line 14) | Config drift on prod deploy |
| M7 | Admin cleanup endpoint body params unvalidated | admin.js:30-31 | Unexpected behavior with bad inputs |
Low โ Address when convenient โ
| ID | Finding | File | Impact |
|---|---|---|---|
| L1 | No HEALTHCHECK directive in Dockerfile | Dockerfile | Slower unhealthy pod detection |
| L2 | No Cloud Monitoring metrics or distributed tracing | โ | Limited production observability |
| L3 | In-memory venue cache uses FIFO eviction (LRU preferred) | venueCacheManager.js | Lower cache hit rates |
| L4 | Module-scoped Sets cleared on HMR in dev (acknowledged) | venueService.js | Dev-only inconvenience |
| L5 | Overpass query has no max-length validation | osm.service.js | Obscure edge case on very large radii |
| L6 | prodUrl: null for venue-api in service registry | services/index.js:21 | Admin API reference incomplete |
10. Recommendations โ
Immediate โ Before production launch โ
Fix
activeLanternCountFirestore rule (C2) Remove thehasOnly(['activeLanternCount', 'updatedAt'])client update branch fromvenues. Lantern count mutations must go through Cloud Functions (Admin SDK) exclusively.Restrict client-side venue CREATE to
source: 'manual'only (C1) Updatefirestore.rules:305torequest.resource.data.source == 'manual'. This closes the client pathway that Issue #224 intended to address.Add Zod validation to
GET /venues(M1) Add a schema matching thelistVenues()parameter types. This is a 15-minute change that prevents unexpected Firestore errors.Add Zod validation to admin endpoints (M7) Add schemas for
daysInactive,limit, anddryRuninadmin.js.Set
--max-instances=1on Cloud Run as a temporary mitigation for C3 Until distributed rate limiting is implemented, prevent scale-out to avoid exceeding Nominatim's 1 req/sec policy.
Short-term โ Next sprint โ
Restrict
venueRefreshMetadatawrites to Admin SDK (H1) Removeallow write: if isAuthenticated()and rely solely on the Admin SDK (Cloud Run) for writes.Add unit tests for the venue-api service (H4) Minimum:
osm.service.js(coal exclusion, lifecycle detection),rateLimiter.js(limit enforcement),venue.service.js(deduplication).Fix RBAC role-absent behavior (H2) Return
403 FORBIDDENwhenrolefield is absent rather than defaulting to'user'.Fix cursor pagination (M3) Pass cursor field values directly to
startAfter()to eliminate the extra Firestore read per page.Add
.env.examplefor venue-api (M6) DocumentFIREBASE_PROJECT_ID,NODE_ENV, andPORT.
Medium-term โ
Distributed rate limiting (C3 full fix) Replace
sourceWindowsanduserWindowsMaps with Firestore counters or Memorystore (Redis) before scaling beyond one instance.Fix
getVenuesNeedingEnrichmentquery (M2) Add anenriched: booleanfield to venue documents at import time and query onwhere('enriched', '==', false).Add Cloud Monitoring metrics and HEALTHCHECK (L1, L2) Emit custom metrics for import success/failure rates, Nominatim errors, and rate limit hits. Add Dockerfile
HEALTHCHECK.Update
prodUrlin service registry when prod is deployed (L6) One-line change when the production Cloud Run service URL is known.
Appendix: Endpoint Reference โ
| Method | Route | Auth | Rate Limit | Status |
|---|---|---|---|---|
| GET | /health | None | โ | โ |
| GET | /openapi.json | None | โ | โ |
| GET | /api-docs | None | โ | โ |
| POST | /venues/import/osm | Firebase + User | 5 req/min | โ |
| POST | /venues/import/hybrid | Firebase + User | 5 req/min | 501 Not Implemented |
| POST | /venues/refresh/enrich/:venueId | Firebase + User | 20 req/min | โ |
| POST | /venues/refresh/batch | Firebase + User | 10 req/min | โ |
| POST | /venues/refresh/scheduled | Cloud Scheduler header | โ | โ |
| GET | /venues | Firebase + User | โ | โ ๏ธ M1 |
| GET | /venues/nearby | Firebase + User | โ | โ |
| GET | /venues/categories | Firebase + User | โ | โ |
| POST | /venues/geocode | Firebase + User | 20 req/min | โ |
| GET | /venues/:venueId/metadata | Firebase + User | โ | โ |
| POST | /venues/consolidate | Firebase + User | โ | โ |
| GET | /venues/admin/stats | Firebase + Admin | โ | โ |
| POST | /venues/admin/cleanup/orphaned | Firebase + Admin | โ | โ ๏ธ M7 |
| POST | /venues/admin/validate/all | Firebase + Admin | โ | โ ๏ธ M7 |
| POST | /venues/admin/deduplicate | Firebase + Admin | โ | โ |
End of Venue API QA Audit
Next steps:
- Create GitHub issues for C1, C2, C3 (pre-production blockers) and link to this audit
- Address short-term items (H1โH4) before the next deployment cycle
- Schedule follow-up audit after prod launch