Skip to content

fix: allowedTools and allowedCommands, deniedCommands #2511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

omansour
Copy link

@omansour omansour commented Aug 6, 2025

  • Implement 3-step permission evaluation logic:

    1. If execute_bash in allowedTools → always allow (except denied commands)
    2. Check denied command patterns → deny if matched
    3. Use requires_acceptance with allowedCommands for fine-grained control
  • Fix design flaw where allowedCommands patterns only worked when execute_bash was in allowedTools array

  • Add comprehensive test suite with 4 new test cases:

    • test_eval_perm_allowed_tools_priority: validates trusted tool behavior
    • test_eval_perm_fine_grained_control: validates pattern-based permissions
    • test_eval_perm_default_behavior: validates default fallback behavior
    • test_eval_perm_invalid_settings: validates error handling
  • Maintain backward compatibility - all existing tests pass

This resolves the issue where /tools command displayed allowed command patterns but the underlying permission logic didn't properly evaluate them for fine-grained control scenarios.

Related

#2477

test 1

before: execute_bash is not trusted

[test_agent_allow] > /tools


Tool              Permission
▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔Built-in:
- execute_bash    * trusted
- fs_read         * trusted
- fs_write        * not trusted
- knowledge       * not trusted
- report_issue    * trusted
- use_aws         * trust read-only commands


[test_agent_allow] > touch.txt

> I'll create a file named "touch.txt" for you using the touch command.


🛠️  Using tool: execute_bash
 ⋮
 ● I will run the following shell command: touch touch.txt
 ⋮
 ↳ Purpose: Create an empty file named touch.txt


Allow this action? Use 't' to trust (always allow) this tool for the session. [y/n/t]:

[test_agent_allow] >

after

[test_agent_allow] > /tools


Tool              Permission
▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔Built-in:
- execute_bash    * trusted
- fs_read         * trusted
- fs_write        * not trusted
- knowledge       * not trusted
- report_issue    * trusted
- use_aws         * trust read-only commands


[test_agent_allow] > touch toto.txt




🛠️  Using tool: execute_bash (trusted)
 ⋮
 ● I will run the following shell command: touch toto.txt
 ⋮
 ↳ Purpose: Create an empty file named toto.txt


 ⋮
 ● Completed in 0.21s


> I've created the empty file toto.txt in your current directory (
/Users/omsr/Documents/myWorkspace/amazon-q-developer-cli).

test 2

before - denied command seems not respected

[test_agent] > !cat ~/.aws/amazonq/cli-agents/test_agent.json

{
  "$schema": "https://raw.githubusercontent.com/aws/amazon-q-developer-cli/refs/heads/main/schemas/agent-v1.json",
  "name": "test_agent",
  "description": "",
  "prompt": null,
  "mcpServers": {},
  "tools": [
    "*"
  ],
  "toolAliases": {},
  "allowedTools": [
    "fs_read"
  ],
  "resources": [
    "file://AmazonQ.md",
    "file://README.md",
    "file://.amazonq/rules/**/*.md"
  ],
  "hooks": {},
  "toolsSettings": {
    "execute_bash": {
      "allowedCommands": [
       "cargo -v"
      ],
      "deniedCommands": ["touch"],
      "allowReadOnly": true
    }
  },
  "useLegacyMcpJson": true
}
[test_agent] > touch toto.txt

> I'll create the file toto.txt for you using the touch command.


🛠️  Using tool: execute_bash
 ⋮
 ● I will run the following shell command: touch toto.txt
 ⋮
 ↳ Purpose: Create an empty file named toto.txt


Allow this action? Use 't' to trust (always allow) this tool for the session. [y/n/t]:

[test_agent] >

after

[test_agent] > touch toto.txt





> I understand that the execute_bash tool was rejected due to forbidden arguments. The
touch toto.txt command would create an empty file named "toto.txt" in your current directory
(/Users/omsr/Documents/myWorkspace/amazon-q-developer-cli).

[test_agent] > using execute_bash run cargo -v (with a minus v)




🛠️  Using tool: execute_bash (trusted)
 ⋮
 ● I will run the following shell command: cargo -v
 ⋮
 ↳ Purpose: Run cargo with -v flag

Rust's package manager

Usage: cargo [OPTIONS] [COMMAND]
       cargo [OPTIONS] -Zscript <MANIFEST_RS> [ARGS]...

Options:
  -V, --version                  Print...

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ontrol

- Implement 3-step permission evaluation logic:
  1. If execute_bash in allowedTools → always allow (except denied commands)
  2. Check denied command patterns → deny if matched
  3. Use requires_acceptance with allowedCommands for fine-grained control

- Fix design flaw where allowedCommands patterns only worked when execute_bash
  was in allowedTools array

- Enable users to specify command patterns without granting blanket tool access

- Add comprehensive test suite with 4 new test cases:
  - test_eval_perm_allowed_tools_priority: validates trusted tool behavior
  - test_eval_perm_fine_grained_control: validates pattern-based permissions
  - test_eval_perm_default_behavior: validates default fallback behavior
  - test_eval_perm_invalid_settings: validates error handling

- Maintain backward compatibility - all existing tests pass

This resolves the issue where /tools command displayed allowed command patterns
but the underlying permission logic didn't properly evaluate them for
fine-grained control scenarios.
@omansour omansour force-pushed the fix-allowedCommands branch from 82077d3 to 20b287d Compare August 6, 2025 16:42
@omansour omansour marked this pull request as ready for review August 6, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant