WW-5698 Scope the ModelDriven exemption in StrutsParameterAuthorizer to the model object

[lukaszlenart]

Fixes WW-5698

Targets 7.4.0. This is a behavioural change (see Compatibility), and requireAnnotations.transitionMode is the supported migration path for applications it affects — that turned out not to be a separate gating decision, because transition mode already is that lever once wired correctly (see the transition mode section).

Problem

StrutsParameterAuthorizer.isAuthorized(...) returned true for any parameter name once the action implemented ModelDriven:

if (target != action && action instanceof ModelDriven) {
    return true;
}

OGNL then resolves that name against the whole CompoundRoot, which holds the model on top of the action. Authorization was decided about the model while the write could land on the action. The practical result: the @StrutsParameter requirement did not apply to a ModelDriven action's own members.

Same unannotated setter, declared on the action class in both cases, with struts.parameters.requireAnnotations=true:

plain action        actionSecret=... -> not bound   (correctly rejected)
ModelDriven action  actionSecret=... -> bound

Change

The exemption now covers what it was meant to cover:

• a property declared by the model is exempt — returning an object from getModel() declares it request surface, and that is the whole point of the exemption
• a property declared by the action is subject to the annotation requirement, as it would be on any other action
• a property declared by neither is still allowed

That third case matters for compatibility. A model bound through a custom OGNL property accessor — a Map-backed model, most commonly — declares no bean property, and such a name cannot be reaching a member of the action either. Rejecting it would break those applications, so it is explicitly allowed.

The model is checked first, so a model property that shadows an action property still binds without an annotation, matching OGNL's own resolution against the stack top.

Compatibility

An application whose ModelDriven action relies on binding unannotated members declared on the action will stop binding them and will need those members annotated with @StrutsParameter. That is the same migration those members would have needed had the action not been ModelDriven. Model binding itself is unchanged.

Transition mode

requireAnnotations.transitionMode exempts depth-0 parameters so an application can enable requireAnnotations while it works through annotating. It was checked after the ModelDriven branch, so it never applied to a ModelDriven action at all. That was harmless while the exemption authorized everything, but it means the actions this change affects would have had no migration path — the one lever built for this situation was unreachable for exactly the actions that need it.

It is now checked first, so an application broken by this change can set requireAnnotations.transitionMode=true and its depth-0 action members keep binding, which is the same migration path any other action already has. This is also why no new opt-out flag is proposed.

Tests

Seven new cases in ParameterAuthorizerTest, covering the rejection, the annotated action member, the model property, the shadowed property, and the declared-on-neither escape.

Both new branches were mutation-checked rather than trusted because they passed:

• removing the model-first check fails only the shadowing test — so the ordering is load-bearing
• removing the declared-on-neither escape fails the new test and the pre-existing modelDriven_targetIsModel_allAuthorized — so that escape is what preserves existing behaviour

Green: core 3202, json 166, rest 124 (including ParameterAuthorizingModuleTest).

Note: a full-reactor mvn test currently fails to compile struts2-tiles-plugin (package org.apache.velocity.tools.view does not exist). That is pre-existing — it reproduces identically on unmodified main — and unrelated to this change, but it does mean the four modules after tiles were not exercised.

Related

WW-5697 / #1871 came from the same triage. Different cause, different fix; the two overlap only in that a ModelDriven action is the easiest way to reach both.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
94.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud