Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add predicates for human reviews #151

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add predicates for human reviews #151

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e1db1d9
Add predicates for human reviews
adityasaky Jun 30, 2022
a6edb01
Review updates
adityasaky Jul 14, 2023
9f98b89
Remove generic review type
adityasaky Jul 14, 2023
10614e7
(WIP) Address comments
adityasaky Jul 28, 2023
c29b165
(WIP) Add target data to VCS reviews
adityasaky Aug 21, 2023
0475f64
(WIP) Fix field names, note versioning
adityasaky Aug 21, 2023
d39f850
Add mergeBase field to VCS predicate
adityasaky Sep 11, 2023
545dc62
(WIP) Address Marcela's comments on crev
adityasaky Sep 19, 2023
8f6a088
(WIP) Address Marcela's comment on VCS predicate
adityasaky Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions protos/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ predicates have protobuf definitions:
chain attributes.
- [Test Result]: Expresses the result of a test run in the software supply
chain.
- [Human Review]: Generic predicate for results of human reviews.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
- [VCS Review]: Predicate for approval reviews issued on VCS and other code
review systems.
- [Crev Review]: Predicate for [Crev] dependency reviews.

## Supported language bindings

Expand Down Expand Up @@ -52,6 +56,10 @@ testing the supported language bindings.
[SLSA Verification Summary]: in_toto_attestation/predicates/vsa/
[in-toto Link]: in_toto_attestation/predicates/link/
[Test Result]: in_toto_attestation/predicates/test_result/
[Human Review]: in_toto_attestation/predicates/human_review/
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
[VCS Review]: in_toto_attestation/predicates/human_review/vcs
[Crev Review]: in_toto_attestation/predicates/human_review/crev
[Crev]: https://github.com/crev-dev/crev
[documentation]: ../docs/protos.md
[go]: ../go/
[python]: ../python/
Expand Down
30 changes: 30 additions & 0 deletions protos/in_toto_attestation/predicates/human_review/crev/v0/crev.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
syntax = "proto3";

package in_toto_attestation.predicates.human_review.crev.v0;

import "google/protobuf/timestamp.proto";

option go_package = "github.com/in-toto/attestation/go/predicates/human_review/crev/v0";
option java_package = "io.github.intoto.attestation.predicates.human_review.crev.v0";

message Reviewer {
string id_type = 1;

string id = 2;

string url = 3;
}

message Review {
string result = 1;

google.protobuf.Timestamp reviewTime = 2;

Reviewer reviewer = 3;

string thoroughness = 4;

string understanding = 5;

string comment = 6;
}
24 changes: 24 additions & 0 deletions protos/in_toto_attestation/predicates/human_review/vcs/v0/vcs.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
syntax = "proto3";

package in_toto_attestation.predicates.human_review.vcs.v0;

import "google/protobuf/struct.proto";
import "google/protobuf/timestamp.proto";
import "in_toto_attestation/v1/resource_descriptor.proto";

option go_package = "github.com/in-toto/attestation/go/predicates/human_review/vcs/v0";
option java_package = "io.github.intoto.attestation.predicates.human_review.vcs.v0";

message Review {
repeated in_toto_attestation.v1.ResourceDescriptor reviewers = 1;

in_toto_attestation.v1.ResourceDescriptor targetTip = 2;

in_toto_attestation.v1.ResourceDescriptor mergeBase = 3;

string review_link = 4;

google.protobuf.Timestamp reviewTime = 5;

google.protobuf.Struct annotations = 6;
}
8 changes: 8 additions & 0 deletions spec/predicates/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ our [vetting process], and may be of general interest:
- [SPDX]: SPDX-formatted BOM for software artifacts.
- [CycloneDX]: CycloneDX BOM for software artifacts.
- [Test Result]: A generic schema to express results of any type of tests.
- [Human Review]: To describe findings of human reviews such as code reviews,
dependency audits, and more.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
- [VCS Code Review]: To describe code review approvals for review and version
control systems.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
- [Crev]: To describe community reviews as part of the Crev project for
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
software dependencies.

[CycloneDX]: https://cyclonedx.org/
[Link]: link.md
Expand All @@ -36,3 +42,5 @@ our [vetting process], and may be of general interest:
[Test Result]: test-result.md
[in-toto 0.9]: https://github.com/in-toto/docs/blob/master/in-toto-spec.md#44-file-formats-namekeyid-prefixlink
[vetting process]: ../../docs/new_predicate_guidelines.md#vetting-process
[VCS Code Review]: human-review-vcs.md
[Crev]: human-review-crev.md
161 changes: 161 additions & 0 deletions spec/predicates/human-review-crev.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
# Predicate type: Dependency Reviews (crev)

Type URI: (tentative) https://in-toto.io/attestation/human-review/crev/v0.1

Note: At the time of writing, the crev project uses a separate versioning
scheme, with the current version being -1. In future, this predicate may adopt
a URI associated with that project.

Version: v0.1.0

## Purpose

This attestation type is used to describe the results of human review of
dependency source code. The format is based on the
[crev project](https://github.com/crev-dev/crev).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this new predicate, I was completely unfamiliar with Crev. To provide extra context for other in-toto users, I think it would be very helpful to give a short description of what Crev is, why it's useful, and how developers can use it. It might even be useful to point to the Rust implementation as an example.

Also, this description only mentions the dependency source code aspect of Crev, though the rest of this spec, and especially the examples, do include the package review capabilities of Crev as well. I'd clarify this.


## Use Cases

As noted above, crev enables social review of popular open source software
dependencies. A crev review includes information such as the thoroughness of
the review, understanding of the source code, and a final rating.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved

## Model

Most modern software have external dependencies. Dependency review is the
process of reviewing and verifying the source code of a particular
dependency, and can be performed by one or more of several actors in the supply
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
chain. The developer importing a new dependency can perform the review or a
dedicated security team can be tasked with it.

## Schema
adityasaky marked this conversation as resolved.
Show resolved Hide resolved

```json
{
"_type": "https://in-toto.io/Statement/v1",
"subject": [{...}],
"predicateType": "https://in-toto.io/attestation/human-review/crev/v0.1",
"predicate": {
"result": "positive|negative",
adityasaky marked this conversation as resolved.
Show resolved Hide resolved
"reviewer": {
"idType": "crev",
"id": "<ID>",
"url": "<URL>"
},
"reviewTime": "<TIMESTAMP>",
"thoroughness": "high|medium|low",
"understanding": "high|medium|low",
"comment": "<STRING>"
}
}
```

### Parsing Rules

This predicate follows the
[in-toto Attestation Framework's parsing rules](../v1/README.md#parsing-rules).

### Fields

The subject of this predicate type is a specific package and its version in some
ecosystem.
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be a bit more specific and say that the subject corresponds to the "package" field of a Crev Package Review Proof.


`result`, _enum_, _required_

Specifies if the overall rating of the dependency is `positive` or `negative`.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved

`reviewer` _object_, _required_

Identifies the reviewer. This has some meaning for crev's trust proliferation
aspects, but the identity of the reviewer can also be mapped based on in-toto's
functionary handling. `idType` is used to determine the contents of `reviewer`.
The `url` is a reference to the reviewer's crev-proofs repository.
Comment on lines +75 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question and a suggestion. Q: What are possible idType values, and who determines these? Rec: I'd add in here that this field is intended to correspond to the from field in the Code Review Proofs format when the idType matches the ID for Crev.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bit of a disconnect here, because the crev code review type includes from but missing from the package review type. Same for date. I suspect date may be omitted as a package-wide review is meant as an ongoing document with updates as new versions / advisories emerge. On the other hand, I'm not sure about the from field. I wonder if in the in-toto context we want to drop it and lean entirely on the signature. That takes us to a broader conversation about how in-toto supports crev, though.


`reviewTime` _Timestamp_, _required_

Indicates time of review creation. `timestamp` in the original crev
specification.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved

`thoroughness` _enum_, _required_

Describes how thorough the reviewer was. Must be set to one of `low`, `medium`,
or `high`.

`understanding` _enum_, _required_

Describes the reviewer's understanding of the dependency code. Must be set to
one of `low`, `medium`, or `high`.
adityasaky marked this conversation as resolved.
Show resolved Hide resolved

`comment` _string_, _optional_

Optional field with any other comments a reviewer may have about the
dependency.
Comment on lines +97 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which consumers, if any, of this predicate need to be able to parse/care about the contents of the comment field? This would be helpful info.


## Example

In the first example, the crev attestation is generated for a dependency using
its binary release. Therefore, it applies to the dependency artifact that will
be fetched from some repository, in this case the Python Packaging Index.

```json
{
"_type": "https://in-toto.io/Statement/v1",
"subject": [
{
"name": "in-toto",
"uri": "purl+pkg:pypi/in-toto@1.3.2",
"digest": {
"sha256": "aa12e63298425cfc4773ed03febd68a384c63b2690959dd788f8c4511ea97bbe"
},
"downloadLocation": "https://github.com/in-toto/in-toto/releases/download/v1.3.2/in_toto-1.3.2-py3-none-any.whl"
},
],
"predicateType": "https://in-toto.io/attestation/human-review/crev/v0.1",
"predicate": {
"result": "positive",
"reviewer": {
"idType": "github",
"id": "adityasaky",
"url": "https://github.com/adityasaky/crev-proofs"
},
"reviewTime": "2023-03-16T00:09:27Z",
"thoroughness": "high",
"understanding": "high",
"comment": "This dependency is well written and can be used safely."
}
}
```

Alternatively, the attestation can be generated for the _source_ of a
dependency. In this case, either the source must then be built locally to
generate the binary or the binary must be accompanied by a separate in-toto link
or SLSA Provenance attestation that shows the binary was built from the same,
reviewed source.

```json
{
"_type": "https://in-toto.io/Statement/v1",
"subject": [
{
"name": "in-toto-v1.3.2",
"uri": "https://github.com/in-toto/in-toto/releases/tag/v1.3.2",
"digest": {
"gitTag": "58ffc2e38382b2a180e542c4933e7befd1e352e8"
}
},
],
"predicateType": "https://in-toto.io/attestation/human-review/crev/v0.1",
"predicate": {
"result": "positive",
"reviewer": {
"idType": "github",
"id": "adityasaky",
"url": "https://github.com/adityasaky/crev-proofs"
},
"reviewTime": "2023-03-16T00:09:27Z",
"thoroughness": "high",
"understanding": "high",
"comment": "This dependency is well written and can be used safely."
}
}
```
Loading