Fix: Prevent environment variable leakage in subprocess calls

[kkaarreell]

Replace deep copy of os.environ with whitelisted environment variables to prevent sensitive credentials (like NEWA_* variables) from leaking to subprocess calls. Only explicitly required variables are now passed to testing-farm CLI and other subprocesses.

🤖 Generated with Claude Code

Summary by Sourcery

Restrict environment variables passed to testing-farm subprocesses to a minimal, whitelisted set to avoid leaking sensitive data while preserving required CLI functionality.

Bug Fixes:

• Prevent leakage of sensitive environment variables (e.g., NEWA_* credentials) into testing-farm and related subprocess calls by no longer deep-copying the full parent environment.

Enhancements:

• Introduce a reusable helper to construct a clean, whitelisted environment for all testing-farm-related subprocess executions.

[sourcery-ai]

Reviewer's Guide

Introduce a whitelisted environment builder for subprocess calls and replace direct deep copies of os.environ with minimal, explicitly controlled environments when invoking the testing-farm CLI, to avoid leaking sensitive NEWA_* and other credentials.

Sequence diagram for whitelisted environment in testing-farm subprocess calls

sequenceDiagram
    actor User
    participant Request
    participant ExecutionModule
    participant subprocess

    User->>Request: initiate_tf_request(ctx)
    Request->>Request: generate_tf_exec_command(ctx)
    Request->>Request: set NO_COLOR, NO_TTY in environment
    Request->>ExecutionModule: build_clean_environment(additional_vars)
    ExecutionModule-->>Request: env (whitelisted)
    Request->>subprocess: run(command, env=env)

    User->>ExecutionModule: check_tf_cli_version(ctx)
    ExecutionModule->>ExecutionModule: build_clean_environment(additional_vars)
    ExecutionModule->>subprocess: run(['testing-farm','version'], env=env)

Loading

File-Level Changes

Change	Details	Files
Add a helper to construct a whitelisted environment for subprocess execution and use it for testing-farm related calls instead of deep-copying the full process environment.	Introduce build_clean_environment(additional_vars) that builds an env dict from a fixed passthrough whitelist (TESTING_FARM_API_TOKEN, PATH, HOME, USER, LANG, LC_ALL) and optional overrides. Update Request.initiate_tf_request to mutate its generated environment with NO_COLOR/NO_TTY and then call build_clean_environment(additional_vars=environment) instead of deep-copying os.environ. Update TFRequest.cancel to construct a minimal env using build_clean_environment with NO_COLOR/NO_TTY instead of using copy.deepcopy(os.environ). Update check_tf_cli_version to construct a minimal env using build_clean_environment with NO_COLOR/NO_TTY instead of using copy.deepcopy(os.environ).	newa/models/execution.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• Consider extracting the passthrough_vars list into a module-level constant so it’s easier to audit and update the set of allowed environment variables in one place.
• The NO_COLOR/NO_TTY additions are repeated in multiple places; you could introduce a small helper (or include them directly in build_clean_environment) to avoid duplication and ensure they stay consistent across all subprocess calls.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Consider extracting the `passthrough_vars` list into a module-level constant so it’s easier to audit and update the set of allowed environment variables in one place.
- The `NO_COLOR`/`NO_TTY` additions are repeated in multiple places; you could introduce a small helper (or include them directly in `build_clean_environment`) to avoid duplication and ensure they stay consistent across all subprocess calls.

## Individual Comments

### Comment 1
<location path="newa/models/execution.py" line_range="61-68" />
<code_context>
+    Returns:
+        Dictionary with whitelisted environment variables
+    """
+    # Whitelist of environment variables to pass through
+    passthrough_vars = [
+        'TESTING_FARM_API_TOKEN',  # Required by testing-farm CLI for authentication
+        'PATH',                     # Required to find executables
+        'HOME',                     # May be needed by CLI tools for config
+        'USER',                     # May be needed by some tools
+        'LANG',                     # Locale settings
+        'LC_ALL',                   # Locale settings
+        ]
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Whitelisting may unintentionally drop proxy-related env vars needed in proxied environments.

This whitelist also omits common, non-sensitive proxy/SSL vars like `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY`, and `REQUESTS_CA_BUNDLE`. In proxied environments this can prevent the CLI from reaching external services. Consider explicitly allowing these (or a defined set of networking-related vars) if you need connectivity in such setups.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}