T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/auth.py:60
- Finding
- API key stored without explicit restrictive permissions and partially disclosed<![CDATA[ ## Vulnerability Details **File Location**: `scripts/auth.py`, lines 60-74 **Vulnerability Type**: Plaintext credential storage with insufficient file-permission hardening **Risk Level**: Medium ### Vulnerable Code ```python # 确保 ~/.upkuajing 目录存在 try: UPKUAJING_DIR.mkdir(parents=True, exist_ok=True) except OSError as e: return { "success": False, "message": f"API密钥申请成功,但创建目录失败:{str(e)}。\n请手动创建目录 {UPKUAJING_DIR} 并设置环境变量 {API_KEY_ENV}。", "envFilePath": str(env_file) } # 保存到 .env 文件 try: with open(env_file, 'w', encoding='utf-8') as f: f.write(f"{API_KEY_ENV}={api_key}\n") ``` Related credential disclosure occurs at `scripts/auth.py`, lines 24-35: ```python if line.startswith(f'{API_KEY_ENV}='): existing_key = line.split('=', 1)[1].strip() if existing_key: return { "success": False, "message": f"错误: {env_file} 中已存在API密钥({existing_key[:10]}...)。\n如需重新申请,请先删除文件中的 {API_KEY_ENV} 后再运行此命令。", "envFilePath": str(env_file) } ``` The accompanying instructions at `SKILL.md`, lines 67-76, also encourage printing the credential file: ```bash cat ~/.upkuajing/.env ``` ### Technical Analysis The API key is legitimately required to authenticate requests to the declared UpKuaJing service, so reading the specific `UPKUAJING_API_KEY` environment variable is within the minimum privilege needed by the skill. However, the fallback storage implementation does not explicitly secure either the directory or the credential file. `Path.mkdir()` and `open(..., 'w')` rely on the process umask. Under a common umask of `022`, the directory can be created with mode `0755` and the file with mode `0644`, allowing other local users to traverse the directory and read the plaintext API key. The code does not check whether the existing path is a symbolic link, either, so execution in a locally compromised account may overwrite a link target. The write operation also truncates ...[truncated 1743 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Prefer an injected environment variable or operating-system credential store instead of a plaintext fallback file. 2. If file storage remains necessary: - Create `~/.upkuajing` with mode `0700`. - Create the credential file atomically with mode `0600`, such as through `os.open()` using `O_CREAT | O_EXCL | O_NOFOLLOW`. - Verify that the directory and file are owned by the current user and are not symbolic links. - Apply `chmod(0o600)` to an existing file before reading or writing it. 3. Update only the `UPKUAJING_API_KEY` entry while preserving unrelated file content. Use an atomic temporary-file replacement inside the protected directory. 4. Never include any API-key prefix in output. Report only that a key is already configured. 5. Replace the documented `cat` instruction with a non-disclosing existence check, for example checking whether the parsed variable is nonempty. 6. Clearly document key revocation and rotation procedures for users who suspect transcript or filesystem exposure. ]]>
