feat: Security fixes for executable validation and state file permissions - #8
feat: Security fixes for executable validation and state file permissions#8Patoruzuy wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the executable validation test suite to accommodate the newly enforced security check that rejects executables located in user-writable paths.
Changes:
- Updated
test_validate_caddy_successto mockis_user_writableso the test “caddy” binary in a temp directory is treated as safe. - Adjusted the test function signature to accept the additional patch mock.
| # Mock writability check to return safe (not user-writable) | ||
| mock_is_writable.return_value = (False, None) | ||
|
|
There was a problem hiding this comment.
Consider asserting that the writability check was actually invoked (e.g., mock_is_writable.assert_called() / assert called with the Path for test_file) so this test doesn’t silently pass if validate_executable() stops performing the security check or the patch target becomes stale.
| @patch("devhost_cli.executable_validation.is_user_writable") | ||
| @patch("subprocess.run") | ||
| def test_validate_caddy_success(self, mock_run): | ||
| def test_validate_caddy_success(self, mock_run, mock_is_writable): |
There was a problem hiding this comment.
The PR title/description says this is a security fix for executable validation and state file permissions, but this change is only updating a unit test by mocking is_user_writable. If the intended security behavior changed in production code, that code change isn’t reflected here; otherwise, consider updating the PR title/description to match the actual scope.
Fixes test failures by properly handling security checks