diff --git a/AGENTS.md b/AGENTS.md index 41ae169a..712788a7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,3 +38,9 @@ CloudFile Server、Hub、Docker 共用以下规则： ## 代码习惯 代码和提交信息用英文，中文维护文档；C 保持上游风格，RPC 优先 `cf_` 前缀；明确内存所有权。不要提交构建产物、密钥、开发环境临时文件。 + +## 开发期数据库变更（2026-10-21） + +- 新版当前只明确新增四张业务表：`cf_library_storage`、`cf_dir_acl`、`cf_sso_group_map`、`cf_edit_guard`。历史表仅作备份或迁移输入，不属于新版运行库及运行依赖；增加业务表须先确认，历史数据单独归档，不直接删除。 +- 开发期建表、升级及数据迁移通过 Docker 仓 `database/` 下独立、人工执行的 SQL 交付，DEV 验证与 PROD 发布复用固定脚本及校验和。启动/重启仅校验，不自动执行扩展 DDL、旧迁移链或兼容补丁。 +- 仅明确审核的成熟版本之间才考虑自动升级；须定义精确源/目标范围和数据/中断恢复验证。不得根据 dev 分支、表名存在或旧台账自动推断升级路径，不自动回滚。 diff --git a/README.testing.md b/README.testing.md index 081d6e53..331c8a5b 100644 --- a/README.testing.md +++ b/README.testing.md @@ -14,6 +14,7 @@ ```bash ./tests/cf-acl/run.sh ./tests/cf-fileop/run.sh +python3 tests/cf-lock/run.py ./tests/cf-permission-many/run.sh ./tests/cf-dir-share/run.sh # 目录分页另需 valac / libjansson 开发包 diff --git a/common/cf-lock.c b/common/cf-lock.c index 24db4932..1a235768 100644 --- a/common/cf-lock.c +++ b/common/cf-lock.c @@ -66,23 +66,19 @@ like_descendant_pattern (const char *path) return g_string_free (pattern, FALSE); } -/* Lock and resource share the Hub migration's UID and seafile-db. The SQL - * retains manual locks and suspended checkout reservations. - * A failed query refuses the write rather than weakening the barrier. */ +/* V2 reservations are path-keyed. Unscoped writes, including the owner's, + * must not consume an exclusive Checkout without a verified publish proof. */ static int load_exact (const char *repo_id, const char *path, const char *user) { + (void)user; char *hash = g_compute_checksum_for_string (G_CHECKSUM_SHA256, path, -1); - int n = 0; - if (n == 0) - n = seaf_db_statement_foreach_row ( - cf_ext_database (), - "SELECT r.uid FROM cf_resource r JOIN cf_edit_guard g ON g.resource_uid=r.uid " - "WHERE r.repo_id=? AND r.path_hash=? AND r.path=? AND r.kind='file' AND r.state='active' " - "AND g.repo_id=r.repo_id AND g.guard_id IS NOT NULL " - "AND (g.lifecycle_ref<>r.lifecycle_ref OR g.mode IS NULL OR g.mode<>'file-lock' OR g.owner_native_user IS NULL OR g.owner_native_user<>?) LIMIT 1", - barrier_row, NULL, 4, "string", repo_id, "string", hash, - "string", path, "string", user ? user : ""); + int n = seaf_db_statement_foreach_row ( + cf_ext_database (), + "SELECT path FROM cf_edit_guard FORCE INDEX (cf_edit_guard_active) " + "WHERE repo_id=? AND path_hash=UNHEX(?) AND path=? " + "AND guard_id IS NOT NULL AND lease_expires_at>UTC_TIMESTAMP(6) LIMIT 1", + barrier_row, NULL, 3, "string", repo_id, "string", hash, "string", path); g_free (hash); return n; @@ -92,14 +88,12 @@ static int load_descendant (const char *repo_id, const char *path) { char *pattern = like_descendant_pattern (path); - int n = 0; - if (n == 0) - n = seaf_db_statement_foreach_row ( - cf_ext_database (), - "SELECT r.uid FROM cf_resource r JOIN cf_edit_guard g ON g.resource_uid=r.uid " - "WHERE r.repo_id=? AND r.path LIKE ? ESCAPE '\\\\' AND r.state='active' " - "AND g.repo_id=r.repo_id AND g.guard_id IS NOT NULL LIMIT 1", - barrier_row, NULL, 2, "string", repo_id, "string", pattern); + int n = seaf_db_statement_foreach_row ( + cf_ext_database (), + "SELECT path FROM cf_edit_guard FORCE INDEX (cf_edit_guard_active) " + "WHERE repo_id=? AND path LIKE ? ESCAPE '\\\\' " + "AND guard_id IS NOT NULL AND lease_expires_at>UTC_TIMESTAMP(6) LIMIT 1", + barrier_row, NULL, 2, "string", repo_id, "string", pattern); g_free (pattern); return n; @@ -112,12 +106,6 @@ check_path (const char *repo_id, const char *path, const char *user, int n; if (!cf_ext_database ()) goto unavailable; - int ready = seaf_db_statement_foreach_row ( - cf_ext_database (), - "SELECT version FROM cf_schema_migration WHERE version='029_editing_core' " - "AND state='applied' AND step=2", barrier_row, NULL, 0); - if (ready != 1) - goto unavailable; n = load_exact (repo_id, path, user); if (n < 0) goto unavailable; @@ -176,13 +164,15 @@ void cf_lock_init (void) { char *backend = cf_ext_config_string ("lock_backend"); - lock_on = cf_ext_config_bool ("file_lock_enabled") && + /* Checkout is unfinished and opt-in. A legacy file-lock switch alone + * must not make normal uploads depend on Guard or the retired ledger. */ + lock_on = cf_ext_config_bool ("checkout_enabled") && (!backend || strcmp (backend, "cloudfile") == 0); if (backend && strcmp (backend, "cloudfile") != 0) seaf_warning ("CloudFile: unsupported lock backend '%s'.\n", backend); g_free (backend); if (lock_on) { cf_fileop_register ("file-lock", cf_lock_prepare, NULL, NULL); - seaf_message ("CloudFile: UID lease write barrier enabled.\n"); + seaf_message ("CloudFile: V2 Checkout write barrier enabled.\n"); } } diff --git a/common/cf-lock.h b/common/cf-lock.h index 009094e7..8ae77467 100644 --- a/common/cf-lock.h +++ b/common/cf-lock.h @@ -1,13 +1,13 @@ /* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* CloudFile's Hub-owned UID lease write barrier. */ +/* CloudFile's path-keyed V2 Checkout write barrier. */ #ifndef CF_LOCK_H #define CF_LOCK_H #include -/* Registers the write-lifecycle provider when [cloudfile] file_lock_enabled +/* Registers the write-lifecycle provider when [cloudfile] checkout_enabled * is true and lock_backend is either unset or "cloudfile". */ void cf_lock_init (void); diff --git a/fileserver/cf_upload_guard_test.go b/fileserver/cf_upload_guard_test.go new file mode 100644 index 00000000..697ac284 --- /dev/null +++ b/fileserver/cf_upload_guard_test.go @@ -0,0 +1,171 @@ +package main + +import ( + "context" + "database/sql" + "database/sql/driver" + "errors" + "io" + "strings" + "testing" + + "github.com/haiwen/seafile-server/fileserver/option" +) + +// Exercise production SQL selection and row validation through database/sql. +// This fixture does not certify MySQL lock behavior or a deployed upload. +type uploadGuardFixture struct { + columns []string + paths []driver.Value + failMetadata, failGuard bool + tableCount int + queries []string +} +type uploadGuardDriver struct{} + +func (uploadGuardDriver) Open(string) (driver.Conn, error) { return nil, errors.New("use connector") } + +type uploadGuardConnector struct{ fixture *uploadGuardFixture } + +func (c uploadGuardConnector) Driver() driver.Driver { return uploadGuardDriver{} } +func (c uploadGuardConnector) Connect(context.Context) (driver.Conn, error) { + return &uploadGuardConn{c.fixture}, nil +} + +type uploadGuardConn struct{ fixture *uploadGuardFixture } + +func (c *uploadGuardConn) Prepare(string) (driver.Stmt, error) { return nil, errors.New("no prepare") } +func (c *uploadGuardConn) Close() error { return nil } +func (c *uploadGuardConn) Begin() (driver.Tx, error) { return uploadGuardTx{}, nil } +func (c *uploadGuardConn) QueryContext(_ context.Context, query string, args []driver.NamedValue) (driver.Rows, error) { + c.fixture.queries = append(c.fixture.queries, query) + rows := &uploadGuardRows{} + if strings.Contains(query, "information_schema.TABLES") { + if c.fixture.failMetadata { + return nil, errors.New("database unavailable") + } + rows.values = append(rows.values, []driver.Value{int64(c.fixture.tableCount)}) + } else if strings.Contains(query, "information_schema.COLUMNS") { + if c.fixture.failMetadata { + return nil, errors.New("database unavailable") + } + for _, name := range c.fixture.columns { + rows.values = append(rows.values, []driver.Value{name}) + } + } else { + if c.fixture.failGuard { + return nil, errors.New("guard unavailable") + } + if len(args) != 1 || args[0].Value != "repo" { + return nil, errors.New("wrong repository scope") + } + for _, path := range c.fixture.paths { + rows.values = append(rows.values, []driver.Value{path}) + } + } + return rows, nil +} + +type uploadGuardTx struct{} + +func (uploadGuardTx) Commit() error { return nil } +func (uploadGuardTx) Rollback() error { return nil } + +type uploadGuardRows struct{ values [][]driver.Value } + +func (r *uploadGuardRows) Columns() []string { return []string{"value"} } +func (r *uploadGuardRows) Close() error { return nil } +func (r *uploadGuardRows) Next(dest []driver.Value) error { + if len(r.values) == 0 { + return io.EOF + } + copy(dest, r.values[0]) + r.values = r.values[1:] + return nil +} + +func TestUploadGuardUsesReviewedPathQueryOnly(t *testing.T) { + old, oldEnabled := option.V2GuardAvailable, option.CloudFileCheckoutEnabled + option.V2GuardAvailable, option.CloudFileCheckoutEnabled = true, true + defer func() { option.V2GuardAvailable, option.CloudFileCheckoutEnabled = old, oldEnabled }() + f := &uploadGuardFixture{} + db := sql.OpenDB(uploadGuardConnector{f}) + defer db.Close() + tx, err := db.Begin() + if err != nil { + t.Fatal(err) + } + defer tx.Rollback() + if err := cfV2EditCheckUnscopedBranch(context.Background(), tx, "repo", "old", "new"); err != nil { + t.Fatal(err) + } + // Startup already qualified the schema: the commit path issues exactly one + // Guard query, with no information_schema read and no cf_resource join. + if len(f.queries) != 1 { + t.Fatalf("expected one guard query, got %v", f.queries) + } + query := f.queries[0] + if strings.Contains(query, "information_schema") || strings.Contains(query, "cf_resource") { + t.Fatalf("legacy or repeated metadata query on commit path: %s", query) + } + if !strings.Contains(query, "FORCE INDEX (cf_edit_guard_active)") || !strings.Contains(query, "lease_expires_at>") { + t.Fatalf("guard query lost its reviewed shape: %s", query) + } +} + +func TestUploadGuardRefusesGuardFailures(t *testing.T) { + old, oldEnabled := option.V2GuardAvailable, option.CloudFileCheckoutEnabled + option.V2GuardAvailable, option.CloudFileCheckoutEnabled = true, true + defer func() { option.V2GuardAvailable, option.CloudFileCheckoutEnabled = old, oldEnabled }() + for _, f := range []*uploadGuardFixture{ + {failGuard: true}, + {paths: []driver.Value{nil}}, + {paths: []driver.Value{"relative/path"}}, + } { + db := sql.OpenDB(uploadGuardConnector{f}) + tx, err := db.Begin() + if err != nil { + t.Fatal(err) + } + if !errors.Is(cfV2EditCheckUnscopedBranch(context.Background(), tx, "repo", "old", "new"), errCFEditGuard) { + t.Fatalf("unavailable or malformed guard was bypassed: %+v", f) + } + tx.Rollback() + db.Close() + } +} + +// Nil DB/transaction proves the disabled path performs no metadata or Guard SQL. +func TestUploadCheckoutDisabledNeverQueriesGuard(t *testing.T) { + old, enabled := option.V2GuardAvailable, option.CloudFileCheckoutEnabled + defer func() { option.V2GuardAvailable, option.CloudFileCheckoutEnabled = old, enabled }() + option.CloudFileCheckoutEnabled, option.V2GuardAvailable = false, true + if available, err := loadV2GuardCapability(context.Background(), nil); err != nil || available { + t.Fatalf("disabled capability touched the database: %v", err) + } + if err := cfV2EditCheckUnscopedBranch(context.Background(), nil, "repo", "old", "new"); err != nil { + t.Fatalf("disabled upload queried Guard: %v", err) + } + option.CloudFileCheckoutEnabled, option.V2GuardAvailable = true, false + if !errors.Is(cfV2EditCheckUnscopedBranch(context.Background(), nil, "repo", "old", "new"), errCFEditGuard) { + t.Fatal("enabled but unavailable Guard was bypassed") + } +} + +func TestUploadEnabledCheckoutRequiresInstalledGuard(t *testing.T) { + enabled := option.CloudFileCheckoutEnabled + option.CloudFileCheckoutEnabled = true + defer func() { option.CloudFileCheckoutEnabled = enabled }() + for _, f := range []*uploadGuardFixture{{tableCount: 1}, {tableCount: 0}, {failMetadata: true}} { + db := sql.OpenDB(uploadGuardConnector{f}) + available, err := loadV2GuardCapability(context.Background(), db) + if f.tableCount == 1 && !f.failMetadata { + if err != nil || !available { + t.Fatal("installed Guard not enabled") + } + } else if err == nil || available { + t.Fatal("enabled Checkout accepted unavailable Guard") + } + db.Close() + } +} diff --git a/fileserver/cf_upload_retry_test.go b/fileserver/cf_upload_retry_test.go new file mode 100644 index 00000000..2e58592e --- /dev/null +++ b/fileserver/cf_upload_retry_test.go @@ -0,0 +1,31 @@ +package main + +// Regression guard for the retry contract: only a moved Branch HEAD may be +// retried. Without this test a refactor could silently turn a schema/Guard +// fault into ten upload attempts reported as "concurrent upload". + +import ( + "errors" + "fmt" + "testing" +) + +func TestCFUploadRetriesOnlyHeadConflict(t *testing.T) { + if !cfBranchConflictRetryable(ErrConflict) { + t.Fatal("direct HEAD conflict must remain retryable") + } + if !cfBranchConflictRetryable(fmt.Errorf("wrapped: %w", ErrConflict)) { + t.Fatal("wrapped HEAD conflict must remain retryable") + } + for _, err := range []error{ + errCFEditGuard, + errCFACL, + errors.New("Error 1176 (42000): Key 'cf_edit_guard_active' doesn't exist in table"), + errors.New("SQL unavailable"), + nil, + } { + if cfBranchConflictRetryable(err) { + t.Fatalf("deterministic failure must never retry: %v", err) + } + } +} diff --git a/fileserver/cf_v2_edit_agent.go b/fileserver/cf_v2_edit_agent.go index 6440aa7e..26ff73cf 100644 --- a/fileserver/cf_v2_edit_agent.go +++ b/fileserver/cf_v2_edit_agent.go @@ -408,9 +408,9 @@ func cfEditStatus(w http.ResponseWriter, r *http.Request, c *cfEditClaims, repo } } out["sha256"] = hex.EncodeToString(hasher.Sum(nil)) - if option.V2GuardAvailable { - // Checkout is schema-derived; ordinary optimistic editing is always available. - // A V2 deployment without the Guard table never queries it. + if option.CloudFileCheckoutEnabled && option.V2GuardAvailable { + // Checkout is explicitly enabled and its Guard is installed. + // Disabled Checkout leaves ordinary optimistic editing independent of Guard SQL. var gID, owner sql.NullString var generation uint64 var expires sql.NullTime @@ -506,7 +506,7 @@ func cfEditPublish(w http.ResponseWriter, r *http.Request, c *cfEditClaims, repo if action == "optimistic" { // No persistent lock or Intent is allocated for an ordinary save. // The Guard query is needed only on deployments with an installed Guard. - if option.V2GuardAvailable { + if option.CloudFileCheckoutEnabled && option.V2GuardAvailable { var held int err = tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM cf_edit_guard WHERE repo_id=? AND path_hash=? AND guard_id IS NOT NULL AND lease_expires_at>UTC_TIMESTAMP(6) FOR UPDATE", repo.ID, cfEditHash(c.FilePath)).Scan(&held) if err != nil || held != 0 { @@ -603,7 +603,7 @@ func cfEditAPI(w http.ResponseWriter, r *http.Request) { cfEditFail(w, 403, "READ_ONLY_TICKET") return } - if !option.V2GuardAvailable && !cfEditAllowedWithoutCheckout(c.Mode, route, r.Header.Get("X-CF-Action")) { + if (!option.CloudFileCheckoutEnabled || !option.V2GuardAvailable) && !cfEditAllowedWithoutCheckout(c.Mode, route, r.Header.Get("X-CF-Action")) { cfEditFail(w, 409, "CHECKOUT_UNSUPPORTED") return } diff --git a/fileserver/cf_v2_edit_agent_test.go b/fileserver/cf_v2_edit_agent_test.go index 6cd1e57f..5d2a48c6 100644 --- a/fileserver/cf_v2_edit_agent_test.go +++ b/fileserver/cf_v2_edit_agent_test.go @@ -112,10 +112,14 @@ func TestCFEditLapsedGuardResumesOnlyOnUnchangedFile(t *testing.T) { } func TestCFEditExclusiveUnavailableWithoutCheckoutSchema(t *testing.T) { - oldFlag, oldSecret := option.V2GuardAvailable, option.JWTPrivateKey + oldFlag, oldSecret, oldEnabled := option.V2GuardAvailable, option.JWTPrivateKey, option.CloudFileCheckoutEnabled option.V2GuardAvailable = false option.JWTPrivateKey = "unit-test-secret" - defer func() { option.V2GuardAvailable = oldFlag; option.JWTPrivateKey = oldSecret }() + defer func() { + option.V2GuardAvailable = oldFlag + option.JWTPrivateKey = oldSecret + option.CloudFileCheckoutEnabled = oldEnabled + }() now := time.Now() ticket := cfEditClaims{ UID: "uid-test", @@ -135,13 +139,18 @@ func TestCFEditExclusiveUnavailableWithoutCheckoutSchema(t *testing.T) { if err != nil { t.Fatal(err) } - req := httptest.NewRequest("POST", "/cf-edit/v1/checkout", nil) - req.Header.Set("Authorization", "Bearer "+signed) - rec := httptest.NewRecorder() - cfEditAPI(rec, req) - if rec.Code != http.StatusConflict || !strings.Contains(rec.Body.String(), "CHECKOUT_UNSUPPORTED") { - t.Fatalf("unexpected disabled checkout response: %d %s", rec.Code, rec.Body.String()) + // A table alone cannot enable Checkout or allow its API to create a lock. + for _, flags := range []struct{ enabled, available bool }{{false, true}, {true, false}} { + option.CloudFileCheckoutEnabled, option.V2GuardAvailable = flags.enabled, flags.available + req := httptest.NewRequest("POST", "/cf-edit/v1/checkout", nil) + req.Header.Set("Authorization", "Bearer "+signed) + rec := httptest.NewRecorder() + cfEditAPI(rec, req) + if rec.Code != http.StatusConflict || !strings.Contains(rec.Body.String(), "CHECKOUT_UNSUPPORTED") { + t.Fatalf("unexpected disabled checkout response: %d %s", rec.Code, rec.Body.String()) + } } + } func TestCFEditOrdinaryRequiresNoCheckoutTable(t *testing.T) { diff --git a/fileserver/cf_v2_edit_fence.go b/fileserver/cf_v2_edit_fence.go index d0d5bb24..cf7d2a9d 100644 --- a/fileserver/cf_v2_edit_fence.go +++ b/fileserver/cf_v2_edit_fence.go @@ -34,10 +34,19 @@ func cfV2EditTouchesPath(entry *diff.DiffEntry, guardPath string) bool { // Caller must already hold Branch(master) FOR UPDATE in this same transaction. // SQL errors and native diff failures deny, never silently fall back to CE. func cfV2EditCheckUnscopedBranch(ctx context.Context, tx *sql.Tx, repoID, oldCommitID, newCommitID string) error { - if !option.V2GuardAvailable { + // Ordinary upload must not depend on Guard schema or SQL when Checkout + // is disabled. Enabled-but-unavailable is an error, never a bypass. + if !option.CloudFileCheckoutEnabled { return nil } - rows, err := tx.QueryContext(ctx, "SELECT path FROM cf_edit_guard FORCE INDEX (cf_edit_guard_active) WHERE repo_id=? AND guard_id IS NOT NULL AND lease_expires_at>UTC_TIMESTAMP(6) FOR UPDATE", repoID) + if !option.V2GuardAvailable { + return errCFEditGuard + } + // The startup gate qualified the installed 15-column/4-index path store + // when Checkout was enabled, so the final transaction uses the reviewed + // query directly instead of re-reading information_schema on every commit. + // Runtime drift still denies: a failed query returns errCFEditGuard. + rows, err := tx.QueryContext(ctx, cfEditGuardFenceSQL, repoID) if err != nil { return errCFEditGuard } @@ -87,3 +96,9 @@ func cfV2EditCheckUnscopedBranch(ctx context.Context, tx *sql.Tx, repoID, oldCom } return nil } + +// Reviewed V2 fence query. Only this path-keyed store is supported; the +// resource-keyed 029 layout is archived by the reviewed SQL package and must +// not remain reachable as a runtime fallback. +const cfEditGuardFenceSQL = "SELECT path FROM cf_edit_guard FORCE INDEX (cf_edit_guard_active) " + + "WHERE repo_id=? AND guard_id IS NOT NULL AND lease_expires_at>UTC_TIMESTAMP(6) FOR UPDATE" diff --git a/fileserver/cf_v2_edit_schema.go b/fileserver/cf_v2_edit_schema.go new file mode 100644 index 00000000..5ec9ee23 --- /dev/null +++ b/fileserver/cf_v2_edit_schema.go @@ -0,0 +1,142 @@ +package main + +// Modification: validate the *installed* cf_edit_guard column/index contract at +// fileserver startup whenever Checkout is explicitly enabled. +// Reason: the opt-in model only probed table existence, so a legacy +// resource-keyed table or a missing cf_edit_guard_active index surfaced as an +// opaque SQL 1176 failure on every upload. The schema-strict variant showed an +// explicit CF_EDIT_SCHEMA_MISMATCH is actionable for operators; this file folds +// that evidence into the opt-in model without making disabled Checkout depend +// on any Guard SQL. +import ( + "context" + "database/sql" + "fmt" + "strings" +) + +type cfGuardColumn struct { + kind string + length int64 + nullable string + unsigned bool +} + +type cfGuardIndexPart struct { + name string + nonUnique int64 + prefix bool +} + +// Reviewed V2 Guard shape. Keep in sync with +// database/v2/20261021-four-table-baseline/00-create.sql; a table name alone is +// not a schema contract. +var cfV2GuardColumns = map[string]cfGuardColumn{ + "repo_id": {"char", 36, "NO", false}, + "path_hash": {"binary", 32, "NO", false}, + "path": {"varchar", 4096, "NO", false}, + "guard_id": {"char", 36, "YES", false}, + "generation": {"bigint", 0, "NO", true}, + "owner_uid": {"varchar", 225, "YES", false}, + "owner_native_user": {"varchar", 255, "YES", false}, + "terminal_type": {"varchar", 16, "YES", false}, + "terminal_name": {"varchar", 128, "YES", false}, + "checkout_ip": {"varchar", 45, "YES", false}, + "proof_digest": {"char", 64, "YES", false}, + "base_file_id": {"char", 40, "YES", false}, + "lease_expires_at": {"datetime", 0, "YES", false}, + "created_at": {"datetime", 0, "NO", false}, + "updated_at": {"datetime", 0, "NO", false}, +} + +var cfV2GuardIndexes = map[string][]cfGuardIndexPart{ + "PRIMARY": {{"repo_id", 0, false}, {"path_hash", 0, false}}, + "cf_edit_guard_id": {{"guard_id", 0, false}}, + "cf_edit_guard_active": {{"repo_id", 1, false}, {"guard_id", 1, false}}, + "cf_edit_guard_owner": {{"owner_uid", 1, false}, {"guard_id", 1, false}}, +} + +// cfV2GuardValidate compares a metadata snapshot with the reviewed contract. +// It performs no data reads and issues no DDL/DML. +func cfV2GuardValidate(columns map[string]cfGuardColumn, indexes map[string][]cfGuardIndexPart) error { + if len(columns) != len(cfV2GuardColumns) { + return fmt.Errorf("expected %d V2 Guard columns, found %d", len(cfV2GuardColumns), len(columns)) + } + for name, want := range cfV2GuardColumns { + got, exists := columns[name] + if !exists || got != want { + return fmt.Errorf("Guard column %s is missing or incompatible", name) + } + } + for name, want := range cfV2GuardIndexes { + got := indexes[name] + if len(got) != len(want) { + return fmt.Errorf("Guard index %s is missing or incompatible", name) + } + for i := range want { + if got[i] != want[i] { + return fmt.Errorf("Guard index %s has incompatible key order or uniqueness", name) + } + } + } + return nil +} + +// cfV2GuardRequireCurrent reads only information_schema metadata and returns an +// error naming the first incompatible column or index. Callers surface it as a +// startup failure so a legacy table is archived before Checkout is enabled +// instead of being retried per upload. +func cfV2GuardRequireCurrent(ctx context.Context, db *sql.DB) error { + rows, err := db.QueryContext(ctx, + "SELECT COLUMN_NAME,DATA_TYPE,CHARACTER_MAXIMUM_LENGTH,IS_NULLABLE,COLUMN_TYPE "+ + "FROM information_schema.COLUMNS WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='cf_edit_guard'") + if err != nil { + return fmt.Errorf("cannot inspect Guard columns: %w", err) + } + columns := make(map[string]cfGuardColumn) + for rows.Next() { + var name, kind, nullable, fullKind string + var length sql.NullInt64 + if err := rows.Scan(&name, &kind, &length, &nullable, &fullKind); err != nil { + rows.Close() + return fmt.Errorf("cannot read Guard column definition: %w", err) + } + columns[name] = cfGuardColumn{kind, length.Int64, nullable, + strings.Contains(strings.ToLower(fullKind), "unsigned")} + } + err = rows.Err() + rows.Close() + if err != nil { + return fmt.Errorf("cannot finish Guard column inspection: %w", err) + } + + rows, err = db.QueryContext(ctx, + "SELECT INDEX_NAME,SEQ_IN_INDEX,COLUMN_NAME,NON_UNIQUE,SUB_PART "+ + "FROM information_schema.STATISTICS "+ + "WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='cf_edit_guard' "+ + "ORDER BY INDEX_NAME,SEQ_IN_INDEX") + if err != nil { + return fmt.Errorf("cannot inspect Guard indexes: %w", err) + } + indexes := make(map[string][]cfGuardIndexPart) + for rows.Next() { + var key, name string + var order, nonUnique int64 + var prefix sql.NullInt64 + if err := rows.Scan(&key, &order, &name, &nonUnique, &prefix); err != nil { + rows.Close() + return fmt.Errorf("cannot read Guard index definition: %w", err) + } + if order != int64(len(indexes[key])+1) { + rows.Close() + return fmt.Errorf("Guard index %s key order is invalid", key) + } + indexes[key] = append(indexes[key], cfGuardIndexPart{name, nonUnique, prefix.Valid}) + } + err = rows.Err() + rows.Close() + if err != nil { + return fmt.Errorf("cannot finish Guard index inspection: %w", err) + } + return cfV2GuardValidate(columns, indexes) +} diff --git a/fileserver/cf_v2_edit_schema_test.go b/fileserver/cf_v2_edit_schema_test.go new file mode 100644 index 00000000..1a4dedc3 --- /dev/null +++ b/fileserver/cf_v2_edit_schema_test.go @@ -0,0 +1,63 @@ +package main + +// Contract tests for the startup Guard validation added on top of the opt-in +// Checkout model. They exercise metadata-only validation, so no database is +// required; the failure cases mirror the legacy 029 layout and index drift +// that previously surfaced as SQL 1176 retries. + +import ( + "testing" +) + +func guardLayout() (map[string]cfGuardColumn, map[string][]cfGuardIndexPart) { + columns := make(map[string]cfGuardColumn) + for name, column := range cfV2GuardColumns { + columns[name] = column + } + indexes := make(map[string][]cfGuardIndexPart) + for name, parts := range cfV2GuardIndexes { + indexes[name] = append([]cfGuardIndexPart(nil), parts...) + } + return columns, indexes +} + +func TestCFV2GuardSchemaContract(t *testing.T) { + columns, indexes := guardLayout() + if err := cfV2GuardValidate(columns, indexes); err != nil { + t.Fatalf("current V2 Guard rejected: %v", err) + } + for _, tc := range []struct { + name string + edit func(map[string]cfGuardColumn, map[string][]cfGuardIndexPart) + }{ + {"old lifecycle_ref instead of path", func(c map[string]cfGuardColumn, _ map[string][]cfGuardIndexPart) { + delete(c, "path") + c["lifecycle_ref"] = cfGuardColumn{kind: "varchar", length: 512, nullable: "NO"} + }}, + {"old lease_until instead of lease_expires_at", func(c map[string]cfGuardColumn, _ map[string][]cfGuardIndexPart) { + delete(c, "lease_expires_at") + c["lease_until"] = cfGuardColumn{kind: "datetime", nullable: "YES"} + }}, + {"path hash stored as text", func(c map[string]cfGuardColumn, _ map[string][]cfGuardIndexPart) { + c["path_hash"] = cfGuardColumn{kind: "char", length: 64, nullable: "NO"} + }}, + {"missing active index", func(_ map[string]cfGuardColumn, idx map[string][]cfGuardIndexPart) { + delete(idx, "cf_edit_guard_active") + }}, + {"reversed active index", func(_ map[string]cfGuardColumn, idx map[string][]cfGuardIndexPart) { + idx["cf_edit_guard_active"][0], idx["cf_edit_guard_active"][1] = + idx["cf_edit_guard_active"][1], idx["cf_edit_guard_active"][0] + }}, + {"old resource key", func(_ map[string]cfGuardColumn, idx map[string][]cfGuardIndexPart) { + idx["PRIMARY"] = []cfGuardIndexPart{{name: "resource_uid", nonUnique: 0}} + }}, + } { + t.Run(tc.name, func(t *testing.T) { + c, i := guardLayout() + tc.edit(c, i) + if err := cfV2GuardValidate(c, i); err == nil { + t.Fatal("outdated or incompatible Guard layout passed") + } + }) + } +} diff --git a/fileserver/fileop.go b/fileserver/fileop.go index 6b0cfae6..106a390f 100644 --- a/fileserver/fileop.go +++ b/fileserver/fileop.go @@ -1950,6 +1950,10 @@ func postMultiFiles(rsp http.ResponseWriter, r *http.Request, repoID, parentDir, }) if errors.Is(err, ErrGCConflict) { return &appError{nil, "GC Conflict.\n", http.StatusConflict} + } else if errors.Is(err, errCFEditGuard) { + // An installed Guard cannot be queried or rejects the publication; + // report that failure directly instead of a fabricated retry timeout. + return &appError{nil, "The server cannot verify the Checkout guard.", http.StatusServiceUnavailable} } else { err := fmt.Errorf("failed to post files and gen commit: %v", err) return &appError{err, "", http.StatusInternalServerError} @@ -2208,6 +2212,15 @@ func fastForwardOrMerge(user, token string, repo *repomgr.Repo, base, newCommit return nil } +// cfBranchConflictRetryable names the only transient commit failure: another +// writer moved the Branch HEAD. Guard, schema, ACL and SQL failures are +// deterministic until the deployment is repaired, so retrying them only turns +// one visible fault into ten hidden ones. Kept as a helper so the contract is +// testable without a database. +func cfBranchConflictRetryable(err error) bool { + return errors.Is(err, ErrConflict) +} + func genCommitNeedRetry(repo *repomgr.Repo, base *commitmgr.Commit, commit *commitmgr.Commit, newRoot, user string, handleConncurrentUpdate bool, commitID *string, lastGCID string, checkGC bool) (bool, error) { var secondParentID string repoID := repo.ID @@ -2265,11 +2278,13 @@ func genCommitNeedRetry(repo *repomgr.Repo, base *commitmgr.Commit, commit *comm if gcConflict { return false, err } - if err == errCFACL { - return false, err + // Only a changed Branch head is a concurrent-publication retry. Guard, + // schema and authority failures are deterministic, not ten hidden retries. + if cfBranchConflictRetryable(err) { + return true, nil } if err != nil { - return true, nil + return false, err } if commitID != nil { @@ -2341,8 +2356,7 @@ func updateBranch(repoID, originRepoID, newCommitID, oldCommitID, secondParentID } if oldCommitID != commitID { trans.Rollback() - err := fmt.Errorf("head commit id has changed") - return false, err + return false, ErrConflict } // Final and transactional: an upload preflight cannot protect against a diff --git a/fileserver/fileserver.go b/fileserver/fileserver.go index 88496303..907c51e8 100644 --- a/fileserver/fileserver.go +++ b/fileserver/fileserver.go @@ -162,18 +162,40 @@ func loadSeafileDB() { seafileDB.SetMaxIdleConns(8) } -// A controlled Guard schema migration requires restarting native services. -// An unhealthy database is fatal: it must never be interpreted as no Checkout. +// A capability change requires restarting Hub and native services. Disabled +// Checkout never queries a Guard table, even when the baseline DDL includes it. +func loadV2GuardCapability(ctx context.Context, db *sql.DB) (bool, error) { + if !option.CloudFileCheckoutEnabled { + return false, nil + } + var count int + if err := db.QueryRowContext(ctx, + "SELECT COUNT(*) FROM information_schema.TABLES WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='cf_edit_guard'").Scan(&count); err != nil { + return false, fmt.Errorf("Checkout Guard discovery failed") + } + if count != 1 { + return false, fmt.Errorf("enabled Checkout requires an installed Guard table") + } + // Modification: validate the installed column/index contract before serving. + // Reason: a present table is not a usable schema. A legacy resource-keyed + // table or a missing cf_edit_guard_active index previously failed every + // upload with SQL 1176; fail once at startup with an operator instruction + // instead, without making disabled Checkout read Guard metadata at all. + if err := cfV2GuardRequireCurrent(ctx, db); err != nil { + return false, fmt.Errorf("CF_EDIT_SCHEMA_MISMATCH: installed cf_edit_guard is not the reviewed V2 layout (%v); "+ + "archive the legacy rows and apply the reviewed database/v2 SQL package before enabling Checkout", err) + } + return true, nil +} + func detectV2Guard() { ctx, cancel := context.WithTimeout(context.Background(), option.DBOpTimeout) defer cancel() - var count int - err := seafileDB.QueryRowContext(ctx, - "SELECT COUNT(*) FROM information_schema.TABLES WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='cf_edit_guard'").Scan(&count) - if err != nil || count < 0 || count > 1 { - log.Fatalf("Failed to determine installed CloudFile Checkout Guard: %v (count=%d)", err, count) + available, err := loadV2GuardCapability(ctx, seafileDB) + if err != nil { + log.Fatal(err) } - option.V2GuardAvailable = count == 1 + option.V2GuardAvailable = available } func writePidFile(pid_file_path string) error { diff --git a/fileserver/option/checkout_test.go b/fileserver/option/checkout_test.go new file mode 100644 index 00000000..a6ff6c27 --- /dev/null +++ b/fileserver/option/checkout_test.go @@ -0,0 +1,30 @@ +package option + +import ( + "os" + "path/filepath" + "testing" +) + +// The existing Docker setting must be read explicitly; a table cannot enable it. +func TestCheckoutRequiresExplicitConfiguration(t *testing.T) { + old := CloudFileCheckoutEnabled + defer func() { CloudFileCheckoutEnabled = old }() + for _, tc := range []struct { + value string + want bool + }{{"", false}, {"false", false}, {"true", true}} { + dir := t.TempDir() + config := "[fileserver]\nport = 8082\n[cloudfile]\n" + if tc.value != "" { + config += "checkout_enabled = " + tc.value + "\n" + } + if err := os.WriteFile(filepath.Join(dir, "seafile.conf"), []byte(config), 0600); err != nil { + t.Fatal(err) + } + LoadFileServerOptions(dir) + if CloudFileCheckoutEnabled != tc.want { + t.Fatalf("setting %q: got %v", tc.value, CloudFileCheckoutEnabled) + } + } +} diff --git a/fileserver/option/option.go b/fileserver/option/option.go index d8dcdc38..aa213e70 100644 --- a/fileserver/option/option.go +++ b/fileserver/option/option.go @@ -24,8 +24,11 @@ var CloudFileManagedLibraryGuard bool var CloudFileAllowLegacyManagedReads bool var CloudFileTrustedTLSProxies []*net.IPNet -// Startup schema detection sets this once before serving requests. Keeping the -// default false preserves ordinary edits when Checkout Guard is not installed. +// Checkout is an explicit existing capability, not implied by a table name. +// Both values are initialized before serving requests; disabled means no Guard SQL. +var CloudFileCheckoutEnabled bool + +// True only after enabled Checkout has a usable installed Guard table. var V2GuardAvailable bool // Storage unit. @@ -151,7 +154,17 @@ func LoadFileServerOptions(centralDir string) { CloudFileTrustedTLSProxies = nil CloudFileManagedLibraryGuard = false CloudFileAllowLegacyManagedReads = false + CloudFileCheckoutEnabled = false if section, sectionErr := config.GetSection("cloudfile"); sectionErr == nil { + // Reuse CF_ENABLE_CHECKOUT -> [cloudfile] checkout_enabled from Docker. + // Invalid values must not silently disable an explicitly requested fence. + if key, keyErr := section.GetKey("checkout_enabled"); keyErr == nil { + var parseErr error + CloudFileCheckoutEnabled, parseErr = key.Bool() + if parseErr != nil { + log.Fatal("Invalid CloudFile checkout_enabled") + } + } if key, keyErr := section.GetKey("allow_legacy_managed_reads"); keyErr == nil { var parseErr error CloudFileAllowLegacyManagedReads, parseErr = key.Bool() diff --git a/server/cloudfile-policy.c b/server/cloudfile-policy.c index 5ee96780..086aaf74 100644 --- a/server/cloudfile-policy.c +++ b/server/cloudfile-policy.c @@ -218,46 +218,27 @@ lease_resource_location (SeafDBTrans *trans, const char *repo, return count >= 0 && count <= 1 && location.valid && (!uid || count == 1) ? 0 : -1; } -/* Only the opt-in lock barrier requires the new checkout schema. The final - * Branch transaction must re-read it after PREPARE, closing that race for - * scoped file publication. A missing or malformed table fails closed. */ -static int -editing_conflict (SeafDBTrans *trans, const char *repo, const char *path, - const char *hash, const char *user) -{ - gboolean error = FALSE; - gboolean guarded = seaf_db_trans_check_for_existence (trans, - "SELECT r.uid FROM cf_resource r JOIN cf_edit_guard g ON g.resource_uid=r.uid " - "WHERE r.repo_id=? AND r.path_hash=? AND r.path=? AND r.state='active' " - "AND g.repo_id=r.repo_id AND g.guard_id IS NOT NULL " - "AND (g.lifecycle_ref<>r.lifecycle_ref OR g.mode IS NULL OR g.mode<>'file-lock' OR g.owner_native_user IS NULL OR g.owner_native_user<>?) FOR UPDATE", - &error, 4, "string", repo, "string", hash, "string", path, - "string", user ? user : ""); - return guarded || error ? -1 : 0; -} - int cf_policy_check_unleased_write (SeafDBTrans *trans, const char *repo, const char *path, const char *user) { if (!cf_lock_enabled ()) return 0; + (void)user; if (!repo || !path || path[0] != '/' || strlen (path) > 4096 || - !g_utf8_validate (path, -1, NULL) || editing_schema (trans) < 0) return -1; + !g_utf8_validate (path, -1, NULL)) return -1; char *digest = g_compute_checksum_for_string (G_CHECKSUM_SHA256, path, -1); if (!digest) return -1; - if (lease_resource_location (trans, repo, path, digest, NULL) < 0) { - g_free (digest); - return -1; - } - /* Current locking read, not a preflight/RR snapshot. The actual sparse UID - * and lease rows stay locked until the caller's Branch transaction ends. - * SQL errors fail closed; orphan/lifecycle reconciliation remains a - * separate native integration prerequisite, not proven by this join. - * Exact stored path is compared after hashing to avoid digest-only identity. - */ - int result = editing_conflict (trans, repo, path, digest, user); + /* V2 publishes against one path-keyed Guard, in the Branch transaction. + * The retired Resource/Intent ledger must not be a file-write dependency. + * Unscoped owner writes also refuse an active exclusive reservation. */ + gboolean error = FALSE; + gboolean guarded = seaf_db_trans_check_for_existence (trans, + "SELECT path FROM cf_edit_guard FORCE INDEX (cf_edit_guard_active) " + "WHERE repo_id=? AND path_hash=UNHEX(?) AND path=? " + "AND guard_id IS NOT NULL AND lease_expires_at>UTC_TIMESTAMP(6) FOR UPDATE", + &error, 3, "string", repo, "string", digest, "string", path); g_free (digest); - return result; + return guarded || error ? -1 : 0; } int diff --git a/tests/cf-lock/fixture.c b/tests/cf-lock/fixture.c new file mode 100644 index 00000000..512fc656 --- /dev/null +++ b/tests/cf-lock/fixture.c @@ -0,0 +1,92 @@ +/* Production barrier/dispatcher; fake config and database only, no network. */ +#include +#include +#include "cf-fileop.h" +#include "cf-lock.h" +#include "seafile-error.h" +#include "log.h" +typedef int SeafDB; +typedef int SeafDBRow; +typedef int SeafDBTrans; +static gboolean transaction_error; +static SeafDB database; +static gboolean checkout; +static int sql_calls, exact_rows, descendant_rows; +static gboolean cf_ext_config_bool(const char *key) +{ + /* A stale file_lock_enabled=true must not activate Checkout. */ + return !strcmp(key, "checkout_enabled") ? checkout : TRUE; +} +static char *cf_ext_config_string(const char *key) { return NULL; } +static SeafDB *cf_ext_database(void) { return &database; } +static int seaf_db_statement_foreach_row(SeafDB *db, const char *sql, + gboolean (*callback)(SeafDBRow *, void *), void *data, int count, ...) +{ + ++sql_calls; + g_assert_nonnull(strstr(sql,"FROM cf_edit_guard")); + g_assert_nonnull(strstr(sql,"cf_edit_guard_active")); + g_assert_nonnull(strstr(sql,"lease_expires_at>UTC_TIMESTAMP(6)")); + g_assert_null(strstr(sql,"cf_resource")); + g_assert_null(strstr(sql,"cf_schema_migration")); + if (strstr(sql,"path_hash=UNHEX(?)")) return exact_rows; + g_assert_nonnull(strstr(sql,"path LIKE ?")); + return descendant_rows; +} +static gboolean seaf_db_trans_check_for_existence(SeafDBTrans *trans, + const char *sql, gboolean *error, int count, ...) +{ + ++sql_calls; + g_assert_nonnull(strstr(sql,"FROM cf_edit_guard")); + g_assert_nonnull(strstr(sql,"path_hash=UNHEX(?)")); + g_assert_nonnull(strstr(sql,"FOR UPDATE")); + g_assert_null(strstr(sql,"cf_resource")); + *error = transaction_error; + return exact_rows > 0; +} +#include "production.c" + +static int write_path(const char *path, GError **error) +{ + CfFileOp op = {.op=CF_OP_UPDATE_FILE,.repo_id="fixture",.dir=path,.user="owner"}; + return cf_fileop_prepare(&op,error); +} +int main(void) +{ + GError *error = NULL; + checkout=FALSE; cf_lock_init(); + g_assert_false(cf_lock_enabled()); + g_assert_cmpint(write_path("/file",&error),==,0); + g_assert_cmpint(sql_calls,==,0); + checkout=TRUE; cf_lock_init(); + g_assert_true(cf_lock_enabled()); + g_assert_cmpint(write_path("/file",&error),==,0); + g_assert_cmpint(sql_calls,==,2); + exact_rows=1; + g_assert_cmpint(write_path("/file",&error),==,-1); + g_assert_error(error,SEAFILE_DOMAIN,CF_ERR_FILE_LOCKED); g_clear_error(&error); + exact_rows=0; descendant_rows=1; + g_assert_cmpint(write_path("/parent",&error),==,-1); + g_assert_error(error,SEAFILE_DOMAIN,CF_ERR_FILE_LOCKED); g_clear_error(&error); + descendant_rows=0; exact_rows=-1; + g_assert_cmpint(write_path("/file",&error),==,-1); + g_assert_error(error,SEAFILE_DOMAIN,SEAF_ERR_GENERAL); g_clear_error(&error); + exact_rows=0; descendant_rows=-1; + g_assert_cmpint(write_path("/parent",&error),==,-1); + g_assert_error(error,SEAFILE_DOMAIN,SEAF_ERR_GENERAL); g_clear_error(&error); + checkout=FALSE; cf_fileop_reset(); cf_lock_init(); sql_calls=0; + g_assert_cmpint(write_path("/file",&error),==,0); + g_assert_cmpint(sql_calls,==,0); + /* The final Branch transaction uses the real production function too. */ + g_assert_cmpint(cf_policy_check_unleased_write(NULL,"fixture","/file","owner"),==,0); + g_assert_cmpint(sql_calls,==,0); + checkout=TRUE; cf_lock_init(); exact_rows=0; + g_assert_cmpint(cf_policy_check_unleased_write(NULL,"fixture","/file","owner"),==,0); + exact_rows=1; + g_assert_cmpint(cf_policy_check_unleased_write(NULL,"fixture","/file","owner"),==,-1); + exact_rows=0; transaction_error=TRUE; + g_assert_cmpint(cf_policy_check_unleased_write(NULL,"fixture","/file","owner"),==,-1); + transaction_error=FALSE; + g_assert_cmpint(cf_policy_check_unleased_write(NULL,"fixture","relative","owner"),==,-1); + g_print("PASS 12 native Checkout barrier and final-transaction cases\n"); + return 0; +} diff --git a/tests/cf-lock/run.py b/tests/cf-lock/run.py new file mode 100644 index 00000000..3b31a066 --- /dev/null +++ b/tests/cf-lock/run.py @@ -0,0 +1,26 @@ +#!/usr/bin/env python3 +"""Compile the complete production lock barrier and real fileop dispatcher. + +Only config/database I/O is doubled. This proves disabled Checkout issues zero +SQL and active reservations/SQL failures refuse native unscoped writes. +""" +from pathlib import Path +import re +import subprocess +import tempfile + +root = Path(__file__).resolve().parents[2] +with tempfile.TemporaryDirectory(prefix='cf-lock-test-') as folder: + build = Path(folder) + # Keep every production statement; replace only external include boundaries. + source = re.sub(r'^#include[^\n]*\n', '', (root/'common/cf-lock.c').read_text(), flags=re.M) + policy = (root/'server/cloudfile-policy.c').read_text() + start = policy.index('int\ncf_policy_check_unleased_write (') + end = policy.index('\n}\n', start) + 3 + (build/'production.c').write_text(source + '\n' + policy[start:end]) + flags = subprocess.check_output(['pkg-config','--cflags','--libs','glib-2.0'], text=True).split() + subprocess.run(['cc','-std=c99','-Wall','-Wextra','-Werror','-Wno-unused-parameter', + '-I'+str(build),'-I'+str(root/'common'),'-I'+str(root/'include'), + str(root/'tests/cf-lock/fixture.c'),str(root/'common/cf-fileop.c'), + str(root/'common/cf-path.c'),'-o',str(build/'test'),*flags],check=True) + subprocess.run([str(build/'test')],check=True)