-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
test(e2e): test harness + signal/prekey fixes uncovered along the way #2432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 11 commits
d604fc1
d8e252b
5b30374
a7c54d0
cefb992
43cd9d3
bda8e52
7b5982f
a9c0840
ce2cefc
97077df
86eaeec
a49aa82
33b87d2
df59a71
978ca8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| name: E2E Tests | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - master | ||
| pull_request_target: | ||
| branches: | ||
| - master | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| e2e: | ||
| name: E2E Tests | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| services: | ||
| mock-server: | ||
| image: ghcr.io/whiskeysockets-devtools/bartender:latest | ||
| credentials: | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.BARTENDER_GHCR_TOKEN }} | ||
| ports: | ||
| - 8080:8080 | ||
| env: | ||
| CHATSTATE_TTL_SECS: "3" | ||
| ADV_SECRET_KEY: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" | ||
| options: --log-driver none | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P0: Prompt for AI agents |
||
|
|
||
| - name: Setup Node.js and Corepack | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20.x | ||
|
|
||
| - name: Enable Corepack and Set Yarn Version | ||
| run: | | ||
| corepack enable | ||
| corepack prepare yarn@4.x --activate | ||
| - name: Restore Yarn Cache | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: .yarn/cache | ||
| key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-yarn- | ||
| - name: Install dependencies | ||
| run: yarn install --immutable | ||
|
|
||
| - name: Wait for mock server | ||
| run: | | ||
| for i in $(seq 1 30); do | ||
| if curl -sk https://localhost:8080/ > /dev/null 2>&1; then | ||
| echo "Mock server is ready" | ||
| exit 0 | ||
| fi | ||
| sleep 1 | ||
| done | ||
| echo "Mock server failed to become ready" | ||
| exit 1 | ||
| - name: Run E2E tests | ||
| env: | ||
| SOCKET_URL: "wss://127.0.0.1:8080/ws/chat" | ||
| ADV_SECRET_KEY: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=" | ||
| run: yarn test:e2e | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,7 +213,7 @@ | |
| return | ||
| } | ||
|
|
||
| let data: any | ||
| try { | ||
| data = JSON.parse(mexNode.content.toString()) | ||
| } catch (error) { | ||
|
|
@@ -309,7 +309,7 @@ | |
| case 'update': | ||
| const settingsNode = getBinaryNodeChild(child, 'settings') | ||
| if (settingsNode) { | ||
| const update: Record<string, any> = {} | ||
| const nameNode = getBinaryNodeChild(settingsNode, 'name') | ||
| if (nameNode?.content) update.name = nameNode.content.toString() | ||
|
|
||
|
|
@@ -534,6 +534,9 @@ | |
| }, authState?.creds?.me?.id || 'sendRetryRequest') | ||
| } | ||
|
|
||
| // Mirrors WAWeb/Handle/PreKeyLow.js: skip a re-issued notification with the same stanza id. | ||
| const inFlightPreKeyLow = new Set<string>() | ||
|
|
||
| /** | ||
| * Fire-and-forget tctoken re-issuance after a peer's device identity changed. | ||
| * Mirrors WAWebSendTcTokenWhenDeviceIdentityChange — runs in parallel with | ||
|
|
@@ -574,13 +577,23 @@ | |
| const handleEncryptNotification = async (node: BinaryNode) => { | ||
| const from = node.attrs.from | ||
| if (from === S_WHATSAPP_NET) { | ||
| const stanzaId = node.attrs.id | ||
| if (stanzaId && inFlightPreKeyLow.has(stanzaId)) { | ||
| return | ||
| } | ||
|
|
||
| const countChild = getBinaryNodeChild(node, 'count') | ||
| const count = +countChild!.attrs.value! | ||
| const shouldUploadMorePreKeys = count < MIN_PREKEY_COUNT | ||
|
|
||
| logger.debug({ count, shouldUploadMorePreKeys }, 'recv pre-key count') | ||
| if (shouldUploadMorePreKeys) { | ||
| await uploadPreKeys() | ||
| if (stanzaId) inFlightPreKeyLow.add(stanzaId) | ||
| try { | ||
| await uploadPreKeys() | ||
| } finally { | ||
| if (stanzaId) inFlightPreKeyLow.delete(stanzaId) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: The stanza-id dedupe only works while upload is in-flight; successful handling removes the id, so later re-issued notifications with the same id will upload prekeys again. (Based on your team's feedback about requiring strict WhatsApp protocol parity for retry/ack flows.) Prompt for AI agents |
||
| } | ||
|
Comment on lines
+537
to
+596
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This dedupe set never catches queued duplicate
Possible fix- // Mirrors WAWeb/Handle/PreKeyLow.js: skip a re-issued notification with the same stanza id.
- const inFlightPreKeyLow = new Set<string>()
+ // Keep recently handled stanza ids long enough to suppress queued re-issues too.
+ const recentPreKeyLow = new NodeCache<boolean>({ stdTTL: 30, useClones: false })
const handleEncryptNotification = async (node: BinaryNode) => {
const from = node.attrs.from
if (from === S_WHATSAPP_NET) {
const stanzaId = node.attrs.id
- if (stanzaId && inFlightPreKeyLow.has(stanzaId)) {
+ if (stanzaId && (await recentPreKeyLow.get(stanzaId))) {
return
}
const countChild = getBinaryNodeChild(node, 'count')
const count = +countChild!.attrs.value!
const shouldUploadMorePreKeys = count < MIN_PREKEY_COUNT
logger.debug({ count, shouldUploadMorePreKeys }, 'recv pre-key count')
if (shouldUploadMorePreKeys) {
- if (stanzaId) inFlightPreKeyLow.add(stanzaId)
- try {
- await uploadPreKeys()
- } finally {
- if (stanzaId) inFlightPreKeyLow.delete(stanzaId)
- }
+ if (stanzaId) await recentPreKeyLow.set(stanzaId, true)
+ await uploadPreKeys()
}
} else {🤖 Prompt for AI Agents |
||
| } | ||
| } else { | ||
| const result = await handleIdentityChange(node, { | ||
|
|
@@ -929,7 +942,7 @@ | |
| const jids = await readTcTokenIndex(authState.keys) | ||
| for (const jid of jids) tcTokenKnownJids.add(jid) | ||
| logger.debug({ count: tcTokenKnownJids.size }, 'loaded tctoken index') | ||
| } catch (err: any) { | ||
| logger.warn({ err: err?.message }, 'failed to load tctoken index') | ||
| } | ||
| })() | ||
|
|
@@ -1365,47 +1378,23 @@ | |
| } | ||
| } | ||
|
|
||
| const errorMessage = msg?.messageStubParameters?.[0] || '' | ||
| const isPreKeyError = errorMessage.includes('PreKey') | ||
|
|
||
| logger.debug(`[handleMessage] Attempting retry request for failed decryption`) | ||
| logger.debug('[handleMessage] Attempting retry request for failed decryption') | ||
|
|
||
| // Handle both pre-key and normal retries in single mutex | ||
| // WAWeb only retry-receipts here; server emits PreKeyLow if prekeys run low. | ||
| await retryMutex.mutex(async () => { | ||
| try { | ||
| if (!ws.isOpen) { | ||
| logger.debug({ node }, 'Connection closed, skipping retry') | ||
| return | ||
| } | ||
|
|
||
| // Handle pre-key errors with upload and delay | ||
| if (isPreKeyError) { | ||
| logger.info({ error: errorMessage }, 'PreKey error detected, uploading and retrying') | ||
|
|
||
| try { | ||
| logger.debug('Uploading pre-keys for error recovery') | ||
| await uploadPreKeys(5) | ||
| logger.debug('Waiting for server to process new pre-keys') | ||
| await delay(1000) | ||
| } catch (uploadErr) { | ||
| logger.error({ uploadErr }, 'Pre-key upload failed, proceeding with retry anyway') | ||
| } | ||
| } | ||
|
|
||
| const encNode = getBinaryNodeChild(node, 'enc') | ||
| await sendRetryRequest(node, !encNode) | ||
| if (retryRequestDelayMs) { | ||
| await delay(retryRequestDelayMs) | ||
| } | ||
| } catch (err) { | ||
| logger.error({ err, isPreKeyError }, 'Failed to handle retry, attempting basic retry') | ||
| // Still attempt retry even if pre-key upload failed | ||
| try { | ||
| const encNode = getBinaryNodeChild(node, 'enc') | ||
| await sendRetryRequest(node, !encNode) | ||
| } catch (retryErr) { | ||
| logger.error({ retryErr }, 'Failed to send retry after error handling') | ||
| } | ||
| logger.error({ err }, 'Failed to send retry') | ||
| } | ||
|
|
||
| acked = true | ||
|
|
@@ -1490,14 +1479,14 @@ | |
| status | ||
| } | ||
|
|
||
| if (status === 'relaylatency') { | ||
| const latencyValue = infoChild.attrs.latency || infoChild.attrs['latency_ms'] || infoChild.attrs['latency-ms'] | ||
| const latencyMs = latencyValue ? Number(latencyValue) : undefined | ||
| if (Number.isFinite(latencyMs)) { | ||
| call.latencyMs = latencyMs | ||
| } | ||
| } | ||
| if (status === 'relaylatency') { | ||
| const latencyValue = infoChild.attrs.latency || infoChild.attrs['latency_ms'] || infoChild.attrs['latency-ms'] | ||
| const latencyMs = latencyValue ? Number(latencyValue) : undefined | ||
| if (Number.isFinite(latencyMs)) { | ||
| call.latencyMs = latencyMs | ||
| } | ||
| } | ||
|
|
||
| if (status === 'offer') { | ||
| call.isVideo = !!getBinaryNodeChild(infoChild, 'video') | ||
| call.isGroup = infoChild.attrs.type === 'group' || !!infoChild.attrs['group-jid'] | ||
|
|
@@ -1625,6 +1614,7 @@ | |
| ) | ||
| ignoreJid = !isNodeFromMe || isJidGroup(attrs.from) ? attrs.from : attrs.recipient | ||
| } | ||
|
|
||
| if (ignoreJid && ignoreJid !== S_WHATSAPP_NET && shouldIgnoreJid(ignoreJid)) { | ||
| await sendMessageAck(node, type === 'message' ? NACK_REASONS.UnhandledError : undefined) | ||
| return | ||
|
|
@@ -1786,7 +1776,7 @@ | |
| for (const jid of survivors) tcTokenKnownJids.add(jid) | ||
|
|
||
| logger.debug({ mutated, remaining: survivors.size }, 'pruned expired tctokens') | ||
| } catch (err: any) { | ||
| logger.warn({ err: err?.message }, 'failed to prune expired tctokens') | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: WhiskeySockets/Baileys
Length of output: 508
🏁 Script executed:
cat -n .github/workflows/e2e.yml | head -80Repository: WhiskeySockets/Baileys
Length of output: 2454
Critical: untrusted PR code runs with repository secrets exposed.
Line 7 triggers
pull_request_target, which executes untrusted fork code with base-repository secrets. Line 35 checks out the PR head SHA, and lines 56 and 74 execute package managers and tests from that code. This allows malicious PRs to exfiltratesecrets.BARTENDER_GHCR_TOKEN(line 24) through install scripts, build hooks, or test code.Replace
pull_request_targetwithpull_request, or gate the job to same-repository PRs only (filter viagithub.event.pull_request.head.repo.full_name). Do not expose secrets to untrusted code paths.🤖 Prompt for AI Agents