mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
d8802e6cc2
* fix(wasm): gate websocket runtime on auth and stop reconnect loop on fatal closes (#2557)
WASM channel runtimes spawned the websocket runtime on `connect_on_start` without
verifying required credentials, and treated auth-rejected close frames as
transient disconnects. When a Discord bot token was missing the runtime would
connect, be rejected with 4003, and the outer reconnect loop retried forever
(capped at 64s backoff) producing continuous network traffic and log spam.
Three guards, all in `src/channels/wasm/wrapper.rs`:
- `websocket_auth_preflight` + `websocket_start_decision` compose capability
parsing and secret-presence into a typed decision; `Channel::start` matches
on it and skips the spawn with a bounded `warn!` when the declared
`identify_secret_name` is not present in the secrets store. Writing the
secret and restarting the channel connects normally.
- Runtime-entry guard: if `resolve_websocket_identify_message` returns `None`
after preflight passed (credential revoked mid-flight), the task exits
instead of connecting.
- `classify_websocket_close_code` maps Discord-documented fatal auth codes
(4003, 4004, 4010-4014) to `Terminal`; the read loop breaks `'reconnect`
on terminal closes instead of falling through to backoff.
Regression tests cover preflight, start-decision, and close-code classification.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* review: address bot feedback on #2557 websocket preflight
Four distinct issues flagged by gemini-code-assist + copilot-pr-reviewer:
1. Close-code classification was global — 4000-series codes are
application-defined (RFC 6455), so another provider using e.g. 4003 with
different semantics would be incorrectly treated as terminal. Scope the
Discord code table to Discord gateway hosts via `is_discord_gateway_host`;
non-Discord URLs always return `Reconnect`.
2. Preflight silently allowed a capability that declares
`identify_secret_name` but omits the `identify` template (no identify
payload could ever be built even with a valid secret). Catch it in
`websocket_start_decision` with a new `MalformedConfig { reason }`
variant so the operator log points at the real cause.
3. `store.exists()` errors were mapped to `MissingSecret`, conflating a
transient DB blip with a genuinely absent secret. Split the `Err`
branch: log the store error and fail open (`Ready`); the runtime-entry
guard still stops any spawn whose identify payload cannot be built.
4. Runtime-entry guard warning said "required auth secret unavailable"
even when the true cause could be a missing `identify` template or a
decrypt error. Rephrased to enumerate possible causes and include
`has_identify_template` as a field.
Tests:
- `test_websocket_start_decision_malformed_config_without_identify_template`
- `test_classify_websocket_close_code_non_discord_host_never_terminal`
(covers plain non-Discord hosts and a host-suffix spoof)
- existing close-code tests now pass the URL so the Discord-gated signature
is exercised.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(wasm): use CredentialName newtype in websocket auth preflight
Per .claude/rules/types.md, names that flow between modules and gate side
effects should be typed identifiers, not `String`. The new enums from the
#2557 fix were carrying `secret_name: String` because they inherited from
`WebsocketRuntimeConfig::identify_secret_name: Option<String>` (the raw
string parsed from capability JSON). That field is a valid boundary value,
but everything below it in the auth-flow path should be typed.
- `WebsocketAuthPreflight::MissingSecret { secret_name: String }` →
`MissingCredential { credential_name: CredentialName }`
- `WebsocketStartDecision::MissingAuth { secret_name: String }` →
`MissingAuth { credential_name: CredentialName }`
- `websocket_auth_preflight` now takes `Option<&CredentialName>`; name
validation is lifted to the orchestrator where it belongs.
- `websocket_start_decision` validates `config.identify_secret_name` via
`CredentialName::new(...)` once, at the boundary. A syntactically
invalid name surfaces as `MalformedConfig { reason }` with the real
`IdentityError` embedded — not silently routed through "missing
credential", which was the exact failure mode bug #2574 fixed in a
different subsystem.
Internal variables and log field names move from `secret_name` →
`credential_name` to match the codebase convention around `CredentialName`
and the existing `credential_name` fields in `bridge::auth_manager`,
`bridge::router`, and `gate::mod`.
Out of scope (larger follow-ups):
- Capability-wire key `identify_secret_name` stays; it lives in every
installed channel's `capabilities.json` (`channels-src/discord/...`) and
renaming it is a breaking change to deployed WASM channels.
- `SecretsStore::{exists, get_decrypted}(&str, &str)` stays; migrating the
trait signature is a wider refactor that should not ride on this fix.
- `WebsocketRuntimeConfig::identify_secret_name: Option<String>` stays as
the boundary type that owns the raw-string-from-JSON contract.
Added regression test
`test_websocket_start_decision_malformed_config_with_invalid_credential_name`
to pin the new "invalid name → MalformedConfig" behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* review: canonicalize identify_secret_name + flag asymmetric identify config
Two correctness issues flagged by copilot-pr-reviewer on c4b9d1d8.
1. **Canonicalization mismatch between preflight and runtime.**
`CredentialName::new()` canonicalizes — it trims whitespace and folds
`-` → `_`. Preflight checks existence via `credential_name.as_str()`
(canonical), but `resolve_websocket_identify_message` later reads the
raw `config.identify_secret_name` and passes that to the store. A
capability declaring `"github-token"` against a store holding
`"github_token"` would therefore pass preflight and fail in the spawn.
Write the canonicalized form back to `config.identify_secret_name`
before returning `Spawn(config)` so every downstream lookup uses the
same string. Regression test
`test_websocket_start_decision_spawn_canonicalizes_secret_name`.
2. **Asymmetric identify config passed through as Spawn.**
The existing check caught `identify_secret_name` without `identify`,
but the reverse (`identify` template present, `identify_secret_name`
missing) slipped through. In that shape `resolve_websocket_identify_message`
returns `None` because it needs a secret name, no Identify is ever
sent, and the peer closes the connection — the exact #2557 spin this
PR exists to prevent. Replaced the one-sided check with a
`match (identify, credential_name)` so both asymmetric shapes surface
as `MalformedConfig` with distinct reasons. Regression test
`test_websocket_start_decision_malformed_config_without_secret_name`.
No changes to wire contract, capability JSON key, or secrets-store trait.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>