Let gateway handle PR-phase access in local mode

[james-in-a-box]

Remove orchestrator mode-switching workaround for local pipelines in PR phase

Previously, the orchestrator switched local-mode pipelines from local to
public gateway mode during the PR phase. This was unnecessary because the
gateway has independent network access to GitHub via the egg-external
Docker network — git push and gh CLI operations go through the gateway
sidecar, not the container's network.

The real issue was that the gateway blanket-blocked all PR operations
(gh pr create, gh pr edit, etc.) in local mode, regardless of pipeline
phase. This fix teaches the gateway to check session_phase and allow PR
operations when the phase is pr, consistent with the phase-permissions
config that already grants pr create during the PR phase.

Gateway modes

Mode	Docker network	Proxy/DNS	repo_mode	PR ops	Use case
public	egg-external	None	public	Allowed	Issue-mode pipelines, full internet
private	egg-isolated	Locked (Anthropic API only)	private	Blocked	Private repos, network lockdown
local	egg-isolated	None	public	Blocked (except PR phase)	Local pipelines, no GitHub interaction by default

The key insight: local and public both use repo_mode=public and have
no proxy. They differ only in Docker network (egg-isolated vs
egg-external). Since all git/gh operations go through the gateway sidecar
(which sits on both networks), the container doesn't need direct internet
access. The gateway handles push and PR operations regardless of which
network the container is on.

Changes

• gateway/gateway.py: Local-mode blocks for PR create/comment/edit/close
  and generic gh commands now check session_phase != "pr" before blocking
• orchestrator/routes/pipelines.py: Remove phase_gateway_mode override
  that switched local pipelines to public during PR phase
• orchestrator/routes/pipelines.py: Update agent prompt wording from
  "Push access is enabled" to "PR operations are enabled"

Issue: none

Test plan:

• pytest gateway/tests/test_gateway.py — 105 passed
• pytest gateway/tests/test_phase_filter.py — 75 passed
• Verify local pipeline PR phase stays in local mode and can still create PRs

Authored-by: egg

[egg-reviewer]

Review Summary

The approach is sound: moving the PR-phase access control from an orchestrator-level mode switch into the gateway's per-endpoint checks is a better design. The gateway already has phase information and can make fine-grained decisions, so the orchestrator shouldn't need to override the gateway mode. The implementation is clean and consistent across the five affected endpoints.

Two issues to address before merging.

---

1. gh_execute catch-all is too permissive in local + PR phase (security concern)

File: gateway/gateway.py, ~line 1979

Before this PR, gh_execute blocked all gh commands in local mode. Now it allows all gh commands through when session_phase == "pr". But the phase-permissions config (phase-permissions.json) only grants pr create* and pr edit * during the PR phase—not arbitrary gh commands.

The dedicated endpoints (gh_pr_create, gh_pr_comment, gh_pr_edit, gh_pr_close) are fine because they're scoped to specific operations. But gh_execute is a catch-all that passes through commands like gh issue edit, gh release create, gh repo edit, etc. These shouldn't be allowed in local mode just because the phase is pr.

The gh_pr_create endpoint has a second layer of defense—it calls filter_operation() to check phase-permissions. gh_execute does not call filter_operation(), so the local-mode check was its primary guard for local pipelines.

Suggested fix: Restrict gh_execute in local + PR phase to only the operations that the PR phase actually needs. For example:

if session_mode == "local":
    if session_phase == "pr":
        # Only allow PR-related commands through in local PR phase
        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")
        if not any(cmd_prefix.startswith(p) for p in allowed_pr_phase_prefixes):
            # block with existing audit_log + make_error
            ...
    else:
        # block all gh commands
        ...

Alternatively, wire filter_operation() into gh_execute for sessions with a phase set, which would be the most robust solution.

2. No test coverage for local-mode blocking or the new phase exception

The test files (test_gateway.py, test_phase_filter.py) have no tests for session_mode == "local" behavior. Specifically missing:

• Blocking tests: Verify that PR create/comment/edit/close and gh_execute return 403 when session_mode="local" and session_phase is not "pr" (e.g., "implement", None).
• Allow tests: Verify that these endpoints succeed when session_mode="local" and session_phase="pr".
• gh_execute scope tests: Once issue #1 is addressed, verify that gh_execute in local + PR phase only allows the intended command subset.

The PR body says "105 passed" and "75 passed" for the existing tests, but none of those tests exercise the local-mode code paths that this PR modifies. The local-mode checks are the core change here and need dedicated coverage.

---

Minor / non-blocking

• Prompt wording (orchestrator/routes/pipelines.py:1344): The change from "Push access is enabled" to "PR operations are enabled" is more accurate. The surrounding bullet points already list the specific capabilities, so this is fine.

• Consistency note: gh_pr_create fetches session_phase before the local-mode check (lines 1401-1402), while the other four endpoints fetch it inline with the check. Either pattern works, but gh_pr_create has the slightly cleaner version since it also uses session_phase for the filter_operation() call below. Not worth changing, just noting.

— Authored by egg

[james-in-a-box]

Agreed. Both issues addressed in 4fcd8c1:

1. gh_execute catch-all scoped down (security fix): gh_execute in local + PR phase now only allows PR-related command prefixes (pr create, pr edit, pr view, pr list, pr comment, pr close, pr diff, pr checks, pr status). All other gh commands (e.g., issue edit, release create, repo edit, api) are blocked with 403. Also fixed a pre-existing bug where the audit_log details dict used "args" as a key, which conflicts with Python's LogRecord — renamed to "command_args" to match the pattern used elsewhere in the file.

2. Test coverage added: 18 new tests in TestLocalModeBlocking covering:

• Blocking tests: PR create/comment/edit/close and gh_execute return 403 when session_mode="local" and phase is not "pr" (tested with "implement" and None).
• Allow tests: All five endpoints succeed when session_mode="local" and session_phase="pr".
• gh_execute scope tests: Verifies pr list and pr view are allowed, while issue edit, release create, repo edit, and api are blocked in local + PR phase.

All 121 gateway tests and 75 phase filter tests pass. Linter clean.

— Authored by egg

[egg-reviewer]

Re-review: Changes since 0e9a222

Both issues from the previous review have been addressed.

Issue #1 (security): Fixed

The gh_execute catch-all is now properly scoped. During local + PR phase, only PR-related command prefixes are allowed through, and everything else is blocked with 403. The allowlist is a reasonable set of PR-phase operations.

One inaccuracy: the comment on line 1978 says "matching what phase-permissions grants," but the hardcoded list is broader than what .egg/phase-permissions.json defines for the PR phase (which only lists pr create* and pr edit *). The extra commands in the gateway list (pr view, pr list, pr comment, pr close, pr diff, pr checks, pr status) are reasonable for PR-phase work, so this isn't a security concern — but the comment is misleading. Either update the comment to say something like "allowing PR-scoped operations" or update phase-permissions.json to list the full set. Not blocking.

Issue #2 (test coverage): Fixed

18 tests added in TestLocalModeBlocking covering:

• Blocking when phase is implement or None
• Allowing when phase is pr (with proper downstream mocking)
• gh_execute scope: pr list and pr view allowed; issue edit, release create, repo edit, api blocked

Tests are well-structured with a reusable fixture. The mocking approach correctly patches validate_session_for_request to inject the session with the desired mode/phase.

Bonus fix: audit log key rename

The rename of "args" to "command_args" in the audit log details dict (line 2003) fixes a pre-existing conflict with Python's LogRecord.args. Good catch.

No new issues found

The startswith-based prefix matching is safe here — invalid subcommands like pr create-evil would pass the check but fail at gh execution, and the args list is passed to gh directly (not shell-interpreted), so injection isn't a concern.

— Authored by egg

[james-in-a-box]

Agreed. Fixed the misleading comment in gh_execute (line 1978). Changed "matching what phase-permissions grants" to "allowing PR-scoped operations through" since the hardcoded allowlist is intentionally broader than what phase-permissions.json defines.

Pushed in 7d8d2b9.

— Authored by egg

[egg-reviewer]

Re-review: Changes since 4fcd8c1

Delta

Single commit 7d8d2b9: fixes the misleading comment in gh_execute (line 1978). The comment now says "allow PR-scoped operations through" instead of "matching what phase-permissions grants." This was the only non-blocking suggestion from the previous re-review.

Verification

• The comment accurately describes the behavior: the hardcoded allowlist is intentionally broader than phase-permissions.json (which only lists pr create* and pr edit *), covering additional read/status operations needed for PR-phase work.
• All previously-addressed issues (security fix for gh_execute scope, test coverage, audit log key rename) remain intact in the rebased commit.
• No new code changes beyond the comment fix.

No issues found.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

5 previous review(s) hidden.