From f76965a45c0bb1bb9d0ed2f1c33be86cf6f2d942 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:08:01 +0000 Subject: [PATCH 1/7] Fix error handling leaking call stacks via unvalidated inputs Adds input validation using `stop(..., call. = FALSE)` and `match.arg()` at the top of `vuongtest()` and `icci()` to prevent users from passing incorrect types (like arrays where scalars are expected) which would bypass the module's secure error boundaries and leak internal call stacks and R execution logic. --- .jules/sentinel.md | 16 ++++++++++++++++ R/icci.R | 4 ++++ R/vuongtest.R | 5 +++++ 3 files changed, 25 insertions(+) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index bc5c6e1..0641d9c 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -12,3 +12,19 @@ **Vulnerability:** Raw `stop()` and `warning()` calls without `call. = FALSE` in `llcont.R` and `vuongtest.R` exposed execution stack/call details when raised. **Learning:** While some instances of `stop()` inside `tryCatch()` were previously fixed to hide the call stack, other standalone exceptions and warnings still leaked call context. Security must be consistently applied across the entire codebase. **Prevention:** Always set `call. = FALSE` when using `stop()` or `warning()` to enforce a secure-by-default boundary and prevent internal execution paths from being disclosed to the end user. +## 2024-08-11 - Enforce secure error boundaries by validating input types early + +**Vulnerability:** +Exported functions like `vuongtest` and `icci` expect certain arguments like `nested` to be logical scalars, and `adj` to be string scalars from a set of choices. When arrays or other types are passed (e.g. `nested=c(TRUE, FALSE)` or `adj=c("aic", "bic")`), the condition expressions (e.g., `if (nested)` or `if (adj == "aic")`) inside internal logic result in `length > 1` warnings/errors, or other raw R execution stack faults. These expose the internal execution flow to the user and violate the "fail securely" principle. They bypass the top-level `stop(..., call. = FALSE)` safeguards. + +**Learning:** +In R, unvalidated arguments can bypass top-level input safeguards and trigger raw R errors deep inside internal logic, leaking internal execution contexts via error stack traces (e.g., ``). Relying on internal control flow `if` statements to implicitly validate inputs is insecure. + +**Prevention:** +Always explicitly validate the type, length, and bounds of user inputs at the very beginning of exported functions (using assertions or functions like `match.arg()`) and use `stop(..., call. = FALSE)` to fail securely. For example: +```R +if (length(nested) > 1 || !is.logical(nested)) { + stop("Argument 'nested' must be a single logical value.", call. = FALSE) +} +adj <- match.arg(adj, c("none", "aic", "bic")) +``` diff --git a/R/icci.R b/R/icci.R index f22278a..300252e 100644 --- a/R/icci.R +++ b/R/icci.R @@ -65,6 +65,10 @@ #' @export icci <- function(object1, object2, conf.level=.95, ll1=llcont, ll2=llcont) { + if (length(conf.level) > 1 || !is.numeric(conf.level) || conf.level <= 0 || conf.level >= 1) { + stop("Argument 'conf.level' must be a single numeric value between 0 and 1.", call. = FALSE) + } + ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) callA <- obinfo$callA; classA <- obinfo$classA diff --git a/R/vuongtest.R b/R/vuongtest.R index 2bdfbf6..5797213 100644 --- a/R/vuongtest.R +++ b/R/vuongtest.R @@ -98,6 +98,11 @@ #' @export vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll2=llcont, score1=NULL, score2=NULL, vc1=vcov, vc2=vcov) { + if (length(nested) > 1 || !is.logical(nested)) { + stop("Argument 'nested' must be a single logical value.", call. = FALSE) + } + adj <- match.arg(adj, c("none", "aic", "bic")) + ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) callA <- obinfo$callA; classA <- obinfo$classA From 727f62b7497b8b14f62a8e9049ca3a9baa4d4028 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 13:16:51 +0900 Subject: [PATCH 2/7] test(security): cover fail-closed scalar validation --- tests/testthat/test-input-validation.R | 58 ++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/testthat/test-input-validation.R diff --git a/tests/testthat/test-input-validation.R b/tests/testthat/test-input-validation.R new file mode 100644 index 0000000..d1024cb --- /dev/null +++ b/tests/testthat/test-input-validation.R @@ -0,0 +1,58 @@ +capture_input_error <- function(expression) { + tryCatch(force(expression), error = identity) +} + +expect_fail_closed_input <- function(error, expected_message) { + expect_s3_class(error, "error") + expect_identical(conditionMessage(error), expected_message) + expect_null(conditionCall(error)) +} + +test_that("vuongtest rejects invalid nested values without exposing calls", { + invalid_values <- list(logical(), c(TRUE, FALSE), NA, "yes") + + for (value in invalid_values) { + error <- capture_input_error(vuongtest(NULL, NULL, nested = value)) + expect_fail_closed_input( + error, + "Argument 'nested' must be a single non-missing logical value." + ) + } +}) + +test_that("vuongtest rejects invalid adjustment values without exposing calls", { + invalid_values <- list(character(), c("aic", "bic"), NA_character_, "other", 1) + + for (value in invalid_values) { + error <- capture_input_error(vuongtest(NULL, NULL, adj = value)) + expect_fail_closed_input( + error, + "Argument 'adj' must be one of \"none\", \"aic\", or \"bic\"." + ) + } +}) + +test_that("icci rejects invalid confidence levels without exposing calls", { + invalid_values <- list( + numeric(), + c(0.90, 0.95), + NA_real_, + NaN, + Inf, + -Inf, + 0, + 1, + "0.95" + ) + + for (value in invalid_values) { + error <- capture_input_error(icci(NULL, NULL, conf.level = value)) + expect_fail_closed_input( + error, + paste( + "Argument 'conf.level' must be a single finite numeric value", + "strictly between 0 and 1." + ) + ) + } +}) From 5e0ef057754115e164b537bb2c8543fceda55ef6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 13:21:28 +0900 Subject: [PATCH 3/7] fix(security): reject non-finite confidence levels --- R/icci.R | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/R/icci.R b/R/icci.R index 300252e..f4db32b 100644 --- a/R/icci.R +++ b/R/icci.R @@ -65,8 +65,15 @@ #' @export icci <- function(object1, object2, conf.level=.95, ll1=llcont, ll2=llcont) { - if (length(conf.level) > 1 || !is.numeric(conf.level) || conf.level <= 0 || conf.level >= 1) { - stop("Argument 'conf.level' must be a single numeric value between 0 and 1.", call. = FALSE) + if (length(conf.level) != 1L || !is.numeric(conf.level) || + !is.finite(conf.level) || conf.level <= 0 || conf.level >= 1) { + stop( + paste( + "Argument 'conf.level' must be a single finite numeric value", + "strictly between 0 and 1." + ), + call. = FALSE + ) } ## check objects, issue warnings/errors, get classes/calls From c71ef7120328788e93cfbfe2edb4fe27767c7e10 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 13:22:03 +0900 Subject: [PATCH 4/7] fix(security): fail closed on invalid test options --- R/vuongtest.R | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/R/vuongtest.R b/R/vuongtest.R index 5797213..123b5f7 100644 --- a/R/vuongtest.R +++ b/R/vuongtest.R @@ -98,10 +98,19 @@ #' @export vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll2=llcont, score1=NULL, score2=NULL, vc1=vcov, vc2=vcov) { - if (length(nested) > 1 || !is.logical(nested)) { - stop("Argument 'nested' must be a single logical value.", call. = FALSE) + if (length(nested) != 1L || !is.logical(nested) || is.na(nested)) { + stop( + "Argument 'nested' must be a single non-missing logical value.", + call. = FALSE + ) + } + if (length(adj) != 1L || !is.character(adj) || is.na(adj) || + !(adj %in% c("none", "aic", "bic"))) { + stop( + 'Argument \'adj\' must be one of "none", "aic", or "bic".', + call. = FALSE + ) } - adj <- match.arg(adj, c("none", "aic", "bic")) ## check objects, issue warnings/errors, get classes/calls obinfo <- check.obj(object1, object2) From cfca064a357c52d416b9f64a11fcef0d20e2221b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 13:22:48 +0900 Subject: [PATCH 5/7] docs(security): record complete scalar validation boundary --- .jules/sentinel.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 0641d9c..5d6cba3 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -12,19 +12,21 @@ **Vulnerability:** Raw `stop()` and `warning()` calls without `call. = FALSE` in `llcont.R` and `vuongtest.R` exposed execution stack/call details when raised. **Learning:** While some instances of `stop()` inside `tryCatch()` were previously fixed to hide the call stack, other standalone exceptions and warnings still leaked call context. Security must be consistently applied across the entire codebase. **Prevention:** Always set `call. = FALSE` when using `stop()` or `warning()` to enforce a secure-by-default boundary and prevent internal execution paths from being disclosed to the end user. -## 2024-08-11 - Enforce secure error boundaries by validating input types early +## 2026-08-11 - Enforce secure error boundaries by validating input types early **Vulnerability:** -Exported functions like `vuongtest` and `icci` expect certain arguments like `nested` to be logical scalars, and `adj` to be string scalars from a set of choices. When arrays or other types are passed (e.g. `nested=c(TRUE, FALSE)` or `adj=c("aic", "bic")`), the condition expressions (e.g., `if (nested)` or `if (adj == "aic")`) inside internal logic result in `length > 1` warnings/errors, or other raw R execution stack faults. These expose the internal execution flow to the user and violate the "fail securely" principle. They bypass the top-level `stop(..., call. = FALSE)` safeguards. +Exported functions like `vuongtest` and `icci` expect scalar, non-missing control values. Zero-length, non-scalar, missing, non-finite, or out-of-domain values could bypass incomplete guards and fail later inside model dispatch or control flow. Those raw failures exposed internal calls instead of the public validation contract. **Learning:** -In R, unvalidated arguments can bypass top-level input safeguards and trigger raw R errors deep inside internal logic, leaking internal execution contexts via error stack traces (e.g., ``). Relying on internal control flow `if` statements to implicitly validate inputs is insecure. +In R, checking only `length(x) > 1` does not reject zero-length values, and numeric type checks do not reject `NA`, `NaN`, or infinities. `match.arg()` also owns its error condition, so it cannot guarantee this package's call-free public error contract. **Prevention:** -Always explicitly validate the type, length, and bounds of user inputs at the very beginning of exported functions (using assertions or functions like `match.arg()`) and use `stop(..., call. = FALSE)` to fail securely. For example: +Validate exact scalar length, type, missingness, finiteness, bounds, and enum membership before model dispatch. Raise package-owned messages with `stop(..., call. = FALSE)`, and cover zero-length, non-scalar, missing, non-finite, wrong-type, and out-of-domain inputs in `testthat`. ```R -if (length(nested) > 1 || !is.logical(nested)) { - stop("Argument 'nested' must be a single logical value.", call. = FALSE) +if (length(nested) != 1L || !is.logical(nested) || is.na(nested)) { + stop( + "Argument 'nested' must be a single non-missing logical value.", + call. = FALSE + ) } -adj <- match.arg(adj, c("none", "aic", "bic")) ``` From 19607c6087284ec787af47490650a4140650215a Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:33:47 +0000 Subject: [PATCH 6/7] Fix error handling leaking call stacks via unvalidated inputs Adds input validation using stop(..., call. = FALSE) at the top of vuongtest() and icci() to prevent users from passing incorrect types (like arrays where scalars are expected, or NA where finite bounds are expected) which would bypass the module's secure error boundaries and leak internal call stacks and R execution logic. From 3f6811a2dc996b430324d85b15c98ad9d1f091be Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:52:19 +0000 Subject: [PATCH 7/7] Fix error handling leaking call stacks via unvalidated inputs Adds input validation using stop(..., call. = FALSE) at the top of vuongtest() and icci() to prevent users from passing incorrect types (like arrays where scalars are expected, or NA where finite bounds are expected) which would bypass the module's secure error boundaries and leak internal call stacks and R execution logic.