fix(plugins): resolve public artifacts from installed plugin roots#98819
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 8:56 PM ET / 00:56 UTC. Summary Reproducibility: yes. Source inspection shows current main still uses bundled-only gateway-auth artifact loading, and the linked issue plus PR body provide packaged-install before/after Mattermost evidence. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow six-file merge result after maintainer acceptance of the installed-root gateway-auth trust boundary, while keeping #98842 as the canonical follow-up for other public artifacts. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main still uses bundled-only gateway-auth artifact loading, and the linked issue plus PR body provide packaged-install before/after Mattermost evidence. Is this the best way to solve the issue? Yes, with maintainer acceptance. A consumer-scoped activation-gated facade resolver is narrower and safer than adding a broad installed-root fallback to the shared public-surface loader. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f59310a555d7. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Superseded by the activation-gated facade seam in gateway-auth-bypass; review found the loader-level fallback broadened every artifact surface past its trust boundary (see PR #98819 discussion).
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Land-ready verification (head
Adversarial review complete: no open findings; disabled/denied plugins can no longer contribute gateway auth bypass paths or execute artifact code. Follow-up for the remaining bundled-only artifact surfaces: #98842. |
|
Merged via squash.
|
…penclaw#98819) * fix(plugins): resolve public artifacts from installed plugin roots Externalized official plugins ship gateway-auth/doctor/secret/message-tool public artifacts in their installed npm package, but the public-surface loader only searched bundled locations. On packaged installs this emptied the gateway auth bypass set for /api/channels/mattermost/command, so every Mattermost slash callback was rejected 401 before the plugin ran. Fall back to the plugin's installed package root (install-record index) when bundled resolution misses; bundled/source locations keep precedence. Fixes openclaw#98740 * fix(gateway): resolve channel auth-bypass artifacts via activation-gated facade Route gateway-auth bypass artifact loading through the activation-gated facade seam so externalized (installed) channel plugins keep their unauthenticated callback paths, while denied/disabled plugins contribute nothing and never execute artifact code. Reject hardlinked artifacts for installed plugin roots in the facade loader per hardlink-policy. Replaces the earlier install-record fallback in the shared bundled artifact loader, which broadened every artifact surface past its trust boundary (see PR review); other externalized-plugin surfaces are openclaw#98842. Fixes openclaw#98740 * fix(gateway): load facade activation runtime via async lazy import The sync createRequire/jiti candidate loader for facade-activation-check.runtime.js only resolves from built dist; under vitest source runs it cannot follow .js->.ts specifiers or workspace package subpaths, so the gateway auth-bypass path threw 'Unable to load facade activation check runtime' (CI shard failure). Add an async activated-load API backed by dynamic import (vitest- and dist-native), warm the shared runtime memo, and make the bypass consumer async - its single caller already awaits behind the config-snapshot promise cache. * revert(plugins): drop install-record fallback from public-surface loader Superseded by the activation-gated facade seam in gateway-auth-bypass; review found the loader-level fallback broadened every artifact surface past its trust boundary (see PR openclaw#98819 discussion). * fix(plugin-sdk): drop unused oxlint disable on async facade loader * test(plugin-sdk): cover facade hardlink policy for installed vs core roots * test(plugin-sdk): cover facade hardlink policy for installed vs core roots --------- Co-authored-by: Alex Knight <[email protected]>
Fixes #98740
What Problem This Solves
After the 2026.6.11 Mattermost plugin externalization, every native
/oc_*slash command callback is rejected with401by the gateway before the plugin's handler runs./api/channels/*is a protected plugin-route prefix (src/gateway/security-path.ts), and the only exemption — the channel'sgateway-auth-api.jspublic artifact — was resolved by a bundled-only loader.[email protected]no longer shipsdist/extensions/mattermost/**(externalized,bundledDist: false), so the bypass set resolved empty and gateway bearer auth was enforced against Mattermost's callbacks. Outbound registration and WebSocket DMs/@mentions were unaffected, matching the reporter's symptoms exactly.Why This Change Was Made
The gateway-auth bypass consumer (
src/channels/plugins/gateway-auth-bypass.ts) now resolves the artifact through the existing activation-gated facade seam (src/plugin-sdk/facade-runtime.ts) instead of the bundled-only public-surface loader. The consumer uses a new async twin (tryLoadActivatedBundledPluginPublicSurfaceModule) of the existing sync seam: the activation-check runtime is a lazy*.runtime.tsboundary whose synccreateRequire/jiti candidate loader only resolves from built dist, so the async call site loads it via dynamicimport()(works in both dist and source/vitest) and warms the shared memo for the sync consumers. Internal-only surface —facade-runtimeis not a published SDK subpath, so no baseline/docs changes are involved.@openclaw/mattermostcase — its published package shipsdist/gateway-auth-api.js.plugins.deny), disabled (entries.<id>.enabled=false), or excluded by an allowlist returnsnull— no bypass paths and, critically, no execution of the disabled plugin's artifact code. Bypass paths grant unauthenticated ingress, so they follow plugin activation, not mere installation.Unable to resolve bundled plugin public surfacesentinel for activated-but-artifactless channels, so the optional-artifact semantics of every other consumer are untouched.src/plugin-sdk/facade-loader.tsadditionally appliesshouldRejectHardlinkedPluginFiles(src/plugins/hardlink-policy.ts) when the resolved boundary root is an installed plugin root (core-shipped/bundled roots keep hardlinks allowed, Nix store excepted). This closes a pre-existing gap in the facade seam that matters more now that a security-granting artifact resolves through it, and aligns it with the sibling installed-artifact loader insrc/secrets/channel-contract-api.ts.Deliberately not changed: the shared bundled-artifact loader (
src/plugins/public-surface-loader.ts). An earlier draft of this PR added an install-record fallback there; an 8-angle review found that broadened all eight artifact surfaces (doctor/secret/message-tool/provider-policy/…) past their intended trust boundaries, executed disabled plugins' artifact code, let store-corruption errors escape into the gateway request path, and re-implemented the facade seam with drifting precedence. That draft was reverted in favor of this consumer-scoped fix. Externalized-plugin support for the other artifact surfaces is explicit follow-up work with per-surface trust review.Scope note: a bundled channel that is configured but explicitly disabled now also stops contributing bypass paths (previously its artifact loaded regardless of enablement). A disabled plugin registers no routes, so those callbacks 404ed anyway — this only makes the auth surface consistent with plugin lifecycle.
User Impact
Mattermost native slash commands work again on packaged installs ≥2026.6.11 with the externalized
@openclaw/mattermostplugin. Disabled/denied plugins can no longer contribute gateway auth bypass paths or have artifact code executed. No behavior change for source checkouts or enabled bundled plugins.Evidence
Tests —
gateway-auth-bypass.test.tsre-pointed to the facade seam plus a new activation-blocked case (4/4);facade-loader.test.ts(7/7),facade-runtime.test.ts(17/17 — installed-package dist resolution, rootDir-basename matching, activation blocking, plus new hardlink-policy coverage: installed roots reject hardlinked artifacts, core-shipped roots keep loading them),public-surface-loader.test.tsrestored to main (8/8).pnpm buildclean (noINEFFECTIVE_DYNAMIC_IMPORT),pnpm check:changedgreen via Testbox.Packaged repro matrix (real npm installs, isolated state dirs, gateway token auth, external plugin installed via
openclaw plugins install @openclaw/mattermost; probe simulates the Mattermost callback — form-encoded POST, no gateway bearer):Live Mattermost round-trip (Mattermost team edition 11.7.0 on a separate tailnet host, real bot, this branch in the externalized topology, commands registered by the monitor):
The MM server invoked the callback; the gateway admitted it via the activation-gated installed-root bypass, the plugin validated the payload token against its startup token and the live MM command record, and channel authorization correctly denied the unallowlisted channel. A bogus-token curl against the same instance returns the plugin's fail-closed
401 "Unauthorized: invalid command token."— the per-command token guard is intact.(Reproducer note: Mattermost refuses webhook targets on private/CGNAT addresses unless listed in
ServiceSettings.AllowedUntrustedInternalConnections; unrelated to this fix but easy to trip over in tailnet/LAN deployments.)Follow-up work: externalized plugins' other public artifacts (doctor contract, secret contract, message-tool, thread-binding, media inbound roots, health checks) still resolve bundled-only; tracking issue to route each through the activation-gated seam with per-surface trust review.
In-the-wild before/after — a second, independent machine running the reporter's exact environment (packaged
openclaw 2026.6.11 (e085fa1)from npm, externalized@openclaw/mattermostat~/.openclaw/npm/projects/openclaw-mattermost-41605c6faa, Mattermost team edition 11.7.0 in Docker/colima on the same host, multi-account channel config):Native slash commands went from structurally broken to fully working on that install with no plugin change — only the core fix.