No idea what the problem with the Ubuntu TeamCity build is but I'm pretty sure it's unrelated to my code.

Hmm. That must be the host that runs two agents. Somehow when I run two build agents on one machine, the second container has an incomplete configuration. Not sure why, though.

I've tested on refs/pull/3993/merge (Merge c35a8e6 into 7b95867)
With -fsanitize=address -fno-omit-frame-pointer don't brings any errors

Would like to squash all these commits, ok?

@droidmonkey Squashing is fine by me, but please scan the comments first to check if there's anything worth keeping. I think I put the source URL of the health check icon somewhere.

When I look at your screenshot, two questions come to mind:

1. Why should a password that was given an expiration date when the entry was created no longer be healthy? The quality of the password does not change after the expiration date.
2. What do positive results of the health check look like? Can you please give an example?

@4jNsY6fCVqZv

1. Why should a password that was given an expiration date when the entry was created no longer be healthy? The quality of the password does not change after the expiration date.

It depends on the individual's interpretation of what it means that a password has expired. A common interpretation is "the password is no longer valid", either because the service requires it to be renewed after a certain time (and you may lose access if you fail to do so), or because you chose to change the password after a certain time for security reasons. In either case, even though expiration doesn't change the actual password quality (as defined by its entropy), the password is no longer "healthy" after it has expired.

2. What do positive results of the health check look like? Can you please give an example?

Health check only reports passwords that are not "healthy" (=that have an issue of some sort). "Positive" as in "all passwords are healthy, no action required" looks like this:

In a medical context that would actually be called a "negative" result, so I hope I understood your question correctly. You know, if you get tested for a disease, a "negative" test result (=no disease found) is "positive" (=good) for you, and vice-versa.

BTW, I kind of cheated, just added that message as a result of your question. Thanks for pointing it out.

@wolframroesler
As for the second part of your answer, I agree with you as far as the medical point of view is concerned. Thank you for taking up my idea! And thanks for working on the feature!

EDIT Maybe it still needs a little styling. For example, a title line does not seem necessary to me. There is quite a lot of white space. And the hint "Hover over reason..." below the widget is probably not necessary either, right? Somehow I also find it strange that the sentence is completely left-aligned. With no space to the left. Do you have any ideas how to make this look more appealing?

the password is no longer "healthy" after it has expired.

For the first part: I would like a clearer solution here.
A password that has expired, for whatever reason, should perhaps be marked as such separately. It might not be functional anymore or it might want to be replaced by users at their own request after a given time. But in my understanding this does not mean that it deserves "red alert". This means that it has the same value as an insecure password, a password that has been assigned multiple times or even a leaked password, although it may still be completely functional and secure.

Could you imagine that expired passwords would simply be marked differently and users would instead be given the opportunity to renew it after a health check?

But the main window already shows this information, right?
So maybe it would be an idea to include only expired passwords in the health check if they also meet the criteria of insecurity. And on that basis be sorted into the color scale accordingly.

As an example from your screenshot "GitHub". It would suggest: "Weak password -> line break -> Password has expired". Additionally, it's crossed out, like in the main window, so there's a coherence here.

How do you feel about my idea?

A simple checkbox would suffice: "Evaluate expired passwords"

@4jNsY6fCVqZv Well, since the KeePass system has a data field named "expired" but doesn't define what what exactly it is supposed to mean, everyone's free to use their own definition, however the fact that entries with expired password are shown crossed out in the entry list indicates that there's some significance to it. Also, flagging expiration as a serious issue in Health Check was one of the initial requirements requested in this issue (see comments above).

I don't feel too good about making Health Check configurable. People may be thinking that everything's OK because of a clean Health Check, when in fact they just forgot about some check mark. Also, a check box for expired passwords is just the beginning, someone could have the idea of making the entire scoring system configurable, and I don't want to open that can of worms. (Well, not now, at least.)

Let's talk about actual use cases. What exactly are your criteria to set an expiration date for a password? (This is a question for everyone.)

Could you imagine that expired passwords would simply be marked differently and users would instead be given the opportunity to renew it after a health check?

You can do that already, double-clicking the line in Health Check puts you right into the entry editor. Marking expired passwords differently isn't so easy because how about passwords that are both expired and have poor entropy? The "Reason" column already says "password has expired", that's all the information people need IMHO.

To me I set an expiration on an entry if it needs to be changed by that time, or actually expires at that time (eg, credit card).

I agree with your comments!

@wolframroesler

My idea was not to make the health check configurable. It was:

So maybe it would be an idea to include only expired passwords in the health check if they also meet the criteria of insecurity. And on that basis be sorted into the color scale accordingly.
As an example from your screenshot "GitHub". It would suggest: "Weak password -> line break -> Password has expired". Additionally, it's crossed out, like in the main window, so there's a coherence here.

What do you say concerning my comments under "EDIT"?

As far as the use cases are concerned, I agree with @droidmonkey .
Usually, I will only set a specific expiration date for passwords if I know that a particular service is likely to expire at a certain point in time or will need to be renewed in some way around that time. In such a case, however, the default behavior of KeePassXC for expired passwords seems sufficient. Having an expired password for a service that no longer works in the password manager does not make it an "unhealthy" password for me.
Another use case I could imagine is to set an expiration date if I don't trust the service not to properly secure my secure passwords. Or if such a service does not allow really secure passwords due to strange character and length restrictions.
In the latter case, however, this would fit my suggested above: A password is only "unhealthy" if it has expired and at the same time is not secure (e.g. sufficient entropy).
A supposedly secure password should not be considered "unhealthy" just because it has expired.
And, to reiterate your advice about configurability: I would, of course, set this as default behavior. Not customizable.

@4jNsY6fCVqZv

So maybe it would be an idea to include only expired passwords in the health check if they also meet the criteria of insecurity. [...] A supposedly secure password should not be considered "unhealthy" just because it has expired.

Well, if I'm about to lose access to a service because my password expires tomorrow, I consider that password "unhealthy" even if it has high entropy. In this regard I'd rather err on the side of caution and report it; also, the current behavior was explicitly requested in the requirements (see comments above).

It would suggest: "Weak password -> line break -> Password has expired".

Assuming that "It" is a typo and you meant "I would suggest", that's exactly how it's implemented now. However, crossing it out additionally would be visual overload IMHO.

Perhaps we can add an "ignore" function to tell KeePassXC to not include certain entries in health reports?

I squashed everything and will do a review for merging.

Sorry if this question is a bit stupid, but when is this feature available in the KeePassXC Snapshot Builds (https://snapshot.keepassxc.org/latest/) ?
In "Database Settings" I see no "Health Check".

When it is merged to the develop branch

I made several substantive changes. One thing to note, the cache in HealthChecker is never used. We can get rid of it, the checking process is extremely fast even with my main database.

One thing to note, the cache in HealthChecker is never used.

You're right. I'll remove it because YAGNI.

As a point of reference please don't merge in develop to feature branches. We prefer to rebase on top of develop.

One thing to note, the cache in HealthChecker is never used.

Removed the cache. I suggest you leave this commit un-squashed so we can recover the cache code in case we need it again in the future.

Sure that is a good compromise.

Sorry if this question is a bit stupid, but when is this feature available in the KeePassXC Snapshot Builds (https://snapshot.keepassxc.org/latest/) ?
In "Database Settings" I see no "Health Check".

same question

Sorry if this question is a bit stupid, but when is this feature available in the KeePassXC Snapshot Builds (https://snapshot.keepassxc.org/latest/) ?
In "Database Settings" I see no "Health Check".

same question

It's under Database Reports.