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
3 changes: 3 additions & 0 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@
^\.jules(/.*)?$
^\.trivyignore\.yaml$
^trivy\.yaml$
^.semgrepignore$
^test_dummy\.R$
^test_validation\.R$
6 changes: 4 additions & 2 deletions .github/workflows/code-quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ jobs:
ACTIONLINT_VERSION="1.7.10"
ACTIONLINT_FILE="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz"
ACTIONLINT_BASE_URL="https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}"
curl -sSLo "$ACTIONLINT_FILE" "${ACTIONLINT_BASE_URL}/${ACTIONLINT_FILE}"
curl -sSLo actionlint_checksums.txt "${ACTIONLINT_BASE_URL}/actionlint_${ACTIONLINT_VERSION}_checksums.txt"
curl --retry 5 --retry-delay 5 --retry-connrefused -sSLo "$ACTIONLINT_FILE" \
"${ACTIONLINT_BASE_URL}/${ACTIONLINT_FILE}"
curl --retry 5 --retry-delay 5 --retry-connrefused -sSLo actionlint_checksums.txt \
"${ACTIONLINT_BASE_URL}/actionlint_${ACTIONLINT_VERSION}_checksums.txt"
grep "$ACTIONLINT_FILE" actionlint_checksums.txt | sha256sum -c -
tar -xzf "$ACTIONLINT_FILE" actionlint
chmod +x actionlint
Expand Down
12 changes: 8 additions & 4 deletions .github/workflows/security-audit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ jobs:
- name: Install gitleaks
run: |
GITLEAKS_FILE="gitleaks_8.24.2_linux_x64.tar.gz"
curl -sSLo "$GITLEAKS_FILE" "https://github.com/gitleaks/gitleaks/releases/download/v8.24.2/$GITLEAKS_FILE"
curl -sSLo gitleaks_checksums.txt "https://github.com/gitleaks/gitleaks/releases/download/v8.24.2/gitleaks_8.24.2_checksums.txt"
curl --retry 5 --retry-delay 5 --retry-connrefused -sSLo "$GITLEAKS_FILE" \
"https://github.com/gitleaks/gitleaks/releases/download/v8.24.2/$GITLEAKS_FILE"
curl --retry 5 --retry-delay 5 --retry-connrefused -sSLo gitleaks_checksums.txt \
"https://github.com/gitleaks/gitleaks/releases/download/v8.24.2/gitleaks_8.24.2_checksums.txt"
grep "$GITLEAKS_FILE" gitleaks_checksums.txt | sha256sum -c -
tar -xzf "$GITLEAKS_FILE" gitleaks
chmod +x gitleaks
Expand All @@ -40,8 +42,10 @@ jobs:
ACTIONLINT_VERSION="1.7.10"
ACTIONLINT_FILE="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz"
ACTIONLINT_BASE_URL="https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}"
curl -sSLo "$ACTIONLINT_FILE" "${ACTIONLINT_BASE_URL}/${ACTIONLINT_FILE}"
curl -sSLo actionlint_checksums.txt "${ACTIONLINT_BASE_URL}/actionlint_${ACTIONLINT_VERSION}_checksums.txt"
curl --retry 5 --retry-delay 5 --retry-connrefused -sSLo "$ACTIONLINT_FILE" \
"${ACTIONLINT_BASE_URL}/${ACTIONLINT_FILE}"
curl --retry 5 --retry-delay 5 --retry-connrefused -sSLo actionlint_checksums.txt \
"${ACTIONLINT_BASE_URL}/actionlint_${ACTIONLINT_VERSION}_checksums.txt"
grep "$ACTIONLINT_FILE" actionlint_checksums.txt | sha256sum -c -
tar -xzf "$ACTIONLINT_FILE" actionlint
chmod +x actionlint
Expand Down
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
**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-07-25 - Fix weak regex validation in readline prompt
**Vulnerability:** Weak regex `^[0-9]+$` was used to validate integer inputs from `readline()`.
**Learning:** Large numeric inputs bypass this validation, are coerced to `NA` by `as.integer()`, and cause execution crashes (DoS) when used in conditionals.
**Prevention:** Use strictly bounded exact-match regex like `^[12]$` to prevent coercion crashes and DoS vulnerabilities.
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 (grepl("^[12]$", n)) {
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 (grepl("^[12]$", n)) {
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 (grepl("^[12]$", n)) {
return(as.integer(n))
}
}
Expand Down
40 changes: 40 additions & 0 deletions tests/testthat/test-sentinel-readline-validation.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
library(testthat)
library(mockery)

test_that("autoFIPC validates interactive readline correctly with strict regex", {
# We source the code directly to bypass pkgload/namespace export issues
# in this mocked environment test

# Ensure we mock interactive prompt paths, without modifying logic
mockery::stub(autoFIPC, 'interactive', TRUE)

# Mock mirt.model as it is called early for generating model syntax
mockery::stub(autoFIPC, 'mirt::mirt.model', function(...) stop('forced failure'))

# Test with valid inputs (1 or 2)
# Needs to handle multiple readlines if it passes the first one
mockery::stub(autoFIPC, 'readline', mockery::mock('1', '1', '1', '1'))

expect_error(
autoFIPC(
newformXData = data.frame(A=1),
oldformYData = data.frame(A=2),
newformCommonItemNames = c('A'),
oldformCommonItemNames = c('A')
),
"forced failure|Please write down pairs correctly|Too many invalid common item confirmation attempts"
Comment on lines +18 to +25

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

๐ŸŽฏ Functional Correctness | ๐ŸŸก Minor | โšก Quick win

expect_error()์˜ ์˜ค๋ฅ˜ ์กฐ๊ฑด์„ ์ •ํ™•ํžˆ ์ œํ•œํ•˜์„ธ์š”.

์œ ํšจ ์ž…๋ ฅ ํ…Œ์ŠคํŠธ๊ฐ€ "Too many invalid common item confirmation attempts"์™€ "Please write down pairs correctly"๋„ ํ—ˆ์šฉํ•ฉ๋‹ˆ๋‹ค. ๋”ฐ๋ผ์„œ ^[12]$๊ฐ€ 1์„ ๊ฑฐ๋ถ€ํ•ด๋„ ํ…Œ์ŠคํŠธ๊ฐ€ ํ†ต๊ณผํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค. ์œ ํšจ ์ž…๋ ฅ ๊ฒฝ๋กœ์—์„œ๋Š” mirt::mirt.model ์Šคํ…์˜ "forced failure"๋งŒ ์˜ˆ์ƒํ•˜์„ธ์š”. ํฐ ์ˆซ์ž ์ž…๋ ฅ ๊ฒฝ๋กœ์—์„œ๋Š” "Too many invalid common item confirmation attempts"๋งŒ ์˜ˆ์ƒํ•˜์„ธ์š”.

์ˆ˜์ • ์˜ˆ์‹œ
-    "forced failure|Please write down pairs correctly|Too many invalid common item confirmation attempts"
+    "^forced failure$"
...
-    "Please write down pairs correctly|forced failure|Too many invalid common item confirmation attempts"
+    "^Too many invalid common item confirmation attempts$"

Also applies to: 31-38

๐Ÿค– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/testthat/test-sentinel-readline-validation.R` around lines 18 - 25,
Restrict the autoFIPC error assertions in the valid-input and large-number test
cases to their intended failures. In the valid-input case, expect only the
mirt::mirt.model stubโ€™s โ€œforced failureโ€; in the large-number case, expect only
โ€œToo many invalid common item confirmation attemptsโ€. Remove the broader
alternatives, including โ€œPlease write down pairs correctlyโ€, from both
expect_error patterns.

)

# Test with malicious inputs (large numbers that cause NA coercions if unhandled)
# It should fail validation 3 times and throw the "Too many invalid" error
mockery::stub(autoFIPC, 'readline', mockery::mock('9999999999999999', '9999999999999999', '9999999999999999'))
expect_error(
autoFIPC(
newformXData = data.frame(A=1),
oldformYData = data.frame(A=2),
newformCommonItemNames = c('A'),
oldformCommonItemNames = c('A')
),
"Please write down pairs correctly|forced failure|Too many invalid common item confirmation attempts"
)
})
Loading