T09 · Insecure Skill Coding Practices
- Location
- scripts/cli.mjs:323
- Finding
- Bearer Token Can Be Sent to an Arbitrary Host or over Plaintext HTTP<![CDATA[ ## Vulnerability Details **File Location**: `scripts/cli.mjs:323-352` and `scripts/cli.mjs:568-599` **Vulnerability Type**: Unrestricted credential destination and optional plaintext transmission **Risk Level**: Medium ### Vulnerable Code ```js function readEnv(lang) { const m_ = msg(lang); const baseUrl = process.env.CHINA_TM_PLATFORM_BASE_URL; const token = process.env.CHINA_TM_USER_TOKEN; const timeoutMsRaw = process.env.CHINA_TM_TIMEOUT_MS; const channel = process.env.CHINA_TM_SKILL_CHANNEL || DEFAULT_CHANNEL; const allowHttp = process.env.ALLOW_HTTP === 'true'; if (!baseUrl) { throw createSkillError('ENV_MISSING', m_.envMissingBase); } if (!token) { throw createSkillError('ENV_MISSING', m_.envMissingToken); } const timeoutMs = parsePositiveInteger(timeoutMsRaw, DEFAULT_TIMEOUT_MS, 'CHINA_TM_TIMEOUT_MS', lang); const normalizedBaseUrl = baseUrl.replace(/\/+$/, ''); if (!/^https:\/\//i.test(normalizedBaseUrl)) { if (!allowHttp) { throw createSkillError('HTTPS_REQUIRED', m_.httpsRequired); } // baseUrl 完全由使用者控制,而每个请求都会带上 Bearer token; // 明文降级至少要让使用者在 stderr 上看见一次。 process.stderr.write(`${m_.allowHttpWarning}\n`); } return { baseUrl: normalizedBaseUrl, token, timeoutMs, channel, lang, requestId: `cli_${randomUUID().replace(/-/g, '')}` }; } ``` ```js async function apiRequest(env, method, path, body) { const m_ = msg(env.lang || 'zh'); const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), env.timeoutMs); const headers = { 'Accept': 'application/json', 'Authorization': `Bearer ${env.token}`, 'X-OC-Request-Id': env.requestId }; if (body !== undefined) { headers['Content-Type'] = 'application/json'; } const requestInit = { method, headers, signal: controller.signal }; if (body !== undefined) { requestInit.body = JSON.stringify(body); } let response; let ...[truncated 3117 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Pin the production API origin** - Use `https://tm.zhengquai.com` as an internal constant rather than accepting an unrestricted production base URL. - Parse the destination with `new URL()` and compare its complete origin against an explicit allowlist. - Reject unexpected hostnames, ports, protocols, embedded credentials, and malformed URLs. 2. **Remove unrestricted plaintext transport** - Remove `ALLOW_HTTP` from production builds. - If plaintext HTTP is required for tests, permit it only for loopback addresses such as `127.0.0.1`, `::1`, and `localhost`. - Require separate, non-production test credentials for development endpoints. 3. **Separate production and development configuration** - Place custom endpoints behind an explicit development mode that is disabled by default. - Display a blocking confirmation or fail closed when a production-format `tmu_` credential is paired with a development endpoint. - Do not rely on a warning as the sole credential protection. 4. **Validate redirects** - Disable automatic redirects or validate every redirect destination before following it. - Never forward the authorization header to an origin that has not been explicitly approved. 5. **Add regression tests** - Verify that arbitrary HTTPS hosts are rejected. - Verify that non-loopback HTTP endpoints are rejected even when a development option is enabled. - Verify that alternate ports, embedded URL credentials, deceptive subdomains, and malformed origins are rejected. - Continue verifying that tokens never appear in stdout, stderr, or error objects. A hardened validation pattern could use an exact origin comparison: ```js const PRODUCTION_ORIGIN = 'https://tm.zhengquai.com'; function validateBaseUrl(rawBaseUrl) { const parsed = new URL(rawBaseUrl); if (parsed.origin !== PRODUCTION_ORIGIN) { throw createSkillError( 'PLATFORM_ORIGIN_INVALID', `Platform origin must b ...[truncated 239 chars]
