Prevent symlink path traversal in local artifact store by BenWilson2 · Pull Request #18964 · mlflow/mlflow

BenWilson2 · 2025-11-21T22:03:28Z

🛠 DevTools 🛠

Install mlflow from this PR

# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/18964/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/18964/merge#subdirectory=libs/skinny

For Databricks, use the following command:

%sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/18964/merge

Related Issues/PRs

#xxx

What changes are proposed in this pull request?

Checks that symlink path traversal cannot extend outside of the base artifact path to prevent a path traversal attack.

How is this PR tested?

Existing unit/integration tests
New unit/integration tests
Manual tests

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

How should the PR be classified in the release notes? Choose one:

rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Should this PR be included in the next patch release?

Yes should be selected for bug fixes, documentation updates, and other small changes. No should be selected for new features and larger changes. If you're unsure about the release classification of this PR, leave this unchecked to let the maintainers decide.

What is a minor/patch release?

Minor release: a release that increments the second part of the version number (e.g., 1.2.0 -> 1.3.0).
Bug fixes, doc updates and new features usually go into minor releases.
Patch release: a release that increments the third part of the version number (e.g., 1.2.0 -> 1.2.1).
Bug fixes and doc updates usually go into patch releases.

Yes (this PR will be cherry-picked and included in the next patch release)
No (this PR will be included in the next minor release)

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

github-actions · 2025-11-21T22:12:19Z

Documentation preview for 275de93 is available at:

https://pr-18964--mlflow-docs-preview.netlify.app/docs/latest/

More info

Ignore this comment if this PR does not change the documentation.
The preview is updated when a new commit is pushed to this PR.
This comment was created by this workflow run.
The documentation was built by this workflow run.

harupy · 2025-11-22T01:47:34Z

mlflow/utils/uri.py

    return path


+def validate_path_within_directory(base_dir, constructed_path):


lets add type hints

harupy · 2025-11-22T01:47:59Z

mlflow/utils/uri.py

+    Returns:
+        The constructed_path if validation passes.
+    """
+    real_base_dir = os.path.realpath(base_dir)


can we leverage pathlib?

harupy · 2025-11-22T01:49:00Z

tests/store/artifact/test_local_artifact_repo.py

+    secret_file.write_text("SECRET_CONTENT")
+    yield secret_dir
+    if secret_dir.exists():
+        shutil.rmtree(str(secret_dir))


we can remove this. pytest removes the tmp directory

harupy · 2025-11-22T03:30:49Z

tests/store/artifact/test_local_artifact_repo.py

+    secret_dir = tmp_path.parent / "secrets_outside"
+    secret_dir.mkdir(exist_ok=True)
+    secret_file = secret_dir / "secret.txt"
+    secret_file.write_text("SECRET_CONTENT")


Suggested change

secret_file.write_text("SECRET_CONTENT")

secret_file.touch()

since contents don't matter

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

harupy · 2025-11-26T02:44:07Z

mlflow/utils/uri.py

+    real_constructed_path = pathlib.Path(constructed_path).resolve()
+
+    try:
+        real_constructed_path.relative_to(real_base_dir)


can we use is_relative_to?

harupy · 2025-11-26T02:44:36Z

tests/store/artifact/test_local_artifact_repo.py

+):
+    artifact_dir = pathlib.Path(local_artifact_repo.artifact_dir)
+    symlink_path = artifact_dir / symlink_name
+    os.symlink(str(external_secret_dir), str(symlink_path))


os.symlink only accepts strings?

pathlib doesn't have a method to create a symlink?

harupy · 2025-11-26T14:34:48Z

mlflow/utils/uri.py


+def validate_path_within_directory(base_dir: str, constructed_path: str) -> str:
+    """
+    Validates that the constructed path (after resolving symlinks) is within the base directory.


Currently, we blindly follow symlinks. I wonder if that's really ok

I don't think we have much of an option since symlinks would be entirely justifiable to use from within the scope of the artifact root directory. Blocking traversal outside of that root seems like effective prevention of attack vectors.

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

harupy

LGTM

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

Initial verification check

4c070f7

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

github-actions bot added v3.6.1 area/models MLmodel format, model serialization/deserialization, flavors rn/bug-fix Mention under Bug Fixes in Changelogs. labels Nov 21, 2025

BenWilson2 added the team-review Trigger a team review request label Nov 21, 2025

github-actions bot requested review from B-Step62, TomeHirata, WeichenXu123, daniellok-db, harupy, kevin-lyn, serena-ruan and xq-yin November 21, 2025 22:18

harupy reviewed Nov 22, 2025

View reviewed changes

mlflow/utils/uri.py Outdated

return path

def validate_path_within_directory(base_dir, constructed_path):

Copy link

Member

harupy Nov 22, 2025

Choose a reason for hiding this comment

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

lets add type hints

harupy reviewed Nov 22, 2025

View reviewed changes

fix

db0fbc5

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

BenWilson2 requested a review from harupy November 25, 2025 17:27

harupy reviewed Nov 26, 2025

View reviewed changes

BenWilson2 added 2 commits December 1, 2025 22:52

Merge branch 'master' into fix-path-traversal

341f2c1

adjust

275de93

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

BenWilson2 requested a review from harupy December 2, 2025 03:59

BenWilson2 added v3.7.0 and removed v3.6.1 labels Dec 2, 2025

harupy approved these changes Dec 2, 2025

View reviewed changes

BenWilson2 added this pull request to the merge queue Dec 2, 2025

Merged via the queue into mlflow:master with commit 6fd54de Dec 2, 2025
72 of 77 checks passed

BenWilson2 deleted the fix-path-traversal branch December 2, 2025 16:56

BenWilson2 added a commit to BenWilson2/mlflow that referenced this pull request Dec 4, 2025

Prevent symlink path traversal in local artifact store (mlflow#18964)

ca3f2c0

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

BenWilson2 added a commit that referenced this pull request Dec 4, 2025

Prevent symlink path traversal in local artifact store (#18964)

0f6ea61

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent symlink path traversal in local artifact store#18964

Prevent symlink path traversal in local artifact store#18964
BenWilson2 merged 4 commits intomlflow:masterfrom
BenWilson2:fix-path-traversal

BenWilson2 commented Nov 21, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Nov 21, 2025 •

edited

Loading

Uh oh!

harupy Nov 22, 2025

Uh oh!

harupy Nov 22, 2025

Uh oh!

harupy Nov 22, 2025

Uh oh!

harupy Nov 22, 2025 •

edited

Loading

Uh oh!

harupy Nov 26, 2025

Uh oh!

harupy Nov 26, 2025

Uh oh!

harupy Nov 26, 2025

Uh oh!

harupy Nov 26, 2025 •

edited

Loading

Uh oh!

BenWilson2 Dec 2, 2025

Uh oh!

harupy left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		return path


		def validate_path_within_directory(base_dir, constructed_path):

Conversation

BenWilson2 commented Nov 21, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Install mlflow from this PR

Related Issues/PRs

What changes are proposed in this pull request?

How is this PR tested?

Does this PR require documentation update?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

Should this PR be included in the next patch release?

Uh oh!

github-actions bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harupy Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

harupy Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

harupy Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

harupy Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harupy Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

harupy Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

harupy Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

harupy Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenWilson2 Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

harupy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BenWilson2 commented Nov 21, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Nov 21, 2025 •

edited

Loading

harupy Nov 22, 2025 •

edited

Loading

harupy Nov 26, 2025 •

edited

Loading