fix: hold secrets in real JS private fields

src/custodians/local/local-signer.ts

@@ -53,14 +53,19 @@ export class LocalSigner implements Custodian {
   /** This custodian signs locally. */
   public readonly kind: CustodianKind = 'local'
 
-  /** Wallets held by this signer, keyed by classic r-address. */
-  private readonly wallets: Map<string, Wallet>
-
   /** The primary account's r-address. */
   private readonly primaryAddress: string
 
+  /**
+   * Wallets held by this signer, keyed by classic r-address. Each carries a seed
+   * and private key, so this is a real JS private field: a TypeScript `private`
+   * is erased at compile time and `console.log`/`util.inspect` would walk
+   * straight into the Map and print the key material.
+   */
+  readonly #wallets: Map<string, Wallet>
+
   private constructor(wallets: Map<string, Wallet>, primaryAddress: string) {
-    this.wallets = wallets
+    this.#wallets = wallets
     this.primaryAddress = primaryAddress
   }
 
@@ -175,7 +180,7 @@ export class LocalSigner implements Custodian {
    * @returns One account per wallet, keyed by r-address.
    */
   public async listAccounts(): Promise<Account[]> {
-    return Array.from(this.wallets.keys(), (address) => ({
+    return Array.from(this.#wallets.keys(), (address) => ({
       address,
       signer: this,
     }))
@@ -262,7 +267,7 @@ export class LocalSigner implements Custodian {
   }
 
   private walletFor(address: string): Wallet {
-    const wallet = this.wallets.get(address)
+    const wallet = this.#wallets.get(address)
     if (wallet === undefined) {
       throw new AccountNotFoundError(address)
     }

src/custodians/palisade/auth/palisade-auth.service.ts

@@ -40,8 +40,11 @@ export interface PalisadeAuthServiceOptions {
   /** The Palisade client ID. */
   clientId: string
   /**
-   * The Palisade client secret. Held in memory only; never logged or
-   * persisted. Long-lived; rotation is the caller's responsibility (TDD §9.5).
+   * The Palisade client secret. Never logged or persisted: the service stores it
+   * in a real JS private field, so it is absent from `console.log` and
+   * `JSON.stringify` output. Note this option object itself is a plain object —
+   * logging the config you pass in will still print the secret. Long-lived;
+   * rotation is the caller's responsibility.
    */
   clientSecret: string
   /** Injectable clock for deterministic tests. Defaults to `Date.now`. */
@@ -67,14 +70,23 @@ export interface PalisadeAuthServiceOptions {
 export class PalisadeAuthService {
   private readonly authPort: PalisadeAuthPort
   private readonly clientId: string
-  private readonly clientSecret: string
   private readonly now: () => number
 
-  private accessToken: string | null = null
   private tokenExpirationMs: number | null = null
   /** Shared in-flight refresh; concurrent callers await this one promise. */
   private refreshPromise: Promise<string> | null = null
 
+  /**
+   * The Palisade client secret. A real JS private field, not a TypeScript
+   * `private`: the latter is erased at compile time, leaving an ordinary
+   * enumerable property that `console.log`, `util.inspect`, and
+   * `JSON.stringify` all print in the clear.
+   */
+  readonly #clientSecret: string
+
+  /** Cached bearer token — `#`-private for the same reason as {@link #clientSecret}. */
+  #accessToken: string | null = null
+
   /**
    * Construct a PalisadeAuthService.
    *
@@ -83,7 +95,7 @@ export class PalisadeAuthService {
   public constructor(options: PalisadeAuthServiceOptions) {
     this.authPort = options.authPort
     this.clientId = options.clientId
-    this.clientSecret = options.clientSecret
+    this.#clientSecret = options.clientSecret
     this.now = options.now ?? Date.now
   }
 
@@ -95,8 +107,8 @@ export class PalisadeAuthService {
    * @returns A valid bearer token.
    */
   public async getToken(forceRefresh = false): Promise<string> {
-    if (!forceRefresh && this.accessToken !== null && !this.isTokenExpired()) {
-      return this.accessToken
+    if (!forceRefresh && this.#accessToken !== null && !this.isTokenExpired()) {
+      return this.#accessToken
     }
     if (!forceRefresh && this.refreshPromise !== null) {
       return this.refreshPromise
@@ -139,7 +151,7 @@ export class PalisadeAuthService {
    * @returns The current cached token, or `null` if none is cached.
    */
   public getCurrentToken(): string | null {
-    return this.accessToken
+    return this.#accessToken
   }
 
   /**
@@ -152,7 +164,7 @@ export class PalisadeAuthService {
     try {
       response = await this.authPort.exchangeCredential(
         this.clientId,
-        this.clientSecret,
+        this.#clientSecret,
       )
     } catch (error) {
       // Never surface credential material; wrap transport/auth failures uniformly.
@@ -168,7 +180,7 @@ export class PalisadeAuthService {
       )
     }
 
-    this.accessToken = token
+    this.#accessToken = token
     this.tokenExpirationMs = this.now() + validityMs(response.expiresIn)
     return token
   }

src/custodians/ripple/auth/custody-auth.service.ts

@@ -56,8 +56,10 @@ export interface CustodyAuthServiceOptions {
   /** Token endpoint port (HTTP in production, in-memory fake in tests). */
   authPort: CustodyAuthPort
   /**
-   * Intent-author private key (PEM). Held in memory only; never logged or
-   * persisted.
+   * Intent-author private key (PEM). Never logged or persisted: the service
+   * stores it in a real JS private field, so it is absent from `console.log`
+   * and `JSON.stringify` output. Note this option object itself is a plain
+   * object — logging the options you pass in will still print the key.
    */
   privateKey: string
   /**
@@ -86,27 +88,37 @@ export interface CustodyAuthServiceOptions {
 export class CustodyAuthService {
   private readonly authPort: CustodyAuthPort
   private readonly keypair: KeypairService
-  private readonly privateKey: string
   private readonly publicKey: string
   private readonly now: () => number
 
-  private accessToken: string | null = null
   private tokenExpirationMs: number | null = null
   /** Shared in-flight refresh; concurrent callers await this one promise. */
   private refreshPromise: Promise<string> | null = null
 
+  /**
+   * The Custody signing key. A real JS private field, not a TypeScript
+   * `private`: the latter is erased at compile time, leaving an ordinary
+   * enumerable property that `console.log`, `util.inspect`, and
+   * `JSON.stringify` all print in the clear. `#`-fields are unreachable at
+   * runtime and skipped by every one of those.
+   */
+  readonly #privateKey: string
+
+  /** Cached bearer token — `#`-private for the same reason as {@link #privateKey}. */
+  #accessToken: string | null = null
+
   /**
    * Construct a CustodyAuthService.
    *
    * @param options - The auth port, private key, and optional public key/clock.
    */
   public constructor(options: CustodyAuthServiceOptions) {
     this.authPort = options.authPort
-    this.privateKey = options.privateKey
+    this.#privateKey = options.privateKey
     this.now = options.now ?? Date.now
-    this.keypair = KeypairService.fromPrivateKey(this.privateKey)
+    this.keypair = KeypairService.fromPrivateKey(this.#privateKey)
     const derivedPublicKey = KeypairService.derivePublicKeyBase64(
-      this.privateKey,
+      this.#privateKey,
     )
     if (
       options.publicKey !== undefined &&
@@ -127,8 +139,8 @@ export class CustodyAuthService {
    * @returns A valid JWT bearer token.
    */
   public async getToken(forceRefresh = false): Promise<string> {
-    if (!forceRefresh && this.accessToken !== null && !this.isTokenExpired()) {
-      return this.accessToken
+    if (!forceRefresh && this.#accessToken !== null && !this.isTokenExpired()) {
+      return this.#accessToken
     }
     if (!forceRefresh && this.refreshPromise !== null) {
       return this.refreshPromise
@@ -171,7 +183,7 @@ export class CustodyAuthService {
    * @returns The current cached token, or `null` if none is cached.
    */
   public getCurrentToken(): string | null {
-    return this.accessToken
+    return this.#accessToken
   }
 
   /**
@@ -198,7 +210,7 @@ export class CustodyAuthService {
       )
     }
 
-    this.accessToken = token
+    this.#accessToken = token
     const exp = extractExpFromJwt(token)
     this.tokenExpirationMs =
       exp === null
@@ -214,7 +226,7 @@ export class CustodyAuthService {
    */
   private signFreshChallenge(): SignedChallenge {
     const challenge = randomUUID()
-    const signature = this.keypair.sign(this.privateKey, challenge)
+    const signature = this.keypair.sign(this.#privateKey, challenge)
     return { challenge, publicKey: this.publicKey, signature }
   }
 }

src/custodians/ripple/auth/intent-signer.ts

@@ -25,7 +25,11 @@ const canonicalize = canonicalizeImport as unknown as (
  */
 export class IntentSigner {
   private readonly keypair: KeypairService
-  private readonly privateKey: string
+  /**
+   * The intent-signing key. A real JS private field so it cannot be reached or
+   * serialized at runtime; a TypeScript `private` is only a compile-time label.
+   */
+  readonly #privateKey: string
 
   /**
    * Construct an IntentSigner.
@@ -43,7 +47,7 @@ export class IntentSigner {
       )
     }
     this.keypair = keypair
-    this.privateKey = privateKey
+    this.#privateKey = privateKey
   }
 
   /**
@@ -61,7 +65,7 @@ export class IntentSigner {
         'Failed to canonicalize Custody intent request body',
       )
     }
-    return this.keypair.sign(this.privateKey, canonical)
+    return this.keypair.sign(this.#privateKey, canonical)
   }
 
   /**

test/unit/security/secret-exposure.test.ts

@@ -0,0 +1,143 @@
+import { inspect } from 'node:util'
+
+import { Wallet } from 'xrpl'
+
+import { PalisadeAuthService } from '../../../src/custodians/palisade/auth/palisade-auth.service.js'
+import { CustodyAuthService } from '../../../src/custodians/ripple/auth/custody-auth.service.js'
+import { IntentSigner } from '../../../src/custodians/ripple/auth/intent-signer.js'
+import { KeypairService } from '../../../src/custodians/ripple/auth/keypair.service.js'
+import { LocalSigner } from '../../../src/index.js'
+import { generateTestKey } from '../custody-auth/test-utils.js'
+
+/**
+ * Secrets must survive being printed or serialized.
+ *
+ * TypeScript's `private` is erased at compile time, so a `private` field is an
+ * ordinary enumerable property at runtime: `console.log(custodian)` (which uses
+ * `util.inspect`) and `JSON.stringify(authService)` both walk into it and emit
+ * the value in the clear. The fields these tests cover therefore use real JS
+ * private fields (`#name`), which are unreachable at runtime and skipped by both.
+ *
+ * The assertions target the *observable* property — the secret is absent from
+ * both renderings — not the mechanism, so they keep holding if the
+ * implementation changes (a redacting `toJSON`, a custom inspect hook, …) and
+ * fail if a field is reverted to `private`.
+ */
+
+const TOKEN = 'test-access-token-value-do-not-leak'
+const CLIENT_SECRET = 'test-client-secret-do-not-leak'
+
+/**
+ * The base64 body of a PEM key — a needle guaranteed to be key material rather
+ * than the `-----BEGIN …-----` boilerplate every PEM shares.
+ *
+ * @param pem - The PEM-encoded key.
+ * @returns A substring of the key body.
+ */
+function keyBody(pem: string): string {
+  const body = pem
+    .split('\n')
+    .filter((line) => !line.startsWith('-----') && line.trim() !== '')
+    .join('')
+  return body.slice(0, 24)
+}
+
+/**
+ * Assert a secret appears in neither of the two renderings that leak it.
+ *
+ * @param value - The object to render.
+ * @param secret - The substring that must not appear.
+ */
+function expectNotLeaked(value: unknown, secret: string): void {
+  expect(secret.length).toBeGreaterThan(8)
+  // `depth: null` walks the whole graph, so a secret nested behind several
+  // objects (custodian → http client → auth service) is still caught.
+  expect(inspect(value, { depth: null })).not.toContain(secret)
+  expect(JSON.stringify(value)).not.toContain(secret)
+}
+
+describe('CustodyAuthService', () => {
+  it('does not expose its signing key to console.log or JSON.stringify', () => {
+    const pem = generateTestKey('ed25519')
+    const service = new CustodyAuthService({
+      authPort: {
+        fetchToken: async (): Promise<{ access_token: string }> => ({
+          access_token: TOKEN,
+        }),
+      },
+      privateKey: pem,
+    })
+    // The needle is genuinely part of the key we handed in, so a pass means
+    // concealment rather than a mis-built assertion.
+    expect(pem).toContain(keyBody(pem))
+    expectNotLeaked(service, keyBody(pem))
+  })
+
+  it('does not expose the cached bearer token once one is fetched', async () => {
+    const service = new CustodyAuthService({
+      authPort: {
+        fetchToken: async (): Promise<{ access_token: string }> => ({
+          access_token: TOKEN,
+        }),
+      },
+      privateKey: generateTestKey('ed25519'),
+    })
+    await service.getToken()
+    // Still reachable through the intended accessor — concealed, not removed.
+    expect(service.getCurrentToken()).toBe(TOKEN)
+    expectNotLeaked(service, TOKEN)
+  })
+})
+
+describe('PalisadeAuthService', () => {
+  it('does not expose its client secret or cached token', async () => {
+    const service = new PalisadeAuthService({
+      // Deliberately not the shared `FakeAuthPort`: that fixture records every
+      // `(clientId, clientSecret)` pair it receives so other tests can assert
+      // on them, which would surface the secret through the *port* rather than
+      // the service under test. A port is handed the secret by design; what
+      // matters here is that the service's own state doesn't retain it visibly.
+      authPort: {
+        exchangeCredential: async (): Promise<{
+          accessToken: string
+          expiresIn: number
+        }> => ({ accessToken: TOKEN, expiresIn: 3600 }),
+      },
+      clientId: 'client-id-is-not-secret',
+      clientSecret: CLIENT_SECRET,
+    })
+    expectNotLeaked(service, CLIENT_SECRET)
+
+    expect(await service.getToken()).toBe(TOKEN)
+    expectNotLeaked(service, CLIENT_SECRET)
+    expectNotLeaked(service, TOKEN)
+  })
+})
+
+describe('IntentSigner', () => {
+  it('does not expose the key it signs with', () => {
+    const pem = generateTestKey('ed25519')
+    const signer = new IntentSigner(KeypairService.fromPrivateKey(pem), pem)
+    expectNotLeaked(signer, keyBody(pem))
+  })
+})
+
+describe('LocalSigner', () => {
+  it('does not expose the wallet seed or private key', () => {
+    const wallet = Wallet.generate()
+    const signer = LocalSigner.fromSeed(wallet.seed as string)
+    // Still functional — the wallets are concealed, not discarded.
+    expect(signer.primary.address).toBe(wallet.classicAddress)
+    expectNotLeaked(signer, wallet.seed as string)
+    expectNotLeaked(signer, wallet.privateKey)
+  })
+
+  it('does not expose any seed when holding multiple wallets', () => {
+    const wallets = [Wallet.generate(), Wallet.generate()]
+    const signer = LocalSigner.create({ wallets })
+    for (const wallet of wallets) {
+      expectNotLeaked(signer, wallet.seed as string)
+      expectNotLeaked(signer, wallet.privateKey)
+    }
+  })
+})