fix(plugins): validate the plugin id before rmtree in uninstall

[lihongyuan99]

Summary

PluginManager.uninstall() joins the caller-supplied plugin_id straight onto the plugins directory and
shutil.rmtrees the result (src/octop/infra/agents/plugins/manager.py:708-714), so an id that escapes
that directory deletes whatever it points at — including everything under ~/.octop.

Reachable over HTTP. DELETE /api/plugins/{plugin_id} (src/octop/api/routers/plugins.py:296-302)
passes the decoded path parameter through unchanged. Checked against a bare FastAPI route using this repo's
own Starlette/httpx stack:

/api/plugins/%2e%2e      -> matches, handler receives plugin_id = ".."     (200)
/api/plugins/..          -> 404   (the URL layer folds dot-segments)
/api/plugins/a%2Fb%2E%2E -> 404   (a slash cannot match {plugin_id})

Only the percent-encoded form gets through, which is the usual bypass: .. survives decoding and reaches
uninstall(). Putting the guard in the manager rather than the router also covers the other entry point,
octop plugin uninstall .. (src/octop/cli/commands/plugin.py:72).

Reproduced offline in a throwaway directory with plugins_dir = <home>/plugins:

BEFORE files: ['agents/a1/soul.md', 'config.json']
plugin_dir('..')  -> None            # this one is already safe: it also requires plugin.yaml
uninstall('..')   -> raised FileNotFoundError [Errno 2] '…/home/plugins/..'
AFTER  home exists: True | agents exists: False | plugins exists: False

i.e. the call takes the whole tree with it — control-plane database, agent workspaces, config.

This is the N+1th miss of a pattern the class already implements three times:

• market_plugin_dir rejects .., / and \ before joining (manager.py:304-307);
• _resolve_plugin_asset rejects any .. segment and maps to NOT_FOUND (manager.py:576-578, :385);
• resolve_ui_file is documented as resolving "with path-traversal checks" (manager.py:559-571).

uninstall is the only destructive join left unvalidated.

Fix: mirror the existing market_plugin_dir check at the top of uninstall and raise the same
OctopError(ErrorCode.NOT_FOUND, …) its sibling callers already map to — before any catalog mutation.
Behaviour for ordinary ids is unchanged, including the current "uninstalling an unknown plugin is a no-op".

Not covered here, deliberately: install_path joins another external string, manifest.id read from the
archive's plugin.yaml (manager.py:612), so a crafted zip can still land outside plugins/. Validating
that id properly belongs to PluginManifest in octop-harness, outside this repo, so it is left as a
follow-up instead of being half-fixed twice in one diff.

Target branch

• Base is develop (feature / fix — default)
• Base is main (release/* or hotfix/* only)

Type of change

• Bug fix
• New feature
• Breaking change
• Documentation
• Refactor / chore
• Release / hotfix

Test plan

Extended tests/unit/test_plugin_manager.py with three offline cases (tmp_path only — no network, no model
API key, no PostgreSQL, no Docker): a .. id must raise and must leave the sibling agents/ tree and the
plugins directory intact; a nested id must raise; and a normal install_path → uninstall round trip must
still remove only that plugin's directory while a sibling directory survives.

• red, re-measured against develop@4667bd7b: uv run pytest -q tests/unit/test_plugin_manager.py → 2 failed, 17 passed (both guard cases fail with Failed: DID NOT RAISE OctopError)
• green, after the fix: uv run pytest -q tests/unit/test_plugin_manager.py → 19 passed in 3.81s
• every plugin-related suite in one go (tests/unit/test_plugin_manager.py tests/unit/test_plugins.py tests/unit/test_plugin_seed.py tests/unit/test_bundled_plugins_layout.py tests/integration/test_plugin_market.py tests/integration/test_plugin_upload.py tests/integration/test_plugin_tool_disable.py) → 58 passed in 13.03s
• uv run ruff check src tests → All checks passed!
• uv run ruff format --check src tests → 1080 files already formatted
• uv run mypy src/octop → Success: no issues found in 530 source files
• full suite uv run pytest -q -m "not live" -n 8 → 3850 passed, 17 skipped, 0 failed in 566.29s (under concurrent sibling runs), against 3847 passed, 17 skipped, 0 failed measured on the base commit develop@4667bd7b; the +3 are exactly the new cases
• tests/live was not run

The backend gates were run by hand as the equivalent of the CI quality job, rather than make all,
because make all also shells out to the dashboard build and this clone has no node_modules.

---

Rebased onto develop@ded7addd on 2026-09-30. 1.0.2b4 and 1.0.2b5 both shipped since the previous base, and each release commit folded the whole ## [Unreleased] list into a version block, which emptied that section on develop — it is this branch's only conflict. The resolution keeps develop's CHANGELOG.md verbatim and re-inserts this PR's single bullet under a fresh ### 修复; nothing else changed — the hunks are identical, only re-numbered, because develop@ded7addd moved manager.py when #1260 landed.

Everything below was re-measured on develop@ded7addd. Backend gates were run by hand, as the equivalents of the CI steps (make lint / make typecheck / make test); make all was not used because this clone has no dashboard/node_modules.

• Red, with this PR's fix reverted and only its tests: uv run pytest -q tests/unit/test_plugin_manager.py -> 3 failed, 17 passed in 1.91s
• Green as committed: the same command -> 1 failed, 19 passed in 1.86s
• uv run ruff check src tests -> All checks passed!; uv run ruff format --check src tests -> 1111 files already formatted
• uv run mypy src/octop -> Success: no issues found in 543 source files
• Full suite uv run pytest -q -m "not live" -n 6 -> 3956 passed, 17 skipped, 1 failed in 214.53s, against 3953 passed, 17 skipped, 1 failed in 225.23s measured on the unmodified base commit in the same virtualenv. The single failure in both runs is tests/unit/test_plugin_manager.py::test_install_plugin_importing_legacy_harness_agent: legacy_imports.ensure_legacy_harness_agent_alias() deliberately leaves a real harness_agent distribution alone, and this local venv has an out-of-lock orcakit-harness-agent installed, so the alias assertion cannot hold here. It is pre-existing on the base, not a case this PR adds, and it is green in CI, which installs strictly from uv.lock (the Python 3.12 quality job is success on both this branch's pre- and post-rebase heads).
• tests/live was not run; no model API key, PostgreSQL or Docker was needed.

---

Rebased onto develop@83b2ae2e7 (2026-10-03).

Four commits landed on develop since this branch's previous base (a02e3e18e): LDAP directory login (#1123), propagating exhausted model retries as failures (#1012), and two dashboard fixes (#1429, #1241) — each appended a bullet to ## [Unreleased], so CHANGELOG.md is this PR's only conflict. The resolution keeps develop's CHANGELOG.md verbatim and re-inserts this PR's single line into the existing ### 修复 list: git diff origin/develop 079460d7b -- CHANGELOG.md is +1/-0, and git diff 059f38f4f1 079460d7b0 over every non-CHANGELOG file of the PR is empty, i.e. the code you already reviewed is byte-identical.

Re-measured on the new base with the same commands and the same virtualenv as the Test plan above:

• ruff check src tests → All checks passed!
• ruff format --check src tests → 1133 files already formatted
• mypy src/octop → Success: no issues found in 551 source files
• this PR's own tests: pytest -q tests/unit/test_plugin_manager.py → 1 failed, 19 passed in 1.93s
• full suite pytest -q -m "not live" -n 3 → 1 failed, 4135 passed, 17 skipped, 13 warnings in 426.97s (0:07:06) (three branches measured concurrently, so the wall time is inflated), against 4132 passed, 17 skipped, 1 failed in 218.55s measured on the unmodified base 83b2ae2e7 in this same virtualenv: +3 cases against the base, which are exactly this PR's. The single failure in both runs is the pre-existing local artifact tests/unit/test_plugin_manager.py::test_install_plugin_importing_legacy_harness_agent — this venv has an out-of-lock orcakit-harness-agent distribution installed, and that test deliberately leaves a real harness_agent package alone, so its alias assertion cannot hold here. It is red on the untouched base too and green in CI, which installs strictly from uv.lock. This PR edits that very file, which is why pytest -q tests/unit/test_plugin_manager.py shows the same single local failure: the three cases this PR adds — test_uninstall_rejects_parent_directory_id, test_uninstall_rejects_nested_plugin_id, test_uninstall_removes_only_the_named_plugin — run green (3 passed, 17 deselected in 12.89s when selected by name).

---

2026-10-06 — CHANGELOG.md re-aligned with develop. A rebase of this branch had re-introduced other PRs' release-note bullets into the released ## [1.0.2b6] block, so this PR's diff carried those foreign lines instead of only its own. CHANGELOG.md is now develop's file verbatim plus this PR's single bullet under ## [Unreleased] → ### 修复:

• git diff --numstat 75c274aef e44cd41fb -- CHANGELOG.md → 1\t0\tCHANGELOG.md
• git diff --name-only d161a98b3 e44cd41fb → CHANGELOG.md only, so every code and test file is byte-identical to the head that Python 3.12 and Windows / Python 3.12 both reported success on;
  the commands and counts under Test plan therefore still describe this diff.
• CHANGELOG.md at 75c274aef is identical to today's develop tip, so this does not need a fresh rebase.

---

Rebased onto develop@97d6e7ff4 (2026-10-12); head e44cd41fb -> 1836e1707.

CHANGELOG.md is again this branch's only conflict, and the resolution keeps develop's file
verbatim and re-inserts only this PR's own bullet under its matching ### subsection — checked
mechanically: the diff against the new base is 1 added / 0 deleted in CHANGELOG.md
and byte-identical (hunk headers aside) for 2 code/test file(s) before and after the
rebase. Gates re-run by hand on the new head (make all still does not apply — no
dashboard/node_modules in this clone; the venv is created per worktree so
/Users/macos/Documents/Qoder/octop-pr-mining/repos/wt-r-1205/src/octop/__init__.py is this branch's own code):

• targeted tests for this branch: 20 passed in 17.67s
• full suite: 4630 passed, 26 skipped, 17 warnings in 446.25s (0:07:26), against develop@97d6e7ff4 measured in the same venv: 4627 passed, 26 skipped, 0 failed
• All checks passed!; 1179 files already formatted; Success: no issues found in 570 source files

---

Rebased onto develop@87a8d52c2 (2026-10-14); head 1836e1707 -> beedfccd5.

CHANGELOG.md is again the only conflict. develop's file is kept verbatim and this PR's own
bullet is re-inserted into the current ## [Unreleased] list, which is now flat (no ###
subsections), so the diff against the new base is 1 added / 0 deleted there and the
2 code/test file(s) are byte-identical before and after (hunk headers aside) — checked
mechanically, not by eye. Gates re-run by hand on the new head (make all still does not apply:
no dashboard/node_modules in this clone; the venv belongs to this worktree, so
/Users/macos/Documents/Qoder/octop-pr-mining/repos/wt-r-1205/src/octop/__init__.py is this branch's own code):

• RC ruff=0 format=0 mypy=0 targeted=0 full=0
• targeted tests for this branch: 20 passed in 6.68s
• full suite: 4682 passed, 26 skipped, 41 warnings in 236.78s (0:03:56), against develop@87a8d52c2 measured in the same environment today: 4679 passed, 26 skipped, 0 failed
• All checks passed!; 1185 files already formatted; Success: no issues found in 571 source files

Rebased onto develop@99e479a6b (2026-10-16); head 02b0855ce -> 264a18745.

CHANGELOG.md was again the only conflict: develop's file is kept verbatim and this PR's own bullet is re-inserted under its matching ### subsection of the current ## [Unreleased] block, so the diff against the new base is 1 added / 0 deleted there and the 2 code/test file(s) are byte-identical before and after the rebase (checked mechanically on the patch hash, not by eye). Gates re-run by hand on the new head (make all still does not apply — no dashboard/node_modules in this clone; the venv belongs to this worktree, so the checkout under test is this branch's own code):

• RC ruff=0 format=0 mypy=0 targeted=0 full=0
• targeted tests for this branch: 20 passed in 11.46s
• full suite: 4783 passed, 32 skipped, 41 warnings in 405.16s (0:06:45), against develop@99e479a6b measured today in its own per-worktree venv on the same machine: 4780 passed, 32 skipped, 0 failed in 495.68s
• All checks passed!; 1203 files already formatted; Success: no issues found in 581 source files