-
Notifications
You must be signed in to change notification settings - Fork 247
Feature/performance #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feature/performance #219
Changes from 13 commits
7ac5e4c
c215ca1
567a278
1ccf85f
945881c
21d2c89
33d80e2
77d4c24
79c48db
274c424
cc124bc
8d61e19
01837a9
d67356c
06977ac
75f2109
59ba06d
c20b1f9
99c2ed4
c3e6ed8
b8d6d8c
3f0817e
c192d45
4e901cc
c4e54b3
36f57d2
f617a13
d05ad80
ed5d8ac
83c6c2f
b51692d
1e4b1a7
46d7978
dbb4657
9086fb5
560df69
a327b96
b2df2c2
3363147
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -323,8 +323,18 @@ async function identify({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| projectId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| properties: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(payload.properties ?? {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(geo ?? {}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...uaInfo, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| country: geo.country, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| city: geo.city, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| region: geo.region, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| longitude: geo.longitude, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| latitude: geo.latitude, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| os: uaInfo.os, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| os_version: uaInfo.osVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| browser: uaInfo.browser, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| browser_version: uaInfo.browserVersion, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| device: uaInfo.device, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| brand: uaInfo.brand, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model: uaInfo.model, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+303
to
315
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid persisting internal (“__”) properties into profile.properties. You spread payload.properties before UA/geo. This may store transient/private keys like __ip, __timestamp, os, etc. Strip keys starting with "" before persisting. Apply: properties: {
- ...(payload.properties ?? {}),
+ ...Object.fromEntries(
+ Object.entries(payload.properties ?? {}).filter(([k]) => !k.startsWith('__')),
+ ),
country: geo.country,
city: geo.city,
region: geo.region,
longitude: geo.longitude,
latitude: geo.latitude,
os: uaInfo.os,
os_version: uaInfo.osVersion,
browser: uaInfo.browser,
browser_version: uaInfo.browserVersion,
device: uaInfo.device,
brand: uaInfo.brand,
model: uaInfo.model,
},📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ import type { FastifyRequest, RawRequestDefaultExpression } from 'fastify'; | |||||||||||||||||||||||||||||||||||||
| import { verifyPassword } from '@openpanel/common/server'; | ||||||||||||||||||||||||||||||||||||||
| import type { IServiceClientWithProject } from '@openpanel/db'; | ||||||||||||||||||||||||||||||||||||||
| import { ClientType, getClientByIdCached } from '@openpanel/db'; | ||||||||||||||||||||||||||||||||||||||
| import { getCache } from '@openpanel/redis'; | ||||||||||||||||||||||||||||||||||||||
| import type { PostEventPayload, TrackHandlerPayload } from '@openpanel/sdk'; | ||||||||||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||||||||||
| IProjectFilterIp, | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -131,7 +132,13 @@ export async function validateSdkRequest( | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (client.secret && clientSecret) { | ||||||||||||||||||||||||||||||||||||||
| if (await verifyPassword(clientSecret, client.secret)) { | ||||||||||||||||||||||||||||||||||||||
| const isVerified = await getCache( | ||||||||||||||||||||||||||||||||||||||
| `client:auth:${clientId}:${clientSecret.slice(0, 5)}`, | ||||||||||||||||||||||||||||||||||||||
| 60 * 5, | ||||||||||||||||||||||||||||||||||||||
| async () => await verifyPassword(clientSecret, client.secret!), | ||||||||||||||||||||||||||||||||||||||
| true, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| if (isVerified) { | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+135
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security: auth bypass risk — cache key uses only secret prefix. Keying by clientId + clientSecret.slice(0, 5) can return a cached true for a different secret with the same 5-char prefix. Use a non-reversible fingerprint of the full secret instead (e.g., HMAC or SHA‑256). Apply this diff: @@
-import { getCache } from '@openpanel/redis';
+import { getCache } from '@openpanel/redis';
+import { createHash } from 'node:crypto';
@@
- const isVerified = await getCache(
- `client:auth:${clientId}:${clientSecret.slice(0, 5)}`,
- 60 * 5,
- async () => await verifyPassword(clientSecret, client.secret!),
- true,
- );
+ const secretFp = createHash('sha256')
+ .update(clientSecret)
+ .digest('hex')
+ .slice(0, 32); // short, non-reversible fingerprint
+ const isVerified = await getCache(
+ `client:auth:${clientId}:${secretFp}`,
+ 60 * 5,
+ async () => verifyPassword(clientSecret, client.secret!),
+ true,
+ );Optionally prefer HMAC with a server-side key for stronger preimage resistance:
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| return client; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanitize profile.properties: drop “__*” keys before upsert.
Same concern as track.controller: don’t persist internal/transient keys from payload.properties.
properties: { - ...(payload.properties ?? {}), + ...Object.fromEntries( + Object.entries(payload.properties ?? {}).filter(([k]) => !k.startsWith('__')), + ), country: geo.country, city: geo.city, region: geo.region, longitude: geo.longitude, latitude: geo.latitude, os: uaInfo.os, os_version: uaInfo.osVersion, browser: uaInfo.browser, browser_version: uaInfo.browserVersion, device: uaInfo.device, brand: uaInfo.brand, model: uaInfo.model, },🤖 Prompt for AI Agents