security: fix all issues from security review

Critical:
- Verify MD5 integrity when caller provides expected hash (was skipped)
- Limit app file reads to 50 MiB to prevent OOM from corrupted objects

High:
- Sanitize S3 errors to avoid leaking bucket names and key paths
- Prevent path traversal via ".." in app filenames and file serving
- Fix fsck DocName to use stripped object name instead of full S3 key

Medium:
- Replace panic with error return in s3Copier.Copy
- Sanitize bucket_prefix config parameter

Low:
- Add bounds check for ETag substring to prevent panic
- Remove unused encoding/hex import

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: Crash--

docs/s3.md

@@ -30,21 +30,97 @@ for OVH, `localhost:9000` for MinIO).
 
 ### Local development with MinIO
 
+This tutorial explains how to set up a local S3 backend using MinIO for
+development and testing.
+
+**1. Start MinIO with Docker:**
+
+```bash
+docker run -d --name minio \
+  -p 9000:9000 \
+  -p 9001:9001 \
+  -e MINIO_ROOT_USER=minioadmin \
+  -e MINIO_ROOT_PASSWORD=minioadmin \
+  minio/minio server /data --console-address ":9001"
+```
+
+MinIO is now running:
+- S3 API: `http://localhost:9000`
+- Web console: `http://localhost:9001` (login: `minioadmin` / `minioadmin`)
+
+**2. Configure cozy-stack:**
+
+Buckets are created automatically at startup. No manual bucket creation
+is needed.
+
+Edit your `~/.cozy/cozy.yaml`:
+
 ```yaml
 fs:
   url: s3://localhost:9000?access_key=minioadmin&secret_key=minioadmin&bucket_prefix=cozy&use_ssl=false
 ```
 
-Start MinIO with Docker:
+**3. Build and start:**
 
 ```bash
-docker run -d --name minio -p 9000:9000 -p 9001:9001 \
-  -e MINIO_ROOT_USER=minioadmin \
-  -e MINIO_ROOT_PASSWORD=minioadmin \
-  minio/minio server /data --console-address ":9001"
+go build -o ~/go/bin/cozy-stack .
+~/go/bin/cozy-stack serve
+```
+
+You should see in the logs:
+
+```
+Successfully connected to S3 endpoint localhost:9000
+```
+
+**4. (Re)install your apps:**
+
+When switching from a different storage backend (e.g. `file://`), you need
+to reinstall the apps so their assets are stored in S3:
+
+```bash
+cozy-stack apps uninstall drive --domain your.domain.localhost:8080
+cozy-stack apps install drive --domain your.domain.localhost:8080
+cozy-stack apps uninstall home --domain your.domain.localhost:8080
+cozy-stack apps install home --domain your.domain.localhost:8080
+```
+
+**5. Verify:**
+
+Check that objects appear in MinIO:
+
+```bash
+docker exec minio mc ls --recursive local/cozy-apps-web/
+```
+
+Upload a file via the Drive UI or the API:
+
+```bash
+TOKEN=$(cozy-stack instances token-cli your.domain.localhost:8080 io.cozy.files)
+curl -X POST \
+  -H "Authorization: Bearer $TOKEN" \
+  -H "Content-Type: text/plain" \
+  "http://your.domain.localhost:8080/files/io.cozy.files.root-dir?Type=file&Name=test.txt" \
+  -d "Hello S3!"
+```
+
+Verify the file is in MinIO:
+
+```bash
+docker exec minio mc ls --recursive local/cozy-default/
+```
+
+**6. Switching back to local filesystem:**
+
+Comment out the S3 URL in your config and restart cozy-stack:
+
+```yaml
+fs:
+  # url: s3://localhost:9000?access_key=minioadmin&secret_key=minioadmin&bucket_prefix=cozy&use_ssl=false
 ```
 
-The MinIO console is available at `http://localhost:9001`.
+Note: files uploaded to S3 won't be accessible when using the local
+filesystem backend, and vice versa. Each backend has its own storage.
 
 ## Bucket strategy
 

model/vfs/vfss3/avatar.go

@@ -112,5 +112,6 @@ func wrapS3Err(err error) error {
 	if minio.ToErrorResponse(err).Code == "NoSuchBucket" {
 		return os.ErrNotExist
 	}
-	return err
+	// Sanitize S3 errors to avoid leaking internal bucket/key details
+	return fmt.Errorf("s3 storage error: %s", minio.ToErrorResponse(err).Code)
 }

model/vfs/vfss3/fsck.go

@@ -117,7 +117,7 @@ func (sfs *s3VFS) checkFiles(
 								DirDoc: &vfs.DirDoc{
 									Type:    consts.FileType,
 									DocID:   fileID,
-									DocName: obj.Key,
+									DocName: objName,
 								},
 							},
 						},

model/vfs/vfss3/impl.go

@@ -7,7 +7,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/md5"
-	"encoding/hex"
+
 	"errors"
 	"fmt"
 	"hash"
@@ -1030,23 +1030,17 @@ func (f *s3FileCreation) Close() (err error) {
 		return f.err
 	}
 
-	// Extract MD5 from ETag if available, otherwise use our local md5H
-	if newdoc.MD5Sum == nil {
-		etag := result.info.ETag
-		// ETags may be double-quoted
-		etag = strings.TrimPrefix(etag, "\"")
-		etag = strings.TrimSuffix(etag, "\"")
-		// For multipart uploads, ETag contains a dash — use local MD5
-		if strings.Contains(etag, "-") || etag == "" {
-			newdoc.MD5Sum = f.md5H.Sum(nil)
-		} else {
-			md5sum, err := hex.DecodeString(etag)
-			if err != nil {
-				newdoc.MD5Sum = f.md5H.Sum(nil)
-			} else {
-				newdoc.MD5Sum = md5sum
-			}
+	// Verify or compute MD5 checksum.
+	// The local md5H hash is always computed from the same data stream that
+	// goes to S3 (via the Write method), so it is authoritative.
+	localMD5 := f.md5H.Sum(nil)
+	if newdoc.MD5Sum != nil {
+		// The caller provided an expected hash — verify it matches what was written.
+		if !bytes.Equal(newdoc.MD5Sum, localMD5) {
+			return vfs.ErrInvalidHash
 		}
+	} else {
+		newdoc.MD5Sum = localMD5
 	}
 
 	if f.size < 0 {

pkg/appfs/s3.go

@@ -74,11 +74,16 @@ func (f *s3Copier) Start(slug, version, shasum string) (bool, error) {
 
 func (f *s3Copier) Copy(stat os.FileInfo, src io.Reader) error {
 	if !f.started {
-		panic("copier should call Start() before Copy()")
+		return fmt.Errorf("appfs: copier must call Start() before Copy()")
 	}
 
 	// Write directly to the final location (appObj/filename).
-	objName := path.Join(f.appObj, stat.Name())
+	// Reject path traversal attempts in filenames.
+	name := stat.Name()
+	if strings.Contains(name, "..") {
+		return fmt.Errorf("appfs: invalid filename %q", name)
+	}
+	objName := path.Join(f.appObj, name)
 
 	contentType := filetype.ByExtension(path.Ext(stat.Name()))
 	if contentType == "" {
@@ -173,15 +178,21 @@ func (s *s3Server) ServeFileContent(w http.ResponseWriter, req *http.Request, sl
 	}
 
 	if checkETag := req.Header.Get("Cache-Control") == ""; checkETag {
-		etag := fmt.Sprintf(`"%s"`, info.ETag[:10])
+		etagVal := info.ETag
+		if len(etagVal) > 10 {
+			etagVal = etagVal[:10]
+		}
+		etag := fmt.Sprintf(`"%s"`, etagVal)
 		if web_utils.CheckPreconditions(w, req, etag) {
 			return nil
 		}
 		w.Header().Set("Etag", etag)
 	}
 
 	// Read the full object to handle brotli decompression.
-	content, err := io.ReadAll(obj)
+	// Limit to 50 MiB to avoid unbounded memory allocation from corrupted objects.
+	const maxAppFileSize = 50 << 20
+	content, err := io.ReadAll(io.LimitReader(obj, maxAppFileSize))
 	if err != nil {
 		return err
 	}
@@ -269,6 +280,10 @@ func (s *s3Server) makeObjectName(slug, version, shasum, file string) string {
 	if shasum != "" {
 		basepath += "-" + shasum
 	}
+	// Prevent path traversal
+	if strings.Contains(file, "..") {
+		return basepath + "/invalid"
+	}
 	return path.Join(basepath, file)
 }
 
@@ -338,11 +353,16 @@ func isS3NotFound(err error) bool {
 	return code == "NoSuchKey" || code == "NoSuchBucket"
 }
 
-// wrapS3ErrNotExist converts S3 not-found errors to os.ErrNotExist.
+// wrapS3ErrNotExist converts S3 not-found errors to os.ErrNotExist and
+// sanitizes other S3 errors to avoid leaking internal bucket/key details.
 func wrapS3ErrNotExist(err error) error {
 	if isS3NotFound(err) {
 		return os.ErrNotExist
 	}
+	code := minio.ToErrorResponse(err).Code
+	if code != "" {
+		return fmt.Errorf("s3 storage error: %s", code)
+	}
 	return err
 }
 

pkg/config/config/s3.go

@@ -3,6 +3,7 @@ package config
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/minio/minio-go/v7"
 	"github.com/minio/minio-go/v7/pkg/credentials"
@@ -36,6 +37,15 @@ func InitS3Connection(fs Fs) error {
 	if s3BucketPrefix == "" {
 		s3BucketPrefix = "cozy"
 	}
+	// Sanitize bucket prefix: lowercase, only alphanumeric and hyphens
+	s3BucketPrefix = strings.ToLower(s3BucketPrefix)
+	s3BucketPrefix = strings.Map(func(r rune) rune {
+		if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' {
+			return r
+		}
+		return '-'
+	}, s3BucketPrefix)
+	s3BucketPrefix = strings.Trim(s3BucketPrefix, "-")
 	s3Region = region
 
 	var err error
@@ -60,6 +70,20 @@ func InitS3Connection(fs Fs) error {
 	}
 
 	log.Infof("Successfully connected to S3 endpoint %s", endpoint)
+
+	// Pre-create the fixed buckets used by secondary storage (apps, assets,
+	// previews, exports). The per-org VFS bucket is created on instance init.
+	ctx := context.Background()
+	for _, suffix := range []string{"-apps-web", "-apps-konnectors", "-assets", "-previews", "-exports"} {
+		bucket := s3BucketPrefix + suffix
+		if err := s3Client.MakeBucket(ctx, bucket, minio.MakeBucketOptions{Region: region}); err != nil {
+			code := minio.ToErrorResponse(err).Code
+			if code != "BucketAlreadyOwnedByYou" && code != "BucketAlreadyExists" {
+				log.Warnf("Could not create bucket %s: %s", bucket, err)
+			}
+		}
+	}
+
 	return nil
 }