Fix cis compliance stemcell issues - #702
Open
karthickaliappa wants to merge 9 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe stemcell build enables sudo pseudo-terminal allocation and logging. It includes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 5
🤖 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 `@bosh-stemcell/spec/support/os_image_shared_examples.rb`:
- Around line 74-80: Update the `/etc/profile.d/01-tmout.sh` InSpec example to
assert that the file is owned by root and its group is root, alongside the
existing file, mode, and content checks.
- Around line 241-244: Update the “sets MaxStartups to 10:30:60” example to
validate the effective configuration rather than mere presence: assert that
active MaxStartups directives contain exactly one entry with the expected value,
or use sshd -T to verify the process-effective value. Preserve the expected
value “MaxStartups 10:30:60”.
In `@stemcell_builder/stages/base_ssh/apply.sh`:
- Around line 27-28: Quote the chroot-derived paths in the sshd_config update
commands: update the sed target and output redirection associated with the
MaxStartups replacement so "$chroot" cannot undergo word splitting or glob
expansion. Preserve the existing deletion and append behavior.
- Around line 27-28: Update the MaxStartups cleanup in apply.sh to remove
directives with either spaces or tabs before appending the stemcell value.
Ensure all existing MaxStartups lines are normalized away so only the appended
`MaxStartups 10:30:60` directive remains.
In `@stemcell_builder/stages/bosh_users/apply.sh`:
- Around line 41-45: Quote the $chroot path expansion in both the here-document
redirection and the chmod command within the TMOUT profile setup, preserving the
existing file path and permissions.
🪄 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 Plus
Run ID: f9395556-6990-419d-af0d-2a4f1a0d305e
📒 Files selected for processing (4)
bosh-stemcell/spec/support/os_image_shared_examples.rbstemcell_builder/stages/base_ssh/apply.shstemcell_builder/stages/bosh_users/apply.shstemcell_builder/stages/bosh_users/assets/sudoers
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
- Set MaxStartups 10:30:100 in sshd_config (CIS 5.1.18) - Enable use_pty and logfile=/var/log/sudo.log in sudoers (CIS 5.2.2, 5.2.3) - Configure TMOUT=900 shell timeout in /etc/profile.d/01-tmout.sh (CIS 5.4.3.2) - Add corresponding rspec tests in os_image_shared_examples
- Change MaxStartups from default 10:30:100 to 10:30:60 in sshd_config - Update corresponding rspec test assertion
- Assert root ownership and group for /etc/profile.d/01-tmout.sh in rspec tests - Ensure MaxStartups scan validates single directive in sshd_config - Normalize MaxStartups sed pattern to match tab/space indentations and quote chroot paths - Quote chroot path expansions in TMOUT profile script creation
karthickaliappa
force-pushed
the
fix-cis-compliance-stemcell-issues
branch
from
August 3, 2026 14:28
1f0b772 to
7936f44
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@bosh-stemcell/spec/support/os_image_shared_examples.rb`:
- Around line 29-30: Update the sudo policy assertions in the shared examples
around the content match checks to validate the effective settings for the
intended sudo user, rather than relying only on ordered /etc/sudoers entries.
Ensure included sudoers files cannot override use_pty or logfile, while
preserving verification of the intended values.
In `@stemcell_builder/stages/bosh_users/assets/sudoers`:
- Around line 7-8: Update the sudoers asset by adding the /etc/sudoers.d include
directive after the privilege entries, preserving the required drop-in policy
loading and existing sudo configuration.
🪄 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 Plus
Run ID: c8b15601-c6dc-4ad1-aeb0-289e7b975a07
📒 Files selected for processing (4)
bosh-stemcell/spec/support/os_image_shared_examples.rbstemcell_builder/stages/base_ssh/apply.shstemcell_builder/stages/bosh_users/apply.shstemcell_builder/stages/bosh_users/assets/sudoers
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bosh-stemcell/spec/support/os_image_shared_examples.rb (1)
247-249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalize
MaxStartupsmatches beforecontain_exactly.The regex allows leading spaces or tabs, but
scanreturns the raw matched text. Ifsshd_configcontainsMaxStartups 10:30:60or\tMaxStartups 10:30:60, this assertion compares against the non-canonical string and fails. Count or build the expected value from stripped matches instead.🤖 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/os_image_shared_examples.rb` around lines 247 - 249, Update the MaxStartups expectation in the “sets MaxStartups to 10:30:60” example to normalize each regex match by removing leading whitespace before applying contain_exactly. Preserve the existing pattern and expected canonical value while ensuring indented configuration lines compare successfully.
🤖 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.
Outside diff comments:
In `@bosh-stemcell/spec/support/os_image_shared_examples.rb`:
- Around line 247-249: Update the MaxStartups expectation in the “sets
MaxStartups to 10:30:60” example to normalize each regex match by removing
leading whitespace before applying contain_exactly. Preserve the existing
pattern and expected canonical value while ensuring indented configuration lines
compare successfully.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e41c3742-0438-43e5-9a09-5e2e3674ce2f
📒 Files selected for processing (3)
bosh-stemcell/spec/support/os_image_shared_examples.rbstemcell_builder/stages/bosh_sudoers/apply.shstemcell_builder/stages/bosh_users/assets/sudoers
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
…pecs Annotate SSH MaxStartups (CIS-5.1.18), sudo use_pty (CIS-5.2.2), sudo logfile (CIS-5.2.3), and TMOUT idle timeout (CIS-5.4.3.2) in build scripts, assets, and RSpec shared examples.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bosh-stemcell/spec/support/os_image_shared_examples.rb (1)
33-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIgnore whitespace-prefixed comments in the
!use_ptycheck.
egrep -v '^#'only skips lines that start with#. A line such as#Defaults!use_ptypasses the negative filter as an active override, so this example can fail even though the policy is unchanged. Filter^[[:space:]]*#instead.Proposed fix
- describe command("egrep -sh '!use_pty' /etc/sudoers /etc/sudoers.d/* | egrep -v '^#' --") do + describe command("egrep -sh '!use_pty' /etc/sudoers /etc/sudoers.d/* | egrep -v '^[[:space:]]*#' --") do🤖 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/os_image_shared_examples.rb` around lines 33 - 35, Update the `command` assertion in the `!use_pty` check to exclude comments beginning with optional whitespace by changing the negative filter to match `^[[:space:]]*#`; preserve the existing file targets and stdout expectation.
🤖 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.
Outside diff comments:
In `@bosh-stemcell/spec/support/os_image_shared_examples.rb`:
- Around line 33-35: Update the `command` assertion in the `!use_pty` check to
exclude comments beginning with optional whitespace by changing the negative
filter to match `^[[:space:]]*#`; preserve the existing file targets and stdout
expectation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d75e3ab0-49d6-494a-acc6-768f1ee4061e
📒 Files selected for processing (5)
bosh-stemcell/spec/support/os_image_shared_examples.rbstemcell_builder/stages/base_ssh/apply.shstemcell_builder/stages/bosh_sudoers/apply.shstemcell_builder/stages/bosh_users/apply.shstemcell_builder/stages/bosh_users/assets/sudoers
Update egrep pattern to '^[[:space:]]*#' so commented-out !use_pty lines with leading spaces or tabs are properly ignored in the spec assertion.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bosh-stemcell/spec/support/os_image_shared_examples.rb (1)
249-251: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNormalize the scanned SSH lines before comparison.
scanreturns the leading whitespace when the directive is indented, butcontain_exactlycompares the full matched string. Strip each entry before the assertion so an indentedMaxStartups 10:30:60line does not fail.Suggested fix
- expect(sshd_config.content.scan(/^[ \t]*MaxStartups\s+\S+$/)).to contain_exactly("MaxStartups 10:30:60") + expect(sshd_config.content.scan(/^[ \t]*MaxStartups\s+\S+$/).map(&:strip)).to contain_exactly("MaxStartups 10:30:60")🤖 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/os_image_shared_examples.rb` around lines 249 - 251, Normalize the matches produced by the MaxStartups assertion in the “sets MaxStartups to 10:30:60” example by stripping leading and trailing whitespace from each scanned entry before contain_exactly compares them. Preserve the expected directive value and existing scan pattern.
🤖 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 `@bosh-stemcell/spec/support/os_image_shared_examples.rb`:
- Around line 33-35: Update the sudoers scan command assertion in the
surrounding shared examples to verify a successful exit status in addition to
empty stdout, ensuring missing or unreadable files fail the check. Follow the
existing no-match assertion pattern and inspect stderr when needed to
distinguish an expected no-match status from command errors.
---
Outside diff comments:
In `@bosh-stemcell/spec/support/os_image_shared_examples.rb`:
- Around line 249-251: Normalize the matches produced by the MaxStartups
assertion in the “sets MaxStartups to 10:30:60” example by stripping leading and
trailing whitespace from each scanned entry before contain_exactly compares
them. Preserve the expected directive value and existing scan pattern.
🪄 Autofix
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 Plus
Run ID: e64eacfb-70af-422b-b7c6-771b574c4ac9
📒 Files selected for processing (1)
bosh-stemcell/spec/support/os_image_shared_examples.rb
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
Strip leading/trailing whitespace from scanned MaxStartups directives before checking contain_exactly in os_image_shared_examples.rb.
github-project-automation
Bot
moved this from Waiting for Changes | Open for Contribution
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
selzoc
requested changes
Aug 6, 2026
selzoc
left a comment
selzoc
left a comment
Member
There was a problem hiding this comment.
Can you please add a human-readable summary of the changes to the PR description?
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
Alphasite
requested changes
Aug 6, 2026
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
3 participants
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.
This PR addresses CIS benchmark compliance failures on BOSH stemcells by implementing hardening controls and corresponding test assertions for identified CIS rule IDs.
Changes are as below
MaxStartups 10:30:60in/etc/ssh/sshd_configto throttle concurrent unauthenticated SSH connections.Defaults use_ptyin/etc/sudoersto ensure sudo commands execute within a pseudo-terminal.Defaults logfile=/var/log/sudo.login/etc/sudoersfor dedicated sudo activity logging./etc/profile.d/01-tmout.shwithreadonly TMOUT=900(declared read-only so a session can't override it) for a 15-minute idle shell session timeout.Impact on Stemcell Users
/var/log/sudo.log.