Implement wisp#61
Conversation
- Install @renderdragonorg/wisp v0.1.1 - Initialize wisp SDK in main.tsx with convex URL - Bind supabase auth state to wisp identity in AuthProvider - Add convex backend (schema, events, stats, http, crons, dashboard) - Fix upsertMachine to update meta fields on existing machines - Expose __wisp on window for console debugging
- Add WISP_SECRET auth check on /ingest endpoint with custom SDK transport - Fix ip-api.com geo lookup to use HTTPS instead of HTTP - Fix getMachineStats crash on machines with zero sessions - Fix computeDailyStats to use by_startedAt index instead of filter - Fix searchMachines to use search index instead of full table scan - Add search_machineId index on machines table in schema - Add x-wisp-token to CORS allowed headers
…ctor - Remove PostHogProvider and posthog-js dependency (replaced by wisp analytics) - Remove posthog.captureException from ErrorBoundary - Refactor AdBlockDetector to use generic CDN endpoints instead of PostHog - Fix Radix DialogContent aria-describedby warning by explicitly setting undefined
- Add by_open_lastActivityAt composite index on sessions - closeStaleSessions now uses index for both endedAt eq and lastActivityAt lt, removing the post-filter scan
|
@Coder-soft is attempting to deploy a commit to the yamura3's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR integrates the
Confidence Score: 3/5The ingest pipeline has three overlapping holes that together mean any visitor can write arbitrary analytics data: the shared secret is visible in the browser bundle, the public recordBatch mutation bypasses the HTTP route entirely, and a missing env var leaves the endpoint fully open. Three distinct auth gaps all affect the same ingest path. The VITE_WISP_SECRET issue alone makes the token scheme inoperative, and the public recordBatch mutation provides a second independent bypass. These need to be addressed before the analytics data can be trusted. src/main.tsx (client-exposed secret), convex/http.ts (open-by-default auth), and convex/events.ts (public mutation bypassing auth) all require changes before merging.
|
| Filename | Overview |
|---|---|
| src/main.tsx | Initializes the @renderdragonorg/wisp analytics SDK; VITE_WISP_SECRET is a client-side env var visible in the browser bundle, undermining the token auth scheme. |
| convex/http.ts | HTTP ingest endpoint with optional token auth; auth is bypassed entirely when WISP_SECRET is unset, and IP is forwarded to a third-party geo API. |
| convex/events.ts | Defines recordBatch as a public mutation, allowing direct Convex client calls that bypass the HTTP auth layer; should be internalMutation. |
| convex/dashboard.ts | Dashboard queries with multiple unbounded .collect() calls; getOverview fallback uses .filter() (table scan) instead of the available by_startedAt index; upper timestamp bounds missing in several pageview queries. |
| convex/stats.ts | Cron-triggered internal mutations for closing stale sessions and computing daily stats; logic is sound though uses .collect() which could stress transaction limits at scale. |
| convex/schema.ts | Well-structured schema with appropriate indexes for all query access patterns; no issues found. |
| convex/crons.ts | Defines two cron jobs (session cleanup every 10 min, daily stats at 00:15 UTC); correctly uses internal references and crons.interval/crons.cron methods. |
| src/providers/AuthProvider.tsx | Binds Supabase auth state to Wisp identity via bindSupabase; cleanup is handled correctly in useEffect return. |
| src/components/AdBlockDetector.tsx | New component that probes Google Analytics URLs to detect ad blockers and prompts users; logic is intentional and cleanup is handled. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Browser as Browser (Wisp SDK)
participant Ingest as Convex HTTP /ingest
participant Auth as isAuthorized()
participant Geo as resolveGeo()
participant CF as Cloudflare Headers
participant IPAPI as ip-api.com
participant Mutation as events.recordBatchWithGeo (internal)
participant DB as Convex DB
participant Cron as Cron Jobs
Browser->>Ingest: "POST /ingest {events[]} + x-wisp-token"
Ingest->>Auth: check x-wisp-token vs WISP_SECRET
alt secret unset — always passes
Auth-->>Ingest: true
else token matches
Auth-->>Ingest: true
end
Ingest->>Geo: resolveGeo(request)
Geo->>CF: read cf-ipcountry / cf-region / cf-ipcity
alt CF headers complete
CF-->>Geo: country, region, city
else incomplete
Geo->>IPAPI: "GET /json/{ip}"
IPAPI-->>Geo: country, regionName, city
end
Geo-->>Ingest: "{ip, country, region, city}"
Ingest->>Mutation: "ctx.runMutation(recordBatchWithGeo, {events, geo})"
Mutation->>DB: upsert machines, upsert sessions, insert events
DB-->>Mutation: ok
Mutation-->>Ingest: "{inserted: N}"
Ingest-->>Browser: 204 No Content
loop Every 10 min
Cron->>DB: close stale sessions
end
loop Daily at 00:15 UTC
Cron->>DB: compute and upsert dailyStats row
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Browser as Browser (Wisp SDK)
participant Ingest as Convex HTTP /ingest
participant Auth as isAuthorized()
participant Geo as resolveGeo()
participant CF as Cloudflare Headers
participant IPAPI as ip-api.com
participant Mutation as events.recordBatchWithGeo (internal)
participant DB as Convex DB
participant Cron as Cron Jobs
Browser->>Ingest: "POST /ingest {events[]} + x-wisp-token"
Ingest->>Auth: check x-wisp-token vs WISP_SECRET
alt secret unset — always passes
Auth-->>Ingest: true
else token matches
Auth-->>Ingest: true
end
Ingest->>Geo: resolveGeo(request)
Geo->>CF: read cf-ipcountry / cf-region / cf-ipcity
alt CF headers complete
CF-->>Geo: country, region, city
else incomplete
Geo->>IPAPI: "GET /json/{ip}"
IPAPI-->>Geo: country, regionName, city
end
Geo-->>Ingest: "{ip, country, region, city}"
Ingest->>Mutation: "ctx.runMutation(recordBatchWithGeo, {events, geo})"
Mutation->>DB: upsert machines, upsert sessions, insert events
DB-->>Mutation: ok
Mutation-->>Ingest: "{inserted: N}"
Ingest-->>Browser: 204 No Content
loop Every 10 min
Cron->>DB: close stale sessions
end
loop Daily at 00:15 UTC
Cron->>DB: compute and upsert dailyStats row
end
Comments Outside Diff (1)
-
convex/dashboard.ts, line 944-949 (link)Table scan with
.filter()—by_startedAtindex is available but unusedThe fallback branch of
getOverviewfetches sessions using.filter()onstartedAt, which scans the entiresessionstable. The schema already defines aby_startedAtindex; using.withIndex("by_startedAt", q => q.gte("startedAt", dayStart).lt("startedAt", dayEnd))would read only the rows in range.
Reviews (1): Last reviewed commit: "fix: eliminate filter scan in closeStale..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const options = { | ||
| api_host: import.meta.env.VITE_PUBLIC_POSTHOG_HOST, | ||
| const convexUrl = import.meta.env.VITE_CONVEX_URL; | ||
| const wispSecret = import.meta.env.VITE_WISP_SECRET; |
There was a problem hiding this comment.
Any variable prefixed with VITE_ is inlined into the browser-side JavaScript bundle at build time — it is visible in plain text in DevTools, not encrypted. This means VITE_WISP_SECRET is readable by every user of the app, which completely negates the token-based auth in isAuthorized. An attacker who opens DevTools can copy the value and call /ingest directly with arbitrary payloads.
| function isAuthorized(request: Request): boolean { | ||
| if (!WISP_SECRET) return true; | ||
| return request.headers.get("x-wisp-token") === WISP_SECRET; |
There was a problem hiding this comment.
WISP_SECRET is unset
When WISP_SECRET is not configured as a Convex environment variable, isAuthorized returns true unconditionally — any caller on the internet can POST to /ingest and inject arbitrary session and event data. This is easy to miss in a fresh deployment before the env var is added.
| export const recordBatch = mutation({ | ||
| args: { events: v.array(eventValidator) }, | ||
| handler: async (ctx, { events }) => { | ||
| // Process oldest-first so ordering-dependent bookkeeping (session creation, counters) is correct. | ||
| const sorted = [...events].sort((a, b) => a.timestamp - b.timestamp); | ||
|
|
||
| for (const event of sorted) { | ||
| const isBookkeeping = event.name === "session_start"; | ||
|
|
||
| await upsertMachine(ctx, { | ||
| machineId: event.machineId, | ||
| userId: event.userId, | ||
| timestamp: event.timestamp, | ||
| meta: isBookkeeping ? event.payload : undefined, | ||
| }); | ||
|
|
||
| await upsertSession(ctx, { | ||
| sessionId: event.sessionId, | ||
| machineId: event.machineId, | ||
| userId: event.userId, | ||
| timestamp: event.timestamp, | ||
| url: event.url, | ||
| isError: event.type === "error", | ||
| }); | ||
|
|
||
| await ctx.db.insert("events", { | ||
| sessionId: event.sessionId, | ||
| machineId: event.machineId, | ||
| userId: event.userId, | ||
| type: event.type, | ||
| name: event.name, | ||
| payload: event.payload, | ||
| url: event.url, | ||
| timestamp: event.timestamp, | ||
| }); | ||
| } | ||
|
|
||
| return { inserted: sorted.length }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
recordBatch is a public mutation — bypasses the HTTP auth layer entirely
The HTTP /ingest route calls the internal mutation recordBatchWithGeo, which is correctly gated behind isAuthorized. However, recordBatch is exported as a public Convex mutation, meaning any Convex client (the SDK, convex dev console, a script) can call api.events.recordBatch directly without going through the HTTP route — no token needed. This lets anyone inject arbitrary events. Changing it to internalMutation would close the gap.
| async function countPageViews(ctx: QueryCtx, dayStart: number, dayEnd: number): Promise<number> { | ||
| const pvs = await ctx.db | ||
| .query("events") | ||
| .withIndex("by_type_time", (q) => q.eq("type", "pageview").gte("timestamp", dayStart)) | ||
| .collect(); | ||
| return pvs.filter((e) => e.timestamp < dayEnd).length; | ||
| } |
There was a problem hiding this comment.
countPageViews (and the same pattern in getTopPages, getPageVisitors, getPageViewsOverTime) queries with only .gte("timestamp", dayStart) in the index and then discards rows where timestamp >= dayEnd in JavaScript. This reads every pageview from dayStart to the present. Adding .lte("timestamp", dayEnd) inside the withIndex callback would limit the read to the target range.
| if (ip) { | ||
| try { | ||
| const res = await fetch( | ||
| `https://ip-api.com/json/${ip}?fields=country,regionName,city`, | ||
| { signal: AbortSignal.timeout(2000) } | ||
| ); | ||
| if (res.ok) { | ||
| const data = await res.json() as { country?: string; regionName?: string; city?: string }; | ||
| return { | ||
| ip, | ||
| country: cfCountry ?? data.country ?? undefined, | ||
| region: cfRegion ?? data.regionName ?? undefined, | ||
| city: cfCity ?? data.city ?? undefined, | ||
| }; | ||
| } | ||
| } catch { | ||
| // Timeout or network error — return Cloudflare data (even partial) silently | ||
| } | ||
| } |
There was a problem hiding this comment.
The extracted IP address (which may include the visitor's real IP via cf-connecting-ip) is sent to ip-api.com in the URL path, which is logged by that service. Depending on the applicable jurisdiction and the site's privacy policy, routing user IP addresses to an external API without disclosure may be a compliance concern (GDPR, CCPA).
- getOverview fallback: use by_startedAt index instead of filter - countPageViews: add .lt(timestamp, dayEnd) to index seek - getTopPages: add .lt(timestamp, dayEnd) to index seek, remove post-filter - getPageVisitors: add .lt(timestamp, dayEnd) to index seek - getPageViewsOverTime: add .lt(timestamp, dayEnd) to index seek All pageview queries now constrain both bounds at the index level, eliminating post-filter full scans.
2 participants
No description provided.