Skip to content

fix: bump Agent Canvas after Automation releases - #240

Draft
enyst wants to merge 2 commits into
mainfrom
fix/bump-agent-canvas-after-release
Draft

fix: bump Agent Canvas after Automation releases#240
enyst wants to merge 2 commits into
mainfrom
fix/bump-agent-canvas-after-release

Conversation

@enyst

@enyst enyst commented Jul 11, 2026

Copy link
Copy Markdown
Member

Summary

  • dispatch an Agent Canvas dependency bump only after Automation publishes successfully to PyPI
  • derive the exact SDK version from the published Automation package metadata
  • create or update a draft Agent Canvas PR using OPENHANDS_BOT_GITHUB_PAT_PUBLIC
  • isolate unprivileged Canvas patch preparation from the fresh runner that receives the public PAT

Why

Agent Canvas pins its Automation and Agent Server SDK versions together, but SDK releases currently stop after opening the Automation dependency bump. This completes the release chain after the corresponding Automation package is actually published.

Validation

  • uv run pre-commit run --files .github/workflows/pypi-release.yml .github/workflows/bump-agent-canvas.yml
  • parsed both workflows and ran bash -n on all seven embedded shell blocks
  • executed the published-metadata resolver against openhands-automation==1.1.5
  • dry-ran the actual patch artifact handoff on fresh Agent Canvas clones
  • verified both the automation-only update and a synthetic six-file SDK upgrade
  • verified stale-release/no-op behavior and preservation of the human-owned PR section
  • independent final review found no remaining correctness, security, or idempotency blockers

No release workflow was dispatched during validation.

This PR was created by an AI agent (OpenHands) on behalf of the requester.

@enyst can click here to continue refining the PR

Dispatch a rerunnable downstream bump only after PyPI publication succeeds.

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions github-actions Bot added the type: fix A bug fix label Jul 11, 2026
@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Coverage

@enyst
enyst marked this pull request as ready for review July 11, 2026 00:14
@enyst

enyst commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

@OpenHands Do a /codereview on this PR.

@openhands-ai

openhands-ai Bot commented Jul 11, 2026

Copy link
Copy Markdown

I'm on it! enyst can track my progress at all-hands.dev

@enyst enyst left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Acceptable — the release ordering and stale-run handling are thoughtful, but the claimed privilege boundary is incomplete.

[CRITICAL ISSUES]

  • [.github/workflows/bump-agent-canvas.yml, line 289] Artifact trust boundary: the fresh PAT-bearing runner accepts the downloaded patch after only checking that Git can apply it. It never independently enforces the six allowed paths or verifies the requested Automation/SDK versions. A compromised preparation runner or action can therefore replace the artifact with any valid Agent Canvas patch; this job will commit and push it using OPENHANDS_BOT_GITHUB_PAT_PUBLIC. Re-run the path allowlist and semantic checks on the staged diff in this fresh job before committing (using NUL-safe path handling), and reject non-regular-file/mode changes. Ideally expose the PAT only after those checks in a step that does not execute target-repository code.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🔴 HIGH
    This adds a release-triggered, cross-repository write path backed by a public-repository PAT. The downstream PR is draft and the preparation job is unprivileged, which are good controls, but artifact contents cross into the privileged job without independent validation. That makes compromise of the preparation path capable of writing attacker-selected changes to a trusted same-repository branch in Agent Canvas.

Recommendation: Do not auto-merge. Request review from a human security/CI maintainer after the privileged job independently validates the staged patch.

VERDICT:
Needs rework: The privilege split must validate data at the trust boundary before using the PAT.

KEY INSIGHT:
A fresh runner is not a security boundary if it treats an artifact produced outside that boundary as trusted code changes.

Validation performed: repository pre-commit passed; all seven embedded shell blocks passed bash -n; I reproduced that the current privileged commands accept and stage an arbitrary valid patch touching README.md.

This review was created by an AI agent (OpenHands) on behalf of the requester.


Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

  1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
  2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
  3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.



if [ -s /tmp/agent-canvas.patch ]; then
git apply --check /tmp/agent-canvas.patch
git apply --index /tmp/agent-canvas.patch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifact trust boundary: git apply --check proves only that the artifact is syntactically applicable; it does not prove that it contains the six changes validated in the unprivileged job. This fresh job never repeats the allowed-path check or verifies config/defaults.json against AUTOMATION_VERSION / SDK_VERSION, so a substituted artifact can modify any Agent Canvas file and then be committed and pushed with the PAT. Please validate the staged diff independently here before git commit: enforce the exact path allowlist with NUL-safe handling, reject mode/symlink changes, verify the exact requested versions, and repeat the old-version/JSON checks. Keep the PAT unavailable until those validations pass.

@openhands-ai

openhands-ai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Reviewed PR #240 and posted the review: #240 (review)

Verdict: Needs rework (🔴 high risk).

Blocking finding: the PAT-bearing job downloads a patch and only runs git apply --check; it does not independently enforce the allowed paths or verify the exact Automation/SDK version changes. I reproduced that the privileged commands accept and stage an arbitrary valid patch touching README.md.

Validation completed:

  • Changed-file pre-commit checks passed.
  • All seven embedded shell blocks passed bash -n.
  • All current PR checks are green.
  • Local and remote PR heads matched; worktree remained clean.

GitHub would not permit REQUEST_CHANGES because the authenticated identity owns the PR, so the blocking verdict was submitted as a COMMENTED review with an inline thread: #240 (comment)

@enyst
enyst marked this pull request as draft July 15, 2026 19:01
@all-hands-bot

Copy link
Copy Markdown
Contributor

🤖 OpenHands is reviewing this PR.

Head commit: edf8c89f3d34d26f16e28d29df5f49f93e0c3638
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/51c92389-0864-48ea-8ba3-edb176bc75b5

This comment was posted by an AI agent (OpenHands).

@all-hands-bot all-hands-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

This PR adds a new bump-agent-canvas.yml workflow that opens a dependency-bump PR in OpenHands/agent-canvas after openhands-automation is published to PyPI, plus a dispatch step in pypi-release.yml to trigger it automatically on release.

Assessment

No material findings. The design is sound and the implementation is thorough.

Security

The two-job architecture provides good isolation:

  • The prepare-agent-canvas-update job has only contents: read permissions, no secrets, and clones the public agent-canvas repo to produce a git patch artifact.
  • The publish-agent-canvas-update job is the only job that holds OPENHANDS_BOT_GITHUB_PAT_PUBLIC, and it receives the patch via artifact — it does not execute untrusted code. git apply only applies file changes, so even a compromised prepare job could not achieve code execution in the privileged job.
  • Version strings ($AUTOMATION_VERSION, $SDK_VERSION) are validated against a strict `^[0-9]+.[0-9]+.[0-9]+$ regex before use in PR titles, commit messages, or branch names, preventing injection.

Correctness

  • Idempotency: Stale-version detection (via sort -V comparison against both main and the existing branch) correctly skips no-op or downgrade scenarios. The skip output gates the publish job.
  • Race conditions: The expected_sha handoff between jobs, re-verified via git ls-remote in the publish job, plus --force-with-lease on push, handles concurrent branch updates safely.
  • SDK pin extraction: The sed regex matches openhands-sdk==X.Y.Z and openhands-workspace==X.Y.Z from PyPI requires_dist, consistent with the == pins in pyproject.toml. The consistency check (sdk_version == workspace_version) fails safely if they diverge.
  • PR body preservation: The Python script that merges the human-owned HUMAN: section from an existing PR body into the generated body correctly handles the section boundary case and falls back to preserving the entire existing body if the markers are missing or malformed.
  • Heredoc indentation: The PR body heredoc content is at the same YAML indentation as the cat command, so after block-scalar dedentation it lands at column 0 — the body will not have spurious leading whitespace.

Risk

Low. The changes are purely additive (a new workflow file and a non-breaking extension to the release workflow). The dispatch step in pypi-release.yml runs only after the release job succeeds and dispatches on main, which is correct since the workflow file will be on main by the time the next release ships. The actions/upload-artifact@v7 and actions/download-artifact@v8 versions are not referenced elsewhere in the repo but are consistent with the repo practice of tracking recent action versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants