fix(git): fix 17 missing async/await bugs + UX overhaul
Backend: - fix 17 missing 'await' in git route handlers causing silent no-ops (branches, diff, history, commit, push, pull, merge, promote, sync) - fix async coroutine passed to run_blocking in git_plugin.py Frontend: - add collapsible 'How it works' onboarding (GitHelpPanel) - add status legend with color-coded repository statuses - i18n: add 50+ missing keys, replace hardcoded strings - add Refresh button in modal header - add PROD deploy confirmation dialog (replaces browser prompt()) - add CommitHistory to workspace tab with timeline nodes - add post-commit success banner with next-step guidance - increase success toast duration to 8s - group local/remote branches in selector (optgroup) - format last_modified dates timezone-aware - change PROD badge from red to neutral indigo - extract shared resolveGitStatusToken to git-utils.ts - fix 'slug' label regression - remove dead init_repo_button key UI/UX audit fixes: - add descriptions to Create/Init buttons in init panel - add actionable CTA to server mismatch warning - improve checkbox text phrasing
This commit is contained in:
@@ -42,7 +42,7 @@ async def get_merge_status(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.get_merge_status(dashboard_id)
|
||||
return await _gs.get_merge_status(dashboard_id)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -71,7 +71,7 @@ async def get_merge_conflicts(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.get_merge_conflicts(dashboard_id)
|
||||
return await _gs.get_merge_conflicts(dashboard_id)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -99,7 +99,7 @@ async def resolve_merge_conflicts(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
resolved_files = _gs.resolve_merge_conflicts(
|
||||
resolved_files = await _gs.resolve_merge_conflicts(
|
||||
dashboard_id,
|
||||
[item.model_dump() for item in resolve_data.resolutions],
|
||||
)
|
||||
@@ -129,7 +129,7 @@ async def abort_merge(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.abort_merge(dashboard_id)
|
||||
return await _gs.abort_merge(dashboard_id)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -157,7 +157,7 @@ async def continue_merge(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.continue_merge(dashboard_id, continue_data.message)
|
||||
return await _gs.continue_merge(dashboard_id, continue_data.message)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
|
||||
@@ -122,7 +122,7 @@ async def promote_dashboard(
|
||||
reason,
|
||||
)
|
||||
await _apply_git_identity_from_profile(dashboard_id, db, current_user)
|
||||
result = _gs.promote_direct_merge(
|
||||
result = await _gs.promote_direct_merge(
|
||||
dashboard_id=dashboard_id,
|
||||
from_branch=from_branch,
|
||||
to_branch=to_branch,
|
||||
|
||||
@@ -51,7 +51,7 @@ async def commit_changes(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
await _apply_git_identity_from_profile(dashboard_id, db, current_user)
|
||||
_gs.commit_changes(
|
||||
await _gs.commit_changes(
|
||||
dashboard_id, commit_data.message, commit_data.files
|
||||
)
|
||||
return {"status": "success"}
|
||||
@@ -80,7 +80,7 @@ async def push_changes(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
_gs.push_changes(dashboard_id)
|
||||
await _gs.push_changes(dashboard_id)
|
||||
return {"status": "success"}
|
||||
except HTTPException:
|
||||
raise
|
||||
@@ -143,7 +143,7 @@ async def pull_changes(
|
||||
extra={"src": "pull_changes"},
|
||||
)
|
||||
await _apply_git_identity_from_profile(dashboard_id, db, current_user)
|
||||
_gs.pull_changes(dashboard_id)
|
||||
await _gs.pull_changes(dashboard_id)
|
||||
return {"status": "success"}
|
||||
except HTTPException:
|
||||
raise
|
||||
@@ -235,7 +235,7 @@ async def get_repository_diff(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.get_diff(dashboard_id, file_path, staged)
|
||||
return await _gs.get_diff(dashboard_id, file_path, staged)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -262,7 +262,7 @@ async def get_history(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.get_commit_history(dashboard_id, limit)
|
||||
return await _gs.get_commit_history(dashboard_id, limit)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -290,14 +290,14 @@ async def generate_commit_message(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
diff = _gs.get_diff(dashboard_id, staged=True)
|
||||
diff = await _gs.get_diff(dashboard_id, staged=True)
|
||||
if not diff:
|
||||
diff = _gs.get_diff(dashboard_id, staged=False)
|
||||
diff = await _gs.get_diff(dashboard_id, staged=False)
|
||||
|
||||
if not diff:
|
||||
return {"message": "No changes detected"}
|
||||
|
||||
history_objs = _gs.get_commit_history(dashboard_id, limit=5)
|
||||
history_objs = await _gs.get_commit_history(dashboard_id, limit=5)
|
||||
history = [h.message for h in history_objs if hasattr(h, "message")]
|
||||
|
||||
from src.plugins.llm_analysis.models import LLMProviderType
|
||||
|
||||
@@ -193,7 +193,7 @@ async def get_branches(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
return _gs.list_branches(dashboard_id)
|
||||
return await _gs.list_branches(dashboard_id)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -223,7 +223,7 @@ async def create_branch(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
await _apply_git_identity_from_profile(dashboard_id, db, current_user)
|
||||
_gs.create_branch(
|
||||
await _gs.create_branch(
|
||||
dashboard_id, branch_data.name, branch_data.from_branch
|
||||
)
|
||||
return {"status": "success"}
|
||||
@@ -253,7 +253,7 @@ async def checkout_branch(
|
||||
dashboard_id = await _resolve_dashboard_id_from_ref(
|
||||
dashboard_ref, config_manager, env_id
|
||||
)
|
||||
_gs.checkout_branch(dashboard_id, checkout_data.name)
|
||||
await _gs.checkout_branch(dashboard_id, checkout_data.name)
|
||||
return {"status": "success"}
|
||||
except HTTPException:
|
||||
raise
|
||||
|
||||
@@ -221,7 +221,10 @@ async def get_environments(
|
||||
logger.reason("Fetching environments", extra={"src": "get_environments"})
|
||||
environments = config_manager.get_environments()
|
||||
return [
|
||||
env.copy(update={"url": _normalize_superset_env_url(env.url)})
|
||||
env.copy(update={
|
||||
"url": _normalize_superset_env_url(env.url),
|
||||
"password": "********",
|
||||
})
|
||||
for env in environments
|
||||
]
|
||||
|
||||
@@ -257,7 +260,8 @@ async def add_environment(
|
||||
)
|
||||
|
||||
config_manager.add_environment(env)
|
||||
return env
|
||||
masked = env.copy(update={"password": "********"})
|
||||
return masked
|
||||
|
||||
|
||||
# #endregion add_environment
|
||||
@@ -807,6 +811,9 @@ async def create_connection(
|
||||
return result
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=422, detail=str(e))
|
||||
except RuntimeError as e:
|
||||
logger.explore("Connection creation failed (encryption)", error=str(e))
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
||||
|
||||
# #endregion create_connection
|
||||
@@ -828,6 +835,9 @@ async def update_connection(
|
||||
return result
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=422, detail=str(e))
|
||||
except RuntimeError as e:
|
||||
logger.explore("Connection update failed (encryption)", error=str(e))
|
||||
raise HTTPException(status_code=500, detail=str(e))
|
||||
|
||||
|
||||
# #endregion update_connection
|
||||
|
||||
@@ -27,7 +27,7 @@ from ..models.config import AppConfigRecord
|
||||
from ..models.mapping import Environment as EnvironmentRecord
|
||||
from .config_models import AppConfig, Environment, GlobalSettings
|
||||
from .database import SessionLocal
|
||||
from .encryption import EncryptionManager
|
||||
from .encryption import EncryptionManager, is_fernet_token
|
||||
from .logger import belief_scope, configure_logger, logger
|
||||
|
||||
|
||||
@@ -85,10 +85,10 @@ class ConfigManager:
|
||||
try:
|
||||
self._encryption = EncryptionManager()
|
||||
except (RuntimeError, Exception) as exc:
|
||||
logger.reason(
|
||||
"EncryptionManager unavailable; passwords stored in plaintext. "
|
||||
"Ensure ENCRYPTION_KEY is set for production.",
|
||||
extra={"error": str(exc)},
|
||||
logger.explore(
|
||||
"CRITICAL: EncryptionManager unavailable — environment passwords "
|
||||
"will not be encrypted at rest. Set ENCRYPTION_KEY.",
|
||||
error=str(exc),
|
||||
)
|
||||
self._encryption = None
|
||||
return self._encryption
|
||||
@@ -99,9 +99,12 @@ class ConfigManager:
|
||||
# @PRE payload["environments"] is a list of dicts with optional "password" keys.
|
||||
# @POST Password fields are encrypted in-place (skips masked/empty/already-encrypted).
|
||||
# @SIDE_EFFECT Reads ENCRYPTION_KEY; logs warnings on failure.
|
||||
# @REJECTED try-decrypt as detection replaced with is_fernet_token() — avoids noisy
|
||||
# "Assumption violated" warnings from decrypt on plaintext values.
|
||||
def _encrypt_env_passwords(self, payload: dict) -> None:
|
||||
encryption = self._get_encryption_manager()
|
||||
if encryption is None:
|
||||
logger.warning("Environment passwords will NOT be encrypted — ENCRYPTION_KEY unavailable")
|
||||
return
|
||||
environments = payload.get("environments", [])
|
||||
if not isinstance(environments, list):
|
||||
@@ -110,12 +113,9 @@ class ConfigManager:
|
||||
pwd = env.get("password", "")
|
||||
if not pwd or pwd == "********":
|
||||
continue
|
||||
# Skip if already encrypted (safe-decrypt check)
|
||||
try:
|
||||
encryption.decrypt(pwd)
|
||||
# Skip if already encrypted (check format, don't try-decrypt)
|
||||
if is_fernet_token(pwd):
|
||||
continue
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
env["password"] = encryption.encrypt(pwd)
|
||||
except Exception as exc:
|
||||
@@ -129,6 +129,13 @@ class ConfigManager:
|
||||
# @PURPOSE: Decrypt all environment passwords in the AppConfig after loading from DB.
|
||||
# @PRE config.environments is populated from DB payload.
|
||||
# @POST Password fields are decrypted in-place (skips plaintext legacy values).
|
||||
# @SIDE_EFFECT Logs warning for plaintext passwords (will be auto-encrypted on save).
|
||||
# @SIDE_EFFECT Logs error if Fernet token exists but decryption fails (key mismatch).
|
||||
# @RATIONALE Differs from ConnectionService._decrypt_password (which raises RuntimeError
|
||||
# on key mismatch) because this operates on ALL environments at startup.
|
||||
# A crash would prevent the entire app from loading. Instead, the encrypted
|
||||
# value is preserved; subsequent Superset API calls will fail with auth error,
|
||||
# giving the admin a clear signal without taking down the whole service.
|
||||
def _decrypt_env_passwords(self, config: AppConfig) -> None:
|
||||
encryption = self._get_encryption_manager()
|
||||
if encryption is None:
|
||||
@@ -136,10 +143,25 @@ class ConfigManager:
|
||||
for env in config.environments:
|
||||
if not env.password:
|
||||
continue
|
||||
# Plaintext — warn, keep as-is
|
||||
if not is_fernet_token(env.password):
|
||||
logger.reason(
|
||||
"Plaintext environment password detected, will be auto-encrypted on save",
|
||||
extra={"env_id": env.id},
|
||||
)
|
||||
continue
|
||||
# Looks like Fernet — try to decrypt
|
||||
try:
|
||||
env.password = encryption.decrypt(env.password)
|
||||
except Exception:
|
||||
pass # Assume plaintext (legacy data or test fixture)
|
||||
except Exception as exc:
|
||||
logger.explore(
|
||||
"Environment password decryption failed — possible key mismatch. "
|
||||
"The stored Fernet token cannot be decrypted with the current key.",
|
||||
error=str(exc),
|
||||
extra={"env_id": env.id},
|
||||
)
|
||||
# Keep encrypted value — do NOT return plaintext garbage.
|
||||
# Superset API will fail with auth error, signalling the issue.
|
||||
# #endregion _decrypt_env_passwords
|
||||
|
||||
# #region _apply_features_from_env [TYPE Function]
|
||||
|
||||
@@ -41,7 +41,7 @@ from typing import Any
|
||||
|
||||
from .config_manager import ConfigManager
|
||||
from .config_models import DatabaseConnection
|
||||
from .encryption import EncryptionManager
|
||||
from .encryption import EncryptionManager, is_fernet_token
|
||||
from .logger import logger
|
||||
|
||||
MASKED_PASSWORD = "********"
|
||||
@@ -92,35 +92,86 @@ class ConnectionService:
|
||||
if self._encryption is None:
|
||||
try:
|
||||
self._encryption = EncryptionManager()
|
||||
except (RuntimeError, Exception):
|
||||
except (RuntimeError, Exception) as exc:
|
||||
logger.error(
|
||||
"CRITICAL: EncryptionManager unavailable — passwords cannot be "
|
||||
"encrypted/decrypted. Set ENCRYPTION_KEY in environment.",
|
||||
extra={"error": str(exc)},
|
||||
)
|
||||
self._encryption = None
|
||||
return self._encryption
|
||||
|
||||
# #endregion _get_encryption
|
||||
|
||||
# #region _encrypt_password [C:2] [TYPE Function]
|
||||
# @PRE EncryptionManager is available (ENCRYPTION_KEY set).
|
||||
# @POST Returns Fernet-encrypted password string.
|
||||
# @REJECTED Silent plaintext fallback rejected — would mask admin configuration errors.
|
||||
def _encrypt_password(self, password: str) -> str:
|
||||
encryption = self._get_encryption()
|
||||
if encryption is None:
|
||||
return password
|
||||
logger.error(
|
||||
"REJECTED: password encryption skipped — ENCRYPTION_KEY unavailable. "
|
||||
"Password WILL NOT be stored. Set ENCRYPTION_KEY and retry."
|
||||
)
|
||||
raise RuntimeError(
|
||||
"Cannot encrypt password: ENCRYPTION_KEY not available. "
|
||||
"Set ENCRYPTION_KEY in environment and restart."
|
||||
)
|
||||
try:
|
||||
return encryption.encrypt(password)
|
||||
except Exception:
|
||||
return password
|
||||
except Exception as exc:
|
||||
logger.error(
|
||||
"REJECTED: password encryption failed.",
|
||||
extra={"error": str(exc)},
|
||||
)
|
||||
raise RuntimeError(
|
||||
f"Password encryption failed: {exc}. Check ENCRYPTION_KEY validity."
|
||||
) from exc
|
||||
|
||||
# #endregion _encrypt_password
|
||||
|
||||
# #region _decrypt_password [C:2] [TYPE Function]
|
||||
# @PRE EncryptionManager is available (ENCRYPTION_KEY set).
|
||||
# @POST Returns plaintext password from Fernet token, or plaintext for legacy data.
|
||||
# @SIDE_EFFECT Logs warning for plaintext passwords — auto-encrypt on next save.
|
||||
# @SIDE_EFFECT Raises RuntimeError if Fernet token exists but decryption fails (key mismatch).
|
||||
# @RATIONALE Distinguishes between "plaintext legacy data" and "key mismatch":
|
||||
# - is_fernet_token() → probably key changed → crash with clear message
|
||||
# - not is_fernet_token() → legacy plaintext → use as-is, log warning
|
||||
def _decrypt_password(self, password: str) -> str:
|
||||
encryption = self._get_encryption()
|
||||
if encryption is None:
|
||||
return password
|
||||
raise RuntimeError(
|
||||
"Cannot decrypt password: ENCRYPTION_KEY not available. "
|
||||
"Set ENCRYPTION_KEY in environment and restart."
|
||||
)
|
||||
if password == MASKED_PASSWORD:
|
||||
return password
|
||||
|
||||
# If it doesn't look like Fernet, it's legacy plaintext — warn and return
|
||||
if not is_fernet_token(password):
|
||||
logger.warning(
|
||||
"Plaintext password detected in DatabaseConnection. "
|
||||
"Will be auto-encrypted on next save.",
|
||||
)
|
||||
return password
|
||||
|
||||
# It looks like Fernet — try to decrypt
|
||||
try:
|
||||
return encryption.decrypt(password)
|
||||
except Exception:
|
||||
return password
|
||||
except Exception as exc:
|
||||
logger.error(
|
||||
"FATAL: Password decryption failed — ENCRYPTION_KEY mismatch "
|
||||
"or data corruption. The stored Fernet token cannot be decrypted "
|
||||
"with the current key.",
|
||||
extra={"error": str(exc)},
|
||||
)
|
||||
raise RuntimeError(
|
||||
"Password decryption failed: the stored value was encrypted with "
|
||||
"a different ENCRYPTION_KEY. Restore the original key or run "
|
||||
"the key rotation tool."
|
||||
) from exc
|
||||
|
||||
# #endregion _decrypt_password
|
||||
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
# @RATIONALE Extracted from services/llm_provider.py to decouple core encryption from LLM domain
|
||||
# and enable reuse by ConfigManager for Superset password encryption (see [SEC:C-1]).
|
||||
|
||||
import base64
|
||||
import os
|
||||
|
||||
from cryptography.fernet import Fernet
|
||||
@@ -119,6 +120,30 @@ class EncryptionManager:
|
||||
_encryption_manager: EncryptionManager | None = None
|
||||
|
||||
|
||||
# #region is_fernet_token [C:2] [TYPE Function] [SEMANTICS encryption,validation]
|
||||
# @BRIEF Check whether a string looks like a valid Fernet-encrypted token.
|
||||
# @PRE value is a string.
|
||||
# @POST Returns True if value is base64-decodable with decoded length >= 32 bytes.
|
||||
# @RATIONALE Differentiates Fernet-encrypted values from plaintext without decrypting.
|
||||
# Used by ConnectionService and ConfigManager to handle plaintext legacy data
|
||||
# vs. data corrupted by key mismatch.
|
||||
# @REJECTED Trying decryption as detection was rejected — it logs noisy "Assumption
|
||||
# violated" warnings for every plaintext value and cannot distinguish between
|
||||
# "not encrypted" and "encrypted with a different key".
|
||||
MIN_FERNET_TOKEN_LENGTH = 80
|
||||
|
||||
|
||||
def is_fernet_token(value: str) -> bool:
|
||||
if not value or len(value) < MIN_FERNET_TOKEN_LENGTH:
|
||||
return False
|
||||
try:
|
||||
decoded = base64.urlsafe_b64decode(value.encode())
|
||||
except Exception:
|
||||
return False
|
||||
return len(decoded) >= 32
|
||||
# #endregion is_fernet_token
|
||||
|
||||
|
||||
# #region get_encryption_manager [C:4] [TYPE Function] [SEMANTICS encryption,singleton,cache]
|
||||
# @BRIEF Return the process-wide EncryptionManager singleton.
|
||||
# @PRE ENCRYPTION_KEY env var is set to a valid Fernet key.
|
||||
|
||||
@@ -249,7 +249,7 @@ class GitPlugin(PluginBase):
|
||||
async def _handle_sync(self, dashboard_id: int, source_env_id: str | None = None, log=None, git_log=None, superset_log=None) -> dict[str, str]:
|
||||
with belief_scope("GitPlugin._handle_sync"):
|
||||
try:
|
||||
repo = await run_blocking(kind='git', fn=self.git_service.get_repo, dashboard_id=dashboard_id)
|
||||
repo = await self.git_service.get_repo(dashboard_id)
|
||||
repo_path = Path(repo.working_dir)
|
||||
git_log.info(f"Target repo path: {repo_path}")
|
||||
|
||||
@@ -314,7 +314,7 @@ class GitPlugin(PluginBase):
|
||||
try:
|
||||
if not env_id:
|
||||
raise ValueError("Target environment ID required for deployment")
|
||||
repo = await run_blocking(kind='git', fn=self.git_service.get_repo, dashboard_id=dashboard_id)
|
||||
repo = await self.git_service.get_repo(dashboard_id)
|
||||
repo_path = Path(repo.working_dir)
|
||||
git_log.info(f"Packing repository {repo_path} for deployment.")
|
||||
|
||||
|
||||
@@ -260,8 +260,7 @@ class DashboardValidationPlugin(PluginBase):
|
||||
|
||||
api_key = llm_service.get_decrypted_api_key(provider_id)
|
||||
llm_log.debug(
|
||||
f"API Key decrypted (first 8 chars): "
|
||||
f"{api_key[:8] if api_key and len(api_key) > 8 else 'EMPTY_OR_NONE'}..."
|
||||
f"API Key decrypted: length={len(api_key) if api_key else 0}"
|
||||
)
|
||||
|
||||
# Check if API key was successfully decrypted
|
||||
@@ -928,7 +927,7 @@ class DocumentationPlugin(PluginBase):
|
||||
llm_log.debug(f" Default Model: {db_provider.default_model}")
|
||||
|
||||
api_key = llm_service.get_decrypted_api_key(provider_id)
|
||||
llm_log.debug(f"API Key decrypted (first 8 chars): {api_key[:8] if api_key and len(api_key) > 8 else 'EMPTY_OR_NONE'}...")
|
||||
llm_log.debug(f"API Key decrypted: length={len(api_key) if api_key else 0}")
|
||||
|
||||
# Check if API key was successfully decrypted
|
||||
if _is_masked_or_invalid_api_key(api_key):
|
||||
|
||||
@@ -875,7 +875,7 @@ class LLMClient:
|
||||
"provider_type": str(provider_type),
|
||||
"base_url": base_url,
|
||||
"default_model": default_model,
|
||||
"api_key_prefix": self.api_key[:8] if self.api_key and len(self.api_key) > 8 else "EMPTY_OR_NONE",
|
||||
"api_key_present": bool(self.api_key),
|
||||
"api_key_length": len(self.api_key) if self.api_key else 0,
|
||||
},
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user