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.

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

Handle tags containing "@" character in buildSLSAProvenancePredicate #1863

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

meriadec
Copy link
Contributor

When using some monorepo-related tools (like changesets), the produced tags have a special format that includes @ character.

For example, a foo package on a monorepo will produce Git tags looking like foo@1.0.0 if using changesets.

When used in combination with actions/attest-build-provenance, the action was not properly re-crafting the tag in buildSLSAProvenancePredicate because it was always splitting the workflow ref by @ and taking the second element.

This result in this error on CI:

Error: Error: Failed to persist attestation: Invalid Argument - values do not match: refs/tags/foo != refs/tags/foo@1.0.0 - https://docs.github.com/rest/repos/repos#create-an-attestation

This PR slightly update the logic there, and rather take "everything located after the first '@'". This shouldn't introduce any breaking change, while giving support for custom tags.

I've added the corresponding test case, it passes, however I couldn't successfully run the full test suite (neither on main). Looking forward for CI outcome. For the test, I tried to minimize the diff and had to introduce a small helper mockIssuer, let me know if it's fine for you.

Thanks in advance for the review 🙏.

When using some monorepo-related tools (like [changesets](https://github.com/changesets/changesets)),
the produced tags have a special format that includes `@` character.

For example, a `foo` package on a monorepo will produce Git tags looking
like `foo@1.0.0` if using changesets.

When used in combination with `actions/attest-build-provenance`, the
action was not properly re-crafting the tag in `buildSLSAProvenancePredicate` because
it was always splitting the workflow ref by `@` and taking the second
element.

This result in this error on CI:

```
Error: Error: Failed to persist attestation: Invalid Argument - values do not match: refs/tags/foo != refs/tags/foo@1.0.0 - https://docs.github.com/rest/repos/repos#create-an-attestation
````

This PR slightly update the logic there, and rather take "everything
located after the first '@'". This shouldn't introduce any breaking
change, while giving support for custom tags.

I've added the corresponding test case, it passes, however I couldn't
successfully run the full test suite (neither on `main`). Looking
forward for CI outcome.

Thanks in advance for the review 🙏.
@meriadec meriadec requested a review from a team as a code owner October 30, 2024 13:37
Copy link
Contributor

@bdehamer bdehamer left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the fix!

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