Skip to content

Commit 38a63f6

Browse files
committed
Security fixes
1 parent 5de0928 commit 38a63f6

File tree

3 files changed

+98
-7
lines changed

3 files changed

+98
-7
lines changed

gefapi/__init__.py

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,14 +298,47 @@ def set_security_headers(response):
298298
# Transfer rate limiting configuration to Flask app config
299299
app.config["RATE_LIMITING"] = SETTINGS.get("RATE_LIMITING", {})
300300

301-
# Ensure JWT_SECRET_KEY is set with proper fallback
301+
# Ensure JWT_SECRET_KEY is set with proper fallback and validation
302302
jwt_secret = (
303303
SETTINGS.get("JWT_SECRET_KEY")
304304
or SETTINGS.get("SECRET_KEY")
305305
or os.getenv("JWT_SECRET_KEY")
306306
or os.getenv("SECRET_KEY")
307307
)
308308

309+
# Security: Validate JWT secret key to prevent insecure configurations
310+
# This list includes actual defaults from .env.example and develop/test configs
311+
INSECURE_KEY_PATTERNS = [
312+
"your_jwt_secret_key_here",
313+
"your_secret_key_here",
314+
"develop_jwt_secret",
315+
"develop_secret_key",
316+
"test_jwt_secret_key",
317+
"test_secret_key",
318+
]
319+
320+
if not jwt_secret:
321+
if os.getenv("ENVIRONMENT") == "prod":
322+
raise RuntimeError(
323+
"CRITICAL SECURITY ERROR: JWT_SECRET_KEY must be set in production. "
324+
"Set the JWT_SECRET_KEY or SECRET_KEY environment variable."
325+
)
326+
logger.warning(
327+
"JWT_SECRET_KEY is not set. This is insecure and must be fixed before "
328+
"deploying to production. Set JWT_SECRET_KEY or SECRET_KEY."
329+
)
330+
elif jwt_secret.lower() in INSECURE_KEY_PATTERNS or len(jwt_secret) < 32:
331+
if os.getenv("ENVIRONMENT") == "prod":
332+
raise RuntimeError(
333+
"CRITICAL SECURITY ERROR: JWT_SECRET_KEY is insecure (too short or "
334+
"uses a known default value). Use a cryptographically secure random "
335+
"string of at least 32 characters."
336+
)
337+
logger.warning(
338+
"JWT_SECRET_KEY appears insecure (too short or uses a default value). "
339+
"This must be fixed before deploying to production."
340+
)
341+
309342
app.config["JWT_SECRET_KEY"] = jwt_secret
310343
app.config["JWT_ACCESS_TOKEN_EXPIRES"] = SETTINGS.get("JWT_ACCESS_TOKEN_EXPIRES")
311344
app.config["JWT_TOKEN_LOCATION"] = SETTINGS.get("JWT_TOKEN_LOCATION")
@@ -431,13 +464,21 @@ def ping():
431464

432465
@app.route("/debug/routes", methods=["GET"])
433466
def debug_routes():
434-
"""Debug endpoint to show all registered routes"""
467+
"""Debug endpoint to show all registered routes.
468+
469+
Security: This endpoint is only available in development/test environments.
470+
It is completely disabled in production and staging unless explicitly enabled.
471+
"""
435472
import os
436473

437-
if (
438-
os.getenv("ENVIRONMENT") == "prod"
439-
and os.getenv("DEBUG_ROUTES_ENABLED", "false").lower() != "true"
440-
):
474+
environment = os.getenv("ENVIRONMENT", "dev")
475+
debug_routes_enabled = os.getenv("DEBUG_ROUTES_ENABLED", "false").lower() == "true"
476+
477+
# Security: Only allow in dev/test environments, or if explicitly enabled
478+
if environment not in ("dev", "test") and not debug_routes_enabled:
479+
logger.warning(
480+
f"[SECURITY]: Blocked debug/routes access in {environment} environment"
481+
)
441482
abort(404)
442483

443484
routes_info = []

gefapi/routes/api/v1/executions.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,30 @@ def run_script(script):
103103
if request.get_json(silent=True):
104104
params.update(request.get_json())
105105
if "token" in params:
106+
# Security: Log deprecated token query parameter usage
107+
# Tokens in URLs are logged in server logs, browser history, and referrer
108+
# headers, which is a security risk (CWE-598)
109+
import rollbar
110+
111+
rollbar.report_message(
112+
"SECURITY WARNING: Deprecated token query parameter used in "
113+
f"script execution request for script '{script}' by user "
114+
f"'{user.email}'. Token-based authentication via query "
115+
"parameters is deprecated and will be removed in a future "
116+
"version. Use Authorization header instead.",
117+
level="warning",
118+
extra_data={
119+
"script": script,
120+
"user_id": str(user.id),
121+
"user_email": user.email,
122+
"endpoint": "run_script",
123+
"security_issue": "CWE-598",
124+
},
125+
)
126+
logger.warning(
127+
f"[SECURITY]: Deprecated token query parameter used by {user.email} "
128+
f"for script {script}. This should be migrated to header-based auth."
129+
)
106130
del params["token"]
107131
execution = ExecutionService.create_execution(script, params, user)
108132
except ScriptNotFound as e:

gefapi/services/user_service.py

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,20 @@ def _validate_password_strength(password: str) -> None:
8383
class UserService:
8484
"""User Class"""
8585

86+
# Security: Explicitly allowed fields for filter and sort operations
87+
# to prevent unauthorized access to sensitive model fields
88+
USER_ALLOWED_FILTER_FIELDS = {
89+
"id",
90+
"email",
91+
"name",
92+
"role",
93+
"country",
94+
"institution",
95+
"created_at",
96+
"updated_at",
97+
}
98+
USER_ALLOWED_SORT_FIELDS = USER_ALLOWED_FILTER_FIELDS
99+
86100
@staticmethod
87101
def create_user(user, legacy=True):
88102
"""Create a new user account.
@@ -295,7 +309,7 @@ def get_users(
295309

296310
query = db.session.query(User)
297311

298-
# SQL-style filter_param
312+
# SQL-style filter_param with field allowlist for security
299313
if filter_param:
300314
import re
301315

@@ -312,6 +326,12 @@ def get_users(
312326
field = field.strip().lower()
313327
op = op.strip().lower()
314328
value = value.strip().strip("'\"")
329+
# Security: Only allow filtering on explicitly allowed fields
330+
if field not in UserService.USER_ALLOWED_FILTER_FIELDS:
331+
logger.warning(
332+
f"[SERVICE]: Rejected filter on disallowed field: {field}"
333+
)
334+
continue
315335
col = getattr(User, field, None)
316336
if col is not None:
317337
# Check if this is a string column for case-insensitive compare
@@ -357,6 +377,12 @@ def get_users(
357377
parts = sort_expr.split()
358378
field = parts[0].lower()
359379
direction = parts[1].lower() if len(parts) > 1 else "asc"
380+
# Security: Only allow sorting on explicitly allowed fields
381+
if field not in UserService.USER_ALLOWED_SORT_FIELDS:
382+
logger.warning(
383+
f"[SERVICE]: Rejected sort on disallowed field: {field}"
384+
)
385+
continue
360386
col = getattr(User, field, None)
361387
if col is not None:
362388
if direction == "desc":

0 commit comments

Comments
 (0)