Skip to content

fix(bright_data): confine get_screenshot output to a configurable directory#522

Draft
yonib05 wants to merge 2 commits into
strands-agents:mainfrom
yonib05:fix/bright-data-output-path
Draft

fix(bright_data): confine get_screenshot output to a configurable directory#522
yonib05 wants to merge 2 commits into
strands-agents:mainfrom
yonib05:fix/bright-data-output-path

Conversation

@yonib05

@yonib05 yonib05 commented Jun 27, 2026

Copy link
Copy Markdown
Member

Summary

get_screenshot previously wrote the downloaded screenshot bytes to the raw output_path without any validation, so the file could land anywhere on the filesystem.

This change resolves output_path and confines it to a single root directory:

  • The root is the current working directory by default, or the directory named by the STRANDS_BRIGHT_DATA_OUTPUT_DIR environment variable when set.
  • The resolved path must be the root itself or a location inside it; otherwise a ValueError is raised.
  • A final path component that is a symlink is rejected so the write is not redirected elsewhere.
  • Relative paths are resolved against the root; the returned value is the resolved absolute path.

Behavior change

get_screenshot now writes within the working directory by default. To write somewhere else, set STRANDS_BRIGHT_DATA_OUTPUT_DIR to the desired directory (it may point anywhere the operator chooses, including a broad location); relative output_path values are taken from that directory and absolute paths inside it are allowed. The docstring, tool parameter docs, and the rejection error message now state this so the override is discoverable.

Testing

  • Added regression tests covering: a relative path written inside the configured directory, a relative traversal (../../etc/x) rejected, an absolute path outside the root rejected, a symlink target rejected, default-root (no env var) traversal/absolute-outside rejection, an actionable rejection message, and an operator-chosen output directory accepting an absolute path inside it.
  • pytest tests/test_bright_data.py passes (21 tests).
  • ruff format --check and ruff check pass on the changed files.

…ectory

Resolve the get_screenshot output_path and require it to be the current
working directory (or the directory named by STRANDS_BRIGHT_DATA_OUTPUT_DIR)
or a location inside it. Reject a final path component that is a symlink.

Add regression tests covering relative traversal, absolute paths outside
the root, and symlink targets.
@yonib05 yonib05 marked this pull request as draft June 29, 2026 21:46
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.

1 participant