Compare commits

..

5 Commits

Author SHA1 Message Date
user
478746c356 rebase: apply audit bug fixes on latest main
Rebased fix/1.0-audit-bugs onto current main (post-merge of PRs #119, #127).

Changes:
- Custom domain types: docker.ImageID, docker.ContainerID, webhook.UnparsedURL
- Type-safe function signatures throughout docker/deploy/webhook packages
- Remove docker-compose.yml, test helper files
- README and Dockerfile updates
- Prettier-formatted JS files
2026-02-23 11:56:09 -08:00
28f014ce95 Merge pull request 'fix: use imageID in createAndStartContainer (closes #124)' (#127) from fix/use-image-id-in-container into main
All checks were successful
Check / check (push) Successful in 11m32s
Reviewed-on: #127
2026-02-23 20:48:23 +01:00
dc638a07f1 Merge pull request 'fix: pin all external refs to cryptographic identity (closes #118)' (#119) from fix/pin-external-refs-crypto-identity into main
Some checks failed
Check / check (push) Has been cancelled
Reviewed-on: #119
2026-02-23 20:48:09 +01:00
user
0e8efe1043 fix: use imageID in createAndStartContainer (closes #124)
All checks were successful
Check / check (pull_request) Successful in 11m24s
Wire the imageID parameter (returned from docker build) through
createAndStartContainer and buildContainerOptions instead of
reconstructing a mutable tag via fmt.Sprintf.

This ensures containers reference the immutable image digest,
avoiding tag-reuse races when deploys overlap.

Changes:
- Rename _ string to imageID string in createAndStartContainer
- Change buildContainerOptions to accept imageID string instead of deploymentID int64
- Use imageID directly as the Image field in container options
- Update rollback path to pass previousImageID directly
- Add test verifying imageID flows through to container options
- Add database.NewTestDatabase and logger.NewForTest test helpers
2026-02-21 02:24:51 -08:00
user
0ed2d02dfe fix: pin all external refs to cryptographic identity (closes #118)
All checks were successful
Check / check (pull_request) Successful in 11m41s
- Pin Docker base images to sha256 digests (golang, alpine)
- Pin go install commands to commit SHAs (not version tags)
  - golangci-lint: 5d1e709b7be35cb2025444e19de266b056b7b7ee (v2.10.1)
  - goimports: 009367f5c17a8d4c45a961a3a509277190a9a6f0 (v0.42.0)
- CI workflow was already correctly pinned to commit SHAs

All references now use cryptographic identity, eliminating RCE risk
from mutable tags.
2026-02-21 00:50:44 -08:00
5 changed files with 21 additions and 30 deletions

View File

@@ -252,7 +252,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID ContainerID) er
c.log.Info("starting container", "id", containerID) c.log.Info("starting container", "id", containerID)
err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{}) err := c.docker.ContainerStart(ctx, string(containerID), 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)
} }
@@ -270,7 +270,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID ContainerID) err
timeout := stopTimeoutSeconds timeout := stopTimeoutSeconds
err := c.docker.ContainerStop(ctx, containerID.String(), container.StopOptions{Timeout: &timeout}) err := c.docker.ContainerStop(ctx, string(containerID), 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)
} }
@@ -290,7 +290,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, containerID.String(), container.RemoveOptions{Force: force}) err := c.docker.ContainerRemove(ctx, string(containerID), 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)
} }
@@ -314,7 +314,7 @@ func (c *Client) ContainerLogs(
Tail: tail, Tail: tail,
} }
reader, err := c.docker.ContainerLogs(ctx, containerID.String(), opts) reader, err := c.docker.ContainerLogs(ctx, string(containerID), opts)
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)
} }
@@ -343,7 +343,7 @@ func (c *Client) IsContainerRunning(
return false, ErrNotConnected return false, ErrNotConnected
} }
inspect, err := c.docker.ContainerInspect(ctx, containerID.String()) inspect, err := c.docker.ContainerInspect(ctx, string(containerID))
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)
} }
@@ -360,7 +360,7 @@ func (c *Client) IsContainerHealthy(
return false, ErrNotConnected return false, ErrNotConnected
} }
inspect, err := c.docker.ContainerInspect(ctx, containerID.String()) inspect, err := c.docker.ContainerInspect(ctx, string(containerID))
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)
} }
@@ -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 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, string(imageID), image.RemoveOptions{
Force: true, Force: true,
PruneChildren: true, PruneChildren: true,
}) })
@@ -609,7 +609,7 @@ func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResu
} }
defer func() { defer func() {
_ = c.docker.ContainerRemove(ctx, gitContainerID.String(), container.RemoveOptions{Force: true}) _ = c.docker.ContainerRemove(ctx, string(gitContainerID), container.RemoveOptions{Force: true})
}() }()
return c.runGitClone(ctx, gitContainerID) return c.runGitClone(ctx, gitContainerID)
@@ -679,12 +679,12 @@ func (c *Client) createGitContainer(
} }
func (c *Client) runGitClone(ctx context.Context, containerID 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, string(containerID), 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, containerID.String(), container.WaitConditionNotRunning) statusCh, errCh := c.docker.ContainerWait(ctx, string(containerID), container.WaitConditionNotRunning)
select { select {
case err := <-errCh: case err := <-errCh:

View File

@@ -3,11 +3,5 @@ package docker
// 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 fmt.Stringer.
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 fmt.Stringer.
func (id ContainerID) String() string { return string(id) }

View File

@@ -431,8 +431,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: containerID.String(), Valid: true} deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true}
_ = deployment.AppendLog(bgCtx, "Rollback container created: "+containerID.String()) _ = deployment.AppendLog(bgCtx, "Rollback container created: "+string(containerID))
startErr := svc.docker.StartContainer(ctx, containerID) startErr := svc.docker.StartContainer(ctx, containerID)
if startErr != nil { if startErr != nil {
@@ -695,11 +695,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 "+imageID.String()+": "+removeErr.Error()) _ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+string(imageID)+": "+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: "+imageID.String()) _ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+string(imageID))
} }
} }
@@ -850,8 +850,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: imageID.String(), Valid: true} deployment.ImageID = sql.NullString{String: string(imageID), Valid: true}
_ = deployment.AppendLog(ctx, "Image built: "+imageID.String()) _ = deployment.AppendLog(ctx, "Image built: "+string(imageID))
return imageID, nil return imageID, nil
} }
@@ -1038,8 +1038,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: containerID.String(), Valid: true} deployment.ContainerID = sql.NullString{String: string(containerID), Valid: true}
_ = deployment.AppendLog(ctx, "Container created: "+containerID.String()) _ = deployment.AppendLog(ctx, "Container created: "+string(containerID))
startErr := svc.docker.StartContainer(ctx, containerID) startErr := svc.docker.StartContainer(ctx, containerID)
if startErr != nil { if startErr != nil {
@@ -1096,7 +1096,7 @@ func (svc *Service) buildContainerOptions(
return docker.CreateContainerOptions{ return docker.CreateContainerOptions{
Name: "upaas-" + app.Name, Name: "upaas-" + app.Name,
Image: imageID.String(), Image: string(imageID),
Env: envMap, Env: envMap,
Labels: buildLabelMap(app, labels), Labels: buildLabelMap(app, labels),
Volumes: buildVolumeMounts(volumes), Volumes: buildVolumeMounts(volumes),
@@ -1148,7 +1148,7 @@ func (svc *Service) updateAppRunning(
app *models.App, app *models.App,
imageID docker.ImageID, imageID docker.ImageID,
) error { ) error {
app.ImageID = sql.NullString{String: imageID.String(), Valid: true} app.ImageID = sql.NullString{String: string(imageID), Valid: true}
app.Status = models.AppStatusRunning app.Status = models.AppStatusRunning
saveErr := app.Save(ctx) saveErr := app.Save(ctx)

View File

@@ -5,6 +5,3 @@ package webhook
// 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 fmt.Stringer.
func (u UnparsedURL) String() string { return string(u) }

View File

@@ -178,7 +178,7 @@ func extractCommitURL(payload GiteaPushPayload) 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 UnparsedURL(payload.Repository.HTMLURL.String() + "/commit/" + payload.After) return UnparsedURL(string(payload.Repository.HTMLURL) + "/commit/" + payload.After)
} }
return "" return ""