Skip to content

permissions set --allow/--deny accepts arbitrary strings with no validation against known operations or categories #688

Description

@vojtechnovotny-heu

Problem

kbagent permissions set --allow/--deny performs zero validation on the pattern strings it is given. Any string is accepted, persisted to config.json, and echoed back as a normal rule -- including strings that can never match anything.

Reproduced on the CLI:

❯ kbagent permissions set --mode deny --allow "cli:read" --allow "tool:read" --allow "serve" --allow "project.refresh" --allow "org.setup" --allow "storage.upload-table" --allow "data-app.deploy" --allow "workspace.create" --allow "workspace.delete" --allow "token.create" --allow "data-app.delete" --allow "auth.login" --allow "tool.admin"
Permission policy updated.
  Allow: cli:read, tool:read, serve, project.refresh, org.setup, storage.upload-table, data-app.deploy, workspace.create, workspace.delete, token.create, data-app.delete, auth.login, tool.admin

❯ kbagent permissions set --mode deny --allow "cli:read" ... --allow "tool.adminasdas"
Permission policy updated.
  Allow: ..., tool.adminasdas

tool.admin and the typo'd tool.adminasdas are accepted identically, with the same "Permission policy updated." success message. Neither is a real category, and neither matches any operation.

Root cause

  • permissions_set (src/keboola_agent_cli/commands/permissions.py:220-277) builds PermissionPolicy(mode=mode, allow=allow or [], deny=deny or []) directly from the raw Typer option values. The only validation in the whole command is the --mode check a few lines above it (commands/permissions.py:259-264) -- nothing checks --allow/--deny against OPERATION_REGISTRY, the four cli:* categories, or even basic shape (dot vs. the retired tool: colon namespace).
  • _matches_pattern (src/keboola_agent_cli/permissions.py:493-521) only special-cases the exact strings cli:read / cli:write / cli:destructive / cli:admin (line 509). Everything else -- including a plausible-looking but nonexistent category like tool.admin, or a typo of a real operation like stroage.upload-table -- falls through to fnmatch.fnmatch(operation, pattern) (line 521) and matches zero entries in OPERATION_REGISTRY, so the rule is permanently inert.
  • find_inert_patterns (src/keboola_agent_cli/permissions.py:472-490), which is what permissions show / doctor use to warn about dead rules, only detects the one specific retired tool: colon-prefix left over from the pre-0.85.0 MCP passthrough (pattern.startswith(INERT_PATTERN_PREFIX), line 488). It does not generalize to "this pattern currently matches nothing," so a fabricated or misspelled pattern is never surfaced -- permissions show prints it back with no warning at all.

Why this matters

permissions set is gated behind require_random_code_confirmation specifically because it's meant to be a deliberate, reviewed security action (see the module docstring: "so that an AI agent constrained by the policy cannot bypass it programmatically"). Silently accepting a no-op pattern is worse than rejecting it outright: the operator walks away believing they tightened (or loosened) the firewall -- e.g. believing tool.admin grants or blocks something -- when the policy is functionally unchanged. It also contradicts CONTRIBUTING.md's own "Validate at system boundaries" rule, which names CLI arguments as exactly the kind of input that should be validated in the service layer.

Suggested fix

  1. In permissions_set, validate each --allow/--deny value before saving: accept only the four cli:* categories, exact OPERATION_REGISTRY keys, or glob patterns that match at least one registry key (e.g. fnmatch.filter(OPERATION_REGISTRY, pattern) non-empty). Reject anything else with ErrorCode.VALIDATION_ERROR and exit code 2, mirroring the existing --mode validation immediately above it.
  2. Generalize find_inert_patterns to flag any persisted pattern that currently matches zero operations and isn't a recognized cli:* category, rather than only the hardcoded tool: prefix. That way a stale or misspelled rule surfaces via permissions show / doctor even for policies written before validation existed.

Happy to send a PR for (1) -- it stays inside the existing commands/permissions.py / permissions.py boundary and reuses OPERATION_REGISTRY / _matches_pattern that already exist.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions