I'm not sure what the advantage of splitting everything into two classes is. Couldn't you just return DiagnosticResult::STATUS_WARNING for the ones you consider recommended?

Also I am not sure if I agree that the lang/en.json check is less important than the tmp/cache/tracker/matomocache_general.php one: Of course sharing the lang/en.json itself doesn't matter. But it is a very good indicator that the .htaccess files are not working (as they protect lang/en.json) and therefore more important files are also public.
And it is more reliable to check as we know the content of lang/en.json and therefore if the HTTP response contains 12HourClock we know for sure that the file is public (checking the content was my original idea in the check I implemented in https://github.com/Findus23/matomo-DiagnosticsExtended/blob/main/Diagnostic/URLCheck.php).

@Findus23 we definitely don't want to show a big security warning when lang/en.json is accessible especially because that likely means they are not using apache etc. Are you saying we should move tmp/cache/tracker/matomocache_general.php" to the not required check? If there are more important files, may be good to create an issue for these to add them? (as long as it's not too many as things can be slow). That might be good in a different issue though.

Isn't this check not mainly a reminder for people not using Apache (and those using Apache with .htaccess disabled) that they need to manually take care of protecting files?
I'm not sure what is exactly in the cache and if there is something in those files that might help attackers.
Then again it isn't that important what exactly we check as long as it points people that they should check their setup.

I have implemented translation and added it for de and en. Not sure the wording is perfect but should give an impression.

Also, I noticed that the translations are handled differently in the other code. Is this a legacy issue? Should I change that?

See https://github.com/matomo-org/matomo/blob/m-17577/plugins/Diagnostics/Diagnostic/RequiredPrivateDirectories.php#L40 for example.

@geekdenz could we refactor this code so that we don't have duplicate code in both classes? Maybe one class could extend the other or so.

Agree. It should be refactored. I was thinking to provide an abstract class that both extend.

Also I think German translations we cannot provide directly in Github as they are managed through https://www.transifex.com/matomo/matomo/ by translators. In Github we only provide default translation in English.

Oh! Noted. Should I revert that change?

Oh! Noted. Should I revert that change?

That be good.

@geekdenz you need to fix this PR before requesting review - it's got the same problem we fixed on another one the other day with your merge gone wrong.

@geekdenz I just started testing and was checking first thing the config file which wasn't working last time. It looks like it's still not working. This is what I'm getting when the config file is not writable. It says the above URLs are having issues but there is no URL. I won't continue testing further for now.

I haven't tested again but I think it looks good to merge if there are no tests failing because of this.

Not sure if these failing tests are related to this issue's fix, @tsteur :

https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/48437/

Looks like those are related to the plugin updates last night @geekdenz.

Hi @tsteur and @geekdenz, I'm still puzzled by the fact that I'm getting this private directories warning for my /lang/en.json file, even though I actually am running on Apache and there is a valid .htaccess file in the /lang directory.

I wrote about this earlier here:
#17577 (comment)

As I understand it, your patch will simply mark this check as recommended, rather than required, but it seems to me that the underlying issue itself won't be resolved on my site, and I'd really like to figure out why Matomo says this file is exposed in my site, even though I'm running on Apache and have the .htaccess file there.

Can either of you help with this?

Hi @GermanKiwi could you maybe paste the content of that .htaccess in the comment below and also let us know what version of Apache you are using?

Cheers

Hi @GermanKiwi could you maybe paste the content of that .htaccess in the comment below and also let us know what version of Apache you are using?

Cheers

Sure thing. Below is the contents of the .htaccess file inside the /lang directory. My site is running on Apache 2.4.41.

# This file is auto generated by Matomo, do not edit directly
# Please report any issue or improvement directly to the Matomo team.

# First, deny access to all files in this directory
<Files "*">
<IfModule mod_version.c>
	<IfVersion < 2.4>
		Order Deny,Allow
		Deny from All
	</IfVersion>
	<IfVersion >= 2.4>
		Require all denied
	</IfVersion>
</IfModule>
<IfModule !mod_version.c>
	<IfModule !mod_authz_core.c>
		Order Deny,Allow
		Deny from All
	</IfModule>
	<IfModule mod_authz_core.c>
		Require all denied
	</IfModule>
</IfModule>
</Files>

@GermanKiwi could you try to access https://yourmatomodomain.com/lang/en.json yourself and see if content is returned or maybe even what HTTP response code it returns? (you need to replace the domain with the domain/path for your Matomo install)

From our perspective things look good and should work. I would actually recommend you get in touch with SiteGround as something doesn't look right there. But let's double check first there isn't an application error. Be great to know if you can request that file or not.

@GermanKiwi could you try to access https://yourmatomodomain.com/lang/en.json yourself and see if content is returned or maybe even what HTTP response code it returns? (you need to replace the domain with the domain/path for your Matomo install)

From our perspective things look good and should work. I would actually recommend you get in touch with SiteGround as something doesn't look right there. But let's double check first there isn't an application error. Be great to know if you can request that file or not.

Yeah, if I paste the URL for /lang/en.json into my browser (corrected for my own domain, of course), then it returns the full contents of that file in the browser. :(

And if I check the HTTP response (using https://www.webconfs.com/http-header-check.php) it returns HTTP/1.1 200 OK

Incidentally, I only got a warning about this from Matomo after updating to version 4.3.0. Not sure if that's relevant.

@GermanKiwi before that we didn't have this check this is why you are seeing it only now. I recommend you get in touch with your hoster and ask them. I would assume they should be able to help you there.

The most likely thing is that in the main apache configuration file of your server overriding settings via .htaccess files is disallowed and they therefore do nothing. (And if this is the case, this check is working as it notified you of this fact)