mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-07 16:19:09 +00:00
Key workflow approval fingerprint on file content hash
Switches the fingerprint from {repo, branch, base_sha, files} to
{repo, branch, files, content_hash}, where content_hash is a SHA-256 of the
workflow-file contents at the pushed head. This keeps the security property:
a content change to the same workflow file requires re-review, while a rebase
that replays the same workflow diff stays auto-approved.
Also updates the prompt and Slack approval copy to match the new behavior, and
extends the test to exercise the remote-branch-exists path (changing base_sha)
and assert content changes produce a different fingerprint.
Refs: 98
This commit is contained in:
parent
2e27a32385
commit
57e8afa19f
3 changed files with 49 additions and 13 deletions
|
|
@ -340,6 +340,14 @@ def _workflow_change_for_push(backend: Any, parsed: ParsedGitPush) -> WorkflowPu
|
|||
if not diff.ok or not diff.output:
|
||||
return None
|
||||
|
||||
content_parts: list[str] = []
|
||||
for path in files:
|
||||
blob = _run_git(backend, root, f"show {shlex.quote(head)}:{shlex.quote(path)}")
|
||||
if not blob.ok:
|
||||
return None
|
||||
content_parts.append(blob.output)
|
||||
content_hash = _fingerprint({"contents": content_parts})
|
||||
|
||||
remote = _run_git(backend, root, "config --get remote.origin.url")
|
||||
repo = _normalize_remote(_first_line(remote.output)) if remote.ok else ""
|
||||
fixed_refspec = f"{head}:refs/heads/{parsed.remote_ref}"
|
||||
|
|
@ -348,14 +356,14 @@ def _workflow_change_for_push(backend: Any, parsed: ParsedGitPush) -> WorkflowPu
|
|||
fixed_args.append("--set-upstream")
|
||||
fixed_args.extend([parsed.remote, fixed_refspec])
|
||||
fixed_command = _git_command(root, " ".join(shlex.quote(arg) for arg in fixed_args))
|
||||
identity_payload = {
|
||||
content_payload = {
|
||||
"repo": repo,
|
||||
"branch": branch_name,
|
||||
"base_sha": base_sha,
|
||||
"files": files,
|
||||
"content_hash": content_hash,
|
||||
}
|
||||
return WorkflowPushChange(
|
||||
fingerprint=_fingerprint(identity_payload),
|
||||
fingerprint=_fingerprint(content_payload),
|
||||
repo=repo,
|
||||
branch=branch_name,
|
||||
base_sha=base_sha,
|
||||
|
|
@ -423,9 +431,9 @@ def _approval_slack_message(change: WorkflowPushChange) -> str:
|
|||
f"Open SWE is trying to push changes to GitHub workflow files in `{repo}` on `{branch}`.\n\n"
|
||||
f"*Files:*\n{files}\n\n"
|
||||
f"*Fingerprint:* `{change.fingerprint}`\n\n"
|
||||
"Approval covers the workflow files listed above on this branch, including future "
|
||||
"rebases or amends of the same change. If the set of workflow files or the branch "
|
||||
"changes, a new fingerprint will be required."
|
||||
"Approval covers the exact workflow-file content diff listed above on this branch, "
|
||||
"including future rebases or amends that replay the same diff. If the set of workflow "
|
||||
"files, the branch, or the workflow-file content changes, a new fingerprint will be required."
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -317,7 +317,7 @@ Steps, in order:
|
|||
|
||||
**IMPORTANT: If `git push` or `gh` returns "403", "Permission denied", or another permanent authorization failure, do not retry. Report the error to the user immediately and stop.**
|
||||
|
||||
**IMPORTANT: Workflow files (`.github/workflows/`) may be changed only when explicitly requested. Any push that includes workflow-file changes requires human approval before it can proceed. Approval is keyed to the repo, branch, and workflow files being pushed, so rebases or amends of the same workflow change do not require a fresh approval; changing the branch or the set of workflow files does require a new approval. Do not attempt to bypass it.**
|
||||
**IMPORTANT: Workflow files (`.github/workflows/`) may be changed only when explicitly requested. Any push that includes workflow-file changes requires human approval before it can proceed. Approval is keyed to the repo, branch, workflow files, and the exact workflow-file content diff, so rebases or amends that replay the same workflow diff do not require a fresh approval; changing the branch, the set of workflow files, or the workflow-file content does require a new approval. Do not attempt to bypass it.**
|
||||
|
||||
4. **Notify the source** immediately after pushing and, when applicable, PR creation/update succeeds. Include a brief summary plus the PR link or branch URL:
|
||||
- Linear-triggered: use `linear_comment` with an `@mention` of the user who triggered the task
|
||||
|
|
|
|||
|
|
@ -20,8 +20,16 @@ class _Response:
|
|||
class _Backend:
|
||||
id = "sandbox-id"
|
||||
|
||||
def __init__(self, *, workflow_files: str = ".github/workflows/ci.yml") -> None:
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
workflow_files: str = ".github/workflows/ci.yml",
|
||||
remote_branch: str | None = None,
|
||||
diff_output: str = "diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml\n",
|
||||
) -> None:
|
||||
self.workflow_files = workflow_files
|
||||
self.remote_branch = remote_branch
|
||||
self.diff_output = diff_output
|
||||
self.commands: list[str] = []
|
||||
self.head = "a" * 40
|
||||
|
||||
|
|
@ -30,7 +38,9 @@ class _Backend:
|
|||
if "rev-parse --show-toplevel" in command:
|
||||
return _Response("/repo\n")
|
||||
if "rev-parse --verify refs/remotes/origin/feature" in command:
|
||||
return _Response("", 1)
|
||||
if self.remote_branch is None:
|
||||
return _Response("", 1)
|
||||
return _Response(f"{self.remote_branch}\n")
|
||||
if "symbolic-ref --short refs/remotes/origin/HEAD" in command:
|
||||
return _Response("origin/main\n")
|
||||
if f"merge-base {self.head} origin/main" in command:
|
||||
|
|
@ -38,11 +48,18 @@ class _Backend:
|
|||
if "diff --name-only" in command:
|
||||
return _Response(f"{self.workflow_files}\n" if self.workflow_files else "")
|
||||
if "diff --binary --full-index" in command:
|
||||
return _Response("diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml\n")
|
||||
return _Response(self.diff_output)
|
||||
if command.startswith("git -C /repo show "):
|
||||
content = "workflow content\n"
|
||||
if "+new content" in self.diff_output:
|
||||
content += "new content\n"
|
||||
return _Response(content)
|
||||
if "config --get remote.origin.url" in command:
|
||||
return _Response("git@github.com:langchain-ai/open-swe.git\n")
|
||||
if "rev-parse --abbrev-ref HEAD" in command:
|
||||
return _Response("feature\n")
|
||||
if "rev-parse refs/remotes/origin/feature" in command:
|
||||
return _Response(f"{self.remote_branch}\n")
|
||||
if "rev-parse HEAD" in command or "rev-parse feature" in command:
|
||||
return _Response(f"{self.head}\n")
|
||||
return _Response("")
|
||||
|
|
@ -333,10 +350,11 @@ async def test_stale_workflow_approval_is_loud_and_blocks(
|
|||
assert posted["channel_id"] == "C123"
|
||||
|
||||
|
||||
async def test_rebased_workflow_push_uses_identity_fingerprint(
|
||||
async def test_rebased_workflow_push_keeps_fingerprint_for_same_diff(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
backend = _Backend()
|
||||
# Remote branch exists, so base_sha is derived from the remote tip and changes on rebase.
|
||||
backend = _Backend(remote_branch="r" * 40)
|
||||
backend.head = "b" * 40
|
||||
guard.SANDBOX_BACKENDS["thread-1"] = backend
|
||||
|
||||
|
|
@ -371,13 +389,23 @@ async def test_rebased_workflow_push_uses_identity_fingerprint(
|
|||
assert payload["fingerprint"]
|
||||
assert payload["files"] == [".github/workflows/ci.yml"]
|
||||
|
||||
# A second push with a different head but same branch/files should produce the same fingerprint.
|
||||
# Rebase onto a new base with the same workflow diff -> fingerprint stays stable.
|
||||
backend.remote_branch = "s" * 40
|
||||
backend.head = "c" * 40
|
||||
result2 = await guard.WorkflowPushGuardMiddleware().awrap_tool_call(_Request(), handler)
|
||||
assert isinstance(result2, ToolMessage)
|
||||
payload2 = json.loads(str(result2.content))
|
||||
assert payload2["fingerprint"] == payload["fingerprint"]
|
||||
|
||||
# A content change to the same workflow file produces a different fingerprint.
|
||||
backend.diff_output = (
|
||||
"diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml\n+new content\n"
|
||||
)
|
||||
result3 = await guard.WorkflowPushGuardMiddleware().awrap_tool_call(_Request(), handler)
|
||||
assert isinstance(result3, ToolMessage)
|
||||
payload3 = json.loads(str(result3.content))
|
||||
assert payload3["fingerprint"] != payload["fingerprint"]
|
||||
|
||||
|
||||
async def test_approved_workflow_push_aborts_when_elevation_fails(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue