diff --git a/coworker/providers/bedrock_provider.py b/coworker/providers/bedrock_provider.py index 50d6b5fc..a1c76010 100644 --- a/coworker/providers/bedrock_provider.py +++ b/coworker/providers/bedrock_provider.py @@ -12,11 +12,18 @@ An id with no family segment falls back to Converse as-is — Converse serves ev model (including Claude, minus the native extras), so a raw model id pasted without the add-model dropdown still works. -Credentials, in order: -1. A **Bedrock API key** (bearer token from the Bedrock console — the no-CLI path). When - present (field or `AWS_BEARER_TOKEN_BEDROCK` env) it WINS over SigV4 credentials, the - same precedence boto3 applies; mixing both makes `AnthropicBedrock` raise outright. -2. Explicit IAM keys → named profile (covers `aws sso login`) → ambient chain. +Auth is ONE method at a time, selected by the profile's `auth_method` (a segmented choice +in Settings — owner call 2026-07-26, directness over field-precedence rules): + +- `api_key` — a **Bedrock API key** (bearer token from the console, the no-CLI path); + rides `AWS_BEARER_TOKEN_BEDROCK`, which boto3 prefers over SigV4 for Bedrock calls. +- `profile` — a named `~/.aws` profile (covers `aws sso login`); blank → the default + credential chain (env vars / ~/.aws / role). +- `iam` — explicit access keys (+ optional STS session token). + +Fields from non-selected methods are dropped at construction, so a stale stored value can +never leak into a different auth path (`AnthropicBedrock` raises outright on a mix). A +missing/unknown method falls back to whichever fields are present, api_key first. boto3 is a lazy import (packaged via the `bedrock` extra) and returns PLAIN DICTS — every response/stream mapping here is dict-shaped, unlike the attribute objects other SDKs return. @@ -449,6 +456,7 @@ class BedrockProvider(ProviderClient): self, *, region: Optional[str] = None, + auth_method: Optional[str] = None, bedrock_api_key: Optional[str] = None, profile_name: Optional[str] = None, access_key_id: Optional[str] = None, @@ -457,6 +465,14 @@ class BedrockProvider(ProviderClient): claude_client: Optional[ProviderClient] = None, converse_client: Optional[ProviderClient] = None, ): + # Narrow to the selected auth method here, once — stale values stored under a + # previously-selected method must never reach a different credential path. + if auth_method == "api_key": + profile_name = access_key_id = secret_access_key = session_token = None + elif auth_method == "profile": + bedrock_api_key = access_key_id = secret_access_key = session_token = None + elif auth_method == "iam": + bedrock_api_key = profile_name = None self._region = region self._bedrock_api_key = bedrock_api_key self._profile_name = profile_name diff --git a/coworker/providers/registry.py b/coworker/providers/registry.py index 50f305e3..9064eb93 100644 --- a/coworker/providers/registry.py +++ b/coworker/providers/registry.py @@ -43,6 +43,12 @@ class ProviderField: # Pre-filled (still editable) form value — e.g. an OpenAI-compatible vendor's official # endpoint, so the user only has to paste a key. Distinct from `placeholder` (grey hint). default: str = "" + # Non-empty → the field renders as a segmented choice control instead of a text input; + # each option is {"value", "label"}. The chosen value is stored like any field value. + choices: tuple = () + # {"other_field_key": "value"} → the field only renders while that other field holds + # that value. Drives auth-method switching (Bedrock) without a per-provider form. + show_when: Optional[dict] = None def to_dict(self) -> dict[str, Any]: return { @@ -53,6 +59,8 @@ class ProviderField: "help": self.help, "placeholder": self.placeholder, "default": self.default, + "choices": [dict(c) for c in self.choices], + "show_when": self.show_when, } @@ -140,6 +148,7 @@ def _build_bedrock(profile: dict[str, Any], secrets: Any) -> ProviderClient: return BedrockProvider( region=get("region"), + auth_method=get("auth_method"), bedrock_api_key=get("bedrock_api_key"), profile_name=get("aws_profile"), access_key_id=get("aws_access_key_id"), @@ -300,44 +309,61 @@ DESCRIPTORS: list[ProviderDescriptor] = [ placeholder="us-east-1", help="The region your Bedrock model access is enabled in.", ), + # One auth method at a time (owner call 2026-07-26): AWS users are advanced — + # a direct choice beats a pile of "(optional)" fields with hidden precedence. + ProviderField( + "auth_method", + "Connect with", + secret=False, + required=False, # the default stands in; builder tolerates absence + default="api_key", + choices=( + {"value": "api_key", "label": "Bedrock API key"}, + {"value": "profile", "label": "AWS profile"}, + {"value": "iam", "label": "IAM keys"}, + ), + ), ProviderField( "bedrock_api_key", - "Bedrock API key (optional)", + "Bedrock API key", secret=True, required=False, placeholder="ABSK…", - help="The easiest way in: generate one on the Bedrock console — no AWS " - "CLI or IAM setup needed. Takes precedence over the fields below.", + show_when={"auth_method": "api_key"}, + help="Generate one on the Bedrock console — no AWS CLI or IAM setup needed.", ), ProviderField( "aws_profile", - "AWS profile (optional)", + "AWS profile", secret=False, required=False, placeholder="default", + show_when={"auth_method": "profile"}, help="A named profile from ~/.aws — works with `aws configure` and " - "`aws sso login` (IAM Identity Center). Leave blank to use explicit " - "keys below, or the default credential chain.", + "`aws sso login` (IAM Identity Center). Leave blank to use your " + "default AWS credentials (env vars or ~/.aws).", ), ProviderField( "aws_access_key_id", - "Access key ID (optional)", + "Access key ID", secret=False, required=False, placeholder="AKIA…", + show_when={"auth_method": "iam"}, ), ProviderField( "aws_secret_access_key", - "Secret access key (optional)", + "Secret access key", secret=True, required=False, + show_when={"auth_method": "iam"}, ), ProviderField( "aws_session_token", - "Session token (optional)", + "Session token (STS only, optional)", secret=True, required=False, - help="Only for temporary credentials (STS).", + show_when={"auth_method": "iam"}, ), ], build=_build_bedrock, @@ -560,19 +586,35 @@ def _verify_bedrock(fields: dict[str, Any], timeout: float) -> dict[str, Any]: "ok": False, "error": "boto3 is not installed — `pip install 'openworker[bedrock]'`.", } + # Exactly one auth method is exercised — the one the form has selected. Per-method + # required fields are checked here so the Test button says what's missing. + method = get("auth_method") or "api_key" + if method == "api_key" and not ( + get("bedrock_api_key") or os.environ.get("AWS_BEARER_TOKEN_BEDROCK") + ): + return {"ok": False, "error": "Enter a Bedrock API key to test."} + if method == "iam" and not ( + get("aws_access_key_id") and get("aws_secret_access_key") + ): + return {"ok": False, "error": "Enter an access key ID and secret access key."} try: - # A Bedrock API key rides the env var (boto3's only bearer channel) and then - # wins over any SigV4 credentials, matching the provider's own precedence. - if get("bedrock_api_key"): - os.environ["AWS_BEARER_TOKEN_BEDROCK"] = get("bedrock_api_key") - session = boto3.session.Session( - **_session_kwargs( - get("aws_profile"), + if method == "api_key": + # The key rides the env var (boto3's only bearer channel); bearer then wins + # over any ambient SigV4 credentials for Bedrock calls. + if get("bedrock_api_key"): + os.environ["AWS_BEARER_TOKEN_BEDROCK"] = get("bedrock_api_key") + session_kwargs: dict[str, Any] = {} + elif method == "profile": + # Blank profile → the default credential chain (env vars / ~/.aws / role). + session_kwargs = _session_kwargs(get("aws_profile"), None, None, None) + else: # iam + session_kwargs = _session_kwargs( + None, get("aws_access_key_id"), get("aws_secret_access_key"), get("aws_session_token"), ) - ) + session = boto3.session.Session(**session_kwargs) client = session.client( "bedrock", region_name=get("region"), diff --git a/surfaces/gui/src/api.ts b/surfaces/gui/src/api.ts index b659988d..516b6662 100644 --- a/surfaces/gui/src/api.ts +++ b/surfaces/gui/src/api.ts @@ -1265,6 +1265,8 @@ export interface ProviderField { help: string; placeholder: string; default?: string; // pre-filled editable value (e.g. an OpenAI-compatible vendor's endpoint) + choices?: { value: string; label: string }[]; // non-empty → segmented choice, not a text input + show_when?: Record | null; // render only while these fields hold these values } export interface ProviderInfo { diff --git a/surfaces/gui/src/providers/ProviderSetup.test.tsx b/surfaces/gui/src/providers/ProviderSetup.test.tsx new file mode 100644 index 00000000..870aa7a0 --- /dev/null +++ b/surfaces/gui/src/providers/ProviderSetup.test.tsx @@ -0,0 +1,96 @@ +// Auth-method segmented choice + show_when field visibility (Bedrock's "Connect with"): +// only the selected method's fields render, and clicking a segment switches them. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { ProviderForm, type ProviderSetupState } from "./ProviderSetup"; +import type { ProviderInfo } from "../api"; + +vi.mock("../tauri", () => ({ openExternal: vi.fn() })); + +afterEach(cleanup); + +const BEDROCK: ProviderInfo = { + name: "bedrock", + title: "AWS Bedrock", + needs_key: true, + configured: false, + values: {}, + suggested_models: [], + recommended_model: null, + fields: [ + { key: "region", label: "AWS region", secret: false, required: true, help: "", placeholder: "us-east-1" }, + { + key: "auth_method", + label: "Connect with", + secret: false, + required: false, + help: "", + placeholder: "", + default: "api_key", + choices: [ + { value: "api_key", label: "Bedrock API key" }, + { value: "profile", label: "AWS profile" }, + { value: "iam", label: "IAM keys" }, + ], + }, + { key: "bedrock_api_key", label: "Bedrock API key", secret: true, required: false, help: "", placeholder: "ABSK…", show_when: { auth_method: "api_key" } }, + { key: "aws_profile", label: "AWS profile", secret: false, required: false, help: "", placeholder: "default", show_when: { auth_method: "profile" } }, + { key: "aws_secret_access_key", label: "Secret access key", secret: true, required: false, help: "", placeholder: "", show_when: { auth_method: "iam" } }, + ], +}; + +function makePs(fields: Record, setFieldValue = vi.fn()): ProviderSetupState { + return { + providers: [BEDROCK], + ordered: [BEDROCK], + refreshProviders: async () => {}, + sel: "bedrock", + info: BEDROCK, + fields, + setFieldValue, + dirty: false, + verify: { state: "idle" }, + showEndpoint: false, + setShowEndpoint: () => {}, + keylessOk: new Set(), + credentialed: false, + savedState: false, + secretFilled: true, + openProvider: () => {}, + backToGallery: () => {}, + runTestAndSave: async () => true, + removeKey: async () => {}, + cancelBackTimer: () => {}, + statusFor: () => null, + saveField: async () => {}, + fieldSaved: null, + }; +} + +describe("ProviderForm auth-method choice", () => { + it("renders only the selected method's fields", () => { + render(); + expect(screen.getByTestId("t-field-bedrock_api_key")).toBeTruthy(); + expect(screen.queryByTestId("t-field-aws_profile")).toBeNull(); + expect(screen.queryByTestId("t-field-aws_secret_access_key")).toBeNull(); + expect(screen.getByTestId("t-choice-auth_method-api_key").getAttribute("aria-checked")).toBe("true"); + }); + + it("switching the segment swaps the visible fields", () => { + const setFieldValue = vi.fn(); + const { rerender } = render( + , + ); + fireEvent.click(screen.getByTestId("t-choice-auth_method-profile")); + expect(setFieldValue).toHaveBeenCalledWith("auth_method", "profile"); + rerender(); + expect(screen.getByTestId("t-field-aws_profile")).toBeTruthy(); + expect(screen.queryByTestId("t-field-bedrock_api_key")).toBeNull(); + }); + + it("iam segment shows the key-pair fields", () => { + render(); + expect(screen.getByTestId("t-field-aws_secret_access_key")).toBeTruthy(); + expect(screen.queryByTestId("t-field-bedrock_api_key")).toBeNull(); + }); +}); diff --git a/surfaces/gui/src/providers/ProviderSetup.tsx b/surfaces/gui/src/providers/ProviderSetup.tsx index 051a1804..27e522e2 100644 --- a/surfaces/gui/src/providers/ProviderSetup.tsx +++ b/surfaces/gui/src/providers/ProviderSetup.tsx @@ -341,6 +341,41 @@ export function ProviderForm({ // A keyed provider's base_url is an expert option — it renders BELOW the key-help // line as its own advanced section (owner nit 2026-07-19), not inside the loop. if (f.key === "base_url" && keyed) return null; + // Conditional fields (Bedrock's per-auth-method inputs) render only while their + // controlling field holds the matching value. + if (f.show_when && !Object.entries(f.show_when).every(([k, v]) => (ps.fields[k] || "") === v)) + return null; + // Segmented choice (e.g. Bedrock's "Connect with"): a button per option, stored + // like any field value. Switching methods requires a fresh Test to save. + if (f.choices?.length) + return ( +
+ +
+ {f.choices.map((c) => { + const active = (ps.fields[f.key] || "") === c.value; + return ( + + ); + })} +
+ {f.help &&

{f.help}

} +
+ ); // Test lives next to the required secret (the API key) when there is one; cloud // providers whose secrets are all optional (Bedrock, Vertex) test from the first // field instead — their credentials may be ambient (~/.aws, ADC). diff --git a/tests/test_bedrock_provider.py b/tests/test_bedrock_provider.py index a4689c9a..97ed179e 100644 --- a/tests/test_bedrock_provider.py +++ b/tests/test_bedrock_provider.py @@ -312,6 +312,32 @@ def test_claude_family_prefers_bedrock_api_key_over_sigv4(monkeypatch): assert sub._client.aws_profile is None +def test_auth_method_narrows_out_other_methods_fields(monkeypatch): + """Stale values stored under a previously-selected method must never leak into a + different auth path — the selected method drops everything else at construction.""" + monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False) + p = BedrockProvider( + region="us-east-1", + auth_method="profile", + bedrock_api_key="ABSKstale", + profile_name="work", + access_key_id="AKIAstale", + secret_access_key="stale", + ) + assert p._bedrock_api_key is None + assert p._access_key_id is None + sub = p._family_client("claude") + assert sub._client.api_key is None + assert sub._client.aws_profile == "work" + + p2 = BedrockProvider( + region="us-east-1", auth_method="api_key", bedrock_api_key="ABSKlive", + profile_name="stale", + ) + assert p2._profile_name is None + assert p2._family_client("claude")._client.api_key == "ABSKlive" + + def test_converse_client_publishes_api_key_as_bearer_env(monkeypatch): import os @@ -369,6 +395,7 @@ def test_bedrock_descriptor_and_builder(): keys = [f.key for f in d.fields] assert keys == [ "region", + "auth_method", "bedrock_api_key", "aws_profile", "aws_access_key_id", @@ -378,6 +405,16 @@ def test_bedrock_descriptor_and_builder(): assert [f.key for f in d.fields if f.required] == ["region"] secret = {f.key for f in d.fields if f.secret} assert secret == {"bedrock_api_key", "aws_secret_access_key", "aws_session_token"} + # One auth method at a time: a defaulted segmented choice drives per-method fields. + method = next(f for f in d.fields if f.key == "auth_method") + assert method.default == "api_key" + assert [c["value"] for c in method.choices] == ["api_key", "profile", "iam"] + by_key = {f.key: f for f in d.fields} + assert by_key["bedrock_api_key"].show_when == {"auth_method": "api_key"} + assert by_key["aws_profile"].show_when == {"auth_method": "profile"} + assert by_key["aws_secret_access_key"].show_when == {"auth_method": "iam"} + assert by_key["region"].show_when is None + assert by_key["region"].to_dict()["choices"] == [] # Recommended model is curated in the matrix (set_provider's auto-add depends on it). from coworker.providers.matrix import models_for_provider @@ -444,7 +481,8 @@ def test_verify_bedrock_ok(monkeypatch): captured: dict = {} _patch_session(monkeypatch, _FakeBedrockControl(), captured) out = verify_provider_key( - "bedrock", fields={"region": "us-east-1", "aws_profile": "work"} + "bedrock", + fields={"region": "us-east-1", "auth_method": "profile", "aws_profile": "work"}, ) assert out == {"ok": True} assert captured["service"] == "bedrock" @@ -452,6 +490,52 @@ def test_verify_bedrock_ok(monkeypatch): assert captured["client"]["region_name"] == "us-east-1" +def test_verify_bedrock_per_method_required_fields(monkeypatch): + from coworker.providers.registry import verify_provider_key + + monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False) + out = verify_provider_key( + "bedrock", fields={"region": "us-east-1", "auth_method": "api_key"} + ) + assert not out["ok"] and "Bedrock API key" in out["error"] + out = verify_provider_key( + "bedrock", + fields={"region": "us-east-1", "auth_method": "iam", "aws_access_key_id": "AKIA"}, + ) + assert not out["ok"] and "secret access key" in out["error"] + # Blank profile is fine — it means the default credential chain. + captured: dict = {} + _patch_session(monkeypatch, _FakeBedrockControl(), captured) + out = verify_provider_key( + "bedrock", fields={"region": "us-east-1", "auth_method": "profile"} + ) + assert out == {"ok": True} + assert captured["session"] == {} + + +def test_verify_bedrock_ignores_other_methods_stale_fields(monkeypatch): + from coworker.providers.registry import verify_provider_key + + monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False) + captured: dict = {} + _patch_session(monkeypatch, _FakeBedrockControl(), captured) + out = verify_provider_key( + "bedrock", + fields={ + "region": "us-east-1", + "auth_method": "iam", + "aws_access_key_id": "AKIA1", + "aws_secret_access_key": "sec", + "aws_profile": "stale", # other method's leftover — must not be used + }, + ) + assert out == {"ok": True} + assert captured["session"] == { + "aws_access_key_id": "AKIA1", + "aws_secret_access_key": "sec", + } + + def test_verify_bedrock_api_key_rides_the_bearer_env(monkeypatch): import os @@ -478,7 +562,9 @@ def test_verify_bedrock_maps_client_errors(monkeypatch): "ListFoundationModels", ) _patch_session(monkeypatch, _FakeBedrockControl(exc=denied), {}) - out = verify_provider_key("bedrock", fields={"region": "us-east-1"}) + out = verify_provider_key( + "bedrock", fields={"region": "us-east-1", "auth_method": "profile"} + ) assert not out["ok"] and "Bedrock access" in out["error"] bad_key = ClientError( @@ -486,5 +572,7 @@ def test_verify_bedrock_maps_client_errors(monkeypatch): "ListFoundationModels", ) _patch_session(monkeypatch, _FakeBedrockControl(exc=bad_key), {}) - out = verify_provider_key("bedrock", fields={"region": "us-east-1"}) + out = verify_provider_key( + "bedrock", fields={"region": "us-east-1", "auth_method": "profile"} + ) assert not out["ok"] and "rejected" in out["error"]