Skip to content

Remove rlang::expr, add custom trigger arithmetic#91

Merged
saumil-sh merged 4 commits into
mainfrom
fix/expr
May 13, 2026
Merged

Remove rlang::expr, add custom trigger arithmetic#91
saumil-sh merged 4 commits into
mainfrom
fix/expr

Conversation

@saumil-sh
Copy link
Copy Markdown
Collaborator

@saumil-sh saumil-sh commented May 7, 2026

Resolves #66 #65 #43

Key changes:

  1. Eliminated RCE vulnerability - no more rlang::expr in analysis_generators
  2. Safe trigger DSL (rxsim_trigger S3 objects)
  3. analysis_args for arbitrary analysis function signatures
  4. Moved trigger logic to R/triggers.R
  5. Simplified: build_trigger removed, Condition$new(where = trigger) is the API
  6. .trigger_to_quosures moved inside Condition.R
  7. Vignettes updated

Security fix. analysis_generators$trigger previously accepted arbitrary rlang::expr / rlang::quos() objects — remote code execution waiting to happen. Triggers are now validated at construction time as inert
rxsim_trigger S3 objects. Anything else is rejected before the trial runs.

New trigger DSL. Four primitives compose safely with & and |:

 # single condition
 enroll_trigger(0.5, 200)
 calendar_trigger(52)
 value_trigger("time", ">=", 52)
 count_trigger("enroll_time", ">=", 100)
 
 # composed
 enroll_trigger(0.5, 200) & calendar_trigger(52)   # both must hold
 enroll_trigger(1.0, 200) | calendar_trigger(100)  # either fires

Pass a trigger directly to Condition$new or use the convenience wrappers:

 # convenience wrappers
 cond <- trigger_by_fraction(0.5, 200, analysis = my_fn, name = "interim")
 cond <- trigger_by_calendar(52,       analysis = my_fn, name = "final")
 
 # or compose and pass directly
 cond <- Condition$new(
   where    = enroll_trigger(0.5, 200) & calendar_trigger(52),
   analysis = my_fn,
   name     = "interim"
 )

For replicate_trial, the trigger field in analysis_generators is the only change:

 analysis_generators <- list(
   interim = list(
     trigger  = enroll_trigger(0.5, 200) & calendar_trigger(52),
     analysis = function(df, current_time) nrow(df)
   )
 )

Arbitrary analysis signatures via analysis_args. The analysis function always receives df and current_time as its first two positional arguments. Scenario-specific values (thresholds, matrices, etc.) can now
be declared as named parameters and injected at call time:

 my_analysis <- function(df, current_time, alpha) { ... }
 
 analysis_generators <- list(
   final = list(
     trigger       = enroll_trigger(1.0, 200),
     analysis      = my_analysis,
     analysis_args = list(alpha = 0.05)
   )
 )

Copy link
Copy Markdown
Contributor

@mvalkobi mvalkobi left a comment

Choose a reason for hiding this comment

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

Looks good to me

@saumil-sh saumil-sh merged commit 67222cb into main May 13, 2026
2 checks passed
@saumil-sh saumil-sh deleted the fix/expr branch May 13, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants