fix kolide - #1475
Conversation
Place StartLimitIntervalSec under unitConfig so systemd reads the directive instead of warning about an unknown [Service] key. Co-authored-by: Codex <noreply@openai.com>
Keep falcon-sensor running across nixos-rebuild switch so vendor processes do not block activation. Add gsettings to Kolide's PATH and order Kolide after Falcon so the CrowdStrike compliance check has the tools and startup sequencing it expects on NixOS. Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request adds infrastructure improvements, enhanced error handling in build scripts, introduces new automation targets in the Makefile, updates NixOS host configurations for the matic node (enabling fingerprint authentication and system services), adds a new Fish function for LLM interactions, and introduces corresponding test specifications for new shell utilities. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRA collection of fixes and minor features across various system configurations, including 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 pull request introduces dynamic build targets for named hosts and ISOs in the Makefile, a new ISO discovery script, and a Python dispatcher for isolated uv tools. It also refactors llm-update.sh for atomic updates and updates service configurations for falcon-sensor and kolide-launcher. Feedback identifies portability and logic issues in the Python dispatcher, suggests ensuring directory existence for symlinks, and notes that dynamic targets are missing from the make help output.
| # Write a dispatcher so `python3 -m <tool>` uses that tool's isolated Python | ||
| cat > "${HOME}/.local/bin/python3" << 'EOF' | ||
| #!/usr/bin/env bash | ||
| prev="" | ||
| for arg in "$@"; do | ||
| if [ "$prev" = "-m" ]; then | ||
| per_tool="${HOME}/.local/bin/python3-${arg}" | ||
| if [ -x "$per_tool" ]; then | ||
| exec "$per_tool" "$@" | ||
| fi | ||
| break | ||
| fi | ||
| prev="$arg" | ||
| done | ||
| exec /etc/profiles/per-user/"${USER}"/bin/python3 "$@" | ||
| EOF |
There was a problem hiding this comment.
The dispatcher logic has two main issues:
- Portability: The fallback path
/etc/profiles/per-user/"${USER}"/bin/python3is NixOS-specific and will fail on other platforms (like macOS, which the Makefile suggests is supported). - Correctness: The loop through all arguments might incorrectly trigger if a script being run happens to have a
-margument (e.g.,python3 script.py -m arg).
A more robust approach is to check for -m as the first argument and use a portable way to find the 'real' python3 by temporarily excluding the wrapper's directory from PATH.
| # Write a dispatcher so `python3 -m <tool>` uses that tool's isolated Python | |
| cat > "${HOME}/.local/bin/python3" << 'EOF' | |
| #!/usr/bin/env bash | |
| prev="" | |
| for arg in "$@"; do | |
| if [ "$prev" = "-m" ]; then | |
| per_tool="${HOME}/.local/bin/python3-${arg}" | |
| if [ -x "$per_tool" ]; then | |
| exec "$per_tool" "$@" | |
| fi | |
| break | |
| fi | |
| prev="$arg" | |
| done | |
| exec /etc/profiles/per-user/"${USER}"/bin/python3 "$@" | |
| EOF | |
| # Write a dispatcher so python3 -m <tool> uses that tool's isolated Python | |
| cat > "${HOME}/.local/bin/python3" << 'EOF' | |
| #!/usr/bin/env bash | |
| # If the first argument is -m, check if we have an isolated tool for it | |
| if [[ "$1" == "-m" && -n "$2" ]]; then | |
| per_tool="${HOME}/.local/bin/python3-$2" | |
| if [[ -x "$per_tool" ]]; then | |
| exec "$per_tool" "$@" | |
| fi | |
| fi | |
| # Fallback to the real python3 (excluding this wrapper from PATH) | |
| REAL_PYTHON3=$(PATH=$(echo "$PATH" | tr ':' '\n' | grep -vx "${HOME}/.local/bin" | paste -sd: -) command -v python3) | |
| if [[ -n "$REAL_PYTHON3" ]]; then | |
| exec "$REAL_PYTHON3" "$@" | |
| else | |
| echo "python3 not found in PATH (excluding wrapper)" >&2 | |
| exit 1 | |
| fi | |
| EOF |
| @echo "Usage: make <target>" | ||
| @echo | ||
| @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z0-9_-]+:.*?## / {printf " \033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | ||
| @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z0-9_%-]+:.*?## / {printf " \033[36m%-20s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) |
There was a problem hiding this comment.
The help target parses $(MAKEFILE_LIST) (the files on disk) using awk. Since the new host-specific targets (e.g., build-matic) are generated dynamically using eval (lines 656-657), they do not exist as literal lines in the Makefile and will not be captured by this awk command. Consequently, they won't appear in the make help output. If you want them to be discoverable, consider using pattern rules (e.g., build-%:) instead of eval, or manually adding a help entry.
| # Symlink each tool's python3 for per-tool access: `python3-<tool> -m <tool>` | ||
| tool_python="${HOME}/.local/share/uv/tools/${name}/bin/python3" | ||
| if [ -f "$tool_python" ]; then | ||
| ln -sf "$tool_python" "${HOME}/.local/bin/python3-${name}" | ||
| fi |
There was a problem hiding this comment.
Ensure the target directory ${HOME}/.local/bin exists before attempting to create symlinks. Also, consider removing 2>/dev/null from the uv tool install command (line 64) to allow error messages to be visible if a tool fails to install, which is helpful for troubleshooting.
| # Symlink each tool's python3 for per-tool access: `python3-<tool> -m <tool>` | |
| tool_python="${HOME}/.local/share/uv/tools/${name}/bin/python3" | |
| if [ -f "$tool_python" ]; then | |
| ln -sf "$tool_python" "${HOME}/.local/bin/python3-${name}" | |
| fi | |
| # Symlink each tool's python3 for per-tool access: python3-<tool> -m <tool> | |
| mkdir -p "${HOME}/.local/bin" | |
| tool_python="${HOME}/.local/share/uv/tools/${name}/bin/python3" | |
| if [ -f "$tool_python" ]; then | |
| ln -sf "$tool_python" "${HOME}/.local/bin/python3-${name}" | |
| fi |
1 participant
Summary by cubic
Stabilizes Kolide and CrowdStrike Falcon on
maticby fixing service ordering and start limits, and hardens ISO and LLM update tooling. Adds small UX improvements: Battery Settings entry, fingerprint auth, betteruvglobals, and host build shortcuts.Bug Fixes
restartIfChanged=falseto prevent flapping duringnixos-rebuild switch.dpkgandglibto servicePATH; start afterfalcon-sensor.servicewhen installed to avoid boot races.jq,sed, etc.); add tests.hyprpolkitagentfrom/libexec.scripts/find-built-iso.shand integrate intobuild-iso; add tests.New Features
rofi“Battery Settings” forauto-cpufreq-gtkand provision/opt/auto-cpufreq.polkit-1PAM.uvglobals idempotent with per-toolpython3-<tool>and apython3dispatcher; add_pixelh_functionfish helper for local Qwen viapi.build-<host>andbuild-<host>-isotargets formaticandviper.Written for commit 2ce2be6. Summary will update on new commits.