Skip to content

fix(admin): encrypt only after successful validation#13729

Merged
shreemaan-abhishek merged 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/encrypt-after-validation
Jul 23, 2026
Merged

fix(admin): encrypt only after successful validation#13729
shreemaan-abhishek merged 1 commit into
apache:masterfrom
shreemaan-abhishek:fix/encrypt-after-validation

Conversation

@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

Description

resource.check_conf calls the per-resource encrypt_conf hook before checking whether self.checker(...) succeeded. For invalid input, the encryption hooks therefore run against unvalidated structures: for example, a non-object plugins value reaches pairs(plugins_conf) in admin/plugins.lua and raises a Lua error, turning a clean 400 schema-validation error into a 500.

This moves the encrypt_conf call below the if not ok then return so encryption runs only on validated config (invalid configs are never persisted anyway).

Also drops the plugin_object.check_schema gate in plugin_metadata.encrypt_conf: plugin.encrypt_conf derives fields from metadata_schema.encrypt_fields and does not need a custom validator, so a plugin that validates via core.schema.check would otherwise skip encryption and persist its metadata secret in plaintext.

Both were introduced in #12603.

Tests

New t/admin/encrypt-after-validation.t:

  • a non-object plugins value now returns 400 (previously a 500 from bad argument #1 to 'pairs' (table expected, got string));
  • a valid consumer with a key-auth secret is still stored as ciphertext at rest when data_encryption is enabled, confirming encryption still runs on the success path.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change (N/A - no user-facing behavior/doc change)
  • I have verified that the code passes the existing lint (luacheck)

check_conf ran the encrypt_conf hook before checking the checker
result, so malformed input (e.g. a non-object plugins value) reached
pairs() in the plugin encrypt hook and turned a 400 schema error into
a 500. Move encryption below the 'if not ok' return so it runs only on
validated config.

Also drop the check_schema gate in plugin_metadata.encrypt_conf so a
plugin that validates via core.schema.check does not skip metadata
secret encryption.

Adds t/admin/encrypt-after-validation.t: invalid plugins -> 400 (not
500); valid config still encrypts a secret field at rest.
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jul 23, 2026

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@shreemaan-abhishek
shreemaan-abhishek merged commit 15c894f into apache:master Jul 23, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants