Add a migration guide from winrm4j to the site documentation - #163
Conversation
New src/site/markdown/migrating-from-winrm4j.md: why migrate, a one-for-one mapping of WinRmTool.Builder / WinRmClientBuilder options and execution calls to the fluent WinRMClient API, the behavioral differences worth knowing (always-on HTTP payload encryption, no Basic auth, Kerberos requires HTTPS, UTF-8 vs code page 437, timeout and retry semantics, shell reuse), and the features winrm4j never had. Linked from the Reference menu, the overview, and the installation page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26d4cac366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * The password is a **`char[]`**, not a `String`, so the caller can wipe the single authoritative | ||
| copy of the secret after closing the client. |
There was a problem hiding this comment.
When callers rely on wiping the supplied char[] after closing the client, this statement gives a false security guarantee: LightWinRMService.resolveAuthScheme() creates an immutable String with new String(winRMEndpoint.getPassword()) (LightWinRMService.java:264), and the NTLM/Kerberos authentication objects retain that string. Wiping the original array therefore does not erase the client's other password copy, so the guide should not describe it as the single authoritative copy unless the implementation is changed.
Useful? React with 👍 / 👎.
| | `authenticationScheme(AuthSchemes.BASIC)` | none — use NTLM; see [behavioral differences](#behavioral-differences) | | ||
| | `disableCertificateChecks(true)` | `trustAllCertificates()` | | ||
| | `sslContext(SSLContext)` | `sslContext(SSLContext)` — hostname verification stays on | | ||
| | `hostnameVerifier(...)`, `sslSocketFactory(...)` | none — covered by `sslContext(...)` / `trustAllCertificates()`; see [TLS / HTTPS](tls.html) | |
There was a problem hiding this comment.
When a winrm4j deployment uses a custom HostnameVerifier to accept an alias while retaining certificate-chain validation, neither suggested replacement covers that behavior: sslContext(...) explicitly keeps hostname verification enabled, while trustAllCertificates() disables both hostname and certificate validation. Calling these options “covered” can either leave the migrated client unable to connect or push users toward an insecure trust-all configuration; document that the certificate must identify the requested hostname or that there is no selective verifier equivalent.
Useful? React with 👍 / 👎.
| * **No Basic authentication.** Basic sends credentials effectively in the clear and is disabled on | ||
| Windows by default; the client does not implement it. Use NTLM — every account that | ||
| authenticates with Basic also authenticates with NTLM, with no host-side change. |
There was a problem hiding this comment.
When migrating a host configured for Basic authentication with WinRM's Negotiate authentication disabled, switching to NTLM does require a host-side change: this client sends NTLM under Negotiate, and preparing-the-host.md identifies enabled Negotiate as a requirement. Accounts are not automatically usable through NTLM merely because Basic accepted them, particularly where policy disables NTLM, so the guide should tell users to enable Negotiate and confirm that NTLM is permitted rather than promise no configuration change.
Useful? React with 👍 / 👎.
…159) Drop the "single wipeable password copy" promise (the client keeps an internal String copy), state that hostname verification has no custom-verifier hook rather than being "covered", and require Negotiate enabled / NTLM permitted on the host when replacing Basic authentication instead of promising no host-side change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #159.
Adds
src/site/markdown/migrating-from-winrm4j.mdto the site documentation, targeting winrm4j users (the project is dormant: last release 0.12.3 in August 2021, last commit March 2023):ServiceLoaderconflicts, active maintenance, and the features winrm4j never had (WQL, file transfers, stdin, streaming, the CLI).WinRmToolvsWinRMClientfor the same command.WinRmTool.Builder/WinRmClientBuilderoptions toWinRMClient.builder(...),executeCommand/executePs(+Writervariants) tocommand(...)/powerShell(...)withonStdout/onStderr/start(), andWinRmToolResponsetoCommandResult. The winrm4j method names and defaults were verified against the winrm4j master sources (including its hardcodedWINRS_CODEPAGE=437, the default of 1 connection retry, and thePayloadEncryptionModedefault).The page is linked from the Reference menu in
site.xml, the overview's "Where to go next" list, and the installation page.mvn verify sitepasses; all cross-page anchors were checked against the rendered HTML.🤖 Generated with Claude Code