= sshexport code audit
:toc:
// spellcheck: add argv CLI INI
// spellcheck: add namespace pexpect pylint
// spellcheck: add SELinux SHA SSH's StrictHostKeyChecking subprocess
// spellcheck: add unmanaged suppressions

This report audits the sshexport source as of 2026-10-14. The existing test
suite passes (46 tests), as do the project's `make check` checks. Findings below
are ordered by severity. Generated HTML and release archives were not reviewed.

== High severity

=== Unknown SSH host keys are accepted automatically

The pexpect loop recognizes the SSH first-connection prompt and sends `yes`
without displaying or asking the user to verify the fingerprint
(`sshexport:103-126`). This removes SSH's normal trust-on-first-use verification
step and can establish trust in an attacker-controlled host.

Suggested fix::
Allow SSH to present the prompt to the user. For unattended operation, provide
an explicit option that selects `StrictHostKeyChecking=accept-new`, documenting
the resulting trust model.

== Medium severity

=== Key comments can overwrite reserved configuration fields

`KeyConfig.remember()` uses an arbitrary public-key comment as an INI option
name (`sshexport:256-280`). Comments named `port` or `password` collide with the
connection settings. For example, a key whose comment is `port` can record
`port = ssh-ed25519`; the next invocation then rejects that value as a malformed
port.

Suggested fix::
Identify keys by a SHA-256 fingerprint derived from the decoded public-key blob.
Use a reserved namespace such as `key.<fingerprint>` and store display comments
as values rather than option names.

== Structural and test improvements

* Move the top-level CLI body into `main(argv)` and return an integer status.
  This would allow complete workflows to be tested without subprocess fixtures.
* Split the internal executor into text-output and status-returning methods, or
  add precise type overloads. Its current conditional return type causes the
  false-positive pylint diagnostics at the output membership and `strip()`
  calls.
* Add integration tests for preservation of unmanaged keys, deletion without
  local keys, SELinux deletion, first-use host-key handling, reserved key
  comments, and interrupted configuration writes.
* Remove the duplicate `too-many-statements` entry and correct the joined
  `missing-function-docstring.no-else-break` token in the module-level pylint
  directive (`sshexport:43`). Then narrow suppressions to individual legacy
  constructs where practical.