Warn the user upon supplying an empty service name
Marco Ricci commited on 2024-10-15 12:47:49
Zeige 2 geänderte Dateien mit 71 Einfügungen und 0 Löschungen.
The `derivepassphrase` command-line largely, and vault(1) completely, treats an empty service name the same as if no service name had been supplied, switching over to global operation instead of service-specific operation (or erroring out in case a service must be given). For `derivepassphrase`, this is mostly a user interface issue – the underlying machinery supports empty service names –, but kept for compatibility with vault(1). However, this is very easy to diagnose, and the user would benefit from seeing a warning about a seemingly omitted service name. So do that, and issue a warning upon encountering an empty service name on the command-line or in an imported configuration. (The warning message changes slightly in each case.) Also, add explicit tests for these two scenarios that trigger the warning.
- src/derivepassphrase/cli.py 5a879b6..16f0fe8
- tests/test_derivepassphrase_cli.py a8c6c4c..00b7573
| ... | ... |
@@ -1358,6 +1358,17 @@ def derivepassphrase_vault( # noqa: C901,PLR0912,PLR0913,PLR0914,PLR0915 |
| 1358 | 1358 |
msg = f'{opt_str} does not take a SERVICE argument'
|
| 1359 | 1359 |
raise click.UsageError(msg) |
| 1360 | 1360 |
|
| 1361 |
+ if service == '': # noqa: PLC1901 |
|
| 1362 |
+ click.echo( |
|
| 1363 |
+ ( |
|
| 1364 |
+ f'{PROG_NAME}: Warning: An empty SERVICE is not '
|
|
| 1365 |
+ f'supported by vault(1). For compatibility, this will be ' |
|
| 1366 |
+ f'treated as if SERVICE was not supplied, i.e., it will ' |
|
| 1367 |
+ f'error out, or operate on global settings.' |
|
| 1368 |
+ ), |
|
| 1369 |
+ err=True, |
|
| 1370 |
+ ) |
|
| 1371 |
+ |
|
| 1361 | 1372 |
if edit_notes: |
| 1362 | 1373 |
assert service is not None |
| 1363 | 1374 |
configuration = get_config() |
| ... | ... |
@@ -1429,6 +1440,15 @@ def derivepassphrase_vault( # noqa: C901,PLR0912,PLR0913,PLR0914,PLR0915 |
| 1429 | 1440 |
f'{json.dumps(step.old_value)}.'
|
| 1430 | 1441 |
) |
| 1431 | 1442 |
click.echo(err_msg, err=True) |
| 1443 |
+ if '' in maybe_config['services']: |
|
| 1444 |
+ err_msg = ( |
|
| 1445 |
+ f'{PROG_NAME}: Warning: An empty SERVICE is not '
|
|
| 1446 |
+ f'supported by vault(1), and the empty-string service ' |
|
| 1447 |
+ f'settings will be inaccessible and ineffective. ' |
|
| 1448 |
+ f'To ensure that vault(1) and {PROG_NAME} see the settings, '
|
|
| 1449 |
+ f'move them into the "global" section.' |
|
| 1450 |
+ ) |
|
| 1451 |
+ click.echo(err_msg, err=True) |
|
| 1432 | 1452 |
form = cast( |
| 1433 | 1453 |
Literal['NFC', 'NFD', 'NFKC', 'NFKD'], |
| 1434 | 1454 |
maybe_config.get('global', {}).get(
|
| ... | ... |
@@ -621,6 +621,57 @@ class TestCLI: |
| 621 | 621 |
result = tests.ReadableResult.parse(_result) |
| 622 | 622 |
assert result.clean_exit(empty_stderr=True), 'expected clean exit' |
| 623 | 623 |
|
| 624 |
+ def test_211a_empty_service_name_causes_warning( |
|
| 625 |
+ self, |
|
| 626 |
+ monkeypatch: pytest.MonkeyPatch, |
|
| 627 |
+ ) -> None: |
|
| 628 |
+ def expected_warning_line(line: str) -> bool: |
|
| 629 |
+ return is_harmless_config_import_warning_line(line) or ( |
|
| 630 |
+ ' Warning: An empty SERVICE is not supported by vault(1)' |
|
| 631 |
+ in line |
|
| 632 |
+ ) |
|
| 633 |
+ |
|
| 634 |
+ monkeypatch.setattr(cli, '_prompt_for_passphrase', tests.auto_prompt) |
|
| 635 |
+ runner = click.testing.CliRunner(mix_stderr=False) |
|
| 636 |
+ with tests.isolated_vault_config( |
|
| 637 |
+ monkeypatch=monkeypatch, |
|
| 638 |
+ runner=runner, |
|
| 639 |
+ config={'services': {}},
|
|
| 640 |
+ ): |
|
| 641 |
+ _result = runner.invoke( |
|
| 642 |
+ cli.derivepassphrase_vault, |
|
| 643 |
+ ['--config', '--length=30', '--', ''], |
|
| 644 |
+ catch_exceptions=False, |
|
| 645 |
+ ) |
|
| 646 |
+ result = tests.ReadableResult.parse(_result) |
|
| 647 |
+ assert result.clean_exit(empty_stderr=False), 'expected clean exit' |
|
| 648 |
+ assert result.stderr is not None, 'expected known error output' |
|
| 649 |
+ assert all( |
|
| 650 |
+ expected_warning_line(line) |
|
| 651 |
+ for line in result.stderr.splitlines(False) |
|
| 652 |
+ ), 'expected known error output' |
|
| 653 |
+ assert cli._load_config() == {
|
|
| 654 |
+ 'global': {'length': 30},
|
|
| 655 |
+ 'services': {},
|
|
| 656 |
+ }, 'requested configuration change was not applied' |
|
| 657 |
+ _result = runner.invoke( |
|
| 658 |
+ cli.derivepassphrase_vault, |
|
| 659 |
+ ['--import', '-'], |
|
| 660 |
+ input=json.dumps({'services': {'': {'length': 40}}}),
|
|
| 661 |
+ catch_exceptions=False, |
|
| 662 |
+ ) |
|
| 663 |
+ result = tests.ReadableResult.parse(_result) |
|
| 664 |
+ assert result.clean_exit(empty_stderr=False), 'expected clean exit' |
|
| 665 |
+ assert result.stderr is not None, 'expected known error output' |
|
| 666 |
+ assert all( |
|
| 667 |
+ expected_warning_line(line) |
|
| 668 |
+ for line in result.stderr.splitlines(False) |
|
| 669 |
+ ), 'expected known error output' |
|
| 670 |
+ assert cli._load_config() == {
|
|
| 671 |
+ 'global': {'length': 30},
|
|
| 672 |
+ 'services': {'': {'length': 40}},
|
|
| 673 |
+ }, 'requested configuration change was not applied' |
|
| 674 |
+ |
|
| 624 | 675 |
@pytest.mark.parametrize( |
| 625 | 676 |
['options', 'service'], |
| 626 | 677 |
[ |
| 627 | 678 |