T09 · Insecure Skill Coding Practices
Warning
- Location
scripts/odoo_mcp.py:132- Finding
Live ERP Mutations Are Not Enforced by a Technical Confirmation Gate
- Content
View full analysis
Vulnerability Details
File Location:
scripts/odoo_mcp.py:132-157
Vulnerability Type: Missing authorization and confirmation enforcement for mutating operations
Risk Level: MediumComplete Code Snippet
python async def cmd_call(args) -> int: if args.json_args is not None: raw = args.json_args elif not sys.stdin.isatty(): raw = sys.stdin.read().strip() else: raw = "" raw = raw or "{}" try: arguments = json.loads(raw) except json.JSONDecodeError as e: return _fail("invalid_arguments", f"Tool arguments must be valid JSON: {e}", code=2) if not isinstance(arguments, dict): return _fail("invalid_arguments", "Tool arguments must be a JSON object.", code=2) async def fn(session): result = await session.call_tool(args.tool, arguments) payload = result.model_dump(mode="json", exclude_none=True) print(json.dumps(payload, indent=2)) # 1.x exposes camelCase `isError`; 2.x snake_case `is_error`. is_error = getattr(result, "isError", None) if is_error is None: is_error = getattr(result, "is_error", False) return 1 if is_error else 0 return await _run(fn)Related write-capable legacy entry points include:
python # scripts/sales.py:42-45 def _create_order(client: OdooClient, args: dict[str, Any]) -> Any: if not args: raise ValueError("create-order requires an order object on stdin.") return client.create_sale_order(args)python # scripts/ap.py:54-69 def _update_po_lines(client: OdooClient, args: dict[str, Any]) -> Any: return client.update_po_lines( _require_int(args, "po_id"), lines=args.get("lines"), freight_cost=args.get("freight_cost"), fees_cost=args.get("fees_cost"), ) def _create_bill(client: OdooClient, args: dict[str, Any]) -> ...[truncated 4094 chars]- Remediation
View remediation
Remediation Suggestions
- Classify every available MCP tool as read-only or mutating using a strict local allowlist; do not rely solely on remote tool descriptions.
- Require mutating calls to include an explicit approval artifact bound to the action, target object, material parameters, requesting user, and short expiration time.
- Validate and consume that approval before calling
session.call_tool; reject absent, expired, reused, or mismatched approvals. - Default all legacy clients to dry-run and require an explicit, narrowly scoped flag to enable live writes.
- Add dry-run handling to every mutating method, including
update_po_lines, so behavior is consistent. - Separate read and write credentials where supported, granting the default execution path only read access.
- Add transaction summaries and server-side audit records containing the approving identity and approved parameters.
- Add automated tests proving that each mutating MCP tool and legacy action fails closed without valid approval.