feat(tools): add ToolPolicy for governance and compliance controls#191
feat(tools): add ToolPolicy for governance and compliance controls#191imran-siddique wants to merge 2 commits into
Conversation
|
cc @dhilloulinoracle @cesarebernardis - this implements the Tool Usage Policies proposal you both approved in #174. Fresh implementation PR as discussed. Let me know if you'd like any changes. |
dhilloulinoracle
requested review from
cesarebernardis,
dhilloulinoracle and
paul-cayet
imran-siddique
force-pushed
the
feat/tool-usage-policies
branch
from
98a4679 to
85d2ff6
Compare
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
oracle-contributor-agreement
Bot
added
the
OCA Required
imran-siddique
force-pushed
the
feat/tool-usage-policies
branch
4 times, most recently
from
2db22ce to
3729d90
Compare
Implement the Tool Usage Policies proposal (PR oracle#174) as accepted by Oracle maintainers. This adds: - ToolPolicy model with data_classification, requires_justification, allowed_callers, and guards fields - ExecutionGuard discriminated union: RateLimitGuard, ApprovalGuard, JustificationGuard - PolicyViolation tracing event for audit trails - tool_policy field on both Tool and ToolBox base classes - Version gating to agentspec_version >= 26.2.0 - Composition semantics documented (stricter classification wins, guards union, allowed_callers intersection) Signed-off-by: Imran Siddique <imran.siddique@microsoft.com>
imran-siddique
force-pushed
the
feat/tool-usage-policies
branch
from
3729d90 to
5964ac2
Compare
|
@sonleoracle I signed the OCA about 24 hours ago at oca.opensource.oracle.com with GitHub username \imran-siddique. The check is still showing as not approved. Could you help verify the OCA status or point me to someone who can expedite the approval? Happy to re-sign if something went wrong on my end. |
| """ | ||
|
|
||
|
|
||
| class ToolPolicy(BaseModel): |
There was a problem hiding this comment.
The ToolPolicy needs to extend Component, so that it automatically uses the reference system
|
|
||
| model_config = ConfigDict(extra="forbid") | ||
|
|
||
| data_classification: Optional[DataClassificationT] = None |
There was a problem hiding this comment.
From the design this was supposed to be a (non-optional) string. Is there a specific reason why null was allowed as a value?
| requires_justification: bool = False | ||
| """Whether a textual justification must be provided before invocation.""" | ||
|
|
||
| allowed_callers: Optional[List[str]] = None |
There was a problem hiding this comment.
In the design, this was supposed to be a non-optional array of components, while here we are asking for a list of IDs.
- I am ok with allowing
Noneif we say thatNonemeans "no restriction", while empty list means "no allowed caller" (a bit weird as a constraint, as it would probably be unusable in that case, but it would have a precise meaning) - I think I am slightly more in favor of having a list of components instead of a list of string IDs, as I think it would align better with the overall language (we usually don't reference IDs directly). But I don't have a strong opinion.
|
|
||
| Guards are checked in declaration order. The first guard whose condition | ||
| is met triggers its ``on_violation`` action. | ||
| """ |
There was a problem hiding this comment.
By design there should be also a denied_callers parameter
| return self | ||
|
|
||
|
|
||
| class JustificationGuard(BaseModel): |
There was a problem hiding this comment.
I think that these guard implementations are a bit different from the design: some fields are missing, some values for the fields are missing, ... Could you please double check and align design and implementation? Or if it's intended, maybe you can just justify the choices.
| caller_id: Optional[str] = None | ||
| """Identifier of the agent or component that attempted the invocation.""" | ||
|
|
||
| inputs: Optional[Dict[str, Any]] = None |
There was a problem hiding this comment.
We have a SensitiveField annotation that I think would fit this inputs field. We use it to tell the span processors whether a field might contain PII or other sensitive information/data that should be masked.
|
Hi @imran-siddique , thank you for your work! I did a first round of review and left some comments. I noticed that the changes to the language specification are missing. To give you a brief explanation, when we introduce new concepts in Agent Spec, we modify the latest language specification file with the new modifications. You can reuse most of the content you prepared for the design, maybe adjusting a bit structure and phrasing to fit the rest of the specification, and adding some details to make it self contained. |
2 participants
Summary
Implements the Tool Usage Policies proposal (formerly PR #174) as accepted by @dhilloulinoracle and @cesarebernardis.
This adds governance and compliance controls to the Agent Spec tool model, enabling declarative policy enforcement for tool invocations.
Changes
Design Decisions
Testing
All 16 new tests pass. Existing tool serialization tests unaffected (13/14 pass; 1 pre-existing crewai sandbox failure unrelated to this change).