Skip to content

feat(line): full [line] section — credentials + connection config-first#1381

Merged
thepagent merged 2 commits into
mainfrom
feat/1376-line-config-first
Jul 13, 2026
Merged

feat(line): full [line] section — credentials + connection config-first#1381
thepagent merged 2 commits into
mainfrom
feat/1376-line-config-first

Conversation

@chaodu-agent

Copy link
Copy Markdown
Collaborator

What problem does this solve?

First platform slice of the config-first parity umbrella #1375: LINE's channel credentials were env-only (LINE_CHANNEL_SECRET / LINE_CHANNEL_ACCESS_TOKEN), violating the invariant that every platform setting resolves [section] config → PLATFORM_* env → default with config always winning.

Proposed Solution

Mirrors the TelegramConfig reference pattern end-to-end:

Layer Change
core config.rs LineConfig gains channel_secret, channel_access_token, webhook_path; resolve() applies config → LINE_* env → default per field (empty-string ${} expansion falls through to env)
gateway lib.rs GatewayLineConfig bridge + AppState::apply_line_config() (keeps the crate free of openab-core); new line_webhook_path state; standalone gateway honors LINE_WEBHOOK_PATH and mounts at the resolved path
unified main.rs Applies [line] before warn_unenforceable_l1, so a config-supplied channel_secret satisfies the #1373 L1 startup check; route mounts at the resolved path
docs config.toml.example, config-reference.md field table, line.md ([line] primary, env fallback)

Backward compatible: env-only deployments resolve identically; default mount path unchanged.

Tests

  • line_section_parses_from_toml — new fields parse
  • line_resolve_all_scenarios — 3 new scenarios: config-wins-over-env for all three fields; empty-string expansion falls through to env; nothing set → defaults
  • apply_line_config_overrides_env_state_and_feeds_l1_warning — pins the feat(gateway): warn when webhook L1 auth is unenforceable (#1356 Phase 1) #1373 integration: env-derived flagged state clears once config supplies the secret

Validation

At head (macOS arm64, main 9af9fcd):

  • cargo clippy -p openab-core -p openab-gateway --all-features -- -D warnings — clean; gateway no-features also clean
  • cargo check --features unified and default — pass
  • core line tests 16/16; gateway l1_audit tests 5/5
  • rustfmt --check isolated per file: zero drift vs main (all three files 0→0)

Closes #1376. Refs #1375 (umbrella), #1373 (L1 warning integration).

First platform slice of the config-first parity umbrella (#1375):
[line] now carries channel_secret, channel_access_token, and
webhook_path alongside the trust fields, resolving each field
config → LINE_* env → default (config always wins; empty-string ${}
expansion falls through to env). Mirrors the TelegramConfig pattern.

- core: LineConfig/ResolvedLine extended; resolve() per-field
- gateway: GatewayLineConfig bridge + AppState::apply_line_config
  (crate-boundary pattern of apply_telegram_config); new
  line_webhook_path state field; standalone gateway honors
  LINE_WEBHOOK_PATH and mounts at the resolved path
- unified: applies [line] before warn_unenforceable_l1, so a
  config-supplied channel_secret satisfies the #1373 L1 startup check
  (test pins this); line route mounts at the resolved path
- docs: config.toml.example, config-reference.md, line.md — [line]
  primary, env fallback

Backward compatible: env-only deployments resolve identically; the
default mount path is unchanged.

Closes #1376. Refs #1375, #1373.
@chaodu-agent chaodu-agent requested a review from thepagent as a code owner July 12, 2026 20:45
@chaodu-agent

This comment has been minimized.

The closing code fence had prose on the same line, so the block never
closed and the following paragraph rendered inside it.
@chaodu-agent

Copy link
Copy Markdown
Collaborator Author

Note

LGTM ✅ — Clean config-first parity slice; mirrors TelegramConfig pattern end-to-end with proper test coverage.

What This PR Does

LINE channel credentials (channel_secret, channel_access_token) and webhook_path were env-only, violating the config-first invariant that every platform setting resolves [section] config → LINE_* env → default. This PR promotes them to first-class [line] TOML fields, completing config-first parity for the LINE adapter (#1376).

How It Works

Mirrors the TelegramConfig reference pattern exactly:

  1. core config.rs: LineConfig gains three new Option<String> fields; resolve() applies the config → env → default chain with empty-string filtering for ${} expansion of unset vars.
  2. gateway lib.rs: GatewayLineConfig bridge struct + apply_line_config() method keeps the crate free of openab-core; line_webhook_path added to AppState; standalone serve() reads LINE_WEBHOOK_PATH env.
  3. unified main.rs: Applies [line] config before warn_unenforceable_l1, so config-supplied channel_secret satisfies the L1 startup check; route mounts at the resolved path.
  4. docs: config.toml.example, config-reference.md, and line.md all updated.

Findings

# Severity Finding Location
1 🟢 Correct ordering: apply_line_config before warn_unenforceable_l1 prevents false L1 flagging src/main.rs:877
2 🟢 Empty-string filter for ${} expansion is identical to Telegram's pattern — correct fallthrough config.rs:828-846
3 🟢 Tests consolidated in single function to avoid env-var race conditions (cargo parallel test model) config.rs
4 🟢 New apply_line_config_overrides_env_state_and_feeds_l1_warning test directly pins the L1 integration contract lib.rs:l1_audit_tests
5 🟢 Standalone gateway also handles LINE_WEBHOOK_PATH with correct #[cfg] gating for non-line builds lib.rs:serve()
What's Good (🟢)
  • Pattern fidelity: Every change mirrors the existing TelegramConfig implementation 1:1 — struct shape, resolve logic, empty-string semantics, gateway bridge pattern, unified integration point. Minimal cognitive overhead for future reviewers.
  • Backward compatibility: Env-only deployments resolve identically since None config fields fall through to env vars. No breaking changes.
  • Test coverage: Three distinct scenarios (config-wins, empty-expansion-fallthrough, nothing-set-defaults) plus the gateway L1 integration test. Comprehensive for the feature scope.
  • Doc consistency: All three documentation surfaces updated simultaneously — example config, reference table, and platform guide.
Baseline Check
  • PR opened: 2025-07-13 by chaodu-agent on branch feat/1376-line-config-first
  • Main already has: LineConfig with allow_all_users + allowed_users (L3 trust fields only); credentials are env-only (LINE_CHANNEL_SECRET, LINE_CHANNEL_ACCESS_TOKEN); webhook path hardcoded to /webhook/line
  • Net-new value: Promotes credentials and webhook path to config-first fields with the standard resolution chain, completing the config-first invariant for LINE

5️⃣ Three Reasons We Might Not Need This PR

  1. Credentials in TOML is a security surface — Putting secrets in config.toml (even with ${} expansion) means the file itself becomes sensitive. Counter: the same pattern already ships for Telegram, and ${} expansion means operators can still reference env vars without literals.

  2. Webhook path flexibility is rarely needed — Most deployments use the default /webhook/line and never change it. Counter: the unified config-first invariant requires all platform fields to be configurable; parity itself is the value, and the cost is near-zero.

  3. Standalone gateway doesn't benefit from TOML config — The serve() path only reads env vars, not TOML. Counter: this is the existing architectural contract (config parsing lives in unified mode); standalone operators use env vars by design and this PR correctly adds LINE_WEBHOOK_PATH env support there too.

@thepagent thepagent merged commit fb46840 into main Jul 13, 2026
38 checks passed
thepagent pushed a commit that referenced this pull request Jul 13, 2026
…irst (#1382)

* feat(wecom): full [wecom] section — credentials + connection config-first

Second platform slice of the config-first parity umbrella (#1375):
[wecom] graduates from the shared PlatformTrustConfig to a dedicated
WecomConfig carrying corp_id, secret, token, encoding_aes_key,
agent_id, webhook_path, streaming_enabled, debounce_secs alongside the
trust fields. Each field resolves config → WECOM_* env → default.

- core: WecomConfig/ResolvedWecom + resolve(); trust_config() view
  keeps the registry override path unchanged
- gateway: GatewayWecomConfig bridge + AppState::apply_wecom_config;
  the apply rebuilds the adapter through the same from_reader
  validation as env-only construction (five mandatory credentials,
  numeric agent_id, 43-char AES key) — incomplete section resolves to
  no adapter, matching env-only semantics
- unified: applies [wecom] before warn_unenforceable_l1
- docs: config.toml.example, config-reference.md ([wecom] gets its own
  full section; combined section shrinks to googlechat/teams), wecom.md

Backward compatible: env-only deployments construct identically;
existing trust-only [wecom] sections keep parsing (all fields optional).

Closes #1378. Refs #1375. Stacked on #1381.

* docs(config): drop graduated [wecom] from PlatformTrustConfig doc (review F1)

---------

Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
chaodu-agent added a commit that referenced this pull request Jul 13, 2026
…refs

The design shipped across #1356/#1375 (#1297, #1365/#1366 trust slices;
#1381-#1385 full config-first parity). Corrections vs implementation:
googlechat field names (sa_key_json + audience), Teams sender-id is the
Bot Framework activity.from.id (not AAD Object ID). The [gateway]
section deprecation warning is deferred to Phase 1c (the WS two-process
model still uses [gateway] legitimately) — tracked on #1356.

Merging also fixes the dangling first-class-platform-config.md link in
the already-merged identity-trust-none ADR.
thepagent pushed a commit that referenced this pull request Jul 13, 2026
* docs(adr): first-class per-platform config & trust-none default

Propose promoting all gateway-connected platforms (Telegram, LINE,
Feishu, WeCom, Google Chat, MS Teams) to top-level config sections,
matching the existing [discord] and [slack] structure.

Key decisions:
- Per-platform [telegram], [line], [feishu], etc. sections
- Trust-none default (empty allowed_users = deny all)
- Single trust gate at AdapterRouter::handle_message()
- Echo sender ID on deny
- Deprecate [gateway] catch-all section

Tracking: #1262

* docs(adr): add trust pyramid with platform auth comparison table

Layer 1 (gateway): Platform authentication mechanisms per adapter
- Telegram: secret token + IP range
- LINE: HMAC-SHA256
- Feishu: SHA256 signature + encrypt key
- WeCom: token signature + AES decrypt
- Google Chat: JWT (RS256 via JWKS)
- MS Teams: JWT (OpenID Connect)
- Slack/Discord: WebSocket token auth

Layer 2 (core): Channel/group trust (existing)
Layer 3 (core): User trust (this ADR - flip to deny-all)

* docs(adr): refine trust pyramid — L2 scope (open default) vs L3 identity (deny default)

Clarify the three layers per review discussion:
- L1 platform auth (security, edge)
- L2 channel/group/DM scope control — NOT security, default OPEN; the
  platform already enforces channel membership, so L2 is operator scoping
- L3 identity trust — THE security gate, default DENY-ALL, covers all paths
- allow_dm is an L2 surface toggle; DMs have no platform membership gate so
  L3 is their sole protection
- L2 must stay open by default for the echo-UID request-access flow to work

* docs(adr): specify trait & type changes — extend carriers, no new trait

- Extend TrustConfig with L2 scope fields (allow_all_channels, allow_dm)
  + surface_allowed(); defaults L2-open / L3-deny
- Add 'Trait & Type Changes' section: pass SenderContext in MessageContext,
  add is_dm to ChannelRef, no new ChatAdapter method/trait (uniform logic)
- Note the real refactor = remove scattered trust checks from
  discord.rs/slack.rs/gateway.rs so the router gate is un-bypassable
- Fix architecture diagram gate labels (L2 optional/open, L3 deny default)

* docs(adr): sharpen title to 'identity trust-none default'

* docs(adr): scope down to per-platform config only

Split the trust/security decision out into a separate ADR
(docs/adr/identity-trust-none.md, PR #1264). This ADR now covers only
the config schema change: first-class [platform] sections + [gateway]
deprecation + migration.

* docs(adr): mark first-class platform config as Accepted with shipped refs

The design shipped across #1356/#1375 (#1297, #1365/#1366 trust slices;
#1381-#1385 full config-first parity). Corrections vs implementation:
googlechat field names (sa_key_json + audience), Teams sender-id is the
Bot Framework activity.from.id (not AAD Object ID). The [gateway]
section deprecation warning is deferred to Phase 1c (the WS two-process
model still uses [gateway] legitimately) — tracked on #1356.

Merging also fixes the dangling first-class-platform-config.md link in
the already-merged identity-trust-none ADR.

---------

Co-authored-by: 超渡法師 <chaodu-agent@openab.dev>
Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(line): full [line] section — credentials + connection config-first

2 participants