Release Java SDK v17.4.0 - #1110
Open
stas-schaller wants to merge 22 commits into
Open
Conversation
Removes a section-header decorator, several em dashes, four "This method" docstring restatements, and inline ticket references from comments across core. Ticket refs belong in commit messages, not code; the substantive explanation in each is kept. No behavior change.
…#1109) KSM-1248: reject server-supplied key_id values outside the keeperPublicKeys map (keys 7-18) before writing them to storage. A hostile server cannot poison the stored key_id and break subsequent calls. KSM-1262: saveCachedValue and LocalConfigStorage.saveToFile now write to a temp file with 0600 permissions set before any data is written, then replace the target via Files.move(ATOMIC_MOVE). This eliminates the window where the file was world-readable between create and chmod. Also removes ticket refs from comments and rewrites affected comments in STE-flavored prose.
When the config or cache directory is not writable, the temp-file creation fails with an AccessDeniedException that names the temp path. Wrap createTempFile in both saveToFile and saveCachedValue to rethrow as SecretsManagerException naming the config/cache path and its parent, matching the guidance already in the changelog. Guard testConfigFileAtomicWriteIsOwnerOnly with org.junit.Assume so it skips rather than fails on Windows or non-POSIX file systems.
* fix(java): use SecureRandom for generatePassword shuffle (KSM-1203) The final shuffle in generatePassword used Kotlin's Random.Default (a non-cryptographic PRNG), making the character arrangement predictable from PRNG state even though character selection was already secure. Switch to Collections.shuffle with SecureRandom.getInstanceStrong() so the entire password generation path uses a CSPRNG. * docs(java): add KSM-1203 changelog entry
* feat(java): expose isEditable on KeeperRecord (KSM-1176) The server returns isEditable in the record envelope and the private DTO already parsed it, but KeeperRecord had no corresponding field. Add isEditable: Boolean = false to KeeperRecord (default preserves existing callers) and forward record.isEditable in decryptRecord(). * docs(java): add KSM-1176 changelog entry
…1116) * fix(java): set connect/read timeouts on all HttpsURLConnection calls (KSM-1207) HttpsURLConnection defaults to timeout 0 (infinite), allowing a stalled or hostile server to block the caller indefinitely. Add connectTimeoutMillis (default 5s) and readTimeoutMillis (default 30s) to SecretsManagerOptions and thread them through to postFunction. Apply module-level constants at the downloadFile and uploadFile private helpers whose callers don't carry options. * docs(java): add KSM-1207 changelog entry * fix(java): use module constants for postFunction timeout defaults
…ase (#1117) * fix(java): keep the four-argument postFunction callable from Java (KSM-1207) Kotlin default arguments do not exist in bytecode, so adding connectTimeoutMillis and readTimeoutMillis replaced the published static postFunction(String, TransmissionKey, EncryptedPayload, boolean) rather than extending it. Java callers stop compiling, and callers already compiled against 17.3.0 fail at runtime with NoSuchMethodError. @jvmoverloads restores the four- and five-argument forms alongside the new one. Affects the ServiceNow credential resolver and the hello-secret-custom-caching example, both of which call the four-argument form. * fix(java): keep KeeperRecord's constructor Java-compatible (KSM-1176) isEditable was inserted between revision and files. KeeperRecord carries no @jvmoverloads, so Java sees only the full-arity constructor and the parameter count went from nine to ten, breaking every Java caller. Moving the field to the end and adding @jvmoverloads restores the original nine-argument constructor exactly, with no change to the Kotlin API or to JSON decoding (KeeperRecord is not @serializable). The internal construction site now uses named arguments so a future field cannot silently shift positions again. * fix(java): apply the caller's timeouts to the file upload transport (KSM-1207) uploadFile(options, ownerRecord, file) already holds the options and passes them to postQuery for the add_file request, but the private helper that performs the actual upload hardcoded the module defaults. A caller raising readTimeoutMillis for a slow link saw it honoured for the metadata request and silently ignored for the upload in the same call. downloadFile keeps the module constants: its public entry points take a KeeperFile and no options, so there is nothing to thread through. * test(java): verify readTimeout actually bounds a stalled server (KSM-1207) The previous test asserted that SecretsManagerOptions echoes its own constructor arguments, which holds for any Kotlin data class and passes unchanged if both connection.readTimeout assignments are deleted. TimeoutTest points postFunction at a ServerSocket that accepts the connection and then goes silent, so the client blocks reading the ServerHello. With the timeout applied the call fails in about one second; without it the read never returns, so the call runs under a watchdog on a daemon thread that fails the test at twenty seconds instead of hanging the Gradle test JVM. The defaults are asserted separately, since 5000/30000 is the documented contract. Connect timeout has no behavioural test: it needs a blackholed address, which is not reproducible across CI environments. * chore(java): bump core version to 17.4.0 The release branch and the changelog both say 17.4.0, but build.gradle.kts and KEEPER_CLIENT_VERSION still said 17.3.0. publish.maven.java.core.yml takes no version input and reads the coordinate straight from build.gradle.kts, so the release would have tried to republish 17.3.0 to Maven Central and reported mj17.3.0 in the client version header. * test(java): anchor the accepted socket so GC cannot close it mid-handshake The socket returned by accept() was discarded, so it turned unreachable as soon as the acceptor thread exited. A GC cycle inside the one-second window then closed the server side underneath the TLS handshake and the client failed on the wrong exception, making the assertion look like the timeout code was broken. Confirmed under forced GC, and not marginal: with the socket discarded the call failed in 30-90ms with SSLException on JDK 8 and SocketException on JDK 21, never once with SocketTimeoutException. Anchored, it times out at ~1.04s on both. The same negative control run through Gradle fails in 0.15s on the wrong exception type. Held in an AtomicReference rather than a captured var: the acceptor thread writes it and the test thread reads it to close, and a captured var compiles to a non-volatile Ref.ObjectRef, so the reader could see a stale null and silently skip the close. Joining the acceptor before closing makes the handoff ordered, and the close in a finally block stops the test leaking the socket either way. Raised by Stas Schaller in review of #1117.
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
This was referenced Aug 19, 2026
…llow-ups (#1120) * docs(java): document the 17.4.0 breaking changes in the changelog The 17.4.0 section listed only features and fixes. Enumerating every class in the compiled jar and diffing javap output against master shows exactly three public members change relative to 17.3.0, plus the two synthetic constructors Kotlin emits for default arguments: deleteFolder() return type changed KeeperRecord.copy() arity changed SecretsManagerOptions.copy() arity changed deleteFolder() was already called out in the release PR description but never reached the changelog, which is what users actually read on upgrade. The second entry covers a case easy to state too narrowly. Adding a constructor parameter changes copy(), but it also changes the synthetic $default constructor, so Kotlin code that merely builds a KeeperRecord while omitting a defaulted argument breaks on a jar swap too. Verified: a caller compiled against 17.3.0 and run against 17.4.0 throws NoSuchMethodError: KeeperRecord.<init>(..., int, DefaultConstructorMarker) Recompiling resolves it. Java call sites are unaffected either way, since both types carry @jvmoverloads. Matches the Breaking Changes block the 17.3.0 section already uses. * fix(java): keep the underlying IOException when a config write fails (KSM-1262) Both temp-file write paths caught IOException and threw a SecretsManagerException that discarded the original exception, so the stack trace and errno were lost. The message also asserted a cause it could not know: createTempFile fails on a full or read-only volume, a missing parent directory, or an fd limit, none of which are a permission problem, and the operator was told the directory was not writable. SecretsManagerException gains an optional cause. @jvmoverloads keeps the single-argument constructor, so Java callers and the existing subclasses that call super(message) are unaffected; javap confirms no member was removed from the class. * docs(java): document KeeperRecord.isEditable semantics (KSM-1176) The field carries a backend permission decision, and the only description of it lived in the changelog. Records the three things a caller needs: the meaning of each value, that the SDK does not enforce it before an update, and that the `false` default is reachable only through direct construction because the response envelope requires the field. * test(java): assert the record decrypts before reading isEditable (KSM-1176) The test indexed records[0] directly. fetchAndDecryptSecrets swallows per-record failures to stderr, so a decrypt regression would have surfaced as IndexOutOfBoundsException rather than a readable assertion; both neighbouring tests already guard on the record count first. Folds the two cases into a loop over true/false. Verified the assertion still discriminates: hardcoding isEditable = true in decryptRecord fails the test, and reverting passes.
Add release/sdk/java/core/** to pull_request and push triggers in test.java.yml so CI fires on PRs targeting or pushing to the release branch.
…1118) * fix(java): KSM-1270 guard getSharedFolderKey against parent-cycle infinite loop Replace unbounded while(true) with a HashSet<String> visited guard. Add testGetFoldersSkipsFolderParentCycle to verify the call completes and returns an empty list instead of hanging. * KSM-1270 Address Mateo's review: test timeout, distinct cycle error, README fix - Add @test(timeout = 5_000) so a regression hangs for at most 5 s rather than blocking the entire Gradle test task indefinitely - Throw SecretsManagerException with a cycle-specific message instead of returning null, so operators can distinguish a parent cycle from other folder-key lookup failures in stderr output - Fold the two-line KSM-1270 README entry into one line to match the surrounding bullets
* feat(sdk/java): add HTTP/HTTPS proxy support (KSM-531)
Adds a proxyUrl option to SecretsManagerOptions that routes all SDK network
traffic through an HTTP proxy. Applies to secret queries, file uploads, and
file downloads (via the new options-taking downloadFile/downloadThumbnail
overloads). Notation lookups that resolve a file attachment also use the proxy.
Proxy resolution precedence when proxyUrl is not set: JVM system properties
(https.proxyHost / http.proxyHost), then HTTPS_PROXY / HTTP_PROXY env vars.
NO_PROXY and http.nonProxyHosts exclusions are honored.
Authenticated proxies supply credentials via a JVM-wide Authenticator scoped
to the proxy host, registered only for explicit proxyUrl values. Credentials
found in ambient env vars are not registered to avoid interfering with other
libraries that may have installed their own Authenticator.
When a CONNECT tunnel returns HTTP 407, the SDK throws SecretsManagerException
with the complete remediation message rather than surfacing a bare 407 or an
opaque IOException. The message branches on credential origin: explicit-config
failures explain the jdk.http.auth.tunneling.disabledSchemes flag requirement;
ambient-credentials failures explain why ambient creds were not registered and
how to pass them explicitly.
Also:
- Redact proxy URL credentials in exception messages using textual stripping,
so reserved-character passwords (containing @ or spaces) that prevent URI
parsing do not appear in logs
- Reject https:// proxy URLs with a clear error; the JDK cannot TLS to a proxy
- Reject explicit proxyUrl values with out-of-range ports or partial userinfo
- Reject explicit proxyUrl values with out-of-range ports (fail closed);
degrade ambient candidates with the same issues to null (fall through)
- Fix isExcluded inference: openProxiedConnection now calls isExcluded() directly
instead of inferring exclusion from resolved == null, which prevented
unparseable ambient proxies (e.g. underscore hostnames) from falling through
to the system ProxySelector
- Blank-guard all four *_PROXY env var reads and the https.proxyHost system
property so set-but-empty values do not mask lower-priority candidates
- Add secondary SecretsManagerOptions constructor for Java callers who need
proxyUrl without specifying all other defaults
- SecretsManagerException gains a (message, cause) constructor so the original
IOException stack trace is preserved when a 407 is reclassified; add explicit
serialVersionUID to maintain Java serialization compatibility with existing jars
- Move trustAllSslSocketFactory to ProxySupport.kt as private (previously
internal top-level = public in JVM bytecode); proxy internals remain internal
but inaccessible from Java via the Kotlin compiler's name mangling
- ProxyTest: 22 tests covering redaction, explicit-config guards, port/scheme
validation, blank env guards, isExcluded semantics, and 407 message branching
Deferred to separate tickets (not in scope for this PR):
- Authenticator chaining/unregister/credential clearing on re-registration
- QueryFunction typedef widening to carry proxy context
- cachingPostFunction proxy support and cache-fallback logging
- http.nonProxyHosts extended wildcard patterns (trailing wildcards, IPv6,
port-qualified entries, JDK built-in localhost wildcard)
- Per-connection HttpURLConnection.setAuthenticator (Java 9+ upgrade path)
* fix(sdk/java): address proxy review round 2 remaining items (KSM-531)
- Remove http.proxyHost fallback from systemPropertyProxy: the JDK's ProxySelector
never applies http.proxyHost to HTTPS URLs, and all KSM traffic is HTTPS
- Remove dead https-scheme branch from parseProxy port calculation (unreachable since
https:// proxy URLs are rejected upstream in parseProxy)
- Add warning log to cachingPostFunction when falling back to cached secrets on
network failure, noting that cachingPostFunction does not carry the proxy
- Thread allowUnverifiedCertificate through the private uploadFile overload so upload
connections honor the same TLS verification setting as downloads and API calls
- Add ProxyTest case confirming http.proxyHost has no effect on HTTPS proxy resolution
* fix(sdk/java): address proxy review round 3 findings (KSM-531)
- ResolvedProxy: override toString() to redact password field, matching the
SecretsManagerOptions treatment; data class toString() would otherwise expose
credentials in debug output, error messages, and structured logs
- SecretsManagerExceptions: replace @JvmField val serialVersionUID with
private const val, generating the conventional private static final bytecode
form recognized by Java serialization (Jenkins remoting, RMI, etc.)
- SecretsManager: remove now-unused import javax.net.ssl.* (trustAllSocketFactory
was moved to ProxySupport.kt; no remaining ssl references in this file)
- ProxyTest: remove unused ambientWithCreds local variable in
proxyAuthFailureMessageAmbientBranchRequiresCredentials
* fix(sdk/java): address proxy review round 5 findings (KSM-531)
- Fix serialVersionUID to 5401507264959279624L (matches released 16.6.6/17.x jars)
- Fix redactProxyUrl to handle scheme-less URLs (e.g. user:p@ssword@proxy:8080)
- Fix https.proxyPort='' yielding port 80 instead of 443 (takeIf isNotBlank)
- Fix NO_PROXY='' masking no_proxy fallback (takeIf isNotBlank on both reads)
- Replace SecretsManagerOptions 2-arg secondary constructor with @JvmStatic withProxy() factory
(copy() binary signature changed from 17.3.0; callers must recompile — documented in README)
- Fix cachingPostFunction: move warning after getCachedValue() so it only fires on cache hit
- Fix IOException 407 guard: add resolved.username != null so credential-less proxies
do not trigger the CVE remediation message
- Rename private downloadFile to downloadFileFromUrl to avoid name confusion
- Move KSM-531 README entry from 17.3.0 to 17.4.0; fix Notation coverage claim
- Add redactProxyUrlHandlesSchemeLessUrl and serialVersionUID tests
* docs(java): fix two README inaccuracies in 17.4.0 changelog
Remove stale sentence describing a two-arg SecretsManagerOptions(storage,
proxyUrl) constructor -- no such constructor existed in the 17.3.0 release,
so it was not a breaking change for callers upgrading from a published build.
Correct the claim that the single-arg downloadFile(file) / downloadThumbnail(file)
forms "open a direct connection." They pass proxyUrl=null to openProxiedConnection,
which still runs ambient proxy detection (HTTPS_PROXY, https.proxyHost, etc.).
* fix(java): redactProxyUrl misses credentials when password contains a slash
lastIndexOf('@', authorityEnd - 1) bounded the search to before the first '/'
in the URL, so a password like "a/b" placed the '@' outside the search window
and the function returned the URL unredacted. Drop the bound: lastIndexOf('@')
with no limit always finds the correct userinfo delimiter.
Add test case: http://user:a/b@proxy.corp:8080 -> http://user:***@proxy.corp:8080
* fix: consolidate proxy credential predicate to ResolvedProxy.hasCredentials
- Add hasCredentials property to ResolvedProxy (username != null && password != null)
- Use it at all 4 call sites that previously used two different spellings of the same check
- Fix cachingPostFunction to chain the original network error when the cache is also unavailable
- Add scheme-less URL with slash-in-password test case to redactProxyUrlStripsCredentialsTextually
- Rename proxyAuthFailureMessageAmbientBranchRequiresCredentials to match what the test asserts
* fix(java): widen breaking-change note and fix hasCredentials non-null assertion
Breaking change note was scoped to copy() callers only; widened to cover all
Kotlin call sites using default or named arguments, which bind to the same
synthetic constructor descriptor. Java callers are unaffected.
ProxyAuthenticator.register() call uses !! to satisfy the compiler after the
hasCredentials guard, since Kotlin cannot smart-cast through a custom property.
* fix(java): address round-7 review items for KSM-531 proxy support
- Reorder resolveProxy to check explicit proxyUrl before parsing targetUrl,
fixing a bypass where non-URI-parseable hosts (e.g. underscore names) could
fall through to a direct connection even with an explicit proxyUrl set
- Drop HTTP_PROXY/http_proxy from ambient proxy fallback; all KSM traffic is
HTTPS, consistent with the http.proxyHost exclusion already in place
- Extend partial-userinfo guard to reject password-only URLs (http://:pw@host)
in addition to username-only URLs
- Thread options.connectTimeoutMillis/readTimeoutMillis through downloadFile,
downloadThumbnail, and uploadFile so timeouts set in SecretsManagerOptions
apply to all outbound connections, not just secret queries
- Fix cachingPostFunction warning text: was "does not support a proxy" (wrong,
ambient proxies still apply); now names only options.proxyUrl as unsupported
and chains the original network error as the exception cause
- Update blankEnvVarDoesNotMaskLowerPriorityVar test to cover https_proxy
(lowercase case variant) now that HTTP_PROXY fallback is removed
- README doc pass: fix getValue/notation wording, add HTTPS_PROXY upgrade
note, correct HTTP_PROXY references, update cachingPostFunction note, fix
CVE latch scope ("HttpURLConnection class init" vs "any HTTPS connection"),
add allowUnverifiedCertificate expansion note, add percent-encoding hint
---------
Co-authored-by: Mateo Gallego <mgallego@keepersecurity.com>
…d gate stderr diagnostics (#1122) * test(java): cover the mixed-category password shuffle (KSM-1203) Every existing testGeneratePassword case requests a single character category, so all 32 characters come from one charset and the final shuffle cannot be observed. Those assertions pass whether the shuffle uses SecureRandom, uses Random.Default, or is deleted outright, which left the KSM-1203 fix with no coverage at all. randomSample() emits characters grouped by category, so a mixed request is the only configuration where the shuffle is visible: without one, position 0 is always lowercase. The new test asserts that the category of the first character varies across 200 passwords, and that each category keeps exactly the requested count. Verified to fail when the shuffle is removed from generatePassword. It does not distinguish a CSPRNG from a weak PRNG, since both produce a uniform permutation; the test comment says so rather than implying coverage it does not have. * test(java): cover the config write failure cause (KSM-1262) PR #1120 changed both write paths to report what the file system returned and to retain the original IOException as the cause, but shipped no test for it, so a later refactor can drop the cause without anything failing. Points LocalConfigStorage at a path whose parent does not exist and asserts the cause survives, that a missing directory is not reported as a permissions problem, and that the message names the directory it could not use. Verified against the implementation now on the release branch. * test(java): prove the config write swaps the file instead of rewriting it (KSM-1262) testConfigFileAtomicWriteIsOwnerOnly asserted only that the finished config file is 0600. The implementation it replaced also ended at 0600, by calling chmod after writing the private key through an 0644 handle, so the test passed against the vulnerable code and could not detect the fix being reverted. What KSM-1262 actually changed is that secrets are never written to the visible path: they go to a 0600 temp file that is renamed into place. A rename installs a different inode, so BasicFileAttributes.fileKey() changes across a write while an in-place rewrite keeps the same value. Comparing fileKey() across two writes makes that deterministic to assert, with no race to lose. Also asserts the permissions survive the swap and that no ksm_*.tmp staging file is left behind on success. Verified to fail against an in-place write plus chmod-after implementation, reporting the unchanged inode. * fix(java): gate delete and folder-skip diagnostics on loggingEnabled Three stderr writes added for KSM-1081 and KSM-1086 ignored the loggingEnabled option that the throttle and key-rotation diagnostics in the same file already respect, so a consumer that had explicitly turned logging off still got stderr output from deleteSecret, deleteFolder and the getFolders skip path. The folder-skip message also diverged from the four equivalent handlers in fetchAndDecryptSecrets, which all include e.javaClass.simpleName. One of those logs the identical "Folder <uid> skipped due to error:" prefix, so the file emitted near-identical lines in two formats. The class name is not decoration here: the causes this path exists for include a null data field, whose KotlinNullPointerException carries no message, so the line read "skipped due to error: null" and told an operator nothing. Tests drive getFolders through an injected queryFunction with one undecryptable folder, capturing stderr to assert nothing is written when loggingEnabled is false and that the exception type appears when it is true. Both verified to fail against the previous handler. * docs(java): correct the 17.4.0 changelog The section had drifted out of the ascending ticket order the file uses, and it documented no breaking change even though deleteFolder's return type changed in KSM-1086. Callers read the changelog, not the release PR description. Changes: - Restores ascending ticket order (1081, 1086, 1176, 1203, 1207, 1248, 1262). - Adds the missing Breaking Changes block for deleteFolder, noting that no working code can be affected: the old return type required a "records" field while the backend sends "folders", so every previous call threw MissingFieldException rather than returning a value. - KSM-1203 gains a statement that existing passwords need no rotation. Character selection always drew from SecureRandom, so only the arrangement was affected: for generatePassword(32, 8, 8, 8, 8) roughly 137 of the 193 bits were never at risk, and the default generatePassword() was unaffected because all of its characters come from a single set. Without this, the entry reads as though every password the SDK has generated is suspect. - KSM-1207 notes that downloadFile and downloadThumbnail take only a KeeperFile, so they use the built-in defaults rather than the values configured on SecretsManagerOptions. - KSM-1262 documents the config file now being read as UTF-8 explicitly, where it previously used the JVM default charset while always being written as UTF-8. - KSM-1081 and KSM-1086 note that the per-item diagnostics are gated on loggingEnabled.
stas-schaller
marked this pull request as ready for review
August 19, 2026 19:28
…17.4.0 changelog (KSM-1298)
…ps (KSM-531, KSM-1207) (#1123) * fix(java): reject incomplete proxy credentials in proxyUrl (KSM-531) parseProxy accepted http://user:@host as a valid credential, producing an empty-password Authenticator registration. The guard tested whether exactly one half of the userinfo was missing, which structurally cannot catch a half that is present but empty: (username == null) != (password == null) An empty password now nulls out during parsing exactly as an empty username already did, so both halves share one notion of absence and the guard reduces to "either half missing". http://user@host, http://:secret@host and http://user:@host are now the same configuration error. This matters because a templated proxy URL whose password variable goes unset (http://$USER:$PASS@host) renders as the third form. Before, it reached the proxy and came back as a 407, sending the operator down the JVM tunneling-flag remediation path for what is actually a config typo. Because hasCredentials derives from both fields, the fix also corrects the Authenticator registration, the ambient-vs-explicit 407 message branch, and the checkedResponseCode fast path without touching any call site. Also documents that clearing jdk.http.auth.tunneling.disabledSchemes is process-wide: it re-enables Basic auth over CONNECT for every HttpURLConnection in the JVM, not only the SDK's. The changelog previously said the SDK "does clear it automatically" without saying whose connections that affects. The SDK's own write stays narrow (explicit proxyUrl only, and a value the host application already set is left untouched), but sharing a JVM with other HTTP clients makes this a deliberate decision rather than an inherited one. Tests: partialUserinfoExplicitProxyThrows now covers all three rejected forms rather than one, and emptyPasswordAmbientProxyIsNotTreatedAsCredentials pins the ambient path, which must not throw but also must not register a credential. Suite is 101 tests, green on JDK 8. * fix(java): report timeouts in options toString and share their defaults (KSM-1207) Two loose ends from the KSM-1207 timeout work. SecretsManagerOptions.toString() is hand-written rather than data-class generated, so it has to be edited whenever a field is added. It was not: connectTimeoutMillis and readTimeoutMillis were absent from the rendered output, which is exactly the output someone reads when diagnosing a timeout. Both now appear. The two defaults were also declared twice, as bare literals in SecretsManagerOptions and again as DEFAULT_CONNECT_TIMEOUT_MS / DEFAULT_READ_TIMEOUT_MS for the overloads that take a KeeperFile without options. TimeoutTest asserted only the data-class side, so the pair could drift silently and callers of downloadFile(file) would quietly get a different timeout than callers of downloadFile(options, file). The constants are now the single source and moved up beside the other connection tunables. That also widens the reach of the existing test rather than needing a new one: with the literals gone, timeoutDefaults_matchDocumentedValues now fails if the constant changes. Verified by drifting DEFAULT_CONNECT_TIMEOUT_MS to 6_000, confirming the test fails, and reverting. Adds toString_reportsTimeoutsAndRedactsProxyUrl, which also covers the proxyUrl redaction. Nothing tested toString before, so the redaction shipped in this release was unguarded. The test uses sentinel credentials because the rendered storage field carries the package name, and asserting on a realistic password like "secret" matches com.keepersecurity.secretsManager and passes for the wrong reason. Suite is 102 tests, green on JDK 8.
…ate (#1124) Two review cleanups with no effect on shipped behavior. The 4-argument postFunction carried @jvmoverloads despite having no default arguments, so the annotation generated nothing and the compiler warned on every build: w: SecretsManager.kt:1762:1 '@jvmoverloads' annotation has no effect for methods without default arguments. Removed, and replaced with a comment explaining why the overload is spelled out by hand instead. Verified ABI-neutral: javap over the SecretsManager facade before and after the change reports the same 76 public members with identical signatures, including all three postFunction entry points. The build is now warning-clean. ProxyTest left three pieces of process-wide state behind. Its teardown reset the default Authenticator but not the ProxyAuthenticator credential store or jdk.http.auth.tunneling.disabledSchemes, both of which register() mutates. Gradle runs the suite in one JVM, so proxy.local:8080 stayed answerable and Basic-over-CONNECT stayed enabled for every test that followed. Harmless today, but it silently weakens any later test that exercises a proxied connection, and a test that depends on the default being intact would pass or fail based on ordering. The teardown now restores all three, and the tunneling property is captured per-test so it is returned to whatever the JVM had rather than assumed unset. Clearing the credential store needs a hook, so ProxyAuthenticator gains reset(), declared the same way as the neighbouring register(): the enclosing object is already internal, so an explicit internal modifier would only add name mangling without changing what consumers can reach. Adds proxyAuthenticatorResetDropsRegisteredCredentials so the teardown itself is guarded, rather than trusting that reset() keeps working. Suite is 103 tests, green on JDK 8.
mgallego-keeper
previously approved these changes
Aug 20, 2026
…es (#1125) Every push to a release branch fired two identical Test-Java runs: the push trigger, plus pull_request synchronize on the open release PR whose head is that branch. Eight simultaneous cold Gradle builds hit Maven Central hard enough to earn an HTTP 429 on the foojay-resolver plugin classpath, and fail-fast then cancelled the other three JDKs. - restrict push to master, which both dedupes and gives the Gradle cache a writable seeding run; it was previously never written at all - set cache-read-only explicitly rather than relying on the action default - fail-fast: false, so one flaky JDK no longer hides the other three - concurrency group per ref, superseding stale runs instead of racing them - retry Build and Test up to 3 times, gated on the output matching a dependency-resolution error so a real test failure still fails at once - workflow_dispatch, to re-run a transient failure without an empty commit
The fix for KSM-1081 gated fetchAndDecryptFolders stderr on loggingEnabled but left four unconditional System.err.println calls in fetchAndDecryptSecrets and decryptRecord: record skip in the top-level records loop, record-in-folder skip and folder-key-decryption skip in the folders loop, and file-attachment skip in decryptRecord. A caller with loggingEnabled=false still received stderr output from all four paths. Each is now guarded identically to the folder skip path. Four unit tests added (104 total, 0 failures).
Coverage previously lived only in the external regression suite, which does not run in CI, so a revert of the response-type fix or the per-item error surfacing would still pass the Gradle build.
mgallego-keeper
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Release branch for Java SDK v17.4.0. Bundles HTTP/HTTPS proxy support (KSM-531), the security and robustness fixes from the OWASP audit,
getFolders()crash safety, per-item delete error surfacing, andisEditableonKeeperRecord. Version bumped to17.4.0(build.gradle.kts) andmj17.4.0(KEEPER_CLIENT_VERSION).Changes
New Features
ProxySupport.ktroutes every SDK connection (secret queries, file upload, file download) through a resolved proxy. NewproxyUrloption onSecretsManagerOptionsplus aSecretsManagerOptions.withProxy(storage, proxyUrl)static factory for Java callers. Precedence is explicitproxyUrl, thenhttps.proxyHost/https.proxyPort, thenHTTPS_PROXY/https_proxy;NO_PROXYandhttp.nonProxyHostsexclusions are honored.HTTP_PROXYandhttp.proxyHostare deliberately ignored because all KSM traffic is HTTPS. Authenticated proxies usehttp://user:pass@host:portand register a proxy-scoped defaultAuthenticator; a 407 raises a typedSecretsManagerExceptioncarrying thejdk.http.auth.tunneling.disabledSchemesremediation instead of a bare status code. An incomplete credential in an explicitproxyUrlis rejected as a configuration error, including an empty half (for example,http://user:@host), which is what a templated proxy URL produces when its password variable goes unset. An ambient proxy setting with an empty password is not treated as a credential.isEditableonKeeperRecord(KSM-1176): forwarded from the server response envelope so callers can check write permission before callingupdateSecret.downloadFile(options, file)anddownloadThumbnail(options, file), which honorproxyUrl,allowUnverifiedCertificate, and the configured timeouts. The single-argument forms are retained.SecretsManagerException(message, cause): the base exception now carries a cause so wrapped failures keep the underlying stack trace.serialVersionUIDis pinned to the value computed for the 17.3.0 class shape so jars either side of this change stay serialization compatible.Security Fixes
generatePassword(KSM-1203): the final character shuffle used KotlinRandom.Default. It now usesCollections.shufflewithSecureRandom. OWASP finding F-02, CWE-338.HttpsURLConnectioncalls (KSM-1207): all three connection sites (postFunction,downloadFile,uploadFile) now enforce a connect timeout (5 s) and a read timeout (30 s). AddedconnectTimeoutMillisandreadTimeoutMillistoSecretsManagerOptionsfor caller override. Both defaults come from a single constant shared with the no-options file-download overloads, so the two code paths cannot drift apart. OWASP finding F-01, CWE-400.MAX_KEY_ROTATION_RETRIES(3).LocalConfigStorage.saveToFile()andsaveCachedValue()now write via a temp-file swap with 0600 permissions set before data is written.SecretsManagerOptions.toString()now redactsproxyUrland reportsconnectTimeoutMillisandreadTimeoutMillis, so a proxy password cannot reach a log through a debug print and both timeout values are visible when diagnosing connection problems.Bug Fixes
getFolders()crash safety (KSM-1081): undecryptable folders are skipped; the remaining folders are returned normally. The skip diagnostic names the exception type and is gated onloggingEnabled.deleteFolder()return type (KSM-1086): returnsSecretsManagerDeleteFolderResponsewith typed per-folder status, matchingdeleteSecret(). Both calls now report per-item server failures to stderr, gated onloggingEnabled.getSharedFolderKey()infinite loop (KSM-1270): a parent cycle in server folder data spun forever. The walk now tracks visited folder UIDs and raises a typed error on revisit;getFolders()skips the affected folders and continues.@JvmOverloadsadded to the widenedKeeperRecordandSecretsManagerOptionsconstructors so every constructor arity published in 17.3.0 still exists for Java callers.CI
test.java.ymldid not run on pull requests targeting release branches. Seven pull requests merged into this branch before the gap was closed, none of them running the Java SDK tests, so their green checks did not cover the SDK. The matrix now triggers onpull_requestandpushfor bothmasterandrelease/sdk/java/core/**.Tests
ProxyTest(26 tests) andTimeoutTest(4 tests);SecretsManagerTestandCryptoUtilsTestextended for key-ID rejection, the rotation retry cap, atomic config writes, config write failure reporting,isEditableforwarding, folder skip behavior, and the password shuffle (test(java): close the untested paths in the 17.4.0 security fixes, and gate stderr diagnostics #1122). Suite is 102 tests (fix(java): proxy credential validation and timeout reporting follow-ups (KSM-531, KSM-1207) #1123 addsemptyPasswordAmbientProxyIsNotTreatedAsCredentialsandtoString_reportsTimeoutsAndRedactsProxyUrl), green on Java 8, 11, 17, and 21.Maintenance
android-exampleandhello-secretexample READMEs.sdk/java/core/README.mdwith full upgrade notes.Breaking Changes
deleteFolder()return type. Now returnsSecretsManagerDeleteFolderResponseinstead ofSecretsManagerDeleteResponse. The new type exposes afolderslist (each entry hasfolderUid,responseCode, and optionalerrorMessage); callers that read.recordsmust switch to.folders. In practice no working code is affected: the old return type required arecordsfield while the backend sendsfolders, so every previous call threwMissingFieldExceptionrather than returning a value.KeeperRecordgainedisEditable;SecretsManagerOptionsgainedconnectTimeoutMillis,readTimeoutMillis, andproxyUrl. Kotlin source needs no edit and Java keeps every 17.3.0 constructor form via@JvmOverloads, but the generatedcopy()and the synthetic default-argument constructors changed arity, so Kotlin code compiled against 17.3.0 throwsNoSuchMethodErrorif the 17.4.0 jar is swapped in without recompiling. Rebuild dependents rather than replacing the jar in place.Behavior Changes to Review Before Upgrading
HTTPS_PROXYandhttps.proxyHost; 17.4.0 routes SDK traffic through them. Deployments where these are set for other tooling should verify the values before upgrading.allowUnverifiedCertificatenow covers file transfers. When set, certificate verification is bypassed on file upload and download (via the options-taking overloads) in addition to secret queries, so the flag now applies to the storage URLs as well as the KSM API host.cachingPostFunctionprints an unconditional stderr warning when it serves stale cached data. It has no access toSecretsManagerOptions, so the output is not gated byloggingEnabled. KSM-1298 tracks the proper fix.getValue(secrets, notation)has nooptionsparameter, so it cannot carryproxyUrl,allowUnverifiedCertificate, or the configured timeouts; it uses ambient proxy settings and the built-in timeout defaults. UsegetNotationResults(options, notation)where explicit-proxy resolution is needed.-Djdk.http.auth.tunneling.disabledSchemes=must be set before the process makes any other HTTP or HTTPS call. Note that when the SDK sets this itself it clears the property process-wide, which re-enables Basic auth over CONNECT for everyHttpURLConnectionin the JVM, not only the SDK's.Related Issues