Add hook safety controls for structkit - #190
Merged
Merged
Conversation
Implement comprehensive hook safety mechanisms to prevent arbitrary shell command execution when structures are generated via MCP or CI/CD. Changes: - Add --no-hooks flag and STRUCTKIT_NO_HOOKS env var to disable all hooks - Add --hooks-allowlist flag and STRUCTKIT_HOOKS_ALLOWLIST env var for command allowlisting - Implement interactive confirmation prompts before running hooks (skipped with --non-interactive) - Auto-detect .struct-hooks-allowlist in current directory - Update MCP generate_structure to skip hooks by default (no_hooks=true) - Add comprehensive test coverage for all safety features - Update documentation (hooks.md, mcp-integration.md) with safety guidance Fixes #100 Co-authored-by: Kenneth Belitzky <kenny@belitzky.com>
Co-authored-by: Kenneth Belitzky <kenny@belitzky.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Fixes #100
Description
This PR implements comprehensive hook safety mechanisms to prevent arbitrary shell command execution when structures are generated via MCP or CI/CD pipelines.
Key Features
1.
--no-hooksFlagSTRUCTKIT_NO_HOOKSenvironment variable2. Interactive Confirmation Prompts
--non-interactivemode3. Allowlist Support
.struct-hooks-allowlistfile to restrict which commands can run#) and empty lines are ignored--hooks-allowlistflag orSTRUCTKIT_HOOKS_ALLOWLISTenv var.struct-hooks-allowlistin current directory4. MCP Safety
generate_structurecalls now skip hooks by default (no_hooks=true)no_hooks=false(not recommended)Implementation Details
Modified Files:
structkit/commands/generate.py: Added safety flags, allowlist parsing, confirmation promptsstructkit/mcp_server.py: Addedno_hooksparameter (defaults totrue)docs/hooks.md: Comprehensive documentation of all safety featuresdocs/mcp-integration.md: Updated to documentno_hooksparameter and safety practicestests/test_hooks.py: Extensive test coverage for all new featurestests/test_commands.py: Updated test to handle new flagstests/test_commands_more.py: Fixed test to use--non-interactiveExamples:
Skip hooks in CI/CD:
Use allowlist for controlled execution:
Interactive confirmation:
Why This Makes MCP/CI Safer
MCP Integration: Hooks are disabled by default when structures are generated through MCP, preventing AI tools from unknowingly executing arbitrary shell commands embedded in structure YAML files.
CI/CD Pipelines: The
--no-hooksflag allows safe execution in automated environments where hooks shouldn't run or need explicit allowlisting.Interactive Safety: When running locally, users are prompted before hooks execute, providing visibility into what will run.
Allowlist Control: Teams can create allowlists for production environments to ensure only approved commands execute.
Checklist
Additional Comments
This implementation follows the requirements from issue #100 while ensuring backward compatibility. Existing users won't see any breaking changes:
--non-interactive)The change is reviewable and focused on security without introducing unrelated refactors.