diff --git a/apps/api/src/federation/command.service.spec.ts b/apps/api/src/federation/command.service.spec.ts index 42e5dd7..a6ea75f 100644 --- a/apps/api/src/federation/command.service.spec.ts +++ b/apps/api/src/federation/command.service.spec.ts @@ -154,6 +154,15 @@ describe("CommandService", () => { signature: "signature-123", }) ); + + // Verify status was checked in the query + expect(prisma.federationConnection.findUnique).toHaveBeenCalledWith({ + where: { + id: mockConnectionId, + workspaceId: mockWorkspaceId, + status: FederationConnectionStatus.ACTIVE, + }, + }); }); it("should throw error if connection not found", async () => { @@ -165,19 +174,21 @@ describe("CommandService", () => { }); it("should throw error if connection is not active", async () => { - const mockConnection = { - id: mockConnectionId, - workspaceId: mockWorkspaceId, - status: FederationConnectionStatus.SUSPENDED, - }; - - vi.spyOn(prisma.federationConnection, "findUnique").mockResolvedValue( - mockConnection as never - ); + // Connection should not be found by query because it's not ACTIVE + vi.spyOn(prisma.federationConnection, "findUnique").mockResolvedValue(null); await expect( service.sendCommand(mockWorkspaceId, mockConnectionId, "test", {}) - ).rejects.toThrow("Connection is not active"); + ).rejects.toThrow("Connection not found"); + + // Verify status was checked in the query + expect(prisma.federationConnection.findUnique).toHaveBeenCalledWith({ + where: { + id: mockConnectionId, + workspaceId: mockWorkspaceId, + status: FederationConnectionStatus.ACTIVE, + }, + }); }); it("should mark command as failed if sending fails", async () => { diff --git a/apps/api/src/federation/command.service.ts b/apps/api/src/federation/command.service.ts index 626c1af..e094a59 100644 --- a/apps/api/src/federation/command.service.ts +++ b/apps/api/src/federation/command.service.ts @@ -41,19 +41,19 @@ export class CommandService { commandType: string, payload: Record ): Promise { - // Validate connection exists and is active + // Validate connection exists and is active (enforced in query) const connection = await this.prisma.federationConnection.findUnique({ - where: { id: connectionId, workspaceId }, + where: { + id: connectionId, + workspaceId, + status: FederationConnectionStatus.ACTIVE, + }, }); if (!connection) { throw new Error("Connection not found"); } - if (connection.status !== FederationConnectionStatus.ACTIVE) { - throw new Error("Connection is not active"); - } - // Get local instance identity const identity = await this.federationService.getInstanceIdentity(); @@ -132,7 +132,7 @@ export class CommandService { throw new Error("Command timestamp is outside acceptable range"); } - // Find connection for remote instance + // Find connection for remote instance (status enforced in query) const connection = await this.prisma.federationConnection.findFirst({ where: { remoteInstanceId: commandMessage.instanceId, @@ -144,11 +144,6 @@ export class CommandService { throw new Error("No connection found for remote instance"); } - // Validate connection is active - if (connection.status !== FederationConnectionStatus.ACTIVE) { - throw new Error("Connection is not active"); - } - // Verify signature const { signature, ...messageToVerify } = commandMessage; const verificationResult = await this.signatureService.verifyMessage( diff --git a/apps/api/src/federation/crypto.service.spec.ts b/apps/api/src/federation/crypto.service.spec.ts index ce0d76b..d1b1d7c 100644 --- a/apps/api/src/federation/crypto.service.spec.ts +++ b/apps/api/src/federation/crypto.service.spec.ts @@ -2,10 +2,11 @@ * Crypto Service Tests */ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import { Test, TestingModule } from "@nestjs/testing"; import { ConfigService } from "@nestjs/config"; import { CryptoService } from "./crypto.service"; +import { Logger } from "@nestjs/common"; describe("CryptoService", () => { let service: CryptoService; @@ -137,6 +138,31 @@ describe("CryptoService", () => { // Act & Assert expect(() => service.decrypt(corrupted)).toThrow("Failed to decrypt data"); }); + + it("should not log sensitive data in error messages", () => { + // Arrange + const loggerErrorSpy = vi.spyOn(Logger.prototype, "error"); + const corruptedData = "corrupted:data:here"; + + // Act & Assert + expect(() => service.decrypt(corruptedData)).toThrow("Failed to decrypt data"); + + // Verify logger was called with safe message + expect(loggerErrorSpy).toHaveBeenCalled(); + const logCall = loggerErrorSpy.mock.calls[0]; + + // First argument should contain error type but not sensitive data + expect(logCall[0]).toMatch(/Decryption failed:/); + + // Should NOT log the actual error object with stack traces + expect(logCall.length).toBe(1); // Only one argument (the message) + + // Verify the corrupted data is not in the log + const logMessage = logCall[0] as string; + expect(logMessage).not.toContain(corruptedData); + + loggerErrorSpy.mockRestore(); + }); }); describe("encrypt/decrypt round-trip", () => { diff --git a/apps/api/src/federation/crypto.service.ts b/apps/api/src/federation/crypto.service.ts index 56140d6..8acee35 100644 --- a/apps/api/src/federation/crypto.service.ts +++ b/apps/api/src/federation/crypto.service.ts @@ -90,7 +90,10 @@ export class CryptoService { return decrypted; } catch (error) { - this.logger.error("Decryption failed", error); + // Security: Do not log error details which may contain sensitive data + // Only log error type/code without stack trace or encrypted content + const errorType = error instanceof Error ? error.constructor.name : "Unknown"; + this.logger.error(`Decryption failed: ${errorType}`); throw new Error("Failed to decrypt data"); } } diff --git a/apps/api/src/federation/federation.service.spec.ts b/apps/api/src/federation/federation.service.spec.ts index fb85ea8..3914270 100644 --- a/apps/api/src/federation/federation.service.spec.ts +++ b/apps/api/src/federation/federation.service.spec.ts @@ -198,6 +198,18 @@ describe("FederationService", () => { expect(result1.publicKey).not.toEqual(result2.publicKey); expect(result1.privateKey).not.toEqual(result2.privateKey); }); + + it("should generate RSA-4096 key pairs for future-proof security", () => { + // Act + const result = service.generateKeypair(); + + // Assert - Verify key size by checking approximate length + // RSA-4096 keys are significantly larger than RSA-2048 + // Private key in PKCS8 format: RSA-2048 ≈ 1700 bytes, RSA-4096 ≈ 3200 bytes + // Public key in SPKI format: RSA-2048 ≈ 400 bytes, RSA-4096 ≈ 800 bytes + expect(result.privateKey.length).toBeGreaterThan(3000); + expect(result.publicKey.length).toBeGreaterThan(700); + }); }); describe("regenerateKeypair", () => { diff --git a/apps/api/src/federation/federation.service.ts b/apps/api/src/federation/federation.service.ts index 390aec3..1e7e03a 100644 --- a/apps/api/src/federation/federation.service.ts +++ b/apps/api/src/federation/federation.service.ts @@ -57,10 +57,11 @@ export class FederationService { /** * Generate a new RSA key pair for instance signing + * Uses RSA-4096 for future-proof security (NIST recommendation) */ generateKeypair(): KeyPair { const { publicKey, privateKey } = generateKeyPairSync("rsa", { - modulusLength: 2048, + modulusLength: 4096, publicKeyEncoding: { type: "spki", format: "pem", diff --git a/apps/api/src/federation/identity-linking.controller.spec.ts b/apps/api/src/federation/identity-linking.controller.spec.ts index 33b8510..56349e3 100644 --- a/apps/api/src/federation/identity-linking.controller.spec.ts +++ b/apps/api/src/federation/identity-linking.controller.spec.ts @@ -90,6 +90,15 @@ describe("IdentityLinkingController", () => { }); describe("POST /identity/verify", () => { + it("should have AuthGuard and Throttle decorators applied", () => { + // Verify that the endpoint has proper guards and rate limiting + const verifyMetadata = Reflect.getMetadata( + "__guards__", + IdentityLinkingController.prototype.verifyIdentity + ); + expect(verifyMetadata).toBeDefined(); + }); + it("should verify identity with valid request", async () => { const dto: VerifyIdentityDto = { localUserId: "local-user-id", diff --git a/apps/api/src/federation/identity-linking.controller.ts b/apps/api/src/federation/identity-linking.controller.ts index a1b45ab..08c69f5 100644 --- a/apps/api/src/federation/identity-linking.controller.ts +++ b/apps/api/src/federation/identity-linking.controller.ts @@ -5,6 +5,7 @@ */ import { Controller, Post, Get, Patch, Delete, Body, Param, UseGuards } from "@nestjs/common"; +import { Throttle } from "@nestjs/throttler"; import { AuthGuard } from "../auth/guards/auth.guard"; import { IdentityLinkingService } from "./identity-linking.service"; import { IdentityResolutionService } from "./identity-resolution.service"; @@ -45,8 +46,11 @@ export class IdentityLinkingController { * * Verify a user's identity from a remote instance. * Validates signature and OIDC token. + * Rate limit: "strict" tier (10 req/min) - public endpoint requiring authentication */ @Post("verify") + @UseGuards(AuthGuard) + @Throttle({ strict: { limit: 10, ttl: 60000 } }) async verifyIdentity(@Body() dto: VerifyIdentityDto): Promise { return this.identityLinkingService.verifyIdentity(dto); } diff --git a/apps/api/src/federation/query.service.spec.ts b/apps/api/src/federation/query.service.spec.ts index 8b1b59f..6d8b431 100644 --- a/apps/api/src/federation/query.service.spec.ts +++ b/apps/api/src/federation/query.service.spec.ts @@ -143,7 +143,11 @@ describe("QueryService", () => { expect(result.messageType).toBe(FederationMessageType.QUERY); expect(result.query).toBe(query); expect(mockPrisma.federationConnection.findUnique).toHaveBeenCalledWith({ - where: { id: connectionId, workspaceId }, + where: { + id: connectionId, + workspaceId, + status: FederationConnectionStatus.ACTIVE, + }, }); expect(mockPrisma.federationMessage.create).toHaveBeenCalled(); expect(mockHttpService.post).toHaveBeenCalledWith( @@ -168,17 +172,21 @@ describe("QueryService", () => { }); it("should throw error if connection not active", async () => { - const mockConnection = { - id: "connection-1", - workspaceId: "workspace-1", - status: FederationConnectionStatus.PENDING, - }; - - mockPrisma.federationConnection.findUnique.mockResolvedValue(mockConnection); + // Connection should not be found by query because it's not ACTIVE + mockPrisma.federationConnection.findUnique.mockResolvedValue(null); await expect( service.sendQuery("workspace-1", "connection-1", "SELECT * FROM tasks") - ).rejects.toThrow("Connection is not active"); + ).rejects.toThrow("Connection not found"); + + // Verify that findUnique was called with status: ACTIVE in the query + expect(mockPrisma.federationConnection.findUnique).toHaveBeenCalledWith({ + where: { + id: "connection-1", + workspaceId: "workspace-1", + status: FederationConnectionStatus.ACTIVE, + }, + }); }); it("should handle network errors gracefully", async () => { @@ -305,19 +313,21 @@ describe("QueryService", () => { signature: "valid-signature", }; - const mockConnection = { - id: "connection-1", - workspaceId: "workspace-1", - remoteInstanceId: "remote-instance-1", - status: FederationConnectionStatus.PENDING, - }; - - mockPrisma.federationConnection.findFirst.mockResolvedValue(mockConnection); + // Connection should not be found because status filter in query excludes non-ACTIVE + mockPrisma.federationConnection.findFirst.mockResolvedValue(null); mockSignatureService.validateTimestamp.mockReturnValue(true); await expect(service.handleIncomingQuery(queryMessage)).rejects.toThrow( - "Connection is not active" + "No connection found for remote instance" ); + + // Verify the findFirst was called with status: ACTIVE filter + expect(mockPrisma.federationConnection.findFirst).toHaveBeenCalledWith({ + where: { + remoteInstanceId: "remote-instance-1", + status: FederationConnectionStatus.ACTIVE, + }, + }); }); it("should reject query from unknown instance", async () => { diff --git a/apps/api/src/federation/query.service.ts b/apps/api/src/federation/query.service.ts index 6a2458b..a56c9e0 100644 --- a/apps/api/src/federation/query.service.ts +++ b/apps/api/src/federation/query.service.ts @@ -38,19 +38,19 @@ export class QueryService { query: string, context?: Record ): Promise { - // Validate connection exists and is active + // Validate connection exists and is active (enforced in query) const connection = await this.prisma.federationConnection.findUnique({ - where: { id: connectionId, workspaceId }, + where: { + id: connectionId, + workspaceId, + status: FederationConnectionStatus.ACTIVE, + }, }); if (!connection) { throw new Error("Connection not found"); } - if (connection.status !== FederationConnectionStatus.ACTIVE) { - throw new Error("Connection is not active"); - } - // Get local instance identity const identity = await this.federationService.getInstanceIdentity(); @@ -129,7 +129,7 @@ export class QueryService { throw new Error("Query timestamp is outside acceptable range"); } - // Find connection for remote instance + // Find connection for remote instance (status enforced in query) const connection = await this.prisma.federationConnection.findFirst({ where: { remoteInstanceId: queryMessage.instanceId, @@ -141,11 +141,6 @@ export class QueryService { throw new Error("No connection found for remote instance"); } - // Validate connection is active - if (connection.status !== FederationConnectionStatus.ACTIVE) { - throw new Error("Connection is not active"); - } - // Verify signature const { signature, ...messageToVerify } = queryMessage; const verificationResult = await this.signatureService.verifyMessage(