You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: configurable extension size limits with namespace overrides - #2287
Implements #2129: an admin-configurable default upload size limit, plus overrides for namespaces and individual extensions, with extension > namespace > default precedence.
What's here
Runtime default — a max-extension-size setting in the existing Setting table, falling back to ovsx.publishing.max-content-size until an admin sets it, plus the field on the admin Settings page.
Overrides and enforcement — a new extension_size_override table (V1_79), resolution, a cached ceiling, and two-stage enforcement.
Admin API — /admin/size-overrides CRUD in a focused controller, audited into PersistedLog, bounded by a new ovsx.publishing.max-override-size ceiling.
Admin UI — a Size overrides page in the admin dashboard.
Clients — GET /api/-/size-limit reports the limit that applies to a given namespace/extension, and /api/version additionally reports the registry ceiling.
Docs — doc/configuration.md gains the new property and an "Extension size limits" section covering precedence, enforcement and what clients can see.
Why enforcement is two-stage
The limit is enforced while the request body is still streaming, but the namespace is not known until the package has been parsed. So the body is capped at the ceiling — the default raised by the highest configured override — and once the manifest is read, the limit that actually applies is resolved and the stored package rejected with 413 if it exceeds it.
The cost: an oversized upload from a namespace with no override is accepted up to the ceiling before being rejected. The namespace is not in the request, so there is nothing earlier to check against. A registry with no overrides configured has a ceiling equal to its default and behaves exactly as before.
What the clients check, and why they differ
The same constraint shapes both, in opposite directions:
The CLI parses the package manifest before uploading, so it knows its namespace and asks /api/-/size-limit for the real limit, refusing an oversized upload locally. Against a registry older than 1.3.0 it falls back to warning against the default and publishing anyway, so this release does not break publishing to older registries.
The web UI cannot do that — the namespace lives inside the package and nothing in the browser parses it — so the publish page gates on the ceiling. That never blocks an upload an override would have allowed; anything below the ceiling is sent and answered by the server.
Notable decisions
A new AccessTokenAction.VerifyPublishVersion for the size-limit lookup. Verify reports no namespace, so a scoped token — every trusted publishing token is one — fails the scope match; PublishVersion would bump the accessed timestamp and delete a one-time token before the publish it was checked for could use it.
No created_by column on the override table. Override mutations are already audited through LogService into PersistedLog; the FK stored the same fact twice where nothing read it.
No per-(namespace, extension) cache. Resolution is one indexed query per publish, alongside VSIX parsing and several writes. Only the ceiling is cached.
A separate controller, not AdminAPI, which is already ~1550 lines.
max-override-size lives on PublishingConfig, which also gains the @PostConstruct validation it was missing.
Known gaps
No public documentation page exists for size limits yet, so the 413 names the applicable limit and points the publisher at the registry administrators rather than linking out.
Test plan
./gradlew unitTests — 1476 passing
./gradlew spotlessCheck
webui yarn test (379), tsc --noEmit and yarn lint
cli yarn test (130), tsc --noEmit and yarn lint
./gradlew test (full container tier) — 1638 passing locally against a real Postgres, jooqCodegen clean
Manual check of the admin Size overrides page against a running registry
Manual check of ovsx publish against a namespace with an override
The persisted audit text records only the new runtime default. The linked requirement calls for previous and new values, so an administrator cannot reconstruct what upload limit this mutation replaced. Include the current value in the message before writing the new one.
byteCountToDisplaySize rounds to whole display units, so common boundary failures produce a contradictory message—for example, 1,048,577 bytes over a 1,048,576-byte limit is reported as “(1 MB) exceeds ... 1 MB.” Include exact byte counts (optionally alongside the friendly values) so the publisher can tell why the 413 occurred.
Empty query values reach VerifyPublishVersion, whose constructor throws IllegalArgumentException; this method catches only ErrorResultException, and the global validation advice handles only constraint violations, so /api/-/size-limit?namespace=&extension= returns a 500. Validate both parameters as nonblank so malformed client input receives a 400.
Parent deletion skips cache invalidation for size-limit overrides
These cascades remove overrides without going through ExtensionSizeLimitService, and the extension/namespace purge paths do not call SettingsService.invalidateCache(). Deleting the highest override through either parent therefore leaves every node reporting and streaming against the old higher ceiling until the settings cache TTL expires, allowing avoidable oversized request buffering. Ensure parent deletion invalidates the ceiling after commit (and publishes the cross-node invalidation).
Stale namespace lookup response enables invalid submission
The namespace field remains editable while lookup is pending. If the admin changes it before the old request resolves, that response sets namespaceConfirmed for the new text and enables submission with extensions from the previous namespace. Disable editing during lookup (or discard responses whose requested namespace is no longer current).
Size-limit error rounds values and hides the actual overage
These display values are rounded down to whole units, so a 536,870,913-byte package against a 536,870,912-byte limit produces the contradictory message “512 MB exceeds 512 MB.” Include exact byte counts (optionally alongside the readable values) so the new 413 explains the actual overage.
The loading state is only visual, so the Delete button remains clickable while the request is in flight and can issue duplicate delete mutations. Disable it while loading, as the existing tier/customer delete dialogs do.
The lookup response is applied even if the namespace input changed while the request was pending. For example, looking up foo, then typing bar before the response arrives marks bar as confirmed and enables creating a namespace-wide override without ever looking it up; closing and reopening during a request has the same stale-response problem. Cancel the request or ignore results unless they still match the current dialog/input generation.
Clarify lower ceiling overrides remain useful
doc/configuration.md:52
This overstates the effect of a ceiling below the default: overrides remain useful for imposing a smaller limit on a namespace or extension. Only overrides that raise the default become impossible.
Picked up the three Previously missed items from the latest review — all three were real, in code none of my earlier rounds had touched.
Stale namespace lookup (size-override-form-dialog.tsx). The answer was applied unconditionally, so looking up foo and typing bar before it landed confirmed bar without anyone looking it up, and an override could be created against it. Answers are now dropped unless they still match the current lookup; the same counter is bumped when the namespace is edited and when the dialog is reset, which covers the close-and-reopen case too. ignores a lookup answer that arrives after the namespace changed reproduces it — I checked it fails without the guard.
Delete button clickable during the request (delete-size-override-dialog.tsx). The loading state was only visual. Both buttons are now disabled while in flight, which is what delete-tier-dialog.tsx — the dialog this was cloned from — already did. I had dropped it in the clone.
Documentation overstated the effect of a low ceiling (doc/configuration.md). Correct: only overrides that raise a limit become impossible; one that tightens a namespace below the default still works, since requireValidSize only rejects values above the ceiling. Reworded.
webui is at 364 tests with a clean typecheck and lint.
Blank query values reach VerifyPublishVersion, whose constructor throws IllegalArgumentException; this method catches only ErrorResultException, so /api/-/size-limit?namespace=&extension=... returns a 500 instead of a client-validation error. Validate both required parameters as nonblank so the existing validation handler returns 400.
After the initial query fails, loading is false and sizeOverrides is empty, so the page shows both the error alert and “No size overrides are configured.” That empty-state claim is false because no list was loaded. Suppress it when there is a load error and no prior data.
/api/-/size-limit answered 500 for a blank parameter (RegistryAPI.java). The value reached VerifyPublishVersion, whose compact constructor rejects blanks with an IllegalArgumentException that the endpoint does not catch — a client mistake reported as a server error. Both parameters are now @NotBlank, so the existing ValidationExceptionHandler answers 400. Two tests cover it.
The overrides page claimed there were none when the load failed (size-overrides.tsx). With loading false and the list empty, the empty state rendered next to the error alert, asserting something about data that never arrived. It is suppressed when there is a load error, since the alert already explains what happened.
Server 1453 tests, webui 366 with clean typecheck and lint.
This refresh can still return the stale ceiling it is meant to bypass. GET /api/version is served with Cache-Control: public, max-age=300 (RegistryAPI.java:1977-1981), and getRegistryVersion reaches fetch without a cache override (extension-registry-service.ts:675-677, server-request.ts:56-86), so a fresh browser/proxy cache entry is reused after an override is granted. During those five minutes the retry still rejects a package the server now accepts. Make this pre-rejection refresh explicitly bypass/revalidate the HTTP cache (while ordinary version reads may remain cached).
Size-limit error rounds values and hides one-byte overages
byteCountToDisplaySize rounds down to whole units, so a package only one byte over the limit can produce the contradictory message “(1 MB) exceeds the size limit of 1 MB.” Include the exact byte counts (optionally alongside the friendly values) so the 413 accurately explains why the package was rejected.
Both Previously missed items were valid and are fixed.
The refresh was served from cache (publish-queue-context.tsx). This one defeated the fix it was part of: /api/version is max-age=300 and sendRequest sets no cache option, so the pre-rejection refresh was handed back the very ceiling it was meant to bypass. The damaging window is exactly the one the refresh exists for — a publisher who loaded the page minutes ago, asked an admin for an override, and retries while the cached copy is still fresh.
getRegistryVersion now takes { revalidate }, which sends Cache-Control: no-cache so the check reaches the registry through any cache in between. Ordinary version reads stay cached, so nothing else pays for it. The queue test asserts the revalidating call rather than just that a call happened.
The 413 could contradict itself (LocalRegistryService.java). byteCountToDisplaySize rounds down to whole units, so a package one byte over produced "(1 MB) exceeds the size limit of 1 MB" — accurate in neither direction and no use to the publisher reading it. It now carries exact byte counts alongside the readable ones, and the test pins both.
I left the stream-time message in ExtensionService alone: it names only the limit, never the actual size, so it cannot contradict itself.
Server 1453 tests, webui 366, cli 129, clean typecheck and lints throughout.
ExtensionSizeLimitService declared none, and open-in-view is off, so every
repository call ran in its own transaction. updateOverride therefore loaded
a detached entity and wrote it back through merge, which rewrites every
column from a snapshot another request may already have moved past: a
concurrent namespace change had its re-pointing reverted, and a concurrent
delete was resurrected as a new row. AGENTS.md asks for find-and-setter over
merge for exactly this.
update and delete now load and write in one transaction, so the entity stays
managed and the setter persists it. create retries once on the unique index,
as postReview does, so losing a concurrent create reports "already exists"
instead of a 500 the caller cannot act on. moveOverridesToNamespace is
MANDATORY: its re-pointing is a bare setter that would be discarded in
silence outside a transaction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The write was skipped when the new value matched what was read back, but
what is read back is a node-local cache entry that can be a minute stale, or
the configuration fallback where nothing has been stored yet. So restoring a
value another node had just moved away from wrote nothing and reported
success, and storing the setting meant to shadow ovsx.publishing
.max-content-size was a no-op until the two differed.
Omitted settings are still left alone, which is what partial updates are
for; present ones are written unconditionally, the upsert being idempotent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A registry too old to report a namespace's limit and one that failed to
answer were both treated as "no limit known", and the fallback only warned
when the package exceeded the registry default. But a size override can
lower a limit as well as raise it, so no package is small enough for that
silence to be safe: a misrouted /api/-/* route looked exactly like a working
preflight, and the upload the check exists to prevent went ahead unremarked.
The two cases are now distinct. A failed lookup always reports that the
check did not happen; a registry below 1.3.0 keeps the advisory warning,
reworded since the namespace's limit may be lower than the default, not only
higher.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The publish gate read maxExtensionSizeCeiling alone, so against a registry
that does not report one - an older server behind a newer bundle, a cached
bundle, a third-party deployment - the field was absent, the check was
skipped, and nothing refused an oversized upload until the server did. The
publish page went on advertising the default limit all the while.
The ceiling is never below the default, so falling back to it can only
refuse earlier than strictly necessary, which is the safe direction. The
revalidation path already guarded this way; the initial read now does too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The field's value was the draft's number rendered back, and Number('') is 0,
so the keystroke that emptied it stored zero and wrote "0" straight back in.
The admin could not retype the size without selecting the injected zero
first, and the field showed an error it had put there itself.
It now holds what was typed, converting on the way out, as the size override
dialog already does. An empty field is held as invalid rather than as zero,
so Save stays disabled until a size is entered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Extension overrides follow their extension, but the namespace-wide override
moves only when the old namespace is going away and the new one has none of
its own. The other two cases - discarded with the old namespace, or left
behind on a namespace that no longer holds the extensions - were missing
from the list of ways an override disappears without an admin asking.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Publishing wrote the body to a temp file and parsed it before looking at the
token, because the scope depends on the namespace and the namespace is
inside the package. A caller with no usable token could therefore make every
node write a file of up to the size ceiling and parse a zip before being
told no - and this feature made that ceiling a value an admin raises from
the dashboard rather than one fixed in the configuration file.
The liveness checks a token has to pass - exists, active, has a user, not
expired, registration intact for a trusted publishing token - are all
independent of the namespace, so they are extracted and asked first. The
scope is still judged where it always was, after the package is parsed:
every scope matches an action against the namespace its token is bound to,
so no action can ask this question through useAccessToken.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It reads as a guarantee that nothing can exceed it, and it is not one: the
registry-wide default is validated only as greater than zero, so a default
set above it gives every namespace a higher limit without an override being
involved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A drop navigates to the publish page without waiting for the queue, and the
queue awaited the re-read of the registry ceiling before adding any card. So
a package that looked too big produced an empty page for the length of a
network round trip - which reads as a drop that did nothing, and invites
dropping the same package again, publishing it twice.
The cards now go up first, in a checking state, and are settled once the
answer arrives. Only when something looks too big: a drop that is plainly
within the limit still reaches the registry in one go, with no extra render.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The service's other tests mock the repository, where no entity is ever
managed, so a write that relies on dirty checking and one that relies on a
save call look the same. These run against a real persistence context and
round-trip through the database, which is the only way to see that the
update persists and that it leaves the scope alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
It combines a database migration, authentication changes, upload enforcement, distributed cache invalidation, and two client implementations, warranting final human validation.
The ten commits from 45b7fe6 to 83b41f1 come from an adversarial review run separately from the Copilot rounds, across four dimensions: concurrency and transactions, authorization and disclosure, schema and data lifecycle, and client-side correctness. It found more defects than the preceding several automated rounds, and the two most serious were ones that reading the diff could not surface, because they were about the code's context rather than its content.
Fixed
Transaction boundaries on the override writes (45b7fe6). ExtensionSizeLimitService declared no transaction at all, and open-in-view is off, so each repository call ran in its own. updateOverride therefore loaded a detached entity and wrote it back through merge, which rewrites every column from a snapshot another request may have moved past. Two reviewers found this independently and gave different triggers: a concurrent delete resurrected as a fresh row, and — needing no second admin, only the ChangeNamespaceJobRequestHandler running — a namespace rename whose re-pointing was reverted, leaving the override attached to a namespace that no longer holds the extensions, where resolveLimit can never match it again. AGENTS.md asks for find-and-setter over merge for exactly this case. Create now retries once on the unique index as postReview does, so losing a concurrent create reports "already exists" rather than a 500; moveOverridesToNamespace is MANDATORY, its re-pointing being a bare setter that would be discarded in silence outside a transaction.
Work performed before authorization (0bdc6a5). Publishing wrote the body to a temp file and parsed the zip before looking at the token, because the scope depends on the namespace and the namespace is inside the package. A caller with no usable token could therefore make every node do that work before being refused — and this PR had made the cap on it max(default, highest override), a value an admin raises from the dashboard, rather than one fixed in the configuration file. The liveness checks a token must pass are all namespace-independent, so they are now extracted and asked first. Note the shape of the fix: an earlier attempt added an Authenticate action to the trusted-publishing token's ActionScoped list, which does nothing — a TPT's scope is ExtensionScoped(...).and(ActionScoped(...)), and ExtensionScoped rejects any action that does not name a namespace before ActionScoped is consulted. Verify fails identically. No action can ask this question through useAccessToken, which is why the check lives beside it.
Settings writes were skipped when the value looked unchanged (e333cbe). The comparison was against a node-local cache entry that can be a minute stale, or against the configuration fallback where nothing has been stored yet. So restoring a value another node had just moved away from wrote nothing and reported success, and storing the setting that is meant to shadow ovsx.publishing.max-content-size was a no-op until the two differed. Settings an update does not carry are still left alone.
The CLI's size check failed silently (e9c9a6c). A registry too old to report a namespace's limit and one that failed to answer were both "no limit known", and the fallback warned only when the package exceeded the registry default. But an override can lower a limit as well as raise it, so no package is small enough for that silence to be safe — a misrouted /api/-/* looked exactly like a working preflight. The two cases are now distinct.
The webui gate disappeared against a registry reporting no ceiling (f5aa68b), the max size field could not be cleared (5b990d4, Number('') is 0, which was written straight back into the input), a failed background refetch blanked the overrides page (fb2f7f7), and an oversized drop showed nothing while the ceiling was re-read (37caaaf), which invited dropping the same package twice.
Documentation: a namespace change is a second and third way an override can vanish or be stranded (3009992), and max-override-size reads as a guarantee that nothing can exceed it, which it is not — the registry-wide default is validated only as greater than zero (3038cf8).
Accepted, not missed
Eager @ManyToOne on ExtensionSizeOverride.resolveLimit reads two scalars but materialises both associations. Deliberately left: the unique index bounds this at two rows, both call sites are per-publish, and LAZY is load-bearing — ExtensionSizeOverrideAPI.toJson reads the association names outside any transaction and would throw LazyInitializationException. Doing it properly means a JOIN FETCH for the list path and a projection for resolveLimit, which is not worth it for a cost that could not be measured here.
Dialog content changes during the ~195 ms close animation, because onClose clears selected in the same batch. The delete confirmation briefly shows an empty name; the form dialog's title flips to "Create". Cosmetic, no data effect.
The runtime default has no upper bound. Capping it at max-override-size would contradict a deliberate decision in this PR not to cross-validate that property against max-content-size at startup, so a registry configured with a larger default would be unable to set it from the dashboard. Documented instead.
VerifyPublishVersion is a scope oracle for someone already holding a stolen token whose extension they do not know, and because it does not "use" the token the probing leaves no accessed timestamp. Requires a prior compromise, and the same person could enumerate by attempting publishes.
Verification
./gradlew test, the full container tier against a real Postgres: 1638 passing. That run includes three new tests (83b41f1) covering what the override writes leave in the database — the service's other tests mock the repository, where nothing is ever managed, so a write relying on dirty checking and one relying on a save call are indistinguishable. They round-trip through flush()/clear() rather than reading the instance back.
Every fix above was fail-first checked: the fix reverted, the specific test confirmed red, then restored.
Two limits worth stating. The concurrent update-vs-delete and update-vs-rename races that motivated 45b7fe6 are not directly tested — they need two simultaneous transactions. What is tested is the mechanism they exploited. And ovsx publish against a namespace with an override above 512 MiB is still the one unticked box; note that the happy path succeeds even if the preflight is broken, so the case that proves anything is a package above the override, which the CLI should refuse by name before uploading.
Three of the new temp-file-cleanup tests broke once rebased past eclipse-openvsx#2129/
eclipse-openvsx#2287's size-limit overhaul:
- 'when it exceeds the registry size limit' relied on the old client-side
size check, which is now advisory for a pre-1.3.0 registry (it warns and
publishes rather than rejecting). Switched it to a namespace size-limit
override on a 1.3.0+ registry, which does still reject before upload.
- 'given as --packagePath' and 'given as --extensionFile, even when the
registry rejects it' called the now-async givenExtensionFile() without
awaiting it, so they exercised a Promise instead of a file path.
Also adds a regression test for the finally-block fix: a cleanup failure
(e.g. EBUSY on a locked temp dir) must not replace the original publish
error.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(cli): delete the temp .vsix that publish packages
publish packaged into a file in the OS temp directory and never removed
it, leaving one .vsix behind per package and target. It is now removed
once publishing finishes, whether it succeeded, failed, or was skipped
as a duplicate. A .vsix supplied by the user is left alone.
Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
* fix(cli): repair tests broken by the rebase onto the size-limit refactor
Three of the new temp-file-cleanup tests broke once rebased past #2129/
#2287's size-limit overhaul:
- 'when it exceeds the registry size limit' relied on the old client-side
size check, which is now advisory for a pre-1.3.0 registry (it warns and
publishes rather than rejecting). Switched it to a namespace size-limit
override on a 1.3.0+ registry, which does still reject before upload.
- 'given as --packagePath' and 'given as --extensionFile, even when the
registry rejects it' called the now-async givenExtensionFile() without
awaiting it, so they exercised a Promise instead of a file path.
Also adds a regression test for the finally-block fix: a cleanup failure
(e.g. EBUSY on a locked temp dir) must not replace the original publish
error.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Co-authored-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
When we introduced the per-setting beans in #2257 the idea was to have the settings open for extension and closed for modification, so adding a setting never touches `SettingsService`. We deprecated `isReadOnly()` on the service back then, but the max extension size that came in with #2287 ended up doing exactly the opposite and added `getMaxExtensionSize()` to the service, so we had one getter deprecated and another one alive, and both settings reading the cache from the same bean that encodes and validates them.
This settles that. A setting bean only knows how to encode and decode its rows, and a setting the server needs at runtime gets its own reader (`ReadOnlyMode`, `ExtensionSizeLimitService`) that asks the service for the current value through a package-private `current(setting)`. Everything else injects the reader, the setting beans, their keys and the cache are package-private now, and the getters on the service are gone.
We're also taking the json mapping out of the service. It returns `SettingRows` now and `SettingsJsonMapper`, next to the beans since it's the only place that can see the keys, does the mapping for `AdminAPI` and `SettingsAPI`, so the DTOs don't name any setting codec anymore.
Assisted-By: anthropic:claude-fable-5
When we introduced the per-setting beans in #2257 the idea was to have the settings open for extension and closed for modification, so adding a setting never touches `SettingsService`. We deprecated `isReadOnly()` on the service back then, but the max extension size that came in with #2287 ended up doing exactly the opposite and added `getMaxExtensionSize()` to the service, so we had one getter deprecated and another one alive, and both settings reading the cache from the same bean that encodes and validates them.
This settles that. A setting bean only knows how to encode and decode its rows, and a setting the server needs at runtime gets its own reader (`ReadOnlyMode`, `ExtensionSizeLimitService`) that asks the service for the current value through a package-private `current(setting)`. Everything else injects the reader, the setting beans, their keys and the cache are package-private now, and the getters on the service are gone.
We're also taking the json mapping out of the service. It returns `SettingRows` now and `SettingsJsonMapper`, next to the beans since it's the only place that can see the keys, does the mapping for `AdminAPI` and `SettingsAPI`, so the DTOs don't name any setting codec anymore.
Assisted-By: anthropic:claude-fable-5
* chore: read site settings through per-setting reader beans
When we introduced the per-setting beans in #2257 the idea was to have the settings open for extension and closed for modification, so adding a setting never touches `SettingsService`. We deprecated `isReadOnly()` on the service back then, but the max extension size that came in with #2287 ended up doing exactly the opposite and added `getMaxExtensionSize()` to the service, so we had one getter deprecated and another one alive, and both settings reading the cache from the same bean that encodes and validates them.
This settles that. A setting bean only knows how to encode and decode its rows, and a setting the server needs at runtime gets its own reader (`ReadOnlyMode`, `ExtensionSizeLimitService`) that asks the service for the current value through a package-private `current(setting)`. Everything else injects the reader, the setting beans, their keys and the cache are package-private now, and the getters on the service are gone.
We're also taking the json mapping out of the service. It returns `SettingRows` now and `SettingsJsonMapper`, next to the beans since it's the only place that can see the keys, does the mapping for `AdminAPI` and `SettingsAPI`, so the DTOs don't name any setting codec anymore.
Assisted-By: anthropic:claude-fable-5
* chore: rename ReadOnlyMode to ReadOnlyModeService
* chore: update ReadOnlyMode references to ReadOnlyModeService
* chore: let a setting read its own value from the settings store
A setting the server consults exposes its own accessor over the cached
snapshot, so the reader beans go, SettingsCache becomes the public
SettingsStore, and each consulted setting moves into a package of its own.
* chore: read settings through their own accessors
The previous commit captured only the file moves. This is the change
itself: ReadOnlySetting.isEnabled() and MaxExtensionSizeSetting.getValue()
over the public SettingsStore, the reader beans and SettingsService.current
removed, callers injecting the setting, and the tests using the key
constants.
* chore: keep the settings in one package and guard every *API mutation
Move the settings out of the readonly and extensionsize subpackages back
into settings, which shrinks the change and lets SettingsStore stay
package-private.
ReadOnlyEndpointAspect only matched RegistryAPI, UserAPI and the admin
package, so the TrustedPublishingAPI mutations ran in read-only mode.
Match every class named *API instead, and add a test that every
@MutatingOperation is guarded.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: add a changelog entry for the read-only trusted publishing fix
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #2129: an admin-configurable default upload size limit, plus overrides for namespaces and individual extensions, with extension > namespace > default precedence.
What's here
max-extension-sizesetting in the existingSettingtable, falling back toovsx.publishing.max-content-sizeuntil an admin sets it, plus the field on the admin Settings page.extension_size_overridetable (V1_79), resolution, a cached ceiling, and two-stage enforcement./admin/size-overridesCRUD in a focused controller, audited intoPersistedLog, bounded by a newovsx.publishing.max-override-sizeceiling.GET /api/-/size-limitreports the limit that applies to a given namespace/extension, and/api/versionadditionally reports the registry ceiling.doc/configuration.mdgains the new property and an "Extension size limits" section covering precedence, enforcement and what clients can see.Why enforcement is two-stage
The limit is enforced while the request body is still streaming, but the namespace is not known until the package has been parsed. So the body is capped at the ceiling — the default raised by the highest configured override — and once the manifest is read, the limit that actually applies is resolved and the stored package rejected with 413 if it exceeds it.
The cost: an oversized upload from a namespace with no override is accepted up to the ceiling before being rejected. The namespace is not in the request, so there is nothing earlier to check against. A registry with no overrides configured has a ceiling equal to its default and behaves exactly as before.
What the clients check, and why they differ
The same constraint shapes both, in opposite directions:
/api/-/size-limitfor the real limit, refusing an oversized upload locally. Against a registry older than 1.3.0 it falls back to warning against the default and publishing anyway, so this release does not break publishing to older registries.Notable decisions
AccessTokenAction.VerifyPublishVersionfor the size-limit lookup.Verifyreports no namespace, so a scoped token — every trusted publishing token is one — fails the scope match;PublishVersionwould bump the accessed timestamp and delete a one-time token before the publish it was checked for could use it.created_bycolumn on the override table. Override mutations are already audited throughLogServiceintoPersistedLog; the FK stored the same fact twice where nothing read it.AdminAPI, which is already ~1550 lines.max-override-sizelives onPublishingConfig, which also gains the@PostConstructvalidation it was missing.Known gaps
Test plan
./gradlew unitTests— 1476 passing./gradlew spotlessCheckyarn test(379),tsc --noEmitandyarn lintyarn test(130),tsc --noEmitandyarn lint./gradlew test(full container tier) — 1638 passing locally against a real Postgres,jooqCodegencleanovsx publishagainst a namespace with an override