diff --git a/docs/docs/installation/github.md b/docs/docs/installation/github.md index e9b33881fd..20e77fd639 100644 --- a/docs/docs/installation/github.md +++ b/docs/docs/installation/github.md @@ -467,6 +467,13 @@ If you encounter rate limiting: pull-requests: write contents: write ``` + If you cannot grant `contents: write`, set `config.restricted_mode = true` in your configuration. In that case you only need: + ```yaml + permissions: + issues: write + pull-requests: write + ``` + See the [Restricted Mode guide](../usage-guide/additional_configurations.md#restricted-mode) for details. **Error: "Invalid JSON format"** diff --git a/docs/docs/usage-guide/additional_configurations.md b/docs/docs/usage-guide/additional_configurations.md index ba67f8dfc4..412920c351 100644 --- a/docs/docs/usage-guide/additional_configurations.md +++ b/docs/docs/usage-guide/additional_configurations.md @@ -280,3 +280,24 @@ ignore_ticket_labels = ["ignore-compliance", "skip-review", "wont-fix"] ``` Where `ignore_ticket_labels` is a list of label names that should be ignored during ticket analysis. + +### Restricted Mode + +When running PR-Agent with limited GitHub/GitLab permissions, set `restricted_mode` to `true` to gracefully skip operations that require elevated access (e.g., pushing changelog changes): + +```toml +[config] +restricted_mode = true +``` + +With restricted mode, the minimum workflow permissions are: + +```yaml +permissions: + issues: write + pull-requests: write +``` + +Within an explicit `permissions:` block, any scope you do not list (such as `contents`) is set to `none`, so you do not need to grant `contents` — restricted mode skips every operation that would require `contents: write`. All tools (`/review`, `/describe`, `/improve`, etc.) continue to work normally with just `pull-requests: write`. + +> **Note:** this only holds when a `permissions:` block is present (as above). If you omit the `permissions:` block entirely, the effective defaults are governed by your repository/organization GitHub Actions settings and may grant broader access. diff --git a/pr_agent/git_providers/bitbucket_provider.py b/pr_agent/git_providers/bitbucket_provider.py index e772cada34..4bc273b19c 100644 --- a/pr_agent/git_providers/bitbucket_provider.py +++ b/pr_agent/git_providers/bitbucket_provider.py @@ -192,6 +192,8 @@ def is_supported(self, capability: str) -> bool: if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels', 'gfm_markdown', 'publish_file_comments']: return False + if capability == "push_code" and get_settings().config.restricted_mode: + return False return True def set_pr(self, pr_url: str): diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 628caf7a19..85ca6f4f49 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -84,6 +84,8 @@ def get_incremental_commits(self, incremental=IncrementalPR(False)): self._get_incremental_commits() def is_supported(self, capability: str) -> bool: + if capability == "push_code" and get_settings().config.restricted_mode: + return False return True def _get_owner_and_repo_path(self, given_url: str) -> str: diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 2128e2670b..c4b131ad75 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -290,6 +290,8 @@ def is_supported(self, capability: str) -> bool: if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments', 'publish_file_comments']: # gfm_markdown is supported in gitlab ! return False + if capability == "push_code" and get_settings().config.restricted_mode: + return False return True def _get_project_path_from_pr_or_issue_url(self, pr_or_issue_url: str) -> str: diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 675f5dad9e..5bffc04d8f 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -57,6 +57,7 @@ ignore_pr_authors = [] # authors to ignore from PR agent when an PR is created ignore_repositories = [] # a list of regular expressions of repository full names (e.g. "org/repo") to ignore from PR agent processing ignore_language_framework = [] # a list of code-generation languages or frameworks (e.g. 'protobuf', 'go_gen') whose auto-generated source files will be excluded from analysis # +restricted_mode = false # when true, skip operations that require elevated permissions (e.g. pushing code to the repository) is_auto_command = false # will be auto-set to true if the command is triggered by an automation enable_ai_metadata = false # will enable adding ai metadata reasoning_effort = "medium" # "none", "minimal", "low", "medium", "high", "xhigh" diff --git a/pr_agent/tools/pr_update_changelog.py b/pr_agent/tools/pr_update_changelog.py index e6c1788bb3..28f96128b5 100644 --- a/pr_agent/tools/pr_update_changelog.py +++ b/pr_agent/tools/pr_update_changelog.py @@ -23,14 +23,30 @@ class PRUpdateChangelog: def __init__(self, pr_url: str, cli_mode=False, args=None, ai_handler: partial[BaseAiHandler,] = LiteLLMAIHandler): self.git_provider = get_git_provider()(pr_url) + + # Determine whether pushing the changelog to the repo is both requested and possible. + # If a push is requested but not possible — the provider has no push support, or + # restricted_mode disables the "push_code" capability — degrade gracefully: still + # generate the changelog and publish it as a comment (which only needs + # pull-requests: write) instead of skipping the tool and dropping the output entirely. + self.push_changelog_changes = get_settings().pr_update_changelog.push_changelog_changes + self.push_skipped_reason = None + if self.push_changelog_changes: + if not hasattr(self.git_provider, "create_or_update_pr_file"): + self.push_skipped_reason = "not supported for this git provider" + elif not self.git_provider.is_supported("push_code"): + self.push_skipped_reason = "restricted by configuration (restricted_mode)" + # Push only when it was requested AND is possible; otherwise fall back to a comment. + self.commit_changelog = self.push_changelog_changes and self.push_skipped_reason is None + self.main_language = get_main_pr_language( self.git_provider.get_languages(), self.git_provider.get_files() ) - self.commit_changelog = get_settings().pr_update_changelog.push_changelog_changes self._get_changelog_file() # self.changelog_file_str self.ai_handler = ai_handler() - self.ai_handler.main_pr_language = self.main_language + if self.main_language: + self.ai_handler.main_pr_language = self.main_language self.patches_diff = None self.prediction = None @@ -54,22 +70,15 @@ def __init__(self, pr_url: str, cli_mode=False, args=None, ai_handler: partial[B async def run(self): get_logger().info('Updating the changelog...') - relevant_configs = {'pr_update_changelog': dict(get_settings().pr_update_changelog), - 'config': dict(get_settings().config)} - get_logger().debug("Relevant configs", artifacts=relevant_configs) - - # check if the git provider supports pushing changelog changes - if get_settings().pr_update_changelog.push_changelog_changes and not hasattr( - self.git_provider, "create_or_update_pr_file" - ): - get_logger().error( - "Pushing changelog changes is not currently supported for this code platform" + + # If a push was requested but isn't possible (unsupported provider or restricted_mode), + # the changelog is still generated and published as a comment below (commit_changelog is + # already False in that case), so the output is not dropped. + if self.push_skipped_reason: + get_logger().info( + f"Pushing changelog changes is {self.push_skipped_reason}; " + f"publishing the changelog as a comment instead" ) - if get_settings().config.publish_output: - self.git_provider.publish_comment( - "Pushing changelog changes is not currently supported for this code platform" - ) - return if get_settings().config.publish_output: self.git_provider.publish_comment("Preparing changelog updates...", is_temporary=True) @@ -89,7 +98,13 @@ async def run(self): if self.commit_changelog: self._push_changelog_update(new_file_content, answer) else: - self.git_provider.publish_comment(f"**Changelog updates:** 🔄\n\n{answer}") + changelog_comment = f"**Changelog updates:** 🔄\n\n{answer}" + if self.push_skipped_reason: + changelog_comment += ( + f"\n\n> ℹī¸ These changes were not pushed to the repository " + f"({self.push_skipped_reason})." + ) + self.git_provider.publish_comment(changelog_comment) async def _prepare_prediction(self, model: str): self.patches_diff = get_pr_diff(self.git_provider, self.token_handler, model) diff --git a/tests/unittest/test_pr_update_changelog.py b/tests/unittest/test_pr_update_changelog.py index fe87b4e1a1..02e2eb892b 100644 --- a/tests/unittest/test_pr_update_changelog.py +++ b/tests/unittest/test_pr_update_changelog.py @@ -138,23 +138,83 @@ def test_prepare_changelog_update_no_commit(self, changelog_tool): assert new_content == "## v1.1.0\n- New feature" assert "to commit the new content" in answer + def _make_no_push_provider(self, extra_spec=None): + spec = ["publish_comment", "remove_initial_comment", "get_pr_branch", "get_pr_description", + "get_commit_messages", "get_languages", "get_files", "get_pr_file_content", + "is_supported", "pr"] + if extra_spec: + spec += extra_spec + provider = MagicMock(spec=spec) + provider.pr = MagicMock() + provider.pr.title = "Test PR" + provider.get_pr_branch.return_value = "feature-branch" + provider.get_pr_description.return_value = "Test description" + provider.get_commit_messages.return_value = "fix: test commit" + provider.get_languages.return_value = {"Python": 80, "JavaScript": 20} + provider.get_files.return_value = ["test.py", "test.js"] + provider.get_pr_file_content.return_value = "" + return provider + @pytest.mark.asyncio - async def test_run_without_push_support(self, changelog_tool, mock_git_provider): - """Test running changelog update when git provider doesn't support pushing.""" - # Arrange - delattr(mock_git_provider, 'create_or_update_pr_file') # Remove the method - changelog_tool.commit_changelog = True - - with patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings: + async def test_run_without_push_support(self, mock_ai_handler): + """When the provider can't push (no create_or_update_pr_file), the changelog must still + be generated and published as a comment (graceful degradation), not dropped entirely.""" + provider = self._make_no_push_provider() # spec omits create_or_update_pr_file + provider.is_supported.return_value = True + + with patch('pr_agent.tools.pr_update_changelog.get_git_provider', return_value=lambda url: provider), \ + patch('pr_agent.tools.pr_update_changelog.get_main_pr_language', return_value="Python"), \ + patch('pr_agent.tools.pr_update_changelog.retry_with_fallback_models'), \ + patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings: mock_settings.return_value.pr_update_changelog.push_changelog_changes = True mock_settings.return_value.config.publish_output = True - - # Act - await changelog_tool.run() - - # Assert - mock_git_provider.publish_comment.assert_called_once() - assert "not currently supported" in str(mock_git_provider.publish_comment.call_args) + mock_settings.return_value.pr_update_changelog.extra_instructions = "" + mock_settings.return_value.pr_update_changelog_prompt.system = "" + mock_settings.return_value.pr_update_changelog_prompt.user = "" + mock_settings.return_value.get.return_value = {} + tool = PRUpdateChangelog("https://example.com/pr/123", ai_handler=lambda: mock_ai_handler) + + # Push isn't possible -> degrade to comment mode (don't push, don't drop the output). + assert tool.push_skipped_reason == "not supported for this git provider" + assert tool.commit_changelog is False + + tool.prediction = "## v1.1.0\n- New feature" + await tool.run() + + published = " ".join(str(c) for c in provider.publish_comment.call_args_list) + assert "Changelog updates" in published # the generated changelog was posted + assert "not pushed" in published # with a note it wasn't committed + + @pytest.mark.asyncio + async def test_run_restricted_mode_publishes_comment_instead_of_pushing(self, mock_ai_handler): + """restricted_mode: the provider supports the push API, but is_supported('push_code') is + False, so the changelog must be published as a comment rather than pushed to the repo.""" + provider = self._make_no_push_provider(extra_spec=["create_or_update_pr_file"]) + provider.is_supported.return_value = False # restricted_mode disables push_code + + with patch('pr_agent.tools.pr_update_changelog.get_git_provider', return_value=lambda url: provider), \ + patch('pr_agent.tools.pr_update_changelog.get_main_pr_language', return_value="Python"), \ + patch('pr_agent.tools.pr_update_changelog.retry_with_fallback_models'), \ + patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings: + mock_settings.return_value.pr_update_changelog.push_changelog_changes = True + mock_settings.return_value.config.publish_output = True + mock_settings.return_value.pr_update_changelog.extra_instructions = "" + mock_settings.return_value.pr_update_changelog_prompt.system = "" + mock_settings.return_value.pr_update_changelog_prompt.user = "" + mock_settings.return_value.get.return_value = {} + tool = PRUpdateChangelog("https://example.com/pr/1", ai_handler=lambda: mock_ai_handler) + + assert tool.push_skipped_reason == "restricted by configuration (restricted_mode)" + assert tool.commit_changelog is False + provider.is_supported.assert_called_with("push_code") + + tool.prediction = "## v1.1.0\n- feat" + await tool.run() + + provider.create_or_update_pr_file.assert_not_called() # never pushed + published = " ".join(str(c) for c in provider.publish_comment.call_args_list) + assert "Changelog updates" in published + assert "not pushed" in published @pytest.mark.asyncio async def test_run_with_push_support(self, changelog_tool, mock_git_provider):