From ebcbf6a69aaeb21ab7733bb3d5175ed4a286edfe Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Tue, 2 Jun 2026 11:26:46 -0700 Subject: [PATCH 01/10] docs: add OData filter sanitization design spec Co-Authored-By: Claude Opus 4.8 (1M context) --- ...-06-02-odata-filter-sanitization-design.md | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md diff --git a/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md b/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md new file mode 100644 index 0000000..8244883 --- /dev/null +++ b/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md @@ -0,0 +1,130 @@ +# OData Filter Sanitization — Design + +**Date:** 2026-06-02 +**Status:** Approved (design) + +## Problem + +The SDK builds OData filter strings by interpolating values directly into +single-quoted string literals, e.g.: + +```ts +new QueryContext().setFilter(`UserName eq '${username}'`) +``` + +OData requires a literal single quote inside a string to be **doubled** +(`O'Brien` → `'O''Brien'`). Because nothing does this, any filter value +containing a `'` produces a malformed query and the search fails. The +canonical case: searching users by an email or name that contains an +apostrophe. + +Sanitizing ad hoc at each call site is error-prone — a developer has to +remember to escape every value. We want a way to escape values +automatically that the SDK exposes, adopted incrementally and **without a +breaking change** for consumers upgrading the package. + +## Constraints + +- **Non-breaking.** Existing `setFilter('...')` string calls must continue + to compile and run unchanged. The package upgrade adds API; it removes + nothing. +- **Incremental migration.** Call sites adopt the new approach one at a + time. No big-bang rewrite. +- Once a value is interpolated into a finished filter string, the delimiter + quote can no longer be distinguished from data. Therefore escaping must + happen **at the point a value is placed into the filter**, not by + inspecting the completed string. + +## Current filter surface + +Filters in `famis_client.ts` use only: + +- `eq` over string literals (`UserName eq '...'`, `DisplayName eq '...'`) +- `eq` over numeric IDs (`Id eq 42`, `RequestTypeId eq ...`) +- the occasional boolean (`ActiveFlag eq true`) +- `and` / `or` composition, including `chunk.map(c => 'Id eq ' + c).join(' or ')` + +No dates, GUID literals, or OData functions (`substringof`, `startswith`, +etc.) appear in the current filter surface. The design targets this surface +and leaves room to extend later. + +## Approach: phased + +### Phase 1 — Escaping primitive + +New module `model/odata.ts`: + +```ts +// "O'Brien" -> "'O''Brien'" +export function odataString(value: string): string { + return `'${value.replace(/'/g, "''")}'`; +} + +// strings -> quoted + escaped; numbers/booleans -> bare +export function odataValue(value: string | number | boolean): string { + if (typeof value === 'string') return odataString(value); + return String(value); +} +``` + +- `odataString` is the actual fix for the apostrophe bug. +- `odataValue` dispatches on type so callers never have to decide whether to + quote. +- Purely additive. These are the safety primitive every other piece builds + on. + +### Phase 2 — Structured `Filter` builder + +Same module. A small composable builder that uses `odataValue` internally so +escaping is automatic and cannot be forgotten: + +```ts +Filter.eq('UserName', username) // UserName eq 'O''Brien' +Filter.eq('Id', 42) // Id eq 42 +Filter.raw('ActiveFlag eq true') // escape hatch for code-controlled fragments +someFilter.and(other) // a and b +someFilter.or(other) // (a) or (b) +Filter.any(filters) // joins with ' or ' — covers the chunk.map(...).join(' or ') pattern +``` + +Rules: + +- Only **values** are escaped. Field names and operators are code-controlled + and passed through unescaped. +- `Filter.raw(fragment)` is the explicit escape hatch for hand-written + fragments (e.g. `ActiveFlag eq true`) — its content is trusted and not + escaped. +- `toString()` returns the OData string. +- `Filter.any([])` (empty) and combining with empty filters should be + defined explicitly in implementation (e.g. empty `any` yields an empty + string; document the chosen behavior). + +### Phase 3 — Non-breaking integration + +- Widen `QueryContext.setFilter(filter: string)` to + `setFilter(filter: string | Filter)`, calling `.toString()` internally. + Old string calls are unaffected. +- Export `Filter`, `odataString`, and `odataValue` from `index.ts`. +- Migrate the highest-risk call sites to `Filter` first: user-supplied + string values such as `UserName eq '...'` and `DisplayName eq '...'` + (email/username/name/free-text search). +- Leave numeric-ID filters as-is initially; migrate incrementally over time. + +## Testing + +Unit tests for `model/odata.ts` (Jest, already configured; no network): + +- `odataString`: doubles a single quote; handles multiple/embedded quotes; + handles a value with no quotes. +- `odataValue`: strings quoted+escaped; numbers bare; booleans bare. +- `Filter`: `eq` with string vs numeric value; `and` / `or` composition and + parenthesization; `any` join behavior including the empty case; + `raw` passthrough (no escaping). + +## Non-goals + +- Not a full OData expression parser. +- Not retroactively rewriting every call site in one pass. +- Not validating field names against a schema. +- Not handling date/GUID/function literals (no current usage; add when a + real need appears). From 4f855f2afd77c939f92c8cc4c9ed96c7737a28bb Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Tue, 2 Jun 2026 11:29:34 -0700 Subject: [PATCH 02/10] docs: add OData filter sanitization implementation plan Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-02-odata-filter-sanitization.md | 362 ++++++++++++++++++ 1 file changed, 362 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-02-odata-filter-sanitization.md diff --git a/docs/superpowers/plans/2026-06-02-odata-filter-sanitization.md b/docs/superpowers/plans/2026-06-02-odata-filter-sanitization.md new file mode 100644 index 0000000..7d4286d --- /dev/null +++ b/docs/superpowers/plans/2026-06-02-odata-filter-sanitization.md @@ -0,0 +1,362 @@ +# OData Filter Sanitization Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a non-breaking, automatic way to sanitize OData filter values so filters containing single quotes (e.g. emails/names like `O'Brien`) no longer break searches. + +**Architecture:** A new `model/odata.ts` module provides escaping primitives (`odataString`, `odataValue`) and a composable `Filter` builder that escapes values automatically. `QueryContext.setFilter` is widened to accept `string | Filter`. High-risk string call sites in `famis_client.ts` are migrated to `Filter`. Everything is additive — existing string filters keep working. + +**Tech Stack:** TypeScript, Jest (ts-jest), existing 360Facility SDK. + +--- + +## File Structure + +- **Create:** `model/odata.ts` — escaping primitives + `Filter` builder. Single responsibility: turning values/fields into safe OData filter strings. +- **Create:** `tests/odata.test.ts` — unit tests for the module. +- **Modify:** `model/request_context.ts` — widen `setFilter` signature to `string | Filter`. +- **Modify:** `index.ts` — re-export the new module. +- **Modify:** `famis_client.ts` — migrate high-risk string filter call sites to `Filter`. + +--- + +## Task 1: Escaping primitives (`odataString`, `odataValue`) + +**Files:** +- Create: `model/odata.ts` +- Test: `tests/odata.test.ts` + +- [ ] **Step 1: Write the failing tests** + +```ts +import { odataString, odataValue } from '../model/odata'; + +describe('odataString', () => { + it('wraps a plain string in single quotes', () => { + expect(odataString('Joe')).toEqual("'Joe'"); + }); + it('doubles a single quote', () => { + expect(odataString("O'Brien")).toEqual("'O''Brien'"); + }); + it('doubles multiple embedded quotes', () => { + expect(odataString("a'b'c")).toEqual("'a''b''c'"); + }); + it('handles an empty string', () => { + expect(odataString('')).toEqual("''"); + }); +}); + +describe('odataValue', () => { + it('quotes and escapes strings', () => { + expect(odataValue("O'Brien")).toEqual("'O''Brien'"); + }); + it('leaves numbers bare', () => { + expect(odataValue(42)).toEqual('42'); + }); + it('leaves booleans bare', () => { + expect(odataValue(true)).toEqual('true'); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npx jest tests/odata.test.ts` +Expected: FAIL — cannot find module `../model/odata`. + +- [ ] **Step 3: Write minimal implementation** + +```ts +// model/odata.ts + +/** Escape a string as an OData string literal: double single quotes, wrap in quotes. */ +export function odataString(value: string): string { + return `'${value.replace(/'/g, "''")}'`; +} + +/** Format a value for an OData filter: strings are quoted+escaped; numbers/booleans are bare. */ +export function odataValue(value: string | number | boolean): string { + if (typeof value === 'string') { + return odataString(value); + } + return String(value); +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npx jest tests/odata.test.ts` +Expected: PASS (7 tests). + +- [ ] **Step 5: Commit** + +```bash +rtk git add model/odata.ts tests/odata.test.ts +rtk git commit -m "feat: add OData value escaping primitives" +``` + +--- + +## Task 2: `Filter` builder + +**Files:** +- Modify: `model/odata.ts` +- Test: `tests/odata.test.ts` + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/odata.test.ts`: + +```ts +import { Filter } from '../model/odata'; + +describe('Filter', () => { + it('eq with a string value escapes the value', () => { + expect(Filter.eq('UserName', "O'Brien").toString()).toEqual("UserName eq 'O''Brien'"); + }); + it('eq with a numeric value leaves it bare', () => { + expect(Filter.eq('Id', 42).toString()).toEqual('Id eq 42'); + }); + it('raw passes the fragment through unescaped', () => { + expect(Filter.raw('ActiveFlag eq true').toString()).toEqual('ActiveFlag eq true'); + }); + it('and joins two filters', () => { + const f = Filter.eq('Id', 1).and(Filter.raw('ActiveFlag eq true')); + expect(f.toString()).toEqual('Id eq 1 and ActiveFlag eq true'); + }); + it('or parenthesizes both sides', () => { + const f = Filter.eq('Id', 1).or(Filter.eq('Id', 2)); + expect(f.toString()).toEqual('(Id eq 1) or (Id eq 2)'); + }); + it('any joins filters with " or "', () => { + const f = Filter.any([Filter.eq('Id', 1), Filter.eq('Id', 2), Filter.eq('Id', 3)]); + expect(f.toString()).toEqual('Id eq 1 or Id eq 2 or Id eq 3'); + }); + it('any with a single filter returns just that filter', () => { + expect(Filter.any([Filter.eq('Id', 1)]).toString()).toEqual('Id eq 1'); + }); + it('any with no filters returns an empty string', () => { + expect(Filter.any([]).toString()).toEqual(''); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npx jest tests/odata.test.ts` +Expected: FAIL — `Filter` is not exported. + +- [ ] **Step 3: Write minimal implementation** + +Append to `model/odata.ts`: + +```ts +export class Filter { + private constructor(private readonly expr: string) {} + + /** field eq value — value is escaped/formatted automatically. */ + static eq(field: string, value: string | number | boolean): Filter { + return new Filter(`${field} eq ${odataValue(value)}`); + } + + /** Trusted, code-controlled fragment. Not escaped. Use only for literals you control. */ + static raw(fragment: string): Filter { + return new Filter(fragment); + } + + /** Join filters with " or " (no extra parentheses). Empty list yields an empty filter. */ + static any(filters: Filter[]): Filter { + return new Filter(filters.map((f) => f.toString()).join(' or ')); + } + + and(other: Filter): Filter { + return new Filter(`${this.toString()} and ${other.toString()}`); + } + + or(other: Filter): Filter { + return new Filter(`(${this.toString()}) or (${other.toString()})`); + } + + toString(): string { + return this.expr; + } +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npx jest tests/odata.test.ts` +Expected: PASS (all Task 1 + Task 2 tests). + +- [ ] **Step 5: Commit** + +```bash +rtk git add model/odata.ts tests/odata.test.ts +rtk git commit -m "feat: add composable OData Filter builder" +``` + +--- + +## Task 3: Widen `setFilter` to accept `Filter` + +**Files:** +- Modify: `model/request_context.ts:9-12` +- Test: `tests/query_context.test.ts` + +- [ ] **Step 1: Write the failing test** + +Append a test inside the `describe('QueryContext', ...)` block in `tests/query_context.test.ts`: + +```ts +it('accepts a Filter object and renders the escaped filter', function () { + const context = new QueryContext().setFilter(Filter.eq('UserName', "O'Brien")); + const queryUrl = context.buildUrl('/test'); + const uri = new URL(`http://example.com/${queryUrl}`); + expect(uri.searchParams.get('$filter')).toEqual("UserName eq 'O''Brien'"); +}); +``` + +Add the import at the top of the file: + +```ts +import { Filter } from '../model/odata'; +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx jest tests/query_context.test.ts` +Expected: FAIL — TypeScript error: `Filter` is not assignable to parameter of type `string` (compile failure in ts-jest). + +- [ ] **Step 3: Update `setFilter`** + +In `model/request_context.ts`, change the `setFilter` method (lines 9-12): + +```ts + setFilter(filter: string | Filter) { + this.filter = filter.toString(); + return this; + } +``` + +Add the import at the top of `model/request_context.ts`: + +```ts +import { Filter } from './odata'; +``` + +Note: `this.filter` stays typed as `string?`. `String.prototype.toString()` on a string returns itself, so existing string callers are unaffected. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx jest tests/query_context.test.ts` +Expected: PASS (all 4 tests, including the new one). + +- [ ] **Step 5: Commit** + +```bash +rtk git add model/request_context.ts tests/query_context.test.ts +rtk git commit -m "feat: allow setFilter to accept a Filter object" +``` + +--- + +## Task 4: Export the module from the package entry point + +**Files:** +- Modify: `index.ts` + +- [ ] **Step 1: Add the export** + +Append to `index.ts`: + +```ts +export * from './model/odata'; +``` + +- [ ] **Step 2: Verify the build compiles** + +Run: `rtk npm run build` +Expected: `tsc` completes with no errors; `Filter`, `odataString`, `odataValue` are present in `dist/index.d.ts`. + +- [ ] **Step 3: Commit** + +```bash +rtk git add index.ts +rtk git commit -m "feat: export odata module from package entry point" +``` + +--- + +## Task 5: Migrate high-risk string filter call sites + +Migrate only call sites that interpolate user-supplied **string** values into a quoted literal. Numeric-ID filters are left as-is (no quote-injection risk) and migrated later. + +**Files:** +- Modify: `famis_client.ts:754` (`UserName eq '${username}'`) +- Modify: `famis_client.ts:1440` (`DisplayName eq '${name}'`) +- Modify: `famis_client.ts:1450` (`names.map((name) => \`DisplayName eq '${name}'\`).join(' or ')`) + +- [ ] **Step 1: Migrate `UserName eq` (around line 754)** + +Before: +```ts +new QueryContext().setFilter(`UserName eq '${username}'`).setSelect(select.join(',')) +``` +After: +```ts +new QueryContext().setFilter(Filter.eq('UserName', username)).setSelect(select.join(',')) +``` + +- [ ] **Step 2: Migrate `DisplayName eq` single (around line 1440)** + +Before: +```ts +context.setFilter(`DisplayName eq '${name}'`); +``` +After: +```ts +context.setFilter(Filter.eq('DisplayName', name)); +``` + +- [ ] **Step 3: Migrate `DisplayName eq` list (around line 1450)** + +Before: +```ts +const filterString = names.map((name) => `DisplayName eq '${name}'`).join(' or '); +const res = await this.getUdfFields(new QueryContext().setFilter(filterString)); +``` +After: +```ts +const filter = Filter.any(names.map((name) => Filter.eq('DisplayName', name))); +const res = await this.getUdfFields(new QueryContext().setFilter(filter)); +``` + +- [ ] **Step 4: Add the import to `famis_client.ts`** + +Ensure the top of `famis_client.ts` imports `Filter`: + +```ts +import { Filter } from './model/odata'; +``` + +(If an import block from `./model/...` already exists, add `Filter` consistent with the existing import style.) + +- [ ] **Step 5: Verify the build compiles and tests pass** + +Run: `rtk npm run build && npm test` +Expected: `tsc` clean; existing unit tests pass (integration tests are excluded by the `test` script's ignore pattern). + +- [ ] **Step 6: Commit** + +```bash +rtk git add famis_client.ts +rtk git commit -m "feat: migrate user/display name filters to escaped Filter builder" +``` + +--- + +## Self-Review Notes + +- **Spec coverage:** Phase 1 → Task 1; Phase 2 → Task 2; Phase 3 (widen setFilter → Task 3, export → Task 4, migrate high-risk sites → Task 5). Testing section → tests embedded in Tasks 1-3. Non-goals respected (no parser, no full migration, no field validation). +- **Type consistency:** `odataString`/`odataValue`/`Filter.eq`/`Filter.raw`/`Filter.any`/`.and`/`.or`/`.toString()` names are consistent across all tasks and match the spec. +- **Line numbers** in Task 5 are from the current `famis_client.ts` snapshot and may drift; each step shows the exact before/after text to match, so locate by content if line numbers differ. From 38ae80cb782a8b82b217c8b89689b249dc912e4e Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 09:07:52 -0700 Subject: [PATCH 03/10] feat: add OData value escaping primitives --- model/odata.ts | 12 ++++++++++++ tests/odata.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 model/odata.ts create mode 100644 tests/odata.test.ts diff --git a/model/odata.ts b/model/odata.ts new file mode 100644 index 0000000..ac36ea2 --- /dev/null +++ b/model/odata.ts @@ -0,0 +1,12 @@ +/** Escape a string as an OData string literal: double single quotes, wrap in quotes. */ +export function odataString(value: string): string { + return `'${value.replace(/'/g, "''")}'`; +} + +/** Format a value for an OData filter: strings are quoted+escaped; numbers/booleans are bare. */ +export function odataValue(value: string | number | boolean): string { + if (typeof value === 'string') { + return odataString(value); + } + return String(value); +} diff --git a/tests/odata.test.ts b/tests/odata.test.ts new file mode 100644 index 0000000..05f90fe --- /dev/null +++ b/tests/odata.test.ts @@ -0,0 +1,28 @@ +import { odataString, odataValue } from '../model/odata'; + +describe('odataString', () => { + it('wraps a plain string in single quotes', () => { + expect(odataString('Joe')).toEqual("'Joe'"); + }); + it('doubles a single quote', () => { + expect(odataString("O'Brien")).toEqual("'O''Brien'"); + }); + it('doubles multiple embedded quotes', () => { + expect(odataString("a'b'c")).toEqual("'a''b''c'"); + }); + it('handles an empty string', () => { + expect(odataString('')).toEqual("''"); + }); +}); + +describe('odataValue', () => { + it('quotes and escapes strings', () => { + expect(odataValue("O'Brien")).toEqual("'O''Brien'"); + }); + it('leaves numbers bare', () => { + expect(odataValue(42)).toEqual('42'); + }); + it('leaves booleans bare', () => { + expect(odataValue(true)).toEqual('true'); + }); +}); From 5d9f514414c525607d894976211b137fb6d6054f Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 09:08:33 -0700 Subject: [PATCH 04/10] feat: add composable OData Filter builder --- model/odata.ts | 31 +++++++++++++++++++++++++++++++ tests/odata.test.ts | 32 +++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/model/odata.ts b/model/odata.ts index ac36ea2..4bd7d5f 100644 --- a/model/odata.ts +++ b/model/odata.ts @@ -10,3 +10,34 @@ export function odataValue(value: string | number | boolean): string { } return String(value); } + +export class Filter { + private constructor(private readonly expr: string) {} + + /** field eq value — value is escaped/formatted automatically. */ + static eq(field: string, value: string | number | boolean): Filter { + return new Filter(`${field} eq ${odataValue(value)}`); + } + + /** Trusted, code-controlled fragment. Not escaped. Use only for literals you control. */ + static raw(fragment: string): Filter { + return new Filter(fragment); + } + + /** Join filters with " or " (no extra parentheses). Empty list yields an empty filter. */ + static any(filters: Filter[]): Filter { + return new Filter(filters.map((f) => f.toString()).join(' or ')); + } + + and(other: Filter): Filter { + return new Filter(`${this.toString()} and ${other.toString()}`); + } + + or(other: Filter): Filter { + return new Filter(`(${this.toString()}) or (${other.toString()})`); + } + + toString(): string { + return this.expr; + } +} diff --git a/tests/odata.test.ts b/tests/odata.test.ts index 05f90fe..60d31a1 100644 --- a/tests/odata.test.ts +++ b/tests/odata.test.ts @@ -1,4 +1,4 @@ -import { odataString, odataValue } from '../model/odata'; +import { odataString, odataValue, Filter } from '../model/odata'; describe('odataString', () => { it('wraps a plain string in single quotes', () => { @@ -26,3 +26,33 @@ describe('odataValue', () => { expect(odataValue(true)).toEqual('true'); }); }); + +describe('Filter', () => { + it('eq with a string value escapes the value', () => { + expect(Filter.eq('UserName', "O'Brien").toString()).toEqual("UserName eq 'O''Brien'"); + }); + it('eq with a numeric value leaves it bare', () => { + expect(Filter.eq('Id', 42).toString()).toEqual('Id eq 42'); + }); + it('raw passes the fragment through unescaped', () => { + expect(Filter.raw('ActiveFlag eq true').toString()).toEqual('ActiveFlag eq true'); + }); + it('and joins two filters', () => { + const f = Filter.eq('Id', 1).and(Filter.raw('ActiveFlag eq true')); + expect(f.toString()).toEqual('Id eq 1 and ActiveFlag eq true'); + }); + it('or parenthesizes both sides', () => { + const f = Filter.eq('Id', 1).or(Filter.eq('Id', 2)); + expect(f.toString()).toEqual('(Id eq 1) or (Id eq 2)'); + }); + it('any joins filters with " or "', () => { + const f = Filter.any([Filter.eq('Id', 1), Filter.eq('Id', 2), Filter.eq('Id', 3)]); + expect(f.toString()).toEqual('Id eq 1 or Id eq 2 or Id eq 3'); + }); + it('any with a single filter returns just that filter', () => { + expect(Filter.any([Filter.eq('Id', 1)]).toString()).toEqual('Id eq 1'); + }); + it('any with no filters returns an empty string', () => { + expect(Filter.any([]).toString()).toEqual(''); + }); +}); From 346f762517ed3babb019049009a079a64b31c4d0 Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 09:22:21 -0700 Subject: [PATCH 05/10] fix: make Filter composition precedence-safe Introduce a composite flag so or/any results parenthesize themselves only when combined with and, preventing the OData operator-precedence trap where 'A or B and C' is silently misread as 'A or (B and C)'. Co-Authored-By: Claude Sonnet 4.6 --- model/odata.ts | 22 ++++++++++++++++------ tests/odata.test.ts | 23 +++++++++++++++++++---- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/model/odata.ts b/model/odata.ts index 4bd7d5f..99a2c67 100644 --- a/model/odata.ts +++ b/model/odata.ts @@ -12,9 +12,12 @@ export function odataValue(value: string | number | boolean): string { } export class Filter { - private constructor(private readonly expr: string) {} + private constructor( + private readonly expr: string, + private readonly composite: boolean = false, + ) {} - /** field eq value — value is escaped/formatted automatically. */ + /** field eq value — value is escaped/formatted automatically. `field` must be a static identifier, never user input. */ static eq(field: string, value: string | number | boolean): Filter { return new Filter(`${field} eq ${odataValue(value)}`); } @@ -24,17 +27,24 @@ export class Filter { return new Filter(fragment); } - /** Join filters with " or " (no extra parentheses). Empty list yields an empty filter. */ + /** Join filters with " or ". Empty -> empty filter; single -> that filter unchanged; 2+ -> a composite that parenthesizes itself when later combined. */ static any(filters: Filter[]): Filter { - return new Filter(filters.map((f) => f.toString()).join(' or ')); + if (filters.length === 0) return new Filter(''); + if (filters.length === 1) return filters[0]; + return new Filter(filters.map((f) => f.wrapped()).join(' or '), true); } and(other: Filter): Filter { - return new Filter(`${this.toString()} and ${other.toString()}`); + return new Filter(`${this.wrapped()} and ${other.wrapped()}`, true); } or(other: Filter): Filter { - return new Filter(`(${this.toString()}) or (${other.toString()})`); + return new Filter(`${this.wrapped()} or ${other.wrapped()}`, true); + } + + /** Parenthesize this expression when it is itself composite (top-level and/or). */ + private wrapped(): string { + return this.composite ? `(${this.expr})` : this.expr; } toString(): string { diff --git a/tests/odata.test.ts b/tests/odata.test.ts index 60d31a1..ce47e1c 100644 --- a/tests/odata.test.ts +++ b/tests/odata.test.ts @@ -25,6 +25,9 @@ describe('odataValue', () => { it('leaves booleans bare', () => { expect(odataValue(true)).toEqual('true'); }); + it('leaves false bare', () => { + expect(odataValue(false)).toEqual('false'); + }); }); describe('Filter', () => { @@ -37,15 +40,15 @@ describe('Filter', () => { it('raw passes the fragment through unescaped', () => { expect(Filter.raw('ActiveFlag eq true').toString()).toEqual('ActiveFlag eq true'); }); - it('and joins two filters', () => { + it('and joins two atomic filters without parentheses', () => { const f = Filter.eq('Id', 1).and(Filter.raw('ActiveFlag eq true')); expect(f.toString()).toEqual('Id eq 1 and ActiveFlag eq true'); }); - it('or parenthesizes both sides', () => { + it('or joins two atomic filters without parentheses', () => { const f = Filter.eq('Id', 1).or(Filter.eq('Id', 2)); - expect(f.toString()).toEqual('(Id eq 1) or (Id eq 2)'); + expect(f.toString()).toEqual('Id eq 1 or Id eq 2'); }); - it('any joins filters with " or "', () => { + it('any joins atomic filters with " or " and no outer parens on its own', () => { const f = Filter.any([Filter.eq('Id', 1), Filter.eq('Id', 2), Filter.eq('Id', 3)]); expect(f.toString()).toEqual('Id eq 1 or Id eq 2 or Id eq 3'); }); @@ -55,4 +58,16 @@ describe('Filter', () => { it('any with no filters returns an empty string', () => { expect(Filter.any([]).toString()).toEqual(''); }); + it('parenthesizes an or-group when combined with and', () => { + const f = Filter.eq('Id', 1).or(Filter.eq('Id', 2)).and(Filter.raw('ActiveFlag eq true')); + expect(f.toString()).toEqual('(Id eq 1 or Id eq 2) and ActiveFlag eq true'); + }); + it('parenthesizes an any-group when combined with and', () => { + const f = Filter.any([Filter.eq('Id', 1), Filter.eq('Id', 2)]).and(Filter.raw('ActiveFlag eq true')); + expect(f.toString()).toEqual('(Id eq 1 or Id eq 2) and ActiveFlag eq true'); + }); + it('does not parenthesize an atomic combined with and', () => { + const f = Filter.eq('Id', 1).and(Filter.raw('ActiveFlag eq true')); + expect(f.toString()).toEqual('Id eq 1 and ActiveFlag eq true'); + }); }); From dbed17bcbb19a9b1b7dd7281e9bedf4b47410d74 Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 09:22:42 -0700 Subject: [PATCH 06/10] docs: note precedence-safe Filter composition in spec --- .../2026-06-02-odata-filter-sanitization-design.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md b/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md index 8244883..58c1878 100644 --- a/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md +++ b/docs/superpowers/specs/2026-06-02-odata-filter-sanitization-design.md @@ -95,9 +95,15 @@ Rules: fragments (e.g. `ActiveFlag eq true`) — its content is trusted and not escaped. - `toString()` returns the OData string. -- `Filter.any([])` (empty) and combining with empty filters should be - defined explicitly in implementation (e.g. empty `any` yields an empty - string; document the chosen behavior). +- **Precedence-safe composition.** OData binds `and` tighter than `or`, so + composite operands must be parenthesized when combined. Each `Filter` + tracks whether it is composite (top-level `and`/`or`); `and`/`or`/`any` + parenthesize an operand only when that operand is itself composite. Atomic + terms (`eq`, `raw`) are never parenthesized. This makes + `any([a, b]).and(c)` render as `(a or b) and c` rather than the incorrect + `a or b and c`. +- `Filter.any([])` yields an empty filter; `Filter.any([x])` returns `x` + unchanged. ### Phase 3 — Non-breaking integration From 8320425b233e9ffdf7ea00cb7dff139ce4a21964 Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 09:24:24 -0700 Subject: [PATCH 07/10] feat: allow setFilter to accept a Filter object --- model/request_context.ts | 6 ++++-- tests/query_context.test.ts | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/model/request_context.ts b/model/request_context.ts index 6fab641..28064b2 100644 --- a/model/request_context.ts +++ b/model/request_context.ts @@ -1,3 +1,5 @@ +import { Filter } from './odata'; + export class QueryContext { filter?: string; expand?: string; @@ -6,8 +8,8 @@ export class QueryContext { skip?: number; orderBy?: string; - setFilter(filter : string) { - this.filter = filter; + setFilter(filter: string | Filter) { + this.filter = filter.toString(); return this; } diff --git a/tests/query_context.test.ts b/tests/query_context.test.ts index 2bb72db..a35ee38 100644 --- a/tests/query_context.test.ts +++ b/tests/query_context.test.ts @@ -1,4 +1,5 @@ import {QueryContext} from "../model/request_context"; +import { Filter } from '../model/odata'; describe('QueryContext', function () { it('should include select statement', function () { @@ -25,4 +26,10 @@ describe('QueryContext', function () { const select = uri.searchParams.get("$filter"); expect(select).toEqual("Name eq 'Joe'") }); + it('accepts a Filter object and renders the escaped filter', function () { + const context = new QueryContext().setFilter(Filter.eq('UserName', "O'Brien")); + const queryUrl = context.buildUrl('/test'); + const uri = new URL(`http://example.com/${queryUrl}`); + expect(uri.searchParams.get('$filter')).toEqual("UserName eq 'O''Brien'"); + }); }); \ No newline at end of file From 7591d91bfddd5dca987eea360e15efdffdf605b3 Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 09:24:39 -0700 Subject: [PATCH 08/10] feat: export odata module from package entry point --- index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/index.ts b/index.ts index d2de1c5..db6fd90 100644 --- a/index.ts +++ b/index.ts @@ -4,4 +4,5 @@ export * from './model/request_context'; export * from './model/famis_models'; export * from './model/common'; export * from './model/geo_locations'; -export * from './model/request_models'; \ No newline at end of file +export * from './model/request_models'; +export * from './model/odata'; \ No newline at end of file From 61b05857cd617dd515830468cf3ef68ad03e6f37 Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Wed, 3 Jun 2026 10:14:26 -0700 Subject: [PATCH 09/10] feat: migrate user/display name filters to escaped Filter builder Co-Authored-By: Claude Sonnet 4.6 --- famis_client.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/famis_client.ts b/famis_client.ts index ff37866..0c79050 100644 --- a/famis_client.ts +++ b/famis_client.ts @@ -12,6 +12,7 @@ import Bottleneck from 'bottleneck'; import _ from 'lodash'; import { ApiError, AuthorizationError } from './errors'; import { OnCompleteCallback, Result, SdkCallInfo } from './model/common'; +import { Filter } from './model/odata'; import { AccountSegment, AccountSegmentValue, @@ -751,7 +752,7 @@ export class FamisClient { select: string[] = DefaultUserSelect, ): Promise { const res = await this.getUsers( - new QueryContext().setFilter(`UserName eq '${username}'`).setSelect(select.join(',')), + new QueryContext().setFilter(Filter.eq('UserName', username)).setSelect(select.join(',')), ); return res.first; } @@ -1437,7 +1438,7 @@ export class FamisClient { //#region Udfs async getUdfField(name: string, context: QueryContext): Promise { - context.setFilter(`DisplayName eq '${name}'`); + context.setFilter(Filter.eq('DisplayName', name)); const fieldResp = await this.getUdfFields(context); return fieldResp.results.find((f) => f.DisplayName === name); } @@ -1447,8 +1448,8 @@ export class FamisClient { } async getUdfFieldsForNames(names: string[]): Promise { - const filterString = names.map((name) => `DisplayName eq '${name}'`).join(' or '); - const res = await this.getUdfFields(new QueryContext().setFilter(filterString)); + const filter = Filter.any(names.map((name) => Filter.eq('DisplayName', name))); + const res = await this.getUdfFields(new QueryContext().setFilter(filter)); return res.results; } From 0edbe42c9f09fcb581aab60774df9bc1ed8460e5 Mon Sep 17 00:00:00 2001 From: Alexis Lasher Date: Tue, 9 Jun 2026 13:24:50 -0700 Subject: [PATCH 10/10] feat: add Filter.contains with automatic value escaping --- model/odata.ts | 5 +++++ package.json | 2 +- tests/odata.test.ts | 21 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/model/odata.ts b/model/odata.ts index 99a2c67..e46b668 100644 --- a/model/odata.ts +++ b/model/odata.ts @@ -22,6 +22,11 @@ export class Filter { return new Filter(`${field} eq ${odataValue(value)}`); } + /** contains(field, value) — value is escaped automatically. `field` must be a static identifier, never user input. */ + static contains(field: string, value: string): Filter { + return new Filter(`contains(${field}, ${odataString(value)})`); + } + /** Trusted, code-controlled fragment. Not escaped. Use only for literals you control. */ static raw(fragment: string): Filter { return new Filter(fragment); diff --git a/package.json b/package.json index 9e82892..5108709 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "facility360", - "version": "1.1.3", + "version": "1.1.4", "description": "A Node based 360Facility client SDK", "main": "dist/index.js", "scripts": { diff --git a/tests/odata.test.ts b/tests/odata.test.ts index ce47e1c..fe29102 100644 --- a/tests/odata.test.ts +++ b/tests/odata.test.ts @@ -71,3 +71,24 @@ describe('Filter', () => { expect(f.toString()).toEqual('Id eq 1 and ActiveFlag eq true'); }); }); + +describe('Filter.contains', () => { + it('builds a contains expression with an escaped string value', () => { + expect(Filter.contains('Name', 'Joe').toString()).toBe("contains(Name, 'Joe')"); + }); + + it('escapes single quotes in the value', () => { + expect(Filter.contains('Name', "O'Brien").toString()).toBe("contains(Name, 'O''Brien')"); + }); + + it('composes with or() and parenthesizes correctly', () => { + const f = Filter.contains('Name', 'a').or(Filter.contains('Description', 'b')); + expect(f.toString()).toBe("contains(Name, 'a') or contains(Description, 'b')"); + }); + + it('composes inside Filter.any().and()', () => { + const f = Filter.any([Filter.contains('Name', 'a'), Filter.contains('Name', 'b')]) + .and(Filter.raw('ActiveFlag eq true')); + expect(f.toString()).toBe("(contains(Name, 'a') or contains(Name, 'b')) and ActiveFlag eq true"); + }); +});