← Back to PRs

#23432: Doctor: prevent permissive secret file modes during --fix

by bmendonca3 open 2026-02-22 09:45 View on GitHub →
commands size: S
## Summary - harden `openclaw doctor --fix` permission repair to tighten additional sensitive artifacts: - `~/.openclaw/.env` -> `0600` - `~/.openclaw/logs` dir -> `0700` and files -> `0600` - agent transcript files (`agents/**.jsonl`) -> `0600` - keep existing config/state tightening behavior and apply this in the same doctor repair flow - add regression coverage that asserts post-fix permissions on state/config/.env/logs/transcripts ## Validation - `pnpm test src/commands/doctor-state-integrity.test.ts` - `pnpm test:e2e src/commands/doctor.warns-state-directory-is-missing.e2e.test.ts src/commands/doctor.migrates-slack-discord-dm-policy-aliases.e2e.test.ts` - `pnpm check` <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR extends `openclaw doctor --fix` to tighten file permissions on additional sensitive artifacts beyond the existing state directory and config file checks: `~/.openclaw/.env` (0o600), `~/.openclaw/logs/` directory (0o700) and files (0o600), and agent transcript `.jsonl` files (0o600). A `listFilesRecursive` helper is added to walk directories while skipping symlinks. - New permission tightening covers `.env`, log files, and session transcript files with appropriate modes - `listFilesRecursive` correctly skips symlinks to avoid following links outside the state directory - Regression test verifies post-fix permissions on all artifact types with a cross-platform `expectPerms` helper - **Logic issue**: The permission comparison at line 380 uses exact-match (`!== target.mode`) instead of checking for excess permission bits. This means files with *more restrictive* permissions than the target (e.g., `0o400` on `.env`) would be "tightened" to `0o600`, actually *adding* write permission. The existing checks in this file correctly use `& 0o077` to only detect overly permissive modes <h3>Confidence Score: 3/5</h3> - The permission comparison logic can inadvertently loosen restrictive permissions, which is counterproductive for a security-hardening feature - Score of 3 reflects that the core intent is sound and most of the implementation is correct, but the exact-match permission comparison is a logic bug that can cause the opposite of the intended behavior in edge cases (loosening instead of tightening). The test only covers the happy path (overly permissive to correct) and doesn't catch this issue. - Pay close attention to `src/commands/doctor-state-integrity.ts` line 380 — the permission comparison logic <sub>Last reviewed commit: fc73b5a</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs