Skip to content

fix: require execute permission before running reconcile.sh#41

Open
xiaolai wants to merge 1 commit intoagenticnotetaking:mainfrom
xiaolai:fix/nlpm-session-orient-reconcile-guard
Open

fix: require execute permission before running reconcile.sh#41
xiaolai wants to merge 1 commit intoagenticnotetaking:mainfrom
xiaolai:fix/nlpm-session-orient-reconcile-guard

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 (LOW)

File: hooks/scripts/session-orient.sh:137

The hook called a vault-owned script at a fixed path with only a file-existence check:

if [ -f ops/scripts/reconcile.sh ]; then
  bash ops/scripts/reconcile.sh --compact 2>/dev/null
fi

A non-executable file accidentally placed at ops/scripts/reconcile.sh — for example, a text file, a downloaded script that was never chmod +x'd, or a symlink pointing elsewhere — would still be executed by bash since bash does not require the executable bit when given a path directly.

Fix

Added a -x (executable) check alongside the existence check:

if [ -f ops/scripts/reconcile.sh ] && [ -x ops/scripts/reconcile.sh ]; then
  bash ops/scripts/reconcile.sh --compact 2>/dev/null
fi

This is a one-character, fully backward-compatible change. A correctly installed vault's reconcile.sh is already executable, so normal operation is unaffected. The check prevents accidental execution of a non-executable file placed at that path.

Files changed

  • hooks/scripts/session-orient.sh

The session-orient hook called ops/scripts/reconcile.sh with only an
existence check. Adding an executable-bit check (-x) ensures that a
non-executable file accidentally placed at that path cannot be run.
The change is one character and fully backward-compatible — a properly
installed vault's reconcile.sh will already be executable.

Co-Authored-By: Claude Code <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