Skip to content

Commit 4a2bb5c

Browse files
committed
fix: fail closed on incomplete CLI imports
1 parent 2a4a50a commit 4a2bb5c

8 files changed

Lines changed: 245 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ uses semantic versioning for public releases.
55

66
## [Unreleased]
77

8+
### Fixed
9+
10+
- accept documented dotted CLI rule IDs while still rejecting unknown rule fields;
11+
- retain input and parser failures in CLI/direct `CliAnalyzer` results so corrupt archives or
12+
malformed classes cannot silently produce passing partial checks; and
13+
- reject incomplete graph imports by default before writing graph output.
14+
815
### Changed
916

1017
- align the project license with the ArchUnitEverything family by adopting the MIT License.

‎docs/CLI_REFERENCE.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ archunitjava <check|graph|explain|validate-config>
2525
Command-line format flags override the corresponding configuration value. Other unknown options
2626
are rejected.
2727

28+
Both `check` and `graph` reject incomplete imports by default, including corrupt JARs,
29+
malformed class files, and exceeded input limits. `check` retains import diagnostics in its
30+
results and returns exit code `4`; `graph` returns `4` without emitting a partial graph.
31+
Set `allowIncompleteAnalysis=true` only when partial results are acceptable. In that mode,
32+
`check` retains the import failures as warnings. A graph command does not enforce the configured
33+
architecture policies: violations alone do not prevent graph export.
34+
2835
## Root keys
2936

3037
| Key | Required | Values and default |

‎docs/MAINTAINER_HANDOVER.md‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# Maintainer handover
2+
3+
Review date: 2026-09-14. Proposed recipient: [Lukas Niessen](https://github.com/LukasNiessen).
4+
This document prepares a transfer; it does not record a completed ownership change.
5+
6+
## Before transferring
7+
8+
1. Resolve the destination name. `LukasNiessen/ArchUnitJava` already exists as a separate,
9+
non-fork repository with `README.md`, `AGENTS.md`, and `CLA.md`. Its owner should preserve any
10+
relevant guidance and rename it before accepting this repository under that name. Transferring
11+
under a different unused name is another option. Do not overwrite or delete the existing repo.
12+
2. Decide whether the independent `ArchUnitJava-TestRepo-RAG` should move too. The library's CI
13+
checks it out explicitly, and its standalone CI tests the published Maven artifact.
14+
3. Arrange Maven Central publishing access separately. The existing artifact is
15+
`io.github.tristankruse:archunitjava:0.1.0`; a GitHub transfer does not change that coordinate or
16+
automatically grant namespace access. Prefer retaining the coordinates for consumer continuity.
17+
Use Central Portal organization membership and namespace permissions, or Central Support if the
18+
necessary organization controls are unavailable. Lukas should use his own publisher account.
19+
4. Review release credentials and approval. The `maven-central` environment currently requires
20+
TristanKruse approval and contains the five secret names listed in [RELEASE.md](RELEASE.md).
21+
GitHub keeps secrets associated with a transferred repository. Agree on replacing the Central
22+
token, signing-key custody, and the release approver before running any release under new
23+
ownership. Never copy secret values into repository files or handover notes.
24+
5. Verify analytics access. Publisher Insights was enabled for `io.github.tristankruse`. Access to
25+
the correct Maven package in Scarf and its public badge still needs independent verification.
26+
The README currently links to the analytics sources but contains no live download-count badge.
27+
Repository transfer alone does not transfer external analytics accounts.
28+
29+
## Repository quality and release follow-up
30+
31+
- Dependabot PRs #76 and #77 were reviewed and merged: Maven Surefire 3.6.0 and Compiler 3.16.0.
32+
- The focused CLI audit reproduced and corrected dotted rule-ID rejection and successful partial
33+
analysis after corrupt input. Regression tests cover strict checks, all six graph renderers,
34+
explicit partial-analysis opt-in, retained violations, and harmless input duplication.
35+
- These fixes are on the development line. Maven Central `0.1.0` remains immutable and does not
36+
contain them. Plan a subsequent beta release after choosing publisher access and ownership.
37+
- The current source uses MIT; the already published `0.1.0` artifact retains its original Apache
38+
2.0 metadata. Keep that distinction explicit until a subsequent release is published.
39+
- CodeQL is active. At the review date, Dependabot security alerts/security updates and secret
40+
scanning/push protection were disabled. Enable the appropriate GitHub security features and
41+
review any resulting alerts. Scheduled dependency-version PRs are already enabled.
42+
- No repository rulesets were present at the review date. A protected default branch is an
43+
optional next step when more maintainers contribute; it should fit the team's merge workflow.
44+
- The public API remains provisional and JDK 25 remains required. A repository transfer is not
45+
a reason to claim 1.0 stability or change the runtime requirement.
46+
47+
## Transfer choices
48+
49+
The recommended option for the current family layout is GitHub's native transfer to
50+
`LukasNiessen`, after resolving the name conflict. It preserves history, issues, PRs, stars, and
51+
release records. Lukas must accept the personal-account transfer invitation within one day.
52+
The original owner remains a collaborator. No transfer invitation has been sent during this review.
53+
54+
A shared GitHub organization is an alternative if the whole ArchUnitEverything family should have
55+
multiple administrators. It requires a separately agreed organization and repository-creation
56+
permission. Copying code into the existing repository would not preserve this repository's issue,
57+
PR, and release continuity in the same way.
58+
59+
## After acceptance
60+
61+
- Update local Git remotes, README badges/links, the repository homepage, POM project/SCM metadata,
62+
issue-template links, support links, and the documentation site's social-preview URL.
63+
- Deploy and verify Pages at the recipient's site URL. Repository URLs redirect after transfer;
64+
GitHub Pages URLs do not. Decide whether to keep a separate redirect site at the old Pages URL.
65+
- Update the RAG repository links and the explicit checkout in `.github/workflows/ci.yml` if the
66+
fixture also moves. Test both the development candidate and the published artifact.
67+
- Review `.github/workflows/prepare-release.yml`, which currently sets Tristan's tagger identity,
68+
and the release environment's approver. Preserve attribution on existing commits and tags.
69+
- Confirm CI, documentation deployment, CodeQL, Central permissions, signing, and analytics from
70+
the new maintainer's account before the next release. Use a staging/dry-run first.
71+
72+
References: [GitHub repository transfers](https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository),
73+
[Central Portal organizations and namespace permissions](https://central.sonatype.org/publish/publish-portal-organizations/),
74+
and [Scarf's Maven integration](https://docs.scarf.sh/package-registry-integrations-maven-central/).

‎docs/RELEASE.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ replaced or deleted.
106106

107107
## Local review commands
108108

109+
For ownership changes, see the [maintainer handover](MAINTAINER_HANDOVER.md) for repository,
110+
publishing, signing, documentation, and analytics dependencies.
111+
109112
```shell
110113
./mvnw --batch-mode --no-transfer-progress verify
111114
./mvnw --batch-mode --no-transfer-progress -Prelease-candidate verify

‎src/main/java/dev/archunitjava/cli/CliAnalyzer.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,17 @@
22

33
import dev.archunitjava.importer.ClassFileInput;
44
import dev.archunitjava.importer.ClassPathImportResolver;
5+
import dev.archunitjava.importer.ImportResolutionResult;
6+
import dev.archunitjava.importer.InputDiagnosticCode;
57
import dev.archunitjava.report.ResultReport;
8+
import dev.archunitjava.result.Diagnostic;
9+
import dev.archunitjava.result.RuleResult;
10+
import dev.archunitjava.result.Severity;
11+
import java.util.ArrayList;
12+
import java.util.List;
13+
import java.util.Map;
614
import java.util.Objects;
15+
import java.util.TreeMap;
716

817
/** Direct Java API for the same bounded import and rule evaluation performed by the CLI. */
918
public final class CliAnalyzer {
@@ -12,9 +21,63 @@ public CliAnalysisResult analyze(CliConfiguration configuration) {
1221
var imports = new ClassPathImportResolver().resolve(config.inputs().stream()
1322
.map(ClassFileInput::path).toList());
1423
var graph = CliGraphBuilder.build(imports.model());
24+
var failures = importFailures(imports);
1525
var results = CliRuleFactory.createRules(config, imports.model(), graph).stream()
1626
.map(rule -> rule.check(config.checkOptions()))
27+
.map(result -> withImportDiagnostics(result, failures,
28+
config.checkOptions().allowIncompleteAnalysis()))
1729
.toList();
1830
return new CliAnalysisResult(imports, graph, ResultReport.of(results));
1931
}
32+
33+
static List<Diagnostic> importFailures(ImportResolutionResult imports) {
34+
List<Diagnostic> failures = new ArrayList<>();
35+
imports.assembly().diagnostics().stream()
36+
.filter(value -> incompleteInput(value.code()))
37+
.forEach(value -> failures.add(importDiagnostic(
38+
"cli.input." + value.code().name(), value.input(), value.context())));
39+
imports.model().classFileDiagnostics().forEach(value -> failures.add(importDiagnostic(
40+
"cli.classfile." + value.code().name(), value.resourceName(), value.context())));
41+
imports.model().diagnostics().forEach(value -> failures.add(importDiagnostic(
42+
"cli.model." + value.code().name(), value.resourceName(), value.context())));
43+
return failures.stream().distinct().sorted().toList();
44+
}
45+
46+
private static boolean incompleteInput(InputDiagnosticCode code) {
47+
return switch (code) {
48+
case ARCHIVE_RESOURCE_REJECTED, DIAGNOSTIC_LIMIT_REACHED, INVALID_IGNORE_RULE,
49+
INVALID_RESOURCE_NAME, IO_FAILURE, MISSING_INPUT, RESOURCE_LIMIT_EXCEEDED,
50+
UNREADABLE_INPUT, UNSUPPORTED_INPUT -> true;
51+
// These record the documented classpath selection and exclusion policy.
52+
case DUPLICATE_INPUT, DUPLICATE_RESOURCE, MANIFEST_CLASS_PATH_REJECTED,
53+
MULTI_RELEASE_ENTRY_IGNORED, NESTED_ARCHIVE_REJECTED, RESOURCE_EXCLUDED,
54+
SYMLINK_SKIPPED -> false;
55+
};
56+
}
57+
58+
private static Diagnostic importDiagnostic(
59+
String code, String resource, Map<String, String> context) {
60+
Map<String, String> details = new TreeMap<>(context);
61+
details.put("resource", resource);
62+
return new Diagnostic(code, Severity.ERROR, details);
63+
}
64+
65+
private static RuleResult withImportDiagnostics(
66+
RuleResult result, List<Diagnostic> failures, boolean allowIncomplete) {
67+
if (failures.isEmpty()) return result;
68+
List<Diagnostic> diagnostics = new ArrayList<>(result.diagnostics());
69+
for (Diagnostic failure : failures) {
70+
diagnostics.add(allowIncomplete
71+
? new Diagnostic(failure.code(), Severity.WARNING, failure.context()) : failure);
72+
}
73+
if (!allowIncomplete) {
74+
return RuleResult.incomplete(result.metadata(), result.violations(), diagnostics);
75+
}
76+
return switch (result.status()) {
77+
case PASSED -> RuleResult.passed(result.metadata(), diagnostics);
78+
case FAILED -> RuleResult.failed(result.metadata(), result.violations(), diagnostics);
79+
case SKIPPED -> RuleResult.skipped(result.metadata(), diagnostics);
80+
case INCOMPLETE -> RuleResult.incomplete(result.metadata(), result.violations(), diagnostics);
81+
};
82+
}
2083
}

‎src/main/java/dev/archunitjava/cli/CliConfigurationLoader.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ private static void validateKeys(Set<String> keys, Set<String> ruleIds) {
168168
throw new CliConfigurationException("Unknown configuration key: " + key);
169169
}
170170
String remainder = key.substring("rule.".length());
171-
int separator = remainder.indexOf('.');
171+
int separator = remainder.lastIndexOf('.');
172172
if (separator < 1 || !ruleIds.contains(remainder.substring(0, separator))
173173
|| !RULE_FIELDS.contains(remainder.substring(separator + 1))) {
174174
throw new CliConfigurationException("Unknown rule configuration key: " + key);

‎src/main/java/dev/archunitjava/cli/CliRunner.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,11 @@ private static int check(CliConfiguration configuration, Appendable out) {
107107

108108
private static int graph(CliConfiguration configuration, Appendable out) {
109109
CliAnalysisResult analysis = new CliAnalyzer().analyze(configuration);
110+
if (!configuration.checkOptions().allowIncompleteAnalysis()
111+
&& !CliAnalyzer.importFailures(analysis.imports()).isEmpty()) {
112+
throw new IllegalStateException("Graph import is incomplete; repair the inputs or "
113+
+ "explicitly set allowIncompleteAnalysis=true to render a partial graph");
114+
}
110115
GraphSnapshot snapshot = switch (configuration.graphDomain()) {
111116
case TYPES -> GraphSnapshotQuery.types(analysis.graph()).snapshot();
112117
case PACKAGES -> GraphSnapshotQuery.packages(analysis.graph()).snapshot();

‎src/test/java/dev/archunitjava/cli/CliRunnerTest.java‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,91 @@ void checkUsesTheSameResultsAsThePublicJavaApi() {
6767
assertTrue(error.isEmpty());
6868
}
6969

70+
@Test
71+
void dottedRuleIdsAreAcceptedAndUnknownFieldsAreRejected() throws IOException {
72+
String base = Files.readString(configuration);
73+
String dotted = base.replace("boundary", "api.boundary");
74+
configuration = writeConfiguration(dotted);
75+
assertEquals(CliExitCode.SUCCESS.code(), run("validate-config").exit());
76+
assertEquals(CliExitCode.POLICY_VIOLATION.code(), run("check").exit());
77+
78+
configuration = writeConfiguration(dotted + "rule.api.boundary.unknown=true\n");
79+
assertEquals(CliExitCode.INVALID_CONFIGURATION.code(), run("validate-config").exit());
80+
81+
configuration = writeConfiguration(dotted + "rule.api.other.domain=types\n");
82+
assertEquals(CliExitCode.INVALID_CONFIGURATION.code(), run("validate-config").exit());
83+
}
84+
85+
@Test
86+
void graphRejectsIncompleteBytecodeUnlessExplicitlyAllowed() throws IOException {
87+
Files.write(classes.resolve("Broken.class"), new byte[] {0, 1, 2, 3});
88+
for (String format : new String[] {"dot", "mermaid", "json", "csv", "d2", "html"}) {
89+
Invocation graph = run("graph", "--graph-format", format);
90+
assertEquals(CliExitCode.ANALYSIS_ERROR.code(), graph.exit(), format);
91+
assertTrue(graph.out().isEmpty(), format);
92+
assertTrue(graph.error().contains("incomplete"), format);
93+
}
94+
configuration = writeConfiguration(Files.readString(configuration)
95+
.replace("allowIncompleteAnalysis=false", "allowIncompleteAnalysis=true"));
96+
Invocation allowed = run("graph", "--graph-format", "json");
97+
assertEquals(CliExitCode.SUCCESS.code(), allowed.exit());
98+
assertTrue(allowed.out().contains("api.A"));
99+
}
100+
101+
@Test
102+
void corruptInputArchiveCannotProduceAPassingPartialAnalysis() throws IOException {
103+
Files.write(root.resolve("broken.jar"), new byte[] {0, 1, 2, 3});
104+
configuration = writeConfiguration(Files.readString(configuration)
105+
.replace("inputs=classes", "inputs=classes,broken.jar")
106+
.replace("origins=exact:api.A", "origins=exact:internal.B"));
107+
Invocation check = run("check", "--result-format", "json");
108+
assertEquals(CliExitCode.ANALYSIS_ERROR.code(), check.exit());
109+
assertTrue(check.out().contains("INCOMPLETE"));
110+
assertEquals(CliExitCode.ANALYSIS_ERROR.code(), run("graph").exit());
111+
assertEquals(dev.archunitjava.result.RuleStatus.INCOMPLETE,
112+
new CliAnalyzer().analyze(CliConfigurationLoader.load(configuration, root))
113+
.results().results().getFirst().status());
114+
configuration = writeConfiguration(Files.readString(configuration)
115+
.replace("allowIncompleteAnalysis=false", "allowIncompleteAnalysis=true"));
116+
Invocation allowed = run("check", "--result-format", "json");
117+
assertEquals(CliExitCode.SUCCESS.code(), allowed.exit());
118+
assertTrue(allowed.out().contains("cli.input.IO_FAILURE"));
119+
assertTrue(allowed.out().contains("WARNING"));
120+
}
121+
122+
@Test
123+
void malformedClassCannotProduceAPassingPartialAnalysis() throws IOException {
124+
Files.write(classes.resolve("Broken.class"), new byte[] {0, 1, 2, 3});
125+
configuration = writeConfiguration(Files.readString(configuration)
126+
.replace("origins=exact:api.A", "origins=exact:internal.B"));
127+
Invocation check = run("check", "--result-format", "json");
128+
assertEquals(CliExitCode.ANALYSIS_ERROR.code(), check.exit());
129+
assertTrue(check.out().contains("cli.classfile."));
130+
assertTrue(check.out().contains("INCOMPLETE"));
131+
}
132+
133+
@Test
134+
void incompleteChecksRetainObservedViolations() throws IOException {
135+
Files.write(classes.resolve("Broken.class"), new byte[] {0, 1, 2, 3});
136+
var result = new CliAnalyzer().analyze(CliConfigurationLoader.load(configuration, root))
137+
.results().results().getFirst();
138+
assertEquals(dev.archunitjava.result.RuleStatus.INCOMPLETE, result.status());
139+
assertFalse(result.violations().isEmpty());
140+
assertTrue(result.diagnostics().stream()
141+
.anyMatch(value -> value.code().startsWith("cli.classfile.")));
142+
}
143+
144+
@Test
145+
void graphIgnoresRuleSelectionFailuresAndHarmlessDuplicateInputs() throws IOException {
146+
configuration = writeConfiguration(Files.readString(configuration)
147+
.replace("inputs=classes", "inputs=classes,./classes")
148+
.replace("targets=exact:internal.B", "targets=exact:absent.C"));
149+
Invocation graph = run("graph", "--graph-format", "json");
150+
assertEquals(CliExitCode.SUCCESS.code(), graph.exit());
151+
assertTrue(graph.out().contains("api.A"));
152+
assertTrue(graph.error().isEmpty());
153+
}
154+
70155
@Test
71156
void graphExplainAndValidationCommandsAreStableAndBounded() {
72157
Invocation validation = run("validate-config");

0 commit comments

Comments
 (0)