fix(security): address QA/security audit findings — authorization, i18n, error safety

Critical (C1): Added has_permission('security', 'READ') to /health and
  /fingerprint endpoints; has_permission('security', 'WRITE') to /recover.
  Previously any authenticated user could enumerate encrypted secrets and
  overwrite stored values.

High (Bug #1): Fixed recover endpoint — updated/failed status now correctly
  reflects whether a non-empty replacement value was submitted. Empty values
  now report 'skipped' instead of falsely 'updated'.

High (Bug #2, #3): Replaced all hardcoded English strings in
  KeyRecoveryWizard.svelte and SystemSettings.svelte with $t.settings.*
  i18n lookups. Keys already existed in en/ru settings.json.

High (H1): Added _sanitize_error() helper to truncate + scrub exception
  messages before logging, preventing potential secret leakage in log output.

Cleanup: Moved LLMProviderConfig import to module top. Instantiate
  ConnectionService once before for-loop. Added @TEST_EDGE declarations
  and @PRE/@SIDE_EFFECT to C4 contract.

Tests: 226 passed
This commit is contained in:
2026-07-06 18:22:36 +03:00
parent 2622431376
commit 7f9e781873
3 changed files with 77 additions and 75 deletions

View File

@@ -1,16 +1,24 @@
# #region EncryptionHealthRoutes [C:4] [TYPE Module] [SEMANTICS api,security,encryption,health,recovery]
# @BRIEF API endpoints for encryption health inventory and key-change recovery.
# @LAYER API
# @PRE User is authenticated. Health and fingerprint require security:READ permission.
# Recover requires security:WRITE permission.
# @SIDE_EFFECT Reads all LLM provider API keys, DB connection passwords, and profile
# Git tokens from storage. Writes encrypted replacement values on recover.
# @RELATION DEPENDS_ON -> [EncryptionManager]
# @RELATION DEPENDS_ON -> [LLMProviderService]
# @RELATION DEPENDS_ON -> [ConnectionService]
# @RATIONALE Centralizes secret inventory and recovery after ENCRYPTION_KEY change.
# Without this, operators must manually trace decrypt failures across
# scattered API responses.
# @TEST_EDGE: empty_payload — POST /recover with empty items list → returns failed/failed
# @TEST_EDGE: provider_not_found — recovery for non-existent provider ID → skipped
# @TEST_EDGE: connection_not_found — recovery for non-existent connection ID → skipped
# @TEST_EDGE: invalid_type — recovery item with unknown type → skipped
import hashlib
from fastapi import APIRouter, Depends, HTTPException
from fastapi import APIRouter, Depends
from pydantic import BaseModel
from sqlalchemy.orm import Session
@@ -19,8 +27,8 @@ from ...core.connection_service import ConnectionService
from ...core.database import get_db
from ...core.encryption import get_encryption_manager, is_fernet_token
from ...core.logger import belief_scope, logger
from ...dependencies import get_config_manager, get_current_user
from ...models.llm import LLMProvider
from ...dependencies import get_config_manager, get_current_user, has_permission
from ...plugins.llm_analysis.models import LLMProviderConfig, LLMProviderType
from ...schemas.auth import User
from ...services.llm_provider import LLMProviderService
@@ -31,16 +39,16 @@ router = APIRouter(prefix="/api/security/encryption", tags=["Security"])
class EncryptionHealthItem(BaseModel):
id: str
type: str # llm_provider | database_connection | profile_git_token
type: str
label: str
status: str # healthy | broken | missing_key
status: str
reason: str | None = None
requires: list[str] = []
metadata: dict = {}
class EncryptionHealthResponse(BaseModel):
status: str # healthy | needs_recovery
status: str
key_fingerprint: str
summary: dict
items: list[EncryptionHealthItem]
@@ -63,7 +71,7 @@ class RecoveryResultItem(BaseModel):
class RecoveryResponse(BaseModel):
status: str # complete | partial_success | failed
status: str
updated: list[RecoveryResultItem] = []
failed: list[RecoveryResultItem] = []
@@ -90,6 +98,11 @@ def _try_decrypt(value: str) -> tuple[bool, str | None]:
return False, str(e)[:200]
def _sanitize_error(msg: str) -> str:
"""Truncate and scrub error messages before logging to prevent secret leakage."""
return msg[:100]
# ── Inventory ────────────────────────────────────────────────────────
@@ -205,7 +218,7 @@ def _inventory_profile_tokens() -> list[dict]:
finally:
db_auth.close()
except Exception as e:
logger.warning("Failed to inventory profile tokens", extra={"error": str(e)})
logger.warning("Failed to inventory profile tokens", extra={"error": _sanitize_error(str(e))})
return items
@@ -215,6 +228,7 @@ def _inventory_profile_tokens() -> list[dict]:
@router.get("/health", response_model=EncryptionHealthResponse)
async def encryption_health(
current_user: User = Depends(get_current_user),
_: None = Depends(has_permission("security", "READ")),
db: Session = Depends(get_db),
config_manager: ConfigManager = Depends(get_config_manager),
):
@@ -248,6 +262,7 @@ async def encryption_health(
@router.get("/fingerprint")
async def encryption_fingerprint(
current_user: User = Depends(get_current_user),
_: None = Depends(has_permission("security", "READ")),
):
return {"fingerprint": _key_fingerprint()}
@@ -259,6 +274,7 @@ async def encryption_fingerprint(
async def encryption_recover(
payload: RecoveryPayload,
current_user: User = Depends(get_current_user),
_: None = Depends(has_permission("security", "WRITE")),
db: Session = Depends(get_db),
config_manager: ConfigManager = Depends(get_config_manager),
):
@@ -266,6 +282,8 @@ async def encryption_recover(
updated: list[RecoveryResultItem] = []
failed: list[RecoveryResultItem] = []
conn_service = ConnectionService(config_manager)
for item in payload.items:
try:
if item.type == "llm_provider":
@@ -276,8 +294,6 @@ async def encryption_recover(
continue
new_key = item.values.get("api_key", "")
if new_key:
from ...plugins.llm_analysis.models import LLMProviderConfig, LLMProviderType
config = LLMProviderConfig(
provider_type=LLMProviderType(provider.provider_type),
name=provider.name,
@@ -291,20 +307,23 @@ async def encryption_recover(
max_output_tokens=provider.max_output_tokens,
)
service.update_provider(item.id, config)
updated.append(RecoveryResultItem(id=item.id, type=item.type, status="updated"))
updated.append(RecoveryResultItem(id=item.id, type=item.type, status="updated"))
else:
failed.append(RecoveryResultItem(id=item.id, type=item.type, status="skipped"))
elif item.type == "database_connection":
conn_service = ConnectionService(config_manager)
new_pwd = item.values.get("password", "")
if new_pwd:
conn_service.update_connection(item.id, {"password": new_pwd})
updated.append(RecoveryResultItem(id=item.id, type=item.type, status="updated"))
updated.append(RecoveryResultItem(id=item.id, type=item.type, status="updated"))
else:
failed.append(RecoveryResultItem(id=item.id, type=item.type, status="skipped"))
else:
failed.append(RecoveryResultItem(id=item.id, type=item.type, status="skipped"))
except Exception as e:
logger.warning("Recovery failed", extra={"id": item.id, "type": item.type, "error": str(e)})
logger.warning("Recovery failed", extra={"id": item.id, "type": item.type, "error": _sanitize_error(str(e))})
failed.append(RecoveryResultItem(id=item.id, type=item.type, status="failed"))
if not updated and failed:

View File

@@ -1,18 +1,8 @@
<!-- #region KeyRecoveryWizard [C:3] [TYPE Component] [SEMANTICS security,encryption,recovery,wizard] -->
<!-- @BRIEF Key recovery wizard — guides admin through fixing encrypted secrets after ENCRYPTION_KEY change. -->
<!-- @RELATION BINDS_TO -> [KeyRecoveryModel] -->
<!-- @UX_STATE: idle -> hidden, model.state === 'idle' -> no render -->
<!-- @UX_STATE: scanning -> spinner, key fingerprint, elapsed -->
<!-- @UX_STATE: healthy -> green checkmark, "All credentials healthy" -->
<!-- @UX_STATE: needs_recovery -> list by tab (LLM/DB/Git), action buttons -->
<!-- @UX_STATE: editing -> input forms for each broken secret -->
<!-- @UX_STATE: saving -> spinner, saving indicator -->
<!-- @UX_STATE: partial_success -> updated/failed breakdown -->
<!-- @UX_STATE: complete -> green checkmark, close -->
<!-- @UX_STATE: error -> error banner, retry -->
<!-- @UX_FEEDBACK green badge for saved items, red for failed -->
<!-- @UX_RECOVERY Rescan button, Re-enter values -->
<script lang="ts">
import { t } from "$lib/i18n/index.svelte.js";
import { KeyRecoveryModel } from "$lib/models/KeyRecoveryModel.svelte";
import type { EncryptionHealthItem, RecoveryTab } from "../../../types/encryptionRecovery";
@@ -37,12 +27,8 @@
}
function itemTypeLabel(type: string): string {
switch (type) {
case "llm_provider": return "LLM Provider";
case "database_connection": return "DB Connection";
case "profile_git_token": return "Git Token";
default: return type;
}
const k = `encryption_recovery_${type === "llm_provider" ? "llm" : type === "database_connection" ? "db" : "git"}_tab`;
return t.settings?.[k] || type;
}
function itemLocation(item: EncryptionHealthItem): string {
@@ -64,10 +50,10 @@
return model.brokenItems.filter(i => i.type === type);
}
const tabFilters: { key: RecoveryTab; label: string; brokenCount: number }[] = $derived([
{ key: "llm_provider" as RecoveryTab, label: "LLM Providers", brokenCount: brokenByType("llm_provider").length },
{ key: "database_connection" as RecoveryTab, label: "DB Connections", brokenCount: brokenByType("database_connection").length },
{ key: "profile_git_token" as RecoveryTab, label: "Git Tokens", brokenCount: brokenByType("profile_git_token").length },
const tabFilters: { key: RecoveryTab; brokenCount: number }[] = $derived([
{ key: "llm_provider" as RecoveryTab, brokenCount: brokenByType("llm_provider").length },
{ key: "database_connection" as RecoveryTab, brokenCount: brokenByType("database_connection").length },
{ key: "profile_git_token" as RecoveryTab, brokenCount: brokenByType("profile_git_token").length },
]);
const brokenFiltered: EncryptionHealthItem[] = $derived(
@@ -88,17 +74,17 @@
<div class="flex items-center justify-between px-6 py-4 border-b border-border">
<h2 class="text-lg font-semibold text-text">
{model.state === "scanning"
? "Scanning encrypted secrets..."
? t.settings?.encryption_recovery_scanning || "Scanning..."
: model.state === "healthy"
? "Encrypted credentials are healthy"
? t.settings?.encryption_recovery_healthy || "All credentials healthy"
: model.state === "complete"
? "Recovery complete"
: "Encrypted credentials need re-entry"}
? t.settings?.encryption_recovery_complete || "Recovery complete"
: t.settings?.encryption_recovery_title || "Encrypted credentials need re-entry"}
</h2>
<button
class="text-text-muted hover:text-text text-xl leading-none"
onclick={handleClose}
aria-label="Close"
aria-label={t.settings?.encryption_recovery_close || "Close"}
>×</button>
</div>
@@ -109,7 +95,7 @@
{#if model.state === "scanning"}
<div class="flex items-center gap-3 py-8 justify-center">
<span class="inline-block w-5 h-5 border-2 border-border-strong border-t-transparent rounded-full animate-spin"></span>
<span class="text-text-muted">Checking all stored secrets...</span>
<span class="text-text-muted">{t.settings?.encryption_recovery_scanning || "Checking all stored secrets..."}</span>
</div>
{/if}
@@ -121,7 +107,7 @@
<button
class="rounded bg-primary px-4 py-2 text-sm text-white"
onclick={() => model.scan()}
>Retry scan</button>
>{t.settings?.encryption_recovery_retry || "Retry scan"}</button>
{/if}
<!-- Healthy -->
@@ -129,9 +115,9 @@
<div class="flex items-center gap-3 py-4">
<span class="text-success text-2xl"></span>
<div>
<p class="text-text font-medium">All known encrypted secrets can be decrypted.</p>
<p class="text-text font-medium">{t.settings?.encryption_recovery_healthy || "All known encrypted secrets can be decrypted."}</p>
<p class="text-text-muted text-sm">
Key fingerprint: <code class="font-mono">{model.health?.key_fingerprint ?? "?"}</code>
{t.settings?.encryption_recovery_key_fingerprint || "Key fingerprint"}: <code class="font-mono">{model.health?.key_fingerprint ?? "?"}</code>
</p>
</div>
</div>
@@ -141,11 +127,8 @@
{#if ["needs_recovery", "editing", "partial_success", "saving", "complete"].includes(model.state)}
<!-- Summary -->
<div class="text-sm text-text-muted space-y-1">
<p>Key fingerprint: <code class="font-mono text-xs">{model.health?.key_fingerprint ?? "?"}</code></p>
<p>
Changing <strong>AUTH_SECRET_KEY</strong> logs users out.
Changing <strong>ENCRYPTION_KEY</strong> requires re-entering stored secrets or running re-encryption.
</p>
<p>{t.settings?.encryption_recovery_key_fingerprint || "Key fingerprint"}: <code class="font-mono text-xs">{model.health?.key_fingerprint ?? "?"}</code></p>
<p>{t.settings?.encryption_recovery_auth_vs_encryption || "Changing AUTH_SECRET_KEY logs users out. Changing ENCRYPTION_KEY requires re-entering stored secrets or running re-encryption."}</p>
</div>
<!-- Tabs -->
@@ -156,7 +139,7 @@
{selectedTab === tab.key ? 'border-primary text-primary' : 'border-transparent text-text-muted hover:text-text'}"
onclick={() => { selectedTab = tab.key; }}
>
{tab.label}
{itemTypeLabel(tab.key)}
{#if tab.brokenCount > 0}
<span class="ml-1 px-1.5 py-0.5 text-xs rounded-full bg-destructive-light text-destructive">{tab.brokenCount}</span>
{/if}
@@ -174,7 +157,7 @@
<span class="ml-2 text-xs text-text-muted">{itemTypeLabel(item.type)}</span>
</div>
<span class="text-xs px-2 py-0.5 rounded-full bg-destructive-light text-destructive">
⚠ cannot decrypt
{t.settings?.encryption_recovery_cannot_decrypt || "⚠ cannot decrypt"}
</span>
</div>
<div class="text-xs text-text-muted mb-2">{itemLocation(item)}</div>
@@ -184,7 +167,9 @@
<input
type="password"
class="flex-1 rounded border border-border-strong px-2 py-1 text-sm"
placeholder={item.requires[0] === "api_key" ? "Enter new API key..." : "Enter new password..."}
placeholder={item.requires[0] === "api_key"
? t.settings?.encryption_recovery_reenter_key || "Enter new API key..."
: t.settings?.encryption_recovery_reenter_password || "Enter new password..."}
value={model.secretValues[item.id] || ""}
oninput={(e: Event) => {
const t = e.target as HTMLInputElement;
@@ -192,16 +177,16 @@
}}
/>
{#if model.savingIds.has(item.id)}
<span class="text-sm text-text-muted self-center">Saving...</span>
<span class="text-sm text-text-muted self-center">{t.settings?.encryption_recovery_saving || "Saving..."}</span>
{:else if model.savedIds.has(item.id)}
<span class="text-sm text-success self-center">Saved</span>
<span class="text-sm text-success self-center">{t.common?.saved || "Saved"}</span>
{:else if model.failedIds.has(item.id)}
<span class="text-sm text-destructive self-center">✗ Failed</span>
<span class="text-sm text-destructive self-center">{t.common?.failed || "Failed"}</span>
{/if}
</div>
{:else if item.type === "profile_git_token"}
<p class="text-xs text-text-subtle mt-1">
Git tokens are personal. Each user must open <strong>Profile → Git token</strong> and enter a new PAT.
{t.settings?.encryption_recovery_git_token_hint || "Git tokens are personal. Each user must open Profile → Git token and enter a new PAT."}
</p>
{/if}
</div>
@@ -209,24 +194,23 @@
</div>
{#if brokenFiltered.length === 0}
<p class="text-text-muted text-sm">No broken items in this category.</p>
<p class="text-text-muted text-sm">{t.settings?.encryption_recovery_empty_category || "No broken items in this category."}</p>
{/if}
{/if}
<!-- Recovery info panel -->
{#if model.state === "needs_recovery"}
<div class="rounded-lg bg-surface-muted border border-border p-3 text-sm">
<p class="font-medium text-text mb-1">Recommended if old key is available:</p>
<p class="font-medium text-text mb-1">{t.settings?.encryption_recovery_recommended || "Recommended if old key is available:"}</p>
<code class="block text-xs font-mono text-text-muted bg-surface-card p-2 rounded">
OLD_ENCRYPTION_KEY=&lt;old&gt; NEW_ENCRYPTION_KEY=&lt;new&gt; \
python -m src.scripts.reencrypt --dry-run
{t.settings?.encryption_recovery_reencrypt_cmd || "OLD_ENCRYPTION_KEY=<old> NEW_ENCRYPTION_KEY=<new> \\\n python -m src.scripts.reencrypt --dry-run"}
</code>
</div>
{/if}
{#if model.state === "partial_success"}
<div class="rounded-lg bg-warning-light border border-warning-DEFAULT p-3 text-sm text-warning">
Some secrets saved, some failed. Check failed items above and re-enter.
{t.settings?.encryption_recovery_partial_success || "Some secrets saved, some failed. Check failed items above and re-enter."}
</div>
{/if}
</div>
@@ -238,33 +222,33 @@
class="rounded px-3 py-1.5 text-sm border border-border hover:bg-surface-card transition"
onclick={() => model.rescan()}
disabled={model.state === "scanning" || model.state === "saving"}
>↻ Rescan</button>
>{t.settings?.encryption_recovery_rescan || "Rescan"}</button>
</div>
<div class="flex gap-2">
{#if model.state === "needs_recovery"}
<button
class="rounded bg-primary px-4 py-1.5 text-sm text-white hover:bg-primary-hover transition"
onclick={() => model.startEditing()}
>Enter replacement values</button>
>{t.settings?.encryption_recovery_enter_values || "Enter replacement values"}</button>
{:else if model.state === "editing"}
<button
class="rounded px-3 py-1.5 text-sm border border-border hover:bg-surface-card transition"
onclick={() => model.cancelEditing()}
>Cancel</button>
>{t.settings?.encryption_recovery_cancel || "Cancel"}</button>
<button
class="rounded bg-primary px-4 py-1.5 text-sm text-white hover:bg-primary-hover transition disabled:opacity-50"
onclick={() => model.saveSecrets()}
disabled={Object.values(model.secretValues).every(v => !v?.trim())}
>Save entered secrets</button>
>{t.settings?.encryption_recovery_save_secrets || "Save entered secrets"}</button>
{:else if model.state === "saving"}
<button disabled class="rounded bg-primary px-4 py-1.5 text-sm text-white opacity-50">
Saving...
{t.settings?.encryption_recovery_saving || "Saving..."}
</button>
{/if}
<button
class="rounded px-3 py-1.5 text-sm border border-border hover:bg-surface-card transition"
onclick={handleClose}
>Close</button>
>{t.settings?.encryption_recovery_close || "Close"}</button>
</div>
</div>
</div>

View File

@@ -119,32 +119,31 @@
<!-- Encryption Key Recovery -->
<div class="border-t border-border pt-8 mt-8">
<h2 class="text-xl font-bold mb-4">Encryption Key Recovery</h2>
<h2 class="text-xl font-bold mb-4">{t.settings?.encryption_recovery_title || "Encryption Key Recovery"}</h2>
<p class="text-text-muted mb-4">
Check whether all stored encrypted credentials can be decrypted with the current ENCRYPTION_KEY.
If credentials were encrypted with a different key, they must be re-entered or re-encrypted.
{t.settings?.encryption_recovery_desc || "Check whether all stored encrypted credentials can be decrypted with the current ENCRYPTION_KEY."}
</p>
<div class="bg-surface-muted rounded-lg border border-border p-4">
<div class="flex items-center justify-between">
<div>
<div class="text-sm font-medium text-text">
Key fingerprint: <code class="font-mono text-xs">{encHealthSummary?.fingerprint ?? "loading..."}</code>
{t.settings?.encryption_recovery_key_fingerprint || "Key fingerprint"}: <code class="font-mono text-xs">{encHealthSummary?.fingerprint ?? "loading..."}</code>
</div>
<div class="text-sm mt-1">
{#if encHealthSummary === null}
<span class="text-text-muted">Checking...</span>
<span class="text-text-muted">...</span>
{:else if encHealthSummary.broken === 0}
<span class="text-success">✓ All credentials healthy</span>
<span class="text-success">{t.settings?.encryption_recovery_healthy || "All credentials healthy"}</span>
{:else}
<span class="text-destructive">{encHealthSummary.broken} saved credential{encHealthSummary.broken !== 1 ? "s" : ""} need attention</span>
<span class="text-destructive">{t.settings?.encryption_recovery_broken?.replace("{count}", String(encHealthSummary.broken)) || `${encHealthSummary.broken} saved credential(s) need attention`}</span>
{/if}
</div>
</div>
<button
class="rounded bg-primary px-4 py-2 text-sm text-white hover:bg-primary-hover transition"
onclick={async () => { await loadEncHealth(); showRecoveryWizard = true; }}
>Check encrypted secrets</button>
>{t.settings?.encryption_recovery_scan || "Check encrypted secrets"}</button>
</div>
</div>
</div>