On Fri, 7 Aug 2026 11:11:04 +0800 Chengwen Feng <[email protected]> wrote:
> From: Chengwen Feng <[email protected]> > > Currently review-patch.py only supports cloud AI providers > (Anthropic, OpenAI, xAI, Google) via REST API, requiring API keys. > > Add a --via option that invokes the locally installed opencode CLI as > the review runner instead of making HTTP calls. opencode reads > AGENTS.md from the DPDK project directory automatically, needing no > configuration beyond opencode on PATH. > > The --via and -p/--provider options are independent -- via routes to > the local agent mode while -p continues to use the cloud API path. > > Signed-off-by: Chengwen Feng <[email protected]> > Acked-by: Morten Brørup <[email protected]> More detailed AI review found some issues where putting this in may expose security risks. > + Note: opencode runs with its default agent toolset, which includes > + write/edit/bash against the working tree (--dir points at the DPDK > + root). A review should ideally be read-only; restricting the toolset > + requires opencode to gain a --read-only or --agent flag, which it > + does not currently expose. This premise is not correct, and the resulting exposure is the one thing I don't want to merge as-is. Problem ------- The input to this tool is patches from a public mailing list, i.e. untrusted text from strangers. In --via mode that text is fed to an agent that holds write, edit and bash on --dir, which is the user's DPDK tree. A patch that talks the agent into running a command gets code execution on the maintainer's machine. webfetch is the other half: even with writes blocked, an agent that can fetch URLs can exfiltrate whatever it read. A note in a function docstring is not a mitigation, and patches.rst says nothing about it at all. Proposed solution ----------------- opencode does expose the needed controls, so this can be fixed rather than documented around: - "opencode run --agent <name>" selects a named agent. - Agent permissions accept allow/ask/deny per tool, keys are matched as wildcard patterns, so "*" works. - Agent rules take precedence over the user's global opencode.json, so a checked-in agent is authoritative. Suggest v6 add .opencode/agent/dpdk-review.md at the tree root: --- description: Read-only DPDK patch review mode: primary tools: write: false edit: false patch: false bash: false webfetch: false permission: "*": deny read: allow grep: allow glob: allow list: allow --- Review the attached patch. Do not modify any file. and pass --agent dpdk-review in the command built by _call_opencode. Two details matter: - Deny by default and allow the read set, not the reverse. Tools get added over time and a blocklist rots. - Use deny, not ask. In headless run an ask is not a boundary, it either blocks or gets approved depending on flags. Please also add a post-run check that the tree is unchanged (compare git status --porcelain before and after) and error out if it is not. That costs nothing and catches both a misconfigured agent and any future regression in opencode's permission handling. This needs a minimum opencode version, documented and preferably checked at startup. On a build where --agent is unrecognised or the frontmatter is ignored you silently get the default toolset back, and the failure is invisible. patches.rst then needs one sentence: the review runs under the read-only dpdk-review agent, patches are untrusted input, do not override the agent permissions or pass --auto. This is a large enough change that it needs to be made and retested by you rather than fixed up on apply. The other v5 comments still stand.
