Skip to content

Commit b5a608a

Browse files
Copilothotlong
andcommitted
Address code review feedback: Enhanced security docs and type safety
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 44bf36e commit b5a608a

2 files changed

Lines changed: 76 additions & 11 deletions

File tree

packages/spec/src/auth/scim.zod.ts

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -404,17 +404,41 @@ export const SCIMEnterpriseUserSchema = z.object({
404404

405405
export type SCIMEnterpriseUser = z.infer<typeof SCIMEnterpriseUserSchema>;
406406

407+
/**
408+
* SCIM Schema URI Validator
409+
* Validates that schema URIs are known SCIM schema identifiers
410+
*/
411+
const SCIMSchemaURISchema = z.enum([
412+
SCIM_SCHEMAS.USER,
413+
SCIM_SCHEMAS.GROUP,
414+
SCIM_SCHEMAS.ENTERPRISE_USER,
415+
SCIM_SCHEMAS.RESOURCE_TYPE,
416+
SCIM_SCHEMAS.SERVICE_PROVIDER_CONFIG,
417+
SCIM_SCHEMAS.SCHEMA,
418+
SCIM_SCHEMAS.LIST_RESPONSE,
419+
SCIM_SCHEMAS.PATCH_OP,
420+
SCIM_SCHEMAS.BULK_REQUEST,
421+
SCIM_SCHEMAS.BULK_RESPONSE,
422+
SCIM_SCHEMAS.ERROR,
423+
]);
424+
407425
/**
408426
* SCIM User Schema (Core)
409427
* Complete SCIM 2.0 User resource
410428
*/
411429
export const SCIMUserSchema = z.object({
412430
/**
413431
* SCIM schema URIs
432+
* Must include at minimum the core User schema URI
414433
*/
415434
schemas: z.array(z.string())
435+
.min(1)
436+
.refine(
437+
(schemas) => schemas.includes(SCIM_SCHEMAS.USER),
438+
'Must include core User schema URI'
439+
)
416440
.default([SCIM_SCHEMAS.USER])
417-
.describe('SCIM schema URIs'),
441+
.describe('SCIM schema URIs (must include User schema)'),
418442

419443
/**
420444
* Unique identifier
@@ -661,10 +685,16 @@ export type SCIMMemberReference = z.infer<typeof SCIMMemberReferenceSchema>;
661685
export const SCIMGroupSchema = z.object({
662686
/**
663687
* SCIM schema URIs
688+
* Must include at minimum the core Group schema URI
664689
*/
665690
schemas: z.array(z.string())
691+
.min(1)
692+
.refine(
693+
(schemas) => schemas.includes(SCIM_SCHEMAS.GROUP),
694+
'Must include core Group schema URI'
695+
)
666696
.default([SCIM_SCHEMAS.GROUP])
667-
.describe('SCIM schema URIs'),
697+
.describe('SCIM schema URIs (must include Group schema)'),
668698

669699
/**
670700
* Unique identifier
@@ -704,9 +734,18 @@ export const SCIMGroupSchema = z.object({
704734

705735
export type SCIMGroup = z.infer<typeof SCIMGroupSchema>;
706736

737+
/**
738+
* SCIM Resource Union Type
739+
* Known SCIM resource types for type-safe list responses
740+
*/
741+
export type SCIMResource = SCIMUser | SCIMGroup;
742+
707743
/**
708744
* SCIM List Response
709745
* Paginated list of resources
746+
*
747+
* Generic type T allows for type-safe responses when the resource type is known.
748+
* For mixed resource types, use SCIMResource union.
710749
*/
711750
export const SCIMListResponseSchema = z.object({
712751
/**
@@ -726,9 +765,10 @@ export const SCIMListResponseSchema = z.object({
726765

727766
/**
728767
* Resources returned in this response
768+
* Use SCIMListResponseOf<T> for type-safe responses
729769
*/
730-
Resources: z.array(z.any())
731-
.describe('Resources array'),
770+
Resources: z.array(z.union([SCIMUserSchema, SCIMGroupSchema, z.record(z.any())]))
771+
.describe('Resources array (Users, Groups, or custom resources)'),
732772

733773
/**
734774
* 1-based index of the first result
@@ -890,10 +930,16 @@ export const SCIM = {
890930
/**
891931
* Create an error response
892932
*/
893-
error: (status: number, detail: string, scimType?: string): SCIMError => ({
933+
error: (
934+
status: number,
935+
detail: string,
936+
scimType?: 'invalidFilter' | 'tooMany' | 'uniqueness' | 'mutability' |
937+
'invalidSyntax' | 'invalidPath' | 'noTarget' | 'invalidValue' |
938+
'invalidVers' | 'sensitive'
939+
): SCIMError => ({
894940
schemas: [SCIM_SCHEMAS.ERROR],
895941
status,
896942
detail,
897-
scimType: scimType as any,
943+
scimType,
898944
}),
899945
} as const;

packages/spec/src/permission/rls.zod.ts

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -261,49 +261,68 @@ export const RowLevelSecurityPolicySchema = z.object({
261261
* This is a SQL-like expression evaluated for each row.
262262
* Only rows where this expression returns TRUE are accessible.
263263
*
264+
* **Security Note**: RLS conditions are executed at the database level with
265+
* parameterized queries. The implementation must use prepared statements
266+
* to prevent SQL injection. Never concatenate user input directly into
267+
* RLS conditions.
268+
*
269+
* **SQL Dialect**: Compatible with PostgreSQL SQL syntax. Implementations
270+
* may adapt to other databases (MySQL, SQL Server, etc.) but should maintain
271+
* semantic equivalence.
272+
*
264273
* Available context variables:
265274
* - `current_user.id` - Current user's ID
266275
* - `current_user.tenant_id` - Current user's tenant
267276
* - `current_user.role` - Current user's role
268277
* - `current_user.department` - Current user's department
269278
* - `current_user.*` - Any custom user field
270279
* - `NOW()` - Current timestamp
280+
* - `CURRENT_DATE` - Current date
281+
* - `CURRENT_TIME` - Current time
271282
*
272283
* Supported operators:
273-
* - Comparison: =, !=, <, >, <=, >=
284+
* - Comparison: =, !=, <, >, <=, >=, <> (not equal)
274285
* - Logical: AND, OR, NOT
275286
* - NULL checks: IS NULL, IS NOT NULL
276287
* - Set operations: IN, NOT IN
277-
* - String: LIKE, NOT LIKE
288+
* - String: LIKE, NOT LIKE, ILIKE (case-insensitive)
289+
* - Pattern matching: ~ (regex), !~ (not regex)
278290
* - Subqueries: (SELECT ...)
291+
* - Array operations: ANY, ALL
292+
*
293+
* **Prohibited**: Dynamic SQL, DDL statements, DML statements (INSERT/UPDATE/DELETE)
279294
*
280295
* @example "tenant_id = current_user.tenant_id"
281296
* @example "owner_id = current_user.id OR created_by = current_user.id"
282297
* @example "department IN (SELECT department FROM user_departments WHERE user_id = current_user.id)"
283298
* @example "status = 'active' AND expiry_date > NOW()"
284299
*/
285300
using: z.string()
286-
.describe('Filter condition (SQL WHERE clause syntax)'),
301+
.describe('Filter condition (PostgreSQL SQL WHERE clause syntax with parameterized context variables)'),
287302

288303
/**
289304
* CHECK clause - Validation for INSERT/UPDATE operations.
290305
*
291306
* Similar to USING but applies to new/modified rows.
292307
* Prevents users from creating/updating rows they wouldn't be able to see.
293308
*
294-
* If not specified, defaults to the USING clause.
309+
* **Default Behavior**: If not specified, implementations should use the
310+
* USING clause as the CHECK clause. This ensures data integrity by preventing
311+
* users from creating records they cannot view.
295312
*
296313
* Use cases:
297314
* - Prevent cross-tenant data creation
298315
* - Enforce mandatory field values
299316
* - Validate data integrity rules
317+
* - Restrict certain operations (e.g., only allow creating "draft" status)
300318
*
301319
* @example "tenant_id = current_user.tenant_id"
302320
* @example "status IN ('draft', 'pending')" - Only allow certain statuses
321+
* @example "created_by = current_user.id" - Must be the creator
303322
*/
304323
check: z.string()
305324
.optional()
306-
.describe('Validation condition for INSERT/UPDATE (defaults to using clause)'),
325+
.describe('Validation condition for INSERT/UPDATE (defaults to USING clause if not specified - enforced at application level)'),
307326

308327
/**
309328
* Restrict this policy to specific roles.

0 commit comments

Comments
 (0)