build: bump version 0.2.0 for deployment, productionise, fix all tests

Author: spwoodcock

.github/workflows/release_binary.yml

@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-latest
 
     container:
-      image: docker.io/goreleaser/goreleaser-cross:v1.25
+      image: docker.io/goreleaser/goreleaser-cross:v1.26
 
     steps:
       - name: Checkout

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.25 AS base
+FROM golang:1.26 AS base
 
 
 # Build statically compiled binary

app/api/main.go

@@ -18,15 +18,16 @@ import (
 
 // Make the API, JobQueue, and WorkflowClient available on each endpoint
 type API struct {
-	api            huma.API
-	workflowClient workflows.WorkflowClient
-	metadataStore  *meta.Store
+	api             huma.API
+	workflowClient  workflows.WorkflowClient
+	metadataStore   *meta.Store
+	downloadHandler http.Handler // raw handler for download redirect
 }
 
 // NewAPI creates the Huma API and registers routes.
 // It returns the API object and the HTTP handler (stdlib mux) that should be served.
 func NewAPI(metadataStore *meta.Store, workflowClient workflows.WorkflowClient) (*API, http.Handler) {
-	apiConfig := huma.DefaultConfig("ScaleODM API", "0.1.0")
+	apiConfig := huma.DefaultConfig("ScaleODM API", "0.2.0")
 	apiConfig.DocsPath = "/"
 	apiConfig.OpenAPIPath = "/openapi.json"
 	apiConfig.Servers = []*huma.Server{
@@ -90,6 +91,12 @@ func NewAPI(metadataStore *meta.Store, workflowClient workflows.WorkflowClient)
 	apiObj.registerNodeODMRoutes()
 	// apiObj.registerScaleODMRoutes()
 
+	// Register the download handler as a raw HTTP route (outside Huma)
+	// so we can issue proper HTTP redirects to pre-signed S3 URLs.
+	if apiObj.downloadHandler != nil {
+		router.Handle("GET /task/{uuid}/download/{asset}", apiObj.downloadHandler)
+	}
+
 	return apiObj, router
 }
 

app/api/nodeodm_api.go

@@ -6,18 +6,39 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"regexp"
 	"strings"
 	"time"
 
 	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
 	"github.com/danielgtaylor/huma/v2"
 	_ "github.com/danielgtaylor/huma/v2/formats/cbor"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 
 	"github.com/hotosm/scaleodm/app/config"
 	"github.com/hotosm/scaleodm/app/s3"
 	"github.com/hotosm/scaleodm/app/workflows"
 )
 
+// isNotFound checks whether an error (possibly wrapped) represents a
+// Kubernetes "not found" response.
+func isNotFound(err error) bool {
+	return k8serrors.IsNotFound(err)
+}
+
+// shellSafePattern matches strings that are safe to embed in shell scripts.
+// Allows alphanumerics, hyphens, underscores, dots, forward slashes, colons,
+// and the equals sign. This prevents shell injection via user-supplied values.
+var shellSafePattern = regexp.MustCompile(`^[a-zA-Z0-9\-_./=:@]+$`)
+
+// validateShellSafe checks that a string is safe to embed in a shell script.
+func validateShellSafe(value, fieldName string) error {
+	if !shellSafePattern.MatchString(value) {
+		return fmt.Errorf("%s contains invalid characters: only alphanumerics, hyphens, underscores, dots, slashes, colons, equals, and @ are allowed", fieldName)
+	}
+	return nil
+}
+
 // NodeODM status codes
 const (
 	StatusCodeQueued    = 10
@@ -167,7 +188,7 @@ func (a *API) registerNodeODMRoutes() {
 		}
 
 		resp := &InfoResponse{}
-		resp.Body.Version = "0.1.0" // The ScaleODM version (normally the NodeODM version)
+		resp.Body.Version = "0.2.0" // The ScaleODM version (normally the NodeODM version)
 		resp.Body.TaskQueueCount = queueCount
 		resp.Body.MaxImages = nil // Unlimited
 		resp.Body.Engine = "odm"
@@ -338,6 +359,22 @@ func (a *API) registerNodeODMRoutes() {
 			projectID = "odm-project"
 		}
 
+		// Validate all values that will be embedded in shell scripts
+		if err := validateShellSafe(projectID, "name"); err != nil {
+			return nil, huma.NewError(400, err.Error())
+		}
+		for _, flag := range odmFlags {
+			if err := validateShellSafe(flag, "options flag"); err != nil {
+				return nil, huma.NewError(400, err.Error())
+			}
+		}
+		if err := validateShellSafe(readPath, "readS3Path"); err != nil {
+			return nil, huma.NewError(400, err.Error())
+		}
+		if err := validateShellSafe(writePath, "writeS3Path"); err != nil {
+			return nil, huma.NewError(400, err.Error())
+		}
+
 		// Determine S3 region & optional endpoint
 		s3Region := req.S3Region
 		if s3Region == "" {
@@ -375,7 +412,9 @@ func (a *API) registerNodeODMRoutes() {
 			s3Region,
 		)
 
-		// Record metadata in database
+		// Record metadata in database. If this fails, the workflow exists in
+		// Argo but won't be visible via the API - treat as a hard error so the
+		// caller knows to retry rather than losing track of the workflow.
 		_, err = a.metadataStore.CreateJob(
 			ctx,
 			wf.Name,
@@ -386,7 +425,8 @@ func (a *API) registerNodeODMRoutes() {
 			s3Region,
 		)
 		if err != nil {
-			log.Printf("Warning: Failed to record job metadata: %v", err)
+			log.Printf("ERROR: Failed to record job metadata for workflow %q: %v", wf.Name, err)
+			return nil, huma.NewError(500, "Workflow created but failed to record metadata - retry the request", err)
 		}
 
 		resp := &TaskNewResponse{}
@@ -601,7 +641,7 @@ func (a *API) registerNodeODMRoutes() {
 
 		err := a.workflowClient.DeleteWorkflow(ctx, input.Body.UUID)
 		if err != nil {
-			if strings.Contains(err.Error(), "not found") {
+			if isNotFound(err) {
 				log.Printf("POST /task/cancel: task %q not found", input.Body.UUID)
 				return nil, huma.NewError(404, "Task not found")
 			}
@@ -636,7 +676,7 @@ func (a *API) registerNodeODMRoutes() {
 
 		// Delete from Argo
 		err := a.workflowClient.DeleteWorkflow(ctx, input.Body.UUID)
-		if err != nil && !strings.Contains(err.Error(), "not found") {
+		if err != nil && !isNotFound(err) {
 			log.Printf("POST /task/remove: failed to delete workflow for %q: %v", input.Body.UUID, err)
 			return nil, huma.NewError(500, "Failed to remove task", err)
 		}
@@ -696,7 +736,7 @@ func (a *API) registerNodeODMRoutes() {
 		}
 
 		// Delete old workflow
-		if err := a.workflowClient.DeleteWorkflow(ctx, input.Body.UUID); err != nil && !strings.Contains(err.Error(), "not found") {
+		if err := a.workflowClient.DeleteWorkflow(ctx, input.Body.UUID); err != nil && !isNotFound(err) {
 			log.Printf("POST /task/restart: failed to delete old workflow for %q: %v", input.Body.UUID, err)
 		}
 
@@ -736,94 +776,41 @@ func (a *API) registerNodeODMRoutes() {
 	})
 
 	// GET /task/{uuid}/download/{asset} - Download task asset (redirects to pre-signed URL)
-	huma.Register(a.api, huma.Operation{
-		OperationID: "task-uuid-download-asset-get",
-		Method:      http.MethodGet,
-		Path:        "/task/{uuid}/download/{asset}",
-		Summary:     "Download task output asset",
-		Description: "Redirects to a pre-signed URL for downloading the asset directly from S3",
-		Tags:        []string{"task"},
-	}, func(ctx context.Context, input *struct {
-		UUID  string `path:"uuid" doc:"UUID of the task"`
-		Asset string `path:"asset" doc:"Asset type (all.zip, orthophoto.tif, etc)"`
-		Token string `query:"token" doc:"Authentication token (optional)"`
-	}) (*struct{ Body string }, error) {
-		log.Printf("GET /task/%s/download/%s: token_provided=%t", input.UUID, input.Asset, input.Token != "")
-
-		// Get job metadata to retrieve write path
-		metadata, err := a.metadataStore.GetJob(ctx, input.UUID)
+	// Registered as a raw HTTP handler on the mux for reliable redirect support.
+	// Huma handlers return structured responses which can't express HTTP redirects,
+	// so we handle this endpoint outside Huma.
+	a.downloadHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		uuid := r.PathValue("uuid")
+		asset := r.PathValue("asset")
+		log.Printf("GET /task/%s/download/%s", uuid, asset)
+
+		metadata, err := a.metadataStore.GetJob(r.Context(), uuid)
 		if err != nil {
-			log.Printf("GET /task/%s/download/%s: failed to retrieve metadata: %v", input.UUID, input.Asset, err)
-			return nil, huma.NewError(500, "Failed to retrieve task metadata", err)
+			log.Printf("GET /task/%s/download/%s: failed to retrieve metadata: %v", uuid, asset, err)
+			http.Error(w, `{"error":"Failed to retrieve task metadata"}`, http.StatusInternalServerError)
+			return
 		}
-
 		if metadata == nil {
-			log.Printf("GET /task/%s/download/%s: task not found in metadata store", input.UUID, input.Asset)
-			return nil, huma.NewError(404, "Task not found")
+			log.Printf("GET /task/%s/download/%s: task not found", uuid, asset)
+			http.Error(w, `{"error":"Task not found"}`, http.StatusNotFound)
+			return
 		}
-
 		if metadata.WriteS3Path == "" {
-			log.Printf("GET /task/%s/download/%s: write S3 path not available", input.UUID, input.Asset)
-			return nil, huma.NewError(400, "Write S3 path not available for this task")
+			log.Printf("GET /task/%s/download/%s: write S3 path not available", uuid, asset)
+			http.Error(w, `{"error":"Write S3 path not available for this task"}`, http.StatusBadRequest)
+			return
 		}
 
-		// Get S3 client
 		s3Client := s3.GetS3Client()
-
-		// Generate pre-signed URL (valid for 1 hour)
-		presignedURL, err := s3.GeneratePresignedURL(ctx, s3Client, metadata.WriteS3Path, input.Asset, 1*time.Hour)
+		presignedURL, err := s3.GeneratePresignedURL(r.Context(), s3Client, metadata.WriteS3Path, asset, 1*time.Hour)
 		if err != nil {
-			log.Printf("GET /task/%s/download/%s: failed to generate pre-signed URL: %v", input.UUID, input.Asset, err)
-			return nil, huma.NewError(404, fmt.Sprintf("File not found: %s", input.Asset), err)
-		}
-
-		log.Printf("GET /task/%s/download/%s: redirecting to pre-signed URL (expires in 1 hour)", input.UUID, input.Asset)
-
-		// Access response writer and request from humago adapter's context
-		// The humago adapter stores the response writer using a specific context key
-		// We need to access it through the adapter's internal context storage
-		type responseWriterKey struct{}
-		type requestKey struct{}
-		
-		var rw http.ResponseWriter
-		var req *http.Request
-		
-		// Try accessing through typed context keys
-		if val := ctx.Value(responseWriterKey{}); val != nil {
-			rw, _ = val.(http.ResponseWriter)
-		}
-		if val := ctx.Value(requestKey{}); val != nil {
-			req, _ = val.(*http.Request)
-		}
-		
-		// Try string-based keys (some adapters use these)
-		if rw == nil {
-			if val := ctx.Value("http.responseWriter"); val != nil {
-				rw, _ = val.(http.ResponseWriter)
-			}
-		}
-		if req == nil {
-			if val := ctx.Value("http.request"); val != nil {
-				req, _ = val.(*http.Request)
-			}
+			log.Printf("GET /task/%s/download/%s: failed to generate pre-signed URL: %v", uuid, asset, err)
+			http.Error(w, fmt.Sprintf(`{"error":"File not found: %s"}`, asset), http.StatusNotFound)
+			return
 		}
-		
-		// Try getting from http.ServerContextKey
-		if req == nil {
-			if httpReq, ok := ctx.Value(http.ServerContextKey).(*http.Request); ok && httpReq != nil {
-				req = httpReq
-			}
-		}
-		
-		if rw != nil && req != nil {
-			// Perform redirect
-			http.Redirect(rw, req, presignedURL, http.StatusFound)
-			return nil, nil
-		}
-		
-		// Fallback: Return the URL in response body if redirect not possible
-		// This provides graceful degradation - client can still get the URL
-		return &struct{ Body string }{Body: presignedURL}, nil
+
+		log.Printf("GET /task/%s/download/%s: redirecting to pre-signed URL (expires in 1 hour)", uuid, asset)
+		http.Redirect(w, r, presignedURL, http.StatusFound)
 	})
 }
 

app/api/nodeodm_api_test.go

@@ -37,7 +37,7 @@ func TestInfoEndpoint(t *testing.T) {
 	}
 	err := json.Unmarshal(w.Body.Bytes(), &response)
 	require.NoError(t, err)
-	assert.Equal(t, "0.1.0", response.Version)
+	assert.Equal(t, "0.2.0", response.Version)
 	assert.Equal(t, "odm", response.Engine)
 }
 

app/db/connection.go

@@ -2,6 +2,7 @@ package db
 
 import (
 	"context"
+	"hash/crc32"
 	_ "embed"
 	"fmt"
 	"strings"
@@ -10,6 +11,11 @@ import (
 	"github.com/jackc/pgx/v5/pgxpool"
 )
 
+// schemaLockID is a PostgreSQL advisory lock ID used to serialise
+// concurrent schema initialisations. Derived from a CRC32 hash of a
+// well-known string so it won't collide with other applications.
+var schemaLockID = int64(crc32.ChecksumIEEE([]byte("scaleodm-schema-init")))
+
 //go:embed schema.sql
 var schemaSQL string
 
@@ -53,9 +59,7 @@ func (db *DB) Close() {
 
 // Create the required tables and indexes
 func (db *DB) InitSchema(ctx context.Context) error {
-	// Use advisory lock to prevent concurrent schema initialization
-	// Lock ID 123456 is arbitrary but consistent
-	lockID := int64(123456)
+	lockID := schemaLockID
 
 	// Use a transaction to ensure we use the same connection for lock/unlock
 	tx, err := db.Pool.Begin(ctx)

app/s3/files.go

@@ -99,9 +99,12 @@ extract_and_clean() {
   find "$dir" -type f \( -name "*.tar.gz" -o -name "*.tar" -o -name "*.TAR.GZ" -o -name "*.TAR" \) | while read tarfile; do
     found_archive=true
     echo "Extracting $tarfile..."
-    # Use --no-same-owner and strip leading path components to prevent path traversal
-    tar --no-same-owner --no-same-permissions -xf "$tarfile" -C "$(dirname "$tarfile")" 2>/dev/null || \
-    tar --no-same-owner --no-same-permissions -xzf "$tarfile" -C "$(dirname "$tarfile")" 2>/dev/null || true
+    # --no-same-owner: don't try to preserve ownership
+    # --no-same-permissions: don't try to preserve permissions
+    # --no-absolute-filenames: strip leading / to prevent writing outside target
+    # --transform: strip leading directory component (like -j for zip)
+    tar --no-same-owner --no-same-permissions --no-absolute-filenames --transform='s|.*/||' -xf "$tarfile" -C "$(dirname "$tarfile")" 2>/dev/null || \
+    tar --no-same-owner --no-same-permissions --no-absolute-filenames --transform='s|.*/||' -xzf "$tarfile" -C "$(dirname "$tarfile")" 2>/dev/null || true
     rm -f "$tarfile"
     check_extract_limits "$dir"
   done

chart/Chart.yaml

@@ -2,8 +2,8 @@ apiVersion: v2
 type: application
 name: scaleodm
 description: Kubernetes-native auto-scaling and load balancing for OpenDroneMap.
-version: "0.1.0"
-appVersion: "0.1.0"
+version: "0.2.0"
+appVersion: "0.2.0"
 maintainers:
 - email: sam.woodcock@hotosm.org
   name: Sam Woodcock

chart/README.md

@@ -98,8 +98,8 @@ At least one database and one S3 configuration path must be provided:
   - `database.postgres.enabled=true` and corresponding `database.postgres.auth.*` for the bundled Postgres subchart.
 
 - **S3** (two secrets required in the release namespace):
-  - `s3.external.secret.name` — Secret containing `SCALEODM_S3_ENDPOINT`, `SCALEODM_S3_ACCESS_KEY`, and `SCALEODM_S3_SECRET_KEY` (used by the API server).
-  - `s3.workflowSecret.name` — Secret containing `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `AWS_DEFAULT_REGION` (used by workflow pods via `secretKeyRef`).
+  - `s3.external.secret.name` - Secret containing `SCALEODM_S3_ENDPOINT`, `SCALEODM_S3_ACCESS_KEY`, and `SCALEODM_S3_SECRET_KEY` (used by the API server).
+  - `s3.workflowSecret.name` - Secret containing `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `AWS_DEFAULT_REGION` (used by workflow pods via `secretKeyRef`).
 
 Both secrets live in the release namespace. If you use the same credentials for both, you can point them at the same secret (provided the key names match).
 

chart/templates/deployment.yaml

@@ -22,10 +22,18 @@ spec:
         {{- toYaml . | nindent 8 }}
       {{- end }}
       serviceAccountName: {{ include "scaleodm.serviceAccountName" . }}
+      {{- with .Values.api.podSecurityContext }}
+      securityContext:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}
       containers:
         - name: {{ .Chart.Name }}
           image: "{{ .Values.global.imageRegistry }}{{ .Values.api.image.repository }}:{{ .Values.api.image.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.api.image.pullPolicy }}
+          {{- with .Values.api.securityContext }}
+          securityContext:
+            {{- toYaml . | nindent 12 }}
+          {{- end }}
           ports:
             - name: http
               containerPort: {{ .Values.api.port }}