T09 · Insecure Skill Coding Practices
- Location
engine/api/mcp_server.py:183- Finding
Destructive cloud operations lack code-enforced user confirmation
- Content
View full analysis
Vulnerability Details
File Location:
engine/api/mcp_server.py:118-120, 183-195, 279-281;engine/api/tools.py:109-114, 154-171
Vulnerability Type: Missing authorization gate for destructive operations
Risk Level: HighVulnerable Code
python @mcp.tool() async def kdoc_file_delete(file_id: str) -> str: return await _wrap("kdoc_file_delete")(file_id) @mcp.tool() async def kdoc_trash_destroy(file_id: str) -> str: return await _wrap("kdoc_trash_destroy")(file_id) @mcp.tool() async def kdoc_version_restore(file_id: str, version: int) -> str: return await _wrap("kdoc_version_restore")(file_id, version) @mcp.tool() async def kdoc_file_permission(file_id: str, members: List[Dict]) -> str: return await _wrap("kdoc_file_permission")(file_id, members)The corresponding backend methods immediately perform authenticated state-changing requests:
python def kdoc_file_delete(self, file_id: str) -> Dict: if not validate_file_id(file_id): raise KingDocError("KD011", "非法 file_id 格式") resp = self.client.delete(f"/personal/files/{file_id}") return resp def kdoc_trash_destroy(self, file_id: str) -> Dict: resp = self.client.post("/personal/trash/destroy", json={"file_id": file_id}) return resp def kdoc_version_restore(self, file_id: str, version: int) -> Dict: resp = self.client.post( f"/appspace/versions/{version}/restore", json={"file_id": file_id}, ) return resp def kdoc_file_permission(self, file_id: str, members: list) -> Dict: resp = self.client.put( f"/personal/files/{file_id}/permissions", json={"members": members}, ) return respTechnical Analysis
The Skill documentation states that deletion, permanent trash destruction, version restoration, and permission changes require explicit user confirmation. Permanent deletion is documented as requiring secondary confirmation.
The executable MCP entry points do not enforce those requi ...[truncated 2047 chars]
- Remediation
View remediation
Remediation Suggestions
- Add a mandatory confirmation object to every destructive or authorization-changing MCP tool.
- Implement a two-step workflow:
- A preview endpoint returns the exact operation, affected resources, and consequences.
- A separate execution endpoint requires a server-generated confirmation token.
- Bind each confirmation token to:
- The authenticated user or session.
- The operation name.
- Target file identifiers.
- All operation parameters.
- A short expiration time.
- A single-use nonce.
- Require a distinct second confirmation token for irreversible deletion.
- Enforce confirmation in the backend immediately before the API request so alternate entry points cannot bypass it.
- Implement
dry_runas an executable backend control rather than relying on agent instructions. - Record confirmation issuance and consumption in an audit log without storing authentication credentials.
- Add tests proving that destructive backend methods reject direct calls without valid approval state.
