sneak/vaultik
1
0
Fork 0
Code Issues 20 Pull Requests 3 Actions Packages Projects Releases Wiki Activity

Ctrl-C during restore exits before cleanup, leaving decrypted data in the temp directory #159

New Issue
Open
opened 2026-09-22 00:55:14 +02:00 by clawbot · 1 comment
clawbot commented 2026-09-22 00:55:14 +02:00
Collaborator
Copy Link
Copy Source

Found by the security review #73. Severity: medium (decrypted backup content and metadata left on disk, mode 0600, on the machine that holds the private key). Line numbers are as of next at 6fcd8e1.

What is wrong

RunOperation (internal/cli/app.go:222-276) runs each command in a goroutine started from an fx OnStart hook. Its OnStop hook only calls v.Cancel() and returns (:253-257). On SIGINT or SIGTERM, RunApp calls app.Stop and returns as soon as that completes (:162-185), and the process exits. Nothing waits for the operation goroutine.

The removal of decrypted data lives in defers inside Restore (internal/vaultik/restore.go:99-109, :248-254): the blob cache $TMPDIR/vaultik-blobcache-*, which holds full decrypted blob plaintext, and $TMPDIR/vaultik-restore-*.db. They run only if the goroutine happens to unwind before exit. writeFileChunks (restore.go:923-964) never looks at the context, so one large file keeps writing after the cancel. The same applies to vaultik-verify-*.db from snapshot verify --deep and to the plaintext index copy vaultik-snapshot-*/snapshot.db made during snapshot create.

Acceptable

  • OnStop cancels the operation and then waits for the operation goroutine to return, bounded by the context OnStop is given (the existing shutdownTimeout), so the existing defers run before the process exits.
  • writeFileChunks checks the context between chunks, and the blob download into the cache stops on cancel, so the wait ends promptly.
  • The exit status of an interrupted run is unchanged; RunOperation documents that an interrupt is not a failure.
  • The cases that cannot be cleaned (kill -9, power loss) and the names of these files are written down for users; that text belongs to #171, not this issue.

Definition of done

  1. Test through the stop path (RunOperation or the fx OnStop hook), not by calling Vaultik.Restore directly (that passes today, because the defers run whenever Restore returns): stop during a restore that is mid-download; assert stop does not return before the operation goroutine has, and that no vaultik-blobcache-* directory and no vaultik-restore-*.db remain.
  2. No existing assertion weakened; make check green.

model: fable-5-1

Found by the security review https://git.eeqj.de/sneak/vaultik/issues/73. Severity: **medium** (decrypted backup content and metadata left on disk, mode 0600, on the machine that holds the private key). Line numbers are as of `next` at `6fcd8e1`. ## What is wrong `RunOperation` (`internal/cli/app.go:222-276`) runs each command in a goroutine started from an fx `OnStart` hook. Its `OnStop` hook only calls `v.Cancel()` and returns (`:253-257`). On SIGINT or SIGTERM, `RunApp` calls `app.Stop` and returns as soon as that completes (`:162-185`), and the process exits. Nothing waits for the operation goroutine. The removal of decrypted data lives in defers inside `Restore` (`internal/vaultik/restore.go:99-109`, `:248-254`): the blob cache `$TMPDIR/vaultik-blobcache-*`, which holds full decrypted blob plaintext, and `$TMPDIR/vaultik-restore-*.db`. They run only if the goroutine happens to unwind before exit. `writeFileChunks` (`restore.go:923-964`) never looks at the context, so one large file keeps writing after the cancel. The same applies to `vaultik-verify-*.db` from `snapshot verify --deep` and to the plaintext index copy `vaultik-snapshot-*/snapshot.db` made during `snapshot create`. ## Acceptable - `OnStop` cancels the operation and then waits for the operation goroutine to return, bounded by the context `OnStop` is given (the existing `shutdownTimeout`), so the existing defers run before the process exits. - `writeFileChunks` checks the context between chunks, and the blob download into the cache stops on cancel, so the wait ends promptly. - The exit status of an interrupted run is unchanged; `RunOperation` documents that an interrupt is not a failure. - The cases that cannot be cleaned (kill -9, power loss) and the names of these files are written down for users; that text belongs to https://git.eeqj.de/sneak/vaultik/issues/171, not this issue. ## Definition of done 1. Test through the stop path (`RunOperation` or the fx `OnStop` hook), not by calling `Vaultik.Restore` directly (that passes today, because the defers run whenever `Restore` returns): stop during a restore that is mid-download; assert stop does not return before the operation goroutine has, and that no `vaultik-blobcache-*` directory and no `vaultik-restore-*.db` remain. 2. No existing assertion weakened; `make check` green. model: fable-5-1
clawbot commented 2026-09-22 13:10:00 +02:00
Author
Collaborator
Copy Link
Copy Source

Fixed in #189.

On interrupt, the shared runner (RunOperation) now cancels the operation and waits for its goroutine to unwind before the process exits, so restore's cleanup runs and the decrypted blob cache (vaultik-blobcache-*) and snapshot-database directory are removed. The wait is bounded by the existing shutdownTimeout. Because the fix is in the shared runner, it covers every command, not only restore. Restore's chunk-write and blob-download loops also check the context between steps so the wait ends promptly. Exit status is unchanged; an interrupt is a cancellation, not a failure.

The regression test drives a real restore through the stop path, catches it mid-download, and asserts no scratch remains after the stop.

One note: the decrypted snapshot database is now a directory (vaultik-restore-*), not a vaultik-restore-*.db file, since the fix for #162; the test asserts on the directory.

Model: opus-4-8

Fixed in https://git.eeqj.de/sneak/vaultik/pulls/189. On interrupt, the shared runner (`RunOperation`) now cancels the operation and waits for its goroutine to unwind before the process exits, so restore's cleanup runs and the decrypted blob cache (`vaultik-blobcache-*`) and snapshot-database directory are removed. The wait is bounded by the existing `shutdownTimeout`. Because the fix is in the shared runner, it covers every command, not only restore. Restore's chunk-write and blob-download loops also check the context between steps so the wait ends promptly. Exit status is unchanged; an interrupt is a cancellation, not a failure. The regression test drives a real restore through the stop path, catches it mid-download, and asserts no scratch remains after the stop. One note: the decrypted snapshot database is now a directory (`vaultik-restore-*`), not a `vaultik-restore-*.db` file, since the fix for https://git.eeqj.de/sneak/vaultik/issues/162; the test asserts on the directory. Model: opus-4-8
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#159