feat: LibreChat v0.8.7 compatibility - hardened (supersedes #73)

docker/runner/files.go

@@ -6,14 +6,16 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 )
 
 // FileInfo represents a file in the working directory.
 type FileInfo struct {
-	Name    string  `json:"name"`
-	Path    string  `json:"path"`
-	Size    int64   `json:"size"`
-	ModTime int64   `json:"mod_time"`
+	Name     string  `json:"name"`
+	Path     string  `json:"path"`
+	Size     int64   `json:"size"`
+	ModTime  int64   `json:"mod_time"`
 	MimeType *string `json:"mime_type,omitempty"`
 }
 
@@ -46,18 +48,57 @@ func (h *FileHandler) validatePath(reqPath string) (string, error) {
 }
 
 // HandleUpload processes POST /files (multipart file upload).
+//
+// RFC 7578 (and Go's net/http multipart parser) strip directory components
+// from the part filename, so nested skill bundles cannot ride on the filename
+// alone. The control plane therefore sends the intended relative path in an
+// out-of-band “path“ form field (one value per file, in order). When absent,
+// the basenamed filename is used, preserving legacy single-file behavior.
 func (h *FileHandler) HandleUpload(w http.ResponseWriter, r *http.Request) {
 	if err := r.ParseMultipartForm(32 << 20); err != nil {
 		writeJSON(w, http.StatusBadRequest, map[string]string{"detail": "Invalid multipart form"})
 		return
 	}
 
+	absWD, _ := filepath.Abs(h.workingDir)
+	explicitPaths := r.MultipartForm.Value["path"]
+
+	// Iterate multipart field names in sorted order: map iteration order is
+	// randomized, and the out-of-band explicit paths are matched to files
+	// positionally. The control plane sends a single field per request, but
+	// determinism must not depend on that.
+	fieldNames := make([]string, 0, len(r.MultipartForm.File))
+	for name := range r.MultipartForm.File {
+		fieldNames = append(fieldNames, name)
+	}
+	sort.Strings(fieldNames)
+
 	var uploaded []FileInfo
+	idx := 0
+	for _, fieldName := range fieldNames {
+		for _, fh := range r.MultipartForm.File[fieldName] {
+			// Prefer the explicit relative path (preserves nested layout);
+			// fall back to the parser-basenamed filename.
+			rawName := fh.Filename
+			if idx < len(explicitPaths) && explicitPaths[idx] != "" {
+				rawName = explicitPaths[idx]
+			}
+			idx++
 
-	for _, fileHeaders := range r.MultipartForm.File {
-		for _, fh := range fileHeaders {
-			safeName := filepath.Base(fh.Filename)
-			if safeName == "" || safeName[0] == '.' {
+			destPath, err := h.validatePath(filepath.ToSlash(rawName))
+			if err != nil {
+				continue
+			}
+			base := filepath.Base(destPath)
+			if base == "" || base == "." || base == ".." || base[0] == '.' || destPath == absWD {
+				// Skip empty, traversal, hidden, or working-dir targets.
+				continue
+			}
+			if hasHiddenSegment(destPath, absWD) {
+				// No segment of the relative path may be hidden: nested
+				// uploads must not smuggle dotfile directories (.ssh, .git,
+				// .bashrc-adjacent paths) that the flat basename check above
+				// cannot see.
 				continue
 			}
 
@@ -66,7 +107,11 @@ func (h *FileHandler) HandleUpload(w http.ResponseWriter, r *http.Request) {
 				continue
 			}
 
-			destPath := filepath.Join(h.workingDir, safeName)
+			if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil {
+				src.Close()
+				continue
+			}
+
 			dst, err := os.Create(destPath)
 			if err != nil {
 				src.Close()
@@ -84,8 +129,12 @@ func (h *FileHandler) HandleUpload(w http.ResponseWriter, r *http.Request) {
 				modTime = fi.ModTime().Unix()
 			}
 
+			rel, err := filepath.Rel(absWD, destPath)
+			if err != nil {
+				rel = base
+			}
 			uploaded = append(uploaded, FileInfo{
-				Name:    safeName,
+				Name:    filepath.ToSlash(rel),
 				Path:    destPath,
 				Size:    n,
 				ModTime: modTime,
@@ -96,30 +145,59 @@ func (h *FileHandler) HandleUpload(w http.ResponseWriter, r *http.Request) {
 	writeJSON(w, http.StatusOK, map[string]any{"uploaded": uploaded})
 }
 
+// hasHiddenSegment reports whether any segment of destPath relative to the
+// working directory starts with a dot.
+func hasHiddenSegment(destPath, absWD string) bool {
+	rel, err := filepath.Rel(absWD, destPath)
+	if err != nil {
+		return true
+	}
+	for _, seg := range strings.Split(filepath.ToSlash(rel), "/") {
+		if seg != "" && seg[0] == '.' {
+			return true
+		}
+	}
+	return false
+}
+
 // HandleList processes GET /files (list working directory).
+//
+// The listing is recursive and returns each regular file's path relative to
+// the working directory (e.g. "skillName/SKILL.md"). This keeps nested skill
+// bundles and nested generated files visible to the control plane, which
+// matches them against mounted files by relative path.
 func (h *FileHandler) HandleList(w http.ResponseWriter, r *http.Request) {
-	entries, err := os.ReadDir(h.workingDir)
+	absWD, err := filepath.Abs(h.workingDir)
 	if err != nil {
 		http.Error(w, `{"detail":"Working directory not found"}`, http.StatusNotFound)
 		return
 	}
 
-	files := make([]FileInfo, 0, len(entries))
-	for _, e := range entries {
-		info, err := e.Info()
+	files := make([]FileInfo, 0)
+	walkErr := filepath.WalkDir(h.workingDir, func(p string, d os.DirEntry, walkErr error) error {
+		if walkErr != nil || d.IsDir() {
+			return nil
+		}
+		info, err := d.Info()
 		if err != nil {
-			continue
+			return nil
 		}
-		size := info.Size()
-		if e.IsDir() {
-			size = 0
+		rel, err := filepath.Rel(absWD, p)
+		if err != nil {
+			return nil
 		}
+		relSlash := filepath.ToSlash(rel)
 		files = append(files, FileInfo{
-			Name:    e.Name(),
-			Path:    e.Name(),
-			Size:    size,
+			Name:    relSlash,
+			Path:    relSlash,
+			Size:    info.Size(),
 			ModTime: info.ModTime().Unix(),
 		})
+		return nil
+	})
+	if walkErr != nil {
+		http.Error(w, `{"detail":"Working directory not found"}`, http.StatusNotFound)
+		return
 	}
 
 	writeJSON(w, http.StatusOK, map[string]any{"files": files})

docker/runner/files_test.go

@@ -1,6 +1,11 @@
 package main
 
 import (
+	"bytes"
+	"encoding/json"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"testing"
@@ -107,3 +112,156 @@ func TestHandleListIncludesModTime(t *testing.T) {
 
 	_ = h // keep linter happy
 }
+
+// uploadMultipart builds a POST /files multipart request. The file part is
+// sent with `basename` as its filename (Go's parser strips directories anyway)
+// and the nested layout is carried out-of-band in the `path` form field, which
+// mirrors how the control plane transports skill bundles like
+// "skillName/SKILL.md".
+func uploadMultipart(t *testing.T, relPath string, content []byte) *http.Request {
+	t.Helper()
+	var body bytes.Buffer
+	mw := multipart.NewWriter(&body)
+	part, err := mw.CreateFormFile("files", filepath.Base(relPath))
+	if err != nil {
+		t.Fatalf("CreateFormFile: %v", err)
+	}
+	if _, err := part.Write(content); err != nil {
+		t.Fatalf("write part: %v", err)
+	}
+	if err := mw.WriteField("path", relPath); err != nil {
+		t.Fatalf("write path field: %v", err)
+	}
+	if err := mw.Close(); err != nil {
+		t.Fatalf("close writer: %v", err)
+	}
+	req := httptest.NewRequest(http.MethodPost, "/files", &body)
+	req.Header.Set("Content-Type", mw.FormDataContentType())
+	return req
+}
+
+func TestHandleUploadPreservesNestedPath(t *testing.T) {
+	dir := t.TempDir()
+	h := NewFileHandler(dir)
+
+	rr := httptest.NewRecorder()
+	h.HandleUpload(rr, uploadMultipart(t, "skillName/SKILL.md", []byte("# skill")))
+
+	if rr.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", rr.Code)
+	}
+
+	// The file must land at the nested path, not be flattened to the basename.
+	nested := filepath.Join(dir, "skillName", "SKILL.md")
+	if _, err := os.Stat(nested); err != nil {
+		t.Fatalf("expected nested file at %s: %v", nested, err)
+	}
+	if _, err := os.Stat(filepath.Join(dir, "SKILL.md")); err == nil {
+		t.Error("file must not be flattened to working-dir root")
+	}
+
+	var resp struct {
+		Uploaded []FileInfo `json:"uploaded"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+	if len(resp.Uploaded) != 1 || resp.Uploaded[0].Name != "skillName/SKILL.md" {
+		t.Errorf("expected uploaded name skillName/SKILL.md, got %+v", resp.Uploaded)
+	}
+}
+
+func TestHandleUploadRejectsTraversal(t *testing.T) {
+	dir := t.TempDir()
+	h := NewFileHandler(dir)
+
+	rr := httptest.NewRecorder()
+	h.HandleUpload(rr, uploadMultipart(t, "../escape.txt", []byte("pwned")))
+
+	// The traversal target must never be written outside the working dir.
+	if _, err := os.Stat(filepath.Join(filepath.Dir(dir), "escape.txt")); err == nil {
+		t.Fatal("path traversal escaped the working directory")
+	}
+
+	var resp struct {
+		Uploaded []FileInfo `json:"uploaded"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+	if len(resp.Uploaded) != 0 {
+		t.Errorf("traversal upload must be skipped, got %+v", resp.Uploaded)
+	}
+}
+
+func TestHandleUploadRejectsHiddenSegments(t *testing.T) {
+	dir := t.TempDir()
+	h := NewFileHandler(dir)
+
+	for _, name := range []string{".git/config", "skill/.ssh/authorized_keys", ".hidden.txt"} {
+		rr := httptest.NewRecorder()
+		h.HandleUpload(rr, uploadMultipart(t, name, []byte("nope")))
+
+		var resp struct {
+			Uploaded []FileInfo `json:"uploaded"`
+		}
+		if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
+			t.Fatalf("decode response: %v", err)
+		}
+		if len(resp.Uploaded) != 0 {
+			t.Errorf("upload of %q must be skipped (hidden segment), got %+v", name, resp.Uploaded)
+		}
+	}
+
+	// Nothing may have been written anywhere under the working dir.
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatalf("read working dir: %v", err)
+	}
+	if len(entries) != 0 {
+		t.Errorf("working dir must stay empty, got %v", entries)
+	}
+}
+
+func TestHandleListIsRecursive(t *testing.T) {
+	dir := t.TempDir()
+	h := NewFileHandler(dir)
+
+	if err := os.MkdirAll(filepath.Join(dir, "skillName"), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "skillName", "SKILL.md"), []byte("x"), 0o644); err != nil {
+		t.Fatalf("write nested: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "top.txt"), []byte("y"), 0o644); err != nil {
+		t.Fatalf("write top: %v", err)
+	}
+
+	rr := httptest.NewRecorder()
+	h.HandleList(rr, httptest.NewRequest(http.MethodGet, "/files", nil))
+	if rr.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d", rr.Code)
+	}
+
+	var resp struct {
+		Files []FileInfo `json:"files"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+
+	names := map[string]bool{}
+	for _, f := range resp.Files {
+		names[f.Name] = true
+	}
+	if !names["skillName/SKILL.md"] {
+		t.Errorf("expected recursive listing to include skillName/SKILL.md, got %+v", resp.Files)
+	}
+	if !names["top.txt"] {
+		t.Errorf("expected listing to include top.txt, got %+v", resp.Files)
+	}
+	// Directories themselves must not be reported as files.
+	if names["skillName"] {
+		t.Error("directory entries must not appear in the file listing")
+	}
+}

scripts/build-images.sh

@@ -362,10 +362,41 @@ main() {
     echo "Building ${#all_images[@]} images..."
     echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
 
+    # Build the runner image FIRST (synchronously). Every language image does
+    # `FROM ${RUNNER_IMAGE} AS runner`, so the runner must already be built and
+    # tagged locally before those builds start — otherwise their `FROM`
+    # resolves against the registry tag (which only exists after a push) and
+    # fails with "not found". Gate the rest of the build on its success.
+    for entry in "${INFRA_IMAGES[@]}"; do
+        IFS=':' read -r dockerfile image_name context_dir <<< "$entry"
+        [[ "$image_name" != "runner" ]] && continue
+
+        if [[ ! -f "$DOCKER_DIR/$dockerfile" ]]; then
+            echo "Error: runner Dockerfile not found: $dockerfile"
+            exit 1
+        fi
+
+        echo "Starting: runner (dependency — built first)"
+        build_image_wrapper "$dockerfile" "$image_name" "$context_dir"
+        if ! ( source "$RESULTS_DIR/${image_name}.result"; [[ "$EXIT_CODE" -eq 0 ]] ); then
+            echo ""
+            echo "runner image build failed — aborting (all language images depend on it)."
+            if [[ -f "$RESULTS_DIR/${image_name}.result.log" ]]; then
+                echo "━━━ runner ━━━"
+                tail -n 30 "$RESULTS_DIR/${image_name}.result.log"
+            fi
+            exit 1
+        fi
+        break
+    done
+
     for entry in "${all_images[@]}"; do
         # Parse entry: dockerfile:image_name:context_dir
         IFS=':' read -r dockerfile image_name context_dir <<< "$entry"
 
+        # runner was already built above as a prerequisite for the rest
+        [[ "$image_name" == "runner" ]] && continue
+
         if [[ ! -f "$DOCKER_DIR/$dockerfile" ]]; then
             echo "Warning: Dockerfile not found: $dockerfile"
             continue