Fix LDAP search to include an optional trailing slash

[webbnh]

User description

When we look a GitHub user up in the RH Rover, we currently expect the URL to omit the trailing slash. However, currently, 5% of associates include the slash (and GitHub doesn't care). So, this PR modifies our search to look for a match either with or without the slash. (Unfortunately, the wildcarding that LDAP provides is well short of generalized regular expressions, so we do it with a conjunction instead.)

Unfortunately, the unit test noticed the change, so I had to update it, too.

---

PR Type

Bug fix

---

Description

• Modified LDAP search filter to match GitHub URLs with or without trailing slash

• Updated filter to use OR clause combining both URL variants

• Updated unit test to verify new filter format

---

Diagram Walkthrough

flowchart LR
  A["GitHub URL lookup"] --> B["Construct LDAP filter"]
  B --> C["Single filter clause"]
  C --> D["OR clause with/without slash"]
  D --> E["LDAP search"]

Loading

File Walkthrough

	Relevant files
Bug fix	lookup.py LDAP filter to support optional trailing slash                      Rover_Lookup/lookup.py Added comment explaining 5% of entries include trailing slash Refactored LDAP filter construction to use OR clause Changed from simple filter to compound filter matching both URL variants Filter now searches for rhatSocialURL=Github->https://github.com/username OR rhatSocialURL=Github->https://github.com/username/ +4/-1
Tests	test_lookup.py Update test for new LDAP filter format                                      Rover_Lookup/tests/test_lookup.py Updated test assertion to match new LDAP filter format Changed from simple filter string to OR clause format Refactored assertion to use intermediate variable for filter part +3/-4

---

[qodo-code-review]

PR Compliance Guide 🔍

(Compliance updated until commit 538ee27)

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	LDAP injection Description: Potential LDAP injection: github_username is interpolated into ldap_filter without escaping/encoding, so a crafted value containing LDAP filter metacharacters (e.g., )(|(objectClass=))() could alter the filter logic and broaden or change the LDAP search. lookup.py [59-62] Referred Code github_url = f"https://github.com/{github_username}" filter_clause = f"rhatSocialURL=Github->{github_url}" ldap_filter = f"(|({filter_clause})({filter_clause}/))"
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: LDAP filter injection: github_username is interpolated directly into ldap_filter without escaping/sanitization, enabling LDAP filter injection or malformed filters if the username contains special characters. Referred Code github_url = f"https://github.com/{github_username}" filter_clause = f"rhatSocialURL=Github->{github_url}" ldap_filter = f"(|({filter_clause})({filter_clause}/))" Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logging: The updated LDAP lookup logic does not add any audit log entries for this directory query, so it is unclear whether this potentially sensitive read action is captured with user/context/outcome elsewhere. Referred Code # Construct the LDAP filter to match the GitHub "Professional Social Media" URL. # The rhatSocialURL field contains values like "Github->https://github.com/username". # However, 5% of the entries include a trailing slash (which GitHub accepts), # so look for those, too. github_url = f"https://github.com/{github_username}" filter_clause = f"rhatSocialURL=Github->{github_url}" ldap_filter = f"(|({filter_clause})({filter_clause}/))" Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

---

Previous compliance checks

Compliance check up to commit bf8e041

Security Compliance
⚪	LDAP injection Description: The LDAP search filter is built via string interpolation using github_username (through github_url) without escaping LDAP filter metacharacters, so a crafted username containing characters like *, (, ), or </code> could alter the filter logic (LDAP injection) and potentially broaden/modify the search. lookup.py [59-61] Referred Code github_url = f"https://github.com/{github_username}" filter_clause = f"rhatSocialURL=Github->{github_url}" ldap_filter = f"(|({filter_clause}) ({filter_clause}/))"
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: LDAP injection risk: The PR constructs ldap_filter by interpolating the external github_username into an LDAP filter without escaping special characters, which can enable LDAP injection or malformed queries. Referred Code github_url = f"https://github.com/{github_username}" filter_clause = f"rhatSocialURL=Github->{github_url}" ldap_filter = f"(|({filter_clause}) ({filter_clause}/))" Learn more about managing compliance generic rules or creating your own custom rules

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Escape LDAP special characters Sanitize the github_username input by escaping special characters to prevent potential LDAP injection attacks. Rover_Lookup/lookup.py [59] -github_url = f"https://github.com/{github_username}" +escaped_username = escape_filter_chars(github_username) +github_url = f"https://github.com/{escaped_username}" Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical security vulnerability (LDAP injection) by not escaping the user-provided github_username and proposes a valid solution to prevent it.	High
Possible issue	✅ Fix invalid LDAP filter syntax Suggestion Impact: Updated the ldap_filter string to remove the space between OR clauses, matching the correct LDAP filter syntax. code diff: filter_clause = f"rhatSocialURL=Github->{github_url}" - ldap_filter = f"(|({filter_clause}) ({filter_clause}/))" + ldap_filter = f"(|({filter_clause})({filter_clause}/))" Remove the whitespace between the clauses in the LDAP OR filter to ensure correct syntax and prevent potential parsing failures. Rover_Lookup/lookup.py [60-61] filter_clause = f"rhatSocialURL=Github->{github_url}" -ldap_filter = f"(|({filter_clause}) ({filter_clause}/))" +ldap_filter = f"(|({filter_clause})({filter_clause}/))" [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the space between clauses in the LDAP OR filter is non-standard and could cause parsing failures, which would break the feature.	Medium
	✅ Align tests with valid filter Suggestion Impact: The test expectation was updated to remove the whitespace in the OR filter string, matching the corrected LDAP filter syntax. code diff: filter_part = "rhatSocialURL=Github->https://github.com/test-user" - expected_filter = f"(|({filter_part}) ({filter_part}/))" + expected_filter = f"(|({filter_part})({filter_part}/))" assert search_args[1]["search_filter"] == expected_filter Update the expected LDAP filter in the test to remove the non-standard whitespace between OR clauses, aligning it with the corrected production code. Rover_Lookup/tests/test_lookup.py [75-77] filter_part = "rhatSocialURL=Github->https://github.com/test-user" -expected_filter = f"(|({filter_part}) ({filter_part}/))" +expected_filter = f"(|({filter_part})({filter_part}/))" assert search_args[1]["search_filter"] == expected_filter [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly points out that the test should be updated to reflect the valid LDAP filter syntax, ensuring the test validates the correct behavior.	Medium
Update

[webbnh]

@qodo-code-review, since GitHub usernames can consist only of alphanumeric characters (or single hyphens), there is no threat of an LDAP-injection attack, and so the complication of adding escapes is unwarranted. And, since the username value is provided by GitHub and not by a user, there should be little likelihood of a malformed query.

[Bala-Sakabattula]

LGTM! Thanks @webbnh