T09 · Insecure Skill Coding Practices
- Location
- executor.py:251
- Finding
- Credential Filtering Is Bypassed by the Live Delegation Path<![CDATA[ ## Vulnerability Details **File Location**: `executor.py:251` and `executor.py:281-289` **Vulnerability Type**: Security-control bypass causing sensitive-data disclosure **Risk Level**: High ### Vulnerable Code ```python # Get routing decision decision = self.router.classify(message, context_tokens) ``` ```python if should_delegate: self._task_counter += 1 task_id = f"router-{self._task_counter}-{int(time.time())}" agent_id = self.AGENT_IDS.get(recommended) spawn_params = { "task": message, "label": f"router-{decision.intent.name.lower()}-{recommended}", } ``` The protected gateway path sanitizes requests before processing: ```python # Sanitize input sanitized = self.sanitizer.sanitize(text) if sanitized.blocked: return RouterResponse( content=f"❌ Request blocked: {sanitized.block_reason}", model_used="none", routing_decision=RoutingDecision( intent=Intent.GENERAL, complexity=Complexity.SIMPLE, selected_model="blocked", fallback_chain=[], reason=sanitized.block_reason or "Security block" ) ) ``` ### Technical Analysis `RouterExecutor.analyze()` calls `SmartRouter.classify()` directly instead of the protected `SmartRouter.route_request()` entry point. The `classify()` method performs routing classification but does not invoke `InputSanitizer`. When delegation is selected, the executor copies the original, unfiltered `message` directly into `spawn_params["task"]`. The Agent is then expected to submit this value through OpenClaw's `sessions_spawn` mechanism. This bypasses the controls enabled by `router_config.json`: ```json "security": { "sanitize_input": true, "block_credentials": true, "warn_pii": true } ``` It also contradicts the security guarantee in `references/security.md:194` that no unsanitized input reaches a model API. Credential patterns intended to be blocked include Anthropic, OpenAI, xAI ...[truncated 1730 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Establish one mandatory input-validation entry point shared by every routing and execution path. 2. Sanitize before classification and delegation: ```python sanitized = self.router.sanitizer.sanitize(message) if sanitized.blocked: raise SecurityError(sanitized.block_reason) safe_message = sanitized.text decision = self.router.classify(safe_message, context_tokens) ``` 3. Populate delegated tasks only with the sanitized value: ```python spawn_params = { "task": safe_message, "label": f"router-{decision.intent.name.lower()}-{recommended}", } ``` 4. Return a structured blocked execution plan instead of relying on an exception if that better fits the integration API. 5. Prevent direct use of `classify()` for execution by clearly separating pure classification from the protected dispatch interface. 6. Apply the same validation to fallback and retry paths so they cannot reintroduce the original raw message. 7. Enforce configuration flags such as `sanitize_input` and `block_credentials` in executable code rather than treating them as documentation-only settings. 8. Add regression tests covering every configured credential pattern through: - `SmartRouter.route_request()` - `RouterExecutor.analyze()` - `ExecutorAgent.should_delegate()` - Fallback delegation 9. Add tests asserting that `spawn_params["task"]` never contains a blocked credential or disallowed control character. 10. Consider explicit user confirmation before sending sensitive but non-blocked content to a provider with a different trust tier. ]]>
