From 6eef4eee253bffdeb15e06158f7882df28d119b9 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Sun, 30 Aug 2026 21:50:57 -0400 Subject: [PATCH 01/19] feat(artifacts): schema models for artifact, dependency and config key --- src/main/java/com/ibm/cldk/schema/CanId.java | 22 +++++++ .../com/ibm/cldk/schema/JApplication.java | 13 ++++ .../java/com/ibm/cldk/schema/JArtifact.java | 53 +++++++++++++++ .../java/com/ibm/cldk/schema/JConfigKey.java | 36 ++++++++++ .../java/com/ibm/cldk/schema/JDependency.java | 54 +++++++++++++++ .../ibm/cldk/schema/ArtifactModelTest.java | 65 +++++++++++++++++++ 6 files changed, 243 insertions(+) create mode 100644 src/main/java/com/ibm/cldk/schema/JArtifact.java create mode 100644 src/main/java/com/ibm/cldk/schema/JConfigKey.java create mode 100644 src/main/java/com/ibm/cldk/schema/JDependency.java create mode 100644 src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java diff --git a/src/main/java/com/ibm/cldk/schema/CanId.java b/src/main/java/com/ibm/cldk/schema/CanId.java index fc5cdfff..ffc85ea2 100644 --- a/src/main/java/com/ibm/cldk/schema/CanId.java +++ b/src/main/java/com/ibm/cldk/schema/CanId.java @@ -46,4 +46,26 @@ public static String ordinalId(String callableId, String tag) { public static String externalId(String appName, String binaryType, String signature) { return applicationId(appName) + "/@external/" + binaryType + "/" + signature; } + + /** + * {@code can://artifact//} — a language-neutral artifact id. The {@code artifact} + * segment is deliberately chosen over {@code java} so a sibling-language analyzer scanning the same + * repository lands on the same node rather than a duplicate. + */ + public static String artifactId(String appName, String relPath) { + return "can://artifact/" + appName + "/" + relPath; + } + + /** + * {@code @key/} — a configuration key nested under its defining artifact, + * making it discoverable and addressable as a sub-node. + */ + public static String configKeyId(String artifactId, String dottedKey) { + return artifactId + "@key/" + dottedKey; + } + + /** {@code pkg:maven//} — a two-segment Package URL for Maven coordinates. */ + public static String purlMaven(String group, String name) { + return "pkg:maven/" + group + "/" + name; + } } diff --git a/src/main/java/com/ibm/cldk/schema/JApplication.java b/src/main/java/com/ibm/cldk/schema/JApplication.java index 3f558c74..27ff173b 100644 --- a/src/main/java/com/ibm/cldk/schema/JApplication.java +++ b/src/main/java/com/ibm/cldk/schema/JApplication.java @@ -29,4 +29,17 @@ public class JApplication { /** L4 {@code formal_out → actual_out} edges (global ordinals); null (absent) below level 4. */ private List paramOut; + + /** + * The repository-artifact layer: build manifests, configuration files, and other non-source + * artifacts indexed by repo-relative path. {@code null} (absent) when the layer produces no + * artifacts. + */ + private Map artifacts; + + /** + * Dependencies declared in the repository's artifacts, indexed by their canonical purl or + * project-level id. {@code null} (absent) when the layer produces no dependencies. + */ + private List dependencies; } diff --git a/src/main/java/com/ibm/cldk/schema/JArtifact.java b/src/main/java/com/ibm/cldk/schema/JArtifact.java new file mode 100644 index 00000000..17757f3d --- /dev/null +++ b/src/main/java/com/ibm/cldk/schema/JArtifact.java @@ -0,0 +1,53 @@ +package com.ibm.cldk.schema; + +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +/** + * A build or configuration artifact in the repository — a build manifest, config file, Dockerfile, + * CI definition, or other non-source file. The {@code id} is the canonical {@code can://artifact/} + * path, and {@code path} is its repo-relative location (also the map key in {@code application.artifacts}), + * allowing consumers to navigate back to the filesystem. + * + *

{@code format} is free-vocabulary and identifies the parser: {@code xml}, {@code yaml}, + * {@code json}, {@code properties}, {@code gradle}, {@code dockerfile}, {@code text}, {@code binary}. + * {@code roles} are also free-vocabulary — {@code build}, {@code ci}, {@code deploy}, {@code config}, + * {@code dependency-lock} — and are user-assigned per artifact. A build manifest is both a build + * artifact and a dependency declaration. + */ +@Data +public class JArtifact { + private String id; + private String kind = "artifact"; + + /** Repo-relative path with {@code /} separators; also the map key. */ + private String path; + + /** Format identifier: xml|yaml|json|properties|gradle|dockerfile|text|binary. */ + private String format; + + /** Semantic roles, free-vocabulary: build, ci, deploy, config, dependency-lock, etc. */ + private List roles = new ArrayList<>(); + + /** File size in bytes. */ + private long sizeBytes; + + /** SHA-256 hash of the whole file. */ + private String sha256; + + /** + * File contents when captured (controlled by {@code --artifact-text}), empty string for binary + * or when capture is disabled. + */ + private String source = ""; + + /** {@code true} if the source was truncated to fit memory limits. */ + private boolean textTruncated; + + /** Extraction/parsing status: {@code none}, {@code partial}, or {@code full}. */ + private String extraction = "none"; + + /** Configuration keys defined or referenced in this artifact. */ + private List configKeys = new ArrayList<>(); +} diff --git a/src/main/java/com/ibm/cldk/schema/JConfigKey.java b/src/main/java/com/ibm/cldk/schema/JConfigKey.java new file mode 100644 index 00000000..4b0364fb --- /dev/null +++ b/src/main/java/com/ibm/cldk/schema/JConfigKey.java @@ -0,0 +1,36 @@ +package com.ibm.cldk.schema; + +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +/** + * A configuration key defined or referenced in an artifact — a property in a {@code .properties} + * file, a YAML key path, an XML element, an environment variable, or a Dockerfile argument. + * {@code id} is the canonical {@code can://artifact//@key/} reference, nesting + * under the artifact's id so the key is discoverable and addressable. + * + *

{@code namespace} is free-vocabulary (properties, yaml, xml, env, dockerfile) and identifies + * the config language. {@code references} are the raw tokens (e.g., environment variable names, + * property placeholders) that this key references in order, deduplicated. + */ +@Data +public class JConfigKey { + /** Canonical id: {@code @key/}. */ + private String id; + + /** Dotted key path (e.g., {@code server.port}, {@code logging.level.root}). */ + private String key; + + /** Config language/namespace: properties|yaml|xml|env|dockerfile. */ + private String namespace; + + /** Configuration value when captured via {@code --artifact-text}, {@code null} otherwise. */ + private String value; + + /** Location in source, best-effort; {@code null} is acceptable. */ + private Span span; + + /** Raw tokens (e.g., env var names, property placeholders) this key references, deduplicated. */ + private List references = new ArrayList<>(); +} diff --git a/src/main/java/com/ibm/cldk/schema/JDependency.java b/src/main/java/com/ibm/cldk/schema/JDependency.java new file mode 100644 index 00000000..eff4052b --- /dev/null +++ b/src/main/java/com/ibm/cldk/schema/JDependency.java @@ -0,0 +1,54 @@ +package com.ibm.cldk.schema; + +import java.util.ArrayList; +import java.util.List; +import lombok.Data; + +/** + * A declared dependency from a build manifest or lock file, representing one pinned package in an + * ecosystem. {@code group} (Maven {@code groupId}) is additive over the reference analyzer + * (codeanalyzer-python), which has no analogue since PyPI names are single-segment; in Maven each + * coordinate is split into a {@code group} and {@code name} ({@code artifactId}). + * + *

{@code ecosystem} and {@code kind} are free-vocabulary, matching the reference implementation: + * ecosystems include {@code maven}, {@code npm}, {@code gradle}, {@code pypi}, {@code golang}; + * kinds include {@code runtime}, {@code dev}, {@code optional}, {@code build}. + * + *

{@code lockedVersion} is {@code null} (omitted from JSON) when the dependency is unpinned — + * recorded only when found in a lock file or package manager. + */ +@Data +public class JDependency { + /** Maven {@code groupId} (additive; PyPI names are single-segment and have no analogue). */ + private String group; + + /** Maven {@code artifactId} (the package name). */ + private String name; + + /** Package ecosystem: maven|npm|gradle|pypi|golang, etc. Defaults to maven. */ + private String ecosystem = "maven"; + + /** Declared version range or spec, verbatim as written (may be empty). */ + private String spec = ""; + + /** Dependency kind: runtime|dev|optional|build. Defaults to runtime. */ + private String kind = "runtime"; + + /** Maven classifiers (free-vocabulary extras, e.g., {@code sources}, {@code javadoc}). */ + private List extras = new ArrayList<>(); + + /** The {@code JArtifact} id where this dependency was declared. */ + private String declaredIn = ""; + + /** {@code true} if directly declared; {@code false} for lockfile-only transitive pins. */ + private boolean direct = true; + + /** + * Pinned or resolved version from a lock file. {@code null} (omitted from JSON) when the + * dependency is unpinned. + */ + private String lockedVersion; + + /** Provenance hints: sources where this dependency was found — declared|lockfile|heuristic. */ + private List prov = new ArrayList<>(); +} diff --git a/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java b/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java new file mode 100644 index 00000000..5d088c47 --- /dev/null +++ b/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java @@ -0,0 +1,65 @@ +package com.ibm.cldk.schema; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.junit.jupiter.api.Test; + +/** The artifact layer's wire shape and id grammar, matching codeanalyzer-python v1.3.0. */ +class ArtifactModelTest { + + @Test + void artifactIdIsLanguageNeutral() { + assertEquals("can://artifact/myapp/deploy/docker-compose.yml", + CanId.artifactId("myapp", "deploy/docker-compose.yml"), + "the scheme carries `artifact`, not `java` — sibling analyzers must land on this node"); + } + + @Test + void configKeyIdNestsUnderItsArtifact() { + String art = CanId.artifactId("myapp", "src/main/resources/application.yml"); + assertEquals(art + "@key/server.port", CanId.configKeyId(art, "server.port")); + } + + @Test + void purlIsTwoSegmentForMaven() { + assertEquals("pkg:maven/org.apache.commons/commons-lang3", + CanId.purlMaven("org.apache.commons", "commons-lang3")); + } + + @Test + void unsetOptionalsAreOmittedNotNulled() { + JDependency d = new JDependency(); + d.setGroup("org.example"); + d.setName("widget"); + String json = V2Json.compact().toJson(d); + assertFalse(json.contains("locked_version"), "an unpinned dependency omits the key: " + json); + assertTrue(json.contains("\"ecosystem\":\"maven\""), json); + assertTrue(json.contains("\"direct\":true"), json); + } + + @Test + void applicationOmitsTheLayerWhenEmpty() { + JApplication app = new JApplication(); + app.setId("can://java/x"); + String json = V2Json.compact().toJson(app); + assertFalse(json.contains("artifacts"), json); + assertFalse(json.contains("dependencies"), json); + } + + @Test + void configKeysNestInsideTheirArtifact() { + JArtifact a = new JArtifact(); + a.setPath("application.properties"); + a.setFormat("properties"); + JConfigKey k = new JConfigKey(); + k.setKey("server.port"); + k.setNamespace("properties"); + a.getConfigKeys().add(k); + String json = V2Json.compact().toJson(a); + assertTrue(json.contains("\"config_keys\""), json); + assertTrue(json.contains("\"server.port\""), json); + } +} From 3d1906448f31f485485a4c2ef4d76ea33413f5b0 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Sun, 30 Aug 2026 22:20:25 -0400 Subject: [PATCH 02/19] feat(artifacts): repo-wide discovery walk with never-drop inventory --- .../ibm/cldk/artifacts/ArtifactDiscovery.java | 275 ++++++++++++++++++ .../cldk/artifacts/ArtifactDiscoveryTest.java | 261 +++++++++++++++++ 2 files changed, 536 insertions(+) create mode 100644 src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java create mode 100644 src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java diff --git a/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java b/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java new file mode 100644 index 00000000..1f29faa2 --- /dev/null +++ b/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java @@ -0,0 +1,275 @@ +package com.ibm.cldk.artifacts; + +import com.ibm.cldk.schema.CanId; +import com.ibm.cldk.schema.JArtifact; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CodingErrorAction; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Repo-wide inventory of every non-source file: build manifests, config, Dockerfiles, CI + * definitions, and anything else a Java project ships beside its {@code .java} sources. + * + *

The governing rule is never drop a file. A path matching no entry in {@link #RULES} is + * still captured — {@code text}/{@code unknown} if it decodes as UTF-8, {@code binary} if it does + * not — because the layer's value is a complete answer to "what is in this repo," and a + * classification miss is a shrug, not an omission. The one carve-out is a {@code .java} file that + * no rule names: the symbol table (L1) already owns those, and inventorying them here too would + * produce two nodes for one file with no way for a consumer to tell which is authoritative. + * + *

Nothing here parses artifact content — later tasks read {@code source} back out of the + * returned {@link JArtifact}s to extract dependencies and config keys. This class only decides + * what a file IS and captures its bytes/text so that later reading never has to touch the + * filesystem again. + */ +public final class ArtifactDiscovery { + + private ArtifactDiscovery() {} + + // First match wins — order matters. Specific names precede generic globs, so + // `pom.xml` is a manifest while any other `*.xml` is a config candidate. + private static final List RULES = List.of( + new Rule("pom.xml", "xml", List.of("dependency-manifest")), + new Rule("build.gradle", "gradle", List.of("dependency-manifest", "tool-config")), + new Rule("build.gradle.kts", "gradle", List.of("dependency-manifest", "tool-config")), + new Rule("settings.gradle", "gradle", List.of("tool-config")), + new Rule("settings.gradle.kts", "gradle", List.of("tool-config")), + new Rule("gradle.lockfile", "text", List.of("dependency-manifest")), + new Rule("gradle.properties", "properties", List.of("tool-config")), + new Rule("gradle/libs.versions.toml", "text", List.of("dependency-manifest")), + new Rule("ivy.xml", "xml", List.of("dependency-manifest")), + new Rule("Dockerfile", "dockerfile", List.of("container-image")), + new Rule("*.dockerfile", "dockerfile", List.of("container-image")), + new Rule("docker-compose*.yml", "yaml", List.of("service-topology")), + new Rule("docker-compose*.yaml", "yaml", List.of("service-topology")), + new Rule("compose.yml", "yaml", List.of("service-topology")), + new Rule("compose.yaml", "yaml", List.of("service-topology")), + new Rule("k8s/*.yml", "yaml", List.of("service-topology")), + new Rule("k8s/*.yaml", "yaml", List.of("service-topology")), + new Rule("Chart.yaml", "yaml", List.of("service-topology")), + new Rule("values.yaml", "yaml", List.of("service-topology")), + new Rule("application*.yml", "yaml", List.of("tool-config")), + new Rule("application*.yaml", "yaml", List.of("tool-config")), + new Rule("application*.properties", "properties", List.of("tool-config")), + new Rule("bootstrap*.yml", "yaml", List.of("tool-config")), + new Rule("logback*.xml", "xml", List.of("tool-config")), + new Rule("web.xml", "xml", List.of("tool-config")), + new Rule("persistence.xml", "xml", List.of("tool-config")), + new Rule("beans.xml", "xml", List.of("tool-config")), + new Rule("*.tf", "text", List.of("iac")), + new Rule(".github/workflows/*.yml", "yaml", List.of("ci")), + new Rule(".github/workflows/*.yaml", "yaml", List.of("ci")), + new Rule(".gitlab-ci.yml", "yaml", List.of("ci")), + new Rule("Jenkinsfile", "text", List.of("ci")), + new Rule(".env", "text", List.of("env")), + new Rule(".env.*", "text", List.of("env")), + new Rule("LICENSE*", "text", List.of("legal")), + new Rule("NOTICE*", "text", List.of("legal")), + new Rule("*.md", "text", List.of("docs")), + new Rule("*.adoc", "text", List.of("docs")), + new Rule("*.properties", "properties", List.of("tool-config")), + new Rule("*.yml", "yaml", List.of("unknown")), + new Rule("*.yaml", "yaml", List.of("unknown")), + new Rule("*.json", "json", List.of("unknown")), + new Rule("*.xml", "xml", List.of("unknown"))); + + // Adapted from python's (which is pip-shaped) to Java's build output. `build` and `target` are + // the same class of exclusion the L1 extractor learned in #199: Gradle's build/resources copies + // of test fixtures are build output, not project code, and the lesson generalises to artifacts. + private static final Set IGNORED = Set.of( + ".git", ".hg", ".svn", "target", "build", "out", "bin", + ".gradle", ".mvn", ".idea", ".settings", "node_modules", + ".codeanalyzer", "_library_dependencies"); + + /** A first-match-wins classification row: glob {@code pattern}, {@code format}, and {@code roles}. */ + private static final class Rule { + private final String pattern; + private final String format; + private final List roles; + + Rule(String pattern, String format, List roles) { + this.pattern = pattern; + this.format = format; + this.roles = roles; + } + } + + /** + * Walk {@code projectDir} and return every regular file as an artifact, keyed by its + * repo-relative {@code /}-separated path with iteration order sorted by that key. + * + * @param captureText when {@code false}, every {@code source} is empty and no file is + * truncated, but classification and hashing are unaffected — the inventory is identical + * either way, only the captured text differs + * @param textMaxBytes byte cap on captured text for a decodable file; a {@code + * dependency-manifest} is exempt (always captured whole when {@code captureText} is on) + * because its {@code source} is what dependency extraction parses, not bulk content the + * cap exists to bound + */ + public static Map discover( + Path projectDir, String appName, boolean captureText, int textMaxBytes) throws IOException { + Path root = projectDir.toAbsolutePath().normalize(); + List files; + try (Stream walk = Files.walk(root)) { + files = walk.filter(Files::isRegularFile).sorted().collect(Collectors.toList()); + } + + Map artifacts = new TreeMap<>(); + for (Path file : files) { + Path relative = root.relativize(file); + if (isIgnored(relative)) { + continue; + } + String relPosix = relative.toString().replace('\\', '/'); + String name = basename(relPosix); + Rule rule = classify(relPosix); + if (rule == null && name.endsWith(".java")) { + continue; // the symbol table owns Java source, not this layer + } + + byte[] raw = Files.readAllBytes(file); + String text = decodeStrict(raw); + boolean decodable = text != null; + + String format; + List roles; + if (!decodable) { + format = "binary"; + roles = rule != null ? rule.roles : List.of("unknown"); + } else if (rule != null) { + format = rule.format; + roles = rule.roles; + } else { + format = "text"; + roles = (!name.contains(".") && text.startsWith("#!")) ? List.of("script") : List.of("unknown"); + } + + JArtifact artifact = new JArtifact(); + artifact.setId(CanId.artifactId(appName, relPosix)); + artifact.setPath(relPosix); + artifact.setFormat(format); + artifact.setRoles(roles); + // sha256/sizeBytes always describe the whole file, independent of capture/truncation + // below -- this is the invariant a consumer relies on to detect a truncated source. + artifact.setSizeBytes(raw.length); + artifact.setSha256(sha256Hex(raw)); + + if (decodable && captureText) { + int cap = roles.contains("dependency-manifest") ? raw.length : textMaxBytes; + if (raw.length <= cap) { + artifact.setSource(text); + } else { + artifact.setSource(decodeLenientPrefix(raw, cap)); + artifact.setTextTruncated(true); + } + } + + artifacts.put(relPosix, artifact); + } + return artifacts; + } + + // Directory-segment check only -- deliberately NOT python's + // `any(part in IGNORED for part in rel.parts)`, which also matches a *file* literally named + // "build" or "target" because it tests every segment including the leaf. Checking only the + // segments strictly above the file name is the fix; do not "correct" this back to match python. + private static boolean isIgnored(Path relative) { + int dirSegments = relative.getNameCount() - 1; + for (int i = 0; i < dirSegments; i++) { + if (IGNORED.contains(relative.getName(i).toString())) { + return true; + } + } + return false; + } + + private static String basename(String relPosix) { + int slash = relPosix.lastIndexOf('/'); + return slash < 0 ? relPosix : relPosix.substring(slash + 1); + } + + // A pattern containing '/' matches the full repo-relative path (e.g. "k8s/*.yml" matches only + // directly under k8s/); a bare pattern matches just the basename (e.g. "*.xml" matches any-depth). + private static Rule classify(String relPosix) { + String name = basename(relPosix); + for (Rule rule : RULES) { + String target = rule.pattern.contains("/") ? relPosix : name; + if (globMatches(rule.pattern, target)) { + return rule; + } + } + return null; + } + + // Only '*' appears anywhere in RULES, so that is all this translates. This mirrors python's + // fnmatch semantics (a plain regex wildcard, so '*' can cross '/') rather than java.nio's glob + // PathMatcher, whose '*' stops at a path separator and would silently narrow "k8s/*.yml". + private static boolean globMatches(String pattern, String target) { + StringBuilder regex = new StringBuilder(); + for (String literal : pattern.split("\\*", -1)) { + if (regex.length() > 0) { + regex.append(".*"); + } + regex.append(Pattern.quote(literal)); + } + return Pattern.matches(regex.toString(), target); + } + + /** Whole-file strict UTF-8 decode; {@code null} means the file is not valid UTF-8 ("binary"). */ + private static String decodeStrict(byte[] raw) { + try { + return StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(raw)).toString(); + } catch (CharacterCodingException e) { + return null; + } + } + + /** + * Decode a byte prefix for a truncated capture. The cap can land mid-character; IGNORE drops + * the dangling partial character at the cut instead of throwing, matching python's {@code + * errors="ignore"}. Only called on bytes already proven fully UTF-8 decodable by {@link + * #decodeStrict}, so the sole possible error is that one boundary character. + */ + private static String decodeLenientPrefix(byte[] raw, int len) { + try { + return StandardCharsets.UTF_8.newDecoder() + .onMalformedInput(CodingErrorAction.IGNORE) + .onUnmappableCharacter(CodingErrorAction.IGNORE) + .decode(ByteBuffer.wrap(raw, 0, len)) + .toString(); + } catch (CharacterCodingException e) { + // IGNORE never reports an error; this path cannot execute. + throw new IllegalStateException(e); + } + } + + // Hand-rolled hex encoding: this project's toolchain is pinned to Java 11 (see the Kotlin + // `jvmToolchain(11)` block), which predates java.util.HexFormat (17+). Mirrors + // L1BuildContext.contentHash()'s identical nibble loop. + private static String sha256Hex(byte[] raw) { + try { + byte[] digest = MessageDigest.getInstance("SHA-256").digest(raw); + StringBuilder hex = new StringBuilder(digest.length * 2); + for (byte b : digest) { + hex.append(Character.forDigit((b >> 4) & 0xF, 16)).append(Character.forDigit(b & 0xF, 16)); + } + return hex.toString(); + } catch (NoSuchAlgorithmException e) { + // SHA-256 is required of every JVM; absence is unrecoverable, not a soft failure. + throw new IllegalStateException("SHA-256 unavailable", e); + } + } +} diff --git a/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java b/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java new file mode 100644 index 00000000..32ee03e4 --- /dev/null +++ b/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java @@ -0,0 +1,261 @@ +package com.ibm.cldk.artifacts; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.ibm.cldk.schema.JArtifact; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Behavioural tests for {@link ArtifactDiscovery}: the never-drop inventory contract (every + * branch in the discovery walk's classification order), the ignore set's directory-vs-file + * distinction, and the invariants a downstream consumer relies on (whole-file {@code sha256}/ + * {@code sizeBytes} regardless of capture/truncation, deterministic key order). + */ +class ArtifactDiscoveryTest { + + private static String sha256(byte[] bytes) { + try { + byte[] digest = MessageDigest.getInstance("SHA-256").digest(bytes); + StringBuilder hex = new StringBuilder(digest.length * 2); + for (byte b : digest) { + hex.append(Character.forDigit((b >> 4) & 0xF, 16)).append(Character.forDigit(b & 0xF, 16)); + } + return hex.toString(); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException(e); + } + } + + @Test + void discover_classifiesPomXmlAsADependencyManifest(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("pom.xml"), "", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + JArtifact pom = artifacts.get("pom.xml"); + assertNotNull(pom); + assertEquals("can://artifact/app/pom.xml", pom.getId()); + assertEquals("pom.xml", pom.getPath()); + // configKeys/extraction are later tasks' fields; discovery must leave their defaults alone. + assertEquals("artifact", pom.getKind()); + assertEquals("none", pom.getExtraction()); + assertTrue(pom.getConfigKeys().isEmpty()); + assertEquals("xml", pom.getFormat()); + assertEquals(List.of("dependency-manifest"), pom.getRoles()); + assertEquals("", pom.getSource()); + } + + @Test + void discover_stillInventoriesAnUnmatchedTextFileAsTextUnknown(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("notes.txt"), "hello", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + JArtifact notes = artifacts.get("notes.txt"); + assertNotNull(notes, "never drop a file: an unmatched extension must still be inventoried"); + assertEquals("text", notes.getFormat()); + assertEquals(List.of("unknown"), notes.getRoles()); + assertEquals("hello", notes.getSource()); + } + + @Test + void discover_excludesFilesUnderTargetDirectory(@TempDir Path tmp) throws IOException { + Path dir = tmp.resolve("target"); + Files.createDirectories(dir); + Files.writeString(dir.resolve("classes.txt"), "compiled", StandardCharsets.UTF_8); + Files.writeString(tmp.resolve("pom.xml"), "", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + assertFalse(artifacts.containsKey("target/classes.txt")); + assertTrue(artifacts.containsKey("pom.xml")); + } + + @Test + void discover_excludesFilesUnderBuildDirectory(@TempDir Path tmp) throws IOException { + Path dir = tmp.resolve("build/resources"); + Files.createDirectories(dir); + // The #199 lesson: Gradle's processResources copies fixtures into build/resources; those + // copies are build output, not project artifacts, and must not be inventoried either. + Files.writeString(dir.resolve("app.properties"), "k=v", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + assertTrue(artifacts.isEmpty()); + } + + @Test + void discover_aFileNamedBuildIsNotExcluded(@TempDir Path tmp) throws IOException { + // The deliberate divergence from python's reference: IGNORED matches a directory segment, + // not python's `any(part in IGNORED for part in rel.parts)`, which also excludes a *file* + // literally named "build" or "target". A directory check must not have that bug. + Files.writeString(tmp.resolve("build"), "not a directory", StandardCharsets.UTF_8); + Files.writeString(tmp.resolve("target"), "not a directory either", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + assertTrue(artifacts.containsKey("build"), "a file named 'build' is not a build-output directory"); + assertTrue(artifacts.containsKey("target"), "a file named 'target' is not a build-output directory"); + } + + @Test + void discover_skipsJavaFilesEntirely(@TempDir Path tmp) throws IOException { + Path pkg = tmp.resolve("src/main/java/com/example"); + Files.createDirectories(pkg); + Files.writeString( + pkg.resolve("Greeter.java"), "package com.example;\npublic class Greeter {}\n", + StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + assertTrue(artifacts.isEmpty(), "the symbol table owns .java files, not the artifact layer"); + } + + @Test + void discover_classifiesNonUtf8FilesAsBinaryButKeepsHashAndSize(@TempDir Path tmp) throws IOException { + byte[] raw = {(byte) 0xFF, (byte) 0xFE, 0x00, 0x01, 0x02}; // not valid UTF-8 + Files.write(tmp.resolve("blob.dat"), raw); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + JArtifact blob = artifacts.get("blob.dat"); + assertNotNull(blob); + assertEquals("binary", blob.getFormat()); + assertEquals(List.of("unknown"), blob.getRoles()); + assertEquals("", blob.getSource()); + assertFalse(blob.isTextTruncated()); + assertEquals(raw.length, blob.getSizeBytes()); + assertEquals(sha256(raw), blob.getSha256()); + } + + @Test + void discover_aRuleMatchedBinaryFileKeepsTheRulesRolesInsteadOfUnknown(@TempDir Path tmp) throws IOException { + // pom.xml is rule-matched (dependency-manifest); if its bytes are not valid UTF-8 it must + // still downgrade to format=binary while KEEPING that role, not falling back to unknown. + byte[] raw = {(byte) 0xFF, (byte) 0xFE, 0x00}; + Files.write(tmp.resolve("pom.xml"), raw); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + JArtifact pom = artifacts.get("pom.xml"); + assertNotNull(pom); + assertEquals("binary", pom.getFormat()); + assertEquals(List.of("dependency-manifest"), pom.getRoles()); + assertEquals("", pom.getSource()); + assertEquals(sha256(raw), pom.getSha256()); + } + + @Test + void discover_extensionlessShebangFileGetsScriptRole(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("run-server"), "#!/bin/sh\necho hi\n", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + JArtifact script = artifacts.get("run-server"); + assertNotNull(script); + assertEquals("text", script.getFormat()); + assertEquals(List.of("script"), script.getRoles()); + } + + @Test + void discover_truncatesOverCapFilesButHashesTheWholeFile(@TempDir Path tmp) throws IOException { + String content = "a".repeat(100); + byte[] raw = content.getBytes(StandardCharsets.UTF_8); + Files.writeString(tmp.resolve("big.txt"), content, StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 10); + + JArtifact big = artifacts.get("big.txt"); + assertNotNull(big); + assertTrue(big.isTextTruncated()); + assertEquals(10, big.getSource().length()); + assertEquals( + raw.length, big.getSizeBytes(), + "sizeBytes must describe the whole file, not the captured prefix"); + assertEquals( + sha256(raw), big.getSha256(), "sha256 must hash the whole file even when source is truncated"); + } + + @Test + void discover_dropsAPartialMultiByteCharacterAtTheTruncationBoundaryRatherThanThrowing(@TempDir Path tmp) + throws IOException { + // "caf" followed by the 2-byte UTF-8 sequence for U+00E9 (e-acute: 0xC3 0xA9), twice. Built + // as explicit bytes rather than a string literal so the split point is unambiguous and the + // fixture does not depend on the source file's own encoding. A cap of 4 lands right after + // the first 0xC3 lead byte, splitting that character. + byte[] raw = {'c', 'a', 'f', (byte) 0xC3, (byte) 0xA9, 'c', 'a', 'f', (byte) 0xC3, (byte) 0xA9}; + Files.write(tmp.resolve("accented.txt"), raw); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 4); + + JArtifact accented = artifacts.get("accented.txt"); + assertNotNull(accented, "a cap landing mid-character must still produce a usable artifact"); + assertTrue(accented.isTextTruncated()); + assertEquals("caf", accented.getSource(), "the split lead byte of 'e' must be dropped, not replaced"); + assertEquals(raw.length, accented.getSizeBytes()); + assertEquals(sha256(raw), accented.getSha256()); + } + + @Test + void discover_aPomXmlOverTheCapIsNotTruncated(@TempDir Path tmp) throws IOException { + String content = "" + "x".repeat(100) + ""; + Files.writeString(tmp.resolve("pom.xml"), content, StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 10); + + JArtifact pom = artifacts.get("pom.xml"); + assertNotNull(pom); + assertFalse(pom.isTextTruncated(), "a dependency manifest is captured whole regardless of the cap"); + assertEquals(content, pom.getSource()); + assertEquals(content.getBytes(StandardCharsets.UTF_8).length, pom.getSizeBytes()); + } + + @Test + void discover_withCaptureTextDisabled_keepsInventoryButEmptiesEverySource(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("pom.xml"), "", StandardCharsets.UTF_8); + Files.writeString(tmp.resolve("notes.txt"), "hello", StandardCharsets.UTF_8); + Files.write(tmp.resolve("blob.dat"), new byte[] {(byte) 0xFF, (byte) 0xFE}); + + Map withText = ArtifactDiscovery.discover(tmp, "app", true, 262144); + Map withoutText = ArtifactDiscovery.discover(tmp, "app", false, 262144); + + assertEquals(withText.keySet(), withoutText.keySet(), "inventory must be identical regardless of capture"); + for (String key : withText.keySet()) { + JArtifact on = withText.get(key); + JArtifact off = withoutText.get(key); + assertEquals(on.getFormat(), off.getFormat(), key); + assertEquals(on.getRoles(), off.getRoles(), key); + assertEquals(on.getSha256(), off.getSha256(), key); + assertEquals(on.getSizeBytes(), off.getSizeBytes(), key); + assertEquals("", off.getSource(), key); + assertFalse(off.isTextTruncated(), key); + } + } + + @Test + void discover_producesIdenticalKeyOrderAcrossRuns(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("pom.xml"), "", StandardCharsets.UTF_8); + Files.writeString(tmp.resolve("notes.txt"), "hello", StandardCharsets.UTF_8); + Path k8s = tmp.resolve("k8s"); + Files.createDirectories(k8s); + Files.writeString(k8s.resolve("deploy.yml"), "kind: Deployment", StandardCharsets.UTF_8); + + Map first = ArtifactDiscovery.discover(tmp, "app", true, 262144); + Map second = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + assertEquals(List.copyOf(first.keySet()), List.copyOf(second.keySet())); + assertEquals(List.of("k8s/deploy.yml", "notes.txt", "pom.xml"), List.copyOf(first.keySet())); + } +} From 5cc0493b99c3be53767d90a353b5acfc05bf1ea7 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Sun, 30 Aug 2026 22:22:39 -0400 Subject: [PATCH 03/19] test(artifacts): cover slash-pattern matching against the full repo-relative path --- .../cldk/artifacts/ArtifactDiscoveryTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java b/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java index 32ee03e4..7547caef 100644 --- a/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java +++ b/src/test/java/com/ibm/cldk/artifacts/ArtifactDiscoveryTest.java @@ -258,4 +258,26 @@ void discover_producesIdenticalKeyOrderAcrossRuns(@TempDir Path tmp) throws IOEx assertEquals(List.copyOf(first.keySet()), List.copyOf(second.keySet())); assertEquals(List.of("k8s/deploy.yml", "notes.txt", "pom.xml"), List.copyOf(first.keySet())); } + + @Test + void discover_slashContainingPatternMatchesTheFullPathNotJustTheBasename(@TempDir Path tmp) throws IOException { + Path k8s = tmp.resolve("k8s"); + Files.createDirectories(k8s); + Files.writeString(k8s.resolve("deploy.yml"), "kind: Deployment", StandardCharsets.UTF_8); + // A bare "*.yml" elsewhere must NOT get k8s's service-topology role -- only "k8s/*.yml" + // does, proving the pattern's '/' is matched against the full repo-relative path rather + // than collapsing to a basename check like every slash-free rule in the table. + Files.writeString(tmp.resolve("other.yml"), "not a k8s manifest", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + JArtifact deploy = artifacts.get("k8s/deploy.yml"); + assertNotNull(deploy); + assertEquals("yaml", deploy.getFormat()); + assertEquals(List.of("service-topology"), deploy.getRoles()); + + JArtifact other = artifacts.get("other.yml"); + assertNotNull(other); + assertEquals(List.of("unknown"), other.getRoles(), "a plain *.yml outside k8s/ falls to the generic rule"); + } } From c5148136ac389245233ca93d9ae72aa085d8fed4 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Sun, 30 Aug 2026 22:55:11 -0400 Subject: [PATCH 04/19] feat(artifacts): pom.xml, Gradle and lockfile dependency parsers --- .../ibm/cldk/artifacts/ManifestParsers.java | 252 +++++++++++++++ .../cldk/artifacts/ManifestParsersTest.java | 286 ++++++++++++++++++ 2 files changed, 538 insertions(+) create mode 100644 src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java create mode 100644 src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java diff --git a/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java b/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java new file mode 100644 index 00000000..681a523e --- /dev/null +++ b/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java @@ -0,0 +1,252 @@ +package com.ibm.cldk.artifacts; + +import java.io.IOException; +import java.io.StringReader; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +/** + * Pure dependency-manifest and lockfile readers: text in, records out. No file I/O, no execution, + * no mutation of anything — {@code DependencyView} (a later task in this layer) is what touches + * disk and assembles these records into the emitted {@code JDependency} model. + * + *

This mirrors codeanalyzer-python's {@code artifacts/parsers.py} {@code (records, partial)} + * convention: a whole-file parse failure (a malformed {@code pom.xml}) differs from a per-line one + * (an unrecognized Gradle declaration). The former keeps the artifact but flags extraction; the + * latter is silently skipped and the file still succeeds. The vocabulary is deliberately free-form + * strings rather than enums, matching v1.3.0: {@code kind} is one of {@code + * runtime|dev|optional|build} — four values, even though Maven has six scopes. + */ +public final class ManifestParsers { + + private ManifestParsers() {} + + /** A parsed declaration, before reconciliation. Mirrors python's frozen {@code RawDep}. */ + public static final class RawDep { + public final String group; + public final String name; + public final String spec; + public final String kind; // runtime|dev|optional|build + public final List extras; + + RawDep(String group, String name, String spec, String kind, List extras) { + this.group = group; + this.name = name; + this.spec = spec; + this.kind = kind; + this.extras = extras; + } + } + + /** Records plus a partial flag — an unparseable manifest keeps its artifact and flags extraction. */ + public static final class ParseResult { + public final List deps; + public final boolean partial; + + ParseResult(List deps, boolean partial) { + this.deps = deps; + this.partial = partial; + } + } + + // Configuration keyword, then a 'g:a:v' or "g:a:v" string literal on the same line. Deliberately + // shallow, exactly as python's setup.py reader is deliberately static: a build.gradle is a + // program (variables, `ext {}` properties, version catalogs, multi-line calls), and evaluating + // it is out of scope. An interpolated version like "$springVersion" is captured verbatim as + // `spec` with no resolved value -- a known gap, not a bug, matching the reference's identical + // setup.py gap. A line spanning a call across multiple lines, or a project(...) reference with + // no coordinate literal at all, simply does not match and is skipped like any other line. + private static final Pattern GRADLE_DEP_LINE = Pattern.compile( + "\\b(implementation|api|compileOnly|runtimeOnly|testImplementation|annotationProcessor)\\b" + + "[^'\"]*['\"]([^:'\"]+):([^:'\"]+):([^'\"]*)['\"]"); + + /** Dispatch on basename. An unknown basename returns an empty, non-partial result. */ + public static ParseResult parseManifest(String path, String text) { + String base = basename(path); + try { + if ("pom.xml".equals(base)) { + return new ParseResult(parsePomDependencies(text), false); + } + if ("build.gradle".equals(base) || "build.gradle.kts".equals(base)) { + return new ParseResult(parseGradleDependencies(text), false); + } + } catch (Exception e) { + // Whole-file failure (malformed XML, or any other unexpected exception type): keep the + // artifact but flag extraction, rather than letting one bad manifest fail the analysis. + return new ParseResult(List.of(), true); + } + return new ParseResult(List.of(), false); + } + + /** name -> resolved version, from a lockfile. Never throws; a malformed lock returns empty. */ + public static Map parseLockPins(String path, String text) { + try { + if ("gradle.lockfile".equals(basename(path))) { + return parseGradleLockfile(text); + } + } catch (Exception e) { + return Map.of(); + } + return Map.of(); + } + + // ---- pom.xml -------------------------------------------------------------------------- + + private static List parsePomDependencies(String text) + throws ParserConfigurationException, SAXException, IOException { + Document doc = newSecureDocumentBuilder().parse(new InputSource(new StringReader(text))); + Element project = doc.getDocumentElement(); + List out = new ArrayList<>(); + // Only /project/dependencies/dependency -- a nested is + // a version constraint, not a declared dependency, and is a different (non-direct) child. + for (Element dependencies : directChildren(project, "dependencies")) { + for (Element dependency : directChildren(dependencies, "dependency")) { + String scope = childText(dependency, "scope"); + String effectiveScope = scope.isEmpty() ? "compile" : scope; // Maven's own default + if ("import".equals(effectiveScope)) { + continue; // BOM inclusion, not a dependency at all + } + List extras = new ArrayList<>(); + String classifier = childText(dependency, "classifier"); + if (!classifier.isEmpty()) { + extras.add(classifier); + } + if ("true".equalsIgnoreCase(childText(dependency, "optional"))) { + // Kind is derived solely from (see kindForMavenScope) -- there is no + // separate boolean slot on RawDep for "optional" -- so the flag instead lands in + // `extras`, the same free-vocabulary bucket a classifier uses. + extras.add("optional"); + } + out.add(new RawDep( + childText(dependency, "groupId"), + childText(dependency, "artifactId"), + childText(dependency, "version"), // "" when inherited from a parent or a BOM + kindForMavenScope(effectiveScope), + List.copyOf(extras))); + } + } + return out; + } + + // The one judgement call in this file; everything else here is transcription. Maven has six + // scopes and RawDep.kind has four slots. compile/runtime keep their obvious "runtime" meaning; + // test maps to dev. provided and system both fall to build: neither ships at runtime, both are + // "present only so the build compiles," the same bucket a build-system requirement occupies. + // import never reaches here -- the caller skips it outright, since it is BOM inclusion rather + // than a dependency. + private static String kindForMavenScope(String scope) { + switch (scope) { + case "test": + return "dev"; + case "provided": + case "system": + return "build"; + default: // "compile", "runtime" + return "runtime"; + } + } + + private static DocumentBuilder newSecureDocumentBuilder() throws ParserConfigurationException { + // pom.xml is untrusted repository content, not a file this process authored. Hardened per + // OWASP's XXE prevention cheat sheet: disallowing DOCTYPE outright is the categorical + // blocker (no DTD means no custom entities of any kind, external or internal); the two + // external-entity toggles are kept as explicit defense in depth alongside it. + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + return factory.newDocumentBuilder(); + } + + private static List directChildren(Element parent, String tagName) { + List children = new ArrayList<>(); + NodeList nodes = parent.getChildNodes(); + for (int i = 0; i < nodes.getLength(); i++) { + Node node = nodes.item(i); + if (node.getNodeType() == Node.ELEMENT_NODE && tagName.equals(node.getNodeName())) { + children.add((Element) node); + } + } + return children; + } + + private static String childText(Element parent, String tagName) { + List children = directChildren(parent, tagName); + return children.isEmpty() ? "" : children.get(0).getTextContent().trim(); + } + + // ---- build.gradle / build.gradle.kts --------------------------------------------------- + + private static List parseGradleDependencies(String text) { + List out = new ArrayList<>(); + for (String line : text.split("\n", -1)) { + Matcher m = GRADLE_DEP_LINE.matcher(line); + if (!m.find()) { + continue; // shallow regex: a line outside this shape is skipped, not a failure + } + out.add(new RawDep( + m.group(2), m.group(3), m.group(4), kindForGradleConfiguration(m.group(1)), List.of())); + } + return out; + } + + private static String kindForGradleConfiguration(String configuration) { + switch (configuration) { + case "testImplementation": + return "dev"; + case "compileOnly": + case "annotationProcessor": + return "build"; + default: // implementation, api, runtimeOnly + return "runtime"; + } + } + + // ---- gradle.lockfile -------------------------------------------------------------------- + + private static Map parseGradleLockfile(String text) { + // "com.group:artifact:1.2.3=compileClasspath,runtimeClasspath" -> {"com.group:artifact": "1.2.3"}. + // Comment lines and the "empty=" marker Gradle writes for a configuration + // with no locked dependencies both lack a second colon on their left-hand side and are + // skipped along with anything else that does not fit the g:a:v shape. + Map out = new LinkedHashMap<>(); + for (String line : text.split("\n", -1)) { + String trimmed = line.trim(); + if (trimmed.isEmpty() || trimmed.startsWith("#")) { + continue; + } + int eq = trimmed.indexOf('='); + if (eq < 0) { + continue; + } + String coordinate = trimmed.substring(0, eq); + int firstColon = coordinate.indexOf(':'); + int secondColon = firstColon < 0 ? -1 : coordinate.indexOf(':', firstColon + 1); + if (secondColon < 0) { + continue; + } + out.put(coordinate.substring(0, secondColon), coordinate.substring(secondColon + 1)); + } + return out; + } + + private static String basename(String path) { + int slash = path.lastIndexOf('/'); + return slash < 0 ? path : path.substring(slash + 1); + } +} diff --git a/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java b/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java new file mode 100644 index 00000000..ea51ae7a --- /dev/null +++ b/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java @@ -0,0 +1,286 @@ +package com.ibm.cldk.artifacts; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * Behavioural tests for {@link ManifestParsers}: one happy-path case per format, the Maven + * scope-to-kind mapping (the one judgement call this class makes), the whole-file-vs-per-line + * failure distinction, and the XXE probe that is the point of hardening the XML factory at all. + */ +class ManifestParsersTest { + + // ---- pom.xml -------------------------------------------------------------------------- + + @Test + void parseManifest_pomXmlExtractsGroupArtifactVersionAndClassifier() { + String pom = "" + + "org.springframeworkspring-core" + + "5.3.21" + + "org.apache.commonscommons-lang3" + + "3.14.0sources" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertFalse(result.partial); + assertEquals(2, result.deps.size()); + ManifestParsers.RawDep spring = result.deps.get(0); + assertEquals("org.springframework", spring.group); + assertEquals("spring-core", spring.name); + assertEquals("5.3.21", spring.spec); + assertEquals("runtime", spring.kind, "default (unscoped) Maven dependency is compile scope -> runtime"); + assertTrue(spring.extras.isEmpty()); + ManifestParsers.RawDep commons = result.deps.get(1); + assertEquals(List.of("sources"), commons.extras, "classifier lands in extras"); + } + + @Test + void parseManifest_pomXmlMissingVersionYieldsEmptySpec() { + // No : inherited from a parent or a BOM's . JDependency.spec's + // own contract says "may be empty" for exactly this case. + String pom = "" + + "ga" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertEquals(1, result.deps.size()); + assertEquals("", result.deps.get(0).spec); + } + + @Test + void parseManifest_pomXmlScopeTestYieldsKindDev() { + String pom = "" + + "junitjunit" + + "4.13.2test" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertEquals("dev", result.deps.get(0).kind); + } + + @Test + void parseManifest_pomXmlProvidedAndSystemScopesBothYieldKindBuild() { + String pom = "" + + "gprovided-dep" + + "1.0provided" + + "gsystem-dep" + + "1.0system" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertEquals(2, result.deps.size()); + assertEquals("build", result.deps.get(0).kind, "provided is compile-time-only, like a build requirement"); + assertEquals("build", result.deps.get(1).kind, "system is compile-time-only, like a build requirement"); + } + + @Test + void parseManifest_pomXmlImportScopedEntryIsAbsent() { + String pom = "" + + "com.examplebom" + + "1.0import" + + "greal-dep1.0" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertEquals(1, result.deps.size(), "import scope is BOM inclusion, not a dependency"); + assertEquals("real-dep", result.deps.get(0).name); + } + + @Test + void parseManifest_pomXmlOptionalTrueAddsOptionalToExtrasWithoutChangingScopeDerivedKind() { + // Kind is derived solely from (there is no fifth RawDep field to carry this flag); + // true instead lands in `extras`, the same free-vocabulary bucket a + // classifier uses. + String pom = "" + + "ga1.0" + + "runtimetrue" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + ManifestParsers.RawDep dep = result.deps.get(0); + assertEquals("runtime", dep.kind); + assertEquals(List.of("optional"), dep.extras); + } + + @Test + void parseManifest_pomXmlDependencyManagementBlockIsNotTreatedAsADeclaredDependency() { + // Only /project/dependencies/dependency is a declared dependency; a + // entry (even a non-import one) is a version constraint, not something the project itself + // depends on unless it also appears in the plain block. + String pom = "" + + "" + + "gmanaged-only1.0" + + "" + + "" + + "gdeclared1.0" + + "" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertEquals(1, result.deps.size()); + assertEquals("declared", result.deps.get(0).name); + } + + @Test + void parseManifest_malformedPomXmlReturnsPartialWithNoRecords() { + String truncated = "g"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", truncated); + + assertTrue(result.partial); + assertTrue(result.deps.isEmpty()); + } + + @Test + void parseManifest_pomXmlXxeProbeDoesNotResolveExternalEntity() { + // Classic XXE PoC: if the entity resolved, group would contain /etc/passwd's contents. The + // hardened factory must refuse the whole document at the DOCTYPE, not silently substitute it. + String xxePom = "\n" + + "]>\n" + + "" + + "&xxe;probe1.0" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", xxePom); + + assertTrue(result.partial, "a DOCTYPE-bearing pom.xml must fail closed, not resolve the entity"); + assertTrue(result.deps.isEmpty(), "no record may carry resolved external-entity content"); + } + + // ---- build.gradle / build.gradle.kts --------------------------------------------------- + + @Test + void parseManifest_buildGradleExtractsEachConfigurationWithItsMappedKind() { + String gradle = "dependencies {\n" + + " implementation 'org.springframework:spring-core:5.3.21'\n" + + " api \"com.google.guava:guava:31.1-jre\"\n" + + " testImplementation 'junit:junit:4.13.2'\n" + + " compileOnly 'org.projectlombok:lombok:1.18.30'\n" + + " annotationProcessor 'org.projectlombok:lombok:1.18.30'\n" + + " runtimeOnly 'mysql:mysql-connector-java:8.0.28'\n" + + "}\n"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("build.gradle", gradle); + + assertFalse(result.partial); + assertEquals(6, result.deps.size()); + assertEquals("runtime", result.deps.get(0).kind, "implementation"); + assertEquals("com.google.guava", result.deps.get(1).group); + assertEquals("guava", result.deps.get(1).name); + assertEquals("31.1-jre", result.deps.get(1).spec); + assertEquals("runtime", result.deps.get(1).kind, "api"); + assertEquals("dev", result.deps.get(2).kind, "testImplementation"); + assertEquals("build", result.deps.get(3).kind, "compileOnly"); + assertEquals("build", result.deps.get(4).kind, "annotationProcessor"); + assertEquals("runtime", result.deps.get(5).kind, "runtimeOnly"); + } + + @Test + void parseManifest_buildGradleKtsBasenameIsAlsoDispatched() { + String gradle = "implementation(\"org.springframework:spring-core:5.3.21\")\n"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("build.gradle.kts", gradle); + + assertEquals(1, result.deps.size()); + assertEquals("spring-core", result.deps.get(0).name); + } + + @Test + void parseManifest_gradleLineWithInterpolatedVersionKeepsTheLiteralSpec() { + // Deliberately shallow: evaluating $springVersion would mean evaluating a program. The + // literal text is captured verbatim and left unresolved -- a known gap, not a bug. + String gradle = "implementation \"org.springframework:spring-core:$springVersion\"\n"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("build.gradle", gradle); + + assertEquals(1, result.deps.size()); + assertEquals("$springVersion", result.deps.get(0).spec); + } + + @Test + void parseManifest_gradleProjectReferenceLineProducesNoRecord() { + // A project reference carries no 'g:a:v' coordinate literal at all, so the shallow regex + // simply does not match -- the line is skipped like any other non-matching line, and the + // file does not fail. + String gradle = "testImplementation project(':core')\n"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("build.gradle", gradle); + + assertFalse(result.partial); + assertTrue(result.deps.isEmpty()); + } + + @Test + void parseManifest_gradleUnmatchedLinesAreSkippedAndTheFileStillSucceeds() { + String gradle = "plugins { id 'java' }\n" + + "// a comment mentioning implementation but no coordinate\n" + + "implementation 'org.springframework:spring-core:5.3.21'\n" + + "this line is complete garbage {{{\n"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("build.gradle", gradle); + + assertFalse(result.partial); + assertEquals(1, result.deps.size()); + } + + // ---- gradle.lockfile -------------------------------------------------------------------- + + @Test + void parseLockPins_gradleLockfileExtractsGroupArtifactToVersionDroppingConfigurations() { + String lock = "# This is a Gradle generated file for dependency locking.\n" + + "com.google.guava:guava:31.1-jre=compileClasspath,runtimeClasspath\n" + + "org.springframework:spring-core:5.3.21=compileClasspath\n" + + "empty=annotationProcessor,testAnnotationProcessor\n"; + + Map pins = ManifestParsers.parseLockPins("gradle.lockfile", lock); + + assertEquals(2, pins.size()); + assertEquals("31.1-jre", pins.get("com.google.guava:guava")); + assertEquals("5.3.21", pins.get("org.springframework:spring-core")); + } + + @Test + void parseLockPins_malformedLockfileReturnsEmptyMapRatherThanThrowing() { + String garbage = "this is not a lockfile\n{{{ not even close }}}\n"; + + Map pins = ManifestParsers.parseLockPins("gradle.lockfile", garbage); + + assertTrue(pins.isEmpty()); + } + + @Test + void parseLockPins_blankLockfileReturnsEmptyMap() { + Map pins = ManifestParsers.parseLockPins("gradle.lockfile", ""); + + assertTrue(pins.isEmpty()); + } + + // ---- dispatch / unknown basenames --------------------------------------------------------- + + @Test + void parseManifest_unknownBasenameReturnsEmptyNonPartialResult() { + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("README.md", "# hello"); + + assertFalse(result.partial); + assertTrue(result.deps.isEmpty()); + } + + @Test + void parseLockPins_unknownBasenameReturnsEmptyMap() { + Map pins = ManifestParsers.parseLockPins("README.md", "# hello"); + + assertTrue(pins.isEmpty()); + } +} From a15864296127f2374af7398bc56d0c05ba10b584 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Sun, 30 Aug 2026 22:58:26 -0400 Subject: [PATCH 05/19] fix(artifacts): optional Maven dependency sets kind=optional, overriding scope --- .../ibm/cldk/artifacts/ManifestParsers.java | 35 +++++++++++-------- .../cldk/artifacts/ManifestParsersTest.java | 28 +++++++++++---- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java b/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java index 681a523e..7504edab 100644 --- a/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java +++ b/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java @@ -125,30 +125,37 @@ private static List parsePomDependencies(String text) if (!classifier.isEmpty()) { extras.add(classifier); } - if ("true".equalsIgnoreCase(childText(dependency, "optional"))) { - // Kind is derived solely from (see kindForMavenScope) -- there is no - // separate boolean slot on RawDep for "optional" -- so the flag instead lands in - // `extras`, the same free-vocabulary bucket a classifier uses. - extras.add("optional"); - } + boolean optional = "true".equalsIgnoreCase(childText(dependency, "optional")); out.add(new RawDep( childText(dependency, "groupId"), childText(dependency, "artifactId"), childText(dependency, "version"), // "" when inherited from a parent or a BOM - kindForMavenScope(effectiveScope), + kindForMavenDependency(effectiveScope, optional), List.copyOf(extras))); } } return out; } - // The one judgement call in this file; everything else here is transcription. Maven has six - // scopes and RawDep.kind has four slots. compile/runtime keep their obvious "runtime" meaning; - // test maps to dev. provided and system both fall to build: neither ships at runtime, both are - // "present only so the build compiles," the same bucket a build-system requirement occupies. - // import never reaches here -- the caller skips it outright, since it is BOM inclusion rather - // than a dependency. - private static String kindForMavenScope(String scope) { + // The two judgement calls in this file; everything else here is transcription. + // + // 1. Maven has six scopes and RawDep.kind has four slots. compile/runtime keep their obvious + // "runtime" meaning; test maps to dev. provided and system both fall to build: neither ships at + // runtime, both are "present only so the build compiles," the same bucket a build-system + // requirement occupies. import never reaches here -- the caller skips it outright, since it is + // BOM inclusion rather than a dependency. + // + // 2. true takes precedence over all of the above: an optional dependency + // is always kind="optional" regardless of its scope, matching the reference, where an optional + // group's entries are "optional" whether or not they would otherwise have been "runtime". + // "optional" is a value in the kind vocabulary itself (runtime|dev|optional|build) and belongs + // nowhere else -- a Maven classifier (which artifact of a coordinate you get) is a different + // axis from optionality (how the dependency is consumed), so this must not be confused with + // `extras`. + private static String kindForMavenDependency(String scope, boolean optional) { + if (optional) { + return "optional"; + } switch (scope) { case "test": return "dev"; diff --git a/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java b/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java index ea51ae7a..67177b64 100644 --- a/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java +++ b/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java @@ -97,10 +97,11 @@ void parseManifest_pomXmlImportScopedEntryIsAbsent() { } @Test - void parseManifest_pomXmlOptionalTrueAddsOptionalToExtrasWithoutChangingScopeDerivedKind() { - // Kind is derived solely from (there is no fifth RawDep field to carry this flag); - // true instead lands in `extras`, the same free-vocabulary bucket a - // classifier uses. + void parseManifest_pomXmlOptionalTrueSetsKindOptional() { + // "optional" is one of the four values in the kind vocabulary (runtime|dev|optional|build) + // and is not part of any other vocabulary here -- extras carries Maven classifiers (which + // artifact of a coordinate you get), a different axis from optionality (how the dependency + // is consumed), so it must not land there. String pom = "" + "ga1.0" + "runtimetrue" @@ -109,8 +110,23 @@ void parseManifest_pomXmlOptionalTrueAddsOptionalToExtrasWithoutChangingScopeDer ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); ManifestParsers.RawDep dep = result.deps.get(0); - assertEquals("runtime", dep.kind); - assertEquals(List.of("optional"), dep.extras); + assertEquals("optional", dep.kind); + assertTrue(dep.extras.isEmpty()); + } + + @Test + void parseManifest_pomXmlOptionalTrueTakesPrecedenceOverScopeDerivedKind() { + // A test dependency that is also true must come out + // "optional", not "dev" -- optionality wins over scope, matching the reference, where an + // optional group's entries are "optional" regardless of what they would otherwise have been. + String pom = "" + + "ga1.0" + + "testtrue" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", pom); + + assertEquals("optional", result.deps.get(0).kind); } @Test From 85440f01a1dfb6f8bf2e9b6f5d7c7be32712043a Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Sun, 30 Aug 2026 23:34:26 -0400 Subject: [PATCH 06/19] fix(artifacts): attribute each XXE hardening switch individually; split parseManifest's catch Each of the four DocumentBuilderFactory switches is now backed by a test that fails when that specific switch alone is removed: disallow-doctype-decl via the existing behavioural probe, the two external-entity features via a getFeature() config assertion, and FEATURE_SECURE_PROCESSING via a dedicated test (getFeature() cannot attribute it -- it reads true on an untouched factory regardless of whether setFeature was ever called). parseManifest now gives pom.xml its own try/catch narrowed to the three checked types it declares, and no catch at all on the Gradle branch, so a future bug in either no longer risks being misreported under the other branch's failure contract. --- .../ibm/cldk/artifacts/ManifestParsers.java | 49 +++++++--- .../cldk/artifacts/ManifestParsersTest.java | 97 +++++++++++++++++++ 2 files changed, 132 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java b/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java index 7504edab..6e752bc7 100644 --- a/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java +++ b/src/main/java/com/ibm/cldk/artifacts/ManifestParsers.java @@ -77,17 +77,24 @@ public static final class ParseResult { /** Dispatch on basename. An unknown basename returns an empty, non-partial result. */ public static ParseResult parseManifest(String path, String text) { String base = basename(path); - try { - if ("pom.xml".equals(base)) { + if ("pom.xml".equals(base)) { + try { return new ParseResult(parsePomDependencies(text), false); + } catch (ParserConfigurationException | SAXException | IOException e) { + // These three are exactly what parsePomDependencies declares -- the full checked + // contract of DocumentBuilderFactory/DocumentBuilder for a malformed or hostile XML + // document. Deliberately NOT a blanket `catch (Exception e)`: an unchecked exception + // here would mean a bug in the DOM helpers below, not bad input, and should surface + // rather than be silently misreported as "malformed pom". + return new ParseResult(List.of(), true); } - if ("build.gradle".equals(base) || "build.gradle.kts".equals(base)) { - return new ParseResult(parseGradleDependencies(text), false); - } - } catch (Exception e) { - // Whole-file failure (malformed XML, or any other unexpected exception type): keep the - // artifact but flag extraction, rather than letting one bad manifest fail the analysis. - return new ParseResult(List.of(), true); + } + if ("build.gradle".equals(base) || "build.gradle.kts".equals(base)) { + // No catch here by design: the brief requires this branch to never fail -- an + // unparseable line is skipped and the file still succeeds. parseGradleDependencies is + // pure regex/string scanning with no throwing path; wrapping it in a catch would + // silently paper over a future bug that broke that contract instead of surfacing it. + return new ParseResult(parseGradleDependencies(text), false); } return new ParseResult(List.of(), false); } @@ -168,16 +175,30 @@ private static String kindForMavenDependency(String scope, boolean optional) { } private static DocumentBuilder newSecureDocumentBuilder() throws ParserConfigurationException { - // pom.xml is untrusted repository content, not a file this process authored. Hardened per - // OWASP's XXE prevention cheat sheet: disallowing DOCTYPE outright is the categorical - // blocker (no DTD means no custom entities of any kind, external or internal); the two - // external-entity toggles are kept as explicit defense in depth alongside it. + return newSecureDocumentBuilderFactory().newDocumentBuilder(); + } + + /** + * Package-private (not {@code private}) solely so {@code ManifestParsersTest} can inspect the + * configured factory directly -- {@link DocumentBuilder} itself exposes no way to ask "what + * features was this built with" after the fact, and that inspection is what lets a test + * attribute each hardening switch individually rather than only as a bundle. No other caller + * exists or is expected; this is not a general-purpose factory. + * + *

pom.xml is untrusted repository content, not a file this process authored. Hardened per + * OWASP's XXE prevention cheat sheet: disallowing DOCTYPE outright is the categorical blocker + * (no DTD means no custom entities of any kind, external or internal); the two external-entity + * 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. + */ + static DocumentBuilderFactory newSecureDocumentBuilderFactory() throws ParserConfigurationException { DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); factory.setFeature("http://xml.org/sax/features/external-general-entities", false); factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - return factory.newDocumentBuilder(); + return factory; } private static List directChildren(Element parent, String tagName) { diff --git a/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java b/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java index 67177b64..e4f02345 100644 --- a/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java +++ b/src/test/java/com/ibm/cldk/artifacts/ManifestParsersTest.java @@ -2,11 +2,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.StringReader; import java.util.List; import java.util.Map; +import javax.xml.parsers.DocumentBuilderFactory; import org.junit.jupiter.api.Test; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; /** * Behavioural tests for {@link ManifestParsers}: one happy-path case per format, the Maven @@ -175,6 +180,83 @@ void parseManifest_pomXmlXxeProbeDoesNotResolveExternalEntity() { assertTrue(result.deps.isEmpty(), "no record may carry resolved external-entity content"); } + @Test + void parseManifest_pomXmlBareExternalDtdReferenceAlsoFailsClosed() { + // A different attack shape from the probe above: no inline at all -- just the + // DOCTYPE's own external subset, SYSTEM-referencing a local file. Also dominated by + // disallow-doctype-decl in production (see the factory-level tests below for why that + // dominance means this alone cannot attribute the other three switches), but it is a + // materially different payload shape and worth its own regression coverage. + String bareExternalDtd = "\n" + + "\n" + + "" + + "gprobe1.0" + + ""; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("pom.xml", bareExternalDtd); + + assertTrue(result.partial, "a bare external-DTD reference must also fail closed"); + assertTrue(result.deps.isEmpty()); + } + + @Test + void newSecureDocumentBuilderFactory_setsDisallowDoctypeAndBothExternalEntityFeatures() throws Exception { + // Each of these three has a Xerces *default* that is the opposite of the secure value used + // here (disallow-doctype-decl defaults to false; both external-entity features default to + // true) -- verified directly against a fresh, untouched factory. That means removing any one + // of the corresponding setFeature calls in newSecureDocumentBuilderFactory flips exactly one + // assertion below, with no dependence on Xerces' feature-checking order. This is what a + // behavioural probe cannot give us: disallow-doctype-decl rejects any DOCTYPE-bearing + // document before the other two are ever consulted, so a payload-based test cannot tell + // "external-general-entities is set correctly" apart from "it doesn't matter here anyway". + // + // FEATURE_SECURE_PROCESSING is deliberately not asserted here -- see the dedicated test + // below for why getFeature() cannot attribute it. + DocumentBuilderFactory factory = ManifestParsers.newSecureDocumentBuilderFactory(); + + assertTrue(factory.getFeature("http://apache.org/xml/features/disallow-doctype-decl")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-general-entities")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-parameter-entities")); + } + + @Test + void newSecureDocumentBuilderFactory_featureSecureProcessingBlocksABareExternalDtdFetchOnItsOwn() + throws Exception { + // FEATURE_SECURE_PROCESSING cannot be attributed by getFeature(): verified directly that it + // reads "true" on a completely untouched factory where setFeature was never called at all, + // so asserting the getter would pass whether or not the setFeature call in + // newSecureDocumentBuilderFactory exists. Only *calling* the setter activates the JAXP + // accessExternalDTD restriction this switch exists for -- confirmed by comparing "never + // called" (file gets read off disk, then fails to parse as a DTD) against "explicitly set + // true" (rejected before the file is ever opened, via accessExternalDTD). + // + // disallow-doctype-decl is deliberately peeled back to false on the real, production-built + // factory below (everything else from newSecureDocumentBuilderFactory is untouched): + // verified directly that with it left in place, removing FEATURE_SECURE_PROCESSING alone + // produces byte-identical behaviour (still rejected at the DOCTYPE token, before + // FEATURE_SECURE_PROCESSING is ever consulted) -- so peeling it back here is the only way to + // observe this switch's own, independent contribution. + DocumentBuilderFactory factory = ManifestParsers.newSecureDocumentBuilderFactory(); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", false); + + // No inline at all, so external-general-entities/external-parameter-entities + // (both still secure here) do not apply to this payload; only accessExternalDTD stands + // between this and reading the file off disk. + String bareExternalDtd = "\n" + + "\n" + + "" + + "gprobe1.0" + + ""; + + SAXException thrown = assertThrows(SAXException.class, + () -> factory.newDocumentBuilder().parse(new InputSource(new StringReader(bareExternalDtd)))); + assertTrue( + thrown.getMessage().contains("accessExternalDTD"), + "must be rejected via the accessExternalDTD restriction specifically -- i.e. before the file is " + + "ever read -- not merely rejected for some other reason; actual message: " + + thrown.getMessage()); + } + // ---- build.gradle / build.gradle.kts --------------------------------------------------- @Test @@ -251,6 +333,21 @@ void parseManifest_gradleUnmatchedLinesAreSkippedAndTheFileStillSucceeds() { assertEquals(1, result.deps.size()); } + @Test + void parseManifest_buildGradleNeverReturnsPartialRegardlessOfContent() { + // Structural, not incidental: the brief requires this branch to never fail, and + // parseManifest enforces it by giving pom.xml its own try/catch and none at all to the + // Gradle branch, rather than one catch shared across both with different contracts. This is + // the dedicated test for that contract, independent of any other Gradle test's specific + // input shape -- genuinely arbitrary, non-UTF-friendly-looking, brace-heavy garbage. + String garbage = " not gradle at all {{{{ )))) ][ \\\\ '''\" \n\n\timplementation\n"; + + ManifestParsers.ParseResult result = ManifestParsers.parseManifest("build.gradle", garbage); + + assertFalse(result.partial); + assertTrue(result.deps.isEmpty()); + } + // ---- gradle.lockfile -------------------------------------------------------------------- @Test From 47bef6468ab0bc5006337a92ff582fde2e2b5e8d Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 00:04:25 -0400 Subject: [PATCH 07/19] feat(artifacts): dependency view with lockfile reconciliation --- .../ibm/cldk/artifacts/DependencyView.java | 177 +++++++++++ .../cldk/artifacts/DependencyViewTest.java | 288 ++++++++++++++++++ 2 files changed, 465 insertions(+) create mode 100644 src/main/java/com/ibm/cldk/artifacts/DependencyView.java create mode 100644 src/test/java/com/ibm/cldk/artifacts/DependencyViewTest.java diff --git a/src/main/java/com/ibm/cldk/artifacts/DependencyView.java b/src/main/java/com/ibm/cldk/artifacts/DependencyView.java new file mode 100644 index 00000000..caa7da5c --- /dev/null +++ b/src/main/java/com/ibm/cldk/artifacts/DependencyView.java @@ -0,0 +1,177 @@ +package com.ibm.cldk.artifacts; + +import com.ibm.cldk.schema.JArtifact; +import com.ibm.cldk.schema.JDependency; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; + +/** + * Assembles the emitted {@link JDependency} list from every {@code dependency-manifest} artifact + * {@code ArtifactDiscovery} found, then reconciles it against any lockfile. Mirrors + * codeanalyzer-python's {@code artifacts/dependencies.py} two-step shape -- declare, then backfill + * from the lock -- but drops its import-binding layer entirely: python needs an alias table because + * a PyPI distribution name is not its import name, so a same-name guess is a genuine heuristic + * worth making. A Java package is declared at the call site with no such naming mismatch, so the + * analogous guess here would be strictly worse than emitting nothing -- {@code provides_imports}/ + * {@code unresolved_imports} are deliberately not ported. + */ +public final class DependencyView { + + private DependencyView() {} + + // The one basename ManifestParsers.parseLockPins recognizes today. Mirrors python's + // _LOCK_BASENAMES tuple (three entries, one per format pip's ecosystem has); Gradle has one. + private static final Set LOCK_BASENAMES = Set.of("gradle.lockfile"); + + /** + * Assemble declared dependencies, reconcile them against lock pins, and set each artifact's + * {@code extraction}. Mutates the artifacts' {@code extraction} in place; returns the sorted + * dependency list. + */ + public static List build(Path projectDir, Map artifacts) { + // Sorted once, iterated twice: both steps below must visit artifacts in a fixed order so + // that a tie in the final (name, declaredIn) sort -- or which lock artifact wins a pin + // collision -- resolves the same way on every run, independent of the caller's Map type. + Set paths = new TreeSet<>(artifacts.keySet()); + List deps = new ArrayList<>(); + + // 1. Declared: every dependency-manifest artifact that is not itself a lockfile (locks are + // handled by step 2 below, matching python's explicit _LOCK_BASENAMES skip here). + for (String path : paths) { + if (isLockfile(path)) { + continue; + } + JArtifact art = artifacts.get(path); + if (!art.getRoles().contains("dependency-manifest")) { + continue; + } + String text = readFromDisk(projectDir, path); + if (text == null) { + // Unreadable/undecodable (deleted mid-run, permissions, bad encoding). Must not + // call parseManifest with null: its pom.xml branch now propagates NPE on null text + // (the checked-exception catch was narrowed in task 3), so this guard is the + // precondition DependencyView owns as the first real caller -- flag and move on + // rather than crash or silently fall back to a possibly stale `source`. + art.setExtraction("partial"); + continue; + } + ManifestParsers.ParseResult result = ManifestParsers.parseManifest(path, text); + art.setExtraction(result.partial ? "partial" : "full"); + for (ManifestParsers.RawDep raw : result.deps) { + deps.add(declaredDependency(raw, art.getId())); + } + } + + // 2. Lock backfill: a pin matching a declared dependency (by group:name) sets its + // lockedVersion; a pin with no declaration is itself emitted as a transitive dependency. + // This is python's reconciliation (dependencies.py:181-186) and knowingly contradicts the + // spec's "transitive out of scope" -- see .github#48. + Map pins = new HashMap<>(); + Map pinLockArtifact = new HashMap<>(); + for (String path : paths) { + if (!isLockfile(path)) { + continue; + } + JArtifact art = artifacts.get(path); + String text = readFromDisk(projectDir, path); + if (text == null) { + art.setExtraction("partial"); + continue; + } + Map lockPins = ManifestParsers.parseLockPins(path, text); + pins.putAll(lockPins); + for (String key : lockPins.keySet()) { + pinLockArtifact.put(key, art.getId()); + } + // Real content that yields zero pins failed to parse (corrupt/unrecognized shape) -- + // "full" would be a false claim of clean extraction. A blank lock has nothing to + // extract, which is not a failure, so it still counts as "full". + art.setExtraction(!lockPins.isEmpty() || text.trim().isEmpty() ? "full" : "partial"); + } + + for (JDependency dep : deps) { + String lockedVersion = pins.get(dep.getGroup() + ":" + dep.getName()); + if (lockedVersion != null) { + dep.setLockedVersion(lockedVersion); + TreeSet prov = new TreeSet<>(dep.getProv()); + prov.add("lockfile"); + dep.setProv(new ArrayList<>(prov)); + } + } + Set declaredKeys = new HashSet<>(); + for (JDependency dep : deps) { + declaredKeys.add(dep.getGroup() + ":" + dep.getName()); + } + for (String key : new TreeSet<>(pins.keySet())) { + if (declaredKeys.contains(key)) { + continue; + } + deps.add(lockOnlyDependency(key, pins.get(key), pinLockArtifact.get(key))); + } + + deps.sort(Comparator.comparing(JDependency::getName).thenComparing(JDependency::getDeclaredIn)); + return deps; + } + + private static JDependency declaredDependency(ManifestParsers.RawDep raw, String declaredInId) { + JDependency dep = new JDependency(); + dep.setGroup(raw.group); + dep.setName(raw.name); + dep.setSpec(raw.spec); + dep.setKind(raw.kind); + dep.setExtras(raw.extras); + dep.setDeclaredIn(declaredInId); + dep.setProv(new ArrayList<>(List.of("declared"))); + return dep; + } + + private static JDependency lockOnlyDependency(String groupColonName, String lockedVersion, String lockArtifactId) { + JDependency dep = new JDependency(); + int colon = groupColonName.indexOf(':'); + dep.setGroup(groupColonName.substring(0, colon)); + dep.setName(groupColonName.substring(colon + 1)); + dep.setKind("runtime"); + dep.setDeclaredIn(lockArtifactId); + dep.setDirect(false); + dep.setLockedVersion(lockedVersion); + dep.setProv(new ArrayList<>(List.of("lockfile"))); + return dep; + } + + private static boolean isLockfile(String path) { + return LOCK_BASENAMES.contains(basename(path)); + } + + private static String basename(String path) { + int slash = path.lastIndexOf('/'); + return slash < 0 ? path : path.substring(slash + 1); + } + + /** + * Read an artifact's full text straight from disk -- never from {@link JArtifact#getSource()}. + * {@code source} is capped by {@code --artifact-text-max-bytes} and emptied outright by {@code + * --no-artifact-text} (both payload-size controls on the emitted JSON, not extraction + * controls), so extraction must not silently degrade under either flag. {@code null} means the + * file could not be read or is not valid UTF-8; the caller marks that artifact {@code partial} + * and skips it rather than handing unreliable text to a parser. + */ + private static String readFromDisk(Path projectDir, String relativePath) { + try { + byte[] raw = Files.readAllBytes(projectDir.resolve(relativePath)); + return StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(raw)).toString(); + } catch (IOException e) { + return null; + } + } +} diff --git a/src/test/java/com/ibm/cldk/artifacts/DependencyViewTest.java b/src/test/java/com/ibm/cldk/artifacts/DependencyViewTest.java new file mode 100644 index 00000000..b5b2b0f6 --- /dev/null +++ b/src/test/java/com/ibm/cldk/artifacts/DependencyViewTest.java @@ -0,0 +1,288 @@ +package com.ibm.cldk.artifacts; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.ibm.cldk.schema.CanId; +import com.ibm.cldk.schema.JArtifact; +import com.ibm.cldk.schema.JDependency; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Behavioural tests for {@link DependencyView}: the declared/lock-backfill two-step reconciliation, + * the lock-extraction status rule, and -- the regression this class exists to prevent -- that every + * read comes from disk rather than {@link JArtifact#getSource()}, which capture flags can empty or + * truncate. + */ +class DependencyViewTest { + + @Test + void build_declaredDependencyCarriesProvDeclaredAndDirectTrue(@TempDir Path tmp) throws IOException { + Files.writeString( + tmp.resolve("pom.xml"), + "" + + "org.examplewidget" + + "1.2.3" + + "", + StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = DependencyView.build(tmp, artifacts); + + assertEquals(1, deps.size()); + JDependency dep = deps.get(0); + assertEquals("org.example", dep.getGroup()); + assertEquals("widget", dep.getName()); + assertEquals("1.2.3", dep.getSpec()); + assertEquals("runtime", dep.getKind()); + assertTrue(dep.getExtras().isEmpty()); + assertTrue(dep.isDirect()); + assertEquals(List.of("declared"), dep.getProv()); + assertEquals(CanId.artifactId("app", "pom.xml"), dep.getDeclaredIn()); + assertNull(dep.getLockedVersion()); + assertEquals("full", artifacts.get("pom.xml").getExtraction()); + } + + @Test + void build_lockPinMatchingADeclaredDependencySetsLockedVersionWithoutDuplicating(@TempDir Path tmp) + throws IOException { + Files.writeString( + tmp.resolve("build.gradle"), + "dependencies { implementation 'com.google.guava:guava:31.1-jre' }\n", + StandardCharsets.UTF_8); + Files.writeString( + tmp.resolve("gradle.lockfile"), + "com.google.guava:guava:31.1-jre=compileClasspath,runtimeClasspath\n", + StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = DependencyView.build(tmp, artifacts); + + assertEquals(1, deps.size(), "a lock pin matching a declared dependency must not duplicate it"); + JDependency dep = deps.get(0); + assertEquals("com.google.guava", dep.getGroup()); + assertEquals("guava", dep.getName()); + assertEquals("31.1-jre", dep.getLockedVersion()); + assertTrue(dep.isDirect(), "still directly declared -- the lock only adds a pinned version"); + assertEquals(List.of("declared", "lockfile"), dep.getProv()); + assertEquals( + CanId.artifactId("app", "build.gradle"), dep.getDeclaredIn(), + "declaredIn stays the manifest that declared it, not the lock"); + } + + @Test + void build_lockPinWithNoDeclarationBecomesTransitiveDependency(@TempDir Path tmp) throws IOException { + Files.writeString( + tmp.resolve("build.gradle"), + "dependencies { implementation 'com.google.guava:guava:31.1-jre' }\n", + StandardCharsets.UTF_8); + Files.writeString( + tmp.resolve("gradle.lockfile"), + "com.google.guava:guava:31.1-jre=compileClasspath\n" + + "org.springframework:spring-core:5.3.21=compileClasspath\n", + StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = DependencyView.build(tmp, artifacts); + + assertEquals(2, deps.size()); + JDependency transitive = deps.get(0).getName().equals("spring-core") ? deps.get(0) : deps.get(1); + assertEquals("org.springframework", transitive.getGroup()); + assertEquals("spring-core", transitive.getName()); + assertEquals("runtime", transitive.getKind()); + assertFalse(transitive.isDirect(), "no manifest declares it -- it is lockfile-only"); + assertEquals(List.of("lockfile"), transitive.getProv()); + assertEquals("5.3.21", transitive.getLockedVersion()); + assertEquals( + CanId.artifactId("app", "gradle.lockfile"), transitive.getDeclaredIn(), + "an undeclared pin is attributed to the lock artifact itself"); + } + + @Test + void build_malformedManifestSetsArtifactPartialButOtherArtifactsStillSucceed(@TempDir Path tmp) + throws IOException { + // Truncated mid-element -- identical shape to ManifestParsersTest's malformed-pom fixture. + Files.writeString( + tmp.resolve("pom.xml"), "g", + StandardCharsets.UTF_8); + Files.writeString( + tmp.resolve("build.gradle"), + "dependencies { implementation 'org.springframework:spring-core:5.3.21' }\n", + StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = assertDoesNotThrow(() -> DependencyView.build(tmp, artifacts)); + + assertEquals("partial", artifacts.get("pom.xml").getExtraction()); + assertEquals(1, deps.size(), "the malformed manifest contributes nothing, but the run still succeeds"); + assertEquals("spring-core", deps.get(0).getName()); + assertEquals("full", artifacts.get("build.gradle").getExtraction()); + } + + @Test + void build_orderingIsStableAcrossTwoIndependentRuns(@TempDir Path tmp) throws IOException { + // Two dependencies named "apple", declared in two different manifests, force the secondary + // sort key (declaredIn) to matter -- a name-only sort could not distinguish them. + Files.writeString( + tmp.resolve("pom.xml"), + "" + + "g1zebra1.0" + + "g1apple2.0" + + "", + StandardCharsets.UTF_8); + Files.writeString( + tmp.resolve("build.gradle"), "dependencies { implementation 'g2:apple:3.0' }\n", + StandardCharsets.UTF_8); + + List first = DependencyView.build(tmp, ArtifactDiscovery.discover(tmp, "app", true, 262144)); + List second = DependencyView.build(tmp, ArtifactDiscovery.discover(tmp, "app", true, 262144)); + + List expectedNames = List.of("apple", "apple", "zebra"); + assertEquals(expectedNames, names(first)); + assertEquals(expectedNames, names(second), "two independent runs over the same input must agree"); + // (name, declaredIn) tie-break: "build.gradle" sorts before "pom.xml" lexicographically. + assertEquals(CanId.artifactId("app", "build.gradle"), first.get(0).getDeclaredIn()); + assertEquals(CanId.artifactId("app", "pom.xml"), first.get(1).getDeclaredIn()); + } + + @Test + void build_manifestParsesFromDiskEvenWhenSourceWasSuppressedByNoArtifactText(@TempDir Path tmp) + throws IOException { + Files.writeString( + tmp.resolve("pom.xml"), + "" + + "org.examplewidget" + + "1.2.3" + + "", + StandardCharsets.UTF_8); + // captureText=false, exactly what --no-artifact-text does: every artifact's source is "". + Map artifacts = ArtifactDiscovery.discover(tmp, "app", false, 262144); + assertEquals("", artifacts.get("pom.xml").getSource(), "sanity check: capture really is suppressed"); + + List deps = DependencyView.build(tmp, artifacts); + + assertEquals(1, deps.size(), "an empty `source` must not be mistaken for an empty manifest"); + assertEquals("widget", deps.get(0).getName()); + } + + @Test + void build_manifestParsesFromDiskEvenWhenSourceIsStaleOrWrong(@TempDir Path tmp) throws IOException { + // The adversarial version of the above: `source` is not merely empty, it is a *different*, + // syntactically valid manifest. An implementation that ever prefers `source` when it happens + // to be non-empty would silently emit the wrong dependency instead of failing loudly. + Files.writeString( + tmp.resolve("pom.xml"), + "" + + "real.groupreal-artifact" + + "9.9.9" + + "", + StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + artifacts.get("pom.xml").setSource( + "" + + "WRONGwrong-artifact" + + "0.0.1" + + ""); + + List deps = DependencyView.build(tmp, artifacts); + + assertEquals(1, deps.size()); + assertEquals("real.group", deps.get(0).getGroup()); + assertEquals("real-artifact", deps.get(0).getName()); + assertEquals("9.9.9", deps.get(0).getSpec()); + } + + @Test + void build_blankLockfileExtractionIsFullNotPartial(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("gradle.lockfile"), "", StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = DependencyView.build(tmp, artifacts); + + assertTrue(deps.isEmpty()); + assertEquals("full", artifacts.get("gradle.lockfile").getExtraction(), "nothing to extract is not a failure"); + } + + @Test + void build_garbageLockfileWithContentExtractionIsPartial(@TempDir Path tmp) throws IOException { + Files.writeString( + tmp.resolve("gradle.lockfile"), "this is not a lockfile\n{{{ garbage }}}\n", StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = DependencyView.build(tmp, artifacts); + + assertTrue(deps.isEmpty()); + assertEquals( + "partial", artifacts.get("gradle.lockfile").getExtraction(), + "real content that yields zero pins must not be reported as a clean extraction"); + } + + @Test + void build_nonManifestArtifactIsIgnoredEntirely(@TempDir Path tmp) throws IOException { + Files.writeString(tmp.resolve("README.md"), "# hello", StandardCharsets.UTF_8); + Map artifacts = ArtifactDiscovery.discover(tmp, "app", true, 262144); + + List deps = DependencyView.build(tmp, artifacts); + + assertTrue(deps.isEmpty()); + assertEquals( + "none", artifacts.get("README.md").getExtraction(), + "DependencyView must not touch an artifact outside its role/basename gates"); + } + + @Test + void build_unreadableManifestFileBecomesPartialInsteadOfCrashing(@TempDir Path tmp) { + // No file is ever written to `tmp` -- simulates a manifest that vanished from disk between + // discovery and this pass (or any other disk-read failure). parseManifest's pom.xml branch + // now propagates NPE on null text (the checked-exception catch was narrowed in task 3), so + // DependencyView must guard the precondition itself rather than ever calling it with + // unreadable text. + JArtifact art = new JArtifact(); + art.setId(CanId.artifactId("app", "pom.xml")); + art.setPath("pom.xml"); + art.setRoles(List.of("dependency-manifest")); + Map artifacts = new HashMap<>(); + artifacts.put("pom.xml", art); + + List deps = assertDoesNotThrow(() -> DependencyView.build(tmp, artifacts)); + + assertNotNull(deps); + assertTrue(deps.isEmpty()); + assertEquals("partial", art.getExtraction()); + } + + @Test + void build_unreadableLockfileBecomesPartialInsteadOfCrashing(@TempDir Path tmp) { + // Same guard as above, exercised through the lock-backfill loop instead of the declared + // loop -- a separate code path (parseLockPins is already null-safe internally, but + // DependencyView must not rely on that; it applies the same disk-read guard uniformly). + JArtifact art = new JArtifact(); + art.setId(CanId.artifactId("app", "gradle.lockfile")); + art.setPath("gradle.lockfile"); + art.setRoles(List.of("dependency-manifest")); + Map artifacts = new HashMap<>(); + artifacts.put("gradle.lockfile", art); + + List deps = assertDoesNotThrow(() -> DependencyView.build(tmp, artifacts)); + + assertTrue(deps.isEmpty()); + assertEquals("partial", art.getExtraction()); + } + + private static List names(List deps) { + return deps.stream().map(JDependency::getName).collect(Collectors.toList()); + } +} From 800df6a7ec4f1876ca9cc8bf1720c4146ecfdc5a Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 00:24:56 -0400 Subject: [PATCH 08/19] fix(artifacts): widen readFromDisk to public so task 6 can reuse it Keeps the from-disk helper written exactly once, per the design plan's constraint, instead of task 6 recreating the same three lines in CodeAnalyzer.java the way the python reference duplicates it across dependencies.py and core.py. --- src/main/java/com/ibm/cldk/artifacts/DependencyView.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/DependencyView.java b/src/main/java/com/ibm/cldk/artifacts/DependencyView.java index caa7da5c..b2a37afe 100644 --- a/src/main/java/com/ibm/cldk/artifacts/DependencyView.java +++ b/src/main/java/com/ibm/cldk/artifacts/DependencyView.java @@ -165,8 +165,15 @@ private static String basename(String path) { * controls), so extraction must not silently degrade under either flag. {@code null} means the * file could not be read or is not valid UTF-8; the caller marks that artifact {@code partial} * and skips it rather than handing unreliable text to a parser. + * + *

Deliberately {@code public}, a narrow named exception to this class's "public surface is + * just {@code build}" rule: the design plan requires this from-disk logic to exist exactly + * once, in contrast to the reference implementation, which duplicates it verbatim across two + * modules with a "keep the two in sync" comment. The config-key extraction pass ({@code + * ConfigKeys}/{@code CodeAnalyzer}, package {@code com.ibm.cldk}) needs the identical read and + * must call this rather than re-write it -- do not shrink this back to {@code private}. */ - private static String readFromDisk(Path projectDir, String relativePath) { + public static String readFromDisk(Path projectDir, String relativePath) { try { byte[] raw = Files.readAllBytes(projectDir.resolve(relativePath)); return StandardCharsets.UTF_8.newDecoder().decode(ByteBuffer.wrap(raw)).toString(); From 8d2e617f5498ac726f836e424feeb3b95e7a5717 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 00:59:51 -0400 Subject: [PATCH 09/19] feat(artifacts): config-key flattening for properties, yaml, xml and dockerfile --- build.gradle | 5 + .../com/ibm/cldk/artifacts/ConfigKeys.java | 601 ++++++++++++++++++ .../ibm/cldk/artifacts/ConfigKeysTest.java | 325 ++++++++++ 3 files changed, 931 insertions(+) create mode 100644 src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java create mode 100644 src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java diff --git a/build.gradle b/build.gradle index 468e2ea5..47048045 100644 --- a/build.gradle +++ b/build.gradle @@ -118,6 +118,11 @@ dependencies { implementation('org.json:json:20231013') implementation('com.google.code.gson:gson:2.10.1') + // YAML config-key flattening (ConfigKeys, task 5 of the repository-artifact layer). Declared + // explicitly at the version already resolved transitively in this project's dependency graph + // (org.yaml:snakeyaml:2.2) -- relying on the transitive alone is how a build breaks the day an + // unrelated dependency drops it. + implementation('org.yaml:snakeyaml:2.2') implementation('org.jgrapht:jgrapht-core:1.5.2') implementation('org.jgrapht:jgrapht-io:1.5.2') implementation('org.jgrapht:jgrapht-ext:1.5.2') diff --git a/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java b/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java new file mode 100644 index 00000000..c7d98f66 --- /dev/null +++ b/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java @@ -0,0 +1,601 @@ +package com.ibm.cldk.artifacts; + +import com.ibm.cldk.schema.CanId; +import com.ibm.cldk.schema.JArtifact; +import com.ibm.cldk.schema.JConfigKey; +import com.ibm.cldk.schema.Span; +import com.ibm.cldk.schema.Spans; +import java.io.IOException; +import java.io.StringReader; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import javax.xml.parsers.ParserConfigurationException; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.yaml.snakeyaml.Yaml; + +/** + * Flattens config-bearing artifacts into {@link JConfigKey} records: pure text-in/records-out, + * same idiom as {@link ManifestParsers} -- one dispatcher over per-format internals, never + * throwing. Mirrors codeanalyzer-python's {@code artifacts/config_keys.py}, but narrower: only + * {@code properties}/{@code yaml}/{@code xml}/{@code dockerfile} plus env-family basenames are + * handled here (python's {@code json}/{@code toml}/{@code ini} vocabulary has no Java-side + * artifact format to hang off, per the plan). The {@code xml} flattener is net-new -- python has + * none to emulate -- and is deliberately the simplest scheme that could work (see {@link + * #walkXml}). + * + *

Namespace dispatch: an env-family basename ({@code .env}, {@code .env.*}) always wins, + * regardless of the artifact's declared {@code format} (matching {@code ArtifactDiscovery}, which + * assigns these {@code format="text"}); otherwise the {@code format} field selects + * yaml/xml/dockerfile/properties. Any other format extracts nothing -- not a failure, there is + * just nothing to flatten. + * + *

A {@code dockerfile}-format artifact mints TWO namespaces from one file: {@code ENV K=v} + * directives mint namespace {@code env} (so an {@code os.getenv}-style reader binds to them), and + * {@code ARG K[=default]} directives mint namespace {@code dockerfile} (build-time only). Both + * share the bare var name as {@code key}; since {@code ARG X} followed by a promotion idiom like + * {@code ENV X=$X} is common, the {@code ARG} mint's id (not its {@code key} field) is + * disambiguated with an {@code arg.} prefix so the two do not collide. A {@code yaml}-format + * artifact additionally recognizes a compose {@code services..environment} map and + * dual-mints those into namespace {@code env} too, on the bare var name, with the same kind of + * collision guard (an {@code env.} id prefix, since a top-level yaml key could otherwise share a + * bare name with a recognized env var). + * + *

{@code value} is populated only when {@code captureValue} is {@code true}; {@code key}, + * {@code namespace}, {@code span}, and {@code references} are extracted unconditionally either + * way. {@code span} is best-effort and frequently {@code null}: {@code dockerfile}/env-family + * files are line-oriented so the parse itself knows the exact line, but {@code properties} (parsed + * via {@link Properties}, which exposes no position info) and {@code yaml}/{@code xml} (tree-shaped) + * carry no span at all here -- an accepted gap, matching the brief ("best-effort... null is + * acceptable"; python's own yaml/json/toml spans are already only a regex approximation). + */ +public final class ConfigKeys { + + private ConfigKeys() {} + + /** Flattened keys plus a success flag. {@code ok=false} means a parse failure. */ + public static final class Result { + public final List keys; + public final boolean ok; + + Result(List keys, boolean ok) { + this.keys = keys; + this.ok = ok; + } + } + + // A parsed leaf before it becomes a JConfigKey: dotted/element/var key, raw value, best-effort span. + private static final class Entry { + final String key; + final Object value; + final Span span; + + Entry(String key, Object value, Span span) { + this.key = key; + this.value = value; + this.span = span; + } + } + + private static final Set ELIGIBLE_FORMATS = Set.of("properties", "yaml", "xml", "dockerfile"); + + /** + * Whether {@code artifact} is worth extracting config keys from: an env-family basename + * ({@code .env}/{@code .env.*}, regardless of declared format), or one of the namespace-bearing + * formats. A {@code binary} artifact is never eligible -- there is no decodable text to + * flatten, and {@code ArtifactDiscovery} downgrades a rule-matched-but-undecodable file to + * {@code format="binary"} regardless of its basename, so that check wins even over an + * env-family name. {@code format="gradle"} is deliberately absent: a build script is a + * program, not a config document (Task 3 already reads its dependencies with a shallow regex); + * {@code gradle.properties} carries {@code format="properties"} and is where Gradle's actual + * key-value config lives. + */ + public static boolean isEligible(JArtifact artifact) { + String format = artifact.getFormat(); + if ("binary".equals(format)) { + return false; + } + // format != null guards Set.of(...)'s contains(), which throws NPE on a null argument + // (its open-addressing probe uses null as an internal sentinel) -- format is nullable on a + // hand-built JArtifact even though real discovery output always sets it. + return isEnvFamily(basename(artifact.getPath())) || (format != null && ELIGIBLE_FORMATS.contains(format)); + } + + /** + * Flatten {@code artifact}'s config format into {@link JConfigKey} records, reading {@code + * text} (the caller's job to supply the real on-disk text, never a possibly-truncated {@code + * artifact.source} -- see {@link DependencyView#readFromDisk}). + * + *

Never throws: the entire per-format dispatch is one {@code try}/{@code catch}, so a + * malformed file (bad YAML, bad XML) degrades to {@code (empty, false)} instead of an + * exception escaping to the caller -- the same {@code (records, ok)} shape as {@link + * ManifestParsers}, opposite polarity: {@code ok=true} includes the not-applicable case (a + * format with no flattener yields {@code (empty, true)}), {@code false} only on a genuine parse + * failure. The returned list is always sorted by {@code key} -- a dockerfile or dual-minted + * yaml artifact concatenates its namespace groups before this one sort, and Java's sort is + * stable, so a same-key tie across namespaces still resolves deterministically. + */ + public static Result extract(JArtifact artifact, String text, boolean captureValue) { + String basename = basename(artifact.getPath()); + try { + List keys; + if (isEnvFamily(basename)) { + keys = buildKeys(artifact.getId(), "env", parseEnvFile(text), captureValue, false, null); + } else if ("dockerfile".equals(artifact.getFormat())) { + keys = new ArrayList<>(buildKeys( + artifact.getId(), "env", dockerfileEnvEntries(text), captureValue, true, null)); + keys.addAll(buildKeys( + artifact.getId(), "dockerfile", dockerfileArgEntries(text), captureValue, true, + k -> "arg." + k)); + } else if ("properties".equals(artifact.getFormat())) { + keys = buildKeys(artifact.getId(), "properties", parseProperties(text), captureValue, false, null); + } else if ("yaml".equals(artifact.getFormat())) { + Object data = new Yaml().load(text); + List flat = flatten(data == null ? new LinkedHashMap<>() : data, ""); + keys = new ArrayList<>(buildKeys(artifact.getId(), "yaml", flat, captureValue, false, null)); + keys.addAll(buildKeys( + artifact.getId(), "env", recognizeComposeEnv(flat), captureValue, false, + k -> "env." + k)); + } else if ("xml".equals(artifact.getFormat())) { + keys = extractXml(artifact.getId(), text, captureValue); + } else { + return new Result(List.of(), true); + } + keys.sort(Comparator.comparing(JConfigKey::getKey)); + return new Result(keys, true); + } catch (Exception e) { + return new Result(List.of(), false); + } + } + + // ---- 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 + // info it does not expose (span stays null; see the class javadoc). ------------------------ + + private static List parseProperties(String text) throws IOException { + Properties props = new Properties(); + props.load(new StringReader(text)); + List out = new ArrayList<>(); + for (String name : props.stringPropertyNames()) { + out.add(new Entry(name, props.getProperty(name), null)); + } + return out; + } + + // ---- yaml: SnakeYAML `load`, flattened to dotted paths with numeric segments for list + // indices, plus a supplemental compose-environment recognition pass over the same flattened + // pairs (see the class javadoc). `new Yaml()` (no custom Constructor) is the hardened default + // as of SnakeYAML 2.x -- it rejects arbitrary-type tags rather than instantiating them, which + // matters here because these files are untrusted repository content. ------------------------ + + private static List flatten(Object obj, String prefix) { + List out = new ArrayList<>(); + if (obj instanceof Map) { + for (Map.Entry e : ((Map) obj).entrySet()) { + String key = String.valueOf(e.getKey()); + out.addAll(flatten(e.getValue(), prefix.isEmpty() ? key : prefix + "." + key)); + } + } else if (obj instanceof List) { + List list = (List) obj; + for (int i = 0; i < list.size(); i++) { + out.addAll(flatten(list.get(i), prefix.isEmpty() ? String.valueOf(i) : prefix + "." + i)); + } + } else { + out.add(new Entry(prefix, obj, null)); + } + return out; + } + + // Compose's `services..environment` block only (a map of KEY: value, or a list of + // "KEY=value"/bare "KEY" strings) -- python's k8s `env[].name`/`.value` list-shape recognition + // is deliberately not ported, the brief names compose only. + private static final Pattern COMPOSE_ENV = Pattern.compile("^services\\.[^.]+\\.environment\\.(.+)$"); + + private static List recognizeComposeEnv(List flat) { + List out = new ArrayList<>(); + for (Entry e : flat) { + Matcher m = COMPOSE_ENV.matcher(e.key); + if (!m.matches()) { + continue; + } + String tail = m.group(1); + if (ENV_KEY_NAME.matcher(tail).matches()) { + out.add(new Entry(tail, e.value, null)); // map form: tail IS the var name + } else if (isDigits(tail)) { + String s = stringify(e.value); // list form: leaf is "KEY=val" or bare "KEY" + int eq = s.indexOf('='); + String key = eq >= 0 ? s.substring(0, eq) : s; + if (ENV_KEY_NAME.matcher(key).matches()) { + out.add(new Entry(key, eq >= 0 ? s.substring(eq + 1) : null, null)); + } + } + } + return out; + } + + private static boolean isDigits(String s) { + if (s.isEmpty()) { + return false; + } + for (int i = 0; i < s.length(); i++) { + if (!Character.isDigit(s.charAt(i))) { + return false; + } + } + return true; + } + + // ---- xml: net-new, python has no XML flattener to emulate. Deliberately the simplest scheme + // that could work -- see walkXml -- not a richer one (no XPath predicates, no namespace-aware + // qualified-name handling). Reuses ManifestParsers' hardened DocumentBuilderFactory rather than + // building a second one: untrusted repository content gets the same XXE hardening either way. - + + private static List extractXml(String artifactId, String text, boolean captureValue) + throws ParserConfigurationException, SAXException, IOException { + Document doc = ManifestParsers.newSecureDocumentBuilderFactory() + .newDocumentBuilder() + .parse(new InputSource(new StringReader(text))); + Element root = doc.getDocumentElement(); + List entries = new ArrayList<>(); + walkXml(root, root.getTagName(), entries); + return buildKeys(artifactId, "xml", entries, captureValue, false, null); + } + + /** + * Element paths, dot-joined by tag name; a numeric segment is added ONLY for a tag name + * repeated among its siblings (a single occurrence keeps a clean path, e.g. {@code + * "server.port"} rather than {@code "server.0.port"}). Attributes flatten as {@code "path@attr"} + * at their own element's path. That is the whole scheme -- kept simple deliberately, per brief. + */ + private static void walkXml(Element element, String path, List out) { + NamedNodeMap attrs = element.getAttributes(); + for (int i = 0; i < attrs.getLength(); i++) { + Node attr = attrs.item(i); + out.add(new Entry(path + "@" + attr.getNodeName(), attr.getNodeValue(), null)); + } + List children = directChildElements(element); + if (children.isEmpty()) { + out.add(new Entry(path, element.getTextContent().trim(), null)); + return; + } + Map> byTag = new LinkedHashMap<>(); + for (Element child : children) { + byTag.computeIfAbsent(child.getTagName(), k -> new ArrayList<>()).add(child); + } + for (Map.Entry> group : byTag.entrySet()) { + List siblings = group.getValue(); + if (siblings.size() == 1) { + walkXml(siblings.get(0), path + "." + group.getKey(), out); + } else { + for (int i = 0; i < siblings.size(); i++) { + walkXml(siblings.get(i), path + "." + group.getKey() + "." + i, out); + } + } + } + } + + private static List directChildElements(Element parent) { + List children = new ArrayList<>(); + NodeList nodes = parent.getChildNodes(); + for (int i = 0; i < nodes.getLength(); i++) { + Node node = nodes.item(i); + if (node.getNodeType() == Node.ELEMENT_NODE) { + children.add((Element) node); + } + } + return children; + } + + // ---- dockerfile: `ENV`/`ARG` directives. Line-based, case-insensitive instruction keywords + // (Dockerfile convention is uppercase; the spec itself is not case-sensitive). + // + // Two known gaps, carried over from python rather than rediscovered: no BuildKit heredoc + // awareness (a heredoc body line is just another line that doesn't match ENV/ARG and is + // silently skipped, same as any other unparseable line); no multi-stage `FROM ... AS` scoping + // (every ENV/ARG in the file is scanned regardless of which stage it is in). -------------- + + private static final Pattern DOCKER_ENV = Pattern.compile("^ENV\\s+(.*)$", Pattern.CASE_INSENSITIVE); + private static final Pattern DOCKER_ARG = Pattern.compile("^ARG\\s+(.*)$", Pattern.CASE_INSENSITIVE); + + // Joins a possibly backslash-continued logical instruction line, starting at lines[startIdx]. + // No separator is inserted between joined parts (matching python) -- a space survives only if + // it was already present before the continuation backslash on the physical line. + private static final class Joined { + final String text; + final int lastIndex; + + Joined(String text, int lastIndex) { + this.text = text; + this.lastIndex = lastIndex; + } + } + + private static Joined joinContinuations(String[] lines, int startIdx) { + int i = startIdx; + StringBuilder joined = new StringBuilder(lines[i].trim()); + while (joined.length() > 0 && joined.charAt(joined.length() - 1) == '\\' && i + 1 < lines.length) { + joined.setLength(joined.length() - 1); // drop just the continuation backslash + i++; + joined.append(lines[i].trim()); + } + return new Joined(joined.toString(), i); + } + + /** + * Whitespace-split {@code s}, except inside a matching quote span (a quoted value may contain + * spaces) or right after an unquoted {@code \} (a backslash escapes the next character). Quote + * characters stay IN the returned tokens, stripped afterward by {@link #envValue} so there is + * one quote-stripping implementation, not two. Direct port of python's {@code + * _split_ws_respecting_quotes}, for Docker's shell-style {@code ENV} splitting. + */ + private static List splitWsRespectingQuotes(String s) { + List tokens = new ArrayList<>(); + StringBuilder buf = new StringBuilder(); + Character quote = null; + int i = 0; + int n = s.length(); + while (i < n) { + char ch = s.charAt(i); + if (quote != null) { + buf.append(ch); + if (ch == quote) { + quote = null; + } + } else if (ch == '\\') { + i++; + buf.append(i < n ? s.charAt(i) : ch); + } else if (ch == '\'' || ch == '"') { + quote = ch; + buf.append(ch); + } else if (Character.isWhitespace(ch)) { + if (buf.length() > 0) { + tokens.add(buf.toString()); + buf.setLength(0); + } + } else { + buf.append(ch); + } + i++; + } + if (buf.length() > 0) { + tokens.add(buf.toString()); + } + return tokens; + } + + private static List dockerfileEnvEntries(String text) { + List out = new ArrayList<>(); + String[] lines = text.split("\n", -1); + int i = 0; + while (i < lines.length) { + String stripped = lines[i].trim(); + if (stripped.isEmpty() || stripped.startsWith("#") || !DOCKER_ENV.matcher(stripped).matches()) { + i++; + continue; + } + int startLineno = i + 1; + Joined joined = joinContinuations(lines, i); + i = joined.lastIndex; + // Re-run against the JOINED text (continuation lines add content after "ENV "): still + // guaranteed to match, since (.*) is greedy to end-of-string regardless of length. + Matcher dm = DOCKER_ENV.matcher(joined.text); + dm.matches(); + String rest = dm.group(1).trim(); + Span span = lineSpan(text, lines[startLineno - 1], startLineno); + String firstToken = rest.isEmpty() ? "" : rest.split("\\s+", 2)[0]; + if (firstToken.contains("=")) { + // multi-key form: ENV a=1 b=2 + for (String tok : splitWsRespectingQuotes(rest)) { + int eq = tok.indexOf('='); + if (eq >= 0) { + String key = tok.substring(0, eq); + if (ENV_KEY_NAME.matcher(key).matches()) { + out.add(new Entry(key, envValue(tok.substring(eq + 1)), span)); + } + } + } + } else { + // legacy single-key form: ENV KEY value -- value taken verbatim, no quote processing + String[] parts = rest.split("\\s+", 2); + if (parts.length == 2 && ENV_KEY_NAME.matcher(parts[0]).matches()) { + out.add(new Entry(parts[0], parts[1], span)); + } + } + i++; + } + return out; + } + + private static List dockerfileArgEntries(String text) { + List out = new ArrayList<>(); + String[] lines = text.split("\n", -1); + int i = 0; + while (i < lines.length) { + String stripped = lines[i].trim(); + if (stripped.isEmpty() || stripped.startsWith("#") || !DOCKER_ARG.matcher(stripped).matches()) { + i++; + continue; + } + int startLineno = i + 1; + Joined joined = joinContinuations(lines, i); + i = joined.lastIndex; + // Same re-run-against-the-joined-text guarantee as dockerfileEnvEntries above. + Matcher dm = DOCKER_ARG.matcher(joined.text); + dm.matches(); + String rest = dm.group(1).trim(); + Span span = lineSpan(text, lines[startLineno - 1], startLineno); + int eq = rest.indexOf('='); + String key = (eq >= 0 ? rest.substring(0, eq) : rest).trim(); + if (ENV_KEY_NAME.matcher(key).matches()) { + // No "=default" means value=null (distinct from "" for an explicitly empty value). + out.add(new Entry(key, eq >= 0 ? envValue(rest.substring(eq + 1)) : null, span)); + } + i++; + } + return out; + } + + // ---- env-family files (.env, .env.*): `KEY=value`, `#` comments, optional `export ` prefix, + // quote stripping. Shared with dockerfile's ENV/ARG value handling via envValue. ------------- + + private static final Pattern ENV_KEY_NAME = Pattern.compile("^[A-Za-z_][A-Za-z0-9_]*$"); + private static final Pattern ENV_LINE = Pattern.compile("^(?:export\\s+)?([A-Za-z_][A-Za-z0-9_]*)\\s*=\\s*(.*)$"); + private static final Pattern COMMENT_MARKER = Pattern.compile("\\s#"); + + /** + * The text after {@code KEY=} on one line -> the value. A quoted value ends at its MATCHING + * closing quote (anything after, including a {@code #}, is a discarded trailing comment); an + * unquoted value ends at the first unescaped whitespace-then-{@code #} (a bare {@code #} stuck + * directly to a token is not a comment marker). + */ + private static String envValue(String raw) { + raw = raw.trim(); + if (!raw.isEmpty() && (raw.charAt(0) == '\'' || raw.charAt(0) == '"')) { + char quote = raw.charAt(0); + int end = raw.indexOf(quote, 1); + return end != -1 ? raw.substring(1, end) : raw.substring(1); + } + Matcher m = COMMENT_MARKER.matcher(raw); + return (m.find() ? raw.substring(0, m.start()) : raw).trim(); + } + + private static boolean isEnvFamily(String basename) { + return ".env".equals(basename) || basename.startsWith(".env."); + } + + private static List parseEnvFile(String text) { + List out = new ArrayList<>(); + String[] lines = text.split("\n", -1); + for (int i = 0; i < lines.length; i++) { + String stripped = lines[i].trim(); + if (stripped.isEmpty() || stripped.startsWith("#")) { + continue; + } + Matcher m = ENV_LINE.matcher(stripped); + if (!m.matches()) { + continue; + } + out.add(new Entry(m.group(1), envValue(m.group(2)), lineSpan(text, lines[i], i + 1))); + } + return out; + } + + // ---- reference recognition: `${...}` first, masked before the bare `$VAR` scan so `${A}` + // does not also yield a spurious `$A`. Java's dominant form is a DOTTED property path (Spring + // placeholder / Maven property, e.g. `${spring.datasource.url}`), unlike a shell env var -- + // hence the braced identifier class allows `.` where the bare one deliberately does not (a + // shell variable name never contains a dot). No `${{ ... }}` template or `%(name)s` + // percent-interpolation form: those are python-reference-only vocabulary the brief does not + // name for Java. -------------------------------------------------------------------------- + + private static final Pattern REF_BRACED = Pattern.compile("\\$\\{[A-Za-z_][A-Za-z0-9_.]*\\}"); + private static final Pattern REF_BARE = Pattern.compile("\\$[A-Za-z_][A-Za-z0-9_]*"); + + private static final class Ref { + final int pos; + final String token; + + Ref(int pos, String token) { + this.pos = pos; + this.token = token; + } + } + + private static List findReferences(String text) { + List found = new ArrayList<>(); + StringBuilder masked = new StringBuilder(text); + Matcher braced = REF_BRACED.matcher(text); + while (braced.find()) { + found.add(new Ref(braced.start(), braced.group())); + for (int i = braced.start(); i < braced.end(); i++) { + masked.setCharAt(i, ' '); + } + } + Matcher bare = REF_BARE.matcher(masked); + while (bare.find()) { + found.add(new Ref(bare.start(), bare.group())); + } + found.sort(Comparator.comparingInt(r -> r.pos)); + List out = new ArrayList<>(); + Set seen = new HashSet<>(); + for (Ref r : found) { + if (seen.add(r.token)) { + out.add(r.token); + } + } + return out; + } + + // ---- shared: coalesce, stringify, span, id ------------------------------------------------ + + private static String stringify(Object value) { + if (value == null) { + return ""; + } + if (value instanceof Boolean) { // yaml spells booleans lowercase on disk + return ((Boolean) value) ? "true" : "false"; + } + return String.valueOf(value); + } + + /** + * Coalesce {@code entries} (last occurrence in file order wins -- e.g. a redefined env var) + * into {@link JConfigKey} records for one namespace. {@code rawValue=true} (dockerfile) passes + * the parsed value straight through instead of {@link #stringify}-ing it, so an ARG's absent + * default surfaces as {@code value=null} rather than {@code stringify}'s {@code ""} for a + * modeled null. {@code idKey} remaps the dotted/bare key for ID CONSTRUCTION only -- the {@code + * key} FIELD always stays the bare key; see the class javadoc for why the dockerfile ARG and + * yaml env-dual-mint call sites need it. + */ + private static List buildKeys( + String artifactId, String namespace, List entries, boolean captureValue, + boolean rawValue, Function idKey) { + Map coalesced = new LinkedHashMap<>(); + for (Entry e : entries) { + coalesced.put(e.key, e); + } + List out = new ArrayList<>(); + for (Entry e : coalesced.values()) { + String textValue = rawValue ? (String) e.value : stringify(e.value); + JConfigKey key = new JConfigKey(); + key.setId(CanId.configKeyId(artifactId, idKey != null ? idKey.apply(e.key) : e.key)); + key.setKey(e.key); + key.setNamespace(namespace); + key.setValue(captureValue ? textValue : null); + key.setSpan(e.span); + key.setReferences(findReferences(textValue == null ? "" : textValue)); + out.add(key); + } + return out; + } + + // Exact line span (1-based [line,col], matching this schema's JavaParser-native convention -- + // see Span's javadoc), reusing Spans' UTF-8 byte-offset math rather than hand-rolling it. + private static Span lineSpan(String text, String line, int lineno) { + Span span = new Span(); + span.setStart(new int[] {lineno, 1}); + span.setEnd(new int[] {lineno, line.length() + 1}); + span.setBytes(Spans.byteOffsets(text, lineno, 0, lineno, line.length())); + return span; + } + + private static String basename(String path) { + int slash = path.lastIndexOf('/'); + return slash < 0 ? path : path.substring(slash + 1); + } +} diff --git a/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java b/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java new file mode 100644 index 00000000..bce676aa --- /dev/null +++ b/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java @@ -0,0 +1,325 @@ +package com.ibm.cldk.artifacts; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.ibm.cldk.schema.CanId; +import com.ibm.cldk.schema.JArtifact; +import com.ibm.cldk.schema.JConfigKey; +import java.util.List; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +/** + * Behavioural tests for {@link ConfigKeys}: one flattening case per eligible format, the + * dockerfile ARG/ENV id-collision guard, the {@code ${...}}-before-{@code $VAR} reference + * masking, the {@code captureValue} gate, and the {@code isEligible} admit list (including the + * two precedence rules python documents: binary wins over an env-family name, and env-family + * basenames are admitted regardless of declared format). + */ +class ConfigKeysTest { + + private static JArtifact artifact(String path, String format) { + JArtifact a = new JArtifact(); + a.setId(CanId.artifactId("app", path)); + a.setPath(path); + a.setFormat(format); + return a; + } + + private static Optional find(List keys, String key) { + return keys.stream().filter(k -> k.getKey().equals(key)).findFirst(); + } + + // ---- properties ------------------------------------------------------------------------- + + @Test + void extract_propertiesFileYieldsDottedKeysWithLastWins() { + JArtifact art = artifact("application.properties", "properties"); + String text = "a.b.c=1\nfoo=bar\na.b.c=2\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals(2, result.keys.size()); + JConfigKey abc = find(result.keys, "a.b.c").orElseThrow(); + assertEquals("2", abc.getValue(), "last occurrence wins"); + assertEquals("properties", abc.getNamespace()); + assertEquals(CanId.configKeyId(art.getId(), "a.b.c"), abc.getId()); + assertEquals("bar", find(result.keys, "foo").orElseThrow().getValue()); + } + + @Test + void extract_propertiesColonSeparatorAndCommentsAreHandledNatively() { + JArtifact art = artifact("application.properties", "properties"); + String text = "! a bang comment\n# a hash comment\nserver.port: 8080\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals(1, result.keys.size()); + assertEquals("8080", result.keys.get(0).getValue()); + } + + // ---- yaml --------------------------------------------------------------------------------- + + @Test + void extract_springApplicationYmlYieldsServerPortAndAListIndex() { + JArtifact art = artifact("application.yml", "yaml"); + String text = "server:\n port: 8080\nservers:\n - host: a\n - host: b\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals("8080", find(result.keys, "server.port").orElseThrow().getValue()); + assertEquals("yaml", find(result.keys, "server.port").orElseThrow().getNamespace()); + assertEquals("a", find(result.keys, "servers.0.host").orElseThrow().getValue()); + assertEquals("b", find(result.keys, "servers.1.host").orElseThrow().getValue()); + } + + @Test + void extract_yamlComposeEnvironmentBlockDualMintsEnvNamespaceWithDisambiguatedId() { + JArtifact art = artifact("docker-compose.yml", "yaml"); + String text = "services:\n web:\n environment:\n DB_HOST: localhost\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + // The plain yaml-namespace dotted path is still minted... + JConfigKey yamlKey = find(result.keys, "services.web.environment.DB_HOST").orElseThrow(); + assertEquals("yaml", yamlKey.getNamespace()); + // ...alongside a dual-minted env-namespace key on the bare var name, id-disambiguated. + List envKeys = result.keys.stream() + .filter(k -> "env".equals(k.getNamespace()) && k.getKey().equals("DB_HOST")) + .collect(java.util.stream.Collectors.toList()); + assertEquals(1, envKeys.size()); + assertEquals("localhost", envKeys.get(0).getValue()); + assertEquals( + CanId.configKeyId(art.getId(), "env.DB_HOST"), envKeys.get(0).getId(), + "env dual-mint id must be disambiguated so it cannot collide with a top-level yaml key of the same name"); + } + + @Test + void extract_malformedYamlReturnsOkFalseAndDoesNotThrow() { + JArtifact art = artifact("application.yml", "yaml"); + String text = "server: [unterminated\nother: value\n"; + + ConfigKeys.Result result = assertDoesNotThrow(() -> ConfigKeys.extract(art, text, true)); + + assertFalse(result.ok); + assertTrue(result.keys.isEmpty()); + } + + // ---- xml ------------------------------------------------------------------------------ + + @Test + void extract_xmlDescriptorYieldsAnElementPath() { + JArtifact art = artifact("web.xml", "xml"); + String text = "8080"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + JConfigKey key = find(result.keys, "config.server.port").orElseThrow(); + assertEquals("8080", key.getValue()); + assertEquals("xml", key.getNamespace()); + } + + @Test + void extract_xmlAttributesFlattenAsPathAtAttrAndRepeatedSiblingsGetNumericSegments() { + JArtifact art = artifact("beans.xml", "xml"); + String text = "" + + "" + + "" + + ""; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals("prod", find(result.keys, "beans@env").orElseThrow().getValue()); + assertEquals( + "a", find(result.keys, "beans.bean.0@id").orElseThrow().getValue(), + "a single occurrence would keep a clean path, but two siblings must be indexed"); + assertEquals("b", find(result.keys, "beans.bean.1@id").orElseThrow().getValue()); + } + + @Test + void extract_malformedXmlReturnsOkFalseAndDoesNotThrow() { + JArtifact art = artifact("web.xml", "xml"); + String text = ""; + + ConfigKeys.Result result = assertDoesNotThrow(() -> ConfigKeys.extract(art, text, true)); + + assertFalse(result.ok); + assertTrue(result.keys.isEmpty()); + } + + // ---- dockerfile ------------------------------------------------------------------------- + + @Test + void extract_dockerfileEnvGoesToEnvNamespaceAndArgGoesToDockerfileNamespaceWithoutIdCollision() { + JArtifact art = artifact("Dockerfile", "dockerfile"); + String text = "ARG VERSION=1.0\nENV VERSION=$VERSION\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + List versionKeys = result.keys.stream() + .filter(k -> k.getKey().equals("VERSION")) + .collect(java.util.stream.Collectors.toList()); + assertEquals(2, versionKeys.size(), "same bare key name from both ENV and ARG"); + + JConfigKey envKey = versionKeys.stream().filter(k -> "env".equals(k.getNamespace())).findFirst().orElseThrow(); + JConfigKey argKey = + versionKeys.stream().filter(k -> "dockerfile".equals(k.getNamespace())).findFirst().orElseThrow(); + assertEquals(CanId.configKeyId(art.getId(), "VERSION"), envKey.getId()); + assertEquals( + CanId.configKeyId(art.getId(), "arg.VERSION"), argKey.getId(), + "ARG's id must be disambiguated with an arg. prefix so it cannot collide with ENV's plain id"); + assertEquals("$VERSION", envKey.getValue()); + assertEquals("1.0", argKey.getValue()); + } + + @Test + void extract_dockerfileArgWithNoDefaultYieldsNullValueNotEmptyString() { + JArtifact art = artifact("Dockerfile", "dockerfile"); + String text = "ARG BUILD_ID\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + JConfigKey key = find(result.keys, "BUILD_ID").orElseThrow(); + assertNull(key.getValue(), "an absent default is a distinct fact from an explicitly empty value"); + } + + @Test + void extract_dockerfileLegacySpaceFormEnvIsParsedVerbatim() { + JArtifact art = artifact("Dockerfile", "dockerfile"); + String text = "ENV NAME John Doe\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals("John Doe", find(result.keys, "NAME").orElseThrow().getValue()); + } + + @Test + void extract_dockerfileMultiKeyEnvLineYieldsBothKeys() { + JArtifact art = artifact("Dockerfile", "dockerfile"); + String text = "ENV FOO=1 BAR=2\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals("1", find(result.keys, "FOO").orElseThrow().getValue()); + assertEquals("2", find(result.keys, "BAR").orElseThrow().getValue()); + } + + // ---- env-family basenames (.env, .env.*) -------------------------------------------------- + + @Test + void extract_dotEnvFileYieldsEnvNamespaceRegardlessOfDeclaredFormat() { + // ArtifactDiscovery assigns .env files format="text" -- extract must dispatch on the + // basename, not the format, exactly like isEligible does. + JArtifact art = artifact(".env", "text"); + String text = "# a comment\nexport DB_HOST=localhost\nDB_PORT=5432\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + assertEquals("localhost", find(result.keys, "DB_HOST").orElseThrow().getValue()); + assertEquals("env", find(result.keys, "DB_HOST").orElseThrow().getNamespace()); + assertEquals("5432", find(result.keys, "DB_PORT").orElseThrow().getValue()); + } + + // ---- references ------------------------------------------------------------------------ + + @Test + void extract_bracedReferenceIsMaskedSoBareFormInsideItIsNotDoubleCounted() { + JArtifact art = artifact("application.properties", "properties"); + String text = "url=jdbc://${DB_HOST}/db?fallback=$DB_HOST\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + JConfigKey key = find(result.keys, "url").orElseThrow(); + assertEquals( + List.of("${DB_HOST}", "$DB_HOST"), key.getReferences(), + "both distinct sigil forms are kept, in order, with no spurious extra entry"); + } + + @Test + void extract_dottedBracedPlaceholderIsRecognized() { + // Spring/Maven's dominant form is a DOTTED property path, unlike a shell env var. + JArtifact art = artifact("application.properties", "properties"); + String text = "url=${spring.datasource.url}\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertEquals(List.of("${spring.datasource.url}"), find(result.keys, "url").orElseThrow().getReferences()); + } + + // ---- captureValue ------------------------------------------------------------------------ + + @Test + void extract_captureValueFalseKeepsKeysAndReferencesButNullsValue() { + JArtifact art = artifact("application.properties", "properties"); + String text = "url=${DB_HOST}\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, false); + + assertTrue(result.ok); + JConfigKey key = find(result.keys, "url").orElseThrow(); + assertNull(key.getValue()); + assertEquals("properties", key.getNamespace()); + assertEquals( + List.of("${DB_HOST}"), key.getReferences(), + "references are extracted from the raw value regardless of captureValue"); + } + + // ---- format with no flattener ------------------------------------------------------------ + + @Test + void extract_formatWithNoFlattenerReturnsEmptyAndOk() { + JArtifact art = artifact("data.json", "json"); + + ConfigKeys.Result result = ConfigKeys.extract(art, "{\"a\":1}", true); + + assertTrue(result.ok, "not applicable is not a failure"); + assertTrue(result.keys.isEmpty()); + } + + // ---- isEligible ------------------------------------------------------------------------ + + @Test + void isEligible_admitsExactlyPropertiesYamlXmlDockerfileAndEnvFamily() { + assertTrue(ConfigKeys.isEligible(artifact("application.properties", "properties"))); + assertTrue(ConfigKeys.isEligible(artifact("application.yml", "yaml"))); + assertTrue(ConfigKeys.isEligible(artifact("web.xml", "xml"))); + assertTrue(ConfigKeys.isEligible(artifact("Dockerfile", "dockerfile"))); + assertTrue(ConfigKeys.isEligible(artifact(".env", "text")), "env-family basename, regardless of format"); + assertTrue(ConfigKeys.isEligible(artifact(".env.local", "text"))); + + assertFalse(ConfigKeys.isEligible(artifact("build.gradle", "gradle")), "a build script is a program, not config"); + assertFalse(ConfigKeys.isEligible(artifact("data.json", "json"))); + assertFalse(ConfigKeys.isEligible(artifact("README.md", "text"))); + } + + @Test + void isEligible_binaryIsNeverEligibleEvenWithAnEnvFamilyName() { + // A rule-matched-but-undecodable file downgrades to format="binary" regardless of its + // basename -- the binary check must win even over an env-family name. + assertFalse(ConfigKeys.isEligible(artifact(".env", "binary"))); + } + + @Test + void isEligible_doesNotThrowWhenFormatIsNull() { + JArtifact art = new JArtifact(); + art.setPath("mystery-file"); + // format left null (never set) + + assertFalse(assertDoesNotThrow(() -> ConfigKeys.isEligible(art))); + } +} From 5950a3d6b6fc7b3bb5355d073a1394f9340c9471 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 01:29:52 -0400 Subject: [PATCH 10/19] fix(artifacts): close yaml config-key gaps from review round 1 - guard the yaml branch against a top-level scalar document (not null, not a Map, not a List) minting a spurious key="" entry - give the compose env-dual-mint a delimiter distinct from CanId.configKeyId's "@key/", so it can no longer collide with a plain yaml dotted path that legitimately starts with "env." - dedupe the ENV/ARG dockerfile scanners into one scanner plus two small per-directive tail parsers --- .../com/ibm/cldk/artifacts/ConfigKeys.java | 141 +++++++++++------- .../ibm/cldk/artifacts/ConfigKeysTest.java | 48 +++++- 2 files changed, 132 insertions(+), 57 deletions(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java b/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java index c7d98f66..5be8d620 100644 --- a/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java +++ b/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java @@ -51,9 +51,10 @@ * {@code ENV X=$X} is common, the {@code ARG} mint's id (not its {@code key} field) is * disambiguated with an {@code arg.} prefix so the two do not collide. A {@code yaml}-format * artifact additionally recognizes a compose {@code services..environment} map and - * dual-mints those into namespace {@code env} too, on the bare var name, with the same kind of - * collision guard (an {@code env.} id prefix, since a top-level yaml key could otherwise share a - * bare name with a recognized env var). + * dual-mints those into namespace {@code env} too, on the bare var name, with an analogous but + * structurally different collision guard: see {@link #ENV_DUAL_MINT_ID_DELIMITER} for why a plain + * text prefix (the dockerfile ARG approach) is not provably safe for yaml's unrestricted dotted + * paths the way it is for dockerfile's identifier-restricted keys. * *

{@code value} is populated only when {@code captureValue} is {@code true}; {@code key}, * {@code namespace}, {@code span}, and {@code references} are extracted unconditionally either @@ -140,16 +141,20 @@ public static Result extract(JArtifact artifact, String text, boolean captureVal artifact.getId(), "env", dockerfileEnvEntries(text), captureValue, true, null)); keys.addAll(buildKeys( artifact.getId(), "dockerfile", dockerfileArgEntries(text), captureValue, true, - k -> "arg." + k)); + k -> CanId.configKeyId(artifact.getId(), "arg." + k))); } else if ("properties".equals(artifact.getFormat())) { keys = buildKeys(artifact.getId(), "properties", parseProperties(text), captureValue, false, null); } else if ("yaml".equals(artifact.getFormat())) { Object data = new Yaml().load(text); - List flat = flatten(data == null ? new LinkedHashMap<>() : data, ""); + // Anything that isn't a Map or a List (null for an empty doc, but also a bare + // top-level scalar like "hello world" or "42") has nothing to flatten -- must not + // fall into flatten()'s leaf branch, which would mint a spurious key="" entry. + Object root = data instanceof Map || data instanceof List ? data : new LinkedHashMap<>(); + List flat = flatten(root, ""); keys = new ArrayList<>(buildKeys(artifact.getId(), "yaml", flat, captureValue, false, null)); keys.addAll(buildKeys( artifact.getId(), "env", recognizeComposeEnv(flat), captureValue, false, - k -> "env." + k)); + k -> artifact.getId() + ENV_DUAL_MINT_ID_DELIMITER + k)); } else if ("xml".equals(artifact.getFormat())) { keys = extractXml(artifact.getId(), text, captureValue); } else { @@ -206,6 +211,23 @@ private static List flatten(Object obj, String prefix) { // is deliberately not ported, the brief names compose only. private static final Pattern COMPOSE_ENV = Pattern.compile("^services\\.[^.]+\\.environment\\.(.+)$"); + /** + * Id delimiter for a compose env-dual-mint entry, used in place of {@code + * CanId.configKeyId}'s {@code "@key/"}. A plain yaml dotted path is an UNRESTRICTED string (any + * map key can be quoted to contain literally anything), so it can legitimately equal + * {@code "env."} -- e.g. a document with both a top-level {@code env:} block one level + * deep and a {@code services.x.environment} block recognizes the same bare name from two + * places. Prefixing "env." onto the bare key and still going through {@code configKeyId}'s + * {@code "@key/"} delimiter therefore cannot be proven collision-free: it works only because no + * test happened to construct that yaml shape, i.e. by luck, not by construction. Using a + * DIFFERENT fixed delimiter instead makes the two id families diverge at a fixed character + * position (right after {@code "@key"}, {@code ':'} here vs. plain {@code configKeyId}'s + * {@code '/'}) that no dotted-key content can ever reach, regardless of what the yaml + * document's keys contain -- an unconditional guarantee, the same rigor the dockerfile + * {@code arg.} prefix has (there, {@code ENV_KEY_NAME} forbids dots on both sides instead). + */ + private static final String ENV_DUAL_MINT_ID_DELIMITER = "@key:env/"; + private static List recognizeComposeEnv(List flat) { List out = new ArrayList<>(); for (Entry e : flat) { @@ -378,78 +400,86 @@ private static List splitWsRespectingQuotes(String s) { return tokens; } - private static List dockerfileEnvEntries(String text) { + /** + * Shared scaffolding for both {@code ENV} and {@code ARG} scanning: split into lines, skip + * blank/comment/non-matching ones, join backslash continuations, recover the directive's + * {@code rest} text and its {@code span}, and hand {@code rest} to {@code parseTail} for the + * part that actually differs between the two directives (multi-key-vs-legacy for {@code ENV}, + * {@code key[=default]} for {@code ARG}). {@code parseTail}'s returned entries carry no span of + * their own (irrelevant -- this method always attaches the one it already computed). + */ + private static List scanDockerfileDirective( + String text, Pattern directive, Function> parseTail) { List out = new ArrayList<>(); String[] lines = text.split("\n", -1); int i = 0; while (i < lines.length) { String stripped = lines[i].trim(); - if (stripped.isEmpty() || stripped.startsWith("#") || !DOCKER_ENV.matcher(stripped).matches()) { + if (stripped.isEmpty() || stripped.startsWith("#") || !directive.matcher(stripped).matches()) { i++; continue; } int startLineno = i + 1; Joined joined = joinContinuations(lines, i); i = joined.lastIndex; - // Re-run against the JOINED text (continuation lines add content after "ENV "): still - // guaranteed to match, since (.*) is greedy to end-of-string regardless of length. - Matcher dm = DOCKER_ENV.matcher(joined.text); + // Re-run against the JOINED text (continuation lines add content after the keyword): + // still guaranteed to match, since (.*) is greedy to end-of-string regardless of length. + Matcher dm = directive.matcher(joined.text); dm.matches(); String rest = dm.group(1).trim(); Span span = lineSpan(text, lines[startLineno - 1], startLineno); - String firstToken = rest.isEmpty() ? "" : rest.split("\\s+", 2)[0]; - if (firstToken.contains("=")) { - // multi-key form: ENV a=1 b=2 - for (String tok : splitWsRespectingQuotes(rest)) { - int eq = tok.indexOf('='); - if (eq >= 0) { - String key = tok.substring(0, eq); - if (ENV_KEY_NAME.matcher(key).matches()) { - out.add(new Entry(key, envValue(tok.substring(eq + 1)), span)); - } - } - } - } else { - // legacy single-key form: ENV KEY value -- value taken verbatim, no quote processing - String[] parts = rest.split("\\s+", 2); - if (parts.length == 2 && ENV_KEY_NAME.matcher(parts[0]).matches()) { - out.add(new Entry(parts[0], parts[1], span)); - } + for (Entry partial : parseTail.apply(rest)) { + out.add(new Entry(partial.key, partial.value, span)); } i++; } return out; } + private static List dockerfileEnvEntries(String text) { + return scanDockerfileDirective(text, DOCKER_ENV, ConfigKeys::parseEnvDirectiveTail); + } + private static List dockerfileArgEntries(String text) { + return scanDockerfileDirective(text, DOCKER_ARG, ConfigKeys::parseArgDirectiveTail); + } + + // rest = everything after "ENV " on the (possibly continuation-joined) logical line. + private static List parseEnvDirectiveTail(String rest) { List out = new ArrayList<>(); - String[] lines = text.split("\n", -1); - int i = 0; - while (i < lines.length) { - String stripped = lines[i].trim(); - if (stripped.isEmpty() || stripped.startsWith("#") || !DOCKER_ARG.matcher(stripped).matches()) { - i++; - continue; + String firstToken = rest.isEmpty() ? "" : rest.split("\\s+", 2)[0]; + if (firstToken.contains("=")) { + // multi-key form: ENV a=1 b=2 + for (String tok : splitWsRespectingQuotes(rest)) { + int eq = tok.indexOf('='); + if (eq >= 0) { + String key = tok.substring(0, eq); + if (ENV_KEY_NAME.matcher(key).matches()) { + out.add(new Entry(key, envValue(tok.substring(eq + 1)), null)); + } + } } - int startLineno = i + 1; - Joined joined = joinContinuations(lines, i); - i = joined.lastIndex; - // Same re-run-against-the-joined-text guarantee as dockerfileEnvEntries above. - Matcher dm = DOCKER_ARG.matcher(joined.text); - dm.matches(); - String rest = dm.group(1).trim(); - Span span = lineSpan(text, lines[startLineno - 1], startLineno); - int eq = rest.indexOf('='); - String key = (eq >= 0 ? rest.substring(0, eq) : rest).trim(); - if (ENV_KEY_NAME.matcher(key).matches()) { - // No "=default" means value=null (distinct from "" for an explicitly empty value). - out.add(new Entry(key, eq >= 0 ? envValue(rest.substring(eq + 1)) : null, span)); + } else { + // legacy single-key form: ENV KEY value -- value taken verbatim, no quote processing + String[] parts = rest.split("\\s+", 2); + if (parts.length == 2 && ENV_KEY_NAME.matcher(parts[0]).matches()) { + out.add(new Entry(parts[0], parts[1], null)); } - i++; } return out; } + // rest = everything after "ARG " on the (possibly continuation-joined) logical line. + private static List parseArgDirectiveTail(String rest) { + int eq = rest.indexOf('='); + String key = (eq >= 0 ? rest.substring(0, eq) : rest).trim(); + if (!ENV_KEY_NAME.matcher(key).matches()) { + return List.of(); + } + // No "=default" means value=null (distinct from "" for an explicitly empty value). + return List.of(new Entry(key, eq >= 0 ? envValue(rest.substring(eq + 1)) : null, null)); + } + // ---- env-family files (.env, .env.*): `KEY=value`, `#` comments, optional `export ` prefix, // quote stripping. Shared with dockerfile's ENV/ARG value handling via envValue. ------------- @@ -558,13 +588,14 @@ private static String stringify(Object value) { * into {@link JConfigKey} records for one namespace. {@code rawValue=true} (dockerfile) passes * the parsed value straight through instead of {@link #stringify}-ing it, so an ARG's absent * default surfaces as {@code value=null} rather than {@code stringify}'s {@code ""} for a - * modeled null. {@code idKey} remaps the dotted/bare key for ID CONSTRUCTION only -- the {@code - * key} FIELD always stays the bare key; see the class javadoc for why the dockerfile ARG and - * yaml env-dual-mint call sites need it. + * modeled null. {@code idOf} builds the FULL id from the bare/dotted key -- defaulting to + * {@code CanId.configKeyId(artifactId, key)} when {@code null} -- while the {@code key} FIELD + * always stays the bare key regardless; see the class javadoc for why the dockerfile ARG and + * yaml env-dual-mint call sites need a non-default one. */ private static List buildKeys( String artifactId, String namespace, List entries, boolean captureValue, - boolean rawValue, Function idKey) { + boolean rawValue, Function idOf) { Map coalesced = new LinkedHashMap<>(); for (Entry e : entries) { coalesced.put(e.key, e); @@ -573,7 +604,7 @@ private static List buildKeys( for (Entry e : coalesced.values()) { String textValue = rawValue ? (String) e.value : stringify(e.value); JConfigKey key = new JConfigKey(); - key.setId(CanId.configKeyId(artifactId, idKey != null ? idKey.apply(e.key) : e.key)); + key.setId(idOf != null ? idOf.apply(e.key) : CanId.configKeyId(artifactId, e.key)); key.setKey(e.key); key.setNamespace(namespace); key.setValue(captureValue ? textValue : null); diff --git a/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java b/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java index bce676aa..ca229d0b 100644 --- a/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java +++ b/src/test/java/com/ibm/cldk/artifacts/ConfigKeysTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -98,8 +99,51 @@ void extract_yamlComposeEnvironmentBlockDualMintsEnvNamespaceWithDisambiguatedId assertEquals(1, envKeys.size()); assertEquals("localhost", envKeys.get(0).getValue()); assertEquals( - CanId.configKeyId(art.getId(), "env.DB_HOST"), envKeys.get(0).getId(), - "env dual-mint id must be disambiguated so it cannot collide with a top-level yaml key of the same name"); + art.getId() + "@key:env/DB_HOST", envKeys.get(0).getId(), + "env dual-mint id must use a delimiter distinct from configKeyId's \"@key/\" so it can never " + + "collide with a plain yaml dotted path of the same text (see ENV_DUAL_MINT_ID_DELIMITER)"); + } + + @Test + void extract_yamlTopLevelScalarDocumentYieldsNoKeysNotAnEmptyKeyEntry() { + // new Yaml().load("hello world") returns the String "hello world" -- neither null, a Map, + // nor a List. Must be treated like an empty document (properties' own degenerate case + // already does this correctly), not flattened into a single entry with key="". + JArtifact art = artifact("weird.yml", "yaml"); + + ConfigKeys.Result result = ConfigKeys.extract(art, "hello world", true); + + assertTrue(result.ok); + assertTrue(result.keys.isEmpty(), "a top-level scalar has nothing to flatten, like an empty document"); + } + + @Test + void extract_yamlTopLevelEnvBlockDoesNotCollideWithComposeEnvDualMintId() { + // A plain yaml dotted path CAN legitimately be "env.DB_HOST" (a top-level "env:" block one + // level deep) -- the exact same string the compose dual-mint would otherwise construct for + // a bare "DB_HOST" var. Both must be minted, with different ids: no two JConfigKey records + // extracted from one artifact may share an id. + JArtifact art = artifact("docker-compose.yml", "yaml"); + String text = "env:\n DB_HOST: from-top-level\n" + + "services:\n web:\n environment:\n DB_HOST: from-compose\n"; + + ConfigKeys.Result result = ConfigKeys.extract(art, text, true); + + assertTrue(result.ok); + JConfigKey plainYamlKey = find(result.keys, "env.DB_HOST").orElseThrow(); + assertEquals("yaml", plainYamlKey.getNamespace()); + assertEquals("from-top-level", plainYamlKey.getValue()); + + JConfigKey dualMintKey = result.keys.stream() + .filter(k -> "env".equals(k.getNamespace()) && k.getKey().equals("DB_HOST")) + .findFirst().orElseThrow(); + assertEquals("from-compose", dualMintKey.getValue()); + + assertNotEquals( + plainYamlKey.getId(), dualMintKey.getId(), + "a top-level env.DB_HOST yaml path and a compose-recognized DB_HOST var must not collide"); + long distinctIds = result.keys.stream().map(JConfigKey::getId).distinct().count(); + assertEquals(result.keys.size(), distinctIds, "every key extracted from one artifact must have a unique id"); } @Test From f04e9babe5c587031f955dabeb749eaec9eb9149 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 01:42:40 -0400 Subject: [PATCH 11/19] fix(schema): move the yaml env-dual-mint id into CanId as a ninth constructor CanId.configKeyEnvDualMintId(artifactId, bareKey) now owns the "@key:env/" delimiter and the collision-freedom rationale, matching the eight other can:// id constructors already funneled through CanId. ConfigKeys.java:157 calls it instead of concatenating the string itself, alongside the existing configKeyId call for the dockerfile arg. case one call earlier. ArtifactModelTest gains a case next to configKeyIdNestsUnderItsArtifact asserting the new shape and that it cannot collide with configKeyId for the same key name. --- .../com/ibm/cldk/artifacts/ConfigKeys.java | 28 ++++--------------- src/main/java/com/ibm/cldk/schema/CanId.java | 20 +++++++++++++ .../ibm/cldk/schema/ArtifactModelTest.java | 18 ++++++++++++ 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java b/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java index 5be8d620..d79550f3 100644 --- a/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java +++ b/src/main/java/com/ibm/cldk/artifacts/ConfigKeys.java @@ -51,10 +51,11 @@ * {@code ENV X=$X} is common, the {@code ARG} mint's id (not its {@code key} field) is * disambiguated with an {@code arg.} prefix so the two do not collide. A {@code yaml}-format * artifact additionally recognizes a compose {@code services..environment} map and - * dual-mints those into namespace {@code env} too, on the bare var name, with an analogous but - * structurally different collision guard: see {@link #ENV_DUAL_MINT_ID_DELIMITER} for why a plain - * text prefix (the dockerfile ARG approach) is not provably safe for yaml's unrestricted dotted - * paths the way it is for dockerfile's identifier-restricted keys. + * dual-mints those into namespace {@code env} too, on the bare var name, via {@link + * CanId#configKeyEnvDualMintId} -- see its javadoc for why a plain text prefix funneled through + * {@link CanId#configKeyId} (the dockerfile ARG approach) is not provably safe for yaml's + * unrestricted dotted paths the way it is for dockerfile's identifier-restricted keys, and so gets + * its own {@code CanId} construction method rather than a prefix. * *

{@code value} is populated only when {@code captureValue} is {@code true}; {@code key}, * {@code namespace}, {@code span}, and {@code references} are extracted unconditionally either @@ -154,7 +155,7 @@ public static Result extract(JArtifact artifact, String text, boolean captureVal keys = new ArrayList<>(buildKeys(artifact.getId(), "yaml", flat, captureValue, false, null)); keys.addAll(buildKeys( artifact.getId(), "env", recognizeComposeEnv(flat), captureValue, false, - k -> artifact.getId() + ENV_DUAL_MINT_ID_DELIMITER + k)); + k -> CanId.configKeyEnvDualMintId(artifact.getId(), k))); } else if ("xml".equals(artifact.getFormat())) { keys = extractXml(artifact.getId(), text, captureValue); } else { @@ -211,23 +212,6 @@ private static List flatten(Object obj, String prefix) { // is deliberately not ported, the brief names compose only. private static final Pattern COMPOSE_ENV = Pattern.compile("^services\\.[^.]+\\.environment\\.(.+)$"); - /** - * Id delimiter for a compose env-dual-mint entry, used in place of {@code - * CanId.configKeyId}'s {@code "@key/"}. A plain yaml dotted path is an UNRESTRICTED string (any - * map key can be quoted to contain literally anything), so it can legitimately equal - * {@code "env."} -- e.g. a document with both a top-level {@code env:} block one level - * deep and a {@code services.x.environment} block recognizes the same bare name from two - * places. Prefixing "env." onto the bare key and still going through {@code configKeyId}'s - * {@code "@key/"} delimiter therefore cannot be proven collision-free: it works only because no - * test happened to construct that yaml shape, i.e. by luck, not by construction. Using a - * DIFFERENT fixed delimiter instead makes the two id families diverge at a fixed character - * position (right after {@code "@key"}, {@code ':'} here vs. plain {@code configKeyId}'s - * {@code '/'}) that no dotted-key content can ever reach, regardless of what the yaml - * document's keys contain -- an unconditional guarantee, the same rigor the dockerfile - * {@code arg.} prefix has (there, {@code ENV_KEY_NAME} forbids dots on both sides instead). - */ - private static final String ENV_DUAL_MINT_ID_DELIMITER = "@key:env/"; - private static List recognizeComposeEnv(List flat) { List out = new ArrayList<>(); for (Entry e : flat) { diff --git a/src/main/java/com/ibm/cldk/schema/CanId.java b/src/main/java/com/ibm/cldk/schema/CanId.java index ffc85ea2..2df4196d 100644 --- a/src/main/java/com/ibm/cldk/schema/CanId.java +++ b/src/main/java/com/ibm/cldk/schema/CanId.java @@ -64,6 +64,26 @@ public static String configKeyId(String artifactId, String dottedKey) { return artifactId + "@key/" + dottedKey; } + /** + * {@code @key:env/} — a compose-recognized environment variable + * dual-minted into namespace {@code env} from a yaml artifact's {@code + * services..environment} block (see {@code ConfigKeys}). Deliberately a DIFFERENT + * delimiter from {@link #configKeyId}'s {@code "@key/"}, not a prefixed dotted key routed + * through it: a plain yaml dotted path is an unrestricted string (any map key can be quoted to + * contain literally anything), so it can legitimately equal {@code "env."} itself — e.g. + * a top-level {@code env:} block one level deep — which a text prefix inside {@link + * #configKeyId}'s argument could not be proven never to collide with. + * + *

The two id forms share the identical {@code @key} prefix and diverge at a + * fixed character position right after it ({@code :} here vs. {@code configKeyId}'s {@code /}), + * before either side has consumed any dotted-key or variable-name content — so the two can + * never collide for any {@code dottedKey}/{@code bareKey} whatsoever, not merely for whatever + * shape a test happens to construct. + */ + public static String configKeyEnvDualMintId(String artifactId, String bareKey) { + return artifactId + "@key:env/" + bareKey; + } + /** {@code pkg:maven//} — a two-segment Package URL for Maven coordinates. */ public static String purlMaven(String group, String name) { return "pkg:maven/" + group + "/" + name; diff --git a/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java b/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java index 5d088c47..a8f654bb 100644 --- a/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java +++ b/src/test/java/com/ibm/cldk/schema/ArtifactModelTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; @@ -23,6 +24,23 @@ void configKeyIdNestsUnderItsArtifact() { assertEquals(art + "@key/server.port", CanId.configKeyId(art, "server.port")); } + @Test + void configKeyEnvDualMintIdNeverCollidesWithConfigKeyId() { + String art = CanId.artifactId("myapp", "docker-compose.yml"); + // A real yaml document can flatten a top-level "env:" block one level deep into exactly + // the dotted key "env.DB_HOST" -- the same text a naive "env." prefix through configKeyId + // would build for a compose-recognized bare "DB_HOST" var. configKeyEnvDualMintId must use + // a different delimiter so the two can never collide, for any key/var name whatsoever. + String plainYamlId = CanId.configKeyId(art, "env.DB_HOST"); + String dualMintId = CanId.configKeyEnvDualMintId(art, "DB_HOST"); + + assertEquals(art + "@key/env.DB_HOST", plainYamlId); + assertEquals(art + "@key:env/DB_HOST", dualMintId); + assertNotEquals(plainYamlId, dualMintId, + "the two share \"@key\" and must diverge right after it (':' vs '/'), before " + + "either consumes any key content, so no dotted key or bare variable name can equalize them"); + } + @Test void purlIsTwoSegmentForMaven() { assertEquals("pkg:maven/org.apache.commons/commons-lang3", From 304d4a034eca9924ec7948c3e073c933c1d93628 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 02:14:43 -0400 Subject: [PATCH 12/19] feat(artifacts): emit the repository-artifact layer at every analysis level --- src/main/java/com/ibm/cldk/CodeAnalyzer.java | 57 +++++- .../java/com/ibm/cldk/schema/V2Emitter.java | 28 +++ .../com/ibm/cldk/CodeAnalyzerV2CliTest.java | 185 ++++++++++++++++++ .../resources/schema/analysis.v2.schema.json | 68 ++++++- 4 files changed, 335 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/ibm/cldk/CodeAnalyzer.java b/src/main/java/com/ibm/cldk/CodeAnalyzer.java index 080201cb..60722375 100644 --- a/src/main/java/com/ibm/cldk/CodeAnalyzer.java +++ b/src/main/java/com/ibm/cldk/CodeAnalyzer.java @@ -21,10 +21,15 @@ import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParser; +import com.ibm.cldk.artifacts.ArtifactDiscovery; +import com.ibm.cldk.artifacts.ConfigKeys; +import com.ibm.cldk.artifacts.DependencyView; import com.ibm.cldk.entities.JavaCompilationUnit; import com.ibm.cldk.neo4j.BoltConfig; import com.ibm.cldk.neo4j.Neo4jEmitter; import com.ibm.cldk.schema.Analysis; +import com.ibm.cldk.schema.JArtifact; +import com.ibm.cldk.schema.JDependency; import com.ibm.cldk.schema.JModule; import com.ibm.cldk.schema.V2Emitter; import com.ibm.cldk.schema.V2Json; @@ -171,6 +176,20 @@ public class CodeAnalyzer implements Runnable { "--graph-field-depth" }, description = "DDG access-path bound k at --analysis-level 3 (default 3).") private int graphFieldDepth = 3; + // fallbackValue = "true" works around a picocli 4.1.0 bug (the version this project is pinned + // to): without it, a bare --artifact-text/--no-artifact-text (no explicit =value) resolves to + // the opposite of what negatable=true implies. Verified empirically against the resolved + // picocli-4.1.0.jar; --artifact-text=true/false and --no-artifact-text=true/false are unaffected + // either way. + @Option(names = { "--artifact-text" }, negatable = true, fallbackValue = "true", + description = "Capture non-source file text into artifact nodes (default: true).") + public static boolean artifactText = true; + + @Option(names = { "--artifact-text-max-bytes" }, + description = "Byte cap on captured artifact text (default: 262144). " + + "Dependency manifests are exempt — they are always captured whole.") + public static int artifactTextMaxBytes = 262144; + /** Handle used to report flag-validation errors as clean, non-zero picocli failures. */ @Spec private CommandSpec spec; @@ -360,6 +379,16 @@ private boolean isV2Schema() { return false; } + /** + * An artifact's full on-disk text, for config-key extraction. Never {@link JArtifact#getSource()}, + * which {@code --artifact-text}/{@code --artifact-text-max-bytes} may have emptied or truncated — + * delegates to {@link DependencyView#readFromDisk} rather than re-reading the file itself, so that + * from-disk logic exists exactly once in this codebase. + */ + private static String readFully(JArtifact artifact) { + return DependencyView.readFromDisk(Paths.get(input), artifact.getPath()); + } + /** * Emit the canonical schema v2 payload. Levels 1 (containment tree), 2 (the {@code call_graph} * overlay), 3 (the intraprocedural {@code cfg}/{@code cdg}/{@code ddg} overlays) and 4 (the @@ -505,6 +534,28 @@ private void analyzeV2() throws Exception { L1Cache.save(cache, application, version, modules); } + // The repository-artifact layer (build manifests, config files, declared dependencies) sits + // beside the call-graph/SDG assembly below because both are application-scope data built once + // -- but unlike them it is L1 data and runs at EVERY analysis level, so it is computed here, + // ahead of the level gate, rather than inside either branch of it. + Map artifacts = + ArtifactDiscovery.discover(Paths.get(input), application, artifactText, artifactTextMaxBytes); + List dependencies = DependencyView.build(Paths.get(input), artifacts); + for (JArtifact a : artifacts.values()) { + if (ConfigKeys.isEligible(a)) { + // Re-read from disk: `source` may be truncated or suppressed, and extraction + // must not silently degrade with a capture flag. + ConfigKeys.Result r = ConfigKeys.extract(a, readFully(a), artifactText); + a.setConfigKeys(r.keys); + // A pre-existing "partial" from the dependency pass is never overwritten. + if (!r.ok) { + a.setExtraction("partial"); + } else if ("none".equals(a.getExtraction())) { + a.setExtraction("full"); + } + } + } + // maxLevel reports the requested level: the L1-L3 passes above always run to that level (or // degrade a specific overlay with a warning), and the L4 vertices/param edges below are // engine-free, so they run whenever analysisLevel >= 4 regardless of the WALA build's fate. @@ -521,9 +572,11 @@ private void analyzeV2() throws Exception { } analysis = V2Emitter.emit(application, analysisLevel, modules, version, l2.callGraph(), l2.externalSymbols(), - sdg == null ? null : sdg.paramIn, sdg == null ? null : sdg.paramOut); + sdg == null ? null : sdg.paramIn, sdg == null ? null : sdg.paramOut, + artifacts, dependencies); } else { - analysis = V2Emitter.emit(application, analysisLevel, modules, version); + analysis = V2Emitter.emit(application, analysisLevel, modules, version, + null, null, null, null, artifacts, dependencies); } if ("neo4j".equalsIgnoreCase(emit)) { diff --git a/src/main/java/com/ibm/cldk/schema/V2Emitter.java b/src/main/java/com/ibm/cldk/schema/V2Emitter.java index dbda9f53..34401be1 100644 --- a/src/main/java/com/ibm/cldk/schema/V2Emitter.java +++ b/src/main/java/com/ibm/cldk/schema/V2Emitter.java @@ -50,6 +50,28 @@ public static Analysis emit( Map externalSymbols, List paramIn, List paramOut) { + return emit(appName, maxLevel, modules, analyzerVersion, callGraph, externalSymbols, + paramIn, paramOut, null, null); + } + + /** + * As above, additionally attaching the repository-artifact layer: build manifests, config files, + * and declared dependencies. {@code artifacts} and {@code dependencies} are set only when + * non-{@code null} and non-empty, the same "absence means no fact" rule every other + * application-scope overlay here follows. Unlike the others, this layer is L1 data — a caller + * passes it at every analysis level, not only when a level-gated overlay is available. + */ + public static Analysis emit( + String appName, + int maxLevel, + Map modules, + String analyzerVersion, + List callGraph, + Map externalSymbols, + List paramIn, + List paramOut, + Map artifacts, + List dependencies) { JApplication application = new JApplication(); application.setId(CanId.applicationId(appName)); @@ -73,6 +95,12 @@ public static Analysis emit( if (paramOut != null && !paramOut.isEmpty()) { application.setParamOut(paramOut); } + if (artifacts != null && !artifacts.isEmpty()) { + application.setArtifacts(artifacts); + } + if (dependencies != null && !dependencies.isEmpty()) { + application.setDependencies(dependencies); + } Analysis analysis = new Analysis(); analysis.setSchemaVersion("2.0.0"); diff --git a/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java b/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java index 7c960caf..e826691b 100644 --- a/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java +++ b/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java @@ -3,17 +3,28 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParser; +import com.networknt.schema.JsonSchema; +import com.networknt.schema.JsonSchemaFactory; +import com.networknt.schema.SpecVersion; +import com.networknt.schema.ValidationMessage; import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import javax.tools.ToolProvider; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -63,6 +74,8 @@ void resetStaticOptions() throws Exception { set("targetFiles", null); set("sourceAnalysis", null); CodeAnalyzer.projectRootPom = null; + CodeAnalyzer.artifactText = true; + CodeAnalyzer.artifactTextMaxBytes = 262144; } private static void set(String field, Object value) throws Exception { @@ -476,4 +489,176 @@ void unknownPrecisionFailsLoudlyUnderEmitNeo4jToo(@TempDir Path tmp) throws IOEx "unrecognized flag values exit non-zero (CLI contract), --emit neo4j included"); } + // ---- repository-artifact layer ------------------------------------------------------------ + + /** + * A project with a {@code pom.xml} (one declared dependency), an {@code application.properties} + * config file, and the usual {@code Widget.java} -- exercises all three repository-artifact + * layer sections (inventory, dependencies, config keys) at once. {@code junit:junit} is the + * declared dependency because it is already present in any local Maven repository that has ever + * built a Java project, so resolving it during the library-download step is instant and offline. + */ + private static Path artifactProject(Path root) throws IOException { + project(root); + Files.writeString(root.resolve("pom.xml"), + "\n" + + " 4.0.0\n" + + " com.example\n" + + " widgets\n" + + " 1.0\n" + + " \n" + + " \n" + + " junit\n" + + " junit\n" + + " 4.13.2\n" + + " \n" + + " \n" + + "\n", + StandardCharsets.UTF_8); + Files.writeString(root.resolve("application.properties"), "server.port=8080\n", StandardCharsets.UTF_8); + return root; + } + + private static JsonObject application(Path outputDir) throws IOException { + JsonObject root = JsonParser.parseString(Files.readString(outputDir.resolve("analysis.json"))) + .getAsJsonObject(); + return root.getAsJsonObject("application"); + } + + /** + * Validates a written {@code analysis.json} against the canonical (strict, additionalProperties: + * false) v2 schema -- so a key this task adds to the emitted payload must also be reflected in + * the schema, and an accidentally misnamed one fails here rather than reaching consumers. + */ + private static void assertConformsToV2Schema(Path outputDir) throws IOException { + JsonNode payload = new ObjectMapper().readTree(Files.readString(outputDir.resolve("analysis.json"))); + try (InputStream schemaIn = + CodeAnalyzerV2CliTest.class.getResourceAsStream("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/schema/analysis.v2.schema.json")) { + assertNotNull(schemaIn, "the canonical v2 schema must be on the test classpath"); + JsonSchema schema = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012).getSchema(schemaIn); + Set problems = schema.validate(payload); + assertTrue(problems.isEmpty(), + "output must validate against the canonical v2 schema, but got:\n " + + problems.stream().map(ValidationMessage::getMessage) + .collect(Collectors.joining("\n "))); + } + } + + @Test + void v2EmitsArtifactsAndDependenciesAtLevelOne(@TempDir Path tmp) throws IOException { + Path in = artifactProject(tmp.resolve("app")); + Path out = tmp.resolve("out"); + assertEquals(0, run("-i", in.toString(), "-o", out.toString(), "-a", "1", "--app-name", "widgets")); + + JsonObject app = application(out); + assertTrue(app.has("artifacts"), "the repository-artifact inventory must be attached"); + JsonObject artifacts = app.getAsJsonObject("artifacts"); + assertTrue(artifacts.has("pom.xml"), "pom.xml is a dependency-manifest artifact: " + artifacts.keySet()); + assertTrue(artifacts.has("application.properties"), + "and application.properties is a config artifact: " + artifacts.keySet()); + + JsonObject pom = artifacts.getAsJsonObject("pom.xml"); + assertEquals("xml", pom.get("format").getAsString()); + assertEquals("full", pom.get("extraction").getAsString()); + + JsonObject props = artifacts.getAsJsonObject("application.properties"); + assertEquals("properties", props.get("format").getAsString()); + JsonArray propsKeys = props.getAsJsonArray("config_keys"); + assertEquals(1, propsKeys.size()); + JsonObject serverPort = propsKeys.get(0).getAsJsonObject(); + assertEquals("server.port", serverPort.get("key").getAsString()); + assertEquals("8080", serverPort.get("value").getAsString()); + + assertTrue(app.has("dependencies"), "the pom.xml's declared dependency must be attached"); + JsonArray deps = app.getAsJsonArray("dependencies"); + boolean sawJunit = false; + for (JsonElement e : deps) { + JsonObject d = e.getAsJsonObject(); + if ("junit".equals(d.get("group").getAsString()) && "junit".equals(d.get("name").getAsString())) { + sawJunit = true; + } + } + assertTrue(sawJunit, "expected the declared junit dependency, got: " + deps); + + assertConformsToV2Schema(out); + } + + @Test + void v2ArtifactLayerIsByteIdenticalAcrossAnalysisLevels(@TempDir Path tmp) throws IOException { + // This layer is L1 data assembled once from the filesystem, independent of --analysis-level -- + // python's own gate, and the property most likely to break under careless wiring. + Path in = artifactProject(tmp.resolve("app")); + Path outLevel1 = tmp.resolve("out1"); + Path outLevel4 = tmp.resolve("out4"); + + assertEquals(0, run("-i", in.toString(), "-o", outLevel1.toString(), "-a", "1", "--app-name", "widgets")); + assertEquals(0, run("-i", in.toString(), "-o", outLevel4.toString(), + "-a", "4", "--no-build", "--app-name", "widgets")); + + JsonObject app1 = application(outLevel1); + JsonObject app4 = application(outLevel4); + + assertTrue(app1.has("artifacts") && app1.has("dependencies"), "precondition: level 1 carries the layer"); + assertTrue(app4.has("artifacts") && app4.has("dependencies"), "precondition: level 4 carries it too"); + + assertEquals(app1.get("artifacts").toString(), app4.get("artifacts").toString(), + "the artifact inventory (including nested config_keys) must be byte-identical across levels"); + assertEquals(app1.get("dependencies").toString(), app4.get("dependencies").toString(), + "declared dependencies must be byte-identical across levels"); + } + + @Test + void noArtifactTextEmptiesSourceAndValueButKeepsInventory(@TempDir Path tmp) throws IOException { + Path in = artifactProject(tmp.resolve("app")); + Path outWith = tmp.resolve("out-with"); + Path outWithout = tmp.resolve("out-without"); + + assertEquals(0, run("-i", in.toString(), "-o", outWith.toString(), "--app-name", "widgets")); + assertEquals(0, run("-i", in.toString(), "-o", outWithout.toString(), + "--app-name", "widgets", "--no-artifact-text")); + + JsonObject artifactsWith = application(outWith).getAsJsonObject("artifacts"); + JsonObject artifactsWithout = application(outWithout).getAsJsonObject("artifacts"); + assertEquals(artifactsWith.keySet(), artifactsWithout.keySet(), "the inventory itself must not change"); + + boolean sawCapturedSource = false; + boolean sawCapturedValue = false; + for (String path : artifactsWith.keySet()) { + JsonObject with = artifactsWith.getAsJsonObject(path); + JsonObject without = artifactsWithout.getAsJsonObject(path); + + assertEquals("", without.get("source").getAsString(), "--no-artifact-text must empty source: " + path); + sawCapturedSource |= !with.get("source").getAsString().isEmpty(); + + assertEquals(with.get("sha256"), without.get("sha256"), "hashing is unaffected by the capture flag"); + assertEquals(with.get("size_bytes"), without.get("size_bytes")); + assertEquals(with.get("extraction"), without.get("extraction"), + "extraction must not silently degrade under the capture flag: " + path); + + JsonArray keysWith = with.getAsJsonArray("config_keys"); + JsonArray keysWithout = without.getAsJsonArray("config_keys"); + assertEquals(keysWith.size(), keysWithout.size(), "the keys themselves must survive: " + path); + for (int i = 0; i < keysWith.size(); i++) { + JsonObject kw = keysWith.get(i).getAsJsonObject(); + JsonObject kwo = keysWithout.get(i).getAsJsonObject(); + assertEquals(kw.get("key"), kwo.get("key")); + assertEquals(kw.get("namespace"), kwo.get("namespace")); + assertFalse(kwo.has("value"), "--no-artifact-text must null out value: " + path); + sawCapturedValue |= kw.has("value"); + } + } + assertTrue(sawCapturedSource, "precondition: the default run must actually capture some source text"); + assertTrue(sawCapturedValue, "precondition: the default run must actually capture some config value"); + } + + @Test + void v2OmitsArtifactsAndDependenciesWhenNoNonSourceFilesExist(@TempDir Path tmp) throws IOException { + Path in = project(tmp.resolve("app")); + Path out = tmp.resolve("out"); + assertEquals(0, run("-i", in.toString(), "-o", out.toString())); + JsonObject app = application(out); + assertFalse(app.has("artifacts"), "no non-source files means the artifact layer has nothing to say"); + assertFalse(app.has("dependencies"), "no manifests means no declared dependencies"); + } + } diff --git a/src/test/resources/schema/analysis.v2.schema.json b/src/test/resources/schema/analysis.v2.schema.json index 1d3a4ee8..6fc50131 100644 --- a/src/test/resources/schema/analysis.v2.schema.json +++ b/src/test/resources/schema/analysis.v2.schema.json @@ -119,7 +119,16 @@ "additionalProperties": { "$ref": "#/$defs/externalSymbol" } }, "param_in": { "type": "array" }, - "param_out": { "type": "array" } + "param_out": { "type": "array" }, + "artifacts": { + "type": "object", + "propertyNames": { + "pattern": "^(?!/)(?!.*\\.\\.).*$", + "description": "Keys are repo-relative paths: never absolute, never escaping the root." + }, + "additionalProperties": { "$ref": "#/$defs/artifact" } + }, + "dependencies": { "type": "array", "items": { "$ref": "#/$defs/dependency" } } } }, @@ -210,6 +219,63 @@ } }, + "artifactCanId": { + "type": "string", + "pattern": "^can://artifact/", + "description": "Id of a repository-artifact node: a language-neutral scheme, distinct from can://java/." + }, + + "artifact": { + "type": "object", + "additionalProperties": false, + "required": ["id", "kind", "path", "format", "roles", "size_bytes", "sha256", "source", "extraction", "config_keys"], + "properties": { + "id": { "$ref": "#/$defs/artifactCanId" }, + "kind": { "const": "artifact" }, + "path": { "type": "string", "minLength": 1 }, + "format": { "type": "string" }, + "roles": { "$ref": "#/$defs/stringList" }, + "size_bytes": { "type": "integer", "minimum": 0 }, + "sha256": { "type": "string", "pattern": "^[0-9a-f]{64}$" }, + "source": { "type": "string" }, + "text_truncated": { "type": "boolean" }, + "extraction": { "enum": ["none", "partial", "full"] }, + "config_keys": { "type": "array", "items": { "$ref": "#/$defs/configKey" } } + } + }, + + "dependency": { + "type": "object", + "additionalProperties": false, + "required": ["group", "name", "ecosystem", "spec", "kind", "extras", "declared_in", "direct", "prov"], + "properties": { + "group": { "type": "string" }, + "name": { "type": "string", "minLength": 1 }, + "ecosystem": { "type": "string" }, + "spec": { "type": "string" }, + "kind": { "type": "string" }, + "extras": { "$ref": "#/$defs/stringList" }, + "declared_in": { "type": "string" }, + "direct": { "type": "boolean" }, + "locked_version": { "type": "string" }, + "prov": { "$ref": "#/$defs/stringList" } + } + }, + + "configKey": { + "type": "object", + "additionalProperties": false, + "required": ["id", "key", "namespace", "references"], + "properties": { + "id": { "$ref": "#/$defs/artifactCanId" }, + "key": { "type": "string" }, + "namespace": { "type": "string" }, + "value": { "type": "string" }, + "span": { "$ref": "#/$defs/span" }, + "references": { "$ref": "#/$defs/stringList" } + } + }, + "module": { "type": "object", "additionalProperties": false, From 26150a502b87923c593abc23119fc84464936193 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 02:59:31 -0400 Subject: [PATCH 13/19] feat(artifacts): project Artifact/Package/ConfigKey; graph contract 2.2.0 Fills in the neutral Artifact/Package labels V2SchemaCatalog already reserved, adds ConfigKey, and emits HAS_ARTIFACT/DEFINES_CONFIG/ DECLARES_DEPENDENCY/LOCKS. This is the Neo4j half of #197 that was deferred on the premise that the Java projector ran off a legacy model and could not carry a v2 addition -- that stopped being true once the v2 graph projection shipped, so the deferral is withdrawn. Package nodes are graph-only (minted from CanId.purlMaven); analysis.json continues to carry only the bare group/name, matching python. Nodes and containment edges are un-prefixed -- cross-language merge targets, same reasoning python's schema.py already states at its own declaration. DECLARES_DEPENDENCY carries a `_k`=kind MERGE discriminant via the L4 overlay's keyedEdge machinery, since one manifest may declare a package under two kinds. LOCKS fans out to every lock artifact present, since lock pins are merged upstream with no per-lock attribution -- a known limitation carried over from codeanalyzer-python as-is. codeanalyzer-python deliberately did not bump its own schema version for this layer ("no consumers yet"), so a consumer cannot detect the layer's presence from python's version alone. Java bumps to 2.2.0 anyway, the better behaviour and a deliberate divergence from python here. --- schema.neo4j.json | 97 ++++++++++++++- .../com/ibm/cldk/neo4j/V2GraphProjector.java | 109 +++++++++++++++- .../com/ibm/cldk/neo4j/V2SchemaCatalog.java | 47 ++++++- .../com/ibm/cldk/CodeAnalyzerV2CliTest.java | 4 +- .../neo4j/V2Neo4jSchemaConformanceTest.java | 116 +++++++++++++++++- 5 files changed, 364 insertions(+), 9 deletions(-) diff --git a/schema.neo4j.json b/schema.neo4j.json index 236d7f25..f6a5f630 100644 --- a/schema.neo4j.json +++ b/schema.neo4j.json @@ -1,5 +1,5 @@ { - "schema_version": "2.1.0", + "schema_version": "2.2.0", "generator": "codeanalyzer-java", "marker_labels": [ "JEntrypoint" @@ -177,6 +177,47 @@ "properties": { "name": "string" } + }, + { + "label": "Artifact", + "merge_label": "Artifact", + "key": "id", + "properties": { + "id": "string", + "path": "string", + "format": "string", + "roles": "string[]", + "size_bytes": "integer", + "sha256": "string", + "source": "string", + "text_truncated": "boolean", + "extraction": "string" + } + }, + { + "label": "Package", + "merge_label": "Package", + "key": "id", + "properties": { + "id": "string", + "ecosystem": "string", + "group": "string", + "name": "string" + } + }, + { + "label": "ConfigKey", + "merge_label": "ConfigKey", + "key": "id", + "properties": { + "id": "string", + "key": "string", + "namespace": "string", + "value": "string", + "references": "string[]", + "start_line": "integer", + "end_line": "integer" + } } ], "relationship_types": [ @@ -407,6 +448,55 @@ "JBodyNode" ], "properties": {} + }, + { + "type": "HAS_ARTIFACT", + "from": [ + "JApplication" + ], + "to": [ + "Artifact" + ], + "properties": {} + }, + { + "type": "DEFINES_CONFIG", + "from": [ + "Artifact" + ], + "to": [ + "ConfigKey" + ], + "properties": {} + }, + { + "type": "DECLARES_DEPENDENCY", + "from": [ + "Artifact" + ], + "to": [ + "Package" + ], + "properties": { + "spec": "string", + "kind": "string", + "extras": "string[]", + "prov": "string[]", + "direct": "boolean", + "_k": "string" + } + }, + { + "type": "LOCKS", + "from": [ + "Artifact" + ], + "to": [ + "Package" + ], + "properties": { + "version": "string" + } } ], "constraints": [ @@ -419,7 +509,10 @@ "CREATE CONSTRAINT jrecordcomponent_id IF NOT EXISTS FOR (x:JRecordComponent) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT jbodynode_id IF NOT EXISTS FOR (x:JBodyNode) REQUIRE x.id IS UNIQUE", "CREATE CONSTRAINT jpackage_name IF NOT EXISTS FOR (x:JPackage) REQUIRE x.name IS UNIQUE", - "CREATE CONSTRAINT jannotation_name IF NOT EXISTS FOR (x:JAnnotation) REQUIRE x.name IS UNIQUE" + "CREATE CONSTRAINT jannotation_name IF NOT EXISTS FOR (x:JAnnotation) REQUIRE x.name IS UNIQUE", + "CREATE CONSTRAINT artifact_id IF NOT EXISTS FOR (x:Artifact) REQUIRE x.id IS UNIQUE", + "CREATE CONSTRAINT package_id IF NOT EXISTS FOR (x:Package) REQUIRE x.id IS UNIQUE", + "CREATE CONSTRAINT configkey_id IF NOT EXISTS FOR (x:ConfigKey) REQUIRE x.id IS UNIQUE" ], "indexes": [ "CREATE INDEX j_callable_name IF NOT EXISTS FOR (c:JCallable) ON (c.name)", diff --git a/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java b/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java index f0696928..09c13fda 100644 --- a/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java +++ b/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java @@ -14,14 +14,19 @@ import com.ibm.cldk.neo4j.GraphRows.NodeRef; import com.ibm.cldk.schema.Analysis; +import com.ibm.cldk.schema.CanId; +import com.ibm.cldk.schema.JApplication; +import com.ibm.cldk.schema.JArtifact; import com.ibm.cldk.schema.JBodyNode; import com.ibm.cldk.schema.JCallEdge; import com.ibm.cldk.schema.JCallable; import com.ibm.cldk.schema.JCdgEdge; import com.ibm.cldk.schema.JCfgEdge; import com.ibm.cldk.schema.JComment; +import com.ibm.cldk.schema.JConfigKey; import com.ibm.cldk.schema.JDdgEdge; import com.ibm.cldk.schema.JDecorator; +import com.ibm.cldk.schema.JDependency; import com.ibm.cldk.schema.JEnumConstant; import com.ibm.cldk.schema.JExternalSymbol; import com.ibm.cldk.schema.JField; @@ -42,7 +47,7 @@ /** * The schema v2 → Neo4j projection: a pure {@code (Analysis, appName) → GraphRows} function, no - * I/O, no driver. The vocabulary is {@link V2SchemaCatalog} (graph contract 2.1.0), mirroring + * I/O, no driver. The vocabulary is {@link V2SchemaCatalog} (graph contract 2.2.0), mirroring * codeanalyzer-python's projection: call sites are {@code :JBodyNode} rows (no call-site nodes), * parameters flatten to {@code parameters_json}, javadoc collapses to {@code docstring}, and the * L3 {@code cfg}/{@code cdg}/{@code ddg} and L4 {@code param_in}/{@code param_out}/{@code summary} @@ -143,6 +148,8 @@ public static GraphRows project(Analysis analysis, String appName) { } } + projectArtifacts(b, analysis.getApplication(), app); + return b.finish(); } @@ -389,6 +396,106 @@ private static String globalOrdinal(String callableId, String localKey) { return localKey.startsWith("@") ? callableId + localKey : callableId + "@" + localKey; } + // ------------------------------------------------------------------------------------------ + // Repository-artifact layer: build manifests, config files, declared dependencies. + // ------------------------------------------------------------------------------------------ + + // Mirrors DependencyView.LOCK_BASENAMES (kept duplicated locally rather than exposing a new + // cross-package constant for one entry -- codeanalyzer-python accepts the identical tradeoff + // for its own two independent lock-basename constants). + private static final String LOCK_BASENAME = "gradle.lockfile"; + + /** + * Neutral {@code Artifact}/{@code Package}/{@code ConfigKey} subgraph -- no {@code J}/{@code J_} + * prefix (see {@link V2SchemaCatalog}'s declaration comment: these are cross-language merge + * targets, unlike everything else this class projects). L1 data, present at every analysis + * level regardless of {@code -a} (mirrors {@code analysis.json}: {@link JApplication#getArtifacts()} + * / {@link JApplication#getDependencies()} are populated ahead of the level gate). + */ + private static void projectArtifacts(RowBuilder b, JApplication application, NodeRef app) { + Map artifacts = application.getArtifacts(); + List lockRefs = new ArrayList<>(); + if (artifacts != null) { + for (Map.Entry e : artifacts.entrySet()) { + JArtifact art = e.getValue(); + Map ap = RowBuilder.props(); + ap.put("id", art.getId()); + ap.put("path", art.getPath()); + ap.put("format", art.getFormat()); + ap.put("roles", art.getRoles()); + ap.put("size_bytes", art.getSizeBytes()); + ap.put("sha256", art.getSha256()); + ap.put("source", art.getSource()); + if (art.isTextTruncated()) { + ap.put("text_truncated", true); + } + ap.put("extraction", art.getExtraction()); + NodeRef artRef = b.node(Arrays.asList("Artifact"), "id", art.getId(), RowBuilder.prune(ap)); + b.edge("HAS_ARTIFACT", app, artRef); + + for (JConfigKey ck : art.getConfigKeys()) { + Map cp = RowBuilder.props(); + cp.put("id", ck.getId()); + cp.put("key", ck.getKey()); + cp.put("namespace", ck.getNamespace()); + cp.put("value", ck.getValue()); + cp.put("references", ck.getReferences()); + putLines(cp, ck.getSpan()); + NodeRef ckRef = b.node(Arrays.asList("ConfigKey"), "id", ck.getId(), RowBuilder.prune(cp)); + b.edge("DEFINES_CONFIG", artRef, ckRef); + } + + if (isLockArtifact(e.getKey())) { + lockRefs.add(artRef); + } + } + } + + List dependencies = application.getDependencies(); + if (dependencies != null) { + for (JDependency dep : dependencies) { + String pkgId = CanId.purlMaven(dep.getGroup(), dep.getName()); + Map pp = RowBuilder.props(); + pp.put("id", pkgId); + pp.put("ecosystem", dep.getEcosystem()); + pp.put("group", dep.getGroup()); + pp.put("name", dep.getName()); + NodeRef pkgRef = b.node(Arrays.asList("Package"), "id", pkgId, RowBuilder.prune(pp)); + + // `_k` (merges per `kind`): the same manifest may declare one package twice under + // different kinds -- same endpoint pair, so a plain MERGE would collapse the two + // declarations into one row. + Map dp = RowBuilder.props(); + dp.put("spec", dep.getSpec()); + dp.put("kind", dep.getKind()); + dp.put("extras", dep.getExtras()); + dp.put("prov", dep.getProv()); + dp.put("direct", dep.isDirect()); + b.keyedEdge("DECLARES_DEPENDENCY", new NodeRef("Artifact", "id", dep.getDeclaredIn()), pkgRef, + RowBuilder.prune(dp), dep.getKind()); + + // Every lock artifact present LOCKS every dependency it pinned. Pins from every lock + // file are already merged into one lockedVersion per dependency upstream + // (DependencyView.build), so there is no per-lock-file attribution left to split on -- + // a dependency locked with N lock artifacts present gets N LOCKS edges (matches + // analysis.json; a known limitation carried over from codeanalyzer-python as-is). + if (dep.getLockedVersion() != null) { + for (NodeRef lockRef : lockRefs) { + Map lp = RowBuilder.props(); + lp.put("version", dep.getLockedVersion()); + b.edge("LOCKS", lockRef, pkgRef, RowBuilder.prune(lp)); + } + } + } + } + } + + private static boolean isLockArtifact(String path) { + int slash = path.lastIndexOf('/'); + String base = slash < 0 ? path : path.substring(slash + 1); + return LOCK_BASENAME.equals(base); + } + // ------------------------------------------------------------------------------------------ // Imports, annotations, shared helpers // ------------------------------------------------------------------------------------------ diff --git a/src/main/java/com/ibm/cldk/neo4j/V2SchemaCatalog.java b/src/main/java/com/ibm/cldk/neo4j/V2SchemaCatalog.java index 89920c2d..9064c410 100644 --- a/src/main/java/com/ibm/cldk/neo4j/V2SchemaCatalog.java +++ b/src/main/java/com/ibm/cldk/neo4j/V2SchemaCatalog.java @@ -21,7 +21,7 @@ import java.util.Map; /** - * The schema v2 Neo4j graph catalog (graph contract {@code 2.1.0}) — the in-repo source of truth + * The schema v2 Neo4j graph catalog (graph contract {@code 2.2.0}) — the in-repo source of truth * for what {@link V2GraphProjector} may emit, serialized by {@code --emit schema} and enforced by * the v2 conformance test. Mirrors codeanalyzer-python's {@code neo4j/schema.py} vocabulary with * {@code J}/{@code J_} namespacing; java-only constructs (enum constants, record components, @@ -42,7 +42,10 @@ private V2SchemaCatalog() {} // 2.1.0: additive MINOR — L4 SDG overlay (JBodyNode.var/call_node; J_PARAM_IN/J_PARAM_OUT/ // J_SUMMARY, reserved at 2.0.0, now actually emitted). - public static final String SCHEMA_VERSION = "2.1.0"; + // 2.2.0: additive MINOR — the repository-artifact layer (#197): Artifact/Package/ConfigKey + // reserved at 2.0.0, now actually emitted, plus HAS_ARTIFACT/DEFINES_CONFIG/ + // DECLARES_DEPENDENCY/LOCKS. + public static final String SCHEMA_VERSION = "2.2.0"; /** Labels layered onto a node in addition to its merge + specific labels. */ public static final List MARKER_LABELS = Arrays.asList("JEntrypoint"); @@ -135,6 +138,32 @@ private static List buildNodeLabels() { n.add(node("JAnnotation", "JAnnotation", "name", new P().put("name", "string").done())); + // Neutral artifact/dependency subgraph (the repository-artifact layer, #197). No `J`/`J_` + // prefix on these three nodes or the containment edges below -- deliberate: `Artifact`, + // `Package` and `ConfigKey` are cross-language merge targets, so a sibling-language analyzer + // over the same repository lands on the same nodes instead of a per-language duplicate. + // Mirrors codeanalyzer-python's identical un-prefixed vocabulary and its rationale. + n.add(node("Artifact", "Artifact", "id", + new P().put("id", "string").put("path", "string").put("format", "string") + .put("roles", "string[]").put("size_bytes", "integer").put("sha256", "string") + .put("source", "string").put("text_truncated", "boolean") + .put("extraction", "string").done())); + + // `group` is additive over codeanalyzer-python's `Package` (PyPI names are single-segment); + // Maven splits a coordinate into groupId + artifactId, so both are carried. + n.add(node("Package", "Package", "id", + new P().put("id", "string").put("ecosystem", "string").put("group", "string") + .put("name", "string").done())); + + // A configuration key flattened out of a config-bearing Artifact. Neutral vocabulary like + // Artifact/Package -- a properties/yaml/xml/env key is not a Java concept. `value` is + // omitted (not null) when the source model's value is null (--no-artifact-text, or a + // namespace with no value at that path); `references` is omitted only when empty (the + // general list-property rule every other node in this catalog already follows). + n.add(node("ConfigKey", "ConfigKey", "id", + lines(new P().put("id", "string").put("key", "string").put("namespace", "string") + .put("value", "string").put("references", "string[]")))); + return n; } @@ -178,6 +207,20 @@ private static List buildRelTypes() { r.add(rel("J_PARAM_OUT", body, body, new P().put("var", "string").done())); r.add(rel("J_SUMMARY", body, body, none)); + // Neutral artifact/dependency subgraph (the repository-artifact layer, #197) -- no `J_` + // prefix, same cross-language-merge-target reasoning as the Artifact/Package/ConfigKey + // nodes above. + r.add(rel("HAS_ARTIFACT", Arrays.asList("JApplication"), Arrays.asList("Artifact"), none)); + r.add(rel("DEFINES_CONFIG", Arrays.asList("Artifact"), Arrays.asList("ConfigKey"), none)); + // `_k` (merges per `kind`): the same manifest may declare one package twice under different + // kinds (e.g. a runtime dependency re-listed under an optional extra) -- same endpoint pair, + // so without the discriminant the plain MERGE collapses the two declarations into one row. + r.add(rel("DECLARES_DEPENDENCY", Arrays.asList("Artifact"), Arrays.asList("Package"), + new P().put("spec", "string").put("kind", "string").put("extras", "string[]") + .put("prov", "string[]").put("direct", "boolean").put("_k", "string").done())); + r.add(rel("LOCKS", Arrays.asList("Artifact"), Arrays.asList("Package"), + new P().put("version", "string").done())); + return r; } diff --git a/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java b/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java index e826691b..1515abdc 100644 --- a/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java +++ b/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java @@ -184,10 +184,10 @@ void emitSchemaAlwaysEmitsTheV2Catalog(@TempDir Path tmp) throws IOException { assertEquals(0, run("--emit", "schema", "-o", out.toString())); JsonObject doc = JsonParser.parseString(Files.readString(out.resolve("schema.neo4j.json"))) .getAsJsonObject(); - assertEquals("2.1.0", doc.get("schema_version").getAsString()); + assertEquals("2.2.0", doc.get("schema_version").getAsString()); assertEquals(0, run("--emit", "schema", "-o", out.toString(), "--schema", "v1"), "--emit schema ignores --schema"); - assertEquals("2.1.0", JsonParser.parseString(Files.readString(out.resolve("schema.neo4j.json"))) + assertEquals("2.2.0", JsonParser.parseString(Files.readString(out.resolve("schema.neo4j.json"))) .getAsJsonObject().get("schema_version").getAsString()); } diff --git a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java index 92120978..6de58bee 100644 --- a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java +++ b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java @@ -15,19 +15,28 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.ibm.cldk.artifacts.ArtifactDiscovery; +import com.ibm.cldk.artifacts.ConfigKeys; +import com.ibm.cldk.artifacts.DependencyView; import com.ibm.cldk.neo4j.GraphRows.EdgeRow; import com.ibm.cldk.neo4j.GraphRows.NodeRow; import com.ibm.cldk.neo4j.SchemaCatalog.NodeLabel; import com.ibm.cldk.neo4j.SchemaCatalog.RelType; import com.ibm.cldk.schema.Analysis; +import com.ibm.cldk.schema.CanId; +import com.ibm.cldk.schema.JArtifact; +import com.ibm.cldk.schema.JDependency; import com.ibm.cldk.schema.JModule; import com.ibm.cldk.schema.V2Emitter; import com.ibm.cldk.syntactic_analysis.L1Extractor; import com.ibm.cldk.syntactic_analysis.L2CallGraph; import com.ibm.cldk.syntactic_analysis.dataflow.SdgVertices; import com.ibm.cldk.syntactic_analysis.dataflow.SummaryPass; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.HashMap; @@ -38,12 +47,13 @@ import java.util.Set; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; /** * Schema v2 graph conformance (no container needed): run the real L1–L3 pipeline plus the L4 SDG * passes ({@link SdgVertices}, {@link SummaryPass}) over a fixture, project with * {@link V2GraphProjector}, and assert the projector only ever produces what - * {@link V2SchemaCatalog} declares — the anti-drift guard for the 2.1.0 graph contract. Also pins + * {@link V2SchemaCatalog} declares — the anti-drift guard for the 2.2.0 graph contract. Also pins * the convergence decisions: body nodes instead of call-site nodes, and the {@code _k}-keyed * CFG/DDG relationships. */ @@ -54,6 +64,13 @@ public class V2Neo4jSchemaConformanceTest { // call-graph-test. private static final Path FIXTURE = Paths.get("src/test/resources/test-applications/l4-sdg-test"); + // A throwaway repository-artifact fixture, independent of FIXTURE above: ArtifactDiscovery / + // DependencyView / ConfigKeys only care about non-.java files, so this is populated with a + // pom.xml, a matching gradle.lockfile pin and an application.properties -- one manifest declared + // AND locked, so HAS_ARTIFACT/DEFINES_CONFIG/DECLARES_DEPENDENCY/LOCKS are all non-empty below. + @TempDir + static Path ARTIFACT_TMP; + private static GraphRows rows; private static final Map BY_LABEL = new HashMap<>(); @@ -75,9 +92,33 @@ static void project() throws Exception { L2CallGraph.Result l2 = L2CallGraph.build("l4-sdg-test", modules, null, true); SdgVertices.Result sdg = SdgVertices.apply(modules); SummaryPass.apply(modules, l2.callGraph(), 3); + + // Repository-artifact layer (Task 7): mirrors CodeAnalyzer's own wiring (discover, then + // build dependencies, then flatten config keys) over ARTIFACT_TMP. + Files.writeString(ARTIFACT_TMP.resolve("pom.xml"), + "" + + "org.examplewidget" + + "1.0.0" + + "", + StandardCharsets.UTF_8); + Files.writeString(ARTIFACT_TMP.resolve("gradle.lockfile"), + "org.example:widget:1.0.0=compileClasspath,runtimeClasspath\n", StandardCharsets.UTF_8); + Files.writeString(ARTIFACT_TMP.resolve("application.properties"), + "server.port=8080\nspring.datasource.url=${DB_URL}\n", StandardCharsets.UTF_8); + Map artifacts = + ArtifactDiscovery.discover(ARTIFACT_TMP, "l4-sdg-test", true, 262144); + List dependencies = DependencyView.build(ARTIFACT_TMP, artifacts); + for (JArtifact a : artifacts.values()) { + if (ConfigKeys.isEligible(a)) { + ConfigKeys.Result r = ConfigKeys.extract( + a, DependencyView.readFromDisk(ARTIFACT_TMP, a.getPath()), true); + a.setConfigKeys(r.keys); + } + } + Analysis analysis = V2Emitter.emit( "l4-sdg-test", 3, modules, "test", l2.callGraph(), l2.externalSymbols(), - sdg.paramIn, sdg.paramOut); + sdg.paramIn, sdg.paramOut, artifacts, dependencies); rows = V2GraphProjector.project(analysis, "l4-sdg-test"); } @@ -198,4 +239,75 @@ void l4OverlayProjectsParamAndSummaryEdges() { assertTrue(paramOut, "J_PARAM_OUT projected from application param_out"); assertTrue(summary, "J_SUMMARY projected from callable summaries"); } + + // ------------------------------------------------------------------------------------------ + // Repository-artifact layer (Task 7): Artifact/Package/ConfigKey, graph contract 2.2.0. + // ------------------------------------------------------------------------------------------ + + @Test + void artifactLayerNodesAreEmitted() { + boolean sawArtifact = false; + boolean sawPackage = false; + boolean sawConfigKey = false; + for (NodeRow node : rows.nodes) { + String merge = node.labels.get(0); + sawArtifact |= merge.equals("Artifact"); + sawPackage |= merge.equals("Package"); + sawConfigKey |= merge.equals("ConfigKey"); + } + assertTrue(sawArtifact, "no :Artifact rows projected"); + assertTrue(sawPackage, "no :Package rows projected"); + assertTrue(sawConfigKey, "no :ConfigKey rows projected"); + } + + @Test + void packageNodeIsKeyedByPurlAndCarriesMavenCoordinates() { + String pkgId = CanId.purlMaven("org.example", "widget"); + NodeRow pkg = findNode("Package", pkgId); + assertNotNull(pkg, "expected a :Package node keyed on " + pkgId); + assertEquals("maven", pkg.props.get("ecosystem")); + assertEquals("org.example", pkg.props.get("group")); + assertEquals("widget", pkg.props.get("name")); + } + + @Test + void artifactLayerEdgesAreEmittedAndDeclaresDependencyIsKeyedByKind() { + boolean hasArtifact = false; + boolean definesConfig = false; + boolean declaresDependency = false; + boolean locks = false; + String declaresDependencyKey = null; + String pkgId = CanId.purlMaven("org.example", "widget"); + for (EdgeRow edge : rows.edges) { + hasArtifact |= edge.type.equals("HAS_ARTIFACT"); + definesConfig |= edge.type.equals("DEFINES_CONFIG"); + if (edge.type.equals("DECLARES_DEPENDENCY") && edge.to.value.equals(pkgId)) { + declaresDependency = true; + declaresDependencyKey = edge.key; + assertEquals("1.0.0", edge.props.get("spec")); + assertEquals("runtime", edge.props.get("kind")); + assertEquals(Boolean.TRUE, edge.props.get("direct")); + } + if (edge.type.equals("LOCKS") && edge.to.value.equals(pkgId)) { + locks = true; + assertEquals("1.0.0", edge.props.get("version")); + assertNull(edge.key, "LOCKS carries no _k discriminant"); + } + } + assertTrue(hasArtifact, "no HAS_ARTIFACT edges projected"); + assertTrue(definesConfig, "no DEFINES_CONFIG edges projected"); + assertTrue(declaresDependency, "no DECLARES_DEPENDENCY edge for the fixture's declared package"); + assertTrue(locks, "no LOCKS edge for the fixture's locked package"); + assertEquals("runtime", declaresDependencyKey, + "DECLARES_DEPENDENCY must carry the _k=kind MERGE discriminant"); + } + + private static NodeRow findNode(String mergeLabel, String value) { + for (NodeRow node : rows.nodes) { + if (node.labels.get(0).equals(mergeLabel) && node.value.equals(value)) { + return node; + } + } + return null; + } } From a025f6cf318f14fe849cdc1e82ff3693e9a8eff6 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 03:11:38 -0400 Subject: [PATCH 14/19] fix(artifacts): DDL + wipe coverage for Artifact/Package/ConfigKey Two gaps found in review of the graph contract 2.2.0 work: - schema.neo4j.json (via V2SchemaCatalog.uniquenessConstraints()) promised artifact_id/package_id/configkey_id uniqueness constraints, but Schema.CONSTRAINTS -- the hand-maintained list CypherWriter and BoltWriter actually execute -- never gained them, so no load ever created what the document promised. Added the same three, verbatim matching the auto-derived names/alias so the document and the executed DDL agree exactly, not merely both exist. - CypherWriter's wipe traversal couldn't reach :Artifact/:ConfigKey at all: HAS_ARTIFACT hangs off :JApplication directly (not off the :JModule/:JCompilationUnit the wipe's first hop was scoped to), so a repeat push of the same application after a config-bearing artifact was removed left the old :Artifact and its :ConfigKey rows as permanent orphans -- the same bug class the v1/v2 unification already guards against, recreated for this new label family. Fixed by folding HAS_ARTIFACT into the app-anchor's first hop and DEFINES_CONFIG into DESCENDANTS, reusing the existing wipe mechanism rather than adding a second one. :Package stays unreachable from the wipe on purpose, same as :JPackage/:JAnnotation -- a purl-keyed Package is a cross-app merge target, not owned by one application's push. Verified live: pushed a fixture into a throwaway Neo4j 5 container, removed a config-bearing artifact from the analyzed repo, pushed again -- the removed Artifact and its ConfigKeys are gone (0 orphans), the still-present pom.xml/Package survive untouched. --- .../java/com/ibm/cldk/neo4j/CypherWriter.java | 27 ++++++++++++------- src/main/java/com/ibm/cldk/neo4j/Schema.java | 10 ++++++- .../neo4j/V2Neo4jSchemaConformanceTest.java | 17 ++++++++++++ 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java b/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java index ffec529f..eedb1a75 100644 --- a/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java +++ b/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java @@ -34,12 +34,18 @@ public final class CypherWriter { /** * Every containment relationship either graph generation emits — v1 (unit-rooted) and v2 * (module-rooted) together, so a v2 push wipes/prunes a prior v1 graph of the same app and vice - * versa (spec: one app name = one graph, latest push wins). + * versa (spec: one app name = one graph, latest push wins). {@code DEFINES_CONFIG} rides along + * too: {@link #wipe} widens the app anchor's first hop with {@code HAS_ARTIFACT} so {@code c} + * can also bind an {@code :Artifact}, and this pattern then reaches that artifact's + * {@code :ConfigKey}s the same way it reaches a module's types/callables/etc. — one wipe + * mechanism, not two. {@code :Package} is deliberately NOT reachable here, same as + * {@code :JPackage}/{@code :JAnnotation} below: a purl-keyed Package is a cross-app, + * cross-language merge target, not owned by any one application's push. */ static final String DESCENDANTS = "[:J_DECLARES_TYPE|J_HAS_NESTED_TYPE|J_HAS_CALLABLE|J_HAS_FIELD|J_HAS_PARAMETER" + "|J_HAS_CALLSITE|J_DECLARES_VAR|J_HAS_ENUM_CONSTANT|J_HAS_RECORD_COMPONENT|J_HAS_INIT_BLOCK" + "|J_HAS_CRUD_OPERATION|J_HAS_CRUD_QUERY|J_HAS_COMMENT" - + "|J_DECLARES|J_HAS_METHOD|J_HAS_BODY_NODE*1..]"; + + "|J_DECLARES|J_HAS_METHOD|J_HAS_BODY_NODE|DEFINES_CONFIG*1..]"; private CypherWriter() {} @@ -55,7 +61,7 @@ public static String renderCypher(GraphRows rows, String appName) { } out.add(""); - out.add("// ── wipe this project's prior subgraph (packages/annotations are shared) ──"); + out.add("// ── wipe this project's prior subgraph (packages/annotations/Package are shared) ──"); out.add(wipe(appName)); out.add(""); @@ -72,14 +78,17 @@ public static String renderCypher(GraphRows rows, String appName) { private static String wipe(String appName) { // The unit hop is unlabeled and lists both generations' rel types (v1 J_HAS_UNIT → - // :JCompilationUnit, v2 J_HAS_MODULE → :JModule) so either generation's push replaces - // whichever generation the DB currently holds for this app. The second statement sweeps - // fully-isolated :JSymbol nodes the containment traversal cannot reach — v1's - // import-materialized bodyless :JType stubs hang off units via J_IMPORTS only, so the - // DETACH DELETE above orphans them; degree-0 symbols are unreferencable junk in any + // :JCompilationUnit, v2 J_HAS_MODULE → :JModule) PLUS HAS_ARTIFACT → :Artifact, so either + // generation's push replaces whichever generation the DB currently holds for this app, AND + // this app's artifact subtree (DEFINES_CONFIG → :ConfigKey, folded into DESCENDANTS) goes + // with it — an Artifact/ConfigKey removed from a later analysis of the same app must not + // survive as an orphan (same bug class the v1/v2 unification above already guards against). + // The second statement sweeps fully-isolated :JSymbol nodes the containment traversal cannot + // reach — v1's import-materialized bodyless :JType stubs hang off units via J_IMPORTS only, + // so the DETACH DELETE above orphans them; degree-0 symbols are unreferencable junk in any // generation, and a symbol another application still uses keeps its edges and survives. return "MATCH (a:JApplication {name: " + cypherValue(appName) + "})\n" - + "OPTIONAL MATCH (a)-[:J_HAS_UNIT|J_HAS_MODULE]->(c)\n" + + "OPTIONAL MATCH (a)-[:J_HAS_UNIT|J_HAS_MODULE|HAS_ARTIFACT]->(c)\n" + "OPTIONAL MATCH (c)-" + DESCENDANTS + "->(x)\n" + "DETACH DELETE x, c, a;\n" + "MATCH (s:JSymbol) WHERE NOT (s)--() DELETE s;"; diff --git a/src/main/java/com/ibm/cldk/neo4j/Schema.java b/src/main/java/com/ibm/cldk/neo4j/Schema.java index acd43e37..31991334 100644 --- a/src/main/java/com/ibm/cldk/neo4j/Schema.java +++ b/src/main/java/com/ibm/cldk/neo4j/Schema.java @@ -43,7 +43,15 @@ private Schema() {} // Schema v2 (graph 2.0.0) additions — the writers run the union so either generation's // graph stays constraint-protected in a shared database. "CREATE CONSTRAINT j_module_id IF NOT EXISTS FOR (m:JModule) REQUIRE m.id IS UNIQUE", - "CREATE CONSTRAINT j_body_node_id IF NOT EXISTS FOR (bn:JBodyNode) REQUIRE bn.id IS UNIQUE"); + "CREATE CONSTRAINT j_body_node_id IF NOT EXISTS FOR (bn:JBodyNode) REQUIRE bn.id IS UNIQUE", + // Repository-artifact layer (graph 2.2.0) additions — no `j_` prefix, matching the + // labels themselves (Artifact/Package/ConfigKey are un-prefixed cross-language merge + // targets, see V2SchemaCatalog). Names/alias match V2SchemaCatalog.uniquenessConstraints()'s + // auto-derived output verbatim so the emitted schema.neo4j.json document and this + // executed DDL agree on these three, not merely both existing. + "CREATE CONSTRAINT artifact_id IF NOT EXISTS FOR (x:Artifact) REQUIRE x.id IS UNIQUE", + "CREATE CONSTRAINT package_id IF NOT EXISTS FOR (x:Package) REQUIRE x.id IS UNIQUE", + "CREATE CONSTRAINT configkey_id IF NOT EXISTS FOR (x:ConfigKey) REQUIRE x.id IS UNIQUE"); public static final List INDEXES = Arrays.asList( "CREATE INDEX j_callable_name IF NOT EXISTS FOR (c:JCallable) ON (c.name)", diff --git a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java index 6de58bee..3d73c864 100644 --- a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java +++ b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java @@ -302,6 +302,23 @@ void artifactLayerEdgesAreEmittedAndDeclaresDependencyIsKeyedByKind() { "DECLARES_DEPENDENCY must carry the _k=kind MERGE discriminant"); } + @Test + void wipeReachesArtifactAndConfigKeySoARepushLeavesNoOrphans() { + // Same cypher-text-assertion shape as wipeCoversBothGenerationsSoV2ReplacesAPriorV1Graph + // above (no in-process Neo4j to actually execute the wipe against and check for orphans). + // The scenario this pins: push, then remove a config-bearing artifact from the analyzed + // repo and push again -- without HAS_ARTIFACT on the app anchor's first hop, the wipe's + // OPTIONAL MATCH (a)-[...]->(c) never binds the prior push's :Artifact nodes at all, so + // DETACH DELETE never reaches them (or their :ConfigKey rows via DEFINES_CONFIG), and both + // survive the second push as permanent orphans. + String cypher = CypherWriter.renderCypher(rows, "l4-sdg-test"); + assertTrue(cypher.contains("J_HAS_UNIT|J_HAS_MODULE|HAS_ARTIFACT"), + "the wipe's app-anchor hop must also reach this app's :Artifact nodes via HAS_ARTIFACT"); + assertTrue(CypherWriter.DESCENDANTS.contains("DEFINES_CONFIG"), + "wipe/prune descendant traversal must include DEFINES_CONFIG so a wiped " + + "Artifact's ConfigKeys are swept too"); + } + private static NodeRow findNode(String mergeLabel, String value) { for (NodeRow node : rows.nodes) { if (node.labels.get(0).equals(mergeLabel) && node.value.equals(value)) { From 9a86241300af9eac8a9f30f40770edbf0d2f3f8e Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 03:39:20 -0400 Subject: [PATCH 15/19] fix(artifacts): revert wipe reaching Artifact/ConfigKey; add negative guards The prior fix (a025f6c) widened CypherWriter's wipe to reach :Artifact/ :ConfigKey so a re-push wouldn't leave them stale. That was wrong: both are un-prefixed cross-language merge targets by design (CanId.artifactId's own javadoc -- the `artifact` id segment exists precisely so a sibling-language analyzer over the same repository lands on the same node). When it does, its own edges attach to that shared node. A Java wipe that reaches it DETACH DELETEs everything on it, including that other analyzer's edges, and recreates only what Java itself declares -- silently destroying data in a tool Java cannot see or repair. Reverted DESCENDANTS and the wipe's first hop to their pre-widening form (functionally identical to 26150a5, comments extended to name all three shared labels -- Package, Artifact, ConfigKey -- and the reason). This restores the tradeoff already accepted for Package/JPackage/ JAnnotation: a stale Artifact/ConfigKey survives a re-push after its file is removed from the repo, recoverable with a full re-push or a separate sweep. That is the safe direction versus the alternative of occasionally corrupting a sibling tool's graph with no way to detect it. Two tests added, both proven to catch what they guard against (verified by temporarily reintroducing the removed code/line and confirming each fails, then restoring): - wipeStaysOffTheCrossLanguageArtifactPackageSubgraph: the negative counterpart to the existing "wipe covers both generations" test -- asserts the app-anchor hop stays exactly J_HAS_UNIT|J_HAS_MODULE and DESCENDANTS never contains HAS_ARTIFACT/DEFINES_CONFIG/ DECLARES_DEPENDENCY/LOCKS, so widening this again requires deliberately overriding an explicit assertion, not just missing a gap. - constraintsStayInSyncWithTheCatalog: every (merge_label, key) pair V2SchemaCatalog.uniquenessConstraints() derives (what schema.neo4j.json promises) has a semantically matching entry in Schema.CONSTRAINTS (what CypherWriter/BoltWriter actually execute) -- the exact document-vs-DDL drift the previous review round found and fixed by hand, now guarded so a future schema addition can't reintroduce it silently. --- .../java/com/ibm/cldk/neo4j/CypherWriter.java | 41 +++++++----- .../neo4j/V2Neo4jSchemaConformanceTest.java | 67 +++++++++++++++---- 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java b/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java index eedb1a75..7a6116a7 100644 --- a/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java +++ b/src/main/java/com/ibm/cldk/neo4j/CypherWriter.java @@ -34,18 +34,24 @@ public final class CypherWriter { /** * Every containment relationship either graph generation emits — v1 (unit-rooted) and v2 * (module-rooted) together, so a v2 push wipes/prunes a prior v1 graph of the same app and vice - * versa (spec: one app name = one graph, latest push wins). {@code DEFINES_CONFIG} rides along - * too: {@link #wipe} widens the app anchor's first hop with {@code HAS_ARTIFACT} so {@code c} - * can also bind an {@code :Artifact}, and this pattern then reaches that artifact's - * {@code :ConfigKey}s the same way it reaches a module's types/callables/etc. — one wipe - * mechanism, not two. {@code :Package} is deliberately NOT reachable here, same as - * {@code :JPackage}/{@code :JAnnotation} below: a purl-keyed Package is a cross-app, - * cross-language merge target, not owned by any one application's push. + * versa (spec: one app name = one graph, latest push wins). {@code :Package}, {@code :Artifact} + * and {@code :ConfigKey} are all deliberately unreachable from this wipe, same as + * {@code :JPackage}/{@code :JAnnotation} below: all three are un-prefixed cross-language merge + * targets ({@code CanId.artifactId}'s own javadoc: the {@code artifact} id segment exists so a + * sibling-language analyzer scanning the same repository lands on the same node rather than a + * duplicate). That other analyzer's own edges may be attached to the very node this wipe would + * {@code DETACH DELETE}, destroying data this tool cannot see and did not write — corruption + * neither detectable nor repairable from here. A stale {@code :Artifact}/{@code :ConfigKey} + * left behind after a file is removed from the analyzed repo is the accepted tradeoff instead: + * recoverable with a full re-push or a separate sweep, unlike another tool's silently deleted + * edges. (Tried once, reverted: see git history — widening this to reach {@code :Artifact}/ + * {@code :ConfigKey} was implemented and shipped before this exact hazard was caught in + * review.) */ static final String DESCENDANTS = "[:J_DECLARES_TYPE|J_HAS_NESTED_TYPE|J_HAS_CALLABLE|J_HAS_FIELD|J_HAS_PARAMETER" + "|J_HAS_CALLSITE|J_DECLARES_VAR|J_HAS_ENUM_CONSTANT|J_HAS_RECORD_COMPONENT|J_HAS_INIT_BLOCK" + "|J_HAS_CRUD_OPERATION|J_HAS_CRUD_QUERY|J_HAS_COMMENT" - + "|J_DECLARES|J_HAS_METHOD|J_HAS_BODY_NODE|DEFINES_CONFIG*1..]"; + + "|J_DECLARES|J_HAS_METHOD|J_HAS_BODY_NODE*1..]"; private CypherWriter() {} @@ -61,7 +67,7 @@ public static String renderCypher(GraphRows rows, String appName) { } out.add(""); - out.add("// ── wipe this project's prior subgraph (packages/annotations/Package are shared) ──"); + out.add("// ── wipe this project's prior subgraph (packages/annotations/artifacts/config keys are shared) ──"); out.add(wipe(appName)); out.add(""); @@ -78,17 +84,16 @@ public static String renderCypher(GraphRows rows, String appName) { private static String wipe(String appName) { // The unit hop is unlabeled and lists both generations' rel types (v1 J_HAS_UNIT → - // :JCompilationUnit, v2 J_HAS_MODULE → :JModule) PLUS HAS_ARTIFACT → :Artifact, so either - // generation's push replaces whichever generation the DB currently holds for this app, AND - // this app's artifact subtree (DEFINES_CONFIG → :ConfigKey, folded into DESCENDANTS) goes - // with it — an Artifact/ConfigKey removed from a later analysis of the same app must not - // survive as an orphan (same bug class the v1/v2 unification above already guards against). - // The second statement sweeps fully-isolated :JSymbol nodes the containment traversal cannot - // reach — v1's import-materialized bodyless :JType stubs hang off units via J_IMPORTS only, - // so the DETACH DELETE above orphans them; degree-0 symbols are unreferencable junk in any + // :JCompilationUnit, v2 J_HAS_MODULE → :JModule) so either generation's push replaces + // whichever generation the DB currently holds for this app. Deliberately does NOT include + // HAS_ARTIFACT → :Artifact: see DESCENDANTS' javadoc for why the whole artifact/config-key + // subtree stays outside every wipe this class runs. The second statement sweeps + // fully-isolated :JSymbol nodes the containment traversal cannot reach — v1's + // import-materialized bodyless :JType stubs hang off units via J_IMPORTS only, so the + // DETACH DELETE above orphans them; degree-0 symbols are unreferencable junk in any // generation, and a symbol another application still uses keeps its edges and survives. return "MATCH (a:JApplication {name: " + cypherValue(appName) + "})\n" - + "OPTIONAL MATCH (a)-[:J_HAS_UNIT|J_HAS_MODULE|HAS_ARTIFACT]->(c)\n" + + "OPTIONAL MATCH (a)-[:J_HAS_UNIT|J_HAS_MODULE]->(c)\n" + "OPTIONAL MATCH (c)-" + DESCENDANTS + "->(x)\n" + "DETACH DELETE x, c, a;\n" + "MATCH (s:JSymbol) WHERE NOT (s)--() DELETE s;"; diff --git a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java index 3d73c864..d3c88046 100644 --- a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java +++ b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java @@ -45,6 +45,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -303,20 +305,57 @@ void artifactLayerEdgesAreEmittedAndDeclaresDependencyIsKeyedByKind() { } @Test - void wipeReachesArtifactAndConfigKeySoARepushLeavesNoOrphans() { - // Same cypher-text-assertion shape as wipeCoversBothGenerationsSoV2ReplacesAPriorV1Graph - // above (no in-process Neo4j to actually execute the wipe against and check for orphans). - // The scenario this pins: push, then remove a config-bearing artifact from the analyzed - // repo and push again -- without HAS_ARTIFACT on the app anchor's first hop, the wipe's - // OPTIONAL MATCH (a)-[...]->(c) never binds the prior push's :Artifact nodes at all, so - // DETACH DELETE never reaches them (or their :ConfigKey rows via DEFINES_CONFIG), and both - // survive the second push as permanent orphans. - String cypher = CypherWriter.renderCypher(rows, "l4-sdg-test"); - assertTrue(cypher.contains("J_HAS_UNIT|J_HAS_MODULE|HAS_ARTIFACT"), - "the wipe's app-anchor hop must also reach this app's :Artifact nodes via HAS_ARTIFACT"); - assertTrue(CypherWriter.DESCENDANTS.contains("DEFINES_CONFIG"), - "wipe/prune descendant traversal must include DEFINES_CONFIG so a wiped " - + "Artifact's ConfigKeys are swept too"); + void wipeStaysOffTheCrossLanguageArtifactPackageSubgraph() { + // The negative counterpart to wipeCoversBothGenerationsSoV2ReplacesAPriorV1Graph above, + // which asserts what the wipe DOES reach; nothing asserted what it must NOT, which is + // exactly how a well-intentioned widening (fold Artifact/ConfigKey into the same wipe that + // already unifies v1/v2) shipped and had to be reverted. :Artifact, :ConfigKey and :Package + // are un-prefixed cross-language merge targets (CanId.artifactId's own javadoc: the + // `artifact` id segment exists so a sibling-language analyzer over the same repository + // lands on the same node, not a duplicate). A wipe reaching any of them would DETACH DELETE + // that other analyzer's own edges on every Java re-push of the same app -- silent + // corruption in a tool this one cannot see or repair. Read CypherWriter.DESCENDANTS' + // javadoc before ever widening either pattern checked below. + assertTrue(CypherWriter.renderCypher(rows, "l4-sdg-test") + .contains("OPTIONAL MATCH (a)-[:J_HAS_UNIT|J_HAS_MODULE]->(c)"), + "the wipe's app-anchor hop must stay exactly J_HAS_UNIT|J_HAS_MODULE -- widening it " + + "to HAS_ARTIFACT lets a Java re-push delete another analyzer's edges on " + + "the shared :Artifact merge target"); + for (String rel : new String[] {"HAS_ARTIFACT", "DEFINES_CONFIG", "DECLARES_DEPENDENCY", "LOCKS"}) { + assertFalse(CypherWriter.DESCENDANTS.contains(rel), + "wipe/prune descendant traversal must never include " + rel + " -- " + + "Artifact/Package/ConfigKey are cross-language merge targets a wipe must not touch"); + } + } + + @Test + void constraintsStayInSyncWithTheCatalog() { + // Schema.CONSTRAINTS is the hand-maintained list CypherWriter/BoltWriter actually execute; + // V2SchemaCatalog.uniquenessConstraints() is what the emitted schema.neo4j.json document + // promises (one entry per distinct (merge_label, key)). The two must agree semantically -- + // not byte-for-byte: Schema.CONSTRAINTS predates the derived naming/alias convention and + // keeps its own descriptive names (e.g. `j_symbol_id`, alias `s`) rather than the derived + // form (`jsymbol_id`, alias `x`) -- but a promised constraint no load ever creates is + // exactly the contract-overpromise defect class this conformance test class already guards + // against, one level up (#197 review: schema.neo4j.json shipped promising three constraints + // no load created because Schema.CONSTRAINTS was never extended alongside the catalog). + Pattern labelAndKey = Pattern.compile("FOR \\(\\w+:(\\w+)\\) REQUIRE \\w+\\.(\\w+) IS UNIQUE"); + for (String derived : V2SchemaCatalog.uniquenessConstraints()) { + Matcher dm = labelAndKey.matcher(derived); + assertTrue(dm.find(), "unparseable derived constraint: " + derived); + Pattern expected = Pattern.compile( + "FOR \\(\\w+:" + dm.group(1) + "\\) REQUIRE \\w+\\." + dm.group(2) + " IS UNIQUE"); + boolean present = false; + for (String executed : Schema.CONSTRAINTS) { + if (expected.matcher(executed).find()) { + present = true; + break; + } + } + assertTrue(present, "Schema.CONSTRAINTS has no uniqueness constraint for (" + + dm.group(1) + ", " + dm.group(2) + ") -- schema.neo4j.json promises one but no load " + + "would create it"); + } } private static NodeRow findNode(String mergeLabel, String value) { From 2b1b12b0df830181cf10bb164f9911f48cb4979e Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 04:19:13 -0400 Subject: [PATCH 16/19] docs(artifacts): correct javadocs that describe behaviour the code does not have Four documentation defects in the repository-artifact layer, all of the same class: prose that survived from an early spec instead of describing what the code emits. - ArtifactDiscovery's class javadoc told a maintainer that later tasks read `source` back out of the returned artifacts "so that later reading never has to touch the filesystem again". That is the exact inverse of the binding constraint: `source` is empty under --no-artifact-text and a truncated prefix past --artifact-text-max-bytes, so extraction reads from disk via DependencyView.readFromDisk. Two adversarial tests already fence the behaviour; the front-door javadoc was inviting the regression they guard. - JArtifact listed roles "build, ci, deploy, config, dependency-lock" and called them user-assigned. They are rule-assigned by ArtifactDiscovery's RULES table, and the emitted vocabulary is dependency-manifest, tool-config, container-image, service-topology, iac, ci, env, legal, docs, script, unknown. - JDependency promised ecosystems "maven, npm, gradle, pypi, golang"; there is no setEcosystem caller in the tree, so the field is always "maven". Matches the reference's candour about "pypi" being the only one it emits. - JApplication called the `dependencies` List "indexed by" a purl; it is one entry per declaration, sorted by (name, declaredIn). - ArtifactDiscovery's classify() comment claimed "k8s/*.yml" matches only directly under k8s/, contradicting globMatches' own correct comment thirteen lines down. The code is right (fnmatch semantics: '*' crosses '/'), so "k8s/base/svc.yml" matches too and the comment was the wrong half. --- .../ibm/cldk/artifacts/ArtifactDiscovery.java | 17 +++++++++++------ .../java/com/ibm/cldk/schema/JApplication.java | 6 ++++-- .../java/com/ibm/cldk/schema/JArtifact.java | 15 +++++++++++---- .../java/com/ibm/cldk/schema/JDependency.java | 9 +++++---- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java b/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java index 1f29faa2..a603d1fa 100644 --- a/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java +++ b/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java @@ -30,10 +30,13 @@ * no rule names: the symbol table (L1) already owns those, and inventorying them here too would * produce two nodes for one file with no way for a consumer to tell which is authoritative. * - *

Nothing here parses artifact content — later tasks read {@code source} back out of the - * returned {@link JArtifact}s to extract dependencies and config keys. This class only decides - * what a file IS and captures its bytes/text so that later reading never has to touch the - * filesystem again. + *

Nothing here parses artifact content. This class only decides what a file IS, hashes it, and + * captures its text for the emitted JSON payload. Extraction (dependencies, config keys) re-reads + * each file from disk via {@link DependencyView#readFromDisk} and must never read {@code + * source} back out of the returned {@link JArtifact}s: {@code source} is a payload-size-controlled + * field, empty under {@code --no-artifact-text} and a truncated prefix past {@code + * --artifact-text-max-bytes}, so extraction driven off it would silently degrade under either flag. + * See {@code readFromDisk}'s javadoc before wiring any new extraction pass. */ public final class ArtifactDiscovery { @@ -201,8 +204,10 @@ private static String basename(String relPosix) { return slash < 0 ? relPosix : relPosix.substring(slash + 1); } - // A pattern containing '/' matches the full repo-relative path (e.g. "k8s/*.yml" matches only - // directly under k8s/); a bare pattern matches just the basename (e.g. "*.xml" matches any-depth). + // A pattern containing '/' matches the full repo-relative path; a bare pattern matches just the + // basename (e.g. "*.xml" matches at any depth). Both are fnmatch-shaped, so '*' crosses '/' -- + // "k8s/*.yml" matches a nested "k8s/base/svc.yml" too, not only files directly under k8s/ (see + // globMatches below, which is where that is deliberate rather than accidental). private static Rule classify(String relPosix) { String name = basename(relPosix); for (Rule rule : RULES) { diff --git a/src/main/java/com/ibm/cldk/schema/JApplication.java b/src/main/java/com/ibm/cldk/schema/JApplication.java index 27ff173b..90ad192e 100644 --- a/src/main/java/com/ibm/cldk/schema/JApplication.java +++ b/src/main/java/com/ibm/cldk/schema/JApplication.java @@ -38,8 +38,10 @@ public class JApplication { private Map artifacts; /** - * Dependencies declared in the repository's artifacts, indexed by their canonical purl or - * project-level id. {@code null} (absent) when the layer produces no dependencies. + * Dependencies declared in the repository's artifacts — one entry per declaration, so a + * coordinate declared in two manifests appears twice, each entry naming its own {@code + * declaredIn}. Sorted by {@code (name, declaredIn)}. {@code null} (absent) when the layer + * produces no dependencies. */ private List dependencies; } diff --git a/src/main/java/com/ibm/cldk/schema/JArtifact.java b/src/main/java/com/ibm/cldk/schema/JArtifact.java index 17757f3d..17ccbf45 100644 --- a/src/main/java/com/ibm/cldk/schema/JArtifact.java +++ b/src/main/java/com/ibm/cldk/schema/JArtifact.java @@ -12,9 +12,16 @@ * *

{@code format} is free-vocabulary and identifies the parser: {@code xml}, {@code yaml}, * {@code json}, {@code properties}, {@code gradle}, {@code dockerfile}, {@code text}, {@code binary}. - * {@code roles} are also free-vocabulary — {@code build}, {@code ci}, {@code deploy}, {@code config}, - * {@code dependency-lock} — and are user-assigned per artifact. A build manifest is both a build - * artifact and a dependency declaration. + * + *

{@code roles} are assigned by {@code ArtifactDiscovery}'s first-match-wins classification + * rules, not by a user, and the vocabulary the rules actually emit is exactly: {@code + * dependency-manifest}, {@code tool-config}, {@code container-image}, {@code service-topology}, + * {@code iac}, {@code ci}, {@code env}, {@code legal}, {@code docs}, {@code script}, {@code + * unknown}. One artifact may carry several — a {@code build.gradle} is both a {@code + * dependency-manifest} and a {@code tool-config}. The list is open in the schema (a consumer must + * tolerate an unseen role), but this analyzer emits no role outside it: {@code + * dependency-manifest} in particular is load-bearing, gating both the text-capture cap exemption + * and dependency extraction. */ @Data public class JArtifact { @@ -27,7 +34,7 @@ public class JArtifact { /** Format identifier: xml|yaml|json|properties|gradle|dockerfile|text|binary. */ private String format; - /** Semantic roles, free-vocabulary: build, ci, deploy, config, dependency-lock, etc. */ + /** Rule-assigned semantic roles; see the class javadoc for the vocabulary actually emitted. */ private List roles = new ArrayList<>(); /** File size in bytes. */ diff --git a/src/main/java/com/ibm/cldk/schema/JDependency.java b/src/main/java/com/ibm/cldk/schema/JDependency.java index eff4052b..d5587ee0 100644 --- a/src/main/java/com/ibm/cldk/schema/JDependency.java +++ b/src/main/java/com/ibm/cldk/schema/JDependency.java @@ -10,9 +10,10 @@ * (codeanalyzer-python), which has no analogue since PyPI names are single-segment; in Maven each * coordinate is split into a {@code group} and {@code name} ({@code artifactId}). * - *

{@code ecosystem} and {@code kind} are free-vocabulary, matching the reference implementation: - * ecosystems include {@code maven}, {@code npm}, {@code gradle}, {@code pypi}, {@code golang}; - * kinds include {@code runtime}, {@code dev}, {@code optional}, {@code build}. + *

{@code ecosystem} exists for SDK symmetry with purl and is always {@code "maven"} — the only + * ecosystem this analyzer emits, exactly as the reference is candid about {@code "pypi"} being the + * only one it emits ({@code schema/py_schema.py:560}). {@code kind} is free-vocabulary; the values + * actually produced are {@code runtime}, {@code dev}, {@code optional} and {@code build}. * *

{@code lockedVersion} is {@code null} (omitted from JSON) when the dependency is unpinned — * recorded only when found in a lock file or package manager. @@ -25,7 +26,7 @@ public class JDependency { /** Maven {@code artifactId} (the package name). */ private String name; - /** Package ecosystem: maven|npm|gradle|pypi|golang, etc. Defaults to maven. */ + /** Package ecosystem; always {@code maven} — the only one this analyzer emits. */ private String ecosystem = "maven"; /** Declared version range or spec, verbatim as written (may be empty). */ From ad677b2ab12b92d69fb88324f2e896321700ffb1 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 04:19:21 -0400 Subject: [PATCH 17/19] docs(schema): state the env dual-mint id divergence from the reference Every deliberate divergence on this branch is stated in a comment where it occurs; this one was not, and it is the one that matters most. configKeyEnvDualMintId builds `@key:env/`. The reference builds `@key/env.` (config_keys.py:577 into ids.py:39). Ours is the correct grammar and stays: the reference's provably self-collides, because a plain yaml dotted path can itself begin with the segment `env.`, so one document with both a top-level `env:` block and a compose environment block yields two records under one id. The reference guarded the bare-name collision and missed the dotted-path one. The javadoc already argued why a prefixed key routed through configKeyId would be unsound, but never said the reference does exactly that, nor named the price: ConfigKey is an un-prefixed cross-language merge target, so in a polyglot repo the two analyzers currently mint different nodes for the same environment variable, splitting the very nodes that label exists to share. That stands until the reference converges. Grammar unchanged. --- src/main/java/com/ibm/cldk/schema/CanId.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/main/java/com/ibm/cldk/schema/CanId.java b/src/main/java/com/ibm/cldk/schema/CanId.java index 2df4196d..aafe1036 100644 --- a/src/main/java/com/ibm/cldk/schema/CanId.java +++ b/src/main/java/com/ibm/cldk/schema/CanId.java @@ -79,6 +79,24 @@ public static String configKeyId(String artifactId, String dottedKey) { * before either side has consumed any dotted-key or variable-name content — so the two can * never collide for any {@code dottedKey}/{@code bareKey} whatsoever, not merely for whatever * shape a test happens to construct. + * + *

Deliberate divergence from the reference implementation. codeanalyzer-python mints + * this id as {@code @key/env.} — a plain {@code "env."} prefix routed through + * its {@code config_key_id} ({@code artifacts/config_keys.py:577} into {@code schema/ids.py:39}). + * We mint {@code @key:env/} instead, and this grammar is the correct one: the + * reference's provably self-collides, because a yaml dotted path can itself begin with the + * segment {@code env.}, so one document carrying both a top-level {@code env:} block and a + * compose {@code environment:} block yields two different records under one id. The reference + * guarded the bare-name collision and missed the dotted-path one; {@code + * ArtifactModelTest#configKeyEnvDualMintIdNeverCollidesWithConfigKeyId} pins exactly that case + * here. Do not "converge" this back to the reference's spelling. + * + *

The consequence, stated plainly: {@code ConfigKey} is an un-prefixed cross-language merge + * target (see {@link #artifactId}), so for a {@code docker-compose.yml} in a polyglot repo the + * two analyzers currently mint different nodes for the same environment variable, + * splitting the very nodes that un-prefixed label exists to share. That split stands until the + * reference adopts this grammar; it is the price of not shipping a known id collision, and the + * convergence has to happen on the reference's side. */ public static String configKeyEnvDualMintId(String artifactId, String bareKey) { return artifactId + "@key:env/" + bareKey; From 0d45076a0d14d8bc3080d0e1a77b375905a84a79 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 04:19:34 -0400 Subject: [PATCH 18/19] test(artifacts): pin LOCKS dedup, edge sources, and the bare --artifact-text form Closes three test blind spots in the repository-artifact layer. LOCKS is a per-package fact, so a coordinate declared in two manifests and pinned by one lockfile walks the mint twice for one (lock, package) pair. The reference guards that with a `seen` set (neo4j/project.py:350-359) because its RowBuilder.finish() only sorts. Ours dedupes edges by (type, from, to, _k) there, so the repeat mint already collapses and a guard would be dead code -- an assertion, not a belief: with that dedupe disabled the new test reports 2 rows, with it 1. The test pins the deduped-bag contract at the observable boundary whichever layer keeps it, and asserts the DECLARES_DEPENDENCY half too: it stays one row per declaring manifest and must never be collapsed per package. The divergence from the reference's implementation is now stated where it occurs. The artifact-layer edge tests asserted only `to`, props and key, never `from` -- one-sided coverage of the kind that already let a wrong decision ship on this layer. All four types now assert both endpoints. Verified by mutation: reversing each of the four fails, and so do two wrong-but-plausible sources (LOCKS from the declaring manifest, DECLARES_DEPENDENCY from a wrong artifact id) that only the new `from` assertions can catch. Nothing exercised a bare --artifact-text. Its fallbackValue = "true" exists only because picocli 4.1.0 resolves the bare negatable form to the opposite of what negatable=true implies, so a cleanup of that "redundant" attribute would silently invert a user-visible flag. The new CLI test asserts the bare form matches the no-flag default, and separately that capture is genuinely on -- equality alone would also hold if both runs captured nothing, which is exactly what the inverted flag produces. Removing fallbackValue fails it. --- .../com/ibm/cldk/neo4j/V2GraphProjector.java | 11 +++ .../com/ibm/cldk/CodeAnalyzerV2CliTest.java | 31 +++++++ .../neo4j/V2Neo4jSchemaConformanceTest.java | 84 ++++++++++++++++++- 3 files changed, 124 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java b/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java index 09c13fda..ec07023a 100644 --- a/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java +++ b/src/main/java/com/ibm/cldk/neo4j/V2GraphProjector.java @@ -479,6 +479,17 @@ private static void projectArtifacts(RowBuilder b, JApplication application, Nod // (DependencyView.build), so there is no per-lock-file attribution left to split on -- // a dependency locked with N lock artifacts present gets N LOCKS edges (matches // analysis.json; a known limitation carried over from codeanalyzer-python as-is). + // + // Deliberately WITHOUT the `seen` set the reference guards this mint with + // (codeanalyzer-python neo4j/project.py:350-359). LOCKS is a per-PACKAGE fact, so a + // package declared in two manifests walks this loop twice for one (lock, package) + // pair -- but python's RowBuilder.finish() only sorts its edge list, while ours + // dedupes by (type, from, to, _k) there, already collapsing the repeat mint. Same + // emitted row count either way, so a guard here would be dead code. Pinned by + // V2Neo4jSchemaConformanceTest#oneLocksRowSurvivesWhenTwoManifestsDeclareTheSameLockedCoordinate: + // dropping that dedupe fails a test instead of silently duplicating rows. + // DECLARES_DEPENDENCY above needs no such collapse and must not get one -- its + // source ref differs per declaring manifest, so one row per declaration is correct. if (dep.getLockedVersion() != null) { for (NodeRef lockRef : lockRefs) { Map lp = RowBuilder.props(); diff --git a/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java b/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java index 1515abdc..f31f6337 100644 --- a/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java +++ b/src/test/java/com/ibm/cldk/CodeAnalyzerV2CliTest.java @@ -651,6 +651,37 @@ void noArtifactTextEmptiesSourceAndValueButKeepsInventory(@TempDir Path tmp) thr assertTrue(sawCapturedValue, "precondition: the default run must actually capture some config value"); } + @Test + void bareArtifactTextFlagMeansTheDefaultRatherThanItsNegation(@TempDir Path tmp) throws IOException { + // Pins CodeAnalyzer's `fallbackValue = "true"` on --artifact-text. That fallback exists only + // because picocli 4.1.0 resolves a bare negatable flag (no explicit =value) to the OPPOSITE + // of what negatable=true implies; --artifact-text=true and --no-artifact-text are unaffected + // and were already covered, so nothing pinned the bare positive form and a "this looks + // redundant" cleanup of the fallback would silently invert a user-visible flag. + Path in = artifactProject(tmp.resolve("app")); + Path outDefault = tmp.resolve("out-default"); + Path outBare = tmp.resolve("out-bare"); + + // Default run first, while the reset in @BeforeEach still holds: were the fallback removed, + // the buggy bare run below could otherwise leave the static field false for a later default. + assertEquals(0, run("-i", in.toString(), "-o", outDefault.toString(), "--app-name", "widgets")); + assertEquals(0, run("-i", in.toString(), "-o", outBare.toString(), + "--app-name", "widgets", "--artifact-text")); + + JsonObject artifactsDefault = application(outDefault).getAsJsonObject("artifacts"); + JsonObject artifactsBare = application(outBare).getAsJsonObject("artifacts"); + assertEquals(artifactsDefault.toString(), artifactsBare.toString(), + "a bare --artifact-text must mean what the no-flag default means, not its negation"); + + boolean sawCapturedSource = false; + for (String path : artifactsBare.keySet()) { + sawCapturedSource |= !artifactsBare.getAsJsonObject(path).get("source").getAsString().isEmpty(); + } + // Equality alone would also hold if BOTH runs captured nothing, which is the exact failure + // an inverted flag produces -- so the bare run has to be shown capturing for real. + assertTrue(sawCapturedSource, "the bare flag must leave text capture ON"); + } + @Test void v2OmitsArtifactsAndDependenciesWhenNoNonSourceFilesExist(@TempDir Path tmp) throws IOException { Path in = project(tmp.resolve("app")); diff --git a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java index d3c88046..9201faa9 100644 --- a/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java +++ b/src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java @@ -280,18 +280,44 @@ void artifactLayerEdgesAreEmittedAndDeclaresDependencyIsKeyedByKind() { boolean locks = false; String declaresDependencyKey = null; String pkgId = CanId.purlMaven("org.example", "widget"); + // Every one of the four types asserts BOTH endpoints. Asserting only `to` is the same + // one-sided coverage that let a wrong decision ship on this layer once already (see + // wipeStaysOffTheCrossLanguageArtifactPackageSubgraph below, which exists because only the + // positive case had ever been asserted): a mis-sourced edge still lands on the expected + // target, so nothing except the source endpoint can catch it. + String appName = "l4-sdg-test"; + String pomId = CanId.artifactId(appName, "pom.xml"); + String lockId = CanId.artifactId(appName, "gradle.lockfile"); for (EdgeRow edge : rows.edges) { - hasArtifact |= edge.type.equals("HAS_ARTIFACT"); - definesConfig |= edge.type.equals("DEFINES_CONFIG"); + if (edge.type.equals("HAS_ARTIFACT")) { + hasArtifact = true; + assertEquals("JApplication", edge.from.label, "HAS_ARTIFACT runs application -> artifact"); + assertEquals(appName, edge.from.value, "the application is the source endpoint"); + assertEquals("Artifact", edge.to.label); + } + if (edge.type.equals("DEFINES_CONFIG")) { + definesConfig = true; + assertEquals("Artifact", edge.from.label, "DEFINES_CONFIG runs artifact -> config key"); + assertEquals("ConfigKey", edge.to.label); + assertTrue(edge.to.value.startsWith(edge.from.value + "@key"), + "a config key nests under the artifact that defines it: " + edge.to.value); + } if (edge.type.equals("DECLARES_DEPENDENCY") && edge.to.value.equals(pkgId)) { declaresDependency = true; declaresDependencyKey = edge.key; + assertEquals("Artifact", edge.from.label, "DECLARES_DEPENDENCY runs manifest -> package"); + assertEquals(pomId, edge.from.value, "the declaring manifest is the source endpoint"); + assertEquals("Package", edge.to.label); assertEquals("1.0.0", edge.props.get("spec")); assertEquals("runtime", edge.props.get("kind")); assertEquals(Boolean.TRUE, edge.props.get("direct")); } if (edge.type.equals("LOCKS") && edge.to.value.equals(pkgId)) { locks = true; + assertEquals("Artifact", edge.from.label, "LOCKS runs lock artifact -> package"); + assertEquals(lockId, edge.from.value, + "the lock file is the source endpoint -- the package does not lock the lockfile"); + assertEquals("Package", edge.to.label); assertEquals("1.0.0", edge.props.get("version")); assertNull(edge.key, "LOCKS carries no _k discriminant"); } @@ -304,6 +330,60 @@ void artifactLayerEdgesAreEmittedAndDeclaresDependencyIsKeyedByKind() { "DECLARES_DEPENDENCY must carry the _k=kind MERGE discriminant"); } + @Test + void oneLocksRowSurvivesWhenTwoManifestsDeclareTheSameLockedCoordinate(@TempDir Path tmp) + throws Exception { + // Two build.gradle files of one multi-module build declaring the same coordinate, with a + // single gradle.lockfile pinning it. `dependencies` then carries that coordinate twice, + // both copies with a lockedVersion, so projectArtifacts walks the LOCKS mint twice for one + // (lock artifact, package) pair -- LOCKS is a per-PACKAGE fact, unlike DECLARES_DEPENDENCY, + // whose source ref differs per declaring manifest and which is correctly one row per + // declaration. GraphRows promises "a deterministic, deduped bag"; this pins that promise at + // the observable boundary, whichever layer keeps it. + Files.writeString(tmp.resolve("build.gradle"), + "dependencies { implementation 'org.example:widget:1.0.0' }\n", StandardCharsets.UTF_8); + Files.createDirectories(tmp.resolve("mod")); + Files.writeString(tmp.resolve("mod/build.gradle"), + "dependencies { implementation 'org.example:widget:1.0.0' }\n", StandardCharsets.UTF_8); + Files.writeString(tmp.resolve("gradle.lockfile"), + "org.example:widget:1.0.0=compileClasspath,runtimeClasspath\n", StandardCharsets.UTF_8); + + Map artifacts = ArtifactDiscovery.discover(tmp, "dup", true, 262144); + List dependencies = DependencyView.build(tmp, artifacts); + String pkgId = CanId.purlMaven("org.example", "widget"); + + int lockedDeclarations = 0; + for (JDependency d : dependencies) { + if ("widget".equals(d.getName()) && d.getLockedVersion() != null) { + lockedDeclarations++; + } + } + assertEquals(2, lockedDeclarations, + "precondition: both manifests must declare the coordinate and both copies must be locked"); + + GraphRows dupRows = V2GraphProjector.project( + V2Emitter.emit("dup", 1, new LinkedHashMap<>(), "test", null, null, null, null, + artifacts, dependencies), + "dup"); + + int locks = 0; + int declares = 0; + for (EdgeRow edge : dupRows.edges) { + if (edge.to.value.equals(pkgId)) { + if (edge.type.equals("LOCKS")) { + locks++; + assertEquals("1.0.0", edge.props.get("version")); + } else if (edge.type.equals("DECLARES_DEPENDENCY")) { + declares++; + } + } + } + assertEquals(1, locks, "one lock artifact pinning one package is exactly one LOCKS row, " + + "however many manifests declared that package"); + assertEquals(2, declares, "DECLARES_DEPENDENCY stays one row per declaring manifest -- " + + "its source ref differs per manifest, so it must NOT be deduped per package"); + } + @Test void wipeStaysOffTheCrossLanguageArtifactPackageSubgraph() { // The negative counterpart to wipeCoversBothGenerationsSoV2ReplacesAPriorV1Graph above, From 699284ca2703cf12f9489ba878bae331d0e88ba3 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 31 Aug 2026 07:54:41 -0400 Subject: [PATCH 19/19] docs(artifacts): fix textMaxBytes javadoc for dependency-manifest exemption --- src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java b/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java index a603d1fa..ae7178db 100644 --- a/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java +++ b/src/main/java/com/ibm/cldk/artifacts/ArtifactDiscovery.java @@ -119,8 +119,8 @@ private static final class Rule { * either way, only the captured text differs * @param textMaxBytes byte cap on captured text for a decodable file; a {@code * dependency-manifest} is exempt (always captured whole when {@code captureText} is on) - * because its {@code source} is what dependency extraction parses, not bulk content the - * cap exists to bound + * so the complete manifest text appears in the emitted JSON payload for consumers, not + * truncated by the cap */ public static Map discover( Path projectDir, String appName, boolean captureText, int textMaxBytes) throws IOException {