Skip to content

fix: hold secrets in real JS private fields - #32

Merged
pdp2121 merged 1 commit into
mainfrom
secret-logging-fixes
Aug 13, 2026
Merged

fix: hold secrets in real JS private fields#32
pdp2121 merged 1 commit into
mainfrom
secret-logging-fixes

Conversation

@pdp2121

@pdp2121 pdp2121 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

TypeScript's private is erased at compile time, so secrets were ordinary enumerable properties at runtime — console.log(custodian) and JSON.stringify(authService) both printed them in the clear, contradicting the code's own "never logged" comments. Reproduced before fixing.

Converted to # private fields, which are unreachable at runtime and skipped by both:

Class Fields
CustodyAuthService #privateKey, #accessToken
PalisadeAuthService #clientSecret, #accessToken
IntentSigner #privateKey
LocalSigner #wallets (seeds + private keys)

Scoped to the leaves that actually hold secrets — every exposure path terminates at one of them, so this also closes the console.log(custodian) case. Neither custodian retains its raw config object.

One correction to the ticket: JSON.stringify did not leak LocalSigner's wallets (a Map has no enumerable own properties) — but console.log did, walking into the Map and printing every seed. Console-only for that field.

Residual, documented not fixed: the options/config objects callers construct (CustodyAuthServiceOptions.privateKey, PalisadeCustodyConfig.credentials.*.clientSecret) are plain objects. #-fields can't protect those; console.log(myConfig) still prints the secret. Both doc comments now say this instead of overclaiming.

Also verified the auth ports don't re-expose: they pass the secret to http.send, retain nothing, and their errors carry only the status.

Also

  • New test/unit/security/secret-exposure.test.ts (6 cases) — asserts the secret is absent from both renderings rather than testing the mechanism, so it survives an implementation change and fails if a field reverts to private.
  • Corrected the two "held in memory only; never logged" comments the ticket cited.

@pdp2121
pdp2121 requested a review from cybele-ripple August 12, 2026 17:13
@pdp2121
pdp2121 merged commit d9795e7 into main Aug 13, 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.

2 participants