From ffc4b5fa6767beb52ead34a0cbb02813a6c0570e Mon Sep 17 00:00:00 2001 From: John Gruber Date: Mon, 24 Aug 2026 06:52:52 -0500 Subject: [PATCH 1/3] fix(#191): validate credential-template provider against the injection set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A credential template accepted any string as `provider`. Only a handful of literals are ever matched when credentials are resolved (`aws`, `ibm`, `gcp`, `azure`, `ssh`), so a natural misspelling like `provider="ibmcloud"` — which matches every adjacent field name (`ibmcloud_api_key`, `ibmcloud_resource_group`) — was stored happily, read back looking healthy, and then matched NO branch in the resolver. The template silently contributed nothing, the deploy fell through to global `.env` credentials that weren't there, and the failure surfaced far away as an opaque Terraform "BearerToken property is required" error. Root cause: `provider` was never validated at create/update, and the canonical set it must belong to was implicit, scattered across the resolver's `if template.provider == ...` branches. Fix: - Add `SUPPORTED_PROVIDERS = {aws, gcp, azure, ibm, ssh}` as the single source of truth in credential_template_service.py, documented against each consumer that injects/resolves credentials for that provider. - Validate `provider` in `create_template` and `update_template` (service layer, covers every caller) -> BadRequestError with an enumerated message. - Add matching Pydantic validation on the create/update route models so the API boundary returns a clean 422 naming the bad value and the supported set, before the service is reached. Update-time validation only fires when the caller is actually changing `provider`. Tests lock: the exact `ibmcloud` misspelling is rejected at both the service (400) and route (422) layers; every canonical provider is still accepted; an update can't switch a template onto a no-op provider; and a resolver-contract test documents that a pre-existing `ibmcloud` row injects nothing (the behavior the validation now prevents from being created). Mutation-tested: reverting the guards reds 5 tests. Closes #191 Claude-Session: https://claude.ai/code/session_01UpRYiFserdBE5ESHn759N4 --- backend/routes/credential_templates.py | 17 ++++- .../services/credential_template_service.py | 43 +++++++++++ .../test_credential_template_service.py | 73 +++++++++++++++++++ .../component/test_credentials_service.py | 34 +++++++++ .../test_routes_credential_templates.py | 21 ++++++ 5 files changed, 187 insertions(+), 1 deletion(-) diff --git a/backend/routes/credential_templates.py b/backend/routes/credential_templates.py index 64be9be6..12c8e8ac 100644 --- a/backend/routes/credential_templates.py +++ b/backend/routes/credential_templates.py @@ -14,9 +14,14 @@ from core.errors import handle_route_errors from database import get_db from routes.auth import require_operator, require_viewer -from services.credential_template_service import CredentialTemplateService +from services.credential_template_service import ( + SUPPORTED_PROVIDERS, + CredentialTemplateService, +) from utils.validators import validate_aws_region +_SUPPORTED_PROVIDERS_MSG = ", ".join(sorted(SUPPORTED_PROVIDERS)) + logger = logging.getLogger(__name__) router = APIRouter(prefix="/api/credential-templates", tags=["credential-templates"]) @@ -62,6 +67,10 @@ class CredentialTemplateBase(BaseModel): @model_validator(mode="after") def _validate_regions(self): + if self.provider not in SUPPORTED_PROVIDERS: + raise ValueError( + f"Unsupported provider '{self.provider}'. Must be one of: {_SUPPORTED_PROVIDERS_MSG}." + ) if self.provider == "aws": validate_aws_region(self.region, field_name="region") validate_aws_region(self.aws_sso_region, field_name="aws_sso_region") @@ -110,6 +119,12 @@ class CredentialTemplateUpdate(BaseModel): @model_validator(mode="after") def _validate_regions(self): + # provider is optional on update; only validate when the caller is + # actually changing it, so an unknown value can't be persisted (issue #191). + if self.provider is not None and self.provider not in SUPPORTED_PROVIDERS: + raise ValueError( + f"Unsupported provider '{self.provider}'. Must be one of: {_SUPPORTED_PROVIDERS_MSG}." + ) validate_aws_region(self.aws_sso_region, field_name="aws_sso_region") if self.region and self.provider == "aws": validate_aws_region(self.region, field_name="region") diff --git a/backend/services/credential_template_service.py b/backend/services/credential_template_service.py index 7fb2d4ef..fcd4d40a 100644 --- a/backend/services/credential_template_service.py +++ b/backend/services/credential_template_service.py @@ -25,6 +25,38 @@ logger = logging.getLogger(__name__) +# Canonical set of providers a credential template may declare. +# +# This is the single source of truth for provider validation. It must stay in +# lock-step with every consumer that branches on ``template.provider`` to inject +# or resolve credentials, otherwise a template can be created that looks healthy +# in the API yet contributes no credentials at deploy time (see issue #191): +# - ``aws`` -> AWS_* env + TF_VAR_* mirror (credentials_service, injection) +# - ``ibm`` -> IC_API_KEY / IBMCLOUD_API_KEY (credentials_service, injection) +# - ``gcp`` -> GCP service-account JSON (credentials_service.get_gcp_service_account_info) +# - ``azure`` -> Azure credential resolution (execution.engine_router) +# - ``ssh`` -> SSH tunnel / on-prem (credential test + tunnel manager) +# These are exactly the four cloud providers the UI offers plus the legacy +# ``ssh`` on-prem provider. Adding a new provider here without wiring its +# injection path (or vice-versa) is the bug this constant exists to prevent. +SUPPORTED_PROVIDERS: frozenset[str] = frozenset({"aws", "gcp", "azure", "ibm", "ssh"}) + + +def validate_provider(provider: Any) -> str: + """Return ``provider`` if it is a supported credential-template provider. + + Raises ``BadRequestError`` with a clear, enumerated message otherwise so a + misspelled or unknown value (e.g. ``"ibmcloud"``) is rejected at the point + of the mistake instead of silently injecting nothing later. + """ + if provider not in SUPPORTED_PROVIDERS: + supported = ", ".join(sorted(SUPPORTED_PROVIDERS)) + raise BadRequestError( + f"Unsupported credential-template provider '{provider}'. " + f"Must be one of: {supported}." + ) + return provider + class _AwsTestError(Exception): """Carrier object mimicking botocore ClientError shape for cloud observation. @@ -215,6 +247,11 @@ def get_template(self, template_id: int) -> dict: def create_template(self, template_data) -> dict: """Create a new credential template.""" + # Reject unknown/misspelled providers before persisting: a template whose + # provider matches no injection path is created "successfully" yet + # contributes no credentials at deploy time (issue #191). + validate_provider(template_data.provider) + # Duplicate name check existing = self.db.query(CloudCredentialTemplate).filter( CloudCredentialTemplate.name == template_data.name @@ -305,6 +342,12 @@ def update_template(self, template_id: int, template_data) -> dict: update_data = template_data.model_dump(exclude_unset=True) + # If the caller is changing the provider, hold it to the same canonical + # set as create so an update can't move a template onto a value that + # injects nothing (issue #191). A None/absent provider leaves it unchanged. + if update_data.get("provider") is not None: + validate_provider(update_data["provider"]) + # Handle encrypted fields encrypted_map = { "aws_secret_access_key": "aws_secret_access_key_encrypted", diff --git a/backend/tests/component/test_credential_template_service.py b/backend/tests/component/test_credential_template_service.py index a639ae87..a5b1622f 100644 --- a/backend/tests/component/test_credential_template_service.py +++ b/backend/tests/component/test_credential_template_service.py @@ -237,6 +237,79 @@ def test_ibm_resource_group_defaults_to_default(self, db): assert result["ibmcloud_resource_group"] == "default" +# --------------------------------------------------------------------------- +# provider validation (issue #191) +# --------------------------------------------------------------------------- + +class TestProviderValidation: + """A credential template's provider must match a real injection path. + + Regression guard for issue #191: ``provider="ibmcloud"`` (a misspelling of + the canonical ``ibm``) used to be stored happily and then inject NO + credentials at deploy time — silently. Every provider the service accepts + must be one a credential-resolution consumer actually handles. + """ + + @patch("services.credential_template_service.get_default", return_value="us-east-1") + def test_create_rejects_misspelled_ibmcloud_provider(self, mock_get_default, db): + """The exact bug from #191: 'ibmcloud' looks right but injects nothing.""" + svc = CredentialTemplateService(db) + with pytest.raises(BadRequestError, match="Unsupported credential-template provider 'ibmcloud'"): + svc.create_template(_make_template_data(name="ibm-roks", provider="ibmcloud")) + # And nothing was persisted. + assert db.query(CloudCredentialTemplate).filter( + CloudCredentialTemplate.name == "ibm-roks" + ).first() is None + + @patch("services.credential_template_service.get_default", return_value="us-east-1") + def test_create_rejects_unknown_provider_with_enumerated_message(self, mock_get_default, db): + svc = CredentialTemplateService(db) + with pytest.raises(BadRequestError) as exc: + svc.create_template(_make_template_data(name="bogus", provider="digitalocean")) + msg = str(exc.value) + # Message names the offender and enumerates the supported set. + assert "digitalocean" in msg + for supported in ("aws", "azure", "gcp", "ibm", "ssh"): + assert supported in msg + + @patch("services.credential_template_service.get_default", return_value="us-east-1") + def test_create_rejects_empty_provider(self, mock_get_default, db): + svc = CredentialTemplateService(db) + with pytest.raises(BadRequestError): + svc.create_template(_make_template_data(name="empty-prov", provider="")) + + @pytest.mark.parametrize("provider", ["aws", "gcp", "azure", "ibm", "ssh"]) + @patch("services.credential_template_service.get_default", return_value="us-east-1") + def test_create_accepts_every_supported_provider(self, mock_get_default, provider, db): + """Each canonical provider is accepted — validation matches the injection set.""" + svc = CredentialTemplateService(db) + result = svc.create_template(_make_template_data( + name=f"tpl-{provider}", + provider=provider, + # give IBM its required key; other providers don't need extra fields here + ibmcloud_api_key="ibm-api-key-value" if provider == "ibm" else None, + )) + assert result["provider"] == provider + + def test_update_rejects_switch_to_unknown_provider(self, db): + """An update can't move a healthy template onto a no-op provider.""" + t = _create_template_in_db(db, name="aws-live", provider="aws") + svc = CredentialTemplateService(db) + with pytest.raises(BadRequestError, match="Unsupported credential-template provider 'ibmcloud'"): + svc.update_template(t.id, _make_update_data(provider="ibmcloud")) + # Provider unchanged. + db.refresh(t) + assert t.provider == "aws" + + def test_update_without_provider_change_is_allowed(self, db): + """Omitting provider on update leaves it untouched (no false rejection).""" + t = _create_template_in_db(db, name="aws-keep", provider="aws") + svc = CredentialTemplateService(db) + result = svc.update_template(t.id, _make_update_data(description="just a note")) + assert result["provider"] == "aws" + assert result["description"] == "just a note" + + # --------------------------------------------------------------------------- # list_templates / get_template # --------------------------------------------------------------------------- diff --git a/backend/tests/component/test_credentials_service.py b/backend/tests/component/test_credentials_service.py index b06e474e..28a2a818 100644 --- a/backend/tests/component/test_credentials_service.py +++ b/backend/tests/component/test_credentials_service.py @@ -127,6 +127,40 @@ def test_ibm_template(self, mock_dec, db): assert env["IBMCLOUD_API_KEY"] == "ibm-api-key" assert env["IBMCLOUD_REGION"] == "us-south" + @patch("services.credentials_service.decrypt_value", return_value="ibm-api-key") + def test_misspelled_ibmcloud_provider_injects_nothing(self, mock_dec, db): + """Issue #191 reproduction at the injection layer. + + A template stored with provider='ibmcloud' (the natural misspelling of + the canonical 'ibm', matching every adjacent ibmcloud_* field name) + matches NO branch in the resolver, so it silently contributes no IBM + credentials — the deploy then fails far away with a BearerToken error. + The create/update validation (issue #191) now prevents such a row from + being created; this locks the resolver contract that made it dangerous. + """ + template = _make_template( + db, + name="IBM ROKs Testing", + provider="ibmcloud", # unknown to the resolver -> no-op + region="us-east", + aws_auth_method=None, + aws_access_key_id=None, + aws_secret_access_key_encrypted=None, + ibmcloud_api_key_encrypted="enc_ibm_key", + ) + project = _make_project( + db, + credential_template_id=template.id, + project_type="cloud-aws", + cloud_provider="ibm", + ) + + env = get_cloud_credentials_env(project, db=db) + + # The stored API key never reaches the deployment environment. + assert "IC_API_KEY" not in env + assert "IBMCLOUD_API_KEY" not in env + @patch("services.credentials_service.decrypt_value", side_effect=Exception("Bad key")) def test_ibm_template_decrypt_failure_omits_api_key(self, mock_dec, db): template = _make_template( diff --git a/backend/tests/integration/test_routes_credential_templates.py b/backend/tests/integration/test_routes_credential_templates.py index 09dea4d8..b145f083 100644 --- a/backend/tests/integration/test_routes_credential_templates.py +++ b/backend/tests/integration/test_routes_credential_templates.py @@ -121,6 +121,27 @@ def test_create_ibm_template(self, mock_svc_cls, client, operator_headers, all_t assert data["provider"] == "ibm" assert data["has_ibmcloud_api_key"] is True + @patch("routes.credential_templates.CredentialTemplateService") + def test_create_rejects_misspelled_provider_with_422(self, mock_svc_cls, client, operator_headers, all_test_users): + """Issue #191: provider='ibmcloud' is a 422 at the API boundary, not a + silently-empty template — and the service is never reached.""" + mock_svc = MagicMock() + mock_svc_cls.return_value = mock_svc + + response = client.post( + "/api/credential-templates", + json={"name": "IBM ROKs Testing", "provider": "ibmcloud", + "region": "us-east", "ibmcloud_api_key": "secret"}, + headers=operator_headers, + ) + assert response.status_code == 422 + body = response.json() + # The validation error names the bad provider and the supported set. + detail = str(body["detail"]) + assert "ibmcloud" in detail + assert "ibm" in detail + mock_svc.create_template.assert_not_called() + class TestGetCredentialTemplate: """GET /api/credential-templates/{id}.""" From 79f08cfd97df5585a0ff6bbfe0e1a9877bf5a78b Mon Sep 17 00:00:00 2001 From: John Gruber Date: Mon, 24 Aug 2026 07:10:24 -0500 Subject: [PATCH 2/3] docs(#191): correct the azure/gcp consumer comment per self-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review M3: the SUPPORTED_PROVIDERS comment overstated azure/gcp as general 'credential resolution' — they inject only a post-provision kubeconfig token (AKS via engine_router, GKE via get_gcp_service_account_info), not terraform-env credentials. Only aws/ibm inject into the terraform env, so #191's 'looks healthy, injects nothing' class fully closes for aws/ibm; azure/gcp are still validated but that terraform-env class never applied to them. Comment-only; no behavior change. Claude-Session: https://claude.ai/code/session_01UpRYiFserdBE5ESHn759N4 --- backend/services/credential_template_service.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/backend/services/credential_template_service.py b/backend/services/credential_template_service.py index fcd4d40a..56e8f71c 100644 --- a/backend/services/credential_template_service.py +++ b/backend/services/credential_template_service.py @@ -31,14 +31,19 @@ # lock-step with every consumer that branches on ``template.provider`` to inject # or resolve credentials, otherwise a template can be created that looks healthy # in the API yet contributes no credentials at deploy time (see issue #191): -# - ``aws`` -> AWS_* env + TF_VAR_* mirror (credentials_service, injection) -# - ``ibm`` -> IC_API_KEY / IBMCLOUD_API_KEY (credentials_service, injection) -# - ``gcp`` -> GCP service-account JSON (credentials_service.get_gcp_service_account_info) -# - ``azure`` -> Azure credential resolution (execution.engine_router) +# - ``aws`` -> AWS_* env + TF_VAR_* mirror (credentials_service, terraform-env injection) +# - ``ibm`` -> IC_API_KEY / IBMCLOUD_API_KEY (credentials_service, terraform-env injection) +# - ``gcp`` -> GKE kubeconfig token (credentials_service.get_gcp_service_account_info; post-provision cluster access, not terraform env) +# - ``azure`` -> AKS kubeconfig token (execution.engine_router; post-provision cluster access, not terraform env) # - ``ssh`` -> SSH tunnel / on-prem (credential test + tunnel manager) # These are exactly the four cloud providers the UI offers plus the legacy # ``ssh`` on-prem provider. Adding a new provider here without wiring its -# injection path (or vice-versa) is the bug this constant exists to prevent. +# consumer (or vice-versa) is the bug this constant exists to prevent. NOTE: +# ``aws``/``ibm`` inject credentials into the terraform provisioning env, so +# #191's "looks healthy, injects nothing" class is fully closed for them; +# ``azure``/``gcp`` are consumed only for post-provision cluster access, so a +# mis-set provider is still rejected here but the underlying #191 class for the +# terraform-env path only ever applied to aws/ibm. SUPPORTED_PROVIDERS: frozenset[str] = frozenset({"aws", "gcp", "azure", "ibm", "ssh"}) From 1ad10991bba14a5e51949f60881e9d875c5f8447 Mon Sep 17 00:00:00 2001 From: John Gruber Date: Tue, 8 Sep 2026 13:18:10 -0500 Subject: [PATCH 3/3] fix(#191 r2): constrain provider via Literal to close the 422 body-leak + wire the enum into the contract bonnyr-f5 round-2 (REVISE). Findings 1+2 share one fix; +minor 3. Findings 1 & 2 - replace the provider membership check in the route @model_validator with a Literal field type on both the create and update models: CredentialTemplateBase.provider: Literal["aws","azure","gcp","ibm","ssh"] CredentialTemplateUpdate.provider: Literal[...] | None = None A model-level ValueError made Pydantic attach the ENTIRE request body to the 422 (no RequestValidationError handler in main.py), leaking the plaintext credential to the MCP client / FE toast. A Literal yields a field-scoped error (no body echo) and maps to an OpenAPI enum (INV-3). Mirrors the auth_type idiom at routes/f5_devices.py:73/82. The service-level validate_provider / SUPPORTED_PROVIDERS stays as defense for direct callers; the two lists agree exactly. Regenerated backend/openapi.json (provider now type:string + enum on both models) and frontend-v2 api-generated.ts (provider is the union). Finding 3 - type TEMPLATE_PROVIDER_OPTIONS against the generated provider union (as const satisfies) so an invalid/divergent entry is a compile error. ssh stays intentionally omitted from the create UI but is now permitted by the type. Minor 3 - PUT {"provider": null} returned HTTP 500: model_dump(exclude_unset= True) includes {"provider": None}, and the generic assignment loop wrote None into the nullable=False column. Guard now checks presence, dropping an explicit null so the provider is left unchanged; fixed the inaccurate comment. Finding 6 - de-vacuumed the misspelled-provider assertion (assert on azure/ssh, not the "ibm" substring of the offender) and turned it into the body-leak reproduction (asserts the plaintext secret is absent from the error). Added a PUT-null regression test. Mutation-checked both new assertions. Verified: 109 passed (affected suite); ruff clean; generate-openapi.py --check OK; FE types regen idempotent; tsc --noEmit clean. Claude-Session: https://claude.ai/code/session_01UCsZXDxBsWV2s4kT47DwDW --- backend/openapi.json | 16 +++++++++++++- backend/routes/credential_templates.py | 22 ++++--------------- .../services/credential_template_service.py | 7 +++++- .../test_credential_template_service.py | 14 ++++++++++++ .../test_routes_credential_templates.py | 18 +++++++++++---- .../settings/CredentialTemplates.tsx | 8 ++++++- frontend-v2/src/types/api-generated.ts | 9 +++++--- 7 files changed, 66 insertions(+), 28 deletions(-) diff --git a/backend/openapi.json b/backend/openapi.json index 8ebd6722..14c241d5 100644 --- a/backend/openapi.json +++ b/backend/openapi.json @@ -40729,6 +40729,13 @@ }, "provider": { "type": "string", + "enum": [ + "aws", + "azure", + "gcp", + "ibm", + "ssh" + ], "title": "Provider" }, "aws_auth_method": { @@ -41531,7 +41538,14 @@ "provider": { "anyOf": [ { - "type": "string" + "type": "string", + "enum": [ + "aws", + "azure", + "gcp", + "ibm", + "ssh" + ] }, { "type": "null" diff --git a/backend/routes/credential_templates.py b/backend/routes/credential_templates.py index 12c8e8ac..19aeaa3c 100644 --- a/backend/routes/credential_templates.py +++ b/backend/routes/credential_templates.py @@ -5,6 +5,7 @@ """ import logging from datetime import datetime +from typing import Literal from fastapi import APIRouter, Depends from fastapi.responses import JSONResponse @@ -14,14 +15,9 @@ from core.errors import handle_route_errors from database import get_db from routes.auth import require_operator, require_viewer -from services.credential_template_service import ( - SUPPORTED_PROVIDERS, - CredentialTemplateService, -) +from services.credential_template_service import CredentialTemplateService from utils.validators import validate_aws_region -_SUPPORTED_PROVIDERS_MSG = ", ".join(sorted(SUPPORTED_PROVIDERS)) - logger = logging.getLogger(__name__) router = APIRouter(prefix="/api/credential-templates", tags=["credential-templates"]) @@ -34,7 +30,7 @@ class CredentialTemplateBase(BaseModel): name: str description: str | None = None - provider: str + provider: Literal["aws", "azure", "gcp", "ibm", "ssh"] aws_auth_method: str | None = None aws_profile: str | None = None region: str | None = None @@ -67,10 +63,6 @@ class CredentialTemplateBase(BaseModel): @model_validator(mode="after") def _validate_regions(self): - if self.provider not in SUPPORTED_PROVIDERS: - raise ValueError( - f"Unsupported provider '{self.provider}'. Must be one of: {_SUPPORTED_PROVIDERS_MSG}." - ) if self.provider == "aws": validate_aws_region(self.region, field_name="region") validate_aws_region(self.aws_sso_region, field_name="aws_sso_region") @@ -86,7 +78,7 @@ class CredentialTemplateCreate(CredentialTemplateBase): class CredentialTemplateUpdate(BaseModel): name: str | None = None description: str | None = None - provider: str | None = None + provider: Literal["aws", "azure", "gcp", "ibm", "ssh"] | None = None aws_auth_method: str | None = None aws_profile: str | None = None region: str | None = None @@ -119,12 +111,6 @@ class CredentialTemplateUpdate(BaseModel): @model_validator(mode="after") def _validate_regions(self): - # provider is optional on update; only validate when the caller is - # actually changing it, so an unknown value can't be persisted (issue #191). - if self.provider is not None and self.provider not in SUPPORTED_PROVIDERS: - raise ValueError( - f"Unsupported provider '{self.provider}'. Must be one of: {_SUPPORTED_PROVIDERS_MSG}." - ) validate_aws_region(self.aws_sso_region, field_name="aws_sso_region") if self.region and self.provider == "aws": validate_aws_region(self.region, field_name="region") diff --git a/backend/services/credential_template_service.py b/backend/services/credential_template_service.py index 56e8f71c..9d01893c 100644 --- a/backend/services/credential_template_service.py +++ b/backend/services/credential_template_service.py @@ -349,9 +349,14 @@ def update_template(self, template_id: int, template_data) -> dict: # If the caller is changing the provider, hold it to the same canonical # set as create so an update can't move a template onto a value that - # injects nothing (issue #191). A None/absent provider leaves it unchanged. + # injects nothing (issue #191). An explicit ``null`` (or an absent + # field) leaves the stored provider unchanged: drop it here so the + # generic assignment loop below can't write None into the + # ``nullable=False`` column and 500 on flush. if update_data.get("provider") is not None: validate_provider(update_data["provider"]) + else: + update_data.pop("provider", None) # Handle encrypted fields encrypted_map = { diff --git a/backend/tests/component/test_credential_template_service.py b/backend/tests/component/test_credential_template_service.py index a5b1622f..f5d59b60 100644 --- a/backend/tests/component/test_credential_template_service.py +++ b/backend/tests/component/test_credential_template_service.py @@ -309,6 +309,20 @@ def test_update_without_provider_change_is_allowed(self, db): assert result["provider"] == "aws" assert result["description"] == "just a note" + def test_update_with_explicit_null_provider_leaves_it_unchanged(self, db): + """Minor 3: ``PUT {"provider": null}`` must leave the provider unchanged, + not assign None into the ``nullable=False`` column and 500 on flush. + + ``model_dump(exclude_unset=True)`` includes ``{"provider": None}`` for an + explicit null, so the presence-not-non-nullness guard must drop it.""" + t = _create_template_in_db(db, name="aws-null-prov", provider="aws") + svc = CredentialTemplateService(db) + result = svc.update_template(t.id, _make_update_data(provider=None, description="edit")) + assert result["provider"] == "aws" + assert result["description"] == "edit" + db.refresh(t) + assert t.provider == "aws" + # --------------------------------------------------------------------------- # list_templates / get_template diff --git a/backend/tests/integration/test_routes_credential_templates.py b/backend/tests/integration/test_routes_credential_templates.py index b145f083..75eed377 100644 --- a/backend/tests/integration/test_routes_credential_templates.py +++ b/backend/tests/integration/test_routes_credential_templates.py @@ -124,22 +124,32 @@ def test_create_ibm_template(self, mock_svc_cls, client, operator_headers, all_t @patch("routes.credential_templates.CredentialTemplateService") def test_create_rejects_misspelled_provider_with_422(self, mock_svc_cls, client, operator_headers, all_test_users): """Issue #191: provider='ibmcloud' is a 422 at the API boundary, not a - silently-empty template — and the service is never reached.""" + silently-empty template — and the service is never reached. + + The provider is a ``Literal`` field, so the 422 is field-scoped: the + error enumerates the supported set (proving the message is real, not + vacuous — ``azure``/``ssh`` are not substrings of the offender) and + must NOT echo the request body, which carries the plaintext secret.""" mock_svc = MagicMock() mock_svc_cls.return_value = mock_svc + secret = "pltxt-ibm-api-key-DO-NOT-LEAK-9f3a7c" response = client.post( "/api/credential-templates", json={"name": "IBM ROKs Testing", "provider": "ibmcloud", - "region": "us-east", "ibmcloud_api_key": "secret"}, + "region": "us-east", "ibmcloud_api_key": secret}, headers=operator_headers, ) assert response.status_code == 422 body = response.json() - # The validation error names the bad provider and the supported set. detail = str(body["detail"]) + # Field-scoped: the offending input is echoed, the supported set named. assert "ibmcloud" in detail - assert "ibm" in detail + assert "azure" in detail # non-vacuous: not a substring of "ibmcloud" + assert "ssh" in detail + # Body-leak guard (finding 1): the plaintext credential must never + # appear anywhere in the error response — no whole-body echo. + assert secret not in response.text mock_svc.create_template.assert_not_called() diff --git a/frontend-v2/src/components/settings/CredentialTemplates.tsx b/frontend-v2/src/components/settings/CredentialTemplates.tsx index 52c5c1dc..5e9e42cf 100644 --- a/frontend-v2/src/components/settings/CredentialTemplates.tsx +++ b/frontend-v2/src/components/settings/CredentialTemplates.tsx @@ -59,6 +59,7 @@ import { notify, notifyError } from '@/lib/notify'; import { api } from '@/lib/api'; import { loadIbmRegionsFromApiKey } from '@/lib/ibm-cloud'; import type { CloudCredentialTemplate, CloudCredentialTemplateCreate, CloudRegionOption, IBCosInstanceOption } from '@/types'; +import type { ApiCredentialTemplateCreate } from '@/types/api-schemas'; import { RegionSelector } from '@/components/aws/RegionSelector'; import { CloudRegionSelector } from '@/components/cloud/CloudRegionSelector'; import { SSOAuthDialog } from './SSOAuthDialog'; @@ -66,12 +67,17 @@ import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/comp import { resolveCredStatus } from './resolveCredStatus'; import { useAppMutation } from '@/hooks/lib/useAppMutation'; +// Typed against the generated `provider` union so an invalid entry (or one +// that drifts from the backend Literal) is a compile error. `ssh` is a valid +// provider but intentionally not offered in this create UI; the type still +// permits it, so adding it later is a one-line change with no cast. +type TemplateProvider = ApiCredentialTemplateCreate['provider']; const TEMPLATE_PROVIDER_OPTIONS = [ { value: 'aws', label: 'Amazon Web Services (AWS)' }, { value: 'gcp', label: 'Google Cloud Platform (GCP)' }, { value: 'azure', label: 'Microsoft Azure' }, { value: 'ibm', label: 'IBM Cloud' }, -] as const; +] as const satisfies ReadonlyArray<{ value: TemplateProvider; label: string }>; /** * Single authoritative AWS credential status badge. diff --git a/frontend-v2/src/types/api-generated.ts b/frontend-v2/src/types/api-generated.ts index 44a03e85..80a79840 100644 --- a/frontend-v2/src/types/api-generated.ts +++ b/frontend-v2/src/types/api-generated.ts @@ -15621,8 +15621,11 @@ export interface components { name: string; /** Description */ description?: string | null; - /** Provider */ - provider: string; + /** + * Provider + * @enum {string} + */ + provider: "aws" | "azure" | "gcp" | "ibm" | "ssh"; /** Aws Auth Method */ aws_auth_method?: string | null; /** Aws Profile */ @@ -15819,7 +15822,7 @@ export interface components { /** Description */ description?: string | null; /** Provider */ - provider?: string | null; + provider?: ("aws" | "azure" | "gcp" | "ibm" | "ssh") | null; /** Aws Auth Method */ aws_auth_method?: string | null; /** Aws Profile */