sneak/upaas
1
0
Fork 0
Code Issues 14 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Fix 1.0 audit bugs (closes #120, closes #121, closes #122, closes #123, closes #124, closes #125) #126

Open
clawbot wants to merge 11 commits from fix/audit-bugs-120-125 into main
pull from: fix/audit-bugs-120-125
merge into: sneak:main
Conversation 33 Commits 11 Files Changed 14 +68 -72
8 changed files with 68 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 86528ad63e - Show all commits

36
internal/docker/client.go
View File

@ -26,7 +26,7 @@ import (
"go.uber.org/fx" "go.uber.org/fx"
"git.eeqj.de/sneak/upaas/internal/config" "git.eeqj.de/sneak/upaas/internal/config"
"git.eeqj.de/sneak/upaas/internal/domain"
"git.eeqj.de/sneak/upaas/internal/logger" "git.eeqj.de/sneak/upaas/internal/logger"
) )
@ -117,7 +117,7 @@ type BuildImageOptions struct {
func (c *Client) BuildImage( func (c *Client) BuildImage(
ctx context.Context, ctx context.Context,
opts BuildImageOptions, opts BuildImageOptions,
) (domain.ImageID, error) { ) (ImageID, error) {
if c.docker == nil { if c.docker == nil {
return "", ErrNotConnected return "", ErrNotConnected
} }
@ -189,7 +189,7 @@ func buildPortConfig(ports []PortMapping) (nat.PortSet, nat.PortMap) {
func (c *Client) CreateContainer( func (c *Client) CreateContainer(
ctx context.Context, ctx context.Context,
opts CreateContainerOptions, opts CreateContainerOptions,
) (domain.ContainerID, error) { ) (ContainerID, error) {
if c.docker == nil { if c.docker == nil {
return "", ErrNotConnected return "", ErrNotConnected
} }
@ -242,11 +242,11 @@ func (c *Client) CreateContainer(
return "", fmt.Errorf("failed to create container: %w", err) return "", fmt.Errorf("failed to create container: %w", err)
} }
return domain.ContainerID(resp.ID), nil return ContainerID(resp.ID), nil
} }
// StartContainer starts a container. // StartContainer starts a container.
func (c *Client) StartContainer(ctx context.Context, containerID domain.ContainerID) error { func (c *Client) StartContainer(ctx context.Context, containerID ContainerID) error {
if c.docker == nil { if c.docker == nil {
return ErrNotConnected return ErrNotConnected
} }
@ -262,7 +262,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID domain.Containe
} }
// StopContainer stops a container. // StopContainer stops a container.
func (c *Client) StopContainer(ctx context.Context, containerID domain.ContainerID) error { func (c *Client) StopContainer(ctx context.Context, containerID ContainerID) error {
if c.docker == nil { if c.docker == nil {
return ErrNotConnected return ErrNotConnected
} }
@ -282,7 +282,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID domain.Container
// RemoveContainer removes a container. // RemoveContainer removes a container.
func (c *Client) RemoveContainer( func (c *Client) RemoveContainer(
ctx context.Context, ctx context.Context,
containerID domain.ContainerID, containerID ContainerID,
sneak commented 2026-02-23 20:41:41 +01:00
Outdated
Review
Copy Link

"domain" is a bad name for this. think of something better.

"domain" is a bad name for this. think of something better.
force bool, force bool,
) error { ) error {
if c.docker == nil { if c.docker == nil {
@ -302,7 +302,7 @@ func (c *Client) RemoveContainer(
// ContainerLogs returns the logs for a container. // ContainerLogs returns the logs for a container.
func (c *Client) ContainerLogs( func (c *Client) ContainerLogs(
ctx context.Context, ctx context.Context,
containerID domain.ContainerID, containerID ContainerID,
tail string, tail string,
) (string, error) { ) (string, error) {
if c.docker == nil { if c.docker == nil {
@ -338,7 +338,7 @@ func (c *Client) ContainerLogs(
// IsContainerRunning checks if a container is running. // IsContainerRunning checks if a container is running.
func (c *Client) IsContainerRunning( func (c *Client) IsContainerRunning(
ctx context.Context, ctx context.Context,
containerID domain.ContainerID, containerID ContainerID,
) (bool, error) { ) (bool, error) {
if c.docker == nil { if c.docker == nil {
return false, ErrNotConnected return false, ErrNotConnected
@ -355,7 +355,7 @@ func (c *Client) IsContainerRunning(
// IsContainerHealthy checks if a container is healthy. // IsContainerHealthy checks if a container is healthy.
func (c *Client) IsContainerHealthy( func (c *Client) IsContainerHealthy(
ctx context.Context, ctx context.Context,
containerID domain.ContainerID, containerID ContainerID,
) (bool, error) { ) (bool, error) {
if c.docker == nil { if c.docker == nil {
return false, ErrNotConnected return false, ErrNotConnected
@ -379,7 +379,7 @@ const LabelUpaasID = "upaas.id"
// ContainerInfo contains basic information about a container. // ContainerInfo contains basic information about a container.
type ContainerInfo struct { type ContainerInfo struct {
ID domain.ContainerID ID ContainerID
Running bool Running bool
} }
@ -414,7 +414,7 @@ func (c *Client) FindContainerByAppID(
ctr := containers[0] ctr := containers[0]
return &ContainerInfo{ return &ContainerInfo{
ID: domain.ContainerID(ctr.ID), ID: ContainerID(ctr.ID),
Running: ctr.State == "running", Running: ctr.State == "running",
}, nil }, nil
} }
@ -483,7 +483,7 @@ func (c *Client) CloneRepo(
// RemoveImage removes a Docker image by ID or tag. // RemoveImage removes a Docker image by ID or tag.
// It returns nil if the image was successfully removed or does not exist. // It returns nil if the image was successfully removed or does not exist.
func (c *Client) RemoveImage(ctx context.Context, imageID domain.ImageID) error { func (c *Client) RemoveImage(ctx context.Context, imageID ImageID) error {
_, err := c.docker.ImageRemove(ctx, imageID.String(), image.RemoveOptions{ _, err := c.docker.ImageRemove(ctx, imageID.String(), image.RemoveOptions{
Force: true, Force: true,
PruneChildren: true, PruneChildren: true,
@ -498,7 +498,7 @@ func (c *Client) RemoveImage(ctx context.Context, imageID domain.ImageID) error
func (c *Client) performBuild( func (c *Client) performBuild(
ctx context.Context, ctx context.Context,
opts BuildImageOptions, opts BuildImageOptions,
) (domain.ImageID, error) { ) (ImageID, error) {
// Create tar archive of build context // Create tar archive of build context
tarArchive, err := archive.TarWithOptions(opts.ContextDir, &archive.TarOptions{}) tarArchive, err := archive.TarWithOptions(opts.ContextDir, &archive.TarOptions{})
if err != nil { if err != nil {
@ -543,7 +543,7 @@ func (c *Client) performBuild(
return "", fmt.Errorf("failed to inspect image: %w", inspectErr) return "", fmt.Errorf("failed to inspect image: %w", inspectErr)
} }
return domain.ImageID(inspect.ID), nil return ImageID(inspect.ID), nil
} }
return "", nil return "", nil
@ -619,7 +619,7 @@ func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResu
func (c *Client) createGitContainer( func (c *Client) createGitContainer(
ctx context.Context, ctx context.Context,
cfg *cloneConfig, cfg *cloneConfig,
) (domain.ContainerID, error) { ) (ContainerID, error) {
gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no" gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no"
// Build the git command using environment variables to avoid shell injection. // Build the git command using environment variables to avoid shell injection.
@ -676,10 +676,10 @@ func (c *Client) createGitContainer(
return "", fmt.Errorf("failed to create git container: %w", err) return "", fmt.Errorf("failed to create git container: %w", err)
} }
return domain.ContainerID(resp.ID), nil return ContainerID(resp.ID), nil
} }
func (c *Client) runGitClone(ctx context.Context, containerID domain.ContainerID) (*CloneResult, error) { func (c *Client) runGitClone(ctx context.Context, containerID ContainerID) (*CloneResult, error) {
err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{}) err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{})
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to start git container: %w", err) return nil, fmt.Errorf("failed to start git container: %w", err)

13
internal/docker/types.go Normal file
View File

@ -0,0 +1,13 @@
package docker
// ImageID is a Docker image identifier (ID or tag).
type ImageID string
// String implements the fmt.Stringer interface.
func (id ImageID) String() string { return string(id) }
// ContainerID is a Docker container identifier.
type ContainerID string
// String implements the fmt.Stringer interface.
func (id ContainerID) String() string { return string(id) }

25
internal/domain/types.go
View File

@ -1,25 +0,0 @@
// Package domain defines domain-specific string types for compile-time safety.
// Using named types prevents accidentally passing the wrong string argument
// (e.g. a container ID where an image ID is expected).
package domain
// ImageID is a Docker image identifier (ID or tag).
type ImageID string
// String implements the fmt.Stringer interface.
func (id ImageID) String() string { return string(id) }
// ContainerID is a Docker container identifier.
type ContainerID string
// String implements the fmt.Stringer interface.
func (id ContainerID) String() string { return string(id) }
// UnparsedURL is a URL stored as a plain string without parsing.
// Use this instead of string when the value is known to be a URL
// but should not be parsed into a net/url.URL (e.g. webhook URLs,
// compare URLs from external payloads).
type UnparsedURL string
// String implements the fmt.Stringer interface.
func (u UnparsedURL) String() string { return string(u) }

21
internal/service/deploy/deploy.go
View File

@ -20,7 +20,6 @@ import (
"git.eeqj.de/sneak/upaas/internal/config" "git.eeqj.de/sneak/upaas/internal/config"
"git.eeqj.de/sneak/upaas/internal/database" "git.eeqj.de/sneak/upaas/internal/database"
"git.eeqj.de/sneak/upaas/internal/docker" "git.eeqj.de/sneak/upaas/internal/docker"
"git.eeqj.de/sneak/upaas/internal/domain"
"git.eeqj.de/sneak/upaas/internal/logger" "git.eeqj.de/sneak/upaas/internal/logger"
"git.eeqj.de/sneak/upaas/internal/models" "git.eeqj.de/sneak/upaas/internal/models"
"git.eeqj.de/sneak/upaas/internal/service/notify" "git.eeqj.de/sneak/upaas/internal/service/notify"
@ -418,7 +417,7 @@ func (svc *Service) executeRollback(
svc.removeOldContainer(ctx, app, deployment) svc.removeOldContainer(ctx, app, deployment)
rollbackOpts, err := svc.buildContainerOptions(ctx, app, domain.ImageID(previousImageID)) rollbackOpts, err := svc.buildContainerOptions(ctx, app, docker.ImageID(previousImageID))
if err != nil { if err != nil {
svc.failDeployment(bgCtx, app, deployment, err) svc.failDeployment(bgCtx, app, deployment, err)
@ -515,7 +514,7 @@ func (svc *Service) buildImageWithTimeout(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
) (domain.ImageID, error) { ) (docker.ImageID, error) {
buildCtx, cancel := context.WithTimeout(ctx, buildTimeout) buildCtx, cancel := context.WithTimeout(ctx, buildTimeout)
defer cancel() defer cancel()
@ -540,7 +539,7 @@ func (svc *Service) deployContainerWithTimeout(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
imageID domain.ImageID, imageID docker.ImageID,
) error { ) error {
deployCtx, cancel := context.WithTimeout(ctx, deployTimeout) deployCtx, cancel := context.WithTimeout(ctx, deployTimeout)
defer cancel() defer cancel()
@ -668,7 +667,7 @@ func (svc *Service) checkCancelled(
bgCtx context.Context, bgCtx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
imageID domain.ImageID, imageID docker.ImageID,
) error { ) error {
if !errors.Is(deployCtx.Err(), context.Canceled) { if !errors.Is(deployCtx.Err(), context.Canceled) {
return nil return nil
@ -688,7 +687,7 @@ func (svc *Service) cleanupCancelledDeploy(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
imageID domain.ImageID, imageID docker.ImageID,
) { ) {
// Clean up the intermediate Docker image if one was built // Clean up the intermediate Docker image if one was built
if imageID != "" { if imageID != "" {
@ -817,7 +816,7 @@ func (svc *Service) buildImage(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
) (domain.ImageID, error) { ) (docker.ImageID, error) {
workDir, cleanup, err := svc.cloneRepository(ctx, app, deployment) workDir, cleanup, err := svc.cloneRepository(ctx, app, deployment)
if err != nil { if err != nil {
return "", err return "", err
@ -1017,8 +1016,8 @@ func (svc *Service) createAndStartContainer(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
imageID domain.ImageID, imageID docker.ImageID,
) (domain.ContainerID, error) { ) (docker.ContainerID, error) {
containerOpts, err := svc.buildContainerOptions(ctx, app, imageID) containerOpts, err := svc.buildContainerOptions(ctx, app, imageID)
if err != nil { if err != nil {
svc.failDeployment(ctx, app, deployment, err) svc.failDeployment(ctx, app, deployment, err)
@ -1063,7 +1062,7 @@ func (svc *Service) createAndStartContainer(
func (svc *Service) buildContainerOptions( func (svc *Service) buildContainerOptions(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
imageID domain.ImageID, imageID docker.ImageID,
) (docker.CreateContainerOptions, error) { ) (docker.CreateContainerOptions, error) {
envVars, err := app.GetEnvVars(ctx) envVars, err := app.GetEnvVars(ctx)
if err != nil { if err != nil {
@ -1147,7 +1146,7 @@ func buildPortMappings(ports []*models.Port) []docker.PortMapping {
func (svc *Service) updateAppRunning( func (svc *Service) updateAppRunning(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
imageID domain.ImageID, imageID docker.ImageID,
) error { ) error {
app.ImageID = sql.NullString{String: imageID.String(), Valid: true} app.ImageID = sql.NullString{String: imageID.String(), Valid: true}
app.Status = models.AppStatusRunning app.Status = models.AppStatusRunning

4
internal/service/deploy/deploy_container_test.go
View File

@ -7,7 +7,7 @@ import (
"testing" "testing"
"git.eeqj.de/sneak/upaas/internal/database" "git.eeqj.de/sneak/upaas/internal/database"
"git.eeqj.de/sneak/upaas/internal/domain" "git.eeqj.de/sneak/upaas/internal/docker"
"git.eeqj.de/sneak/upaas/internal/models" "git.eeqj.de/sneak/upaas/internal/models"
"git.eeqj.de/sneak/upaas/internal/service/deploy" "git.eeqj.de/sneak/upaas/internal/service/deploy"
) )
@ -28,7 +28,7 @@ func TestBuildContainerOptionsUsesImageID(t *testing.T) {
log := slog.New(slog.NewTextHandler(os.Stderr, nil)) log := slog.New(slog.NewTextHandler(os.Stderr, nil))
svc := deploy.NewTestService(log) svc := deploy.NewTestService(log)
const expectedImageID = domain.ImageID("sha256:abc123def456") const expectedImageID = docker.ImageID("sha256:abc123def456")
opts, err := svc.BuildContainerOptionsExported(context.Background(), app, expectedImageID) opts, err := svc.BuildContainerOptionsExported(context.Background(), app, expectedImageID)
if err != nil { if err != nil {

3
internal/service/deploy/export_test.go
View File

@ -10,7 +10,6 @@ import (
"git.eeqj.de/sneak/upaas/internal/config" "git.eeqj.de/sneak/upaas/internal/config"
"git.eeqj.de/sneak/upaas/internal/docker" "git.eeqj.de/sneak/upaas/internal/docker"
"git.eeqj.de/sneak/upaas/internal/domain"
"git.eeqj.de/sneak/upaas/internal/models" "git.eeqj.de/sneak/upaas/internal/models"
) )
@ -87,7 +86,7 @@ func (svc *Service) GetBuildDirExported(appName string) string {
func (svc *Service) BuildContainerOptionsExported( func (svc *Service) BuildContainerOptionsExported(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
imageID domain.ImageID, imageID docker.ImageID,
) (docker.CreateContainerOptions, error) { ) (docker.CreateContainerOptions, error) {
return svc.buildContainerOptions(ctx, app, imageID) return svc.buildContainerOptions(ctx, app, imageID)
} }

10
internal/service/webhook/types.go Normal file
View File

@ -0,0 +1,10 @@
package webhook
// UnparsedURL is a URL stored as a plain string without parsing.
// Use this instead of string when the value is known to be a URL
// but should not be parsed into a net/url.URL (e.g. webhook URLs,
// compare URLs from external payloads).
type UnparsedURL string
// String implements the fmt.Stringer interface.
func (u UnparsedURL) String() string { return string(u) }

14
internal/service/webhook/webhook.go
View File

@ -11,7 +11,7 @@ import (
"go.uber.org/fx" "go.uber.org/fx"
"git.eeqj.de/sneak/upaas/internal/database" "git.eeqj.de/sneak/upaas/internal/database"
"git.eeqj.de/sneak/upaas/internal/domain"
"git.eeqj.de/sneak/upaas/internal/logger" "git.eeqj.de/sneak/upaas/internal/logger"
"git.eeqj.de/sneak/upaas/internal/models" "git.eeqj.de/sneak/upaas/internal/models"
"git.eeqj.de/sneak/upaas/internal/service/deploy" "git.eeqj.de/sneak/upaas/internal/service/deploy"
@ -51,12 +51,12 @@ type GiteaPushPayload struct {
Ref string `json:"ref"` Ref string `json:"ref"`
Before string `json:"before"` Before string `json:"before"`
After string `json:"after"` After string `json:"after"`
CompareURL domain.UnparsedURL `json:"compare_url"` CompareURL UnparsedURL `json:"compare_url"`
Repository struct { Repository struct {
FullName string `json:"full_name"` FullName string `json:"full_name"`
CloneURL domain.UnparsedURL `json:"clone_url"` CloneURL UnparsedURL `json:"clone_url"`
SSHURL string `json:"ssh_url"` SSHURL string `json:"ssh_url"`
HTMLURL domain.UnparsedURL `json:"html_url"` HTMLURL UnparsedURL `json:"html_url"`
} `json:"repository"` } `json:"repository"`
Pusher struct { Pusher struct {
Username string `json:"username"` Username string `json:"username"`
@ -64,7 +64,7 @@ type GiteaPushPayload struct {
} `json:"pusher"` } `json:"pusher"`
Commits []struct { Commits []struct {
ID string `json:"id"` ID string `json:"id"`
URL domain.UnparsedURL `json:"url"` URL UnparsedURL `json:"url"`
Message string `json:"message"` Message string `json:"message"`
Author struct { Author struct {
Name string `json:"name"` Name string `json:"name"`
@ -169,7 +169,7 @@ func extractBranch(ref string) string {
// extractCommitURL extracts the commit URL from the webhook payload. // extractCommitURL extracts the commit URL from the webhook payload.
// Prefers the URL from the head commit, falls back to constructing from repo URL. // Prefers the URL from the head commit, falls back to constructing from repo URL.
func extractCommitURL(payload GiteaPushPayload) domain.UnparsedURL { func extractCommitURL(payload GiteaPushPayload) UnparsedURL {
// Try to find the URL from the head commit (matching After SHA) // Try to find the URL from the head commit (matching After SHA)
for _, commit := range payload.Commits { for _, commit := range payload.Commits {
if commit.ID == payload.After && commit.URL != "" { if commit.ID == payload.After && commit.URL != "" {
@ -179,7 +179,7 @@ func extractCommitURL(payload GiteaPushPayload) domain.UnparsedURL {
// Fall back to constructing URL from repo HTML URL // Fall back to constructing URL from repo HTML URL
if payload.Repository.HTMLURL != "" && payload.After != "" { if payload.Repository.HTMLURL != "" && payload.After != "" {
return domain.UnparsedURL(payload.Repository.HTMLURL.String() + "/commit/" + payload.After) return UnparsedURL(payload.Repository.HTMLURL.String() + "/commit/" + payload.After)
} }
return "" return ""