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

Add a new input argument pr_number to support trigger types other than pull_request? #101

Closed
leofang opened this issue Jan 13, 2025 · 2 comments

Comments

@leofang
Copy link

leofang commented Jan 13, 2025

Hi! Thank you for creating this nice action. We'd like to use it in our repo, but our project has a delicate CI so that using a standalone workflow to trigger this action following the instruction

on:
  pull_request:
    types:
      ...

jobs:
  deploy-preview:
    runs-on: ubuntu-latest
    steps:
      ...
      - uses: rossjrw/pr-preview-action@v1
        with:
         ...

is not suitable for our CI. Instead, we'd like to call this action as part of our doc build workflow, which in turn is a reusable workflow launched as part of the main CI. But it means our doc build workflow has

on:
  workflow_call:
    ...

and therefore the pr-preview-action cannot get the current PR number.

My suggestion is instead of looking it up for users based on the pull_request trigger, we allow a new input argument for users to overwrite this behavior (and this can be looked up in any workflow from the github context, I think):

on:
  workflow_call:
    ...

jobs:
  build-and-preview:
    runs-on: ubuntu-latest
    steps:
      ...
      - uses: rossjrw/pr-preview-action@v1
        with:
          pr_number: ${ user-provided-number, fetch-able from the main workflow }
          ...

Is this feasible? Thanks!

@rossjrw
Copy link
Owner

rossjrw commented Jan 14, 2025

Closing as duplicate of #6

This is something that's necessary in order to do lots of things like enable previews for forks from the pull_request_target event, or have the flexibility of the workflow_call event like you hope to leverage. But in the same way, doing this opens the doors for all sorts of potential security risks, detailed in #6.

Long story short, you'll be able to do this action once I implement #6, but I don't know when that will be.


In the meantime, your best bet is probably to look through the action.yml file here and take what you need to set up previews yourself. There's not a lot going on behind the scenes really - copy the files in the source dir to the target dir on the target branch, commit, work out where that Pages URL will be and then leave a comment. You'll probably be able to do that much more simply than this action does.

Downside is that you'll have to be super careful about untrusted input. Off the top of my head, I know that workflows on the pull_request event use the workflow files from the source branch and for pull_request_target it uses the files from the target branch - I'm not sure about workflow_call or other event types, so you'll have to research whether an attacker could potentially compromise your repo by changing the workflow files, and even if they can't, you'd still be committing untrusted content directly to your repository. You'll need to set up checks and measures to counteract this if it's something you're worried about.

@rossjrw rossjrw closed this as completed Jan 14, 2025
@rossjrw rossjrw closed this as not planned Won't fix, can't repro, duplicate, stale Jan 14, 2025
@leofang
Copy link
Author

leofang commented Jan 14, 2025

Thanks, @rossjrw, for pointers! I searched the issue tracker prior to filing this but I did not bother to search the open PRs, nor did I make a connection of my ask to enabling forks 🙂

The CI in our repo is guarded by a maintainer-initiated command (due to scarce self-hosted runner resources), so arbitrary PRs cannot be executed in the CI without approval. Also,

so you'll have to research whether an attacker could potentially compromise your repo by changing the workflow files

we've experienced that workflow files cannot be modified by a GHA, which is actually annoying because then we could not easily backport a PR using GHA if the PR touches workflow files (NVIDIA/cuda-python#365). These security settings become an obstacle to our use cases 🥲

Anyway, I agree that some steps can be copied/pasted from here. But it's just easier to use your action as-is if it's ready, because for enterprise use cases copy-pastas of 3rd-party repos require additional legal approvals which is not fun and the process can be tedious. For now I'll just rewrite the key steps myself.

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