Flag per-group Telegram wildcard even with top-level allowlist

This guard suppresses the new critical finding whenever hasTopLevelRestriction is true, but Telegram runtime auth does not intersect top-level groupAllowFrom with per-group/topic overrides; it replaces it (groupAllowOverride ?? groupAllowFrom in extensions/telegram/src/bot-message-context.ts and extensions/telegram/src/bot/helpers.ts). In configurations like groupAllowFrom: ["123456"] plus groups.<id>.allowFrom: ["*"], group messages are still effectively wildcard-authorized, so the audit now returns a false negative for a real command-control exposure.

Useful? React with 👍 / 👎.

Early continue suppresses invalid-entry warnings

Moving the top-level wildcard block before invalidTelegramAllowFromEntries means that if a config has both a top-level * wildcard and other non-numeric entries (e.g., @username alongside *), those invalid entries will no longer be reported. Previously the invalidTelegramAllowFromEntries check ran before the continue, so both findings were emitted.

While the wildcard is the dominant concern, silently dropping the invalid-entry WARN could hide misconfigured allowlists that users still need to fix after removing the wildcard. Consider reporting invalid entries even in the wildcard path:

// report invalid entries first, then bail
if (invalidTelegramAllowFromEntries.size > 0) {
  // ... push warn finding ...
}
if (storeHasWildcard || groupAllowFromHasWildcard) {
  findings.push({ checkId: "channels.telegram.groups.allowFrom.wildcard", ... });
  continue;
}

This is a comment left during a code review.
Path: src/security/audit-channel.ts
Line: 816-827

Comment:
**Early `continue` suppresses invalid-entry warnings**

Moving the top-level wildcard block before `invalidTelegramAllowFromEntries` means that if a config has **both** a top-level `*` wildcard **and** other non-numeric entries (e.g., `@username` alongside `*`), those invalid entries will no longer be reported. Previously the `invalidTelegramAllowFromEntries` check ran before the `continue`, so both findings were emitted.

While the wildcard is the dominant concern, silently dropping the invalid-entry WARN could hide misconfigured allowlists that users still need to fix after removing the wildcard. Consider reporting invalid entries even in the wildcard path:

```typescript
// report invalid entries first, then bail
if (invalidTelegramAllowFromEntries.size > 0) {
  // ... push warn finding ...
}
if (storeHasWildcard || groupAllowFromHasWildcard) {
  findings.push({ checkId: "channels.telegram.groups.allowFrom.wildcard", ... });
  continue;
}
```

How can I resolve this? If you propose a fix, please make it concise.

* wildcard generates spurious invalid-entry WARN alongside CRITICAL

When a group-level wildcard is detected and the CRITICAL finding is pushed (lines 830–840), the code falls through to the invalidTelegramAllowFromEntries check at line 842. Because * is non-numeric, collectInvalidTelegramAllowFromEntries will have already collected it, so a redundant WARN finding (channels.telegram.allowFrom.invalid_entries) will also be emitted for every group that has allowFrom: ["*"].

This produces confusing output: a CRITICAL for the wildcard plus a WARN for * being non-numeric. Consider either:

1. Filtering * out of invalidTelegramAllowFromEntries before/during collection, or
2. Adding a continue after the CRITICAL push (while still reporting legitimate invalid entries via a pre-check, similar to the suggestion above).

This is a comment left during a code review.
Path: src/security/audit-channel.ts
Line: 829-840

Comment:
**`*` wildcard generates spurious invalid-entry WARN alongside CRITICAL**

When a group-level wildcard is detected and the CRITICAL finding is pushed (lines 830–840), the code falls through to the `invalidTelegramAllowFromEntries` check at line 842. Because `*` is non-numeric, `collectInvalidTelegramAllowFromEntries` will have already collected it, so a redundant WARN finding (`channels.telegram.allowFrom.invalid_entries`) will also be emitted for every group that has `allowFrom: ["*"]`.

This produces confusing output: a CRITICAL for the wildcard plus a WARN for `*` being non-numeric. Consider either:
1. Filtering `*` out of `invalidTelegramAllowFromEntries` before/during collection, or
2. Adding a `continue` after the CRITICAL push (while still reporting legitimate invalid entries via a pre-check, similar to the suggestion above).

How can I resolve this? If you propose a fix, please make it concise.

Verify storeAllowFrom acts as a global gate for group access

storeAllowFrom (the pairing store) is used here to mark a "top-level restriction" that suppresses the group-level wildcard CRITICAL. If a non-empty, non-wildcard storeAllowFrom does not actually filter Telegram group senders at runtime (e.g., it only gates DM-paired users), then this could produce false negatives — a group with allowFrom: ["*"] would be silently allowed despite an unrelated store entry.

The remediation text only mentions channels.telegram.groupAllowFrom as the global mechanism. It would be worth confirming that storeAllowFrom entries are also enforced before group-level allowFrom overrides in the Telegram runtime, and if not, removing storeAllowFrom.length > 0 && !storeHasWildcard from hasTopLevelRestriction.

This is a comment left during a code review.
Path: src/security/audit-channel.ts
Line: 812-814

Comment:
**Verify `storeAllowFrom` acts as a global gate for group access**

`storeAllowFrom` (the pairing store) is used here to mark a "top-level restriction" that suppresses the group-level wildcard CRITICAL. If a non-empty, non-wildcard `storeAllowFrom` does **not** actually filter Telegram group senders at runtime (e.g., it only gates DM-paired users), then this could produce false negatives — a group with `allowFrom: ["*"]` would be silently allowed despite an unrelated store entry.

The remediation text only mentions `channels.telegram.groupAllowFrom` as the global mechanism. It would be worth confirming that `storeAllowFrom` entries are also enforced before group-level `allowFrom` overrides in the Telegram runtime, and if not, removing `storeAllowFrom.length > 0 && !storeHasWildcard` from `hasTopLevelRestriction`.

How can I resolve this? If you propose a fix, please make it concise.

Do not suppress per-group wildcard findings with global allowlist

The new !hasTopLevelRestriction gate hides a real command-control exposure: Telegram runtime authorization replaces groupAllowFrom when a group/topic override exists (groupAllowOverride ?? groupAllowFrom in extensions/telegram/src/bot-message-context.ts and extensions/telegram/src/bot/helpers.ts), so groups.<id>.allowFrom: ["*"] still authorizes any member even if top-level groupAllowFrom is restricted. In configs that set a global allowlist plus per-group/topic wildcard, the audit now returns a false negative instead of the critical finding.

Useful? React with 👍 / 👎.

Flag per-group wildcard even when top-level list is set

This condition suppresses a real critical exposure: Telegram runtime authorization does not intersect top-level groupAllowFrom with per-group/topic allowFrom; it replaces it via groupAllowOverride ?? groupAllowFrom (see extensions/telegram/src/bot-message-context.ts and extensions/telegram/src/bot/helpers.ts). In configs like groupAllowFrom: ["123456"] plus groups.<id>.allowFrom: ["*"], group messages are still wildcard-authorized, so the audit now produces a false negative instead of reporting the command-control risk.

Useful? React with 👍 / 👎.