docs(java): document the 17.4.0 breaking changes, and three review follow-ups

[mgallego-keeper]

Follow-ups from the review pass over the v17.4.0 release branch, picking up what #1117 did not cover. Four fixes, one commit each; every commit builds and tests green on its own (70 tests, 0 failures).

Nothing here changes runtime behaviour except the exception message in item 2.

1. The changelog never documented the 17.4.0 breaking changes

The 17.4.0 section listed only features and fixes. #1110's description calls out the deleteFolder() return type change under a Breaking Changes heading, but that never reached README.md, which is what users actually read on upgrade. The 17.3.0 section right below it already uses a Breaking Changes block, so the convention exists.

To make the list defensible rather than a judgment call, I enumerated every class in the compiled jar and diffed javap output against master. Exactly three public members change, plus two synthetic constructors:

Member	Change
deleteFolder()	return type SecretsManagerDeleteResponse to SecretsManagerDeleteFolderResponse
KeeperRecord.copy()	arity 9 to 10
SecretsManagerOptions.copy()	arity 7 to 9

Nothing else is removed anywhere in the SDK.

The second entry is easy to state too narrowly, and my first draft did. Adding a constructor parameter changes copy(), but it also changes the synthetic $default constructor, which is the real call target whenever Kotlin source omits a defaulted argument. So Kotlin code that merely builds a KeeperRecord breaks on a jar swap too, not just copy() callers. Verified by compiling a caller against 17.3.0 and running it against 17.4.0:

run against 17.3.0 : OK uid=uid rev=1
run against 17.4.0 : java.lang.NoSuchMethodError: 'void KeeperRecord.<init>(
                       byte[], String, String, byte[], String, KeeperRecordData,
                       long, List, List, int, DefaultConstructorMarker)'

Recompiling resolves it, and Java call sites are unaffected either way thanks to the @JvmOverloads added in #1117. The changelog entry now says exactly that.

2. A failed config write threw away the underlying IOException (KSM-1262)

Both temp-file write paths in LocalConfigStorage caught IOException and threw a SecretsManagerException constructed from a message alone, so the stack trace and the errno were gone. The message also asserted a cause it could not know:

Cannot write config <path>: directory <parent> is not writable.

Files.createTempFile also fails on a full or read-only volume, a missing parent directory, and an fd limit. In each of those cases the operator was told, confidently, that it was a permission problem.

SecretsManagerException gains an optional cause. @JvmOverloads keeps the single-argument constructor, so Java callers and the two existing subclasses that call super(message) are untouched; javap confirms no member was removed from the class.

3. KeeperRecord.isEditable had no KDoc

The field carries a backend permission decision and its only description lived in the changelog. The doc records the three things a caller needs: what each value means, that the SDK does not enforce it before an update (per KSM-1173), and that the false default is reachable only through direct construction, because the response envelope requires the field.

Worth knowing for the last point: SecretsManagerResponseRecord.isEditable has no default, so a server that omitted the field would fail the whole getSecrets() call rather than yielding false:

field absent -> MissingFieldException: Field 'isEditable' is required ...
field null   -> JsonDecodingException: Expected valid boolean literal ...

That is pre-existing and not changed here, but it is why the default is documented as construction-only.

4. The isEditable test could fail as IndexOutOfBounds

testIsEditableForwardedToKeeperRecord indexed records[0] directly. fetchAndDecryptSecrets swallows per-record failures to stderr, so a decrypt regression would have surfaced as IndexOutOfBoundsException instead of a readable assertion. Both neighbouring tests already guard on the record count first.

Also folds the two cases into a loop. The assertion still discriminates: hardcoding isEditable = true in decryptRecord fails the test, reverting passes.

Verification

• 70 tests, 0 failures. Each of the four commits builds and tests green on its own.
• javap sweep over every class in the jar: the three members above are the only public removals between master and this branch.
• javap on SecretsManagerException: two additions, nothing removed.
• Jar-swap probe reproduces and then confirms the NoSuchMethodError described in item 1.

Not addressed here

Found during the same pass, left out as pre-existing and outside the release-blocker scope. Happy to open tickets:

• The atomic write has no fsync before the rename, so a power cut can leave a truncated config. Outside the permissions threat model KSM-1262 states, but "atomic write" invites the expectation.
• keyId at SecretsManager.kt:1738 is a public mutable top-level var, mutated as a side effect of the keeperPublicKeys initializer and exported as getKeyId()/setKeyId().
• testConfigFileAtomicWriteIsOwnerOnly asserts permissions but not atomicity despite the name, and does not cover saveCachedValue, the other half of KSM-1262.
• LocalConfigStorage switched from FileReader to readText(Charsets.UTF_8) in fix(java): validate server key_id range and eliminate config write race window #1109, which pairs correctly with the UTF-8 write side and fixes a latent platform-charset mismatch. Harmless for ASCII configs, but it went unremarked in the changelog.

[stas-schaller]

👍