Skip to content

Conversation

@venkateshpabbati
Copy link
Owner

Potential fix for https://github.com/venkateshpabbati/adk-python/security/code-scanning/2

General fix:
To ensure that no sensitive or potentially sensitive information from user prompts is inadvertently written to the .env file, we should restrict the cleartext contents of that file to strictly non-sensitive configuration. Specifically, only include information that is not secret (such as generic flags or non-sensitive identifiers), and strictly avoid writing API keys, credentials, or potentially sensitive user input. Additionally, if there is any doubt about information sensitivity, default to not writing it, and update documentation and onscreen messages to remind users how and where to set required secrets.

Detailed fix:

  • In _generate_files, make sure that the lines written to .env only include non-sensitive settings. Project and region names, while not as sensitive as API keys, should be handled per organizational standards. The existing code already avoids writing the API key, but we can reinforce this by refactoring the block to:
    • Write only GOOGLE_GENAI_USE_VERTEXAI to .env.
    • Avoid writing GOOGLE_CLOUD_PROJECT and GOOGLE_CLOUD_LOCATION as well, replacing this with a message instructing the user to set them as environment variables. This maximizes security and is consistent with the handling of GOOGLE_API_KEY.
    • Update the warning message to mention all required environment variables (API key, project, location).

Code changes required:

  • In file src/google/adk/cli/cli_create.py, lines associated with writing "GOOGLE_CLOUD_PROJECT=..." and "GOOGLE_CLOUD_LOCATION=..." should be commented or removed (lines 198–200).
  • Update the warning message (lines 193–196) to mention all variables that need to be set by the user.
  • Ensure only GOOGLE_GENAI_USE_VERTEXAI=0 or =1 is written to the .env file.
  • No additional imports or external packages are required.

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

…nsitive information

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@venkateshpabbati venkateshpabbati marked this pull request as ready for review September 8, 2025 07:24
@venkateshpabbati venkateshpabbati merged commit 29dd3c3 into main Sep 8, 2025
3 checks passed
venkateshpabbati pushed a commit that referenced this pull request Nov 8, 2025
Merge google#2582

Relate to google#3306

## Description
Fixes issues #6 and google#1785 where ADK commands crash on Windows due to symlink creation requiring admin privileges.

## Problem
On Windows, running ADK commands (like `adk run`) fails with `OSError: [WinError 1314] A required privilege is not held by the client: ...` because the logging system attempts to create a symlink for the latest log file. Windows requires administrator privileges for symlink creation by default (unless Developer Mode is enabled), causing crashes for non-admin users.

## Root Cause
The issue was in [`logs.py`](https://github.com/google/adk-python/blob/main/src/google/adk/cli/utils/logs.py#L72) where `os.symlink()` was called unconditionally without error handling, causing the entire CLI to crash when symlink creation failed.

## Solution
This PR implements graceful symlink handling. Changes made:
- Extracted symlink creation into separate function with error handling
- Added graceful fallback when symlink creation fails
- Replaced crashes with warnings to keep CLI functional
- Improved user messaging about log file locations

This solution follows a similar pattern to the one used in [ROS2](https://github.com/ros2) in its [logging module](https://github.com/ros2/launch/blob/rolling/launch/launch/logging/__init__.py#L93).

## Testing
### Before (broken)
```bash
> adk run my_agent
Log setup complete: C:\Users\username\AppData\Local\Temp\agents_log\agent.20250817_215119.log
Traceback (most recent call last):
  File "google\adk\cli\utils\logs.py", line 72, in log_to_tmp_folder
    os.symlink(log_filepath, latest_log_link)
OSError: [WinError 1314] A required privilege is not held by the client
```

### After (fixed)
```bash
> adk run my_agent
Log setup complete: C:\Users\username\AppData\Local\Temp\agents_log\agent.20250817_215319.log
UserWarning: Cannot create symlink for latest log file: [WinError 1314] A required privilege is not held by the client
To access latest log: tail -F C:\Users\username\AppData\Local\Temp\agents_log\agent.20250817_215319.log
Running agent my_agent, type exit to exit.
```

Co-authored-by: Wei Sun (Jack) <[email protected]>
COPYBARA_INTEGRATE_REVIEW=google#2582 from lorenzofavaro:fix/windows-symlink-permissions 1e99a62
PiperOrigin-RevId: 828690483
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