Codebase evaluation

Codebase evaluation

Date: 2026-09-10 · Revision: develop @ 4668bdeaf · Method: six parallel review passes (Java, XSLT, CSS/frontend assets, testing, build & infrastructure, security), each reporting file-level evidence; scores synthesized from the combined findings.

Scale: 5 = average professional codebase, 8+ = top decile.

Scorecard

Category	Score
XSLT codebase	8
Release engineering & versioning	8
Architecture & design	7.5
Documentation & comments	7.5
CSS authoring (LDH layer)	7
Java implementation	6
Testing	5
Build system & deployment	5
Security	4.5
Frontend assets & performance	3

Overall: ~6/10 — a codebase with top-decile discipline in the areas of recent investment (XSLT layering, i18n, release process, comment quality) sitting on a foundation with real, specific defects: a handful of concurrency bugs on the request path, a TLS client configuration that undermines the auth stack, an untested UI layer, and a 12+ MB dead-weight asset payload.

XSLT — 8

The strongest engineering in the repo, and the recent restructuring claims verify mechanically:

• The layout.xsl (SSR) closure contains zero ixsl tokens across all 19 modules; the trunk/masters import tree is real (17 shared / 2 SSR-only / 41 CSR-only modules).
• Mode namespacing is 100% (677 ac:, 617 ldh:, zero unprefixed modes across 214 distinct mode names).
• i18n is near-perfect: 398 keys × 2 locales in translations.rdf with 100% pair coverage and only 2 hardcoded UI strings in 36k lines (client/modal.xsl:356, :2058).
• Max conditional nesting anywhere is 3; 97% of xsl:param declarations carry as= type annotations; 208 named templates with only 3 never called.

What keeps it from 9:

• client/block/view.xsl is 3,480 lines largely because one render-pipeline block (cache read → busy cursor → ldh:RenderView with 7 params → promise) is copy-pasted ~11 times (lines 1702, 1744, 1787, 1906, 1954, 2019, 2061, 2361, 2417, 2501, 2623, 3459).
• client/3d-force-graph.xsl embeds 155 lines of JavaScript as XML attribute strings fed to ixsl:eval() — unlintable, unhighlightable, escaping-hazardous. js/functions.js already exists as the extraction target.
• ~225 lines of dead mode="ldh:logo" templates in resource.xsl:99-320 (35 templates, zero applications anywhere) — orphaned by the block-system refactor and never swept.
• The ldt:base/sd:endpoint duals live in imports/default.xsl:159,163 outside the documented server.xsl ↔ client/functions.xsl mirror, resolved by undocumented include-vs-import precedence; server.xsl:20-21's "mirror each other name for name" claim is false for these two.
• ~95 non-fatal xsl:message calls ship to production consoles, including 7 unconditional startup dumps in client.xsl:244-251.
• No XSLT test framework of any kind; every invariant above is maintained by discipline and compile-time checks alone.

Release engineering & versioning — 8

• release.sh (279 lines) encodes hard-won operational knowledge: pre-flight branch-switch assertions before the irreversible Maven Central publish (assert_can_switch_to, written after a release once died on git checkout master post-publish), refuse-rollback-after-publish with printed manual recovery steps, PREPARE_BASE anchoring.
• 138 consistently-named linkeddatahub-* tags; CHANGELOG.md is a genuinely maintained Keep-a-Changelog (77 sections, dated entries, a ### Migration block on the 6.0.0 major).

Deductions:

• cli/pom.xml has already drifted to 5.10.1-SNAPSHOT against the platform's 6.0.0-SNAPSHOT, despite make cli-version existing — nothing enforces alignment.
• release.yml triggers on any tag (tags: '*') and publishes atomgraph/linkeddatahub:latest to Docker Hub with zero test dependency.
• Two SNAPSHOT dependencies in the release-bound pom (twirl:2.0.1-SNAPSHOT, client:6.0.0-SNAPSHOT) make tagged releases non-reproducible by construction.

Architecture & design — 7.5

• The data-driven single-dispatcher design (Dispatcher.java routing everything to DocumentHierarchyGraphStoreImpl) is the right shape for a Linked Data server where every URI is a document.
• The JAX-RS filter pipeline is explicitly ordered with @Priority and per-filter rationale comments; HK2 constructor injection is used correctly with 7 request-scoped factories; package cohesion is high (70 packages, median 3 files).

The cost side:

• Application.java is 2,609 lines (8.8% of the codebase) with 53 fields, a 51-parameter constructor, and 163 getSystem() call sites across 37 classes making it a de-facto service locator — directly why 189 of 215 classes have no unit test. Obvious extraction seams: client factory (~330 lines), app matcher, context-dataset repository, refresh-token store.
• Three distinct types named Application (plus Service ×2, GraphStoreClient ×2) force 200+ fully-qualified type references into method bodies.
• ServiceContext.getGraphStoreClient() returns the base Core client, so the 295-line LDH GraphStoreClient subclass (retry, Retry-After, WebID/OIDC delegation) is silently bypassed on the primary document read/write path (DocumentHierarchyGraphStoreImpl.java:475,501).
• A vendored fork of AtomGraph Processor (25 files, 1,630 LOC under com.atomgraph.server) lives in the source tree, neither upstreamed nor absorbed.

Documentation & comments — 7.5

• Javadoc coverage is library-grade: 1,628 blocks, 917 @param, @author on 210/215 files — though nothing (doclint) enforces it.
• The Varnish VCL security comments are load-bearing and correct (why On-Behalf-Of must bypass cache, why xkey.purge over softpurge, naming the dependent classes).
• ldh.css is 51% decision-rationale comments; XSLT comment density tracks complexity (highest exactly in the biggest files).
• The README's browser/cert setup walkthrough is above the median for self-hosted software, but has drifted: the data mount no longer exists (it's fuseki/), the healthcheck claim ("every 20s") is wrong (30s interval, ~230s worst case), two conflicting server-cert-gen.sh invocations are documented, and 12 compose variables never made it into .env_sample.

CSS authoring — 7

ldh.css itself is close to exemplary:

• A stated layering contract in its header; ~40 section banners; zero !important (3 in the entire 8.8k-line CSS tree); 228 var() uses vs 11 hex literals; documented divergences from the design system with rationale.
• Vendored kits currently measure pristine: 0 changed lines across 5 kit files vs the design-system repo, with 2 documented, necessary deltas — though history shows 17+ local-edit commits to app.css before the invariant was enforced, and nothing mechanical protects it.

Deductions:

• No z-index scale anywhere — 40+ magic numbers across six files with live collisions (#rdfa-editor-overlay and #output-modal both at 10001; ldh.css:220-221 states a stacking contract the same file breaks 100 lines later).
• Two competing chrome-height token families (--ldh-header-h in app.css:33 vs --ldh-header-height in ldh.css:214), both live, agreeing by coincidence.
• The five-colour SPARQL syntax palette is written out three times in three class vocabularies (controls.css:313, app.css:3255 — dead, ldh.css:426), zero tokens.
• rdfa-editor.css is un-migrated: 601 lines, 58 raw hex, zero var() uses.
• Zero media queries in the LDH layer — the fixed navbar, sticky bars, drawer, and 1280px content column have no small-viewport handling.

Java implementation — 6

The craft is unusually disciplined: zero printStackTrace, zero catch (Throwable), only 6 catch (Exception) in 92 catch clauses, near-total field finality, correct multi-catch and InterruptedException handling, 48 lines of commented-out code in 30k, effectively zero unused imports.

But the review found real defects, all machine-detectable:

• Context-dataset read/write race — Application.updateApp (Application.java:2296) mutates the config dataset under synchronized, while every reader (matchApp on every request via ApplicationFilter, XSLTWriterBase.java:123 on every HTML response) is unsynchronized. Jena's in-memory dataset is unsafe for concurrent read/write; a PATCH /settings concurrent with any in-flight request can throw ConcurrentModificationException or return a torn model. The synchronized block actively disguises the gap. Also: updateApp truncate-writes the live config file in place (no temp-and-rename), and never rebuilds serviceContextMap/graphVersioningService derived from the dataset.
• Two containsKey-then-get TOCTOUs on expiring caches in the authentication path (WebIDFilter.java:262, IDTokenFilterBase.java:206) — a TTL expiry between the calls returns null and produces an intermittent 500 on login.
• EMailListener's dedicated thread pool is never used (EMailListener.java:41,65) — CompletableFuture.runAsync without the executor argument puts blocking SMTP I/O on the common ForkJoinPool; the pool is created and shut down but never submitted to.
• A null check that runs after the dereference it guards (DocumentHierarchyGraphStoreImpl.java:204-206: service.get() before if (service.isEmpty())).
• A provably dead branch (ValidatingModelProvider.java:220-226: Resource type = null; if (type != null) …).
• A production invariant expressed as assert (AuthorizationFilter.java:204) — disabled in default JVMs.
• Non-atomic check-then-put on the XSLT executable cache (XsltExecutableFilter.java:144,341) — thundering-herd recompiles on cold start; computeIfAbsent fixes it.
• storeRefreshToken (Application.java:2600) — concurrent OAuth callbacks interleave truncating writes to the same properties file.

Java 21 is the compile target but the code reads as Java 8: no var, no text blocks, one switch, 4 records, 242 redundant isDebugEnabled() guards around parameterized SLF4J calls. ~500 lines of mechanical duplication concentrate in three places (three near-identical HTTP client builders in Application.java, the ImportExecutor CSV/RDF fork, four copy-pasted literal-conversion blocks in ValidatingModelProvider). No static analysis in the pom — no SpotBugs, ErrorProne, PMD, or JaCoCo; every bug above is catchable in one run.

Testing — 5

The HTTP suite is legitimately good:

• 176 executing scripts (~9.6k LOC bash), near-universal set -euo pipefail, disciplined fixture reset (initialize_dataset in 183/196) plus three-tier Varnish purge (190/196), fixtures built via the ldh CLI.
• The 73-script document-hierarchy matrix covers CRUD × auth × conditional-request combinations (ETag pairs, 403/404/405 variants, content negotiation, skolemization edge cases, SPIN 422 body shapes) more thoroughly than most projects of this size.
• The CLI module is disproportionately the healthiest-tested: 84 @Test over an in-process StubServer driving the real Jersey/PKCS12 stack, with RDF graph-isomorphism assertions.

Everything else drags the score down:

• 11.6% unit-test class coverage (25 test classes / 215 main). Zero unit tests for the entire request-authentication path (WebIDFilter, AuthenticationFilter, all 5 security contexts), all 18 exception mappers, all JAX-RS resources including SignUp, and the whole CSV/RDF import pipeline. Ten pure utilities (Skolemizer, PatchUpdateVisitor, LanguageNegotiator, SecureXML…) need no mocking and have none.
• http-tests/system/ holds 19 committed, actively-maintained auth-boundary tests that run.sh has never invoked — maintained through the CLI migration without anyone noticing they never run.
• The versioning suite reports 5/5 green on forks while skipping everything (exit 0 on missing GITHUB_TOKEN counts as passed; the CTRF summary hard-codes "skipped": 0).
• The CLAUDE.md DEBUG: Expected/Got convention is followed by 3 of 196 scripts; the last local run artifact shows six failures with completely empty diagnostic messages.
• The XSLT/UI layer — 64 stylesheets, half the product, and the subject of most recent commits — has zero checked-in automated tests. No XSpec, no Playwright project, no package.json. Commit messages citing "Playwright batteries" refer to ad-hoc agent-driven browser sessions that exist nowhere in the repository; the history reads as if a standing regression suite exists when it does not. The canonical "HTML rendering works" test (document-hierarchy/GET-html.sh) asserts only the Content-Type header and never looks at the body.
• CI gaps: on: push only (fork PRs run nothing, no required PR check), the platform's 181 unit tests run only as a side effect of the Dockerfile's mvn install (a failure reads as an image build error, no surefire report), and the release workflow publishes with no test gate.

Build system & deployment — 5

• Plugin pinning is disciplined and the pom's exclusion comments preserve real institutional knowledge — but there is no dependencyManagement, no version properties, and Jena 6.1.0 is hardcoded in 6 places across 4 files.
• The WAR overlay silently ships four artifacts at two versions each (jersey-client, jersey-common, slf4j-api, error_prone_annotations). Tomcat's sorted classloading means the platform actually runs Jersey 3.1.11 core against 3.1.12 media modules — the inverse of what the pom declares — and the documented jakarta.json exclusion is defeated by the same path (jakarta.json-2.0.1.jar arrives via the overlay anyway).
• platform/entrypoint.sh is 1,267 lines of untested, root-executed bash: three evals over secret-bearing strings (:93, :1229, :1243 — a password containing ' breaks quoting and executes), a word-splitting bug in dataspace provisioning (app_array=(${app}) at :751 silently mis-parses any SPARQL binding containing a space), and 122 lines of copy-pasted env checks.
• 71% of docker-compose.yml (435 of 608 lines) is heredoc nginx/VCL config that no tool can validate (nginx -t, varnishd -C, linters all unavailable); the three VCL preambles are ~35 lines triplicated, and the two nginx server blocks are ~90% duplicated.
• The Dockerfile copies src before pom.xml and has no dependency-warming or ~/.m2 cache mount — every image build re-resolves the full dependency graph and re-downloads Node; images are 1.27 GB. The healthcheck hardcodes port 7070 while HTTP_PORT is configurable. Makefile still invokes EOL docker-compose v1 while CI uses v2; make sef's entity-inlining uses a different implementation (xmlstarlet c14n) than the pom (net.sf.saxon.Query), so a locally-built SEF is not necessarily the image's SEF.
• .dockerignore misses secrets/ (real cert passwords enter the build context on every local make up --build), cli/target, and lists a data dir that no longer exists.

Security — 4.5

The 2025 pen-test remediations verify as real, and the strong parts are genuinely strong:

• SPARQL injection is designed out: 53 ParameterizedSparqlString sites, user queries arriving pre-parsed as Query/UpdateRequest objects, exactly one string-concatenated query in the codebase (upload/Item.java:319, server-shaped input).
• XXE is centrally hardened in SecureXML and consistently applied to both consumers.
• Uploads defeat path traversal by construction (server-computed SHA-1 filenames) with a correct default-src 'none'; sandbox CSP on served files (LNK-011).
• JWT verification is algorithm-pinned (Algorithm.RSA256 from JWKS), issuer-allowlisted before fetch, and refresh-then-verify ordered correctly. WebID key matching is parameterized and identity-anchored; delegation requires an explicit acl:delegates triple.
• The proxy's origin split (ApplicationFilter setting AC.uri as the authoritative marker), HTML-Accept refusal, and bidirectional header allowlists are carefully reasoned.

The residual findings outside the LNK-* scope are serious:

1. Hostname verification is disabled on every HTTP client against a public-CA truststore. Application.java:1531,1639 apply NoopHostnameVerifier with a comment asserting the truststore is pinned — but entrypoint.sh:999-1008 imports the entire JDK cacerts bundle into it, and CHANGELOG.md:199 states the contradiction outright. The OAuth token exchange (carrying client_secret), the JWKS fetch that anchors all ID-token trust, ORCID userinfo, and all proxy egress accept any valid public-CA certificate for any hostname. The correct client (getVerifiedClient(), Application.java:1752) exists, is constructed, and has zero call sites. Highest-impact finding.
2. The import path has no SSRF validation on HEAD — ImportExecutor dereferences attacker-supplied file/query URIs through the credentialed client with no URLValidator call. The fix exists on the unmerged branch ft-imports-ssrf-validation (787db7acf).
3. Redirect-following bypasses URLValidator on every guarded sink — all clients set FOLLOW_REDIRECTS=true; the target is validated once, then any 3xx goes to a new host unchecked.
4. OAuth ?referer is never validated (AuthorizeBase.java:82 → LoginBase.java:304) — open redirect delivering the ID token fragment to an arbitrary origin. The OIDC nonce is generated but never checked; the state cookie carries no Secure/HttpOnly/SameSite; the session token is JS-set and JS-readable by design.
5. WebID authentication reduces to a trusted Client-Cert header with no app-tier proof of possession and no proxy authentication — anything reaching Tomcat :7070 directly bypasses WebID auth. The originating commit's TO-DO: security! (f13ae053c) still stands.
6. ldh:send-request (SendHTTPRequest.java:100-146) is an unvalidated request primitive with arbitrary headers, registered on the secretary-certificate-bearing client — reachable from an acl:Write on /settings via ac:stylesheet rewrite (a stylesheet source fetch that itself skips URL validation and accepts file:/jndi: schemes).
7. ACL evaluation is duplicated and already divergent: resource/acl/Access.java:124 loads the document-type result but never applies it, so /acl/access reports a superset of what AuthorizationFilter grants.
8. No site-wide CSP; web.xml:386-394 explicitly disables anti-clickjacking, nosniff, and XSS protection; docker-compose.yml:38 runs the container as root, defeating the Dockerfile's USER ldh; the nginx rate-limit RFC-1918 exemption likely matches the Docker gateway address of all external traffic (no real_ip_header configured).
9. Secrets are handled correctly at rest (Docker secrets, nothing tracked, no defaults) but then pass through xsltproc argv and eval into cleartext ROOT.xml/web.xml; OAuth refresh tokens land in a plaintext properties file keyed on subject alone (cross-provider collision); signup-generated agent private keys (.p12) are left in /tmp indefinitely.

Frontend assets & performance — 3

The weakest area, with the cheapest fixes:

• 7.2 MB of the deployed WAR is never loaded: js/saxon-js/SaxonJS3.js (2.4 MB — only the .rt build is referenced) and js/ol.js.map (4.8 MB).
• The icon font is the full unsubsetted Material Symbols Rounded at 5.36 MB to deliver 52 glyphs, with font-display: block in front of it — icons invisible for up to 3s on cold cache. Subsetting is a ~5.3 MB → ~30 KB change, the single largest available win.
• ~4.7 MB of parser-blocking JavaScript in <head> without defer (YASQE 2.5 MB, OpenLayers 844 KB, three.js 668 KB, 3d-force-graph 691 KB), loading unconditionally on pages with no map, chart, graph, or query block. Cold-cache first render ≈ 7.9 MB compressed transfer including the font and the 19 MB (929 KB gzipped) SEF.
• 55% of the vendored kits' 969 class selectors are unreachable from any XSLT (.ldh-auth*, .ar-*, .bc-*, most of surfaces.css); 33 rules key on a data-tweak-* mechanism that only exists in the design repo's prototype; 34.7 KB of retro.css belongs to six skins that can never be selected; both dark modes are unreachable (nothing sets data-theme at runtime, zero prefers-color-scheme queries).
• retro.css:20 @imports five Google Fonts display families on every page load — all belonging exclusively to the dead skins — the same third-party import deliberately removed from colors_and_type.css one file over.
• No frontend build step exists (no minification, purge, or lint); vendored library versions and licenses are unrecorded (three.min.js is r159, the final UMD build — upgrading requires an ESM migration; ol.js references a LICENSE.txt that is absent).

The hand-written JS surface deserves credit: 98 lines total, every line necessary (two IXSL trampolines, one 429-aware retry wrapper). For a Saxon-JS-first architecture that is the right outcome.

Cross-cutting pattern

The recurring failure mode is finished work that never got wired in:

• getVerifiedClient() — written, constructed, zero call sites
• the import-path SSRF fix — written, sitting on an unmerged branch
• http-tests/system/ — 19 tests written and maintained, never added to run.sh
• EMailListener's thread pool — created and shut down, never submitted to
• the LDH GraphStoreClient — written, bypassed by a ServiceContext type mismatch
• make cli-version — written, documented, not run when it mattered

Each fix exists; the last mile of integration is missing, and nothing mechanical (static analysis, coverage instrumentation, a suite-inventory check, a version-alignment check) exists to catch the gap.

Priority list

1. Wire getVerifiedClient() into the client stack and merge ft-imports-ssrf-validation.
2. Fix the context-dataset race (Application.updateApp + unguarded readers) and the two auth-cache TOCTOUs.
3. Add SpotBugs/ErrorProne + JaCoCo to the pom — every Java defect found here is machine-detectable.
4. Add system/ to run.sh, a pull_request trigger to http-tests.yml, and a test gate on release.yml.
5. Delete SaxonJS3.js and ol.js.map from the WAR; subset the icon font; defer the head scripts.