Skip to content

Conversation

@AdamKorcz
Copy link
Contributor

Adds another input for the Maven builder to allow the user to specify the project directory. The current problem this solves is to make slsa-framework/example-package#253 work.

This was suggested by @laurentsimon in slsa-framework/example-package#253 (comment)

@laurentsimon laurentsimon changed the title chore: Add directory input to Maven builder feat: Add directory input to Maven builder Aug 4, 2023
Copy link
Collaborator

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

Thanks. You could do some verification on the path to make sure it's under the workspace, something like https://github.com/slsa-framework/slsa-github-generator/blob/main/.github/actions/secure-download-artifact/action.yml#L42-L48

Signed-off-by: AdamKorcz <[email protected]>
Signed-off-by: AdamKorcz <[email protected]>
@AdamKorcz
Copy link
Contributor Author

@laurentsimon Could you have another look, please?

@ianlewis
Copy link
Member

ianlewis commented Aug 7, 2023

Thanks. You could do some verification on the path to make sure it's under the workspace, something like https://github.com/slsa-framework/slsa-github-generator/blob/main/.github/actions/secure-download-artifact/action.yml#L42-L48

TBF, I believe the chdir would happen in the "untrusted" build's job anyway. So I don't think this is super important (though necessary if it's used elsewhere).

Signed-off-by: AdamKorcz <[email protected]>
@laurentsimon
Copy link
Collaborator

@AdamKorcz friendly ping to update this PR

@AdamKorcz
Copy link
Contributor Author

@AdamKorcz friendly ping to update this PR

Thanks for the ping! Resolved the comments.

@laurentsimon laurentsimon enabled auto-merge (squash) August 15, 2023 17:55
@laurentsimon laurentsimon merged commit 324ff12 into slsa-framework:main Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants