Skip to content

fix(recovery): refuse implicit bulk V2 library backups; require legacy-only scope - #36

Draft
servicememeai wants to merge 19 commits into
devfrom
fix/storage-backup-managed-library-root-20261024
Draft

servicememeai wants to merge 19 commits into
devfrom
fix/storage-backup-managed-library-root-20261024

Conversation

@servicememeai

@servicememeai servicememeai commented Oct 24, 2026 •

Copy link
Copy Markdown

Scope correction (2026-10-24)

This PR was originally drafted to silently include all data/libraries objects in a whole-instance snapshot. That would cause a large time/space operation and could still miss externally mounted library roots. It has been reworked to reject this unsafe behavior, honoring the user's per-library default.

Current changes

  • Restore the original legacy CE cf_recovery.py volume scope (data/db, data/seafile, optional legacy data/minio). It does not copy data/libraries implicitly.
  • Any manual full-instance legacy snapshot now requires both the existing --ack-writers-stopped and new --legacy-ce-only flags.
  • Refuse to initiate the large snapshot when V2 mode is detected by environment, persisted Seafile config or existing populated data/libraries directory. Do not return a misleading successful backup for a partial V2 snapshot.
  • Maintain a separate explicit per-repo offline workflow in Docker PR #40 (inspect → selected repo seaf-fsck export → optional verify).
  • Targeted synthetic tests and path-filtered cheap CI; no service, image or dataset build.

Distinction

cf_recovery backup --legacy-ce-only is an explicitly requested whole-deployment legacy CE disaster snapshot, not a normal per-library operation. This PR does not convert it into a V2 per-library exporter. Current V2 libraries must be handled only by explicit repo-scoped maintenance; future whole-instance DR requires a separate reviewed design/backup contract.

Draft until the new gating tests and an actual CE / V2 config case are checked. No real backups or services were touched.

@servicememeai servicememeai left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

存储可靠性评审:当前不能认定离线备份完整

补齐 data/libraries 是正确且重要的修复;但本 PR 仍有会使恢复声明失真的前提缺口,建议暂不合并为“可靠备份”。

  1. validate_managed_root() 自行解析 .env 里的 CF_LIBRARY_STORAGE_HOST_DIR,并读取固定路径配置;实际有效 bind mount 可能来自 shell 环境、Compose override/不同 COMPOSE_FILE。例如 .env 仍写默认目录、启动时用环境变量改到外部目录,工具可复制未被使用的旧目录并生成校验通过的 manifest。必须验证实际解析后的 Compose 挂载或明确拒绝无法确认的部署,不能以路径字符串猜测完整性。
  2. stopped() 仅以基础 -f docker-compose.yml 运行 ps,还需明确怎么覆盖 worker、可选服务、同数据根的其他写者;仅有 --ack-writers-stopped 参数并不能自动保证一致快照。
  3. 工具明确不支持 S3 一致备份,却仍复制可选 data/minio,没有在 S3/multi 模式 fail closed;RustFS 也是可选 S3 后端但不在清单。建议严格把此工具限制为 offline local-only,发现实际 S3 后端配置就拒绝并说明使用独立备份方案,避免造成错误的完整性印象。

最小验收:真实 default compose 及覆盖 mount 的拒绝、旧 CE / 新 auto-local 的三类对象和 SQL 同一停写点、恢复后 HEAD/版本/字节一致 + seaf-fsck;合成 copy/hash 测试不能证明全栈恢复。避免为此扩建备份框架。

@servicememeai
servicememeai marked this pull request as draft October 24, 2026 07:45
@servicememeai servicememeai changed the title fix(storage): include per-library managed root in offline snapshots fix(recovery): refuse implicit bulk V2 library backups; require legacy-only scope Oct 24, 2026

Copy link
Copy Markdown
Author

2026-10-24 P0 最小修复更新(继续 Draft):

  • 旧 cf_recovery.py 全实例操作必须显式 --legacy-ce-only,发现 V2 data/libraries、auto-local、S3/multiple 即拒绝;不自动打包 V2 对象根或可选 MinIO 大目录,历史已有含 MinIO 的快照仍可读取/恢复。
  • 检查 COMPOSE_FILE 不允许额外 overlay,停写检查前通过 docker compose -f docker-compose.yml config --format json 对实际解析的 db/cloudfile/cf-worker 挂载源和目标做校验,不再依赖 .env 猜路径。与实际数据路径不一致就拒绝。
  • 添了轻量合成测试(配置不一致、覆盖 Compose、V2/S3 拒绝),Quick Source Checks success。
    待验收:真实 legacy-only Compose 解析、停写一致点及恢复。V2 逐库显式导出另见 Docker Draft feat(storage): locate and explicitly export one Seafile library offline #40;本 PR 不做 V2 备份,不阻塞核心默认运行。

Copy link
Copy Markdown
Author

2026-10-24 effective Compose path verification

PASS — GitHub Actions run #40081101691 (commit 6c975aa4ebd19b139678d69217d8efc3703c6913): actual GitHub Runner Docker Compose config --format json resolved the standard CE local DB/data bind mounts; injected an incorrect DB bind source and confirmed cf_recovery.stopped() rejected it before any backup. The separate synthetic safety/unit checks passed as well. No services were launched and no library bytes were copied.

The PR-only Compose job was removed after passing to prevent high-cost routine CI and avoid shipping a special branch rule; ordinary quick CI remains.

Remaining acceptance: actual stopped Seafile CE local volumes + consistent MariaDB/filesystem restore verification, plus proof that writer containers are stopped. This utility stays legacy-only and refuses V2/S3. Keep Draft; never run a default/all-library backup.

The fail-closed bulk-backup gate parsed .env with a raw partition("=") and
strip() of quotes, so Compose-legal forms such as 'export KEY=auto', quoted
values and trailing '# comments' resolved to V2/S3 in Docker but passed this
safety check. Normalise each line like the Compose dotenv parser for the
guarded keys and cover the bypass formats with tests.
@opendev9

Copy link
Copy Markdown

Independent review fix pushed in fb5d9cc (review-only commit rebased onto the current PR head, no history rewrite):

  • tools/migrate/cf_recovery.py: the .env V2/S3 guard parsed every line with partition('=') + strip('\'"'). Docker Compose still honours export KEY=auto, quoted values and trailing # comments (measured with Compose 5.5.0), so a Compose-valid .env could bypass the refusal and let a V2/S3 whole-instance legacy snapshot through when the library root is an external mount and seafile.conf is not yet present. New dotenv_entry() normalises guarded keys exactly like Compose; 6 regression tests added (suite now 14/14).
  • Verified: 14/14 tests pass, py_compile OK, quick static CI on the new head passes. No change to the passing mount/V2 refusal logic.

Review verdict unchanged: MERGE AFTER one real legacy rehearsal (stop writers -> backup --legacy-ce-only -> verify -> restore into an isolated dir) and a rebase onto dev. Two report-only gaps worth fixing before or soon after: auto-loaded docker-compose.override.yml is not refused, and the PR body is stale about data/minio no longer being copied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants