Skip to content

fix(security): harden exec permissions and remove dangerous defaults#439

Closed
OliviaPp8 wants to merge 1 commit into
TeamWiseFlow:masterfrom
OliviaPp8:fix/security-hardening
Closed

fix(security): harden exec permissions and remove dangerous defaults#439
OliviaPp8 wants to merge 1 commit into
TeamWiseFlow:masterfrom
OliviaPp8:fix/security-hardening

Conversation

@OliviaPp8

Copy link
Copy Markdown

Summary

This PR addresses several security concerns identified in a code audit:

Changes

  1. Restore command substitution blocking in patch 001

    • Keep \\ and backticks blocked to prevent arbitrary code execution
    • Still allow useful operators: &&, ||, ;, <, >\
  2. *Change IT Engineer from \security: full\ to \security: allowlist*

    • Add comprehensive \exec-approvals.json\ with sysadmin commands
    • Enable \�sk: on-miss\ for user confirmation on unlisted commands
  3. Remove dangerous browser defaults

    • Remove \dangerouslyAllowPrivateNetwork: true\ (SSRF risk)
    • Remove \�llowRfc2544BenchmarkRange: true\
  4. Add security documentation

    • New \docs/SECURITY.md\ explaining the tier system
    • Document rationale for security decisions

Rationale

Issue Risk Fix
Command substitution allowed Arbitrary code execution via allowlist bypass Block \\ and backticks
IT Engineer \security: full\ Prompt injection → full system access Use allowlist + ask
Private network browser access SSRF attacks on internal services Remove from defaults

Breaking Changes

  • IT Engineer may prompt for confirmation on some commands (previously ran everything silently)
  • Browser cannot access private IPs by default (users can re-enable if needed)

Testing

  • Verify IT Engineer can still perform common sysadmin tasks
  • Confirm command substitution is properly blocked
  • Test that allowed operators (&&, ||, etc.) still work

/cc @MilkWind

Security improvements:
- Keep command substitution ( and backticks) blocked in patch 001
- Change IT Engineer from security:full to security:allowlist
- Add IT Engineer exec-approvals.json with comprehensive sysadmin commands
- Enable ask:on-miss for IT Engineer (user confirmation for unlisted commands)
- Remove browser.ssrfPolicy.dangerouslyAllowPrivateNetwork from defaults
- Remove tools.web.fetch.ssrfPolicy.allowRfc2544BenchmarkRange from defaults
- Add docs/SECURITY.md documenting the security model

Rationale:
- Command substitution allows arbitrary code execution bypass
- security:full with ask:off is dangerous under prompt injection
- Private network access enables SSRF attacks on internal services

Breaking changes:
- IT Engineer may now prompt for confirmation on some commands
- Browser cannot access private IPs by default (add back if needed)
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants