Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@
**Vulnerability:** Unvalidated inputs passed to `if()` statements can cause process crashes (`condition has length > 1`) or unexpected coercion vulnerabilities.
**Learning:** In R, optional boolean parameters that default to `NULL` should be validated using explicit runtime type validation (e.g., `if (!is.null(flag) && (!is.logical(flag) || length(flag) != 1 || is.na(flag)))`).
**Prevention:** Always implement explicit runtime type validation for optional boolean parameters.
## 2024-09-27 - Fix unconstrained integer validation
**Vulnerability:** Unconstrained regex matching `grepl("^[0-9]+$", n)` allows excessively large numbers to be converted by `as.integer(n)`, resulting in integer overflow (`NA` warning) which crashes downstream conditional checks expecting a boolean or integer outcome.
**Learning:** In R, validating input bounds is important especially for menu selections. Overly broad regexes leave the type coercion vulnerable to overflow logic errors.
**Prevention:** Rather than regex checks and coercion, validate the raw string explicitly against expected enum values (e.g., `n == "1" || n == "2"`) to safely guard internal logic flow.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Description: Automates fixed item parameter linking for test linking under
the item response theory paradigm using mirt package estimates.
License: GPL-3 | file LICENSE
Imports: mirt, methods
Suggests: testthat (>= 3.0.0)
Suggests: testthat (>= 3.0.0), mockery
Encoding: UTF-8
Config/testthat/edition: 3
Config/roxygen2/version: 8.0.0
6 changes: 3 additions & 3 deletions R/aFIPC.R
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ autoFIPC <-
}
for (attempt in seq_len(3)) {
n <- readline(prompt = "Is it correct? (1: Yes 2: No) : ")
if (grepl("^[0-9]+$", n)) {
if (n == "1" || n == "2") {
return(as.integer(n))
}
}
Expand Down Expand Up @@ -171,7 +171,7 @@ autoFIPC <-
readline(
prompt = "Do you want to use default BILOG-MG priors for oldform Data? (1: Yes 2: No) : "
)
if (grepl("^[0-9]+$", n)) {
if (n == "1" || n == "2") {
return(as.integer(n))
}
}
Expand Down Expand Up @@ -390,7 +390,7 @@ autoFIPC <-
readline(
prompt = "Do you want to use default BILOG-MG priors for newform Data? (1: Yes 2: No) : "
)
if (grepl("^[0-9]+$", n)) {
if (n == "1" || n == "2") {
return(as.integer(n))
}
}
Expand Down
27 changes: 27 additions & 0 deletions tests/testthat/test-sentinel-integer-validation.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
test_that("autoFIPC integer overflow in readline input parsing is prevented", {
# Provide small dataset
dummy_data <- data.frame(
Item1 = c(1, 0, 1, 0, 1, 1),
Item2 = c(0, 1, 0, 1, 0, 0),
Item3 = c(0, 0, 1, 1, 0, 0),
Item4 = c(1, 1, 0, 0, 1, 1)
)

# Mock interactive to return TRUE
mockery::stub(aFIPC::autoFIPC, "interactive", function() TRUE)

# Mock readline to return a huge number first, then a valid "1"
mockery::stub(aFIPC::autoFIPC, "readline", mockery::mock("100000000000000", "1", cycle = TRUE))

# As we fixed the bug, it won't crash with "missing value where TRUE/FALSE needed" NA coercion error.
# We use try() to catch whatever it does down the line (which is estimation failing).
expect_error(
aFIPC::autoFIPC(
newformXData = dummy_data,
oldformYData = dummy_data,
newformCommonItemNames = "Item1",
oldformCommonItemNames = "Item1",
confirmCommonItems = NULL
), regexp = "completely failed|could not estimate"
)
})
Loading