mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(mcp): route empty-API-key 401s to the Configure dialog instead of OAuth (#7203)
This commit is contained in:
parent
323adca6a1
commit
bee2b836ce
7 changed files with 90 additions and 22 deletions
|
|
@ -564,6 +564,25 @@ def _unauthorized_response(message: str, *, include_oauth_challenge: bool = True
|
|||
)
|
||||
|
||||
|
||||
_EMPTY_API_KEY_MESSAGE = (
|
||||
"Empty Skyvern API key. Paste your API key from https://app.skyvern.com Settings into your client's Skyvern "
|
||||
"config (Claude Desktop: Settings -> Extensions -> Skyvern -> Configure)."
|
||||
)
|
||||
_EMPTY_BEARER_MESSAGE = (
|
||||
"Empty Bearer token. Send a valid OAuth token or use an API key from https://app.skyvern.com Settings "
|
||||
"(Claude Desktop: Settings -> Extensions -> Skyvern -> Configure)."
|
||||
)
|
||||
_MISSING_CREDENTIALS_MESSAGE = (
|
||||
"Missing credentials. Use an API key from https://app.skyvern.com Settings (Claude Desktop: Settings -> "
|
||||
"Extensions -> Skyvern -> Configure) or connect with an OAuth-capable client. Docs: "
|
||||
"https://www.skyvern.com/docs/developers/getting-started/mcp"
|
||||
)
|
||||
_INVALID_API_KEY_MESSAGE = (
|
||||
"Invalid Skyvern API key. Check the key from https://app.skyvern.com Settings in your client's Skyvern config "
|
||||
"(Claude Desktop: Settings -> Extensions -> Skyvern -> Configure)."
|
||||
)
|
||||
|
||||
|
||||
def _internal_error_response() -> JSONResponse:
|
||||
return JSONResponse(
|
||||
{"error": {"code": "INTERNAL_ERROR", "message": "Internal server error"}},
|
||||
|
|
@ -604,16 +623,20 @@ class MCPAPIKeyMiddleware:
|
|||
auth_header = request.headers.get(AUTHORIZATION_HEADER, "")
|
||||
if auth_header.startswith(BEARER_PREFIX):
|
||||
bearer_token = auth_header[len(BEARER_PREFIX) :]
|
||||
if not bearer_token:
|
||||
response = _unauthorized_response("Empty Bearer token")
|
||||
if not bearer_token.strip():
|
||||
response = _unauthorized_response(_EMPTY_BEARER_MESSAGE)
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
await self._handle_bearer(scope, receive, send, bearer_token)
|
||||
return
|
||||
|
||||
api_key = request.headers.get(API_KEY_HEADER)
|
||||
if not api_key:
|
||||
response = _unauthorized_response("Missing x-api-key or Authorization: Bearer header")
|
||||
if api_key is None:
|
||||
response = _unauthorized_response(_MISSING_CREDENTIALS_MESSAGE)
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
if not api_key.strip():
|
||||
response = _unauthorized_response(_EMPTY_API_KEY_MESSAGE, include_oauth_challenge=False)
|
||||
await response(scope, receive, send)
|
||||
return
|
||||
|
||||
|
|
@ -696,7 +719,7 @@ class MCPAPIKeyMiddleware:
|
|||
scope["state"]["organization_id"] = validation.organization_id
|
||||
except HTTPException as e:
|
||||
if e.status_code in {401, 403}:
|
||||
response = _unauthorized_response("Invalid API key", include_oauth_challenge=False)
|
||||
response = _unauthorized_response(_INVALID_API_KEY_MESSAGE, include_oauth_challenge=False)
|
||||
elif e.status_code == 503:
|
||||
response = _service_unavailable_response(e.detail or _VALIDATION_RETRY_EXHAUSTED_MESSAGE)
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -56,7 +56,7 @@
|
|||
"api_key": {
|
||||
"type": "string",
|
||||
"title": "Skyvern API Key",
|
||||
"description": "Create an API key in Skyvern Settings, then paste it here.",
|
||||
"description": "Paste your API key from app.skyvern.com -> Settings. Claude cannot use Skyvern until this is set.",
|
||||
"sensitive": true,
|
||||
"required": true
|
||||
},
|
||||
|
|
|
|||
|
|
@ -259,18 +259,15 @@ async def download_file(
|
|||
# response.headers stays a CIMultiDictProxy: dict() would make header
|
||||
# lookups case-sensitive and miss lowercase wire headers.
|
||||
file_name = _determine_download_filename(filename, response.headers, url)
|
||||
allowed_dir = os.path.realpath(download_dir_resolved)
|
||||
resolved_final_path = os.path.realpath(os.path.join(allowed_dir, file_name))
|
||||
final_path = (download_dir_resolved / file_name).resolve()
|
||||
# sanitize_filename strips separators but keeps dots, so a dots-only name can
|
||||
# still resolve outside the download dir; require a direct child. Checked
|
||||
# before streaming so a rejected name downloads zero bytes.
|
||||
if (
|
||||
resolved_final_path == allowed_dir
|
||||
or not resolved_final_path.startswith(allowed_dir + os.sep)
|
||||
or os.path.dirname(resolved_final_path) != allowed_dir
|
||||
not final_path.is_relative_to(download_dir_resolved)
|
||||
or final_path.parent != download_dir_resolved
|
||||
):
|
||||
raise ValueError(f"Unsafe filename derived from download: {file_name!r}")
|
||||
final_path = Path(resolved_final_path)
|
||||
|
||||
temp_file = tempfile.NamedTemporaryFile(mode="wb", dir=download_dir_resolved, delete=False)
|
||||
file_path = Path(temp_file.name).resolve()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue