Skip to content

maintenance: extend SFTP connection options - #4293

Open
zqr10159 wants to merge 1 commit into
apache:masterfrom
zqr10159:maintenance/sftp-connection-options
Open

maintenance: extend SFTP connection options#4293
zqr10159 wants to merge 1 commit into
apache:masterfrom
zqr10159:maintenance/sftp-connection-options

Conversation

@zqr10159

@zqr10159 zqr10159 commented Jul 30, 2026

Copy link
Copy Markdown
Member

Summary

  • Pin SFTP server identity to one or more configured SHA-256 host-key fingerprints.
  • Accept a current and next fingerprint together so operators can rotate host keys without an outage.
  • Keep pre-upgrade SFTP monitors running by explicitly migrating only legacy records with no host-key policy to a temporary compatibility flag.
  • Emit controlled warnings for that migration and for every connection that still skips verification.
  • Mark the unsafe UI option as dangerous and conditional on SFTP, and document trusted fingerprint acquisition, upgrade, and rotation in English and Chinese.

Regression proof

The rotation contract was first run against the previous PR head. It rejected the second verified key because the implementation treated the entire multi-line value as one fingerprint. The legacy-upgrade contract also had no implementation on the previous head. Both outcome paths pass with this revision.

Validation

  • Focused protocol, verifier, and migration contracts: 15 tests passed (FtpProtocolTest, FtpCollectImplTest, and SftpHostKeyCompatibilityMigrationTest).
  • YamlCheckScript: all monitor definitions parsed successfully.
  • ./mvnw -pl hertzbeat-startup -am -DskipTests package: all 24 reactor modules passed and produced the source-built startup artifact.
  • Checkstyle and git diff --check: passed.
  • Touched Java and test files contain no hard-coded CJK text; localized YAML and documentation intentionally contain locale content.

Upgrade behavior

Existing ftp monitors with SFTP enabled and neither a fingerprint nor an explicit verification policy receive insecureSkipVerify=true once, before scheduler initialization. This preserves collection but produces warnings until the operator verifies fingerprints, adds them, disables the unsafe option, and runs detection. Pinned and previously migrated monitors are not changed. New monitors and newly imported configurations remain fail-closed unless the operator explicitly selects the dangerous temporary option.

ssh-keyscan is documented only as a way to retrieve a key, not as proof of identity. Operators must compare the resulting fingerprint through a trusted channel.

AI assistance: used for draft implementation and test iteration.
Human validation: the old-head rotation contract failed; focused outcome tests, YAML parsing, Checkstyle, whitespace checks, and the 24-module startup package proof passed.
Risk notes: upgraded monitors remain temporarily unauthenticated until an operator completes the documented pinning step; warnings make this compatibility state visible.

@zqr10159
zqr10159 force-pushed the maintenance/sftp-connection-options branch from f5318a0 to 6a36885 Compare July 30, 2026 15:58
@zqr10159 zqr10159 changed the title Add SFTP connection options maintenance: extend SFTP connection options Jul 30, 2026
@zqr10159

Copy link
Copy Markdown
Member Author

Author remediation update:

The rollout path now preserves existing SFTP monitors through an explicit one-time compatibility migration instead of making them fail immediately. New configurations still fail closed unless they provide a trusted fingerprint or explicitly select the dangerous compatibility option. Multiple fingerprints are accepted for current and next key rotation, unsafe connections emit controlled warnings, and the UI plus English and Chinese docs explain trusted acquisition, upgrade, rotation, and exit steps.

Protocol, verifier, and migration tests passed (15), monitor YAML parsing passed, and the 24-module startup source package passed. GitHub checks are currently queued. Maintainer review remains required.

@github-actions github-actions Bot added the doc Improvements or additions to documentation label Jul 30, 2026
@zqr10159

Copy link
Copy Markdown
Member Author

CI follow-up: the only failing check after the remediation is DOC CI Dead Link Check. It was rerun once and failed again on the same unchanged historical URL, https://gitee.com/hertzbeat/hertzbeat, returning HTTP 405. The changed SFTP documentation passed Markdown and filename checks and adds no failing external link. Backend build, Maven E2E, image E2E, license, and label checks all passed. I am leaving the unrelated global link configuration and historical pages out of this single-purpose PR; the DOC CI result should be retried after the external endpoint or repository-wide link policy is addressed.

@zqr10159

Copy link
Copy Markdown
Member Author

DOC CI has now passed on the unchanged current head (6a368854b0). The clean rerun completed Markdown, filename, dead-link, dependency-install, and documentation-build steps successfully. The earlier Gitee 405 was transient; no unrelated link-check policy change was needed. Together with the already green backend build, Maven E2E, image E2E, license, and label checks, this removes the remaining CI blocker.

@zqr10159
zqr10159 marked this pull request as ready for review July 31, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend collector doc Improvements or additions to documentation monitoring-template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant