diff --git a/.changeset/21880-search-companion-field-scope.md b/.changeset/21880-search-companion-field-scope.md new file mode 100644 index 0000000000..ddc169f137 --- /dev/null +++ b/.changeset/21880-search-companion-field-scope.md @@ -0,0 +1,11 @@ +--- +'@objectstack/objectql': patch +--- + +A field-narrowed `$search` no longer matches through the pinyin search companion of a field outside the search-field set (#21880). + +Clause-②: no + +- **What changed.** When the optional pinyin search companion is on (`OS_SEARCH_PINYIN_ENABLED`), the engine's search expansion (`expandSearchToFilter`) adds the companion clause only when every field the companion mirrors is inside the effective search-field set: the set `resolveSearchFields` computes, after any `$searchFields` narrowing. The mirrored fields are read from `resolveSearchCompanionSources`, the same function the companion is provisioned and filled from. +- **What stays the same.** A search with no narrowing keeps the clause whenever the display/name field is in the object's searchable set, so pinyin recall there is unchanged. A CJK term still skips the clause. Deployments with the companion off see no change. +- **Who notices.** A search narrowed to fields that leave out the display/name field, by a `$searchFields` override, by the narrowing global search applies to the fields a caller may query, or by a declared `searchableFields` that omits it, no longer matches through that field's pinyin form. diff --git a/packages/objectql/src/search-companion.test.ts b/packages/objectql/src/search-companion.test.ts index 1d88799f83..92f2a87aaf 100644 --- a/packages/objectql/src/search-companion.test.ts +++ b/packages/objectql/src/search-companion.test.ts @@ -182,6 +182,123 @@ describe('expandSearchToFilter with companion column (query-time, additive)', () }); }); +/** + * [#21880] The companion clause follows the effective search-field set. + * + * The companion is a normalized copy of its source fields, so it may join a + * search only when every field it mirrors is inside the set + * `resolveSearchFields` computed. A field-narrowed search that leaves a + * mirrored field out gets no companion clause; a search with no narrowing + * keeps it, so recall is unchanged there. + */ +describe('[#21880] the companion clause follows the effective search-field set', () => { + // `crm_contact`: the companion mirrors `name` (the derived display field). + const fields = provisionSearchCompanion(contact()).fields as any; + const companionClause = (term: string) => ({ [SEARCH_COMPANION_FIELD]: { $contains: term } }); + + it('premise: the companion is provisioned and mirrors exactly `name`', () => { + expect(fields[SEARCH_COMPANION_FIELD]).toBeDefined(); + expect(resolveSearchCompanionSources({ name: 'crm_contact', fields })).toEqual(['name']); + }); + + describe('(a) a field-narrowed search that leaves the mirrored field out gets no companion clause', () => { + it('a `$searchFields` override without the mirrored field', () => { + expect(expandSearchToFilter('zhangwei', { fields, requestedFields: ['email'] })).toEqual({ + $or: [{ email: { $icontains: 'zhangwei' } }], + }); + // The comma-separated spelling a URL query parameter arrives as. + expect(expandSearchToFilter('zhangwei', { fields, requestedFields: 'email,notes' })).toEqual({ + $or: [{ email: { $icontains: 'zhangwei' } }, { notes: { $icontains: 'zhangwei' } }], + }); + }); + + it('the narrowing carried on the search term itself (`{ query, fields }`)', () => { + expect(expandSearchToFilter({ query: 'zhangwei', fields: ['notes'] }, { fields })).toEqual({ + $or: [{ notes: { $icontains: 'zhangwei' } }], + }); + }); + + it('a declared `searchableFields` set without the mirrored field', () => { + expect(expandSearchToFilter('zhangwei', { fields, searchableFields: ['email'] })).toEqual({ + $or: [{ email: { $icontains: 'zhangwei' } }], + }); + }); + + it('every term of a multi-term search', () => { + const filter = expandSearchToFilter('zhang wei', { fields, requestedFields: ['email'] }); + expect(filter).toEqual({ + $and: [ + { $or: [{ email: { $icontains: 'zhang' } }] }, + { $or: [{ email: { $icontains: 'wei' } }] }, + ], + }); + }); + + it('reads the REAL source: an explicit `nameField` pointer moves what the companion mirrors', () => { + // `crm_ticket` names `subject` as its title, so the companion mirrors + // `subject` — not `name`, although a `name` field exists. + const ticket = provisionSearchCompanion({ + name: 'crm_ticket', + nameField: 'subject', + fields: { subject: { type: 'text' }, name: { type: 'text' } }, + }); + const ticketFields = ticket.fields as any; + expect(resolveSearchCompanionSources(ticket)).toEqual(['subject']); + + const withoutSubject = expandSearchToFilter('zhangwei', { + fields: ticketFields, displayField: 'subject', requestedFields: ['name'], + }); + expect(withoutSubject).toEqual({ $or: [{ name: { $icontains: 'zhangwei' } }] }); + + const withSubject = expandSearchToFilter('zhangwei', { + fields: ticketFields, displayField: 'subject', requestedFields: ['subject'], + }); + expect(withSubject.$or).toContainEqual(companionClause('zhangwei')); + }); + }); + + describe('(b) a search with no narrowing keeps the companion clause (recall unchanged)', () => { + it('the auto-default set, which leads with the mirrored field', () => { + expect(expandSearchToFilter('ZhangWei', { fields })).toEqual({ + $or: [ + { name: { $icontains: 'ZhangWei' } }, + { email: { $icontains: 'ZhangWei' } }, + { notes: { $icontains: 'ZhangWei' } }, + companionClause('zhangwei'), + ], + }); + }); + + it('a narrowed set that still holds the mirrored field', () => { + expect(expandSearchToFilter('zw', { fields, requestedFields: ['name'] })).toEqual({ + $or: [{ name: { $icontains: 'zw' } }, companionClause('zw')], + }); + }); + + it('the gate reads the RESOLVED set: a request naming no allowed field falls back to the full set', () => { + // `resolveSearchFields` drops unknown names and falls back to the + // allowed set when none survives — so the effective set holds `name`. + const filter = expandSearchToFilter('zw', { fields, requestedFields: ['no_such_field'] }); + expect(filter.$or).toContainEqual(companionClause('zw')); + }); + }); + + describe('(c) a CJK term still skips the companion clause, as before', () => { + it('with and without narrowing', () => { + expect(expandSearchToFilter('张伟', { fields })).toEqual({ + $or: [ + { name: { $icontains: '张伟' } }, + { email: { $icontains: '张伟' } }, + { notes: { $icontains: '张伟' } }, + ], + }); + expect(expandSearchToFilter('张伟', { fields, requestedFields: ['name'] })).toEqual({ + $or: [{ name: { $icontains: '张伟' } }], + }); + }); + }); +}); + describe('containsCJK / isCompanionMatchableTerm', () => { it('detects Han characters', () => { expect(containsCJK('张伟')).toBe(true); diff --git a/packages/objectql/src/search-filter.ts b/packages/objectql/src/search-filter.ts index 44a09d96d5..5a2afee370 100644 --- a/packages/objectql/src/search-filter.ts +++ b/packages/objectql/src/search-filter.ts @@ -42,6 +42,13 @@ * `resolveSearchFields` still returns only source fields (the companion is * invisible to `$searchFields` overrides and to clients). * + * [#21880] …and bounded by the same set. The companion is a normalized copy of + * named source fields, so its clause is a match on THOSE fields. It joins a + * search only when every field it mirrors is inside the effective search-field + * set `resolveSearchFields` computed — after any `$searchFields` narrowing — and + * a set that leaves a mirrored field out leaves the companion out with it. See + * {@link companionWithinSearchFields}. + * * [#21009] A field the object declares MULTI-VALUED (`isMultiValueField`: a * `tags` / `multiselect` / `checkboxes` field, or a `select` / `lookup` / * `user` / … declared `multiple: true`) is matched by MEMBERSHIP, `$contains`, @@ -68,7 +75,12 @@ import { type SearchFieldMeta, type SearchFieldResolutionOptions, } from '@objectstack/spec/data'; -import { SEARCH_COMPANION_FIELD, isCompanionMatchableTerm } from './search-companion.js'; +import { + SEARCH_COMPANION_FIELD, + isCompanionMatchableTerm, + resolveSearchCompanionSources, + type CompanionObjectMeta, +} from './search-companion.js'; export { resolveSearchFields, @@ -147,6 +159,44 @@ function fieldClausesForTerm(field: string, term: string, meta: SearchFieldMeta) return [{ [field]: { $icontains: term } }]; } +/** + * [#21880] May the `__search` companion clause join a search over + * `searchFields`? Only when every source field the companion mirrors is in + * that set. + * + * The mirrored fields are read from {@link resolveSearchCompanionSources} — + * the one function the registry's provisioning seam and plugin-pinyin-search's + * populate hook already derive the companion from — over the same `fields` and + * the same display-field pointer the engine handed in. So the answer is the + * companion's real source, never a second guess at it. + * + * ⛔ The gate is `searchFields` and nothing else: the set `resolveSearchFields` + * already computed, with the declared/auto-default precedence and any + * `$searchFields` narrowing applied. No second eligibility rule for the + * companion is consulted here — whatever a caller's narrowing removed from the + * source columns, it removes from their normalized copy too. + * + * The companion is ONE column holding the normalized form of its sources, so + * the test is "every source is in the set", never "some source is": a clause + * over the shared column matches through every field it mirrors at once. + * A search whose set holds every mirrored field — any search with no + * narrowing, whenever the display/name field is in the object's searchable + * set — keeps the clause, so recall there is unchanged. + * + * An empty source list passes vacuously. The registry never provisions a + * companion without a source (`provisionSearchCompanion` returns early on an + * empty list), so that case is only an author-declared `__search` column — + * an ordinary field the platform does not fill — and it keeps today's answer. + */ +function companionWithinSearchFields(searchFields: readonly string[], opts: ExpandSearchOptions): boolean { + const sources = resolveSearchCompanionSources({ + nameField: opts.displayField, + fields: opts.fields as CompanionObjectMeta['fields'], + }); + const inSet = new Set(searchFields); + return sources.every((f) => inSet.has(f)); +} + /** * Expand a `$search` term into a `{ $or: [...] }` (single term) or * `{ $and: [{ $or: [...] }, ...] }` (multi-term) filter. Returns `null` when @@ -177,10 +227,14 @@ export function expandSearchToFilter(raw: unknown, opts: ExpandSearchOptions): a // different mechanism from the source-column clauses in // `fieldClausesForTerm`, which compare against raw stored text and therefore // need `$icontains`. Do not "align" the two. - const hasCompanion = !!opts.fields[SEARCH_COMPANION_FIELD]; + // + // [#21880] …and only when the companion mirrors no field outside + // `searchFields` — see `companionWithinSearchFields`. + const withCompanion = !!opts.fields[SEARCH_COMPANION_FIELD] + && companionWithinSearchFields(searchFields, opts); const andClauses = terms.map((term) => { const clauses = searchFields.flatMap((f) => fieldClausesForTerm(f, term, opts.fields[f] || {})); - if (hasCompanion && isCompanionMatchableTerm(term)) { + if (withCompanion && isCompanionMatchableTerm(term)) { clauses.push({ [SEARCH_COMPANION_FIELD]: { $contains: term.toLowerCase() } }); } return { $or: clauses }; diff --git a/packages/qa/dogfood/package.json b/packages/qa/dogfood/package.json index 7e32e308e6..b6a46783fc 100644 --- a/packages/qa/dogfood/package.json +++ b/packages/qa/dogfood/package.json @@ -26,6 +26,7 @@ "@objectstack/plugin-audit": "workspace:*", "@objectstack/plugin-auth": "workspace:*", "@objectstack/plugin-email": "workspace:*", + "@objectstack/plugin-pinyin-search": "workspace:*", "@objectstack/plugin-security": "workspace:*", "@objectstack/plugin-sharing": "workspace:*", "@objectstack/plugin-webhooks": "workspace:*", diff --git a/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts b/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts new file mode 100644 index 0000000000..e9a3d0a106 --- /dev/null +++ b/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// END-TO-END: with the optional pinyin search companion ON, a member's search +// answers only what the member may see — the rows their row scope admits, and +// matches on the fields they may query. +// +// ## What this boot pins +// +// A real kernel with the real `SecurityPlugin` and the real +// `PinyinSearchPlugin`, over HTTP. Every row is named in CJK, so a pinyin term +// can match it only through the `__search` companion — never through the +// source column. +// +// - ROW-SCOPED: on a `private` object both the member and the administrator +// own a row matching the term. The member's search answers the member's +// own row only; the administrator gets both. +// - FIELD HIDDEN FROM THE MEMBER: the member's permission set hides `name` +// on a second object, and the term is present only in that field. The +// member's search yields no hit — through the global search door and +// through a `searchFields`-narrowed data-door query. A field-narrowed +// search does not match through the companion of a field outside its +// search-field set (`objectql` `expandSearchToFilter`). +// +// Controls keep both "no hit" readings from being vacuous: the administrator's +// search DOES hit through the companion (so the companion is provisioned and +// filled in this boot, and a search with no narrowing keeps it), and the member +// DOES hit the same row through a field they may query (so the object itself is +// searched, not skipped). +// +// Env toggle ⇒ this file stays in the `isolated` vitest project, per the +// eligibility rules in vitest.config.ts. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { PinyinSearchPlugin } from '@objectstack/plugin-pinyin-search'; + +/** `翟璐` normalizes to `zhailu zl` — the term below lives in the companion only. */ +const CJK_NAME = '翟璐'; +const PINYIN_TERM = 'zhailu'; +/** A latin value in a field the member MAY query. */ +const CODE = 'qv7code'; + +const ROWS = 'cmpscope_rows'; +const HIDDEN = 'cmpscope_hidden'; + +const RowScoped = ObjectSchema.create({ + name: ROWS, + sharingModel: 'private', + label: 'Companion Scope Rows', + pluralLabel: 'Companion Scope Rows', + fields: { name: Field.text({ label: 'Name', required: true, searchable: true }) }, +}); + +const HiddenField = ObjectSchema.create({ + name: HIDDEN, + sharingModel: 'public_read_write', + label: 'Companion Scope Hidden', + pluralLabel: 'Companion Scope Hidden', + fields: { + name: Field.text({ label: 'Name', required: true, searchable: true }), + code: Field.text({ label: 'Code', searchable: true }), + }, +}); + +const stackDef = defineStack({ + manifest: { + id: 'com.dogfood.search-companion-field-scope', + namespace: 'cmpscope', + version: '0.0.0', + type: 'app', + name: 'Search Companion Field Scope Fixture', + description: 'A row-scoped object and an object whose name field is hidden from the member.', + }, + objects: [RowScoped, HiddenField], +}); + +const objectGrants = { + [ROWS]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + [HIDDEN]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, +}; + +/** Everyone's fallback: the object grants, no field rule. */ +const baselineSet: PermissionSet = PermissionSetSchema.parse({ + name: 'cmpscope_baseline', + label: 'Companion Scope Baseline — object grants only', + objects: objectGrants, +}); + +/** The member's own set: the same grants, with `name` hidden on `cmpscope_hidden`. */ +const memberSet: PermissionSet = PermissionSetSchema.parse({ + name: 'cmpscope_member', + label: 'Companion Scope Member — name hidden on cmpscope_hidden', + objects: objectGrants, + fields: { [`${HIDDEN}.name`]: { readable: false, editable: false } }, +}); + +const SYS = { context: { isSystem: true } } as const; + +interface SearchBody { + hits: Array<{ object: string; id: string; title: string }>; + totalObjects: number; + totalHits: number; +} + +let stack: VerifyStack; +let adminToken: string; +let memberToken: string; +let memberRowId: string; +let adminRowId: string; +let hiddenRowId: string; + +const PINYIN_ENV = process.env.OS_SEARCH_PINYIN_ENABLED; + +function createdId(body: unknown): string { + const b = body as { id?: unknown; record?: { id?: unknown }; data?: { id?: unknown } }; + const id = b.id ?? b.record?.id ?? b.data?.id; + expect(typeof id).toBe('string'); + return String(id); +} + +async function create(token: string, object: string, data: Record): Promise { + const res = await stack.apiAs(token, 'POST', `/data/${object}`, data); + const text = await res.text(); + expect(res.status, text).toBeLessThan(300); + return createdId(JSON.parse(text)); +} + +async function search(token: string, query: string): Promise<{ status: number; text: string; body: SearchBody }> { + const res = await stack.apiAs(token, 'GET', `/search?${query}`); + const text = await res.text(); + return { status: res.status, text, body: JSON.parse(text) as SearchBody }; +} + +function hitIds(body: SearchBody, object: string): string[] { + return body.hits.filter((h) => h.object === object).map((h) => h.id).sort(); +} + +async function dataQuery(token: string, object: string, body: Record): Promise<{ status: number; text: string; ids: string[] }> { + const res = await stack.apiAs(token, 'POST', `/data/${object}/query`, body); + const text = await res.text(); + const parsed = JSON.parse(text) as { data?: { records?: unknown[] }; records?: unknown[] }; + const records = (parsed?.data?.records ?? parsed?.records ?? []) as Array<{ id?: unknown }>; + return { status: res.status, text, ids: records.map((r) => String(r.id)).sort() }; +} + +describe('dogfood: with the pinyin search companion on, a member searches only what they may see', () => { + beforeAll(async () => { + // The registry provisions `__search` only while this is on, and reads it + // when it is constructed — so it is set before the boot. + process.env.OS_SEARCH_PINYIN_ENABLED = '1'; + stack = await bootStack(stackDef as never, { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, baselineSet, memberSet], + fallbackPermissionSet: baselineSet.name, + }), + extraPlugins: [new PinyinSearchPlugin({ enabled: true, backfill: false })], + }); + adminToken = await stack.signIn(); + const memberEmail = 'cmpscope-member@verify.test'; + memberToken = await stack.signUp(memberEmail); + + // Bind the member to its own set (the fallback stays everyone else's). + const ql = await stack.kernel.getServiceAsync<{ + findOne(object: string, opts: unknown): Promise<{ id?: unknown } | null>; + insert(object: string, data: Record, opts: unknown): Promise; + }>('objectql'); + const idOf = async (object: string, where: Record) => + String((await ql.findOne(object, { where, ...SYS }))?.id ?? ''); + const userId = await idOf('sys_user', { email: memberEmail }); + const setId = await idOf('sys_permission_set', { name: memberSet.name }); + expect(userId, 'member user seeded').toBeTruthy(); + expect(setId, 'member permission set seeded').toBeTruthy(); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + + memberRowId = await create(memberToken, ROWS, { name: CJK_NAME }); + adminRowId = await create(adminToken, ROWS, { name: CJK_NAME }); + hiddenRowId = await create(adminToken, HIDDEN, { name: CJK_NAME, code: CODE }); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + if (PINYIN_ENV === undefined) delete process.env.OS_SEARCH_PINYIN_ENABLED; + else process.env.OS_SEARCH_PINYIN_ENABLED = PINYIN_ENV; + }); + + describe('a row-scoped object', () => { + it('control: the administrator gets both matching rows through the companion', async () => { + const { status, text, body } = await search(adminToken, `q=${PINYIN_TERM}&objects=${ROWS}`); + expect(status, text).toBe(200); + expect(hitIds(body, ROWS)).toEqual([memberRowId, adminRowId].sort()); + }); + + it("the member's search answers only the member's own matching row", async () => { + const { status, text, body } = await search(memberToken, `q=${PINYIN_TERM}&objects=${ROWS}`); + expect(status, text).toBe(200); + expect(hitIds(body, ROWS)).toEqual([memberRowId]); + expect(text).not.toContain(adminRowId); + + // The data door answers the same. + const viaData = await dataQuery(memberToken, ROWS, { search: PINYIN_TERM }); + expect(viaData.status, viaData.text).toBe(200); + expect(viaData.ids).toEqual([memberRowId]); + }); + }); + + describe('a term present only in a field hidden from the member', () => { + it('control: `name` is hidden from the member at the data door', async () => { + const res = await stack.apiAs(memberToken, 'GET', `/data/${HIDDEN}/${hiddenRowId}`); + const text = await res.text(); + expect(res.status, text).toBe(200); + expect(text).toContain(CODE); + expect(text).not.toContain(CJK_NAME); + }); + + it('control: the administrator hits the row through the companion (no narrowing keeps it)', async () => { + const { status, text, body } = await search(adminToken, `q=${PINYIN_TERM}&objects=${HIDDEN}`); + expect(status, text).toBe(200); + expect(hitIds(body, HIDDEN)).toEqual([hiddenRowId]); + }); + + it('control: the member hits the same row through a field they may query', async () => { + const { status, text, body } = await search(memberToken, `q=${CODE}&objects=${HIDDEN}`); + expect(status, text).toBe(200); + expect(hitIds(body, HIDDEN)).toEqual([hiddenRowId]); + // …and the hit carries nothing of the hidden field. + expect(text).not.toContain(CJK_NAME); + }); + + it("the member's global search yields no hit", async () => { + const { status, text, body } = await search(memberToken, `q=${PINYIN_TERM}&objects=${HIDDEN}`); + expect(status, text).toBe(200); + expect(body.hits).toEqual([]); + expect(body.totalHits).toBe(0); + }); + + it("the member's searchFields-narrowed data-door query yields no hit", async () => { + const narrowed = await dataQuery(memberToken, HIDDEN, { search: PINYIN_TERM, searchFields: ['code'] }); + expect(narrowed.status, narrowed.text).toBe(200); + expect(narrowed.ids).toEqual([]); + + // Same door, same narrowing, a term in the queryable field: the row. + const control = await dataQuery(memberToken, HIDDEN, { search: CODE, searchFields: ['code'] }); + expect(control.status, control.text).toBe(200); + expect(control.ids).toEqual([hiddenRowId]); + }); + }); +}); diff --git a/packages/qa/dogfood/vitest.config.ts b/packages/qa/dogfood/vitest.config.ts index 841365bfeb..9d2fb25c69 100644 --- a/packages/qa/dogfood/vitest.config.ts +++ b/packages/qa/dogfood/vitest.config.ts @@ -295,6 +295,15 @@ export default defineConfig({ find: /^@objectstack\/service-datasource$/, replacement: path.resolve(__dirname, '../../services/service-datasource/src/index.ts'), }, + // [#21880] `search-companion-field-scope.dogfood.test.ts` mounts + // `PinyinSearchPlugin` on a real boot, and the companion values it + // writes are what every search in that file matches through. The + // plugin's fill path is part of the pin's subject, so the verdict + // is aliased to THIS checkout's source, not to the last `pnpm build`. + { + find: /^@objectstack\/plugin-pinyin-search$/, + replacement: path.resolve(__dirname, '../../plugins/plugin-pinyin-search/src/index.ts'), + }, ], }, test: { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b383960ab3..54d0f0a6d0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2069,6 +2069,9 @@ importers: '@objectstack/plugin-email': specifier: workspace:* version: link:../../plugins/plugin-email + '@objectstack/plugin-pinyin-search': + specifier: workspace:* + version: link:../../plugins/plugin-pinyin-search '@objectstack/plugin-security': specifier: workspace:* version: link:../../plugins/plugin-security