-
Notifications
You must be signed in to change notification settings - Fork 141
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.
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
chore: update docs for verifier version #1825
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Why is it complaining about
|
It looks like
And we run various actions and scripts from that checkout.
I'm actually not sure what the right thing to do is here but it feels wrong to me that we are running actions and scripts from the checkout. We should just be using the source code from the checkout to build the builder (I think?). |
Hmm I assumed that this would checkout the source code from the action reference like here
But yes, it appears that its pulling v1.2.2 for the action -- when in realtime, we are usually using and then pull the builder from v1.2.2 -- this is the one that downloads from the release and uses slsa verifier. I think we should split generate-builders inputs into source-repository, source-ref, and builder-repository, builder-ref. Otherwise, we have no way of updating this TBH. I think in production those inputs will likely be the same, but for testing, we can't separate this out correctly. |
Updated to include an optional |
Signed-off-by: Asra Ali <asraa@google.com>
…a-github-generator repo Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Blocked by #1836 |
This fixes the behavior of detect-workflow-js for PRs against slsa-github-generator, where the ref should not be the refs/pulls/ but rather the head_sha of the PR. Adds a test. This wasn't detected before, and then in PR #1825 I had to add an explicit `git checkout` when compiling the builder from source at the particular builder-ref, which comes from the detect-workflow action. See this run: https://github.com/slsa-framework/slsa-github-generator/actions/runs/4449301907/jobs/7813310086 Before this, secure-builder-checkout was OK checkout out at a PR ref - I think it was checkout out the merge PR. --------- Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
…changed action Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
|
||
COMPILE_BUILDER: "${{ inputs.compile-builder }}" | ||
BUILDER_REF: "${{ inputs.ref }}" | ||
BUILDER_REF: "${{ inputs.builder-ref || inputs.ref }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment why we need the ||
. By default we take the builder-ref
but if empty we take the ref
, why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only relevant for tests - where the builder-ref may be different than the source-ref.
When testing this, we need to use a builder-ref that already exists, while the source-ref indicates what source code reference to use for the generate-builder.sh
code.
When we are not testing, it is OK to use the generate-builder.sh
code for that builder we want to checkout - they will be the same.
In pre-submit, we have no way of testing the current generate-builder.sh
at main with no compile. Because we don't have a release at main.
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
See script slsa-framework/slsa-verifier#530 for SHA verification