fix(datasource-customizer): let a narrowed search be permission-checked instead of refused

packages/agent/test/security/related-read-permissions.test.ts

@@ -1,5 +1,7 @@
+import type { SearchReplaceDefinition } from '@forestadmin/datasource-customizer';
 import type { CollectionDecorator, DataSource, RecordData } from '@forestadmin/datasource-toolkit';
 
+import { DataSourceCustomizer } from '@forestadmin/datasource-customizer';
 import { CollectionActionEvent } from '@forestadmin/forestadmin-client';
 import { createMockContext } from '@shopify/jest-koa-mocks';
 
@@ -529,6 +531,90 @@ describe('read permissions on related collections', () => {
     });
   });
 
+  // The suite above stubs `getSearchedFields` to pin the route's policy. These run the real
+  // `replaceSearch` field selection through the whole stack instead, so what the decorator reports
+  // and what the route does with it are checked together.
+  describe('extended search through a real replaceSearch field selection', () => {
+    const buildCustomizedDataSource = async (definition: SearchReplaceDefinition) => {
+      const base = buildDataSource();
+      const customizer = new DataSourceCustomizer();
+
+      customizer.addDataSource(async () => base);
+      customizer.customizeCollection('cards', collection => collection.replaceSearch(definition));
+
+      return { base, dataSource: await customizer.getDataSource(() => {}) };
+    };
+
+    it('should refuse an extended search by naming a denied path, not for want of a footprint', async () => {
+      const { base, dataSource } = await buildCustomizedDataSource({
+        includeFields: ['holder:nationalId'],
+      });
+      const services = buildServices(['accounts', 'contracts', 'organizations']);
+      const list = jest.spyOn(base.getCollection('cards'), 'list').mockResolvedValue([]);
+
+      // The refusal a field selection buys: the denied collection is named, where the handler form
+      // could only be told the fields were undeterminable.
+      await expect(
+        new List(services, options, dataSource, 'cards').handleList(
+          buildContext({ query: { search: 'martin', searchExtended: '1' } }),
+        ),
+      ).rejects.toThrow(
+        /You cannot search on 'holder:\w+': you are not allowed to read the 'holders' collection\./,
+      );
+
+      expect(list).not.toHaveBeenCalled();
+    });
+
+    it('should refuse it on a plain search too, where the handler form was served', async () => {
+      const { base, dataSource } = await buildCustomizedDataSource({
+        includeFields: ['holder:nationalId'],
+      });
+      const services = buildServices();
+      const list = jest.spyOn(base.getCollection('cards'), 'list').mockResolvedValue([]);
+
+      await expect(
+        new List(services, options, dataSource, 'cards').handleList(
+          buildContext({ query: { search: 'martin' } }),
+        ),
+      ).rejects.toThrow("You cannot search on 'holder:nationalId'");
+
+      expect(list).not.toHaveBeenCalled();
+    });
+
+    it('should serve the extended search a handler form would have refused', async () => {
+      const { base, dataSource } = await buildCustomizedDataSource({
+        includeFields: ['holder:nationalId'],
+      });
+      const services = buildServices(['holders', 'accounts', 'contracts', 'organizations']);
+      const list = jest.spyOn(base.getCollection('cards'), 'list').mockResolvedValue([]);
+
+      await new List(services, options, dataSource, 'cards').handleList(
+        buildContext({ query: { search: 'martin', searchExtended: '1' } }),
+      );
+
+      // Served, and the search really reached the included path rather than being dropped.
+      expect(JSON.stringify(list.mock.calls[0][1].conditionTree)).toContain('holder:nationalId');
+    });
+
+    it('should still refuse the extended search of the equivalent handler', async () => {
+      const { base, dataSource } = await buildCustomizedDataSource(value => ({
+        field: 'panLast4',
+        operator: 'Contains',
+        value,
+      }));
+      const services = buildServices(['holders', 'accounts', 'contracts', 'organizations']);
+      const list = jest.spyOn(base.getCollection('cards'), 'list').mockResolvedValue([]);
+
+      await expect(
+        new List(services, options, dataSource, 'cards').handleList(
+          buildContext({ query: { search: 'martin', searchExtended: '1' } }),
+        ),
+      ).rejects.toThrow("You cannot run an extended search on the 'cards' collection");
+
+      expect(list).not.toHaveBeenCalled();
+    });
+  });
+
   describe('csv export', () => {
     it('should refuse an export naming a column of an unreadable collection', async () => {
       const dataSource = buildDataSource();

packages/datasource-customizer/src/collection-customizer.ts

@@ -12,7 +12,7 @@ import type {
   UpdateOverrideHandler,
 } from './decorators/override/types';
 import type { RelationDefinition } from './decorators/relation/types';
-import type { SearchDefinition } from './decorators/search/types';
+import type { SearchReplaceDefinition } from './decorators/search/types';
 import type { SegmentDefinition } from './decorators/segment/types';
 import type { WriteDefinition } from './decorators/write/write-replace/types';
 import type {
@@ -595,27 +595,35 @@ export default class CollectionCustomizer<
   }
 
   /**
-   * Replace the behavior of the search bar
+   * Replace the behavior of the search bar, either with a handler or with a field selection.
    *
-   * On a plain search, the fields the handler reads are exempt from read permissions: the caller
-   * supplies the text and the handler picks the fields, so the agent cannot tell an intended one
-   * from one the role may not read. Point a handler at a column of a collection a role cannot read
-   * and that role can test values against it, reading each answer from whether rows come back.
-   * An extended search is refused rather than exempted, because the caller owns that flag and can
-   * compare the same term with it off and on.
-   * @param definition handler to describe the new behavior
+   * A field selection narrows the same default search, so the agent knows which columns are read
+   * and checks them against the caller's read permissions: an extended search keeps working, and a
+   * path the role may not read is refused by name. Prefer it whenever it expresses what you need.
+   * On a collection whose datasource searches natively (`enableSearch()`), it does not narrow that
+   * native search — it replaces it with the agent's own per-column one, restricted to the selection.
+   *
+   * A handler is unrestricted, and pays for it. The fields it reads are exempt from read
+   * permissions on a plain search: the caller supplies the text and the handler picks the fields,
+   * so the agent cannot tell an intended one from one the role may not read. Point a handler at a
+   * column of a collection a role cannot read and that role can test values against it, reading
+   * each answer from whether rows come back. An extended search is refused outright rather than
+   * exempted, because the caller owns that flag and can compare the same term with it off and on.
+   * @param definition a handler describing the new behavior, or the fields the default search reads
    * @see {@link https://docs.forestadmin.com/developer-guide-agents-nodejs/agent-customization/search Documentation Link}
    * @example
+   * .replaceSearch({ includeFields: ['project:name'], excludeFields: ['description'] });
+   * @example
    * .replaceSearch(async (searchString) => {
    *   return { field: 'name', operator: 'Contains', value: searchString };
    * });
    */
-  replaceSearch(definition: SearchDefinition<S, N>): this {
+  replaceSearch(definition: SearchReplaceDefinition<S, N>): this {
     return this.pushCustomization(async () => {
       this.stack.search
         .getCollection(this.name)
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        .replaceSearch(definition as SearchDefinition<any, any>);
+        .replaceSearch(definition as SearchReplaceDefinition<any, any>);
     });
   }
 

packages/datasource-customizer/src/decorators/search/collection.ts

@@ -1,5 +1,10 @@
 import type { SearchOptions } from './collection-search-context';
-import type { SearchDefinition } from './types';
+import type { QueryContext } from './generated-parser/QueryParser';
+import type {
+  SearchFieldsDefinition,
+  SearchHandlerDefinition,
+  SearchReplaceDefinition,
+} from './types';
 import type {
   Caller,
   Collection,
@@ -15,20 +20,28 @@ import type {
 import { CollectionDecorator, ConditionTreeFactory } from '@forestadmin/datasource-toolkit';
 
 import CollectionSearchContext from './collection-search-context';
-import { getLeafCollectionName, getSearchedFieldPaths, lenientGetSchema } from './field-paths';
+import { getLeafCollectionName, lenientGetSchema } from './field-paths';
 import { extractSpecifiedFields, generateConditionTree, parseQuery } from './parse-query';
 
 export default class SearchCollectionDecorator extends CollectionDecorator {
   override dataSource: DataSourceDecorator<SearchCollectionDecorator>;
-  replacer: SearchDefinition = null;
+  replacer: SearchReplaceDefinition = null;
   searchable = true;
 
-  replaceSearch(replacer: SearchDefinition): void {
+  replaceSearch(replacer: SearchReplaceDefinition): void {
     this.searchable = true;
     this.replacer = replacer;
     this.markSchemaAsDirty();
   }
 
+  private get handler(): SearchHandlerDefinition | null {
+    return typeof this.replacer === 'function' ? this.replacer : null;
+  }
+
+  private get fieldSelection(): SearchFieldsDefinition | null {
+    return this.replacer && typeof this.replacer !== 'function' ? this.replacer : null;
+  }
+
   disable() {
     this.searchable = false;
     this.markSchemaAsDirty();
@@ -53,11 +66,12 @@ export default class SearchCollectionDecorator extends CollectionDecorator {
       );
       let tree: ConditionTree;
 
-      if (this.replacer) {
-        const plainTree = await this.replacer(filter.search, filter.searchExtended, ctx);
+      if (this.handler) {
+        const plainTree = await this.handler(filter.search, filter.searchExtended, ctx);
         tree = ConditionTreeFactory.fromPlainObject(plainTree);
       } else {
         tree = this.generateSearchFilter(caller, filter.search, {
+          ...this.fieldSelection,
           extended: filter.searchExtended,
         });
       }
@@ -75,20 +89,28 @@ export default class SearchCollectionDecorator extends CollectionDecorator {
     return filter;
   }
 
-  private generateSearchFilter(
-    caller: Caller,
-    searchText: string,
+  /**
+   * Both the condition tree and the footprint `getSearchedFields` reports must come from here:
+   * a path the search reads without appearing in the footprint is a column read unchecked.
+   */
+  private getSearchableFields(
+    parsedQuery: QueryContext,
     options?: SearchOptions,
-  ): ConditionTree {
-    const parsedQuery = parseQuery(searchText);
-
+  ): Map<string, ColumnSchema> {
     const specifiedFields = options?.onlyFields ? [] : extractSpecifiedFields(parsedQuery);
 
     const defaultFields = options?.onlyFields
       ? []
       : this.getFields(this.childCollection, Boolean(options?.extended));
 
-    const searchableFields = new Map(
+    // Resolved the same way the included ones are, or `excludeFields: ['panLast4']` would silently
+    // fail to drop a `pan_last4` column that `includeFields` resolves. An unresolvable name is kept
+    // as written: it excludes nothing either way.
+    const excludedFields = new Set(
+      (options?.excludeFields ?? []).map(name => lenientGetSchema(this, name)?.field ?? name),
+    );
+
+    return new Map(
       [
         ...defaultFields,
         ...[...specifiedFields, ...(options?.onlyFields ?? []), ...(options?.includeFields ?? [])]
@@ -97,8 +119,17 @@ export default class SearchCollectionDecorator extends CollectionDecorator {
           .map(schema => [schema.field, schema.schema] as [string, ColumnSchema]),
       ]
         .filter(Boolean)
-        .filter(([field]) => !options?.excludeFields?.includes(field)),
+        .filter(([field]) => !excludedFields.has(field)),
     );
+  }
+
+  private generateSearchFilter(
+    caller: Caller,
+    searchText: string,
+    options?: SearchOptions,
+  ): ConditionTree {
+    const parsedQuery = parseQuery(searchText);
+    const searchableFields = this.getSearchableFields(parsedQuery, options);
 
     const conditionTree = generateConditionTree(caller, parsedQuery, [...searchableFields]);
 
@@ -121,15 +152,14 @@ export default class SearchCollectionDecorator extends CollectionDecorator {
 
   /**
    * Answers against `childCollection`, which is what the search actually reads — a field hidden by
-   * the publication or renaming layers above is still searched. Returns `null` when a replacer is
-   * installed: the customer's handler chooses the fields, and the caller only supplies the text.
+   * the publication or renaming layers above is still searched. Returns `null` only when a handler
+   * is installed: it chooses the fields, and the caller only supplies the text.
    */
   override getSearchedFields(search: string, extended: boolean): SearchedField[] | null {
-    if (this.replacer) return null;
+    if (this.handler) return null;
 
     const paths = [
-      ...getSearchedFieldPaths(this.childCollection, search),
-      ...this.getFields(this.childCollection, extended).map(([path]) => path),
+      ...this.getSearchableFields(parseQuery(search), { ...this.fieldSelection, extended }).keys(),
     ];
 
     return paths.map(path => ({

packages/datasource-customizer/src/decorators/search/field-paths.ts

@@ -44,8 +44,12 @@ export function lenientGetSchema(
 }
 
 /**
- * The field paths a search string reaches through the `relation.column:term` syntax, resolved the
- * same way the search decorator resolves them — fuzzily, and across to-many relations too.
+ * The field paths a search string reaches through the `relation.column:term` syntax, resolved
+ * fuzzily and across to-many relations too.
+ *
+ * Unused by the decorator, which derives its own footprint from `getSearchableFields` so the paths
+ * it reports and the ones it searches cannot diverge. Kept only because it has been exported from
+ * the package index since 1.71.x; nothing keeps it in step with what a search actually reads.
  */
 export function getSearchedFieldPaths(collection: Collection, search: string): string[] {
   return extractSpecifiedFields(parseQuery(search))

packages/datasource-customizer/src/decorators/search/types.ts

@@ -1,11 +1,33 @@
 import type CollectionSearchContext from './collection-search-context';
+import type { SearchOptions } from './collection-search-context';
 import type { TCollectionName, TConditionTree, TSchema } from '../../templates';
 
-export type SearchDefinition<
+export type SearchHandlerDefinition<
   S extends TSchema = TSchema,
   N extends TCollectionName<S> = TCollectionName<S>,
 > = (
   value: string,
   extended: boolean,
   context: CollectionSearchContext<S, N>,
 ) => Promise<TConditionTree<S, N>> | TConditionTree<S, N>;
+
+/**
+ * The handler form's published name, kept pointing at the handler alone: it shipped as a callable
+ * type, and widening it to a union would stop compiling for anyone who calls what they typed with
+ * it. `SearchReplaceDefinition` is the union `replaceSearch` accepts.
+ */
+export type SearchDefinition<
+  S extends TSchema = TSchema,
+  N extends TCollectionName<S> = TCollectionName<S>,
+> = SearchHandlerDefinition<S, N>;
+
+/** `extended` is omitted on purpose: that flag is the caller's, and comes from the request. */
+export type SearchFieldsDefinition<
+  S extends TSchema = TSchema,
+  N extends TCollectionName<S> = TCollectionName<S>,
+> = Omit<SearchOptions<S, N>, 'extended'>;
+
+export type SearchReplaceDefinition<
+  S extends TSchema = TSchema,
+  N extends TCollectionName<S> = TCollectionName<S>,
+> = SearchHandlerDefinition<S, N> | SearchFieldsDefinition<S, N>;

packages/datasource-customizer/src/index.ts

@@ -14,7 +14,12 @@ export { ComputedDefinition } from './decorators/computed/types';
 export { OperatorDefinition } from './decorators/operators-emulate/types';
 export { RelationDefinition } from './decorators/relation/types';
 export { getSearchedFieldPaths } from './decorators/search/field-paths';
-export { SearchDefinition } from './decorators/search/types';
+export {
+  SearchDefinition,
+  SearchFieldsDefinition,
+  SearchHandlerDefinition,
+  SearchReplaceDefinition,
+} from './decorators/search/types';
 export { SegmentDefinition } from './decorators/segment/types';
 export * from './decorators/write/write-replace/types';
 export * from './decorators/hook/types';