Skip to content

Conversation

@tabrisnet
Copy link
Contributor

just what it says on the tin.
this is related to issues with ping and setuid

@github-actions github-actions bot added the 02 Milestone: First quarter release label Dec 6, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2025

Walkthrough

This pull request adds libcap2-bin to the runtime dependencies of the armbian-common package in debian/control. The change is a single-line addition to the Depends field, specifying an additional required package for the armbian-common installation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modified with one dependency line added
  • Straightforward syntax with no logic or structural implications
  • Homogeneous, repetitive-pattern change (simple package listing)

Areas requiring attention:

  • Verify the justification for adding libcap2-bin (the diff does not explain the reason for this dependency)
  • Confirm libcap2-bin is the correct package name and available for all supported architectures
  • Check whether this dependency should be conditional or applied to other packages in the project

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding libcap2-bin dependency to armbian-common for setcap functionality.
Description check ✅ Passed The description is related to the changeset, explaining the rationale for adding libcap2-bin and linking to relevant issues about ping and setuid.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a30dbbd and 8d6104a.

📒 Files selected for processing (1)
  • debian/control (1 hunks)
🔇 Additional comments (1)
debian/control (1)

25-25: Addition of libcap2-bin is necessary and appropriate for capability-based privilege management.

Adding libcap2-bin to the base system dependencies is correct. The package is widely available across Debian (bullseye/bookworm/trixie) and Ubuntu (jammy/24.04+) with minimal installed size (~100-150 kB). It provides the setcap/getcap utilities needed to configure ping and other utilities to use capability-based privileges (e.g., CAP_NET_RAW) instead of setuid bits. Debian/Ubuntu packages automatically configure ping with this approach when libcap2-bin is present; without it, they fall back to less secure setuid root. This dependency ensures the base system follows modern security practices.


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.

@github-actions github-actions bot added size/small PR with less then 50 lines Needs review Seeking for review labels Dec 6, 2025
@tabrisnet
Copy link
Contributor Author

  • Verify the justification for adding libcap2-bin (the diff does not explain the reason for this dependency)

you've previously told me I can't inject comments to the Depends list...
It is explained, if only by reference, in the PR description.
#25 (comment)
#25 (comment)
armbian/build#9033

  • Confirm libcap2-bin is the correct package name and available for all supported architectures

iputils_20240905-3.dsc says Architecture: any
Somehow I don't expect architecture to be a problem.

  • Check whether this dependency should be conditional or applied to other packages in the project

it's being applied to armbian-common which is the pkg that almost all other packages in this project rely upon.

@coderabbitai

@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2025

Tip

For best results, initiate chat on the files or code changes.

@tabrisnet, thank you for the thorough clarification! You're absolutely right—the justification is well documented in the PR description and linked discussions. I appreciate you taking the time to address each point:

  1. Justification: The references to the ping/setuid issues and the related discussions clearly establish the need for setcap utility provided by libcap2-bin.

  2. Architecture availability: Good catch on the Architecture: any from the Debian package source—this confirms cross-architecture support.

  3. Package placement: Placing this in armbian-common as a base dependency makes perfect sense given its role in the project.

The change looks good to me. My apologies for not initially checking the linked context in the PR description more carefully.

@tabrisnet tabrisnet requested a review from leggewie December 6, 2025 17:54
@tabrisnet tabrisnet mentioned this pull request Dec 6, 2025
@leggewie
Copy link
Collaborator

leggewie commented Dec 7, 2025

not necessary, iputils-ping will take care of whatever it needs as you managed to show by inspecting postinst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

02 Milestone: First quarter release Needs review Seeking for review size/small PR with less then 50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants