T09 · Insecure Skill Coding Practices
Warning
- Location
- index.js:43
- Finding
- Webhook credentials are exposed through logs and returned objects<![CDATA[ ## Vulnerability Details **File Location**: `index.js:26-43`, `index.js:94-102`, and `index.js:263-269` **Vulnerability Type**: Sensitive credential exposure **Risk Level**: Medium ### Technical Analysis The webhook URL contains a bot token and is explicitly identified as sensitive in `SKILL.md`. Although `_maskWebhook()` exists, several code paths expose the unmasked URL. The constructor logs the complete configuration, including `defaultWebhook`: ```javascript this.config = { enabled: config.enabled !== false, defaultWebhook: config.defaultWebhook || '', timeout: config.timeout || 10000, retryCount: config.retryCount || 3, retryDelay: config.retryDelay || 1000, ...config }; // HTTP client this.httpClient = axios.create({ timeout: this.config.timeout, headers: { 'Content-Type': 'application/json', 'User-Agent': 'OpenClaw-Qywx-Notify/1.0.0' } }); this.log(`Skill initialized with config: ${JSON.stringify(this.config, null, 2)}`); ``` When message delivery fails, `send()` returns the original parameter object. If the caller supplied a webhook through `params.webhook`, its token is returned without masking: ```javascript } catch (error) { this.error('Failed to send notification:', error.message); return { success: false, message: `Send failed: ${error.message}`, error: error.response?.data || error.message, request: params }; } ``` The `config` command similarly returns the entire unredacted configuration even though it also provides a separately masked value: ```javascript case 'config': return { success: true, config: this.config, maskedWebhook: this.config.defaultWebhook ? this._maskWebhook(this.config.defaultWebhook) : null }; ``` A WeCom webhook URL is a bearer credential: possession of the URL and embedded bot token may be sufficient to submit messages as the configured bot. Masking only the separately generated `maskedWebhook` field does not protect the raw value still pre ...[truncated 1457 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Never serialize the full configuration object. Log only an explicit allowlist of non-sensitive fields: ```javascript this.log('Skill initialized', { enabled: this.config.enabled, timeout: this.config.timeout, retryCount: this.config.retryCount, hasDefaultWebhook: Boolean(this.config.defaultWebhook) }); ``` 2. Remove `config: this.config` from the `config` command. Return a sanitized object instead: ```javascript case 'config': return { success: true, config: { enabled: this.config.enabled, timeout: this.config.timeout, retryCount: this.config.retryCount, retryDelay: this.config.retryDelay, hasDefaultWebhook: Boolean(this.config.defaultWebhook) }, maskedWebhook: this.config.defaultWebhook ? this._maskWebhook(this.config.defaultWebhook) : null }; ``` 3. Sanitize failed request information before returning it: ```javascript request: { ...params, webhook: params.webhook ? this._maskWebhook(params.webhook) : undefined } ``` Prefer returning only fields that are necessary for troubleshooting. 4. Apply centralized redaction to URLs, authorization values, tokens, and configuration fields before logging or sending telemetry. 5. Restrict access to existing logs and delete retained entries containing webhook URLs where operationally feasible. 6. Rotate any webhook credential that may already have appeared in logs or command responses. 7. Add tests asserting that complete webhook tokens never occur in logs, successful responses, failed responses, or configuration output. ]]>
