Skip to content

Implement security hardening with CSP and localized assets #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Carl Sixsmith (carlsixsmith-moj) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement security hardening with CSP and localized assets #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Visualiser/Pages/Index.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,6 @@
@Html.AntiForgeryToken()

@section Scripts {
<script type="text/javascript" src="https://unpkg.com/vis-network/standalone/umd/vis-network.min.js"></script>
<script type="text/javascript" src="~/js/vis-network.min.js"></script>
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

This script is now served from a local static file but doesn't use cache-busting (asp-append-version) like the other local assets in this project. Consider adding asp-append-version="true" to avoid clients being stuck with an old vis-network.min.js after deployments.

Suggested change
<script type="text/javascript" src="~/js/vis-network.min.js"></script>
<script type="text/javascript" src="~/js/vis-network.min.js" asp-append-version="true"></script>

Copilot uses AI. Check for mistakes.
<script type="text/javascript" src="~/js/network.js" asp-append-version="true"></script>
}
2 changes: 1 addition & 1 deletion src/Visualiser/Pages/Index.cshtml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
using System.Text;
using System.Text.Json;
namespace Visualiser.Pages;

[ValidateAntiForgeryToken]
public class IndexModel(IDownstreamApi api) : PageModel
{
public void OnGet() { }
Expand Down
1 change: 1 addition & 0 deletions src/Visualiser/Pages/ProcessedFiles.cshtml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

namespace Visualiser.Pages
{
[ValidateAntiForgeryToken]
public class ProcessedFilesModel(IDownstreamApi api) : PageModel
{

Expand Down
17 changes: 17 additions & 0 deletions src/Visualiser/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,23 @@
app.UseAuthentication();
app.UseAuthorization();

app.Use((context, next) =>
{
context.Request.Scheme = "https";
return next();
});
Comment on lines +59 to +63
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Setting context.Request.Scheme = "https" does not enforce HTTPS (it doesn't redirect) and can also override the actual scheme determined by UseForwardedHeaders/UseHttpsRedirection, potentially leading to incorrect URL generation and masking HTTP traffic. Remove this middleware and rely on app.UseHttpsRedirection() (already present) plus correctly configured forwarded headers / HSTS for enforcement behind a proxy.

Copilot uses AI. Check for mistakes.

app.Use(async (context, next) =>
{
if(context.Response.Headers.IsReadOnly == false)
{
string csp = app.Configuration["Content-Security-Policy"]
?? "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'self' data:; frame-src 'self' data:;";
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The default CSP includes 'unsafe-inline' for script-src and style-src, which substantially weakens XSS protection. Prefer removing 'unsafe-inline' and using nonces/hashes (or moving inline code to external files) so the CSP actually blocks injected inline script/style.

Suggested change
?? "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'self' data:; frame-src 'self' data:;";
?? "default-src 'self'; script-src 'self'; style-src 'self'; img-src 'self' data:; font-src 'self'; object-src 'self' data:; frame-src 'self' data:;";

Copilot uses AI. Check for mistakes.
context.Response.Headers.Append("Content-Security-Policy", csp);
}
Comment on lines +69 to +72
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

Using Response.Headers.Append("Content-Security-Policy", ...) can result in multiple CSP headers if something upstream/downstream also sets CSP, and browsers will apply the intersection which can be confusing to debug. Prefer setting the header value (e.g., Headers["Content-Security-Policy"] = ...) or only add it when it's not already present.

Copilot uses AI. Check for mistakes.
await next();
});

app.MapStaticAssets();
app.MapRazorPages()
.WithStaticAssets();
Expand Down
34 changes: 34 additions & 0 deletions src/Visualiser/wwwroot/js/vis-network.min.js
View file
Open in desktop

Large diffs are not rendered by default.

Loading