feat(browser): bind protocol identifiers to OriginWeave authority - #40
Open
seonghobae wants to merge 24 commits into
Open
feat(browser): bind protocol identifiers to OriginWeave authority#40seonghobae wants to merge 24 commits into
seonghobae wants to merge 24 commits into
Conversation
📝 WalkthroughWalkthroughOriginWeave에 보안 계약, 브라우저 권한 레지스트리, 확장 프로그램 접근 평가를 추가했습니다. 브라우저 리소스의 문서 epoch와 origin을 검증합니다. 커버리지 누락 라인은 파일과 CI 로그에 동시에 출력합니다. Changes브라우저 권한 계약
커버리지 로그 출력
Estimated code review effort: 5 (Critical) | ~90 minutes 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 |
seonghobae
marked this pull request as ready for review
August 10, 2026 14:23
![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 (5)
crates/originweave-core/src/lib.rs (1)
15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
contracts재export도 명시적으로 열거하십시오.Line 15-17은
browser_registry의 공개 항목을 명시적으로 열거합니다. Line 18은contracts::*를 글로브로 재export합니다. 이 비대칭 때문에contracts.rs에 새 public 항목을 추가하면 crate 공개 API가 의도 없이 확장됩니다. 두 모듈 모두 명시적 목록으로 통일하십시오.♻️ 명시적 재export 예시
pub use browser_registry::{ BrowserAuthorityRegistry, BrowserRegistryError, MAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES, }; -pub use contracts::*; +pub use contracts::{ + ActionIntentDigest, ActionIntentDigestError, ActionKind, ActionRequest, ApprovalEvidence, + ApprovalScope, BrowserSessionId, BrowsingContextId, Capability, DocumentEpoch, + ExecutionPurpose, ExtensionAccessDecision, ExtensionAccessRequest, ExtensionAgentCapability, + ExtensionAgentGrant, ExtensionId, ExtensionIdError, InstructionSource, NodeHandleError, + ObservedNodeHandle, Origin, OriginError, PolicyContext, RiskClass, RobotsDecision, + SecretDelivery, SessionMode, evaluate_extension_access, +};🤖 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 `@crates/originweave-core/src/lib.rs` around lines 15 - 18, Update the contracts re-export in lib.rs to replace pub use contracts::* with an explicit list of the intended public contracts symbols, matching the existing explicit browser_registry export style. Include the currently supported public items and ensure future additions to contracts.rs do not become crate exports automatically.crates/originweave-core/src/browser_registry_coverage.rs (1)
1-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win이 모듈을
browser_registry.rs의mod tests로 통합하십시오.이 파일은
browser_registry.rs가 이미 검증하는 동작을 다시 검증합니다. 반복 바인딩은crates/originweave-core/tests/browser_authority_registry.rsLine 97-99가, 미등록 세션 거부는browser_registry.rsLine 430-433이,ContextSessionMismatch는crates/originweave-core/tests/browser_authority_registry.rsLine 125-131이 검증합니다. 또한 Line 3-5의values헬퍼는browser_registry.rsLine 311-313과 동일합니다.두 테스트 함수를
browser_registry.rs의mod tests로 이동하고, 이 파일과lib.rsLine 11-12의 배선을 제거하십시오. 그러면 헬퍼 중복이 사라지고 테스트 대상 모듈과 테스트 위치가 일치합니다.🤖 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 `@crates/originweave-core/src/browser_registry_coverage.rs` around lines 1 - 60, Move the tests repeated_node_binding_exercises_the_unit_crate_existing_node_path and session_authority_failures_are_exercised_in_the_unit_crate into browser_registry.rs’s existing mod tests, reusing its values helper. Remove this standalone module and its lib.rs wiring, while preserving the current assertions and coverage.crates/originweave-core/src/browser_registry.rs (2)
18-30: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win세션과 컨텍스트 등록 해제 API를 추가하십시오.
레지스트리는 단조 증가만 수행합니다. 브라우저 세션이 종료되거나 컨텍스트가 폐기될 때 항목을 제거하는 공개 메서드가 없습니다.
advance_document는 노드 바인딩만 정리하고session_by_external,context_by_external,context_session,context_epoch는 유지합니다. 외부 식별자는 최대 512바이트이고 기본 한도는 네임스페이스별 1,000,000이므로 장기 실행 어댑터에서 상주 메모리가 계속 증가합니다. 또한 식별자 공간이 소진되면 레지스트리를 재생성해야만 복구할 수 있습니다.
remove_context와remove_session을 추가하고, 세션 제거 시 소유 컨텍스트와 노드 바인딩도 함께 제거하십시오. 내부 식별자는 재사용하지 않도록 계속 단조 증가시키십시오.Also applies to: 124-143
🤖 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 `@crates/originweave-core/src/browser_registry.rs` around lines 18 - 30, Extend BrowserAuthorityRegistry with public remove_context and remove_session methods. Remove a context from context_by_external, context_session, context_epoch, context_origin, and all node_by_external entries for that context; remove_session must also remove its external mapping and every owned context and node binding. Keep next_session_id, next_context_id, and next_node_id strictly monotonically increasing without reusing identifiers.
230-232: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win메시지에 바이트 한도를 하드코딩하지 마십시오.
메시지가
512를 문자열에 직접 포함합니다.MAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES를 변경하면 메시지가 실제 한도와 어긋납니다. 상수를 서식에 넣으면 출력 텍스트는 동일하게 유지되며 기존 테스트 단정도 계속 통과합니다.♻️ 상수 기반 메시지
- Self::InvalidExternalIdentifier => { - formatter.write_str("external browser identifier must contain 1 to 512 UTF-8 bytes") - } + Self::InvalidExternalIdentifier => write!( + formatter, + "external browser identifier must contain 1 to {MAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES} UTF-8 bytes" + ),🤖 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 `@crates/originweave-core/src/browser_registry.rs` around lines 230 - 232, Update the Self::InvalidExternalIdentifier formatting branch to derive the byte limit from MAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES instead of hardcoding 512 in the message, while preserving the existing output text and test expectations.crates/originweave-core/src/contracts.rs (1)
214-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
OriginError가std::error::Error를 구현하지 않아 호출자가?를 쓸 수 없습니다.NodeHandleError와BrowserRegistryError는Display와std::error::Error를 구현합니다.OriginError는 구현하지 않습니다. 그 결과Box<dyn Error>를 반환하는 테스트와 어댑터가expect()와 린트 해제에 의존합니다.
crates/originweave-core/src/contracts.rs#L214-L235:OriginError에 결정론적 메시지를 가진Display와std::error::Error구현을 추가하십시오.ActionIntentDigestError와ExtensionIdError도 동일하게 처리하십시오.crates/originweave-core/tests/browser_authority_registry.rs#L10-L12:loopback_origin이Result<Origin, Box<dyn Error>>를 반환하도록 바꾸고.expect()를?로 교체하십시오. Line 141-142의.expect()도 함께 교체하십시오.crates/originweave-core/tests/browser_authority_registry.rs#L1-L1:.expect()제거 후#![allow(clippy::expect_used)]를 삭제하십시오.🤖 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 `@crates/originweave-core/src/contracts.rs` around lines 214 - 235, Implement deterministic Display and std::error::Error for OriginError, ActionIntentDigestError, and ExtensionIdError in crates/originweave-core/src/contracts.rs#L214-L235. Update loopback_origin and the expect call at crates/originweave-core/tests/browser_authority_registry.rs#L10-L12 and `#L141-L142` to propagate errors with ?, returning Result<Origin, Box<dyn Error>>; then remove the now-unneeded clippy expect allowance at crates/originweave-core/tests/browser_authority_registry.rs#L1-L1.Source: Coding guidelines
🤖 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 `@crates/originweave-core/tests/browser_authority_registry.rs`:
- Around line 152-169: Update
external_identifiers_are_bounded_without_assuming_protocol_syntax to replace the
hardcoded 513-byte boundary with the publicly re-exported
MAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES constant, preserving the existing
invalid-boundary assertion and all other test behavior.
---
Nitpick comments:
In `@crates/originweave-core/src/browser_registry_coverage.rs`:
- Around line 1-60: Move the tests
repeated_node_binding_exercises_the_unit_crate_existing_node_path and
session_authority_failures_are_exercised_in_the_unit_crate into
browser_registry.rs’s existing mod tests, reusing its values helper. Remove this
standalone module and its lib.rs wiring, while preserving the current assertions
and coverage.
In `@crates/originweave-core/src/browser_registry.rs`:
- Around line 18-30: Extend BrowserAuthorityRegistry with public remove_context
and remove_session methods. Remove a context from context_by_external,
context_session, context_epoch, context_origin, and all node_by_external entries
for that context; remove_session must also remove its external mapping and every
owned context and node binding. Keep next_session_id, next_context_id, and
next_node_id strictly monotonically increasing without reusing identifiers.
- Around line 230-232: Update the Self::InvalidExternalIdentifier formatting
branch to derive the byte limit from MAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES
instead of hardcoding 512 in the message, while preserving the existing output
text and test expectations.
In `@crates/originweave-core/src/contracts.rs`:
- Around line 214-235: Implement deterministic Display and std::error::Error for
OriginError, ActionIntentDigestError, and ExtensionIdError in
crates/originweave-core/src/contracts.rs#L214-L235. Update loopback_origin and
the expect call at
crates/originweave-core/tests/browser_authority_registry.rs#L10-L12 and
`#L141-L142` to propagate errors with ?, returning Result<Origin, Box<dyn Error>>;
then remove the now-unneeded clippy expect allowance at
crates/originweave-core/tests/browser_authority_registry.rs#L1-L1.
In `@crates/originweave-core/src/lib.rs`:
- Around line 15-18: Update the contracts re-export in lib.rs to replace pub use
contracts::* with an explicit list of the intended public contracts symbols,
matching the existing explicit browser_registry export style. Include the
currently supported public items and ensure future additions to contracts.rs do
not become crate exports automatically.
🪄 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: be8aeee8-bcf6-4ae9-b0f5-fed109254c60
📒 Files selected for processing (6)
.github/workflows/ci.ymlcrates/originweave-core/src/browser_registry.rscrates/originweave-core/src/browser_registry_coverage.rscrates/originweave-core/src/contracts.rscrates/originweave-core/src/lib.rscrates/originweave-core/tests/browser_authority_registry.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Buyer-visible gap
Partial implementation of #28. OriginWeave needs a bounded session-scoped registry between raw WebDriver BiDi/CDP/renderer identifiers and durable OriginWeave browser authority so protocol-local strings never become reusable authorization tokens.
Implemented boundary
Current exact head
9e635e80e9813a1d2a9c408155d52221b76eeed3implements the publicBrowserAuthorityRegistry/BrowserRegistryErrorboundary with realistic coverage for:Raw external protocol identifiers remain private registry lookup keys. Callers receive only OriginWeave-owned authority values.
Coverage and review follow-up
The production registry itself reaches exact owned function/line/region/branch coverage. Earlier coverage-only follow-up removed compiler-instrumented synthetic error branches from cfg(test)-only setup code without changing production behavior.
CodeRabbit subsequently identified one maintainability defect in the public integration test: the invalid external-identifier boundary was hard-coded as
513. Exact head9e635e80e9813a1d2a9c408155d52221b76eeed3now derives that boundary from the publicMAX_EXTERNAL_BROWSER_IDENTIFIER_BYTES + 1contract. The addressed inline thread was resolved only after fresh exact-head validation passed.Exact-head evidence
On unchanged exact head
9e635e80e9813a1d2a9c408155d52221b76eeed3against protected main67af7c87589edc2039545af335c95064d9b8391c:31405013082: success, including repository contracts, formatting, locked workspace checks/tests, strict Clippy, rustdoc, and exact owned production coverage;31405013043: success;31405013024: success;31405013010: success;No predecessor-head evidence is transferred to this head.
Scope boundary
This slice adds no browser I/O, Chromium fork, BiDi/CDP transport, unrestricted JavaScript execution, persistence, secret handling, network permission, or model call. It is a production prerequisite for the first real pinned-Chromium observation → policy → typed action → post-condition → evidence path in #28, not completion of that end-to-end runtime.
Governance
The scheduled actor does not merge or self-approve. This exact head is technically gate-clean and Ready for review; any later head movement invalidates this evidence and requires fresh exact-head validation.
Partial implementation of #28.
Summary by CodeRabbit
새 기능
버그 수정
개선