Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.template
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,7 @@ SERVER_PRIVATE_KEY=
# Stalwart webhook (ingest events)
STALWART_WEBHOOK_USERNAME=stalwart
STALWART_WEBHOOK_SECRET=

# MTA hooks (RCPT)
MTA_HOOKS_USERNAME=stalwart
MTA_HOOKS_SECRET=
2 changes: 2 additions & 0 deletions src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { GatewayModule } from './modules/gateway/gateway.module';
import { HttpGlobalExceptionFilter } from './common/filters/http-global-exception.filter';
import { AddressesModule } from './modules/addresses/addresses.module';
import { StalwartEventsModule } from './modules/stalwart-events/stalwart-events.module';
import { MtaHooksModule } from './modules/mta-hooks/mta-hooks.module';

@Module({
imports: [
Expand Down Expand Up @@ -89,6 +90,7 @@ import { StalwartEventsModule } from './modules/stalwart-events/stalwart-events.
AddressesModule,
GatewayModule,
StalwartEventsModule,
MtaHooksModule,
],
controllers: [],
providers: [
Expand Down
5 changes: 5 additions & 0 deletions src/config/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,9 @@ export default () => ({
username: process.env.STALWART_WEBHOOK_USERNAME ?? 'stalwart',
secret: process.env.STALWART_WEBHOOK_SECRET ?? '',
},

mtaHooks: {
username: process.env.MTA_HOOKS_USERNAME ?? 'stalwart',
secret: process.env.MTA_HOOKS_SECRET ?? '',
},
Comment on lines +57 to +60

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the auth guard rejects empty secrets
rg -n 'secret' src/modules/mta-hooks/mta-hooks-auth.guard.ts -C3

Repository: internxt/mail-server

Length of output: 875


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== configuration.ts ==\n'
cat -n src/config/configuration.ts | sed -n '45,75p'

printf '\n== mta-hooks auth guard ==\n'
cat -n src/modules/mta-hooks/mta-hooks-auth.guard.ts | sed -n '1,120p'

printf '\n== search for mtaHooks.secret validation/usages ==\n'
rg -n "mtaHooks\.secret|MTA_HOOKS_SECRET|mtaHooks" src -C 2

Repository: internxt/mail-server

Length of output: 5316


Reject empty MTA_HOOKS_SECRET values
If MTA_HOOKS_SECRET is unset, the guard compares against '', so a Basic auth value of stalwart: will satisfy the check and expose the MTA hooks endpoint. Fail startup or reject requests when the secret is empty.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config/configuration.ts` around lines 57 - 60, The MTA hooks Basic auth
check in configuration handling currently allows an empty secret, which can let
`stalwart:` pass as valid credentials. Update the logic around `mtaHooks` in
`configuration.ts` and the related startup/request guard so `MTA_HOOKS_SECRET`
must be non-empty: fail startup or reject access when the secret resolves to an
empty string, and ensure the check uses the `MTA_HOOKS_SECRET`/`mtaHooks` values
consistently.

});
43 changes: 43 additions & 0 deletions src/modules/infrastructure/bridge/bridge.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,47 @@ describe('BridgeClient', () => {
expect(error.details).toBe('entry not found');
});
});

describe('getUserUsage', () => {
it('when Bridge returns 200, then signs a gateway token, GETs the usage, and returns the snapshot', async () => {
const snapshot = { maxSpaceBytes: 5000, totalUsedSpaceBytes: 1200 };
jwtService.sign.mockReturnValue('signed-jwt');
httpRequest.mockResolvedValue({
statusCode: 200,
body: { text: () => Promise.resolve(JSON.stringify(snapshot)) },
});

const result = await service.getUserUsage('user-1');

expect(result).toStrictEqual(snapshot);
expect(httpRequest).toHaveBeenCalledWith(
expect.objectContaining({
method: 'GET',
path: '/v2/gateway/users/user-1/usage',
headers: expect.objectContaining({
authorization: 'Bearer signed-jwt',
}) as unknown,
}),
);
});

it('when Bridge returns a non-200 status, then throws BridgeApiError with statusCode and details', async () => {
jwtService.sign.mockReturnValue('signed-jwt');
httpRequest.mockResolvedValue({
statusCode: 500,
body: { text: () => Promise.resolve('internal error') },
});

const error: unknown = await service
.getUserUsage('user-1')
.catch((e: unknown) => e);

expect(error).toBeInstanceOf(BridgeApiError);
if (!(error instanceof BridgeApiError)) {
throw new Error('expected BridgeApiError');
}
expect(error.statusCode).toBe(500);
expect(error.details).toBe('internal error');
});
});
});
31 changes: 30 additions & 1 deletion src/modules/infrastructure/bridge/bridge.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import {
import { ConfigService } from '@nestjs/config';
import { JwtService } from '@nestjs/jwt';
import { Client } from 'undici';
import type { BucketEntry, MailBucket } from './bridge.types.js';
import type {
BucketEntry,
MailBucket,
UserSpaceSnapshot,
} from './bridge.types.js';

@Injectable()
export class BridgeClient implements OnModuleInit, OnModuleDestroy {
Expand Down Expand Up @@ -157,6 +161,31 @@ export class BridgeClient implements OnModuleInit, OnModuleDestroy {
}
}

async getUserUsage(userUuid: string): Promise<UserSpaceSnapshot> {
const token = this.signGatewayToken(userUuid);

const { statusCode, body } = await this.httpClient.request({
method: 'GET',
path: `${this.basePath}/v2/gateway/users/${encodeURIComponent(userUuid)}/usage`,
headers: {
accept: 'application/json',
authorization: `Bearer ${token}`,
},
});

const text = await body.text();

if (statusCode !== 200) {
throw new BridgeApiError(
`Failed to fetch usage for user '${userUuid}': HTTP ${statusCode}`,
statusCode,
text,
);
}

return JSON.parse(text) as UserSpaceSnapshot;
}

private signGatewayToken(userUuid: string): string {
return this.jwtService.sign(
{ payload: { uuid: userUuid } },
Expand Down
72 changes: 72 additions & 0 deletions src/modules/mta-hooks/mta-hooks-auth.guard.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { createMock } from '@golevelup/ts-vitest';
import { type ConfigService } from '@nestjs/config';
import { UnauthorizedException, type ExecutionContext } from '@nestjs/common';
import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js';

const username = 'stalwart';
const secret = 'secret';

describe('MtaHooksAuthGuard', () => {
let guard: MtaHooksAuthGuard;

const contextWithAuth = (header?: string): ExecutionContext =>
createMock<ExecutionContext>({
switchToHttp: () => ({
getRequest: () => ({
headers: header ? { authorization: header } : {},
}),
}),
});

const basic = (username: string, secret: string): string =>
`Basic ${Buffer.from(`${username}:${secret}`).toString('base64')}`;

beforeEach(() => {
const configService = createMock<ConfigService>();
configService.getOrThrow.mockImplementation((key: string) => {
if (key === 'mtaHooks.username') return username;
if (key === 'mtaHooks.secret') return secret;
throw new Error(`unknown key: ${key}`);
});
guard = new MtaHooksAuthGuard(configService);
});

it('when credentials match, then allows the request', () => {
const context = contextWithAuth(basic(username, secret));

expect(guard.canActivate(context)).toBe(true);
});

it('when the secret is wrong, then throws Unauthorized', () => {
const context = contextWithAuth(basic(username, 'wrong'));

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the username is wrong, then throws Unauthorized', () => {
const context = contextWithAuth(basic('wrong', secret));

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the Authorization header is missing, then throws Unauthorized', () => {
const context = contextWithAuth(undefined);

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the scheme is not Basic, then throws Unauthorized', () => {
const context = contextWithAuth('Bearer some-token');

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});

it('when the decoded credentials lack a colon separator, then throws Unauthorized', () => {
const context = contextWithAuth(
`Basic ${Buffer.from('nocolon').toString('base64')}`,
);

expect(() => guard.canActivate(context)).toThrow(UnauthorizedException);
});
});
57 changes: 57 additions & 0 deletions src/modules/mta-hooks/mta-hooks-auth.guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { timingSafeEqual } from 'node:crypto';
import {
CanActivate,
type ExecutionContext,
Injectable,
UnauthorizedException,
} from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import type { Request } from 'express';

@Injectable()
export class MtaHooksAuthGuard implements CanActivate {
private readonly expectedUsername: string;
private readonly expectedSecret: string;

constructor(configService: ConfigService) {
this.expectedUsername =
configService.getOrThrow<string>('mtaHooks.username');
this.expectedSecret = configService.getOrThrow<string>('mtaHooks.secret');
}
Comment on lines +16 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Empty secret default enables authentication bypass when MTA_HOOKS_SECRET is unset.

The config contract (src/config/configuration.ts:59) defaults mtaHooks.secret to '':

secret: process.env.MTA_HOOKS_SECRET ?? '',

getOrThrow only throws on undefined, not empty strings, so the guard silently accepts '' as the expected secret. An attacker can then authenticate with Basic c3RhbHdhcnQ6 (decodes to stalwart:), since safeEqual('', '') passes the length check and timingSafeEqual returns true for two zero-length buffers.

Validate non-empty at construction time:

🔒️ Proposed fix
  constructor(configService: ConfigService) {
    this.expectedUsername =
      configService.getOrThrow<string>('mtaHooks.username');
    this.expectedSecret = configService.getOrThrow<string>('mtaHooks.secret');
+   if (!this.expectedSecret) {
+     throw new Error(
+       'mtaHooks.secret must be configured with a non-empty value',
+     );
+   }
  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/mta-hooks/mta-hooks-auth.guard.ts` around lines 16 - 20, The auth
guard in MTAHooksAuthGuard is accepting an empty secret because
ConfigService.getOrThrow only rejects undefined, not ''. Update the constructor
to validate that both mtaHooks.username and mtaHooks.secret are non-empty
strings before storing them, and fail fast if the secret is missing or blank.
Use the existing MTAHooksAuthGuard constructor and
expectedSecret/expectedUsername fields to locate the change, and keep the
validation aligned with the mtaHooks configuration contract.


canActivate(context: ExecutionContext): boolean {
const request = context.switchToHttp().getRequest<Request>();
const header = request.headers.authorization ?? '';

const [scheme, encoded] = header.split(' ');
if (scheme !== 'Basic' || !encoded) {
throw new UnauthorizedException('Missing or malformed Basic credentials');
}

const decoded = Buffer.from(encoded, 'base64').toString('utf8');
const separatorIndex = decoded.indexOf(':');
if (separatorIndex === -1) {
throw new UnauthorizedException('Malformed Basic credentials');
}

const username = decoded.slice(0, separatorIndex);
const secret = decoded.slice(separatorIndex + 1);

const usernameMatches = this.safeEqual(username, this.expectedUsername);
const secretMatches = this.safeEqual(secret, this.expectedSecret);
if (!usernameMatches || !secretMatches) {
throw new UnauthorizedException('Invalid MTA hook credentials');
}

return true;
}

private safeEqual(a: string, b: string): boolean {
const bufferA = Buffer.from(a, 'utf8');
const bufferB = Buffer.from(b, 'utf8');
if (bufferA.length !== bufferB.length) {
return false;
}
return timingSafeEqual(bufferA, bufferB);
}
}
23 changes: 23 additions & 0 deletions src/modules/mta-hooks/mta-hooks.controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Body, Controller, Post, UseGuards } from '@nestjs/common';
import { ApiBasicAuth, ApiOperation, ApiTags } from '@nestjs/swagger';
import { Public } from '../auth/decorators/public.decorator.js';
import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js';
import { MtaHooksService } from './mta-hooks.service.js';
import type { MtaHookRequest, MtaHookResponse } from './mta-hooks.types.js';

@ApiTags('MTA Hooks')
@ApiBasicAuth('mta-hooks')
@Public()
@UseGuards(MtaHooksAuthGuard)
@Controller('mta-hooks')
export class MtaHooksController {
constructor(private readonly mtaHooksService: MtaHooksService) {}

@Post('rcpt')
@ApiOperation({
summary: 'RCPT-stage hook',
})
rcpt(@Body() request: MtaHookRequest): Promise<MtaHookResponse> {
return this.mtaHooksService.handleRcpt(request);
}
}
13 changes: 13 additions & 0 deletions src/modules/mta-hooks/mta-hooks.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Module } from '@nestjs/common';
import { AccountModule } from '../account/account.module.js';
import { BridgeModule } from '../infrastructure/bridge/bridge.module.js';
import { MtaHooksAuthGuard } from './mta-hooks-auth.guard.js';
import { MtaHooksController } from './mta-hooks.controller.js';
import { MtaHooksService } from './mta-hooks.service.js';

@Module({
imports: [AccountModule, BridgeModule],
controllers: [MtaHooksController],
providers: [MtaHooksService, MtaHooksAuthGuard],
})
export class MtaHooksModule {}
Loading
Loading