Skip to content

Refine recursive mode design: type conflicts, cycle detection, delete semantics #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
messa wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refine recursive mode design: type conflicts, cycle detection, delete semantics #11

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 42 additions & 13 deletions add_recursive_plan_v2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Tento dokument popisuje návrh implementace volby `--recursive` pro nástroj blo

1. **Jednosměrný tok dat** - zachován stávající model: checksum → retrieve → save
2. **Přenos pouze rozdílů** - přečtou se a zahashují všechny soubory na obou stranách, přenesou se jen změněné bloky
3. **Deterministické pořadí** - položky se zpracovávají v abecedním pořadí relativních cest (lexikografické řazení UTF-8 bytů), což zajišťuje reprodukovatelné chování
3. **Deterministické pořadí** - checksum odesílá položky v abecedním pořadí relativních cest (lexikografické řazení UTF-8 bytů). Retrieve odpovídá ve stejném pořadí, pak přidává nové položky ze source (abecedně), a nakonec příkazy `remv`. Toto zajišťuje reprodukovatelné chování a správné pořadí vytváření (rodiče před potomky).

## Návrh protokolu

Expand Down Expand Up @@ -245,17 +245,19 @@ done
c. Pro každý `'file'` z dest:
- Přidat cestu do `seen_paths`
- Zkontrolovat on-demand, jestli soubor existuje na source
- Pokud existuje: porovnat hashe, poslat `'file'` + změněné datové bloky + `'meta'`
- Pokud existuje jako soubor: porovnat hashe, poslat `'file'` + změněné datové bloky + `'meta'`
- Pokud existuje jako adresář: vypsat warning "type changed: file → dir", přeskočit
- Pokud neexistuje na source: přidat cestu do `to_delete`
Comment on lines +248 to 250
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The algorithm for handling type conflicts appears incomplete. When a file on dest exists as a directory on source (line 249), the item is skipped but it's unclear whether the path is added to seen_paths. If it's not added to seen_paths, then step (e) at line 257-259 would later send the directory and all its children as new items when walking the source. However, save cannot create these because the conflicting file still exists on dest. The algorithm should clarify that type-conflicted paths should be added to seen_paths to prevent attempting to send the source version, and should also skip all descendants of type-conflicted directories to avoid attempting impossible operations.

Copilot uses AI. Check for mistakes.
d. Pro každý `'dire'` z dest:
- Přidat cestu do `seen_paths`
- Zkontrolovat on-demand, jestli adresář existuje na source
- Pokud existuje: poslat `'dire'` + `'dmet'`
- Pokud existuje jako adresář: poslat `'dire'` + `'dmet'`
- Pokud existuje jako soubor: vypsat warning "type changed: dir → file", přeskočit
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

Similar to the file→dir conflict, when a directory on dest exists as a file on source (line 255), the algorithm should clarify whether the path is added to seen_paths. If not added, step (e) at line 257-259 would send the file as a new item, but save cannot create it because the conflicting directory still exists on dest. Type-conflicted paths should be added to seen_paths to prevent retransmission.

Suggested change
- Pokud existuje jako soubor: vypsat warning "type changed: dir → file", přeskočit
- Pokud existuje jako soubor: vypsat warning "type changed: dir → file", cesta zůstává v `seen_paths` (nesmí být znovu poslána jako nová položka ve kroku (e)), přeskočit

Copilot uses AI. Check for mistakes.
- Pokud neexistuje na source: přidat cestu do `to_delete`
e. Po `'done'` ze stdin:
- Projít source adresář (`followlinks=True`)
- Pro každou položku která není v `seen_paths`: poslat jako novou
- Pokud `--delete`: poslat `'remv'` pro položky v `to_delete`
- Poslat `'remv'` pro položky v `to_delete` (save bez `--delete` je ignoruje)
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The algorithm specifies sending remv commands for items in to_delete but doesn't specify the order. Section "Pořadí mazání (--delete)" at line 407 states that remv commands should be sorted in descending order by path depth (children before parents) to enable proper deletion. The algorithm should clarify that remv commands are sent in descending depth order, or that they use recursive deletion.

Suggested change
- Poslat `'remv'` pro položky v `to_delete` (save bez `--delete` je ignoruje)
- Seřadit položky v `to_delete` podle hloubky cesty sestupně (potomci před rodiči, při stejné hloubce např. abecedně podle cesty)
- Poslat `'remv'` pro položky v `to_delete` v tomto pořadí (save bez `--delete` je ignoruje)

Copilot uses AI. Check for mistakes.
f. Odeslat `'done'`
2. Jinak (původní chování):
- Zpracovat jako dosud
Expand All @@ -275,7 +277,8 @@ done
- Truncate na velikost z `'meta'.total_size`
- Aplikovat metadata z `'meta'`
- Pro každý `'remv'`:
- Smazat soubor nebo adresář (rekurzivně)
- Pokud `--delete`: smazat soubor nebo adresář (rekurzivně)
- Bez `--delete`: ignorovat (položka zůstane na dest)
c. Po `'done'`:
- Aplikovat metadata adresářů z `pending_dir_metadata` (v opačném pořadí - potomci před rodiči)
d. Skončit
Expand All @@ -302,7 +305,7 @@ Smazat soubory na dest, které neexistují na source:
blockcopy save --recursive --delete /path/to/dest_dir
```

**Výchozí chování**: bez `--delete` se soubory na dest zachovávají.
**Výchozí chování**: bez `--delete` se příkazy `remv` od retrieve ignorují a soubory na dest zůstávají zachovány. Retrieve vždy posílá `remv` příkazy pro položky chybějící na source.

### Volba `--exclude` (budoucí rozšíření)

Expand Down Expand Up @@ -336,6 +339,8 @@ Symlinky se následují (followlinks=True při os.walk). Toto znamená:
- Symlink na adresář: rekurzivně se projde cílový adresář
- Broken symlink: přeskočí se s varováním na stderr (cíl neexistuje)

**Detekce cyklů:** Při followlinks=True může dojít k cyklům (např. `/a/b/link → /a`). Funkce `walk_directory()` sleduje navštívené adresáře pomocí `(st_dev, st_ino)` a přeskočí již navštívené s varováním na stderr.

Výhody tohoto přístupu:
- Jednodušší protokol (není potřeba FTYPE_SYMLINK)
- Destination dostane skutečná data, ne závislost na lokálních cestách
Expand All @@ -345,8 +350,11 @@ Výhody tohoto přístupu:

Pokud se typ položky změní (soubor → adresář nebo adresář → soubor):

1. Poslat příkaz `remv` pro starou položku
2. Poslat příkaz `file` nebo `dire` pro novou položku (s daty/metadaty)
- Vypsat varování na stderr (logging warning)
- Položku přeskočit (nepřenášet ani nemazat)
- Uživatel musí ručně vyřešit konflikt na destination

**Zdůvodnění:** Automatická změna typu (smazání + vytvoření nového) by mohla vést k neočekávané ztrátě dat. Explicitní varování a manuální zásah je bezpečnější.

### Soubory s nulovou velikostí

Expand Down Expand Up @@ -433,6 +441,7 @@ def walk_directory(path: Path, followlinks: bool = True) -> Iterator[Tuple[Path,

Položky jsou seřazené abecedně podle cesty.
Symlinky se následují (followlinks=True).
Detekuje cykly pomocí (st_dev, st_ino) a přeskočí navštívené adresáře.
Comment on lines 443 to +444
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The design of walk_directory() with followlinks=True and always following symlinks ("Symlinky se následují (followlinks=True)") introduces a path traversal / data exfiltration risk: an unprivileged user can place a symlink inside the synced tree pointing to any path readable by the blockcopy process (e.g., /etc/shadow), and the recursive sync will read and transfer the target contents under the symlink path. In multi-user or partially untrusted source directories, this lets less-privileged users cause the tool (often running with elevated privileges) to leak arbitrary files outside the intended source root. To mitigate this, avoid following symlinks by default for recursive mode (treat them as symlinks or skip them), or make symlink following an explicit opt-in with strict checks that the resolved target remains within the allowed source root and does not cross sensitive filesystem boundaries.

Copilot uses AI. Check for mistakes.
"""

def send_file_header(out: IO, path: str, size: int):
Expand Down Expand Up @@ -770,12 +779,25 @@ Původní návrh obsahoval FLAG_NEW, FLAG_MODIFIED, FLAG_UNCHANGED pro indikaci

Toto zjednodušuje protokol a eliminuje redundanci.

### Umístění `--delete` na retrieve (ne na save)
### Umístění `--delete` na save

Volba `--delete` určuje, zda se mají mazat položky chybějící na source. Save je správné místo, protože:
- Uživatel typicky sedí u destination serveru a spouští pipeline odtud
- Uživatel má kontrolu nad tím, co se děje s jeho destination adresářem
- Retrieve vždy posílá příkazy `remv` pro položky k smazání
- Save bez `--delete` příkazy `remv` ignoruje
- Save s `--delete` příkazy `remv` vykoná

### Metadata se posílají vždy

I pro nezměněné soubory (kde se nepřenášejí datové bloky) se posílá příkaz `meta` s metadaty. Důvody:

Volba `--delete` určuje, zda se mají mazat položky chybějící na source. Retrieve je správné místo, protože:
- Retrieve zná stav source adresáře
- Retrieve generuje příkazy `remv` pro položky k smazání
- Save pouze vykonává přijaté příkazy
- Umožňuje aktualizovat permissions/timestamps i bez změny obsahu souboru
- Uživatel může chtít `save --times --perms` pro synchronizaci metadat
- Jednodušší logika (není potřeba porovnávat metadata na obou stranách)
- Bandwidth overhead je akceptovatelný (~60-70 bytes na soubor)

Pro 1M souborů je overhead ~60 MB, což je zanedbatelné oproti typickým přenosům VM images nebo databází.

### On-demand kontrola source místo manifestu

Expand All @@ -802,6 +824,11 @@ Symlinky se nepřenášejí jako symlinky, ale jako jejich cílový obsah:

### Rozlišení recursive vs. single-file režimu

Blockcopy má dva rovnocenné režimy (oba jsou "first-class citizens"):

1. **Single-file režim** - primární use case pro kopírování LVM volumes, VM images, block devices
2. **Recursive režim** - pro kopírování adresářových stromů (databázové datadiry apod.)

Receiver může rozlišit režim podle prvního příkazu:
- `'dire'` → recursive režim (první příkaz je vždy root adresář s `path_len=0`)
- `'Hash'` → single-file režim
Expand All @@ -811,6 +838,8 @@ Toto umožňuje:
- Root adresář je legitimní položka - jeho metadata se také přenesou
- I prázdný dest adresář pošle `'dire'` s prázdnou cestou, takže recursive režim je vždy rozpoznatelný

**Poznámka:** Recursive režim není "nová verze" protokolu - je to alternativní režim. Oba režimy budou podporovány dlouhodobě.

## Budoucí rozšíření (mimo scope tohoto plánu)

1. `--exclude` / `--include` vzory
Expand Down
Loading