Skip to content

fix(result): enforce durable consent reference integrity - #273

Merged
seonghobae merged 19 commits into
mainfrom
fix/result-consent-array-integrity-20260821
Aug 25, 2026
Merged

seonghobae merged 19 commits into
mainfrom
fix/result-consent-array-integrity-20260821

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Why

ResultSnapshot::new rejects missing, malformed, and duplicate consent-snapshot references before they become immutable product result evidence. Protected-main PostgreSQL persistence only checked that consent_snapshot_refs was non-empty, so direct SQL, recovery tooling, or a damaged writer could persist NULL, blank, numeric-like, or duplicate consent identities that the domain cannot produce.

TDD lineage

  • RED 29bc60d3de887a56989a256939a5bc62873bff37: real PostgreSQL tests require NULL, blank, numeric-like, and duplicate consent references to fail closed and require migration reapplication to repair a weakened same-named constraint.
  • GREEN 1ed1e6a1c8a0da8a723f70bd6a5228c39528b4f2: add an immutable SQL validator and an owned result_snapshot_consent_refs_integrity_check; reapplication drops/recreates that constraint so existing product schemas are strengthened rather than only fresh installs.

Boundary

This is result-evidence persistence hardening only. It does not change consent purpose semantics, scoring/numeric psychometrics, result calculation, cross-service ownership, or the existing non-empty consent requirement. The validator preserves the current persisted opaque-reference syntax; broader Unicode reference-parity work remains separate.

Verification required before merge

Exact-current-head Runtime CI, PostgreSQL contract tests, rustfmt/Clippy/rustdoc, exact owned line/branch coverage, Security/SAST/SBOM/provenance, zero valid unresolved findings, and qualifying independent non-author last-push review are required. Never self-approve or transfer evidence from another head.


Open in Devin Review

Summary by CodeRabbit

  • 버그 수정
    • 결과 스냅샷 참조값 검증을 강화했습니다.
    • 공백, 제어 문자, 빈 값, 잘못된 유니코드 및 숫자 형식 참조를 차단합니다.
    • 동의 참조 배열의 중복 값과 유효하지 않은 항목을 거부합니다.
    • supersedes_ref가 자기 자신을 참조하는 경우를 방지합니다.
    • 기존 데이터베이스 제약조건도 새 검증 기준에 맞게 복구됩니다.
    • 저장 시 식별자와 참조값의 앞뒤 유니코드 공백을 자동으로 정리합니다.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4df34cac-4a41-45dd-beb4-49724b1a7558

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 513b4df6-2416-4855-a0e1-50081aa3f139

📥 Commits

Reviewing files that changed from the base of the PR and between 89bf03d and 5da80b7.

📒 Files selected for processing (3)
  • migrations/0007_result_snapshot.sql
  • tests/postgres_result_snapshot_reference_parity.rs
  • tests/result_required_references.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

PostgreSQL 마이그레이션에 Rust 호환 참조 검증 함수와 CHECK 제약조건을 추가했습니다. 기존 제약조건을 새 규칙으로 재생성하고, 결과 스냅샷 참조와 동의 참조 배열의 무효 값을 검증하는 통합 테스트를 추가했습니다.

Changes

결과 스냅샷 참조 검증

Layer / File(s) Summary
참조 검증 함수와 CHECK 제약조건
migrations/0007_result_snapshot.sql
Unicode 숫자, 공백, 제어문자, 빈 참조, 중복 동의 참조 및 자기 참조를 검증하는 함수를 추가했습니다. 결과 스냅샷과 observation 참조 필드가 새 함수를 사용하도록 변경했습니다.
기존 제약조건 재적용
migrations/0007_result_snapshot.sql
기존 참조 관련 CHECK 제약조건을 삭제하고 새 제약조건을 다시 생성합니다. 기존 행도 새 검증 규칙으로 확인합니다. 기존 불변성 차단 트리거는 유지합니다.
Rust 참조 경계 패리티 테스트
tests/postgres_result_snapshot_reference_parity.rs, tests/result_required_references.rs
결과 스냅샷의 참조 컬럼, 동의 배열 및 observation construct 참조가 Rust opaque-reference 규칙에 따라 무효 값을 거부하는지 검증합니다. 결과 생성 시 참조값의 앞뒤 Unicode 공백 제거도 확인합니다.
동의 참조 배열 무결성 테스트
tests/postgres_result_snapshot_consent_refs.rs
NULL, 숫자형 문자열, 공백, 개행 및 중복 동의 참조가 무결성 제약조건에 의해 거부되는지 검증합니다. 약화된 제약조건을 재적용해 복구하는 동작도 확인합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to 5da80

The migration strengthens consent-reference validation, but a non-transactional failure during reapplication could leave result data without the existing reference checks. Merge should wait for a rollback-preserving migration approach or explicit owner acceptance of this bounded data-integrity risk.

Sequence Diagram(s)

sequenceDiagram
  participant RustTests
  participant Migration_0007
  participant PostgreSQL
  RustTests->>Migration_0007: 스키마 마이그레이션 적용
  Migration_0007->>PostgreSQL: 검증 함수와 CHECK 제약조건 생성
  RustTests->>PostgreSQL: 참조 및 동의 배열 삽입
  PostgreSQL->>PostgreSQL: Unicode 참조 규칙 검증
  PostgreSQL-->>RustTests: 유효한 값 저장 또는 CHECK 위반 반환
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 3 files. (1 skipped: 1 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 결과 스냅샷의 동의 참조 무결성을 강화하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/result-consent-array-integrity-20260821

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.

@devin-ai-integration devin-ai-integration 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Preserve the #273 durable result consent-reference integrity constraint and PostgreSQL regression contract on top of current protected main. The #258 Rust toolchain-refresh paths are disjoint, so this reconciliation retains both change sets without force-pushing or changing result semantics beyond the existing PR delta.
@seonghobae

Copy link
Copy Markdown
Contributor Author

Fixed and pushed current head c126491d1303be98d4637d070998a86086cbce73. The result-snapshot parity tests now use clone_into for owned reference replacements and are formatted; no migration behavior was changed. Focused consent/reference tests, fmt, diff check, and clippy pass. The exact-head all-target log contains no failure markers and all final test result lines are ok; the wrapper only failed after cargo completion because zsh reserves the variable name status. Please review this exact head; required GitHub checks remain queued.

coderabbitai[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration 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.

Devin Review found 6 new potential issues.

Open in Devin Review

Comment thread migrations/0007_result_snapshot.sql
Comment thread migrations/0007_result_snapshot.sql
Comment thread migrations/0007_result_snapshot.sql
Comment thread migrations/0007_result_snapshot.sql
@seonghobae
seonghobae merged commit 0cbb099 into main Aug 25, 2026
34 checks passed
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