Review for Package: ledmon

[Summary]
Since the last time we looked at that plenty of rules got improved
and a re-review was likely to discover things we would have missed in the
past.
Indeed I found some things that are required and some that are recommended
to be resolved before a promotion to main.

MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

This does not need a security review (see comment #9)

List of specific binary packages to be promoted to main: ledmon
Specific binary packages built, but NOT to be promoted to main: <none>

Since there are required TODOs I'll assign it back to Foundations who want
to own it and mark it incomplete. Please ping once the open TODOs are resolved
for a final Ack to get it promoted.

Required TODOs:
- The service should use some security features like private spaces,
  not running as root and so on - please evaluate if we can use this
  to lower the risk of any potential exploitation.
- Please add automated tests (likely impossible) or as fallback outline
  the test steps that you will execute and the HW this will run on (details
  below)
- fix bad dependencies and ledctl failing to start due to bad lib references
  (details below)

Recommended TODOs:
- Please try to have the service not start (or exit quickly, but sucessfully)
  on the 99.9% of systems it won't be needed.
- Please have a look if this makes no sense to run in a container ever
  and if so add a systemd condition to not start there

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- does not use webkit1,2
- does not use lib*v8 directly
- does not parse data formats
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does not run a daemon as root
  => If this really is only monitoring and controlling leds, could it
     run in a much more restricted way please - private tmp/home, less user
     permissions. If it needs just one special device probably get that owned
     by a group and run the service as that. It seems having it to be root
     isn't appropriate for the use case.

[Common blockers]
RULE: - There are plenty of testing requirements, hopefully already resolved
RULE:   by the reporter upfront, see "Quality assurance - testing" section of
RULE:   the Main Inclusion requirements

OK:
- does not FTBFS currently
- no new python2 dependency

Problems:
- does not have a test suite that runs at build time
- does not have a non-trivial test suite that runs as autopkgtest
- special HW does prevent build/autopkgtest.
=> In this case we have too often had packages too rarely tested.
   Nowadays teams owngin the package (in this case foundations and or the
   requesting OEM team) are asked to describe their regular test plan.
   If automated please link to the test code.
   Even if it can only be run manually please provide a test log.
   I understand that comment #15 can be seen as kind of a test log.
   But in that case please ensure you have the HW and people to re-run that test
   at least on uploads and feature freeze of a cycle.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok (if needed, e.g. non-native)
- Upstream update history is rather slow, but ok in this case
- Debian/Ubuntu update history is slow (matching upstream)
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- d/rules is rather clean
- It is not on the lto-disabled list

Problems:
- Service issues I: this comes with a service that is enabled by default
  (ledmon.service) We have taken so many efforts to keep services disabled that
  are not needed (memory & cpu footprint) that in this case which only applies
  to a very few special cases the service should get a way to not start up
  (or early exit) if no supported HW is present.
- Service issues II:
  In a container it comes up crashes quickly a few times and then
  gives up failing with bad state. If it makes no sense to run in a container
  then a condition to not start there would be very helpful. Less resources
  wasted trying to start it and no bad status service being around after boot.
  If there also is no sense to run it in non-container virtualization then also
  block this case.
- Currently the delivered control program ledctl does not work in jammy.
  Right after install if started it complains about missing libraries.
  This indicates a lack of proper dependencies.
  root@j-proposed:~# ledctl
   ledctl: error while loading shared libraries: libsgutils2-1.45.so.2:
   cannot open shared object file: No such file or directory
  Seems there was an soname bump and this wasn't updated correctly.
  ii  libsgutils2-2:amd64 1.46-1       amd64        utilities for devices using the SCSI command set (shared libraries)
  root@j-proposed:~# dpkg -L libsgutils2-2
   /usr/lib/x86_64-linux-gnu/libsgutils2-1.46.so.2.0.0


[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as we can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
  tests)
- no use of user nobody
- no use of setuid
- use of setuid, but ok because <TBD> (prefer systemd to set those
  for services)
- no important open bugs (crashers, etc) in Debian or Ubuntu
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this case (user visible)?