Merge branch 'main' into feature/te-ELID-466-further-cicd-enhancements

TOEL2 · web-flow · commit 4ef1cf7aa313 · 2025-11-11T09:44:38.000Z
diff --git a/.github/actions/owasp-dependency-scan/action.yaml b/.github/actions/owasp-dependency-scan/action.yaml
@@ -0,0 +1,20 @@
+name: "OWASP Dependency Scan"
+description: "Scan dependencies for known vulnerabilities using OWASP Dependency-Check"
+runs:
+  using: "composite"
+  steps:
+    - name: "Run OWASP Dependency-Check"
+      uses: dependency-check/Dependency-Check_Action@main
+      id: Depcheck
+      with:
+        project: "eligibility-signposting-api"
+        path: "."
+        format: "SARIF"
+        out: "reports"
+        args: >
+          --failOnCVSS 7
+          --enableRetired
+    - name: "Upload OWASP results to GitHub Security tab"
+      uses: github/codeql-action/upload-sarif@v3
+      with:
+        sarif_file: reports/dependency-check-report.sarif
diff --git a/.github/workflows/stage-1-commit.yaml b/.github/workflows/stage-1-commit.yaml
@@ -89,6 +89,9 @@ jobs:
   checkov-terraform:
     name: "Checkov Terraform"
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      security-events: write
     timeout-minutes: 3
     steps:
       - name: "Checkout code"
@@ -100,11 +103,10 @@ jobs:
           soft_fail: false
           output_format: sarif
           output_file_path: checkov-report.sarif
-      - name: Upload Checkov results to GitHub Security tab
-        uses: actions/upload-artifact@v5
+      - name: "Upload Checkov results to GitHub Security tab"
+        uses: github/codeql-action/upload-sarif@v3
         with:
-          name: checkov_results
-          path: checkov-report.sarif
+          sarif_file: checkov-report.sarif
   count-lines-of-code:
     name: "Count lines of code"
     runs-on: ubuntu-latest
@@ -143,3 +145,15 @@ jobs:
           idp_aws_report_upload_region: "${{ secrets.IDP_AWS_REPORT_UPLOAD_REGION }}"
           idp_aws_report_upload_role_name: "${{ secrets.IDP_AWS_REPORT_UPLOAD_ROLE_NAME }}"
           idp_aws_report_upload_bucket_endpoint: "${{ secrets.IDP_AWS_REPORT_UPLOAD_BUCKET_ENDPOINT }}"
+  owasp-dependency-scan:
+    name: "OWASP Dependency Scan"
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      security-events: write
+    timeout-minutes: 5
+    steps:
+      - name: "Checkout code"
+        uses: actions/checkout@v5
+      - name: "Run OWASP Dependency Scan"
+        uses: ./.github/actions/owasp-dependency-scan
diff --git a/docs/security-headers.md b/docs/security-headers.md
@@ -0,0 +1,48 @@
+# Security Headers Implementation
+
+This document describes the implementation of security headers for the Eligibility Signposting API.
+
+## Overview
+
+Security headers have been implemented using a two-layer approach:
+
+1. **API Gateway Responses** (Terraform) - For error responses (4xx, 5xx) generated by API Gateway
+2. **Flask middleware** (Python) - For all successful responses (200, 201, etc.) from the Lambda function
+
+This ensures security headers are present on **all** responses, regardless of where they originate.
+
+## Security Headers
+
+The following security headers are applied to all responses:
+
+| Header | Value | Purpose |
+|--------|-------|---------|
+| `Cache-Control` | `no-store, private` | Prevents caching of sensitive data |
+| `Strict-Transport-Security` | `max-age=31536000; includeSubDomains` | Enforces HTTPS for 1 year on all subdomains |
+| `X-Content-Type-Options` | `nosniff` | Prevents MIME type sniffing |
+
+## Implementation Details
+
+### 1. API Gateway Responses (Terraform)
+
+File: `infrastructure/stacks/api-layer/gateway_responses.tf`
+
+Gateway responses handle error responses generated by API Gateway itself (e.g., validation failures, authorization errors, throttling).
+
+The following response types have been configured:
+
+- `DEFAULT_4XX` - All 4xx errors
+- `DEFAULT_5XX` - All 5xx errors
+- `UNAUTHORIZED` (401)
+- `ACCESS_DENIED` (403)
+- `THROTTLED` (429)
+
+### 2. Flask middleware
+
+Files:
+
+- `src/eligibility_signposting_api/middleware/security_headers.py` - middleware implementation
+- `src/eligibility_signposting_api/middleware/__init__.py` - Package exports
+- `src/eligibility_signposting_api/app.py` - middleware registration
+
+The middleware uses Flask's `after_request` hook to add security headers to all responses from the Lambda function. It's non-overriding: Allows specific endpoints to override headers if needed
diff --git a/infrastructure/stacks/api-layer/gateway_responses.tf b/infrastructure/stacks/api-layer/gateway_responses.tf
@@ -0,0 +1,61 @@
+# Gateway Responses with Security Headers
+# These responses are used when API Gateway itself returns an error (e.g., validation failures, auth errors)
+# They ensure security headers are present even on API Gateway-generated error responses
+
+resource "aws_api_gateway_gateway_response" "response_4xx" {
+  rest_api_id   = module.eligibility_signposting_api_gateway.rest_api_id
+  response_type = "DEFAULT_4XX"
+
+  response_parameters = {
+    "gatewayresponse.header.Cache-Control"             = "'no-store, private'"
+    "gatewayresponse.header.Strict-Transport-Security" = "'max-age=31536000; includeSubDomains'"
+    "gatewayresponse.header.X-Content-Type-Options"    = "'nosniff'"
+  }
+}
+
+resource "aws_api_gateway_gateway_response" "response_5xx" {
+  rest_api_id   = module.eligibility_signposting_api_gateway.rest_api_id
+  response_type = "DEFAULT_5XX"
+
+  response_parameters = {
+    "gatewayresponse.header.Cache-Control"             = "'no-store, private'"
+    "gatewayresponse.header.Strict-Transport-Security" = "'max-age=31536000; includeSubDomains'"
+    "gatewayresponse.header.X-Content-Type-Options"    = "'nosniff'"
+  }
+}
+
+resource "aws_api_gateway_gateway_response" "unauthorized" {
+  rest_api_id   = module.eligibility_signposting_api_gateway.rest_api_id
+  response_type = "UNAUTHORIZED"
+  status_code   = "401"
+
+  response_parameters = {
+    "gatewayresponse.header.Cache-Control"             = "'no-store, private'"
+    "gatewayresponse.header.Strict-Transport-Security" = "'max-age=31536000; includeSubDomains'"
+    "gatewayresponse.header.X-Content-Type-Options"    = "'nosniff'"
+  }
+}
+
+resource "aws_api_gateway_gateway_response" "access_denied" {
+  rest_api_id   = module.eligibility_signposting_api_gateway.rest_api_id
+  response_type = "ACCESS_DENIED"
+  status_code   = "403"
+
+  response_parameters = {
+    "gatewayresponse.header.Cache-Control"             = "'no-store, private'"
+    "gatewayresponse.header.Strict-Transport-Security" = "'max-age=31536000; includeSubDomains'"
+    "gatewayresponse.header.X-Content-Type-Options"    = "'nosniff'"
+  }
+}
+
+resource "aws_api_gateway_gateway_response" "throttled" {
+  rest_api_id   = module.eligibility_signposting_api_gateway.rest_api_id
+  response_type = "THROTTLED"
+  status_code   = "429"
+
+  response_parameters = {
+    "gatewayresponse.header.Cache-Control"             = "'no-store, private'"
+    "gatewayresponse.header.Strict-Transport-Security" = "'max-age=31536000; includeSubDomains'"
+    "gatewayresponse.header.X-Content-Type-Options"    = "'nosniff'"
+  }
+}
diff --git a/infrastructure/stacks/api-layer/patient_check.tf b/infrastructure/stacks/api-layer/patient_check.tf
@@ -112,5 +112,8 @@ resource "aws_api_gateway_gateway_response" "bad_request_parameters" {
   response_parameters = {
     "gatewayresponse.header.Access-Control-Allow-Origin" = "'*'"
     "gatewayresponse.header.Content-Type"                = "'application/fhir+json'"
+    "gatewayresponse.header.Cache-Control"               = "'no-store, private'"
+    "gatewayresponse.header.Strict-Transport-Security"   = "'max-age=31536000; includeSubDomains'"
+    "gatewayresponse.header.X-Content-Type-Options"      = "'nosniff'"
   }
 }
diff --git a/scripts/config/vale/styles/config/vocabularies/words/accept.txt b/scripts/config/vale/styles/config/vocabularies/words/accept.txt
@@ -8,6 +8,8 @@ Gitleaks
 Grype
 idempotence
 Makefile
+Middleware
+middleware
 OAuth
 Octokit
 onboarding
diff --git a/src/eligibility_signposting_api/app.py b/src/eligibility_signposting_api/app.py
@@ -17,6 +17,7 @@
 from eligibility_signposting_api.logging.logs_helper import log_request_ids_from_headers
 from eligibility_signposting_api.logging.logs_manager import add_lambda_request_id_to_logger, init_logging
 from eligibility_signposting_api.logging.tracing_helper import tracing_setup
+from eligibility_signposting_api.middleware import SecurityHeadersMiddleware
 from eligibility_signposting_api.views import eligibility_blueprint
 
 if os.getenv("ENABLE_XRAY_PATCHING"):
@@ -62,6 +63,9 @@ def create_app() -> Flask:
     app = Flask(__name__)
     logger.info("app created")
 
+    # Register security headers middleware
+    SecurityHeadersMiddleware(app)
+
     # Register views & error handler
     app.register_blueprint(eligibility_blueprint, url_prefix=f"/{URL_PREFIX}")
     app.register_error_handler(Exception, handle_exception)
diff --git a/src/eligibility_signposting_api/middleware/__init__.py b/src/eligibility_signposting_api/middleware/__init__.py
@@ -0,0 +1,5 @@
+"""Middleware package for the Eligibility Signposting API."""
+
+from eligibility_signposting_api.middleware.security_headers import SecurityHeadersMiddleware
+
+__all__ = ["SecurityHeadersMiddleware"]
diff --git a/src/eligibility_signposting_api/middleware/security_headers.py b/src/eligibility_signposting_api/middleware/security_headers.py
@@ -0,0 +1,60 @@
+"""Security headers middleware for Flask application.
+
+This middleware adds security headers to all responses from the Lambda function.
+These headers are applied to successful responses (200, 201, etc.) from the application.
+
+For API Gateway error responses (4xx, 5xx generated by API Gateway itself),
+see gateway_responses.tf in the infrastructure configuration.
+"""
+
+from typing import ClassVar
+
+from flask import Flask, Response
+
+
+class SecurityHeadersMiddleware:
+    """Middleware to add security headers to all Flask responses."""
+
+    # Security headers recommended for NHS APIs
+    SECURITY_HEADERS: ClassVar[dict[str, str]] = {
+        "Cache-Control": "no-store, private",
+        "Strict-Transport-Security": "max-age=31536000; includeSubDomains",
+        "X-Content-Type-Options": "nosniff",
+    }
+
+    def __init__(self, app: Flask | None = None) -> None:
+        """Initialize the middleware.
+
+        Args:
+            app: Flask application instance. Can be provided later via init_app()
+        """
+        if app is not None:
+            self.init_app(app)
+
+    def init_app(self, app: Flask) -> None:
+        """Initialize the middleware with a Flask application.
+
+        Args:
+            app: Flask application instance to apply middleware to
+        """
+        app.after_request(self.add_security_headers)
+
+    @classmethod
+    def add_security_headers(cls, response: Response) -> Response:
+        """Add security headers to the Flask response.
+
+        This is called automatically by Flask after each request.
+
+        Args:
+            response: Flask response object
+
+        Returns:
+            Modified response object with security headers added
+        """
+        for header, value in cls.SECURITY_HEADERS.items():
+            # Only add header if it doesn't already exist
+            # This allows specific endpoints to override if needed
+            if header not in response.headers:
+                response.headers[header] = value
+
+        return response
diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py
@@ -138,12 +138,16 @@ def normalize_virtual(cls, value: str) -> Virtual:
 class IterationRule(BaseModel):
     type: RuleType = Field(..., alias="Type")
     name: RuleName = Field(..., alias="Name")
-    code: RuleCode | None = Field(None, alias="Code", description="use the `rule_code` property instead.")
-    description: RuleDescription = Field(..., alias="Description", description="use the `rule_text` property instead.")
+    code: RuleCode | None = Field(None, alias="Code", description="use `rule_code` property instead.")
+    description: RuleDescription = Field(..., alias="Description", description="use `rule_text` property instead.")
     priority: RulePriority = Field(..., alias="Priority")
     attribute_level: RuleAttributeLevel = Field(..., alias="AttributeLevel")
     attribute_name: RuleAttributeName | None = Field(None, alias="AttributeName")
-    cohort_label: CohortLabel | None = Field(None, alias="CohortLabel")
+    cohort_label: CohortLabel | None = Field(
+        None,
+        alias="CohortLabel",
+        description="Raw label input. Prefer using `parsed_cohort_labels` for normalized access.",
+    )
     operator: RuleOperator = Field(..., alias="Operator")
     comparator: RuleComparator = Field(..., alias="Comparator")
     attribute_target: RuleAttributeTarget | None = Field(None, alias="AttributeTarget")
@@ -197,6 +201,18 @@ def rule_text(self) -> str:
                     rule_text = rule_entry.rule_text
         return rule_text or self.description
 
+    @cached_property
+    def parsed_cohort_labels(self) -> list[str]:
+        """
+        Parses the cohort_label string into a list of individual labels.
+
+        Returns:
+            A list of cohort labels, split by comma. If no label is set, returns an empty list.
+        """
+        if not self.cohort_label:
+            return []
+        return [label.strip() for label in self.cohort_label.split(",") if label.strip()]
+
     def __str__(self) -> str:
         return json.dumps(self.model_dump(by_alias=True), indent=2)
 
diff --git a/src/eligibility_signposting_api/services/processors/rule_processor.py b/src/eligibility_signposting_api/services/processors/rule_processor.py
@@ -123,8 +123,10 @@ def is_actionable(
 
     @staticmethod
     def _should_skip_rule_group(cohort: IterationCohort, group_rules: list[IterationRule]) -> bool:
-        cohort_specific_rules = [rule for rule in group_rules if rule.cohort_label is not None]
-        matching_specific_rules = [rule for rule in cohort_specific_rules if rule.cohort_label == cohort.cohort_label]
+        cohort_specific_rules = [rule for rule in group_rules if rule.parsed_cohort_labels]
+        matching_specific_rules = [
+            rule for rule in cohort_specific_rules if cohort.cohort_label in rule.parsed_cohort_labels
+        ]
         return bool(cohort_specific_rules and not matching_specific_rules)
 
     def evaluate_rules_priority_group(
@@ -148,13 +150,7 @@ def evaluate_rules_priority_group(
 
     @staticmethod
     def get_exclusion_rules(cohort: IterationCohort, rules: Iterable[IterationRule]) -> Iterator[IterationRule]:
-        return (
-            ir
-            for ir in rules
-            if ir.cohort_label is None
-            or cohort.cohort_label == ir.cohort_label
-            or (isinstance(ir.cohort_label, (list, set, tuple)) and cohort.cohort_label in ir.cohort_label)
-        )
+        return (ir for ir in rules if not ir.parsed_cohort_labels or cohort.cohort_label in ir.parsed_cohort_labels)
 
     def get_cohort_group_results(
         self, person: Person, active_iteration: Iteration
diff --git a/tests/unit/middleware/__init__.py b/tests/unit/middleware/__init__.py
@@ -0,0 +1 @@
+"""Tests for middleware components."""
diff --git a/tests/unit/middleware/test_security_headers.py b/tests/unit/middleware/test_security_headers.py
diff --git a/tests/unit/services/calculators/test_eligibility_calculator.py b/tests/unit/services/calculators/test_eligibility_calculator.py
diff --git a/tests/unit/services/processors/test_cohort_handler.py b/tests/unit/services/processors/test_cohort_handler.py
diff --git a/tests/unit/services/processors/test_rule_processor.py b/tests/unit/services/processors/test_rule_processor.py
diff --git a/tests/unit/validation/test_iteration_rules_validator.py b/tests/unit/validation/test_iteration_rules_validator.py
diff --git a/tests/unit/views/test_eligibility.py b/tests/unit/views/test_eligibility.py

Original file line number	Diff line number	Diff line change
`@@ -112,5 +112,8 @@ resource "aws_api_gateway_gateway_response" "bad_request_parameters" {`
`112`	`112`	`response_parameters = {`
`113`	`113`	`"gatewayresponse.header.Access-Control-Allow-Origin" = "'*'"`
`114`	`114`	`"gatewayresponse.header.Content-Type" = "'application/fhir+json'"`
	`115`	`+ "gatewayresponse.header.Cache-Control" = "'no-store, private'"`
	`116`	`+ "gatewayresponse.header.Strict-Transport-Security" = "'max-age=31536000; includeSubDomains'"`
	`117`	`+ "gatewayresponse.header.X-Content-Type-Options" = "'nosniff'"`
`115`	`118`	`}`
`116`	`119`	`}`