Skip to content

refactor(install): address audit findings (stdlib reuse, dead-code, comment hygiene)#382

Merged
BANANASJIM merged 1 commit into
mainfrom
fix/audit-cli-install
Jun 7, 2026
Merged

refactor(install): address audit findings (stdlib reuse, dead-code, comment hygiene)#382
BANANASJIM merged 1 commit into
mainfrom
fix/audit-cli-install

Conversation

@BANANASJIM

Copy link
Copy Markdown
Owner

Addresses a batch of LOW-severity audit findings in src/cli/install/.

Changes

  • plan.zig: replace hand-rolled ensureDirAll/ensureDir with std.fs.cwd().makePath (behavior-identical, absolute paths work; ~20 LoC removed).
  • plan.zig: replace copyFile body with std.fs.copyFileAbsolute — atomic temp+rename, no partial/truncated destination on mid-copy failure, preserves source mode.
  • plan.zig: userInGroup supplementary-group buffer 64 -> 1024 so heavy LDAP/AD memberships no longer silently miss the input-group hint.
  • plan.zig: remove duplicated InstallPlan doc sentence; strip PR-N forward-compat markers.
  • udev.zig: fix service-sentinel ISO date month off-by-one (stdlib Month enum is 1-based; + 1 produced 2..13).
  • services.zig: buildSystemctlSystemArgv / buildSystemctlUserArgv now free already-duped strings on allocation error instead of leaking them.
  • mappings.zig: gate config backup on has_target so a pure force/interactive add (no matching entry) no longer writes a spurious config.toml.bak.*.
  • phase.zig / tests.zig: remove PR-N markers from comments.
  • tests.zig: add regression test writeBinding force pure-add does not create backup (asserts new entry added, old preserved, no backup file).

Skipped findings

  • plan.zig groupGid /etc/group 4096-byte cap: fix requires threading an allocator through groupGid/hostHasInputGroup/userInGroup + call sites; disproportionate to a hint-only edge (LDAP /etc/group >4KB). Left as-is.
  • udev.zig generateUdevRules dead-code wrappers: not truly dead — exercised by 10+ tests via the documented _internals_for_tests boundary; removing them expands test code and churns many call sites with no behavior benefit.
  • probeAndRebindDrivers O(N*M) sysfs: explicitly flagged near-zero impact (install-time only, tiny N/M); not worth a structural rewrite.

Test plan

  • Docker oracle ./scripts/padctl-docker test -> EXIT=0.
  • New backup test proven falsifiable: temporarily reverting the has_target gate makes exactly this test fail at expect(!found_bak).
  • zig fmt --check (0.15.2) clean on all changed files.

refs: codebase audit

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@BANANASJIM, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 3 minutes and 10 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

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.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b7af5022-d736-43bb-b6d6-c2388c52f303

📥 Commits

Reviewing files that changed from the base of the PR and between 8e1c4d6 and 0da4e51.

📒 Files selected for processing (6)
  • src/cli/install/mappings.zig
  • src/cli/install/phase.zig
  • src/cli/install/plan.zig
  • src/cli/install/services.zig
  • src/cli/install/tests.zig
  • src/cli/install/udev.zig
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-cli-install

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.

…omment hygiene)

- plan.zig: replace hand-rolled ensureDirAll/ensureDir with std.fs.cwd().makePath
- plan.zig: replace copyFile body with std.fs.copyFileAbsolute (atomic, no partial-file on failure, preserves source mode)
- plan.zig: userInGroup buffer 64 -> 1024 gids so heavy LDAP/AD memberships are not silently missed
- plan.zig: drop duplicated InstallPlan doc sentence; remove PR-N forward-compat markers
- udev.zig: fix sentinel ISO date month off-by-one (Month enum is 1-based)
- services.zig: free already-duped argv strings on error in buildSystemctlSystemArgv and buildSystemctlUserArgv
- mappings.zig: gate config backup on has_target so a pure force/interactive add no longer writes a spurious backup
- phase.zig/tests.zig: remove PR-N markers from comments
- tests.zig: add regression test for writeBinding force pure-add (entry added, no backup)

refs: codebase audit
@BANANASJIM BANANASJIM force-pushed the fix/audit-cli-install branch from c785c31 to 0da4e51 Compare June 7, 2026 04:30
@BANANASJIM BANANASJIM merged commit f4e5acd into main Jun 7, 2026
36 checks passed
@BANANASJIM BANANASJIM deleted the fix/audit-cli-install branch June 7, 2026 04:44
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.

1 participant