Skip to content

fix(careful): BSD sed compatibility for safe exception detection on macOS#1242

Open
ToraDady wants to merge 1 commit intogarrytan:mainfrom
ToraDady:fix/bsd-sed-compatibility
Open

fix(careful): BSD sed compatibility for safe exception detection on macOS#1242
ToraDady wants to merge 1 commit intogarrytan:mainfrom
ToraDady:fix/bsd-sed-compatibility

Conversation

@ToraDady
Copy link
Copy Markdown

@ToraDady ToraDady commented Apr 27, 2026

Summary

careful/bin/check-careful.sh uses \s+ in its sed regex, which is a GNU sed extension not supported by BSD sed (the default on macOS). On macOS this causes the safe-exception detection to fail silently, so even safe commands like rm -rf node_modules trigger the destructive warning instead of being permitted as designed.

Replacing \s+ with the POSIX [[:space:]]+ makes the expression work correctly on both GNU sed (Linux) and BSD sed (macOS).

Reproduction (on macOS)

echo '{"tool_input":{"command":"rm -rf node_modules"}}' | bash careful/bin/check-careful.sh
# Expected: {} (safe exception)
# Actual:   {"permissionDecision":"ask",...} (warning fires, bug)

bash -x tracing confirmed that sed -E 's/.*rm\s+(-[a-zA-Z]+\s+)*//' does not strip anything on macOS BSD sed.

Fix

- RM_ARGS=$(printf '%s' "$CMD" | sed -E 's/.*rm\s+(-[a-zA-Z]+\s+)*//;s/--recursive\s*//')
+ RM_ARGS=$(printf '%s' "$CMD" | sed -E 's/.*rm[[:space:]]+(-[a-zA-Z]+[[:space:]]+)*//;s/--recursive[[:space:]]*//')

Note: the grep -qE line in the same file also uses \s+, but BSD grep on macOS does support \s (verified via bash -x trace), so only the sed expression needs the fix.

Test cleanup

test/hook-scripts.test.ts already documented this limitation with a detectSafeRmWorks() helper and a platform-conditional assertion (`if GNU sed: expect undefined, else: expect ask`). Now that the regex works on both platforms, this dead path is removed and the safe-exception tests assert the same expectation on every OS.

Net diff: 3 insertions(+), 21 deletions(-) across the two files.

Test plan

After applying the fix, verified on macOS:

  • `rm -rf node_modules` → `{}` (safe exception, fixed)
  • `rm -rf ./fake_dir/node_modules` → `{}` (matches `*/node_modules` pattern, fixed)
  • `rm -rf .next dist` → `{}` (multiple safe targets, fixed)
  • `rm -rf node_modules /var/data` → warning fires (mixed safe+unsafe still flagged correctly)
  • `rm -rf /etc/important` → warning fires (unchanged)
  • `git reset --hard HEAD~3` → warning fires (unchanged)
  • `ls -la` → `{}` (non-destructive, unchanged)

`bun test test/hook-scripts.test.ts` → 32 pass, 0 fail. Full `bun test` shows the same 6 pre-existing failures on `main` (all in `browse/test/server-auth.test.ts`, `integration smoke`, `gstack-gbrain-lib.sh`) — unrelated to this change.

Discovery

I found this while translating gstack's careful skill into Japanese for a derivative project (uzustack — https://github.com/uzumaki-inc/uzustack). During integration testing on macOS, the safe exception logic appeared broken, and `bash -x` tracing revealed the BSD sed compatibility issue.

Reference: same fix applied in uzustack at uzumaki-inc/uzustack@bc67c8d

…acOS

The sed regex in check-careful.sh uses \s+, which is a GNU sed
extension not supported by BSD sed (macOS default). On macOS, this
causes the RM_ARGS strip to fail silently, making rm -rf of safe
exceptions (node_modules, .next, dist, etc.) trigger the destructive
warning instead of being permitted as designed.

Fix: replace \s+ with POSIX [[:space:]]+, which works on both GNU sed
(Linux) and BSD sed (macOS).

The existing test/hook-scripts.test.ts already documented this
limitation via a detectSafeRmWorks() helper and a platform-conditional
assertion ("if GNU sed: expect undefined, else: expect ask"). Now that
the regex works on both platforms, this dead path is removed and the
safe-exception tests assert the same expectation on every OS.

Note: the grep regex in the same file also uses \s+, but BSD grep -E
on macOS does support \s (verified via bash -x trace), so only the
sed expression needs the fix.

Discovered while translating the careful skill for a Japanese
derivative project (uzustack). Reference:
uzumaki-inc/uzustack@bc67c8d
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