Add issue_url in pr body

[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?

Currently we don't link issue_url when creating PRs

Issue Number: N/A

What is the new behavior?

Now we link issue_url when creating PRs

Other information

[patched-admin]

The pull request review highlights multiple concerns related to the introduction of a new issue_url field in the PRInputs TypedDict, emphasizing potential bugs, security vulnerabilities, and adherence to coding standards. The review warns of possible KeyErrors or unexpected behavior if code interactions with PRInputs don’t properly incorporate this new field, and it stresses the need for URL validation to avoid security risks such as SSRF or phishing. Coding standards are generally followed, but there is critique over changed initialization practices affecting code modularity and readability. Moreover, the removal of the test_init_required_keys function raises concerns about maintaining test coverage, potentially leaving the required_keys feature unsafeguarded against future changes. The review underscores the importance of thorough validation and testing to ensure the robustness and security compliance of the code.

---

• File changed: patchwork/steps/PR/typed.py
  1. Potential Bugs:
  • The new line adds an issue_url field to the PRInputs TypedDict. It is important to ensure that all areas where PRInputs is used are updated to correctly handle the issue_url key. Any code that constructs or interacts with PRInputs without expecting this key might result in unexpected behavior or raise a KeyError if access to issue_url is attempted without verification.

2. Security Vulnerabilities:

   • Adding the issue_url field as a configuration parameter should prompt a review of how these URLs will be handled. If these URLs are used in HTTP requests or stored, ensure the proper validation and sanitization are in place to avoid vulnerabilities such as Server-Side Request Forgery (SSRF) or other injection-based attacks.

3. Coding Standards:

   • Assuming StepTypeConfig and Annotated are already used consistently throughout the codebase for configuration values, the new code adheres to this pattern by following the existing design. However, ensure that related documentation is updated to reflect the addition of issue_url where necessary, maintaining documentation standards.

• File changed: patchwork/steps/PreparePR/PreparePR.py
  1. Potential Bugs:
  • The logic to construct the header seems inconsistent based on whether an issue_url or pr_header is provided. If neither is provided, it defaults to saying it fixes a number of issues based on len(self.modified_code_files), which might not always make sense. It should be clear whether it is indeed fixing issues only or there might be other types of changes.

2. Security Vulnerabilities:

   • There is no evident security vulnerability in the diff itself; however, the use of string formatting with external inputs should generally verify that these inputs are safe to use, especially when URLs are involved. It's important always to sanitize and validate such inputs to avoid issues like injection attacks.

3. Coding Standards:

   • The code does not follow the original coding style where initialization and key checks were done separately. It now mixes initialization with validation, which could impact readability. The previous implementation had a clear separation of concerns, making it more modular and potentially easier to extend or modify independently in the future. The code should continue to enforce required keys before accessing self.modified_code_files and properly using the inputs dictionary.

• File changed: patchwork/steps/PreparePR/typed.py
  1. Potential Bugs:
  • None identified within the provided diff.

2. Security Vulnerabilities:

   • Including an issue_url field introduces a potential security risk if URLs are not properly validated. Malicious URLs could lead to phishing attacks or other security compromises. Ensure there is adequate validation and sanitization for URLs.

3. Coding Standards Adherence:

   • The new code appears to adhere to the existing coding standards as it maintains the pattern of using Annotated and StepTypeConfig for configurations. No deviations from original coding style are observed.

• File changed: tests/steps/test_PreparePR.py
  The changes in the pull request remove the test_init_required_keys test function, which checks for the presence of a required attribute required_keys in the PreparePR instance. Removing this test could potentially lead to issues if the required_keys functionality is essential for the operation of the PreparePR class but goes untested now. This could result in a bug if future changes modify or remove required_keys without detection.

Additionally, the removal of this test might indicate a deviation from the original testing strategy if required_keys was part of the original contract of the class. It's crucial to ensure that critical attributes or functionalities are adequately tested to maintain code reliability.