Skip to content

security: enforce approval ownership on every transport, not just Slack - #567

Open
coderdailyone wants to merge 1 commit into
andrewyng:mainfrom
coderdailyone:security/approval-owner-check-every-transport
Open

security: enforce approval ownership on every transport, not just Slack#567
coderdailyone wants to merge 1 commit into
andrewyng:mainfrom
coderdailyone:security/approval-owner-check-every-transport

Conversation

@coderdailyone

Copy link
Copy Markdown

Fixes #519.

The human-approval workflow validated the resolver only on the Slack lane: both _resolve_inbox_reply (channel replies) and _on_interaction (button clicks) gated protected items (approval/directory/plan) behind _slack_actor_owns_item only when platform == "slack". So:

  • a reply or button arriving over Telegram resolved protected items with no owner or channel-binding validation at all;
  • items mirrored to a Slack inbox were resolvable from Telegram, bypassing Slack's owner enforcement.

Change

A single _actor_owns_protected_item gate, used by both resolution paths for all transports:

  • cross-transport is refused — an item bound to one channel is only resolvable from that same channel; an in-app-only item is not remotely resolvable;
  • Slack defers to the existing owner + bound-channel check (_slack_actor_owns_item);
  • no other transport (e.g. Telegram) has an approval-owner model, so it cannot resolve protected items remotely — they stay pending for in-app resolution.

Questions and free-text answers are unchanged: still answerable by any allow-listed member on any transport.

Tests

New tests/test_telegram_approval_bypass.py:

  • a Telegram reply and a Telegram button each fail to resolve a protected approval (stays pending; the button is rejected via the gateway);
  • a Slack-bound approval is not resolvable from a Telegram reply (cross-transport);
  • a question is still answerable from Telegram (non-protected path preserved).

Verified the three bypass tests fail on main (Telegram resolves the protected item) and pass with the fix. Existing test_slack_approval_owners (all 10) and the inbox/routing/connector suites stay green (90 in the regression run).

🤖 Generated with Claude Code

https://claude.ai/code/session_01CLxsdFGXjdztTRNHjNgPXP

The human-approval workflow validated the resolver ONLY on the Slack
lane: both _resolve_inbox_reply and _on_interaction gated protected
items (approval/directory/plan) behind _slack_actor_owns_item only when
platform == "slack". A reply or button arriving over Telegram resolved
protected items with NO owner or channel-binding check at all, and items
mirrored to a Slack inbox were resolvable from Telegram — bypassing
Slack's owner enforcement entirely.

Fix: a single _actor_owns_protected_item gate used by both paths for all
transports:
  - cross-transport is refused (an item bound to one channel is only
    resolvable from that channel; an in-app-only item is not remotely
    resolvable);
  - Slack defers to the existing owner + bound-channel check;
  - no other transport has an approval-owner model, so it cannot resolve
    protected items remotely — they stay pending for in-app resolution.

Questions and free-text answers are unchanged: still answerable by any
allow-listed member on any transport.

Fixes andrewyng#519

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CLxsdFGXjdztTRNHjNgPXP
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant