Skip to content

Revamp #280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shunkakinoki merged 10 commits into from
Oct 11, 2025
Merged

Revamp #280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
54697bb
feat(dock): update persistent apps in dock configuration
shunkakinoki Oct 8, 2025
e9181ca
feat(package.json): add trustedDependencies section for clarity
shunkakinoki Oct 8, 2025
c29ad38
style(package.json): format trustedDependencies for consistency
shunkakinoki Oct 8, 2025
f22dd9b
feat(bun.lock): add trustedDependencies section for package clarity
shunkakinoki Oct 8, 2025
db091a0
style(package.json): reorder trustedDependencies for improved clarity
shunkakinoki Oct 8, 2025
cb76b6c
feat(default.nix): add postinstall script trust for global npm packages
shunkakinoki Oct 8, 2025
118694b
feat(default.nix): trust postinstall scripts for packages in trustedD…
shunkakinoki Oct 8, 2025
dadb501
fix(yek): make activation script non-fatal on installation failure
shunkakinoki Oct 11, 2025
0b427b9
feat(default.nix, homebrew.nix): add claude-code to package lists for…
shunkakinoki Oct 11, 2025
fd04c92
feat(homebrew.nix): move claude-code from brews to casks for better o…
shunkakinoki Oct 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion config/claude/settings.local.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"hooks": [
{
"type": "command",
"command": "cat | jq -r '\"display notification \\\"\" + .message + \"\\\" with title \\\"\" + .title + \"\\\" sound name \\\"Sonar\\\"\"' | xargs -I {} osascript -e '{}'"
"command": "jq -r '.message' | xargs -I {} osascript -e 'display notification \"{}\" with title \"Claude Code\" sound name \"Sonar\"'"

@gemini-code-assist gemini-code-assist Bot Oct 8, 2025

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.

medium

The updated command hardcodes the notification title to "Claude Code", which removes the dynamic title previously generated from the .title field. While this aligns with the Stop hook's behavior, it could be an unintended loss of functionality for this specific notification. If retaining the dynamic title is preferred, the command can be improved for readability while keeping the feature. The suggestion below uses jq's string interpolation, which is cleaner than the original string concatenation, and it also removes the unnecessary cat command.

Suggested change
"command": "jq -r '.message' | xargs -I {} osascript -e 'display notification \"{}\" with title \"Claude Code\" sound name \"Sonar\"'"
"command": "jq -r 'display notification \"\\(.message)\" with title \"\\(.title)\" sound name \"Sonar\"' | xargs -I {} osascript -e '{}'"

@coderabbitai coderabbitai Bot Oct 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Shell injection vulnerability in notification command.

The current command directly interpolates the message content into the osascript command string without proper escaping. Messages containing double quotes, backticks, dollar signs, or other shell metacharacters will break the command or potentially execute arbitrary code.

Example problematic inputs:

  • Message with quotes: Task "completed" → breaks AppleScript syntax
  • Message with command substitution: Done $(date) → executes date command

Apply this diff to fix the injection vulnerability using a safer approach:

-            "command": "jq -r '.message' | xargs -I {} osascript -e 'display notification \"{}\" with title \"Claude Code\" sound name \"Sonar\"'"
+            "command": "jq -r '.message' | xargs -0 -I {} osascript -e 'on run argv' -e 'display notification (item 1 of argv) with title \"Claude Code\" sound name \"Sonar\"' -e 'end run' -- {}"

Alternatively, use a more robust solution that avoids shell interpolation entirely:

-            "command": "jq -r '.message' | xargs -I {} osascript -e 'display notification \"{}\" with title \"Claude Code\" sound name \"Sonar\"'"
+            "command": "MESSAGE=$(jq -r '.message') && osascript - <<EOF\ndisplay notification \"$MESSAGE\" with title \"Claude Code\" sound name \"Sonar\"\nEOF"

Note: The same vulnerability pattern appears in line 20 for the Stop hook, though it uses jq to construct the message. Consider applying similar fixes there if the constructed message could contain untrusted input.

Committable suggestion skipped: line range outside the PR's diff.

@cubic-dev-ai cubic-dev-ai Bot Oct 8, 2025 •
edited
Loading

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.

Interpolating the message directly into the AppleScript source can break quoting and enable AppleScript injection. Pass the message as an argument to osascript (via argv) instead of embedding it in the -e string.

Prompt for AI agents 
Address the following comment on config/claude/settings.local.json at line 9:

<comment>Interpolating the message directly into the AppleScript source can break quoting and enable AppleScript injection. Pass the message as an argument to osascript (via argv) instead of embedding it in the -e string.</comment>

<file context>
@@ -6,7 +6,7 @@
           {
             &quot;type&quot;: &quot;command&quot;,
-            &quot;command&quot;: &quot;cat | jq -r &#39;\&quot;display notification \\\&quot;\&quot; + .message + \&quot;\\\&quot; with title \\\&quot;\&quot; + .title + \&quot;\\\&quot; sound name \\\&quot;Sonar\\\&quot;\&quot;&#39; | xargs -I {} osascript -e &#39;{}&#39;&quot;
+            &quot;command&quot;: &quot;jq -r &#39;.message&#39; | xargs -I {} osascript -e &#39;display notification \&quot;{}\&quot; with title \&quot;Claude Code\&quot; sound name \&quot;Sonar\&quot;&#39;&quot;
           }
         ]
</file context>
Fix with Cubic

}
Comment on lines 8 to 10

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Quote entire AppleScript command before piping to osascript

The new notification hook now runs jq -r '.message' | xargs -I {} osascript -e 'display notification \"{}\" with title \"Claude Code\" sound name \"Sonar\"'. Because the script passed to osascript is not wrapped as a single argument, xargs splits it into multiple tokens (display, notification, etc.) and osascript -e receives only display, which produces a syntax error and the notification never fires. Previously the hook constructed a single quoted AppleScript string, so notifications worked. Any Claude notification will now fail on macOS until the command is re-quoted so the full script is supplied as one argument.

Useful? React with 👍 / 👎.

]
}
Expand Down
1 change: 1 addition & 0 deletions home-manager/modules/ghostty/config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#

auto-update-channel = tip
background-opacity = 0.8
window-decoration = true
window-theme = "dark"
window-save-state = always
Expand Down
10 changes: 10 additions & 0 deletions home-manager/modules/npm-globals/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@
if [ -f "$PACKAGE_JSON" ]; then
echo "Installing npm global packages from package.json using bun..."
cd "${config.home.homeDirectory}/dotfiles"

# Trust postinstall scripts for packages listed in trustedDependencies before installing
TRUSTED_DEPS=$(${pkgs.jq}/bin/jq -r '.trustedDependencies[]?' "$PACKAGE_JSON" 2>/dev/null)
if [ -n "$TRUSTED_DEPS" ]; then
echo "Trusting postinstall scripts for: $TRUSTED_DEPS"
echo "$TRUSTED_DEPS" | while read -r dep; do
${pkgs.bun}/bin/bun pm -g trust "$dep" 2>/dev/null || true
done
fi
Comment on lines +19 to +26

@coderabbitai coderabbitai Bot Oct 11, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider improving error visibility for the trust operation.

The trust mechanism silently suppresses all errors (2>/dev/null || true), which could hide genuine issues such as:

  • Invalid dependency names
  • Network failures when trusting
  • Permission issues
  • Malformed package.json entries

While silent failure prevents the activation script from breaking, it provides no feedback when something goes wrong.

Consider this refactor to log failures while still allowing the script to continue:

       # Trust postinstall scripts for packages listed in trustedDependencies before installing
       TRUSTED_DEPS=$(${pkgs.jq}/bin/jq -r '.trustedDependencies[]?' "$PACKAGE_JSON" 2>/dev/null)
       if [ -n "$TRUSTED_DEPS" ]; then
         echo "Trusting postinstall scripts for: $TRUSTED_DEPS"
         echo "$TRUSTED_DEPS" | while read -r dep; do
-          ${pkgs.bun}/bin/bun pm -g trust "$dep" 2>/dev/null || true
+          if ! ${pkgs.bun}/bin/bun pm -g trust "$dep" 2>&1; then
+            echo "Warning: Failed to trust $dep" >&2
+          fi
         done
       fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Trust postinstall scripts for packages listed in trustedDependencies before installing
TRUSTED_DEPS=$(${pkgs.jq}/bin/jq -r '.trustedDependencies[]?' "$PACKAGE_JSON" 2>/dev/null)
if [ -n "$TRUSTED_DEPS" ]; then
echo "Trusting postinstall scripts for: $TRUSTED_DEPS"
echo "$TRUSTED_DEPS" | while read -r dep; do
${pkgs.bun}/bin/bun pm -g trust "$dep" 2>/dev/null || true
done
fi
# Trust postinstall scripts for packages listed in trustedDependencies before installing
TRUSTED_DEPS=$(${pkgs.jq}/bin/jq -r '.trustedDependencies[]?' "$PACKAGE_JSON" 2>/dev/null)
if [ -n "$TRUSTED_DEPS" ]; then
echo "Trusting postinstall scripts for: $TRUSTED_DEPS"
echo "$TRUSTED_DEPS" | while read -r dep; do
if ! ${pkgs.bun}/bin/bun pm -g trust "$dep" 2>&1; then
echo "Warning: Failed to trust $dep" >&2
fi
done
fi
🤖 Prompt for AI Agents 
In home-manager/modules/npm-globals/default.nix around lines 19 to 26, the bun
pm trust call suppresses all errors (2>/dev/null || true) which hides useful
failure details; change it to capture the command's stderr and exit status, and
if it fails print a warning including the dependency name and the captured error
output while still continuing (do not exit); ensure the message is sent to
stderr or logged so operators can see which dependency failed and why, but keep
the loop behavior that does not abort activation.


${pkgs.bun}/bin/bun install --global \
$(${pkgs.jq}/bin/jq -r '.dependencies | keys[]' "$PACKAGE_JSON") 2>/dev/null || true
fi
Expand Down
2 changes: 1 addition & 1 deletion home-manager/modules/yek/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ in
# Install yek on activation
home.activation.installYek = lib.hm.dag.entryAfter [ "writeBoundary" ] ''
if [ ! -f "$HOME/.local/bin/yek" ]; then
$DRY_RUN_CMD ${installScript}/bin/install-yek
$DRY_RUN_CMD ${installScript}/bin/install-yek || echo "⚠️ Failed to install yek. You can install it later by running: install-yek"
fi
'';
};
Expand Down
3 changes: 2 additions & 1 deletion home-manager/packages/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ with pkgs;
ast-grep
bat
bun
claude-code
cloudflared
curl
curlie
Expand Down Expand Up @@ -56,11 +55,13 @@ with pkgs;
uv
wget
yarn
yazi
yq
zellij
zoxide
]
++ lib.optionals stdenv.isLinux [
claude-code
codex
docker
docker-compose
Expand Down
2 changes: 2 additions & 0 deletions nix-darwin/config/dock.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
persistent-apps = [
"/System/Applications/Reminders.app"
"/System/Applications/Notes.app"
"/System/Applications/Mail.app"
"/Applications/ChatGPT.app"
"/Applications/Claude.app"
"/Applications/Google Chrome.app"
"/Applications/Wispr Flow.app"
"/Applications/Docker.app"
"/Applications/Docker.app/Contents/MacOS/Docker Desktop.app"
"/Applications/LM Studio.app"
Expand Down
1 change: 1 addition & 0 deletions nix-darwin/config/homebrew.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"block-goose"
"chatgpt"
"claude"
"claude-code"
"conductor"
"copilot-money"
"cursor"
Expand Down
7 changes: 6 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,10 @@
"@github/copilot": "^0.0.335",
"@google/jules": "^0.1.30",
"open-composer": "^0.8.14"
}
},
"trustedDependencies": [
"@github/copilot",
"@google/jules",
"open-composer"
]
}