Add hook safety controls for structkit

docs/hooks.md

@@ -2,6 +2,15 @@
 
 You can define shell commands to run before and after structure generation using the `pre_hooks` and `post_hooks` keys in your YAML configuration. These are optional and allow you to automate setup or cleanup steps.
 
+## Safety Controls
+
+Hooks execute shell commands with `shell=True`, which can be powerful but also risky. StructKit provides several safety mechanisms:
+
+- **Interactive Confirmation**: When running interactively, StructKit prompts for confirmation before executing hooks
+- **Skip Hooks**: Use `--no-hooks` flag or `STRUCTKIT_NO_HOOKS=true` to disable all hooks
+- **Allowlist**: Create a `.struct-hooks-allowlist` file to restrict which commands can run
+- **MCP Safety**: MCP calls skip hooks by default (`no_hooks=true`)
+
 ## Hook Types
 
 - **pre_hooks**: List of shell commands to run before generation. If any command fails (non-zero exit), generation is aborted.
@@ -170,6 +179,94 @@ files:
         }
 ```
 
+## Safety Features
+
+### Disabling Hooks
+
+You can disable hooks entirely using the `--no-hooks` flag or environment variable:
+
+```bash
+# Using CLI flag
+structkit generate .struct.yaml --no-hooks
+
+# Using environment variable
+export STRUCTKIT_NO_HOOKS=true
+structkit generate .struct.yaml
+```
+
+This is recommended for:
+- CI/CD pipelines where hooks shouldn't run
+- MCP/automation contexts
+- Untrusted structure definitions
+
+### Interactive Confirmation
+
+By default, StructKit prompts for confirmation before running hooks in interactive mode:
+
+```bash
+$ structkit generate .struct.yaml
+
+⚠️  The following pre-hooks will be executed:
+  - echo "Preparing environment..."
+  - ./scripts/prep.sh
+
+Do you want to run these pre-hooks? [y/N]:
+```
+
+To skip the prompt:
+- Use `--non-interactive` flag
+- Set `STRUCTKIT_NON_INTERACTIVE=true`
+
+**Note**: Non-interactive mode without `--no-hooks` will execute hooks without confirmation.
+
+### Allowlist File
+
+Create a `.struct-hooks-allowlist` file in your project directory to restrict which commands can run:
+
+```text
+# .struct-hooks-allowlist
+# One command per line. Lines starting with # are comments.
+
+echo
+git
+npm
+python
+./scripts/prep.sh
+./scripts/cleanup.sh
+```
+
+When an allowlist exists:
+- Only commands in the allowlist can run
+- Both exact matches and base commands (first word) are checked
+- Blocked hooks cause generation to fail
+
+You can also specify a custom allowlist path:
+
+```bash
+structkit generate .struct.yaml --hooks-allowlist /path/to/allowlist.txt
+
+# Or via environment variable
+export STRUCTKIT_HOOKS_ALLOWLIST=/path/to/allowlist.txt
+structkit generate .struct.yaml
+```
+
+### MCP Integration Safety
+
+When using StructKit through MCP (Model Context Protocol), hooks are **disabled by default** for security:
+
+```json
+{
+  "name": "generate_structure",
+  "arguments": {
+    "structure_definition": "project/python",
+    "base_path": "/tmp/myproject",
+    "no_hooks": true  // Default for MCP calls
+  }
+}
+```
+
+To enable hooks in MCP calls (not recommended), explicitly set `no_hooks: false`.
+
 ## Best Practices
 
 1. **Keep hooks simple**: Use external scripts for complex operations
@@ -178,6 +275,8 @@ files:
 4. **Log important actions**: Use echo statements for user feedback
 5. **Test independently**: Ensure hook commands work outside StructKit
 6. **Consider dependencies**: Order hooks based on their requirements
+7. **Use allowlists**: For production environments, always use an allowlist
+8. **Disable in CI/CD**: Use `--no-hooks` in automated environments unless hooks are required and safe
 
 ## Error Handling
 
@@ -199,6 +298,37 @@ post_hooks:
   - echo "Setup complete (some warnings may have occurred)"
 ```
 
+### Safe Hook Example with Allowlist
+
+```yaml
+# .struct.yaml
+pre_hooks:
+  - echo "Preparing environment..."
+  - python -c "import sys; print(sys.version)"
+
+post_hooks:
+  - echo "Generation complete!"
+  - git --version
+
+files:
+  - README.md:
+      content: |
+        # My Project
+```
+
+```text
+# .struct-hooks-allowlist
+echo
+python
+git
+```
+
+With this setup:
+- Only `echo`, `python`, and `git` commands can run
+- Interactive users will be prompted for confirmation
+- Use `--no-hooks` to skip entirely
+- Use `--non-interactive` to run without prompts (requires allowlist or trust)
+
 ## Variables in Hooks
 
 You can use template variables in hook commands:

docs/mcp-integration.md

@@ -70,6 +70,7 @@ Generate a project structure using specified definition and options.
 - `mappings` (optional): Variable mappings for template substitution
 - `structures_path` (optional): Custom path to structure definitions
 - `source` (optional): Named source configured with `manage_sources`. The structure definition can also use a `<source>/<structure>` prefix.
+- `no_hooks` (optional): Skip all pre/post hooks for safety (default: **true** for MCP calls)
 
 ### 4. get_structure_vars
 Inspect variables declared by a specific structure without generating files.
@@ -296,6 +297,36 @@ if __name__ == "__main__":
 
 The MCP integration is particularly powerful for AI-assisted development workflows:
 
+### Hook Safety in MCP
+
+**Important**: For security, MCP calls to `generate_structure` skip hooks by default (`no_hooks: true`). This prevents arbitrary shell command execution when structures are generated via automation or AI tools.
+
+```json
+{
+  "name": "generate_structure",
+  "arguments": {
+    "structure_definition": "project/python",
+    "base_path": "/tmp/myproject",
+    "no_hooks": true  // Default for MCP - hooks are skipped
+  }
+}
+```
+
+To enable hooks in MCP calls (not recommended unless you trust the structure source):
+
+```json
+{
+  "name": "generate_structure",
+  "arguments": {
+    "structure_definition": "project/python",
+    "base_path": "/tmp/myproject",
+    "no_hooks": false  // Explicitly enable hooks (use with caution)
+  }
+}
+```
+
+See the [Hooks documentation](hooks.md) for more information about hook safety controls.
+
 ### Console Output Mode
 Using `output: "console"` with `generate_structure` allows piping structure content to stdout for subsequent AI prompts:
 
@@ -434,7 +465,8 @@ without writing files:
     "dry_run": true,
     "mappings": {
       "module_name": "network-observability"
-    }
+    },
+    "no_hooks": true
   }
 }
 ```
@@ -465,7 +497,8 @@ After approval, call the same structure with `output: "files"` and
     "dry_run": false,
     "mappings": {
       "module_name": "network-observability"
-    }
+    },
+    "no_hooks": true
   }
 }
 ```

structkit/commands/generate.py

@@ -50,6 +50,8 @@ def __init__(self, parser):
                         help='Path to a YAML file containing mappings to be used in templates (can be specified multiple times)')
     parser.add_argument('-o', '--output', type=str,
                         choices=['console', 'file'], default=os.getenv('STRUCTKIT_OUTPUT_MODE', 'file'), help='Output mode (env: STRUCTKIT_OUTPUT_MODE)')
+    parser.add_argument('--no-hooks', action='store_true', help='Skip all pre/post hooks (env: STRUCTKIT_NO_HOOKS)', default=os.getenv('STRUCTKIT_NO_HOOKS', '').lower() in ('true', '1', 'yes'))
+    parser.add_argument('--hooks-allowlist', type=str, help='Path to hooks allowlist file (env: STRUCTKIT_HOOKS_ALLOWLIST)', default=os.getenv('STRUCTKIT_HOOKS_ALLOWLIST', None))
     parser.set_defaults(func=self.execute)
 
   def _parse_template_vars(self, vars_str):
@@ -91,9 +93,108 @@ def _deep_merge_dicts(self, dict1, dict2):
         result[key] = value
     return result
 
-  def _run_hooks(self, hooks, hook_type="pre"):  # helper for running hooks
+  def _load_hooks_allowlist(self, allowlist_path):
+    """Load the hooks allowlist from a file.
+    Returns a set of allowed commands or None if file doesn't exist.
+    """
+    if not allowlist_path:
+      return None
+
+    allowlist_file = None
+    if os.path.isabs(allowlist_path):
+      allowlist_file = allowlist_path
+    else:
+      allowlist_file = os.path.join(os.getcwd(), allowlist_path)
+
+    if not os.path.exists(allowlist_file):
+      # Also check for .struct-hooks-allowlist in current directory if no explicit path given
+      default_allowlist = os.path.join(os.getcwd(), '.struct-hooks-allowlist')
+      if allowlist_path == default_allowlist and not os.path.exists(default_allowlist):
+        return None
+      self.logger.warning(f"Hooks allowlist file not found: {allowlist_file}")
+      return None
+
+    try:
+      with open(allowlist_file, 'r') as f:
+        lines = f.readlines()
+      # Parse allowlist: ignore empty lines and comments (lines starting with #)
+      allowlist = set()
+      for line in lines:
+        line = line.strip()
+        if line and not line.startswith('#'):
+          allowlist.add(line)
+      return allowlist
+    except Exception as e:
+      self.logger.error(f"Failed to read hooks allowlist: {e}")
+      return None
+
+  def _check_hook_allowed(self, cmd, allowlist):
+    """Check if a command is allowed by the allowlist.
+    If allowlist is None, all commands are allowed.
+    If allowlist is a set, only commands in the set are allowed.
+    """
+    if allowlist is None:
+      return True
+
+    # Check exact match first
+    if cmd in allowlist:
+      return True
+
+    # Check if the base command (first word) is allowed
+    base_cmd = cmd.split()[0] if cmd.split() else cmd
+    if base_cmd in allowlist:
+      return True
+
+    return False
+
+  def _confirm_hooks(self, hooks, hook_type="pre"):
+    """Ask user to confirm hook execution.
+    Returns True if user confirms, False otherwise.
+    """
     if not hooks:
       return True
+
+    print(f"\n⚠️  The following {hook_type}-hooks will be executed:")
+    for cmd in hooks:
+      print(f"  - {cmd}")
+
+    response = input(f"\nDo you want to run these {hook_type}-hooks? [y/N]: ").strip().lower()
+    return response in ('y', 'yes')
+
+  def _run_hooks(self, hooks, hook_type="pre", skip_hooks=False, non_interactive=False, allowlist=None):
+    """Run pre/post hooks with safety controls.
+
+    Args:
+      hooks: List of shell commands to run
+      hook_type: Type of hooks ("pre" or "post")
+      skip_hooks: If True, skip all hooks
+      non_interactive: If True, skip confirmation prompt
+      allowlist: Set of allowed commands or None to allow all
+
+    Returns:
+      True if all hooks succeeded or were skipped, False if any failed
+    """
+    if not hooks or skip_hooks:
+      if skip_hooks and hooks:
+        self.logger.info(f"Skipping {hook_type}-hooks (--no-hooks enabled)")
+      return True
+
+    # Check if any hooks are blocked by allowlist
+    if allowlist is not None:
+      blocked_hooks = [cmd for cmd in hooks if not self._check_hook_allowed(cmd, allowlist)]
+      if blocked_hooks:
+        self.logger.error(f"The following {hook_type}-hooks are not in the allowlist:")
+        for cmd in blocked_hooks:
+          self.logger.error(f"  - {cmd}")
+        self.logger.error("Hook execution blocked. Update allowlist or use --no-hooks to skip.")
+        return False
+
+    # Ask for confirmation in interactive mode
+    if not non_interactive:
+      if not self._confirm_hooks(hooks, hook_type):
+        self.logger.info(f"User declined to run {hook_type}-hooks. Aborting.")
+        return False
+
     for cmd in hooks:
       self.logger.info(f"Running {hook_type}-hook: {cmd}")
       try:
@@ -201,8 +302,23 @@ def execute(self, args):
     pre_hooks = config.get('pre_hooks', [])
     post_hooks = config.get('post_hooks', [])
 
+    skip_hooks = getattr(args, 'no_hooks', False)
+    non_interactive = getattr(args, 'non_interactive', False)
+
+    # Load hooks allowlist if provided (only if hooks are enabled)
+    allowlist = None
+    if not skip_hooks:
+      allowlist_path = getattr(args, 'hooks_allowlist', None)
+      if not allowlist_path:
+        # Check for default .struct-hooks-allowlist in current directory
+        default_allowlist = os.path.join(os.getcwd(), '.struct-hooks-allowlist')
+        if os.path.exists(default_allowlist):
+          allowlist_path = default_allowlist
+
+      allowlist = self._load_hooks_allowlist(allowlist_path) if allowlist_path else None
+
     # Run pre-hooks
-    if not self._run_hooks(pre_hooks, hook_type="pre"):
+    if not self._run_hooks(pre_hooks, hook_type="pre", skip_hooks=skip_hooks, non_interactive=non_interactive, allowlist=allowlist):
       self.logger.error("Aborting generation due to pre-hook failure.")
       return
 
@@ -214,7 +330,7 @@ def execute(self, args):
       raise SystemExit(1) from None
 
     # Run post-hooks
-    if not self._run_hooks(post_hooks, hook_type="post"):
+    if not self._run_hooks(post_hooks, hook_type="post", skip_hooks=skip_hooks, non_interactive=non_interactive, allowlist=allowlist):
       self.logger.error("Post-hook failed.")
       return