fix: apply nft batches in the selected namespace, complete the swap rollback [P0]
Two release blockers, the first a regression introduced by dd5be47.
1. The anti-torrent batch was applied on the host.
Making the rebuild atomic moved it from run_step(transport, ..) to run_nft_batch(script),
and run_nft_batch spawned `nft` directly with Command::new. An nft transaction belongs to
the network namespace of the process that runs nft, so with the production default
apply_in_amnezia_netns = true the sequence became: inspect inside the container, install
the table on the host, verify inside the container, fail. The guard was silently
ineffective, a stray table accumulated on the host, and the next reconcile could fail on
`add table` because the host copy already existed.
NsTransport gains run_with_stdin, implemented for all three transports - host, nsenter,
and docker exec with -i, which is required or the child sees EOF immediately.
run_nft_batch now takes the transport, and both callers pass the one they inspected
with. A recording fake transport asserts the script goes through the transport rather
than to a host process, which is the regression this could not previously catch.
2. Swap replacement rolled back with the wrong priority and not from every branch.
The rollback reactivated with desired.priority - the value that may be exactly what the
kernel had just rejected - because the previously active priority was dropped at the
match arm and never passed in. The active entry's priority is now carried into
replace_active_swapfile and used by the rollback.
Three early failure branches also returned without undoing anything: a failed swapoff of
the original, a failed rename to .previous, and a failed swapoff of the staging file all
left the staging swap active and the transition half-applied, after which the next
reconcile tried to unlink a file that was still active and could not proceed.
The transition now records what it completed - staging_active, original_active,
original_moved, new_installed - and one rollback undoes exactly that much, in an order
that never leaves the host without swap: restore the original file, reactivate it at its
previous priority, confirm it, and only then release and remove the staging swap. The
returned error carries the original cause plus what the recovery achieved.
Leftovers from a killed process are no longer removed blindly. A .new or .previous that
is still active cannot be unlinked, so it is reported with the swapoff command that
clears it instead of failing the same way on every reconcile.
Also in this commit, from the same audit:
- Requires=docker.service dropped in favour of Wants=. The daemon is built to outlive
Docker being down, but Requires= had systemd stop the harness whenever Docker stopped,
and Restart=on-failure does not cover a dependency-ordered stop - so the reconnect loop
never got to run.
- apply_antitorrent_disabled swallowed every discover_target error, not just "no container
running". Docker being unreachable or a permission error on the socket reported success
for a cleanup that never happened. Only AppError::NotFound is tolerated now.
- FstabTransaction::begin asserts /etc/fstab is a regular non-symlink file. read_to_string
and metadata both follow links, so a symlinked fstab would be read through and then
replaced by the rename, detaching the real file from the path the system boots from.
- append_event serialises its read-modify-write behind a mutex; the reconcile loop and the
Docker events thread both append and could drop an entry.
- tool_exists loses the fallocate, chmod and dd arms, dead since mkswap --file replaced the
allocation chain.
Docs corrected where they no longer matched the code: findmnt is described as mandatory
rather than skippable, the swap creation wording is mkswap --file, and the README package
examples use 0.1.6 to match Cargo.toml. Remaining fallocate mentions are explicitly
historical ("the previous chain"), which is accurate.
Seven tests added: batch routed through the given transport, batch failure reported as
such, rollback state defaults and previous-priority carry, active leftovers reported rather
than deleted, inactive leftovers cleared.
cargo fmt --check, clippy -D warnings, cargo test --locked --workspace --release: 188 + 4
passed.
This commit is contained in:
@@ -649,7 +649,7 @@ inode = 123456
|
||||
- нет пробелов, управляющих символов и обратных слэшей;
|
||||
- путь равен `/swapfile` **либо** лежит непосредственно в `/var/lib/awg2-harness/` (без вложенных каталогов).
|
||||
|
||||
Это allow-list, а не deny-list, и это принципиально. Список запрещённых путей закрывает только те случаи, которые кто-то придумал; значение имеет другое — может ли родительский каталог писать кто-то кроме root. Между preflight-проверкой и запуском `fallocate`/`dd`/`mkswap` непривилегированный пользователь с правом записи в родительский каталог успевает подменить финальный компонент на symlink и перенаправить операцию, выполняемую от root. Это классический TOCTOU, и никакое количество `stat` перед использованием его не закрывает — окно устраняется только тем, что запись в родительский каталог доступна лишь root.
|
||||
Это allow-list, а не deny-list, и это принципиально. Список запрещённых путей закрывает только те случаи, которые кто-то придумал; значение имеет другое — может ли родительский каталог писать кто-то кроме root. Между preflight-проверкой и запуском `mkswap --file` непривилегированный пользователь с правом записи в родительский каталог успевает подменить финальный компонент на symlink и перенаправить операцию, выполняемую от root. Это классический TOCTOU, и никакое количество `stat` перед использованием его не закрывает — окно устраняется только тем, что запись в родительский каталог доступна лишь root.
|
||||
|
||||
Оба разрешённых корня root-owned. `/swapfile` оставлен, чтобы уже развёрнутые конфигурации не требовали правки.
|
||||
|
||||
@@ -664,7 +664,7 @@ inode = 123456
|
||||
- резервная копия обязательна. Если её не удалось записать, целевой файл не трогается вовсе — ситуация, в которой копия не пишется, и есть та, в которой она нужна;
|
||||
- копий две: `/etc/fstab.awg2-harness.bak` перезаписывается перед каждой правкой и содержит состояние до неё, а `/etc/fstab.awg2-harness.original` создаётся один раз через `create_new` и хранит содержимое до первого вмешательства пакета;
|
||||
- запись идёт во временный файл рядом с целевым, с `create_new`, `sync_all` и переносом прав с оригинала; устаревший временный файл от убитого процесса удаляется явно, а не переиспользуется;
|
||||
- кандидат проверяется через `findmnt --verify --tab-file` до подмены. Если `findmnt` недоступен, шаг пропускается; если он отработал и отверг файл — операция прекращается;
|
||||
- кандидат проверяется через `findmnt --verify --tab-file` до подмены. Проверка обязательна: `findmnt` входит в `util-linux`, от которого пакет и так зависит, и включён в обязательные host tools при `swap.enabled = true`. Невозможность его запустить и отказ файла одинаково прекращают операцию — молча пропускать единственную структурную проверку файла, от которого зависит загрузка хоста, недопустимо;
|
||||
- подмена — атомарный `rename`, после него каталог синхронизируется;
|
||||
- после `rename` содержимое перечитывается и сверяется с записанным. Расхождение приводит к восстановлению из резервной копии.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user