Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions gateway/gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -1511,15 +1511,22 @@ def git_push() -> tuple[Response, int] | Response:
# The downstream anchor/phase/agent-restriction checks already gate on
# `not is_infrastructure_push`; this gate makes the role check symmetric.
session_role = None
# Pipeline base branch (#3024): used as the preferred diff base for the
# new-branch fallback so a branch forked from a non-trunk base is not
# blamed for files it inherited unchanged from that base.
session_base_branch = None
changed_files = None # populated below; reused by attribution + phase checks
if hasattr(g, "session") and g.session:
session_role = getattr(g.session, "agent_role", None)
session_base_branch = getattr(g.session, "base_branch", None)

if session_role and not is_infrastructure_push:
# Get the list of files being pushed for downstream attribution-aware
# role enforcement (the canonical agent-role check below) and the
# phase-restriction check further down.
changed_files, check_error = get_changed_files_in_push(exec_path, remote, branch)
changed_files, check_error = get_changed_files_in_push(
exec_path, remote, branch, base_branch=session_base_branch
)

# SECURITY: Fail closed - if we can't determine changed files, block the push.
# This prevents bypass via git diff manipulation (timeout, corrupt refs, etc.)
Expand Down Expand Up @@ -1613,7 +1620,11 @@ def git_push() -> tuple[Response, int] | Response:
# Resolve attribution for every commit in the push range.
try:
attributed_push = _get_attributed_fn(
exec_path, remote, branch, session_role=session_role
exec_path,
remote,
branch,
session_role=session_role,
base_branch=session_base_branch,
)
except Exception as exc:
logger.warning("attribution_lookup_exception", error=str(exc), exc_info=True)
Expand Down Expand Up @@ -1830,7 +1841,9 @@ def git_push() -> tuple[Response, int] | Response:
if session_phase and not is_infrastructure_push:
# Get the list of files being pushed (reuse if already fetched for role check)
if changed_files is None:
changed_files, check_error = get_changed_files_in_push(exec_path, remote, branch)
changed_files, check_error = get_changed_files_in_push(
exec_path, remote, branch, base_branch=session_base_branch
)
if check_error:
audit_log(
"push_denied_file_check_failed",
Expand Down Expand Up @@ -8214,6 +8227,11 @@ def session_create() -> tuple[Response, int] | Response:
agent_anchor_id = data.get("agent_anchor_id") # Optional agent anchor ID
claude_code_version = data.get("claude_code_version") # Optional Claude Code version
branch = data.get("branch") # Optional git branch for non-pushing sessions
# Optional pipeline base branch (PR base). Stored on the session and used
# as the preferred diff base for the new-branch restricted-path push check
# (#3024). Distinct from the locally-derived ``worktree_base_branch`` below,
# which selects the ref a fresh worktree forks from.
session_base_branch = data.get("base_branch")
jira_ticket = data.get("jira_ticket") # Optional Atlassian ticket key — advisory only
synthetic = data.get("synthetic", False) # Orchestrator-internal temp session
# Per-session upstream routing (issue #2769). Default to "anthropic" so
Expand Down Expand Up @@ -8254,6 +8272,15 @@ def session_create() -> tuple[Response, int] | Response:
if len(pipeline_id) > 256:
return make_error("Invalid pipeline_id: must be 256 characters or fewer")

# Validate base_branch if provided (#3024)
if session_base_branch is not None:
if not isinstance(session_base_branch, str):
return make_error("Invalid base_branch: must be a string")
if not session_base_branch:
return make_error("Invalid base_branch: must be a non-empty string")
if len(session_base_branch) > 256:
return make_error("Invalid base_branch: must be 256 characters or fewer")

# Validate issue_number if provided
if issue_number is not None and (not isinstance(issue_number, int) or issue_number < 1):
return make_error("Invalid issue_number: must be a positive integer")
Expand Down Expand Up @@ -8516,6 +8543,7 @@ def session_create() -> tuple[Response, int] | Response:
agent_anchor_id=agent_anchor_id,
claude_code_version=claude_code_version,
branch=branch,
base_branch=session_base_branch,
jira_ticket=jira_ticket if isinstance(jira_ticket, str) and jira_ticket else None,
synthetic=synthetic,
upstream=upstream,
Expand Down
92 changes: 81 additions & 11 deletions gateway/git_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1375,8 +1375,31 @@ def cleanup_credential_helper(path: str | None) -> None:
# =============================================================================


def _fallback_base_candidates(base_branch: str | None) -> list[str]:
"""Ordered, de-duplicated diff-base candidates for new-branch pushes.

Used only by the merge-base fallback that fires when ``origin/<branch>``
does not yet exist (a pipeline's first push). The pipeline's configured
``base_branch`` is tried first: a branch forked from a non-trunk base
carries that base's content, and diffing against trunk (``main``/
``master``) instead would attribute every file inherited unchanged from
the base to the pushing role — falsely blocking non-documenter roles on
bases that legitimately carry documenter-owned files (#3024). ``main``
and ``master`` remain as trailing candidates so sessions with no
``base_branch`` (legacy / non-pipeline) keep today's behavior, and so the
merge-base still resolves if the configured base ref can't be fetched.
"""
candidates: list[str] = []
if base_branch and base_branch != "HEAD":
candidates.append(base_branch)
for trunk in ("main", "master"):
if trunk not in candidates:
candidates.append(trunk)
return candidates


def get_changed_files_in_push(
repo_path: str, remote: str, branch: str
repo_path: str, remote: str, branch: str, base_branch: str | None = None
) -> tuple[list[str], str | None]:
"""
Get the list of files that would be changed by a push.
Expand All @@ -1396,6 +1419,11 @@ def get_changed_files_in_push(
repo_path: Path to the git repository
remote: Remote name (e.g., "origin")
branch: Branch name being pushed
base_branch: The pipeline's configured base branch (PR base). Used as
the preferred diff base for the new-branch merge-base fallback so a
branch forked from a non-trunk base is not blamed for files it
inherited unchanged from that base (#3024). ``None`` (legacy /
non-pipeline sessions) falls back to ``main``/``master`` as before.

Returns:
Tuple of (changed_files, error_message)
Expand Down Expand Up @@ -1494,7 +1522,28 @@ def get_changed_files_in_push(
# If remote branch doesn't exist yet, use per-commit file detection via
# merge-base + diff-tree. This avoids false positives from inherited
# differences between worktree branches and origin/main (Bug #1239).
for default_branch in ("main", "master"):
#
# Diff against the pipeline's configured base_branch first (#3024): a
# branch forked from a non-trunk base carries that base's content, and
# diffing against trunk would mis-attribute those inherited files to the
# pushing role. main/master remain trailing fallbacks.
fallback_bases = _fallback_base_candidates(base_branch)
if base_branch and base_branch != "HEAD":
# Best-effort fetch so origin/<base_branch> resolves for the
# merge-base below — the top-of-function fetch only refreshed the
# pushed branch. main/master are assumed already present locally.
try:
subprocess.run(
git_cmd("fetch", remote, base_branch),
cwd=repo_path,
capture_output=True,
text=True,
timeout=30,
check=False,
)
except Exception:
pass # merge-base will fall through to the next candidate
for default_branch in fallback_bases:
merge_base_result = subprocess.run(
git_cmd("merge-base", f"{remote}/{default_branch}", "HEAD"),
cwd=repo_path,
Expand Down Expand Up @@ -1632,17 +1681,19 @@ class AttributedPushRange:


def _enumerate_push_commits(
repo_path: str, remote: str, branch: str
repo_path: str, remote: str, branch: str, base_branch: str | None = None
) -> tuple[list[str], str | None]:
"""Return (commits_oldest_first, error).

Mirrors ``get_changed_files_in_push`` rev-list logic: prefers
``<remote>/<branch>..HEAD``, then falls back to merge-base with
main/master for new-branch pushes. On any error, returns
``([], "...")`` — the caller then fails closed. Output lines
that don't parse as a git SHA (7–64 lowercase hex) are
rejected so that a misbehaving git wrapper can't smuggle
arbitrary strings into the commit list.
``<remote>/<branch>..HEAD``, then falls back to merge-base with the
pipeline's ``base_branch`` (then main/master) for new-branch pushes.
Diffing against ``base_branch`` first keeps commits inherited from a
non-trunk base out of the range so they are not attributed to the
pushing role (#3024). On any error, returns ``([], "...")`` — the
caller then fails closed. Output lines that don't parse as a git SHA
(7–64 lowercase hex) are rejected so that a misbehaving git wrapper
can't smuggle arbitrary strings into the commit list.
"""
import subprocess

Expand Down Expand Up @@ -1687,7 +1738,21 @@ def _rev_list(base: str) -> list[str] | None:
if primary is not None:
return primary, None

for default_branch in ("main", "master"):
# New-branch fallback: prefer the pipeline's configured base_branch so
# commits inherited from a non-trunk base stay out of the range (#3024).
if base_branch and base_branch != "HEAD":
try:
subprocess.run(
git_cmd("fetch", remote, base_branch),
cwd=repo_path,
capture_output=True,
text=True,
timeout=30,
check=False,
)
except Exception:
pass # merge-base will fall through to the next candidate
for default_branch in _fallback_base_candidates(base_branch):
mb = subprocess.run(
git_cmd("merge-base", f"{remote}/{default_branch}", "HEAD"),
cwd=repo_path,
Expand Down Expand Up @@ -1822,12 +1887,17 @@ def get_attributed_changed_files_in_push(
branch: str,
session_role: str | None = None,
registry_client: object | None = None,
base_branch: str | None = None,
) -> AttributedPushRange:
"""Return attributed files + commit range for a planned push.

``session_role`` is advisory (currently used only for audit logging
in the caller); the function itself does not filter on it.

``base_branch`` is forwarded to ``_enumerate_push_commits`` as the
preferred new-branch diff base so commits inherited from a non-trunk
base are excluded from the attributed range (#3024).

The caller supplies a ``registry_client`` with a ``lookup_bulk``
method so the gateway's push handler can mock the client in tests
and avoid the network round-trip when it already knows the
Expand All @@ -1837,7 +1907,7 @@ def get_attributed_changed_files_in_push(
On any detection failure, returns an ``AttributedPushRange`` with
``error`` set and ``files`` empty — the caller MUST fail closed.
"""
commits, err = _enumerate_push_commits(repo_path, remote, branch)
commits, err = _enumerate_push_commits(repo_path, remote, branch, base_branch=base_branch)
if err:
logger.error(
"get_attributed_changed_files_in_push enumeration failed — failing closed",
Expand Down
15 changes: 15 additions & 0 deletions gateway/session_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,12 @@ class Session:
last_branch: str | None = None # Last known branch from git push
claude_code_version: str | None = None # Claude Code version from container
assigned_branch: str | None = None # Worktree branch locked to this session
# Pipeline's configured base branch (PR base). The push handler uses it as
# the preferred diff base for the new-branch restricted-path check so a
# branch forked from a non-trunk base is not blamed for files inherited
# unchanged from that base (#3024). Orchestrator-authoritative (set from the
# launcher-authenticated register_session) — never agent-settable.
base_branch: str | None = None
auto_commit_sha: str | None = None # SHA from post-agent auto-commit
jira_ticket: str | None = None # Advisory Jira ticket key (issue #1556)
synthetic: bool = False # Orchestrator-internal temp session — skip auto-commit
Expand Down Expand Up @@ -271,6 +277,8 @@ def to_dict_for_persistence(self) -> dict[str, Any]:
result["claude_code_version"] = self.claude_code_version
if self.assigned_branch is not None:
result["assigned_branch"] = self.assigned_branch
if self.base_branch is not None:
result["base_branch"] = self.base_branch
if self.auto_commit_sha is not None:
result["auto_commit_sha"] = self.auto_commit_sha
if self.jira_ticket is not None:
Expand Down Expand Up @@ -307,6 +315,7 @@ def from_persistence(cls, data: dict[str, Any]) -> Session:
last_branch=data.get("last_branch"),
claude_code_version=data.get("claude_code_version"),
assigned_branch=data.get("assigned_branch"),
base_branch=data.get("base_branch"),
auto_commit_sha=data.get("auto_commit_sha"),
jira_ticket=data.get("jira_ticket"),
synthetic=bool(data.get("synthetic", False)),
Expand Down Expand Up @@ -468,6 +477,7 @@ def register_session(
agent_anchor_id: str | None = None,
claude_code_version: str | None = None,
branch: str | None = None,
base_branch: str | None = None,
jira_ticket: str | None = None,
synthetic: bool = False,
upstream: str = "anthropic",
Expand All @@ -488,6 +498,10 @@ def register_session(
agent_anchor_id: Optional agent anchor ID for scoped anchor file writes
claude_code_version: Optional Claude Code version string
branch: Optional git branch for non-pushing pipeline sessions
base_branch: Optional pipeline base branch (PR base). Stored on the
session and used as the preferred diff base for the new-branch
restricted-path push check so a branch forked from a non-trunk
base is not blamed for files inherited unchanged from it (#3024).
upstream: Upstream registry name driving ``/v1/messages``
traffic for this session (issue #2769). Defaults to
``"anthropic"`` so all pre-#2769 callers keep byte-identical
Expand Down Expand Up @@ -549,6 +563,7 @@ def register_session(
agent_role=agent_role,
agent_anchor_id=agent_anchor_id,
claude_code_version=claude_code_version,
base_branch=base_branch,
jira_ticket=jira_ticket,
synthetic=synthetic,
upstream=upstream,
Expand Down
Loading
Loading