Signal fixes

apps/bridge-worker/crontab

@@ -1,2 +1,2 @@
-*/1 * * * * signal:fetch-signal-messages ?max=1&id=fetchSignalMessagesCron {"scheduleTasks": "true"}
+*/1 * * * * fetch-signal-messages ?max=1&id=fetchSignalMessagesCron {"scheduleTasks": "true"}
-*/2 * * * * signal:check-group-membership ?max=1&id=checkGroupMembershipCron {}
+*/2 * * * * check-group-membership ?max=1&id=checkGroupMembershipCron {}

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";

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,
+              channelId: zammadChannel.id,
-              phoneNumber: signalChannel.phone_number,
+              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",
     );

packages/bridge-common/lib/database.ts

@@ -1,19 +1,12 @@
 import { PostgresDialect, CamelCasePlugin } from "kysely";
-import type {
+import type { GeneratedAlways, Generated, ColumnType, Selectable } from "kysely";
-  GeneratedAlways,
-  Generated,
-  ColumnType,
-  Selectable,
-} from "kysely";
 import pg from "pg";
 import { KyselyAuth } from "@auth/kysely-adapter";
 const { Pool, types } = pg;
 
 type Timestamp = ColumnType<Date, Date | string>;
 
-types.setTypeParser(types.builtins.TIMESTAMPTZ, (val) =>
+types.setTypeParser(types.builtins.TIMESTAMPTZ, (val) => new Date(val).toISOString());
-  new Date(val).toISOString(),
-);
 
 type GraphileJob = {
   taskIdentifier: string;
@@ -153,13 +146,23 @@ function getDb(): KyselyAuth<Database> {
   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) {
+  if (
-    throw new Error('Missing required database environment variables: DATABASE_HOST, DATABASE_NAME, DATABASE_PORT, DATABASE_USER, DATABASE_PASSWORD');
+    !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<Database>({
@@ -185,7 +188,7 @@ export const db = new Proxy({} as KyselyAuth<Database>, {
     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);
     }
 

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
 

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
+        # Use PostgreSQL JSONB queries to efficiently search preferences without loading all tickets into memory
-        # since users may have duplicate phone numbers
+        # This prevents DoS attacks from memory exhaustion
-        all_tickets = Ticket.where.not(state_id: state_ids)
+        ticket = Ticket.where.not(state_id: state_ids)
-                           .order(updated_at: :desc)
+                      .where("preferences->>'channel_id' = ?", channel.id.to_s)
-
+                      .where("preferences->'cdr_signal'->>'chat_id' = ?", receiver_phone_number)
-        Rails.logger.info "Found #{all_tickets.count} active tickets (searching all customers for group match)"
+                      .order(updated_at: :desc)
-
+                      .first
-        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
 
         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|
+    ticket = Ticket.where.not(state_id: state_ids)
-      t.preferences.is_a?(Hash) &&
+                  .where("preferences->'cdr_signal'->>'chat_id' = ?", params[:group_id])
-        t.preferences['cdr_signal'].is_a?(Hash) &&
+                  .order(updated_at: :desc)
-        t.preferences['cdr_signal']['chat_id'] == params[:group_id]
+                  .first
-    end
 
     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
+    # Idempotency check: if already marked as joined, skip update and return success
-    original_recipient = ticket.preferences.dig('cdr_signal', 'original_recipient')
+    # This prevents unnecessary database writes when the cron job sends duplicate notifications
-    member_phone = params[:member_phone]
+    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