Skip to content

feat(reborn): add ProductAdapter contracts + Telegram v2 tracer-bulle… - #3316

Closed
nickpismenkov wants to merge 2 commits into
reborn-integrationfrom
feat/migrate-adapters
Closed

nickpismenkov wants to merge 2 commits into
reborn-integrationfrom
feat/migrate-adapters

Conversation

@nickpismenkov

@nickpismenkov nickpismenkov commented May 6, 2026 •

Copy link
Copy Markdown
Contributor

First-slice landing for #3285 (Migrate external channel adapters onto ProductAdapter contract). Three new crates prove the ProductAdapter boundary end-to-end against fake Reborn services and recorded Telegram payloads, while keeping the legacy v1 Telegram WASM channel default-on and unchanged.

Summary

  • New ironclaw_product_adapters crate defines the ProductAdapter contract ([Reborn] Define ProductAdapter replacement for stale transport PR #3269 first slice): typed inbound/outbound DTOs, sealed ProtocolAuthEvidence::Verified, ProtocolHttpEgress with declared hosts + opaque credential handles, OutboundDeliverySink, projection cursor stub, in-memory fakes, and architecture/redaction boundary tests.
  • New ironclaw_wasm_product_adapters host crate provides constant-time webhook auth verifiers (HMAC-SHA256, shared-secret-header), EgressPolicy enforcement, NativeProductAdapterRunner glue, and the wit/product_adapter.wit contract for the eventual wasmtime component-model build.
  • New ironclaw_telegram_v2_adapter is the Telegram v2 tracer-bullet ProductAdapter: greenfield against the v2 contract, normalizes external refs, gates group/supergroup messages on explicit triggers (mention/reply-to-bot/recognized command), exposes bounded attachment descriptors, and renders projection-derived final replies through constrained egress to api.telegram.org.
  • Default-off wiring via REBORN_TELEGRAM_V2_ENABLED=false (env + ChannelsConfig) plus validate_telegram_v1_v2_exclusivity startup fail-closed when v1 and v2 both target the same installation. Legacy v1 Telegram path is unmodified.
  • 116 new tests cover all 16 acceptance criteria from [Reborn] Migrate external channel adapters onto ProductAdapter contract #3285 plus deterministic protocol smoke fixtures and redaction sentinels.

Change Type

  • New feature (Reborn product-adapter boundary, behind a default-off flag)
  • Documentation (two new contract docs + freeze-index update)

Linked Issue

Implements #3285. Coordinates with #3269 (ProductAdapter contract — first slice landed inline), #3266 (outbound egress policy), #3193 (ConversationBindingService), #3094 (gate UX, deferred), #3032 (no-exposure), #3020 (compatibility-gate), #3279 (TurnCoordinator product-flow tests).

What's in this PR

crates/ironclaw_product_adapters/

ProductAdapter contract types and traits:

  • DTOs: ProductInboundEnvelope, ProductInboundPayload (UserMessage/Command/ApprovalResolution/AuthResolution/SubscriptionRequest/NoOp), ProductInboundAck (Accepted/DeferredBusy/Rejected/Duplicate/NoOp), ProductOutboundEnvelope, ProductOutboundPayload (FinalReply/Progress/GatePrompt/AuthPrompt/ProjectionSnapshot/ProjectionUpdate).
  • External refs: ExternalActorRef, ExternalConversationRef (keyed by space + conversation + optional topic; reply target message id is not part of the conversation key), ExternalEventId, ProductAttachmentDescriptor (no raw bytes, no source URLs, no host paths).
  • Auth: ProtocolAuthEvidence::Verified is sealed via a crate-private HostAuthSeal; only the public mark_*_verified helpers in ironclaw_product_adapters::auth can mint a Verified value. WASM components and downstream adapters cannot fabricate verification.
  • Egress: ProtocolHttpEgress trait, DeclaredEgressHost, EgressCredentialHandle (opaque), OutboundDeliverySink, DeliveryStatus (Delivered/FailedRetryable/FailedUnauthorized/Deferred).
  • Capabilities: typed ProductAdapterCapabilities (inbound messages/commands/attachments, final-reply push, opt-in progress push, opt-in gate push, projection subscription, sync wait, delivery status reporting).
  • Traits: ProductAdapter, ProductWorkflow, ProjectionStream.
  • In-memory fakes: FakeProductWorkflow (programmable outcomes + dedupe by external_event_id), FakeProtocolHttpEgress (records calls, validates declared host + credential handle), FakeOutboundDeliverySink, FakeProjectionStream.
  • Boundary tests forbid dependencies on ironclaw_dispatcher, _capabilities, _host_runtime, _network, _secrets, _filesystem, _wasm, _processes, _mcp, _scripts, _runtime_policy, _authorization, _run_state, _approvals, _resources, _trust, _extensions, _safety, _skills, _engine, _gateway, _tui, _memory, _events, _reborn_event_store, _architecture, and ironclaw_turns::runner.
  • Redaction tests prove DTOs and errors do not leak raw secrets, host paths, raw prompts, raw tool input, provider/runtime internals, or backend diagnostics. RedactedString wraps any value originating from a protocol payload and renders as <redacted> in Debug/Display/Serialize.

crates/ironclaw_wasm_product_adapters/

Host runtime glue (currently runtime-free; the wasmtime component-model binary build lands alongside this crate in a follow-up):

  • WebhookAuthVerifier trait + HmacWebhookAuth (Slack-style v0 HMAC-SHA256 with timestamp prefix) and SharedSecretHeaderAuth (Telegram-style). Both use subtle::ConstantTimeEq to avoid timing oracles.
  • EgressPolicy — declared-host + credential-handle allowlist enforcement.
  • NativeProductAdapterRunner — receives webhook headers + body, verifies auth, mints a sealed Verified evidence, calls ProductAdapter::parse_inbound, forwards the envelope to ProductWorkflow::accept_inbound, and returns either Acknowledged { ack } or NoOp. RunnerError::is_auth_failure() and is_retryable() map the protocol-status response.
  • wit/product_adapter.wit — agreed shape of the eventual WASM component contract: exports for manifest, parse-inbound, render-outbound; constrained host imports for log, now-millis, http-egress. No raw filesystem, env, random, or arbitrary HTTP capability is exposed.

crates/ironclaw_telegram_v2_adapter/

Telegram WASM v2 ProductAdapter (greenfield, no v1 channel-type imports):

Telegram field Reborn ref
update_id ExternalEventId (tg-<installation>-<update_id>)
message.from.id ExternalActorRef (kind telegram_user)
message.chat.id ExternalConversationRef.conversation_id
message.message_thread_id ExternalConversationRef.topic_id
message.message_id ExternalConversationRef.reply_target_message_id (NOT part of conversation key)
  • Group/supergroup gating: ambient messages → NoOp (200 ack, workflow never sees them). Envelopes only when (a) mention entity matches configured bot_username (case-insensitive, UTF-16-aware offset slicing), or (b) reply_to_message.from.is_bot && from.id == bot_user_id, or (c) bot_command entity for a recognized command (with /foo@botname suffix matching).
  • Attachments: bounded ProductAttachmentDescriptor with external_file_id, mime_type, optional filename, optional size_bytes, and kind. No raw bytes, no source URLs, no host paths.
  • Outbound: FinalReply → POST api.telegram.org/sendMessage with chat_id + optional message_thread_id + optional reply_to_message_id. Progress { Typing } → sendChatAction (only when ExternalProgressPush is opted-in per [Reborn] Define outbound egress and subscription policy #3266). GatePrompt/AuthPrompt are deferred to [Reborn] Add approval/auth interaction services #3094 (no side effects in the first slice). ProjectionSnapshot/Update are dropped (Telegram doesn't subscribe).
  • Egress targets only the declared api.telegram.org host. Bot token travels as an opaque EgressCredentialHandle = "telegram_bot_token"; the host resolves the underlying secret at request time.

Default-off wiring

  • REBORN_TELEGRAM_V2_ENABLED=false (default) keeps legacy v1 Telegram running unchanged through the v1 channel manager.
  • New ironclaw::config::validate_telegram_v1_v2_exclusivity(v1_active, v2_active) -> Result<(), ConfigError> fails closed at startup when both paths are configured for the same telegram installation.
  • tests/telegram_v2_default_off_integration.rs is a caller-tier integration test (not just a helper test) that drives the validator + asserts Config::for_testing(...).channels.reborn_telegram_v2_enabled == false.

Docs

  • docs/reborn/contracts/product-adapters.md — frozen contract: layering, invariants, capability grid, ack-to-status mapping.
  • docs/reborn/contracts/telegram-v2.md — Telegram-specific normalization, group gating, attachment policy, idempotency, and AC test pointer.
  • _contract-freeze-index.md updated.
  • .env.example documents REBORN_TELEGRAM_V2_ENABLED.

Acceptance criteria coverage (#3285)

crates/ironclaw_telegram_v2_adapter/tests/product_adapter_telegram_contract.rs has one named test per AC bullet:

AC Tests
1 — only v2 DTOs ac1_telegram_v2_does_not_import_v1_channel_types
2 — outside src/channels ac2_product_adapter_contracts_live_outside_src_channels
3 — host verifies auth before envelope ac3_runner_blocks_envelope_construction_on_bad_secret, _on_missing_secret, ac3_adapter_refuses_unverified_evidence_directly
4 — refs normalized ac4_parse_normalizes_all_refs
5 — no canonical-state writes ac5_adapter_does_not_import_turn_coordinator, ac5_adapter_path_only_invokes_workflow_facade
6 — workflow durable outcomes ac6_workflow_returns_each_durable_outcome_kind
7 — update_id dedupe ac7_duplicate_update_id_returns_prior_outcome_no_double_submit
8 — bounded attachment descriptors ac8_attachments_have_no_raw_bytes_or_source_urls
9 — group gating ac9_private_chat_creates_inbound, _group_ambient_is_noop_ack, _group_explicit_mention_creates_inbound, _group_command_creates_inbound
10 — conversation key shape ac10_conversation_key_uses_chat_and_topic_not_message_id
11 — webhook ack semantics ac11_durable_outcome_classification_for_each_path, ac11_duplicate_returns_200_no_op_ack
12 — projection-derived outbound ac12_final_reply_renders_to_reply_target_binding
13 — constrained egress ac13_egress_to_undeclared_host_is_blocked, _telegram_only_declares_telegram_api, _egress_request_carries_credential_handle_not_token
14 — separate delivery status ac14_delivery_failure_records_status_separately, _does_not_mutate_canonical_workflow_state
15 — gate UX deferred to #3094 ac15_gate_prompt_envelope_is_no_op_egress_in_first_slice
16 — default-off ac16_default_off_marker_present_in_workspace_root_config + tests/telegram_v2_default_off_integration.rs
smoke / redaction smoke_recorded_payloads_match_expected_outcomes, redaction_sentinels_in_envelope_debug, _in_error_display, telegram_default_capabilities_pin_first_slice_behavior, telegram_does_not_consume_projection_subscriptions, projection_stream_can_drive_telegram_via_render_outbound_chain

Validation

  • cargo fmt --all -- --check — clean
  • cargo clippy --all --benches --tests --examples --all-features — zero warnings
  • cargo build — passes
  • cargo test -p ironclaw_product_adapters -p ironclaw_wasm_product_adapters -p ironclaw_telegram_v2_adapter — 112 tests pass
  • cargo test --test telegram_v2_default_off_integration — 5 tests pass
  • cargo test -p ironclaw config::channels::telegram_v2_tests --lib — 4 tests pass
  • cargo test --features integration not run for this PR — no DB-backed code paths added

Security Impact

  • New crates introduce a constrained boundary for protocol authentication. Webhook signatures are verified in constant time (subtle::ConstantTimeEq).
  • ProtocolAuthEvidence::Verified is sealed: only crate-internal helpers can construct it. WASM components (when wired up) cannot mint verification.
  • ProtocolHttpEgress enforces declared hosts + credential-handle allowlist. Bot tokens never reach adapter code or WASM linear memory; the host resolves them at request time.
  • DTOs and errors carry RedactedString for any value originating from a protocol payload, secret store, or backend error text. Redaction is asserted by tests across Debug, Display, and Serialize.
  • Default-off: legacy v1 Telegram path is byte-for-byte unchanged. The mutual-exclusion validator fails closed at startup if both paths target the same installation.

Database Impact

None. No migrations, no schema changes, no database access in any of the three new crates.

Blast Radius

  • New crates only add code; they are not wired into the v1 channel manager or any production webhook router. The single touch point in legacy code is the additive reborn_telegram_v2_enabled field on ChannelsConfig (default false everywhere it is constructed) plus the new public validate_telegram_v1_v2_exclusivity symbol.
  • src/channels/wasm/telegram_host_config.rs and channels-src/telegram/ are not modified — the v1 path is byte-for-byte unchanged.
  • Three workspace Cargo.toml lines added (members list).

Rollback Plan

Revert the single commit. The feature flag is default-off, so even with the code present, no production behavior changes until an operator explicitly sets REBORN_TELEGRAM_V2_ENABLED=true and wires the v2 webhook route.

Review Follow-Through

  • The wasmtime component-model binary build (compile channels-src-v2/telegram/ to a .wasm, instantiate via wasmtime in ironclaw_wasm_product_adapters) is intentionally deferred — the WIT contract is in wit/product_adapter.wit and the Rust-native adapter implementation in ironclaw_telegram_v2_adapter is the same logic that will move into the component.
  • Production wiring of the v2 webhook route (an axum handler driving NativeProductAdapterRunner) is a follow-up. The default-off flag is the cutover seam.
  • Slack/Discord/WhatsApp/Feishu/Signal v2 adapters are explicit non-goals for this slice.
  • Real ProductWorkflow / ConversationBindingService / SessionThreadService implementations land with [Reborn] Define conversation binding and session thread contracts #3193/[Reborn] Add ProductWorkflow and InboundTurnService facade #3280 and replace the in-memory fakes used here.

Review track: B (new feature, default-off, contract-freeze landing for #3269/#3285)

#3285)

First-slice landing for #3285 (Migrate external channel adapters onto
ProductAdapter contract). The PR ships three new crates that prove the
ProductAdapter boundary end-to-end against fake Reborn services and
recorded Telegram payloads, while keeping the legacy v1 Telegram WASM
channel default-on and unchanged.

* `ironclaw_product_adapters` (#3269 first slice) — ProductAdapter trait,
  inbound/outbound DTOs (`ProductInboundEnvelope`, `ProductInboundAck`,
  `ProductOutboundEnvelope`), sealed `ProtocolAuthEvidence::Verified`,
  `ProtocolHttpEgress` with declared hosts + opaque credential handles,
  `OutboundDeliverySink`, projection cursor stub, and in-memory fakes.
  Boundary tests forbid dependencies on `ironclaw_dispatcher`,
  `_capabilities`, `_host_runtime`, `_network`, `_secrets`, `_filesystem`,
  raw runtime lanes, and `ironclaw_turns::runner`. Redaction tests prove
  DTOs/errors do not leak secrets, host paths, or backend internals.

* `ironclaw_wasm_product_adapters` — host runtime glue: HMAC-SHA256 +
  shared-secret-header webhook auth verifiers (constant-time compare),
  `EgressPolicy` (declared host + credential-handle allowlist), and
  `NativeProductAdapterRunner` that wires authentication +
  `ProductAdapter::parse_inbound` + `ProductWorkflow::accept_inbound`
  into one webhook flow. WIT contract for the eventual wasmtime
  component-model build is documented at
  `wit/product_adapter.wit`; the binary build lands in a follow-up.

* `ironclaw_telegram_v2_adapter` — Telegram WASM v2 ProductAdapter,
  greenfield against the v2 contract (no `IncomingMessage`/v1 channel
  types). Normalizes `update_id` to `ExternalEventId`, `chat_id +
  message_thread_id` to `ExternalConversationRef`, and `message_id` as
  reply-target idempotency data (NOT part of the conversation key).
  Group/supergroup gating: only `bot_mention`/`reply_to_bot`/recognized
  bot commands trigger envelopes; ambient group messages produce a 200
  no-op ack. Attachments expose bounded `ProductAttachmentDescriptor`
  values only — no raw bytes, source URLs, or host paths. Outbound
  renders `FinalReply` to `sendMessage` and (capability-gated)
  `Progress` to `sendChatAction`, both via constrained egress to the
  declared `api.telegram.org` host with an opaque credential handle.

Acceptance-criteria test matrix: 32 tests in
`crates/ironclaw_telegram_v2_adapter/tests/product_adapter_telegram_contract.rs`
cover every AC bullet from #3285 (ac1..ac16 plus deterministic protocol
smoke + redaction sentinels). Each test is named after the bullet it
proves and drives the adapter through `NativeProductAdapterRunner` +
`FakeProductWorkflow` + `FakeProtocolHttpEgress` +
`FakeOutboundDeliverySink`.

Default-off wiring:

* New env var `REBORN_TELEGRAM_V2_ENABLED` (default false). Plumbed into
  `ChannelsConfig::reborn_telegram_v2_enabled`.
* `ironclaw::config::validate_telegram_v1_v2_exclusivity` fails closed
  at startup when both v1 and v2 are configured for the same telegram
  installation.
* `tests/telegram_v2_default_off_integration.rs` is a caller-tier
  integration test that drives the validator and confirms
  `Config::for_testing(...).channels.reborn_telegram_v2_enabled ==
  false`.

Docs:

* `docs/reborn/contracts/product-adapters.md` — contract summary,
  layering, frozen invariants, capability grid, default-off rules.
* `docs/reborn/contracts/telegram-v2.md` — Telegram-specific
  normalization, group gating, attachment policy, idempotency, and AC
  test pointer.
* `_contract-freeze-index.md` updated with the new contracts.
* `.env.example` documents `REBORN_TELEGRAM_V2_ENABLED`.

Test coverage: 116 new tests pass. `cargo fmt --check` clean. `cargo
clippy --all --tests --all-features` zero warnings. Legacy v1 Telegram
path (`channels-src/telegram`, `src/channels/wasm/telegram_host_config.rs`)
is unmodified.

Refs: #3269 #3266 #3193 #3094 #3032 #3020 #3279

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules scope: docs Documentation scope: dependencies Dependency updates contributor: core 20+ merged PRs and removed risk: medium Business logic, config, or moderate-risk modules labels May 6, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the Reborn ProductAdapter contract and its initial implementation for Telegram v2, establishing a clean boundary between transport-specific logic and the core pipeline. The changes include core DTO definitions, host runtime glue for authentication and egress enforcement, and comprehensive documentation. Key feedback identifies critical security concerns regarding potential forgery of authentication evidence via deserialization defaults and a replay attack vulnerability in HMAC verification. Additionally, improvements are needed for UTF-16 string slicing logic and error mapping to ensure transient egress failures are correctly handled as retryable.

Comment thread crates/ironclaw_product_adapters/src/auth.rs Outdated
Comment thread crates/ironclaw_wasm_product_adapters/src/auth_verifier.rs
Comment thread crates/ironclaw_telegram_v2_adapter/src/payload.rs
Comment thread crates/ironclaw_telegram_v2_adapter/src/adapter.rs Outdated
@nickpismenkov nickpismenkov linked an issue May 6, 2026 that may be closed by this pull request
18 tasks
Four security/correctness fixes from Gemini code review on PR #3316 plus
the No-panics CI check.

* **HIGH security — Auth-evidence serde forgery (Gemini #1).** The
  `ProtocolAuthEvidence::Verified` variant previously round-tripped
  through serde because its `seal` field used
  `#[serde(skip, default = "HostAuthSeal::host_only")]`. An attacker
  could mint a `Verified` value just by sending the matching JSON. Fix:
  custom `Deserialize` that REJECTS any wire payload claiming
  `kind == "verified"` (and any unknown field). Only `Failed` outcomes
  may cross trust boundaries. The inbound envelope no longer carries
  the full evidence enum — it carries only the sanitized
  `VerifiedAuthClaim`, which round-trips safely without re-opening the
  loophole. Tests pin: forged `Verified` payloads error, serialized
  `Verified` does not round-trip back, `Failed` round-trips, unknown
  fields fail.

* **HIGH security — HMAC replay attack window (Gemini #2).**
  `HmacWebhookAuth` previously verified only the signature, not the
  timestamp. An attacker could replay a captured request indefinitely.
  Fix: enforce a symmetric `|now - ts| <= max_age_secs` window
  (default 300s = 5 min) BEFORE computing the HMAC; injectable `Clock`
  seam (`SystemClock` for prod, `FixedClock` for tests). New tests:
  in-window accept, stale reject, far-future reject, malformed
  timestamp reject, exact-boundary accept, just-outside reject.

* **MEDIUM — Zero-length slice at offset 0 (Gemini #3).**
  `slice_text_by_offset(_, 0, 0)` previously returned `None` for empty
  strings (and at the start of any string), so a zero-length entity at
  position 0 was lost. Fix: initialize `byte_start = Some(0)` when
  `start == 0`, and treat `end == 0` symmetrically; same fix shape on
  `slice_text_to_end`. Nine new tests cover empty strings, slice-at-end,
  multibyte UTF-16 boundaries (🦀), and past-end / boundary cases.

* **MEDIUM — Egress error retryability (Gemini #4).** `render_outbound`
  previously folded every egress failure into the non-retryable
  `EgressDenied`, so the host glue could not re-deliver on transient
  errors. Fix: map `Timeout` / `Network` / `LeakDetected` egress errors
  AND HTTP 5xx / 429 status responses to the retryable
  `WorkflowTransient`; keep `UndeclaredHost` /
  `Unknown/UnauthorizedCredentialHandle` / `PolicyDenied` /
  4xx-non-429 as `EgressDenied`. New `ac14_egress_retryable_classification_matrix`
  drives 8 cases and pins each classification.

* **CI No-panics check.** The check fails on `.unwrap()`/`.expect()` in
  newly-added production code. Two-pronged fix:
  - Feature-gate the test-fakes module (`fakes`) behind a new
    `test-support` Cargo feature so production binaries don't carry
    fake state machinery; downstream crates enable it from
    `[dev-dependencies]`.
  - Annotate the remaining intentional `.expect()` sites (static-host
    construction, JSON serialization of owned scalars) with inline
    `// safety: <reason>` comments per the script's documented
    suppression mechanism.

Test totals across the new crates: 130 passing (35 product-adapters
unit + 18 product-adapters contract + 31 telegram-v2 unit + 33
telegram-v2 contract + 13 wasm-host unit). Plus 5 default-off
integration tests on the main crate.

`cargo fmt --check` clean. `cargo clippy --all --tests --all-features`
zero warnings. `python3 scripts/check_no_panics.py` clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the risk: medium Business logic, config, or moderate-risk modules label May 6, 2026
@zmanian

zmanian commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Overview

7,360-line first-slice landing of the ProductAdapter contract for #3285. Three new crates: ironclaw_product_adapters (DTOs + traits + sealed auth evidence), ironclaw_wasm_product_adapters (host glue: webhook verifiers, egress policy, native runner — no wasmtime yet), ironclaw_telegram_v2_adapter (greenfield Telegram v2). Default-off behind REBORN_TELEGRAM_V2_ENABLED=false; v1 path byte-for-byte unchanged.

Strengths

  • ProtocolAuthEvidence::Verified sealing is layered correctly. HostAuthSeal has a pub(crate) constructor; the custom Deserialize impl only accepts Failed wire payloads. Tests pin the asymmetry: verified_evidence_in_memory_serializes_but_not_back proves a Verified value can be logged/audited but cannot round-trip back. Closes the #[serde(default)] re-mint loophole the PR body calls out.
  • Constant-time webhook auth. subtle::ConstantTimeEq on both verifiers; replay-window check happens before HMAC compute with a symmetric |now - ts| > max_age test. Boundary-inclusive (drift == max_age passes; drift == max_age + 1 fails) and pinned by tests.
  • UTF-16 entity slicing for Telegram is correctly implemented with explicit handling for zero-length entities at offset 0, end-of-string, past-end-returns-None, and multibyte (🦀 case in unit tests).
  • Default-off validator exercised at the caller tier in tests/telegram_v2_default_off_integration.rs, including Config::for_testing(). Matches the "Test Through the Caller" rule.
  • #![forbid(unsafe_code)] on the wasm host glue.

Suggestions

Minor:

  1. Home-rolled mod hex in auth_verifier.rs (~6 lines). The hex crate is already a transitive dep of HMAC tooling. Either drop the inline module and use hex::encode, or skip hex-encoding entirely and compare raw bytes (mac.finalize().into_bytes().ct_eq(decoded_signature_bytes)) to avoid the string allocation.

  2. HmacWebhookAuth fields are pub (pub signing_secret: Vec<u8>, etc.). The struct deliberately omits Debug so direct formatting won't compile, but a downstream caller composing this into a #[derive(Debug)] struct would silently leak the signing secret. Make fields private with constructor + builder, or wrap signing_secret in a RedactedBytes mirroring RedactedString.

  3. now_unix_seconds() as i64 cast in HmacWebhookAuth::verify — for any clock value before year 2262 this is fine, but FixedClock(u64::MAX) test value would silently wrap. Either keep arithmetic in u64 with now.abs_diff(timestamp), or document the year-2262 ceiling.

  4. WebhookAuth::mint_evidence re-clones header names that already live on the verifier. Minor allocation churn per request; not load-bearing.

Non-issues (intentional, called out in PR body)

Risk

  • Blast radius: very low — three new crates, one additive ChannelsConfig field, one new public symbol. v1 path untouched.
  • Security: auth seal + constant-time + sealed serde + redaction is the right shape, and tested at the right tier.
  • Correctness: 116 named tests against 16 ACs; UTF-16 edge-case coverage. Real risk surface is the fakes-only execution paths; the real workflow's dedupe story isn't on this branch.

Recommendation

Approve. The four minor suggestions above are non-blocking polish — drop the home-rolled hex module and tighten signing_secret field visibility in a follow-up. Everything else is acceptable as documented tracer-bullet tradeoffs.

@serrrfirat

Copy link
Copy Markdown
Collaborator

This is like 3 PRs in one. chop it up, agents wont be able to review this correctly.

return Err(RunnerError::AuthenticationFailed { failure });
}
};
let Some(envelope) = self.adapter.parse_inbound(body, evidence)? else {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High Severity

The runner verifies the webhook, but it then trusts authority fields supplied by the adapter-produced envelope.

process_webhook mints trusted evidence, passes it into parse_inbound, and forwards the returned ProductInboundEnvelope directly to accept_inbound. Since the envelope carries public adapter_id, installation_id, and auth_claim fields (and Rust adapters can also call the public mark_*_verified helpers), a buggy or malicious adapter can return an envelope for a different installation/subject than the one the host just verified. That breaks the documented “adapters cannot fabricate verification” boundary and can confuse downstream binding/dedupe/auth decisions.

Please have trusted host/runner code inject or overwrite the authority fields before workflow submission, or change adapters to return only payload/protocol refs and let the runner build the final envelope. Add a malicious fake-adapter test that returns mismatched install/claim data and assert the runner rejects it before accept_inbound.

Comment thread src/config/channels.rs
/// `wasm_channels_enabled` is true).
///
/// `v2_active`: value of `reborn_telegram_v2_enabled`.
pub fn validate_telegram_v1_v2_exclusivity(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

The Telegram v1/v2 exclusivity guard is defined but never invoked by production startup/config resolution.

The docs/tests say startup fails closed when legacy Telegram v1 and Telegram v2 are both active, but the real config/startup path only parses REBORN_TELEGRAM_V2_ENABLED; validate_telegram_v1_v2_exclusivity is exercised directly in tests and is not called before setup_wasm_channels/route registration. A deployment with WASM_CHANNELS_ENABLED=true, a configured legacy telegram channel, and REBORN_TELEGRAM_V2_ENABLED=true therefore proceeds instead of failing at startup.

Please call this validator from the real config or startup path after computing whether v1 and v2 are active, before registering Telegram/WASM webhook handling. Add a caller-level test that drives actual config/startup resolution with both paths enabled and expects ConfigError.

};
let now_secs = self.clock.now_unix_seconds() as i64;
let max_age = self.max_age_secs as i64;
let drift = (now_secs - timestamp_secs).abs();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

The HMAC replay-window calculation can overflow before returning an auth failure.

timestamp_str is parsed as i64, then the verifier computes (now_secs - timestamp_secs).abs(). A hostile timestamp such as -9223372036854775808 overflows the subtraction in overflow-check builds, so an unauthenticated request can panic the verifier instead of receiving a fail-closed auth result.

Please reject negative timestamps (or parse as u64) and use checked arithmetic / abs_diff for the drift calculation. Add tests for i64::MIN, negative timestamps, and very large future timestamps.

failure: ProtocolAuthFailure::Missing,
};
};
if !bool::from(received.as_bytes().ct_eq(self.expected_secret.as_bytes())) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

Webhook verifiers accept empty secrets, which makes a missing secret fail open.

SharedSecretHeaderAuth has public fields and no empty-secret guard, so expected_secret == "" verifies when the request supplies an empty X-Telegram-Bot-Api-Secret-Token value. HmacWebhookAuth::new likewise accepts an empty signing key; HMAC with an empty key is computable by anyone. If host wiring builds these verifiers from a missing/empty env/config value, authentication becomes trivially forgeable.

Please hide fields behind validated constructors (or defensively fail in verify) and reject empty header names, subjects, shared secrets, and HMAC keys at startup. Add tests proving empty shared-secret and empty HMAC-key configurations reject even when the request matches the empty value.

from.is_bot && from.id == bot_user_id
}

fn recognized_bot_command(message: &TelegramMessage, policy: &GroupTriggerPolicy) -> bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

Group command detection accepts recognized commands anywhere in the message, not only at the start.

The policy comment says recognized commands trigger when a group message starts with /foo or /foo@botusername, but this loop accepts every bot_command entity regardless of its offset. In a supergroup, text such as I ran /help yesterday can include a bot_command entity at offset 6; if help is recognized, the adapter forwards ambient chatter as a bot invocation.

Please require entity.offset == 0 for command triggers, or update the contract to explicitly allow mid-message commands. Add a fixture with a non-leading /help entity and assert it returns NoOp.

"chat_id".into(),
serde_json::Value::Number(reply.chat_id.into()),
);
body.insert("text".into(), serde_json::Value::String(view.text.clone()));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

Final replies are sent as one unbounded Telegram sendMessage body.

Telegram rejects message text over its 4096-character limit. As written, a long model reply is rendered into a single request; Telegram returns a 400, and render_outbound maps that 4xx to non-retryable EgressDenied, so the user receives no reply instead of chunked delivery.

Please split final replies into Telegram-sized chunks (preserving topic/reply target, and code-block safety if parse mode is added) or enforce a documented truncation policy. Add render/adapter tests that an over-limit final reply produces multiple valid sendMessage requests or a deliberate truncation outcome.

chat_kind: TelegramChatKind,
policy: &GroupTriggerPolicy,
) -> Option<ProductTriggerReason> {
if chat_kind == TelegramChatKind::Private {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

Private-chat bot commands are downgraded to ordinary user messages.

classify_trigger returns DirectChat immediately for private chats, and build_payload only emits ProductInboundPayload::Command when the trigger is BotCommand. That means a normal private /start or /help message with a Telegram bot_command entity is delivered as UserMessage { text: "/start", trigger: DirectChat } even though the adapter advertises InboundCommands. The common onboarding/help/control path will not reach command handling.

Please detect/extract recognized bot commands before the private-chat early return, or have build_payload emit Command whenever command extraction matches regardless of trigger. Add a private /start or /help fixture through parse_inbound/the runner and assert it produces ProductInboundPayload::Command.

}
}

async fn render_outbound(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

Outbound rendering does not fail closed when an envelope is routed to the wrong adapter installation.

render_outbound never checks that envelope.adapter_id and envelope.installation_id match this TelegramV2Adapter instance. If a projection-router bug, stale queue entry, or corrupted outbound envelope for installation B reaches adapter instance A, the adapter will parse the target and send the reply using A's credential handle. This is the outbound confused-deputy analogue: a misrouted envelope can be delivered with the wrong bot credentials and incorrect delivery accounting.

Please validate envelope.adapter_id == self.config.adapter_id and envelope.installation_id == self.config.installation_id at the top of render_outbound, and fail closed before egress on mismatch. Add a test with mismatched IDs and assert FakeProtocolHttpEgress records no send calls.

}
};

let response = egress.send(request).await.map_err(map_egress_error)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

DeliveryStatusReporting is advertised, but the real outbound path never records a DeliveryStatus.

The contract/docs say outbound delivery reports statuses to OutboundDeliverySink, and Telegram's default capabilities include DeliveryStatusReporting. In the actual path here, however, render_outbound only calls egress.send and returns Ok/Err; there is no sink parameter or host wrapper recording Delivered, FailedRetryable, or FailedUnauthorized for the envelope's delivery_attempt_id. The AC14 test records sink.record(...) manually after observing the error, so it does not prove production/caller behavior.

Please add a host outbound runner/wrapper that owns the original ProductOutboundEnvelope, invokes render_outbound, classifies the result, and records DeliveryStatus to a sink (or pass a sink through the render API). Replace the manual sink-record test with caller-level tests for 2xx, 5xx/429, and 4xx outcomes.

None
}

fn has_bot_mention(message: &TelegramMessage, policy: &GroupTriggerPolicy) -> bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity

Group attachment captions cannot trigger the bot by mention/command.

The group trigger checks only message.text plus entities, while Telegram sends formatting/entities for captions in caption_entities and this DTO does not deserialize that field. As a result, a supergroup photo/document captioned @ironclaw_bot analyze this is treated as ambient NoOp unless it is also a reply to the bot, even though the adapter advertises inbound attachments and explicit group triggers.

Please deserialize caption_entities and run mention/command extraction over (text, entities) or (caption, caption_entities) as appropriate, then strip leading caption mentions when building UserMessagePayload. Add a recorded group photo/document caption-mention fixture and assert the adapter returns an envelope with BotMention plus an attachment descriptor.

@serrrfirat

Copy link
Copy Markdown
Collaborator

Split this XL PR into a 7-PR stack so each slice is reviewable while preserving the original implementation and retaining Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com> on every split commit.

Stack order:

  1. feat(reborn): add product adapter contract #3351 — ProductAdapter core contract
  2. feat(reborn): add product adapter host auth and egress primitives #3352 — host auth/egress primitives
  3. feat(reborn): add native product adapter runner #3353 — native ProductAdapter runner
  4. feat(reborn): add telegram v2 payload normalization #3354 — Telegram v2 payload normalization
  5. feat(reborn): add telegram v2 product adapter tracer bullet #3355 — Telegram v2 ProductAdapter tracer bullet
  6. feat(reborn): add telegram v2 default-off config guard #3356 — Telegram v2 default-off config guard
  7. docs(reborn): add product adapter contract docs #3357 — ProductAdapter / Telegram v2 contract docs

I did not close this original PR; it can stay as the umbrella/reference until the split stack is reviewed/merged.

@serrrfirat

Copy link
Copy Markdown
Collaborator

Closing these as i already chopped them up in other 7 PRs.

@serrrfirat serrrfirat closed this May 7, 2026
serrrfirat added a commit that referenced this pull request May 7, 2026
Split from PR #3316. Preserves the core ProductAdapter boundary before host runtime glue or Telegram-specific implementation.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
serrrfirat added a commit that referenced this pull request May 8, 2026
Split from PR #3316. Lands constant-time webhook auth verification, declared-host/credential-handle egress policy, and the WIT contract before adding runner glue or Telegram behavior.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
serrrfirat added a commit that referenced this pull request May 8, 2026
Split from PR #3316. Lands constant-time webhook auth verification, declared-host/credential-handle egress policy, and the WIT contract before adding runner glue or Telegram behavior.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
serrrfirat added a commit that referenced this pull request May 8, 2026
Split from PR #3316. Lands constant-time webhook auth verification, declared-host/credential-handle egress policy, and the WIT contract before adding runner glue or Telegram behavior.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
serrrfirat added a commit to serrrfirat/ironclaw that referenced this pull request May 13, 2026
Split from PR nearai#3316. Adds the Telegram ProductAdapter implementation, outbound rendering through constrained egress, recorded payload fixtures, delivery-status behavior, and end-to-end adapter contract tests.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
serrrfirat added a commit to serrrfirat/ironclaw that referenced this pull request May 13, 2026
Split from PR nearai#3316. Adds the default-off REBORN_TELEGRAM_V2_ENABLED marker, config plumbing, and fail-closed v1/v2 Telegram exclusivity validator without registering production v2 routes.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
serrrfirat added a commit to serrrfirat/ironclaw that referenced this pull request May 13, 2026
Split from PR nearai#3316. Freezes the ProductAdapter and Telegram v2 contract docs after the code/test slices are present.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Preserves the core ProductAdapter boundary before host runtime glue or Telegram-specific implementation.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Lands constant-time webhook auth verification, declared-host/credential-handle egress policy, and the WIT contract before adding runner glue or Telegram behavior.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Adds the trusted native runner that verifies protocol auth, mints sealed evidence, invokes ProductAdapter parsing, and forwards envelopes to the ProductWorkflow facade.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Lands Telegram update parsing, explicit group trigger gating, normalized Reborn external refs, and bounded attachment descriptors before ProductAdapter outbound delivery.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Adds the Telegram ProductAdapter implementation, outbound rendering through constrained egress, recorded payload fixtures, delivery-status behavior, and end-to-end adapter contract tests.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Adds the default-off REBORN_TELEGRAM_V2_ENABLED marker, config plumbing, and fail-closed v1/v2 Telegram exclusivity validator without registering production v2 routes.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Freezes the ProductAdapter and Telegram v2 contract docs after the code/test slices are present.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Preserves the core ProductAdapter boundary before host runtime glue or Telegram-specific implementation.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
Split from PR nearai#3316. Lands constant-time webhook auth verification, declared-host/credential-handle egress policy, and the WIT contract before adding runner glue or Telegram behavior.

Co-authored-by: Nikolay Pismenkov <nickpismenkov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: dependencies Dependency updates scope: docs Documentation size: XL 500+ changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Reborn] Migrate external channel adapters onto ProductAdapter contract

3 participants