Repository navigation
fix(cryptor): reject missing response fields - #7903
Merged
Aias00 merged 1 commit intoOct 21, 2026
Merged
Conversation
Signed-off-by: cloudbase-ai <22498243+cloudbase-ai@users.noreply.github.com>
Aias00
approved these changes
Oct 21, 2026
Aias00
left a comment
Contributor
There was a problem hiding this comment.
This is a security fix, not just a cleanup: returning originalBody from fieldErrorParse meant that a misconfigured response rule (a field name that does not exist in the upstream payload) sent the decrypted plaintext straight back to the client. Failing closed is the right call.
What I verified:
ResponsiveException(int code, String message, ServerWebExchange)exists with exactly this signature, andShenyuResultEnum.CRYPTOR_RESPONSE_ERROR_CONFIGURATION.getCode()returnsint(value-116), so the constructor call compiles and matches the plugin's own error enum viacheckErrorEnum().- The end-to-end path actually writes the error: the test wires a real
DefaultShenyuResultthroughSpringBeanUtils, and asserts both the JSON body (code == -116, message == enum message +[fieldNames]) and thatContent-Lengthequals the rewritten body length. That last assertion matters — the upstream had already set aContent-Lengthfor the original body, so a response written without fixing it would be truncated or hang. assertFalse(responseBody.contains("confidential-marker"))is the assertion that proves the leak is closed, rather than just proving an exception is thrown.- Covering
encrypt/decryptxfield/allx single / multiple missing fields is the right matrix, andtestExistingFieldIsStillTransformedguards against over-correcting into "always fail". - Generating a 2048-bit key pair in
@BeforeAllinstead of shipping a fixture is better than the old 512-bit PKCS#1 constant — it keeps the test valid if the default RSA strategy is tightened later.
Non-blocking notes:
- The message is the literal string
"... plugin's " + "[fieldNames]", i.e. users see the placeholder text[fieldNames]rather than the fields that are actually missing. The enum name reads like it was intended for a placeholder substitution. Since the point of this change is to make misconfiguration debuggable, substituting the real names (or at least logging them) would deliver the value the fix is aiming at. writeResponsesets the application context on theSpringBeanUtilssingleton and resets it tonullinfinally. Saving and restoring the previous value would be safer than nulling it — if another test in the same JVM installed a context, this wipes it for whatever runs next.fieldErrorParsetakesoriginalBodyand now ignores it entirely. If the missing-field list can be derived from it, that is the natural place; otherwise the parameter is dead weight and worth removing.
CI: pr_build is still pending on this head; approving on the code.
This was referenced Oct 21, 2026
Merged
This was referenced Oct 22, 2026
3 tasks done
This was referenced Oct 22, 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
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.
Fixes #6476.
When response field extraction returns no value,
CryptorResponsePlugin.fieldErrorParsecurrently returns the original body. A stalefieldNamesrule can therefore silently bypass the configured transformation.Throw the existing
ResponsiveExceptionwithCRYPTOR_RESPONSE_ERROR_CONFIGURATION(-116) and[fieldNames], matching the request cryptor's missing-field handling. The existing response error handler writes the configured ShenYu error result before the buffered body is emitted.Add tests that write actual downstream response bodies through the plugin and its response decorator. Six regression cases cover encryption/decryption with missing top-level, nested and multiple fields; they reproduce the passthrough before this change and now verify the error code/message, absence of the original confidential marker and corrected Content-Length. Four controls use generated RSA keys to verify successful
fieldandallencryption/decryption.Compatibility and scope
Routes that previously relied on missing-field passthrough now receive a configuration error and should correct or narrow their rules. This change covers the buffered
writeWithconversion path. The shared response decorator's streamingwriteAndFlushWithconversion bypass remains a separate follow-up; this PR does not claim to enforce transformations on streaming responses.Validation
All 33 tests in the cryptor module pass, including the existing request, RSA and JSON tests. The selected reactor passes Checkstyle and Apache RAT:
The full repository test suite and deployment end-to-end tests were not run.
Make sure that:
./mvnw clean install -Dmaven.javadoc.skip=true. Focused verification is listed above.