security(clawrouter): require configurable owner for /wallet and /stats; set /stats requireAuth true#22
security(clawrouter): require configurable owner for /wallet and /stats; set /stats requireAuth true#22hubofvalley wants to merge 1 commit intoBlockRunAI:mainfrom
Conversation
…ts; set /stats requireAuth true - Add isAllowedUser() and resolveAllowedUsers() guard helpers - Set /stats requireAuth to true (matches /wallet) - Implement owner guard in both command handlers - Resolve allowed IDs from CLAWROUTER_ALLOWED_USERS env var or plugin config - Improved "secure by default" behavior (blocks if no users configured)
|
Thanks for the security improvement! The direction is right, but I have some concerns before merging: 1. Breaking Change Without Migration PathThis will immediately block all existing users who haven't configured Suggestion: Consider backwards compatibility - either allow unrestricted access when no users are configured (with a deprecation warning), or make this an opt-in feature. 2. Does
|
Summary of Changes:
Dynamic Owner List: Added isAllowedUser() and resolveAllowedUsers() to handle configurable owner IDs.
Improved Security: /stats now requires authentication (requireAuth: true) and both /stats and /wallet now enforce the owner check.
Flexible Matching: Supports both exact IDs (e.g., 123) and platform-prefixed IDs (e.g., telegram:123).
Secure Default: If no users are configured, sensitive commands return a clear configuration error instead of potentially exposing data.
How to use: Set the allowed users via environment variable or plugin config:
export CLAWROUTER_ALLOWED_USERS="telegram:123,whatsapp:456"
Or add allowedUsers: ["telegram:123"] to your OpenClaw plugin configuration.