fix: preserve sender-level groupAllowFrom check when group is allowed

Wren-Fincher · Wren-Fincher · commit b98fd1256f68 · 2026-02-27T16:59:16.000-05:00
When a group is explicitly allowed via groups config, still enforce
sender-level gating if groupAllowFrom is configured. This prevents
an authorization regression where previously blocked senders would
be accepted in allowed groups.

Addresses review feedback from code review bot.
diff --git a/src/signal/monitor/event-handler.ts b/src/signal/monitor/event-handler.ts
@@ -523,7 +523,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
         hasGroupAllowFrom: deps.groupAllowFrom.length > 0,
       });
       if (!channelGroupPolicy.allowed) {
-        // Fall back to sender-level check only if no explicit groups config matched
+        // Group not explicitly allowed — check sender-level groupAllowFrom
         const groupAccess = resolveAccessDecision(true);
         if (groupAccess.decision !== "allow") {
           if (groupAccess.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED) {
@@ -537,6 +537,15 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
           }
           return;
         }
+      } else if (deps.groupAllowFrom.length > 0) {
+        // Group is allowed, but still enforce sender-level gating if groupAllowFrom is configured
+        const groupAccess = resolveAccessDecision(true);
+        if (groupAccess.decision !== "allow") {
+          logVerbose(
+            `Blocked signal group sender ${senderDisplay} (group allowed, sender not in groupAllowFrom)`,
+          );
+          return;
+        }
       }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -523,7 +523,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {`
`523`	`523`	`hasGroupAllowFrom: deps.groupAllowFrom.length > 0,`
`524`	`524`	`});`
`525`	`525`	`if (!channelGroupPolicy.allowed) {`
`526`		`- // Fall back to sender-level check only if no explicit groups config matched`
	`526`	`+ // Group not explicitly allowed — check sender-level groupAllowFrom`
`527`	`527`	`const groupAccess = resolveAccessDecision(true);`
`528`	`528`	`if (groupAccess.decision !== "allow") {`
`529`	`529`	`if (groupAccess.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED) {`
`@@ -537,6 +537,15 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {`
`537`	`537`	`}`
`538`	`538`	`return;`
`539`	`539`	`}`
	`540`	`+ } else if (deps.groupAllowFrom.length > 0) {`
	`541`	`+ // Group is allowed, but still enforce sender-level gating if groupAllowFrom is configured`
	`542`	`+ const groupAccess = resolveAccessDecision(true);`
	`543`	`+ if (groupAccess.decision !== "allow") {`
	`544`	`+ logVerbose(`
	`545`	+ `Blocked signal group sender ${senderDisplay} (group allowed, sender not in groupAllowFrom)`,
	`546`	`+ );`
	`547`	`+ return;`
	`548`	`+ }`
`540`	`549`	`}`
`541`	`550`	`}`
`542`	`551`