isAllowedBot is duplicated byte-for-byte across checkHumanActor and checkWritePermissions

[rajarshidattapy]

Type: maintainability / drift risk in a security control
Severity: medium
Area: src/github/validation/actor.ts, src/github/validation/permissions.ts
Effort: trivial

Summary

The allowed_bots allow-list matcher exists twice, as two identical private
copies in two different files. Both are part of the action's authorization path.
Verified identical:

$ diff <(sed -n 8,26p src/github/validation/permissions.ts) \
       <(sed -n 11,29p src/github/validation/actor.ts)
# (no output)

Affected code

• src/github/validation/permissions.ts:8-25 - used by checkActorWritePermissions
  when the collaborator-permission API reports the actor "is not a user"
• src/github/validation/actor.ts:11-28 - used by checkHumanActor for
  non-User account types and for actors the Users API cannot resolve

function isAllowedBot(actor: string, allowedBots: string): boolean {
  const trimmed = allowedBots.trim();
  if (trimmed === "*") return true;
  if (!trimmed) return false;

  const allowedList = trimmed
    .split(",")
    .map((bot) => bot.trim().toLowerCase().replace(/\[bot\]$/, ""))
    .filter((bot) => bot.length > 0);

  const normalizedActor = actor.toLowerCase().replace(/\[bot\]$/, "");
  return allowedList.includes(normalizedActor);
}

Impact

Both call sites gate whether an automated actor may trigger Claude. A fix or
hardening applied to one copy - handling whitespace inside entries, supporting a
*[bot] wildcard the way actorMatchesPattern does, normalising a different
suffix - silently does not apply to the other. The two checks then disagree about
whether a given bot is allowed, which is the worst outcome for an authorization
predicate: one layer permits what the other denies, and which one wins depends on
which code path the event happens to take.

There is no test asserting the two stay in agreement.

Suggested fix

Move the function to the existing actor-filter utility and import it in both
validators:

// src/github/utils/actor-filter.ts
export function isAllowedBot(actor: string, allowedBots: string): boolean { ... }

src/github/utils/actor-filter.ts already owns the neighbouring concerns
(parseActorFilter, resolveActorName, actorMatchesPattern,
shouldIncludeCommentByActor) and already has coverage in
test/actor-filter.test.ts, so the shared copy inherits a test home.

Worth noting while consolidating: isAllowedBot does exact matching after
stripping a [bot] suffix, while actorMatchesPattern in the same area supports
a *[bot] wildcard. allowed_bots documents '*' for "all bots" so the
behaviours are defensible, but having them in one file makes the difference
visible instead of accidental.