The disclosure rules only forbade publishing exploit/PoC code, so a
contributor who opens a public PR that fixes or hints at a suspected
vulnerability reads them as satisfied — the fix itself telegraphs the
weakness before a fixed release exists.
Add a dedicated "Do not disclose through a pull request, commit, or issue"
section directing reporters to email security@struts.apache.org first, and
extend the PoC rule in Report Quality Rules to state that a fix, patch, or
hardening change is a public disclosure in the same way a PoC is. Aligns
SECURITY.md with the rule already stated in CLAUDE.md/AGENTS.md.
🤖 Generated by AI Assistant
Add two tests to JSONInterceptorTest exercising the nested-object path for
the name/value filtering added in WW-4858:
- testAcceptedNamePatternRejectsNestedKey: accepted name patterns are raw
full-match regexes with no hierarchy expansion, so the intermediate node
("bean") must itself match an accepted pattern or the whole subtree is
dropped before the leaf is visited.
- testIncludePropertiesAppliedToNestedInputWhenEnabled: include patterns do
expand across the hierarchy, so "bean.stringField" also matches the
intermediate "bean" and the nested leaf populates while the excluded
sibling "bean.intField" is dropped.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(json): confine StrutsJSONWriter write state to the writing thread
JSONUtil obtains its JSONWriter once via @Inject and reuses that same
instance across every concurrent response handled by that JSONResult/
JSONInterceptor configuration. StrutsJSONWriter kept its output buffer,
cyclic-reference stack, root object, and expression-path state
(buf/stack/root/buildExpr/exprStack/excludeProperties/
includeProperties/excludeNullProperties) as plain instance fields, all
reset in place at the start of write().
Two concurrent write() calls on the same instance therefore race on
that reset: one call's in-progress buffer can be wiped and overwritten
by a second, unrelated concurrent call before the first call reads it
back via buf.toString(), so one request's serialized JSON can be
returned as a completely different, concurrently-served request's
response body.
Move buf/stack/root/buildExpr/exprStack/excludeProperties/
includeProperties/excludeNullProperties into a WriteState confined to
a ThreadLocal, scoped to a single write() call. Method signatures and
behavior are otherwise unchanged so existing StrutsJSONWriter
subclasses keep working; ignoreHierarchy/dateFormat/enumAsBean/
excludeProxyProperties stay as plain instance fields since they are
set to the same value on every call for a given writer configuration
and are safe to share.
* test(json): raise writer concurrency test to 16 threads for reliable repro
Verified independently that the 2-thread version can miss the race on
machines with more cores than contending threads (with no CPU
contention, the OS scheduler has no need to preempt either thread
mid-call, so the corruption window is rarely hit): 0 reproductions in
8 reruns against unpatched code on a 10-core machine. Sixteen threads
reproduced it reliably (44,646/320,000 corrupted responses against
unpatched StrutsJSONWriter), and confirmed zero corruption against the
fix under the same load.
---------
Co-authored-by: g0w6y <g0w6y@users.noreply.github.com>
* fix(json): confine StrutsJSONReader parse state to the parsing thread
JSONInterceptor obtains its JSONReader once via @Inject and reuses that
same instance across every concurrent request handled by that
interceptor. StrutsJSONReader kept its parse cursor, token buffer and
nesting-depth counter (used to enforce maxDepth/maxElements/
maxStringLength/maxKeyLength) as plain instance fields, so two
concurrent read() calls on the same instance tore each other's state:
one request's depth counter could be decremented by an unrelated
concurrent request finishing its own parse, letting payloads deeper
than the configured maxDepth through, and the shared character cursor
and string/number buffer let fragments of one request's JSON body leak
into a different, concurrently-parsed request's result.
Move the cursor, current character, token, buffer and depth into a
ParseState confined to a ThreadLocal, scoped to a single read() call.
Method signatures and behavior are otherwise unchanged so existing
StrutsJSONReader subclasses keep working; the limit fields
(maxElements/maxDepth/maxStringLength/maxKeyLength) stay as plain
instance fields since they are set to the same value on every call for
a given interceptor configuration and are safe to share.
* test(json): raise reader concurrency test to 16 threads for reliable repro
Verified independently that the 2-thread version can miss the race on
machines with more cores than contending threads (with no CPU
contention, the OS scheduler has no need to preempt either thread
mid-call, so the corruption window is rarely hit): 0 reproductions in
8 reruns against unpatched code on a 10-core machine. Sixteen threads
reproduced both symptoms reliably against unpatched StrutsJSONReader
(81 cross-thread data leaks and 79 maxDepth bypasses out of 160,000
attempts), and confirmed zero of either against the fix under the
same load. Combined the two prior tests into one, since both symptoms
come from the same shared parse state and are naturally checked
together per thread.
---------
Co-authored-by: g0w6y <g0w6y@users.noreply.github.com>
* WW-4858 docs(json): design for honoring parameter filtering during JSON population
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 docs(json): implementation plan for JSON parameter filtering
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 feat(json): enforce excluded/accepted name patterns on JSON population
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 feat(json): enforce param-name max length on JSON population
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 feat(json): honor ParameterNameAware and ParameterValueAware on JSON population
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 feat(json): add opt-in excluded/accepted value patterns on JSON population
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 feat(json): opt-in applying excludeProperties/includeProperties to JSON input
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-4858 test(json): cover nested and list-element paths; clarify filter comments
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5641 docs: design spec for JSON writer/reader override regression
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5641 docs: implementation plan for JSON writer/reader override fix
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* WW-5641 fix: run JSON bean-selection from struts-deferred.xml
The JSON plugin declared <bean-selection> in struts-plugin.xml, which runs
at plugin-parse time, before the application struts.xml is folded in. That
froze the JSONWriter/JSONReader default binding to StrutsJSONWriter/Reader,
so struts.json.writer / struts.json.reader overrides were ignored.
Move the element to struts-deferred.xml, which Dispatcher loads last (after
the app config and core's StrutsBeanSelectionProvider), so the alias honors
the override. Mirrors the velocity plugin. JSONUtil is unchanged from main.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* 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>
Lukasz Lenart