Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 172 additions & 56 deletions crates/openab-core/src/gateway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,15 +745,15 @@ impl ChatAdapter for GatewayAdapter {
// --- Run the gateway adapter (connects to gateway WS, routes events to AdapterRouter) ---

/// Resolved gateway configuration passed to the adapter at startup.
/// Channel/user allowlists are NOT carried here anymore: L2/L3 enforcement for
/// the WebSocket path moved to the shared per-platform trust registry
/// (`AdapterRouter::gate_incoming`), seeded in main.rs with precedence
/// `GATEWAY_*` env < `[gateway]` section < `[<platform>]` section (#1356).
pub struct GatewayParams {
pub url: String,
pub platform: String,
pub token: Option<String>,
pub bot_username: Option<String>,
pub allow_all_channels: bool,
pub allowed_channels: Vec<String>,
pub allow_all_users: bool,
pub allowed_users: Vec<String>,
pub allow_bot_messages: bool,
pub trusted_bot_ids: Vec<String>,
pub streaming: bool,
Expand All @@ -774,10 +774,6 @@ pub async fn run_gateway_adapter(
// Append auth token as query param if configured
let gateway_url = params.url;
let bot_username = params.bot_username;
let allow_all_channels = params.allow_all_channels;
let allowed_channels: HashSet<String> = params.allowed_channels.into_iter().collect();
let allow_all_users = params.allow_all_users;
let allowed_users: HashSet<String> = params.allowed_users.into_iter().collect();
let allow_bot_messages = params.allow_bot_messages;
let trusted_bot_ids: HashSet<String> = params.trusted_bot_ids.into_iter().collect();
// Cosmetic streaming edits a placeholder in place. On platforms without an
Expand Down Expand Up @@ -850,12 +846,17 @@ pub async fn run_gateway_adapter(
let slash_ws_tx = ws_tx.clone(); // for fire-and-forget slash command responses
let mut tasks: tokio::task::JoinSet<()> = tokio::task::JoinSet::new();

// Hoist filter params outside loop — all fields are loop-invariant
// Hoist filter params outside loop — all fields are loop-invariant.
// Structural gating (bot filter + @mention) stays in should_skip_event.
// L2 (channel) + L3 (identity) are enforced by the shared ingress gate
// (`gate_gateway_event`) below — same registry as the unified path —
// so channel/user checks are neutered here by passing allow-all.
let no_ids: HashSet<String> = HashSet::new();
let filter = EventFilterParams {
allow_all_channels,
allowed_channels: &allowed_channels,
allow_all_users,
allowed_users: &allowed_users,
allow_all_channels: true,
allowed_channels: &no_ids,
allow_all_users: true,
allowed_users: &no_ids,
allow_bot_messages,
trusted_bot_ids: &trusted_bot_ids,
bot_username: bot_username.as_deref(),
Expand Down Expand Up @@ -884,6 +885,31 @@ pub async fn run_gateway_adapter(
continue;
}

// Shared ingress trust gate (L2 scope + L3
// identity) — same per-platform registry as
// the unified path. Placed before slash
// handling so untrusted senders cannot
// execute /reset|/cancel|/config. The echo
// is SPAWNED, never awaited here: streaming
// send_message waits for a GatewayResponse
// that only this loop can dispatch, so an
// inline await would stall all event
// processing for the reply timeout.
match gate_gateway_event(&router, &event) {
GateOutcome::Allow => {}
GateOutcome::Deny { echo } => {
if let Some((echo_channel, msg)) = echo {
let echo_adapter = adapter.clone();
tasks.spawn(async move {
let _ = echo_adapter
.send_message(&echo_channel, &msg)
.await;
});
}
continue;
}
}

info!(
platform = %event.platform,
sender = %event.sender.name,
Expand Down Expand Up @@ -1205,7 +1231,6 @@ pub async fn run_gateway_adapter(
} // outer reconnect loop
}


// --- Public API for unified mode (Phase 2) ---

/// Context required to process a gateway event without a WebSocket connection.
Expand All @@ -1214,10 +1239,6 @@ pub struct GatewayEventContext {
pub adapter: Arc<dyn ChatAdapter>,
pub dispatcher: Arc<crate::dispatch::Dispatcher>,
pub router: Arc<crate::adapter::AdapterRouter>,
pub allow_all_channels: bool,
pub allowed_channels: HashSet<String>,
pub allow_all_users: bool,
pub allowed_users: HashSet<String>,
pub allow_bot_messages: bool,
pub trusted_bot_ids: HashSet<String>,
pub bot_username: Option<String>,
Expand Down Expand Up @@ -1254,42 +1275,39 @@ fn echo_allowed(key: &str) -> bool {
}
}

pub async fn process_gateway_event(
event_json: &str,
ctx: &GatewayEventContext,
) -> anyhow::Result<bool> {
let event: GatewayEvent = serde_json::from_str(event_json)
.map_err(|e| anyhow::anyhow!("invalid gateway event JSON: {e}"))?;

// Structural gating (bot filter + @mention) stays in should_skip_event.
// L2 (channel) + L3 (identity) are now enforced by the shared ingress gate
// (`router.gate_incoming`) below, so we neuter should_skip_event's channel/user
// checks here by passing allow-all for them.
let no_ids: HashSet<String> = HashSet::new();
let filter = EventFilterParams {
allow_all_channels: true,
allowed_channels: &no_ids,
allow_all_users: true,
allowed_users: &no_ids,
allow_bot_messages: ctx.allow_bot_messages,
trusted_bot_ids: &ctx.trusted_bot_ids,
bot_username: ctx.bot_username.as_deref(),
};
if should_skip_event(&event, &filter) {
return Ok(false);
}
/// Outcome of the shared ingress trust gate. `Deny { echo }` carries the
/// throttled request-access echo payload (channel + message) when the deny is
/// an identity deny and the per-sender throttle admits it; the CALLER decides
/// how to deliver it. Delivery must NOT be awaited inline inside the WS event
/// loop: `GatewayAdapter::send_message` (streaming mode) waits for a
/// `GatewayResponse` that is dispatched by that same loop — awaiting there
/// would stall all event processing for the reply timeout. The WS path spawns
/// the echo; the unified path (axum/bridge task) awaits it directly.
enum GateOutcome {
Allow,
Deny { echo: Option<(ChannelRef, String)> },
}

// Shared ingress trust gate (L2 scope + L3 identity), keyed by platform.
// Phase 1: `is_dm = false` preserves today's behavior where gateway DMs are
// evaluated against the channel allowlist like any other channel (the
// `allow_dm` surface semantics arrive with the per-platform trust flip).
// TODO(phase-2): derive is_dm from the event/ChannelRef carrier so the
// `allow_dm` L2 surface can be enforced and tested for gateway platforms.
/// Shared ingress trust gate for gateway events — used by BOTH the standalone
/// WebSocket path (`run_gateway_adapter`) and the unified path
/// (`process_gateway_event`), so L2 (channel scope) and L3 (identity) are
/// enforced by the same per-platform registry regardless of deployment mode
/// (#1356 Phase 1c prerequisite).
///
/// On `DenyIdentity`, returns the throttled request-access echo payload; on
/// `DenyScope` (and any future variant), denies silently — scope is not a
/// security boundary, so no echo.
///
/// Phase 1: `is_dm = false` preserves today's behavior where gateway DMs are
/// evaluated against the channel allowlist like any other channel (the
/// `allow_dm` surface semantics arrive with the per-platform trust flip).
/// TODO(phase-2): derive is_dm from the event/ChannelRef carrier so the
/// `allow_dm` L2 surface can be enforced and tested for gateway platforms.
fn gate_gateway_event(router: &crate::adapter::AdapterRouter, event: &GatewayEvent) -> GateOutcome {
let decision =
ctx.router
.gate_incoming(&event.platform, &event.channel.id, false, &event.sender.id);
router.gate_incoming(&event.platform, &event.channel.id, false, &event.sender.id);
match decision {
crate::trust::Decision::Allow => {}
crate::trust::Decision::Allow => GateOutcome::Allow,
crate::trust::Decision::DenyIdentity => {
// L3 identity deny → echo the sender their ID so they can request
// access (throttled to avoid amplification). Bots never reach here
Expand All @@ -1301,21 +1319,23 @@ pub async fn process_gateway_event(
"gateway event denied (identity); echoing request-access"
);
let throttle_key = format!("{}:{}", event.platform, event.sender.id);
if echo_allowed(&throttle_key) {
let echo = if echo_allowed(&throttle_key) {
let echo_channel = ChannelRef {
platform: event.platform.clone(),
channel_id: event.channel.id.clone(),
thread_id: event.channel.thread_id.clone(),
parent_id: None,
origin_event_id: Some(event.event_id.clone()),
};
let echo = format!(
let msg = format!(
"⚠️ You are not on this bot's trusted list.\nYour ID: {}\nAsk the admin to add it to allowed_users.",
event.sender.id
);
let _ = ctx.adapter.send_message(&echo_channel, &echo).await;
}
return Ok(false);
Some((echo_channel, msg))
} else {
None
};
GateOutcome::Deny { echo }
}
// DenyScope (and any future variant) → silent drop (scope is not a
// security boundary; no request-access echo).
Expand All @@ -1327,6 +1347,45 @@ pub async fn process_gateway_event(
?decision,
"gateway event denied (scope); silent"
);
GateOutcome::Deny { echo: None }
}
}
}

pub async fn process_gateway_event(
event_json: &str,
ctx: &GatewayEventContext,
) -> anyhow::Result<bool> {
let event: GatewayEvent = serde_json::from_str(event_json)
.map_err(|e| anyhow::anyhow!("invalid gateway event JSON: {e}"))?;

// Structural gating (bot filter + @mention) stays in should_skip_event.
// L2 (channel) + L3 (identity) are now enforced by the shared ingress gate
// (`router.gate_incoming`) below, so we neuter should_skip_event's channel/user
// checks here by passing allow-all for them.
let no_ids: HashSet<String> = HashSet::new();
let filter = EventFilterParams {
allow_all_channels: true,
allowed_channels: &no_ids,
allow_all_users: true,
allowed_users: &no_ids,
allow_bot_messages: ctx.allow_bot_messages,
trusted_bot_ids: &ctx.trusted_bot_ids,
bot_username: ctx.bot_username.as_deref(),
};
if should_skip_event(&event, &filter) {
return Ok(false);
}

// Shared ingress trust gate (L2 scope + L3 identity), keyed by platform.
// Awaiting echo delivery here is safe: this runs on the axum/bridge task,
// not inside the WS event loop.
match gate_gateway_event(&ctx.router, &event) {
GateOutcome::Allow => {}
GateOutcome::Deny { echo } => {
if let Some((echo_channel, msg)) = echo {
let _ = ctx.adapter.send_message(&echo_channel, &msg).await;
}
return Ok(false);
}
}
Expand Down Expand Up @@ -1628,6 +1687,63 @@ mod tests {
}
}

#[test]
fn ws_path_filter_is_structural_only() {
// The WS path's hoisted filter neuters channel/user checks (allow-all)
// because L2/L3 moved to the shared trust registry (#1356 Phase 1c
// prerequisite). This pins the two properties that combination relies
// on: unknown channels/users PASS the structural filter (the gate
// decides), while bot admission and @mention gating still apply.
let no_ids: HashSet<String> = HashSet::new();
let trusted: HashSet<String> = ["good-bot".to_string()].into_iter().collect();
let filter = EventFilterParams {
allow_all_channels: true,
allowed_channels: &no_ids,
allow_all_users: true,
allowed_users: &no_ids,
allow_bot_messages: false,
trusted_bot_ids: &trusted,
bot_username: Some("mybot"),
};

// Unknown human in unknown channel: structural filter passes it through.
let ev = make_event(
false,
"stranger",
"unlisted-channel",
"private",
None,
vec!["mybot"],
);
assert!(!should_skip_event(&ev, &filter));

// Untrusted bot still skipped (structural, stays on this path).
let ev = make_event(
true,
"evil-bot",
"unlisted-channel",
"private",
None,
vec![],
);
assert!(should_skip_event(&ev, &filter));

// Trusted bot admitted.
let ev = make_event(
true,
"good-bot",
"unlisted-channel",
"private",
None,
vec![],
);
assert!(!should_skip_event(&ev, &filter));

// Group without @mention still skipped (structural, stays on this path).
let ev = make_event(false, "stranger", "group-1", "group", None, vec![]);
assert!(should_skip_event(&ev, &filter));
}

#[test]
fn echo_allowed_throttles_repeat_within_window() {
// Unique key so we don't collide with other tests touching the global map.
Expand Down
4 changes: 2 additions & 2 deletions docs/config-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ Custom Gateway adapter for platforms like Telegram, LINE, Feishu/Lark, and Googl

First-class LINE section — credentials, connection, and L3 identity trust (config-first parity, #1376). Replaces the uniform `GATEWAY_ALLOW_ALL_USERS` / `GATEWAY_ALLOWED_USERS` env vars for LINE trust — relying on those for LINE is deprecated and warns at startup.

> **Mode scoping:** takes effect on the **embedded/unified adapter path** (see the note under `[wecom]` / `[googlechat]` / `[teams]` below — the same applies here).
> **Trust resolution:** applies in **both** deployment modes (see the note under `[wecom]` / `[googlechat]` / `[teams]` below — the same applies here).

Each field resolves: config value → `LINE_*` env var → default (deny-all).

Expand Down Expand Up @@ -214,7 +214,7 @@ Full first-class Teams section (config-first parity, #1380) — credentials, con

First-class L3 identity trust — same shape and semantics as `[line]`. Each section replaces the uniform `GATEWAY_ALLOW_ALL_USERS` / `GATEWAY_ALLOWED_USERS` env vars for its platform (deprecated: warns at startup, becomes an error in Phase 2). Platform credentials remain on the gateway env vars (`TEAMS_APP_ID`/`TEAMS_APP_SECRET`) until the Teams config-first parity slice lands (#1380).

> **Mode scoping:** these sections (like `[line]`) take effect on the **embedded/unified adapter path**, where events pass the shared ingress trust gate. Deployments using the standalone `openab-gateway` companion over WebSocket enforce trust via `[gateway].allow_all_users` / `allowed_users` instead; Phase 1c consolidates the two paths.
> **Trust resolution:** these sections (like `[line]`) apply in **both** deployment modes — the embedded/unified adapter path and the broker's WebSocket path to the standalone `openab-gateway` companion both consult the shared per-platform trust registry. Precedence per platform: `GATEWAY_*` env < `[gateway]` section < `[<platform>]` section (the platform section wins when both are set).

Each field resolves: config value → `TEAMS_*` env var → default (deny-all).

Expand Down
2 changes: 1 addition & 1 deletion docs/google-chat.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ allowed_users = ["users/123456789"]

### User Trust (`[googlechat]` section)

> **Mode scoping:** the `[googlechat]` section applies when the Google Chat adapter is **embedded in the OAB binary** (unified mode). Enable it with `[googlechat] enabled = true`; `GOOGLE_CHAT_ENABLED=true` remains the env-only fallback. In the standalone-gateway mode shown above, trust is enforced by `[gateway].allow_all_users` / `allowed_users` instead — the `[googlechat]` section has no effect on that path yet (Phase 1c consolidates the two).
> **Trust resolution:** the `[googlechat]` section's trust settings apply in **both** deployment modes (enable the embedded adapter with `[googlechat] enabled = true`; `GOOGLE_CHAT_ENABLED=true` remains the env-only fallback). Broker-side enforcement goes through the shared per-platform trust registry with precedence `GATEWAY_*` env < `[gateway]` section < `[googlechat]` sectionin the standalone-gateway mode, the broker's WebSocket path consults the same registry, so a `[googlechat]` section overrides `[gateway].allow_all_users` / `allowed_users` for this platform.

Identity trust defaults to **deny-all** (identity-trust-none ADR): unknown senders are rejected until explicitly admitted. Configure trust with a first-class `[googlechat]` section:

Expand Down
2 changes: 1 addition & 1 deletion docs/line.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ platform = "line"

### User Trust (`[line]` section)

> **Mode scoping:** the `[line]` section applies when the LINE adapter is **embedded in the OAB binary** (unified mode, `LINE_CHANNEL_SECRET` env set on the OAB container). In the standalone-gateway mode shown above, trust is enforced by `[gateway].allow_all_users` / `allowed_users` instead — the `[line]` section has no effect on that path yet (Phase 1c consolidates the two).
> **Trust resolution:** the `[line]` section's trust settings apply in **both** deployment modes. Broker-side enforcement goes through the shared per-platform trust registry with precedence `GATEWAY_*` env < `[gateway]` section < `[line]` section — in the standalone-gateway mode, the broker's WebSocket path consults the same registry, so a `[line]` section overrides `[gateway].allow_all_users` / `allowed_users` for this platform.

Identity trust defaults to **deny-all** (identity-trust-none ADR): unknown senders are rejected until explicitly admitted. Configure trust with a first-class `[line]` section:

Expand Down
Loading
Loading