← Back to PRs

#23574: security: P0 critical remediation — plugin sandbox, password hashing, default-deny

by lumeleopard001 open 2026-02-22 13:11 View on GitHub →
gateway extensions: lobster size: L
## Summary Critical security hardening for the OpenClaw plugin system and gateway authentication. This PR addresses 5 P0 (CRITICAL) findings from a comprehensive security audit. ### Changes - **Default-deny for plugins** — Non-bundled plugins are now disabled unless explicitly allowed in config. The `allow` list acts as both a filter and an enable gate. - **Environment isolation** — Plugins receive a filtered copy of `process.env` with all sensitive credentials (API keys, tokens, secrets) stripped. 50+ prefixes and 9 suffixes are blocked. - **Capability guard** — Non-bundled plugins are restricted from `writeConfigFile` and `runCommandWithTimeout`. Bundled plugins retain full runtime access. - **Bcrypt password hashing** — Gateway authentication now uses bcrypt (cost factor 12) instead of plaintext comparison. Legacy plaintext passwords are supported with timing-safe comparison and flagged for rehash. - **Prototype pollution detection** — Takes a snapshot of monitored prototypes before plugin loading and detects any additions after registration. `JSON`, `Math`, and `Reflect` are frozen. ### New files | File | Purpose | |------|---------| | `src/plugins/env-sandbox.ts` | Environment variable filtering for plugins | | `src/plugins/capability-guard.ts` | Runtime capability restrictions | | `src/security/password-hash.ts` | Bcrypt hashing with legacy fallback | | `src/plugins/globals-freeze.ts` | Prototype pollution detection | ### Modified files | File | Change | |------|--------| | `src/plugins/config-state.ts` | Default-deny logic + allowlist enables | | `src/plugins/registry.ts` | Integrated env sandbox + capability guard | | `src/plugins/types.ts` | Added `env` field to `OpenClawPluginApi` | | `src/plugins/loader.ts` | Integrated prototype pollution detection | | `src/gateway/auth.ts` | Replaced plaintext comparison with bcrypt | | `package.json` | Added `bcryptjs` dependency | ## Test plan - [x] 46 new tests added across 4 new test files - [x] All 424 existing tests pass (`pnpm vitest run`) - [x] Default-deny verified: unknown plugins disabled, allowlisted plugins enabled - [x] Env sandbox verified: API keys stripped, safe vars preserved - [x] Capability guard verified: non-bundled plugins blocked from dangerous operations - [x] Bcrypt hashing verified: hash + verify + legacy fallback + rehash detection - [x] Prototype pollution verified: detection of added properties + safe globals frozen ## Security considerations - No breaking changes for existing configurations with explicit `allow` lists - Legacy plaintext passwords continue to work but are flagged with `needsRehash: true` - Prototype freezing limited to `JSON`/`Math`/`Reflect` (safe) — `Object.prototype` uses detection instead of freezing to avoid breaking libraries - All sensitive env filtering uses uppercase normalization to prevent case-based bypasses <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR implements critical security hardening for the OpenClaw plugin system and gateway authentication. The changes introduce defense-in-depth measures addressing P0 security findings through default-deny policies, environment isolation, capability restrictions, password hashing, and prototype pollution detection. **Major improvements:** - Non-bundled plugins now require explicit allowlisting, preventing unvetted code from auto-loading - Environment variables are sanitized to strip API keys and credentials from plugin access (50+ prefixes blocked) - Runtime capability restrictions prevent non-bundled plugins from executing shell commands or writing config files - Gateway authentication upgraded from plaintext to bcrypt (cost factor 12) with timing-safe legacy fallback - Prototype pollution detection monitors Object/Array/Function/String prototypes and freezes JSON/Math/Reflect globals **Code quality:** - Well-structured with clear separation of concerns across new modules - Comprehensive test coverage (46 new tests) validating all security controls - Backwards-compatible design maintains support for existing configurations <h3>Confidence Score: 4/5</h3> - This PR is generally safe to merge with one minor timing consideration for legacy password migration - Score reflects solid security hardening with comprehensive test coverage and well-designed architecture. The one concern is the legacy password fallback using SHA-256 hashing before `timingSafeEqual`, which while timing-safe for comparison, doesn't provide the same computational hardness as bcrypt against offline brute-force attacks during the migration window. This is a reasonable tradeoff for backwards compatibility but should be time-boxed. - Pay attention to `src/security/password-hash.ts:38-40` for the legacy password comparison logic <sub>Last reviewed commit: 0507223</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs