* WW-5640 docs: design for WebJars support in Struts core
Adds first-class WebJars support so client-side libraries can be
referenced by a version-less logical path and served through the
existing static-content pipeline. Grounded against 7.2.x source.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 docs: implementation plan for WebJars support
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 build: add webjars-locator-lite dependency
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 feat: add webjars config constants and defaults
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 docs: correct plan test framework to JUnit 4
core uses JUnit 4 + AssertJ + Mockito, not JUnit 5 Jupiter (no
Jupiter engine on the classpath). Test tasks translate accordingly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 feat: add WebJarUrlProvider resolution seam
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 feat: register WebJarUrlProvider bean
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 feat: extend static content-type map for webjar assets
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 feat: serve webjar assets via static content loader
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 feat: add <s:webjar> tag and <@s.webjar> macro
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 docs: add generated tag reference for <s:webjar>
Annotation-processor-generated tag reference (attributes + description),
tracked like every other tag's docs under core/src/site/resources/tags/.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 fix: address final review (log level, resolveUrl traversal test, javadoc)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5640 refactor: address SonarCloud code smells
- getContentType: replace long if/else chain with a static extension->
MIME map (S3776 cognitive complexity)
- DefaultWebJarUrlProvider.split: return Optional<String[]> instead of a
null sentinel (S1168; Optional fits the reject semantics, empty-array
would not)
- serving tests: rename local 'loader' -> 'webJarLoader' to stop hiding
the ContentTypeProbe field (S1117)
- WebJarTest: use assertThat(writer).hasToString(...) (S5838)
S110 (WebJarTag inheritance depth) is inherent to the Struts tag base
class hierarchy shared by every tag; left as-is.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Add draft security threat model (THREAT_MODEL.md)
Generated-by: Claude Opus 4.8 (1M context)
* SECURITY.md: link to THREAT_MODEL.md for scanner/triager discoverability
Generated-by: Claude Opus 4.8 (1M context)
* Fix dangling §14 refs and tighten provenance in threat model
Address code-review findings on the THREAT_MODEL.md draft:
- Add the missing §14 Q-env and Q-egress open questions, so every
*(inferred)* claim that cites them now resolves (restores the
"each inferred claim has a matching §14 question" invariant).
- Tag the two previously bare *(inferred)* claims (examples/showcase,
on-path attacker) with their matching question IDs.
- Soften the §1 header: drop the ASF Security team / PMC authorship
attribution on an unratified draft; state it is drafted for PMC review.
- Correct the OGNL Java Security Manager wording: SecurityManager is
deprecated for removal since JDK 17 and permanently disabled in JDK 24,
not simply "does not work on JDK 21+".
- AGENTS.md: point the Assess step at THREAT_MODEL.md's disposition guide
so the triage wrapper links the model directly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Lukasz Lenart <lukaszlenart@apache.org>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5256 docs: design to decouple FreeMarker whitespace stripping from devMode
Fixes s:textarea rendering blank lines and HTML whitespace bloat in devMode
by honoring struts.freemarker.whitespaceStripping unconditionally.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5256 docs: implementation plan to decouple whitespace stripping from devMode
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5256 test: prove whitespace stripping wrongly disabled in devMode
* WW-5256 fix(freemarker): honor whitespaceStripping regardless of devMode
* WW-5256 docs: drop devMode note from whitespaceStripping constant
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Implement test for status code 200 with HTML escaping
* Escape HTML in ServletRedirectResult response
Escape HTML in the final location before writing to the response.
Collapse 12 overlapping cache tests to 5 focused ones, replace the
~80-entry JDK class-name literal with a synthetic-name loop bounded by
the inner-cache limit, and drop reflection from the behavioral tests
(load-count assertions only). Reflection is retained solely in the two
size-bound tests, where Caffeine exposes no public seam.
Production ConfigParseUtil caching logic is unchanged.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5632 docs: add commons-fileupload2 milestone-hardening design spec
Design for hardening the commons-fileupload2 dependency against
milestone binary-incompatibility (manage -core, activate a scoped
enforcer rule, add a runtime API guard in AbstractMultiPartRequest).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5632 docs: add implementation plan for fileupload2 milestone hardening
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5632 build(deps): manage commons-fileupload2-core alongside jakarta-servlet6
Pin both commons-fileupload2 artifacts to a single
commons-fileupload2.version property so the volatile -core API can no
longer skew from -jakarta-servlet6 in the reactor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5632 build: enforce a single commons-fileupload2 version
Activate maven-enforcer-plugin (previously dormant in pluginManagement)
with a fileupload-scoped bannedDependencies rule so any divergent
commons-fileupload2 version fails the build early.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5632 fix(fileupload): fail fast on incompatible commons-fileupload2 API
Verify once per JVM that the fileupload size-limit setters exist and
throw a clear StrutsException reporting the core/jakarta version skew,
replacing an opaque deep-stack NoSuchMethodError in downstream runtimes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5632 fix(fileupload): make API-verification guard static
Resolve Sonar java:S2696 (instance method writing a static field) by
making ensureFileUploadApiVerified() static; verification is JVM-global.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5630 - Performance Issue SecurityMemberAccess
* Add size bound cache, 50, for Class lookup
* Add unit test
Code generated by Copilot
* WW-5630 - Add additional UT
* WW-5630 - Add UT for non-existent class
* WW-5630 - Review feedback changes
* Cache ClassLoader directly
* Use weakKeys and weakValues
* Comment on the ClassLookupException
* Additional Unit Tests
Assistance in coding using co-pilot
* WW-5630 - Additional review
* Limit outer, Classloader, to 25. Ensure memory bounding.
* Limit inner, Classes, to 50. Ensure memory bounding.
* Additional UTs
With co-pilot assitance
* WW-5631 feat(chaining): add struts.chaining.requireAnnotations constant
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 feat(chaining): default struts.chaining.requireAnnotations=false
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 test(chaining): add annotated/unannotated chaining fixtures
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 test(chaining): add failing @StrutsParameter enforcement tests
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 feat(chaining): enforce @StrutsParameter on target when opted in
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 refactor(chaining): align requireAnnotations parsing with BooleanUtils
Use BooleanUtils.toBoolean for the chaining requireAnnotations flag so it
accepts the same values (yes/on/1) as the sibling
struts.parameters.requireAnnotations switch, and unify the enforcement WARN
message prefix.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 test(chaining): cover includes interaction and proxied target
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 docs(chaining): document struts.chaining.requireAnnotations
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 test(chaining): cover fail-closed introspection; clarify target==action
Add a test asserting nothing is copied when the target action cannot be
introspected (fail-closed), and document why isAuthorized is called with
target == action for chaining (no ModelDriven exemption).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5631 fix(chaining): address SonarCloud findings
- Mark injected parameterAuthorizer/ognlUtil fields transient (S1948);
they are re-injected by the container, not serialized.
- Extract per-object copy into copyObjectToAction so the copyStack loop
uses no break/continue (S135); fail-closed path now returns from the
helper instead of continuing the loop.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Add an agent skill for triaging privately-disclosed security reports:
research each claim from source without trusting the reporter, verify
effective runtime defaults (config overrides field initializers), avoid
introducing unverified facts into responses, and frame findings as
vulnerability vs. operator responsibility.
Developed test-first: a baseline run produced contradictory, unverified
claims about defaults; the skill closes that gap and was verified to also
avoid over-correcting into reflexive rejection of valid reports.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The `cooldown` block expects `default-days`, not `default`. Using the
wrong key fails schema validation, causing Dependabot to silently fall
back to the previously valid config — which still targets
`release/struts-6-8-x` instead of `support/struts-6-x-x`.
Also adds a 3-day cooldown to the `main` maven entry for consistency.
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Closes the test gap noted in the WW-5535 research: no integration test
exercised HttpMethodInterceptor against a real DefaultActionProxy resolving
a wildcard action with an unannotated method.
Uses xwork-test-allowed-methods.xml's existing <action name="Wild-*"
method="{1}"> on HttpMethodsTestAction. URL "Wild-execute" resolves to
ActionSupport.execute() (no method-level HTTP annotation); the class-level
@AllowedHttpMethod(POST) must still reject GET end-to-end.
Together with the prior MockActionProxy regression tests, this locks in
both halves of the fix:
- DefaultActionProxy.resolveMethod() sets isMethodSpecified()=true for
wildcard-resolved methods (WW-5535 / #1592)
- HttpMethodInterceptor falls back to class-level annotations when the
resolved method is unannotated (#1690)
The WW-5535 fix (commit 4d2eb93) corrected isMethodSpecified() for wildcard-resolved
methods but introduced a structural gap in HttpMethodInterceptor.intercept().
The if/else-if structure made the class-level annotation check unreachable whenever
isMethodSpecified()=true and the resolved method carries no method-level annotation:
if (isMethodSpecified()) {
if (isAnnotatedBy(method)) { ... }
// falls through silently
} else if (isAnnotatedBy(class)) { ... } // never reached
return invocation.invoke(); // no enforcement
Fix: convert else-if to standalone if so the class-level check is always evaluated
as a fallback when the method itself has no annotation. Method-level annotations
still take precedence (checked first).
Add two regression tests covering the wildcard-resolved unannotated method scenario.
Co-authored-by: g0w6y <g0w6y@users.noreply.github.com>
* WW-5627 add ParameterAllowlister interface and STRUTS_PARAMETER_ALLOWLISTER constant
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5627 add OgnlParameterAllowlister default implementation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5627 register ParameterAllowlister bean in struts-default DI
* WW-5627 delegate ParametersInterceptor OGNL allowlisting to OgnlParameterAllowlister
Also register ParameterAllowlister in DefaultConfiguration bootstrap
factories so it is available in test containers (parallel to how
ParameterAuthorizer was already registered there).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5627 test(cookie): failing test for unannotated setter skip
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5627 gate CookieInterceptor cookie injection through ParameterAuthorizer
Adds a 5-arg `populateCookieValueIntoStack(name, value, map, stack, action)` hook
that runs cookie writes through `ParameterAuthorizer.isAuthorized` and primes
`ThreadAllowlist` via `ParameterAllowlister` for nested paths, then delegates
to the legacy 4-arg form. The 4-arg form is `@Deprecated(since="7.2.0")` but
its body is unchanged, so existing subclass overrides automatically receive
only authorized cookies. Default-config behavior is preserved because the
authorizer short-circuits when `requireAnnotations=false`.
Existing `CookieInterceptorTest` instantiates `new CookieInterceptor()` rather
than going through the container, leaving the new injected fields null. Wires
explicit pass-through lambdas through a `disableAuthorizationGate(...)` helper
so those tests continue to exercise default-config behavior.
* WW-5627 cover CookieInterceptor authorization matrix in CookieInterceptorAnnotationTest
* WW-5627 docs(cookie): document new 5-arg extension hook and deprecation
* WW-5627 wire OgnlParameterAllowlister in StrutsParameterAnnotationTest fixture
* WW-5627 address SonarCloud findings on PR #1681
- S1948: mark transient on the new ParameterAuthorizer/ParameterAllowlister
fields in CookieInterceptor and ParametersInterceptor (the host classes
are Serializable; the injected services are not).
- S1874: suppress the deprecation warning on the new 5-arg
populateCookieValueIntoStack — the delegation to the deprecated 4-arg
form is the contract that lets existing subclass overrides participate.
- S3776: extract `allowlistViaPropertyDescriptor` and
`allowlistViaPublicField` from `OgnlParameterAllowlister.allowlistAuthorizedPath`
to drop cognitive complexity below the threshold.
- S1068: remove the unused `mapping` test fixture field.
* WW-5627 clarify ParameterAllowlister contract and tidy ParametersInterceptor
Rename `ParameterAllowlister#allowlistAuthorizedPath` to `primeAllowlistForPath`
to make the contract explicit: the SAM is a side-effect-only priming hook that
runs after `ParameterAuthorizer#isAuthorized` has already decided. A no-op
return means "no priming needed or possible", never "rejected". The interface
name stays channel-agnostic; only the impl class (`OgnlParameterAllowlister`)
binds the priming to OGNL's `ThreadAllowlist`.
Add a `LOG.debug` in `OgnlParameterAllowlister` for the case where authorization
passed but no `@StrutsParameter` could be located on the root property
(e.g. `ModelDriven` models without per-property annotations) so the
authorize-vs-prime gap is observable instead of surfacing later as an opaque
OGNL traversal failure.
Drop the dead `performOgnlAllowlisting` pass-through and its unused `paramDepth`
parameter from `ParametersInterceptor` — the depth check is already enforced
inside `OgnlParameterAllowlister.primeAllowlistForPath`, so the outer guard was
a redundant computation.
No behavior change.
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore(agents): defines a new AGENTS.md focused on reporting vulnerabilities
* chore(security): makes SECURITY.md the source of truth
* Fix typos in SECURITY.md and minor grammar correction
* chore(security): clarify public GitHub repo PoC counts as disclosure
Per @ppkarwasz review on #1680: expand the PoC bullet to make explicit
that pushing a PoC to a public GitHub repo, gist, fork, or branch is
public disclosure, and note that private repos require granting access
to each PMC member individually.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: René Gielen <rgielen@apache.org>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5626 spike: validate Jackson per-property authorization mechanism
Validates that the Approach C design is feasible before committing to a detailed
implementation plan. Wraps each SettableBeanProperty via BeanDeserializerModifier;
intercepts deserializeAndSet to authorize against a path built from a ThreadLocal
Deque; uses skipChildren() to discard unauthorized values; uses [0] suffix for
collection/map/array elements to match ParametersInterceptor depth semantics.
Findings:
- Delegating base class via 'protected delegate' field is the right pattern
- addOrReplaceProperty(prop, true) is the correct builder API
- Reject-at-parent skips all nested deserialization (better security than two-phase
copy: setter side effects on unauthorized properties never fire)
- JavaType#isCollectionLikeType/isMapLikeType/isArrayType detects the indexed-path case
Spike is kept under .../spike/ as a learning artifact; it will be replaced by
production code + tests in subsequent commits.
* WW-5626 add ParameterAuthorizationContext for deserializer-level authorization
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5626 address review feedback on ParameterAuthorizationContext
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5626 add AuthorizationAwareContentTypeHandler marker interface
* WW-5626 add AuthorizingSettableBeanProperty for Jackson per-property authorization
* WW-5626 add ParameterAuthorizingModule installing the property wrapper on Jackson mappers
* WW-5626 register ParameterAuthorizingModule on default Jackson REST handlers
* WW-5626 use AuthorizationAwareContentTypeHandler path when handler supports it
* WW-5626 add integration tests proving the new Jackson authorization path is used
* WW-5626 deprecate XStreamHandler in favor of JacksonXmlHandler
* WW-5626 remove Jackson auth spike; replaced by production tests
* WW-5626 make JuneauXmlHandler authorization-aware via post-parse walk
Implements AuthorizationAwareContentTypeHandler. When ParameterAuthorizationContext
is active (set by ContentTypeInterceptor when requireAnnotations=true), the handler
walks the parsed result tree and copies only authorized properties to the target,
descending into nested beans/collections/maps/arrays with indexed-path semantics
([0] suffix) for parity with ParametersInterceptor.
Note: Juneau parses the entire result tree before our walk runs, so setter side
effects on transient nested objects can fire even for unauthorized properties —
those transient objects are then discarded. This is functionally equivalent to the
legacy two-phase copy in ContentTypeInterceptor; only the Jackson handlers achieve
the stronger guarantee where unauthorized subtrees are never instantiated at all
(they use Jackson's BeanDeserializerModifier + skipChildren).
When no context is bound (default config), behavior is unchanged: parser.parse +
BeanUtils.copyProperties.
* WW-5626 add JuneauXmlHandler integration tests for @StrutsParameter authorization
* WW-5626 test(rest): cover JuneauXmlHandler post-parse walk for collections, maps, arrays
Sonar reported 51 uncovered new lines in JuneauXmlHandler (48.8% coverage on the
post-parse authorization walk — the security-critical code path the branch exists
to introduce). Add integration coverage for the previously-uncovered branches:
- collection-of-scalars (List<String> tags)
- collection-of-beans (List<Address> addresses)
- map-of-scalars (Map<String,String> attributes)
- array-of-scalars (String[] aliases)
- empty collection
- malformed XML wrapped as IOException
Also drop two unnecessary casts (Sonar S1905) on lines 243/252 — the unchecked
conversion happens at the return statement, the explicit casts were redundant
under the existing @SuppressWarnings("unchecked").
Add @Override on the inline AnyConstraintMatcher.matches override (Sonar S1161).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5626 test(rest): cover AuthorizingSettableBeanProperty builder-path deserialization
Sonar reported 11 uncovered new lines on AuthorizingSettableBeanProperty (66.7%
coverage). All 11 are in deserializeSetAndReturn — the alternate Jackson entry
point used for builder-pattern deserialization, never triggered by setter-based
fixtures like Person.
Add an @JsonDeserialize(builder=...) fixture (ImmutablePerson) that forces
Jackson to use BuilderBasedDeserializer, which dispatches property writes
through deserializeSetAndReturn. Three new tests exercise the path:
inactive-context pass-through, top-level authorization, and full rejection.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* WW-5626 refactor(rest): extract helpers from ContentTypeInterceptor.intercept
Sonar S3776 flagged intercept() at cognitive complexity 16 (limit 15). Extract
the body-handling branches into named helpers:
- openBodyReader: encoding-aware reader from the request InputStream
- applyRequestBody: dispatcher between requireAnnotations on/off paths
- applyWithAuthorizationContext: bind + delegate + unbind for AuthorizationAware handlers
- applyTwoPhaseDeserialize: legacy fresh-instance + copyAuthorizedProperties path
intercept() drops to ~12 lines and reads as a flat sequence: resolve target,
delegate body application, invoke. Each helper carries the comment that
explains the security model for its branch.
Add @Override on the inline AnyConstraintMatcher.matches override (Sonar S1161).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* WW-5626 add ParameterAuthorizer#resolveTarget for centralized ModelDriven resolution
Move the ValueStack peek logic that derives the target object from action+ModelDriven
state out of ParametersInterceptor and into ParameterAuthorizer. Callers that need
both authorization and the resolved target (for downstream OGNL allowlisting) can
now call resolveTarget once and reuse the result.
* WW-5626 delegate ModelDriven target resolution to ParameterAuthorizer
Replace the inline ValueStack peek in ParametersInterceptor#isParameterAnnotatedAndAllowlist
with a call to ParameterAuthorizer#resolveTarget. The ModelDriven import is no longer
needed in this class.
* WW-5626 defensively skip non-String JSON keys in authorization filter
The (String) cast in filterUnauthorizedKeysRecursive threw ClassCastException
for any custom JSONReader producing non-String keys. Replace with an instanceof
pattern that debug-logs and skips entries whose key cannot be converted to a
parameter path.
* WW-5626 add real JacksonJsonHandler integration tests for @StrutsParameter filtering
The existing ContentTypeInterceptorTest uses mock ContentTypeHandlers, so its
requireAnnotations=true tests verify only that intercept() returns SUCCESS — they
assert nothing about which properties were actually filtered. These integration
tests use a real JacksonJsonHandler + a real StrutsParameterAuthorizer to verify
end-to-end property-level filtering for top-level annotated/unannotated properties
and nested properties at varying authorized depths.
The SecureRestAction fixture documents a semantic divergence: REST's recursive
copy authorizes each path level independently, so depth-0 authorization on the
top-level property requires @StrutsParameter on the setter even when nested
field access is the actual goal. ParametersInterceptor only requires the getter
annotation. This divergence is tracked for the Approach C refactor.
* WW-5626 make ParameterAuthorizer#resolveTarget a default method to preserve SAM
Making resolveTarget abstract broke ParameterAuthorizer as a functional interface,
which the existing JSON and REST plugin tests rely on for lambda-based stubs:
interceptor.setParameterAuthorizer((parameterName, target, action) -> true);
The default returns the action unchanged — adequate for lambda test stubs whose
authorization decisions don't depend on the resolved target. The production
implementation (StrutsParameterAuthorizer) overrides this with the proper
ModelDriven value-stack peek.
Lukasz Lenart