@@ -104,6 +104,10 @@ recistClass <- R6::R6Class(
104104 },
105105
106106 # ---- Helper: Escape Variable Names ----
107+ # TODO (cleanup): .escapeVar is dead code — defined here but never called anywhere
108+ # in recist.b.R. Remove it. If a future formula ever needs safe variable names, use
109+ # jmvcore::composeTerm()/toB64() rather than make.names() mangling (which can collide
110+ # distinct non-syntactic column names onto the same name).
107111 .escapeVar = function (x ) {
108112 # Make syntactically valid R names for variables with spaces/special chars
109113 gsub(" [^A-Za-z0-9_]+" " _" x ))
@@ -119,6 +123,12 @@ recistClass <- R6::R6Class(
119123 return ()
120124 }
121125
126+ # TODO (data hygiene): private$.summaryStatus$warnings and $exclusions are R6
127+ # fields initialised once at class generation; jamovi reuses the instance across
128+ # .run() calls, and .addWarning()/.selectTargetLesions() only ever append (c(...)).
129+ # They are never reset, so warnings/exclusions accumulate and duplicate across
130+ # successive runs (the scalar counts are overwritten each run, so only these two
131+ # leak). Reset both to character() here at the top of .run() before any append.
122132 tryCatch({
123133 private $ .prepareData()
124134 private $ .selectTargetLesions()
@@ -167,7 +177,7 @@ recistClass <- R6::R6Class(
167177 self $ results $ instructions $ setContent(
168178 paste0(" <div class='error'>"
169179 " <h4>Error in Analysis</h4>"
170- " <p>" e $ message , " </p>"
180+ " <p>" htmltools :: htmlEscape( e $ message ) , " </p>"
171181 " <p><b>Common issues:</b></p>"
172182 " <ul>"
173183 " <li>Ensure data is in long format (one row per lesion per assessment)</li>"
@@ -176,7 +186,7 @@ recistClass <- R6::R6Class(
176186 " <li>Ensure each patient has baseline assessment (earliest time)</li>"
177187 " </ul></div>"
178188 )
179- stop( e )
189+ jmvcore :: reject( e $ message )
180190 })
181191 },
182192
@@ -185,14 +195,21 @@ recistClass <- R6::R6Class(
185195
186196 mydata <- self $ data
187197
198+ # TODO (correctness): composeTerm() backtick-quotes non-syntactic names
199+ # (e.g. "Patient ID" -> `Patient ID`), so mydata[[composeTerm(...)]] returns NULL
200+ # for any column whose name contains spaces/special chars. composeTerm is for
201+ # FORMULA terms, NOT data-frame indexing — index with the raw self$options$X.
202+ # Affects every composeTerm()+mydata[[...]] pair below in .prepareData()
203+ # (patientId, assessmentTime, lesionId, lesionType, lesionDiameter, organ,
204+ # nonTargetStatus) and the groupVar/patientId merge in .populateStratifiedTable().
188205 patientId <- jmvcore :: composeTerm(self $ options $ patientId )
189206 assessmentTime <- jmvcore :: composeTerm(self $ options $ assessmentTime )
190207 lesionId <- jmvcore :: composeTerm(self $ options $ lesionId )
191208 lesionType <- jmvcore :: composeTerm(self $ options $ lesionType )
192209
193210 data <- data.frame (
194211 patientId = mydata [[patientId ]],
195- assessmentTime = as.numeric (mydata [[assessmentTime ]]),
212+ assessmentTime = jmvcore :: toNumeric (mydata [[assessmentTime ]]),
196213 lesionId = as.character(mydata [[lesionId ]]),
197214 lesionType = tolower(as.character(mydata [[lesionType ]])),
198215 stringsAsFactors = FALSE
@@ -201,7 +218,7 @@ recistClass <- R6::R6Class(
201218 # Add diameter if provided
202219 if (! is.null(self $ options $ lesionDiameter )) {
203220 lesionDiameter <- jmvcore :: composeTerm(self $ options $ lesionDiameter )
204- data $ diameter <- as.numeric (mydata [[lesionDiameter ]])
221+ data $ diameter <- jmvcore :: toNumeric (mydata [[lesionDiameter ]])
205222 } else {
206223 data $ diameter <- NA
207224 }
@@ -628,6 +645,10 @@ recistClass <- R6::R6Class(
628645 }
629646 },
630647
648+ # TODO (correctness): for an all-NA patient (no measurable target sum),
649+ # min(..., na.rm=TRUE) emits a warning and returns Inf/-Inf, which then
650+ # renders as Inf in nadirSum / bestPercentChange. Guard with an all-NA
651+ # check (return NA_real_ when all values are NA).
631652 nadirSum = min(targetSum , na.rm = TRUE ),
632653 bestPercentChange = min(changeFromBaseline , na.rm = TRUE ),
633654
@@ -928,15 +949,15 @@ recistClass <- R6::R6Class(
928949 if (length(status $ exclusions ) > 0 ) {
929950 html <- paste0(html , " <h5 style='color: orange;'>Excluded Lesions (Rules)</h5><ul>"
930951 for (m in status $ exclusions ) {
931- html <- paste0(html , " <li>" m , " </li>"
952+ html <- paste0(html , " <li>" htmltools :: htmlEscape( m ) , " </li>"
932953 }
933954 html <- paste0(html , " </ul>"
934955 }
935956
936957 if (length(status $ warnings ) > 0 ) {
937958 html <- paste0(html , " <h5 style='color: red;'>Warnings</h5><ul>"
938959 for (w in status $ warnings ) {
939- html <- paste0(html , " <li>" w , " </li>"
960+ html <- paste0(html , " <li>" htmltools :: htmlEscape( w ) , " </li>"
940961 }
941962 html <- paste0(html , " </ul>"
942963 }
@@ -949,6 +970,11 @@ recistClass <- R6::R6Class(
949970 .exportData = function () {
950971 data <- private $ .lesionData
951972 if (! is.null(data ) && nrow(data ) > 0 ) {
973+ # TODO (security): writes recist_lesion_data.csv to getwd() as a .run() side
974+ # effect. On multi-tenant cloud jamovi the cwd is shared/process-level and the
975+ # user never chose it — a silent, unconfirmed write. Not RCE/exfiltration
976+ # (fixed filename, user's own data), but route exports through a jamovi
977+ # Output/path option (or tempdir()) instead of getwd().
952978 filepath <- file.path(getwd(), " recist_lesion_data.csv"
953979 tryCatch({
954980 write.csv(data , filepath , row.names = FALSE )
@@ -957,7 +983,7 @@ recistClass <- R6::R6Class(
957983 if (is.null(currentHtml )) currentHtml <- " "
958984 newHtml <- paste0(currentHtml ,
959985 " <p style='color: green;'><b> Data Exported:</b> "
960- filepath , " </p>"
986+ htmltools :: htmlEscape( filepath ) , " </p>"
961987 self $ results $ runSummary $ setContent(newHtml )
962988 }, error = function (e ) {
963989 private $ .addWarning(paste0(" Failed to export data: " e $ message ))
0 commit comments