Skip to content

feat(agent-server): Canvas Extensions REST API surface - #4395

Open
VascoSch92 wants to merge 3 commits into
OpenHands:mainfrom
VascoSch92:canvas-extensions-5
Open

feat(agent-server): Canvas Extensions REST API surface#4395
VascoSch92 wants to merge 3 commits into
OpenHands:mainfrom
VascoSch92:canvas-extensions-5

Conversation

@VascoSch92

@VascoSch92 VascoSch92 commented Aug 6, 2026

Copy link
Copy Markdown
Member

HUMAN:

Adding REST API to menage extensions.

AGENT:

Why

Issue #4352 asks for the REST API surface for Canvas Extensions — the install / list / get / enable-disable / uninstall / bundle endpoints on top of the already-merged installation-persistence and staged-refresh service layer (#4350, #4351). The issue explicitly calls out plugins_router.py / skills_router.py as the precedent to mirror (thin router delegating to a service layer, kebab-case name-pattern path guards, Installed*Response model shape, HTTPException mapping for 400/404/409/422) rather than inventing new conventions.

Summary

  • Add canvas_extensions_router.py: POST /install, GET /installed, GET /installed/{name}, PATCH /installed/{name}, DELETE /installed/{name}, GET /installed/{name}/bundle — wired into api.py alongside plugins_router / skills_router.
  • Add get_canvas_extension_bundle_path() to canvas_extensions/installed.py: re-validates the manifest and entrypoint containment against the live install path on every bundle request, not just at install time (a symlink could change between requests). The bundle response relies on Starlette's own file-stat-based ETag/Last-Modified plus Cache-Control: no-cache so a client never keeps serving pre-refresh content after a staged-refresh revision lands.
  • Add tests/agent_server/test_canvas_extensions_router.py (21 tests): install/get/patch/delete happy paths, 400/409/422/404 error mapping (including a missing-manifest FileNotFoundError that previously fell through to an unhandled 500 — fixed), disabled-by-default + smuggled-enabled-field-ignored + force-reinstall-preserves-state through the HTTP layer, path-parameter injection rejection, symlink-escape-after-install, entrypoint-deleted, and cache-busting across a force-reinstalled revision.

Issue Number

Closes #4352
Linear: OSS-9021 (https://linear.app/all-hands-ai/issue/OSS-9021/canvas-extensions-rest-api-surface)

How to Test

  1. uv run pytest tests/agent_server/test_canvas_extensions_router.py -v → 21 passed.

  2. Full regression: uv run pytest tests/agent_server/ -q → 1988 passed, 13 deselected, 0 failed.

  3. uv run ruff check, uv run ruff format --check, uv run pyright on all changed files → clean.

  4. End-to-end smoke test against a real FastAPI app + TestClient (not just pytest), exercising the full lifecycle with real filesystem I/O (temp install store, a real manifest + JS entrypoint on disk):

    POST /install -> 200 {'name': 'demo-ext', ..., 'enabled': False, ...}
    GET /installed -> 200 {'canvas_extensions': [{'name': 'demo-ext', 'enabled': False, ...}]}
    PATCH /installed/demo-ext -> 200 {'name': 'demo-ext', 'enabled': True}
    GET /installed/demo-ext/bundle -> 200 content-type=text/javascript; charset=utf-8 cache-control=no-cache body=console.log('hello canvas')
    DELETE /installed/demo-ext -> 200 {'message': "Canvas extension 'demo-ext' uninstalled"}
    GET /installed/demo-ext (after delete) -> 404 {'detail': "Canvas extension 'demo-ext' is not installed"}
    

    Confirms: fresh install lands disabled, PATCH flips it, the bundle is served with correct auto-detected content-type and the no-cache header, and uninstall actually removes the tracked entry (404 afterward).

  5. Verified route registration against the real create_app() (not a bare router mount): all 6 endpoints resolve under /api/canvas-extensions/*.

  6. Independently reproduced (before/after) the missing-manifest bug this PR fixes: a source directory with no canvas-extension.json previously raised an uncaught FileNotFoundError, surfacing as a raw 500 with an internal filesystem path leaked into the response body; now returns a clean 422.

Video/Screenshots

Not applicable — backend-only REST API change, no GUI surface to screenshot. See the end-to-end request/response transcript above instead.

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • Staged-refresh (check/apply) already exists in the service layer (canvas_extensions/installed.py, from the prior #4351 PR) but is intentionally not exposed over HTTP here — the issue's endpoint list doesn't include it, so it's left out of scope rather than guessed at.
  • install_path (an absolute server filesystem path) is exposed in API responses, matching the existing InstalledPluginResponse / InstalledSkillResponse convention rather than introducing a new divergence.

VascoSch92 and others added 3 commits August 6, 2026 14:43
Adds canvas_extensions_router.py mirroring plugins_router.py /
skills_router.py: install / list / get / enable-disable / uninstall,
plus a bundle endpoint that re-validates entrypoint containment at
serve time and busts caches across staged-refresh revisions.
…e schema

GET /canvas-extensions/installed/{name}/bundle returns a FileResponse,
which FastAPI can't derive a real OpenAPI schema for -- same known
weak-type-ratchet exception already granted to every other FileResponse
endpoint (file/download, file/archive, workspace file serving, etc.).
@VascoSch92
VascoSch92 marked this pull request as ready for review August 6, 2026 12:58
@all-hands-bot

Copy link
Copy Markdown
Collaborator

🤖 OpenHands is reviewing this PR.

Head commit: 4a157cd89b0149955a25ae82d8b192a9cba8296e
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/5b6e1c7b-bfc9-42ef-b712-197495087e55

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

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

Copy link
Copy Markdown
Collaborator

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 the Canvas Extensions REST API surface (canvas_extensions_router.py) — install/list/get/enable-disable/uninstall plus a bundle-serving endpoint — mirroring the existing plugins_router.py / skills_router.py conventions. The implementation is clean, well-tested (21 tests covering happy paths, all error mappings, path-injection rejection, symlink-escape-after-install, entrypoint-deletion, cache-busting across revisions), and follows the codebase's established patterns consistently.

Findings

No material correctness or security issues found.

Risk assessment: LOW.

The security-critical bundle endpoint is sound:

  • get_canvas_extension_bundle_path re-validates the manifest and entrypoint containment on every request (not just at install time), and resolve_entrypoint resolves symlinks via Path.resolve() before checking is_relative_to(root) + is_file(). A post-install symlink swap pointing outside the package root is correctly rejected.
  • The resolved (dereferenced) path is passed to FileResponse, which mitigates the TOCTOU window between validation and file read — FileResponse opens the already-resolved real path, not the symlink, so a between-request symlink swap cannot redirect the read.
  • Path-parameter injection is guarded at the router level by the kebab-case CANVAS_EXTENSION_NAME_PATTERN before any service-layer code runs (defense-in-depth on top of validate_extension_name).
  • The install endpoint's broadened except (ValidationError, ValueError, OSError) -> 422 mapping fixes a genuine bug (missing-manifest FileNotFoundError previously fell through to an unhandled 500 leaking a filesystem path), an improvement over the plugins_router precedent which only catches ValueError.

Non-blocking observation

get_canvas_extension_bundle_path calls CanvasExtensionInstallationInterface.load_from_dir(), which itself calls resolve_entrypoint() internally (manifest.py line 59), and then calls resolve_entrypoint() again on the returned manifest (installed.py line 230). The second call is redundant — load_from_dir already validates containment. Harmless (arguably defensive), but removing it would avoid a duplicate filesystem resolve()/stat() round-trip per bundle request. Not blocking.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Canvas Extensions: REST API surface

2 participants