#16320: security: harden default tool policies and secure shell execution

## Security Improvements This PR implements several security hardenings identified during a recent audit. ### 1. Default Tool Policy Hardening (Critical) - **Problem**: The system previously defaulted to "allow-all" tools if no specific policy was configured for a group or sender. This potentially exposed dangerous tools (like `exec`) to untrusted users in default configurations. - **Fix**: Changed `isToolAllowedByPolicyName` in `src/agents/pi-tools.policy.ts` to default to `false` (deny-all). - **Context**: Added `senderIsOwner` parameter to policy checks. Only verified owners bypass the policy check when no policy is present. All other users are denied by default if no explicit policy allows access. ### 2. Privilege Escalation Fix (High) - **Problem**: Unauthorized `/exec` directives (e.g. `host=gateway`) persisted in the session context even when the sender was not authorized to run commands. - **Fix**: Updated `src/auto-reply/reply/get-reply-directives.ts` to explicitly clear `hasExecDirective` and related fields when `commandAuthorized` is false. ### 3. Secure Windows Shell Execution (High) - **Problem**: `runCommandWithTimeout` used `shell: true` implicitly for non-exe files on Windows, which is vulnerable to command injection via shell metacharacters in arguments. - **Fix**: Refactored `src/process/exec.ts` to explicitly detect batch files (`.cmd`, `.bat`) and execute them using `cmd.exe /c` with `windowsVerbatimArguments: true`. All other executions now use `shell: false`. ### 4. Audit Enhancements (Medium) - **Problem**: `trusted-proxy` auth mode is insecure if `trustedProxies` is empty, allowing header spoofing. - **Fix**: Added a critical check in `src/security/audit.ts` to flag empty `trustedProxies` configuration when `trusted-proxy` mode is enabled. ### Verification - `pnpm build` passes. - Existing tests should pass (no logic changes for configured policies). - Manual verification of Windows execution and directive parsing logic. Thanks to @SuccessSoham for the contribution. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR hardens security defaults across tool policies, exec directive handling, Windows shell execution, and audit messaging. The most impactful change is the deny-by-default policy in `isToolAllowedByPolicyName` when no explicit policy is configured, with an `senderIsOwner` bypass for verified owners. All call sites have been updated to propagate the new parameter. - **Default tool policy** (`pi-tools.policy.ts`): Changed from allow-all to deny-all when no policy exists; only verified owners bypass. The `senderIsOwner` parameter is correctly threaded through `filterToolsByPolicy`, `isToolAllowedByPolicies`, and `applyToolPolicyPipeline`. - **Exec directive clearing** (`get-reply-directives.ts`): Unauthorized senders now have exec directive fields cleared to prevent privilege escalation of `/exec` directives persisting in session context. - **Windows shell hardening** (`exec.ts`): Replaces implicit `shell: true` with explicit `cmd.exe /c` for `.cmd`/`.bat` files and `shell: false` everywhere else. Caller-supplied `windowsVerbatimArguments` is honored via `||` fallback. - **Audit message** (`audit.ts`): The updated detail text for empty `trustedProxies` is factually incorrect — the gateway rejects all requests in this case (see `src/gateway/auth.ts:318-319`), it does not allow identity spoofing. This should be corrected before merge. - **Audit extras** (`audit-extra.async.ts`, `audit-extra.sync.ts`): Pass `senderIsOwner: true` to preserve worst-case exposure modeling in audit checks. <h3>Confidence Score: 3/5</h3> - Generally safe security hardening, but contains one factually incorrect audit message that could mislead operators. - The core policy changes (deny-by-default, senderIsOwner threading) are sound and all call sites are updated. The exec directive clearing and Windows shell hardening are correct. However, the audit.ts change introduces an incorrect description: it claims empty trustedProxies allows identity spoofing, when the actual code rejects all requests. This misinformation in a security audit tool could lead to wrong operational decisions. - src/security/audit.ts — the updated finding detail text is factually incorrect and contradicts the actual runtime behavior in src/gateway/auth.ts _{Last reviewed commit: fba4dee} <!-- greptile_other_comments_section --> <!-- /greptile_comment -->