Skip to content

In toto attestation refactor #2734

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

In toto attestation refactor #2734

wants to merge 1 commit into from

Conversation

@kpauljoseph
Copy link
Contributor

@kpauljoseph kpauljoseph commented Jul 18, 2025 •
edited
Loading

Description of the PR

PR Checklist

  • All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
  • All new changes are covered by tests
  • If GraphQL schema is changed, make generate has been run
  • If GraphQL schema is changed, GraphQL client updates/additions have been made
  • If OpenAPI spec is changed, make generate has been run
  • If ent schema is changed, make generate has been run
  • If collectsub protobuf has been changed, make proto has been run
  • All CI checks are passing (tests and formatting)
  • All dependent PRs have already been merged

@kusari-inspector
Copy link

kusari-inspector bot commented Jul 18, 2025 •
edited
Loading

Kusari Inspector

Kusari Analysis Results:

Proceed with these changes

✅ No Flagged Issues Detected
All values appear to be within acceptable risk parameters.

No pinned version dependency changes, code issues or exposed secrets detected!

@kusari-inspector rerun - Trigger a re-analysis of this PR
@kusari-inspector feedback [your message] - Send feedback to our AI and team
See Kusari's documentation for setup and configuration.
Commit: f780169, performed at: 2025-09-04T03:38:04Z

Found this helpful? Give it a 👍 or 👎 reaction!

@kpauljoseph kpauljoseph force-pushed the in-toto-attestation-refactor branch 2 times, most recently from f946fc3 to fb562d4 Compare July 18, 2025 10:12
@kusari-inspector
Copy link

kusari-inspector bot commented Jul 18, 2025

Kusari PR Analysis rerun based on - fb562d4 performed at: 2025-07-18T10:13:01Z - link to updated analysis

@kusari-inspector
Copy link

kusari-inspector bot commented Jul 18, 2025

Kusari PR Analysis rerun based on - f946fc3 performed at: 2025-07-18T10:10:17Z - link to updated analysis

@kpauljoseph kpauljoseph force-pushed the in-toto-attestation-refactor branch 2 times, most recently from 7de2874 to a66ed42 Compare July 18, 2025 10:29
@kusari-inspector
Copy link

kusari-inspector bot commented Jul 18, 2025

Kusari PR Analysis rerun based on - a66ed42 performed at: 2025-07-18T10:29:26Z - link to updated analysis

@kpauljoseph kpauljoseph force-pushed the in-toto-attestation-refactor branch from a66ed42 to bfccefa Compare July 18, 2025 10:33
@kusari-inspector
Copy link

kusari-inspector bot commented Jul 18, 2025

Kusari PR Analysis rerun based on - bfccefa performed at: 2025-07-18T10:33:06Z - link to updated analysis

@kusari-inspector
Copy link

kusari-inspector bot commented Jul 18, 2025

Kusari PR Analysis rerun based on - 7de2874 performed at: 2025-07-18T10:25:57Z - link to updated analysis

@kpauljoseph
refactor in-toto-golang to in-toto/attestation
f780169 
refactor v0.1/v0.2 slsa predicates with new attestation
protos.
update parser to use new protobuf types and timestamp handling
fix tests and remove obsolete fields from test data

Signed-off-by: Paul Joseph <[email protected]>
@kpauljoseph kpauljoseph force-pushed the in-toto-attestation-refactor branch from bfccefa to f780169 Compare September 4, 2025 03:37
@kusari-inspector
Copy link

kusari-inspector bot commented Sep 4, 2025

Kusari PR Analysis rerun based on - f780169 performed at: 2025-09-04T03:38:14Z - link to updated analysis

@kpauljoseph kpauljoseph mentioned this pull request Sep 4, 2025
Open
SantiagoTorres
SantiagoTorres requested changes Sep 4, 2025
View reviewed changes
Copy link
Collaborator

@SantiagoTorres SantiagoTorres left a comment

Choose a reason for hiding this comment

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

I think the new interface is a bit counter-intuitive coming from the old in-toto-golang, but we are removing the statement layer (I'm surprised the tests didn't explode)

pkg/ingestor/parser/slsa/parser_slsa_test.go
Recipe: slsa01.ProvenanceRecipe{
Type: "test",
},
stmt: &slsa01.Provenance{
Copy link
Collaborator

@SantiagoTorres SantiagoTorres Sep 4, 2025

Choose a reason for hiding this comment

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

This is sticking the provenance predicate at the statement layer. Instead, this should be using in-toto/attestation/go/v1/statement and adding slsa01.Provenance as the predicate

pkg/ingestor/parser/slsa/parser_slsa_test.go
expectedFinishTime := time.Unix(test.args.stmt.Metadata.BuildFinishedOn.GetSeconds(), int64(test.args.stmt.Metadata.BuildFinishedOn.GetNanos()))
if test.args.inp.FinishedOn != nil && !test.args.inp.FinishedOn.Equal(expectedFinishTime) {
t.Errorf("fillSLSA01() inp.FinishedOn not equal to expected time")
}
Copy link
Collaborator

@SantiagoTorres SantiagoTorres Sep 4, 2025

Choose a reason for hiding this comment

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

This needs to restore the nesting based on the comment above. It should also check for the subject field

@stale
Copy link

stale bot commented Nov 5, 2025

This pull request has been automatically marked as stale because it has not had recent activity (60 days of inactivity).
It will be closed in 30 days if no further activity occurs.
Thank you for your contribution!

@stale stale bot added the wontfix This will not be worked on label Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SantiagoTorres SantiagoTorres SantiagoTorres requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

size/L wontfix This will not be worked on

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kpauljoseph @SantiagoTorres