diff --git a/apps/bridge-worker/crontab b/apps/bridge-worker/crontab index 9c8bb8c..bebd9c8 100644 --- a/apps/bridge-worker/crontab +++ b/apps/bridge-worker/crontab @@ -1,2 +1,2 @@ -*/1 * * * * signal:fetch-signal-messages ?max=1&id=fetchSignalMessagesCron {"scheduleTasks": "true"} -*/2 * * * * signal:check-group-membership ?max=1&id=checkGroupMembershipCron {} +*/1 * * * * fetch-signal-messages ?max=1&id=fetchSignalMessagesCron {"scheduleTasks": "true"} +*/2 * * * * check-group-membership ?max=1&id=checkGroupMembershipCron {} diff --git a/apps/bridge-worker/tasks/signal/check-group-membership.ts b/apps/bridge-worker/tasks/check-group-membership.ts similarity index 94% rename from apps/bridge-worker/tasks/signal/check-group-membership.ts rename to apps/bridge-worker/tasks/check-group-membership.ts index 2e41efb..1a6f8d6 100644 --- a/apps/bridge-worker/tasks/signal/check-group-membership.ts +++ b/apps/bridge-worker/tasks/check-group-membership.ts @@ -5,6 +5,10 @@ * This task queries the Signal CLI API to check if users have joined * their assigned groups. When a user joins (moves from pendingInvites to members), * it updates the ticket's group_joined flag in Zammad. + * + * Note: This task sends webhooks for all group members every time it runs. + * The Zammad webhook handler is idempotent and will ignore duplicate notifications + * if group_joined is already true. */ import { db, getWorkerUtils } from "@link-stack/bridge-common"; diff --git a/apps/bridge-worker/tasks/signal/fetch-signal-messages.ts b/apps/bridge-worker/tasks/fetch-signal-messages.ts similarity index 100% rename from apps/bridge-worker/tasks/signal/fetch-signal-messages.ts rename to apps/bridge-worker/tasks/fetch-signal-messages.ts diff --git a/apps/bridge-worker/tasks/formstack/create-ticket-from-form.ts b/apps/bridge-worker/tasks/formstack/create-ticket-from-form.ts index bc5ea72..7776a81 100644 --- a/apps/bridge-worker/tasks/formstack/create-ticket-from-form.ts +++ b/apps/bridge-worker/tasks/formstack/create-ticket-from-form.ts @@ -1,4 +1,5 @@ import { createLogger } from "@link-stack/logger"; +import { db } from "@link-stack/bridge-common"; import { Zammad, getUser } from "../../lib/zammad.js"; import { loadFieldMapping, @@ -233,23 +234,47 @@ const createTicketFromFormTask = async ( // Check if this is a Signal ticket let signalArticleType = null; - let signalChannel = null; + let signalChannelId = null; + let signalBotToken = null; if (signalAccount) { try { logger.info({ signalAccount }, "Looking up Signal channel and article type"); - // Look up Signal channels (admin-only endpoint) + // Look up Signal channels from Zammad (admin-only endpoint) + // Note: bot_token is NOT included in this response for security reasons const channels = await zammad.get("cdr_signal_channels"); if (channels.length > 0) { - signalChannel = channels[0]; // Use first active Signal channel + const zammadChannel = channels[0]; // Use first active Signal channel + signalChannelId = zammadChannel.id; + logger.info( { - channelId: signalChannel.id, - phoneNumber: signalChannel.phone_number, + channelId: zammadChannel.id, + phoneNumber: zammadChannel.phone_number, }, - "Found active Signal channel", + "Found active Signal channel from Zammad", ); + + // Look up the bot_token from our own cdr database using the phone number + const signalBot = await db + .selectFrom("SignalBot") + .selectAll() + .where("phoneNumber", "=", zammadChannel.phone_number) + .executeTakeFirst(); + + if (signalBot) { + signalBotToken = signalBot.token; + logger.info( + { botId: signalBot.id, phoneNumber: signalBot.phoneNumber }, + "Found Signal bot token from cdr database", + ); + } else { + logger.warn( + { phoneNumber: zammadChannel.phone_number }, + "Signal bot not found in cdr database", + ); + } } else { logger.warn("No active Signal channels found"); } @@ -306,18 +331,18 @@ const createTicketFromFormTask = async ( // Add Signal preferences if we have Signal channel and article type // Note: signalAccount from Formstack is the phone number the user typed in // Groups are added later via update_group webhook from bridge-worker - if (signalChannel && signalArticleType && signalAccount) { + if (signalChannelId && signalBotToken && signalArticleType && signalAccount) { ticketData.preferences = { - channel_id: signalChannel.id, + channel_id: signalChannelId, cdr_signal: { - bot_token: signalChannel.bot_token, + bot_token: signalBotToken, chat_id: signalAccount, // Use Signal phone number as chat_id }, }; logger.info( { - channelId: signalChannel.id, + channelId: signalChannelId, chatId: signalAccount, }, "Adding Signal preferences to ticket", @@ -339,7 +364,7 @@ const createTicketFromFormTask = async ( const ticket = await zammad.ticket.create(ticketData); // Set create_article_type_id for Signal tickets to enable proper replies - if (signalArticleType && signalChannel) { + if (signalArticleType && signalChannelId) { try { await zammad.ticket.update(ticket.id, { create_article_type_id: signalArticleType.id, @@ -367,7 +392,7 @@ const createTicketFromFormTask = async ( ticketId: ticket.id, ticketNumber: ticket.id, title, - isSignalTicket: !!signalChannel, + isSignalTicket: !!signalChannelId, }, "Successfully created ticket from Formstack submission", ); diff --git a/packages/bridge-common/lib/database.ts b/packages/bridge-common/lib/database.ts index cad2cfe..a726da0 100644 --- a/packages/bridge-common/lib/database.ts +++ b/packages/bridge-common/lib/database.ts @@ -1,19 +1,12 @@ import { PostgresDialect, CamelCasePlugin } from "kysely"; -import type { - GeneratedAlways, - Generated, - ColumnType, - Selectable, -} from "kysely"; +import type { GeneratedAlways, Generated, ColumnType, Selectable } from "kysely"; import pg from "pg"; import { KyselyAuth } from "@auth/kysely-adapter"; const { Pool, types } = pg; type Timestamp = ColumnType; -types.setTypeParser(types.builtins.TIMESTAMPTZ, (val) => - new Date(val).toISOString(), -); +types.setTypeParser(types.builtins.TIMESTAMPTZ, (val) => new Date(val).toISOString()); type GraphileJob = { taskIdentifier: string; @@ -153,13 +146,23 @@ function getDb(): KyselyAuth { const DATABASE_USER = process.env.DATABASE_USER; const DATABASE_PASSWORD = process.env.DATABASE_PASSWORD; - if (!DATABASE_HOST || !DATABASE_NAME || !DATABASE_PORT || !DATABASE_USER || !DATABASE_PASSWORD) { - throw new Error('Missing required database environment variables: DATABASE_HOST, DATABASE_NAME, DATABASE_PORT, DATABASE_USER, DATABASE_PASSWORD'); + if ( + !DATABASE_HOST || + !DATABASE_NAME || + !DATABASE_PORT || + !DATABASE_USER || + !DATABASE_PASSWORD + ) { + throw new Error( + "Missing required database environment variables: DATABASE_HOST, DATABASE_NAME, DATABASE_PORT, DATABASE_USER, DATABASE_PASSWORD", + ); } const port = parseInt(DATABASE_PORT, 10); if (isNaN(port) || port < 1 || port > 65535) { - throw new Error(`Invalid DATABASE_PORT: ${DATABASE_PORT}. Must be a number between 1 and 65535.`); + throw new Error( + `Invalid DATABASE_PORT: ${DATABASE_PORT}. Must be a number between 1 and 65535.`, + ); } _db = new KyselyAuth({ @@ -185,7 +188,7 @@ export const db = new Proxy({} as KyselyAuth, { const value = (instance as any)[prop]; // If it's a function, bind it to the actual instance to preserve 'this' context - if (typeof value === 'function') { + if (typeof value === "function") { return value.bind(instance); } diff --git a/packages/zammad-addon-bridge/src/app/controllers/cdr_signal_channels_controller.rb b/packages/zammad-addon-bridge/src/app/controllers/cdr_signal_channels_controller.rb index f478f44..9776d17 100644 --- a/packages/zammad-addon-bridge/src/app/controllers/cdr_signal_channels_controller.rb +++ b/packages/zammad-addon-bridge/src/app/controllers/cdr_signal_channels_controller.rb @@ -8,8 +8,8 @@ class CdrSignalChannelsController < ApplicationController { id: channel.id, phone_number: channel.options['phone_number'], - bot_token: channel.options['bot_token'], bot_endpoint: channel.options['bot_endpoint'] + # bot_token intentionally excluded - bridge-worker should look it up from cdr database } end diff --git a/packages/zammad-addon-bridge/src/app/controllers/channels_cdr_signal_controller.rb b/packages/zammad-addon-bridge/src/app/controllers/channels_cdr_signal_controller.rb index 812750c..3d75b2f 100644 --- a/packages/zammad-addon-bridge/src/app/controllers/channels_cdr_signal_controller.rb +++ b/packages/zammad-addon-bridge/src/app/controllers/channels_cdr_signal_controller.rb @@ -115,7 +115,11 @@ class ChannelsCdrSignalController < ApplicationController channel = channel_for_token(token) return render json: {}, status: 401 if !channel || !channel.active - return render json: {}, status: 401 if channel.options[:token] != token + # Use constant-time comparison to prevent timing attacks + return render json: {}, status: 401 unless ActiveSupport::SecurityUtils.secure_compare( + channel.options[:token].to_s, + token.to_s + ) # Handle group creation events if params[:event] == 'group_created' @@ -218,38 +222,13 @@ class ChannelsCdrSignalController < ApplicationController Rails.logger.info "Channel ID: #{channel.id}" begin - # For group messages, search all tickets regardless of customer - # since users may have duplicate phone numbers - all_tickets = Ticket.where.not(state_id: state_ids) - .order(updated_at: :desc) - - Rails.logger.info "Found #{all_tickets.count} active tickets (searching all customers for group match)" - - ticket = all_tickets.find do |t| - begin - has_preferences = t.preferences.is_a?(Hash) - has_cdr_signal = has_preferences && t.preferences['cdr_signal'].is_a?(Hash) - has_channel_id = has_preferences && t.preferences['channel_id'] == channel.id - - if has_cdr_signal && has_channel_id - stored_chat_id = t.preferences['cdr_signal']['chat_id'] - - Rails.logger.info " - stored_chat_id: #{stored_chat_id}" - Rails.logger.info " - incoming_group_id: #{receiver_phone_number}" - - matches = receiver_phone_number == stored_chat_id - Rails.logger.info " - MATCH: #{matches}" - - matches - else - Rails.logger.info "Ticket ##{t.number} has no cdr_signal preferences or wrong channel" - false - end - rescue => e - Rails.logger.error "Error checking ticket #{t.id}: #{e.message}" - false - end - end + # Use PostgreSQL JSONB queries to efficiently search preferences without loading all tickets into memory + # This prevents DoS attacks from memory exhaustion + ticket = Ticket.where.not(state_id: state_ids) + .where("preferences->>'channel_id' = ?", channel.id.to_s) + .where("preferences->'cdr_signal'->>'chat_id' = ?", receiver_phone_number) + .order(updated_at: :desc) + .first if ticket Rails.logger.info "=== FOUND MATCHING TICKET BY GROUP ID: ##{ticket.number} ===" @@ -441,14 +420,13 @@ class ChannelsCdrSignalController < ApplicationController end # Find ticket(s) with this group_id in preferences - # Search all active tickets for matching group + # Use PostgreSQL JSONB queries for efficient lookup (prevents DoS from loading all tickets) state_ids = Ticket::State.where(name: %w[closed merged removed]).pluck(:id) - ticket = Ticket.where.not(state_id: state_ids).find do |t| - t.preferences.is_a?(Hash) && - t.preferences['cdr_signal'].is_a?(Hash) && - t.preferences['cdr_signal']['chat_id'] == params[:group_id] - end + ticket = Ticket.where.not(state_id: state_ids) + .where("preferences->'cdr_signal'->>'chat_id' = ?", params[:group_id]) + .order(updated_at: :desc) + .first unless ticket Rails.logger.warn "Signal group member joined: Ticket not found for group_id #{params[:group_id]}" @@ -456,11 +434,22 @@ class ChannelsCdrSignalController < ApplicationController return end - # Check if the member who joined matches the original recipient - original_recipient = ticket.preferences.dig('cdr_signal', 'original_recipient') - member_phone = params[:member_phone] + # Idempotency check: if already marked as joined, skip update and return success + # This prevents unnecessary database writes when the cron job sends duplicate notifications + if ticket.preferences.dig('cdr_signal', 'group_joined') == true + Rails.logger.debug "Signal group member #{params[:member_phone]} already marked as joined for group #{params[:group_id]} ticket #{ticket.id}, skipping update" + render json: { + success: true, + ticket_id: ticket.id, + ticket_number: ticket.number, + group_joined: true, + already_joined: true + }, status: :ok + return + end # Update group_joined flag + member_phone = params[:member_phone] ticket.preferences[:cdr_signal][:group_joined] = true ticket.preferences[:cdr_signal][:group_joined_at] = params[:timestamp] if params[:timestamp].present? ticket.preferences[:cdr_signal][:group_joined_by] = member_phone