🛡️ Sentinel: [security improvement] Add input validation to vuongtest and icci#9
🛡️ Sentinel: [security improvement] Add input validation to vuongtest and icci#9seonghobae wants to merge 1 commit into
Conversation
Added input validation to `R/icci.R` for `conf.level` and to `R/vuongtest.R` for `nested` and `adj` arguments to prevent unexpected behavior.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
Adds defensive argument validation to two public-facing R APIs (vuongtest() and icci()) to fail fast on invalid inputs and avoid confusing downstream errors, aligning with the “Sentinel” security-hardening goal for functions that may be exposed via Shiny/web contexts.
Changes:
- Add validation for
nestedand constrainadjviamatch.arg()invuongtest(). - Add validation for
conf.levelbounds inicci(). - Record the input-validation hardening in
.jules/sentinel.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| R/vuongtest.R | Adds validation for nested and constrains adj values. |
| R/icci.R | Adds bounds checking for conf.level. |
| .jules/sentinel.md | Documents the validation hardening as a Sentinel learning/prevention note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!is.logical(nested) || length(nested) != 1) { | ||
| stop("Argument 'nested' must be a single logical value.") | ||
| } |
| ## Input validation for security and robustness | ||
| if (!is.logical(nested) || length(nested) != 1) { | ||
| stop("Argument 'nested' must be a single logical value.") | ||
| } | ||
| adj <- match.arg(adj) |
| if (!is.numeric(conf.level) || length(conf.level) != 1 || conf.level <= 0 || conf.level >= 1) { | ||
| stop("Argument 'conf.level' must be a single numeric value strictly between 0 and 1.") | ||
| } |
| ## Input validation | ||
| if (!is.numeric(conf.level) || length(conf.level) != 1 || conf.level <= 0 || conf.level >= 1) { | ||
| stop("Argument 'conf.level' must be a single numeric value strictly between 0 and 1.") | ||
| } |
🚨 Severity: LOW/MEDIUM
💡 Vulnerability: Missing input validation in public-facing R functions (
vuongtestandicci). This could lead to unexpected behavior, obscure downstream errors, or potential manipulation if these functions are exposed via a web API (like Shiny).🎯 Impact: Invalid parameters such as non-logical
nested, out-of-boundsconf.level, or invalidadjvalues would propagate deeply into the function before failing or producing incorrect statistical results.🔧 Fix: Added defensive programming checks.
match.arg()is now used foradj.is.logical()and length checks are added fornested. Strict numeric bounds and length checks are added forconf.level.✅ Verification: Changes were reviewed to be correctly placed and idiomatic for R. Testing can be verified by running the test suite via CI.
PR created automatically by Jules for task 1463648741099773841 started by @seonghobae