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 +32 -23
5 changed files with 32 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 75cad7d2ad - Show all commits

20
internal/docker/client.go
View File

@ -253,7 +253,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID domain.Containe
c.log.Info("starting container", "id", containerID) c.log.Info("starting container", "id", containerID)
err := c.docker.ContainerStart(ctx, string(containerID), container.StartOptions{}) err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{})
if err != nil { if err != nil {
return fmt.Errorf("failed to start container: %w", err) return fmt.Errorf("failed to start container: %w", err)
} }
@ -271,7 +271,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID domain.Container
timeout := stopTimeoutSeconds timeout := stopTimeoutSeconds
err := c.docker.ContainerStop(ctx, string(containerID), container.StopOptions{Timeout: &timeout}) err := c.docker.ContainerStop(ctx, containerID.String(), container.StopOptions{Timeout: &timeout})
if err != nil { if err != nil {
return fmt.Errorf("failed to stop container: %w", err) return fmt.Errorf("failed to stop container: %w", err)
} }
@ -291,7 +291,7 @@ func (c *Client) RemoveContainer(
c.log.Info("removing container", "id", containerID, "force", force) c.log.Info("removing container", "id", containerID, "force", force)
err := c.docker.ContainerRemove(ctx, string(containerID), container.RemoveOptions{Force: force}) err := c.docker.ContainerRemove(ctx, containerID.String(), container.RemoveOptions{Force: force})
if err != nil { if err != nil {
return fmt.Errorf("failed to remove container: %w", err) return fmt.Errorf("failed to remove container: %w", err)
} }
@ -315,7 +315,7 @@ func (c *Client) ContainerLogs(
Tail: tail, Tail: tail,
} }
reader, err := c.docker.ContainerLogs(ctx, string(containerID), opts) reader, err := c.docker.ContainerLogs(ctx, containerID.String(), opts)
sneak commented 2026-02-23 20:42:17 +01:00
Outdated
Review
Copy Link

ContainerLogs signature should be updated to take the correct type (if that's our code). that's the whole point of using custom string types.

ContainerLogs signature should be updated to take the correct type (if that's our code). that's the whole point of using custom string types.
if err != nil { if err != nil {
return "", fmt.Errorf("failed to get container logs: %w", err) return "", fmt.Errorf("failed to get container logs: %w", err)
} }
@ -344,7 +344,7 @@ func (c *Client) IsContainerRunning(
return false, ErrNotConnected return false, ErrNotConnected
} }
inspect, err := c.docker.ContainerInspect(ctx, string(containerID)) inspect, err := c.docker.ContainerInspect(ctx, containerID.String())
if err != nil { if err != nil {
return false, fmt.Errorf("failed to inspect container: %w", err) return false, fmt.Errorf("failed to inspect container: %w", err)
} }
@ -361,7 +361,7 @@ func (c *Client) IsContainerHealthy(
return false, ErrNotConnected return false, ErrNotConnected
} }
inspect, err := c.docker.ContainerInspect(ctx, string(containerID)) inspect, err := c.docker.ContainerInspect(ctx, containerID.String())
sneak commented 2026-02-23 20:42:40 +01:00
Outdated
Review
Copy Link

is c.docker our object (and can these function signatures be updated) or is that docker's code?

is c.docker our object (and can these function signatures be updated) or is that docker's code?
if err != nil { if err != nil {
return false, fmt.Errorf("failed to inspect container: %w", err) return false, fmt.Errorf("failed to inspect container: %w", err)
} }
@ -484,7 +484,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 domain.ImageID) error {
_, err := c.docker.ImageRemove(ctx, string(imageID), image.RemoveOptions{ _, err := c.docker.ImageRemove(ctx, imageID.String(), image.RemoveOptions{
Force: true, Force: true,
PruneChildren: true, PruneChildren: true,
}) })
@ -610,7 +610,7 @@ func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResu
} }
defer func() { defer func() {
_ = c.docker.ContainerRemove(ctx, string(gitContainerID), container.RemoveOptions{Force: true}) _ = c.docker.ContainerRemove(ctx, gitContainerID.String(), container.RemoveOptions{Force: true})
}() }()
return c.runGitClone(ctx, gitContainerID) return c.runGitClone(ctx, gitContainerID)
@ -680,12 +680,12 @@ func (c *Client) createGitContainer(
} }
func (c *Client) runGitClone(ctx context.Context, containerID domain.ContainerID) (*CloneResult, error) { func (c *Client) runGitClone(ctx context.Context, containerID domain.ContainerID) (*CloneResult, error) {
err := c.docker.ContainerStart(ctx, string(containerID), 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)
} }
statusCh, errCh := c.docker.ContainerWait(ctx, string(containerID), container.WaitConditionNotRunning) statusCh, errCh := c.docker.ContainerWait(ctx, containerID.String(), container.WaitConditionNotRunning)
select { select {
case err := <-errCh: case err := <-errCh:

9
internal/domain/types.go
View File

@ -6,11 +6,20 @@ package domain
// ImageID is a Docker image identifier (ID or tag). // ImageID is a Docker image identifier (ID or tag).
type ImageID string type ImageID string
// String implements the fmt.Stringer interface.
func (id ImageID) String() string { return string(id) }
// ContainerID is a Docker container identifier. // ContainerID is a Docker container identifier.
type ContainerID string 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. // 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 // 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, // but should not be parsed into a net/url.URL (e.g. webhook URLs,
// compare URLs from external payloads). // compare URLs from external payloads).
type UnparsedURL string type UnparsedURL string
// String implements the fmt.Stringer interface.
func (u UnparsedURL) String() string { return string(u) }

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

@ -432,8 +432,8 @@ func (svc *Service) executeRollback(
return fmt.Errorf("failed to create rollback container: %w", err) return fmt.Errorf("failed to create rollback container: %w", err)
} }
deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true} deployment.ContainerID = sql.NullString{String: containerID.String(), Valid: true}
_ = deployment.AppendLog(bgCtx, "Rollback container created: "+string(containerID)) _ = deployment.AppendLog(bgCtx, "Rollback container created: "+containerID.String())
startErr := svc.docker.StartContainer(ctx, containerID) startErr := svc.docker.StartContainer(ctx, containerID)
if startErr != nil { if startErr != nil {
@ -696,11 +696,11 @@ func (svc *Service) cleanupCancelledDeploy(
if removeErr != nil { if removeErr != nil {
svc.log.Error("failed to remove image from cancelled deploy", svc.log.Error("failed to remove image from cancelled deploy",
"error", removeErr, "app", app.Name, "image", imageID) "error", removeErr, "app", app.Name, "image", imageID)
_ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+string(imageID)+": "+removeErr.Error()) _ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+imageID.String()+": "+removeErr.Error())
} else { } else {
svc.log.Info("cleaned up image from cancelled deploy", svc.log.Info("cleaned up image from cancelled deploy",
"app", app.Name, "image", imageID) "app", app.Name, "image", imageID)
_ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+string(imageID)) _ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+imageID.String())
} }
} }
@ -851,8 +851,8 @@ func (svc *Service) buildImage(
return "", fmt.Errorf("failed to build image: %w", err) return "", fmt.Errorf("failed to build image: %w", err)
} }
deployment.ImageID = sql.NullString{String: string(imageID), Valid: true} deployment.ImageID = sql.NullString{String: imageID.String(), Valid: true}
_ = deployment.AppendLog(ctx, "Image built: "+string(imageID)) _ = deployment.AppendLog(ctx, "Image built: "+imageID.String())
return imageID, nil return imageID, nil
} }
@ -1039,8 +1039,8 @@ func (svc *Service) createAndStartContainer(
return "", fmt.Errorf("failed to create container: %w", err) return "", fmt.Errorf("failed to create container: %w", err)
} }
deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true} deployment.ContainerID = sql.NullString{String: containerID.String(), Valid: true}
_ = deployment.AppendLog(ctx, "Container created: "+string(containerID)) _ = deployment.AppendLog(ctx, "Container created: "+containerID.String())
startErr := svc.docker.StartContainer(ctx, containerID) startErr := svc.docker.StartContainer(ctx, containerID)
if startErr != nil { if startErr != nil {
@ -1097,7 +1097,7 @@ func (svc *Service) buildContainerOptions(
return docker.CreateContainerOptions{ return docker.CreateContainerOptions{
Name: "upaas-" + app.Name, Name: "upaas-" + app.Name,
Image: string(imageID), Image: imageID.String(),
Env: envMap, Env: envMap,
Labels: buildLabelMap(app, labels), Labels: buildLabelMap(app, labels),
Volumes: buildVolumeMounts(volumes), Volumes: buildVolumeMounts(volumes),
@ -1149,7 +1149,7 @@ func (svc *Service) updateAppRunning(
app *models.App, app *models.App,
imageID domain.ImageID, imageID domain.ImageID,
) error { ) error {
app.ImageID = sql.NullString{String: string(imageID), Valid: true} app.ImageID = sql.NullString{String: imageID.String(), Valid: true}
app.Status = models.AppStatusRunning app.Status = models.AppStatusRunning
saveErr := app.Save(ctx) saveErr := app.Save(ctx)

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

@ -35,7 +35,7 @@ func TestBuildContainerOptionsUsesImageID(t *testing.T) {
t.Fatalf("buildContainerOptions returned error: %v", err) t.Fatalf("buildContainerOptions returned error: %v", err)
} }
if opts.Image != string(expectedImageID) { if opts.Image != expectedImageID.String() {
t.Errorf("expected Image=%q, got %q", expectedImageID, opts.Image) t.Errorf("expected Image=%q, got %q", expectedImageID, opts.Image)
} }

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

@ -105,7 +105,7 @@ func (svc *Service) HandleWebhook(
event.EventType = eventType event.EventType = eventType
event.Branch = branch event.Branch = branch
event.CommitSHA = sql.NullString{String: commitSHA, Valid: commitSHA != ""} event.CommitSHA = sql.NullString{String: commitSHA, Valid: commitSHA != ""}
event.CommitURL = sql.NullString{String: string(commitURL), Valid: commitURL != ""} event.CommitURL = sql.NullString{String: commitURL.String(), Valid: commitURL != ""}
event.Payload = sql.NullString{String: string(payload), Valid: true} event.Payload = sql.NullString{String: string(payload), Valid: true}
event.Matched = matched event.Matched = matched
event.Processed = false event.Processed = false
@ -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(string(payload.Repository.HTMLURL) + "/commit/" + payload.After) return domain.UnparsedURL(payload.Repository.HTMLURL.String() + "/commit/" + payload.After)
} }
return "" return ""