Skip to content

Conversation

@Pierozi
Copy link
Contributor

@Pierozi Pierozi commented Jan 28, 2026

Summary

  • Add comprehensive path injection prevention with SeatbeltError enum and validation
  • Replace Arc<Mutex> antipattern with idiomatic owned TraceOutput enum
  • Convert expect() calls to proper Result types with ProfileError and ExecutionError for better error chaining
  • Eliminate silent failures by adding warning logs for security-critical operations (profile loads, process cleanup)

Security Improvements

  • Path Injection Prevention: Validate all paths for unescaped quotes, newlines, and null bytes before Seatbelt interpolation
  • Type-Safe Violations: Replace magic ANSI strings with ViolationKind enum for compile-time safety
  • Error Visibility: Log warnings instead of silently skipping failed profile loads or process operations
  • Performance: Optimize config merging from O(n²) to O(n) using HashSet

Test plan

  • All 185 tests pass
  • cargo build --release succeeds
  • cargo clippy reports no critical warnings
  • Path validation tests cover injection vectors (quotes, newlines, null bytes)
  • Error types properly chain with source() implementation
  • Profile load warnings logged appropriately for builtin vs custom profiles

Files Changed

  • Core security: src/sandbox/seatbelt.rs (path validation), src/sandbox/trace.rs (error handling)
  • Error handling: src/config/profile.rs, src/sandbox/executor.rs
  • Performance: src/config/merge.rs (O(n) optimization)
  • Code quality: Remove duplicate NetworkMode, unused dependencies

Ready for open-source release.

- Add SeatbeltError enum with path injection prevention
  - Validates paths for unescaped quotes, newlines, null bytes
  - Prevents malicious paths from breaking Seatbelt syntax
  - All path interpolation now validates before use

- Replace Arc<Mutex<File>> with owned TraceOutput enum
  - Simplifies violation trace output handling
  - Removes unnecessary thread-safe wrapper pattern
  - More idiomatic Rust for single-owner code

- Add ViolationKind enum for type-safe categorization
  - Replace 130+ chars of magic ANSI strings with compile-time safety
  - Supports colored() and plain() output methods
  - Cleaner, more maintainable violation reporting

- Convert expect() to Result types throughout codebase
  - ProfileError: Io, Parse, InvalidBuiltin variants
  - ExecutionError: wraps IO and Seatbelt errors
  - SeatbeltError: path validation failures
  - Proper error chaining with source() implementation

- Improve error logging for security-critical operations
  - Log warnings for failed profile loads (not silent skipping)
  - Add zshrc write failure warning instead of silent ignore
  - Implement one-time violation trace write error logging

- Improve zombie process handling
  - Better error handling for kill/wait operations
  - Proper result propagation instead of silent swallowing

- Optimize config merging performance
  - Change merge_unique() from O(n²) to O(n)
  - Use HashSet for O(1) path lookups instead of contains()

- Remove code quality issues
  - Delete duplicate NetworkMode enum (re-export from config::schema)
  - Remove dangling profile_path reference in ExecutionResult
  - Remove unused helper functions and dependencies

Testing: All 185 tests pass. Code ready for open-source release.
The commit.github.username variable triggers GitHub API requests that fail with 404 errors when the repository is not accessible. Remove this reference to allow changelog generation to succeed with local git data only.
}
}

// Profile not found - this is not an error, it might be an unknown name
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We must add a notice about profile not found

- Add warning when unknown profile is specified instead of silent skipping
- Fallback to 'online' profile for unknown names to prevent unexpected network blocking
- Remove references to removed profiles (node, python, go, git) from help text
- Update shell integrations (bash, zsh, fish) to list only available profiles
- Remove deprecated aliases (sxn for node, sxp for python)
- Update init template and tests to reflect actual builtin profiles

Available profiles: base, online, localhost, rust, claude, gpg

All 185 tests pass.
@Pierozi Pierozi merged commit 0b86463 into main Jan 28, 2026
5 of 6 checks passed
@Pierozi Pierozi deleted the feat/rust-qa branch January 28, 2026 18:56
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.

2 participants