Repository navigation
feat(core): bind enterprise approvals to immutable intent - #220
seonghobae wants to merge 70 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughopaque principal 검증, 불변 승인 범위, maker-checker 승인 수명주기를 추가했다. 승인 전환에 단조 시간과 만료 처리를 적용했다. 소비 결과를 비복제 일회성 정책 평가 사용으로 변경하고 공개 API, 테스트 및 ADR을 추가했다. Changes엔터프라이즈 승인
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🔵 Low · up to The approval lifecycle and privacy-ordering changes are supported by passing repository checks and complete production coverage. Merge is reasonable with owner follow-up for the ADR authority-boundary wording and for tightening clock-rollback coverage to assert the required error type. Sequence Diagram(s)sequenceDiagram
participant Requester
participant EnterpriseApprovalRequest
participant EnterpriseApprovalUse
participant PolicyEvaluator
Requester->>EnterpriseApprovalRequest: 정확한 ApprovalScope로 consume
EnterpriseApprovalRequest-->>Requester: 비복제 EnterpriseApprovalUse 반환
Requester->>EnterpriseApprovalRequest: checker가 revoke 수행
EnterpriseApprovalRequest->>EnterpriseApprovalUse: 공유 revocation signal 설정
Requester->>EnterpriseApprovalUse: evaluate_at 호출
EnterpriseApprovalUse->>EnterpriseApprovalUse: 시간과 철회 신호 검증
EnterpriseApprovalUse->>PolicyEvaluator: private PolicyContext로 승인 평가
PolicyEvaluator-->>Requester: Decision 반환
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 5 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
|
Current-head governance audit:
|
|
Exact-head blocker audit for
|
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
CHANGELOG.md— repository behaviorcrates/originweave-policy/src/enterprise_approval.rs— Rust workspace crate API and testscrates/originweave-policy/src/lib.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_clone_contract.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_lifecycle.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_principal_invisible_format.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_scope_privacy.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_single_use.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_time_integrity.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_transition_guards.rs— Rust workspace crate API and testscrates/originweave-policy/tests/enterprise_approval_use_scope_binding.rs— Rust workspace crate API and testsdocs/README.md— operator or user guidancedocs/adr/0017-enterprise-maker-checker-approval.md— operator or user guidancedocs/adr/README.md— operator or user guidancedocs/doctoring.md— operator or user guidance
Changed behavior
classDiagram
class ApprovalPrincipalRef
class new
class issuer
class subject
class ApprovalPrincipalRefError
class ApprovalLifecycleState
class EnterpriseApprovalUse
class evaluate_at
Changed API
ApprovalPrincipalRefnewissuersubjectApprovalPrincipalRefErrorApprovalLifecycleStateEnterpriseApprovalUseevaluate_atEnterpriseApprovalRequestapprovedenywithdrawconsumerevokeApprovalLifecycleErrorDecisionDenialReasonevaluate_mcpevaluate
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
b11db2be68f9b6d71aa4c4290b97a8b22097b353 - Workflow run: 33190553132
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
classDiagram
class ApprovalPrincipalRef
class new
class issuer
class subject
class ApprovalPrincipalRefError
class ApprovalLifecycleState
class EnterpriseApprovalUse
class evaluate_at
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Integrated protected main at |
…0-v2 Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Adopted current protected |
|
Fresh exact-head review for |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head focused security review for e545b94e1de499b96b867694f80ac04ad247becd: no actionable authority-widening defect found. The authorizing path is bound to the exact action/origin/intent digest, distinct exact issuer+subject principals, monotonic trusted time, bounded consumption, a non-cloneable one-shot use, and shared expiry/revocation invalidation before ordinary fail-closed policy evaluation. Local exact-head evidence: rustfmt passed; all 72 originweave-policy tests passed; strict Clippy and warnings-denied rustdoc passed; and all 12 focused repository-contract tests passed. Hosted exact-head state remains non-terminal (9 queued checks), and no counted independent approval exists, so this review does not authorize merge.
|
Admission-state correction for exact head protected This PR is being moved from Ready to Draft/Proposed. It remains open and its commits, reviews, threads, and valid delta are preserved. Return it to Ready only after the causal blocker is repaired and the unchanged/reconciled exact head has fresh terminal Checks, zero substantive unresolved findings, and any required current-head/latest-push independent approval. No bypass, synthetic status, manual rerun, force push, review dismissal, or Close is used. |
Partial implementation of #202. This PR owns the bounded in-memory enterprise maker-checker approval lifecycle and one-shot policy-use boundary.
Authority boundary
EnterpriseApprovalRequestbinds an exact immutableApprovalScope, opaque already-authenticated principal references, a trusted validity window, monotonic transition time, and bounded use count. The policy crate enforces exact(issuer, subject)requester/checker tuple separation, lifecycle/time invariants, expiry, use accounting, one-shot evaluation, and process-local outstanding-use terminal invalidation. A successfulconsumereturns a non-cloneableEnterpriseApprovalUse;evaluate_atconsumes it, rechecks live action/origin/intent scope and trusted time, observes process-local revocation/expiry invalidation, and only then introduces approval evidence into ordinary policy evaluation. R5 legal consent remains non-delegable.This boundary does not authenticate principals, prove real-person separation, establish checker role, resolve tenant membership, persist consumption/revocation, provide distributed cancellation/consensus, or grant business authorization. Those remain identity/workflow/durable-control-plane responsibilities under #202.
Current protected-main adoption
Protected
mainis exact87c4daa1830bac5a5228b6036752ad5633232085through #286. Current exact head ise545b94e1de499b96b867694f80ac04ad247becd, a normal non-force merge adoption of that protected generation. Fresh compare is 70 ahead / 0 behind, merge base exactly protected main, and the effective current-main delta is 16 policy/product/docs/test paths with no.github/**delta. GitHub reports the PR open, Ready and mergeable.The protected workflow changes visible inside the topology merge are inherited from protected main; they are not effective PR workflow changes and were not authored as this product delta.
Exact-current evidence
The previous body was stale: it named predecessor
22b484cc..., described the PR as Draft, and retained old queued workflow states. Exact currente545b94e1de499b96b867694f80ac04ad247becdhas terminal evidence:33929051692: success;33929051660: success;33929051651: failure; and33929051661: skipped on the Ready transition.The skipped native CI is not GREEN and cannot validate repository contracts, Rust gates, rustdoc or exact production coverage for this head. This reproduces the #279 protected Ready-transition control-plane gap; the failed central CodeQL verdict remains independently fail-closed. Do not rerun blindly, weaken Draft/closed guards, or substitute predecessor evidence.
Keep Ready but unmerged until an exact-head native execution actually runs and succeeds and the required central review/security/ruleset verdicts are satisfied.
Durability boundary
The terminal signal coordinates only one live in-process request and issued uses. Once evaluation has passed the validity check it is in flight. Crash-safe/distributed replay prevention, canonical actor correlation, tenant identity, signed/auditable evidence, operator workflow, and durable enterprise control-plane semantics remain open #202 work.
Protected-main
AGENTS.mdand live GitHub governance remain authoritative. This scheduled writer does not merge, self-approve, bypass, force-push, destructively rebase, alter workflows/rulesets/secrets, weaken checks, tag, release, or publish.