fix(storage): merge tos/s3/oss/ks3 tenant configs in buildStorageConfig (#1117)

knowledgeService.buildStorageConfig had hardcoded switches in two places that
only handled local/minio/cos. Knowledge bases configured for tos, s3, oss, or
ks3 reached the docreader with a DocParserStorageConfig containing only the
Provider tag — bucket, endpoint, and credentials were silently dropped, and
extraction stalled or fetched from the wrong location.

- Tenant-merge switch: add tos/s3/oss/ks3 cases mapping
  StorageEngineConfig.{TOS,S3,OSS,KS3} fields onto DocParserStorageConfig.
  Same shape as the existing minio/cos branches; AccessKey -> AccessKeyID,
  SecretKey -> SecretAccessKey, no AppID.
- Legacy compat switch: list tos/s3/oss/ks3 alongside local with hasKBFull=false
  so fall-through to the tenant-merge path is intentional rather than an
  unrecognised provider sliding past the switch. The legacy `cos_config` column
  predates these providers and they should never resolve via that path.
- Doc drift: types/custom_agent.go ImageStorageProvider comment and
  handler/system.go StorageEngineStatusItem.Name comment now list all 7
  providers (matched against types.StorageEngineConfig and ParseProviderScheme).

Tests (knowledge_storage_config_test.go, +268 LOC, 17 cases):
- TestBuildStorageConfig_TenantMergeAllProviders pins per-provider field
  population for all 7 providers; the tos/s3/oss/ks3 sub-tests are the
  regression guard for #1117.
- TestBuildStorageConfig_LegacyPathOnlyForCOSAndMinIO pins that legacy
  StorageConfig data is only consumed for cos/minio and skipped for the
  others (intentional fall-through).
- TestBuildStorageConfig_NoTenantFallsThroughToEmpty pins the no-tenant case.

Out of scope (called out in the linked issue):
- The systemic refactor (single source-of-truth provider registry) — left as
  a follow-up; this PR keeps changes local to the buggy code path.
- initialization.go validateStorageEngineConfig multimodal validation gap —
  the InitializationRequest struct does not carry tos/s3/oss/ks3 fields, so
  fixing it requires API surface additions; can ship separately.

Closes #1117 (the buildStorageConfig switches; remaining items deferred).

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

internal/application/service/knowledge.go

@@ -7642,6 +7642,10 @@ func (s *knowledgeService) buildStorageConfig(ctx context.Context, kb *types.Kno
 	}
 
 	// Backward compatibility: if legacy cos_config has full params for the chosen provider, use them.
+	// Note: legacy StorageConfig predates tos/s3/oss/ks3, so those providers always
+	// resolve via the tenant-merge path below. Listing them here keeps the fall-through
+	// intentional (instead of an unrecognised provider silently sliding past the switch).
+	// See issue #1117: provider enum was missing tos/s3/oss in this switch.
 	sc := &kb.StorageConfig
 	hasKBFull := false
 	switch provider {
@@ -7649,7 +7653,7 @@ func (s *knowledgeService) buildStorageConfig(ctx context.Context, kb *types.Kno
 		hasKBFull = sc.SecretID != "" && sc.BucketName != ""
 	case "minio":
 		hasKBFull = sc.BucketName != ""
-	case "local":
+	case "local", "tos", "s3", "oss", "ks3":
 		hasKBFull = false
 	}
 
@@ -7678,6 +7682,10 @@ func (s *knowledgeService) buildStorageConfig(ctx context.Context, kb *types.Kno
 			provider = strings.ToLower(strings.TrimSpace(sec.DefaultProvider))
 			out.Provider = strings.ToUpper(provider)
 		}
+		// Provider list must match types.StorageEngineConfig + ParseProviderScheme.
+		// Missing a case here causes DocParserStorageConfig to be returned with only
+		// Provider set — bucket/endpoint/credentials are silently dropped, and the
+		// docreader then fails or fetches from the wrong location. See issue #1117.
 		switch provider {
 		case "local":
 			if sec.Local != nil {
@@ -7706,6 +7714,42 @@ func (s *knowledgeService) buildStorageConfig(ctx context.Context, kb *types.Kno
 				out.AppID = sec.COS.AppID
 				out.PathPrefix = sec.COS.PathPrefix
 			}
+		case "tos":
+			if sec.TOS != nil {
+				out.Endpoint = sec.TOS.Endpoint
+				out.Region = sec.TOS.Region
+				out.AccessKeyID = sec.TOS.AccessKey
+				out.SecretAccessKey = sec.TOS.SecretKey
+				out.BucketName = sec.TOS.BucketName
+				out.PathPrefix = sec.TOS.PathPrefix
+			}
+		case "s3":
+			if sec.S3 != nil {
+				out.Endpoint = sec.S3.Endpoint
+				out.Region = sec.S3.Region
+				out.AccessKeyID = sec.S3.AccessKey
+				out.SecretAccessKey = sec.S3.SecretKey
+				out.BucketName = sec.S3.BucketName
+				out.PathPrefix = sec.S3.PathPrefix
+			}
+		case "oss":
+			if sec.OSS != nil {
+				out.Endpoint = sec.OSS.Endpoint
+				out.Region = sec.OSS.Region
+				out.AccessKeyID = sec.OSS.AccessKey
+				out.SecretAccessKey = sec.OSS.SecretKey
+				out.BucketName = sec.OSS.BucketName
+				out.PathPrefix = sec.OSS.PathPrefix
+			}
+		case "ks3":
+			if sec.KS3 != nil {
+				out.Endpoint = sec.KS3.Endpoint
+				out.Region = sec.KS3.Region
+				out.AccessKeyID = sec.KS3.AccessKey
+				out.SecretAccessKey = sec.KS3.SecretKey
+				out.BucketName = sec.KS3.BucketName
+				out.PathPrefix = sec.KS3.PathPrefix
+			}
 		}
 	}
 

internal/application/service/knowledge_storage_config_test.go

@@ -0,0 +1,294 @@
+package service
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Tencent/WeKnora/internal/types"
+)
+
+// TestBuildStorageConfig_TenantMergeAllProviders pins the tenant-merge branch
+// of (knowledgeService).buildStorageConfig: every provider listed in
+// types.StorageEngineConfig and types.ParseProviderScheme must produce a
+// fully-populated DocParserStorageConfig when the tenant carries the matching
+// engine config. Before issue #1117 was fixed, tos/s3/oss/ks3 fell through the
+// switch and produced a result with only Provider set, silently dropping the
+// bucket / endpoint / credentials and stalling docreader.
+func TestBuildStorageConfig_TenantMergeAllProviders(t *testing.T) {
+	t.Parallel()
+
+	// Each table entry sets one engine config on a fresh tenant and checks that
+	// the merged DocParserStorageConfig surfaces the values the docreader needs.
+	type want struct {
+		provider        string // expected DocParserStorageConfig.Provider (uppercase)
+		region          string
+		bucket          string
+		accessKeyID     string
+		secretAccessKey string
+		appID           string
+		pathPrefix      string
+		endpoint        string
+	}
+
+	tests := []struct {
+		name     string
+		kbConfig *types.StorageProviderConfig
+		tenant   *types.Tenant
+		want     want
+	}{
+		{
+			name:     "local — only path prefix",
+			kbConfig: &types.StorageProviderConfig{Provider: "local"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				Local: &types.LocalEngineConfig{PathPrefix: "/data/wk"},
+			}},
+			want: want{provider: "LOCAL", pathPrefix: "/data/wk"},
+		},
+		{
+			name:     "minio remote — full credentials",
+			kbConfig: &types.StorageProviderConfig{Provider: "minio"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				MinIO: &types.MinIOEngineConfig{
+					Mode:            "remote",
+					Endpoint:        "minio.example.com:9000",
+					AccessKeyID:     "minio-ak",
+					SecretAccessKey: "minio-sk",
+					BucketName:      "kb-minio",
+					PathPrefix:      "wk/",
+				},
+			}},
+			want: want{
+				provider: "MINIO", endpoint: "minio.example.com:9000",
+				accessKeyID: "minio-ak", secretAccessKey: "minio-sk",
+				bucket: "kb-minio", pathPrefix: "wk/",
+			},
+		},
+		{
+			name:     "cos — appid carried through",
+			kbConfig: &types.StorageProviderConfig{Provider: "cos"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				COS: &types.COSEngineConfig{
+					Region: "ap-guangzhou", BucketName: "kb-cos-1255000000",
+					SecretID: "cos-sid", SecretKey: "cos-sk",
+					AppID: "1255000000", PathPrefix: "wk/",
+				},
+			}},
+			want: want{
+				provider: "COS", region: "ap-guangzhou", bucket: "kb-cos-1255000000",
+				accessKeyID: "cos-sid", secretAccessKey: "cos-sk",
+				appID: "1255000000", pathPrefix: "wk/",
+			},
+		},
+		{
+			name:     "tos — issue #1117 regression guard",
+			kbConfig: &types.StorageProviderConfig{Provider: "tos"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				TOS: &types.TOSEngineConfig{
+					Endpoint: "tos-cn-beijing.volces.com", Region: "cn-beijing",
+					AccessKey: "tos-ak", SecretKey: "tos-sk",
+					BucketName: "kb-tos", PathPrefix: "wk/",
+				},
+			}},
+			want: want{
+				provider: "TOS", endpoint: "tos-cn-beijing.volces.com", region: "cn-beijing",
+				accessKeyID: "tos-ak", secretAccessKey: "tos-sk",
+				bucket: "kb-tos", pathPrefix: "wk/",
+			},
+		},
+		{
+			name:     "s3 — issue #1117 regression guard",
+			kbConfig: &types.StorageProviderConfig{Provider: "s3"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				S3: &types.S3EngineConfig{
+					Endpoint: "s3.us-east-1.amazonaws.com", Region: "us-east-1",
+					AccessKey: "AKIA...", SecretKey: "s3-sk",
+					BucketName: "kb-s3", PathPrefix: "wk/",
+				},
+			}},
+			want: want{
+				provider: "S3", endpoint: "s3.us-east-1.amazonaws.com", region: "us-east-1",
+				accessKeyID: "AKIA...", secretAccessKey: "s3-sk",
+				bucket: "kb-s3", pathPrefix: "wk/",
+			},
+		},
+		{
+			name:     "oss — issue #1117 regression guard",
+			kbConfig: &types.StorageProviderConfig{Provider: "oss"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				OSS: &types.OSSEngineConfig{
+					Endpoint: "oss-cn-hangzhou.aliyuncs.com", Region: "cn-hangzhou",
+					AccessKey: "oss-ak", SecretKey: "oss-sk",
+					BucketName: "kb-oss", PathPrefix: "wk/",
+				},
+			}},
+			want: want{
+				provider: "OSS", endpoint: "oss-cn-hangzhou.aliyuncs.com", region: "cn-hangzhou",
+				accessKeyID: "oss-ak", secretAccessKey: "oss-sk",
+				bucket: "kb-oss", pathPrefix: "wk/",
+			},
+		},
+		{
+			name:     "ks3 — added in #1109, must not regress",
+			kbConfig: &types.StorageProviderConfig{Provider: "ks3"},
+			tenant: &types.Tenant{StorageEngineConfig: &types.StorageEngineConfig{
+				KS3: &types.KS3EngineConfig{
+					Endpoint: "ks3-cn-beijing.ksyuncs.com", Region: "cn-beijing",
+					AccessKey: "ks3-ak", SecretKey: "ks3-sk",
+					BucketName: "kb-ks3", PathPrefix: "wk/",
+				},
+			}},
+			want: want{
+				provider: "KS3", endpoint: "ks3-cn-beijing.ksyuncs.com", region: "cn-beijing",
+				accessKeyID: "ks3-ak", secretAccessKey: "ks3-sk",
+				bucket: "kb-ks3", pathPrefix: "wk/",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			kb := &types.KnowledgeBase{
+				ID:                    "kb-" + tt.want.provider,
+				StorageProviderConfig: tt.kbConfig,
+			}
+			ctx := context.WithValue(context.Background(), types.TenantInfoContextKey, tt.tenant)
+
+			s := &knowledgeService{}
+			got := s.buildStorageConfig(ctx, kb)
+			if got == nil {
+				t.Fatalf("buildStorageConfig returned nil")
+			}
+			if got.Provider != tt.want.provider {
+				t.Errorf("Provider = %q, want %q", got.Provider, tt.want.provider)
+			}
+			if got.Region != tt.want.region {
+				t.Errorf("Region = %q, want %q", got.Region, tt.want.region)
+			}
+			if got.BucketName != tt.want.bucket {
+				t.Errorf("BucketName = %q, want %q", got.BucketName, tt.want.bucket)
+			}
+			if got.AccessKeyID != tt.want.accessKeyID {
+				t.Errorf("AccessKeyID = %q, want %q", got.AccessKeyID, tt.want.accessKeyID)
+			}
+			if got.SecretAccessKey != tt.want.secretAccessKey {
+				t.Errorf("SecretAccessKey = %q, want %q", got.SecretAccessKey, tt.want.secretAccessKey)
+			}
+			if got.AppID != tt.want.appID {
+				t.Errorf("AppID = %q, want %q", got.AppID, tt.want.appID)
+			}
+			if got.PathPrefix != tt.want.pathPrefix {
+				t.Errorf("PathPrefix = %q, want %q", got.PathPrefix, tt.want.pathPrefix)
+			}
+			if got.Endpoint != tt.want.endpoint {
+				t.Errorf("Endpoint = %q, want %q", got.Endpoint, tt.want.endpoint)
+			}
+		})
+	}
+}
+
+// TestBuildStorageConfig_LegacyPathOnlyForCOSAndMinIO verifies that the legacy
+// (kb.StorageConfig "cos_config" column) path is only used for the providers
+// that historically wrote into it — cos and minio. tos/s3/oss/ks3 must always
+// resolve through the tenant-merge path, so a populated legacy struct on those
+// providers is intentionally ignored rather than silently aliased onto the
+// wrong fields.
+func TestBuildStorageConfig_LegacyPathOnlyForCOSAndMinIO(t *testing.T) {
+	t.Parallel()
+
+	// Legacy StorageConfig with full COS-shape fields. cos+minio should pick it
+	// up; everything else falls through to the tenant merge.
+	legacy := types.StorageConfig{
+		SecretID:   "legacy-sid",
+		SecretKey:  "legacy-sk",
+		Region:     "ap-guangzhou",
+		BucketName: "legacy-bucket",
+		AppID:      "1255000000",
+		PathPrefix: "wk/",
+	}
+
+	tests := []struct {
+		name        string
+		provider    string
+		wantBucket  string // populated when legacy path was taken
+		wantSecret  string
+		wantAppID   string
+		wantTenants bool // true when the test should fall through to tenant merge (empty result)
+	}{
+		{name: "cos uses legacy", provider: "cos", wantBucket: "legacy-bucket", wantSecret: "legacy-sid", wantAppID: "1255000000"},
+		{name: "minio uses legacy", provider: "minio", wantBucket: "legacy-bucket", wantSecret: "legacy-sid", wantAppID: "1255000000"},
+		{name: "local skips legacy", provider: "local", wantTenants: true},
+		{name: "tos skips legacy", provider: "tos", wantTenants: true},
+		{name: "s3 skips legacy", provider: "s3", wantTenants: true},
+		{name: "oss skips legacy", provider: "oss", wantTenants: true},
+		{name: "ks3 skips legacy", provider: "ks3", wantTenants: true},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			kb := &types.KnowledgeBase{
+				ID:                    "kb-legacy-" + tt.provider,
+				StorageConfig:         legacy,
+				StorageProviderConfig: &types.StorageProviderConfig{Provider: tt.provider},
+			}
+			ctx := context.Background() // no tenant config → tenant-merge yields empty
+
+			s := &knowledgeService{}
+			got := s.buildStorageConfig(ctx, kb)
+			if got == nil {
+				t.Fatalf("buildStorageConfig returned nil")
+			}
+
+			if tt.wantTenants {
+				// Legacy path NOT taken → fields stay empty (Provider is the only thing set).
+				if got.BucketName != "" || got.AccessKeyID != "" || got.AppID != "" {
+					t.Errorf("provider=%s expected legacy path skipped, got bucket=%q ak=%q appid=%q",
+						tt.provider, got.BucketName, got.AccessKeyID, got.AppID)
+				}
+				return
+			}
+			if got.BucketName != tt.wantBucket {
+				t.Errorf("BucketName = %q, want %q", got.BucketName, tt.wantBucket)
+			}
+			if got.AccessKeyID != tt.wantSecret {
+				t.Errorf("AccessKeyID (mapped from legacy SecretID) = %q, want %q", got.AccessKeyID, tt.wantSecret)
+			}
+			if got.AppID != tt.wantAppID {
+				t.Errorf("AppID = %q, want %q", got.AppID, tt.wantAppID)
+			}
+		})
+	}
+}
+
+// TestBuildStorageConfig_NoTenantFallsThroughToEmpty pins the no-tenant case:
+// when neither the kb nor the context carry storage info, the result is an
+// empty config tagged with the requested provider — matches the "fall back to
+// global default" intent rather than panicking.
+func TestBuildStorageConfig_NoTenantFallsThroughToEmpty(t *testing.T) {
+	t.Parallel()
+
+	for _, p := range []string{"local", "minio", "cos", "tos", "s3", "oss", "ks3"} {
+		p := p
+		t.Run(p, func(t *testing.T) {
+			t.Parallel()
+			kb := &types.KnowledgeBase{
+				ID:                    "kb-empty-" + p,
+				StorageProviderConfig: &types.StorageProviderConfig{Provider: p},
+			}
+			s := &knowledgeService{}
+			got := s.buildStorageConfig(context.Background(), kb)
+			if got == nil {
+				t.Fatalf("buildStorageConfig returned nil for %s", p)
+			}
+			// Provider tag is uppercase per the implementation contract.
+			if got.Provider == "" {
+				t.Errorf("Provider tag missing for %s", p)
+			}
+		})
+	}
+}

internal/handler/system.go

@@ -454,7 +454,7 @@ func (h *SystemHandler) isTOSEnvAvailable() bool {
 
 // StorageEngineStatusItem describes one storage engine's availability and description.
 type StorageEngineStatusItem struct {
-	Name        string `json:"name"` // "local", "minio", "cos", "tos"
+	Name        string `json:"name"` // "local", "minio", "cos", "tos", "s3", "oss", "ks3"
 	Allowed     bool   `json:"allowed"`
 	Available   bool   `json:"available"`   // whether the engine can be used
 	Description string `json:"description"` // short description for UI

internal/types/custom_agent.go

@@ -158,7 +158,7 @@ type CustomAgentConfig struct {
 	AudioUploadEnabled bool `yaml:"audio_upload_enabled" json:"audio_upload_enabled"`
 	// ASR model ID for audio transcription (optional)
 	ASRModelID string `yaml:"asr_model_id" json:"asr_model_id"`
-	// Storage provider for image uploads: "local", "minio", "cos", "tos"
+	// Storage provider for image uploads: "local", "minio", "cos", "tos", "s3", "oss", "ks3".
 	// Empty means use the global/tenant default provider.
 	ImageStorageProvider string `yaml:"image_storage_provider" json:"image_storage_provider"`