Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add automated CI checks and git hooks #2739

Closed
jose1711 opened this issue Jun 2, 2019 · 15 comments · Fixed by #4643
Closed

Add automated CI checks and git hooks #2739

jose1711 opened this issue Jun 2, 2019 · 15 comments · Fixed by #4643
Labels
enhancement New feature request

Comments

@jose1711
Copy link
Contributor

jose1711 commented Jun 2, 2019

It would be nice to have automated checks for things that tend to repeat in new profile submissions:

  • header present (with description, persistent globals and local include)
  • items sorted inside profile sections
  • sections are divided using a single '\n' character
  • firecfg.section, disable-programs.inc and similar files being sorted
@rusty-snake rusty-snake added the enhancement New feature request label Jun 2, 2019
@rusty-snake
Copy link
Collaborator

rusty-snake commented Jun 2, 2019

I had a similar idea a few days ago and started to write it in rust, but I'm actually thinking if it's still necessary if you prominently recommend the profile.template for it (e.g. use GH pull request templates (and provieder there a checklist), CONTRIBUTIN.md, manpage (see my comment in the add template issue)), because it will never be possible to check it with human inteliegenz (e.g. include disable-xdg.inc in an image-viewer-profile but not noblacklist ${PICTURES})

EDIT: The most review comments are ordering (which can prevent with the profile.template and/or GH pull request templates with checklist) and the second most are special thing that can only found with human intelligence.

@jose1711
Copy link
Contributor Author

jose1711 commented Jun 2, 2019

I agree for the most part - I wonder though how is this GH pull request template supposed to work. Is it difficult to implement?

@rusty-snake
Copy link
Collaborator

Its a issue template but for pull request, see https://help.github.com/en/articles/about-issue-and-pull-request-templates.

@rusty-snake
Copy link
Collaborator

rusty-snake commented Jun 4, 2019

With this script you get alphabetical sorted private-etc and private-bin lines. I will add support for automaticaly fixing profile.

#!/usr/bin/env python3
from sys import argv

with open(argv[1], "r") as profile:
    for line in profile:
        if line[:11] == "private-etc":
            print("private-etc", ",".join(sorted(line[12:-1].split(","),
                key=lambda s: s.casefold())))
        elif line[:11] == "private-bin":
            print("private-bin", ",".join(sorted(line[12:-1].split(","),
                key=lambda s: s.casefold())))

Or if you prever to use a shell:

echo "alternatives,ca-certificates,crypto-policies,fonts,machine-id,pki,resolv.conf,ssl" | sed "s/,/\n/g" | sort - | awk '{printf $1 ","}END{print ""}'

@jose1711
Copy link
Contributor Author

jose1711 commented Jun 4, 2019

note that [12:-1] means excluding the last character. not sure if you want that.

@rusty-snake
Copy link
Collaborator

@jose1711 thanks. The last character is \n. I test this code, it works.

@jose1711
Copy link
Contributor Author

jose1711 commented Jun 4, 2019

ok, but what if private-etc is the last line and is not terminated with \n? like so:
echo -n 'private-etc foo,bar' > profile
then I get this:

./fix_sorting profile                          
private-etc ba,foo

@rusty-snake
Copy link
Collaborator

rusty-snake commented Jun 7, 2019

@jose1711 Most profiles end with a newline, but you're right that we have to consider the missing of it.


New version, with support for automatic fixing several profiles.
https://gist.github.com/rusty-snake/a1010a3daf3c54e93dfe03f2f5ce3d96

Issues:

  • "[ Fixed ] {filename}" is always printed, even if nothing was fixed. Fixed, only if no private-etc or private-bin is present. Readly Fixed.
  • All files will be rewritten. Fixed, only if no private-etc or private-bin is present. Readly Fixed.
  • Names starting with two uppercase letters are not handled correctly. Sure? No
  • Names containing an uppercase letter are not handled correctly (e.g. QOwnNotes). was never an issue
  • Handling of special characters (e.g. _ - . ). ignoring is fine

TODOs:

  • private-lib Done, #private-etc, # private-etc, #private-bin, # private-bin Won't fix, protocol Done

@glitsj16
Copy link
Collaborator

@rusty-snake Can't track the item right now (you referred to my fork of your sort.py script), but if you like to integrate sorting caps.{drop,keep} and seccomp.{drop,keep}, go right ahead. You did a great job creating that tool. The better it can cover this wide array of firejail options, the more changes there are this gets into CI. My personal little profile regression tester caught the first one just a few minutes ago 😄. Cheers!

@rusty-snake
Copy link
Collaborator

@glitsj16 I already add caps.{drop,keep} and seccomp.{drop,keep}. All supported options are: private-bin, private-etc, private-lib, seccomp.drop, seccomp.keep, caps.drop, caps.keep, protocol.

rusty-snake pushed a commit that referenced this issue Jun 24, 2019
* add contrib/sort.py and .github/pull_request_temp…

* Add usage to sort.py

* Install sort.py if contrib-install is set

* sort.py: 0644 -> 0755

* Update sort.py

* Update pull_request_template.md

* Remove checkboxes from PR-Template

* Update sort.py

* Add examples to sort.py

* Update pull_request_template.md

Fix path to sort.py, it depend on the distro.

* Update pull_request_template.md

* Update pull_request_template.md

add hint about template
@rusty-snake
Copy link
Collaborator

Is there anything else we want to do here?

@matu3ba
Copy link
Contributor

matu3ba commented Apr 9, 2020

@rusty-snake Is fetching the program binary with checking, if the program runs and kill it afterwards possible?

@rusty-snake
Copy link
Collaborator

You mean checking if firejail runs? The are a lot of test under test.

@matu3ba
Copy link
Contributor

matu3ba commented Apr 11, 2020

@rusty-snake Does this include checking meaningful shell options as well? I dont see according shell commands for firejail execution in the .travis-ci and .gitlab-ci.
They should be able to simulate all .local and .global configurations.

@rusty-snake
Copy link
Collaborator

in the travis.yml is make test-travis which runs the test under test.

@rusty-snake rusty-snake linked a pull request Oct 27, 2021 that will close this issue
@kmk3 kmk3 added this to Done (on RELNOTES) in Release 0.9.68 Feb 6, 2022
kmk3 added a commit that referenced this issue Feb 6, 2022
And move the profile checks item to the ci section.

Relates to #2739 #4643 #4774.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
No open projects
Release 0.9.68
  
Done (on RELNOTES)
Development

Successfully merging a pull request may close this issue.

4 participants