Add profile linter

[nobody43]

Abstractions/tunables checks weren't done, they will take a while.
I don't know how you want to integrate it, so leaving it up to you. As well as applying the suggestions (or tell me how you see it being handled).
I will work in the same direction, after 3 weeks.

Related #570

[nobody43]

Modernize the name? (apparmor.d -> AppArmor.d)

[roddhjav]

I would rather not. apparmor.d refers to /etc/apparmor.d not to AppArmor. Plus I don't want to have to update all headers (again).

[nobody43]

OK, reverted.

[roddhjav]

Nice work thanks a lot. It is really useful to have you working on this.

I think that for now, your current structure will do it. Ideally (later), it would be nice to provide a proper aa-lint tool. I think we should try to make a linter that is not specific for this project (and that can ignore some check).

I think the implementation of aa-lint should looks like existing linter in python such as yamllint. Thus, you will have to create a new directory in the apparmor.d project, e.g.: aa-lint with the python code.

I am going to create a new issue where we can discuss both the structure of the linter and the list of checks it should implement.

[roddhjav]

When I run the script over the profiles with python tests/profile_check.py -d apparmor.d, I get the following error for
all profiles:

{
  "filename": "apparmor.d/groups/apt/apt-config",
  "profile": null,
  "severity": "WARNING",
  "line": null,
  "reason": "No trailing syntax hint",
  "suggestion": "# vim:syntax=apparmor"
}

That does not sound normal...

Suggesting owner for some file pattern is a good thing, however, it can raise some issues. E.g: a program running as root can have some legitimate access inside @{HOME}/ or temporary files owner by another user.

[roddhjav]

This should be move outside the main with other import.

See, for example: https://github.com/roddhjav/pass-import/blob/19805430712d3b8b82f58373e3771d21bbe1fa64/pass_import/formats/kdbx.py#L10-L16

Then in the main: https://github.com/roddhjav/pass-import/blob/19805430712d3b8b82f58373e3771d21bbe1fa64/pass_import/formats/kdbx.py#L216

[nobody43]

That does not sound normal...

That was because of trailing '\n'. What would be the policy - add to suggestion or remove from the line?

Suggesting owner for some file pattern is a good thing, however, it can raise some issues.

Should I remove it?

Also I don't understand why your filename isn't an absolute path.

[roddhjav]

We don't care if the vim syntax has a trailing line, so remove it. To be more precise, this is:

1. Not a bit issue, so let's not annoy profile developer with that
2. Not what has been done in the profile and thus, it would require to update all of them.

Also I don't understand why your filename isn't an absolute path.

It is a full path, I manually anonymized the data.

[nobody43]

We don't care if the vim syntax has a trailing line, so remove it.

I don't think that's possible while preserving the check. Each line in file have trailing \n at the end. While making a comparison I have to preserve it. Similar issue when taking original line as suggestion - see with output of handleFileMessages().

It is a full path, I manually anonymized the data.

Then you should delete the comment entirely because of edit history.

UPD: on the other hand, it would be better to obscure it:

    for m in messages:
        if m.get('suggestion'):
            if m['suggestion'].endswith('\n'):
                m['suggestion'] = m.get('suggestion').removesuffix('\n')

[nobody43]

Should we ignore orphan profiles like apparmor.d/groups/children/? (in future PRs)

[roddhjav]

Yes, they will have to be ignored.

[nobody43]

profiles-m-r/pam_roles move to profiles-m-r/pam/ and also exclude?

[roddhjav]

Thanks, merged!

For future PR, try to follow the same architecture as yamllint.