Skip to content

An allowlist makes every OnyxBot answer invisible: send_as_ephemeral is never readΒ #14460

Description

@wkoutre

What happens

Setting respond_member_group_list to a real, resolvable email makes every OnyxBot answer invisible. The bot adds its πŸ‘€ reaction, removes it, and posts nothing β€” in a DM and in a private channel alike. Remove the key entirely and it answers normally. Single variable.

Observed on v4.4.8; the same code shape is on main today (adf2ef0f1).

Why

respond_in_thread_or_channel in backend/onyx/onyxbot/slack/utils.py accepts send_as_ephemeral and never reads it β€” the # noqa: ARG001 marks it unused:

def respond_in_thread_or_channel(
    ...
    receiver_ids: list[str] | None = None,
    send_as_ephemeral: bool | None = True,  # noqa: ARG001
) -> list[str]:
    ...
    if not receiver_ids:
        response = client.chat_postMessage(...)
    else:
        for receiver in receiver_ids:
            response = client.chat_postEphemeral(...)

Delivery is chosen solely by whether receiver_ids is non-empty, so a caller that explicitly asks for a public post is silently overridden whenever it also passes receivers.

That makes the bot-DM carve-out in handle_regular_answer.py dead code. It computes the flag correctly and comments the intent:

# Capture whether response mode for channel is ephemeral. Even if the channel is set
# to respond with an ephemeral message, we still send as non-ephemeral if
# the message is a dm with the Onyx bot.
send_as_ephemeral = (
    slack_channel_config.channel_config.get("is_ephemeral", False)
    or message_info.is_slash_command
) and not message_info.is_bot_dm

then falls through to the allowlist anyway:

target_receiver_ids = (
    [message_info.sender_id]
    if message_info.sender_id and send_as_ephemeral
    else receiver_ids          # <- the channel allowlist
)

handle_message.py supplies that list as send_to: list[str] | None = allowed_user_ids. So with an allowlist configured and send_as_ephemeral False, the answer is delivered ephemerally.

And it goes to the wrong people. target_receiver_ids is the allowlist, not the sender, so the ephemeral is addressed to the allowlisted users while the person who actually asked sees nothing. That holds in a channel as well as a DM.

The two handle_buttons.py call sites show the parameter's intended meaning: send_as_ephemeral=False with receiver_ids=None is commented "Post in thread as non-ephemeral message", and send_as_ephemeral=True with receiver_ids=[sender] is the ephemeral case. The flag was meant to select delivery.

Reproducing

  1. Set a channel config's members field to one real email that resolves to a Slack user.
  2. Ask the bot something in that channel, or DM it.

The reaction appears and is removed; no answer is posted.

The design question behind it

handle_message.py comments that the allowlist is "the ephemeral response-visibility scope", so scoping visibility is intended. But is_ephemeral is already the dedicated per-channel setting for that, and overloading respond_member_group_list to mean both "who may invoke" and "who may see" is what produced this bug.

My proposed fix (PR to follow) makes the flag load-bearing: ephemeral delivery requires both a target audience and the caller's consent. An allowlist then gates invocation while is_ephemeral controls visibility. If you would rather keep allowlist-driven visibility scoping, the fix should instead address only the DM carve-out and send to the sender rather than the allowlist β€” happy to go that way instead.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions