Skip to content

Add Content-Security-Policy and X-Frame-Options#465

Merged
mtelvers merged 2 commits intoocurrent:masterfrom
mtelvers:html-header
Oct 29, 2025
Merged

Add Content-Security-Policy and X-Frame-Options#465
mtelvers merged 2 commits intoocurrent:masterfrom
mtelvers:html-header

Conversation

@mtelvers
Copy link
Copy Markdown
Member

While there is no login form with current_web as it uses OAuth authentication, there is a possible attack vector with clickjacking, whereby a malicious person may trick an administrator into needlessly rebuilding or cancelling pipeline stages.

This PR adds the recommended header options to mitigate such attacks.

@avsm avsm requested a review from Copilot October 28, 2025 16:53
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds security headers (X-Frame-Options and Content-Security-Policy) to mitigate clickjacking attacks in the current_web application. These headers prevent the application from being embedded in iframes, protecting administrators from being tricked into performing unwanted actions.

Key changes:

  • Introduced a new add_security_headers utility function that adds both security headers to response headers
  • Applied the security headers across all response generation points in the application

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib_web/utils.ml Added the add_security_headers helper function to centralize security header management
lib_web/resource.ml Applied security headers to static file and crunched resource responses
lib_web/pipeline.ml Applied security headers to SVG pipeline visualization responses
lib_web/log_rules.ml Applied security headers to CSV export responses
lib_web/job.ml Applied security headers to job log streaming responses
lib_web/current_web.ml Applied security headers to Prometheus metrics endpoint responses
lib_web/context.ml Applied security headers to session cookie responses

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

@@ -108,6 +108,7 @@ let static ~content_type ?(max_age=86400) body = object
("Content-Type", content_type);
("Cache-Control", Printf.sprintf "public, max-age=%d;" max_age);
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing semicolon in Cache-Control header value. The semicolon should only be used as a separator between directives, not as a terminator.

Suggested change
("Cache-Control", Printf.sprintf "public, max-age=%d;" max_age);
("Cache-Control", Printf.sprintf "public, max-age=%d" max_age);

Copilot uses AI. Check for mistakes.
@@ -130,6 +131,7 @@ let crunch ?content_type ?(max_age=86400) _ = object
("Content-Type", content_type);
("Cache-Control", Printf.sprintf "public, max-age=%d;" max_age);
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing semicolon in Cache-Control header value. The semicolon should only be used as a separator between directives, not as a terminator.

Suggested change
("Cache-Control", Printf.sprintf "public, max-age=%d;" max_age);
("Cache-Control", Printf.sprintf "public, max-age=%d" max_age);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

@mtelvers mtelvers merged commit 7cb6b9f into ocurrent:master Oct 29, 2025
1 check passed
@mtelvers mtelvers deleted the html-header branch October 29, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants