Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the THOR Seed script by renaming the -UseThorCloud parameter to -UseCloud for consistency, improving error handling, and adding validation and retry logic. The changes enhance the script's reliability and user experience.
Changes:
- Renamed parameter from
-UseThorCloudto-UseCloudthroughout the codebase - Added validation for CustomUrl parameter and improved error handling with retry logic for downloads
- Enhanced OS detection with fallback mechanisms and improved logging throughout the script
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| thor-seed/thor-seed.ps1 | Renamed parameter, added URL validation, improved error handling with retries, enhanced OS detection fallback, added SSL callback restoration, improved logging and summary output |
| thor-seed/README.md | Updated documentation to reflect parameter rename and corrected spelling errors, added unblocking instructions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
thor-seed/thor-seed.ps1
Outdated
| [Alias('CU')] | ||
| [string]$CustomUrl, | ||
|
|
||
| [Parameter(HelpMessage = 'Add a random sleep delay to the scan start to avoid all scripts starting at the exact same second')] |
There was a problem hiding this comment.
The ValidateRange attribute replaces ValidateNotNullOrEmpty for RandomDelay. However, ValidateRange already prevents null values for value types like [int], making the previous validation redundant. Consider documenting the 0-3600 range (0-60 minutes) in the parameter's HelpMessage to clarify the acceptable values for users.
| [Parameter(HelpMessage = 'Add a random sleep delay to the scan start to avoid all scripts starting at the exact same second')] | |
| [Parameter(HelpMessage = 'Add a random sleep delay in seconds (0-3600, i.e. 0-60 minutes) to the scan start to avoid all scripts starting at the exact same second')] |
thor-seed/thor-seed.ps1
Outdated
| # THOR still running -------------------------------------------------- | ||
| # --------------------------------------------------------------------- | ||
| $ThorProcess = Get-Process -Name "thor64" -ErrorAction SilentlyContinue | ||
| $ThorProcess = Get-Process -Name "thor64", "thor" -ErrorAction SilentlyContinue | Select-Object -First 1 |
There was a problem hiding this comment.
The improved process detection now checks for both 'thor64' and 'thor' processes. Consider whether the order matters (thor64 before thor) if both processes exist simultaneously, as Select-Object -First 1 will only return the first match found.
| $ThorProcess = Get-Process -Name "thor64", "thor" -ErrorAction SilentlyContinue | Select-Object -First 1 | |
| $ThorProcess = Get-Process -Name "thor64" -ErrorAction SilentlyContinue | Select-Object -First 1 | |
| if (-not $ThorProcess) | |
| { | |
| $ThorProcess = Get-Process -Name "thor" -ErrorAction SilentlyContinue | Select-Object -First 1 | |
| } |
thor-seed/thor-seed.ps1
Outdated
| throw "Downloaded package too small ($FileSize bytes) - likely corrupted or error response" | ||
| } | ||
| # Verify ZIP header (PK signature = 0x504B) | ||
| $ZipHeader = [System.IO.File]::ReadAllBytes($TempPackage)[0..1] |
There was a problem hiding this comment.
ReadAllBytes loads the entire file into memory before extracting the first 2 bytes. For potentially large ZIP files, this is inefficient. Consider using a FileStream to read only the first 2 bytes: $stream = [System.IO.File]::OpenRead($TempPackage); $ZipHeader = New-Object byte[] 2; $stream.Read($ZipHeader, 0, 2); $stream.Close()
| $ZipHeader = [System.IO.File]::ReadAllBytes($TempPackage)[0..1] | |
| $ZipHeader = New-Object byte[] 2 | |
| $stream = [System.IO.File]::OpenRead($TempPackage) | |
| $null = $stream.Read($ZipHeader, 0, 2) | |
| $stream.Close() |
Codehardt
left a comment
Codehardt
left a comment
There was a problem hiding this comment.
The changes regarding UseThorCloud and the newComment parameter look good. Unfortunately, I can't comment on the rest as I don't speak Powershell.
Implements robust failure detection and analysis throughout the THOR download and execution workflow. Introduces exit code tracking, failure reason capture, and automated diagnosis of scan termination causes including crashes, resource safeguards, and external interruptions. Adds CPU limit control and resource safeguard override options to provide flexibility in managing system impact during scans. Includes preflight checks for administrator privileges and output path writability to catch configuration issues early. Enhances user guidance with contextual error messages and actionable recommendations based on detected failure patterns. Improves cleanup logic to preserve artifacts when troubleshooting is needed. Strengthens Analysis Cockpit upload workflow with better error handling for missing logs and failed uploads. Adds parameter validation guards against common user mistakes like typos in command-line flags. Expands documentation with comprehensive usage examples covering ASGARD, cloud, and custom deployment scenarios.
2 participants
DRAFT