Skip to content

[rqd] Improve logging on fs and caps errors#2402

Merged
DiegoTavares merged 3 commits into
AcademySoftwareFoundation:masterfrom
DiegoTavares:rqd_log_fs_error
Jun 10, 2026
Merged

[rqd] Improve logging on fs and caps errors#2402
DiegoTavares merged 3 commits into
AcademySoftwareFoundation:masterfrom
DiegoTavares:rqd_log_fs_error

Conversation

@DiegoTavares

@DiegoTavares DiegoTavares commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Whenever rqd starts without its required capabilities, it now fails right away, instead of waiting for a frame to fail trying to execute unauthorized ops. Besides that, endure fs errors are properly logged.

Summary by CodeRabbit

  • Improvements
    • Much richer diagnostics when log initialization fails: error messages now include process ownership, directory permissions and mount info to aid troubleshooting on Linux/macOS.
    • Startup now performs an early capability/privilege preflight when running as a non-default user and surfaces any missing privileges immediately to fail fast.
  • Tests
    • Added unit/integration tests validating diagnostics content and preflight capability checks on supported platforms.

Whenever starts without its required capabilities, it now fails right away, instead of waiting for a
frame to fail trying to execute unauthorized ops. Besides that, ensire fs errors are properly
logged.
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Linux capability preflight invoked during startup when run_as_user is enabled, enriches FrameFileLogger startup errors with best-effort path and filesystem diagnostics, and wires the preflight into main to surface privilege issues early.

Changes

Startup Robustness Improvements

Layer / File(s) Summary
Capability preflight verification module
rust/crates/rqd/src/system/capabilities.rs
New Linux-only startup module reads effective capability bitmask from /proc/self/status, checks for required capabilities (CAP_SETUID, CAP_SETGID, CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_FOWNER) when run_as_user is enabled, and returns a miette error listing missing capabilities. Non-Linux platforms provide a stub preflight that returns Ok(()). Unit tests validate decoding and no-op behavior.
Enhanced log initialization error context
rust/crates/rqd/src/frame/logging.rs
FrameFileLogger::init now wraps rotation, parent-directory creation, and log-file creation failures with operation-specific miette context and uses a best-effort describe_path_failure helper on Unix to include process euid/egid, intended ownership, parent directory metadata, and mount info. Adds Linux/macOS tests asserting diagnostic contents and failure behavior on unwritable directories.
Startup flow wiring
rust/crates/rqd/src/system/mod.rs, rust/crates/rqd/src/main.rs, rust/Cargo.toml
Exports capabilities from system; consolidates use crate::{...} imports in main.rs; calls capabilities::preflight(&CONFIG.runner)? after logging initialization to fail fast on privilege/elevation misconfiguration; bumps workspace version.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I sniff the startup morning dew,
Check caps and paths to see us through,
From /proc a whisper, logs made bright,
Errors now speak with clearer light,
Hop on—startup’s safer, too!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: improved logging for filesystem errors and capability checks at startup.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@rust/crates/rqd/src/frame/logging.rs`:
- Around line 572-588: The test test_init_into_unwritable_dir_reports_failing_op
currently only gates on unsafe { geteuid() } != 0 which is insufficient when
capabilities like CAP_DAC_OVERRIDE are present; instead, before calling
FrameFileLogger::init, attempt a real write/create probe inside the locked dir
(e.g., try creating/opening a temporary file for writing in locked) and if that
probe succeeds, skip the test — only proceed to call FrameFileLogger::init and
assert failure when the probe confirms the directory is actually unwritable;
ensure you still restore permissions for cleanup afterward.

In `@rust/crates/rqd/src/system/capabilities.rs`:
- Around line 57-85: Remove the early-return that skips capability validation
when geteuid() == 0 and instead always run the capability checks via
effective_caps() so root users with reduced CapEff (e.g., in containers or user
namespaces) fail fast; specifically, delete or disable the branch that returns
Ok() on the unsafe nix::libc::geteuid() == 0 check and allow the existing logic
that computes caps = effective_caps(), compares REQUIRED_CAPS, builds missing,
and returns the miette! error to execute for root as well.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4cb73bbe-24ea-4077-b288-009a76c57b7a

📥 Commits

Reviewing files that changed from the base of the PR and between e504bc9 and dc5aed8.

📒 Files selected for processing (4)
  • rust/crates/rqd/src/frame/logging.rs
  • rust/crates/rqd/src/main.rs
  • rust/crates/rqd/src/system/capabilities.rs
  • rust/crates/rqd/src/system/mod.rs

Comment thread rust/crates/rqd/src/frame/logging.rs
Comment thread rust/crates/rqd/src/system/capabilities.rs Outdated
@DiegoTavares DiegoTavares merged commit 2b4a304 into AcademySoftwareFoundation:master Jun 10, 2026
24 of 25 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