From b03d4fcc0883d3fc1c475da119027db887f9c13c Mon Sep 17 00:00:00 2001 From: likawa3b <42701727+likawa3b@users.noreply.github.com> Date: Sat, 27 Jun 2026 14:57:30 -0400 Subject: [PATCH 1/2] Harden auth and secret handling --- .gitignore | 2 + backend_api_python/app/routes/auth.py | 4 +- backend_api_python/app/routes/settings.py | 4 +- backend_api_python/app/routes/strategy.py | 5 +- backend_api_python/app/services/strategy.py | 70 +++++++++++++++++++ backend_api_python/app/utils/agent_auth.py | 26 ++++++- .../tests/test_agent_security.py | 14 ++++ .../tests/test_auth_legacy_fallback.py | 32 +++++++++ .../tests/test_settings_secret_masking.py | 28 ++++++++ .../tests/test_strategy_secret_handling.py | 28 ++++++++ docker-compose.ghcr.yml | 2 +- docker-compose.yml | 2 +- 12 files changed, 209 insertions(+), 8 deletions(-) create mode 100644 backend_api_python/tests/test_auth_legacy_fallback.py create mode 100644 backend_api_python/tests/test_settings_secret_masking.py create mode 100644 backend_api_python/tests/test_strategy_secret_handling.py diff --git a/.gitignore b/.gitignore index 6dfe7e370..a42ba0186 100644 --- a/.gitignore +++ b/.gitignore @@ -22,7 +22,9 @@ .env .env.local .env.*.local +backend.env backend_api_python/.env +.codex-review/ # ======================== # IDE & OS diff --git a/backend_api_python/app/routes/auth.py b/backend_api_python/app/routes/auth.py index ebf6c2409..a0c54c653 100644 --- a/backend_api_python/app/routes/auth.py +++ b/backend_api_python/app/routes/auth.py @@ -253,8 +253,8 @@ def login(): except Exception as e: logger.warning(f"Multi-user auth failed, trying legacy: {e}") - # Fallback to legacy single-user mode - if not user: + # Legacy env-admin auth is only valid in explicit single-user mode. + if not user and _is_single_user_mode(): user = authenticate_legacy(username, password) if not user: diff --git a/backend_api_python/app/routes/settings.py b/backend_api_python/app/routes/settings.py index e29d49f23..f7809e78f 100644 --- a/backend_api_python/app/routes/settings.py +++ b/backend_api_python/app/routes/settings.py @@ -1923,9 +1923,11 @@ def get_settings_values(): for item in group['items']: key = item['key'] value = env_values.get(key, item.get('default', '')) - result[group_key][key] = value if item['type'] == 'password': + result[group_key][key] = '' result[group_key][f'{key}_configured'] = bool(value) + else: + result[group_key][key] = value return jsonify({ 'code': 1, diff --git a/backend_api_python/app/routes/strategy.py b/backend_api_python/app/routes/strategy.py index fdf722e5e..ad565ecd0 100644 --- a/backend_api_python/app/routes/strategy.py +++ b/backend_api_python/app/routes/strategy.py @@ -17,6 +17,7 @@ from app.utils.db import get_db_connection from app.utils.auth import login_required +from app.services.strategy import redact_strategy_row logger = get_logger(__name__) @@ -332,7 +333,7 @@ def list_strategies(): try: user_id = g.user_id items = get_strategy_service().list_strategies(user_id=user_id) - return jsonify({'code': 1, 'msg': 'success', 'data': {'strategies': items}}) + return jsonify({'code': 1, 'msg': 'success', 'data': {'strategies': [redact_strategy_row(item) for item in items]}}) except Exception as e: logger.error(f"list_strategies failed: {str(e)}") logger.error(traceback.format_exc()) @@ -350,7 +351,7 @@ def get_strategy_detail(): st = get_strategy_service().get_strategy(strategy_id, user_id=user_id) if not st: return jsonify({'code': 0, 'msg': 'Strategy not found', 'data': None}), 404 - return jsonify({'code': 1, 'msg': 'success', 'data': st}) + return jsonify({'code': 1, 'msg': 'success', 'data': redact_strategy_row(st)}) except Exception as e: logger.error(f"get_strategy_detail failed: {str(e)}") logger.error(traceback.format_exc()) diff --git a/backend_api_python/app/services/strategy.py b/backend_api_python/app/services/strategy.py index 0f09e1274..819a8626f 100644 --- a/backend_api_python/app/services/strategy.py +++ b/backend_api_python/app/services/strategy.py @@ -24,6 +24,74 @@ def get_strategy_service() -> "StrategyService": return _strategy_service_singleton +_STRATEGY_SECRET_KEYS = { + "api_key", + "apikey", + "secret_key", + "secretkey", + "secret", + "passphrase", + "password", + "private_key", + "privatekey", + "access_token", + "accesstoken", + "refresh_token", + "refreshtoken", + "bot_token", + "bottoken", + "webhook_secret", + "webhooksecret", + "signing_secret", + "signingsecret", + "client_secret", + "clientsecret", +} + + +def _secret_key_name(key: Any) -> bool: + return str(key or "").replace("-", "_").lower() in _STRATEGY_SECRET_KEYS + + +def _contains_secret(value: Any) -> bool: + if isinstance(value, dict): + return any((_secret_key_name(k) and v not in (None, "", False)) or _contains_secret(v) for k, v in value.items()) + if isinstance(value, list): + return any(_contains_secret(v) for v in value) + return False + + +def reject_inline_strategy_secrets(exchange_config: Any) -> None: + if not isinstance(exchange_config, dict) or exchange_config.get("credential_id"): + return + if _contains_secret(exchange_config): + raise ValueError("Inline exchange secrets are not stored in strategies. Save credentials first and use credential_id.") + + +def redact_strategy_secrets(value: Any) -> Any: + if isinstance(value, dict): + out: Dict[str, Any] = {} + for k, v in value.items(): + if _secret_key_name(k) and v not in (None, "", False): + out[k] = "***" + else: + out[k] = redact_strategy_secrets(v) + return out + if isinstance(value, list): + return [redact_strategy_secrets(v) for v in value] + return value + + +def redact_strategy_row(row: Dict[str, Any] | None) -> Dict[str, Any] | None: + if not row: + return row + out = dict(row) + for field in ("exchange_config", "trading_config", "notification_config", "ai_model_config"): + if field in out: + out[field] = redact_strategy_secrets(out[field]) + return out + + def _normalize_cross_sectional_symbol_list( symbol_list: List[Any], market_category: str, @@ -1143,6 +1211,7 @@ def create_strategy(self, payload: Dict[str, Any]) -> int: from app.services.exchange_execution import coalesce_exchange_config_from_payload, resolve_exchange_config exchange_config = coalesce_exchange_config_from_payload(payload) + reject_inline_strategy_secrets(exchange_config) resolved_ex_cfg = resolve_exchange_config( exchange_config if isinstance(exchange_config, dict) else {}, @@ -1578,6 +1647,7 @@ def update_strategy(self, strategy_id: int, payload: Dict[str, Any], user_id: in 'exchange_config': exchange_config, 'trading_config': trading_config, }) + reject_inline_strategy_secrets(exchange_config) _merged_ex = _resolve_ex_upd( exchange_config if isinstance(exchange_config, dict) else {}, diff --git a/backend_api_python/app/utils/agent_auth.py b/backend_api_python/app/utils/agent_auth.py index 2e1936287..27845f847 100644 --- a/backend_api_python/app/utils/agent_auth.py +++ b/backend_api_python/app/utils/agent_auth.py @@ -266,7 +266,31 @@ def _touch_token_last_used(token_id: int) -> None: # ─────────────────────────── audit ─────────────────────────── -_REDACT_KEYS = {"password", "secret", "token", "apikey", "api_key", "authorization"} +_REDACT_KEYS = { + "password", + "secret", + "secret_key", + "secretkey", + "token", + "apikey", + "api_key", + "authorization", + "passphrase", + "private_key", + "privatekey", + "access_token", + "accesstoken", + "refresh_token", + "refreshtoken", + "bot_token", + "bottoken", + "webhook_secret", + "webhooksecret", + "signing_secret", + "signingsecret", + "client_secret", + "clientsecret", +} def _redact(obj: Any, depth: int = 0) -> Any: diff --git a/backend_api_python/tests/test_agent_security.py b/backend_api_python/tests/test_agent_security.py index 4e71d4a82..1df93736c 100644 --- a/backend_api_python/tests/test_agent_security.py +++ b/backend_api_python/tests/test_agent_security.py @@ -9,6 +9,7 @@ redact_secrets, redact_strategy_row, ) +from app.utils.agent_auth import _redact def test_redact_secrets_masks_known_keys(): @@ -24,6 +25,19 @@ def test_redact_secrets_masks_known_keys(): assert out["items"][0]["secret"] == "***" +def test_agent_audit_redact_masks_common_secret_keys(): + out = _redact({ + "secret_key": "s", + "secretKey": "camel", + "passphrase": "p", + "nested": {"client_secret": "c"}, + }) + assert out["secret_key"] == "" + assert out["secretKey"] == "" + assert out["passphrase"] == "" + assert out["nested"]["client_secret"] == "" + + def test_redact_strategy_row_covers_config_blocks(): row = { "id": 1, diff --git a/backend_api_python/tests/test_auth_legacy_fallback.py b/backend_api_python/tests/test_auth_legacy_fallback.py new file mode 100644 index 000000000..6fe490bea --- /dev/null +++ b/backend_api_python/tests/test_auth_legacy_fallback.py @@ -0,0 +1,32 @@ +def test_legacy_admin_fallback_requires_single_user_mode(client, monkeypatch): + import app.routes.auth as auth_route + import app.services.user_service as user_service_mod + + class DummySecurity: + def verify_turnstile(self, token, ip_address): + return True, "ok" + + def check_login_allowed(self, username, ip_address): + return True, "" + + def record_login_attempt(self, *args, **kwargs): + return True + + def log_security_event(self, *args, **kwargs): + return True + + class DummyUsers: + def authenticate(self, username, password, update_last_login=False): + return None + + monkeypatch.setattr("app.services.security_service.get_security_service", lambda: DummySecurity()) + monkeypatch.setattr(user_service_mod, "get_user_service", lambda: DummyUsers()) + monkeypatch.setattr(auth_route, "_is_single_user_mode", lambda: False) + + def fail_legacy(username, password): + raise AssertionError("legacy auth should not run") + + monkeypatch.setattr(auth_route, "authenticate_legacy", fail_legacy) + + resp = client.post("/api/auth/login", json={"username": "testadmin", "password": "testpass123"}) + assert resp.status_code == 401 diff --git a/backend_api_python/tests/test_settings_secret_masking.py b/backend_api_python/tests/test_settings_secret_masking.py new file mode 100644 index 000000000..92f53c734 --- /dev/null +++ b/backend_api_python/tests/test_settings_secret_masking.py @@ -0,0 +1,28 @@ +def test_settings_values_masks_password_fields(client, monkeypatch): + import app.routes.settings as settings_route + import app.utils.auth as auth_mod + + monkeypatch.setattr(auth_mod, "verify_token", lambda token: { + "sub": "admin", + "user_id": 1, + "role": "admin", + }) + monkeypatch.setattr(settings_route, "read_env_file", lambda: { + "CUSTOM_API_KEY": "real-secret", + "LLM_PROVIDER": "custom", + }) + monkeypatch.setattr(settings_route, "CONFIG_SCHEMA", { + "ai": { + "items": [ + {"key": "CUSTOM_API_KEY", "type": "password"}, + {"key": "LLM_PROVIDER", "type": "select"}, + ] + } + }) + + resp = client.get("/api/settings/values", headers={"Authorization": "Bearer token"}) + assert resp.status_code == 200 + data = resp.get_json()["data"]["ai"] + assert data["CUSTOM_API_KEY"] == "" + assert data["CUSTOM_API_KEY_configured"] is True + assert data["LLM_PROVIDER"] == "custom" diff --git a/backend_api_python/tests/test_strategy_secret_handling.py b/backend_api_python/tests/test_strategy_secret_handling.py new file mode 100644 index 000000000..36e862666 --- /dev/null +++ b/backend_api_python/tests/test_strategy_secret_handling.py @@ -0,0 +1,28 @@ +import pytest + +from app.services.strategy import ( + redact_strategy_row, + reject_inline_strategy_secrets, +) + + +def test_reject_inline_strategy_secrets_without_credential_id(): + with pytest.raises(ValueError, match="credential_id"): + reject_inline_strategy_secrets({"exchange_id": "binance", "secret_key": "s"}) + + +def test_allow_strategy_credential_reference_without_inline_keys(): + reject_inline_strategy_secrets({"exchange_id": "binance", "credential_id": 12, "secret_key": "s"}) + + +def test_redact_strategy_row_masks_nested_secrets(): + row = { + "id": 1, + "exchange_config": {"api_key": "k"}, + "trading_config": {"exchange_config": {"secretKey": "s"}}, + "notification_config": {"webhook_secret": "w"}, + } + out = redact_strategy_row(row) + assert out["exchange_config"]["api_key"] == "***" + assert out["trading_config"]["exchange_config"]["secretKey"] == "***" + assert out["notification_config"]["webhook_secret"] == "***" diff --git a/docker-compose.ghcr.yml b/docker-compose.ghcr.yml index 91517bde8..2d36e27b8 100644 --- a/docker-compose.ghcr.yml +++ b/docker-compose.ghcr.yml @@ -134,7 +134,7 @@ services: container_name: quantdinger-frontend restart: unless-stopped ports: - - "${FRONTEND_PORT:-8888}:80" + - "${FRONTEND_HOST:-127.0.0.1}:${FRONTEND_PORT:-8888}:80" environment: - BACKEND_URL=${BACKEND_URL:-http://backend:5000} depends_on: diff --git a/docker-compose.yml b/docker-compose.yml index 2bd5bc3d0..a053a85d4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -167,7 +167,7 @@ services: container_name: quantdinger-frontend restart: unless-stopped ports: - - "${FRONTEND_PORT:-8888}:80" + - "${FRONTEND_HOST:-127.0.0.1}:${FRONTEND_PORT:-8888}:80" environment: # nginx envsubst will inject this into the proxy_pass directive at start. # Override with the backend's reachable URL when deploying outside compose From 71d96fb8b5cf5ba42d1f80cf68255199794ef10b Mon Sep 17 00:00:00 2001 From: likawa3b <42701727+likawa3b@users.noreply.github.com> Date: Sat, 27 Jun 2026 15:26:24 -0400 Subject: [PATCH 2/2] Fix reviewed secret redaction gaps --- .../app/routes/agent_v1/_security.py | 6 ++--- backend_api_python/app/routes/settings.py | 5 ++-- backend_api_python/app/utils/agent_auth.py | 2 +- .../tests/test_agent_security.py | 9 ++++--- .../tests/test_settings_secret_masking.py | 25 +++++++++++++++++++ 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/backend_api_python/app/routes/agent_v1/_security.py b/backend_api_python/app/routes/agent_v1/_security.py index 3549dd461..0c94f7b42 100644 --- a/backend_api_python/app/routes/agent_v1/_security.py +++ b/backend_api_python/app/routes/agent_v1/_security.py @@ -6,9 +6,9 @@ # Upper bound for indicator Python source accepted via agent/MCP paths. MAX_INDICATOR_CODE_BYTES = 512 * 1024 -# Keys stripped or masked anywhere in agent-facing JSON (case-sensitive). +# Keys stripped or masked anywhere in agent-facing JSON. _SECRET_KEYS = frozenset({ - "api_key", "secret_key", "passphrase", "apiKey", "secret", "password", + "apikey", "api_key", "secretkey", "secret_key", "passphrase", "secret", "password", "private_key", "access_token", "refresh_token", "bot_token", "webhook_secret", "signing_secret", "client_secret", }) @@ -33,7 +33,7 @@ def redact_secrets(value: Any, *, depth: int = 0, max_depth: int = 6) -> Any: out: dict[str, Any] = {} for k, v in value.items(): key = str(k) - if key in _SECRET_KEYS and v not in (None, "", False): + if key.replace("-", "_").lower() in _SECRET_KEYS and v not in (None, "", False): out[key] = "***" elif isinstance(v, Mapping): out[key] = redact_secrets(v, depth=depth + 1, max_depth=max_depth) diff --git a/backend_api_python/app/routes/settings.py b/backend_api_python/app/routes/settings.py index f7809e78f..880cd73f7 100644 --- a/backend_api_python/app/routes/settings.py +++ b/backend_api_python/app/routes/settings.py @@ -1922,10 +1922,11 @@ def get_settings_values(): result[group_key] = {} for item in group['items']: key = item['key'] - value = env_values.get(key, item.get('default', '')) + raw_value = env_values.get(key) + value = raw_value if raw_value is not None else item.get('default', '') if item['type'] == 'password': result[group_key][key] = '' - result[group_key][f'{key}_configured'] = bool(value) + result[group_key][f'{key}_configured'] = bool(raw_value) else: result[group_key][key] = value diff --git a/backend_api_python/app/utils/agent_auth.py b/backend_api_python/app/utils/agent_auth.py index 27845f847..5a9c0ead7 100644 --- a/backend_api_python/app/utils/agent_auth.py +++ b/backend_api_python/app/utils/agent_auth.py @@ -299,7 +299,7 @@ def _redact(obj: Any, depth: int = 0) -> Any: if isinstance(obj, dict): out = {} for k, v in obj.items(): - if str(k).lower() in _REDACT_KEYS: + if str(k).replace("-", "_").lower() in _REDACT_KEYS: out[k] = "" else: out[k] = _redact(v, depth + 1) diff --git a/backend_api_python/tests/test_agent_security.py b/backend_api_python/tests/test_agent_security.py index 1df93736c..e808fa2ec 100644 --- a/backend_api_python/tests/test_agent_security.py +++ b/backend_api_python/tests/test_agent_security.py @@ -27,25 +27,28 @@ def test_redact_secrets_masks_known_keys(): def test_agent_audit_redact_masks_common_secret_keys(): out = _redact({ + "secret-key": "dash", "secret_key": "s", "secretKey": "camel", "passphrase": "p", - "nested": {"client_secret": "c"}, + "nested": {"client-secret": "c"}, }) + assert out["secret-key"] == "" assert out["secret_key"] == "" assert out["secretKey"] == "" assert out["passphrase"] == "" - assert out["nested"]["client_secret"] == "" + assert out["nested"]["client-secret"] == "" def test_redact_strategy_row_covers_config_blocks(): row = { "id": 1, - "exchange_config": {"apiKey": "k"}, + "exchange_config": {"apiKey": "k", "secretKey": "s"}, "notification_config": {"webhook_secret": "wh"}, } out = redact_strategy_row(row) assert out["exchange_config"]["apiKey"] == "***" + assert out["exchange_config"]["secretKey"] == "***" assert out["notification_config"]["webhook_secret"] == "***" diff --git a/backend_api_python/tests/test_settings_secret_masking.py b/backend_api_python/tests/test_settings_secret_masking.py index 92f53c734..2f1bc2083 100644 --- a/backend_api_python/tests/test_settings_secret_masking.py +++ b/backend_api_python/tests/test_settings_secret_masking.py @@ -26,3 +26,28 @@ def test_settings_values_masks_password_fields(client, monkeypatch): assert data["CUSTOM_API_KEY"] == "" assert data["CUSTOM_API_KEY_configured"] is True assert data["LLM_PROVIDER"] == "custom" + + +def test_settings_values_does_not_treat_password_default_as_configured(client, monkeypatch): + import app.routes.settings as settings_route + import app.utils.auth as auth_mod + + monkeypatch.setattr(auth_mod, "verify_token", lambda token: { + "sub": "admin", + "user_id": 1, + "role": "admin", + }) + monkeypatch.setattr(settings_route, "read_env_file", lambda: {}) + monkeypatch.setattr(settings_route, "CONFIG_SCHEMA", { + "security": { + "items": [ + {"key": "ADMIN_PASSWORD", "type": "password", "default": "123456"}, + ] + } + }) + + resp = client.get("/api/settings/values", headers={"Authorization": "Bearer token"}) + assert resp.status_code == 200 + data = resp.get_json()["data"]["security"] + assert data["ADMIN_PASSWORD"] == "" + assert data["ADMIN_PASSWORD_configured"] is False