fix(plugins): allow hardlinks for bundled plugins (fixes #28175, #28404)#32119
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd496e5ab9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/plugins/discovery.ts
Outdated
| const globalDir = path.join(resolveConfigDir(), "extensions"); | ||
| discoverInDirectory({ |
There was a problem hiding this comment.
Discovering global plugins before bundled plugins flips duplicate-ID precedence, because loadOpenClawPlugins keeps the first origin it sees for a plugin ID and marks later ones as overridden. In setups where a user has ~/.openclaw/extensions/<id> and the same bundled plugin ID exists, this change makes the global plugin win and disables the bundled one, which regresses the previous trust/override behavior (and breaks the duplicate-ID flow expected by src/plugins/loader.ts around the seenIds first-wins check).
Useful? React with 👍 / 👎.
src/plugins/loader.ts
Outdated
| setActivePluginRegistry(cached, cacheKey); | ||
| return cached; |
There was a problem hiding this comment.
The cache-hit branch now returns after setActivePluginRegistry without calling initializeGlobalHookRunner, so any flow that clears the global runner while leaving the registry cache populated (for example resetGlobalHookRunner() used in loader lifecycle tests/reload paths) will return a cached registry but leave getGlobalHookRunner() null and skip hook execution.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR fixes the hardlink rejection issue for bundled plugins installed via Critical Issues Found:
Security Assessment: Confidence Score: 1/5
Last reviewed commit: 99f3da2 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (3)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugins/loader.ts
Line: 179
Comment:
incorrect field name - should be `httpRoutes` not `httpHandlers` (see PluginRecord type definition at src/plugins/registry.ts:115)
```suggestion
httpRoutes: 0,
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugins/loader.ts
Line: 383
Comment:
`initializeGlobalHookRunner(cached)` must be called here - the removed `activatePluginRegistry` function did both `setActivePluginRegistry` and `initializeGlobalHookRunner`, but now the cached path only calls the former
```suggestion
setActivePluginRegistry(cached, cacheKey);
initializeGlobalHookRunner(cached);
```
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/plugins/discovery.ts
Line: 619-627
Comment:
global plugins now discovered before bundled (order reversed from original) - previously bundled was discovered first with comment "Keep auto-discovered global extensions behind bundled plugins". Is this intentional or should bundled remain first?
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99f3da2113
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Discovery stores rootDir as realpath but source may still be a lexical alias | ||
| // (e.g. /var/... vs /private/var/... on macOS). Canonical boundary checks | ||
| // still enforce containment; skip lexical pre-check to avoid false escapes. | ||
| rejectHardlinks: candidate.origin !== "bundled", |
There was a problem hiding this comment.
This only relaxes hardlink checks for plugin entry files, but bundled plugins still pass through loadPluginManifestRegistry (src/plugins/manifest-registry.ts) where loadPluginManifest opens openclaw.plugin.json with default hardlink rejection (src/plugins/manifest.ts). In pnpm/bun global installs where package files are hardlinked, the manifest load still fails with unsafe plugin manifest path, so bundled plugins remain unavailable despite this change.
Useful? React with 👍 / 👎.
markfietje
force-pushed
the
fix/pnpm-hardlink-final-try
branch
from
da2b04c to
be3a0b1
Compare
|
This PR is strictly scoped to the pnpm hardlink fix to maintain the smallest possible change surface. The fix for the blocking CI regression is being tracked in PR #32122. Once #32122 is merged, the CI for this PR will pass. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56ff0871c8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Discovery stores rootDir as realpath but source may still be a lexical alias | ||
| // (e.g. /var/... vs /private/var/... on macOS). Canonical boundary checks | ||
| // still enforce containment; skip lexical pre-check to avoid false escapes. | ||
| rejectHardlinks: candidate.origin !== "bundled", |
There was a problem hiding this comment.
This change only disables hardlink rejection when opening plugin entry files, but bundled plugins are filtered earlier by manifest loading: loadPluginManifestRegistry still calls loadPluginManifest(candidate.rootDir), and loadPluginManifest uses openBoundaryFileSync with default rejectHardlinks: true. In pnpm/bun global installs where openclaw.plugin.json is hardlinked, bundled plugins will still be rejected as unsafe plugin manifest path, so the startup regression this patch targets remains unresolved.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ae852a71c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Discovery stores rootDir as realpath but source may still be a lexical alias | ||
| // (e.g. /var/... vs /private/var/... on macOS). Canonical boundary checks | ||
| // still enforce containment; skip lexical pre-check to avoid false escapes. | ||
| rejectHardlinks: candidate.origin !== "bundled", |
There was a problem hiding this comment.
Setting rejectHardlinks to false for bundled entry files here does not fix bundled plugin loading in pnpm/bun installs, because manifest loading still goes through loadPluginManifestRegistry → loadPluginManifest(candidate.rootDir) with hardlink rejection enabled by default (src/plugins/manifest-registry.ts, src/plugins/manifest.ts). When openclaw.plugin.json is hardlinked (common in content-addressable installs), the plugin is still rejected as an unsafe manifest path, so startup behavior remains broken in the same environments this patch targets.
Useful? React with 👍 / 👎.
|
✅ Deep Verification & Greptile Resolution I have performed a deep investigation into the automatic Greptile review and the current HEAD (
|
markfietje
force-pushed
the
fix/pnpm-hardlink-final-try
branch
from
96d1ac0 to
c0665e9
Compare
Takhoffman
force-pushed
the
fix/pnpm-hardlink-final-try
branch
from
e13dfaf to
7b6b5cd
Compare
|
PR #32119 - fix(plugins): allow hardlinks for bundled plugins (fixes #28175, #28404) (#32119) Merged after verification.
|
…ty (fixes openclaw#29525, openclaw#29072, openclaw#21918, openclaw#32937) [AI-assisted] - Add rejectHardlinks: false to openBoundaryFileSync() call in resolveSafeControlUiFile() - Matches successful pattern from PR openclaw#32119 for bundled plugins - Allows pnpm global installs to serve Control UI assets (200 OK) - Maintains all security boundary checks - All 18 existing tests pass
…ty (fixes openclaw#29525, openclaw#29072, openclaw#21918, openclaw#32937) [AI-assisted] - Add rejectHardlinks: false to openBoundaryFileSync() call in resolveSafeControlUiFile() - Matches successful pattern from PR openclaw#32119 for bundled plugins - Allows pnpm global installs to serve Control UI assets (200 OK) - Maintains all security boundary checks - All 18 existing tests pass
…ty (fixes openclaw#29525, openclaw#29072, openclaw#21918, openclaw#32937) [AI-assisted] Add rejectHardlinks: false to openBoundaryFileSync() call in resolveSafeControlUiFile(). Matches successful pattern from PR openclaw#32119 for bundled plugins. All tests pass.
…, openclaw#28404) (openclaw#32119) thanks @markfietje Verified: - pnpm install --frozen-lockfile - pnpm build - pnpm check - pnpm test:macmini Co-authored-by: markfietje <4325889+markfietje@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
…ty (fixes openclaw#29525, openclaw#29072, openclaw#21918, openclaw#32937) [AI-assisted] Add rejectHardlinks: false to openBoundaryFileSync() call in resolveSafeControlUiFile(). Matches successful pattern from PR openclaw#32119 for bundled plugins. All tests pass.
2 participants
Summary
This PR fixes the "unsafe plugin manifest path" error encountered when installing OpenClaw globally via
pnpmorbun(#28175, #28404).Problem
In v2026.2.26, strict security validation was added to plugin loading which rejects any manifest file that is a hardlink (
nlink > 1). Sincepnpmandbunuse hardlinks in their content-addressable stores for global installs, all bundled plugins were being rejected, causing the gateway to crash on startup.Solution
This PR relaxes the hardlink rejection check only for bundled plugins. Since bundled plugins are part of the trusted computing base, allowing them to be hardlinks is safe. Non-bundled plugins (workspace, config, global) retain the strict hardlink rejection for security.
Verification
upstream/mainhas unrelated regressions (gateway-chat.tstype error andsynology-chattest conflict). This PR is strictly scoped to the hardlink fix and does not attempt to fix those unrelated issues.Fixes #28175
Fixes #28404
Fixes #29455