security: keep provider credentials server-side
This commit is contained in:
parent
d19e52fe28
commit
c6ce3e4803
@ -20,6 +20,9 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
|
||||
### Changed
|
||||
- **API Boundary Hardening.** External API authentication is enabled by default, request and mutation limits are
|
||||
enforced, and settings responses redact provider credentials for non-browser callers.
|
||||
- **Server-side Provider Credentials.** Provider keys are now masked even for same-origin Settings responses, are
|
||||
preserved when masked/empty fields are saved, and model/health probes resolve configured keys inside the server.
|
||||
Browser localStorage is no longer used as a credential store.
|
||||
- **Security Regression Coverage.** Added a Python test gate for character metadata validation, private-download URL
|
||||
rejection and oversized JSON request rejection.
|
||||
- **Navigation Reliability.** Rehearser phase requests now survive the feature-module initialization boundary.
|
||||
|
||||
8
TODO.md
8
TODO.md
@ -17,8 +17,9 @@ the first block before investing in architecture or polish.
|
||||
|
||||
### P0 — do first
|
||||
|
||||
- [ ] **S — Stop API keys from reaching frontend JavaScript or logs.** Audit responses,
|
||||
browser storage, error paths and debug output.
|
||||
- [ ] **S — Stop API keys from reaching frontend JavaScript or logs.** Provider keys are now masked in settings
|
||||
responses and probes use server-side credentials; finish the external-client key handoff and audit remaining
|
||||
transient input/error paths.
|
||||
- [ ] **S — Validate uploaded filenames and paths.** Reject traversal, unsafe names and
|
||||
writes outside the intended data directories.
|
||||
- [ ] **M — Add upload, request-size, temporary-file and concurrent-job limits.** Apply
|
||||
@ -124,7 +125,8 @@ the first block before investing in architecture or polish.
|
||||
|
||||
## P0 — Security
|
||||
|
||||
- [ ] Never expose API keys to frontend JavaScript or logs.
|
||||
- [ ] Never expose API keys to frontend JavaScript or logs. Provider keys are masked on read and absent from browser
|
||||
storage; the deliberate external-client key display and transient entry path still need a final design review.
|
||||
- [ ] Validate uploaded filenames and paths.
|
||||
- [ ] Strictly validate custom URLs to prevent SSRF.
|
||||
- [ ] Add request-size limits, rate limits, and authentication for destructive/API endpoints.
|
||||
|
||||
@ -10,6 +10,7 @@ import requests
|
||||
from fastapi import APIRouter, Header, HTTPException
|
||||
|
||||
from core.docker_client import _docker_get_json, _docker_post
|
||||
from core.config import _load_settings
|
||||
from core.validation import _normalize_service_url
|
||||
|
||||
router = APIRouter()
|
||||
@ -127,7 +128,14 @@ async def probe_url(url: str, type: str = "", provider_api_key: str = Header("",
|
||||
if base.endswith(_suffix):
|
||||
base = base[: -len(_suffix)]
|
||||
break
|
||||
key_to_use = provider_api_key if provider_api_key else "sk-dummy-key"
|
||||
settings = _load_settings()
|
||||
key_to_use = {
|
||||
"anthropic": settings.get("llm_api_key"),
|
||||
"llm": settings.get("llm_api_key"),
|
||||
"openai": settings.get("llm_api_key"),
|
||||
"stt": settings.get("groq_api_key") or settings.get("whisper_api_key"),
|
||||
"tts": settings.get("tts_api_key"),
|
||||
}.get(type) or provider_api_key or "sk-dummy-key"
|
||||
# Anthropic doesn't speak the OpenAI-style Bearer auth every other card
|
||||
# here uses — it needs x-api-key + an anthropic-version header, so the
|
||||
# generic Bearer probe below would 401 even with a perfectly valid key.
|
||||
@ -192,7 +200,8 @@ async def engine_models(url: str, type: str = "", provider_api_key: str = Header
|
||||
last_err = "No response"
|
||||
seen: set[str] = set()
|
||||
models: list[str] = []
|
||||
key_to_use = provider_api_key if provider_api_key else "sk-dummy-key"
|
||||
settings = _load_settings()
|
||||
key_to_use = settings.get("llm_api_key") or provider_api_key or "sk-dummy-key"
|
||||
for endpoint in candidates:
|
||||
try:
|
||||
r = requests.get(endpoint, timeout=6, headers={"Authorization": f"Bearer {key_to_use}", "User-Agent": "TTS-Voice-Creator/models"})
|
||||
|
||||
@ -15,6 +15,7 @@ _SECRET_SETTING_KEYS = {
|
||||
"groq_api_key", "whisper_api_key", "tts_api_key", "voice_design_api_key",
|
||||
"elevenlabs_api_key", "llm_api_key", "engine_api_keys", "external_api_key",
|
||||
}
|
||||
_SECRET_MASK = "__configured__"
|
||||
|
||||
|
||||
@router.get("/api/settings")
|
||||
@ -34,8 +35,17 @@ async def get_settings(request: Request):
|
||||
)
|
||||
except Exception:
|
||||
same_origin = False
|
||||
if not same_origin:
|
||||
s = {key: value for key, value in s.items() if key not in _SECRET_SETTING_KEYS}
|
||||
# Never return provider credentials to browser JavaScript. The mask lets
|
||||
# the Settings UI indicate that a value exists without making it usable in
|
||||
# client-side requests or browser storage.
|
||||
for key in _SECRET_SETTING_KEYS:
|
||||
value = s.get(key)
|
||||
if key == "engine_api_keys" and isinstance(value, dict):
|
||||
s[key] = {name: _SECRET_MASK for name, secret in value.items() if secret}
|
||||
elif value:
|
||||
s[key] = _SECRET_MASK
|
||||
else:
|
||||
s.pop(key, None)
|
||||
return s
|
||||
|
||||
|
||||
@ -52,7 +62,26 @@ async def regenerate_api_key():
|
||||
async def post_settings(request: Request):
|
||||
data = await request.json()
|
||||
s = _load_settings()
|
||||
s.update({k: v for k, v in data.items() if k in _SETTINGS_KEYS})
|
||||
clear = {str(k) for k in (data.get("clear_secrets") or [])} if isinstance(data.get("clear_secrets"), list) else set()
|
||||
for key, value in data.items():
|
||||
if key not in _SETTINGS_KEYS:
|
||||
continue
|
||||
if key in _SECRET_SETTING_KEYS:
|
||||
if value == _SECRET_MASK:
|
||||
continue
|
||||
if key == "engine_api_keys" and isinstance(value, dict):
|
||||
old = s.get(key) if isinstance(s.get(key), dict) else {}
|
||||
merged = dict(old)
|
||||
for name, secret in value.items():
|
||||
if secret and secret != _SECRET_MASK:
|
||||
merged[name] = secret
|
||||
for name in clear:
|
||||
merged.pop(name, None)
|
||||
s[key] = merged
|
||||
continue
|
||||
if value in (None, "") and key not in clear:
|
||||
continue
|
||||
s[key] = value
|
||||
s = _normalize_settings(s)
|
||||
_save_settings(s)
|
||||
return {"ok": True}
|
||||
|
||||
@ -19,13 +19,11 @@
|
||||
});
|
||||
});
|
||||
|
||||
// API key inputs — persist to localStorage, eye toggle, saved badge
|
||||
// API key inputs — save only to the server, eye toggle, saved badge
|
||||
document.querySelectorAll('.llm-input[data-llm-key]').forEach(inp => {
|
||||
const key = inp.dataset.llmKey;
|
||||
const storageKey = 'llm-key-' + key;
|
||||
const saved = typeof window.engineApiKeyValue === 'function'
|
||||
? window.engineApiKeyValue(key, storageKey)
|
||||
: localStorage.getItem(storageKey);
|
||||
const saved = typeof window.engineApiKeyValue === 'function' ? window.engineApiKeyValue(key) : '';
|
||||
if (saved) inp.value = saved;
|
||||
|
||||
const eye = document.createElement('button');
|
||||
@ -47,8 +45,8 @@
|
||||
clearTimeout(t);
|
||||
t = setTimeout(() => {
|
||||
if (typeof window.saveEngineApiKey === 'function') window.saveEngineApiKey(key, inp.value, storageKey);
|
||||
else if (inp.value) localStorage.setItem(storageKey, inp.value);
|
||||
else localStorage.removeItem(storageKey);
|
||||
// No browser-storage fallback: provider credentials belong on the
|
||||
// server and must not survive in localStorage.
|
||||
badge.hidden = false;
|
||||
setTimeout(() => { badge.hidden = true; }, 1800);
|
||||
}, 600);
|
||||
@ -142,8 +140,7 @@
|
||||
const url = normalizeProbeUrl(rawUrl);
|
||||
const params = { url };
|
||||
if (type) params.type = type;
|
||||
const headers = apiKey ? { 'X-Provider-API-Key': apiKey } : {};
|
||||
const r = await fetch('/api/probe-url?' + new URLSearchParams(params), { headers });
|
||||
const r = await fetch('/api/probe-url?' + new URLSearchParams(params));
|
||||
return r.json();
|
||||
}
|
||||
|
||||
@ -207,7 +204,11 @@
|
||||
const resp = await fetch('/api/settings').then(r => r.json());
|
||||
const updated = { ...resp, ...patch };
|
||||
await fetch('/api/settings', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(patch) });
|
||||
Object.assign(_appSettings || {}, patch);
|
||||
if (_appSettings) {
|
||||
const visiblePatch = { ...patch };
|
||||
['groq_api_key','whisper_api_key','tts_api_key','voice_design_api_key','elevenlabs_api_key','llm_api_key','engine_api_keys','external_api_key'].forEach(k => delete visiblePatch[k]);
|
||||
Object.assign(_appSettings, visiblePatch);
|
||||
}
|
||||
// refresh visible inputs in Settings if open
|
||||
for (const [id, val] of Object.entries(patch)) {
|
||||
const inp = $('s-' + id.replace(/_/g, '-'));
|
||||
@ -310,8 +311,7 @@
|
||||
if (refreshBtn) refreshBtn.disabled = true;
|
||||
try {
|
||||
const fetchUrl = '/api/conversation/llm-models?url=' + encodeURIComponent(url);
|
||||
const headers = apiKey ? { 'X-Provider-API-Key': apiKey } : {};
|
||||
const d = await fetch(fetchUrl, { headers }).then(r => r.json());
|
||||
const d = await fetch(fetchUrl).then(r => r.json());
|
||||
if (d.error) {
|
||||
let msg = 'Failed';
|
||||
try {
|
||||
|
||||
@ -522,7 +522,7 @@ function renderLocalContainers(containers) {
|
||||
// API key row — only for LLMs
|
||||
const savedApiKey = isCustom
|
||||
? ((typeof window.engineApiKeyValue === 'function' ? window.engineApiKeyValue(c.name, 'dc-apikey-' + c.name) : '') || c.apiKey || '')
|
||||
: ((typeof window.engineApiKeyValue === 'function' ? window.engineApiKeyValue(c.name, 'dc-apikey-' + c.name) : '') || (_appSettings?.engine_api_keys?.[c.name]) || localStorage.getItem('dc-apikey-' + n) || '');
|
||||
: ((typeof window.engineApiKeyValue === 'function' ? window.engineApiKeyValue(c.name) : '') || '');
|
||||
const apiKeyRowHtml = (c.role === 'llm' || c.name === 'litellm') ? `
|
||||
<div class="llm-local-url">
|
||||
<span class="llm-local-url-label" title="API Key"><span class="mdi mdi-key-outline"></span></span>
|
||||
@ -673,21 +673,18 @@ function renderLocalContainers(containers) {
|
||||
}
|
||||
});
|
||||
|
||||
// API key input — save to server-backed engine_api_keys and localStorage fallback
|
||||
// API key input — save only to server-backed engine_api_keys
|
||||
grid.querySelectorAll('.dc-apikey-inp[data-dc-apikey-key]').forEach(inp => {
|
||||
const key = inp.dataset.dcApikeyKey;
|
||||
const storageKey = 'dc-apikey-' + key;
|
||||
const saved = typeof window.engineApiKeyValue === 'function'
|
||||
? window.engineApiKeyValue(key, storageKey)
|
||||
: localStorage.getItem(storageKey);
|
||||
const saved = typeof window.engineApiKeyValue === 'function' ? window.engineApiKeyValue(key) : '';
|
||||
if (saved && inp.value !== saved) inp.value = saved;
|
||||
let keyTimer;
|
||||
inp.addEventListener('input', () => {
|
||||
clearTimeout(keyTimer);
|
||||
keyTimer = setTimeout(() => {
|
||||
if (typeof window.saveEngineApiKey === 'function') window.saveEngineApiKey(key, inp.value, storageKey);
|
||||
else if (inp.value) localStorage.setItem(storageKey, inp.value);
|
||||
else localStorage.removeItem(storageKey);
|
||||
// No browser-storage fallback for provider credentials.
|
||||
}, 300);
|
||||
});
|
||||
});
|
||||
@ -727,7 +724,6 @@ function renderLocalContainers(containers) {
|
||||
const url = inp?.value.trim() || inp?.placeholder || '';
|
||||
if (settingKey && url) {
|
||||
const patch = { [settingKey]: url };
|
||||
if (apiKey && settingKey === 'llm_url') patch.llm_api_key = apiKey;
|
||||
await window.applyAndSaveSettings(patch);
|
||||
toast(`✓ Connected and set as active ${useBtn.textContent.replace('Use as ', '').trim()}`, 'success');
|
||||
} else {
|
||||
|
||||
@ -239,7 +239,11 @@ async function refreshTtsBackendAvailability(selected = '') {
|
||||
async function _patchSettings(patch) {
|
||||
try {
|
||||
await fetch('/api/settings', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(patch) });
|
||||
if (_appSettings) Object.assign(_appSettings, patch || {});
|
||||
if (_appSettings) {
|
||||
const visiblePatch = { ...(patch || {}) };
|
||||
['groq_api_key','whisper_api_key','tts_api_key','voice_design_api_key','elevenlabs_api_key','llm_api_key','engine_api_keys','external_api_key'].forEach(k => delete visiblePatch[k]);
|
||||
Object.assign(_appSettings, visiblePatch);
|
||||
}
|
||||
if (typeof window.refreshStatusBarEngines === 'function') window.refreshStatusBarEngines({ force: true });
|
||||
} catch (e) { console.warn('[settings] patch failed', e); }
|
||||
}
|
||||
@ -282,24 +286,24 @@ const ENGINE_API_KEY_SETTING_MAP = {
|
||||
const _engineApiKeySaveTimers = {};
|
||||
function engineApiKeyValue(key, localStorageKey = '') {
|
||||
const settingKey = ENGINE_API_KEY_SETTING_MAP[key];
|
||||
const fromSetting = settingKey && _appSettings ? (_appSettings[settingKey] || '') : '';
|
||||
const fromMap = _appSettings?.engine_api_keys?.[key] || '';
|
||||
const fromLocal = localStorageKey ? localStorage.getItem(localStorageKey) : '';
|
||||
return fromSetting || fromMap || fromLocal || localStorage.getItem('llm-key-' + key) || localStorage.getItem('dc-apikey-' + key) || '';
|
||||
const fromSetting = settingKey && _appSettings && _appSettings[settingKey] !== '__configured__' ? (_appSettings[settingKey] || '') : '';
|
||||
const fromMap = _appSettings?.engine_api_keys?.[key] && _appSettings.engine_api_keys[key] !== '__configured__' ? _appSettings.engine_api_keys[key] : '';
|
||||
return fromSetting || fromMap || '';
|
||||
}
|
||||
function saveEngineApiKey(key, value, localStorageKey = '') {
|
||||
const val = String(value || '').trim();
|
||||
if (localStorageKey) {
|
||||
if (val) localStorage.setItem(localStorageKey, val);
|
||||
else localStorage.removeItem(localStorageKey);
|
||||
}
|
||||
const apiKeys = { ...(_appSettings?.engine_api_keys || {}) };
|
||||
if (val) apiKeys[key] = val;
|
||||
else delete apiKeys[key];
|
||||
const patch = { engine_api_keys: apiKeys };
|
||||
const settingKey = ENGINE_API_KEY_SETTING_MAP[key];
|
||||
if (settingKey) patch[settingKey] = val;
|
||||
if (_appSettings) Object.assign(_appSettings, patch);
|
||||
if (_appSettings) {
|
||||
if (settingKey) _appSettings[settingKey] = val ? '__configured__' : undefined;
|
||||
const safeMap = { ...(_appSettings.engine_api_keys || {}) };
|
||||
if (val) safeMap[key] = '__configured__'; else delete safeMap[key];
|
||||
_appSettings.engine_api_keys = safeMap;
|
||||
}
|
||||
clearTimeout(_engineApiKeySaveTimers[key]);
|
||||
_engineApiKeySaveTimers[key] = setTimeout(() => _patchSettings(patch), 600);
|
||||
}
|
||||
@ -495,12 +499,13 @@ function _populateImageGenModelSelect(selectEl, provider, savedModel) {
|
||||
async function loadSettings() {
|
||||
const s = await fetch('/api/settings').then(r => r.json());
|
||||
const sv = (id, val) => { const el = $(id); if (el) el.value = val; };
|
||||
const svSecret = (id, val) => { const el = $(id); if (el) { el.value = ''; el.placeholder = val === '__configured__' ? 'Configured — enter a new key to replace' : ''; } };
|
||||
sv('s-whisper-url', s.whisper_url || '');
|
||||
sv('s-whisper-key', s.whisper_api_key || '');
|
||||
svSecret('s-whisper-key', s.whisper_api_key);
|
||||
sv('s-tts-url', s.tts_url || '');
|
||||
sv('s-faster-whisper-url', s.faster_whisper_url || '');
|
||||
sv('s-whisper-cpp-url', s.whisper_cpp_url || '');
|
||||
sv('s-groq-api-key', s.groq_api_key || '');
|
||||
svSecret('s-groq-api-key', s.groq_api_key);
|
||||
sv('s-kokoro-url', s.kokoro_url || '');
|
||||
sv('s-vibevoice-url', s.vibevoice_url || '');
|
||||
sv('s-xtts-url', s.xtts_url || '');
|
||||
@ -542,7 +547,7 @@ async function loadSettings() {
|
||||
sv('s-nvidia-zeroshot-url', s.nvidia_zeroshot_url || s.nvidia_clone_url || 'http://host.docker.internal:8093');
|
||||
sv('s-nvidia-flow-url', s.nvidia_flow_url || 'http://host.docker.internal:8094');
|
||||
sv('s-tts-stream-mode', s.tts_stream_mode || 'auto');
|
||||
sv('s-tts-key', s.tts_api_key || '');
|
||||
svSecret('s-tts-key', s.tts_api_key);
|
||||
sv('s-tts-backend', s.tts_backend || 'openai');
|
||||
const defaultTtsParams = {temperature:0.1, top_p:0.8, seed:0};
|
||||
const byBackend = s.tts_extra_params_by_backend || {};
|
||||
@ -557,12 +562,12 @@ async function loadSettings() {
|
||||
sv('s-tts-extra-kokoro', JSON.stringify(byBackend.kokoro || {}, null, 2));
|
||||
sv('s-tts-extra-vibevoice', JSON.stringify(byBackend.vibevoice || {}, null, 2));
|
||||
sv('s-voice-design-url', s.voice_design_url || 'http://host.docker.internal:8021');
|
||||
sv('s-vd-key', s.voice_design_api_key || '');
|
||||
svSecret('s-vd-key', s.voice_design_api_key);
|
||||
sv('s-voices-scan-dir', s.voices_scan_dir || '');
|
||||
sv('s-output-dir', s.output_dir || '');
|
||||
restoreEngineApiKeyInputs();
|
||||
const extKeyInp = $('s-external-api-key');
|
||||
if (extKeyInp) extKeyInp.value = s.external_api_key || '';
|
||||
if (extKeyInp) { extKeyInp.value = ''; extKeyInp.placeholder = s.external_api_key === '__configured__' ? 'Configured — regenerate to replace' : ''; }
|
||||
const extKeyReq = $('s-external-api-key-required');
|
||||
if (extKeyReq) extKeyReq.checked = !!s.external_api_key_required;
|
||||
if (extKeyReq && !extKeyReq.dataset.wired) {
|
||||
|
||||
@ -249,8 +249,7 @@ async function _statusOpenLlmFlyup(anchor) {
|
||||
let models = [];
|
||||
try {
|
||||
const fetchUrl = '/api/conversation/llm-models?url=' + encodeURIComponent(url);
|
||||
const headers = apiKey ? { 'X-Provider-API-Key': apiKey } : {};
|
||||
const data = await fetch(fetchUrl, { headers }).then(r => r.json());
|
||||
const data = await fetch(fetchUrl).then(r => r.json());
|
||||
models = Array.isArray(data.models) ? data.models : [];
|
||||
} catch (_) {}
|
||||
if (!_statusFlyup || _statusFlyup.dataset.forKind !== 'llm') return; // closed/switched while awaiting
|
||||
@ -319,8 +318,7 @@ async function refreshStatusBarEngines({ force = false } = {}) {
|
||||
STATUS_LLM_CACHE.pending = true;
|
||||
try {
|
||||
const fetchUrl = '/api/conversation/llm-models?url=' + encodeURIComponent(url);
|
||||
const headers = apiKey ? { 'X-Provider-API-Key': apiKey } : {};
|
||||
const data = await fetch(fetchUrl, { headers }).then(r => r.json());
|
||||
const data = await fetch(fetchUrl).then(r => r.json());
|
||||
const models = Array.isArray(data.models) ? data.models : [];
|
||||
STATUS_LLM_CACHE.url = url;
|
||||
STATUS_LLM_CACHE.model = model;
|
||||
|
||||
@ -11,6 +11,7 @@ os.environ.setdefault("CONFIG_DIR", tempfile.mkdtemp(prefix="ttvc-test-config-")
|
||||
from fastapi import HTTPException
|
||||
|
||||
from core.validation import _validate_character_record, _validate_http_url
|
||||
from core.config import _load_settings, _save_settings
|
||||
from server import RequestSizeLimitMiddleware
|
||||
|
||||
|
||||
@ -48,6 +49,35 @@ class SecurityValidationTests(unittest.TestCase):
|
||||
self.assertEqual(sent[0]["status"], 413)
|
||||
self.assertNotIn({"type": "downstream"}, sent)
|
||||
|
||||
def test_settings_never_returns_provider_credentials(self):
|
||||
from fastapi.testclient import TestClient
|
||||
from server import app
|
||||
settings = _load_settings()
|
||||
settings.update({
|
||||
"llm_api_key": "provider-secret",
|
||||
"engine_api_keys": {"local": "engine-secret"},
|
||||
})
|
||||
_save_settings(settings)
|
||||
with TestClient(app) as client:
|
||||
response = client.get("/api/settings", headers={"origin": "http://testserver"})
|
||||
self.assertEqual(response.status_code, 200)
|
||||
payload = response.json()
|
||||
self.assertEqual(payload.get("llm_api_key"), "__configured__")
|
||||
self.assertEqual(payload.get("engine_api_keys", {}).get("local"), "__configured__")
|
||||
self.assertNotIn("provider-secret", response.text)
|
||||
self.assertNotIn("engine-secret", response.text)
|
||||
|
||||
def test_empty_masked_secret_does_not_overwrite_saved_value(self):
|
||||
from fastapi.testclient import TestClient
|
||||
from server import app
|
||||
settings = _load_settings()
|
||||
settings["llm_api_key"] = "keep-this-secret"
|
||||
_save_settings(settings)
|
||||
with TestClient(app) as client:
|
||||
response = client.post("/api/settings", json={"llm_api_key": ""}, headers={"origin": "http://testserver"})
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(_load_settings().get("llm_api_key"), "keep-this-secret")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user