Introducing Azure ACA sandbox provider in AGT#2236
Conversation
github-actions
Bot
added
documentation
🤖 AI Agent: security-scanner — View detailsNo security issues found. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
🤖 AI Agent: docs-sync-checker — Docs SyncDocs Sync
|
🤖 AI Agent: code-reviewer — Action Items:TL;DR: 0 blockers, 1 warning. Solid implementation with minor follow-up needed.
Action Items:
Warnings:
|
🤖 AI Agent: test-generator — `agent-governance-python/agent-os/src/agent_os/policies/schema.py`
|
🤖 AI Agent: breaking-change-detector — API CompatibilityAPI Compatibility
|
PR Review Summary
Verdict: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
amolr
changed the title
amolr
force-pushed
the
dev/amolr/azuresandbox
branch
from
bc5d497 to
bcfd103
Compare
|
Hey @amolr awesome work, the design doc and the quickstart are genuinely great 🙌 The two main areas I'd love the proposal to firm up before we implement:
|
Yes, we assume the clients use az login and provide their subscription Id to create the required resources |
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
TL;DR: 3 blockers, request changes.
| # | Sev | Issue | Where |
|---|---|---|---|
| 1 | Blocker | Local filesystem path Q:\ADC\python\python\azure-sandbox leaked into public docs. Must use PyPI package name only. |
docs/proposals/azure-aca-sandbox.md |
| 2 | Blocker | Internal names still present: management.azuredevcompute.io, host=adc-sandbox. These are internal Azure infra names that should not appear in a public repo. Replace with ACA-branded equivalents or redact. |
docs/proposals/azure-aca-sandbox.md |
| 3 | Blocker | PR checklist is completely unchecked (type of change, packages affected, CLA, AI assistance, IP). Fill these out before merge. | PR body |
| 4 | Warn | 10 CI checks failing (DCO, spell check, secret scanning, etc.). Verify these are not blocking. | CI |
| 5 | Warn | Docs-sync bot flagged: README sandbox providers section needs ACA mention, CHANGELOG entry missing. | Follow-up OK |
Action items:
- Remove the
Q:\ADC\...local paths from the proposal doc, replace withazure-sandbox/azure-mgmt-sandbox(just the package names). - Scrub
azuredevcompute.ioandadc-sandboxreferences from all docs. Usecontainerapps.azure.comper the team's agreed branding. - Fill out the PR checklist checkboxes.
Code quality is solid: fail-closed defaults, input validation, thread-safe state, good test coverage (786-line unit test file + integration tests). Schema changes are backward-compatible with sensible defaults. No secrets or credential leaks detected.
There was a problem hiding this comment.
Hey @amolr — took a proper look through the full diff, design doc, and the quickstart. Really nice work here.
miyannishar
left a comment
miyannishar
left a comment
There was a problem hiding this comment.
One minor nit on the __init__.py exports — see inline. Otherwise this looks great.
| ) | ||
|
|
||
| __all__ = [ | ||
| "ACASandboxProvider", |
There was a problem hiding this comment.
nit: exporting underscore-prefixed helpers (_network_allowlist, _network_default, _validate_resource_name) in __all__ sends mixed signals — underscore says "internal", __all__ says "public API".
If they're genuinely useful for downstream consumers (e.g. someone building a custom sandbox provider), consider dropping the underscore. Otherwise, keep them out of __all__ so you can refactor freely later.
|
Went through the full diff — implementation, tests, design doc, quickstart. This is solid work @amolr. Fail-closed egress default, base64-pipe execution, and the test coverage (especially One nit: |
Signed-off-by: Amol Ravande <[email protected]>
Signed-off-by: Amol Ravande <[email protected]>
Signed-off-by: Amol Ravande <[email protected]>
…nal helpers - Replace management.azuredevcompute.io with management.containerapps.azure.com in proposal docs. - Remove Q:\ADC\python\python\... local filesystem paths from azure-aca-sandbox.md; reference PyPI package names directly. - Rename adc-sandbox host marker to aca-sandbox in azure_sandbox_step5_test.py docstring. - Drop _network_allowlist, _network_default, and _validate_resource_name from agent_sandbox.aca_sandbox_provider package __all__; tests now import them directly from the implementation module to keep them as internal helpers. Signed-off-by: Amol Ravande <[email protected]>
amolr
force-pushed
the
dev/amolr/azuresandbox
branch
from
4605bda to
69fadcd
Compare
…lation design - Remove the now-unused entry from the cloud-providers comparison diagram. - Fold the ephemeral-container bullet into the existing ACA bullet, preserving the sub-second cold-start point without naming the internal service. Signed-off-by: Amol Ravande <[email protected]>
amolr
force-pushed
the
dev/amolr/azuresandbox
branch
from
3fc9319 to
04e6835
Compare
Required by scripts/ci/security-audit-required.sh because this PR touches agent-governance-python/agent-os/src/agent_os/policies/schema.py. Covers: - What changed (new sandbox-provider extension fields on PolicyDocument / PolicyDefaults: max_cpu, max_memory_mb, timeout_seconds, network_default, network_allowlist, tool_allowlist). - Threat model impact (additive, fail-closed: network_default defaults to "deny"; no existing check weakened; rule-engine evaluation byte-identical). - Test coverage (test_policy_sandbox_fields.py, test_azure_sandbox*.py, updated test_docker_sandbox.py). Signed-off-by: Amol Ravande <[email protected]>
Spell-check workflow flagged terms from sandbox tests, schema, and the new security audit doc. All are legitimate technical terms: - Python stdlib idioms: aenter, aexit, alnum, argparse, asctime, caplog, dataclasses, ensurepip, getattr, gethostname, getuid, gmtime, hasattr, kwargs, levelname, monkeypatch, noqa, popen, pytestmark, pythonhosted, setattr, setitem, skipif, splitlines, startswith, staticmethod, strftime, subpro, textwrap, urllib - Azure / sandbox terminology: containerapps, egresspolicy, mgmt, millicores, sandboxprovider, stepreceipt, westus - Security / governance: SIEM, SLSA, repudiable, lockdown, unconfigured, toolkits - Test fixtures: evilpypi, héllo (unicode print-test snippet) - Project shorthand: AGTS Signed-off-by: Amol Ravande <[email protected]>
5 participants
This pull request introduces and tests new sandbox-related policy fields in the
PolicyDocumentschema, enhances documentation and initialization for the Azure Container Apps (ACA) sandbox provider, and adds a comprehensive test for the StepReceipt-to-MerkleAuditChain adapter. The most important changes are grouped below:Policy Schema Extensions and Tests:
PolicyDefaults(max_cpu,max_memory_mb,timeout_seconds, andnetwork_default) and new allowlist fields (network_allowlist,tool_allowlist) toPolicyDocument. These fields are consumed by sandbox providers for resource and network/tool access control, but ignored by the rule engine. ([[1]](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-78b670c488d20b239360fefc85ce139209ff60b5e57fad532f8ac5620467579fL66-R100),[[2]](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-78b670c488d20b239360fefc85ce139209ff60b5e57fad532f8ac5620467579fR119-R138))test_policy_sandbox_fields.pyto pin the contract for these new fields, ensuring correct defaults, allowed values, and YAML round-trip compatibility. ([agent-governance-python/agent-os/tests/test_policy_sandbox_fields.pyR1-R118](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-00666e6fe40dbdce8395620bd859731a2d7f64e93621ba279f2a1ec9836ca8e6R1-R118))ACA Sandbox Provider Integration:
agent_sandbox/__init__.py, and added a lazy import forACASandboxProviderto handle optional dependencies. ([[1]](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-2a35f8bda57f0e7f2924f1fc2cd5536bccaedb2c8c55957c421d3efcbb963bffL6-R6),[[2]](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-2a35f8bda57f0e7f2924f1fc2cd5536bccaedb2c8c55957c421d3efcbb963bffR15-R17),[[3]](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-2a35f8bda57f0e7f2924f1fc2cd5536bccaedb2c8c55957c421d3efcbb963bffR61-R75))__init__.pyto theaca_sandbox_providerpackage to re-export the provider and helper functions for convenient imports. ([agent-governance-python/agent-sandbox/src/agent_sandbox/aca_sandbox_provider/__init__.pyR1-R25](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-5f2f4f34a9972d7a14400f2a6940e62b91c8b41ac2e50f69df0fb24834eed6b6R1-R25))StepReceipt to MerkleAuditChain Testing:
test_step_receipt_to_audit_chain.pyto verify the adapter logic and ensure documentation snippets remain in sync with the actualAuditEntryandMerkleAuditChainschema. This includes tests for hash chaining, field preservation, tampering detection, and round-tripping of egress decisions. ([agent-governance-python/agent-mesh/tests/test_step_receipt_to_audit_chain.pyR1-R164](https://github.com/microsoft/agent-governance-toolkit/pull/2236/files#diff-91f5a4f47b1c117f350968e278d01295f9977657d2294dba016a4b33c42fc880R1-R164))This pull request introduces and tests new schema fields in the policy system to support sandbox resource constraints and egress/network controls, and adds Azure ACA sandbox support to the agent sandbox package. The key changes are grouped below by theme:Policy Schema: Sandbox Resource and Network Controls
max_cpu,max_memory_mb,timeout_seconds, andnetwork_default) to thePolicyDefaultsclass inpolicy/schema.py. These fields are intended for sandbox providers (such as Azure, Docker, Hyperlight) and not for the rule engine itself. Thenetwork_defaultfield is a required Literal with allowed values"allow"or"deny", defaulting to"deny"for fail-closed behavior.network_allowlistandtool_allowlisttoPolicyDocumentfor specifying allowed network hosts and tools, respectively. These are consumed by sandbox providers and default to empty lists.Testing: Policy Schema Contract
test_policy_sandbox_fields.pyto verify the new schema fields, including their defaults, allowed values, YAML round-trip, and backward compatibility with older YAML files.Agent Sandbox: Azure Provider Integration
agent_sandbox/__init__.pyto document and lazily import the newAzureSandboxProvider, making it available as a built-in sandbox backend alongside Docker and Hyperlight. [1] [2] [3]agent_sandbox/azureadc_sandbox_providerwith an__init__.pythat re-exports the main provider class and helpers, supportingfrom agent_sandbox.azureadc_sandbox_provider import AzureSandboxProvider.Documentation and Testing: Audit Chain Adapter
test_step_receipt_to_audit_chain.py) to ensure the documentation'sStepReceipttoMerkleAuditChainadapter remains in sync with the actual schema, and to pin the contract for audit entries generated from sandbox steps.## DescriptionType of Change
Package(s) Affected
Checklist
Attribution & Prior Art
Prior art / related projects (if any):
AI Assistance
If AI tools materially shaped this change, briefly note what was used:
IP, Patents, and Licensing
Related Issues