Skip to content

Python: fix(python): check the full download path in SessionsPythonTool - #14791

Merged
westey (westey-m) merged 3 commits into
microsoft:mainfrom
westey-m:python-sessions-plugin-fullpath
Nov 13, 2026
Merged

westey (westey-m) merged 3 commits into
microsoft:mainfrom
westey-m:python-sessions-plugin-fullpath

Conversation

@westey-m

Copy link
Copy Markdown
Contributor

Motivation and Context

When allowed_download_directories is set, SessionsPythonTool.download_file() checks that the local path is inside one of the allowed folders. The check only resolved the folder part of the path and then added the file name back without resolving it. When the file name is a symbolic link, the path that was checked and the place the file was actually written could be different, so a path could pass the check and still be written outside the allowed folders.

This also brings the Python check in line with how the .NET SessionsPythonPlugin resolves download paths.

Description

  • _validate_local_path_for_download now resolves the whole path, including the file name, with os.path.realpath. It compares that result with the allowed folders and returns the same value, so the path that is checked is the path that is written.
  • A path that resolves to an allowed folder itself is still rejected.
  • Paths that resolve to a folder (for example allowed/.) are now rejected with the normal "not within allowed download directories" error. Before, they passed the check and failed later when opening the folder for writing.
  • No change when allowed_download_directories is not set, and no change to uploads.
  • The "denied" log line now also shows the resolved path.
  • New tests:
    • a link to a file outside the allowed folder is rejected, both when the target file exists and when it does not;
    • a link to a file inside the allowed folder still works.

Validation:

  • uv run --no-sync pytest tests/unit/core_plugins/test_sessions_python_plugin.py: 89 passed
  • pre-commit hooks (ruff, ruff-format, pyupgrade, bandit and the rest) on both changed files: all passed

Contribution Checklist

@semantic-kernel-automation semantic-kernel-automation Bot added the python Pull requests for the Python Semantic Kernel label Nov 13, 2026
@westey-m
westey (westey-m) marked this pull request as ready for review November 13, 2026 15:40
@westey-m
westey (westey-m) requested a review from a team as a code owner November 13, 2026 15:40
Copilot AI balanced review requested due to automatic review settings November 13, 2026 15:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the download-path validation changes.

No open issues 🎉

What changed in this PR

This PR strengthens SessionsPythonTool download-path validation by resolving the entire destination, including filename symlinks, before checking allowed directories.

Changes:

  • Validates and returns the fully resolved path, rejects destinations equal to an allowed root, and includes the resolved path in denial logs.
  • Adds regression tests for symlinks targeting existing or missing files outside allowed directories and permitted targets inside them.
  • Updates the nbQA and pyupgrade pre-commit hooks.
File Description
python/​tests/​unit/​core_plugins/​test_sessions_python_plugin.py Adds download regression tests for allowed and denied symlink targets.
python/​semantic_kernel/​core_plugins/​sessions_python_tool/​sessions_python_plugin.py Resolves complete download paths for allowlist checks and improves denial logging.
python/​.pre-commit-config.yaml Updates nbQA to 1.10.0 and pyupgrade to v3.21.2.

Review effort: Balanced

@westey-m
westey (westey-m) added this pull request to the merge queue Nov 13, 2026
Merged via the queue into microsoft:main with commit ff22ec3 Nov 13, 2026
40 checks passed
@westey-m
westey (westey-m) deleted the python-sessions-plugin-fullpath branch November 13, 2026 16:16

This branch was successfully deployed

1 active deployment
github-app-auth — 5c0977a1 Deployed Nov 13, 2026 by westey-m via add_label #29685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants