CCM-17440: Fix dependabot issues by simonlabarere · Pull Request #321 · NHSDigital/nhs-notify-digital-letters

simonlabarere · 2026-04-30T09:57:20Z

Description

Updated the following dependencies:

axios
uuid override for mermaid
erb

node-jose uses a vulnerable version of uuid and it couldn't be overridden (I hacked it to force it use a new version and node-jose wouldn't work because it detected something was wrong).

I used copilot to replace node-jose with a combination of jose (which is maintained) and node:crypto. The problem is that jose version 6 (latest) is not compatible out of the box with the way we use jest for testing. I spent some time trying to make it work but it involved using an experimental version of jest and it made all the unit tests flaky in general. Version 5 of jose didn't have that problem so this is what I ended up using but it's a year old. On the other hand it doesn't have any vulnerabilities so it's still better than the 3 year old version of node-jose.

Testing involved generating a new public/private key pair and making sure it's working with the APIM authentication mechanism.

Testing

Running the key-generation lambda manually

Generated private key

Generated public key

APIM Updated to point at pr321

Token generation

Other changes

Since we last ran the build docs step:

The package-lock.json that previously lived inside the docs folder no longer exists, as docs is now part of the root workspace. The build-docs action was running cd docs && npm ci, which no longer installs anything because there is no lockfile in that directory.
GitHub Actions (including build-docs) have been centralised and moved into the nhs-notify-shared-modules repository.
The workflow was previously referencing an older shared-modules release where the build-docs action was pinned to Node 18.

There is a corresponding PR in the shared-modules repository to address these changes. Until a new version of shared-modules is released, this PR references the commit SHA directly. Once a new release is available, we can update this to use the release tag instead.

Type of changes

Refactoring (non-breaking change)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would change existing functionality)
Bug fix (non-breaking change which fixes an issue)

Checklist

I am familiar with the contributing guidelines
I have followed the code style of the project
I have added tests to cover my changes
I have updated the documentation accordingly
This PR is a result of pair or mob programming

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

Ian-Hodges · 2026-05-08T13:21:06Z

@@ -1,4 +1,4 @@
-import { KeyStore } from '../../key-generation-utils/jwk';
+import { KeyStore } from '../../key-generation-utils/jwk-key-store';


Sorry, it's super minor, but this can be shortened.
import { KeyStore } from '../../key-generation-utils';

Actually, it can be added to the one below.
import { KeyStore, generateNewKey } from '../../key-generation-utils';

Ian-Hodges · 2026-05-08T13:24:23Z

@@ -0,0 +1,73 @@
+import { Key } from '../../key-generation-utils/jwk-key';


Suggested change

import { Key } from '../../key-generation-utils/jwk-key';

import { Key } from '../../key-generation-utils';

Ian-Hodges · 2026-05-08T13:25:12Z

+import { asKeyStore } from '../../key-generation-utils/jwk';
 import { logger } from '../../logger';
 import { uploadPublicKeystoreToS3 } from '../../key-generation-utils';


Suggested change

import { asKeyStore } from '../../key-generation-utils/jwk';

import { logger } from '../../logger';

import { uploadPublicKeystoreToS3 } from '../../key-generation-utils';

import { logger } from '../../logger';

import { asKeyStore, uploadPublicKeystoreToS3 } from '../../key-generation-utils';

Ian-Hodges · 2026-05-08T13:26:52Z

+import { asKey } from '../../key-generation-utils/jwk';
 import {
  ValidateKeyResult,
  validatePrivateKey,
 } from '../../key-generation-utils';



Suggested change

import {

asKey,

ValidateKeyResult,

validatePrivateKey,

} from '../../key-generation-utils';

gareth-allan · 2026-05-08T14:18:57Z

        uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5
      - name: "Build docs"
-        uses: NHSDigital/nhs-notify-shared-modules/.github/actions/build-docs@3.0.0
+        uses: NHSDigital/nhs-notify-shared-modules/.github/actions/build-docs@53e42b5046ec10ce54d732c0051c96968aaebeb2


This is a change that needs reverting before merge, right? Or updating to point at a new tag.

gareth-allan · 2026-05-08T15:13:08Z


-      const key = await JWK.asKey(keyPem, 'pem');
-      const { kid } = key.toJSON() as KeyJson;
+      await asKey(keyPem, 'pem');


The result of this asKey call isn't being used. Is it redundant? You seem to be getting the Key ID from the SSM parameter name instead?

gareth-allan · 2026-05-08T15:14:57Z

-      const key = await JWK.asKey(keyPem, 'pem');
-      const { kid } = key.toJSON() as KeyJson;
+      await asKey(keyPem, 'pem');
+      const privateKeyRegex = /privatekey_(\d{8})_(.+)\.pem/;


This regex also appears to be defined in validateParamName above. It could perhaps be extracted to a constant, or perhaps wrapped with a utility to use it to extract the components of a key name that can be reused?

gareth-allan · 2026-05-08T15:35:56Z

We should probably have tests for this class, shouldn't we?

gareth-allan · 2026-05-08T15:39:19Z

+    const pem = nodePrivateKey.export({
+      type: 'pkcs8',
+      format: 'pem',
+    }) as string;
+
+    const reImportedKey = createPrivateKey(pem);


Why export nodePrivateKey and then re-import it? I assume there's a reason for it, but it would be worth a comment to explain what it is.

simonlabarere requested a review from a team as a code owner April 30, 2026 09:57

simonlabarere force-pushed the feature/CCM-17440_dependabot_updates branch 5 times, most recently from 0dfc38a to 2e522f2 Compare May 5, 2026 12:19

simonlabarere added the dependencies Pull requests that update a dependency file label May 5, 2026

simonlabarere force-pushed the feature/CCM-17440_dependabot_updates branch 2 times, most recently from 7d404da to 63ce4d2 Compare May 6, 2026 08:42

simonlabarere added 3 commits May 6, 2026 15:04

CCM-17440: Simple dependency updates

5ce6ef1

CCM-17440: Update mermaid dependencies + override uuid to 14.0.0

77d8acc

CCM-17440: node-jose ridance attempt

72c1f93

simonlabarere force-pushed the feature/CCM-17440_dependabot_updates branch from 63ce4d2 to 72c1f93 Compare May 6, 2026 14:05

simonlabarere added 2 commits May 6, 2026 15:22

CCM-17440: Update gitleaks ignore

f86ddf5

CCM-17440: Fix failing unit test following merge

eea0f88

lapenna-bjss previously approved these changes May 7, 2026

View reviewed changes

lapenna-bjss and others added 2 commits May 7, 2026 12:17

Merge branch 'main' into feature/CCM-17440_dependabot_updates

9088c9a

CCM-17440: remove --with-deps flag from playwright install

6247c13

lapenna-bjss dismissed their stale review via 6247c13 May 7, 2026 11:20

lapenna-bjss added 2 commits May 7, 2026 14:42

CCM-17440: Possible build docs fix

ab7707a

CCM-17440: Bump build-docs action to the latest version

6e22d2a

lapenna-bjss requested a review from a team as a code owner May 7, 2026 14:28

lapenna-bjss added 4 commits May 7, 2026 17:32

CCM-17440: Test build-docs fix

ef05815

CCM-17440: Update SHA

d038b0f

CCM-17440: Update SHA

18bbbd2

CCM-17440: Update SHA

1dac457

lapenna-bjss previously approved these changes May 8, 2026

View reviewed changes

Ian-Hodges reviewed May 8, 2026

View reviewed changes

Comment thread utils/utils/src/key-generation-utils/jwk.ts Outdated

Ian-Hodges previously approved these changes May 8, 2026

View reviewed changes

CCM-17440: Address review comments

cb90136

simonlabarere dismissed stale reviews from Ian-Hodges and lapenna-bjss via cb90136 May 8, 2026 10:10

Merge branch 'main' into feature/CCM-17440_dependabot_updates

030a207

Ian-Hodges reviewed May 8, 2026

View reviewed changes

gareth-allan self-assigned this May 8, 2026

gareth-allan reviewed May 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CCM-17440: Fix dependabot issues#321

CCM-17440: Fix dependabot issues#321
simonlabarere wants to merge 15 commits intomainfrom
feature/CCM-17440_dependabot_updates

simonlabarere commented Apr 30, 2026 •

edited by lapenna-bjss

Loading

Uh oh!

Uh oh!

Ian-Hodges May 8, 2026

Uh oh!

Ian-Hodges May 8, 2026

Uh oh!

Ian-Hodges May 8, 2026

Uh oh!

Ian-Hodges May 8, 2026

Uh oh!

Ian-Hodges May 8, 2026

Uh oh!

gareth-allan May 8, 2026

Uh oh!

gareth-allan May 8, 2026

Uh oh!

gareth-allan May 8, 2026

Uh oh!

gareth-allan May 8, 2026

Uh oh!

gareth-allan May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -1,4 +1,4 @@
		import { KeyStore } from '../../key-generation-utils/jwk';
		import { KeyStore } from '../../key-generation-utils/jwk-key-store';

		@@ -0,0 +1,73 @@
		import { Key } from '../../key-generation-utils/jwk-key';

Conversation

simonlabarere commented Apr 30, 2026 • edited by lapenna-bjss Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Running the key-generation lambda manually

Generated private key

Generated public key

APIM Updated to point at pr321

Token generation

Other changes

Type of changes

Checklist

Sensitive Information Declaration

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

simonlabarere commented Apr 30, 2026 •

edited by lapenna-bjss

Loading