Skip to content

fix: mask the Custody login challenge in debug logs, and harden timeouts and key handling before 0.3.0 - #61

Merged
pdp2121 merged 2 commits into
mainfrom
fix/pre-release-review
Nov 5, 2026
Merged

pdp2121 merged 2 commits into
mainfrom
fix/pre-release-review

Conversation

@pdp2121

@pdp2121 pdp2121 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

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, and public_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 are secret, password, passphrase, and encryptedSeed, which can appear in bodies sent or returned through api.call.

Timeouts throw a typed CustodyTimeoutError. Previously a request that hit the 30 s default rejected with a bare DOMException. The new error:

  • names the method, path, and timeout, and says the request "may still have been processed";
  • for an intent proposal, carries intentId, so the caller checks it with api.waitForIntent or re-submits with the same idempotency key instead of proposing a duplicate;
  • is recognised by name, not class, so it also works across realms (e.g. a consumer's Jest sandbox).

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, and client.intent.await, which share the loop.

Unusable public keys throw instead of passing garbage on. getPublicKey and raw signing used to hex-encode any unrecognised key encoding as the SigningPubKey. They now throw a clear error. An ed25519 SPKI key is converted to XRPL's ED… 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, and openapi/README.md says to fetch the official one only when a feature needs it.

errors.ts joins 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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Test Plan

  • Unit:
    • Masking: the exact Custody token form is logged with signature and challenge masked and the token hidden; secret, password, and encryptedSeed are masked in request and response bodies.
    • Timeouts: a timed-out request becomes CustodyTimeoutError with its method, path, timeout, and cause. A timed-out intent proposal carries its intentId. Other transport errors pass through unchanged.
    • Waits: with a 6.5 s budget, reads land at 0, 1, 3, and 6.5 s and the wait resolves only at the deadline. A timed-out read is retried until the intent executes.
    • Keys: ed25519 SPKI converts to ED…. P-256 SPKI and unrecognised bytes throw.
  • Full suite: typecheck, lint, format, unit tests (888), script tests, and build all pass. Typegen shows no drift.
  • Script tests: they drop from 22 to 21 because one test runs once per vendored spec and the official internal spec is gone.

@pdp2121
pdp2121 merged commit 9acf5a4 into main Nov 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant