WIP: Feature/hote 703/db multi schema v2 by mikeeq · Pull Request #189 · NHSDigital/hometest-service

mikeeq · 2026-03-03T15:08:29Z

Description

Context

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.

…ature/hote-703/db-multi-schema

Copilot

Pull request overview

This PR appears to be evolving the project toward multi-schema Postgres support (via search_path) and adding optional RDS IAM authentication for Lambda DB connections, plus associated dependency/build updates.

Changes:

Add schema-aware DB connectivity in test utilities and introduce DB_SCHEMA env var.
Add RDS IAM auth token support for Lambda Postgres connections (USE_IAM_AUTH / DB_REGION) and include @aws-sdk/rds-signer.
Add schema-provisioning/migration scripts for creating per-environment schemas and running goose migrations.

Reviewed changes

Copilot reviewed 13 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
ui/package-lock.json	Lockfile updates (peer metadata changes).
tests/package-lock.json	Lockfile updates (peer metadata changes).
tests/db/TestResultDbClient.ts	Switch SQL to rely on `search_path` instead of hard-coded `hometest.` schema prefix.
tests/db/BaseDbClient.ts	Configure DB `search_path` via `DB_SCHEMA` (defaults to `hometest`).
tests/configuration/EnvironmentVariables.ts	Add `DB_SCHEMA` env var key.
tests/configuration/.env.local	Local env config updated (currently contains conflict markers).
lambdas/src/lib/db/test-result-db-client.ts	Modify query join in results lookup (currently introduces a bad join).
lambdas/src/lib/db/rds-iam-auth.ts	New helper for generating RDS IAM auth tokens.
lambdas/src/lib/db/db-config.ts	Add IAM-auth-based Postgres config and env-based switch between IAM and Secrets Manager.
lambdas/src/lib/db/db-config.test.ts	Add unit tests for IAM-auth config and IAM mode selection in env config.
lambdas/src/get-results-lambda/init.ts	Minor import ordering change.
lambdas/scripts/build.ts	Update esbuild options/minify/sourcemaps and external deps (currently contains conflict markers).
lambdas/package.json	Add `@aws-sdk/rds-signer` dependency.
lambdas/package-lock.json	Lockfile update to include new AWS SDK dependency tree.
database/db-migrate-schema.sh	New schema-aware migration helper script for provisioning schema + goose + seed.
database/create-schema.sql	New SQL script to create schema and grant privileges.

Files not reviewed (3)

lambdas/package-lock.json: Language not supported
tests/package-lock.json: Language not supported
ui/package-lock.json: Language not supported

Copilot · 2026-03-03T15:13:09Z

lambdas/src/lib/db/db-config.ts

-    ? `-c search_path=${options.schema}`
-    : undefined;
+function buildSearchPathOptions(schema?: string): string | undefined {
+  return schema ? `-c search_path=${schema}` : undefined;


buildSearchPathOptions embeds the provided schema directly into the libpq options string. Since libpq parses options like command-line args, an unexpected schema value containing spaces could inject additional -c settings. Validate/normalize the schema value (e.g., strict identifier or comma-separated identifiers) before building the options string.

Suggested change

return schema ? `-c search_path=${schema}` : undefined;

if (!schema) {

return undefined;

}

const trimmedSchema = schema.trim();

// Allow only strict identifiers or a comma-separated list of them:

// identifier := [A-Za-z_][A-Za-z0-9_]*

// list := identifier (',' identifier)*

// with optional whitespace around commas.

const identifierPattern = "[A-Za-z_][A-Za-z0-9_]*";

const schemaListRegex = new RegExp(

`^${identifierPattern}(\\s*,\\s*${identifierPattern})*$`,

);

if (!schemaListRegex.test(trimmedSchema)) {

throw new Error("Invalid schema name for search_path");

}

return `-c search_path=${trimmedSchema}`;

Copilot · 2026-03-03T15:13:09Z

tests/configuration/.env.local

+<<<<<<< Updated upstream
+API_BASE_URL=http://localhost:4566/_aws/execute-api/qnb74xisbw/local
+=======
+API_BASE_URL=http://localhost:4566/_aws/execute-api/8udhtoyjp0/local
+>>>>>>> Stashed changes


This file contains unresolved Git merge conflict markers (<<<<<<<, =======, >>>>>>>), which will break local env loading and is invalid to commit. Resolve the conflict and keep a single, correct API_BASE_URL value.

Suggested change

<<<<<<< Updated upstream

API_BASE_URL=http://localhost:4566/_aws/execute-api/qnb74xisbw/local

=======

API_BASE_URL=http://localhost:4566/_aws/execute-api/8udhtoyjp0/local

>>>>>>> Stashed changes

API_BASE_URL=http://localhost:4566/_aws/execute-api/qnb74xisbw/local

Copilot · 2026-03-03T15:13:10Z

lambdas/scripts/build.ts

+<<<<<<< Updated upstream
    external: ['aws-sdk', '@aws-sdk/*'],
+=======
+    // The Lambda nodejs24.x runtime provides @aws-sdk/client-* and @aws-sdk/lib-*
+    // packages. @aws-sdk/rds-signer is NOT in the runtime and must be bundled,
+    // along with its transitive deps (@smithy/*, @aws-sdk/credential-providers).
+    external: [
+      'aws-sdk',
+      '@aws-sdk/client-*',
+      '@aws-sdk/lib-*',
+    ],
+>>>>>>> Stashed changes


There are unresolved Git merge conflict markers in the esbuild config (<<<<<<< / ======= / >>>>>>>). Resolve the conflict and ensure the final 'external' list matches the intended bundling behavior before merging.

Copilot · 2026-03-03T15:13:10Z

lambdas/src/lib/db/test-result-db-client.ts

      FROM test_order o
      INNER JOIN patient_mapping p ON p.patient_uid = o.patient_uid
-      INNER JOIN result_status rs ON rs.order_uid = o.order_uid
+      INNER JOIN result_status rs ON o.order_uid = o.order_uid


The join condition is incorrect: ON o.order_uid = o.order_uid is always true and turns this into a Cartesian join, which can return incorrect rows (or duplicates) and hurt performance. Join result_status rs on rs.order_uid = o.order_uid as before.

Suggested change

INNER JOIN result_status rs ON o.order_uid = o.order_uid

INNER JOIN result_status rs ON rs.order_uid = o.order_uid

Copilot · 2026-03-03T15:13:10Z

database/create-schema.sql

+CREATE SCHEMA IF NOT EXISTS :schema_name;
+
+-- Make the migration user own the schema
+ALTER SCHEMA :schema_name OWNER TO app_migrator;
+
+-- Grant schema-level privileges to app_migrator
+GRANT CREATE, USAGE ON SCHEMA :schema_name TO app_migrator;
+
+-- Grant DML privileges on existing objects
+GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER
+ON ALL TABLES IN SCHEMA :schema_name TO app_migrator;
+
+GRANT USAGE, SELECT, UPDATE
+ON ALL SEQUENCES IN SCHEMA :schema_name TO app_migrator;
+
+-- Auto-grant privileges on future tables/sequences for app_migrator
+ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name
+GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER
+ON TABLES TO app_migrator;
+
+ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name
+GRANT USAGE, SELECT, UPDATE
+ON SEQUENCES TO app_migrator;
+
+-- Grant schema privileges to app_user
+GRANT USAGE ON SCHEMA :schema_name TO app_user;
+
+GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER
+ON ALL TABLES IN SCHEMA :schema_name TO app_user;
+
+GRANT USAGE, SELECT, UPDATE
+ON ALL SEQUENCES IN SCHEMA :schema_name TO app_user;
+
+-- Auto-grant privileges on future tables/sequences for app_user
+ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name
+GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER
+ON TABLES TO app_user;
+
+ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name
+GRANT USAGE, SELECT, UPDATE
+ON SEQUENCES TO app_user;
+
+-- Grant admin access
+GRANT CREATE, USAGE ON SCHEMA :schema_name TO admin;


The psql variable substitution uses :schema_name unquoted in identifier positions (CREATE/ALTER/GRANT). This allows SQL injection if schema_name contains malicious characters and will also break for schema names that need quoting. Use psql’s identifier-quoting form (e.g. :"schema_name") or otherwise safely quote/validate the schema name.

Suggested change

CREATE SCHEMA IF NOT EXISTS :schema_name;

-- Make the migration user own the schema

ALTER SCHEMA :schema_name OWNER TO app_migrator;

-- Grant schema-level privileges to app_migrator

GRANT CREATE, USAGE ON SCHEMA :schema_name TO app_migrator;

-- Grant DML privileges on existing objects

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON ALL TABLES IN SCHEMA :schema_name TO app_migrator;

GRANT USAGE, SELECT, UPDATE

ON ALL SEQUENCES IN SCHEMA :schema_name TO app_migrator;

-- Auto-grant privileges on future tables/sequences for app_migrator

ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON TABLES TO app_migrator;

ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name

GRANT USAGE, SELECT, UPDATE

ON SEQUENCES TO app_migrator;

-- Grant schema privileges to app_user

GRANT USAGE ON SCHEMA :schema_name TO app_user;

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON ALL TABLES IN SCHEMA :schema_name TO app_user;

GRANT USAGE, SELECT, UPDATE

ON ALL SEQUENCES IN SCHEMA :schema_name TO app_user;

-- Auto-grant privileges on future tables/sequences for app_user

ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON TABLES TO app_user;

ALTER DEFAULT PRIVILEGES IN SCHEMA :schema_name

GRANT USAGE, SELECT, UPDATE

ON SEQUENCES TO app_user;

-- Grant admin access

GRANT CREATE, USAGE ON SCHEMA :schema_name TO admin;

CREATE SCHEMA IF NOT EXISTS :"schema_name";

-- Make the migration user own the schema

ALTER SCHEMA :"schema_name" OWNER TO app_migrator;

-- Grant schema-level privileges to app_migrator

GRANT CREATE, USAGE ON SCHEMA :"schema_name" TO app_migrator;

-- Grant DML privileges on existing objects

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON ALL TABLES IN SCHEMA :"schema_name" TO app_migrator;

GRANT USAGE, SELECT, UPDATE

ON ALL SEQUENCES IN SCHEMA :"schema_name" TO app_migrator;

-- Auto-grant privileges on future tables/sequences for app_migrator

ALTER DEFAULT PRIVILEGES IN SCHEMA :"schema_name"

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON TABLES TO app_migrator;

ALTER DEFAULT PRIVILEGES IN SCHEMA :"schema_name"

GRANT USAGE, SELECT, UPDATE

ON SEQUENCES TO app_migrator;

-- Grant schema privileges to app_user

GRANT USAGE ON SCHEMA :"schema_name" TO app_user;

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON ALL TABLES IN SCHEMA :"schema_name" TO app_user;

GRANT USAGE, SELECT, UPDATE

ON ALL SEQUENCES IN SCHEMA :"schema_name" TO app_user;

-- Auto-grant privileges on future tables/sequences for app_user

ALTER DEFAULT PRIVILEGES IN SCHEMA :"schema_name"

GRANT SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER

ON TABLES TO app_user;

ALTER DEFAULT PRIVILEGES IN SCHEMA :"schema_name"

GRANT USAGE, SELECT, UPDATE

ON SEQUENCES TO app_user;

-- Grant admin access

GRANT CREATE, USAGE ON SCHEMA :"schema_name" TO admin;

Copilot · 2026-03-03T15:13:11Z

database/db-migrate-schema.sh

+DB_URL="postgresql://${MIGRATOR_USER}:${MIGRATOR_PASSWORD}@${DB_HOST}:5432/${LOCAL_DB}?search_path=${SCHEMA_NAME}"
+
+export PGHOST="$DB_HOST"
+
+echo "Starting database migration for schema: ${SCHEMA_NAME}..."
+
+# Step 1: Create schema and grant permissions (as admin)
+export PGPASSWORD="$ADMIN_PASSWORD"
+export PGUSER="$ADMIN_USER"
+
+echo "Step 1: Creating schema '${SCHEMA_NAME}' and granting permissions..."
+psql $PSQL_OPTIONS -d "$LOCAL_DB" -v schema_name="${SCHEMA_NAME}" -f "$SQL_DIR/create-schema.sql"
+
+# Step 2: Run goose migrations (as migrator, with search_path set to target schema)
+export PGPASSWORD="$MIGRATOR_PASSWORD"
+export PGUSER="$MIGRATOR_USER"
+
+echo "Step 2: Running goose migrations against schema '${SCHEMA_NAME}'..."
+goose -dir "$SQL_DIR/migrations" postgres "$DB_URL" up
+
+# Step 3: Load seed data (as migrator, with search_path)
+echo "Step 3: Loading seed data into schema '${SCHEMA_NAME}'..."
+psql $PSQL_OPTIONS -d "$LOCAL_DB" -c "SET search_path TO ${SCHEMA_NAME};" -f "$SQL_DIR/03-seed-hometest-data.sql"


Schema name is interpolated into the goose connection string and later into a SET search_path TO ... command without validation/quoting. This can be exploited (or just break) if SCHEMA_NAME contains spaces/quotes/semicolons. Validate SCHEMA_NAME against a safe identifier regex and quote it when used in SQL (e.g., via psql variable quoting).

Copilot · 2026-03-03T15:13:11Z

tests/db/BaseDbClient.ts

+    const schema = process.env[EnvironmentVariables.DB_SCHEMA] ?? 'hometest';
    this.client = new Client({
      host:     process.env[EnvironmentVariables.DB_HOST]     ?? 'localhost',
      port:     parseInt(process.env[EnvironmentVariables.DB_PORT] ?? '5432', 10),
      database: process.env[EnvironmentVariables.DB_NAME]     ?? 'local_hometest_db',
      user:     process.env[EnvironmentVariables.DB_USER]     ?? 'admin',
      password: process.env[EnvironmentVariables.DB_PASSWORD] ?? 'admin',
+      options:  `-c search_path=${schema}`,
    });


schema is taken directly from an environment variable and embedded into the libpq options string. Because options is parsed like command-line flags, a crafted value (e.g. containing spaces and another -c ...) could inject extra connection options. Consider validating schema (allow only safe identifier characters / comma-separated identifiers) before constructing the options string.

mikeeq added 8 commits March 2, 2026 11:48

HOTE-703: Add support for multiple schemas

a54e9b9

Merge branch 'main' of github.com:NHSDigital/hometest-service into fe…

c2adbe8

…ature/hote-703/db-multi-schema

remove duplicated import

20425fc

Merge branch 'main' of github.com:NHSDigital/hometest-service into fe…

c9feb61

…ature/hote-703/db-multi-schema

Merge branch 'main' of github.com:NHSDigital/hometest-service into fe…

7340991

…ature/hote-703/db-multi-schema

Add iam auth support v2

3ab0178

Merge branch 'main' of github.com:NHSDigital/hometest-service into fe…

c7feb18

…ature/hote-703/db-multi-schema

Add prod optimizations

72d0384

Copilot AI review requested due to automatic review settings March 3, 2026 15:08

Copilot started reviewing on behalf of mikeeq March 3, 2026 15:09 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Feature/hote 703/db multi schema v2#189

WIP: Feature/hote 703/db multi schema v2#189
mikeeq wants to merge 8 commits intomainfrom
feature/hote-703/db-multi-schema-v2

mikeeq commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Copilot AI Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-  return schema ? `-c search_path=${schema}` : undefined;
+  if (!schema) {
+    return undefined;
+  }
+  const trimmedSchema = schema.trim();
+  // Allow only strict identifiers or a comma-separated list of them:
+  //   identifier := [A-Za-z_][A-Za-z0-9_]*
+  //   list       := identifier (',' identifier)*
+  // with optional whitespace around commas.
+  const identifierPattern = "[A-Za-z_][A-Za-z0-9_]*";
+  const schemaListRegex = new RegExp(
+    `^${identifierPattern}(\\s*,\\s*${identifierPattern})*$`,
+  );
+  if (!schemaListRegex.test(trimmedSchema)) {
+    throw new Error("Invalid schema name for search_path");
+  }
+  return `-c search_path=${trimmedSchema}`;

	INNER JOIN result_status rs ON o.order_uid = o.order_uid
	INNER JOIN result_status rs ON rs.order_uid = o.order_uid

Conversation

mikeeq commented Mar 3, 2026

Description

Context

Type of changes

Checklist

Sensitive Information Declaration

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants