Overwritting someone elses reviews.

[dpc]

Sometimes people give a negative review to a crate, and we conciously would like to overwrite them.

Current example. Me and other crev user reviewed smallvec and gave it a negative review, because it was ridden with unsafe and had a history of unsoundness. Then, I submitted a fuzzer to smallvec, and I think with a fuzzer we can have more confidence, and this crate does not deserve a negative rating anymore. At very least I don't want it to fail in my own crate verify output.

So I'd like to overwrite the other persons (which I still trust) review in this case. I could ask them to change their review, but that's not the point.

I am thinking, that a review could have an optional overrule/overwrite/override section with a list of reviews of other other people that should be ignored for some reason, because this review says they no longer apply, were a mistake, etc.

We could identify other reviews by:

• by id of their author
• digest
• signature

The id is the longest, but is the easiest for the humans to understand and use, so I think I'll stick with it.

For other users my override would only apply if they trust me more than then a person I'm trying to override here (more or equal maybe?).

Should be fairly easy to implement, though I could use some second opinion.

[programmerjake]

maybe also have the overriding condition be dependent on if the overridden reviewer trusts the overrider? We want to avoid override wars, so, unless special circumstances apply (very high level of trust, trust relationship between overrider and overridee, etc.), I don't think overrides should apply for cases of equal trust.

[programmerjake]

also would be nice to have local overrides that aren't published.

[BatmanAoD]

Who would be affected by an override/overwrite?

"Overwriting" sounds like changing someone else's review globally, which surely isn't what you mean. "Override" sounds like it could somehow affect the trust status of the review globally (i.e. even for users who don't have a trust relationship with the overrider), but that the review itself would still exist.

Conversely, "ignore" sounds like it would not affect anyone else, even if they have a trust relationship with the user doing the ignoring.

I'm guessing you mean something between these two extremes, i.e., ignoring a review would break transitivity, so that users who trust your reviews would not automatically have any trust in that specific review. Maybe "hide" would be a better term?

Another way of thinking about this is that you can trust someone, but disagree with them on a certain issue. In this case, you trust the reviewer in general (and used to agree), but believe the situation has changed and no longer agree with their (or your old) assessment. So this sounds like a mechanism for more granular levels of trust. Maybe it should be possible to just set trust levels for particular reviews directly, which would handle this case as well as the opposite (where you agree with a specific review but don't trust the reviewer in general).

[dpc]

@BatmanAoD

Another way of thinking about this is that you can trust someone, but disagree with them on a certain issue. In this case, you trust the reviewer in general (and used to agree), but believe the situation has changed and no longer agree with their (or your old) assessment.

This is exactly the motivation. I still trust the other reviewer (otherwise I would just change their trust level to none and be done with it), but eg. can not wait for them to re-review, reconsider, or just in this one case / in my case I want an exception.

"Overwriting" sounds like changing someone else's review globally,

Right now I think about "just ignore this one particular review".

We want to avoid override wars, so

I am not really concerned about overriding wars. If people have deep disagreements that they can't resolve, they can just stop trusting each other, and possibly other people should too. :D . This feature is purely for tactical, one time exceptions from otherwise mutually respecting parties.

@programmerjake

also would be nice to have local overrides that aren't published.

Good idea. I wonder if it should be "don't publish" or just "don't get into effect for other people".

Also, you just reminded me that I'd like this ignore (I think it's more of an ignore than override), to apply only to a specific version of a review. If the ignored author redid their review, it would require redoing current review to ignore it again.

[BatmanAoD]

Another interesting question this brings up is whether reviews should be set to auto-expire at some point, e.g. on some kind of SemVer bump.

If your and the other reviewer's original reviews had been restricted to the current minor version, the newly fuzzed version would have been implicitly "unreviewed" by both of you.

[dpc]

@BatmanAoD Reviews are version specific right now. New version is unreviewed by definition. Previous reviews can be used as a base for differential reviews, etc. but that's about it.

[BatmanAoD]

That makes a lot of sense (and, prior to this issue, is how I assumed it worked), but in that case I don't understand how the smallvec issue arose. Did the smallvec version number not get bumped after you submitted the fuzzer?

[dpc]

@BatmanAoD Not yet. There is no new release. Technically the actual code people use is the same. I just have more confidence in it than before, because my fuzzer did not catch anything.

[dpc]

I was thinking, how could I achieve the same effect without introducing a completely new, one-off feature.

First, this field does not really fit well in package review. It seems more of a trust-level exception.

Maybe trust proofs could have conditional, or exception, or something. But that makes the trust set context-dependent, which is not great.

[uberjay]

In my opinion this would be better solved in other ways. Perhaps by including an additional age-based weight to a review's trust would help? I think this specific problem is a bit of a degenerate case. (both because of the infancy of cargo crev and the size of the reviewer pool.)

I guess what I'm trying to say is, maybe this doesn't even need to be solved?

EDIT: I'm also hoping this doesn't need to be solved, since managing a cryptographic web of trust is already a pretty hard problem to solve. Adding additional, more complex ways of overriding the trust calculations isn't going to make the UX better and easier to understand. 😉

[dpc]

@uberjay I agree that this sort of stuff would have a tendency to get solved by just aggregate volume.

I very like the idea of age-based logic here. If someone with higher trust / closer in the WoT is saying something is OK and that review was also much more recent, then it is a good indicator that probably that person knew better.

This way if you disagree with something, you can always write your own review, that indicates that "you knew better". Howerver, for this to be effective crev should make sure that any contradicting information during the review was taken into account.

Example: when writting a review for crate x crev should do a final check and double-check that the author is aware of any existing issues already reported for x and not listed in the new review, and any reviews of x with ratings different then the one given and so on.

[kornelski]

I think some form of this is needed. People will make mistakes and make wrong reviews. Currently the only way to ignore a wrong review is to distrust the author, but distrusting a person for one wrong review is a nuclear option, a creates a "too big to fail" problem.

Non-code review websites have a "Helpful/Unhelpful" buttons for reviews. Such thing could be exposed in a crev GUI as well.

[kornelski]

Referring to a review is a tricky problem. If it's identified by content/signature, then small edit of the review will make it dodge other people's metadata about it. If it's identified by (crev id, package, version), then metadata will still apply even if someone flips review from negative to positive.

So I think reviews should be identified by (crev id of the reviewer, package, version, rating). This way metadata won't apply anymore if someone changes the rating, but disagreement with a negative review will remain as long as the review is negative.

[kornelski]

So I suggest having a vote proof type. You can vote yes/no for a review. Votes will be weighed by trust of who cast the vote. Reviews that cross some threshold of downvotes will be ignored.

[dpc]

Related: #329

[dpc]

Since this is sitting unaddressed for so long:

I think I'm going to add a list of ignore field in trust proof:

----- BEGIN CREV PROOF -----
kind: trust
version: -1
date: "2021-03-15T13:00:15.785981436-07:00"
from:
  id-type: crev
  id: FYlr8YoYGVvDwHQxqEIs89FYlr8YoYGVvDwHQxqEIs8
  url: "https://github.com/foob/crev-proofs"
ids:
  - id-type: crev
    id: VvDwHQxqEIs8Nt5eFtWWwvm3pVvDwHQxqEIs8bGdhm4
    url: "https://github.com/bar/crev-proofs"
trust: none
ignore:
- kind: trust
  id: Is8Nt5eFtWWwvmIs8Nt5eFtWWwvmIs8Nt5eFtWWwvm
  comment: I disagree with their judgment criteria, but I'm OK with the rest of the WoT.
- kind: package review
  source: "https://crates.io"
  name: some-package
  version: 0.1.4
  comment: I think they've missed the unsoundness of this crate.
----- SIGN CREV PROOF -----
tQZPgXr1MKGm1m8O7gs6OSUS0vUL3XGHTWzKDtC0A2JrDnCaZWp2RMtEVJkD6pN3L01Kl4Exm6BmtIgT7o8bCg
----- END CREV PROOF -----

The proof itself would mean the same thing as before, but the ignore list would mark things that in the author's opinion should be ignored when trusting the target(s) id.

From the perspective of the downstream consumers of such trust proof (direct or transitive) the ignore list will be only effective if the trust level of the author is higher than that of the target.

I think this should be enough to get some non-nuclear nuanced corrections in the WoT, easy (enough) to implement and hopefully understand.

[dpc]

Note: it's only possible to ignore the given trust / review altogether, not change or override anything about it.

Hmm.... Maybe comment should be mandatory to encourage some explanations.

ignore could be named except or maybe something else.

[kornelski]

Can you make it a new kind of proof? I wouldn't want to mess with my trust settings when I only downvote reviews.

[dpc]

Putting such data into trust proof seem to make sense because it is a part of your trust declaration. You trust given ID with a given level (possibly none), and you just add some particular exceptions/overrides for stuff you think it's better to ignore. "downvote" is also a wrong word. There is not voting. Everything is still a part of WoT and resolved according to it.

cargo-crev will already open the current existing trust proof when you try run cargo crev trust on an ID you've trusted before. Plus/minus some lame bug, there is no room for messing with anything.

When normally creating trust proofs the ignore part would be omitted (since it's empty) or shown with the existing items. When invoking the right command trust proof editing would show up filling in the right ignore item.

It is technically possible to have a separate kind of proof, but it seems to me that it would just create additional hassle for something that fits well into definition of a trust proof.

[dpc]

One thing that seems non-trivial is the exact cargo crev command invocation to let users conveniently add a new ignore item.

[dpc]

After couple of evenings of toying with it, I've finally pushed 12d2517 that is implementing it.

I had to reverse the placement of overrides. An override item with an id a on trust proof created for b means that trust from a to b is recommended to be ignored. That's the opposite from what I previously described. The reason is that it's easier to make a convenient UI for it. When editing trust proof for b cargo-crev will conveniently display (commented out) all Ids that trust a. So adding an override means just uncommenting one of the Ids. No need to invent weird command line invocation. The only thing needed is --overrides switch to show that "list of suggestions".

Similarly for package review, --overrides will make cargo-crev show a list of Ids that already reviewed given package, and all it takes is to comment out one we want to ignore.

It's important to note that unlike distrust trust level, overrides will only work if the effective trust level of person suggesting override is higher than that of the person that is being overriden. To say differently: if a created override of a trust "from b to c" (in a trust proof for c), then trust between b to c will be ignored in WoT of a person d, only if d effectively trusts a more than b.

The exact details of keyword names, etc. might still be changed, so you might not go too crazy with it just yet, but please give it a try and let me know what you think.