Python: fix: hyperlight skips symlinks when staging sandbox input#5919
Open
eavanvalkenburg wants to merge 3 commits into
Open
Python: fix: hyperlight skips symlinks when staging sandbox input#5919eavanvalkenburg wants to merge 3 commits into
eavanvalkenburg wants to merge 3 commits into
Conversation
…ndbox The helpers that populate the sandbox input tree (``_copy_path`` and the ``_path_tree_signature`` walker used for cache invalidation) relied on ``Path.is_file()``, ``Path.is_dir()`` and ``shutil.copy2`` - all of which follow symlinks by default. When the source tree contains symlinks, that let entries from outside the configured input source surface inside the sandbox. Harden both code paths to never follow symlinks: - ``_copy_path`` now bails out via ``Path.is_symlink()`` before any ``is_dir()`` / ``is_file()`` check, skips non-regular files, and uses ``shutil.copy2(..., follow_symlinks=False)`` as defense in depth. - New ``_iter_real_entries`` walker replaces the previous ``Path.rglob`` call inside ``_path_tree_signature`` (rglob follows directory symlinks). - ``_path_tree_signature`` switches to ``Path.lstat()`` so size/mtime are never read through a symlink target. Added regression tests covering: - A pre-placed file symlink in ``workspace_root`` (top level). - A pre-placed directory symlink in ``workspace_root``. - A nested file symlink inside a real subdirectory. - ``_path_tree_signature`` ignoring symlinks so the cache key reflects only what is actually staged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Hyperlight sandbox input staging logic to avoid following symlinks when copying workspace_root and computing cache-key signatures, preventing unintended inclusion of files/directories outside configured inputs.
Changes:
- Update
_copy_pathto detect and skip symlinks and non-regular filesystem entries, and to copy withfollow_symlinks=False. - Update
_path_tree_signatureto avoid traversing symlinks by walking the tree with a symlink-aware iterator and usinglstat(). - Add regression tests ensuring symlinked files/directories are not staged and do not affect cache-key signatures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/hyperlight/agent_framework_hyperlight/_execute_code_tool.py | Implements symlink-avoiding staging and signature computation for sandbox inputs. |
| python/packages/hyperlight/tests/hyperlight/test_hyperlight_codeact.py | Adds tests covering symlink skip behavior for staging and cache-key signatures. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
- _iter_real_entries now yields directories and regular files only, skipping non-regular entries (sockets/FIFOs/devices). Keeps the cache-key signature consistent with what _copy_path actually stages. - The four new symlink regression tests skip when the platform does not support symlink creation (e.g. unprivileged Windows runners), via a local _symlinks_supported helper modelled on the one in packages/core/tests/core/test_skills.py. Prevents OSError / NotImplementedError from failing CI jobs that have nothing to do with the change under test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
eavanvalkenburg
added a commit
to eavanvalkenburg/agent-framework
that referenced
this pull request
May 18, 2026
Apply the same Windows-CI safety guard as the hyperlight fix in PR microsoft#5919: the three symlink integration tests create symlinks via Path.symlink_to(), which fails with OSError / NotImplementedError on unprivileged Windows runners. Add a local _symlinks_supported helper (mirroring the one in packages/core/tests/core/test_skills.py) and pytest.skip when symlinks aren't available, so the tests no longer fail for environment reasons. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SergeyMenshykh
approved these changes
May 18, 2026
moonbox3
reviewed
May 19, 2026
…eedback - _copy_path docstring: narrow the scope to "symlink entries present in the source tree at rest" and explicitly call out that the copy is NOT atomic with respect to concurrent mutation of the source tree. Callers who need that stronger guarantee should snapshot their workspace before passing it in. Avoids overpromising on a TOCTOU window that pathlib cannot express; closing it properly would need fd-based traversal (O_NOFOLLOW | O_DIRECTORY + os.scandir(fd)) with a separate Windows story, which is out of scope for this targeted fix. - _path_tree_signature: drop the `if path.is_symlink(): return ()` short-circuit. Resolve a symlink root to its real target before walking instead. The public construction flow already resolves workspace_root / file_mounts[].host_path up front so this never affected user-facing code, but the short-circuit was misleading and would have produced an empty, stable signature for any direct caller that builds a _RunConfig without going through the public constructor. Defense in depth: even if a future call site forgets to resolve the root, the cache key still reflects real contents. - Added regression test test_path_tree_signature_walks_through_symlinked_root: a symlinked workspace root must produce a non-empty signature, AND the signature must change when the real target's contents change so the cache key actually invalidates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
The helpers in
agent-framework-hyperlightthat populate the sandbox inputtree from
workspace_rootandfile_mountsfollowed symlinks duringstaging. When the source tree may contain symlinks, the resulting input
directory could include entries that did not physically live inside the
configured input source. This PR keeps the staged tree limited to real
entries that actually reside under the configured paths.
Description
Two helpers harden their walking behaviour:
_copy_pathdetects symlinks viaPath.is_symlink()before anyis_dir()/is_file()check, skips non-regular files (sockets, FIFOs,devices), and uses
shutil.copy2(..., follow_symlinks=False)as defensein depth.
_path_tree_signature(used for the per-config cache key) now walkswith a new
_iter_real_entrieshelper that performs aniterdir()+is_symlink()check at every directory level, mirroring what_copy_pathactually stages. It also switches toPath.lstat()sosize/mtime are read from the link entry itself, never from a target.
Behaviour change: symlinks inside
workspace_rootor a directoryfile_mountare silently skipped rather than dereferenced. Explicitly configured
file_mountswhosehost_pathis itself a symlink are already resolved to areal path by the existing
_resolve_existing_pathcall at mount-registrationtime, so that flow is unaffected.
Regression tests:
workspace_root(top level).workspace_root._path_tree_signatureignoring symlinks so the cache key matches thestaged tree.
Contribution Checklist
callers who configured a source tree containing symlinks (now skipped
instead of dereferenced).