← Back to PRs

#19032: fix(security): tighten permissions for cron/, browser/, settings/ in doctor --fix

by moxunjinmu open 2026-02-17 08:21 View on GitHub →
size: XS
## Problem `openclaw doctor --fix` secures `stateDir`, `configPath`, `credentials/`, and `agents/*/sessions/` but misses `cron/`, `browser/`, and `settings/` directories. These contain sensitive data: - `cron/jobs.json` — scheduled task details including payload/message content - `browser/` — session state and cookies - `settings/` — user settings These directories are created with default permissions (755/644) and never tightened, leaving them world-readable even after running `doctor --fix`. Reported in #18866, root cause confirmed by community analysis in the comments. ## Solution Add `applyPerms` calls in `fixSecurityFootguns` for: - `cron/` → 700 - `cron/jobs.json` → 600 - `cron/jobs.json.bak` → 600 - `browser/` → 700 - `settings/` → 700 Uses the existing `applyPerms` closure which handles both Unix (`chmod`) and Windows (`icacls`) platforms, and gracefully skips missing paths. ## Test Plan - Added test case "tightens perms for cron, browser, and settings directories" that creates all three directories with 755 and files with 644, runs `fixSecurityFootguns`, and asserts directories are 700 and files are 600 - All 6 tests pass (`npx vitest run src/security/fix.test.ts`) Closes #18866 <!-- greptile_comment --> <h3>Greptile Summary</h3> Adds permission tightening for `cron/`, `browser/`, and `settings/` directories in `openclaw doctor --fix`. These directories contain sensitive data (scheduled task details, browser session state/cookies, and user settings) that were previously left world-readable with default permissions (755/644). The implementation uses the existing `applyPerms` closure pattern, which handles both Unix (`chmod`) and Windows (`icacls`) platforms and gracefully skips missing paths. Test coverage validates that directories are set to 700 and files to 600 after running the security fix. **Changes:** - Secures `cron/`, `browser/`, and `settings/` directories (700) - Secures `cron/jobs.json` and `cron/jobs.json.bak` files (600) - Adds comprehensive test coverage for the new permission fixes This is a targeted security hardening fix that closes a gap in the existing `doctor --fix` security tooling. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with minimal risk - it's a focused security improvement - Score reflects that the implementation is clean, follows established patterns, and includes proper test coverage. The main concern is the incomplete coverage mentioned in the previous thread (`cron/runs/` subdirectory is not secured), but this doesn't introduce new vulnerabilities - it's an incremental improvement that addresses part of the security gap reported in #18866 - No files require special attention - the implementation is straightforward and follows existing patterns <sub>Last reviewed commit: bc5aae0</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs