fix(agent-hypervisor): prevent path traversal and prefix collision in session isolation by Kaap10 · Pull Request #4356 · microsoft/agent-governance-toolkit

Vardhman Gupta (Kaap10) · 2026-10-18T08:07:39Z

Related Issue

Fixes #4355

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Maintenance (dependency updates, CI/CD, refactoring)
Security fix

Package(s) Affected

Core & runtime:

Governance & security:

Platform & tooling:

CLI plugins:

agent-governance CLI plugins (copilot-cli / claude-code / opencode / antigravity-cli)

Shared / other:

schemas
action (GitHub Action)
examples
docs / root

Testing

Unit Testing

Updated tests/test_session_isolation.py:
- test_path_traversal_with_relative_segments_denied: verifies .. traversal escapes (/var/agt/sessions/session-1/../session-2/data.json and .../../../etc/passwd) are denied.
- test_sibling_prefix_collision_denied: verifies sibling prefix collision attempts (session-1-extra, session-10) are denied.
- test_nested_subdirectories_allowed: verifies authorized paths within own working directory remain allowed.
- test_granted_session_relative_traversal_denied: verifies traversal outside granted cross-session scope is denied.
- test_scope_widening_via_traversal_session_id_denied: verifies session_id=".." and session_id="s/../other" cannot widen access to parent directory or other scopes.
- test_create_scope_invalid_session_ids_rejected: verifies create_scope raises ValueError on traversal/slash identifiers (.., ., s/../other, foo/bar, "").
- test_grant_access_invalid_session_ids_rejected: verifies grant_cross_session_access returns False and SessionScope.grant_access raises ValueError on invalid session identifiers.
Conformance test verification:
- tests/test_spec_hypervisor_conformance.py Section 11 session isolation conformance tests pass.

Manual Testing

Executed full test suite for session isolation:

$env:PYTHONPATH="src"; pytest tests/test_session_isolation.py
# 26 passed in 0.73s

Executed lint and format checks:

ruff check agent-governance-python/agent-hypervisor/src/hypervisor/session/isolation.py agent-governance-python/agent-hypervisor/tests/test_session_isolation.py
ruff format --check agent-governance-python/agent-hypervisor/src/hypervisor/session/isolation.py agent-governance-python/agent-hypervisor/tests/test_session_isolation.py

Checklist

I have linked a related issue above, or completed "Problem & Solution", "Impact on Your Work", and "Alternatives Considered"
My code follows the project style guidelines (ruff check)
I have added tests that prove my fix/feature works
All new and existing tests pass (pytest)
I have updated documentation as needed
I have signed the Microsoft CLA

Attribution & Prior Art

This contribution does not contain code copied or derived from other projects without attribution
Any external projects that inspired this design are credited in code comments or documentation
If this PR implements functionality similar to an existing open-source project, I have listed it below

Prior art / related projects (if any):
None

AI Assistance

I can explain every meaningful change in this PR: what it does, why, and what tradeoffs were considered
I have run tests and verification appropriate for this change
No part of this PR was autonomously submitted by an AI agent without my review
I have not used AI to generate review comments on others' PRs

If AI tools materially shaped this change, briefly note what was used:
Antigravity for implementation assistance and review response — all code, tests, and diffs reviewed and verified by author.

IP, Patents, and Licensing

This contribution does not implement patent-pending or patent-encumbered techniques
This contribution does not require an NDA or licensing agreement to understand or use
Any AI tools used have terms compatible with the MIT License

… session isolation Normalize paths using posixpath.normpath and enforce directory hierarchy containment in SessionScope.is_path_allowed. Add regression tests covering relative segment breakouts, sibling directory prefix collisions, nested subdirectories, and granted cross-session boundaries. Fixes microsoft#4355 Signed-off-by: Vardhman Gupta <[email protected]>

azure-pipelines · 2026-10-18T08:07:48Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

github-actions · 2026-10-18T08:08:04Z

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.

github-actions · 2026-10-18T08:08:08Z

🔴 Contributor Check: HIGH

Check	Result
Profile	HIGH
Credential	LOW
Overall	HIGH

Automated check by AGT Contributor Check.

MohammadHaroonAbuomar

Thank you for the report and the fix. I verified both bugs on main and confirmed the fix closes them: traversal through .. and sibling-prefix collisions are denied, nested paths and the exact directory are still allowed, the granted-session path gets the same containment check, and three of the four new tests fail on main and pass here. The hypervisor suite passes locally (667 passed).

Three things before I can approve:

Scope widening through the session or granted id. _is_path_contained normalizes the base directory as well as the target, so an id containing .. or / now widens the scope instead of matching nothing. I reproduced it on this head: SessionScope(session_id="..", agent_did=..., base_path="/var/agt/sessions") has working directory /var/agt/sessions/.. and is_path_allowed("/var/agt/secret") returns True (main returns False); session_id="s/../other" allows /var/agt/sessions/other/x; and grant_cross_session_access("s1", "..") under READ_COMMITTED grants every sibling session. Ids come from the control plane rather than the agent, but the change turns a formerly inert misconfiguration into a wide grant. Please either require the normalized working and granted directories to sit strictly inside posixpath.normpath(self.base_path), or reject ids that contain / or equal . or .. in create_scope and grant_access, and add a test for each case.
PR template. Please use the repository template for the description: Type of Change, Package(s) Affected, Testing and Checklist sections, and the AI Assistance and IP attestations (with a note on any AI tools used).
CHANGELOG. Please add an entry to the root CHANGELOG.md under ## [Unreleased] > ### Fixed, in the style of the existing Hypervisor entries, naming both bugs and the two behaviour changes that come with the fix: a relative base_path and the session parent directory (.../session-1/..) are now denied.

Optional: run ruff format on tests/test_session_isolation.py (three create_scope(...) lines exceed 100 columns), and a docstring note that the check is a string containment test and does not resolve symlinks would help future readers.

The gated CI workflows for first-time contributors need a maintainer to approve them before they run; that is not on you.

…entifiers Reject traversal identifiers in create_scope and grant_access, ensure working directories remain strictly inside base_path, add CHANGELOG entry, and expand regression tests. Addresses review on microsoft#4356. Signed-off-by: Vardhman Gupta <[email protected]>

Vardhman Gupta (Kaap10) · 2026-10-18T09:07:05Z

Hi MohammadHaroonAbuomar, Thanks for the review!
All feedback items have been addressed:

Scope widening & identifier validation: Added validation to reject /, \, ., .., and empty IDs in create_scope, grant_access, and is_path_allowed. Enforced strict containment (_is_strictly_within) against base_path for working and granted directories, and added regression tests for direct scope traversal, invalid create_scope IDs, and invalid grants.
PR template: Updated the PR description to match .github/pull_request_template.md with full checklist and attestations.
CHANGELOG: Added an entry under ## [Unreleased] > ### Fixed documenting both bugs and the two behavior changes.
Formatting & docs: Formatted tests/test_session_isolation.py with ruff format and added the symlink disclaimer docstring to _is_path_contained.

All 26 isolation tests and Section 11 conformance tests pass locally, with ruff check passing clean.

All three asks are addressed on 9aec40d. I re-ran my probe against this head: a session id of '..' or 's/../other' and an empty id are denied, create_scope and grant_access reject such ids, grant_cross_session_access returns False for them, and the normal cases (traversal denied, sibling prefix denied, nested and exact paths allowed) are unchanged. The description now follows the template with attestations, and the changelog entry names both bugs and the behaviour changes. Hypervisor suite passes locally (670 passed), ruff check and ruff format are clean. Nothing left to fix. The gated CI workflows for first-time contributors still need a maintainer to approve them; once they run green I will approve and merge.

Vardhman Gupta (Kaap10) · 2026-10-22T06:55:46Z

Hi MohammadHaroonAbuomar,

Gentle reminder when you have a moment - could you please approve and trigger the gated CI workflows for this PR so they can run? Everything is ready to go. Thanks!

MohammadHaroonAbuomar · 2026-10-22T07:37:40Z

Thanks for the reminder. Gated CI runs on a first contribution are approved by a maintainer by hand; I have flagged this one and will come back once they have run.

Vardhman Gupta (Kaap10) · 2026-10-29T11:10:50Z

Hi MohammadHaroonAbuomar liamcrumm,

Checking in gently to see if someone could please approve the gated CI workflows for this PR when you have a moment.

Also, please let me know if it's alright for me to pick up and work on another issue in the meantime, or if you prefer I wait until this PR is merged first. Thanks for your time and guidance!

MohammadHaroonAbuomar · 2026-10-29T11:29:51Z

Vardhman Gupta (@Kaap10) thanks for the patience. The gated runs on a first contribution need a maintainer with the right permission, and this one is in the queue; I will make sure it is not forgotten. Please do pick up another issue in the meantime, no need to wait on this one.

Vardhman Gupta (Kaap10) requested review from MohammadHaroonAbuomar, liamcrumm and Prayag (prayagupa) as code owners October 18, 2026 08:07

github-actions Bot added tests agent-hypervisor agent-hypervisor package size/M Medium PR (< 200 lines) labels Oct 18, 2026

github-actions Bot added the needs-review:HIGH Contributor reputation check flagged HIGH risk label Oct 18, 2026

MohammadHaroonAbuomar previously requested changes Oct 18, 2026

View reviewed changes

github-actions Bot added the documentation Improvements or additions to documentation label Oct 18, 2026

Merge branch 'main' into fix/session-isolation-path-traversal

7e2d8ca

Navigation Menu

Repository navigation

fix(agent-hypervisor): prevent path traversal and prefix collision in session isolation - #4356

fix(agent-hypervisor): prevent path traversal and prefix collision in session isolation#4356
Vardhman Gupta (Kaap10) wants to merge 3 commits into
microsoft:mainfrom
Kaap10:fix/session-isolation-path-traversal

Vardhman Gupta (Kaap10) commented Oct 18, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Oct 18, 2026

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

MohammadHaroonAbuomar left a comment

Uh oh!

Vardhman Gupta (Kaap10) commented Oct 18, 2026

Uh oh!

Vardhman Gupta (Kaap10) commented Oct 22, 2026

Uh oh!

MohammadHaroonAbuomar commented Oct 22, 2026

Uh oh!

Vardhman Gupta (Kaap10) commented Oct 29, 2026

Uh oh!

MohammadHaroonAbuomar commented Oct 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Vardhman Gupta (Kaap10) commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue

Type of Change

Package(s) Affected

Testing

Unit Testing

Manual Testing

Checklist

Attribution & Prior Art

AI Assistance

IP, Patents, and Licensing

Uh oh!

azure-pipelines Bot commented Oct 18, 2026

Uh oh!

github-actions Bot commented Oct 18, 2026

PR Review Summary

Uh oh!

github-actions Bot commented Oct 18, 2026

Uh oh!

MohammadHaroonAbuomar left a comment

Choose a reason for hiding this comment

Uh oh!

Vardhman Gupta (Kaap10) commented Oct 18, 2026

Uh oh!

Vardhman Gupta (Kaap10) commented Oct 22, 2026

Uh oh!

MohammadHaroonAbuomar commented Oct 22, 2026

Uh oh!

Vardhman Gupta (Kaap10) commented Oct 29, 2026

Uh oh!

MohammadHaroonAbuomar commented Oct 29, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Vardhman Gupta (Kaap10) commented Oct 18, 2026 •

edited

Loading