opencode-config/docs/decisions/085-pr-193-reviewer-bash-allowlist.md
Sergey f0701558d3
fix(agents): add safe read-only commands to reviewer bash allow-list (#193)
* fix(agents): add safe read-only commands to reviewer bash allow-list

* docs(adr): add ADR-085 reviewer allow-list expansion rationale

* docs(handoff): add handoff for reviewer allow-list expansion

* docs(handoff): set PR number

---------

Co-authored-by: opencode-agent <agent@opencode.local>
2026-08-01 00:01:57 +03:00

7.1 KiB
Raw Permalink Blame History

ADR-085: Expand reviewer bash allow-list with safe read-only commands

Статус

Accepted (2026-07-31, PR#193)

Контекст

Анализ логов ~/.local/share/opencode/log/opencode.log (202 740 строк) выявил 313 срабатываний catch-all "*": deny в frontmatter reviewer.md (permission.bash). Среди заблокированных — безопасные read-only inspection-команды, которые не нужны для write-bypass, но мешают ревьюеру продуктивно работать:

  • date +%Y-%m-%d — временные метки (лог: line 4877)
  • sed -n '305,320p' .opencode/opencode.json — печать диапазона строк (лог: line 9955)
  • sort — сортировка в stdout (лог: line 3148)
  • git ls-files .opencode/skills/ — листинг файлов в индексе (лог: line 9773) — git ls-files* уже в allow-list (строка 66); срабатывание catch-all вероятно артефакт устаревшей версии opencode или нюанс matching-движка. Паттерн git ls-files* формально верен и не требует правки.

Catch-all "*": deny — by design: read-only контракт ревьюера (edit: deny + whitelist). ADR-005 убрал echo * чтобы закрыть bypass echo "x" > file.txt. Но catch-all блокирует и команды, не создающих write-bypass, снижая продуктивность ревьюера.

Решение

Расширить allow-list в .opencode/agents/reviewer.md секция permission.bash, добавив безопасные read-only команды (findLast — последние правила побеждают; новые allow стоят после catch-all "*": deny):

  • date — allow
  • date * — allow
  • sed -n * — allow (print-mode: -n подавляет авто-вывод, p печатает; без -n sed печатает все строки + может w/i/c команды → write-bypass)
  • sort — allow
  • sort * — allow
  • sort * -o * — deny (defense-in-depth: sort -o out.txt пишет файл через процесс, обход edit: deny — прямой аналог ADR-005 про echo). Поставлен ПОСЛЕ sort *: allow: findLast → deny побеждает для -o варианта, allow побеждает для остальных.

НЕ добавлены команды с write-bypass риском:

  • echo * (ADR-005 — echo "x" > file.txt)
  • cp (mutation)
  • awk (может > писать)
  • xxd (может -r писать)
  • sed без -n (может w/i/c команды писать файлы)

Паттерн git ls-files* оставлен без изменений — формально корректен, срабатывание в логе не воспроизведено.

Risk Assessment

sed -n * с командой w: sed -n '1p; w out.txt' пишет файл внутренними средствами sed. Нельзя полностью закрыть через glob (команда w embedded в sed-скрипт, не отдельный аргумент). Риск принят как низкий:

  • Редкая конструкция (reviewer использует sed -n для печати диапазонов, не для записи).
  • Альтернатива (cat/head/tail для диапазона) неудобна для больших файлов (нужен offset+limit, head/tail требуют byte-count или line-count, а не range).
  • cat * уже в allow-list и тоже может cat > file через shell redirect — но shell redirect обрабатывается shell, не командой; sed -n ... w file — внутренняя команда sed. Аналогичный уровень риска.
  • Defense-in-depth: edit: deny остаётся; write-bypass через sed -n ... w теоретически возможен, но reviewer не имеет мотивации писать файлы (read-only агент).

date --set: date --set="2024-01-01" меняет системное время. Риск принят:

  • В контейнере (основной deployment) обычно нет root — date --set упадёт с permission denied.
  • На хосте нужен sudo (не в allow-list → deny).
  • date *: allow допускает --set, но альтернатива (date --set*: deny после date *: allow) создаёт избыточное правило для нереалистичного сценария. Решено задокументировать риск здесь, а не множить deny-правила.
  • date без аргументов и date +FORMAT (основные use-cases ревьюера) — безопасны.

Альтернативы

  • Не добавлять sed -n — отклонено: head/tail требуют byte/line count, а не range; sed -n '305,320p' — стандартный способ печати диапазона строк. Без него ревьюер вынужден cat весь файл и искать глазами, что не работает для больших файлов.
  • Запретить sed -n ... w через glob — отклонено: glob-matching opencode не поддерживает гранулярные опции внутри аргумента (команда w embedded в sed-скрипт). Аналогично ADR-005 про echo: "bash permission matching не поддерживает гранулярные операторы".
  • Добавить date --set*: deny — отклонено: избыточное правило для нереалистичного сценария (нужен root/sudo, которых нет в allow-list). Документирование риска в ADR предпочтительнее множить deny-правила.
  • Убрать catch-all "*": deny — отклонено: catch-all by design (read-only контракт, defense-in-depth). Расширение allow-list точечно, без ослабления perimeter.
  • Параллельно расширить allow-list для других агентов (general, docs-reviewer) — вне scope (issue #191 явно ограничен reviewer.md).

Примечание о нумерации ADR

Спека issue #191 в секции "Контракты" указывала номер ADR как 084 (со словами "номер = max(existing)+1 = 084, фикс ADR-079"). Фактически 084 уже занят (084-pr-192-add-boosty-chat-id-env.md, PR#192). Согласно ADR-079 (нумерация = max(existing)+1, не count+1), следующий свободный номер = 085. Использован 085. Опечатка в спеке зафиксирована в handoff, продолжено без отклонений.