Upgrade vanilla base 7.6.5 → 7.6.7 (diff-apply two-parent merge)

[milanmajchrak]

Upgrades the fork's vanilla base from 7.6.5 to 7.6.7 (7.6.6 + 7.6.7, mostly security/bugfix maintenance).

Problem

The fork sat on vanilla 7.6.5 and was missing two maintenance releases, including the CVE-2026-49830/49831 cluster and the content-disposition XSS hardening.

Root cause of the awkward merge

git merge-base dtq-dev vanilla is dspace-7.6.1, not 7.6.5 — the fork carries 7.6.2–7.6.5 textually (via CLARIN ports), not as ancestry. A plain git merge dspace-7.6.7 re-litigates the whole 7.6.1→7.6.7 range: 111 conflicted files.

Change set

Applied only the true 7.6.5→7.6.7 delta (git merge-tree --merge-base=dspace-7.6.5) = 26 conflicts, committed via git commit-tree as a real two-parent merge so merge-base is permanently repaired — 7.6.8 will be an ordinary small merge.

Commit	What
81dac88c07	the merge itself (^1 = fork 00501a2db0, ^2 = dspace-7.6.7), 26 conflicts resolved per recipe + 6 out-of-marker fixups
51f4b9e012	CLARIN content-disposition allowlist; D6 Shib ITs moved to the reload-safe helper; test-config pin
e4978e2b95	comment-only correction (8 MB threshold)
c6a1101410	OAI double-escaping fix; allowlist completed
b28457142a	OAI sanitizer also strips unpaired surrogates (see below)
1bfc45b888	CLARIN S3 stack ported to AWS SDK v2 (decision D1 reversed by the user)
75848b2639	S3 defects found by the 10-reviewer audit (retryable multipart, abort-on-error, fd leak, doesBucketExist 403, ContentDispositionUtils, presign region + disposition)
4b5390f9d4	OAIPMHSanitizeIT — the OAI sanitizer's first test, mutation-verified

Test evidence

CI green on the head being merged (4b5390f9d4) — 9 success + 1 skipped (dspace-redeploy, skipped on PRs by design), 0 failures. Maven prints no reactor-wide total, so these are the per-module summaries (run 30882484223):

• Integration -DskipIntegrationTests=false — 540 (dspace-api) + 1 (Additions) + 3362 (server-webapp) + 1 (Local Customizations) = 3904 tests, 0 failures, 0 errors, 60 skipped. dspace-api went 532 -> 540: the eight new ClarinS3BitStoreServiceIT cases, confirmed by name in the CI log alongside S3BitStoreServiceIT at 15; server-webapp gained the 4 OAIPMHSanitizeIT cases
• Unit -DskipUnitTests=false — 78 (services) + 1328 (dspace-api) + 1 (deprecated REST) + 8 (dspace-oai) + 145 (server-webapp) = 1560 tests, 0 failures, 0 errors, 12 skipped
• no flaky reruns despite rerunFailingTestsCount=2; all docker-builds pass; checkstyle 0 violations - from a local mvn verify, NOT from this CI run: the Build workflow passes -Dcheckstyle.skip=true

Note for future reviews: PR CI tests the merge of this head with the moving base, so the server-webapp figure moved 3357→3358 when dtq-dev-767-base advanced — not nondeterminism.

Live rehearsal on a local stack built from this branch:

• Flyway out-of-order trap confirmed, not just predicted. On a DB migrated to the fork's pre-upgrade state both new migrations show Ignored; plain dspace database migrate (what the compose entrypoint runs) does not apply them; dspace database migrate ignored does. entity_type_label_upper_idx created.
• V7.6_2025.10.29 blast radius measured: of 3 requestitem rows it deleted exactly 1 — the one bound to bitstream.deleted IS TRUE. Rows with NULL bitstream_id and rows on live bitstreams survived.
• content-disposition matrix: text/plain, text/csv, image/tiff, application/pdf → inline; text/html, text/xml/TEI → attachment.
• Solr core from this branch keeps both vanilla (*_year, HtmlEncoder highlight) and fork (ASCIIFolding, items_owning_community, dspaceAutoComplete) changes; index-discovery -b completes and search returns results.

Feature parity vs dtq-dev: the merge touched none of the 198 CLARIN-named files; 7 files disappeared: 6 are vanilla deletes that were byte-identical to 7.6.5, the 7th is CmdiDownloadLinkIT.java, a CLARIN-only IT that arrived on the base branch in #1384 after this branch was cut - the merge restores it in the fork. Every class="org.dspace..." in Spring XML resolves to a source file — re-measured at HEAD: 588 distinct candidates, 0 unresolved. (Six were flagged by a naive scan and all six are artefacts: three Constants.* static fields, one nested enum Role.Scope, one bean name rather than a class, and CiniiMetadataContributor, which sits inside an XML comment and is equally absent from vanilla 7.6.7.) Flyway migration scripts 149→152 (the 183→186 figure counted everything under storage/rdbms, Java sources included), 0 lost.

Scope beyond the 26 conflicts (each load-bearing, none incidental)

1. AbstractCurationTask.java — re-added dereference(Context,String) that 7.6.7 deleted; CLARIN-only ItemMetadataQAChecker:409 calls it. Compile fix.
2. ItemServiceTest.java — 5 call sites updated to vanilla's new String q parameter.
3. RequestItemRepositoryIT.java — context.reloadEntity before bitstreamService.delete; CLARIN updateItemFilesMetadata needs an attached item. Test-scoped, not a production change.
4. dspace/config/clarin-dspace.cfg — CLARIN inline allowlist. 7.6.7 flipped this key from blocklist to allowlist, so without it text/plain corpora, audio, TIFF scans and CSV silently became force-download. HTML/XML/TEI/SVG/JS deliberately excluded.
5. dspace-api/src/test/data/dspaceFolder/config/local.cfg — pins the vanilla value for tests. testEnvironment.zip ships clarin-dspace.cfg into dspace.dir, so item 4 alone turned BitstreamRestControllerIT.checkContentDispositionOfFormats red. Keeps the upstream test file byte-identical to vanilla.
6. AuthenticationRestControllerIT.java — the 2 new vanilla Shib tests were the only 2 of 22 sites still using plain setProperty() for the auth plugin sequence, which this file's own javadoc documents as silently discarded under CLARIN config auto-reload. Switched to the fork's setAuthenticationMethodSequence helper.
7. dspace-oai/.../ItemUtils.java — see below.

The one vanilla defect we had to work around

7.6.6 added ItemUtils.sanitize() = StringEscapeUtils.escapeXml10(value) and applies it to every metadata value before the XOAI serializer escapes it again. Everything containing & < > " ' reaches harvesters double-escaped. Measured live:

REST:  u767xssprobe <script>alert(1)</script> end
cmdi:  u767xssprobe &amp;lt;script&amp;gt;alert(1)&amp;lt;/script&amp;gt; end

That corrupts cmdi and olac — CLARIN/LINDAT's primary machine interface (VLO, OLAC). sanitize() now removes characters XML 1.0 forbids instead of escaping them, which is what its own javadoc says it does. Escaping is not lost: Element$Field.write() hands every value to XMLStreamWriter.writeCharacters(), which escapes on its own — it was simply being done twice.

b28457142a completes that fix. escapeXml10 was an AggregateTranslator that also ran UnicodeUnpairedSurrogateRemover, and the first version reproduced only the control-character part. A lone surrogate (truncated 4-byte character, mojibake corpus) then makes the StAX writer throw Broken surrogate pair inside XOAI.index(), and since that is caught per item the record is silently dropped from the OAI index — 2048 code units affected, against 0 for vanilla. Verified across the whole BMP: 0 illegal code units leak, 0 legal ones are removed, valid surrogate pairs survive, unpaired ones do not, metacharacters still pass unescaped. Deliberate remaining difference: C0/DEL U+007F–U+009F are left literal instead of becoming &#NNN;; they are legal XML 1.0 characters, so well-formedness is unaffected.

This is an upstream defect and should be reported to DSpace/DSpace.

Risk

• dspace database migrate ignored is irreversible and V7.6_2025.10.29 deletes rows. Snapshot requestitem first (blast radius above).
• Deploy via sha tag, not dspace-7_x.
• Decision D1 was reversed on 2026-08-03: the S3 stack is now on AWS SDK v2, see the section below. (v1 is past AWS end-of-support.)
• Decision D3: the allowlist is only half the story — webui.content_disposition_threshold (code default 8 MB) still sends larger files as attachment. Left unchanged deliberately; raising it widens the inline-render surface.
• audio/flac, audio/ogg and audio/wav are inert until those formats are added to the registry (backlog).

Rollback

Revert the 7 CLARIN commits above plus the merge 81dac88c07, or simply reset the branch to 00501a2db0. Nothing is deployed by merging. Already-run migrate ignored is not revertible — restore from the pre-migration snapshot.

Non-goals / still open

Playwright suite, CLARIN license-gated download / preview+tgz / Shibboleth / request-a-copy happy path, staged rollout via sha tag, v9-ledger closure, customer branches. ui.baseUrl must be set in the deployment overlay (see the FE PR).

Sign-off status: D1, D5 and D7 were answered by the maintainer and are implemented. D2, D3, D4 and D6
were queried, explained in UPGRADE_767_QUESTIONS.md, and still have no decision — the branch ships the
documented default for each. Merging accepts those defaults. What the audit deliberately did not fix,
and why, is listed in UPGRADE_767_AUDIT.md §2.

🤖 Generated with Claude Code

---

⚠️ Deployment prerequisites found by the final review — read before deploying

No compose path applies the two new migrations. Verified in DatabaseUtils.java: migrate ignored passes outOfOrder=true, but migrate force passes outOfOrder=false and plain migrate defaults to false. This repo's docker-compose.yml:67 runs plain migrate; the FE composes run migrate force. So both new migrations stay Ignored forever — the entity_type_label_upper_idx index is never created and the requestitem cleanup never runs, with no error and no log line.

dspace database migrate ignored therefore has to be run manually per instance, after snapshotting requestitem. That is exactly what the G4 runbook says; this note records why it cannot be skipped. Switching compose to migrate ignored would make the destructive migration run automatically on every stack start — deliberately not done here.

Pair with the FE PR's ui.baseUrl prerequisite (see dataquest-dev/dspace-angular#1398).

---

AWS SDK v2 port (1bfc45b888) — added 2026-08-03

7.6.7 rewrote S3BitStoreService from com.amazonaws (v1) to software.amazon.awssdk (v2 CRT client). This PR
originally rejected that and kept v1; the user reversed the decision — v1 is past AWS end-of-support anyway.

The fork owns three S3 classes vanilla does not know, so this was not a file takeover:

File	Treatment
S3BitStoreService	vanilla 7.6.7 + exactly three CLARIN items (+58/-8 vs dspace-7.6.7 at HEAD; was +47/-5 before the audit fixes): getFile(Bitstream), a protected client so the subclasses can reuse it, and a configurable pathStyleAccessEnabled. Vanilla hardcodes forcePathStyle(true) on an endpoint override; keeping the property preserves the fork's v1 behaviour and keeps bitstore.xml valid.
SyncS3BitStoreService (339 lines, 199 non-comment)	CLARIN-only, hand-ported including the explicit multipart path
S3DirectDownloadServiceImpl (204 lines, 142 non-comment)	CLARIN-only; presigning moved to S3Presigner, built from the same credentials/region/endpoint as the bitstore client
S3BitStoreServiceIT	taken from 7.6.7 wholesale — the fork's copy was byte-identical to 7.6.5, so no CLARIN test was lost. LocalStack replaces s3mock, which only ever spoke v1.
ClarinS3BitStoreServiceIT	new — the CLARIN-only paths had no test at all
bitstore.xml, assetstore.cfg	the merge had dropped vanilla's four new tuning keys (targetThroughputGbps, minPartSizeBytes, maxConcurrency, s3ChecksumAlgorithm)

getFile is not a new invention: it is byte-identical to the fork's own v9 implementation (5e930d1f2a), so the
7.x and v9 branches do not diverge on it.

Two defects the new tests caught — neither visible by reading the code

1. ETag quoting. v1 returned part ETags unquoted, v2 returns them quoted. The multipart checksum comparison
   would have failed on every part, and because that path logs and swallows, completeMultipartUpload would
   never run — a silently failed upload with a bitstream row pointing at nothing.
2. Leaked FileOutputStream. SyncS3BitStoreService.put() never closed the output stream on the local
   assetstore copy. Pre-existing, not a port regression, but it leaks a handle per stored bitstream and made
   remove() silently fail to delete the local file.

Evidence

• mvn test-compile -DskipTests — Reactor Summary 13/13 modules SUCCESS
• S3BitStoreServiceIT (vanilla, LocalStack) — Tests run: 15, Failures: 0, Errors: 0
• ClarinS3BitStoreServiceIT (new) — Tests run: 8, Failures: 0, Errors: 0
• S3DirectDownloadServiceTest — Tests run: 11, Failures: 0, Errors: 0
• ContentDispositionUtilsTest (new) — Tests run: 9, Failures: 0, Errors: 0
• OAIPMHSanitizeIT (new) — Tests run: 4, Failures: 0, Errors: 0
• checkstyle 0 violations; git grep com.amazonaws finds nothing anywhere in the tree

s3Store is lazy-init and assetstore.index.primary defaults to the local store, so nothing otherwise
instantiates the bean — springWiringBindsStoreProperties pulls it out of the service manager on purpose. It is the
only check that the blank maxConcurrency binds to a null Integer and that s3ChecksumAlgorithm binds to the enum.

Not covered by tests

No test talks to a real non-AWS endpoint, so the CESNET/MinIO path (endpoint + pathStyleAccessEnabled) and the
presigned-URL path are verified against LocalStack and mocks only. Worth one manual download check on a dev instance
with S3 enabled before this reaches production.

---

Independent audit round (2026-08-04) — 10 reviewers, and what they found

Ten independent reviewers audited this PR, each on a separate area, all told to disprove rather than
confirm. They found real defects; the fixes are the two commits above 1bfc45b888 (75848b2639, 4b5390f9d4) plus ff96c2cada in the FE PR.

Fixed here

What	Why it mattered
Multipart upload lost retryability	uploadPart got a non-resettable stream, so any transient S3 error became permanent; the failure was then logged and swallowed and put() still recorded the bitstream — a DB row pointing at an object S3 does not have. Now uses AsyncRequestBody.fromFile(FileRequestBodyConfiguration), aborts the upload, and throws.
The multipart loop never ran twice	50 MB part size vs a 23-byte fixture. The offset arithmetic this PR rewrote had zero coverage. syncStoreUploadsInMultipleParts now crosses a real part boundary and verifies the object byte for byte.
A tripwire in amazonClientBuilderBy left all 21 S3 tests green	Every test injected a ready-made client, so endpointOverride, forcePathStyle, maxConcurrency and the part sizes never executed. The fork's headline delta — configurable pathStyleAccessEnabled — was verified only at its getter. Now covered by a test proven to go red when endpointOverride is removed.
Two Content-Disposition implementations, the newer one wrong	It escaped " but not \, and used URLEncoder so a space arrived as +. Extracted to ContentDispositionUtils, used by both call sites.
Direct downloads silently killed inline preview	The redirect hardcoded attachment, discarding the disposition the allowlist had just computed.
Presigner region could diverge from the client's	Hardcoded us-east-1; now mirrors init() branch for branch.
A 403 read as "bucket absent"	init() would try to create an existing bucket; a least-privilege policy denies that and the assetstore comes up dead.
s3ChecksumAlgorithm was dead config	bitstore.xml wires SyncS3BitStoreService, which overrides put(), so the only consumer never ran.
ItemUtils.sanitize() had no test	Now OAIPMHSanitizeIT, verified non-vacuous: disabling the sanitiser turns 3 of 4 red with WritingXmlException and SAXParseException.

Corrections to earlier claims in this description

• The content-disposition allowlist flip was 7.6.7, not 7.6.6; ItemUtils.sanitize() arrived in 7.6.6, not 7.6.7.
• Of the 7 disappeared files, the 7th is a CLARIN-only IT (CmdiDownloadLinkIT), not a vanilla delete. Risk is nil — the base branch restores it — but the original sentence implied no CLARIN file was affected.
• Retracted twice. "523 of 525 Spring FQCNs resolve; the 2 that do not are Solr plugins" was wrong, and so was the follow-up claim that "2 were lost". Neither figure reproduces under any scope. Re-measured: 588 distinct candidates at HEAD, 0 unresolved — see the parity paragraph above.
• "Flyway 183→186" counted everything under storage/rdbms; the migration-script count is 149→152.
• "No compose path applies the two new migrations" is a false absolute: db.entities.yml in both repos runs migrate ignored. The conclusion about the deploy path stands — deploy.yml uses only docker-compose.yml and -rest.yml.
• "checkstyle clean" was quoted next to CI evidence, but the Build workflow passes -Dcheckstyle.skip=true; that claim comes from local runs.

Known and deliberately not fixed

• The CLARIN allowlist is not exercised by any test. dspace-api/src/test/data/dspaceFolder/config/local.cfg pins the vanilla list and beats clarin-dspace.cfg, which is what keeps the upstream IT green. Fixing it means giving the CLARIN list its own IT.
• Relative to dtq-dev, 68 of 97 registry formats flip inline → attachment (office, OpenDocument, archives, TeX). That is the allowlist semantics change working as intended, but users will notice.
• dspace checker -a <handle> was renamed to -i upstream, and checker-emailer's long options were recased. Any cron using the old spellings breaks silently after deploy.
• get() no longer reads in 5 MB ranges (vanilla dropped S3LazyInputStream); a single stream is held open for the whole object, which exposes proxy idle timeouts on multi-GB corpora that chunking used to hide.
• Preview temp files rely on deleteOnExit, which only fires on a clean JVM shutdown. Pre-existing, but it scales with corpus size.
• The presigned-URL redirect IT never runs — @IfProfileValue(name="s3.download.direct.enabled", value="true") and nothing sets that property.
• No test talks to a real non-AWS endpoint, so the CESNET/MinIO path is verified against LocalStack only.