Skip to content

fix(cryptor): reject missing response fields - #7903

Merged
Aias00 merged 1 commit into
apache:masterfrom
cloudbase-ai:fix/issue-6476-cryptor-missing-field
Oct 21, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
cloudbase-ai:fix/issue-6476-cryptor-missing-field

Conversation

@cloudbase-ai

Copy link
Copy Markdown
Contributor

Fixes #6476.

When response field extraction returns no value, CryptorResponsePlugin.fieldErrorParse currently returns the original body. A stale fieldNames rule can therefore silently bypass the configured transformation.

Throw the existing ResponsiveException with CRYPTOR_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 field and all encryption/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 writeWith conversion path. The shared response decorator's streaming writeAndFlushWith conversion 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:

./mvnw -B -ntp -nsu \
  -pl shenyu-plugin/shenyu-plugin-security/shenyu-plugin-cryptor -am verify \
  -Dtest=CryptorResponsePluginTest,CryptorRequestPluginTest,RSAStrategyTest,JsonUtilTest \
  -Dsurefire.failIfNoSpecifiedTests=false \
  -Djacoco.skip=true -Dmaven.javadoc.skip=true

The full repository test suite and deployment end-to-end tests were not run.

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true. Focused verification is listed above.

Signed-off-by: cloudbase-ai <22498243+cloudbase-ai@users.noreply.github.com>

@Aias00 Aias00 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, and ShenyuResultEnum.CRYPTOR_RESPONSE_ERROR_CONFIGURATION.getCode() returns int (value -116), so the constructor call compiles and matches the plugin's own error enum via checkErrorEnum().
  • The end-to-end path actually writes the error: the test wires a real DefaultShenyuResult through SpringBeanUtils, and asserts both the JSON body (code == -116, message == enum message + [fieldNames]) and that Content-Length equals the rewritten body length. That last assertion matters — the upstream had already set a Content-Length for 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 / decrypt x field / all x single / multiple missing fields is the right matrix, and testExistingFieldIsStillTransformed guards against over-correcting into "always fail".
  • Generating a 2048-bit key pair in @BeforeAll instead 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.
  • writeResponse sets the application context on the SpringBeanUtils singleton and resets it to null in finally. 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.
  • fieldErrorParse takes originalBody and 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.

@Aias00
Aias00 merged commit 11a39fd into apache:master Oct 21, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Response cryptor returns plaintext when the configured field is missing

2 participants