feat(NODE-5393): Migrate AWS signature v4 logic into driver #4824

PavelSafronov · 2025-12-12T22:06:09Z

Description

Summary of Changes

Replace optional dependency on aws4 package with a minimal equivalent implementation.

What is the motivation for this change?

This helps us reduce our runtime dependencies, as part of https://jira.mongodb.org/browse/NODE-6601

Release Highlight

Replace optional dependency on aws4 package with a minimal equivalent implementation

This reduces the number of optional dependencies.

Double check the following

Lint is passing (npm run check:lint)
Self-review completed using the steps outlined here
PR title follows the correct format: type(NODE-xxxx)[!]: description
- Example: feat(NODE-1234)!: rewriting everything in coffeescript
Changes are covered by tests
New TODOs have a related JIRA ticket

…ompare the output with the old aws4 library

baileympearson · 2025-12-16T21:17:33Z

src/aws4.ts

+  const date = options.date || new Date();
+  const requestDateTime = date.toISOString().replace(/[:-]|\.\d{3}/g, '');
+  const requestDate = requestDateTime.substring(0, 8);


This is sort of difficult to parse but seems like it's constructing an ISO 8601 date with no timezone or hyphens?

Maybe we could either comment this, or just move it into an appropriately named function to make it clear

(https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv-signing-elements.html#date <- I found these docs)

Added comments to the logic and called out each variable that is relevant to the logic.

baileympearson · 2025-12-16T21:26:28Z

src/aws4.ts

+export type AwsSessionCredentials = {
+  accessKeyId: string;
+  secretAccessKey: string;
+  sessionToken: string;
+};
+
+export type AwsLongtermCredentials = {
+  accessKeyId: string;
+  secretAccessKey: string;
+};


Could we reuse our existing AWSCredentials type instead?

Absolutely, removing these unnecessary types.

baileympearson · 2025-12-16T21:31:19Z

src/aws4.ts

+  };
+};
+
+const convertHeaderValue = (value: string | number) => {


Is this function URI encoding the header value? Could we just use encodeUriComponent instead?

Nope, this is not URI encoding, this is replacing consecutive spaces with a single space.

I'll add comments for all of these methods.

baileympearson · 2025-12-16T21:33:07Z

src/aws4.ts

+  };
+  service: string;
+  region: string;
+  date?: Date;


Is this only exposed so we can unit test sigv4?

Yup. Do we have a different way to override new Date() in tests?

baileympearson · 2025-12-16T21:33:36Z

src/aws4.ts

+  };
+};
+
+export interface AWS4 {


is there a reason you can think of to continue using this type? I'm not sure it's necessary anymore, now that we've taken on ownership of this function.

Given that all of this is internal and specific to just a single call, yeah, we can drop this interface.

baileympearson · 2025-12-16T21:34:52Z

src/aws4.ts

+  const requestDateTime = date.toISOString().replace(/[:-]|\.\d{3}/g, '');
+  const requestDate = requestDateTime.substring(0, 8);
+
+  const headers: string[] = [


Could we make this a bit simpler if we used Headers?

something like:

const headers = new Headers({ 'content-length': ..., 'content-type': ..., ... });

That'd make things further down a bit nicer, ex:

const signedHeaders = Array.from(headers.keys()).join(';');

Simplifies things a bit, using it.

baileympearson · 2025-12-16T21:35:15Z

src/aws4.ts

+  return value.toString().trim().replace(/\s+/g, ' ');
+};
+
+export function aws4Sign(


Could you add some comments and/or docs about where the algorithm here is defined? Just to make it easier to maintain for future reviewers and devs.

Added comments and references.

baileympearson · 2025-12-16T21:36:50Z

test/integration/auth/aws4.test.ts

+// This is done by calculating a signature, then using it to make a real request to the AWS STS service.
+// To run this test, simply run `./etc/aws-test.sh`.
+
+describe('AwsSigV4', function () {


Instead of a separate test suite, what if we just included these tests in the existing MongoDB AWS test suite and then only ran them if we are running in AWS ECS?

That seems like a good approach, looking into it.

Removed the new tests and added the signing tests to an existing test.

baileympearson · 2025-12-16T21:39:41Z

etc/aws-test.sh

@@ -0,0 +1,24 @@
+#!/usr/bin/env bash
+
+cd $DRIVERS_TOOLS/.evergreen/auth_aws


If we do decide to leave these tests in their own test suite, can we break this into three separate tasks?

That is the approach we generally prefer because each test run produces an xunit file. This means only the last test run's results are actually uploaded to evergreen

Removed this script.

baileympearson · 2025-12-16T21:40:05Z

src/aws4.ts

+  this: void,
+  options: Options,
+  credentials: AwsSessionCredentials | AwsLongtermCredentials | undefined
+): SignedHeaders {


Suggested change

): SignedHeaders {

export function aws4Sign(

options: Options,

credentials: AwsSessionCredentials | AwsLongtermCredentials | undefined

): SignedHeaders {

this isn't necessary

addaleax · 2025-12-17T13:18:49Z

src/aws4.ts

+const getHash = (str: string): string => {
+  return crypto.createHash('sha256').update(str, 'utf8').digest('hex');
+};
+const getHmacArray = (key: string | Uint8Array, str: string): Uint8Array => {


A name like getHmacArray would imply to me that this returns an actual Array, not a typed array – if you want to keep the brevity, I'd suggest something like getHmacBuffer, which, even if it doesn't return an actual Buffer, still communicates the return type better (or just have it be verbose and call it getHmacUint8Array)

Good point, renamed to getHmacBuffer.

addaleax · 2025-12-17T13:19:42Z

src/aws4.ts

+};
+const getHmacString = (key: Uint8Array, str: string): string => {
+  return crypto.createHmac('sha256', key).update(str, 'utf8').digest('hex');
+};


Can we use the webcrypto API for this? It's supported as a global in all versions of Node.js and browsers that we target, right?

Seems like webcrypto may be more limited:

Secure context: This feature is available only in secure contexts (HTTPS), in some or all supporting browsers.

We weren't planning on moving off crypto yet, though @tadjik1 may have some thoughts on this going forward.

like webcrypto may be more limited:

Well, node:crypto only works in Node.js, webcrypto works in Node.js and almost all browser contexts – that means it's definitely less limited than the current solution

addaleax · 2025-12-17T13:22:28Z

src/deps.ts

-  let aws4: AWS4 | { kModuleError: MongoMissingDependencyError };
-  try {
-    // eslint-disable-next-line @typescript-eslint/no-require-imports
-    aws4 = require('aws4');


Let's make sure to mark the ticket as having DevTools impact (we can now also remove this dependency)

aws4 was used as an optional dependency, so we weren't directly referencing it in the driver.

Right, but we are referencing it in devtools precisely because it's optional for the driver, but not for us :) Not a big thing, just a reminder that we need to do this

Oh, I hadn't realized that. Can you point me to the devtools repo? Don't think I've seen that one yet.

Like it is used here in mongosh: https://github.com/mongodb-js/mongosh/blob/e26f538b5f0b0c40b7f7643695cc0edd73372ef6/packages/service-provider-node-driver/package.json#L54

Anna's referring to in JIRA we have a field for "dev tools changes needed" that will auto file an attached ticket so that our team can do whatever follow up there might be, in this case, some welcome dep clean up :)

Ah, gotcha, still trying to figure out what's automated (well done, btw!) and what we have to update manually.

- removed extraneous new types - removed unnecessary AWS4 interface - getHmacArray renamed - removed unnecessary env-reading code - added a bunch of comments about the sigv4 algorithm - removed tests that did not pass in any credentials, we never do this

…s test

PavelSafronov added 3 commits December 11, 2025 14:14

sigv4 implementation

7cc1156

fix issues and add a test, and a simple way to run it

811d453

added unit tests for the signing logic, added comments about how to c…

a0ba1ec

…ompare the output with the old aws4 library

PavelSafronov changed the title ~~WIP: feat(NODE-5393): Migrate AWS signature v4 logic into driver~~ feat(NODE-5393): Migrate AWS signature v4 logic into driver Dec 15, 2025

Merge branch 'main' into NODE-5393

449d677

PavelSafronov marked this pull request as ready for review December 15, 2025 20:36

PavelSafronov requested a review from a team as a code owner December 15, 2025 20:36

added test for undefined credentials

a44f3b4

baileympearson self-assigned this Dec 16, 2025

baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Dec 16, 2025

baileympearson requested changes Dec 16, 2025

View reviewed changes

addaleax reviewed Dec 17, 2025

View reviewed changes

PavelSafronov added 3 commits December 17, 2025 09:48

pr feedback:

221044d

- removed extraneous new types - removed unnecessary AWS4 interface - getHmacArray renamed - removed unnecessary env-reading code - added a bunch of comments about the sigv4 algorithm - removed tests that did not pass in any credentials, we never do this

removed extraneous integ test and moved its logic into an existing aw…

72ab61d

…s test

minor fixes

037bcf8

		@@ -0,0 +1,24 @@
		#!/usr/bin/env bash

		cd $DRIVERS_TOOLS/.evergreen/auth_aws

feat(NODE-5393): Migrate AWS signature v4 logic into driver #4824

Are you sure you want to change the base?

feat(NODE-5393): Migrate AWS signature v4 logic into driver #4824

Conversation

PavelSafronov commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary of Changes

What is the motivation for this change?

Release Highlight

Replace optional dependency on aws4 package with a minimal equivalent implementation

Double check the following

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

PavelSafronov commented Dec 12, 2025 •

edited

Loading