[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Request for join the team



Hello Tian,

Sorry for the delay in the reply,

I have created a repo under the team's org and gave you permission,
you can push start pushing your changes there:
https://salsa.debian.org/pkg-security-team/pocsuite3

The packaging seems to be in a good shape, I have a few suggestions/questions:

1) d/copyright: You can remove the comments on lines 7-8 and also make
the first Files entry (on line 10) shorter by stating "Files: *", this
means that anything not called out in the other copyright entries
below will fall into the wildcard one.

2) pocsuite3/thirdparty/: There seems to be a few python libraries
vendored in that folder, I'm not sure if they're patched or just plain
vendored. Generally speaking we can't use them and should instead rely
on their packaged versions on Debian, eg.: make the package depend on
"python3-prettytable" instead of using the vendored version. This can
also help by making d/copyright even simpler if you can remove it from
the orig tarball (either with a repack or if you can provide a tarball
without it, as upstream).
Now, I understand that ansistrm, for example, is not packaged on
Debian, and so It should be fine to keep this one as it is, but
otherwise could you try to use the ones already packaged on Debian and
make sure they are not shipped in a vendored manner for pocsuite3? You
can keep them in the orig tarball if you'd like, and we can also
discuss using vendored libs if you need them, please let me know if
that's the case, and also let me know if you need help or would like
more details about it.

3) pocsuite3/data/cacert.pem: I noticed this file contains both the
public and private parts of the key, to initialize an http server on
port 666 and wrap the socket with ssl. I believe this is fine (it's
gonna be up to the ftp-master to confirm that it's ok), but I wonder
if you thought about generating a self-signed cert at runtime[0]
instead of reusing the same one for everyone? Note that you don't need
to make this change, I'm just wondering if there's any pros and cons
that I'm not considering since there's a chance you've already
discussed this with other developers of pocsuite3.

4) flake8 + black: Just a suggestion here, not a blocker for having
pocsuite3 on Debian; flake8 seems to detect a lot of small thing that
you probably want to have it fixed, and black can automate some of
those changes for you. None of them seem to really be causing any
bugs, but having flake8 enforced at development stage will definitely
spot an issue for you eventually.

5) docstrings: This is also just a suggestion and definitely not
required for packaging pocsuite3 on Debian: I noticed some docstrings
in the code are not in english, this is not a big deal since the code
itself is in english and I could understand it without issues (at as
far as I went, since I didn't read everything). I think it's a good
idea to eventually translate them to english (you can keep both
languages) to make it easier for others to contribute. But again,
please take this as a suggestion for a low priority improvement.

[0] Alternatively, it should also be possible to generate it at the
install-time of the deb package with the pre/postinst maintainer
scripts.

Thank you for your contributions!

--
Samuel Henrique <samueloph>


Reply to: