Skip to content

feat: allowlist enum method invocation and return types#1323

Merged
jasmith-hs merged 1 commit into
masterfrom
jasmith_hubspot/san-salvador
Jul 14, 2026
Merged

feat: allowlist enum method invocation and return types#1323
jasmith-hs merged 1 commit into
masterfrom
jasmith_hubspot/san-salvador

Conversation

@jasmith-hs

Copy link
Copy Markdown
Contributor

Description

This PR description was authored by Claude Code.

jinjava sandboxes template rendering with two allowlists — a method allowlist and a return-type allowlist — both of which match a value's canonical class name against configured names/prefixes.

A simple enum (enum Color { RED, GREEN }) could already be allowlisted by adding its canonical class name. But an enum with a constant-specific class body (enum Op { PLUS { int apply(){...} } }) is realized at runtime as an anonymous subclass (Op$1) whose getCanonicalName() returns null. Both validators bail on a null canonical name, so these enums — their values and their overridden methods — could never be allowlisted, even when the caller explicitly added the enum type.

This change resolves the runtime enum-constant class to its base enum type before the existing canonical-name check, so allowlisting a specific enum by class name works for its values and methods, including constant-specific bodies.

Scope is intentionally narrow: specific enums only. No blanket "allow all enums" toggle, no new builder helper, no new AllowlistGroup — just correct resolution of the value/method class back to the enum type the caller already allowlisted.

  • AllowlistReturnTypeValidator: when the value is an enum, validate against ((Enum<?>) o).getDeclaringClass() instead of the runtime class.
  • AllowlistMethodValidator: when a method's declaring class has a null canonical name and its superclass is an enum, use the enum type's canonical name for the check.

Both are no-ops for simple enums and non-enum values.

BRAVE

Backwards Compatibility

  • Fully backwards compatible.
  • No API changes; the two validators only change how they derive the class name used for the existing allowlist check.
  • No-op for simple enums and all non-enum values — behavior only changes for enum values that were previously rejected due to a null canonical name.

Rollout and Rollback Plan

  • Standard library release; no data migration or feature gate.
  • Rollback is a straight revert of this commit.

Automated Testing

  • New AllowlistEnumTest (9 tests): direct validator checks for a simple enum and a constant-specific-body enum, rejection of a non-allowlisted enum (java.time.Month), and end-to-end renders ({{ op.apply(2, 3) }}5, {{ color.label }}, {{ op }} → name). The constant-body method test pins the regression by asserting the declaring class canonical name is null.
  • New test enums SimpleColorEnum and OperationEnum under the already-allowlisted testobjects package.
  • Full com.hubspot.jinjava.el.** suite (236 tests) passes; spotless:check clean.

Verification

  • mvn -Dtest=AllowlistEnumTest,AllowlistGroupTest,ValidatorConfigBannedConstructsTest test — green.

Expect Dependencies to Fail

  • None expected. The sandbox is only widened for enum types a caller has explicitly allowlisted; the @Value.Check guardrails still reject banned classes at config time, so resolution cannot expose anything not already permitted.
REVIEWERS: Please review both the code changes and the answers above, and validate that they match the expectations for BRAVE

Enum values with constant-specific class bodies are realized at runtime as
anonymous subclasses (e.g. Op$1) whose getCanonicalName() returns null. The
method and return-type allowlists key off canonical class name, so they bailed
on null and such enums could never be allowlisted even when the user explicitly
added the enum type. Resolve the runtime enum-constant class to its base enum
type before the canonical-name check so allowlisting a specific enum works for
its values and methods, including constant-specific bodies.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jasmith-hs jasmith-hs self-assigned this Jul 14, 2026
@jasmith-hs jasmith-hs marked this pull request as ready for review July 14, 2026 18:30
@jasmith-hs jasmith-hs merged commit cfcfb40 into master Jul 14, 2026
7 checks passed
@jasmith-hs jasmith-hs deleted the jasmith_hubspot/san-salvador branch July 14, 2026 19:51
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.

2 participants