Marco Ricci commited on 2025-12-25 12:32:59
Zeige 1 geänderte Dateien mit 79 Einfügungen und 21 Löschungen.
Introduce a proper formal type for SSH agent spawn handlers in the test configuration (as a type-checked named tuple). This alone increases readability by removing the tuple indices (magic numbers) from the code. Also use this opportunity to introduce a real label and an explicit key for each entry, instead of (ab)using the executable name for these purposes. The executable name is no longer usable as a unique key if the SSH agent behaves differently (with respect to spawning) on different operating systems... as is the case for both PuTTY/Pageant and OpenSSH.
| ... | ... |
@@ -19,6 +19,7 @@ from typing import TYPE_CHECKING, Protocol, TypeVar |
| 19 | 19 |
import hypothesis |
| 20 | 20 |
import packaging.version |
| 21 | 21 |
import pytest |
| 22 |
+from typing_extensions import NamedTuple |
|
| 22 | 23 |
|
| 23 | 24 |
import tests.data |
| 24 | 25 |
import tests.data.callables |
| ... | ... |
@@ -295,28 +296,67 @@ def spawn_noop( # pragma: no cover [unused] |
| 295 | 296 |
"""Placeholder function. Does nothing.""" |
| 296 | 297 |
|
| 297 | 298 |
|
| 298 |
-spawn_handlers: dict[str, tuple[str, SpawnFunc, tests.data.KnownSSHAgent]] = {
|
|
| 299 |
- "pageant": ( |
|
| 299 |
+class SpawnHandler(NamedTuple): |
|
| 300 |
+ """A handler for a spawned SSH agent. |
|
| 301 |
+ |
|
| 302 |
+ Attributes: |
|
| 303 |
+ key: |
|
| 304 |
+ The key under which this handler is registered. |
|
| 305 |
+ executable: |
|
| 306 |
+ The (optional) full path to the executable. |
|
| 307 |
+ spawn_func: |
|
| 308 |
+ The spawn function. |
|
| 309 |
+ agent_type: |
|
| 310 |
+ The type of the spawned SSH agent. |
|
| 311 |
+ agent_name: |
|
| 312 |
+ The (optional) display name for this handler. |
|
| 313 |
+ |
|
| 314 |
+ """ |
|
| 315 |
+ |
|
| 316 |
+ key: str |
|
| 317 |
+ executable: str | None |
|
| 318 |
+ spawn_func: SpawnFunc |
|
| 319 |
+ agent_type: tests.data.KnownSSHAgent |
|
| 320 |
+ agent_name: str | None |
|
| 321 |
+ |
|
| 322 |
+ |
|
| 323 |
+spawn_handlers: dict[str, SpawnHandler] = {
|
|
| 324 |
+ "pageant": SpawnHandler( |
|
| 300 | 325 |
"pageant", |
| 326 |
+ None, |
|
| 301 | 327 |
spawn_pageant_on_posix, |
| 302 | 328 |
tests.data.KnownSSHAgent.Pageant, |
| 329 |
+ "Pageant (UNIX)", |
|
| 303 | 330 |
), |
| 304 |
- "ssh-agent": ( |
|
| 331 |
+ "ssh-agent": SpawnHandler( |
|
| 305 | 332 |
"ssh-agent", |
| 333 |
+ None, |
|
| 306 | 334 |
spawn_openssh_agent_on_posix, |
| 307 | 335 |
tests.data.KnownSSHAgent.OpenSSHAgent, |
| 336 |
+ "ssh-agent (OpenSSH)", |
|
| 308 | 337 |
), |
| 309 | 338 |
"stub_agent_with_address": ( |
| 310 | 339 |
"stub_agent_with_address", |
| 340 |
+ None, |
|
| 311 | 341 |
spawn_noop, |
| 312 | 342 |
tests.data.KnownSSHAgent.StubbedSSHAgent, |
| 343 |
+ "stub_agent_with_address (derivepassphrase test suite)", |
|
| 313 | 344 |
), |
| 314 |
- "stub_agent_with_address_and_deterministic_dsa": ( |
|
| 345 |
+ "stub_agent_with_address_and_deterministic_dsa": SpawnHandler( |
|
| 315 | 346 |
"stub_agent_with_address_and_deterministic_dsa", |
| 347 |
+ None, |
|
| 316 | 348 |
spawn_noop, |
| 317 | 349 |
tests.data.KnownSSHAgent.StubbedSSHAgent, |
| 350 |
+ "stub_agent_with_address_and_deterministic_dsa " |
|
| 351 |
+ "(derivepassphrase test suite)", |
|
| 352 |
+ ), |
|
| 353 |
+ "(system)": SpawnHandler( |
|
| 354 |
+ "(system)", |
|
| 355 |
+ None, |
|
| 356 |
+ spawn_noop, |
|
| 357 |
+ tests.data.KnownSSHAgent.UNKNOWN, |
|
| 358 |
+ None, |
|
| 318 | 359 |
), |
| 319 |
- "(system)": ("(system)", spawn_noop, tests.data.KnownSSHAgent.UNKNOWN),
|
|
| 320 | 360 |
} |
| 321 | 361 |
""" |
| 322 | 362 |
The standard registry of agent spawning functions. |
| ... | ... |
@@ -351,9 +391,10 @@ class CannotSpawnError(RuntimeError): |
| 351 | 391 |
|
| 352 | 392 |
|
| 353 | 393 |
def spawn_named_agent( |
| 354 |
- exec_name: str, |
|
| 394 |
+ executable: str | None, |
|
| 355 | 395 |
spawn_func: SpawnFunc, |
| 356 | 396 |
agent_type: tests.data.KnownSSHAgent, |
| 397 |
+ agent_name: str, |
|
| 357 | 398 |
) -> Iterator[tests.data.SpawnedSSHAgentInfo]: # pragma: no cover [external] |
| 358 | 399 |
"""Spawn the named SSH agent and check that it is operational. |
| 359 | 400 |
|
| ... | ... |
@@ -369,12 +410,16 @@ def spawn_named_agent( |
| 369 | 410 |
there. |
| 370 | 411 |
|
| 371 | 412 |
Args: |
| 372 |
- exec_name: |
|
| 373 |
- The executable to spawn. |
|
| 413 |
+ executable: |
|
| 414 |
+ The full path of the executable to spawn. If not given, we |
|
| 415 |
+ attempt to spawn the agent under its conventional executable |
|
| 416 |
+ name. |
|
| 374 | 417 |
spawn_func: |
| 375 | 418 |
The agent-specific spawn function. |
| 376 | 419 |
agent_type: |
| 377 | 420 |
The agent type. |
| 421 |
+ agent_name: |
|
| 422 |
+ The agent's display name. |
|
| 378 | 423 |
|
| 379 | 424 |
Yields: |
| 380 | 425 |
A 3-tuple containing the agent type, an SSH agent client |
| ... | ... |
@@ -402,12 +447,17 @@ def spawn_named_agent( |
| 402 | 447 |
# Here, we verify at most major steps that SSH_AUTH_SOCK didn't |
| 403 | 448 |
# change under our nose. |
| 404 | 449 |
assert os.environ.get("SSH_AUTH_SOCK") == startup_ssh_auth_sock, (
|
| 405 |
- f"SSH_AUTH_SOCK mismatch when checking for spawnable {exec_name}"
|
|
| 450 |
+ f"SSH_AUTH_SOCK mismatch when checking for spawnable {agent_name}"
|
|
| 406 | 451 |
) |
| 407 | 452 |
exit_stack = contextlib.ExitStack() |
| 408 | 453 |
agent_env = os.environ.copy() |
| 409 | 454 |
ssh_auth_sock = agent_env.pop("SSH_AUTH_SOCK", None)
|
| 410 |
- proc = spawn_func(executable=shutil.which(exec_name), env=agent_env) |
|
| 455 |
+ proc = spawn_func( |
|
| 456 |
+ executable=shutil.which(executable) |
|
| 457 |
+ if executable is not None |
|
| 458 |
+ else None, |
|
| 459 |
+ env=agent_env, |
|
| 460 |
+ ) |
|
| 411 | 461 |
with exit_stack: |
| 412 | 462 |
if ( |
| 413 | 463 |
spawn_func is spawn_noop |
| ... | ... |
@@ -417,12 +467,12 @@ def spawn_named_agent( |
| 417 | 467 |
elif spawn_func is spawn_noop: |
| 418 | 468 |
ssh_auth_sock = os.environ["SSH_AUTH_SOCK"] |
| 419 | 469 |
elif proc is None: # pragma: no cover [external] |
| 420 |
- err_msg = f"Cannot spawn usable {exec_name}"
|
|
| 470 |
+ err_msg = f"Cannot spawn usable {agent_name}"
|
|
| 421 | 471 |
raise CannotSpawnError(err_msg) |
| 422 | 472 |
else: |
| 423 | 473 |
exit_stack.enter_context(terminate_on_exit(proc)) |
| 424 | 474 |
assert os.environ.get("SSH_AUTH_SOCK") == startup_ssh_auth_sock, (
|
| 425 |
- f"SSH_AUTH_SOCK mismatch after spawning {exec_name}"
|
|
| 475 |
+ f"SSH_AUTH_SOCK mismatch after spawning {agent_name}"
|
|
| 426 | 476 |
) |
| 427 | 477 |
assert proc.stdout is not None |
| 428 | 478 |
ssh_auth_sock_line = proc.stdout.readline() |
| ... | ... |
@@ -455,13 +505,13 @@ def spawn_named_agent( |
| 455 | 505 |
ssh_agent.SSHAgentClient, |
| 456 | 506 |
"SOCKET_PROVIDERS", |
| 457 | 507 |
["stub_agent_with_address_and_deterministic_dsa"] |
| 458 |
- if exec_name == "stub_agent_with_address_and_deterministic_dsa" |
|
| 508 |
+ if "stub_agent_with_address_and_deterministic_dsa" in agent_name |
|
| 459 | 509 |
else ["stub_agent_with_address"], |
| 460 | 510 |
) |
| 461 | 511 |
client = exit_stack.enter_context( |
| 462 | 512 |
ssh_agent.SSHAgentClient.ensure_agent_subcontext( |
| 463 | 513 |
tests.machinery.StubbedSSHAgentSocketWithAddressAndDeterministicDSA() |
| 464 |
- if exec_name == "stub_agent_with_address_and_deterministic_dsa" |
|
| 514 |
+ if "stub_agent_with_address_and_deterministic_dsa" in agent_name |
|
| 465 | 515 |
else tests.machinery.StubbedSSHAgentSocketWithAddress() |
| 466 | 516 |
) |
| 467 | 517 |
) |
| ... | ... |
@@ -491,7 +541,7 @@ def spawn_named_agent( |
| 491 | 541 |
agent_type, client, spawn_func is not spawn_noop |
| 492 | 542 |
) |
| 493 | 543 |
assert os.environ.get("SSH_AUTH_SOCK", None) == startup_ssh_auth_sock, (
|
| 494 |
- f"SSH_AUTH_SOCK mismatch after tearing down {exec_name}"
|
|
| 544 |
+ f"SSH_AUTH_SOCK mismatch after tearing down {agent_name}"
|
|
| 495 | 545 |
) |
| 496 | 546 |
|
| 497 | 547 |
|
| ... | ... |
@@ -532,7 +582,7 @@ skip marks.) Used by some test fixtures. |
| 532 | 582 |
for key, handler in spawn_handlers.items(): |
| 533 | 583 |
marks = [ |
| 534 | 584 |
pytest.mark.skipif( |
| 535 |
- not is_agent_permitted(handler[2]), |
|
| 585 |
+ not is_agent_permitted(handler.agent_type), |
|
| 536 | 586 |
reason="SSH agent excluded via PERMITTED_AGENTS " |
| 537 | 587 |
"environment variable.", |
| 538 | 588 |
), |
| ... | ... |
@@ -607,14 +657,17 @@ def running_ssh_agent( # pragma: no cover [external] |
| 607 | 657 |
monkeypatch.setenv("SSH_AUTH_SOCK", startup_ssh_auth_sock)
|
| 608 | 658 |
else: |
| 609 | 659 |
monkeypatch.delenv("SSH_AUTH_SOCK", raising=False)
|
| 610 |
- for exec_name, spawn_func, agent_type in spawn_handlers.values(): |
|
| 611 |
- if not is_agent_permitted(agent_type): |
|
| 660 |
+ for handler in spawn_handlers.values(): |
|
| 661 |
+ if not is_agent_permitted(handler.agent_type): |
|
| 612 | 662 |
continue |
| 613 | 663 |
try: |
| 614 | 664 |
for _agent_info in spawn_named_agent( |
| 615 |
- exec_name, spawn_func, agent_type |
|
| 665 |
+ executable=handler.executable, |
|
| 666 |
+ spawn_func=handler.spawn_func, |
|
| 667 |
+ agent_type=handler.agent_type, |
|
| 668 |
+ agent_name=handler.agent_name or handler.key, |
|
| 616 | 669 |
): |
| 617 |
- yield from prepare_environment(agent_type) |
|
| 670 |
+ yield from prepare_environment(handler.agent_type) |
|
| 618 | 671 |
except (KeyError, OSError, CannotSpawnError): |
| 619 | 672 |
continue |
| 620 | 673 |
return |
| ... | ... |
@@ -657,7 +710,12 @@ def spawn_ssh_agent( |
| 657 | 710 |
else: # pragma: no cover [external] |
| 658 | 711 |
monkeypatch.delenv("SSH_AUTH_SOCK", raising=False)
|
| 659 | 712 |
try: |
| 660 |
- yield from spawn_named_agent(*request.param) |
|
| 713 |
+ yield from spawn_named_agent( |
|
| 714 |
+ executable=request.param.executable, |
|
| 715 |
+ spawn_func=request.param.spawn_func, |
|
| 716 |
+ agent_type=request.param.agent_type, |
|
| 717 |
+ agent_name=request.param.agent_name or request.param.key, |
|
| 718 |
+ ) |
|
| 661 | 719 |
except KeyError as exc: |
| 662 | 720 |
pytest.skip( |
| 663 | 721 |
f"The environment variable {exc.args[0]!r} was not found"
|
| 664 | 722 |