agentscope-ai / agentscope-ai/agentscope

feat(powershell): complete PowerShell permission checks, rule matching, and dynamic read-only

Offen
#2,156 2 Kommentare 0 Reaktionen 1 zugewiesene Person Beansprucht von @Shaurya2k06 Auf GitHub ansehen
Help Wanted
Vorherrschende Sprache
Python
Sterne
31.5k
Forks
3.5k
Ø Merge
1 T. 23 Std.
Gemergte PRs (30 T.)
95

Beschreibung

**Background**

PR #2132 added the PowerShell tool but deferred all command-level security. Three gaps remain vs. `Bash`:

1. `check_permissions` returns a flat ASK for every command — no read-only auto-allow, no safety classification.
2. `match_rule` is not overridden, so it inherits the default (rule_content is None). Only tool-name-level rules work; any rule with content (e.g. Remove-Item*) silently never matches.
3. No dynamic read-only (`check_read_only` not overridden), so under EXPLORE mode even Get-ChildItem is denied.

**Changes**

1. Override `match_rule` for content matching (grammar TBD — see Note).
2. Override `check_read_only` for per-invocation read-only detection.
3. Rework `check_permissions` into Bash-style tiers: read-only auto-allow + bypass-immune safety ASKs for dangerous commands.
4. Add unit tests for each.

**Suggestion**

- **Parser** — Bash's parser (`_bash_parser.py`) is tree-sitter-based but tied to the bash grammar and **not reusable** for PowerShell. Reuse the *architecture* instead: a new `PowerShellCommandParser` mirroring Bash's method surface (`is_read_only_command` / `check_dangerous_command` / `check_injection_risk`), with command sets in `_constants.py`.
- **tree-sitter** — the right direction (consistent with Bash), but needs a `tree-sitter-powershell` grammar whose maturity / PyPI packaging must be verified. Fallback: tokenizer + curated command sets. Avoid `[Parser]::ParseInput()` (would require spawning PowerShell to decide whether to run PowerShell).
- **Command coverage (draft):**
- *Read-only (auto-allow):* `Get-*`, `Test-Path`, `Resolve-Path`, `Select-*`, `Where/Sort/Measure-Object`, `Format-*`, `ConvertTo/From-*`, and aliases (`ls/cat/gc/dir/sls/…`). Anything containing a script block `{...}`, `&`, `iex`, or a pipeline into a mutating cmdlet is treated as non-read-only.
- *Dangerous (bypass-immune ASK):* `Remove-Item -Recurse/-Force`, `Clear-Content/Item`, `Format-Volume`, registry writes (`Set-ItemProperty HKLM:`), `Invoke-Expression` / `Start-Process` / `Add-Type`, download-to-`iex`, `Set-ExecutionPolicy` / `Set-MpPreference`, `Stop/Restart-Computer`, `Stop-Process -Force`, `Register-ScheduledTask`.
- *Injection (force non-read-only + ASK):* `$(...)` with side effects, `&` / `.` on dynamic strings, `iex`, nested `-EncodedCommand`, backtick obfuscation, string-built cmdlet names.
- **`match_rule` grammar** — reuse Bash's pattern language (wildcard `*`, prefix `:*`, substring), plus two PowerShell specifics: **case-insensitive** matching and **alias normalization** before matching (so `Get-ChildItem*` also covers `ls`). Verb-noun structure makes `Get-*` prefixes natural.

**Note**

This change alters the tool's security granularity, so the design must be **confirmed in this issue before implementation**:

1. **Rule-match grammar** — adopt the Suggestion above (Bash grammar + case-insensitive + alias normalization), or a different scheme?
2. **Safety-check scope** — confirm/adjust the dangerous vs. read-only command sets listed under Suggestion.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.