Skip to content

feat: Implement command execution handler for server#148

Merged
inureyes merged 3 commits intomainfrom
feature/issue-128-implement-exec-handler
Jan 22, 2026
Merged

feat: Implement command execution handler for server#148
inureyes merged 3 commits intomainfrom
feature/issue-128-implement-exec-handler

Conversation

@inureyes
Copy link
Member

@inureyes inureyes commented Jan 22, 2026

Summary

  • Implement CommandExecutor for SSH exec requests
  • Add stdout/stderr streaming to SSH channel
  • Support command timeout and allow/block lists
  • Integrate with SSH handler's exec_request method

Changes

New Files

  • src/server/exec.rs - CommandExecutor implementation

Modified Files

  • src/server/mod.rs - Export exec module
  • src/server/config.rs - Add ExecConfig options
  • src/server/handler.rs - Implement exec_request handler

Implementation Details

CommandExecutor Features

  • Execute commands via configurable shell (-c flag)
  • Environment variable configuration (HOME, USER, SHELL, PATH)
  • Working directory configuration
  • Command timeout with process kill
  • Command allow/block list validation
  • Exit code propagation

Security Features

  • Command validation against blocked patterns
  • Clear inherited environment, set safe defaults
  • Logging of all command executions
  • Timeout to prevent runaway processes

Test plan

  • Unit tests for command validation
  • Unit tests for ExecConfig
  • All existing tests pass
  • Clippy passes with no warnings

Closes #128

@inureyes
feat: Implement command execution handler for server
b522454 
Implement the exec_request handler for bssh-server, enabling clients
to execute remote commands via SSH. Features include:

- CommandExecutor with configurable shell and timeout
- Stdout/stderr streaming to SSH channel
- Command allow/block list validation for security
- Exit code propagation to client
- Environment variable configuration (HOME, USER, SHELL, PATH)
- Working directory configuration
- Proper channel lifecycle (success, eof, close)

Closes #128
@inureyes inureyes added type:enhancement New feature or request priority:high High priority issue labels Jan 22, 2026
@inureyes
security: Fix critical and high security issues in command execution
a9eaa1d 
This commit addresses all CRITICAL and HIGH severity security
vulnerabilities identified in PR #148 code review.

CRITICAL Issues Fixed:
1. Command Injection via Blocklist Bypass
   - Implement normalized validation (lowercase, whitespace collapse)
   - Add regex-based pattern detection for variable expansion
   - Detect all shell metacharacters (;, &&, ||, |, $(), etc.)
   - Case-insensitive and normalized blocklist matching

2. Allowlist Validation Bypass
   - Detect and reject command chaining in allowlist mode
   - Change to exact match only (no prefix matching)
   - Prevent "ls; rm -rf /" style bypasses

3. Process Privilege Control
   - Add process group creation for better isolation
   - Document OS-level privilege control requirements
   - Use kill_on_drop for automatic cleanup

HIGH Issues Fixed:
4. Zombie Process Risk on Timeout
   - Create new process group with process_group(0)
   - Kill entire process group on timeout using kill(-pid, SIGKILL)
   - Implement fallback to immediate child kill

5. Resource Limits
   - Add documentation for OS-level resource limits
   - Recommend systemd service limits and ulimit configuration

6. Environment Variable Security
   - Block dangerous environment variables (LD_PRELOAD, LD_LIBRARY_PATH, etc.)
   - Add runtime filtering with warning logs
   - Prevent library injection and environment-based attacks

Implementation Details:
- Enhanced validation with multiple defense layers
- Comprehensive test suite with 17 passing tests
- Updated default blocklist to block command categories
- Added dangerous environment variable blocklist
- Process group management for Unix systems
- Updated dependencies: nix (process, signal features), libc

Breaking Changes:
- Default blocklist now blocks command names instead of full patterns
- Command chaining strictly blocked when allowlist is configured
- Case-insensitive command blocking (RM, Rm blocked along with rm)
- Dangerous environment variables are now filtered

Testing:
- All 17 exec module tests pass
- Comprehensive injection prevention tests added
- Blocklist normalization tests
- Allowlist bypass prevention tests
- Environment variable filtering tests
@inureyes
chore: Fix formatting and dead code warning in exec handler
0d10586 
Apply cargo fmt formatting and add #[allow(dead_code)] for test helper
@inureyes
Copy link
Member Author

inureyes commented Jan 22, 2026

PR Finalization Report

Project Structure Discovered

  • Project Type: Rust
  • Test Framework: cargo test
  • Documentation System: Plain markdown (ARCHITECTURE.md + docs/architecture/)
  • Multi-language Docs: No
  • Lint Tools: cargo fmt, cargo clippy

Checklist

Tests

  • Analyzed existing test structure (inline #[cfg(test)] modules)
  • Identified test coverage: 17 tests for exec.rs, 8 tests for handler.rs, 16 tests for config.rs
  • All tests passing (83 server module tests)
  • Security-critical code coverage verified:
    • Command validation with blocked/allowed lists
    • Command injection prevention (chaining, metacharacters, variable expansion)
    • Case-insensitive blocklist normalization
    • Allowlist exact matching
    • Dangerous environment variable blocking

Documentation

  • ARCHITECTURE.md already updated with CommandExecutor details (in PR changes)
  • docs/architecture/README.md references server module
  • Code-level rustdoc comments comprehensive

Code Quality

  • cargo fmt applied (formatting fixes)
  • cargo clippy -- -D warnings passes
  • #[allow(dead_code)] added for RejectAllAuthProvider test helper
  • All warnings resolved

Changes Made

  • Applied cargo fmt formatting to src/server/exec.rs
  • Added #[allow(dead_code)] annotation to unused test helper RejectAllAuthProvider in handler.rs

Notes

  • Test coverage for security-critical command validation is comprehensive (17 dedicated tests)
  • The CommandExecutor includes multiple layers of security:
    • Shell metacharacter detection
    • Blocked command patterns (case-insensitive)
    • Optional allowlist with exact matching
    • Dangerous environment variable blocking
    • Process group management for timeout cleanup

@inureyes inureyes merged commit 382f506 into main Jan 22, 2026
1 of 2 checks passed
@inureyes inureyes deleted the feature/issue-128-implement-exec-handler branch January 22, 2026 08:26
@inureyes inureyes self-assigned this Jan 24, 2026
@inureyes inureyes added the status:done Completed label Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@inureyes inureyes

Labels

priority:high High priority issue status:done Completed type:enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement command execution handler for server

1 participant

@inureyes