Skip to content

fix: Set mode for bootloader user and grub conf files to 0600#220

Merged
spetrosi merged 3 commits into
linux-system-roles:mainfrom
spetrosi:conf-mode-cis
Jul 15, 2026
Merged

fix: Set mode for bootloader user and grub conf files to 0600#220
spetrosi merged 3 commits into
linux-system-roles:mainfrom
spetrosi:conf-mode-cis

Conversation

@spetrosi

@spetrosi spetrosi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Enhancement: Set mode for bootloader user and grub conf files to 0600

Reason: Due to CIS Benchmark 1.4.2 Ensure access to bootloader config is configured, mode of grub config files should be 0600. Default grub config doesn't fall under this benchmark, its default mode is 0644. CIS Benchmar PDF is available upon registering at https://www.cisecurity.org/cis-benchmarks

Result: The mode of grub conf and user conf is set to 0600

Issue Tracker Tickets (Jira or BZ if any): https://redhat.atlassian.net/browse/RHEL-184795

Summary by CodeRabbit

  • Bug Fixes
    • Standardized file permission handling for bootloader configuration artifacts using dedicated, configuration-driven permission settings.
    • Ensured the GRUB default file and the GRUB/user configuration files consistently receive the expected secure modes.
  • Tests
    • Extended password/config tests to validate the exact file modes of the generated bootloader configuration files.
    • Improved test cleanup by restoring the system’s original default kernel after removing cloned kernels.

@spetrosi
spetrosi requested a review from richm as a code owner July 13, 2026 14:35
@spetrosi

Copy link
Copy Markdown
Contributor Author

[citest]

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.94%. Comparing base (63d7c0c) to head (d2aeaeb).
⚠️ Report is 125 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #220      +/-   ##
==========================================
+ Coverage   78.43%   79.94%   +1.51%     
==========================================
  Files           2        3       +1     
  Lines         255      344      +89     
==========================================
+ Hits          200      275      +75     
- Misses         55       69      +14     
Flag Coverage Δ
sanity ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@spetrosi, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9d3446ae-6ce3-4bf4-8fcb-db6b20d0b985

📥 Commits

Reviewing files that changed from the base of the PR and between d00f8fb and d2aeaeb.

📒 Files selected for processing (4)
  • tests/tests_add_rm.yml
  • tests/tests_include_vars_from_parent.yml
  • tests/tests_password.yml
  • tests/tests_settings.yml
📝 Walkthrough

Walkthrough

Changes

Bootloader permission modes

Layer / File(s) Summary
Centralized mode application
vars/main.yml, tasks/main.yml
Defines default GRUB and bootloader configuration modes, then applies them when creating or updating GRUB and password configuration files.
Permission mode validation
tests/tests_password.yml
Marks the cleanup block as public and verifies the expected modes on generated bootloader configuration files.
Default kernel cleanup
tests/tests_add_rm.yml
Restores the original default kernel before cloned kernels are removed during cleanup.

Suggested reviewers: richm

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description Format ⚠️ Warning PR text has Enhancement/Reason/Result, but it omits the required Signed-off-by line. Add a Signed-off-by: Name line (and Assisted-by if desired) to the PR description.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the bootloader file mode change.
Description check ✅ Passed The description matches the required template and includes enhancement, reason, result, and issue tracker ticket.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread tasks/main.yml
@richm

richm commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

[citest]

@spetrosi

Copy link
Copy Markdown
Contributor Author

newer Fedora use 0664 for default grub that's why the test fails, will fix it

@spetrosi

Copy link
Copy Markdown
Contributor Author

[citest]

@spetrosi

Copy link
Copy Markdown
Contributor Author

[citest]

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/tests_add_rm.yml`:
- Around line 139-143: Guard both cleanup tasks in the always section, including
“Restore the original default kernel” and the corresponding removal task, with a
condition requiring __default_kernel and __default_kernel.kernel to be defined
before templating or using the value.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 47fb729e-975f-436a-a0f9-8f9089afb102

📥 Commits

Reviewing files that changed from the base of the PR and between d6bfeed and 9fc7609.

📒 Files selected for processing (4)
  • tasks/main.yml
  • tests/tests_add_rm.yml
  • tests/tests_password.yml
  • vars/main.yml
🚧 Files skipped from review as they are similar to previous changes (3)
  • tasks/main.yml
  • tests/tests_password.yml
  • vars/main.yml

Comment thread tests/tests_add_rm.yml
@spetrosi

Copy link
Copy Markdown
Contributor Author

[citest]

* Due to CIS Benchmark 1.4.2 Ensure access to bootloader config is
  configured, mode of grub config files should be 0600.
  Default grub config doesn't fall under this benchmark, it's default
  mode is 0644.
  CIS Benchmar PDF is available upon registering at
  https://www.cisecurity.org/cis-benchmarks
* Add test to verify that the mode is set correctly

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/tests_add_rm.yml`:
- Around line 140-146: Add the `tests::cleanup` tag to the cleanup tasks around
`__default_kernel`, including the “Restore the original default kernel” task and
the following cloned-kernel removal task, so debug runs can skip both cleanup
operations.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d51ad9dd-7b57-4c09-9e20-e35928457eec

📥 Commits

Reviewing files that changed from the base of the PR and between 9fc7609 and d00f8fb.

📒 Files selected for processing (4)
  • tasks/main.yml
  • tests/tests_add_rm.yml
  • tests/tests_password.yml
  • vars/main.yml
🚧 Files skipped from review as they are similar to previous changes (3)
  • tests/tests_password.yml
  • tasks/main.yml
  • vars/main.yml

Comment thread tests/tests_add_rm.yml
@spetrosi

Copy link
Copy Markdown
Contributor Author

[citest]

@spetrosi

Copy link
Copy Markdown
Contributor Author

[citest]

@spetrosi
spetrosi merged commit 2d0e24b into linux-system-roles:main Jul 15, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants