fix(router): restrict KB-scoped file proxy to exports/ paths
Borrowers with shared-KB read access could previously request any owner-tenant storage path via /knowledge-bases/:id/files. Limit the proxy to exports/ embedded images; raw knowledge uploads continue to use /knowledge/:id/download.
This commit is contained in:
@@ -1642,8 +1642,8 @@ func newKBScopedFileServeHandler(
|
||||
return
|
||||
}
|
||||
|
||||
if err := secutils.ValidateStoragePathTenant(filePath, ownerTenantID); err != nil {
|
||||
logger.Warnf(ctx, "[Router] /knowledge-bases/:id/files denied path not owned by KB tenant: owner_tenant_id=%d file_path=%q err=%v",
|
||||
if err := secutils.ValidateKBScopedStoragePath(filePath, ownerTenantID); err != nil {
|
||||
logger.Warnf(ctx, "[Router] /knowledge-bases/:id/files denied path not allowed for KB proxy: owner_tenant_id=%d file_path=%q err=%v",
|
||||
ownerTenantID, filePath, err)
|
||||
c.JSON(http.StatusForbidden, gin.H{"error": "forbidden: file path not accessible"})
|
||||
return
|
||||
|
||||
@@ -266,6 +266,35 @@ func TestKBScopedFilesRejectsPathNotOwnedByKBTenant(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// KB-scoped proxy is for embedded exports/ images only; raw knowledge uploads
|
||||
// must use /knowledge/:id/download even when the tenant matches.
|
||||
func TestKBScopedFilesRejectsNonExportsPath(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
t.Setenv("STORAGE_TYPE", "local")
|
||||
|
||||
const ownerTenantID = uint64(10008)
|
||||
engine := newKBScopedFilesTestEngine(
|
||||
ownerTenantID,
|
||||
&stubTenantService{get: func(_ context.Context, id uint64) (*types.Tenant, error) {
|
||||
t.Fatalf("GetTenantByID should not be called for non-exports path, got %d", id)
|
||||
return nil, nil
|
||||
}},
|
||||
&stubFileService{getFile: func(_ context.Context, filePath string) (io.ReadCloser, error) {
|
||||
t.Fatalf("GetFile should not be called for non-exports path, got %q", filePath)
|
||||
return nil, nil
|
||||
}},
|
||||
)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet,
|
||||
"/knowledge-bases/kb-1/files?file_path="+url.QueryEscape("local://10008/knowledge-id/123.pdf"), nil)
|
||||
recorder := httptest.NewRecorder()
|
||||
engine.ServeHTTP(recorder, req)
|
||||
|
||||
if got, want := recorder.Code, http.StatusForbidden; got != want {
|
||||
t.Fatalf("status = %d, want %d", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestKBScopedFilesRequiresFilePath(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
|
||||
|
||||
@@ -95,9 +95,16 @@ func VerifyFileURLSig(filePath string, tenantID uint64, expiresStr, sig string)
|
||||
return hmac.Equal([]byte(expected), []byte(sig))
|
||||
}
|
||||
|
||||
// kbScopedExportsSegment is the only storage prefix served by the KB-scoped
|
||||
// file proxy. Embedded wiki/chunk images land under exports/; raw knowledge
|
||||
// uploads use {tenant}/{knowledgeID}/... and are served via
|
||||
// /knowledge/{id}/download instead.
|
||||
const kbScopedExportsSegment = "exports"
|
||||
|
||||
// ValidateStoragePathTenant ensures the tenant segment embedded in a provider://
|
||||
// storage path matches the authenticated caller's tenant. Cross-tenant access
|
||||
// must use /api/v1/files/presigned with an HMAC bound to the resource owner.
|
||||
// for arbitrary tenant paths uses /api/v1/files/presigned with an HMAC bound to
|
||||
// the resource owner; KB-scoped shared rendering uses ValidateKBScopedStoragePath.
|
||||
func ValidateStoragePathTenant(filePath string, tenantID uint64) error {
|
||||
pathTenant := ParseTenantIDFromStoragePath(filePath)
|
||||
if pathTenant == 0 {
|
||||
@@ -109,6 +116,46 @@ func ValidateStoragePathTenant(filePath string, tenantID uint64) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateKBScopedStoragePath is used by GET /knowledge-bases/:id/files. It
|
||||
// requires the path to belong to the KB owner tenant and to live under the
|
||||
// exports/ namespace used for embedded images (SaveBytes / multimodal output).
|
||||
// This prevents borrowers with shared-KB read access from using the proxy to
|
||||
// fetch arbitrary owner-tenant objects such as raw knowledge uploads.
|
||||
func ValidateKBScopedStoragePath(filePath string, tenantID uint64) error {
|
||||
if err := ValidateStoragePathTenant(filePath, tenantID); err != nil {
|
||||
return err
|
||||
}
|
||||
if !storagePathHasExportsScope(filePath, tenantID) {
|
||||
return fmt.Errorf("storage path is outside KB-scoped exports namespace")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// storagePathHasExportsScope reports whether tenantID appears next to an
|
||||
// exports segment in either canonical layout:
|
||||
// - {tenant}/exports/... (local, minio, s3, most cloud backends)
|
||||
// - exports/{tenant}/... (OSS temp-bucket layout)
|
||||
func storagePathHasExportsScope(filePath string, tenantID uint64) bool {
|
||||
_, rest, ok := strings.Cut(filePath, "://")
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
tenantSeg := strconv.FormatUint(tenantID, 10)
|
||||
parts := strings.Split(rest, "/")
|
||||
for i, part := range parts {
|
||||
if part != tenantSeg {
|
||||
continue
|
||||
}
|
||||
if i+1 < len(parts) && parts[i+1] == kbScopedExportsSegment {
|
||||
return true
|
||||
}
|
||||
if i > 0 && parts[i-1] == kbScopedExportsSegment {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// ParseTenantIDFromStoragePath extracts the tenant ID from a provider:// storage path.
|
||||
// Storage paths follow the convention: {scheme}://.../{tenantID}/...
|
||||
// Returns 0 if the path does not contain a valid tenant ID.
|
||||
|
||||
@@ -92,6 +92,18 @@ func TestValidateStoragePathTenant(t *testing.T) {
|
||||
assert.Error(t, ValidateStoragePathTenant("local://docs/example.txt", 42))
|
||||
}
|
||||
|
||||
func TestValidateKBScopedStoragePath(t *testing.T) {
|
||||
const tenantID uint64 = 10008
|
||||
|
||||
assert.NoError(t, ValidateKBScopedStoragePath("local://10008/exports/img.jpg", tenantID))
|
||||
assert.NoError(t, ValidateKBScopedStoragePath("minio://bucket/10008/exports/uuid.png", tenantID))
|
||||
assert.NoError(t, ValidateKBScopedStoragePath("oss://bucket/exports/10008/uuid.png", tenantID))
|
||||
|
||||
assert.Error(t, ValidateKBScopedStoragePath("local://10008/knowledge-id/123.pdf", tenantID))
|
||||
assert.Error(t, ValidateKBScopedStoragePath("local://9999/exports/img.jpg", tenantID))
|
||||
assert.Error(t, ValidateKBScopedStoragePath("local://10008/other/img.jpg", tenantID))
|
||||
}
|
||||
|
||||
func TestParseTenantIDFromStoragePath(t *testing.T) {
|
||||
tests := []struct {
|
||||
path string
|
||||
|
||||
Reference in New Issue
Block a user