refactor: code consistency and best practices
- add type hints to error handlers in app/__init__.py - add docstrings to nested callback functions - remove deprecated X-XSS-Protection header (superseded by CSP) - fix typo in cleanup log message (entr(ies) -> entries) - standardize loop variable naming in fpaste CLI - update test for intentional header removal
This commit is contained in:
@@ -6,6 +6,7 @@ import sys
|
|||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
from flask import Flask, Response, g, request
|
from flask import Flask, Response, g, request
|
||||||
|
from werkzeug.exceptions import HTTPException
|
||||||
|
|
||||||
from app.config import VERSION, config
|
from app.config import VERSION, config
|
||||||
|
|
||||||
@@ -39,12 +40,14 @@ def setup_security_headers(app: Flask) -> None:
|
|||||||
|
|
||||||
@app.after_request
|
@app.after_request
|
||||||
def add_security_headers(response: Response) -> Response:
|
def add_security_headers(response: Response) -> Response:
|
||||||
|
"""Apply security headers to response.
|
||||||
|
|
||||||
|
Headers follow OWASP recommendations for API security.
|
||||||
|
"""
|
||||||
# Prevent MIME type sniffing
|
# Prevent MIME type sniffing
|
||||||
response.headers["X-Content-Type-Options"] = "nosniff"
|
response.headers["X-Content-Type-Options"] = "nosniff"
|
||||||
# Prevent clickjacking
|
# Prevent clickjacking
|
||||||
response.headers["X-Frame-Options"] = "DENY"
|
response.headers["X-Frame-Options"] = "DENY"
|
||||||
# XSS protection (legacy but still useful)
|
|
||||||
response.headers["X-XSS-Protection"] = "1; mode=block"
|
|
||||||
# Referrer policy
|
# Referrer policy
|
||||||
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
|
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
|
||||||
# Content Security Policy (restrictive for API)
|
# Content Security Policy (restrictive for API)
|
||||||
@@ -64,8 +67,8 @@ def setup_request_id(app: Flask) -> None:
|
|||||||
"""Add request ID tracking for log correlation and tracing."""
|
"""Add request ID tracking for log correlation and tracing."""
|
||||||
|
|
||||||
@app.before_request
|
@app.before_request
|
||||||
def assign_request_id():
|
def assign_request_id() -> None:
|
||||||
# Use incoming X-Request-ID from proxy, or generate a new one
|
"""Assign unique request ID from header or generate new UUID."""
|
||||||
request_id = request.headers.get("X-Request-ID", "").strip()
|
request_id = request.headers.get("X-Request-ID", "").strip()
|
||||||
if not request_id:
|
if not request_id:
|
||||||
request_id = str(uuid.uuid4())
|
request_id = str(uuid.uuid4())
|
||||||
@@ -73,7 +76,7 @@ def setup_request_id(app: Flask) -> None:
|
|||||||
|
|
||||||
@app.after_request
|
@app.after_request
|
||||||
def add_request_id_header(response: Response) -> Response:
|
def add_request_id_header(response: Response) -> Response:
|
||||||
# Echo request ID back to client for tracing
|
"""Echo request ID in response header and log access."""
|
||||||
request_id = getattr(g, "request_id", None)
|
request_id = getattr(g, "request_id", None)
|
||||||
if request_id:
|
if request_id:
|
||||||
response.headers["X-Request-ID"] = request_id
|
response.headers["X-Request-ID"] = request_id
|
||||||
@@ -90,11 +93,12 @@ def setup_request_id(app: Flask) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def setup_error_handlers(app: Flask) -> None:
|
def setup_error_handlers(app: Flask) -> None:
|
||||||
"""Register global error handlers."""
|
"""Register global error handlers with JSON responses."""
|
||||||
import json
|
import json
|
||||||
|
|
||||||
@app.errorhandler(400)
|
@app.errorhandler(400)
|
||||||
def bad_request(error):
|
def bad_request(error: HTTPException) -> Response:
|
||||||
|
"""Handle 400 Bad Request errors."""
|
||||||
app.logger.warning("Bad request: %s [rid=%s]", request.path, getattr(g, "request_id", "-"))
|
app.logger.warning("Bad request: %s [rid=%s]", request.path, getattr(g, "request_id", "-"))
|
||||||
return Response(
|
return Response(
|
||||||
json.dumps({"error": "Bad request"}),
|
json.dumps({"error": "Bad request"}),
|
||||||
@@ -103,7 +107,8 @@ def setup_error_handlers(app: Flask) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
@app.errorhandler(404)
|
@app.errorhandler(404)
|
||||||
def not_found(error):
|
def not_found(error: HTTPException) -> Response:
|
||||||
|
"""Handle 404 Not Found errors."""
|
||||||
return Response(
|
return Response(
|
||||||
json.dumps({"error": "Not found"}),
|
json.dumps({"error": "Not found"}),
|
||||||
status=404,
|
status=404,
|
||||||
@@ -111,7 +116,8 @@ def setup_error_handlers(app: Flask) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
@app.errorhandler(429)
|
@app.errorhandler(429)
|
||||||
def rate_limit_exceeded(error):
|
def rate_limit_exceeded(error: HTTPException) -> Response:
|
||||||
|
"""Handle 429 Too Many Requests errors."""
|
||||||
app.logger.warning(
|
app.logger.warning(
|
||||||
"Rate limit exceeded: %s from %s [rid=%s]",
|
"Rate limit exceeded: %s from %s [rid=%s]",
|
||||||
request.path,
|
request.path,
|
||||||
@@ -125,7 +131,8 @@ def setup_error_handlers(app: Flask) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
@app.errorhandler(500)
|
@app.errorhandler(500)
|
||||||
def internal_error(error):
|
def internal_error(error: HTTPException) -> Response:
|
||||||
|
"""Handle 500 Internal Server errors."""
|
||||||
app.logger.error(
|
app.logger.error(
|
||||||
"Internal error: %s - %s [rid=%s]",
|
"Internal error: %s - %s [rid=%s]",
|
||||||
request.path,
|
request.path,
|
||||||
@@ -139,7 +146,8 @@ def setup_error_handlers(app: Flask) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
@app.errorhandler(Exception)
|
@app.errorhandler(Exception)
|
||||||
def handle_exception(error):
|
def handle_exception(error: Exception) -> Response:
|
||||||
|
"""Handle unhandled exceptions with generic 500 response."""
|
||||||
app.logger.exception(
|
app.logger.exception(
|
||||||
"Unhandled exception: %s [rid=%s]", str(error), getattr(g, "request_id", "-")
|
"Unhandled exception: %s [rid=%s]", str(error), getattr(g, "request_id", "-")
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -59,7 +59,7 @@ def run_scheduled_cleanup():
|
|||||||
|
|
||||||
count = cleanup_rate_limits()
|
count = cleanup_rate_limits()
|
||||||
if count > 0:
|
if count > 0:
|
||||||
current_app.logger.info(f"Cleaned up {count} rate limit entr(ies)")
|
current_app.logger.info(f"Cleaned up {count} rate limit entries")
|
||||||
|
|
||||||
|
|
||||||
from app.api import routes # noqa: E402, F401
|
from app.api import routes # noqa: E402, F401
|
||||||
|
|||||||
14
fpaste
14
fpaste
@@ -640,7 +640,7 @@ def cmd_delete(args: argparse.Namespace, config: dict[str, Any]) -> None:
|
|||||||
|
|
||||||
delete_all = getattr(args, "all", False)
|
delete_all = getattr(args, "all", False)
|
||||||
confirm_count = getattr(args, "confirm", None)
|
confirm_count = getattr(args, "confirm", None)
|
||||||
paste_ids = [pid.split("/")[-1] for pid in (args.ids or [])]
|
paste_ids = [paste_id.split("/")[-1] for paste_id in (args.ids or [])]
|
||||||
|
|
||||||
# Validate arguments
|
# Validate arguments
|
||||||
if delete_all and paste_ids:
|
if delete_all and paste_ids:
|
||||||
@@ -890,20 +890,20 @@ def cmd_export(args: argparse.Namespace, config: dict[str, Any]) -> None:
|
|||||||
exported, skipped, errors = 0, 0, 0
|
exported, skipped, errors = 0, 0, 0
|
||||||
manifest: list[dict[str, Any]] = []
|
manifest: list[dict[str, Any]] = []
|
||||||
|
|
||||||
for p in pastes:
|
for paste in pastes:
|
||||||
paste_id = p["id"]
|
paste_id = paste["id"]
|
||||||
mime_type = p.get("mime_type", "application/octet-stream")
|
mime_type = paste.get("mime_type", "application/octet-stream")
|
||||||
|
|
||||||
if not args.quiet:
|
if not args.quiet:
|
||||||
print(f"exporting {paste_id}...", end=" ", file=sys.stderr)
|
print(f"exporting {paste_id}...", end=" ", file=sys.stderr)
|
||||||
|
|
||||||
if p.get("burn_after_read"):
|
if paste.get("burn_after_read"):
|
||||||
if not args.quiet:
|
if not args.quiet:
|
||||||
print("skipped (burn-after-read)", file=sys.stderr)
|
print("skipped (burn-after-read)", file=sys.stderr)
|
||||||
skipped += 1
|
skipped += 1
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if p.get("password_protected"):
|
if paste.get("password_protected"):
|
||||||
if not args.quiet:
|
if not args.quiet:
|
||||||
print("skipped (password-protected)", file=sys.stderr)
|
print("skipped (password-protected)", file=sys.stderr)
|
||||||
skipped += 1
|
skipped += 1
|
||||||
@@ -939,7 +939,7 @@ def cmd_export(args: argparse.Namespace, config: dict[str, Any]) -> None:
|
|||||||
"filename": filename,
|
"filename": filename,
|
||||||
"mime_type": mime_type,
|
"mime_type": mime_type,
|
||||||
"size": len(content),
|
"size": len(content),
|
||||||
"created_at": p.get("created_at"),
|
"created_at": paste.get("created_at"),
|
||||||
"decrypted": decrypted,
|
"decrypted": decrypted,
|
||||||
"encrypted": paste_id in keys and not decrypted,
|
"encrypted": paste_id in keys and not decrypted,
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,10 +16,10 @@ class TestSecurityHeaders:
|
|||||||
response = client.get("/")
|
response = client.get("/")
|
||||||
assert response.headers.get("X-Frame-Options") == "DENY"
|
assert response.headers.get("X-Frame-Options") == "DENY"
|
||||||
|
|
||||||
def test_x_xss_protection(self, client):
|
def test_x_xss_protection_not_present(self, client):
|
||||||
"""X-XSS-Protection header is set."""
|
"""X-XSS-Protection header is not set (deprecated, superseded by CSP)."""
|
||||||
response = client.get("/")
|
response = client.get("/")
|
||||||
assert response.headers.get("X-XSS-Protection") == "1; mode=block"
|
assert response.headers.get("X-XSS-Protection") is None
|
||||||
|
|
||||||
def test_content_security_policy(self, client):
|
def test_content_security_policy(self, client):
|
||||||
"""Content-Security-Policy header is set."""
|
"""Content-Security-Policy header is set."""
|
||||||
|
|||||||
Reference in New Issue
Block a user