fix(permissions): validate set --allow/--deny patterns against known operations (#688)

CLAUDE.md

@@ -687,6 +687,14 @@ kbagent token refresh --project NAME --token-id ID [--yes]
 # like cli:read, cli:write, cli:destructive). `tool:*` patterns are INERT since 0.85.0 (the MCP
 # passthrough is gone): they load but match nothing, so a mode=deny policy whose only allowance was
 # tool:read now denies everything. The agent guards rails against mistakes; not a sandbox.
+# `permissions set` (since vNEXT, issue #688) validates every --allow/--deny pattern BEFORE the
+#   interactive confirmation: each must be a cli:* category, an exact operation name (incl. a
+#   flag-escalated string like "auth.logout --remove-projects"), or a glob matching >=1 known
+#   operation -- an unknown pattern (typo, fabricated category) is rejected with VALIDATION_ERROR,
+#   exit 2, listing every offending pattern (--json: error.details.invalid_patterns), and nothing
+#   is persisted. `permissions show` / `kbagent doctor` also generalized: they now flag ANY
+#   persisted pattern matching zero known operations, not only the retired tool:* namespace.
+#   PermissionEngine itself stays lenient at evaluation time -- only `permissions set` is strict.
 kbagent permissions list [--category read|write|destructive|admin]
 kbagent permissions show
 kbagent permissions set --mode allow|deny [--allow PATTERN ...] [--deny PATTERN ...]

docs/guide.md

@@ -68,7 +68,7 @@ The agent can browse configs, list jobs and trace lineage -- but cannot create b
 | `cli:read` | All read-only CLI commands |
 | `branch.delete` | Exact command |
 | `sync.*` | All sync subcommands |
-| `tool:*` | Nothing -- inert since 0.85.0 (the MCP passthrough is gone); a `--mode deny` policy that only allowed `tool:read` now denies everything |
+| `tool:*` | Nothing -- inert since 0.85.0 (the MCP passthrough is gone); a `--mode deny` policy that only allowed `tool:read` now denies everything; `permissions set` REJECTS it as input (since vNEXT) -- only an already-persisted `tool:*` sticks around |
 
 ### Management commands
 

plugins/kbagent/agents/keboola-expert.md

@@ -372,6 +372,12 @@ its absence is NOT a promise the entry is version-independent (see §1 Rule 6).
   (`fallback_used: "heuristic"`), not the full AI wizard (that is the `sl-build`
   skill).
 
+**`permissions set --allow/--deny` validates patterns (since vNEXT)**
+- A typo'd or fabricated pattern (`tool.admin`, `stroage.*`) fails fast with
+  `VALIDATION_ERROR`, exit 2, instead of persisting silently -- valid inputs
+  are `cli:*` categories, exact operation names, or globs matching >=1
+  operation (check with `kbagent permissions list`).
+
 ---
 
 ## 4. WORKFLOWS (reference playbooks)

plugins/kbagent/skills/kbagent/references/commands-reference.md

@@ -93,8 +93,8 @@ All seven commands authenticate via `KBC_MANAGE_API_TOKEN` (Manage API), not the
 ## Permissions (session firewall commands)
 The `permissions` subcommands persist a write/destructive policy to config.json (the `--deny-*` flags above are the one-shot form). The engine guards against agent mistakes; it is not a sandbox.
 - `permissions list [--category read|write|destructive|admin]` -- list all operations with their risk category and current allowed/denied status
-- `permissions show` -- show the current active permission policy
-- `permissions set --mode allow|deny [--allow PATTERN ...] [--deny PATTERN ...]` -- set the permission policy (firewall rules); patterns like `cli:read`, `cli:write`, `cli:destructive`. `tool:*` patterns are INERT since v0.85.0 (the MCP passthrough is gone) -- they load but match nothing, so a `--mode deny` policy whose only allowance was `tool:read` now denies everything
+- `permissions show` -- show the current active permission policy. *(since vNEXT)* also flags ANY persisted pattern matching zero known operations, not only the retired `tool:*` namespace (same generalization applies to `kbagent doctor`) -- a typo like `cli:reed` or `stroage.upload-table` is surfaced too, with a generic "check for typos" hint instead of the MCP-migration one
+- `permissions set --mode allow|deny [--allow PATTERN ...] [--deny PATTERN ...]` -- set the permission policy (firewall rules); patterns like `cli:read`, `cli:write`, `cli:destructive`. `tool:*` patterns are INERT since v0.85.0 (the MCP passthrough is gone) -- they load but match nothing, so a `--mode deny` policy whose only allowance was `tool:read` now denies everything. *(since vNEXT, issue #688)* Every pattern is validated BEFORE the interactive confirmation: it must be a `cli:*` category, an exact operation name, or a glob matching at least one known operation, or the call fails fast with `VALIDATION_ERROR` (exit 2, `error.details.invalid_patterns` in `--json`) and nothing is written
 - `permissions reset` -- remove all permission restrictions
 - `permissions check OPERATION` -- check if a specific operation is allowed (e.g. `permissions check storage.delete-table`)
 

plugins/kbagent/skills/kbagent/references/gotchas.md

@@ -4601,3 +4601,46 @@ directly to a workspace via the `KBC_<STACK>_<PROJECT>` shared database --
 `workspace query` runs against it with zero load and zero extra storage.
 Reach for `workspace load` only when the workflow actually needs the data
 materialized inside the workspace.
+
+## `permissions set` now rejects unknown patterns instead of silently persisting them (since vNEXT)
+
+Before this fix (issue #688), `kbagent permissions set --allow/--deny PATTERN`
+accepted any string with zero validation. A typo like `tool.admin` or
+`stroage.upload-table` (missing the `-` in `storage`) was written straight to
+`config.json` as a dead rule that would never match anything -- the failure
+was silent, and the only way to notice was reading `permissions show` (or
+`kbagent doctor`) after the fact.
+
+- **Now validated up front, before the interactive confirmation.** Every
+  `--allow`/`--deny` pattern must be one of: a `cli:*` category
+  (`cli:read`/`cli:write`/`cli:destructive`/`cli:admin`), an exact operation
+  name (an `OPERATION_REGISTRY` key, or a flag-escalated string like
+  `"auth.logout --remove-projects"`), or a glob that matches at least one
+  known operation. Anything else fails fast with `VALIDATION_ERROR`, exit 2 --
+  `--json` lists every offending pattern (deduplicated) in
+  `error.details.invalid_patterns`. Nothing is persisted, and the random-code
+  confirmation prompt is never shown for a call that was going to fail
+  anyway.
+- **`permissions show` / `kbagent doctor` detection is generalized the same
+  way.** Both used to flag only the retired `tool:*` namespace (see the MCP
+  removal gotcha above); they now flag ANY persisted pattern matching zero
+  known operations, so a policy written before this gate existed (or one
+  edited directly in `config.json`) still surfaces its typos. The `tool:`
+  patterns keep their specific "MCP passthrough removed, see
+  docs/mcp-migration.md" hint; every other dead pattern gets a generic
+  "check for typos against `kbagent permissions list`" hint instead. A policy
+  mixing both kinds shows both hints.
+- **`PermissionEngine` itself stays lenient at evaluation time** -- a dead
+  pattern already on disk is a silent no-op when the policy is enforced, not
+  a crash. Only `permissions set` is strict; this preserves backward
+  compatibility for a pre-existing policy that happens to carry a typo.
+- **Version skew is a real rejection case, not just a typo.** A pattern
+  naming an operation that only exists on a newer kbagent version (a command
+  added after the running build was installed) is rejected the same as a
+  typo -- it would be inert on this build anyway -- so upgrade first, then
+  persist the pattern.
+- **`doctor --json`'s `inert_permission_patterns` check carries
+  `details.inert_since` only when at least one offending pattern is
+  `tool:`-prefixed** (since vNEXT) -- a purely typo'd policy (e.g.
+  `stroage.upload-table`) omits the key entirely, so JSON consumers must not
+  assume `inert_since` is always present on `status: "warn"`.

plugins/kbagent/skills/kbagent/references/permissions-workflow.md

@@ -27,6 +27,16 @@ The agent CANNOT:
 > denies everything. Rewrite such a policy with `cli:read`. `kbagent permissions
 > show` names any such pattern (key `inert_patterns` in `--json`) and `kbagent
 > doctor` WARNs via its `inert_permission_patterns` check.
+>
+> **(since vNEXT)** `permissions set` now validates every `--allow`/`--deny`
+> pattern before persisting it: it must be a `cli:*` category, an exact
+> operation name, or a glob matching >=1 known operation, else the call fails
+> with `VALIDATION_ERROR`, exit 2, before the confirmation prompt is even
+> shown. `permissions show` / `doctor` are generalized the same way -- they
+> now flag ANY persisted pattern matching zero operations (a typo included),
+> not only `tool:*`. `tool:*` remains the one historical case with its own
+> "MCP passthrough removed" hint; every other dead pattern gets a generic
+> "check for typos" hint instead.
 
 ## Common restriction recipes
 
@@ -96,7 +106,7 @@ kbagent --json permissions list
 | `cli:read` | All read-only CLI commands |
 | `branch.delete` | Exact command match |
 | `sync.*` | All sync subcommands (glob) |
-| `tool:*` | Nothing -- inert since v0.85.0 (the MCP passthrough is gone) |
+| `tool:*` | Nothing -- inert since v0.85.0 (the MCP passthrough is gone); `permissions set` REJECTS it as input (since vNEXT) -- only an already-persisted `tool:*` sticks around |
 
 ## Session firewall flags
 

src/keboola_agent_cli/commands/context.py

@@ -1941,6 +1941,12 @@
     they matched is gone. A persisted policy still loads with them, but they
     match nothing, so a mode=deny policy whose only allowance was `tool:read`
     now denies everything. Rewrite such a policy with `cli:read`.
+    Since vNEXT (issue #688): every pattern is validated BEFORE the
+    interactive confirmation -- it must be a cli:* category, an exact
+    operation name, or a glob matching >=1 known operation, or the whole
+    call fails with VALIDATION_ERROR (exit 2, invalid patterns listed) and
+    nothing is written. `permissions show` / `kbagent doctor` likewise flag
+    ANY dead pattern already on disk, not only `tool:*`.
 
   kbagent permissions reset
     Remove all restrictions.

src/keboola_agent_cli/commands/permissions.py

@@ -19,9 +19,12 @@
 from ..models import PermissionPolicy
 from ..permissions import (
     INERT_PATTERN_HINT,
+    INERT_PATTERN_PREFIX,
     INERT_SINCE_VERSION,
+    UNMATCHED_PATTERN_HINT,
     PermissionEngine,
     find_inert_patterns,
+    pattern_matches_known_operation,
 )
 from ._helpers import get_formatter, get_service, require_random_code_confirmation
 
@@ -202,10 +205,31 @@ def permissions_show(
         if persisted.deny:
             formatter.console.print(f"[bold]Deny:[/bold] {', '.join(persisted.deny)}")
         if inert_patterns:
+            # Generalized (issue #688): ANY pattern matching no known operation
+            # is reported here, not only the retired `tool:` namespace. The
+            # `tool:`-specific MCP-migration hint is appended only when at
+            # least one offending pattern starts with that prefix; the
+            # generic typo hint is appended whenever any other kind is
+            # present too -- both can show up together in a mixed policy.
+            has_tool_prefix = any(p.startswith(INERT_PATTERN_PREFIX) for p in inert_patterns)
+            has_other = any(not p.startswith(INERT_PATTERN_PREFIX) for p in inert_patterns)
+            since_clause = (
+                f" since v{INERT_SINCE_VERSION} (the 'tool:' namespace was removed "
+                "with the MCP passthrough)"
+                if has_tool_prefix
+                else ""
+            )
+            hints = [
+                h
+                for h, present in (
+                    (INERT_PATTERN_HINT, has_tool_prefix),
+                    (UNMATCHED_PATTERN_HINT, has_other),
+                )
+                if present
+            ]
             formatter.console.print(
-                f"[yellow]{len(inert_patterns)} inert pattern(s) since v{INERT_SINCE_VERSION} "
-                "(the 'tool:' namespace was removed with the MCP passthrough): "
-                f"{', '.join(inert_patterns)}. {INERT_PATTERN_HINT}[/yellow]"
+                f"[yellow]{len(inert_patterns)} inert pattern(s){since_clause} match no "
+                f"known operation: {', '.join(inert_patterns)}. {' '.join(hints)}[/yellow]"
             )
     else:
         formatter.console.print("[dim]No persisted permission policy (config.json is clean).[/dim]")
@@ -244,6 +268,13 @@ def permissions_set(
     Requires interactive confirmation (type a random code) to prevent
     AI agents from modifying permissions programmatically.
 
+    Every ``--allow`` / ``--deny`` pattern must be a ``cli:*`` category, an
+    exact operation name, or a glob matching at least one known operation
+    (issue #688) -- rejected up front (VALIDATION_ERROR, exit 2) BEFORE the
+    interactive confirmation, so a typo'd pattern never gets silently
+    persisted as a dead rule and no confirmation prompt is wasted on a call
+    that was going to fail anyway.
+
     Examples:
       # Block all write operations (Vojta's use case):
       kbagent permissions set --mode allow --deny "cli:write"
@@ -263,6 +294,24 @@ def permissions_set(
         )
         raise typer.Exit(code=2) from None
 
+    invalid: list[str] = []
+    for pattern in [*(allow or []), *(deny or [])]:
+        if not pattern_matches_known_operation(pattern) and pattern not in invalid:
+            invalid.append(pattern)
+    if invalid:
+        formatter.error(
+            message=(
+                "Unknown operation pattern(s): "
+                + ", ".join(invalid)
+                + ". Patterns must be a cli:* category (cli:read, cli:write, "
+                "cli:destructive, cli:admin), an exact operation name, or a glob "
+                "matching at least one operation. See `kbagent permissions list`."
+            ),
+            error_code=ErrorCode.VALIDATION_ERROR,
+            details={"invalid_patterns": invalid},
+        )
+        raise typer.Exit(code=2) from None
+
     require_random_code_confirmation("update permission policy")
 
     config_store: ConfigStore = get_service(ctx, "config_store")

src/keboola_agent_cli/permissions.py

@@ -412,6 +412,22 @@
 # show` and `kbagent doctor` cannot drift apart.
 INERT_PATTERN_HINT = "Rewrite the intent with cli:* categories -- see docs/mcp-migration.md."
 
+# Generic hint for a pattern that matches nothing for a reason other than the
+# retired `tool:` namespace (most commonly a typo). Kept distinct from
+# INERT_PATTERN_HINT so `permissions show` / `doctor` can point at the right
+# fix instead of always mentioning the MCP migration.
+UNMATCHED_PATTERN_HINT = (
+    "Check for typos against `kbagent permissions list`, or use a cli:* category "
+    "(cli:read, cli:write, cli:destructive, cli:admin)."
+)
+
+# The four risk-category patterns `_matches_pattern` special-cases. Exported
+# so `pattern_matches_known_operation` cannot drift from the engine's own
+# notion of "valid category pattern".
+CLI_CATEGORY_PATTERNS: frozenset[str] = frozenset(
+    {"cli:read", "cli:write", "cli:destructive", "cli:admin"}
+)
+
 
 def apply_firewall_flags(
     persisted: PermissionPolicy | None,
@@ -469,13 +485,36 @@ def apply_firewall_flags(
     )
 
 
+def pattern_matches_known_operation(pattern: str) -> bool:
+    """True if the pattern is a cli:* category or matches >=1 known operation.
+
+    Known operations are ``OPERATION_REGISTRY`` keys plus ``FLAG_ESCALATIONS``
+    keys (flag-escalated strings like ``auth.logout --remove-projects`` are
+    real operation strings passed to ``PermissionEngine.is_allowed()``, see
+    ``commands/auth.py``). A glob pattern counts as valid when it matches at
+    least one of those operation strings via ``fnmatch.filter``.
+
+    Used both to reject unknown patterns at write time (``permissions set``)
+    and to flag ones already on disk (``find_inert_patterns``) -- kept as a
+    single source of truth so the two checks cannot drift apart.
+    """
+    if pattern in CLI_CATEGORY_PATTERNS:
+        return True
+    return bool(fnmatch.filter([*OPERATION_REGISTRY, *FLAG_ESCALATIONS], pattern))
+
+
 def find_inert_patterns(policy: PermissionPolicy | None) -> list[str]:
     """Patterns in a persisted policy that can no longer match any operation.
 
-    The MCP passthrough was removed in 0.85.0 and with it the ``tool:``
-    operation namespace; patterns targeting it fall through to fnmatch and
-    match nothing. Surfaced by ``permissions show`` and ``kbagent doctor``
-    so a pre-0.85 policy does not silently carry dead rules.
+    Generalized (issue #688): flags ANY pattern that matches zero known
+    operations, not only the retired ``tool:`` namespace. That namespace --
+    gone since the MCP passthrough was removed in 0.85.0 -- is simply the
+    most common historical cause; a typo'd operation name or a stale glob is
+    just as inert and just as worth surfacing. Surfaced by ``permissions
+    show`` and ``kbagent doctor`` so a policy does not silently carry dead
+    rules. ``permissions set`` (issue #688) additionally REJECTS such
+    patterns at write time -- this function stays for what is already
+    persisted (pre-fix policies, or the engine's own leniency).
 
     Returns the offending patterns in policy order (allow first, then deny),
     de-duplicated -- the same dead pattern listed twice is one problem.
@@ -485,7 +524,7 @@ def find_inert_patterns(policy: PermissionPolicy | None) -> list[str]:
 
     inert: list[str] = []
     for pattern in [*policy.allow, *policy.deny]:
-        if pattern.startswith(INERT_PATTERN_PREFIX) and pattern not in inert:
+        if not pattern_matches_known_operation(pattern) and pattern not in inert:
             inert.append(pattern)
     return inert
 
@@ -506,7 +545,7 @@ def _matches_pattern(operation: str, pattern: str) -> bool:
     match nothing, and stay inert instead of raising.
     """
     # Category patterns: cli:read, cli:write, cli:destructive, cli:admin
-    if pattern in ("cli:read", "cli:write", "cli:destructive", "cli:admin"):
+    if pattern in CLI_CATEGORY_PATTERNS:
         target_category = pattern.split(":")[1]
         # Fail-closed: unknown CLI ops default to 'write' so they are
         # blocked by cli:write policies. This prevents new commands from