Skip to content

Fix NIP-46 auth challenge timeout#2255

Open
SnowCait wants to merge 2 commits into
mainfrom
codex/-auth-challenge
Open

Fix NIP-46 auth challenge timeout#2255
SnowCait wants to merge 2 commits into
mainfrom
codex/-auth-challenge

Conversation

@SnowCait

Copy link
Copy Markdown
Owner

Motivation

  • establishBunkerConnection()onauth を受け取った後に既存の接続タイマーで reject しなくなり、ユーザーが外部で認証を完了しないとログイン処理が無期限待機する不具合を修正するため。
  • NIP-46 の仕様上、Auth Challenge の後で最終レスポンスが届かないことがあるため、Auth Challenge 後にも有限のタイムアウトを設ける必要がある。

Description

  • web/src/lib/Constants.ts に新しい定数 nip46AuthTimeout = 5 * 60 * 1000 を追加して、Auth Challenge 後の待機上限を定義した。
  • web/src/lib/signer-strategy.tsestablishBunkerConnection() を接続/認証で別々のタイマー管理に差し替え、connectTimer(接続タイムアウト)と authTimer(Auth Challenge 後タイムアウト)を導入し、終了済みを示す settled フラグと clearTimers() を実装して不要な再起動や解除漏れを防止した。
  • タイムアウト時のエラーメッセージを区別可能にして、接続タイムアウトは NIP-46 connection timed out、認証待機タイムアウトは NIP-46 authentication timed out を投げるようにした。
  • web/src/lib/signer-strategy.test.ts に最小限の nostr-tools/nip46 モックと fake timers を用いたテストを追加し、接続タイムアウト、Auth Challenge 後のタイムアウト、Auth Challenge 後の正常完了、終了後に来る遅延 onauth の無視 を検証するテストを追加した。

Testing

  • npm test を実行し、プロジェクトのテストスイートは 17 files / 165 tests すべてが通過した(成功)。
  • npm run check を実行し、svelte-check0 errors and 0 warnings を報告した(成功)。
  • npm run lint を実行し、prettiereslint のチェックが通った(成功)。
  • 追加した単体テストでは次を確認しており、いずれも期待どおりに動作した:接続タイムアウト(Auth Challenge が発生しない)、Auth Challenge 後の認証タイムアウト(接続タイムアウトでは終わらない)、Auth Challenge 後に connect()/getPublicKey() が完了する場合の正常終了、および接続終了後に届く遅延 onauth の無視。

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 13, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
nostter dc0dbf9 Commit Preview URL

Branch Preview URL
Jul 13 2026, 06:44 PM

@SnowCait

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5ee6c7e99

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread web/src/lib/signer-strategy.ts Outdated
Comment on lines +67 to +69
authTimer = setTimeout(() => {
rejectOnTimeout(new Error('NIP-46 authentication timed out'));
}, nip46AuthTimeout);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cancel the stale signer branch on auth timeout

When this new auth timer rejects the Promise.race, the async signer.connect()/getPublicKey() branch below is not cancelled and can still resolve later. In the auth-timeout path, a user can retry or switch accounts and then complete the old auth challenge; that old branch can still assign the module-level nip46CachedPublicKey, leaving the current NIP-46 signer paired with a stale pubkey. Guard the later assignment with the settled/current-signer state or close/cancel the stale signer before rejecting.

Useful? React with 👍 / 👎.

@gomurin0428

Copy link
Copy Markdown

@codex review

Comment thread web/src/lib/signer-strategy.test.ts Outdated
() => 'resolved',
() => 'rejected'
),
Promise.resolve('pending')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「まだ pending であること」を確認しているつもりの
Promise.race([promise.then(...), Promise.resolve('pending')])
は、promise が既に reject 済みでも常に 'pending' を返します

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// before
await vi.advanceTimersByTimeAsync(nip46ConnectTimeout);
await expect(
    Promise.race([
        promise.then(() => 'resolved', () => 'rejected'),
        Promise.resolve('pending')
    ])
).resolves.toBe('pending');

// after
let settled = false;
promise.then(
    () => (settled = true),
    () => (settled = true)
);

await vi.advanceTimersByTimeAsync(nip46ConnectTimeout);
expect(settled).toBe(false); // 接続タイムアウトでは終わっていない

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5ee6c7e99

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread web/src/lib/signer-strategy.ts Outdated
Comment on lines +67 to +69
authTimer = setTimeout(() => {
rejectOnTimeout(new Error('NIP-46 authentication timed out'));
}, nip46AuthTimeout);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear auth timer once authentication succeeds

When an auth URL arrives this starts one timer that remains armed until both connect() and the follow-up getPublicKey() call finish. If the user completes the external auth near the 5-minute limit, connect() can resolve successfully but getPublicKey() is still racing an almost-expired auth timer, so the login can fail with NIP-46 authentication timed out even though authentication already succeeded. Clear or restart this timer when the authenticated request completes so the post-connect public-key request is not failed by the stale auth deadline.

Useful? React with 👍 / 👎.

@gomurin0428 gomurin0428 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

初回接続後のNIP-46 Auth Challenge処理に、後続の署名・暗号化操作を停止させる回帰があるため指摘します。

Comment thread web/src/lib/signer-strategy.ts Outdated
bunkerSigner = BunkerSigner.fromBunker(clientSeckey, bunkerPointer, {
onauth: (url) => {
authRequested = true;
if (settled) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 接続後の正当な Auth Challenge を無視しないでください

settled は初回の接続処理が正常終了した後も true のままですが、この onauth コールバックは接続処理専用ではなく、長寿命の BunkerSigner が行う後続の sign_event / encrypt など全リクエストで再利用されます。nostr-tools@2.23.5 は各 sendRequest() で Auth Challenge を受け付け、NIP-46 も sign_event での challenge を明示しています。そのため、接続後にユーザー承認が必要な署名要求が来ると URL を開かずに return し、ユーザーが承認できないまま最終応答を無期限に待ちます。成功した signer ではこのコールバックを有効なままにし、失敗・破棄された接続試行だけを無効化してください。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants