feat(sandbox): add agt-sandbox AppArmor profile for command-denylist enforcement#3114
feat(sandbox): add agt-sandbox AppArmor profile for command-denylist enforcement#3114qubeena07 wants to merge 5 commits into
Conversation
qubeena07
requested review from
MohammadHaroonAbuomar and
imran-siddique
as code owners
🤖 AI Agent: breaking-change-detector — API Compatibility
API Compatibility
|
🤖 AI Agent: docs-sync-checker — Docs Sync
Docs Sync
|
🤖 AI Agent: test-generator — `agent-governance-python/agent-sandbox/src/agent_sandbox/docker_provider/provider.py`
|
🤖 AI Agent: code-reviewer — Action items:
TL;DR: 0 blockers, 1 warning. The PR introduces a robust AppArmor profile for kernel-level command-denylist enforcement, with proper fallback mechanisms and tests. One improvement is suggested for better test coverage.
Action items:
|
🤖 AI Agent: security-scanner — View details
No security issues found. |
|
🟡 Contributor Check: MEDIUM
Automated check by AGT Contributor Check. |
github-actions
Bot
added
the
needs-review:MEDIUM
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. |
Closes microsoft#3012. The existing CONTEXT_RULES covered first-person social-engineering phrases but had no coverage for indirect / content-channel injection — payloads that arrive inside retrieved content (web pages, emails, RAG documents, tool results) and address the model directly. Four new rules added to CONTEXT_RULES in the `context:` family: - `context:instructions_for_ai_reading` (High / 0.90) — "instructions for the AI/assistant/model/LLM reading/processing/analyzing this" - `context:system_note_to_assistant` (High / 0.90) — "system note:" or "note to the assistant/AI/model/LLM:" impersonating a system channel - `context:embedded_tool_directive` (Medium / 0.75) — HTML-comment- smuggled directive addressed to the AI (<!-- ai: … -->) - `context:retrieved_doc_override` (High / 0.90) — "this document/page/ email/file/result instructs you to ignore/override/reveal/…" All patterns use the Rust `regex` crate (no lookarounds, no backrefs) and match after normalize_for_detection(), consistent with the existing corpus. All four IDs are registered as known built-in rule IDs so they can be disabled via BuiltInRuleOverrides::disable. Integration tests added for each rule: positive matches across wording variants, benign inputs that must not fire, and a round-trip test verifying each ID is accepted by validate_disable_list.
…enforcement (microsoft#3068) Implements option 3 of the command-denylist stack from microsoft#2662. Adds a custom AppArmor profile (docker/apparmor/agt-sandbox) that denies exec of dangerous network, cloud, and infra CLIs at the kernel level — complementing the minimal-PATH image (option 1) and logging shim (option 2) already in place. DockerSandboxProvider probes /sys/kernel/security/apparmor/profiles at container creation time and applies the custom profile when loaded, falling back to docker-default transparently on hosts where AppArmor is unavailable (e.g. macOS / Docker Desktop).
qubeena07
force-pushed
the
feat/3068-apparmor-sandbox-profile
branch
from
0c6747c to
d3174a6
Compare
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
LGTM. AppArmor profile structure is correct (deny rules override the broad file allow), DockerSandboxProvider fallback is clean and tested, and the Rust prompt-injection improvements are well-covered. Minor: Python-based CLI entry points and local bin paths aren't covered by the profile, but that's expected for a defense-in-depth layer and is noted in the PR. Closing #3068.
|
Reviewed and approved -- LGTM. The CI suite hasn't run yet for this PR (fork contributor first-run protection). Could you push a trivial update (e.g. add a blank line to the AppArmor profile comment block) so CI triggers? Once the test suite is green we can merge. |
|
Done, pushed a trivial doc note to the AppArmor profile comment block to unblock CI. Will update once the test suite passes. |
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Re-approving after qubeena07's latest push. Profile correctly denies exec of network fetch tools, SSH, git, cloud CLIs, and k8s/terraform/helm. Fallback to docker-default is safe. Approved.
|
@MohammadHaroonAbuomar -- AppArmor profile for sandbox command denylist. Re-approved on my end after qubeena07's latest push. Waiting on your approval to merge. |
|
@MohammadHaroonAbuomar -- this PR is fully approved and all required checks are green. Could you merge it when you get a chance? Side note: my merge access stopped working -- looks like I was removed from the |
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Solid implementation of option 3 in the command-denylist stack. The _apparmor_profile_loaded helper is correct: reads the kernel sysfs file, splits each line safely, returns False on any OSError so macOS / Docker Desktop hosts fall through cleanly.
The profile mirrors docker-default for base rules and adds targeted deny ... x, blocks covering the same binary set as DENIED_LOGGED_BIN_NAMES in Dockerfile.sandbox. The three-layer stack (minimal PATH + shim + AppArmor) is well thought out and the in-profile comments explain the relationship clearly.
The Rust side (indirect injection rules) tightens the patterns in a compatible, non-breaking way: requiring this at the end of instructions_for_ai_reading, adding llm to system_note_to_assistant, adding model to the HTML-comment variant, and expanding retrieved_doc_override to cover article/text/result as document nouns and the ai/assistant/model as addressees. Test coverage is comprehensive and the benign-not-flagged tests prevent over-trigger regressions.
Minor nit: line.split() is called twice per line in _apparmor_profile_loaded. Not a bug, but splitting once into a local variable would be marginally cleaner. Not a blocker.
LGTM.
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Two additions that complement each other: AppArmor kernel-level enforcement + new prompt injection detection rules.
AppArmor profile:
- The deny rules correctly target both named and absolute-path invocations. AppArmor deny rules fire before allow rules, so the broad
file,allow above doesn't bypass the denies -- this is correct behavior. - The
_apparmor_profile_loaded()probe approach (reading from/sys/kernel/security/apparmor/profiles) and the graceful fallback todocker-defaultare the right design for a multi-platform tool. - Profile structure mirrors docker-default (moby/moby), which means security updates to docker-default can be ported here systematically.
Prompt injection Rust additions:
- The four new indirect injection rule IDs (
context:instructions_for_ai_reading,context:system_note_to_assistant,context:embedded_tool_directive,context:retrieved_doc_override) cover important attack vectors in RAG/agentic contexts. - The benign test cases (ordinary HTML comments, documents describing their own purpose) prevent false positives.
new_indirect_rules_are_known_built_in_idstest pins each rule ID in the registry, which catches typos or registration gaps.
Only the maintainer gate is failing.
Split the line once into a local variable instead of calling split() twice on the same string.
|
Fixed the nit from the review. Split the line once into a local variable instead of calling line.split() twice. Pushed to the branch. |
2 participants
Closes #3068.
Summary
docker/apparmor/agt-sandbox— a custom AppArmor profile that denies exec of dangerous network, cloud, and infra CLIs at the kernel level (option 3 of the command-denylist stack from feat: command denylist enforcement inside sandbox execution #2662)DockerSandboxProviderprobes/sys/kernel/security/apparmor/profilesat container creation and appliesagt-sandboxwhen loaded, falling back todocker-defaulton hosts where AppArmor is unavailable (macOS / Docker Desktop)_apparmor_profile_loaded()helper and_APPARMOR_PROFILE_NAMEclass constantRelationship to other layers
Test plan
TestAppArmorProfileLoaded— 4 tests: profile present/absent/OSError/no-partial-matchTestAppArmorProfileSelection— 3 tests: custom profile used when loaded, fallback to docker-default, constant valueTestContainerCreationHardening::test_hardening_flagsandTestHardeningAdditions::test_security_opt_includes_seccomp_and_apparmorstill pass (fallback path, no AppArmor on macOS CI)test_docker_sandbox.pysuite: 226 passedInstall the profile (one-time, requires root on the Docker host)