Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 7, 2025

Remove the restriction of bash: ["*"] in strict mode

Summary

This PR removes the restriction that prevented bash wildcards (* and :*) from being used in strict mode. Bash tools can now use wildcards even when strict mode is enabled.

Changes Made

  • Explore the repository structure
  • Understand current strict mode validation for bash tools
  • Run baseline tests to verify current state
  • Remove validateStrictBashTools function call from strict mode validation
  • Update tests to reflect new behavior (bash wildcards should be allowed)
  • Run tests to verify changes
  • Build and manually verify the changes
  • Merge main branch to incorporate latest updates
  • Remove unused validateStrictBashTools function to fix linting errors

Technical Details

Files Modified:

  1. pkg/workflow/strict_mode.go

    • Removed call to validateStrictBashTools from the strict mode orchestrator
    • Updated documentation to remove reference to bash wildcard validation
  2. pkg/workflow/validation_strict_mode.go

    • Updated file header documentation to remove bash wildcard validation
    • Removed the validateStrictBashTools function completely (was causing linting errors as unused)
  3. pkg/workflow/strict_mode_test.go

    • Changed three tests that expected bash wildcards to be refused
    • Tests now verify that bash wildcards are allowed in strict mode:
      • bash wildcard star allowed in strict mode
      • bash wildcard colon-star allowed in strict mode
      • bash wildcard star mixed with commands allowed in strict mode

Verification

✅ All unit tests passing (including strict mode tests)
✅ Build successful
✅ Linting passes without errors
✅ Manual verification with test workflows:

  • bash: ["*"] compiles successfully in strict mode
  • bash: [":*"] compiles successfully in strict mode
  • bash: ["echo", "ls", "*", "pwd"] compiles successfully in strict mode
  • Other strict mode validations (permissions, network) still work correctly
    ✅ Merged with latest main branch changes

Other Strict Mode Validations (Still Active)

The following strict mode validations remain in effect:

  1. Write permissions on sensitive scopes (contents, issues, pull-requests) are refused
  2. Network configuration is required and "*" wildcard in allowed domains is refused
  3. Custom MCP servers with containers require network configuration
Original prompt

Remove the restriction of bash: ["*"] in strict mode.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Remove call to validateStrictBashTools from validateStrictMode
- Update documentation to reflect that bash wildcards are now allowed
- Mark validateStrictBashTools as deprecated but keep for historical reference
- Update tests: bash wildcards ("*" and ":*") now allowed in strict mode
- All tests passing, build successful
- Manual verification completed with test workflows

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove restriction of bash: ["*"] in strict mode Remove bash wildcard restriction in strict mode Nov 7, 2025
Copilot AI requested a review from pelikhan November 7, 2025 03:46
Copilot finished work on behalf of pelikhan November 7, 2025 03:46
@pelikhan pelikhan marked this pull request as ready for review November 7, 2025 04:18
Copilot AI review requested due to automatic review settings November 7, 2025 04:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the bash wildcard restriction from strict mode validation, allowing workflows compiled with --strict to use bash wildcards like * and :*. The validation function validateStrictBashTools() is deprecated but retained for historical reference.

Key changes:

  • Removed the call to validateStrictBashTools() from the strict mode validation chain
  • Updated documentation comments to reflect that bash wildcards are now allowed in strict mode
  • Changed test cases from expecting errors to expecting success for bash wildcard usage

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
pkg/workflow/strict_mode.go Removed the call to validateStrictBashTools() and updated documentation to remove bash wildcard validation from the list of strict mode checks
pkg/workflow/validation_strict_mode.go Added deprecation notice to validateStrictBashTools() function and updated documentation comments
pkg/workflow/strict_mode_test.go Updated test cases to expect bash wildcards to be allowed in strict mode rather than rejected
Comments suppressed due to low confidence (1)

pkg/workflow/strict_mode.go:13

  • Documentation comment is outdated. This line mentions 'Bash wildcard tool usage' as a constraint enforced by strict mode, but bash wildcards are now allowed. This line should be removed from the list.
//   - Bash wildcard tool usage

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

🔍 Nitpick Review - Clean Code!

I've reviewed PR #3407 and found no nitpicks to report. This is exceptionally clean code! 🎉

What I Reviewed

  • Files Changed: 3 files (+11, -17 lines)
  • Focus Areas: Code structure, documentation, naming conventions, and test consistency

✅ Positive Highlights

Your code demonstrates excellent practices:

  1. Clear Documentation: The deprecation notice on validateStrictBashTools() is exemplary - it explains why the function is kept, what changed, and provides historical context.

  2. Consistent Updates: All three test cases were updated uniformly:

    • Test names changed from "refused" to "allowed"
    • Error expectations properly removed
    • Error messages cleaned up
  3. Clean Refactoring: The removal follows the principle of least surprise:

    • Function preserved for historical reference
    • Documentation comments updated in all locations
    • No orphaned code or comments left behind
  4. Well-Structured Comments: The package-level documentation in both strict_mode.go and validation_strict_mode.go was updated to reflect the new validation flow (3 validators instead of 4).

📋 Review Summary

Category Issues Found Notes
Naming & Conventions 0 All names clear and consistent
Code Structure 0 Clean, focused changes
Comments & Documentation 0 Excellent deprecation notice
Best Practices 0 Follows Go conventions
Testing 0 All tests updated properly

This PR is a textbook example of how to deprecate functionality while maintaining code clarity and historical context. No changes needed! ✨


Reviewed: 2025-11-07
Total Nitpicks: 0

AI generated by PR Nitpick Reviewer 🔍 for #3407

@pelikhan
Copy link
Contributor

pelikhan commented Nov 7, 2025

@copilot merge main and not

@pelikhan
Copy link
Contributor

pelikhan commented Nov 7, 2025

@copilot lint

Copy link
Contributor Author

Copilot AI commented Nov 7, 2025

@copilot merge main and not

Merged main branch successfully in commit 75d2915. All tests passing and build successful.

Copilot finished work on behalf of pelikhan November 7, 2025 04:38
Copilot AI requested a review from pelikhan November 7, 2025 04:38
The function was deprecated but unused, causing linting errors.
Removed completely to fix linting issues.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 7, 2025

@copilot lint

Linting passes successfully. Removed the unused validateStrictBashTools function that was causing linting errors in commit c4300f9.

Copilot finished work on behalf of pelikhan November 7, 2025 04:46
@pelikhan pelikhan merged commit 640ed30 into main Nov 7, 2025
30 checks passed
@pelikhan pelikhan deleted the copilot/remove-bash-restriction branch November 7, 2025 04:49
github-actions bot added a commit that referenced this pull request Nov 7, 2025
Update documentation to reflect recent feature changes and improvements.

Changes made:
- Remove outdated bash wildcard restriction in strict mode (PR #3407)
- Update CLI documentation for improved workflow discovery (PR #3402, #3406)
- Document wildcard workflow addition with better error handling (PR #3404)
- Update security guide to use neutral language about wildcards

Referenced PRs:
- #3407 - Remove bash wildcard restriction in strict mode
- #3406 - Display workflow list as table with ID, Name, and Description
- #3402 - Show available workflows when workflow not found in add command
- #3404 - Emit warning instead of error when wildcard adds encounter existing workflows

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
pelikhan pushed a commit that referenced this pull request Nov 7, 2025
Update documentation to reflect recent feature changes and improvements.

Changes made:
- Remove outdated bash wildcard restriction in strict mode (PR #3407)
- Update CLI documentation for improved workflow discovery (PR #3402, #3406)
- Document wildcard workflow addition with better error handling (PR #3404)
- Update security guide to use neutral language about wildcards

Referenced PRs:
- #3407 - Remove bash wildcard restriction in strict mode
- #3406 - Display workflow list as table with ID, Name, and Description
- #3402 - Show available workflows when workflow not found in add command
- #3404 - Emit warning instead of error when wildcard adds encounter existing workflows

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
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.

2 participants