Repository navigation
Python: Improve regex validation for shell and file access - #8507
Merged
westey (westey-m) merged 4 commits intoSep 18, 2026
Merged
Conversation
westey (westey-m)
requested review from
SergeyMenshykh,
Tao Chen (TaoChenOSU),
Vincent Biret (baywet),
Eduard van Valkenburg (eavanvalkenburg),
Giles Odigwe (giles17),
Jose Alvarez (jpalvarezl),
Evan Mattson (moonbox3),
Peter Ibekwe (peibekwe) and
Roger Barreto (rogerbarreto)
as code owners
September 18, 2026 11:18
westey (westey-m)
temporarily deployed
to
github-app-auth
September 18, 2026 11:18 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
temporarily deployed
to
github-app-auth
September 18, 2026 11:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
temporarily deployed
to
github-app-auth
September 18, 2026 11:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
temporarily deployed
to
integration
September 18, 2026 11:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
had a problem deploying
to
integration
September 18, 2026 11:18 — with
GitHub Actions
Failure
GitHub Actions
Failure
agent-framework-automation
Bot
added
documentation
github-actions
Bot
changed the title
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Precompiled Python patterns still permit unbounded ReDoS, and invalid patterns introduce an exception compatibility break.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds bounded regex evaluation to prevent ReDoS in file searches and shell policies across Python and .NET.
Changes:
- Uses timeout-capable regex matching with fail-closed shell-policy behavior.
- Adds shared search deadlines and ReDoS regression tests.
- Updates dependencies, documentation, and related formatting.
File summaries
| File | Description |
|---|---|
python/uv.lock |
Locks the new regex dependency. |
python/packages/tools/tests/test_policy.py |
Tests shell-policy timeouts and compiled patterns. |
python/packages/tools/pyproject.toml |
Adds regex to tools dependencies. |
python/packages/tools/agent_framework_tools/shell/_policy.py |
Implements bounded policy matching. |
python/packages/core/tests/workflow/test_agent_executor_tool_calls.py |
Formatting-only change. |
python/packages/core/tests/core/test_harness_tool_approval.py |
Formatting-only change. |
python/packages/core/tests/core/test_harness_file_access.py |
Adds file-search ReDoS tests. |
python/packages/core/tests/core/test_function_invocation_logic.py |
Formatting-only change. |
python/packages/core/pyproject.toml |
Adds regex to core dependencies. |
python/packages/core/AGENTS.md |
Documents the search timeout contract. |
python/packages/core/agent_framework/_workflows/_agent_executor.py |
Formatting-only change. |
python/packages/core/agent_framework/_tools.py |
Formatting-only change. |
python/packages/core/agent_framework/_harness/_file_access.py |
Implements deadline-bounded file searching. |
dotnet/tests/Microsoft.Agents.AI.Tools.Shell.UnitTests/ShellPolicyTests.cs |
Tests .NET timeout behavior. |
dotnet/src/Microsoft.Agents.AI.Tools.Shell/ShellPolicy.cs |
Adds per-match .NET regex timeouts. |
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): b2ee4b2de3f9
Model: gpt-5.6-sol-fast
Overview
The PR replaces uninterruptible string-pattern matching with timeout-capable engines, fails shell policy checks closed, and applies a shared deadline plus an outer timeout to file searches. The new tests cover catastrophic backtracking, event-loop responsiveness, normal policy decisions, and all built-in file-store paths. The remaining risk is compatibility: the Python engine switch changes established Unicode case-insensitive behavior in both shell policy and file search, and shell-policy construction no longer preserves the prior re.error exception contract.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (3 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_harness/_file_access.py, python/packages/tools/agent_framework_tools/shell/_policy.py
westey (westey-m)
temporarily deployed
to
github-app-auth
September 18, 2026 13:51 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
had a problem deploying
to
integration
September 18, 2026 13:51 — with
GitHub Actions
Failure
GitHub Actions
Failure
westey (westey-m)
temporarily deployed
to
integration
September 18, 2026 13:51 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
temporarily deployed
to
github-app-auth
September 18, 2026 14:31 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
had a problem deploying
to
integration
September 18, 2026 14:31 — with
GitHub Actions
Failure
GitHub Actions
Failure
westey (westey-m)
temporarily deployed
to
integration
September 18, 2026 14:31 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
temporarily deployed
to
integration
September 18, 2026 14:46 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Eduard van Valkenburg (eavanvalkenburg)
approved these changes
Sep 18, 2026
Eduard van Valkenburg (eavanvalkenburg)
left a comment
Eduard van Valkenburg (eavanvalkenburg)
left a comment
Member
There was a problem hiding this comment.
small nit on the code
westey (westey-m)
temporarily deployed
to
github-app-auth
September 18, 2026 15:54 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
enabled auto-merge
September 18, 2026 15:54
westey (westey-m)
temporarily deployed
to
integration
September 18, 2026 15:54 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
westey (westey-m)
temporarily deployed
to
integration
September 18, 2026 15:54 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Eduard van Valkenburg (eavanvalkenburg)
approved these changes
Sep 18, 2026
Manohar Paturi (ManoharPaturi)
added a commit
to ManoharPaturi/agent-framework
that referenced
this pull request
Oct 2, 2026
The upstream microsoft#8507 assertion expected list_children("") to stay empty, which held while create_directory was a no-op. This PR makes explicit directories listable, so the session-1 directory created during before_run now appears at the root. The invariant the test protects, that a rejected nested write leaves nothing behind, is preserved: the session directory exists and stays empty, with no notes child.
5 tasks done
Eduard van Valkenburg (eavanvalkenburg)
pushed a commit
to MohammadHaroonAbuomar/agent-framework
that referenced
this pull request
Oct 26, 2026
…ies in InMemoryAgentFileStore (microsoft#9608) * Python: fix(core): prevent path collisions between files and directories in InMemoryAgentFileStore Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> * Index directory prefixes and persist explicit directories in InMemoryAgentFileStore Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> * test(harness): account for session dir in nested-path rejection test The upstream microsoft#8507 assertion expected list_children("") to stay empty, which held while create_directory was a no-op. This PR makes explicit directories listable, so the session-1 directory created during before_run now appears at the root. The invariant the test protects, that a rejected nested write leaves nothing behind, is preserved: the session directory exists and stays empty, with no notes child. * Python: let the in-memory store delete empty explicit directories Explicitly created directories had no deletion path, so session-scoped runs kept accumulating entries in the shared map. delete() now removes an explicit directory once nothing is stored under it and no other explicit directory is nested below, giving session directories a lifecycle. Non-empty directories stay put, matching the filesystem store's file-only deletes. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> * Python: keep delete file-only and add delete_directory to AgentFileStore Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> * Python: bound store path complexity before materializing ancestor prefixes The in-memory store's directory index retains the full key of every ancestor directory, so an unbounded path let a single write hold quadratic space in the path depth (an 8000 deep path retained roughly 64 million characters of prefix strings). The shared normalizer now refuses paths beyond 255 segments or 4096 characters with ENAMETOOLONG, the same errno the filesystem backend's OS layer returns, so the bound is checked before any prefix is allocated and is identical across backends. Deep but realistic paths keep working, and regression tests cover the rejection, the empty index afterward, and backend parity on both sides of the bound. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> * Python: refuse the store root in delete_directory on both backends The filesystem backend resolved the empty path to the configured store root and rmdir'd it, returning True, while the in-memory backend raised on the same call from re-normalizing the already normalized path as a file path. Both now return False for the empty or whitespace-only path before touching storage, the in-memory store normalizes once, and the parity test covers the empty path and whitespace spelling on both backends. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --------- Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Co-authored-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Co-authored-by: Manohar Paturi <manoharpaturi@users.noreply.github.com>
This branch was previously deployed
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 & Context
Description & Review Guide
Related Issue
Fixes #
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.