security: fix critical vulnerabilities found in code review

installers/oi-mac-installer.sh

@@ -42,7 +42,7 @@ else
         Darwin)
             echo "Installing Git on macOS..."
             # Install Git using Xcode Command Line Tools
-            xcode-select --install
+            xcode-select --install || echo "Xcode Command Line Tools installation failed or already installed"
             ;;
         *)
             echo "Unsupported OS: $OS"

interpreter/computer_use/loop.py

@@ -110,6 +110,39 @@ class APIProvider(StrEnum):
 </IMPORTANT>"""
 
 
+async def _process_response_chunks(raw_response, response_content):
+    """Process response chunks and yield formatted output."""
+    current_block = None
+
+    for chunk in raw_response:
+        if isinstance(chunk, BetaRawContentBlockStartEvent):
+            current_block = chunk.content_block
+        elif isinstance(chunk, BetaRawContentBlockDeltaEvent):
+            if chunk.delta.type == "text_delta":
+                print(f"{chunk.delta.text}", end="", flush=True)
+                yield {"type": "chunk", "chunk": chunk.delta.text}
+                await asyncio.sleep(0)
+                if current_block and current_block.type == "text":
+                    current_block.text += chunk.delta.text
+            elif chunk.delta.type == "input_json_delta":
+                print(f"{chunk.delta.partial_json}", end="", flush=True)
+                if current_block and current_block.type == "tool_use":
+                    if not hasattr(current_block, "partial_json"):
+                        current_block.partial_json = ""
+                    current_block.partial_json += chunk.delta.partial_json
+        elif isinstance(chunk, BetaRawContentBlockStopEvent):
+            if current_block:
+                if hasattr(current_block, "partial_json"):
+                    current_block.input = json.loads(current_block.partial_json)
+                    delattr(current_block, "partial_json")
+                else:
+                    print("\n")
+                    yield {"type": "chunk", "chunk": "\n"}
+                    await asyncio.sleep(0)
+                response_content.append(current_block)
+                current_block = None
+
+
 async def sampling_loop(
     *,
     model: str,
@@ -162,37 +195,8 @@ async def sampling_loop(
         response_content = []
         current_block = None
 
-        for chunk in raw_response:
-            if isinstance(chunk, BetaRawContentBlockStartEvent):
-                current_block = chunk.content_block
-            elif isinstance(chunk, BetaRawContentBlockDeltaEvent):
-                if chunk.delta.type == "text_delta":
-                    print(f"{chunk.delta.text}", end="", flush=True)
-                    yield {"type": "chunk", "chunk": chunk.delta.text}
-                    await asyncio.sleep(0)
-                    if current_block and current_block.type == "text":
-                        current_block.text += chunk.delta.text
-                elif chunk.delta.type == "input_json_delta":
-                    print(f"{chunk.delta.partial_json}", end="", flush=True)
-                    if current_block and current_block.type == "tool_use":
-                        if not hasattr(current_block, "partial_json"):
-                            current_block.partial_json = ""
-                        current_block.partial_json += chunk.delta.partial_json
-            elif isinstance(chunk, BetaRawContentBlockStopEvent):
-                if current_block:
-                    if hasattr(current_block, "partial_json"):
-                        # Finished a tool call
-                        # print()
-                        current_block.input = json.loads(current_block.partial_json)
-                        # yield {"type": "chunk", "chunk": current_block.input}
-                        delattr(current_block, "partial_json")
-                    else:
-                        # Finished a message
-                        print("\n")
-                        yield {"type": "chunk", "chunk": "\n"}
-                        await asyncio.sleep(0)
-                    response_content.append(current_block)
-                    current_block = None
+        async for processed_chunk in _process_response_chunks(raw_response, response_content):
+            yield processed_chunk
 
         response = BetaMessage(
             id=str(uuid.uuid4()),

interpreter/core/async_core.py

@@ -856,7 +856,7 @@ async def chat_completion(request: ChatCompletionRequest):
             and last_message.content.lower().strip(".!?").strip() == "yes"
         ):
             run_code = True
-        elif type(last_message.content) == str:
+        elif isinstance(last_message.content, str):
             async_interpreter.messages.append(
                 {
                     "role": "user",
@@ -865,7 +865,7 @@ async def chat_completion(request: ChatCompletionRequest):
                 }
             )
             print(">", last_message.content)
-        elif type(last_message.content) == list:
+        elif isinstance(last_message.content, list):
             for content in last_message.content:
                 if content["type"] == "text":
                     async_interpreter.messages.append(

interpreter/core/computer/ai/ai.py

@@ -84,10 +84,10 @@ def fast_llm(llm, system_message, user_message):
         llm.interpreter.system_message = system_message
         llm.interpreter.messages = []
         response = llm.interpreter.chat(user_message)
+        return response[-1].get("content")
     finally:
         llm.interpreter.messages = old_messages
         llm.interpreter.system_message = old_system_message
-        return response[-1].get("content")
 
 
 def query_map_chunks(chunks, llm, query):

interpreter/core/computer/contacts/contacts.py

@@ -33,10 +33,10 @@ def get_phone_number(self, contact_name):
         if "Can’t get person" in stderr or not stout:
             names = self.get_full_names_from_first_name(contact_name)
             if "No contacts found" in names or not names:
-                raise Exception("Contact not found")
+                raise ValueError("Contact not found")
             else:
                 # Language model friendly error message
-                raise Exception(
+                raise ValueError(
                     f"A contact for '{contact_name}' was not found, perhaps one of these similar contacts might be what you are looking for? {names} \n Please try again and provide a more specific contact name."
                 )
         else:

interpreter/core/computer/display/display.py

@@ -269,7 +269,7 @@ def find(self, description, screenshot=None):
                     )
                     return response.json()
                 except Exception as e:
-                    raise Exception(
+                    raise ConnectionError(
                         str(e)
                         + "\n\nIcon locating API not available, or we were unable to find the icon. Please try another method to find this icon."
                     )
@@ -334,14 +334,19 @@ def get_text_as_list_of_lists(self, screenshot=None):
         try:
             return pytesseract_get_text(screenshot)
         except:
-            raise Exception(
+            raise RuntimeError(
                 "Failed to find text locally.\n\nTo find text in order to use the mouse, please make sure you've installed `pytesseract` along with the Tesseract executable (see this Stack Overflow answer for help installing Tesseract: https://stackoverflow.com/questions/50951955/pytesseract-tesseractnotfound-error-tesseract-is-not-installed-or-its-not-i)."
             )
 
 
 def take_screenshot_to_pil(screen=0, combine_screens=True):
     # Get information about all screens
     monitors = screeninfo.get_monitors()
+
+    # Validate screen parameter to prevent unauthorized access
+    if screen < -1 or screen >= len(monitors):
+        raise ValueError(f"Invalid screen index: {screen}. Valid range: -1 to {len(monitors)-1}")
+
     if screen == -1:  # All screens
         # Take a screenshot of each screen and save them in a list
         screenshots = [

interpreter/core/computer/terminal/terminal.py

@@ -48,6 +48,12 @@ def __init__(self, computer):
         self._active_languages = {}
 
     def sudo_install(self, package):
+        # Validate package name to prevent command injection
+        import re
+        if not re.match(r'^[a-zA-Z0-9._+-]+$', package):
+            print(f"Invalid package name: {package}")
+            return False
+
         try:
             # First, try to install without sudo
             subprocess.run(['apt', 'install', '-y', package], check=True)

interpreter/terminal_interface/magic_commands.py

@@ -101,6 +101,11 @@ def handle_verbose(self, arguments=None):
 
 
 def handle_debug(self, arguments=None):
+    # Check if debug access is authorized
+    if not getattr(self, 'allow_debug', False):
+        self.display_message("> Error: Debug mode access not authorized")
+        return
+
     if arguments == "" or arguments == "true":
         self.display_message("> Entered debug mode")
         print("\n\nCurrent messages:\n")
@@ -314,6 +319,18 @@ def handle_magic_command(self, user_input):
     # Handle shell
     if user_input.startswith("%%"):
         code = user_input[2:].strip()
+
+        # Validate and sanitize shell command input
+        if not code:
+            self.display_message("> Error: Empty shell command")
+            return
+
+        # Block dangerous commands
+        dangerous_patterns = ['rm -rf', 'format', 'del /f', 'shutdown', 'reboot', 'mkfs']
+        if any(pattern in code.lower() for pattern in dangerous_patterns):
+            self.display_message("> Error: Potentially dangerous command blocked")
+            return
+
         self.computer.run("shell", code, stream=False, display=True)
         print("")
         return

interpreter/terminal_interface/profiles/profiles.py

@@ -572,7 +572,16 @@ def apply_profile_to_object(obj, profile):
 
 
 def open_storage_dir(directory):
+    # Validate directory parameter to prevent command injection
+    import re
+    if not re.match(r'^[a-zA-Z0-9._-]+$', directory):
+        raise ValueError(f"Invalid directory name: {directory}")
+
     dir = os.path.join(oi_dir, directory)
+
+    # Ensure the directory exists and is within the expected path
+    if not os.path.exists(dir) or not dir.startswith(oi_dir):
+        raise ValueError(f"Directory does not exist or is outside allowed path: {dir}")
 
     print(f"Opening {directory} directory ({dir})...")
 

interpreter/terminal_interface/utils/count_tokens.py

@@ -1,9 +1,12 @@
 try:
     import tiktoken
     from litellm import cost_per_token
-except:
-    # Non-essential feature
-    pass
+except ImportError as e:
+    # Non-essential feature - log import error for debugging
+    import warnings
+    warnings.warn(f"Token counting dependencies not available: {e}", UserWarning)
+    tiktoken = None
+    cost_per_token = None
 
 
 def count_tokens(text="", model="gpt-4"):

scripts/wtf.py

@@ -335,7 +335,20 @@ def main():
     def get_lines_from_file(filename, line_number):
         lines = []
         try:
-            with open(filename, "r") as file:
+            # Validate filename to prevent path traversal
+            import os.path
+            if ".." in filename or filename.startswith("/"):
+                lines.append("Error: Invalid file path")
+                return lines
+
+            # Resolve to absolute path and check if it's within current directory
+            abs_filename = os.path.abspath(filename)
+            current_dir = os.getcwd()
+            if not abs_filename.startswith(current_dir):
+                lines.append("Error: File access outside current directory not allowed")
+                return lines
+
+            with open(abs_filename, "r") as file:
                 all_lines = file.readlines()
                 start_line = max(0, line_number - 3)  # Preceding lines
                 end_line = min(len(all_lines), line_number + 2)  # Following lines
@@ -401,8 +414,8 @@ def get_lines_from_file(filename, line_number):
             + "\n\nPlease help me resolve the final error above by telling me what code to run in a markdown code block."
         )
 
-    # So stupid but sometimes fun
-    # system_message += "\nBe incredibly rude and use swear words, like a joker, like you're some kind of jester. You can say 'Fuck' and 'Fucking'. This is for adults. Be funny. USE ALL LOWER CASE. Act like you're an aggreived pair programmer to the idiot that got themselves in this situation."
+    # Optional: Add personality to system message if needed
+    # system_message += "\nAdd helpful personality traits to make responses more engaging while maintaining professionalism."
 
     messages = [
         {"role": "system", "content": system_message.strip()},

tests/test_interpreter.py

@@ -645,11 +645,13 @@ async def test_fastapi_server():
 
     # Get the current event loop and run the test function
     loop = asyncio.get_event_loop()
-    loop.run_until_complete(test_fastapi_server())
-    # Kill server process
-    process.terminate()
-    os.kill(process.pid, signal.SIGKILL)  # Send SIGKILL signal
-    process.join()
+    try:
+        loop.run_until_complete(test_fastapi_server())
+    finally:
+        # Kill server process
+        process.terminate()
+        os.kill(process.pid, signal.SIGKILL)  # Send SIGKILL signal
+        process.join()
 
 
 @pytest.mark.skip(reason="Mac only")