Skip to content

fix: validate KEY against allowlist in read_config.sh#39

Open
xiaolai wants to merge 1 commit intoagenticnotetaking:mainfrom
xiaolai:fix/nlpm-read-config-key-sanitize
Open

fix: validate KEY against allowlist in read_config.sh#39
xiaolai wants to merge 1 commit intoagenticnotetaking:mainfrom
xiaolai:fix/nlpm-read-config-key-sanitize

Conversation

@xiaolai
Copy link
Copy Markdown

@xiaolai xiaolai commented Apr 21, 2026

Automated audit: This PR was generated by NLPM, a natural language programming linter, running via claude-code-action. Please evaluate the diff on its merits.

Security Fix (MEDIUM)

File: hooks/scripts/read_config.sh:30

The KEY argument was interpolated directly into a grep -E regex pattern without validation:

VALUE=$(grep -E "^${KEY}:" "$CONFIG_FILE" ...)

Special regex characters in KEY could cause unexpected matches (e.g., KEY=".+" would match every line). While the current callers (session-orient.sh and auto-commit.sh) always pass literal strings ("git", "session_capture"), any future caller passing user-influenced input could trigger unintended behaviour.

Fix

Added a case allowlist before the grep call. Only the two known valid keys (git, session_capture) pass through; anything else returns the default value immediately. This is a safe, backward-compatible hardening — existing behaviour is identical for all current callers.

case "$KEY" in
  git|session_capture) ;;
  *) echo "$DEFAULT"; exit 0 ;;
esac

The allowlist documents the intended API of the function and makes the safe path the only path. Adding new config keys in the future requires an explicit allowlist update, which is the right forcing function.

Files changed

  • hooks/scripts/read_config.sh

KEY was interpolated directly into grep -E as a regex pattern without
validation. Callers currently only pass literal strings ("git",
"session_capture"), but any future caller passing untrusted input could
trigger unexpected regex matches. An allowlist ensures only known keys
can be queried, making the safe path the only path.

Co-Authored-By: Claude Code <[email protected]>
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