Skip to content

Fix SCP confirmation and endpoint validation; enforce SFTP - #3

Merged
nivaisman merged 1 commit into
mainfrom
fix/scp-security-validation
Sep 9, 2026
Merged

nivaisman merged 1 commit into
mainfrom
fix/scp-security-validation

Conversation

@nivaisman

Copy link
Copy Markdown
Owner

Summary

  • Fix uppercase Y selecting Download after the user chose Upload.
  • Validate local/remote endpoint types, usernames, hosts, paths, and input completion before any transfer.
  • Require explicit absolute local paths so host:path, URIs, and raw Windows drive paths cannot silently become a second SSH endpoint.
  • Explicitly select SFTP with scp -s --; never retry with legacy SCP.
  • Display the operation and both endpoints before confirmation, warn about destination overwrites, and preserve SCP failure status.
  • Add 24 no-network regression tests and a GitHub Actions workflow with read-only permissions, disabled credential persistence, and a commit-pinned checkout action.
  • Document supported runtimes, input restrictions, and remaining operational limitations.

Validation

  • bash -n Shell-scripts/scp.sh: passed.
  • shellcheck --severity=style Shell-scripts/scp.sh: passed with ShellCheck 0.11.0.
  • python3 -m unittest discover -s tests -v: all 24 tests passed locally.
  • git diff --check: passed.
  • Separate local OpenSSH parser verification confirmed explicit -s requests the SFTP subsystem, using a fake SSH program that never connects.
  • No live remote transfer or remote command was performed.
  • GitHub Actions results will be checked after publication; local success does not imply hosted CI has already passed.

Compatibility and scope

  • Local runtimes: Linux, macOS, or Windows through WSL using POSIX OpenSSH. Automated coverage is Linux-only; live macOS/WSL/remote-Windows transfers remain unverified.
  • Native Windows PowerShell/CMD, Git Bash, and Cygwin are explicitly unsupported instead of guessing at their path semantics.
  • Local paths must begin with /; use /mnt/c/... for Windows files in WSL.
  • Remote Windows paths retain forward-slash drive syntax such as C:/Users/me/file.txt.
  • Remote wildcards/backslashes, domain-qualified usernames, and inline ports are unsupported. Trusted SSH configuration may specify ports/jump hosts.
  • Existing destination files can still be overwritten after confirmation. The wrapper is for trusted interactive input, not a privilege boundary for untrusted automation.
  • GitHub secret scanning, push protection, and branch-protection settings are unchanged. Enable those controls separately and require the new CI check after its first successful run.

This pull request targets main but does not merge or modify that branch.

@nivaisman nivaisman left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review complete

@nivaisman nivaisman self-assigned this Sep 9, 2026
@nivaisman
nivaisman merged commit bf23b2d into main Sep 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant