feat(rings): add command denylist enforcement for sandbox execution

[jlaportebot]

Summary

Adds command denylist enforcement to the RingEnforcer for sandbox execution environments.

Changes

• CommandCheckResult dataclass: Structured result type for command validation with allowed, reason, command, and matched_denylist_entry fields.

• check_command() method: New method on RingEnforcer that validates subprocess commands against the global DENIED_COMMANDS list from hypervisor.sandbox.

• Security features:

  • Case-insensitive matching to prevent bypasses via case variation (e.g., "CURL", "CuRl")
  • Shell metacharacter stripping (;, &, |) to prevent command injection bypasses
  • Handles empty/None commands and whitespace variations
  • Only checks the base command name (first token before whitespace)

• Comprehensive test coverage: 21 tests covering:

  • Safe commands allowed (python3, ls, cat)
  • Denied commands blocked (curl, wget, shells, compilers, network tools, alternative interpreters)
  • Case-insensitive matching
  • Commands with arguments
  • Empty/None/whitespace handling
  • Shell metacharacter injection attempts
  • Partial match handling (curl123 != curl)
  • Large input handling
  • Full DENIED_COMMANDS list validation
  • Integration with ring-level resource checks

Test Results

All 21 new tests pass, plus all 54 existing ring enforcement tests pass (75 total).

Integration

The check_command() method operates independently of ring-level resource checks. Callers should validate both:

1. Ring-level subprocess permission via check_resource(ring, ResourceType.SUBPROCESS)
2. Command-level denylist via check_command(command)

This follows the existing pattern where resource constraints and command validation are separate concerns.

[github-actions]

🤖 AI Agent: test-generator — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Test coverage looks good. No gaps identified.

[github-actions]

🤖 AI Agent: security-scanner — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

No security issues found.

[github-actions]

🤖 AI Agent: breaking-change-detector — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

No breaking changes detected.

[github-actions]

🤖 AI Agent: code-reviewer — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

TL;DR: 0 blockers, 1 warning. The PR introduces a robust command denylist enforcement mechanism, but the denylist source and update process should be clarified.

#	Sev	Issue	Where
1	Warn	Lack of clarity on how DENIED_COMMANDS is defined and maintained.	hypervisor.sandbox

Action items: None.

Warnings:

1. Clarify the source and update process for DENIED_COMMANDS in hypervisor.sandbox (fine as follow-up PR).

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Docs Sync

• check_command() in RingEnforcer -- missing docstring
• README.md -- section on sandbox execution needs update
• CHANGELOG.md -- missing entry for new check_command() method and denylist enforcement feature

[github-actions]

🔴 Contributor Check: HIGH

Check	Result
Profile	HIGH
Credential	LOW
Overall	HIGH

Automated check by AGT Contributor Check.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[imran-siddique]

This PR has developed a merge conflict since earlier today (likely after #3107 and #3110 landed). Please rebase onto main and we can get this in quickly.

[imran-siddique]

Reviewing this alongside #3114 (AppArmor profile, just merged) -- the two are complementary layers, #3109 provides the Python-side ring enforcement before a subprocess is even spawned. That's the right defense-in-depth shape.

A few things before I can approve:

1. Does check_command() handle shell-expanded commands? A policy that denies curl can be bypassed with sh -c 'cu'+'rl ...' unless the check happens at the shell-expanded level. How does the ring handle indirect invocation?

2. What's the behavior when the ring enforcement is bypassed entirely -- i.e. code that directly calls os.system or subprocess.run without going through RingEnforcer? Is there a hook or is this trust-on-call-site?

3. Does this need a rebase on top of the recent AppArmor merge (#3114)? The branch may have conflicts with the shim logging changes from #3019.

Address those and I'll approve.

[imran-siddique]

Good defense-in-depth addition. The implementation is clean:

• Case-insensitive matching via .lower() prevents the obvious bypass.
• Shell metacharacter stripping (;, &, |) before checking is the right order -- strip injection-relevant chars, then check the base name.
• Checking only the first token (command name) is correct; arguments don't affect whether the binary is denylisted.
• The CommandCheckResult dataclass with matched_denylist_entry is useful for structured logging.
• 21 tests covering the full DENIED_COMMANDS list, injection attempts, case variations, and partial matches.

The comment in the PR body about callers needing to check both ring-level subprocess permission AND command-level denylist is important and worth documenting in the method docstring too (if not already there). Only the maintainer gate is failing.

[imran-siddique]

@MohammadHaroonAbuomar can you take a look and merge when you get a chance? @imran-siddique has reviewed and approved this one.