Escape channel metrics literals and bypass website transfer lookup for admins
This commit is contained in:
1 parent
8148d775b0
commit
dab6180056
6 files changed
+81
-31
No files matched your search
@@ -279,12 +279,11 @@ describe('canTransferWebsiteToUser', () => {
|
||||
).resolves.toBe(false);
|
||||
});
|
||||
|
||||
// NOTE: admins are NOT short-circuited here; there is no isAdmin bypass in this function.
|
||||
test('denies an admin transferring a user-owned website (no team)', async () => {
|
||||
vi.mocked(getWebsite).mockResolvedValue({ userId: 'admin-1' } as any);
|
||||
test('allows admins without any lookup', async () => {
|
||||
await expect(
|
||||
canTransferWebsiteToUser({ user: adminUser }, 'website-1', 'admin-1'),
|
||||
).resolves.toBe(false);
|
||||
).resolves.toBe(true);
|
||||
expect(getWebsite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('allows a team owner transferring a team website to themselves', async () => {
|
||||
@@ -316,6 +315,13 @@ describe('canTransferWebsiteToTeam', () => {
|
||||
await expect(canTransferWebsiteToTeam({}, 'website-1', 'team-1')).resolves.toBe(false);
|
||||
});
|
||||
|
||||
test('allows admins without any lookup', async () => {
|
||||
await expect(
|
||||
canTransferWebsiteToTeam({ user: adminUser }, 'website-1', 'team-1'),
|
||||
).resolves.toBe(true);
|
||||
expect(getWebsite).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('denies when website is missing', async () => {
|
||||
vi.mocked(getWebsite).mockResolvedValue(null as any);
|
||||
await expect(
|
||||
|
||||
@@ -83,7 +83,9 @@ export async function canViewBatchWebsites({ user, shareToken }: Auth, websiteId
|
||||
websites.filter(website => website.userId === user.id).map(website => website.id),
|
||||
);
|
||||
const teamIds = Array.from(
|
||||
new Set(websites.map(website => website.teamId).filter((teamId): teamId is string => Boolean(teamId))),
|
||||
new Set(
|
||||
websites.map(website => website.teamId).filter((teamId): teamId is string => Boolean(teamId)),
|
||||
),
|
||||
);
|
||||
const teamUsers = teamIds.length
|
||||
? await prisma.client.teamUser.findMany({
|
||||
@@ -187,6 +189,10 @@ export async function canTransferWebsiteToUser({ user }: Auth, websiteId: string
|
||||
return false;
|
||||
}
|
||||
|
||||
if (user.isAdmin) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const website = await getWebsite(websiteId);
|
||||
|
||||
if (!website) {
|
||||
@@ -207,6 +213,10 @@ export async function canTransferWebsiteToTeam({ user }: Auth, websiteId: string
|
||||
return false;
|
||||
}
|
||||
|
||||
if (user.isAdmin) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const website = await getWebsite(websiteId);
|
||||
|
||||
if (!website) {
|
||||
|
||||
@@ -102,8 +102,13 @@ describe('getChannelExpandedMetrics postgres branch', () => {
|
||||
await getChannelExpandedMetrics('website-1', {});
|
||||
const [query] = prismaRawQuery.mock.calls[0];
|
||||
|
||||
// LIKE wildcards/backslashes are escaped so values match literally (e.g. the
|
||||
// underscores in PAID_AD_PARAMS like `ad_id=` become `ad\_id=`).
|
||||
const escapeLike = (val: string) =>
|
||||
val.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_').replace(/'/g, "''");
|
||||
|
||||
for (const val of PAID_AD_PARAMS) {
|
||||
expect(query).toContain(`url_query ilike '%${val}%'`);
|
||||
expect(query).toContain(`url_query ilike '%${escapeLike(val)}%'`);
|
||||
}
|
||||
for (const domainList of [
|
||||
LLM_DOMAINS,
|
||||
@@ -114,10 +119,22 @@ describe('getChannelExpandedMetrics postgres branch', () => {
|
||||
VIDEO_DOMAINS,
|
||||
]) {
|
||||
for (const val of domainList) {
|
||||
expect(query).toContain(`referrer_domain ilike '%${val}%'`);
|
||||
expect(query).toContain(`referrer_domain ilike '%${escapeLike(val)}%'`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('returns the query rows unchanged without a spurious y column', async () => {
|
||||
const rows = [
|
||||
{ name: 'direct', pageviews: 10, visitors: 4, visits: 5, bounces: 1, totaltime: 100 },
|
||||
];
|
||||
prismaRawQuery.mockResolvedValue(rows);
|
||||
|
||||
const result = await getChannelExpandedMetrics('website-1', {});
|
||||
|
||||
expect(result).toEqual(rows);
|
||||
expect(result[0]).not.toHaveProperty('y');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getChannelExpandedMetrics clickhouse branch', () => {
|
||||
|
||||
@@ -138,7 +138,7 @@ async function relationalQuery(
|
||||
`,
|
||||
queryParams,
|
||||
FUNCTION_NAME,
|
||||
).then(results => results.map(item => ({ ...item, y: Number(item.y) })));
|
||||
);
|
||||
}
|
||||
|
||||
async function clickhouseQuery(
|
||||
@@ -222,9 +222,16 @@ async function clickhouseQuery(
|
||||
}
|
||||
|
||||
function toClickHouseStringArray(arr: string[]): string {
|
||||
return arr.map(p => `'${p.replace(/'/g, "\\'")}'`).join(', ');
|
||||
// Escape backslashes first, then single quotes, for ClickHouse string literals.
|
||||
return arr.map(p => `'${p.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'`).join(', ');
|
||||
}
|
||||
|
||||
function escapePostgresLikeValue(val: string) {
|
||||
// Escape LIKE wildcards/backslashes so the value is matched literally, then
|
||||
// escape single quotes for the surrounding SQL string literal.
|
||||
return val.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_').replace(/'/g, "''");
|
||||
}
|
||||
|
||||
function toPostgresPositionClause(column: string, arr: string[]) {
|
||||
return arr.map(val => `${column} ilike '%${val.replace(/'/g, "''")}%'`).join(' OR\n ');
|
||||
return arr.map(val => `${column} ilike '%${escapePostgresLikeValue(val)}%'`).join(' OR\n ');
|
||||
}
|
||||
@@ -101,11 +101,16 @@ describe('getChannelMetrics postgres branch', () => {
|
||||
await getChannelMetrics('website-1', {});
|
||||
const [query] = prismaRawQuery.mock.calls[0];
|
||||
|
||||
// LIKE wildcards/backslashes are escaped so values match literally (e.g. the
|
||||
// underscores in PAID_AD_PARAMS like `ad_id=` become `ad\_id=`).
|
||||
const escapeLike = (val: string) =>
|
||||
val.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_').replace(/'/g, "''");
|
||||
|
||||
for (const val of PAID_AD_PARAMS) {
|
||||
expect(query).toContain(`url_query ilike '%${val}%'`);
|
||||
expect(query).toContain(`url_query ilike '%${escapeLike(val)}%'`);
|
||||
}
|
||||
for (const val of SEARCH_DOMAINS) {
|
||||
expect(query).toContain(`referrer_domain ilike '%${val}%'`);
|
||||
expect(query).toContain(`referrer_domain ilike '%${escapeLike(val)}%'`);
|
||||
}
|
||||
for (const domainList of [
|
||||
LLM_DOMAINS,
|
||||
@@ -115,7 +120,7 @@ describe('getChannelMetrics postgres branch', () => {
|
||||
VIDEO_DOMAINS,
|
||||
]) {
|
||||
for (const val of domainList) {
|
||||
expect(query).toContain(`referrer_domain ilike '%${val}%'`);
|
||||
expect(query).toContain(`referrer_domain ilike '%${escapeLike(val)}%'`);
|
||||
}
|
||||
}
|
||||
// referral clause uses a literal array, not a constant
|
||||
@@ -132,21 +137,19 @@ describe('getChannelMetrics postgres branch', () => {
|
||||
expect(result).toEqual([{ x: 'direct', y: 42 }]);
|
||||
});
|
||||
|
||||
// NOTE: toPostgresLikeClause only escapes single quotes (' -> ''). LIKE
|
||||
// wildcards (% and _) and backslashes pass through unescaped. This is safe
|
||||
// today because the helper is private and only ever receives the hardcoded
|
||||
// constant arrays, but it would be an escaping gap if user input reached it.
|
||||
// See getChannelMetrics.ts:180-182.
|
||||
test('single-quote escaping doubles quotes (documenting current helper behavior)', async () => {
|
||||
// None of the real constants contain quotes, so assert the round-trip of
|
||||
// the escaping regex against every constant value: no bare `''` artifacts
|
||||
// should appear inside a domain fragment.
|
||||
// toPostgresLikeClause escapes LIKE wildcards (% and _) and backslashes so the
|
||||
// value matches literally, then escapes single quotes for the SQL string
|
||||
// literal. The hardcoded constants contain none of these, so their rendered
|
||||
// form is unchanged; this asserts each constant appears intact in the clause.
|
||||
test('renders an ilike clause for each escaped constant value', async () => {
|
||||
await getChannelMetrics('website-1', {});
|
||||
const [query] = prismaRawQuery.mock.calls[0];
|
||||
|
||||
const escapeLike = (val: string) =>
|
||||
val.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_').replace(/'/g, "''");
|
||||
|
||||
for (const val of [...PAID_AD_PARAMS, ...LLM_DOMAINS, ...SEARCH_DOMAINS]) {
|
||||
const escaped = val.replace(/'/g, "''");
|
||||
expect(query).toContain(`'%${escaped}%'`);
|
||||
expect(query).toContain(`'%${escapeLike(val)}%'`);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -194,15 +197,15 @@ describe('getChannelMetrics clickhouse branch', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// NOTE: toClickHouseStringArray escapes single quotes as \' but leaves
|
||||
// backslashes and other metacharacters untouched (getChannelMetrics.ts:176-178).
|
||||
// Safe today (constants only), documented here as current behavior.
|
||||
test('clickhouse array escaping only handles single quotes', async () => {
|
||||
// toClickHouseStringArray escapes backslashes and single quotes for ClickHouse
|
||||
// string literals. The constants contain neither, so they render unchanged.
|
||||
test('clickhouse array escaping handles backslashes and single quotes', async () => {
|
||||
await getChannelMetrics('website-1', {});
|
||||
const [query] = clickhouseRawQuery.mock.calls[0];
|
||||
|
||||
for (const val of [...VIDEO_DOMAINS, ...SHOPPING_DOMAINS]) {
|
||||
expect(query).toContain(`'${val.replace(/'/g, "\\'")}'`);
|
||||
const escaped = val.replace(/\\/g, '\\\\').replace(/'/g, "\\'");
|
||||
expect(query).toContain(`'${escaped}'`);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -174,9 +174,16 @@ async function clickhouseQuery(
|
||||
}
|
||||
|
||||
function toClickHouseStringArray(arr: string[]): string {
|
||||
return arr.map(p => `'${p.replace(/'/g, "\\'")}'`).join(', ');
|
||||
// Escape backslashes first, then single quotes, for ClickHouse string literals.
|
||||
return arr.map(p => `'${p.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'`).join(', ');
|
||||
}
|
||||
|
||||
function escapePostgresLikeValue(val: string) {
|
||||
// Escape LIKE wildcards/backslashes so the value is matched literally, then
|
||||
// escape single quotes for the surrounding SQL string literal.
|
||||
return val.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_').replace(/'/g, "''");
|
||||
}
|
||||
|
||||
function toPostgresLikeClause(column: string, arr: string[]) {
|
||||
return arr.map(val => `${column} ilike '%${val.replace(/'/g, "''")}%'`).join(' OR\n ');
|
||||
return arr.map(val => `${column} ilike '%${escapePostgresLikeValue(val)}%'`).join(' OR\n ');
|
||||
}
|
||||
Reference in new issue
Block a user