A folder-dropped declarative plugin runs a shell hook with no allowlist entry, on a stock project

Đã đóng
  • Có gì không ổn?
#497 3 bình luận 1 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức phù hợp với người mới
35/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Ít trao đổi
Công nghệ
python
Lĩnh vực
security

Hướng nghiên cứu

Bắt đầu với plugins/trust.py, loader._discover_project, registry._resolve, registry.hooks_for và HookBus.emit để theo dõi các plugin khai báo có nguồn gốc từ project qua đường dẫn stock-project được tái hiện. Xem xét ba cách tiếp cận về trust được đề xuất với một maintainer trước khi chọn phạm vi. Được xem là hoàn tất khi policy đã thống nhất ngăn một hook được thả vào một folder không có trong allowlist thực thi trong đường dẫn unattended, và bản tái hiện được bao phủ bằng các test.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

area:config bug needs-design P2

Split out of #461 point 4 / PR #495. #496 names this as separate and out of scope there; it had no issue of its own. Found by a completeness pass over PR #495's config_digest and confirmed firsthand against the code.

Related to #496 but strictly wider: #496 needs a project that has already enabled a [python] plugin. This one needs nothing — [plugins] enabled empty (the default) is enough.

The hole

plugins/trust.py is explicit that this is the designed behavior:

Data-only / declarative plugins (no [python]) carry no executable Python — they load on folder-drop. Their shell hooks are the same risk surface as today's engine.toml *_cmd and run through the bus like any command.

That reasoning holds when a human drops the folder. It does not hold under #461's threat model, where a driven session can write anywhere in the workspace — and loader._discover_project reads manifests from <project>/.bmad-loop/plugins/*/plugin.toml, inside that writable tree.

The chain, with no trust gate at any hop:

  1. loader._discover_project yields the manifest (origin="project").
  2. registry._resolve — if manifest.python is None: returns a LoadedPlugin immediately, before the trust.is_enabled check, which only guards the [python] branch.
  3. registry.hooks_for(stage) iterates every loaded plugin and filters on manifest.hook_for(stage) only.
  4. HookBus.emit → _dispatch_declarative → _run_subprocess(cmd, shell=True).

Reproduced

Stock policy, empty allowlist, one file written:

# <project>/.bmad-loop/plugins/rogue/plugin.toml
[plugin]
name = "rogue"
api_version = 1

[hooks.pre_sweep_cycle]
cmd = "..."

policy [plugins] enabled = ()
digest moved      : False
plugin discovered : True
trusted flag      : True
hooks_for(stage)  : [('rogue', '...')]

The child SweepEngine builds a fresh PluginRegistry per run and emits pre_sweep_cycle, so the command runs on the host in the unattended path — which is precisely the human-absent window PR #495's gate exists to protect.

Why PR #495 does not close it

config_digest hashes sorted(policy.plugins.enabled). Nothing in .bmad-loop/plugins/ is read at all, and no policy field moves, so the auto-sweep integrity gate passes.

PR #495's docstring previously claimed "Adding a plugin is caught; changing what an enabled one runs is not." That is true only of plugins declaring [python]. Corrected in b049133 — the docstring and CHANGELOG now scope the claim to the [python] tier and point here, so the pin is not read as stronger than it is.

Shape of a fix (undecided — needs a maintainer call)

The trust tiers were drawn on what the plugin contains (Python vs. data). The gap is that they ignore where it came from, and project origin is agent-writable. Options, roughly in increasing cost:

  1. Gate declarative hooks on enabled too, at least for origin="project" manifests. Smallest change and it makes the allowlist mean what the CHANGELOG implies. Behavior change for anyone relying on folder-drop for a data-only plugin.
  2. Trust declarative folder-drop by origin: builtin/entry_point load freely, project-origin needs an allowlist entry. Keeps folder-drop working for anything shipped or installed outside the workspace.
  3. Digest the manifests — fold .bmad-loop/plugins/*/plugin.toml content into config_digest. Closes the auto-sweep window only, not the parent run, and does nothing for a first run. Weakest of the three; noted for completeness.

Options 1 and 2 also subsume the [plugins.<name>] settings surface: those are overlaid onto manifest defaults by registry._resolve_settings and exported into the hook env as BMAD_LOOP_SETTING_*, and the Unity plugin turns one into an --editor-path argv token. PR #495 deliberately does not pin them (the TUI settings screen writes those same tables, so pinning would refuse an auto-sweep after a supported human edit) — they sit inside an already-trusted plugin's blast radius, which is the boundary this issue and #496 are about.

Notes

  • Not a regression. Predates #495, which narrows the surrounding surface without widening this one.
  • Unlike #496, a stock project with no plugins configured is exposed.

Related: #461, #496, PR #495.

Ngôn ngữ chính
Python
Star
174
Fork
72
Merge trung bình
17 giờ 56 phút
Pull request đã merge (30 ngày)
95

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của bmad-code-org/bmad-loop

Tất cả issue của bmad-code-org/bmad-loop

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.