Skip to content

security(reviewer): fail closed on unknown manifest evidence - #427

Merged
seonghobae merged 2 commits into
mainfrom
security/fail-closed-review-manifest-schema
Aug 19, 2026
Merged

security(reviewer): fail closed on unknown manifest evidence#427
seonghobae merged 2 commits into
mainfrom
security/fail-closed-review-manifest-schema

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Scope

Fail closed when untrusted review-manifest payloads contain fields outside the reviewed schema. The protected implementation inherited Pydantic's default extra='ignore', so unexpected evidence could be silently discarded at the reviewer trust boundary.

Test-first repair

  • RED a0aa2b73fd282022d5c2b94f83bec80a61bc2971 exercises the top-level manifest and every nested wire model with an unexpected evidence field and requires ValidationError.
  • GREEN candidate/current head 78e798b7c8d28844d44621d1129b96e4f3c80072 introduces one strict manifest base model with ConfigDict(extra='forbid') and makes all six wire models inherit it. Existing field semantics and blocking-finding logic are unchanged.

Boundaries

No central .github, naruon, contextual-orchestrator, image-build, credential, merge-policy, or external-admin changes. This branch is independent of #407, #428, #429, and the Cursor-owned #425 documentation lane.

Keep Draft until fresh exact-head application CI, reviewer-ci, eligible Security Scan, and current review-thread evidence are terminal-success on one unchanged head.

Summary by CodeRabbit

  • 개선 사항

    • 매니페스트 데이터가 정의되지 않은 필드를 포함할 경우 검증 오류로 거부되도록 강화했습니다.
    • 의존성, 보안, 리뷰 댓글, 검사 결과 및 변경 파일 관련 데이터의 입력 검증이 일관되게 적용됩니다.
  • 테스트

    • 알 수 없는 필드가 올바르게 거부되는지 확인하는 검증 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 32b577e5-b99c-4445-9c6f-6d3e54da4d0c

📥 Commits

Reviewing files that changed from the base of the PR and between a0b27ee and 78e798b.

📒 Files selected for processing (2)
  • reviewer/noema_reviewer/manifest.py
  • reviewer/tests/test_manifest.py

📝 Walkthrough

Walkthrough

모든 매니페스트 모델이 extra="forbid"를 사용하는 _StrictManifestModel을 상속합니다. 테스트는 알 수 없는 필드에 대한 ValidationError를 검증하고, 의존성 차단 결과 테스트 데이터를 정리합니다.

Changes

엄격한 매니페스트 검증

Layer / File(s) Summary
엄격한 매니페스트 모델 정의
reviewer/noema_reviewer/manifest.py
_StrictManifestModel을 추가하고 DependencyFinding, SecurityFinding, ReviewComment, CheckConclusion, ChangedFile, ReviewManifest가 이를 상속하도록 변경했습니다.
매니페스트 검증 테스트
reviewer/tests/test_manifest.py
각 모델에 unexpected_evidence 필드를 전달하면 ValidationError가 발생하는지 파라미터화하여 검증했습니다. 의존성 테스트 입력과 결과 검증도 정리했습니다.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/fail-closed-review-manifest-schema

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.

@seonghobae
seonghobae marked this pull request as ready for review August 19, 2026 04:22
@seonghobae
seonghobae merged commit 5bdfe2a into main Aug 19, 2026
15 of 16 checks passed
@seonghobae
seonghobae deleted the security/fail-closed-review-manifest-schema branch August 19, 2026 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant