diff --git a/skyvern-frontend/src/api/types.ts b/skyvern-frontend/src/api/types.ts index eb5074ac..fb49527b 100644 --- a/skyvern-frontend/src/api/types.ts +++ b/skyvern-frontend/src/api/types.ts @@ -501,6 +501,7 @@ export type Createv2TaskRequest = { export type PasswordCredentialApiResponse = { username: string; totp_type: "authenticator" | "email" | "text" | "none"; + totp_identifier?: string | null; }; export type CreditCardCredentialApiResponse = { diff --git a/skyvern-frontend/src/routes/credentials/CredentialItem.tsx b/skyvern-frontend/src/routes/credentials/CredentialItem.tsx index 2857cbea..0c204dc7 100644 --- a/skyvern-frontend/src/routes/credentials/CredentialItem.tsx +++ b/skyvern-frontend/src/routes/credentials/CredentialItem.tsx @@ -4,14 +4,27 @@ import { isPasswordCredential, isSecretCredential, } from "@/api/types"; +import { useState } from "react"; +import { Pencil1Icon } from "@radix-ui/react-icons"; +import { Button } from "@/components/ui/button"; +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from "@/components/ui/tooltip"; import { DeleteCredentialButton } from "./DeleteCredentialButton"; +import { CredentialsModal } from "./CredentialsModal"; +import { credentialTypeToModalType } from "./useCredentialModalState"; type Props = { credential: CredentialApiResponse; }; function CredentialItem({ credential }: Props) { + const [editModalOpen, setEditModalOpen] = useState(false); const credentialData = credential.credential; + const modalType = credentialTypeToModalType(credential.credential_type); const getTotpTypeDisplay = (totpType: string) => { switch (totpType) { case "authenticator": @@ -98,9 +111,31 @@ function CredentialItem({ credential }: Props) {

{credential.credential_id}

{credentialDetails} -
+
+ + + + + + Edit Credential + +
+
); } diff --git a/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx b/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx index 625eeb15..a0337abf 100644 --- a/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx +++ b/skyvern-frontend/src/routes/credentials/CredentialsModal.tsx @@ -9,18 +9,26 @@ import { useCredentialModalState, CredentialModalTypes, } from "./useCredentialModalState"; +import type { CredentialModalType } from "./useCredentialModalState"; import { PasswordCredentialContent } from "./PasswordCredentialContent"; import { SecretCredentialContent } from "./SecretCredentialContent"; import { useState, useEffect } from "react"; import { Button } from "@/components/ui/button"; import { CreditCardCredentialContent } from "./CreditCardCredentialContent"; import { useMutation, useQueryClient } from "@tanstack/react-query"; -import { CreateCredentialRequest } from "@/api/types"; +import { + CreateCredentialRequest, + CredentialApiResponse, + isPasswordCredential, + isCreditCardCredential, + isSecretCredential, +} from "@/api/types"; import { getClient } from "@/api/AxiosClient"; import { useCredentialGetter } from "@/hooks/useCredentialGetter"; import { toast } from "@/components/ui/use-toast"; import { AxiosError } from "axios"; -import { ReloadIcon } from "@radix-ui/react-icons"; +import { InfoCircledIcon, ReloadIcon } from "@radix-ui/react-icons"; +import { Alert, AlertDescription } from "@/components/ui/alert"; import { useCredentialsQuery } from "@/routes/workflows/hooks/useCredentialsQuery"; const PASSWORD_CREDENTIAL_INITIAL_VALUES = { @@ -70,24 +78,33 @@ type Props = { /** Optional controlled mode: pass isOpen and onOpenChange to control modal state locally */ isOpen?: boolean; onOpenChange?: (open: boolean) => void; + /** When provided, the modal opens in edit mode and pre-fills available fields */ + editingCredential?: CredentialApiResponse; + /** Override the modal type (used in edit mode to set the correct form) */ + overrideType?: CredentialModalType; }; function CredentialsModal({ onCredentialCreated, isOpen: controlledIsOpen, onOpenChange: controlledOnOpenChange, + editingCredential, + overrideType, }: Props) { const credentialGetter = useCredentialGetter(); const queryClient = useQueryClient(); const { isOpen: urlIsOpen, - type, + type: urlType, setIsOpen: setUrlIsOpen, } = useCredentialModalState(); + const isEditMode = !!editingCredential; + // Use controlled props if provided, otherwise fall back to URL-based state const isOpen = controlledIsOpen ?? urlIsOpen; const setIsOpen = controlledOnOpenChange ?? setUrlIsOpen; + const type = overrideType ?? urlType; const { data: credentials } = useCredentialsQuery({ page_size: 100, }); @@ -101,10 +118,43 @@ function CredentialsModal({ SECRET_CREDENTIAL_INITIAL_VALUES, ); - // Set default name when modal opens + // Set default name when modal opens, or pre-populate fields in edit mode useEffect(() => { - if (isOpen && credentials) { - const existingNames = credentials.map((cred) => cred.name); + if (!isOpen) return; + + if (isEditMode) { + reset(); + const cred = editingCredential.credential; + if (isPasswordCredential(cred)) { + setPasswordCredentialValues({ + name: editingCredential.name, + username: cred.username, + password: "", + totp: "", + totp_type: cred.totp_type, + totp_identifier: cred.totp_identifier ?? "", + }); + } else if (isCreditCardCredential(cred)) { + setCreditCardCredentialValues({ + name: editingCredential.name, + cardNumber: "", + cardExpirationDate: "", + cardCode: "", + cardBrand: cred.brand, + cardHolderName: "", + }); + } else if (isSecretCredential(cred)) { + setSecretCredentialValues({ + name: editingCredential.name, + secretLabel: cred.secret_label ?? "", + secretValue: "", + }); + } + return; + } + + if (credentials) { + const existingNames = credentials.map((c) => c.name); const defaultName = generateDefaultCredentialName(existingNames); setPasswordCredentialValues((prev) => ({ @@ -120,7 +170,7 @@ function CredentialsModal({ name: defaultName, })); } - }, [isOpen, credentials]); + }, [isOpen, credentials, isEditMode, editingCredential]); function reset() { setPasswordCredentialValues(PASSWORD_CREDENTIAL_INITIAL_VALUES); @@ -157,6 +207,41 @@ function CredentialsModal({ }, }); + const updateCredentialMutation = useMutation({ + mutationFn: async (request: CreateCredentialRequest) => { + const client = await getClient(credentialGetter, "sans-api-v1"); + const response = await client.post( + `/credentials/${editingCredential?.credential_id}/update`, + request, + ); + return response.data; + }, + onSuccess: () => { + reset(); + setIsOpen(false); + queryClient.invalidateQueries({ + queryKey: ["credentials"], + }); + toast({ + title: "Credential updated", + description: "Your credential has been updated successfully", + variant: "success", + }); + }, + onError: (error: AxiosError) => { + const detail = (error.response?.data as { detail?: string })?.detail; + toast({ + title: "Error", + description: detail ? detail : error.message, + variant: "destructive", + }); + }, + }); + + const activeMutation = isEditMode + ? updateCredentialMutation + : createCredentialMutation; + const handleSave = () => { const name = type === CredentialModalTypes.PASSWORD @@ -187,7 +272,7 @@ function CredentialsModal({ }); return; } - createCredentialMutation.mutate({ + activeMutation.mutate({ name, credential_type: "password", credential: { @@ -242,7 +327,7 @@ function CredentialsModal({ } // remove all spaces from the card number const number = creditCardCredentialValues.cardNumber.replace(/\s/g, ""); - createCredentialMutation.mutate({ + activeMutation.mutate({ name, credential_type: "credit_card", credential: { @@ -267,7 +352,7 @@ function CredentialsModal({ return; } - createCredentialMutation.mutate({ + activeMutation.mutate({ name, credential_type: "secret", credential: { @@ -315,18 +400,26 @@ function CredentialsModal({ > - Add Credential + + {isEditMode ? "Edit Credential" : "Add Credential"} + + {isEditMode && ( + + + + For security, saved passwords and secrets are never retrieved. + Please re-enter all fields to update this credential. + + + )} {credentialContent} - diff --git a/skyvern-frontend/src/routes/credentials/useCredentialModalState.ts b/skyvern-frontend/src/routes/credentials/useCredentialModalState.ts index be336d85..c41324ef 100644 --- a/skyvern-frontend/src/routes/credentials/useCredentialModalState.ts +++ b/skyvern-frontend/src/routes/credentials/useCredentialModalState.ts @@ -57,4 +57,25 @@ function useCredentialModalState(): ReturnType { }; } +/** + * Convert a backend credential_type ("password" | "credit_card" | "secret") + * to the modal type used by CredentialsModal ("password" | "credit-card" | "secret"). + */ +export function credentialTypeToModalType( + credentialType: "password" | "credit_card" | "secret", +): CredentialModalType { + switch (credentialType) { + case "password": + return CredentialModalTypes.PASSWORD; + case "credit_card": + return CredentialModalTypes.CREDIT_CARD; + case "secret": + return CredentialModalTypes.SECRET; + default: { + const _exhaustive: never = credentialType; + throw new Error(`Unhandled credential type: ${_exhaustive}`); + } + } +} + export { useCredentialModalState }; diff --git a/skyvern/forge/sdk/routes/credentials.py b/skyvern/forge/sdk/routes/credentials.py index af6a528e..0a3e686f 100644 --- a/skyvern/forge/sdk/routes/credentials.py +++ b/skyvern/forge/sdk/routes/credentials.py @@ -1,3 +1,30 @@ +"""Credential management API endpoints. + +SECURITY INVARIANT — NO RAW CREDENTIAL RETRIEVAL +================================================= +Credential endpoints must NEVER return sensitive credential data (passwords, +TOTP secrets, full card numbers, CVVs, expiration dates, card holder names, +or secret values) in any API response. The only fields that may be returned +are non-sensitive metadata: + + - Password credentials: ``username``, ``totp_type``, ``totp_identifier`` + - Credit card credentials: ``last_four``, ``brand`` + - Secret credentials: ``secret_label`` + +This is enforced by the ``*CredentialResponse`` Pydantic models and the +``_convert_to_response()`` helper. When adding new credential types or +modifying existing ones, ensure that: + + 1. The response model never includes the raw secret material. + 2. The ``_convert_to_response()`` function only maps non-sensitive fields. + 3. No endpoint (including ``get_credential`` and ``get_credentials``) ever + fetches and returns the decrypted secret from the vault. + +Violating this invariant would allow any caller with a valid API key to +exfiltrate stored passwords, card numbers, and secrets — which is the +exact threat the vault architecture is designed to prevent. +""" + import json import structlog @@ -442,6 +469,13 @@ async def get_credential( ), current_org: Organization = Depends(org_auth_service.get_current_org), ) -> CredentialResponse: + """Return non-sensitive metadata for a single credential. + + SECURITY: This endpoint intentionally does NOT return the raw secret + material (password, card number, CVV, secret value, etc.). Only + non-sensitive fields are included in the response. See the module + docstring for the full security invariant. + """ credential = await app.DATABASE.get_credential( credential_id=credential_id, organization_id=current_org.organization_id ) @@ -493,6 +527,11 @@ async def get_credentials( openapi_extra={"x-fern-sdk-parameter-name": "page_size"}, ), ) -> list[CredentialResponse]: + """Return non-sensitive metadata for all credentials (paginated). + + SECURITY: Like ``get_credential``, this endpoint never returns raw secret + material. See the module docstring for the full security invariant. + """ credentials = await app.DATABASE.get_credentials(current_org.organization_id, page=page, page_size=page_size) return [_convert_to_response(credential) for credential in credentials] @@ -825,6 +864,13 @@ async def _get_credential_vault_service() -> CredentialVaultService: def _convert_to_response(credential: Credential) -> CredentialResponse: + """Convert an internal ``Credential`` to a safe API response. + + SECURITY: This function must ONLY copy non-sensitive metadata into the + response. Never include passwords, TOTP secrets, full card numbers, CVVs, + expiration dates, card holder names, or secret values. See the module + docstring for the full security invariant. + """ if credential.credential_type == CredentialType.PASSWORD: credential_response = PasswordCredentialResponse( username=credential.username or credential.credential_id, diff --git a/skyvern/forge/sdk/schemas/credentials.py b/skyvern/forge/sdk/schemas/credentials.py index 236f2854..f4689376 100644 --- a/skyvern/forge/sdk/schemas/credentials.py +++ b/skyvern/forge/sdk/schemas/credentials.py @@ -28,7 +28,10 @@ class TotpType(StrEnum): class PasswordCredentialResponse(BaseModel): - """Response model for password credentials, containing only the username.""" + """Response model for password credentials — non-sensitive fields only. + + SECURITY: Must NEVER include password, TOTP secret, or TOTP identifier. + """ username: str = Field(..., description="The username associated with the credential", examples=["user@example.com"]) totp_type: TotpType = Field( @@ -44,14 +47,20 @@ class PasswordCredentialResponse(BaseModel): class CreditCardCredentialResponse(BaseModel): - """Response model for credit card credentials, containing only the last four digits and brand.""" + """Response model for credit card credentials — non-sensitive fields only. + + SECURITY: Must NEVER include full card number, CVV, expiration date, or card holder name. + """ last_four: str = Field(..., description="Last four digits of the credit card number", examples=["1234"]) brand: str = Field(..., description="Brand of the credit card", examples=["visa"]) class SecretCredentialResponse(BaseModel): - """Response model for secret credentials.""" + """Response model for secret credentials — non-sensitive fields only. + + SECURITY: Must NEVER include the secret_value. + """ secret_label: str | None = Field(default=None, description="Optional label for the stored secret")