Compare commits
3 Commits
feat/ci-ma
...
08377058c2
| Author | SHA1 | Date | |
|---|---|---|---|
| 08377058c2 | |||
|
|
02f0a12626 | ||
|
|
9f2d62da05 |
@@ -1,20 +0,0 @@
|
||||
name: check
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
pull_request:
|
||||
|
||||
jobs:
|
||||
check:
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: golang:1.25
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- name: Install golangci-lint
|
||||
run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
|
||||
|
||||
- name: Run make check
|
||||
run: make check
|
||||
@@ -14,23 +14,19 @@ linters:
|
||||
- wsl # Deprecated, replaced by wsl_v5
|
||||
- wrapcheck # Too verbose for internal packages
|
||||
- varnamelen # Short names like db, id are idiomatic Go
|
||||
settings:
|
||||
gosec:
|
||||
excludes:
|
||||
- G117 # false positives on exported fields named Password/Secret/Key
|
||||
- G703 # path traversal — paths from internal config, not user input
|
||||
- G704 # SSRF — URLs come from server config, not user input
|
||||
- G705 # XSS — log endpoints with text/plain content type
|
||||
lll:
|
||||
line-length: 120
|
||||
funlen:
|
||||
lines: 80
|
||||
statements: 50
|
||||
cyclop:
|
||||
max-complexity: 15
|
||||
dupl:
|
||||
threshold: 150
|
||||
|
||||
linters-settings:
|
||||
lll:
|
||||
line-length: 88
|
||||
funlen:
|
||||
lines: 80
|
||||
statements: 50
|
||||
cyclop:
|
||||
max-complexity: 15
|
||||
dupl:
|
||||
threshold: 100
|
||||
|
||||
issues:
|
||||
exclude-use-default: false
|
||||
max-issues-per-linter: 0
|
||||
max-same-issues: 0
|
||||
|
||||
@@ -480,20 +480,6 @@ func (c *Client) CloneRepo(
|
||||
return c.performClone(ctx, cfg)
|
||||
}
|
||||
|
||||
// RemoveImage removes a Docker image by ID or tag.
|
||||
// It returns nil if the image was successfully removed or does not exist.
|
||||
func (c *Client) RemoveImage(ctx context.Context, imageID string) error {
|
||||
_, err := c.docker.ImageRemove(ctx, imageID, image.RemoveOptions{
|
||||
Force: true,
|
||||
PruneChildren: true,
|
||||
})
|
||||
if err != nil && !client.IsErrNotFound(err) {
|
||||
return fmt.Errorf("failed to remove image %s: %w", imageID, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *Client) performBuild(
|
||||
ctx context.Context,
|
||||
opts BuildImageOptions,
|
||||
@@ -754,6 +740,20 @@ func (c *Client) connect(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveImage removes a Docker image by ID or tag.
|
||||
// It returns nil if the image was successfully removed or does not exist.
|
||||
func (c *Client) RemoveImage(ctx context.Context, imageID string) error {
|
||||
_, err := c.docker.ImageRemove(ctx, imageID, image.RemoveOptions{
|
||||
Force: true,
|
||||
PruneChildren: true,
|
||||
})
|
||||
if err != nil && !client.IsErrNotFound(err) {
|
||||
return fmt.Errorf("failed to remove image %s: %w", imageID, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *Client) close() error {
|
||||
if c.docker != nil {
|
||||
err := c.docker.Close()
|
||||
|
||||
@@ -70,7 +70,7 @@ func TestValidCommitSHARegex(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCloneRepoRejectsInjection(t *testing.T) {
|
||||
func TestCloneRepoRejectsInjection(t *testing.T) { //nolint:funlen // table-driven test
|
||||
t.Parallel()
|
||||
|
||||
c := &Client{
|
||||
|
||||
@@ -218,6 +218,15 @@ func (h *Handlers) HandleAPICreateApp() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
repoURLErr := validateRepoURL(req.RepoURL)
|
||||
if repoURLErr != nil {
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "invalid repository URL: " + repoURLErr.Error()},
|
||||
http.StatusBadRequest)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
createdApp, createErr := h.appService.CreateApp(request.Context(), app.CreateAppInput{
|
||||
Name: req.Name,
|
||||
RepoURL: req.RepoURL,
|
||||
|
||||
@@ -6,7 +6,6 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"html"
|
||||
"net/http"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -40,7 +39,7 @@ func (h *Handlers) HandleAppNew() http.HandlerFunc {
|
||||
}
|
||||
|
||||
// HandleAppCreate handles app creation.
|
||||
func (h *Handlers) HandleAppCreate() http.HandlerFunc {
|
||||
func (h *Handlers) HandleAppCreate() http.HandlerFunc { //nolint:funlen // validation adds necessary length
|
||||
tmpl := templates.GetParsed()
|
||||
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
@@ -78,6 +77,14 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
repoURLErr := validateRepoURL(repoURL)
|
||||
if repoURLErr != nil {
|
||||
data["Error"] = "Invalid repository URL: " + repoURLErr.Error()
|
||||
h.renderTemplate(writer, tmpl, "app_new.html", data)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if branch == "" {
|
||||
branch = "main"
|
||||
}
|
||||
@@ -193,7 +200,7 @@ func (h *Handlers) HandleAppEdit() http.HandlerFunc {
|
||||
}
|
||||
|
||||
// HandleAppUpdate handles app updates.
|
||||
func (h *Handlers) HandleAppUpdate() http.HandlerFunc {
|
||||
func (h *Handlers) HandleAppUpdate() http.HandlerFunc { //nolint:funlen // validation adds necessary length
|
||||
tmpl := templates.GetParsed()
|
||||
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
@@ -226,6 +233,17 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
repoURLErr := validateRepoURL(request.FormValue("repo_url"))
|
||||
if repoURLErr != nil {
|
||||
data := h.addGlobals(map[string]any{
|
||||
"App": application,
|
||||
"Error": "Invalid repository URL: " + repoURLErr.Error(),
|
||||
}, request)
|
||||
_ = tmpl.ExecuteTemplate(writer, "app_edit.html", data)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
application.Name = newName
|
||||
application.RepoURL = request.FormValue("repo_url")
|
||||
application.Branch = request.FormValue("branch")
|
||||
@@ -500,7 +518,7 @@ func (h *Handlers) HandleAppLogs() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
_, _ = writer.Write([]byte(html.EscapeString(logs)))
|
||||
_, _ = writer.Write([]byte(logs))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -583,8 +601,6 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
|
||||
}
|
||||
|
||||
// Check if file exists
|
||||
logPath = filepath.Clean(logPath)
|
||||
|
||||
_, err := os.Stat(logPath)
|
||||
if os.IsNotExist(err) {
|
||||
http.NotFound(writer, request)
|
||||
|
||||
67
internal/handlers/repo_url_validation.go
Normal file
67
internal/handlers/repo_url_validation.go
Normal file
@@ -0,0 +1,67 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Repo URL validation errors.
|
||||
var (
|
||||
errRepoURLEmpty = errors.New("repository URL must not be empty")
|
||||
errRepoURLScheme = errors.New("file:// URLs are not allowed for security reasons")
|
||||
errRepoURLInvalid = errors.New("repository URL must use https://, http://, ssh://, git://, or git@host:path format")
|
||||
errRepoURLNoHost = errors.New("repository URL must include a host")
|
||||
errRepoURLNoPath = errors.New("repository URL must include a path")
|
||||
)
|
||||
|
||||
// scpLikeRepoRe matches SCP-like git URLs: git@host:path (e.g. git@github.com:user/repo.git).
|
||||
// Only the "git" user is allowed, as that is the standard for SSH deploy keys.
|
||||
var scpLikeRepoRe = regexp.MustCompile(`^git@[a-zA-Z0-9._-]+:.+$`)
|
||||
|
||||
// validateRepoURL checks that the given repository URL is valid and uses an allowed scheme.
|
||||
func validateRepoURL(repoURL string) error {
|
||||
if strings.TrimSpace(repoURL) == "" {
|
||||
return errRepoURLEmpty
|
||||
}
|
||||
|
||||
// Reject path traversal in any URL format
|
||||
if strings.Contains(repoURL, "..") {
|
||||
return errRepoURLInvalid
|
||||
}
|
||||
|
||||
// Check for SCP-like git URLs first (git@host:path)
|
||||
if scpLikeRepoRe.MatchString(repoURL) {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Reject file:// explicitly
|
||||
if strings.HasPrefix(strings.ToLower(repoURL), "file://") {
|
||||
return errRepoURLScheme
|
||||
}
|
||||
|
||||
// Parse as standard URL
|
||||
parsed, err := url.Parse(repoURL)
|
||||
if err != nil {
|
||||
return errRepoURLInvalid
|
||||
}
|
||||
|
||||
// Must have a recognized scheme
|
||||
switch strings.ToLower(parsed.Scheme) {
|
||||
case "https", "http", "ssh", "git":
|
||||
// OK
|
||||
default:
|
||||
return errRepoURLInvalid
|
||||
}
|
||||
|
||||
if parsed.Host == "" {
|
||||
return errRepoURLNoHost
|
||||
}
|
||||
|
||||
if parsed.Path == "" || parsed.Path == "/" {
|
||||
return errRepoURLNoPath
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
56
internal/handlers/repo_url_validation_test.go
Normal file
56
internal/handlers/repo_url_validation_test.go
Normal file
@@ -0,0 +1,56 @@
|
||||
package handlers
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestValidateRepoURL(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
url string
|
||||
wantErr bool
|
||||
}{
|
||||
// Valid URLs
|
||||
{name: "https URL", url: "https://github.com/user/repo.git", wantErr: false},
|
||||
{name: "http URL", url: "http://github.com/user/repo.git", wantErr: false},
|
||||
{name: "ssh URL", url: "ssh://git@github.com/user/repo.git", wantErr: false},
|
||||
{name: "git URL", url: "git://github.com/user/repo.git", wantErr: false},
|
||||
{name: "SCP-like URL", url: "git@github.com:user/repo.git", wantErr: false},
|
||||
{name: "SCP-like with dots", url: "git@git.example.com:org/repo.git", wantErr: false},
|
||||
{name: "https without .git", url: "https://github.com/user/repo", wantErr: false},
|
||||
{name: "https with port", url: "https://git.example.com:8443/user/repo.git", wantErr: false},
|
||||
|
||||
// Invalid URLs
|
||||
{name: "empty string", url: "", wantErr: true},
|
||||
{name: "whitespace only", url: " ", wantErr: true},
|
||||
{name: "file URL", url: "file:///etc/passwd", wantErr: true},
|
||||
{name: "file URL uppercase", url: "FILE:///etc/passwd", wantErr: true},
|
||||
{name: "bare path", url: "/some/local/path", wantErr: true},
|
||||
{name: "relative path", url: "../repo", wantErr: true},
|
||||
{name: "just a word", url: "notaurl", wantErr: true},
|
||||
{name: "ftp URL", url: "ftp://example.com/repo.git", wantErr: true},
|
||||
{name: "no host https", url: "https:///path", wantErr: true},
|
||||
{name: "no path https", url: "https://github.com", wantErr: true},
|
||||
{name: "no path https trailing slash", url: "https://github.com/", wantErr: true},
|
||||
{name: "SCP-like non-git user", url: "root@github.com:user/repo.git", wantErr: true},
|
||||
{name: "SCP-like arbitrary user", url: "admin@github.com:user/repo.git", wantErr: true},
|
||||
{name: "path traversal SCP", url: "git@github.com:../../etc/passwd", wantErr: true},
|
||||
{name: "path traversal https", url: "https://github.com/user/../../../etc/passwd", wantErr: true},
|
||||
{name: "path traversal in middle", url: "https://github.com/user/repo/../secret", wantErr: true},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
err := validateRepoURL(tc.url)
|
||||
if tc.wantErr && err == nil {
|
||||
t.Errorf("validateRepoURL(%q) = nil, want error", tc.url)
|
||||
}
|
||||
|
||||
if !tc.wantErr && err != nil {
|
||||
t.Errorf("validateRepoURL(%q) = %v, want nil", tc.url, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -706,6 +706,7 @@ func TestAppGetWebhookEvents(t *testing.T) {
|
||||
|
||||
// Cascade Delete Tests.
|
||||
|
||||
//nolint:funlen // Test function with many assertions - acceptable for integration tests
|
||||
func TestCascadeDelete(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -726,7 +726,6 @@ func (svc *Service) cleanupCancelledDeploy(
|
||||
} else {
|
||||
svc.log.Info("cleaned up build dir from cancelled deploy",
|
||||
"app", app.Name, "path", dirPath)
|
||||
|
||||
_ = deployment.AppendLog(ctx, "Cleaned up build directory")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,7 +32,7 @@ func TestCleanupCancelledDeploy_RemovesBuildDir(t *testing.T) {
|
||||
require.NoError(t, os.MkdirAll(deployDir, 0o750))
|
||||
|
||||
// Create a file inside to verify full removal
|
||||
require.NoError(t, os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o600))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o640))
|
||||
|
||||
// Also create a dir for a different deployment (should NOT be removed)
|
||||
otherDir := filepath.Join(buildDir, "99-xyz789")
|
||||
|
||||
@@ -52,10 +52,10 @@ func NewTestServiceWithConfig(log *slog.Logger, cfg *config.Config, dockerClient
|
||||
// cleanupCancelledDeploy for testing. It removes build directories matching
|
||||
// the deployment ID prefix.
|
||||
func (svc *Service) CleanupCancelledDeploy(
|
||||
_ context.Context,
|
||||
ctx context.Context,
|
||||
appName string,
|
||||
deploymentID int64,
|
||||
_ string,
|
||||
imageID string,
|
||||
) {
|
||||
// We can't create real models.App/Deployment in tests easily,
|
||||
// so we test the build dir cleanup portion directly.
|
||||
|
||||
@@ -102,6 +102,7 @@ func createTestApp(
|
||||
return app
|
||||
}
|
||||
|
||||
//nolint:funlen // table-driven test with comprehensive test cases
|
||||
func TestExtractBranch(testingT *testing.T) {
|
||||
testingT.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user