Skip to content

superseded: option validation evidence consolidated in #126 - #84

Closed
seonghobae wants to merge 9 commits into
masterfrom
fix-input-validation-14175515133901881384
Closed

superseded: option validation evidence consolidated in #126#84
seonghobae wants to merge 9 commits into
masterfrom
fix-input-validation-14175515133901881384

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #126

Fresh effective-delta audit at d75babd1fd7814a045f8538d8e9e4a16872d03fb found the same nested/adj/conf.level option-domain semantics now owned by canonical Draft #126@6e68c37df52d3b7407bcb3c028d64ac7b5f2e122, plus useful zero-length/non-finite regression coverage.

#126 now explicitly inherits that evidence: zero-length logical/character/numeric controls, NaN, Inf, -Inf, non-scalar, wrong-type, missing, boundary and out-of-domain values all exercise the exported boundary, with stable package-owned messages and conditionCall(err) == NULL. The selected source predicate already rejects NaN through missingness and infinities through the strict (0,1) bounds.

The branch-local .jules/sentinel.md doctrine is intentionally not inherited as repository-wide policy. No callback-validation contract is present here; that distinct work remains in #122 and is not being collapsed by this closure.

Every valid semantic/test contract unique to this PR is therefore present in #126. This closure transfers neither historical GREEN evidence nor merge authority.

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.
@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

vuongtest()icci()에 제어 인자 검증을 추가했다. 잘못된 입력은 호출 정보 없이 오류를 반환한다. 관련 입력 사례와 오류 동작을 testthat으로 검증하고 보안 변경 이력을 기록했다.

Changes

입력 검증

Layer / File(s) Summary
제어 인자 검증
R/icci.R, R/vuongtest.R
icci()conf.level의 단일 유한 숫자와 범위를 검증한다. vuongtest()nested의 논리형 조건과 adj의 허용 문자열을 검증한다.
검증 테스트 및 변경 이력
tests/testthat/test-input-validation.R, .jules/sentinel.md
잘못된 입력, 지정된 오류 메시지, 호출 정보 비노출을 테스트한다. 입력 검증 및 오류 경계를 보안 변경 이력에 기록한다.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 모델 디스패치 전에 잘못된 공개 입력을 거부하도록 검증을 수정한 주요 변경 사항을 정확하고 간결하게 설명합니다.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-input-validation-14175515133901881384

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 727f62b7497b8b14f62a8e9049ca3a9baa4d4028.

  • Head SHA: 727f62b7497b8b14f62a8e9049ca3a9baa4d4028

  • Workflow run: 31458096008

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test-input-validation.R"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test-input-validation.R"]
  R2 --> V2["targeted test run"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 11, 2026

Copy link
Copy Markdown

OpenCode Review Overview

  • Head SHA: 3d8848c97db6a2d132dbcb01923b401bbd6821f9
  • Workflow run: 31497350819
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 3d8848c97db6a2d132dbcb01923b401bbd6821f9.

  • Head SHA: 3d8848c97db6a2d132dbcb01923b401bbd6821f9

  • Workflow run: 31497350819

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test-input-validation.R"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test-input-validation.R"]
  R2 --> V2["targeted test run"]
Loading

seonghobae and others added 4 commits August 11, 2026 13:21
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.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 19607c6087284ec787af47490650a4140650215a.

  • Head SHA: 19607c6087284ec787af47490650a4140650215a

  • Workflow run: 31462496395

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test-input-validation.R"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test-input-validation.R"]
  R2 --> V2["targeted test run"]
Loading

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

seonghobae and others added 2 commits August 11, 2026 12:52
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.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 3d8848c97db6a2d132dbcb01923b401bbd6821f9.

  • Head SHA: 3d8848c97db6a2d132dbcb01923b401bbd6821f9

  • Workflow run: 31497350819

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (3 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (3 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Test: test-input-validation.R"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test-input-validation.R"]
  R2 --> V2["targeted test run"]
Loading

@opencode-agent opencode-agent Bot added priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: maintenance Maintenance, build, dependency, or operational upkeep labels Aug 22, 2026
@seonghobae seonghobae removed the type: maintenance Maintenance, build, dependency, or operational upkeep label Sep 2, 2026
@seonghobae
seonghobae marked this pull request as draft September 4, 2026 03:10
@seonghobae seonghobae changed the title 🛡️ Sentinel: [MEDIUM] Fix error handling leaking call stacks via unvalidated inputs fix(validation): reject invalid public inputs before model dispatch Sep 4, 2026
@seonghobae seonghobae changed the title fix(validation): reject invalid public inputs before model dispatch superseded: option validation evidence consolidated in #126 Sep 6, 2026
@seonghobae seonghobae closed this Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant