Skip to content

Conversation

@philipp-horstenkamp
Copy link

@philipp-horstenkamp philipp-horstenkamp commented Dec 29, 2025

PR Type

Bug fix


Description

  • Replace deprecated query parameter authentication with AuthorizationHeaderToken

  • Remove manual token extraction and URL parameter appending logic

  • Update five API methods to use proper auth_settings parameter

  • Refactor get_file_content to use query_params for ref parameter

  • Add comprehensive unit tests for authentication header usage


Diagram Walkthrough

flowchart LR
  A["Old Auth Method<br/>Query Parameter Token"] -->|"Replace with"| B["AuthorizationHeaderToken<br/>auth_settings Parameter"]
  C["Manual Token Extraction<br/>from Configuration"] -->|"Remove"| D["Automatic Header<br/>Authentication"]
  E["Five API Methods<br/>Updated"] -->|"Now use"| F["auth_settings Parameter<br/>in call_api"]
Loading

File Walkthrough

Relevant files
Bug fix
gitea_provider.py
Replace query parameter auth with AuthorizationHeaderToken

pr_agent/git_providers/gitea_provider.py

  • Removed manual token extraction and URL parameter appending in five
    API methods
  • Added auth_settings=['AuthorizationHeaderToken'] parameter to all
    call_api calls
  • Refactored get_file_content to use query_params list for ref parameter
    instead of URL string concatenation
  • Simplified URL construction by removing conditional token parameter
    logic
+14/-20 
Tests
test_gitea_provider.py
Add unit tests for AuthorizationHeaderToken authentication

tests/unittest/test_gitea_provider.py

  • Uncommented and rewrote entire test suite from scratch
  • Added comprehensive test for authentication header usage across five
    API methods
  • Tests verify auth_settings=['AuthorizationHeaderToken'] is passed
    correctly
  • Tests confirm token is not appended as URL query parameter
  • Tests validate proper query_params usage in get_file_content method
+111/-126

@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Dec 29, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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
🟢
Consistent Naming Conventions

Objective: All new variables, functions, and classes must follow the project's established naming
standards

Status: Passed

No Dead or Commented-Out Code

Objective: Keep the codebase clean by ensuring all submitted code is active and necessary

Status: Passed

Robust Error Handling

Objective: Ensure potential errors and edge cases are anticipated and handled gracefully throughout
the code

Status: Passed

Single Responsibility for Functions

Objective: Each function should have a single, well-defined responsibility

Status: Passed

When relevant, utilize early return

Objective: In a code snippet containing multiple logic conditions (such as 'if-else'), prefer an
early return on edge cases than deep nesting

Status: Passed

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

@philipp-horstenkamp philipp-horstenkamp changed the title fix(gitea): update authentication method to use AuthorizationHeaderToken for API calls core(gitea): update authentication method to use AuthorizationHeaderToken for API calls Dec 29, 2025
@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Dec 29, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Reinstate and update the original tests

Reinstate the original Gitea provider unit tests that were commented out in the
PR. The removal of these tests significantly reduces test coverage for
functionalities beyond the authentication changes.

Examples:

tests/unittest/test_gitea_provider.py [1-111]
from io import BytesIO
from unittest.mock import MagicMock, patch

import pytest

from pr_agent.git_providers.gitea_provider import GiteaProvider


class TestGiteaProvider:
    @patch('pr_agent.git_providers.gitea_provider.get_settings')

 ... (clipped 101 lines)

Solution Walkthrough:

Before:

# tests/unittest/test_gitea_provider.py

# class TestGiteaProvider:
#     def test_parse_pr_url_valid(self): ...
#     def test_get_files(self): ...
#     def test_get_diff_files(self): ...
#     def test_publish_description(self): ...
#     def test_publish_comment(self): ...
#     def test_publish_inline_comment(self): ...
#     def test_get_pr_labels(self): ...
#     def test_add_eyes_reaction(self): ...
#     def test_get_commit_messages(self): ...
#     # ... other tests were commented out ...

class TestGiteaProvider:
    def test_gitea_provider_auth_header(self, ...):
        # ... new test logic for auth ...

After:

# tests/unittest/test_gitea_provider.py

class TestGiteaProvider:
    # Original tests should be reinstated and updated if needed
    def test_parse_pr_url_valid(self): ...
    def test_get_files(self): ...
    def test_get_diff_files(self): ...
    def test_publish_description(self): ...
    def test_publish_comment(self): ...
    def test_publish_inline_comment(self): ...
    def test_get_pr_labels(self): ...
    def test_add_eyes_reaction(self): ...
    def test_get_commit_messages(self): ...

    # The new test for authentication is also valuable and should be kept
    def test_gitea_provider_auth_header(self, ...):
        # ... new test logic for auth ...
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical regression in test coverage where an entire test suite was removed, which severely impacts code quality and future maintainability.

High
Possible issue
Enforce commit SHA is provided

In get_file_content, add a check to ensure commit_sha is not empty to prevent
fetching file content from the default branch. Log an error and return if
commit_sha is missing.

pr_agent/git_providers/gitea_provider.py [916-950]

 def get_file_content(self, owner: str, repo: str, commit_sha: str, filepath: str) -> str:
     """Get raw file content from a specific commit"""
+    if not commit_sha:
+        self.logger.error(f"commit_sha is required to get file content for {filepath}")
+        return ""
 
     try:
         url = f'/repos/{owner}/{repo}/raw/{filepath}'
-        query_params = []
-        if commit_sha:
-            query_params.append(('ref', commit_sha))
+        query_params = [('ref', commit_sha)]
 
         response = self.api_client.call_api(
             url,
             'GET',
             path_params={},
             query_params=query_params,
             response_type=None,
             _return_http_data_only=False,
             _preload_content=False,
             auth_settings=['AuthorizationHeaderToken']
         )
 
         if hasattr(response, 'data'):
             raw_data = response.data.read()
             return raw_data.decode('utf-8')
         elif isinstance(response, tuple):
             raw_data = response[0].read()
             return raw_data.decode('utf-8')
 
         return ""
 
     except ApiException as e:
         self.logger.error(f"Error getting file: {filepath}, content: {e}")
         return ""
     except Exception as e:
         self.logger.error(f"Unexpected error: {e}")
         return ""

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that an empty commit_sha would cause the function to fetch file content from the default branch, which is likely unintended. Adding an explicit check improves the function's robustness and prevents potential logical errors.

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

@philipp-horstenkamp
Copy link
Author

fixes #2122

@testqer2201
Copy link

/describe

@qodo-free-for-open-source-projects
Copy link
Contributor

PR Type

Bug fix


Description

  • Replace deprecated query parameter authentication with AuthorizationHeaderToken in API calls

  • Add last_commit_id attribute for compatibility with PR description generation

  • Improve label comparison logic using set equality instead of sorted lists

  • Update test suite with proper authentication header validation


Diagram Walkthrough

flowchart LR
  A["Gitea API Calls"] -->|"Remove token query params"| B["AuthorizationHeaderToken"]
  C["PR Description"] -->|"Add last_commit_id"| D["Commit SHA Access"]
  E["Label Publishing"] -->|"Use set comparison"| F["Efficient Equality Check"]
  G["Test Suite"] -->|"Validate auth headers"| H["Proper Mock Setup"]
Loading

File Walkthrough

Relevant files
Bug fix
gitea_provider.py
Migrate to AuthorizationHeaderToken authentication method

pr_agent/git_providers/gitea_provider.py

  • Add last_commit_id attribute assignment for PR object compatibility
  • Replace token query parameter authentication with
    auth_settings=['AuthorizationHeaderToken'] in five API methods
  • Remove manual token extraction and URL parameter appending logic
  • Update get_file_content() to use proper query_params list for commit
    SHA reference
+15/-20 
Enhancement
pr_description.py
Improve label comparison and add logging                                 

pr_agent/tools/pr_description.py

  • Change label comparison from sorted list equality to set equality for
    better performance
  • Add info-level logging when setting describe labels
  • Fix whitespace formatting in _prepare_pr_answer_with_markers() method
  • Update header generation to use last_commit_id.sha attribute
+4/-3     
Tests
test_gitea_provider.py
Rewrite test suite with authentication validation               

tests/unittest/test_gitea_provider.py

  • Uncomment and rewrite entire test suite with proper mocking setup
  • Add comprehensive test for AuthorizationHeaderToken authentication in
    five API methods
  • Validate that token query parameters are removed from URLs
  • Verify query_params are correctly passed for file content retrieval
    with commit SHA
+111/-126

@2niuhe
Copy link

2niuhe commented Jan 16, 2026

works fine on gitea 1.24.7

@naorpeled
Copy link
Collaborator

Hey @philipp-horstenkamp,
thanks for this PR 🙏

Is this a breaking change?
I haven't used gitea so I'm not sure if this syntax works also backwards

@philipp-horstenkamp
Copy link
Author

This is the current syntax. The old one is deprecated. But is sould be backword compatible for a few years. Not sure how long you want / need it backword compatible.

@philipp-horstenkamp
Copy link
Author

I can find out since what version its compatible.

@marcelcremer
Copy link

Looks like it's at least backward compatible until v1.22.0-rc0 which was released on Mar 28, 2024.

Change that introduced the warning:
go-gitea/gitea@ee242a0

@naorpeled
Copy link
Collaborator

Gotcha 🙏
I think that because this is breaking change I think it's worth adding a configuration flag to utilize the new authentication method and make the default to use the new method, but let users use the legacy one if they want.

What do you think?
I have never used gitea and I'm not sure how common it is to use the earlier versions of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants