Manual merge jammy2noble#641
Merged
Merged
Conversation
CentOS stemcells are no longer built, so this drops dead stages, assets, and code paths. Also removes the default_su_directive override (already removed on ubuntu-noble), which silently overrode the correct 'su root syslog' set globally in ubuntu-logrotate.conf.
Explicitly pass: - GEM_HOME - BUILD_TIME - UBUNTU_ADVANTAGE_TOKEN - UBUNTU_FIPS_USE_IAAS_KERNEL Fixes: ``` sudo: preserving the entire environment is not supported, '--preserve-env' is ignored ubuntu@21f652d6-b208-4386-b0c0-097e26578228:/tmp/build/44575cf5$ set -e ubuntu@21f652d6-b208-4386-b0c0-097e26578228:/tmp/build/44575cf5$ ubuntu@21f652d6-b208-4386-b0c0-097e26578228:/tmp/build/44575cf5$ cd "/tmp/build/44575cf5/bosh-linux-stemcell-builder" ubuntu@21f652d6-b208-4386-b0c0-097e26578228:/tmp/build/44575cf5/bosh-linux-stemcell-builder$ bundle install Bundler 4.0.13 is running, but your lockfile was generated with 2.5.23. Installing Bundler 2.5.23 and restarting using that version. Fetching gem metadata from https://rubygems.org/. Fetching bundler 2.5.23 Retrying download gem from https://rubygems.org/ due to error (2/4): Bundler::PermissionError There was an error while trying to write to `/usr/local/lib/ruby/gems/3.3.0/cache/bundler-2.5.23.gem`. It is likely that you need to grant write permissions for that path. ``` ^ https://bosh.ci.cloudfoundry.org/teams/stemcell/pipelines/ubuntu-resolute-builder/jobs/build-os-image/builds/1#L6a052874:4:13
Remove CentOS-specific code and assets
…VolumePattern in aws_spec.rb
Revert "Harden monit-access-helper.sh cgroupv2 mount point detection"
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR updates the stemcell build and test configuration from Ubuntu Jammy to Ubuntu Noble. It changes CI variables, workflow references, documentation, package lists, boot and image stages, and multiple OS/image specs. It also replaces several legacy runit, softlayer, vcloud, and IPv6-disabling paths with systemd-based, EFI-oriented, and Noble-specific configuration. Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
|
Member
Author
|
/easycla |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR appears to be a merge-forward alignment update that refreshes the Jammy base OS metalink and brings along related stemcell/test/doc changes (AWS agent settings expectations, Alicloud infrastructure expectations, openSUSE service detection, and PR template updates).
Changes:
- Bump
image-metalinks/ubuntu-jammyartifacts to1088.0.0with updated hashes/sizes and publish timestamp. - Add/adjust stemcell RSpec coverage for AWS agent settings JSON and Alicloud infrastructure properties.
- Broaden service distro detection to allow openSUSE and update the PR merge-forward template.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
stemcell_builder/stages/image_install_grub/apply.sh |
Adds a variable guide comment block for the grub install stage. |
image-metalinks/ubuntu-jammy/ubuntu-jammy.meta4 |
Updates Jammy metalink hashes/sizes and version to 1088.0.0. |
bosh-stemcell/spec/stemcells/aws_spec.rb |
Adds expectations for /var/vcap/bosh/agent.json AWS platform storage patterns. |
bosh-stemcell/spec/bosh/stemcell/infrastructure_spec.rb |
Adds Alicloud infrastructure expectations (but currently duplicated). |
bosh-stemcell/lib/shellout_types/service.rb |
Allows openSUSE detection when checking whether a service is enabled. |
.github/pull_request_template.md |
Updates merge-forward workflow instructions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aramprice
force-pushed
the
manual-merge-jammy2noble
branch
from
2f94914 to
c423fe5
Compare
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stemcell_builder/stages/image_create/apply.sh (1)
15-16:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftPartition layout no longer matches downstream GRUB stage contract.
Lines 15-16 still create a single partition, but
stemcell_builder/stages/image_install_grub/apply.shnow treats EFI and root as separate partitions (Line 20/21 there), mounts/boot/efi, and writesvfatfstab for EFI. This mismatch can produce non-bootable images or fail GRUB EFI installation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/image_create/apply.sh` around lines 15 - 16, The image_create partitioning must be changed to produce two partitions (EFI vfat and root) matching the downstream GRUB stage: switch the disk label from msdos to gpt, create an EFI partition (FAT32/vfat) as the first partition and a root partition (ext2/ext4) as the second; update the parted invocations that currently reference ${disk_image} and $part_offset/$part_size to use mklabel gpt, mkpart primary fat32 <efi_start> <efi_end> and mkpart primary ext2 <root_start> 100%, ensuring the EFI partition is the first partition and will be formatted vfat so the later image_install_grub/apply.sh mount and fstab entries (which expect /boot/efi and vfat) align with these partition numbers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/sbom.yml:
- Around line 19-20: Replace the floating reference
aquasecurity/trivy-action@master with an immutable, audited revision (e.g., a
specific commit SHA or a fixed release tag) wherever it appears in the workflow;
update each usage of aquasecurity/trivy-action@master so the workflow becomes
deterministic and reproducible by using the same pinned SHA/tag for all steps
that call the trivy action.
In `@bosh-stemcell/spec/bosh/stemcell/infrastructure_spec.rb`:
- Around line 95-111: Remove the duplicated Infrastructure::Alicloud spec block:
delete the entire describe Infrastructure::Alicloud example group that starts
with its(:name) { should eq("alicloud") } and the it "has alicloud specific
additional cloud properties" block (which asserts
subject.additional_cloud_properties), leaving the later, non-duplicated describe
Infrastructure::Alicloud block intact so tests run once and maintenance is
reduced.
In `@bosh-stemcell/spec/support/stemcell_image.rb`:
- Around line 15-17: The teardown currently calls
ShelloutTypes::Chroot.unmount_proc then disk_image.unmount sequentially so if
unmount_proc raises the disk_image.unmount is skipped; change the
config.after(:suite) block to guarantee disk_image.unmount always runs by moving
disk_image.unmount into an ensure (or separate rescue/ensure) so it executes
regardless of errors from ShelloutTypes::Chroot.unmount_proc, and optionally
rescue/log errors from each call (referencing the config.after(:suite) block,
ShelloutTypes::Chroot.unmount_proc, and disk_image.unmount).
In `@ci/docker/os-image-stemcell-builder/Dockerfile`:
- Line 70: The Dockerfile appends to /etc/sudoers and later runs chown
ubuntu:ubuntu but never creates the ubuntu user/group; add creation of the
ubuntu group and user before the line that echoes into /etc/sudoers (e.g., run
groupadd/create group and useradd/adduser -m -s /bin/bash -g ubuntu ubuntu or
equivalent), ensure the user home is created and shell set, then append the
sudoers line and keep the existing chown ubuntu:ubuntu references so they
succeed; update any USER or ownership steps that assume that UID/GID if you need
a specific numeric ID.
In `@docs/noble-dev.md`:
- Around line 2-23: The Noble notes contain incorrect branch and file path
references and typos that break navigation; update the header and every
referenced path/text to match the repository exactly (fix "ubunty-noble/master"
→ "ubuntu-noble/master" or the actual branch name), correct typos and filenames
and ensure they match real files: verify and fix
"stemcell_builder/stages/base-ubuntu-package/apply.sh" (libpam package note),
"stemcell_builder/stages/password_policies/apply.sh" (patching common-password),
"bosh-stemcell/spec/os-image/ubuntu_noble.spec" (ordering/gnats/rescan-scsi-bus
notes), and test paths "src/ipv4director/auditd/smoke_test.go" and
"src/ipv4director/ipv6basic/basic_test.go"; also correct misspellings
("availble"→"available", "adiscon" if incorrect,
"passowrd_policies"→"password_policies", "paching"→"patching",
"autitd"→"auditd") and verify any repository links point to the actual
branch/file so navigation and debugging work.
In `@scripts/stemcell-repack-debugger.sh`:
- Around line 78-80: The check for stemcell_tgz is using an unquoted expansion
and only prints a message without exiting, which can cause later commands like
tar xvf $stemcell_tgz to run with an empty variable; update the conditional to
use a safe, quoted parameter expansion and fail-fast: test with if [ -z
"${stemcell_tgz:-}" ] then echo the suggested export using
"${example_stemcell_tgz_path}" and exit 1 (or set
stemcell_tgz="${example_stemcell_tgz_path}" if you want a default), and ensure
all later uses (e.g., tar xvf "$stemcell_tgz") quote the variable.
- Around line 131-136: The heredoc inside scripts/stemcell-repack-debugger.sh
interpolates BOSH_DEBUG_PUB_KEY into the chrooted bash (echo $BOSH_DEBUG_PUB_KEY
>> .ssh/authorized_keys) which allows shell injection; change the flow so the
key is written as data rather than executed—e.g., stop passing the raw variable
through the unquoted heredoc and instead write the key with a safe data write
(for example using printf '%s\n' "$BOSH_DEBUG_PUB_KEY" piped to tee or
redirecting a safely quoted heredoc) into the target file under
$mnt_dir/home/bosh_debug/.ssh/authorized_keys before or without interpolating it
inside chroot; update the section around the chroot/useradd/echo lines to
perform a non-interpolating write so the inner shell never interprets
BOSH_DEBUG_PUB_KEY.
- Around line 44-70: The argument parsing loop is broken because it iterates
over for arg in "$@" while also shifting positional parameters, causing wrong
values for convert_cert_file and brittle shifts; change to a standard while [
"$#" -gt 0 ]; do case "$1" in ... esac; done pattern so shifts affect the
current parameter correctly, update branches that consume a value (e.g.,
--debug_pub_key) to read the value from "$2" only after matching "$1" and then
shift 2, keep boolean flags like bump_version and logic that reads OUT/version
(using version=$(bosh int <(tar xf "$OUT" stemcell.MF --occurrence=1 -O) --path
/version) or fallback from "$1" value) but adjust to use "$1" or extracted value
consistently, and preserve show_help and --debug handling in the new while-loop.
- Around line 99-103: The EXIT trap currently only deletes the temp directory
(mnt_dir) and can leave the mountpoint and kpartx device mapping behind; update
the trap setup (the traps variable and the trap "${traps}" EXIT invocation) to
also attempt to unmount mnt_dir (using sudo umount -l or sudo umount if
preferred) and to remove the kpartx mapping (sudo kpartx -dv for $device or by
passing $DISK_NAME) guarded by checks that $mnt_dir is mounted and $device (or
/dev/mapper/$device) exists; ensure these commands are appended to the traps
string before trap is invoked so they run on any EXIT and handle empty/unset
$device safely.
In `@stemcell_builder/stages/base_warden/apply.sh`:
- Around line 51-74: The current rosetta_services array and loop unconditionally
install systemd drop-in overrides, weakening hardening for all images; change
this to run only when an explicit opt-in is set (e.g. check an env var like
ENABLE_ROSETTA_COMPAT or call a detection function like is_rosetta_emulation())
before defining rosetta_services and executing the for-loop; ensure the guard
returns false by default, add a clear log message when opt-in is enabled, and
leave the rosetta_services variable and the mkdir/cp actions (the loop that
references rosetta_services and the cp of rosetta-compat.conf) unchanged except
for being wrapped in that conditional.
In `@stemcell_builder/stages/bosh_alicloud_agent_settings/assets/agent.md`:
- Around line 15-16: Update the replacement example in agent.md so it matches
the configured DiskIDTransformPattern/DiskIDTransformReplacement behavior:
ensure the DiskIDTransformReplacement uses the captured group from
DiskIDTransformPattern (the suffix like gw8ibkvqdrmdqzwpo4rk) or otherwise
preserves that suffix instead of swapping to an unrelated string (e.g.,
gw8a1...), and make the example output show the transformed value that results
from applying DiskIDTransformReplacement to the two captured groups declared in
DiskIDTransformPattern.
In `@stemcell_builder/stages/bosh_aws_agent_settings/apply.sh`:
- Line 6: The cp command uses unquoted variable expansions which can cause
word-splitting/globbing; update the cp invocation to quote both source and
destination expansions (e.g., use "$assets_dir/agent.json" and
"$chroot/var/vcap/bosh/agent.json") so that the command in apply.sh reliably
copies the file even if variables contain spaces or glob characters; apply the
same quoting pattern to the corresponding lines that reference assets_dir and
chroot in bosh_azure_agent_settings/apply.sh and
bosh_cloudstack_agent_settings/apply.sh.
In `@stemcell_builder/stages/bosh_google_agent_settings/assets/agent.md`:
- Around line 45-61: The fenced code block under the /dev/disk/by-id example is
malformed: the line starting with the nvme identifier
(nvme.1d0f-766f6c3063363764333330306239326665383531-416d617a6f6e20456c617374696320426c6f636b2053746f7265-00000001-part1)
was placed outside/inside backticks incorrectly and breaks rendering; fix it by
making the fenced block consistent — remove the stray backticks before that nvme
line and ensure the block begins with ``` and ends with ``` with the nvme
identifier (and the other device names) inside the fence so the /dev/disk/by-id
example renders as a single valid code block.
In `@stemcell_builder/stages/bosh_openstack_agent_settings/apply.sh`:
- Line 6: The cp command in bosh_openstack_agent_settings apply.sh uses unquoted
expansions which can lead to word-splitting/globbing; update cp to use quoted
paths by changing references to "$assets_dir/agent.json" and
"$chroot/var/vcap/bosh/agent.json". Also ensure any computed base_dir expression
used to source prelude files is quoted (use source "$base_dir"/... ) so base_dir
with spaces works safely; apply the same quoting pattern in the other stage
scripts mentioned (bosh_systemd_resolved/apply.sh and
bosh_vsphere_agent_settings/apply.sh) for consistency.
In `@stemcell_builder/stages/bosh_openstack_agent_settings/assets/agent.md`:
- Around line 1-58: Fix markdownlint warnings in agent.md: add blank lines
before and after headings like "Introduction", "Disk ID transform rules", "what
it does", and "related information" to address MD022; ensure fenced code blocks
(the JSON block containing "DiskIDTransformPattern"/"DiskIDTransformReplacement"
and the persistent_disk_hints.json block and the /dev/disk/by-id listing) are
each surrounded by a blank line to fix MD031; add fence languages (use "json"
for the JSON blocks and "text" or nothing specific for the /dev/disk/by-id list)
to resolve MD040; remove trailing whitespace and ensure the file ends with
exactly one newline to satisfy MD047.
- Line 16: Update the transformed disk-id example so it matches the documented
replacement rule: replace the incorrect `scsi-305c185c6e8a34216ab301f5cf7d17f0d`
with the correct `scsi-05c185c6e8a34216ab301f5cf7d17f0d` (the output of the
DiskIDTransformReplacement using `scsi-${1}${2}${3}${4}${5}`), i.e., edit the
example in agent.md to show the expected `scsi-05c...` value.
In
`@stemcell_builder/stages/dev_tools_config/assets/generate_dev_tools_file_list.sh`:
- Around line 31-34: The loop that checks each package uses the variable
package_name and runs dpkg-query unconditionally, but when a package is missing
the script echoes "$package_name is NOT installed." then falls through to
dpkg-query (and with set -e the script aborts); modify the loop in
generate_dev_tools_file_list.sh so that after the missing-package detection (the
dpkg -s check that prints the NOT installed message) you immediately continue to
the next iteration (add a continue) to skip running dpkg-query -L for absent
packages.
In `@stemcell_builder/stages/image_create/apply.sh`:
- Around line 29-33: The loop that precreates loop devices using mknod stops on
the first failure (in the for ... seq 0 64 block), so if /dev/loop0 already
exists the script breaks and never attempts to create higher-numbered
/dev/loop$i nodes; update the loop to skip existing devices and continue on
non-fatal errors: for each iteration check if /dev/loop$i already exists and if
so continue, otherwise attempt mknod and on failure log/continue instead of
break so the loop tries all indices (refer to the for loop using seq, the
/dev/loop$i path and the mknod call).
In `@stemcell_builder/stages/image_install_grub/apply.sh`:
- Around line 20-21: The current code runs kpartx -sav ${device} twice to set
device_partition_efi and device_partition_root which re-adds mappings and is
brittle; instead run kpartx -sav ${device} a single time, capture its output
into a variable (or a temp file), then parse that single output to extract both
device_partition_efi and device_partition_root (e.g., by selecting the first and
last mapped device names from the captured output) and use those parsed values
when assigning device_partition_efi and device_partition_root.
---
Outside diff comments:
In `@stemcell_builder/stages/image_create/apply.sh`:
- Around line 15-16: The image_create partitioning must be changed to produce
two partitions (EFI vfat and root) matching the downstream GRUB stage: switch
the disk label from msdos to gpt, create an EFI partition (FAT32/vfat) as the
first partition and a root partition (ext2/ext4) as the second; update the
parted invocations that currently reference ${disk_image} and
$part_offset/$part_size to use mklabel gpt, mkpart primary fat32 <efi_start>
<efi_end> and mkpart primary ext2 <root_start> 100%, ensuring the EFI partition
is the first partition and will be formatted vfat so the later
image_install_grub/apply.sh mount and fstab entries (which expect /boot/efi and
vfat) align with these partition numbers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b691170f-6282-4952-b298-337ea069da37
⛔ Files ignored due to path filters (1)
stemcell_builder/stages/bosh_monit/assets/monit-5.29.0.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (148)
.envrc.github/workflows/sbom.ymlREADME.mdacceptance-tests/ipv4director/auditd/smoke_test.goacceptance-tests/ipv4director/ipv6basic/basic_test.goacceptance-tests/ipv4director/ipv6basic/manifest.ymlacceptance-tests/ipv4director/smoke/smoke_test.goacceptance-tests/ipv6director/ipv6full/full_test.gobosh-stemcell/lib/bosh/stemcell/build_environment.rbbosh-stemcell/lib/bosh/stemcell/builder_options.rbbosh-stemcell/lib/bosh/stemcell/definition.rbbosh-stemcell/lib/bosh/stemcell/infrastructure.rbbosh-stemcell/lib/bosh/stemcell/stage_collection.rbbosh-stemcell/spec/assets/dpkg-list-ubuntu-azure-additions.txtbosh-stemcell/spec/assets/dpkg-list-ubuntu-kernel.txtbosh-stemcell/spec/assets/dpkg-list-ubuntu-softlayer-additions.txtbosh-stemcell/spec/assets/dpkg-list-ubuntu-vsphere-additions.txtbosh-stemcell/spec/assets/dpkg-list-ubuntu.txtbosh-stemcell/spec/bosh/stemcell/builder_options_spec.rbbosh-stemcell/spec/bosh/stemcell/definition_spec.rbbosh-stemcell/spec/bosh/stemcell/infrastructure_spec.rbbosh-stemcell/spec/bosh/stemcell/stage_collection_spec.rbbosh-stemcell/spec/bosh/stemcell/stemcell_packager_spec.rbbosh-stemcell/spec/os_image/ubuntu_spec.rbbosh-stemcell/spec/spec_helper.rbbosh-stemcell/spec/stemcells/alicloud_spec.rbbosh-stemcell/spec/stemcells/aws_spec.rbbosh-stemcell/spec/stemcells/azure_spec.rbbosh-stemcell/spec/stemcells/cis_spec.rbbosh-stemcell/spec/stemcells/fips_spec.rbbosh-stemcell/spec/stemcells/google_spec.rbbosh-stemcell/spec/stemcells/openstack_spec.rbbosh-stemcell/spec/stemcells/softlayer_spec.rbbosh-stemcell/spec/stemcells/stig_spec.rbbosh-stemcell/spec/stemcells/ubuntu_spec.rbbosh-stemcell/spec/stemcells/vcloud_spec.rbbosh-stemcell/spec/stemcells/vsphere_spec.rbbosh-stemcell/spec/stemcells/warden_spec.rbbosh-stemcell/spec/support/os_image_chrony_shared_examples.rbbosh-stemcell/spec/support/os_image_linux_kernel_modules_shared_examples.rbbosh-stemcell/spec/support/os_image_linux_kernel_shared_examples.rbbosh-stemcell/spec/support/os_image_shared_examples.rbbosh-stemcell/spec/support/os_image_systemd_shared_examples.rbbosh-stemcell/spec/support/stemcell_image.rbbosh-stemcell/spec/support/stemcell_shared_examples.rbci/configure.shci/docker/os-image-stemcell-builder/Dockerfileci/docker/os-image-stemcell-builder/Vagrantfileci/pipelines/builder.ymlci/pipelines/vars.ymldocs/noble-dev.mdimage-metalinks/ubuntu-noble/ubuntu-noble.meta4scripts/repack-helpers/convert-raw-to-vmdk.shscripts/stemcell-repack-debugger.shstemcell_builder/lib/prelude_agent.bashstemcell_builder/stages/base_apt/apply.shstemcell_builder/stages/base_apt/assets/etc/apt/apt.conf.d/50unattended-upgradesstemcell_builder/stages/base_ssh/apply.shstemcell_builder/stages/base_ssh/assets/10-ssh-firstboot-done.confstemcell_builder/stages/base_ubuntu_build_essential/apply.shstemcell_builder/stages/base_ubuntu_firstboot/apply.shstemcell_builder/stages/base_ubuntu_firstboot/assets/etc/rc.localstemcell_builder/stages/base_ubuntu_firstboot/assets/etc/systemd/system/firstboot.servicestemcell_builder/stages/base_ubuntu_firstboot/assets/root/firstboot.shstemcell_builder/stages/base_ubuntu_packages/apply.shstemcell_builder/stages/base_ubuntu_packages/assets/apparmor-local-sbin.dhclientstemcell_builder/stages/base_ubuntu_packages/assets/runit.servicestemcell_builder/stages/base_warden/apply.shstemcell_builder/stages/base_warden/assets/rosetta-compat.confstemcell_builder/stages/base_warden/assets/runit/cron/runstemcell_builder/stages/base_warden/assets/runit/rsyslog/runstemcell_builder/stages/base_warden/assets/runit/ssh/runstemcell_builder/stages/base_warden/assets/runsvdir-startstemcell_builder/stages/base_warden/assets/servicestemcell_builder/stages/base_warden/assets/unsharestemcell_builder/stages/bosh_alicloud_agent_settings/apply.shstemcell_builder/stages/bosh_alicloud_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_alicloud_agent_settings/assets/agent.mdstemcell_builder/stages/bosh_aws_agent_settings/apply.shstemcell_builder/stages/bosh_aws_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_aws_agent_settings/assets/agent.mdstemcell_builder/stages/bosh_azure_agent_settings/apply.shstemcell_builder/stages/bosh_azure_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_cloudstack_agent_settings/apply.shstemcell_builder/stages/bosh_cloudstack_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_enable_password_authentication/apply.shstemcell_builder/stages/bosh_go_agent/apply.shstemcell_builder/stages/bosh_go_agent/assets/bosh-agent.servicestemcell_builder/stages/bosh_go_agent/assets/runit/agent/log/runstemcell_builder/stages/bosh_go_agent/assets/runit/agent/runstemcell_builder/stages/bosh_go_agent/assets/runit/monit/downstemcell_builder/stages/bosh_go_agent/assets/runit/monit/log/runstemcell_builder/stages/bosh_go_agent/assets/runit/monit/runstemcell_builder/stages/bosh_google_agent_settings/apply.shstemcell_builder/stages/bosh_google_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_google_agent_settings/assets/agent.mdstemcell_builder/stages/bosh_monit/apply.shstemcell_builder/stages/bosh_monit/assets/monitstemcell_builder/stages/bosh_monit/assets/monit-access-helper.shstemcell_builder/stages/bosh_monit/assets/monit.servicestemcell_builder/stages/bosh_monit/assets/restrict-monit-api-accessstemcell_builder/stages/bosh_ntp/apply.shstemcell_builder/stages/bosh_openstack_agent_settings/apply.shstemcell_builder/stages/bosh_openstack_agent_settings/assets/agent.jsonstemcell_builder/stages/bosh_openstack_agent_settings/assets/agent.mdstemcell_builder/stages/bosh_softlayer_agent_settings/apply.shstemcell_builder/stages/bosh_sysctl/assets/60-bosh-sysctl.confstemcell_builder/stages/bosh_systemd_resolved/apply.shstemcell_builder/stages/bosh_systemd_resolved/assets/add-container-listener-address.confstemcell_builder/stages/bosh_systemd_resolved/assets/create-systemd-resolved-listener-address.servicestemcell_builder/stages/bosh_vsphere_agent_settings/apply.shstemcell_builder/stages/bosh_vsphere_agent_settings/assets/agent.jsonstemcell_builder/stages/delay_monit_start/apply.shstemcell_builder/stages/dev_tools_config/assets/generate_dev_tools_file_list.shstemcell_builder/stages/image_create/apply.shstemcell_builder/stages/image_create_softlayer_two_partitions/apply.shstemcell_builder/stages/image_create_softlayer_two_partitions/config.shstemcell_builder/stages/image_install_grub/apply.shstemcell_builder/stages/image_install_grub_efi/apply.shstemcell_builder/stages/image_install_grub_efi/config.shstemcell_builder/stages/image_install_grub_softlayer_two_partitions/apply.shstemcell_builder/stages/image_install_grub_softlayer_two_partitions/config.shstemcell_builder/stages/logrotate_config/assets/logrotate.timerstemcell_builder/stages/logrotate_config/assets/opensuse-logrotate.confstemcell_builder/stages/password_policies/assets/opensuse/common-account.patchstemcell_builder/stages/password_policies/assets/opensuse/common-auth.patchstemcell_builder/stages/password_policies/assets/opensuse/common-password.patchstemcell_builder/stages/password_policies/assets/ubuntu/common-password.patchstemcell_builder/stages/rsyslog_config/apply.shstemcell_builder/stages/rsyslog_config/assets/rsyslog_90-bosh-agent.confstemcell_builder/stages/rsyslog_config/assets/rsyslog_logrotate.confstemcell_builder/stages/static_libraries_config/apply.shstemcell_builder/stages/static_libraries_config/assets/static_libraries_list.txtstemcell_builder/stages/system_azure_init/apply.shstemcell_builder/stages/system_azure_init/assets/etc/systemd/system/cloud-config.service.d/firstboot-blocker.confstemcell_builder/stages/system_azure_init/assets/etc/waagent/walinuxagent.servicestemcell_builder/stages/system_azure_network/apply.shstemcell_builder/stages/system_azure_network/assets/01-azure-sriov-unmanaged.networkstemcell_builder/stages/system_azure_network/assets/10-azure-sriov-unmanaged.rulesstemcell_builder/stages/system_google_packages/assets/instance_configs.cfg.templatestemcell_builder/stages/system_grub/apply.shstemcell_builder/stages/system_kernel_modules/apply.shstemcell_builder/stages/system_parameters/apply.shstemcell_builder/stages/system_softlayer_multipath_tools/apply.shstemcell_builder/stages/system_softlayer_multipath_tools/assets/etc/multipath.confstemcell_builder/stages/system_softlayer_netplan/apply.shstemcell_builder/stages/system_softlayer_open_iscsi/apply.shstemcell_builder/stages/udev_aws_rules/assets/nvme-id
💤 Files with no reviewable changes (51)
- stemcell_builder/stages/delay_monit_start/apply.sh
- stemcell_builder/stages/system_softlayer_netplan/apply.sh
- stemcell_builder/stages/image_install_grub_efi/config.sh
- stemcell_builder/stages/base_warden/assets/runsvdir-start
- stemcell_builder/stages/bosh_go_agent/assets/runit/agent/log/run
- bosh-stemcell/spec/stemcells/vcloud_spec.rb
- stemcell_builder/stages/base_warden/assets/runit/ssh/run
- stemcell_builder/stages/system_azure_network/assets/10-azure-sriov-unmanaged.rules
- stemcell_builder/stages/image_create_softlayer_two_partitions/apply.sh
- stemcell_builder/lib/prelude_agent.bash
- stemcell_builder/stages/bosh_softlayer_agent_settings/apply.sh
- stemcell_builder/stages/system_softlayer_multipath_tools/apply.sh
- stemcell_builder/stages/image_install_grub_efi/apply.sh
- stemcell_builder/stages/base_ubuntu_build_essential/apply.sh
- stemcell_builder/stages/base_warden/assets/runit/rsyslog/run
- stemcell_builder/stages/base_ubuntu_firstboot/assets/etc/rc.local
- stemcell_builder/stages/bosh_go_agent/assets/runit/agent/run
- stemcell_builder/stages/password_policies/assets/opensuse/common-auth.patch
- stemcell_builder/stages/bosh_go_agent/assets/runit/monit/run
- stemcell_builder/stages/base_apt/apply.sh
- stemcell_builder/stages/system_softlayer_open_iscsi/apply.sh
- stemcell_builder/stages/logrotate_config/assets/opensuse-logrotate.conf
- stemcell_builder/stages/bosh_ntp/apply.sh
- stemcell_builder/stages/system_azure_network/assets/01-azure-sriov-unmanaged.network
- bosh-stemcell/spec/assets/dpkg-list-ubuntu-softlayer-additions.txt
- stemcell_builder/stages/bosh_monit/assets/monit-access-helper.sh
- stemcell_builder/stages/base_apt/assets/etc/apt/apt.conf.d/50unattended-upgrades
- stemcell_builder/stages/bosh_monit/assets/restrict-monit-api-access
- bosh-stemcell/spec/bosh/stemcell/builder_options_spec.rb
- stemcell_builder/stages/bosh_go_agent/assets/runit/monit/log/run
- stemcell_builder/stages/system_azure_network/apply.sh
- stemcell_builder/stages/base_warden/assets/runit/cron/run
- stemcell_builder/stages/bosh_enable_password_authentication/apply.sh
- stemcell_builder/stages/image_install_grub_softlayer_two_partitions/config.sh
- stemcell_builder/stages/system_softlayer_multipath_tools/assets/etc/multipath.conf
- stemcell_builder/stages/bosh_monit/assets/monit
- bosh-stemcell/spec/support/os_image_linux_kernel_modules_shared_examples.rb
- stemcell_builder/stages/password_policies/assets/opensuse/common-account.patch
- stemcell_builder/stages/base_warden/assets/service
- stemcell_builder/stages/base_ubuntu_packages/assets/runit.service
- stemcell_builder/stages/base_ubuntu_packages/assets/apparmor-local-sbin.dhclient
- stemcell_builder/stages/image_create_softlayer_two_partitions/config.sh
- stemcell_builder/stages/image_install_grub_softlayer_two_partitions/apply.sh
- stemcell_builder/stages/password_policies/assets/opensuse/common-password.patch
- stemcell_builder/stages/system_parameters/apply.sh
- bosh-stemcell/spec/stemcells/softlayer_spec.rb
- bosh-stemcell/lib/bosh/stemcell/build_environment.rb
- acceptance-tests/ipv4director/ipv6basic/manifest.yml
- bosh-stemcell/lib/bosh/stemcell/definition.rb
- bosh-stemcell/spec/support/os_image_linux_kernel_shared_examples.rb
- bosh-stemcell/spec/stemcells/stig_spec.rb
github-project-automation
Bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
aramprice
force-pushed
the
manual-merge-jammy2noble
branch
from
c423fe5 to
ec92973
Compare
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (19)
.github/workflows/sbom.yml (1)
19-20:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify mutable action refs in this workflow. rg -n 'uses:\s*aquasecurity/trivy-action@master' .github/workflows/sbom.ymlRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 228
Pin
aquasecurity/trivy-actionto an immutable revision (avoid@master)
.github/workflows/sbom.ymlusesaquasecurity/trivy-action@masterin multiple steps (lines 19, 27, 36); pin to a commit SHA (or other immutable audited reference) to make the security pipeline deterministic.🧰 Tools
🪛 zizmor (1.25.2)
[error] 19-19: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/sbom.yml around lines 19 - 20, Replace the floating reference aquasecurity/trivy-action@master with an immutable, audited revision (e.g., a specific commit SHA or a fixed release tag) wherever it appears in the workflow; update each usage of aquasecurity/trivy-action@master so the workflow becomes deterministic and reproducible by using the same pinned SHA/tag for all steps that call the trivy action.bosh-stemcell/spec/bosh/stemcell/infrastructure_spec.rb (1)
95-111:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the duplicated
Infrastructure::Alicloudspec block.Lines 95-111 duplicate the same
Infrastructure::Alicloudexamples already present at Lines 113-129, which adds redundant execution and maintenance overhead.♻️ Proposed fix
- describe Infrastructure::Alicloud do - its(:name) { should eq("alicloud") } - its(:hypervisor) { should eq("kvm") } - its(:default_disk_size) { should eq(5120) } - its(:disk_formats) { should eq(["raw"]) } - its(:stemcell_formats) { should eq(["alicloud-raw"]) } - - it { should eq Infrastructure.for("alicloud") } - it { should_not eq Infrastructure.for("aws") } - - it "has alicloud specific additional cloud properties" do - expect(subject.additional_cloud_properties).to eq({ - "root_device_name" => "/dev/vda1", - "nvme_support" => "supported" - }) - end - end📝 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.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bosh-stemcell/spec/bosh/stemcell/infrastructure_spec.rb` around lines 95 - 111, Remove the duplicated Infrastructure::Alicloud spec block: delete the entire describe Infrastructure::Alicloud example group that starts with its(:name) { should eq("alicloud") } and the it "has alicloud specific additional cloud properties" block (which asserts subject.additional_cloud_properties), leaving the later, non-duplicated describe Infrastructure::Alicloud block intact so tests run once and maintenance is reduced.bosh-stemcell/spec/support/stemcell_image.rb (1)
15-17:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnsure disk unmount always executes during suite teardown.
If
ShelloutTypes::Chroot.unmount_procfails,disk_image.unmountis skipped, leaving mounted state behind for later runs.Suggested fix
- config.after(:suite) do |example| - ShelloutTypes::Chroot.unmount_proc - disk_image.unmount - end + config.after(:suite) do |_example| + begin + ShelloutTypes::Chroot.unmount_proc + ensure + disk_image.unmount + end + end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bosh-stemcell/spec/support/stemcell_image.rb` around lines 15 - 17, The teardown currently calls ShelloutTypes::Chroot.unmount_proc then disk_image.unmount sequentially so if unmount_proc raises the disk_image.unmount is skipped; change the config.after(:suite) block to guarantee disk_image.unmount always runs by moving disk_image.unmount into an ensure (or separate rescue/ensure) so it executes regardless of errors from ShelloutTypes::Chroot.unmount_proc, and optionally rescue/log errors from each call (referencing the config.after(:suite) block, ShelloutTypes::Chroot.unmount_proc, and disk_image.unmount).ci/docker/os-image-stemcell-builder/Dockerfile (1)
70-70:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore explicit
ubuntuuser/group creation before using it.Line 70 and downstream ownership steps assume
ubuntuexists, but this Dockerfile no longer creates that account. WithBASE_IMAGE=ubuntu:noble,chown ubuntu:ubuntuon Lines 74/79/113 will fail and break image build.💡 Suggested fix
ARG OVF_TOOL_INSTALLER ARG OVF_TOOL_INSTALLER_SHA1 +ARG USER_ID=1000 +ARG GROUP_ID=1000 # ENV VARs ENV DEBIAN_FRONTEND="noninteractive" @@ RUN apt-get update \ @@ && locale-gen ${LANG} +RUN getent group ubuntu >/dev/null || groupadd --gid "${GROUP_ID}" ubuntu \ + && id -u ubuntu >/dev/null 2>&1 || useradd --uid "${USER_ID}" --gid "${GROUP_ID}" --create-home --shell /bin/bash ubuntu + RUN echo 'ubuntu ALL=NOPASSWD:ALL' >> /etc/sudoers🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/docker/os-image-stemcell-builder/Dockerfile` at line 70, The Dockerfile appends to /etc/sudoers and later runs chown ubuntu:ubuntu but never creates the ubuntu user/group; add creation of the ubuntu group and user before the line that echoes into /etc/sudoers (e.g., run groupadd/create group and useradd/adduser -m -s /bin/bash -g ubuntu ubuntu or equivalent), ensure the user home is created and shell set, then append the sudoers line and keep the existing chown ubuntu:ubuntu references so they succeed; update any USER or ownership steps that assume that UID/GID if you need a specific numeric ID.docs/noble-dev.md (1)
2-23:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix incorrect branch/file path references in the Noble notes.
Several identifiers in this section do not match repository paths and will break navigation during debugging.
Suggested corrections
-ubunty-noble/master cloned from ubuntu-jammy/master on 07-02-2024 (dd/mm/yyyy) +ubuntu-noble/master cloned from ubuntu-jammy/master on 07-02-2024 (dd/mm/yyyy) -#### stemcell_builder/stages/base-ubuntu-package/apply.sh +#### stemcell_builder/stages/base_ubuntu_packages/apply.sh -#### stemcell_builder/stages/passowrd_policies/apply.sh +#### stemcell_builder/stages/password_policies/apply.sh -#### bosh-stemcell/spec/os-image/ubuntu_noble.spec +#### bosh-stemcell/spec/os_image/ubuntu_spec.rb -#### src/ipv4director/autitd/smoke_test.go +#### acceptance-tests/ipv4director/auditd/smoke_test.go -### src/ipv4director/ipv6basic/basic_test.go +### acceptance-tests/ipv4director/ipv6basic/basic_test.go🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: Ensure spelling is correct
Context: ...-package/apply.sh - libpam-cracklib not availble (yet) installed in | PAM module to enab...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~9-~9: Ensure spelling is correct
Context: ...log-mmjsonparse rsyslog-mmnormalize not availble as adiscon does not have a noble repo ...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~11-~11: Ensure spelling is correct
Context: ...uilder/stages/passowrd_policies/apply.sh - paching common-password is failing as cracklib ...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~12-~12: Ensure spelling is correct
Context: ...-password is failing as cracklib is not availble #### bosh-stemcell/spec/os-image/ubuntu_noble...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
[warning] 6-6: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4(MD001, heading-increment)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 19-19: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
[warning] 22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/noble-dev.md` around lines 2 - 23, The Noble notes contain incorrect branch and file path references and typos that break navigation; update the header and every referenced path/text to match the repository exactly (fix "ubunty-noble/master" → "ubuntu-noble/master" or the actual branch name), correct typos and filenames and ensure they match real files: verify and fix "stemcell_builder/stages/base-ubuntu-package/apply.sh" (libpam package note), "stemcell_builder/stages/password_policies/apply.sh" (patching common-password), "bosh-stemcell/spec/os-image/ubuntu_noble.spec" (ordering/gnats/rescan-scsi-bus notes), and test paths "src/ipv4director/auditd/smoke_test.go" and "src/ipv4director/ipv6basic/basic_test.go"; also correct misspellings ("availble"→"available", "adiscon" if incorrect, "passowrd_policies"→"password_policies", "paching"→"patching", "autitd"→"auditd") and verify any repository links point to the actual branch/file so navigation and debugging work.scripts/stemcell-repack-debugger.sh (4)
44-70:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "== File check ==" ls -l "$FILE" echo echo "== Lines 1-120 ==" sed -n '1,120p' "$FILE" | nl -ba -w2 -s':' echo echo "== Lines 121-220 ==" sed -n '121,220p' "$FILE" | nl -ba -w2 -s':' echo echo "== Lines 221-320 ==" sed -n '221,320p' "$FILE" | nl -ba -w2 -s':' echo echo "== Search for heredoc / injection-related patterns ==" rg -n --hidden --no-ignore-vcs "(<<[[:alpha:]_]+|<<-|\$\(.*\)|eval|bash -c|cat\s*<<|printf\s*%s\s*<<|mktemp|/dev/mapper|dmsetup|losetup|mount|umount|trap|set -e)" "$FILE" || true echo echo "== Search for option parsing / shift patterns ==" rg -n --hidden --no-ignore-vcs "for arg in \"\\$@\"|shift[[:space:]]+[0-9]+|case[[:space:]]+\\$arg|case[[:space:]]+\\\"\\$1\\\"|--debug_pub_key|--bump-version" "$FILE" || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 267
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "== Lines 1-220 ==" sed -n '1,220p' "$FILE" | cat -n echo echo "== Lines 221-420 (if present) ==" sed -n '221,420p' "$FILE" | cat -n echo echo "== Search: option parsing / shift patterns ==" rg -n "(for arg in \"\\$@\"|shift[[:space:]]+[0-9]+|case[[:space:]]+\\$arg|case[[:space:]]+\\\"\\$1\\\"|--debug_pub_key|--bump-version)" "$FILE" || true echo echo "== Search: heredocs / likely injection sinks ==" rg -n "(<<[[:alpha:]_]+|<<-|\beval\b|bash -c|cat[[:space:]]*<<|printf[[:space:]]*%s[[:space:]]*<<|/dev/mapper|dmsetup|losetup|mount|umount|trap\\b|set -x\\b)" "$FILE" || true echo echo "== Search: mount/cleanup lifecycle signals ==" rg -n "(mount|umount|dmsetup|losetup|trap\\b|cleanup|cleanup_|disarm|detach|detach_|losetup -d)" "$FILE" || true echo echo "== Exit/cleanup guards around the cited blocks ==" rg -n "(success|if[[:space:]]+.*success|cleanup|umount|losetup -d|dmsetup|trap\\b|exit[[:space:]]+[0-9]*)" "$FILE" || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 6437
Fix CLI argument parsing: remove the
for arg in "$@"+shiftpatternThe loop iterates over the original
"$@"while also mutating positional parameters viashift; for--debug_pub_key,convert_cert_file="$2"may point at the wrong value after earlier shifts, andshiftin the following iterations can fail underset -e(e.g., when the consumed value becomes the next iteratedarg).💡 Suggested fix
-convert_cert_file="" -bump_version=false -for arg in "$@"; do - case $arg in - --help|-h) - show_help - exit 0 - ;; - --debug) - set -x - ;; - --debug_pub_key) - convert_cert_file="$2" - shift 2 - ;; - --bump-version*) - bump_version=true - if [ -n "$OUT" -a -f "$OUT" ]; then - version=$(bosh int <(tar xf "$OUT" stemcell.MF --occurrence=1 -O) --path /version) - else - version=$(echo "$arg" | awk -F'=' '{print $2}') - fi - shift - ;; - *) - shift - ;; - esac -done +convert_cert_file="" +bump_version=false +while [[ $# -gt 0 ]]; do + case "$1" in + --help|-h) + show_help + exit 0 + ;; + --debug) + set -x + shift + ;; + --debug_pub_key) + convert_cert_file="${2:-}" + shift 2 + ;; + --bump-version*) + bump_version=true + if [[ -n "${OUT:-}" && -f "${OUT}" ]]; then + version="$(bosh int <(tar xf "${OUT}" stemcell.MF --occurrence=1 -O) --path /version)" + else + version="$(echo "$1" | awk -F'=' '{print $2}')" + fi + shift + ;; + *) + shift + ;; + esac +done🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 59-59: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
(SC2166)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/stemcell-repack-debugger.sh` around lines 44 - 70, The argument parsing loop is broken because it iterates over for arg in "$@" while also shifting positional parameters, causing wrong values for convert_cert_file and brittle shifts; change to a standard while [ "$#" -gt 0 ]; do case "$1" in ... esac; done pattern so shifts affect the current parameter correctly, update branches that consume a value (e.g., --debug_pub_key) to read the value from "$2" only after matching "$1" and then shift 2, keep boolean flags like bump_version and logic that reads OUT/version (using version=$(bosh int <(tar xf "$OUT" stemcell.MF --occurrence=1 -O) --path /version) or fallback from "$1" value) but adjust to use "$1" or extracted value consistently, and preserve show_help and --debug handling in the new while-loop.
78-80:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "=== File: $FILE ===" nl -ba "$FILE" | sed -n '60,120p' echo echo "=== Usages of stemcell_tgz in $FILE ===" rg -n "stemcell_tgz" "$FILE" || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 195
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "=== File: $FILE (lines 60-120) ===" awk 'NR>=60 && NR<=120 {printf "%d:%s\n", NR, $0}' "$FILE" echo echo "=== Usages of stemcell_tgz in $FILE ===" rg -n "stemcell_tgz" "$FILE" || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 2883
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "=== File: $FILE (lines 1-70) ===" awk 'NR>=1 && NR<=70 {printf "%d:%s\n", NR, $0}' "$FILE" echo echo "=== Context around line 78-96 ===" awk 'NR>=72 && NR<=100 {printf "%d:%s\n", NR, $0}' "$FILE"Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 3897
Fail fast when
stemcell_tgzis missing.
stemcell_tgzis checked with an unquoted/unsafe expansion ([ -z ${stemcell_tgz} ]) and the script only prints a message, then continues totar xvf $stemcell_tgz(which will fail later). Exit immediately with a safe default expansion.💡 Suggested fix
-if [ -z ${stemcell_tgz} ]; then -echo "stemcell_tgz is not set. export stemcell_tgz=\"${example_stemcell_tgz_path}\"" -fi +if [[ -z "${stemcell_tgz:-}" ]]; then + echo "stemcell_tgz is not set. export stemcell_tgz=\"${example_stemcell_tgz_path}\"" >&2 + 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.if [[ -z "${stemcell_tgz:-}" ]]; then echo "stemcell_tgz is not set. export stemcell_tgz=\"${example_stemcell_tgz_path}\"" >&2 exit 1 fi🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 78-78: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/stemcell-repack-debugger.sh` around lines 78 - 80, The check for stemcell_tgz is using an unquoted expansion and only prints a message without exiting, which can cause later commands like tar xvf $stemcell_tgz to run with an empty variable; update the conditional to use a safe, quoted parameter expansion and fail-fast: test with if [ -z "${stemcell_tgz:-}" ] then echo the suggested export using "${example_stemcell_tgz_path}" and exit 1 (or set stemcell_tgz="${example_stemcell_tgz_path}" if you want a default), and ensure all later uses (e.g., tar xvf "$stemcell_tgz") quote the variable.
99-103:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "== File: $FILE ==" wc -l "$FILE" echo "== Lines 70-170 ==" nl -ba "$FILE" | sed -n '70,170p'Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 252
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "== File: $FILE ==" wc -l "$FILE" echo "== Lines 70-170 ==" nl -ba "$FILE" | sed -n '70,170p'Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 252
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "== trap/umount/kpartx occurrences ==" rg -n --hidden -S "trap|umount|kpartx|mktemp|-o loop" "$FILE" || true echo echo "== Lines 85-120 (with line numbers) ==" awk 'NR>=85 && NR<=120 {printf "%d:%s\n", NR, $0}' "$FILE" echo echo "== Lines 125-155 (with line numbers) ==" awk 'NR>=125 && NR<=155 {printf "%d:%s\n", NR, $0}' "$FILE"Repository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 2764
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="scripts/stemcell-repack-debugger.sh" echo "== Header (1-40) ==" awk 'NR>=1 && NR<=40 {printf "%d:%s\n", NR, $0}' "$FILE" echo echo "== set -e / set -o occurrences ==" rg -n "set\s+-e|set\s+-o\s+(errexit|pipefail|nounset|xtrace)" "$FILE" || true echo echo "== Any other trap handlers ==" rg -n "trap\s+" "$FILE" || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 2315
Ensure EXIT cleanup unmounts and removes the kpartx mapping on failure (not just temp dir removal).
With
set -e, any error after lines 99-103 exits immediately, whilesudo umount/sudo kpartx -dvonly run on the success path at 141-142. The currentEXITtraps onlyrm -rfthe temp directories, so the mountpoint and/dev/mapper/*mappings can be left behind.💡 Suggested fix
-mnt_dir=$(mktemp -d) -traps+='rm -rf "${mnt_dir}";' -trap "${traps}" EXIT -device=$(sudo kpartx -sav $DISK_NAME | grep '^add' | tail -n1 | cut -d' ' -f3) -sudo mount -o loop,rw /dev/mapper/$device $mnt_dir +mnt_dir="$(mktemp -d)" +disk_path="${image_dir}/${DISK_NAME}" +device="" + +cleanup() { + set +e + if mountpoint -q "$mnt_dir"; then + sudo umount "$mnt_dir" + fi + if [[ -n "${device}" ]]; then + sudo kpartx -dv "$disk_path" + fi + rm -rf "$mnt_dir" "$temp_dir" +} +trap cleanup EXIT + +device="$(sudo kpartx -sav "$disk_path" | awk '/^add/ {dev=$3} END {print dev}')" +sudo mount -o rw "/dev/mapper/$device" "$mnt_dir" @@ -sudo umount $mnt_dir -sudo kpartx -dv "$DISK_NAME" +sudo umount "$mnt_dir" +sudo kpartx -dv "$disk_path"🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 100-100: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
[warning] 101-101: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
[info] 103-103: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 103-103: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/stemcell-repack-debugger.sh` around lines 99 - 103, The EXIT trap currently only deletes the temp directory (mnt_dir) and can leave the mountpoint and kpartx device mapping behind; update the trap setup (the traps variable and the trap "${traps}" EXIT invocation) to also attempt to unmount mnt_dir (using sudo umount -l or sudo umount if preferred) and to remove the kpartx mapping (sudo kpartx -dv for $device or by passing $DISK_NAME) guarded by checks that $mnt_dir is mounted and $device (or /dev/mapper/$device) exists; ensure these commands are appended to the traps string before trap is invoked so they run on any EXIT and handle empty/unset $device safely.
131-136:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the file and print relevant lines ls -la scripts/stemcell-repack-debugger.sh nl -ba scripts/stemcell-repack-debugger.sh | sed -n '110,170p' # Find all references to BOSH_DEBUG_PUB_KEY rg -n "BOSH_DEBUG_PUB_KEY" -S . || true # Show how the key is read/used, if sourced from env/args/config rg -n "DEBUG_PUB_KEY|authorized_keys|authorized_keys" -S scripts/stemcell-repack-debugger.sh || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 230
🏁 Script executed:
#!/bin/bash set -euo pipefail # Print relevant region with line numbers via awk (since nl isn't available) echo "=== scripts/stemcell-repack-debugger.sh (110-170) ===" awk 'NR>=110 && NR<=170 {printf "%d\t%s\n", NR, $0}' scripts/stemcell-repack-debugger.sh echo echo "=== References to BOSH_DEBUG_PUB_KEY ===" rg -n "BOSH_DEBUG_PUB_KEY" -S . || true echo echo "=== References to authorized_keys / SSH debug key handling (broader) ===" rg -n "authorized_keys|\\.ssh|DEBUG_PUB_KEY|BOSH_DEBUG_PUB_KEY" -S scripts/stemcell-repack-debugger.sh || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 2737
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== scripts/stemcell-repack-debugger.sh (1-110) ===" awk 'NR>=1 && NR<=110 {printf "%d\t%s\n", NR, $0}' scripts/stemcell-repack-debugger.sh echo echo "=== convert_multiline_to_single definition region ===" # Try to locate the function and print a window around it rg -n "convert_multiline_to_single" scripts/stemcell-repack-debugger.sh line=$(rg -n "convert_multiline_to_single" scripts/stemcell-repack-debugger.sh | head -n1 | cut -d: -f1) awk -v start=$((line-10)) -v end=$((line+30)) 'NR>=start && NR<=end {printf "%d\t%s\n", NR, $0}' scripts/stemcell-repack-debugger.shRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 5994
Avoid command injection via heredoc interpolation of
BOSH_DEBUG_PUB_KEY.
scripts/stemcell-repack-debugger.shuses an unquoted heredoc (<<EOF) withecho $BOSH_DEBUG_PUB_KEY >> .ssh/authorized_keys. BecauseBOSH_DEBUG_PUB_KEYis only newline-stripped (no sanitization), malicious shell syntax in the key (e.g.,$(...)/backticks) can be injected into the inner chrooted/bin/bashscript. Write the key as data instead (e.g.,printf '%s\n' "$BOSH_DEBUG_PUB_KEY" | tee "$mnt_dir/home/bosh_debug/.ssh/authorized_keys") or otherwise prevent it from being interpreted by the inner shell.🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 131-131: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/stemcell-repack-debugger.sh` around lines 131 - 136, The heredoc inside scripts/stemcell-repack-debugger.sh interpolates BOSH_DEBUG_PUB_KEY into the chrooted bash (echo $BOSH_DEBUG_PUB_KEY >> .ssh/authorized_keys) which allows shell injection; change the flow so the key is written as data rather than executed—e.g., stop passing the raw variable through the unquoted heredoc and instead write the key with a safe data write (for example using printf '%s\n' "$BOSH_DEBUG_PUB_KEY" piped to tee or redirecting a safely quoted heredoc) into the target file under $mnt_dir/home/bosh_debug/.ssh/authorized_keys before or without interpolating it inside chroot; update the section around the chroot/useradd/echo lines to perform a non-interpolating write so the inner shell never interprets BOSH_DEBUG_PUB_KEY.stemcell_builder/stages/base_warden/apply.sh (1)
51-74:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftGate Rosetta compatibility overrides behind explicit opt-in.
This block runs unconditionally, so all warden images get weakened service hardening (
NoNewPrivileges, syscall filters, etc.), not just Rosetta cases. That’s broader than the stated threat model and degrades baseline security posture.Suggested fix
-# Rosetta x86_64 emulation compatibility for Apple Silicon Macs -# -# When running warden stemcells under Rosetta emulation on Apple Silicon, -# several systemd services fail because their security hardening features -# (MemoryDenyWriteExecute, SystemCallFilter, etc.) conflict with Rosetta's -# JIT compilation which requires writable+executable memory. -# -# We create systemd drop-in overrides to disable these security features. -# This is acceptable for warden stemcells since they run in containerized -# environments where the host provides security isolation. - -rosetta_services=( - systemd-journald - systemd-resolved - systemd-networkd - systemd-logind - systemd-timesyncd - auditd -) - -for service in "${rosetta_services[@]}"; do - mkdir -p "$chroot/etc/systemd/system/${service}.service.d" - cp "$assets_dir/rosetta-compat.conf" "$chroot/etc/systemd/system/${service}.service.d/rosetta-compat.conf" -done +# Rosetta compatibility should be opt-in; keep hardened defaults otherwise. +if [ "${ENABLE_WARDEN_ROSETTA_COMPAT:-0}" = "1" ]; then + rosetta_services=( + systemd-journald + systemd-resolved + systemd-networkd + systemd-logind + systemd-timesyncd + auditd + ) + + for service in "${rosetta_services[@]}"; do + mkdir -p "$chroot/etc/systemd/system/${service}.service.d" + cp "$assets_dir/rosetta-compat.conf" "$chroot/etc/systemd/system/${service}.service.d/rosetta-compat.conf" + done +fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.# Rosetta compatibility should be opt-in; keep hardened defaults otherwise. if [ "${ENABLE_WARDEN_ROSETTA_COMPAT:-0}" = "1" ]; then rosetta_services=( systemd-journald systemd-resolved systemd-networkd systemd-logind systemd-timesyncd auditd ) for service in "${rosetta_services[@]}"; do mkdir -p "$chroot/etc/systemd/system/${service}.service.d" cp "$assets_dir/rosetta-compat.conf" "$chroot/etc/systemd/system/${service}.service.d/rosetta-compat.conf" done fi🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 73-73: assets_dir is referenced but not assigned.
(SC2154)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/base_warden/apply.sh` around lines 51 - 74, The current rosetta_services array and loop unconditionally install systemd drop-in overrides, weakening hardening for all images; change this to run only when an explicit opt-in is set (e.g. check an env var like ENABLE_ROSETTA_COMPAT or call a detection function like is_rosetta_emulation()) before defining rosetta_services and executing the for-loop; ensure the guard returns false by default, add a clear log message when opt-in is enabled, and leave the rosetta_services variable and the mkdir/cp actions (the loop that references rosetta_services and the cp of rosetta-compat.conf) unchanged except for being wrapped in that conditional.stemcell_builder/stages/bosh_alicloud_agent_settings/assets/agent.md (1)
15-16:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the replacement example so it matches the configured transform.
Line 16 replaces the captured suffix with a different value (
...gw8a1...), which contradicts theDiskIDTransformPattern/DiskIDTransformReplacementbehavior shown on Lines 10-11. This makes the troubleshooting example inaccurate.📝 Proposed fix
-the `DiskIDTransformReplacement` will match and replace `gw8ibkvqdrmdqzwpo4rk` with `virtio-gw8a1q0dqyqmcjrnetpj` +the `DiskIDTransformReplacement` will match and replace `gw8ibkvqdrmdqzwpo4rk` with `virtio-gw8ibkvqdrmdqzwpo4rk`📝 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.the `DiskIDTransformPattern` will match `d-gw8ibkvqdrmdqzwpo4rk` in to 2 groups `d-` and `gw8ibkvqdrmdqzwpo4rk` the `DiskIDTransformReplacement` will match and replace `gw8ibkvqdrmdqzwpo4rk` with `virtio-gw8ibkvqdrmdqzwpo4rk`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/bosh_alicloud_agent_settings/assets/agent.md` around lines 15 - 16, Update the replacement example in agent.md so it matches the configured DiskIDTransformPattern/DiskIDTransformReplacement behavior: ensure the DiskIDTransformReplacement uses the captured group from DiskIDTransformPattern (the suffix like gw8ibkvqdrmdqzwpo4rk) or otherwise preserves that suffix instead of swapping to an unrelated string (e.g., gw8a1...), and make the example output show the transformed value that results from applying DiskIDTransformReplacement to the two captured groups declared in DiskIDTransformPattern.stemcell_builder/stages/bosh_aws_agent_settings/apply.sh (1)
6-6:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "cp\s+\$assets_dir/agent\.json\s+\$chroot/var/vcap/bosh/agent\.json" -S . || true rg -n "bosh_.*_agent_settings/apply\.sh" -S stemcell_builder/stages || true # Print the relevant apply.sh lines for the three files mentioned for f in \ stemcell_builder/stages/bosh_aws_agent_settings/apply.sh \ stemcell_builder/stages/bosh_azure_agent_settings/apply.sh \ stemcell_builder/stages/bosh_cloudstack_agent_settings/apply.sh do if [ -f "$f" ]; then echo "----- $f -----" nl -ba "$f" | sed -n '1,120p' else echo "Missing: $f" fi done # Look for other similar unquoted cp patterns in the repo rg -n "cp\s+\$assets_dir/agent\.json\s+\$chroot/var/vcap/bosh/agent\.json" -S stemcell_builder/stages || true rg -n "cp\s+\$assets_dir/.*\s+\$chroot/.*" -S stemcell_builder/stages || trueRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 215
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "cp\s+\$assets_dir/agent\.json\s+\$chroot/var/vcap/bosh/agent\.json" -S stemcell_builder/stages || true rg -n "cp\s+\$assets_dir/agent\.json\s+\$chroot/var/vcap/bosh/agent\.json" -S . || true for f in \ stemcell_builder/stages/bosh_aws_agent_settings/apply.sh \ stemcell_builder/stages/bosh_azure_agent_settings/apply.sh \ stemcell_builder/stages/bosh_cloudstack_agent_settings/apply.sh do echo "----- $f (first 20 lines) -----" if [ -f "$f" ]; then sed -n '1,20p' "$f" echo else echo "Missing: $f" fi doneRepository: cloudfoundry/bosh-linux-stemcell-builder
Length of output: 852
Quote
cpsource/destination expansions in agent settings apply scripts.In
stemcell_builder/stages/bosh_aws_agent_settings/apply.sh,stemcell_builder/stages/bosh_azure_agent_settings/apply.sh, andstemcell_builder/stages/bosh_cloudstack_agent_settings/apply.sh, quote both paths to avoid word-splitting/globbing issues (e.g.,cp "$assets_dir/agent.json" "$chroot/var/vcap/bosh/agent.json").🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 6-6: assets_dir is referenced but not assigned.
(SC2154)
[info] 6-6: Double quote to prevent globbing and word splitting.
(SC2086)
[warning] 6-6: chroot is referenced but not assigned.
(SC2154)
[info] 6-6: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/bosh_aws_agent_settings/apply.sh` at line 6, The cp command uses unquoted variable expansions which can cause word-splitting/globbing; update the cp invocation to quote both source and destination expansions (e.g., use "$assets_dir/agent.json" and "$chroot/var/vcap/bosh/agent.json") so that the command in apply.sh reliably copies the file even if variables contain spaces or glob characters; apply the same quoting pattern to the corresponding lines that reference assets_dir and chroot in bosh_azure_agent_settings/apply.sh and bosh_cloudstack_agent_settings/apply.sh.Source: Linters/SAST tools
stemcell_builder/stages/bosh_google_agent_settings/assets/agent.md (1)
45-61:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix malformed fenced code block near the
/dev/disk/by-idexample.Line 60 starts with backticks plus inline content, which breaks fence structure and rendering for the section.
Proposed fix
scsi-0Google_PersistentDisk_persistent-disk-0-part2 scsi-0Google_PersistentDisk_persistent-disk-0-part3 scsi-0Google_PersistentDisk_persistent-disk-0-part4 -```nvme.1d0f-766f6c3063363764333330306239326665383531-416d617a6f6e20456c617374696320426c6f636b2053746f7265-00000001-part1</details> <details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.22.1)</summary> [warning] 45-45: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- [warning] 45-45: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- [warning] 61-61: Files should end with a single newline character (MD047, single-trailing-newline) </details> </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@stemcell_builder/stages/bosh_google_agent_settings/assets/agent.mdaround
lines 45 - 61, The fenced code block under the /dev/disk/by-id example is
malformed: the line starting with the nvme identifier
(nvme.1d0f-766f6c3063363764333330306239326665383531-416d617a6f6e20456c617374696320426c6f636b2053746f7265-00000001-part1)
was placed outside/inside backticks incorrectly and breaks rendering; fix it by
making the fenced block consistent — remove the stray backticks before that nvme
line and ensure the block begins withand ends withwith the nvme
identifier (and the other device names) inside the fence so the /dev/disk/by-id
example renders as a single valid code block.</details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:7941f9ff655625140dbd5f63 --> _Source: Linters/SAST tools_ <!-- This is an auto-generated comment by CodeRabbit --> </blockquote></details> <details> <summary>stemcell_builder/stages/bosh_openstack_agent_settings/apply.sh (1)</summary><blockquote> 6-6: _⚠️ Potential issue_ | _🟡 Minor_ <details> <summary>🧩 Analysis chain</summary> 🏁 Script executed: ```shell #!/bin/bash # Verify remaining unquoted-expansion findings in the reviewed files. shellcheck -s bash \ stemcell_builder/stages/bosh_openstack_agent_settings/apply.sh \ stemcell_builder/stages/bosh_systemd_resolved/apply.sh \ stemcell_builder/stages/bosh_vsphere_agent_settings/apply.sh ``` Repository: cloudfoundry/bosh-linux-stemcell-builder Length of output: 3823 --- 🏁 Script executed: ```shell #!/bin/bash set -euo pipefail # Locate prelude_apply.bash and inspect assignments/exports of assets_dir and chroot fd -a "prelude_apply\.bash" stemcell_builder/stages -t f || true fd -a "prelude_apply\.bash" -t f || true # Read the relevant prelude files (small scope: only the likely ones) for f in $(fd -a "prelude_apply\.bash" -t f); do echo "---- $f ----" # print first ~120 lines and any mentions of assets_dir/chroot sed -n '1,160p' "$f" | cat -n rg -n "^(assets_dir|chroot)\b|assets_dir|chroot" "$f" || true done # Also read the three apply.sh files around the flagged lines for f in \ stemcell_builder/stages/bosh_openstack_agent_settings/apply.sh \ stemcell_builder/stages/bosh_systemd_resolved/apply.sh \ stemcell_builder/stages/bosh_vsphere_agent_settings/apply.sh do echo "---- $f ----" sed -n '1,120p' "$f" | cat -n done ``` Repository: cloudfoundry/bosh-linux-stemcell-builder Length of output: 5557 --- 🏁 Script executed: ```shell #!/bin/bash set -euo pipefail # Find where assets_dir is defined/assigned in the project rg -n --hidden --no-ignore-vcs "^[^#]*\bassets_dir\b" stemcell_builder/lib stemcell_builder/stages || true # Find where base_dir and chroot are sourced/derived (context for shellcheck SC2154) rg -n --hidden --no-ignore-vcs "\bchroot\s*=" stemcell_builder/lib stemcell_builder/stages || true rg -n --hidden --no-ignore-vcs "\bassets_dir\s*=" stemcell_builder/lib stemcell_builder/stages || true # Show prelude_common/helper excerpts that likely set up assets_dir/work/chroot for f in $(fd -a "prelude_common\.bash|helpers\.sh" stemcell_builder/lib -t f); do echo "---- $f (first 220 lines) ----" sed -n '1,220p' "$f" | cat -n done ``` Repository: cloudfoundry/bosh-linux-stemcell-builder Length of output: 9114 --- </details> **Fix unquoted path expansions in stage `apply.sh` scripts to avoid word splitting/globbing** - Quote the computed `base_dir` expression and use `source "$base_dir"/...` in `stemcell_builder/stages/bosh_openstack_agent_settings/apply.sh`, `stemcell_builder/stages/bosh_systemd_resolved/apply.sh`, and `stemcell_builder/stages/bosh_vsphere_agent_settings/apply.sh`. - Quote `$assets_dir/agent.json` and `$chroot/var/vcap/bosh/agent.json` in the `cp` commands in `bosh_openstack_agent_settings/apply.sh` and `bosh_vsphere_agent_settings/apply.sh`. - Shellcheck’s “referenced but not assigned” warnings for `assets_dir`/`chroot` come from variables set in sourced prelude scripts; the quoting issues are the ones to address. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Shellcheck (0.11.0)</summary> [warning] 6-6: assets_dir is referenced but not assigned. (SC2154) --- [info] 6-6: Double quote to prevent globbing and word splitting. (SC2086) --- [warning] 6-6: chroot is referenced but not assigned. (SC2154) --- [info] 6-6: Double quote to prevent globbing and word splitting. (SC2086) </details> </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/bosh_openstack_agent_settings/apply.sh` at line 6, The cp command in bosh_openstack_agent_settings apply.sh uses unquoted expansions which can lead to word-splitting/globbing; update cp to use quoted paths by changing references to "$assets_dir/agent.json" and "$chroot/var/vcap/bosh/agent.json". Also ensure any computed base_dir expression used to source prelude files is quoted (use source "$base_dir"/... ) so base_dir with spaces works safely; apply the same quoting pattern in the other stage scripts mentioned (bosh_systemd_resolved/apply.sh and bosh_vsphere_agent_settings/apply.sh) for consistency. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:188402c26d995b84323620d8 --> _Source: Linters/SAST tools_ <!-- This is an auto-generated comment by CodeRabbit --> </blockquote></details> <details> <summary>stemcell_builder/stages/bosh_openstack_agent_settings/assets/agent.md (2)</summary><blockquote> 1-58: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Address markdownlint violations before merge.** This file currently has MD022/MD031/MD040/MD047 warnings (heading/fence spacing, missing fence languages, trailing newline). Please normalize formatting so the docs pass lint consistently. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [style] ~3-~3: This phrasing can be wordy, so consider using a more descriptive and concise alternative. Context: ... issue by using diskIdTransformation as you can see in the example below Do note that if this... (AS_YOU_CAN_SEE) </details> <details> <summary>🪛 markdownlint-cli2 (0.22.1)</summary> [warning] 1-1: Headings should be surrounded by blank lines Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) --- [warning] 7-7: Headings should be surrounded by blank lines Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) --- [warning] 9-9: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- [warning] 9-9: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- [warning] 14-14: Headings should be surrounded by blank lines Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) --- [warning] 20-20: Headings should be surrounded by blank lines Expected: 1; Actual: 0; Below (MD022, blanks-around-headings) --- [warning] 22-22: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- [warning] 22-22: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- [warning] 45-45: Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- [warning] 45-45: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- [warning] 58-58: Files should end with a single newline character (MD047, single-trailing-newline) </details> </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/bosh_openstack_agent_settings/assets/agent.md` around lines 1 - 58, Fix markdownlint warnings in agent.md: add blank lines before and after headings like "Introduction", "Disk ID transform rules", "what it does", and "related information" to address MD022; ensure fenced code blocks (the JSON block containing "DiskIDTransformPattern"/"DiskIDTransformReplacement" and the persistent_disk_hints.json block and the /dev/disk/by-id listing) are each surrounded by a blank line to fix MD031; add fence languages (use "json" for the JSON blocks and "text" or nothing specific for the /dev/disk/by-id list) to resolve MD040; remove trailing whitespace and ensure the file ends with exactly one newline to satisfy MD047. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:4f217dc17e83ee0e79b5f937 --> _Source: Linters/SAST tools_ <!-- This is an auto-generated comment by CodeRabbit --> --- 16-16: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix transformed disk-id example to match the configured replacement rule.** Line 16 shows `scsi-305c...`, but with the documented groups and `scsi-${1}${2}${3}${4}${5}`, the expected value starts with `scsi-05c...`. <details> <summary>Proposed fix</summary> ```diff -the `DiskIDTransformReplacement` will match and replace `scsi-` with the 5 groups `scsi-305c185c6e8a34216ab301f5cf7d17f0d` +the `DiskIDTransformReplacement` will compose `scsi-` with the 5 groups as `scsi-05c185c6e8a34216ab301f5cf7d17f0d` ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. ```suggestion the `DiskIDTransformReplacement` will compose `scsi-` with the 5 groups as `scsi-05c185c6e8a34216ab301f5cf7d17f0d` ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/bosh_openstack_agent_settings/assets/agent.md` at line 16, Update the transformed disk-id example so it matches the documented replacement rule: replace the incorrect `scsi-305c185c6e8a34216ab301f5cf7d17f0d` with the correct `scsi-05c185c6e8a34216ab301f5cf7d17f0d` (the output of the DiskIDTransformReplacement using `scsi-${1}${2}${3}${4}${5}`), i.e., edit the example in agent.md to show the expected `scsi-05c...` value. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:9580c56065799a915756d93c --> <!-- This is an auto-generated comment by CodeRabbit --> </blockquote></details> <details> <summary>stemcell_builder/stages/dev_tools_config/assets/generate_dev_tools_file_list.sh (1)</summary><blockquote> 31-34: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Missing `continue` after absent-package check causes premature script exit.** At Line 31 you detect missing packages, but Line 34 still executes `dpkg-query -L`. With `set -e` (Line 2), the first missing package aborts the script instead of being skipped. <details> <summary>Proposed fix</summary> ```diff for package_name in ${PACKAGES[*]} ; do if ! dpkg -s "$package_name" &> /dev/null ; then echo "$package_name is NOT installed." + continue fi dpkg-query -L "$package_name" | xargs file | grep -Ev ':\s+directory\s*$|:\s+symbolic link to usr/lib\s*$' | awk -F ':' '{ print $1 }' done ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/dev_tools_config/assets/generate_dev_tools_file_list.sh` around lines 31 - 34, The loop that checks each package uses the variable package_name and runs dpkg-query unconditionally, but when a package is missing the script echoes "$package_name is NOT installed." then falls through to dpkg-query (and with set -e the script aborts); modify the loop in generate_dev_tools_file_list.sh so that after the missing-package detection (the dpkg -s check that prints the NOT installed message) you immediately continue to the next iteration (add a continue) to skip running dpkg-query -L for absent packages. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:b435138733e28f89da3a7ec5 --> <!-- This is an auto-generated comment by CodeRabbit --> </blockquote></details> <details> <summary>stemcell_builder/stages/image_create/apply.sh (1)</summary><blockquote> 29-33: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Loop-device precreation loop exits too early and misses intended nodes.** At Line 30, `mknod` failure triggers `break` (Line 31). If `/dev/loop0` already exists, the loop exits immediately and does not create missing higher loop nodes. <details> <summary>Proposed fix</summary> ```diff -# create 64 loopback mappings. This fixes failures with losetup --show --find ${disk_image} -for i in $(seq 0 64); do - if ! mknod -m 0660 /dev/loop$i b 7 $i; then - break - fi +# create loopback mappings. This fixes failures with losetup --show --find ${disk_image} +for i in $(seq 0 63); do + if [ ! -b "/dev/loop$i" ]; then + mknod -m 0660 "/dev/loop$i" b 7 "$i" + fi done ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. ```suggestion for i in $(seq 0 63); do if [ ! -b "/dev/loop$i" ]; then mknod -m 0660 "/dev/loop$i" b 7 "$i" fi done ``` </details> <!-- suggestion_end --> <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Shellcheck (0.11.0)</summary> [info] 30-30: Double quote to prevent globbing and word splitting. (SC2086) --- [info] 30-30: Double quote to prevent globbing and word splitting. (SC2086) </details> </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/image_create/apply.sh` around lines 29 - 33, The loop that precreates loop devices using mknod stops on the first failure (in the for ... seq 0 64 block), so if /dev/loop0 already exists the script breaks and never attempts to create higher-numbered /dev/loop$i nodes; update the loop to skip existing devices and continue on non-fatal errors: for each iteration check if /dev/loop$i already exists and if so continue, otherwise attempt mknod and on failure log/continue instead of break so the loop tries all indices (refer to the for loop using seq, the /dev/loop$i path and the mknod call). ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:34638fbf522332598c9738ef --> <!-- This is an auto-generated comment by CodeRabbit --> </blockquote></details> <details> <summary>stemcell_builder/stages/image_install_grub/apply.sh (1)</summary><blockquote> 20-21: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Avoid invoking `kpartx -sav` twice for partition discovery.** Line 20 and Line 21 each run `kpartx -sav ${device}`. Re-adding mappings during discovery is brittle; parse one command result once to derive both partitions. <details> <summary>Proposed fix</summary> ```diff -device_partition_efi=$(kpartx -sav ${device} | cut -d" " -f3 | head -1) -device_partition_root=$(kpartx -sav ${device} | cut -d" " -f3 | tail -1) +partition_maps=$(kpartx -sav "${device}" | awk '{print $3}') +device_partition_efi=$(echo "${partition_maps}" | head -1) +device_partition_root=$(echo "${partition_maps}" | tail -1) ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **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. ```suggestion partition_maps=$(kpartx -sav "${device}" | awk '{print $3}') device_partition_efi=$(echo "${partition_maps}" | head -1) device_partition_root=$(echo "${partition_maps}" | tail -1) ``` </details> <!-- suggestion_end --> <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Shellcheck (0.11.0)</summary> [info] 20-20: Double quote to prevent globbing and word splitting. (SC2086) --- [info] 21-21: Double quote to prevent globbing and word splitting. (SC2086) </details> </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stemcell_builder/stages/image_install_grub/apply.sh` around lines 20 - 21, The current code runs kpartx -sav ${device} twice to set device_partition_efi and device_partition_root which re-adds mappings and is brittle; instead run kpartx -sav ${device} a single time, capture its output into a variable (or a temp file), then parse that single output to extract both device_partition_efi and device_partition_root (e.g., by selecting the first and last mapped device names from the captured output) and use those parsed values when assigning device_partition_efi and device_partition_root. ``` </details> <!-- fingerprinting:phantom:poseidon:hawk --> <!-- cr-comment:v1:7c8d656b6f793f76085b300d --> <!-- This is an auto-generated comment by CodeRabbit --> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
mkocher
approved these changes
Jun 12, 2026
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: this repository uses a "Merge Forward" strategy
Changes should be made in the earliest applicable branch, and
merged forward through subsequent branches.
ubuntu-<short_name-N>ubuntu-<short_name-N>intoubuntu-<short_name-N+1>