Skip to content

Trusted publishing changes - #2114

Draft
netomi wants to merge 8 commits into
mainfrom
feat/trusted-publishing
Draft

Trusted publishing changes#2114
netomi wants to merge 8 commits into
mainfrom
feat/trusted-publishing

Conversation

@netomi

@netomi netomi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Work in progress on the trusted publishing feature. Opened as a draft so the changes are visible as they land.

So far

  • Move TrustedPublishingAPI into org.eclipse.openvsx.trustedpublishing — the controller sat in the root package while the rest of the feature (service, config, providers, entities' repository) is in the trustedpublishing package. Pure move, endpoint paths unchanged.
  • Add TrustedPublishingAPITest — there was no controller-level test coverage. 26 tests in a @WebMvcTest slice (service, Eclipse service and user service mocked) covering all five endpoints:
    • POST /user/namespace/{ns}/trusted-publishing/create — anonymous access, each missing mandatory field, path/body namespace mismatch, successful registration, unknown namespace, non-owner, rejected registration
    • GET /user/namespace/{ns}/trusted-publishing — anonymous access, publishers + registrable extensions, empty result, unknown namespace, feature disabled, non-owner
    • POST /user/namespace/{ns}/trusted-publishing/delete/{id} — anonymous access, success, unknown publisher, non-owner
    • POST /api/-/trusted-publishing/token — each missing field, successful exchange (asserting no login is consulted), no matching publisher, unusable token
    • GET /api/-/trusted-publishing/status — anonymous access, feature disabled, missing publisher agreement, provider listing, error mapping

Noted while writing the tests — since fixed

In createTrustedPublisher, eclipseService.checkPublisherAgreement(user) was called outside the try/catch, and no @ControllerAdvice handles ErrorResultException. A user without a signed publisher agreement therefore got a 500 with an empty body instead of the 403 and the {"error": "..."} message the exception carries. Fixed in 39416689d.

See the comments below for the changes that followed.

🤖 Generated with Claude Code

netomi and others added 4 commits September 1, 2026 08:48
The controller lived in the root org.eclipse.openvsx package while the rest
of the feature (service, config, providers) is in org.eclipse.openvsx.trustedpublishing.
Endpoint paths are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers all five endpoints in a @WebMvcTest slice with the service, the
Eclipse service and the user service mocked: request validation, status
codes, response shape and the mapping of NotFoundException and
ErrorResultException.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every GitLab instance behaves identically and differs only in id, name,
URL and OIDC issuer, so EclipseGitLabTrustedPublishingProvider carried
nothing but four constants. Instances are now configuration:

  ovsx.trusted-publishing.gitlab.<id>.name/url/issuer

with the public and the Eclipse instance configured by default, so
existing deployments are unaffected. Whether an instance can be used is
still decided by ovsx.trusted-publishing.active-providers.

GitLabTrustedPublishingProviderSupport and its two subclasses collapse
into a single concrete GitLabTrustedPublishingProvider. The ci_config_ref_uri
claim is now derived from the host *and* the path of the instance URL, so
an instance served under a relative URL root registers a matchable claim.

Startup rejects an instance without a name or URL, with a malformed URL,
or taking the GitHub provider id, and warns about active provider ids
that no provider is configured for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An instance the upstream registry does not itself use has no place in the
built-in defaults; only the public GitLab instance is configured out of
the box now. Deployments that want the Eclipse Foundation instance add it
the same way as any other one:

  ovsx:
    trusted-publishing:
      active-providers: github,eclipse-gitlab
      gitlab:
        eclipse-gitlab:
          name: Eclipse GitLab
          url: https://gitlab.eclipse.org

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@netomi

netomi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Configurable GitLab instances (63c03568b, 4a1fdb862)

EclipseGitLabTrustedPublishingProvider carried nothing but four constants — every GitLab instance behaves identically and differs only in id, name, URL and OIDC issuer. So instances are configuration now, not code:

ovsx:
  trusted-publishing:
    active-providers: github,eclipse-gitlab
    gitlab:
      eclipse-gitlab:
        name: Eclipse GitLab
        url: https://gitlab.eclipse.org
        issuer: https://gitlab.eclipse.org   # optional, defaults to the URL
  • GitLabTrustedPublishingProviderSupport and its two subclasses collapse into a single concrete GitLabTrustedPublishingProvider.
  • New TrustedPublishingProperties (@ConfigurationProperties("ovsx.trusted-publishing"), same pattern as RemoteScannerProperties). Only the public gitlab instance is configured out of the box — an instance this registry does not itself use does not belong in the built-in defaults.
  • active-providers is unchanged: it still decides what is actually usable.
  • TrustedPublishingService builds GitHub plus one provider per configured instance, instead of the hardcoded Map.of(...).

Two behavioural details worth a look

  • Redefining an instance replaces it wholly. Spring's map binder builds a fresh GitLabInstance per key rather than patching the default, so gitlab.gitlab.url: … on its own would silently drop the name. Startup now rejects an instance with no name or URL, and redefiningTheDefaultInstanceReplacesItAsAWhole pins that down.
  • ci_config_ref_uri is derived from host and path. It used URI.create(providerUrl).getHost() only, so an instance served under a relative URL root (https://acme.example/gitlab) registered a claim that could never match. Output is identical for normal installs.

Startup also fails on a malformed instance URL or an instance taking the github id, and logs a warning for active-providers entries that no provider is configured for — previously those were silently inert.

Deployment note: open-vsx.org serves eclipse-gitlab today, and existing trusted_publisher rows carry that provider id. It needs the config block above once this ships, or those registrations stop being listed. The id itself is unchanged, so they reappear as soon as it is configured.

Tests: 7 new in TrustedPublishingPropertiesTest (defaults, adding an instance, whole-instance replacement, the three validation failures), 3 new in GitLabTrustedPublishingProviderTest (self-hosted instance, relative URL root, malformed URL). Full unit suite: 998 passing. Six unrelated test files needed a one-line update for the new TrustedPublishingConfig constructor.

Not addressed: the webui infers the provider kind from the id (providerId.includes('gitlab') in registration-fields.ts), so a custom instance whose id lacks "gitlab" falls back to the generic icon. Having the server send the kind would be the proper fix.

createTrustedPublisher called eclipseService.checkPublisherAgreement()
outside its try/catch. Nothing else handles ErrorResultException - no
@ControllerAdvice covers it - so a user without a signed publisher
agreement got a 500 with an empty body instead of the 403 and the
{"error": "..."} message the exception carries.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@netomi

netomi commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Publisher agreement error is a 403 again (39416689d)

Addresses the note in the description. createTrustedPublisher called eclipseService.checkPublisherAgreement(user) before its try, so the ErrorResultException it throws escaped the controller. Nothing else handles that type — ServerExceptionResolver extends DefaultHandlerExceptionResolver and ValidationExceptionHandler only covers ConstraintViolationException — so a user without a signed agreement got a 500 with an empty body instead of the 403 and the message the exception carries ("You must sign a Publisher Agreement with the Eclipse Foundation before publishing any extension.").

The check now runs inside the existing try, ahead of the field validation exactly as before, so the ordering of errors is unchanged and the existing catch (ErrorResultException) maps it through toResponseEntity(TrustedPublisherJson.class) like every other failure on this endpoint.

createTrustedPublisher_returns403_whenPublisherAgreementIsMissing covers it, and asserts the registration is not attempted. Verified it fails (500 ≠ 403) against the previous code.

Scope note: the underlying gap — an ErrorResultException that escapes any controller becomes a 500 — is repo-wide, since every controller catches it locally. A @RestControllerAdvice mapping it to its status and {"error": …} would be a safety net against this recurring, but that changes behaviour beyond trusted publishing, so I left it alone. Happy to add it if you want it.

netomi and others added 3 commits September 1, 2026 09:46
…edPublishing

The field says whether a version was published through trusted publishing,
not which trusted publisher it belongs to, and the old name reads like the
latter. Nothing consumes it yet - not the webui, not the CLI - and it has
not been in a release, so the rename breaks no client.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A rocket icon next to "Published by" says the version came from a trusted
publishing workflow rather than a personal access token, driven by the new
publishedWithTrustedPublishing field. It links to the deployment's trusted
publishing documentation when one is configured, and nothing is rendered
for the ordinary case.

The rocket is the same one that stands for trusted publishing in the user
settings; a shield would collide with the verified-publisher shield sitting
in the same row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant