New CSRF middleware by jontyms · Pull Request #352 · HackUCF/OnboardLite

jontyms · 2026-01-31T21:03:14Z

No description provided.

Copilot

Pull request overview

This PR introduces a CSRF protection middleware and corresponding configuration hooks to the application, aiming to enforce origin-based request validation and make security behavior configurable.

Changes:

Added a security section in options-example.yml to document CSRF-related configuration.
Introduced SecurityConfig in app/util/settings.py and exposed it on the global Settings object.
Implemented CSRFMiddleware in app/util/csrf.py and registered it in app/main.py so all requests pass through CSRF checks.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
options-example.yml	Adds a `security` config section with CSRF-related knobs intended to control the new middleware.
app/util/settings.py	Defines `SecurityConfig` (trusted origins and bypass paths) and wires it into the global `Settings` singleton.
app/util/csrf.py	Implements `CSRFMiddleware` that enforces CSRF checks using `Sec-Fetch-Site`/`Origin` and logs/blocks violations.
app/main.py	Registers the new `CSRFMiddleware` on the FastAPI app so it applies globally.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-31T21:06:19Z

app/util/csrf.py

+
+from fastapi import Request, Response
+from starlette.middleware.base import BaseHTTPMiddleware


The re module is imported but not used anywhere in this module. This unused import can be removed to keep the file clean and avoid confusion about planned regex-based behavior that is not actually implemented.

Copilot · 2026-01-31T21:06:20Z

options-example.yml

+  csrf_check_enable: true
+  csrf_failure_action: "exception" # Options: "block", "exception", "log"


The example security section here defines csrf_check_enable and csrf_failure_action, but these fields do not exist on SecurityConfig (app/util/settings.py) and are never read by CSRFMiddleware, so they are currently ignored at runtime. This makes the sample config misleading; consider either adding matching fields and wiring them into the middleware, or updating the example to document the actual supported options (e.g., trusted_origins, bypass_paths).

Suggested change

csrf_check_enable: true

csrf_failure_action: "exception" # Options: "block", "exception", "log"

# List of allowed origins for CSRF-protected requests

trusted_origins:

- "https://join.hackucf.org"

- "https://example.com"

# Paths that should bypass CSRF checks

# (e.g., health checks, webhooks, or other endpoints where CSRF is not applicable)

bypass_paths:

- "^/api/webhook/"

- "^/healthz$"

Copilot · 2026-01-31T21:06:20Z

app/util/settings.py

+        bypass_paths (List[str]): List of path prefixes to bypass CSRF checks (e.g. "/api/webhooks").
+    """
+
+    trusted_origins: List[str] = Field(default_factory=list)
+    bypass_paths: List[str] = Field(default_factory=list)


SecurityConfig only defines trusted_origins and bypass_paths, but options-example.yml documents security.csrf_check_enable and security.csrf_failure_action. Because these keys are not modeled here, Pydantic will silently ignore them when constructing security_config, so operators cannot actually toggle CSRF checks or failure behavior via configuration; consider adding typed fields for these options and using them in the CSRF middleware, or removing them from the documented config to avoid dead settings.

Suggested change

bypass_paths (List[str]): List of path prefixes to bypass CSRF checks (e.g. "/api/webhooks").

"""

trusted_origins: List[str] = Field(default_factory=list)

bypass_paths: List[str] = Field(default_factory=list)

bypass_paths (List[str]): List of path prefixes to bypass CSRF checks (e.g. "/api/webhooks").

csrf_check_enable (bool): Whether CSRF checks are enabled.

csrf_failure_action (str): Action to take when a CSRF check fails (e.g. "reject").

"""

trusted_origins: List[str] = Field(default_factory=list)

bypass_paths: List[str] = Field(default_factory=list)

csrf_check_enable: bool = True

csrf_failure_action: str = "reject"

Copilot · 2026-01-31T21:06:20Z

app/util/csrf.py

+        return origin in trusted_origins
+
+    async def _handle_violation(self, request: Request, reason: str):
+        msg = f"CSRF Violation: {reason} | Path: {request.url.path} | UA: {request.headers.get('user-agent')}"
+
+        # Always Log
+        logger.error(msg)
+
+        # Report to Sentry if enabled (Monitoring)
+        if Settings().telemetry.enable:
+            try:
+                import sentry_sdk
+
+                with sentry_sdk.push_scope() as scope:
+                    scope.set_tag("csrf_violation", "true")
+                    sentry_sdk.capture_message(msg, level="error")
+            except ImportError:
+                pass
+            except Exception as e:
+                logger.error(f"Failed to report to Sentry: {e}")
+


The CSRF failure handling logic is hardcoded to log and return a 403 response, but the sample configuration (options-example.yml) advertises a security.csrf_failure_action option with modes like block, exception, and log. Since _handle_violation does not read any such setting from Settings().security, the documented behavior cannot be configured; consider wiring this function to a configurable action (e.g., raising, logging-only, or blocking) or updating the config docs to match the actual behavior.

Copilot · 2026-01-31T21:06:21Z

app/main.py


 # Initiate FastAPI.
 app = FastAPI()
+app.add_middleware(CSRFMiddleware)


CSRFMiddleware is added to the FastAPI app unconditionally, but the example configuration introduces a security.csrf_check_enable flag. As implemented, that flag has no effect because the middleware is always registered; if the intent is to allow CSRF protection to be disabled in some environments, you likely want to gate middleware registration (or add an early exit in dispatch) based on that setting.

Suggested change

app.add_middleware(CSRFMiddleware)

if Settings().security.csrf_check_enable:

app.add_middleware(CSRFMiddleware)

Copilot · 2026-01-31T21:06:21Z

app/util/csrf.py

+                with sentry_sdk.push_scope() as scope:
+                    scope.set_tag("csrf_violation", "true")
+                    sentry_sdk.capture_message(msg, level="error")
+            except ImportError:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except ImportError:

except ImportError:

# Sentry SDK is not installed; skip telemetry reporting but do not block the request.

jontyms requested a review from Copilot January 31, 2026 21:03

jontyms marked this pull request as ready for review January 31, 2026 21:03

Copilot started reviewing on behalf of jontyms January 31, 2026 21:03 View session

New CSRF middleware

425204a

jontyms force-pushed the csrf-middleware branch from ee11023 to 425204a Compare January 31, 2026 21:04

Copilot AI reviewed Jan 31, 2026

View reviewed changes

jontyms added 2 commits January 31, 2026 16:08

Update options-example.yml

e4cf81f

I like whitespace

ca90c2d

jontyms merged commit 307f88c into HackUCF:dev Jan 31, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New CSRF middleware#352

New CSRF middleware#352
jontyms merged 3 commits intoHackUCF:devfrom
jontyms:csrf-middleware

jontyms commented Jan 31, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Copilot AI Jan 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments


		from fastapi import Request, Response
		from starlette.middleware.base import BaseHTTPMiddleware

		csrf_check_enable: true
		csrf_failure_action: "exception" # Options: "block", "exception", "log"

-  csrf_check_enable: true
-  csrf_failure_action: "exception" # Options: "block", "exception", "log"
+  # List of allowed origins for CSRF-protected requests
+  trusted_origins:
+    - "https://join.hackucf.org"
+    - "https://example.com"
+  # Paths that should bypass CSRF checks
+  # (e.g., health checks, webhooks, or other endpoints where CSRF is not applicable)
+  bypass_paths:
+    - "^/api/webhook/"
+    - "^/healthz$"

	app.add_middleware(CSRFMiddleware)
	if Settings().security.csrf_check_enable:
	app.add_middleware(CSRFMiddleware)

	except ImportError:
	except ImportError:
	# Sentry SDK is not installed; skip telemetry reporting but do not block the request.

Conversation

jontyms commented Jan 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments