fix: validate signature inputs across Keypair.verify and authorizeEntry - #1649
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds stricter signature validation while supporting XDR signature wrappers across SDK instances.
Changes:
- Accepts
xdr.Signaturein verification APIs. - Validates signer outputs and custom authorization
ScValvalues. - Adds runtime/type tests and updates generated references and changelog.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/base/keypair.ts |
Validates verification inputs and unwraps signatures. |
src/base/auth.ts |
Validates authorization signer results. |
test/unit/base/keypair.test.ts |
Tests verification input behavior. |
test/unit/base/auth.test.ts |
Tests authorization validation. |
test/types/keypair-verify.ts |
Locks accepted verification types. |
docs/reference/core-keys.md |
Updates generated key API reference. |
docs/reference/core-soroban-primitives.md |
Refreshes generated source links. |
CHANGELOG.md |
Documents breaking validation changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/base/keypair.ts:323
- A structurally branded object with a callable
toBytescan still throw from that method (or from a getter used byisSignature). Because extraction happens before the validation/catch, that arbitrary error escapes instead of the documentedTypeErrorfor an unusable signature input. Normalize extraction failures through the same type guard error.
const signatureBytes = isSignature(signature)
? signature.toBytes()
: signature;
src/base/auth.ts:91
- The
tryonly covers invokingtoXdrObject; a throwingconstructorortoXdrObjectgetter escapes directly. Such a malformed branded value therefore bypasses the intendedsignatureScVal must be an xdr.ScValTypeError, even though an equally unusable throwing method is normalized correctly. Include all structural property access in the guarded block.
const ctor = value.constructor as { schema?: { name?: string } } | undefined;
if (ctor?.schema?.name !== ScVal.schema.name) return null;
const toXdrObject = (value as { toXdrObject?: () => ScValWire }).toXdrObject;
Ryang-21
reviewed
Aug 14, 2026
Ryang-21
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Keypair.verifyandverifyMessagereporting a valid signature as invalid when handed thexdr.Signaturewrapper thatDecoratedSignature.signatureholds —kp.verify(tx.hash(), tx.signatures[0].signature)returnedfalseTypeErrorfor arguments whose type they don't accept instead of returningfalse— breaking, and the reason the above went unnoticed: a caller mistake was indistinguishable from a forgery.verifytakesdataas aUint8Arrayandsignatureas aUint8Arrayorxdr.Signature;verifyMessagetakes the samesignatureplus a string orUint8Arraymessage. A base64 signature string, a plainnumber[], or the wholexdr.DecoratedSignaturenow throws, andsignMessagedoes the same for a message that is neither string nor bytesauthorizeEntryaccepting asignatureScValthat isn't anxdr.ScVal, which produced a signed-looking entry that only failed later attoXdr(). The value is round-tripped through the localScValschema, so a lookalike carrying the brand but not encoding as anScValis rejected at the call, and the credential stores a locally rebuilt value rather than the caller's objectsignatureandpublicKeya signer returns, where the callback and signer-object paths converge — nothing checked them before, so a wrong shape drew a misleadingsignature doesn't match payload, and onceverifylearned to acceptxdr.Signature, a wrapper would have reachednativeToScValtwo layers down insteadxdr.Signatureandxdr.ScValwrappers structurally instead of withinstanceof, and byte arrays withisUint8Array, so values from another realm or a second copy of the SDK in one process still workverifyandverifyMessageaccept@throwsdocs forverify,verifyMessageandsignMessageto name the accepted and rejected types — the old wording contradicted a string message and anxdr.Signature— and regenerateddocs/reference/