Bootstrap cortex linux distro hybrid ISO (Live boot + Full installer mode) #10
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds comprehensive Cortex branding (assets, themes, packaging), reorganizes build tooling into script-driven commands and CI multi-arch jobs, introduces preseeds and live-build hooks, and provides an idempotent, network-aware first-boot provisioning service/script alongside developer documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Systemd as systemd
participant Unit as cortex-first-boot.service
participant Script as /opt/cortex/provisioning/first-boot.sh
participant Network as Network
participant APT as APT
participant SSHD as sshd
participant System as System
Systemd ->> Unit: start (ConditionPathExists)
Unit ->> Script: ExecStart
Script ->> System: check .first-boot-complete
alt not completed
Script ->> Network: probe connectivity
Network -->> Script: reachable / unreachable
alt reachable
Script ->> APT: add repo, apt update/upgrade
APT -->> Script: success/fail
end
Script ->> SSHD: generate/verify host keys, apply SSH hardening
Script ->> System: configure users, sudo, firewall, fail2ban, unattended-upgrades
Script ->> System: cleanup installer artifacts
Script ->> System: write .first-boot-complete
Script ->> Systemd: disable/stop first-boot unit
else already completed
Script -->> Unit: exit
end
Script -->> Systemd: exit status
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Summary of ChangesHello @Anshgrover23, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a foundational build system for Cortex Linux, enabling the creation of diverse ISO images tailored for various use cases, from minimal servers to security-hardened desktops. It standardizes the distribution's visual identity through comprehensive branding and streamlines the post-installation experience with automated provisioning, significantly enhancing the development and deployment workflow. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 impressive and comprehensive pull request that bootstraps an entire Linux distribution build system. The structure is well-thought-out, with a powerful Makefile, support for different profiles, and a clear separation of concerns for branding, configuration, and documentation. My review focuses on improving the robustness and security of the build and provisioning process. I've identified a few areas where error handling can be improved to make the build process more reliable, and a critical security issue related to the handling of the default passphrase for full-disk encryption that needs to be addressed.
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot
Outdated
Show resolved
Hide resolved
iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @branding/gdm/gdm-branding.conf:
- Around line 1-8: The GDM config references a non-existent logo path
'/usr/share/cortex/logos/cortex-logo-light.svg'; update the login-screen logo
setting to the actual filename 'cortex-logo.svg' and ensure the asset is
installed to that path by either modifying the install-branding.sh script to
copy 'branding/gdm/cortex-logo.svg' into '/usr/share/cortex/logos/' (creating
the directory if needed) or change the config to point to wherever
install-branding.sh currently places the logo; update the value in
[org/gnome/login-screen] (logo=) and adjust install-branding.sh accordingly so
the file and path match.
- Around line 20-28: The GDM config sets icon-theme='cortex-icons' in the
[org/gnome/desktop/interface] section but that theme isn’t provided; either
remove that cortex-icons entry or install/ship the cortex-icons theme and ensure
it’s referenced identically in the live session hooks (the
live/99-configure-gnome.hook.chroot usage that currently uses
icon-theme='Adwaita'); update the GDM key icon-theme='cortex-icons' to
icon-theme='Adwaita' if you prefer to align with the live session, or add the
cortex-icons theme package and update both GDM and live session hooks to use it.
In @docs/DEVELOPERS.md:
- Around line 9-15: The documented live-build version requirement is invalid;
update the version string in the DEVELOPER doc block where the live-build
install instructions and the "Verify live-build version" check appear so it
references a real release for your supported distro (e.g., change "live-build >=
1:20210814" to a valid version such as "live-build >= 1:20230502" for bookworm+
or to the specific minimum available in your minimum-supported release); ensure
the comment and the dpkg verification line reflect the chosen valid version.
In @docs/QUICK-START.md:
- Around line 54-63: The example commands in the docs use incorrect flags;
replace `cortex-upgrade --check` with the supported `cortex-upgrade plan` (and
mention the valid subcommands: plan, execute, rollback, status) and change
`cortex-verify --quick` to just `cortex-verify` (or note supported output flags
like `--json`) while leaving `cortex-gpu status` as-is; update the README
examples and any occurrences of `cortex-upgrade --check` and `cortex-verify
--quick` to the correct command forms (`cortex-upgrade plan` and
`cortex-verify`) and optionally add a brief note about `--json` output for
`cortex-verify`.
In @iso/live-build/config/includes.binary/boot/grub/themes/cortex/theme.txt:
- Line 12: Remove the stray '+' prefix from GRUB theme block declarations (e.g.,
boot_menu) so block names are bare identifiers followed by their opening brace;
locate each block declaration that begins with '+' and delete the '+' character
so the syntax matches "block_name { ... }" for all theme blocks.
In @iso/live-build/config/includes.chroot/boot/grub/themes/cortex/theme.txt:
- Around line 11-32: The theme file contains literal '+' characters before block
names which breaks GRUB parsing; remove the leading '+' characters from the
block definitions such as boot_menu, every label block, and progress_bar so the
blocks read e.g. "boot_menu {", "label {", and "progress_bar {" instead of "+
boot_menu {" etc.; update any occurrences (e.g., boot_menu, label, progress_bar)
throughout theme.txt so all block headers start without '+' and ensure
spacing/indentation remains valid for the GRUB theme parser.
In
@iso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/cortex.script:
- Around line 98-137: display_normal_callback may call password_dialogue_opacity
before globals set by password_dialogue_setup, causing null-derefs; modify
password_dialogue_opacity to guard uses of global.password_entry,
global.password_prompt, and iterate only if global.password_bullets is non-null
(or initialize those globals to sensible defaults at top-level), e.g., check
each global is defined before calling .sprite or looping, and return early if
none exist so display_normal_callback can safely call it.
In @iso/preseed/profiles/cortex-full.preseed:
- Around line 23-30: The preseed currently sets a deterministic/placeholder
password via the debconf key d-i passwd/user-password-crypted (and related keys
like d-i passwd/username and d-i user-setup/allow-password-weak), which creates
a predictable backdoor; remove the hardcoded d-i passwd/user-password-crypted
and user-setup/allow-password-weak lines so the installer prompts for a password
interactively, or convert the value into a required build-time substitution
variable (fail the build if unset) and document the substitution; ensure any
CI/build script that generates this file validates the substitution and rejects
commits that still contain the placeholder.
In @iso/preseed/profiles/cortex-secops.preseed:
- Around line 47-51: The preseed embeds a static full-disk-encryption passphrase
via the partman-crypto/passphrase and partman-crypto/passphrase-again entries,
which exposes all builds; remove these hardcoded values and instead require
interactive entry or generate a unique passphrase per ISO build and display it
once during build output; alternatively disable automated FDE in the preseed
(remove partman-crypto/* passphrase keys and related auto-enable flags) and
document manual FDE setup, and if you implement per-build generation use the
REQUIRE_FDE_PASSPHRASE_CHANGE variable only for enforcement and ensure build
tooling injects the secret securely (not committed in the preseed).
In @packages/cortex-branding/DEBIAN/postinst:
- Around line 10-25: Postinst sets GRUB_THEME in /etc/default/grub but prerm
does not restore it, leaving a broken GRUB_THEME pointing to a removed file;
modify postinst to save the previous GRUB_THEME value (or empty marker) to
/var/lib/cortex-branding/grub-theme.prev before writing the new GRUB_THEME and
modify prerm to read /var/lib/cortex-branding/grub-theme.prev and either restore
the original GRUB_THEME line in /etc/default/grub (if a previous value exists)
or remove the GRUB_THEME line entirely (if marker indicates none), then run
update-grub (or update-grub 2>/dev/null || true) to apply changes; reference the
scripts postinst and prerm, the GRUB_THEME entry in /etc/default/grub, and the
state file /var/lib/cortex-branding/grub-theme.prev when implementing.
In @packages/cortex-branding/DEBIAN/prerm:
- Around line 6-21: The prerm currently calls "plymouth-set-default-theme -R"
which only rebuilds initrd for the current theme; change the flow so the package
either saves the prior theme in postinst (e.g., write the original theme name to
a persistent state file) and then prerm reads that state and restores it, or
replace "-R" with "-r -R" to reset to system default and rebuild; likewise, stop
indiscriminately chmod'ing all /etc/update-motd.d/??-* scripts — have postinst
record exactly which MOTD scripts this package disabled (e.g., write their
filenames to a state file) and make prerm iterate only over that recorded list
to re-enable (chmod +x) those files, leaving other scripts untouched (refer to
the postinst logic that disables non-cortex scripts in lines 34–39 for exact
criteria).
In @packages/cortex-core/DEBIAN/control:
- Around line 1-20: The control file's Depends field lists non-existent packages
causing install failures; open the DEBIAN control and update the Depends line to
reference only existing packages (keep cortex-archive-keyring), remove or
replace cortex-upgrade and cortex-verify with their correct package names if
they exist, and replace cortex-gpu with the actual GPU package(s) (e.g.,
cortex-gpu-amd and/or cortex-gpu-nvidia) or change it to a Suggests/Provides if
it’s meant to be virtual; ensure the Depends field remains a comma-separated
list and adjust Recommends/Suggests accordingly if you move packages out of
Depends.
🟠 Major comments (18)
branding/motd/99-cortex-footer-14-24 (1)
14-24: Verify that MOTD tips reference only universally available commands.The MOTD is installed system-wide for all installations, but several referenced commands are either missing or conditionally available:
cortex— no implementation found in codebasecortex-update— not found (only20-cortex-updatesscript exists)lynis— only included in thecortex-secopsprofile, not in base installationsConsider either implementing these commands, making the MOTD profile-aware, or replacing these tips with universally available alternatives.
branding/motd/10-cortex-sysinfo-17-67 (1)
17-67: Fix fallback correctness forDISTRO/IP_ADDR/USERSand guard numeric comparisons.Right now, several
|| echo ...fallbacks won’t trigger because the last command in the pipeline (cut/awk/wc) commonly exits 0 even when the “real” source command failed or produced empty output.Proposed diff
--- a/branding/motd/10-cortex-sysinfo +++ b/branding/motd/10-cortex-sysinfo @@ HOSTNAME=$(hostname) -DISTRO=$(grep PRETTY_NAME /etc/os-release 2>/dev/null | cut -d'"' -f2 || echo "Cortex Linux") +DISTRO=$( + awk -F= '/^PRETTY_NAME=/{gsub(/^"|"$/, "", $2); print $2; exit}' /etc/os-release 2>/dev/null +) +[ -n "$DISTRO" ] || DISTRO="Cortex Linux" @@ -IP_ADDR=$(hostname -I 2>/dev/null | awk '{print $1}' || echo "N/A") +IP_ADDR="$(hostname -I 2>/dev/null | awk '{print $1}')" +[ -n "$IP_ADDR" ] || IP_ADDR="N/A" @@ -USERS=$(who 2>/dev/null | wc -l || echo "0") +USERS="$(who 2>/dev/null | wc -l 2>/dev/null)" +case "$USERS" in (''|*[!0-9]*) USERS=0 ;; esac @@ mem_color() { - if [ "$1" -lt 50 ]; then echo -e "${GREEN}"; - elif [ "$1" -lt 80 ]; then echo -e "${YELLOW}"; + case "$1" in (''|*[!0-9]*) set -- 0 ;; esac + if [ "$1" -lt 50 ]; then echo -e "${GREEN}"; + elif [ "$1" -lt 80 ]; then echo -e "${YELLOW}"; else echo -e "${RED}"; fi } @@ disk_color() { - if [ "$1" -lt 70 ]; then echo -e "${GREEN}"; - elif [ "$1" -lt 90 ]; then echo -e "${YELLOW}"; + case "$1" in (''|*[!0-9]*) set -- 0 ;; esac + if [ "$1" -lt 70 ]; then echo -e "${GREEN}"; + elif [ "$1" -lt 90 ]; then echo -e "${YELLOW}"; else echo -e "${RED}"; fi }iso/preseed/partitioning/simple-uefi.preseed-10-12 (1)
10-12: Installer robustness: avoid hardcoding/dev/sda, add explicit ESP mountpoint, and clarify the EFI debconf key.
/dev/sdawill break on NVMe, virtio, and other storage types. Usepartman/early_commandto setpartman-auto/diskdynamically (e.g.,list-devices disk | head -n1), or reference stable/dev/disk/by-idor/dev/disk/by-pathinstead.- Add explicit
mountpoint{ /boot/efi }to the ESP recipe (lines 20–26) for clarity and to avoid relying on undocumented partman defaults.- The name
partman-efi/non_efi_systemis counterintuitive (setting ittruemeans "force UEFI-only"), but your usage is correct; document it locally for maintainers.Same issues apply to all UEFI preseed variants: fde-uefi, lvm-uefi, raid1-uefi, and simple-uefi.
iso/preseed/partitioning/fde-uefi.preseed-33-34 (1)
33-34: Security concern: Hardcoded LUKS passphrase in version-controlled file.The plaintext passphrase
cortex-temp-passphraseis committed to the repository. While comments indicate it should be changed at first boot, this creates risk:
- Users may forget or skip the passphrase change step
- The default passphrase becomes publicly known if the repo is public
- Automated scanners may flag this as a credential leak
Consider using a preseed hook to prompt for passphrase entry, or document this very prominently in installation guides.
iso/preseed/partitioning/raid1-uefi.preseed-62-67 (1)
62-67: Missing LVM logical volume definitions inside RAID.The preseed defines the RAID arrays and volume group
cortex-vg, but doesn't include the logical volume definitions for root (20GB), swap (8GB), and home (remaining) as described in the header comments (lines 10-13). Without these, the installation will fail or produce unexpected results.Compare with
lvm-uefi.preseedlines 44-61 which properly define the LV layout within the expert_recipe.packages/cortex-branding/DEBIAN/postinst-26-40 (1)
26-40: Track which MOTD scripts you disable (avoid blanket chmod behavior on uninstall).
The loop disables “non-cortex” scripts (Lines 34-39) but you don’t record which ones changed—soprermcan’t safely revert without re-enabling everything.Proposed change (write disabled list for prerm)
if [ -d /etc/update-motd.d ]; then @@ # Disable default Ubuntu/Debian MOTD scripts + mkdir -p /var/lib/cortex-branding + : > /var/lib/cortex-branding/motd-disabled.list for f in /etc/update-motd.d/??-* ; do case "$(basename $f)" in *cortex*) ;; # Keep our scripts - *) chmod -x "$f" 2>/dev/null || true ;; + *) + chmod -x "$f" 2>/dev/null || true + printf '%s\n' "$f" >> /var/lib/cortex-branding/motd-disabled.list + ;; esac done fiiso/live-build/config/bootloaders/grub-pc/theme.cfg-5-8 (1)
5-8: Fix GRUB theme path — it will not load as configured.
set theme=/boot/grub/live-theme/theme.txt(Line 6) does not match the actual deployment path used throughout the codebase. All build hooks, installation scripts, and the Makefile consistently deploy the theme to/boot/grub/themes/cortex/theme.txt. GRUB will fail to find the file at the configured path and silently fall back to defaults, removing custom branding.Proposed fix
-# Always load Cortex theme -set theme=/boot/grub/live-theme/theme.txt +# Always load Cortex theme +set theme=/boot/grub/themes/cortex/theme.txt set menu_color_normal=light-gray/black set menu_color_highlight=cyan/blackbranding/wallpapers/ASSETS.md-1-19 (1)
1-19: Wallpaper assets incomplete: only single resolution per wallpaper exists instead of documented 3 resolutions.ASSETS.md requires all 6 wallpapers at three resolutions (4K, QHD, FHD), but only 6 PNG files exist in
branding/wallpapers/images/— one per wallpaper. Either the documentation should be updated to reflect single-resolution assets, or the missing QHD and FHD variants (12 additional files) must be generated to match the documented requirements.branding/gdm/INSTALL.md-49-71 (1)
49-71: Method 3 + Troubleshooting: fixglib-compile-resourcesinvocation and restore commands (current doc is likely broken).
glib-compile-resourcestypically needs--target(and often--sourcedir) to produce the expected output file.- The XML references
gnome-shell.css, but the guide never creates/copiesgnome-shell.cssfromcortex-gdm.css.- The “restore backup” command uses relative filenames and won’t restore the actual backed-up paths; also consider restoring the
.gresourcebackup (not just CSS).Proposed doc fix
# Build resource -glib-compile-resources gnome-shell-theme.gresource.xml +cp cortex-gdm.css gnome-shell.css +glib-compile-resources gnome-shell-theme.gresource.xml \ + --target=gnome-shell-theme.gresource \ + --sourcedir=. # Install sudo cp gnome-shell-theme.gresource /usr/share/gnome-shell/### Black screen after changes 1. Boot to recovery mode -2. Restore backup: `sudo cp gnome-shell.css.backup gnome-shell.css` +2. Restore backup: + - `sudo cp /usr/share/gnome-shell/theme/gnome-shell.css.backup /usr/share/gnome-shell/theme/gnome-shell.css` + - `sudo cp /usr/share/gnome-shell/gnome-shell-theme.gresource.backup /usr/share/gnome-shell/gnome-shell-theme.gresource` 3. Reboot normallyAlso applies to: 96-99
iso/live-build/config/bootloaders/grub-pc/install_start_text.cfg-1-4 (1)
1-4: Fix duplicate--hotkey=ientries causing menu collision. Multiple menuentry definitions acrossinstall_start_text.cfg,install_text.cfg,install_start_gui.cfg, andinstall_gui.cfguse--hotkey=i. When GRUB loads these configurations, only the first match becomes active, making the hotkey unreliable and breaking expected menu behavior. Each installer variant (text/GUI, Start vs. Install, themed vs. unthemed) should have unique hotkeys.iso/preseed/profiles/cortex-full.preseed-36-83 (1)
36-83: Document EFI-only limitation and provide BIOS alternative for profiles.These three profiles (cortex-core, cortex-full, cortex-secops) all hardcode
/dev/sdaand use GPT + EFI-only partitioning recipes. This will fail on BIOS-only systems. Whilesimple-bios.preseedexists iniso/preseed/partitioning/, it is not integrated or documented as an alternative.At minimum, add a comment in each profile documenting:
- That
/dev/sdais unconditionally targeted (state how to override via boot parameter if supported)- That these are EFI-only; provide guidance to use the BIOS preseed for legacy systems
- Or unify the recipe conditionally (like the main
cortex.preseeddoes with$iflabel{ gpt }) to support both architecturespackages/cortex-secops/DEBIAN/control-6-38 (1)
6-38: Remove or replaceossec-hidsfrom Recommends — it is not available in Debian 12 (bookworm) official repositories.The package
ossec-hidsrequires a third-party repository (Atomicorp/Wazuh) and will cause apt resolver errors if that repo is not configured. The other flagged packages (tiger,tripwire,psad) do exist in bookworm. Either removeossec-hids, document the third-party repo requirement explicitly, or substitute an alternative that exists in official repos (e.g.,wazuh-agentif using Wazuh's official repo, or remove it entirely if it is optional).iso/preseed/partitioning/simple-bios.preseed-11-11 (1)
11-11: Hard-coded disk path limits installation flexibility.The fixed
/dev/sdapath will fail on systems with NVMe drives (/dev/nvme0n1), virtio (/dev/vda), or when the target disk is not the first SCSI device. This significantly restricts where this preseed can be used.💡 Suggested fix: Use dynamic disk detection
Consider one of these approaches:
Option 1: Omit the disk directive to let the installer prompt or auto-detect:
d-i partman-auto/method string regular -d-i partman-auto/disk string /dev/sdaOption 2: Use a preseed early_command to detect the first available disk dynamically:
+# Detect first available disk dynamically +d-i partman/early_command string \ + DISK=$(list-devices disk | head -n1); \ + debconf-set partman-auto/disk "$DISK" d-i partman-auto/method string regular -d-i partman-auto/disk string /dev/sdaiso/preseed/cortex-base.preseed-36-44 (1)
36-44: Clarify weak password handling and first-boot provisioning dependency.The preseed configuration creates a default
cortexuser with a placeholder password hash ($6$rounds=4096$cortexsalt$placeholder) and relies on first-boot provisioning or cloud-init to set a real password. This is operationally risky if:
- First-boot provisioning fails or is skipped.
- The system is used before first-boot provisioning runs.
- An operator mistakes the placeholder for a real password.
Recommendations:
- Document clearly in comments that this password must be changed before the system is accessible to users.
- Consider enforcing password expiration in PAM (e.g.,
chage -d 0 cortex) to force a password change on first login if first-boot provisioning is unavailable.- Add validation/logging in first-boot provisioning to confirm password was changed.
- Consider requiring a secure boot sequence that ensures first-boot provisioning runs before network access is granted.
branding/os-release/os-release-1-13 (1)
1-13: Fix incorrect GitHub repository name in URLs.The
SUPPORT_URLandBUG_REPORT_URLreference a non-existent repositorycortexlinux/cortex-distro. The actual repository iscortexlinux/cortex. Update lines 9–10:SUPPORT_URL="https://github.com/cortexlinux/cortex/discussions" BUG_REPORT_URL="https://github.com/cortexlinux/cortex/issues"docs/automated-installation.md-324-328 (1)
324-328: Security: Reconsider exposing the default FDE passphrase in documentation.Documenting the default FDE passphrase (
cortex-temp-fde-passphrase) in plain text creates a security risk. If users deploy SecOps installations without changing this passphrase, attackers with access to this documentation can decrypt the disk.Consider:
- Using a randomly generated passphrase during installation that's displayed once
- Forcing passphrase change on first boot for SecOps profile
- At minimum, adding a more prominent warning about this being publicly documented
branding/generate-assets.sh-9-11 (1)
9-11: Add ImageMagick version compatibility check.The script uses the
magickcommand (ImageMagick 7 syntax) extensively, but Debian Bookworm ships ImageMagick 6 by default, which usesconvertinstead. Without a compatibility layer, the script will fail on systems with only IM6 installed.Implement the suggested wrapper to detect and use the available command:
Compatibility wrapper
set -e SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# ImageMagick compatibility: IM7 uses 'magick', IM6 uses 'convert' +if command -v magick &>/dev/null; then + IM_CMD="magick" +elif command -v convert &>/dev/null; then + IM_CMD="convert" +else + echo "ERROR: ImageMagick not installed" + exit 1 +fiThen replace all
magickinvocations with$IM_CMD.iso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/cortex.script-138-147 (1)
138-147: Reuse the message sprite instead of allocating a new one per message.
message_callback()creates a newSpriteevery time; repeated status updates can stack sprites and waste memory.Proposed fix
fun message_callback(text) { message.image = Image.Text(text, 0.6, 0.6, 0.7, 1, "Sans 11"); - message.sprite = Sprite(message.image); + if (!message.sprite) { + message.sprite = Sprite(message.image); + } else { + message.sprite.SetImage(message.image); + } message.x = screen_x + screen_width / 2 - message.image.GetWidth() / 2; message.y = screen_y + screen_height - 50; message.sprite.SetPosition(message.x, message.y, 10); }
🟡 Minor comments (17)
iso/provisioning/first-boot.sh-358-378 (1)
358-378: Inconsistent volume group name in FDE notice.Line 378 references
cortex-sec-vgbut the FDE preseed usescortex-vgas the volume group name. This will confuse users trying to verify their encryption status.🐛 Proposed fix
To verify encryption status: - sudo cryptsetup status cortex-sec-vg + sudo cryptsetup status cortex-vgbranding/plymouth/cortex/ASSETS.md-1-51 (1)
1-51: **Fix Markdown fencing (currently likely to render broken due to mixedand ````).** The file starts/ends with ```` which conflicts with the innerbash block.Proposed change (use standard triple-backtick fences only)
-```` # Plymouth Theme Assets @@ ```bash # Generate placeholder assets (requires ImageMagick) convert -size 200x200 xc:transparent \ @@ progress-bar.png-````
</details> </blockquote></details> <details> <summary>packages/cortex-branding/DEBIAN/postinst-42-46 (1)</summary><blockquote> `42-46`: **Guard `dconf update` with a command check, not just file check.** On minimal or server installations where GDM (`gdm3`) is not installed, the `dconf` command will be absent. While your file check on line 43 ensures the config exists, `dconf` itself may not be present, causing the command to fail (though suppressed by `|| true`). This pattern is already used elsewhere in the script (line 11 with `command -v`). Apply the same guard: <details> <summary>Proposed change</summary> ```diff # Update GDM branding if [ -f /etc/gdm3/greeter.dconf-defaults ]; then - dconf update 2>/dev/null || true + command -v dconf >/dev/null 2>&1 && dconf update 2>/dev/null || true fiiso/live-build/config/includes.chroot/boot/grub/themes/cortex/ASSETS.md-42-52 (1)
42-52: Update optional icons section to match actual files present.The required theme assets and theme.txt references are correct and all PNG files listed in the asset table exist. However, the optional icons section documents six icons (cortex.png, debian.png, linux.png, windows.png, recovery.png, settings.png) but only three are present in the repository: cortex.png, linux.png, and recovery.png. Either add the missing icon files (debian.png, windows.png, settings.png) or update the documentation to reflect only the icons that exist.
The installation instructions assume manual post-installation setup, which is standard; note that in this live-build context, the theme is pre-included in the ISO build process.
branding/gdm/INSTALL.md-7-13 (1)
7-13: Clarify exact install destinations (paths) for “GDM theme / GDM resources”.
Right now the table mixes conceptual destinations (“GDM theme”, “GDM resources”) with one concrete path; it’ll be hard to automate/review correctness across Debian/GNOME versions. Consider listing the exact on-disk paths (and whether assets must be embedded intognome-shell-theme.gresource).branding/gdm/cortex-gdm.css-148-151 (1)
148-151: Consider ignoring Biome's unknown pseudo-class warning on.user-list-item:selected.
While:selectedis valid in GNOME Shell CSS (part of GTK widget states), Biome's CSS linter will flag it since it's not standard web CSS. The suggestion to add an inline ignore comment is optional for developers who lint locally, but this is not a CI blocker since Biome is not currently invoked in your build pipeline.-.user-list-item:selected { +/* biome-ignore lint/correctness/noUnknownPseudoClass: GNOME Shell theme pseudo-class */ +.user-list-item:selected { background-color: rgba(107, 33, 168, 0.5); border: 1px solid #6B21A8; }packages/cortex-full/DEBIAN/control-12-26 (1)
12-26: firefox-esr package name may not be available on Ubuntu.Line 22 recommends
firefox-esr, which is the Debian package name. Ubuntu typically provides Firefox via snap or asfirefox(notfirefox-esr). This won't block installation (Recommends vs Depends) but may result in missing Firefox on Ubuntu systems.Consider a more portable browser recommendation
If targeting both Debian and Ubuntu:
- firefox-esr, + firefox-esr | firefox,This allows either package name to satisfy the recommendation. Alternatively, document this difference in the installation guide.
iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot-75-97 (1)
75-97: Address icon theme inconsistency across branding configurations.The background image path at lines 85-86, 91 (
/usr/share/backgrounds/cortex/cortex-default.png) is properly installed—the file exists iniso/live-build/config/includes.chroot/usr/share/backgrounds/cortex/and is verified by the50-cortex-branding.hook.chroothook.However, icon theme inconsistency exists: Line 80 sets
icon-theme='Adwaita'for the live user session, whilebranding/gdm/gdm-branding.confspecifiesicon-theme='cortex-icons'. This creates a mismatch in icon themes across environments. The actual GDM hook (50-cortex-branding.hook.chroot) does not set an icon-theme for the login screen, leaving it unspecified. Clarify the intended icon theme strategy and update configurations consistently—either usecortex-iconsthroughout or document why different themes apply to different contexts.Line 95's
welcome-dialog-last-shown-version='9999999999'is functional but non-standard. Consider adding a comment explaining this approach.iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot-61-61 (1)
61-61: Remove unnecessary execute permission on.desktopfile.The execute permission is not required by the XDG/Freedesktop autostart specification for files in
~/.config/autostart. GNOME's autostart mechanism reads the.desktopfile as configuration and executes the command specified in theExeckey regardless of filesystem permissions. Thechmod +xline serves no purpose and should be removed.Execute permissions are only needed if the file is intended to be double-clicked/launched from a file manager, which is not the case here.
iso/preseed/profiles/cortex-secops.preseed-41-41 (1)
41-41: Same hardcoded disk path issue as core profile.See the comment on
cortex-core.preseedregarding/dev/sdahardcoding.Makefile-204-206 (1)
204-206: Silent failure on missing ISO output.The
mv ... || truepattern will silently succeed even if the ISO wasn't built, making it hard to detect build failures. Consider checking if the source file exists first or failing explicitly.🐛 Proposed fix
@# Move output - @mv $(BUILD_DIR)/$(1)/live-image-$(ARCH).hybrid.iso \ - $(OUTPUT_DIR)/$(ISO_NAME)-$(1)-$(ISO_VERSION)-$(ARCH).iso 2>/dev/null || true + @if [ -f "$(BUILD_DIR)/$(1)/live-image-$(ARCH).hybrid.iso" ]; then \ + mv $(BUILD_DIR)/$(1)/live-image-$(ARCH).hybrid.iso \ + $(OUTPUT_DIR)/$(ISO_NAME)-$(1)-$(ISO_VERSION)-$(ARCH).iso; \ + else \ + echo "ERROR: ISO not found at $(BUILD_DIR)/$(1)/live-image-$(ARCH).hybrid.iso"; \ + exit 1; \ + fi @echo "ISO built: $(OUTPUT_DIR)/$(ISO_NAME)-$(1)-$(ISO_VERSION)-$(ARCH).iso"branding/generate-assets.sh-403-403 (1)
403-403: Fixfindcommand to count all image files correctly.The
-ooperator has lower precedence than the implicit.svgfiles will be counted. The.pngfiles won't be included in the output.🐛 Proposed fix
-find "${SCRIPT_DIR}" -name "*.png" -o -name "*.svg" | wc -l | xargs echo " Total images:" +find "${SCRIPT_DIR}" \( -name "*.png" -o -name "*.svg" \) | wc -l | xargs echo " Total images:"iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot-101-104 (1)
101-104: Fix GRUB_DISTRIBUTOR configuration logic.The
sed -icommand returns success (exit 0) even when the pattern isn't found, so the fallbackechowill never execute. IfGRUB_DISTRIBUTORdoesn't exist in the file, it won't be added. Use the samegrep-then-sed-or-echopattern used forGRUB_THEMEabove.🐛 Proposed fix
- # Also set timeout and other branding options - sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub || \ - echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub + # Also set distributor branding + if grep -q "^GRUB_DISTRIBUTOR=" /etc/default/grub; then + sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub + else + echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub + fiiso/preseed/profiles/cortex-core.preseed-41-41 (1)
41-41: Hardcoded disk path may cause installation failures.Hardcoding
/dev/sdawill fail on systems with NVMe drives (/dev/nvme0n1), virtio disks (/dev/vda), or other storage configurations. Consider using an empty string to let the installer auto-detect, or documenting this as a template that must be customized.♻️ Suggested alternative for auto-detection
### Partitioning - Simple UEFI (default for core) d-i partman-auto/method string regular -d-i partman-auto/disk string /dev/sda +# Leave blank for auto-detection, or set via boot parameter +d-i partman-auto/disk stringiso/preseed/profiles/cortex-core.preseed-30-31 (1)
30-31: Password setup requires user action at first login; document security expectations clearly.The preseed uses a placeholder password hash intentionally to force users to set their own credentials at first login, rather than shipping a pre-configured password. However, this design choice should be more explicitly documented in the preseed file itself—a comment like
# IMPORTANT: User MUST set password on first loginwould prevent confusion about whether this is a misconfiguration or intentional.Additionally,
allow-password-weak boolean true(line 31) should be reconsidered for a server-focused core profile. Even with first-login setup, allowing weak passwords creates an exploitable window. Consider changing this tofalseand documenting the password requirements for users.iso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/cortex.script-123-129 (1)
123-129: Avoid leakingiinto global scope in the bullets loop.
iisn’t declaredlocal, so it may become global and collide with other loops/callbacks.Proposed fix
fun password_dialogue_opacity(opacity) { + local.i; if (global.password_entry && global.password_entry.sprite) global.password_entry.sprite.SetOpacity(opacity); if (global.password_prompt && global.password_prompt.sprite) global.password_prompt.sprite.SetOpacity(opacity); for (i = 0; global.password_bullets[i]; i++) { global.password_bullets[i].sprite.SetOpacity(opacity); } }iso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/cortex.script-73-96 (1)
73-96: Clamp progress values and clear progress bar on zero to avoid stale visuals.The current code only updates the progress bar image when
new_width > 1, leaving smaller values with the previously scaled image visible. Additionally,Scale()expects integer dimensions; floating-point values from out-of-range progress may cause unexpected behavior. Clampprogressto the expected range for each callback (0–1 for boot progress, 0–100 for system updates), convert to integer withMath.Int(), and useSetOpacity(0)to hide the bar whennew_width <= 0.Proposed fix
fun progress_callback(duration, progress) { if (progress_bar.original_image.GetWidth() > 0) { + if (progress < 0) progress = 0; + if (progress > 1) progress = 1; - new_width = progress_bar.original_image.GetWidth() * progress; - if (new_width > 1) { + new_width = Math.Int(progress_bar.original_image.GetWidth() * progress); + if (new_width <= 0) { + progress_bar.sprite.SetOpacity(0); + } else { + progress_bar.sprite.SetOpacity(1); progress_bar.image = progress_bar.original_image.Scale(new_width, progress_bar.original_image.GetHeight()); progress_bar.sprite.SetImage(progress_bar.image); } } } @@ fun system_update_callback(progress) { if (progress_bar.original_image.GetWidth() > 0) { + if (progress < 0) progress = 0; + if (progress > 100) progress = 100; - new_width = progress_bar.original_image.GetWidth() * progress / 100; - if (new_width > 1) { + new_width = Math.Int(progress_bar.original_image.GetWidth() * progress / 100); + if (new_width <= 0) { + progress_bar.sprite.SetOpacity(0); + } else { + progress_bar.sprite.SetOpacity(1); progress_bar.image = progress_bar.original_image.Scale(new_width, progress_bar.original_image.GetHeight()); progress_bar.sprite.SetImage(progress_bar.image); } } }
🧹 Nitpick comments (36)
iso/live-build/config/includes.binary/boot/grub/themes/cortex/ASSETS.md (1)
1-53: Consider DRY principle: documentation duplication detected.This file is identical to
branding/grub/cortex/ASSETS.md. While the duplication may be intentional for different deployment contexts (binary ISO vs. branding source), it creates a maintenance burden where asset specifications must be kept synchronized manually.Consider one of these approaches:
- Use a single source of truth with symlinks
- Generate this file from the canonical version during build
- Add a comment referencing the canonical location if duplication is required
iso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/ASSETS.md (1)
16-24: Add language specifier to color palette code block.The code block lacks a language identifier, which affects rendering and syntax highlighting. Consider specifying a language for better documentation clarity.
📝 Proposed fix for markdown linting
## Color Palette -``` +```text Primary Purple: #6B21A8 (rgb: 107, 33, 168) Electric Cyan: #06B6D4 (rgb: 6, 182, 212) Dark Background: #0F0F23 (rgb: 15, 15, 35) Text Light: #E2E8F0 (rgb: 226, 232, 240) Text Muted: #94A3B8 (rgb: 148, 163, 184)</details> </blockquote></details> <details> <summary>iso/live-build/config/package-lists/live.list.chroot (1)</summary><blockquote> `1-42`: **Confirm target Debian release + consider trimming size-heavy choices (`task-gnome-desktop`, `live-boot-doc`).** - Please verify every package here exists in (and is appropriate for) the target Debian suite you’re building Cortex on; `task-gnome-desktop` in particular can pull a large dependency set and may make ISO size/control harder. - `live-boot-doc` is usually safe to drop from a production ISO unless you explicitly want offline docs. <details> <summary>Proposed diff (optional size trim)</summary> ```diff --- a/iso/live-build/config/package-lists/live.list.chroot +++ b/iso/live-build/config/package-lists/live.list.chroot @@ live-boot -live-boot-doc initramfs-toolsbranding/BRAND-GUIDELINES.md (1)
149-175: Fix markdownlint MD040 by adding languages to the fenced blocks (Buttons/Cards/Input Fields).Proposed diff
--- a/branding/BRAND-GUIDELINES.md +++ b/branding/BRAND-GUIDELINES.md @@ ### Buttons - -``` +```text Primary: #6B21A8 background, white text Secondary: transparent, #6B21A8 border Ghost: transparent, #94A3B8 text Danger: #EF4444 background, white text@@
Cards
-
+text
Background: #1E1E3F (dark) or #FFFFFF (light)
Border: 1px solid rgba(107, 33, 168, 0.2)
Border radius: 12px
Shadow: 0 4px 20px rgba(0, 0, 0, 0.15)@@ ### Input Fields - -``` +```text Background: #0F0F23 (dark) or #F8FAFC (light) Border: 1px solid #2D2D5A Focus border: #06B6D4 Border radius: 8px</details> </blockquote></details> <details> <summary>iso/live-build/config/includes.chroot/boot/grub/themes/cortex/theme.txt (1)</summary><blockquote> `4-10`: **Verify font support in your GRUB build (likely needs `.pf2` fonts + `loadfont`).** Theme entries like `terminal-font: "DejaVu Sans Mono Regular 14"` and `item_font = "DejaVu Sans Regular 16"` only work if the corresponding GRUB font assets are present/loaded in your GRUB config. Also applies to: 17-31 </blockquote></details> <details> <summary>branding/motd/10-cortex-sysinfo (1)</summary><blockquote> `28-48`: **Optional: avoid calling `free`/`df` multiple times.** Minor perf/cleanliness: you can capture `free` and `df` output once to avoid 3 subprocesses each (useful on slow systems and reduces MOTD latency). </blockquote></details> <details> <summary>iso/preseed/partitioning/fde-uefi.preseed (1)</summary><blockquote> `91-91`: **Duplicate directive: `weak_passphrase` already set on line 27.** This directive is redundant and can be removed. <details> <summary>♻️ Proposed fix</summary> ```diff -# Skip LUKS warning about weak passphrase (will be changed at first-boot) -d-i partman-crypto/weak_passphrase boolean truebranding/motd/20-cortex-updates (1)
51-52: Minor style: Useless use ofcat.The
catis unnecessary here and in line 62.♻️ Proposed fix
- REBOOT_PKGS=$(cat /var/run/reboot-required.pkgs | head -3 | tr '\n' ', ' | sed 's/,$//') + REBOOT_PKGS=$(head -3 /var/run/reboot-required.pkgs | tr '\n' ', ' | sed 's/,$//')- cat "$CORTEX_NOTICE" | sed 's/^/ /' + sed 's/^/ /' "$CORTEX_NOTICE"iso/preseed/partitioning/raid1-uefi.preseed (1)
30-31: Duplicate directive:new_vg_nameset twice.Lines 30 and 63 both set
partman-auto-lvm/new_vg_name. Remove the duplicate.Also applies to: 63-63
iso/provisioning/first-boot.sh (2)
61-72: Validate profile file content before sourcing.The script sources
${PROFILE_FILE}directly without validating its contents. While the file should be root-owned, consider adding basic validation to ensure it only contains expected variable assignments.♻️ Safer profile loading
load_profile() { if [[ ! -f "${PROFILE_FILE}" ]]; then log_warn "Profile file not found, defaulting to 'core'" CORTEX_PROFILE="core" else - # shellcheck source=/dev/null - source "${PROFILE_FILE}" + # Only read CORTEX_PROFILE variable, ignore other content + CORTEX_PROFILE=$(grep -Po '^CORTEX_PROFILE=\K(core|full|secops)$' "${PROFILE_FILE}" 2>/dev/null || echo "core") fi log_info "Profile: ${CORTEX_PROFILE:-core}" export CORTEX_PROFILE="${CORTEX_PROFILE:-core}" }
74-83: Consider alternative network checks for restricted environments.Pinging
8.8.8.8may fail in environments where ICMP is blocked. Consider falling back to DNS resolution or HTTP connectivity checks.♻️ More robust network check
check_network() { - if ping -c 1 -W 3 8.8.8.8 &>/dev/null; then + if ping -c 1 -W 3 8.8.8.8 &>/dev/null || \ + curl -s --max-time 3 -o /dev/null https://apt.cortexlinux.com &>/dev/null; then log_info "Network: Online" export NETWORK_AVAILABLE=true elseiso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/cortex.plymouth (1)
1-8: Consider DRY: duplicate Plymouth theme configuration.This file is identical to
branding/plymouth/cortex/cortex.plymouth. While having separate copies for the branding package and live-build is a valid deployment strategy, consider symlinking or copying from a single source during build to avoid drift.iso/live-build/config/bootloaders/grub-pc/live-theme/theme.txt (1)
12-25: Remove duplicateselected_item_colorproperty.
selected_item_coloris defined twice (lines 20 and 24) with the same value. Remove the duplicate to improve maintainability.Suggested fix
+ boot_menu { left = 15% top = 25% width = 70% height = 50% item_font = "DejaVu Sans Regular 14" item_color = "#E2E8F0" selected_item_font = "DejaVu Sans Bold 14" selected_item_color = "#06B6D4" item_height = 36 item_padding = 12 item_spacing = 4 - selected_item_color = "#06B6D4" }branding/motd/00-cortex-banner (1)
7-27: Preferprintfoverecho -e(and drop/justify unused color vars).
echo -ebehavior can vary;printfis deterministic. AlsoPURPLE/CYAN/WHITEare currently unused (Lines 8-10).Proposed change
-echo -e "${GRAY} L I N U X${RESET}" -echo "" +printf '%b\n\n' "${GRAY} L I N U X${RESET}"docs/DEVELOPERS.md (1)
145-150: Verify QEMU testing commands are compatible with build output.The QEMU testing commands (lines 145-150) assume the build process generates ISO files in
output/with the expected naming scheme. Ensure:
- The ISO naming matches the pattern
cortex-linux-full-*.iso- The wildcard expansion will work correctly in shell commands
- QEMU paths are correct for common Linux distributions
Note: These are illustrative examples, but developers may copy them verbatim.
Consider making the QEMU examples more robust:
Optional improvement: More robust QEMU examples
### Test ISO in VM ```bash -# QEMU (amd64) -qemu-system-x86_64 -m 4G -cdrom output/cortex-linux-full-*.iso -boot d - -# QEMU (arm64) -qemu-system-aarch64 -M virt -cpu cortex-a72 -m 4G -cdrom output/cortex-linux-full-*-arm64.iso +# QEMU (amd64) +ISO=$(ls output/cortex-linux-full-*-amd64.iso 2>/dev/null | tail -1) +qemu-system-x86_64 -m 4G -cdrom "$ISO" -boot d + +# QEMU (arm64) +ISO=$(ls output/cortex-linux-full-*-arm64.iso 2>/dev/null | tail -1) +qemu-system-aarch64 -M virt -cpu cortex-a72 -m 4G -cdrom "$ISO"</details> </blockquote></details> <details> <summary>branding/wallpapers/ASSETS.md (1)</summary><blockquote> `45-77`: **Design guidelines are well-documented, but implementation needs verification.** The design guidelines for each of the 6 wallpaper themes are detailed and provide clear visual direction: - Neural Dark/Light with network patterns - Gradient Purple/Cyan variations - Minimal Dark with logo - Circuit Board tech aesthetic These provide good guidance for designers, but confirm: 1. The actual asset files match these design specifications 2. The design is cohesive and professional across all themes 3. Colors and patterns are rendered correctly at all specified resolutions Consider adding design asset sources or generation instructions if wallpapers are being generated programmatically (e.g., via `branding/generate-assets.sh` mentioned in AI summary). This would help future maintainers reproduce or modify the wallpapers. <details> <summary>Optional enhancement: Document wallpaper generation</summary> If wallpapers are generated (not hand-created), document: ```markdown ## Generation Wallpapers can be generated using the `branding/generate-assets.sh` script: \`\`\`bash ./branding/generate-assets.sh --wallpapers \`\`\` This regenerates all wallpapers at the specified resolutions.branding/gdm/INSTALL.md (1)
16-37: Method 1: verify thatupdate-alternativesongnome-shell.cssactually affects GDM on the target GNOME Shell build.
On many distros the greeter theme is effectively driven by the compiledgnome-shell-theme.gresource; if GDM doesn’t read/usr/share/gnome-shell/theme/gnome-shell.css, this method won’t apply (or will apply inconsistently). If it does work for your target, consider adding the missing “select it” step so users don’t forget.Proposed doc tweak
sudo update-alternatives --install \ /usr/share/gnome-shell/theme/gnome-shell.css \ gdm-theme /usr/share/gnome-shell/theme/Cortex/gnome-shell.css 100 + +# Select the alternative (if multiple are installed) +sudo update-alternatives --config gdm-themepackages/cortex-secops/DEBIAN/control (1)
1-6: Consider version-pinning the bundle to the matchingcortex-coreversion.
For metapackages,Depends: cortex-core (= ${binary:Version})(or at least>=) helps prevent mixed-version installs when repos lag or users partially upgrade.Also applies to: 38-55
iso/preseed/profiles/cortex-full.preseed (1)
140-148: Late-command: addsystemctl daemon-reloadbefore enabling the new unit.
Enabling a freshly-copied unit can fail if systemd hasn’t noticed it yet.Proposed change
d-i preseed/late_command string \ in-target mkdir -p /opt/cortex/provisioning; \ in-target mkdir -p /var/log/cortex; \ cp /cdrom/provisioning/first-boot.sh /target/opt/cortex/provisioning/; \ cp /cdrom/provisioning/cortex-first-boot.service /target/etc/systemd/system/; \ echo "CORTEX_PROFILE=full" > /target/opt/cortex/provisioning/profile; \ in-target chmod +x /opt/cortex/provisioning/first-boot.sh; \ + in-target systemctl daemon-reload; \ in-target systemctl enable cortex-first-boot.serviceiso/preseed/partitioning/simple-bios.preseed (1)
17-37: Consider documenting the absence of swap or making it configurable.The partition recipe defines no swap space, which may be intentional for modern systems with adequate RAM or container/cloud deployments. However, many traditional installations benefit from swap (especially for hibernation, memory-constrained systems, or swap-on-zram).
📋 Optional: Add a swap partition or document the rationale
If swap should be included, add a swap partition entry before the root partition:
. \ + 2048 4096 8192 linux-swap \ + $primary{ } \ + method{ swap } format{ } \ + . \ 8192 16384 -1 ext4 \Alternatively, add a comment explaining why swap is omitted:
# Define partition recipe +# Note: No swap partition - assumes adequate RAM or swap-on-zram at runtime d-i partman-auto/expert_recipe string \docs/QUICK-START.md (1)
42-48: Consider increasing disk recommendation for cortex-full.The 20GB disk requirement for
cortex-fullmight be insufficient for active development. Docker images, Node.jsnode_modules, Go/Rust build artifacts, and development projects can quickly consume space. Consider recommending 40GB+ for comfortable development use.| Profile | RAM | Disk | Use Case | |---------|-----|------|----------| | cortex-core | 512MB | 2GB | Servers, embedded, minimal | -| cortex-full | 4GB | 20GB | Workstations, development | +| cortex-full | 4GB | 40GB | Workstations, development | | cortex-secops | 4GB | 20GB | Security-critical systems |iso/preseed/cortex-base.preseed (1)
60-77: Package selection is well-curated for a minimal server install.The included packages balance essential tools (ssh, sudo, curl, git, firewall) with a minimal footprint. This is a solid foundation for base installations.
✓ Security tools (sudo, ufw, fail2ban, ca-certificates, gnupg) included.
✓ Admin/dev tools (vim, htop, python3, git) included.
✓ No unnecessary bloat (no X11, no heavy applications).If this base is intended for minimal/core profiles, consider whether to add
grub-pcorgrub-efiexplicitly (they should be pulled in by task selection, but explicit inclusion reduces surprises).iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot (2)
102-123: Consider avoiding blanket error suppression on schema compilation.Line 123 suppresses all errors from
glib-compile-schemaswith|| true. While this prevents build failures, it may hide genuine schema syntax errors or permission issues.Consider logging errors for debugging:
♻️ Proposed refinement
# Compile schemas -glib-compile-schemas /usr/share/glib-2.0/schemas/ 2>/dev/null || true +if ! glib-compile-schemas /usr/share/glib-2.0/schemas/ 2>&1; then + echo "Warning: glib-compile-schemas failed, but continuing..." >&2 +fi
25-40: Remove redundant GDM configuration file.
/etc/gdm3/daemon.confand/etc/gdm3/custom.confserve the same purpose and contain identical settings. GDM3 reads only one system override file per distribution;custom.confis the standard and recommended file across modern GDM3 versions. Keep onlycustom.confand remove thedaemon.confcreation to avoid unnecessary duplication.branding/grub/cortex/theme.txt (1)
46-56: Hardcoded version string requires manual maintenance.Line 52 contains the hardcoded version string
"v1.0 Synapse". This will require manual updates for each new release. Consider:
- Documenting this as a required update step in release procedures, or
- Generating this file dynamically during the build process using version information from
branding/os-release/os-release.docs/automated-installation.md (1)
36-38: Add language specifier to fenced code blocks for better rendering.Per static analysis hints, several code blocks are missing language specifiers (lines 36, 140, 185, 207, 310). Adding language identifiers improves syntax highlighting and accessibility.
Example fix
-``` +```text preseed/file=/cdrom/preseed/profiles/cortex-{profile}.preseedbranding/install-branding.sh (2)
152-155: Handle empty glob safely to avoid unexpected behavior.If
/etc/update-motd.d/is empty or doesn't exist, the glob*won't expand and the loop will iterate once with the literal string*, causing the[ -f "$f" ]check to fail unexpectedly. While this won't break the script due to|| true, it's cleaner to handle explicitly.Suggested fix
# Disable existing MOTD scripts - for f in /etc/update-motd.d/*; do - [ -f "$f" ] && chmod -x "$f" 2>/dev/null || true - done + if [ -d /etc/update-motd.d ]; then + for f in /etc/update-motd.d/*; do + [ -f "$f" ] && chmod -x "$f" 2>/dev/null || true + done + fi
131-134: Clarify GDM detection logic.The condition
! command -v gdm3 &>/dev/null && ! [ -d /etc/gdm3 ]returns true when both the command is missing and the directory doesn't exist. This is correct for an OR-skip scenario due to De Morgan's law, but could be clearer for maintainability.Consider using explicit OR logic:
Suggested refactor
- if ! command -v gdm3 &>/dev/null && ! [ -d /etc/gdm3 ]; then + if ! command -v gdm3 &>/dev/null && ! [ -d /etc/gdm3 ]; then + # Neither gdm3 command nor config directory exists log_warn "GDM not found, skipping" return fiActually, the logic appears intentional - skip only if both are absent. Adding a comment would improve clarity.
packages/cortex-branding/DEBIAN/control (1)
6-8: Consider makingplymoutha Recommends instead of Depends.The package's postinst and prerm scripts already gracefully handle systems without Plymouth (using
if command -vchecks), and no content files require Plymouth to be present. Movingplymouthandplymouth-themesto Recommends would allow users to install cortex-branding for other features (OS identity, GRUB theme, wallpapers, GDM styling, MOTD) without pulling in unnecessary graphics dependencies on headless systems.Note: The current architecture already mitigates this concern—cortex-core (the minimal profile) doesn't depend on cortex-branding at all, while cortex-full (desktop) includes both. This suggestion would add flexibility for users who want branding components without the boot splash.
Suggested change
-Depends: plymouth, plymouth-themes +Depends: +Recommends: plymouth, plymouth-themes, gnome-shell, gdm3 -Recommends: gnome-shell, gdm3 Suggests: neofetchiso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (2)
55-61: Add timeout and retry logic for network operation.The
curlcommand to download the GPG key lacks timeout options and retry logic. During ISO build, network issues could cause the build to hang indefinitely or fail unpredictably.♻️ Suggested improvement
# Download and install GPG key mkdir -p /usr/share/keyrings -curl -fsSL https://apt.cortexlinux.com/keys/cortex-linux.gpg.asc | gpg --dearmor -o /usr/share/keyrings/cortex-linux.gpg +curl --retry 3 --retry-delay 5 --max-time 30 -fsSL https://apt.cortexlinux.com/keys/cortex-linux.gpg.asc | gpg --dearmor -o /usr/share/keyrings/cortex-linux.gpg
196-196: Nitpick: Avoid useless use ofcat.The
cat /proc/loadavg | awkpattern can be simplified.♻️ Suggested fix
-LOAD=$(cat /proc/loadavg | awk '{print $1, $2, $3}') +LOAD=$(awk '{print $1, $2, $3}' /proc/loadavg)branding/generate-assets.sh (1)
18-18: Remove unused variable.
LIGHT_CYANis defined but never used in the script, as flagged by shellcheck.♻️ Suggested fix
ELECTRIC_CYAN="#06B6D4" -LIGHT_CYAN="#22D3EE" DARK_BG="#0F0F23"branding/plymouth/cortex/cortex.script (1)
139-146: Message sprites may accumulate without cleanup.Each call to
message_callbackcreates a new sprite without removing the previous one. While this is unlikely to cause issues during a typical boot sequence, rapid message updates could lead to visual artifacts or memory accumulation.♻️ Suggested fix to reuse message sprite
+// Initialize message sprite once +global.message_sprite = Sprite(); +global.message_sprite.SetOpacity(0); + // Message display fun message_callback(text) { message.image = Image.Text(text, 0.6, 0.6, 0.7, 1, "Sans 11"); - message.sprite = Sprite(message.image); + global.message_sprite.SetImage(message.image); message.x = screen_x + screen_width / 2 - message.image.GetWidth() / 2; message.y = screen_y + screen_height - 50; - message.sprite.SetPosition(message.x, message.y, 10); + global.message_sprite.SetPosition(message.x, message.y, 10); + global.message_sprite.SetOpacity(1); }Makefile (2)
227-237: ARM64 targets run validation twice.The ARM64 targets have
check-deps validateas prerequisites, but then call$(MAKE) ARCH=arm64 iso-<profile>which also has these prerequisites. This causes redundant validation runs.♻️ Suggested simplification
# ARM64 ISO targets iso-arm64: iso-arm64-full -iso-arm64-core: check-deps validate $(BUILD_DIR) $(OUTPUT_DIR) +iso-arm64-core: $(MAKE) ARCH=arm64 iso-core -iso-arm64-full: check-deps validate $(BUILD_DIR) $(OUTPUT_DIR) +iso-arm64-full: $(MAKE) ARCH=arm64 iso-full -iso-arm64-secops: check-deps validate $(BUILD_DIR) $(OUTPUT_DIR) +iso-arm64-secops: $(MAKE) ARCH=arm64 iso-secops
367-373: Branding package may be incomplete due to silent copy failures.The
cp ... || truepatterns on lines 367-373 will silently skip missing source files, potentially producing an incomplete.debpackage. Consider adding validation or removing the|| trueto catch missing assets.♻️ Suggested improvement to validate sources
+ @# Validate required assets exist + @test -f $(BRANDING_DIR)/plymouth/cortex/cortex.plymouth || { echo "ERROR: Plymouth theme missing"; exit 1; } + @test -f $(BRANDING_DIR)/grub/cortex/theme.txt || { echo "ERROR: GRUB theme missing"; exit 1; } @# Copy Plymouth theme - @cp $(BRANDING_DIR)/plymouth/cortex/* $(BUILD_DIR)/cortex-branding/usr/share/plymouth/themes/cortex/ 2>/dev/null || true + @cp $(BRANDING_DIR)/plymouth/cortex/* $(BUILD_DIR)/cortex-branding/usr/share/plymouth/themes/cortex/ @# Copy GRUB theme - @cp $(BRANDING_DIR)/grub/cortex/* $(BUILD_DIR)/cortex-branding/boot/grub/themes/cortex/ 2>/dev/null || true + @cp $(BRANDING_DIR)/grub/cortex/* $(BUILD_DIR)/cortex-branding/boot/grub/themes/cortex/iso/live-build/config/includes.chroot/usr/share/plymouth/themes/cortex/cortex.script (1)
55-71: Pulse animation is coupled to progress updates (may "freeze" early).
pulse_logo()is only called fromprogress_callback(). If boot progress updates are infrequent, the pulse won't animate. Additionally,global.progressis declared but never used.Consider decoupling the animation using
Plymouth.SetRefreshFunction(), which is the supported mechanism for independent frame-based animations in Plymouth and runs up to 50 times per second:Proposed fix (decouple animation from progress)
// Animation variables global.progress = 0; global.pulse_opacity = 1; global.pulse_direction = -1; +fun refresh_callback() { + pulse_logo(); +} +Plymouth.SetRefreshFunction(refresh_callback); + // Progress callback fun progress_callback(duration, progress) { + global.progress = progress; if (progress_bar.original_image.GetWidth() > 0) { new_width = progress_bar.original_image.GetWidth() * progress; @@ - pulse_logo(); } Plymouth.SetBootProgressFunction(progress_callback);This applies to
progress_callback()andsystem_update_callback()patterns.
| # Cortex Linux GDM Configuration | ||
| # | ||
| # This file configures GDM branding settings. | ||
| # Place in /etc/gdm3/greeter.dconf-defaults | ||
|
|
||
| [org/gnome/login-screen] | ||
| # Logo displayed on login screen | ||
| logo='/usr/share/cortex/logos/cortex-logo-light.svg' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for cortex logo files in branding
echo "=== Searching for cortex logo files ==="
fd -t f "cortex-logo" branding/
echo -e "\n=== Listing branding directory structure ==="
find branding/ -type f -name "*.svg" -o -name "*.png" | head -20
echo -e "\n=== Checking for installation scripts/hooks ==="
fd -t f "(install|hook|spec|debian|postinst)" branding/ | head -20
echo -e "\n=== Checking gdm-branding.conf directory ==="
ls -la branding/gdm/Repository: cortexlinux/cortex-distro
Length of output: 1629
🏁 Script executed:
#!/bin/bash
# Check installation script and documentation
echo "=== install-branding.sh content ==="
cat branding/install-branding.sh
echo -e "\n=== INSTALL.md content ==="
cat branding/gdm/INSTALL.mdRepository: cortexlinux/cortex-distro
Length of output: 9247
Fix logo path and filename in GDM configuration.
Line 8 references /usr/share/cortex/logos/cortex-logo-light.svg, but this file doesn't exist in the branding assets. The actual logo file is branding/gdm/cortex-logo.svg (without the "-light" suffix), and the install-branding.sh script doesn't copy any logo to /usr/share/cortex/logos/. Update the configuration to use the correct filename and ensure the logo asset is properly installed to the referenced path, or revise the installation script to copy the logo file appropriately.
🤖 Prompt for AI Agents
In @branding/gdm/gdm-branding.conf around lines 1 - 8, The GDM config references
a non-existent logo path '/usr/share/cortex/logos/cortex-logo-light.svg'; update
the login-screen logo setting to the actual filename 'cortex-logo.svg' and
ensure the asset is installed to that path by either modifying the
install-branding.sh script to copy 'branding/gdm/cortex-logo.svg' into
'/usr/share/cortex/logos/' (creating the directory if needed) or change the
config to point to wherever install-branding.sh currently places the logo;
update the value in [org/gnome/login-screen] (logo=) and adjust
install-branding.sh accordingly so the file and path match.
| [org/gnome/desktop/interface] | ||
| # Set GTK theme for login screen | ||
| gtk-theme='Adwaita-dark' | ||
|
|
||
| # Icon theme | ||
| icon-theme='cortex-icons' | ||
|
|
||
| # Cursor theme | ||
| cursor-theme='Adwaita' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for cortex-icons theme references and installation
echo "=== Searching for cortex-icons references ==="
fd -type f -name "*cortex-icons*" 2>/dev/null || echo "No cortex-icons files found by name"
rg -l "cortex-icons" --type conf --type hook 2>/dev/null || echo "No cortex-icons in config/hook files"
echo -e "\n=== Searching for gnome hook configuration ==="
fd -type f -name "*configure-gnome*" 2>/dev/null
fd -type f -name "*.hook.chroot" 2>/dev/null | head -20
echo -e "\n=== Searching for icon-theme references ==="
rg "icon-theme" --type conf --type hook -n 2>/dev/nullRepository: cortexlinux/cortex-distro
Length of output: 280
🏁 Script executed:
#!/bin/bash
echo "=== Repository structure ==="
git ls-files | head -30
echo -e "\n=== Searching branding directory ==="
fd -type f . branding/ 2>/dev/null | head -20
echo -e "\n=== All hook files ==="
fd -type f -name "*.hook*" 2>/dev/null
echo -e "\n=== Searching for gnome in all files ==="
rg "gnome|GNOME" -l --max-count 3 2>/dev/null | head -20
echo -e "\n=== All icon-theme references globally ==="
rg "icon-theme" -n 2>/dev/nullRepository: cortexlinux/cortex-distro
Length of output: 2154
🏁 Script executed:
#!/bin/bash
echo "=== Contents of branding/gdm/gdm-branding.conf ==="
cat -n branding/gdm/gdm-branding.conf
echo -e "\n=== Contents of iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot (lines 70-115) ==="
sed -n '70,115p' iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot
echo -e "\n=== Search for cortex-icons package definition ==="
fd -type f "cortex-icons|cortex-branding" packages/ 2>/dev/null
find packages -type f -name "control" -o -name "*.list" | xargs grep -l "cortex-icons" 2>/dev/null
echo -e "\n=== Check if cortex-icons is mentioned in any package lists ==="
rg "cortex-icons|icon.*theme" packages/ iso/preseed/ 2>/dev/nullRepository: cortexlinux/cortex-distro
Length of output: 3056
🏁 Script executed:
#!/bin/bash
echo "=== Search for any cortex-icons definition or theme files ==="
find . -type d -name "cortex-icons" 2>/dev/null
rg "cortex-icons" -n 2>/dev/null
echo -e "\n=== Check if cortex-icons is in any asset/installation scripts ==="
cat branding/install-branding.sh | head -50
echo -e "\n=== Check GDM install documentation ==="
cat branding/gdm/INSTALL.mdRepository: cortexlinux/cortex-distro
Length of output: 4244
Remove 'cortex-icons' reference or implement the icon theme.
Line 25 specifies icon-theme='cortex-icons', which does not appear to be defined or installed anywhere in the codebase. The live GNOME environment (iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot lines 80, 107) uses icon-theme='Adwaita' instead.
GDM will fail to apply the cortex-icons theme, falling back to defaults and breaking the intended login screen branding. Either implement the cortex-icons theme and install it in both configurations, or align both GDM and the live session to use the same icon theme (e.g., Adwaita).
🤖 Prompt for AI Agents
In @branding/gdm/gdm-branding.conf around lines 20 - 28, The GDM config sets
icon-theme='cortex-icons' in the [org/gnome/desktop/interface] section but that
theme isn’t provided; either remove that cortex-icons entry or install/ship the
cortex-icons theme and ensure it’s referenced identically in the live session
hooks (the live/99-configure-gnome.hook.chroot usage that currently uses
icon-theme='Adwaita'); update the GDM key icon-theme='cortex-icons' to
icon-theme='Adwaita' if you prefer to align with the live session, or add the
cortex-icons theme package and update both GDM and live session hooks to use it.
| ```bash | ||
| # Debian/Ubuntu | ||
| sudo apt update | ||
| sudo apt install live-build gpg python3 shellcheck debootstrap | ||
|
|
||
| # Verify live-build version (need >= 1:20210814) | ||
| dpkg -l live-build |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
live-build version 1:20210814 Debian Ubuntu 2025 2026 repository support
💡 Result:
Short answer: live-build version "1:20210814" is not present in current Debian or Ubuntu archives — Debian packages in stable/testing/unstable use other timestamps (examples: 1:20210407 in bullseye, 1:20230502 in bookworm, 1:20250505 in trixie and 1:20250814 in forky/sid), and Ubuntu publishes live-build as the 3.0~a57... series. See Debian and Ubuntu package pages below for details. [1][2][3]
Sources:
- Debian live-build package listings (bullseye/bookworm/trixie/forky/sid). [1]
- Debian live-build (source in trixie). [2]
- Ubuntu / Launchpad live-build package versions (jammy/noble). [3]
🌐 Web query:
live-build latest stable version Debian bullseye bookworm trixie 2025 2026
💡 Result:
Summary (live-build package versions in Debian suites):
- Trixie (stable): 1:20250505+deb13u1. [1][2]
- Bookworm (oldstable): 1:20230502. [1][3]
- Bullseye (oldoldstable): 1:20210407. [1][4]
Source: Debian package pages for live-build. [1][2][3][4]
🌐 Web query:
live-build package version history Debian live-build >= 1.20210814
💡 Result:
Here are Debian live-build source-package versions (chronological, versions >= 20210814) with Debian references:
- 1:20220505 — appeared in testing (after 2021). [1]
- 1:20230131 — migrated to testing Feb 2023. [2]
- 1:20230502 — migrated to testing May 2023 / in bookworm. [1][3]
- 1:20240810 — (found in bug reports / tracker as a later release). [4]
- 1:20250225 — later unstable/testing snapshot (seen in bug tracker). [4]
- 1:20250814 — current unstable/forky/sid source package (Debian VCS / package page). [1][5]
Sources:
- packages.debian.org — live-build package listings (versions by suite). [packages.debian.org live-build]. [1]
- Debian package tracker / news (migration 2023-02-03: 1:20230131). [tracker.debian.org]. [2]
- PTS / news (1:20230502 migrated to testing). [packages.qa.debian.org]. [3]
- Debian Bug tracker / package news entries citing versions 20240810, 20250225. [bugs.debian.org / tracker]. [4]
- live-build source package page (sid) and Salsa git (1:20250814). [packages.debian.org source live-build]. [5]
If you want a full changelog (full entries/dates) I can fetch the Debian changelog file (debian/changelog) and paste the entries for each version.
The version requirement live-build >= 1:20210814 does not exist in any Debian or Ubuntu repository.
The version "1:20210814" is not published in Debian (bullseye, bookworm, trixie, or sid) or Ubuntu archives. Current available versions are: 1:20210407 (bullseye), 1:20230502 (bookworm), and 1:20250505 (trixie).
Update the version requirement to a valid version that exists in the target distribution. If bullseye support is not required, change this to live-build >= 1:20230502 (bookworm+) or the appropriate version for your minimum supported Debian release.
🤖 Prompt for AI Agents
In @docs/DEVELOPERS.md around lines 9 - 15, The documented live-build version
requirement is invalid; update the version string in the DEVELOPER doc block
where the live-build install instructions and the "Verify live-build version"
check appear so it references a real release for your supported distro (e.g.,
change "live-build >= 1:20210814" to a valid version such as "live-build >=
1:20230502" for bookworm+ or to the specific minimum available in your
minimum-supported release); ensure the comment and the dpkg verification line
reflect the chosen valid version.
| ```bash | ||
| # System upgrade with rollback support | ||
| cortex-upgrade --check | ||
|
|
||
| # GPU detection and driver status | ||
| cortex-gpu status | ||
|
|
||
| # Verify system integrity | ||
| cortex-verify --quick | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find tool implementations
echo "=== Searching for tool implementations ==="
find . -type f -name "*cortex*" | grep -E "(cortex-upgrade|cortex-gpu|cortex-verify)" | head -20
find . -type f \( -name "*.py" -o -name "*.js" -o -name "*.ts" -o -name "*.sh" \) | xargs grep -l "cortex-upgrade\|cortex-gpu\|cortex-verify" 2>/dev/null | head -20Repository: cortexlinux/cortex-distro
Length of output: 347
🏁 Script executed:
#!/bin/bash
# Search for CLI argument parsing for these tools
echo "=== Searching for argument/flag definitions ==="
rg "add_argument.*--check|argparse|click.*--check" -t py -t js -A 2 | head -50
rg "cortex-upgrade|cortex-gpu|cortex-verify" -t py -A 5 | head -100Repository: cortexlinux/cortex-distro
Length of output: 4796
🏁 Script executed:
#!/bin/bash
# Look for main entry points and CLI definitions
fd -t f "main.py\|cli.py\|index.js\|app.py" | xargs grep -l "cortex-upgrade\|cortex-gpu\|cortex-verify" 2>/dev/null
ls -la . | head -20
git ls-files | grep -E "cortex|cli|main" | head -30Repository: cortexlinux/cortex-distro
Length of output: 2189
🏁 Script executed:
#!/bin/bash
# Read cortex-upgrade.py to check all arguments
echo "=== cortex-upgrade.py arguments ==="
cat packages/cortex-core/cortex-upgrade.py | grep -A 50 "parser.add_argument"Repository: cortexlinux/cortex-distro
Length of output: 2414
🏁 Script executed:
#!/bin/bash
# Read cortex-gpu.py to check all arguments
echo "=== cortex-gpu.py arguments and subcommands ==="
cat packages/cortex-core/cortex-gpu.py | grep -A 50 "parser.add_argument\|subparsers\|add_parser"Repository: cortexlinux/cortex-distro
Length of output: 3027
🏁 Script executed:
#!/bin/bash
# Read cortex-verify.py to check all arguments
echo "=== cortex-verify.py arguments ==="
cat packages/cortex-core/cortex-verify.py | grep -A 30 "parser.add_argument"Repository: cortexlinux/cortex-distro
Length of output: 1299
Update command examples to match actual tool interfaces.
The documentation contains inaccurate command flags:
cortex-upgrade --checkdoes not exist; usecortex-upgrade planinstead (commands:plan,execute,rollback,status)cortex-verify --quickdoes not exist; usecortex-verifyto run all checks or add flags like--jsonfor output formattingcortex-gpu statusis correct and supported
🤖 Prompt for AI Agents
In @docs/QUICK-START.md around lines 54 - 63, The example commands in the docs
use incorrect flags; replace `cortex-upgrade --check` with the supported
`cortex-upgrade plan` (and mention the valid subcommands: plan, execute,
rollback, status) and change `cortex-verify --quick` to just `cortex-verify` (or
note supported output flags like `--json`) while leaving `cortex-gpu status`
as-is; update the README examples and any occurrences of `cortex-upgrade
--check` and `cortex-verify --quick` to the correct command forms
(`cortex-upgrade plan` and `cortex-verify`) and optionally add a brief note
about `--json` output for `cortex-verify`.
iso/live-build/config/includes.binary/boot/grub/themes/cortex/theme.txt
Outdated
Show resolved
Hide resolved
| # Set Plymouth theme | ||
| if command -v plymouth-set-default-theme &>/dev/null; then | ||
| plymouth-set-default-theme cortex || true | ||
| update-initramfs -u || true | ||
| fi | ||
|
|
||
| # Update GRUB theme | ||
| if [ -f /etc/default/grub ]; then | ||
| if ! grep -q "GRUB_THEME=" /etc/default/grub; then | ||
| echo 'GRUB_THEME="/boot/grub/themes/cortex/theme.txt"' >> /etc/default/grub | ||
| else | ||
| sed -i 's|^GRUB_THEME=.*|GRUB_THEME="/boot/grub/themes/cortex/theme.txt"|' /etc/default/grub | ||
| fi | ||
| update-grub 2>/dev/null || true | ||
| fi | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/cortex-branding/DEBIAN -type fRepository: cortexlinux/cortex-distro
Length of output: 187
🏁 Script executed:
cat -n packages/cortex-branding/DEBIAN/postinstRepository: cortexlinux/cortex-distro
Length of output: 2625
🏁 Script executed:
[ -f packages/cortex-branding/DEBIAN/prerm ] && cat -n packages/cortex-branding/DEBIAN/prerm || echo "No prerm script found"Repository: cortexlinux/cortex-distro
Length of output: 942
🏁 Script executed:
[ -f packages/cortex-branding/DEBIAN/postrm ] && cat -n packages/cortex-branding/DEBIAN/postrm || echo "No postrm script found"Repository: cortexlinux/cortex-distro
Length of output: 91
GRUB_THEME not restored on package removal — leaves broken reference pointing to deleted file.
The postinst script mutates /etc/default/grub (lines 17-22) but prerm does not restore it. When the package is removed, GRUB_THEME="/boot/grub/themes/cortex/theme.txt" remains in the file even though the directory no longer exists, causing boot-time failures or GRUB warnings.
Either save the previous GRUB_THEME value to /var/lib/cortex-branding/grub-theme.prev in postinst and restore it in prerm, or remove the GRUB_THEME line entirely during package removal.
Note: Plymouth theme restoration is already correctly handled via plymouth-set-default-theme -R in prerm (line 12).
🤖 Prompt for AI Agents
In @packages/cortex-branding/DEBIAN/postinst around lines 10 - 25, Postinst sets
GRUB_THEME in /etc/default/grub but prerm does not restore it, leaving a broken
GRUB_THEME pointing to a removed file; modify postinst to save the previous
GRUB_THEME value (or empty marker) to /var/lib/cortex-branding/grub-theme.prev
before writing the new GRUB_THEME and modify prerm to read
/var/lib/cortex-branding/grub-theme.prev and either restore the original
GRUB_THEME line in /etc/default/grub (if a previous value exists) or remove the
GRUB_THEME line entirely (if marker indicates none), then run update-grub (or
update-grub 2>/dev/null || true) to apply changes; reference the scripts
postinst and prerm, the GRUB_THEME entry in /etc/default/grub, and the state
file /var/lib/cortex-branding/grub-theme.prev when implementing.
| set -e | ||
|
|
||
| case "$1" in | ||
| remove|upgrade|deconfigure) | ||
| # Restore default Plymouth theme | ||
| if command -v plymouth-set-default-theme &>/dev/null; then | ||
| plymouth-set-default-theme -R 2>/dev/null || true | ||
| fi | ||
|
|
||
| # Re-enable default MOTD scripts | ||
| if [ -d /etc/update-motd.d ]; then | ||
| for f in /etc/update-motd.d/??-* ; do | ||
| chmod +x "$f" 2>/dev/null || true | ||
| done | ||
| fi | ||
| ;; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the Debian package files related to cortex-branding
find packages/cortex-branding/DEBIAN -type f | head -20Repository: cortexlinux/cortex-distro
Length of output: 187
🏁 Script executed:
# Check the postinst script to see what state is actually persisted
if [ -f packages/cortex-branding/DEBIAN/postinst ]; then
cat -n packages/cortex-branding/DEBIAN/postinst
fiRepository: cortexlinux/cortex-distro
Length of output: 2625
🏁 Script executed:
# Check for any other relevant scripts (preinst, postrm)
ls -la packages/cortex-branding/DEBIAN/Repository: cortexlinux/cortex-distro
Length of output: 350
🌐 Web query:
plymouth-set-default-theme -R flag documentation
💡 Result:
The -R flag is --rebuild-initrd. It tells plymouth-set-default-theme to rebuild the initial ramdisk (initrd) after changing the Plymouth theme (necessary because Plymouth is loaded from initrd). [1][2]
Example: sudo plymouth-set-default-theme -R THEME. [1]
Sources:
[1] Debian man page — plymouth-set-default-theme.
[2] Mankier / man page summary for plymouth-set-default-theme.
🌐 Web query:
what does plymouth-set-default-theme -R do restore default theme
💡 Result:
-
-R (or --rebuild-initrd) tells plymouth-set-default-theme to rebuild the initrd so the chosen theme is embedded into the initial ramdisk (necessary for the theme to actually appear at boot). [1][2]
-
It does not reset the theme to the default. Use -r (or --reset) to reset to the default theme; use -R after changing or resetting to ensure the initrd is rebuilt. [1][2]
Sources: [1] Debian manpage for plymouth-set-default-theme. [2] Arch/manpage (mankier) for plymouth-set-default-theme.
🌐 Web query:
plymouth-set-default-theme command syntax requires theme argument
💡 Result:
The script requires a theme name when you intend to change the theme. Usage (from the man page):
- Show current theme: plymouth-set-default-theme
- List available themes: plymouth-set-default-theme -l. [1]
- Set a theme: sudo plymouth-set-default-theme THEME (optionally add -R or --rebuild-initrd to rebuild initrd). [1][2]
Note: On Ubuntu the recommended method is update-alternatives --config default.plymouth to choose the default.plymouth file. [2]
Sources:
[1] plymouth-set-default-theme man page. [2] Ask Ubuntu — how to install/choose Plymouth themes.
🌐 Web query:
what happens plymouth-set-default-theme -R without theme name error
💡 Result:
If you run plymouth-set-default-theme -R without giving a THEME, the command uses the currently selected theme and rebuilds the initrd (i.e. regenerates the initial ramdisk for that current theme). The manpage documents that invoking with no parameters shows the current theme, and -R/--rebuild-initrd rebuilds the initrd. [1][2]
Sources:
- plymouth-set-default-theme man page (description and -R option). [1]
- Arch/manpage copy (synopsis/behavior). [2]
Removal path doesn't reliably restore Plymouth theme and indiscriminately re-enables MOTD scripts.
plymouth-set-default-theme -R(Line 12) rebuilds the initrd for the currently set theme but does not restore the theme that existed before the package was installed. The package should either save the previous theme in postinst and restore it here, or use-r -Rto reset to the system default and rebuild.- The loop
chmod +x /etc/update-motd.d/??-*(Lines 17-19) re-enables all scripts indiscriminately, but postinst selectively disables non-cortex scripts (postinst lines 34–39). Without tracking which scripts were actually disabled by this package, prerm cannot safely restore the original state.
Recommended approach (requires pairing with postinst to persist state)
- if command -v plymouth-set-default-theme &>/dev/null; then
- plymouth-set-default-theme -R 2>/dev/null || true
- fi
+ if command -v plymouth-set-default-theme &>/dev/null; then
+ if [ -f /var/lib/cortex-branding/plymouth-theme.prev ]; then
+ prev_theme="$(cat /var/lib/cortex-branding/plymouth-theme.prev)"
+ plymouth-set-default-theme "$prev_theme" -R 2>/dev/null || true
+ rm -f /var/lib/cortex-branding/plymouth-theme.prev
+ fi
+ fi
@@
- if [ -d /etc/update-motd.d ]; then
- for f in /etc/update-motd.d/??-* ; do
- chmod +x "$f" 2>/dev/null || true
- done
- fi
+ # Re-enable only scripts we disabled (tracked in postinst)
+ if [ -f /var/lib/cortex-branding/motd-disabled.list ]; then
+ while IFS= read -r f; do
+ [ -e "$f" ] && chmod +x "$f" 2>/dev/null || true
+ done < /var/lib/cortex-branding/motd-disabled.list
+ rm -f /var/lib/cortex-branding/motd-disabled.list
+ fi🤖 Prompt for AI Agents
In @packages/cortex-branding/DEBIAN/prerm around lines 6 - 21, The prerm
currently calls "plymouth-set-default-theme -R" which only rebuilds initrd for
the current theme; change the flow so the package either saves the prior theme
in postinst (e.g., write the original theme name to a persistent state file) and
then prerm reads that state and restores it, or replace "-R" with "-r -R" to
reset to system default and rebuild; likewise, stop indiscriminately chmod'ing
all /etc/update-motd.d/??-* scripts — have postinst record exactly which MOTD
scripts this package disabled (e.g., write their filenames to a state file) and
make prerm iterate only over that recorded list to re-enable (chmod +x) those
files, leaving other scripts untouched (refer to the postinst logic that
disables non-cortex scripts in lines 34–39 for exact criteria).
| Package: cortex-core | ||
| Version: 0.1.0 | ||
| Section: metapackages | ||
| Priority: optional | ||
| Architecture: all | ||
| Depends: cortex-archive-keyring, cortex-upgrade, cortex-gpu, cortex-verify | ||
| Recommends: neofetch, htop, curl, wget, git | ||
| Suggests: cortex-full, cortex-secops | ||
| Maintainer: Cortex Linux Team <team@cortexlinux.com> | ||
| Homepage: https://cortexlinux.com | ||
| Description: Cortex Linux core meta-package with CLI tools | ||
| This meta-package installs the core Cortex Linux components: | ||
| . | ||
| - cortex-upgrade: Safe system upgrades with LVM/Btrfs/ZFS snapshot rollback | ||
| - cortex-gpu: GPU detection, NVIDIA/AMD driver enablement, Secure Boot MOK setup | ||
| - cortex-verify: Offline integrity verification | ||
| - cortex-archive-keyring: GPG keyring for Cortex APT repository trust | ||
| . | ||
| Install this package to get the essential Cortex Linux tools on any | ||
| Debian or Ubuntu system. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the dependency packages in this repository
for pkg in cortex-archive-keyring cortex-upgrade cortex-gpu cortex-verify cortex-full cortex-secops; do
echo "Searching for package: $pkg"
fd -t d "^${pkg}$" packages/ || echo " Not found in packages/"
doneRepository: cortexlinux/cortex-distro
Length of output: 374
🏁 Script executed:
#!/bin/bash
# Search more broadly for the missing packages
echo "=== Searching entire repository for package definitions ==="
for pkg in cortex-upgrade cortex-gpu cortex-verify; do
echo ""
echo "Looking for $pkg:"
fd -t d "$pkg" . 2>/dev/null | head -20
done
echo ""
echo "=== Checking git ls-files for these package names ==="
git ls-files | grep -E "(cortex-upgrade|cortex-gpu|cortex-verify)" | head -20Repository: cortexlinux/cortex-distro
Length of output: 756
🏁 Script executed:
#!/bin/bash
# Check if cortex-gpu-amd and cortex-gpu-nvidia have proper control files
echo "=== Checking GPU package control files ==="
for dir in packages/cortex-gpu-amd packages/cortex-gpu-nvidia; do
if [ -f "$dir/DEBIAN/control" ]; then
echo "Found: $dir/DEBIAN/control"
sed -n '1,3p' "$dir/DEBIAN/control"
elif [ -f "$dir/debian/control" ]; then
echo "Found: $dir/debian/control"
sed -n '1,3p' "$dir/debian/control"
fi
done
echo ""
echo "=== Searching for cortex-upgrade and cortex-verify package definitions ==="
find packages -name "control" -type f -exec grep -l "Package: cortex-upgrade\|Package: cortex-verify" {} \;
echo ""
echo "=== List all packages with DEBIAN or debian control files ==="
find packages -name "control" -type f | sed 's|/[^/]*/control||' | sort -uRepository: cortexlinux/cortex-distro
Length of output: 655
Fix critical dependency declaration on line 6.
The Depends field declares 4 packages, but only 1 exists in the repository:
- ✓
cortex-archive-keyring— exists - ✗
cortex-upgrade— does not exist - ✗
cortex-gpu— does not exist (repository hascortex-gpu-amdandcortex-gpu-nvidiainstead) - ✗
cortex-verify— does not exist
Missing or incorrectly named dependencies will cause installation failures. Update line 6 to declare only packages that exist, or replace cortex-gpu with the appropriate GPU variant(s) if this is a virtual package that should be provided elsewhere.
🤖 Prompt for AI Agents
In @packages/cortex-core/DEBIAN/control around lines 1 - 20, The control file's
Depends field lists non-existent packages causing install failures; open the
DEBIAN control and update the Depends line to reference only existing packages
(keep cortex-archive-keyring), remove or replace cortex-upgrade and
cortex-verify with their correct package names if they exist, and replace
cortex-gpu with the actual GPU package(s) (e.g., cortex-gpu-amd and/or
cortex-gpu-nvidia) or change it to a Suggests/Provides if it’s meant to be
virtual; ensure the Depends field remains a comma-separated list and adjust
Recommends/Suggests accordingly if you move packages out of Depends.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @Makefile:
- Around line 194-201: The Makefile rule currently copies preseed and
provisioning files using cp with failures suppressed (commands with 2>/dev/null
|| true), which hides missing files; update the rule to perform the copies
normally and then explicitly validate that expected files exist in
$(BUILD_DIR)/$(1)/config/includes.binary/preseed/,
$(BUILD_DIR)/$(1)/config/includes.binary/preseed/profiles/,
$(BUILD_DIR)/$(1)/config/includes.binary/preseed/partitioning/ and
$(BUILD_DIR)/$(1)/config/includes.binary/provisioning/ (use shell tests like [
-e ] or glob checks) and if any required files from $(PRESEED_DIR) or
$(PROVISION_DIR) are missing emit an error message and exit non‑zero so the
build fails early; keep references to the existing variables PRESEED_DIR,
PROVISION_DIR, BUILD_DIR and the copy commands to locate where to add the
checks.
- Around line 205-206: The mv invocation that moves the generated ISO currently
swallows errors via `2>/dev/null || true`, masking failures; remove the
redirection and the `|| true` and instead ensure the move reports failures and
fails the build (e.g., let mv return a non-zero exit code or on failure print a
clear error and exit 1). Update the line with the `mv
$(BUILD_DIR)/$(1)/live-image-$(ARCH).hybrid.iso
$(OUTPUT_DIR)/$(ISO_NAME)-$(1)-$(ISO_VERSION)-$(ARCH).iso` invocation so any
failure is propagated (or explicitly handled by printing an error mentioning the
ISO name and exiting non-zero) to prevent false-success builds.
- Around line 160-177: The Makefile silences copy failures with "2>/dev/null ||
true" causing missing branding/hook/bootloader assets to go unnoticed; update
the copy blocks referencing BUILD_DIR and ISO_DIR (the package-lists cp, hooks
cp, includes.chroot/includes.binary/bootloaders cp blocks) to validate source
existence before copying and let failures surface: remove the "2>/dev/null ||
true" for critical assets, replace with explicit existence checks (e.g. if [ -d
"$(ISO_DIR)/live-build/config/includes.chroot" ]; then cp -r ... || { echo >&2
"Failed to copy includes.chroot"; exit 1; }; else echo >&2 "Missing
includes.chroot"; exit 1; fi), and decide which noncritical assets may log a
warning instead of exiting so builds fail visibly when required branding or
bootloader files are absent.
In @scripts/build.sh:
- Around line 180-197: In scripts/build.sh, validate the required branding
assets before copying instead of silencing errors: check that all expected files
(e.g., "${PROJECT_ROOT}/branding/grub/cortex/theme.txt" and the specific PNGs
like background.png and any other required *.png) exist and log a clear error
then exit non‑zero if any required file is missing; replace the silent cp
"${PROJECT_ROOT}/branding/grub/cortex/"*.png 2>/dev/null || true with an
explicit existence check and selective copy of known filenames (or iterate the
glob after confirming it expands) so missing files cause a clear failure, and
avoid using a broad wildcard when copying to
config/includes.chroot/boot/grub/themes/cortex—use the validated list of files
to copy and use consistent logging (log error + exit) on validation failures
rather than mixing silent suppression and hard failures.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.gitignoreMakefileiso/live-build/auto/buildiso/live-build/auto/cleaniso/live-build/auto/configiso/live-build/config/bootloaders/grub-pc/config.cfgiso/live-build/config/package-lists/cortex-core.list.chrootscripts/build.sh
💤 Files with no reviewable changes (4)
- iso/live-build/auto/clean
- iso/live-build/auto/build
- iso/live-build/config/package-lists/cortex-core.list.chroot
- iso/live-build/auto/config
🚧 Files skipped from review as they are similar to previous changes (2)
- iso/live-build/config/bootloaders/grub-pc/config.cfg
- .gitignore
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 52-52: Target body for "help" exceeds allowed length of 5 (45).
(maxbodylength)
[warning] 136-136: Target body for "lint" exceeds allowed length of 5 (6).
(maxbodylength)
[warning] 241-241: Target body for "test" exceeds allowed length of 5 (14).
(maxbodylength)
[warning] 258-258: Target body for "clean" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 321-321: Target body for "config" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 320-320: Missing required phony target "clean"
(minphony)
[warning] 320-320: Missing required phony target "test"
(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). (1)
- GitHub Check: Build (First Pass)
🔇 Additional comments (1)
Makefile (1)
100-112: LGTM! Dependency checking is thorough and user-friendly.The dependency validation logic properly checks for:
- Required commands using
command -v- live-build version compatibility with Debian Bookworm
- Python 3.11+ with graceful degradation (warning vs error)
The implementation provides clear, actionable error messages to help users resolve dependency issues.
- Consolidated validation checks into a single build script. - Updated dependency checks and validation functions for better error handling. - Enhanced preseed and provisioning file management. - Streamlined ISO build process with clearer output and logging. - Improved branding package creation with better file handling and checks. - Removed obsolete first-boot provisioning script and integrated its functionality into the build process. - Added support for generating SBOM (Software Bill of Materials) during builds. - Updated hooks and configuration scripts for better compatibility and error handling.
- Added support for multiple ISO profiles (core, full) in the build process. - Introduced a validation job to check dependencies before building. - Created a dedicated job for building the branding package. - Updated workflow to include SBOM generation and improved artifact handling. - Enhanced release process to include combined checksums and branding package downloads. - Improved logging and output management for better traceability.
…treamline build process - Added installation of git and sudo in the validation and build jobs. - Consolidated dependency installation steps for improved clarity. - Introduced a containerized environment using Debian Bookworm for all jobs. - Removed redundant steps and improved overall workflow efficiency.
…lation - Added 'make' to the list of packages installed in the build process. - Improved clarity and completeness of the dependency management step.
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Around line 44-60: The container jobs "validate" (runs 'make validate') and
the Branding Package job (runs 'make branding-package') install git/shellcheck
but not GNU make, causing the make invocations to fail; update the
package-install step(s) named "Install git" in the 'validate' job and the
corresponding install step in the Branding Package job to include make (e.g.,
add 'make' or 'build-essential' to the apt-get install -y list) so that the
subsequent 'make validate' and 'make branding-package' commands succeed.
- Around line 27-39: The workflow_dispatch input profile is unused and
misleading because jobs build-iso and test-iso currently run a fixed matrix
([core, full]) and ignore secops; update the jobs to honor the workflow_dispatch
input by filtering the matrix on manual runs (or by generating the matrix from
the input). Specifically, in the build-iso and test-iso jobs, reference
github.event.workflow_dispatch.inputs.profile and add an if condition to skip
matrix entries when the job was manually triggered (e.g., if: github.event_name
!= 'workflow_dispatch' ||
contains(split(github.event.workflow_dispatch.inputs.profile, ','),
matrix.profile)), or alternatively compute the matrix dynamically from the input
via fromJSON/values so secops can be included when provided; make sure to update
the matrix key name (matrix.profile) used by those jobs so selected profiles
actually run.
- Around line 19-27: The pull_request path filter currently omits the workflow
itself so PRs that only change .github/workflows/build-iso.yml won’t trigger CI;
update the pull_request: paths list (the pull_request.paths key in the workflow)
to include the workflow file (for example add '.github/workflows/build-iso.yml'
or a broader '.github/workflows/**' pattern), or remove the paths filter
entirely so workflow changes trigger the workflow; edit the build-iso.yml to
modify the pull_request.paths entry accordingly.
- Around line 235-236: Replace the GitHub Action reference to
softprops/action-gh-release@v1 with the v2 release to avoid Node 16 usage;
update the workflow step that currently uses softprops/action-gh-release@v1 to
softprops/action-gh-release@v2 (or a specific v2.x tag such as v2.5.0) so the
action runs on Node 20+.
In @iso/live-build/config/hooks/live/0100-cortex-config.hook.chroot:
- Around line 133-163: The unit can still run and fail if the first-boot script
is absent and calling systemctl enable in a chroot is unreliable; update the
unit (cortex-first-boot.service) to add a
ConditionPathExists=/opt/cortex/provisioning/first-boot.sh in addition to the
existing ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete so
the unit won’t be scheduled when the script is missing, and replace the runtime
systemctl enable call with creating the multi-user.target symlink yourself
(create a symlink from the cortex-first-boot.service unit into
multi-user.target.wants) so the unit is enabled in the offline/chroot build
environment without requiring a running systemd.
In @iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot:
- Around line 9-12: The build hook currently uses non-deterministic network
fetches and only sets "set -e"; change the script to be failure-safe and
deterministic by adding "set -euo pipefail" at the top and replacing the remote
key download with a local, versioned key file shipped in the tree (e.g., use a
bundled file like cortex-apt-key.gpg and import it via apt-key add or gpg
--dearmor into /usr/share/keyrings), update the repository entry to reference
the local keyring, and ensure all commands that read from pipes will fail on
errors so failures are clear and reproducible.
- Around line 93-104: The GRUB_DISTRIBUTOR update uses `sed -i
's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub ||
echo ...` which fails to append when no match is present because `sed` exits 0;
change the logic to explicitly test for existence (e.g., `if grep -q
"^GRUB_DISTRIBUTOR=" /etc/default/grub; then sed -i
's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub;
else echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub; fi`) so that
`GRUB_DISTRIBUTOR` is replaced if present and appended if missing.
- Around line 49-63: This hook redundantly adds the apt.cortexlinux.com
repository and a different keyring (cortex-linux.gpg) which conflicts with the
canonical repo.cortexlinux.com/apt configured in 0100-cortex-config.hook.chroot
(which uses cortex-archive-keyring.gpg); remove the block that creates
/usr/share/keyrings/cortex-linux.gpg and the echo writing "deb
[signed-by=/usr/share/keyrings/cortex-linux.gpg] https://apt.cortexlinux.com
stable main" so only the canonical repo (repo.cortexlinux.com/apt with
cortex-archive-keyring.gpg) remains, or if consolidation is required replace the
apt.cortexlinux.com lines to match the repo.cortexlinux.com/apt entry and use
cortex-archive-keyring.gpg.
In @iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot:
- Around line 8-44: The hook must skip GNOME/GDM steps when dconf isn't present
to avoid failing non-GNOME profiles; wrap the GNOME-specific sections (creation
of /etc/live/config.conf.d/cortex-gnome.conf, /etc/gdm3/daemon.conf and
/etc/gdm3/custom.conf, and the systemctl set-default graphical.target call) in a
conditional that checks for dconf (e.g. if command -v dconf >/dev/null 2>&1;
then ... fi), and replace the unconditional dconf update call with the same
guarded call pattern (mirroring the existing conditional used for
glib-compile-schemas).
In @iso/provisioning/first-boot.sh:
- Around line 41-58: check_already_complete calls log_info before logging is
initialized which can fail because LOG_DIR/LOG_FILE may not exist; move logging
initialization (create LOG_DIR and LOG_FILE) before any log_info calls or ensure
check_already_complete creates/validates LOG_DIR and LOG_FILE itself.
Specifically, either invoke init_logging() before calling check_already_complete
or add the same mkdir -p "${LOG_DIR}" && touch "${LOG_FILE}" && chmod 640
"${LOG_FILE}" steps into check_already_complete so log_info can safely write;
reference symbols: check_already_complete, init_logging, log_info, LOG_DIR,
LOG_FILE, STATE_FILE.
- Around line 471-484: The cleanup() function uses rm -f /var/log/installer/*
which can return non-zero if any entries are directories (and under set -e would
abort); change that to safely remove only files or tolerate directories: either
replace that line with a find invocation (e.g. use find /var/log/installer
-mindepth 1 -maxdepth 1 -type f -exec rm -f -- {} +) to delete only regular
files, or use rm -rf -- /var/log/installer/* || true to force recursive removal
and avoid exiting on non-zero. Ensure you update the line inside cleanup() where
rm -f /var/log/installer/* appears.
- Around line 97-131: Several places assume the cortex user exists and will
hard-fail; add an existence guard (e.g., check id -u cortex) before performing
user-specific actions in setup_password_change (guard the chage -d 0 cortex), in
full_desktop_setup (guard the groups cortex and usermod -aG docker cortex
calls), and in secops_hardening (guard writes to
/home/cortex/SECURITY-NOTICE.txt and chown cortex:cortex). If the check fails,
skip the user-specific steps and log a notice instead so the script remains
idempotent and tolerant of systems where the cortex user isn’t present.
- Around line 412-449: In setup_cortex_repository: ensure the keyring directory
exists before writing by running mkdir -p for the local variable keyring_dir,
stop piping curl directly into gpg and instead download the key to a temporary
file, verify its fingerprint against the expected value before importing,
dearmor and write the final key to ${keyring_dir}/cortex-linux.gpg, and set its
permissions to 0644 (chmod 644 "${keyring_dir}/cortex-linux.gpg"); keep using
the existing sources_file variable and existing apt-get update logic once the
key is validated and installed.
- Around line 132-184: In setup_ssh, the SSH hardening block is contradictory
and uses the wrong service name: the comment says "Disable password
authentication for root" while the file sets PasswordAuthentication yes, and the
reload checks for sshd service; change PasswordAuthentication to no (or update
the comment to accurately reflect that password auth remains enabled if
intended) so the config and comment align, and replace systemctl
is-active/reload checks referencing sshd with the Debian-standard service name
ssh (i.e., use systemctl is-active --quiet ssh and systemctl reload ssh ||
true); refer to the setup_ssh function and the sshd_config variable to locate
the changes.
In @repository/scripts/repo-manage.sh:
- Around line 136-143: The script currently uses touch "${arch_dir}/Packages"
which can leave stale content if the file already exists; change it to truncate
the file instead (overwrite with an empty file) when dpkg-scanpackages is not
run so Packages is emptied rather than preserved; update the branch in
repo-manage.sh that handles the else path after dpkg-scanpackages (the block
that currently calls touch "${arch_dir}/Packages") to create an empty/truncated
"${arch_dir}/Packages" before gzip -9 -k -f runs.
In @scripts/build.sh:
- Around line 94-154: The script currently treats dpkg-deb as optional in
cmd_check_deps but branding-package needs it; either mark dpkg-deb as required
by changing its check in cmd_check_deps to mirror other required checks (use
check_command dpkg-deb || failed=1 and ensure the final failed logic returns
non-zero), or add a dedicated runtime check inside cmd_branding_package that
tests command -v dpkg-deb and exits/fails with a clear error message if missing;
update references to the failed variable and error/warn calls so the failure
path is deterministic and the message explains that branding-package requires
dpkg-deb.
- Around line 67-76: The function copy_glob_if_exists has an unquoted glob and
uses cp without -r which breaks on filenames with spaces, directories, and
creates flaky builds; update it to safely expand globs into an array (or enable
nullglob), check if the array is non-empty, and iterate over matches quoting
each item when copying, and use cp -a (or cp -r --) to preserve directories and
handle recursive copies; keep references to the function name
copy_glob_if_exists and the variables pattern and dest when making these
changes.
- Around line 570-572: The live-build commands use unconditional sudo which
fails when running as root; update both invocations of "lb build" and "lb clean"
to run conditionally based on EUID: replace each `sudo lb build` and `sudo lb
clean` with an EUID check that runs `lb build`/`lb clean` directly if $EUID is
0, otherwise prefixes with sudo (e.g., if [[ $EUID -eq 0 ]]; then lb build; else
sudo lb build; fi and same pattern for lb clean).
🟡 Minor comments (4)
packages/cortex-branding/DEBIAN/prerm-15-20 (1)
15-20: Re-enabling every/etc/update-motd.d/??-*script can unintentionally override admin/system intent.If you can’t restore prior chmod state, consider only re-enabling known distro defaults (or only undo what your
postinstchanged).scripts/build.sh-600-610 (1)
600-610: SBOM generation should create the output dir (and consider notchmod +x-ing repo files during builds).At least
mkdir -p "${OUTPUT_DIR}/sbom"before invoking the generator.iso/provisioning/first-boot.sh-361-389 (1)
361-389: Hardcoded FDE device path (/dev/sda3) is risky and profile-dependent.This guidance appears only for the secops profile (when
REQUIRE_FDE_PASSPHRASE_CHANGE=true), but assumes a fixed partition scheme. On systems installed to different disks (nvme0n1, /dev/sdb, etc.), the command will fail. Consider either:
- Detecting the actual encrypted device at runtime (e.g.,
cryptsetup status cortex-sec-vg | grep device)- Injecting the device path via preseed/profile configuration
- Documenting the assumption that installations must use
/dev/sdaiso/live-build/config/hooks/live/50-cortex-branding.hook.chroot-124-147 (1)
124-147: Warn if GDM logo asset is missing (dconf will reference a non-existent file).Line 135 hardcodes
/usr/share/pixmaps/cortex-logo.pngwith no existence check (unlike wallpapers/plymouth/grub checks). Add a warning (or install it here) to avoid silent broken branding.
🧹 Nitpick comments (3)
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (1)
263-266: Don’t let initramfs update break the ISO build unexpectedly.Even if
update-initramfsexists (Line 264), it can fail depending on kernel/initramfs state in the chroot. Consider making it non-fatal or gating on presence of kernels.iso/live-build/config/hooks/live/0100-cortex-config.hook.chroot (2)
148-153: Hardening flags are explicitly disabled (verify this is intentional).Lines 149-151 set
NoNewPrivileges=false,ProtectSystem=false,ProtectHome=false. If the provisioning script truly needs broad writes, that’s fine, but then the “security hardening” intent is undermined.If you want hardening here, consider
NoNewPrivileges=trueat minimum, andProtectSystem=full+ReadWritePaths=for only the directories the script must mutate.
168-173: Service-existence check may be ineffective; confirmsystemctl list-unit-files <unit>behavior.Depending on systemd version,
systemctl list-unit-files "$svc"can exit 0 even if the unit isn’t present, which defeats the guard.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/build-iso.ymlMakefilebranding/install-branding.shiso/live-build/config/hooks/live/0100-cortex-config.hook.chrootiso/live-build/config/hooks/live/50-cortex-branding.hook.chrootiso/live-build/config/hooks/live/99-configure-gnome.hook.chrootiso/live-build/config/includes.chroot/usr/lib/cortex/firstboot.shiso/provisioning/first-boot.shpackages/cortex-branding/DEBIAN/postinstpackages/cortex-branding/DEBIAN/prermrepository/scripts/repo-manage.shscripts/build.sh
💤 Files with no reviewable changes (1)
- iso/live-build/config/includes.chroot/usr/lib/cortex/firstboot.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- branding/install-branding.sh
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
236-236: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 checkmake (0.2.2)
Makefile
[warning] 46-46: Target body for "help" exceeds allowed length of 5 (44).
(maxbodylength)
[warning] 204-204: Target body for "config" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 16-16: Missing required phony target "clean"
(minphony)
[warning] 16-16: Missing required phony target "test"
(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). (1)
- GitHub Check: Build (First Pass)
🔇 Additional comments (3)
packages/cortex-branding/DEBIAN/prerm (1)
6-21:plymouth-set-default-theme -Rwithout specifying a theme may not restore the “default” theme as intended.If the goal is “undo cortex theme,” consider restoring the previously active theme (stored during
postinst) or explicitly setting a known baseline theme for Debian Bookworm before running-R.Makefile (1)
16-21: Makefile structure is clear and consistent with script-driven build.(Optional) checkmake warnings about long
help/configbodies are cosmetic and can be ignored unless you enforce that linter.Also applies to: 46-91, 204-211
iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot (1)
25-44: No action needed. The GDM autologin configuration andgraphical.targetchange are already isolated to the live environment. The installed system is created fresh via debian-installer and preseed, which do not copy/etc/gdm3/or other live-only configurations. If GNOME is installed on the target system (e.g., viacortex-full.preseed), it will be installed from packages with default GDM settings, not inheriting the live autologin configuration.Likely an incorrect or invalid review comment.
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - 'iso/**' | ||
| - 'branding/**' | ||
| - 'packages/**' | ||
| - 'scripts/**' | ||
| - 'Makefile' | ||
| workflow_dispatch: |
There was a problem hiding this comment.
PR path filter omits the workflow file, so CI won’t run when only the workflow changes.
Push includes .github/workflows/build-iso.yml (Line 18) but pull_request does not (Lines 21-27).
Proposed fix
pull_request:
branches: [main]
paths:
- 'iso/**'
- 'branding/**'
- 'packages/**'
- 'scripts/**'
- 'Makefile'
+ - '.github/workflows/build-iso.yml'🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 19 - 27, The pull_request path
filter currently omits the workflow itself so PRs that only change
.github/workflows/build-iso.yml won’t trigger CI; update the pull_request: paths
list (the pull_request.paths key in the workflow) to include the workflow file
(for example add '.github/workflows/build-iso.yml' or a broader
'.github/workflows/**' pattern), or remove the paths filter entirely so workflow
changes trigger the workflow; edit the build-iso.yml to modify the
pull_request.paths entry accordingly.
| # Cortex first-boot service (canonical version from iso/provisioning/) | ||
| # Note: The actual first-boot.sh is deployed via preseed to /opt/cortex/provisioning/ | ||
| cat > /etc/systemd/system/cortex-first-boot.service << 'EOF' | ||
| [Unit] | ||
| Description=Cortex Linux First Boot Provisioning | ||
| After=network-online.target | ||
| Wants=network-online.target | ||
| ConditionPathExists=!/etc/cortex/.firstboot-complete | ||
| Description=Cortex Linux First-Boot Provisioning | ||
| Documentation=https://github.com/cortexlinux/cortex-distro | ||
| After=network-pre.target systemd-resolved.service | ||
| Before=network.target getty@tty1.service display-manager.service | ||
| Wants=network-pre.target | ||
| ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete | ||
| [Service] | ||
| Type=oneshot | ||
| ExecStart=/usr/lib/cortex/firstboot.sh | ||
| ExecStartPost=/usr/bin/touch /etc/cortex/.firstboot-complete | ||
| ExecStart=/opt/cortex/provisioning/first-boot.sh | ||
| RemainAfterExit=yes | ||
| TimeoutSec=600 | ||
| NoNewPrivileges=false | ||
| ProtectSystem=false | ||
| ProtectHome=false | ||
| PrivateTmp=true | ||
| StandardOutput=journal+console | ||
| StandardError=journal+console | ||
| SyslogIdentifier=cortex-first-boot | ||
| [Install] | ||
| WantedBy=multi-user.target | ||
| EOF | ||
|
|
||
| # Enable first-boot service | ||
| systemctl enable cortex-firstboot.service | ||
| systemctl enable cortex-first-boot.service | ||
|
|
There was a problem hiding this comment.
First-boot unit: ensure it won’t run (and fail) on live boot, and don’t rely on systemctl enable working in chroot.
- The unit only checks
!/opt/cortex/provisioning/.first-boot-complete(Line 142). If/opt/cortex/provisioning/first-boot.shisn’t present in the live environment, it’ll still attempt to start and error. systemctl enable ...in a build chroot (Line 162) is a common failure mode unless done in offline mode (or by creating the symlink yourself).
Proposed fix (safer conditions + enable without needing a running systemd)
[Unit]
@@
-ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete
+ConditionPathExists=/opt/cortex/provisioning/first-boot.sh
+ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete-# Enable first-boot service
-systemctl enable cortex-first-boot.service
+# Enable first-boot service (avoid requiring a running systemd in chroot)
+mkdir -p /etc/systemd/system/multi-user.target.wants
+ln -sf ../cortex-first-boot.service /etc/systemd/system/multi-user.target.wants/cortex-first-boot.service📝 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.
| # Cortex first-boot service (canonical version from iso/provisioning/) | |
| # Note: The actual first-boot.sh is deployed via preseed to /opt/cortex/provisioning/ | |
| cat > /etc/systemd/system/cortex-first-boot.service << 'EOF' | |
| [Unit] | |
| Description=Cortex Linux First Boot Provisioning | |
| After=network-online.target | |
| Wants=network-online.target | |
| ConditionPathExists=!/etc/cortex/.firstboot-complete | |
| Description=Cortex Linux First-Boot Provisioning | |
| Documentation=https://github.com/cortexlinux/cortex-distro | |
| After=network-pre.target systemd-resolved.service | |
| Before=network.target getty@tty1.service display-manager.service | |
| Wants=network-pre.target | |
| ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete | |
| [Service] | |
| Type=oneshot | |
| ExecStart=/usr/lib/cortex/firstboot.sh | |
| ExecStartPost=/usr/bin/touch /etc/cortex/.firstboot-complete | |
| ExecStart=/opt/cortex/provisioning/first-boot.sh | |
| RemainAfterExit=yes | |
| TimeoutSec=600 | |
| NoNewPrivileges=false | |
| ProtectSystem=false | |
| ProtectHome=false | |
| PrivateTmp=true | |
| StandardOutput=journal+console | |
| StandardError=journal+console | |
| SyslogIdentifier=cortex-first-boot | |
| [Install] | |
| WantedBy=multi-user.target | |
| EOF | |
| # Enable first-boot service | |
| systemctl enable cortex-firstboot.service | |
| systemctl enable cortex-first-boot.service | |
| # Cortex first-boot service (canonical version from iso/provisioning/) | |
| # Note: The actual first-boot.sh is deployed via preseed to /opt/cortex/provisioning/ | |
| cat > /etc/systemd/system/cortex-first-boot.service << 'EOF' | |
| [Unit] | |
| Description=Cortex Linux First-Boot Provisioning | |
| Documentation=https://github.com/cortexlinux/cortex-distro | |
| After=network-pre.target systemd-resolved.service | |
| Before=network.target getty@tty1.service display-manager.service | |
| Wants=network-pre.target | |
| ConditionPathExists=/opt/cortex/provisioning/first-boot.sh | |
| ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete | |
| [Service] | |
| Type=oneshot | |
| ExecStart=/opt/cortex/provisioning/first-boot.sh | |
| RemainAfterExit=yes | |
| TimeoutSec=600 | |
| NoNewPrivileges=false | |
| ProtectSystem=false | |
| ProtectHome=false | |
| PrivateTmp=true | |
| StandardOutput=journal+console | |
| StandardError=journal+console | |
| SyslogIdentifier=cortex-first-boot | |
| [Install] | |
| WantedBy=multi-user.target | |
| EOF | |
| # Enable first-boot service (avoid requiring a running systemd in chroot) | |
| mkdir -p /etc/systemd/system/multi-user.target.wants | |
| ln -sf ../cortex-first-boot.service /etc/systemd/system/multi-user.target.wants/cortex-first-boot.service |
🤖 Prompt for AI Agents
In @iso/live-build/config/hooks/live/0100-cortex-config.hook.chroot around lines
133 - 163, The unit can still run and fail if the first-boot script is absent
and calling systemctl enable in a chroot is unreliable; update the unit
(cortex-first-boot.service) to add a
ConditionPathExists=/opt/cortex/provisioning/first-boot.sh in addition to the
existing ConditionPathExists=!/opt/cortex/provisioning/.first-boot-complete so
the unit won’t be scheduled when the script is missing, and replace the runtime
systemctl enable call with creating the multi-user.target symlink yourself
(create a symlink from the cortex-first-boot.service unit into
multi-user.target.wants) so the unit is enabled in the offline/chroot build
environment without requiring a running systemd.
| set -e | ||
|
|
||
| echo "Installing Cortex Linux branding..." | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make the APT key/repo setup deterministic and failure-safe (avoid network fetch in build hook).
Line 57 downloads a signing key during the build; that’s brittle (network/endpoint changes) and a supply-chain risk. Also, with set -e (Line 9) and no pipefail, failures can be confusing in pipelines.
Proposed direction (ship key in tree + safer bash settings)
-set -e
+set -euo pipefail-# Download and install GPG key
-mkdir -p /usr/share/keyrings
-curl -fsSL https://apt.cortexlinux.com/keys/cortex-linux.gpg.asc | gpg --dearmor -o /usr/share/keyrings/cortex-linux.gpg
+mkdir -p /usr/share/keyrings
+# Prefer: include the armored key in the repo and install it from includes.chroot (no network).
+# If you must download here, at least verify a pinned fingerprint after download.
+curl -fsSL "$CORTEX_KEY_URL" | gpg --dearmor -o /usr/share/keyrings/cortex-linux.gpgAlso applies to: 55-61
🤖 Prompt for AI Agents
In @iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot around lines
9 - 12, The build hook currently uses non-deterministic network fetches and only
sets "set -e"; change the script to be failure-safe and deterministic by adding
"set -euo pipefail" at the top and replacing the remote key download with a
local, versioned key file shipped in the tree (e.g., use a bundled file like
cortex-apt-key.gpg and import it via apt-key add or gpg --dearmor into
/usr/share/keyrings), update the repository entry to reference the local
keyring, and ensure all commands that read from pipes will fail on errors so
failures are clear and reproducible.
| # Generate Packages file | ||
| log "Generating Packages for ${suite}/${arch}..." | ||
| (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages" 2>/dev/null || true) | ||
| if [ -d "${REPO_DIR}/pool/main" ]; then | ||
| (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages") | ||
| else | ||
| touch "${arch_dir}/Packages" | ||
| fi | ||
| gzip -9 -k -f "${arch_dir}/Packages" |
There was a problem hiding this comment.
touch Packages can leave stale repository metadata; truncate instead.
Proposed fix
- if [ -d "${REPO_DIR}/pool/main" ]; then
- (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages")
- else
- touch "${arch_dir}/Packages"
- fi
+ if [ -d "${REPO_DIR}/pool/main" ]; then
+ (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages")
+ else
+ : > "${arch_dir}/Packages"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Generate Packages file | |
| log "Generating Packages for ${suite}/${arch}..." | |
| (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages" 2>/dev/null || true) | |
| if [ -d "${REPO_DIR}/pool/main" ]; then | |
| (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages") | |
| else | |
| touch "${arch_dir}/Packages" | |
| fi | |
| gzip -9 -k -f "${arch_dir}/Packages" | |
| # Generate Packages file | |
| log "Generating Packages for ${suite}/${arch}..." | |
| if [ -d "${REPO_DIR}/pool/main" ]; then | |
| (cd "${REPO_DIR}" && dpkg-scanpackages --arch "$arch" pool/main > "${arch_dir}/Packages") | |
| else | |
| : > "${arch_dir}/Packages" | |
| fi | |
| gzip -9 -k -f "${arch_dir}/Packages" |
🤖 Prompt for AI Agents
In @repository/scripts/repo-manage.sh around lines 136 - 143, The script
currently uses touch "${arch_dir}/Packages" which can leave stale content if the
file already exists; change it to truncate the file instead (overwrite with an
empty file) when dpkg-scanpackages is not run so Packages is emptied rather than
preserved; update the branch in repo-manage.sh that handles the else path after
dpkg-scanpackages (the block that currently calls touch "${arch_dir}/Packages")
to create an empty/truncated "${arch_dir}/Packages" before gzip -9 -k -f runs.
| copy_glob_if_exists() { | ||
| local pattern="$1" | ||
| local dest="$2" | ||
| # shellcheck disable=SC2086 | ||
| if ls $pattern 1>/dev/null 2>&1; then | ||
| cp $pattern "$dest" | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
copy_glob_if_exists: unquoted glob + cp without -r will break on directories/spaces and can create flaky builds.
Proposed fix (safe globbing + recursive copy)
copy_glob_if_exists() {
local pattern="$1"
local dest="$2"
- # shellcheck disable=SC2086
- if ls $pattern 1>/dev/null 2>&1; then
- cp $pattern "$dest"
- return 0
- fi
- return 1
+ shopt -s nullglob
+ local matches=($pattern)
+ shopt -u nullglob
+ if ((${#matches[@]} > 0)); then
+ cp -r -- "${matches[@]}" "$dest"
+ return 0
+ fi
+ return 1
}📝 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.
| copy_glob_if_exists() { | |
| local pattern="$1" | |
| local dest="$2" | |
| # shellcheck disable=SC2086 | |
| if ls $pattern 1>/dev/null 2>&1; then | |
| cp $pattern "$dest" | |
| return 0 | |
| fi | |
| return 1 | |
| } | |
| copy_glob_if_exists() { | |
| local pattern="$1" | |
| local dest="$2" | |
| shopt -s nullglob | |
| local matches=($pattern) | |
| shopt -u nullglob | |
| if ((${#matches[@]} > 0)); then | |
| cp -r -- "${matches[@]}" "$dest" | |
| return 0 | |
| fi | |
| return 1 | |
| } |
🤖 Prompt for AI Agents
In @scripts/build.sh around lines 67 - 76, The function copy_glob_if_exists has
an unquoted glob and uses cp without -r which breaks on filenames with spaces,
directories, and creates flaky builds; update it to safely expand globs into an
array (or enable nullglob), check if the array is non-empty, and iterate over
matches quoting each item when copying, and use cp -a (or cp -r --) to preserve
directories and handle recursive copies; keep references to the function name
copy_glob_if_exists and the variables pattern and dest when making these
changes.
| cmd_check_deps() { | ||
| local failed=0 | ||
|
|
||
| header "Checking build dependencies" | ||
|
|
||
| # Required dependencies | ||
| check_command lb live-build || failed=1 | ||
| check_command gpg gnupg || failed=1 | ||
| check_command python3 python3 || failed=1 | ||
|
|
||
| # Check live-build version | ||
| if command -v lb &>/dev/null; then | ||
| local lb_version | ||
| lb_version=$(dpkg-query -W -f='${Version}' live-build 2>/dev/null || echo "0") | ||
| if dpkg --compare-versions "$lb_version" lt "1:20210814"; then | ||
| warn "live-build version $lb_version may be too old. Recommended: >= 1:20210814" | ||
| else | ||
| log "live-build version: $lb_version" | ||
| fi | ||
| fi | ||
|
|
||
| # Check Python version | ||
| if command -v python3 &>/dev/null; then | ||
| if python3 -c "import sys; sys.exit(0 if sys.version_info >= (3, 11) else 1)" 2>/dev/null; then | ||
| log "Python version: $(python3 --version 2>&1 | cut -d' ' -f2)" | ||
| else | ||
| warn "Python 3.11+ recommended (found: $(python3 --version 2>&1 | cut -d' ' -f2))" | ||
| fi | ||
| fi | ||
|
|
||
| # Optional dependencies | ||
| echo "" | ||
| log "Checking optional dependencies..." | ||
|
|
||
| if command -v shellcheck &>/dev/null; then | ||
| log "shellcheck: OK" | ||
| else | ||
| warn "Non-Debian OS - build may not work correctly" | ||
| fi | ||
|
|
||
| # Check for required commands | ||
| local required_cmds=( | ||
| "dpkg-buildpackage" | ||
| "lb" | ||
| "debootstrap" | ||
| "xorriso" | ||
| "mksquashfs" | ||
| ) | ||
|
|
||
| for cmd in "${required_cmds[@]}"; do | ||
| if command -v "$cmd" &>/dev/null; then | ||
| log "Found: $cmd" | ||
| warn "shellcheck not installed (optional, for linting)" | ||
| fi | ||
|
|
||
| if command -v convert &>/dev/null; then | ||
| log "ImageMagick: OK" | ||
| else | ||
| warn "ImageMagick not installed (optional, for GRUB theme images)" | ||
| fi | ||
|
|
||
| if command -v dpkg-deb &>/dev/null; then | ||
| log "dpkg-deb: OK" | ||
| else | ||
| warn "dpkg-deb not installed (needed for branding-package)" | ||
| fi | ||
|
|
||
| echo "" | ||
| if [ $failed -eq 0 ]; then | ||
| log "All required dependencies found." | ||
| return 0 | ||
| else | ||
| error "Missing required dependencies." | ||
| return 1 | ||
| fi | ||
| } |
There was a problem hiding this comment.
branding-package needs dpkg-deb but dependency check treats it as optional.
Either make dpkg-deb required in cmd_check_deps (or check it inside cmd_branding_package and fail with a clear message).
Also applies to: 730-818
🤖 Prompt for AI Agents
In @scripts/build.sh around lines 94 - 154, The script currently treats dpkg-deb
as optional in cmd_check_deps but branding-package needs it; either mark
dpkg-deb as required by changing its check in cmd_check_deps to mirror other
required checks (use check_command dpkg-deb || failed=1 and ensure the final
failed logic returns non-zero), or add a dedicated runtime check inside
cmd_branding_package that tests command -v dpkg-deb and exits/fails with a clear
error message if missing; update references to the failed variable and
error/warn calls so the failure path is deterministic and the message explains
that branding-package requires dpkg-deb.
| # Copy provisioning files | ||
| if [ -d "$PROVISION_DIR" ]; then | ||
| if copy_glob_if_exists "${PROVISION_DIR}/*" "${build_path}/config/includes.binary/provisioning/"; then | ||
| log "Copied provisioning files" | ||
| fi | ||
| fi | ||
| } |
There was a problem hiding this comment.
Provisioning copy will inherit the copy_glob_if_exists problems (dirs inside provisioning will fail).
Fixing copy_glob_if_exists as above should address this too.
| log "Starting live-build (this may take a while)..." | ||
| sudo lb build | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the build.sh file
fd -t f "build.sh" scripts/Repository: cortexlinux/cortex-distro
Length of output: 85
🏁 Script executed:
# Check if the file exists and get line count
wc -l scripts/build.sh 2>/dev/null || echo "File not found"Repository: cortexlinux/cortex-distro
Length of output: 89
🏁 Script executed:
# Read the context around lines 570-572
if [ -f scripts/build.sh ]; then
sed -n '560,580p' scripts/build.sh
fiRepository: cortexlinux/cortex-distro
Length of output: 549
🏁 Script executed:
# Search for all instances of 'lb build' and 'lb clean' in the script
if [ -f scripts/build.sh ]; then
echo "=== Instances of 'lb build' ===" && grep -n "lb build" scripts/build.sh
echo "=== Instances of 'lb clean' ===" && grep -n "lb clean" scripts/build.sh
fiRepository: cortexlinux/cortex-distro
Length of output: 180
🏁 Script executed:
# Read the beginning of the file to see shebang and initial documentation
head -50 scripts/build.shRepository: cortexlinux/cortex-distro
Length of output: 1529
🏁 Script executed:
# Check for existing EUID checks or sudo patterns in the file
grep -n "EUID\|getuid\|whoami\|id -u" scripts/build.sh || echo "No explicit user checks found"Repository: cortexlinux/cortex-distro
Length of output: 98
🏁 Script executed:
# Look for other sudo commands to understand the pattern
grep -n "^[[:space:]]*sudo " scripts/build.sh | head -20Repository: cortexlinux/cortex-distro
Length of output: 116
🏁 Script executed:
# Check surrounding context of the build_iso function
sed -n '550,600p' scripts/build.shRepository: cortexlinux/cortex-distro
Length of output: 1419
Unconditional sudo will fail when running as root (common in CI/CD environments); use conditional sudo for both lb build and lb clean.
Apply the EUID check to both occurrences:
- Line 571:
sudo lb build - Line 643:
sudo lb clean
Use: if [[ $EUID -eq 0 ]]; then lb build; else sudo lb build; fi (and same pattern for lb clean).
🤖 Prompt for AI Agents
In @scripts/build.sh around lines 570 - 572, The live-build commands use
unconditional sudo which fails when running as root; update both invocations of
"lb build" and "lb clean" to run conditionally based on EUID: replace each `sudo
lb build` and `sudo lb clean` with an EUID check that runs `lb build`/`lb clean`
directly if $EUID is 0, otherwise prefixes with sudo (e.g., if [[ $EUID -eq 0
]]; then lb build; else sudo lb build; fi and same pattern for lb clean).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Line 236: The workflow currently pins the GitHub Action as
"softprops/action-gh-release@v1" which is outdated; update the uses entry for
softprops/action-gh-release to a newer release (e.g., "@v2" or a specific commit
SHA) to ensure compatibility with current runners and satisfy actionlint; locate
the "uses: softprops/action-gh-release@v1" line in the workflow and replace the
version tag with the chosen newer tag or SHA, then run the workflow lint/CI to
verify the change.
- Around line 28-38: The workflow defines an unused inputs.profile with options
[core, full, secops, all] while the build-iso and test-iso strategy matrices
(matrix in the build-iso job and matrix in the test-iso job) are hardcoded to
[core, full]; either remove the extra options from inputs.profile or make the
matrices respect the input by replacing their static arrays with the dynamic
value (use the inputs.profile value in the matrix expressions for both the
build-iso and test-iso jobs) so selecting secops or all actually affects which
profiles are built and tested.
🧹 Nitpick comments (3)
.github/workflows/build-iso.yml (3)
152-158: Re-evaluate continue-on-error for SBOM uploads.Setting
continue-on-error: truefor SBOM uploads (software bill of materials) may hide issues with SBOM generation. While SBOMs might be optional for the build to succeed, their absence could have compliance or security implications.Consider one of the following approaches:
- Remove
continue-on-errorif SBOMs are required for releases- Add conditional logic to fail on tags but allow errors on PR builds
- Add explicit logging when SBOM generation fails
Example for conditional strictness:
- name: Upload SBOM uses: actions/upload-artifact@v4 with: name: sbom-${{ matrix.profile }}-${{ matrix.arch }} path: output/sbom/ retention-days: 14 continue-on-error: ${{ github.ref_type != 'tag' }}
182-198: Add explicit verification that ISO files exist.The shell loops using
*.isowill fail gracefully if no ISOs exist (thanks to the conditional check on line 186), but could be more explicit about the failure case.Consider adding an explicit check:
- name: Verify checksums run: | shopt -s nullglob isos=(*.iso) if [ ${#isos[@]} -eq 0 ]; then echo "Error: No ISO files found" exit 1 fi for iso in "${isos[@]}"; do if [ -f "${iso}.sha256" ]; then sha256sum -c "${iso}.sha256" fi doneThis makes the workflow fail more explicitly if artifacts are missing.
229-233: Add file existence checks before generating checksums.The checksum generation assumes
*.isoand*.debfiles exist. If artifact downloads fail or produce no files, thesha256sumcommand will fail with a cryptic error.Add verification before checksum generation:
- name: Generate combined checksums run: | cd release # Verify files exist if ! ls *.iso >/dev/null 2>&1; then echo "Error: No ISO files found for release" exit 1 fi if ! ls *.deb >/dev/null 2>&1; then echo "Error: No DEB packages found for release" exit 1 fi sha256sum *.iso *.deb > SHA256SUMS sha512sum *.iso *.deb > SHA512SUMS
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-iso.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
236-236: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (3)
- GitHub Check: Build ISO (core)
- GitHub Check: Build ISO (full)
- GitHub Check: Build (First Pass)
🔇 Additional comments (3)
.github/workflows/build-iso.yml (3)
44-59: LGTM!The validation job correctly sets up dependencies and runs validation checks in a containerized environment.
76-84: Verify artifact path pattern matches expected output.The wildcard
output/*.debassumes the branding package build produces at least one.debfile. If the build succeeds but no package is generated, the upload step will fail silently.Consider adding a verification step before upload:
- name: Verify package output run: | if [ ! -f output/*.deb ]; then echo "Error: No branding package found in output/" exit 1 fi ls -lh output/*.deb
89-92: Document the security implications of --privileged.The
--privilegedflag grants extended privileges to the container, which is necessary forlive-buildto access loop devices and create bootable ISOs. While this is acceptable in a controlled CI environment, it does expand the attack surface.Ensure that:
- The repository has appropriate access controls
- Workflow permissions follow the principle of least privilege
- This requirement is documented in the build documentation
Consider adding a comment explaining why --privileged is necessary:
container: image: debian:bookworm # Privileged mode required for live-build to access loop devices options: --privileged
| sha512sum *.iso *.deb > SHA512SUMS | ||
| - name: Create Release | ||
| uses: softprops/action-gh-release@v1 |
There was a problem hiding this comment.
Update to the latest version of softprops/action-gh-release.
The v1 tag uses an outdated runner version that may not work on GitHub Actions. Update to a more recent version (e.g., v2 or a specific commit SHA).
Based on the static analysis hint from actionlint.
📦 Proposed fix
- name: Create Release
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2
with:📝 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.
| uses: softprops/action-gh-release@v1 | |
| uses: softprops/action-gh-release@v2 |
🧰 Tools
🪛 actionlint (1.7.10)
236-236: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml at line 236, The workflow currently pins the
GitHub Action as "softprops/action-gh-release@v1" which is outdated; update the
uses entry for softprops/action-gh-release to a newer release (e.g., "@v2" or a
specific commit SHA) to ensure compatibility with current runners and satisfy
actionlint; locate the "uses: softprops/action-gh-release@v1" line in the
workflow and replace the version tag with the chosen newer tag or SHA, then run
the workflow lint/CI to verify the change.
…availability. Added a message to indicate the repository is currently skipped.
- Adjusted boot menu dimensions and item font sizes for better readability. - Enhanced title and subtitle labels with updated text and styling. - Refined help text positioning and font size for clarity. - Added new message and background colors for improved visual appeal.
- Removed the container specification for the build job, simplifying the workflow. - Added a disk cleanup step to free up space before the build process. - Consolidated dependency installation into a Docker run command for better isolation and management. - Ensured that the build process includes dependency checks and ISO creation in a single step.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Around line 27-38: The workflow_dispatch input "profile" is misleading because
the build matrix doesn't use it; update the workflow to either (A) drive the
build matrix from the "profile" input by mapping the input value to the
matrix.include or matrix.strategy (e.g., read github.event.inputs.profile in the
job and set matrix values to run core/full/secops/all accordingly), or (B)
restrict the workflow_dispatch options to only "core" and "full" to match the
current matrix; locate the workflow_dispatch block and the job that defines the
build matrix and change the matrix definition to conditionally use
github.event.inputs.profile (or add a mapping step to populate matrix.include)
or prune the input options to match the existing matrix.
- Around line 245-250: The "Generate combined checksums" step can fail when glob
patterns match nothing; change the run block to avoid bare globs by enabling
nullglob (shopt -s nullglob) or by building an explicit file list (e.g.,
files=(release/*.{iso,deb}) or use find) and only run sha256sum/sha512sum when
the list is non-empty; update the commands that call sha256sum and sha512sum to
use the guarded file array/variable so the step exits successfully when there
are no matches.
- Around line 193-215: The workflow's "Download ISO", "Verify checksums", and
"Check ISO structure" steps assume .iso files in the runner root but
actions/download-artifact extracts into a subfolder, causing the for-loops to
see a literal "*.iso" and skip checks; change the download step to save
artifacts into a known directory (e.g., an explicit target dir name) and then in
the "Verify checksums" and "Check ISO structure" steps cd into that directory
(or reference that directory in the loops) and enable safe globbing (e.g.,
nullglob) or use a secure find pattern so the verification and xorriso checks
actually iterate real .iso files.
- Around line 251-252: Update the GitHub Actions step that currently uses
softprops/action-gh-release@v1 to the maintained v2 release; locate the workflow
step with the uses: softprops/action-gh-release@v1 entry in the Create Release
job and change the reference to softprops/action-gh-release@v2 so the action
runs on Node 20-compatible version.
🧹 Nitpick comments (3)
.github/workflows/build-iso.yml (3)
44-60: Containerizedvalidatejob is good; dropsudoor set strict shell flags (Line 44–60).
In a Debian container you’re typically root already;sudois extra surface area. Considerset -euo pipefailin the install step for cleaner failures.
96-151: Re-check need fordocker run --privileged+ add timeouts for apt (Line 96–151).
--privilegedis a big hammer; if live-build only needs loop devices/mounts, consider narrower caps/devices. Also,apt-getwith no retries/timeouts can hang a runner.
168-175: SBOM upload: considerif-no-files-found: ignoreinstead ofcontinue-on-error(Line 168–175).
This avoids masking other upload errors while still allowing “no SBOM produced” to be non-fatal.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-iso.ymlbranding/grub/cortex/theme.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- branding/grub/cortex/theme.txt
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
252-252: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (2)
- GitHub Check: Build Branding Package
- GitHub Check: Build (First Pass)
🔇 Additional comments (2)
.github/workflows/build-iso.yml (2)
12-26: Workflow triggers look fine, but ensure new paths are intentional (Line 12–26).
Addingbranding/**andscripts/**will increase CI frequency; if that’s expected, LGTM.
61-85: Branding package build/upload looks consistent (Line 61–85).
Artifact name +output/*.debconvention is clear.
| - name: Download ISO | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: cortex-linux-${{ matrix.profile }}-${{ matrix.arch }} | ||
|
|
||
| - name: Verify checksums | ||
| run: | | ||
| ls -la | ||
| for iso in *.iso; do | ||
| if [ -f "${iso}.sha256" ]; then | ||
| sha256sum -c "${iso}.sha256" | ||
| fi | ||
| done | ||
| - name: Check ISO structure | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y xorriso | ||
| for iso in *.iso; do | ||
| echo "=== Checking $iso ===" | ||
| xorriso -indev "$iso" -find / -maxdepth 1 2>/dev/null | head -20 | ||
| done | ||
There was a problem hiding this comment.
test-iso likely doesn’t test anything due to artifact extraction layout (Line 193–215).
actions/download-artifact will usually extract into a subfolder (artifact name) unless you flatten; your for iso in *.iso loops can end up iterating the literal *.iso and silently skipping verification.
Proposed fix: download into a known dir and `cd` there (also make globbing safe)
test-iso:
@@
steps:
- name: Download ISO
uses: actions/download-artifact@v4
with:
name: cortex-linux-${{ matrix.profile }}-${{ matrix.arch }}
+ path: artifacts
- name: Verify checksums
run: |
- ls -la
- for iso in *.iso; do
+ set -euo pipefail
+ shopt -s nullglob
+ cd artifacts
+ # If download-artifact created a subdir, enter it
+ d="cortex-linux-${{ matrix.profile }}-${{ matrix.arch }}"
+ [ -d "$d" ] && cd "$d"
+ ls -la
+ for iso in *.iso; do
if [ -f "${iso}.sha256" ]; then
sha256sum -c "${iso}.sha256"
fi
done
- name: Check ISO structure
run: |
+ set -euo pipefail
+ shopt -s nullglob
sudo apt-get update
sudo apt-get install -y xorriso
- for iso in *.iso; do
+ cd artifacts
+ d="cortex-linux-${{ matrix.profile }}-${{ matrix.arch }}"
+ [ -d "$d" ] && cd "$d"
+ for iso in *.iso; do
echo "=== Checking $iso ==="
xorriso -indev "$iso" -find / -maxdepth 1 2>/dev/null | head -20
done🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 193 - 215, The workflow's
"Download ISO", "Verify checksums", and "Check ISO structure" steps assume .iso
files in the runner root but actions/download-artifact extracts into a
subfolder, causing the for-loops to see a literal "*.iso" and skip checks;
change the download step to save artifacts into a known directory (e.g., an
explicit target dir name) and then in the "Verify checksums" and "Check ISO
structure" steps cd into that directory (or reference that directory in the
loops) and enable safe globbing (e.g., nullglob) or use a secure find pattern so
the verification and xorriso checks actually iterate real .iso files.
| - name: Generate combined checksums | ||
| run: | | ||
| cd release | ||
| sha256sum *.iso *.deb > SHA256SUMS | ||
| sha512sum *.iso *.deb > SHA512SUMS | ||
There was a problem hiding this comment.
Release checksum generation can fail on empty globs (Line 245–250).
If anything changes in naming/paths, sha256sum *.iso *.deb will error. Consider nullglob + explicit checks (or checksum everything under release/).
🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 245 - 250, The "Generate
combined checksums" step can fail when glob patterns match nothing; change the
run block to avoid bare globs by enabling nullglob (shopt -s nullglob) or by
building an explicit file list (e.g., files=(release/*.{iso,deb}) or use find)
and only run sha256sum/sha512sum when the list is non-empty; update the commands
that call sha256sum and sha512sum to use the guarded file array/variable so the
step exits successfully when there are no matches.
- Introduced a new action to free up disk space before the build process. - Added additional cleanup commands to remove various unnecessary files and directories. - Improved overall efficiency of the workflow by ensuring more space is available for builds.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Around line 29-38: The workflow_dispatch input declares profile options
'secops' and 'all' that the build matrix doesn't support; either remove those
unsupported options from the profile input (leave options as 'core' and 'full'
only) or modify the strategy.matrix.profile logic to evaluate github.event_name
and github.event.inputs.profile so a manual dispatch uses the input (supporting
'all' to expand to multiple profiles or a single-profile array) as shown in the
suggested conditional matrix expression; update both the input block named
profile and the strategy.matrix.profile handling so they stay consistent.
- Around line 259-260: The workflow's "Create Release" step uses the outdated
action reference softprops/action-gh-release@v1 which may fail on modern
runners; update the uses entry to the newer major version (e.g.,
softprops/action-gh-release@v2) in the Create Release job so the workflow pulls
the updated action release instead of @v1.
- Around line 96-105: The workflow step named "Free disk space" currently
references the external action jlumbroso/free-disk-space@main; change that
reference to a fixed, published version (e.g., jlumbroso/free-disk-space@v1.3.1)
to avoid supply-chain risks, updating the uses field in that step while leaving
the existing with: inputs unchanged.
🧹 Nitpick comments (1)
.github/workflows/build-iso.yml (1)
206-222: Consider more thorough ISO structure validation.The current test only lists top-level contents. Consider verifying essential files exist (e.g., boot loader, squashfs, etc.) to catch structural issues:
💡 Enhanced structure verification
- name: Check ISO structure run: | sudo apt-get update sudo apt-get install -y xorriso for iso in *.iso; do echo "=== Checking $iso ===" xorriso -indev "$iso" -find / -maxdepth 1 2>/dev/null | head -20 + # Verify essential boot components exist + xorriso -indev "$iso" -find / -name "*.squashfs" 2>/dev/null | grep -q squashfs || echo "WARNING: No squashfs found" + xorriso -indev "$iso" -find /boot -maxdepth 2 2>/dev/null | head -10 done
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-iso.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
260-260: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (3)
- GitHub Check: Build ISO (core)
- GitHub Check: Build ISO (full)
- GitHub Check: Build (First Pass)
🔇 Additional comments (4)
.github/workflows/build-iso.yml (4)
44-59: LGTM!The validate job is well-structured with appropriate dependencies installed in a consistent Debian container environment.
61-84: LGTM!The branding package build job properly chains after validation and uploads the artifact for downstream jobs.
130-159: LGTM!The containerized build approach with
--privilegedis appropriate for live-build operations that require loop device access and chroot capabilities. Theset -eensures the build fails fast on errors.
253-267: LGTM with a defensive improvement suggestion.The checksum generation and release assets are well-organized. The branding package should always be present from the earlier job, so the glob pattern should match.
For robustness, you could add
shopt -s nullglobto avoid errors if a glob pattern unexpectedly matches nothing:💡 Optional: Defensive glob handling
- name: Generate combined checksums run: | cd release + shopt -s nullglob + files=(*.iso *.deb) + if [ ${#files[@]} -eq 0 ]; then + echo "ERROR: No ISO or deb files found" + exit 1 + fi sha256sum *.iso *.deb > SHA256SUMS sha512sum *.iso *.deb > SHA512SUMS
| - name: Free disk space | ||
| uses: jlumbroso/free-disk-space@main | ||
| with: | ||
| tool-cache: true | ||
| android: true | ||
| dotnet: true | ||
| haskell: true | ||
| large-packages: true | ||
| docker-images: true | ||
| swap-storage: true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
jlumbroso/free-disk-space GitHub Action latest release version
💡 Result:
Latest release: v1.3.1 — released Oct 18, 2023. [1]
Sources
[1] jlumbroso/free-disk-space tags (v1.3.1).
Pin jlumbroso/free-disk-space to a stable version instead of @main.
Using @main exposes the build to supply chain risks—breaking changes or malicious commits could affect builds without notice. Pin to a stable release like v1.3.1:
Recommended change
- name: Free disk space
- uses: jlumbroso/free-disk-space@main
+ uses: jlumbroso/free-disk-space@v1.3.1📝 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.
| - name: Free disk space | |
| uses: jlumbroso/free-disk-space@main | |
| with: | |
| tool-cache: true | |
| android: true | |
| dotnet: true | |
| haskell: true | |
| large-packages: true | |
| docker-images: true | |
| swap-storage: true | |
| - name: Free disk space | |
| uses: jlumbroso/free-disk-space@v1.3.1 | |
| with: | |
| tool-cache: true | |
| android: true | |
| dotnet: true | |
| haskell: true | |
| large-packages: true | |
| docker-images: true | |
| swap-storage: true |
🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 96 - 105, The workflow step
named "Free disk space" currently references the external action
jlumbroso/free-disk-space@main; change that reference to a fixed, published
version (e.g., jlumbroso/free-disk-space@v1.3.1) to avoid supply-chain risks,
updating the uses field in that step while leaving the existing with: inputs
unchanged.
| - name: Create Release | ||
| uses: softprops/action-gh-release@v1 |
There was a problem hiding this comment.
Update softprops/action-gh-release to a newer version.
The static analysis tool indicates that @v1 is too old to run on current GitHub Actions runners. This could cause failures or deprecated behavior warnings.
🔧 Update to v2
- name: Create Release
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2
with:
files: |📝 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.
| - name: Create Release | |
| uses: softprops/action-gh-release@v1 | |
| - name: Create Release | |
| uses: softprops/action-gh-release@v2 |
🧰 Tools
🪛 actionlint (1.7.10)
260-260: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 259 - 260, The workflow's
"Create Release" step uses the outdated action reference
softprops/action-gh-release@v1 which may fail on modern runners; update the uses
entry to the newer major version (e.g., softprops/action-gh-release@v2) in the
Create Release job so the workflow pulls the updated action release instead of
@v1.
…ld process - Simplified ISO build targets in the Makefile, consolidating profiles into a single command. - Updated help text to reflect changes in ISO build commands and removed obsolete profiles. - Enhanced GitHub Actions workflow to support both amd64 and arm64 architectures with improved artifact handling. - Removed profile-specific inputs from the workflow, focusing on a unified build approach. - Improved preseed file management and validation checks in the build script for better reliability.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Around line 238-246: The GitHub Actions step named "Create Release" is using
the outdated action reference softprops/action-gh-release@v1; update the uses
value to softprops/action-gh-release@v2, then verify the step's inputs (the
files multiline input) remain compatible with v2 and run the workflow to confirm
the release artifacts (release/*.iso, release/*.deb, release/SHA256SUMS,
release/SHA512SUMS, release/sbom/*) are still uploaded correctly.
🧹 Nitpick comments (11)
scripts/install-deps.sh (3)
14-16: Minor inconsistency with logging format.The logging functions here use
[+]and[!]prefixes, whilescripts/build.shuses[BUILD]and[WARN]prefixes. Consider aligning these for consistency across the build tooling.Based on the relevant code snippets from
scripts/build.sh:log() { echo -e "${GREEN}[BUILD]${NC} $*"; } warn() { echo -e "${YELLOW}[WARN]${NC} $*"; }♻️ Optional: Align with build.sh format
-log() { echo -e "${GREEN}[+]${NC} $1"; } -warn() { echo -e "${YELLOW}[!]${NC} $1"; } -error() { echo -e "${RED}[ERROR]${NC} $1" >&2; } +log() { echo -e "${GREEN}[DEPS]${NC} $1"; } +warn() { echo -e "${YELLOW}[WARN]${NC} $1"; } +error() { echo -e "${RED}[ERROR]${NC} $1" >&2; }
28-29: Consider quieting apt-get update output.The
apt-get updateoutput can be verbose. Consider adding♻️ Suggested improvement
# Update package lists -apt-get update +apt-get update -qq
51-52: Add--no-install-recommendsto reduce image size.For build tooling, installing only the required packages without recommends can reduce disk usage and build time.
♻️ Suggested improvement
log "Installing common packages..." -apt-get install -y "${COMMON_PACKAGES[@]}" +apt-get install -y --no-install-recommends "${COMMON_PACKAGES[@]}".github/workflows/build-iso.yml (2)
186-192: Checksum verification may silently skip ISOs without checksums.The loop continues silently if a
.sha256file doesn't exist. Consider logging or failing explicitly if expected checksums are missing.♻️ Suggested improvement
- name: Verify checksums run: | ls -la for iso in *.iso; do if [ -f "${iso}.sha256" ]; then sha256sum -c "${iso}.sha256" + else + echo "Warning: No checksum file found for ${iso}" fi done
232-236: Checksum generation may fail if no.debfiles exist.If the branding package download fails (despite
continue-on-error: trueon SBOM), thesha256sum *.debglob may fail. Consider adding a fallback.♻️ Suggested improvement
- name: Generate combined checksums run: | cd release - sha256sum *.iso *.deb > SHA256SUMS - sha512sum *.iso *.deb > SHA512SUMS + sha256sum *.iso > SHA256SUMS + sha256sum *.deb >> SHA256SUMS 2>/dev/null || true + sha512sum *.iso > SHA512SUMS + sha512sum *.deb >> SHA512SUMS 2>/dev/null || trueMakefile (2)
137-142: Root check implementation is correct but could be extracted.The inline root check works, but duplicating this logic (also in
install-deps.sh) could be consolidated into a shared helper if more targets require root.
152-160: Add security note forchroot-shelltarget.Entering a chroot with
sudo chrootis powerful. Consider adding a comment or warning that this is for development only and the chroot environment should not be trusted.♻️ Optional: Add development-only warning
# Enter interactive shell inside the chroot filesystem +# WARNING: Development use only - chroot environment is not sanitized chroot-shell: @if [ -d "$(BUILD_DIR)/chroot" ]; then \ + echo "WARNING: Entering unsanitized chroot environment"; \ echo "Entering chroot..."; \scripts/build.sh (4)
100-109: Live-build version comparison may fail if dpkg-query returns non-version string.If
dpkg-queryfails or returns an unexpected format, the comparison could behave unexpectedly. The fallback to "0" is good, but consider also handling the case wheredpkg --compare-versionsitself fails.♻️ Optional: Add defensive error handling
if command -v lb &>/dev/null; then local lb_version lb_version=$(dpkg-query -W -f='${Version}' live-build 2>/dev/null || echo "0") - if dpkg --compare-versions "$lb_version" lt "1:20210814"; then + if dpkg --compare-versions "$lb_version" lt "1:20210814" 2>/dev/null; then warn "live-build version $lb_version may be too old. Recommended: >= 1:20210814" else log "live-build version: $lb_version"
659-662: Hardcoded package version should be parameterized.The
pkg_version="1.0.0"is hardcoded. Consider reading this from the DEBIAN/control file or making it configurable via environment variable for release versioning.♻️ Suggested improvement
cmd_branding_package() { local pkg_name="cortex-branding" - local pkg_version="1.0.0" + local pkg_version="${BRANDING_VERSION:-1.0.0}" local pkg_dir="${BUILD_DIR}/${pkg_name}"Alternatively, extract version from the control file:
local pkg_version pkg_version=$(grep '^Version:' "${PACKAGES_DIR}/${pkg_name}/DEBIAN/control" | cut -d' ' -f2)
602-612:cmd_cleanuses sudo but doesn't verify build directory safety.Before running
sudo lb cleanin an arbitrary directory, consider validating thatBUILD_DIRis within the expected project structure to prevent accidental cleanup of unintended paths.♻️ Add path validation
cmd_clean() { header "Cleaning build" if [ -d "$BUILD_DIR" ]; then + # Ensure BUILD_DIR is within PROJECT_ROOT + case "$BUILD_DIR" in + "${PROJECT_ROOT}/"*) ;; + *) error "BUILD_DIR is outside project root"; return 1 ;; + esac cd "$BUILD_DIR" sudo lb clean log "Clean complete"
536-545: Consider addingsudo -nflag to fail fast if authentication is required.While the build runs in a
--privilegedDocker container (which typically allows sudo without TTY), usingsudo -nis defensive and will cause the build to fail immediately if sudo authentication is required, rather than hanging.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build-iso.ymlMakefilescripts/build.shscripts/install-deps.sh
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/install-deps.sh (1)
scripts/build.sh (3)
error(44-44)log(42-42)warn(43-43)
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
239-239: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 checkmake (0.2.2)
Makefile
[warning] 43-43: Target body for "help" exceeds allowed length of 5 (32).
(maxbodylength)
[warning] 166-166: Target body for "config" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 14-14: Missing required phony target "clean"
(minphony)
[warning] 14-14: Missing required phony target "test"
(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). (2)
- GitHub Check: Build ISO (amd64)
- GitHub Check: Build (First Pass)
🔇 Additional comments (9)
scripts/install-deps.sh (1)
55-64: LGTM on architecture handling.Good conditional handling for amd64 and arm64 architectures with a sensible fallback warning for unknown architectures.
.github/workflows/build-iso.yml (1)
75-86: LGTM on matrix strategy.Good use of
fail-fast: falseto allow both architectures to complete independently, and appropriate runner selection for arm64.Makefile (3)
14-17: LGTM on .PHONY declaration.The static analysis warning about missing
cleanandtestphony targets appears to be a false positive—both are listed on line 15.
20-21: Good shell strictness settings.Using
-eu -o pipefailensures errors are caught and pipelines fail properly. This is a best practice for robust Makefiles.
103-108: Consider addingvalidatedependency toiso-arm64as well.The
isotarget hascheck-deps validateas dependencies, butiso-arm64only invokes$(MAKE) ARCH=arm64 iso. This is correct since it delegates toiso, but verify the recursive make call inherits the dependencies properly.scripts/build.sh (4)
7-7: LGTM on strict mode.Using
set -euo pipefailensures the script fails fast on errors, unset variables, and pipeline failures.
63-72: Glob expansion is intentionally unquoted.The shellcheck disable comment correctly documents that the unquoted glob is intentional for pattern matching. Good use of the disable directive.
798-839: LGTM on main entry point.Clean command dispatcher pattern with sensible defaults and help fallback for unknown commands.
286-295: Error accumulation pattern loses granularity.Using
|| errors=$((errors + $?))to capture exit codes is creative, but$?after||will always be 0 since the||clause succeeded. The validation functions return error counts directly, which works, but this pattern is fragile.🐛 Issue explanation and fix
When you write
validate_provision || errors=$((errors + $?)), the$?in the right-hand side reflects the exit status of the||expression itself (which is 0 if the right side executes). The actual error count fromvalidate_provisionis lost.Consider capturing the return value explicitly:
provision) - validate_provision || errors=$((errors + $?)) + validate_provision || { local rc=$?; errors=$((errors + rc)); } ;; hooks) - validate_hooks || errors=$((errors + $?)) + validate_hooks || { local rc=$?; errors=$((errors + rc)); } ;; lint) - run_shellcheck || errors=$((errors + $?)) + run_shellcheck || { local rc=$?; errors=$((errors + rc)); }Or restructure to use:
validate_provision; errors=$((errors + $?))Likely an incorrect or invalid review comment.
- Removed obsolete preseed profiles (core, full, secops) to streamline configuration management. - Updated the base preseed file to clarify its purpose and settings. - Simplified the first-boot provisioning script by removing profile-specific logic and enhancing core setup functions. - Improved logging and error handling in the provisioning script for better maintainability.
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In @iso/preseed/cortex-base.preseed:
- Around line 59-77: The preseed is missing the unattended-upgrades package so
first-boot.sh’s check for unattended-upgrades will always skip; add
"unattended-upgrades" to the d-i pkgsel/include string in the preseed (alongside
sudo, openssh-server, etc.) so the package is present on install, or
alternatively update the first-boot.sh patch that checks for
"unattended-upgrades" to attempt to install the package when online (i.e.,
detect missing package and run apt-get install unattended-upgrades before
configuring it).
- Around line 11-17: The preseed contains two hostname keys that conflict:
netcfg/get_hostname and netcfg/hostname; pick one canonical key and remove the
other to avoid ambiguity (e.g., keep netcfg/hostname string cortex and delete
the netcfg/get_hostname line, or vice versa), and ensure any downstream scripts
that read the hostname use the chosen key (update references if needed) so
preseeding yields a single authoritative hostname source.
- Around line 30-44: Replace the invalid placeholder in d-i
passwd/user-password-crypted with a real crypt(3) SHA-512 hash for a known
temporary password (so the installer accepts it), do not rely on
allow-password-weak as the only safeguard (keep or set d-i
user-setup/allow-password-weak appropriately), and ensure the account is forced
to rotate the password at first login by adding a late_command that runs chage
-d 0 for the cortex user (targeting the username "cortex") so the temporary
password cannot remain in use.
- Around line 96-103: The late_command uses semicolon-separated commands so
failures are ignored; replace the cp + in-target chmod sequence with atomic
installs using install -m to copy files with correct permissions (for example
install -m 0755 for first-boot.sh and appropriate mode for
cortex-first-boot.service), and chain commands with && (or explicit conditional
checks) so the chain aborts on error; ensure you still use in-target for
systemctl enable cortex-first-boot.service and make the overall
preseed/late_command fail-fast if any install or mkdir step fails so
provisioning errors are not silently ignored.
In @iso/provisioning/first-boot.sh:
- Around line 235-256: The setup_unattended_upgrades function is effectively a
no-op under the current preseed because unattended-upgrades isn't installed;
either ensure the package is installed before the check or remove the function
call. Modify setup_unattended_upgrades to invoke package installation (e.g.,
apt-get install -y unattended-upgrades) if command -v returns false, or delete
the setup_unattended_upgrades invocation and its helper if you intend not to
enable unattended upgrades by default; refer to the function name
setup_unattended_upgrades and the auto_upgrades variable to locate the relevant
code to change.
- Around line 107-114: The setup_password_change function currently only
enforces chage -d 0 when REQUIRE_PASSWORD_CHANGE is explicitly true, which can
leave installs with unchanged placeholder passwords; update
setup_password_change to default to forcing rotation (treat
REQUIRE_PASSWORD_CHANGE as true when unset) or additionally detect a known
temporary/placeholder hash for the cortex account and force chage -d 0 when that
hash is present; make changes to the function that reference
REQUIRE_PASSWORD_CHANGE, chage -d 0 and the cortex username, and ensure you log
the decision via log_info so the behavior is auditable.
- Around line 258-277: The setup_desktop function calls groups cortex and
usermod -aG docker cortex without checking the cortex user exists, which can
cause the script to fail under set -e; before invoking groups or usermod, guard
those calls with a check like getent passwd cortex (or id -u cortex) to ensure
the user exists, and only run groups and usermod when that check succeeds so
setup_desktop safely skips Docker group configuration if the cortex user is
missing.
- Around line 81-105: The sudo Defaults in setup_sudo are currently global;
modify the here-doc written to sudoers_file (variable sudoers_file and function
setup_sudo) so each Defaults line is scoped to the cortex user by prefixing with
"Defaults:cortex" (e.g., Defaults:cortex env_reset, Defaults:cortex
logfile=...), keep the cortex ALL=(ALL:ALL) ALL line as-is, then after writing
the file run a syntax check with visudo -cf "${sudoers_file}" before chmod 440
to ensure validity.
- Around line 58-67: The check_network() function currently invokes ping
directly which under set -e will cause the script to exit if ping is missing;
update check_network to first test for ping's existence (e.g., using command -v
ping or type ping) and if not found log_warn "Network: Offline (ping missing)"
and export NETWORK_AVAILABLE=false, otherwise run the existing ping check and
set NETWORK_AVAILABLE=true/false based on its result; ensure any invocation of
ping is guarded so a missing binary or non-zero exit does not trigger an
unexpected script termination.
- Around line 16-55: The script currently logs in check_already_complete and
mark_complete before ensuring LOG_DIR/LOG_FILE and PROVISION_DIR exist, causing
non-idempotent failures; modify startup to bootstrap directories and log file
first (mkdir -p "${LOG_DIR}" and "${PROVISION_DIR}", touch/chmod "${LOG_FILE}")
before any call to log or running check_already_complete, and update
mark_complete() to ensure PROVISION_DIR exists (mkdir -p "${PROVISION_DIR}")
before creating STATE_FILE so writing the state is safe and idempotent.
- Around line 318-336: In setup_ssh_keys(), the systemctl call uses the
Debian-incompatible unit name "sshd"; change the restart to use the canonical
"ssh" unit instead (i.e., replace the systemctl restart sshd || true call with
systemctl restart ssh || true) so the SSH service restart is reliable across
Debian/Ubuntu systems after generating host keys.
- Around line 116-168: The SSH hardening block in setup_ssh has a mismatched
comment for PasswordAuthentication, a non‑FIPS-first cipher order in Ciphers,
and uses the nonstandard service name sshd; update the comment near
PasswordAuthentication to reflect that it enables or disables password auth
globally (adjust text for PasswordAuthentication yes/no to match the actual
value of the setting), reorder the Ciphers line to list AES-GCM ciphers first
(e.g., aes256-gcm@openssh.com,aes128-gcm@openssh.com) or remove
chacha20-poly1305@openssh.com if FIPS compliance is required, and change the
systemctl checks/reload that reference sshd to use ssh (i.e., replace systemctl
is-active --quiet sshd and systemctl reload sshd with ssh) while keeping the
rest of setup_ssh and the sshd_config filename unchanged.
🧹 Nitpick comments (2)
iso/provisioning/first-boot.sh (2)
170-193: Useufw --force enableinstead of pipingecho "y".Less brittle under
pipefail, and avoids interactive behavior changes across UFW versions.Proposed change
- echo "y" | ufw enable + ufw --force enable
370-403: Add an explicit root check early.Right now, a non-root invocation will fail mid-script in less obvious places; fail fast with a clear log.
Proposed addition
main() { + if [[ "${EUID}" -ne 0 ]]; then + echo "first-boot.sh must be run as root" >&2 + exit 1 + fi
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
iso/preseed/cortex-base.preseediso/provisioning/first-boot.sh
🧰 Additional context used
🧬 Code graph analysis (1)
iso/provisioning/first-boot.sh (3)
repository/scripts/repo-manage.sh (1)
log(31-31)scripts/build.sh (2)
log(42-42)main(798-839)branding/install-branding.sh (4)
log_info(22-22)log_warn(24-24)log_error(25-25)main(213-242)
⏰ 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). (2)
- GitHub Check: Build ISO (amd64)
- GitHub Check: Build (First Pass)
| set -euo pipefail | ||
|
|
||
| readonly CORTEX_DIR="/opt/cortex" | ||
| readonly PROVISION_DIR="${CORTEX_DIR}/provisioning" | ||
| readonly LOG_DIR="/var/log/cortex" | ||
| readonly LOG_FILE="${LOG_DIR}/first-boot.log" | ||
| readonly STATE_FILE="${PROVISION_DIR}/.first-boot-complete" | ||
|
|
||
| # Logging functions | ||
| log() { | ||
| local level="$1" | ||
| shift | ||
| local message="$*" | ||
| local timestamp | ||
| timestamp=$(date '+%Y-%m-%d %H:%M:%S') | ||
| echo "[${timestamp}] [${level}] ${message}" | tee -a "${LOG_FILE}" | ||
| } | ||
|
|
||
| log_info() { log "INFO" "$@"; } | ||
| log_warn() { log "WARN" "$@"; } | ||
| log_error() { log "ERROR" "$@"; } | ||
|
|
||
| # Check if already completed | ||
| check_already_complete() { | ||
| if [[ -f "${STATE_FILE}" ]]; then | ||
| log_info "First-boot provisioning already completed. Exiting." | ||
| exit 0 | ||
| fi | ||
| } | ||
|
|
||
| # Initialize logging | ||
| init_logging() { | ||
| mkdir -p "${LOG_DIR}" | ||
| touch "${LOG_FILE}" | ||
| chmod 640 "${LOG_FILE}" | ||
| log_info "=== Cortex Linux First-Boot Provisioning Started ===" | ||
| log_info "Hostname: $(hostname)" | ||
| log_info "Date: $(date)" | ||
| log_info "Kernel: $(uname -r)" | ||
| } |
There was a problem hiding this comment.
Make idempotency truly safe: init logging/state dirs before any possible logging and before writing STATE_FILE.
Today, a “completed” run can still fail if ${LOG_DIR} or ${LOG_FILE} is missing (e.g., user cleanup) because check_already_complete logs before init_logging, and mark_complete assumes ${PROVISION_DIR} exists.
Proposed fix (directory bootstrap first, then state check)
main() {
- check_already_complete
- init_logging
+ # Bootstrap dirs early so logging/state is safe even on "already complete" paths.
+ mkdir -p "${LOG_DIR}" "${PROVISION_DIR}"
+ touch "${LOG_FILE}"
+ chmod 640 "${LOG_FILE}"
+
+ check_already_complete
+ init_logging
check_networkAnd in mark_complete():
mark_complete() {
+ mkdir -p "${PROVISION_DIR}"
local completion_time🤖 Prompt for AI Agents
In @iso/provisioning/first-boot.sh around lines 16 - 55, The script currently
logs in check_already_complete and mark_complete before ensuring
LOG_DIR/LOG_FILE and PROVISION_DIR exist, causing non-idempotent failures;
modify startup to bootstrap directories and log file first (mkdir -p
"${LOG_DIR}" and "${PROVISION_DIR}", touch/chmod "${LOG_FILE}") before any call
to log or running check_already_complete, and update mark_complete() to ensure
PROVISION_DIR exists (mkdir -p "${PROVISION_DIR}") before creating STATE_FILE so
writing the state is safe and idempotent.
| # Configure SSH hardening | ||
| setup_ssh() { | ||
| log_info "Configuring SSH..." | ||
|
|
||
| local sshd_config="/etc/ssh/sshd_config.d/cortex-hardening.conf" | ||
|
|
||
| if [[ ! -f "${sshd_config}" ]]; then | ||
| mkdir -p /etc/ssh/sshd_config.d | ||
| cat > "${sshd_config}" << 'EOF' | ||
| # Cortex Linux SSH Hardening | ||
| # Disable root login | ||
| PermitRootLogin no | ||
| # Disable password authentication for root | ||
| PasswordAuthentication yes | ||
| PubkeyAuthentication yes | ||
| # Disable empty passwords | ||
| PermitEmptyPasswords no | ||
| # Limit authentication attempts | ||
| MaxAuthTries 3 | ||
| # Disable X11 forwarding (enable if needed) | ||
| X11Forwarding no | ||
| # Client alive settings | ||
| ClientAliveInterval 300 | ||
| ClientAliveCountMax 2 | ||
| # Disable TCP forwarding by default | ||
| AllowTcpForwarding no | ||
| # Use strong ciphers only | ||
| Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com | ||
| # Use strong MACs | ||
| MACs hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com | ||
| # Use strong key exchange | ||
| KexAlgorithms curve25519-sha256,curve25519-sha256@libssh.org | ||
| EOF | ||
| chmod 644 "${sshd_config}" | ||
| log_info "SSH hardening configured" | ||
|
|
||
| # Reload SSH if running | ||
| if systemctl is-active --quiet sshd; then | ||
| systemctl reload sshd || true | ||
| fi | ||
| else | ||
| log_info "SSH hardening already configured" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if there's any documentation or context about Cortex Linux in the repo
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.sh" \) | head -20 | xargs grep -l "cortex\|Cortex" 2>/dev/null | head -5Repository: cortexlinux/cortex-distro
Length of output: 198
🌐 Web query:
Debian systemctl SSH service name ssh vs sshd openssh-server
💡 Result:
Short answer: On Debian and Ubuntu the unit provided by the openssh-server package is ssh.service (you should use systemctl start|stop|status ssh). sshd (the daemon binary) and its config remain named sshd/sshd_config; sshd.service is an alias for compatibility on Debian-derived systems. [1][2][3]
Useful commands:
- Check unit: systemctl status ssh
- Binary path: which sshd or ps aux | grep sshd
- Config: /etc/ssh/sshd_config
Sources:
[1] Debian mailing list / openssh packaging (describes ssh.service, socket activation and aliasing).
[2] Ubuntu OpenSSH docs showing use of systemctl restart ssh.service and /etc/ssh/sshd_config.
[3] Discussion/examples showing ssh.service with Alias=sshd.service and note that sshd is the daemon program.
🌐 Web query:
openssh ciphers chacha20-poly1305 aes256-gcm aes128-gcm FIPS compliance compatibility
💡 Result:
Short answer
- OpenSSH supports chacha20-poly1305@openssh.com and the AES‑GCM variants (aes128-gcm@openssh.com, aes256-gcm@openssh.com). (docs.gigamon.com)
- ChaCha20‑Poly1305 is not NIST FIPS‑approved (and therefore is normally disallowed in FIPS mode / by FIPS crypto policies). AES‑GCM variants are accepted by FIPS policies and commonly allowed when the crypto provider is FIPS‑validated. (docs.redhat.com)
- Practically this means: on a system or distro in FIPS mode (or using a system‑wide FIPS crypto policy) OpenSSH will not offer/use ChaCha20‑Poly1305; AES‑GCM (and other FIPS‑approved AES ciphers/CTR modes) should be used for FIPS compliance. System/distro policies or OpenSSH/FIPS configuration can also remove or reorder advertised ciphers, so explicit sshd_config/ssh_config entries may be required for compatibility. (docs.redhat.com)
Compatibility guidance (practical)
- If you must be FIPS compliant: remove/disable chacha20-poly1305@openssh.com and prefer aes256-gcm@openssh.com or aes128-gcm@openssh.com (and ensure the underlying crypto library is FIPS‑validated and the host is in FIPS mode). (docs.redhat.com)
- If interoperating with non‑FIPS clients/servers and you want the best performance on low‑CPU devices, chacha20-poly1305 offers good performance but will break FIPS requirements — test and document exceptions if you decide to permit it. (docs.gigamon.com)
If you want, I can:
- show the exact sshd_config/ssh_config lines to enforce AES‑GCM only, or
- check how your OS/distribution’s FIPS policy will affect OpenSSH on a specific release (please tell me the OS and version).
Citations:
- 1: https://docs.gigamon.com/doclib612/Content/GV-Admin/SSH.html?utm_source=openai
- 2: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/8/epub/security_hardening/disabling-sha-1-by-customizing-a-system-wide-cryptographic-policy_using-the-system-wide-cryptographic-policies?utm_source=openai
- 3: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/8/epub/securing_networks/making-openssh-more-secure_assembly_using-secure-communications-between-two-systems-with-openssh?utm_source=openai
- 4: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/8/epub/security_hardening/disabling-sha-1-by-customizing-a-system-wide-cryptographic-policy_using-the-system-wide-cryptographic-policies?utm_source=openai
- 5: https://docs.gigamon.com/doclib612/Content/GV-Admin/SSH.html?utm_source=openai
🏁 Script executed:
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "README*" \) | xargs grep -i "fips\|compliance" 2>/dev/null | head -20Repository: cortexlinux/cortex-distro
Length of output: 230
🏁 Script executed:
# Check the provisioning script context and any related hardening docs
ls -la iso/provisioning/ 2>/dev/null || ls -la . | head -20Repository: cortexlinux/cortex-distro
Length of output: 322
🏁 Script executed:
# Look for any OS distro identification or FIPS mode checks in the script
cat iso/provisioning/first-boot.sh 2>/dev/null | head -50Repository: cortexlinux/cortex-distro
Length of output: 1288
🏁 Script executed:
# Check for more context on the SSH setup function and security requirements
sed -n '116,168p' iso/provisioning/first-boot.sh | head -60Repository: cortexlinux/cortex-distro
Length of output: 1362
🏁 Script executed:
# Look for any FIPS-related setup or security hardening docs in the repo
find . -type f -name "*.md" | xargs grep -i "ssh\|sshd\|hardening" 2>/dev/null | head -20Repository: cortexlinux/cortex-distro
Length of output: 1168
🌐 Web query:
Cortex Linux distro FIPS compliance security requirements
💡 Result:
Do you mean (A) Palo Alto Networks’ Cortex cloud product, (B) a specific Linux distribution named “Cortex”, or (C) running a Linux system on ARM Cortex processors? I can proceed once you confirm.
Meanwhile, here are the concise, general FIPS compliance security requirements for Linux distributions (what vendors follow and you must implement to claim FIPS compliance):
- Use NIST‑validated cryptographic modules (CMVP certificates) for all crypto operations; only the validated module versions and configurations listed in the module’s security policy may be used. [1][2][3]
- Enable “FIPS mode” at installation (or follow the vendor’s supported FIPS enablement procedure). Enabling FIPS after installing workloads may not guarantee compliance; many vendors require enabling during install or immediate post‑install steps and regenerating keys. [2][4][5]
- Restrict algorithms/ciphers to FIPS‑approved algorithms and approved protocol parameters (TLS suites, key sizes, KDFs, RNGs) and enforce system‑wide crypto policies so non‑approved ciphers are not offered. [4][5]
- Use the exact package versions of crypto libraries/modules that were validated (and keep to vendor guidance about updates/patch channels for validated packages). [6][3]
- Ensure kernel/OS module boundaries meet FIPS module definitions (some distros implement a kernel FIPS object module with self‑tests and early loading). Follow distro-specific kernel guidance. [7][4]
- Perform and preserve self‑tests (power‑up, conditional) and continuous integrity checks required by the validated module’s security policy. [1][7]
- Protect key material lifecycle: generate keys in FIPS mode, avoid exporting/using keys in non‑FIPS contexts unless allowed by policy (wrapping rules). [2][4]
- Maintain documentation and evidence (security policy, validated module certificate numbers, tested configuration) for audits and for CMVP mapping. [1][6]
- Follow vendor/distro-specific caveats and interoperability notes (e.g., TLS EMS requirements, disabled algorithms, compatibility impacts). Test workloads for functionality in FIPS mode. [4][5]
If you want, I can:
- Check whether a specific “Cortex” product/distribution has an assigned CMVP certificate and its security policy (and cite the exact certificate and requirements).
- Provide a step‑by‑step FIPS enablement checklist for a given distro (RHEL/Ubuntu/SLES/Oracle).
Which “Cortex” did you mean?
Sources: NIST CMVP and vendor FIPS docs (examples): [1] NIST CMVP certificate listing; [2] Red Hat FIPS docs; [3] Oracle Linux FIPS modules; [4] Linux kernel FIPS blog; [5] SUSE FIPS guidance; [6] Ubuntu FIPS announcement.
Fix comment mismatch, reorder crypto preferences for FIPS compliance, and use standard ssh service name.
- Comment states "Disable password authentication for root" but
PasswordAuthentication yesapplies globally (enables password auth). Update the comment to reflect the actual setting. - Listing
chacha20-poly1305@openssh.comfirst breaks FIPS compliance (ChaCha20-Poly1305 is not FIPS-approved). If FIPS compliance is a requirement, reorder to list only AES-GCM ciphers, or conditionally include ChaCha20 with documentation. If FIPS is not required, clarify this in a comment. - Use
ssh.serviceinstead ofsshd.serviceto match Debian/Ubuntu conventions (the openssh-server package providesssh.service;sshd.serviceis an alias for compatibility).
Suggested fixes
-# Disable password authentication for root
-PasswordAuthentication yes
+# Password authentication is enabled globally (set to "no" for key-only SSH)
+PasswordAuthentication yesReorder ciphers to prioritize FIPS-approved algorithms:
-Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com
+Ciphers aes256-gcm@openssh.com,aes128-gcm@openssh.com,chacha20-poly1305@openssh.comUse standard ssh service name:
- if systemctl is-active --quiet sshd; then
- systemctl reload sshd || true
+ if systemctl is-active --quiet ssh; then
+ systemctl reload ssh || true
fi🤖 Prompt for AI Agents
In @iso/provisioning/first-boot.sh around lines 116 - 168, The SSH hardening
block in setup_ssh has a mismatched comment for PasswordAuthentication, a
non‑FIPS-first cipher order in Ciphers, and uses the nonstandard service name
sshd; update the comment near PasswordAuthentication to reflect that it enables
or disables password auth globally (adjust text for PasswordAuthentication
yes/no to match the actual value of the setting), reorder the Ciphers line to
list AES-GCM ciphers first (e.g., aes256-gcm@openssh.com,aes128-gcm@openssh.com)
or remove chacha20-poly1305@openssh.com if FIPS compliance is required, and
change the systemctl checks/reload that reference sshd to use ssh (i.e., replace
systemctl is-active --quiet sshd and systemctl reload sshd with ssh) while
keeping the rest of setup_ssh and the sshd_config filename unchanged.
| # Configure automatic security updates | ||
| setup_unattended_upgrades() { | ||
| log_info "Configuring automatic security updates..." | ||
|
|
||
| if command -v unattended-upgrade &>/dev/null; then | ||
| local auto_upgrades="/etc/apt/apt.conf.d/20auto-upgrades" | ||
|
|
||
| if [[ ! -f "${auto_upgrades}" ]]; then | ||
| cat > "${auto_upgrades}" << 'EOF' | ||
| APT::Periodic::Update-Package-Lists "1"; | ||
| APT::Periodic::Unattended-Upgrade "1"; | ||
| APT::Periodic::Download-Upgradeable-Packages "1"; | ||
| APT::Periodic::AutocleanInterval "7"; | ||
| EOF | ||
| log_info "Automatic security updates enabled" | ||
| else | ||
| log_info "Automatic updates already configured" | ||
| fi | ||
| else | ||
| log_warn "unattended-upgrades not installed, skipping" | ||
| fi | ||
| } |
There was a problem hiding this comment.
Either install unattended-upgrades or remove this step (currently a no-op in the default package set).
Given the preseed package list, this will almost always log “not installed, skipping.”
🤖 Prompt for AI Agents
In @iso/provisioning/first-boot.sh around lines 235 - 256, The
setup_unattended_upgrades function is effectively a no-op under the current
preseed because unattended-upgrades isn't installed; either ensure the package
is installed before the check or remove the function call. Modify
setup_unattended_upgrades to invoke package installation (e.g., apt-get install
-y unattended-upgrades) if command -v returns false, or delete the
setup_unattended_upgrades invocation and its helper if you intend not to enable
unattended upgrades by default; refer to the function name
setup_unattended_upgrades and the auto_upgrades variable to locate the relevant
code to change.
| # Desktop environment setup | ||
| setup_desktop() { | ||
| log_info "Configuring desktop environment..." | ||
|
|
||
| # Add Flathub repository if flatpak is installed | ||
| if command -v flatpak &>/dev/null && [[ "${NETWORK_AVAILABLE}" == "true" ]]; then | ||
| if ! flatpak remote-list | grep -q flathub; then | ||
| flatpak remote-add --if-not-exists flathub https://flathub.org/repo/flathub.flatpakrepo || true | ||
| log_info "Flathub repository added" | ||
| fi | ||
| fi | ||
|
|
||
| # Configure Docker for cortex user | ||
| if command -v docker &>/dev/null; then | ||
| if ! groups cortex | grep -q docker; then | ||
| usermod -aG docker cortex | ||
| log_info "Added cortex user to docker group" | ||
| fi | ||
| fi | ||
| } |
There was a problem hiding this comment.
Guard groups/usermod with an existence check for the cortex user.
If the user creation ever changes (or fails), groups cortex/usermod will hard-fail provisioning under set -e.
Proposed guard
# Configure Docker for cortex user
if command -v docker &>/dev/null; then
- if ! groups cortex | grep -q docker; then
- usermod -aG docker cortex
- log_info "Added cortex user to docker group"
- fi
+ if id -u cortex &>/dev/null; then
+ if ! id -nG cortex | tr ' ' '\n' | grep -qx docker; then
+ usermod -aG docker cortex
+ log_info "Added cortex user to docker group"
+ fi
+ else
+ log_warn "User 'cortex' not found; skipping docker group setup"
+ fi
fi🤖 Prompt for AI Agents
In @iso/provisioning/first-boot.sh around lines 258 - 277, The setup_desktop
function calls groups cortex and usermod -aG docker cortex without checking the
cortex user exists, which can cause the script to fail under set -e; before
invoking groups or usermod, guard those calls with a check like getent passwd
cortex (or id -u cortex) to ensure the user exists, and only run groups and
usermod when that check succeeds so setup_desktop safely skips Docker group
configuration if the cortex user is missing.
| # Add Cortex repository (requires network) | ||
| setup_cortex_repository() { | ||
| if [[ "${NETWORK_AVAILABLE}" != "true" ]]; then | ||
| log_warn "Network unavailable, skipping Cortex repository setup" | ||
| return 0 | ||
| fi | ||
|
|
||
| log_info "Setting up Cortex package repository..." | ||
|
|
||
| local keyring_dir="/usr/share/keyrings" | ||
| local sources_file="/etc/apt/sources.list.d/cortex-linux.list" | ||
|
|
||
| # Check if already configured (from live image or previous run) | ||
| if [[ -f "${sources_file}" ]]; then | ||
| log_info "Cortex repository already configured" | ||
| return 0 | ||
| fi | ||
|
|
||
| log_info "Adding Cortex Linux APT repository..." | ||
|
|
||
| # Download and install GPG key | ||
| if curl -fsSL https://apt.cortexlinux.com/keys/cortex-linux.gpg.asc | gpg --dearmor -o "${keyring_dir}/cortex-linux.gpg"; then | ||
| log_info "Cortex GPG key installed" | ||
| else | ||
| log_error "Failed to download Cortex GPG key" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Add repository source | ||
| echo "deb [signed-by=${keyring_dir}/cortex-linux.gpg] https://apt.cortexlinux.com stable main" > "${sources_file}" | ||
|
|
||
| # Update package lists | ||
| if apt-get update -qq; then | ||
| log_info "Cortex repository configured successfully" | ||
| else | ||
| log_warn "apt-get update failed, repository may not be accessible" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
/usr/share/keyrings directory Debian Ubuntu default exists standard
💡 Result:
Short answer: No — /usr/share/keyrings is not guaranteed to be present on a fresh Debian/Ubuntu install; it is the conventional location for keyring files that are installed and managed by packages, and packages that provide keyrings will place files there. For locally-managed repository keys you should use /etc/apt/keyrings instead. [1][2][3]
Sources:
- Debian Wiki — DebianKeyring (notes that installed keyrings are placed in /usr/share/keyrings). [Debian Wiki]. [1]
- Debian Wiki — UseThirdParty / guidance: /usr/share/keyrings for package-managed keys, /etc/apt/keyrings for locally managed keys. [2]
- APT notes / documentation explaining how /usr/share/keyrings is used (ignored by default unless referenced with signed-by). [3]
🏁 Script executed:
# Check if there are other instances of keyring setup in the codebase
rg -n "keyrings|gpg.*--dearmor" iso/provisioning/ --type shRepository: cortexlinux/cortex-distro
Length of output: 313
🏁 Script executed:
# Check if there are other repository setup examples in the script
rg -n "apt-key|sources.list|signed-by" iso/provisioning/first-boot.sh -A 3 -B 3Repository: cortexlinux/cortex-distro
Length of output: 618
🌐 Web query:
/etc/apt/keyrings directory Debian Ubuntu APT local repository keys exists
💡 Result:
Short answer: Yes — /etc/apt/keyrings is the recommended place for locally-managed APT repository OpenPGP key files (keys managed by you, not by a package). Newer APT (since ~2.4) uses this location and you should reference the key with the signed-by= option in your source entry. On older Debian/Ubuntu releases the directory may not exist by default; create it (mode 0755) and place dearmoured .gpg key files there. [1][2][3]
Useful references:
- Debian guidance: Use /etc/apt/keyrings for locally-managed keys (use /usr/share/keyrings for keys installed by packages). [1]
- Debian SecureApt / apt-key deprecation details. [2]
- Ubuntu/apt manpage and examples showing /etc/apt/keyrings + signed-by usage. [3]
🌐 Web query:
apt-get GPG key file permissions requirements 0644 readable
💡 Result:
apt (and apt-key/apt-key-like checks) requires repository public-key files to be readable by the unprivileged _apt user. If a key file in /etc/apt/trusted.gpg or /etc/apt/trusted.gpg.d/ is not world- or _apt-readable apt will ignore it and you'll see messages like "the key(s) ... are ignored as the file is not readable by user '_apt'". Recommended permissions are 0644 (rw‑r--r--) with root ownership (or root:root); 0644 makes the file readable by _apt. Alternatively you may set owner to _apt so only that user needs read (chown _apt:root). Example fixes:
-
Make file world-readable:
sudo chmod 644 /etc/apt/trusted.gpg.d/NAME.gpg -
Or make it readable by _apt:
sudo chown _apt:root /etc/apt/trusted.gpg.d/NAME.gpg
sudo chmod 640 /etc/apt/trusted.gpg.d/NAME.gpg
Note: current best practice is to store third‑party keys under /etc/apt/keyrings or /usr/share/keyrings and use the signed-by option in your sources, instead of putting many keys into /etc/apt/trusted.gpg.d. (blog.cynexia.com)
Citations:
🏁 Script executed:
# Check if there are other uses of /etc/apt or keyrings in the codebase
rg -n "/etc/apt" iso/provisioning/ --type shRepository: cortexlinux/cortex-distro
Length of output: 271
Use /etc/apt/keyrings for locally-managed repository keys, not /usr/share/keyrings — and ensure directory and key file have correct permissions.
The current code uses /usr/share/keyrings, which is reserved for package-managed keys per Debian guidelines. Locally-downloaded keys should go in /etc/apt/keyrings. Additionally, create the directory with proper permissions and ensure the key file is readable by the _apt user.
Proposed fix
- local keyring_dir="/usr/share/keyrings"
+ local keyring_dir="/etc/apt/keyrings"
local sources_file="/etc/apt/sources.list.d/cortex-linux.list"
+ mkdir -p "${keyring_dir}"
+ chmod 0755 "${keyring_dir}"
# Download and install GPG key
if curl -fsSL https://apt.cortexlinux.com/keys/cortex-linux.gpg.asc | gpg --dearmor -o "${keyring_dir}/cortex-linux.gpg"; then
log_info "Cortex GPG key installed"
+ chmod 0644 "${keyring_dir}/cortex-linux.gpg"
else
log_error "Failed to download Cortex GPG key"
return 1| # Generate SSH host keys if missing | ||
| setup_ssh_keys() { | ||
| log_info "Checking SSH host keys..." | ||
|
|
||
| local keys_generated=false | ||
|
|
||
| for keytype in rsa ecdsa ed25519; do | ||
| local keyfile="/etc/ssh/ssh_host_${keytype}_key" | ||
| if [[ ! -f "${keyfile}" ]]; then | ||
| ssh-keygen -t "${keytype}" -f "${keyfile}" -N "" -q | ||
| keys_generated=true | ||
| log_info "Generated SSH ${keytype} host key" | ||
| fi | ||
| done | ||
|
|
||
| if [[ "${keys_generated}" == "true" ]]; then | ||
| systemctl restart sshd || true | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Debian SSH systemd service name sshd vs ssh unit
💡 Result:
Short answer
- On Debian the packaged systemd unit is named ssh.service (not sshd.service); the daemon binary remains /usr/sbin/sshd and config files are still sshd_config. [1][2]
- Debian also ships socket/template units ssh.socket and ssh@.service for socket-activated or traditional modes. Some recent/unreleased changes add an sshd@.service template in experimental/sid. [1][2][4]
What that means for you
- Use systemctl start|stop|restart ssh (or manage ssh.socket / ssh@.service for socket activation). systemctl restart sshd may not exist on some Debian/Ubuntu installs. [1][2]
- The “sshd” names are still used for the binary, configs and PAM files (sshd_config, /etc/pam.d/sshd). Renaming the unit to ssh is a packaging/compatibility choice. [2][5]
References
[1] Debian openssh-server file list (shows ssh.service, ssh.socket, ssh@.service).
[2] Debian mailing-list discussion / systemd support in openssh-server.
[3] Ask Ubuntu explanation that Debian/Ubuntu use ssh.service (not sshd).
[4] Debian bug/changelog showing addition of sshd@.service template in newer packaging.
[5] Debian package file lists showing /usr/sbin/sshd and sshd_config.
🌐 Web query:
systemctl restart sshd Debian compatibility service alias
💡 Result:
Short answer: On Debian the canonical systemd unit is ssh.service; sshd.service is provided as a compatibility alias so "systemctl restart sshd" usually works. Use ssh.service when creating drop-ins or editing the unit (override files must use the real unit name). [1][2][3]
Sources:
- Debian Handbook — ssh.service unit (includes "Alias=sshd.service"). [1]
- Debian Wiki (systemd guidance: use canonical service name). [2]
- Debian mailing-list note: sshd.service is an alias for ssh.service (implications for drop‑ins). [3]
🏁 Script executed:
cat -n iso/provisioning/first-boot.sh | head -50Repository: cortexlinux/cortex-distro
Length of output: 1638
🏁 Script executed:
# Check for any OS detection or Debian-specific logic in the script
rg -i "debian|ubuntu|systemd" iso/provisioning/first-boot.sh -B 2 -A 2Repository: cortexlinux/cortex-distro
Length of output: 195
Use the correct SSH systemd unit name for Debian: ssh.service instead of sshd.
On Debian, the canonical systemd unit is ssh.service, not sshd. While sshd is provided as a compatibility alias on most installs, it may not exist on all Debian/Ubuntu systems, causing the restart to fail silently or error. Use ssh.service for consistency and reliability:
systemctl restart ssh || true
🤖 Prompt for AI Agents
In @iso/provisioning/first-boot.sh around lines 318 - 336, In setup_ssh_keys(),
the systemctl call uses the Debian-incompatible unit name "sshd"; change the
restart to use the canonical "ssh" unit instead (i.e., replace the systemctl
restart sshd || true call with systemctl restart ssh || true) so the SSH service
restart is reliable across Debian/Ubuntu systems after generating host keys.
- Updated README.md to streamline installation instructions and clarify build commands for ISO and dependencies. - Removed obsolete references to specific profiles in the automated installation guide, consolidating to a single preseed file. - Enhanced the developers' guide with clearer instructions for building ISOs and managing package lists. - Introduced a new package list for complete system installations, replacing the removed core profile. - Improved overall documentation structure for better clarity and usability.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @docs/DEVELOPERS.md:
- Around line 137-142: The QEMU ARM64 example is missing firmware specification
causing boot failures; update the qemu-system-aarch64 invocation to add a
firmware flag (use either -bios or -pflash) and point it to the typical UEFI
firmware file (e.g., edk2-aarch64-code.fd), for example modify the
qemu-system-aarch64 command shown so it includes "-bios edk2-aarch64-code.fd"
(or "-pflash <path-to-edk2-aarch64-code.fd>") alongside the existing -M, -cpu,
-m and -cdrom options to ensure the VM can boot the ISO.
In @README.md:
- Around line 39-43: Update the README examples to reflect the date-based ISO
versioning used by ISO_VERSION: replace the hardcoded
cortex-linux-0.1.0-amd64.iso examples with a date-based pattern like
cortex-linux-YYYYMMDD-amd64.iso (e.g., cortex-linux-20250112-amd64.iso) or add a
note that the ISO name is generated from ISO_VERSION (date format), while
leaving the SBOM examples (which use CORTEX_VERSION and default to 0.1.0)
unchanged.
🧹 Nitpick comments (4)
docs/automated-installation.md (1)
25-27: Add language specifiers to fenced code blocks.Several code blocks are missing language specifiers, which affects syntax highlighting and accessibility tools. Per static analysis hints, add appropriate language identifiers.
Suggested fixes
Line 25: Add language specifier for kernel boot parameters
-``` +```text preseed/file=/cdrom/preseed/cortex.preseedLine 66: Add language specifier for preseed syntax
-``` +```text d-i partman-auto/expert_recipe string \Line 109: Add language specifier for DHCP config
-``` +```text filename "pxelinux.0";Line 131: Add language specifier for PXE config
-``` +```text DEFAULT cortexLine 216: Add language specifier for preseed
-``` +```text d-i netcfg/disable_autoconfig boolean trueAlso applies to: 66-74, 109-112, 131-136, 216-222
docs/DEVELOPERS.md (1)
208-212: Consider adding automated link verification to CI.The Resources section links to external Debian documentation. Consider implementing periodic link validation in the CI/build process to catch broken references early, though this is optional.
README.md (2)
93-95: Clarify what's included versus what needs additional installation.The terms "GPU support prerequisites" and "AI/ML prerequisites" are ambiguous. Consider specifying whether:
- GPU drivers (NVIDIA/AMD) are pre-installed, or just kernel modules/dependencies
- AI/ML packages (numpy, scipy, pandas) are pre-installed, or users need to install them post-boot
Suggestion: Use consistent language like "GPU driver support for NVIDIA and AMD" or "Pre-installed AI/ML libraries (numpy, scipy, pandas)" to clarify what's actually in the ISO.
📝 Proposed clarification
-- GPU support prerequisites (NVIDIA, AMD) +- GPU driver support for NVIDIA and AMD hardware - Modern CLI tools (htop, btop, fzf, ripgrep, bat) -- AI/ML prerequisites (numpy, scipy, pandas) +- Pre-installed AI/ML libraries (numpy, scipy, pandas)
48-72: Consider adding a language specifier to the directory tree code block.The static analysis tool flagged the fenced code block for missing a language specifier. While this is a minor issue, adding
textorplaintextafter the opening backticks would resolve the warning and improve Markdown compliance.📝 Proposed fix for Markdown compliance
-``` +```text cortex-distro/ ├── iso/ # ISO build configurationBased on static analysis hints from markdownlint-cli2.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mddocs/DEVELOPERS.mddocs/QUICK-START.mddocs/automated-installation.mdiso/live-build/config/package-lists/cortex-core.list.chrootiso/live-build/config/package-lists/cortex.list.chroot
💤 Files with no reviewable changes (1)
- iso/live-build/config/package-lists/cortex-core.list.chroot
🧰 Additional context used
🪛 LanguageTool
docs/automated-installation.md
[grammar] ~177-~177: Ensure spelling is correct
Context: ...rd:** Must be set at first login > The preseed uses a placeholder password hash. The f...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
README.md
56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/automated-installation.md
25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
131-131: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
216-216: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (2)
- GitHub Check: Build ISO (amd64)
- GitHub Check: Build (First Pass)
🔇 Additional comments (17)
docs/DEVELOPERS.md (2)
1-33: Clear and well-structured setup instructions.The prerequisites section provides both automated (
install-deps.sh) and manual installation paths, which is helpful for different user preferences. The version verification step forlive-buildis a good practice.
54-81: Project structure documentation is comprehensive.The tree structure clearly documents the repository layout, including the new branding, provisioning, and live-build components introduced in this PR. This aligns well with the modular build system changes.
iso/live-build/config/package-lists/cortex.list.chroot (3)
233-238: Cortex package placeholders are appropriately commented.The commented placeholders for
cortex-archive-keyring,cortex-core, andcortex-consolecorrectly indicate these will be enabled when the Cortex APT repository is available. This aligns with the modular branding approach documented in the PR.
1-14: LGTM on the refactored system base section.The reorganization from "INCLUDE CORE" to "SYSTEM BASE" with explicit essential system utilities improves clarity. The package list now clearly defines the foundational system components.
151-157: No action needed.nvidia-smiis available as a standalone package in Debian's non-free archive and can be installed independently. The package is properly listed.Likely an incorrect or invalid review comment.
docs/automated-installation.md (2)
76-101: First-boot provisioning documentation is thorough.The documentation clearly explains idempotency, offline capability, state tracking location (
/opt/cortex/provisioning/.first-boot-complete), and log locations. This aligns with the provisioning scripts introduced iniso/provisioning/.
230-236: Security considerations section provides essential guidance.The recommendations for SSH key regeneration, system updates, firewall review, and fail2ban configuration are appropriate for production deployments.
docs/QUICK-START.md (4)
1-4: Clear and concise introduction.The "2 minutes" promise sets appropriate expectations for the repository installation path. The guide effectively provides two clear installation options.
36-42: System requirements are reasonable.The minimum (2GB RAM, 10GB disk) and recommended (4GB+ RAM, 20GB+ disk) requirements are appropriate for a development-oriented Linux distribution with container and AI/ML tooling.
86-89: Cross-references to related documentation are helpful.The links to
automated-installation.mdandDEVELOPERS.mdprovide clear next steps for users who need more advanced workflows.
7-17: Verify APT repository URL is correct and accessible.The documentation references
https://apt.cortexlinux.comfor the GPG key and repository installation. Ensure this domain is properly configured and both the/keys/cortex-linux.gpg.ascand repository endpoints are accessible, or update to the correct URL if different.README.md (6)
46-72: LGTM! Clear and well-organized architecture documentation.The directory structure accurately reflects the new branding-centric organization and clearly documents the preseed/provisioning infrastructure for automated installations.
161-171: LGTM! Build targets are clear and well-documented.The simplified build target documentation aligns well with the streamlined build workflow. The explicit ARM64 target and install-deps convenience target improve usability.
196-196: Good addition of ARM64 architecture support.Explicitly documenting ARM64 support aligns with the
make iso ARCH=arm64build target and makes hardware requirements clear for users.
134-134: Verify the cortex-branding package path and version.The example references
packages/cortex-branding_1.0.0_all.deb. Confirm:
- The package directory structure matches this path
- The version
1.0.0is intentional (differs from the ISO version0.1.0)- The package naming convention follows Debian standards (package_version_arch.deb)
103-103: Preseed file is properly included.The preseed file exists at
iso/preseed/cortex.preseedand is tracked in the repository, so the boot parameter reference at line 103 is correct and will be properly included in the ISO build.
24-25: Thescripts/install-deps.shscript exists with executable permissions set correctly. No action required.
| qemu-system-x86_64 \ | ||
| -enable-kvm \ | ||
| -m 4096 \ | ||
| -cpu host \ | ||
| -cdrom cortex-linux-*.iso \ | ||
| -drive file=cortex.qcow2,format=qcow2 \ | ||
| -boot d \ | ||
| -append "auto=true priority=critical preseed/file=/cdrom/preseed/cortex.preseed" | ||
| ``` |
There was a problem hiding this comment.
QEMU -append flag requires -kernel to function.
The -append parameter only works when booting with -kernel directly, not when booting from an ISO via -cdrom. For ISO-based installation with preseed, the boot parameters need to be edited interactively at the GRUB menu or baked into the ISO's bootloader configuration.
Suggested documentation fix
# Boot from ISO with preseed
qemu-system-x86_64 \
-enable-kvm \
-m 4096 \
-cpu host \
-cdrom cortex-linux-*.iso \
-drive file=cortex.qcow2,format=qcow2 \
- -boot d \
- -append "auto=true priority=critical preseed/file=/cdrom/preseed/cortex.preseed"
+ -boot d
+
+# At the GRUB menu, edit the boot entry and append:
+# auto=true priority=critical preseed/file=/cdrom/preseed/cortex.preseed📝 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.
| qemu-system-x86_64 \ | |
| -enable-kvm \ | |
| -m 4096 \ | |
| -cpu host \ | |
| -cdrom cortex-linux-*.iso \ | |
| -drive file=cortex.qcow2,format=qcow2 \ | |
| -boot d \ | |
| -append "auto=true priority=critical preseed/file=/cdrom/preseed/cortex.preseed" | |
| ``` | |
| qemu-system-x86_64 \ | |
| -enable-kvm \ | |
| -m 4096 \ | |
| -cpu host \ | |
| -cdrom cortex-linux-*.iso \ | |
| -drive file=cortex.qcow2,format=qcow2 \ | |
| -boot d | |
| # At the GRUB menu, edit the boot entry and append: | |
| # auto=true priority=critical preseed/file=/cdrom/preseed/cortex.preseed |
| # QEMU (amd64) | ||
| qemu-system-x86_64 -m 4G -cdrom output/cortex-linux-*.iso -boot d | ||
|
|
||
| # QEMU (arm64) | ||
| qemu-system-aarch64 -M virt -cpu cortex-a72 -m 4G -cdrom output/cortex-linux-*-arm64.iso | ||
| ``` |
There was a problem hiding this comment.
QEMU arm64 command may not boot correctly without a firmware path.
The qemu-system-aarch64 command for ARM64 testing is missing the -bios or -pflash flag to specify UEFI firmware (typically edk2-aarch64-code.fd). Without this, the VM may fail to boot the ISO.
Suggested fix
# QEMU (arm64)
-qemu-system-aarch64 -M virt -cpu cortex-a72 -m 4G -cdrom output/cortex-linux-*-arm64.iso
+qemu-system-aarch64 -M virt -cpu cortex-a72 -m 4G \
+ -bios /usr/share/qemu-efi-aarch64/QEMU_EFI.fd \
+ -cdrom output/cortex-linux-*-arm64.iso -boot d🤖 Prompt for AI Agents
In @docs/DEVELOPERS.md around lines 137 - 142, The QEMU ARM64 example is missing
firmware specification causing boot failures; update the qemu-system-aarch64
invocation to add a firmware flag (use either -bios or -pflash) and point it to
the typical UEFI firmware file (e.g., edk2-aarch64-code.fd), for example modify
the qemu-system-aarch64 command shown so it includes "-bios
edk2-aarch64-code.fd" (or "-pflash <path-to-edk2-aarch64-code.fd>") alongside
the existing -M, -cpu, -m and -cdrom options to ensure the VM can boot the ISO.
| ├── cortex-linux-0.1.0-amd64.iso # Bootable ISO | ||
| ├── cortex-linux-0.1.0-amd64.iso.sha256 | ||
| └── sbom/ | ||
| ├── cortex-linux-0.1.0.cdx.json # CycloneDX SBOM | ||
| └── cortex-linux-0.1.0.spdx.json # SPDX SBOM | ||
| ├── cortex-linux-0.1.0.cdx.json # CycloneDX SBOM | ||
| └── cortex-linux-0.1.0.spdx.json # SPDX SBOM |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for version references in build configuration and common config files
rg -n 'version\s*[=:]\s*["\']?0\.1\.0' -i --max-count=20Repository: cortexlinux/cortex-distro
Length of output: 137
🏁 Script executed:
# Look for version definitions in common configuration files
fd -e 'py|toml|json|yaml|yml|sh|Makefile|mk|cfg|ini' --max-depth 3 | head -20Repository: cortexlinux/cortex-distro
Length of output: 51
🏁 Script executed:
# Check the README.md file for version context
cat -n README.md | sed -n '30,50p'Repository: cortexlinux/cortex-distro
Length of output: 632
🏁 Script executed:
# Search for version definitions without quote issues
rg -n '0\.1\.0' --max-count=30Repository: cortexlinux/cortex-distro
Length of output: 1199
🏁 Script executed:
# Search for VERSION variable definitions
rg -n 'VERSION\s*=' --max-count=20Repository: cortexlinux/cortex-distro
Length of output: 800
🏁 Script executed:
# List top-level files to identify build config
ls -la | grep -E 'Makefile|setup|version|VERSION'Repository: cortexlinux/cortex-distro
Length of output: 127
🏁 Script executed:
# Check if there's a Makefile and search for version there
if [ -f Makefile ]; then head -50 Makefile; fiRepository: cortexlinux/cortex-distro
Length of output: 1483
🏁 Script executed:
# Check Makefile for how ISO_VERSION and ISO_NAME are used
cat -n Makefile | grep -A 5 -B 5 'ISO_NAME\|ISO_VERSION'Repository: cortexlinux/cortex-distro
Length of output: 1880
🏁 Script executed:
# Check the build.sh script to see actual ISO filename generation
rg -n 'ISO_NAME|ISO_VERSION' scripts/build.sh -A 2 -B 2Repository: cortexlinux/cortex-distro
Length of output: 727
Update README.md output examples to reflect actual versioning scheme.
The documented ISO filename examples in lines 39-43 are inaccurate. The build system uses a date-based version format (YYYYMMDD) for ISO_VERSION, not the hardcoded 0.1.0 shown in the README. The actual ISO output will be cortex-linux-YYYYMMDD-amd64.iso (e.g., cortex-linux-20250112-amd64.iso), not cortex-linux-0.1.0-amd64.iso. Update the documentation examples to match the actual build output, or clearly indicate these are example filenames only.
Note: SBOM filenames default to 0.1.0 via CORTEX_VERSION, so those examples are correct; only the ISO filename examples require updating.
🤖 Prompt for AI Agents
In @README.md around lines 39 - 43, Update the README examples to reflect the
date-based ISO versioning used by ISO_VERSION: replace the hardcoded
cortex-linux-0.1.0-amd64.iso examples with a date-based pattern like
cortex-linux-YYYYMMDD-amd64.iso (e.g., cortex-linux-20250112-amd64.iso) or add a
note that the ISO name is generated from ISO_VERSION (date format), while
leaving the SBOM examples (which use CORTEX_VERSION and default to 0.1.0)
unchanged.
- Updated GitHub Actions workflow to use xz compression for release builds and lz4 for faster CI builds, optimizing build times and output size. - Modified build.sh to support dynamic compression type based on the build context, improving efficiency and logging. - Added liblz4-tool to the dependency installation script to support lz4 compression during builds.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Around line 245-253: The GitHub Actions step named "Create Release" uses
softprops/action-gh-release@v1 which is outdated; update the uses value to
softprops/action-gh-release@v2 in that step (keep the existing inputs like files
unchanged), commit the workflow change, and validate by running the workflow or
a dry-run to ensure the v2 action accepts the same inputs and produces the
expected release artifacts.
- Around line 192-199: The Verify checksums step's loop can exit silently if no
.sha256 files exist; modify the shell block in the "Verify checksums" step to
detect whether any checksum files were found and validated (e.g., set a counter
or use nullglob to list *.sha256) and fail the step if none were verified by
exiting non-zero or printing an error; ensure you still run sha256sum -c for
each existing "${iso}.sha256" but after the loop check the counter (or absence
of matches) and call exit 1 with a clear message if zero checksum files were
processed.
- Around line 240-243: The checksum commands can fail when no .iso or .deb files
exist; update the workflow step that runs sha256sum and sha512sum to first
detect matching files (e.g., using a glob check or find) and only run sha256sum
*.iso *.deb and sha512sum *.iso *.deb when the file list is non-empty, otherwise
create empty/placeholder SHA256SUMS and SHA512SUMS or skip generation; modify
the shell block containing the sha256sum and sha512sum invocations so it guards
execution with a file-existence check before calling those tools.
In @scripts/build.sh:
- Around line 682-691: The script block that copies DEBIAN control files then
unconditionally runs chmod on "${pkg_dir}/DEBIAN/postinst" and
"${pkg_dir}/DEBIAN/prerm" can fail if those files are absent; update the logic
in scripts/build.sh to test for each file's existence before changing
permissions (or only chmod if the file exists) after the cp step, referencing
the variables PACKAGES_DIR, pkg_name and pkg_dir and the filenames postinst and
prerm so the script skips chmod when a file is missing and avoids a failure.
- Around line 286-294: The arithmetic expansion currently uses `$?` after an
assignment so it captures the exit status of the assignment (always 0) instead
of the function's return value; for each call (validate_provision,
validate_hooks, run_shellcheck) capture the function's exit code into a
temporary variable immediately after the call (e.g., local ret=0;
validate_provision || ret=$?) and then add that ret to errors with
errors=$((errors + ret)); this avoids miscapturing `$?` and is safe if set -e is
enabled.
🧹 Nitpick comments (2)
scripts/install-deps.sh (1)
14-16: Minor inconsistency in log prefix format.The logging functions here use
[+],[!], and[ERROR]prefixes, whilescripts/build.shuses[BUILD],[WARN], and[ERROR]. Consider aligning the prefix style for consistency across scripts. Based on the relevant code snippets frombuild.sh.♻️ Optional: Align with build.sh logging style
-log() { echo -e "${GREEN}[+]${NC} $1"; } -warn() { echo -e "${YELLOW}[!]${NC} $1"; } +log() { echo -e "${GREEN}[DEPS]${NC} $1"; } +warn() { echo -e "${YELLOW}[WARN]${NC} $1"; }scripts/build.sh (1)
547-547:sudo lb buildmay fail silently underset -ein certain edge cases.The
sudo lb buildcommand runs the actual ISO build. Consider capturing and checking the exit status explicitly, and logging more context on failure to aid debugging.♻️ Proposed enhancement
build_iso() { header "Building ISO" cd "$BUILD_DIR" log "Starting live-build (this may take a while)..." - sudo lb build + if ! sudo lb build; then + error "live-build failed. Check build.log for details." + exit 1 + fi log "Build complete" }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-iso.ymlscripts/build.shscripts/install-deps.sh
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/install-deps.sh (1)
scripts/build.sh (3)
error(44-44)log(42-42)warn(43-43)
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
246-246: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (2)
- GitHub Check: Build ISO (amd64)
- GitHub Check: Build (First Pass)
🔇 Additional comments (8)
scripts/install-deps.sh (2)
32-50: LGTM! Comprehensive package list for live-build.The package selection covers all necessary build tools including
live-build,debootstrap,squashfs-tools,xorriso, bootloader utilities, and the newly addedliblz4-toolfor lz4 compression support as mentioned in the PR objectives.
56-65: No changes needed. The current bootloader package installation is correct for amd64 builds. The standardgrub-pc-bin(64-bit) already supports legacy BIOS systems on amd64 architecture. There is no evidence in the codebase indicating that i386 cross-compilation support or multiarch configuration is required for this project's build process..github/workflows/build-iso.yml (2)
33-48: LGTM! Validation job properly configured.The validation job correctly runs in a Debian bookworm container with necessary dependencies (git, sudo, make, shellcheck) and delegates to the Makefile's validate target.
122-141: Verify privileged Docker mode is acceptable for your CI environment.The
--privilegedflag grants extensive capabilities to the container. While this is typically required forlive-buildoperations (loopback mounts, chroot), ensure this aligns with your security policies.scripts/build.sh (4)
7-7: Good use of strict mode.
set -euo pipefailprovides robust error handling: exit on error, undefined variable access, and pipeline failures. This is best practice for build scripts.
63-72: Unquoted glob expansion is intentional but fragile with special characters.The
copy_glob_if_existsfunction correctly disables shellcheck for the intentional glob expansion. However, if filenames contain spaces or special characters, this will break. Consider usingfindwith-execfor robustness if this becomes an issue.
758-797: LGTM! Comprehensive help documentation.The help text clearly documents all commands, environment variables, and provides usage examples. This improves developer experience significantly.
803-844: LGTM! Clean entry point with proper command dispatch.The main function handles command dispatch cleanly with a sensible default (
help) and proper error handling for unknown commands.
| - name: Verify checksums | ||
| run: | | ||
| ls -la | ||
| for iso in *.iso; do | ||
| if [ -f "${iso}.sha256" ]; then | ||
| sha256sum -c "${iso}.sha256" | ||
| fi | ||
| done |
There was a problem hiding this comment.
Checksum verification loop may silently pass when no checksums exist.
If no .sha256 files are present, the loop completes without error. Consider adding a check to ensure at least one checksum file was verified.
🔧 Proposed fix
- name: Verify checksums
run: |
ls -la
+ verified=0
for iso in *.iso; do
if [ -f "${iso}.sha256" ]; then
sha256sum -c "${iso}.sha256"
+ verified=$((verified + 1))
fi
done
+ if [ $verified -eq 0 ]; then
+ echo "WARNING: No checksum files found to verify"
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Verify checksums | |
| run: | | |
| ls -la | |
| for iso in *.iso; do | |
| if [ -f "${iso}.sha256" ]; then | |
| sha256sum -c "${iso}.sha256" | |
| fi | |
| done | |
| - name: Verify checksums | |
| run: | | |
| ls -la | |
| verified=0 | |
| for iso in *.iso; do | |
| if [ -f "${iso}.sha256" ]; then | |
| sha256sum -c "${iso}.sha256" | |
| verified=$((verified + 1)) | |
| fi | |
| done | |
| if [ $verified -eq 0 ]; then | |
| echo "WARNING: No checksum files found to verify" | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 192 - 199, The Verify checksums
step's loop can exit silently if no .sha256 files exist; modify the shell block
in the "Verify checksums" step to detect whether any checksum files were found
and validated (e.g., set a counter or use nullglob to list *.sha256) and fail
the step if none were verified by exiting non-zero or printing an error; ensure
you still run sha256sum -c for each existing "${iso}.sha256" but after the loop
check the counter (or absence of matches) and call exit 1 with a clear message
if zero checksum files were processed.
| run: | | ||
| cd release | ||
| sha256sum *.iso *.deb > SHA256SUMS | ||
| sha512sum *.iso *.deb > SHA512SUMS |
There was a problem hiding this comment.
Checksum generation may fail if no matching files exist.
If the build produces no .iso or .deb files, sha256sum and sha512sum will fail with "no input files". Consider adding a guard.
🔧 Proposed fix
- name: Generate combined checksums
run: |
cd release
- sha256sum *.iso *.deb > SHA256SUMS
- sha512sum *.iso *.deb > SHA512SUMS
+ if ls *.iso *.deb 1>/dev/null 2>&1; then
+ sha256sum *.iso *.deb > SHA256SUMS
+ sha512sum *.iso *.deb > SHA512SUMS
+ else
+ echo "No ISO or DEB files found for checksumming"
+ exit 1
+ fi🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 240 - 243, The checksum
commands can fail when no .iso or .deb files exist; update the workflow step
that runs sha256sum and sha512sum to first detect matching files (e.g., using a
glob check or find) and only run sha256sum *.iso *.deb and sha512sum *.iso *.deb
when the file list is non-empty, otherwise create empty/placeholder SHA256SUMS
and SHA512SUMS or skip generation; modify the shell block containing the
sha256sum and sha512sum invocations so it guards execution with a file-existence
check before calling those tools.
| ;; | ||
| provision) | ||
| validate_provision || errors=$((errors + $?)) | ||
| ;; | ||
| hooks) | ||
| validate_hooks || errors=$((errors + $?)) | ||
| ;; | ||
| lint) | ||
| run_shellcheck || errors=$((errors + $?)) |
There was a problem hiding this comment.
Arithmetic expansion of $? after || always yields 0 or 1.
When using validate_provision || errors=$((errors + $?)), the $? after || captures the exit status of the assignment (always 0), not validate_provision. The functions return actual error counts, but this pattern doesn't capture them correctly.
🔧 Proposed fix using command substitution or direct capture
provision)
- validate_provision || errors=$((errors + $?))
+ validate_provision || errors=1
;;
hooks)
- validate_hooks || errors=$((errors + $?))
+ validate_hooks || errors=1
;;
lint)
- run_shellcheck || errors=$((errors + $?))
+ run_shellcheck || errors=1
;;
all)
validate_preseed
echo ""
- validate_provision || errors=$((errors + $?))
+ validate_provision || errors=1
echo ""
- validate_hooks || errors=$((errors + $?))
+ validate_hooks || errors=1
echo ""
- run_shellcheck || errors=$((errors + $?))
+ run_shellcheck || errors=1
;;Alternatively, if you need the actual error count:
if ! validate_provision; then
errors=$((errors + $?))
fiBut note that set -e may interfere. A cleaner approach:
local ret=0
validate_provision || ret=$?
errors=$((errors + ret))📝 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.
| ;; | |
| provision) | |
| validate_provision || errors=$((errors + $?)) | |
| ;; | |
| hooks) | |
| validate_hooks || errors=$((errors + $?)) | |
| ;; | |
| lint) | |
| run_shellcheck || errors=$((errors + $?)) | |
| ;; | |
| provision) | |
| validate_provision || errors=1 | |
| ;; | |
| hooks) | |
| validate_hooks || errors=1 | |
| ;; | |
| lint) | |
| run_shellcheck || errors=1 |
🤖 Prompt for AI Agents
In @scripts/build.sh around lines 286 - 294, The arithmetic expansion currently
uses `$?` after an assignment so it captures the exit status of the assignment
(always 0) instead of the function's return value; for each call
(validate_provision, validate_hooks, run_shellcheck) capture the function's exit
code into a temporary variable immediately after the call (e.g., local ret=0;
validate_provision || ret=$?) and then add that ret to errors with
errors=$((errors + ret)); this avoids miscapturing `$?` and is safe if set -e is
enabled.
- Changed environment variable from LB_COMPRESSION to SQUASHFS_COMP in the GitHub Actions workflow and build.sh script to ensure uniformity in compression handling. - This update enhances clarity and maintains consistency across the build process.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build-iso.yml:
- Around line 245-253: The workflow is using the outdated
softprops/action-gh-release@v1 action which is incompatible with newer runners;
update the uses entry to softprops/action-gh-release@v2 (replace
"softprops/action-gh-release@v1" with "@v2") and ensure the existing inputs (the
multi-line files list: release/*.iso, release/*.deb, release/SHA256SUMS,
release/SHA512SUMS, release/sbom/*) remain unchanged and still supported by v2;
run a test pipeline to confirm the release step executes successfully and adjust
any v2-specific input names if the action's docs require it.
🧹 Nitpick comments (3)
.github/workflows/build-iso.yml (1)
192-208: Addnullglobor check for ISO existence to handle missing files gracefully.If no
.isofiles exist (e.g., build failure),*.isoremains literal and the loops will fail confusingly. Consider adding a check or using bash'snullglob.Suggested improvement
- name: Verify checksums run: | ls -la + shopt -s nullglob + isos=(*.iso) + if [ ${#isos[@]} -eq 0 ]; then + echo "ERROR: No ISO files found" + exit 1 + fi for iso in *.iso; do if [ -f "${iso}.sha256" ]; then sha256sum -c "${iso}.sha256" fi donescripts/build.sh (2)
541-550: Consider validating sudo availability before use.The script uses
sudo lb buildwithout first checking if sudo is available or if the user has the necessary privileges. This could lead to confusing failures partway through the build process.💡 Suggested improvement
Add a sudo check early in
cmd_build:cmd_build() { header "Building Cortex Linux ISO" log "Architecture: ${ARCH}" log "Debian version: ${DEBIAN_VERSION}" + # Verify sudo access early to fail fast + if ! sudo -v 2>/dev/null; then + error "sudo access required for building ISO" + exit 1 + fi + prepare_build_dir
619-626: Add safety check forrm -rfon variable paths.While
set -uprovides some protection against unset variables, adding an explicit check before destructive operations is a defensive practice that prevents accidental data loss if variable definitions are ever modified.💡 Suggested defensive check
cmd_clean_all() { header "Cleaning all build artifacts" + # Safety check to prevent accidental rm -rf on root or home + if [[ -z "$BUILD_DIR" || "$BUILD_DIR" == "/" || "$BUILD_DIR" == "$HOME" ]]; then + error "BUILD_DIR has unsafe value: '$BUILD_DIR'" + exit 1 + fi + if [[ -z "$OUTPUT_DIR" || "$OUTPUT_DIR" == "/" || "$OUTPUT_DIR" == "$HOME" ]]; then + error "OUTPUT_DIR has unsafe value: '$OUTPUT_DIR'" + exit 1 + fi + rm -rf "$BUILD_DIR" rm -rf "$OUTPUT_DIR" log "Full clean complete" }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-iso.ymlscripts/build.sh
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/build-iso.yml
246-246: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ 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). (2)
- GitHub Check: Build ISO (amd64)
- GitHub Check: Build (First Pass)
🔇 Additional comments (16)
.github/workflows/build-iso.yml (4)
75-86: LGTM on the matrix strategy and job dependencies.The multi-architecture build matrix with
fail-fast: falseis appropriate - it allows both architectures to complete even if one fails, which is useful for debugging. The dependency chain (validate→build-branding-package→build-iso) ensures proper build ordering.
122-141: Docker-based build approach is solid.The compression strategy (xz for releases, lz4 for PR builds) provides a good balance between size and build speed. Using
set -eensures the build fails fast on errors. The privileged container is necessary for live-build operations.
210-237: Release artifact collection is well-structured.The
continue-on-error: truefor SBOM download is appropriate since SBOM generation may be optional. Usingmerge-multiple: truewith patterns cleanly consolidates artifacts from the matrix builds.
88-97: Thejlumbroso/free-disk-spaceaction does not publish versioned releases. The repository is maintained only on the main branch, making@mainthe only available reference. The suggested pin tov1.3.1is not a valid version tag for this action and cannot be implemented.If reproducibility and supply-chain safety are concerns with using
@main, consider evaluating alternative disk-cleanup actions that publish releases, or accept the trade-off of using@mainfor this particular action.Likely an incorrect or invalid review comment.
scripts/build.sh (12)
1-36: LGTM!The script header and configuration section are well-structured with sensible defaults, proper
set -euo pipefailfor strict error handling, and clear organization.
42-47: LGTM!Clean logging functions with appropriate color coding and stderr output for errors.
63-72: LGTM!The
copy_glob_if_existsfunction correctly uses shellcheck disable for intentional glob expansion. The pattern check before copy prevents errors on non-matching globs.
90-150: LGTM!Thorough dependency checking with clear separation between required and optional dependencies. The version checks using
dpkg --compare-versionsare appropriate for a Debian-based distro build script.
156-180: LGTM!The preseed validation correctly checks for trailing whitespace which could cause issues in preseed files. The regex
'^[^#]*[[:space:]]$'appropriately ignores commented lines.
277-318: LGTM!The
cmd_validatedispatcher is well-structured with clear mode handling. The error accumulation pattern|| errors=$((errors + $?))correctly captures function return values before the||short-circuit evaluates.
324-420: LGTM!Comprehensive test suite covering preseed files, provisioning scripts, branding assets, hooks, and package control files. The syntax validation for shell scripts and consistent pass/fail reporting provide good pre-build verification.
491-518: LGTM!The
configure_live_buildfunction has a well-documented compression choice (lz4 for CI speed vs xz for release size), comprehensivelb configoptions, and good logging.
644-658: LGTM!The sync function correctly checks for directory existence before copying and provides appropriate warnings.
664-752: LGTM!The branding package function correctly structures the .deb package with appropriate directory hierarchy, sets proper permissions on maintainer scripts, and includes the GRUB background conversion for compatibility. The cleanup of the temporary package directory prevents accumulation of build artifacts.
758-797: LGTM!Comprehensive help documentation with clear command descriptions, environment variable documentation, and practical examples.
803-844: LGTM!Clean command dispatcher with sensible default to help, graceful handling of missing arguments via
shift || true, and proper error handling for unknown commands.
| - name: Create Release | ||
| uses: softprops/action-gh-release@v1 | ||
| with: | ||
| files: | | ||
| *.iso | ||
| *.deb | ||
| SHA256SUMS | ||
| SHA512SUMS | ||
| release/*.iso | ||
| release/*.deb | ||
| release/SHA256SUMS | ||
| release/SHA512SUMS | ||
| release/sbom/* |
There was a problem hiding this comment.
Update softprops/action-gh-release to v2.
Static analysis indicates @v1 is too old for current GitHub Actions runners. The action should be updated to v2 for compatibility and continued support.
Suggested fix
- name: Create Release
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2
with:
files: |📝 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.
| - name: Create Release | |
| uses: softprops/action-gh-release@v1 | |
| with: | |
| files: | | |
| *.iso | |
| *.deb | |
| SHA256SUMS | |
| SHA512SUMS | |
| release/*.iso | |
| release/*.deb | |
| release/SHA256SUMS | |
| release/SHA512SUMS | |
| release/sbom/* | |
| - name: Create Release | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| files: | | |
| release/*.iso | |
| release/*.deb | |
| release/SHA256SUMS | |
| release/SHA512SUMS | |
| release/sbom/* |
🧰 Tools
🪛 actionlint (1.7.10)
246-246: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/build-iso.yml around lines 245 - 253, The workflow is
using the outdated softprops/action-gh-release@v1 action which is incompatible
with newer runners; update the uses entry to softprops/action-gh-release@v2
(replace "softprops/action-gh-release@v1" with "@v2") and ensure the existing
inputs (the multi-line files list: release/*.iso, release/*.deb,
release/SHA256SUMS, release/SHA512SUMS, release/sbom/*) remain unchanged and
still supported by v2; run a test pipeline to confirm the release step executes
successfully and adjust any v2-specific input names if the action's docs require
it.
- Implemented functionality to copy the GRUB theme from the branding directory to the includes.chroot directory during the build process. - Ensured that the theme is copied both in the prepare_build_dir and cmd_sync functions, enhancing the customization of the installed system's boot experience.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @scripts/build.sh:
- Around line 695-704: The script unconditionally chmods DEBIAN/postinst and
DEBIAN/prerm which will fail if those files are absent; update the block after
the cp to test each file exists before changing mode (e.g., if [ -f
"${pkg_dir}/DEBIAN/postinst" ]; then chmod 755 "${pkg_dir}/DEBIAN/postinst"; fi
and same for prerm), or alternatively iterate over existing files matching
"${pkg_dir}/DEBIAN/postinst" and "${pkg_dir}/DEBIAN/prerm" and chmod only those
present to avoid "No such file or directory" errors.
🧹 Nitpick comments (2)
scripts/build.sh (2)
375-387: Minor inconsistency: Hook test only checks.hook.chrootfiles.The
cmd_testfunction only tests*.hook.chrootfiles, whilevalidate_hooks(line 217) tests both*.hook.chrootand*.hook.binary. Consider aligning the test coverage.♻️ Suggested fix
# Test hooks log "Testing live-build hooks..." local hooks_dir="${ISO_DIR}/live-build/config/hooks/live" - for hook in "$hooks_dir"/*.hook.chroot; do + for hook in "$hooks_dir"/*.hook.chroot "$hooks_dir"/*.hook.binary; do if [ -f "$hook" ]; then if bash -n "$hook"; then pass "$(basename "$hook"): syntax OK"
652-670: Silent failure on copy errors.The
[ -d ... ] && cp -r ...pattern masks copy failures. If a directory exists but the copy fails (e.g., permission denied, disk full), no error is reported and the function logs "Config synced" despite incomplete sync.♻️ Suggested fix using explicit error handling
cmd_sync() { header "Syncing config" if [ -d "$BUILD_DIR" ]; then - [ -d "${ISO_DIR}/live-build/config/hooks" ] && \ - cp -r "${ISO_DIR}/live-build/config/hooks" "${BUILD_DIR}/config/" - [ -d "${ISO_DIR}/live-build/config/includes.chroot" ] && \ - cp -r "${ISO_DIR}/live-build/config/includes.chroot" "${BUILD_DIR}/config/" - [ -d "${ISO_DIR}/live-build/config/includes.binary" ] && \ - cp -r "${ISO_DIR}/live-build/config/includes.binary" "${BUILD_DIR}/config/" + if [ -d "${ISO_DIR}/live-build/config/hooks" ]; then + cp -r "${ISO_DIR}/live-build/config/hooks" "${BUILD_DIR}/config/" && log "Synced hooks" + fi + if [ -d "${ISO_DIR}/live-build/config/includes.chroot" ]; then + cp -r "${ISO_DIR}/live-build/config/includes.chroot" "${BUILD_DIR}/config/" && log "Synced includes.chroot" + fi + if [ -d "${ISO_DIR}/live-build/config/includes.binary" ]; then + cp -r "${ISO_DIR}/live-build/config/includes.binary" "${BUILD_DIR}/config/" && log "Synced includes.binary" + fi # Copy GRUB theme to includes.chroot if [ -d "${BRANDING_DIR}/grub/cortex" ]; then mkdir -p "${BUILD_DIR}/config/includes.chroot/boot/grub/themes/cortex" - cp -r "${BRANDING_DIR}/grub/cortex/"* "${BUILD_DIR}/config/includes.chroot/boot/grub/themes/cortex/" + cp -r "${BRANDING_DIR}/grub/cortex/"* "${BUILD_DIR}/config/includes.chroot/boot/grub/themes/cortex/" && log "Synced GRUB theme" fi log "Config synced" else warn "Build directory not found: ${BUILD_DIR}" fi }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/build.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). (2)
- GitHub Check: Build ISO (amd64)
- GitHub Check: Build (First Pass)
🔇 Additional comments (8)
scripts/build.sh (8)
1-48: LGTM!The configuration and logging setup is well-structured. Good use of
set -euo pipefailfor strict error handling, and the path derivation fromSCRIPT_DIRis safe.
53-84: LGTM!Helper functions are appropriately designed. The intentional unquoted glob expansion in
copy_glob_if_existsis acceptable for this controlled build environment, and the shellcheck disable comment correctly acknowledges this.
90-150: LGTM!Thorough dependency checking with good separation between required and optional dependencies. The version checking for live-build and Python is well implemented with appropriate fallbacks and warnings.
152-318: LGTM!The validation subsystem is well-designed with clear separation of concerns. The
|| errors=$((errors + $?))pattern correctly captures error counts while preventing script exit underset -e.
422-609: LGTM!The ISO build functions are well-structured. The dual GRUB theme copying (in
prepare_build_dirfor the installed system andcopy_grub_themefor live boot) aligns with the commit message objectives. Theexit 1on missing requiredtheme.txtis appropriate.
611-646: LGTM!Clean functions are appropriately designed. The
rm -rfoperations on build/output directories are safe since the paths are derived from the project root.
771-810: LGTM!Comprehensive and well-organized help text covering all commands, environment variables, and examples.
816-857: LGTM!Clean dispatcher pattern with good defaults. The
shift || truepattern correctly handles the case when no arguments are provided.
| # Copy DEBIAN control files | ||
| log "Copying DEBIAN control files..." | ||
| if [ -d "${PACKAGES_DIR}/${pkg_name}/DEBIAN" ]; then | ||
| cp "${PACKAGES_DIR}/${pkg_name}/DEBIAN/"* "${pkg_dir}/DEBIAN/" | ||
| chmod 755 "${pkg_dir}/DEBIAN/postinst" | ||
| chmod 755 "${pkg_dir}/DEBIAN/prerm" | ||
| else | ||
| error "DEBIAN control files not found" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Potential error if postinst or prerm don't exist.
Lines 699-700 unconditionally run chmod 755 on postinst and prerm, but the glob copy on line 698 doesn't guarantee these specific files exist. If either file is missing from the source DEBIAN/ directory, the script will fail with "No such file or directory".
🐛 Suggested fix
# Copy DEBIAN control files
log "Copying DEBIAN control files..."
if [ -d "${PACKAGES_DIR}/${pkg_name}/DEBIAN" ]; then
cp "${PACKAGES_DIR}/${pkg_name}/DEBIAN/"* "${pkg_dir}/DEBIAN/"
- chmod 755 "${pkg_dir}/DEBIAN/postinst"
- chmod 755 "${pkg_dir}/DEBIAN/prerm"
+ [ -f "${pkg_dir}/DEBIAN/postinst" ] && chmod 755 "${pkg_dir}/DEBIAN/postinst"
+ [ -f "${pkg_dir}/DEBIAN/prerm" ] && chmod 755 "${pkg_dir}/DEBIAN/prerm"
else
error "DEBIAN control files not found"
exit 1
fi🤖 Prompt for AI Agents
In @scripts/build.sh around lines 695 - 704, The script unconditionally chmods
DEBIAN/postinst and DEBIAN/prerm which will fail if those files are absent;
update the block after the cp to test each file exists before changing mode
(e.g., if [ -f "${pkg_dir}/DEBIAN/postinst" ]; then chmod 755
"${pkg_dir}/DEBIAN/postinst"; fi and same for prerm), or alternatively iterate
over existing files matching "${pkg_dir}/DEBIAN/postinst" and
"${pkg_dir}/DEBIAN/prerm" and chmod only those present to avoid "No such file or
directory" errors.
- Deleted the .github/workflows/reproducible-builds.yml file, removing the entire reproducible builds workflow from the CI/CD pipeline. - This change simplifies the workflow configuration and eliminates the need for maintaining the associated build and comparison steps.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot:
- Around line 99-102: The sed replacement will never trigger the echo fallback
because sed exits 0 when no match is found; change the logic to explicitly test
for an existing GRUB_DISTRIBUTOR line first (e.g., grep -q '^GRUB_DISTRIBUTOR='
/etc/default/grub), and if found run the sed substitution to replace its value,
otherwise append GRUB_DISTRIBUTOR="Cortex Linux" to /etc/default/grub; update
the block that currently runs sed ... || echo to use this conditional check so
the echo fallback actually occurs when the variable is missing.
- Around line 19-31: The branding assets use two different domains; standardize
to a single canonical domain across all files by replacing all occurrences of
the alternate domain with the chosen canonical one: update the os-release
entries HOME_URL, SUPPORT_URL and PRIVACY_POLICY_URL (currently set to
cortex.io) to match the canonical domain you pick, and update the MOTD footer
(the footer text that references cortexlinux.com/support and
docs.cortexlinux.com) and any branding comments that reference
apt.cortexlinux.com so they all use the same domain string consistently; ensure
you change every occurrence (HOME_URL, SUPPORT_URL, PRIVACY_POLICY_URL, MOTD
footer text, and branding comments) to the chosen canonical domain.
🧹 Nitpick comments (5)
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (5)
160-162: Loop may silently skip on glob mismatch.When
/etc/update-motd.d/*matches no files, bash defaults to iterating over the literal string. The[ -f "$f" ]guard handles this, but addingshopt -s nullglobor checking for directory existence first would be cleaner.♻️ Optional improvement
# Disable default Debian MOTD scripts +if [ -d /etc/update-motd.d ]; then -for f in /etc/update-motd.d/*; do - [ -f "$f" ] && chmod -x "$f" -done + for f in /etc/update-motd.d/*; do + [ -f "$f" ] && chmod -x "$f" + done +fi
198-206: Minor: Avoid useless use of cat.
cat /proc/loadavg | awkcan be simplified toawk '{print $1, $2, $3}' /proc/loadavg.♻️ Suggested simplification
-LOAD=$(cat /proc/loadavg | awk '{print $1, $2, $3}') +LOAD=$(awk '{print $1, $2, $3}' /proc/loadavg)
232-245: Potential slow login if apt cache is stale.
apt list --upgradablecan be slow without a recentapt update. Consider caching results or adding a timeout to avoid blocking user login.♻️ Add timeout to prevent login delays
else # Quick check (may not be accurate without apt update) - UPDATES=$(apt list --upgradable 2>/dev/null | grep -c upgradable || echo "0") + UPDATES=$(timeout 2 apt list --upgradable 2>/dev/null | grep -c upgradable || echo "0") SECURITY="?" fi
269-274: Potential command injection if notice file contains shell metacharacters.While
/opt/cortex/noticeis likely controlled, piping file contents directly tosedand displaying could have unexpected behavior with special characters. Consider usingcatwith proper quoting orprintf '%s\n'.♻️ Safer approach
CORTEX_NOTICE="/opt/cortex/notice" if [ -f "$CORTEX_NOTICE" ]; then echo -e " ${CYAN}◆ Cortex Notice:${RESET}" - cat "$CORTEX_NOTICE" | sed 's/^/ /' + while IFS= read -r line; do + printf ' %s\n' "$line" + done < "$CORTEX_NOTICE" echo "" fi
352-362: Consider usingimage_sourceinstead ofascii_distrofor custom ASCII.Neofetch's
ascii_distrois designed for built-in distro names. For custom ASCII art, usingimage_source="/usr/share/neofetch/ascii/distro/cortex"in the config may be more reliable.♻️ More reliable custom ASCII configuration
# Cortex branding colors (purple/cyan) colors=(5 6 7 5 6 7) bold="on" -ascii_distro="cortex" +image_source="/usr/share/neofetch/ascii/distro/cortex" +image_backend="ascii"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/reproducible-builds.ymliso/live-build/config/hooks/live/50-cortex-branding.hook.chrootiso/live-build/config/package-lists/cortex.list.chroot
💤 Files with no reviewable changes (1)
- .github/workflows/reproducible-builds.yml
⏰ 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). (1)
- GitHub Check: Build ISO (amd64)
🔇 Additional comments (7)
iso/live-build/config/package-lists/cortex.list.chroot (4)
1-14: LGTM! System base packages are well-organized.The essential system utilities section correctly includes the minimal required packages for a functioning Debian-based system (systemd, dbus, udev, init).
191-191: Good addition to support branding.Adding
neofetchaligns with the branding hook script that creates custom Cortex neofetch configuration and ASCII art.
234-239: Placeholder packages are clearly documented.The commented Cortex packages with the explanatory note about APT repository availability is a reasonable approach for bootstrapping.
151-156: No action needed — nvidia-smi is available as a standalone package in Debian Bookworm.
nvidia-smiis provided as a standalone non-free package in Debian Bookworm and resolves independently with its dependencies (libnvidia-ml runtime and related packages). The package list inclusion is valid.Likely an incorrect or invalid review comment.
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (3)
9-11: Good use of strict error handling.
set -eensures the script fails fast on errors, appropriate for a build-time hook where partial branding would be problematic.
368-373: Good: initramfs update included for Plymouth changes.Running
update-initramfs -uensures the Plymouth theme is properly included in the initramfs for boot splash display.
345-349: No action required. Theascii_distro="cortex"configuration will work correctly because the hook script itself creates the ASCII file at/usr/share/neofetch/ascii/distro/cortex(lines 352–354) before the neofetch config references it. The ASCII art is properly formatted with valid color tokens.Likely an incorrect or invalid review comment.
| cat > /etc/os-release << 'EOF' | ||
| PRETTY_NAME="Cortex Linux" | ||
| NAME="Cortex Linux" | ||
| VERSION_ID="1.0" | ||
| VERSION="1.0 (Bookworm)" | ||
| VERSION_CODENAME=bookworm | ||
| ID=cortex | ||
| ID_LIKE=debian | ||
| HOME_URL="https://cortex.io" | ||
| SUPPORT_URL="https://cortex.io/support" | ||
| BUG_REPORT_URL="https://github.com/cortex/cortex-distro/issues" | ||
| PRIVACY_POLICY_URL="https://cortex.io/privacy" | ||
| EOF |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for domain references in the entire repository
echo "=== All references to cortex domains ==="
rg "cortex\.io|cortexlinux\.com|cortexlinux\.io|docs\.cortexlinux" -n --type sh --type txt
echo ""
echo "=== Checking the specific file ==="
cat -n iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot 2>/dev/null | head -40Repository: cortexlinux/cortex-distro
Length of output: 2331
🏁 Script executed:
#!/bin/bash
# Check file length and search for MOTD
echo "=== File line count ==="
wc -l iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot
echo ""
echo "=== Looking for MOTD section ==="
rg -n "MOTD|motd" iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot
echo ""
echo "=== Search for domain references in the branding file ==="
rg -n "cortex\.io|cortexlinux\.com|cortexlinux\.io|docs\.cortexlinux" iso/live-build/config/hooks/live/50-cortex-branding.hook.chrootRepository: cortexlinux/cortex-distro
Length of output: 1566
Resolve domain inconsistency across branding files.
The os-release file uses cortex.io (HOME_URL, SUPPORT_URL, PRIVACY_POLICY_URL) while the MOTD footer references cortexlinux.com and docs.cortexlinux.com. Choose one canonical domain and update all branding files accordingly:
- os-release (lines 27-30):
cortex.io - MOTD footer (line 306):
cortexlinux.com/supportanddocs.cortexlinux.com - Branding file comments (lines 53, 56-57): reference
apt.cortexlinux.com
🤖 Prompt for AI Agents
In @iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot around lines
19 - 31, The branding assets use two different domains; standardize to a single
canonical domain across all files by replacing all occurrences of the alternate
domain with the chosen canonical one: update the os-release entries HOME_URL,
SUPPORT_URL and PRIVACY_POLICY_URL (currently set to cortex.io) to match the
canonical domain you pick, and update the MOTD footer (the footer text that
references cortexlinux.com/support and docs.cortexlinux.com) and any branding
comments that reference apt.cortexlinux.com so they all use the same domain
string consistently; ensure you change every occurrence (HOME_URL, SUPPORT_URL,
PRIVACY_POLICY_URL, MOTD footer text, and branding comments) to the chosen
canonical domain.
| # Also set timeout and other branding options | ||
| sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub || \ | ||
| echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub | ||
| fi |
There was a problem hiding this comment.
Bug: GRUB_DISTRIBUTOR will not be appended if missing.
The sed command returns exit code 0 even when the pattern isn't found (it just produces no substitutions). The || echo fallback will never execute.
🐛 Proposed fix
# Also set timeout and other branding options
- sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub || \
- echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub
+ if grep -q "^GRUB_DISTRIBUTOR=" /etc/default/grub; then
+ sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub
+ else
+ echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Also set timeout and other branding options | |
| sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub || \ | |
| echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub | |
| fi | |
| # Also set timeout and other branding options | |
| if grep -q "^GRUB_DISTRIBUTOR=" /etc/default/grub; then | |
| sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub | |
| else | |
| echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In @iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot around lines
99 - 102, The sed replacement will never trigger the echo fallback because sed
exits 0 when no match is found; change the logic to explicitly test for an
existing GRUB_DISTRIBUTOR line first (e.g., grep -q '^GRUB_DISTRIBUTOR='
/etc/default/grub), and if found run the sed substitution to replace its value,
otherwise append GRUB_DISTRIBUTOR="Cortex Linux" to /etc/default/grub; update
the block that currently runs sed ... || echo to use this conditional check so
the echo fallback actually occurs when the variable is missing.
- Introduced a new GNOME terminal profile for Cortex Linux, enhancing user experience with customized colors and settings. - Updated PAM configuration files for login and SSH services to enable dynamic message of the day (MOTD) display. - Added a new .bashrc file with Cortex-specific aliases and prompt customization for improved usability and branding.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot:
- Around line 98-124: The GNOME Terminal profile block uses an invalid literal
name "cortex"; replace the profile path
[org/gnome/terminal/legacy/profiles:/:cortex] with a valid UUID-based path
(e.g., [org/gnome/terminal/legacy/profiles:/:<UUID>]) and update
default='cortex' and list=['cortex'] to reference that same UUID string; keep
visible-name='Cortex' unchanged and generate the UUID with uuidgen (or similar)
so all three places (profile section name, default, and list) use the identical
UUID.
In @iso/live-build/config/includes.chroot/etc/pam.d/login:
- Line 16: Remove the deprecated and insecure user_readenv=1 option from the
pam_env.so session line: locate the line that invokes pam_env.so (currently
"session required pam_env.so user_readenv=1 envfile=/etc/default/locale") and
either drop the user_readenv=1 entirely or change it to user_readenv=0 so the
entry reads "session required pam_env.so envfile=/etc/default/locale" (or with
user_readenv=0) to mitigate the CVE issues.
🧹 Nitpick comments (9)
iso/live-build/config/includes.chroot/etc/skel/.bashrc (2)
46-61: Consider using bash pattern matching for efficiency.The function spawns multiple subshells (
echo | grep) for each status check. Using bash's built-in pattern matching would be faster, especially since this runs on every prompt.♻️ Suggested refactor using bash patterns
__git_status() { local status="" local git_status=$(git status --porcelain 2>/dev/null) if [[ -n "$git_status" ]]; then - if echo "$git_status" | grep -q "^??"; then + if [[ "$git_status" == *$'\n?? '* ]] || [[ "$git_status" == '?? '* ]]; then status+="?" fi - if echo "$git_status" | grep -q "^.M\|^M"; then + if [[ "$git_status" =~ (^|$'\n')[^\ ]?M ]]; then status+="*" fi - if echo "$git_status" | grep -q "^A\|^.A"; then + if [[ "$git_status" =~ (^|$'\n')[^\ ]?A ]]; then status+="+" fi fi [[ -n "$status" ]] && echo " $status" }
87-89: Remove redundant stderr redirection.
&>/dev/nullalready redirects both stdout and stderr, making the trailing2>&1redundant.♻️ Suggested fix
- if git rev-parse --is-inside-work-tree &>/dev/null 2>&1; then + if git rev-parse --is-inside-work-tree &>/dev/null; theniso/live-build/config/hooks/live/99-configure-gnome.hook.chroot (3)
25-40: Duplicate GDM configuration files with identical content.Both
daemon.conf(Lines 27-32) andcustom.conf(Lines 35-40) contain the same[daemon]section. GDM3 typically reads fromcustom.conf, makingdaemon.confredundant. This duplication may cause maintenance confusion.Consider removing
daemon.confand keeping onlycustom.conf:Proposed fix
# Configure GDM to start automatically in live mode mkdir -p /etc/gdm3 -cat > /etc/gdm3/daemon.conf << 'EOF' -# Cortex Linux GDM Configuration for Live Environment -[daemon] -AutomaticLoginEnable=true -AutomaticLogin=cortex -EOF - -# Configure GDM session cat > /etc/gdm3/custom.conf << 'EOF' # Cortex Linux GDM Custom Configuration [daemon] AutomaticLoginEnable=true AutomaticLogin=cortex + +[security] + +[xdmcp] + +[chooser] + +[debug] EOF
61-61: Unnecessary executable permission on .desktop file.Desktop entry files don't require executable permission in modern GNOME environments. The
Execline is interpreted by the desktop environment, not executed directly.Proposed fix
-chmod +x /etc/skel/.config/autostart/welcome.desktop
150-153: Schema compilation may fail silently in chroot.The
glib-compile-schemascall suppresses all output, making debugging difficult if it fails. Consider adding error handling or at least logging the result.Proposed fix
# Compile schemas if command -v glib-compile-schemas &>/dev/null; then - glib-compile-schemas /usr/share/glib-2.0/schemas/ + glib-compile-schemas /usr/share/glib-2.0/schemas/ || echo " WARNING: Failed to compile GLib schemas" fiiso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (4)
99-102: Potential double-append of GRUB_DISTRIBUTOR.If
sedfails for a reason other than the pattern not being found (e.g., permission issue), the|| echofallback will append a duplicateGRUB_DISTRIBUTORline. Consider checking if the key exists first:Proposed fix
# Also set timeout and other branding options - sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub || \ - echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub + if grep -q "^GRUB_DISTRIBUTOR=" /etc/default/grub; then + sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub + else + echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub + fi fi
232-245: Update check logic conflates errors with "no updates".Line 239's
grep -c upgradable || echo "0"returns"0"both when there are no updates and whenapt listfails. This could mislead users into thinking their system is up-to-date when apt encountered an error.Proposed fix
else # Quick check (may not be accurate without apt update) - UPDATES=$(apt list --upgradable 2>/dev/null | grep -c upgradable || echo "0") + UPDATES=$(apt list --upgradable 2>/dev/null | grep -c upgradable) || UPDATES="?" SECURITY="?" fi
260-263: Complex pipeline may produce unexpected output with special characters.If package names contain unusual characters, the
tr '\n' ','andsedpipeline could produce malformed output. Additionally,cat | sedis a UUOC (useless use of cat).Proposed fix
if [ -f "/var/run/reboot-required.pkgs" ]; then - REBOOT_PKGS=$(cat /var/run/reboot-required.pkgs | head -3 | tr '\n' ', ' | sed 's/,$//') + REBOOT_PKGS=$(head -3 /var/run/reboot-required.pkgs | paste -sd ', ') echo -e " ${GRAY} Packages: ${REBOOT_PKGS}${RESET}" fi
269-274: UUOC and potential issues with untrusted notice content.Using
cat "$CORTEX_NOTICE" | sedis unnecessary, and if the notice file contains escape sequences, they could be interpreted unexpectedly byecho -eelsewhere or cause display issues.Proposed fix
CORTEX_NOTICE="/opt/cortex/notice" if [ -f "$CORTEX_NOTICE" ]; then echo -e " ${CYAN}◆ Cortex Notice:${RESET}" - cat "$CORTEX_NOTICE" | sed 's/^/ /' + sed 's/^/ /' "$CORTEX_NOTICE" echo "" fi
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
branding/terminal/cortex-terminal.dconfiso/live-build/config/hooks/live/50-cortex-branding.hook.chrootiso/live-build/config/hooks/live/99-configure-gnome.hook.chrootiso/live-build/config/includes.chroot/etc/pam.d/loginiso/live-build/config/includes.chroot/etc/pam.d/sshdiso/live-build/config/includes.chroot/etc/skel/.bashrc
✅ Files skipped from review due to trivial changes (1)
- branding/terminal/cortex-terminal.dconf
⏰ 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). (1)
- GitHub Check: Build ISO (amd64)
🔇 Additional comments (8)
iso/live-build/config/includes.chroot/etc/skel/.bashrc (5)
1-8: LGTM - Well-structured shell configuration.The interactive shell check, history configuration, shell options, dircolors setup, and bash-completion sourcing all follow standard Debian/Ubuntu conventions and are implemented correctly.
Also applies to: 10-24, 103-114, 146-155
26-43: LGTM - Thoughtful prompt design.The branded prompt correctly captures exit code at function entry, differentiates root vs. regular user styling, and includes git branch/status integration. The use of
PROMPT_COMMANDfor dynamic prompt generation is appropriate.Also applies to: 64-86, 90-101
119-144: LGTM - Reasonable alias set.The safety aliases (
rm -i,cp -i,mv -i) are user-friendly defaults, and the htop-to-top fallback (line 139) gracefully handles missing packages. Cortex-specific aliases provide convenient shortcuts for system maintenance.
157-164: LGTM - Smart neofetch gate.Using
CORTEX_WELCOMEDenvironment variable to prevent duplicate neofetch runs in subshells is a good pattern. Thecommand -vcheck ensures graceful handling when neofetch isn't installed.
166-181: LGTM - PATH handling is correct.The directory existence checks before PATH modification are appropriate, and prepending user directories ensures user-local binaries take precedence over system-wide ones.
iso/live-build/config/includes.chroot/etc/pam.d/sshd (1)
1-15: LGTM!The PAM configuration follows standard Debian patterns for sshd. The dual
pam_motd.soapproach correctly displays both dynamic (/run/motd.dynamic) and static MOTD content. Themodule_unknown=ignorehandling forpam_selinux.soensures graceful fallback on systems without SELinux.iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (2)
1-11: Overall script structure is well-organized.The branding hook follows good practices:
set -efor early failure detection, clear section comments, and graceful fallbacks for missing commands. The modular approach with verification warnings for missing assets aids debugging during ISO builds.Also applies to: 366-374
159-162: This code is intentional branding behavior; no action needed.The MOTD script disabling is documented through the section header, file purpose, and immediate replacement with a custom Cortex MOTD. Disabling default MOTD scripts does not break system behavior—MOTD only displays informational messages at login. Being selective would defeat the purpose of a branding hook that replaces default Debian messaging with Cortex identity.
Likely an incorrect or invalid review comment.
| [org/gnome/terminal/legacy/profiles:/:cortex] | ||
| audible-bell=false | ||
| background-color='#0D0D0D' | ||
| bold-color='#06B6D4' | ||
| bold-color-same-as-fg=false | ||
| bold-is-bright=true | ||
| cursor-background-color='#06B6D4' | ||
| cursor-colors-set=true | ||
| cursor-foreground-color='#0D0D0D' | ||
| cursor-shape='ibeam' | ||
| default-size-columns=120 | ||
| default-size-rows=32 | ||
| font='JetBrains Mono 11' | ||
| foreground-color='#E5E5E5' | ||
| highlight-background-color='#6B21A8' | ||
| highlight-colors-set=true | ||
| highlight-foreground-color='#FFFFFF' | ||
| palette=['#0D0D0D', '#EF4444', '#22C55E', '#EAB308', '#3B82F6', '#A855F7', '#06B6D4', '#A3A3A3', '#525252', '#F87171', '#4ADE80', '#FACC15', '#60A5FA', '#C084FC', '#22D3EE', '#F5F5F5'] | ||
| scroll-on-output=false | ||
| scrollback-lines=10000 | ||
| use-system-font=false | ||
| use-theme-colors=false | ||
| visible-name='Cortex' | ||
| [org/gnome/terminal/legacy/profiles:] | ||
| default='cortex' | ||
| list=['cortex'] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file and its full context
head -n 150 iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot | tail -n 60Repository: cortexlinux/cortex-distro
Length of output: 1907
🏁 Script executed:
# Search for how GNOME Terminal profiles are configured in the repository
rg -n "profiles" iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot -A 2 -B 2Repository: cortexlinux/cortex-distro
Length of output: 422
🌐 Web query:
GNOME Terminal dconf profile format UUID vs arbitrary names
💡 Result:
Short answer
- GNOME Terminal stores each profile under a path that uses a UUID (GUID): /org/gnome/terminal/legacy/profiles:/:/ — the profile list key (/org/gnome/terminal/legacy/profiles:/ list) contains those UUID strings. The human-visible name is stored separately as the visible-name key inside the profile. You cannot use arbitrary plain names as the profile path; use a UUID (you can generate one with uuidgen). [1][2][3]
Sources
- GNOME Terminal FAQ (profile ID & gsettings examples). [1]
- Explanation of relocatable schema and profile storage (org.gnome.Terminal.Legacy.Profile at /.../profiles:/:/). [2]
- Examples showing gsettings/dconf paths and profile UUID list. [3]
🌐 Web query:
org/gnome/terminal/legacy/profiles dconf configuration requirements
💡 Result:
Summary — what dconf expects for /org/gnome/terminal/legacy/profiles:
- Top-level path: /org/gnome/terminal/legacy/profiles:/ contains at least:
- default — string UUID of the default profile. [1]
- list — array of profile UUID strings (order not important). [1][3]
- Each profile is stored under the path template:
/org/gnome/terminal/legacy/profiles:/:/
and must be a keyfile section (i.e., keys/values under that path). Typical keys come from the org.gnome.Terminal.Legacy.Profile schema (visible-name, font, use-system-font, palette, background-color, foreground-color, use-theme-colors, default-size-columns/rows, etc.). Types follow the schema (strings, booleans, integers, arrays). [2][3] - dconf/gsettings syntax notes:
- dconf paths include the colon after profiles (profiles:/). Use the full path when writing with dconf. Example: dconf write /org/gnome/terminal/legacy/profiles:/:/font "'Monospace 12'". Note the nested quoting for string values. [2][3]
- Equivalent gsettings form uses the schema prefix: gsettings set org.gnome.Terminal.Legacy.Profile:/org/gnome/terminal/legacy/profiles:/:/ . [2]
- Backup/restore and reset:
- Use dconf dump /org/gnome/terminal/ to export, dconf load /org/gnome/terminal/ to import, and dconf reset -f /org/gnome/terminal/ to wipe. [3]
- dconf profile/database behavior:
- System/user databases and dconf profiles determine precedence (user-db then system-db lines in /etc/dconf/profile/*). Changes require re-login to apply profile changes. [1]
If you want, I can show exact dconf write/gsettings commands for a specific key or produce a sample dconf dump for one profile.
Sources: [1] GNOME dconf profiles docs; [2] gsettings/dconf notes/example usage; [3] example dconf dump and keys.
Terminal profile must use a valid UUID, not arbitrary names.
GNOME Terminal identifies profiles by UUID in dconf, not arbitrary names. The path [org/gnome/terminal/legacy/profiles:/:cortex] and default='cortex' will be silently ignored. The profile path must follow the format [org/gnome/terminal/legacy/profiles:/:UUID] where UUID is a valid UUID string (e.g., generated with uuidgen), and both default and list values must reference this UUID.
Proposed fix (example with generated UUID)
-[org/gnome/terminal/legacy/profiles:/:cortex]
+[org/gnome/terminal/legacy/profiles:/:b1dcc9dd-5262-4d8d-a863-c897e6d979b9]
audible-bell=false
...
visible-name='Cortex'
[org/gnome/terminal/legacy/profiles:]
-default='cortex'
-list=['cortex']
+default='b1dcc9dd-5262-4d8d-a863-c897e6d979b9'
+list=['b1dcc9dd-5262-4d8d-a863-c897e6d979b9']🤖 Prompt for AI Agents
In @iso/live-build/config/hooks/live/99-configure-gnome.hook.chroot around lines
98 - 124, The GNOME Terminal profile block uses an invalid literal name
"cortex"; replace the profile path [org/gnome/terminal/legacy/profiles:/:cortex]
with a valid UUID-based path (e.g.,
[org/gnome/terminal/legacy/profiles:/:<UUID>]) and update default='cortex' and
list=['cortex'] to reference that same UUID string; keep visible-name='Cortex'
unchanged and generate the UUID with uuidgen (or similar) so all three places
(profile section name, default, and list) use the identical UUID.
| session optional pam_motd.so motd=/run/motd.dynamic | ||
| session optional pam_motd.so noupdate | ||
| session optional pam_mail.so standard | ||
| session required pam_env.so user_readenv=1 envfile=/etc/default/locale |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
pam_env user_readenv deprecated security vulnerability
💡 Result:
Short answer: Yes — pam_env's user_readenv is deprecated because it’s insecure, and its use has been linked to real vulnerabilities (e.g. a DoS in cockpit and local privilege-escalation issues). Mitigation: remove/disable user_readenv (set user_readenv=0 or remove the option) or upgrade to a Linux-PAM release where it’s disabled by default (>=1.4.0) — the option was marked deprecated in 1.5.0 and planned for removal. Sources and details below.
Key references
- CVE-2024-6126 (cockpit): authenticated users can kill processes when pam_env user_readenv is enabled — disable user_readenv to mitigate. [1]
- CVE-2025-6018 / advisories: LPE/vector where pam_env reading user ~/.pam_environment (user_readenv) enabled by default on some distros; vendors recommend setting user_readenv=0 and note it is deprecated and removed in newer PAM. [2][3]
- pam_env manpage / upstream: documents user_readenv, notes default-off in many builds and that the functionality is deprecated since PAM 1.5.0. [4]
Recommended actions (pick all that apply for your environment)
- Check PAM config files in /etc/pam.d/ (and any packaged pambase/pambase-like configs) for pam_env.so user_readenv=1 and remove that option or set user_readenv=0. [4]
- Upgrade Linux-PAM to a version where user_readenv is disabled by default (>=1.4.0) or to a release that removed the feature. [3][4]
- Apply vendor/security advisories and patches for affected packages (e.g., cockpit, distro packages) and follow distro-specific guidance. [1][2]
- If you must allow user environment reading for specific services, evaluate the PAM stack ordering (avoid calling pam_env before modules like pam_systemd/polkit) and restrict which services invoke pam_env. [2]
If you want, tell me your distribution and I’ll list exactly which PAM files to check and give the precise edits/commands.
Sources:
[1] NVD / CVE-2024-6126 (cockpit DoS).
[2] Wiz / CVE-2025-6018 advisory (pam_env user_readenv LPE analysis).
[3] Snyk advisory and recommendations (workaround: set user_readenv=0; deprecated in 1.5.0).
[4] pam_env manpage (notes user_readenv deprecated since 1.5.0).
Remove user_readenv=1 due to deprecated security vulnerability.
The user_readenv=1 option has been deprecated since PAM 1.5.0 and is linked to multiple security vulnerabilities:
- CVE-2024-6126: Authenticated users can kill arbitrary processes when
user_readenvis enabled - CVE-2025-6018: Local privilege escalation vector through user-controlled
~/.pam_environmentfile manipulation
Security advisories from Wiz, Snyk, and Linux-PAM upstream recommend removing this option (or setting user_readenv=0). Modern PAM versions (>=1.4.0) disable it by default.
Proposed fix
-session required pam_env.so user_readenv=1 envfile=/etc/default/locale
+session required pam_env.so envfile=/etc/default/locale📝 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.
| session required pam_env.so user_readenv=1 envfile=/etc/default/locale | |
| session required pam_env.so envfile=/etc/default/locale |
🤖 Prompt for AI Agents
In @iso/live-build/config/includes.chroot/etc/pam.d/login at line 16, Remove the
deprecated and insecure user_readenv=1 option from the pam_env.so session line:
locate the line that invokes pam_env.so (currently "session required pam_env.so
user_readenv=1 envfile=/etc/default/locale") and either drop the user_readenv=1
entirely or change it to user_readenv=0 so the entry reads "session required
pam_env.so envfile=/etc/default/locale" (or with user_readenv=0) to mitigate the
CVE issues.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot`:
- Around line 92-97: The current post-check using "$?" after running
plymouth-set-default-theme is ineffective under set -e because the script will
exit on failure; replace the two-step run+exit-code-check with a non-fatal
failure idiom so failures emit a warning but do not abort: run
plymouth-set-default-theme cortex with a failure-safe construct (e.g., a logical
OR fallback or an if ! ...; then ...; fi) so that plymouth-set-default-theme can
fail without causing immediate exit and the warning is printed (refer to the
plymouth-set-default-theme invocation and the surrounding if command -v ...
block).
- Around line 399-411: Replace the current "run command then check $? later"
pattern for update-initramfs: either run the command inside a conditional (if
update-initramfs -u -k all; then ... else echo "ERROR: Failed to update
initramfs" ; exit 1; fi) or immediately capture its exit code (rc=$? right after
update-initramfs) before any potential early-exit behavior; update the echo/exit
logic around the update-initramfs invocation and remove the delayed if [ $? -eq
0 ] check so the failure message and exit happen reliably.
In `@scripts/build.sh`:
- Around line 742-748: The unquoted glob in the chmod call can break on
filenames with spaces or if the glob expands to nothing; update the post-copy
permission step (the block that calls copy_glob_if_exists and then chmod on
"${pkg_dir}/etc/update-motd.d/"*) to iterate safely over matched files or use a
safe find/exec pattern: ensure you check for existence before chmod and quote
each filename when changing permissions (refer to the copy_glob_if_exists
invocation and the chmod invocation) so filenames with spaces are handled and
the case of no matches does not cause an unintended chmod argument.
♻️ Duplicate comments (9)
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (3)
9-9: Consider using stricter bash settings.Using
set -euo pipefailinstead of justset -ewould catch unset variable references and pipeline failures, making the script more robust.-set -e +set -euo pipefail
27-30: Domain inconsistency across branding files persists.These URLs use
cortex.io, but line 334 (MOTD footer) referencescortexlinux.comanddocs.cortexlinux.com. Standardize on one canonical domain for consistency.
127-130: Bug: GRUB_DISTRIBUTOR will not be appended if missing.The
sedcommand returns exit code 0 even when the pattern isn't matched (no substitutions made). The|| echofallback will never execute. This issue was flagged in previous reviews.Apply the same fix pattern used for
GRUB_THEMEon lines 121-125:# Also set timeout and other branding options - sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub || \ - echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub + if grep -q "^GRUB_DISTRIBUTOR=" /etc/default/grub; then + sed -i 's|^GRUB_DISTRIBUTOR=.*|GRUB_DISTRIBUTOR="Cortex Linux"|' /etc/default/grub + else + echo 'GRUB_DISTRIBUTOR="Cortex Linux"' >> /etc/default/grub + fiscripts/build.sh (6)
63-72: Unquoted glob and missing-rflag remain unaddressed.This was flagged in a prior review. The unquoted glob can break on filenames with spaces, and
cpwithout-rwill fail if directories are matched.
136-140:dpkg-debstill treated as optional despite being required forbranding-package.This was flagged in a prior review.
cmd_branding_packageat line 759 callsdpkg-deb --buildwhich will fail if the tool is missing.
287-304: Arithmetic expansion of$?after||still captures wrong value.This was flagged in a prior review. In
func || errors=$((errors + $?)), the$?captures the exit status of the||short-circuit (always 0 when the assignment runs), not the function's return value.
549-558: Unconditionalsudo lb buildstill present.This was flagged in a prior review. Running
sudowhen already root fails in many CI environments. Use conditional sudo based on$EUID.
615-625: Unconditionalsudo lb cleanhas the same issue.This was flagged in a prior review alongside
sudo lb build. Both need conditional sudo based on$EUID.
695-704: Unconditionalchmodonpostinstandprermstill present.This was flagged in a prior review. If either file is missing from the source
DEBIAN/directory, the script will fail.
🧹 Nitpick comments (1)
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (1)
263-264: Consider PCRE availability.
grep -oP(Perl-Compatible Regular Expressions) may not be available on all systems. If busybox grep or older GNU grep is present without PCRE support, these lines will fail silently (stderr redirected).Consider using extended regex (
grep -oE) or basic pattern matching as a fallback:- UPDATES=$(grep -oP '\d+ updates' "$UPDATES_FILE" 2>/dev/null | grep -oP '\d+' || echo "0") - SECURITY=$(grep -oP '\d+ security' "$UPDATES_FILE" 2>/dev/null | grep -oP '\d+' || echo "0") + UPDATES=$(grep -oE '[0-9]+ updates' "$UPDATES_FILE" 2>/dev/null | grep -oE '[0-9]+' || echo "0") + SECURITY=$(grep -oE '[0-9]+ security' "$UPDATES_FILE" 2>/dev/null | grep -oE '[0-9]+' || echo "0")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
iso/live-build/config/hooks/live/50-cortex-branding.hook.chrootscripts/build.sh
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-12T20:51:13.828Z
Learnt from: CR
Repo: cortexlinux/cortex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-12T20:51:13.828Z
Learning: Applies to **/*.py : Do not use silent sudo - require explicit user confirmation for privilege escalation
Applied to files:
scripts/build.sh
🔇 Additional comments (9)
iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot (4)
49-61: LGTM!The APT repository configuration is appropriately commented out with a clear TODO, avoiding the previously flagged duplicate repository issue.
137-149: LGTM!Wallpaper verification with appropriate warnings for missing assets.
151-177: LGTM!GDM branding configuration is well-guarded with directory and command existence checks.
343-392: LGTM!Neofetch configuration with custom ASCII art is well-structured.
scripts/build.sh (5)
7-47: LGTM: Clean configuration and logging setup.The use of
set -euo pipefailfor strict error handling and the modular logging functions are well-structured.
499-526: LGTM: Configurable compression and well-structured live-build options.The
SQUASHFS_COMPvariable for fast CI builds (lz4) vs smaller release builds (xz) is a good pattern. The boot parameters and live-build configuration are comprehensive.
320-420: LGTM: Comprehensive test suite with clear pass/fail reporting.The test function covers key build artifacts (preseed, provisioning, branding, hooks, package controls) with proper syntax validation. The
[ -f "$hook" ]check handles the glob edge case when no files match.
816-857: LGTM: Clean command dispatch pattern.The entry point is well-structured with a clear case-based dispatch, proper help aliases, and argument passthrough to subcommands.
470-497: LGTM: Good validation for required GRUB theme file.The explicit check for
theme.txtwith a clear error message (lines 483-489) addresses the branding validation concern from prior reviews.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if command -v plymouth-set-default-theme &>/dev/null; then | ||
| plymouth-set-default-theme cortex | ||
| if [ $? -ne 0 ]; then | ||
| echo " WARNING: plymouth-set-default-theme failed" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Ineffective exit code check due to set -e.
With set -e active, if plymouth-set-default-theme cortex fails on line 93, the script exits immediately before line 94 executes. The if [ $? -ne 0 ] check is dead code.
If you want to warn and continue on failure:
if command -v plymouth-set-default-theme &>/dev/null; then
- plymouth-set-default-theme cortex
- if [ $? -ne 0 ]; then
- echo " WARNING: plymouth-set-default-theme failed"
- fi
+ if ! plymouth-set-default-theme cortex; then
+ echo " WARNING: plymouth-set-default-theme failed"
+ fi
fi🤖 Prompt for AI Agents
In `@iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot` around lines
92 - 97, The current post-check using "$?" after running
plymouth-set-default-theme is ineffective under set -e because the script will
exit on failure; replace the two-step run+exit-code-check with a non-fatal
failure idiom so failures emit a warning but do not abort: run
plymouth-set-default-theme cortex with a failure-safe construct (e.g., a logical
OR fallback or an if ! ...; then ...; fi) so that plymouth-set-default-theme can
fail without causing immediate exit and the warning is printed (refer to the
plymouth-set-default-theme invocation and the surrounding if command -v ...
block).
| if command -v update-initramfs &>/dev/null; then | ||
| # Update ALL kernels | ||
| update-initramfs -u -k all | ||
|
|
||
| if [ $? -eq 0 ]; then | ||
| echo " Initramfs updated successfully" | ||
| else | ||
| echo " ERROR: Failed to update initramfs" | ||
| exit 1 | ||
| fi | ||
| else | ||
| echo " WARNING: update-initramfs command not found" | ||
| fi |
There was a problem hiding this comment.
Ineffective exit code check pattern.
Same issue as the Plymouth section: with set -e, if update-initramfs -u -k all fails, the script exits before the if [ $? -eq 0 ] check. The error message on line 406-407 will never be displayed.
If you want custom error handling:
if command -v update-initramfs &>/dev/null; then
# Update ALL kernels
- update-initramfs -u -k all
-
- if [ $? -eq 0 ]; then
+ if update-initramfs -u -k all; then
echo " Initramfs updated successfully"
else
echo " ERROR: Failed to update initramfs"
exit 1
fi
else
echo " WARNING: update-initramfs command not found"
fi🤖 Prompt for AI Agents
In `@iso/live-build/config/hooks/live/50-cortex-branding.hook.chroot` around lines
399 - 411, Replace the current "run command then check $? later" pattern for
update-initramfs: either run the command inside a conditional (if
update-initramfs -u -k all; then ... else echo "ERROR: Failed to update
initramfs" ; exit 1; fi) or immediately capture its exit code (rc=$? right after
update-initramfs) before any potential early-exit behavior; update the echo/exit
logic around the update-initramfs invocation and remove the delayed if [ $? -eq
0 ] check so the failure message and exit happen reliably.
| # Copy MOTD scripts | ||
| log "Copying MOTD scripts..." | ||
| if [ -d "${BRANDING_DIR}/motd" ]; then | ||
| if copy_glob_if_exists "${BRANDING_DIR}/motd/*" "${pkg_dir}/etc/update-motd.d/"; then | ||
| chmod 755 "${pkg_dir}/etc/update-motd.d/"* | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Unquoted glob in chmod can break on filenames with spaces or empty matches.
Line 746 uses an unquoted glob "${pkg_dir}/etc/update-motd.d/"* which will fail if filenames contain spaces or if no files were copied.
🔧 Proposed fix using a loop or find
# Copy MOTD scripts
log "Copying MOTD scripts..."
if [ -d "${BRANDING_DIR}/motd" ]; then
if copy_glob_if_exists "${BRANDING_DIR}/motd/*" "${pkg_dir}/etc/update-motd.d/"; then
- chmod 755 "${pkg_dir}/etc/update-motd.d/"*
+ find "${pkg_dir}/etc/update-motd.d/" -type f -exec chmod 755 {} +
fi
fi🤖 Prompt for AI Agents
In `@scripts/build.sh` around lines 742 - 748, The unquoted glob in the chmod call
can break on filenames with spaces or if the glob expands to nothing; update the
post-copy permission step (the block that calls copy_glob_if_exists and then
chmod on "${pkg_dir}/etc/update-motd.d/"*) to iterate safely over matched files
or use a safe find/exec pattern: ensure you check for existence before chmod and
quote each filename when changing permissions (refer to the copy_glob_if_exists
invocation and the chmod invocation) so filenames with spaces are handled and
the case of no matches does not cause an unintended chmod argument.
Screen.Recording.2026-01-14.at.5.33.35.PM.mov
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.