Skip to content

feat: Implement command execution handler for server #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
inureyes merged 3 commits into from
Jan 22, 2026
Merged

feat: Implement command execution handler for server #148

inureyes merged 3 commits into from
Jan 22, 2026

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