github-starred/firejail

Fork 0

mirror of https://github.com/netblue30/firejail.git synced 2026-05-15 14:16:14 -06:00

[GH-ISSUE #3979] KMail profile in 0.9.64.4 has multiple fatal errors #2488

New issue

Closed

opened 2026-05-05 09:10:21 -06:00 by gitea-mirror · 8 comments

gitea-mirror commented

2026-05-05 09:10:21 -06:00

Owner

Originally created by @creideiki on GitHub (Feb 12, 2021).
Original GitHub issue: https://github.com/netblue30/firejail/issues/3979

Bug and expected behavior
Gentoo has been lagging a bit with updates, so I did not test any version between 0.9.64 and 0.9.64.4. In 0.9.64, KMail (and Akonadi) ran without issues using the default profile. In 0.9.64.4, the application does not run at all. It seems the refactoring of the profile done in 5532fbdb97 introduced several errors.

First, there are syntax errors. Several mkdir lines have file names containing asterisks. This gives the following error:

Error: "${HOME}/.cache/akonadi*" is an invalid filename: rejected character: "*"

I am not sure what they intend to do, but whatever it is it's not working. Especially confusing is the line

mkdir /tmp/akonadi-*

Yes, Akonadi creates a directory in /tmp, but its name is random and seems to have been created using mkstemp(3) or similar. I'm not sure how Firejail is supposed to be able to pre-create it.

Removing the asterisks makes Firejail at least accept the profile syntactically and try to run the program.

At startup, Firejail now prints the following warning:

***
*** Warning: cannot whitelist ${DOCUMENTS} directory
*** Any file saved in this directory will be lost when the sandbox is closed.
***

This seems like it would be a problem for saving e-mail attachments.

The next error comes when KMail tries to start Akonadi, which tries to start PostgreSQL. This fails with the following error:

org.kde.pim.akonadiserver: stdout: "waiting for server to start....2021-02-12 11:03:25.168 CET [53] FATALT:  kunde inte \xC3\xB6ppna katalog \"/usr/share/postgresql-13/timezonesets\": Filen eller katalogen finns inte\n2021-02-12 11:03:25.168 CET [53] TIPS:  Detta tyder p\xC3\xA5 en inkomplett PostgreSQL-installation alternativt att filen \"/usr/lib64/postgresql-13/bin/postgres\" har flyttats bort fr\xC3\xA5n sin korrekta plats.\n stopped waiting\n"

I am not sure why it is in Swedish, when I have set LC_MESSAGES=POSIX, but that's a different problem. The actual error is that PostgreSQL needs access to /usr/share/postgresql-13/ in order to run. Adding the following line to kmail.profile fixes that:

whitelist /usr/share/postgresql*

The next problem is this message on the console:

kf.config.core: Couldn't write "/home/creideiki/.config/kmail2rc" . Disk full?

Which may have something to do with the profile creating a directory with that name:

mkdir ${HOME}/.config/kmail2rc

when it's supposed to be a file:

$ stat ~/.config/kmail2rc
  File: /home/creideiki/.config/kmail2rc
  Size: 24660           Blocks: 56         IO Block: 4096   regular file

There are several similar lines in the profile creating directories with names that are supposed to be files. Commenting them all out makes KMail start:

#mkdir ${HOME}/.config/emaildefaults
#mkdir ${HOME}/.config/emailidentities
#mkdir ${HOME}/.config/kmail2rc
#mkdir ${HOME}/.config/kmailsearchindexingrc
#mkdir ${HOME}/.config/mailtransports
#mkdir ${HOME}/.config/specialmailcollectionsrc

And now it actually reads mail. However, the error message

kf.config.core: Couldn't write "/home/creideiki/.config/kmail2rc" . Disk full?

still appears. I have not tried to change any settings to see what would happen, since I don't want it to corrupt my working production configuration. Neither have I tried running GnuPG.

Finally, when exiting KMail, it crashes with a SIGSEGV:

*** KMail got signal 11 (Exiting)
*** Dead letters dumped.
KCrash: crashing... crashRecursionCounter = 2
KCrash: Application Name = kmail path = /usr/bin pid = 20
KCrash: Arguments: /usr/bin/kmail

I tried restoring an older kmail.profile, from commit 319f2dc8d6, and it has none of the above problems.

Given the multitude of problems with commit 5532fbdb97, I'd suggest reverting it until it can be fixed.

Environment

Gentoo Linux unstable
firejail version 0.9.64.4

Compile time support:
- AppArmor support is disabled
- AppImage support is enabled
- chroot support is enabled
- D-BUS proxy support is enabled
- file and directory whitelisting support is enabled
- file transfer support is enabled
- firetunnel support is disabled
- networking support is enabled
- overlayfs support is disabled
- private-home support is enabled
- private-cache and tmpfs as user enabled
- SELinux support is disabled
- user namespace support is enabled
- X11 sandboxing support is enabled

Checklist

The profile (and redirect profile if exists) hasn't already been fixed upstream.
The program has a profile. (If not, request one in https://github.com/netblue30/firejail/issues/1139)
I have performed a short search for similar issues (to avoid opening a duplicate).
If it is a AppImage, --profile=PROFILENAME is used to set the right profile.
Used LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 PROGRAM to get english error-messages.
I'm aware of browser-allow-drm yes/browser-disable-u2f no in firejail.config to allow DRM/U2F in browsers.

debug output

Autoselecting /bin/bash as shell
Building quoted command line: 'kmail' 
Command name #kmail#
Found kmail.profile profile in /etc/firejail directory
Reading profile /etc/firejail/kmail.profile
Found disable-common.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-common.inc
Found disable-devel.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-devel.inc
Found disable-exec.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-exec.inc
Found disable-interpreters.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-interpreters.inc
Found disable-passwdmgr.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-passwdmgr.inc
Found disable-programs.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-programs.inc
Found disable-xdg.inc profile in /etc/firejail directory
Reading profile /etc/firejail/disable-xdg.inc
Error: "${HOME}/.cache/akonadi*" is an invalid filename: rejected character: "*"

Originally created by @creideiki on GitHub (Feb 12, 2021). Original GitHub issue: https://github.com/netblue30/firejail/issues/3979 **Bug and expected behavior** Gentoo has been lagging a bit with updates, so I did not test any version between 0.9.64 and 0.9.64.4. In 0.9.64, KMail (and Akonadi) ran without issues using the default profile. In 0.9.64.4, the application does not run at all. It seems the refactoring of the profile done in 5532fbdb9749c5333ac03152f8c94fd364182d32 introduced several errors. First, there are syntax errors. Several `mkdir` lines have file names containing asterisks. This gives the following error: ``` Error: "${HOME}/.cache/akonadi*" is an invalid filename: rejected character: "*" ``` I am not sure what they intend to do, but whatever it is it's not working. Especially confusing is the line ``` mkdir /tmp/akonadi-* ``` Yes, Akonadi creates a directory in `/tmp`, but its name is random and seems to have been created using `mkstemp(3)` or similar. I'm not sure how Firejail is supposed to be able to pre-create it. Removing the asterisks makes Firejail at least accept the profile syntactically and try to run the program. At startup, Firejail now prints the following warning: ``` *** *** Warning: cannot whitelist ${DOCUMENTS} directory *** Any file saved in this directory will be lost when the sandbox is closed. *** ``` This seems like it would be a problem for saving e-mail attachments. The next error comes when KMail tries to start Akonadi, which tries to start PostgreSQL. This fails with the following error: ``` org.kde.pim.akonadiserver: stdout: "waiting for server to start....2021-02-12 11:03:25.168 CET [53] FATALT: kunde inte \xC3\xB6ppna katalog \"/usr/share/postgresql-13/timezonesets\": Filen eller katalogen finns inte\n2021-02-12 11:03:25.168 CET [53] TIPS: Detta tyder p\xC3\xA5 en inkomplett PostgreSQL-installation alternativt att filen \"/usr/lib64/postgresql-13/bin/postgres\" har flyttats bort fr\xC3\xA5n sin korrekta plats.\n stopped waiting\n" ``` I am not sure why it is in Swedish, when I have set `LC_MESSAGES=POSIX`, but that's a different problem. The actual error is that PostgreSQL needs access to `/usr/share/postgresql-13/` in order to run. Adding the following line to `kmail.profile` fixes that: ``` whitelist /usr/share/postgresql* ``` The next problem is this message on the console: ``` kf.config.core: Couldn't write "/home/creideiki/.config/kmail2rc" . Disk full? ``` Which may have something to do with the profile creating a directory with that name: ``` mkdir ${HOME}/.config/kmail2rc ``` when it's supposed to be a file: ``` $ stat ~/.config/kmail2rc File: /home/creideiki/.config/kmail2rc Size: 24660 Blocks: 56 IO Block: 4096 regular file ``` There are several similar lines in the profile creating directories with names that are supposed to be files. Commenting them all out makes KMail start: ``` #mkdir ${HOME}/.config/emaildefaults #mkdir ${HOME}/.config/emailidentities #mkdir ${HOME}/.config/kmail2rc #mkdir ${HOME}/.config/kmailsearchindexingrc #mkdir ${HOME}/.config/mailtransports #mkdir ${HOME}/.config/specialmailcollectionsrc ``` And now it actually reads mail. However, the error message ``` kf.config.core: Couldn't write "/home/creideiki/.config/kmail2rc" . Disk full? ``` still appears. I have not tried to change any settings to see what would happen, since I don't want it to corrupt my working production configuration. Neither have I tried running GnuPG. Finally, when exiting KMail, it crashes with a SIGSEGV: ``` *** KMail got signal 11 (Exiting) *** Dead letters dumped. KCrash: crashing... crashRecursionCounter = 2 KCrash: Application Name = kmail path = /usr/bin pid = 20 KCrash: Arguments: /usr/bin/kmail ``` I tried restoring an older `kmail.profile`, from commit 319f2dc8d65fe0264e4eb6006aab024751bb5bd4, and it has none of the above problems. Given the multitude of problems with commit 5532fbdb9749c5333ac03152f8c94fd364182d32, I'd suggest reverting it until it can be fixed. **Environment** - Gentoo Linux unstable - firejail version 0.9.64.4 Compile time support: - AppArmor support is disabled - AppImage support is enabled - chroot support is enabled - D-BUS proxy support is enabled - file and directory whitelisting support is enabled - file transfer support is enabled - firetunnel support is disabled - networking support is enabled - overlayfs support is disabled - private-home support is enabled - private-cache and tmpfs as user enabled - SELinux support is disabled - user namespace support is enabled - X11 sandboxing support is enabled **Checklist** - [x] The profile (and redirect profile if exists) hasn't already been fixed [upstream](https://github.com/netblue30/firejail/tree/master/etc). - [x] The program has a profile. (If not, request one in `https://github.com/netblue30/firejail/issues/1139`) - [x] I have performed a short search for similar issues (to avoid opening a duplicate). - [x] If it is a AppImage, `--profile=PROFILENAME` is used to set the right profile. - [x] Used `LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8 PROGRAM` to get english error-messages. - [x] I'm aware of `browser-allow-drm yes`/`browser-disable-u2f no` in `firejail.config` to allow DRM/U2F in browsers. <details><summary> debug output </summary> ``` Autoselecting /bin/bash as shell Building quoted command line: 'kmail' Command name #kmail# Found kmail.profile profile in /etc/firejail directory Reading profile /etc/firejail/kmail.profile Found disable-common.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-common.inc Found disable-devel.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-devel.inc Found disable-exec.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-exec.inc Found disable-interpreters.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-interpreters.inc Found disable-passwdmgr.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-passwdmgr.inc Found disable-programs.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-programs.inc Found disable-xdg.inc profile in /etc/firejail directory Reading profile /etc/firejail/disable-xdg.inc Error: "${HOME}/.cache/akonadi*" is an invalid filename: rejected character: "*" ``` </details>

gitea-mirror closed this issue

2026-05-05 09:10:22 -06:00

gitea-mirror commented

2026-05-05 09:10:24 -06:00

Author

Owner

@kmk3 commented on GitHub (Feb 12, 2021):

First, there are syntax errors. Several mkdir lines have file names
containing asterisks. This gives the following error:
Error: "${HOME}/.cache/akonadi*" is an invalid filename: rejected character: "*"
I am not sure what they intend to do, but whatever it is it's not working.
Especially confusing is the line
mkdir /tmp/akonadi-*
Yes, Akonadi creates a directory in /tmp, but its name is random and seems
to have been created using mkstemp(3) or similar. I'm not sure how Firejail
is supposed to be able to pre-create it.

Removing the asterisks makes Firejail at least accept the profile
syntactically and try to run the program.

That's a pretty bad one. Related to that, I have been working on a profile
linter, to catch things such as what commit deae31301 ("use ${DOWNLOADS} in
lutris.profile (#3955)") fixes, and now I've created another script to catch
this specific semantic error. It boils down to this:

grep -n "$(printf '^mkdir\n^mkfile')" -- "$@" | grep -F '*'

Before commit bb9107e2a ("Revert "Merge pull request #3607 from
kortewegdevries/wemail""), it correctly detects those akonadi entries:

etc/profile-a-l/kmail.profile:50:mkdir ${HOME}/.cache/akonadi*
etc/profile-a-l/kmail.profile:52:mkdir ${HOME}/.config/akonadi*
etc/profile-a-l/kmail.profile:60:mkdir ${HOME}/.local/share/akonadi*
etc/profile-a-l/kmail.profile:69:mkdir /tmp/akonadi-*

And after that commit, it fortunately doesn't find anything.

At startup, Firejail now prints the following warning:
***
*** Warning: cannot whitelist ${DOCUMENTS} directory
*** Any file saved in this directory will be lost when the sandbox is closed.
***
This seems like it would be a problem for saving e-mail attachments.

The next error comes when KMail tries to start Akonadi, which tries to start
PostgreSQL. This fails with the following error:
org.kde.pim.akonadiserver: stdout: "waiting for server to start....2021-02-12 11:03:25.168 CET [53] FATALT:  kunde inte \xC3\xB6ppna katalog \"/usr/share/postgresql-13/timezonesets\": Filen eller katalogen finns inte\n2021-02-12 11:03:25.168 CET [53] TIPS:  Detta tyder p\xC3\xA5 en inkomplett PostgreSQL-installation alternativt att filen \"/usr/lib64/postgresql-13/bin/postgres\" har flyttats bort fr\xC3\xA5n sin korrekta plats.\n stopped waiting\n"
I am not sure why it is in Swedish, when I have set LC_MESSAGES=POSIX, but
that's a different problem. The actual error is that PostgreSQL needs access
to /usr/share/postgresql-13/ in order to run. Adding the following line to
kmail.profile fixes that:

LC_MESSAGES appears to be intended only for prompts. From POSIX.1‐2017
Section 8.2 Internationalization
Variables:

LC_MESSAGES

This variable shall determine the locale category for processing
affirmative and negative responses and the language and cultural
conventions in which messages should be written. It also affects the
behavior of the catopen() function in determining the message catalog.
Additional semantics of this variable, if any, are
implementation-defined. The language and cultural conventions of
diagnostic and informative messages whose format is unspecified by
POSIX.1-2017 should be affected by the setting of LC_MESSAGES.

I would suggest trying with LC_ALL=C or LC_ALL=POSIX instead.

whitelist /usr/share/postgresql*
The next problem is this message on the console:
kf.config.core: Couldn't write "/home/creideiki/.config/kmail2rc" . Disk full?
Which may have something to do with the profile creating a directory with
that name:
mkdir ${HOME}/.config/kmail2rc
when it's supposed to be a file:
$ stat ~/.config/kmail2rc
  File: /home/creideiki/.config/kmail2rc
  Size: 24660           Blocks: 56         IO Block: 4096   regular file
There are several similar lines in the profile creating directories with
names that are supposed to be files. Commenting them all out makes KMail
start:
#mkdir ${HOME}/.config/emaildefaults
#mkdir ${HOME}/.config/emailidentities
#mkdir ${HOME}/.config/kmail2rc
#mkdir ${HOME}/.config/kmailsearchindexingrc
#mkdir ${HOME}/.config/mailtransports
#mkdir ${HOME}/.config/specialmailcollectionsrc

That's bad as well. I have caught some of these on an adjacent PR here:

https://github.com/netblue30/firejail/pull/3849#issuecomment-753663010

And I suppose that there's unfortunately no way to automatically detect those
without prior knowledge of the paths, but it might be worth it to add some
sanity checks to pull_request_template.md, such as:

* [ ] I have verified that each new mkdir line refers to a directory
* [ ] I have verified that each new mkfile line refers to a file

@rusty-snake Thoughts on that?

@kmk3 commented on GitHub (Feb 12, 2021): > First, there are syntax errors. Several `mkdir` lines have file names > containing asterisks. This gives the following error: > > ``` > Error: "${HOME}/.cache/akonadi*" is an invalid filename: rejected character: "*" > ``` > > I am not sure what they intend to do, but whatever it is it's not working. > Especially confusing is the line > > ``` > mkdir /tmp/akonadi-* > ``` > > Yes, Akonadi creates a directory in `/tmp`, but its name is random and seems > to have been created using `mkstemp(3)` or similar. I'm not sure how Firejail > is supposed to be able to pre-create it. > > Removing the asterisks makes Firejail at least accept the profile > syntactically and try to run the program. That's a pretty bad one. Related to that, I have been working on a profile linter, to catch things such as what commit deae31301 ("use ${DOWNLOADS} in lutris.profile (#3955)") fixes, and now I've created another script to catch this specific semantic error. It boils down to this: ```sh grep -n "$(printf '^mkdir\n^mkfile')" -- "$@" | grep -F '*' ``` Before commit bb9107e2a ("Revert "Merge pull request #3607 from kortewegdevries/wemail""), it correctly detects those akonadi entries: ``` etc/profile-a-l/kmail.profile:50:mkdir ${HOME}/.cache/akonadi* etc/profile-a-l/kmail.profile:52:mkdir ${HOME}/.config/akonadi* etc/profile-a-l/kmail.profile:60:mkdir ${HOME}/.local/share/akonadi* etc/profile-a-l/kmail.profile:69:mkdir /tmp/akonadi-* ``` And after that commit, it fortunately doesn't find anything. > At startup, Firejail now prints the following warning: > > ``` > *** > *** Warning: cannot whitelist ${DOCUMENTS} directory > *** Any file saved in this directory will be lost when the sandbox is closed. > *** > ``` > > This seems like it would be a problem for saving e-mail attachments. > > The next error comes when KMail tries to start Akonadi, which tries to start > PostgreSQL. This fails with the following error: > > ``` > org.kde.pim.akonadiserver: stdout: "waiting for server to start....2021-02-12 11:03:25.168 CET [53] FATALT: kunde inte \xC3\xB6ppna katalog \"/usr/share/postgresql-13/timezonesets\": Filen eller katalogen finns inte\n2021-02-12 11:03:25.168 CET [53] TIPS: Detta tyder p\xC3\xA5 en inkomplett PostgreSQL-installation alternativt att filen \"/usr/lib64/postgresql-13/bin/postgres\" har flyttats bort fr\xC3\xA5n sin korrekta plats.\n stopped waiting\n" > ``` > > I am not sure why it is in Swedish, when I have set `LC_MESSAGES=POSIX`, but > that's a different problem. The actual error is that PostgreSQL needs access > to `/usr/share/postgresql-13/` in order to run. Adding the following line to > `kmail.profile` fixes that: `LC_MESSAGES` appears to be intended only for prompts. From [POSIX.1‐2017 Section 8.2 Internationalization Variables](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html): > LC_MESSAGES > > This variable shall determine the locale category for processing > affirmative and negative responses and the language and cultural > conventions in which messages should be written. It also affects the > behavior of the catopen() function in determining the message catalog. > Additional semantics of this variable, if any, are > implementation-defined. The language and cultural conventions of > diagnostic and informative messages whose format is unspecified by > POSIX.1-2017 should be affected by the setting of LC_MESSAGES. I would suggest trying with `LC_ALL=C` or `LC_ALL=POSIX` instead. > ``` > whitelist /usr/share/postgresql* > ``` > > The next problem is this message on the console: > > ``` > kf.config.core: Couldn't write "/home/creideiki/.config/kmail2rc" . Disk full? > ``` > > Which may have something to do with the profile creating a directory with > that name: > > ``` > mkdir ${HOME}/.config/kmail2rc > ``` > > when it's supposed to be a file: > > ``` > $ stat ~/.config/kmail2rc > File: /home/creideiki/.config/kmail2rc > Size: 24660 Blocks: 56 IO Block: 4096 regular file > ``` > > There are several similar lines in the profile creating directories with > names that are supposed to be files. Commenting them all out makes KMail > start: > > ``` > #mkdir ${HOME}/.config/emaildefaults > #mkdir ${HOME}/.config/emailidentities > #mkdir ${HOME}/.config/kmail2rc > #mkdir ${HOME}/.config/kmailsearchindexingrc > #mkdir ${HOME}/.config/mailtransports > #mkdir ${HOME}/.config/specialmailcollectionsrc > ``` That's bad as well. I have caught some of these on an adjacent PR here: * <https://github.com/netblue30/firejail/pull/3849#issuecomment-753663010> And I suppose that there's unfortunately no way to automatically detect those without prior knowledge of the paths, but it might be worth it to add some sanity checks to `pull_request_template.md`, such as: ```markdown * [ ] I have verified that each new mkdir line refers to a directory * [ ] I have verified that each new mkfile line refers to a file ``` @rusty-snake Thoughts on that?

gitea-mirror commented

2026-05-05 09:10:25 -06:00

Author

Owner

@rusty-snake commented on GitHub (Feb 13, 2021):

And I suppose that there's unfortunately no way to automatically detect those
without prior knowledge of the paths

Heuristic: ${HOME}/.config/*rc should have mkfile

but it might be worth it to add some
sanity checks to pull_request_template.md, such as:
* [ ] I have verified that each new mkdir line refers to a directory
* [ ] I have verified that each new mkfile line refers to a file

If we add checks for an option (mkfile) that is used by 32 out of 1112 profiles, we should add checks for the remaining 1080 cases too. Is to say: No, just compare on how many PRs we added "add it to firecfg.config" and on how many "this should be mkfile".

@rusty-snake commented on GitHub (Feb 13, 2021): > And I suppose that there's unfortunately no way to automatically detect those without prior knowledge of the paths Heuristic: `${HOME}/.config/*rc` should have `mkfile` > but it might be worth it to add some > sanity checks to `pull_request_template.md`, such as: > > ```gfm > * [ ] I have verified that each new mkdir line refers to a directory > * [ ] I have verified that each new mkfile line refers to a file > ``` If we add checks for an option (mkfile) that is used by 32 out of 1112 profiles, we should add checks for the remaining 1080 cases too. Is to say: No, just compare on how many PRs we added "add it to firecfg.config" and on how many "this should be mkfile".

gitea-mirror commented

2026-05-05 09:10:26 -06:00

Author

Owner

@kmk3 commented on GitHub (Feb 13, 2021):

And I suppose that there's unfortunately no way to automatically detect
those without prior knowledge of the paths

Heuristic: ${HOME}/.config/*rc should have mkfile

That sounds good. To add to that, for heuristics I think it would probably
make sense to emit a warning (rather than error), just in case there's
something like ${HOME}/.config/arc/config.

but it might be worth it to add some
sanity checks to pull_request_template.md, such as:
* [ ] I have verified that each new mkdir line refers to a directory
* [ ] I have verified that each new mkfile line refers to a file
If we add checks for an option (mkfile) that is used by 32 out of 1112
profiles,

If you consider mkdir as well, there's quite a bit more:

$ git show --pretty='%h %ai %s' -s
3eac07647 2021-02-13 20:14:28 +0000 Merge pull request #3864 from haraldkubota/master
$ find etc/inc/ etc/profile-* -type f | wc -l
1102
$ grep -Flr mkfile etc/inc etc/profile-* | wc -l
31
$ grep -Flr mkdir etc/inc etc/profile-* | wc -l
371

To be clear, the sanity checks would imply checking for both of these errors:

mkdir file
mkfile dir

we should add checks for the remaining 1080 cases too.

What do you mean? Are you saying that it's not worth it because not many
profiles are affected?

Is to say: No, just compare on how many PRs we added "add it to
firecfg.config" and on how many "this should be mkfile".

I don't understand; do you mean that only profiles that are listed on or added
to firecfg.config should be validated? Or that this isn't a big enough of a
problem to warrant the checks?

To clarify, I didn't mean that performing the checks should be mandatory; they
would primarily serve as a reminder of something to be aware of when creating a
PR. Like the checkslist on bug_report.md, which is usually not fully
completed by the issue reporters.

@kmk3 commented on GitHub (Feb 13, 2021): > > And I suppose that there's unfortunately no way to automatically detect > > those without prior knowledge of the paths > > Heuristic: `${HOME}/.config/*rc` should have `mkfile` That sounds good. To add to that, for heuristics I think it would probably make sense to emit a warning (rather than error), just in case there's something like `${HOME}/.config/arc/config`. > > but it might be worth it to add some > > sanity checks to `pull_request_template.md`, such as: > > ``` > > * [ ] I have verified that each new mkdir line refers to a directory > > * [ ] I have verified that each new mkfile line refers to a file > > ``` > > If we add checks for an option (mkfile) that is used by 32 out of 1112 > profiles, If you consider `mkdir` as well, there's quite a bit more: ```console $ git show --pretty='%h %ai %s' -s 3eac07647 2021-02-13 20:14:28 +0000 Merge pull request #3864 from haraldkubota/master $ find etc/inc/ etc/profile-* -type f | wc -l 1102 $ grep -Flr mkfile etc/inc etc/profile-* | wc -l 31 $ grep -Flr mkdir etc/inc etc/profile-* | wc -l 371 ``` To be clear, the sanity checks would imply checking for both of these errors: ```firejail mkdir file mkfile dir ``` > we should add checks for the remaining 1080 cases too. What do you mean? Are you saying that it's not worth it because not many profiles are affected? > Is to say: No, just compare on how many PRs we added "add it to > firecfg.config" and on how many "this should be mkfile". I don't understand; do you mean that only profiles that are listed on or added to firecfg.config should be validated? Or that this isn't a big enough of a problem to warrant the checks? To clarify, I didn't mean that performing the checks should be mandatory; they would primarily serve as a reminder of something to be aware of when creating a PR. Like the checkslist on `bug_report.md`, which is usually not fully completed by the issue reporters.

gitea-mirror commented

2026-05-05 09:10:28 -06:00

Author

Owner

@rusty-snake commented on GitHub (Feb 13, 2021):

I mean that the wrong usage of mkdir/mkfile is a very rare mistake in profile PRs (nearly very mk* is a mkdir) in contrast to missing update of firecfg.config or similar which is remarked on a lot PRs. It's not worth to add it except we add a list with more then 50 reminders. Meaning if we add such reminders it makes no sense to me to add reminders for rare mistakes rather then common mistakes. Sure, we can add the rare and the common one, however then we have this long list of reminders.

@rusty-snake commented on GitHub (Feb 13, 2021): I mean that the wrong usage of mkdir/mkfile is a very rare mistake in profile PRs (nearly very mk* is a mkdir) in contrast to missing update of firecfg.config or similar which is remarked on a lot PRs. It's not worth to add it except we add a list with more then 50 reminders. Meaning if we add such reminders it makes no sense to me to add reminders for rare mistakes rather then common mistakes. Sure, we can add the rare and the common one, however then we have this long list of reminders.

gitea-mirror commented

2026-05-05 09:10:29 -06:00

Author

Owner

@kmk3 commented on GitHub (Feb 14, 2021):

I mean that the wrong usage of mkdir/mkfile is a very rare mistake in profile
PRs (nearly very mk* is a mkdir) in contrast to missing update of
firecfg.config or similar which is remarked on a lot PRs. It's not worth to
add it except we add a list with more then 50 reminders. Meaning if we add
such reminders it makes no sense to me to add reminders for rare mistakes
rather then common mistakes. Sure, we can add the rare and the common one,
however then we have this long list of reminders.

I see; that makes sense. Casting aside the template idea and to expand a bit
on the rationale: the main thing that worries me with mkdir/mkfile is that the
errors can be rather subtle (especially if e.g.: a file does not match *rc),
such that likely only people who use the program/profile would notice, which
might not include any the reviewers.

Also, it's a little far-fetched, but it might still be useful to later add a
file elsewhere with the 50+ reminders. I probably couldn't name half of that,
so it would be interesting to see the known pitfalls. Maybe we could also
enumerate and refer to them like CWEs when reviewing PRs, which would
enable counting how often each one happens.

@kmk3 commented on GitHub (Feb 14, 2021): > I mean that the wrong usage of mkdir/mkfile is a very rare mistake in profile > PRs (nearly very mk\* is a mkdir) in contrast to missing update of > firecfg.config or similar which is remarked on a lot PRs. It's not worth to > add it except we add a list with more then 50 reminders. Meaning if we add > such reminders it makes no sense to me to add reminders for rare mistakes > rather then common mistakes. Sure, we can add the rare and the common one, > however then we have this long list of reminders. I see; that makes sense. Casting aside the template idea and to expand a bit on the rationale: the main thing that worries me with mkdir/mkfile is that the errors can be rather subtle (especially if e.g.: a file does not match `*rc`), such that likely only people who use the program/profile would notice, which might not include any the reviewers. Also, it's a little far-fetched, but it might still be useful to later add a file elsewhere with the 50+ reminders. I probably couldn't name half of that, so it would be interesting to see the known pitfalls. Maybe we could also enumerate and refer to them like [CWEs][1] when reviewing PRs, which would enable counting how often each one happens. [1]: https://cwe.mitre.org/

gitea-mirror commented

2026-05-05 09:10:29 -06:00

Author

Owner

@rusty-snake commented on GitHub (Feb 14, 2021):

Maybe we could also enumerate and refer to them like CWEs when reviewing PRs, which would enable counting how often each one happens.

This would also have a documenting nature, like "foo is know to break bar" where it's not obviously (nobar breaks bar). And it could be used to document the interaction (complementary and contradictory) of commands, so that situations like https://github.com/netblue30/firejail/issues/3877#issuecomment-761596394 where I know that it works this way but can not tell you way it works this way, are better to understand.

@rusty-snake commented on GitHub (Feb 14, 2021): > Maybe we could also enumerate and refer to them like CWEs when reviewing PRs, which would enable counting how often each one happens. This would also have a documenting nature, like "foo is know to break bar" where it's not obviously (nobar breaks bar). And it could be used to document the interaction (complementary and contradictory) of commands, so that situations like https://github.com/netblue30/firejail/issues/3877#issuecomment-761596394 where I know that it works this way but can not tell you way it works this way, are better to understand.

gitea-mirror commented

2026-05-05 09:10:30 -06:00

Author

Owner

@rusty-snake commented on GitHub (Mar 8, 2021):

Do we want to keep this open until someone tested and fixed those hardening changed or should we close?

@rusty-snake commented on GitHub (Mar 8, 2021): Do we want to keep this open until someone tested and fixed those hardening changed or should we close?

gitea-mirror commented

2026-05-05 09:10:30 -06:00

Author

Owner

@rusty-snake commented on GitHub (May 12, 2021):

I'm closing here due to inactivity.

@rusty-snake commented on GitHub (May 12, 2021): I'm closing here due to inactivity.

gitea-mirror referenced this issue

2026-05-05 10:18:25 -06:00

[PR #2488] [MERGED] Add new profile for regextester #4351

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/firejail#2488

No description provided.

Rows
Columns