Skip to content

fix(api): Sign in with CoinPay sends PKCE + nonce; Connect/Disconnect in settings - #173

Merged
sh1pt-actions-fleet[bot] merged 1 commit into
mainfrom
fix/coinpay-pkce-nonce
Oct 29, 2026
Merged

sh1pt-actions-fleet[bot] merged 1 commit into
mainfrom
fix/coinpay-pkce-nonce

Conversation

@sh1pt-actions-fleet

Copy link
Copy Markdown
Contributor

Audit of Sign in with CoinPay against the house relying-party pattern and the STANDING sub rule.

Already right (#171): binds to the CoinPay sub; the CoinPay email never links to an existing account, is never marked verified, and a password signup can reclaim it; no admin/plan from it.

Fixed here (so client cp_56ff186e… can leave CoinPay's legacy mode):

  • PKCE S256 + state + nonce on authorize (via @profullstack/coinpay-onboard/oauth 0.3.1); pending request in one httpOnly cookie scoped to /api/auth/coinpay.
  • Callback: constant-time state compare; code exchange sends the verifier and the client secret (post only; previously Basic + post together, no verifier); id_token iss/aud/exp/nonce checked; userinfo sub must match.
  • Scopes now exactly openid profile email. The old did wallet:read were never registered on the client, so CoinPay had been dropping them.
  • New: Settings → Connect CoinPay (session required) / Disconnect (refused when the account has no password). /api/auth/me adds coinpayConnected, canDisconnectCoinpay.

Note: bumping coinpay-onboard 0.2.0 → 0.3.1 brings zod 4.6.7, which the langchain packages now resolve as their zod peer (they accept ^3.25 || ^4). Typecheck + all API tests pass.

Tests: services/api vitest 126 pass (new: connect needs a session, linked-elsewhere/already-connected refused, authorize URL has S256/state/nonce, forged state, wrong nonce, pending cookie), tsc clean, eslint 0 errors.

🤖 Generated with Claude Code

… in settings

The CoinPay client (cp_56ff186e…) never sent PKCE, so it had to stay in
CoinPay's legacy mode. Now, via @profullstack/coinpay-onboard/oauth 0.3.1:

- start: PKCE S256 + state + nonce, kept in one httpOnly cookie scoped to
  /api/auth/coinpay; scopes exactly openid profile email (the old request
  asked for did + wallet:read, which the client was never registered for,
  so CoinPay dropped them; sign-in only uses the sub).
- callback: constant-time state check; code exchange with the verifier and
  the client secret (client_secret_post only, not both methods); id_token
  iss/aud/exp/nonce checked and the userinfo sub matched.
- Sub binding unchanged (#171): a CoinPay login never claims an account by
  email.
- New: Settings -> Connect CoinPay (signed in only) / Disconnect (refused
  when the account has no password). /api/auth/me reports both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

ThreatCrush Security Scan

51 finding(s)

HIGH/CRITICAL: 2 | MEDIUM: 28 | LOW: 21

Severity Rule Location
HIGH py-ssrf-outbound-request apps/desktop/launcher/tron-tor-helper:303
HIGH py-ssrf-outbound-request apps/desktop/launcher/tron-tor-helper:437
MEDIUM js-open-redirect apps/desktop/extensions/ai-sidebar/install-helper.js:156
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/media.js:34
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/media.js:57
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/newtab.js:246
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/newtab.js:275
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/newtab.js:345
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/options.js:306
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/sidepanel.js:78
MEDIUM js-unescaped-html-sink apps/desktop/extensions/ai-sidebar/sidepanel.js:166
MEDIUM redos-nested-quantifier apps/desktop/extensions/ai-sidebar/w3bs.js:17
MEDIUM insecure-randomness-for-secret apps/extensions/public/seal.js:107
MEDIUM js-unescaped-html-sink apps/extensions/public/store.js:77
MEDIUM js-unescaped-html-sink apps/extensions/public/store.js:286
MEDIUM js-unescaped-html-sink apps/extensions/public/store.js:652
MEDIUM js-unescaped-html-sink apps/web/public/app.js:29
MEDIUM js-unescaped-html-sink apps/web/public/dns.js:54
MEDIUM sh-remote-script-execution apps/web/public/install.sh:182
MEDIUM sh-remote-script-execution apps/web/public/install.sh:422
MEDIUM sh-remote-script-execution apps/web/public/install.sh:440
MEDIUM js-unescaped-html-sink apps/web/public/settings.js:122
MEDIUM js-unescaped-html-sink apps/web/public/settings.js:194
MEDIUM js-dynamic-code-execution packages/sdk/src/mcp/tools.ts:125
MEDIUM js-dynamic-code-execution packages/sdk/src/page.ts:68
MEDIUM js-dynamic-code-execution packages/sdk/src/page.ts:73
MEDIUM js-unescaped-html-sink services/api/src/profile.ts:59
MEDIUM sql-template-interpolation services/api/src/store/db.ts:116
MEDIUM js-dynamic-code-execution services/api/src/store/scanner.ts:44
MEDIUM sh-predictable-temp-path start.sh:28
LOW js-dynamic-code-execution apps/desktop/extensions/ai-sidebar/pit-proxy.test.js:8
LOW js-dynamic-code-execution packages/agent-runtime/src/analyze/form-script.test.ts:6
LOW js-unescaped-html-sink packages/agent-runtime/src/analyze/form-script.test.ts:26
LOW js-unescaped-html-sink packages/agent-runtime/src/analyze/form-script.test.ts:49
LOW js-dynamic-code-execution packages/browser-core/src/automation/extract-script.test.ts:6
LOW js-unescaped-html-sink packages/browser-core/src/automation/extract-script.test.ts:16
LOW js-unescaped-html-sink packages/browser-core/src/automation/extract-script.test.ts:45
LOW js-unescaped-html-sink packages/browser-core/src/automation/extract-script.test.ts:57
LOW js-unescaped-html-sink packages/browser-core/src/automation/extract-script.test.ts:80
LOW js-unescaped-html-sink packages/browser-core/src/automation/extract-script.test.ts:99
LOW js-dynamic-code-execution packages/browser-core/src/automation/snapshot-script.test.ts:11
LOW js-unescaped-html-sink packages/browser-core/src/automation/snapshot-script.test.ts:62
LOW js-unescaped-html-sink packages/browser-core/src/automation/snapshot-script.test.ts:101
LOW js-unescaped-html-sink packages/provenance/src/scan.test.ts:47
LOW js-unescaped-html-sink packages/provenance/src/scan.test.ts:170
LOW js-unescaped-html-sink packages/provenance/src/scan.test.ts:216
LOW js-dynamic-code-execution packages/sdk/src/mcp/automate.test.ts:261
LOW secret-generic-api-key packages/storage/src/config.ts:51
LOW secret-generic-credential packages/storage/src/config.ts:51
LOW js-dynamic-code-execution services/api/src/store/scanner.test.ts:32

…and 1 more. Full results in the Security tab.

Snippets are redacted; ThreatCrush never prints matched credential material.

@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updated@​profullstack/​coinpay-onboard@​0.2.0 ⏵ 0.3.176100100 +293 +1100

View full report

@sh1pt-actions-fleet
sh1pt-actions-fleet Bot merged commit 9153267 into main Oct 29, 2026
11 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