From ad1bb15ede515684923847184a200901e188e63d Mon Sep 17 00:00:00 2001 From: Anant Date: Wed, 7 Oct 2026 01:46:30 +0530 Subject: [PATCH 1/2] Redact central Signing Server error logs --- src/index.ts | 6 +++-- src/middleware/errorHandling.ts | 7 ++--- src/middleware/validation.ts | 2 +- src/utilities/logger.ts | 8 +++--- tests/unit/errorLogging.test.ts | 47 +++++++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 tests/unit/errorLogging.test.ts diff --git a/src/index.ts b/src/index.ts index 7e3a325..c37b39d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -21,7 +21,10 @@ class SigningServer { private configure(): void { this.app.set("router", express.Router()); this.app.set("config", config); - this.app.use(morgan("combined", { stream: { write: message => logger.info(message.trim()) } })); + // Avoid logging request URLs, referrers or user agents with sensitive values. + this.app.use(morgan(":method :status :res[content-length] :response-time ms", { + stream: { write: message => logger.info(message.trim()) }, + })); this.app.use(bodyParser.json()); this.app.use(bodyParser.urlencoded({ extended: true })); this.app.use(initializeRoutes(this.app)); @@ -38,4 +41,3 @@ class SigningServer { } } new SigningServer(); - diff --git a/src/middleware/errorHandling.ts b/src/middleware/errorHandling.ts index 606fe57..8623295 100644 --- a/src/middleware/errorHandling.ts +++ b/src/middleware/errorHandling.ts @@ -6,7 +6,7 @@ import { logger } from "../utilities/logger"; /** * Middleware function to handle errors occurring in routes. * - * Logs the error details using the logger and sends a JSON response + * Logs a fixed failure message and sends a JSON response * with the appropriate HTTP status code and error message. * * @param err - The error object, which can be an instance of `Error` or any other type. @@ -20,10 +20,11 @@ import { logger } from "../utilities/logger"; * The error message is extracted from the `Error` object if available, or a generic message is used. */ export const handleRouteError = (err: any, req: Request, res: Response, next: NextFunction) => { - logger.error(`Error in route ${req.path}:`, err); + // Provider and database exceptions can contain request or configuration data. + logger.error("Signing Server route failed"); const message = err instanceof Error ? err.message : 'An unexpected error occurred.'; const statusCode = err.statusCode || StatusCodes.BAD_REQUEST; res.status(statusCode).json({ err: message, }); -}; \ No newline at end of file +}; diff --git a/src/middleware/validation.ts b/src/middleware/validation.ts index 329f339..a47d191 100644 --- a/src/middleware/validation.ts +++ b/src/middleware/validation.ts @@ -40,7 +40,7 @@ export const validateRequest = (schema: AnyZodObject) => }); } // Pass other errors to the global error handler - logger.error('Unhandled Error in Validation Middleware:', error); + logger.error('Validation middleware failed'); return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ err: 'An unexpected error occurred' }); // use next(error) if we've a dedicated error handling middleware // return next(error); diff --git a/src/utilities/logger.ts b/src/utilities/logger.ts index 18df2d6..7d1584f 100644 --- a/src/utilities/logger.ts +++ b/src/utilities/logger.ts @@ -7,12 +7,12 @@ export const logger = winston.createLogger({ }); export const unhandledLogger = () => { - process.on("unhandledRejection", (reason, promise) => { - logger.error(`Unhandled Rejection at: ${promise}\nReason: ${reason}`); + process.on("unhandledRejection", () => { + logger.error("Unhandled promise rejection"); }); - process.on("uncaughtException", (error) => { - logger.error(`Uncaught Exception: ${error.message}`); + process.on("uncaughtException", () => { + logger.error("Uncaught exception"); process.exit(1); }); }; diff --git a/tests/unit/errorLogging.test.ts b/tests/unit/errorLogging.test.ts new file mode 100644 index 0000000..581770f --- /dev/null +++ b/tests/unit/errorLogging.test.ts @@ -0,0 +1,47 @@ +jest.mock("../../src/routes/router", () => ({ + StatusCodes: { BAD_REQUEST: 400, INTERNAL_SERVER_ERROR: 500 }, +})); + +import { handleRouteError } from "../../src/middleware/errorHandling"; +import { validateRequest } from "../../src/middleware/validation"; +import { logger } from "../../src/utilities/logger"; + +describe("central error logging", () => { + const canary = "synthetic-private-data-canary"; + + afterEach(() => jest.restoreAllMocks()); + + const response = () => { + const res: any = { status: jest.fn(), json: jest.fn() }; + res.status.mockReturnValue(res); + res.json.mockReturnValue(res); + return res; + }; + + it("keeps the route response contract without logging exception contents", () => { + const log = jest.spyOn(logger, "error").mockImplementation(() => logger); + const res = response(); + const err: any = new Error(canary); + err.statusCode = 400; + + handleRouteError(err, {} as any, res, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ err: canary }); + expect(log).toHaveBeenCalledTimes(1); + expect(JSON.stringify(log.mock.calls)).not.toContain(canary); + }); + + it("keeps generic validation failure response without logging exception contents", async () => { + const log = jest.spyOn(logger, "error").mockImplementation(() => logger); + const res = response(); + const schema: any = { parseAsync: jest.fn().mockRejectedValue(new Error(canary)) }; + + await validateRequest(schema)({ body: {} } as any, res, jest.fn()); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ err: "An unexpected error occurred" }); + expect(log).toHaveBeenCalledTimes(1); + expect(JSON.stringify(log.mock.calls)).not.toContain(canary); + }); +}); From 73399398ed54237cf552461ae1001774ba431bff Mon Sep 17 00:00:00 2001 From: Anant Date: Wed, 7 Oct 2026 18:40:06 +0530 Subject: [PATCH 2/2] Remove private details from operational Signing Server logs --- README.md | 6 +++ src/databases/dbV2.ts | 2 +- .../inheritance/inheritanceKeyV2.ts | 4 +- .../inheritance/inheritanceKeyV3.ts | 14 +++-- .../assistedKeys.ts/ss-signer/signerV3.ts | 4 +- .../notifications/pushNotification.ts | 6 +-- src/utilities/crons.ts | 12 ++--- tests/unit/operationalLogging.test.ts | 51 +++++++++++++++++++ tests/unit/signerV3.test.ts | 4 +- 9 files changed, 78 insertions(+), 25 deletions(-) create mode 100644 tests/unit/operationalLogging.test.ts diff --git a/README.md b/README.md index 251f728..1db0e8b 100644 --- a/README.md +++ b/README.md @@ -34,3 +34,9 @@ Copy `.env.example` to a local `.env` and supply only disposable development val - `tests/`: isolated unit tests See [CONTRIBUTING.md](CONTRIBUTING.md) for changes and [SECURITY.md](SECURITY.md) for reporting security issues. The public distribution uses MIT while retaining the ISC notice for inherited code; see [LICENSE](LICENSE), [LICENSE-ISC](LICENSE-ISC), and [NOTICE.md](NOTICE.md). + +## Security review boundaries + +Operational logs record failure categories without provider/database exception contents, request IDs or database access identifiers. Access logs retain HTTP method, status, response size and elapsed time. Route status codes and the mobile `{ err }` response contract remain unchanged; changing the public error contract needs route-specific mobile compatibility fixtures before hosted adoption. + +Use the private reporting channel in SECURITY.md for suspected credential or log exposure. Source and history scans establish only what was scanned in this public repository. A separate hosted system's historical logging and production credential lifecycle require a private operational review; do not put its configuration or real logs in public issues. This repository's review does not authorize hosted deployment. diff --git a/src/databases/dbV2.ts b/src/databases/dbV2.ts index dc661dc..841d1c4 100644 --- a/src/databases/dbV2.ts +++ b/src/databases/dbV2.ts @@ -275,7 +275,7 @@ class DatabaseV2 { ); const databaseCreds = response[config.DATABASE_SECRET_IDENTIFIER_V2]; let databaseURL: string; - console.log("Connected to:", databaseCreds.name); + console.log("Signing Server V2 database configuration loaded"); if (config.DB_MODE === DATABASE_TYPE.DEV) databaseURL = `mongodb+srv://${databaseCreds.name}:${databaseCreds.password}@development.razn9q8.mongodb.net/?retryWrites=true&w=majority`; else diff --git a/src/services/assistedKeys.ts/inheritance/inheritanceKeyV2.ts b/src/services/assistedKeys.ts/inheritance/inheritanceKeyV2.ts index 5dc47b4..d8cbe88 100644 --- a/src/services/assistedKeys.ts/inheritance/inheritanceKeyV2.ts +++ b/src/services/assistedKeys.ts/inheritance/inheritanceKeyV2.ts @@ -61,7 +61,7 @@ export class InheritanceKeyServiceV2 { const { sent } = response.data; if (!sent) throw new Error(`keeper-relay send failure`); } catch (err) { - logger.error(`Failed to send notification for ${requestId}, err: ${err}`); + logger.error("Inheritance V2 notification delivery failed"); } }; @@ -79,7 +79,7 @@ export class InheritanceKeyServiceV2 { options, ); } catch (err) { - logger.error(`Failed to send email for ${emailType}, err: ${err}`); + logger.error("Inheritance V2 email delivery failed"); } }; diff --git a/src/services/assistedKeys.ts/inheritance/inheritanceKeyV3.ts b/src/services/assistedKeys.ts/inheritance/inheritanceKeyV3.ts index c3a8fea..e005a16 100644 --- a/src/services/assistedKeys.ts/inheritance/inheritanceKeyV3.ts +++ b/src/services/assistedKeys.ts/inheritance/inheritanceKeyV3.ts @@ -68,7 +68,7 @@ export class InheritanceKeyServiceV3 { options, ); } catch (err) { - logger.error(`Failed to send email for ${emailType}, err: ${err}`); + logger.error("Inheritance V3 email delivery failed"); } }; @@ -175,7 +175,7 @@ export class InheritanceKeyServiceV3 { if (targets.length) { const notificationType = getNotificationType(requestType) pushIKNotification(targets, notificationType, { requestId: requestId, requestAutoApprovesIn }); - logger.info(`Notification sent: ${inheritanceDoc.id}:${requestId}, ${Date()}`); + logger.info("Inheritance notification sent"); } } @@ -186,7 +186,7 @@ export class InheritanceKeyServiceV3 { if (emails.length) { const emailType = this.getEmailType(requestType) this.sendEmailsViaMailer(emails, emailType, { requestId: requestId, requestAutoApprovesIn }); - logger.info(`Email sent: ${inheritanceDoc.id}:${requestId}, ${Date()}`); + logger.info("Inheritance email sent"); } } } @@ -362,7 +362,7 @@ export class InheritanceKeyServiceV3 { const emails = idx(decryptedPolicy, (_) => _.alert.emails) || []; if (emails.length) { this.sendEmailsViaMailer(emails, Mailer.EMAIL_TYPE.IKS_SETUP, {}); - logger.info(`Setup Confirmation email sent: ${doc.id}, ${Date()}`); + logger.info("Inheritance setup confirmation email sent"); } } doc.policy = updatedEncryptedPolicy; @@ -852,9 +852,7 @@ export class InheritanceKeyServiceV3 { ); pushIKNotification(targets, notificationType, { requestId: request.requestId, requestAutoApprovesIn }); // send notification - logger.info( - `Notification sent: ${doc.id}:${request.requestId}, ${Date()}` - ); + logger.info("Inheritance notification sent"); count++; } } @@ -910,7 +908,7 @@ export class InheritanceKeyServiceV3 { const emailType = this.getEmailType((request as InheritanceKeyRequestV3).type) this.sendEmailsViaMailer(emails, emailType, { requestId: request.requestId, requestAutoApprovesIn }); - logger.info(`Emails sent: ${doc.id}:${request.requestId}, ${Date()}`); + logger.info("Inheritance emails sent"); count++; } } diff --git a/src/services/assistedKeys.ts/ss-signer/signerV3.ts b/src/services/assistedKeys.ts/ss-signer/signerV3.ts index 66a98cc..2b02a77 100644 --- a/src/services/assistedKeys.ts/ss-signer/signerV3.ts +++ b/src/services/assistedKeys.ts/ss-signer/signerV3.ts @@ -876,7 +876,7 @@ export class SignerV3 { if (tx.FCM) pushServerKeyNotification([tx.FCM], NOTIFICATION_TYPE.SIGNED_DELAYED_TRANSACTION, { id: tx.txid, signedTx: signedPSBT }) } catch (err) { - console.log(`Failed to process delayed transaction: ${err.message}`); + console.log("Delayed transaction processing failed"); } } }; @@ -917,7 +917,7 @@ export class SignerV3 { } } catch (err) { - console.error(`Failed to process delayed policy update ${update.policyId}: ${err.message}`); + console.error("Delayed policy update processing failed"); } } }; diff --git a/src/services/notifications/pushNotification.ts b/src/services/notifications/pushNotification.ts index 87e9d5c..495284e 100644 --- a/src/services/notifications/pushNotification.ts +++ b/src/services/notifications/pushNotification.ts @@ -104,9 +104,7 @@ export const pushIKNotification = async ( const { sent } = response.data; if (!sent) throw new Error(`keeper-relay send failure`); } catch (err) { - logger.error( - `Failed to send notification for ${options.requestId}, err: ${err}` - ); + logger.error("Inheritance notification delivery failed"); } }; @@ -160,6 +158,6 @@ export const pushServerKeyNotification = async ( const { sent } = response.data; if (!sent) throw new Error(`keeper-relay send failure`); } catch (err) { - logger.error(`Failed to send notification for ${options.id}, err: ${err}`); + logger.error("Server Key notification delivery failed"); } }; diff --git a/src/utilities/crons.ts b/src/utilities/crons.ts index 85ae59c..b7e6baa 100644 --- a/src/utilities/crons.ts +++ b/src/utilities/crons.ts @@ -13,7 +13,7 @@ const notificationIKCron = () => { if (!sent) throw new Error("Unable to send notification IK V3"); logger.info(`------------- Sent ${count} IK notifications V3------------- ${Date()}`); } catch (err) { - logger.error(`------------- Failed cron: notificationIK V3------------- ${err}`); + logger.error("Inheritance V3 notification job failed"); } }); @@ -25,7 +25,7 @@ const notificationIKCron = () => { if (!sent) throw new Error("Unable to send notification IK V2"); logger.info(`------------- Sent IK notifications V2------------- ${Date()}`); } catch (err) { - logger.error(`------------- Failed cron: notificationIK V2------------- ${err}`); + logger.error("Inheritance V2 notification job failed"); } }); }; @@ -39,7 +39,7 @@ const emailIKCron = () => { if (!sent) throw new Error("Unable to send emails V3"); logger.info(`------------- Sent ${count} IK emails V3------------- ${Date()}`); } catch (err) { - logger.error(`------------- Failed cron: emailIK V3------------- ${err}`); + logger.error("Inheritance V3 email job failed"); } }); @@ -51,7 +51,7 @@ const emailIKCron = () => { if (!sent) throw new Error("Unable to send emails V2"); logger.info(`------------- Sent IK emails V2------------- ${Date()}`); } catch (err) { - logger.error(`------------- Failed cron: emailIK V2------------- ${err}`); + logger.error("Inheritance V2 email job failed"); } }); }; @@ -63,7 +63,7 @@ const processDelayedTransactionsCron = () => { await signerV3.processDelayedTransactions(); logger.info(`------------- Processed delayed transactions------------- ${Date()}`); } catch (err) { - logger.error(`------------- Failed to process delayed transactions: ${err.message} ------------- ${Date()}`); + logger.error("Delayed transaction job failed"); } }); }; @@ -76,7 +76,7 @@ const processDelayedPolicyUpdatesCron = () => { await signerV3.processDelayedPolicyUpdates(); logger.info(`------------- Processed delayed policy updates ------------- ${Date()}`); } catch (err) { - logger.error(`------------- Failed cron: process delayed policy updates ------------- ${err}`); + logger.error("Delayed policy update job failed"); } }); }; diff --git a/tests/unit/operationalLogging.test.ts b/tests/unit/operationalLogging.test.ts new file mode 100644 index 0000000..947f86b --- /dev/null +++ b/tests/unit/operationalLogging.test.ts @@ -0,0 +1,51 @@ +jest.mock("axios", () => ({ __esModule: true, default: { post: jest.fn() } })); +jest.mock("node-cron", () => ({ __esModule: true, default: { schedule: jest.fn() } })); +jest.mock("../../src/services/assistedKeys.ts/inheritance/inheritanceKeyV2", () => ({ + __esModule: true, default: { + sendNotifications: jest.fn(async () => { throw Error("synthetic-private-data-canary"); }), + sendIKRequestEmails: jest.fn(async () => { throw Error("synthetic-private-data-canary"); }), + }, +})); +jest.mock("../../src/services/assistedKeys.ts/inheritance/inheritanceKeyV3", () => ({ + __esModule: true, default: { + sendNotifications: jest.fn(async () => { throw Error("synthetic-private-data-canary"); }), + sendIKRequestEmails: jest.fn(async () => { throw Error("synthetic-private-data-canary"); }), + }, +})); +jest.mock("../../src/services/assistedKeys.ts/ss-signer/signerV3", () => ({ + __esModule: true, default: { + processDelayedTransactions: jest.fn(async () => { throw Error("synthetic-private-data-canary"); }), + processDelayedPolicyUpdates: jest.fn(async () => { throw Error("synthetic-private-data-canary"); }), + }, +})); + +import axios from "axios"; +import cron from "node-cron"; +import { initializeCrons } from "../../src/utilities/crons"; +import { logger } from "../../src/utilities/logger"; +import { pushIKNotification, pushServerKeyNotification, NOTIFICATION_TYPE } from "../../src/services/notifications/pushNotification"; + +describe("operational log privacy", () => { + const canary = "synthetic-private-data-canary"; + afterEach(() => { jest.restoreAllMocks(); jest.clearAllMocks(); }); + + it("logs a category for every failed scheduled job without exception contents", async () => { + const errorLog = jest.spyOn(logger, "error").mockImplementation(() => logger); + jest.spyOn(logger, "info").mockImplementation(() => logger); + initializeCrons(); + const callbacks = (cron.schedule as jest.Mock).mock.calls.map(call => call[1]); + expect(callbacks).toHaveLength(6); + for (const callback of callbacks) await callback(); + expect(errorLog).toHaveBeenCalledTimes(6); + expect(JSON.stringify(errorLog.mock.calls)).not.toContain(canary); + }); + + it("does not log notification identifiers or underlying provider errors", async () => { + const errorLog = jest.spyOn(logger, "error").mockImplementation(() => logger); + (axios.post as jest.Mock).mockRejectedValue(new Error(canary)); + await pushIKNotification(["synthetic-token"], NOTIFICATION_TYPE.IKS_REQUEST, { requestId: canary, requestAutoApprovesIn: 60000 }); + await pushServerKeyNotification(["synthetic-token"], NOTIFICATION_TYPE.POLICY_UPDATE_APPLIED, { id: canary }); + expect(errorLog).toHaveBeenCalledTimes(2); + expect(JSON.stringify(errorLog.mock.calls)).not.toContain(canary); + }); +}); diff --git a/tests/unit/signerV3.test.ts b/tests/unit/signerV3.test.ts index 210d26a..bb36ba0 100644 --- a/tests/unit/signerV3.test.ts +++ b/tests/unit/signerV3.test.ts @@ -1203,7 +1203,7 @@ describe("SignerV3", () => { ); expect(consoleErrorSpy).toHaveBeenCalledWith( - "Failed to process delayed policy update policy2: Singer not found against vault: invalid-signer" + "Delayed policy update processing failed" ); consoleErrorSpy.mockRestore(); }); @@ -1435,7 +1435,7 @@ describe("SignerV3", () => { ); expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining("Failed to process delayed transaction") + "Delayed transaction processing failed" ); signPSBT.mockRestore(); getWalletCreds.mockRestore();