-
Notifications
You must be signed in to change notification settings - Fork 10
[MINOR]: Adding guideline #438
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
Conversation
Summary of ChangesHello @vibhatha, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the project's documentation infrastructure by introducing a robust Docusaurus setup and deployment guideline. This new resource standardizes best practices for publishing documentation to GitHub Pages, including detailed GitHub Actions workflows. Concurrently, a substantial cleanup effort has been undertaken, removing numerous deprecated documentation files to streamline and modernize the project's informational assets. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new guideline for setting up Docusaurus documentation and removes a significant amount of deprecated documentation files. The new guideline is well-structured and provides valuable instructions for CI/CD setup.
My review focuses on the new docs/DOCS_GUIDELINE.md file. I've identified a critical security vulnerability in the proposed PR preview workflow due to the use of pull_request_target. I've also found a configuration error in the build script that would cause the workflow to fail, and an inconsistency in the AI agent prompt. Please see my detailed comments for suggestions on how to address these issues.
| name: Deploy PR Preview | ||
|
|
||
| on: | ||
| pull_request_target: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using pull_request_target with ref: ${{ github.event.pull_request.head.sha }} is a significant security risk. This combination allows running untrusted code from a pull request with access to repository secrets. A malicious actor could modify the build script in package.json to exfiltrate secrets, such as the GITHUB_TOKEN.
It is strongly recommended to use the pull_request trigger instead. This ensures the workflow runs with read-only permissions and without access to secrets.
If the ability to comment on PRs is essential, consider a safer pattern using two workflows (one on pull_request to build, and another on workflow_run to deploy and comment).
| pull_request_target: | |
| pull_request: |
| preview_branch: gh-pages | ||
| umbrella_dir: pr-preview | ||
| action: auto | ||
| build_script: npm run build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build_script is executed from the root of the repository. Since the Docusaurus project is in the docs/ subdirectory, npm run build will fail because it won't find package.json at the root. You should specify the project directory using the --prefix flag to ensure the build command runs in the correct directory.
| build_script: npm run build | |
| build_script: npm run build --prefix docs |
| > * Use `actions/setup-node@v4` with version `20`. | ||
| > * Use `peaceiris/actions-gh-pages@v3` for deployment. | ||
| > * Create `.github/workflows/preview-docs.yml` (Optional): | ||
| > * Trigger on `pull_request` to `main`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the prompt correctly suggests using pull_request, which is more secure than pull_request_target. However, the YAML template provided for the PR preview workflow (lines 111-155) uses pull_request_target. Please ensure the template is updated to match this safer recommendation to maintain consistency and security.
This PR includes a minor clean-up and guideline addition for docs making.