diff --git a/README.md b/README.md index 1ccbb5a..a445751 100644 --- a/README.md +++ b/README.md @@ -389,7 +389,7 @@ List all user segments. | Option | Description | |---|---| | `--title` | Segment title | -| `--filters` | JSON array of canonical `{field,op,value}` filter objects | +| `--filters` | JSON array of canonical `{field,op,value}` filter objects. Array string members cannot contain `|` | ### `segments delete ` Delete a user segment. @@ -421,7 +421,7 @@ Pre-built analytics pipes — the same data that powers the Formo dashboard — |---|---| | `--date-from` | Inclusive start date `YYYY-MM-DD` (default: 7 days before `--date-to`) | | `--date-to` | Inclusive end date `YYYY-MM-DD` (default: today) | -| `--filters` | JSON array of `[{field,op,value}]`. For `in`/`nin`, array values are preferred; pipe-delimited strings are also accepted | +| `--filters` | JSON array of `[{field,op,value}]`. For `in`/`nin`, array values are preferred; pipe-delimited strings are also accepted. Array string members cannot contain `|` | | `--params` | JSON object of pipe-specific params merged into the query (e.g. `{"limit":10,"group_by":"device"}`) | ```bash diff --git a/SKILLS.md b/SKILLS.md index 311a25b..85acf52 100644 --- a/SKILLS.md +++ b/SKILLS.md @@ -203,7 +203,7 @@ formo analytics [options] |---|---| | `--date-from` | Inclusive start date `YYYY-MM-DD` (default: 7 days before `--date-to`) | | `--date-to` | Inclusive end date `YYYY-MM-DD` (default: today) | -| `--filters` | JSON array of `[{field,op,value}]`. For `in`/`nin`, array values are preferred; pipe-delimited strings are also accepted | +| `--filters` | JSON array of `[{field,op,value}]`. For `in`/`nin`, array values are preferred; pipe-delimited strings are also accepted. Array string members cannot contain `|` | | `--params` | JSON object of pipe-specific params merged into the query (e.g. `{"limit":10,"group_by":"device"}`) | **Examples:** @@ -575,7 +575,7 @@ formo segments create --title --filters '<json>' | Option | Description | |---|---| | `--title` | Segment title | -| `--filters` | JSON array of canonical `{field,op,value}` filter objects | +| `--filters` | JSON array of canonical `{field,op,value}` filter objects. Array string members cannot contain `|` | > Requires `segments:write` scope. diff --git a/src/commands/analytics.ts b/src/commands/analytics.ts index 635b584..2209e22 100644 --- a/src/commands/analytics.ts +++ b/src/commands/analytics.ts @@ -1,5 +1,11 @@ import { Cli, z } from 'incur' import { createClient, requireApiKey } from '../lib/client' +import { + hasTinybirdMembershipDelimiter, + isCanonicalFilterOperator, + isCanonicalFilterValue, + isValuelessFilterOperator, +} from '../lib/filters' import { parseJsonObject } from '../lib/json' export const analytics = Cli.create('analytics', { @@ -51,6 +57,71 @@ const RESERVED_PARAM_KEYS = new Set([ 'filters', ]) +const ANALYTICS_FILTER_KEYS = new Set(['field', 'op', 'value', 'filters']) +const ANALYTICS_NESTED_FILTER_KEYS = new Set(['field', 'op', 'value']) + +function validateAnalyticsFilter( + filter: unknown, + path: string, + allowNested: boolean, +): void { + if (!filter || typeof filter !== 'object' || Array.isArray(filter)) { + throw new Error(`${path} must be a {field, op, value} object`) + } + + const record = filter as Record<string, unknown> + if (!allowNested && record.filters !== undefined) { + throw new Error(`${path}.filters must be a one-level array of leaf filters`) + } + const allowedKeys = allowNested + ? ANALYTICS_FILTER_KEYS + : ANALYTICS_NESTED_FILTER_KEYS + if (Object.keys(record).some((key) => !allowedKeys.has(key))) { + throw new Error( + `${path} may only contain field, op, value${allowNested ? ', and filters' : ''}`, + ) + } + if (typeof record.field !== 'string' || record.field.length === 0) { + throw new Error(`${path} requires a non-empty string "field"`) + } + if (!isCanonicalFilterOperator(record.op)) { + throw new Error(`${path} requires a canonical "op"`) + } + if ( + !isValuelessFilterOperator(record.op) && + (record.value === undefined || + record.value === null || + !isCanonicalFilterValue(record.value)) + ) { + throw new Error( + `${path}: "value" is required for every operator except notEmpty/isEmpty`, + ) + } + if ( + record.value !== undefined && + record.value !== null && + !isCanonicalFilterValue(record.value) + ) { + throw new Error( + `${path}: "value" must be a string, number, boolean, or string/number array`, + ) + } + if (hasTinybirdMembershipDelimiter(record.value)) { + throw new Error( + `${path}: array string members cannot contain "|" because it is the Tinybird membership separator`, + ) + } + + if (record.filters !== undefined) { + if (!allowNested || !Array.isArray(record.filters)) { + throw new Error(`${path}.filters must be a one-level array of leaf filters`) + } + record.filters.forEach((nested, index) => + validateAnalyticsFilter(nested, `${path}.filters[${index}]`, false), + ) + } +} + /** * Build the query-string params for an analytics pipe request. * @@ -106,6 +177,9 @@ export function buildAnalyticsParams( '--filters must be a valid JSON array of {field,op,value} objects', ) } + parsed.forEach((filter, index) => + validateAnalyticsFilter(filter, `--filters[${index}]`, true), + ) out.filters = JSON.stringify(parsed) } @@ -132,7 +206,7 @@ const sharedOptions = z.object({ .optional() .describe( 'JSON array of filter conditions: [{"field","op","value"}]. ' + - 'Use op "in"/"nin" with an array value (e.g. ["chrome","firefox"]); pipe-delimited strings are also accepted.', + 'Use op "in"/"nin" with an array value (e.g. ["chrome","firefox"]); pipe-delimited strings are also accepted. Array string members cannot contain "|".', ), params: z .string() diff --git a/src/commands/segments.ts b/src/commands/segments.ts index 6fef649..1b21d8e 100644 --- a/src/commands/segments.ts +++ b/src/commands/segments.ts @@ -1,6 +1,8 @@ import { Cli, z } from 'incur' import { createClient, requireApiKey } from '../lib/client' import { + hasTinybirdMembershipDelimiter, + isCanonicalFilterValue, isCanonicalFilterOperator, isValuelessFilterOperator, } from '../lib/filters' @@ -44,18 +46,6 @@ export interface CreateSegmentOptions { const SEGMENT_FILTER_KEYS = new Set(['field', 'op', 'value']) -function isSegmentFilterValue(value: unknown): boolean { - return ( - typeof value === 'string' || - typeof value === 'number' || - typeof value === 'boolean' || - (Array.isArray(value) && - value.every( - (item) => typeof item === 'string' || typeof item === 'number', - )) - ) -} - export function buildCreateSegmentBody(options: CreateSegmentOptions) { const filters = parseJsonArray(options.filters, '--filters') if (filters.length === 0) { @@ -84,7 +74,7 @@ export function buildCreateSegmentBody(options: CreateSegmentOptions) { !isValuelessFilterOperator(record.op) && (record.value === undefined || record.value === null || - !isSegmentFilterValue(record.value)) + !isCanonicalFilterValue(record.value)) ) { throw new Error( '--filters: "value" is required for every operator except notEmpty/isEmpty', @@ -93,12 +83,17 @@ export function buildCreateSegmentBody(options: CreateSegmentOptions) { if ( record.value !== undefined && record.value !== null && - !isSegmentFilterValue(record.value) + !isCanonicalFilterValue(record.value) ) { throw new Error( '--filters: "value" must be a string, number, boolean, or string/number array', ) } + if (hasTinybirdMembershipDelimiter(record.value)) { + throw new Error( + '--filters: array string members cannot contain "|" because it is the Tinybird membership separator', + ) + } } return { @@ -120,7 +115,7 @@ segments.command('create', { filters: z .string() .describe( - 'JSON array of canonical filter objects: [{"field","op","value"}]', + 'JSON array of canonical filter objects: [{"field","op","value"}]. Array string members cannot contain "|".', ), }), examples: [ diff --git a/src/lib/filters.ts b/src/lib/filters.ts index 0f2a512..3949cb8 100644 --- a/src/lib/filters.ts +++ b/src/lib/filters.ts @@ -25,3 +25,22 @@ export function isCanonicalFilterOperator(op: unknown): op is string { export function isValuelessFilterOperator(op: unknown): boolean { return op === 'notEmpty' || op === 'isEmpty' } + +export function isCanonicalFilterValue(value: unknown): boolean { + return ( + typeof value === 'string' || + typeof value === 'number' || + typeof value === 'boolean' || + (Array.isArray(value) && + value.every( + (item) => typeof item === 'string' || typeof item === 'number', + )) + ) +} + +export function hasTinybirdMembershipDelimiter(value: unknown): boolean { + return ( + Array.isArray(value) && + value.some((item) => typeof item === 'string' && item.includes('|')) + ) +} diff --git a/test/commands/analytics.test.ts b/test/commands/analytics.test.ts index 677a0f2..3947465 100644 --- a/test/commands/analytics.test.ts +++ b/test/commands/analytics.test.ts @@ -40,6 +40,85 @@ describe('commands/analytics', function () { ); }); + it('validates every canonical filter entry', function () { + expect(() => + buildAnalyticsParams({ + filters: '[{"operand":"location","operator":"eq","value":"US"}]', + }), + ).to.throw(/field, op, value/); + expect(() => + buildAnalyticsParams({ + filters: '[{"field":"location","op":"equals","value":"US"}]', + }), + ).to.throw(/canonical "op"/); + expect(() => + buildAnalyticsParams({ + filters: '[{"field":"location","op":"contains"}]', + }), + ).to.throw(/"value" is required/); + }); + + it('accepts value-less and one-level nested canonical filters', function () { + const filters = [ + { field: 'referrer', op: 'notEmpty' }, + { + field: 'event', + op: 'eq', + value: 'purchase', + filters: [{ field: 'amount', op: 'gte', value: 100 }], + }, + ]; + expect( + buildAnalyticsParams({ filters: JSON.stringify(filters) }).filters, + ).to.equal(JSON.stringify(filters)); + }); + + it('rejects recursive nested filters', function () { + expect(() => + buildAnalyticsParams({ + filters: JSON.stringify([ + { + field: 'event', + op: 'eq', + value: 'purchase', + filters: [ + { + field: 'amount', + op: 'gte', + value: 100, + filters: [{ field: 'currency', op: 'eq', value: 'USD' }], + }, + ], + }, + ]), + }), + ).to.throw(/one-level array of leaf filters/); + }); + + it('rejects literal pipes in membership array members', function () { + expect(() => + buildAnalyticsParams({ + filters: JSON.stringify([ + { field: 'browser', op: 'in', value: ['Chrome|Mobile', 'Safari'] }, + ]), + }), + ).to.throw(/cannot contain "\|"/); + expect(() => + buildAnalyticsParams({ + filters: JSON.stringify([ + { + field: 'event', + op: 'eq', + value: 'purchase', + filters: [ + { field: 'sku', op: 'in', value: ['alpha|beta', 'gamma'] }, + ], + }, + ]), + }), + ).to.throw(/cannot contain "\|"/); + }); + it('merges primitive params through unchanged', function () { const params = buildAnalyticsParams({ params: '{"limit":10,"group_by":"device"}', diff --git a/test/commands/bodyBuilders.test.ts b/test/commands/bodyBuilders.test.ts index 6281373..b3b2bbb 100644 --- a/test/commands/bodyBuilders.test.ts +++ b/test/commands/bodyBuilders.test.ts @@ -530,6 +530,16 @@ describe('commands / body builders', function () { { field: 'browser', op: 'notEmpty' }, ]); }); + + it('rejects literal pipes in membership array members', function () { + expect(() => + buildCreateSegmentBody({ + title: 'x', + filters: + '[{"field":"browser","op":"in","value":["Chrome|Mobile","Safari"]}]', + }), + ).to.throw(/cannot contain "\|"/); + }); }); describe('buildImportBody() mutually exclusive flags', function () {