Repository navigation
fix: mask the Custody login challenge in debug logs, and harden timeouts and key handling before 0.3.0 - #61
Merged
Merged
Conversation
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.
High Level Overview of Change
Fixes from the pre-release review of everything since v0.2.0.
Debug logging leaked the Custody login credential (blocker). The token request sends
challenge,signature, andpublic_key, and only the response's tokens were masked. The SDK picks the challenge itself, so a logged challenge and its signature could be exchanged for a token. Both are now masked. So aresecret,password,passphrase, andencryptedSeed, which can appear in bodies sent or returned throughapi.call.Timeouts throw a typed
CustodyTimeoutError. Previously a request that hit the 30 s default rejected with a bareDOMException. The new error:intentId, so the caller checks it withapi.waitForIntentor re-submits with the same idempotency key instead of proposing a duplicate;A read that times out while waiting on an intent now counts as "no answer this time", and polling continues.
Intent waits use their whole budget. The loop used to stop as soon as the next backoff would pass the deadline, so a 30 s wait gave up after about 15 s. It now sleeps up to the deadline and makes a last read there, so the reported "still Open after N ms" is accurate. This also fixes
waitForIntent,proposeAndWait,submitAndWait, andclient.intent.await, which share the loop.Unusable public keys throw instead of passing garbage on.
getPublicKeyand raw signing used to hex-encode any unrecognised key encoding as theSigningPubKey. They now throw a clear error. An ed25519 SPKI key is converted to XRPL'sED…form.Dropped the official internal Custody spec (
openapi/custody/official/internal.json). Nothing used it; the cMPT inbox decryption needs only the devbox internal spec. Internal types are now generated from the devbox spec alone, andopenapi/README.mdsays to fetch the official one only when a feature needs it.errors.tsjoins the other facade files under the 350-line lint allowance.Context of Change
An independent pre-release review of v0.2.0..main found the logging leak (blocker), plus the untyped timeout, the wait budget, and the key-encoding fallback. The timeout one could lead to a duplicate payment on retry. The internal spec is dropped because the internal API isn't meant for clients, and the official spec was the one part with no consumer.
Type of Change
Test Plan
signatureandchallengemasked and the token hidden;secret,password, andencryptedSeedare masked in request and response bodies.CustodyTimeoutErrorwith its method, path, timeout, and cause. A timed-out intent proposal carries itsintentId. Other transport errors pass through unchanged.ED…. P-256 SPKI and unrecognised bytes throw.