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

クローズ
#497 コメント 3 件 リアクション 1 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
35/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
静か
技術スタック
python
領域
security

調査の方向性

plugins/trust.py、loader._discover_project、registry._resolve、registry.hooks_for、HookBus.emit から始めて、プロジェクト由来の宣言的プラグインが再現された stock-project パスを通る経路を追跡します。スコープを決める前に、提案されている3つの trust アプローチを maintainer と確認します。合意したポリシーによって、allowlist にないフォルダーに置かれた hook が unattended パスで実行されないことが保証され、その再現がテストでカバーされれば完了です。

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
Python
スター
174
フォーク
72
平均マージ
17時間 56分
マージ済み PR(30日)
95

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

bmad-code-org/bmad-loop のほかの issue

bmad-code-org/bmad-loop の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。