feat(policy): pluggable PolicyProvider subsystem with mutation gate#1668
Closed
dvavili wants to merge 2 commits into
Closed
feat(policy): pluggable PolicyProvider subsystem with mutation gate#1668dvavili wants to merge 2 commits into
dvavili wants to merge 2 commits into
Conversation
Promote policy delivery to a pluggable subsystem in the gateway. The
gRPC handlers route sandbox-scoped set, merge-ops, and global delete
through a registered PolicyProvider; the existing in-tree store is
exposed as LocalPolicyProvider, preserving today's behavior.
- PolicyProvider trait: id, get_effective_policy, plus three mutators
(set_policy, update_policy, delete_policy) with default impls
returning PolicyError::Unsupported { policy_type, operation }.
- LocalPolicyProvider wraps Store. set_policy fully delegates;
update_policy and delete_policy gate the operation through the
trait while the handler retains the merge-with-retry loop and the
global-settings-map mutation respectively (asymmetry documented in
the trait module doc).
- [openshell.policy] type = "local" | "attested" added to the config
file, aligning with the existing `type`-selector convention used by
ProviderPlugin. "attested" is accepted as a known-but-not-yet-
available value (clear startup error rather than panic or unknown-
type error).
- PolicyError::Unsupported maps to tonic::Status::unimplemented, so
any provider that does not override the three mutators (e.g. the
forthcoming AttestedPolicyProvider) refuses set/update/delete
without per-handler conditionals.
Covered by 12 new unit tests (policy_provider trait defaults + Local
flows over a fresh sqlite store) and 4 new handler-level integration
tests (Local set succeeds; refusing provider yields unimplemented on
set, merge-ops, and global delete).
The agentic approval loop introduced in NVIDIA#1528 added seven chunk-mutation RPCs (submit_policy_analysis, approve/reject/approve_all/edit/undo/clear _draft_chunk) that mutate policy state through the draft-chunk store without going through the three set/update/delete mutators. The PolicyProvider trait gated only the latter, leaving the chunk surface as a bypass for any read-only driver. Add `permits_mutation()` to the trait with a default `Unsupported` return; override in LocalPolicyProvider to `Ok(())`. Thread the gate through every mutation entry point in grpc/policy.rs as the first post-authz action, so a refusing provider fails fast without touching the store: - handle_submit_policy_analysis - handle_approve_draft_chunk - handle_reject_draft_chunk - handle_approve_all_draft_chunks - handle_edit_draft_chunk - handle_undo_draft_chunk - handle_clear_draft_chunks - handle_update_config sandbox-set, sandbox-merge, global-delete arms - handle_update_config global-set inline arm (separate from the per-op trait path) The per-op set_policy/update_policy/delete_policy provider calls remain in place as the natural seam for a future driver that wants to override one mutation without refusing all of them; permits_mutation is the coarse gate that runs first. Tests: 7 new chunk-handler refusing_provider integration tests (each asserts the draft-chunk store is unchanged after the refused call), plus permits_mutation_is_ok_for_local_provider and stub_provider_permits_mutation_returns_unsupported. 3 pre-existing refusing_provider tests updated to expect `mutation` instead of the per-op operation name. handle_report_policy_status is intentionally not gated — it is the sandbox-side load-result pong, not a control-plane mutation; gating would break legitimate sandbox status reports under attested mode.
dvavili
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
|
Thank you for your interest in contributing to OpenShell, @dvavili. This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer. To get vouched:
See CONTRIBUTING.md for details. |
|
Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text: I have read the DCO document and I hereby sign the DCO. You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot. |
3 tasks
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.
Summary
Promote policy delivery and mutation acceptance from inline
Storecalls in the gRPC layer to aPolicyProvidertrait. Lets an alternate provider refuse policy mutations while still serving an authoritative effective policy at admission time.PolicyProvidertrait:id(),get_effective_policy(),permits_mutation(), and three mutators (set/update/delete_policy) — mutators have defaultUnsupportedimpls.LocalPolicyProviderwraps the existingStore; current behavior unchanged.[openshell.policy] type = "local" | "attested"config selector, following theProviderPlugin-styletypeconvention.attestedis reserved (returns a clear startup error until its impl lands in a follow-up).permits_mutation()is the coarse gate threaded through every mutation entry point ingrpc/policy.rs: the three RPC mutators plus the seven chunk-approval handlers introduced by feat(policy): add agentic approval loop #1528 (submit_policy_analysis,approve/reject/approve_all/edit/undo/clear_draft_chunks). A refusing provider yieldsStatus::unimplementedwithout touching the store.PolicyError::Unsupportedmaps totonic::Status::unimplementedat the gRPC edge — theopenshell policy {set,update,delete}rejection falls out of the trait default.Standalone-useful, and the seam an attested-policy follow-up plugs into.
Test plan
cargo test -p openshell-server --lib -- --test-threads=1— 734 passingUnsupported,LocalPolicyProvidersemantics, refusing-provider integration through every gated handler with before/after store-snapshot checksopenshell policy {set,update,delete}continues to work viaLocalPolicyProvider