NR-929: implementing the kainos npm security measurements by sekhar-kainos · Pull Request #205 · UKForeignOffice/notarial-api

sekhar-kainos · 2026-09-11T09:06:28Z

Implement Kainos npm security measures: notarial-api
JIRA: https://consular.atlassian.net/browse/NR-929

changes description:

Migrated dependency management from Yarn to npm workspaces.
Added npm security configuration to restrict lifecycle scripts and Git dependencies.
Added moderate-severity dependency auditing to the CircleCI pipeline.
Updated API and worker Docker builds to use secure npm installation.
Upgraded Node.js to version 24 across CI, releases, and containers.

pipeline on this feature branch - circle -ci

marc-temp · 2026-09-17T15:08:31Z

+    "build": "tsc",
    "start": "node dist/index.js",
-    "start:local": "yarn tsc --watch & node --watch dist/index.js",
+    "start:local": "tsc --watch & node --watch dist/index.js",


Thinking these need npx to run them from Node's install of tsc

marc-temp · 2026-09-17T15:09:20Z

    "build": "tsc",
    "start": "node dist/index.js",
-    "start:local": "yarn tsc -w & node --watch dist/index.js",
+    "start:local": "tsc -w & node --watch dist/index.js",


Same here

Need npx

marc-temp · 2026-09-17T15:10:48Z

+min-release-age=3
+engine-strict=true
+allow-git=none
+audit=true


Align with our service's

https://github.com/UKForeignOffice/notarial-marriage-frontend/pull/327/changes#diff-e813096d69c49812e40e00be9ac8fa14d77f0ec1f97ab4c45cb096b3bedd35de

Namely we can't have min-release-age and allow-git until we upgrade to Node v26

marc-temp · 2026-09-17T15:11:47Z

+- Use `npm install` only when intentionally changing dependencies, then review the `package-lock.json` diff.
+- Run application builds explicitly with `npm run build` after dependencies are installed.
+- Run `npm run security` for the full audit used by CI or `npm run security:production` to check production dependencies only.
+- The `qs@6.16.0` override addresses current Express 4 advisories and crosses Express's declared `~6.15.1` range. Verify API tests before changing or removing it.


Few extra bullets in here that shouldn't be

Again, align with our service's

marc-temp · 2026-09-17T15:17:15Z

+RUN npm run build --workspace @notarialapi/worker
 # reinstall only production deps
-RUN yarn workspaces focus @notarialapi/worker --production
+RUN npm prune --omit=dev --ignore-scripts=true


Shouldn't need --ignore-scripts here

We've already installed all packages without

Unless we do and we haven't done this correctly in our service?

marc-temp · 2026-09-17T15:25:23Z

+    "@typescript-eslint/eslint-plugin": "^8.70.0",
+    "@typescript-eslint/parser": "^8.70.0",
    "babel-jest": "^29.7.0",
    "eslint": "^8.56.0",


Run npm outdated --min-release-age 5

Install safe package increases

Would recommend doing them in groups per commit so it's easy to revert any given

I.e.

eslint 8.57.1 8.57.1 10.10.0 node_modules/eslint api@npm:@notarialapi/api@1.0.0 eslint 8.57.1 8.57.1 10.10.0 node_modules/eslint notarial-api eslint-config-prettier 9.1.2 9.1.2 10.1.8 node_modules/eslint-config-prettier api@npm:@notarialapi/api@1.0.0 eslint-config-prettier 9.1.2 9.1.2 10.1.8 node_modules/eslint-config-prettier notarial-api

Would do these ESLints together across all that have it

Also, I'm 99% there shouldn't be any pkgs in this root folder with exception of eslint and prettier

But I think these are just leftovers from moving them into workspaces

One to follow up with Simon and co to see what we can tidy up

marc-temp · 2026-09-18T11:01:34Z

  },
  "scripts": {
-    "build": "yarn tsc",
+    "build": "tsc",


Missing npx

marc-temp

Looks fine to me

Will need supporting evidence of testing to ensure no regression and works BAU

Would ofc want Simon/Conor opinion/testing on this too

Copilot

🟡 Changes recommended

Critical Docker build failures and unresolved npm/documentation migration issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Migrates the monorepo from Yarn to npm workspaces, adds npm security auditing, and upgrades tooling to Node.js 24.

Changes:

Replaced Yarn commands and configuration with npm equivalents.
Added npm security settings and dependency auditing.
Updated CI, release workflows, Docker builds, documentation, and formatting configuration.

File summaries

File	Summary
`worker/src/queues/ses/helpers/SESEmail.ts`	Formatting update.
`worker/package.json`	npm scripts and dependency updates.
`worker/Dockerfile`	Critical (3 votes): stale `.yarn` copy causes the Docker build to fail.
`worker/.prettierrc`	Formatting update.
`README.md`	npm workflow and security documentation updates.
`package.json`	Moderate (1 vote): port Yarn resolutions to npm `overrides` and regenerate the lockfile.
`eslint.config.mjs`	Flat ESLint configuration.
`dynamic-content/README.md`	npm usage instructions.
`dynamic-content/babel.config.js`	Formatting update.
`dynamic-content/.prettierrc`	Formatting update.
`docs/testing.md`	Nit (3 votes): replace unsupported Yarn Cypress and smoke-test commands.
`api/src/middlewares/services/UserService/UserService.ts`	Formatting update.
`api/src/middlewares/services/UserService/NotifyTemplates/MarriageUserTemplates.ts`	Formatting update.
`api/src/middlewares/services/UserService/NotifyTemplates/CertifyCopyUserTemplates.ts`	Formatting update.
`api/src/middlewares/services/UserService/__tests__/userService.test.ts`	Formatting update.
`api/src/middlewares/services/SubmitService/SubmitService.ts`	Formatting update.
`api/src/middlewares/services/QueueService/QueueService.ts`	Formatting update.
`api/src/middlewares/services/CaseService/requestDocument/requestDocumentCaseService.ts`	Formatting update.
`api/src/middlewares/services/CaseService/certifyCopy/CertifyCopyCaseService.ts`	Formatting update.
`api/README.md`	Node.js and npm setup updates.
`api/package.json`	npm scripts and dependency updates.
`api/Dockerfile`	Critical (3 votes): stale `.yarn` copy causes the Docker build to fail.
`.yarnrc.yml`	Removed Yarn configuration.
`.yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs`	Removed Yarn workspace plugin.
`.prettierrc`	Formatting configuration update.
`.prettierignore`	Formatting exclusions.
`.npmrc`	npm security configuration.
`.gitignore`	Tracks the npm lockfile.
`.github/workflows/release.yml`	Node.js 24 and npm release updates.
`.eslintrc.json`	Removed legacy ESLint configuration.
`.dockerignore`	Removed Yarn-specific exclusions.
`.circleci/config.yml`	npm installation, auditing, and Node.js 24 updates.

Review details

Suppressed comments (2)

api/Dockerfile:17

The npm workspace install records API-only dependencies such as joi@18.2.9 under api/node_modules, while the root lock tree also contains joi@17.13.7 for dynamic-content. This image copies only /usr/src/app/node_modules, then relocates the compiled API to /usr/dist/app/dist, so the API's import joi either resolves the wrong major or cannot resolve it at runtime. Copy/flatten the API workspace's production node_modules into the runtime tree, or build a production install layout that preserves the API dependency tree.

RUN npm ci --ignore-scripts=true --workspace @notarialapi/api --include-workspace-root

package.json:35

This migration removes the Yarn resolutions block but does not add npm overrides for its transitive security pins. Although the current lockfile records versions, a later intentional npm install can re-resolve packages such as follow-redirects, qs, or tar without those constraints; port the required pins to overrides and regenerate the lockfile.

    "semantic-release": "^25.0.9"

Files reviewed: 25/35 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 COPY . .
-RUN yarn workspaces focus @notarialapi/api
-RUN yarn api build
+RUN npm ci --ignore-scripts=true --workspace @notarialapi/api --include-workspace-root


-RUN yarn worker build
-# reinstall only production deps
-RUN yarn workspaces focus @notarialapi/worker --production
+RUN npm ci --ignore-scripts=true --workspace @notarialapi/worker --include-workspace-root


 ### running unit tests
 Unit tests will be run every time a new commit is pushed to any branch, however you can also, and are encouraged to, run the tests locally before pushing up your changes.
-To run unit tests across the entire repo, execute the command `yarn test`. This will then delegate the command to each workspace independently, and have each workspace execute its own tests.
+To run unit tests across the entire repo, execute the command `npm test`. This will then delegate the command to each workspace independently, and have each workspace execute its own tests.


marc-temp

Need to leave this PR open and point to main branch

sekhar-kainos force-pushed the feature/NR-929_npm-security-measurement-implementation branch 2 times, most recently from 679c900 to d5f5e87 Compare September 15, 2026 14:04

sekhar-kainos requested review from ethanh90 and simonmoore100 September 15, 2026 14:23

sekhar-kainos marked this pull request as ready for review September 15, 2026 14:23

sekhar-kainos force-pushed the feature/NR-929_npm-security-measurement-implementation branch from d5f5e87 to 00cae2e Compare September 17, 2026 14:02

marc-temp reviewed Sep 17, 2026

View reviewed changes

Comment thread worker/package.json

marc-temp reviewed Sep 17, 2026

View reviewed changes

marc-temp reviewed Sep 18, 2026

View reviewed changes

Comment thread api/package.json Outdated

},

"scripts": {

"build": "yarn tsc",

"build": "tsc",

marc-temp Sep 18, 2026

Copy link
Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

Missing npx

marc-temp requested changes Sep 18, 2026

View reviewed changes

marc-temp requested a lite review from Copilot September 18, 2026 11:13

Copilot started reviewing on behalf of marc-temp September 18, 2026 11:14 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

sekhar-kainos changed the base branch from main to deploy-test September 18, 2026 13:02

sekhar-kainos changed the base branch from deploy-test to deploy-dev September 18, 2026 13:07

sekhar-kainos changed the base branch from deploy-dev to main September 18, 2026 13:25

sekhar-kainos force-pushed the feature/NR-929_npm-security-measurement-implementation branch from a4480c9 to bb87812 Compare September 18, 2026 13:25

NR-929: implementing the kainos npm security measurements

69bf353

sekhar-kainos force-pushed the feature/NR-929_npm-security-measurement-implementation branch from bb87812 to 69bf353 Compare September 18, 2026 13:26

sekhar-kainos changed the base branch from main to deploy-test September 18, 2026 13:26

sekhar-kainos merged commit 69bf353 into deploy-test Sep 21, 2026
1 check passed

marc-temp reviewed Sep 21, 2026

View reviewed changes

Navigation Menu

NR-929: implementing the kainos npm security measurements - #205

NR-929: implementing the kainos npm security measurements#205
sekhar-kainos merged 1 commit into
deploy-testfrom
feature/NR-929_npm-security-measurement-implementation

sekhar-kainos commented Sep 11, 2026 •

edited

Loading

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

marc-temp Sep 17, 2026

Uh oh!

marc-temp Sep 18, 2026

Uh oh!

marc-temp left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

marc-temp left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sekhar-kainos commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

marc-temp left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

marc-temp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sekhar-kainos commented Sep 11, 2026 •

edited

Loading