Skip to content

[BLOCKER][V1] Явный permission.acp_status: "deny" молча переписывается в "allow" #457

Closed

Description

@akrhin

Problem

An explicit user permission.acp_status: "deny" in opencode.json is silently rewritten to "allow" by the V1 plugin, inverting the access decision without any log or error.

Location

lib/v1/plugin.ts:227-234

if (!hasExplicitToolPermission(opencodeConfig.permission, "compress")) {
    const permission = opencodeConfig.permission ?? {}
    opencodeConfig.permission = {
        ...permission,
        compress: config.compress.permission,
        acp_status: "allow",      // ← overrides the user's deny (key order wins)
    } as typeof permission
}

hasExplicitToolPermission (lib/host-permissions.ts:129-134) only checks for the presence of the "compress" key — it never looks at acp_status.

Reproduction

User config:

{ "permission": { "acp_status": "deny" } }
  1. hasExplicitToolPermission(permission, "compress") → false (no compress key), so the guard passes.
  2. {...{acp_status:"deny"}, compress: "allow", acp_status: "allow"} → acp_status: "allow" wins by key order.
  3. hostPermissions.global = opencodeConfig.permission (lib/v1/plugin.ts:236) caches the already-inverted map.
  4. dispose (:244-249) does not restore the host object — there is no way back in code.

Result: the V1 host reads its own permissions from the same object, so the deny is not merely ignored by ACP — it is not honoured by the host either. Recovery requires editing the config and restarting.

Impact

  • Severity: BLOCKER (silent inversion of an explicit access decision; V1 line ships in the published package; no in-code escape hatch).
  • Any user who denies only acp_status and does not also declare a compress rule gets the opposite of what they configured.

Suggested fix

Do not write acp_status when the user already set that key. Either drop acp_status from the written object entirely (the tool already has its own permission path) or guard it with the same presence check used for compress:

const next = { ...permission, compress: config.compress.permission }
if (!Object.prototype.hasOwnProperty.call(permission, "acp_status")) next.acp_status = "allow"

Add a regression test asserting an explicit acp_status: "deny" survives the config write (green only after the fix).

Evidence

Found by an independent read-only bug sweep of the V1 line; the guard, the helper semantics, the cache write and the non-restoring dispose were all verified against the code.

Activity

  1. ranxianglei commented on Sep 29, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    Баг подтверждён и исправлен. Спасибо за отчёт — ваш анализ полностью верен.

    Верификация до реализации (read-only аудит):

    • guard проверяет только ключ compress (hasExplicitToolPermission, lib/host-permissions.ts) — acp_status он не рассматривает;
    • в spread побеждает последний ключ: {...{acp_status:"deny"}, compress:X, acp_status:"allow"} → "allow";
    • инвертированная карта кэшируется в hostPermissions.global, а V1-хост читает свои разрешения из того же объекта — deny не соблюдает никто;
    • dispose исходный объект не восстанавливает — восстановление только через правку конфига и рестарт.

    Нюанс по локации: на текущем master этот код находится в index.ts (config hook, строки ~252–259), а не в lib/v1/plugin.ts — логика идентична, сменились путь и номера строк.

    Фикс — PR: #466

    Логика слияния вынесена в чистую функцию applyAcpToolPermissions() в lib/host-permissions.ts (фабрика плагина в index.ts имеет тяжёлые runtime-зависимости и не юнит-тестируется напрямую):

    • явное пользовательское правило compress → карта возвращается без изменений (defer-by-design сохранён, включая identity объекта);
    • иначе записывается compress из конфига ACP, а acp_status: "allow" — только если ключ отсутствует. Явные deny/ask теперь переживают запись.

    Изменение поведения (старое → новое):

    Сценарий (user config) Было Стало
    acp_status в конфиге отсутствует пишется "allow" без изменений
    явный acp_status: "deny", без правила compress молча переписывалось в "allow" ❌ остаётся "deny" ✅ (инструмент уходит из tool list LLM — именно то, что должен делать deny)
    явный acp_status: "ask", без правила compress молча переписывалось в "allow" ❌ остаётся "ask" ✅
    есть явное правило compress полный defer, ничего не пишется без изменений

    Без изменений: формат состояния, схема конфига, persistence, public API, версия пакета. Путь bili-proxy (намеренный полный deny) не тронут.

    Верификация: RED→GREEN — при временном возврате безусловного перезаписи падают ровно два новых регрессионных теста (13 pass / 2 fail); после фикса — зелёные. Добавлено 6 тестов в tests/host-permissions.test.ts. typecheck / build / целевой саут — pass; полный саут 1346/1348 (2 падения — артефакты локального песочного окружения с read-only /tmp, файлы не пересекаются с изменёнными модулями, в CI проходят).

    Вне скоупа (осознанно): предсуществующая асимметрия, при которой явный compress полностью деферирует и acp_status падает на хостовский дефолт ("ask") — это согласуется с defer-by-design; фиксируем отдельным issue'м, если решите, что это тоже дефект.

    中文摘要:修复了显式配置 permission.acp_status: "deny" 被 config hook 的 spread 静默改写为 "allow" 的问题(guard 只查 compress 键);合并逻辑抽为纯函数 applyAcpToolPermissions(),缺失键才写默认值,附 RED→GREEN 回归测试;CI 绿后可由人工合并。

  2. ranxianglei commented on Sep 29, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    CI по PR #466 полностью зелёный: pr-validation ✅ · build ✅ · test (22) ✅ · test (24) ✅ · build-artifact ✅ · e2e (Docker) ✅.

    PR готов к ручному merge'у (merge — только человек): #466

  3. ranxianglei commented on Sep 29, 2026

    @ranxianglei
    Owner

    This one is different from the rest — and you're right. It applies to the live OpenCode 1.x path, not just the retired V2 port. On current master, index.ts (~lines 252–258) does exactly what you describe: when there's no explicit "compress" permission, the plugin writes acp_status: "allow" into the merged permission object, silently overriding an explicit user acp_status: "deny". hasExplicitToolPermission (lib/host-permissions.ts) only checks for the "compress" key — it never looks at acp_status.

    So this is a genuine V1 bug and stays in opencode-acp (it is not redirected to billion-context). I've flagged it to the maintainer for a proper fix; the intended semantics need a decision (e.g., don't force acp_status when the user has set it explicitly). We'll track it here.

  4. ranxianglei commented on Sep 29, 2026

    @ranxianglei
    Owner

    🤖 Powered by ework · qwen3.8-27b

    This exact bug is already fixed — PR #466 (open, all 6 CI checks green, mergeable_state: clean, head 2a500024): #466

    The semantics implemented are exactly the one you propose — don't force acp_status when the user has set it explicitly. Concretely, the merge logic was extracted into a pure applyAcpToolPermissions() in lib/host-permissions.ts:

    • explicit user acp_status (deny / ask) → survives the config hook untouched;
    • acp_status absent → "allow" default written as before (no behavior change for existing users);
    • explicit user compress rule → full defer, the map is returned unchanged (no ACP key injected at all — pre-existing defer-by-design preserved).

    Verified RED→GREEN: with the unconditional overwrite temporarily reintroduced, exactly the two new regression tests fail; after the fix the whole suite is green. No state-format, config-schema, or public-API change; no version bump.

    So no separate decision is needed — the fix already encodes "explicit user values win". The only remaining step is a human merge of #466 (agent merges are forbidden by repo policy).

    中文摘要:这个 V1 bug 已在 PR #466 修好(CI 全绿、mergeable clean),语义正是「用户显式设置的 acp_status 不覆盖、缺失才写默认 allow」,回归测试 RED→GREEN 已验证;只差人工合并。

  5. added 2 commits that reference this issue on Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions