Wrap every tool with exception handling

[CTY-git]

PR Checklist

• The commit message follows our guidelines: Code of conduct
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Does this PR introduce a breaking change?
• Include PR in release notes?

PR Type

• Bugfix
• Feature
• Refactoring
• Build /CI
• Documentation
• Others

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Other information

[patched-admin]

File Changed: patchwork/common/tools/grep_tool.py

Rule 1: Do not ignore potential bugs in the code

Details: The code changes from raising exceptions to returning error messages as strings. This creates an inconsistent error handling pattern that could lead to bugs where error conditions are not properly handled by calling code. The code now mixes two different error handling approaches - some errors raise exceptions while others return strings.

Affected Code Snippet:

if pattern is None:
    return "`pattern` argument is required!"

try:
    path = Path(path).resolve()
except FileNotFoundError:
    return f"`path` does not exist"

if not path.is_relative_to(self.__working_dir):
    return f"Path must be relative to working dir {self.__working_dir}"

Start Line: 164
End Line: 173

---

Rule 2: Do not overlook possible security vulnerabilities

Details: Converting exceptions to returned strings for path validation could lead to security vulnerabilities. An attacker could potentially bypass path validation if the calling code doesn't properly check the returned error strings. The original exception-based approach provided stronger guarantees against path traversal attacks.

Affected Code Snippet:

if not path.is_relative_to(self.__working_dir):
    return f"Path must be relative to working dir {self.__working_dir}"

Start Line: 180
End Line: 181

File Changed: patchwork/common/tools/tool.py

Rule 1: Do not ignore potential bugs in the code

Details: The code modification introduces a potential bug by catching all exceptions and converting them to string messages. This could mask critical errors and make debugging more difficult. The broad exception handling could catch and hide important system-level exceptions that should be propagated up the stack.

Affected Code Snippet:

try:
    return func(self, *args, **kwargs)
except Exception as e:
    logger.error(f"Error executing Tool: {self.name}: {e}")
    return f"Error: {e}"

Start Line: 77
End Line: 80

---

Rule 2: Do not overlook possible security vulnerabilities

Details: The code change introduces a potential security vulnerability by exposing exception details in the return value. Exception messages might contain sensitive information about the system, stack traces, or internal errors that could be exploited by malicious actors.

Affected Code Snippet:

except Exception as e:
    logger.error(f"Error executing Tool: {self.name}: {e}")
    return f"Error: {e}"

Start Line: 79
End Line: 80