feat(sdk): out of box controls phase 2#247
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
namrataghadi-galileo
changed the base branch from
main
to
feature/67101-out-of-box-controls
| r"\b(?:rm\s+-rf\s+(?:/|~|\$HOME)|sudo\s+rm\s+-rf|" | ||
| r"mkfs(?:\.[a-z0-9]+)?|dd\s+if=[^\s]+\s+of=/dev/[^\s]+|" | ||
| r"chmod\s+-R\s+777\s+/|chown\s+-R\s+[^|;&]*\s+/|" | ||
| r"shutdown\s+(?:-h\s+)?now|reboot)\b" |
There was a problem hiding this comment.
The shared trailing \b cannot match after / or ~, so rm -rf /, rm -rf ~, chmod -R 777 /, and chown -R root / pass this control. Please use explicit end/separator handling per alternative and add these cases to the test.
| "anyOf": [ | ||
| { | ||
| "required": ["approved"], | ||
| "properties": {"approved": {"const": True}}, |
There was a problem hiding this comment.
Could we avoid treating a caller-controlled approved flag as authorization evidence? A prompt-injected agent can set or replay it, while tools without this field cannot complete the flow. This should use a trusted approval artifact or host workflow bound to the action; the outbound template below has the same issue.
| name="oob-sensitive-tool-requires-approved-role", | ||
| data=_leaf_control_payload( | ||
| description="Deny sensitive tool use when runtime context has an unapproved role.", | ||
| selector_path="context.user.role", |
There was a problem hiding this comment.
This fails open when context.user.role is missing: the selector returns None, and ListEvaluator returns matched=False before applying match_on="no_match". Please fail closed for missing/unknown roles and source the role from trusted principal context.
| selector_path="name", | ||
| evaluator_name="list", | ||
| evaluator_config={ | ||
| "values": ["search", "web_search", "retrieve", "calculator"], |
There was a problem hiding this comment.
Exact bare names do not match integrations that qualify tool names. Google ADK emits names such as <agent>.web_search, so this denies an explicitly approved tool. Please normalize tool identity across integrations or match a canonical raw name, with an integration-level test.
| try: | ||
| if namespace_key == default_out_of_box_namespace_key(): | ||
| return | ||
| if await _namespace_has_active_controls(db, namespace_key=namespace_key): |
There was a problem hiding this comment.
Checking for any active control makes partial seeding permanent: templates commit separately, so one transient failure leaves rows that cause every later request to return here. It also skips tenants with one custom control. Please check expected seed identities or always run the idempotent per-template pass.
| return | ||
| if await _namespace_has_active_controls(db, namespace_key=namespace_key): | ||
| return | ||
| await seed_out_of_box_controls( |
There was a problem hiding this comment.
This path is reached from a GET authorized only with controls.read, but it commits Control and ControlVersion rows. Could seeding move to trusted tenant provisioning, or require controls.create resolved to the same namespace, so read-only callers cannot trigger writes?
2 participants
Summary
Scope
OUT_OF_BOX_CONTROL_TEMPLATESand expanded bootstrap/evaluator behavior tests.Risk and Rollout
server/src/agent_control_server/bootstrap/out_of_box_controls.py, or remove specific templates fromOUT_OF_BOX_CONTROL_TEMPLATES.Testing
make check(not run becauseuvdependency resolution hit private index/auth issues previously).venvChecklist