mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 13:53:15 +00:00
Some checks are pending
CI / Lint (push) Waiting to run
CI / Format check (push) Waiting to run
CI / Unit tests (push) Waiting to run
CI / Playwright E2E (push) Waiting to run
CI / Docker build smoke (push) Waiting to run
CI / Triage ledger up to date (push) Waiting to run
CI / ui bun.lock in sync (push) Waiting to run
* feat: port plan-review & workflow-approval UX (#135)
Port six upstream commits onto dev:
- c03a6be7 (already ported): keep plan guidance high-level
- 546042a4: add workflow approval UI with diff preview, approval URLs,
web review links, and polling for approval status during active runs
- 216cf181: remove workflow token elevation; approved pushes pass
through directly without proxy token rewriting
- 3dbc0282: preserve plan redirects after login by accepting relative
same-origin redirect_to values and rejecting blocked paths
- bb104d93: submit plan comments with cmd+enter
- 90cb6caa: terse Slack replies, shared content via save_plan outside
plan mode (PLAN_STATUS_SHARED), reject shared-content mutations
Refs: #135
* fix: restore login page render and clear CI lint/format
The plan-review port removed the authRedirectUrl import from login.tsx
but left its call site, crashing the login page at runtime (blank page,
no 'Sign in to open-swe'). Pass the relative path straight to loginUrl,
matching the plan route and the backend relative-redirect handling.
Also drop an unused os import in the guard test and reformat
workflow_push_guard.py to satisfy ruff.
* fix: carry workflows:write on the standing proxy token
Complete the half-ported upstream 216cf181 cascade. The port dropped
_run_with_workflow_token from the guard but missed the paired github_app
change, so an approved .github/workflows push ran with the base token
(no workflows:write) and GitHub 403'd it.
Add workflows:write to BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS and delete the
now-orphaned WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS constant; update the
github_app and proxy_auth tests to match. The HITL approval gate in
workflow_push_guard.py is unchanged — this only lets the standing token
push once a human approves.
* fix: restore transient workflow-token elevation (revert standing workflows:write)
The standing GitHub-App proxy token (BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS) is
ALWAYS-ON, so carrying workflows:write on it made the fork's HITL workflow-push
guard the sole control over unapproved workflow pushes. The guard's git-push
parser has gaps (obfuscated-expansion push, `gh api` REST contents PUT,
fully-qualified cross-branch refspecs); with a permanently workflows-scoped
token those gaps become live unapproved-workflow-push exploits (1 critical, 2
high — security review BLOCK on #159).
Restore dev's transient-elevation model:
- Drop workflows:write from BASE_RUNTIME_PROXY_TOKEN_PERMISSIONS; re-add the
WORKFLOW_RUNTIME_PROXY_TOKEN_PERMISSIONS constant (base + workflows:write).
- Re-introduce _run_with_workflow_token in the guard: it mints the
workflows-scoped token via refresh_proxy_token around the approved,
guard-normalized fixed_command, then downscopes to RUNTIME then BASE in a
finally. Route the approval branch through it.
- Restore the dev token/elevation tests.
The standing token no longer carries workflows:write, so the three parser
bypasses hit GitHub 403 again; an approved push still succeeds because the
elevation grants workflows:write only around the normalized command. Keeps all
of #159's diff-preview / approval-URL / Slack-card guard additions.
* fix: reject protocol-relative path from sanitizeAuthRedirect (open redirect)
sanitizeAuthRedirect returned parsed.pathname+search+hash, which `new URL` can
resolve to a protocol-relative `//host` (e.g. input `/..//evil.com` normalizes
same-origin, passing the origin check, but yields a path starting with `//`).
ClientRedirect / login.tsx feed that path to window.location.replace, so it
navigates cross-origin — an open redirect. Reject any resolved path that is not
a single-leading-slash path (`^/[^/]`), falling back to the default. Adds
coverage for `/..//evil.com`, `/.//evil.com`, and `//evil.com`.
* fix: log SECURITY error when workflow-token downscope fails
The elevate->push->downscope finally block was silent on failure. If both
refresh_proxy_token calls fail, the sandbox retains workflows:write for the
rest of the run with no signal. Log a SECURITY error on the partial and full
downscope-failure paths so the retention is observable.
Addresses the GPT-4.1 cross-family review of the token-scope remediation.
---------
Co-authored-by: amoussa1229 <166072409+amoussa1229@users.noreply.github.com>
Co-authored-by: Adam Moussa <adam@seahavenind.com>
133 lines
4.8 KiB
Python
133 lines
4.8 KiB
Python
"""Tool: ``save_plan``. Publish sandbox Markdown for review or sharing.
|
|
|
|
Reads the Markdown file the agent created in the sandbox and publishes it to the
|
|
plan-review page. In plan mode it is an approvable implementation plan; outside
|
|
plan mode it is read-only shared content.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from collections.abc import Mapping
|
|
from typing import Annotated, Any
|
|
|
|
from langgraph.config import get_config
|
|
from langgraph.prebuilt import InjectedState
|
|
|
|
from ..dashboard.plan_store import (
|
|
PLAN_FILE_DIRECTORY,
|
|
PLAN_STATUS_READY,
|
|
PLAN_STATUS_SHARED,
|
|
save_plan_content,
|
|
)
|
|
from ..utils.sandbox_state import get_sandbox_backend
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
_MAX_PLAN_LINES = 20_000
|
|
_MARKDOWN_EXTENSIONS = (".md", ".markdown")
|
|
|
|
|
|
async def save_plan(
|
|
plan_file_path: str,
|
|
state: Annotated[dict[str, Any] | None, InjectedState] = None,
|
|
) -> dict[str, Any]:
|
|
"""Publish a Markdown plan file from the sandbox for review.
|
|
|
|
Use this in plan mode once your plan is ready. Outside plan mode, use it to
|
|
share long Slack responses without switching the thread into plan mode. First
|
|
create a Markdown file under ``/workspace/plans/`` using a dated, descriptive
|
|
filename, then pass that file path here. The file contents are published to
|
|
the plan-review page linked in the conversation. In plan mode, the user can
|
|
comment, approve, or request changes; outside plan mode, the page is read-only
|
|
shared content.
|
|
|
|
Write the content in standard Markdown — headings, bullet/numbered lists, and
|
|
fenced code blocks all render.
|
|
|
|
Args:
|
|
plan_file_path: Path to the Markdown plan file in the sandbox.
|
|
|
|
Returns:
|
|
``{success: True, path}`` on success, or ``{success: False, error}``.
|
|
"""
|
|
if not isinstance(plan_file_path, str):
|
|
return {"success": False, "error": "plan_file_path must be a string"}
|
|
path = plan_file_path.strip()
|
|
if not path:
|
|
return {"success": False, "error": "plan_file_path cannot be empty"}
|
|
if not _is_markdown_path(path):
|
|
return {
|
|
"success": False,
|
|
"error": f"plan_file_path must point to a Markdown file in {PLAN_FILE_DIRECTORY}",
|
|
}
|
|
|
|
try:
|
|
config = get_config()
|
|
except Exception:
|
|
config = {}
|
|
configurable = config.get("configurable", {}) if isinstance(config, dict) else {}
|
|
thread_id = configurable.get("thread_id") if isinstance(configurable, dict) else None
|
|
if not thread_id:
|
|
return {"success": False, "error": "no thread_id in run config"}
|
|
|
|
try:
|
|
content = (await _read_plan_file(str(thread_id), path)).strip()
|
|
if not content:
|
|
return {"success": False, "error": "plan file cannot be empty"}
|
|
await _save(str(thread_id), content, path, plan_mode=_active_plan_mode(state, configurable))
|
|
except Exception as exc: # noqa: BLE001
|
|
logger.exception("save_plan failed for thread %s", thread_id)
|
|
return {"success": False, "error": f"failed to save plan: {exc}"}
|
|
return {"success": True, "path": path}
|
|
|
|
|
|
async def _save(thread_id: str, content: str, path: str, *, plan_mode: bool) -> None:
|
|
await save_plan_content(
|
|
thread_id,
|
|
markdown=content,
|
|
status=PLAN_STATUS_READY if plan_mode else PLAN_STATUS_SHARED,
|
|
plan_file_path=path,
|
|
plan_mode=plan_mode or None,
|
|
)
|
|
|
|
|
|
def _active_plan_mode(state: dict[str, Any] | None, configurable: Any) -> bool:
|
|
if isinstance(state, dict) and state.get("plan_mode") is True:
|
|
return True
|
|
return isinstance(configurable, dict) and configurable.get("plan_mode") is True
|
|
|
|
|
|
async def _read_plan_file(thread_id: str, path: str) -> str:
|
|
backend = await get_sandbox_backend(thread_id)
|
|
result = await backend.aread(path, offset=0, limit=_MAX_PLAN_LINES)
|
|
error = _value(result, "error")
|
|
if error:
|
|
raise ValueError(error)
|
|
file_data = _value(result, "file_data")
|
|
if file_data is None:
|
|
raise ValueError("plan file could not be read")
|
|
encoding = _value(file_data, "encoding")
|
|
if encoding is not None and encoding != "utf-8":
|
|
raise ValueError("plan file must be UTF-8 text")
|
|
content = _value(file_data, "content")
|
|
if not isinstance(content, str):
|
|
raise ValueError("plan file content was not text")
|
|
if content.count("\n") + 1 >= _MAX_PLAN_LINES:
|
|
raise ValueError("plan file is too large")
|
|
return content
|
|
|
|
|
|
def _value(value: Any, key: str) -> Any:
|
|
if isinstance(value, Mapping):
|
|
return value.get(key)
|
|
return getattr(value, key, None)
|
|
|
|
|
|
def _is_markdown_path(path: str) -> bool:
|
|
if "\x00" in path or not path.startswith(f"{PLAN_FILE_DIRECTORY}/"):
|
|
return False
|
|
filename = path.removeprefix(f"{PLAN_FILE_DIRECTORY}/")
|
|
if not filename or "/" in filename:
|
|
return False
|
|
return filename.lower().endswith(_MARKDOWN_EXTENSIONS)
|