diff --git a/apps/sim/lib/workflows/editing/selector-reference-guard.test.ts b/apps/sim/lib/workflows/editing/selector-reference-guard.test.ts new file mode 100644 index 00000000000..0853eb0eeb2 --- /dev/null +++ b/apps/sim/lib/workflows/editing/selector-reference-guard.test.ts @@ -0,0 +1,78 @@ +/** + * @vitest-environment node + * + * Integration coverage for the Tier-2 reference guard, against the REAL block registry. + * + * `validation.test.ts` exercises the same guard with hand-written block fixtures, which cannot + * catch a fixture that has drifted from the shipped block config. This file unmocks the registry + * so the canonical pair, its `mode`s and its sub-block types come from `knowledge.ts` itself. + * Only the database lookup is mocked - it is the one dependency the guard deliberately protects. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.unmock('@/blocks/registry') + +const { mockValidateSelectorIds } = vi.hoisted(() => ({ + mockValidateSelectorIds: vi.fn(), +})) + +vi.mock('@/lib/workflows/editing/selector-validator', () => ({ + validateSelectorIds: mockValidateSelectorIds, +})) + +import { collectUnresolvedReferences } from '@/lib/workflows/editing/validation' +import { getBlock } from '@/blocks/registry' + +const CTX = { userId: 'user-1', workspaceId: 'workspace-1' } as const + +/** The real basic member of the knowledge block's `knowledgeBaseId` canonical pair. */ +const KB_SELECTOR_ID = 'knowledgeBaseSelector' + +function knowledgeGraph(value: string) { + return { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { [KB_SELECTOR_ID]: { value } }, + }, + }, + } +} + +describe('Tier-2 reference guard (real block registry)', () => { + beforeEach(() => { + vi.clearAllMocks() + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [] }) + }) + + it('the fixture matches the shipped knowledge block, so the cases below are meaningful', () => { + const config = getBlock('knowledge') + const member = config?.subBlocks?.find((s) => s.id === KB_SELECTOR_ID) + expect(member).toBeDefined() + expect(member?.type).toBe('knowledge-base-selector') + expect(member?.canonicalParamId).toBe('knowledgeBaseId') + expect(member?.mode).toBe('basic') + }) + + it.each([ + ['a block-output reference', ''], + ['an env-var reference', '{{KB_ID}}'], + ['a partially templated value', 'kb_'], + ])('never hits the database for %s', async (_label, value) => { + // Seeded so `toHaveLength(0)` is load-bearing: with a permissive mock it would pass + // whether or not the guard ran. + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [value] }) + const refs = await collectUnresolvedReferences(knowledgeGraph(value), CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('still reports a literal id that does not resolve', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_gone'] }) + const refs = await collectUnresolvedReferences(knowledgeGraph('kb_gone'), CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith('knowledge-base-selector', 'kb_gone', CTX) + expect(refs).toHaveLength(1) + expect(refs[0]).toMatchObject({ blockId: 'kb1', field: KB_SELECTOR_ID, kind: 'resource' }) + }) +}) diff --git a/apps/sim/lib/workflows/editing/validation.test.ts b/apps/sim/lib/workflows/editing/validation.test.ts index bd1a65a2d89..3aae3aa0ad4 100644 --- a/apps/sim/lib/workflows/editing/validation.test.ts +++ b/apps/sim/lib/workflows/editing/validation.test.ts @@ -1197,6 +1197,425 @@ describe('collectUnresolvedReferences', () => { expect(refs).toHaveLength(1) expect(refs[0]).toMatchObject({ field: 'credential', kind: 'credential' }) }) + + it('does not validate a selector holding a reference', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [''] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: '' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('does not validate a selector holding a {{ENV_VAR}} reference', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['{{KB_ID}}'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: '{{KB_ID}}' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('does not validate a partially templated selector value', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: 'kb_' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('validates only the literal ids in a mixed comma-separated value', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: 'kb_missing,' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith( + 'knowledge-base-selector', + ['kb_missing'], + CTX + ) + expect(refs).toHaveLength(1) + }) + + it('validates the literal entries when a multi-select opens AND closes with a template', async () => { + // `isReference` is unanchored (startsWith '<' && endsWith '>'), so this value reads as one + // whole reference. Filtering per entry is what keeps `kb_real` validated. + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_real'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: ',kb_real,' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith( + 'knowledge-base-selector', + ['kb_real'], + CTX + ) + expect(refs).toHaveLength(1) + }) + + it('never hits the database when every entry of a mixed-delimiter list is templated', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['{{A}}'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: '{{A}},' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('skips a single oversized entry, which cannot be classified cheaply', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['x'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: `kb_${'a'.repeat(10_000)}` } }, + }, + }, + } + const startedAt = performance.now() + const refs = await collectUnresolvedReferences(state, CTX) + expect(performance.now() - startedAt).toBeLessThan(1000) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('keeps a comma-bearing reference intact even in an oversized list', async () => { + // Splitting scans reference regions directly and stays linear, so size does not force a + // fallback that would tear `` into fragments validated as ids. + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] }) + const padding = Array.from({ length: 400 }, (_, index) => `kb_${'x'.repeat(30)}${index}`) + const value = [...padding, '', 'kb_missing'].join(',') + expect(value.length).toBeGreaterThan(10_000) + const state = { + blocks: { + kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value } } }, + }, + } + await collectUnresolvedReferences(state, CTX) + + const [, ids] = mockValidateSelectorIds.mock.calls[0] + expect(ids).toContain('kb_missing') + expect(ids).not.toContain('') + }) + + it('still validates the literal entries of an oversized list', async () => { + // The cap gives up reference-aware splitting, not validation: the entries are still short, + // so each is classified and the literals are still checked. + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] }) + const padding = Array.from({ length: 400 }, (_, index) => `kb_${'x'.repeat(30)}${index}`) + const value = [...padding, '', 'kb_missing'].join(',') + expect(value.length).toBeGreaterThan(10_000) + const state = { + blocks: { + kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value } } }, + }, + } + const startedAt = performance.now() + const refs = await collectUnresolvedReferences(state, CTX) + + expect(performance.now() - startedAt).toBeLessThan(1000) + const [, ids] = mockValidateSelectorIds.mock.calls[0] + expect(ids).toContain('kb_missing') + expect(ids).not.toContain('') + expect(refs).toHaveLength(1) + }) + + it('still validates a plain literal id (the guard must not over-skip)', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: 'kb_missing' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith( + 'knowledge-base-selector', + 'kb_missing', + CTX + ) + expect(refs).toHaveLength(1) + }) + + // `splitOutsideReferences` is what keeps a comma INSIDE a reference from becoming a separator. + // Every other reference test above would still pass with a naive `.split(',')` (no comma -> + // never split at all), so these two are the only ones that pin the reference-aware split from + // the consumer's side: a torn reference reads as plain literals and gets validated as ids. + it('does not split a reference that contains a comma', async () => { + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: '' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('does not split a {{ENV_VAR}} reference that contains a comma', async () => { + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: '{{KB_A,KB_B}}' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('skips a comma-separated value whose entries are ALL templates', async () => { + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: ',{{KB_ID}}' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + // A multi-select that already stores a native array never reaches the comma split, so the + // array filter is entered by a second, independent route. + it('filters templates out of a value that is already an array', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: ['kb_missing', '', '{{KB_ID}}'] } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith( + 'knowledge-base-selector', + ['kb_missing'], + CTX + ) + expect(refs).toHaveLength(1) + }) + + it('skips an array value whose entries are ALL templates', async () => { + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: ['', '{{KB_ID}}'] } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + // A separator-only string is truthy, so it survives the `!subBlockValue` bail and reaches the + // split, which returns nothing. Reusing the all-references bail is what stops an empty list + // from being sent to the database as if it were a set of ids. + it('skips a value that is nothing but separators', async () => { + const state = { + blocks: { + kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value: ' , , ' } } }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + it('skips an empty array value rather than validating an empty list', async () => { + const state = { + blocks: { + kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value: [] } } }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) + + // The per-entry filter calls `containsReference` on whatever the array holds, so its + // non-string bail is load-bearing here - without it a numeric entry throws. A non-string can + // never be a reference, so it must survive untouched, `null` included: the `filter(Boolean)` + // that would have dropped it lives inside the comma split, which a native array never reaches. + it('does not throw on a non-string entry inside an array value', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: [42, null, 'kb_ok', ''] } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith( + 'knowledge-base-selector', + [42, null, 'kb_ok'], + CTX + ) + expect(refs).toHaveLength(0) + }) + + // Both delimiters are required. A lone `<` (or a lone `{{`) is a malformed literal, not a + // template, and must keep being reported rather than silently waved through. + it.each([ + ['an unclosed < delimiter', 'kb_ delimiter', 'start.kbId>'], + ['an unclosed {{ delimiter', 'kb_{{KB_ID'], + ])('still validates a value with %s', async (_label, value) => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [value] }) + const state = { + blocks: { + kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value } } }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith('knowledge-base-selector', value, CTX) + expect(refs).toHaveLength(1) + }) + + it('drops whitespace-only entries without validating an empty id', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_missing'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: 'kb_missing, , ,' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).toHaveBeenCalledWith( + 'knowledge-base-selector', + ['kb_missing'], + CTX + ) + expect(refs).toHaveLength(1) + }) + + // The guard runs AFTER the canonical active-member check, so a template in the active member + // must be skipped by the guard - and must not push mode resolution onto the empty twin. + it('skips a template held by the ACTIVE canonical member', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [''] }) + const state = { + blocks: { + c1: { + type: 'canonicalcred', + name: 'Cred', + subBlocks: { credential: { value: '' }, manualCredential: { value: '' } }, + }, + }, + } + const refs = await collectUnresolvedReferences(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(refs).toHaveLength(0) + }) +}) + +// The lint path (collectUnresolvedReferences) and the agent edit path share collectSelectorFields, +// but only the edit path can REJECT an operation. A dynamically-bound selector must not block an +// edit - that rejection is the user-visible failure this guard exists to prevent. +describe('validateWorkflowSelectorIds (reference guard)', () => { + beforeEach(() => { + vi.clearAllMocks() + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [] }) + }) + + it.each([ + ['a block-output reference', ''], + ['an env-var reference', '{{KB_ID}}'], + ['a partially templated value', 'kb_'], + ])('does not reject an edit whose selector holds %s', async (_label, value) => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: [value] }) + const state = { + blocks: { + kb1: { type: 'knowledge', name: 'KB', subBlocks: { knowledgeBaseId: { value } } }, + }, + } + const errors = await validateWorkflowSelectorIds(state, CTX) + expect(mockValidateSelectorIds).not.toHaveBeenCalled() + expect(errors).toHaveLength(0) + }) + + it('still rejects an edit whose selector holds a literal id that does not resolve', async () => { + mockValidateSelectorIds.mockResolvedValue({ valid: [], invalid: ['kb_gone'] }) + const state = { + blocks: { + kb1: { + type: 'knowledge', + name: 'KB', + subBlocks: { knowledgeBaseId: { value: 'kb_gone' } }, + }, + }, + } + const errors = await validateWorkflowSelectorIds(state, CTX) + expect(errors).toHaveLength(1) + expect(errors[0]?.error).toContain('kb_gone') + }) }) describe('validateInputsForBlock - agent tools (tool-input)', () => { diff --git a/apps/sim/lib/workflows/editing/validation.ts b/apps/sim/lib/workflows/editing/validation.ts index ec2a366a1bd..a9ad055967a 100644 --- a/apps/sim/lib/workflows/editing/validation.ts +++ b/apps/sim/lib/workflows/editing/validation.ts @@ -10,7 +10,7 @@ import type { PermissionGroupConfig } from '@/lib/permission-groups/fields' import { resolveAccessControlBlockType } from '@/lib/permission-groups/integration-allowlist' import { getCustomToolById } from '@/lib/workflows/custom-tools/operations' import { validateSelectorIds } from '@/lib/workflows/editing/selector-validator' -import { containsReference } from '@/lib/workflows/sanitization/references' +import { containsReference, splitOutsideReferences } from '@/lib/workflows/sanitization/references' import { getSkillById } from '@/lib/workflows/skills/operations' import { buildCanonicalIndex, @@ -1082,6 +1082,17 @@ interface SelectorFieldToValidate { value: string | string[] } +/** + * Longest single selector entry `containsReference` will classify. + * + * Classifying one entry tokenizes it, and `findWorkflowReferenceTokens` is superlinear in + * candidate count, so an entry of unbounded length is expensive on a write path that admits + * megabytes. Splitting is unaffected - it scans reference regions directly and stays linear - so + * only an individual oversized ENTRY is skipped, where there is no cheap way to tell a literal + * from a dynamic binding. A long LIST of ordinary ids still splits and validates normally. + */ +const MAX_SELECTOR_ENTRY_LENGTH = 10_000 + /** * Walk a workflow state and collect selector/credential fields to validate. * For canonical pairs only the ACTIVE member is collected (an intentionally-empty @@ -1136,13 +1147,27 @@ function collectSelectorFields( const subBlockValue = blockData.subBlocks?.[subBlockConfig.id]?.value if (!subBlockValue) continue + const isOversized = (entry: unknown) => + typeof entry === 'string' && entry.length > MAX_SELECTOR_ENTRY_LENGTH + // Handle comma-separated values for multi-select let values: string | string[] = subBlockValue if (typeof subBlockValue === 'string' && subBlockValue.includes(',')) { - values = subBlockValue - .split(',') - .map((v: string) => v.trim()) - .filter(Boolean) + values = splitOutsideReferences(subBlockValue) + } + + // A dynamically bound value only acquires its id at execution time, so a static + // id-existence check cannot evaluate it. Filtered per entry rather than on the whole + // string, because a multi-select can mix literal ids with dynamic ones: testing + // `,kb_real,` as a whole would drop `kb_real` along with the references. + if (Array.isArray(values)) { + const literalValues = values.filter( + (entry) => !isOversized(entry) && !containsReference(entry) + ) + if (literalValues.length === 0) continue + values = literalValues + } else if (isOversized(values) || containsReference(values)) { + continue } fields.push({ diff --git a/apps/sim/lib/workflows/sanitization/references.test.ts b/apps/sim/lib/workflows/sanitization/references.test.ts index 42786b5b340..23d2416ee06 100644 --- a/apps/sim/lib/workflows/sanitization/references.test.ts +++ b/apps/sim/lib/workflows/sanitization/references.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { containsReference, isLikelyReferenceSegment, + splitOutsideReferences, splitReferenceSegment, } from '@/lib/workflows/sanitization/references' @@ -90,3 +91,63 @@ describe('containsReference', () => { expect(containsReference('ad')).toBe(false) }) }) + +describe('splitOutsideReferences', () => { + it('splits on separator commas', () => { + expect(splitOutsideReferences('kb_a,kb_b')).toEqual(['kb_a', 'kb_b']) + }) + + it('trims entries and drops empties', () => { + expect(splitOutsideReferences(' kb_a , , kb_b ')).toEqual(['kb_a', 'kb_b']) + }) + + it('keeps a comma that sits inside a workflow reference', () => { + expect(splitOutsideReferences('')).toEqual(['']) + }) + + it('keeps a comma inside a reference while still splitting around it', () => { + expect(splitOutsideReferences('kb_a,,kb_b')).toEqual([ + 'kb_a', + '', + 'kb_b', + ]) + }) + + it('keeps a comma inside an env-var placeholder', () => { + expect(splitOutsideReferences('{{A,B}},kb_a')).toEqual(['{{A,B}}', 'kb_a']) + }) + + it('returns a single entry when there is no separator', () => { + expect(splitOutsideReferences('kb_a')).toEqual(['kb_a']) + }) + + it('stays linear on a large value instead of rescanning tokens per comma', () => { + // A per-comma `tokens.some()` is O(commas x tokens) and took ~2.5s on this input. + const value = '{{A}},'.repeat(40000) + const startedAt = performance.now() + const parts = splitOutsideReferences(value) + const elapsedMs = performance.now() - startedAt + + expect(parts).toHaveLength(40000) + expect(elapsedMs).toBeLessThan(1000) + }) + + it('protects a comma inside a reference that nests an env-var placeholder', () => { + // `findWorkflowReferenceTokens` is non-overlapping, so it reports only the inner `{{A}}` and + // drops the outer candidate. The candidate pass is what keeps the outer region protected. + expect(splitOutsideReferences(',kb_literal')).toEqual([ + '', + 'kb_literal', + ]) + expect(splitOutsideReferences(',kb_literal')).toEqual([ + '', + 'kb_literal', + ]) + }) + + it('still splits a near-miss that does not read as a reference', () => { + // `` fails `isLikelyReferenceSegment` (the `+`), so it is not a protected region. + // Loud rather than silent: the fragments are reported as ids that do not resolve. + expect(splitOutsideReferences('')).toEqual(['']) + }) +}) diff --git a/apps/sim/lib/workflows/sanitization/references.ts b/apps/sim/lib/workflows/sanitization/references.ts index 5579605148e..fd5963f31c8 100644 --- a/apps/sim/lib/workflows/sanitization/references.ts +++ b/apps/sim/lib/workflows/sanitization/references.ts @@ -1,4 +1,5 @@ import { + ENV_REFERENCE_PATTERN, findWorkflowReferenceTokens, isLikelyWorkflowReferenceSegment, splitWorkflowReferenceSegment, @@ -23,6 +24,79 @@ export function containsReference(value: unknown): boolean { return findWorkflowReferenceTokens(value).length > 0 } +/** + * Mark every region of `value` that belongs to a reference, as a union rather than a partition. + * + * Deliberately does NOT use `findWorkflowReferenceTokens`. That returns contractually + * non-overlapping tokens, which costs an O(tokens^2) overlap check and drops a `<...>` candidate + * that wraps an `{{ENV_VAR}}` - both wrong here. A splitter only needs to know whether an index is + * inside SOME reference, so scanning each kind independently is both cheaper and more accurate. + */ +function markReferenceRegions(value: string, insideReference: Uint8Array): void { + const mark = (start: number, end: number) => { + for (let index = Math.max(start, 0); index < Math.min(end, value.length); index += 1) { + insideReference[index] = 1 + } + } + + for (const match of value.matchAll(ENV_REFERENCE_PATTERN)) { + mark(match.index, match.index + match[0].length) + } + + let candidateStart = -1 + for (let index = 0; index < value.length; index += 1) { + const character = value[index] + if (character === REFERENCE.START && candidateStart === -1) { + candidateStart = index + continue + } + if (character === '\r' || character === '\n') { + candidateStart = -1 + continue + } + if (character !== REFERENCE.END || candidateStart === -1) continue + + const candidate = value.slice(candidateStart, index + REFERENCE.END.length) + const split = splitReferenceSegment(candidate) + if (split && isLikelyReferenceSegment(candidate)) { + const start = candidateStart + split.leading.length + mark(start, start + split.reference.length) + } + candidateStart = -1 + } +} + +/** + * Split a comma-separated multi-select value without tearing a reference apart. + * + * A `` or `{{ENV_VAR}}` token may legitimately contain a comma (``), + * and its fragments read as plain literals once split, so a naive `.split(',')` turns one dynamic + * value into several bogus ones. Only commas outside every reference region are separators. + */ +export function splitOutsideReferences(value: string): string[] { + if (!value.includes(REFERENCE.START) && !value.includes(REFERENCE.ENV_VAR_START)) { + return value + .split(',') + .map((part) => part.trim()) + .filter(Boolean) + } + + const insideReference = new Uint8Array(value.length) + markReferenceRegions(value, insideReference) + + const parts: string[] = [] + let partStart = 0 + for (let index = 0; index < value.length; index += 1) { + if (value[index] === ',' && !insideReference[index]) { + parts.push(value.slice(partStart, index)) + partStart = index + 1 + } + } + parts.push(value.slice(partStart)) + + return parts.map((part) => part.trim()).filter(Boolean) +} + export function extractReferencePrefixes(value: string): Array<{ raw: string; prefix: string }> { if (!value || typeof value !== 'string') { return [] diff --git a/packages/utils/src/workflow-references.ts b/packages/utils/src/workflow-references.ts index 64bc63b76ab..3fc84c8a1a4 100644 --- a/packages/utils/src/workflow-references.ts +++ b/packages/utils/src/workflow-references.ts @@ -3,7 +3,11 @@ const REFERENCE_END = '>' const REFERENCE_PATH_DELIMITER = '.' const INVALID_REFERENCE_CHARS = /[+*/=<>!&|]/ const LEADING_REFERENCE_PATTERN = /^[<>=!\s]*$/ -const ENV_REFERENCE_PATTERN = /\{\{[^{}\r\n]+\}\}/g +/** + * `{{ENV_VAR}}` placeholders. Exported so a consumer that needs the UNION of reference regions + * can find them without going through the non-overlapping token pass. + */ +export const ENV_REFERENCE_PATTERN = /\{\{[^{}\r\n]+\}\}/g export type WorkflowReferenceTokenKind = 'environment' | 'workflow'