Skip to content

Conversation

@alaahong
Copy link
Member

Purpose of the pull request

#707

What's changed?

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@netlify
Copy link

netlify bot commented Nov 23, 2025

Deploy Preview for fesod ready!

Name Link
🔨 Latest commit 8c1c2c4
🔍 Latest deploy log https://app.netlify.com/projects/fesod/deploys/69232908360e350008489966
😎 Deploy Preview https://deploy-preview-709--fesod.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@alaahong
Copy link
Member Author

@ongdisheng can you help to check the reason why four netlify job trigger here? And any gap on the preview ci?

@ongdisheng
Copy link
Contributor

ongdisheng commented Nov 23, 2025

Hi @alaahong, I checked the Netlify logs and found the issue. The deployment is failing with this error:

No package.json was found in "/opt/build/repo"

This happens because Netlify is trying to run pnpm build from the repository root. Looking at the Netlify build settings, they appear to have been changed or not configured properly. Since I don't have access to the fesod Netlify project to verify or update these settings. Could you help check what the current configuration is? And also since we're already using GitHub Actions to handle the build and deployment through the workflow, the Netlify automatic builds should be disabled to avoid conflicts. This is my current repo project build settings:
image

Regarding your commit with pull_request_target, I've been looking into this and found there are some security concerns with that approach. I've researched a two-workflow pattern and implemented it on my fork. Feel free to check it out and give any feedback.

@ongdisheng
Copy link
Contributor

Hi @alaahong, I checked the Netlify logs and noticed something interesting. Your deploy URL shows:
app.netlify.com/projects/fesod/...

When I checked @delei recent deployment on PR #707, it shows app.netlify.com/projects/apache-fesod/.... instead. This would probably mean there are two separate Netlify projects connected to fesod repository, which explains why you're seeing multiple Netlify jobs triggering for each PR.

@alaahong
Copy link
Member Author

Hi @alaahong, I checked the Netlify logs and noticed something interesting. Your deploy URL shows: app.netlify.com/projects/fesod/...

When I checked @delei recent deployment on PR #707, it shows app.netlify.com/projects/apache-fesod/.... instead. This would probably mean there are two separate Netlify projects connected to fesod repository, which explains why you're seeing multiple Netlify jobs triggering for each PR.

Can you share the working configuration?
As delei version still meet issue and I set another project as fesod for this, actually we didn't find out the correct way yet.

@alaahong
Copy link
Member Author

Hi @alaahong, I checked the Netlify logs and found the issue. The deployment is failing with this error:

No package.json was found in "/opt/build/repo"

This happens because Netlify is trying to run pnpm build from the repository root. Looking at the Netlify build settings, they appear to have been changed or not configured properly. Since I don't have access to the fesod Netlify project to verify or update these settings. Could you help check what the current configuration is? And also since we're already using GitHub Actions to handle the build and deployment through the workflow, the Netlify automatic builds should be disabled to avoid conflicts. This is my current repo project build settings: image

Regarding your commit with pull_request_target, I've been looking into this and found there are some security concerns with that approach. I've researched a two-workflow pattern and implemented it on my fork. Feel free to check it out and give any feedback.

Both pull_request and pull_request_target won't work as expected...

@ongdisheng
Copy link
Contributor

Both pull_request and pull_request_target won't work as expected...

Hi @alaahong, you're right that pull_request_target isn't working yet because it's not on the main branch. Actually, both pull_request_target and the workflow_run pattern I'm proposing need to be merged to main before they work properly. The key difference is security where pull_request_target has security risks because it checks out and runs PR code while secrets are available. The two-workflow pattern with workflow_run is safer because it separates the untrusted code execution from the secrets access.

@alaahong alaahong marked this pull request as draft November 23, 2025 15:38
@alaahong
Copy link
Member Author

Both pull_request and pull_request_target won't work as expected...

Hi @alaahong, you're right that pull_request_target isn't working yet because it's not on the main branch. Actually, both pull_request_target and the workflow_run pattern I'm proposing need to be merged to main before they work properly. The key difference is security where pull_request_target has security risks because it checks out and runs PR code while secrets are available. The two-workflow pattern with workflow_run is safer because it separates the untrusted code execution from the secrets access.

As token/site id had been resolved now. Can you try to confirm the right configuration on Netlify and CI in Github Action?
This PR is used to verify the result, you can raise a new PR if any modify required later.

@ongdisheng
Copy link
Contributor

Hi @alaahong, thanks for confirming the token/site ID are resolved! Looking at the Netlify build settings screenshot you shared, I notice it has:

Build command: pnpm build
Publish directory: ./website/build

However, this configuration would require setting base directory=website and a pnpm-lock.yaml file in the website folder, which we don't have. This is likely why the builds are failing.
image

I have a suggestion: Since either pull_request_target or workflow_run approach need to be merged to main before they can work anyway, how about I submit a PR with the secure workflow_run approach I've tested on my fork? This approach uses GitHub Actions to handle the entire build and deployment process, so we can leave the Netlify build settings empty similar to what I showed earlier. Once it's merged to main, we can test it with your PR to verify everything works correctly. Let me know what do you think about this?

@ongdisheng
Copy link
Contributor

Hi @alaahong @delei, below is the current configuration I used in my forked repo:

  1. Netlify Settings: I noticed that the Netlify bot was still commenting on PRs even when there were no website changes. To fix this, I switched off the automatic deploy previews for PR in the Netlify settings.
image image
  1. GitHub Actions Workflows: Since we switched off Netlify's automatic deploy previews, all preview builds are now handled by GitHub Actions. I've implemented a secure two-workflow pattern that only builds when website/** files change:

Summary

  • The workflows only trigger for PR when website/** files change.
  • The solution is more secure for fork PRs using the two-workflow pattern instead of using pull_request_target.
  • Only one GitHub Actions bot comment will appear per PR with the deploy preview URL, which updates automatically on each push.
  • All actions use Apache-approved versions with SHA pinning.

Testing Results on My Own Repo:

Feel free to let me know if you have any questions or suggestions for improvements. Thanks!

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.

2 participants