feat: add baseline package tests and private-safe security audit - #5
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit릴리스 노트
Walkthrough보안 감사 워크플로우(gitleaks/actionlint) 및 관련 설정 추가, 테스트 인프라(testthat) 도입, DESCRIPTION·문서 업데이트, 그리고 Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (push/PR)
participant GH as GitHub Actions
participant Runner as Hardened Runner
participant Gitleaks as gitleaks
participant Actionlint as actionlint
participant Repo as Repository
Dev->>GH: push/PR to main/master
GH->>Runner: start workflow (security-audit)
Runner->>Repo: checkout code
Runner->>Gitleaks: install & run gitleaks --config .gitleaks.toml
Gitleaks-->>Runner: scan results
Runner->>Actionlint: download & run actionlint on .github/workflows
Actionlint-->>Runner: lint results
Runner-->>GH: report status (success/failure)
GH-->>Dev: workflow status/annotations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3df121cbaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| - name: Validate workflows | ||
| run: | | ||
| curl -sSL https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash | bash -s -- 1.7.10 |
There was a problem hiding this comment.
Pin actionlint installer script to immutable revision
This step pipes and executes https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash, so every run trusts whatever is on the moving main branch at that moment; if upstream changes or is compromised, CI will execute unreviewed code and private-repo source can be exposed from the runner. This also conflicts with the pinned-workflow guardrail in /workspace/aFIPC/AGENTS.md ("Keep .github/workflows/ green and action SHAs pinned"). Use a commit-pinned URL (or vendor the script) so the executed installer is reproducible and auditable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved by removing the mutable installer-script pipeline from main and using versioned release assets with checksum verification.
PR checks evidenceRequired checks
Merge state
Effective branch rules (master)
Code scanning API state
Head check-runs
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
R/aFIPC.R (1)
101-118:⚠️ Potential issue | 🟠 Major비정상 입력 처리에서 undefined 함수 호출.
숫자가 아닌 입력 시readinteger()가 호출되는데 정의가 없어 즉시 실패합니다. 동일 함수로 재질의하도록 수정하세요.🛠️ 수정 제안
- if (!grepl("^[0-9]+$", n)) { - return(readinteger()) - } + if (!grepl("^[0-9]+$", n)) { + return(checkoldformBILOGprior()) + }
🤖 Fix all issues with AI agents
In @.github/workflows/security-audit.yml:
- Around line 25-38: The workflow currently downloads and runs external binaries
without integrity checks; update the "Install gitleaks" and "Validate workflows"
steps to fetch and verify release checksums (or signatures) before execution:
for the gitleaks step (refer to the "Install gitleaks" and subsequent
"./gitleaks detect" usage) download the official checksum (e.g., SHA256SUMS or
.sha256) for v8.24.2 from the gitleaks release, verify the tarball with
sha256sum (or gpg verify if a signature is provided) and fail the job if
verification fails, then extract and chmod only after verification; for
actionlint (the script piped from rhysd/actionlint and the "./actionlint"
invocation in "Validate workflows") either download a released artifact and its
checksum/signature and verify before running or fetch the script plus the
published checksum/signature and validate the downloaded binary/version matches
the checksum, aborting on mismatch.
In `@R/aFIPC.R`:
- Around line 200-215: The while loop that repeatedly calls mirt::mirt to create
oldFormModel can loop forever if model fitting keeps failing; modify the code
around oldFormModel (the try + while (!exists('oldFormModel')) block that calls
mirt::mirt with data = oldformYDataK and itemtype = itemtype) to implement a
bounded retry: introduce a max_retries counter (e.g., 5–10), increment an
attempt counter on each failure, catch and store the error from mirt::mirt, add
a short backoff (Sys.sleep) between attempts, and after exceeding max_retries
stop retrying and handle the failure (log or return a clear error/NA) instead of
spinning; apply the same change to the analogous block that creates the new form
model.
- Around line 137-165: The code wraps mirt::mirt calls in try() (e.g. the
assignment to oldFormModel via mirt::mirt) but never checks whether the call
actually returned a valid model before later accessing slots like `@OptimInfo`;
update the try usage to capture the result (e.g. store try(...) into a variable
or use tryCatch), verify the object is a valid mirt model (or non-NULL) after
the call, and if it failed either stop with a clear error or branch to a
fallback path (same fix for oldFormModel and the corresponding newFormModel
block around the second mirt call); reference oldFormModel, mirt::mirt, and
`@OptimInfo` when making the check and handling failure.
- Around line 590-597: The code unconditionally removes parameters matching
"^ak" from IPDParmNames, which incorrectly excludes ak* for nominal items;
update the logic around the IPDParmNames filtering so that ak parameters are
only removed when the item's type is not "nominal" (i.e., check the itemtype or
a variable indicating nominal vs non-nominal before applying the grep("^ak",
IPDParmNames) removal), keeping the existing removals for "^MEAN", "^COV", and
"^d0$" unchanged and only conditionally including grep("^ak", IPDParmNames) in
the subtraction when itemtype != "nominal".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@R/aFIPC.R`:
- Around line 101-108: The function checkoldformBILOGprior calls an undefined
readinteger(), causing a runtime error; change the error branch to recursively
call checkoldformBILOGprior() (matching the pattern used in checkCorrect and
checknewformBILOGprior) so invalid input re-prompts correctly and returns the
validated value from checkoldformBILOGprior().
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai resolve |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Addressed or triaged in-thread for solo-maintained compatibility constraints.
✅ Actions performedComments resolved and changes approved. |
Summary
Validation
Risk