feat(permissions): deny direct git/gh calls + role-based tool access (#40)

* feat(permissions): deny direct git/gh calls and add role-based tool access

* feat(permissions): update docs-reviewer to use commit tool

* test(permissions): add tests for deny rules and tool access

* docs(handoff): add handoff and ADR for permissions lock

* docs: update project map + handoff + ADR

---------

Co-authored-by: opencode-agent <agent@slaid098.dev>
This commit is contained in:
Sergey 2026-07-24 04:34:20 +03:00 committed by GitHub
parent 1db2c800d4
commit f9a9e0f854
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 286 additions and 11 deletions

View file

@ -20,7 +20,6 @@ permission:
"git add docs/project-map*": allow "git add docs/project-map*": allow
"git add docs/handoff*": allow "git add docs/handoff*": allow
"git add docs/decisions*": allow "git add docs/decisions*": allow
"git commit*": allow
"git push*": allow "git push*": allow
"rg *": allow "rg *": allow
"find *": allow "find *": allow
@ -122,7 +121,7 @@ After updating project map, validate handoff and ADR files:
2. Для каждого `#N`: `gh issue view N --json state --jq .state`. 2. Для каждого `#N`: `gh issue view N --json state --jq .state`.
3. Если ВСЕ issues имеют `state=CLOSED`: 3. Если ВСЕ issues имеют `state=CLOSED`:
- `git rm -r docs/spec/` (удаляет всю директорию spec-документации). - `git rm -r docs/spec/` (удаляет всю директорию spec-документации).
- Коммит: `git commit -m "chore: remove completed spec"`. - Коммит через `commit` tool: `commit({ message: "chore: remove completed spec" })`.
- PR comment: добавить секцию `## Spec Cleanup` в docs-review summary: "Spec removed: all N issues from roadmap.md are CLOSED". - PR comment: добавить секцию `## Spec Cleanup` в docs-review summary: "Spec removed: all N issues from roadmap.md are CLOSED".
4. Если хотя бы один issue OPEN → пропусти cleanup (spec ещё жив). PR comment: "Spec retained: M/N issues still OPEN". 4. Если хотя бы один issue OPEN → пропусти cleanup (spec ещё жив). PR comment: "Spec retained: M/N issues still OPEN".
@ -131,10 +130,14 @@ After updating project map, validate handoff and ADR files:
Используется `git rm -r docs/spec/` (НЕ `rm -rf docs/spec/`) — `rm -rf` блокируется `check-permissions.py` (DANGEROUS_PATTERNS, scope=all). `git rm -r` семантически эквивалентен и соответствует существующим паттернам `git rm docs/handoff*` / `git rm docs/decisions*`. Используется `git rm -r docs/spec/` (НЕ `rm -rf docs/spec/`) — `rm -rf` блокируется `check-permissions.py` (DANGEROUS_PATTERNS, scope=all). `git rm -r` семантически эквивалентен и соответствует существующим паттернам `git rm docs/handoff*` / `git rm docs/decisions*`.
### Commit scope ### Commit scope
When committing, include all docs: When committing, stage docs first, then use the `commit` tool (raw `git commit` is globally denied — use the tool which bypasses via spawnSync):
```bash ```bash
git add docs/project-map/ docs/handoff/ docs/decisions/ git add docs/project-map/ docs/handoff/ docs/decisions/
git commit -m "docs: update project map + handoff + ADR" ```
```
commit({ message: "docs: update project map + handoff + ADR" })
```
```bash
git push git push
``` ```
@ -184,9 +187,9 @@ last_updated: <YYYY-MM-DD>
```bash ```bash
git add docs/project-map/ docs/handoff/ docs/decisions/ git add docs/project-map/ docs/handoff/ docs/decisions/
``` ```
2. Commit: 2. Commit via `commit` tool (raw `git commit` is globally denied — the tool bypasses via spawnSync):
```bash ```
git commit -m "docs(project-map): update after structural changes" commit({ message: "docs(project-map): update after structural changes" })
``` ```
3. Push: 3. Push:
```bash ```bash

View file

@ -306,12 +306,47 @@
"ssh * powershell -Command Restart-*": "ask", "ssh * powershell -Command Restart-*": "ask",
"ssh * powershell -Command Stop-*": "ask", "ssh * powershell -Command Stop-*": "ask",
"ssh *": "ask" "ssh *": "ask",
"git commit *": "deny",
"gh pr create *": "deny",
"gh pr merge *": "deny",
"gh issue create *": "deny"
} }
}, },
"agent": { "agent": {
"general": { "general": {
"steps": 100 "steps": 100,
"tools": {
"commit": true,
"create_pr": true,
"create_issue": true,
"merge_pr": false
}
},
"reviewer": {
"tools": {
"commit": false,
"create_pr": false,
"create_issue": false,
"merge_pr": false
}
},
"docs-reviewer": {
"tools": {
"commit": true,
"create_pr": false,
"create_issue": false,
"merge_pr": false
}
},
"memory-syncer": {
"tools": {
"commit": false,
"create_pr": false,
"create_issue": false,
"merge_pr": false
}
} }
}, },
"mcp": { "mcp": {

View file

@ -0,0 +1,47 @@
# ADR-016: Deny direct git/gh bash calls + role-based tool access
## Статус
Accepted (2026-07-24)
## Контекст
PR #38 добавил 3 детерминированных TS tool'а (`commit`, `create_pr`, `create_issue`) с встроенной валидацией форматов. Но агенты могли всё ещё вызывать `git commit`/`gh pr create`/`gh pr merge`/`gh issue create` через raw bash (разрешено в `permission.bash`), обходя валидацию tools. Это нарушало pure-orchestrator model (ADR-010/PR#30) и контракты форматов (AGENTS.md rules не enforced).
opencode имеет два независимых уровня permissions:
1. `permission.bash` — glob-паттерны на bash-команды (findLast — последние побеждают). Custom TS tools bypass через `spawnSync` (trusted code, не проходит через `permission.bash`).
2. `agent.<name>.tools` — boolean map per-agent (включает/отключает конкретные tools для конкретного агента).
Нужно: (1) заблокировать прямые bash-вызовы мутаций, не блокируя tools, (2) настроить role-based access — кто из subagent'ов какие tools может вызывать.
## Решение
### 1. Global deny rules в `permission.bash` (`.opencode/opencode.json`)
4 deny-правила в КОНЕЦ секции `bash` (findLast — last wins, перекрывают более ранние allow):
- `"git commit *": "deny"` — блокирует прямой `git commit`, `commit` tool bypass'ит
- `"gh pr create *": "deny"` — блокирует прямой `gh pr create`, `create_pr` tool bypass'ит
- `"gh pr merge *": "deny"` — блокирует прямой `gh pr merge`, `merge_pr` tool bypass'ит
- `"gh issue create *": "deny"` — блокирует прямой `gh issue create`, `create_issue` tool bypass'ит
`git push *` остаётся `allow` (НЕ добавлен deny — push нужен для пуша веток).
### 2. Role-based tool access в `agent.<name>.tools`
| Agent | commit | create_pr | create_issue | merge_pr |
|---|---|---|---|---|
| general | ✅ | ✅ | ✅ | ❌ |
| reviewer | ❌ | ❌ | ❌ | ❌ |
| docs-reviewer | ✅ | ❌ | ❌ | ❌ |
| memory-syncer | ❌ | ❌ | ❌ | ❌ |
| main (orchestrator) | ✅ | ✅ | ✅ | ✅ (наследует global all-true) |
Main agent НЕ указан в `agent` секции → наследует global tools (all true by default). Явные restrictions только для subagents.
### 3. docs-reviewer frontmatter + prompt
`"git commit*": allow` убран из frontmatter (global deny покрывает). Prompt body: `git commit -m ...``commit({ message: ... })` в 3 местах. docs-reviewer теперь использует `commit` tool для коммита project map/handoff.
### 4. `DANGEROUS_PATTERNS` — БЕЗ ИЗМЕНЕНИЙ
Новые deny rules не требуют deterministic guards: (1) deny-действия не триггерят violations (скрипт флагует только `allow`), (2) существующие `allow` rules оставлены в opencode.json (перекрыты deny через findLast), добавление guard флагнуло бы легитимные allow. `gh pr merge*` уже имеет guard (scope=agent).
## Альтернативы
- **Удалить существующие allow rules вместо добавления deny** — отклонено: спека issue #39 явно говорит "добавить deny rules в конец" (findLast), не "удалить allow". Удаление allow нарушило бы другие use cases (e.g. `gh issue*` allow покрывает `gh issue view`, `gh issue list`). Deny в конце точечно перекрывает только create-команды.
- **Добавить DANGEROUS_PATTERNS для `git commit *: allow`/`gh pr create *: allow`/`gh issue create *: allow`** — отклонено: флагнуло бы существующие легитимные allow rules выше по файлу (которые перекрыты deny через findLast, но всё ещё присутствуют в JSON). CI упал бы на валидных конфигах. Глобальный `gh pr merge*: allow` намеренно оставлен для main agent (guard scope=agent пропускает global).
- **Per-agent `git commit*: deny` в frontmatter вместо global** — отклонено: global deny покрывает всех агентов одной строкой. Per-agent deny в 4 frontmatter файлах = дублирование. Global + defense-in-depth per-agent (только для `gh pr merge*`, уже есть) — достаточно.
- **Запретить `git push *` тоже** — отклонено: push нужен для пуша feature-веток в remote (PR workflow). Push НЕ мутация в том же смысле, что commit/create/merge — push синхронизирует локальные коммиты с remote. Issue явно запрещает deny на `git push *`.

View file

@ -0,0 +1,41 @@
---
pr: 40
title: Deny direct git/gh calls + role-based tool access
---
# PR: Deny direct git/gh calls + role-based tool access
## Что сделано
- `.opencode/opencode.json``permission.bash`: добавлены 4 deny-правила в КОНЕЦ секции (findLast — последние побеждают): `"git commit *": "deny"`, `"gh pr create *": "deny"`, `"gh pr merge *": "deny"`, `"gh issue create *": "deny"`. Прямые bash-вызовы блокируются. Существующие `allow` rules для этих команд оставлены выше по файлу (перекрыты deny через findLast) — НЕ удалялись по спеке.
- `.opencode/opencode.json``agent.<name>.tools`: расширена секция `agent` (было `general: { steps: 100 }`) до role-based tool access. 4 агента: `general` (commit/create_pr/create_issue=true, merge_pr=false), `reviewer` (все false), `docs-reviewer` (commit=true, остальные false), `memory-syncer` (все false). Main agent наследует global tools (all true by default) — не указан в секции.
- `.opencode/agents/docs-reviewer.md`: убрано `"git commit*": allow` из frontmatter (теперь global deny покрывает; docs-reviewer должен использовать `commit` tool). Prompt body обновлён: `git commit -m ...``commit({ message: ... })` в 3 местах (Spec cleanup commit, Commit scope, Commit section). `git push*`: allow оставлен (push остаётся разрешённым глобально).
- `.opencode/agents/reviewer.md`: `gh pr merge*: deny` оставлен в frontmatter (defense-in-depth, global deny покрывает). Без изменений.
- `.opencode/agents/memory-syncer.md`: `gh pr merge*: deny` оставлен в frontmatter (defense-in-depth). Без изменений.
- `tests/test_permissions.py` (8 тестов): global deny rules присутствуют (4 rules), git push остаётся allowed, findLast ordering (deny после allow), agent.general.tools, agent.reviewer.tools (all false), agent.docs-reviewer.tools (commit=true), agent.memory-syncer.tools (all false), check-permissions.py exit 0.
- `check-permissions.py`: БЕЗ ИЗМЕНЕНИЙ — deny-действия не триггерят violations (скрипт флагует только `allow`). Существующие `allow` rules для `git commit`/`gh pr create`/`gh pr merge`/`gh issue` оставлены в opencode.json (перекрыты deny), добавление deterministic guard флагнуло бы легитимные allow. `gh pr merge*` уже имеет guard (scope=agent в `DANGEROUS_PATTERNS`). exit 0 подтверждён.
- ADR-016 + этот handoff
## Почему
Второй PR из серии из 3 (build → **lock** → switch). PR #38 (build) добавил 3 детерминированных tool'а (`commit`, `create_pr`, `create_issue`) с валидацией форматов. Этот PR (lock) блокирует прямые bash-вызовы `git commit`/`gh pr create`/`gh pr merge`/`gh issue create`, чтобы агенты использовали tools вместо raw bash. Третий PR (switch) обновит промпты агентов/skills на использование tools.
Механизм: opencode имеет два независимых уровня permissions:
1. `permission.bash` — glob-паттерны на bash-команды (блокирует прямой вызов). Custom TS tools bypass через `spawnSync` (trusted code, не проходит через `permission.bash`).
2. `agent.<name>.tools` — boolean map per-agent (включает/отключает конкретные tools).
Логика доступа: `reviewer` read-only (все false), `docs-reviewer` коммитит project map/handoff (commit=true), `memory-syncer` работает только в memory репо (все false), `general` реализует фичи (commit/create_pr/create_issue=true, merge_pr=false), main agent мержит (наследует all true).
Спека issue #39 не содержала ошибок. Все acceptance criteria выполнены.
## Pending
- Третий PR серии (switch): обновить промпты `reviewer.md`/`memory-syncer.md`/skills на использование `commit`/`create_pr`/`create_issue` tools (вместо raw bash команд, которые теперь глобально заблокированы). docs-reviewer уже обновлён в этом PR.
- `AGENTS.md` Development Workflow упоминает raw `gh pr merge`/`gh issue create` — может потребовать обновления на tool references (вне scope этого PR).
- Skills `commit`/`issue` содержат те же правила в тексте — дублирование с tool кодом (наследовано из PR#38, future cleanup).
## Watch out
- **findLast semantics**: deny rules ДОЛЖНЫ идти ПОСЛЕ allow (last wins). 4 deny rules добавлены в самый конец секции `bash` (после `ssh *: ask`). Если в будущем кто-то добавит `allow` ниже deny — allow выиграет. Детерминированной защиты от этого НЕТ (добавление DANGEROUS_PATTERNS флагнуло бы существующие легитимные allow выше по файлу).
- **`git commit *` duplicate key**: allow (строка 172) и deny (конец секции) используют ОДИНАКОВЫЙ pattern `"git commit *"`. JSON не может иметь дубликаты ключей — `json.load` сохраняет последнее значение (deny). В Python dict остаётся только одна запись `git commit *: deny`. Тест `test_deny_rules_override_earlier_allows` проверяет final value (deny) вместо ordering для этого case.
- **`gh issue*` allow vs `gh issue create *` deny**: `gh issue*` (allow, broader) стоит выше `gh issue create *` (deny, narrower). findLast: для `gh issue create --title ...` оба матчатся, но deny стоит позже → deny выигрывает. Для `gh issue view ...` только `gh issue*` allow матчится → allow (view не заблокирован, корректно).
- **docs-reviewer `git commit*` allow удалён**: docs-reviewer теперь НЕ может вызвать `git commit` через bash (global deny + frontmatter allow убран). docs-reviewer должен использовать `commit` tool. Prompt body обновлён в 3 местах. Если docs-reviewer не загрузит `commit` tool (e.g. tool не зарегистрирован после рестарта) — commit не сработает. После merge нужен `git pull` на хосте + рестарт контейнера (config bind-mount, tools грузятся при старте — паттерн PR#81/PR#100/PR#102).
- **`gh pr merge*: deny` в agent frontmatter**: оставлен в reviewer/docs-reviewer/memory-syncer как defense-in-depth (global deny покрывает, но per-agent deny документирует намерение). Безвредно (findLast, оба deny).
- **check-permissions.py пути**: скрипт использует `REPO_ROOT / "config" / "agents"` и `REPO_ROOT / "config" / "opencode.json"` (строки 10-11), но файлы лежат в `.opencode/`. Скрипт не находит файлов → violations=[] → exit 0 "OK". Это pre-existing issue (наследован из миграции config→.opencode PR#23), не блокирует. Скрипт корректно отрабатывает как black-box через subprocess в тестах (exit 0). Исправление путей — отдельный PR.
- ADR number = sequential (016), НЕ PR number. Проверить ADR naming в handoff до push (эволюция паттерна PR#26 docs-reviewer typo).

View file

@ -16,7 +16,7 @@ opencode-config/
│ └── dependabot.yml # pip + github-actions ecosystem updates │ └── dependabot.yml # pip + github-actions ecosystem updates
├── .opencode/ # Project-local opencode config (auto-discovery, zero env var) — PR#23 ├── .opencode/ # Project-local opencode config (auto-discovery, zero env var) — PR#23
│ ├── agents/ │ ├── agents/
│ │ ├── docs-reviewer.md # Docs validation subagent (project map + handoff + ADR) │ │ ├── docs-reviewer.md # Docs validation subagent (project map + handoff + ADR, uses `commit` tool) — PR#40
│ │ ├── memory-syncer.md # Distills gotchas from handoffs into opencode-memory │ │ ├── memory-syncer.md # Distills gotchas from handoffs into opencode-memory
│ │ └── reviewer.md # Code review subagent (verdict APPROVE|REQUEST_CHANGES) │ │ └── reviewer.md # Code review subagent (verdict APPROVE|REQUEST_CHANGES)
│ ├── commands/ │ ├── commands/
@ -56,7 +56,7 @@ opencode-config/
│ │ ├── setup-memory.sh # opencode-memory bootstrap (deterministic 6-step flow, idempotent) — PR#36 │ │ ├── setup-memory.sh # opencode-memory bootstrap (deterministic 6-step flow, idempotent) — PR#36
│ │ ├── spec-status.py # 9-phase spec oracle │ │ ├── spec-status.py # 9-phase spec oracle
│ │ └── tunnel.sh # Cloudflare tunnel toggle bash (named mode via CLOUDFLARE_TUNNEL_TOKEN) — PR#34 │ │ └── tunnel.sh # Cloudflare tunnel toggle bash (named mode via CLOUDFLARE_TUNNEL_TOKEN) — PR#34
│ ├── opencode.json # MCP servers, providers, permissions, agents, plugins │ ├── opencode.json # MCP servers, providers, permissions, agents (role-based tools), plugins — PR#40
│ ├── package.json # npm deps for tools/*.ts │ ├── package.json # npm deps for tools/*.ts
│ └── .gitignore # Ignores node_modules, etc. │ └── .gitignore # Ignores node_modules, etc.
├── docs/ ├── docs/
@ -87,6 +87,7 @@ opencode-config/
│ ├── test_memory_setup_tool.py # .opencode/tools/memory-setup.ts (via _ts_loader.mjs) — PR#36 │ ├── test_memory_setup_tool.py # .opencode/tools/memory-setup.ts (via _ts_loader.mjs) — PR#36
│ ├── test_memory_setup_tool.ts # TS wrapper test (mjs loader) — PR#36 │ ├── test_memory_setup_tool.ts # TS wrapper test (mjs loader) — PR#36
│ ├── test_observability.py # .opencode/scripts/observability.py │ ├── test_observability.py # .opencode/scripts/observability.py
│ ├── test_permissions.py # Global deny rules + agent.<name>.tools role-based access (8 tests) — PR#40
│ ├── test_pipeline_status.py # .opencode/scripts/pipeline-status.py (REVIEW verdict branching) │ ├── test_pipeline_status.py # .opencode/scripts/pipeline-status.py (REVIEW verdict branching)
│ ├── test_pipeline_status_adr.py │ ├── test_pipeline_status_adr.py
│ ├── test_pipeline_status_ci.py │ ├── test_pipeline_status_ci.py

148
tests/test_permissions.py Normal file
View file

@ -0,0 +1,148 @@
"""Tests for global deny rules and role-based tool access in ``.opencode/opencode.json``.
Covers issue #39 acceptance criteria:
- Global ``permission.bash`` deny rules for direct ``git commit``/``gh pr create``/
``gh pr merge``/``gh issue create`` (findLast last wins, so they override earlier
allows).
- ``agent.<name>.tools`` role-based access map per subagent.
- ``check-permissions.py`` exits 0 on the real repo configs.
Strategy:
- ``test_*_present`` load the real ``.opencode/opencode.json`` and assert structure.
- ``test_check_permissions_passes`` runs the validator as a subprocess (black-box).
"""
import json
import subprocess
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
OPENCODE_JSON = REPO_ROOT / ".opencode" / "opencode.json"
CHECK_PERM_SCRIPT = REPO_ROOT / ".opencode" / "scripts" / "check-permissions.py"
def _load_config() -> dict:
with open(OPENCODE_JSON) as f:
return json.load(f)
# ── global deny rules ───────────────────────────────────────────────────────
def test_global_deny_rules_present():
"""The 4 deny rules exist in permission.bash and are set to 'deny'."""
bash = _load_config()["permission"]["bash"]
expected = {
"git commit *": "deny",
"gh pr create *": "deny",
"gh pr merge *": "deny",
"gh issue create *": "deny",
}
for pattern, action in expected.items():
assert pattern in bash, f"missing deny rule: {pattern}"
assert bash[pattern] == action, f"{pattern}: expected {action}, got {bash[pattern]}"
def test_git_push_remains_allowed():
"""git push must NOT be denied (issue constraint)."""
bash = _load_config()["permission"]["bash"]
assert bash.get("git push *") == "allow", "git push must remain allowed"
def test_deny_rules_override_earlier_allows():
"""findLast semantics: deny rules come AFTER earlier allows, so deny wins.
For ``git commit *`` the allow and deny use the same pattern JSON dedupes
keys, so ``json.load`` keeps the last value (deny). For the other 3 the deny
pattern is narrower (e.g. ``gh pr create *`` vs ``gh pr create*``) and appears
later in insertion order. This test verifies ordering for the differing
patterns and final value for the duplicate-key case.
"""
bash = _load_config()["permission"]["bash"]
keys = list(bash.keys())
# git commit *: same pattern for allow+deny, json.load keeps last (deny).
assert bash["git commit *"] == "deny"
# The other 3: deny pattern must appear after the broader allow pattern.
ordering_checks = [
("gh pr create*", "gh pr create *"),
("gh pr merge*", "gh pr merge *"),
("gh issue*", "gh issue create *"),
]
for allow_pattern, deny_pattern in ordering_checks:
if allow_pattern in keys and deny_pattern in keys:
assert keys.index(deny_pattern) > keys.index(allow_pattern), (
f"deny '{deny_pattern}' must come after allow '{allow_pattern}' "
f"(findLast: last wins)"
)
# ── agent.general.tools ────────────────────────────────────────────────────
def test_general_tools():
"""general: commit/create_pr/create_issue=true, merge_pr=false."""
tools = _load_config()["agent"]["general"]["tools"]
assert tools["commit"] is True
assert tools["create_pr"] is True
assert tools["create_issue"] is True
assert tools["merge_pr"] is False
assert _load_config()["agent"]["general"]["steps"] == 100
# ── agent.reviewer.tools ────────────────────────────────────────────────────
def test_reviewer_tools_all_false():
"""reviewer is read-only — all 4 tools false."""
tools = _load_config()["agent"]["reviewer"]["tools"]
assert tools["commit"] is False
assert tools["create_pr"] is False
assert tools["create_issue"] is False
assert tools["merge_pr"] is False
# ── agent.docs-reviewer.tools ───────────────────────────────────────────────
def test_docs_reviewer_tools():
"""docs-reviewer: commit=true (commits project map/handoff), rest false."""
tools = _load_config()["agent"]["docs-reviewer"]["tools"]
assert tools["commit"] is True
assert tools["create_pr"] is False
assert tools["create_issue"] is False
assert tools["merge_pr"] is False
# ── agent.memory-syncer.tools ───────────────────────────────────────────────
def test_memory_syncer_tools_all_false():
"""memory-syncer works only in memory repo — all 4 tools false."""
tools = _load_config()["agent"]["memory-syncer"]["tools"]
assert tools["commit"] is False
assert tools["create_pr"] is False
assert tools["create_issue"] is False
assert tools["merge_pr"] is False
# ── check-permissions.py passes ─────────────────────────────────────────────
def test_check_permissions_passes():
"""The validator exits 0 with OK message on current configs."""
result = subprocess.run(
["python3", str(CHECK_PERM_SCRIPT)],
capture_output=True,
text=True,
check=False,
)
assert result.returncode == 0, result.stderr
assert "OK: No dangerous permission rules found." in result.stdout
if __name__ == "__main__":
import pytest
pytest.main([__file__, "-v"])