Skip to content

Add regression tests for Copilot SDK shell-permission parsing on issue command shapes#37574

Merged
pelikhan merged 3 commits into
mainfrom
copilot/review-copilot-sdk-bash-parser
Jun 7, 2026
Merged

Add regression tests for Copilot SDK shell-permission parsing on issue command shapes#37574
pelikhan merged 3 commits into
mainfrom
copilot/review-copilot-sdk-bash-parser

Conversation

Copilot AI commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

The Copilot SDK permission parser was reviewed against the denied command set from the failing draft-pr-cleanup run, including repo-style path tokens (github/gh-aw) inside shell arguments. This change adds focused regression coverage to ensure those commands are denied under a restricted policy and allowed when corresponding permissions are granted.

  • Problem scope

    • The failure raised concern that parser ordering/path handling might incorrectly deny valid commands.
    • In this workflow, bash policy is intentionally narrow (shell(jq:*)), so non-jq commands should be rejected regardless of argument structure.
  • Change

    • Added regression tests in actions/setup/js/copilot_sdk_driver.test.cjs:
      • denies issue-37538 commands when workflow only allows jq shell usage
      • allows issue-37538 commands when corresponding shell permissions are granted
    • Tests exercise the exact command shapes from the issue (gh ..., safeoutputs ... | head ..., git ... && gh ..., echo "test"), with commands: [] to intentionally exercise the parser fallback path over fullCommandText when SDK identifiers are omitted.
  • Behavior locked in

    • Confirms denial decisions are policy-correct under a restricted allowlist.
    • Confirms the same command shapes are accepted when the workflow explicitly grants the required shell permissions.
    • Guards future parser changes from accidentally regressing either deny or allow behavior for these forms.
const handler = await makePermissionHandlerViaSDK(["shell(jq:*)"]);
expect(
  handler({ kind: "shell", commands: [], fullCommandText: 'echo "test"' }),
).toEqual({ kind: "reject", feedback: "Tool invocation is not allowed by workflow tool permissions." });

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Add regression test for issue #37538 Copilot SDK permission parser Add regression test for Copilot SDK shell-permission parsing on issue command shapes Jun 7, 2026
Copilot AI requested a review from pelikhan June 7, 2026 13:49
@pelikhan

pelikhan commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

@copilot review permissions issues in #37538
And add regression tests from the permissions asked

@pelikhan

pelikhan commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

@copilot add logging in parser to explain rejection

Copilot AI and others added 2 commits June 7, 2026 14:02
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review permissions issues in #37538
And add regression tests from the permissions asked

Addressed in commits 57a19c5 and 9b1a931. I reviewed issue #37538 and added regression coverage for the exact denied command shapes both for jq-only denial and for approval when the corresponding shell permissions are granted, using the SDK commands: [] fallback shape.

Copilot AI changed the title Add regression test for Copilot SDK shell-permission parsing on issue command shapes Add regression tests for Copilot SDK shell-permission parsing on issue command shapes Jun 7, 2026
@pelikhan pelikhan marked this pull request as ready for review June 7, 2026 14:05
Copilot AI review requested due to automatic review settings June 7, 2026 14:05
@pelikhan pelikhan merged commit 9e51cbe into main Jun 7, 2026
1 check failed
@pelikhan pelikhan deleted the copilot/review-copilot-sdk-bash-parser branch June 7, 2026 14:05
Copilot stopped work on behalf of pelikhan due to an error June 7, 2026 14:05
Copilot AI requested a review from pelikhan June 7, 2026 14:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds focused regression coverage for the Copilot SDK permission handler’s fullCommandText fallback parsing when commands: [] is provided, ensuring real-world “issue command shapes” are correctly rejected under a narrow shell allowlist and accepted when corresponding shell permissions are granted.

Changes:

  • Added a regression test that denies non-jq shell commands when policy is restricted to shell(jq:*).
  • Added a regression test that allows the same command shapes when the workflow grants the needed shell permissions.
Show a summary per file
File Description
actions/setup/js/copilot_sdk_driver.test.cjs Adds regression tests covering deny/allow behavior for specific complex shell command shapes when SDK identifiers are omitted.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment on lines +920 to +927
expect(
handler({
kind: "shell",
// Intentional: exercise fullCommandText fallback when SDK omits identifiers.
commands: [],
fullCommandText: command,
})
).toEqual({ kind: "reject", feedback: "Tool invocation is not allowed by workflow tool permissions." });
Comment on lines +941 to +948
expect(
handler({
kind: "shell",
// Intentional: exercise fullCommandText fallback when SDK omits identifiers.
commands: [],
fullCommandText: command,
})
).toEqual({ kind: "approve-once" });
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.

3 participants