Local validation for forked PRs

[Ishan-Jadhav]

Instead of splitting the validate.yml file, this PR introduces a streamlined approach to handle validation tasks based on the pull request lifecycle.

• When a PR is opened:

  • Local validation tasks will run immediately, ensuring the changes meet the necessary checks that do not require secrets.

• When the PR is merged:

  • Validation tasks that require secrets will execute, ensuring sensitive operations are secure and only run after the PR is approved and integrated into the main branch.

[joyeecheung]

Thanks for opening a PR!

I am not sure if the changes here are an improvement, however, since this removes full validation from PRs, so the validated URL can still be invalid/unreachable even if the PR is opened from this repository (which means it is already fine to trust the author, if they want the secret they already can push random changes to the main branch to get it). The original motivation was to support a subset of validation for PRs from forks, without removing full validation for PRs from this repo.

[Ishan-Jadhav]

Secrets are not accessible during the validation stage of the PR. Only after the PR is reviewed, approved by admins, and merged into the main branch can methods like signing in or posting access the secrets.

Full validation occurs only after the PR (both forked PRs and PRs from the same repo) is merged. The PR workflow ensures partial validation when a PR (both forked PRs and PRs from the same repo) is opened, restricting access to secrets.

If it is required that forked PRs should only access certain files, validation checks can be implemented to restrict their file modifications. For example, forked PRs can be limited to creating only JSON files, with other file types being disallowed

[joyeecheung]

Secrets are accessible for PRs opened from a branch in this repo though, which means those who has write access to this repo can get a better validation during review. IIUC this PR is removing that for PRs opened from branches here?

[Ishan-Jadhav]

Whoever opens a PR (whether from this repository or a fork) will need to go through the same validation process, correct? Could you please elaborate on what you mean by "better validation"?

[joyeecheung]

Whoever opens a PR (whether from this repository or a fork) will need to go through the same validation process, correct

The idea is that they don't. Those with write access can open a PR from a branch of this repo and get extra validation of the links (e.g. post is reachable) and that the rich text can be expanded properly, even during PR review phase, instead of having to wait until it's merged and we can only catch these errors then.

Those without write access and have to open a PR from their forks will not get these validation but can still get some local checks that don't require secrets. If there are extra errors from unreachable posts or incorrect rich text, we'll have to wait until it's merged to catch it for them but it is what it is.

[Ishan-Jadhav]

In this commit, I have refactored the validation logic by splitting the original file into two separate files:

• One to handle validations for pull requests opened from forked repositories.
• Another to handle validations for pull requests opened from the same repository.

The local validation checks have been kept unchanged as they apply equally to both types of pull requests. However, this refactoring now allows for easier addition of specific validation checks in the future for either type of pull request as needed.

Please review the changes and let me know if any additional validation logic should be incorporated for either case.

[joyeecheung]

This workflow doesn't look right? It should run validate_cred_fork.js on the files and it should not run the commit and push stuff?

[Ishan-Jadhav]

It pushes changes only after logging into the account; otherwise, it will result in an error.

[aduh95]

TBH I would rather keep the relatively simpler workflow, but change it to use pull_request_target so it has access to secrets. I can open a PR for doing that. EDIT: #31

[aduh95]

Regarding the process workflow, I don't see a reason for a separate process for forks, the current one is enough.

Regarding validate workflow, I'm not convinced we need so many changes, it looks like there are many unrelated changes making hard to review.

[aduh95]

Let's remove this, it's unrelated with validate workflow – and not needed anyway IMO.