From 20e06917b52d317ff96018427a7a9ea20fde48e6 Mon Sep 17 00:00:00 2001 From: therealbibson <136495186+therealbibson@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:40:30 +0100 Subject: [PATCH] feat(signing): validate key format before signing in KeypairSigner (#768) --- src/client.ts | 6 +-- src/errors.ts | 20 ++++++++ src/index.ts | 3 ++ src/signing/adapters/KeypairSigner.ts | 32 ++++++++++-- test/sdkExports.test.ts | 2 + test/signing.test.ts | 73 ++++++++++++++++++++++++++- 6 files changed, 129 insertions(+), 7 deletions(-) diff --git a/src/client.ts b/src/client.ts index 262ffb2..8b44892 100644 --- a/src/client.ts +++ b/src/client.ts @@ -2512,7 +2512,7 @@ export class StellarSplitClient extends TypedEventEmitter { * * @returns The new invoice ID and the transaction hash. * @example - * const result = await client.createInvoice({ /* params */ }); + * const result = await client.createInvoice({ ...params }); * @param params - The parameters for the method. * @throws {Error} If the method fails. */ @@ -2909,7 +2909,7 @@ export class StellarSplitClient extends TypedEventEmitter { * * @returns The transaction hash. * @example - * const result = await client.pay({ /* params */ }); + * const result = await client.pay({ ...params }); * @param params - The parameters for the method. * @throws {Error} If the method fails. */ @@ -3454,7 +3454,7 @@ export class StellarSplitClient extends TypedEventEmitter { * the invoice data is returned. Throws {@link TokenGateAccessDeniedError} when * the caller does not meet the balance requirement (and `strict !== false`). * @example - * const result = await client.getInvoice({ /* params */ }); + * const result = await client.getInvoice({ ...params }); * @param params - The parameters for the method. * @returns The result of the method. * @throws {Error} If the method fails. diff --git a/src/errors.ts b/src/errors.ts index 0dfeae4..abc7da7 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -2211,3 +2211,23 @@ export class MergeConflictError extends StellarSplitError { export function isMergeConflictError(err: unknown): err is MergeConflictError { return err instanceof MergeConflictError; } + +// --------------------------------------------------------------------------- +// Keypair format and signing validation errors (issue #768) +// --------------------------------------------------------------------------- + +/** + * Thrown when a KeypairSigner is constructed with an invalid secret key or keypair. + */ +export class InvalidKeypairError extends StellarSplitError { + constructor(message: string, context?: Record, raw?: string) { + super(message, "INVALID_KEYPAIR", context, raw); + this.name = "InvalidKeypairError"; + Object.setPrototypeOf(this, new.target.prototype); + } +} + +export function isInvalidKeypairError(err: unknown): err is InvalidKeypairError { + return err instanceof InvalidKeypairError; +} + diff --git a/src/index.ts b/src/index.ts index 5295dac..4941fad 100644 --- a/src/index.ts +++ b/src/index.ts @@ -213,6 +213,9 @@ export { SdkError, SdkErrorCode, isSdkError, + // Keypair format and signing validation (issue #768) + InvalidKeypairError, + isInvalidKeypairError, } from "./errors.js"; // Invoice metadata JSON Schema validator (issue #533) diff --git a/src/signing/adapters/KeypairSigner.ts b/src/signing/adapters/KeypairSigner.ts index e795a69..6ddc542 100644 --- a/src/signing/adapters/KeypairSigner.ts +++ b/src/signing/adapters/KeypairSigner.ts @@ -1,9 +1,10 @@ import { Keypair } from "@stellar/stellar-sdk"; import type { Signer } from "../signer.js"; +import { InvalidKeypairError } from "../../errors.js"; /** * {@link Signer} backed by an in-memory {@link Keypair} from - * `@stellar/stellar-sdk`. + * `@stellar/stellar-sdk` or a raw Stellar secret key string. * * Useful for local development and for the common case where the secret seed * already lives in the process (e.g. loaded from an environment variable). @@ -12,8 +13,32 @@ export class KeypairSigner implements Signer { /** The wrapped keypair. */ readonly keypair: Keypair; - constructor(keypair: Keypair) { - this.keypair = keypair; + constructor(secretOrKeypair: Keypair | string) { + if (typeof secretOrKeypair === "string") { + try { + this.keypair = Keypair.fromSecret(secretOrKeypair); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new InvalidKeypairError(`Invalid secret key format: ${message}`); + } + } else if ( + secretOrKeypair instanceof Keypair || + (secretOrKeypair && + typeof secretOrKeypair === "object" && + typeof (secretOrKeypair as Keypair).canSign === "function" && + typeof (secretOrKeypair as Keypair).sign === "function") + ) { + if (!secretOrKeypair.canSign()) { + throw new InvalidKeypairError( + "Keypair does not contain a secret key for signing", + ); + } + this.keypair = secretOrKeypair as Keypair; + } else { + throw new InvalidKeypairError( + "Invalid secret key: expected a Stellar secret key string or Keypair instance", + ); + } } /** @@ -24,3 +49,4 @@ export class KeypairSigner implements Signer { return Buffer.from(this.keypair.sign(txHash)); } } + diff --git a/test/sdkExports.test.ts b/test/sdkExports.test.ts index dcded5f..ebf18b3 100644 --- a/test/sdkExports.test.ts +++ b/test/sdkExports.test.ts @@ -4,6 +4,8 @@ import * as sdk from "../src/index.js"; describe("public API surface (issues #586, #588, #589)", () => { it("exports the signing vault adapters", () => { expect(typeof sdk.KeypairSigner).toBe("function"); + expect(typeof sdk.InvalidKeypairError).toBe("function"); + expect(typeof sdk.isInvalidKeypairError).toBe("function"); expect(typeof sdk.EncryptedFileSigner).toBe("function"); expect(typeof sdk.CloudKmsSigner).toBe("function"); expect(typeof sdk.encryptSigningKeyToPem).toBe("function"); diff --git a/test/signing.test.ts b/test/signing.test.ts index 3c27ab1..c8b4d8c 100644 --- a/test/signing.test.ts +++ b/test/signing.test.ts @@ -13,10 +13,16 @@ import { encryptSigningKeyToPem, } from "../src/signing/adapters/EncryptedFileSigner.js"; +import { + InvalidKeypairError, + isInvalidKeypairError, + StellarSplitError, +} from "../src/errors.js"; + const TX_HASH = randomBytes(32); describe("KeypairSigner", () => { - it("produces a 64-byte ed25519 signature verifiable by Keypair.verify", async () => { + it("produces a 64-byte ed25519 signature verifiable by Keypair.verify when initialized with Keypair", async () => { const keypair = Keypair.random(); const signer = new KeypairSigner(keypair); @@ -35,6 +41,71 @@ describe("KeypairSigner", () => { expect(other.verify(TX_HASH, signature)).toBe(false); }); + + it("constructs and signs correctly when initialized with a valid secret key string", async () => { + const keypair = Keypair.random(); + const secret = keypair.secret(); + const signer = new KeypairSigner(secret); + + expect(signer.keypair.publicKey()).toBe(keypair.publicKey()); + const signature = await signer.sign(TX_HASH); + expect(signature).toHaveLength(64); + expect(keypair.verify(TX_HASH, signature)).toBe(true); + }); + + it("throws InvalidKeypairError when secret key does not start with 'S' (e.g. public key)", () => { + const publicKey = "GBYVQHUDHLWQMS5GZZ7W4P6OCBW5MVAOUXCTFB7VOVLIKGOOQT5ATOZ4"; + expect(() => new KeypairSigner(publicKey)).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner(publicKey)).toThrow(/Invalid secret key format/); + }); + + it("throws InvalidKeypairError when secret key has invalid length or base32 encoding", () => { + expect(() => new KeypairSigner("S123")).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner("S123")).toThrow(/Invalid secret key format/); + expect(() => new KeypairSigner("not-a-secret-key")).toThrow(InvalidKeypairError); + }); + + it("throws InvalidKeypairError when secret key has invalid checksum", () => { + const invalidChecksumKey = "SAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + expect(() => new KeypairSigner(invalidChecksumKey)).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner(invalidChecksumKey)).toThrow(/invalid checksum/i); + }); + + it("throws InvalidKeypairError when secret key is empty string", () => { + expect(() => new KeypairSigner("")).toThrow(InvalidKeypairError); + }); + + it("throws InvalidKeypairError when Keypair cannot sign (public key only)", () => { + const pubKeypair = Keypair.fromPublicKey("GBYVQHUDHLWQMS5GZZ7W4P6OCBW5MVAOUXCTFB7VOVLIKGOOQT5ATOZ4"); + expect(() => new KeypairSigner(pubKeypair)).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner(pubKeypair)).toThrow(/Keypair does not contain a secret key for signing/); + }); + + it("throws InvalidKeypairError for invalid input types", () => { + expect(() => new KeypairSigner(null as unknown as string)).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner(undefined as unknown as string)).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner(12345 as unknown as string)).toThrow(InvalidKeypairError); + expect(() => new KeypairSigner({} as unknown as Keypair)).toThrow(InvalidKeypairError); + }); + + it("InvalidKeypairError has correct code, name, and prototype hierarchy", () => { + const err = new InvalidKeypairError("format error"); + expect(err).toBeInstanceOf(Error); + expect(err).toBeInstanceOf(StellarSplitError); + expect(err).toBeInstanceOf(InvalidKeypairError); + expect(err.name).toBe("InvalidKeypairError"); + expect(err.code).toBe("INVALID_KEYPAIR"); + expect(err.message).toBe("format error"); + }); + + it("isInvalidKeypairError correctly identifies InvalidKeypairError instances", () => { + const err = new InvalidKeypairError("test"); + expect(isInvalidKeypairError(err)).toBe(true); + expect(isInvalidKeypairError(new Error("test"))).toBe(false); + expect(isInvalidKeypairError(null)).toBe(false); + expect(isInvalidKeypairError(undefined)).toBe(false); + expect(isInvalidKeypairError({ name: "InvalidKeypairError" })).toBe(false); + }); }); describe("CloudKmsSigner", () => {