diff --git a/docs/gateway/security/audit-checks.md b/docs/gateway/security/audit-checks.md index 5a7ac57a35cd..30f1d640452d 100644 --- a/docs/gateway/security/audit-checks.md +++ b/docs/gateway/security/audit-checks.md @@ -21,130 +21,138 @@ either level depending on config (for example, whether the Gateway is remotely exposed). High-signal values you will most likely see in real deployments (not exhaustive): -| `checkId` | Severity | Why it matters | Primary fix key/path | Auto-fix | -| --------------------------------------------------------------- | ------------------ | --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | -------- | -| `fs.state_dir.perms_world_writable` | critical | Other users/processes can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes | -| `fs.state_dir.perms_group_writable` | warn | Group users can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes | -| `fs.state_dir.perms_readable` | warn | State dir is readable by others | filesystem perms on `~/.openclaw` | yes | -| `fs.state_dir.symlink` | warn | State dir target becomes another trust boundary | state dir filesystem layout | no | -| `fs.config.perms_writable` | critical | Others can change auth/tool policy/config | filesystem perms on `~/.openclaw/openclaw.json` | yes | -| `fs.config.symlink` | warn | Symlinked config files are unsupported for writes and add another trust boundary | replace with a regular config file or point `OPENCLAW_CONFIG_PATH` at the real file | no | -| `fs.config.perms_group_readable` | warn | Group users can read config tokens/settings | filesystem perms on config file | yes | -| `fs.config.perms_world_readable` | critical | Config can expose tokens/settings | filesystem perms on config file | yes | -| `fs.config_include.perms_writable` | critical | Config include file can be modified by others | include-file perms referenced from `openclaw.json` | yes | -| `fs.config_include.perms_group_readable` | warn | Group users can read included secrets/settings | include-file perms referenced from `openclaw.json` | yes | -| `fs.config_include.perms_world_readable` | critical | Included secrets/settings are world-readable | include-file perms referenced from `openclaw.json` | yes | -| `fs.auth_profiles.perms_writable` | critical | Others can inject or replace stored model credentials | Agent-local SQLite auth database/sidecar perms and legacy `auth-profiles.json` perms | yes | -| `fs.auth_profiles.perms_readable` | warn | Others can read API keys and OAuth tokens | Agent-local SQLite auth database/sidecar perms and legacy `auth-profiles.json` perms | yes | -| `fs.credentials_dir.perms_writable` | critical | Others can modify channel pairing/credential state | filesystem perms on `~/.openclaw/credentials` | yes | -| `fs.credentials_dir.perms_readable` | warn | Others can read channel credential state | filesystem perms on `~/.openclaw/credentials` | yes | -| `fs.sessions_store.perms_readable` | warn | Others can read session transcripts/metadata | session store perms | yes | -| `fs.log_file.perms_readable` | warn | Others can read redacted-but-still-sensitive logs | gateway log file perms | yes | -| `fs.synced_dir` | warn | State/config in iCloud/Dropbox/Drive broadens token/transcript exposure | move config/state off synced folders | no | -| `gateway.bind_no_auth` | critical | Remote bind without shared secret | `gateway.bind`, `gateway.auth.*` | no | -| `gateway.loopback_no_auth` | critical | Reverse-proxied loopback may become unauthenticated | `gateway.auth.*`, proxy setup | no | -| `gateway.trusted_proxies_missing` | warn | Reverse-proxy headers are present but not trusted | `gateway.trustedProxies` | no | -| `gateway.http.no_auth` | warn/critical | Gateway HTTP APIs reachable with `auth.mode="none"` | `gateway.auth.mode`, `gateway.http.endpoints.*`, `plugins.entries.admin-http-rpc` | no | -| `gateway.http.session_key_override_enabled` | info | HTTP API callers can override `sessionKey` | `gateway.http.allowSessionKeyOverride` | no | -| `gateway.tools_invoke_http.dangerous_allow` | warn/critical | Re-enables dangerous tools over HTTP API for owner/admin callers | `gateway.tools.allow` | no | -| `gateway.nodes.allow_commands_dangerous` | warn/critical | Enables high-impact node commands (desktop input/camera/screen/contacts/calendar/SMS) | `gateway.nodes.commands.allow` | no | -| `gateway.nodes.deny_commands_ineffective` | warn | Pattern-like deny entries do not match shell text or groups | `gateway.nodes.commands.deny` | no | -| `gateway.tailscale_funnel` | critical | Public internet exposure | `gateway.tailscale.mode` | no | -| `gateway.tailscale_serve` | info | Tailnet exposure is enabled via Serve | `gateway.tailscale.mode` | no | -| `gateway.control_ui.allowed_origins_required` | critical | Non-loopback Control UI without explicit browser-origin allowlist | `gateway.controlUi.allowedOrigins` | no | -| `gateway.control_ui.allowed_origins_wildcard` | warn/critical | `allowedOrigins=["*"]` disables browser-origin allowlisting | `gateway.controlUi.allowedOrigins` | no | -| `gateway.control_ui.host_header_origin_fallback` | warn/critical | Enables Host-header origin fallback (DNS rebinding hardening downgrade) | `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` | no | -| `gateway.real_ip_fallback_enabled` | warn/critical | Trusting `X-Real-IP` fallback can enable source-IP spoofing via proxy misconfig | `gateway.allowRealIpFallback`, `gateway.trustedProxies` | no | -| `gateway.token_too_short` | warn | Short shared token is easier to brute force | `gateway.auth.token` | no | -| `gateway.auth_no_rate_limit` | warn | Exposed auth without rate limiting increases brute-force risk | `gateway.auth.rateLimit` | no | -| `gateway.trusted_proxy_auth` | critical | Proxy identity now becomes the auth boundary | `gateway.auth.mode="trusted-proxy"` | no | -| `gateway.trusted_proxy_no_proxies` | critical | Trusted-proxy auth without trusted proxy IPs is unsafe | `gateway.trustedProxies` | no | -| `gateway.trusted_proxy_no_user_header` | critical | Trusted-proxy auth cannot resolve user identity safely | `gateway.auth.trustedProxy.userHeader` | no | -| `gateway.trusted_proxy_no_allowlist` | warn | Trusted-proxy auth accepts any authenticated upstream user | `gateway.auth.trustedProxy.allowUsers` | no | -| `gateway.trusted_proxy_allow_loopback` | warn | Trusted-proxy auth accepts explicitly allowed loopback proxy sources | `gateway.auth.trustedProxy.allowLoopback` | no | -| `gateway.probe_auth_secretref_unavailable` | warn | Deep probe could not resolve auth SecretRefs in this command path | deep-probe auth source / SecretRef availability | no | -| `gateway.probe_failed` | warn | Live Gateway probe failed (`--deep` only) | gateway reachability/auth | no | -| `discovery.mdns_full_mode` | warn/critical | mDNS full mode advertises `cliPath`/`sshPort` metadata on local network | `discovery.mdns.mode`, `gateway.bind` | no | -| `config.insecure_or_dangerous_flags` | warn | One insecure/dangerous debug flag is enabled | key named in finding detail | no | -| `security.audit.suppressions.active` | info | Audit output has configured suppressions and may be filtered | `security.audit.suppressions` | no | -| `config.secrets.gateway_password_in_config` | warn | Gateway password is stored directly in config | `gateway.auth.password` | no | -| `config.secrets.hooks_token_in_config` | warn | Hook bearer token is stored directly in config | `hooks.token` | no | -| `hooks.token_reuse_gateway_token` | critical | Hook ingress token also unlocks Gateway auth | `hooks.token`, `gateway.auth.token`, `gateway.auth.password` | no | -| `hooks.token_too_short` | warn | Easier brute force on hook ingress | `hooks.token` | no | -| `hooks.default_session_key_unset` | warn | Hook agent runs fan out into generated per-request sessions | `hooks.defaultSessionKey` | no | -| `hooks.allowed_agent_ids_unrestricted` | warn/critical | Authenticated hook callers may route to any configured agent | `hooks.allowedAgentIds` | no | -| `hooks.request_session_key_enabled` | warn/critical | External caller can choose sessionKey | `hooks.allowRequestSessionKey` | no | -| `hooks.request_session_key_prefixes_missing` | warn/critical | No bound on external session key shapes | `hooks.allowedSessionKeyPrefixes` | no | -| `hooks.path_root` | critical | Hook path is `/`, making ingress easier to collide or misroute | `hooks.path` | no | -| `hooks.installs_unpinned_npm_specs` | warn | Hook install records are not pinned to immutable npm specs | hook install metadata | no | -| `hooks.installs_missing_integrity` | warn | Hook install records lack integrity metadata | hook install metadata | no | -| `hooks.installs_version_drift` | warn | Hook install records drift from installed packages | hook install metadata | no | -| `browser.control_invalid_config` | warn | Browser control config is invalid before runtime | `browser.*` | no | -| `browser.control_no_auth` | critical | Browser control exposed without token/password auth | `gateway.auth.*` | no | -| `browser.remote_cdp_http` | warn | Remote CDP over plain HTTP lacks transport encryption | browser profile `cdpUrl` | no | -| `browser.remote_cdp_private_host` | warn | Remote CDP targets a private/internal host | browser profile `cdpUrl`, `browser.ssrfPolicy.*` | no | -| `browser.extension_relay_legacy_auth` | warn | Legacy extension relay bearer/Basic/token authentication remains enabled | `browser.extensionRelay.allowLegacyAuth` | no | -| `sandbox.docker_config_mode_off` | warn | Sandbox Docker config present but inactive | `agents.*.sandbox.mode` | no | -| `sandbox.bind_mount_non_absolute` | warn | Relative bind mounts can resolve unpredictably | `agents.*.sandbox.docker.binds[]` | no | -| `sandbox.dangerous_bind_mount` | critical | Sandbox bind mount targets blocked system, credential, or Docker socket paths | `agents.*.sandbox.docker.binds[]` | no | -| `sandbox.dangerous_network_mode` | critical | Sandbox Docker network uses `host` or `container:*` namespace-join mode | `agents.*.sandbox.docker.network` | no | -| `sandbox.dangerous_seccomp_profile` | critical | Sandbox seccomp profile weakens container isolation | `agents.*.sandbox.docker.securityOpt` | no | -| `sandbox.dangerous_apparmor_profile` | critical | Sandbox AppArmor profile weakens container isolation | `agents.*.sandbox.docker.securityOpt` | no | -| `sandbox.browser_cdp_bridge_unrestricted` | warn | Sandbox browser bridge is exposed without source-range restriction | `sandbox.browser.cdpSourceRange` | no | -| `sandbox.browser_container.non_loopback_publish` | critical | Existing browser container publishes CDP on non-loopback interfaces | browser sandbox container publish config | no | -| `sandbox.browser_container.hash_label_missing` | warn | Existing browser container predates current config-hash labels | `openclaw sandbox recreate --browser --all` | no | -| `sandbox.browser_container.hash_epoch_stale` | warn | Existing browser container predates current browser config epoch | `openclaw sandbox recreate --browser --all` | no | -| `sandbox.browser_container.docker_probe_timeout` | warn | Docker label probe for the browser container timed out | Docker daemon reachability | no | -| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` fails closed when sandbox is off | `tools.exec.host`, `agents.defaults.sandbox.mode` | no | -| `tools.exec.host_sandbox_no_sandbox_agents` | warn | Per-agent `exec host=sandbox` fails closed when sandbox is off | `agents.entries.*.tools.exec.host`, `agents.entries.*.sandbox.mode` | no | -| `tools.exec.security_full_configured` | warn/critical | Host exec is running with `security="full"` | `tools.exec.security`, `agents.entries.*.tools.exec.security` | no | -| `tools.exec.agent_skill_mcp_boundary_drift` | warn | Agent skill allowlists are present while host exec can reach MCP clients/registries | `agents.entries.*.tools.exec.*`, sandbox/OS isolation, MCP server credentials | no | -| `tools.exec.fs_tools_disabled_but_exec_enabled` | warn | Filesystem tool policy does not make shell execution read-only | `tools.deny`, `agents.entries.*.tools.deny`, `agents.*.sandbox.workspaceAccess` | no | -| `tools.exec.auto_allow_skills_enabled` | warn | Exec approvals trust skill bins implicitly | host approvals file | no | -| `tools.exec.allowlist_interpreter_without_strict_inline_eval` | warn | Interpreter allowlists permit inline eval without forced reapproval | `tools.exec.strictInlineEval`, `agents.entries.*.tools.exec.strictInlineEval`, exec approvals allowlist | no | -| `tools.exec.safe_bins_interpreter_unprofiled` | warn | Interpreter/runtime bins in `safeBins` without explicit profiles broaden exec risk | `tools.exec.safeBins`, `tools.exec.safeBinProfiles`, `agents.entries.*.tools.exec.*` | no | -| `tools.exec.safe_bins_broad_behavior` | warn | Broad-behavior tools in `safeBins` weaken the low-risk stdin-filter trust model | `tools.exec.safeBins`, `agents.entries.*.tools.exec.safeBins` | no | -| `tools.exec.safe_bin_trusted_dirs_risky` | warn | `safeBinTrustedDirs` includes mutable or risky directories | `tools.exec.safeBinTrustedDirs`, `agents.entries.*.tools.exec.safeBinTrustedDirs` | no | -| `tools.elevated.allowFrom..wildcard` | critical | `tools.elevated.allowFrom.` includes `"*"`, approving every sender | `tools.elevated.allowFrom.` | no | -| `tools.elevated.allowFrom..large` | warn | Elevated allowlist for `` has more than 25 entries | `tools.elevated.allowFrom.` | no | -| `skills.workspace.symlink_escape` | warn | Workspace `skills/**/SKILL.md` resolves outside workspace root (symlink-chain drift) | workspace `skills/**` filesystem state | no | -| `skills.workspace.scan_truncated` | warn | Workspace skill scan hit its directory-visit cap before finishing | flatten/simplify the workspace `skills/` directory tree | no | -| `plugins.extensions_no_allowlist` | warn | Plugins are installed without an explicit plugin allowlist | `plugins.allowlist` | no | -| `plugins.allow_phantom_entries` | warn | `plugins.allow` lists an ID with no matching installed plugin | `plugins.allow` | no | -| `plugins.installs_unpinned_npm_specs` | warn | Plugin index records are not pinned to immutable npm specs | plugin install metadata | no | -| `plugins.installs_missing_integrity` | warn | Plugin index records lack integrity metadata | plugin install metadata | no | -| `plugins.installs_version_drift` | warn | Plugin index records drift from installed packages | plugin install metadata | no | -| `plugins.code_safety` | warn/critical | Plugin code scan found suspicious or dangerous patterns (`--deep` only) | plugin code / install source | no | -| `plugins.code_safety.entry_path` | warn | Plugin entry path points into hidden or `node_modules` locations | plugin manifest `entry` | no | -| `plugins.code_safety.entry_escape` | critical | Plugin entry escapes the plugin directory | plugin manifest `entry` | no | -| `plugins.code_safety.manifest_parse_error` | warn | Plugin manifest could not be parsed during the code-safety scan | plugin manifest file | no | -| `plugins.code_safety.scan_failed` | warn | Plugin code scan could not complete (`--deep` only) | plugin path / scan environment | no | -| `plugins..security_audit_failed` | warn | A plugin-owned security audit collector threw an error | that plugin's security-audit collector | no | -| `skills.code_safety` | warn/critical | Skill installer metadata/code contains suspicious or dangerous patterns (`--deep` only) | skill install source | no | -| `skills.code_safety.scan_failed` | warn | Skill code scan could not complete (`--deep` only) | skill scan environment | no | -| `channels.discord.allowlisted_groups.broad_members` | warn | Allowlisted Discord guild/channel targets have no member or role restriction | `channels.discord.guilds.*.users/roles`, per-channel `users/roles` | no | -| `security.exposure.open_channels_with_exec` | warn/critical | Shared/public rooms can reach exec-enabled agents | `channels.*.dmPolicy`, `channels.*.groupPolicy`, `tools.exec.*`, `agents.entries.*.tools.exec.*` | no | -| `security.exposure.open_groups_with_elevated` | critical | Open DMs/groups + elevated tools create high-impact prompt-injection paths | top-level or nested DM policy paths, account overrides, `channels.*.groupPolicy` | no | -| `security.exposure.open_groups_with_runtime_or_fs` | critical/warn | Open DMs/groups can reach command/file tools without sandbox/workspace guards | DM/group policy paths, `tools.profile/deny`, `tools.fs.workspaceOnly`, `agents.*.sandbox.mode` | no | -| `security.exposure.open_groups_with_control_plane_tools` | critical | Open DMs/groups can reach gateway/cron control-plane tools | DM/group policy paths, `tools.allow`, `tools.alsoAllow`, `tools.profile`, `gateway`, `cron` | no | -| `security.trust_model.multi_user_heuristic` | warn | Config looks multi-user while gateway trust model is personal-assistant | split trust boundaries, or shared-user hardening (`sandbox.mode`, tool deny/workspace scoping) | no | -| `tools.profile_minimal_overridden` | warn | Agent overrides bypass global minimal profile | `agents.entries.*.tools.profile` | no | -| `plugins.tools_reachable_permissive_policy` | warn | Extension tools reachable in permissive contexts | `tools.profile` + tool allow/deny | no | -| `models.legacy` | warn | Legacy model families are still configured | model selection | no | -| `models.weak_tier` | warn | Configured models are below current recommended tiers | model selection | no | -| `models.small_params` | critical/info | Small models + unsafe tool surfaces raise injection risk | model choice + sandbox/tool policy | no | -| `channels..dm.open` | critical | `` DM policy is `"open"`; anyone can DM the bot | `channels..dmPolicy`, `.allowFrom` | no | -| `channels..dm.open_invalid` | warn | `dmPolicy="open"` without `"*"` in `allowFrom` is inconsistent | `channels..allowFrom` | no | -| `channels..dm.scope_main_multiuser` | warn | Multiple DM senders currently share the main session | `session.dmScope` | no | -| `channels..allowFrom.dangerous_name_matching_enabled` | info | `dangerouslyAllowNameMatching` re-enables mutable name/email/tag sender matching | disable `dangerouslyAllowNameMatching`, use stable sender IDs | no | -| `channels..account.read_only_resolution` | warn | A channel account could not be fully resolved for audit (missing secret/gateway) | ensure referenced secrets are resolvable, or run against a live gateway snapshot | no | -| `channels..warning.` | info/warn/critical | Provider-specific security warning, classified from free-form plugin text | see finding detail | no | -| `summary.attack_surface` | info | Roll-up summary of auth, channel, tool, and exposure posture | multiple keys (see finding detail) | no | +| `checkId` | Severity | Why it matters | Primary fix key/path | Auto-fix | +| --------------------------------------------------------------- | ------------------ | ---------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | -------- | +| `fs.state_dir.perms_world_writable` | critical | Other users/processes can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes | +| `fs.state_dir.perms_group_writable` | warn | Group users can modify full OpenClaw state | filesystem perms on `~/.openclaw` | yes | +| `fs.state_dir.perms_readable` | warn | State dir is readable by others | filesystem perms on `~/.openclaw` | yes | +| `fs.state_dir.symlink` | warn | State dir target becomes another trust boundary | state dir filesystem layout | no | +| `fs.config.perms_writable` | critical | Others can change auth/tool policy/config | filesystem perms on `~/.openclaw/openclaw.json` | yes | +| `fs.config.symlink` | warn | Symlinked config files are unsupported for writes and add another trust boundary | replace with a regular config file or point `OPENCLAW_CONFIG_PATH` at the real file | no | +| `fs.config.perms_group_readable` | warn | Group users can read config tokens/settings | filesystem perms on config file | yes | +| `fs.config.perms_world_readable` | critical | Config can expose tokens/settings | filesystem perms on config file | yes | +| `fs.config_include.perms_writable` | critical | Config include file can be modified by others | include-file perms referenced from `openclaw.json` | yes | +| `fs.config_include.perms_group_readable` | warn | Group users can read included secrets/settings | include-file perms referenced from `openclaw.json` | yes | +| `fs.config_include.perms_world_readable` | critical | Included secrets/settings are world-readable | include-file perms referenced from `openclaw.json` | yes | +| `fs.auth_profiles.perms_writable` | critical | Others can inject or replace stored model credentials | Agent-local SQLite auth database/sidecar perms and legacy `auth-profiles.json` perms | yes | +| `fs.auth_profiles.perms_readable` | warn | Others can read API keys and OAuth tokens | Agent-local SQLite auth database/sidecar perms and legacy `auth-profiles.json` perms | yes | +| `fs.credentials_dir.perms_writable` | critical | Others can modify channel pairing/credential state | filesystem perms on `~/.openclaw/credentials` | yes | +| `fs.credentials_dir.perms_readable` | warn | Others can read channel credential state | filesystem perms on `~/.openclaw/credentials` | yes | +| `fs.sessions_store.perms_readable` | warn | Others can read session transcripts/metadata | session store perms | yes | +| `fs.log_file.perms_readable` | warn | Others can read redacted-but-still-sensitive logs | gateway log file perms | yes | +| `fs.synced_dir` | warn | State/config in iCloud/Dropbox/Drive broadens token/transcript exposure | move config/state off synced folders | no | +| `gateway.bind_no_auth` | critical | Remote bind without shared secret | `gateway.bind`, `gateway.auth.*` | no | +| `gateway.loopback_no_auth` | critical | Reverse-proxied loopback may become unauthenticated | `gateway.auth.*`, proxy setup | no | +| `gateway.trusted_proxies_missing` | warn | Reverse-proxy headers are present but not trusted | `gateway.trustedProxies` | no | +| `gateway.http.no_auth` | warn/critical | Gateway HTTP APIs reachable with `auth.mode="none"` | `gateway.auth.mode`, `gateway.http.endpoints.*`, `plugins.entries.admin-http-rpc` | no | +| `gateway.http.session_key_override_enabled` | info | HTTP API callers can override `sessionKey` | `gateway.http.allowSessionKeyOverride` | no | +| `gateway.tools_invoke_http.dangerous_allow` | warn/critical | Re-enables dangerous tools over HTTP API for owner/admin callers | `gateway.tools.allow` | no | +| `gateway.nodes.allow_commands_dangerous` | warn/critical | Enables high-impact node commands (desktop input/camera/screen/contacts/calendar/SMS) | `gateway.nodes.commands.allow` | no | +| `gateway.nodes.deny_commands_ineffective` | warn | Pattern-like deny entries do not match shell text or groups | `gateway.nodes.commands.deny` | no | +| `gateway.tailscale_funnel` | critical | Public internet exposure | `gateway.tailscale.mode` | no | +| `gateway.tailscale_serve` | info | Tailnet exposure is enabled via Serve | `gateway.tailscale.mode` | no | +| `gateway.control_ui.allowed_origins_required` | critical | Non-loopback Control UI without explicit browser-origin allowlist | `gateway.controlUi.allowedOrigins` | no | +| `gateway.control_ui.allowed_origins_wildcard` | warn/critical | `allowedOrigins=["*"]` disables browser-origin allowlisting | `gateway.controlUi.allowedOrigins` | no | +| `gateway.control_ui.host_header_origin_fallback` | warn/critical | Enables Host-header origin fallback (DNS rebinding hardening downgrade) | `gateway.controlUi.dangerouslyAllowHostHeaderOriginFallback` | no | +| `gateway.real_ip_fallback_enabled` | warn/critical | Trusting `X-Real-IP` fallback can enable source-IP spoofing via proxy misconfig | `gateway.allowRealIpFallback`, `gateway.trustedProxies` | no | +| `gateway.token_too_short` | warn | Short shared token is easier to brute force | `gateway.auth.token` | no | +| `gateway.auth_no_rate_limit` | warn | Exposed auth without rate limiting increases brute-force risk | `gateway.auth.rateLimit` | no | +| `gateway.trusted_proxy_auth` | critical | Proxy identity now becomes the auth boundary | `gateway.auth.mode="trusted-proxy"` | no | +| `gateway.trusted_proxy_no_proxies` | critical | Trusted-proxy auth without trusted proxy IPs is unsafe | `gateway.trustedProxies` | no | +| `gateway.trusted_proxy_no_user_header` | critical | Trusted-proxy auth cannot resolve user identity safely | `gateway.auth.trustedProxy.userHeader` | no | +| `gateway.trusted_proxy_no_allowlist` | warn | Trusted-proxy auth accepts any authenticated upstream user | `gateway.auth.trustedProxy.allowUsers` | no | +| `gateway.trusted_proxy_allow_loopback` | warn | Trusted-proxy auth accepts explicitly allowed loopback proxy sources | `gateway.auth.trustedProxy.allowLoopback` | no | +| `gateway.probe_auth_secretref_unavailable` | warn | Deep probe could not resolve auth SecretRefs in this command path | deep-probe auth source / SecretRef availability | no | +| `gateway.probe_failed` | warn | Live Gateway probe failed (`--deep` only) | gateway reachability/auth | no | +| `discovery.mdns_full_mode` | warn/critical | mDNS full mode advertises `cliPath`/`sshPort` metadata on local network | `discovery.mdns.mode`, `gateway.bind` | no | +| `config.insecure_or_dangerous_flags` | warn | One insecure/dangerous debug flag is enabled | key named in finding detail | no | +| `security.audit.suppressions.active` | info | Audit output has configured suppressions and may be filtered | `security.audit.suppressions` | no | +| `config.secrets.gateway_password_in_config` | warn | Gateway password is stored directly in config | `gateway.auth.password` | no | +| `config.secrets.hooks_token_in_config` | warn | Hook bearer token is stored directly in config | `hooks.token` | no | +| `hooks.token_reuse_gateway_token` | critical | Hook ingress token also unlocks Gateway auth | `hooks.token`, `gateway.auth.token`, `gateway.auth.password` | no | +| `hooks.token_too_short` | warn | Easier brute force on hook ingress | `hooks.token` | no | +| `hooks.default_session_key_unset` | warn | Hook agent runs fan out into generated per-request sessions | `hooks.defaultSessionKey` | no | +| `hooks.allowed_agent_ids_unrestricted` | warn/critical | Authenticated hook callers may route to any configured agent | `hooks.allowedAgentIds` | no | +| `hooks.request_session_key_enabled` | warn/critical | External caller can choose sessionKey | `hooks.allowRequestSessionKey` | no | +| `hooks.request_session_key_prefixes_missing` | warn/critical | No bound on external session key shapes | `hooks.allowedSessionKeyPrefixes` | no | +| `hooks.path_root` | critical | Hook path is `/`, making ingress easier to collide or misroute | `hooks.path` | no | +| `hooks.installs_unpinned_npm_specs` | warn | Hook install records are not pinned to immutable npm specs | hook install metadata | no | +| `hooks.installs_missing_integrity` | warn | Hook install records lack integrity metadata | hook install metadata | no | +| `hooks.installs_version_drift` | warn | Hook install records drift from installed packages | hook install metadata | no | +| `browser.control_invalid_config` | warn | Browser control config is invalid before runtime | `browser.*` | no | +| `browser.control_no_auth` | critical | Browser control exposed without token/password auth | `gateway.auth.*` | no | +| `browser.remote_cdp_http` | warn | Remote CDP over plain HTTP lacks transport encryption | browser profile `cdpUrl` | no | +| `browser.remote_cdp_private_host` | warn | Remote CDP targets a private/internal host | browser profile `cdpUrl`, `browser.ssrfPolicy.*` | no | +| `browser.extension_relay_legacy_auth` | warn | Legacy extension relay bearer/Basic/token authentication remains enabled | `browser.extensionRelay.allowLegacyAuth` | no | +| `sandbox.docker_config_mode_off` | warn | Sandbox Docker config present but inactive | `agents.*.sandbox.mode` | no | +| `sandbox.bind_mount_non_absolute` | warn | Relative bind mounts can resolve unpredictably | `agents.*.sandbox.docker.binds[]` | no | +| `sandbox.dangerous_bind_mount` | critical | Sandbox bind mount targets blocked system, credential, or Docker socket paths | `agents.*.sandbox.docker.binds[]` | no | +| `sandbox.dangerous_network_mode` | critical | Sandbox Docker network uses `host` or `container:*` namespace-join mode | `agents.*.sandbox.docker.network` | no | +| `sandbox.dangerous_seccomp_profile` | critical | Sandbox seccomp profile weakens container isolation | `agents.*.sandbox.docker.securityOpt` | no | +| `sandbox.dangerous_apparmor_profile` | critical | Sandbox AppArmor profile weakens container isolation | `agents.*.sandbox.docker.securityOpt` | no | +| `sandbox.browser_cdp_bridge_unrestricted` | warn | Sandbox browser bridge is exposed without source-range restriction | `sandbox.browser.cdpSourceRange` | no | +| `sandbox.browser_container.non_loopback_publish` | critical | Existing browser container publishes CDP on non-loopback interfaces | browser sandbox container publish config | no | +| `sandbox.browser_container.hash_label_missing` | warn | Existing browser container predates current config-hash labels | `openclaw sandbox recreate --browser --all` | no | +| `sandbox.browser_container.hash_epoch_stale` | warn | Existing browser container predates current browser config epoch | `openclaw sandbox recreate --browser --all` | no | +| `sandbox.browser_container.docker_probe_timeout` | warn | Docker label probe for the browser container timed out | Docker daemon reachability | no | +| `tools.exec.host_sandbox_no_sandbox_defaults` | warn | `exec host=sandbox` fails closed when sandbox is off | `tools.exec.host`, `agents.defaults.sandbox.mode` | no | +| `tools.exec.host_sandbox_no_sandbox_agents` | warn | Per-agent `exec host=sandbox` fails closed when sandbox is off | `agents.entries.*.tools.exec.host`, `agents.entries.*.sandbox.mode` | no | +| `tools.exec.security_full_configured` | warn/critical | Host exec is running with `security="full"` | `tools.exec.security`, `agents.entries.*.tools.exec.security` | no | +| `tools.exec.agent_skill_mcp_boundary_drift` | warn | Agent skill allowlists are present while host exec can reach MCP clients/registries | `agents.entries.*.tools.exec.*`, sandbox/OS isolation, MCP server credentials | no | +| `tools.exec.fs_tools_disabled_but_exec_enabled` | warn | Filesystem tool policy does not make shell execution read-only | `tools.deny`, `agents.entries.*.tools.deny`, `agents.*.sandbox.workspaceAccess` | no | +| `tools.exec.auto_allow_skills_enabled` | warn | Exec approvals trust skill bins implicitly | host approvals file | no | +| `tools.exec.allowlist_interpreter_without_strict_inline_eval` | warn | Interpreter allowlists permit inline eval without forced reapproval | `tools.exec.strictInlineEval`, `agents.entries.*.tools.exec.strictInlineEval`, exec approvals allowlist | no | +| `tools.exec.safe_bins_interpreter_unprofiled` | warn | Interpreter/runtime bins in `safeBins` without explicit profiles broaden exec risk | `tools.exec.safeBins`, `tools.exec.safeBinProfiles`, `agents.entries.*.tools.exec.*` | no | +| `tools.exec.safe_bins_broad_behavior` | warn | Broad-behavior tools in `safeBins` weaken the low-risk stdin-filter trust model | `tools.exec.safeBins`, `agents.entries.*.tools.exec.safeBins` | no | +| `tools.exec.safe_bin_trusted_dirs_risky` | warn | `safeBinTrustedDirs` includes mutable or risky directories | `tools.exec.safeBinTrustedDirs`, `agents.entries.*.tools.exec.safeBinTrustedDirs` | no | +| `tools.elevated.allowFrom..wildcard` | critical | `tools.elevated.allowFrom.` includes `"*"`, approving every sender | `tools.elevated.allowFrom.` | no | +| `tools.elevated.allowFrom..large` | warn | Elevated allowlist for `` has more than 25 entries | `tools.elevated.allowFrom.` | no | +| `skills.workspace.symlink_escape` | warn | Workspace `skills/**/SKILL.md` resolves outside workspace root (symlink-chain drift) | workspace `skills/**` filesystem state | no | +| `skills.workspace.scan_truncated` | warn | Workspace skill scan hit its directory-visit cap before finishing | flatten/simplify the workspace `skills/` directory tree | no | +| `plugins.extensions_no_allowlist` | warn | Plugins are installed without an explicit plugin allowlist | `plugins.allowlist` | no | +| `plugins.allow_phantom_entries` | warn | `plugins.allow` lists an ID with no matching installed plugin | `plugins.allow` | no | +| `plugins.installs_unpinned_npm_specs` | warn | Plugin index records are not pinned to immutable npm specs | plugin install metadata | no | +| `plugins.installs_missing_integrity` | warn | Plugin index records lack integrity metadata | plugin install metadata | no | +| `plugins.installs_version_drift` | warn | Plugin index records drift from installed packages | plugin install metadata | no | +| `plugins.code_safety` | warn/critical | Plugin code scan found suspicious or dangerous patterns (`--deep` only) | plugin code / install source | no | +| `plugins.code_safety.entry_path` | warn | Plugin entry path points into hidden or `node_modules` locations | plugin manifest `entry` | no | +| `plugins.code_safety.entry_escape` | critical | Plugin entry escapes the plugin directory | plugin manifest `entry` | no | +| `plugins.code_safety.manifest_parse_error` | warn | Plugin manifest could not be parsed during the code-safety scan | plugin manifest file | no | +| `plugins.code_safety.scan_failed` | warn | Plugin code scan could not complete (`--deep` only) | plugin path / scan environment | no | +| `plugins..security_audit_failed` | warn | A plugin-owned security audit collector threw an error | that plugin's security-audit collector | no | +| `skills.code_safety` | warn/critical | Skill installer metadata/code contains suspicious or dangerous patterns (`--deep` only) | skill install source | no | +| `skills.code_safety.scan_failed` | warn | Skill code scan could not complete (`--deep` only) | skill scan environment | no | +| `channels.discord.allowlisted_groups.broad_members` | warn | Allowlisted Discord guild/channel targets have no member or role restriction | `channels.discord.guilds.*.users/roles`, per-channel `users/roles` | no | +| `security.exposure.open_channels_with_exec` | warn/critical | Shared/public rooms can reach exec-enabled agents | `channels.*.dmPolicy`, `channels.*.groupPolicy`, `tools.exec.*`, `agents.entries.*.tools.exec.*` | no | +| `security.exposure.open_groups_with_elevated` | critical | Open DMs/groups + elevated tools create high-impact prompt-injection paths | top-level or nested DM policy paths, account overrides, `channels.*.groupPolicy` | no | +| `security.exposure.open_groups_with_runtime_or_fs` | critical/warn | Open DMs/groups can reach command/file tools without sandbox/workspace guards | DM/group policy paths, `tools.profile/deny`, `tools.fs.workspaceOnly`, `agents.*.sandbox.mode` | no | +| `security.exposure.open_groups_with_control_plane_tools` | critical | Open DMs/groups can reach gateway/cron control-plane tools | DM/group policy paths, `tools.allow`, `tools.alsoAllow`, `tools.profile`, `gateway`, `cron` | no | +| `security.trust_model.multi_user_heuristic` | warn | Config looks multi-user while gateway trust model is personal-assistant | split trust boundaries, or shared-user hardening (`sandbox.mode`, tool deny/workspace scoping) | no | +| `tools.profile_minimal_overridden` | warn | Agent overrides bypass global minimal profile | `agents.entries.*.tools.profile` | no | +| `plugins.tools_reachable_permissive_policy` | warn | Extension tools reachable in permissive contexts | `tools.profile` + tool allow/deny | no | +| `models.legacy` | warn | Legacy model families are still configured | model selection | no | +| `models.weak_tier` | warn | Configured models are below current recommended tiers | model selection | no | +| `models.small_params` | critical/info | Small models + unsafe tool surfaces raise injection risk | model choice + sandbox/tool policy | no | +| `channels..dm.open` | critical | `` DM policy is `"open"`; anyone can DM the bot | `channels..dmPolicy`, `.allowFrom` | no | +| `channels..dm.open_invalid` | warn | `dmPolicy="open"` without `"*"` in `allowFrom` is inconsistent | `channels..allowFrom` | no | +| `channels..dm.scope_main_multiuser` | warn | Multiple DM senders currently share the main session | `session.dmScope` | no | +| `channels..allowFrom.mutable_entries_inert` | warn | Configured entries match only mutable aliases and cannot authorize with name matching disabled | replace affected `allowFrom` entries with stable sender IDs | no | +| `channels..allowFrom.dangerous_name_matching_enabled` | info | Mutable matching is enabled; classifier-aware channels report how many entries depend on it | replace mutable `allowFrom` entries with stable sender IDs, then disable `dangerouslyAllowNameMatching` | no | +| `channels..account.read_only_resolution` | warn | A channel account could not be fully resolved for audit (missing secret/gateway) | ensure referenced secrets are resolvable, or run against a live gateway snapshot | no | +| `channels..warning.` | info/warn/critical | Provider-specific security warning, classified from free-form plugin text | see finding detail | no | +| `summary.attack_surface` | info | Roll-up summary of auth, channel, tool, and exposure posture | multiple keys (see finding detail) | no | `channels..*` and `tools.elevated.allowFrom..*` checkIds are generated per configured channel/provider, so `` is a real channel id (for example `telegram`, `discord`) in actual output, not a literal string. +The mutable-entry checks use configured `allowFrom` entries, excluding `"*"` and +pairing-store approvals, and report counts and config paths without raw values. +Symbolic `accessGroup:` references are not counted as mutable identifiers. +Mutable aliases are changeable or shared labels; exact, stable identifiers can +instead have unproven ownership. Ingress diagnostics distinguish +`mutable_identifier_disabled` from `identifier_authentication_too_weak`. + ## Related - [Security](/gateway/security) diff --git a/docs/plugins/sdk-channel-ingress.md b/docs/plugins/sdk-channel-ingress.md index 714ba0797bbc..ef1ed9cdb1a7 100644 --- a/docs/plugins/sdk-channel-ingress.md +++ b/docs/plugins/sdk-channel-ingress.md @@ -162,6 +162,19 @@ field it wants counted. A field missing from a supplied map is treated as `unverified`, even if its identity descriptor declares a stronger static claim. Channels with static strength omit the map entirely. +Expose `classifyEntryAuthentication: identityEntryAuthenticationClassifier(identity)` +from the security adapter's `resolveDmPolicy` result, importing the helper from +`openclaw/plugin-sdk/channel-ingress-runtime`. It uses the identity descriptor's +entry normalizers and returns the strongest static claim among accepting fields, +or `undefined` when none accepts the entry; wildcard entries are excluded. +The [security audit](/gateway/security/audit-checks) counts configured `allowFrom` +entries that depend only on mutable identifiers: it warns when name matching is +disabled and, when enabled, previews how many entries would stop authorizing +after disabling it. Findings contain counts and config paths, not raw entries; +pairing-store approvals are outside this check. +Symbolic `accessGroup:` references resolve membership separately and are not +counted as mutable identifiers. + Existing plugins remain source-compatible during the deprecation window: | Deprecated field | Exact mapping | diff --git a/extensions/discord/src/monitor/dm-command-auth.ts b/extensions/discord/src/monitor/dm-command-auth.ts index 08b3ef4702c1..dda9f3dbea27 100644 --- a/extensions/discord/src/monitor/dm-command-auth.ts +++ b/extensions/discord/src/monitor/dm-command-auth.ts @@ -3,10 +3,8 @@ import { type AccessGroupMembershipFact, type ChannelIngressEventInput, type ChannelIngressContextBinding, - type ChannelIngressIdentifierKind, type IdentifierAuthentication, createChannelIngressResolver, - defineStableChannelIngressIdentity, type ChannelIngressIdentitySubjectInput, type ResolveChannelMessageIngressParams, } from "openclaw/plugin-sdk/channel-ingress-runtime"; @@ -14,78 +12,12 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; import type { RequestClient } from "../internal/discord.js"; import { canViewDiscordGuildChannel } from "../send.permissions.js"; -import { normalizeDiscordAllowList } from "./allow-list.js"; +import { discordIngressIdentity } from "./ingress-identity.js"; -const DISCORD_ALLOW_LIST_PREFIXES = ["discord:", "user:", "pk:"]; const DISCORD_CHANNEL_ID = "discord"; -const DISCORD_USER_ID_KIND = "stable-id" satisfies ChannelIngressIdentifierKind; -const DISCORD_USER_NAME_KIND = "username" satisfies ChannelIngressIdentifierKind; export type DiscordDmPolicy = "open" | "pairing" | "allowlist" | "disabled"; -function normalizeDiscordIdEntry(entry: string): string | null { - const text = entry.trim(); - if (!text) { - return null; - } - const maybeId = text.replace(/^<@!?/, "").replace(/>$/, ""); - if (/^\d+$/.test(maybeId)) { - return maybeId; - } - const prefix = DISCORD_ALLOW_LIST_PREFIXES.find((entryPrefix) => text.startsWith(entryPrefix)); - if (prefix) { - const candidate = text.slice(prefix.length).trim(); - return candidate || null; - } - return null; -} - -function normalizeDiscordNameEntry(entry: string): string | null { - const text = entry.trim(); - if (!text || text === "*" || normalizeDiscordIdEntry(text) || /#\d{4}$/.test(text)) { - return null; - } - const nameSlug = normalizeDiscordAllowList([text], DISCORD_ALLOW_LIST_PREFIXES) - ?.names.values() - .next().value; - return typeof nameSlug === "string" && nameSlug ? nameSlug : null; -} - -function normalizeDiscordTagEntry(entry: string): string | null { - const text = entry.trim(); - return /#\d{4}$/.test(text) ? normalizeDiscordNameSubject(text) : null; -} - -function normalizeDiscordNameSubject(value: string): string | null { - const nameSlug = normalizeDiscordAllowList([value], DISCORD_ALLOW_LIST_PREFIXES) - ?.names.values() - .next().value; - return typeof nameSlug === "string" && nameSlug ? nameSlug : null; -} - -const discordIngressIdentity = defineStableChannelIngressIdentity({ - key: "discordUserId", - kind: DISCORD_USER_ID_KIND, - // Discord binds author/user.id on events delivered over the authenticated bot-token session. - authentication: "verified", - normalizeEntry: normalizeDiscordIdEntry, - normalizeSubject: (value) => value.trim() || null, - sensitivity: "pii", - aliases: ( - [ - ["discordUserName", normalizeDiscordNameEntry], - ["discordUserTag", normalizeDiscordTagEntry], - ] as const - ).map(([key, normalizeEntry]) => ({ - key, - kind: DISCORD_USER_NAME_KIND, - normalizeEntry, - normalizeSubject: normalizeDiscordNameSubject, - authentication: "mutable", - sensitivity: "pii", - })), -}); - function createDiscordDmIngressSubject(sender: { id: string; name?: string; diff --git a/extensions/discord/src/monitor/ingress-identity.ts b/extensions/discord/src/monitor/ingress-identity.ts new file mode 100644 index 000000000000..fb0e6a46fcde --- /dev/null +++ b/extensions/discord/src/monitor/ingress-identity.ts @@ -0,0 +1,72 @@ +import { + defineStableChannelIngressIdentity, + type ChannelIngressIdentifierKind, +} from "openclaw/plugin-sdk/channel-ingress-runtime"; +import { normalizeDiscordAllowList } from "./allow-list.js"; + +const DISCORD_ALLOW_LIST_PREFIXES = ["discord:", "user:", "pk:"]; +const DISCORD_USER_ID_KIND = "stable-id" satisfies ChannelIngressIdentifierKind; +const DISCORD_USER_NAME_KIND = "username" satisfies ChannelIngressIdentifierKind; + +function normalizeDiscordIdEntry(entry: string): string | null { + const text = entry.trim(); + if (!text) { + return null; + } + const maybeId = text.replace(/^<@!?/, "").replace(/>$/, ""); + if (/^\d+$/.test(maybeId)) { + return maybeId; + } + const prefix = DISCORD_ALLOW_LIST_PREFIXES.find((entryPrefix) => text.startsWith(entryPrefix)); + if (prefix) { + const candidate = text.slice(prefix.length).trim(); + return candidate || null; + } + return null; +} + +function normalizeDiscordNameEntry(entry: string): string | null { + const text = entry.trim(); + if (!text || text === "*" || normalizeDiscordIdEntry(text) || /#\d{4}$/.test(text)) { + return null; + } + const nameSlug = normalizeDiscordAllowList([text], DISCORD_ALLOW_LIST_PREFIXES) + ?.names.values() + .next().value; + return typeof nameSlug === "string" && nameSlug ? nameSlug : null; +} + +function normalizeDiscordTagEntry(entry: string): string | null { + const text = entry.trim(); + return /#\d{4}$/.test(text) ? normalizeDiscordNameSubject(text) : null; +} + +function normalizeDiscordNameSubject(value: string): string | null { + const nameSlug = normalizeDiscordAllowList([value], DISCORD_ALLOW_LIST_PREFIXES) + ?.names.values() + .next().value; + return typeof nameSlug === "string" && nameSlug ? nameSlug : null; +} + +export const discordIngressIdentity = defineStableChannelIngressIdentity({ + key: "discordUserId", + kind: DISCORD_USER_ID_KIND, + // Discord binds author/user.id on events delivered over the authenticated bot-token session. + authentication: "verified", + normalizeEntry: normalizeDiscordIdEntry, + normalizeSubject: (value) => value.trim() || null, + sensitivity: "pii", + aliases: ( + [ + ["discordUserName", normalizeDiscordNameEntry], + ["discordUserTag", normalizeDiscordTagEntry], + ] as const + ).map(([key, normalizeEntry]) => ({ + key, + kind: DISCORD_USER_NAME_KIND, + normalizeEntry, + normalizeSubject: normalizeDiscordNameSubject, + authentication: "mutable", + sensitivity: "pii", + })), +}); diff --git a/extensions/discord/src/security-audit.test.ts b/extensions/discord/src/security-audit.test.ts index 84a5da657ec9..bae936617041 100644 --- a/extensions/discord/src/security-audit.test.ts +++ b/extensions/discord/src/security-audit.test.ts @@ -207,7 +207,7 @@ describe("Discord security audit findings", () => { config: { enabled: true, token: "t", - allowFrom: ["Alice#1234", "<@123456789012345678>"], + allowFrom: ["Alice#1234", " Alice#1234 ", "second.operator", "<@123456789012345678>"], guilds: { "123": { users: ["trusted.operator"], @@ -222,12 +222,21 @@ describe("Discord security audit findings", () => { storeAllowFrom: ["team.owner"], expectNameBasedSeverity: "warn", detailIncludes: [ - "channels.discord.allowFrom:Alice#1234", - "channels.discord.guilds.123.users:trusted.operator", - "channels.discord.guilds.123.channels.general.users:security-team", - "~/.openclaw/credentials/discord-allowFrom.json:team.owner", + "Found 5 name/tag entries", + "channels.discord.allowFrom (2)", + "channels.discord.guilds.123.users (1)", + "channels.discord.guilds.123.channels.general.users (1)", + "Discord pairing store (1)", + ], + detailExcludes: [ + "Alice#1234", + "second.operator", + "trusted.operator", + "security-team", + "team.owner", + "<@123456789012345678>", + "987654321098765432", ], - detailExcludes: ["<@123456789012345678>"], }, { name: "marks Discord name-based allowlists as break-glass when dangerous matching is enabled", @@ -239,6 +248,7 @@ describe("Discord security audit findings", () => { } satisfies DiscordAccountConfig, expectNameBasedSeverity: "info", detailIncludes: ["out-of-scope"], + detailExcludes: ["Alice#1234"], }, { name: "audits name-based allowlists on non-default Discord accounts", @@ -251,7 +261,8 @@ describe("Discord security audit findings", () => { allowFrom: ["Alice#1234"], } satisfies DiscordAccountConfig, expectNameBasedSeverity: "warn", - detailIncludes: ["channels.discord.accounts.beta.allowFrom:Alice#1234"], + detailIncludes: ["channels.discord.accounts.beta.allowFrom (1)"], + detailExcludes: ["Alice#1234"], }, { name: "does not warn when Discord allowlists use ID-style entries only", @@ -302,7 +313,7 @@ describe("Discord security audit findings", () => { expect(nameBasedFinding.detail).toContain(snippet); } for (const snippet of testCase.detailExcludes ?? []) { - expect(nameBasedFinding.detail).not.toContain(snippet); + expect(JSON.stringify(findings)).not.toContain(snippet); } } }); diff --git a/extensions/discord/src/security-audit.ts b/extensions/discord/src/security-audit.ts index 6d88c8532d62..0906f830fa5c 100644 --- a/extensions/discord/src/security-audit.ts +++ b/extensions/discord/src/security-audit.ts @@ -56,22 +56,18 @@ function listBroadMemberTargetPaths(params: { } function addDiscordNameBasedEntries(params: { - target: Set; + target: Map; values: unknown; source: string; }) { if (!Array.isArray(params.values)) { return; } - for (const value of params.values) { - if (!isDiscordMutableAllowEntry(String(value))) { - continue; - } - const text = normalizeOptionalString(String(value)) ?? ""; - if (!text) { - continue; - } - params.target.add(`${params.source}:${text}`); + const entries = new Set( + params.values.map((value) => String(value).trim()).filter(isDiscordMutableAllowEntry), + ); + if (entries.size > 0) { + params.target.set(params.source, entries.size); } } @@ -96,7 +92,7 @@ export async function collectDiscordSecurityAuditFindings(params: { const storeAllowFrom = await readChannelAllowFromStore("discord", process.env, accountId).catch( () => [], ); - const discordNameBasedAllowEntries = new Set(); + const discordNameBasedAllowEntries = new Map(); const discordPathPrefix = params.orderedAccountIds.length > 1 || params.hasExplicitAccountPath ? `channels.discord.accounts.${accountId}` @@ -136,7 +132,7 @@ export async function collectDiscordSecurityAuditFindings(params: { addDiscordNameBasedEntries({ target: discordNameBasedAllowEntries, values: storeAllowFrom, - source: "~/.openclaw/credentials/discord-allowFrom.json", + source: "Discord pairing store", }); const guildEntries = (discordCfg.guilds as Record | undefined) ?? {}; @@ -168,11 +164,12 @@ export async function collectDiscordSecurityAuditFindings(params: { } if (discordNameBasedAllowEntries.size > 0) { - const examples = Array.from(discordNameBasedAllowEntries).slice(0, 5); + const counts = Array.from(discordNameBasedAllowEntries); + const entryCount = counts.reduce((total, [, count]) => total + count, 0); + const sources = counts.slice(0, 5).map(([source, count]) => `${source} (${count})`); const more = - discordNameBasedAllowEntries.size > examples.length - ? ` (+${discordNameBasedAllowEntries.size - examples.length} more)` - : ""; + counts.length > sources.length ? ` (+${counts.length - sources.length} more sources)` : ""; + const summary = `Found ${entryCount} name/tag entries: ${sources.join(", ")}${more}.`; findings.push({ checkId: "channels.discord.allowFrom.name_based_entries", severity: dangerousNameMatchingEnabled ? "info" : "warn", @@ -181,9 +178,9 @@ export async function collectDiscordSecurityAuditFindings(params: { : "Discord allowlist contains name or tag entries", detail: dangerousNameMatchingEnabled ? "Discord name/tag allowlist matching is explicitly enabled via dangerouslyAllowNameMatching. This mutable-identity mode is operator-selected break-glass behavior and out-of-scope for vulnerability reports by itself. " + - `Found: ${examples.join(", ")}${more}.` + summary : "Discord name/tag allowlist matching uses normalized slugs and can collide across users. " + - `Found: ${examples.join(", ")}${more}.`, + summary, remediation: dangerousNameMatchingEnabled ? "Prefer stable Discord IDs (or <@id>/user:/pk:), then disable dangerouslyAllowNameMatching." : "Prefer stable Discord IDs (or <@id>/user:/pk:) in channels.discord.allowFrom and channels.discord.guilds.*.users, or explicitly opt in with dangerouslyAllowNameMatching=true if you accept the risk.", diff --git a/extensions/discord/src/security.test.ts b/extensions/discord/src/security.test.ts new file mode 100644 index 000000000000..17b9c9bf1f23 --- /dev/null +++ b/extensions/discord/src/security.test.ts @@ -0,0 +1,21 @@ +// Discord tests cover the security adapter's entry-authentication classification. +import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; +import { describe, expect, it } from "vitest"; +import { resolveDiscordAccount } from "./accounts.js"; +import { discordSecurityAdapter } from "./security.js"; + +describe("discordSecurityAdapter.resolveDmPolicy", () => { + it("classifies snowflake entries verified and tag entries mutable", () => { + const cfg = { channels: { discord: { token: "test-token" } } } as OpenClawConfig; + const account = resolveDiscordAccount({ cfg, accountId: "default" }); + const policy = discordSecurityAdapter.resolveDmPolicy?.({ + cfg, + accountId: "default", + account, + }); + + expect(policy?.classifyEntryAuthentication?.("123456789012345678")).toBe("verified"); + expect(policy?.classifyEntryAuthentication?.("alice#0001")).toBe("mutable"); + expect(policy?.classifyEntryAuthentication?.("alice")).toBe("mutable"); + }); +}); diff --git a/extensions/discord/src/security.ts b/extensions/discord/src/security.ts index 1e8025c7fd94..4f0933e32d14 100644 --- a/extensions/discord/src/security.ts +++ b/extensions/discord/src/security.ts @@ -1,5 +1,6 @@ // Discord plugin module implements security behavior. import { createScopedDmSecurityResolver } from "openclaw/plugin-sdk/channel-config-helpers"; +import { identityEntryAuthenticationClassifier } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { createConditionalWarningCollector, createOpenProviderConfiguredRouteWarningCollector, @@ -11,6 +12,7 @@ import { type ResolvedDiscordAccount, } from "./accounts.js"; import type { ChannelPlugin } from "./channel-api.js"; +import { discordIngressIdentity } from "./monitor/ingress-identity.js"; const resolveDiscordDmPolicy = createScopedDmSecurityResolver({ channelKey: "discord", @@ -21,6 +23,7 @@ const resolveDiscordDmPolicy = createScopedDmSecurityResolver raw .trim() diff --git a/extensions/googlechat/src/channel.adapters.ts b/extensions/googlechat/src/channel.adapters.ts index 8b87c9f2b0ae..ec22c9be86df 100644 --- a/extensions/googlechat/src/channel.adapters.ts +++ b/extensions/googlechat/src/channel.adapters.ts @@ -4,6 +4,7 @@ import type { ChannelThreadingContext, ChannelThreadingToolContext, } from "openclaw/plugin-sdk/channel-contract"; +import { identityEntryAuthenticationClassifier } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { createMessageReceiptFromOutboundResults, defineChannelMessageAdapter, @@ -39,6 +40,7 @@ import { sanitizeGoogleChatText, } from "./format.js"; import { resolveGoogleChatGroupRequireMention } from "./group-policy.js"; +import { googleChatIngressIdentity } from "./ingress-identity.js"; const loadGoogleChatChannelRuntime = createLazyRuntimeNamedExport( () => import("./channel.runtime.js"), @@ -124,6 +126,7 @@ export const googlechatSecurityAdapter = { resolvePolicy: (account: ResolvedGoogleChatAccount) => account.config.dmPolicy, resolveAllowFrom: (account: ResolvedGoogleChatAccount) => account.config.allowFrom, allowFromPathSuffix: "", + classifyEntryAuthentication: identityEntryAuthenticationClassifier(googleChatIngressIdentity), normalizeEntry: (raw: string) => formatGoogleChatAllowFromEntry(raw), }, collectWarnings: collectGoogleChatSecurityWarnings, diff --git a/extensions/googlechat/src/ingress-identity.ts b/extensions/googlechat/src/ingress-identity.ts new file mode 100644 index 000000000000..f86522bcea57 --- /dev/null +++ b/extensions/googlechat/src/ingress-identity.ts @@ -0,0 +1,66 @@ +import { defineStableChannelIngressIdentity } from "openclaw/plugin-sdk/channel-ingress-runtime"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "openclaw/plugin-sdk/string-coerce-runtime"; + +export function normalizeGoogleChatUserId(raw?: string | null): string { + const trimmed = normalizeOptionalString(raw) ?? ""; + if (!trimmed) { + return ""; + } + return normalizeLowercaseStringOrEmpty(trimmed.replace(/^users\//i, "")); +} + +const GOOGLECHAT_EMAIL_KIND = "plugin:googlechat-email" as const; + +function normalizeEntryValue(raw?: string | null): string { + return normalizeLowercaseStringOrEmpty(raw ?? ""); +} + +function normalizeGoogleChatStableEntry(entry: string): string | null { + const withoutProvider = normalizeEntryValue(entry).replace( + /^(googlechat|google-chat|gchat):/i, + "", + ); + if (!withoutProvider) { + return null; + } + return withoutProvider.startsWith("users/") + ? normalizeGoogleChatUserId(withoutProvider) + : withoutProvider; +} + +function normalizeGoogleChatEmailEntry(entry: string): string | null { + const withoutProvider = normalizeEntryValue(entry).replace( + /^(googlechat|google-chat|gchat):/i, + "", + ); + if (withoutProvider.startsWith("users/")) { + return null; + } + const stable = normalizeGoogleChatStableEntry(entry); + return stable?.includes("@") ? stable : null; +} + +export const googleChatIngressIdentity = defineStableChannelIngressIdentity({ + key: "sender-id", + // Google signs the webhook for the configured audience before sender.name is consumed. + authentication: "verified", + normalizeEntry: normalizeGoogleChatStableEntry, + normalizeSubject: normalizeGoogleChatUserId, + aliases: [ + { + key: "email", + kind: GOOGLECHAT_EMAIL_KIND, + normalizeEntry: normalizeGoogleChatEmailEntry, + normalizeSubject: normalizeEntryValue, + authentication: "mutable", + }, + ], + isWildcardEntry: (entry) => normalizeEntryValue(entry) === "*", + resolveEntryId: ({ entryIndex, fieldKey }) => + fieldKey === "stableId" + ? `entry-${entryIndex + 1}:user` + : `entry-${entryIndex + 1}:${fieldKey}`, +}); diff --git a/extensions/googlechat/src/monitor-access.ts b/extensions/googlechat/src/monitor-access.ts index f50238ad48fd..0ac36c8aa7ab 100644 --- a/extensions/googlechat/src/monitor-access.ts +++ b/extensions/googlechat/src/monitor-access.ts @@ -2,7 +2,6 @@ import { channelIngressRoutes, createChannelIngressResolver, - defineStableChannelIngressIdentity, type ChannelIngressContextBinding, } from "openclaw/plugin-sdk/channel-ingress-runtime"; import type { ChannelBotLoopProtectionConfig } from "openclaw/plugin-sdk/config-contracts"; @@ -23,68 +22,10 @@ import { import type { ResolvedGoogleChatAccount } from "./accounts.js"; import { sendGoogleChatMessage } from "./api.js"; import { buildGoogleChatGroupPolicyScope } from "./group-policy.js"; +import { googleChatIngressIdentity, normalizeGoogleChatUserId } from "./ingress-identity.js"; import type { GoogleChatCoreRuntime } from "./monitor-types.js"; import type { GoogleChatAnnotation, GoogleChatMessage, GoogleChatSpace } from "./types.js"; -function normalizeUserId(raw?: string | null): string { - const trimmed = normalizeOptionalString(raw) ?? ""; - if (!trimmed) { - return ""; - } - return normalizeLowercaseStringOrEmpty(trimmed.replace(/^users\//i, "")); -} - -const GOOGLECHAT_EMAIL_KIND = "plugin:googlechat-email" as const; - -function normalizeEntryValue(raw?: string | null): string { - return normalizeLowercaseStringOrEmpty(raw ?? ""); -} - -function normalizeGoogleChatStableEntry(entry: string): string | null { - const withoutProvider = normalizeEntryValue(entry).replace( - /^(googlechat|google-chat|gchat):/i, - "", - ); - if (!withoutProvider) { - return null; - } - return withoutProvider.startsWith("users/") ? normalizeUserId(withoutProvider) : withoutProvider; -} - -function normalizeGoogleChatEmailEntry(entry: string): string | null { - const withoutProvider = normalizeEntryValue(entry).replace( - /^(googlechat|google-chat|gchat):/i, - "", - ); - if (withoutProvider.startsWith("users/")) { - return null; - } - const stable = normalizeGoogleChatStableEntry(entry); - return stable?.includes("@") ? stable : null; -} - -const googleChatIngressIdentity = defineStableChannelIngressIdentity({ - key: "sender-id", - // Google signs the webhook for the configured audience before sender.name is consumed. - authentication: "verified", - normalizeEntry: normalizeGoogleChatStableEntry, - normalizeSubject: normalizeUserId, - aliases: [ - { - key: "email", - kind: GOOGLECHAT_EMAIL_KIND, - normalizeEntry: normalizeGoogleChatEmailEntry, - normalizeSubject: normalizeEntryValue, - authentication: "mutable", - }, - ], - isWildcardEntry: (entry) => normalizeEntryValue(entry) === "*", - resolveEntryId: ({ entryIndex, fieldKey }) => - fieldKey === "stableId" - ? `entry-${entryIndex + 1}:user` - : `entry-${entryIndex + 1}:${fieldKey}`, -}); - type GoogleChatGroupEntry = { requireMention?: boolean; enabled?: boolean; @@ -147,7 +88,7 @@ function extractMentionInfo(annotations: GoogleChatAnnotation[], botUser?: strin if (botTargets.has(userName)) { return true; } - return normalizeUserId(userName) === "app"; + return normalizeGoogleChatUserId(userName) === "app"; }); return { hasAnyMention, wasMentioned }; } diff --git a/extensions/irc/src/channel.ts b/extensions/irc/src/channel.ts index d6005b7caa59..434717af321a 100644 --- a/extensions/irc/src/channel.ts +++ b/extensions/irc/src/channel.ts @@ -7,6 +7,7 @@ import { createScopedDmSecurityResolver, } from "openclaw/plugin-sdk/channel-config-helpers"; import { createChatChannelPlugin } from "openclaw/plugin-sdk/channel-core"; +import { identityEntryAuthenticationClassifier } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { createAllowlistProviderOpenWarningCollector, createConditionalWarningCollector, @@ -35,6 +36,7 @@ import { import { IrcChannelConfigSchema } from "./config-schema.js"; import { collectIrcMutableAllowlistWarnings } from "./doctor.js"; import { startIrcGatewayAccount } from "./gateway.js"; +import { ircIngressIdentity } from "./ingress-identity.js"; import { ircMessageAdapter, sendFormattedIrcText } from "./message-adapter.js"; import { isChannelTarget, @@ -129,6 +131,7 @@ const resolveIrcDmPolicy = createScopedDmSecurityResolver({ resolvePolicy: (account) => account.config.dmPolicy, resolveAllowFrom: (account) => account.config.allowFrom, policyPathSuffix: "dmPolicy", + classifyEntryAuthentication: identityEntryAuthenticationClassifier(ircIngressIdentity), normalizeEntry: (raw) => normalizeIrcAllowEntry(raw), }); diff --git a/extensions/irc/src/inbound.ts b/extensions/irc/src/inbound.ts index 2ab942b8e895..e446eafc3602 100644 --- a/extensions/irc/src/inbound.ts +++ b/extensions/irc/src/inbound.ts @@ -6,7 +6,6 @@ import { import { channelIngressRoutes, createChannelIngressResolver, - defineStableChannelIngressIdentity, } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { bindIngressLifecycleToReplyOptions, @@ -32,8 +31,9 @@ import { normalizeStringEntries, } from "openclaw/plugin-sdk/string-coerce-runtime"; import type { ResolvedIrcAccount } from "./accounts.js"; +import { createIrcIngressSubject, ircIngressIdentity } from "./ingress-identity.js"; import type { IrcIngressDispatchResult, IrcIngressLifecycle } from "./irc-ingress.js"; -import { buildIrcAllowlistCandidates, normalizeIrcAllowEntry } from "./normalize.js"; +import { normalizeIrcAllowEntry } from "./normalize.js"; import { sanitizeIrcAssistantText } from "./outbound-base.js"; import { resolveIrcGroupMatch, resolveIrcGroupRequireMention } from "./policy.js"; import { getIrcRuntime } from "./runtime.js"; @@ -41,47 +41,8 @@ import { sendMessageIrc } from "./send.js"; import type { CoreConfig, IrcInboundMessage } from "./types.js"; const CHANNEL_ID = "irc" as const; -const IRC_NICK_KIND = "plugin:irc-nick" as const; type IrcGroupPolicy = "open" | "allowlist" | "disabled"; -const ircIngressIdentity = defineStableChannelIngressIdentity({ - key: "irc-id", - // The IRC server vouches for the connection prefix, but does not bind it to an account owner. - authentication: "asserted", - normalizeEntry: normalizeIrcStableEntry, - normalizeSubject: normalizeLowercaseStringOrEmpty, - sensitivity: "pii", - aliases: [ - { - key: "irc-id-nick-user", - kind: "stable-id" as const, - normalizeEntry: normalizeIrcNickUserEntry, - normalizeSubject: normalizeLowercaseStringOrEmpty, - authentication: "mutable", - sensitivity: "pii" as const, - }, - { - key: "irc-id-nick-host", - kind: "stable-id" as const, - authentication: "asserted", - normalizeEntry: normalizeIrcNickHostEntry, - normalizeSubject: normalizeLowercaseStringOrEmpty, - sensitivity: "pii" as const, - }, - { - key: "irc-nick", - kind: IRC_NICK_KIND, - normalizeEntry: normalizeIrcNickEntry, - normalizeSubject: normalizeLowercaseStringOrEmpty, - authentication: "mutable", - sensitivity: "pii", - }, - ], - isWildcardEntry: (entry) => normalizeIrcAllowEntry(entry) === "*", - resolveEntryId: ({ entryIndex, fieldKey }) => - `irc-entry-${entryIndex + 1}:${fieldKey === "irc-nick" ? "nick" : "id"}`, -}); - const escapeIrcRegexLiteral = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); // IRC nicknames permit punctuation, so ASCII word boundaries lose valid leading/trailing chars. const IRC_NICK_CHARACTER = String.raw`[A-Za-z0-9_\-\[\]\\\x60^{}|~]`; @@ -105,67 +66,10 @@ function buildIrcNickMentionPattern(value: string): string { }).join(""); } -function isBareNick(value: string): boolean { - return !value.includes("!") && !value.includes("@"); -} - -function hasVerifiedHost(value: string): boolean { - return value.includes("@"); -} - -function isHostlessNickUser(value: string): boolean { - return value.includes("!") && !value.includes("@"); -} - -function normalizeIrcStableEntry(value: string): string | null { - const normalized = normalizeIrcAllowEntry(value); - if (!normalized.includes("!") || !hasVerifiedHost(normalized)) { - return null; - } - return normalized; -} - -function normalizeIrcNickHostEntry(value: string): string | null { - const normalized = normalizeIrcAllowEntry(value); - return !normalized.includes("!") && hasVerifiedHost(normalized) ? normalized : null; -} - -function normalizeIrcNickUserEntry(value: string): string | null { - const normalized = normalizeIrcAllowEntry(value); - if (!normalized || normalized === "*" || !isHostlessNickUser(normalized)) { - return null; - } - return normalized; -} - -function normalizeIrcNickEntry(value: string): string | null { - const normalized = normalizeIrcAllowEntry(value); - if (!normalized || normalized === "*" || !isBareNick(normalized)) { - return null; - } - return normalized; -} - function hasEntries(entries: Array | undefined): boolean { return normalizeStringEntries(entries).some((entry) => normalizeIrcAllowEntry(entry)); } -function createIrcIngressSubject(message: IrcInboundMessage) { - const candidates = buildIrcAllowlistCandidates(message, { allowNameMatching: true }); - const stableCandidates = candidates.filter((candidate) => hasVerifiedHost(candidate)); - const nick = normalizeLowercaseStringOrEmpty(message.senderNick); - return { - stableId: stableCandidates[stableCandidates.length - 1] ?? nick, - aliases: { - "irc-id-nick-user": candidates.find((candidate) => isHostlessNickUser(candidate)), - "irc-id-nick-host": stableCandidates.find( - (candidate) => !candidate.includes("!") && candidate.includes("@"), - ), - "irc-nick": nick, - }, - }; -} - function routeDescriptorsForIrcGroup(params: { isGroup: boolean; groupPolicy: IrcGroupPolicy; diff --git a/extensions/irc/src/ingress-identity.ts b/extensions/irc/src/ingress-identity.ts new file mode 100644 index 000000000000..c5cf560e989a --- /dev/null +++ b/extensions/irc/src/ingress-identity.ts @@ -0,0 +1,101 @@ +import { defineStableChannelIngressIdentity } from "openclaw/plugin-sdk/channel-ingress-runtime"; +import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { buildIrcAllowlistCandidates, normalizeIrcAllowEntry } from "./normalize.js"; +import type { IrcInboundMessage } from "./types.js"; + +const IRC_NICK_KIND = "plugin:irc-nick" as const; + +export const ircIngressIdentity = defineStableChannelIngressIdentity({ + key: "irc-id", + // The IRC server vouches for the connection prefix, but does not bind it to an account owner. + authentication: "asserted", + normalizeEntry: normalizeIrcStableEntry, + normalizeSubject: normalizeLowercaseStringOrEmpty, + sensitivity: "pii", + aliases: [ + { + key: "irc-id-nick-user", + kind: "stable-id" as const, + normalizeEntry: normalizeIrcNickUserEntry, + normalizeSubject: normalizeLowercaseStringOrEmpty, + authentication: "mutable", + sensitivity: "pii" as const, + }, + { + key: "irc-id-nick-host", + kind: "stable-id" as const, + authentication: "asserted", + normalizeEntry: normalizeIrcNickHostEntry, + normalizeSubject: normalizeLowercaseStringOrEmpty, + sensitivity: "pii" as const, + }, + { + key: "irc-nick", + kind: IRC_NICK_KIND, + normalizeEntry: normalizeIrcNickEntry, + normalizeSubject: normalizeLowercaseStringOrEmpty, + authentication: "mutable", + sensitivity: "pii", + }, + ], + isWildcardEntry: (entry) => normalizeIrcAllowEntry(entry) === "*", + resolveEntryId: ({ entryIndex, fieldKey }) => + `irc-entry-${entryIndex + 1}:${fieldKey === "irc-nick" ? "nick" : "id"}`, +}); + +function isBareNick(value: string): boolean { + return !value.includes("!") && !value.includes("@"); +} + +function hasVerifiedHost(value: string): boolean { + return value.includes("@"); +} + +function isHostlessNickUser(value: string): boolean { + return value.includes("!") && !value.includes("@"); +} + +function normalizeIrcStableEntry(value: string): string | null { + const normalized = normalizeIrcAllowEntry(value); + if (!normalized.includes("!") || !hasVerifiedHost(normalized)) { + return null; + } + return normalized; +} + +function normalizeIrcNickHostEntry(value: string): string | null { + const normalized = normalizeIrcAllowEntry(value); + return !normalized.includes("!") && hasVerifiedHost(normalized) ? normalized : null; +} + +function normalizeIrcNickUserEntry(value: string): string | null { + const normalized = normalizeIrcAllowEntry(value); + if (!normalized || normalized === "*" || !isHostlessNickUser(normalized)) { + return null; + } + return normalized; +} + +function normalizeIrcNickEntry(value: string): string | null { + const normalized = normalizeIrcAllowEntry(value); + if (!normalized || normalized === "*" || !isBareNick(normalized)) { + return null; + } + return normalized; +} + +export function createIrcIngressSubject(message: IrcInboundMessage) { + const candidates = buildIrcAllowlistCandidates(message, { allowNameMatching: true }); + const stableCandidates = candidates.filter((candidate) => hasVerifiedHost(candidate)); + const nick = normalizeLowercaseStringOrEmpty(message.senderNick); + return { + stableId: stableCandidates[stableCandidates.length - 1] ?? nick, + aliases: { + "irc-id-nick-user": candidates.find((candidate) => isHostlessNickUser(candidate)), + "irc-id-nick-host": stableCandidates.find( + (candidate) => !candidate.includes("!") && candidate.includes("@"), + ), + "irc-nick": nick, + }, + }; +} diff --git a/extensions/mattermost/src/channel.ts b/extensions/mattermost/src/channel.ts index 38773a7b05b0..f0e2df456c8a 100644 --- a/extensions/mattermost/src/channel.ts +++ b/extensions/mattermost/src/channel.ts @@ -16,6 +16,7 @@ import type { ChannelToolSend, } from "openclaw/plugin-sdk/channel-contract"; import { createChatChannelPlugin } from "openclaw/plugin-sdk/channel-core"; +import { identityEntryAuthenticationClassifier } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { createChannelMessageAdapterFromOutbound } from "openclaw/plugin-sdk/channel-outbound"; import { createLoggedPairingApprovalNotifier } from "openclaw/plugin-sdk/channel-pairing"; import { createRestrictSendersChannelSecurity } from "openclaw/plugin-sdk/channel-policy"; @@ -67,6 +68,7 @@ import { type ResolvedMattermostAccount, } from "./mattermost/accounts.js"; import { normalizeMattermostEmojiName } from "./mattermost/emoji.js"; +import { mattermostIngressIdentity } from "./mattermost/ingress-identity.js"; import type { MattermostSendResult } from "./mattermost/send.js"; import { looksLikeMattermostTargetId, @@ -137,6 +139,7 @@ const mattermostSecurityAdapter = createRestrictSendersChannelSecurity normalizeAllowEntry(raw), }); diff --git a/extensions/mattermost/src/mattermost/ingress-identity.ts b/extensions/mattermost/src/mattermost/ingress-identity.ts new file mode 100644 index 000000000000..23af1cfa5481 --- /dev/null +++ b/extensions/mattermost/src/mattermost/ingress-identity.ts @@ -0,0 +1,46 @@ +import { parseAccessGroupAllowFromEntry } from "openclaw/plugin-sdk/access-groups"; +import type { + ChannelIngressIdentifierKind, + StableChannelIngressIdentityParams, +} from "openclaw/plugin-sdk/channel-ingress-runtime"; +import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; + +const MATTERMOST_USER_NAME_KIND = + "plugin:mattermost-user-name" as const satisfies ChannelIngressIdentifierKind; +export const mattermostIngressIdentity = { + key: "sender-id", + // Authenticated Mattermost WebSocket post events carry the server-owned post.user_id. + authentication: "verified", + normalize: normalizeMattermostAllowEntry, + aliases: [ + { + key: "sender-name", + kind: MATTERMOST_USER_NAME_KIND, + normalizeEntry: normalizeMattermostAllowEntry, + normalizeSubject: normalizeMattermostAllowEntry, + authentication: "mutable", + }, + ], + isWildcardEntry: (entry) => normalizeMattermostAllowEntry(entry) === "*", + resolveEntryId: ({ entryIndex, fieldKey }) => + `mattermost-entry-${entryIndex + 1}:${fieldKey === "sender-name" ? "name" : "user"}`, +} satisfies StableChannelIngressIdentityParams; + +export function normalizeMattermostAllowEntry(entry: string): string { + const trimmed = entry.trim(); + if (!trimmed) { + return ""; + } + if (trimmed === "*") { + return "*"; + } + const accessGroupName = parseAccessGroupAllowFromEntry(trimmed); + if (accessGroupName) { + return `accessGroup:${accessGroupName}`; + } + const normalized = trimmed + .replace(/^(mattermost|user):/i, "") + .replace(/^@/, "") + .trim(); + return normalized ? normalizeLowercaseStringOrEmpty(normalized) : ""; +} diff --git a/extensions/mattermost/src/mattermost/monitor-auth.test.ts b/extensions/mattermost/src/mattermost/monitor-auth.test.ts index 4cce0e3de643..c4ab985c7c3b 100644 --- a/extensions/mattermost/src/mattermost/monitor-auth.test.ts +++ b/extensions/mattermost/src/mattermost/monitor-auth.test.ts @@ -13,7 +13,7 @@ describe("mattermost monitor auth", () => { let authorizeMattermostCommandInvocation: typeof import("./monitor-auth.js").authorizeMattermostCommandInvocation; let formatMattermostDirectMessageDropLog: typeof import("./monitor-auth.js").formatMattermostDirectMessageDropLog; let isMattermostSenderAllowed: typeof import("./monitor-auth.js").isMattermostSenderAllowed; - let normalizeMattermostAllowEntry: typeof import("./monitor-auth.js").normalizeMattermostAllowEntry; + let normalizeMattermostAllowEntry: typeof import("./ingress-identity.js").normalizeMattermostAllowEntry; let normalizeMattermostAllowList: typeof import("./monitor-auth.js").normalizeMattermostAllowList; beforeAll(async () => { @@ -21,9 +21,9 @@ describe("mattermost monitor auth", () => { authorizeMattermostCommandInvocation, formatMattermostDirectMessageDropLog, isMattermostSenderAllowed, - normalizeMattermostAllowEntry, normalizeMattermostAllowList, } = await import("./monitor-auth.js")); + ({ normalizeMattermostAllowEntry } = await import("./ingress-identity.js")); }); beforeEach(() => { diff --git a/extensions/mattermost/src/mattermost/monitor-auth.ts b/extensions/mattermost/src/mattermost/monitor-auth.ts index 3c52ca6131fa..15adedd58ac6 100644 --- a/extensions/mattermost/src/mattermost/monitor-auth.ts +++ b/extensions/mattermost/src/mattermost/monitor-auth.ts @@ -1,61 +1,16 @@ // Mattermost plugin module implements monitor auth behavior. -import { parseAccessGroupAllowFromEntry } from "openclaw/plugin-sdk/access-groups"; import { type ChannelIngressDecision, type ChannelIngressEventInput, - type ChannelIngressIdentifierKind, resolveStableChannelMessageIngress, - type StableChannelIngressIdentityParams, } from "openclaw/plugin-sdk/channel-ingress-runtime"; -import { - normalizeLowercaseStringOrEmpty, - uniqueStrings, -} from "openclaw/plugin-sdk/string-coerce-runtime"; +import { uniqueStrings } from "openclaw/plugin-sdk/string-coerce-runtime"; import type { ResolvedMattermostAccount } from "./accounts.js"; import type { MattermostChannel } from "./client.js"; +import { mattermostIngressIdentity, normalizeMattermostAllowEntry } from "./ingress-identity.js"; import type { ChatType, OpenClawConfig } from "./runtime-api.js"; import { isDangerousNameMatchingEnabled, resolveAllowlistMatchSimple } from "./runtime-api.js"; -const MATTERMOST_USER_NAME_KIND = - "plugin:mattermost-user-name" as const satisfies ChannelIngressIdentifierKind; -const mattermostIngressIdentity = { - key: "sender-id", - // Authenticated Mattermost WebSocket post events carry the server-owned post.user_id. - authentication: "verified", - normalize: normalizeMattermostAllowEntry, - aliases: [ - { - key: "sender-name", - kind: MATTERMOST_USER_NAME_KIND, - normalizeEntry: normalizeMattermostAllowEntry, - normalizeSubject: normalizeMattermostAllowEntry, - authentication: "mutable", - }, - ], - isWildcardEntry: (entry) => normalizeMattermostAllowEntry(entry) === "*", - resolveEntryId: ({ entryIndex, fieldKey }) => - `mattermost-entry-${entryIndex + 1}:${fieldKey === "sender-name" ? "name" : "user"}`, -} satisfies StableChannelIngressIdentityParams; - -export function normalizeMattermostAllowEntry(entry: string): string { - const trimmed = entry.trim(); - if (!trimmed) { - return ""; - } - if (trimmed === "*") { - return "*"; - } - const accessGroupName = parseAccessGroupAllowFromEntry(trimmed); - if (accessGroupName) { - return `accessGroup:${accessGroupName}`; - } - const normalized = trimmed - .replace(/^(mattermost|user):/i, "") - .replace(/^@/, "") - .trim(); - return normalized ? normalizeLowercaseStringOrEmpty(normalized) : ""; -} - export function normalizeMattermostAllowList(entries: Array): string[] { const normalized = entries .map((entry) => normalizeMattermostAllowEntry(String(entry))) diff --git a/extensions/mattermost/src/mattermost/monitor-posts.ts b/extensions/mattermost/src/mattermost/monitor-posts.ts index 47adfa93c6af..24d1b15ee7a5 100644 --- a/extensions/mattermost/src/mattermost/monitor-posts.ts +++ b/extensions/mattermost/src/mattermost/monitor-posts.ts @@ -15,10 +15,10 @@ import { uniqueStrings, } from "openclaw/plugin-sdk/string-coerce-runtime"; import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; +import { normalizeMattermostAllowEntry } from "./ingress-identity.js"; import { resolveMattermostInboundMentionDecision } from "./monitor-activation.js"; import { formatMattermostDirectMessageDropLog, - normalizeMattermostAllowEntry, resolveMattermostMonitorInboundAccess, } from "./monitor-auth.js"; import { resolveMattermostPendingHistoryKey } from "./monitor-context.js"; diff --git a/extensions/mattermost/src/mattermost/monitor-turn.ts b/extensions/mattermost/src/mattermost/monitor-turn.ts index 06d261bf8ac2..a75dae18b95b 100644 --- a/extensions/mattermost/src/mattermost/monitor-turn.ts +++ b/extensions/mattermost/src/mattermost/monitor-turn.ts @@ -19,7 +19,7 @@ import { createMattermostDraftPreviewBoundaryController, createMattermostDraftStream, } from "./draft-stream.js"; -import { normalizeMattermostAllowEntry } from "./monitor-auth.js"; +import { normalizeMattermostAllowEntry } from "./ingress-identity.js"; import { formatMattermostFinalDeliveryOutcomeLog, resolveMattermostReplyRootId, diff --git a/extensions/slack/src/monitor/auth.ts b/extensions/slack/src/monitor/auth.ts index 7da25ae15a18..deea55436d76 100644 --- a/extensions/slack/src/monitor/auth.ts +++ b/extensions/slack/src/monitor/auth.ts @@ -2,11 +2,9 @@ import { type ChannelIngressEventInput, type ChannelIngressContextBinding, - type ChannelIngressIdentifierKind, type ChannelIngressPolicyInput, type ChannelIngressStateInput, createChannelIngressResolver, - defineStableChannelIngressIdentity, readChannelIngressStoreAllowFromForDmPolicy, } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; @@ -15,20 +13,22 @@ import { resolveExpiresAtMsFromDurationMs, } from "openclaw/plugin-sdk/number-runtime"; import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; -import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { collectSlackCursorPages } from "../cursor-pages.js"; -import { parseSlackTarget } from "../target-parsing.js"; import { allowListMatches, normalizeAllowListLower, normalizeSlackAllowOwnerEntry, - normalizeSlackSlug, resolveSlackUserAllowListForTeam, } from "./allow-list.js"; import { resolveSlackChannelConfig } from "./channel-config.js"; import { inferSlackChannelType } from "./channel-type.js"; import { normalizeSlackChannelType, type SlackMonitorContext } from "./context.js"; import type { SlackEventScope } from "./event-scope.js"; +import { + createSlackIngressSubject, + slackIngressIdentity, + SLACK_USER_NAME_KIND, +} from "./ingress-identity.js"; import { isTransientSlackThreadLookupError } from "./thread-resolution.js"; type SlackChannelMembersCacheEntry = { @@ -48,129 +48,7 @@ const slackChannelMembersCache = new WeakMap< const DEFAULT_CHANNEL_MEMBERS_CACHE_TTL_MS = 60_000; const CHANNEL_MEMBERS_CACHE_MAX = 512; const SLACK_CHANNEL_ID = "slack"; -const SLACK_USER_NAME_KIND = - "plugin:slack-user-name" as const satisfies ChannelIngressIdentifierKind; -const SLACK_WORKSPACE_USER_ID_KIND = - "plugin:slack-workspace-user-id" as const satisfies ChannelIngressIdentifierKind; - export class SlackSystemEventAuthRetryError extends Error {} -function normalizeSlackUserId(raw?: string | null): string { - const value = (raw ?? "").trim().toLowerCase(); - if (!value) { - return ""; - } - const mention = value.match(/^<@([a-z0-9_]+)>$/i); - if (mention?.[1]) { - return mention[1]; - } - return value.replace(/^(slack:|user:)/, ""); -} - -function isSlackStableUserId(value: string): boolean { - return /^[ubw][a-z0-9_]+$/i.test(value); -} - -function normalizeSlackWorkspaceUserEntry(entry: string): string | null { - const normalized = entry.trim().toLowerCase(); - if (!normalized) { - return null; - } - try { - const target = parseSlackTarget(normalized); - if (target?.kind === "user" && target.teamId) { - return target.normalized; - } - } catch { - return null; - } - return null; -} - -function normalizeSlackBareUserEntry(entry: string): string | null { - const normalized = entry.trim().toLowerCase(); - if (!normalized || normalizeSlackWorkspaceUserEntry(normalized)) { - return null; - } - const userId = normalizeSlackUserId(normalized); - return isSlackStableUserId(userId) ? userId : null; -} - -function normalizeSlackStableEntry(entry: string): string | null { - return normalizeSlackBareUserEntry(entry) ?? normalizeSlackWorkspaceUserEntry(entry); -} - -function normalizeSlackNameEntry(entry: string): string | null { - const normalized = entry.trim().toLowerCase(); - if (!normalized || normalizeSlackStableEntry(normalized)) { - return null; - } - return normalized.replace(/^slack:/, "") || null; -} - -function normalizeSlackNameSubject(value: string): string | null { - return value.trim().toLowerCase() || null; -} - -function normalizeSlackNameSlugEntry(entry: string): string | null { - const name = normalizeSlackNameEntry(entry); - if (!name) { - return null; - } - return normalizeSlackSlug(name) || null; -} - -const slackIngressIdentity = defineStableChannelIngressIdentity({ - key: "senderId", - kind: "stable-id", - // Direct Slack transports bind this id, while relay mode only authenticates its relay peer. - // The shared declaration therefore uses the strongest claim defensible for every mode. - authentication: "asserted", - normalizeEntry: normalizeSlackBareUserEntry, - normalizeSubject: normalizeSlackUserId, - sensitivity: "pii", - aliases: [ - { - key: "workspaceSenderId", - kind: SLACK_WORKSPACE_USER_ID_KIND, - authentication: "asserted", - normalizeEntry: normalizeSlackWorkspaceUserEntry, - normalizeSubject: normalizeSlackWorkspaceUserEntry, - sensitivity: "pii", - }, - ...( - [ - ["senderName", normalizeSlackNameEntry], - ["senderNameSlug", normalizeSlackNameSlugEntry], - ] as const - ).map(([key, normalizeEntry]) => ({ - key, - kind: SLACK_USER_NAME_KIND, - normalizeEntry, - normalizeSubject: normalizeSlackNameSubject, - authentication: "mutable" as const, - sensitivity: "pii" as const, - })), - ], -}); - -function createSlackIngressSubject(params: { - senderId: string; - senderName?: string; - teamId?: string; -}) { - const senderId = normalizeSlackUserId(params.senderId); - const teamId = normalizeOptionalLowercaseString(params.teamId); - const senderName = params.senderName?.trim().toLowerCase(); - const senderNameSlug = senderName ? normalizeSlackSlug(senderName) : undefined; - return { - stableId: senderId, - aliases: { - workspaceSenderId: teamId && senderId ? `team:${teamId}:user:${senderId}` : undefined, - senderName, - senderNameSlug, - }, - }; -} function createSlackIngressResolver(ctx: SlackMonitorContext) { return createChannelIngressResolver({ diff --git a/extensions/slack/src/monitor/ingress-identity.ts b/extensions/slack/src/monitor/ingress-identity.ts new file mode 100644 index 000000000000..00d426a439f0 --- /dev/null +++ b/extensions/slack/src/monitor/ingress-identity.ts @@ -0,0 +1,130 @@ +import { + defineStableChannelIngressIdentity, + type ChannelIngressIdentifierKind, +} from "openclaw/plugin-sdk/channel-ingress-runtime"; +import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { parseSlackTarget } from "../target-parsing.js"; +import { normalizeSlackSlug } from "./allow-list.js"; + +export const SLACK_USER_NAME_KIND = + "plugin:slack-user-name" as const satisfies ChannelIngressIdentifierKind; +const SLACK_WORKSPACE_USER_ID_KIND = + "plugin:slack-workspace-user-id" as const satisfies ChannelIngressIdentifierKind; + +function normalizeSlackUserId(raw?: string | null): string { + const value = (raw ?? "").trim().toLowerCase(); + if (!value) { + return ""; + } + const mention = value.match(/^<@([a-z0-9_]+)>$/i); + if (mention?.[1]) { + return mention[1]; + } + return value.replace(/^(slack:|user:)/, ""); +} + +function isSlackStableUserId(value: string): boolean { + return /^[ubw][a-z0-9_]+$/i.test(value); +} + +function normalizeSlackWorkspaceUserEntry(entry: string): string | null { + const normalized = entry.trim().toLowerCase(); + if (!normalized) { + return null; + } + try { + const target = parseSlackTarget(normalized); + if (target?.kind === "user" && target.teamId) { + return target.normalized; + } + } catch { + return null; + } + return null; +} + +function normalizeSlackBareUserEntry(entry: string): string | null { + const normalized = entry.trim().toLowerCase(); + if (!normalized || normalizeSlackWorkspaceUserEntry(normalized)) { + return null; + } + const userId = normalizeSlackUserId(normalized); + return isSlackStableUserId(userId) ? userId : null; +} + +function normalizeSlackStableEntry(entry: string): string | null { + return normalizeSlackBareUserEntry(entry) ?? normalizeSlackWorkspaceUserEntry(entry); +} + +function normalizeSlackNameEntry(entry: string): string | null { + const normalized = entry.trim().toLowerCase(); + if (!normalized || normalizeSlackStableEntry(normalized)) { + return null; + } + return normalized.replace(/^slack:/, "") || null; +} + +function normalizeSlackNameSubject(value: string): string | null { + return value.trim().toLowerCase() || null; +} + +function normalizeSlackNameSlugEntry(entry: string): string | null { + const name = normalizeSlackNameEntry(entry); + if (!name) { + return null; + } + return normalizeSlackSlug(name) || null; +} + +export const slackIngressIdentity = defineStableChannelIngressIdentity({ + key: "senderId", + kind: "stable-id", + // Direct Slack transports bind this id, while relay mode only authenticates its relay peer. + // The shared declaration therefore uses the strongest claim defensible for every mode. + authentication: "asserted", + normalizeEntry: normalizeSlackBareUserEntry, + normalizeSubject: normalizeSlackUserId, + sensitivity: "pii", + aliases: [ + { + key: "workspaceSenderId", + kind: SLACK_WORKSPACE_USER_ID_KIND, + authentication: "asserted", + normalizeEntry: normalizeSlackWorkspaceUserEntry, + normalizeSubject: normalizeSlackWorkspaceUserEntry, + sensitivity: "pii", + }, + ...( + [ + ["senderName", normalizeSlackNameEntry], + ["senderNameSlug", normalizeSlackNameSlugEntry], + ] as const + ).map(([key, normalizeEntry]) => ({ + key, + kind: SLACK_USER_NAME_KIND, + normalizeEntry, + normalizeSubject: normalizeSlackNameSubject, + authentication: "mutable" as const, + sensitivity: "pii" as const, + })), + ], +}); + +export function createSlackIngressSubject(params: { + senderId: string; + senderName?: string; + teamId?: string; +}) { + const senderId = normalizeSlackUserId(params.senderId); + const teamId = normalizeOptionalLowercaseString(params.teamId); + const senderName = params.senderName?.trim().toLowerCase(); + const senderNameSlug = senderName ? normalizeSlackSlug(senderName) : undefined; + return { + stableId: senderId, + aliases: { + workspaceSenderId: teamId && senderId ? `team:${teamId}:user:${senderId}` : undefined, + senderName, + senderNameSlug, + }, + }; +} diff --git a/extensions/slack/src/security.ts b/extensions/slack/src/security.ts index 3fcca5b00fff..409d4259349b 100644 --- a/extensions/slack/src/security.ts +++ b/extensions/slack/src/security.ts @@ -1,5 +1,6 @@ // Slack plugin module implements security behavior. import { createScopedDmSecurityResolver } from "openclaw/plugin-sdk/channel-config-helpers"; +import { identityEntryAuthenticationClassifier } from "openclaw/plugin-sdk/channel-ingress-runtime"; import { createConditionalWarningCollector, createOpenProviderConfiguredRouteWarningCollector, @@ -11,6 +12,7 @@ import { type ResolvedSlackAccount, } from "./accounts.js"; import type { ChannelPlugin } from "./channel-api.js"; +import { slackIngressIdentity } from "./monitor/ingress-identity.js"; const resolveSlackDmPolicy = createScopedDmSecurityResolver({ channelKey: "slack", @@ -21,6 +23,7 @@ const resolveSlackDmPolicy = createScopedDmSecurityResolver raw .trim() diff --git a/scripts/check-plugin-sdk-exports.mts b/scripts/check-plugin-sdk-exports.mts index 989f5d75b9ac..ce4b6f1d2ce1 100755 --- a/scripts/check-plugin-sdk-exports.mts +++ b/scripts/check-plugin-sdk-exports.mts @@ -69,6 +69,7 @@ let missing = 0; join(consumerRoot, "index.ts"), `import { buildChannelConfigSchema, DmPolicySchema } from "openclaw/plugin-sdk/channel-config-schema"; import { defineChannelPluginEntry } from "openclaw/plugin-sdk/core"; +import { identityEntryAuthenticationClassifier } from "openclaw/plugin-sdk/channel-ingress-runtime"; import type { ChannelIngressIdentitySubjectInput, IdentifierAuthentication, @@ -88,6 +89,11 @@ const subject: ChannelIngressIdentitySubjectInput = { authentication: { "provider-user-id": identifierAuthentication }, }; void subject; +const classifyEntryAuthentication = identityEntryAuthenticationClassifier({ + primary: { authentication: identifierAuthentication }, +}); +const entryAuthentication: IdentifierAuthentication | undefined = classifyEntryAuthentication("provider-user-id"); +void entryAuthentication; const runtimeStore = createPluginRuntimeStore({ pluginId: "package-consumer", diff --git a/scripts/plugin-sdk-surface-report.mts b/scripts/plugin-sdk-surface-report.mts index ab2bca76c6e1..c5c4ed962498 100644 --- a/scripts/plugin-sdk-surface-report.mts +++ b/scripts/plugin-sdk-surface-report.mts @@ -321,7 +321,8 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // +2: canonical session-model selection and auxiliary runtime-auth preparation. // +1: identifier authentication input type for external channel plugins. // +1: shared channel-account logout config cleanup. - 4347, + // +1: descriptor-based allowFrom authentication classifier for channel security audits. + 4348, env, ), publicFunctionExports: readPluginSdkSurfaceBudgetEnv( @@ -419,7 +420,8 @@ export function readPluginSdkSurfaceBudgets(env: NodeJS.ProcessEnv = process.env // +2: bounded ask_user owner-order map builder and option resolver. // +2: canonical session-model selection and auxiliary runtime-auth preparation. // +1: shared channel-account logout config cleanup. - 2583, + // +1: descriptor-based allowFrom authentication classifier for channel security audits. + 2584, env, ), publicDeprecatedExports: readPluginSdkSurfaceBudgetEnv( diff --git a/src/channels/message-access/runtime-identity.test.ts b/src/channels/message-access/runtime-identity.test.ts new file mode 100644 index 000000000000..681ad7a5f81c --- /dev/null +++ b/src/channels/message-access/runtime-identity.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from "vitest"; +import { + defineStableChannelIngressIdentity, + identityEntryAuthenticationClassifier, +} from "./runtime-identity.js"; +import type { ChannelIngressIdentityDescriptor } from "./runtime-types.js"; + +describe("identityEntryAuthenticationClassifier", () => { + const identity = defineStableChannelIngressIdentity({ + normalizeEntry: (raw) => (raw.startsWith("id:") ? raw.slice(3) : null), + authentication: "verified", + aliases: [ + { + key: "name", + normalizeEntry: (raw) => (raw.startsWith("name:") ? raw.slice(5) : null), + authentication: "mutable", + }, + { + key: "tag", + normalizeEntry: (raw) => (raw.startsWith("tag:") ? raw.slice(4) : null), + dangerous: true, + }, + { key: "legacy", normalizeEntry: (raw) => (raw.startsWith("legacy:") ? raw.slice(7) : null) }, + ], + }); + + it.each([ + ["id:123", "verified"], + ["name:Alice", "mutable"], + ["tag:alice#0001", "mutable"], + ["legacy:123", "asserted"], + ["rejected", undefined], + ["id: ", undefined], + ["*", undefined], + ] as const)("classifies %s as %s", (raw, expected) => { + expect(identityEntryAuthenticationClassifier(identity)(raw)).toBe(expected); + }); + + it.each(["verified", "asserted", "unverified", "mutable"] as const)( + "resolves predicate strength %s on the raw entry", + (authentication) => { + const classify = identityEntryAuthenticationClassifier({ + primary: { + normalizeEntry: (raw) => raw.trim().toLowerCase(), + authentication: (raw) => (raw === " RAW " ? authentication : "mutable"), + dangerous: true, + }, + }); + expect(classify(" RAW ")).toBe(authentication); + }, + ); + + it.each([false, true])("takes the strongest accepting field (reverse=%s)", (reverse) => { + const fields = [ + { key: "name", authentication: "mutable" as const }, + { key: "id", authentication: "verified" as const }, + { key: "legacy" }, + ]; + if (reverse) { + fields.reverse(); + } + const descriptor: ChannelIngressIdentityDescriptor = { + primary: { authentication: "unverified" }, + aliases: fields, + isWildcardEntry: (raw) => raw === "everyone", + }; + const classify = identityEntryAuthenticationClassifier(descriptor); + expect(classify("123")).toBe("verified"); + expect(classify("everyone")).toBeUndefined(); + }); + + it.each([ + [true, "mutable"], + [false, "asserted"], + ] as const)("resolves the legacy dangerous predicate (%s)", (dangerous, expected) => { + const classify = identityEntryAuthenticationClassifier({ + primary: { + authentication: () => undefined, + dangerous: (raw) => raw === " RAW " && dangerous, + }, + }); + expect(classify(" RAW ")).toBe(expected); + }); + + it("accepts the raw stable-identity input used by channel runtimes", () => { + const classify = identityEntryAuthenticationClassifier({ + normalize: (raw) => raw.trim(), + authentication: "asserted", + aliases: [{ key: "name", authentication: "mutable" }], + }); + expect(classify("id")).toBe("asserted"); + }); +}); diff --git a/src/channels/message-access/runtime-identity.ts b/src/channels/message-access/runtime-identity.ts index ce2064656c45..184802e98c66 100644 --- a/src/channels/message-access/runtime-identity.ts +++ b/src/channels/message-access/runtime-identity.ts @@ -1,5 +1,9 @@ import { expectDefined } from "@openclaw/normalization-core"; -import type { IdentifierAuthentication } from "./identifier-authentication.js"; +import { + identifierAuthenticationFrom, + meetsIdentifierAuthentication, + type IdentifierAuthentication, +} from "./identifier-authentication.js"; /** * Channel ingress identity adapter helpers. * @@ -35,6 +39,33 @@ export function defineStableChannelIngressIdentity( }; } +/** Classify configured entries without needing a sender or granting admission. */ +export function identityEntryAuthenticationClassifier( + identity: ChannelIngressIdentityDescriptor | StableChannelIngressIdentityParams, +) { + const descriptor = + "primary" in identity ? identity : defineStableChannelIngressIdentity(identity); + const fields = identityFields(descriptor); + const isWildcardEntry = descriptor.isWildcardEntry ?? ((value: string) => value === "*"); + return (raw: string): IdentifierAuthentication | undefined => { + if (isWildcardEntry(raw)) { + return undefined; + } + let strongest: IdentifierAuthentication | undefined; + // An entry accepted by a stable field does not depend solely on its alias match. + for (const field of fields) { + if (!normalizeFieldValue(field, raw, "entry")) { + continue; + } + const authentication = fieldAuthentication(field, raw, fieldDangerous(field, raw)); + if (strongest === undefined || meetsIdentifierAuthentication(authentication, strongest)) { + strongest = authentication; + } + } + return strongest; + }; +} + function defaultNormalize(value: string): string { return value; } @@ -59,10 +90,11 @@ function fieldDangerous(field: ResolvedIdentityField, value: string): boolean | function fieldAuthentication( field: ResolvedIdentityField, value: string, -): IdentifierAuthentication | undefined { - return typeof field.authentication === "function" - ? field.authentication(value) - : field.authentication; + dangerous: boolean | undefined, +): IdentifierAuthentication { + const authentication = + typeof field.authentication === "function" ? field.authentication(value) : field.authentication; + return identifierAuthenticationFrom({ authentication, dangerous }); } function identityFields(identity: ChannelIngressIdentityDescriptor): ResolvedIdentityField[] { @@ -109,8 +141,7 @@ function adapterEntry(params: { value: params.value, identityFieldKey: params.field.key, ...(params.wildcard ? { wildcard: true } : {}), - authentication: - fieldAuthentication(params.field, params.entry) ?? (dangerous ? "mutable" : "asserted"), + authentication: fieldAuthentication(params.field, params.entry, dangerous), dangerous, sensitivity: params.field.sensitivity, }; @@ -231,7 +262,7 @@ export function createIdentitySubject( const authentication = input.authentication !== undefined ? (input.authentication[field.key] ?? "unverified") - : (fieldAuthentication(field, value) ?? (dangerous ? "mutable" : "asserted")); + : fieldAuthentication(field, value, dangerous); return [ { opaqueId: field.key, diff --git a/src/channels/plugins/helpers.ts b/src/channels/plugins/helpers.ts index c0805dd7d98f..68cb9552d18a 100644 --- a/src/channels/plugins/helpers.ts +++ b/src/channels/plugins/helpers.ts @@ -46,6 +46,7 @@ export function buildAccountScopedDmSecurityPolicy(params: { approveChannelId?: string; approveHint?: string; normalizeEntry?: (raw: string) => string; + classifyEntryAuthentication?: ChannelSecurityDmPolicy["classifyEntryAuthentication"]; inheritSharedDefaultsFromDefaultAccount?: boolean; }): ChannelSecurityDmPolicy { const resolvedAccountId = params.accountId ?? params.fallbackAccountId ?? DEFAULT_ACCOUNT_ID; @@ -101,5 +102,6 @@ export function buildAccountScopedDmSecurityPolicy(params: { approveHint: params.approveHint ?? formatPairingApproveHint(params.approveChannelId ?? params.channelKey), normalizeEntry: params.normalizeEntry, + classifyEntryAuthentication: params.classifyEntryAuthentication, }; } diff --git a/src/channels/plugins/types.core.ts b/src/channels/plugins/types.core.ts index c75d0c8e4c70..5b75896e8497 100644 --- a/src/channels/plugins/types.core.ts +++ b/src/channels/plugins/types.core.ts @@ -17,6 +17,7 @@ import type { MessagePresentation } from "../../interactive/payload.js"; import type { OutboundMediaAccess } from "../../media/load-options.js"; import type { ChatType } from "../chat-type.js"; import type { InboundEventKind } from "../inbound-event/kind.js"; +import type { IdentifierAuthentication } from "../message-access/identifier-authentication.js"; import type { ChannelMessageSendPollContext, MessageReceipt, @@ -310,6 +311,7 @@ export type ChannelSecurityDmPolicy = { allowFromPath: string; approveHint: string; normalizeEntry?: (raw: string) => string; + classifyEntryAuthentication?: (raw: string) => IdentifierAuthentication | undefined; }; export type ChannelSecurityContext = { diff --git a/src/plugin-sdk/channel-config-helpers.ts b/src/plugin-sdk/channel-config-helpers.ts index e1cd80a805bf..b64d117b1d8f 100644 --- a/src/plugin-sdk/channel-config-helpers.ts +++ b/src/plugin-sdk/channel-config-helpers.ts @@ -18,6 +18,7 @@ import { } from "../channels/plugins/config-write-policy-shared.js"; import { buildAccountScopedDmSecurityPolicy } from "../channels/plugins/helpers.js"; import type { ChannelConfigAdapter } from "../channels/plugins/types.adapters.js"; +import type { ChannelSecurityDmPolicy } from "../channels/plugins/types.core.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../routing/session-key.js"; @@ -574,6 +575,7 @@ export function createScopedDmSecurityResolver< approveChannelId?: string; approveHint?: string; normalizeEntry?: (raw: string) => string; + classifyEntryAuthentication?: ChannelSecurityDmPolicy["classifyEntryAuthentication"]; inheritSharedDefaultsFromDefaultAccount?: boolean; }) { return ({ @@ -599,6 +601,7 @@ export function createScopedDmSecurityResolver< approveChannelId: params.approveChannelId, approveHint: params.approveHint, normalizeEntry: params.normalizeEntry, + classifyEntryAuthentication: params.classifyEntryAuthentication, inheritSharedDefaultsFromDefaultAccount: params.inheritSharedDefaultsFromDefaultAccount, }); }; diff --git a/src/plugin-sdk/channel-ingress-runtime.ts b/src/plugin-sdk/channel-ingress-runtime.ts index 0796c706baa5..197a60a6df7d 100644 --- a/src/plugin-sdk/channel-ingress-runtime.ts +++ b/src/plugin-sdk/channel-ingress-runtime.ts @@ -21,7 +21,10 @@ export { resolveStableChannelMessageIngress, } from "../channels/message-access/runtime.js"; export type { IdentifierAuthentication } from "../channels/message-access/identifier-authentication.js"; -export { defineStableChannelIngressIdentity } from "../channels/message-access/runtime-identity.js"; +export { + defineStableChannelIngressIdentity, + identityEntryAuthenticationClassifier, +} from "../channels/message-access/runtime-identity.js"; export { readChannelIngressStoreAllowFromForDmPolicy } from "../channels/message-access/store-allow-from.js"; export { resolveChannelImplicitMentions } from "../config/implicit-mentions.js"; export type { diff --git a/src/plugin-sdk/channel-policy.test.ts b/src/plugin-sdk/channel-policy.test.ts index 3886ef00048d..a0c3208ea7d8 100644 --- a/src/plugin-sdk/channel-policy.test.ts +++ b/src/plugin-sdk/channel-policy.test.ts @@ -147,6 +147,7 @@ describe("mutable allowlist table helpers", () => { expect(detector("demo:user:U123")).toBe(false); expect(detector("demo:alice")).toBe(true); expect(detector("*")).toBe(false); + expect(detector("accessGroup:operators")).toBe(false); }); }); @@ -173,6 +174,7 @@ describe("createRestrictSendersChannelSecurity", () => { mentionGated: false, findingTitle: "LINE security warning", policyPathSuffix: "dmPolicy", + classifyEntryAuthentication: () => "asserted", dmRouting, }); @@ -195,8 +197,19 @@ describe("createRestrictSendersChannelSecurity", () => { allowFromPath: "channels.line.", approveHint: formatPairingApproveHint("line"), normalizeEntry: undefined, + classifyEntryAuthentication: expect.any(Function), }); + expect( + security + .resolveDmPolicy?.({ + cfg: {}, + accountId: "default", + account: { accountId: "default" }, + }) + ?.classifyEntryAuthentication?.("line:user:abc"), + ).toBe("asserted"); + expect( security.collectWarnings?.({ cfg: { channels: { line: {} } } as never, diff --git a/src/plugin-sdk/channel-policy.ts b/src/plugin-sdk/channel-policy.ts index 4ea52aff494b..221263472e47 100644 --- a/src/plugin-sdk/channel-policy.ts +++ b/src/plugin-sdk/channel-policy.ts @@ -5,11 +5,13 @@ import { uniqueStrings, } from "../../packages/normalization-core/src/string-normalization.js"; import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js"; +import { parseAccessGroupAllowFromEntry } from "../channels/allow-from.js"; import { createAllowlistProviderRestrictSendersWarningCollector, createConditionalWarningCollector, } from "../channels/plugins/group-policy-warnings.js"; import type { ChannelSecurityAdapter } from "../channels/plugins/types.adapters.js"; +import type { ChannelSecurityDmPolicy } from "../channels/plugins/types.core.js"; import { collectProviderDangerousNameMatchingScopes } from "../config/dangerous-name-matching.js"; import type { GroupPolicy } from "../config/types.base.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; @@ -267,7 +269,7 @@ export function buildMutableAllowEntryDetector(params: { const prefixes = (params.prefixes ?? []).filter((prefix) => prefix.length > 0); return (entry) => { const text = entry.trim(); - if (!text || text === "*") { + if (!text || text === "*" || parseAccessGroupAllowFromEntry(text) !== null) { return false; } const normalized = stripMutableAllowEntryPrefixes(text, prefixes); @@ -395,6 +397,7 @@ export function createRestrictSendersChannelSecurity< approveHint?: string; /** Normalizes configured DM allowlist entries before sender matching. */ normalizeDmEntry?: (raw: string) => string; + classifyEntryAuthentication?: ChannelSecurityDmPolicy["classifyEntryAuthentication"]; /** Allows non-default accounts to inherit shared defaults from the default account. */ inheritSharedDefaultsFromDefaultAccount?: boolean; dmRouting?: ChannelSecurityAdapter["dmRouting"]; @@ -426,6 +429,7 @@ export function createRestrictSendersChannelSecurity< approveChannelId: params.approveChannelId, approveHint: params.approveHint, normalizeEntry: params.normalizeDmEntry, + classifyEntryAuthentication: params.classifyEntryAuthentication, inheritSharedDefaultsFromDefaultAccount: params.inheritSharedDefaultsFromDefaultAccount, }), ...(params.dmRouting ? { dmRouting: params.dmRouting } : {}), diff --git a/src/plugin-sdk/core.test.ts b/src/plugin-sdk/core.test.ts index 42fdacd81600..a90a86b20433 100644 --- a/src/plugin-sdk/core.test.ts +++ b/src/plugin-sdk/core.test.ts @@ -183,7 +183,7 @@ describe("createChannelPluginBase", () => { }); describe("createChatChannelPlugin", () => { - it("preserves DM routing through the declarative security shorthand", () => { + it("preserves DM routing and entry classification through the security shorthand", () => { const dmRouting = { resolveDmScope: () => "per-peer" as const, resolveDmRoute: () => ({ kind: "core" as const }), @@ -195,12 +195,19 @@ describe("createChatChannelPlugin", () => { channelKey: "security-routing", resolvePolicy: () => "allowlist", resolveAllowFrom: () => [], + classifyEntryAuthentication: () => "mutable", }, dmRouting, }, }); expect(plugin.security?.dmRouting).toBe(dmRouting); + const policy = plugin.security?.resolveDmPolicy?.({ + cfg: {}, + accountId: "default", + account: { accountId: "default" }, + }); + expect(policy?.classifyEntryAuthentication?.("alias")).toBe("mutable"); }); it("preserves account-scoped current-conversation binding support", () => { diff --git a/src/plugin-sdk/core.ts b/src/plugin-sdk/core.ts index 035c407dfa8b..3159f243ef6d 100644 --- a/src/plugin-sdk/core.ts +++ b/src/plugin-sdk/core.ts @@ -21,6 +21,7 @@ import type { ChannelMessagingAdapter, ChannelOutboundSessionRoute, ChannelPollResult, + ChannelSecurityDmPolicy, ChannelThreadingAdapter, } from "../channels/plugins/types.core.js"; import type { ChannelPlugin } from "../channels/plugins/types.plugin.js"; @@ -630,6 +631,7 @@ type ChatChannelSecurityOptions string; + classifyEntryAuthentication?: ChannelSecurityDmPolicy["classifyEntryAuthentication"]; inheritSharedDefaultsFromDefaultAccount?: boolean; }; dmRouting?: ChannelSecurityAdapter["dmRouting"]; @@ -740,6 +742,7 @@ function resolveChatChannelSecurity ({ + readChannelIngressStoreAllowFromForDmPolicy: async () => ["stored-only#9999"], +})); + +const classifyEntryAuthentication = identityEntryAuthenticationClassifier({ + key: "stable-id", + kind: "stable-id", + authentication: "verified", + normalizeEntry: (value) => (/^\d+$/.test(value.trim()) ? value.trim() : null), + aliases: [ + { + key: "display-name", + kind: "username", + authentication: "mutable", + normalizeEntry: (value) => { + const text = value.trim(); + return text && !/^\d+$/.test(text) ? text.toLowerCase() : null; + }, + }, + ], +}); + +function createFixturePlugin(params: { + accounts: Record< + string, + { + allowFrom: Array; + dangerouslyAllowNameMatching?: boolean; + } + >; +}): ChannelPlugin { + const accountIds = Object.keys(params.accounts); + return { + id: "whatsapp", + meta: { + id: "whatsapp", + label: "WhatsApp", + selectionLabel: "WhatsApp", + docsPath: "/channels/whatsapp", + blurb: "Test", + }, + capabilities: { chatTypes: ["direct"] }, + config: { + listAccountIds: () => accountIds, + defaultAccountId: () => accountIds[0] ?? "default", + inspectAccount: (_cfg, accountId) => { + const resolvedAccountId = accountId ?? "default"; + return { + accountId: resolvedAccountId, + enabled: true, + configured: true, + config: { + dangerouslyAllowNameMatching: + params.accounts[resolvedAccountId]?.dangerouslyAllowNameMatching, + }, + }; + }, + resolveAccount: (_cfg, accountId) => { + const resolvedAccountId = accountId ?? "default"; + return { + accountId: resolvedAccountId, + config: { + dangerouslyAllowNameMatching: + params.accounts[resolvedAccountId]?.dangerouslyAllowNameMatching, + }, + }; + }, + isEnabled: () => true, + isConfigured: () => true, + }, + security: { + resolveDmPolicy: ({ accountId }: { accountId?: string | null }) => { + const resolvedAccountId = accountId ?? "default"; + const account = params.accounts[resolvedAccountId]; + if (!account) { + return null; + } + const scope = resolvedAccountId === "default" ? "" : `accounts.${resolvedAccountId}.`; + return { + policy: "allowlist", + allowFrom: account.allowFrom, + policyPath: `channels.whatsapp.${scope}dmPolicy`, + allowFromPath: `channels.whatsapp.${scope}`, + approveHint: `approve ${resolvedAccountId}`, + classifyEntryAuthentication, + }; + }, + }, + }; +} + +describe("channel entry authentication audit", () => { + const stableId = "123456789012345678"; + const name = "Alice Example"; + const inertCheckId = "channels.whatsapp.allowFrom.mutable_entries_inert"; + const cfg: OpenClawConfig = { session: { dmScope: "per-account-channel-peer" } }; + + it.each([ + { caseName: "default policy", allowFrom: [name, stableId], enabled: undefined, count: 1 }, + { caseName: "stable IDs only", allowFrom: [stableId], enabled: false, count: 0 }, + { + caseName: "access group reference", + allowFrom: ["accessGroup:operators"], + enabled: false, + count: 0, + }, + { + caseName: "access group preview", + allowFrom: ["accessGroup:operators", name], + enabled: true, + count: 1, + }, + { caseName: "wildcard excluded", allowFrom: [name, stableId, "*"], enabled: false, count: 1 }, + { caseName: "lockout preview", allowFrom: [name, stableId], enabled: true, count: 1 }, + { caseName: "zero lockout preview", allowFrom: [stableId], enabled: true, count: 0 }, + ])("reports counts without identifiers: $caseName", async ({ allowFrom, enabled, count }) => { + const plugin = createFixturePlugin({ + accounts: { default: { allowFrom, dangerouslyAllowNameMatching: enabled } }, + }); + const findings = await collectChannelSecurityFindingsCore({ cfg, plugins: [plugin] }); + + const configuredEntries = allowFrom.filter((raw) => raw !== "*"); + const inert = findings.find((finding) => finding.checkId === inertCheckId); + if (!enabled && count > 0) { + expect(inert).toMatchObject({ severity: "warn" }); + expect(inert?.detail).toContain( + `${count} of ${configuredEntries.length} entries in channels.whatsapp.allowFrom`, + ); + expect(inert?.detail).toContain("silently inert"); + } else { + expect(inert).toBeUndefined(); + } + if (enabled) { + const preview = findings.find((finding) => + finding.checkId.endsWith("dangerous_name_matching_enabled"), + ); + expect(preview?.detail).toContain( + `${count} of ${configuredEntries.length} allowFrom entries depend on mutable matching`, + ); + expect(preview?.detail).toContain("mutable_identifier_disabled"); + expect(preview?.detail).toContain("identifier_authentication_too_weak"); + } + const serialized = JSON.stringify(findings); + for (const raw of [name, stableId, "stored-only#9999"]) { + expect(serialized).not.toContain(raw); + } + }); + + it("uses the account policy and exposes inert entries to doctor", async () => { + const plugin = createFixturePlugin({ + accounts: { + default: { allowFrom: [stableId], dangerouslyAllowNameMatching: true }, + work: { allowFrom: [name], dangerouslyAllowNameMatching: false }, + }, + }); + const findings = await collectChannelSecurityFindingsCore({ + cfg, + plugins: [plugin], + mode: "doctor", + }); + expect(findings.find((finding) => finding.checkId === inertCheckId)).toMatchObject({ + severity: "warn", + detail: expect.stringContaining( + "1 of 1 entries in channels.whatsapp.accounts.work.allowFrom", + ), + }); + expect(JSON.stringify(findings)).not.toContain(name); + }); + + it("classifies through the identity declaration exactly once", () => { + expect(classifyEntryAuthentication(stableId)).toBe("verified"); + expect(classifyEntryAuthentication(name)).toBe("mutable"); + expect(classifyEntryAuthentication("*")).toBeUndefined(); + }); +}); diff --git a/src/security/audit-channel.ts b/src/security/audit-channel.ts index 5c0a74c75b76..e9435f834b1b 100644 --- a/src/security/audit-channel.ts +++ b/src/security/audit-channel.ts @@ -6,6 +6,7 @@ import { hasConfiguredUnavailableCredentialStatus, hasResolvedCredentialValue, } from "../channels/account-snapshot-fields.js"; +import { parseAccessGroupAllowFromEntry } from "../channels/allow-from.js"; import { resolveDmAllowAuditState } from "../channels/message-access/dm-allow-state.js"; import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js"; import type { ChannelPlugin } from "../channels/plugins/types.plugin.js"; @@ -340,23 +341,46 @@ export async function collectChannelSecurityFindingsCore(params: { }); const accountConfig = (account as { config?: Record } | null | undefined) ?.config; - if (includeAuditOnly && isDangerousNameMatchingEnabled(accountConfig)) { - findings.push({ - checkId: `channels.${plugin.id}.allowFrom.dangerous_name_matching_enabled`, - severity: "info", - title: `${plugin.meta.label ?? plugin.id} dangerous name matching is enabled${accountNote}`, - detail: - "dangerouslyAllowNameMatching=true re-enables mutable name/email/tag matching for sender authorization. This is a break-glass compatibility mode, not a hardened default.", - remediation: - "Prefer stable sender IDs in allowlists, then disable dangerouslyAllowNameMatching.", - }); - } - const dmPolicy = plugin.security.resolveDmPolicy?.({ cfg: params.cfg, accountId, account, }); + const nameMatchingEnabled = isDangerousNameMatchingEnabled(accountConfig); + const configuredEntries = (dmPolicy?.allowFrom ?? []) + .map(String) + .filter((raw) => raw.trim() !== "*"); + const mutableEntries = configuredEntries.filter( + // Symbolic groups resolve membership separately; they are not identifier entries. + (raw) => + parseAccessGroupAllowFromEntry(raw) === null && + dmPolicy?.classifyEntryAuthentication?.(raw) === "mutable", + ).length; + if (includeAuditOnly && nameMatchingEnabled) { + findings.push({ + checkId: `channels.${plugin.id}.allowFrom.dangerous_name_matching_enabled`, + severity: "info", + title: `${plugin.meta.label ?? plugin.id} dangerous name matching is enabled${accountNote}`, + detail: + "dangerouslyAllowNameMatching=true enables mutable aliases (changeable/shared labels, weak even when honestly set) for sender authorization. Exact, stable identifiers with unproven ownership are a separate weak class; ingress diagnostics distinguish mutable_identifier_disabled from identifier_authentication_too_weak." + + (dmPolicy?.classifyEntryAuthentication + ? ` ${mutableEntries} of ${configuredEntries.length} allowFrom entries depend on mutable matching and would stop authorizing if dangerouslyAllowNameMatching is disabled.` + : ""), + remediation: + "Prefer stable sender IDs in allowlists, then disable dangerouslyAllowNameMatching.", + }); + } + + if (!nameMatchingEnabled && mutableEntries > 0 && dmPolicy) { + findings.push({ + checkId: `channels.${plugin.id}.allowFrom.mutable_entries_inert`, + severity: "warn", + title: `${plugin.meta.label ?? plugin.id} mutable allowFrom entries are inert${accountNote}`, + detail: `${mutableEntries} of ${configuredEntries.length} entries in ${dmPolicy.allowFromPath}allowFrom only match mutable identifiers (display names/tags/aliases) and can never authorize a sender under the current policy, so they are silently inert.`, + remediation: + "Replace them with stable sender IDs; enabling dangerouslyAllowNameMatching is a discouraged break-glass alternative.", + }); + } if (dmPolicy) { const auditState = await warnDmPolicy({ label: `${plugin.meta.label ?? plugin.id}${accountNote}`,