Skip to content

fix(cursor): manage hooks declaratively, fix GUI PATH timeout - #1839

Merged
shunkakinoki merged 4 commits into
mainfrom
claude/jovial-murdock-7b622f
May 23, 2026
Merged

fix(cursor): manage hooks declaratively, fix GUI PATH timeout#1839
shunkakinoki merged 4 commits into
mainfrom
claude/jovial-murdock-7b622f

Conversation

@shunkakinoki

@shunkakinoki shunkakinoki commented May 23, 2026

Copy link
Copy Markdown
Owner

Summary

  • Restructured config/cursor/ to mirror the config/codex/ pattern: a hooks/ subdirectory with scripts deployed via home.file, and an activation script that copies hooks.json into ~/.cursor/.
  • Fixed the root cause of Cursor hook timeouts: the previous hooks.json referenced bare git-ai, but Cursor on macOS launches GUI apps with a minimal PATH that excludes ~/.cargo/bin, ~/.nix-profile/bin, /opt/homebrew/bin, etc. The binary wasn't found, so Cursor reported the hook as failing/hung.
  • Added cursor-flavored security.sh, notify.sh, pushover.sh, and a with-env.sh PATH-bootstrap wrapper for invoking the shared hooks.

Event wiring

Cursor event Hooks
beforeShellExecution security.sh, block-git-push.sh (via with-env.sh), block-gh-settings.sh (via with-env.sh), notify.sh, pushover.sh
afterFileEdit git-ai checkpoint cursor (absolute path)
beforeSubmitPrompt git-ai checkpoint cursor (absolute path)
stop notify.sh, pushover.sh

Why this fixes the hang

Every command now uses absolute $HOME/... paths or routes through with-env.sh, which exports a full PATH (~/.cargo/bin, /etc/profiles/per-user/..., /opt/homebrew/bin, ...) so Cursor's minimal GUI env can resolve git-ai, jq, gitleaks, and git.

Verified under env -i PATH=/usr/bin:/bin:

  • Safe inputs exit 0
  • Deny inputs (e.g. sudo ls) emit cursor's blocking JSON ({permission: "deny", user_message, agent_message}) and exit 2

What's intentionally NOT ported

  • secret-guard.sh — Cursor has no pre-write hook (afterFileEdit is non-blocking).
  • rtk-rewrite.sh — Cursor's blocking output schema has no "modified command" field.
  • dcg — Cursor's beforeShellExecution input schema isn't compatible with dcg's expected input.

Test plan

  • Restart Cursor (or reload window) and confirm hooks no longer time out.
  • Trigger a shell command in Cursor and verify notify.sh / pushover.sh fire.
  • Edit a file in Cursor and confirm git-ai checkpoint cursor runs.
  • Run home-manager switch (or darwin-rebuild switch) and confirm files match the manually-deployed copies in ~/.cursor/.

Summary by cubic

Manage Cursor hooks declaratively and fix macOS GUI PATH timeouts so hooks run reliably. Also consolidates rtk-rewrite.sh and security.sh into config/shared/hooks/ for Codex, Copilot, and Cursor, and switches git-ai calls to absolute paths.

  • Bug Fixes

    • Fixed Cursor hook timeouts with absolute $HOME/... commands and a PATH bootstrap for GUI launches.
    • Synced PATH bootstraps (including /usr/sbin) across shared security.sh, block-git-push.sh, and block-gh-settings.sh so scutil resolves in notifications and scripts work consistently from Cursor; stop handlers now run in the background.
    • Local notifications are only suppressed when pushover-notify is actually available.
    • Shared security.sh now fails closed if jq is missing and uses pure-bash trimming to avoid xargs quirks.
  • New Features

    • Consolidated rtk-rewrite.sh and security.sh into config/shared/hooks/; updated Claude/Codex/Copilot/Cursor to reference shared paths and removed per‑agent copies where applicable.
    • Managed Cursor hooks via home.file and wired beforeShellExecution, afterFileEdit, beforeSubmitPrompt, and stop; added config/cursor/hooks/notify.sh and pushover.sh for stop events and risky‑command alerts.

Written for commit f973a29. Summary will update on new commits. Review in cubic

Cursor on macOS launches with a minimal PATH that excludes ~/.cargo/bin,
~/.nix-profile/bin, /opt/homebrew/bin, etc. The previous hooks.json used
bare `git-ai`, so hooks failed with "command not found" and Cursor
reported timeouts.

- Restructure config/cursor/ to mirror config/codex/ (hooks/ subdir,
  scripts deployed via home.file)
- Add with-env.sh wrapper that bootstraps PATH for shared scripts
- Add cursor-flavored security.sh, notify.sh, pushover.sh
- Wire events: beforeShellExecution (security, block-git-push,
  block-gh-settings, notify, pushover), afterFileEdit + beforeSubmitPrompt
  (git-ai checkpoint, absolute path), stop (notify, pushover)
@indent-zero

indent-zero Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Brings Cursor's hook configuration to parity with Codex, consolidates duplicated agent-specific hooks into config/shared/hooks/, and addresses review feedback to harden the shared security guard and make Cursor's notification hooks non-blocking. The cumulative effect is one source of truth for security.sh/rtk-rewrite.sh, no more macOS GUI PATH timeouts, fail-closed semantics for the security hook, and asynchronous local/Pushover notifications.

  • Commit 1 (fix(cursor)): adds config/cursor/hooks/{security,notify,pushover,with-env}.sh deployed via home.file, wires beforeShellExecution (security + block-git-push/gh-settings via wrapper + notify + pushover), afterFileEdit/beforeSubmitPrompt (absolute $HOME/.cargo/bin/git-ai), stop (notify + pushover), and adds per-hook timeouts.
  • Commit 2 (refactor(hooks)): promotes security.sh and rtk-rewrite.sh to config/shared/hooks/, widens command extraction to cover Cursor's top-level .command, adds a tool_name case-guard, drops Cursor's JSON deny body (exit 2 alone), and deletes per-agent copies plus with-env.sh; shared scripts self-bootstrap PATH.
  • Commit 3 (fix(hooks)): security.sh now fails closed when jq is missing and uses pure-bash trim instead of xargs (so deny patterns with quotes/backslashes aren't reinterpreted); cursor's notify.sh / pushover.sh add /usr/sbin to PATH (so scutil resolves) and wrap their stop handlers in ( … ) </dev/null >/dev/null 2>&1 & disown (so the agent loop isn't held open by the 3s/6s timeouts); notify.sh only suppresses local popups when the pushover-notify binary is actually deliverable, not just env vars are set.
  • Configs across config/{claude,codex,copilot,cursor} reference the shared paths directly; sync-codex-security.sh / sync-rtk-rewrite.sh retargeted; specs reorganized (shared_security_spec.sh, rtk_rewrite_spec.sh, coverage_spec.sh) and per-agent copy assertions added.

Issues

1 potential issue found:

  • shared_security_spec.sh doesn't test the Cursor input shape ({"command": "rm -rf /*"} at top level, no tool_name) for the deny path — only an empty {} covers the new "" arm of the tool_name case. A one-line It block under blocked commands (e.g. Data '{"command":"rm -rf /*"}' expecting status 2 + BLOCKED on stderr) would prevent a future change to the case-statement from silently breaking Cursor's block path. → Autofix
4 issues already resolved
  • The hardcoded PATH in pushover.sh (and notify.sh, security.sh, with-env.sh) omits /usr/sbin, so scutil --get ComputerName always fails and silently falls back to hostname -s — Pushover messages will show Shuns-MacBook-Pro instead of the friendlier ComputerName. Add /usr/sbin to the PATH if you want the prettier hostname. (fixed by commit b416c74)
  • security.sh trims each command segment via echo "$segment" | xargs, which runs xargs's default echo invocation that interprets quotes/backslash escapes — quoted segments can be rewritten before deny-pattern matching. Not exploitable against today's literal-prefix deny list (and inherited from config/codex/hooks/security.sh), but expanding the list to patterns with embedded quotes would surface false negatives/positives; safer to use bash parameter expansion (segment="${segment#"${segment%%[![:space:]]*}"}" style) instead. (fixed by commit b416c74)
  • PATH bootstrap is now inlined in 5 hook scripts (config/shared/hooks/{block-gh-settings,block-git-push,security}.sh and config/cursor/hooks/{notify,pushover}.sh) after commit 2 deleted with-env.sh. A future PATH change (e.g. adding /usr/sbin for scutil) needs five identical edits; consider extracting the export to a small sourced helper (e.g. config/shared/hooks/path-bootstrap.sh) and source-ing it from each script. (fixed by commit f973a29)
  • stop hooks run synchronously (Cursor has no async field — only command, timeout, failClosed, matcher), so every agent session-end blocks for up to 6s on pushover.sh and 3s on notify.sh before Cursor reports completion. Codex's async: true does not transfer; consider backgrounding via nohup … & or ( … ) & inside the scripts if the latency is noticeable. (fixed by commit b416c74)

CI Checks

All CI checks passed on f973a296.


⚡ Autofix All Issues

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@mesa-dot-dev

mesa-dot-dev Bot commented May 23, 2026

Copy link
Copy Markdown

You do not have enough credits to review this pull request. Please purchase more credits to continue.

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR establishes a Cursor editor hook system that enforces pre-execution security checks, sets up environment variables for GUI app compatibility, and delivers work-status notifications through local and remote notification services.

Changes

Cursor Hook System with Security and Notifications

Layer / File(s) Summary
Command control hooks: environment and security
config/cursor/hooks/with-env.sh, config/cursor/hooks/security.sh
with-env.sh prepends a macOS-friendly PATH with common binary directories and executes the wrapped command. security.sh parses JSON hook events, tokenizes shell commands, and blocks execution (exit code 2) if any segment matches deny patterns such as rm -rf, sudo, chmod, docker system prune, or database destructive operations.
Notification delivery hooks
config/cursor/hooks/notify.sh, config/cursor/hooks/pushover.sh
notify.sh sends local desktop notifications via notify-local for work completion and risky commands, skipping if Pushover credentials are configured. pushover.sh sends remote Pushover notifications for the same events, including hostname and working directory context, and exits with differentiated codes (0 for completed/aborted, 1 for error).
Hook configuration and home-manager deployment
config/cursor/hooks.json, config/cursor/default.nix
hooks.json restructures hook entries to include explicit timeout values, adds multiple beforeShellExecution hooks (security, notify, pushover), and configures afterFileEdit, beforeSubmitPrompt, and stop hooks with timeouts. default.nix installs all four hook scripts into ~/.cursor/hooks/ as executable files via home.file with force overwrite.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Hooks are in place, security's tight,
Notifications flow left and right,
With PATH and deny lists set just so,
Cursor works safer, swift and slow!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(cursor): manage hooks declaratively, fix GUI PATH timeout' clearly and specifically describes the main changes: fixing Cursor hook timeouts and managing hooks declaratively via restructured config.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description clearly explains the changeset: restructuring config/cursor/ to manage hooks declaratively and fixing macOS GUI PATH timeouts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/jovial-murdock-7b622f

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/cursor/hooks/notify.sh`:
- Around line 20-22: The early return in notify.sh currently exits when
PUSHOVER_API_TOKEN and PUSHOVER_USER_KEY are set, which silences local
notifications even if the Pushover client is missing; change the conditional so
it also verifies the actual notifier exists (e.g., check for the pushover-notify
executable with command -v or which) and only exit when both env vars are
present AND the pushover-notify command is available, leaving local
notifications as a fallback otherwise.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 06da0fea-d06c-4394-9edf-37c162825b11

📥 Commits

Reviewing files that changed from the base of the PR and between 084667a and 9279a6e.

📒 Files selected for processing (6)
  • config/cursor/default.nix
  • config/cursor/hooks.json
  • config/cursor/hooks/notify.sh
  • config/cursor/hooks/pushover.sh
  • config/cursor/hooks/security.sh
  • config/cursor/hooks/with-env.sh

Comment thread config/cursor/hooks/notify.sh Outdated
Comment thread config/cursor/hooks.json
Comment thread config/cursor/hooks/pushover.sh
Comment thread config/cursor/hooks/security.sh

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread config/shared/hooks/security.sh Outdated
Three identical copies of rtk-rewrite.sh (claude/codex/copilot) and three
near-identical copies of security.sh (codex/copilot/cursor) collapsed into
one each under config/shared/hooks/. Claude's settings.json-driven
security.sh stays separate since it has a different runtime model.

- Move rtk-rewrite.sh to shared/hooks; delete agent-specific copies
- Move security.sh to shared/hooks; extend extraction to include cursor's
  top-level .command; add PATH bootstrap so cursor GUI launches work
- Add PATH bootstrap to shared block-git-push.sh and block-gh-settings.sh
  for the same reason; drop cursor's with-env.sh wrapper
- Update settings.json/hooks.json/config.json to reference shared paths
- Remove home.file declarations that deployed per-agent copies
- Update sync-rtk-rewrite.sh and sync-codex-security.sh to target shared
- Rename spec/codex_security_spec.sh to shared_security_spec.sh
- Delete spec/codex_rtk_rewrite_spec.sh (redundant with rtk_rewrite_spec)
- Update coverage_spec.sh entries
- security.sh: fail closed when jq is unavailable (was fail-open, a
  security antipattern — a hook that can't parse its input must block)
- security.sh: replace xargs trim with pure-bash trim so deny patterns
  containing quotes/backslashes aren't reinterpreted
- cursor notify/pushover: add /usr/sbin to PATH so scutil resolves and
  HOSTNAME reflects ComputerName instead of kebab-case hostname fallback
- cursor notify/pushover stop handler: background the work since Cursor
  doesn't honor async on hooks; previously held the agent loop open for
  up to ~6s after every session
- cursor notify: only suppress local notifications when pushover-notify
  binary is actually available, not just env vars set
Keeps the PATH string identical across all 5 hook scripts so future
divergence is easier to spot. None of the shared scripts call scutil
today, but the consistency matters more than the bytes saved.
@shunkakinoki
shunkakinoki merged commit c4dd2af into main May 23, 2026
27 of 31 checks passed
@shunkakinoki
shunkakinoki deleted the claude/jovial-murdock-7b622f branch May 23, 2026 06:34
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.

1 participant