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

Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable when PR from a fork to upstream #99

Open
hk21702 opened this issue Jun 2, 2024 · 3 comments

Comments

@hk21702
Copy link

hk21702 commented Jun 2, 2024

The ACTIONS_ID_TOKEN_REQUEST_URL env variable appears to be unavailable if this action is run on a pull request from a fork into an upstream branch.

It works perfectly fine when ran in the fork itself, or if the pull request is from a branch belonging to the upstream repository itself. It could be my mistake with configuration, or just a misunderstanding of how the action is supposed to work on my part. It'd be great if a fix could be provided for this, or at least a workaround where for this specific situation the step is skipped.

Here is an example of a run with the situation I'm describing: https://github.com/RimSort/RimSort/actions/runs/9338649185

@hk21702 hk21702 changed the title Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable when PR from a fork Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable when PR from a fork to upstream Jun 2, 2024
@phillmv
Copy link
Contributor

phillmv commented Jun 3, 2024

Hi, and thanks for trying out our feature.

What a puzzling error! Glancing over your workflows, it sure looks like you're doing everything right.

That said… this error smells like a permissions issue, and once I read "from a fork" my spidey sense started tingling; for security & abuse prevention reasons, workflows in forks have all sorts of not-necessarily-intuitive behaviour.

I looked this up in our public docs and found the following link: https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs#changing-the-permissions-in-a-forked-repository

You can use the permissions key to add and remove read permissions for forked repositories, but typically you can't grant write access.

Which would explain why the workflow is not fetching the id-token. Is it possible that this setting is not enabled?

Caveat emptor: I suggest thinking carefully about what you might be enabling if you turn this feature on, i.e. you might inadvertently allow project outsiders to push packages / you might want to consider how to adequately scope permissions or workflow triggers (which I see some evidence of in the build file). I myself am not an expert on how to structure this properly, and am just raising the possibility.

@hk21702
Copy link
Author

hk21702 commented Jun 4, 2024

Thanks for the suggestion! It does seem like a permission issue. I think a potentially safe way of doing this would be to just disable the attestation step for this specific case? Or at least let it safely fail only in this specific case. I'm not sure how to do this such that it safely fails in the use case of a PR into upstream from a fork but not within a fork itself.

@phillmv
Copy link
Contributor

phillmv commented Jun 5, 2024

I think a potentially safe way of doing this would be to just disable the attestation step for this specific case?

Without looking into your project specifically, my recommendation is as follows:

  1. you want to attest any built artifacts that are distributed to end-users
  2. often we build artifacts as part of the pull request process
  3. but this model is kinda awkward in the context of open pull requests, SO:
  • What if you move the build artifact step to a workflow that is fired after merges are made to your main branch? That way you step over any permissions issues since it only fires in the context of your repo, not in the fork, and after a project maintainer has blessed it.

(if you're building artifacts in a pull request where the tokens don't have write access this might be wasted effort/compute anyways 🤔, cos the artifacts will by definition be thrown away)

s1204IT added a commit to Chipppppppppp/LIME that referenced this issue Jun 7, 2024
プルリク時に確実に失敗するため

actions/attest-build-provenance#99
s1204IT added a commit to Chipppppppppp/LIME that referenced this issue Jun 8, 2024
プルリク時に確実に失敗するため

actions/attest-build-provenance#99
Chipppppppppp pushed a commit to Chipppppppppp/LIME that referenced this issue Jun 9, 2024
プルリク時に確実に失敗するため

actions/attest-build-provenance#99
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

No branches or pull requests

2 participants