https://bugzilla.redhat.com/show_bug.cgi?id=2448590



--- Comment #8 from Daniel BerrangĂ© <[email protected]> ---
> - %{?sysusers_requires_compat} should be added for rhel9

Ok

> - Node.js.adoc: All Node.js module spec files must include a `+%check+`, add
>   %check
>   %{__nodejs} -e
> 'require("%{buildroot}%{nodejs_sitearch}/pccs/pccs_server.js")' || :

That isn't appropriate for application code, only modules, because it starts
executing the main() entry point and things rapidly go down hill from there.


> - ReviewGuidelines.adoc:  Each architecture listed in `+ExcludeArch+` *MUST*
> have a bug filed in bugzilla

That is only relevant where the architecture exclusion is due to a bug that
needs tracking.

This exclude is a reflection of architecture specific hardware, so there's no
bug that needs fixing.

> - PatchUpstreamStatus.adoc: All patches in Fedora spec files *SHOULD* have a
> comment

The patches are all git commits that have detailed descriptions in the commit
message, so I don't think we need to duplicate comments in the spec

> - pccs.service line 16: InaccessibleDirectories= has been deprecated in
> favor of InaccessiblePaths=. (ok for rhel9)

Ok

> - The spec comments (lines 68-73) say PCCS can run on any platform since it
> doesn't require local SGX hardware. But the service file has
> ConditionPathExists=/dev/sgx_enclave

Bug, as is the later device acl for that.

> - Systemd Unit File: Consider Adding Security Hardening
> 
>   The service file already has DevicePolicy=closed and
> InaccessibleDirectories=/home, which is good. Per the systemd guidelines,
> consider adding more hardening:
> 
>   ProtectSystem=strict
>   ProtectHome=yes
>   NoNewPrivileges=yes
>   ReadWritePaths=/var/lib/pccs /var/log/pccs /etc/pccs

Yes, and ProtectHome obsoletes the InaccessibleDirectories cnofig

> - -admin Subpackage Missing Versioned Dependency on Base
>   Line 133: The Recommends uses %{?_isa} but the admin subpackage is
> BuildArch: noarch (line 134). Using %{?_isa} on a noarch package
> recommending an arch-specific package will resolve to the build architecture,
>   which may cause issues. It should be:
> 
>   Recommends: sgx-pccs = %{epoch}:%{version}-%{release}

Hmm, yes, I forgot the epoch too


The changes are at the same URLs

Spec URL: https://berrange.fedorapeople.org/review/sgx-pccs/sgx-pccs.spec
SRPM URL:
https://berrange.fedorapeople.org/review/sgx-pccs/sgx-pccs-1.25-1.fc45.src.rpm


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2448590

Report this comment as SPAM: 
https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202448590%23c8

-- 
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/[email protected]
Do not reply to spam, report it: 
https://forge.fedoraproject.org/infra/tickets/issues/new

Reply via email to