feat: govern accumulated context across delegated agent workflows#2800
feat: govern accumulated context across delegated agent workflows#2800Knapp-Kevin wants to merge 2 commits into
Conversation
|
🔴 Contributor Check: HIGH
Automated check by AGT Contributor Check. |
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
98983ff to
db8e1bd
Compare
db8e1bd to
4ac2f13
Compare
imran-siddique
left a comment
There was a problem hiding this comment.
The lattice semantics are correct, fail-closed logic is properly guarded (including the vacuous-empty-ObligationSet case), and validate() regression coverage is solid. A few issues before this merges:\n\nBlocker: is misnamed. The function computes a union (parent | child_declared), not an intersection. Union is the right behavior for grow-only inheritance, but the name will cause real confusion when someone audits restriction handling under time pressure. Rename to or .\n\nBlocker: commit messages contain a vendor plug. Both commits end with "Prepared with Qor-logic agentic gated-SDLC governance: https://github.com/MythologIQ-Labs-LLC/Qor-logic". That is promotional content in AGT's git history. Please squash/reword before merge to remove it.\n\nStrong recommendation: the security audit doc is self-authored. The "verified by" column only points to tests in the same PR. That's not a security audit — it's a test summary. Either remove it, or get a second committer to sign off on it before it lands in .\n\nMinor nit: passed as a bare with no default across all call sites is easy to mis-order. Consider a named constant or a field on .
c5ae117 to
de2e7d4
Compare
|
Thanks! All three addressed:
Re-requesting review. |
|
The Knapp-Kevin comment notes that |
imran-siddique
left a comment
There was a problem hiding this comment.
Re-reviewed after de2e7d4. The attribution and security-doc items are resolved: the vendor plug is gone from the commit, and the doc is retitled "Security Design Notes" with a banner stating it is a self-review, not an independent audit. Good.
The primary blocker is still open, though. The commit message says intersect_restrictions was renamed to merge_restrictions, but the diff still defines and calls intersect_restrictions everywhere (the export in __all__, the def intersect_restrictions(...), and every call site and test). So the code and the commit message now contradict each other, which is more confusing than before. Please either:
- actually apply the rename to
merge_restrictions(orunion_restrictions) across the definition,__all__, all call sites, and the tests, or - if you have decided to keep the name, drop the rename claim from the commit message and add a short comment on the function explaining it computes a grow-only union despite the
intersectname.
Separately, the PR is now CONFLICTING with main after the recent batch of merges. Please rebase onto main so CI can run cleanly.
The lattice semantics and fail-closed logic remain correct; this is just the naming consistency and a rebase.
…kflows (v1) Adds a ContextEnvelope that accumulates sensitivity and restriction state across a workflow and gates subsequent actions and delegations against it, closing the gap where individually permitted actions aggregate into sensitive context and where a delegated agent can quietly lose the constraints its parent carried. - ContextEnvelope: immutable, versioned; max-lattice sensitivity, grow-only restrictions - Aggregation evaluation over label combinations with a monotone escalation backstop - Post-execution accumulation of result_labels; constrain = allow + obligations, fails closed on the declarative policy path - Grow-only restriction inheritance on delegation: merge_restrictions plus the additive DelegationChain.effective_restrictions, a union of parent and child-declared restrictions (renamed from intersect_restrictions, which misdescribed the union); validate() unchanged - CONTEXT_* audit events, classified at the envelope sensitivity - Opaque ContextEnvelope cross-boundary reference (envelope_reference / EnvelopeReference): carries only an id and a coarse sensitivity tier, never envelope contents, so the in-process shape can evolve independently Reuses the existing DataClassification/DataLabel/ABACPolicy types and the policy-engine result_labels. No new dependencies. Security design notes (a self-review by the author, not an independent audit) are in docs/security/context-accumulation-governance.md. Refs microsoft#2797, microsoft#2798. AI-assisted disclosure. Portions of this contribution were generated under human direction through an agentic, gated SDLC governance process. The governance controls and decision records that shaped this code are maintained in their open-source project and can be furnished in fuller detail on request. Signed-off-by: Knapp-Kevin <krknapp@gmail.com>
Addresses maintainer review on microsoft#2800. The function computes a grow-only union of parent and child-declared restrictions, not an intersection, so the previous name was misleading during restriction-handling audits. Applies the rename consistently across the definition, the policies __all__ export, every call site, and the tests, so the code and the prior commit message no longer contradict each other. Updates the security design notes to reference merge_restrictions. No behavior change; lattice semantics and fail-closed logic are unchanged. Refs microsoft#2797, microsoft#2798. AI-assisted disclosure. Portions of this contribution were generated under human direction through an agentic, gated SDLC governance process. The governance controls and decision records that shaped this code are maintained in their open-source project and can be furnished in fuller detail on request. Signed-off-by: Knapp-Kevin <krknapp@gmail.com>
de2e7d4 to
54d7192
Compare
|
The The design work here looks solid -- will do a full pass once #3006 is merged and this rebases clean. |
imran-siddique
left a comment
There was a problem hiding this comment.
Both blockers are now resolved:
intersect_restrictionsis fully renamed tomerge_restrictionsacross the definition,__all__, all call sites, and tests.- The doc is correctly titled 'Security Design Notes' with the self-review banner.
The PR is still CONFLICTING with main after the recent batch of merges. Please rebase onto main so CI runs clean -- then this is ready to merge.
|
Approved from my side. The PR is still showing as CONFLICTING -- please rebase onto main to resolve the conflict and this can land. |
|
Ping @Knapp-Kevin — this PR is approved and just needs a rebase onto current main to clear the merge conflict. Once you rebase and push, it should land automatically. |
|
This is approved and just needs a rebase onto main — the branch has a conflict that can't be auto-resolved. Please rebase and force-push and we'll merge right away. |
|
Second ping — this has been approved since our last review and is ready to merge the moment it's rebased. The branch still has a conflict that can't be auto-resolved. Please rebase onto main and force-push; we'll merge immediately. |
What this does
Adds a first cut of context accumulation governance: a ContextEnvelope that tracks the labels and sensitivity a workflow has accumulated and checks later actions and delegations against that running state, rather than evaluating each action in isolation. Background and motivation are in #2797, and there's a fuller design writeup in the discussion at #2798.
Why
A multi-agent workflow can pass every individual policy check and still end up somewhere sensitive, because sensitivity builds up as context accumulates. A customer name plus financial indicators plus behavioral analytics can combine into a sensitive customer risk profile even though each piece was fine on its own. A delegated agent can also quietly lose the constraints its parent was carrying. This closes that gap for the in-process case.
What's in it
merge_restrictionsand an additiveDelegationChain.effective_restrictions, both a union of the parent's restrictions and the child's declared ones (renamed fromintersect_restrictions, which misdescribed a union). A child can add restrictions but never drop a parent's.validate()is untouched.It reuses the existing DataClassification, DataLabel, and ABACPolicy types and the policy-engine result_labels, and adds no new dependencies.
Scope and what's deferred
This is the in-process, single-writer first cut. A few things are deliberately out of scope for now and probably deserve their own issues:
Security design notes
docs/security/context-accumulation-governance.mdcaptures the author's own threat model and design rationale (fail-closed constrain mapping, restriction-gating independent of the sensitivity floor, the grow-only union, and the deferred cross-boundary/lifecycle work). It is a self-review, not an independent security audit — the "Test coverage" column points at tests in this PR — and a second-maintainer review is welcome.Tests
Closes #2797.
AI-assisted disclosure. Portions of this contribution were generated under human direction through an agentic, gated SDLC governance process. The governance controls and decision records that shaped this code are maintained in their open-source project and can be furnished in fuller detail on request.