kyber - #408
Conversation
Mesa DescriptionTL;DRSwitched Kyber SSH to Tailscale, locked down management access, added a Bash security hook for Claude Code, introduced ShellSpec/ShellCheck in CI, and updated Nix installation to Determinate Nix. What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent pull request that brings substantial improvements to security, maintainability, and testing. The switch to Tailscale for SSH access is a major security win. Hardening the cliproxyapi and adding the security.sh hook to block dangerous commands are great security enhancements. The introduction of ShellSpec and ShellCheck, along with their integration into the CI workflow, will significantly improve the quality and reliability of the shell scripts. My review includes a few suggestions for improving script robustness and efficiency.
| .PHONY: shell-check | ||
| shell-check: ## Run ShellCheck on shell scripts. | ||
| @echo "🔍 Running ShellCheck..." | ||
| @find . -name '*.sh' -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' | xargs shellcheck |
There was a problem hiding this comment.
Using find ... | xargs can be problematic if filenames contain spaces or special characters. While less likely for shell scripts, it's more robust to use find -exec which handles all filenames correctly.
@find . -name '*.sh' -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' -exec shellcheck {} +
| input=$(cat) | ||
|
|
||
| # Extract tool name - only process Bash commands | ||
| tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null) | ||
| [[ "$tool_name" != "Bash" ]] && exit 0 | ||
|
|
||
| # Extract the command to be executed | ||
| command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null) | ||
| [[ -z "$command" ]] && exit 0 |
There was a problem hiding this comment.
This section reads the entire standard input into a variable and then invokes jq twice on it. This is inefficient as it parses the same JSON multiple times. You can optimize this by parsing the required fields in a single pass directly from stdin.
| input=$(cat) | |
| # Extract tool name - only process Bash commands | |
| tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null) | |
| [[ "$tool_name" != "Bash" ]] && exit 0 | |
| # Extract the command to be executed | |
| command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null) | |
| [[ -z "$command" ]] && exit 0 | |
| mapfile -t fields < <(jq -r '[.tool.name // "", .tool.input.command // ""] | .[]' 2>/dev/null) | |
| tool_name="${fields[0]}" | |
| command="${fields[1]}" | |
| # Only process Bash commands with a non-empty command | |
| [[ "$tool_name" != "Bash" ]] && exit 0 | |
| [[ -z "$command" ]] && exit 0 |
|
|
||
| for segment in "${segments[@]}"; do | ||
| # Trim leading/trailing whitespace | ||
| segment=$(echo "$segment" | xargs 2>/dev/null) || continue |
There was a problem hiding this comment.
Using echo ... | xargs to trim whitespace is a classic trick, but it's not very efficient as it involves a subshell and an external command. A more modern and efficient Bash-native approach is to use read with a here-string, which avoids creating a subshell.
| segment=$(echo "$segment" | xargs 2>/dev/null) || continue | |
| read -r segment <<< "$segment" |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 26 files
Prompt for AI agents (all 5 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="install.sh">
<violation number="1" location="install.sh:45">
P2: Missing `--no-confirm` flag for macOS installation. The Docker installation includes this flag for non-interactive use, but the macOS installation (which the comment indicates runs in CI) doesn't. This will cause the installer to hang waiting for user input in automated environments.</violation>
<violation number="2" location="install.sh:67">
P2: Missing `--no-confirm` flag for Linux multi-user installation. The Docker installation includes this flag for non-interactive use, but the Linux multi-user installation doesn't. This will cause the installer to hang waiting for user input in automated environments.</violation>
</file>
<file name="home-manager/services/cliproxyapi/start.sh">
<violation number="1" location="home-manager/services/cliproxyapi/start.sh:24">
P2: Password variable may break sed if it contains special characters (`|`, `&`, `\`). Consider escaping the variable or using `envsubst` for safer template substitution.</violation>
</file>
<file name="config/claude/security.sh">
<violation number="1" location="config/claude/security.sh:57">
P1: Security bypass: Command substitution (`$(...)` and backticks) is not handled. A malicious command like `echo $(rm -rf /)` bypasses detection since the entire string is treated as one segment starting with `echo`, not `rm`. Consider parsing nested commands or documenting this limitation.</violation>
<violation number="2" location="config/claude/security.sh:61">
P2: `xargs` for trimming whitespace can interpret quotes and backslashes, potentially transforming the command before the security check. Consider using `sed 's/^[[:space:]]*//;s/[[:space:]]*$//'` or bash parameter expansion instead.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| echo "Performing multi-user Nix installation..." | ||
| curl -L https://nixos.org/nix/install | bash -s -- --daemon | ||
| echo "Performing Determinate Nix multi-user installation..." | ||
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install linux |
There was a problem hiding this comment.
P2: Missing --no-confirm flag for Linux multi-user installation. The Docker installation includes this flag for non-interactive use, but the Linux multi-user installation doesn't. This will cause the installer to hang waiting for user input in automated environments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.sh, line 67:
<comment>Missing `--no-confirm` flag for Linux multi-user installation. The Docker installation includes this flag for non-interactive use, but the Linux multi-user installation doesn't. This will cause the installer to hang waiting for user input in automated environments.</comment>
<file context>
@@ -42,29 +42,31 @@ fi
- echo "Performing multi-user Nix installation..."
- curl -L https://nixos.org/nix/install | bash -s -- --daemon
+ echo "Performing Determinate Nix multi-user installation..."
+ curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install linux
# For Linux multi-user installations, add the default Nix path for the current shell.
- export PATH=/nix/var/nix/profiles/default/bin:$PATH
</file context>
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install linux | |
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install linux --no-confirm |
| echo "Installing Nix..." | ||
| if [ "$OS" = "macos" ]; then | ||
| curl -L https://nixos.org/nix/install | bash | ||
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install |
There was a problem hiding this comment.
P2: Missing --no-confirm flag for macOS installation. The Docker installation includes this flag for non-interactive use, but the macOS installation (which the comment indicates runs in CI) doesn't. This will cause the installer to hang waiting for user input in automated environments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.sh, line 45:
<comment>Missing `--no-confirm` flag for macOS installation. The Docker installation includes this flag for non-interactive use, but the macOS installation (which the comment indicates runs in CI) doesn't. This will cause the installer to hang waiting for user input in automated environments.</comment>
<file context>
@@ -42,29 +42,31 @@ fi
echo "Installing Nix..."
if [ "$OS" = "macos" ]; then
- curl -L https://nixos.org/nix/install | bash
+ curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install
# For macOS, source the Nix profile immediately to update PATH in CI.
+ # shellcheck source=/dev/null
</file context>
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install | |
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install --no-confirm |
| if [ -f "$TEMPLATE" ]; then | ||
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" | ||
| sed -e "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" \ | ||
| -e "s|__CLIPROXY_MANAGEMENT_PASSWORD__|${CLIPROXY_MANAGEMENT_PASSWORD:-}|g" \ |
There was a problem hiding this comment.
P2: Password variable may break sed if it contains special characters (|, &, \). Consider escaping the variable or using envsubst for safer template substitution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/cliproxyapi/start.sh, line 24:
<comment>Password variable may break sed if it contains special characters (`|`, `&`, `\`). Consider escaping the variable or using `envsubst` for safer template substitution.</comment>
<file context>
@@ -15,9 +15,14 @@ if [ -f "$ENV_FILE" ]; then
if [ -f "$TEMPLATE" ]; then
- sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG"
+ sed -e "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" \
+ -e "s|__CLIPROXY_MANAGEMENT_PASSWORD__|${CLIPROXY_MANAGEMENT_PASSWORD:-}|g" \
+ "$TEMPLATE" >"$CONFIG"
fi
</file context>
|
|
||
| for segment in "${segments[@]}"; do | ||
| # Trim leading/trailing whitespace | ||
| segment=$(echo "$segment" | xargs 2>/dev/null) || continue |
There was a problem hiding this comment.
P2: xargs for trimming whitespace can interpret quotes and backslashes, potentially transforming the command before the security check. Consider using sed 's/^[[:space:]]*//;s/[[:space:]]*$//' or bash parameter expansion instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/claude/security.sh, line 61:
<comment>`xargs` for trimming whitespace can interpret quotes and backslashes, potentially transforming the command before the security check. Consider using `sed 's/^[[:space:]]*//;s/[[:space:]]*$//'` or bash parameter expansion instead.</comment>
<file context>
@@ -0,0 +1,73 @@
+
+for segment in "${segments[@]}"; do
+ # Trim leading/trailing whitespace
+ segment=$(echo "$segment" | xargs 2>/dev/null) || continue
+ [[ -z "$segment" ]] && continue
+
</file context>
| # Split command at logical operators to catch hidden dangerous commands | ||
| # This handles: cmd1 ; cmd2, cmd1 && cmd2, cmd1 || cmd2, cmd1 | cmd2 | ||
| # shellcheck disable=SC2001 | ||
| IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed 's/[;&|]\+/\n/g' && printf '\0') || true |
There was a problem hiding this comment.
P1: Security bypass: Command substitution ($(...) and backticks) is not handled. A malicious command like echo $(rm -rf /) bypasses detection since the entire string is treated as one segment starting with echo, not rm. Consider parsing nested commands or documenting this limitation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/claude/security.sh, line 57:
<comment>Security bypass: Command substitution (`$(...)` and backticks) is not handled. A malicious command like `echo $(rm -rf /)` bypasses detection since the entire string is treated as one segment starting with `echo`, not `rm`. Consider parsing nested commands or documenting this limitation.</comment>
<file context>
@@ -0,0 +1,73 @@
+# Split command at logical operators to catch hidden dangerous commands
+# This handles: cmd1 ; cmd2, cmd1 && cmd2, cmd1 || cmd2, cmd1 | cmd2
+# shellcheck disable=SC2001
+IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed 's/[;&|]\+/\n/g' && printf '\0') || true
+
+for segment in "${segments[@]}"; do
</file context>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .PHONY: switch | ||
| switch: nix-switch ## Apply Nix configuration and restart launchd agents. | ||
| switch: nix-switch launchctl ## Apply Nix configuration and restart launchd agents. |
There was a problem hiding this comment.
The switch target now depends on launchctl (lines 137‑138), but the launchctl target further down the Makefile unconditionally runs the macOS launchctl kickstart … commands. On Linux or CI runners, make switch (and any target that invokes it, e.g., make install/update) will now abort with launchctl: command not found, whereas previously it succeeded because switch only ran nix-switch. This makes the standard configuration apply workflow unusable off macOS.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Makefile (1)
96-106: Consider addingallandcleantargets to satisfy checkmake (or adjust the rule).Static analysis flagged missing required phony targets. Low cost to add:
+.PHONY: all +all: help ## Default build target. + +.PHONY: clean +clean: ## Clean generated artifacts. + @rm -f vm_commands.txtnamed-hosts/kyber/README.md (1)
15-24: Doc will break on Ubuntu ifmake switchrunslaunchctl(align with Makefile behavior).Either fix
switchto be OS-conditional (preferred) or update Kyber docs to usenix-switch:-# 4. Apply configuration -make switch +# 4. Apply configuration +make nix-switch
🧹 Nitpick comments (8)
home-manager/programs/fish/functions/_kyber_function.fish (1)
1-3: Optional: add a tiny guard / friendlier failure whentailscaleisn’t available.
Right now this will just error iftailscaleisn’t installed or not logged in; consider an early check (e.g.,type -q tailscale; or begin; echo "tailscale not found"; return 127; end) if you want a cleaner UX.home-manager/services/code-syncer/sync.sh (1)
1-2: Avoid global ShellCheck suppression; fix or scope the underlying warnings.
Line 2 disables several rules for the whole script, which can mask real issues. Prefer: (a) fix patterns (e.g.,local x; x="$(...)",if ! cmd; then ...,read -r) and (b) if still needed, disable per-line. Based on learnings/coding guidelines.config/claude/default.nix (1)
16-20: Good addition; consider consistent ordering ofhome.fileentries
Adding.claude/security.shas an executable is correct for the new PreToolUse hook. Optionally reorder/grouphome.filekeys (e.g., alphabetical) to match the repo’s Nix style guidelines.config/claude/pushover.sh (1)
114-123: SkippingSessionEnd.reason == "other"reduces notification noise (good)
Minor robustness thought: if non-SessionEnd payloads can also include areasonkey, consider additionally gating on an explicit event field (e.g.,hook_event_name == "SessionEnd") when available.spec/pushover_spec.sh (2)
17-38:mock_curlis currently unused; stubcurlvia PATH (or delete the mock)
As written,mock_curl()won’t affectbash "$SCRIPT"(separate process), so the test doesn’t actually verify “notification attempted”. Either remove the dead mock or replace it with a PATH-based stub and assert it was invoked for the"user_exit"case.Describe 'SessionEnd hook' setup() { export PUSHOVER_API_TOKEN="test_token" export PUSHOVER_USER_KEY="test_user" + STUB_BIN="$(mktemp -d)" + export PATH="$STUB_BIN:$PATH" + cat >"$STUB_BIN/curl" <<'EOF' +#!/usr/bin/env bash +echo "CURL_CALLED" +EOF + chmod +x "$STUB_BIN/curl" } Before 'setup' - mock_curl() { - echo "CURL_CALLED" - } - It 'skips notification for "other" reason' When run bash "$SCRIPT" <<< '{"reason": "other"}' The status should be success The output should eq '' End It 'processes notification for "user_exit" reason' When run bash "$SCRIPT" <<< '{"reason": "user_exit"}' The status should be success + The output should eq 'CURL_CALLED' End End
40-57: Same note for Notification tests: assert “sent” vs “skipped” via a stub
Right now these only assert exit status; consider also asserting the curl stub was (not) called for each scenario.spec/notify_spec.sh (1)
21-51: LGTM as status-only smoke tests
Ifnotify.shis expected to emit output or touch a file/command in these cases, consider asserting that side effect (otherwise this is fine as a regression guard for “must not fail”).spec/security_spec.sh (1)
7-32: Good isolated HOME-based fixture; tighten execution to avoidbash -cquoting pitfalls
Usingbash -c "HOME='...'"is avoidable here; preferenv HOME="$TEMP_HOME" bash "$SCRIPT"for safer quoting.- When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'" + When run env HOME="$TEMP_HOME" bash "$SCRIPT"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
.env.example(1 hunks).github/workflows/shell.yml(1 hunks).shellspec(1 hunks)Makefile(3 hunks)README-nix.md(1 hunks)config/claude/default.nix(1 hunks)config/claude/notify.sh(1 hunks)config/claude/pushover.sh(2 hunks)config/claude/security.sh(1 hunks)config/claude/settings.json(4 hunks)config/cliproxyapi/config.yaml(1 hunks)devenv.nix(1 hunks)home-manager/packages/default.nix(1 hunks)home-manager/programs/fish/default.nix(2 hunks)home-manager/programs/fish/functions/_kyber_function.fish(1 hunks)home-manager/programs/fish/functions/_kybers_function.fish(1 hunks)home-manager/services/cliproxyapi/start.sh(1 hunks)home-manager/services/code-syncer/sync.sh(1 hunks)install.sh(1 hunks)named-hosts/kyber/README.md(1 hunks)spec/notify_spec.sh(1 hunks)spec/pushover_spec.sh(1 hunks)spec/security_spec.sh(1 hunks)spec/spec_helper.sh(1 hunks)spec/support/custom_matcher.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{json,yaml,yml,toml}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
**/*.{json,yaml,yml,toml}: Use consistent indentation (2 spaces) in configuration files
Sort keys alphabetically when possible in configuration files
Use clear, descriptive names in configuration files
Files:
config/cliproxyapi/config.yamlconfig/claude/settings.json
**/*.{yaml,yml,toml}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
Add comments for complex configurations
Files:
config/cliproxyapi/config.yaml
**/*.{sh,bash}
📄 CodeRabbit inference engine (CLAUDE.md)
Use shfmt with 2-space indentation for shell scripts
**/*.{sh,bash}: Use 2 spaces for indentation in shell scripts
Add proper shebang lines to shell scripts
Follow shellcheck recommendations in shell scripts
Document complex commands in shell scripts
Use consistent variable naming in shell scripts
Files:
spec/spec_helper.shspec/notify_spec.shconfig/claude/security.shhome-manager/services/code-syncer/sync.shspec/support/custom_matcher.shconfig/claude/notify.shspec/pushover_spec.shspec/security_spec.shinstall.shhome-manager/services/cliproxyapi/start.shconfig/claude/pushover.sh
**/*.nix
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.nix: Use nixfmt for formatting all Nix files
Document complex configurations with comments in Nix files
**/*.nix: Use 2 spaces for indentation in Nix files
Keep line length under 100 characters in Nix files
Sort attribute sets alphabetically in Nix files
Use consistent spacing around operators in Nix files
Format lists and sets consistently in Nix filesUse treefmt.toml for formatting Nix files
**/*.nix: UsemkOptionfor configurable options in Nix modules
Implement proper typing for all options in Nix modules
Follow the Nix expression language style guide
Files:
home-manager/programs/fish/default.nixconfig/claude/default.nixdevenv.nixhome-manager/packages/default.nix
**/default.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Use
default.nixfiles for module exports
Files:
home-manager/programs/fish/default.nixconfig/claude/default.nixhome-manager/packages/default.nix
home-manager/programs/*/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Program configurations should be located in
home-manager/programs/<name>/and use home-manager's built-in modules when available
Files:
home-manager/programs/fish/default.nix
home-manager/programs/**/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Program configurations must include all necessary dependencies in their configuration
Files:
home-manager/programs/fish/default.nix
home-manager/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
home-manager/**/*.nix: Use typed options whenever possible in Nix configurations
Document all configuration options in Nix modules and programs
Follow home-manager's module structure and keep configurations modular
Use proper indentation and formatting in Nix configuration files
Files:
home-manager/programs/fish/default.nixhome-manager/packages/default.nix
home-manager/programs/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Follow program-specific best practices in program configuration files
Program configurations in
home-manager/programs/should be organized by program name, include all necessary dependencies, usehome.packagesfor package installations, and useprograms.<name>when available in home-manager
Files:
home-manager/programs/fish/default.nix
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
GitHub Actions workflows must be properly configured in .github/workflows/ directory
Files:
.github/workflows/shell.yml
.github/workflows/*.yml
📄 CodeRabbit inference engine (.cursor/rules/github-workflows.mdc)
.github/workflows/*.yml: CI Pipeline must run on all pull requests and main branch pushes, validating Nix flake checks, home-manager builds, nix-darwin builds, and system configurations
Run Biome for code formatting and validate Nix expressions, commit message format, and documentation updates in code quality checks
Use specific action versions (not @main or @master) in GitHub Actions workflows
Cache Nix store and build artifacts in GitHub Actions workflows to improve performance
Set appropriate timeout limits in GitHub Actions workflow jobs
Use concise job and step names in GitHub Actions workflows and add helpful annotations and comments
Use GITHUB_TOKEN when possible, secure sensitive data in repository secrets, and limit permissions to minimum required in GitHub Actions workflows
Review third-party actions before use in GitHub Actions workflows
Set appropriate failure conditions, add helpful error messages, configure notifications for failures, and archive build artifacts for debugging in GitHub Actions workflows
Files:
.github/workflows/shell.yml
{.github/workflows/*.yml,renovate.json}
📄 CodeRabbit inference engine (.cursor/rules/github-workflows.mdc)
Use Renovate for automated dependency updates, configure update schedule in renovate.json, group related updates together, and auto-merge minor and patch updates
Files:
.github/workflows/shell.yml
**/*.{js,jsx,ts,tsx,json,jsonc,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Use Biome for code formatting as configured in biome.json
Files:
named-hosts/kyber/README.mdREADME-nix.mdconfig/claude/settings.json
**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome with 2-space indentation, 80-character line width, double quotes, and trailing commas (ES5) for JSON/JavaScript/TypeScript files
Files:
config/claude/settings.json
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
**/*.{js,jsx,ts,tsx,json}: Format JavaScript/TypeScript/JSON files using Biome with 2-space indentation and 80 character line width
Enable Biome linter with recommended rules for JavaScript/TypeScript/JSON files
Files:
config/claude/settings.json
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Follow shellcheck recommendations in shell scripts
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Follow shellcheck recommendations in shell scripts
Applied to files:
config/claude/security.shhome-manager/services/code-syncer/sync.shconfig/claude/notify.shspec/security_spec.shMakefile
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/**/*.nix : Follow program-specific best practices in program configuration files
Applied to files:
config/claude/default.nixREADME-nix.md
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Add proper shebang lines to shell scripts
Applied to files:
home-manager/services/code-syncer/sync.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Use consistent variable naming in shell scripts
Applied to files:
home-manager/services/code-syncer/sync.shspec/support/custom_matcher.sh
📚 Learning: 2025-11-25T09:34:23.224Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:34:23.224Z
Learning: Applies to **/*.{sh,bash} : Use shfmt with 2-space indentation for shell scripts
Applied to files:
home-manager/services/code-syncer/sync.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Document complex commands in shell scripts
Applied to files:
home-manager/services/code-syncer/sync.shspec/support/custom_matcher.shspec/security_spec.sh
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Document all major changes in Nix configurations
Applied to files:
home-manager/packages/default.nixMakefileinstall.shnamed-hosts/kyber/README.mdREADME-nix.md
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/**/default.nix : Program configurations must include all necessary dependencies in their configuration
Applied to files:
home-manager/packages/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/**/*.nix : Document all configuration options in Nix modules and programs
Applied to files:
home-manager/packages/default.nixREADME-nix.md
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : CI Pipeline must run on all pull requests and main branch pushes, validating Nix flake checks, home-manager builds, nix-darwin builds, and system configurations
Applied to files:
.github/workflows/shell.ymlMakefile
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Use concise job and step names in GitHub Actions workflows and add helpful annotations and comments
Applied to files:
.github/workflows/shell.yml
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Run Biome for code formatting and validate Nix expressions, commit message format, and documentation updates in code quality checks
Applied to files:
.github/workflows/shell.ymlMakefile
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Cache Nix store and build artifacts in GitHub Actions workflows to improve performance
Applied to files:
.github/workflows/shell.yml
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Set appropriate failure conditions, add helpful error messages, configure notifications for failures, and archive build artifacts for debugging in GitHub Actions workflows
Applied to files:
.github/workflows/shell.yml
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Set appropriate timeout limits in GitHub Actions workflow jobs
Applied to files:
.github/workflows/shell.yml
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Use specific action versions (not main or master) in GitHub Actions workflows
Applied to files:
.github/workflows/shell.yml
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Test Nix and home-manager configurations locally before pushing using `make test`
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Test configurations before committing using `nix flake check` and `home-manager build --show-trace`
Applied to files:
MakefileREADME-nix.md
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Applies to flake.nix : Maintain flake.nix as the main Nix configuration with proper structure
Applied to files:
MakefileREADME-nix.md
📚 Learning: 2025-11-25T09:34:23.224Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:34:23.224Z
Learning: Applies to nix-darwin/**/*.nix : Use Homebrew only for macOS-specific applications
Applied to files:
Makefileinstall.sh
📚 Learning: 2025-11-25T09:34:23.224Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:34:23.224Z
Learning: Prefer Nix packages over external package managers
Applied to files:
install.shREADME-nix.md
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Applies to **/*.nix : Follow the Nix expression language style guide
Applied to files:
README-nix.md
📚 Learning: 2025-11-25T09:34:23.224Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:34:23.224Z
Learning: Applies to **/*.nix : Document complex configurations with comments in Nix files
Applied to files:
README-nix.md
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Applies to flake.nix : Always pin package versions in `flake.lock`
Applied to files:
README-nix.md
🪛 checkmake (0.2.2)
Makefile
[warning] 645-645: Missing required phony target "all"
(minphony)
[warning] 645-645: Missing required phony target "clean"
(minphony)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 5-5: [UnorderedKey] The CLIPROXY_MANAGEMENT_PASSWORD key should go before the MY_SECRET key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Agent
- GitHub Check: nix-linux
- GitHub Check: shellcheck
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: nix-darwin
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: nix-nixos
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: lua-hammerspoon
- GitHub Check: lua-neovim
- GitHub Check: lua-neovim-test
- GitHub Check: shellspec
🔇 Additional comments (19)
home-manager/programs/fish/default.nix (1)
46-72: kybers abbreviation/function export wiring looks consistent (abbr +xdg.configFileexport list updated together).
Only thing to double-check: thekybersname is close tokyber—if that’s intentional (plain SSH vs “SSH+zellij”), consider a short comment above the two abbrs to aid future-you.Also applies to: 119-140
home-manager/programs/fish/functions/_kybers_function.fish (1)
1-3: Verifyzellij attach -cbehavior is what you want across machines (session selection / “create” semantics can be surprising). If you rely on a specific session, considerzellij attach <name> --create(or whatever your zellij version supports) to make it deterministic.README-nix.md (1)
6-7: Doc update is clear and consistent with the new Determinate Nix flow.config/claude/notify.sh (1)
10-13: Good, narrowly-scoped ShellCheck suppression for dynamic.envsourcing. Based on learnings.home-manager/packages/default.nix (1)
63-66: Addingshellcheckandshellspechere makes sense for local parity with CI.devenv.nix (1)
4-10: Good: dev shell now includes the same shell tooling CI expects..github/workflows/shell.yml (1)
1-48: The workflow already uses correct, current action versions.actions/checkout@v6is the latest stable major version (v6.0.1 as of Dec 2025), andcachix/install-nix-action@v31is the current version with v31.9.0 being the latest patch. Both actions are properly pinned to specific versions per guidelines. No changes needed.Likely an incorrect or invalid review comment.
install.sh (1)
45-71: Add--determinateflag to all Determinate Nix installer invocations to ensure Determinate Nix with flakes is installed.The current commands on lines 52–54 and 66 omit the
--determinateflag, which per 2025 Determinate Systems documentation is the recommended way to explicitly request Determinate Nix (with flakes enabled). Without it, the installer may default to upstream Nix instead. Update:
- Line 52:
sh -s -- install linux --init none --no-confirm→sh -s -- install --determinate --init none --no-confirm- Line 66:
sh -s -- install linux→sh -s -- install --determinate linuxLine 45 (macOS) should also add
--determinateunless upstream Nix is intentionally required. The sourced profile paths are correct for their respective contexts.⛔ Skipped due to learnings
Learnt from: CR Repo: shunkakinoki/dotfiles PR: 0 File: .cursor/rules/formatting.mdc:0-0 Timestamp: 2025-11-25T09:34:32.423Z Learning: Applies to **/*.{sh,bash} : Follow shellcheck recommendations in shell scriptsLearnt from: CR Repo: shunkakinoki/dotfiles PR: 0 File: .cursor/rules/general.mdc:0-0 Timestamp: 2025-11-25T09:34:40.062Z Learning: Applies to flake.nix : Maintain flake.nix as the main Nix configuration with proper structureLearnt from: CR Repo: shunkakinoki/dotfiles PR: 0 File: .cursor/rules/nix.mdc:0-0 Timestamp: 2025-11-25T09:35:01.066Z Learning: Test configurations before committing using `nix flake check` and `home-manager build --show-trace`Learnt from: CR Repo: shunkakinoki/dotfiles PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-25T09:34:23.224Z Learning: Applies to nix-darwin/**/*.nix : Use Homebrew only for macOS-specific applicationsspec/support/custom_matcher.sh (1)
1-1: Remove the shebang fromspec/support/custom_matcher.shif this file is sourced viaimport. Shebangs are unnecessary for files meant to be sourced, not executed directly. Verify that this file actually defines matcher functions that are used by the specs; if it remains empty or unused, it may be dead code.Makefile (1)
202-208: The Determinate Nix installer invocation is correct. The installer supports platform autodetection when using justinstallwithout an explicit planner argument. The current commandcurl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- installcorrectly leverages this autodetection and will select the appropriate planner for macOS or Linux at runtime. Explicitly specifyinginstall linuxis unnecessary..shellspec (1)
1-3: The .shellspec configuration is correct. The--require spec_helperdirective follows ShellSpec's standard pattern and will loadspec/spec_helper.shby default from thespec/directory. The--shell bashoption is stable and consistent across ShellSpec versions (bash ≥ 2.03 is supported). No verification against specific CI versions is necessary—ShellSpec's path resolution and shell compatibility are well-established and documented.spec/spec_helper.sh (1)
1-11: minimum_version "0.28.1" is valid for ShellSpec. Confirm that.shellspecand CI configuration correctly defineSHELLSPEC_LOAD_PATHto include the directory containingsupport/custom_matcher.sh, as ShellSpec'simportsearches these paths rather than resolving filesystem paths directly.config/claude/pushover.sh (1)
10-16: ShellCheck directive now correctly scoped to thesourceline
Directive placement is better/clearer and keeps the export semantics viaset -aintact.spec/pushover_spec.sh (1)
1-16: Nice basic coverage for “no credentials” early-exit
This guards the “do nothing quietly” behavior well.spec/notify_spec.sh (1)
7-20: Good coverage of “Pushover configured => skip local notify”
Clear setup and expectations.spec/security_spec.sh (1)
33-89: Coverage matrix looks solid (non-Bash pass-through + safe + blocked + empty input)
These cases map well to the deny-pattern enforcement goal.config/claude/settings.json (3)
70-93: Good: explicit read/write deny rules for sensitive files
This is a strong improvement to prevent accidental secret exfiltration/modification even when broader tooling is allowed.
123-143: Good hook order: runsecurity.shbefore notify/pushover
Having the guardrail execute first is the right default.
230-232: ThedefaultMode: "bypassPermissions"setting only bypasses interactive prompts, not permission enforcement"bypassPermissions" automatically allows tool use and file access without asking, but does not bypass enforcement from
permissions.denyor security checks. The setting skips user prompts in trusted environments;permissions.denyrules and prehook validation remain in effect.
| # Copy to home-manager/.env and provide real values. The .env file stays local. | ||
| MY_SECRET=replace-me | ||
| # GITHUB_TOKEN=ghp_your_token_here | ||
| CLIPROXY_MANAGEMENT_PASSWORD=your-management-key-here |
There was a problem hiding this comment.
Optional: reorder keys if dotenv-linter is enforced in CI.
If dotenv-linter warnings fail the pipeline, move CLIPROXY_MANAGEMENT_PASSWORD above MY_SECRET; otherwise this is fine.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 5-5: [UnorderedKey] The CLIPROXY_MANAGEMENT_PASSWORD key should go before the MY_SECRET key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env.example around line 5, dotenv-linter may flag variable order; move
CLIPROXY_MANAGEMENT_PASSWORD above MY_SECRET to satisfy lint ordering. Edit the
file to place the CLIPROXY_MANAGEMENT_PASSWORD entry before the MY_SECRET entry
so dotenv-linter (if enforced in CI) no longer reports ordering warnings.
| set -euo pipefail | ||
|
|
||
| # Read JSON input from stdin | ||
| input=$(cat) | ||
|
|
||
| # Extract tool name - only process Bash commands | ||
| tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null) | ||
| [[ "$tool_name" != "Bash" ]] && exit 0 | ||
|
|
||
| # Extract the command to be executed | ||
| command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null) | ||
| [[ -z "$command" ]] && exit 0 | ||
|
|
||
| # Path to settings file with deny patterns | ||
| settings="$HOME/.claude/settings.json" | ||
| [[ ! -f "$settings" ]] && exit 0 | ||
|
|
||
| # Read deny patterns from settings | ||
| mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null) | ||
|
|
There was a problem hiding this comment.
Fail-open safely when jq is missing or JSON is invalid (today it may fail-closed under set -euo pipefail).
With set -euo pipefail, any jq error in tool_name=$(echo ... | jq ...) / command=... / mapfile ... < <(jq ...) can exit the hook non-zero and unintentionally block all Bash tool usage.
set -euo pipefail
+# If jq isn't available (or JSON is malformed), don't block tool execution.
+command -v jq >/dev/null 2>&1 || exit 0
+
# Read JSON input from stdin
input=$(cat)
# Extract tool name - only process Bash commands
-tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null)
+tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null || true)
[[ "$tool_name" != "Bash" ]] && exit 0
# Extract the command to be executed
-command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null)
+command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null || true)
[[ -z "$command" ]] && exit 0
@@
# Read deny patterns from settings
-mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null)
+mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null || true)As per coding guidelines (shellcheck resiliency for **/*.{sh,bash}), this also prevents “tool broke because jq glitched” failures. Based on learnings, follow ShellCheck recommendations.
📝 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.
| set -euo pipefail | |
| # Read JSON input from stdin | |
| input=$(cat) | |
| # Extract tool name - only process Bash commands | |
| tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null) | |
| [[ "$tool_name" != "Bash" ]] && exit 0 | |
| # Extract the command to be executed | |
| command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null) | |
| [[ -z "$command" ]] && exit 0 | |
| # Path to settings file with deny patterns | |
| settings="$HOME/.claude/settings.json" | |
| [[ ! -f "$settings" ]] && exit 0 | |
| # Read deny patterns from settings | |
| mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null) | |
| set -euo pipefail | |
| # If jq isn't available (or JSON is malformed), don't block tool execution. | |
| command -v jq >/dev/null 2>&1 || exit 0 | |
| # Read JSON input from stdin | |
| input=$(cat) | |
| # Extract tool name - only process Bash commands | |
| tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null || true) | |
| [[ "$tool_name" != "Bash" ]] && exit 0 | |
| # Extract the command to be executed | |
| command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null || true) | |
| [[ -z "$command" ]] && exit 0 | |
| # Path to settings file with deny patterns | |
| settings="$HOME/.claude/settings.json" | |
| [[ ! -f "$settings" ]] && exit 0 | |
| # Read deny patterns from settings | |
| mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null || true) | |
| matches_pattern() { | ||
| local cmd="$1" | ||
| local pattern="$2" | ||
|
|
||
| # Extract pattern from Bash(...) format | ||
| if [[ "$pattern" =~ ^Bash\((.+)\)$ ]]; then | ||
| local check_pattern="${BASH_REMATCH[1]}" | ||
| # Remove trailing :* if present (legacy format) | ||
| check_pattern="${check_pattern%:*}" | ||
|
|
||
| # Use bash glob matching (extended globbing) | ||
| shopt -s extglob | ||
| # shellcheck disable=SC2053 | ||
| if [[ "$cmd" == $check_pattern ]]; then | ||
| return 0 | ||
| fi | ||
| shopt -u extglob | ||
| fi | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
Restore extglob even on a successful match (currently it can stay enabled until process exit).
Right now, return 0 happens before shopt -u extglob. Easy to make deterministic:
matches_pattern() {
local cmd="$1"
local pattern="$2"
@@
# Use bash glob matching (extended globbing)
- shopt -s extglob
- # shellcheck disable=SC2053
- if [[ "$cmd" == $check_pattern ]]; then
- return 0
- fi
- shopt -u extglob
+ local extglob_was_set=0 matched=1
+ shopt -q extglob && extglob_was_set=1
+ shopt -s extglob
+ # shellcheck disable=SC2053
+ [[ "$cmd" == $check_pattern ]] && matched=0
+ (( extglob_was_set == 1 )) || shopt -u extglob
+ return "$matched"
fi
return 1
}🤖 Prompt for AI Agents
In config/claude/security.sh around lines 33 to 52, extglob is enabled with
shopt -s but on a successful match the function returns before shopt -u is
called, leaving extglob enabled for the rest of the shell; fix by saving the
prior extglob state before enabling and restoring it before any return (or use a
subshell), e.g., capture the output of shopt -q extglob to a variable, enable
extglob, perform the match, then restore extglob to its original state (call
shopt -s extglob if it was on, or shopt -u extglob if it was off) before
returning.
| # Split command at logical operators to catch hidden dangerous commands | ||
| # This handles: cmd1 ; cmd2, cmd1 && cmd2, cmd1 || cmd2, cmd1 | cmd2 | ||
| # shellcheck disable=SC2001 | ||
| IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed 's/[;&|]\+/\n/g' && printf '\0') || true | ||
|
|
||
| for segment in "${segments[@]}"; do | ||
| # Trim leading/trailing whitespace | ||
| segment=$(echo "$segment" | xargs 2>/dev/null) || continue | ||
| [[ -z "$segment" ]] && continue | ||
|
|
There was a problem hiding this comment.
Avoid xargs for trimming: it normalizes whitespace and can change what you’re matching.
For a security hook, don’t mutate the command string more than necessary—use a trim that preserves internal spacing:
for segment in "${segments[@]}"; do
# Trim leading/trailing whitespace
- segment=$(echo "$segment" | xargs 2>/dev/null) || continue
+ segment=$(printf '%s' "$segment" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//') || continue
[[ -z "$segment" ]] && continueBased on learnings, follow ShellCheck recommendations.
📝 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.
| # Split command at logical operators to catch hidden dangerous commands | |
| # This handles: cmd1 ; cmd2, cmd1 && cmd2, cmd1 || cmd2, cmd1 | cmd2 | |
| # shellcheck disable=SC2001 | |
| IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed 's/[;&|]\+/\n/g' && printf '\0') || true | |
| for segment in "${segments[@]}"; do | |
| # Trim leading/trailing whitespace | |
| segment=$(echo "$segment" | xargs 2>/dev/null) || continue | |
| [[ -z "$segment" ]] && continue | |
| # Split command at logical operators to catch hidden dangerous commands | |
| # This handles: cmd1 ; cmd2, cmd1 && cmd2, cmd1 || cmd2, cmd1 | cmd2 | |
| # shellcheck disable=SC2001 | |
| IFS=$'\n' read -r -d '' -a segments < <(echo "$command" | sed 's/[;&|]\+/\n/g' && printf '\0') || true | |
| for segment in "${segments[@]}"; do | |
| # Trim leading/trailing whitespace | |
| segment=$(printf '%s' "$segment" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//') || continue | |
| [[ -z "$segment" ]] && continue | |
🤖 Prompt for AI Agents
In config/claude/security.sh around lines 54 to 63, the trimming uses "xargs"
which normalizes internal whitespace and can change the command being inspected;
replace the xargs-based trim with a safe leading/trailing-only trim that
preserves internal spacing (use POSIX shell parameter expansion to remove
leading and trailing whitespace, or use sed with explicit ^[[:space:]]* and
[[:space:]]*$ patterns), ensure you do not spawn unnecessary subshells or
collapse internal spaces, and keep the check that skips empty segments.
| { | ||
| "cleanupPeriodDays": 99999, | ||
| "permissions": { |
There was a problem hiding this comment.
Revisit cleanupPeriodDays: 99999 retention choice
That’s effectively “keep almost forever”; if the goal is privacy or disk hygiene, consider a more bounded value (or document why extremely long retention is desired).
🤖 Prompt for AI Agents
In config/claude/settings.json around lines 1 to 3, the cleanupPeriodDays is set
to an effectively permanent value (99999); change this to a sensible bounded
retention (e.g., 30, 90, or 365 days) or replace the literal with a named
constant/env var and add a short comment or documentation explaining why a long
retention is required if you must keep it long-term; update tests/config docs
accordingly.
| remote-management: | ||
| # Whether to allow remote (non-localhost) management access. | ||
| # When false, only localhost can access management endpoints (a key is still required). | ||
| allow-remote: true | ||
| allow-remote: false | ||
| # Management key. If a plaintext value is provided here, it will be hashed on startup. | ||
| # All management requests (even from localhost) require this key. | ||
| # Leave empty to disable the Management API entirely (404 for all /v0/management routes). | ||
| secret-key: "" | ||
| secret-key: "__CLIPROXY_MANAGEMENT_PASSWORD__" | ||
| # Disable the bundled management control panel asset download and HTTP route when true. | ||
| disable-control-panel: false |
There was a problem hiding this comment.
❓ Verification inconclusive
🏁 Script executed:
# First, let's find the start.sh file mentioned in the comment
find . -name "start.sh" -type fRepository: shunkakinoki/dotfiles
Length of output: 186
🏁 Script executed:
# Search for Management API implementation and secret-key handling
rg -i "secret.?key|management.*password" --type yaml --type py --type js --type ts -A 3 -B 1Repository: shunkakinoki/dotfiles
Length of output: 186
🏁 Script executed:
# Check the YAML file itself for formatting compliance
cat -n config/cliproxyapi/config.yaml | head -20Repository: shunkakinoki/dotfiles
Length of output: 186
🏁 Script executed:
# Search for how MANAGEMENT_PASSWORD env var is used
rg "MANAGEMENT_PASSWORD|CLIPROXY_MANAGEMENT_PASSWORD" -A 3 -B 1Repository: shunkakinoki/dotfiles
Length of output: 186
Prevent deploying a config with the literal placeholder as the management key.
If template substitution fails, secret-key becomes a predictable non-empty credential. Consider enforcing "no placeholders remain" validation in the generator (as mentioned in start.sh), or defaulting secret-key to empty and relying on MANAGEMENT_PASSWORD environment variable exclusively.
Verification needed: Confirm whether secret-key being set to a non-empty value always enables management auth (even if MANAGEMENT_PASSWORD is empty), and whether an empty string is guaranteed to disable /v0/management routes.
🤖 Prompt for AI Agents
config/cliproxyapi/config.yaml lines 4-13: the template leaves the literal
placeholder "__CLIPROXY_MANAGEMENT_PASSWORD__" in secret-key which can become a
predictable credential if substitution fails; update the generator/start.sh to
detect this placeholder and either (a) fail generation with a clear error, or
(b) replace the placeholder with an empty string so secret-key is default-empty,
and add a unit/integration check that validates no placeholders remain in the
rendered config; additionally verify in code that a non-empty secret-key always
enables management auth and that an empty string reliably disables
/v0/management routes, and add tests or runtime log messages to assert this
behavior.
| # Export management password for Management API (CLIProxyAPI requires MANAGEMENT_PASSWORD env var) | ||
| export MANAGEMENT_PASSWORD="${CLIPROXY_MANAGEMENT_PASSWORD:-}" | ||
|
|
||
| # Generate config from template with secrets injected | ||
| if [ -f "$TEMPLATE" ]; then | ||
| sed "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" "$TEMPLATE" >"$CONFIG" | ||
| sed -e "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" \ | ||
| -e "s|__CLIPROXY_MANAGEMENT_PASSWORD__|${CLIPROXY_MANAGEMENT_PASSWORD:-}|g" \ | ||
| "$TEMPLATE" >"$CONFIG" | ||
| fi |
There was a problem hiding this comment.
Escape sed replacements + write config with umask 077 + fail if placeholders remain.
set -euo pipefail
+umask 077
@@
# Generate config from template with secrets injected
if [ -f "$TEMPLATE" ]; then
- sed -e "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" \
- -e "s|__CLIPROXY_MANAGEMENT_PASSWORD__|${CLIPROXY_MANAGEMENT_PASSWORD:-}|g" \
+ escape_sed_replacement() {
+ # Escape backslash, &, and the chosen delimiter '|'
+ printf '%s' "$1" | sed -e 's/[\\&|]/\\&/g'
+ }
+
+ OPENROUTER_API_KEY_ESC=$(escape_sed_replacement "${OPENROUTER_API_KEY:-}")
+ CLIPROXY_MANAGEMENT_PASSWORD_ESC=$(escape_sed_replacement "${CLIPROXY_MANAGEMENT_PASSWORD:-}")
+
+ sed -e "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY_ESC}|g" \
+ -e "s|__CLIPROXY_MANAGEMENT_PASSWORD__|${CLIPROXY_MANAGEMENT_PASSWORD_ESC}|g" \
"$TEMPLATE" >"$CONFIG"
+
+ # Refuse to start if template placeholders survived injection.
+ if grep -qE '__OPENROUTER_API_KEY__|__CLIPROXY_MANAGEMENT_PASSWORD__' "$CONFIG"; then
+ echo "config.yaml still contains template placeholders; refusing to start" >&2
+ exit 1
+ fi
fiBased on learnings, follow ShellCheck recommendations.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In home-manager/services/cliproxyapi/start.sh around lines 18 to 26, the current
sed replacements can break on values containing slashes or ampersands, the
config is written directly with insecure permissions, and the script doesn’t
fail if placeholders remain; fix by: (1) escape each replacement value before
passing to sed (e.g. replace problematic characters like / and & with
backslash-escaped versions using a small helper or printf/sed pipeline) so sed
substitutions are safe; (2) set umask 077 before creating the config and write
to a secure temporary file then atomically mv it into place to avoid
race/permission issues; and (3) after generation, check the resulting file for
any remaining placeholder tokens (e.g. grep for __OPENROUTER_API_KEY__ or
__CLIPROXY_MANAGEMENT_PASSWORD__) and exit non‑zero with a clear error if any
are found.
| .PHONY: switch | ||
| switch: nix-switch ## Apply Nix configuration and restart launchd agents. | ||
| switch: nix-switch launchctl ## Apply Nix configuration and restart launchd agents. | ||
|
|
||
| .PHONY: test | ||
| test: neovim-test | ||
| test: neovim-test shell-test ## Run all tests (neovim + shell). | ||
|
|
There was a problem hiding this comment.
make switch will fail on Linux now (because launchctl is a hard dependency).
This is especially problematic because named-hosts/kyber/README.md instructs running make switch on Ubuntu.
-.PHONY: switch
-switch: nix-switch launchctl ## Apply Nix configuration and restart launchd agents.
+.PHONY: switch
+switch: nix-switch ## Apply Nix configuration and restart launchd agents (macOS only).
+ifeq ($(OS),Darwin)
+ @$(MAKE) launchctl
+else
+ @echo "ℹ️ launchctl skipped on $(OS)"
+endifCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
Makefile lines 137-142: the switch target currently hard-depends on launchctl
which exists only on macOS, causing make switch to fail on Linux; change the
Makefile so the launchctl dependency is conditional: detect the OS (uname) or
set a variable (e.g., LAUNCHCTL := launchctl on Darwin, empty otherwise) and
make switch depend on that variable (or make launchctl a no-op on non-Darwin),
ensuring make switch runs on Linux without requiring launchctl.
| .PHONY: shell-test | ||
| shell-test: ## Run shell script tests using ShellSpec. | ||
| @echo "🧪 Running shell tests..." | ||
| @shellspec | ||
|
|
||
| .PHONY: shell-test-dev | ||
| shell-test-dev: ## Run shell tests inside the Nix dev shell (mirrors CI). | ||
| @echo "🧪 Running shell tests inside the Nix dev shell..." | ||
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-test | ||
|
|
||
| .PHONY: shell-check | ||
| shell-check: ## Run ShellCheck on shell scripts. | ||
| @echo "🔍 Running ShellCheck..." | ||
| @find . -name '*.sh' -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' | xargs shellcheck | ||
|
|
||
| .PHONY: shell-check-dev | ||
| shell-check-dev: ## Run ShellCheck inside the Nix dev shell (mirrors CI). | ||
| @echo "🔍 Running ShellCheck inside the Nix dev shell..." | ||
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-check |
There was a problem hiding this comment.
Make shell-check robust to spaces/newlines and “no files found” cases.
find ... | xargs shellcheck is brittle. Prefer -exec ... {} +:
shell-check: ## Run ShellCheck on shell scripts.
@echo "🔍 Running ShellCheck..."
- @find . -name '*.sh' -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' | xargs shellcheck
+ @find . -name '*.sh' \
+ -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' \
+ -exec shellcheck {} +Based on learnings, follow ShellCheck recommendations.
📝 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.
| .PHONY: shell-test | |
| shell-test: ## Run shell script tests using ShellSpec. | |
| @echo "🧪 Running shell tests..." | |
| @shellspec | |
| .PHONY: shell-test-dev | |
| shell-test-dev: ## Run shell tests inside the Nix dev shell (mirrors CI). | |
| @echo "🧪 Running shell tests inside the Nix dev shell..." | |
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-test | |
| .PHONY: shell-check | |
| shell-check: ## Run ShellCheck on shell scripts. | |
| @echo "🔍 Running ShellCheck..." | |
| @find . -name '*.sh' -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' | xargs shellcheck | |
| .PHONY: shell-check-dev | |
| shell-check-dev: ## Run ShellCheck inside the Nix dev shell (mirrors CI). | |
| @echo "🔍 Running ShellCheck inside the Nix dev shell..." | |
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-check | |
| .PHONY: shell-test | |
| shell-test: ## Run shell script tests using ShellSpec. | |
| @echo "🧪 Running shell tests..." | |
| @shellspec | |
| .PHONY: shell-test-dev | |
| shell-test-dev: ## Run shell tests inside the Nix dev shell (mirrors CI). | |
| @echo "🧪 Running shell tests inside the Nix dev shell..." | |
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-test | |
| .PHONY: shell-check | |
| shell-check: ## Run ShellCheck on shell scripts. | |
| @echo "🔍 Running ShellCheck..." | |
| @find . -name '*.sh' \ | |
| -not -path './node_modules/*' -not -path './.git/*' -not -path './result/*' \ | |
| -exec shellcheck {} + | |
| .PHONY: shell-check-dev | |
| shell-check-dev: ## Run ShellCheck inside the Nix dev shell (mirrors CI). | |
| @echo "🔍 Running ShellCheck inside the Nix dev shell..." | |
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-check |
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 645-645: Missing required phony target "all"
(minphony)
[warning] 645-645: Missing required phony target "clean"
(minphony)
🤖 Prompt for AI Agents
In Makefile around lines 630 to 648, the shell-check target uses "find ... |
xargs shellcheck", which breaks on filenames with spaces/newlines and when no
matches are found; replace the pipeline with a find -exec form (or use -print0
with xargs -0) so ShellCheck is invoked safely and gracefully when no files
exist, preserving the same exclusion patterns; update shell-check-dev similarly
to mirror the change inside the Nix dev shell.
| # 2. Install Nix | ||
| curl -L https://nixos.org/nix/install | sh -s -- --daemon | ||
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install linux | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Determinate Systems Nix installer documentation official command
💡 Result:
Run this one-liner (Determinate Nix):
curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install --determinate. [1][2]
(If you prefer upstream Nix, omit --determinate: curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install.) [2]
Sources: Determinate Systems installer docs and repo. [1][2]
Use --determinate flag instead of linux in the Nix install command. The correct command is:
curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install --determinate
The current command uses install linux which is not a recognized flag. The --determinate flag is the official, recommended way to install Determinate's Nix flavor.
🤖 Prompt for AI Agents
In named-hosts/kyber/README.md around lines 15 to 17 the Nix install command
uses an incorrect flag ("install linux"); replace that argument with the correct
Determinate installer flag by changing the invocation to use "install
--determinate" so the script receives the proper installation mode; update the
README command accordingly and ensure the example shows the full curl | sh -s --
install --determinate form.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
config/claude/security.sh (4)
11-29: Fail-open safely ifjqis missing / JSON is invalid underset -euo pipefail.
Right nowjqerrors can exit non-zero and unintentionally block all Bash tool usage (availability regression for the hook). Also consider single-pass extraction to avoid parsing stdin twice.set -euo pipefail # Read JSON input from stdin input=$(cat) +# If jq isn't available (or JSON is malformed), don't block tool execution. +command -v jq >/dev/null 2>&1 || exit 0 + # Extract tool name - only process Bash commands -tool_name=$(echo "$input" | jq -r '.tool.name // empty' 2>/dev/null) +tool_name=$(printf '%s' "$input" | jq -r '.tool.name // empty' 2>/dev/null || true) [[ $tool_name != "Bash" ]] && exit 0 # Extract the command to be executed -command=$(echo "$input" | jq -r '.tool.input.command // empty' 2>/dev/null) +command=$(printf '%s' "$input" | jq -r '.tool.input.command // empty' 2>/dev/null || true) [[ -z $command ]] && exit 0 @@ # Read deny patterns from settings -mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null) +mapfile -t deny_patterns < <(jq -r '.permissions.deny[]?' "$settings" 2>/dev/null || true)
33-52: Restoreextglobeven on successful match (state leak).
return 0happens beforeshopt -u extglob, leaving extglob enabled for the rest of the script.matches_pattern() { local cmd="$1" local pattern="$2" @@ - shopt -s extglob + local extglob_was_set=0 matched=1 + shopt -q extglob && extglob_was_set=1 + shopt -s extglob # shellcheck disable=SC2053 - if [[ $cmd == $check_pattern ]]; then - return 0 - fi - shopt -u extglob + [[ $cmd == $check_pattern ]] && matched=0 + (( extglob_was_set == 1 )) || shopt -u extglob + return "$matched" fi return 1 }
54-58: Command splitting doesn’t match the comment (and misses$()/backticks).
The sed regex splits on;,&,|but not on&&/||as claimed, and it won’t surface dangerous commands inside command substitution / backticks / process substitution. Either tighten the comment + document limitations, or implement a more robust parser.
59-63: Don’t usexargsto trim in a security check; it can rewrite the string.
Use a leading/trailing trim that preserves internal whitespace and doesn’t interpret quotes/backslashes.for segment in "${segments[@]}"; do # Trim leading/trailing whitespace - segment=$(echo "$segment" | xargs 2>/dev/null) || continue + segment=$(printf '%s' "$segment" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//') || continue [[ -z $segment ]] && continue
🧹 Nitpick comments (2)
spec/notify_spec.sh (2)
14-18: Avoid flakiness: assert/handle stderr explicitly for “skips local notification”.This test only checks stdout is empty; if
notify.shemits debug/warnings to stderr, it can still “pass” while being noisy (or fail later if you start asserting combined output). Consider asserting stderr is empty too (or explicitly allowing it, if intended).It 'exits early and skips local notification' When run bash "$SCRIPT" <<<'{"message": "Test message"}' The status should be success The output should eq '' +The error should eq '' End
1-3: Narrow/justifyshellcheck disable=SC2329.Disabling SC2329 for the whole file can hide real “unused function” issues. If this is only to appease ShellCheck for ShellSpec DSL, consider scoping it to the specific lines or adding a brief reason comment.
-# shellcheck disable=SC2329 +# shellcheck disable=SC2329 # ShellSpec DSL defines functions dynamically; ShellCheck flags them as unused.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
config/claude/security.sh(1 hunks)home-manager/services/cliproxyapi/start.sh(1 hunks)spec/notify_spec.sh(1 hunks)spec/pushover_spec.sh(1 hunks)spec/security_spec.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- home-manager/services/cliproxyapi/start.sh
- spec/security_spec.sh
- spec/pushover_spec.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{sh,bash}
📄 CodeRabbit inference engine (CLAUDE.md)
Use shfmt with 2-space indentation for shell scripts
**/*.{sh,bash}: Use 2 spaces for indentation in shell scripts
Add proper shebang lines to shell scripts
Follow shellcheck recommendations in shell scripts
Document complex commands in shell scripts
Use consistent variable naming in shell scripts
Files:
spec/notify_spec.shconfig/claude/security.sh
🧠 Learnings (3)
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Follow shellcheck recommendations in shell scripts
Applied to files:
config/claude/security.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Document complex commands in shell scripts
Applied to files:
config/claude/security.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Use consistent variable naming in shell scripts
Applied to files:
config/claude/security.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: lua-neovim-test
- GitHub Check: lua-hammerspoon
- GitHub Check: lua-neovim
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
- GitHub Check: nix-nixos
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: shellspec
- GitHub Check: shellcheck
| Describe 'notify.sh' | ||
| SCRIPT="$PWD/config/claude/notify.sh" | ||
|
|
There was a problem hiding this comment.
Make SCRIPT path robust (don’t rely on $PWD).
SCRIPT="$PWD/config/claude/notify.sh" will break if ShellSpec runs from a different working directory (e.g., CI invoking from spec/, or via a wrapper). Prefer resolving relative to the spec file location.
Describe 'notify.sh'
-SCRIPT="$PWD/config/claude/notify.sh"
+SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)"
+SCRIPT="$SCRIPT_DIR/../config/claude/notify.sh"📝 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.
| Describe 'notify.sh' | |
| SCRIPT="$PWD/config/claude/notify.sh" | |
| Describe 'notify.sh' | |
| SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" | |
| SCRIPT="$SCRIPT_DIR/../config/claude/notify.sh" | |
🤖 Prompt for AI Agents
In spec/notify_spec.sh around lines 4 to 6, the SCRIPT variable uses $PWD which
fails when tests are run from another working directory; replace it with a path
resolved relative to the spec file location (e.g., compute the spec directory
via dirname of the test file and join ../config/claude/notify.sh), using a
robust fallback (use ${BASH_SOURCE[0]} if available else $0) so the script path
is correct regardless of the current working directory.
There was a problem hiding this comment.
Pull request overview
This PR enhances the dotfiles repository with comprehensive shell script testing infrastructure, security improvements for Claude Code, and streamlined SSH access via Tailscale. The changes focus on three key areas: establishing ShellSpec-based testing with CI integration, implementing security hooks to block dangerous bash commands, and migrating Nix installations to the Determinate Systems installer.
Key Changes
- Testing Infrastructure: Adds ShellSpec framework with comprehensive tests for security, notification, and utility scripts, integrated into CI workflow
- Security Enhancements: Implements security.sh hook to block dangerous bash commands before execution, with configurable deny patterns and enhanced file access restrictions
- Tailscale Migration: Simplifies Kyber server SSH access by replacing password-based authentication with Tailscale, adding both standard and zellij-attached connection functions
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/security_spec.sh | New ShellSpec tests for security.sh command blocking functionality |
| spec/pushover_spec.sh | New ShellSpec tests for Pushover notification script behavior |
| spec/notify_spec.sh | New ShellSpec tests for local notification script logic |
| spec/spec_helper.sh | ShellSpec configuration requiring minimum version 0.28.1 |
| spec/support/custom_matcher.sh | Empty custom matcher support file for ShellSpec |
| .shellspec | ShellSpec configuration with bash shell and progress format |
| config/claude/security.sh | New security hook that blocks dangerous bash commands based on deny patterns |
| config/claude/pushover.sh | Updates to skip noisy "other" session end notifications |
| config/claude/notify.sh | Adds shellcheck directive for env file sourcing |
| config/claude/settings.json | Major security configuration changes including deny patterns, file access restrictions, defaultMode change, and removal of go/python permissions |
| config/claude/default.nix | Adds security.sh to home-manager configuration |
| config/cliproxyapi/config.yaml | Updates management API to localhost-only with password injection |
| home-manager/services/cliproxyapi/start.sh | Injects CLIPROXY_MANAGEMENT_PASSWORD into config template |
| home-manager/programs/fish/functions/_kyber_function.fish | Simplifies SSH to use Tailscale instead of password authentication |
| home-manager/programs/fish/functions/_kybers_function.fish | New function for SSH with zellij attachment via Tailscale |
| home-manager/programs/fish/default.nix | Registers new kybers alias and function |
| home-manager/packages/default.nix | Adds shellcheck and shellspec to development packages |
| install.sh | Migrates all Nix installation paths to Determinate Systems installer |
| named-hosts/kyber/README.md | Updates Nix installation command to use Determinate installer |
| README-nix.md | Updates documentation to recommend Determinate Nix installer |
| Makefile | Adds shell-test, shell-check targets and updates Nix installation |
| devenv.nix | Adds shellcheck and shellspec to development environment |
| .github/workflows/shell.yml | New CI workflow for running ShellSpec tests and ShellCheck validation |
| .env.example | Adds CLIPROXY_MANAGEMENT_PASSWORD placeholder |
| flake.lock | Updates nixpkgs and NUR dependency versions |
| home-manager/services/code-syncer/sync.sh | Adds shellcheck disable directives |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| It 'blocks rm -rf /*' | ||
| Data '{"tool": {"name": "Bash", "input": {"command": "rm -rf /*"}}}' | ||
| When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'" | ||
| The status should eq 2 | ||
| The stderr should include 'BLOCKED' | ||
| End |
There was a problem hiding this comment.
The test only checks that status code 2 is returned and stderr includes 'BLOCKED', but it doesn't verify that the command was actually blocked or that the correct error message is produced. Consider adding assertions for the complete error message format to ensure the security script provides useful feedback about which pattern blocked the command.
| Describe 'blocked commands' | ||
| It 'blocks rm -rf /*' | ||
| Data '{"tool": {"name": "Bash", "input": {"command": "rm -rf /*"}}}' | ||
| When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'" | ||
| The status should eq 2 | ||
| The stderr should include 'BLOCKED' | ||
| End | ||
| End |
There was a problem hiding this comment.
The test suite only validates one dangerous command (rm -rf /*) but doesn't test the other patterns defined in the deny list such as sudo, chmod -R 777, mkfs, or dd if=. Consider adding test cases for all the dangerous command patterns to ensure comprehensive security coverage.
| . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh | ||
| NIX_EFFECTIVE_BIN_PATH="/nix/var/nix/profiles/default/bin" | ||
| else # Linux | ||
| if [ "$IN_DOCKER" = "true" ]; then |
There was a problem hiding this comment.
The conditional check for 'IN_DOCKER' uses string comparison with 'true', but IN_DOCKER might not be set at all. If IN_DOCKER is unset, the test will fail with an error when 'set -u' is active (which is common in shell scripts). Consider using parameter expansion like '${IN_DOCKER:-false}' to provide a default value.
| if [ "$IN_DOCKER" = "true" ]; then | |
| if [ "${IN_DOCKER:-false}" = "true" ]; then |
| github_access_token: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Run ShellCheck (Dev Shell) | ||
| run: make shell-check-dev | ||
| shell-check: |
There was a problem hiding this comment.
The job name 'shell-check' is inconsistent with the actual jobs defined above ('shellspec' and 'shellcheck'). While this is just the final status check job, the naming could be clearer - consider renaming to 'shell-tests-status' or similar to better indicate it's an aggregation of the shell testing jobs.
| shell-check: | |
| shell-tests-status: |
| @@ -1,4 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
| # shellcheck disable=SC2034,SC2155,SC2181,SC2162 | |||
There was a problem hiding this comment.
The blanket shellcheck disable directive (SC2034,SC2155,SC2181,SC2162) disables multiple important checks for the entire file. While sometimes necessary, this approach can hide legitimate issues. Consider using more targeted disable directives on specific lines where these warnings are unavoidable, or addressing the underlying issues flagged by shellcheck.
| # All management requests (even from localhost) require this key. | ||
| # Leave empty to disable the Management API entirely (404 for all /v0/management routes). | ||
| secret-key: "" | ||
| secret-key: "__CLIPROXY_MANAGEMENT_PASSWORD__" |
There was a problem hiding this comment.
The placeholder CLIPROXY_MANAGEMENT_PASSWORD is being used for the secret-key, but if the CLIPROXY_MANAGEMENT_PASSWORD environment variable is empty or unset, this will result in an empty secret-key in the config. According to the comment on line 10, an empty secret-key disables the Management API entirely (404 for all /v0/management routes). Consider whether this is the intended behavior, or if the service should fail to start when the password is not provided for security reasons.
| }, | ||
| "model": "sonnet" | ||
| "model": "sonnet", | ||
| "defaultMode": "bypassPermissions" |
There was a problem hiding this comment.
Setting "defaultMode" to "bypassPermissions" means Claude Code will bypass permission checks by default. This effectively disables the security restrictions defined in the permissions section (allow/deny lists, read/write deny patterns) unless explicitly changed. Consider whether this is the intended security posture, as it reduces the protection provided by the carefully configured permissions.
| "defaultMode": "bypassPermissions" | |
| "defaultMode": "enforcePermissions" |
| Describe 'blocked commands' | ||
| It 'blocks rm -rf /*' | ||
| Data '{"tool": {"name": "Bash", "input": {"command": "rm -rf /*"}}}' | ||
| When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'" | ||
| The status should eq 2 | ||
| The stderr should include 'BLOCKED' | ||
| End | ||
| End | ||
|
|
||
| Describe 'edge cases' | ||
| It 'passes with empty input' | ||
| Data '{"tool": {"name": "Bash", "input": {}}}' | ||
| When run bash -c "HOME='$TEMP_HOME' bash '$SCRIPT'" | ||
| The status should be success | ||
| End | ||
| End | ||
| End |
There was a problem hiding this comment.
There are no test cases for the command splitting logic (line 57 in security.sh) that handles commands with logical operators like semicolons, pipes, and logical AND/OR. Consider adding tests for commands like "ls ; rm -rf /*" or "echo test && sudo rm -rf /" to ensure the security script properly detects dangerous commands even when they're chained with safe commands.
| set -eu | ||
|
|
||
| spec_helper_precheck() { | ||
| minimum_version "0.28.1" |
There was a problem hiding this comment.
The spec helper requires ShellSpec version 0.28.1 as a minimum, but the CI workflow and Makefile don't verify this version is installed. If an older version is present, tests may fail with unclear error messages. Consider documenting this requirement in the README or adding a version check in the test setup.
| minimum_version "0.28.1" | |
| required_version="0.28.1" | |
| installed_version="$(shellspec --version 2>/dev/null | awk '{print $2}')" | |
| if [ -z "$installed_version" ]; then | |
| echo "Error: ShellSpec is not installed or not found in PATH." >&2 | |
| exit 1 | |
| fi | |
| # Compare versions | |
| if [ "$(printf '%s\n' "$required_version" "$installed_version" | sort -V | head -n1)" != "$required_version" ]; then | |
| echo "Error: ShellSpec version $required_version or higher is required. Found: $installed_version" >&2 | |
| exit 1 | |
| fi |
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "cleanupPeriodDays": 99999, | |||
There was a problem hiding this comment.
Setting cleanupPeriodDays to 99999 effectively disables automatic cleanup of old sessions (approximately 273 years). While this preserves history, it could lead to unbounded storage growth over time. Consider whether this is intentional for debugging purposes or if a more reasonable retention period (e.g., 90-180 days) would be more appropriate for production use.
| "cleanupPeriodDays": 99999, | |
| "cleanupPeriodDays": 180, |
…ial handling test
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
Makefile (2)
137-138:switchtarget hard-depends onlaunchctl(breaks non-macOS).This matches prior review feedback:
make switchwill fail on Linux/CI due tolaunchctlbeing macOS-only.
640-644:shell-checkusesfind | xargs(spaces/newlines + empty set).This matches prior review feedback: prefer
find ... -exec shellcheck {} +(or-print0 | xargs -0) for robustness. Based on learnings, follow ShellCheck recommendations.
🧹 Nitpick comments (1)
Makefile (1)
630-634: Simplifyshell-testinvocation (avoid extrabash -c).Unless you specifically need
bash -c,@shellspecis typically sufficient and avoids an extra shell hop.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
Makefile(3 hunks)overlays/default.nix(1 hunks)spec/notify_spec.sh(1 hunks)spec/pushover_spec.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- spec/pushover_spec.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- spec/notify_spec.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.nix
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.nix: Use nixfmt for formatting all Nix files
Document complex configurations with comments in Nix files
**/*.nix: Use 2 spaces for indentation in Nix files
Keep line length under 100 characters in Nix files
Sort attribute sets alphabetically in Nix files
Use consistent spacing around operators in Nix files
Format lists and sets consistently in Nix filesUse treefmt.toml for formatting Nix files
**/*.nix: UsemkOptionfor configurable options in Nix modules
Implement proper typing for all options in Nix modules
Follow the Nix expression language style guide
Files:
overlays/default.nix
**/default.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Use
default.nixfiles for module exports
Files:
overlays/default.nix
🧠 Learnings (12)
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Follow shellcheck recommendations in shell scripts
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Use consistent variable naming in shell scripts
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Document complex commands in shell scripts
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Add proper shebang lines to shell scripts
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:23.224Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:34:23.224Z
Learning: Applies to **/*.{sh,bash} : Use shfmt with 2-space indentation for shell scripts
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Use 2 spaces for indentation in shell scripts
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.nix : Use consistent spacing around operators in Nix files
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Test Nix and home-manager configurations locally before pushing using `make test`
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Test configurations before committing using `nix flake check` and `home-manager build --show-trace`
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Applies to flake.nix : Maintain flake.nix as the main Nix configuration with proper structure
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : CI Pipeline must run on all pull requests and main branch pushes, validating Nix flake checks, home-manager builds, nix-darwin builds, and system configurations
Applied to files:
Makefile
📚 Learning: 2025-11-25T09:34:47.854Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/github-workflows.mdc:0-0
Timestamp: 2025-11-25T09:34:47.854Z
Learning: Applies to .github/workflows/*.yml : Run Biome for code formatting and validate Nix expressions, commit message format, and documentation updates in code quality checks
Applied to files:
Makefile
🪛 checkmake (0.2.2)
Makefile
[warning] 645-645: Missing required phony target "all"
(minphony)
[warning] 645-645: Missing required phony target "clean"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: nix-nixos
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
- GitHub Check: shellcheck
- GitHub Check: shellspec
- GitHub Check: lua-neovim-test
- GitHub Check: lua-hammerspoon
- GitHub Check: lua-neovim
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
🔇 Additional comments (1)
Makefile (1)
140-142:testnow includesshell-test—nice CI alignment.The target wiring looks consistent with adding ShellSpec-based coverage.
| echo "🚀 Installing Determinate Nix environment for $(NIX_CONFIG_TYPE) on $(OS) $(ARCH) for USER=$(NIX_USERNAME)"; \ | ||
| curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install; \ | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Determinate Nix installer official documentation recommended installation command
💡 Result:
Recommended one‑liner (from Determinate official docs):
curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install --determinate
Sources: Determinate manual and installer repo. [1][2]
References
[1] Determinate Nix Quick Start / Introduction.
[2] Determinate Systems — nix-installer (GitHub / project page).
🌐 Web query:
Determinate Nix installer checksum signature verification security
💡 Result:
Short answer
- The Determinate Nix installer is distributed over HTTPS from install.determinate.systems and its source is public on GitHub (DeterminateSystems/nix-installer). That gives TLS protection and an auditable source tree, but the common one‑liner (curl … | sh) does not, by itself, perform an independent checksum/GPG signature verification step in the client-side flow. [1][2]
What that means for security
- TLS (HTTPS) protects against network interception/modification if you trust the CA and DNS; it does not protect you from a compromised server, supply‑chain compromise, or a malicious push to the repo. [2]
- The project’s repo and releases are publicly auditable, so the strongest protections are: pinning to a specific, reviewed release (or commit) and verifying that release in ways you control (release artifacts, checksums, or Git tag signatures) before running installers. [1]
Practical verification / hardening steps
- Pin a specific installer release or commit instead of using the floating one‑liner. Example: download a pinned tag from the repo/releases or the installer URL that includes a version. Then verify the content locally. [1]
- Verify the downloaded installer binary/script: check its SHA256 against a checksum you obtained from a trusted channel (release page, reproducible CI artifact) or verify a signed Git tag if the project provides one. If no signed checksum exists, prefer downloading the source and building/inspecting it locally. [1]
- Inspect the installer script before running it (curl … -o nix-installer && less ./nix-installer) and run it only after review. Use sh -x or run in a contained environment (VM/container) first. [1][4]
- Use reproducible/pinned Nix packages (pin nixpkgs or use Determinate Nix’s pinned flake outputs) so post-install provenance is bounded. Determinate docs encourage pinning and provide flake/module outputs. [2][6]
- Prefer platform packages (e.g., Determinate.pkg on macOS) or official release artifacts over the network one‑liner when possible. [2]
If you need absolute integrity guarantees
- Ask the maintainers for cryptographic checksums and detached signatures, or check whether GitHub release artifacts are signed by a maintainer GPG key you trust. If those aren’t available, you must rely on TLS + pinning + review or build-from-source for higher assurance. (I couldn’t find a documented detached signature or signed checksum for the installer on the public pages I checked.) [1][2]
Sources
- DeterminateSystems/nix-installer (GitHub repo, README — installer, pinning, usage notes). [1]
- Determinate Systems documentation and install pages (install.determinate.systems, docs.determinate.systems — installer over HTTPS, recommended install flows, macOS package recommendation). [2][6]
If you want, I can: (a) check a specific release page for release artifacts/checksums/signatures, (b) show the exact curl+verify commands to pin and verify SHA256, or (c) walk you through verifying a downloaded installer on your machine. Which would you like?
Add --determinate flag to the Nix installer invocation.
The Determinate official documentation recommends: curl --proto '=https' --tlsv1.2 -sSf -L https://install.determinate.systems/nix | sh -s -- install --determinate. The Makefile command is missing the --determinate flag, which enables Determinate's managed/pinned installation mode. Update line 206 to include this flag.
Note: The curl | sh pattern is Determinate's own recommended invocation. The HTTPS and TLS settings are appropriate. Cryptographic checksum or signature verification is not offered as part of Determinate's standard one-liner; if supply-chain hardening is required, custom pinning and verification steps would need to be added separately.
🤖 Prompt for AI Agents
Makefile around lines 205 to 207: the Determinate Nix installer invocation is
missing the --determinate flag; update the curl | sh command on line 206 to pass
--determinate to the installer (i.e., the sh -s -- install invocation should
include --determinate) so the installer runs in Determinate's managed/pinned
mode.
| .PHONY: shell-check-dev | ||
| shell-check-dev: ## Run ShellCheck inside the Nix dev shell (mirrors CI). | ||
| @echo "🔍 Running ShellCheck inside the Nix dev shell..." | ||
| @DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) shell-check |
There was a problem hiding this comment.
checkmake warns missing all / clean phony targets—add minimal stubs if you care about that policy.
If you want to satisfy checkmake’s minphony, consider adding:
+.PHONY: all
+all: help ## Default “all” target.
+
+.PHONY: clean
+clean: ## No-op clean (dotfiles repo).
+ @trueCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 checkmake (0.2.2)
[warning] 645-645: Missing required phony target "all"
(minphony)
[warning] 645-645: Missing required phony target "clean"
(minphony)
🤖 Prompt for AI Agents
In Makefile around lines 645 to 648, checkmake warns that `all` and `clean` are
missing as phony targets; add minimal stub targets named `all` and `clean` and
mark them phony so the policy is satisfied. Create simple no-op implementations
(e.g., a brief echo or a `true`/`:` command) for each target and ensure they are
listed in the .PHONY declaration so they don’t conflict with files of the same
name.
| (final: prev: { | ||
| # Fix shellspec wrapper script that breaks when called via symlinks | ||
| shellspec = prev.shellspec.overrideAttrs (oldAttrs: { | ||
| postInstall = (oldAttrs.postInstall or "") + '' | ||
| # Replace the wrapper with one that uses an absolute path | ||
| cat > $out/bin/shellspec << EOF | ||
| #!${prev.bash}/bin/sh | ||
| exec "$out/lib/shellspec/shellspec" "\$@" | ||
| EOF | ||
| chmod +x $out/bin/shellspec | ||
| ''; | ||
| }); | ||
| }) |
There was a problem hiding this comment.
Fix the generated shellspec wrapper (indented heredoc breaks the shebang).
Because the heredoc content is indented in the Nix string, the produced file will likely start with spaces before #!, causing execution failures.
Suggested patch (also switches to a clearer shebang):
(final: prev: {
# Fix shellspec wrapper script that breaks when called via symlinks
shellspec = prev.shellspec.overrideAttrs (oldAttrs: {
postInstall = (oldAttrs.postInstall or "") + ''
- # Replace the wrapper with one that uses an absolute path
- cat > $out/bin/shellspec << EOF
- #!${prev.bash}/bin/sh
- exec "$out/lib/shellspec/shellspec" "\$@"
- EOF
- chmod +x $out/bin/shellspec
+ # Replace the wrapper with one that uses an absolute path
+ real="$out/lib/shellspec/shellspec"
+ printf '%s\n' \
+ '#!${prev.runtimeShell}' \
+ "exec \"$real\" \"\$@\"" \
+ > "$out/bin/shellspec"
+ chmod +x "$out/bin/shellspec"
'';
});
})As per coding guidelines, keep Nix formatting consistent and prefer unambiguous wrapper generation.
📝 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.
| (final: prev: { | |
| # Fix shellspec wrapper script that breaks when called via symlinks | |
| shellspec = prev.shellspec.overrideAttrs (oldAttrs: { | |
| postInstall = (oldAttrs.postInstall or "") + '' | |
| # Replace the wrapper with one that uses an absolute path | |
| cat > $out/bin/shellspec << EOF | |
| #!${prev.bash}/bin/sh | |
| exec "$out/lib/shellspec/shellspec" "\$@" | |
| EOF | |
| chmod +x $out/bin/shellspec | |
| ''; | |
| }); | |
| }) | |
| (final: prev: { | |
| # Fix shellspec wrapper script that breaks when called via symlinks | |
| shellspec = prev.shellspec.overrideAttrs (oldAttrs: { | |
| postInstall = (oldAttrs.postInstall or "") + '' | |
| # Replace the wrapper with one that uses an absolute path | |
| real="$out/lib/shellspec/shellspec" | |
| printf '%s\n' \ | |
| '#!${prev.runtimeShell}' \ | |
| "exec \"$real\" \"\$@\"" \ | |
| > "$out/bin/shellspec" | |
| chmod +x "$out/bin/shellspec" | |
| ''; | |
| }); | |
| }) |
🤖 Prompt for AI Agents
In overlays/default.nix around lines 21 to 33, the indented heredoc in
postInstall will write leading spaces before the shebang (breaking execution);
remove the indentation so the heredoc marker and its contents start in column 1
and use an unquoted EOF (so ${prev.bash} is expanded at build time) to write a
clean wrapper whose first line is exactly #!${prev.bash}/bin/sh, then set
executable permissions as before; ensure the heredoc terminator EOF is also at
column 1 so no extra whitespace is introduced.
Summary by cubic
Switch Kyber SSH to Tailscale, lock down management access, and add a Bash security hook for Claude Code. Also introduce ShellSpec/ShellCheck in CI and switch Nix installation to Determinate Nix.
Security
New Features
Written for commit 5ec2977. Summary will update automatically on new commits.