|
| 1 | +# GitHub Copilot Instructions for SSH.NET |
| 2 | + |
| 3 | +## What this repository does |
| 4 | + |
| 5 | +SSH.NET is a .NET library for SSH-2 protocol communication, optimized for parallelism. It provides: |
| 6 | + |
| 7 | +- **SSH command execution** (synchronous and async) |
| 8 | +- **SFTP** file operations (synchronous and async) |
| 9 | +- **SCP** file transfers |
| 10 | +- **Port forwarding** (local, remote, dynamic/SOCKS) |
| 11 | +- **Interactive shell** via `ShellStream` |
| 12 | +- **NetConf** protocol client |
| 13 | +- **Multi-factor and certificate-based authentication** |
| 14 | + |
| 15 | +Primary entry points are `SshClient`, `SftpClient`, `ScpClient`, and `NetConfClient`, all extending `BaseClient`. |
| 16 | + |
| 17 | +--- |
| 18 | + |
| 19 | +## Core technologies |
| 20 | + |
| 21 | +| Area | Technology | |
| 22 | +|---|---| |
| 23 | +| Language | C# (`LangVersion=latest`) with `#nullable enable` everywhere | |
| 24 | +| Runtimes | .NET Framework 4.6.2, .NET Standard 2.0, .NET 8+, .NET 9+, .NET 10+ | |
| 25 | +| Cryptography | BouncyCastle (`BouncyCastle.Cryptography`) | |
| 26 | +| Logging | `Microsoft.Extensions.Logging.Abstractions` (`ILogger`/`ILoggerFactory`) | |
| 27 | +| Testing | MSTest v4, Moq, Testcontainers (Docker for integration tests) | |
| 28 | +| Build tooling | .NET SDK 10.0.100, Nerdbank.GitVersioning | |
| 29 | +| Static analysis | StyleCop.Analyzers, Meziantou.Analyzer, SonarAnalyzer.CSharp | |
| 30 | + |
| 31 | +--- |
| 32 | + |
| 33 | +## Code organization |
| 34 | + |
| 35 | +``` |
| 36 | +src/Renci.SshNet/ |
| 37 | +├── Channels/ # SSH channel types (session, forwarded, X11…) |
| 38 | +├── Common/ # Shared utilities, extension methods, custom exceptions |
| 39 | +├── Compression/ # Zlib compression support |
| 40 | +├── Connection/ # Socket connectors, proxy support (SOCKS4/5, HTTP), key exchange |
| 41 | +├── Messages/ # SSH protocol message types |
| 42 | +│ ├── Transport/ |
| 43 | +│ ├── Authentication/ |
| 44 | +│ └── Connection/ |
| 45 | +├── Security/ # Algorithms, key-exchange, cryptography wrappers |
| 46 | +│ └── Cryptography/ # Ciphers, signatures, key types |
| 47 | +├── Sftp/ # SFTP session, requests, responses |
| 48 | +├── Netconf/ # NetConf client |
| 49 | +└── Abstractions/ # Platform and I/O abstractions |
| 50 | +``` |
| 51 | + |
| 52 | +Large classes are split into **partial class files** per concern (e.g., `PrivateKeyFile.PKCS1.cs`, `PrivateKeyFile.OpenSSH.cs`). |
| 53 | + |
| 54 | +--- |
| 55 | + |
| 56 | +## Naming and style conventions |
| 57 | + |
| 58 | +| Element | Convention | Example | |
| 59 | +|---|---|---| |
| 60 | +| Classes, methods, properties | PascalCase | `SftpClient`, `ListDirectory()`, `IsConnected` | |
| 61 | +| Private fields | `_camelCase` | `_isDisposed`, `_sftpSession` | |
| 62 | +| Interfaces | `I` prefix + PascalCase | `ISftpClient`, `IChannel` | |
| 63 | +| Constants / static readonly | PascalCase | `MaximumSshPacketSize` | |
| 64 | +| Local variables | camelCase | `operationTimeout`, `connectionInfo` | |
| 65 | + |
| 66 | +**StyleCop is enforced.** Follow existing file conventions: |
| 67 | + |
| 68 | +- `#nullable enable` at the top of every `.cs` file. |
| 69 | +- `using` directives **outside** the namespace block, grouped with `System.*` first, then a blank line, then other namespaces. |
| 70 | +- 4-space indentation (spaces, not tabs). |
| 71 | +- XML doc comments (`///`) on all public and internal members; use `<inheritdoc/>` when inheriting from an interface. |
| 72 | +- Describe exception conditions in `/// <exception cref="…">` tags. |
| 73 | +- No Hungarian notation. |
| 74 | + |
| 75 | +--- |
| 76 | + |
| 77 | +## Error handling |
| 78 | + |
| 79 | +Use the existing exception hierarchy; do not throw `Exception` or `ApplicationException` directly. |
| 80 | + |
| 81 | +``` |
| 82 | +SshException |
| 83 | +├── SshConnectionException # connection-level failures |
| 84 | +├── SshAuthenticationException # auth failures |
| 85 | +├── SshOperationTimeoutException # operation timed out |
| 86 | +├── SshPassPhraseNullOrEmptyException |
| 87 | +├── ProxyException |
| 88 | +├── SftpException |
| 89 | +│ ├── SftpPermissionDeniedException |
| 90 | +│ └── SftpPathNotFoundException |
| 91 | +├── ScpException |
| 92 | +└── NetConfServerException |
| 93 | +``` |
| 94 | + |
| 95 | +- Annotate new exception classes with `#if NETFRAMEWORK [Serializable] #endif` and add the protected serialization constructor inside the same `#if` block, matching the pattern in `SshException.cs`. |
| 96 | +- Surface async errors by storing them in a `TaskCompletionSource` or re-throwing via `ExceptionDispatchInfo.Throw()` — never swallow exceptions silently. |
| 97 | +- Raise `ErrorOccurred` events on long-running components (e.g., `ForwardedPort`, `Session`) rather than propagating the exception across thread boundaries. |
| 98 | + |
| 99 | +--- |
| 100 | + |
| 101 | +## API design |
| 102 | + |
| 103 | +- **Every public blocking method should have a `…Async(CancellationToken cancellationToken = default)` counterpart.** Keep both in the same partial class file or co-located partial file. |
| 104 | +- Validate all arguments at the top of public methods; prefer `ArgumentNullException`, `ArgumentException`, `ArgumentOutOfRangeException` with descriptive messages. |
| 105 | +- Return `IEnumerable<T>` for sequences that are already materialized; use `IAsyncEnumerable<T>` when data streams asynchronously. |
| 106 | +- Prefer `ReadOnlyMemory<byte>` / `Memory<byte>` over `byte[]` in new protocol-layer code. |
| 107 | +- Do not expose mutable collections directly; use `.AsReadOnly()` or copy-on-return. |
| 108 | + |
| 109 | +--- |
| 110 | + |
| 111 | +## Async and concurrency |
| 112 | + |
| 113 | +- Use `async Task` / `async ValueTask` with `CancellationToken` for all new async methods. |
| 114 | +- The socket receive loop and keep-alive timer run on dedicated background threads; protect shared state with `lock` or the custom internal `Lock` type used in `Session`. |
| 115 | +- Prefer `TaskCompletionSource<T>` to bridge event-driven or callback-based code into the async model. |
| 116 | +- Never block inside async code with `.Result` or `.Wait()` — this can cause deadlocks on synchronization-context-bound callers. |
| 117 | +- Use `ConfigureAwait(false)` in library code (this is a class library, not an app). |
| 118 | + |
| 119 | +--- |
| 120 | + |
| 121 | +## Security-sensitive areas |
| 122 | + |
| 123 | +These areas require extra care: |
| 124 | + |
| 125 | +- **`src/Renci.SshNet/Security/`** — key exchange, algorithm negotiation. Algorithm choices have direct security implications. |
| 126 | +- **`src/Renci.SshNet/PrivateKeyFile*.cs`** — key format parsing (PKCS#1, PKCS#8, OpenSSH, PuTTY, ssh.com). Input is untrusted; validate lengths and offsets before indexing. |
| 127 | +- **`src/Renci.SshNet/Connection/`** — host key verification. Never bypass host key checking silently. |
| 128 | +- **Sensitive data in memory**: clear key material as soon as it is no longer needed; do not log private key bytes or plaintext passwords even at `Debug` level. |
| 129 | +- **`SshNetLoggingConfiguration.WiresharkKeyLogFilePath`** is a Debug-only diagnostic that writes session keys to disk. It must never be enabled in production builds. |
| 130 | +- **Cryptographic primitives** come from BouncyCastle. Prefer existing wrappers over direct calls to BouncyCastle APIs; adding new algorithms requires corresponding unit tests and algorithm negotiation entries. |
| 131 | + |
| 132 | +--- |
| 133 | + |
| 134 | +## Testing |
| 135 | + |
| 136 | +### Unit tests (`test/Renci.SshNet.Tests/`) |
| 137 | + |
| 138 | +- Framework: **MSTest** (`[TestClass]`, `[TestMethod]`, `[TestInitialize]`, `[TestCleanup]`) |
| 139 | +- Mocking: **Moq** — use `Mock<T>`, `.Setup(…)`, `.Verify(…)` |
| 140 | +- File naming: `{ClassName}Test[_{Scenario}].cs` (e.g., `SftpClientTest.ConnectAsync.cs`) |
| 141 | +- Each scenario lives in its own partial class file inside `Classes/` |
| 142 | +- Base classes: `TestBase`, `SessionTestBase`, `BaseClientTestBase` — extend the appropriate base |
| 143 | +- Arrange-Act-Assert style; each test method is focused on a single observable behaviour |
| 144 | + |
| 145 | +```csharp |
| 146 | +[TestMethod] |
| 147 | +public void MethodName_Scenario_ExpectedOutcome() |
| 148 | +{ |
| 149 | + // Arrange |
| 150 | + var connectionInfo = new PasswordConnectionInfo("host", 22, "user", "pwd"); |
| 151 | + var target = new SftpClient(connectionInfo); |
| 152 | + |
| 153 | + // Act |
| 154 | + var actual = target.SomeProperty; |
| 155 | + |
| 156 | + // Assert |
| 157 | + Assert.AreEqual(expected, actual); |
| 158 | +} |
| 159 | +``` |
| 160 | + |
| 161 | +### Integration tests (`test/Renci.SshNet.IntegrationTests/`) |
| 162 | + |
| 163 | +- Require **Docker** (via Testcontainers); a running Docker daemon is a prerequisite. |
| 164 | +- Run with `dotnet test` like any other project once Docker is available. |
| 165 | + |
| 166 | +### Running tests |
| 167 | + |
| 168 | +```bash |
| 169 | +# Unit tests only |
| 170 | +dotnet test test/Renci.SshNet.Tests/ |
| 171 | + |
| 172 | +# All tests (requires Docker) |
| 173 | +dotnet test |
| 174 | + |
| 175 | +# With code coverage |
| 176 | +dotnet test --collect:"XPlat Code Coverage" |
| 177 | +``` |
| 178 | + |
| 179 | +### What to test |
| 180 | + |
| 181 | +- Add unit tests for every new public method and every non-trivial internal method. |
| 182 | +- Test both the happy path and error/edge cases (e.g., `null` arguments, disposed state, cancellation). |
| 183 | +- For async methods, test cancellation via `CancellationToken` and timeout behaviour. |
| 184 | +- Do **not** remove or weaken existing tests; add new tests instead. |
| 185 | + |
| 186 | +--- |
| 187 | + |
| 188 | +## Performance considerations |
| 189 | + |
| 190 | +- SSH.NET is designed for parallelism; avoid introducing `lock` contention on hot paths. |
| 191 | +- Prefer `ArrayPool<byte>.Shared` or `MemoryPool<byte>.Shared` over allocating new `byte[]` in protocol encoding/decoding. |
| 192 | +- The `BufferSize` on `SftpClient` controls read/write payload sizes; keep protocol overhead in mind when changing defaults. |
| 193 | +- Benchmark significant changes with `test/Renci.SshNet.Benchmarks/` using BenchmarkDotNet. |
| 194 | + |
| 195 | +--- |
| 196 | + |
| 197 | +## Making changes safely |
| 198 | + |
| 199 | +1. **Run the unit tests first** (`dotnet test test/Renci.SshNet.Tests/`) to establish a clean baseline. |
| 200 | +2. **Keep changes focused.** Small, targeted PRs merge faster. Separate refactoring from behaviour changes. |
| 201 | +3. **Multi-targeting**: the library targets .NET Framework 4.6.2, .NET Standard 2.0, and modern .NET. Use `#if NETFRAMEWORK` / `#if NET` guards for platform-specific code; confirm all TFMs build with `dotnet build`. |
| 202 | +4. **Public API changes**: adding new overloads or members is generally safe. Removing or changing existing signatures is a breaking change — discuss in an issue first. |
| 203 | +5. **Cryptographic changes**: consult existing algorithm registration lists (in `ConnectionInfo.cs` and `Session.cs`) before adding or removing algorithms. |
| 204 | +6. **StyleCop and analyzers**: the build treats analyzer warnings as errors in Release/CI mode. Run `dotnet build -c Release` locally to catch these before pushing. |
| 205 | +7. **CI note**: some integration tests can flake due to timing or networking. If an existing (unrelated) test fails intermittently in CI but passes locally, it is likely a known flake. |
0 commit comments