diff --git a/orchestrator/routes/pipelines.py b/orchestrator/routes/pipelines.py index 6d8043f43b..c87dfe0443 100644 --- a/orchestrator/routes/pipelines.py +++ b/orchestrator/routes/pipelines.py @@ -10211,7 +10211,28 @@ def _open_context_pr_for_pipeline( error=str(fetch_err), ) - tmp_worktree = Path(tempfile.mkdtemp(prefix=f"egg-context-{pipeline_id}-", dir="/tmp")) + # Root under WORKTREE_BASE_DIR so the path falls inside the gateway's + # repo-path allowlist (gateway/git_client.py ALLOWED_REPO_PATHS) — a + # ``/tmp`` location would be rejected by ``validate_repo_path`` and + # the subsequent ``push_worktree_branch`` call would fail with + # ``repo_path must be within allowed directories`` (#2684). Falls + # back to the system temp dir in environments where the base path + # is absent (e.g. unit tests) — emit a warning on that branch so a + # broken docker volume mount in production is noisy rather than + # silently recreating the #2684 push-rejection. + if WORKTREE_BASE_DIR.exists(): + tmp_dir_base = str(WORKTREE_BASE_DIR) + else: + logger.warning( + "Context PR hook: WORKTREE_BASE_DIR missing — falling back to " + "system temp (likely a broken volume mount in production; the " + "push to the context branch will be rejected by the gateway " + "allowlist) (#2684)", + pipeline_id=pipeline_id, + worktree_base_dir=str(WORKTREE_BASE_DIR), + ) + tmp_dir_base = None + tmp_worktree = Path(tempfile.mkdtemp(prefix=f"egg-context-{pipeline_id}-", dir=tmp_dir_base)) # Use a unique sub-path so ``git worktree add`` doesn't collide with # the (already-created-by-mkdtemp) directory. ``git worktree add`` # refuses to add to an existing non-empty directory. @@ -10968,8 +10989,34 @@ def _commit_slice_brc_history_to_integration_branch( error=str(fetch_err), ) + # Root under WORKTREE_BASE_DIR so the temp path falls inside the + # gateway's repo-path allowlist (gateway/git_client.py + # ALLOWED_REPO_PATHS). A ``/tmp`` location is rejected by + # ``validate_repo_path``, which silently failed the BRC-history + # push and left slice PRs without their consensus transcript + # (#2684). Falls back to system temp when the base dir is absent + # (e.g. unit tests) — emit a warning on that branch so a broken + # docker volume mount in production is noisy rather than silently + # recreating the #2684 push-rejection. + if WORKTREE_BASE_DIR.exists(): + tmp_dir_base = str(WORKTREE_BASE_DIR) + else: + logger.warning( + "Per-slice BRC commit: WORKTREE_BASE_DIR missing — falling " + "back to system temp (likely a broken volume mount in " + "production; the push to the integration branch will be " + "rejected by the gateway allowlist) (#2684)", + pipeline_id=pipeline_id, + slice_id=slice_id, + integration_branch=integration_branch, + worktree_base_dir=str(WORKTREE_BASE_DIR), + ) + tmp_dir_base = None tmp_worktree = Path( - tempfile.mkdtemp(prefix=f"egg-slice-brc-{pipeline_id}-{slice_id}-", dir="/tmp") + tempfile.mkdtemp( + prefix=f"egg-slice-brc-{pipeline_id}-{slice_id}-", + dir=tmp_dir_base, + ) ) wt_path = tmp_worktree / "wt" diff --git a/orchestrator/tests/test_context_pr.py b/orchestrator/tests/test_context_pr.py index e1902f8466..661776b94a 100644 --- a/orchestrator/tests/test_context_pr.py +++ b/orchestrator/tests/test_context_pr.py @@ -1334,3 +1334,54 @@ def test_call_site_swallows_any_exception(self): "_open_context_pr_for_pipeline in try/except Exception so a " "hook failure can never escape into the transition path (D3)" ) + + +# ---------------------------------------------------------------------- +# Gateway allowlist compatibility (#2684) +# ---------------------------------------------------------------------- + + +class TestContextPRGatewayAllowlistCompatibility: + """Regression coverage for #2684 (context-PR sibling). + + Same shape as the slice-BRC hook: the context-PR hook builds a + temp worktree and passes ``repo_path=str(wt_path)`` to + ``gateway.push_worktree_branch``. The gateway's + ``validate_repo_path`` rejects ``/tmp`` paths, so the push fails + silently and the context PR opens without the curated refine/plan + artifacts. The hook must root the temp worktree inside + ``WORKTREE_BASE_DIR``. + """ + + def test_temp_worktree_is_rooted_under_worktree_base_dir( + self, tmp_path, pipeline, make_spawner, monkeypatch + ): + import routes.pipelines as pipelines_mod + + fake_base = tmp_path / "egg-worktrees-root" + fake_base.mkdir() + monkeypatch.setattr(pipelines_mod, "WORKTREE_BASE_DIR", fake_base) + + _seed_repo(tmp_path, identifier=2548) + spawner = make_spawner() + load, save, _ = _stub_load_save_contract(contract_pr_factory=lambda: _make_contract()) + with ( + patch("egg_contracts.loader.load_contract", load), + patch("egg_contracts.loader.save_contract", save), + ): + _open_context_pr_for_pipeline(pipeline, spawner, tmp_path) + + ctx_pushes = [ + c + for c in spawner.gateway.push_worktree_branch.call_args_list + if c.kwargs.get("branch") == "egg/issue-2548/context" + ] + assert ctx_pushes, ( + "context-branch push must happen at least once so the regression " + "test has a repo_path to inspect" + ) + repo_path = ctx_pushes[0].kwargs["repo_path"] + assert repo_path.startswith(str(fake_base) + "/"), ( + f"context-PR temp worktree must live under WORKTREE_BASE_DIR; " + f"got {repo_path!r}, expected prefix {str(fake_base)!r}" + ) diff --git a/orchestrator/tests/test_per_slice_brc_commit.py b/orchestrator/tests/test_per_slice_brc_commit.py index f37924dd61..3f6f828662 100644 --- a/orchestrator/tests/test_per_slice_brc_commit.py +++ b/orchestrator/tests/test_per_slice_brc_commit.py @@ -721,3 +721,68 @@ def _spy_copy(src, dst, *args, **kwargs): # The seeded files should have been picked up. assert str(paths["slice-1-md"]) in copy_srcs assert str(paths["slice-1-json"]) in copy_srcs + + +# ---------------------------------------------------------------------- +# Gateway allowlist compatibility (#2684) +# ---------------------------------------------------------------------- + + +class TestGatewayAllowlistCompatibility: + """Regression coverage for #2684. + + The hook constructs a temp worktree and passes ``repo_path=str(wt_path)`` + to ``gateway.push_worktree_branch``. The gateway's + ``validate_repo_path`` only accepts paths under + ``ALLOWED_REPO_PATHS`` (``/home/egg/.egg-worktrees/`` et al.); a + ``/tmp`` location is rejected and the push fails silently — slice + PRs then open without their consensus-history file. The hook must + therefore root its temp worktree inside ``WORKTREE_BASE_DIR``. + """ + + def test_temp_worktree_is_rooted_under_worktree_base_dir( + self, tmp_path, pipeline, make_spawner, monkeypatch + ): + import routes.pipelines as pipelines_mod + + fake_base = tmp_path / "egg-worktrees-root" + fake_base.mkdir() + monkeypatch.setattr(pipelines_mod, "WORKTREE_BASE_DIR", fake_base) + + _seed_per_slice_brc_files(tmp_path, identifier=2548, slice_ids=["slice-1"]) + spawner = make_spawner() + with patch("routes.pipelines._write_brc_history", _no_op_write_brc_history): + _commit_slice_brc_history_to_integration_branch( + pipeline, + spawner, + tmp_path, + slice_id="slice-1", + integration_branch="egg/issue-2548/slice-1", + ) + + spawner.gateway.push_worktree_branch.assert_called_once() + repo_path = spawner.gateway.push_worktree_branch.call_args.kwargs["repo_path"] + assert repo_path.startswith(str(fake_base) + "/"), ( + f"slice BRC temp worktree must live under WORKTREE_BASE_DIR; " + f"got {repo_path!r}, expected prefix {str(fake_base)!r}" + ) + + def test_production_worktree_base_dir_lies_within_gateway_allowlist(self): + """Pin the contract between orchestrator and gateway. + + ``WORKTREE_BASE_DIR`` is where the slice-BRC temp worktree + lives; the gateway's ``ALLOWED_REPO_PATHS`` must contain a + prefix that covers it. This test reads ``WORKTREE_BASE_DIR`` + from the orchestrator module rather than hardcoding the + production path, so a drift on *either* side + (orchestrator-side path move OR gateway-side allowlist tweak) + trips the regression instead of silently passing against a + stale hardcoded prefix. + """ + from routes.pipelines import WORKTREE_BASE_DIR + + from gateway.git_client import validate_repo_path + + candidate = str(WORKTREE_BASE_DIR / "egg-slice-brc-pipeline-x-slice-y-abc" / "wt") + ok, error = validate_repo_path(candidate) + assert ok, f"WORKTREE_BASE_DIR drifted out of gateway ALLOWED_REPO_PATHS: {error}"