Refactor is_bot_user function to improve actor type handling

[mrT23]

User description

fixes #1356

---

PR Type

enhancement, bug fix

---

Description

• Refactored the is_bot_user function to improve handling of Bitbucket actor types by using the get method for safer dictionary access.
• Enhanced logic to allow only 'user' actor types, treating 'AppUser' and 'team' as bot users.
• Improved logging to provide clearer information when the actor type is not 'user'.
• Fixed error message formatting in exception handling to correctly display exceptions.

---

Changes walkthrough 📝

	Relevant files
Enhancement	bitbucket_app.py Refactor and fix actor type handling in `is_bot_user` function pr_agent/servers/bitbucket_app.py Refactored is_bot_user function to improve handling of actor types. Changed logic to check actor type using get method for safer access. Improved logging for non-user actor types. Fixed error message formatting in exception handling. +6/-3

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

(Review updated until commit fe27f96)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 1356 - Fully compliant Compliant requirements: Fix 'data' field missing error by using safe dictionary access with get() Fix is_bot_user function validation by improving payload structure handling Fix Bitbucket webhook payload handling with better error handling and logging

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 95

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Edge Case The allowed_actor_types set contains only 'user'. Consider if there are other valid actor types that should be allowed. Error Handling Multiple nested try-except blocks for username extraction could be simplified using a more streamlined approach

[mrT23]

/review

[qodo-merge-pro]

Persistent review updated to latest commit 2c3aa7b

[mrT23]

/improve

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to fe27f96

Category	Suggestion	Score
Security	Return True on exceptions in bot detection to prevent potential security bypass The error handling in is_bot_user function will always return False when an exception occurs, potentially allowing bot users through. Consider returning True on exceptions for safer handling. pr_agent/servers/bitbucket_app.py [100-109] try: actor = data.get("data", {}).get("actor", {}) allowed_actor_types = {"user"} if actor and actor["type"].lower() not in allowed_actor_types: get_logger().info(f"BitBucket actor type is not 'user', skipping: {actor}") return True except Exception as e: get_logger().error(f"Failed 'is_bot_user' logic: {e}") + return True return False Apply this suggestion Suggestion importance[1-10]: 9 Why: This is a critical security suggestion that prevents potential bot access in error cases. The current implementation could allow bots to bypass restrictions when exceptions occur.	9
Possible issue	Use safe dictionary access to prevent potential runtime errors The actor dictionary access for type checking should use .get() to avoid potential KeyError if 'type' key is missing. pr_agent/servers/bitbucket_app.py [104] -if actor and actor["type"].lower() not in allowed_actor_types: +if actor and actor.get("type", "").lower() not in allowed_actor_types: Apply this suggestion Suggestion importance[1-10]: 7 Why: Using .get() instead of direct dictionary access is a good defensive programming practice that prevents KeyError exceptions and makes the code more robust.	7
General	Simplify nested error handling with a more robust fallback chain The nested try-except blocks for username extraction could lead to unclear error handling. Consider using get() with a default value. pr_agent/servers/bitbucket_app.py [169-175] -try: - username = actor["username"] -except KeyError: - try: - username = actor["display_name"] - except KeyError: - username = actor["nickname"] +username = actor.get("username") or actor.get("display_name") or actor.get("nickname", "unknown") Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion simplifies complex nested try-except blocks into a more readable and maintainable chain of fallbacks, while preserving the same functionality.	6

• Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

---

Previous suggestions

Suggestions up to commit c934523

Category	Suggestion	Score
Possible issue	Fix incorrect bot user detection by returning False instead of falling through to default return value when exceptions occur The function returns True for bot users but also when there's an exception, which could lead to incorrectly identifying human users as bots. Return False for exceptions to avoid false positives. pr_agent/servers/bitbucket_app.py [100-109] try: actor = data.get("data", {}).get("actor", {}) allowed_actor_types = {"user"} if actor and actor["type"].lower() not in allowed_actor_types: get_logger().info(f"BitBucket actor type is not 'user', skipping: {actor}") return True + return False except Exception as e: get_logger().error(f"Failed 'is_bot_user' logic: {e}") -return False + return False Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies a logical flaw where exceptions in bot detection could lead to false negatives. Moving the return False inside the try block and keeping it after exceptions improves the function's reliability and error handling.	8
	Prevent potential KeyError by using safe dictionary access method Direct dictionary access to actor["type"] can raise KeyError if type key is missing. Use get() method for safer access. pr_agent/servers/bitbucket_app.py [104] -if actor and actor["type"].lower() not in allowed_actor_types: +if actor and actor.get("type", "").lower() not in allowed_actor_types: Suggestion importance[1-10]: 7 Why: The suggestion addresses a potential runtime error by using the safer get() method for dictionary access, which is particularly important when dealing with external API responses that might have varying structures.	7

[okotek]

/review

[qodo-merge-pro-for-open-source]

Persistent review updated to latest commit fe27f96