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

src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java

@@ -8,10 +8,12 @@
 import java.io.IOException;
 import java.io.StringReader;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
@@ -155,6 +157,9 @@ public static Result extract(JArtifact artifact, String text, boolean captureVal
                         artifact.getId(), "env", recognizeComposeEnv(flat), captureValue, false,
                         k -> CanId.configKeyEnvDualMintId(artifact.getId(), k)));
             } else if ("xml".equals(artifact.getFormat())) {
+                if (!isConfigXml(artifact.getPath())) {
+                    return new Result(List.of(), true);
+                }
                 keys = extractXml(artifact.getId(), text, captureValue);
             } else {
                 return new Result(List.of(), true);
@@ -166,6 +171,115 @@ public static Result extract(JArtifact artifact, String text, boolean captureVal
         }
     }
 
+    /**
+     * XML basenames worth flattening. Discovery classifies every {@code *.xml} in a repository as an
+     * artifact, and the flattener mints a key per element path -- which is what configuration looks
+     * like, and also what a data file looks like. On ThingsBoard v4.0 that produced 39,551 keys from
+     * 453 XML files, 30,408 of them from LwM2M object registries under {@code src/main/data/}: static
+     * data resources, not settings (#210).
+     *
+     * <p>No size threshold separates the two -- that repository's {@code pom.xml} legitimately carries
+     * 1332 keys while a data registry carries 615 -- so recognition is by name, not by volume. A file
+     * that is not recognized stays in the artifact inventory with its {@code source}, {@code sha256}
+     * and roles intact; only flattening is skipped, so nothing disappears from the file listing and
+     * the result is {@code ok=true}, not a parse failure.
+     *
+     * <p>A name list alone is not enough, because vendor deployment descriptors are open-ended
+     * ({@code ibm-web-bnd.xml}, {@code jboss-web.xml}, {@code weblogic.xml}, ...) -- daytrader8's two
+     * {@code ibm-web-*.xml} descriptors are real configuration and no plausible list contains them.
+     * So location counts too: {@code WEB-INF/} and {@code 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 rather than by guess. Data resources do not live there.
+     *
+     * <p>The accepted cost is a project whose configuration lives in an unlisted XML name outside
+     * those directories. That file is still inventoried and still discoverable; it just contributes
+     * no {@code config_keys}. Adding a name here is the fix when one turns up -- prefer that to
+     * widening the rule to "any XML under a config-ish directory", which is how the unbounded
+     * behaviour arose in the first place.
+     */
+    private static final Set<String> CONFIG_XML_NAMES = new HashSet<>(Arrays.asList(
+            // build / project
+            "pom.xml", "build.xml", "settings.xml", "toolchains.xml", "ivy.xml", "assembly.xml",
+            // servlet / Java EE descriptors
+            "web.xml", "ejb-jar.xml", "application.xml", "application-client.xml", "persistence.xml",
+            "beans.xml", "faces-config.xml", "orm.xml", "validation.xml", "portlet.xml", "ra.xml",
+            // containers and app servers
+            "server.xml", "context.xml", "tomcat-users.xml", "standalone.xml", "domain.xml",
+            "jboss-web.xml", "jboss-ejb3.xml", "jboss-deployment-structure.xml", "weblogic.xml",
+            "glassfish-web.xml", "glassfish-application.xml", "blueprint.xml",
+            // frameworks
+            "struts.xml", "tiles.xml", "hibernate.cfg.xml", "quartz.xml", "ehcache.xml",
+            "liquibase.xml", "bindings.xml", "sun-jaxb.episode",
+            // logging
+            "logback.xml", "logback-test.xml", "logback-spring.xml",
+            "log4j.xml", "log4j2.xml", "log4j2-spring.xml", "log4j2-test.xml",
+            // static analysis / tooling
+            "checkstyle.xml", "spotbugs.xml", "findbugs-exclude.xml", "pmd.xml", "suppressions.xml",
+            // misc
+            "config.xml", "androidmanifest.xml"));
+
+    /**
+     * Name stems that mark a configuration file whatever separator precedes them. Matching only
+     * hyphenated forms was the first attempt and it missed the canonical spellings of the very
+     * frameworks it was meant to cover: {@code applicationContext.xml} (Spring) and {@code
+     * UserMapper.xml} (MyBatis) both failed a {@code -context.xml}/{@code -mapper.xml} test, while
+     * {@code spring-servlet.xml} passed. Comparing against the lowercased stem catches camelCase,
+     * hyphenated and underscored spellings alike.
+     *
+     * <p>Deliberately excludes short stems that occur inside ordinary words -- {@code "ds"} would
+     * admit {@code records.xml} -- so those stay hyphen-only in {@link #CONFIG_XML_SUFFIXES}.
+     */
+    private static final List<String> CONFIG_XML_STEMS =
+            Arrays.asList("context", "config", "configuration", "settings", "beans", "servlet",
+                    "mapper", "changelog", "datasource");
+
+    /** Suffixes that must keep their separator, being too short or too common to match bare. */
+    private static final List<String> CONFIG_XML_SUFFIXES = Arrays.asList("-ds.xml", ".hbm.xml");
+
+    /**
+     * Whether an XML artifact is a configuration file worth flattening: a recognized basename, a
+     * conventional suffix, or any XML in a descriptor directory. A suffix never matches bare --
+     * {@code notweb.xml} must not pass as {@code web.xml}.
+     *
+     * @param path the artifact's project-relative path (the directory part carries the location signal)
+     */
+    static boolean isConfigXml(String path) {
+        String lower = basename(path).toLowerCase(Locale.ROOT);
+        if (CONFIG_XML_NAMES.contains(lower)) {
+            return true;
+        }
+        for (String suffix : CONFIG_XML_SUFFIXES) {
+            // A suffix match needs something in front of it, so ".hbm.xml" itself is not a match.
+            if (lower.length() > suffix.length() && lower.endsWith(suffix)) {
+                return true;
+            }
+        }
+        if (lower.endsWith(".xml")) {
+            String stem = lower.substring(0, lower.length() - ".xml".length());
+            for (String token : CONFIG_XML_STEMS) {
+                // The stem must be preceded by something, so a file named exactly "config.xml" is
+                // matched by name above rather than here, and "context.xml" likewise.
+                if (stem.length() > token.length() && stem.endsWith(token)) {
+                    return true;
+                }
+            }
+        }
+        return inDescriptorDirectory(path);
+    }
+
+    /** True when the file sits directly inside a {@code WEB-INF} or {@code META-INF} directory. */
+    private static boolean inDescriptorDirectory(String path) {
+        String normalized = path.replace('\\', '/');
+        int slash = normalized.lastIndexOf('/');
+        if (slash < 0) {
+            return false;
+        }
+        String parent = normalized.substring(0, slash);
+        int parentSlash = parent.lastIndexOf('/');
+        String dir = parentSlash < 0 ? parent : parent.substring(parentSlash + 1);
+        return "WEB-INF".equalsIgnoreCase(dir) || "META-INF".equalsIgnoreCase(dir);
+    }
+
     // ---- properties: java.util.Properties handles key=value/key:value, `\` continuations, and
     // `#`/`!` comments natively -- and last-wins, since a later duplicate key simply overwrites the
     // earlier one on load. Using the JDK parser buys all of that for free, at the cost of position
@@ -251,8 +365,7 @@ private static boolean isDigits(String s) {
 
     private static List<JConfigKey> extractXml(String artifactId, String text, boolean captureValue)
             throws ParserConfigurationException, SAXException, IOException {
-        Document doc = ManifestParsers.newSecureDocumentBuilderFactory()
-                .newDocumentBuilder()
+        Document doc = ManifestParsers.newConfigDocumentBuilder()
                 .parse(new InputSource(new StringReader(text)));
         Element root = doc.getDocumentElement();
         List<Entry> entries = new ArrayList<>();

src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java

@@ -16,8 +16,10 @@
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
+import org.xml.sax.ErrorHandler;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
 
 /**
  * Pure dependency-manifest and lockfile readers: text in, records out. No file I/O, no execution,
@@ -174,7 +176,7 @@ private static String kindForMavenDependency(String scope, boolean optional) {
     }
 
     private static DocumentBuilder newSecureDocumentBuilder() throws ParserConfigurationException {
-        return newSecureDocumentBuilderFactory().newDocumentBuilder();
+        return quiet(newSecureDocumentBuilderFactory().newDocumentBuilder());
     }
 
     /**
@@ -190,6 +192,11 @@ private static DocumentBuilder newSecureDocumentBuilder() throws ParserConfigura
      * toggles are kept as explicit defense in depth alongside it, and {@code
      * FEATURE_SECURE_PROCESSING} restricts external DTD/entity resource access (JAXP's {@code
      * accessExternalDTD}) independently of both.
+     *
+     * <p>A dependency manifest has no legitimate reason to declare a DOCTYPE, so this stays
+     * categorical. Configuration files do -- {@code logback.xml} routinely carries one -- and they
+     * are parsed through {@link #newConfigDocumentBuilder} instead, which permits an internal subset
+     * while keeping every external switch here identical.
      */
     static DocumentBuilderFactory newSecureDocumentBuilderFactory() throws ParserConfigurationException {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
@@ -200,6 +207,66 @@ static DocumentBuilderFactory newSecureDocumentBuilderFactory() throws ParserCon
         return factory;
     }
 
+    /**
+     * The parser for configuration files, which differs from {@link #newSecureDocumentBuilderFactory}
+     * in exactly one respect: an internal DOCTYPE subset is allowed.
+     *
+     * <p>Refusing every DOCTYPE rejects the whole document rather than the dangerous part. On
+     * ThingsBoard v4.0 that discarded 43 files, 35 of them {@code logback.xml} -- precisely the
+     * configuration this analyzer exists to report, silently reduced to {@code extraction=partial}
+     * with no keys. XXE needs an <em>external</em> reference to resolve, and all four external
+     * routes stay closed here (general entities, parameter entities, external DTD loading,
+     * XInclude), with {@code FEATURE_SECURE_PROCESSING} capping entity expansion so an internal
+     * "billion laughs" is refused rather than expanded. {@code setExpandEntityReferences(false)}
+     * additionally keeps entity bodies out of the walked tree.
+     *
+     * <p>Manifests deliberately do NOT use this: a pom.xml declaring a DOCTYPE has no legitimate
+     * shape, so it keeps the categorical refusal.
+     */
+    static DocumentBuilderFactory newConfigDocumentBuilderFactory() throws ParserConfigurationException {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+        factory.setXIncludeAware(false);
+        factory.setExpandEntityReferences(false);
+        return factory;
+    }
+
+    /** A configuration-file builder that does not narrate to stderr. */
+    static DocumentBuilder newConfigDocumentBuilder() throws ParserConfigurationException {
+        return quiet(newConfigDocumentBuilderFactory().newDocumentBuilder());
+    }
+
+    /**
+     * Silence the JDK parser's own commentary. With no error handler installed it prints every
+     * problem to {@code System.err} itself -- raw {@code [Fatal Error] :19:10: ...} lines that
+     * bypass {@link com.ibm.cldk.utils.Log}, tell the user nothing actionable, and on a large
+     * repository arrive dozens at a time. Failures already travel in each caller's
+     * {@code (records, ok)} result, so the thrown exception carries the signal; this removes only
+     * the duplicate narration.
+     */
+    static DocumentBuilder quiet(DocumentBuilder builder) {
+        builder.setErrorHandler(new ErrorHandler() {
+            @Override
+            public void warning(SAXParseException e) {}
+
+            @Override
+            public void error(SAXParseException e) throws SAXException {
+                throw e;
+            }
+
+            @Override
+            public void fatalError(SAXParseException e) throws SAXException {
+                throw e;
+            }
+        });
+        return builder;
+    }
+
     private static List<Element> directChildren(Element parent, String tagName) {
         List<Element> children = new ArrayList<>();
         NodeList nodes = parent.getChildNodes();

src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java

@@ -35,6 +35,87 @@ private static Optional<JConfigKey> find(List<JConfigKey> keys, String key) {
         return keys.stream().filter(k -> k.getKey().equals(key)).findFirst();
     }
 
+    // ---- xml: only recognized configuration shapes are flattened (#210) ----------------------
+
+    /** A data file shaped like the LwM2M object registries that produced 30k keys on ThingsBoard. */
+    private static String dataXml() {
+        StringBuilder b = new StringBuilder("<LWM2M><Object ObjectID=\"10326\">");
+        for (int i = 0; i < 40; i++) {
+            b.append("<Item ID=\"").append(i).append("\"><Name>n").append(i)
+                    .append("</Name><Type>Integer</Type></Item>");
+        }
+        return b.append("</Object></LWM2M>").toString();
+    }
+
+    @Test
+    void aRecognizedConfigXmlIsStillFlattened() {
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("src/main/webapp/WEB-INF/web.xml", "xml"),
+                "<web-app><context-param><param-name>mode</param-name></context-param></web-app>", true);
+        assertTrue(r.ok);
+        assertFalse(r.keys.isEmpty(), "web.xml is configuration and must still be flattened");
+    }
+
+    @Test
+    void pomXmlIsStillFlattened() {
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("pom.xml", "xml"),
+                "<project><properties><java.version>11</java.version></properties></project>", true);
+        assertTrue(r.ok);
+        assertFalse(r.keys.isEmpty(), "pom.xml carries real configuration");
+    }
+
+    @Test
+    void anUnrecognizedXmlYieldsNoKeysButIsNotAFailure() {
+        // The artifact stays in the inventory; only flattening is skipped. ok=true, exactly as for a
+        // format with no flattener -- marking it a parse failure would misreport a healthy file.
+        ConfigKeys.Result r = ConfigKeys.extract(
+                artifact("application/src/main/data/lwm2m-registry/10326.xml", "xml"), dataXml(), true);
+        assertTrue(r.ok, "skipping extraction is not a parse failure");
+        assertTrue(r.keys.isEmpty(), "a data registry is not configuration and must not be flattened");
+    }
+
+    @Test
+    void aSuffixMatchDoesNotAdmitAnArbitraryFile() {
+        // "notweb.xml" ends with "web.xml" but is not it; recognition is on the whole basename.
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("src/notweb.xml", "xml"), dataXml(), true);
+        assertTrue(r.ok);
+        assertTrue(r.keys.isEmpty(), "recognition must match the basename, not a suffix of it");
+    }
+
+    @Test
+    void aVendorDescriptorInWebInfIsRecognizedByItsLocation() {
+        // daytrader8 ships ibm-web-bnd.xml / ibm-web-ext.xml. Vendor descriptors are open-ended, so
+        // no name list catches them; WEB-INF is a spec-defined descriptor directory, so location does.
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("src/main/webapp/WEB-INF/ibm-web-bnd.xml", "xml"),
+                "<web-bnd><virtual-host name=\"default_host\"/></web-bnd>", true);
+        assertTrue(r.ok);
+        assertFalse(r.keys.isEmpty(), "an XML in WEB-INF is a deployment descriptor");
+    }
+
+    @Test
+    void anXmlInMetaInfIsRecognizedByItsLocation() {
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("src/main/resources/META-INF/anything.xml", "xml"),
+                "<a><b>1</b></a>", true);
+        assertTrue(r.ok);
+        assertFalse(r.keys.isEmpty(), "META-INF is a descriptor directory");
+    }
+
+    @Test
+    void aDescriptorDirectoryDeeperInThePathDoesNotCount() {
+        // Only the immediate parent counts: data nested far below a WEB-INF is not a descriptor.
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("WEB-INF/data/registry/10326.xml", "xml"),
+                dataXml(), true);
+        assertTrue(r.ok);
+        assertTrue(r.keys.isEmpty(), "the file must sit directly in the descriptor directory");
+    }
+
+    @Test
+    void aSuffixPatternStillMatchesItsFamily() {
+        ConfigKeys.Result r = ConfigKeys.extract(artifact("src/main/resources/app-context.xml", "xml"),
+                "<beans><bean id=\"a\"/></beans>", true);
+        assertTrue(r.ok);
+        assertFalse(r.keys.isEmpty(), "*-context.xml is a recognized Spring configuration shape");
+    }
+
     // ---- properties -------------------------------------------------------------------------
 
     @Test