security(release): bind sealed evidence root identity - #211
Conversation
|
Warning Review limit reached
Next review available in: 7 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough릴리스 증거 검증이 원본 파일 직접 접근에서 descriptor-bound 개인 스냅샷 기반 검증으로 변경되었습니다. 루트와 파일 identity, 크기 제한, 검증 중 변경 여부를 확인합니다. 관련 회귀 테스트도 추가 및 갱신되었습니다. Changes릴리스 증거 스냅샷 검증
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to The change tightens release-evidence directory membership checks and is backed by passing current-head validation, but snapshot-root path normalization may reject valid temporary-directory paths on macOS and cause release verification failures there. This is a bounded portability risk requiring owner awareness or follow-up. Sequence Diagram(s)sequenceDiagram
participant Caller
participant build_evidence_manifest
participant _snapshot_selected_evidence
participant _verify_sbom
Caller->>build_evidence_manifest: manifest 생성 요청
build_evidence_manifest->>_snapshot_selected_evidence: 증거 파일 스냅샷 요청
_snapshot_selected_evidence-->>build_evidence_manifest: private snapshot과 원본 digest 반환
build_evidence_manifest->>_verify_sbom: snapshot SBOM 검증
_verify_sbom-->>build_evidence_manifest: SBOM 검증 결과 반환
build_evidence_manifest->>build_evidence_manifest: snapshot과 원본 변경 여부 확인
build_evidence_manifest-->>Caller: 결정적 manifest 반환
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/test_sealed_release_evidence_snapshot_boundary.py (1)
342-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value이 테스트는
SHA256SUMS해시 호출 횟수에 강하게 결합됩니다.
hash_then_restore_root는 세 번째SHA256SUMS해시 시점에 원본 루트를 복원합니다. 이 값은_load_checksums의 내부 해시 횟수(사전/사후 2회)와 최종 체크섬 재해시 1회에 의존합니다._load_checksums구현이 바뀌면 테스트 의도가 조용히 달라집니다.라벨과 횟수 대신 검증 단계 이름을 기준으로 복원 시점을 고정하는 방법을 권장합니다. 예를 들어 최종 원본 재검증 직전 호출되는
_select_evidence_paths또는_evidence_root_identity호출 지점을 후크로 사용하십시오.🤖 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/test_sealed_release_evidence_snapshot_boundary.py` around lines 342 - 403, Update test_manifest_keeps_original_root_authority_across_transient_replacement to trigger root restoration using a stable verification-stage hook, such as the final call to _select_evidence_paths or _evidence_root_identity, instead of counting SHA256SUMS calls in hash_then_restore_root. Remove the checksum_hash_count dependency while preserving restoration immediately before final original-root revalidation.tests/test_sealed_release_evidence_root_membership.py (1)
41-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win정규화된 루트와
root가 다르면 후크가 동작하지 않습니다.
_snapshot_selected_evidence는_require_canonical_evidence_root의 결과를_select_evidence_paths에 전달합니다. 정규화 결과가root와 다른 경로 객체이면(예: 심링크 구성 요소가 해석되는 플랫폼) 47행 조건이 성립하지 않습니다. 그러면 추가 자식이 생성되지 않아 테스트가 의도와 다르게 실패합니다.경로 문자열 비교 대신 파일시스템 identity 비교로 후크 조건을 고정하는 방법을 권장합니다.
♻️ 제안 리팩터
+ root_identity = (root.stat().st_dev, root.stat().st_ino) + def select_then_add_unexpected_member( candidate: Path, ) -> tuple[Path, Path, Path, Path, Path, Path]: """Add one unreviewed direct child immediately after source selection.""" nonlocal source_selections selected = original_select(candidate) - if candidate == root: + candidate_state = candidate.stat() + if (candidate_state.st_dev, candidate_state.st_ino) == root_identity: source_selections += 1 if source_selections == 1: (root / "unexpected.txt").write_bytes(b"unreviewed evidence") return selected🤖 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/test_sealed_release_evidence_root_membership.py` around lines 41 - 51, Update select_then_add_unexpected_member to detect the selected evidence root using filesystem identity rather than Path equality with root. Compare candidate and root via resolved/stat-based identity while preserving the existing first source selection behavior and unexpected.txt creation.src/egressweave/release_evidence.py (1)
240-255: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win한도와 라벨 튜플이 두 곳에 중복됩니다.
_snapshot_selected_evidence의maximums/labels(240-255행)와build_evidence_manifest의original_maximums/original_labels(670-685행)는 동일한 6개 순서를 반복합니다. 두 목록이 갈라지면 원본 재검증이 스냅샷과 다른 한도를 적용합니다. 순서와 한도는 보안 경계이므로 한 곳에서만 정의하십시오.모듈 수준 상수 한 쌍으로 통합하고 양쪽에서 재사용하는 방법을 권장합니다.
♻️ 제안 리팩터
+EVIDENCE_MAXIMUM_BYTES = ( + MAX_ARTIFACT_BYTES, + MAX_ARTIFACT_BYTES, + MAX_SBOM_BYTES, + MAX_SBOM_BYTES, + MAX_SOURCE_IDENTITY_BYTES, + MAX_CHECKSUM_BYTES, +) +EVIDENCE_LABELS = ( + "wheel", + "source distribution", + "wheel SBOM", + "source-distribution SBOM", + "sealed source identity", + "SHA256SUMS", +)
_snapshot_selected_evidence안에서:- maximums = ( - MAX_ARTIFACT_BYTES, - MAX_ARTIFACT_BYTES, - MAX_SBOM_BYTES, - MAX_SBOM_BYTES, - MAX_SOURCE_IDENTITY_BYTES, - MAX_CHECKSUM_BYTES, - ) - labels = ( - "wheel", - "source distribution", - "wheel SBOM", - "source-distribution SBOM", - "sealed source identity", - "SHA256SUMS", - ) + maximums = EVIDENCE_MAXIMUM_BYTES + labels = EVIDENCE_LABELS
build_evidence_manifest안에서original_maximums/original_labels도 동일 상수를 사용하십시오.🤖 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 `@src/egressweave/release_evidence.py` around lines 240 - 255, Define one module-level constants pair containing the six evidence maximums and labels in their existing order, then update both _snapshot_selected_evidence and build_evidence_manifest to reuse it instead of maintaining local maximums/original_maximums and labels/original_labels tuples. Preserve the current limits and label ordering exactly.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/egressweave/release_evidence.py`:
- Around line 284-299: Normalize snapshot_root to its strict resolved path
before passing it to _select_evidence_paths, so symlinked temporary-directory
paths such as /var and /private/var compare consistently. Preserve the existing
snapshot creation and return behavior, changing only the path value supplied to
_select_evidence_paths.
---
Nitpick comments:
In `@src/egressweave/release_evidence.py`:
- Around line 240-255: Define one module-level constants pair containing the six
evidence maximums and labels in their existing order, then update both
_snapshot_selected_evidence and build_evidence_manifest to reuse it instead of
maintaining local maximums/original_maximums and labels/original_labels tuples.
Preserve the current limits and label ordering exactly.
In `@tests/test_sealed_release_evidence_root_membership.py`:
- Around line 41-51: Update select_then_add_unexpected_member to detect the
selected evidence root using filesystem identity rather than Path equality with
root. Compare candidate and root via resolved/stat-based identity while
preserving the existing first source selection behavior and unexpected.txt
creation.
In `@tests/test_sealed_release_evidence_snapshot_boundary.py`:
- Around line 342-403: Update
test_manifest_keeps_original_root_authority_across_transient_replacement to
trigger root restoration using a stable verification-stage hook, such as the
final call to _select_evidence_paths or _evidence_root_identity, instead of
counting SHA256SUMS calls in hash_then_restore_root. Remove the
checksum_hash_count dependency while preserving restoration immediately before
final original-root revalidation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 148c41bc-a175-40e1-8323-666280b5b829
📒 Files selected for processing (5)
src/egressweave/release_evidence.pytests/test_sealed_release_evidence_checksum_snapshot.pytests/test_sealed_release_evidence_root_membership.pytests/test_sealed_release_evidence_sbom_digest_binding.pytests/test_sealed_release_evidence_snapshot_boundary.py
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review follow-up
The current head has addressed the canonical snapshot-root and filesystem-identity hook findings, but two valid review items remain in the exact diff:
release_evidence.pystill duplicates the six security-significant evidence limits and labels in_snapshot_selected_evidence()andbuild_evidence_manifest(). A future edit can make original-source revalidation use different ordering or ceilings from snapshot admission. Define one module-level immutable tuple for the six(maximum_bytes, label)contracts and derive both loops from it.test_manifest_keeps_original_root_authority_across_transient_replacementstill restores the root on the thirdSHA256SUMShash, coupling the security regression to an internal call count. Trigger restoration at a stable final-original-root verification seam—such as the final_select_evidence_pathsor_evidence_root_identityobservation—so refactoring digest calls cannot silently move the attack timing.
Also update the PR body: its stated exact head 560271d..., 2-line production delta, test counts, and review state no longer describe current head ece3ec3556e91b07ee85592cc15289c050387f92 or the five-file snapshot/digest implementation. Do not transfer predecessor evidence when correcting the description.
|
@opencode-agent fix Address the exact-current-head review follow-up on |
1 participant
Purpose
Bind one sealed release-evidence verification decision to the exact admitted evidence-directory identity and exact direct-child membership on protected
main7faf7a3b8a47980113982914000e724ab6a6cda5.The existing verifier already canonicalized the evidence root, descriptor-bound the six admitted child files, copied them into a private finite snapshot, and rejected whole-root identity replacement. A second current-head audit found a narrower membership gap: an extra direct child could be added immediately after the original six-file selection while the directory inode and every selected file identity remained unchanged. The private snapshot would then contain only the originally selected six files, so exact direct-child cardinality was no longer revalidated at the copy/return boundaries.
Exact TDD lineage
Current exact head:
560271d2b66b6efc89f4330a84ab00f023d05624over unchanged live base7faf7a3b8a47980113982914000e724ab6a6cda5.RED
Test-only commit
f06d82f0bc82d06f20321d0da20814c7b37ba36daddedtests/test_sealed_release_evidence_root_membership.py. The regression addsunexpected.txtimmediately after the first admitted source membership selection and requires_snapshot_selected_evidence(...)to fail closed with the existing cardinality-mismatch contract.Hosted CI run
31647739770reproduced the defect on the exact RED head across the supported Python matrix. The Python 3.14 lane checked outf06d82f..., Ruff/package checks passed, and pytest reported 1 failed, 1035 passed because the expectedSystemExitwas not raised.Narrow GREEN
Production commit
560271d2b66b6efc89f4330a84ab00f023d05624changes onlysrc/egressweave/release_evidence.pywith 2 additions / 0 deletions:_select_evidence_paths(canonical_root)before snapshot copy;_select_evidence_paths(canonical_original_root)before accepting the final original-source digest comparison.This keeps the private bounded snapshot, finite streaming ceilings, exact six-file cardinality, no-symlink path contract, selected-file descriptor/path identity, credential-free operation and deterministic manifest semantics unchanged. It adds no publication/signing authority and does not claim provenance from a consistency verifier.
Exact current-head proof
On unchanged
560271d2b66b6efc89f4330a84ab00f023d05624:31648071796: success;560271d..., CPython 3.14.6;src/egressweave/release_evidence.py: 434/434 statements, 138/138 branches, 0 partial, 100%;31648071861: success;Live non-passing gates
This PR intentionally remains Draft and must not merge yet.
Security Scan run
31648071888is aggregate/wrapper green, but dependency-review job94286139256explicitly warns that Dependency Review is unavailable and its actual immutable-pinnedDependency reviewaction was skipped. Under the repository evidence contract this is non-passing supply-chain evidence. The root-cause repair remains the read-only organization-ownedContextualWisdomLab/.github#897; no EgressWeave-local substitute or weaker gate is acceptable.The exact-head required Strix check
94286191545is currently in progress. Pending evidence is non-passing. If it later terminates without an actual semantic report, including the knownStrix backend unavailableclass tracked by read-only central issueContextualWisdomLab/.github#891, wrapper success still will not count as passing semantic review. Do not churn this clean head merely to retrigger a provider.No predecessor check/review/status/model/synthetic evidence transfers to this exact head or a changed live base.
Merge constraints
Merge remains prohibited until the unchanged exact head/live base satisfy every actual live ruleset and required workflow; the real immutable Dependency Review action executes and succeeds; Strix produces an actual exact-head semantic result; every valid current finding is addressed with zero unresolved review threads; exact coverage/package/SBOM/provenance/release evidence remains passing; and any review/approval requirement actually imposed by live governance is satisfied without bypass, self-approval or invented authority.
Summary by CodeRabbit
개선 사항
테스트