Fix various issues reported by SonarCloud (#268)

* Fix various issues reported by sonarcloud

Dockerfiles:
- Run containers as non-root user

Python:
- Add CSRF protection
- Split safe and unsafe routes
- Don't run in debug mode

JavaScript:
- Don't use asynchronous constructor side-effects
- Add integrity check to external resources

#patch

* Run fixtures on non-privileged port

But it will still be providing 443 at the load balancer.

#patch

fixtures/Dockerfile

@@ -4,7 +4,7 @@ WORKDIR /app
 
 COPY fixtures/package.json package.json
 COPY fixtures/package-lock.json package-lock.json
-RUN npm ci
+RUN npm ci --ignore-scripts
 
 FROM python:3-alpine3.17
 
@@ -24,6 +24,9 @@ COPY fixtures/static static
 COPY fixtures/templates templates
 COPY docs/schemas static/schemas
 
-EXPOSE 80
+EXPOSE 8080
 
-CMD [ "flask", "run", "--host", "0.0.0.0", "--port", "80"]
+RUN addgroup -S app && adduser -S -g app app
+USER app
+
+CMD [ "flask", "run", "--host", "0.0.0.0", "--port", "8080"]

fixtures/app.py

@@ -1,10 +1,18 @@
-import requests, os, logging, sys, json
+import requests, os, logging, sys, json, uuid
 from lib.aws_auth import AwsAuth
 from lib.jwt import generate_jwt
+from urllib.parse import quote
 
 from flask import Flask, render_template, request, jsonify
+from flask_wtf import CSRFProtect
 
 app = Flask(__name__, static_url_path="/assets")
+app.config.update(
+    SECRET_KEY=uuid.uuid4().__str__(),
+)
+
+csrf = CSRFProtect()
+csrf.init_app(app)
 
 logger = logging.getLogger()
 logger.setLevel(logging.DEBUG)
@@ -49,52 +57,57 @@ def health_check_dependencies():
         return jsonify({"ok": False})
 
 
-@app.route("/", methods=["GET", "POST"])
-def index():
-    aws_auth = AwsAuth()
-
-    uid = request.form.get("uid", "")
-    json_data = request.form.get("json-data", "{}")
+@app.route("/", methods=["GET"])
+def get_index():
     base_url = os.environ["BASE_URL"]
 
-    template_data = {
-        "base_url": base_url,
-        "uid": uid,
-        "json_data": json_data,
-    }
-
-    if request.method == "GET":
-        return render_template("index.html", **template_data)
-
-    if request.method == "POST":
-        url = base_url + "/lpas/" + uid
-
-        if aws_auth.is_authed:
-            headers = aws_auth.get_headers(method="PUT", url=url, data=json_data)
-        else:
-            headers = {}
+    return render_template(
+        "index.html",
+        **{
+            "base_url": base_url,
+            "json_data": "{}",
+        },
+    )
 
-        token = generate_jwt(os.environ["JWT_SECRET_KEY"])
 
-        resp = requests.put(
-            url,
-            json_data,
-            headers={
-                **headers,
-                "Content-Type": "application/json",
-                "X-Jwt-Authorization": "Bearer " + token,
-            },
-        )
+@app.route("/", methods=["POST"])
+def post_index():
+    aws_auth = AwsAuth()
 
-        return render_template(
-            "index.html",
-            **template_data,
-            success=resp.status_code < 400,
-            error=json.loads(resp.text),
-        )
+    uid = request.form.get("uid", "")
+    json_data = request.form.get("json-data", "{}")
+    base_url = os.environ["BASE_URL"]
 
-    return "error"
+    url = base_url + "/lpas/" + quote(uid)
+
+    if aws_auth.is_authed:
+        headers = aws_auth.get_headers(method="PUT", url=url, data=json_data)
+    else:
+        headers = {}
+
+    token = generate_jwt(os.environ["JWT_SECRET_KEY"])
+
+    resp = requests.put(
+        url,
+        json_data,
+        headers={
+            **headers,
+            "Content-Type": "application/json",
+            "X-Jwt-Authorization": "Bearer " + token,
+        },
+    )
+
+    return render_template(
+        "index.html",
+        **{
+            "base_url": base_url,
+            "uid": uid,
+            "json_data": json_data,
+        },
+        success=resp.status_code < 400,
+        error=json.loads(resp.text),
+    )
 
 
 if __name__ == "__main__":
-    app.run(debug=True, host="0.0.0.0", port=80)
+    app.run(host="0.0.0.0", port=80)

fixtures/requirements.txt

@@ -1,4 +1,5 @@
 Flask==3.0.3
+Flask-WTF==1.2.1
 requests==2.32.3
 Jinja2==3.1.4
 jsonschema==4.23.0

fixtures/static/js/json-schema-editor.mjs

@@ -36,8 +36,6 @@ export class JsonSchemaEditor {
     }
 
     this.$module = $module;
-
-    this.init();
   }
 
   async init() {
@@ -319,4 +317,14 @@ export class JsonSchemaEditor {
 
     return $container;
   }
+
+  /**
+   * @param {Element} $module
+   */
+  static async create($module) {
+    const $editor = new JsonSchemaEditor($module);
+    $editor.init();
+
+    return $editor;
+  }
 }

fixtures/static/js/main.mjs

@@ -10,6 +10,6 @@ export function initAll() {
   Object.entries(initiators).forEach(([name, Component]) => {
     const $elements = document.querySelectorAll(`[data-module="${name}"]`);
 
-    $elements.forEach(($element) => new Component($element));
+    $elements.forEach(($element) => Component.create($element));
   });
 }

fixtures/static/js/uid-generator.mjs

@@ -52,4 +52,11 @@ export class UidGenerator {
       `M-` +
       [value.slice(0, 4), value.slice(4, 8), value.slice(8, 12)].join("-");
   }
+
+  /**
+   * @param {Element} $module
+   */
+  static async create($module) {
+    return new UidGenerator($module);
+  }
 }

fixtures/templates/index.html

@@ -63,6 +63,8 @@ <h2 class="govuk-error-summary__title">{{ error.detail }}</h2>
         {% endif %} {% endif %}
 
         <form method="post">
+          <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
+
           <div class="govuk-form-group">
             <label class="govuk-label govuk-label--m" for="f-uid">UID</label>
             <input
@@ -100,7 +102,11 @@ <h2 class="govuk-error-summary__title">{{ error.detail }}</h2>
       </main>
     </div>
 
-    <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/jsonSchemaLibrary.min.js"></script>
+    <script
+      src="https://cdn.jsdelivr.net/npm/[email protected]/dist/jsonSchemaLibrary.min.js"
+      integrity="sha384-RePbUf/gtYzyS1nEErY0oNbL9zMtjU3TXf4Dj4FBEmhclyRv8pPvibkBEMUpd/c3"
+      crossorigin="anonymous"
+    ></script>
     <script type="module">
       import { initAll as govukInitAll } from "./assets/govuk-frontend.min.js";
       import { initAll as appInitAll } from "./assets/js/main.mjs";

lambda/Dockerfile

@@ -19,4 +19,7 @@ RUN apk upgrade libssl3 libcrypto3
 
 COPY --from=build-env /go/bin/main /var/task/main
 
+RUN addgroup -S app && adduser -S -g app app
+USER app
+
 ENTRYPOINT [ "/var/task/main" ]

mock-apigw/Dockerfile

@@ -15,4 +15,7 @@ FROM alpine:3
 
 COPY --from=build-env /go/bin/main /var/task/main
 
+RUN addgroup -S app && adduser -S -g app app
+USER app
+
 ENTRYPOINT [ "/var/task/main" ]

sonar-project.properties

@@ -0,0 +1 @@
+sonar.exclusions=**/mock_*_test.go

terraform/environment/region/fixtures.tf

@@ -4,6 +4,7 @@ module "fixtures" {
 
   application_subnet_ids = data.aws_subnets.application.ids
   cloudwatch_kms_key_id  = aws_kms_key.cloudwatch.arn
+  container_port         = 8080
   ecr_image_uri          = "${data.aws_ecr_repository.fixtures[0].repository_url}:${var.app_version}"
   environment_name       = var.environment_name
   public_subnet_ids      = data.aws_subnets.public.ids

terraform/modules/fixtures_service/ecs.tf

@@ -19,7 +19,7 @@ resource "aws_ecs_service" "fixtures" {
   load_balancer {
     target_group_arn = aws_lb_target_group.fixtures.arn
     container_name   = "fixtures"
-    container_port   = 80
+    container_port   = var.container_port
   }
 
   network_configuration {
@@ -60,8 +60,8 @@ locals {
       name                   = "fixtures",
       portMappings = [
         {
-          containerPort = 80,
-          hostPort      = 80,
+          containerPort = var.container_port,
+          hostPort      = var.container_port,
           protocol      = "tcp"
         }
       ],

terraform/modules/fixtures_service/load_balancer.tf

@@ -57,7 +57,7 @@ resource "aws_lb_listener_rule" "fixtures" {
 
 resource "aws_lb_target_group" "fixtures" {
   name                 = "fixtures-${var.environment_name}-http"
-  port                 = 80
+  port                 = var.container_port
   protocol             = "HTTP"
   target_type          = "ip"
   vpc_id               = var.vpc_id

terraform/modules/fixtures_service/load_balancer_security.tf

@@ -39,8 +39,8 @@ resource "aws_security_group" "loadbalancer_gov_wifi" {
 resource "aws_security_group_rule" "loadbalancer_egress_to_ecs" {
   type                     = "egress"
   protocol                 = "tcp"
-  from_port                = 80
-  to_port                  = 80
+  from_port                = var.container_port
+  to_port                  = var.container_port
   security_group_id        = aws_security_group.loadbalancer_gov_wifi.id
   source_security_group_id = aws_security_group.ecs.id
 

terraform/modules/fixtures_service/security_group.tf

@@ -8,8 +8,8 @@ resource "aws_security_group" "ecs" {
 resource "aws_security_group_rule" "alb_ingress" {
   type                     = "ingress"
   protocol                 = "tcp"
-  from_port                = 80
-  to_port                  = 80
+  from_port                = var.container_port
+  to_port                  = var.container_port
   source_security_group_id = aws_security_group.loadbalancer_gov_wifi.id
   security_group_id        = aws_security_group.ecs.id
   description              = "Inbound from the ALB"

terraform/modules/fixtures_service/variables.tf

@@ -3,21 +3,26 @@ variable "application_subnet_ids" {
   type        = list(string)
 }
 
-variable "environment_name" {
-  description = "The name of the environment the fixtures container is deployed to"
-  type        = string
-}
-
 variable "cloudwatch_kms_key_id" {
   description = "KMS key used to encrypt CloudWatch logs"
   type        = string
 }
 
+variable "container_port" {
+  description = "Port exposed by container"
+  type        = number
+}
+
 variable "ecr_image_uri" {
   description = "The URI of the image the container should use"
   type        = string
 }
 
+variable "environment_name" {
+  description = "The name of the environment the fixtures container is deployed to"
+  type        = string
+}
+
 variable "public_subnet_ids" {
   description = "Public subnet IDs in VPC"
   type        = list(string)