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
10 changes: 9 additions & 1 deletion apps/api/src/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Module } from '@nestjs/common';
import { ConfigModule } from '@nestjs/config';
import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler';
import { APP_GUARD } from '@nestjs/core';
import { APP_GUARD, APP_INTERCEPTOR } from '@nestjs/core';
import { AuthContextResponseInterceptor } from './auth/auth-context-response.interceptor';
import { AppController } from './app.controller';
import { AppService } from './app.service';
import { AttachmentsModule } from './attachments/attachments.module';
Expand Down Expand Up @@ -144,6 +145,13 @@ import { OffboardingChecklistModule } from './offboarding-checklist/offboarding-
provide: APP_GUARD,
useClass: ThrottlerGuard,
},
{
// Appends authType / authenticatedUser to authenticated JSON responses
// so the contract is uniform across every endpoint rather than only the
// 17 controllers that hand-wrote it. Opt out with @SkipAuthContextResponse().
provide: APP_INTERCEPTOR,
useClass: AuthContextResponseInterceptor,

@cubic-dev-ai cubic-dev-ai Bot Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Platform-admin endpoints will not receive the auth context fields because PlatformAdminGuard does not populate request.authType. Either populate the same auth context used by the interceptor or explicitly exclude these routes from the uniform-response contract.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/app.module.ts, line 153:

<comment>Platform-admin endpoints will not receive the auth context fields because `PlatformAdminGuard` does not populate `request.authType`. Either populate the same auth context used by the interceptor or explicitly exclude these routes from the uniform-response contract.</comment>

<file context>
@@ -144,6 +145,13 @@ import { OffboardingChecklistModule } from './offboarding-checklist/offboarding-
+      // so the contract is uniform across every endpoint rather than only the
+      // 17 controllers that hand-wrote it. Opt out with @SkipAuthContextResponse().
+      provide: APP_INTERCEPTOR,
+      useClass: AuthContextResponseInterceptor,
+    },
   ],
</file context>
Fix with cubic

},
],
})
export class AppModule {}
164 changes: 164 additions & 0 deletions apps/api/src/auth/auth-context-response.interceptor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
import { StreamableFile } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { Readable } from 'node:stream';
import { firstValueFrom, of } from 'rxjs';
import { AuthContextResponseInterceptor } from './auth-context-response.interceptor';
import { IS_PUBLIC_KEY } from './public.decorator';
import { SKIP_AUTH_CONTEXT_RESPONSE_KEY } from './skip-auth-context-response.decorator';

type RequestOverrides = {
authType?: 'api-key' | 'session' | 'service';
userId?: string;
userEmail?: string;
};

function run(
body: unknown,
{
request = { authType: 'api-key' as const },
metadata = {},
type = 'http',
}: {
request?: RequestOverrides;
metadata?: Record<string, boolean>;
type?: string;
} = {},
): Promise<unknown> {
const reflector = {
getAllAndOverride: (key: string) => metadata[key],
} as unknown as Reflector;

const context = {
getType: () => type,
getHandler: () => undefined,
getClass: () => undefined,
switchToHttp: () => ({ getRequest: () => request }),
} as never;

const interceptor = new AuthContextResponseInterceptor(reflector);
return firstValueFrom(
interceptor.intercept(context, { handle: () => of(body) }) as never,
);
}

describe('AuthContextResponseInterceptor', () => {
describe('appends the context', () => {
it('adds authType for api-key auth', async () => {
await expect(run({ data: [] })).resolves.toEqual({
data: [],
authType: 'api-key',
});
});

it('adds authenticatedUser for session auth', async () => {
const result = await run(
{ id: 'x' },
{
request: {
authType: 'session',
userId: 'usr_1',
userEmail: 'a@b.com',
},
},
);
expect(result).toEqual({
id: 'x',
authType: 'session',
authenticatedUser: { id: 'usr_1', email: 'a@b.com' },
});
});

it('omits authenticatedUser when only one of id/email is present', async () => {
const result = (await run(
{ id: 'x' },
{ request: { authType: 'session', userId: 'usr_1' } },
)) as Record<string, unknown>;
expect(result.authenticatedUser).toBeUndefined();
expect(result.authType).toBe('session');
});
});

describe('leaves the body alone', () => {
it('when the endpoint opts out', async () => {
const body = { data: [] };
await expect(
run(body, { metadata: { [SKIP_AUTH_CONTEXT_RESPONSE_KEY]: true } }),
).resolves.toEqual(body);
});

it('when the endpoint is @Public()', async () => {
const body = { data: [] };
await expect(
run(body, { metadata: { [IS_PUBLIC_KEY]: true } }),
).resolves.toEqual(body);
});

it('when the request carries no auth context', async () => {
const body = { data: [] };
await expect(run(body, { request: {} })).resolves.toEqual(body);
});

it('when the controller already set authType itself', async () => {
const body = { data: [], authType: 'session' };
await expect(run(body)).resolves.toEqual(body);
});

it('for a non-http context', async () => {
const body = { data: [] };
await expect(run(body, { type: 'rpc' })).resolves.toEqual(body);
});
});

describe('refuses to corrupt non-object payloads', () => {
it('passes arrays through untouched', async () => {
await expect(run([{ id: 1 }])).resolves.toEqual([{ id: 1 }]);
});

it.each([
['null', null],
['undefined', undefined],
['a string', 'ok'],
['a number', 42],
['a boolean', true],
])('passes %s through untouched', async (_label, value) => {
await expect(run(value)).resolves.toEqual(value);
});

it('passes a Buffer through untouched', async () => {
const buf = Buffer.from('pdf-bytes');
const result = await run(buf);
expect(Buffer.isBuffer(result)).toBe(true);
expect(result).toEqual(buf);
});

it('passes a stream through untouched', async () => {
const stream = Readable.from(['chunk']);
expect(await run(stream)).toBe(stream);
});

it('passes a StreamableFile through untouched', async () => {
const file = new StreamableFile(Buffer.from('x'));
expect(await run(file)).toBe(file);
});

it('passes a Date through untouched', async () => {
const date = new Date('2026-01-01T00:00:00Z');
expect(await run(date)).toBe(date);
});

it('passes a class instance through untouched', async () => {
class Dto {
constructor(public id: string) {}
}
const dto = new Dto('x');
expect(await run(dto)).toBe(dto);
});
});

it('does not mutate the original body object', async () => {
const body = { data: [] };
await run(body);
expect(body).toEqual({ data: [] });
expect('authType' in body).toBe(false);
});
});
109 changes: 109 additions & 0 deletions apps/api/src/auth/auth-context-response.interceptor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import {
CallHandler,
ExecutionContext,
Injectable,
NestInterceptor,
StreamableFile,
} from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { Observable, map } from 'rxjs';
import { IS_PUBLIC_KEY } from './public.decorator';
import { SKIP_AUTH_CONTEXT_RESPONSE_KEY } from './skip-auth-context-response.decorator';
import type { AuthenticatedRequest } from './types';

/**
* Shape appended to authenticated responses.
*/
export interface AuthContextResponseFields {
authType: AuthenticatedRequest['authType'];
authenticatedUser?: { id: string; email: string };
}

/**
* Appends `authType` (and `authenticatedUser` for session auth) to response
* bodies.
*
* These fields were previously hand-written into each controller — 81 copies
* across 14 of 94 controllers — so the same documented contract was honoured
* by some endpoints and not others. Generated clients (the OpenAPI-derived MCP
* server) key off consistent response shapes, so the inconsistency was a real
* interoperability problem rather than a cosmetic one.
*
* Deliberately conservative about what it will touch: only plain JSON objects.
* Arrays, primitives, null, buffers and streams pass through untouched, because
* grafting fields onto them would corrupt the payload rather than annotate it.
*/
@Injectable()
export class AuthContextResponseInterceptor implements NestInterceptor {
constructor(private readonly reflector: Reflector) {}

intercept(context: ExecutionContext, next: CallHandler): Observable<unknown> {
if (context.getType() !== 'http') {
return next.handle();
}

const skip = this.reflector.getAllAndOverride<boolean>(
SKIP_AUTH_CONTEXT_RESPONSE_KEY,
[context.getHandler(), context.getClass()],
);
const isPublic = this.reflector.getAllAndOverride<boolean>(IS_PUBLIC_KEY, [
context.getHandler(),
context.getClass(),
]);

if (skip || isPublic) {
return next.handle();
}

const request = context.switchToHttp().getRequest<AuthenticatedRequest>();

return next.handle().pipe(
map((body: unknown) => {
if (!request.authType) {
// No auth context (unauthenticated route, or a guard that does not
// populate it) — nothing meaningful to report.
return body;
}

if (!isPlainJsonObject(body)) {
return body;
}

// A controller that still sets the fields itself wins, so the migration
// can proceed file by file without double-writing.
if ('authType' in body) {

@cubic-dev-ai cubic-dev-ai Bot Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: When an authenticated response has a domain-level authType, this guard mistakes it for the auth-context field and omits the caller context. Mark fixed-contract endpoints with @SkipAuthContextResponse() or move the context field to a non-colliding shape.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/auth/auth-context-response.interceptor.ts, line 74:

<comment>When an authenticated response has a domain-level `authType`, this guard mistakes it for the auth-context field and omits the caller context. Mark fixed-contract endpoints with `@SkipAuthContextResponse()` or move the context field to a non-colliding shape.</comment>

<file context>
@@ -0,0 +1,109 @@
+
+        // A controller that still sets the fields itself wins, so the migration
+        // can proceed file by file without double-writing.
+        if ('authType' in body) {
+          return body;
+        }
</file context>
Fix with cubic

return body;
}

const fields: AuthContextResponseFields = { authType: request.authType };

if (request.userId && request.userEmail) {
fields.authenticatedUser = {

@cubic-dev-ai cubic-dev-ai Bot Sep 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This global APP_INTERCEPTOR appends authenticatedUser (id + email) to every session-authenticated JSON response, yet the @SkipAuthContextResponse() opt-out it ships with is not applied to a single endpoint. The decorator's own docs say to use it on trust-portal and externally consumed payloads 'where leaking an internal user id/email would be a disclosure', but no endpoint opts out while the interceptor is enabled for all 94 controllers. Any such endpoint now echoes the user's email in its body for the first time. Audit which endpoints are externally consumed and apply @SkipAuthContextResponse() to them before enabling this globally.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/auth/auth-context-response.interceptor.ts, line 81:

<comment>This global APP_INTERCEPTOR appends `authenticatedUser` (id + email) to every session-authenticated JSON response, yet the `@SkipAuthContextResponse()` opt-out it ships with is not applied to a single endpoint. The decorator's own docs say to use it on trust-portal and externally consumed payloads 'where leaking an internal user id/email would be a disclosure', but no endpoint opts out while the interceptor is enabled for all 94 controllers. Any such endpoint now echoes the user's email in its body for the first time. Audit which endpoints are externally consumed and apply `@SkipAuthContextResponse()` to them before enabling this globally.</comment>

<file context>
@@ -0,0 +1,109 @@
+        const fields: AuthContextResponseFields = { authType: request.authType };
+
+        if (request.userId && request.userEmail) {
+          fields.authenticatedUser = {
+            id: request.userId,
+            email: request.userEmail,
</file context>
Fix with cubic

id: request.userId,
email: request.userEmail,
};
}

return { ...body, ...fields };
}),
);
}
}

/**
* True only for plain objects safe to spread. Excludes arrays, null, class
* instances with custom prototypes, buffers, streams and dates — anything
* where spreading would lose behaviour or corrupt the payload.
*/
function isPlainJsonObject(value: unknown): value is Record<string, unknown> {
if (value === null || typeof value !== 'object') return false;
if (Array.isArray(value)) return false;
if (value instanceof StreamableFile) return false;
if (value instanceof Date) return false;
if (Buffer.isBuffer(value)) return false;
// Streams expose pipe(); spreading one yields a broken object.
if (typeof (value as { pipe?: unknown }).pipe === 'function') return false;

const proto: unknown = Object.getPrototypeOf(value);
return proto === Object.prototype || proto === null;
}
16 changes: 16 additions & 0 deletions apps/api/src/auth/skip-auth-context-response.decorator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { SetMetadata } from '@nestjs/common';

export const SKIP_AUTH_CONTEXT_RESPONSE_KEY = 'skipAuthContextResponse';

/**
* Opt an endpoint out of having `authType` / `authenticatedUser` appended to
* its response body.
*
* Use it where echoing who called is wrong or unhelpful:
* - public and webhook endpoints, where there is no caller identity to report
* - trust-portal and other externally consumed payloads, where leaking an
* internal user id/email would be a disclosure
* - endpoints whose response shape is fixed by an external contract
*/
export const SkipAuthContextResponse = () =>
SetMetadata(SKIP_AUTH_CONTEXT_RESPONSE_KEY, true);
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ConnectionRepository } from '../repositories/connection.repository';
import { CredentialVaultService } from '../services/credential-vault.service';
import { OAuthCredentialsService } from '../services/oauth-credentials.service';
import { IntegrationSyncLoggerService } from '../services/integration-sync-logger.service';
import { GenericDeviceSyncService } from '../services/generic-device-sync.service';
import { GenericEmployeeSyncService } from '../services/generic-employee-sync.service';
import { DynamicIntegrationRepository } from '../repositories/dynamic-integration.repository';
import { CheckRunRepository } from '../repositories/check-run.repository';
Expand Down Expand Up @@ -104,6 +105,9 @@ describe('SyncController - Google Workspace employees', () => {
useValue: { logSync: jest.fn() },
},
{ provide: GenericEmployeeSyncService, useValue: {} },
// Required by SyncController's constructor; the suite could not
// instantiate the controller without it.
{ provide: GenericDeviceSyncService, useValue: {} },
{ provide: DynamicIntegrationRepository, useValue: {} },
{ provide: CheckRunRepository, useValue: {} },
],
Expand Down Expand Up @@ -306,7 +310,8 @@ describe('SyncController - Google Workspace employees', () => {
expect(result.skipped).toBe(0);
expect(mockedDb.member.update).toHaveBeenCalledWith({
where: { id: 'mem_back' },
data: { deactivated: false, isActive: true },
// offboardDate is cleared on reactivation (97636c4ea).
data: { deactivated: false, isActive: true, offboardDate: null },
});
});

Expand Down
Loading