feat: add custom credential service #4129
Conversation
Add support for custom credential service allowing Skyvern to integrate with external HTTP APIs for credential management. This provides flexibility for organizations that want to use their own credential management systems. Features: - HTTP API integration with Bearer token authentication - Support for password and credit card credentials - Environment variable configuration (self-hosted) - Organization-specific API configuration (cloud) - UI configuration through Skyvern settings page - Connection testing functionality - Comprehensive error handling and logging Backend changes: - Add CustomCredentialAPIClient for HTTP API communication - Add CustomCredentialVaultService implementing CredentialVaultService interface - Add custom credential API endpoints and configuration schemas - Add CUSTOM credential vault type and database enums - Integrate service into forge app and credential vault registry Frontend changes: - Add CustomCredentialServiceConfigForm React component - Add useCustomCredentialServiceConfig hook with connection testing - Add TypeScript types for API integration - Integrate into Settings page Documentation: - Add comprehensive API documentation in fern/credentials - Add configuration examples and troubleshooting guide - Update README with custom credential service support
WalkthroughThis pull request adds support for a Custom Credential Service (HTTP API) integration. It includes frontend UI components and hooks for configuration management, backend service implementations for credential operations, new API endpoints for managing the service configuration, and comprehensive documentation explaining the integration model. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant Backend API
participant DB
participant Custom Service
participant Auth Token Store
Frontend->>Backend API: POST /credentials/custom_credential/create<br/>(api_base_url, api_token)
Backend API->>DB: Invalidate existing custom config for org
Backend API->>DB: Create OrganizationAuthToken<br/>(type: custom_credential_service,<br/>token: JSON config)
Backend API->>Auth Token Store: Store auth token
Backend API-->>Frontend: CustomCredentialServiceConfigResponse<br/>(token metadata)
Frontend->>Backend API: GET /credentials/custom_credential/get
Backend API->>Auth Token Store: Fetch auth token
Backend API-->>Frontend: CustomCredentialServiceConfigResponse
Frontend->>Frontend: POST testConnection<br/>(config)
Frontend->>Custom Service: GET /api/v1/credentials<br/>(Bearer token)
Custom Service-->>Frontend: 200 OK
Frontend->>Frontend: Display success toast
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
v1k45
changed the title
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 3b56f29 in 2 minutes and 7 seconds. Click for details.
- Reviewed
1481lines of code in16files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. skyvern/forge/sdk/services/credential/custom_credential_vault_service.py:42
- Draft comment:
Consider caching the organization‐specific API client after retrieval to avoid repeated DB calls for the same organization. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a new file being added, so all code is technically "changed". The comment is suggesting a performance optimization - caching clients per organization. However, this is a speculative optimization. We don't know if this is actually a performance problem in practice. The comment doesn't point to a bug or clear issue - it's suggesting a potential improvement that may or may not be necessary. According to the rules, I should not keep speculative comments. Additionally, this is a "nice to have" optimization rather than something that needs to be fixed. The code works correctly as-is. The rules state that code quality refactors are good "but only if they are actionable and clear" - while this is actionable, it's not clear that it's necessary or important. This could be a legitimate performance concern if the same organization makes many credential operations in quick succession. The suggestion is actionable and clear about what to do. It's not speculative in the sense of "if X then Y" - it's pointing out that repeated DB calls will happen with the current implementation. While the observation about repeated DB calls is technically correct, this is still an optimization suggestion rather than a bug fix. Without evidence that this is actually a performance bottleneck, this falls into the category of premature optimization. The rules emphasize not making comments unless there's clearly a code change required, and this optimization isn't clearly required. This comment should be deleted. It's a performance optimization suggestion without evidence that it's needed. The code works correctly as-is, and the comment doesn't identify a clear bug or required change. It's the type of "nice to have" suggestion that should be avoided in PR reviews.
2. skyvern/forge/sdk/api/custom_credential_client.py:137
- Draft comment:
Ensure the API response always includes the 'id' field; consider validating the response schema upstream if the field is missing. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. skyvern/forge/sdk/routes/credentials.py:691
- Draft comment:
In the custom credential configuration update endpoint, ensure that token invalidation and creation are performed atomically to prevent race conditions. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% This comment is speculative - it says "ensure that" which is asking the author to verify/confirm something rather than pointing out a definite issue. The comment uses the phrase "to prevent race conditions" which is hypothetical. The same pattern is used consistently throughout the file in other similar endpoints (OnePassword, Azure), so if this were a real concern, it would apply to all of them. The comment doesn't provide evidence that race conditions are actually occurring or that this is a known problem. According to the rules, I should not keep speculative comments like "If X, then Y is an issue" - only comment if it's definitely an issue. This falls into that category. Perhaps there is a legitimate concurrency concern here if multiple requests could be processed simultaneously for the same organization. The comment might be highlighting a real architectural issue that could lead to data inconsistency. Without seeing the database implementation, I can't be certain this isn't a valid concern. While concurrency could theoretically be a concern, the comment is still speculative and doesn't provide evidence of an actual problem. The same pattern exists in multiple other endpoints in this file without any atomicity guarantees, suggesting this is the accepted pattern in this codebase. If this were a real issue, it would need to be addressed systematically across all similar endpoints, not just this one. The comment also violates the rule about not asking authors to "ensure" things. This comment should be deleted. It's speculative ("to prevent race conditions"), uses language that asks the author to verify/ensure something rather than pointing out a definite issue, and the same pattern is used consistently in other similar endpoints in the file without any atomicity concerns being raised.
4. skyvern/forge/forge_app.py:191
- Draft comment:
The integration of CustomCredentialVaultService is well done. Consider adding a clarifying comment on the fallback behavior when no global client is provided. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. skyvern/forge/sdk/routes/credentials.py:640
- Draft comment:
The error handling in the custom credential configuration retrieval endpoint is consistent; consider distinguishing between missing configuration and JSON parsing errors for better diagnostics. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is about the GET endpoint for custom credential service config. Looking at the code, this endpoint doesn't actually parse JSON - it just retrieves a token from the database and returns it. The JSON parsing happens in the POST/create endpoint (line 698) wherejson.dumps(request.config.model_dump())is used to store the config. The GET endpoint just returns the raw token. So the comment seems to be suggesting error handling for something that doesn't happen in this function. The comment appears to be speculative or based on a misunderstanding of what this endpoint does. There's no JSON parsing in the GET endpoint, so there are no JSON parsing errors to distinguish from missing configuration errors. Could there be JSON parsing happening in the CustomCredentialServiceConfigResponse constructor that I'm not seeing? Maybe the response model does some parsing internally that could fail? Even if the response model does some internal parsing, that would be caught by the generic exception handler already in place (lines 654-664). The comment suggests adding specific handling for JSON parsing errors, but without seeing evidence that this is actually a problem or that the current error handling is insufficient, this is speculative. The comment doesn't point to a specific code change that needs to be made. This comment should be deleted. It suggests handling JSON parsing errors in a GET endpoint that doesn't appear to do any JSON parsing. The endpoint simply retrieves a token from the database and returns it. The comment is speculative and not clearly actionable, and there's no strong evidence that the current error handling is insufficient.
6. skyvern/forge/sdk/schemas/organizations.py:83
- Draft comment:
The custom credential service configuration models are clear; consider adding additional examples in the Field definitions to further aid API consumers. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_jZEVoVajBbGTlZla
You can customize 
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
fern/credentials/custom-credential-service.mdx (1)
145-147: Example uses deprecated Pydantic v1 method.The example code uses
.dict()which is deprecated in Pydantic v2. Consider updating to.model_dump()for users on newer Pydantic versions.- credentials_store[credential_id] = request.dict() + credentials_store[credential_id] = request.model_dump()skyvern/forge/forge_app.py (1)
26-30: Import placement breaks alphabetical grouping.The
CustomCredentialAPIClientimport on line 26 is placed betweensdk.apiandsdk.services.credentialmodules, breaking the apparent grouping. Consider moving it to line 12 area with othersdk.apiimports for consistency.from skyvern.forge.sdk.api.azure import AzureClientFactory +from skyvern.forge.sdk.api.custom_credential_client import CustomCredentialAPIClient from skyvern.forge.sdk.api.llm.api_handler_factory import LLMAPIHandlerFactory from skyvern.forge.sdk.api.llm.models import LLMAPIHandler from skyvern.forge.sdk.api.real_azure import RealAzureClientFactory ... from skyvern.forge.sdk.schemas.credentials import CredentialVaultType from skyvern.forge.sdk.schemas.organizations import AzureClientSecretCredential, Organization -from skyvern.forge.sdk.api.custom_credential_client import CustomCredentialAPIClient from skyvern.forge.sdk.services.credential.azure_credential_vault_service import AzureCredentialVaultServiceskyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsx (2)
72-76: Consider using form.reset outside of render cycle.While
react-hook-form'sformobject is stable, including it in the dependency array can sometimes cause issues if the reference changes. You could exclude it and use// eslint-disable-next-line react-hooks/exhaustive-depsor restructure to only depend onparsedConfig.useEffect(() => { if (parsedConfig) { form.reset({ config: parsedConfig }); } + // eslint-disable-next-line react-hooks/exhaustive-deps - }, [parsedConfig, form]); + }, [parsedConfig]);Note: This is a minor suggestion since react-hook-form's form object reference is typically stable.
204-208: Token masking approach is good but verify minimum length.The token masking on line 207 assumes the token has at least 8 characters. If a shorter token is provided (edge case), this could display the entire token or cause issues.
- <div><strong>Token (masked):</strong> {parsedConfig.api_token.slice(0, 8)}...</div> + <div><strong>Token (masked):</strong> {parsedConfig.api_token.length > 8 ? `${parsedConfig.api_token.slice(0, 8)}...` : '********'}</div>skyvern-frontend/src/hooks/useCustomCredentialServiceConfig.ts (2)
20-27: Silent error swallowing may hide configuration issues.The
.catch(() => null)on line 25 silently discards all errors, making it difficult to debug when the fetch fails for reasons other than "not configured". Consider logging the error or handling specific cases differently.return await client .get("/credentials/custom_credential/get") .then((response) => response.data.token) - .catch(() => null); + .catch((error) => { + // 404 likely means not configured yet - return null silently + if (error?.response?.status === 404) { + return null; + } + // Log other errors for debugging but still return null + console.warn("Failed to fetch custom credential service config:", error); + return null; + });
118-121: Network error detection may miss some cases.The check for
error.message.includes('fetch')may not reliably catch all network errors across different browsers. Consider a more robust check.- if (error instanceof TypeError && error.message.includes('fetch')) { + // Network errors typically manifest as TypeError with various messages + if (error instanceof TypeError) { throw new Error('Network error: Cannot reach the API server. Check the URL and network connectivity.'); }skyvern/forge/sdk/services/credential/custom_credential_vault_service.py (1)
48-80: Use custom exception class and proper exception chaining.The exception handling has several issues flagged by static analysis:
- Generic
Exceptionis raised instead of a custom exception class- Missing
raise ... from efor proper exception chaining (B904)LOG.errorshould useLOG.exceptionwhen logging in except blocks (TRY400)Consider defining a custom exception and applying proper chaining:
+from skyvern.exceptions import SkyvernException + +class CustomCredentialConfigurationError(SkyvernException): + """Raised when custom credential service configuration is invalid or missing.""" + pass + async def _get_client_for_organization(self, organization_id: str) -> CustomCredentialAPIClient: # If we have a global client (from environment variables), use it if self._client: return self._client # Otherwise, get organization-specific configuration try: auth_token = await app.DATABASE.get_valid_org_auth_token( organization_id=organization_id, token_type=OrganizationAuthTokenType.custom_credential_service.value, ) if not auth_token: - raise Exception(f"Custom credential service not configured for organization {organization_id}") + raise CustomCredentialConfigurationError( + f"Custom credential service not configured for organization {organization_id}" + ) # Parse the stored configuration config_data = json.loads(auth_token.token) # Create and return the API client return CustomCredentialAPIClient( api_base_url=config_data["api_base_url"], api_token=config_data["api_token"], ) except json.JSONDecodeError as e: - LOG.error( + LOG.exception( "Failed to parse custom credential service configuration", organization_id=organization_id, - error=str(e), ) - raise Exception(f"Invalid custom credential service configuration for organization {organization_id}") + raise CustomCredentialConfigurationError( + f"Invalid custom credential service configuration for organization {organization_id}" + ) from eskyvern/forge/sdk/api/custom_credential_client.py (3)
58-59: UseTypeErrorfor invalid type checks.As flagged by static analysis (TRY004),
TypeErroris more appropriate thanValueErrorwhen the issue is an unexpected type.- raise ValueError(f"Unsupported credential type: {type(credential)}") + raise TypeError(f"Unsupported credential type: {type(credential)}")
61-94: Missing fields could cause KeyError; consider defensive handling.The
_api_response_to_credentialmethod directly accesses dictionary keys (e.g.,credential_data["username"]) which will raiseKeyErrorif the external API returns incomplete data. Consider using.get()with validation or wrapping in a try-except for clearer error messages.def _api_response_to_credential(self, credential_data: Dict[str, Any], name: str, item_id: str) -> CredentialItem: """Convert API response to Skyvern CredentialItem.""" credential_type = credential_data.get("type") if credential_type == "password": + required_fields = ["username", "password"] + missing = [f for f in required_fields if f not in credential_data] + if missing: + raise ValueError(f"Missing required password fields from API: {missing}") credential = PasswordCredential( username=credential_data["username"], password=credential_data["password"], totp=credential_data.get("totp"), totp_type=credential_data.get("totp_type", "none"), ) # ... elif credential_type == "credit_card": + required_fields = ["card_holder_name", "card_number", "card_exp_month", "card_exp_year", "card_cvv", "card_brand"] + missing = [f for f in required_fields if f not in credential_data] + if missing: + raise ValueError(f"Missing required credit card fields from API: {missing}") credential = CreditCardCredential( # ... )
155-165: Use exception chaining withfrom e.When re-raising as
HttpException, the original exception context is lost. Useraise ... from eto preserve the exception chain for better debugging.except Exception as e: LOG.error( "Failed to create credential via custom API", url=url, name=name, error=str(e), exc_info=True, ) - raise HttpException(500, f"Failed to create credential via custom API: {str(e)}") + raise HttpException(500, f"Failed to create credential via custom API: {e!s}") from eApply the same pattern to
get_credential(line 218) anddelete_credential(line 262).skyvern/forge/sdk/routes/credentials.py (2)
697-697: Moveimport jsonto the top of the file.The
jsonimport is placed inside the function body. While functional, this deviates from Python conventions (PEP 8) which recommend imports at the top of the file for clarity and consistency.import structlog +import json from fastapi import BackgroundTasks, Body, Depends, HTTPException, Path, QueryAnd remove the inline import at line 697.
715-725: Add exception chaining withfrom e.The static analysis correctly flags missing exception chaining (B904). This preserves the original error context for debugging.
except Exception as e: LOG.error( "Failed to create or update custom credential service configuration", organization_id=current_org.organization_id, error=str(e), exc_info=True, ) raise HTTPException( status_code=500, - detail=f"Failed to create or update custom credential service configuration: {str(e)}", - ) + detail=f"Failed to create or update custom credential service configuration: {e!s}", + ) from eApply the same pattern to
get_custom_credential_service_config(lines 661-664).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
README.md(1 hunks)fern/credentials/custom-credential-service.mdx(1 hunks)fern/credentials/introduction.mdx(2 hunks)fern/docs.yml(1 hunks)skyvern-frontend/src/api/types.ts(1 hunks)skyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsx(1 hunks)skyvern-frontend/src/hooks/useCustomCredentialServiceConfig.ts(1 hunks)skyvern-frontend/src/routes/settings/Settings.tsx(2 hunks)skyvern/config.py(1 hunks)skyvern/forge/forge_app.py(3 hunks)skyvern/forge/sdk/api/custom_credential_client.py(1 hunks)skyvern/forge/sdk/db/enums.py(1 hunks)skyvern/forge/sdk/routes/credentials.py(2 hunks)skyvern/forge/sdk/schemas/credentials.py(1 hunks)skyvern/forge/sdk/schemas/organizations.py(1 hunks)skyvern/forge/sdk/services/credential/custom_credential_vault_service.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Useruff checkandruff formatfor linting and formatting Python code
Usemypyfor type checking Python code
Use type hints in Python code
Maintain line length of 120 characters for Python code
**/*.py: Use Python 3.11+ features and type hints in Python code
Follow PEP 8 with a line length of 100 characters in Python code
Use absolute imports for all modules in Python code
Document all public functions and classes with Google-style docstrings in Python code
Usesnake_casefor variables and functions,PascalCasefor classes in Python code
Prefer async/await over callbacks for asynchronous programming in Python
Useasynciofor concurrency in Python asynchronous code
Always handle exceptions in async code in Python
Use context managers for resource cleanup in Python
Use specific exception classes rather than generic exceptions in error handling
Include meaningful error messages in exceptions
Log errors with appropriate severity levels in Python
Never expose sensitive information in error messages
Optimize database queries in Python for performance
Use appropriate data structures in Python for performance optimization
Implement caching where beneficial in Python code
Validate all inputs in Python code
Use environment variables for configuration instead of hardcoding values
Files:
skyvern/config.pyskyvern/forge/sdk/db/enums.pyskyvern/forge/sdk/schemas/organizations.pyskyvern/forge/sdk/schemas/credentials.pyskyvern/forge/sdk/api/custom_credential_client.pyskyvern/forge/sdk/routes/credentials.pyskyvern/forge/forge_app.pyskyvern/forge/sdk/services/credential/custom_credential_vault_service.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use async/await patterns for asynchronous operations
Files:
skyvern/config.pyskyvern-frontend/src/routes/settings/Settings.tsxskyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsxskyvern/forge/sdk/db/enums.pyskyvern/forge/sdk/schemas/organizations.pyskyvern-frontend/src/hooks/useCustomCredentialServiceConfig.tsskyvern-frontend/src/api/types.tsskyvern/forge/sdk/schemas/credentials.pyskyvern/forge/sdk/api/custom_credential_client.pyskyvern/forge/sdk/routes/credentials.pyskyvern/forge/forge_app.pyskyvern/forge/sdk/services/credential/custom_credential_vault_service.py
skyvern-frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
skyvern-frontend/**/*.{ts,tsx,js,jsx}: Usenpm run lintandnpm run formatfor linting and formatting frontend code in skyvern-frontend/
Maintain line length of 120 characters for TypeScript/JavaScript code
Files:
skyvern-frontend/src/routes/settings/Settings.tsxskyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsxskyvern-frontend/src/hooks/useCustomCredentialServiceConfig.tsskyvern-frontend/src/api/types.ts
skyvern/forge/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
FastAPI-based REST API and WebSocket support should be located in
skyvern/forge/directory
Files:
skyvern/forge/sdk/db/enums.pyskyvern/forge/sdk/schemas/organizations.pyskyvern/forge/sdk/schemas/credentials.pyskyvern/forge/sdk/api/custom_credential_client.pyskyvern/forge/sdk/routes/credentials.pyskyvern/forge/forge_app.pyskyvern/forge/sdk/services/credential/custom_credential_vault_service.py
🧠 Learnings (2)
📚 Learning: 2025-10-13T15:41:41.294Z
Learnt from: Valeran86
Repo: Skyvern-AI/skyvern PR: 3534
File: .env.example:128-132
Timestamp: 2025-10-13T15:41:41.294Z
Learning: For Skyvern's Bitwarden CLI server integration, port 8002 is the desired default port for the local Vaultwarden setup, not 11001. This applies to BITWARDEN_SERVER_PORT configuration in .env.example, docker-compose.yml, skyvern/config.py, and related documentation.
Applied to files:
README.md
📚 Learning: 2025-11-24T17:46:29.802Z
Learnt from: CR
Repo: Skyvern-AI/skyvern PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:46:29.802Z
Learning: Applies to skyvern/forge/**/*.py : FastAPI-based REST API and WebSocket support should be located in `skyvern/forge/` directory
Applied to files:
skyvern/forge/forge_app.py
🧬 Code graph analysis (7)
skyvern-frontend/src/routes/settings/Settings.tsx (1)
skyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsx (1)
CustomCredentialServiceConfigForm(37-215)
skyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsx (1)
skyvern-frontend/src/hooks/useCustomCredentialServiceConfig.ts (1)
useCustomCredentialServiceConfig(12-154)
skyvern/forge/sdk/schemas/organizations.py (1)
skyvern-frontend/src/api/types.ts (3)
CustomCredentialServiceConfig(230-233)CustomCredentialServiceConfigResponse(249-251)CreateCustomCredentialServiceConfigRequest(245-247)
skyvern-frontend/src/api/types.ts (1)
skyvern/forge/sdk/schemas/organizations.py (3)
CustomCredentialServiceConfig(83-95)CreateCustomCredentialServiceConfigRequest(107-110)CustomCredentialServiceConfigResponse(98-104)
skyvern/forge/sdk/routes/credentials.py (4)
skyvern/forge/sdk/schemas/organizations.py (3)
CustomCredentialServiceConfigResponse(98-104)CreateCustomCredentialServiceConfigRequest(107-110)Organization(8-21)skyvern/forge/sdk/db/client.py (5)
get_valid_org_auth_token(881-885)get_valid_org_auth_token(888-892)get_valid_org_auth_token(894-918)invalidate_org_auth_tokens(1016-1037)create_org_auth_token(980-1014)skyvern/forge/sdk/db/enums.py (1)
OrganizationAuthTokenType(4-8)skyvern/forge/sdk/schemas/credentials.py (1)
CredentialVaultType(7-10)
skyvern/forge/forge_app.py (3)
skyvern/forge/sdk/api/custom_credential_client.py (1)
CustomCredentialAPIClient(17-262)skyvern/forge/sdk/services/credential/custom_credential_vault_service.py (1)
CustomCredentialVaultService(18-239)skyvern/forge/sdk/schemas/credentials.py (1)
CredentialVaultType(7-10)
skyvern/forge/sdk/services/credential/custom_credential_vault_service.py (4)
skyvern/forge/sdk/api/custom_credential_client.py (4)
CustomCredentialAPIClient(17-262)create_credential(96-165)delete_credential(220-262)get_credential(167-218)skyvern/forge/sdk/db/enums.py (1)
OrganizationAuthTokenType(4-8)skyvern/forge/sdk/schemas/credentials.py (4)
CreateCredentialRequest(117-126)Credential(140-164)CredentialItem(108-114)CredentialVaultType(7-10)skyvern/forge/sdk/services/credential/credential_vault_service.py (2)
CredentialVaultService(13-70)_create_db_credential(39-70)
🪛 Ruff (0.14.6)
skyvern/forge/sdk/api/custom_credential_client.py
59-59: Prefer TypeError exception for invalid type
(TRY004)
59-59: Avoid specifying long messages outside the exception class
(TRY003)
94-94: Avoid specifying long messages outside the exception class
(TRY003)
134-134: Abstract raise to an inner function
(TRY301)
144-144: Abstract raise to an inner function
(TRY301)
165-165: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
165-165: Use explicit conversion flag
Replace with conversion flag
(RUF010)
198-198: Abstract raise to an inner function
(TRY301)
218-218: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
218-218: Use explicit conversion flag
Replace with conversion flag
(RUF010)
262-262: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
262-262: Use explicit conversion flag
Replace with conversion flag
(RUF010)
skyvern/forge/sdk/routes/credentials.py
634-634: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
645-648: Abstract raise to an inner function
(TRY301)
661-664: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
663-663: Use explicit conversion flag
Replace with conversion flag
(RUF010)
681-681: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
722-725: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
724-724: Use explicit conversion flag
Replace with conversion flag
(RUF010)
skyvern/forge/sdk/services/credential/custom_credential_vault_service.py
55-55: Abstract raise to an inner function
(TRY301)
55-55: Create your own exception
(TRY002)
55-55: Avoid specifying long messages outside the exception class
(TRY003)
67-71: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
72-72: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
72-72: Create your own exception
(TRY002)
72-72: Avoid specifying long messages outside the exception class
(TRY003)
125-125: Consider moving this statement to an else block
(TRY300)
228-228: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (24)
skyvern/forge/sdk/db/enums.py (1)
8-8: LGTM!The new enum member follows the existing naming convention and integrates seamlessly with the current token type structure.
README.md (1)
333-333: LGTM!The documentation update clearly communicates the new custom credential service integration option.
fern/credentials/introduction.mdx (2)
60-60: LGTM!The documentation addition is clear and consistent with the existing list format.
92-98: LGTM!The Card component follows the established pattern and provides clear navigation to the custom credential service documentation.
skyvern/forge/sdk/schemas/credentials.py (1)
10-10: LGTM!The new vault type enum member is consistent with existing patterns and properly extends the credential vault type options.
skyvern-frontend/src/routes/settings/Settings.tsx (2)
21-21: LGTM!The import follows the existing pattern for credential configuration form components.
101-111: LGTM!The new UI card follows the established pattern for credential service configuration and maintains consistency with existing sections.
fern/docs.yml (1)
134-135: LGTM!The navigation entry is properly structured and positioned in the appropriate section of the documentation.
skyvern/forge/sdk/schemas/organizations.py (1)
83-110: LGTM!The new Pydantic models are well-structured with:
- Clear field descriptions and examples
- Proper type hints
- Consistent pattern with existing credential service models (Azure, 1Password)
skyvern/config.py (1)
343-345: LGTM!The new configuration fields follow the established pattern for credential service settings and are appropriately positioned in the configuration file.
fern/credentials/custom-credential-service.mdx (1)
1-203: Well-structured documentation for the custom credential service.The documentation comprehensively covers the API contract, configuration options (environment variables and cloud), UI configuration steps, a practical example implementation, security considerations, and troubleshooting guidance. This will be helpful for users integrating their own credential services.
skyvern/forge/forge_app.py (1)
191-204: Custom credential vault service integration looks good.The initialization logic correctly follows the established pattern:
- Creates a fully configured service when both
CUSTOM_CREDENTIAL_API_BASE_URLandCUSTOM_CREDENTIAL_API_TOKENare provided- Falls back to a client-less service for organization-based configuration
- Properly registers in
CREDENTIAL_VAULT_SERVICESdictionaryThis is consistent with how the Azure vault service is initialized.
skyvern-frontend/src/api/types.ts (2)
230-247: Type definitions are well-structured and consistent.The new interfaces follow the established naming conventions and patterns used for similar types (e.g.,
AzureClientSecretCredential,AzureOrganizationAuthToken). The comment on line 238 clarifying thattokenis a JSON string is helpful for developers.
249-251: I'm unable to complete the verification due to repository access issues—the codebase cannot be cloned in the sandbox environment. Without access to both the frontend types file and the backend schema definition, I cannot definitively confirm whether the type alignment concern is valid.However, the original review comment raises a reasonable and testable concern: if the backend
CustomCredentialServiceConfigResponseuses a genericOrganizationAuthTokentype but the frontend uses the more specificCustomCredentialServiceOrganizationAuthToken, this could indicate either:
- An intentional frontend-specific typing decision (acceptable if documented)
- A genuine mismatch that could cause runtime type errors
Verify that
CustomCredentialServiceConfigResponse.tokentype matches the actual API response structure by inspecting the backend schema inskyvern/forge/sdk/schemas/organizations.py.skyvern-frontend/src/components/CustomCredentialServiceConfigForm.tsx (1)
37-213: Well-designed form component with good UX patterns.The component includes:
- Proper form validation with Zod
- Token visibility toggle for security
- Clear status indicators
- Test connection functionality before saving
- Disabled states during loading/updating
- Informative metadata display
skyvern-frontend/src/hooks/useCustomCredentialServiceConfig.ts (1)
72-143: Connection test implementation is well-thought-out.Good design decisions:
- Using native
fetchfor external API calls (appropriate since it's testing an external service, not the Skyvern API)- 10-second timeout with AbortController
- Treating 4xx as "reachable" since they indicate server connectivity
- Clear, user-friendly error messages for different failure scenarios
The approach of considering 401/403/404/405 as successful connections (with appropriate messaging) aligns with the documented behavior in the troubleshooting guide.
skyvern/forge/sdk/services/credential/custom_credential_vault_service.py (2)
18-28: LGTM!The class structure and initialization follow the existing
CredentialVaultServicepattern well. The optional client parameter allows for both global configuration (via environment variables) and per-organization configuration.
180-193: LGTM!The no-op implementation for
post_delete_credential_itemis appropriate since the custom vault handles deletion synchronously. The logging provides useful traceability.skyvern/forge/sdk/api/custom_credential_client.py (3)
31-36: LGTM!The auth headers implementation is clean and follows best practices for Bearer token authentication.
96-153: LGTM for the create flow, with one minor note.The create_credential method has good error handling and logging. The response validation for the
idfield is appropriate.
220-262: LGTM!The delete_credential implementation follows the same pattern as other methods and appropriately handles HTTP errors. Just ensure exception chaining is added per the earlier suggestion.
skyvern/forge/sdk/routes/credentials.py (3)
621-664: LGTM!The GET endpoint follows the established pattern from OnePassword and Azure credential endpoints. Authorization is properly handled via
org_auth_service.get_current_org. The static analysis warning aboutDepends(B008) is a false positive for FastAPI.
735-738: LGTM!The
_get_credential_vault_servicefunction is correctly extended to handleCredentialVaultType.CUSTOM, following the same pattern as the existing vault types.
689-704: Manual verification required—repository access unavailable to confirm type hint overloads.The review comment raises a concern about
get_valid_org_auth_tokenlacking type hints for thecustom_credential_servicetoken type. However, repository access is blocked, and web searches cannot locate the enum or method definitions.The code snippet shows the method is being called with
OrganizationAuthTokenType.custom_credential_service, which suggests either:
- The enum has been extended in this PR to include the new token type, and the method's type hints need updating alongside it
- The method already supports generic token types and explicit overloads aren't necessary
This requires manual verification by inspecting:
- The
OrganizationAuthTokenTypeenum definition to confirmcustom_credential_serviceexists- The
get_valid_org_auth_tokenmethod signature and its overload definitions
| try: | ||
| # Get the API client for this organization | ||
| client = await self._get_client_for_organization(organization_id) | ||
|
|
||
| # Create credential in the external API | ||
| item_id = await client.create_credential( | ||
| name=data.name, | ||
| credential=data.credential, | ||
| ) | ||
|
|
||
| # Create record in Skyvern database | ||
| credential = await self._create_db_credential( | ||
| organization_id=organization_id, | ||
| data=data, | ||
| item_id=item_id, | ||
| vault_type=CredentialVaultType.CUSTOM, | ||
| ) | ||
|
|
||
| LOG.info( | ||
| "Successfully created credential in custom vault", | ||
| organization_id=organization_id, | ||
| credential_id=credential.credential_id, | ||
| item_id=item_id, | ||
| ) | ||
|
|
||
| return credential | ||
|
|
||
| except Exception as e: | ||
| LOG.error( | ||
| "Failed to create credential in custom vault", | ||
| organization_id=organization_id, | ||
| name=data.name, | ||
| credential_type=data.credential_type, | ||
| error=str(e), | ||
| exc_info=True, | ||
| ) | ||
| raise |
There was a problem hiding this comment.
Potential orphaned external credential on DB creation failure.
If client.create_credential succeeds (line 105) but _create_db_credential fails (line 111), the credential exists in the external vault but not in the local database. Consider wrapping in a try-except to attempt cleanup of the external credential on DB failure.
try:
# Get the API client for this organization
client = await self._get_client_for_organization(organization_id)
# Create credential in the external API
item_id = await client.create_credential(
name=data.name,
credential=data.credential,
)
- # Create record in Skyvern database
- credential = await self._create_db_credential(
- organization_id=organization_id,
- data=data,
- item_id=item_id,
- vault_type=CredentialVaultType.CUSTOM,
- )
+ # Create record in Skyvern database
+ try:
+ credential = await self._create_db_credential(
+ organization_id=organization_id,
+ data=data,
+ item_id=item_id,
+ vault_type=CredentialVaultType.CUSTOM,
+ )
+ except Exception:
+ # Attempt to clean up the external credential
+ LOG.warning(
+ "DB creation failed, attempting to clean up external credential",
+ organization_id=organization_id,
+ item_id=item_id,
+ )
+ try:
+ await client.delete_credential(item_id)
+ except Exception as cleanup_error:
+ LOG.error(
+ "Failed to clean up orphaned external credential",
+ organization_id=organization_id,
+ item_id=item_id,
+ error=str(cleanup_error),
+ )
+ raise🧰 Tools
🪛 Ruff (0.14.6)
125-125: Consider moving this statement to an else block
(TRY300)
| async def delete_credential(self, credential: Credential) -> None: | ||
| """ | ||
| Delete a credential from the custom vault and database. | ||
|
|
||
| Args: | ||
| credential: Credential record to delete | ||
| """ | ||
| LOG.info( | ||
| "Deleting credential from custom vault", | ||
| organization_id=credential.organization_id, | ||
| credential_id=credential.credential_id, | ||
| item_id=credential.item_id, | ||
| ) | ||
|
|
||
| try: | ||
| # Delete from Skyvern database first | ||
| await app.DATABASE.delete_credential(credential.credential_id, credential.organization_id) | ||
|
|
||
| # Get the API client for this organization | ||
| client = await self._get_client_for_organization(credential.organization_id) | ||
|
|
||
| # Delete from external API | ||
| await client.delete_credential(credential.item_id) | ||
|
|
||
| LOG.info( | ||
| "Successfully deleted credential from custom vault", | ||
| organization_id=credential.organization_id, | ||
| credential_id=credential.credential_id, | ||
| item_id=credential.item_id, | ||
| ) | ||
|
|
||
| except Exception as e: | ||
| LOG.error( | ||
| "Failed to delete credential from custom vault", | ||
| organization_id=credential.organization_id, | ||
| credential_id=credential.credential_id, | ||
| item_id=credential.item_id, | ||
| error=str(e), | ||
| exc_info=True, | ||
| ) | ||
| raise |
There was a problem hiding this comment.
Potential data inconsistency: external API deletion may fail after DB deletion.
The delete operation deletes from the database first (line 154), then from the external API (line 160). If the external API call fails, the credential is already deleted from the local database but still exists in the external vault, creating an orphaned credential.
Consider reversing the order or implementing a compensating transaction:
async def delete_credential(self, credential: Credential) -> None:
LOG.info(
"Deleting credential from custom vault",
organization_id=credential.organization_id,
credential_id=credential.credential_id,
item_id=credential.item_id,
)
try:
- # Delete from Skyvern database first
- await app.DATABASE.delete_credential(credential.credential_id, credential.organization_id)
-
# Get the API client for this organization
client = await self._get_client_for_organization(credential.organization_id)
# Delete from external API
await client.delete_credential(credential.item_id)
+ # Delete from Skyvern database after successful external deletion
+ await app.DATABASE.delete_credential(credential.credential_id, credential.organization_id)
+
LOG.info(
"Successfully deleted credential from custom vault",
organization_id=credential.organization_id,
credential_id=credential.credential_id,
item_id=credential.item_id,
)
except Exception as e:
LOG.error(
"Failed to delete credential from custom vault",
organization_id=credential.organization_id,
credential_id=credential.credential_id,
item_id=credential.item_id,
error=str(e),
exc_info=True,
)
raise🤖 Prompt for AI Agents
In skyvern/forge/sdk/services/credential/custom_credential_vault_service.py
around lines 138 to 178, the code deletes the credential from the local DB
before deleting it from the external vault, which can leave an orphaned external
credential if the external delete fails; change the sequence to call the
external client's delete_credential(item_id) first and only if that call
succeeds call app.DATABASE.delete_credential(credential_id, organization_id),
add logging around both steps, propagate errors so failures prevent local
deletion, and optionally add retry logic or a compensating rollback strategy if
your external API semantics require it.
1 participant
Add support for custom credential service allowing Skyvern to integrate with external HTTP APIs for credential management. This provides flexibility for organizations that want to use their own credential management systems.
Features:
Backend changes:
Frontend changes:
Documentation:
🔌 This PR adds a custom credential service feature that allows Skyvern to integrate with external HTTP APIs for credential management, providing organizations the flexibility to use their own credential infrastructure instead of relying solely on third-party services like Bitwarden.
🔍 Detailed Analysis
Key Changes
CustomCredentialAPIClientfor HTTP communication with external credential APIs using Bearer token authenticationCustomCredentialVaultServicethat integrates with Skyvern's existing credential vault architectureTechnical Implementation
sequenceDiagram participant UI as Skyvern UI participant API as Skyvern API participant Vault as Custom Credential Service participant DB as Database UI->>API: Configure custom credential service API->>DB: Store encrypted configuration UI->>API: Create credential API->>Vault: POST /credentials (with Bearer token) Vault-->>API: Return credential ID API->>DB: Store credential metadata UI->>API: Retrieve credential API->>DB: Get credential metadata API->>Vault: GET /credentials/{id} Vault-->>API: Return credential data API-->>UI: Return credential UI->>API: Delete credential API->>DB: Delete metadata API->>Vault: DELETE /credentials/{id}Impact
Created with Palmier
Important
Add custom credential service feature for integrating with external HTTP APIs, including backend and frontend changes, configuration options, and documentation.
CustomCredentialAPIClientfor HTTP API communication with Bearer token authentication.CustomCredentialVaultServicefor credential management, integrating withCredentialVaultServiceinterface.credentials.pyand configuration schemas inschemas/organizations.py.CUSTOMcredential vault type inschemas/credentials.pyanddb/enums.py.forge_app.pyand credential vault registry.CustomCredentialServiceConfigFormcomponent anduseCustomCredentialServiceConfighook for UI configuration and connection testing.types.ts.Settings.tsx.custom-credential-service.mdx.README.mdandintroduction.mdxwith custom credential service support.This description was created by
for 3b56f29. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.