# Security Review: scheduler.rs - IcyCompass

## Review Date
2026-04-17

## File Examined
`/data/projects/pi_agent_rust/src/scheduler.rs` - Deterministic event loop scheduler for PiJS runtime

## Imports/Dependencies
- Used by: `extension_dispatcher.rs`, `extensions_js.rs`, `hostcall_amac.rs`, `extensions.rs`
- Core component for extension runtime scheduling

## Security Bug Hunt Results

Applied systematic security review for the following patterns:

### ✅ 1. Missing saturating_add
**Status**: GOOD - Already using `saturating_add()` correctly
- **Location**: Line 38 - `Self(self.0.saturating_add(1))`
- **Analysis**: Proper overflow protection in sequence counter

### ✅ 2. Raw == on hashes/MACs
**Status**: GOOD - No hash comparisons found
- **Analysis**: No cryptographic operations in this scheduler

### ✅ 3. Fail-open > instead of >=  
**Status**: GOOD - Boundary checks are correct
- **Location**: Line 471 - `if entry.deadline_ms > now` (correct for timer firing logic)
- **Location**: Line 865 - `if self.queue.len() >= self.capacity` (correct capacity check)

### ✅ 4. Unbounded Vec::push
**Status**: GOOD - No unbounded growth
- **Analysis**: All push operations are bounded by scheduler design
- **Locations**: BinaryHeap::push() for timers, VecDeque::push_back() for tasks - both processed regularly

### ✅ 5. let _ = hiding Result
**Status**: GOOD - Only found in test code with explicit error handling
- **Analysis**: Test code uses `let _ =` with `.expect()` calls, which is acceptable

### ✅ 6. .unwrap() on fallible ops
**Status**: GOOD - Only in justified cases
- **Location**: Line 475 - `.expect("peeked")` is safe (just verified entry exists)
- **Analysis**: All other unwraps are in test code

### ✅ 7. Path traversal
**Status**: N/A - No file path operations in scheduler

### ✅ 8. NaN/Inf arithmetic unguarded
**Status**: N/A - No floating point arithmetic

## Cross-Review: Recent Commit (b3974d21)

### File: package_manager.rs 
**Change**: Fix to preserve lockfile when settings.json is missing

### Security Analysis
- **Path operations**: Uses safe `join()` with hardcoded paths like "keep-me", ".pi"
- **Error handling**: Proper `.expect()` usage in test code only  
- **Input validation**: Extensive path traversal protection already in place
- **Channel usage**: `let _ = tx.send()` pattern is acceptable (error handled at recv end)

### ✅ No security issues found in the commit

## Overall Assessment

Both the `scheduler.rs` file and the recent `package_manager.rs` commit demonstrate excellent security practices:

- Proper arithmetic overflow protection
- Appropriate error handling patterns  
- No unsafe code usage
- Good boundary condition handling
- Strong path traversal protections (in package_manager)

**No security fixes required.**

## Build Verification
Running `rch exec -- cargo check --all-targets && clippy && test` to verify compilation integrity.