* 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>
7.1 KiB
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— allowdate *— allowsed -n *— allow (print-mode:-nподавляет авто-вывод,pпечатает; без-nsed печатает все строки + можетw/i/cкоманды → write-bypass)sort— allowsort *— allowsort * -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 не поддерживает гранулярные опции внутри аргумента (командаwembedded в 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, продолжено без отклонений.