From 0fac9c3ed4305c61350462c890c04a8f7b7adba8 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Wed, 3 Sep 2025 15:02:00 +0100 Subject: [PATCH 01/18] CCM-6437: Fix Sonar security issues --- .github/workflows/pr-lint.yaml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-lint.yaml b/.github/workflows/pr-lint.yaml index e0cff6e65..1eebce767 100644 --- a/.github/workflows/pr-lint.yaml +++ b/.github/workflows/pr-lint.yaml @@ -5,15 +5,19 @@ jobs: runs-on: ubuntu-latest steps: - name: Check ticket name conforms to requirements - run: echo ${{ github.event.pull_request.head.ref }} | grep -i -E -q "(ccm-[0-9]+)|(dependabot\/)" + env: + BRANCH_REF: ${{ github.event.pull_request.head.ref }} + run: echo "$BRANCH_REF" | grep -i -E -q "(ccm-[0-9]+)|(dependabot\/)" continue-on-error: true - name: Grab ticket name if: contains(github.event.pull_request.head.ref, 'ccm-') || contains(github.event.pull_request.head.ref, 'CCM-') - run: echo ::set-env name=TICKET_NAME::$(echo ${{ github.event.pull_request.head.ref }} | grep -i -o '\(ccm-[0-9]\+\)' | tr '[:lower:]' '[:upper:]') - continue-on-error: true env: - ACTIONS_ALLOW_UNSECURE_COMMANDS: true + BRANCH_REF: ${{ github.event.pull_request.head.ref }} + run: | + TICKET=$(echo "$BRANCH_REF" | grep -i -o '\(ccm-[0-9]\+\)' | tr '[:lower:]' '[:upper:]') + echo "TICKET_NAME=$TICKET" >> $GITHUB_ENV + continue-on-error: true - name: Comment on PR with link to JIRA ticket if: contains(github.event.pull_request.head.ref, 'ccm-') || contains(github.event.pull_request.head.ref, 'CCM-') From 6e7f905e2e6d5fd68adeda624305b2040e0e4fef Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Wed, 3 Sep 2025 16:52:18 +0100 Subject: [PATCH 02/18] CCM-6437: Fix Sonar consistency issues --- nhsd-git-secrets/nhsd-git-secrets.dockerfile | 51 +++++++++----------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/nhsd-git-secrets/nhsd-git-secrets.dockerfile b/nhsd-git-secrets/nhsd-git-secrets.dockerfile index 636afdd60..44a14a69b 100644 --- a/nhsd-git-secrets/nhsd-git-secrets.dockerfile +++ b/nhsd-git-secrets/nhsd-git-secrets.dockerfile @@ -19,43 +19,38 @@ FROM ubuntu:24.10 -RUN echo "Installing required modules" -RUN apt-get update \ - && apt-get -y install curl git build-essential \ - && apt-get clean +RUN echo "Installing required modules" \ + && apt-get update \ + && apt-get -y install build-essential curl git \ + && apt-get clean \ + && echo "Copying source files" # By default, we copy the entire project into the dockerfile for secret scanning # Tweak that COPY if you only want some of the source -RUN echo "Copying source files" WORKDIR /secrets-scanner COPY . source -RUN ls -l source - -RUN echo "Downloading secrets scanner" -RUN curl https://codeload.github.com/awslabs/git-secrets/tar.gz/master | tar -xz --strip=1 git-secrets-master - -RUN echo "Installing secrets scanner" -RUN make install +RUN ls -l source \ + && echo "Downloading secrets scanner" \ + && curl https://codeload.github.com/awslabs/git-secrets/tar.gz/master | tar -xz --strip=1 git-secrets-master \ + && RUN echo "Installing secrets scanner" \ + && RUN make install \ + && echo "Configuring git" # even though running secrets scanner on a folder, must still be in some kind of git repo # for the git-secrets config to attach to something # so init an empty git repo here -RUN echo "Configuring git" WORKDIR /secrets-scanner/source -RUN git init - -RUN echo "Downloading regex files from engineering-framework" -RUN curl https://codeload.github.com/NHSDigital/software-engineering-quality-framework/tar.gz/main | tar -xz --strip=3 software-engineering-quality-framework-main/tools/nhsd-git-secrets/nhsd-rules-deny.txt +RUN git init \ + && echo "Downloading regex files from engineering-framework" \ + && curl https://codeload.github.com/NHSDigital/software-engineering-quality-framework/tar.gz/main | tar -xz --strip=3 software-engineering-quality-framework-main/tools/nhsd-git-secrets/nhsd-rules-deny.txt \ + && echo "Copying allowed secrets list" -RUN echo "Copying allowed secrets list" COPY .gitallowed . -RUN echo .gitallowed - -# Register additional providers: adds AWS by default -RUN echo "Configuring secrets scanner" -RUN /secrets-scanner/git-secrets --register-aws -RUN /secrets-scanner/git-secrets --add-provider -- cat nhsd-rules-deny.txt - -# build will fail here, if secrets are found -RUN echo "Running scan..." -RUN /secrets-scanner/git-secrets --scan -r . +RUN echo .gitallowed \ + # Register additional providers: adds AWS by default + && echo "Configuring secrets scanner" \ + && /secrets-scanner/git-secrets --register-aws \ + && /secrets-scanner/git-secrets --add-provider -- cat nhsd-rules-deny.txt \ + # build will fail here, if secrets are found + && echo "Running scan..." \ + && /secrets-scanner/git-secrets --scan -r . From 6b26589e0c207f4c20ee8ceba2cb950c0b867d4e Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 8 Sep 2025 08:59:11 +0100 Subject: [PATCH 03/18] CCM-6437: Fix various SonarCloud issues and code smells --- .../jsc/MessageBatches.Create.Validate.js | 2 +- sandbox/handlers/batch_send.js | 2 +- scripts/publish_zap_compatible.py | 35 +- tests/api/message_batches/test_too_large.py | 4 +- tests/api/rate_limiting/test_429_errors.py | 402 +++++++++--------- tests/lib/fixtures.py | 13 +- tests/lib/generators.py | 17 +- tests/mtls/test_mtls.py | 135 +++--- .../create_message_batches/test_too_large.py | 2 +- zap/Dockerfile | 8 +- 10 files changed, 310 insertions(+), 310 deletions(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 31237d7cf..12ce9c8c8 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -54,7 +54,7 @@ const validate = () => { errors.push(invalidError(pointer)); } else { - var pointer = "/data/attributes/messages/" + index + "/messageReference"; + pointer = "/data/attributes/messages/" + index + "/messageReference"; const validMessageReference = validateString(errors, message.messageReference, pointer) if (validMessageReference) { if (seenMessages[message.messageReference]) { diff --git a/sandbox/handlers/batch_send.js b/sandbox/handlers/batch_send.js index ea85738d0..255fd1e46 100644 --- a/sandbox/handlers/batch_send.js +++ b/sandbox/handlers/batch_send.js @@ -46,7 +46,7 @@ export async function batchSend(req, res, next) { } const odsCodes = messages.map( - (message) => message && message.originator?.odsCode + (message) => message?.originator?.odsCode ); for (const odsCode of odsCodes) { const odsCodeError = getOdsCodeError(odsCode, req.headers.authorization); diff --git a/scripts/publish_zap_compatible.py b/scripts/publish_zap_compatible.py index a5ea34ad3..f9bd467da 100644 --- a/scripts/publish_zap_compatible.py +++ b/scripts/publish_zap_compatible.py @@ -1,25 +1,32 @@ import json +def clean_dict(obj, mappings): + new_obj = {} + for k, v in obj.items(): + should_remove = False + for m in mappings: + if k == m[0] and (m[1] is None or v == m[1]): + should_remove = True + if not should_remove: + new_obj[k] = scan_and_remove(v, mappings) + return new_obj + + +def clean_list(obj, mappings): + new_list = [] + for i in obj: + new_list.append(scan_and_remove(i, mappings)) + return new_list + + def scan_and_remove(obj, mappings): t = type(obj) if t is dict: - new_obj = {} - for k, v in obj.items(): - found = False - for m in mappings: - if k == m[0] and (m[1] is None or v == m[1]): - found = True - if not found: - new_obj[k] = scan_and_remove(v, mappings) - return new_obj + return clean_dict(obj, mappings) elif t is list: - new_list = [] - for i in obj: - new_list.append(scan_and_remove(i, mappings)) - return new_list - + return clean_list(obj, mappings) return obj diff --git a/tests/api/message_batches/test_too_large.py b/tests/api/message_batches/test_too_large.py index 0e3d03ddd..4948f2ece 100644 --- a/tests/api/message_batches/test_too_large.py +++ b/tests/api/message_batches/test_too_large.py @@ -21,7 +21,7 @@ def test_too_many_messages(url, bearer_token): messages = [] data["data"]["attributes"]["messages"] = messages - for i in range(MESSAGE_LIMIT+1): + for _ in range(MESSAGE_LIMIT+1): messages.append({ "messageReference": str(uuid.uuid1()), "recipient": { @@ -59,7 +59,7 @@ def test_payload_too_large(url, bearer_token): messages = [] data["data"]["attributes"]["messages"] = messages - for i in range(MESSAGE_LIMIT): + for _ in range(MESSAGE_LIMIT): messages.append({ "messageReference": str(uuid.uuid1()), "recipient": valid_recipient, diff --git a/tests/api/rate_limiting/test_429_errors.py b/tests/api/rate_limiting/test_429_errors.py index 5074558c5..674313e33 100644 --- a/tests/api/rate_limiting/test_429_errors.py +++ b/tests/api/rate_limiting/test_429_errors.py @@ -1,201 +1,201 @@ -import requests -import pytest -from lib import Assertions, Generators -from lib.fixtures import * # NOSONAR -from lib.constants.messages_paths import MESSAGES_ENDPOINT -import time - -test_client_1_details = { - "email": "ian.hodges1@nhs.net", - "name": "NHS Notify Test Client 1" -} - - -def default_request_url(url): - return f"{url}{MESSAGES_ENDPOINT}/pending_enrichment_request_item_id" - - -def send_multiple_requests(url, headers, count=10, delay=0): - for i in range(count): - resp = requests.get(url, headers=headers) - if delay > 0: - time.sleep(delay) - return resp - - -@pytest.mark.devperftest -def test_429_triggered_app_quota(url, bearer_token, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_429_global_app_quota.rst - """ - rate_limiting.set_rate_limit(app_quota=4) - headers = Generators.generate_valid_headers(bearer_token.value) - resp = send_multiple_requests(default_request_url(url), headers) - - Assertions.assert_error_with_optional_correlation_id( - resp, - 429, - Generators.generate_quota_error_custom( - "Your application, Comms-manager-local, " - "has exceeded its quota of 4 requests every 1 minute(s) and is being rate limited."), - None - ) - - assert "Retry-After" in resp.headers - assert resp.headers.get("Retry-After") == "60" - - -@pytest.mark.devperftest -def test_429_triggered_app_spikearrest(url, bearer_token, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_429_global_app_spikearrest.rst - """ - rate_limiting.set_rate_limit(app_spikearrest="4pm") - headers = Generators.generate_valid_headers(bearer_token.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - Assertions.assert_error_with_optional_correlation_id( - resp, - 429, - Generators.generate_quota_error_custom( - "Your application, Comms-manager-local, " - "has created a spike in traffic and is being rate limited. Please reduce the frequency of your requests."), - None - ) - - assert "Retry-After" in resp.headers - assert resp.headers.get("Retry-After") == "1" - - -@pytest.mark.devperftest -def test_429_triggered_proxy_quota(url, bearer_token, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_429_proxy_quota.rst - """ - rate_limiting.set_rate_limit(proxy_quota=4) - headers = Generators.generate_valid_headers(bearer_token.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - Assertions.assert_error_with_optional_correlation_id( - resp, - 429, - Generators.generate_quota_error_custom( - "This API is currently receiving a high volume of requests " - "and is being rate limited."), - None - ) - - assert "Retry-After" in resp.headers - assert resp.headers.get("Retry-After") == "60" - - -@pytest.mark.devperftest -def test_429_triggered_proxy_spikearrest(url, bearer_token, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_429_proxy_spikearrest.rst - """ - rate_limiting.set_rate_limit(proxy_spikearrest="4pm") - headers = Generators.generate_valid_headers(bearer_token.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - Assertions.assert_error_with_optional_correlation_id( - resp, - 429, - Generators.generate_quota_error_custom( - "This API is currently receiving a high volume of requests " - "and is being rate limited."), - None - ) - - assert "Retry-After" in resp.headers - assert resp.headers.get("Retry-After") == "1" - - -@pytest.mark.devperftest -def test_429_triggered_specific_app_quota(url, bearer_token_internal_dev_test_1, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_429_specific_app_quota.rst - """ - rate_limiting.set_default_rate_limit() - rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], quota=4) - headers = Generators.generate_valid_headers(bearer_token_internal_dev_test_1.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - Assertions.assert_error_with_optional_correlation_id( - resp, - 429, - Generators.generate_quota_error_custom( - "Your application, NHS Notify Test Client 1, " - "has exceeded its quota of 4 requests every 1 minute(s) and is being rate limited."), - None - ) - - assert "Retry-After" in resp.headers - assert resp.headers.get("Retry-After") == "60" - - -@pytest.mark.devperftest -def test_429_not_triggered_other_specific_app_quota(url, bearer_token, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_200_specific_app_quota_different_app.rst - """ - rate_limiting.set_default_rate_limit() - rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], quota=4) - headers = Generators.generate_valid_headers(bearer_token.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - assert resp.status_code == 200, f"Response: {resp.status_code}: {resp.text}" - - -@pytest.mark.devperftest -def test_429_triggered_specific_app_spikearrest(url, - bearer_token, - bearer_token_internal_dev_test_1, - rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_429_specific_app_spikearrest.rst - """ - rate_limiting.set_default_rate_limit() - rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], spikearrest="4pm") - headers = Generators.generate_valid_headers(bearer_token_internal_dev_test_1.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - Assertions.assert_error_with_optional_correlation_id( - resp, - 429, - Generators.generate_quota_error_custom( - "Your application, NHS Notify Test Client 1, " - "has created a spike in traffic and is being rate limited. Please reduce the frequency of your requests."), - None - ) - - assert "Retry-After" in resp.headers - assert resp.headers.get("Retry-After") == "1" - - -@pytest.mark.devperftest -def test_429_not_triggered_other_specific_spikearrest(url, bearer_token, rate_limiting): - - """ - .. include:: ../partials/too_many_requests/test_200_specific_app_spikearrest_different_app.rst - """ - rate_limiting.set_default_rate_limit() - rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], spikearrest="4pm") - headers = Generators.generate_valid_headers(bearer_token.value) - - resp = send_multiple_requests(default_request_url(url), headers) - - assert resp.status_code == 200, f"Response: {resp.status_code}: {resp.text}" +import requests +import pytest +from lib import Assertions, Generators +from lib.fixtures import * # NOSONAR +from lib.constants.messages_paths import MESSAGES_ENDPOINT +import time + +test_client_1_details = { + "email": "ian.hodges1@nhs.net", + "name": "NHS Notify Test Client 1" +} + + +def default_request_url(url): + return f"{url}{MESSAGES_ENDPOINT}/pending_enrichment_request_item_id" + + +def send_multiple_requests(url, headers, count=10, delay=0): + for _ in range(count): + resp = requests.get(url, headers=headers) + if delay > 0: + time.sleep(delay) + return resp + + +@pytest.mark.devperftest +def test_429_triggered_app_quota(url, bearer_token, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_429_global_app_quota.rst + """ + rate_limiting.set_rate_limit(app_quota=4) + headers = Generators.generate_valid_headers(bearer_token.value) + resp = send_multiple_requests(default_request_url(url), headers) + + Assertions.assert_error_with_optional_correlation_id( + resp, + 429, + Generators.generate_quota_error_custom( + "Your application, Comms-manager-local, " + "has exceeded its quota of 4 requests every 1 minute(s) and is being rate limited."), + None + ) + + assert "Retry-After" in resp.headers + assert resp.headers.get("Retry-After") == "60" + + +@pytest.mark.devperftest +def test_429_triggered_app_spikearrest(url, bearer_token, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_429_global_app_spikearrest.rst + """ + rate_limiting.set_rate_limit(app_spikearrest="4pm") + headers = Generators.generate_valid_headers(bearer_token.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + Assertions.assert_error_with_optional_correlation_id( + resp, + 429, + Generators.generate_quota_error_custom( + "Your application, Comms-manager-local, " + "has created a spike in traffic and is being rate limited. Please reduce the frequency of your requests."), + None + ) + + assert "Retry-After" in resp.headers + assert resp.headers.get("Retry-After") == "1" + + +@pytest.mark.devperftest +def test_429_triggered_proxy_quota(url, bearer_token, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_429_proxy_quota.rst + """ + rate_limiting.set_rate_limit(proxy_quota=4) + headers = Generators.generate_valid_headers(bearer_token.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + Assertions.assert_error_with_optional_correlation_id( + resp, + 429, + Generators.generate_quota_error_custom( + "This API is currently receiving a high volume of requests " + "and is being rate limited."), + None + ) + + assert "Retry-After" in resp.headers + assert resp.headers.get("Retry-After") == "60" + + +@pytest.mark.devperftest +def test_429_triggered_proxy_spikearrest(url, bearer_token, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_429_proxy_spikearrest.rst + """ + rate_limiting.set_rate_limit(proxy_spikearrest="4pm") + headers = Generators.generate_valid_headers(bearer_token.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + Assertions.assert_error_with_optional_correlation_id( + resp, + 429, + Generators.generate_quota_error_custom( + "This API is currently receiving a high volume of requests " + "and is being rate limited."), + None + ) + + assert "Retry-After" in resp.headers + assert resp.headers.get("Retry-After") == "1" + + +@pytest.mark.devperftest +def test_429_triggered_specific_app_quota(url, bearer_token_internal_dev_test_1, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_429_specific_app_quota.rst + """ + rate_limiting.set_default_rate_limit() + rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], quota=4) + headers = Generators.generate_valid_headers(bearer_token_internal_dev_test_1.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + Assertions.assert_error_with_optional_correlation_id( + resp, + 429, + Generators.generate_quota_error_custom( + "Your application, NHS Notify Test Client 1, " + "has exceeded its quota of 4 requests every 1 minute(s) and is being rate limited."), + None + ) + + assert "Retry-After" in resp.headers + assert resp.headers.get("Retry-After") == "60" + + +@pytest.mark.devperftest +def test_429_not_triggered_other_specific_app_quota(url, bearer_token, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_200_specific_app_quota_different_app.rst + """ + rate_limiting.set_default_rate_limit() + rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], quota=4) + headers = Generators.generate_valid_headers(bearer_token.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + assert resp.status_code == 200, f"Response: {resp.status_code}: {resp.text}" + + +@pytest.mark.devperftest +def test_429_triggered_specific_app_spikearrest(url, + bearer_token, + bearer_token_internal_dev_test_1, + rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_429_specific_app_spikearrest.rst + """ + rate_limiting.set_default_rate_limit() + rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], spikearrest="4pm") + headers = Generators.generate_valid_headers(bearer_token_internal_dev_test_1.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + Assertions.assert_error_with_optional_correlation_id( + resp, + 429, + Generators.generate_quota_error_custom( + "Your application, NHS Notify Test Client 1, " + "has created a spike in traffic and is being rate limited. Please reduce the frequency of your requests."), + None + ) + + assert "Retry-After" in resp.headers + assert resp.headers.get("Retry-After") == "1" + + +@pytest.mark.devperftest +def test_429_not_triggered_other_specific_spikearrest(url, bearer_token, rate_limiting): + + """ + .. include:: ../partials/too_many_requests/test_200_specific_app_spikearrest_different_app.rst + """ + rate_limiting.set_default_rate_limit() + rate_limiting.set_app_ratelimit(test_client_1_details["email"], test_client_1_details["name"], spikearrest="4pm") + headers = Generators.generate_valid_headers(bearer_token.value) + + resp = send_multiple_requests(default_request_url(url), headers) + + assert resp.status_code == 200, f"Response: {resp.status_code}: {resp.text}" diff --git a/tests/lib/fixtures.py b/tests/lib/fixtures.py index 4b9449da5..0e1db49ee 100644 --- a/tests/lib/fixtures.py +++ b/tests/lib/fixtures.py @@ -6,6 +6,9 @@ from .rate_limiting import RateLimiting from lib.constants.constants import PROD_URL +EMAIL = 'ian.hodges1@nhs.net' +APP_NAME = 'NHS Notify Test Client 1' + @pytest.fixture(scope='session') def proxy_name(): @@ -52,14 +55,14 @@ def api_product_in_nhs_notify_test_client_1(developer_app_keys_api, api_product_ if api_product_name.startswith('communications-manager-pr-'): yield ensure_api_product_in_application( developer_app_keys_api, - 'ian.hodges1@nhs.net', - 'NHS Notify Test Client 1', + EMAIL, + APP_NAME, os.environ['NON_PROD_API_KEY_TEST_1'], api_product_name) ensure_api_product_not_in_application( developer_app_keys_api, - 'ian.hodges1@nhs.net', - 'NHS Notify Test Client 1', + EMAIL, + APP_NAME, os.environ['NON_PROD_API_KEY_TEST_1'], api_product_name) else: @@ -86,7 +89,7 @@ def rate_limiting(products_api, api_product_name, developer_apps_api): rate_limiting = RateLimiting(products_api, developer_apps_api, api_product_name, 5) yield rate_limiting rate_limiting.set_default_rate_limit() - rate_limiting.remove_app_ratelimit('ian.hodges1@nhs.net', 'NHS Notify Test Client 1') + rate_limiting.remove_app_ratelimit(EMAIL, APP_NAME) @pytest.fixture(scope='session') diff --git a/tests/lib/generators.py b/tests/lib/generators.py index cd47caaeb..7b2baac97 100644 --- a/tests/lib/generators.py +++ b/tests/lib/generators.py @@ -298,19 +298,4 @@ def generate_error(error, source=None, meta=None, detail=None): ret["meta"] = meta return ret - @staticmethod - def generate_error_with_custom_detail(error, detail=None, source=None, meta=None): - ret = { - "code": error.code, - "links": error.links, - "status": error.status, - "title": error.title, - "detail": detail or error.detail - } - - if source: - ret["source"] = source - - if meta: - ret["meta"] = meta - return ret + generate_error_with_custom_detail = generate_error diff --git a/tests/mtls/test_mtls.py b/tests/mtls/test_mtls.py index 32d2ee85a..669554653 100644 --- a/tests/mtls/test_mtls.py +++ b/tests/mtls/test_mtls.py @@ -1,65 +1,70 @@ -""" -This test suite ensures that our mutual TLS security is enabled on -all of the environments. - -This is important as it ensures that we have not accidentally disabled -mutual TLS post deployment, or by accidentally making a manual change -which causes it to become disabled. - -These tests are run post deployment for: - -* internal-dev -* uat -* int -* prod - -They are also run every 10 minutes, with failures sent to Teams. -""" - -import requests -import pytest -from lib.constants.constants import * - - -@pytest.mark.mtlstest -@pytest.mark.inttest -def test_mtls_connection_reset_by_peer_int(): - """ - Ensures that mTLS is enabled on the integration API backend service. - """ - with pytest.raises(Exception) as e: - requests.get(INT_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) - assert e.value is not None - - -@pytest.mark.mtlstest -@pytest.mark.devtest -def test_mtls_connection_reset_by_peer_dev(): - """ - Ensures that mTLS is enabled on the internal-dev API backend service. - """ - with pytest.raises(Exception) as e: - requests.get(DEV_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) - assert e.value is not None - - -@pytest.mark.mtlstest -@pytest.mark.prodtest -def test_mtls_connection_reset_by_peer_prod(): - """ - Ensures that mTLS is enabled on the production API backend service. - """ - with pytest.raises(Exception) as e: - requests.get(PROD_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) - assert e.value is not None - - -@pytest.mark.mtlstest -@pytest.mark.uattest -def test_mtls_connection_reset_by_peer_uat(): - """ - Ensures that mTLS is enabled on the UAT, API backend service. - """ - with pytest.raises(Exception) as e: - requests.get(UAT_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) - assert e.value is not None +""" +This test suite ensures that our mutual TLS security is enabled on +all of the environments. + +This is important as it ensures that we have not accidentally disabled +mutual TLS post deployment, or by accidentally making a manual change +which causes it to become disabled. + +These tests are run post deployment for: + +* internal-dev +* uat +* int +* prod + +They are also run every 10 minutes, with failures sent to Teams. +""" + +import requests +import pytest +from lib.constants.constants import ( + INT_API_GATEWAY_URL, + DEV_API_GATEWAY_URL, + PROD_API_GATEWAY_URL, + UAT_API_GATEWAY_URL, +) + + +@pytest.mark.mtlstest +@pytest.mark.inttest +def test_mtls_connection_reset_by_peer_int(): + """ + Ensures that mTLS is enabled on the integration API backend service. + """ + with pytest.raises(Exception) as e: + requests.get(INT_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) + assert e.value is not None + + +@pytest.mark.mtlstest +@pytest.mark.devtest +def test_mtls_connection_reset_by_peer_dev(): + """ + Ensures that mTLS is enabled on the internal-dev API backend service. + """ + with pytest.raises(Exception) as e: + requests.get(DEV_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) + assert e.value is not None + + +@pytest.mark.mtlstest +@pytest.mark.prodtest +def test_mtls_connection_reset_by_peer_prod(): + """ + Ensures that mTLS is enabled on the production API backend service. + """ + with pytest.raises(Exception) as e: + requests.get(PROD_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) + assert e.value is not None + + +@pytest.mark.mtlstest +@pytest.mark.uattest +def test_mtls_connection_reset_by_peer_uat(): + """ + Ensures that mTLS is enabled on the UAT, API backend service. + """ + with pytest.raises(Exception) as e: + requests.get(UAT_API_GATEWAY_URL, headers={"X-Client-Id": "hello"}) + assert e.value is not None diff --git a/tests/sandbox/message_batches/create_message_batches/test_too_large.py b/tests/sandbox/message_batches/create_message_batches/test_too_large.py index 1e654c236..59fdf8521 100644 --- a/tests/sandbox/message_batches/create_message_batches/test_too_large.py +++ b/tests/sandbox/message_batches/create_message_batches/test_too_large.py @@ -21,7 +21,7 @@ def test_too_many_messages(nhsd_apim_proxy_url): messages = [] data["data"]["attributes"]["messages"] = messages - for i in range(MESSAGE_LIMIT+1): + for _ in range(MESSAGE_LIMIT+1): messages.append({ "messageReference": str(uuid.uuid1()), "recipient": { diff --git a/zap/Dockerfile b/zap/Dockerfile index 5f27c5f69..b2dd8de7d 100644 --- a/zap/Dockerfile +++ b/zap/Dockerfile @@ -2,7 +2,7 @@ FROM softwaresecurityproject/zap-stable:2.15.0 COPY ./zap/policies/ /home/zap/.ZAP/policies/ -RUN ./zap.sh -addoninstallall -cmd -RUN ./zap.sh -addonuninstall communityScripts -cmd -RUN ./zap.sh -addonuninstall packpentester -cmd -RUN ./zap.sh -addonuninstall wappalyzer -cmd +RUN ./zap.sh -addoninstallall -cmd \ + && ./zap.sh -addonuninstall communityScripts -cmd \ + && ./zap.sh -addonuninstall packpentester -cmd \ + && ./zap.sh -addonuninstall wappalyzer -cmd From a5df150d370400b182214e1143a76001d66eecc0 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 8 Sep 2025 11:22:11 +0100 Subject: [PATCH 04/18] CCM-6437: test --- proxies/shared/resources/jsc/MessageBatches.Create.Validate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 12ce9c8c8..31237d7cf 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -54,7 +54,7 @@ const validate = () => { errors.push(invalidError(pointer)); } else { - pointer = "/data/attributes/messages/" + index + "/messageReference"; + var pointer = "/data/attributes/messages/" + index + "/messageReference"; const validMessageReference = validateString(errors, message.messageReference, pointer) if (validMessageReference) { if (seenMessages[message.messageReference]) { From c607db4edc6d92c392b1b3a09434d1b00b4155e6 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 8 Sep 2025 11:55:03 +0100 Subject: [PATCH 05/18] CCM-6437: Remove generate_error_with_custom_detail function --- .../resources/jsc/MessageBatches.Create.Validate.js | 2 +- tests/lib/generators.py | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 31237d7cf..12ce9c8c8 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -54,7 +54,7 @@ const validate = () => { errors.push(invalidError(pointer)); } else { - var pointer = "/data/attributes/messages/" + index + "/messageReference"; + pointer = "/data/attributes/messages/" + index + "/messageReference"; const validMessageReference = validateString(errors, message.messageReference, pointer) if (validMessageReference) { if (seenMessages[message.messageReference]) { diff --git a/tests/lib/generators.py b/tests/lib/generators.py index 7b2baac97..ea4a546a6 100644 --- a/tests/lib/generators.py +++ b/tests/lib/generators.py @@ -130,7 +130,7 @@ def generate_invalid_value_error(pointer): @staticmethod def generate_invalid_value_error_custom_detail(pointer, detail): - return Generators.generate_error_with_custom_detail(constants.ERROR_INVALID_VALUE, detail, source={ + return Generators.generate_error(constants.ERROR_INVALID_VALUE, detail, source={ "pointer": pointer }) @@ -148,7 +148,7 @@ def generate_missing_value_error(pointer): @staticmethod def generate_missing_value_error_custom_detail(pointer, detail): - return Generators.generate_error_with_custom_detail(constants.ERROR_MISSING_VALUE, detail, source={ + return Generators.generate_error(constants.ERROR_MISSING_VALUE, detail, source={ "pointer": pointer }) @@ -172,7 +172,7 @@ def generate_too_few_items_error(pointer): @staticmethod def generate_too_few_items_error_custom_detail(pointer, detail): - return Generators.generate_error_with_custom_detail(constants.ERROR_TOO_FEW_ITEMS, detail, source={ + return Generators.generate_error(constants.ERROR_TOO_FEW_ITEMS, detail, source={ "pointer": pointer }) @@ -276,13 +276,13 @@ def generate_bad_gateway_error(): def generate_invalid_personalisation_error(detail, pointer): return Generators.generate_error( constants.ERROR_INVALID_PERSONALISATION, - detail=detail, + detail, source={ "pointer": pointer }) @staticmethod - def generate_error(error, source=None, meta=None, detail=None): + def generate_error(error, detail=None, source=None, meta=None): ret = { "code": error.code, "links": error.links, @@ -297,5 +297,3 @@ def generate_error(error, source=None, meta=None, detail=None): if meta: ret["meta"] = meta return ret - - generate_error_with_custom_detail = generate_error From 02b3c03c1ca4f9d99858e21a9bf23a5df7a77d22 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 8 Sep 2025 15:05:52 +0100 Subject: [PATCH 06/18] CCM-6437: test --- .../jsc/MessageBatches.Create.Validate.js | 51 ++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 12ce9c8c8..45345ac09 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -46,41 +46,34 @@ const validate = () => { return null; } - if (isUndefined(message)) { - errors.push(missingError(pointer)); - } else if (message === null) { - errors.push(nullError(pointer)); - } else if (typeof message !== "object" || Array.isArray(message)) { - errors.push(invalidError(pointer)); - } else { - - pointer = "/data/attributes/messages/" + index + "/messageReference"; - const validMessageReference = validateString(errors, message.messageReference, pointer) - if (validMessageReference) { - if (seenMessages[message.messageReference]) { - errors.push(duplicateError(pointer)); - } else { - seenMessages[message.messageReference] = 1; - } + if (!validateObject(errors, message, pointer)) return; + + pointer = "/data/attributes/messages/" + index + "/messageReference"; + const validMessageReference = validateString(errors, message.messageReference, pointer) + if (validMessageReference) { + if (seenMessages[message.messageReference]) { + errors.push(duplicateError(pointer)); + } else { + seenMessages[message.messageReference] = 1; } + } - const validRecipientObject = validateObject(errors, message.recipient, "/data/attributes/messages/" + index + "/recipient") - if (validRecipientObject) { + const validRecipientObject = validateObject(errors, message.recipient, "/data/attributes/messages/" + index + "/recipient") + if (validRecipientObject) { - validateNhsNumber(errors, message.recipient.nhsNumber, "/data/attributes/messages/" + index + "/recipient/nhsNumber") - } + validateNhsNumber(errors, message.recipient.nhsNumber, "/data/attributes/messages/" + index + "/recipient/nhsNumber") + } - if (!isUndefined(message.originator)) { - const validOriginatorObject = validateObject(errors, message.originator, "/data/attributes/messages/" + index + "/originator") - if (validOriginatorObject) { - validateOdsCode(errors, message.originator.odsCode, "/data/attributes/messages/" + index + "/originator/odsCode") - } + if (!isUndefined(message.originator)) { + const validOriginatorObject = validateObject(errors, message.originator, "/data/attributes/messages/" + index + "/originator") + if (validOriginatorObject) { + validateOdsCode(errors, message.originator.odsCode, "/data/attributes/messages/" + index + "/originator/odsCode") } + } - pointer = "/data/attributes/messages/" + index + "/personalisation"; - if (!isUndefined(message.personalisation)) { - validateObject(errors, message.personalisation, pointer) - } + pointer = "/data/attributes/messages/" + index + "/personalisation"; + if (!isUndefined(message.personalisation)) { + validateObject(errors, message.personalisation, pointer) } }); } From 3d580a306059ebc0da06591dd1c7e861d3992174 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 8 Sep 2025 16:41:21 +0100 Subject: [PATCH 07/18] CCM-6437: fix Sonar cognitive complexity issues --- .../jsc/MessageBatches.Create.Validate.js | 117 ++++++++++-------- .../resources/jsc/Messages.Create.Validate.js | 66 +++++----- 2 files changed, 99 insertions(+), 84 deletions(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 45345ac09..1e80afe18 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -15,73 +15,80 @@ try { errors.push(invalidError("/")) } +function validateBatchMessage(errors, message, index, seenMessages) { + var pointer = "/data/attributes/messages/" + index; -const validate = () => { - if (all) { - var seenMessages = {}; - var data = all.data; - - const validDataObject = validateObject(errors, data, "/data") - if (validDataObject) { - validateConstantString(errors, data.type, "/data/type", "MessageBatch") - - const validAttributesObject = validateObject(errors, data.attributes, "/data/attributes") - if (validAttributesObject) { - - validateUuid(errors, data.attributes.routingPlanId, "/data/attributes/routingPlanId") - - validateString(errors, data.attributes.messageBatchReference, "/data/attributes/messageBatchReference") - - const validArray = validateArray(errors, data.attributes.messages, "/data/attributes/messages", 1) - if (validArray) { - if (data.attributes.messages.length > 45000) { - errors.push(tooManyItemsError("/data/attributes/messages")); - return null; - } - data.attributes.messages.forEach((message, index) => { - var pointer = "/data/attributes/messages/" + index; - // Limit the amount of errors returned to 100 entries - if (errors.length >= 100) { - errors = errors.slice(0, 100); - return null; - } + // Limit the amount of errors returned to 100 entries + if (errors.length >= 100) { + errors = errors.slice(0, 100); + return null; + } - if (!validateObject(errors, message, pointer)) return; + if (!validateObject(errors, message, pointer)) return; - pointer = "/data/attributes/messages/" + index + "/messageReference"; - const validMessageReference = validateString(errors, message.messageReference, pointer) - if (validMessageReference) { - if (seenMessages[message.messageReference]) { - errors.push(duplicateError(pointer)); - } else { - seenMessages[message.messageReference] = 1; - } - } + pointer = "/data/attributes/messages/" + index + "/messageReference"; + const validMessageReference = validateString(errors, message.messageReference, pointer); + if (validMessageReference) { + if (seenMessages[message.messageReference]) { + errors.push(duplicateError(pointer)); + } else { + seenMessages[message.messageReference] = 1; + } + } - const validRecipientObject = validateObject(errors, message.recipient, "/data/attributes/messages/" + index + "/recipient") - if (validRecipientObject) { + const validRecipientObject = validateObject(errors, message.recipient, "/data/attributes/messages/" + index + "/recipient"); + if (validRecipientObject) { + validateNhsNumber(errors, message.recipient.nhsNumber, "/data/attributes/messages/" + index + "/recipient/nhsNumber"); + } - validateNhsNumber(errors, message.recipient.nhsNumber, "/data/attributes/messages/" + index + "/recipient/nhsNumber") - } + if (!isUndefined(message.originator)) { + const validOriginatorObject = validateObject(errors, message.originator, "/data/attributes/messages/" + index + "/originator"); + if (validOriginatorObject) { + validateOdsCode(errors, message.originator.odsCode, "/data/attributes/messages/" + index + "/originator/odsCode"); + } + } - if (!isUndefined(message.originator)) { - const validOriginatorObject = validateObject(errors, message.originator, "/data/attributes/messages/" + index + "/originator") - if (validOriginatorObject) { - validateOdsCode(errors, message.originator.odsCode, "/data/attributes/messages/" + index + "/originator/odsCode") - } - } + pointer = "/data/attributes/messages/" + index + "/personalisation"; + if (!isUndefined(message.personalisation)) { + validateObject(errors, message.personalisation, pointer); + } +} - pointer = "/data/attributes/messages/" + index + "/personalisation"; - if (!isUndefined(message.personalisation)) { - validateObject(errors, message.personalisation, pointer) - } - }); - } +function validateBatchAttributes(errors, attributes, seenMessages) { + const validAttributesObject = validateObject(errors, attributes, "/data/attributes"); + if (validAttributesObject) { + validateUuid(errors, attributes.routingPlanId, "/data/attributes/routingPlanId"); + validateString(errors, attributes.messageBatchReference, "/data/attributes/messageBatchReference"); + + const validArray = validateArray(errors, attributes.messages, "/data/attributes/messages", 1); + if (validArray) { + if (attributes.messages.length > 45000) { + errors.push(tooManyItemsError("/data/attributes/messages")); + return null; } + attributes.messages.forEach((message, index) => { + validateBatchMessage(errors, message, index, seenMessages); + }); } } } +function validateBatchData(errors, data, seenMessages) { + const validDataObject = validateObject(errors, data, "/data"); + if (validDataObject) { + validateConstantString(errors, data.type, "/data/type", "MessageBatch"); + validateBatchAttributes(errors, data.attributes, seenMessages); + } +} + +const validate = () => { + if (all) { + var seenMessages = {}; + var data = all.data; + validateBatchData(errors, data, seenMessages); + } +} + validate(); if (errors.length > 0) { diff --git a/proxies/shared/resources/jsc/Messages.Create.Validate.js b/proxies/shared/resources/jsc/Messages.Create.Validate.js index 2a7b708f9..d4892f3d3 100644 --- a/proxies/shared/resources/jsc/Messages.Create.Validate.js +++ b/proxies/shared/resources/jsc/Messages.Create.Validate.js @@ -16,42 +16,50 @@ try { errors.push(invalidError("/")) } +function validateRecipient(errors, recipient) { + const validRecipientObject = validateObject(errors, recipient, "/data/attributes/recipient"); + if (validRecipientObject) { + validateNhsNumber(errors, recipient.nhsNumber, "/data/attributes/recipient/nhsNumber"); + } +} +function validateOriginator(errors, originator) { + const validOriginatorObject = validateObject(errors, originator, "/data/attributes/originator"); + if (validOriginatorObject) { + validateOdsCode(errors, originator.odsCode, "/data/attributes/originator/odsCode"); + } +} -const validate = () => { - if (all) { - var data = all.data; - - const validDataObject = validateObject(errors, data, "/data") - if (validDataObject) { - - validateConstantString(errors, data.type, "/data/type", "Message") - - const validAttributesObject = validateObject(errors, data.attributes, "/data/attributes") - if (validAttributesObject) { - - validateUuid(errors, data.attributes.routingPlanId, "/data/attributes/routingPlanId") +function validateAttributes(errors, attributes) { + const validAttributesObject = validateObject(errors, attributes, "/data/attributes"); + if (validAttributesObject) { + validateUuid(errors, attributes.routingPlanId, "/data/attributes/routingPlanId"); + validateString(errors, attributes.messageReference, "/data/attributes/messageReference"); - validateString(errors, data.attributes.messageReference, "/data/attributes/messageReference") + validateRecipient(errors, attributes.recipient); - const validRecipientObject = validateObject(errors, data.attributes.recipient, "/data/attributes/recipient") - if (validRecipientObject) { + if (!isUndefined(attributes.originator)) { + validateOriginator(errors, attributes.originator); + } - validateNhsNumber(errors, data.attributes.recipient.nhsNumber, "/data/attributes/recipient/nhsNumber") - } + if (!isUndefined(attributes.personalisation)) { + validateObject(errors, attributes.personalisation, "/data/attributes/personalisation"); + } + } +} - if (!isUndefined(data.attributes.originator)) { - const validOriginatorObject = validateObject(errors, data.attributes.originator, "/data/attributes/originator") - if (validOriginatorObject) { - validateOdsCode(errors, data.attributes.originator.odsCode, "/data/attributes/originator/odsCode") - } - } +function validateData(errors, data) { + const validDataObject = validateObject(errors, data, "/data"); + if (validDataObject) { + validateConstantString(errors, data.type, "/data/type", "Message"); + validateAttributes(errors, data.attributes); + } +} - if (!isUndefined(data.attributes.personalisation)) { - validateObject(errors, data.attributes.personalisation, "/data/attributes/personalisation") - } - } - } +const validate = () => { + if (all) { + var data = all.data; + validateData(errors, data); } } From 17a39d546e76b4762736df9a8e184629c64d2fce Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 8 Sep 2025 16:51:21 +0100 Subject: [PATCH 08/18] CCM-6437: fix Sonar Maintainability issue --- proxies/shared/resources/jsc/MessageBatches.Create.Validate.js | 1 - 1 file changed, 1 deletion(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 1e80afe18..0b9f48497 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -20,7 +20,6 @@ function validateBatchMessage(errors, message, index, seenMessages) { // Limit the amount of errors returned to 100 entries if (errors.length >= 100) { - errors = errors.slice(0, 100); return null; } From 053ddb3331e5b3c0698496cc90a361c3703fa9f3 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Tue, 9 Sep 2025 09:07:28 +0100 Subject: [PATCH 09/18] CCM-6437: fix Sonar Maintainability issues --- .../shared/resources/jsc/MessageBatches.Create.Validate.js | 7 +++++-- proxies/shared/resources/jsc/Messages.Create.Validate.js | 3 +++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 0b9f48497..ce2fe0fa2 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -9,10 +9,12 @@ var content = context.getVariable("request.content") var errors = [] var all +var parseFailed = false; try { - all = JSON.parse(content) + all = JSON.parse(content) } catch (e) { - errors.push(invalidError("/")) + errors.push(invalidError("/")) + parseFailed = true; } function validateBatchMessage(errors, message, index, seenMessages) { @@ -81,6 +83,7 @@ function validateBatchData(errors, data, seenMessages) { } const validate = () => { + if (parseFailed) return; if (all) { var seenMessages = {}; var data = all.data; diff --git a/proxies/shared/resources/jsc/Messages.Create.Validate.js b/proxies/shared/resources/jsc/Messages.Create.Validate.js index d4892f3d3..1d6c019f9 100644 --- a/proxies/shared/resources/jsc/Messages.Create.Validate.js +++ b/proxies/shared/resources/jsc/Messages.Create.Validate.js @@ -10,10 +10,12 @@ var content = context.getVariable("request.content") var errors = [] var all +var parseFailed = false; try { all = JSON.parse(content) } catch (e) { errors.push(invalidError("/")) + parseFailed = true; } function validateRecipient(errors, recipient) { @@ -57,6 +59,7 @@ function validateData(errors, data) { } const validate = () => { + if (parseFailed) return; if (all) { var data = all.data; validateData(errors, data); From 1d9bc05f91ec8d49faa00a853e30b396d161ddcb Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Tue, 9 Sep 2025 09:08:19 +0100 Subject: [PATCH 10/18] CCM-6437: test --- proxies/shared/resources/jsc/SetResponseDefaults.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/proxies/shared/resources/jsc/SetResponseDefaults.js b/proxies/shared/resources/jsc/SetResponseDefaults.js index a3e7b79b8..0cc330fd0 100644 --- a/proxies/shared/resources/jsc/SetResponseDefaults.js +++ b/proxies/shared/resources/jsc/SetResponseDefaults.js @@ -32,13 +32,8 @@ headerNames.forEach(function (header) { }); // format errors and response objects -try { - const errorContent = context.getVariable("error.content") - const responseContent = context.getVariable("response.content") +const errorContent = context.getVariable("error.content") +const responseContent = context.getVariable("response.content") - if (errorContent) context.setVariable("error.content", JSON.stringify(JSON.parse(errorContent))) - if (responseContent) context.setVariable("response.content", JSON.stringify(JSON.parse(responseContent))) - -} catch (e) { - // -} +if (errorContent) context.setVariable("error.content", JSON.stringify(JSON.parse(errorContent))) +if (responseContent) context.setVariable("response.content", JSON.stringify(JSON.parse(responseContent))) From 5e682b217659a9d946d86aec75ab23e31a5a649c Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Wed, 10 Sep 2025 08:21:35 +0100 Subject: [PATCH 11/18] CCM-6437: fix Sonar Maintainability issues --- proxies/shared/resources/jsc/HealthCheck.SetResponse.js | 2 +- tests/lib/authentication.py | 2 +- tests/lib/rate_limiting.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/proxies/shared/resources/jsc/HealthCheck.SetResponse.js b/proxies/shared/resources/jsc/HealthCheck.SetResponse.js index cff918fe8..d87df6aee 100644 --- a/proxies/shared/resources/jsc/HealthCheck.SetResponse.js +++ b/proxies/shared/resources/jsc/HealthCheck.SetResponse.js @@ -8,7 +8,7 @@ function json_tryparse(raw) { try { return JSON.parse(raw); } - catch (e) { + catch (e) { // NOSONAR return raw; } } diff --git a/tests/lib/authentication.py b/tests/lib/authentication.py index 255268e1b..8427a0a5d 100644 --- a/tests/lib/authentication.py +++ b/tests/lib/authentication.py @@ -91,7 +91,7 @@ def generate_and_test_new_token(self, api_key, private_key, url, kid, test_url): return new_token print("Could not generate token") - raise Exception("Could not generate token") + raise RuntimeError("Could not generate token") def generate_new_token(self, api_key, private_key, url, kid): pk_pem = None diff --git a/tests/lib/rate_limiting.py b/tests/lib/rate_limiting.py index a5377dbf7..d06ee203d 100644 --- a/tests/lib/rate_limiting.py +++ b/tests/lib/rate_limiting.py @@ -13,7 +13,7 @@ def __init__(self, products_api, developer_apps_api, api_product_name, delay): def extract_ratelimiting_attribute(self): if self.product["attributes"] is None: - raise Exception(f'Product "{self.api_product_name}" does not have any attributes') + raise ValueError(f'Product "{self.api_product_name}" does not have any attributes') ratelimit = None for attribute in self.product["attributes"]: @@ -22,7 +22,7 @@ def extract_ratelimiting_attribute(self): break if ratelimit is None: - raise Exception(f'Product "{self.api_product_name}" does not have a ratelimit attribute') + raise ValueError(f'Product "{self.api_product_name}" does not have a ratelimit attribute') return ratelimit From 878e774ba8cb3227828d3391a0d6b493d271d497 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Thu, 11 Sep 2025 08:44:50 +0100 Subject: [PATCH 12/18] CCM-6437: test --- proxies/shared/resources/jsc/MessageBatches.Create.Validate.js | 1 + 1 file changed, 1 insertion(+) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index ce2fe0fa2..7796c80fc 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -22,6 +22,7 @@ function validateBatchMessage(errors, message, index, seenMessages) { // Limit the amount of errors returned to 100 entries if (errors.length >= 100) { + errors = errors.slice(0, 100); return null; } From f365fdbebaa6c6a6d25448d48a0c2aedfa180aa2 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Thu, 11 Sep 2025 09:00:24 +0100 Subject: [PATCH 13/18] CCM-6437: test --- proxies/shared/resources/jsc/MessageBatches.Create.Validate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index 7796c80fc..dba84a2a5 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -22,7 +22,7 @@ function validateBatchMessage(errors, message, index, seenMessages) { // Limit the amount of errors returned to 100 entries if (errors.length >= 100) { - errors = errors.slice(0, 100); + // errors = errors.slice(0, 100); return null; } From dffbfdb22e621f3d3ac1c35539f61b7c8d61e0f8 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Thu, 11 Sep 2025 11:41:20 +0100 Subject: [PATCH 14/18] CCM-6437: test --- .../jsc/MessageBatches.Create.Validate.js | 71 ++++++++++++++----- 1 file changed, 54 insertions(+), 17 deletions(-) diff --git a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js index dba84a2a5..eaa8d15a0 100644 --- a/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js +++ b/proxies/shared/resources/jsc/MessageBatches.Create.Validate.js @@ -5,16 +5,16 @@ * It is called by the JavaScript.MessageBatches.Create.Validate policy. */ -var content = context.getVariable("request.content") -var errors = [] +var content = context.getVariable("request.content"); +var errors = []; -var all +var all; var parseFailed = false; try { - all = JSON.parse(content) + all = JSON.parse(content); } catch (e) { - errors.push(invalidError("/")) - parseFailed = true; + errors.push(invalidError("/")); + parseFailed = true; } function validateBatchMessage(errors, message, index, seenMessages) { @@ -22,14 +22,17 @@ function validateBatchMessage(errors, message, index, seenMessages) { // Limit the amount of errors returned to 100 entries if (errors.length >= 100) { - // errors = errors.slice(0, 100); return null; } if (!validateObject(errors, message, pointer)) return; pointer = "/data/attributes/messages/" + index + "/messageReference"; - const validMessageReference = validateString(errors, message.messageReference, pointer); + const validMessageReference = validateString( + errors, + message.messageReference, + pointer + ); if (validMessageReference) { if (seenMessages[message.messageReference]) { errors.push(duplicateError(pointer)); @@ -38,15 +41,31 @@ function validateBatchMessage(errors, message, index, seenMessages) { } } - const validRecipientObject = validateObject(errors, message.recipient, "/data/attributes/messages/" + index + "/recipient"); + const validRecipientObject = validateObject( + errors, + message.recipient, + "/data/attributes/messages/" + index + "/recipient" + ); if (validRecipientObject) { - validateNhsNumber(errors, message.recipient.nhsNumber, "/data/attributes/messages/" + index + "/recipient/nhsNumber"); + validateNhsNumber( + errors, + message.recipient.nhsNumber, + "/data/attributes/messages/" + index + "/recipient/nhsNumber" + ); } if (!isUndefined(message.originator)) { - const validOriginatorObject = validateObject(errors, message.originator, "/data/attributes/messages/" + index + "/originator"); + const validOriginatorObject = validateObject( + errors, + message.originator, + "/data/attributes/messages/" + index + "/originator" + ); if (validOriginatorObject) { - validateOdsCode(errors, message.originator.odsCode, "/data/attributes/messages/" + index + "/originator/odsCode"); + validateOdsCode( + errors, + message.originator.odsCode, + "/data/attributes/messages/" + index + "/originator/odsCode" + ); } } @@ -57,12 +76,29 @@ function validateBatchMessage(errors, message, index, seenMessages) { } function validateBatchAttributes(errors, attributes, seenMessages) { - const validAttributesObject = validateObject(errors, attributes, "/data/attributes"); + const validAttributesObject = validateObject( + errors, + attributes, + "/data/attributes" + ); if (validAttributesObject) { - validateUuid(errors, attributes.routingPlanId, "/data/attributes/routingPlanId"); - validateString(errors, attributes.messageBatchReference, "/data/attributes/messageBatchReference"); + validateUuid( + errors, + attributes.routingPlanId, + "/data/attributes/routingPlanId" + ); + validateString( + errors, + attributes.messageBatchReference, + "/data/attributes/messageBatchReference" + ); - const validArray = validateArray(errors, attributes.messages, "/data/attributes/messages", 1); + const validArray = validateArray( + errors, + attributes.messages, + "/data/attributes/messages", + 1 + ); if (validArray) { if (attributes.messages.length > 45000) { errors.push(tooManyItemsError("/data/attributes/messages")); @@ -90,7 +126,8 @@ const validate = () => { var data = all.data; validateBatchData(errors, data, seenMessages); } -} + errors = errors.slice(0, 100); +}; validate(); From 2ead3d8e0bc639241e3e9855b2b35f136d8055f8 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Thu, 11 Sep 2025 15:49:54 +0100 Subject: [PATCH 15/18] CCM-6437: test --- .../shared/resources/jsc/SetResponseDefaults.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/proxies/shared/resources/jsc/SetResponseDefaults.js b/proxies/shared/resources/jsc/SetResponseDefaults.js index 0cc330fd0..525953b9a 100644 --- a/proxies/shared/resources/jsc/SetResponseDefaults.js +++ b/proxies/shared/resources/jsc/SetResponseDefaults.js @@ -32,8 +32,12 @@ headerNames.forEach(function (header) { }); // format errors and response objects -const errorContent = context.getVariable("error.content") -const responseContent = context.getVariable("response.content") - -if (errorContent) context.setVariable("error.content", JSON.stringify(JSON.parse(errorContent))) -if (responseContent) context.setVariable("response.content", JSON.stringify(JSON.parse(responseContent))) +try { + const errorContent = context.getVariable("error.content") + const responseContent = context.getVariable("response.content") + + if (errorContent) context.setVariable("error.content", JSON.stringify(JSON.parse(errorContent))) + if (responseContent) context.setVariable("response.content", JSON.stringify(JSON.parse(responseContent))) +} catch (e) { + +} From 3087986dabefacf1b11eb11a02efa30f13302dba Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Fri, 12 Sep 2025 08:13:54 +0100 Subject: [PATCH 16/18] CCM-6437: disable sonar in SetResponseDefaults.js --- proxies/shared/resources/jsc/SetResponseDefaults.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxies/shared/resources/jsc/SetResponseDefaults.js b/proxies/shared/resources/jsc/SetResponseDefaults.js index 525953b9a..840bd0768 100644 --- a/proxies/shared/resources/jsc/SetResponseDefaults.js +++ b/proxies/shared/resources/jsc/SetResponseDefaults.js @@ -38,6 +38,6 @@ try { if (errorContent) context.setVariable("error.content", JSON.stringify(JSON.parse(errorContent))) if (responseContent) context.setVariable("response.content", JSON.stringify(JSON.parse(responseContent))) -} catch (e) { - +} catch (e) { // NOSONAR + // Intentionally ignore parsing errors. } From 6f5d420154e1dd18538649b11ea7b1bdf62c7ff2 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 15 Sep 2025 10:42:51 +0100 Subject: [PATCH 17/18] CCM-6437: bump ZAP base image from 2.15 to 2.16.1 --- zap/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zap/Dockerfile b/zap/Dockerfile index b2dd8de7d..94b9856b3 100644 --- a/zap/Dockerfile +++ b/zap/Dockerfile @@ -1,4 +1,4 @@ -FROM softwaresecurityproject/zap-stable:2.15.0 +FROM softwaresecurityproject/zap-stable:2.16.1 COPY ./zap/policies/ /home/zap/.ZAP/policies/ From de37f361c10d1851e9de063391a7bee1bdb39b34 Mon Sep 17 00:00:00 2001 From: lapenna-bjss Date: Mon, 15 Sep 2025 10:56:54 +0100 Subject: [PATCH 18/18] CCM-6437: bump ZAP base image from 2.15 to 2.16.1 --- zap/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zap/Dockerfile b/zap/Dockerfile index 94b9856b3..6cab073a1 100644 --- a/zap/Dockerfile +++ b/zap/Dockerfile @@ -1,4 +1,4 @@ -FROM softwaresecurityproject/zap-stable:2.16.1 +FROM zaproxy/zap-stable:2.16.1 COPY ./zap/policies/ /home/zap/.ZAP/policies/