let Ctrl-C / SIGINT abort interactive prompts (y/n and passphrase), fixes #8521 - #10208
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.4-maint #10208 +/- ##
=============================================
+ Coverage 82.06% 82.14% +0.07%
=============================================
Files 38 38
Lines 11471 11475 +4
Branches 1807 1807
=============================================
+ Hits 9414 9426 +12
+ Misses 1470 1464 -6
+ Partials 587 585 -2 ☔ View full report in Codecov by Harness. |
c610529 to
fbfb168
Compare
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 an answer to a y/n question - so a Ctrl-C, or a SIGINT sent by a frontend, was just remembered while input() kept waiting. While waiting for an answer there is nothing to finish, so temporarily install the raising SIGINT handler around the input() call. Ctrl-C / SIGINT now aborts right away (rc 130), which is what an interactive user expects and what frontends like Pika Backup need for their abort feature. Forward port of borgbackup#10208 (1.4-maint). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Extended to the passphrase prompt (second commit), 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/key.py 64 passed, testsuite/helpers.py 146 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>
8da0274 to
92c7f07
Compare
Fixes #8521.
While a command runs,
main()wraps it inwith sig_int:, 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, was just remembered while
input()kept waiting — which is what @sophie-h reported for Pika Backup's abort feature (they currently work around it by also writing"\n"to borg's stdin).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.Verification
Real borg process, prompt from
borg check --repair, a single SIGINT sent to it:borg check --repair, one SIGINT at the promptTwo tests added next to the other
yes()tests:test_yes_sigint_aborts— sends SIGINT to itself from inside the input function, whilesig_intis active as it is during a real command, and expectsKeyboardInterrupt. Verified that it fails (DID NOT RAISE) without the fix.test_yes_restores_sigint_handler— the question must not change SIGINT handling for whatever runs after it.src/borg/testsuite/helpers.pypasses completely (146 tests), and the prompt-related archiver tests (-k "repair or delete or unencrypted or check_usage or i_know") pass: 29 passed, 14 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