fix: audit — crash fixes, SSRF, API key leak, whitelist merge, stale closure

Frontend:
- SetupTab: restore missing Input import (runtime crash on render)
- SetupTab: wrap onSubmit in event adapter — button outside <form> passed undefined
- Pricing: load() → useCallback, add to useEffect deps (stale closure)
- Pricing: credits_label hardcoded string → t() interpolation

Backend:
- admin/ai-config/test: remove endpoint/api_key from request body (SSRF fix)
  — endpoint and key now read exclusively from DB or env vars
- admin/ai-config/test: str(exc) → sanitized message, full exc logged server-side only
  (prevents API key leak via OpenAI SDK error context)
- admin/ai-config PUT: whitelist allowed fields on provider/model merge
  (prevents injection of arbitrary keys into DB document)
- admin/ai-config PUT: validate active_provider/model are non-empty strings
- admin.py: move deferred imports (time, os, AsyncOpenAI) to module level
- billing.py: guard get_json() with or {} (prevents AttributeError on empty body)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Vadym Samoilenko 2026-05-24 14:51:56 +01:00
parent de990fb486
commit ef7ac32a2b
4 changed files with 60 additions and 35 deletions

View file

@ -12,9 +12,12 @@ Pricing: GET /api/admin/pricing
"""
import logging
import os
import time
from datetime import datetime, timezone, timedelta
from quart import Blueprint, jsonify, request
from bson import ObjectId
from openai import AsyncOpenAI
from app.auth.quart_jwt import jwt_required, get_jwt_identity
from app.utils import admin_required, make_serializable
@ -678,12 +681,18 @@ async def get_ai_config():
}), 200
_PROVIDER_ALLOWED_FIELDS = {'id', 'name', 'enabled', 'endpoint', 'api_key', 'models'}
_MODEL_ALLOWED_FIELDS = {'id', 'display_name', 'role', 'enabled'}
@admin_bp.route('/ai-config', methods=['PUT'])
@jwt_required()
@admin_required
async def update_ai_config():
"""PUT /api/admin/ai-config — update AI provider config.
API key is only updated when passed non-empty.
API key is only updated when passed as a non-empty, non-masked value.
Provider objects are merged against a whitelist of allowed fields.
active_provider/model values are validated to be non-empty strings.
"""
body = await request.get_json() or {}
settings = await get_settings()
@ -691,25 +700,38 @@ async def update_ai_config():
update: dict = {}
if 'active_provider' in body:
update['active_provider'] = body['active_provider']
if 'active_main_model' in body:
update['active_main_model'] = body['active_main_model']
if 'active_mini_model' in body:
update['active_mini_model'] = body['active_mini_model']
for field in ('active_provider', 'active_main_model', 'active_mini_model'):
if field in body:
val = body[field]
if not isinstance(val, str) or not val.strip():
return jsonify({'message': f'{field} must be a non-empty string'}), 400
update[field] = val.strip()
if 'ai_providers' in body:
if not isinstance(body['ai_providers'], list):
return jsonify({'message': 'ai_providers must be a list'}), 400
merged = []
for p in body['ai_providers']:
pid = p.get('id')
existing = existing_providers.get(pid, {})
mp = {**existing, **p}
if not pid or pid not in existing_providers:
continue # silently skip unknown providers — never create new ones from client
existing = existing_providers[pid]
# Whitelist: only allow known fields from client
safe_update = {k: v for k, v in p.items() if k in _PROVIDER_ALLOWED_FIELDS}
mp = {**existing, **safe_update}
# Preserve models whitelist too
if 'models' in safe_update and isinstance(safe_update['models'], list):
mp['models'] = [
{k: v for k, v in m.items() if k in _MODEL_ALLOWED_FIELDS}
for m in safe_update['models']
]
# Only replace key if a non-empty, non-masked value is supplied
new_key = p.get('api_key', '')
new_key = safe_update.get('api_key', '')
if not new_key or '***' in new_key:
mp['api_key'] = existing.get('api_key', '')
merged.append(mp)
update['ai_providers'] = merged
if merged:
update['ai_providers'] = merged
await update_settings(update)
return jsonify({'ok': True}), 200
@ -719,28 +741,28 @@ async def update_ai_config():
@jwt_required()
@admin_required
async def test_ai_config():
"""POST /api/admin/ai-config/test — send a minimal request to the configured endpoint."""
import time
from openai import AsyncOpenAI
"""POST /api/admin/ai-config/test — test connection using DB-stored provider config.
Accepts optional provider_id to select which provider to test.
Never reads endpoint/api_key from the request body to prevent SSRF.
"""
body = await request.get_json() or {}
settings = await get_settings()
providers = {p['id']: p for p in settings.get('ai_providers', [])}
provider_id = body.get('provider_id') or settings.get('active_provider', 'azure_openai')
provider = providers.get(provider_id, {})
endpoint = body.get('endpoint') or provider.get('endpoint', '')
api_key = body.get('api_key', '')
# If masked or empty, fall back to stored key
if not api_key or '***' in api_key:
api_key = provider.get('api_key', '')
# Final fallback: environment variable
if not api_key:
import os
api_key = os.environ.get('AZURE_AI_API_KEY', '')
if not endpoint:
import os
endpoint = os.environ.get('AZURE_AI_ENDPOINT', '')
# Validate provider_id is one we actually know about — prevents probing arbitrary IDs
if provider_id not in providers:
return jsonify({'ok': False, 'error': 'Unknown provider'}), 400
provider = providers[provider_id]
# Endpoint and API key come exclusively from DB or env — never from the request body
endpoint = provider.get('endpoint', '') or os.environ.get('AZURE_AI_ENDPOINT', '')
api_key = provider.get('api_key', '') or os.environ.get('AZURE_AI_API_KEY', '')
if not endpoint or not api_key:
return jsonify({'ok': False, 'error': 'Endpoint or API key not configured'}), 200
model = settings.get('active_main_model', 'gpt-5.4')
@ -756,4 +778,6 @@ async def test_ai_config():
reply = resp.choices[0].message.content.strip() if resp.choices else ''
return jsonify({'ok': True, 'latency_ms': latency_ms, 'reply': reply}), 200
except Exception as exc:
return jsonify({'ok': False, 'error': str(exc)}), 200
# Log full exception (may contain key fragments) only server-side
logger.warning('AI config test failed for provider %s: %s', provider_id, exc)
return jsonify({'ok': False, 'error': 'Connection failed — check endpoint and API key'}), 200

View file

@ -59,7 +59,7 @@ async def get_transactions():
@jwt_required()
async def create_checkout():
user_id = get_jwt_identity()
data = await request.get_json()
data = await request.get_json() or {}
pack_id = data.get("pack_id")
if not pack_id:

View file

@ -13,6 +13,7 @@ import {
FormLabel,
FormMessage,
} from "@/components/ui/form";
import { Input } from "@/components/ui/input";
import { Textarea } from "@/components/ui/textarea";
import {
Select,
@ -274,7 +275,7 @@ export function SetupTab({
</Button>
<Button
type="button"
onClick={onSubmit}
onClick={(e) => onSubmit(e as unknown as React.FormEvent)}
disabled={isGenerating}
className="min-w-32"
>

View file

@ -1,4 +1,4 @@
import { useState, useEffect } from 'react';
import { useState, useEffect, useCallback } from 'react';
import { motion } from 'framer-motion';
import { useNavigate } from 'react-router-dom';
import { CheckCircle2, Info, RefreshCw } from 'lucide-react';
@ -94,7 +94,7 @@ export default function Pricing() {
const [loading, setLoading] = useState(true);
const [error, setError] = useState(false);
const load = () => {
const load = useCallback(() => {
setLoading(true);
setError(false);
billingApi.getPacks()
@ -108,9 +108,9 @@ export default function Pricing() {
})
.catch(() => setError(true))
.finally(() => setLoading(false));
};
}, []);
useEffect(() => { load(); }, []);
useEffect(() => { load(); }, [load]);
const personaCost = data?.persona_cost ?? 2;
const runCost = data?.run_cost ?? 40;
@ -195,7 +195,7 @@ export default function Pricing() {
<span className="text-muted-foreground text-sm mb-1.5">{t('pricing.price_suffix')}</span>
</div>
<p className="text-xs text-primary font-semibold mb-1">{pack.credits} credits included</p>
<p className="text-xs text-primary font-semibold mb-1">{t('pricing.credits_label', { count: pack.credits })}</p>
<div className="mb-6">
<div className="flex items-center justify-between text-[11px] mb-1">