let Ctrl-C / SIGINT abort interactive prompts (y/n and passphrase), fixes #8521 - #10209
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10209 +/- ##
==========================================
+ Coverage 87.24% 87.29% +0.04%
==========================================
Files 102 102
Lines 18429 18444 +15
Branches 2832 2834 +2
==========================================
+ Hits 16079 16101 +22
+ Misses 1643 1638 -5
+ Partials 707 705 -2 ☔ View full report in Codecov by Harness. |
93807c4 to
fb519c4
Compare
|
Updated together with #10208: the test now uses Measurements and the full analysis are in #10208 (comment) — the fix itself was verified to work on NetBSD end to end. Master tests here: 538 passed. |
|
Extended to the passphrase prompt (second commit), same as #10208, because it has exactly the same problem. BeforeMeasured with a real pty, an encrypted repo and no
Cause is the same as for the y/n questions: AfterBoth cases exit with rc 130. Terminal echo is not left disabled -
|
| case | result |
|---|---|
| repokey, first prompt | rc 3 "Cancelled by user.", repo directory gone, keys directory gone |
| repokey, "Enter same passphrase again" | rc 3 "Cancelled by user.", repo directory gone, keys directory gone |
| keyfile, first prompt | rc 3 "Cancelled by user.", repo directory gone, keys directory gone |
So no half-written key file and no orphaned repokey blob.
Tests
test_getpass_sigint_aborts- sends the signal from inside the patchedgetpass, expectsKeyboardInterrupt. Verified it fails when the fix is reverted.test_getpass_restores_sigint_handler- the prompt must not change SIGINT handling of what runs afterwards.- The tests now use their own
SigIntManager()instance instead of the globalsig_int. The global one can only be entered once per process (its__exit__drops the context), so a second test using it would break as soon as two such tests run in the same worker.
Full runs: testsuite/helpers/ 540 passed. The y/n end-to-end check still aborts with rc 130.
Not included here
borg key import --paperreads its lines with plaininput()too (crypto/keymanager.py), so Ctrl-C is swallowed there as well. Happy to cover it in the same PR if you want it.- Unrelated pre-existing bug found while testing this: at the passphrase prompt, EOF (e.g.
borg repo-create < /dev/nullwithoutBORG_PASSPHRASE) is turned intoNoPassphraseFailurebyPassphrase.getpass(), which is not in theexcept (EOFError, KeyboardInterrupt)list above - sorepository.destroy()is skipped and a half-created repository stays behind (A repository already exists ...on retry,Repository has no manifest.on use). Same on 1.4-maint. Tell me if you want an issue for that.
While a command runs, borg installs a SIGINT handler (SigIntManager) that only remembers that Ctrl-C was pressed, so that a running operation like borg create can still be finished in an orderly way. That handler is also active while borg waits for input from the user - so a Ctrl-C, or a SIGINT sent by a frontend, was just remembered while input() / getpass() kept waiting. While waiting for an answer or a passphrase there is nothing to finish, so temporarily install the raising SIGINT handler around the input call - for the y/n questions in yes() and for the passphrase prompt. Both abort right away with rc 130 now, which is what an interactive user expects and what frontends like Pika Backup need for their abort feature. Terminal echo is not left disabled: getpass() restores the termios settings in a finally clause, also when the read raises. borg repo-create keeps cleaning up after itself: an interrupt at the "Enter new passphrase" prompt runs into the existing `except (EOFError, KeyboardInterrupt): repository.destroy()`, so neither a repository directory nor a key file is left behind (verified for the repokey and the keyfile case, at the first and at the confirmation prompt). The tests use their own SigIntManager instance rather than the global sig_int, which can only be entered once per process. They send the signal with signal.raise_signal(): os.kill() would send it to the process and some kernels (e.g. NetBSD) then deliver it to another thread if there is one - the main thread would only run the handler later, after the prompt restored the previous handler, and the test would flap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3f90f5b to
29022db
Compare
Forward port of #10208 to master. Fixes #8521.
While a command runs,
main()wraps it inwith sig_int:(archiver/init.py:647), soSigIntManager.handleris installed for the whole run. That handler deliberately does not raise — it only remembers that Ctrl-C was pressed, so that a running operation likeborg createcan still finish the archive in an orderly way.That same handler is also active while borg waits for an answer to a y/n question, so a Ctrl-C — or a SIGINT sent by a frontend such as Pika Backup — was just remembered while
input()kept waiting.While waiting for an answer there is nothing to finish, so this temporarily installs the raising SIGINT handler around the
input()call and restores the previous one afterwards.Same change as #10208, adapted to
helpers/yes_no.pyandtestsuite/helpers/yes_no_test.py.Verification (on this branch, not just on 1.4-maint)
Real borg process, prompt from
borg check --repair, a single SIGINT sent to it:borg check --repair, one SIGINT at the prompttest_yes_sigint_aborts(new) — sends SIGINT to itself from inside the input function whilesig_intis active, as it is during a real command, and expectsKeyboardInterrupt. Verified it fails (DID NOT RAISE) without the fix.test_yes_restores_sigint_handler(new) — the question must not change SIGINT handling for whatever runs after it.src/borg/testsuite/helpers/passes completely: 538 tests.check_cmd_test.py,delete_cmd_test.py,repo_delete_cmd_test.py: 35 passed, 38 skipped.Notes
getpassand is not touched here.prompt=False,env_var_override, EOF) are unchanged — the handler is installed only around the actualinput()call.🤖 Generated with Claude Code