Skip to content

feat(core): RundownPlaylist filters in Action Triggers#1652

Merged
jstarpl merged 3 commits intoSofie-Automation:mainfrom
nrkno:contrib/sofie-4219/actions-adlib-rundown-state-filter
Mar 3, 2026
Merged

feat(core): RundownPlaylist filters in Action Triggers#1652
jstarpl merged 3 commits intoSofie-Automation:mainfrom
nrkno:contrib/sofie-4219/actions-adlib-rundown-state-filter

Conversation

@jstarpl
Copy link
Copy Markdown
Contributor

@jstarpl jstarpl commented Feb 13, 2026

About the Contributor

This pull request is posted on behalf of the NRK.

Type of Contribution

This is a:

Feature

Current Behavior

The RundownPlaylist filter type for Action Triggers has no UI available.

New Behavior

obraz

A UI for different types of Filters is added. Rehearsal state of the Rundown can be used as a filter condition.

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

Affected areas

This PR affects the Action Triggers system.

Time Frame

Not urgent, but we would like to get this merged into the in-development release.

Other Information

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

Add UI for RundownPlaylist filters for AdLibs
@jstarpl jstarpl requested a review from a team as a code owner February 13, 2026 14:21
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 13, 2026

Walkthrough

Adds studioId and rehearsal to rundown-playlist types/projections, extends filter link types and UI to switch filter kinds, and integrates rundown-playlist constraints into ad-lib filter compilation with early-skip evaluation.

Changes

Cohort / File(s) Summary
Server: deviceTriggers types & projections
meteor/server/api/deviceTriggers/StudioObserver.ts, meteor/server/api/deviceTriggers/reactiveContentCache.ts
Added studioId and rehearsal to RundownPlaylistFields and updated Mongo projection specifiers.
Server: runtime filter context
meteor/server/api/deviceTriggers/triggersContext.ts
Switched studioId matching to exact protected-string equality and added rehearsal filter handling.
Blueprints type defs
packages/blueprints-integration/src/triggers.ts
Added rehearsal variant to IRundownPlaylistFilterLink and exported FilterType union.
Meteor lib: action/filter compilation
packages/meteor-lib/src/triggers/actionFactory.ts, packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts
Exposed studioId/rehearsal on reactive playlist context; added RundownSelector, compileRundownPlaylistFilter, replaced CompiledFilter with CompiledAdLibFilter, and integrated rundown-playlist constraint checks (activationId, name, studioId, rehearsal) with optional early-skip.
Client runtime handler
packages/webui/src/client/lib/triggers/TriggersHandler.tsx
Expanded playlist query projection and local cast to include studioId and rehearsal.
UI: filter-type switching component
packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/SwitchFilterType.tsx
New presentational ButtonGroup component to select among adLib, rundownPlaylist, and view filter types.
UI: editor integration & props
packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/ActionEditor.tsx, .../FilterEditor.tsx
Added FilterType usage, onChangeType callbacks, and integrated SwitchFilterType into the FilterEditor UI.
UI: filter preview components
packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/AdLibFilter.tsx, .../ViewFilter.tsx
Added onChangeType prop and wiring to propagate type changes from the editor.
UI: rundown-playlist filter editor refactor
packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/RundownPlaylistFilter.tsx
Refactored into a full FilterEditor-driven component, added props (index, readonly, opened, onChangeType, etc.), field mappings and support for activationId, name, studioId, rehearsal, and dynamic studio options.
UI: small wiring
packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/AdLibFilter.tsx, .../FilterEditor.tsx
Prop and signature updates to accept and propagate onChangeType and filterType props.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ActionEditor
    participant SwitchFilterType
    participant FilterEditor
    participant RundownPlaylistFilter
    participant Compiler as ActionFilterCompiler
    participant Runtime as TriggersRuntime

    User->>ActionEditor: open editor / edit filter
    ActionEditor->>FilterEditor: render(filterType)
    FilterEditor->>SwitchFilterType: render switch (allowedTypes)
    User->>SwitchFilterType: click "rundownPlaylist"
    SwitchFilterType->>FilterEditor: onChangeType("rundownPlaylist")
    FilterEditor->>ActionEditor: onChangeType(index, "rundownPlaylist")
    ActionEditor->>RundownPlaylistFilter: render editor for rundownPlaylist
    User->>RundownPlaylistFilter: select field (e.g., studioId) and value
    RundownPlaylistFilter->>ActionEditor: onChange(link)
    ActionEditor->>Compiler: save & compile filter chain
    Compiler->>Compiler: compileRundownPlaylistFilter() -> selector
    Runtime->>Runtime: evaluate selector vs current rundownPlaylist
    alt selector matches
        Runtime->>Runtime: continue ad-lib filter evaluation
    else selector does not match
        Runtime->>Runtime: early-skip ad-lib filters
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • nytamin
  • hummelstrand
  • PeterC89

Poem

🐇 I nibbled through types in moonlit code,
Added studios and rehearsals down the road,
Buttons to switch, editors to play,
Compilers check playlists before they say “go” — hooray! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main feature addition: RundownPlaylist filters for Action Triggers, matching the comprehensive changes across the codebase.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation (no UI available), the solution (UI added), and the new capability (rehearsal state filter).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jstarpl jstarpl added the Contribution from NRK Contributions sponsored by NRK (nrk.no) label Feb 13, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 13, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...eteor/server/api/deviceTriggers/triggersContext.ts 20.00% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts`:
- Around line 535-536: The code assigns new RegExp(link.value) directly to
selector.name which creates a ReDoS risk for untrusted input; fix by treating
link.value as a literal string unless regex is explicitly required—escape the
value (e.g., use an escapeRegExp utility) before constructing the RegExp or use
a safe substring match alternative, and if you must accept regexes validate the
pattern and wrap the RegExp construction in a try-catch to handle invalid or
malicious patterns; update the logic around selector.name and link.value
accordingly and add tests for both escaped-literal and intentional-regex cases.
- Around line 622-652: The compiled filter mutates closure-scoped state
(adLibPieceTypeFilter.skip, adLibActionTypeFilter.skip, clearAdLibs.length,
stickyAdLibs.length) inside the runtime check, causing future invocations of
compileAdLibFilter to be corrupted; fix by not mutating those outer
variables—inside the inner function compute local values (e.g., localSkip or
localClear/localSticky arrays or filter copies) and either return early with an
empty result or use those local copies when matchAll fails so the closure state
(adLibPieceTypeFilter, adLibActionTypeFilter, clearAdLibs, stickyAdLibs) is
never modified across invocations.

In
`@packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/ActionEditor.tsx`:
- Around line 78-85: The onChangeType handler in ActionEditor.tsx currently
mutates only action.filterChain[filterIndex].object, leaving stale field/value
and producing invalid filter links; change onChangeType to replace the entire
filter link at filterIndex with a fresh default link that matches the selected
FilterType (e.g., switch on newType and return a valid
IRundownPlaylistFilterLink, IAdLibFilterLink, IViewFilterLink, etc.), then call
overrideHelper().replaceItem(actionId, action).commit() with the updated action;
update references to action.filterChain and overrideHelper in the onChangeType
function so the old mixed objects are never kept.

In
`@packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/RundownPlaylistFilter.tsx`:
- Around line 41-62: fieldToOptions currently returns only {[t('Active')]: true}
for the activationId case, so users cannot choose the false ("Not Active")
value; update the activationId branch inside function fieldToOptions (which
handles IRundownPlaylistFilterLink['field']) to include both t('Active') -> true
and t('Not Active') -> false, or if excluding the false option was intentional,
add a clarifying comment in the activationId case explaining why filtering for
non-active playlists is unsupported.
🧹 Nitpick comments (2)
packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts (1)

571-573: Remove the unnecessary @ts-ignore.

rundownPlaylistFilter is used at Line 623 (rundownPlaylistFilter.matchAll) and Line 627+ (rundownPlaylistFilter.selector.*), so the "unused" suppression is incorrect.

♻️ Proposed fix
-	// eslint-disable-next-line `@typescript-eslint/ban-ts-comment`
-	// `@ts-ignore` ignore unused
 	const rundownPlaylistFilter = compileRundownPlaylistFilter(onlyRundownPlaylistLinks)
packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/RundownPlaylistFilter.tsx (1)

64-82: studioId value label shows raw ID instead of studio name.

fieldValueToValueLabel for 'studioId' (Line 73-74) returns String(link.value), which is the studio's MongoDB _id. For better UX, consider looking up the studio name similarly to how fieldToOptions already queries Studios:

Proposed improvement
 	case 'name':
-	case 'studioId':
 		return String(link.value)
+	case 'studioId': {
+		const studio = Studios.findOne(link.value)
+		return studio ? `${studio.name} (${link.value})` : String(link.value)
+	}

Note: Since fieldValueToValueLabel is called outside useTracker, this won't be reactive. If reactivity is needed, this lookup would need to move into the component body. However, since the value typically doesn't change while the editor is open, a non-reactive lookup may be acceptable here.

Comment on lines +535 to +536
case 'name':
selector.name = new RegExp(link.value)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

ReDoS risk: link.value is used directly in new RegExp() without sanitization.

User-provided filter values flow into this regex constructor. Malicious or complex patterns could cause catastrophic backtracking. This was also flagged by the static analysis tool.

Consider escaping the input or using a safe regex library if the intent is literal substring matching. If regex is intentional, consider wrapping in a try-catch and/or validating the pattern.

🧰 Tools
🪛 ast-grep (0.40.5)

[warning] 535-535: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(link.value)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

🤖 Prompt for AI Agents
In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts` around lines
535 - 536, The code assigns new RegExp(link.value) directly to selector.name
which creates a ReDoS risk for untrusted input; fix by treating link.value as a
literal string unless regex is explicitly required—escape the value (e.g., use
an escapeRegExp utility) before constructing the RegExp or use a safe substring
match alternative, and if you must accept regexes validate the pattern and wrap
the RegExp construction in a try-catch to handle invalid or malicious patterns;
update the logic around selector.name and link.value accordingly and add tests
for both escaped-literal and intentional-regex cases.

Comment thread packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts
Comment on lines +78 to +85
const onChangeType = useCallback(
(filterIndex: number, newType: FilterType) => {
action.filterChain[filterIndex].object = newType

overrideHelper().replaceItem(actionId, action).commit()
},
[action, overrideHelper]
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Changing object without resetting field/value produces invalid filter links.

When switching types (e.g., from 'adLib' to 'rundownPlaylist'), only the object property is updated. The stale field and value from the previous type remain, producing an invalid hybrid object like { object: 'rundownPlaylist', field: 'label', value: ['test'] } — which doesn't conform to IRundownPlaylistFilterLink.

Similarly, switching from 'view' (which has no field/value) to 'adLib' or 'rundownPlaylist' would leave field and value as undefined.

You should replace the entire filter link with a valid default for the target type.

🐛 Proposed fix
 	const onChangeType = useCallback(
 		(filterIndex: number, newType: FilterType) => {
-			action.filterChain[filterIndex].object = newType
+			let newLink: ChainLink
+			switch (newType) {
+				case 'view':
+					newLink = { object: 'view' }
+					break
+				case 'rundownPlaylist':
+					newLink = { object: 'rundownPlaylist', field: 'activationId', value: true }
+					break
+				case 'adLib':
+					newLink = { object: 'adLib', field: 'label', value: [] }
+					break
+			}
+			action.filterChain.splice(filterIndex, 1, newLink)
 
 			overrideHelper().replaceItem(actionId, action).commit()
 		},
 		[action, overrideHelper]
 	)
🤖 Prompt for AI Agents
In
`@packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/ActionEditor.tsx`
around lines 78 - 85, The onChangeType handler in ActionEditor.tsx currently
mutates only action.filterChain[filterIndex].object, leaving stale field/value
and producing invalid filter links; change onChangeType to replace the entire
filter link at filterIndex with a fresh default link that matches the selected
FilterType (e.g., switch on newType and return a valid
IRundownPlaylistFilterLink, IAdLibFilterLink, IViewFilterLink, etc.), then call
overrideHelper().replaceItem(actionId, action).commit() with the updated action;
update references to action.filterChain and overrideHelper in the onChangeType
function so the old mixed objects are never kept.

Comment on lines +41 to +62
function fieldToOptions(t: TFunction, field: IRundownPlaylistFilterLink['field']): Record<string, any> {
switch (field) {
case 'activationId':
return {
[t('Active')]: true,
}
case 'name':
return {}
case 'rehearsal':
return {
[t('In rehearsal')]: true,
[t('Not in rehearsal')]: false,
}
case 'studioId':
return Studios.find()
.fetch()
.map((studio) => ({ name: `${studio.name} (${studio._id})`, value: studio._id }))
default:
assertNever(field)
return field
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

activationId dropdown only offers "Active" — no way to deselect.

fieldToOptions for 'activationId' only provides { [t('Active')]: true }. Since IRundownPlaylistFilterLink allows value: boolean for this field, and fieldValueToValueLabel handles both true ("Active") and false ("Not Active"), the dropdown has no way to select the false option. If this is intentional (filtering for non-active playlists is not a useful scenario), consider adding a brief comment to clarify. Otherwise, add the missing option:

Proposed fix if both options should be available
 		case 'activationId':
 			return {
 				[t('Active')]: true,
+				[t('Not Active')]: false,
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function fieldToOptions(t: TFunction, field: IRundownPlaylistFilterLink['field']): Record<string, any> {
switch (field) {
case 'activationId':
return {
[t('Active')]: true,
}
case 'name':
return {}
case 'rehearsal':
return {
[t('In rehearsal')]: true,
[t('Not in rehearsal')]: false,
}
case 'studioId':
return Studios.find()
.fetch()
.map((studio) => ({ name: `${studio.name} (${studio._id})`, value: studio._id }))
default:
assertNever(field)
return field
}
}
function fieldToOptions(t: TFunction, field: IRundownPlaylistFilterLink['field']): Record<string, any> {
switch (field) {
case 'activationId':
return {
[t('Active')]: true,
[t('Not Active')]: false,
}
case 'name':
return {}
case 'rehearsal':
return {
[t('In rehearsal')]: true,
[t('Not in rehearsal')]: false,
}
case 'studioId':
return Studios.find()
.fetch()
.map((studio) => ({ name: `${studio.name} (${studio._id})`, value: studio._id }))
default:
assertNever(field)
return field
}
}
🤖 Prompt for AI Agents
In
`@packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/RundownPlaylistFilter.tsx`
around lines 41 - 62, fieldToOptions currently returns only {[t('Active')]:
true} for the activationId case, so users cannot choose the false ("Not Active")
value; update the activationId branch inside function fieldToOptions (which
handles IRundownPlaylistFilterLink['field']) to include both t('Active') -> true
and t('Not Active') -> false, or if excluding the false option was intentional,
add a clarifying comment in the activationId case explaining why filtering for
non-active playlists is unsupported.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts (1)

535-537: ⚠️ Potential issue | 🟠 Major

Unsafe dynamic regex construction remains in rundown name filter.

Line 536 uses new RegExp(link.value) directly. This allows expensive patterns (ReDoS) and can also throw on invalid input.

🔧 Safer literal-match approach
+const escapeRegExp = (value: string): string => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+
 function compileRundownPlaylistFilter(filterChain: IRundownPlaylistFilterLink[]): {
@@
 			case 'name':
-				selector.name = new RegExp(link.value)
+				selector.name = new RegExp(escapeRegExp(link.value), 'i')
 				return
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts` around lines
535 - 537, The code in the 'name' switch branch assigns selector.name = new
RegExp(link.value) using untrusted input (link.value) which can throw or enable
ReDoS; instead, escape regex metacharacters and build a safe anchored regex or
use a direct string equality depending on expected semantics: implement/consume
an escape helper (e.g. escapeRegex(value)) and replace the assignment with
selector.name = new RegExp('^' + escapeRegex(link.value) + '$') (and optionally
enforce a max input length and wrap RegExp creation in try/catch), referencing
the 'name' case and the selector.name = ... assignment in
actionFilterChainCompilers.ts.
🧹 Nitpick comments (1)
packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts (1)

571-573: Remove stale @ts-ignore suppression.

Line 573 assigns rundownPlaylistFilter, and that symbol is used later (Line 623+). Keeping this suppression can mask real type issues.

🧹 Cleanup
-	// eslint-disable-next-line `@typescript-eslint/ban-ts-comment`
-	// `@ts-ignore` ignore unused
 	const rundownPlaylistFilter = compileRundownPlaylistFilter(onlyRundownPlaylistLinks)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts` around lines
571 - 573, Remove the stale eslint/ts-ignore suppression above the declaration
of rundownPlaylistFilter: delete the two comment lines and ensure the symbol is
properly typed instead of suppressed (e.g., give rundownPlaylistFilter an
explicit type such as ReturnType<typeof compileRundownPlaylistFilter> or the
concrete filter interface expected where it’s later used). Verify
compileRundownPlaylistFilter’s signature matches that type and adjust its return
type if needed so the code compiles without any `@ts-ignore`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts`:
- Around line 631-634: The nameMatches expression incorrectly evaluates to true
when currentRundownPlaylist is undefined because optional chaining yields
undefined which !== null is true; update the check in actionFilterChainCompilers
to explicitly ensure a playlist and name exist before testing the regex
(reference rundownPlaylistFilter.selector.name, currentRundownPlaylist, and
nameMatches). Replace the existing optional-chaining-only check with a boolean
guard that confirms currentRundownPlaylist (and its name) is present and that
name.match(...) returns a non-null value (e.g., check currentRundownPlaylist &&
currentRundownPlaylist.name && currentRundownPlaylist.name.match(...) !== null
or use an explicit null-check/assertion to yield a true/false result).

---

Duplicate comments:
In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts`:
- Around line 535-537: The code in the 'name' switch branch assigns
selector.name = new RegExp(link.value) using untrusted input (link.value) which
can throw or enable ReDoS; instead, escape regex metacharacters and build a safe
anchored regex or use a direct string equality depending on expected semantics:
implement/consume an escape helper (e.g. escapeRegex(value)) and replace the
assignment with selector.name = new RegExp('^' + escapeRegex(link.value) + '$')
(and optionally enforce a max input length and wrap RegExp creation in
try/catch), referencing the 'name' case and the selector.name = ... assignment
in actionFilterChainCompilers.ts.

---

Nitpick comments:
In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts`:
- Around line 571-573: Remove the stale eslint/ts-ignore suppression above the
declaration of rundownPlaylistFilter: delete the two comment lines and ensure
the symbol is properly typed instead of suppressed (e.g., give
rundownPlaylistFilter an explicit type such as ReturnType<typeof
compileRundownPlaylistFilter> or the concrete filter interface expected where
it’s later used). Verify compileRundownPlaylistFilter’s signature matches that
type and adjust its return type if needed so the code compiles without any
`@ts-ignore`.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d117426 and c76273b.

📒 Files selected for processing (1)
  • packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts

Comment thread packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts
@jstarpl jstarpl force-pushed the contrib/sofie-4219/actions-adlib-rundown-state-filter branch from c76273b to fb99a01 Compare March 3, 2026 09:48
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts (2)

535-537: ⚠️ Potential issue | 🟠 Major

Harden rundown name regex compilation against malformed and hostile input.

At Line 536, link.value is passed directly into new RegExp(...). This can throw on invalid patterns and exposes regex-complexity risk if input is user-controlled.

Proposed safer literal-match fix
 			case 'name':
-				selector.name = new RegExp(link.value)
+				selector.name = new RegExp(link.value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'i')
 				return
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts` around lines
535 - 537, The code in actionFilterChainCompilers that sets selector.name = new
RegExp(link.value) is unsafe for user-controlled input; replace it by validating
and escaping link.value before constructing a regex (or use an exact-match
alternative). Implement a small helper (e.g., escapeRegExp) to escape regex
meta-characters, enforce a reasonable max length on link.value, then set
selector.name = new RegExp('^' + escapeRegExp(link.value) + '$') inside a
try/catch that falls back to a safe equality match (selector.name = link.value
or null) on error; reference selector.name, link.value, and new RegExp when
editing.

631-634: ⚠️ Potential issue | 🟠 Major

Fix false-positive nameMatches when no rundown playlist exists.

Line 633 currently evaluates undefined !== null as true, so the name filter can pass when currentRundownPlaylist is missing.

Boolean guard fix
 			const nameMatches =
 				rundownPlaylistFilter.selector.name !== undefined
-					? currentRundownPlaylist?.name.match(rundownPlaylistFilter.selector.name) !== null
+					? currentRundownPlaylist !== undefined &&
+						currentRundownPlaylist.name.match(rundownPlaylistFilter.selector.name) !== null
 					: true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts` around lines
631 - 634, The nameMatches ternary incorrectly treats undefined (no
currentRundownPlaylist) as a truthy match; change the conditional to require an
actual RegExp match by ensuring currentRundownPlaylist and its name exist before
checking match. For example, in the nameMatches assignment (using
rundownPlaylistFilter.selector.name and currentRundownPlaylist?.name.match),
replace the expression with a check that coerces the match result to boolean
(e.g.,
!!currentRundownPlaylist?.name?.match(rundownPlaylistFilter.selector.name)) or
explicitly compare against null/undefined so that when currentRundownPlaylist is
missing the name filter evaluates to false. Ensure the else (selector.name
undefined) still returns true.
🧹 Nitpick comments (1)
packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts (1)

571-573: Remove stale @ts-ignore around rundown playlist filter compilation.

Lines 571-573 suppress type checking on a normal typed call and can hide future regressions.

Cleanup diff
-	// eslint-disable-next-line `@typescript-eslint/ban-ts-comment`
-	// `@ts-ignore` ignore unused
 	const rundownPlaylistFilter = compileRundownPlaylistFilter(onlyRundownPlaylistLinks)
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts`:
- Around line 535-537: The code in actionFilterChainCompilers that sets
selector.name = new RegExp(link.value) is unsafe for user-controlled input;
replace it by validating and escaping link.value before constructing a regex (or
use an exact-match alternative). Implement a small helper (e.g., escapeRegExp)
to escape regex meta-characters, enforce a reasonable max length on link.value,
then set selector.name = new RegExp('^' + escapeRegExp(link.value) + '$') inside
a try/catch that falls back to a safe equality match (selector.name = link.value
or null) on error; reference selector.name, link.value, and new RegExp when
editing.
- Around line 631-634: The nameMatches ternary incorrectly treats undefined (no
currentRundownPlaylist) as a truthy match; change the conditional to require an
actual RegExp match by ensuring currentRundownPlaylist and its name exist before
checking match. For example, in the nameMatches assignment (using
rundownPlaylistFilter.selector.name and currentRundownPlaylist?.name.match),
replace the expression with a check that coerces the match result to boolean
(e.g.,
!!currentRundownPlaylist?.name?.match(rundownPlaylistFilter.selector.name)) or
explicitly compare against null/undefined so that when currentRundownPlaylist is
missing the name filter evaluates to false. Ensure the else (selector.name
undefined) still returns true.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c76273b and fb99a01.

📒 Files selected for processing (2)
  • packages/meteor-lib/src/triggers/actionFilterChainCompilers.ts
  • packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/SwitchFilterType.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/webui/src/client/ui/Settings/components/triggeredActions/actionEditors/filterPreviews/SwitchFilterType.tsx

@jstarpl
Copy link
Copy Markdown
Contributor Author

jstarpl commented Mar 3, 2026

Having not received any feedback on this for the past 3 weeks, I'm merging it in.

@jstarpl jstarpl merged commit c5dd995 into Sofie-Automation:main Mar 3, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Contribution from NRK Contributions sponsored by NRK (nrk.no)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant