fix(security): allow Windows read commands#2399
Conversation
📝 WalkthroughWalkthroughThis PR adds Windows read-equivalent commands ( ChangesWindows Command Allowlisting
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/config/schema/autonomy.rs`:
- Line 60: The entry "date" in the default allowlist is unsafe because it
permits clock changes; remove the "date".into() element from the default list in
autonomy.rs or alternatively update the risk-classification path that handles
command risk so that any invocation of "date" is treated as medium/high risk
(and requires approval or blocking) and ensure argument checks detect mutating
forms (e.g., non-empty args like "date 2026-05-21") in the command-arg gating
logic; locate the string "date" in the defaults and either delete that element
or add an explicit rule mapping "date" to elevated risk in the
risk-classification function used by your policy enforcement.
In `@src/openhuman/security/policy_tests.rs`:
- Around line 98-116: Expand both tests to assert the full newly-expanded
Windows read-only allowlist (including "more" and "date" in addition to "dir",
"type README.md", "where node", "findstr pattern file.txt") so we cover all
commands; update the unnamed loop test to include "more" and "date" in the
command array checked with p.is_command_allowed(command), and update
config_default_policy_includes_windows_read_equivalents to assert
p.is_command_allowed for each of those same commands created via
SecurityPolicy::from_config(&AutonomyConfig::default(),
std::path::Path::new(".")), using the existing is_command_allowed checks to
prevent partial regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80c18006-fb09-41bd-b8cb-6b8e1243e593
📒 Files selected for processing (3)
src/openhuman/config/schema/autonomy.rssrc/openhuman/security/policy.rssrc/openhuman/security/policy_tests.rs
|
@graycyrus @senamakel This PR is ready for human review/merge as well. It is a small Windows read-command allowlist hardening change; latest checks are green and CodeRabbit approved. The CodeRabbit docstring note is advisory in the walkthrough, not a failing status check. |
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Looks good, nice work!
2 participants
Summary
dir,type,where,findstr, andmore.AutonomyConfig::default()without broadening config-derived policy to allow mutatingdateusage.Problem
ls,cat,grep, andwhichdo not map to native Windows read/lookup workflows.Solution
SecurityPolicyallowlist with native Windows read/lookup equivalents.AutonomyConfig::default_allowed_commands()with the same Windows read/lookup entries, while leavingdateout of config defaults because mutating forms can change the system clock.date 2026-05-21being allowed.node_exec/npm_execquoting and should be handled as a separate, targeted follow-up if needed.Submission Checklist
## Related— N/A: no matrix feature ID applies.docs/RELEASE-MANUAL-SMOKE.md) — N/A: no release-cut surface changed.Closes #NNNin the## Relatedsection — N/A: this complements fix(tools): preserve Windows process env #2382 for Windows 11 bug #2379 and should not close the issue alone.Impact
ls,cat,grep,which, paged output). Config defaults intentionally do not adddate.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
codex/GH-2379-windows-tool-env05e8203aacce2b99d68465d113018863c97c5b09Validation Run
pnpm --filter openhuman-app format:check— passed via pre-push hook.pnpm typecheck— passed via pre-push hook (pnpm compile).GGML_NATIVE=OFF pnpm debug rust allowed_commands_include_windows_read_equivalents— log confirms 1 passed.GGML_NATIVE=OFF pnpm debug rust config_default_policy_includes_windows_read_equivalents— log confirms 1 passed.cargo fmt --manifest-path Cargo.toml --allcargo fmt --manifest-path Cargo.toml --all --checkGGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlpnpm rust:checkpassed via pre-push hook.git diff --checknode scripts/codex-pr-preflight.mjs --lightweightpnpm lintandpnpm lint:commands-tokenssuccessfully; lint emitted existing warnings only.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
SecurityPolicy::default()andAutonomyConfig::default()both include the Windows read/lookup command set; config-derived policy does not adddate.Duplicate / Superseded PR Handling