From 8c35adf3b99b551ca1866f4028deb3db5d62a03c Mon Sep 17 00:00:00 2001 From: Suchintan Date: Wed, 11 Feb 2026 00:04:51 -0500 Subject: [PATCH] Add endpoint to update existing credential data (#SKY-7883) (#4693) Co-authored-by: Suchintan Singh --- skyvern/forge/sdk/db/agent_db.py | 39 ++++++++ skyvern/forge/sdk/routes/credentials.py | 76 ++++++++++++++- .../azure_credential_vault_service.py | 70 +++++++++++++- .../bitwarden_credential_service.py | 54 +++++++++++ .../credential/credential_vault_service.py | 55 ++++++++++- .../custom_credential_vault_service.py | 96 +++++++++++++++++++ 6 files changed, 387 insertions(+), 3 deletions(-) diff --git a/skyvern/forge/sdk/db/agent_db.py b/skyvern/forge/sdk/db/agent_db.py index 5ee30770..70e4e442 100644 --- a/skyvern/forge/sdk/db/agent_db.py +++ b/skyvern/forge/sdk/db/agent_db.py @@ -5192,6 +5192,45 @@ class AgentDB(BaseAlchemyDB): await session.refresh(credential) return Credential.model_validate(credential) + async def update_credential_vault_data( + self, + credential_id: str, + organization_id: str, + item_id: str, + name: str, + credential_type: CredentialType, + username: str | None = None, + totp_type: str = "none", + totp_identifier: str | None = None, + card_last4: str | None = None, + card_brand: str | None = None, + secret_label: str | None = None, + ) -> Credential: + async with self.Session() as session: + credential = ( + await session.scalars( + select(CredentialModel) + .filter_by(credential_id=credential_id) + .filter_by(organization_id=organization_id) + .filter(CredentialModel.deleted_at.is_(None)) + .with_for_update() + ) + ).first() + if not credential: + raise NotFoundError(f"Credential {credential_id} not found") + credential.item_id = item_id + credential.name = name + credential.credential_type = credential_type + credential.username = username + credential.totp_type = totp_type + credential.totp_identifier = totp_identifier + credential.card_last4 = card_last4 + credential.card_brand = card_brand + credential.secret_label = secret_label + await session.commit() + await session.refresh(credential) + return Credential.model_validate(credential) + async def delete_credential(self, credential_id: str, organization_id: str) -> None: async with self.Session() as session: credential = ( diff --git a/skyvern/forge/sdk/routes/credentials.py b/skyvern/forge/sdk/routes/credentials.py index 784f47e4..af6a528e 100644 --- a/skyvern/forge/sdk/routes/credentials.py +++ b/skyvern/forge/sdk/routes/credentials.py @@ -280,6 +280,75 @@ async def create_credential( raise HTTPException(status_code=400, detail=f"Unsupported credential type: {data.credential_type}") +@legacy_base_router.put("/credentials/{credential_id}") +@legacy_base_router.put("/credentials/{credential_id}/", include_in_schema=False) +@base_router.post( + "/credentials/{credential_id}/update", + response_model=CredentialResponse, + summary="Update credential", + description="Overwrites the stored credential data (e.g. username/password) while keeping the same credential_id.", + tags=["Credentials"], + openapi_extra={ + "x-fern-sdk-method-name": "update_credential", + }, +) +@base_router.post( + "/credentials/{credential_id}/update/", + response_model=CredentialResponse, + include_in_schema=False, +) +async def update_credential( + background_tasks: BackgroundTasks, + credential_id: str = Path( + ..., + description="The unique identifier of the credential to update", + examples=["cred_1234567890"], + openapi_extra={"x-fern-sdk-parameter-name": "credential_id"}, + ), + data: CreateCredentialRequest = Body( + ..., + description="The new credential data to store", + example={ + "name": "My Credential", + "credential_type": "PASSWORD", + "credential": {"username": "user@example.com", "password": "newpassword123"}, + }, + openapi_extra={"x-fern-sdk-parameter-name": "data"}, + ), + current_org: Organization = Depends(org_auth_service.get_current_org), +) -> CredentialResponse: + existing_credential = await app.DATABASE.get_credential( + credential_id=credential_id, organization_id=current_org.organization_id + ) + if not existing_credential: + raise HTTPException(status_code=404, detail=f"Credential not found, credential_id={credential_id}") + + vault_type = existing_credential.vault_type or CredentialVaultType.BITWARDEN + credential_service = app.CREDENTIAL_VAULT_SERVICES.get(vault_type) + if not credential_service: + raise HTTPException(status_code=400, detail="Unsupported credential storage type") + + old_item_id = existing_credential.item_id + + updated_credential = await credential_service.update_credential( + credential=existing_credential, + data=data, + ) + + # Schedule background cleanup of old vault item if the item_id changed + if old_item_id != updated_credential.item_id: + background_tasks.add_task( + credential_service.post_delete_credential_item, + old_item_id, + existing_credential.organization_id, + ) + + if updated_credential.vault_type == CredentialVaultType.BITWARDEN: + background_tasks.add_task(fetch_credential_item_background, updated_credential.item_id) + + return _convert_to_response(updated_credential) + + @legacy_base_router.delete("/credentials/{credential_id}") @legacy_base_router.delete("/credentials/{credential_id}/", include_in_schema=False) @base_router.post( @@ -329,7 +398,12 @@ async def delete_credential( await credential_service.delete_credential(credential) # Schedule background cleanup if the service implements it - background_tasks.add_task(credential_service.post_delete_credential_item, credential.item_id) + if vault_type != CredentialVaultType.CUSTOM: + background_tasks.add_task( + credential_service.post_delete_credential_item, + credential.item_id, + credential.organization_id, + ) return None diff --git a/skyvern/forge/sdk/services/credential/azure_credential_vault_service.py b/skyvern/forge/sdk/services/credential/azure_credential_vault_service.py index a9cf293d..1bc748b8 100644 --- a/skyvern/forge/sdk/services/credential/azure_credential_vault_service.py +++ b/skyvern/forge/sdk/services/credential/azure_credential_vault_service.py @@ -66,6 +66,35 @@ class AzureCredentialVaultService(CredentialVaultService): return credential + async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential: + # Azure supports in-place secret updates, so we reuse the same item_id. + # NOTE: If the DB update below fails, the vault will contain the new data + # while DB metadata (name, type, username) remains stale. The actual credential + # data in the vault is still correct since it uses the same item_id. A retry + # of the update call will reconcile the DB metadata. + await self._update_azure_secret_item( + item_id=credential.item_id, + credential=data.credential, + ) + + try: + updated_credential = await self._update_db_credential( + credential=credential, + data=data, + item_id=credential.item_id, + ) + except Exception: + LOG.error( + "DB update failed after Azure vault secret was already overwritten. " + "Vault data is updated but DB metadata may be stale.", + organization_id=credential.organization_id, + credential_id=credential.credential_id, + item_id=credential.item_id, + ) + raise + + return updated_credential + async def delete_credential( self, credential: Credential, @@ -78,7 +107,7 @@ class AzureCredentialVaultService(CredentialVaultService): secret_value="", ) - async def post_delete_credential_item(self, item_id: str) -> None: + async def post_delete_credential_item(self, item_id: str, _organization_id: str | None = None) -> None: """ Background task to delete the credential item from Azure Key Vault. This allows the API to respond quickly while the deletion happens asynchronously. @@ -184,3 +213,42 @@ class AzureCredentialVaultService(CredentialVaultService): secret_name=secret_name, secret_value=secret_value, ) + + async def _update_azure_secret_item( + self, + item_id: str, + credential: PasswordCredential | CreditCardCredential | SecretCredential, + ) -> None: + if isinstance(credential, PasswordCredential): + data = AzureCredentialVaultService._PasswordCredentialDataImage( + type="password", + username=credential.username, + password=credential.password, + totp=credential.totp, + ) + elif isinstance(credential, CreditCardCredential): + data = AzureCredentialVaultService._CreditCardCredentialDataImage( + type="credit_card", + card_number=credential.card_number, + card_cvv=credential.card_cvv, + card_exp_month=credential.card_exp_month, + card_exp_year=credential.card_exp_year, + card_brand=credential.card_brand, + card_holder_name=credential.card_holder_name, + ) + elif isinstance(credential, SecretCredential): + data = AzureCredentialVaultService._SecretCredentialDataImage( + type="secret", + secret_value=credential.secret_value, + secret_label=credential.secret_label, + ) + else: + raise TypeError(f"Invalid credential type: {type(credential)}") + + secret_value = data.model_dump_json(exclude_none=True) + + await self._client.create_or_update_secret( + vault_name=self._vault_name, + secret_name=item_id, + secret_value=secret_value, + ) diff --git a/skyvern/forge/sdk/services/credential/bitwarden_credential_service.py b/skyvern/forge/sdk/services/credential/bitwarden_credential_service.py index af930d16..baf49157 100644 --- a/skyvern/forge/sdk/services/credential/bitwarden_credential_service.py +++ b/skyvern/forge/sdk/services/credential/bitwarden_credential_service.py @@ -46,6 +46,45 @@ class BitwardenCredentialVaultService(CredentialVaultService): return credential + async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential: + org_collection = await app.DATABASE.get_organization_bitwarden_collection(credential.organization_id) + + if not org_collection: + raise HTTPException(status_code=404, detail="Credential account not found. It might have been deleted.") + + # Create new vault item with the updated data + new_item_id = await BitwardenService.create_credential_item( + collection_id=org_collection.collection_id, + name=data.name, + credential=data.credential, + ) + + # Update DB record to point to the new vault item + try: + updated_credential = await self._update_db_credential( + credential=credential, + data=data, + item_id=new_item_id, + ) + except Exception: + LOG.warning( + "DB update failed, attempting to clean up new Bitwarden vault item", + organization_id=credential.organization_id, + new_item_id=new_item_id, + ) + try: + await BitwardenService.delete_credential_item(new_item_id) + except Exception as cleanup_error: + LOG.error( + "Failed to clean up orphaned Bitwarden vault item", + organization_id=credential.organization_id, + new_item_id=new_item_id, + error=str(cleanup_error), + ) + raise + + return updated_credential + async def delete_credential( self, credential: Credential, @@ -59,5 +98,20 @@ class BitwardenCredentialVaultService(CredentialVaultService): await app.DATABASE.delete_credential(credential.credential_id, credential.organization_id) await BitwardenService.delete_credential_item(credential.item_id) + async def post_delete_credential_item(self, item_id: str, organization_id: str | None = None) -> None: + try: + await BitwardenService.delete_credential_item(item_id) + LOG.info( + "Successfully deleted credential item from Bitwarden in background", + item_id=item_id, + ) + except Exception as e: + LOG.warning( + "Failed to delete credential item from Bitwarden in background", + item_id=item_id, + error=str(e), + exc_info=True, + ) + async def get_credential_item(self, db_credential: Credential) -> CredentialItem: return await BitwardenService.get_credential_item(db_credential.item_id) diff --git a/skyvern/forge/sdk/services/credential/credential_vault_service.py b/skyvern/forge/sdk/services/credential/credential_vault_service.py index 509382a6..3b19a1e7 100644 --- a/skyvern/forge/sdk/services/credential/credential_vault_service.py +++ b/skyvern/forge/sdk/services/credential/credential_vault_service.py @@ -21,11 +21,15 @@ class CredentialVaultService(ABC): async def create_credential(self, organization_id: str, data: CreateCredentialRequest) -> Credential: """Create a new credential in the vault and database.""" + @abstractmethod + async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential: + """Update an existing credential's vault data. Returns the updated credential.""" + @abstractmethod async def delete_credential(self, credential: Credential) -> None: """Delete a credential from the vault and database.""" - async def post_delete_credential_item(self, item_id: str) -> None: + async def post_delete_credential_item(self, item_id: str, organization_id: str | None = None) -> None: """ Optional hook for scheduling background cleanup tasks after credential deletion. Default implementation does nothing. Override in subclasses as needed. @@ -84,3 +88,52 @@ class CredentialVaultService(ABC): ) else: raise Exception(f"Unsupported credential type: {data.credential_type}") + + @staticmethod + async def _update_db_credential( + credential: Credential, + data: CreateCredentialRequest, + item_id: str, + ) -> Credential: + if data.credential_type == CredentialType.PASSWORD: + return await app.DATABASE.update_credential_vault_data( + credential_id=credential.credential_id, + organization_id=credential.organization_id, + item_id=item_id, + name=data.name, + credential_type=data.credential_type, + username=data.credential.username, + totp_type=data.credential.totp_type, + totp_identifier=data.credential.totp_identifier, + card_last4=None, + card_brand=None, + ) + elif data.credential_type == CredentialType.CREDIT_CARD: + return await app.DATABASE.update_credential_vault_data( + credential_id=credential.credential_id, + organization_id=credential.organization_id, + item_id=item_id, + name=data.name, + credential_type=data.credential_type, + username=None, + totp_type="none", + card_last4=data.credential.card_number[-4:], + card_brand=data.credential.card_brand, + totp_identifier=None, + ) + elif data.credential_type == CredentialType.SECRET: + return await app.DATABASE.update_credential_vault_data( + credential_id=credential.credential_id, + organization_id=credential.organization_id, + item_id=item_id, + name=data.name, + credential_type=data.credential_type, + username=None, + totp_type="none", + card_last4=None, + card_brand=None, + totp_identifier=None, + secret_label=data.credential.secret_label, + ) + else: + raise Exception(f"Unsupported credential type: {data.credential_type}") diff --git a/skyvern/forge/sdk/services/credential/custom_credential_vault_service.py b/skyvern/forge/sdk/services/credential/custom_credential_vault_service.py index 581629be..7ab9bed9 100644 --- a/skyvern/forge/sdk/services/credential/custom_credential_vault_service.py +++ b/skyvern/forge/sdk/services/credential/custom_credential_vault_service.py @@ -160,6 +160,102 @@ class CustomCredentialVaultService(CredentialVaultService): ) raise + async def update_credential(self, credential: Credential, data: CreateCredentialRequest) -> Credential: + LOG.info( + "Updating credential in custom vault", + organization_id=credential.organization_id, + credential_id=credential.credential_id, + name=data.name, + credential_type=data.credential_type, + ) + + try: + client = await self._get_client_for_organization(credential.organization_id) + + # Create new credential in the external API + new_item_id = await client.create_credential( + name=data.name, + credential=data.credential, + ) + + # Update DB record to point to the new vault item + try: + updated_credential = await self._update_db_credential( + credential=credential, + data=data, + item_id=new_item_id, + ) + except Exception: + LOG.warning( + "DB update failed, attempting to clean up new external credential", + organization_id=credential.organization_id, + new_item_id=new_item_id, + ) + try: + await client.delete_credential(new_item_id) + except Exception as cleanup_error: + LOG.error( + "Failed to clean up orphaned external credential", + organization_id=credential.organization_id, + new_item_id=new_item_id, + error=str(cleanup_error), + ) + raise + + LOG.info( + "Successfully updated credential in custom vault", + organization_id=credential.organization_id, + credential_id=credential.credential_id, + old_item_id=credential.item_id, + new_item_id=new_item_id, + ) + + return updated_credential + + except Exception as e: + LOG.error( + "Failed to update credential in custom vault", + organization_id=credential.organization_id, + credential_id=credential.credential_id, + error=str(e), + exc_info=True, + ) + raise + + async def post_delete_credential_item(self, item_id: str, organization_id: str | None = None) -> None: + """ + Background task to delete the old credential item from the custom vault + after an update or delete operation. + """ + try: + if organization_id is None and self._client is None: + LOG.warning( + "Skipping custom vault cleanup; organization_id is required for per-organization configuration", + item_id=item_id, + organization_id=organization_id, + ) + return + + if self._client is not None: + client = self._client + else: + assert organization_id is not None + client = await self._get_client_for_organization(organization_id) + await client.delete_credential(item_id) + LOG.info( + "Successfully deleted credential item from custom vault in background", + organization_id=organization_id, + item_id=item_id, + ) + except Exception as e: + LOG.warning( + "Failed to delete credential item from custom vault in background", + organization_id=organization_id, + item_id=item_id, + error=str(e), + exc_info=True, + ) + async def delete_credential(self, credential: Credential) -> None: """ Delete a credential from the custom vault and database.