Skip to content

Add fixes to GithubAgent and fix types for some other steps #1574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 11, 2025

Conversation

whoisarpit
Copy link
Contributor

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

@whoisarpit whoisarpit requested a review from CTY-git April 11, 2025 11:04
@patched-admin
Copy link
Contributor

File Changed: patchwork/steps/AgenticLLMV2/AgenticLLMV2.py

Rule 1: Do not ignore potential bugs in the code

Details: Potential bug identified - The code modification introduces default value fallbacks using .get() but doesn't validate the input types or values. This could lead to runtime errors if invalid model names are provided.

Affected Code Snippet:

model=inputs.get("strategy_model", "claude-3-5-sonnet-latest"),
...
model=inputs.get("agent_model", "claude-3-7-sonnet-latest"),

Start Line: 21
End Line: 28


Rule 2: Do not overlook possible security vulnerabilities

Details: Potential security vulnerability - The code accepts model names from input without validation or sanitization. This could potentially lead to injection attacks if the model names are used in API calls or command execution.

Affected Code Snippet:

model=inputs.get("strategy_model", "claude-3-5-sonnet-latest"),
...
model=inputs.get("agent_model", "claude-3-7-sonnet-latest"),

Start Line: 21
End Line: 28

File Changed: patchwork/steps/AgenticLLMV2/typed.py

Rule 1: Do not ignore potential bugs in the code

Details: Potential bug identified in the or_op configuration. The circular dependency between API keys could lead to a situation where none of the keys are available if the validation logic is not properly handled.

Affected Code Snippet:

    openai_api_key: Annotated[
        str,
        StepTypeConfig(
            is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "anthropic_api_key"]
        ),
    ]
    anthropic_api_key: Annotated[
        str,
        StepTypeConfig(is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "openai_api_key"]),
    ]
    google_api_key: Annotated[
        str,
        StepTypeConfig(
            is_config=True, or_op=["patched_api_key", "openai_api_key", "client_is_gcp", "anthropic_api_key"]
        ),
    ]
    client_is_gcp: Annotated[
        str,
        StepTypeConfig(
            is_config=True, or_op=["patched_api_key", "openai_api_key", "anthropic_api_key", "google_api_key"]
        ),
    ]

Start Line: 16
End Line: 36


Rule 2: Do not overlook possible security vulnerabilities

Details: Security concern identified in the API key handling. The or_op configuration allows fallback between different API keys which could lead to unintended access or key exposure between different services.

Affected Code Snippet:

    openai_api_key: Annotated[
        str,
        StepTypeConfig(
            is_config=True, or_op=["patched_api_key", "google_api_key", "client_is_gcp", "anthropic_api_key"]
        ),
    ]

Start Line: 16
End Line: 22

File Changed: patchwork/steps/GitHubAgent/GitHubAgent.py

Rule 1: Do not ignore potential bugs in the code

Details: Found a potentially breaking change in GitHub API token parameter name that could cause runtime errors.

Affected Code Snippet:

-                    tool_set=dict(github_tool=GitHubTool(base_path, inputs["github_api_token"])),
+                    tool_set=dict(github_tool=GitHubTool(base_path, inputs["github_api_key"])),

Start Line: 37
End Line: 37

Explanation: The code changes the expected input parameter name from "github_api_token" to "github_api_key". This could cause KeyError exceptions if calling code is still using the old parameter name. No validation or fallback is provided.


Rule 2: Do not overlook possible security vulnerabilities

Details: Found a potential security issue with hardcoded example JSON in default parameters.

Affected Code Snippet:

-            example_json=inputs.get("example_json"),
+            example_json=inputs.get(
+                "example_json", '{"summary_of_actions": "1. Retrieved the list of repositories. 2. ..."}'
+            ),

Start Line: 45
End Line: 47

Explanation: Adding a default JSON string could potentially expose internal system information or expected data structures. It's better to keep such examples in separate configuration files or documentation.

File Changed: patchwork/steps/GitHubAgent/typed.py

Rule 2: Do not overlook possible security vulnerabilities

Details: Security concern detected - The addition of github_api_key and openai_api_key in the TypedDict suggests these sensitive credentials will be handled by the code. These should be properly secured and not exposed in logs or error messages.

Affected Code Snippet:

class __GitHubAgentRequiredInputs(TypedDict):
    github_api_key: str
    task: str

class GitHubAgentInputs(__GitHubAgentRequiredInputs, total=False):
    openai_api_key: Annotated[
        str,
        StepTypeConfig(

Start Line: 6
End Line: 16

@whoisarpit whoisarpit merged commit 1be911c into main Apr 11, 2025
2 of 4 checks passed
@whoisarpit whoisarpit deleted the fix/gh-agent branch April 11, 2025 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants