← Back to PRs

#21119: Security/Browser: fail closed when control server has no auth

by bmendonca3 open 2026-02-19 17:55 View on GitHub →
size: S
## Summary - fail closed in browser control startup when no gateway auth is resolved - require explicit break-glass env (`OPENCLAW_UNSAFE_ALLOW_BROWSER_CONTROL_NO_AUTH=1`) to allow unauthenticated browser control outside test-like environments - add unit coverage for the new no-auth override policy helper ## Testing - `pnpm test src/browser/control-auth.auto-token.test.ts` *(fails in this environment: `pnpm` not found)* <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR implements a fail-closed security policy for the browser control server, preventing it from starting without authentication unless explicitly overridden. **Key Changes:** - Added `shouldAllowUnsafeBrowserControlNoAuth()` helper that allows unauthenticated browser control only in test-like environments or when a break-glass environment variable is set - Modified `startBrowserControlServerFromConfig()` to return `null` and log an error when no auth is configured and the override is not set - Added comprehensive unit tests covering the new policy helper - Updated CHANGELOG.md with security fix details **Security Analysis:** The implementation correctly prevents a potentially dangerous scenario where the browser control server could start without authentication. The fail-closed approach is appropriate for security-sensitive features. The break-glass escape hatch with an explicit, clearly-named environment variable makes it obvious this is unsafe and intended for short-lived use only. **Implementation Quality:** The code follows existing patterns in the codebase, properly handles test environments (via `NODE_ENV=test` and `VITEST` detection), and integrates cleanly with the existing auth resolution flow. Test coverage is thorough and validates all branches of the new logic. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with only a minor typo to fix - The security improvement is well-implemented with proper fail-closed behavior, comprehensive test coverage, and correct handling of test environments. The only issue found is a minor grammatical typo in an error message that doesn't affect functionality. - No files require special attention beyond fixing the typo in `src/browser/server.ts:56` <sub>Last reviewed commit: 6cef266</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs