fix: allow piped bash commands through Copilot SDK permission checker#37369
Conversation
- Add bash_command_parser.cjs: dedicated parser splitting on &&, ||, |, ; with quote and $() awareness - Update copilot_sdk_driver.cjs: fallback for empty commandIdentifiers uses pipeline parser so piped commands (e.g. ls && cat || echo) are allowed instead of denied - Add bash_command_parser.test.cjs: 55 unit tests - Add fuzz_bash_command_parser_harness.cjs: property-based fuzz harness - Add fuzz_bash_command_parser_harness.test.cjs: 177 property/fuzz tests - Update copilot_sdk_driver.test.cjs: 9 new integration tests for piped permission handling - Add bash_tool_compiler.py: Python implementation of the same bash pipeline parser and permission checker - Add test_bash_tool_compiler.py: 87 Python unittest+pytest tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- copilot_sdk_driver.cjs: expand JSDoc on isIdentifierAllowedByShellRules to detail all three rule formats (wildcard :*, single-word, full-command) - bash_command_parser.cjs: expand SHELL_KEYWORDS comment explaining why keywords must be excluded from permission checking - fuzz_bash_command_parser_harness.test.cjs: extract nested flatMap/filter/map chain into generateCommandPairs helper - bash_tool_compiler.py: add note explaining why linear scan in is_identifier_allowed_by_shell_rules is acceptable Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add extensive unit test suite for parser. Mine existing AW for patterns. |
There was a problem hiding this comment.
Pull request overview
This PR fixes Copilot SDK permission-checking for chained/piped shell commands when the SDK provides an empty commands: [] identifiers list, by falling back to parsing fullCommandText and requiring every pipeline stage’s command name to be permitted.
Changes:
- Added a bash pipeline parser (
splitOnPipelineOperators,extractCommandName,extractCommandNamesFromPipeline) and used it as a fallback in the Node Copilot SDK permission handler. - Added comprehensive JS unit + fuzz/property tests covering pipeline splitting and command-name extraction.
- Added a Python port of the parser and permission helpers with a matching test suite.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/copilot_sdk_driver.cjs | Adds pipeline-aware fallback permission logic for shell requests with missing SDK identifiers. |
| actions/setup/js/bash_command_parser.cjs | Introduces state-machine parsing for pipeline operators and command-name extraction. |
| actions/setup/js/copilot_sdk_driver.test.cjs | Adds integration tests for piped/chained command permission scenarios. |
| actions/setup/js/bash_command_parser.test.cjs | Adds unit tests for the new parser utilities. |
| actions/setup/js/fuzz_bash_command_parser_harness.cjs | Provides a structured harness for fuzz/property invariants (no-throw, quoted operators, safe defaults). |
| actions/setup/js/fuzz_bash_command_parser_harness.test.cjs | Adds vitest-based fuzz/property tests over operators/quoting matrices and invariants. |
| .github/drivers/bash_tool_compiler.py | Adds Python implementation of the parser + permission helpers for Python-based drivers. |
| .github/drivers/test_bash_tool_compiler.py | Adds a comprehensive Python unittest suite for the new Python parser/helpers. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 6
| // ── $(...) subshell: skip balanced parentheses as a unit ── | ||
| if (ch === "$" && i + 1 < len && commandText[i + 1] === "(") { | ||
| current += ch; | ||
| i++; | ||
| let depth = 0; | ||
| while (i < len) { | ||
| const sc = commandText[i]; | ||
| if (sc === "(") depth++; | ||
| else if (sc === ")") { | ||
| depth--; | ||
| current += sc; | ||
| i++; | ||
| if (depth === 0) break; | ||
| continue; | ||
| } | ||
| current += sc; | ||
| i++; | ||
| } | ||
| continue; | ||
| } |
| // Skip leading env-var assignments: IDENTIFIER=anything (repeat) | ||
| const envAssignRe = /^[A-Za-z_][A-Za-z0-9_]*=\S*\s*/; |
| it("handles nested $() subshells", () => { | ||
| const segments = splitOnPipelineOperators("echo $(echo $(ls && pwd)) && date"); | ||
| expect(segments).toHaveLength(2); | ||
| expect(segments[0]).toContain("echo $(echo $(ls && pwd))"); | ||
| expect(segments[1]).toContain("date"); | ||
| }); | ||
|
|
| # ── $(...) subshell: balanced parentheses ───────────────────────── | ||
| if ch == "$" and i + 1 < n and command_text[i + 1] == "(": | ||
| current.append(ch) | ||
| i += 1 | ||
| depth = 0 | ||
| while i < n: | ||
| sc = command_text[i] | ||
| if sc == "(": | ||
| depth += 1 | ||
| elif sc == ")": | ||
| depth -= 1 | ||
| current.append(sc) | ||
| i += 1 | ||
| if depth == 0: | ||
| break | ||
| continue | ||
| current.append(sc) | ||
| i += 1 | ||
| continue |
| # Regex to detect leading env-var assignment: WORD=anything | ||
| _ENV_ASSIGN_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*=\S*") |
| names = extract_command_names_from_pipeline(command_text) | ||
| if not names: | ||
| return False | ||
| return all(is_identifier_allowed_by_shell_rules(name, shell_rules) for name in names) |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added an extensive parser unit test vector suite in |
|
@copilot merge main and recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in |
| @@ -0,0 +1,362 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
@copilot delete this file. We are not maintaimimg a python runtime
There was a problem hiding this comment.
Done in 424abea — removed .github/drivers/bash_tool_compiler.py and the related .github/drivers/test_bash_tool_compiler.py file.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
When the Copilot SDK sends a permission request for a piped/chained command (e.g.
ls /tmp && cat file.json || echo "not found"), it sendscommands: []— an empty identifiers array. The original checker only testedcommandIdentifiers.includes(rule), so every piped command was unconditionally denied, hitting the 5/5 denial threshold and killing the GEO Optimizer agent session.Core fix —
copilot_sdk_driver.cjsAdded a fallback path in
isAllowedfor the"shell"case: whencommandIdentifiersis empty, parsefullCommandTextvia the new pipeline parser and require all pipeline stages to be individually permitted (every(), notsome()— a disallowed stage anywhere in the chain still denies the whole command).New parser —
bash_command_parser.cjsDedicated state-machine parser that splits on
&&,||,|,;while correctly handling single/double quotes and$(...)subshells. Exports:splitOnPipelineOperators(text)— returns pipeline segmentsextractCommandName(segment)— skips env-var assignments, redirections, shell keywords (then,fi, etc.), and negation (!)extractCommandNamesFromPipeline(text)— deduplicated end-to-endRuntime scope
This PR now targets the JavaScript runtime only. The previously added Python driver/runtime artifacts were removed because this repository does not maintain a Python runtime for this path.
Tests
bash_command_parser.test.cjsfuzz_bash_command_parser_harness.test.cjscopilot_sdk_driver.test.cjs