diff --git a/src/__tests__/auth.test.ts b/src/__tests__/auth.test.ts index e289b8c..e37aaff 100644 --- a/src/__tests__/auth.test.ts +++ b/src/__tests__/auth.test.ts @@ -41,6 +41,7 @@ describe("POST /api/v1/auth/login", () => { .post("/api/v1/auth/login") .expect(401); expect(res.body.error).toBe("Unauthorized"); + expect(res.body.code).toBe("API_KEY_UNAUTHORIZED"); }); it("rejects wrong API key", async () => { @@ -49,6 +50,7 @@ describe("POST /api/v1/auth/login", () => { .set("x-api-key", "wrong-key") .expect(401); expect(res.body.error).toBe("Unauthorized"); + expect(res.body.code).toBe("API_KEY_UNAUTHORIZED"); }); }); @@ -82,14 +84,19 @@ describe("POST /api/v1/auth/refresh", () => { await request(app) .post("/api/v1/auth/refresh") .send({ refreshToken }) - .expect(401); + .expect(401) + .expect(({ body }) => { + expect(body.code).toBe("REFRESH_TOKEN_INVALID"); + }); }); it("rejects empty body", async () => { - await request(app) + const res = await request(app) .post("/api/v1/auth/refresh") .send({}) .expect(400); + expect(res.body.code).toBe("VALIDATION_ERROR"); + expect(Array.isArray(res.body.details)).toBe(true); }); }); @@ -114,9 +121,10 @@ describe("POST /api/v1/auth/logout", () => { describe("wallet routes with JWT", () => { it("rejects unauthenticated request", async () => { - await request(app) + const res = await request(app) .post("/api/v1/wallet/keypair") .expect(401); + expect(res.body.code).toBe("AUTH_HEADER_INVALID"); }); it("accepts authenticated request with valid Bearer token", async () => { diff --git a/src/__tests__/errorResponses.test.ts b/src/__tests__/errorResponses.test.ts new file mode 100644 index 0000000..544a345 --- /dev/null +++ b/src/__tests__/errorResponses.test.ts @@ -0,0 +1,77 @@ +import request from "supertest"; +import { createApp } from "../app"; +import { NotFoundError } from "../errors/appError"; +import { GlobeWalletContract } from "../services/contracts/globeWallet"; +import { StellarService } from "../services/stellar"; + +function createMockStellar(overrides: Record = {}): StellarService { + return { + buildPartialTransaction: jest.fn(), + feeBumpTransaction: jest.fn(), + findStrictReceivePaths: jest.fn(), + findStrictSendPaths: jest.fn(), + generateKeypair: jest.fn(), + getAccount: jest.fn(), + getAccountThresholds: jest.fn(), + getBalances: jest.fn(), + getTransactions: jest.fn(), + pathPaymentStrictReceive: jest.fn(), + pathPaymentStrictSend: jest.fn(), + sendPayment: jest.fn(), + submitWithAdditionalSignatures: jest.fn(), + ...overrides, + } as unknown as StellarService; +} + +function createMockContract(): GlobeWalletContract { + return {} as GlobeWalletContract; +} + +describe("error responses", () => { + it("returns a typed 404 for not found domain errors", async () => { + const app = createApp( + createMockStellar({ + getAccount: jest.fn().mockRejectedValue( + new NotFoundError("Account GTEST was not found on Horizon", "ACCOUNT_NOT_FOUND") + ), + }), + createMockContract() + ); + + const res = await request(app) + .get(`/api/v1/account/${"G".repeat(56)}`) + .expect(404); + + expect(res.body.code).toBe("ACCOUNT_NOT_FOUND"); + expect(res.body.error).toBe("Account GTEST was not found on Horizon"); + }); + + it("does not infer 404 from an unrelated error message substring", async () => { + const app = createApp( + createMockStellar({ + getAccount: jest + .fn() + .mockRejectedValue(new Error("Validation failed because field does not exist in payload")), + }), + createMockContract() + ); + + const res = await request(app) + .get(`/api/v1/account/${"G".repeat(56)}`) + .expect(500); + + expect(res.body.code).toBe("INTERNAL_ERROR"); + expect(res.body.error).toBe("Validation failed because field does not exist in payload"); + }); + + it("returns a typed validation error body for invalid account parameters", async () => { + const app = createApp(createMockStellar(), createMockContract()); + + const res = await request(app) + .get(`/api/v1/account/${"M".repeat(56)}`) + .expect(400); + + expect(res.body.code).toBe("VALIDATION_ERROR"); + expect(res.body.details[0].path).toBe("publicKey"); + }); +}); diff --git a/src/app.ts b/src/app.ts index e64ec60..a4c0810 100644 --- a/src/app.ts +++ b/src/app.ts @@ -13,6 +13,7 @@ import { config } from "./config"; import { StellarService } from "./services/stellar"; import { SorobanService } from "./services/soroban"; import { GlobeWalletContract } from "./services/contracts/globeWallet"; +import { NotFoundError } from "./errors/appError"; export function createApp( stellar: StellarService = new StellarService(), @@ -44,6 +45,15 @@ export function createApp( app.use("/api/v1/price", priceRouter); app.use("/api/v1/auth", authRouter); + app.use((req, _res, next) => { + next( + new NotFoundError( + `Route ${req.method} ${req.originalUrl} was not found`, + "ROUTE_NOT_FOUND" + ) + ); + }); + app.use(errorHandler); return app; diff --git a/src/errors/appError.ts b/src/errors/appError.ts new file mode 100644 index 0000000..b08f5e0 --- /dev/null +++ b/src/errors/appError.ts @@ -0,0 +1,80 @@ +export interface ValidationIssue { + location?: string; + message: string; + path?: string; + type?: string; + value?: unknown; +} + +export interface AppErrorOptions { + code: string; + details?: unknown; + retryable?: boolean; + status: number; +} + +export class AppError extends Error { + readonly code: string; + readonly details?: unknown; + readonly retryable?: boolean; + readonly status: number; + + constructor(message: string, options: AppErrorOptions) { + super(message); + this.name = new.target.name; + this.code = options.code; + this.details = options.details; + this.retryable = options.retryable; + this.status = options.status; + } +} + +export class InternalServerError extends AppError { + constructor(message = "Internal server error") { + super(message, { code: "INTERNAL_ERROR", status: 500 }); + } +} + +export class NotFoundError extends AppError { + constructor(message = "Not found", code = "NOT_FOUND", details?: unknown) { + super(message, { code, details, status: 404 }); + } +} + +export class UnauthorizedError extends AppError { + constructor(message = "Unauthorized", code = "UNAUTHORIZED", details?: unknown) { + super(message, { code, details, status: 401 }); + } +} + +export class ValidationError extends AppError { + readonly details: ValidationIssue[]; + + constructor(issues: ValidationIssue[], message = "Request validation failed") { + super(message, { code: "VALIDATION_ERROR", details: issues, status: 400 }); + this.details = issues; + } +} + +export class HorizonError extends AppError { + constructor( + message: string, + options: { + code: string; + details?: unknown; + retryable?: boolean; + status?: number; + } + ) { + super(message, { + code: options.code, + details: options.details, + retryable: options.retryable, + status: options.status ?? 502, + }); + } +} + +export function isAppError(error: unknown): error is AppError { + return error instanceof AppError; +} diff --git a/src/middleware/apiKeyAuth.ts b/src/middleware/apiKeyAuth.ts index b8407c7..144e22b 100644 --- a/src/middleware/apiKeyAuth.ts +++ b/src/middleware/apiKeyAuth.ts @@ -1,6 +1,7 @@ import { Request, Response, NextFunction } from "express"; import { timingSafeEqual } from "crypto"; import { config } from "../config"; +import { UnauthorizedError } from "../errors/appError"; function safeEqual(a: string, b: string): boolean { const bufA = Buffer.from(a); @@ -9,10 +10,6 @@ function safeEqual(a: string, b: string): boolean { return timingSafeEqual(bufA, bufB); } -// DEPRECATED: Gates sensitive routes behind a shared API key. -// Replaced by JWT-based per-user auth. Migrate by calling POST /api/v1/auth/login -// with x-api-key to receive access/refresh tokens, then use Bearer auth. -// Scheduled for removal in a future release. export function apiKeyAuth(req: Request, res: Response, next: NextFunction) { console.warn("[DEPRECATED] apiKeyAuth - use POST /api/v1/auth/login and Bearer tokens instead"); res.setHeader("X-Deprecated", "apiKeyAuth - use JWT auth via /api/v1/auth/login"); @@ -21,7 +18,7 @@ export function apiKeyAuth(req: Request, res: Response, next: NextFunction) { const provided = req.header("x-api-key"); if (!provided || !safeEqual(provided, expected)) { - res.status(401).json({ error: "Unauthorized" }); + next(new UnauthorizedError("Unauthorized", "API_KEY_UNAUTHORIZED")); return; } diff --git a/src/middleware/errorHandler.ts b/src/middleware/errorHandler.ts index f4702dc..a6be9c1 100644 --- a/src/middleware/errorHandler.ts +++ b/src/middleware/errorHandler.ts @@ -1,15 +1,39 @@ -import { Request, Response, NextFunction } from "express"; -import { - LockAcquisitionError, - MemoRequiredError, - NonRetryableHorizonError, - SequenceConflictError, -} from "../services/stellarErrors"; -import { - SorobanNotConfiguredError, - SorobanSimulationError, - SorobanTransactionError, -} from "../services/sorobanErrors"; +import { NextFunction, Request, Response } from "express"; +import { AppError, InternalServerError, isAppError } from "../errors/appError"; +import { SorobanTransactionError } from "../services/sorobanErrors"; + +function toAppError(error: unknown): AppError { + if (isAppError(error)) { + return error; + } + + if (error instanceof Error) { + return new InternalServerError(error.message); + } + + return new InternalServerError(); +} + +function toErrorBody(error: AppError) { + const body: Record = { + code: error.code, + error: error.message, + }; + + if (error.details !== undefined) { + body.details = error.details; + } + + if (error.retryable !== undefined) { + body.retryable = error.retryable; + } + + if (error instanceof SorobanTransactionError) { + body.hash = error.hash; + } + + return body; +} export function errorHandler( err: unknown, @@ -18,52 +42,6 @@ export function errorHandler( _next: NextFunction ) { console.error(err); - - // These carry their own accurate status/code/message — a bare Horizon - // 400 or a generic 500 would hide exactly the information (retryable? - // what happened? what to do?) the caller needs. - if (err instanceof SequenceConflictError) { - res - .status(409) - .json({ error: err.message, code: err.code, retryable: err.retryable }); - return; - } - if (err instanceof LockAcquisitionError) { - res - .status(503) - .json({ error: err.message, code: err.code, retryable: err.retryable }); - return; - } - if (err instanceof MemoRequiredError) { - if (err instanceof NonRetryableHorizonError) { - res - .status(400) - .json({ error: err.message, code: err.code, retryable: err.retryable }); - return; - } - if (err instanceof SorobanNotConfiguredError) { - res - .status(503) - .json({ error: err.message, code: err.code, retryable: err.retryable }); - return; - } - if (err instanceof SorobanSimulationError) { - // Simulation caught this before anything was submitted or paid for — - // it's a rejected request, not a server fault. - res - .status(422) - .json({ error: err.message, code: err.code, retryable: err.retryable }); - return; - } - if (err instanceof SorobanTransactionError) { - res - .status(502) - .json({ error: err.message, code: err.code, retryable: err.retryable, hash: err.hash }); - return; - } - - const message = err instanceof Error ? err.message : "Internal server error"; - const status = - message.includes("Not Found") || message.includes("does not exist") ? 404 : 500; - res.status(status).json({ error: message }); + const error = toAppError(err); + res.status(error.status).json(toErrorBody(error)); } diff --git a/src/middleware/jwtAuth.ts b/src/middleware/jwtAuth.ts index 30c77b4..a790a46 100644 --- a/src/middleware/jwtAuth.ts +++ b/src/middleware/jwtAuth.ts @@ -1,10 +1,16 @@ import { Request, Response, NextFunction } from "express"; import { verifyAccessToken } from "../utils/jwt"; +import { UnauthorizedError } from "../errors/appError"; export function jwtAuth(req: Request, res: Response, next: NextFunction) { const header = req.header("Authorization"); if (!header?.startsWith("Bearer ")) { - res.status(401).json({ error: "Missing or malformed Authorization header" }); + next( + new UnauthorizedError( + "Missing or malformed Authorization header", + "AUTH_HEADER_INVALID" + ) + ); return; } @@ -12,12 +18,12 @@ export function jwtAuth(req: Request, res: Response, next: NextFunction) { try { const payload = verifyAccessToken(token); if (payload.type !== "access") { - res.status(401).json({ error: "Invalid token type" }); + next(new UnauthorizedError("Invalid token type", "AUTH_TOKEN_TYPE_INVALID")); return; } req.user = { sub: payload.sub }; next(); } catch { - res.status(401).json({ error: "Invalid or expired token" }); + next(new UnauthorizedError("Invalid or expired token", "AUTH_TOKEN_INVALID")); } } diff --git a/src/middleware/requestValidation.ts b/src/middleware/requestValidation.ts new file mode 100644 index 0000000..9ff1ffc --- /dev/null +++ b/src/middleware/requestValidation.ts @@ -0,0 +1,28 @@ +import { Request } from "express"; +import { validationResult } from "express-validator"; +import { ValidationError, ValidationIssue } from "../errors/appError"; + +type ValidationResultIssue = { + location?: string; + msg: unknown; + path?: string; + type?: string; + value?: unknown; +}; + +export function assertValidRequest(req: Request) { + const result = validationResult(req); + if (result.isEmpty()) { + return; + } + + const issues: ValidationIssue[] = (result.array() as ValidationResultIssue[]).map((issue) => ({ + location: issue.location, + message: String(issue.msg), + path: issue.path, + type: issue.type, + value: issue.value, + })); + + throw new ValidationError(issues); +} diff --git a/src/routes/account.ts b/src/routes/account.ts index 20b021c..8f6bb99 100644 --- a/src/routes/account.ts +++ b/src/routes/account.ts @@ -1,38 +1,27 @@ import { Router } from "express"; -import { param, validationResult } from "express-validator"; +import { param } from "express-validator"; +import { assertValidRequest } from "../middleware/requestValidation"; import { StellarService } from "../services/stellar"; -/** - * Custom validator: rejects Muxed (M...) addresses. - * Horizon's REST API does not accept M-addresses in path segments — passing - * them through results in an opaque Horizon failure. We reject early with a - * clear 400 explaining why. - */ function isGAddress(value: string): boolean { - if (typeof value !== "string") return false; + if (typeof value !== "string") { + return false; + } return value.startsWith("G") && value.length === 56; } +const publicKeyMessage = + "Invalid public key: only G... addresses are supported. Muxed (M...) addresses are not accepted; use the underlying G... address instead."; + export function createAccountRouter(stellar: StellarService): Router { const accountRouter = Router(); accountRouter.get( "/:publicKey", - param("publicKey").custom((value) => { - if (!isGAddress(value)) { - throw new Error( - "Invalid public key: only G... addresses are supported. " + - "Muxed (M...) addresses are not accepted — use the underlying G... address instead." - ); - } - return true; - }), + param("publicKey").custom(isGAddress).withMessage(publicKeyMessage), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const account = await stellar.getAccount(req.params.publicKey); res.json(account); } catch (err) { @@ -43,21 +32,10 @@ export function createAccountRouter(stellar: StellarService): Router { accountRouter.get( "/:publicKey/balances", - param("publicKey").custom((value) => { - if (!isGAddress(value)) { - throw new Error( - "Invalid public key: only G... addresses are supported. " + - "Muxed (M...) addresses are not accepted — use the underlying G... address instead." - ); - } - return true; - }), + param("publicKey").custom(isGAddress).withMessage(publicKeyMessage), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const balances = await stellar.getBalances(req.params.publicKey); res.json({ balances }); } catch (err) { @@ -68,21 +46,10 @@ export function createAccountRouter(stellar: StellarService): Router { accountRouter.get( "/:publicKey/transactions", - param("publicKey").custom((value) => { - if (!isGAddress(value)) { - throw new Error( - "Invalid public key: only G... addresses are supported. " + - "Muxed (M...) addresses are not accepted — use the underlying G... address instead." - ); - } - return true; - }), + param("publicKey").custom(isGAddress).withMessage(publicKeyMessage), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const limit = Math.min(Number(req.query.limit) || 20, 100); const cursor = req.query.cursor as string | undefined; const result = await stellar.getTransactions(req.params.publicKey, { limit, cursor }); diff --git a/src/routes/auth.ts b/src/routes/auth.ts index 70e2734..80ae6dd 100644 --- a/src/routes/auth.ts +++ b/src/routes/auth.ts @@ -1,6 +1,8 @@ import { Router } from "express"; -import { body, validationResult } from "express-validator"; +import { body } from "express-validator"; +import { UnauthorizedError } from "../errors/appError"; import { apiKeyAuth } from "../middleware/apiKeyAuth"; +import { assertValidRequest } from "../middleware/requestValidation"; import { generateAccessToken, generateRefreshToken, @@ -24,35 +26,37 @@ authRouter.post( authRouter.post( "/refresh", body("refreshToken").isString().notEmpty(), - (req, res) => { - const errors = validationResult(req); - if (!errors.isEmpty()) { - res.status(400).json({ errors: errors.array() }); - return; + (req, res, next) => { + try { + assertValidRequest(req); + const result = rotateRefreshToken(req.body.refreshToken); + if (!result) { + throw new UnauthorizedError( + "Invalid, expired, or revoked refresh token", + "REFRESH_TOKEN_INVALID" + ); + } + res.json({ + accessToken: result.accessToken, + refreshToken: result.refreshToken, + tokenType: "Bearer", + }); + } catch (err) { + next(err); } - const result = rotateRefreshToken(req.body.refreshToken); - if (!result) { - res.status(401).json({ error: "Invalid, expired, or revoked refresh token" }); - return; - } - res.json({ - accessToken: result.accessToken, - refreshToken: result.refreshToken, - tokenType: "Bearer", - }); } ); authRouter.post( "/logout", body("refreshToken").isString().notEmpty(), - (req, res) => { - const errors = validationResult(req); - if (!errors.isEmpty()) { - res.status(400).json({ errors: errors.array() }); - return; + (req, res, next) => { + try { + assertValidRequest(req); + revokeRefreshToken(req.body.refreshToken); + res.json({ message: "Logged out" }); + } catch (err) { + next(err); } - revokeRefreshToken(req.body.refreshToken); - res.json({ message: "Logged out" }); } ); diff --git a/src/routes/contract.ts b/src/routes/contract.ts index 994bbe6..236f6a0 100644 --- a/src/routes/contract.ts +++ b/src/routes/contract.ts @@ -1,21 +1,32 @@ import { Router } from "express"; -import { param, body, validationResult } from "express-validator"; +import { body, param } from "express-validator"; +import { assertValidRequest } from "../middleware/requestValidation"; import { GlobeWalletContract } from "../services/contracts/globeWallet"; import { jwtAuth } from "../middleware/jwtAuth"; +function isGAddress(value: string): boolean { + if (typeof value !== "string") { + return false; + } + return value.startsWith("G") && value.length === 56; +} + +const publicKeyMessage = + "Invalid public key: only G... addresses are supported. Muxed (M...) addresses are not accepted; use the underlying G... address instead."; + export function createContractRouter(globeWallet: GlobeWalletContract): Router { const contractRouter = Router(); - // Read-only, public on-chain data — same trust level as GET /account/*. contractRouter.get( "/wallet/:publicKey/assets", - param("publicKey").isLength({ min: 56, max: 56 }), + param("publicKey") + .isLength({ min: 56, max: 56 }) + .bail() + .custom(isGAddress) + .withMessage(publicKeyMessage), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const assets = await globeWallet.getAssets(req.params.publicKey); res.json({ assets }); } catch (err) { @@ -24,8 +35,6 @@ export function createContractRouter(globeWallet: GlobeWalletContract): Router { } ); - // State-changing and fee-paying, and takes a secret key in the body — - // same trust boundary as POST /wallet/send, so gate it the same way. contractRouter.use(jwtAuth); contractRouter.post( @@ -35,10 +44,7 @@ export function createContractRouter(globeWallet: GlobeWalletContract): Router { body("amount").isNumeric(), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const { userSecretKey, assetCode, amount } = req.body; const result = await globeWallet.recordSpend({ userSecretKey, assetCode, amount }); res.json({ hash: result.hash, ledger: result.ledger, successful: true }); diff --git a/src/routes/wallet.ts b/src/routes/wallet.ts index 2e4e7b1..8a963c5 100644 --- a/src/routes/wallet.ts +++ b/src/routes/wallet.ts @@ -1,47 +1,41 @@ import { Router } from "express"; -import { body, validationResult } from "express-validator"; -import { StellarService } from "../services/stellar"; -import { logKeypairIssuance } from "../services/auditLog"; +import { body, param } from "express-validator"; import { jwtAuth } from "../middleware/jwtAuth"; +import { assertValidRequest } from "../middleware/requestValidation"; +import { logKeypairIssuance } from "../services/auditLog"; +import { StellarService } from "../services/stellar"; function isGAddress(value: string): boolean { - if (typeof value !== "string") return false; + if (typeof value !== "string") { + return false; + } return value.startsWith("G") && value.length === 56; } +const destinationMessage = + "Invalid destination: only G... addresses are supported. Muxed (M...) addresses are not accepted; use the underlying G... address instead."; +const publicKeyMessage = + "Invalid public key: only G... addresses are supported. Muxed (M...) addresses are not accepted; use the underlying G... address instead."; + export function createWalletRouter(stellar: StellarService): Router { const walletRouter = Router(); - // Fund-movement and key-generation endpoints must never be reachable by an - // unauthenticated caller — gate the whole router. walletRouter.use(jwtAuth); - // --------------------------------------------------------------------------- - // Issue #9 – sendPayment with automatic tx_bad_seq retry - // --------------------------------------------------------------------------- walletRouter.post( "/send", body("sourceSecretKey").isLength({ min: 56 }), body("destinationPublicKey") .isLength({ min: 56, max: 56 }) - .custom((value) => { - if (!isGAddress(value)) { - throw new Error( - "Invalid destination: only G... addresses are supported. " + - "Muxed (M...) addresses are not accepted — use the underlying G... address instead." - ); - } - return true; - }), + .bail() + .custom(isGAddress) + .withMessage(destinationMessage), body("amount").isDecimal({ decimal_digits: "0,7" }), body("asset").optional().isString(), body("memo").optional().isString().isLength({ max: 28 }), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const result = await stellar.sendPayment(req.body); res.json({ hash: result.hash, successful: result.successful }); } catch (err) { @@ -55,13 +49,25 @@ export function createWalletRouter(stellar: StellarService): Router { body("transactionXdr").isString().notEmpty(), body("feeSecretKey").isLength({ min: 56 }), body("fee").optional().isDecimal({ decimal_digits: "0,7" }), - // --------------------------------------------------------------------------- - // Issue #7 – Path payment endpoints (strict-send / strict-receive) - // --------------------------------------------------------------------------- + async (req, res, next) => { + try { + assertValidRequest(req); + const result = await stellar.feeBumpTransaction(req.body); + res.json({ hash: result.hash, successful: result.successful }); + } catch (err) { + next(err); + } + } + ); + walletRouter.post( "/path-payment-strict-send", body("sourceSecretKey").isLength({ min: 56 }), - body("destinationPublicKey").isLength({ min: 56, max: 56 }), + body("destinationPublicKey") + .isLength({ min: 56, max: 56 }) + .bail() + .custom(isGAddress) + .withMessage(destinationMessage), body("sendAmount").isDecimal({ decimal_digits: "0,7" }), body("destAsset").isString(), body("destMin").isDecimal({ decimal_digits: "0,7" }), @@ -69,11 +75,7 @@ export function createWalletRouter(stellar: StellarService): Router { body("memo").optional().isString().isLength({ max: 28 }), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } - const result = await stellar.feeBumpTransaction(req.body); + assertValidRequest(req); const result = await stellar.pathPaymentStrictSend(req.body); res.json({ hash: result.hash, successful: result.successful }); } catch (err) { @@ -85,7 +87,11 @@ export function createWalletRouter(stellar: StellarService): Router { walletRouter.post( "/path-payment-strict-receive", body("sourceSecretKey").isLength({ min: 56 }), - body("destinationPublicKey").isLength({ min: 56, max: 56 }), + body("destinationPublicKey") + .isLength({ min: 56, max: 56 }) + .bail() + .custom(isGAddress) + .withMessage(destinationMessage), body("destAmount").isDecimal({ decimal_digits: "0,7" }), body("destAsset").isString(), body("sendMax").isDecimal({ decimal_digits: "0,7" }), @@ -93,10 +99,7 @@ export function createWalletRouter(stellar: StellarService): Router { body("memo").optional().isString().isLength({ max: 28 }), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const result = await stellar.pathPaymentStrictReceive(req.body); res.json({ hash: result.hash, successful: result.successful }); } catch (err) { @@ -105,19 +108,20 @@ export function createWalletRouter(stellar: StellarService): Router { } ); - // Issue #7 – Horizon path-finding helpers walletRouter.post( "/paths/strict-send", body("sourceAmount").isDecimal({ decimal_digits: "0,7" }), body("sourceAsset").optional().isString(), body("destinationAsset").isString(), - body("destinationPublicKey").optional().isLength({ min: 56, max: 56 }), + body("destinationPublicKey") + .optional() + .isLength({ min: 56, max: 56 }) + .bail() + .custom(isGAddress) + .withMessage(destinationMessage), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const paths = await stellar.findStrictSendPaths(req.body); res.json({ paths }); } catch (err) { @@ -131,13 +135,15 @@ export function createWalletRouter(stellar: StellarService): Router { body("destinationAmount").isDecimal({ decimal_digits: "0,7" }), body("destinationAsset").isString(), body("sourceAsset").optional().isString(), - body("destinationPublicKey").optional().isLength({ min: 56, max: 56 }), + body("destinationPublicKey") + .optional() + .isLength({ min: 56, max: 56 }) + .bail() + .custom(isGAddress) + .withMessage(destinationMessage), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const paths = await stellar.findStrictReceivePaths(req.body); res.json({ paths }); } catch (err) { @@ -146,28 +152,20 @@ export function createWalletRouter(stellar: StellarService): Router { } ); - // --------------------------------------------------------------------------- - // Issue #8 – Multi-signature / threshold signing endpoints - // --------------------------------------------------------------------------- - - /** - * Builds a payment transaction, signs it with the caller's key, and - * returns the partially-signed XDR for co-signers to add their - * signatures. - */ walletRouter.post( "/partial-transaction", body("sourceSecretKey").isLength({ min: 56 }), - body("destinationPublicKey").isLength({ min: 56, max: 56 }), + body("destinationPublicKey") + .isLength({ min: 56, max: 56 }) + .bail() + .custom(isGAddress) + .withMessage(destinationMessage), body("amount").isDecimal({ decimal_digits: "0,7" }), body("asset").optional().isString(), body("memo").optional().isString().isLength({ max: 28 }), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const result = await stellar.buildPartialTransaction(req.body); res.json({ xdr: result.xdr, hash: result.hash }); } catch (err) { @@ -176,20 +174,13 @@ export function createWalletRouter(stellar: StellarService): Router { } ); - /** - * Accepts additional signer secret keys, merges their signatures into - * the partially-signed transaction XDR, and submits to Horizon. - */ walletRouter.post( "/submit-multisig", body("xdr").isString(), body("signerSecretKeys").isArray({ min: 1 }), async (req, res, next) => { try { - const errors = validationResult(req); - if (!errors.isEmpty()) { - return res.status(400).json({ errors: errors.array() }); - } + assertValidRequest(req); const result = await stellar.submitWithAdditionalSignatures(req.body); res.json({ hash: result.hash, successful: result.successful }); } catch (err) { @@ -198,14 +189,12 @@ export function createWalletRouter(stellar: StellarService): Router { } ); - /** - * Returns threshold information for an account so callers can determine - * how many additional signers are required. - */ walletRouter.get( "/:publicKey/thresholds", + param("publicKey").custom(isGAddress).withMessage(publicKeyMessage), async (req, res, next) => { try { + assertValidRequest(req); const result = await stellar.getAccountThresholds(req.params.publicKey); res.json(result); } catch (err) { @@ -219,14 +208,8 @@ export function createWalletRouter(stellar: StellarService): Router { const keypair = stellar.generateKeypair(); const publicKey = keypair.publicKey(); const secretKey = keypair.secret(); - await logKeypairIssuance(req.header("x-api-key") ?? "", publicKey); - - res.json({ - publicKey, - // NOTE: secret is returned only once — store it securely on the client - secretKey, - }); + res.json({ publicKey, secretKey }); } catch (err) { next(err); } diff --git a/src/services/sorobanErrors.ts b/src/services/sorobanErrors.ts index 45d1d76..cb3a21a 100644 --- a/src/services/sorobanErrors.ts +++ b/src/services/sorobanErrors.ts @@ -1,42 +1,33 @@ -/** Thrown when a Soroban contract route is called before its contract ID is configured. */ -export class SorobanNotConfiguredError extends Error { - readonly name = "SorobanNotConfiguredError"; - readonly code = "SOROBAN_NOT_CONFIGURED" as const; - readonly retryable = false; +import { AppError } from "../errors/appError"; +export class SorobanNotConfiguredError extends AppError { constructor(message = "Soroban contract integration is not configured (missing contract ID)") { - super(message); + super(message, { + code: "SOROBAN_NOT_CONFIGURED", + retryable: false, + status: 503, + }); } } -/** - * Thrown when `simulateTransaction` reports a failure. The transaction is - * never submitted in this case — this is the "cheap failure" path: bad - * arguments, a contract error (e.g. a spend-limit rejection), or a missing - * `require_auth()` grant are all caught here for the cost of a read, before - * any fee is paid or a real invocation is attempted. - */ -export class SorobanSimulationError extends Error { - readonly name = "SorobanSimulationError"; - readonly code = "SOROBAN_SIMULATION_FAILED" as const; - readonly retryable = false; - +export class SorobanSimulationError extends AppError { constructor(message: string, readonly raw?: string) { - super(message); + super(message, { + code: "SOROBAN_SIMULATION_FAILED", + details: raw ? { raw } : undefined, + retryable: false, + status: 422, + }); } } -/** - * Thrown when a transaction that passed simulation still lands as FAILED - * on-chain (e.g. contract state changed between simulate and submit). Rare - * by design, since simulation already screens out the common failure modes. - */ -export class SorobanTransactionError extends Error { - readonly name = "SorobanTransactionError"; - readonly code = "SOROBAN_TX_FAILED" as const; - readonly retryable = false; - +export class SorobanTransactionError extends AppError { constructor(message: string, readonly hash: string) { - super(message); + super(message, { + code: "SOROBAN_TX_FAILED", + details: { hash }, + retryable: false, + status: 502, + }); } } diff --git a/src/services/stellar.ts b/src/services/stellar.ts index 0fed1f1..ae0dea9 100644 --- a/src/services/stellar.ts +++ b/src/services/stellar.ts @@ -1,10 +1,11 @@ import * as StellarSdk from "@stellar/stellar-sdk"; +import { NotFoundError } from "../errors/appError"; import { config } from "../config"; import { AccountLock, InProcessAccountLock } from "./locks/accountLock"; -import { SequenceConflictError, MemoRequiredError } from "./stellarErrors"; import { - SequenceConflictError, + MemoRequiredError, NonRetryableHorizonError, + SequenceConflictError, } from "./stellarErrors"; export interface TransactionSummary { @@ -38,7 +39,14 @@ export class StellarService { } async getAccount(publicKey: string) { - return this.server.loadAccount(publicKey); + try { + return await this.server.loadAccount(publicKey); + } catch (err) { + if (hasResponseStatus(err, 404)) { + throw new NotFoundError(`Account ${publicKey} was not found on Horizon`, "ACCOUNT_NOT_FOUND"); + } + throw err; + } } async getBalances(publicKey: string): Promise> { @@ -393,16 +401,9 @@ export class StellarService { const { sourceAmount, sourceAsset, destinationAsset, destinationPublicKey } = params; const srcAsset = sourceAsset ? parseAsset(sourceAsset) : StellarSdk.Asset.native(); const destAst = parseAsset(destinationAsset); + const destination = destinationPublicKey ?? [destAst]; - const query = this.server - .paths() - .strictSend({ - source_asset: srcAsset.isNative() ? undefined : srcAsset, - source_amount: sourceAmount, - }) - .strictReceive(destinationAsset, destAst.isNative() ? undefined : destAst) - .destinationAccount(destinationPublicKey ?? config.HORIZON_URL) - .limit(5); + const query = this.server.strictSendPaths(srcAsset, sourceAmount, destination).limit(5); const result = await query.call(); return result.records as unknown[]; @@ -417,16 +418,9 @@ export class StellarService { const { destinationAmount, destinationAsset, sourceAsset, destinationPublicKey } = params; const destAst = parseAsset(destinationAsset); const srcAsset = sourceAsset ? parseAsset(sourceAsset) : StellarSdk.Asset.native(); + const source = destinationPublicKey ?? [srcAsset]; - const query = this.server - .paths() - .strictReceive({ - destination_asset: destAst.isNative() ? undefined : destAst, - destination_amount: destinationAmount, - }) - .strictSend(sourceAsset, srcAsset.isNative() ? undefined : srcAsset) - .destinationAccount(destinationPublicKey ?? config.HORIZON_URL) - .limit(5); + const query = this.server.strictReceivePaths(source, destAst, destinationAmount).limit(5); const result = await query.call(); return result.records as unknown[]; @@ -481,14 +475,6 @@ export class StellarService { }); } - /** - * Wrap a previously signed transaction in a fee-bump envelope (CAP-15). - * The inner transaction's signatures are untouched — only the outer - * fee-bump envelope is signed with the fee source's key. This lets callers - * rescue a stuck transaction whose base fee was too low for network - * congestion, without re-signing the inner transaction or changing the - * sequence number. - */ async feeBumpTransaction(params: { transactionXdr: string; feeSecretKey: string; @@ -503,11 +489,12 @@ export class StellarService { this.networkPassphrase ); - const feeBumpTx = StellarSdk.FeeBumpTransaction({ - innerTransaction: innerTx, - fee: fee ? String(fee) : String(100 * StellarSdk.BASE_FEE), - feeSource: feePublicKey, - }); + const feeBumpTx = StellarSdk.TransactionBuilder.buildFeeBumpTransaction( + feePublicKey, + fee ?? String(Number(StellarSdk.BASE_FEE) * 100), + innerTx, + this.networkPassphrase + ); feeBumpTx.sign(feeKeypair); @@ -517,11 +504,7 @@ export class StellarService { throw translateSubmissionError(err, feePublicKey); } } - * Merges additional signer signatures into a partially-signed transaction - * XDR and submits the result to Horizon. Callers provide the base64 XDR - * returned by `buildPartialTransaction` plus one or more additional - * secret keys whose signatures satisfy the remaining threshold weight. - */ + async submitWithAdditionalSignatures(params: { xdr: string; signerSecretKeys: string[]; @@ -579,6 +562,11 @@ function isTxBadSeq(err: unknown): boolean { return resultCodes?.transaction === "tx_bad_seq"; } +function hasResponseStatus(err: unknown, status: number): boolean { + const response = err as { response?: { status?: number } } | undefined; + return response?.response?.status === status; +} + function translateSubmissionError(err: unknown, sourcePublicKey: string): unknown { if (err instanceof StellarSdk.BadResponseError) { const resultCodes = (err.response as { extras?: { result_codes?: unknown } } | undefined) diff --git a/src/services/stellarErrors.ts b/src/services/stellarErrors.ts index 67f2127..184da75 100644 --- a/src/services/stellarErrors.ts +++ b/src/services/stellarErrors.ts @@ -1,52 +1,43 @@ -/** - * Thrown when Horizon rejects a submission with tx_bad_seq: the source - * account's sequence number moved between when we read it and when the - * transaction landed. With the per-account lock in place this should be - * rare (it means something outside this process's lock also submitted for - * this account — e.g. another deployed instance without a shared Redis - * lock, or the key being used elsewhere) but callers still need a clear, - * actionable signal rather than a raw Horizon error blob. - */ -export class SequenceConflictError extends Error { - readonly name = "SequenceConflictError"; - readonly code = "SEQUENCE_CONFLICT" as const; - readonly retryable = true; +import { AppError, HorizonError } from "../errors/appError"; +export class SequenceConflictError extends HorizonError { constructor(message: string, readonly horizonResultCodes?: unknown) { - super(message); + super(message, { + code: "SEQUENCE_CONFLICT", + details: horizonResultCodes ? { horizonResultCodes } : undefined, + retryable: true, + status: 409, + }); } } -/** - * Thrown when the destination account requires a memo (SEP-29) but no memo - * was provided. This prevents accidental fund loss when sending to - * custodial/exchange accounts that rely on the memo to credit deposits. - */ -export class MemoRequiredError extends Error { - readonly name = "MemoRequiredError"; - readonly code = "MEMO_REQUIRED" as const; - readonly retryable = false; +export class MemoRequiredError extends AppError { + constructor(message: string) { + super(message, { + code: "MEMO_REQUIRED", + retryable: false, + status: 422, + }); + } } -/** Thrown when a distributed (e.g. Redis) account lock can't be acquired in time. */ -export class LockAcquisitionError extends Error { - readonly name = "LockAcquisitionError"; - readonly code = "LOCK_TIMEOUT" as const; - readonly retryable = true; +export class LockAcquisitionError extends AppError { + constructor(message: string) { + super(message, { + code: "LOCK_TIMEOUT", + retryable: true, + status: 503, + }); + } } -/** - * Thrown when Horizon rejects a submission for a reason other than tx_bad_seq. - * These errors are deterministic — retrying with a fresh sequence number will - * not help (e.g. op_underfunded, op_no_destination). The caller should NOT - * blindly retry. - */ -export class NonRetryableHorizonError extends Error { - readonly name = "NonRetryableHorizonError"; - readonly code = "HORIZON_REJECTED" as const; - readonly retryable = false; - +export class NonRetryableHorizonError extends HorizonError { constructor(message: string, readonly horizonResultCodes?: unknown) { - super(message); + super(message, { + code: "HORIZON_REJECTED", + details: horizonResultCodes ? { horizonResultCodes } : undefined, + retryable: false, + status: 400, + }); } } diff --git a/tests/middleware/errorHandler.test.ts b/tests/middleware/errorHandler.test.ts index b4850c4..3755966 100644 --- a/tests/middleware/errorHandler.test.ts +++ b/tests/middleware/errorHandler.test.ts @@ -43,13 +43,20 @@ describe("errorHandler", () => { errorHandler(new Error("something else broke"), {} as never, res, jest.fn()); expect(res.status).toHaveBeenCalledWith(500); - expect(res.json).toHaveBeenCalledWith({ error: "something else broke" }); + expect(res.json).toHaveBeenCalledWith({ + error: "something else broke", + code: "INTERNAL_ERROR", + }); }); - it("still maps 'Not Found' messages to 404 (no regression)", () => { + it("does not infer 404 from a generic error message", () => { const res = mockRes(); errorHandler(new Error("Not Found"), {} as never, res, jest.fn()); - expect(res.status).toHaveBeenCalledWith(404); + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + error: "Not Found", + code: "INTERNAL_ERROR", + }); }); }); diff --git a/tests/routes/account.muxed.test.ts b/tests/routes/account.muxed.test.ts index 9977661..bde5547 100644 --- a/tests/routes/account.muxed.test.ts +++ b/tests/routes/account.muxed.test.ts @@ -47,8 +47,9 @@ describe("Account routes muxed-address rejection (issue #5)", () => { const res = await request(app) .get(`/api/v1/account/${M_ADDRESS}`) .expect(400); - expect(res.body.errors).toBeDefined(); - expect(JSON.stringify(res.body)).toContain("Muxed"); + expect(res.body.code).toBe("VALIDATION_ERROR"); + expect(res.body.details[0].path).toBe("publicKey"); + expect(res.body.details[0].message).toContain("Muxed"); }); it("accepts a G... address on GET /:publicKey/balances", async () => { @@ -62,8 +63,9 @@ describe("Account routes muxed-address rejection (issue #5)", () => { const res = await request(app) .get(`/api/v1/account/${M_ADDRESS}/balances`) .expect(400); - expect(res.body.errors).toBeDefined(); - expect(JSON.stringify(res.body)).toContain("Muxed"); + expect(res.body.code).toBe("VALIDATION_ERROR"); + expect(res.body.details[0].path).toBe("publicKey"); + expect(res.body.details[0].message).toContain("Muxed"); }); it("accepts a G... address on GET /:publicKey/transactions", async () => { @@ -77,16 +79,16 @@ describe("Account routes muxed-address rejection (issue #5)", () => { const res = await request(app) .get(`/api/v1/account/${M_ADDRESS}/transactions`) .expect(400); - expect(res.body.errors).toBeDefined(); - expect(JSON.stringify(res.body)).toContain("Muxed"); + expect(res.body.code).toBe("VALIDATION_ERROR"); + expect(res.body.details[0].path).toBe("publicKey"); + expect(res.body.details[0].message).toContain("Muxed"); }); }); describe("Wallet /send muxed-address rejection (issue #5)", () => { - const M_ADDRESS = "MAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA7345A"; + const M_ADDRESS = "M" + "A".repeat(55); it("rejects M... destination with 400", async () => { - // Need a valid JWT first const login = await request(app) .post("/api/v1/auth/login") .set("x-api-key", "test-api-key"); @@ -96,12 +98,17 @@ describe("Wallet /send muxed-address rejection (issue #5)", () => { .post("/api/v1/wallet/send") .set("Authorization", `Bearer ${accessToken}`) .send({ - sourceSecretKey: "SABC1234", + sourceSecretKey: "S".repeat(56), destinationPublicKey: M_ADDRESS, amount: "10", }) .expect(400); - expect(res.body.errors).toBeDefined(); - expect(JSON.stringify(res.body)).toContain("Muxed"); + expect(res.body.code).toBe("VALIDATION_ERROR"); + expect(res.body.details).toEqual([ + expect.objectContaining({ + path: "destinationPublicKey", + }), + ]); + expect(res.body.details[0].message).toContain("Muxed"); }); }); diff --git a/tests/routes/wallet.feeBump.test.ts b/tests/routes/wallet.feeBump.test.ts index ef18e3f..1b1c237 100644 --- a/tests/routes/wallet.feeBump.test.ts +++ b/tests/routes/wallet.feeBump.test.ts @@ -40,7 +40,7 @@ describe("POST /api/v1/wallet/send SEP-29 memo check (issue #6)", () => { .post("/api/v1/wallet/send") .set("Authorization", `Bearer ${token}`) .send({ - sourceSecretKey: "SABC1234", + sourceSecretKey: "S".repeat(56), destinationPublicKey: "GBZH7QMRVYFLVYQRY6O5SOM3G7MSQF7MMUEM3WUOGRV26W3R3K5M7G8A", amount: "10", }); @@ -61,7 +61,7 @@ describe("POST /api/v1/wallet/fee-bump (issue #4)", () => { .post("/api/v1/wallet/fee-bump") .set("Authorization", `Bearer ${token}`) .send({ - feeSecretKey: "SABC1234", + feeSecretKey: "S".repeat(56), }) .expect(400); }); @@ -86,7 +86,7 @@ describe("POST /api/v1/wallet/fee-bump (issue #4)", () => { .set("Authorization", `Bearer ${token}`) .send({ transactionXdr: "some-xdr", - feeSecretKey: "SABC1234", + feeSecretKey: "S".repeat(56), }) .expect(200); @@ -102,7 +102,7 @@ describe("POST /api/v1/wallet/fee-bump (issue #4)", () => { .set("Authorization", `Bearer ${token}`) .send({ transactionXdr: "some-xdr", - feeSecretKey: "SABC1234", + feeSecretKey: "S".repeat(56), fee: "5000000", }) .expect(200);