Skip to content

fix(artifacts): flatten only configuration XML, not every XML in the repository - #212

Merged
rahlk merged 2 commits into
mainfrom
fix/issue-210-xml-config-allowlist
Sep 1, 2026
Merged

rahlk merged 2 commits into
mainfrom
fix/issue-210-xml-config-allowlist

Conversation

@rahlk

@rahlk rahlk commented Aug 31, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Discovery classifies every *.xml as an artifact, and ConfigKeys mints a key per element path. That is what configuration looks like — and equally what a data file looks like. On ThingsBoard v4.0: 39,551 keys from 453 XML files, of which 30,408 came from LwM2M object registries under src/main/data/. Those are static data resources, not settings.

The rule

Recognition by name and by location, never by volume:

  • a recognized basename (pom.xml, web.xml, persistence.xml, logback.xml, faces-config.xml, …)
  • a conventional suffix (-context.xml, -config.xml, -changelog.xml, …), never matching bare — notweb.xml must not pass as web.xml
  • any XML directly inside WEB-INF/ or META-INF/

An unrecognized XML keeps its artifact entry, source, sha256 and roles. Only flattening is skipped, and the result stays ok=true — skipping is not a parse failure.

The location rule is not decoration

My first implementation was name-only, and it silently dropped 41 keys from daytrader8 — its ibm-web-bnd.xml and ibm-web-ext.xml, which are real WebSphere deployment descriptors. Vendor descriptors are open-ended (ibm-*, jboss-*, weblogic-*, glassfish-*); no plausible name list contains them.

WEB-INF and META-INF are directories the servlet and Java EE specs define to hold descriptors, so an XML sitting directly in one is configuration by construction rather than by guess — and data resources do not live there. Only the immediate parent counts, so WEB-INF/data/registry/10326.xml is still excluded.

The issue's definition of done ("daytrader8 unchanged") is what caught this.

Measurements

before after
ThingsBoard config keys 42,768 8,447
daytrader8 analysis.json — byte-identical

Remaining ThingsBoard keys, all from genuine configuration: xml 5230, yaml 2829, properties 340, dockerfile 30, text 18.

Closes #210

…repository

Discovery classifies every *.xml in a repository as an artifact, and the
flattener mints a key per element path. That is what configuration looks like,
and equally what a data file looks like. On ThingsBoard v4.0 it produced 39,551
keys from 453 XML files, 30,408 of them from LwM2M object registries under
src/main/data -- static data resources, not settings.

No size threshold separates the two: that repository's pom.xml legitimately
carries 1332 keys while a data registry carries 615. So recognition is by name
and by location, never by volume.

A name list alone is not enough. Vendor deployment descriptors are open-ended,
and daytrader8's ibm-web-bnd.xml / ibm-web-ext.xml are real configuration that
no plausible list contains -- a name-only rule silently dropped 41 of its keys.
WEB-INF and META-INF are directories the servlet and Java EE specifications
define to hold descriptors, so an XML sitting directly in one is configuration
by construction. Data resources do not live there.

An unrecognized XML keeps its artifact entry, source, sha256 and roles; only
flattening is skipped, and the result stays ok=true rather than being reported
as a parse failure.

Measured: ThingsBoard config keys 42,768 -> 8,447, with the remaining XML keys
coming from genuine descriptors. daytrader8's analysis.json is byte-identical to
before this change.

Note for the issue: this removes meaningless keys, but it is NOT a size fix.
The document goes 480 MB -> 469 MB, because its bulk is the symbol table for
4131 files, not config keys. Anyone chasing output size should look there.

Closes #210
@rahlk rahlk self-assigned this Aug 31, 2026
@rahlk
rahlk requested a review from georgesafta August 31, 2026 20:50
…CTYPE configs

Two gaps found by probing the first allowlist against real framework names
rather than trusting it.

The suffix rules assumed a hyphen, so they matched spring-servlet.xml but missed
the canonical spellings of the very frameworks they targeted: applicationContext
.xml (Spring) and UserMapper.xml (MyBatis) both failed a -context.xml/-mapper.xml
test. Matching against the lowercased stem instead catches camelCase, hyphenated
and underscored spellings alike. Short stems that occur inside ordinary words are
excluded -- "ds" would admit records.xml -- so those keep their separator. The
name list also gains Ant, Jenkins, WildFly, OSGi, Tomcat, Quartz, Ehcache,
Hibernate mappings and the Spring Boot logging variants. Probed: 16 of 16 common
framework configs now flatten, and records.xml, productCatalog.xml and the data
registries still do not.

Separately, config files were being discarded wholesale. The parser refused any
DOCTYPE, which rejects the whole document rather than the dangerous part: on
ThingsBoard that silently reduced 43 files to extraction=partial with no keys, 35
of them logback.xml -- exactly the configuration this layer exists to report.

Configuration files now parse through their own factory that permits an internal
subset while keeping every external route closed: general entities, parameter
entities, external DTD loading and XInclude all stay off, FEATURE_SECURE_PROCESSING
still caps entity expansion, and entity references are not expanded into the tree.
XmlHardeningTest pins each of those separately, including that an external entity
cannot leak file contents and that a billion-laughs document is refused.

Manifests deliberately keep the categorical refusal. A pom.xml has no legitimate
reason to declare a DOCTYPE, and the four existing tests that attribute each
switch on that factory are untouched and still pass -- widening the shared factory
was my first attempt and it broke exactly those, which is what showed the split
was the right shape.

The parser also no longer narrates: an error handler replaces the JDK's raw
[Fatal Error] stderr lines, which bypassed Log and arrived 43 at a time.

Measured: ThingsBoard config keys 42,768 -> 9,352 with all 35 logback.xml now
extraction=full contributing 905 real keys, and stderr noise 43 lines -> 0.
daytrader8's analysis.json stays byte-identical.
@rahlk

rahlk commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up: the allowlist was too narrow, and DOCTYPE configs were being discarded

Probed the first allowlist against real framework names instead of trusting it. 12 of 16 common configs were skipped, including applicationContext.xml and UserMapper.xml — the canonical spellings of the two frameworks the suffix rules were written for.

The flaw was systematic: -context.xml and -mapper.xml assume a hyphen, so they matched spring-servlet.xml but missed the camelCase forms. Recognition is now against the lowercased stem, which catches camelCase, hyphenated and underscored spellings alike. Short stems that appear inside ordinary words are excluded — "ds" would admit records.xml — so those keep their separator. The name list also gains Ant, Jenkins, WildFly, OSGi, Tomcat, Quartz, Ehcache, Hibernate *.hbm.xml, and the Spring Boot logging variants.

Re-probed: 16 of 16 flatten, and records.xml, productCatalog.xml and the data registries still do not.

The bigger find: 35 logback.xml were being thrown away

Auditing the excluded set turned up something the allowlist did not cause. 43 ThingsBoard XML files declare a <!DOCTYPE>, 35 of them logback.xml — an allowlisted name. The parser refused any DOCTYPE outright, so those were reduced to extraction=partial with zero keys, while spraying raw [Fatal Error] lines at stderr through the JDK parser rather than Log.

Refusing every DOCTYPE rejects the whole document rather than the dangerous part. Configuration files now parse through their own factory permitting an internal subset only, with every external route still closed:

  • external general entities — off
  • external parameter entities — off
  • external DTD loading / accessExternalDTD — off
  • XInclude — off
  • FEATURE_SECURE_PROCESSING — on, so entity expansion stays capped
  • setExpandEntityReferences(false) — entity bodies never enter the walked tree

XmlHardeningTest pins each separately, including that an external entity cannot leak file contents and that a billion-laughs document is refused rather than expanded.

Manifests keep the categorical refusal. A pom.xml has no legitimate reason to declare a DOCTYPE. Widening the shared factory was my first attempt and it broke the four existing tests that attribute each switch on that factory — which is what showed the split was the right shape. Those four are untouched and still pass.

Updated measurements

before after
ThingsBoard config keys 42,768 9,352
logback.xml (35 files) 0 keys, partial 905 keys, full
stderr [Fatal Error] lines 43 0
daytrader8 analysis.json — byte-identical

Suite: 501 tests, 1 failure (the pre-existing Docker CodeAnalyzerIntegrationTest), 4 skips.

Standing caveat

An allowlist is still an allowlist — a project whose configuration lives in an unlisted XML name outside WEB-INF/META-INF contributes no config_keys. The file stays inventoried and discoverable, and adding a name is the fix when one turns up. If that trade stops being acceptable, the shape-based alternative (judging by parsed content rather than filename) is the real answer and deserves its own design.

@georgesafta georgesafta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm ok with a curated list for now

@rahlk
rahlk merged commit 7312929 into main Sep 1, 2026
@rahlk
rahlk deleted the fix/issue-210-xml-config-allowlist branch September 1, 2026 21:21
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.

fix(artifacts): every XML data file becomes config keys — 42,768 keys on ThingsBoard, most of them not configuration

2 participants