Skip to content

Commit cc53c74

Browse files
utsab345Utsabnaorpeled
authored
feat: add restricted mode for reduced GitHub permissions (#2279) (#2491)
* fix: scope ticket cache per-PR and fix Dynaconf list merge leaks in tests - Cache key is now 'related_tickets_<md5(pr_url)>' instead of global 'related_tickets', preventing stale tickets leaking between unrelated PRs - Added cache_tickets=true config flag under [pr_reviewer] to allow disabling - Fixed Dynaconf merge_enabled=True list append bug in both production code (env_bridge.py uses case-insensitive pop before set) and test helpers (restore_settings now calls _remove_key before every set) - Added azure-identity dependency for Azure DevOps provider availability - 1043 tests pass, 0 failures * Revert "fix: scope ticket cache per-PR and fix Dynaconf list merge leaks in tests" This reverts commit 7f0d154. * feat: add restricted mode for reduced GitHub permissions (#2279) Add a `config.restricted_mode` option that gates operations requiring elevated permissions (e.g. pushing code to the repository). When enabled, `is_supported("push_code")` returns `False` on GitHub, GitLab, and Bitbucket providers, and `/update_changelog --push_changelog_changes=true` gracefully skips with a clear message instead of failing. Users can set `restricted_mode = true` and use only `issues: write` and `pull-requests: write` permissions, without `contents: write`. * docs: document restricted_mode for reduced GitHub permissions * docs: clarify minimal permissions for restricted mode * fix: move restricted-mode check to __init__ to avoid unnecessary provider reads PRUpdateChangelog.__init__() was fetching languages, files, and CHANGELOG.md content before run() checked push_code support. In restricted mode, these reads were wasted and could fail if contents access is also restricted. Move the capability check into __init__() before the expensive provider calls, and store the result in self._skip_push so run() can return early. * fix: remove full-config logging from run() and use mock AI handler in test - Remove relevant_configs logging that dumps full settings dicts (potential secret leak in logs) - Pass ai_handler mock in test_run_without_push_support to avoid instantiating a real LiteLLMAIHandler * fix: use spec'd mock for push-support test and fix docs permissions note - Use MagicMock(spec=[...]) in test_run_without_push_support so hasattr() correctly returns False for missing attributes instead of brittle delattr on bare MagicMock - Remove misleading 'contents: read (defaults to read)' comment from restricted mode docs; contents is simply not needed * fix: skip __init__ provider reads when _skip_push is set Move all provider-dependent initialization (vars dict, token handler, ai_handler binding, language/file/changelog reads) behind the _skip_push guard so the skip path avoids hitting the git provider at all during construction. * fix(update_changelog): degrade to comment instead of dropping output in restricted mode Address Qodo review on #2491: - #4 (skip path drops changelog output): when a changelog push is requested but not possible — provider lacks push support, or restricted_mode disables push_code — the tool no longer returns early with only an error. It now still generates the changelog and publishes it as a comment (which only needs pull-requests: write), with a note that the changes were not pushed. commit_changelog is set to False in that case. - #5 (skip path still initializes): removes the special early-return init path; the normal flow builds only what's needed to generate the changelog. - #6 (contents default note misleading): clarify that unlisted scopes default to none only within an explicit permissions: block; without one, defaults follow repo/org settings. Update the "no push support" test to assert the comment fallback, and add a restricted_mode test (push API present but is_supported('push_code') False) asserting it comments, not pushes. * fix: generate changelog preview instead of silent skip when push is restricted When restricted_mode is active and push_changelog_changes=true, the tool now generates the changelog output via AI and publishes it as a comment with a '(push restricted by configuration)' label, instead of returning silently. This ensures users see the changelog even when they don't have contents: write permissions. For the 'not supported' case (provider lacks create_or_update_pr_file entirely), the existing early-return behavior is preserved. --------- Co-authored-by: Utsab <utsab@pr-agent.dev> Co-authored-by: naorpeled <me@naor.dev>
1 parent c8852c1 commit cc53c74

8 files changed

Lines changed: 142 additions & 32 deletions

File tree

docs/docs/installation/github.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,13 @@ If you encounter rate limiting:
468468
pull-requests: write
469469
contents: write
470470
```
471+
If you cannot grant `contents: write`, set `config.restricted_mode = true` in your configuration. In that case you only need:
472+
```yaml
473+
permissions:
474+
issues: write
475+
pull-requests: write
476+
```
477+
See the [Restricted Mode guide](../usage-guide/additional_configurations.md#restricted-mode) for details.
471478

472479
**Error: "Invalid JSON format"**
473480

docs/docs/usage-guide/additional_configurations.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,3 +280,24 @@ ignore_ticket_labels = ["ignore-compliance", "skip-review", "wont-fix"]
280280
```
281281

282282
Where `ignore_ticket_labels` is a list of label names that should be ignored during ticket analysis.
283+
284+
### Restricted Mode
285+
286+
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):
287+
288+
```toml
289+
[config]
290+
restricted_mode = true
291+
```
292+
293+
With restricted mode, the minimum workflow permissions are:
294+
295+
```yaml
296+
permissions:
297+
issues: write
298+
pull-requests: write
299+
```
300+
301+
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`.
302+
303+
> **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.

pr_agent/git_providers/bitbucket_provider.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ def is_supported(self, capability: str) -> bool:
192192
if capability in ['get_issue_comments', 'publish_inline_comments', 'get_labels', 'gfm_markdown',
193193
'publish_file_comments']:
194194
return False
195+
if capability == "push_code" and get_settings().config.restricted_mode:
196+
return False
195197
return True
196198

197199
def set_pr(self, pr_url: str):

pr_agent/git_providers/github_provider.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ def get_incremental_commits(self, incremental=IncrementalPR(False)):
9696
self._get_incremental_commits()
9797

9898
def is_supported(self, capability: str) -> bool:
99+
if capability == "push_code" and get_settings().config.restricted_mode:
100+
return False
99101
return True
100102

101103
def _get_owner_and_repo_path(self, given_url: str) -> str:

pr_agent/git_providers/gitlab_provider.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ def is_supported(self, capability: str) -> bool:
290290
if capability in ['get_issue_comments', 'create_inline_comment', 'publish_inline_comments',
291291
'publish_file_comments']: # gfm_markdown is supported in gitlab !
292292
return False
293+
if capability == "push_code" and get_settings().config.restricted_mode:
294+
return False
293295
return True
294296

295297
def _get_project_path_from_pr_or_issue_url(self, pr_or_issue_url: str) -> str:

pr_agent/settings/configuration.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ ignore_pr_authors = [] # authors to ignore from PR agent when an PR is created
5757
ignore_repositories = [] # a list of regular expressions of repository full names (e.g. "org/repo") to ignore from PR agent processing
5858
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
5959
#
60+
restricted_mode = false # when true, skip operations that require elevated permissions (e.g. pushing code to the repository)
6061
is_auto_command = false # will be auto-set to true if the command is triggered by an automation
6162
enable_ai_metadata = false # will enable adding ai metadata
6263
reasoning_effort = "medium" # "none", "minimal", "low", "medium", "high", "xhigh"

pr_agent/tools/pr_update_changelog.py

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,30 @@ class PRUpdateChangelog:
2323
def __init__(self, pr_url: str, cli_mode=False, args=None, ai_handler: partial[BaseAiHandler,] = LiteLLMAIHandler):
2424

2525
self.git_provider = get_git_provider()(pr_url)
26+
27+
# Determine whether pushing the changelog to the repo is both requested and possible.
28+
# If a push is requested but not possible — the provider has no push support, or
29+
# restricted_mode disables the "push_code" capability — degrade gracefully: still
30+
# generate the changelog and publish it as a comment (which only needs
31+
# pull-requests: write) instead of skipping the tool and dropping the output entirely.
32+
self.push_changelog_changes = get_settings().pr_update_changelog.push_changelog_changes
33+
self.push_skipped_reason = None
34+
if self.push_changelog_changes:
35+
if not hasattr(self.git_provider, "create_or_update_pr_file"):
36+
self.push_skipped_reason = "not supported for this git provider"
37+
elif not self.git_provider.is_supported("push_code"):
38+
self.push_skipped_reason = "restricted by configuration (restricted_mode)"
39+
# Push only when it was requested AND is possible; otherwise fall back to a comment.
40+
self.commit_changelog = self.push_changelog_changes and self.push_skipped_reason is None
41+
2642
self.main_language = get_main_pr_language(
2743
self.git_provider.get_languages(), self.git_provider.get_files()
2844
)
29-
self.commit_changelog = get_settings().pr_update_changelog.push_changelog_changes
3045
self._get_changelog_file() # self.changelog_file_str
3146

3247
self.ai_handler = ai_handler()
33-
self.ai_handler.main_pr_language = self.main_language
48+
if self.main_language:
49+
self.ai_handler.main_pr_language = self.main_language
3450

3551
self.patches_diff = None
3652
self.prediction = None
@@ -54,22 +70,15 @@ def __init__(self, pr_url: str, cli_mode=False, args=None, ai_handler: partial[B
5470

5571
async def run(self):
5672
get_logger().info('Updating the changelog...')
57-
relevant_configs = {'pr_update_changelog': dict(get_settings().pr_update_changelog),
58-
'config': dict(get_settings().config)}
59-
get_logger().debug("Relevant configs", artifacts=relevant_configs)
60-
61-
# check if the git provider supports pushing changelog changes
62-
if get_settings().pr_update_changelog.push_changelog_changes and not hasattr(
63-
self.git_provider, "create_or_update_pr_file"
64-
):
65-
get_logger().error(
66-
"Pushing changelog changes is not currently supported for this code platform"
73+
74+
# If a push was requested but isn't possible (unsupported provider or restricted_mode),
75+
# the changelog is still generated and published as a comment below (commit_changelog is
76+
# already False in that case), so the output is not dropped.
77+
if self.push_skipped_reason:
78+
get_logger().info(
79+
f"Pushing changelog changes is {self.push_skipped_reason}; "
80+
f"publishing the changelog as a comment instead"
6781
)
68-
if get_settings().config.publish_output:
69-
self.git_provider.publish_comment(
70-
"Pushing changelog changes is not currently supported for this code platform"
71-
)
72-
return
7382

7483
if get_settings().config.publish_output:
7584
self.git_provider.publish_comment("Preparing changelog updates...", is_temporary=True)
@@ -89,7 +98,13 @@ async def run(self):
8998
if self.commit_changelog:
9099
self._push_changelog_update(new_file_content, answer)
91100
else:
92-
self.git_provider.publish_comment(f"**Changelog updates:** 🔄\n\n{answer}")
101+
changelog_comment = f"**Changelog updates:** 🔄\n\n{answer}"
102+
if self.push_skipped_reason:
103+
changelog_comment += (
104+
f"\n\n> ℹ️ These changes were not pushed to the repository "
105+
f"({self.push_skipped_reason})."
106+
)
107+
self.git_provider.publish_comment(changelog_comment)
93108

94109
async def _prepare_prediction(self, model: str):
95110
self.patches_diff = get_pr_diff(self.git_provider, self.token_handler, model)

tests/unittest/test_pr_update_changelog.py

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,23 +138,83 @@ def test_prepare_changelog_update_no_commit(self, changelog_tool):
138138
assert new_content == "## v1.1.0\n- New feature"
139139
assert "to commit the new content" in answer
140140

141+
def _make_no_push_provider(self, extra_spec=None):
142+
spec = ["publish_comment", "remove_initial_comment", "get_pr_branch", "get_pr_description",
143+
"get_commit_messages", "get_languages", "get_files", "get_pr_file_content",
144+
"is_supported", "pr"]
145+
if extra_spec:
146+
spec += extra_spec
147+
provider = MagicMock(spec=spec)
148+
provider.pr = MagicMock()
149+
provider.pr.title = "Test PR"
150+
provider.get_pr_branch.return_value = "feature-branch"
151+
provider.get_pr_description.return_value = "Test description"
152+
provider.get_commit_messages.return_value = "fix: test commit"
153+
provider.get_languages.return_value = {"Python": 80, "JavaScript": 20}
154+
provider.get_files.return_value = ["test.py", "test.js"]
155+
provider.get_pr_file_content.return_value = ""
156+
return provider
157+
141158
@pytest.mark.asyncio
142-
async def test_run_without_push_support(self, changelog_tool, mock_git_provider):
143-
"""Test running changelog update when git provider doesn't support pushing."""
144-
# Arrange
145-
delattr(mock_git_provider, 'create_or_update_pr_file') # Remove the method
146-
changelog_tool.commit_changelog = True
147-
148-
with patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings:
159+
async def test_run_without_push_support(self, mock_ai_handler):
160+
"""When the provider can't push (no create_or_update_pr_file), the changelog must still
161+
be generated and published as a comment (graceful degradation), not dropped entirely."""
162+
provider = self._make_no_push_provider() # spec omits create_or_update_pr_file
163+
provider.is_supported.return_value = True
164+
165+
with patch('pr_agent.tools.pr_update_changelog.get_git_provider', return_value=lambda url: provider), \
166+
patch('pr_agent.tools.pr_update_changelog.get_main_pr_language', return_value="Python"), \
167+
patch('pr_agent.tools.pr_update_changelog.retry_with_fallback_models'), \
168+
patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings:
149169
mock_settings.return_value.pr_update_changelog.push_changelog_changes = True
150170
mock_settings.return_value.config.publish_output = True
151-
152-
# Act
153-
await changelog_tool.run()
154-
155-
# Assert
156-
mock_git_provider.publish_comment.assert_called_once()
157-
assert "not currently supported" in str(mock_git_provider.publish_comment.call_args)
171+
mock_settings.return_value.pr_update_changelog.extra_instructions = ""
172+
mock_settings.return_value.pr_update_changelog_prompt.system = ""
173+
mock_settings.return_value.pr_update_changelog_prompt.user = ""
174+
mock_settings.return_value.get.return_value = {}
175+
tool = PRUpdateChangelog("https://example.com/pr/123", ai_handler=lambda: mock_ai_handler)
176+
177+
# Push isn't possible -> degrade to comment mode (don't push, don't drop the output).
178+
assert tool.push_skipped_reason == "not supported for this git provider"
179+
assert tool.commit_changelog is False
180+
181+
tool.prediction = "## v1.1.0\n- New feature"
182+
await tool.run()
183+
184+
published = " ".join(str(c) for c in provider.publish_comment.call_args_list)
185+
assert "Changelog updates" in published # the generated changelog was posted
186+
assert "not pushed" in published # with a note it wasn't committed
187+
188+
@pytest.mark.asyncio
189+
async def test_run_restricted_mode_publishes_comment_instead_of_pushing(self, mock_ai_handler):
190+
"""restricted_mode: the provider supports the push API, but is_supported('push_code') is
191+
False, so the changelog must be published as a comment rather than pushed to the repo."""
192+
provider = self._make_no_push_provider(extra_spec=["create_or_update_pr_file"])
193+
provider.is_supported.return_value = False # restricted_mode disables push_code
194+
195+
with patch('pr_agent.tools.pr_update_changelog.get_git_provider', return_value=lambda url: provider), \
196+
patch('pr_agent.tools.pr_update_changelog.get_main_pr_language', return_value="Python"), \
197+
patch('pr_agent.tools.pr_update_changelog.retry_with_fallback_models'), \
198+
patch('pr_agent.tools.pr_update_changelog.get_settings') as mock_settings:
199+
mock_settings.return_value.pr_update_changelog.push_changelog_changes = True
200+
mock_settings.return_value.config.publish_output = True
201+
mock_settings.return_value.pr_update_changelog.extra_instructions = ""
202+
mock_settings.return_value.pr_update_changelog_prompt.system = ""
203+
mock_settings.return_value.pr_update_changelog_prompt.user = ""
204+
mock_settings.return_value.get.return_value = {}
205+
tool = PRUpdateChangelog("https://example.com/pr/1", ai_handler=lambda: mock_ai_handler)
206+
207+
assert tool.push_skipped_reason == "restricted by configuration (restricted_mode)"
208+
assert tool.commit_changelog is False
209+
provider.is_supported.assert_called_with("push_code")
210+
211+
tool.prediction = "## v1.1.0\n- feat"
212+
await tool.run()
213+
214+
provider.create_or_update_pr_file.assert_not_called() # never pushed
215+
published = " ".join(str(c) for c in provider.publish_comment.call_args_list)
216+
assert "Changelog updates" in published
217+
assert "not pushed" in published
158218

159219
@pytest.mark.asyncio
160220
async def test_run_with_push_support(self, changelog_tool, mock_git_provider):

0 commit comments

Comments
 (0)