Repository navigation
Set provider flags to 0 in Claude Code so a shell with CLAUDE_CODE_USE_BEDROCK=1 can't bypass the gateway - #1356
Open
jcampabadal-db wants to merge 4 commits into
Conversation
…K can't bypass the gateway Claude Code selects Bedrock, Vertex, Foundry, Anthropic AWS, Anthropic Google Cloud or Mantle from the CLAUDE_CODE_USE_* flags before it reads ANTHROPIC_BASE_URL. A flag left from a direct-provider setup (CLAUDE_CODE_USE_BEDROCK=1) made `ug claude` send every request straight to that provider instead of Unity AI Gateway, after a successful `ug configure` (ES-2317318). ug now writes each flag as "0" next to the credential blanks from databricks#1287, with the same rules: always in ucode-settings.json (relayed launches included), in the managed file only from `ug configure`, and a true value already in the managed file is kept with a warning, since the managed file outranks ug's settings. `ug doctor` reports a shell flag (info) and a managed-file flag (warn). Co-authored-by: Isaac <no-reply@databricks.com>
jcampabadal-db
requested review from
AarushiShah-db,
lilly-luo and
rohita5l
as code owners
October 29, 2026 15:29
jcampabadal-db
changed the title
Collaborator
|
/ug-review |
Collaborator
|
/integration-tests |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if os.environ.get(name, "").strip().lower() in truthy | ||
| ] | ||
| path = claude.managed_settings_path() | ||
| managed_env = read_json_safe(path).get("env") if path is not None and path.exists() else None |
Contributor
There was a problem hiding this comment.
can we drop the path.exists() here? it runs outside read_json_safe's try, so a root-locked parent dir raises PermissionError and crashes ug doctor. read_json_safe already treats a missing file as {}.
Suggestion:
Suggested change
| managed_env = read_json_safe(path).get("env") if path is not None and path.exists() else None | |
| managed_env = read_json_safe(path).get("env") if path is not None else None |
| managed = [ | ||
| name | ||
| for name in claude.CLAUDE_PROVIDER_ENV_KEYS | ||
| if isinstance(managed_env.get(name), str) and managed_env[name].strip().lower() in truthy |
Contributor
There was a problem hiding this comment.
claude.py already does this check in _takes_claude_off_gateway. can we expose one public helper (e.g. provider_flag_enabled(value)) and use it for both lists, so the truthy rules stay the same in both places?
Contributor
Polly AI ReviewBlocking issues
Security vulnerabilitiesNone identified in the changed code. Non-blocking notes
ApproachThe production approach follows the existing credential-neutralization path. Sharing the neutral-value map across overlay generation, managed ownership, and conflict handling avoids a separate configuration mechanism. SummaryDiff size (computed): 8 files; +455 / -14 lines (469 changed text lines); 0 binary files.
Shares use additions + deletions, including generated text and lockfiles. Binary files have no line count. Tests include fixtures/helpers under test paths and colocated test/spec files; categories are path-based, not coverage measurements. ChangesThe PR prevents inherited Claude provider flags from bypassing the gateway. It extends the existing settings reconciliation, adds diagnostic advice, documents ownership, and includes unit/component tests plus native-process coverage and CI wiring. No dependencies or generated assets change. The implementation is proportional to the goal. The main reduction opportunity is duplicated lifecycle setup and scenarios, detailed below. TestsTest-by-test assessmentThe frozen snapshot checksum and both trusted policy files were verified. No PR code or tests were executed; reported passes and pre-existing failures remain unverified.
Missing coverage: add a doctor caller regression where the managed path’s existence check raises When consolidating, extend the existing relayed-launch and configure → launch rewrite → revert scenarios in To verify after fixing the caller, run in a secret-free checkout with dependencies installed: uv run --no-sync pytest tests/test_claude_provider_env.py tests/test_claude_credential_blanks.py tests/test_doctor.py tests/test_config_io.py -qScopeAll changed files support the stated provider-neutralization outcome, including diagnostics, ownership documentation, and CI coverage. No unrelated changes or unresolved scope questions were found. Automated review by Polly · workflow run |
…re the provider-flag check
- doctor: remove the `path.exists()` guard before read_json_safe. On Python 3.12 it raises
PermissionError when the managed settings directory is not readable, and ug doctor crashed.
read_json_safe already reads a missing or unreadable file as {}. Add a regression test that
makes Path.stat raise PermissionError.
- claude: add provider_flag_enabled(value) and use it in _takes_claude_off_gateway and in the
doctor check, so both use the same truthy rule.
- tests: list the six provider flag names in test_claude_provider_env.py instead of taking them
from CLAUDE_PROVIDER_ENV_KEYS, so a flag dropped from that tuple fails the tests.
- claude: remove the ticket reference from the provider-flag comment.
Co-authored-by: Isaac <no-reply@databricks.com>
…ude_credential_blanks.py The provider flags use the same write path and rules as the credential blanks, so the lifecycle tests now check both groups together (NEUTRAL = blanks + zeros): overlay, configure writes, plain launch, second writer, conflicts, relayed launch, skip mode, revert, and admin passthrough. The admin-value test also covers a true provider flag, and the credential-warning test checks that the provider-flag text is not used. The two provider-only cases stay as separate tests: a true flag is kept with a warning, and a false flag is replaced. tests/test_claude_provider_env.py is removed and tests/README.md describes the merged file. Co-authored-by: Isaac <no-reply@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What did you change, and why?
Claude Code selects its provider from the
CLAUDE_CODE_USE_*flags before it readsANTHROPIC_BASE_URL. A flag left over from a direct-provider setup (for exampleCLAUDE_CODE_USE_BEDROCK=1in a shell profile) makesug claudesend every request straight to that provider instead of Unity AI Gateway, even after a successfulug configure. The customer in ES-2317318 had toexport CLAUDE_CODE_USE_BEDROCK=0by hand to makeug claudework.ug now writes the six provider flags (
CLAUDE_CODE_USE_BEDROCK,_VERTEX,_FOUNDRY,_ANTHROPIC_AWS,_ANTHROPIC_GOOGLE_CLOUD,_MANTLE) as"0". It uses the same path and rules as the credential blanks from #1287:ucode-settings.json, relayed launches included. The--settingsoverlay outranks the shell env and usersettings.json, so an inherited=1no longer wins.ug configure.1/true/yes/on, case-insensitive) that someone else put in the managed file is kept, with a warning, because the managed file outranks ug's settings. A false value is replaced with"0".ug doctoradds one check:infowhen a flag is set in the shell (ug ignores it),warnwhen the managed file has a true flag (ug cannot override it).Files:
src/ucode/agents/claude.py(CLAUDE_PROVIDER_ENV_KEYS,CLAUDE_NEUTRAL_ENV,_settle_credential_blanksloop),src/ucode/doctor.py(_check_claude_provider_env, which uses the sharedclaude.provider_flag_enabled),AGENTS.md(managed-fields table row),tests/README.md, and the native CI job in.github/workflows/ci.ymlnow also runstests/native_claude/test_provider_env.py.How do you know it works?
tests/test_claude_credential_blanks.py(overlay, configure writes, plain launch, second writer, conflicts, relayed launch, skip mode, revert, passthrough), which now check both groups. Provider-only cases: a true admin value is kept with a warning, and a false value is replaced. The six flag names are listed in the test, so a flag dropped fromCLAUDE_PROVIDER_ENV_KEYSfails 18 tests.tests/test_doctor.pyhas 5 new tests, including a regression for an unreadable managed directory.tests/native_claude/test_provider_env.pyruns realclaude -pagainst a fake server for Bedrock and Vertex. With the flag zeroed the request stays on/v1/messages; the control case without the overlay reaches the provider path. All 8 pass on the CI-pinned Claude Code 2.1.290.CLAUDE_CODE_USE_BEDROCK=1and withCLAUDE_CODE_USE_VERTEX=1exported in the shell.ucode-settings.jsonshows all six flags as"0", andug doctorshows the info line. Onmain, the same setup withCLAUDE_CODE_USE_BEDROCK=1fails with a Bedrock400 The provided model identifier is invalid.ruff check,ruff format --checkandty check src/pass. The full suite has 6 failures intest_e2e_user_agentandtest_smart_router; the same tests fail on cleanmainin the same environment.This pull request and its description were written by Isaac.