Open
Conversation
Signed-off-by: Martin M Sheriff <smartin772@yahoo.com>
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Reviewer's GuideAdds a GitHub Actions workflow that deploys the repository contents to an FTP server whenever changes are pushed to the main branch, using credentials stored in repository secrets. Sequence diagram for GitHub push triggering FTP deployment workflowsequenceDiagram
actor Developer
participant GitHub
participant ActionsRunner
participant FTPDeployAction
participant FTPServer
Developer->>GitHub: Push commit to main
GitHub-->>ActionsRunner: Trigger workflow deploy yml
ActionsRunner->>ActionsRunner: Checkout repo using actions_checkout_v4
ActionsRunner->>FTPDeployAction: Invoke with server username password local_dir
FTPDeployAction->>GitHub: Retrieve secrets FTP_HOST FTP_USER FTP_PASS
FTPDeployAction->>FTPServer: Connect via FTP using credentials
FTPDeployAction->>FTPServer: Upload files from local_dir ./
FTPServer-->>FTPDeployAction: Acknowledge file uploads
FTPDeployAction-->>ActionsRunner: Return deployment result
ActionsRunner-->>GitHub: Report workflow status
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
1989shack.comProject ID: Sites (1)
Tip GraphQL API works alongside REST and WebSocket protocols |
❌ Deploy Preview for 1989shack-e-c failed. Why did it fail? →
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider removing
git-ftp-args: --insecureor documenting why it’s required, and instead configuring proper TLS on the FTP server to avoid weakening connection security. - You may want to restrict the workflow trigger with a
paths:filter so that a push tomainthat only changes non-deployable files (e.g. README, tooling) doesn’t cause an unnecessary FTP deployment. - Double-check that
local-dir: ./is intentional; if the deployable assets live in a subdirectory (e.g.,build/ordist/), pointinglocal-dirthere will avoid uploading unnecessary repository files to the FTP server.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider removing `git-ftp-args: --insecure` or documenting why it’s required, and instead configuring proper TLS on the FTP server to avoid weakening connection security.
- You may want to restrict the workflow trigger with a `paths:` filter so that a push to `main` that only changes non-deployable files (e.g. README, tooling) doesn’t cause an unnecessary FTP deployment.
- Double-check that `local-dir: ./` is intentional; if the deployable assets live in a subdirectory (e.g., `build/` or `dist/`), pointing `local-dir` there will avoid uploading unnecessary repository files to the FTP server.
## Individual Comments
### Comment 1
<location> `.github/workflows/deploy.yml:21` </location>
<code_context>
+ username: ${{ secrets.FTP_USER }}
+ password: ${{ secrets.FTP_PASS }}
+ local-dir: ./ # directory to upload (adjust if your site is in a subfolder)
+ git-ftp-args: --insecure
</code_context>
<issue_to_address>
**🚨 issue (security):** Reconsider `--insecure` as it disables TLS verification for FTP over SSL.
This flag disables certificate checks and should be avoided in routine deployments. If you truly need it for a specific host or environment, gate it behind a condition or config option so that normal runs use full TLS verification.
</issue_to_address>
### Comment 2
<location> `.github/workflows/deploy.yml:20` </location>
<code_context>
+ server: ${{ secrets.FTP_HOST }}
+ username: ${{ secrets.FTP_USER }}
+ password: ${{ secrets.FTP_PASS }}
+ local-dir: ./ # directory to upload (adjust if your site is in a subfolder)
+ git-ftp-args: --insecure
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Uploading from `./` may unintentionally deploy workflow files and other project artifacts.
Using the repo root means everything gets uploaded, including `.github/`, config, and tooling files that shouldn’t be on the server. Prefer a dedicated build/output directory (e.g., `dist/`, `build/`), or at least configure `exclude`/`exclude-glob` to prevent internal files from being deployed.
Suggested implementation:
```
- name: Upload files to FTP
uses: SamKirkland/FTP-Deploy-Action@4.6.0
with:
server: ${{ secrets.FTP_HOST }}
username: ${{ secrets.FTP_USER }}
password: ${{ secrets.FTP_PASS }}
# Use the build/output directory instead of the repo root to avoid deploying workflow/config files
local-dir: ./dist
# As an extra safeguard, exclude internal/tooling files if they end up in the output directory
exclude: |
.github/
.git/
.gitignore
node_modules/
**/*.test.*
**/*.spec.*
git-ftp-args: --insecure
```
1. Ensure your build step outputs the deployable site to `./dist` (or adjust `local-dir` to match your actual output directory, e.g. `build/`, `public/`).
2. If your project structure differs, update the `exclude` list to reflect directories/files that should never be deployed (e.g. `src/`, tooling configs, etc.).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| server: ${{ secrets.FTP_HOST }} | ||
| username: ${{ secrets.FTP_USER }} | ||
| password: ${{ secrets.FTP_PASS }} | ||
| local-dir: ./ # directory to upload (adjust if your site is in a subfolder) |
There was a problem hiding this comment.
🚨 suggestion (security): Uploading from ./ may unintentionally deploy workflow files and other project artifacts.
Using the repo root means everything gets uploaded, including .github/, config, and tooling files that shouldn’t be on the server. Prefer a dedicated build/output directory (e.g., dist/, build/), or at least configure exclude/exclude-glob to prevent internal files from being deployed.
Suggested implementation:
- name: Upload files to FTP
uses: SamKirkland/FTP-Deploy-Action@4.6.0
with:
server: ${{ secrets.FTP_HOST }}
username: ${{ secrets.FTP_USER }}
password: ${{ secrets.FTP_PASS }}
# Use the build/output directory instead of the repo root to avoid deploying workflow/config files
local-dir: ./dist
# As an extra safeguard, exclude internal/tooling files if they end up in the output directory
exclude: |
.github/
.git/
.gitignore
node_modules/
**/*.test.*
**/*.spec.*
git-ftp-args: --insecure
- Ensure your build step outputs the deployable site to
./dist(or adjustlocal-dirto match your actual output directory, e.g.build/,public/). - If your project structure differs, update the
excludelist to reflect directories/files that should never be deployed (e.g.src/, tooling configs, etc.).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Deployment: