Skip to content

[Docs]: Minor script and documentation improvements from security review #163

@sjnims

Description

@sjnims

Which documentation needs improvement?

Multiple files - Skill documentation and utility scripts

Specific Location

  1. plugins/plugin-dev/skills/hook-development/SKILL.md
  2. plugins/plugin-dev/skills/plugin-settings/examples/read-settings-hook.sh
  3. plugins/plugin-dev/skills/hook-development/scripts/test-hook.sh
  4. plugins/plugin-dev/skills/plugin-settings/scripts/parse-frontmatter.sh

What's unclear or missing?

A security review identified several minor improvements across the plugin. These are low-priority quality improvements that would enhance usability and defensive coding practices.


Item 1: Add chmod reminder for example scripts

File: plugins/plugin-dev/skills/hook-development/SKILL.md

Issue: The example hook scripts in examples/ are not executable by default. While hook-linter.sh warns about this, users copying examples directly encounter permission errors.

Suggestion: Add a note in the "Example Hook Scripts" section:

### Example Hook Scripts

Working examples in `examples/`:

> **Note:** After copying example scripts, make them executable: `chmod +x script.sh`

- **`validate-write.sh`** - File write validation example
...

Item 2: Use parameterized plugin name instead of hardcoded path

File: plugins/plugin-dev/skills/plugin-settings/examples/read-settings-hook.sh:7

Current:

SETTINGS_FILE=".claude/my-plugin.local.md"

Issue: This hardcodes my-plugin rather than demonstrating the portable pattern users should follow.

Suggestion:

# Allow plugin name to be configured, with sensible default
PLUGIN_NAME="${PLUGIN_NAME:-my-plugin}"
SETTINGS_FILE=".claude/${PLUGIN_NAME}.local.md"

This teaches users how to make their hooks configurable.


Item 3: Add timeout to jq validation

File: plugins/plugin-dev/skills/hook-development/scripts/test-hook.sh:68

Current:

if ! jq empty "$TEST_INPUT" 2>/dev/null; then

Issue: Unlike the hook execution which has a configurable timeout, the jq validation has no timeout. Extremely large or malformed files could cause jq to hang.

Suggestion:

if ! timeout 5 jq empty "$TEST_INPUT" 2>/dev/null; then

This maintains consistency with the defensive timeout patterns used elsewhere in the script.


Item 4: Document race condition behavior for settings files

File: plugins/plugin-dev/skills/plugin-settings/scripts/parse-frontmatter.sh or SKILL.md

Issue: The settings parsing scripts read files without locking. While this is appropriate for the use case (settings files are written once and read many times), it's worth documenting.

Suggestion: Add a note to the plugin-settings skill or the script itself:

> **Note:** Settings files should be fully written before hooks execute. Avoid 
> modifying settings files while Claude Code is running, as changes won't take 
> effect until the next session anyway.

Or as a comment in the script:

# Note: This script assumes the settings file is stable (not being written to).
# Settings changes require a Claude Code restart to take effect.

Type of issue

  • Incorrect information
  • Missing information
  • Unclear explanation
  • Broken link
  • Typo or grammar
  • Outdated content
  • Other

Additional Context

These findings came from a comprehensive security review of the plugin. All items are low priority and represent minor quality improvements rather than functional issues. They can be addressed individually or as a batch cleanup.

Priority order (suggested):

  1. Item 2 (hardcoded path) - Quick fix, improves example quality
  2. Item 1 (chmod reminder) - Documentation addition
  3. Item 3 (jq timeout) - Minor robustness improvement
  4. Item 4 (race condition docs) - Documentation clarification

Metadata

Metadata

Assignees

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions