Skip to content
Closed
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 @@ -12,3 +12,7 @@
**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-05-14 - [Input Validation for Type Safety and Security]
**Vulnerability:** Unvalidated inputs in exported functions (`vuongtest`'s `nested`, `adj` and `icci`'s `conf.level`) allow execution to reach deeper internal code branches with invalid types (e.g., NA for `nested` causing "missing value where TRUE/FALSE needed" inside `if(nested)`).
**Learning:** In R, missing early type and bounds checks can leak internal execution stack paths via uncaught exceptions from standard internal operations. Short-circuit operators (`||`) must be used carefully with type and length checks before value comparisons.
**Prevention:** Implement strict input validation (`is.logical`, `is.numeric`, bounds checking) at the absolute beginning of all exported interface functions, paired with `stop(..., call. = FALSE)` to fail securely before processing begins.
4 changes: 4 additions & 0 deletions R/icci.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@
#' @export
icci <- function(object1, object2, conf.level=.95, ll1=llcont, ll2=llcont) {

if (!is.numeric(conf.level) || length(conf.level) != 1 || is.na(conf.level) || conf.level <= 0 || conf.level >= 1) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟨 Complex input bypasses sanitized validation

A complex conf.level passes is.numeric, then range checking emits a raw base error. Malformed input can still expose internal execution details.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

stop("conf.level must be a single numeric value between 0 and 1", call. = FALSE)
}
Comment on lines +68 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Public validation lacks regression coverage

Three public argument contracts changed without tests for invalid values, clean errors, or boundaries. Add coverage before merging to prevent validation regressions.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +68 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Generated signatures remain current

The function signatures and parameter text did not change. Existing generated usage documentation therefore remains current without regeneration.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


## check objects, issue warnings/errors, get classes/calls
obinfo <- check.obj(object1, object2)
callA <- obinfo$callA; classA <- obinfo$classA
Expand Down
7 changes: 7 additions & 0 deletions R/vuongtest.R
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,13 @@
#' @export
vuongtest <- function(object1, object2, nested=FALSE, adj="none", ll1=llcont, ll2=llcont, score1=NULL, score2=NULL, vc1=vcov, vc2=vcov) {

if (!is.logical(nested) || length(nested) != 1 || is.na(nested)) {
stop("nested must be a single logical value", call. = FALSE)
}
if (!is.character(adj) || length(adj) != 1 || is.na(adj) || !(adj %in% c("none", "aic", "bic"))) {
stop("adj must be 'none', 'aic', or 'bic'", call. = FALSE)
}
Comment on lines +104 to +106

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Ignored nested adjustment now fails

Nested comparisons reject unknown adj values even though the public contract says this argument is ignored. Existing callers can now fail before evaluation.

Prompt for agents
Restore the documented nested-comparison contract in R/vuongtest.R. When nested is TRUE, adj must have no effect and must not be rejected. Gate adj validation to non-nested calls, and ensure the later AIC/BIC parameter-count and likelihood-ratio adjustment block is also skipped for nested calls. Add regression coverage comparing nested results for default, supported, and otherwise invalid adj values.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +101 to +106

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Validation change missing from NEWS

The repository requires user-visible changes in NEWS. These exported functions now reject inputs differently, but the release notes remain unchanged.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


## check objects, issue warnings/errors, get classes/calls
obinfo <- check.obj(object1, object2)
callA <- obinfo$callA; classA <- obinfo$classA
Expand Down
Loading