Skip to content

fix(feishu): fail-closed approval card operator authorization - #70420

Open
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/feishu-approval-card-authorization-fail-closed
Open

necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/feishu-approval-card-authorization-fail-closed

Conversation

@necoweb3

Copy link
Copy Markdown
Contributor

Summary

Enforces fail-closed authorization in plugins/platforms/feishu/adapter.py::_is_interactive_operator_authorized(). When no explicit allowlist (self._admins or self._allowed_group_users) is configured, interactive card approval clicks now fail closed by default unless FEISHU_ALLOW_ALL_USERS=true or GATEWAY_ALLOW_ALL_USERS=true is set.

Why

When an agent turn encounters a dangerous command, the Feishu adapter emits an interactive approval card with buttons ("Approve Once", "Approve for Session", "Approve Always", "Deny").

_handle_approval_card_action() checks _is_interactive_operator_authorized() to authorize button clicks. When no explicit admin or group allowlist was configured, allowed_ids evaluated to set(). The implementation previously contained if not allowed_ids: return True, returning True (fail-open) for any clicker.

As a result, unlisted users in Feishu channels or DMs could click approval card buttons and unblock dangerous command execution on the host machine without admin authorization.

This brings Feishu into exact parity with the fail-closed approval fixes in Telegram (PR #28494), Teams (PR #27290), and Matrix (PR #34567 / #33328 / #30062).

Key Changes

  • Feishu Operator Authorization: Updated _is_interactive_operator_authorized() in plugins/platforms/feishu/adapter.py to require FEISHU_ALLOW_ALL_USERS or GATEWAY_ALLOW_ALL_USERS when allowed_ids is empty, failing closed by default.
  • Tests: Added tests/gateway/test_feishu_approval_card_fail_closed.py with 4 test cases verifying authorized, unauthorized, empty-allowlist fail-closed, and env-opt-in behaviors.

Test

python -m pytest tests/gateway/test_feishu_approval_card_fail_closed.py -q --timeout-method=thread

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/plugins Plugin system and bundled plugins platform/feishu Feishu / Lark adapter area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists needs-repro Bug needs reproduction steps labels Jul 24, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related competing work: #52535 makes empty allowlists approve interactive card actions, while this patch makes them fail closed unless explicitly opted in. Maintainer policy decision needed.

@egilewski

Copy link
Copy Markdown
Contributor

obsolete

The issue this PR closes appears to be resolved already. Please reopen with a fresh target if this still covers a distinct gap.

Signed: GPT-5.6-terra-low in Codex

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The focused approval-button suite is still red: TestResolveApproval::test_resolves_once creates empty admin/group-user sets and still expects resolve_gateway_approval to run. Update the fixture to configure an authorized operator or assert the new fail-closed result, then add a callback-level test through _handle_approval_card_action and _resolve_approval. Do not return the green ✅ Approved once card until authorization succeeds: in an open group with no allowlist, execution is blocked but the operator is told approval succeeded. Also preserve configured per-chat group_rules allowlist authorization.

Security evidence:

  • trust boundary: a Feishu card callback carries an untrusted operator identity and approval action.
  • source/sink/invariant: event.operator.open_id reaches resolve_gateway_approval; empty allowlists must deny before that sink, while explicitly authorized operators must still resolve approvals.
  • current-main reproduction: with empty allowlists and no opt-in, the callback reaches resolve_gateway_approval and removes the pending approval.
  • PR-head or patch-replay validation: the same empty-allowlist callback avoids the sink, but the focused suite exposes the stale test_resolves_once expectation and the callback returns a false green approval card.
  • positive/negative cases: explicit Feishu/global opt-ins, an admin, and * authorize; an unlisted operator is denied.
  • residual bypass search: an operator allowed only by a per-chat group_rules allowlist passes the callback policy gate but is rejected by the new operator check; cover and reconcile this path.
  • reviewer validation: update the failing regression test and gate the success card on the effective authorization result before merging.

Signed: GPT-5.6-sol-xhigh in Codex

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

Labels

area/auth Authentication, OAuth, credential pools comp/plugins Plugin system and bundled plugins needs-decision Awaiting maintainer decision before any implementation needs-repro Bug needs reproduction steps P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants