From fe68fec69e06b149bf31838b5a07dedef2586819 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 21:33:33 +0100 Subject: [PATCH 1/6] feat: docsgpt backup and docsgpt restore `docsgpt backup` writes one archive holding a pg_dump of the database, a tar of each data volume and a manifest of what it came from; `docsgpt restore` puts it back over an install. The settings file is left out unless --with-settings asks for it, since it holds the install's secrets, and a backup taken with a newer DocsGPT is refused without --force. The volume tars go through the image the install already runs, so a backup pulls nothing extra, and compose calls can now redirect stdout and stdin so the dump never passes through this process. --- docs/content/Deploying/Docker-Deploying.mdx | 30 +++++ docs/content/changelog.mdx | 10 ++ docsgpt/cli.py | 12 ++ docsgpt/deploy/backup.py | 107 ++++++++++++++++ docsgpt/deploy/commands.py | 115 +++++++++++++++++ docsgpt/deploy/docker.py | 36 +++++- tests/deploy/test_backup.py | 131 ++++++++++++++++++++ tests/deploy/test_commands.py | 25 +++- tests/deploy/test_docker.py | 30 ++++- 9 files changed, 489 insertions(+), 7 deletions(-) create mode 100644 docsgpt/deploy/backup.py create mode 100644 tests/deploy/test_backup.py diff --git a/docs/content/Deploying/Docker-Deploying.mdx b/docs/content/Deploying/Docker-Deploying.mdx index b4c5f87f..847c7453 100644 --- a/docs/content/Deploying/Docker-Deploying.mdx +++ b/docs/content/Deploying/Docker-Deploying.mdx @@ -79,6 +79,36 @@ the questions again. | `docsgpt down` | Stop the stack; data and settings stay | | `docsgpt uninstall [--purge]` | Remove the containers; `--purge` also deletes the settings and all data | +### Backups + +`docsgpt backup` writes one archive holding a dump of the database and a tar of +each data volume (`indexes`, `inputs`, `vectors`): + +```bash +docsgpt backup # into /backups +docsgpt backup --out /mnt/backups # somewhere else, e.g. a mounted disk +``` + +The archive does **not** include `.env`, because that file holds the install's +secrets. `docsgpt backup --with-settings` puts it in, for when the archive +itself is stored somewhere private. Keep `.env` safe separately otherwise: the +database password in it is what an existing Postgres volume expects. + +Restoring replaces the data in an install: + +```bash +docsgpt restore ~/.docsgpt/server/backups/docsgpt-20260916-120000.tar.gz +``` + +It asks first, then stops the stack, puts the volumes and the database back, and +starts DocsGPT again. `--yes` skips the question for scripts. A backup taken +with a newer DocsGPT is refused, since its data may not fit this version's +schema; upgrade first, or pass `--force` if you know the two match. + +The Postgres data directory itself is not archived: the dump is the database +backup, and copying a directory Postgres is writing to would capture a torn +copy. Caddy's certificates are not archived either, as it obtains them again. + More `docsgpt up` options: `--port`, `--docling` (the image with the docling parser engine and OCR), `--image-tag develop` (follow the `main` branch) and `--adopt` (manage a stack you started from the standalone Compose file in diff --git a/docs/content/changelog.mdx b/docs/content/changelog.mdx index 6e59a1f5..2c5f9501 100644 --- a/docs/content/changelog.mdx +++ b/docs/content/changelog.mdx @@ -11,6 +11,16 @@ The notable changes in each release. Every release on GitHub also carries [auto-generated notes](https://github.com/arc53/DocsGPT/releases) listing every merged pull request, and [Upgrading](/upgrading) covers the steps an existing deployment has to take. +## Unreleased + +### Back up and restore an install + +`docsgpt backup` writes a dump of the database and a tar of each data volume into one archive, and +`docsgpt restore ` puts them back. The settings file is left out unless +`--with-settings` asks for it, since it holds the install's secrets, and a backup from a newer +DocsGPT is refused unless you pass `--force`. See +[Backups](/Deploying/Docker-Deploying#backups). + ## 0.21.0 ### Install with one command diff --git a/docsgpt/cli.py b/docsgpt/cli.py index 873cd099..e5a5af2a 100644 --- a/docsgpt/cli.py +++ b/docsgpt/cli.py @@ -230,6 +230,18 @@ def _add_deploy_commands(commands) -> None: uninstall.add_argument("-y", "--yes", action="store_true", help="do not ask for confirmation") uninstall.add_argument("--purge", action="store_true", help="also delete the settings and all data") + backup = stack_command("backup", "backup", "write a backup of the database and the uploaded data") + backup.add_argument("--out", help="directory for the archive (default: /backups)") + backup.add_argument("--with-settings", action="store_true", + help="include .env in the archive; it holds this install's secrets") + + restore = stack_command("restore", "restore", "restore a backup over this install") + restore.add_argument("archive", help="the .tar.gz written by `docsgpt backup`") + restore.add_argument("-y", "--yes", action="store_true", help="do not ask for confirmation") + restore.add_argument("--force", action="store_true", help="restore a backup taken with a newer DocsGPT") + restore.add_argument("--timeout", type=int, default=300, + help="seconds to wait for the API afterwards (default: 300)") + env = stack_command("env", "env", "show, get or set the stack's settings") env_actions = env.add_subparsers(dest="env_action", metavar="") get = env_actions.add_parser("get", help="print one setting") diff --git a/docsgpt/deploy/backup.py b/docsgpt/deploy/backup.py new file mode 100644 index 00000000..5b041c23 --- /dev/null +++ b/docsgpt/deploy/backup.py @@ -0,0 +1,107 @@ +"""The archive ``docsgpt backup`` writes and ``docsgpt restore`` reads. + +One gzipped tar holds a SQL dump of the database, a tar per data volume, and a +manifest saying which version and image the backup came from. The settings file +is left out unless it is asked for: it holds the secrets. + +``postgres_data`` is not tarred, because the dump is the database backup and a +copy of a running data directory would be a torn one. Caddy's volumes are left +out too: they hold certificates it obtains again on the next start. +""" + +from __future__ import annotations + +import io +import json +import tarfile +import time +from collections.abc import Mapping +from datetime import datetime +from pathlib import Path +from typing import Optional + +from docsgpt.deploy.docker import DeployError + +FORMAT = 1 +MANIFEST = "manifest.json" +DUMP = "database.sql" +SETTINGS = "settings.env" +VOLUME_DIR = "volumes" +DATA_VOLUMES = ("indexes", "inputs", "vectors") + + +def archive_name(when: datetime) -> str: + """The file name for a backup taken at ``when``.""" + return f"docsgpt-{when.strftime('%Y%m%d-%H%M%S')}.tar.gz" + + +def volume_member(name: str) -> str: + """Where a volume's tar sits inside the archive.""" + return f"{VOLUME_DIR}/{name}.tar" + + +def write_archive( + path: Path, + *, + dump: Path, + volume_tars: Mapping[str, Path], + manifest: Mapping[str, object], + settings: Optional[Path] = None, +) -> None: + """Write the backup archive; the manifest is added last so a truncated file has none.""" + path.parent.mkdir(parents=True, exist_ok=True) + with tarfile.open(path, "w:gz") as archive: + archive.add(dump, arcname=DUMP) + for name, tar_path in sorted(volume_tars.items()): + archive.add(tar_path, arcname=volume_member(name)) + if settings is not None: + archive.add(settings, arcname=SETTINGS) + body = json.dumps(dict(manifest), indent=2).encode("utf-8") + info = tarfile.TarInfo(MANIFEST) + info.size = len(body) + info.mtime = int(time.time()) + info.mode = 0o600 + archive.addfile(info, io.BytesIO(body)) + + +def read_manifest(path: Path) -> dict: + """The archive's manifest, or a DeployError naming what is wrong with the file.""" + if not path.is_file(): + raise DeployError(f"{path} does not exist") + try: + with tarfile.open(path, "r:gz") as archive: + member = archive.extractfile(MANIFEST) + if member is None: + raise KeyError(MANIFEST) + manifest = json.loads(member.read().decode("utf-8")) + except (tarfile.TarError, KeyError, ValueError, OSError) as exc: + raise DeployError(f"{path} is not a DocsGPT backup: {exc}") from exc + if not isinstance(manifest, dict) or "volumes" not in manifest: + raise DeployError(f"{path} is not a DocsGPT backup: its manifest is missing what to restore") + return manifest + + +def extract(path: Path, destination: Path) -> None: + """Unpack the archive into ``destination`` (data filter: no paths outside it, no devices).""" + with tarfile.open(path, "r:gz") as archive: + archive.extractall(destination, filter="data") + + +def _parts(version: str) -> tuple[int, ...]: + numbers = [] + for chunk in str(version).split("."): + digits = "".join(character for character in chunk if character.isdigit()) + numbers.append(int(digits) if digits else 0) + return tuple(numbers) + + +def check_version(manifest: Mapping[str, object], current: str, force: bool) -> None: + """Refuse a backup from a newer DocsGPT: its data may not fit this version's schema.""" + taken_with = str(manifest.get("version") or "") + if force or not taken_with: + return + if _parts(taken_with) > _parts(current): + raise DeployError( + f"this backup is from DocsGPT {taken_with}, newer than the installed {current}. " + "Upgrade first with `docsgpt upgrade`, or pass --force to restore it anyway." + ) diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index eaa38f44..fb9c5e25 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -9,6 +9,7 @@ import re import shutil import subprocess import sys +import tempfile import webbrowser from collections.abc import Callable, Mapping from dataclasses import dataclass @@ -16,6 +17,7 @@ from datetime import datetime, timezone from pathlib import Path from typing import Any, Optional +from docsgpt.deploy import backup as backup_format from docsgpt.deploy import envfile, stack from docsgpt.deploy.docker import DeployError, Docker, lan_ip, wait_healthy @@ -350,6 +352,119 @@ def env(args, context: Optional[Context] = None) -> int: return 0 +def _stack_image(env: Mapping[str, str]) -> str: + """The image this install runs; the volume tars go through it, so nothing extra is pulled.""" + tag = env.get("DOCSGPT_IMAGE_TAG") or "latest" + return f"arc53/docsgpt:{tag}{env.get('DOCSGPT_IMAGE_VARIANT', '')}" + + +def backup(args, context: Optional[Context] = None) -> int: + """Write a dump of the database and a tar of each data volume into one archive.""" + context = context or Context.default(args) + directory = stack.stack_dir(args.dir) + env = _installed(directory) + if env is None: + return 1 + + out_dir = Path(args.out).expanduser() if args.out else directory / "backups" + taken_at = datetime.now(timezone.utc) + target = out_dir / backup_format.archive_name(taken_at) + image = _stack_image(env) + + with tempfile.TemporaryDirectory() as workspace: + work = Path(workspace) + dump = work / backup_format.DUMP + print("Dumping the database ...") + with dump.open("w", encoding="utf-8") as handle: + context.docker.compose( + directory, "exec", "-T", "postgres", + "pg_dump", "--clean", "--if-exists", "-U", "docsgpt", "-d", "docsgpt", + stdout=handle, + ) + volume_tars = {} + for name in backup_format.DATA_VOLUMES: + print(f"Archiving the {name} volume ...") + tar_path = work / f"{name}.tar" + context.docker.export_volume(f"{PROJECT}_{name}", tar_path, image) + volume_tars[name] = tar_path + manifest = { + "format": backup_format.FORMAT, + "created_at": taken_at.isoformat(timespec="seconds"), + "version": context.version, + "image_tag": env.get("DOCSGPT_IMAGE_TAG", ""), + "volumes": list(backup_format.DATA_VOLUMES), + "settings_included": bool(args.with_settings), + } + settings = directory / ".env" if args.with_settings else None + backup_format.write_archive(target, dump=dump, volume_tars=volume_tars, manifest=manifest, settings=settings) + + size = target.stat().st_size / 1_000_000 + print(f"\nBackup written to {target} ({size:.1f} MB)") + if args.with_settings: + print("It contains .env, so it holds this install's secrets: keep it somewhere private.") + else: + print("Settings are not in it; `docsgpt backup --with-settings` includes .env, secrets and all.") + print(f"Restore it with: docsgpt restore {target}") + return 0 + + +def restore(args, context: Optional[Context] = None) -> int: + """Put a backup's database and data volumes back over this install.""" + context = context or Context.default(args) + archive = Path(args.archive).expanduser() + manifest = backup_format.read_manifest(archive) + backup_format.check_version(manifest, context.version, args.force) + + directory = stack.stack_dir(args.dir) + env = _installed(directory) + if env is None: + return 1 + + taken_at = manifest.get("created_at", "an unknown time") + if not args.yes: + if not context.interactive: + raise DeployError("restore needs --yes when there is no terminal to confirm on") + question = f"Replace the data in {directory} with the backup from {taken_at}? This cannot be undone." + if not context.prompter.confirm(question, default=False): + print("Nothing restored.") + return 1 + + image = _stack_image(env) + print("Stopping the stack ...") + context.docker.compose(directory, *_EVERY_PROFILE, "down") + + with tempfile.TemporaryDirectory() as workspace: + work = Path(workspace) + backup_format.extract(archive, work) + for name in manifest.get("volumes", []): + tar_path = work / backup_format.volume_member(name) + if not tar_path.is_file(): + raise DeployError(f"{archive} is missing the {name} volume it says it contains") + print(f"Restoring the {name} volume ...") + context.docker.import_volume(f"{PROJECT}_{name}", tar_path, image) + + dump = work / backup_format.DUMP + if not dump.is_file(): + raise DeployError(f"{archive} is missing its database dump") + print("Starting the database ...") + context.docker.compose(directory, "up", "-d", "--wait", "postgres") + print("Restoring the database ...") + with dump.open("r", encoding="utf-8") as handle: + context.docker.compose( + directory, "exec", "-T", "postgres", + "psql", "--quiet", "-U", "docsgpt", "-d", "docsgpt", + stdin=handle, + ) + + print("Starting DocsGPT ...") + context.docker.compose(directory, "up", "-d", "--remove-orphans") + if not context.wait(stack.health_url(env), args.timeout): + print("DocsGPT did not answer after the restore. See `docsgpt logs backend`.", file=sys.stderr) + return 1 + print(f"\nRestored the backup from {taken_at}. DocsGPT is running at {stack.url(env, context.lan_ip())}") + return 0 + + def _uninstall_hint(installer: str) -> str: return {"uv": "uv tool uninstall docsgpt", "pipx": "pipx uninstall docsgpt"}.get(installer, "pip uninstall docsgpt") diff --git a/docsgpt/deploy/docker.py b/docsgpt/deploy/docker.py index 795aaeb9..69cbe187 100644 --- a/docsgpt/deploy/docker.py +++ b/docsgpt/deploy/docker.py @@ -22,10 +22,16 @@ class DeployError(Exception): """A problem the user can act on; the command prints it without a traceback.""" -def run(args: Sequence[str], *, cwd: Optional[Path] = None, capture: bool = False, check: bool = True): - """Run a command, streaming its output unless ``capture``; with ``check`` a failure raises DeployError.""" +def run(args: Sequence[str], *, cwd: Optional[Path] = None, capture: bool = False, check: bool = True, + stdout=None, stdin=None): + """Run a command, streaming its output unless ``capture`` or a file is given for ``stdout``. + + ``stdout`` and ``stdin`` take open files, so a dump goes straight to disk and + back in again without passing through this process. + """ + streams = {"capture_output": True} if capture else {"stdout": stdout, "stdin": stdin} try: - result = subprocess.run(list(args), cwd=cwd, text=True, capture_output=capture, check=False) + result = subprocess.run(list(args), cwd=cwd, text=True, check=False, **streams) except FileNotFoundError as exc: raise DeployError(f"{args[0]} is not installed or not on PATH") from exc if check and result.returncode != 0: @@ -94,9 +100,29 @@ class Docker: raise DeployError("Docker is not running. Start it with `sudo systemctl start docker` and run this again.") raise DeployError("Docker is not running. Start Docker Desktop and run this again.") - def compose(self, directory: Path, *args: str, capture: bool = False, check: bool = True): + def compose(self, directory: Path, *args: str, capture: bool = False, check: bool = True, + stdout=None, stdin=None): """``docker compose `` in ``directory``, which holds the Compose file and its ``.env``.""" - return self._run(["docker", "compose", *args], cwd=directory, capture=capture, check=check) + return self._run(["docker", "compose", *args], cwd=directory, capture=capture, check=check, + stdout=stdout, stdin=stdin) + + def export_volume(self, volume: str, dest: Path, image: str) -> None: + """Write ``volume`` to ``dest`` as a tar, through an image the install already has.""" + dest.parent.mkdir(parents=True, exist_ok=True) + with dest.open("wb") as handle: + self._run( + ["docker", "run", "--rm", "-v", f"{volume}:/data:ro", image, "tar", "cf", "-", "-C", "/data", "."], + stdout=handle, + ) + + def import_volume(self, volume: str, source: Path, image: str) -> None: + """Replace ``volume``'s contents with the tar at ``source``; the volume is created when missing.""" + with source.open("rb") as handle: + self._run( + ["docker", "run", "--rm", "-i", "-v", f"{volume}:/data", image, + "sh", "-c", "find /data -mindepth 1 -delete && tar xf - -C /data"], + stdin=handle, + ) def volume_exists(self, name: str) -> bool: return self._run(["docker", "volume", "inspect", name], capture=True, check=False).returncode == 0 diff --git a/tests/deploy/test_backup.py b/tests/deploy/test_backup.py new file mode 100644 index 00000000..be43bfcf --- /dev/null +++ b/tests/deploy/test_backup.py @@ -0,0 +1,131 @@ +"""`docsgpt backup` and `docsgpt restore`, against a fake Docker.""" + +import io +import json +import tarfile + +import pytest + +from docsgpt.deploy import backup as backup_module +from docsgpt.deploy.docker import DeployError + +from .test_commands import FakeDocker, FakePrompter, _context, _run + + +def _installed(tmp_path, *extra): + assert _run(["up", "--yes", "--dir", str(tmp_path), *extra], _context()) == 0 + + +def _archive_names(path): + with tarfile.open(path, "r:gz") as archive: + return sorted(member.name for member in archive.getmembers() if member.isfile()) + + +def _manifest(path): + with tarfile.open(path, "r:gz") as archive: + return json.loads(archive.extractfile(backup_module.MANIFEST).read()) + + +def _copy_with_version(archive, version, dest): + """The same archive, with another DocsGPT version written into its manifest.""" + with tarfile.open(archive, "r:gz") as source, tarfile.open(dest, "w:gz") as out: + for member in source.getmembers(): + body = source.extractfile(member).read() if member.isfile() else None + if member.name == backup_module.MANIFEST: + manifest = json.loads(body.decode()) + manifest["version"] = version + body = json.dumps(manifest).encode() + member.size = len(body) + out.addfile(member, io.BytesIO(body) if body is not None else None) + return dest + + +class TestBackup: + def test_writes_a_dump_a_tar_per_volume_and_a_manifest(self, tmp_path, capsys): + _installed(tmp_path) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + out = tmp_path / "backups" + assert _run(["backup", "--dir", str(tmp_path), "--out", str(out)], _context(docker)) == 0 + + archives = list(out.glob("docsgpt-*.tar.gz")) + assert len(archives) == 1, archives + names = _archive_names(archives[0]) + assert backup_module.DUMP in names + for volume in ("indexes", "inputs", "vectors"): + assert f"volumes/{volume}.tar" in names + manifest = _manifest(archives[0]) + assert manifest["version"] == "0.21.0" + assert manifest["image_tag"] == "0.21.0" + assert manifest["volumes"] == ["indexes", "inputs", "vectors"] + assert manifest["settings_included"] is False + assert "Backup written to" in capsys.readouterr().out + assert [op for op in docker.volume_ops if op[0] == "export"], docker.volume_ops + + def test_the_settings_file_is_left_out_unless_asked_for(self, tmp_path): + _installed(tmp_path) + out = tmp_path / "backups" + assert _run(["backup", "--dir", str(tmp_path), "--out", str(out)], _context()) == 0 + assert backup_module.SETTINGS not in _archive_names(next(out.glob("*.tar.gz"))) + + with_settings = tmp_path / "with-settings" + argv = ["backup", "--dir", str(tmp_path), "--out", str(with_settings), "--with-settings"] + assert _run(argv, _context()) == 0 + assert backup_module.SETTINGS in _archive_names(next(with_settings.glob("*.tar.gz"))) + + def test_the_database_is_dumped_from_the_running_container(self, tmp_path): + _installed(tmp_path) + docker = FakeDocker() + assert _run(["backup", "--dir", str(tmp_path), "--out", str(tmp_path / "b")], _context(docker)) == 0 + dumps = [args for _, args in docker.calls if "pg_dump" in " ".join(args)] + assert dumps, docker.calls + assert dumps[0][:3] == ["exec", "-T", "postgres"] + + def test_without_an_install(self, tmp_path, capsys): + assert _run(["backup", "--dir", str(tmp_path)], _context()) == 1 + assert "docsgpt up" in capsys.readouterr().err + + +class TestRestore: + def _backup(self, tmp_path, *extra): + _installed(tmp_path) + out = tmp_path / "backups" + assert _run(["backup", "--dir", str(tmp_path), "--out", str(out), *extra], _context()) == 0 + return next(out.glob("*.tar.gz")) + + def test_restores_the_volumes_and_the_database(self, tmp_path): + archive = self._backup(tmp_path) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + argv = ["restore", str(archive), "--dir", str(tmp_path), "--yes"] + assert _run(argv, _context(docker)) == 0 + joined = [" ".join(args) for _, args in docker.calls] + assert any(call.startswith("--profile https down") for call in joined), joined + assert any("psql" in call for call in joined), joined + assert any(call.startswith("up -d") for call in joined), joined + + def test_asks_before_replacing_data(self, tmp_path): + archive = self._backup(tmp_path) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + prompter = FakePrompter([False]) + assert _run(["restore", str(archive), "--dir", str(tmp_path)], _context(docker, prompter, interactive=True)) == 1 + assert docker.calls == [] + + def test_refuses_an_archive_that_is_not_a_docsgpt_backup(self, tmp_path): + stray = tmp_path / "stray.tar.gz" + with tarfile.open(stray, "w:gz") as archive: + note = tmp_path / "note.txt" + note.write_text("not a backup") + archive.add(note, arcname="note.txt") + with pytest.raises(DeployError, match="not a DocsGPT backup"): + _run(["restore", str(stray), "--dir", str(tmp_path), "--yes"], _context()) + + def test_a_missing_archive(self, tmp_path): + with pytest.raises(DeployError, match="does not exist"): + _run(["restore", str(tmp_path / "nope.tar.gz"), "--dir", str(tmp_path), "--yes"], _context()) + + def test_a_newer_backup_is_refused_without_force(self, tmp_path): + """Restoring a 0.22 backup into 0.21 would hand an older schema newer data.""" + newer = _copy_with_version(self._backup(tmp_path), "0.22.0", tmp_path / "newer.tar.gz") + with pytest.raises(DeployError, match="newer"): + _run(["restore", str(newer), "--dir", str(tmp_path), "--yes"], _context(FakeDocker())) + argv = ["restore", str(newer), "--dir", str(tmp_path), "--yes", "--force"] + assert _run(argv, _context(FakeDocker())) == 0 diff --git a/tests/deploy/test_commands.py b/tests/deploy/test_commands.py index 30faa350..b73fedb2 100644 --- a/tests/deploy/test_commands.py +++ b/tests/deploy/test_commands.py @@ -1,8 +1,10 @@ """`docsgpt up` and the commands that manage the stack, against a fake Docker.""" +import io import json import subprocess import sys +import tarfile from pathlib import Path import pytest @@ -19,17 +21,38 @@ class FakeDocker: self.volumes = set(volumes) self.dirs = {Path(d) for d in project_dirs} self.calls = [] + self.volume_ops = [] self.preflights = 0 def preflight(self, interactive=False): self.preflights += 1 - def compose(self, directory, *args, capture=False, check=True): + def compose(self, directory, *args, capture=False, check=True, stdout=None, stdin=None): self.calls.append((Path(directory), list(args))) if "down" in args and "-v" in args: self.volumes.clear() + if stdout is not None: + stdout.write("-- fake pg_dump\n") + if stdin is not None: + self.restored_sql = stdin.read() return subprocess.CompletedProcess(["docker", "compose", *args], 0, stdout="", stderr="") + def export_volume(self, volume, dest, image): + """Write a small tar, as the real one does with `docker run ... tar cf -`.""" + self.volume_ops.append(("export", volume, image)) + dest = Path(dest) + dest.parent.mkdir(parents=True, exist_ok=True) + with tarfile.open(dest, "w") as tar: + body = f"{volume} contents".encode() + info = tarfile.TarInfo(f"{volume}.marker") + info.size = len(body) + tar.addfile(info, io.BytesIO(body)) + + def import_volume(self, volume, source, image): + self.volume_ops.append(("import", volume, image)) + self.volumes.add(volume) + assert Path(source).is_file(), source + def volume_exists(self, name): return name in self.volumes diff --git a/tests/deploy/test_docker.py b/tests/deploy/test_docker.py index 8e42556e..0ae2dfe4 100644 --- a/tests/deploy/test_docker.py +++ b/tests/deploy/test_docker.py @@ -15,9 +15,11 @@ class FakeRunner: def __init__(self, answers=None): self.answers = list((answers or {}).items()) self.calls = [] + self.streams = [] - def __call__(self, args, *, cwd=None, capture=False, check=True): + def __call__(self, args, *, cwd=None, capture=False, check=True, stdout=None, stdin=None): self.calls.append((list(args), cwd)) + self.streams.append((stdout, stdin)) for prefix, answer in self.answers: if list(args[: len(prefix)]) == list(prefix): if callable(answer): @@ -105,6 +107,32 @@ class TestQueries: assert "label=com.docker.compose.project=docsgpt" in args +class TestVolumes: + def test_export_writes_the_volume_through_the_stack_image(self, tmp_path): + runner = FakeRunner() + dest = tmp_path / "volumes" / "inputs.tar" + _docker(runner).export_volume("docsgpt_inputs", dest, "arc53/docsgpt:0.21.0") + args, _ = runner.calls[0] + assert args[:4] == ["docker", "run", "--rm", "-v"] + assert args[4] == "docsgpt_inputs:/data:ro" + assert args[5] == "arc53/docsgpt:0.21.0" + assert args[6:] == ["tar", "cf", "-", "-C", "/data", "."] + assert dest.is_file(), "the tar is written to the destination" + assert runner.streams[0][0] is not None, "stdout goes to the file, not through this process" + + def test_import_replaces_the_volume_contents(self, tmp_path): + source = tmp_path / "inputs.tar" + source.write_bytes(b"tar") + runner = FakeRunner() + _docker(runner).import_volume("docsgpt_inputs", source, "arc53/docsgpt:0.21.0") + args, _ = runner.calls[0] + assert "docsgpt_inputs:/data" in args + assert args[-2] == "-c" + assert "tar xf - -C /data" in args[-1] + assert "find /data -mindepth 1 -delete" in args[-1], "old contents go first" + assert runner.streams[0][1] is not None, "the tar is fed in on stdin" + + class TestWaitHealthy: def test_succeeds_once_the_api_answers(self): attempts = {"n": 0} From 447ae72fe25ff91d1c91bcca46d39d909d54ddbd Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 21:55:33 +0100 Subject: [PATCH 2/6] fix: harden docsgpt backup and docsgpt restore From review of #2799: - The archive is created 0600 rather than at the process umask: it holds the install's data, and --with-settings puts .env and its secrets in it. - restore validates everything the manifest declares before the stack is stopped, so a damaged archive fails while DocsGPT is still running rather than after `compose down` has taken it away. - Only the volumes a backup is made of are restored. A hand-made manifest can no longer point import_volume at postgres_data, whose contents it empties. - psql runs with ON_ERROR_STOP=on, so a restore that fails halfway cannot start DocsGPT again and call it a success. - import_volume unpacks into the container's own filesystem first and clears the live volume only once the tar has come out whole, so a corrupt one leaves the volume as it was. - The backend and the worker stop while the archive is made and start again even if the dump fails, so the dump and the volume tars describe the same moment instead of drifting apart as ingestion writes. --- docs/content/Deploying/Docker-Deploying.mdx | 6 ++ docs/content/changelog.mdx | 3 +- docsgpt/deploy/backup.py | 34 ++++++++- docsgpt/deploy/commands.py | 35 +++++++--- docsgpt/deploy/docker.py | 7 +- tests/deploy/test_backup.py | 76 +++++++++++++++++++++ tests/deploy/test_docker.py | 5 +- 7 files changed, 150 insertions(+), 16 deletions(-) diff --git a/docs/content/Deploying/Docker-Deploying.mdx b/docs/content/Deploying/Docker-Deploying.mdx index 847c7453..ac835442 100644 --- a/docs/content/Deploying/Docker-Deploying.mdx +++ b/docs/content/Deploying/Docker-Deploying.mdx @@ -89,6 +89,12 @@ docsgpt backup # into /backups docsgpt backup --out /mnt/backups # somewhere else, e.g. a mounted disk ``` +While the archive is made, the backend and the worker stop and start again, so +the database dump and the files in the volumes describe the same moment; Postgres +itself keeps running. On a small install that pause is seconds, but count on it +if you run `docsgpt backup` from cron. The archive is written readable only by +the user who took it. + The archive does **not** include `.env`, because that file holds the install's secrets. `docsgpt backup --with-settings` puts it in, for when the archive itself is stored somewhere private. Keep `.env` safe separately otherwise: the diff --git a/docs/content/changelog.mdx b/docs/content/changelog.mdx index 2c5f9501..056fb101 100644 --- a/docs/content/changelog.mdx +++ b/docs/content/changelog.mdx @@ -18,7 +18,8 @@ request, and [Upgrading](/upgrading) covers the steps an existing deployment has `docsgpt backup` writes a dump of the database and a tar of each data volume into one archive, and `docsgpt restore ` puts them back. The settings file is left out unless `--with-settings` asks for it, since it holds the install's secrets, and a backup from a newer -DocsGPT is refused unless you pass `--force`. See +DocsGPT is refused unless you pass `--force`. The archive is written readable only by its owner, +and the backend and the worker pause while it is made so the dump and the volume tars match. See [Backups](/Deploying/Docker-Deploying#backups). ## 0.21.0 diff --git a/docsgpt/deploy/backup.py b/docsgpt/deploy/backup.py index 5b041c23..eb9dbfac 100644 --- a/docsgpt/deploy/backup.py +++ b/docsgpt/deploy/backup.py @@ -13,6 +13,7 @@ from __future__ import annotations import io import json +import os import tarfile import time from collections.abc import Mapping @@ -50,7 +51,10 @@ def write_archive( ) -> None: """Write the backup archive; the manifest is added last so a truncated file has none.""" path.parent.mkdir(parents=True, exist_ok=True) - with tarfile.open(path, "w:gz") as archive: + # 0600 from the start: the dump is the install's data, and --with-settings adds its secrets. + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + os.fchmod(descriptor, 0o600) + with os.fdopen(descriptor, "wb") as handle, tarfile.open(fileobj=handle, mode="w:gz") as archive: archive.add(dump, arcname=DUMP) for name, tar_path in sorted(volume_tars.items()): archive.add(tar_path, arcname=volume_member(name)) @@ -81,6 +85,34 @@ def read_manifest(path: Path) -> dict: return manifest +def validate(path: Path, manifest: Mapping[str, object]) -> list[str]: + """The volumes to restore, once the archive is known to hold everything it declares. + + Checked before the stack is stopped: a damaged or hand-made archive must fail while DocsGPT is + still running, not after ``docker compose down``. Only the volumes a backup is made of are + accepted, so a manifest cannot name ``postgres_data`` and have it emptied on the way in. + """ + volumes = manifest.get("volumes") + if not isinstance(volumes, list) or not all(isinstance(name, str) for name in volumes): + raise DeployError(f"{path} is not a DocsGPT backup: its manifest does not list the volumes it holds") + unsupported = sorted(set(volumes) - set(DATA_VOLUMES)) + if unsupported: + raise DeployError( + f"{path} names volumes that are not part of a backup: {', '.join(unsupported)}. " + f"A DocsGPT backup holds {', '.join(DATA_VOLUMES)}." + ) + required = [DUMP, *(volume_member(name) for name in volumes)] + try: + with tarfile.open(path, "r:gz") as archive: + present = {member.name for member in archive.getmembers() if member.isfile()} + except (tarfile.TarError, OSError) as exc: + raise DeployError(f"{path} is not a DocsGPT backup: {exc}") from exc + missing = [name for name in required if name not in present] + if missing: + raise DeployError(f"{path} is missing {', '.join(missing)}, so there is nothing to restore from") + return list(volumes) + + def extract(path: Path, destination: Path) -> None: """Unpack the archive into ``destination`` (data filter: no paths outside it, no devices).""" with tarfile.open(path, "r:gz") as archive: diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index fb9c5e25..b3bf7b28 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -371,6 +371,26 @@ def backup(args, context: Optional[Context] = None) -> int: target = out_dir / backup_format.archive_name(taken_at) image = _stack_image(env) + print("Pausing the backend and the worker so the database and the files match ...") + context.docker.compose(directory, "stop", "backend", "worker") + try: + _write_backup(args, context, directory, env, target, image, taken_at) + finally: + print("Starting the backend and the worker again ...") + context.docker.compose(directory, "up", "-d", "backend", "worker") + + size = target.stat().st_size / 1_000_000 + print(f"\nBackup written to {target} ({size:.1f} MB)") + if args.with_settings: + print("It contains .env, so it holds this install's secrets: keep it somewhere private.") + else: + print("Settings are not in it; `docsgpt backup --with-settings` includes .env, secrets and all.") + print(f"Restore it with: docsgpt restore {target}") + return 0 + + +def _write_backup(args, context: Context, directory: Path, env, target: Path, image: str, taken_at) -> None: + """Dump the database and the data volumes into ``target``, with the writers stopped.""" with tempfile.TemporaryDirectory() as workspace: work = Path(workspace) dump = work / backup_format.DUMP @@ -398,15 +418,6 @@ def backup(args, context: Optional[Context] = None) -> int: settings = directory / ".env" if args.with_settings else None backup_format.write_archive(target, dump=dump, volume_tars=volume_tars, manifest=manifest, settings=settings) - size = target.stat().st_size / 1_000_000 - print(f"\nBackup written to {target} ({size:.1f} MB)") - if args.with_settings: - print("It contains .env, so it holds this install's secrets: keep it somewhere private.") - else: - print("Settings are not in it; `docsgpt backup --with-settings` includes .env, secrets and all.") - print(f"Restore it with: docsgpt restore {target}") - return 0 - def restore(args, context: Optional[Context] = None) -> int: """Put a backup's database and data volumes back over this install.""" @@ -414,6 +425,8 @@ def restore(args, context: Optional[Context] = None) -> int: archive = Path(args.archive).expanduser() manifest = backup_format.read_manifest(archive) backup_format.check_version(manifest, context.version, args.force) + # Everything the archive declares is checked here, while DocsGPT is still up. + volumes = backup_format.validate(archive, manifest) directory = stack.stack_dir(args.dir) env = _installed(directory) @@ -436,7 +449,7 @@ def restore(args, context: Optional[Context] = None) -> int: with tempfile.TemporaryDirectory() as workspace: work = Path(workspace) backup_format.extract(archive, work) - for name in manifest.get("volumes", []): + for name in volumes: tar_path = work / backup_format.volume_member(name) if not tar_path.is_file(): raise DeployError(f"{archive} is missing the {name} volume it says it contains") @@ -452,7 +465,7 @@ def restore(args, context: Optional[Context] = None) -> int: with dump.open("r", encoding="utf-8") as handle: context.docker.compose( directory, "exec", "-T", "postgres", - "psql", "--quiet", "-U", "docsgpt", "-d", "docsgpt", + "psql", "--quiet", "--set", "ON_ERROR_STOP=on", "-U", "docsgpt", "-d", "docsgpt", stdin=handle, ) diff --git a/docsgpt/deploy/docker.py b/docsgpt/deploy/docker.py index 69cbe187..c44eb241 100644 --- a/docsgpt/deploy/docker.py +++ b/docsgpt/deploy/docker.py @@ -119,8 +119,11 @@ class Docker: """Replace ``volume``'s contents with the tar at ``source``; the volume is created when missing.""" with source.open("rb") as handle: self._run( - ["docker", "run", "--rm", "-i", "-v", f"{volume}:/data", image, - "sh", "-c", "find /data -mindepth 1 -delete && tar xf - -C /data"], + ["docker", "run", "--rm", "-i", "-v", f"{volume}:/data", image, "sh", "-c", + # Unpack into the container's own filesystem first: a truncated or corrupt tar must + # fail before the live volume is touched, not halfway through emptying it. + "set -e; rm -rf /stage; mkdir /stage; tar xf - -C /stage; " + "find /data -mindepth 1 -delete; tar cf - -C /stage . | tar xf - -C /data"], stdin=handle, ) diff --git a/tests/deploy/test_backup.py b/tests/deploy/test_backup.py index be43bfcf..371023a5 100644 --- a/tests/deploy/test_backup.py +++ b/tests/deploy/test_backup.py @@ -2,6 +2,7 @@ import io import json +import stat import tarfile import pytest @@ -26,6 +27,22 @@ def _manifest(path): return json.loads(archive.extractfile(backup_module.MANIFEST).read()) +def _rewrite(archive, dest, *, volumes=None, drop=()): + """The same archive with other volumes in its manifest, or with members left out.""" + with tarfile.open(archive, "r:gz") as source, tarfile.open(dest, "w:gz") as out: + for member in source.getmembers(): + if member.name in drop: + continue + body = source.extractfile(member).read() if member.isfile() else None + if member.name == backup_module.MANIFEST and volumes is not None: + manifest = json.loads(body.decode()) + manifest["volumes"] = volumes + body = json.dumps(manifest).encode() + member.size = len(body) + out.addfile(member, io.BytesIO(body) if body is not None else None) + return dest + + def _copy_with_version(archive, version, dest): """The same archive, with another DocsGPT version written into its manifest.""" with tarfile.open(archive, "r:gz") as source, tarfile.open(dest, "w:gz") as out: @@ -80,6 +97,40 @@ class TestBackup: assert dumps, docker.calls assert dumps[0][:3] == ["exec", "-T", "postgres"] + def test_the_archive_is_private_to_whoever_took_it(self, tmp_path): + """It can hold .env, and the dump is the install's data either way.""" + _installed(tmp_path) + out = tmp_path / "backups" + argv = ["backup", "--dir", str(tmp_path), "--out", str(out), "--with-settings"] + assert _run(argv, _context()) == 0 + archive = next(out.glob("*.tar.gz")) + assert stat.S_IMODE(archive.stat().st_mode) == 0o600 + + def test_the_writers_are_stopped_while_the_archive_is_made(self, tmp_path): + """Ingestion writes files and rows; a dump taken alongside live writes would not match them.""" + _installed(tmp_path) + docker = FakeDocker() + assert _run(["backup", "--dir", str(tmp_path), "--out", str(tmp_path / "b")], _context(docker)) == 0 + joined = [" ".join(args) for _, args in docker.calls] + stopped = next(index for index, call in enumerate(joined) if call.startswith("stop backend worker")) + dumped = next(index for index, call in enumerate(joined) if "pg_dump" in call) + started = next(index for index, call in enumerate(joined) if call.startswith("up -d backend worker")) + assert stopped < dumped < started, joined + + def test_the_writers_come_back_even_when_the_dump_fails(self, tmp_path): + _installed(tmp_path) + + class FailingDump(FakeDocker): + def compose(self, directory, *args, **kwargs): + if "pg_dump" in args: + raise DeployError("pg_dump exploded") + return super().compose(directory, *args, **kwargs) + + docker = FailingDump() + with pytest.raises(DeployError, match="pg_dump exploded"): + _run(["backup", "--dir", str(tmp_path), "--out", str(tmp_path / "b")], _context(docker)) + assert any(" ".join(args).startswith("up -d backend worker") for _, args in docker.calls), docker.calls + def test_without_an_install(self, tmp_path, capsys): assert _run(["backup", "--dir", str(tmp_path)], _context()) == 1 assert "docsgpt up" in capsys.readouterr().err @@ -122,6 +173,31 @@ class TestRestore: with pytest.raises(DeployError, match="does not exist"): _run(["restore", str(tmp_path / "nope.tar.gz"), "--dir", str(tmp_path), "--yes"], _context()) + def test_a_manifest_naming_another_volume_is_refused(self, tmp_path): + """import_volume empties what it is given, so a hand-made manifest must not name postgres_data.""" + crafted = _rewrite(self._backup(tmp_path), tmp_path / "crafted.tar.gz", volumes=["postgres_data"]) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + with pytest.raises(DeployError, match="not part of a backup"): + _run(["restore", str(crafted), "--dir", str(tmp_path), "--yes"], _context(docker)) + assert docker.calls == [], "nothing was stopped" + assert docker.volume_ops == [], "and nothing was touched" + + def test_a_damaged_archive_fails_before_the_stack_is_stopped(self, tmp_path): + """Discovering a missing member after `compose down` would leave DocsGPT down for nothing.""" + without_dump = _rewrite(self._backup(tmp_path), tmp_path / "nodump.tar.gz", drop=(backup_module.DUMP,)) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + with pytest.raises(DeployError, match="nothing to restore"): + _run(["restore", str(without_dump), "--dir", str(tmp_path), "--yes"], _context(docker)) + assert docker.calls == [], "DocsGPT is still running" + + def test_psql_stops_at_the_first_failing_statement(self, tmp_path): + """Without it psql runs on after an error and a half-restored database looks like success.""" + archive = self._backup(tmp_path) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + assert _run(["restore", str(archive), "--dir", str(tmp_path), "--yes"], _context(docker)) == 0 + psql = next(args for _, args in docker.calls if "psql" in args) + assert "ON_ERROR_STOP=on" in psql + def test_a_newer_backup_is_refused_without_force(self, tmp_path): """Restoring a 0.22 backup into 0.21 would hand an older schema newer data.""" newer = _copy_with_version(self._backup(tmp_path), "0.22.0", tmp_path / "newer.tar.gz") diff --git a/tests/deploy/test_docker.py b/tests/deploy/test_docker.py index 0ae2dfe4..61c15dd8 100644 --- a/tests/deploy/test_docker.py +++ b/tests/deploy/test_docker.py @@ -129,7 +129,10 @@ class TestVolumes: assert "docsgpt_inputs:/data" in args assert args[-2] == "-c" assert "tar xf - -C /data" in args[-1] - assert "find /data -mindepth 1 -delete" in args[-1], "old contents go first" + command = args[-1] + assert command.index("tar xf - -C /stage") < command.index("find /data -mindepth 1 -delete"), ( + "the incoming tar is unpacked outside the volume first, so a corrupt one leaves it alone" + ) assert runner.streams[0][1] is not None, "the tar is fed in on stdin" From 434763649629eae7ae0de14a7350bebfa36875a7 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 22:01:39 +0100 Subject: [PATCH 3/6] fix: stage a restored volume under /tmp, where the image can write The image does not run as root, so the staging directory could not be created at the container root: `mkdir /stage` failed with permission denied and every restore would have failed. It goes under /tmp now, and the command is built as one string instead of concatenated pieces inside the argument list. Checked against a real volume and the published image: a truncated tar fails and leaves the volume exactly as it was, and a whole one restores it. --- docsgpt/deploy/docker.py | 14 +++++++++----- tests/deploy/test_docker.py | 3 ++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/docsgpt/deploy/docker.py b/docsgpt/deploy/docker.py index c44eb241..2b3dcdfa 100644 --- a/docsgpt/deploy/docker.py +++ b/docsgpt/deploy/docker.py @@ -117,13 +117,17 @@ class Docker: def import_volume(self, volume: str, source: Path, image: str) -> None: """Replace ``volume``'s contents with the tar at ``source``; the volume is created when missing.""" + # Unpack into the container's own filesystem first, so a truncated or corrupt tar fails + # before the live volume is touched rather than halfway through emptying it. It goes under + # /tmp because the image does not run as root and cannot write to /. + staging = "/tmp/docsgpt-restore" + script = ( + f"set -e; rm -rf {staging}; mkdir -p {staging}; tar xf - -C {staging}; " + f"find /data -mindepth 1 -delete; tar cf - -C {staging} . | tar xf - -C /data" + ) with source.open("rb") as handle: self._run( - ["docker", "run", "--rm", "-i", "-v", f"{volume}:/data", image, "sh", "-c", - # Unpack into the container's own filesystem first: a truncated or corrupt tar must - # fail before the live volume is touched, not halfway through emptying it. - "set -e; rm -rf /stage; mkdir /stage; tar xf - -C /stage; " - "find /data -mindepth 1 -delete; tar cf - -C /stage . | tar xf - -C /data"], + ["docker", "run", "--rm", "-i", "-v", f"{volume}:/data", image, "sh", "-c", script], stdin=handle, ) diff --git a/tests/deploy/test_docker.py b/tests/deploy/test_docker.py index 61c15dd8..0f1a3e8b 100644 --- a/tests/deploy/test_docker.py +++ b/tests/deploy/test_docker.py @@ -130,7 +130,8 @@ class TestVolumes: assert args[-2] == "-c" assert "tar xf - -C /data" in args[-1] command = args[-1] - assert command.index("tar xf - -C /stage") < command.index("find /data -mindepth 1 -delete"), ( + assert "mkdir -p /tmp/" in command, "the image does not run as root, so staging goes under /tmp" + assert command.index("tar xf - -C /tmp/") < command.index("find /data -mindepth 1 -delete"), ( "the incoming tar is unpacked outside the volume first, so a corrupt one leaves it alone" ) assert runner.streams[0][1] is not None, "the tar is fed in on stdin" From e269743bf878177249fd16b859d1a87affadc327 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 22:03:48 +0100 Subject: [PATCH 4/6] fix: start DocsGPT again when a restore fails after the stack is down Validating the archive catches a damaged one while DocsGPT is still up, but a well-formed archive can still hold a corrupt volume tar or a dump statement psql refuses, and those only surface once the stack is down. The work after the shutdown now runs inside an error boundary that starts the stack again before the failure is reported, so a failed restore never leaves the install stopped. --- docsgpt/deploy/commands.py | 56 ++++++++++++++++++++++--------------- tests/deploy/test_backup.py | 15 ++++++++++ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index b3bf7b28..fab64d45 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -419,6 +419,32 @@ def _write_backup(args, context: Context, directory: Path, env, target: Path, im backup_format.write_archive(target, dump=dump, volume_tars=volume_tars, manifest=manifest, settings=settings) +def _restore_data(context: Context, directory: Path, archive: Path, volumes: list, image: str) -> None: + """Put the archive's volumes and database back, with the stack stopped.""" + with tempfile.TemporaryDirectory() as workspace: + work = Path(workspace) + backup_format.extract(archive, work) + for name in volumes: + tar_path = work / backup_format.volume_member(name) + if not tar_path.is_file(): + raise DeployError(f"{archive} is missing the {name} volume it says it contains") + print(f"Restoring the {name} volume ...") + context.docker.import_volume(f"{PROJECT}_{name}", tar_path, image) + + dump = work / backup_format.DUMP + if not dump.is_file(): + raise DeployError(f"{archive} is missing its database dump") + print("Starting the database ...") + context.docker.compose(directory, "up", "-d", "--wait", "postgres") + print("Restoring the database ...") + with dump.open("r", encoding="utf-8") as handle: + context.docker.compose( + directory, "exec", "-T", "postgres", + "psql", "--quiet", "--set", "ON_ERROR_STOP=on", "-U", "docsgpt", "-d", "docsgpt", + stdin=handle, + ) + + def restore(args, context: Optional[Context] = None) -> int: """Put a backup's database and data volumes back over this install.""" context = context or Context.default(args) @@ -446,28 +472,14 @@ def restore(args, context: Optional[Context] = None) -> int: print("Stopping the stack ...") context.docker.compose(directory, *_EVERY_PROFILE, "down") - with tempfile.TemporaryDirectory() as workspace: - work = Path(workspace) - backup_format.extract(archive, work) - for name in volumes: - tar_path = work / backup_format.volume_member(name) - if not tar_path.is_file(): - raise DeployError(f"{archive} is missing the {name} volume it says it contains") - print(f"Restoring the {name} volume ...") - context.docker.import_volume(f"{PROJECT}_{name}", tar_path, image) - - dump = work / backup_format.DUMP - if not dump.is_file(): - raise DeployError(f"{archive} is missing its database dump") - print("Starting the database ...") - context.docker.compose(directory, "up", "-d", "--wait", "postgres") - print("Restoring the database ...") - with dump.open("r", encoding="utf-8") as handle: - context.docker.compose( - directory, "exec", "-T", "postgres", - "psql", "--quiet", "--set", "ON_ERROR_STOP=on", "-U", "docsgpt", "-d", "docsgpt", - stdin=handle, - ) + try: + _restore_data(context, directory, archive, volumes, image) + except BaseException: + # The stack is down by now. A corrupt payload inside an otherwise well-formed archive, or a + # statement psql refuses, must not leave DocsGPT stopped: start it again, then report. + print("The restore failed. Starting DocsGPT again ...", file=sys.stderr) + context.docker.compose(directory, "up", "-d", "--remove-orphans", check=False) + raise print("Starting DocsGPT ...") context.docker.compose(directory, "up", "-d", "--remove-orphans") diff --git a/tests/deploy/test_backup.py b/tests/deploy/test_backup.py index 371023a5..ee6d9889 100644 --- a/tests/deploy/test_backup.py +++ b/tests/deploy/test_backup.py @@ -198,6 +198,21 @@ class TestRestore: psql = next(args for _, args in docker.calls if "psql" in args) assert "ON_ERROR_STOP=on" in psql + def test_a_failure_after_the_stack_is_down_starts_it_again(self, tmp_path, capsys): + """A corrupt payload only shows up once the stack is down; it must not be left stopped.""" + archive = self._backup(tmp_path) + + class FailingImport(FakeDocker): + def import_volume(self, volume, source, image): + raise DeployError("tar: unexpected EOF in archive") + + docker = FailingImport(volumes={"docsgpt_postgres_data"}) + with pytest.raises(DeployError, match="unexpected EOF"): + _run(["restore", str(archive), "--dir", str(tmp_path), "--yes"], _context(docker)) + joined = [" ".join(args) for _, args in docker.calls] + assert any(call.startswith("up -d --remove-orphans") for call in joined), joined + assert "Starting DocsGPT again" in capsys.readouterr().err + def test_a_newer_backup_is_refused_without_force(self, tmp_path): """Restoring a 0.22 backup into 0.21 would hand an older schema newer data.""" newer = _copy_with_version(self._backup(tmp_path), "0.22.0", tmp_path / "newer.tar.gz") From 8cfa3fbd18405bb43ee79688aa88b50b367d9737 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 22:05:22 +0100 Subject: [PATCH 5/6] fix: let the container pick the staging directory for a restored volume A fixed path under /tmp was both a guess about what the image can write to and a temp-file smell that Bandit flags. The container makes the directory itself with mktemp -d and removes it afterwards. --- docsgpt/deploy/docker.py | 13 +++++++------ tests/deploy/test_docker.py | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docsgpt/deploy/docker.py b/docsgpt/deploy/docker.py index 2b3dcdfa..cba24375 100644 --- a/docsgpt/deploy/docker.py +++ b/docsgpt/deploy/docker.py @@ -117,13 +117,14 @@ class Docker: def import_volume(self, volume: str, source: Path, image: str) -> None: """Replace ``volume``'s contents with the tar at ``source``; the volume is created when missing.""" - # Unpack into the container's own filesystem first, so a truncated or corrupt tar fails - # before the live volume is touched rather than halfway through emptying it. It goes under - # /tmp because the image does not run as root and cannot write to /. - staging = "/tmp/docsgpt-restore" + # Unpack into a throwaway directory inside the container first, so a truncated or corrupt + # tar fails before the live volume is touched rather than halfway through emptying it. The + # container makes the directory itself: the image does not run as root, and a fixed path + # would be both a guess about what is writable and a temp-file smell. script = ( - f"set -e; rm -rf {staging}; mkdir -p {staging}; tar xf - -C {staging}; " - f"find /data -mindepth 1 -delete; tar cf - -C {staging} . | tar xf - -C /data" + 'set -e; stage=$(mktemp -d); tar xf - -C "$stage"; ' + 'find /data -mindepth 1 -delete; tar cf - -C "$stage" . | tar xf - -C /data; ' + 'rm -rf "$stage"' ) with source.open("rb") as handle: self._run( diff --git a/tests/deploy/test_docker.py b/tests/deploy/test_docker.py index 0f1a3e8b..f42d57c9 100644 --- a/tests/deploy/test_docker.py +++ b/tests/deploy/test_docker.py @@ -130,8 +130,8 @@ class TestVolumes: assert args[-2] == "-c" assert "tar xf - -C /data" in args[-1] command = args[-1] - assert "mkdir -p /tmp/" in command, "the image does not run as root, so staging goes under /tmp" - assert command.index("tar xf - -C /tmp/") < command.index("find /data -mindepth 1 -delete"), ( + assert "mktemp -d" in command, "the container picks the staging directory, not a fixed path" + assert command.index('tar xf - -C "$stage"') < command.index("find /data -mindepth 1 -delete"), ( "the incoming tar is unpacked outside the volume first, so a corrupt one leaves it alone" ) assert runner.streams[0][1] is not None, "the tar is fed in on stdin" From f90c442a41e743477d1ebd0ea94b84ba8a29b638 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 16 Sep 2026 22:20:40 +0100 Subject: [PATCH 6/6] fix: cover the shutdown calls and check every volume before replacing one Both shutdown calls sat outside the recovery that undoes them: a `compose down` that failed partway left the stack down, and a `compose stop` that failed left the backend and worker stopped. Each now runs inside its own try. A restore also replaced volumes one at a time, checking each tar as it reached it, so a damaged third payload was found with the first two already swapped in. Every declared tar is read through first, and the imports start only once they all come out whole. --- docsgpt/deploy/backup.py | 10 ++++++++ docsgpt/deploy/commands.py | 15 +++++++---- tests/deploy/test_backup.py | 50 +++++++++++++++++++++++++++++++++++-- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/docsgpt/deploy/backup.py b/docsgpt/deploy/backup.py index eb9dbfac..e823b9ee 100644 --- a/docsgpt/deploy/backup.py +++ b/docsgpt/deploy/backup.py @@ -113,6 +113,16 @@ def validate(path: Path, manifest: Mapping[str, object]) -> list[str]: return list(volumes) +def check_volume_tar(name: str, path: Path) -> None: + """Read a volume tar through, so a damaged one is found before any volume is replaced.""" + try: + with tarfile.open(path, "r:*") as archive: + for _ in archive: + pass + except (tarfile.TarError, OSError) as exc: + raise DeployError(f"the {name} volume in this backup is damaged: {exc}") from exc + + def extract(path: Path, destination: Path) -> None: """Unpack the archive into ``destination`` (data filter: no paths outside it, no devices).""" with tarfile.open(path, "r:gz") as archive: diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index fab64d45..5aa5fa42 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -371,9 +371,9 @@ def backup(args, context: Optional[Context] = None) -> int: target = out_dir / backup_format.archive_name(taken_at) image = _stack_image(env) - print("Pausing the backend and the worker so the database and the files match ...") - context.docker.compose(directory, "stop", "backend", "worker") try: + print("Pausing the backend and the worker so the database and the files match ...") + context.docker.compose(directory, "stop", "backend", "worker") _write_backup(args, context, directory, env, target, image, taken_at) finally: print("Starting the backend and the worker again ...") @@ -424,10 +424,16 @@ def _restore_data(context: Context, directory: Path, archive: Path, volumes: lis with tempfile.TemporaryDirectory() as workspace: work = Path(workspace) backup_format.extract(archive, work) + # Read every volume tar through before replacing any of them: a damaged third tar must not + # be discovered with the first two already swapped in. + tars = {} for name in volumes: tar_path = work / backup_format.volume_member(name) if not tar_path.is_file(): raise DeployError(f"{archive} is missing the {name} volume it says it contains") + backup_format.check_volume_tar(name, tar_path) + tars[name] = tar_path + for name, tar_path in tars.items(): print(f"Restoring the {name} volume ...") context.docker.import_volume(f"{PROJECT}_{name}", tar_path, image) @@ -469,10 +475,9 @@ def restore(args, context: Optional[Context] = None) -> int: return 1 image = _stack_image(env) - print("Stopping the stack ...") - context.docker.compose(directory, *_EVERY_PROFILE, "down") - try: + print("Stopping the stack ...") + context.docker.compose(directory, *_EVERY_PROFILE, "down") _restore_data(context, directory, archive, volumes, image) except BaseException: # The stack is down by now. A corrupt payload inside an otherwise well-formed archive, or a diff --git a/tests/deploy/test_backup.py b/tests/deploy/test_backup.py index ee6d9889..9c80237a 100644 --- a/tests/deploy/test_backup.py +++ b/tests/deploy/test_backup.py @@ -4,6 +4,7 @@ import io import json import stat import tarfile +from pathlib import Path import pytest @@ -27,13 +28,16 @@ def _manifest(path): return json.loads(archive.extractfile(backup_module.MANIFEST).read()) -def _rewrite(archive, dest, *, volumes=None, drop=()): - """The same archive with other volumes in its manifest, or with members left out.""" +def _rewrite(archive, dest, *, volumes=None, drop=(), corrupt=()): + """The same archive with other volumes in its manifest, or with members left out or damaged.""" with tarfile.open(archive, "r:gz") as source, tarfile.open(dest, "w:gz") as out: for member in source.getmembers(): if member.name in drop: continue body = source.extractfile(member).read() if member.isfile() else None + if member.name in corrupt: + body = body[:60] + member.size = len(body) if member.name == backup_module.MANIFEST and volumes is not None: manifest = json.loads(body.decode()) manifest["volumes"] = volumes @@ -117,6 +121,22 @@ class TestBackup: started = next(index for index, call in enumerate(joined) if call.startswith("up -d backend worker")) assert stopped < dumped < started, joined + def test_the_writers_come_back_even_when_stopping_them_fails(self, tmp_path): + """A stop that fails partway must not leave the backend or the worker down.""" + _installed(tmp_path) + + class FailingStop(FakeDocker): + def compose(self, directory, *args, **kwargs): + if args[:1] == ("stop",): + self.calls.append((Path(directory), list(args))) + raise DeployError("compose stop exploded") + return super().compose(directory, *args, **kwargs) + + docker = FailingStop() + with pytest.raises(DeployError, match="compose stop exploded"): + _run(["backup", "--dir", str(tmp_path), "--out", str(tmp_path / "b")], _context(docker)) + assert any(" ".join(args).startswith("up -d backend worker") for _, args in docker.calls), docker.calls + def test_the_writers_come_back_even_when_the_dump_fails(self, tmp_path): _installed(tmp_path) @@ -213,6 +233,32 @@ class TestRestore: assert any(call.startswith("up -d --remove-orphans") for call in joined), joined assert "Starting DocsGPT again" in capsys.readouterr().err + def test_a_shutdown_that_fails_still_starts_the_stack_again(self, tmp_path): + """`compose down` can fail with containers already stopped; the stack must not stay down.""" + archive = self._backup(tmp_path) + + class FailingDown(FakeDocker): + def compose(self, directory, *args, **kwargs): + if "down" in args: + self.calls.append((Path(directory), list(args))) + raise DeployError("compose down exploded") + return super().compose(directory, *args, **kwargs) + + docker = FailingDown(volumes={"docsgpt_postgres_data"}) + with pytest.raises(DeployError, match="compose down exploded"): + _run(["restore", str(archive), "--dir", str(tmp_path), "--yes"], _context(docker)) + joined = [" ".join(args) for _, args in docker.calls] + assert any(call.startswith("up -d --remove-orphans") for call in joined), joined + + def test_a_damaged_volume_tar_is_found_before_any_volume_is_replaced(self, tmp_path): + """vectors sorts last, so a per-volume check would already have swapped indexes and inputs.""" + archive = _rewrite(self._backup(tmp_path), tmp_path / "bad-volume.tar.gz", + corrupt=(backup_module.volume_member("vectors"),)) + docker = FakeDocker(volumes={"docsgpt_postgres_data"}) + with pytest.raises(DeployError, match="damaged"): + _run(["restore", str(archive), "--dir", str(tmp_path), "--yes"], _context(docker)) + assert [op for op in docker.volume_ops if op[0] == "import"] == [], docker.volume_ops + def test_a_newer_backup_is_refused_without_force(self, tmp_path): """Restoring a 0.22 backup into 0.21 would hand an older schema newer data.""" newer = _copy_with_version(self._backup(tmp_path), "0.22.0", tmp_path / "newer.tar.gz")