Skip to content

Conversation

@athrael-soju
Copy link
Owner

Potential fix for https://github.com/athrael-soju/Snappy/security/code-scanning/6

General fix approach: When constructing filesystem paths from user input, anchor them to a known-safe base and enforce that the resolved path remains within that base. In this case, we should (1) treat the bucket as configuration once it’s validated, not as part of the untrusted path, and (2) ensure that the security check compares against the bucket’s directory (e.g., <storage_base>/<bucket_name>) instead of just the broad storage base. The rest of the code can remain the same.

Concrete changes in backend/clients/local_storage_utils.py:

  1. In resolve_storage_path, after validating the bucket value, derive a bucket_base directory using the configured bucket name, not the user-provided argument.
  2. Construct file_path using bucket_base / relative_path instead of storage_base / bucket / relative_path.
  3. For the traversal check, compute bucket_resolved = bucket_base.resolve() and then use resolved_path.is_relative_to(bucket_resolved) instead of checking only against storage_base.

This preserves existing behavior (only the configured bucket is allowed; files must exist; traversal is blocked) while tightening the root against which traversal is enforced. No new methods or imports are needed; we only adjust the path construction and comparison logic in the shown function. This single change also safely handles calls from files.py and from _load_image_from_url in interpretability.py that flow through parse_files_url and into resolve_storage_path.


Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@athrael-soju athrael-soju marked this pull request as ready for review December 22, 2025 23:55
@athrael-soju athrael-soju merged commit 41a1b0f into main Dec 22, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants