Link for github issues in PR

[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 issues to PRs.

Issue Number: N/A

What is the new behavior?

When provided with issue_url, the CreatePR step now will link the newly created PR with the issue.

Other information

[patched-admin]

The pull request review highlights concerns across three main areas: security vulnerabilities, adherence to coding standards, and potential bugs. There is a potential security risk identified with hardcoded URLs in the workflow file, which might necessitate using parameters or secrets to mitigate unauthorized actions or exposure of sensitive information, while also ensuring that exception logging does not inadvertently expose sensitive details. It is recommended to validate URLs to avoid errors like 404s, address potential unpacking issues with the resource_slug_and_id, and revise the logging strategy to mask sensitive data. Coding standards are moderately adhered to with clear parameter names, though inconsistencies in naming conventions (e.g., branch-related parameters) are noted, and the use of Python 3.10 syntax might cause compatibility issues with earlier versions. Ensuring all new variables are documented and that appropriate error handling and unit tests are added for new functionalities are also advised. Additionally, the review notes metadata changes, such as version updates, which do not appear to introduce any functional risks. The overall recommendation is to ensure consistency, proper exception handling, validation, and that potential security concerns are adequately addressed.

---

• File changed: .github/workflows/test.yml
  1. Security Vulnerability: Including hardcoded issue URLs directly in the workflow file might create a security concern if the URL triggers any unauthorized actions, or if the issue URL is sensitive or confidential. Consider passing this as a parameter or using a secret if it's sensitive.

2. Adherence to Coding Standards: Ensure that the inclusion of the --issue_url parameter is consistent with other optional parameters' use within the workflow. This includes spacing, alignment, and any commenting standards used to describe the purpose of new flags or parameters.

3. Potential Bugs: It's essential to validate that the provided URLs are reachable or correct where applicable, as hardcoding attributes like URL could lead to 404 errors or incorrect issue associations if even a minor typo is made.

• File changed: patchwork/common/client/scm.py
  1. Potential Bugs:
  • In the create_pr method for GitHub, when issue_obj is determined and issue_url is non-null, the method self.get_slug_and_id_from_url(issue_url) is called. There is no validation performed post call to ensure if slug and issue_id are correctly set from the tuple. If there is any issue within get_slug_and_id_from_url, it would lead to TypeError as the tuple will not unpack properly. This can cause issues if resource_slug_and_id is None.

2. Security Vulnerabilities:

   • Logging the exception e directly in find_issue_by_id method without sanitization or masking can unintentionally expose sensitive information within the logs. It is critical to ensure that any output logs are adequately scrubbed of sensitive repository or issue details.

3. Adherence to Coding Standards:

   • Usage of | in type annotations like issue_url: str | None = None is Python 3.10+ syntax. Make sure this does not break compatibility if the project supports older Python versions.
   • The use of NotSet from GithubObject to denote a non-existing issue is a good practice; however, this should be implemented consistently across all methods that are handling GitHub issues.
   • There's no immediate indication of test functions being updated or tested for these scenarios. Ensure that any call paths using these functions have corresponding unit tests to evaluate the new code paths and parameter handling.

Overall, it's recommended to double-check the handling of resource_slug_and_id post assignment to prevent potential None dereferencing and ensure proper exception handling for logging.

• File changed: patchwork/steps/CreatePR/CreatePR.py
  1. Potential Bugs:
  • The typing_extensions.Optional import might be unnecessary if just using Optional. Instead, use from typing import Optional unless there's a specific reason to use typing_extensions.

2. Security Vulnerabilities:

   • There is no obvious introduction of security vulnerabilities, but it's important to ensure that any URLs or inputs passed from user input are properly validated and sanitized to prevent issues like injection attacks (even though they may not appear in this snippet, considerations should be made in the context).

3. Code Standards:

   • The code appears to adhere to basic Python conventions with clear naming and consistent parameter passing.
   • However, there's a lack of consistency in naming for parameters compared to their usage in functions (original_branch vs base_branch, feature_branch vs target_branch). It might be beneficial to unify these terms for clarity unless they have distinct meanings in different contexts.
   • Ensure all newly introduced variables (like issue_url) are documented if there is a documentation standard in place for the project.
   • There isn't a change in exception handling around the create_pr call. If this operation can fail, considering adding appropriate error handling or logging if not done elsewhere in the project.

• File changed: patchwork/steps/CreatePR/typed.py
  1. Potential Bugs: The addition of the issue_url: Annotated[str, StepTypeConfig(is_config=True)] field looks straightforward; however, there's no indication of how this new field is being utilized within the rest of the codebase. It's important to check if this field needs initialization or default validation to prevent any runtime errors.

2. Security Vulnerabilities: Adding a new string for a URL shouldn't inherently introduce a security vulnerability. However, if this URL is later used for making web requests or integrations, ensure that proper validation and sanitization are in place to prevent potential vulnerabilities such as injection attacks or request forgeries.

3. Coding Standards: There's no explicit violation of coding standards from this diff alone, as it follows the existing pattern for other URL fields like scm_url. However, please ensure consistency in the documentation and usage across the codebase related to this new field.

• File changed: pyproject.toml
  The change in the version number from 0.0.84 to 0.0.85 in the pyproject.toml file appears to be a standard update to indicate a new version of the software. There doesn't seem to be any potential bugs or security vulnerabilities introduced by this change as it only modifies metadata.

Since this is a metadata change, it does not involve functional code, so it does not break any original coding standards in regard to code logic. However, ensure that any corresponding changes that depend on this version update (e.g., in changelogs or documentation) are also addressed outside of this diff.

Overall, the modification maintains coding standards in terms of proper semantic versioning.

[CTY-git]

Not required, original method of adding fixes {issue_url} works when relevant permission is set.