[codex] Fix dotenv credential precedence#358
Conversation
|
@codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production. |
There was a problem hiding this comment.
Pull request overview
This PR fixes credential resolution precedence to prevent YANDEX_DIRECT_TOKEN / YANDEX_DIRECT_LOGIN loaded from a CWD .env (or other environment sources) from silently overriding an active OAuth auth profile, while still allowing explicit command-line --token/--login to override everything. It also adds regression coverage for the precedence rules and applies repo-wide ruff format changes so formatting checks pass.
Changes:
- Update root CLI credential handling to pass only command-line supplied root credential options into
get_credentials()(preventing env/.env from overriding an active profile). - Add/extend tests covering: active profile + CWD
.env, no profile + CWD.env, explicit--token/--login, and the login-only contract (balance). - Apply
ruff formatnormalization across tests, scripts, and command modules.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
direct_cli/cli.py |
Implements “command-line only” root credential overrides using ParameterSource and preserves login-only behavior when appropriate. |
tests/test_auth_oauth.py |
Adds regression tests for profile vs CWD .env precedence and explicit CLI override behavior. |
tests/test_balance.py |
Adds coverage for the login-only contract when no active profile is present. |
tests/test_env_loading.py |
Normalizes string formatting in .env loading tests (supports the new precedence coverage). |
tests/test_wsdl_parity_gate.py |
Formatting-only updates (assert/message layout). |
tests/test_v5_live_write.py |
Formatting-only updates (assert/message layout). |
tests/test_v4account.py |
Formatting-only updates (invoke/assert layout). |
tests/test_v4_live_contracts.py |
Formatting-only updates (assert/message layout). |
tests/test_v4_exit_codes.py |
Formatting-only updates (uses parenthesized with (...) for patches). |
tests/test_v4_contracts.py |
Formatting-only updates (inline list literal). |
tests/test_transport_contract.py |
Formatting-only updates (assert/message layout). |
tests/test_integration.py |
Formatting-only updates (assert message concatenation; blank line normalization). |
tests/test_integration_write.py |
Formatting-only updates (assert/skip message layout). |
tests/test_cli.py |
Formatting-only updates (list comprehension layout). |
tests/test_auth_op.py |
Formatting-only updates (test signature wrapping). |
tests/test_auth_bw.py |
Formatting-only updates (test signature wrapping). |
tests/test_api_coverage.py |
Formatting-only updates (assert/message layout and tuple formatting). |
tests/conftest.py |
Formatting-only updates (regex/substitution formatting and fixture invocation layout). |
direct_cli/commands/v4account.py |
Formatting-only updates (function definitions, option decorators, and call layout). |
direct_cli/commands/strategies.py |
Formatting-only update (error message f-string). |
direct_cli/commands/retargeting.py |
Formatting-only update (field_names assignment wrapping). |
direct_cli/commands/keywords.py |
Formatting-only update (error message f-string). |
direct_cli/commands/keywordbids.py |
Formatting-only update (dict assignment wrapping). |
direct_cli/commands/dynamicads.py |
Formatting-only update (error message concatenation). |
direct_cli/commands/campaigns.py |
Formatting-only updates (f-string, help string, ternary wrapping). |
direct_cli/commands/ads.py |
Formatting-only update (help/error message concatenation). |
direct_cli/commands/adgroups.py |
Formatting-only updates (help string concatenation). |
direct_cli/utils.py |
Formatting-only updates (f-strings and message wrapping). |
direct_cli/v4_contracts.py |
Formatting-only updates (string concatenation). |
scripts/sandbox_write_live.py |
Formatting-only update (string concatenation). |
scripts/refresh_wsdl_cache.py |
Formatting-only update (wrapped print call). |
scripts/patch_vendor_imports.py |
Formatting-only update (single-line f-string). |
scripts/check_wsdl_drift.py |
Formatting-only update (single-line conditional return). |
scripts/check_reports_drift.py |
Formatting-only update (dict literal wrapping). |
scripts/build_api_coverage_report.py |
Formatting-only update (lambda body wrapping). |
scripts/anonymize_cassettes.py |
Formatting-only updates (spacing, regex wrapping, and line wrapping). |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
.envloading supported while preventing baseYANDEX_DIRECT_TOKEN/YANDEX_DIRECT_LOGINfrom silently overriding an active auth profile.env, no active profile + CWD.env, explicit--token/--login, and the balance login-only contractruff formatsoruff format --check .passesFixes #356.
Validation
python3 -m pytest tests/test_env_loading.py tests/test_auth_oauth.py tests/test_cli.py tests/test_balance.py -q-> 115 passed, 30 subtests passedpython3 -m pytest -q-> 1329 passed, 44 skipped, 30 subtests passedpython3 -m ruff format --check .-> 109 files already formattedpython3 -m ruff check .-> All checks passedmypy .-> Success: no issues found in 56 source filesgit diff --check-> passeddirect adextensions get --types CALLOUT --format jsonfrom both plugin CWD and direct-cli CWD returned the same 3 ids: 19718116, 19718117, 19718118