From 3cde635f1e11db69dfb5821ed019b37c03faf1ea Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:12:10 -0700 Subject: [PATCH] fix(connectors): derive the connect-code shape beside the minter (TASK-159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route carried its own `CONNECT_CODE_SHAPE = /^[0-9a-f]{32}$/`, tied to `mintConnectCode` by a comment only. Widen the minter, update the seven pins on minted output, and every real code is refused with the shape message while the suite stays green — no fixture is a minted code. - `CONNECT_CODE_BYTES = 16` is now the one number: the minter draws from it and the predicate is built from it (`{CONNECT_CODE_BYTES * 2}`), so neither can be changed without the other. - `isConnectCodeShape` is exported beside `mintConnectCode`; the route calls it and its local regex (and the comment that tied them) is gone. - Witnesses: the predicate accepts what the minter produces and refuses one character either side, refusing non-hex and uppercase (the caller lowercases first); the route asserts it ASKED the service for the shape, so re-adding a local copy reddens instead of drifting; the existing route-tier `it.each` of four malformed inputs stays the negative side. --- .../telegram.webhook.connectCode.test.js | 22 +++++++++++++-- .../unit/services/telegramConnectCode.test.js | 27 +++++++++++++++++-- backend/routes/webhooks/telegram.ts | 10 +++---- backend/services/telegramConnectCode.ts | 18 ++++++++++++- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js b/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js index ce746addc..590daa2fa 100644 --- a/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js +++ b/backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js @@ -25,7 +25,14 @@ jest.mock('../../../models/WebhookDelivery', () => ({ // so count the calls while keeping the real budget implementation behind them. jest.mock('../../../services/telegramConnectCode', () => { const actual = jest.requireActual('../../../services/telegramConnectCode'); - return { ...actual, registerEnableAttempt: jest.fn(actual.registerEnableAttempt) }; + return { + ...actual, + registerEnableAttempt: jest.fn(actual.registerEnableAttempt), + // TASK-159: the route must ask this module for the shape. Wrapping the real + // implementation keeps the behaviour while recording the call, so a local + // regex reappearing in the route reddens instead of drifting silently. + isConnectCodeShape: jest.fn(actual.isConnectCodeShape), + }; }); const Integration = require('../../../models/Integration'); @@ -33,7 +40,7 @@ const Pod = require('../../../models/Pod'); const WebhookDelivery = require('../../../models/WebhookDelivery'); const telegramService = require('../../../services/telegramService'); const { - registerEnableAttempt, resetEnableAttempts, ENABLE_ATTEMPT_LIMIT, + registerEnableAttempt, resetEnableAttempts, ENABLE_ATTEMPT_LIMIT, isConnectCodeShape, } = require('../../../services/telegramConnectCode'); const telegramRoutes = require('../../../routes/webhooks/telegram'); @@ -182,6 +189,17 @@ describe('/commonly-enable hardening', () => { expect(telegramService.sendMessage.mock.calls.at(-1)[2]).toMatch(/invalid or expired/i); }); + // TASK-159. Everything above pins the VALUES the shape gate produces; this + // pins WHERE the answer comes from. Without it, re-adding a local + // `CONNECT_CODE_SHAPE` to the route keeps every arm green while the two + // definitions drift apart — the failure this row exists to close. + it('asks the service for the shape instead of matching its own copy', async () => { + Integration.findOne = jest.fn().mockResolvedValue(null); + await enable(SPACED_CODE.toUpperCase()); + // Normalised before the predicate: lowercased, groups joined (vera 74615). + expect(isConnectCodeShape).toHaveBeenCalledWith(JOINED_CODE); + }); + it.each(['1964', 'abc123', `${JOINED_CODE}a`, JOINED_CODE.slice(0, 31)])( 'spends no attempt on the malformed code %s', async (input) => { diff --git a/backend/__tests__/unit/services/telegramConnectCode.test.js b/backend/__tests__/unit/services/telegramConnectCode.test.js index cf7b2d9ab..bfc840b63 100644 --- a/backend/__tests__/unit/services/telegramConnectCode.test.js +++ b/backend/__tests__/unit/services/telegramConnectCode.test.js @@ -1,8 +1,9 @@ // Connect-code lifecycle: 128-bit, 10-minute TTL, legacy codes (no expiry) // are dead, and /commonly-enable attempts are rate-limited per chat. const { - mintConnectCode, isConnectCodeExpired, registerEnableAttempt, resetEnableAttempts, - CONNECT_CODE_TTL_MS, ENABLE_ATTEMPT_LIMIT, ENABLE_ATTEMPT_WINDOW_MS, ENABLE_ATTEMPT_MAX_CHATS, + mintConnectCode, isConnectCodeShape, isConnectCodeExpired, registerEnableAttempt, resetEnableAttempts, + CONNECT_CODE_TTL_MS, CONNECT_CODE_BYTES, ENABLE_ATTEMPT_LIMIT, ENABLE_ATTEMPT_WINDOW_MS, + ENABLE_ATTEMPT_MAX_CHATS, } = require('../../../services/telegramConnectCode'); describe('telegramConnectCode', () => { @@ -16,6 +17,28 @@ describe('telegramConnectCode', () => { expect(mintConnectCode().connectCode).not.toBe(connectCode); }); + // TASK-159. The shape is derived from the minter's byte count, so these arms + // are written against what the minter PRODUCES rather than against a literal + // width: widening the code cannot leave the predicate refusing real codes. + describe('isConnectCodeShape', () => { + it('accepts what the minter produces and refuses one character either side', () => { + const { connectCode } = mintConnectCode(); + expect(isConnectCodeShape(connectCode)).toBe(true); + expect(isConnectCodeShape(`${connectCode}a`)).toBe(false); + expect(isConnectCodeShape(connectCode.slice(0, -1))).toBe(false); + }); + + it('refuses non-hex and uppercase, because the caller lowercases first', () => { + const { connectCode } = mintConnectCode(); + expect(isConnectCodeShape(connectCode.replace(/[0-9a-f]/, 'z'))).toBe(false); + expect(isConnectCodeShape(connectCode.toUpperCase())).toBe(false); + }); + + it('draws its width from the same constant as the minter', () => { + expect(mintConnectCode().connectCode).toHaveLength(CONNECT_CODE_BYTES * 2); + }); + }); + it('treats a code with no expiry (legacy 24-bit) as expired', () => { expect(isConnectCodeExpired({ connectCode: 'abc123' })).toBe(true); expect(isConnectCodeExpired(undefined)).toBe(true); diff --git a/backend/routes/webhooks/telegram.ts b/backend/routes/webhooks/telegram.ts index b84996c75..e4d06a54c 100644 --- a/backend/routes/webhooks/telegram.ts +++ b/backend/routes/webhooks/telegram.ts @@ -10,7 +10,9 @@ const AgentEventService = require('../../services/agentEventService'); const telegramService = require('../../services/telegramService'); const { escapeHtml } = telegramService; const deliveryFailures = require('../../services/connectorDeliveryFailureService'); -const { isConnectCodeExpired, registerEnableAttempt } = require('../../services/telegramConnectCode'); +const { + isConnectCodeShape, isConnectCodeExpired, registerEnableAttempt, +} = require('../../services/telegramConnectCode'); const { claimDelivery: claimWebhookDelivery, releaseDelivery: releaseWebhookDelivery, @@ -34,10 +36,6 @@ const ENABLE_COMMAND = '/commonly-enable'; // Underscore alias: Telegram's registered-command menu forbids hyphens, so // the menu carries /commonly_enable while typed /commonly-enable keeps working. const ENABLE_COMMAND_ALIAS = '/commonly_enable'; -// Every minted code is exactly this shape (mintConnectCode is the only mint -// path: `crypto.randomBytes(16).toString('hex')`), so the enable handler can -// tell a typo from a guess without touching the database. -const CONNECT_CODE_SHAPE = /^[0-9a-f]{32}$/; const SUMMARY_COMMAND = '/summary'; const POD_SUMMARY_COMMAND = '/pod_summary'; const TLDR_COMMAND = '/tldr'; @@ -107,7 +105,7 @@ const handleEnableCommand = async (chat: any, code: any) => { // Malformed input is left to the route's outer rate limiter // (telegramWebhookRateLimit), because input that costs a regex is not worth a // per-chat counter. - if (!CONNECT_CODE_SHAPE.test(code)) { + if (!isConnectCodeShape(code)) { await telegramService.sendMessage( botToken, chatId, diff --git a/backend/services/telegramConnectCode.ts b/backend/services/telegramConnectCode.ts index 84148d6d8..5f3d8fde8 100644 --- a/backend/services/telegramConnectCode.ts +++ b/backend/services/telegramConnectCode.ts @@ -10,14 +10,28 @@ import crypto from 'crypto'; export const CONNECT_CODE_TTL_MS = 10 * 60 * 1000; +// The one number the minted code and its shape both come from. Changing the +// entropy without the shape (or the shape without the entropy) refuses every +// real code, so the two are derived from this rather than restated. +export const CONNECT_CODE_BYTES = 16; export const ENABLE_ATTEMPT_WINDOW_MS = 10 * 60 * 1000; export const ENABLE_ATTEMPT_LIMIT = 5; export const mintConnectCode = (now: number = Date.now()): { connectCode: string; connectCodeExpiresAt: Date } => ({ - connectCode: crypto.randomBytes(16).toString('hex'), + connectCode: crypto.randomBytes(CONNECT_CODE_BYTES).toString('hex'), connectCodeExpiresAt: new Date(now + CONNECT_CODE_TTL_MS), }); +// What a minted code looks like, derived from the minter's own constant rather +// than written out: `mintConnectCode` is the only mint path, and the enable +// webhook cannot authenticate the redeemer, so telling a typo from a guess has +// to happen without touching the database. The route must consult THIS — a +// second regex in the route is a second answer to the same question, and the +// failure it invites is silent (widen the minter, update the pins, every real +// code is refused). +const CONNECT_CODE_SHAPE = new RegExp(`^[0-9a-f]{${CONNECT_CODE_BYTES * 2}}$`); +export const isConnectCodeShape = (code: string): boolean => CONNECT_CODE_SHAPE.test(code); + // A code without an expiry predates the TTL — treat it as expired so legacy // 24-bit codes can never be redeemed; the owner re-mints from the UI. export const isConnectCodeExpired = ( @@ -64,10 +78,12 @@ export const resetEnableAttempts = (): void => { attempts.clear(); }; module.exports = { CONNECT_CODE_TTL_MS, + CONNECT_CODE_BYTES, ENABLE_ATTEMPT_WINDOW_MS, ENABLE_ATTEMPT_LIMIT, ENABLE_ATTEMPT_MAX_CHATS, mintConnectCode, + isConnectCodeShape, isConnectCodeExpired, registerEnableAttempt, resetEnableAttempts,