diff --git a/gateway/gateway.py b/gateway/gateway.py index 0c64d7366a..6112fcb6db 100644 --- a/gateway/gateway.py +++ b/gateway/gateway.py @@ -1398,8 +1398,12 @@ def gh_pr_create() -> tuple[Response, int] | Response: # Get session mode from request context (set by @require_session_auth decorator) session_mode = getattr(g, "session_mode", None) - # Block PR creation in local SDLC mode - if session_mode == "local": + # Get session phase from request context (set by @require_session_auth decorator) + session_phase = getattr(g, "session_phase", None) + + # Block PR creation in local SDLC mode (except during PR phase, where + # phase-permissions grant it and the gateway provides push access). + if session_mode == "local" and session_phase != "pr": audit_log( "pr_create_blocked_local_mode", "gh_pr_create", @@ -1412,9 +1416,6 @@ def gh_pr_create() -> tuple[Response, int] | Response: details={"session_mode": "local"}, ) - # Get session phase from request context (set by @require_session_auth decorator) - session_phase = getattr(g, "session_phase", None) - # Check phase restrictions (if session has a phase set) if session_phase: try: @@ -1599,8 +1600,9 @@ def gh_pr_comment() -> tuple[Response, int] | Response: # Get session mode from request context (set by @require_session_auth decorator) session_mode = getattr(g, "session_mode", None) - # Block PR comment in local SDLC mode - if session_mode == "local": + # Block PR comment in local SDLC mode (except during PR phase) + session_phase = getattr(g, "session_phase", None) + if session_mode == "local" and session_phase != "pr": audit_log( "pr_comment_blocked_local_mode", "gh_pr_comment", @@ -1731,8 +1733,9 @@ def gh_pr_edit() -> tuple[Response, int] | Response: # Get session mode from request context (set by @require_session_auth decorator) session_mode = getattr(g, "session_mode", None) - # Block PR edit in local SDLC mode - if session_mode == "local": + # Block PR edit in local SDLC mode (except during PR phase) + session_phase = getattr(g, "session_phase", None) + if session_mode == "local" and session_phase != "pr": audit_log( "pr_edit_blocked_local_mode", "gh_pr_edit", @@ -1853,8 +1856,9 @@ def gh_pr_close() -> tuple[Response, int] | Response: # Get session mode from request context (set by @require_session_auth decorator) session_mode = getattr(g, "session_mode", None) - # Block PR close in local SDLC mode - if session_mode == "local": + # Block PR close in local SDLC mode (except during PR phase) + session_phase = getattr(g, "session_phase", None) + if session_mode == "local" and session_phase != "pr": audit_log( "pr_close_blocked_local_mode", "gh_pr_close", @@ -1970,19 +1974,39 @@ def gh_execute() -> tuple[Response, int] | Response: # Get session mode from request context (set by @require_session_auth decorator) session_mode = getattr(g, "session_mode", None) - # Block all gh commands in local SDLC mode + # Block gh commands in local SDLC mode. + # During PR phase, only allow PR-scoped operations through. + # All other gh commands remain blocked. + session_phase = getattr(g, "session_phase", None) if session_mode == "local": - audit_log( - "gh_command_blocked_local_mode", - "gh_execute", - success=False, - details={"args": args, "reason": "gh commands blocked in local SDLC mode"}, - ) - return make_error( - "Operation blocked in local SDLC mode. Run gh commands manually when the pipeline completes.", - status_code=403, - details={"session_mode": "local"}, - ) + allowed = False + if session_phase == "pr": + cmd_prefix = " ".join(args[:2]) if len(args) >= 2 else args[0] if args else "" + allowed_pr_phase_prefixes = ( + "pr create", + "pr edit", + "pr view", + "pr list", + "pr comment", + "pr close", + "pr diff", + "pr checks", + "pr status", + ) + allowed = any(cmd_prefix.startswith(p) for p in allowed_pr_phase_prefixes) + + if not allowed: + audit_log( + "gh_command_blocked_local_mode", + "gh_execute", + success=False, + details={"command_args": args, "reason": "gh commands blocked in local SDLC mode"}, + ) + return make_error( + "Operation blocked in local SDLC mode. Run gh commands manually when the pipeline completes.", + status_code=403, + details={"session_mode": "local"}, + ) # Check for commands blocked entirely in private mode (too broad to filter by repo) if session_mode == "private" and args and args[0] in GH_COMMANDS_BLOCKED_IN_PRIVATE_MODE: diff --git a/gateway/tests/test_gateway.py b/gateway/tests/test_gateway.py index a251d8cf47..723f0e459c 100644 --- a/gateway/tests/test_gateway.py +++ b/gateway/tests/test_gateway.py @@ -2931,3 +2931,469 @@ def test_session_create_validates_phase(self, client, launcher_auth_headers): assert response.status_code == 400 data = json.loads(response.data) assert "invalid" in data["message"].lower() + + +class TestLocalModeBlocking: + """Tests for local SDLC mode blocking across PR endpoints and gh_execute. + + Verifies that: + - Endpoints return 403 when session_mode='local' and phase is not 'pr' + - Endpoints allow requests when session_mode='local' and phase='pr' + - gh_execute only allows PR-related commands in local + PR phase + """ + + @pytest.fixture + def local_mode_headers(self): + """Return session headers with local mode and configurable phase.""" + import sys + + import auth + + def _make_headers(phase: str | None): + mock_session = MagicMock() + mock_session.mode = "local" + mock_session.container_id = "test-container" + mock_session.expires_at = None + mock_session.phase = phase + + mock_result = SessionValidationResult(valid=True, session=mock_session) + + from private_repo_policy import PrivateRepoPolicyResult + + mock_policy_result = PrivateRepoPolicyResult( + allowed=True, + reason="Test mode - access allowed", + visibility="public", + ) + + auth._session_manager = None + auth._rate_limiter = None + + if "gateway.auth" in sys.modules: + sys.modules["gateway.auth"]._session_manager = None + sys.modules["gateway.auth"]._rate_limiter = None + + current_session_manager = sys.modules.get("session_manager", session_manager) + + return ( + {"Authorization": "Bearer test-session-token"}, + mock_result, + mock_policy_result, + current_session_manager, + ) + + return _make_headers + + # --- Blocking tests: phase is not 'pr' --- + + def test_pr_create_blocked_local_mode_implement_phase(self, client, local_mode_headers): + """PR create returns 403 in local mode when phase is 'implement'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers( + "implement" + ) + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/pr/create", + headers=headers, + data=json.dumps({"repo": "test/repo", "title": "Test PR", "head": "feature"}), + content_type="application/json", + ) + + assert response.status_code == 403 + data = json.loads(response.data) + assert "local" in data["message"].lower() + + def test_pr_comment_blocked_local_mode_no_phase(self, client, local_mode_headers): + """PR comment returns 403 in local mode when phase is None.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers(None) + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/pr/comment", + headers=headers, + data=json.dumps({"repo": "test/repo", "pr_number": 1, "body": "comment"}), + content_type="application/json", + ) + + assert response.status_code == 403 + data = json.loads(response.data) + assert "local" in data["message"].lower() + + def test_pr_edit_blocked_local_mode_implement_phase(self, client, local_mode_headers): + """PR edit returns 403 in local mode when phase is 'implement'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers( + "implement" + ) + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/pr/edit", + headers=headers, + data=json.dumps({"repo": "test/repo", "pr_number": 1, "title": "New Title"}), + content_type="application/json", + ) + + assert response.status_code == 403 + data = json.loads(response.data) + assert "local" in data["message"].lower() + + def test_pr_close_blocked_local_mode_implement_phase(self, client, local_mode_headers): + """PR close returns 403 in local mode when phase is 'implement'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers( + "implement" + ) + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/pr/close", + headers=headers, + data=json.dumps({"repo": "test/repo", "pr_number": 1}), + content_type="application/json", + ) + + assert response.status_code == 403 + data = json.loads(response.data) + assert "local" in data["message"].lower() + + def test_gh_execute_blocked_local_mode_no_phase(self, client, local_mode_headers): + """gh execute returns 403 in local mode when phase is None.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers(None) + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["pr", "list"]}), + content_type="application/json", + ) + + assert response.status_code == 403 + data = json.loads(response.data) + assert "local" in data["message"].lower() + + def test_gh_execute_blocked_local_mode_implement_phase(self, client, local_mode_headers): + """gh execute returns 403 in local mode when phase is 'implement'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers( + "implement" + ) + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["issue", "list"]}), + content_type="application/json", + ) + + assert response.status_code == 403 + + # --- Allow tests: phase is 'pr' --- + + def test_pr_create_allowed_local_mode_pr_phase(self, client, local_mode_headers): + """PR create succeeds in local mode when phase is 'pr'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + patch.object(gateway, "get_github_client") as mock_gh, + ): + mock_gh_result = MagicMock() + mock_gh_result.success = True + mock_gh_result.stdout = "https://github.com/test/repo/pull/1" + mock_gh_result.stderr = "" + mock_gh_result.to_dict.return_value = { + "success": True, + "stdout": "https://github.com/test/repo/pull/1", + "stderr": "", + } + mock_gh.return_value.execute.return_value = mock_gh_result + + response = client.post( + "/api/v1/gh/pr/create", + headers=headers, + data=json.dumps({"repo": "test/repo", "title": "Test PR", "head": "feature"}), + content_type="application/json", + ) + + assert response.status_code == 200 + + def test_pr_comment_allowed_local_mode_pr_phase(self, client, local_mode_headers): + """PR comment succeeds in local mode when phase is 'pr'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + patch.object(gateway, "get_policy_engine") as mock_policy, + patch.object(gateway, "get_github_client") as mock_gh, + ): + mock_engine = MagicMock() + mock_engine.check_pr_ownership.return_value = PolicyResult( + allowed=True, + reason="PR is owned by bot", + details={"author": "bot"}, + ) + mock_policy.return_value = mock_engine + + mock_gh_result = MagicMock() + mock_gh_result.success = True + mock_gh_result.stdout = "comment posted" + mock_gh_result.stderr = "" + mock_gh.return_value.execute.return_value = mock_gh_result + + response = client.post( + "/api/v1/gh/pr/comment", + headers=headers, + data=json.dumps({"repo": "test/repo", "pr_number": 1, "body": "LGTM"}), + content_type="application/json", + ) + + assert response.status_code == 200 + + def test_pr_edit_allowed_local_mode_pr_phase(self, client, local_mode_headers): + """PR edit succeeds in local mode when phase is 'pr'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + patch.object(gateway, "get_policy_engine") as mock_policy, + patch.object(gateway, "get_github_client") as mock_gh, + ): + mock_engine = MagicMock() + mock_engine.check_pr_ownership.return_value = PolicyResult( + allowed=True, + reason="PR is owned by bot", + details={"author": "bot"}, + ) + mock_policy.return_value = mock_engine + + mock_gh_result = MagicMock() + mock_gh_result.success = True + mock_gh_result.stdout = "PR updated" + mock_gh_result.stderr = "" + mock_gh.return_value.execute.return_value = mock_gh_result + + response = client.post( + "/api/v1/gh/pr/edit", + headers=headers, + data=json.dumps({"repo": "test/repo", "pr_number": 1, "title": "Updated"}), + content_type="application/json", + ) + + assert response.status_code == 200 + + def test_pr_close_allowed_local_mode_pr_phase(self, client, local_mode_headers): + """PR close succeeds in local mode when phase is 'pr'.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + patch.object(gateway, "get_policy_engine") as mock_policy, + patch.object(gateway, "get_github_client") as mock_gh, + ): + mock_engine = MagicMock() + mock_engine.check_pr_ownership.return_value = PolicyResult( + allowed=True, + reason="PR is owned by bot", + details={"author": "bot"}, + ) + mock_policy.return_value = mock_engine + + mock_gh_result = MagicMock() + mock_gh_result.success = True + mock_gh_result.stdout = "PR closed" + mock_gh_result.stderr = "" + mock_gh.return_value.execute.return_value = mock_gh_result + + response = client.post( + "/api/v1/gh/pr/close", + headers=headers, + data=json.dumps({"repo": "test/repo", "pr_number": 1}), + content_type="application/json", + ) + + assert response.status_code == 200 + + # --- gh_execute scope tests: only PR commands allowed in local + PR phase --- + + def test_gh_execute_allows_pr_list_local_pr_phase(self, client, local_mode_headers): + """gh execute allows 'pr list' in local mode during PR phase.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + patch.object(gateway, "get_github_client") as mock_gh, + ): + mock_gh_result = MagicMock() + mock_gh_result.success = True + mock_gh_result.stdout = "PR #1: Feature" + mock_gh_result.stderr = "" + mock_gh_result.to_dict.return_value = { + "success": True, + "stdout": "PR #1: Feature", + "stderr": "", + } + mock_gh.return_value.execute.return_value = mock_gh_result + + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["pr", "list"]}), + content_type="application/json", + ) + + assert response.status_code == 200 + + def test_gh_execute_allows_pr_view_local_pr_phase(self, client, local_mode_headers): + """gh execute allows 'pr view' in local mode during PR phase.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + patch.object(gateway, "get_github_client") as mock_gh, + ): + mock_gh_result = MagicMock() + mock_gh_result.success = True + mock_gh_result.stdout = "PR details" + mock_gh_result.stderr = "" + mock_gh_result.to_dict.return_value = { + "success": True, + "stdout": "PR details", + "stderr": "", + } + mock_gh.return_value.execute.return_value = mock_gh_result + + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["pr", "view", "123"]}), + content_type="application/json", + ) + + assert response.status_code == 200 + + def test_gh_execute_blocks_issue_edit_local_pr_phase(self, client, local_mode_headers): + """gh execute blocks 'issue edit' in local mode during PR phase.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["issue", "edit", "1", "--title", "new"]}), + content_type="application/json", + ) + + assert response.status_code == 403 + data = json.loads(response.data) + assert "local" in data["message"].lower() + + def test_gh_execute_blocks_release_create_local_pr_phase(self, client, local_mode_headers): + """gh execute blocks 'release create' in local mode during PR phase.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["release", "create", "v1.0"]}), + content_type="application/json", + ) + + assert response.status_code == 403 + + def test_gh_execute_blocks_repo_edit_local_pr_phase(self, client, local_mode_headers): + """gh execute blocks 'repo edit' in local mode during PR phase.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["repo", "edit", "--description", "new"]}), + content_type="application/json", + ) + + assert response.status_code == 403 + + def test_gh_execute_blocks_api_command_local_pr_phase(self, client, local_mode_headers): + """gh execute blocks 'api' command in local mode during PR phase.""" + headers, mock_result, mock_policy_result, current_session_manager = local_mode_headers("pr") + + with ( + patch.object( + current_session_manager, "validate_session_for_request", return_value=mock_result + ), + patch.object(gateway, "check_private_repo_access", return_value=mock_policy_result), + ): + response = client.post( + "/api/v1/gh/execute", + headers=headers, + data=json.dumps({"args": ["api", "repos/test/repo/issues"]}), + content_type="application/json", + ) + + assert response.status_code == 403 diff --git a/orchestrator/routes/pipelines.py b/orchestrator/routes/pipelines.py index 9fe3d8a276..bafe08d8d3 100644 --- a/orchestrator/routes/pipelines.py +++ b/orchestrator/routes/pipelines.py @@ -1341,7 +1341,7 @@ def _build_phase_prompt( lines.extend( [ "This is a **local** pipeline entering the PR phase.", - "Push access is enabled for this phase only.", + "PR operations are enabled for this phase.", "- You CAN push code (git push)", "- You CAN create and edit PRs (gh pr create, gh pr edit)", "- You CANNOT merge PRs (human must merge)", @@ -2454,12 +2454,6 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: repos = [pipeline.repo] if pipeline.repo else [] - # PR phase gets push access even for local pipelines, unless - # the pipeline is in private mode (which must stay isolated). - phase_gateway_mode = gateway_mode - if current_phase.value == "pr" and pipeline_mode == "local" and gateway_mode != "private": - phase_gateway_mode = "public" - phase_failed = False review_feedback: str | None = hitl_revision_feedback hitl_revision_feedback = None @@ -2522,7 +2516,7 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: phase=current_phase.value, spawner=spawner, repo_volumes=repo_volumes, - gateway_mode=phase_gateway_mode, + gateway_mode=gateway_mode, repos=repos, sandbox_env=sandbox_env, store=store, @@ -2590,7 +2584,7 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: agent_role=AgentRole.CODER, issue_number=pipeline.issue_number, repo_volumes=repo_volumes, - gateway_mode=phase_gateway_mode, + gateway_mode=gateway_mode, repos=repos, phase=current_phase.value, sandbox_env=sandbox_env, @@ -2691,7 +2685,7 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: agent_role=AgentRole.CHECKER, issue_number=pipeline.issue_number, repo_volumes=repo_volumes, - gateway_mode=phase_gateway_mode, + gateway_mode=gateway_mode, repos=repos, phase=current_phase.value, sandbox_env=checker_env, @@ -2760,7 +2754,7 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: agent_role=AgentRole.CODER, issue_number=pipeline.issue_number, repo_volumes=repo_volumes, - gateway_mode=phase_gateway_mode, + gateway_mode=gateway_mode, repos=repos, phase=current_phase.value, sandbox_env=sandbox_env, @@ -2854,7 +2848,7 @@ def _run_pipeline(pipeline_id: str, repo_path: Path) -> None: agent_role=orch_role, issue_number=pipeline.issue_number, repo_volumes=repo_volumes, - gateway_mode=phase_gateway_mode, + gateway_mode=gateway_mode, repos=repos, phase=current_phase.value, sandbox_env=reviewer_env,